Re: [PATCH] i2c: rcar: disable runtime PM correctly in slave mode

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]

On Wed, Dec 16, 2015 at 04:55:28PM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Wed, Dec 16, 2015 at 4:43 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > I have another case, may I ask your advice about this, too? When an I2C
> > bus is marked in DT as multi-master, then RuntimePM also needs to be
> > disabled, because arbitration detection needs to stay awake. I am
> > currently implementing this for the i2c-rcar driver:
> >
> > -       pm_runtime_enable(dev);
> > +       /* No RuntimePM in multi-master to keep arbitration working */
> > +       if (!of_get_property(dev->of_node, "multi-master", NULL)) {
> > +               pm_runtime_enable(dev);
> > +               priv->flags |= ID_P_PM;
> > +       }
> > +
> >         pm_runtime_get_sync(dev);
> >         ...
> >
> > @@ -664,7 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
> >         struct device *dev = &pdev->dev;
> >
> >         i2c_del_adapter(&priv->adap);
> > -       pm_runtime_disable(dev);
> > +       if (priv->flags & ID_P_PM)
> > +               pm_runtime_disable(dev);
> >
> >         return 0;
> >  }
> >
> > Here, I'd tend to keep using enable/disable, although get/put would
> > probably also work. What is the rule of thumb using this pattern or the
> > other?
> 
> Have you actually tried the above?

Nope, I just finished the sketching phase when Alan's mail came along.

> All our drivers rely on Runtime PM for the power/clock domain handling.
> I believe the pm_runtime_get_sync() won't do anything if you haven't enabled
> Runtime PM for the device, so the device's module clock won't be enabled at
> all.

This was another question I had: Is this a bug or a feature :) But if
you say it is policy, I will count this as "feature" and switch to
get/put here as well.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]