[PATCH 0/2] fix loadable module for DPAA Ethernet

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] fix loadable module for DPAA Ethernet

Madalin Bucur-2
The DPAA Ethernet makes use of a symbol that is not exported.
Address the issue by propagating the dma_ops rather than calling
arch_setup_dma_ops().

Madalin Bucur (2):
  fsl/fman: propagate dma_ops
  dpaa_eth: reuse the dma_ops provided by the FMan MAC device

 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
 drivers/net/ethernet/freescale/fman/mac.c      | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

--
2.1.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] fsl/fman: propagate dma_ops

Madalin Bucur-2
Make sure dma_ops are set, to be later used by the Ethernet driver.

Signed-off-by: Madalin Bucur <[hidden email]>
---
 drivers/net/ethernet/freescale/fman/mac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 0b31f85..6e67d22f 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -623,6 +623,8 @@ static struct platform_device *dpaa_eth_add_device(int fman_id,
  goto no_mem;
  }
 
+ set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
+
  ret = platform_device_add_data(pdev, &data, sizeof(data));
  if (ret)
  goto err;
--
2.1.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] dpaa_eth: reuse the dma_ops provided by the FMan MAC device

Madalin Bucur-2
In reply to this post by Madalin Bucur-2
Remove the use of arch_setup_dma_ops() that was not exported
and was breaking loadable module compilation.

Signed-off-by: Madalin Bucur <[hidden email]>
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 9a520e4..290ad05 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2647,7 +2647,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
  priv->buf_layout[TX].priv_data_size = DPAA_TX_PRIV_DATA_SIZE; /* Tx */
 
  /* device used for DMA mapping */
- arch_setup_dma_ops(dev, 0, 0, NULL, false);
+ set_dma_ops(dev, get_dma_ops(&pdev->dev));
  err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
  if (err) {
  dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
--
2.1.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] fix loadable module for DPAA Ethernet

David Miller-13
In reply to this post by Madalin Bucur-2
From: Madalin Bucur <[hidden email]>
Date: Mon, 19 Jun 2017 18:04:15 +0300

> The DPAA Ethernet makes use of a symbol that is not exported.
> Address the issue by propagating the dma_ops rather than calling
> arch_setup_dma_ops().

Series applied, thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] fsl/fman: propagate dma_ops

Geert Uytterhoeven
In reply to this post by Madalin Bucur-2
Hi Madalin,

On Mon, Jun 19, 2017 at 5:04 PM, Madalin Bucur <[hidden email]> wrote:

> Make sure dma_ops are set, to be later used by the Ethernet driver.
>
> Signed-off-by: Madalin Bucur <[hidden email]>
> ---
>  drivers/net/ethernet/freescale/fman/mac.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
> index 0b31f85..6e67d22f 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -623,6 +623,8 @@ static struct platform_device *dpaa_eth_add_device(int fman_id,
>                 goto no_mem;
>         }
>
> +       set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
> +

When compile-testing with f NO_DMA=y:

    drivers/net/ethernet/freescale/fman/mac.c: In function
‘dpaa_eth_add_device’:
    drivers/net/ethernet/freescale/fman/mac.c:626: error: implicit
declaration of function ‘set_dma_ops’

Reverting commit 5567e989198b5a8d fixes this regression in v4.12-rc7.

Why is this change needed?
There's no single other call to the DMA API in this file?

If it's really needed, can't set_dma_ops() be called from the driver that
needs it, cfr. your other patch "[PATCH 2/2] dpaa_eth: reuse the dma_ops
provided by the FMan MAC device"?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [hidden email]

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
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH 1/2] fsl/fman: propagate dma_ops

