| Message ID | 20180430083431.72606-1-manu@freebsd.org |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | None | expand |
On Mon, 30 Apr 2018 10:37:50 +0200 "Dr. Philipp Tomsich" <philipp.tomsich@theobroma-systems.com> wrote: > > > On 30 Apr 2018, at 10:34, Emmanuel Vadot <manu@freebsd.org> wrote: > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org> > > --- > > api/api.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/api/api.c b/api/api.c > > index 7eee2fc083..7d1608b520 100644 > > --- a/api/api.c > > +++ b/api/api.c > > @@ -290,6 +290,17 @@ static int API_dev_close(va_list ap) > > if (!err) > > di->state = DEV_STA_CLOSED; > > > > +#if defined(CONFIG_SYS_HAVE_DCACHE_MAINTENANCE) && \ > > + !defined(CONFIG_SYS_DCACHE_OFF) > > + if (dcache_status()) > > + flush_dcache_all(); > > +#endif > > +#if defined(CONFIG_SYS_HAVE_ICACHE_MAINTENANCE) && \ > > + !defined(CONFIG_SYS_ICACHE_OFF) > > + if (icache_status()) > > + invalidate_icache_all(); > > +#endif > > Wouldn?t it be a cleaner option to make flush_dcache_all and invalidate_icache_all > weak-functions and provide a default implementation that does nothing. Those > architectures that then need to implement specific cache maintenance, could > override these as required. Tom had some concern about using weak function for this, see https://lists.denx.de/pipermail/u-boot/2017-February/280652.html But if everybody is happy with me adding some global weak function for caches I'll do that. > > return err; > > } > > > > -- > > 2.16.3 > >
Hi, On 4 May 2018 at 01:12, Emmanuel Vadot <manu@bidouilliste.com> wrote: > On Mon, 30 Apr 2018 10:37:50 +0200 > "Dr. Philipp Tomsich" <philipp.tomsich@theobroma-systems.com> wrote: > >> >> > On 30 Apr 2018, at 10:34, Emmanuel Vadot <manu@freebsd.org> wrote: >> > >> > Signed-off-by: Emmanuel Vadot <manu@freebsd.org> >> > --- >> > api/api.c | 11 +++++++++++ >> > 1 file changed, 11 insertions(+) >> > >> > diff --git a/api/api.c b/api/api.c >> > index 7eee2fc083..7d1608b520 100644 >> > --- a/api/api.c >> > +++ b/api/api.c >> > @@ -290,6 +290,17 @@ static int API_dev_close(va_list ap) >> > if (!err) >> > di->state = DEV_STA_CLOSED; >> > >> > +#if defined(CONFIG_SYS_HAVE_DCACHE_MAINTENANCE) && \ >> > + !defined(CONFIG_SYS_DCACHE_OFF) >> > + if (dcache_status()) >> > + flush_dcache_all(); >> > +#endif >> > +#if defined(CONFIG_SYS_HAVE_ICACHE_MAINTENANCE) && \ >> > + !defined(CONFIG_SYS_ICACHE_OFF) >> > + if (icache_status()) >> > + invalidate_icache_all(); >> > +#endif >> >> Wouldn?t it be a cleaner option to make flush_dcache_all and invalidate_icache_all >> weak-functions and provide a default implementation that does nothing. Those >> architectures that then need to implement specific cache maintenance, could >> override these as required. > > Tom had some concern about using weak function for this, see > https://lists.denx.de/pipermail/u-boot/2017-February/280652.html > > But if everybody is happy with me adding some global weak function for > caches I'll do that. > I'm not very happy about it. Weak functions are just so hard to figure out. Is it called or not? The lack of link-time error is painful when debugging. Regards, Simon
On Fri, May 04, 2018 at 09:12:49AM +0200, Emmanuel Vadot wrote: > On Mon, 30 Apr 2018 10:37:50 +0200 > "Dr. Philipp Tomsich" <philipp.tomsich@theobroma-systems.com> wrote: > > > > > > On 30 Apr 2018, at 10:34, Emmanuel Vadot <manu@freebsd.org> wrote: > > > > > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org> > > > --- > > > api/api.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/api/api.c b/api/api.c > > > index 7eee2fc083..7d1608b520 100644 > > > --- a/api/api.c > > > +++ b/api/api.c > > > @@ -290,6 +290,17 @@ static int API_dev_close(va_list ap) > > > if (!err) > > > di->state = DEV_STA_CLOSED; > > > > > > +#if defined(CONFIG_SYS_HAVE_DCACHE_MAINTENANCE) && \ > > > + !defined(CONFIG_SYS_DCACHE_OFF) > > > + if (dcache_status()) > > > + flush_dcache_all(); > > > +#endif > > > +#if defined(CONFIG_SYS_HAVE_ICACHE_MAINTENANCE) && \ > > > + !defined(CONFIG_SYS_ICACHE_OFF) > > > + if (icache_status()) > > > + invalidate_icache_all(); > > > +#endif > > > > Wouldn?t it be a cleaner option to make flush_dcache_all and invalidate_icache_all > > weak-functions and provide a default implementation that does nothing. Those > > architectures that then need to implement specific cache maintenance, could > > override these as required. > > Tom had some concern about using weak function for this, see > https://lists.denx.de/pipermail/u-boot/2017-February/280652.html In this specific case before we could use weak functions we would need to do some wide-ranging cleanups for consistently named and functional cache functions.
diff --git a/api/api.c b/api/api.c index 7eee2fc083..7d1608b520 100644 --- a/api/api.c +++ b/api/api.c @@ -290,6 +290,17 @@ static int API_dev_close(va_list ap) if (!err) di->state = DEV_STA_CLOSED; +#if defined(CONFIG_SYS_HAVE_DCACHE_MAINTENANCE) && \ + !defined(CONFIG_SYS_DCACHE_OFF) + if (dcache_status()) + flush_dcache_all(); +#endif +#if defined(CONFIG_SYS_HAVE_ICACHE_MAINTENANCE) && \ + !defined(CONFIG_SYS_ICACHE_OFF) + if (icache_status()) + invalidate_icache_all(); +#endif + return err; }
Signed-off-by: Emmanuel Vadot <manu@freebsd.org> --- api/api.c | 11 +++++++++++ 1 file changed, 11 insertions(+)