In [1], Christoph Hellwig has proposed to remove the wrappers in
include/linux/pci-dma-compat.h.
Some reasons why this API should be removed have been given by Julia
Lawall in [2].
However, updating 'drivers/char/xillybus' is a bit tricky because of its use of
'struct xilly_mapping' and 'struct xilly_endpoint'.
The first patch is just the easy part of the pci_ --> dma_ conversion. This is
mostly done with a coccinelle script.
The second updates 'struct xilly_mapping' (and code using it) to explicitly use
the dma_ API.
The third takes care of 'struct xilly_endpoint' (and code using it).
Finally, the 4th patch is a clean-up to remove a now useless parameter from
'xillybus_init_endpoint()'
These changes also offert the opportunity to merge some code from
'xillybus_pcie.c' and 'xillybus_of.c' into 'xillybus_core.c'.
See 'xilly_dma_sync_single_for_cpu_of()' and
'xilly_dma_sync_single_for_cpu_pci()' for example.
This goes beyond the scope of removing the usage of the deprecated
'pci-dma-compat.h' API. The need of 'xilly_pci_direction()' should then be
discussed.
It could be included afterwards it required.
All these patches have been compile tested only.
[1]: https://lore.kernel.org/kernel-janitors/[email protected]/
[2]: https://lore.kernel.org/kernel-janitors/alpine.DEB.2.22.394.2007120902170.2424@hadrien/
Christophe JAILLET (4):
char: xillybus: Remove usage of the deprecated 'pci-dma-compat.h' API
char: xillybus: Remove usage of 'pci_unmap_single()'
char: xillybus: Remove usage of remaining deprecated pci_ API
char: xillybus: Simplify 'xillybus_init_endpoint()'
drivers/char/xillybus/xillybus.h | 10 ++------
drivers/char/xillybus/xillybus_core.c | 4 +---
drivers/char/xillybus/xillybus_of.c | 2 +-
drivers/char/xillybus/xillybus_pcie.c | 33 ++++++++++++---------------
4 files changed, 18 insertions(+), 31 deletions(-)
--
2.30.2
Ths first argument of 'xillybus_init_endpoint()' is now useless.
Remove it.
Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/char/xillybus/xillybus.h | 3 +--
drivers/char/xillybus/xillybus_core.c | 4 +---
drivers/char/xillybus/xillybus_of.c | 2 +-
drivers/char/xillybus/xillybus_pcie.c | 2 +-
4 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/char/xillybus/xillybus.h b/drivers/char/xillybus/xillybus.h
index 55d47cb13a7b..afce5bb4d127 100644
--- a/drivers/char/xillybus/xillybus.h
+++ b/drivers/char/xillybus/xillybus.h
@@ -134,8 +134,7 @@ struct xilly_mapping {
irqreturn_t xillybus_isr(int irq, void *data);
-struct xilly_endpoint *xillybus_init_endpoint(struct pci_dev *pdev,
- struct device *dev,
+struct xilly_endpoint *xillybus_init_endpoint(struct device *dev,
struct xilly_endpoint_hardware
*ephw);
diff --git a/drivers/char/xillybus/xillybus_core.c b/drivers/char/xillybus/xillybus_core.c
index 0ced9ec6977f..02f30140c2d5 100644
--- a/drivers/char/xillybus/xillybus_core.c
+++ b/drivers/char/xillybus/xillybus_core.c
@@ -1772,8 +1772,7 @@ static const struct file_operations xillybus_fops = {
.poll = xillybus_poll,
};
-struct xilly_endpoint *xillybus_init_endpoint(struct pci_dev *pdev,
- struct device *dev,
+struct xilly_endpoint *xillybus_init_endpoint(struct device *dev,
struct xilly_endpoint_hardware
*ephw)
{
@@ -1783,7 +1782,6 @@ struct xilly_endpoint *xillybus_init_endpoint(struct pci_dev *pdev,
if (!endpoint)
return NULL;
- (void)pdev; // silence a compiler warning, will be removed
endpoint->dev = dev;
endpoint->ephw = ephw;
endpoint->msg_counter = 0x0b;
diff --git a/drivers/char/xillybus/xillybus_of.c b/drivers/char/xillybus/xillybus_of.c
index 1a20b286fd1d..4e6e0c19d8c8 100644
--- a/drivers/char/xillybus/xillybus_of.c
+++ b/drivers/char/xillybus/xillybus_of.c
@@ -120,7 +120,7 @@ static int xilly_drv_probe(struct platform_device *op)
if (of_property_read_bool(dev->of_node, "dma-coherent"))
ephw = &of_hw_coherent;
- endpoint = xillybus_init_endpoint(NULL, dev, ephw);
+ endpoint = xillybus_init_endpoint(dev, ephw);
if (!endpoint)
return -ENOMEM;
diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
index f4be61349ca6..a6ef4ce90649 100644
--- a/drivers/char/xillybus/xillybus_pcie.c
+++ b/drivers/char/xillybus/xillybus_pcie.c
@@ -124,7 +124,7 @@ static int xilly_probe(struct pci_dev *pdev,
struct xilly_endpoint *endpoint;
int rc;
- endpoint = xillybus_init_endpoint(pdev, &pdev->dev, &pci_hw);
+ endpoint = xillybus_init_endpoint(&pdev->dev, &pci_hw);
if (!endpoint)
return -ENOMEM;
--
2.30.2
On 27/08/21 20:17, Christophe JAILLET wrote:
> In [1], Christoph Hellwig has proposed to remove the wrappers in
> include/linux/pci-dma-compat.h.
>
Xillybus' driver is an example for why this is a good idea. But has this
been decided upon? Are we sure that there isn't a single platform where
the DMA mapping for PCI is different from non-PCI, and that such
platform will never be?
If so, is there any reference to that decision?
I think the best way is to put a comment at the top of pci-dma-compat.h
saying that the functions in that header file are deprecated and will go
away soon. That would, more than anything else, convince people like me
to get rid of those PCI-DMA function calls.
The bonus is that the discussion on the patch inserting that comment,
along with the decision to apply or reject it, will become the
authoritative word on this matter.
Thanks,
Eli
On Sat, Aug 28, 2021 at 5:07 PM Eli Billauer <[email protected]> wrote:
>
> On 27/08/21 20:17, Christophe JAILLET wrote:
> > In [1], Christoph Hellwig has proposed to remove the wrappers in
> > include/linux/pci-dma-compat.h.
> >
> Xillybus' driver is an example for why this is a good idea. But has this
> been decided upon? Are we sure that there isn't a single platform where
> the DMA mapping for PCI is different from non-PCI, and that such
> platform will never be?
Yes.
> If so, is there any reference to that decision?
The documentation was updated 11 years ago to only describe the modern
linux/dma-mapping.h interfaces and mark the old bus-specific ones as
no longer recommended, see 216bf58f4092 ("Documentation: convert
PCI-DMA-mapping.txt to use the generic DMA API").
> I think the best way is to put a comment at the top of pci-dma-compat.h
> saying that the functions in that header file are deprecated and will go
> away soon. That would, more than anything else, convince people like me
> to get rid of those PCI-DMA function calls.
The only reason for keeping the old interface around any day longer would
be to identify drivers that have been unmaintained for the past decade
and ignored all the previous cleanup patches that got sent to them.
Arnd
On 29/08/21 00:26, Arnd Bergmann wrote:
>
> The documentation was updated 11 years ago to only describe the modern
> linux/dma-mapping.h interfaces and mark the old bus-specific ones as
> no longer recommended, see 216bf58f4092 ("Documentation: convert
> PCI-DMA-mapping.txt to use the generic DMA API").
>
Thanks, Arnd. That's exactly the kind of reference I was asking about.
And of course, a thanks goes to Christophe as well for drawing my
attention to this issue. A bit surprising it didn't happen back in 2013,
when the driver was included in the kernel. Or possibly in 2014, when it
went out of staging.
As for this patch set, three out of four patches make modifications in
functions that should be deleted altogether. Their only purpose is to
wrap DMA-related calls made by the core driver, so that the pci_* API is
used for PCI devices, and the dma_* API otherwise. As it turns out, this
was a lot of nonsense code from day one.
I'll prepare a patch that removes all this.
Thanks again,
Eli