[PATCH] power: regulator: scmi: Move regulator subnode hack to scmi

U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
@ 2025-03-22  1:44 Marek Vasut
  2025-04-03 10:56 ` 回复: [EXT] " Alice Guo (OSS)
  2025-04-23 16:05 ` Peng Fan (OSS)
  0 siblings, 2 replies; 7+ messages in thread
From: Marek Vasut @ 2025-03-22  1:44 UTC (permalink / raw)
  To: u-boot; +Cc: Marek Vasut, Alice Guo, Ilias Apalodimas, Jaehoon Chung, Tom Rini

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@denx.de>
---
Cc: Alice Guo <alice.guo@nxp.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
  2025-03-22  1:44 [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator Marek Vasut
@ 2025-04-03 10:56 ` Alice Guo (OSS)
  2025-04-08 18:30   ` Marek Vasut
  2025-04-23 16:05 ` Peng Fan (OSS)
  1 sibling, 1 reply; 7+ messages in thread
From: Alice Guo (OSS) @ 2025-04-03 10:56 UTC (permalink / raw)
  To: Marek Vasut, u-boot@lists.denx.de
  Cc: Alice Guo, Ilias Apalodimas, Jaehoon Chung, Tom Rini

> -----邮件原件-----
> 发件人: Marek Vasut <marex@denx.de>
> 发送时间: 2025年3月22日 9:45
> 收件人: u-boot@lists.denx.de
> 抄送: Marek Vasut <marex@denx.de>; Alice Guo <alice.guo@nxp.com>; Ilias
> Apalodimas <ilias.apalodimas@linaro.org>; Jaehoon Chung
> <jh80.chung@samsung.com>; Tom Rini <trini@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@denx.de>
> ---
> Cc: Alice Guo <alice.guo@nxp.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
  2025-04-03 10:56 ` 回复: [EXT] " Alice Guo (OSS)
@ 2025-04-08 18:30   ` Marek Vasut
  2025-04-09  6:42     ` 回复: " Alice Guo (OSS)
  0 siblings, 1 reply; 7+ messages in thread
From: Marek Vasut @ 2025-04-08 18:30 UTC (permalink / raw)
  To: Alice Guo (OSS), u-boot@lists.denx.de
  Cc: Alice Guo, Ilias Apalodimas, Jaehoon Chung, Tom Rini

On 4/3/25 12:56 PM, Alice Guo (OSS) wrote:
>> -----邮件原件-----
>> 发件人: Marek Vasut <marex@denx.de>
>> 发送时间: 2025年3月22日 9:45
>> 收件人: u-boot@lists.denx.de
>> 抄送: Marek Vasut <marex@denx.de>; Alice Guo <alice.guo@nxp.com>; Ilias
>> Apalodimas <ilias.apalodimas@linaro.org>; Jaehoon Chung
>> <jh80.chung@samsung.com>; Tom Rini <trini@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@denx.de>
>> ---
>> Cc: Alice Guo <alice.guo@nxp.com>
>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: u-boot@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:
What kind of problem ? Can you please elaborate ?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 回复: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
  2025-04-08 18:30   ` Marek Vasut
@ 2025-04-09  6:42     ` Alice Guo (OSS)
  2025-04-18  2:52       ` Peng Fan
  0 siblings, 1 reply; 7+ messages in thread
From: Alice Guo (OSS) @ 2025-04-09  6:42 UTC (permalink / raw)
  To: Marek Vasut, Alice Guo (OSS), u-boot@lists.denx.de
  Cc: Alice Guo, Ilias Apalodimas, Jaehoon Chung, Tom Rini

> -----邮件原件-----
> 发件人: Marek Vasut <marex@denx.de>
> 发送时间: 2025年4月9日 2:31
> 收件人: Alice Guo (OSS) <alice.guo@oss.nxp.com>; u-boot@lists.denx.de
> 抄送: Alice Guo <alice.guo@nxp.com>; Ilias Apalodimas
> <ilias.apalodimas@linaro.org>; Jaehoon Chung <jh80.chung@samsung.com>;
> Tom Rini <trini@konsulko.com>
> 主题: Re: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator
> subnode hack to scmi_regulator
> 
> On 4/3/25 12:56 PM, Alice Guo (OSS) wrote:
> >> -----邮件原件-----
> >> 发件人: Marek Vasut <marex@denx.de>
> >> 发送时间: 2025年3月22日 9:45
> >> 收件人: u-boot@lists.denx.de
> >> 抄送: Marek Vasut <marex@denx.de>; Alice Guo <alice.guo@nxp.com>;
> Ilias
> >> Apalodimas <ilias.apalodimas@linaro.org>; Jaehoon Chung
> >> <jh80.chung@samsung.com>; Tom Rini <trini@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@denx.de>
> >> ---
> >> Cc: Alice Guo <alice.guo@nxp.com>
> >> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> >> Cc: Tom Rini <trini@konsulko.com>
> >> Cc: u-boot@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:
> What kind of problem ? Can you please elaborate ?

I think that in the scmi_regulator_bind() function, the second parameter passed into ofnode_for_each_subnode() should be the node named "regulators".

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(regul_node)) {
		dev_err(dev, "no regulators node\n");
		return -ENXIO;
	}

	drv = DM_DRIVER_GET(scmi_regulator);

	ofnode_for_each_subnode(node, regul_node) {
		ret = device_bind(dev, drv, ofnode_get_name(node),
				  NULL, node, NULL);
		if (ret)
			return ret;
	}

	return 0;
}

Best Regards,
Alice Guo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 回复: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
  2025-04-09  6:42     ` 回复: " Alice Guo (OSS)
@ 2025-04-18  2:52       ` Peng Fan
  2025-04-23  3:18         ` Peng Fan
  0 siblings, 1 reply; 7+ messages in thread
From: Peng Fan @ 2025-04-18  2:52 UTC (permalink / raw)
  To: Alice Guo (OSS)
  Cc: Marek Vasut, u-boot@lists.denx.de, Alice Guo, Ilias Apalodimas,
	Jaehoon Chung, Tom Rini

On Wed, Apr 09, 2025 at 06:42:14AM +0000, Alice Guo (OSS) wrote:
>> -----邮件原件-----
>> 发件人: Marek Vasut <marex@denx.de>
>> 发送时间: 2025年4月9日 2:31
>> 收件人: Alice Guo (OSS) <alice.guo@oss.nxp.com>; u-boot@lists.denx.de
>> 抄送: Alice Guo <alice.guo@nxp.com>; Ilias Apalodimas
>> <ilias.apalodimas@linaro.org>; Jaehoon Chung <jh80.chung@samsung.com>;
>> Tom Rini <trini@konsulko.com>
>> 主题: Re: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator
>> subnode hack to scmi_regulator
>> 
>> On 4/3/25 12:56 PM, Alice Guo (OSS) wrote:
>> >> -----邮件原件-----
>> >> 发件人: Marek Vasut <marex@denx.de>
>> >> 发送时间: 2025年3月22日 9:45
>> >> 收件人: u-boot@lists.denx.de
>> >> 抄送: Marek Vasut <marex@denx.de>; Alice Guo <alice.guo@nxp.com>;
>> Ilias
>> >> Apalodimas <ilias.apalodimas@linaro.org>; Jaehoon Chung
>> >> <jh80.chung@samsung.com>; Tom Rini <trini@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@denx.de>
>> >> ---
>> >> Cc: Alice Guo <alice.guo@nxp.com>
>> >> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> >> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> >> Cc: Tom Rini <trini@konsulko.com>
>> >> Cc: u-boot@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:
>> What kind of problem ? Can you please elaborate ?
>
>I think that in the scmi_regulator_bind() function, the second parameter passed into ofnode_for_each_subnode() should be the node named "regulators".
>
>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");

Right. I could correct this when apply the patch.

Thanks,
Peng

>	if (!ofnode_valid(regul_node)) {
>		dev_err(dev, "no regulators node\n");
>		return -ENXIO;
>	}
>
>	drv = DM_DRIVER_GET(scmi_regulator);
>
>	ofnode_for_each_subnode(node, regul_node) {
>		ret = device_bind(dev, drv, ofnode_get_name(node),
>				  NULL, node, NULL);
>		if (ret)
>			return ret;
>	}
>
>	return 0;
>}
>
>Best Regards,
>Alice Guo
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: 回复: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
  2025-04-18  2:52       ` Peng Fan
@ 2025-04-23  3:18         ` Peng Fan
  0 siblings, 0 replies; 7+ messages in thread
