On Mon, Jun 5, 2023 at 8:37 PM Xavier Drudis Ferran <xdru...@tinet.cat> wrote: > > This clock doesn't seem needed but appears in a phandle list used by > ehci-generic.c to bulk enable it. The phandle list comes from linux, > where it is needed for suspend/resume to work [1]. > > My tests give the same results with or without this patch, but Marek > Vasut found it weird to declare an empty clk_ops [2]. > > So I adapted the code from linux 6.1-rc8 so that it hopefully works > if it ever has some user. For now, without real use, it seems to > at least not give any errors when called. > > Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ > [2] > https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/ > > Cc: Simon Glass <s...@chromium.org> > Cc: Philipp Tomsich <philipp.toms...@vrull.eu> > Cc: Kever Yang <kever.y...@rock-chips.com> > Cc: Lukasz Majewski <lu...@denx.de> > Cc: Sean Anderson <sean...@gmail.com> > Cc: Marek Vasut <ma...@denx.de> > Cc: Christoph Fritz <chf.fr...@googlemail.com> > Cc: Jagan Teki <ja...@amarulasolutions.com> > > Signed-off-by: Xavier Drudis Ferran <xdru...@tinet.cat> > --- > > v7: add clkout_ctl values for rk3568 (from linux). > UNTESTED (I don't have the hardware). > > v6: just retested over current next branch and some corrections > to message and headers > (no changes to code). > > v5: ignores the return value from property_enable() which is not > an error code in U-Boot (unlike in linux). This avoid a false > failure of rockchip_usb2phy_clk_disable() that interfered with > clock disable and appeared to cause hang or reset. > --- > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++- > 1 file changed, 78 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > index 732d37201d..be5f79490c 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > @@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg { > > struct rockchip_usb2phy_cfg { > unsigned int reg; > + struct usb2phy_reg clkout_ctl; > const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS]; > }; > > @@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base, > return writel(val, reg_base + reg->offset); > } > > +static inline bool property_enabled(void *reg_base, > + const struct usb2phy_reg *reg) > +{ > + unsigned int tmp, orig; > + unsigned int mask = GENMASK(reg->bitend, reg->bitstart); > + > + orig = readl(reg_base + reg->offset); > + > + tmp = (orig & mask) >> reg->bitstart; > + return tmp != reg->disable; > +} > + > static const > struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) > { > @@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = { > .of_xlate = rockchip_usb2phy_of_xlate, > }; > > +/** > + * round_rate() - Adjust a rate to the exact rate a clock can provide. > + * @clk: The clock to manipulate. > + * @rate: Desidered clock rate in Hz. > + * > + * Return: rounded rate in Hz, or -ve error code. > + */
I forgot to comment, this last time. I feel these explicit comments wouldn't be required as clk-uclass clearly documented. > +ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate) static > +{ > + return 480000000; > +} > + > +/** > + * enable() - Enable a clock. > + * @clk: The clock to manipulate. > + * > + * Return: zero on success, or -ve error code. > + */ ditto. > +int rockchip_usb2phy_clk_enable(struct clk *clk) ditto. > +{ > + struct udevice *parent = dev_get_parent(clk->dev); > + struct rockchip_usb2phy *priv = dev_get_priv(parent); > + const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg; > + > + /* turn on 480m clk output if it is off */ > + if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) { > + property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true); > + > + /* waiting for the clk become stable */ > + usleep_range(1200, 1300); > + } > + > + return 0; > +} > + > +/** > + * disable() - Disable a clock. > + * @clk: The clock to manipulate. > + * > + * Return: zero on success, or -ve error code. > + */ ditto. > +int rockchip_usb2phy_clk_disable(struct clk *clk) ditto. Thanks, Jagan.