On 09/25/2012 05:24 AM, Peter Ujfalusi wrote:
On 09/25/2012 02:11 PM, Mark Brown wrote:
On Mon, Sep 24, 2012 at 04:37:23PM +0200, Linus Walleij wrote:
On Mon, Sep 24, 2012 at 12:17 PM, Mark Brown
Well, the problem here is that people keep wanting to add one shot pinctrl calls in drivers which clearly suggests that it ought to be factored out.
OK so can we find some suitable middle ground here?
Something like drivers could add some boolean flag to "opt-in" for the core to handle this or so?
Well, I don't know if we need to - it sounds like the platforms that are running around adding these pinctrl calls to all the drivers are doing it wrong and should be fixed.
In the scope of audio... I guess it is one thing to use the pinctrl framework to configure the mux for the pins (which was the original idea of the patch). This should be taken care in some centralized place, I agree. But the pinctrl could be used to change the mux runtime. For example in OMAP for most of the audio related pins we need to select MODE0 mux configuration in order to route the signal to the pin. However the same pin can be configured to SAFE_MODE which disconnects the pin from outside. During audio activity we obviously need to configure these pins to MODE0 but when there is not activity we could set them to SAFE_MODE which could result some power savings (preventing leakage, etc).
For this to work it would be ideal to use the pm_runtime as a centralized place to handle clocks, pinctrl, etc... But again, what to do with the cases when this is not needed (the pinctrl part for the devices)?
Maybe it can be opt-in, like pm_runtime support at all, e.g. a driver that supports it can:
pm_runtime_enable_pinctrl(dev, pinctrl_state_name);
where if pinctrl_state_name==NULL, it uses PINCTRL_STATE_IDLE. (Making this a parameter would allow for devices that use a more complex set of states where the default state names may not be appropriate). Perhaps we could phase that in if/when needed, by introducing pm_runtime_enable_pinctrl(dev) first, then adding pm_runtime_enable_pinctrl_state(dev, state) later?
Of course, I suppose that only really addresses the case where the IP/driver itself knows if/when to enable this feature - if an I2S IP block is used in 10 different SoCs, and runtime pinctrl is only useful on 5 of them, how does it know when to make that call?