[RESEND,v2] dmaengine: mxs: fix driver registering

Driver registration fails on SOC imx8mn as its supplier, the clock
control module, is not ready. Since platform_driver_probe(), as
reported by its description, is incompatible with deferred probing,
we have to use platform_driver_register().

Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: stable@vger.kernel.org

---

Changes in v2:
- Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.

 drivers/dma/mxs-dma.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
  

On 07-06-22, 11:58, Dario Binacchi wrote:
> Driver registration fails on SOC imx8mn as its supplier, the clock
> control module, is not ready. Since platform_driver_probe(), as
> reported by its description, is incompatible with deferred probing,
> we have to use platform_driver_register().
> 
> Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
> Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Cc: stable@vger.kernel.org
> 
> ---
> 
> Changes in v2:
> - Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.
> 
>  drivers/dma/mxs-dma.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index 994fc4d2aca4..b8a3e692330d 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -670,7 +670,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
>  	return mxs_chan->status;
>  }
>  
> -static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> +static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)

why drop __init for these...?

>  {
>  	int ret;
>  
> @@ -741,7 +741,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
>  				     ofdma->of_node);
>  }
>  
> -static int __init mxs_dma_probe(struct platform_device *pdev)
> +static int mxs_dma_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
>  	const struct mxs_dma_type *dma_type;
> @@ -839,10 +839,7 @@ static struct platform_driver mxs_dma_driver = {
>  		.name	= "mxs-dma",
>  		.of_match_table = mxs_dma_dt_ids,
>  	},
> +	.probe = mxs_dma_probe,
>  };
>  
> -static int __init mxs_dma_module_init(void)
> -{
> -	return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
> -}

> -subsys_initcall(mxs_dma_module_init);
> +module_platform_driver(mxs_dma_driver);
> -- 
> 2.32.0
  

Hi

On Thu, Jun 9, 2022 at 7:48 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 07-06-22, 11:58, Dario Binacchi wrote:
> > Driver registration fails on SOC imx8mn as its supplier, the clock
> > control module, is not ready. Since platform_driver_probe(), as
> > reported by its description, is incompatible with deferred probing,
> > we have to use platform_driver_register().
> >
> > Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
> > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > Cc: stable@vger.kernel.org
> >
> > ---
> >
> > Changes in v2:
> > - Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.
> >
> >  drivers/dma/mxs-dma.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > index 994fc4d2aca4..b8a3e692330d 100644
> > --- a/drivers/dma/mxs-dma.c
> > +++ b/drivers/dma/mxs-dma.c
> > @@ -670,7 +670,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
> >       return mxs_chan->status;
> >  }
> >
> > -static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > +static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
>
> why drop __init for these...?
>

I think that you refer to the fact that it can not be compiled as a
module, am I right?

Michael

> >  {
> >       int ret;
> >
> > @@ -741,7 +741,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
> >                                    ofdma->of_node);
> >  }
> >
> > -static int __init mxs_dma_probe(struct platform_device *pdev)
> > +static int mxs_dma_probe(struct platform_device *pdev)
> >  {
> >       struct device_node *np = pdev->dev.of_node;
> >       const struct mxs_dma_type *dma_type;
> > @@ -839,10 +839,7 @@ static struct platform_driver mxs_dma_driver = {
> >               .name   = "mxs-dma",
> >               .of_match_table = mxs_dma_dt_ids,
> >       },
> > +     .probe = mxs_dma_probe,
> >  };
> >
> > -static int __init mxs_dma_module_init(void)
> > -{
> > -     return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
> > -}
>
> > -subsys_initcall(mxs_dma_module_init);
> > +module_platform_driver(mxs_dma_driver);
> > --
> > 2.32.0
>
> --
> ~Vinod
  

On 09-06-22, 08:01, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> On Thu, Jun 9, 2022 at 7:48 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 07-06-22, 11:58, Dario Binacchi wrote:
> > > Driver registration fails on SOC imx8mn as its supplier, the clock
> > > control module, is not ready. Since platform_driver_probe(), as
> > > reported by its description, is incompatible with deferred probing,
> > > we have to use platform_driver_register().
> > >
> > > Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
> > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > Cc: stable@vger.kernel.org
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.
> > >
> > >  drivers/dma/mxs-dma.c | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > index 994fc4d2aca4..b8a3e692330d 100644
> > > --- a/drivers/dma/mxs-dma.c
> > > +++ b/drivers/dma/mxs-dma.c
> > > @@ -670,7 +670,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
> > >       return mxs_chan->status;
> > >  }
> > >
> > > -static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > > +static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> >
> > why drop __init for these...?
> >
> 
> I think that you refer to the fact that it can not be compiled as a
> module, am I right?

It is still declared as a module_platform_driver... From changelog I can
understand that you are changing init level from subsys to module (in
fact clocks should be moved up as arch level and dmaengine users as
module) ...

