| Message ID | 20240806121157.479212-7-virag.david003@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add USB clocks to Exynos7885 | expand |
On 06/08/2024 14:11, David Virag wrote: > pll1418x is used in Exynos7885 SoC for USB PHY clock. > Operation-wise it is very similar to pll0822x, except that MDIV is only > 9 bits wide instead of 10, and we use the CON1 register in the PLL > macro's "con" parameter instead of CON3 like this: > > PLL(pll_1418x, CLK_FOUT_USB_PLL, "fout_usb_pll", "oscclk", > PLL_LOCKTIME_PLL_USB, PLL_CON0_PLL_USB, > pll_usb_rate_table), > > Technically the PLL should work fine with pll0822x code if the PLL > tables are correct, but it's more "correct" to actually update the mask. > > Signed-off-by: David Virag <virag.david003@gmail.com> > --- > drivers/clk/samsung/clk-pll.c | 20 ++++++++++++++++---- > drivers/clk/samsung/clk-pll.h | 1 + > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c > index 4be879ab917e..c61a2810737c 100644 > --- a/drivers/clk/samsung/clk-pll.c > +++ b/drivers/clk/samsung/clk-pll.c > @@ -430,6 +430,9 @@ static const struct clk_ops samsung_pll36xx_clk_min_ops = { > #define PLL0822X_LOCK_STAT_SHIFT (29) > #define PLL0822X_ENABLE_SHIFT (31) > > +/* PLL1418x is similar to PLL0822x, except that MDIV is one bit smaller */ > +#define PLL1418X_MDIV_MASK (0x1FF) > + > static unsigned long samsung_pll0822x_recalc_rate(struct clk_hw *hw, > unsigned long parent_rate) > { > @@ -438,7 +441,10 @@ static unsigned long samsung_pll0822x_recalc_rate(struct clk_hw *hw, > u64 fvco = parent_rate; > > pll_con3 = readl_relaxed(pll->con_reg); > - mdiv = (pll_con3 >> PLL0822X_MDIV_SHIFT) & PLL0822X_MDIV_MASK; > + if (pll->type != pll_1418x) > + mdiv = (pll_con3 >> PLL0822X_MDIV_SHIFT) & PLL0822X_MDIV_MASK; > + else > + mdiv = (pll_con3 >> PLL0822X_MDIV_SHIFT) & PLL1418X_MDIV_MASK; > pdiv = (pll_con3 >> PLL0822X_PDIV_SHIFT) & PLL0822X_PDIV_MASK; > sdiv = (pll_con3 >> PLL0822X_SDIV_SHIFT) & PLL0822X_SDIV_MASK; > > @@ -468,9 +474,14 @@ static int samsung_pll0822x_set_rate(struct clk_hw *hw, unsigned long drate, > > /* Change PLL PMS values */ > pll_con3 = readl_relaxed(pll->con_reg); > - pll_con3 &= ~((PLL0822X_MDIV_MASK << PLL0822X_MDIV_SHIFT) | > - (PLL0822X_PDIV_MASK << PLL0822X_PDIV_SHIFT) | > - (PLL0822X_SDIV_MASK << PLL0822X_SDIV_SHIFT)); > + if (pll->type != pll_1418x) > + pll_con3 &= ~((PLL0822X_MDIV_MASK << PLL0822X_MDIV_SHIFT) | > + (PLL0822X_PDIV_MASK << PLL0822X_PDIV_SHIFT) | > + (PLL0822X_SDIV_MASK << PLL0822X_SDIV_SHIFT)); I think this part could be simpler. If I read it correctly, only mdiv mask is different, so create local variable mdiv_mask and set it in if-else block accordingly to the type. Then here you have non-conditional code: pll_con3 &= ~((mdiv_mask << PLL0822X_MDIV_SHIFT) | Best regards, Krzysztof
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c index 4be879ab917e..c61a2810737c 100644 --- a/drivers/clk/samsung/clk-pll.c +++ b/drivers/clk/samsung/clk-pll.c @@ -430,6 +430,9 @@ static const struct clk_ops samsung_pll36xx_clk_min_ops = { #define PLL0822X_LOCK_STAT_SHIFT (29) #define PLL0822X_ENABLE_SHIFT (31) +/* PLL1418x is similar to PLL0822x, except that MDIV is one bit smaller */ +#define PLL1418X_MDIV_MASK (0x1FF) + static unsigned long samsung_pll0822x_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -438,7 +441,10 @@ static unsigned long samsung_pll0822x_recalc_rate(struct clk_hw *hw, u64 fvco = parent_rate; pll_con3 = readl_relaxed(pll->con_reg); - mdiv = (pll_con3 >> PLL0822X_MDIV_SHIFT) & PLL0822X_MDIV_MASK; + if (pll->type != pll_1418x) + mdiv = (pll_con3 >> PLL0822X_MDIV_SHIFT) & PLL0822X_MDIV_MASK; + else + mdiv = (pll_con3 >> PLL0822X_MDIV_SHIFT) & PLL1418X_MDIV_MASK; pdiv = (pll_con3 >> PLL0822X_PDIV_SHIFT) & PLL0822X_PDIV_MASK; sdiv = (pll_con3 >> PLL0822X_SDIV_SHIFT) & PLL0822X_SDIV_MASK; @@ -468,9 +474,14 @@ static int samsung_pll0822x_set_rate(struct clk_hw *hw, unsigned long drate, /* Change PLL PMS values */ pll_con3 = readl_relaxed(pll->con_reg); - pll_con3 &= ~((PLL0822X_MDIV_MASK << PLL0822X_MDIV_SHIFT) | - (PLL0822X_PDIV_MASK << PLL0822X_PDIV_SHIFT) | - (PLL0822X_SDIV_MASK << PLL0822X_SDIV_SHIFT)); + if (pll->type != pll_1418x) + pll_con3 &= ~((PLL0822X_MDIV_MASK << PLL0822X_MDIV_SHIFT) | + (PLL0822X_PDIV_MASK << PLL0822X_PDIV_SHIFT) | + (PLL0822X_SDIV_MASK << PLL0822X_SDIV_SHIFT)); + else + pll_con3 &= ~((PLL1418X_MDIV_MASK << PLL0822X_MDIV_SHIFT) | + (PLL0822X_PDIV_MASK << PLL0822X_PDIV_SHIFT) | + (PLL0822X_SDIV_MASK << PLL0822X_SDIV_SHIFT)); pll_con3 |= (rate->mdiv << PLL0822X_MDIV_SHIFT) | (rate->pdiv << PLL0822X_PDIV_SHIFT) | (rate->sdiv << PLL0822X_SDIV_SHIFT); @@ -1317,6 +1328,7 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx, init.ops = &samsung_pll35xx_clk_ops; break; case pll_1417x: + case pll_1418x: case pll_0818x: case pll_0822x: case pll_0516x: diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h index ffd3d52c0dec..1efbe4c446d0 100644 --- a/drivers/clk/samsung/clk-pll.h +++ b/drivers/clk/samsung/clk-pll.h @@ -30,6 +30,7 @@ enum samsung_pll_type { pll_2650x, pll_2650xx, pll_1417x, + pll_1418x, pll_1450x, pll_1451x, pll_1452x,
pll1418x is used in Exynos7885 SoC for USB PHY clock. Operation-wise it is very similar to pll0822x, except that MDIV is only 9 bits wide instead of 10, and we use the CON1 register in the PLL macro's "con" parameter instead of CON3 like this: PLL(pll_1418x, CLK_FOUT_USB_PLL, "fout_usb_pll", "oscclk", PLL_LOCKTIME_PLL_USB, PLL_CON0_PLL_USB, pll_usb_rate_table), Technically the PLL should work fine with pll0822x code if the PLL tables are correct, but it's more "correct" to actually update the mask. Signed-off-by: David Virag <virag.david003@gmail.com> --- drivers/clk/samsung/clk-pll.c | 20 ++++++++++++++++---- drivers/clk/samsung/clk-pll.h | 1 + 2 files changed, 17 insertions(+), 4 deletions(-)