Re: [PATCH v4 08/20] clk: scmi: check the clock state/parent/rate control permissions

On 1/15/25 2:28 PM, Alice Guo wrote:

[...]

+static int scmi_clk_get_permissions(struct udevice *dev, int clkid, u32 *perm) +{

• u32 version;
• int ret;
• ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK, &version);
• if (ret) {

• debug("get SCMI clock management protocol version failed\n");
  

• return ret;
  

• }
• if (version >= CLOCK_PROTOCOL_VERSION_3_0) {

if (version < CLOCK_PROTOCOL_VERSION_3_0) return -EINVAL;

• struct scmi_clk_get_permissions_in in = {
  

• 	.clock_id = clkid,
  

• };
  

[...]

+static int scmi_clk_set_parent(struct clk *clk, struct clk *parent) +{

• struct clk_scmi *clkscmi;
• struct clk *c;
• int ret;
• if (CONFIG_IS_ENABLED(CLK_CCF)) {

Either do this to reduce indent:

if (!CONFIG_IS_ENABLED(CLK_CCF)) return -ENOTSUPP;

Or even better, introduce dedicated clock ops structure for the CCF variant with these extra callbacks .

• ret = clk_get_by_id(clk->id, &c);
  

• if (ret)
  

• 	return ret;
  

• clkscmi = container_of(c, struct clk_scmi, clk);
  

• if (clkscmi->ctrl_flags & SUPPORT_CLK_PARENT_CONTROL)
  

• 	return __scmi_clk_set_parent(clk, parent);
  

• /* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */
  

• debug("SCMI CLOCK: the clock's parent cannot be changed by the agent.\n");
  

• return 0;
  

• }
• return -ENOTSUPP;

+}

[...]

@@ -858,6 +861,27 @@ struct scmi_clk_parent_set_out { s32 status; };

+/**

• • @clock_id: Identifier for the clock device.
• */

+struct scmi_clk_get_permissions_in {

• u32 clock_id;

+};

+/**

• • @status: Negative 32-bit integers are used to return error status codes.
• • @permissions: Bit[31] Clock state control, Bit[30] Clock parent control,

• • 			Bit[29] Clock rate control, Bits[28:0] Reserved, must be zero
    

Please make sure the indent is consistent (it seems there are too many tabs here) and does not overflow 80 characters line length .