From: Peng Fan @ 2025-04-23  3:18 UTC (permalink / raw)
  To: Alice Guo (OSS)
  Cc: Marek Vasut, u-boot@lists.denx.de, Alice Guo, Ilias Apalodimas,
	Jaehoon Chung, Tom Rini

On Fri, Apr 18, 2025 at 10:52:00AM +0800, Peng Fan wrote:
>On Wed, Apr 09, 2025 at 06:42:14AM +0000, Alice Guo (OSS) wrote:
>>> -----邮件原件-----
>>> 发件人: Marek Vasut <marex@denx.de>
>>> 发送时间: 2025年4月9日 2:31
>>> 收件人: Alice Guo (OSS) <alice.guo@oss.nxp.com>; u-boot@lists.denx.de
>>> 抄送: Alice Guo <alice.guo@nxp.com>; Ilias Apalodimas
>>> <ilias.apalodimas@linaro.org>; Jaehoon Chung <jh80.chung@samsung.com>;
>>> Tom Rini <trini@konsulko.com>
>>> 主题: Re: 回复: [EXT] [PATCH] power: regulator: scmi: Move regulator
>>> subnode hack to scmi_regulator
>>> 
>>> On 4/3/25 12:56 PM, Alice Guo (OSS) wrote:
>>> >> -----邮件原件-----
>>> >> 发件人: Marek Vasut <marex@denx.de>
>>> >> 发送时间: 2025年3月22日 9:45
>>> >> 收件人: u-boot@lists.denx.de
>>> >> 抄送: Marek Vasut <marex@denx.de>; Alice Guo <alice.guo@nxp.com>;
>>> Ilias
>>> >> Apalodimas <ilias.apalodimas@linaro.org>; Jaehoon Chung
>>> >> <jh80.chung@samsung.com>; Tom Rini <trini@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@denx.de>
>>> >> ---
>>> >> Cc: Alice Guo <alice.guo@nxp.com>
>>> >> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>> >> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> >> Cc: Tom Rini <trini@konsulko.com>
>>> >> Cc: u-boot@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:
>>> What kind of problem ? Can you please elaborate ?
>>
>>I think that in the scmi_regulator_bind() function, the second parameter passed into ofnode_for_each_subnode() should be the node named "regulators".
>>
>>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");
>
>Right. I could correct this when apply the patch.

