Messages in this thread | | | Date | Fri, 26 Jun 2020 19:36:10 +0200 | From | Paul Cercueil <> | Subject | Re: [PATCH 2/2] clk: X1000: Add support for calculat REFCLK of USB PHY. |
| |
Hi Zhou,
Le sam. 27 juin 2020 à 0:48, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> a écrit : > Add new functions to "x1000_otg_phy_ops" to calculat the rate of > REFCLK, > which is needed by USB PHY in the Ingenic X1000 SoC. > > Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com> > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> > --- > drivers/clk/ingenic/x1000-cgu.c | 113 > ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 113 insertions(+) > > diff --git a/drivers/clk/ingenic/x1000-cgu.c > b/drivers/clk/ingenic/x1000-cgu.c > index 453f3323cb99..a61c16f98a11 100644 > --- a/drivers/clk/ingenic/x1000-cgu.c > +++ b/drivers/clk/ingenic/x1000-cgu.c > @@ -48,8 +48,114 @@ > #define USBPCR_SIDDQ BIT(21) > #define USBPCR_OTG_DISABLE BIT(20) > > +/* bits within the USBPCR1 register */ > +#define USBPCR1_REFCLKSEL_SHIFT 26 > +#define USBPCR1_REFCLKSEL_MASK (0x3 << USBPCR1_REFCLKSEL_SHIFT) > +#define USBPCR1_REFCLKSEL_CORE (0x2 << USBPCR1_REFCLKSEL_SHIFT) > +#define USBPCR1_REFCLKDIV_SHIFT 24 > +#define USBPCR1_REFCLKDIV_MASK (0x3 << USBPCR1_REFCLKDIV_SHIFT) > +#define USBPCR1_REFCLKDIV_48 (0x2 << USBPCR1_REFCLKDIV_SHIFT) > +#define USBPCR1_REFCLKDIV_24 (0x1 << USBPCR1_REFCLKDIV_SHIFT) > +#define USBPCR1_REFCLKDIV_12 (0x0 << USBPCR1_REFCLKDIV_SHIFT) > + > static struct ingenic_cgu *cgu; > > +static u8 x1000_otg_phy_get_parent(struct clk_hw *hw) > +{ > + /* we only use CLKCORE, revisit if that ever changes */ > + return 0; > +} > + > +static int x1000_otg_phy_set_parent(struct clk_hw *hw, u8 idx) > +{ > + unsigned long flags; > + u32 usbpcr1; > + > + if (idx > 0) > + return -EINVAL; > + > + spin_lock_irqsave(&cgu->lock, flags); > + > + usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); > + usbpcr1 &= ~USBPCR1_REFCLKSEL_MASK; > + /* we only use CLKCORE */ > + usbpcr1 |= USBPCR1_REFCLKSEL_CORE; > + writel(usbpcr1, cgu->base + CGU_REG_USBPCR1); > + > + spin_unlock_irqrestore(&cgu->lock, flags); > + return 0; > +}
If you only support one parent, maybe set that bit in the x1000_cgu_init(), then you can drop the get_parent/set_parent.
> + > +static unsigned long x1000_otg_phy_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + u32 usbpcr1; > + unsigned refclk_div; > + > + usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); > + refclk_div = usbpcr1 & USBPCR1_REFCLKDIV_MASK; > + > + switch (refclk_div) { > + case USBPCR1_REFCLKDIV_12: > + return 12000000; > + > + case USBPCR1_REFCLKDIV_24: > + return 24000000; > + > + case USBPCR1_REFCLKDIV_48: > + return 48000000; > + }
On your setup, what frequency is configured for the "otg" clock? Is it 48 MHz?
I believe CLKCORE is the OTG core's clock (aka "otg"), and I'm pretty sure that these fields only represent CLKCORE/4, CLKCORE/2, CLKCORE/1, but the doc expects CLKCORE==48MHz.
In that case the "otg_phy" should be parented to "otg", and the rate should be computed according to the parent rate and the divider configured.
> + > + BUG();
Don't use BUG() - it pisses off Linus :) And it's reserved for bugs that will take the whole system down, I think. Better use WARN().
Cheers, -Paul
> + return parent_rate; > +} > + > +static long x1000_otg_phy_round_rate(struct clk_hw *hw, unsigned > long req_rate, > + unsigned long *parent_rate) > +{ > + if (req_rate < 18000000) > + return 12000000; > + > + if (req_rate < 36000000) > + return 24000000; > + > + return 48000000; > +} > + > +static int x1000_otg_phy_set_rate(struct clk_hw *hw, unsigned long > req_rate, > + unsigned long parent_rate) > +{ > + unsigned long flags; > + u32 usbpcr1, div_bits; > + > + switch (req_rate) { > + case 12000000: > + div_bits = USBPCR1_REFCLKDIV_12; > + break; > + > + case 24000000: > + div_bits = USBPCR1_REFCLKDIV_24; > + break; > + > + case 48000000: > + div_bits = USBPCR1_REFCLKDIV_48; > + break; > + > + default: > + return -EINVAL; > + } > + > + spin_lock_irqsave(&cgu->lock, flags); > + > + usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1); > + usbpcr1 &= ~USBPCR1_REFCLKDIV_MASK; > + usbpcr1 |= div_bits; > + writel(usbpcr1, cgu->base + CGU_REG_USBPCR1); > + > + spin_unlock_irqrestore(&cgu->lock, flags); > + return 0; > +} > + > static int x1000_usb_phy_enable(struct clk_hw *hw) > { > void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; > @@ -80,6 +186,13 @@ static int x1000_usb_phy_is_enabled(struct clk_hw > *hw) > } > > static const struct clk_ops x1000_otg_phy_ops = { > + .get_parent = x1000_otg_phy_get_parent, > + .set_parent = x1000_otg_phy_set_parent, > + > + .recalc_rate = x1000_otg_phy_recalc_rate, > + .round_rate = x1000_otg_phy_round_rate, > + .set_rate = x1000_otg_phy_set_rate, > + > .enable = x1000_usb_phy_enable, > .disable = x1000_usb_phy_disable, > .is_enabled = x1000_usb_phy_is_enabled, > -- > 2.11.0 >
| |