linux-kernel - Re: [RESEND PATCH v2] dmaengine: mxs: fix driver registering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Jun 2022 19:18:10 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Michael Nazzareno Trimarchi <michael@...rulasolutions.com>
Cc:     Dario Binacchi <dario.binacchi@...rulasolutions.com>,
        linux-kernel@...r.kernel.org,
        Amarula patchwork <linux-amarula@...rulasolutions.com>,
        stable@...r.kernel.org, Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>, dmaengine@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [RESEND PATCH v2] dmaengine: mxs: fix driver registering

On 09-06-22, 08:18, Michael Nazzareno Trimarchi wrote:
> Hi Vinod
> 
> On Thu, Jun 9, 2022 at 8:07 AM Vinod Koul <vkoul@...nel.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@...nel.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@...rulasolutions.com>
> > > > > Signed-off-by: Michael Trimarchi <michael@...rulasolutions.com>
> > > > > Signed-off-by: Dario Binacchi <dario.binacchi@...rulasolutions.com>
> > > > > Cc: stable@...r.kernel.org
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Add the tag "Cc: stable@...r.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()

-- 
~Vinod