> -----邮件原件-----
> 发件人: Marek Vasut <ma...@denx.de>
> 发送时间: 2025年4月9日 2:31
> 收件人: Alice Guo (OSS) <alice....@oss.nxp.com>; u-boot@lists.denx.de
> 抄送: Alice Guo <alice....@nxp.com>; Ilias Apalodimas
> <ilias.apalodi...@linaro.org>; Jaehoon Chung <jh80.ch...@samsung.com>;
> Tom Rini <tr...@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 <ma...@denx.de>
> >> 发送时间: 2025年3月22日 9:45
> >> 收件人: u-boot@lists.denx.de
> >> 抄送: Marek Vasut <ma...@denx.de>; Alice Guo <alice....@nxp.com>;
> Ilias
> >> Apalodimas <ilias.apalodi...@linaro.org>; Jaehoon Chung
> >> <jh80.ch...@samsung.com>; Tom Rini <tr...@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 <ma...@denx.de>
> >> ---
> >> Cc: Alice Guo <alice....@nxp.com>
> >> Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> >> Cc: Jaehoon Chung <jh80.ch...@samsung.com>
> >> Cc: Tom Rini <tr...@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
