Re: [PATCH v6 1/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2.

[Xavier Drudis Ferran <xdrudis@tinet.cat>]

Thanks for your feedback.

El Sun, Jun 04, 2023 at 11:31:28AM +0200, Marek Vasut deia:
> > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > index 55e1dbcfef..2f31350134 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > @@ -7,7 +7,7 @@
> >    */
> >   #include <common.h>
> > -#include <clk.h>
> > +#include <clk-uclass.h>
> >   #include <dm.h>
> >   #include <asm/global_data.h>
> >   #include <dm/device_compat.h>
> > @@ -168,6 +168,9 @@ static struct phy_ops rockchip_usb2phy_ops = {
> >   	.of_xlate = rockchip_usb2phy_of_xlate,
> >   };
> > +static struct clk_ops rockchip_usb2phy_clk_ops = {
> > +};
> > +
> >   static int rockchip_usb2phy_probe(struct udevice *dev)
> >   {
> >   	struct rockchip_usb2phy *priv = dev_get_priv(dev);
> > @@ -249,6 +252,18 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
> >   		}
> >   	}
> > +	if (!ret) {
> 
> Can $ret ever be != 0 here ?
>

No, you're right. I can get rid of the if in v7.

> btw. the dev_for_each_subnode() above is missing error handling, in case
> device_bind_driver_to_node() there returns non-zero, there should be some
> 'goto err' and 'err: dev_for_each_subnode() device_unbind()' fail path.
>
> > +		node = dev_ofnode(dev);
> > +		name = ofnode_get_name(node);
> > +		dev_dbg(dev, "clk for node %s\n", name);
> > +		ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock",
> > +						 name, node, &usb2phy_dev);
> > +		if (ret) {
> > +			dev_err(dev,
> > +				"'%s' cannot bind 'rockchip_usb2phy_clock'\n", name);
> 
> Use device_unbind() in fail path here too.
>

Well, dev_for_each_subnode wouldn't give me the dev to pass to
device_unbind, but I can simply call device_chld_unbind(dev) on the
error path (on the parent device) and that should clean up any bound
children.

I'll fix it in v7, thanks.