But why remove __init declaration from these? Whatever purpose that may
solve needs to be documented in changelog and perhaps a different patch


> 
> Michael
> 
> > >  {
> > >       int ret;
> > >
> > > @@ -741,7 +741,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
> > >                                    ofdma->of_node);
> > >  }
> > >
> > > -static int __init mxs_dma_probe(struct platform_device *pdev)
> > > +static int mxs_dma_probe(struct platform_device *pdev)
> > >  {
> > >       struct device_node *np = pdev->dev.of_node;
> > >       const struct mxs_dma_type *dma_type;
> > > @@ -839,10 +839,7 @@ static struct platform_driver mxs_dma_driver = {
> > >               .name   = "mxs-dma",
> > >               .of_match_table = mxs_dma_dt_ids,
> > >       },
> > > +     .probe = mxs_dma_probe,
> > >  };
> > >
> > > -static int __init mxs_dma_module_init(void)
> > > -{
> > > -     return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
> > > -}
> >
> > > -subsys_initcall(mxs_dma_module_init);
> > > +module_platform_driver(mxs_dma_driver);
> > > --
> > > 2.32.0
> >
> > --
> > ~Vinod
  

Hi Vinod

On Thu, Jun 9, 2022 at 8:07 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 09-06-22, 08:01, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Thu, Jun 9, 2022 at 7:48 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > On 07-06-22, 11:58, Dario Binacchi wrote:
> > > > Driver registration fails on SOC imx8mn as its supplier, the clock
> > > > control module, is not ready. Since platform_driver_probe(), as
> > > > reported by its description, is incompatible with deferred probing,
> > > > we have to use platform_driver_register().
> > > >
> > > > Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
> > > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > Cc: stable@vger.kernel.org
> > > >
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.
> > > >
> > > >  drivers/dma/mxs-dma.c | 11 ++++-------
> > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > > index 994fc4d2aca4..b8a3e692330d 100644
> > > > --- a/drivers/dma/mxs-dma.c
> > > > +++ b/drivers/dma/mxs-dma.c
> > > > @@ -670,7 +670,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
> > > >       return mxs_chan->status;
> > > >  }
> > > >
> > > > -static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > > > +static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > >
> > > why drop __init for these...?
> > >
> >
> > I think that you refer to the fact that it can not be compiled as a
> > module, am I right?
>
> It is still declared as a module_platform_driver... From changelog I can
> understand that you are changing init level from subsys to module (in
> fact clocks should be moved up as arch level and dmaengine users as
> module) ...

The way the driver was using to register was:
platform_driver_probe(&driver, driver_probe);

The function try to register the driver, one time and if the
dependences is not satisfied,
then there will not a next try, so the driver initialized that way can
not depends to anything
apart himself, or all the dependencies should be ready at the time the
driver_probe is called

>
> But why remove __init declaration from these? Whatever purpose that may
> solve needs to be documented in changelog and perhaps a different patch
>

I was thinking that driver can be compiled as module as other driver
but is bool and not tristate

Michael

>
> >
> > Michael
> >
> > > >  {
> > > >       int ret;
> > > >
> > > > @@ -741,7 +741,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
> > > >                                    ofdma->of_node);
> > > >  }
> > > >
> > > > -static int __init mxs_dma_probe(struct platform_device *pdev)
> > > > +static int mxs_dma_probe(struct platform_device *pdev)
> > > >  {
> > > >       struct device_node *np = pdev->dev.of_node;
> > > >       const struct mxs_dma_type *dma_type;
> > > > @@ -839,10 +839,7 @@ static struct platform_driver mxs_dma_driver = {
> > > >               .name   = "mxs-dma",
> > > >               .of_match_table = mxs_dma_dt_ids,
> > > >       },
> > > > +     .probe = mxs_dma_probe,
> > > >  };
> > > >
> > > > -static int __init mxs_dma_module_init(void)
> > > > -{
> > > > -     return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
> > > > -}
> > >
> > > > -subsys_initcall(mxs_dma_module_init);
> > > > +module_platform_driver(mxs_dma_driver);
> > > > --
> > > > 2.32.0
> > >
> > > --
> > > ~Vinod
>
> --
> ~Vinod
  

