Re: [PATCH 2/5] spi: i.MX: Move to coredevice_initcall

[Alexander Shiyan]

Mon, 12 May 2014 12:38:50 +0200 от Sascha Hauer <s.hauer at pengutronix.de>:
> On Sun, May 11, 2014 at 10:49:26PM +0400, Alexander Shiyan wrote:
> > Thu,  8 May 2014 09:31:36 +0200 от Sascha Hauer <s.hauer at pengutronix.de>:
> > > SPI is often used by other devices, so make sure it's initialized
> > > early.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer at pengutronix.de>
> > > ---
> > >  drivers/spi/imx_spi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
> > > index e07cf1a..3146339 100644
> > > --- a/drivers/spi/imx_spi.c
> > > +++ b/drivers/spi/imx_spi.c
> > > @@ -586,4 +586,4 @@ static struct driver_d imx_spi_driver = {
> > >  	.of_compatible = DRV_OF_COMPAT(imx_spi_dt_ids),
> > >  	.id_table = imx_spi_ids,
> > >  };
> > > -device_platform_driver(imx_spi_driver);
> > > +coredevice_platform_driver(imx_spi_driver);
> > > -- 
> > 
> > Maybe we should put all drivers in a single runlevel and implement
> > handle of EPROBE_DEFER?
> 
> I think about EPROBE_DEFER for longer now. What has hold me back was
> that we would have to release all resources before returning
> EPROBE_DEFER. Most drivers don't have proper error pathes implemented
> and I don't see much gain in fixing this. For a bootloader the error
> release pathes are unnecessary overhead, because nobody will ever
> use the same resources again (at least without EPROBE_DEFER).
> My recent idea to work around that issue is this:
> 
> - put the drivers private data into dev->priv
> - when returning an error from probe, instead of freeing the data
>   keep it.
> - when probing again re-use the data.
> 
> With this drivers could be written like:
> 
> 	priv = dev->priv;
> 	if (!priv) {
> 		priv = xzalloc(sizeof(priv));
> 		dev->priv = priv;
> 	}
> 
> 	if (!priv->clk) {
> 		clk = clk_get(dev, NULL);
> 		if (IS_ERR(clk))
> 			return PTR_ERR(clk);
> 		priv->clk = clk;
> 		clk_enable(priv->clk);
> 	}
> 
> 	if (!priv->regulator) {
> 		regulator = regulator_get(dev, "vmmc");
> 		if (IS_ERR(regulator))
> 			return PTR_ERR(regulator);
> 		priv->regulator = regulator;
> 	}
> 
> This way drivers could allocate the missing resources as needed without
> freeing them between different calls to probe().

The idea is good, but I think that it will not be so easy to do.
In addition, we obtain additional incompatibility with the kernel code.

---