RE: [PATCH v2] soc: imx: imx8mp-blk-ctrl: Add PCIe SYSPLL configurations

[Hongxing Zhu]

> -----Original Message-----
> From: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Sent: 2022年11月18日 19:02
> To: Hongxing Zhu <hongxing.zhu@xxxxxxx>; Marcel Ziswiler
> <marcel.ziswiler@xxxxxxxxxxx>; marex@xxxxxxx; tharvey@xxxxxxxxxxxxx;
> bhelgaas@xxxxxxxxxx; lorenzo.pieralisi@xxxxxxx; shawnguo@xxxxxxxxxx;
> alexander.stein@xxxxxxxxxxxxxxx; richard.leitner@xxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> kernel@xxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: Re: [PATCH v2] soc: imx: imx8mp-blk-ctrl: Add PCIe SYSPLL
> configurations
> 
> Am Freitag, dem 18.11.2022 um 10:56 +0800 schrieb Richard Zhu:
> > Add PCIe SYSPLL configurations, thus the internal SYSPLL can be used
> > as i.MX8MP PCIe reference clock.
> >
> > The following properties of PHY dts node should be changed accordingly.
> >   - Set 'fsl,refclk-pad-mode' as '<IMX8_PCIE_REFCLK_PAD_OUTPUT>'.
> >   - Change 'clocks' to '<&clk IMX8MP_CLK_HSIO_AXI>'.
> 
> This is still not what I meant. There is no direct relation between the PCIe PHY
> domain being powered up and the PCIe PLL being needed. The PLL really only
> needs to be active when the reference clock isn't sourced externally. So the
> HSIO blk-ctrl should expose a proper clock for the PLL. As the PLL is effectively
> fixed rate it should be enough to expose get_rate, prepare and unprepare clk
> ops.
> 
> The PHY should then point at the blk-ctrl as the reference clock source.
Hi Lucas:
Thanks for your comments.
Understand what're your means.
Can you help to provide some specific pseudo example codes to let blk-ctrl
 expose such kind of clock to PCIe PHY module?

Best Regards
Richard Zhu
> 
> Regards,
> Lucas
> 
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > ---
> > v1->v2:
> > Refer to Lucas' comments, don't expose IMX8MP_CLK_HSIO_ROOT to dts
> node.
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1666590189-1364-1-gi
> t
> >
> -send-email-hongxing.zhu%40nxp.com%2F&amp;data=05%7C01%7Chongxing.
> zhu%
> >
> 40nxp.com%7C4e559030b13d408f66c108dac9546393%7C686ea1d3bc2b4c6
> fa92cd99
> >
> c5c301635%7C0%7C0%7C638043661537710772%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjo
> >
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300
> 0%7C%
> >
> 7C%7C&amp;sdata=GN5WnzNW1oclh7ZJFUUK3B68QYAqRMw6kPv0oEgPmFY
> %3D&amp;res
> > erved=0
> >
> > Use <&clk IMX8MP_CLK_HSIO_AXI> as referrence clock source when
> > internal clock mode is used by i.MX8MP PCIe module.
> >
> > Verified on i.MX8MP EVK board with removing R131/R132/R137/R138, and
> > populating R135/R136.
> > ---
> >  drivers/soc/imx/imx8mp-blk-ctrl.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c
> > b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > index 0e3b6ba22f94..5ad20a8ea25e 100644
> > --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > @@ -21,6 +21,16 @@
> >  #define  USB_CLOCK_MODULE_EN	BIT(1)
> >  #define  PCIE_PHY_APB_RST	BIT(4)
> >  #define  PCIE_PHY_INIT_RST	BIT(5)
> > +#define GPR_REG2		0x8
> > +#define  P_PLL_MASK		GENMASK(5, 0)
> > +#define  M_PLL_MASK		GENMASK(15, 6)
> > +#define  S_PLL_MASK		GENMASK(18, 16)
> > +#define  P_PLL			(0xc << 0)
> > +#define  M_PLL			(0x320 << 6)
> > +#define  S_PLL			(0x4 << 16)
> > +#define GPR_REG3		0xc
> > +#define  PLL_CKE		BIT(17)
> > +#define  PLL_RST		BIT(31)
> >
> >  struct imx8mp_blk_ctrl_domain;
> >
> > @@ -86,6 +96,18 @@ static void imx8mp_hsio_blk_ctrl_power_on(struct
> imx8mp_blk_ctrl *bc,
> >  	case IMX8MP_HSIOBLK_PD_PCIE_PHY:
> >  		regmap_set_bits(bc->regmap, GPR_REG0,
> >  				PCIE_PHY_APB_RST | PCIE_PHY_INIT_RST);
> > +
> > +		/* Set the PLL configurations, P = 12, M = 800, S = 4. */
> > +		regmap_update_bits(bc->regmap, GPR_REG2,
> > +				   P_PLL_MASK | M_PLL_MASK | S_PLL_MASK,
> > +				   P_PLL | M_PLL | S_PLL);
> > +		udelay(1);
> > +
> > +		regmap_update_bits(bc->regmap, GPR_REG3, PLL_RST, PLL_RST);
> > +		udelay(10);
> > +
> > +		/* Set 1b'1 to pll_cke of GPR_REG3 */
> > +		regmap_update_bits(bc->regmap, GPR_REG3, PLL_CKE, PLL_CKE);
> >  		break;
> >  	default:
> >  		break;
>