Re: [PATCH 2/2] clk: X1000: Add support for calculat REFCLK of USB PHY.

From: Paul Cercueil
Date: Fri Jun 26 2020 - 13:36:26 EST


Hi Zhou,

Le sam. 27 juin 2020 Ã 0:48, åçæ (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx> 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@xxxxxxxxxxx>
Signed-off-by: åçæ (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
---
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