All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 3/5] api: Flush cache when closing api
@ 2018-04-30  8:34 Emmanuel Vadot
  2018-04-30  8:37 ` Dr. Philipp Tomsich
  0 siblings, 1 reply; 5+ messages in thread
From: Emmanuel Vadot @ 2018-04-30  8:34 UTC (permalink / raw)
  To: u-boot

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;
 }
 
-- 
2.16.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH 3/5] api: Flush cache when closing api
  2018-04-30  8:34 [U-Boot] [PATCH 3/5] api: Flush cache when closing api Emmanuel Vadot
@ 2018-04-30  8:37 ` Dr. Philipp Tomsich
  2018-05-04  7:12   ` Emmanuel Vadot
  0 siblings, 1 reply; 5+ messages in thread
From: Dr. Philipp Tomsich @ 2018-04-30  8:37 UTC (permalink / raw)
  To: u-boot


> 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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH 3/5] api: Flush cache when closing api
  2018-04-30  8:37 ` Dr. Philipp Tomsich
@ 2018-05-04  7:12   ` Emmanuel Vadot
  2018-05-04 21:37     ` Simon Glass
  2018-05-04 22:10     ` Tom Rini
  0 siblings, 2 replies; 5+ messages in thread
From: Emmanuel Vadot @ 2018-05-04  7:12 UTC (permalink / raw)
  To: u-boot

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
> > 


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH 3/5] api: Flush cache when closing api
  2018-05-04  7:12   ` Emmanuel Vadot
@ 2018-05-04 21:37     ` Simon Glass
  2018-05-04 22:10     ` Tom Rini
  1 sibling, 0 replies; 5+ messages in thread
From: Simon Glass @ 2018-05-04 21:37 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH 3/5] api: Flush cache when closing api
  2018-05-04  7:12   ` Emmanuel Vadot
  2018-05-04 21:37     ` Simon Glass
@ 2018-05-04 22:10     ` Tom Rini
  1 sibling, 0 replies; 5+ messages in thread
From: Tom Rini @ 2018-05-04 22:10 UTC (permalink / raw)
  To: u-boot

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180504/681acce0/attachment.sig>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-04 22:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-30  8:34 [U-Boot] [PATCH 3/5] api: Flush cache when closing api Emmanuel Vadot
2018-04-30  8:37 ` Dr. Philipp Tomsich
2018-05-04  7:12   ` Emmanuel Vadot
2018-05-04 21:37     ` Simon Glass
2018-05-04 22:10     ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.