tegra_wm8903 dapm_nc_pins

tegra_wm8903 dapm_nc_pins

[Julian Scheel]

Hi,

is there any special reason for the dapm_nc_pins being hardcoded into
tegra_wm8903.c instead of being set through the platform data?
We are currently making up a platform setting for our devices and would
like to have all relevant settings within the platform driver instead of
having to hack the tegra_wm8903-driver with further machine_is_XX()
checks.

So I would suggest to add an entry to the platform data which contains a
list of NC-pins which are then set to nc by the tegra_wm8903 driver on
startup.

Regards,
Julian

Re: tegra_wm8903 dapm_nc_pins

[Stephen Warren]

Julian Scheel wrote at Wednesday, October 19, 2011 9:29 AM:
> is there any special reason for the dapm_nc_pins being hardcoded into
> tegra_wm8903.c instead of being set through the platform data?
...
> So I would suggest to add an entry to the platform data which contains a
> list of NC-pins which are then set to nc by the tegra_wm8903 driver on
> startup.

I believe the idea was to keep as much as possible of the audio related
information in the audio drivers rather than in board files. The GPIO
IDs are in platform data since the audio driver can't reach into the
mach-tegra gpio_names header since it's not public.

What I'd like to see in the nc_pins case is the DAPM core automatically
perform these calls based on which pins the codec has which aren't
mentioned in card->dapm_routes; I haven't investigated whether that's
actually possible without breaking unrelated boards though.

If you were to move the nc_pins list into platform data, you'd end up
wanting to move the snd_soc_dapm_route tables too, and perhaps even
tegra_wm8903_controls[] and maybe more.

Eventually, I see most of these tables moving into device-tree, with
the only thing hard-coded into tegra_wm8903.c being all the clock and
format setup code.

-- 
nvpublic

Re: tegra_wm8903 dapm_nc_pins

[Mark Brown]

On Wed, Oct 19, 2011 at 09:12:46AM -0700, Stephen Warren wrote:

> I believe the idea was to keep as much as possible of the audio related
> information in the audio drivers rather than in board files. The GPIO
> IDs are in platform data since the audio driver can't reach into the
> mach-tegra gpio_names header since it's not public.

Well, it's also that the model the stack has is that all this stuff is
going to be in board files.

> What I'd like to see in the nc_pins case is the DAPM core automatically
> perform these calls based on which pins the codec has which aren't
> mentioned in card->dapm_routes; I haven't investigated whether that's
> actually possible without breaking unrelated boards though.

This wouldn't be too hard to do but needs to be entirely optional,
just wiring the CODEC driver in should be enough.  Keeping things
powered down is an optimisation, getting audio out is critical
functionality.

> If you were to move the nc_pins list into platform data, you'd end up
> wanting to move the snd_soc_dapm_route tables too, and perhaps even
> tegra_wm8903_controls[] and maybe more.

> Eventually, I see most of these tables moving into device-tree, with
> the only thing hard-coded into tegra_wm8903.c being all the clock and
> format setup code.

Until someone decides to connect MCLK somewhere else...

Re: tegra_wm8903 dapm_nc_pins

[Julian Scheel]

Am 19.10.2011 um 18:12 schrieb Stephen Warren:
> I believe the idea was to keep as much as possible of the audio related
> information in the audio drivers rather than in board files. The GPIO
> IDs are in platform data since the audio driver can't reach into the
> mach-tegra gpio_names header since it's not public.

Ok.

> What I'd like to see in the nc_pins case is the DAPM core automatically
> perform these calls based on which pins the codec has which aren't
> mentioned in card->dapm_routes; I haven't investigated whether that's
> actually possible without breaking unrelated boards though.

Well actually the current state can break unrelated boards as well.
For example the lineout lines are set to NC for any boards besides aebl.
This is the actual reason why I started to look at this.

> If you were to move the nc_pins list into platform data, you'd end up
> wanting to move the snd_soc_dapm_route tables too, and perhaps even
> tegra_wm8903_controls[] and maybe more.

Ok, agreed.

> Eventually, I see most of these tables moving into device-tree, with
> the only thing hard-coded into tegra_wm8903.c being all the clock and
> format setup code.

I am not aware how much work this would be, as I have not yet worked with
device tree at all. I would propose to change the init code to auto detection
based on the dapm routes for now. I should find the time to make up a patch,
if you agree.

Julian

Re: tegra_wm8903 dapm_nc_pins

[Stephen Warren]

Julian Scheel wrote at Wednesday, October 19, 2011 11:27 PM:
> Am 19.10.2011 um 18:12 schrieb Stephen Warren:
> > I believe the idea was to keep as much as possible of the audio related
> > information in the audio drivers rather than in board files. The GPIO
> > IDs are in platform data since the audio driver can't reach into the
> > mach-tegra gpio_names header since it's not public.
> 
> Ok.
> 
> > What I'd like to see in the nc_pins case is the DAPM core automatically
> > perform these calls based on which pins the codec has which aren't
> > mentioned in card->dapm_routes; I haven't investigated whether that's
> > actually possible without breaking unrelated boards though.
> 
> Well actually the current state can break unrelated boards as well.

By "unrelated", I meant boards other than Tegra boards; AFAIK, the current
Tegra driver isn't broken for any board that the current driver expects to
support.

...
> > Eventually, I see most of these tables moving into device-tree, with
> > the only thing hard-coded into tegra_wm8903.c being all the clock and
> > format setup code.
> 
> I am not aware how much work this would be, as I have not yet worked with
> device tree at all. I would propose to change the init code to auto detection
> based on the dapm routes for now. I should find the time to make up a patch,
> if you agree.

I believe a secretlab employee was working on DT bindings for the Tegra+WM8903
machine driver, although I haven't seen any updates on this front recently.

By all means make the existing driver calculate the nc pins automatically.
I suspect the implementation should probably be in the ASoC core, and Mark
pointed out it should be optional so as not to break any other (non-Tegra
WM8903) boards.

-- 
nvpublic