Madalin Bucur-2
> -----Original Message-----
> From: [hidden email] [mailto:[hidden email]]
> On Behalf Of Geert Uytterhoeven
> Sent: Monday, June 26, 2017 10:49 AM
> To: Madalin-cristian Bucur <[hidden email]>
> Cc: [hidden email]; David S. Miller <[hidden email]>;
> [hidden email]; [hidden email]
> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
>
> Hi Madalin,
>
> On Mon, Jun 19, 2017 at 5:04 PM, Madalin Bucur <[hidden email]>
> wrote:
> > Make sure dma_ops are set, to be later used by the Ethernet driver.
> >
> > Signed-off-by: Madalin Bucur <[hidden email]>
> > ---
> >  drivers/net/ethernet/freescale/fman/mac.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> b/drivers/net/ethernet/freescale/fman/mac.c
> > index 0b31f85..6e67d22f 100644
> > --- a/drivers/net/ethernet/freescale/fman/mac.c
> > +++ b/drivers/net/ethernet/freescale/fman/mac.c
> > @@ -623,6 +623,8 @@ static struct platform_device
> *dpaa_eth_add_device(int fman_id,
> >                 goto no_mem;
> >         }
> >
> > +       set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
> > +
>
> When compile-testing with f NO_DMA=y:
>
>     drivers/net/ethernet/freescale/fman/mac.c: In function
> ‘dpaa_eth_add_device’:
>     drivers/net/ethernet/freescale/fman/mac.c:626: error: implicit
> declaration of function ‘set_dma_ops’
>
> Reverting commit 5567e989198b5a8d fixes this regression in v4.12-rc7.
>
> Why is this change needed?
> There's no single other call to the DMA API in this file?

We're setting here the dma_ops that are later used in the other driver/patch.
The problem is we now depend upon DMA but do not explicitly declare it:

<<Code that depends on the dma-mapping API needs to set 'depends on HAS_DMA'
in its Kconfig>>

I'll need to add this to the FMan driver Kconfig.

> If it's really needed, can't set_dma_ops() be called from the driver that
> needs it, cfr. your other patch "[PATCH 2/2] dpaa_eth: reuse the dma_ops
> provided by the FMan MAC device"?
>
> Thanks!
>
> 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

Thanks,
Madalin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] fsl/fman: propagate dma_ops

Geert Uytterhoeven
Hi Madalin,

On Mon, Jun 26, 2017 at 4:55 PM, Madalin-cristian Bucur
<[hidden email]> wrote:

>> -----Original Message-----
>> From: [hidden email] [mailto:[hidden email]]
>> On Behalf Of Geert Uytterhoeven
>> Sent: Monday, June 26, 2017 10:49 AM
>> To: Madalin-cristian Bucur <[hidden email]>
>> Cc: [hidden email]; David S. Miller <[hidden email]>;
>> [hidden email]; [hidden email]
>> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
>>
> On Mon, Jun 19, 2017 at 5:04 PM, Madalin Bucur <[hidden email]>
>> wrote:
>> > Make sure dma_ops are set, to be later used by the Ethernet driver.
>> >
>> > Signed-off-by: Madalin Bucur <[hidden email]>
>> > ---
>> >  drivers/net/ethernet/freescale/fman/mac.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/drivers/net/ethernet/freescale/fman/mac.c
>> b/drivers/net/ethernet/freescale/fman/mac.c
>> > index 0b31f85..6e67d22f 100644
>> > --- a/drivers/net/ethernet/freescale/fman/mac.c
>> > +++ b/drivers/net/ethernet/freescale/fman/mac.c
>> > @@ -623,6 +623,8 @@ static struct platform_device
>> *dpaa_eth_add_device(int fman_id,
>> >                 goto no_mem;
>> >         }
>> >
>> > +       set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
>> > +
>>
>> When compile-testing with f NO_DMA=y:
>>
>>     drivers/net/ethernet/freescale/fman/mac.c: In function
>> ‘dpaa_eth_add_device’:
>>     drivers/net/ethernet/freescale/fman/mac.c:626: error: implicit
>> declaration of function ‘set_dma_ops’
>>
>> Reverting commit 5567e989198b5a8d fixes this regression in v4.12-rc7.
>>
>> Why is this change needed?
>> There's no single other call to the DMA API in this file?
>
> We're setting here the dma_ops that are later used in the other driver/patch.
> The problem is we now depend upon DMA but do not explicitly declare it:
>
> <<Code that depends on the dma-mapping API needs to set 'depends on HAS_DMA'
> in its Kconfig>>

Sure. But only if the driver really uses DMA.
I can stick a set_dma_ops() call in whatever driver, but that doesn't
mean it will
suddenly use DMA.
Why does the FMan driver suddenly has a dependency on DMA, if it doesn't
use DMA?

> I'll need to add this to the FMan driver Kconfig.

Why does the FMan driver need this?
Why can't his call be done in the driver that uses the DMA APIO?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [hidden email]

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
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH 1/2] fsl/fman: propagate dma_ops

