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

[Marek Vasut]

On 4/3/25 12:56 PM, Alice Guo (OSS) wrote:
>> -----邮件原件-----
>> 发件人: 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:
What kind of problem ? Can you please elaborate ?