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] [day] [month] [year] [list]

Date:   Fri, 10 Jun 2022 16:44:10 +0200
From:   Michael Nazzareno Trimarchi <michael@...rulasolutions.com>
To:     Vinod Koul <vkoul@...nel.org>
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

HI

On Fri, Jun 10, 2022 at 3:48 PM Vinod Koul <vkoul@...nel.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@...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...
>

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



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@...rulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@...rulasolutions.com
www.amarulasolutions.com