>
>Thanks,
>Peng
>
>>	if (!ofnode_valid(regul_node)) {
>>		dev_err(dev, "no regulators node\n");

This patch has issue, it will always return failure here.

Regards,
Peng

>>		return -ENXIO;
>>	}
>>
>>	drv = DM_DRIVER_GET(scmi_regulator);
>>
>>	ofnode_for_each_subnode(node, regul_node) {
>>		ret = device_bind(dev, drv, ofnode_get_name(node),
>>				  NULL, node, NULL);
>>		if (ret)
>>			return ret;
>>	}
>>
>>	return 0;
>>}
>>
>>Best Regards,
>>Alice Guo
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
  2025-03-22  1:44 [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator Marek Vasut
  2025-04-03 10:56 ` 回复: [EXT] " Alice Guo (OSS)
@ 2025-04-23 16:05 ` Peng Fan (OSS)
  1 sibling, 0 replies; 7+ messages in thread
From: Peng Fan (OSS) @ 2025-04-23 16:05 UTC (permalink / raw)
  To: u-boot, Marek Vasut
  Cc: Peng Fan, Alice Guo, Ilias Apalodimas, Jaehoon Chung, Tom Rini

From: Peng Fan <peng.fan@nxp.com>


On Sat, 22 Mar 2025 02:44:40 +0100, Marek Vasut wrote:
> 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.
> 
> 
> [...]

After apply a fix from Alice Guo, applied, thanks!

[1/1] power: regulator: scmi: Move regulator subnode hack to scmi_regulator
      commit: 2a8e7ea4f948e3de7033b297fd827fe082cd63da

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-04-23 14:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-22  1:44 [PATCH] power: regulator: scmi: Move regulator subnode hack to scmi_regulator Marek Vasut
2025-04-03 10:56 ` 回复: [EXT] " Alice Guo (OSS)
2025-04-08 18:30   ` Marek Vasut
2025-04-09  6:42     ` 回复: " Alice Guo (OSS)
2025-04-18  2:52       ` Peng Fan
2025-04-23  3:18         ` Peng Fan
2025-04-23 16:05 ` Peng Fan (OSS)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox