LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] parisc/parport_gsc: switch from 'pci_' to 'dma_' API
@ 2021-08-23 21:30 Christophe JAILLET
  2021-08-24 10:24 ` Robin Murphy
  0 siblings, 1 reply; 3+ messages in thread
From: Christophe JAILLET @ 2021-08-23 21:30 UTC (permalink / raw)
  To: James.Bottomley, deller, sudipm.mukherjee, sumit.semwal,
	christian.koenig
  Cc: linux-parisc, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel, kernel-janitors, Christophe JAILLET

The wrappers in include/linux/pci-dma-compat.h should go away.

The patch has been generated with the coccinelle script below.

@@
expression e1, e2, e3, e4;
@@
-    pci_free_consistent(e1, e2, e3, e4)
+    dma_free_coherent(&e1->dev, e2, e3, e4)

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
If needed, see post from Christoph Hellwig on the kernel-janitors ML:
   https://marc.info/?l=kernel-janitors&m=158745678307186&w=4

This has *NOT* been compile tested because I don't have the needed
configuration.
ssdfs
---
 drivers/parport/parport_gsc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
index 1e43b3f399a8..db912fa6b6df 100644
--- a/drivers/parport/parport_gsc.c
+++ b/drivers/parport/parport_gsc.c
@@ -390,9 +390,8 @@ static int __exit parport_remove_chip(struct parisc_device *dev)
 		if (p->irq != PARPORT_IRQ_NONE)
 			free_irq(p->irq, p);
 		if (priv->dma_buf)
-			pci_free_consistent(priv->dev, PAGE_SIZE,
-					    priv->dma_buf,
-					    priv->dma_handle);
+			dma_free_coherent(&priv->dev->dev, PAGE_SIZE,
+					  priv->dma_buf, priv->dma_handle);
 		kfree (p->private_data);
 		parport_put_port(p);
 		kfree (ops); /* hope no-one cached it */
-- 
2.30.2


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

* Re: [PATCH] parisc/parport_gsc: switch from 'pci_' to 'dma_' API
  2021-08-23 21:30 [PATCH] parisc/parport_gsc: switch from 'pci_' to 'dma_' API Christophe JAILLET
@ 2021-08-24 10:24 ` Robin Murphy
  2021-08-24 19:39   ` Christophe JAILLET
  0 siblings, 1 reply; 3+ messages in thread
From: Robin Murphy @ 2021-08-24 10:24 UTC (permalink / raw)
  To: Christophe JAILLET, James.Bottomley, deller, sudipm.mukherjee,
	sumit.semwal, christian.koenig
  Cc: linux-parisc, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel, kernel-janitors

On 2021-08-23 22:30, Christophe JAILLET wrote:
> The wrappers in include/linux/pci-dma-compat.h should go away.
> 
> The patch has been generated with the coccinelle script below.
> 
> @@
> expression e1, e2, e3, e4;
> @@
> -    pci_free_consistent(e1, e2, e3, e4)
> +    dma_free_coherent(&e1->dev, e2, e3, e4)
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> If needed, see post from Christoph Hellwig on the kernel-janitors ML:
>     https://marc.info/?l=kernel-janitors&m=158745678307186&w=4
> 
> This has *NOT* been compile tested because I don't have the needed
> configuration.
> ssdfs
> ---
>   drivers/parport/parport_gsc.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c
> index 1e43b3f399a8..db912fa6b6df 100644
> --- a/drivers/parport/parport_gsc.c
> +++ b/drivers/parport/parport_gsc.c
> @@ -390,9 +390,8 @@ static int __exit parport_remove_chip(struct parisc_device *dev)
>   		if (p->irq != PARPORT_IRQ_NONE)
>   			free_irq(p->irq, p);
>   		if (priv->dma_buf)
> -			pci_free_consistent(priv->dev, PAGE_SIZE,
> -					    priv->dma_buf,
> -					    priv->dma_handle);
> +			dma_free_coherent(&priv->dev->dev, PAGE_SIZE,
> +					  priv->dma_buf, priv->dma_handle);

Hmm, seeing a free on its own made me wonder where the corresponding 
alloc was, but on closer inspection it seems there isn't one. AFAICS 
priv->dma_buf is only ever assigned with NULL (and priv->dev doesn't 
seem to be assigned at all), so this could likely just be removed. In 
fact it looks like all the references to DMA in this driver are just 
copy-paste from parport_pc and unused.

Robin.

>   		kfree (p->private_data);
>   		parport_put_port(p);
>   		kfree (ops); /* hope no-one cached it */
> 

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

* Re: [PATCH] parisc/parport_gsc: switch from 'pci_' to 'dma_' API
  2021-08-24 10:24 ` Robin Murphy
@ 2021-08-24 19:39   ` Christophe JAILLET
  0 siblings, 0 replies; 3+ messages in thread
From: Christophe JAILLET @ 2021-08-24 19:39 UTC (permalink / raw)
  To: Robin Murphy, James.Bottomley, deller, sudipm.mukherjee,
	sumit.semwal, christian.koenig
  Cc: linux-parisc, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel, kernel-janitors

Le 24/08/2021 à 12:24, Robin Murphy a écrit :
> On 2021-08-23 22:30, Christophe JAILLET wrote:
>> The wrappers in include/linux/pci-dma-compat.h should go away.
>>
>> The patch has been generated with the coccinelle script below.
>>
>> @@
>> expression e1, e2, e3, e4;
>> @@
>> -    pci_free_consistent(e1, e2, e3, e4)
>> +    dma_free_coherent(&e1->dev, e2, e3, e4)
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> If needed, see post from Christoph Hellwig on the kernel-janitors ML:
>>     https://marc.info/?l=kernel-janitors&m=158745678307186&w=4
>>
>> This has *NOT* been compile tested because I don't have the needed
>> configuration.
>> ssdfs
>> ---
>>   drivers/parport/parport_gsc.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/parport/parport_gsc.c 
>> b/drivers/parport/parport_gsc.c
>> index 1e43b3f399a8..db912fa6b6df 100644
>> --- a/drivers/parport/parport_gsc.c
>> +++ b/drivers/parport/parport_gsc.c
>> @@ -390,9 +390,8 @@ static int __exit parport_remove_chip(struct 
>> parisc_device *dev)
>>           if (p->irq != PARPORT_IRQ_NONE)
>>               free_irq(p->irq, p);
>>           if (priv->dma_buf)
>> -            pci_free_consistent(priv->dev, PAGE_SIZE,
>> -                        priv->dma_buf,
>> -                        priv->dma_handle);
>> +            dma_free_coherent(&priv->dev->dev, PAGE_SIZE,
>> +                      priv->dma_buf, priv->dma_handle);
> 
> Hmm, seeing a free on its own made me wonder where the corresponding 
> alloc was, but on closer inspection it seems there isn't one. AFAICS 
> priv->dma_buf is only ever assigned with NULL (and priv->dev doesn't 
> seem to be assigned at all), so this could likely just be removed. In 
> fact it looks like all the references to DMA in this driver are just 
> copy-paste from parport_pc and unused.
> 
> Robin.
> 

Agreed. I had the same reaction, but as the patch should basically be a 
no-op, it looked safe, even if non-optimal.

Looking at parport_gsc_private, pword, readIntrThreshold and 
writeIntrThreshold also look unused.

My own goal is to remove the 'pci_free_consistent()' call in order to 
remove a deprecated API.

As said, I can not compile this driver.
I could send a blind fix that axes 'pci_free_consistent()' and remove 
some fields in parport_gsc_private, but I'm not sure that it is the best 
way to go.

Do you prefer to look at it by yourself or do you prefer to compile test 
my trials?


CJ



>>           kfree (p->private_data);
>>           parport_put_port(p);
>>           kfree (ops); /* hope no-one cached it */
>>
> 


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

end of thread, other threads:[~2021-08-24 19:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-23 21:30 [PATCH] parisc/parport_gsc: switch from 'pci_' to 'dma_' API Christophe JAILLET
2021-08-24 10:24 ` Robin Murphy
2021-08-24 19:39   ` Christophe JAILLET

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