linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Andrew F. Davis" <afd@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	<alsa-devel@alsa-project.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ASoC: tlv320aic31xx: Add MICBIAS off setting
Date: Tue, 4 Sep 2018 10:10:24 -0500	[thread overview]
Message-ID: <85973bf4-1acb-1731-d93e-29c8141a3c53@ti.com> (raw)
In-Reply-To: <20180904145503.GD12993@sirena.org.uk>

On 09/04/2018 09:55 AM, Mark Brown wrote:
> On Tue, Sep 04, 2018 at 09:43:03AM -0500, Andrew F. Davis wrote:
>> On 09/04/2018 09:41 AM, Mark Brown wrote:
> 
>>>>> If we really want to pay attention to something setting this up we'd
>>>>> need to completely remove the widget - what the code is doing at the
>>>>> minute is setting the voltage that the bias will go to when enabled,
>>>>> there's still a widget for turning it on and off.  There's some chance
>>>>> that this will break existing boards.
> 
>>>> Turning on bias is controlled separately, automatically in user-space in
>>>> many cases, not based on the board. The DT needs to have a way to state
>>>> that 0v is the needed bias on this board, without this you can not set
>>>> 0v bias and 2v is chosen by default (which is IMHO should be 0v but that
>>>> would change existing behavior so I won't touch that).
> 
>>> Surely turning on MICBIAS at 0V is equivalent to leaving the bias off?
> 
>> It is, but DT doesn't control when or why MICBIAS is turned on or off,
>> only what voltage on should be.
> 
> The DT controls this in so far as it will arrange for the bias to be
> connected to something, if it's not connected to anything then it will
> only be turned on if the machine driver explicitly forces it on which is
> a potential source of problems but seems comfortably in "you broke it,
> you get to keep the pieces" territory.  If we really want to have a way
> of explicitly specifying that some widgets should never be turned on
> then it feels like rather than have something device and widget specific
> like this we should instead have a higher level way of doing that which
> can be applied to any widget, that is something that could be useful
> especially with things like speaker drivers where there's real potential
> for physical damage to the system.
> 


Not sure how that could even look, it would need to be in DT as only
that layer knows the connections, but then the it would also have to
have internal knowledge of the widgets used in the driver..


> Like I said in my original reply I'm also worried that this will break
> existing boards by causing them to change to a voltage of 0 when they
> had managed to end up with the default of 2V which happened to work for
> them.
> 

Then only time this could happen is if they specified MICBIAS_OFF in DT
but got the default 2v instead. In which case, yes, the behavior would
change, but it would also be changing to the correct behavior. We can't
avoid fixing code on the off chance someone depended on the broken
behavior, no progress could ever be made.

  reply	other threads:[~2018-09-04 15:10 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-31 18:05 [PATCH] ASoC: tlv320aic31xx: Add MICBIAS off setting Andrew F. Davis
2018-09-03 11:26 ` Mark Brown
2018-09-04 13:55   ` Andrew F. Davis
2018-09-04 14:41     ` Mark Brown
2018-09-04 14:43       ` Andrew F. Davis
2018-09-04 14:55         ` Mark Brown
2018-09-04 15:10           ` Andrew F. Davis [this message]
2018-09-04 15:56             ` Mark Brown
2018-09-04 16:02               ` Andrew F. Davis
2018-09-04 16:21                 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=85973bf4-1acb-1731-d93e-29c8141a3c53@ti.com \
    --to=afd@ti.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).