回复: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator

[Alice Guo (OSS)]

> -----邮件原件-----
> 发件人: Marek Vasut <marex at denx.de>
> 发送时间: 2025年3月22日 9:45
> 收件人: u-boot at lists.denx.de
> 抄送: Marek Vasut <marex at denx.de>; Alice Guo <alice.guo at nxp.com>; Ilias
> Apalodimas <ilias.apalodimas at linaro.org>; Jaehoon Chung
> <jh80.chung at samsung.com>; Tom Rini <trini at konsulko.com>
> 主题: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to
> scmi_regulator
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> The current code attempts to bind scmi_voltage_domain to regulator subnode
> of the SCMI protocol node, so scmi_voltage_domain can then bind regulators
> directly to subnodes of its node. This kind of behavior should not be in core code,
> move it into scmi_voltage_domain driver code. Let the driver descend into
> regulator node and bind regulators to its subnodes.
> 
> Fixes: 1f213ee4dbf2 ("firmware: scmi: voltage regulator")
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Alice Guo <alice.guo at nxp.com>
> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Cc: Jaehoon Chung <jh80.chung at samsung.com>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: u-boot at lists.denx.de
> ---
>  drivers/firmware/scmi/scmi_agent-uclass.c | 8 +-------
> drivers/power/regulator/scmi_regulator.c  | 6 ++++++
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
> b/drivers/firmware/scmi/scmi_agent-uclass.c
> index 8c907c3b032..e6e43ae936a 100644
> --- a/drivers/firmware/scmi/scmi_agent-uclass.c
> +++ b/drivers/firmware/scmi/scmi_agent-uclass.c
> @@ -427,14 +427,8 @@ static int scmi_bind_protocols(struct udevice *dev)
>                         break;
>                 case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
>                         if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)
> &&
> -                           scmi_protocol_is_supported(dev, protocol_id))
> {
> -                               node = ofnode_find_subnode(node,
> "regulators");
> -                               if (!ofnode_valid(node)) {
> -                                       dev_err(dev, "no regulators
> node\n");
> -                                       return -ENXIO;
> -                               }
> +                           scmi_protocol_is_supported(dev,
> + protocol_id))
>                                 drv =
> DM_DRIVER_GET(scmi_voltage_domain);
> -                       }
>                         break;
>                 default:
>                         break;
> diff --git a/drivers/power/regulator/scmi_regulator.c
> b/drivers/power/regulator/scmi_regulator.c
> index 99f6506f162..2550b27246f 100644
> --- a/drivers/power/regulator/scmi_regulator.c
> +++ b/drivers/power/regulator/scmi_regulator.c
> @@ -178,6 +178,12 @@ static int scmi_regulator_bind(struct udevice *dev)
>         ofnode node;
>         int ret;
> 
> +       node = ofnode_find_subnode(node, "regulators");
> +       if (!ofnode_valid(node)) {
> +               dev_err(dev, "no regulators node\n");
> +               return -ENXIO;
> +       }
> +
>         drv = DM_DRIVER_GET(scmi_regulator);
> 
>         ofnode_for_each_subnode(node, dev_ofnode(dev)) {
> --
> 2.47.2

Hi Marek,

Is there a problem in your patch? Should it be changed like this:
--- a/drivers/power/regulator/scmi_regulator.c
+++ b/drivers/power/regulator/scmi_regulator.c
@@ -175,12 +175,19 @@ U_BOOT_DRIVER(scmi_regulator) = {
 static int scmi_regulator_bind(struct udevice *dev)
 {
        struct driver *drv;
+       ofnode regul_node;
        ofnode node;
        int ret;

+       regul_node = ofnode_find_subnode(dev_ofnode(dev), "regulators");
+       if (!ofnode_valid(node)) {
+               dev_err(dev, "no regulators node\n");
+               return -ENXIO;
+       }
+
        drv = DM_DRIVER_GET(scmi_regulator);

-       ofnode_for_each_subnode(node, dev_ofnode(dev)) {
+       ofnode_for_each_subnode(node, regul_node) {
                ret = device_bind(dev, drv, ofnode_get_name(node),
                                  NULL, node, NULL);
                if (ret)

Best Regards,
Alice Guo