[PATCH V3 13/14] ARM: dts: stm32: Adjust PLL4 settings on AV96

[Manivannan Sadhasivam]

Hi,

On Wed, Apr 01, 2020 at 04:49:45PM +0000, Gerald BAEZA wrote:
> Hi Marek and Patrick
> 
> > From: Marek Vasut <marex at denx.de>
> > Sent: mercredi 1 avril 2020 13:09
> > 
> > On 4/1/20 12:24 PM, Patrick DELAUNAY wrote:
> > > Hi Gerald and Manivannan,
> > 
> > Hi,
> > 
> > >> From: Marek Vasut <marex at denx.de>
> > >> Sent: mardi 31 mars 2020 19:52
> > >>
> > >> The PLL4 is supplying SDMMC12, SDMMC3 and SPDIF with 120 MHz and
> > >> FDCAN with 96 MHz. This isn't good for the SDMMC interfaces, which
> > >> can not easily divide the clock down to e.g. 50 MHz for high speed SD
> > >> and eMMC devices, so those devices end up running at 30 MHz as that is
> > 120 MHz / 4.
> > >> Adjust the PLL4 settings such that both PLL4P and PLL4R run at 100
> > >> MHz instead, which is easy to divide to 50MHz for optimal operation
> > >> of both SD and eMMC, SPDIF clock are not that much slower and FDCAN is
> > also unaffected.
> 
> As far as I see, SPDIF is not supported on Avenger board so we don't care for this one.
> FDCAN can be supported via the expansion connector, so better to keep it high (and < 100 MHz).
> Be careful because the LTDC pixel clock also comes from the PLL4Q and it is used for the HDMI on Avenger.
> The pixel clock is very constraint and I am surprised by the initial 40 MHz configuration (that becomes 50 MHz with your patch).
> I would recommend to align the Avenger configuration to ST boards one, that is the best compromise found so far (99 MHz for SDMMC and 74.250 MHz for HDMI):
> 	https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree
> 	/* VCO = 594.0 MHz => P = 99, Q = 74, R = 74 */
> 	pll4: st,pll at 3 {
> 		cfg = < 3 98 5 7 7 PQR(1,1,1) >;
> 		u-boot,dm-pre-reloc;
> 	};
> 

This looks good to me.

> > >> Reviewed-by: Patrice Chotard <patrice.chotard at st.com>
> > >> Reviewed-by: Patrick Delaunay <patrick.delaunay at st.com>
> > >> Signed-off-by: Marek Vasut <marex at denx.de>
> > >> Cc: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>
> > >> Cc: Patrick Delaunay <patrick.delaunay at st.com>
> > >> Cc: Patrice Chotard <patrice.chotard at st.com>
> > >> ---
> > >> V2: Move this patch before the split of AV96 into SoM and carrier
> > >> V3: No change
> > >> ---
> > >
> > > This patch update the PLL4 frequency used on AV96 board, with
> > > different of reference clock tree used on ST board, this new setting
> > > allow to optimize the SDMMC frequency (50MHz vs 30Mz).
> > >
> > > I don't know why the previous PLL4 frequency was chosen as a
> > > compromise on reference clock-tree  (PLL4 is used by mostly all the
> > > peripheral, with display and audio requirements).
> > >
> > > Can you cross check the proposed clock tree and ack this patch if
> > > these ST requirements are not applicable on AV96 board.
> > >
> > > Anyway the code is correct.
> > 
> > Likely because these PLL settings are being copied from reference platform
> > to other platforms etc.
> 
> As far as I remember, we never had this configuration for PLL4 on ST boards, so the copy certainly comes from somewhere else.
> 

I took the PLL settings from Downstream code done by Arrow at that time. Since
I never had access to PMIC info, I just used them blindly since that worked for
initial upstream bringup on old board. We had several discussions about PMIC
at that time but never touched PLL4.

So feel free to modify it as Gerald suggested.

Thanks,
Mani

> > But I did notice one odd thing, which is when running the SD1 in SDR104, the
> > read data transfers can be unstable, which I suspect is because the bus runs
> > at actual 100 MHz instead of some 60 MHz. I need to look at that with a
> > scope, so that's to be checked. For now I turned the SDR104 off.
>