[PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
balbi at ti.com
Thu Jan 30 21:29:54 EST 2014
On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
> <maxime.ripard at free-electrons.com> wrote:
> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
> >> > +config SPI_SUN6I
> >> > + tristate "Allwinner A31 SPI controller"
> >> > + depends on ARCH_SUNXI || COMPILE_TEST
> >> > + select PM_RUNTIME
> >> > + help
> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
> >> > +
> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
> >> The usual idiom is that the device starts out powered up (flagged using
> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
> >> compiled in. That way if for some reason people want to avoid runtime
> >> PM they can still use the device.
> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
> > general are defined to pretty much empty functions, how the
> > suspend/resume callbacks are called then? Obviously, we need them to
> > be run, hence why I added the select here, but now I'm seeing a
> > construct like what's following acceptable then?
> Even with your 'select', The runtime PM callbacks will never be called
> in the current driver. pm_runtime_enable() doesn't do any runtime PM
> transitions. It just allows transitions to happen when they're
> triggered by _get()/_put()/etc.
> > pm_runtime_enable(&pdev->dev);
> > if (!pm_runtime_enabled(&pdev->dev))
> > sun6i_spi_runtime_resume(&pdev->dev);
> Similarily here, it's not the pm_runtime_enable that will fail when
> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
> that will fail.
> What you want is something like this in ->probe()
> /* now, device is always activated whether or not runtime PM is enabled */
> pm_runtime_set_active(); /* tells runtime PM core device is
> already active */
shouldn't this be done before pm_runtime_enable() ?
> This 'get' will increase the usecount, but not actually call the
> callbacks because we told the RPM core that the device was already
> activated with _set_active().
> And then, in ->remove(), you'll want
in ->remove() you actually want a put_sync() right ? You don't want to
schedule anything since you're just about to disable pm_runtime.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 819 bytes
Desc: Digital signature
More information about the linux-arm-kernel