On 09-06-22, 08:18, Michael Nazzareno Trimarchi wrote:
> Hi Vinod
> 
> On Thu, Jun 9, 2022 at 8:07 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 09-06-22, 08:01, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Thu, Jun 9, 2022 at 7:48 AM Vinod Koul <vkoul@kernel.org> wrote:
> > > >
> > > > On 07-06-22, 11:58, Dario Binacchi wrote:
> > > > > Driver registration fails on SOC imx8mn as its supplier, the clock
> > > > > control module, is not ready. Since platform_driver_probe(), as
> > > > > reported by its description, is incompatible with deferred probing,
> > > > > we have to use platform_driver_register().
> > > > >
> > > > > Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
> > > > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > > Cc: stable@vger.kernel.org
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.
> > > > >
> > > > >  drivers/dma/mxs-dma.c | 11 ++++-------
> > > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > > > index 994fc4d2aca4..b8a3e692330d 100644
> > > > > --- a/drivers/dma/mxs-dma.c
> > > > > +++ b/drivers/dma/mxs-dma.c
> > > > > @@ -670,7 +670,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
> > > > >       return mxs_chan->status;
> > > > >  }
> > > > >
> > > > > -static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > > > > +static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > > >
> > > > why drop __init for these...?
> > > >
> > >
> > > I think that you refer to the fact that it can not be compiled as a
> > > module, am I right?
> >
> > It is still declared as a module_platform_driver... From changelog I can
> > understand that you are changing init level from subsys to module (in
> > fact clocks should be moved up as arch level and dmaengine users as
> > module) ...
> 
> The way the driver was using to register was:
> platform_driver_probe(&driver, driver_probe);
> 
> The function try to register the driver, one time and if the
> dependences is not satisfied,
> then there will not a next try, so the driver initialized that way can
> not depends to anything
> apart himself, or all the dependencies should be ready at the time the
> driver_probe is called

There are two ways to solve this, you lowered the init level of this
driver but your consumers are going to have same issue...

> 
> >
> > But why remove __init declaration from these? Whatever purpose that may
> > solve needs to be documented in changelog and perhaps a different patch
> >
> 
> I was thinking that driver can be compiled as module as other driver
> but is bool and not tristate

Ok, but why drop __init()
  

HI

On Fri, Jun 10, 2022 at 3:48 PM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 09-06-22, 08:18, Michael Nazzareno Trimarchi wrote:
> > Hi Vinod
> >
> > On Thu, Jun 9, 2022 at 8:07 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > On 09-06-22, 08:01, Michael Nazzareno Trimarchi wrote:
> > > > Hi
> > > >
> > > > On Thu, Jun 9, 2022 at 7:48 AM Vinod Koul <vkoul@kernel.org> wrote:
> > > > >
> > > > > On 07-06-22, 11:58, Dario Binacchi wrote:
> > > > > > Driver registration fails on SOC imx8mn as its supplier, the clock
> > > > > > control module, is not ready. Since platform_driver_probe(), as
> > > > > > reported by its description, is incompatible with deferred probing,
> > > > > > we have to use platform_driver_register().
> > > > > >
> > > > > > Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
> > > > > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > > > Cc: stable@vger.kernel.org
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Add the tag "Cc: stable@vger.kernel.org" in the sign-off area.
> > > > > >
> > > > > >  drivers/dma/mxs-dma.c | 11 ++++-------
> > > > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > > > > index 994fc4d2aca4..b8a3e692330d 100644
> > > > > > --- a/drivers/dma/mxs-dma.c
> > > > > > +++ b/drivers/dma/mxs-dma.c
> > > > > > @@ -670,7 +670,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
> > > > > >       return mxs_chan->status;
> > > > > >  }
> > > > > >
> > > > > > -static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > > > > > +static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
> > > > >
> > > > > why drop __init for these...?
> > > > >
> > > >
> > > > I think that you refer to the fact that it can not be compiled as a
> > > > module, am I right?
> > >
> > > It is still declared as a module_platform_driver... From changelog I can
> > > understand that you are changing init level from subsys to module (in
> > > fact clocks should be moved up as arch level and dmaengine users as
> > > module) ...
> >
> > The way the driver was using to register was:
> > platform_driver_probe(&driver, driver_probe);
> >
> > The function try to register the driver, one time and if the
> > dependences is not satisfied,
> > then there will not a next try, so the driver initialized that way can
> > not depends to anything
> > apart himself, or all the dependencies should be ready at the time the
> > driver_probe is called
>
> There are two ways to solve this, you lowered the init level of this
> driver but your consumers are going to have same issue...
>

Consumers are platform drivers that support -EPROBE_DEFER. Is a problem
this approach?

> >
> > >
> > > But why remove __init declaration from these? Whatever purpose that may
> > > solve needs to be documented in changelog and perhaps a different patch
> > >
> >
> > I was thinking that driver can be compiled as module as other driver
> > but is bool and not tristate
>
> Ok, but why drop __init()

Was a mistake. Things marked as __init are dropped in the init
section. As I said this driver
can not be a .ko and must be in the kernel image, the __init can stay
there. Now I don't remember
how init section are used when a part of the kernel is compiled as
module, but anyway this driver
as only one initialization time

Sorry Vinod, but if you are not happy with the answer, I have the
feeling that you need to give more large
lesson on this topic

Michael


>
> --
> ~Vinod