dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
@ 2020-02-03 10:18 Peter Ujfalusi
  2020-02-03 10:18 ` [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason() Peter Ujfalusi
                   ` (3 more replies)
  0 siblings, 4 replies; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-03 10:18 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams

Hi,

dma_request_slave_channel_reason() no longer have user in mainline, it
can be removed.

Advise users of dma_request_slave_channel() and
dma_request_slave_channel_compat() to move to dma_request_slave_chan()

Regards,
Peter
---
Peter Ujfalusi (3):
  dmaengine: Remove unused define for (dma_request_slave_channel_reason)()
  dmaengine: Mark dma_request_slave_channel() deprecated
  dmaengine: Encourage dma_request_slave_channel_compat() users to
    migrate

 drivers/dma/dmaengine.c   | 18 ------------------
 include/linux/dmaengine.h | 28 ++++++++++++++++++----------
 2 files changed, 18 insertions(+), 28 deletions(-)

-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason()
  2020-02-03 10:18 [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Peter Ujfalusi
@ 2020-02-03 10:18 ` Peter Ujfalusi
  2020-02-03 13:35   ` Geert Uytterhoeven
  2020-02-03 10:18 ` [PATCH 2/3] dmaengine: Mark dma_request_slave_channel() deprecated Peter Ujfalusi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-03 10:18 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams

No users left in the kernel, it can be removed.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/linux/dmaengine.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9f232b7618f1..6702df107d79 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1495,8 +1495,6 @@ static inline int dma_get_slave_caps(struct dma_chan *chan,
 }
 #endif
 
-#define dma_request_slave_channel_reason(dev, name) dma_request_chan(dev, name)
-
 static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)
 {
 	struct dma_slave_caps caps;
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* [PATCH 2/3] dmaengine: Mark dma_request_slave_channel() deprecated
  2020-02-03 10:18 [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Peter Ujfalusi
  2020-02-03 10:18 ` [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason() Peter Ujfalusi
@ 2020-02-03 10:18 ` Peter Ujfalusi
  2020-02-03 10:18 ` [PATCH 3/3] dmaengine: Encourage dma_request_slave_channel_compat() users to migrate Peter Ujfalusi
  2020-02-03 10:37 ` [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Andy Shevchenko
  3 siblings, 0 replies; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-03 10:18 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams

New drivers should use dma_request_chan() instead
dma_request_slave_channel()

dma_request_slave_channel() is a simple wrapper for dma_request_chan()
eating up the error code for channel request failure and makes deferred
probing impossible.

Move the dma_request_slave_channel() into the header as inline function,
mark it as deprecated.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/dmaengine.c   | 18 ------------------
 include/linux/dmaengine.h | 15 +++++++++------
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 121231300d35..8ec2257a8d88 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -781,24 +781,6 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name)
 }
 EXPORT_SYMBOL_GPL(dma_request_chan);
 
-/**
- * dma_request_slave_channel - try to allocate an exclusive slave channel
- * @dev:	pointer to client device structure
- * @name:	slave channel name
- *
- * Returns pointer to appropriate DMA channel on success or NULL.
- */
-struct dma_chan *dma_request_slave_channel(struct device *dev,
-					   const char *name)
-{
-	struct dma_chan *ch = dma_request_chan(dev, name);
-	if (IS_ERR(ch))
-		return NULL;
-
-	return ch;
-}
-EXPORT_SYMBOL_GPL(dma_request_slave_channel);
-
 /**
  * dma_request_chan_by_mask - allocate a channel satisfying certain capabilities
  * @mask: capabilities that the channel must satisfy
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 6702df107d79..4c522bf6ac25 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1440,7 +1440,6 @@ void dma_issue_pending_all(void);
 struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
 				       dma_filter_fn fn, void *fn_param,
 				       struct device_node *np);
-struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
 
 struct dma_chan *dma_request_chan(struct device *dev, const char *name);
 struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
@@ -1470,11 +1469,6 @@ static inline struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
 {
 	return NULL;
 }
-static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
-							 const char *name)
-{
-	return NULL;
-}
 static inline struct dma_chan *dma_request_chan(struct device *dev,
 						const char *name)
 {
@@ -1544,6 +1538,15 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
 #define dma_request_channel(mask, x, y) \
 	__dma_request_channel(&(mask), x, y, NULL)
 
+/* Deprecated, please use dma_request_chan() directly */
+static inline struct dma_chan * __deprecated
+dma_request_slave_channel(struct device *dev, const char *name)
+{
+	struct dma_chan *ch = dma_request_chan(dev, name);
+
+	return IS_ERR(ch) ? NULL : ch;
+}
+
 static inline struct dma_chan
 *dma_request_slave_channel_compat(const dma_cap_mask_t mask,
 				  dma_filter_fn fn, void *fn_param,
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* [PATCH 3/3] dmaengine: Encourage dma_request_slave_channel_compat() users to migrate
  2020-02-03 10:18 [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Peter Ujfalusi
  2020-02-03 10:18 ` [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason() Peter Ujfalusi
  2020-02-03 10:18 ` [PATCH 2/3] dmaengine: Mark dma_request_slave_channel() deprecated Peter Ujfalusi
@ 2020-02-03 10:18 ` Peter Ujfalusi
  2020-02-03 10:35   ` Andy Shevchenko
  2020-02-03 10:37 ` [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Andy Shevchenko
  3 siblings, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-03 10:18 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams

Users of dma_request_slave_channel_compat() can be migrated to
dma_request_chan() by correct dma_slave_map for the platform.

Start nagging users in hope that they will move.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/linux/dmaengine.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 4c522bf6ac25..581f6822a7a5 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1547,6 +1547,10 @@ dma_request_slave_channel(struct device *dev, const char *name)
 	return IS_ERR(ch) ? NULL : ch;
 }
 
+/*
+ * Please use dma_request_chan() directly.
+ * Legacy support should use dma_slave_map + dma_request_chan()
+ */
 static inline struct dma_chan
 *dma_request_slave_channel_compat(const dma_cap_mask_t mask,
 				  dma_filter_fn fn, void *fn_param,
@@ -1554,13 +1558,16 @@ static inline struct dma_chan
 {
 	struct dma_chan *chan;
 
-	chan = dma_request_slave_channel(dev, name);
-	if (chan)
+	chan = dma_request_chan(dev, name);
+	if (!IS_ERR(chan))
 		return chan;
 
 	if (!fn || !fn_param)
 		return NULL;
 
+	dev_info(dev, "Please add dma_slave_map entry for %s:%s and migrate to"
+		 " dma_request_chan()", dev_name(dev), name);
+
 	return __dma_request_channel(&mask, fn, fn_param, NULL);
 }
 
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* Re: [PATCH 3/3] dmaengine: Encourage dma_request_slave_channel_compat() users to migrate
  2020-02-03 10:18 ` [PATCH 3/3] dmaengine: Encourage dma_request_slave_channel_compat() users to migrate Peter Ujfalusi
@ 2020-02-03 10:35   ` Andy Shevchenko
  0 siblings, 0 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-02-03 10:35 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Vinod Koul, dmaengine, Linux Kernel Mailing List, Dan Williams

On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:


> +       dev_info(dev, "Please add dma_slave_map entry for %s:%s and migrate to"
> +                " dma_request_chan()", dev_name(dev), name);

Please, don't split string literals.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 10:18 [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Peter Ujfalusi
                   ` (2 preceding siblings ...)
  2020-02-03 10:18 ` [PATCH 3/3] dmaengine: Encourage dma_request_slave_channel_compat() users to migrate Peter Ujfalusi
@ 2020-02-03 10:37 ` Andy Shevchenko
  2020-02-03 10:59   ` Peter Ujfalusi
  2020-02-04  6:21   ` Vinod Koul
  3 siblings, 2 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-02-03 10:37 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Vinod Koul, dmaengine, Linux Kernel Mailing List, Dan Williams

On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

> dma_request_slave_channel_reason() no longer have user in mainline, it
> can be removed.
>
> Advise users of dma_request_slave_channel() and
> dma_request_slave_channel_compat() to move to dma_request_slave_chan()

How? There are legacy ARM boards you have to care / remove before.
DMAengine subsystem makes a p*s off decisions without taking care of
(I'm talking now about dma release callback, for example) end users.
They will be scary for no reason.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 10:37 ` [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Andy Shevchenko
@ 2020-02-03 10:59   ` Peter Ujfalusi
  2020-02-03 11:16     ` Andy Shevchenko
  2020-02-04  6:21   ` Vinod Koul
  1 sibling, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-03 10:59 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Vinod Koul, dmaengine, Linux Kernel Mailing List, Dan Williams

Hi Andy,

On 03/02/2020 12.37, Andy Shevchenko wrote:
> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
>> dma_request_slave_channel_reason() no longer have user in mainline, it
>> can be removed.
>>
>> Advise users of dma_request_slave_channel() and
>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> 
> How? There are legacy ARM boards you have to care / remove before.
> DMAengine subsystem makes a p*s off decisions

The dma_slave_map support is added few years back for the legacy ARM
boards, because we do care.
daVinci, OMAP1, pxa, s3cx4xx and even m68k/coldfire moved over.

Imho it is confusing to have 4+ APIs to do the same thing, but in a
slightly different way.

> without taking care of
> (I'm talking now about dma release callback, for example) end users.

I have been converting users in the background, but the _compat() is a
bit more problematic as I need to maintainers of those legacy platforms
to craft the map. If they care.

Obviously the APIs are not going to be removed if we have a single user
and if there is clearly a need for something the _compat() was doing and
it can not be done via the dma_slave_map, then rest assured there will
be a clean API to achieve just that.

> They will be scary for no reason.

There is a reason: to clean up the API to make it non confusing for the
users.
New drivers should not use the old API i new code and developers tend to
pick the API they use after a quick 'git grep dma_request_' and see what
the majority is using.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 10:59   ` Peter Ujfalusi
@ 2020-02-03 11:16     ` Andy Shevchenko
  2020-02-03 12:09       ` Peter Ujfalusi
  0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2020-02-03 11:16 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Vinod Koul, dmaengine, Linux Kernel Mailing List, Dan Williams

On Mon, Feb 3, 2020 at 12:59 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 03/02/2020 12.37, Andy Shevchenko wrote:
> > On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

> >> Advise users of dma_request_slave_channel() and
> >> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> >
> > How? There are legacy ARM boards you have to care / remove before.
> > DMAengine subsystem makes a p*s off decisions
>
> The dma_slave_map support is added few years back for the legacy ARM
> boards, because we do care.
> daVinci, OMAP1, pxa, s3cx4xx and even m68k/coldfire moved over.

Then why simple not to convert (start converting) those few drivers to
new API and simple remove the old one?

> Imho it is confusing to have 4+ APIs to do the same thing, but in a
> slightly different way.

It was always an excuse by authors "that too many drivers to convert..."

> > without taking care of
> > (I'm talking now about dma release callback, for example) end users.
>
> I have been converting users in the background, but the _compat() is a
> bit more problematic as I need to maintainers of those legacy platforms
> to craft the map. If they care.

Why not to remove them and don't punish users of new drivers / platforms?

> Obviously the APIs are not going to be removed if we have a single user
> and if there is clearly a need for something the _compat() was doing and
> it can not be done via the dma_slave_map, then rest assured there will
> be a clean API to achieve just that.
>
> > They will be scary for no reason.
>
> There is a reason: to clean up the API to make it non confusing for the
> users.

No, it's a reason when you first take care of existing users and
decide to obsolete an API followed by removal few releases later. But
I see no reason to keep such APIs at all, so, instead of this
*wonderful* messages perhaps somebody should do better work?

> New drivers should not use the old API i new code and developers tend to
> pick the API they use after a quick 'git grep dma_request_' and see what
> the majority is using.

Isn't it a point to do better review rather than scary end users?

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 11:16     ` Andy Shevchenko
@ 2020-02-03 12:09       ` Peter Ujfalusi
  2020-02-03 12:48         ` Andy Shevchenko
  2020-02-03 13:32         ` Geert Uytterhoeven
  0 siblings, 2 replies; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-03 12:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Vinod Koul, dmaengine, Linux Kernel Mailing List, Dan Williams

Hi Andy,

On 03/02/2020 13.16, Andy Shevchenko wrote:
> On Mon, Feb 3, 2020 at 12:59 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>> On 03/02/2020 12.37, Andy Shevchenko wrote:
>>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
>>>> Advise users of dma_request_slave_channel() and
>>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
>>>
>>> How? There are legacy ARM boards you have to care / remove before.
>>> DMAengine subsystem makes a p*s off decisions
>>
>> The dma_slave_map support is added few years back for the legacy ARM
>> boards, because we do care.
>> daVinci, OMAP1, pxa, s3cx4xx and even m68k/coldfire moved over.
> 
> Then why simple not to convert (start converting) those few drivers to
> new API and simple remove the old one?

_if_ the dma_request_slave_channel_compat() is really falling back after
dma_request_chan() - this is what it calls first, then it is not a
simple convert to a new API.
1. The arch needs to define the dma_slave_map for the SoC.
2. The dma driver needs few lines to add it to DMAengine core (+pdata
change).
After this the _compat() can be replaced with dma_request_chan()

In most cases I do not have access to documentation and boards to test.

Looking at the output of 'git grep dma_request_slave_channel_compat':
drivers/mmc/host/renesas_sdhi_sys_dmac.c
drivers/spi/spi-pxa2xx-dma.c
drivers/spi/spi-rspi.c
drivers/spi/spi-sh-msiof.c
drivers/tty/serial/8250/8250_dma.c

From these rspi boots only in DT and I'm not sure about the others.
8250_dma is a tricky one as it is used as generic DMA layer for many arch.

>> Imho it is confusing to have 4+ APIs to do the same thing, but in a
>> slightly different way.
> 
> It was always an excuse by authors "that too many drivers to convert..."

Sure, but before you accuse anyone with neglect, can you check the git
log for 'dma_request_slave_channel' in the commit messages?

>>> without taking care of
>>> (I'm talking now about dma release callback, for example) end users.
>>
>> I have been converting users in the background, but the _compat() is a
>> bit more problematic as I need to maintainers of those legacy platforms
>> to craft the map. If they care.
> 
> Why not to remove them and don't punish users of new drivers / platforms?

The _compat() can not be removed, but we just don't know who really
needs the fallback after dma_request_chan().
With the print the hope is that users will come forward and we can help
migrate or address their specific needs.

>> Obviously the APIs are not going to be removed if we have a single user
>> and if there is clearly a need for something the _compat() was doing and
>> it can not be done via the dma_slave_map, then rest assured there will
>> be a clean API to achieve just that.
>>
>>> They will be scary for no reason.
>>
>> There is a reason: to clean up the API to make it non confusing for the
>> users.
> 
> No, it's a reason when you first take care of existing users and
> decide to obsolete an API followed by removal few releases later.

I'm fine to drop the pr_info() and the __deprecated mark for
dma_request_slave_channel.

> But
> I see no reason to keep such APIs at all, so, instead of this
> *wonderful* messages perhaps somebody should do better work?

To touch the _compat() variant one needs to have access to the
documentation of the SoC on which the code falls back. It is not a
matter of sloppy/poor/ignorant/etc work attitude.
I have kept clear on touching those few drivers using it [1] as I don't
have documentation.

>> New drivers should not use the old API i new code and developers tend to
>> pick the API they use after a quick 'git grep dma_request_' and see what
>> the majority is using.
> 
> Isn't it a point to do better review rather than scary end users?

Sure, but we rarely CCd on new client drivers for DMAengine API usage.

[1] fwiw, there are drivers using dma_request_channel() and by the look
their use is either _compat() or the dma_request_chan_by_mask() style
and some even have a twist here and there...

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 12:09       ` Peter Ujfalusi
@ 2020-02-03 12:48         ` Andy Shevchenko
  2020-02-03 13:32         ` Geert Uytterhoeven
  1 sibling, 0 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-02-03 12:48 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Vinod Koul, dmaengine, Linux Kernel Mailing List, Dan Williams

On Mon, Feb 3, 2020 at 2:09 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 03/02/2020 13.16, Andy Shevchenko wrote:
> > On Mon, Feb 3, 2020 at 12:59 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >> On 03/02/2020 12.37, Andy Shevchenko wrote:
> >>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >
> >>>> Advise users of dma_request_slave_channel() and
> >>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> >>>
> >>> How? There are legacy ARM boards you have to care / remove before.
> >>> DMAengine subsystem makes a p*s off decisions
> >>
> >> The dma_slave_map support is added few years back for the legacy ARM
> >> boards, because we do care.
> >> daVinci, OMAP1, pxa, s3cx4xx and even m68k/coldfire moved over.
> >
> > Then why simple not to convert (start converting) those few drivers to
> > new API and simple remove the old one?
>
> _if_ the dma_request_slave_channel_compat() is really falling back after
> dma_request_chan() - this is what it calls first, then it is not a
> simple convert to a new API.
> 1. The arch needs to define the dma_slave_map for the SoC.
> 2. The dma driver needs few lines to add it to DMAengine core (+pdata
> change).
> After this the _compat() can be replaced with dma_request_chan()
>

> In most cases I do not have access to documentation and boards to test.

So, why everyone with the boards outside of your scope has to suffer?

...

> >> Imho it is confusing to have 4+ APIs to do the same thing, but in a
> >> slightly different way.
> >
> > It was always an excuse by authors "that too many drivers to convert..."
>
> Sure, but before you accuse anyone with neglect, can you check the git
> log for 'dma_request_slave_channel' in the commit messages?

It's good people, and you, are taking care of it, but make user suffer
because somebody wants to call for *developers* is a bad idea. And it
happened not first time in the DMA engine subsystem.

...

> > No, it's a reason when you first take care of existing users and
> > decide to obsolete an API followed by removal few releases later.
>
> I'm fine to drop the pr_info()

Yes, please do.

> and the __deprecated mark for
> dma_request_slave_channel.

This OTOH is for *developers* and its scope won't scary people. So,
it's more or less fine.

> > But
> > I see no reason to keep such APIs at all, so, instead of this
> > *wonderful* messages perhaps somebody should do better work?
>
> To touch the _compat() variant one needs to have access to the
> documentation of the SoC on which the code falls back. It is not a
> matter of sloppy/poor/ignorant/etc work attitude.
> I have kept clear on touching those few drivers using it [1] as I don't
> have documentation.
>
> >> New drivers should not use the old API i new code and developers tend to
> >> pick the API they use after a quick 'git grep dma_request_' and see what
> >> the majority is using.
> >
> > Isn't it a point to do better review rather than scary end users?
>
> Sure, but we rarely CCd on new client drivers for DMAengine API usage.

Linux Next allows people to `git grep -n dma_slave_DO_NOT_CALL` and
tell developers. Isn't it the job good maintainer can do?

> [1] fwiw, there are drivers using dma_request_channel() and by the look
> their use is either _compat() or the dma_request_chan_by_mask() style
> and some even have a twist here and there...

Bottom line, if you need to tell *developers* about something, please
don't bother *end users*.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 12:09       ` Peter Ujfalusi
  2020-02-03 12:48         ` Andy Shevchenko
@ 2020-02-03 13:32         ` Geert Uytterhoeven
  2020-02-03 20:21           ` John Paul Adrian Glaubitz
  1 sibling, 1 reply; 30+ messages in thread
From: Geert Uytterhoeven @ 2020-02-03 13:32 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Andy Shevchenko, Vinod Koul, dmaengine,
	Linux Kernel Mailing List, Dan Williams, Linux-sh list,
	Linux-Renesas

Hi Peter,

On Mon, Feb 3, 2020 at 1:49 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 03/02/2020 13.16, Andy Shevchenko wrote:
> > On Mon, Feb 3, 2020 at 12:59 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >> On 03/02/2020 12.37, Andy Shevchenko wrote:
> >>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >>>> Advise users of dma_request_slave_channel() and
> >>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> >>>
> >>> How? There are legacy ARM boards you have to care / remove before.
> >>> DMAengine subsystem makes a p*s off decisions
> >>
> >> The dma_slave_map support is added few years back for the legacy ARM
> >> boards, because we do care.
> >> daVinci, OMAP1, pxa, s3cx4xx and even m68k/coldfire moved over.
> >
> > Then why simple not to convert (start converting) those few drivers to
> > new API and simple remove the old one?
>
> _if_ the dma_request_slave_channel_compat() is really falling back after
> dma_request_chan() - this is what it calls first, then it is not a
> simple convert to a new API.
> 1. The arch needs to define the dma_slave_map for the SoC.
> 2. The dma driver needs few lines to add it to DMAengine core (+pdata
> change).
> After this the _compat() can be replaced with dma_request_chan()
>
> In most cases I do not have access to documentation and boards to test.
>
> Looking at the output of 'git grep dma_request_slave_channel_compat':
> drivers/mmc/host/renesas_sdhi_sys_dmac.c
> drivers/spi/spi-pxa2xx-dma.c
> drivers/spi/spi-rspi.c
> drivers/spi/spi-sh-msiof.c
> drivers/tty/serial/8250/8250_dma.c
>
> From these rspi boots only in DT and I'm not sure about the others.

Both rspi and sh-msiof have users on legacy SH (i.e. without DT):

    arch/sh/kernel/cpu/sh4a/setup-sh7757.c: .name   = "rspi",
    arch/sh/boards/mach-ecovec24/setup.c:   .name           = "spi_sh_msiof",

The former seems to be used with drivers/dma/sh/shdmac.c:

    arch/sh/include/cpu-sh4/cpu/sh7757.h:   SHDMA_SLAVE_RSPI_TX,
    arch/sh/include/cpu-sh4/cpu/sh7757.h:   SHDMA_SLAVE_RSPI_RX,
    arch/sh/kernel/cpu/sh4a/setup-sh7757.c:         .slave_id       =
SHDMA_SLAVE_RSPI_TX,
    arch/sh/kernel/cpu/sh4a/setup-sh7757.c:         .slave_id       =
SHDMA_SLAVE_RSPI_RX,

but I have no idea if it still works (and no hardware).

The latter doesn't seem to be used with DMA on ecovec24/sh7724, so
probably we can just drop the _compat() from sh-sh-msiof.c.

BTW, we no longer support non-legacy DMA in drivers/tty/serial/sh-sci.c
(DMA was completely broken in that driver when I added DT suppport),
but it seems it was used at some point on various SH, cfr. e.g.
arch/sh/kernel/cpu/sh4a/setup-sh7724.c still setting up the channels.
Which might be a motivation for just dropping _compat() from spi-rspi.c,
too? ;-)

Anyone who cares for DMA on SuperH?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason()
  2020-02-03 10:18 ` [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason() Peter Ujfalusi
@ 2020-02-03 13:35   ` Geert Uytterhoeven
  0 siblings, 0 replies; 30+ messages in thread
From: Geert Uytterhoeven @ 2020-02-03 13:35 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: Vinod, dmaengine, Linux Kernel Mailing List, Dan Williams

On Mon, Feb 3, 2020 at 11:32 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> No users left in the kernel, it can be removed.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

Last user removed in commit f339f979bb333ed5 ("iio: buffer-dmaengine: Use
dma_request_chan() directly for channel request"), so

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 13:32         ` Geert Uytterhoeven
@ 2020-02-03 20:21           ` John Paul Adrian Glaubitz
  2020-02-03 20:34             ` Geert Uytterhoeven
  0 siblings, 1 reply; 30+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-02-03 20:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Peter Ujfalusi
  Cc: Andy Shevchenko, Vinod Koul, dmaengine,
	Linux Kernel Mailing List, Dan Williams, Linux-sh list,
	Linux-Renesas

On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):

FWIW, there is a patch set by Yoshinori Sato to add device tree support
for classical SuperH hardware. It was never merged, unfortunately :(.

> Anyone who cares for DMA on SuperH?

What is DMA used for on SuperH? Wouldn't dropping it cut support for
essential hardware features?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 20:21           ` John Paul Adrian Glaubitz
@ 2020-02-03 20:34             ` Geert Uytterhoeven
  2020-02-03 21:26               ` John Paul Adrian Glaubitz
  2020-02-04  6:52               ` Peter Ujfalusi
  0 siblings, 2 replies; 30+ messages in thread
From: Geert Uytterhoeven @ 2020-02-03 20:34 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Peter Ujfalusi, Andy Shevchenko, Vinod Koul, dmaengine,
	Linux Kernel Mailing List, Dan Williams, Linux-sh list,
	Linux-Renesas

Hi Adrian,

On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
> > Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>
> FWIW, there is a patch set by Yoshinori Sato to add device tree support
> for classical SuperH hardware. It was never merged, unfortunately :(.

True.

> > Anyone who cares for DMA on SuperH?
>
> What is DMA used for on SuperH? Wouldn't dropping it cut support for
> essential hardware features?

It may make a few things slower.

Does any of your SuperH boards use DMA?
Anything interesting in /proc or /sys w.r.t. DMA?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 20:34             ` Geert Uytterhoeven
@ 2020-02-03 21:26               ` John Paul Adrian Glaubitz
  2020-02-04  8:13                 ` Geert Uytterhoeven
  2020-02-04  6:52               ` Peter Ujfalusi
  1 sibling, 1 reply; 30+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-02-03 21:26 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Peter Ujfalusi, Andy Shevchenko, Vinod Koul, dmaengine,
	Linux Kernel Mailing List, Dan Williams, Linux-sh list,
	Linux-Renesas

Hi Geert!

On 2/3/20 9:34 PM, Geert Uytterhoeven wrote:
> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>>
>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
>> for classical SuperH hardware. It was never merged, unfortunately :(.
> 
> True.

Would it be possible to keep DMA support if device tree support was
added for SuperH? I think Rich eventually wanted to merge the patches,
there were just some minor issues with them.

It would be great if we could still get them merged. I would be happy
to help with the testing.

>>> Anyone who cares for DMA on SuperH?
>>
>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
>> essential hardware features?
> 
> It may make a few things slower.
> 
> Does any of your SuperH boards use DMA?
> Anything interesting in /proc or /sys w.r.t. DMA?

I have:

root@tirpitz:/sys> find . -iname "*dma*"
./bus/dma
./bus/dma/devices/dma0
./bus/dma/devices/dma1
./bus/dma/devices/dma2
./bus/dma/devices/dma3
./bus/dma/devices/dma4
./bus/dma/devices/dma5
./bus/dma/devices/dma6
./bus/dma/devices/dma7
./bus/dma/devices/dma8
./bus/dma/devices/dma9
./bus/dma/devices/dma10
./bus/dma/devices/dma11
./bus/platform/devices/sh_dmac
./bus/platform/devices/sh-dma-engine.0
./bus/platform/devices/sh-dma-engine.1
./devices/pci0000:00/0000:00:00.0/consistent_dma_mask_bits
./devices/pci0000:00/0000:00:00.0/dma_mask_bits
./devices/pci0000:00/0000:00:01.0/ata1/host0/scsi_host/host0/unchecked_isa_dma
./devices/pci0000:00/0000:00:01.0/ata1/link1/dev1.0/ata_device/dev1.0/dma_mode
./devices/pci0000:00/0000:00:01.0/ata2/host1/scsi_host/host1/unchecked_isa_dma
./devices/pci0000:00/0000:00:01.0/ata2/link2/dev2.0/ata_device/dev2.0/dma_mode
./devices/pci0000:00/0000:00:01.0/consistent_dma_mask_bits
./devices/pci0000:00/0000:00:01.0/dma_mask_bits
./devices/system/dma
./devices/system/dma/dma0
./devices/system/dma/dma1
./devices/system/dma/dma2
./devices/system/dma/dma3
./devices/system/dma/dma4
./devices/system/dma/dma5
./devices/system/dma/dma6
root@tirpitz:~> find /proc -iname "*dma*"
/proc/dma
/proc/irq/52/DMAC Address Error1
/proc/irq/33/DMAC Address Error0
/proc/sys/fs/nfs/idmap_cache_timeout
find: ‘/proc/5703’: No such file or directory
find: ‘/proc/5704’: No such file or directory
find: ‘/proc/5705’: No such file or directory
root@tirpitz:~> cat /proc/dma
root@tirpitz:~> find /sys -iname "*dma*"      
/sys/bus/dma
/sys/bus/dma/devices/dma0
/sys/bus/dma/devices/dma1
/sys/bus/dma/devices/dma2
/sys/bus/dma/devices/dma3
/sys/bus/dma/devices/dma4
/sys/bus/dma/devices/dma5
/sys/bus/dma/devices/dma6
/sys/bus/dma/devices/dma7
/sys/bus/dma/devices/dma8
/sys/bus/dma/devices/dma9
/sys/bus/dma/devices/dma10
/sys/bus/dma/devices/dma11
/sys/bus/platform/devices/sh_dmac
/sys/bus/platform/devices/sh-dma-engine.0
/sys/bus/platform/devices/sh-dma-engine.1
/sys/devices/pci0000:00/0000:00:00.0/consistent_dma_mask_bits
/sys/devices/pci0000:00/0000:00:00.0/dma_mask_bits
/sys/devices/pci0000:00/0000:00:01.0/ata1/host0/scsi_host/host0/unchecked_isa_dma
/sys/devices/pci0000:00/0000:00:01.0/ata1/link1/dev1.0/ata_device/dev1.0/dma_mode
/sys/devices/pci0000:00/0000:00:01.0/ata2/host1/scsi_host/host1/unchecked_isa_dma
/sys/devices/pci0000:00/0000:00:01.0/ata2/link2/dev2.0/ata_device/dev2.0/dma_mode
/sys/devices/pci0000:00/0000:00:01.0/consistent_dma_mask_bits
/sys/devices/pci0000:00/0000:00:01.0/dma_mask_bits
/sys/devices/system/dma
/sys/devices/system/dma/dma0
/sys/devices/system/dma/dma1
/sys/devices/system/dma/dma2
/sys/devices/system/dma/dma3
/sys/devices/system/dma/dma4
/sys/devices/system/dma/dma5
/sys/devices/system/dma/dma6
/sys/devices/system/dma/dma7
/sys/devices/system/dma/dma8
/sys/devices/system/dma/dma9
/sys/devices/system/dma/dma10
/sys/devices/system/dma/dma11
/sys/devices/virtual/misc/cpu_dma_latency
/sys/devices/platform/sh_dmac
/sys/devices/platform/sh_dmac/dma0
/sys/devices/platform/sh_dmac/dma1
/sys/devices/platform/sh_dmac/dma2
/sys/devices/platform/sh_dmac/dma3
/sys/devices/platform/sh_dmac/dma4
/sys/devices/platform/sh_dmac/dma5
/sys/devices/platform/sh_dmac/dma6
/sys/devices/platform/sh_dmac/dma7
/sys/devices/platform/sh_dmac/dma8
/sys/devices/platform/sh_dmac/dma9
/sys/devices/platform/sh_dmac/dma10
/sys/devices/platform/sh_dmac/dma11
/sys/devices/platform/r8a66597_hcd/usb1/1-1/1-1:1.0/host2/scsi_host/host2/unchecked_isa_dma
/sys/devices/platform/sh-dma-engine.0
/sys/devices/platform/sh-dma-engine.1
/sys/class/misc/cpu_dma_latency
/sys/module/nfs/parameters/nfs4_disable_idmapping
/sys/module/nfs/parameters/nfs_idmap_cache_timeout
/sys/module/libata/parameters/dma
/sys/module/libata/parameters/atapi_dmadir
root@tirpitz:~>

On my SH-7785LCR.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 10:37 ` [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Andy Shevchenko
  2020-02-03 10:59   ` Peter Ujfalusi
@ 2020-02-04  6:21   ` Vinod Koul
  2020-02-04 11:21     ` Andy Shevchenko
  1 sibling, 1 reply; 30+ messages in thread
From: Vinod Koul @ 2020-02-04  6:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Peter Ujfalusi, dmaengine, Linux Kernel Mailing List, Dan Williams

On 03-02-20, 12:37, Andy Shevchenko wrote:
> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
> > dma_request_slave_channel_reason() no longer have user in mainline, it
> > can be removed.
> >
> > Advise users of dma_request_slave_channel() and
> > dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> 
> How? There are legacy ARM boards you have to care / remove before.
> DMAengine subsystem makes a p*s off decisions without taking care of
> (I'm talking now about dma release callback, for example) end users.

Can you elaborate issue you are seeing with dma_release callback?

-- 
~Vinod

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 20:34             ` Geert Uytterhoeven
  2020-02-03 21:26               ` John Paul Adrian Glaubitz
@ 2020-02-04  6:52               ` Peter Ujfalusi
  2020-02-04  8:01                 ` Geert Uytterhoeven
  1 sibling, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-04  6:52 UTC (permalink / raw)
  To: Geert Uytterhoeven, John Paul Adrian Glaubitz
  Cc: Andy Shevchenko, Vinod Koul, dmaengine,
	Linux Kernel Mailing List, Dan Williams, Linux-sh list,
	Linux-Renesas

Hi Geert, Adrian,

On 03/02/2020 22.34, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>>
>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
>> for classical SuperH hardware. It was never merged, unfortunately :(.
> 
> True.
> 
>>> Anyone who cares for DMA on SuperH?
>>
>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
>> essential hardware features?
> 
> It may make a few things slower.

I would not drop DMA support but I would suggest to add dma_slave_map
for non DT boot so the _compat() can be dropped.

Imho on lower spec SoC (and I believe SuperH is) the DMA makes big
difference offloading data movement from the CPU.

> Does any of your SuperH boards use DMA?
> Anything interesting in /proc or /sys w.r.t. DMA?
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  6:52               ` Peter Ujfalusi
@ 2020-02-04  8:01                 ` Geert Uytterhoeven
  2020-02-04  8:15                   ` Peter Ujfalusi
  2020-02-04  9:16                   ` Rob Landley
  0 siblings, 2 replies; 30+ messages in thread
From: Geert Uytterhoeven @ 2020-02-04  8:01 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: John Paul Adrian Glaubitz, Andy Shevchenko, Vinod Koul,
	dmaengine, Linux Kernel Mailing List, Dan Williams,
	Linux-sh list, Linux-Renesas

Hi Peter,

On Tue, Feb 4, 2020 at 7:52 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 03/02/2020 22.34, Geert Uytterhoeven wrote:
> > On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> >> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
> >>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
> >>
> >> FWIW, there is a patch set by Yoshinori Sato to add device tree support
> >> for classical SuperH hardware. It was never merged, unfortunately :(.
> >
> > True.
> >
> >>> Anyone who cares for DMA on SuperH?
> >>
> >> What is DMA used for on SuperH? Wouldn't dropping it cut support for
> >> essential hardware features?
> >
> > It may make a few things slower.
>
> I would not drop DMA support but I would suggest to add dma_slave_map
> for non DT boot so the _compat() can be dropped.

Which is similar in spirit to gpiod_lookup and clk_register_clkdev(),
right?

> Imho on lower spec SoC (and I believe SuperH is) the DMA makes big
> difference offloading data movement from the CPU.

Assumed it is actually used...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-03 21:26               ` John Paul Adrian Glaubitz
@ 2020-02-04  8:13                 ` Geert Uytterhoeven
  0 siblings, 0 replies; 30+ messages in thread
From: Geert Uytterhoeven @ 2020-02-04  8:13 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Peter Ujfalusi, Andy Shevchenko, Vinod Koul, dmaengine,
	Linux Kernel Mailing List, Dan Williams, Linux-sh list,
	Linux-Renesas

Hi Adrian,

On Mon, Feb 3, 2020 at 10:26 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On 2/3/20 9:34 PM, Geert Uytterhoeven wrote:
> > On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> >> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
> >>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
> >>
> >> FWIW, there is a patch set by Yoshinori Sato to add device tree support
> >> for classical SuperH hardware. It was never merged, unfortunately :(.
> >
> > True.
>
> Would it be possible to keep DMA support if device tree support was
> added for SuperH? I think Rich eventually wanted to merge the patches,
> there were just some minor issues with them.

Adding DT support would definitely make things easier, but would be a lot
of work.
However, using dma_slave_map would be an alternative.

> >>> Anyone who cares for DMA on SuperH?
> >>
> >> What is DMA used for on SuperH? Wouldn't dropping it cut support for
> >> essential hardware features?
> >
> > It may make a few things slower.
> >
> > Does any of your SuperH boards use DMA?
> > Anything interesting in /proc or /sys w.r.t. DMA?
>
> I have:
>
> root@tirpitz:/sys> find . -iname "*dma*"
> ./bus/dma
> ./bus/dma/devices/dma0
> ./bus/dma/devices/dma1
> ./bus/dma/devices/dma2
> ./bus/dma/devices/dma3
> ./bus/dma/devices/dma4
> ./bus/dma/devices/dma5
> ./bus/dma/devices/dma6
> ./bus/dma/devices/dma7
> ./bus/dma/devices/dma8
> ./bus/dma/devices/dma9
> ./bus/dma/devices/dma10
> ./bus/dma/devices/dma11
> ./bus/platform/devices/sh_dmac
> ./bus/platform/devices/sh-dma-engine.0
> ./bus/platform/devices/sh-dma-engine.1

So you do have the two dmaengines...

> On my SH-7785LCR.

... but are they actually used?

git grep -E "(SHDMA|sh_dmae_slave_config)" -- "arch/sh/*7785*"
doesn't come up with any matches, so I don't think any sh7785 platform
is wired to use DMA (yet), only sh7757 and sh772[234].

To double-check:
With current upstream, you can look for "slave" symlinks in sysfs.
With older kernels, you can look at the interrupt counts for the DMACs
in /proc/interrupts.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  8:01                 ` Geert Uytterhoeven
@ 2020-02-04  8:15                   ` Peter Ujfalusi
  2020-02-04  8:21                     ` Peter Ujfalusi
  2020-02-04  9:16                   ` Rob Landley
  1 sibling, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-04  8:15 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: John Paul Adrian Glaubitz, Andy Shevchenko, Vinod Koul,
	dmaengine, Linux Kernel Mailing List, Dan Williams,
	Linux-sh list, Linux-Renesas

Hi Geert,

On 04/02/2020 10.01, Geert Uytterhoeven wrote:
> Hi Peter,
> 
> On Tue, Feb 4, 2020 at 7:52 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>> On 03/02/2020 22.34, Geert Uytterhoeven wrote:
>>> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
>>> <glaubitz@physik.fu-berlin.de> wrote:
>>>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
>>>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>>>>
>>>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
>>>> for classical SuperH hardware. It was never merged, unfortunately :(.
>>>
>>> True.
>>>
>>>>> Anyone who cares for DMA on SuperH?
>>>>
>>>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
>>>> essential hardware features?
>>>
>>> It may make a few things slower.
>>
>> I would not drop DMA support but I would suggest to add dma_slave_map
>> for non DT boot so the _compat() can be dropped.
> 
> Which is similar in spirit to gpiod_lookup and clk_register_clkdev(),
> right?

Yes, it is similar:

/* OMAP730, OMAP850 */
static const struct dma_slave_map omap7xx_sdma_map[] = {
	{ "omap-mcbsp.1", "tx", SDMA_FILTER_PARAM(8) },
	{ "omap-mcbsp.1", "rx", SDMA_FILTER_PARAM(9) },
	{ "omap-mcbsp.2", "tx", SDMA_FILTER_PARAM(10) },
	{ "omap-mcbsp.2", "rx", SDMA_FILTER_PARAM(11) },
	{ "mmci-omap.0", "tx", SDMA_FILTER_PARAM(21) },
	{ "mmci-omap.0", "rx", SDMA_FILTER_PARAM(22) },
	{ "omap_udc", "rx0", SDMA_FILTER_PARAM(26) },
	{ "omap_udc", "rx1", SDMA_FILTER_PARAM(27) },
	{ "omap_udc", "rx2", SDMA_FILTER_PARAM(28) },
	{ "omap_udc", "tx0", SDMA_FILTER_PARAM(29) },
	{ "omap_udc", "tx1", SDMA_FILTER_PARAM(30) },
	{ "omap_udc", "tx2", SDMA_FILTER_PARAM(31) },
};

"device name", "channel name", "parameter for filter"

The in the DMA driver (omap-dma.c):
	od->ddev.filter.map = od->plat->slave_map;
	od->ddev.filter.mapcnt = od->plat->slavecnt;
	od->ddev.filter.fn = omap_dma_filter_fn;

When things are converted the filter function no longer needs to be
exported, it is local to the DMA driver.

> 
>> Imho on lower spec SoC (and I believe SuperH is) the DMA makes big
>> difference offloading data movement from the CPU.
> 
> Assumed it is actually used...

Right, imho (again) we should not decide if given SoC needs it or not.
It is up to the drivers to use it or not, but with the dma_slave_map
there is no difference between DT or legacy boot handling towards DMA.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  8:15                   ` Peter Ujfalusi
@ 2020-02-04  8:21                     ` Peter Ujfalusi
  0 siblings, 0 replies; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-04  8:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: John Paul Adrian Glaubitz, Andy Shevchenko, Vinod Koul,
	dmaengine, Linux Kernel Mailing List, Dan Williams,
	Linux-sh list, Linux-Renesas



On 04/02/2020 10.15, Peter Ujfalusi wrote:
> Hi Geert,
> 
> On 04/02/2020 10.01, Geert Uytterhoeven wrote:
>> Hi Peter,
>>
>> On Tue, Feb 4, 2020 at 7:52 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>>> On 03/02/2020 22.34, Geert Uytterhoeven wrote:
>>>> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
>>>> <glaubitz@physik.fu-berlin.de> wrote:
>>>>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
>>>>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>>>>>
>>>>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
>>>>> for classical SuperH hardware. It was never merged, unfortunately :(.
>>>>
>>>> True.
>>>>
>>>>>> Anyone who cares for DMA on SuperH?
>>>>>
>>>>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
>>>>> essential hardware features?
>>>>
>>>> It may make a few things slower.
>>>
>>> I would not drop DMA support but I would suggest to add dma_slave_map
>>> for non DT boot so the _compat() can be dropped.
>>
>> Which is similar in spirit to gpiod_lookup and clk_register_clkdev(),
>> right?
> 
> Yes, it is similar:
> 
> /* OMAP730, OMAP850 */
> static const struct dma_slave_map omap7xx_sdma_map[] = {
> 	{ "omap-mcbsp.1", "tx", SDMA_FILTER_PARAM(8) },
> 	{ "omap-mcbsp.1", "rx", SDMA_FILTER_PARAM(9) },
> 	{ "omap-mcbsp.2", "tx", SDMA_FILTER_PARAM(10) },
> 	{ "omap-mcbsp.2", "rx", SDMA_FILTER_PARAM(11) },
> 	{ "mmci-omap.0", "tx", SDMA_FILTER_PARAM(21) },
> 	{ "mmci-omap.0", "rx", SDMA_FILTER_PARAM(22) },
> 	{ "omap_udc", "rx0", SDMA_FILTER_PARAM(26) },
> 	{ "omap_udc", "rx1", SDMA_FILTER_PARAM(27) },
> 	{ "omap_udc", "rx2", SDMA_FILTER_PARAM(28) },
> 	{ "omap_udc", "tx0", SDMA_FILTER_PARAM(29) },
> 	{ "omap_udc", "tx1", SDMA_FILTER_PARAM(30) },
> 	{ "omap_udc", "tx2", SDMA_FILTER_PARAM(31) },
> };
> 
> "device name", "channel name", "parameter for filter"

fwiw for EDMA on daVinci we have these (for da850):
static const struct dma_slave_map da850_edma0_map[] = {
	{ "davinci-mcasp.0", "rx", EDMA_FILTER_PARAM(0, 0) },
	{ "davinci-mcasp.0", "tx", EDMA_FILTER_PARAM(0, 1) },
	{ "davinci-mcbsp.0", "rx", EDMA_FILTER_PARAM(0, 2) },
	{ "davinci-mcbsp.0", "tx", EDMA_FILTER_PARAM(0, 3) },
	{ "davinci-mcbsp.1", "rx", EDMA_FILTER_PARAM(0, 4) },
	{ "davinci-mcbsp.1", "tx", EDMA_FILTER_PARAM(0, 5) },
	{ "spi_davinci.0", "rx", EDMA_FILTER_PARAM(0, 14) },
	{ "spi_davinci.0", "tx", EDMA_FILTER_PARAM(0, 15) },
	{ "da830-mmc.0", "rx", EDMA_FILTER_PARAM(0, 16) },
	{ "da830-mmc.0", "tx", EDMA_FILTER_PARAM(0, 17) },
	{ "spi_davinci.1", "rx", EDMA_FILTER_PARAM(0, 18) },
	{ "spi_davinci.1", "tx", EDMA_FILTER_PARAM(0, 19) },
};

static const struct dma_slave_map da850_edma1_map[] = {
	{ "da830-mmc.1", "rx", EDMA_FILTER_PARAM(1, 28) },
	{ "da830-mmc.1", "tx", EDMA_FILTER_PARAM(1, 29) },
};

and in the DMA driver:
	ecc->dma_slave.filter.map = info->slave_map;
	ecc->dma_slave.filter.mapcnt = info->slavecnt;
	ecc->dma_slave.filter.fn = edma_filter_fn;

When I added the dma_slave_map support it was done in a way that it
could be used by anyone having the same issue as we were facing with
legacy daVinci and OMAP1 boards (no DT support in sight).

Drivers do not need to care about legacy/DT boot.

> 
> The in the DMA driver (omap-dma.c):
> 	od->ddev.filter.map = od->plat->slave_map;
> 	od->ddev.filter.mapcnt = od->plat->slavecnt;
> 	od->ddev.filter.fn = omap_dma_filter_fn;
> 
> When things are converted the filter function no longer needs to be
> exported, it is local to the DMA driver.
> 
>>
>>> Imho on lower spec SoC (and I believe SuperH is) the DMA makes big
>>> difference offloading data movement from the CPU.
>>
>> Assumed it is actually used...
> 
> Right, imho (again) we should not decide if given SoC needs it or not.
> It is up to the drivers to use it or not, but with the dma_slave_map
> there is no difference between DT or legacy boot handling towards DMA.
> 
>> Gr{oetje,eeting}s,
>>
>>                         Geert
>>
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  8:01                 ` Geert Uytterhoeven
  2020-02-04  8:15                   ` Peter Ujfalusi
@ 2020-02-04  9:16                   ` Rob Landley
  2020-02-04  9:27                     ` Geert Uytterhoeven
  1 sibling, 1 reply; 30+ messages in thread
From: Rob Landley @ 2020-02-04  9:16 UTC (permalink / raw)
  To: Geert Uytterhoeven, Peter Ujfalusi
  Cc: John Paul Adrian Glaubitz, Andy Shevchenko, Vinod Koul,
	dmaengine, Dan Williams, Linux-sh list, Linux-Renesas

On 2/4/20 2:01 AM, Geert Uytterhoeven wrote:
> Hi Peter,
> 
> On Tue, Feb 4, 2020 at 7:52 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>> On 03/02/2020 22.34, Geert Uytterhoeven wrote:
>>> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
>>> <glaubitz@physik.fu-berlin.de> wrote:
>>>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
>>>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>>>>
>>>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
>>>> for classical SuperH hardware. It was never merged, unfortunately :(.
>>>
>>> True.
>>>
>>>>> Anyone who cares for DMA on SuperH?
>>>>
>>>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
>>>> essential hardware features?
>>>
>>> It may make a few things slower.

The j-core stuff has DMA but we haven't hooked it up to dmaengine yet. (It's on
the todo list but pretty far down.)

I fought with dmaengine in a 7760 board in 2018, and got it to run its tests but
the ship deadline arrived before I got the ethernet working with it.

I found the documentation fairly impenetrable, is there a good primer on what's
_current_ for new implementations? (I had similar questions for gpio. It's easy
to google for "here's how you did it in 2010"...)

>> I would not drop DMA support but I would suggest to add dma_slave_map
>> for non DT boot so the _compat() can be dropped.
> 
> Which is similar in spirit to gpiod_lookup and clk_register_clkdev(),
> right?
> 
>> Imho on lower spec SoC (and I believe SuperH is) the DMA makes big
>> difference offloading data movement from the CPU.
> 
> Assumed it is actually used...

The turtle boards need it USB, ethernet, and sdcard, but Rich Felker hasn't
finished the j32 port yet (we just got him the updated docs last month) and the
existing implementation is nommu so the things that are using it are reaching
around behind the OS's back...

Rob

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  9:16                   ` Rob Landley
@ 2020-02-04  9:27                     ` Geert Uytterhoeven
  2020-02-04 10:18                       ` Rob Landley
  0 siblings, 1 reply; 30+ messages in thread
From: Geert Uytterhoeven @ 2020-02-04  9:27 UTC (permalink / raw)
  To: Rob Landley
  Cc: Peter Ujfalusi, John Paul Adrian Glaubitz, Andy Shevchenko,
	Vinod Koul, dmaengine, Dan Williams, Linux-sh list,
	Linux-Renesas

Hi Rob,

On Tue, Feb 4, 2020 at 10:12 AM Rob Landley <rob@landley.net> wrote:
> On 2/4/20 2:01 AM, Geert Uytterhoeven wrote:
> > On Tue, Feb 4, 2020 at 7:52 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >> On 03/02/2020 22.34, Geert Uytterhoeven wrote:
> >>> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
> >>> <glaubitz@physik.fu-berlin.de> wrote:
> >>>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
> >>>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
> >>>>
> >>>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
> >>>> for classical SuperH hardware. It was never merged, unfortunately :(.
> >>>
> >>> True.
> >>>
> >>>>> Anyone who cares for DMA on SuperH?
> >>>>
> >>>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
> >>>> essential hardware features?
> >>>
> >>> It may make a few things slower.
>
> The j-core stuff has DMA but we haven't hooked it up to dmaengine yet. (It's on
> the todo list but pretty far down.)

And would use DT.  Hence the issue is not applicable to j-core.

> The turtle boards need it USB, ethernet, and sdcard, but Rich Felker hasn't
> finished the j32 port yet (we just got him the updated docs last month) and the
> existing implementation is nommu so the things that are using it are reaching
> around behind the OS's back...

Is j32 the (rebranded) j4?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  9:27                     ` Geert Uytterhoeven
@ 2020-02-04 10:18                       ` Rob Landley
  0 siblings, 0 replies; 30+ messages in thread
From: Rob Landley @ 2020-02-04 10:18 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Peter Ujfalusi, John Paul Adrian Glaubitz, Andy Shevchenko,
	Vinod Koul, dmaengine, Dan Williams, Linux-sh list,
	Linux-Renesas

On 2/4/20 3:27 AM, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Tue, Feb 4, 2020 at 10:12 AM Rob Landley <rob@landley.net> wrote:
>> On 2/4/20 2:01 AM, Geert Uytterhoeven wrote:
>>> On Tue, Feb 4, 2020 at 7:52 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>>>> On 03/02/2020 22.34, Geert Uytterhoeven wrote:
>>>>> On Mon, Feb 3, 2020 at 9:21 PM John Paul Adrian Glaubitz
>>>>> <glaubitz@physik.fu-berlin.de> wrote:
>>>>>> On 2/3/20 2:32 PM, Geert Uytterhoeven wrote:
>>>>>>> Both rspi and sh-msiof have users on legacy SH (i.e. without DT):
>>>>>>
>>>>>> FWIW, there is a patch set by Yoshinori Sato to add device tree support
>>>>>> for classical SuperH hardware. It was never merged, unfortunately :(.

The problem with converting everything else to j-core has always been regression
testing. I have 2 pieces of classing sh4 hardware, and one classing sh2 board,
and none of them are currently on the same continent I am. :)

(I've got 3 different j-core boards, though. And am currently in the same room
as 2 others. :)

>>>>> True.
>>>>>
>>>>>>> Anyone who cares for DMA on SuperH?
>>>>>>
>>>>>> What is DMA used for on SuperH? Wouldn't dropping it cut support for
>>>>>> essential hardware features?
>>>>>
>>>>> It may make a few things slower.
>>
>> The j-core stuff has DMA but we haven't hooked it up to dmaengine yet. (It's on
>> the todo list but pretty far down.)
> 
> And would use DT.  Hence the issue is not applicable to j-core.

Indeed.

The last classic sh4 board I used did not have DMA hooked up in Linux, and
trying to make current kernels work hit the fact that the device tree people
kept breaking various pre-device tree APIs, and my patches to fix them were met
with "you should convert everything to device tree rather than fix this obvious
regression we caused" and thus never went upstream. (Johnson Controls shipped
them locally and I handed the patches over to the lawyers to do the license
compliance tarball before I left. And of course I posted the fixes to this list...)

It's nice that at least this time somebody is _asking_ before breaking
non-device-tree support for stuff that used to work...

>> The turtle boards need it USB, ethernet, and sdcard, but Rich Felker hasn't
>> finished the j32 port yet (we just got him the updated docs last month) and the
>> existing implementation is nommu so the things that are using it are reaching
>> around behind the OS's back...
> 
> Is j32 the (rebranded) j4?

Yes.

Rob

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04  6:21   ` Vinod Koul
@ 2020-02-04 11:21     ` Andy Shevchenko
  2020-02-05  4:43       ` Vinod Koul
  0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2020-02-04 11:21 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Peter Ujfalusi, dmaengine, Linux Kernel Mailing List, Dan Williams

On Tue, Feb 4, 2020 at 8:21 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 03-02-20, 12:37, Andy Shevchenko wrote:
> > On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >
> > > dma_request_slave_channel_reason() no longer have user in mainline, it
> > > can be removed.
> > >
> > > Advise users of dma_request_slave_channel() and
> > > dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> >
> > How? There are legacy ARM boards you have to care / remove before.
> > DMAengine subsystem makes a p*s off decisions without taking care of
> > (I'm talking now about dma release callback, for example) end users.
>
> Can you elaborate issue you are seeing with dma_release callback?


[    7.980381] intel-lpss 0000:00:1e.3: WARN: Device release is not
defined so it is not safe to unbind this driver while in use

It's not limited to that driver, but actually all I'm maintaining.

Users are not happy!

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-04 11:21     ` Andy Shevchenko
@ 2020-02-05  4:43       ` Vinod Koul
  2020-02-05  8:10         ` Peter Ujfalusi
  0 siblings, 1 reply; 30+ messages in thread
From: Vinod Koul @ 2020-02-05  4:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Peter Ujfalusi, dmaengine, Linux Kernel Mailing List, Dan Williams

On 04-02-20, 13:21, Andy Shevchenko wrote:
> On Tue, Feb 4, 2020 at 8:21 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 03-02-20, 12:37, Andy Shevchenko wrote:
> > > On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> > >
> > > > dma_request_slave_channel_reason() no longer have user in mainline, it
> > > > can be removed.
> > > >
> > > > Advise users of dma_request_slave_channel() and
> > > > dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> > >
> > > How? There are legacy ARM boards you have to care / remove before.
> > > DMAengine subsystem makes a p*s off decisions without taking care of
> > > (I'm talking now about dma release callback, for example) end users.
> >
> > Can you elaborate issue you are seeing with dma_release callback?
> 
> 
> [    7.980381] intel-lpss 0000:00:1e.3: WARN: Device release is not
> defined so it is not safe to unbind this driver while in use

Yes that is expected but is not valid in your case.

Anyway this will be turned off before the release.

> It's not limited to that driver, but actually all I'm maintaining.
> 
> Users are not happy!
> 
> -- 
> With Best Regards,
> Andy Shevchenko

-- 
~Vinod

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-05  4:43       ` Vinod Koul
@ 2020-02-05  8:10         ` Peter Ujfalusi
  2020-02-05 11:31           ` Vinod Koul
  0 siblings, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-05  8:10 UTC (permalink / raw)
  To: Vinod Koul, Andy Shevchenko
  Cc: dmaengine, Linux Kernel Mailing List, Dan Williams

Vinod,

On 05/02/2020 6.43, Vinod Koul wrote:
> On 04-02-20, 13:21, Andy Shevchenko wrote:
>> On Tue, Feb 4, 2020 at 8:21 AM Vinod Koul <vkoul@kernel.org> wrote:
>>>
>>> On 03-02-20, 12:37, Andy Shevchenko wrote:
>>>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>>>>
>>>>> dma_request_slave_channel_reason() no longer have user in mainline, it
>>>>> can be removed.
>>>>>
>>>>> Advise users of dma_request_slave_channel() and
>>>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
>>>>
>>>> How? There are legacy ARM boards you have to care / remove before.
>>>> DMAengine subsystem makes a p*s off decisions without taking care of
>>>> (I'm talking now about dma release callback, for example) end users.
>>>
>>> Can you elaborate issue you are seeing with dma_release callback?
>>
>>
>> [    7.980381] intel-lpss 0000:00:1e.3: WARN: Device release is not
>> defined so it is not safe to unbind this driver while in use
> 
> Yes that is expected but is not valid in your case.

In which case it is valid?

> Anyway this will be turned off before the release.

Looking at the commit which added it and I still don't get the point.
If any of the channel is in use then we should not allow the DMA driver
to go away at all.
Imho there should be a function to check if we can proceed with the
.remove of the driver and fail it if any of the channels are in use.

Hrm, base/dd.c __device_release_driver() does not check the .remove's
return value, so it can not fail.

What is expected if the .remove returns with OK but we still have
channels in use?

After the remove all sorts of things got yanked which might makes the
still in use channels cause issues down the road.

I'm curious why it is a good thing to remotely try to support unbind
when the driver is in use.
It is like one wants to support ext4 removal even when your rootfs is ext4.

I think krefing the DMA driver for channel request/release is just fine,
if user wants to break the system we should not assist...

>> It's not limited to that driver, but actually all I'm maintaining.
>>
>> Users are not happy!
>>
>> -- 
>> With Best Regards,
>> Andy Shevchenko
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-05  8:10         ` Peter Ujfalusi
@ 2020-02-05 11:31           ` Vinod Koul
  2020-02-05 11:56             ` Peter Ujfalusi
  0 siblings, 1 reply; 30+ messages in thread
From: Vinod Koul @ 2020-02-05 11:31 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Andy Shevchenko, dmaengine, Linux Kernel Mailing List, Dan Williams

Hi Peter,

On 05-02-20, 10:10, Peter Ujfalusi wrote:
> Vinod,
> 
> On 05/02/2020 6.43, Vinod Koul wrote:
> > On 04-02-20, 13:21, Andy Shevchenko wrote:
> >> On Tue, Feb 4, 2020 at 8:21 AM Vinod Koul <vkoul@kernel.org> wrote:
> >>>
> >>> On 03-02-20, 12:37, Andy Shevchenko wrote:
> >>>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> >>>>
> >>>>> dma_request_slave_channel_reason() no longer have user in mainline, it
> >>>>> can be removed.
> >>>>>
> >>>>> Advise users of dma_request_slave_channel() and
> >>>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> >>>>
> >>>> How? There are legacy ARM boards you have to care / remove before.
> >>>> DMAengine subsystem makes a p*s off decisions without taking care of
> >>>> (I'm talking now about dma release callback, for example) end users.
> >>>
> >>> Can you elaborate issue you are seeing with dma_release callback?
> >>
> >>
> >> [    7.980381] intel-lpss 0000:00:1e.3: WARN: Device release is not
> >> defined so it is not safe to unbind this driver while in use
> > 
> > Yes that is expected but is not valid in your case.
> 
> In which case it is valid?

It is valid for cases where device can be hotplugged. We didnt handle
that very well earlier. TBH we never had a reason as most of the
embedded cases that is not really doable.

> > Anyway this will be turned off before the release.
> 
> Looking at the commit which added it and I still don't get the point.
> If any of the channel is in use then we should not allow the DMA driver
> to go away at all.

Not really, if the device is already gone, we cant do much about it. We
have to handle that gracefully rather than oopsing

The important part is that the device is gone. Think about a device on
PCI card which is yanked off or a USB device unplugged. Device is
already gone, you can't communicate with it anymore. So all we can do is
handle the condition and exit, hence the new method to let driver know.

> Imho there should be a function to check if we can proceed with the
> .remove of the driver and fail it if any of the channels are in use.
> 
> Hrm, base/dd.c __device_release_driver() does not check the .remove's
> return value, so it can not fail.
> 
> What is expected if the .remove returns with OK but we still have
> channels in use?
> 
> After the remove all sorts of things got yanked which might makes the
> still in use channels cause issues down the road.
> 
> I'm curious why it is a good thing to remotely try to support unbind
> when the driver is in use.
> It is like one wants to support ext4 removal even when your rootfs is ext4.
> 
> I think krefing the DMA driver for channel request/release is just fine,
> if user wants to break the system we should not assist...
> 
> >> It's not limited to that driver, but actually all I'm maintaining.
> >>
> >> Users are not happy!
> >>
> >> -- 
> >> With Best Regards,
> >> Andy Shevchenko
> > 
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 
~Vinod

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-05 11:31           ` Vinod Koul
@ 2020-02-05 11:56             ` Peter Ujfalusi
  2020-02-05 11:59               ` Vinod Koul
  0 siblings, 1 reply; 30+ messages in thread
From: Peter Ujfalusi @ 2020-02-05 11:56 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Shevchenko, dmaengine, Linux Kernel Mailing List, Dan Williams

Hi Vinod,

On 05/02/2020 13.31, Vinod Koul wrote:
>> Looking at the commit which added it and I still don't get the point.
>> If any of the channel is in use then we should not allow the DMA driver
>> to go away at all.
> 
> Not really, if the device is already gone, we cant do much about it. We
> have to handle that gracefully rather than oopsing

Ah, I have not thought about that. True.

> The important part is that the device is gone. Think about a device on
> PCI card which is yanked off or a USB device unplugged. Device is
> already gone, you can't communicate with it anymore. So all we can do is
> handle the condition and exit, hence the new method to let driver know.

But for most devices this is not applicable, I also wondered what should
I do in order to silence the print. Just add an empty device_release?

>> Imho there should be a function to check if we can proceed with the
>> .remove of the driver and fail it if any of the channels are in use.
>>
>> Hrm, base/dd.c __device_release_driver() does not check the .remove's
>> return value, so it can not fail.
>>
>> What is expected if the .remove returns with OK but we still have
>> channels in use?
>>
>> After the remove all sorts of things got yanked which might makes the
>> still in use channels cause issues down the road.
>>
>> I'm curious why it is a good thing to remotely try to support unbind
>> when the driver is in use.
>> It is like one wants to support ext4 removal even when your rootfs is ext4.
>>
>> I think krefing the DMA driver for channel request/release is just fine,
>> if user wants to break the system we should not assist...
>>
>>>> It's not limited to that driver, but actually all I'm maintaining.
>>>>
>>>> Users are not happy!
>>>>
>>>> -- 
>>>> With Best Regards,
>>>> Andy Shevchenko
>>>
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()
  2020-02-05 11:56             ` Peter Ujfalusi
@ 2020-02-05 11:59               ` Vinod Koul
  0 siblings, 0 replies; 30+ messages in thread
From: Vinod Koul @ 2020-02-05 11:59 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Andy Shevchenko, dmaengine, Linux Kernel Mailing List, Dan Williams

On 05-02-20, 13:56, Peter Ujfalusi wrote:
> Hi Vinod,
> 
> On 05/02/2020 13.31, Vinod Koul wrote:
> >> Looking at the commit which added it and I still don't get the point.
> >> If any of the channel is in use then we should not allow the DMA driver
> >> to go away at all.
> > 
> > Not really, if the device is already gone, we cant do much about it. We
> > have to handle that gracefully rather than oopsing
> 
> Ah, I have not thought about that. True.
> 
> > The important part is that the device is gone. Think about a device on
> > PCI card which is yanked off or a USB device unplugged. Device is
> > already gone, you can't communicate with it anymore. So all we can do is
> > handle the condition and exit, hence the new method to let driver know.
> 
> But for most devices this is not applicable, I also wondered what should
> I do in order to silence the print. Just add an empty device_release?

I will send a patch removing this before we hit release :) so nothing to
be done unless you have a hotpluggable device then would be good to add this.

Thanks
-- 
~Vinod

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

end of thread, other threads:[~2020-02-05 11:59 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-03 10:18 [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Peter Ujfalusi
2020-02-03 10:18 ` [PATCH 1/3] dmaengine: Remove unused define for dma_request_slave_channel_reason() Peter Ujfalusi
2020-02-03 13:35   ` Geert Uytterhoeven
2020-02-03 10:18 ` [PATCH 2/3] dmaengine: Mark dma_request_slave_channel() deprecated Peter Ujfalusi
2020-02-03 10:18 ` [PATCH 3/3] dmaengine: Encourage dma_request_slave_channel_compat() users to migrate Peter Ujfalusi
2020-02-03 10:35   ` Andy Shevchenko
2020-02-03 10:37 ` [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan() Andy Shevchenko
2020-02-03 10:59   ` Peter Ujfalusi
2020-02-03 11:16     ` Andy Shevchenko
2020-02-03 12:09       ` Peter Ujfalusi
2020-02-03 12:48         ` Andy Shevchenko
2020-02-03 13:32         ` Geert Uytterhoeven
2020-02-03 20:21           ` John Paul Adrian Glaubitz
2020-02-03 20:34             ` Geert Uytterhoeven
2020-02-03 21:26               ` John Paul Adrian Glaubitz
2020-02-04  8:13                 ` Geert Uytterhoeven
2020-02-04  6:52               ` Peter Ujfalusi
2020-02-04  8:01                 ` Geert Uytterhoeven
2020-02-04  8:15                   ` Peter Ujfalusi
2020-02-04  8:21                     ` Peter Ujfalusi
2020-02-04  9:16                   ` Rob Landley
2020-02-04  9:27                     ` Geert Uytterhoeven
2020-02-04 10:18                       ` Rob Landley
2020-02-04  6:21   ` Vinod Koul
2020-02-04 11:21     ` Andy Shevchenko
2020-02-05  4:43       ` Vinod Koul
2020-02-05  8:10         ` Peter Ujfalusi
2020-02-05 11:31           ` Vinod Koul
2020-02-05 11:56             ` Peter Ujfalusi
2020-02-05 11:59               ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).