| Message ID | 20191206160007.331801-2-jean.pihet@newoldbits.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TI QSPI: Add support for large flash devices | expand |
Hi Jean, * Jean Pihet <jean.pihet@newoldbits.com> [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? Regards, Tony > --- > drivers/spi/spi-ti-qspi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c > index 3cb65371ae3b..4680dad38ab2 100644 > --- a/drivers/spi/spi-ti-qspi.c > +++ b/drivers/spi/spi-ti-qspi.c > @@ -79,7 +79,7 @@ struct ti_qspi { > > #define QSPI_COMPLETION_TIMEOUT msecs_to_jiffies(2000) > > -#define QSPI_FCLK 192000000 > +#define QSPI_FCLK 48000000 > > /* Clock Control */ > #define QSPI_CLK_EN (1 << 31) > @@ -748,7 +748,7 @@ static int ti_qspi_probe(struct platform_device *pdev) > } > } > > - qspi->fclk = devm_clk_get(&pdev->dev, "fck"); > + qspi->fclk = devm_clk_get(&pdev->dev, "dpll_per_m2_div4_ck"); > if (IS_ERR(qspi->fclk)) { > ret = PTR_ERR(qspi->fclk); > dev_err(&pdev->dev, "could not get clk: %d\n", ret); > -- > 2.23.0 >
Hi Tony, On Fri, Dec 6, 2019 at 5:24 PM Tony Lindgren <tony@atomide.com> wrote: > > Hi Jean, > > * Jean Pihet <jean.pihet@newoldbits.com> [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? Thank you for reviewing. Regards, Jean > > Regards, > > Tony > > > --- > > drivers/spi/spi-ti-qspi.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c > > index 3cb65371ae3b..4680dad38ab2 100644 > > --- a/drivers/spi/spi-ti-qspi.c > > +++ b/drivers/spi/spi-ti-qspi.c > > @@ -79,7 +79,7 @@ struct ti_qspi { > > > > #define QSPI_COMPLETION_TIMEOUT msecs_to_jiffies(2000) > > > > -#define QSPI_FCLK 192000000 > > +#define QSPI_FCLK 48000000 > > > > /* Clock Control */ > > #define QSPI_CLK_EN (1 << 31) > > @@ -748,7 +748,7 @@ static int ti_qspi_probe(struct platform_device *pdev) > > } > > } > > > > - qspi->fclk = devm_clk_get(&pdev->dev, "fck"); > > + qspi->fclk = devm_clk_get(&pdev->dev, "dpll_per_m2_div4_ck"); > > if (IS_ERR(qspi->fclk)) { > > ret = PTR_ERR(qspi->fclk); > > dev_err(&pdev->dev, "could not get clk: %d\n", ret); > > -- > > 2.23.0 > >
* Jean Pihet <jean.pihet@newoldbits.com> [191206 16:30]: > Hi Tony, > > On Fri, Dec 6, 2019 at 5:24 PM Tony Lindgren <tony@atomide.com> wrote: > > > > Hi Jean, > > > > * Jean Pihet <jean.pihet@newoldbits.com> [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? 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 {
On 06/12/19 9:30 pm, Jean Pihet wrote: > 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. > --- > drivers/spi/spi-ti-qspi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c > index 3cb65371ae3b..4680dad38ab2 100644 > --- a/drivers/spi/spi-ti-qspi.c > +++ b/drivers/spi/spi-ti-qspi.c > @@ -79,7 +79,7 @@ struct ti_qspi { > > #define QSPI_COMPLETION_TIMEOUT msecs_to_jiffies(2000) > > -#define QSPI_FCLK 192000000 > +#define QSPI_FCLK 48000000 > This macro is unused and should be dropped. > /* Clock Control */ > #define QSPI_CLK_EN (1 << 31) > @@ -748,7 +748,7 @@ static int ti_qspi_probe(struct platform_device *pdev) > } > } > > - qspi->fclk = devm_clk_get(&pdev->dev, "fck"); > + qspi->fclk = devm_clk_get(&pdev->dev, "dpll_per_m2_div4_ck"); > if (IS_ERR(qspi->fclk)) { > ret = PTR_ERR(qspi->fclk); > dev_err(&pdev->dev, "could not get clk: %d\n", ret); >
Hi Vignesh, On Mon, Dec 9, 2019 at 10:59 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > > > On 06/12/19 9:30 pm, Jean Pihet wrote: > > 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. > > --- > > drivers/spi/spi-ti-qspi.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c > > index 3cb65371ae3b..4680dad38ab2 100644 > > --- a/drivers/spi/spi-ti-qspi.c > > +++ b/drivers/spi/spi-ti-qspi.c > > @@ -79,7 +79,7 @@ struct ti_qspi { > > > > #define QSPI_COMPLETION_TIMEOUT msecs_to_jiffies(2000) > > > > -#define QSPI_FCLK 192000000 > > +#define QSPI_FCLK 48000000 > > > > This macro is unused and should be dropped. That is correct. It is a left over from older versions. Will remove it. Thanks, Jean > > > /* Clock Control */ > > #define QSPI_CLK_EN (1 << 31) > > @@ -748,7 +748,7 @@ static int ti_qspi_probe(struct platform_device *pdev) > > } > > } > > > > - qspi->fclk = devm_clk_get(&pdev->dev, "fck"); > > + qspi->fclk = devm_clk_get(&pdev->dev, "dpll_per_m2_div4_ck"); > > if (IS_ERR(qspi->fclk)) { > > ret = PTR_ERR(qspi->fclk); > > dev_err(&pdev->dev, "could not get clk: %d\n", ret); > > > > -- > Regards > Vignesh
Hi Tony, Tero, On Fri, Dec 6, 2019 at 6:57 PM Tony Lindgren <tony@atomide.com> wrote: > > * Jean Pihet <jean.pihet@newoldbits.com> [191206 16:30]: > > Hi Tony, > > > > On Fri, Dec 6, 2019 at 5:24 PM Tony Lindgren <tony@atomide.com> wrote: > > > > > > Hi Jean, > > > > > > * Jean Pihet <jean.pihet@newoldbits.com> [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
Hi, * Jean Pihet <jean.pihet@newoldbits.com> [191210 16:24]: > On Fri, Dec 6, 2019 at 6:57 PM Tony Lindgren <tony@atomide.com> wrote: > > 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. Hmm OK so I think we need fck at both the module level and qspi level. > 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. Yes you can configure both fck and ick there, and also additional clocks. But the clkctrl clock is the fck clock gate for this module, and it's source can be the same as the interface clock for some modules. When I sent the experimental patch I confirmed that just the fck as <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0>, ti-sysc.c driver can read the qspi module revision register just fine. So that means that it's enough for the module, and the spi_clk is another clock specific to the child qspi IP in the module. So based on that, I think we should set up the clocks in the following way for the module and it's qspi child: target-module@47900000 { ... clocks = <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0>; clock-names = "fck"; ... qspi: spi@0 { ... clocks = <&dpll_per_m2_div4_ck>; clock-names = "fck"; ... }; }; That way the qspi driver can set the divider on it's fck based on "spi-max-frequency" dts property. > 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"; Yeah so that's <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0> for am437x. > What is best to do from here? Well can test again with the patch below to see if that is enough to make it work :) 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,35 @@ gpmc: gpmc@50000000 { 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"; + clocks = <&dpll_per_m2_div4_ck>; + clock-names = "fck"; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <0 138 0x4>; + num-cs = <4>; + }; }; dss: dss@4832a000 {
Hi Tony, On Tue, Dec 10, 2019 at 6:03 PM Tony Lindgren <tony@atomide.com> wrote: > > Hi, > > * Jean Pihet <jean.pihet@newoldbits.com> [191210 16:24]: > > On Fri, Dec 6, 2019 at 6:57 PM Tony Lindgren <tony@atomide.com> wrote: > > > 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. > > Hmm OK so I think we need fck at both the module level and qspi level. > > > 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. > > Yes you can configure both fck and ick there, and also additional > clocks. But the clkctrl clock is the fck clock gate for this module, > and it's source can be the same as the interface clock for some > modules. > > When I sent the experimental patch I confirmed that just the fck > as <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0>, ti-sysc.c driver can > read the qspi module revision register just fine. So that means > that it's enough for the module, and the spi_clk is another > clock specific to the child qspi IP in the module. > > So based on that, I think we should set up the clocks in the > following way for the module and it's qspi child: > > target-module@47900000 { > ... > clocks = <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0>; > clock-names = "fck"; > ... > > qspi: spi@0 { > ... > clocks = <&dpll_per_m2_div4_ck>; > clock-names = "fck"; > ... > }; > }; > > That way the qspi driver can set the divider on it's fck based > on "spi-max-frequency" dts property. > > > 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"; > > Yeah so that's <&l3s_clkctrl AM4_L3S_QSPI_CLKCTRL 0> for > am437x. > > > What is best to do from here? > > Well can test again with the patch below to see if that is > enough to make it work :) This patch works OK! The correct clock is in use by the driver. The hwmod warning shows up at boot: [ 0.103567] omap_hwmod: qspi: no dt node [ 0.103599] ------------[ cut here ]------------ [ 0.103639] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2414 _init.constprop.29+0x198/0x4a0 [ 0.103654] omap_hwmod: qspi: doesn't have mpu register target base Glad to help to get to the final solution, please let me know how I can help on that. Regards, Jean > > 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,35 @@ gpmc: gpmc@50000000 { > 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"; > + clocks = <&dpll_per_m2_div4_ck>; > + clock-names = "fck"; > + #address-cells = <1>; > + #size-cells = <0>; > + interrupts = <0 138 0x4>; > + num-cs = <4>; > + }; > }; > > dss: dss@4832a000 { > -- > 2.24.0
* Jean Pihet <jean.pihet@newoldbits.com> [191210 19:22]: > On Tue, Dec 10, 2019 at 6:03 PM Tony Lindgren <tony@atomide.com> wrote: > > Well can test again with the patch below to see if that is > > enough to make it work :) > > This patch works OK! The correct clock is in use by the driver. The > hwmod warning shows up at boot: > [ 0.103567] omap_hwmod: qspi: no dt node > [ 0.103599] ------------[ cut here ]------------ > [ 0.103639] WARNING: CPU: 0 PID: 1 at > arch/arm/mach-omap2/omap_hwmod.c:2414 _init.constprop.29+0x198/0x4a0 > [ 0.103654] omap_hwmod: qspi: doesn't have mpu register target base OK good to hear. That warning will go away when the legacy platform data is removed. So the patch needs to initially still keep the "ti,hwmods" property until we remove the legacy platform data. > Glad to help to get to the final solution, please let me know how I > can help on that. Well is this needed as a fix or can it wait for the v5.6 merge window? If it's needed as a fix, some kind of description for the issue fixed is needed. Any ideas there? We know the right clock is not found by the driver, but I'm now wondering if this ever worked or has there been some bootloader dependency? Regards, Tony
Tony, On Tue, Dec 10, 2019 at 10:02 PM Tony Lindgren <tony@atomide.com> wrote: > > * Jean Pihet <jean.pihet@newoldbits.com> [191210 19:22]: > > On Tue, Dec 10, 2019 at 6:03 PM Tony Lindgren <tony@atomide.com> wrote: > > > Well can test again with the patch below to see if that is > > > enough to make it work :) > > > > This patch works OK! The correct clock is in use by the driver. The > > hwmod warning shows up at boot: > > [ 0.103567] omap_hwmod: qspi: no dt node > > [ 0.103599] ------------[ cut here ]------------ > > [ 0.103639] WARNING: CPU: 0 PID: 1 at > > arch/arm/mach-omap2/omap_hwmod.c:2414 _init.constprop.29+0x198/0x4a0 > > [ 0.103654] omap_hwmod: qspi: doesn't have mpu register target base > > OK good to hear. That warning will go away when the legacy platform > data is removed. So the patch needs to initially still keep the > "ti,hwmods" property until we remove the legacy platform data. Can the patch be submitted like it is now? If so I am preparing a v2 series. > > > Glad to help to get to the final solution, please let me know how I > > can help on that. > > Well is this needed as a fix or can it wait for the v5.6 merge window? It can wait since this is an improvement, not a bugfix. Does that sound good? > > If it's needed as a fix, some kind of description for the issue > fixed is needed. Any ideas there? > > We know the right clock is not found by the driver, but I'm now > wondering if this ever worked or has there been some bootloader > dependency? The motivation is to optimize the SPI transfer speed, especially for the SPI flash devices. With the current code if a 48MHz SPI clock is required, the effective clock will be at a 16MHz frequency. There is no bootloader dependency afaik, U-Boot uses a macro that defines the fixed 48MHz of the PER clock. This description comes in the patch desciption. Thanks! Regards, Jean > > Regards, > > Tony
* Jean Pihet <jean.pihet@newoldbits.com> [191210 21:46]: > Tony, > > On Tue, Dec 10, 2019 at 10:02 PM Tony Lindgren <tony@atomide.com> wrote: > > > > * Jean Pihet <jean.pihet@newoldbits.com> [191210 19:22]: > > > On Tue, Dec 10, 2019 at 6:03 PM Tony Lindgren <tony@atomide.com> wrote: > > > > Well can test again with the patch below to see if that is > > > > enough to make it work :) > > > > > > This patch works OK! The correct clock is in use by the driver. The > > > hwmod warning shows up at boot: > > > [ 0.103567] omap_hwmod: qspi: no dt node > > > [ 0.103599] ------------[ cut here ]------------ > > > [ 0.103639] WARNING: CPU: 0 PID: 1 at > > > arch/arm/mach-omap2/omap_hwmod.c:2414 _init.constprop.29+0x198/0x4a0 > > > [ 0.103654] omap_hwmod: qspi: doesn't have mpu register target base > > > > OK good to hear. That warning will go away when the legacy platform > > data is removed. So the patch needs to initially still keep the > > "ti,hwmods" property until we remove the legacy platform data. > Can the patch be submitted like it is now? If so I am preparing a v2 series. I just posted these two patches with you in Cc: ARM: OMAP2+: Drop legacy platform data for am4 qspi ARM: dts: Configure interconnect target module for am4 qspi I'll be adding these two into omap-for-v5.6/dt in few days if no comments. It seems the driver changes can be sent separately from the dts changes. > > > Glad to help to get to the final solution, please let me know how I > > > can help on that. > > > > Well is this needed as a fix or can it wait for the v5.6 merge window? > It can wait since this is an improvement, not a bugfix. Does that sound good? OK sounds good to me. > > If it's needed as a fix, some kind of description for the issue > > fixed is needed. Any ideas there? > > > > We know the right clock is not found by the driver, but I'm now > > wondering if this ever worked or has there been some bootloader > > dependency? > The motivation is to optimize the SPI transfer speed, especially for > the SPI flash devices. > With the current code if a 48MHz SPI clock is required, the effective > clock will be at a 16MHz frequency. > There is no bootloader dependency afaik, U-Boot uses a macro that > defines the fixed 48MHz of the PER clock. Oh OK, makes sense now. So wrong clock rate, thanks for explaining. Regards, Tony
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c index 3cb65371ae3b..4680dad38ab2 100644 --- a/drivers/spi/spi-ti-qspi.c +++ b/drivers/spi/spi-ti-qspi.c @@ -79,7 +79,7 @@ struct ti_qspi { #define QSPI_COMPLETION_TIMEOUT msecs_to_jiffies(2000) -#define QSPI_FCLK 192000000 +#define QSPI_FCLK 48000000 /* Clock Control */ #define QSPI_CLK_EN (1 << 31) @@ -748,7 +748,7 @@ static int ti_qspi_probe(struct platform_device *pdev) } } - qspi->fclk = devm_clk_get(&pdev->dev, "fck"); + qspi->fclk = devm_clk_get(&pdev->dev, "dpll_per_m2_div4_ck"); if (IS_ERR(qspi->fclk)) { ret = PTR_ERR(qspi->fclk); dev_err(&pdev->dev, "could not get clk: %d\n", ret);