> -----邮件原件-----
> 发件人: Marek Vasut <ma...@denx.de>
> 发送时间: 2025年3月22日 10:02
> 收件人: Alice Guo (OSS) <alice....@oss.nxp.com>; Tom Rini
> <tr...@konsulko.com>; Lukasz Majewski <lu...@denx.de>; Sean Anderson
> <sean...@gmail.com>; Simon Glass <s...@chromium.org>; Stefano Babic
> <sba...@denx.de>; Fabio Estevam <feste...@gmail.com>; dl-uboot-imx
> <uboot-...@nxp.com>; Alper Nebi Yasak <alpernebiya...@gmail.com>; Alice
> Guo <alice....@nxp.com>; Lothar Waßmann <l...@karo-electronics.de>
> 抄送: u-boot@lists.denx.de; Ye Li <ye...@nxp.com>; Peng Fan
> <peng....@nxp.com>
> 主题: Re: [PATCH v8 07/19] clk: scmi: check the clock state/parent/rate control
> permissions
>
> On 3/21/25 8:15 AM, Alice Guo (OSS) wrote:
>
> [...]
>
> > +static int scmi_clk_get_permissions(struct udevice *dev, int clkid,
> > +u32 *perm) {
> > + u32 version;
> > + int ret;
> > +
> > + struct scmi_clk_get_permissions_in in = {
> > + .clock_id = clkid,
> > + };
> > + struct scmi_clk_get_permissions_out out;
> > + struct scmi_msg msg = {
> > + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > + .message_id = SCMI_CLOCK_GET_PERMISSIONS,
> > + .in_msg = (u8 *)&in,
> > + .in_msg_sz = sizeof(in),
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > +
> > + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK,
> > +&version);
>
> Would it be possible to query the version in .probe() once and cache the
> value in
> private data ?
>
> > + if (ret) {
> > + log_debug("%s: get SCMI clock management protocol version
> failed\n", __func__);
> > + return ret;
> > + }
> > +
> > + if (version < CLOCK_PROTOCOL_VERSION_3_0) {
> > + log_debug("%s: SCMI clock management protocol version is less
> > than
> 3.0.\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + ret = devm_scmi_process_msg(dev, &msg);
> > + if (ret) {
> > + log_debug("%s: get SCMI clock management protocol permissions
> failed\n", __func__);
> > + return ret;
> > + }
> > +
> > + ret = scmi_to_linux_errno(out.status);
> > + if (ret < 0) {
> > + log_debug("%s: the status code of getting permissions: %d\n",
> __func__, ret);
> > + return ret;
> > + }
> > +
> > + *perm = out.permissions;
> > + return 0;
> > +}
>
> [...]
>
> > @@ -156,29 +267,44 @@ static int scmi_clk_probe(struct udevice *dev)
> >
> > for (i = 0; i < num_clocks; i++) {
> > char *clock_name;
> > + u32 attributes;
> >
> > - if (!scmi_clk_get_attibute(dev, i, &clock_name)) {
> > - clk = kzalloc(sizeof(*clk), GFP_KERNEL);
> > - if (!clk || !clock_name)
> > + if (!scmi_clk_get_attibute(dev, i, &clock_name, &attributes)) {
> > + clk_scmi = kzalloc(sizeof(*clk_scmi), GFP_KERNEL);
> > + if (!clk_scmi || !clock_name)
> > ret = -ENOMEM;
> > else
> > - ret = clk_register(clk, dev->driver->name,
> > + ret = clk_register(&clk_scmi->clk,
> > dev->driver->name,
> > clock_name, dev->name);
> >
> > if (ret) {
> > - free(clk);
> > + free(clk_scmi);
> > free(clock_name);
> > + free(&attributes);
>
> Do you really need to free() an u32 attributes here ?
>
> > return ret;
> > }
> >
> > - clk_dm(i, clk);
> > + clk_dm(i, &clk_scmi->clk);
> > +
> > + if (CLK_HAS_RESTRICTIONS(attributes)) {
> > + u32 perm;
> > +
> > + ret = scmi_clk_get_permissions(dev, i, &perm);
> > + if (ret < 0)
> > + clk_scmi->ctrl_flags = 0;
> > + else
> > + clk_scmi->ctrl_flags = perm;
> > + } else {
> > + clk_scmi->ctrl_flags = SUPPORT_CLK_STAT_CONTROL
> > |
> SUPPORT_CLK_PARENT_CONTROL |
> > + SUPPORT_CLK_RATE_CONTROL;
> > + }
> > }
> > }
> >
> > return 0;
> > }
>
> [...]
>
> > enum scmi_clock_message_id {
> > SCMI_CLOCK_ATTRIBUTES = 0x3,
> > @@ -738,6 +739,7 @@ enum scmi_clock_message_id {
> > SCMI_CLOCK_RATE_GET = 0x6,
> > SCMI_CLOCK_CONFIG_SET = 0x7,
> > SCMI_CLOCK_PARENT_SET = 0xD,
> > + SCMI_CLOCK_GET_PERMISSIONS = 0xF,
>
> Better call it SCMI_CLOCK_PERMISSIONS_GET to be consistent with the rest of
> these fields , unless this is some SCMI specification name ?
CLOCK_GET_PERMISSIONS is from Arm System Control and Management Interface
Platform Design Document 3.2.
Best Regards,
Alice Guo
