Re: [PATCH 3/6] ASoC: ux500: Drop pinctrl sleep support

From: Mark Brown
Date: Wed May 08 2013 - 09:54:30 EST


On Wed, May 08, 2013 at 03:10:20PM +0200, Fabio Baltieri wrote:
> On Wed, May 08, 2013 at 01:32:25PM +0100, Mark Brown wrote:

> > I'm saying that if functions like enable_msp() don't work reliably then
> > removing some but not all of their functionality isn't an obviously good
> > approach to fixing that. Why does the other functionality work well but
> > not this bit? It sounds like there's some reference counting bug here
> > is all...

> Yes, it started as a reference counting bug, due to the actual counter
> not being shared between ux500-msp-i2s instances.

> That said, the actual fork of this driver deployed by STE internally
> does not handle I2S pin sleep state, and I was not able to find any
> other ASoC driver that does that, which seems reasonable to me as I
> can't come up with a reason to put those pins in hi-z anyway.

But why does the rest of the code work well if the reference counting is
wrong, it's in the middle of a big block of code? This all smells like
this change is papering over a specific symptom of some underlying issue
- if that's not the case then it needs to be clearer why.

> If I understood the problem correctly you do that when you want to cut
> power completely to some peripherals to avoid spurious current paths,
> and that should not be the case for the audio codec, especially in this
> case where it's part of a big multifuntion IC.

Being a MFD should have nothing to do with this?

Attachment: signature.asc
Description: Digital signature