Re: [PATCH 1/3] TI QSPI: Fix fclk frequency

[Jean Pihet <jean.pihet@xxxxxxxxxxxxxx>]

Hi Tony, Tero,

On Fri, Dec 6, 2019 at 6:57 PM Tony Lindgren <tony@xxxxxxxxxxx> wrote:
>
> * Jean Pihet <jean.pihet@xxxxxxxxxxxxxx> [191206 16:30]:
> > Hi Tony,
> >
> > On Fri, Dec 6, 2019 at 5:24 PM Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > >
> > > Hi Jean,
> > >
> > > * Jean Pihet <jean.pihet@xxxxxxxxxxxxxx> [191206 16:01]:
> > > > The QSPI IP is clocked by two clocks:
> > > > - CORE_CLKOUTM4 / 2 (L3) as interface clock,
> > > > - PER_CLKOUTM2 / 4 (L4) as functional clock, which is PER_CLKOUTM2
> > > >   divided by 4, so at 192Mhz / 4 = 48MHz.
> > > >
> > > > Fix the use of the correct fclk by the driver and fix the frequency
> > > > value so that the divider is correctly programmed to generate the
> > > > desired frequency of QSPI_CLK.
> > >
> > > This source clock can be different between the SoC models, the
> > > related fck probably needs to be fixed in the SoC specific dtsi
> > > file.
> > >
> > > Currently qspi it's there only in dra7.dtsi, sounds like you
> > > are using it on am3/am4 based on the clock name?
> >
> > I am using the AM4376 chipset. Only the interface is fixed in the
> > hwmod data as fck.
> > What is the best solution to add the extra fck?
>
> Well the long term solution would be to make it probe with
> ti-sysc interconnect target module driver, then you can specify
> both fck and ick there as needed.
>
> Care to test with the patch below (without your changes) to see if
> something else is still needed?

With the patch applied fck still is the ick, not the L4 clock as required.

Could both ick and fck be defined in the DT files, like here below?
   clocks = <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0>,
                 <&l4per2_clkctrl AM4_L4PER2_QSPI_CLKCTRL 0>;
   clock-names = "ick", "fck";
The issue is that there is no l4_per for AM4.

Looking at the DRA7 DT files there is an fck defined (in dra7.dtsi):
   clocks = <&l4per2_clkctrl DRA7_L4PER2_QSPI_CLKCTRL 25>;
   clock-names = "fck";

What is best to do from here?

Thanks,
Jean

>
> I only added the fck there, not sure if we should also the l3 ick.
> Eventually this node will be a child of the l3 interconnect, and
> genpd will ensure the l3 ick is in use when pm_runtime_get() is
> done in the qspi driver.
>
> Note that this will produce a boot time warning until the related
> hwmod data is dropped. I'll be posting a proper patch once we
> know what's going on here, not sure what's the right fix for the
> clock issue for v5.5-rc series though.
>
> Regards,
>
> Tony
>
> 8< ----------------------
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -302,17 +302,33 @@
>                         status = "disabled";
>                 };
>
> -               qspi: spi@47900000 {
> -                       compatible = "ti,am4372-qspi";
> -                       reg = <0x47900000 0x100>,
> -                             <0x30000000 0x4000000>;
> -                       reg-names = "qspi_base", "qspi_mmap";
> +               target-module@47900000 {
> +                       compatible = "ti,sysc-omap4", "ti,sysc";
> +                       //ti,hwmods = "qspi";
> +                       reg = <0x47900000 0x4>,
> +                             <0x47900010 0x4>;
> +                       reg-names = "rev", "sysc";
> +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +                                       <SYSC_IDLE_NO>,
> +                                       <SYSC_IDLE_SMART>,
> +                                       <SYSC_IDLE_SMART_WKUP>;
> +                       clocks = <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0>;
> +                       clock-names = "fck";
>                         #address-cells = <1>;
> -                       #size-cells = <0>;
> -                       ti,hwmods = "qspi";
> -                       interrupts = <0 138 0x4>;
> -                       num-cs = <4>;
> -                       status = "disabled";
> +                       #size-cells = <1>;
> +                       ranges = <0x0 0x47900000 0x1000>,
> +                                <0x30000000 0x30000000 0x4000000>;
> +
> +                       qspi: spi@0 {
> +                               compatible = "ti,am4372-qspi";
> +                               reg = <0 0x100>,
> +                                     <0x30000000 0x4000000>;
> +                               reg-names = "qspi_base", "qspi_mmap";
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               interrupts = <0 138 0x4>;
> +                               num-cs = <4>;
> +                       };
>                 };
>
>                 dss: dss@4832a000 {
> --
> 2.24.0