> -----Original Message----- > From: Russell King - ARM Linux [mailto:li...@armlinux.org.uk] > Sent: 2017年1月11日 19:18 > To: Jean-Jacques Hiblot <jjhib...@gmail.com> > Cc: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; Alexandre Belloni > <alexandre.bell...@free-electrons.com>; Mark Rutland <mark.rutl...@arm.com>; > devicetree <devicet...@vger.kernel.org>; Nicolas Ferre > <nicolas.fe...@atmel.com>; Linux Kernel Mailing List <linux- > ker...@vger.kernel.org>; robh+dt <robh...@kernel.org>; linux-arm- > ker...@lists.infradead.org > Subject: Re: [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu > idle > > On Wed, Jan 11, 2017 at 12:05:05PM +0100, Jean-Jacques Hiblot wrote: > > 2017-01-11 9:15 GMT+01:00 <wenyou.y...@microchip.com>: > > > Hi Jean-Jacques, > > > > > >> -----Original Message----- > > >> From: Jean-Jacques Hiblot [mailto:jjhib...@gmail.com] > > >> Sent: 2017年1月11日 0:51 > > >> To: Alexandre Belloni <alexandre.bell...@free-electrons.com> > > >> Cc: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; Mark Rutland > > >> <mark.rutl...@arm.com>; devicetree <devicet...@vger.kernel.org>; > > >> Russell King <li...@arm.linux.org.uk>; Wenyou Yang - A41535 > > >> <wenyou.y...@microchip.com>; Nicolas Ferre > > >> <nicolas.fe...@atmel.com>; Linux Kernel Mailing List > > >> <linux-kernel@vger.kernel.org>; Rob Herring <robh...@kernel.org>; > > >> linux-arm-ker...@lists.infradead.org > > >> Subject: Re: [PATCH 1/3] ARM: at91: flush the L2 cache before > > >> entering cpu idle > > >> > > >> 2017-01-10 17:18 GMT+01:00 Alexandre Belloni > > >> <alexandre.bell...@free-electrons.com>: > > >> > I though a bit more about it, and I don't really like the new > > >> > compatible string. I don't feel this should be necessary. > > >> > > > >> > What about the following: > > >> > > > >> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c > > >> > index > > >> > b4332b727e9c..0333aca63e44 100644 > > >> > --- a/arch/arm/mach-at91/pm.c > > >> > +++ b/arch/arm/mach-at91/pm.c > > >> > @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void); > > >> > static struct { > > >> > unsigned long uhp_udp_mask; > > >> > int memctrl; > > >> > + bool has_l2_cache; > > >> > } at91_pm_data; > > >> > > > >> > void __iomem *at91_ramc_base[2]; @@ -267,6 +268,11 @@ static > > >> > void at91_ddr_standby(void) > > >> > u32 lpr0, lpr1 = 0; > > >> > u32 saved_lpr0, saved_lpr1 = 0; > > >> > > > >> > > >> > + if (at91_pm_data.has_l2_cache) { > > >> > + flush_cache_all(); > > >> what is the point of calling flush_cache_all() here ? Do we really > > >> care that dirty data in L1 is written to DDR ? I may be missing > > >> something but to me it's just extra latency. > > > > > > Are you mean use outer_flush_all() to flush all cache lines in the outer > > > cache > only? > > > > Yes that's what I meant. You see, you don't flush the cache for > > sama5d3 so it shouldn't be required either for sam5d4. You should be > > able to test it quickly and see if L1 flush is indeed required by > > replacing flush_cache_all() with outer_flush_all(). BTW is highly > > probable that L2 cache flush is done in outer_disable() so calling > > outer_flush_all() is probably no required. > > Please don't. Read the comments in the code, and understand the APIs that > you're suggesting people use _before_ making the suggestion: > > /** > * outer_flush_all - clean and invalidate all cache lines in the outer cache > * > * Note: depending on implementation, this may not be atomic - it must > * only be called with interrupts disabled and no other active outer > * cache masters. > * > * It is intended that this function is only used by implementations > * needing to override the outer_cache.disable() method due to security. > * (Some implementations perform this as a clean followed by an invalidate.) > */ > > So, outer_flush_all() should not be called except from L2 cache code > implementing the outer_disable() function - it's not intended for platforms > to use. > > There are, however, sadly three users of outer_flush_all() which have crept in > through arm-soc, that should be outer_disable() instead.
Here, outer_flush_all() should not be called, calling outer_disable() is enough. Is it right? In the implementation of l2c_disable(void) of in mm/cache-l2x0.c, the outer_cache.flush_all() is called. > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. Best Regards, Wenyou Yang