[U-Boot] [PATCH 3/5] api: Flush cache when closing api
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 + return err; }
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.
return err; }
-- 2.16.3
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.
participants (5)
-
Dr. Philipp Tomsich -
Emmanuel Vadot -
Emmanuel Vadot -
Simon Glass -
Tom Rini