| Message ID | 20190318075158.21784-1-judge.packham@gmail.com |
|---|---|
| State | Accepted |
| Commit | 599f7aa541bb5a658cbfd2af73bd9d2f6e828d43 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [U-Boot,v2] ARM: kirkwood: disable dcache for Kirkwood boards | expand |
On 18.03.19 08:51, Chris Packham wrote: > Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate cache > routines to common file") the kirkwood boards didn't have and dcache > support. The network and usb drivers rely on this. Set > CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. > > Reported-by: Leigh Brown <leigh@solinno.co.uk> > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > > Changes in v2: > - expand the comment in config.h to provide more info > > arch/arm/mach-kirkwood/include/mach/config.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/mach-kirkwood/include/mach/config.h b/arch/arm/mach-kirkwood/include/mach/config.h > index fcd903887bff..aea60688c2d3 100644 > --- a/arch/arm/mach-kirkwood/include/mach/config.h > +++ b/arch/arm/mach-kirkwood/include/mach/config.h > @@ -26,6 +26,12 @@ > #define CONFIG_KIRKWOOD_EGIGA_INIT /* Enable GbePort0/1 for kernel */ > #define CONFIG_KIRKWOOD_RGMII_PAD_1V8 /* Set RGMII Pad voltage to 1.8V */ > #define CONFIG_KIRKWOOD_PCIE_INIT /* Enable PCIE Port0 for kernel */ > +/* > + * Disable the dcache. Currently the network driver (mvgbe.c) and USB > + * EHCI driver (ehci-marvell.c) and possibly others rely on the data > + * cache being disabled. > + */ > +#define CONFIG_SYS_DCACHE_OFF Thanks. Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
On 18.03.19 08:51, Chris Packham wrote: > Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate cache > routines to common file") the kirkwood boards didn't have and dcache > support. The network and usb drivers rely on this. Set > CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. > > Reported-by: Leigh Brown <leigh@solinno.co.uk> > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > > Changes in v2: > - expand the comment in config.h to provide more info > > arch/arm/mach-kirkwood/include/mach/config.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/mach-kirkwood/include/mach/config.h b/arch/arm/mach-kirkwood/include/mach/config.h > index fcd903887bff..aea60688c2d3 100644 > --- a/arch/arm/mach-kirkwood/include/mach/config.h > +++ b/arch/arm/mach-kirkwood/include/mach/config.h > @@ -26,6 +26,12 @@ > #define CONFIG_KIRKWOOD_EGIGA_INIT /* Enable GbePort0/1 for kernel */ > #define CONFIG_KIRKWOOD_RGMII_PAD_1V8 /* Set RGMII Pad voltage to 1.8V */ > #define CONFIG_KIRKWOOD_PCIE_INIT /* Enable PCIE Port0 for kernel */ > +/* > + * Disable the dcache. Currently the network driver (mvgbe.c) and USB > + * EHCI driver (ehci-marvell.c) and possibly others rely on the data > + * cache being disabled. > + */ > +#define CONFIG_SYS_DCACHE_OFF While collecting the queued fixes for the upcoming release, I do have one question regarding this Kirkwood cache issue (I don't have such a board here, so I can't test anything): Do we need this patch applied [1], if the patch from this thread is applied [2]? Thanks, Stefan [1] http://patchwork.ozlabs.org/patch/1048863/ [2] http://patchwork.ozlabs.org/patch/1057716/
On Tue, 19 Mar 2019, 8:39 PM Stefan Roese, <sr@denx.de> wrote: > > > On 18.03.19 08:51, Chris Packham wrote: > > Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate cache > > routines to common file") the kirkwood boards didn't have and dcache > > support. The network and usb drivers rely on this. Set > > CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. > > > > Reported-by: Leigh Brown <leigh@solinno.co.uk> > > Signed-off-by: Chris Packham <judge.packham@gmail.com> > > --- > > > > Changes in v2: > > - expand the comment in config.h to provide more info > > > > arch/arm/mach-kirkwood/include/mach/config.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/arch/arm/mach-kirkwood/include/mach/config.h > b/arch/arm/mach-kirkwood/include/mach/config.h > > index fcd903887bff..aea60688c2d3 100644 > > --- a/arch/arm/mach-kirkwood/include/mach/config.h > > +++ b/arch/arm/mach-kirkwood/include/mach/config.h > > @@ -26,6 +26,12 @@ > > #define CONFIG_KIRKWOOD_EGIGA_INIT /* Enable GbePort0/1 for kernel */ > > #define CONFIG_KIRKWOOD_RGMII_PAD_1V8 /* Set RGMII Pad voltage > to 1.8V */ > > #define CONFIG_KIRKWOOD_PCIE_INIT /* Enable PCIE Port0 for > kernel */ > > +/* > > + * Disable the dcache. Currently the network driver (mvgbe.c) and USB > > + * EHCI driver (ehci-marvell.c) and possibly others rely on the data > > + * cache being disabled. > > + */ > > +#define CONFIG_SYS_DCACHE_OFF > > While collecting the queued fixes for the upcoming release, I do > have one question regarding this Kirkwood cache issue (I don't have > such a board here, so I can't test anything): > > Do we need this patch applied [1], if the patch from this thread is > applied [2]? > > Thanks, > Stefan > > [1] http://patchwork.ozlabs.org/patch/1048863/ > [2] http://patchwork.ozlabs.org/patch/1057716/ I'm pretty sure you just need [2]. I've tested with just that on it's own and Ethernet works on the kirkwood db. Functionally it's the same change as Michael's just for more boards. Leigh, can you see if [2] above works for you?
On 19.03.19 09:00, Chris Packham wrote: > > > On Tue, 19 Mar 2019, 8:39 PM Stefan Roese, <sr@denx.de <mailto:sr@denx.de>> wrote: > > > > On 18.03.19 08:51, Chris Packham wrote: > > Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate cache > > routines to common file") the kirkwood boards didn't have and dcache > > support. The network and usb drivers rely on this. Set > > CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. > > > > Reported-by: Leigh Brown <leigh@solinno.co.uk <mailto:leigh@solinno.co.uk>> > > Signed-off-by: Chris Packham <judge.packham@gmail.com <mailto:judge.packham@gmail.com>> > > --- > > > > Changes in v2: > > - expand the comment in config.h to provide more info > > > > arch/arm/mach-kirkwood/include/mach/config.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/arch/arm/mach-kirkwood/include/mach/config.h b/arch/arm/mach-kirkwood/include/mach/config.h > > index fcd903887bff..aea60688c2d3 100644 > > --- a/arch/arm/mach-kirkwood/include/mach/config.h > > +++ b/arch/arm/mach-kirkwood/include/mach/config.h > > @@ -26,6 +26,12 @@ > > #define CONFIG_KIRKWOOD_EGIGA_INIT /* Enable GbePort0/1 for kernel */ > > #define CONFIG_KIRKWOOD_RGMII_PAD_1V8 /* Set RGMII Pad voltage to 1.8V */ > > #define CONFIG_KIRKWOOD_PCIE_INIT /* Enable PCIE Port0 for kernel */ > > +/* > > + * Disable the dcache. Currently the network driver (mvgbe.c) and USB > > + * EHCI driver (ehci-marvell.c) and possibly others rely on the data > > + * cache being disabled. > > + */ > > +#define CONFIG_SYS_DCACHE_OFF > > While collecting the queued fixes for the upcoming release, I do > have one question regarding this Kirkwood cache issue (I don't have > such a board here, so I can't test anything): > > Do we need this patch applied [1], if the patch from this thread is > applied [2]? > > Thanks, > Stefan > > [1] http://patchwork.ozlabs.org/patch/1048863/ > [2] http://patchwork.ozlabs.org/patch/1057716/ > > > I'm pretty sure you just need [2]. I've tested with just that on it's > own and Ethernet works on the kirkwood db. Functionally it's the same > change as Michael's just for more boards. > > Leigh, can you see if [2] above works for you? Patch [1] from Leigh also removes icache_enable() in arch/arm/mach-kirkwood/cpu.c. Not sure if this is still needed? Did any of you run into issues with [1] not applied but with CONFIG_SYS_DCACHE_OFF set? Thanks, Stefan
Hi Stefan, On 2019-03-19 08:04, Stefan Roese wrote: > On 19.03.19 09:00, Chris Packham wrote: >> >> >> On Tue, 19 Mar 2019, 8:39 PM Stefan Roese, <sr@denx.de >> <mailto:sr@denx.de>> wrote: >> >> >> >> On 18.03.19 08:51, Chris Packham wrote: >> > Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate >> cache >> > routines to common file") the kirkwood boards didn't have and >> dcache >> > support. The network and usb drivers rely on this. Set >> > CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. >> > >> > Reported-by: Leigh Brown <leigh@solinno.co.uk >> <mailto:leigh@solinno.co.uk>> >> > Signed-off-by: Chris Packham <judge.packham@gmail.com >> <mailto:judge.packham@gmail.com>> >> > --- >> > >> > Changes in v2: >> > - expand the comment in config.h to provide more info >> > >> > arch/arm/mach-kirkwood/include/mach/config.h | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> > diff --git a/arch/arm/mach-kirkwood/include/mach/config.h >> b/arch/arm/mach-kirkwood/include/mach/config.h >> > index fcd903887bff..aea60688c2d3 100644 >> > --- a/arch/arm/mach-kirkwood/include/mach/config.h >> > +++ b/arch/arm/mach-kirkwood/include/mach/config.h >> > @@ -26,6 +26,12 @@ >> > #define CONFIG_KIRKWOOD_EGIGA_INIT /* Enable GbePort0/1 for >> kernel */ >> > #define CONFIG_KIRKWOOD_RGMII_PAD_1V8 /* Set RGMII Pad >> voltage to 1.8V */ >> > #define CONFIG_KIRKWOOD_PCIE_INIT /* Enable PCIE Port0 >> for kernel */ >> > +/* >> > + * Disable the dcache. Currently the network driver (mvgbe.c) >> and USB >> > + * EHCI driver (ehci-marvell.c) and possibly others rely on >> the data >> > + * cache being disabled. >> > + */ >> > +#define CONFIG_SYS_DCACHE_OFF >> >> While collecting the queued fixes for the upcoming release, I do >> have one question regarding this Kirkwood cache issue (I don't >> have >> such a board here, so I can't test anything): >> >> Do we need this patch applied [1], if the patch from this thread >> is >> applied [2]? >> >> Thanks, >> Stefan >> >> [1] http://patchwork.ozlabs.org/patch/1048863/ >> [2] http://patchwork.ozlabs.org/patch/1057716/ >> >> >> I'm pretty sure you just need [2]. I've tested with just that on it's >> own and Ethernet works on the kirkwood db. Functionally it's the same >> change as Michael's just for more boards. >> >> Leigh, can you see if [2] above works for you? > > Patch [1] from Leigh also removes icache_enable() in > arch/arm/mach-kirkwood/cpu.c. > > Not sure if this is still needed? Did any of you run into issues with > [1] not applied but with CONFIG_SYS_DCACHE_OFF set? When the cache consolidation patch was applied, it added code to enable the icache much earlier in the process. Therefore, this line of code to enable it became superfluous (assuming enabling the icache multiple times does nothing). I'd suggest removing it if you agree with that statement, to reduce possible future confusion. > > Thanks, > Stefan Regards, Leigh.
Hi Leigh, On 19.03.19 10:29, Leigh Brown wrote: <snip> >>> While collecting the queued fixes for the upcoming release, I do >>> have one question regarding this Kirkwood cache issue (I don't >>> have >>> such a board here, so I can't test anything): >>> >>> Do we need this patch applied [1], if the patch from this thread >>> is >>> applied [2]? >>> >>> Thanks, >>> Stefan >>> >>> [1] http://patchwork.ozlabs.org/patch/1048863/ >>> [2] http://patchwork.ozlabs.org/patch/1057716/ >>> >>> >>> I'm pretty sure you just need [2]. I've tested with just that on it's >>> own and Ethernet works on the kirkwood db. Functionally it's the same >>> change as Michael's just for more boards. >>> >>> Leigh, can you see if [2] above works for you? >> >> Patch [1] from Leigh also removes icache_enable() in >> arch/arm/mach-kirkwood/cpu.c. >> >> Not sure if this is still needed? Did any of you run into issues with >> [1] not applied but with CONFIG_SYS_DCACHE_OFF set? > > When the cache consolidation patch was applied, it added code to enable > the > icache much earlier in the process. Therefore, this line of code to > enable > it became superfluous (assuming enabling the icache multiple times does > nothing). > > I'd suggest removing it if you agree with that statement, to reduce > possible future confusion. You mean to remove the icache_enable() call in arch/arm/mach-kirkwood/cpu.c? Yes, with your explanation above I agree with it. Unfortunately your patch also sets CONFIG_SYS_DCACHE_OFF for dreamplug which is not needed any more with Chris's generic approach for Kirkwood. Also I find your patch subject: Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug not optimal. Could you please send a new patch with a short subject and a small commit message with the background (icache is enabled earlier in the common code now) to the list? That would be great. Thanks, Stefan
On 18.03.19 08:51, Chris Packham wrote: > Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate cache > routines to common file") the kirkwood boards didn't have and dcache > support. The network and usb drivers rely on this. Set > CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. > > Reported-by: Leigh Brown <leigh@solinno.co.uk> > Signed-off-by: Chris Packham <judge.packham@gmail.com> Applied to u-boot-marvell/master. Thanks, Stefan
diff --git a/arch/arm/mach-kirkwood/include/mach/config.h b/arch/arm/mach-kirkwood/include/mach/config.h index fcd903887bff..aea60688c2d3 100644 --- a/arch/arm/mach-kirkwood/include/mach/config.h +++ b/arch/arm/mach-kirkwood/include/mach/config.h @@ -26,6 +26,12 @@ #define CONFIG_KIRKWOOD_EGIGA_INIT /* Enable GbePort0/1 for kernel */ #define CONFIG_KIRKWOOD_RGMII_PAD_1V8 /* Set RGMII Pad voltage to 1.8V */ #define CONFIG_KIRKWOOD_PCIE_INIT /* Enable PCIE Port0 for kernel */ +/* + * Disable the dcache. Currently the network driver (mvgbe.c) and USB + * EHCI driver (ehci-marvell.c) and possibly others rely on the data + * cache being disabled. + */ +#define CONFIG_SYS_DCACHE_OFF /* * By default kwbimage.cfg from board specific folder is used
Prior to commit 93b283d49f93 ("ARM: CPU: arm926ejs: Consolidate cache routines to common file") the kirkwood boards didn't have and dcache support. The network and usb drivers rely on this. Set CONFIG_SYS_DCACHE_OFF in the Kirkwood specific config.h. Reported-by: Leigh Brown <leigh@solinno.co.uk> Signed-off-by: Chris Packham <judge.packham@gmail.com> --- Changes in v2: - expand the comment in config.h to provide more info arch/arm/mach-kirkwood/include/mach/config.h | 6 ++++++ 1 file changed, 6 insertions(+)