Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1343718 > unrolled thread

Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-26 02:40 +0100
    Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Mark Brown <broonie@kernel.org> - 2016-02-26 03:40 +0100
      Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-26 22:10 +0100
        Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Mark Brown <broonie@kernel.org> - 2016-02-27 03:10 +0100

#1343718 — Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec

Mark Brown <broonie@kernel.org> writes:

>> > It will eumerate the AC'97 bus by itself and does not need the CODEC to
>> > be described.
>
>> I think I still don't get it.
>
>> So let's rephrase it another way : how will the function wm9713_probe() be
>> called, ie. what is the possible function backtrace leading to that call ?
>
> It will not be called, the generic AC'97 code will be used.

Ok, if it's not called no code in sound/soc/codecs/wm9713.c will be used, right
?
In that case wm9713_set_dai_clkdiv() will never be used, nor will the
wm9713_audio_map or wm9713_dapm_widgets be created, which will break all
userspace programs relying on these mixers and DAPM routes.

Or am I missing something ?

>> Do you have a devicetree example somewhere, with (ac97 host, audio codec) pair I
>> can have a look at to understand ?
>
> Some Atmel boards do this IIRC, as does the AACI driver (via AMBA but
> same effect).
I suppose you mean sound/arm/aaci.c, which is more a platform_data like driver
(if I understood the integrator code correctly).

I suppose we can achieve comparable result with sound/arm/pxa2xx-ac97.c, but as
to know if the functionality will be comparable to sound/soc/pxa/pxa2xx-ac97.c,
it's hard to say. If I count the DMA requestors, I see 5 in the sound/soc
version, and 2 in sound/arm.

That makes me believe the sound/arm version is inferior.

Cheers.

-- 
Robert

[toc] | [next] | [standalone]

#1343736

[Multipart message — attachments visible in raw view] — view raw

On Fri, Feb 26, 2016 at 02:33:49AM +0100, Robert Jarzmik wrote:
> Mark Brown <broonie@kernel.org> writes:

> > It will not be called, the generic AC'97 code will be used.

> Ok, if it's not called no code in sound/soc/codecs/wm9713.c will be used, right
> ?
> In that case wm9713_set_dai_clkdiv() will never be used, nor will the
> wm9713_audio_map or wm9713_dapm_widgets be created, which will break all
> userspace programs relying on these mixers and DAPM routes.

> Or am I missing something ?

No, you're not missing anything - that'll be what happens.  If you need
to preserve the userspace ABI on your board you'd need a much bigger
(but very welcome) refactoring of the AC'97 code to be less hacky and
use the device model more directly, or at least define a generic AC'97
binding somehow.  All the AC'97 support has never really been properly
moved over to the device model and unfortunately nobody's yet cared
about it for device tree except in the simple cases supported by the
generic AC'97 code.  I appreciate that this isn't very helpful, it's
an unfortunate consequence of DT as an ABI.

We probably want to end up with something like what the Intel folks have
been doing recently for HDA to get that working within ASoC.

[toc] | [prev] | [next] | [standalone]

#1344683

Mark Brown <broonie@kernel.org> writes:

> On Fri, Feb 26, 2016 at 02:33:49AM +0100, Robert Jarzmik wrote:
>> Mark Brown <broonie@kernel.org> writes:
>
>> > It will not be called, the generic AC'97 code will be used.
>
>> Ok, if it's not called no code in sound/soc/codecs/wm9713.c will be used, right
>> ?
>> In that case wm9713_set_dai_clkdiv() will never be used, nor will the
>> wm9713_audio_map or wm9713_dapm_widgets be created, which will break all
>> userspace programs relying on these mixers and DAPM routes.
>
>> Or am I missing something ?
>
> No, you're not missing anything - that'll be what happens.  If you need
> to preserve the userspace ABI on your board you'd need a much bigger
> (but very welcome) refactoring of the AC'97 code to be less hacky and
> use the device model more directly, or at least define a generic AC'97
> binding somehow.  All the AC'97 support has never really been properly
> moved over to the device model and unfortunately nobody's yet cared
> about it for device tree except in the simple cases supported by the
> generic AC'97 code.  I appreciate that this isn't very helpful, it's
> an unfortunate consequence of DT as an ABI.
>
> We probably want to end up with something like what the Intel folks have
> been doing recently for HDA to get that working within ASoC.

Ok, let me think about it and propose something, an approach.

I must admit I like the structure I saw in drivers/amba/bus.c, ie. to have
something like :
 - a bus driver core (sound/ac97/bus.c ?)
   Split between this and sound/pci/ac97_codec.c I don't know yet.
   => an instance of this bus will be instanciated from each snd_ac97_bus() call
      just as now
   => bus_register(&ac97_bustype)
   => ac97_bus_probe/remove(), match/uevent/dev_attrs
   => ac97_driver_register(struct ac97_driver *drv)

 - a ac97 driver structure (struct ac97_driver) with :
   => u32 vendor_id (vendor_id = lambda(vendor_id1, vendor_id2))
   => u32 vendor_id_mask (mask of bits to match)
   ...

 - each ac97 controller will call snd_ac97_bus().
   In my case, that's sound/arm/pxa2xx-ac97.c or sound/soc/pxa2xx-ac97.c,
   whatever.

 - each ac97 codec will subscribe to the bus
   ac97_driver_register(struct ac97_driver *drv, u32 vendor_id, u32 vendor_mask)
   For example wm9713.c will call :
     ac97_driver_register(drv, AC97_VENDOR(0x...., 0x....), 0xffffffff);

Well, if I'm totally mistaken, tell me. If not it will take me a bit of time to
write is down properly in a Documentation/ file.

Cheers.

-- 
Robert

[toc] | [prev] | [next] | [standalone]

#1344884

[Multipart message — attachments visible in raw view] — view raw

On Fri, Feb 26, 2016 at 10:04:15PM +0100, Robert Jarzmik wrote:

> Ok, let me think about it and propose something, an approach.

> I must admit I like the structure I saw in drivers/amba/bus.c, ie. to have
> something like :

...

> Well, if I'm totally mistaken, tell me. If not it will take me a bit of time to
> write is down properly in a Documentation/ file.

That seems pretty reasonable at first pass I think, the main issue is
just managing the transition safely for all the users.  There's a lot of
fragility in the AC'97 hardware and software so people have been very
reluctant to touch it.  Equally I think a lot of the more problematic
users probably aren't in use any more so you can probably get away with
more now than would have been the case in the past.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web