Madalin Bucur-2
> -----Original Message-----
> From: [hidden email] [mailto:[hidden email]]
> On Behalf Of Geert Uytterhoeven
> Sent: Monday, June 26, 2017 7:24 PM
> To: Madalin-cristian Bucur <[hidden email]>
> Cc: [hidden email]; David S. Miller <[hidden email]>;
> [hidden email]; [hidden email]
> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
>
> Hi Madalin,
>
> On Mon, Jun 26, 2017 at 4:55 PM, Madalin-cristian Bucur
> <[hidden email]> wrote:
> >> -----Original Message-----
> >> From: [hidden email]
> [mailto:[hidden email]]
> >> On Behalf Of Geert Uytterhoeven
> >> Sent: Monday, June 26, 2017 10:49 AM
> >> To: Madalin-cristian Bucur <[hidden email]>
> >> Cc: [hidden email]; David S. Miller <[hidden email]>;
> >> [hidden email]; [hidden email]
> >> Subject: Re: [PATCH 1/2] fsl/fman: propagate dma_ops
> >>
> > On Mon, Jun 19, 2017 at 5:04 PM, Madalin Bucur <[hidden email]>
> >> wrote:
> >> > Make sure dma_ops are set, to be later used by the Ethernet driver.
> >> >
> >> > Signed-off-by: Madalin Bucur <[hidden email]>
> >> > ---
> >> >  drivers/net/ethernet/freescale/fman/mac.c | 2 ++
> >> >  1 file changed, 2 insertions(+)
> >> >
> >> > diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> >> b/drivers/net/ethernet/freescale/fman/mac.c
> >> > index 0b31f85..6e67d22f 100644
> >> > --- a/drivers/net/ethernet/freescale/fman/mac.c
> >> > +++ b/drivers/net/ethernet/freescale/fman/mac.c
> >> > @@ -623,6 +623,8 @@ static struct platform_device
> >> *dpaa_eth_add_device(int fman_id,
> >> >                 goto no_mem;
> >> >         }
> >> >
> >> > +       set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
> >> > +
> >>
> >> When compile-testing with f NO_DMA=y:
> >>
> >>     drivers/net/ethernet/freescale/fman/mac.c: In function
> >> ‘dpaa_eth_add_device’:
> >>     drivers/net/ethernet/freescale/fman/mac.c:626: error: implicit
> >> declaration of function ‘set_dma_ops’
> >>
> >> Reverting commit 5567e989198b5a8d fixes this regression in v4.12-rc7.
> >>
> >> Why is this change needed?
> >> There's no single other call to the DMA API in this file?
> >
> > We're setting here the dma_ops that are later used in the other
> driver/patch.
> > The problem is we now depend upon DMA but do not explicitly declare it:
> >
> > <<Code that depends on the dma-mapping API needs to set 'depends on
> HAS_DMA'
> > in its Kconfig>>
>
> Sure. But only if the driver really uses DMA.
> I can stick a set_dma_ops() call in whatever driver, but that doesn't
> mean it will
> suddenly use DMA.
> Why does the FMan driver suddenly has a dependency on DMA, if it doesn't
> use DMA?
>
> > I'll need to add this to the FMan driver Kconfig.
>
> Why does the FMan driver need this?
> Why can't his call be done in the driver that uses the DMA APIO?

The DPAA Ethernet driver makes use of DMA ops. It used to get them from
an API call (arch_setup_dma_ops) that was not exported. The DPAA Ethernet
that makes use of the FMan devices does not get the dma_ops as it does not
probe neither as an OF platform device nor thorough ACPI. It probes as a
platform device based on information prepared by the FMan driver. What the
FMan change [1] does is supplement the information shared with the Ethernet
driver with the dma_ops that the FMan driver gets during OF probing. There
are no scenarios one can use the DPAA drivers with NO_DMA, as far as I know.

For general info on the DPAA drivers please refer to the documentation
found in Documentation/networking/dpaa.txt. For the probing of the Ethernet
driver see change [2] and dpaa_eth_add_device() in fsl/fman, dpaa_eth_probe()
in dpaa_eth.
 
[1] 5567e989198b5a8d fsl/fman: propagate dma_ops
[2] fb52728a9294d97d dpaa_eth: reuse the dma_ops provided by the FMan MAC device

> 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

Thanks,
Madalin