All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Mark Brown <broonie@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh@kernel.org>,
	Linux-ALSA <alsa-devel@alsa-project.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	devicetree@vger.kernel.org, Sascha Hauer <s.hauer@pengutronix.de>,
	Marco Felsch <m.felsch@pengutronix.de>,
	Liam Girdwood <lgirdwood@gmail.com>,
	kernel@pengutronix.de
Subject: Re: [PATCH] ASoC: ssm2602: Fix ADC powerup sequencing
Date: Tue, 05 Jun 2018 11:58:51 +0200	[thread overview]
Message-ID: <1528192731.4074.7.camel@pengutronix.de> (raw)
In-Reply-To: <20180525172441.GN4828@sirena.org.uk>

Hi Mark,

On Fri, 2018-05-25 at 18:24 +0100, Mark Brown wrote:
> On Fri, May 25, 2018 at 05:18:09PM +0200, Philipp Zabel wrote:
> > On Fri, 2018-05-25 at 15:52 +0100, Mark Brown wrote:
> > > On Fri, May 25, 2018 at 01:42:53PM +0200, Marco Felsch wrote:
> > > > Also the formula for the delay time (t = C × 25,000/3.5) depends only on
> > > > the capacity size.
> > > Why not just have the user specify the capacitance of the capacitor on
> > > the rail which they can directly read from the schematic rather than
> > > forcing them to do the calcualtion?  That seems a bit clearer and more
> > > user friendly (plus if someone decides the spec was wrong it's easier to
> > > roll out fixes).
> > The exact capacitance may not be known or vary above the nominal value
> > because of cheap components, and the formula from the datasheet is just
> > a guideline.
> 
> That variability is going to apply just as much to the charge time
> calculations/measurements as it is to the initial capacitance value -
> the results are going to be very much garbage in, garbage out.

True.

> > I'd expect the usual method to set this delay to be semi-empirical:
> > "start from the value calculated from datasheet and schematics and then
> > increase until no more audio artifacts on a representative sample of
> > boards".
> > I think it is be better to specify a delay that works than a bogus
> > capacitance value that happens to correspond to a delay that works.
> 
> If this is varying so drastically per board/system that it's relevant
> then we're already into problematic territory.  For most devices we just
> have a number for the part, not something that varies so wildly that
> each system needs to configure it.

I'm not arguing this should be configured per individual device, just
per board DT.

It's just that my experience of one datapoint consists of a board where
I had to increase the delay to about three times the delay calculated
from the nominal capacitance according to the datasheet before audible
artifacts disappeared reliably on multiple devices.
Putting the extended delay into the device tree with a comment
explaining its empirical nature seemed more straightforward than putting
a bogus capacitance value, that differs from the schematics, and that I
have never measured.

Also, by using the capacitance we are guaranteed to end up with a codec
specific property name (adi,vmid-decoupling-capacitance ?) as opposed to
the possibility of defining a common delay property that could be used
for different codecs.

regards
Philipp
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply	other threads:[~2018-06-05  9:58 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-17 13:30 [PATCH] ASoC: ssm2602: Fix ADC powerup sequencing Marco Felsch
2018-05-17 16:14 ` Rob Herring
2018-05-18 10:46   ` Philipp Zabel
2018-05-23 16:53     ` Rob Herring
2018-05-25  9:47       ` Marco Felsch
2018-05-25 10:26         ` Mark Brown
2018-05-25 11:42           ` Marco Felsch
2018-05-25 14:52             ` Mark Brown
2018-05-25 15:18               ` Philipp Zabel
2018-05-25 17:24                 ` Mark Brown
2018-06-05  9:58                   ` Philipp Zabel [this message]
2018-06-05 16:06                     ` Mark Brown
2018-05-25 15:42         ` Rob Herring

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=1528192731.4074.7.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=m.felsch@pengutronix.de \
    --cc=mark.rutland@arm.com \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.