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

Groups > linux.kernel > #1319042 > unrolled thread

Re: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio card

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: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio  card Mark Brown <broonie@kernel.org> - 2016-01-27 16:00 +0100
    Re: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio  card Damien Horsley <Damien.Horsley@imgtec.com> - 2016-01-27 16:20 +0100
      Re: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio  card Mark Brown <broonie@kernel.org> - 2016-01-27 17:10 +0100
        Re: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio  card Damien Horsley <Damien.Horsley@imgtec.com> - 2016-01-27 18:20 +0100
          Re: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio  card Mark Brown <broonie@kernel.org> - 2016-01-27 21:20 +0100

#1319042 — Re: [RFC V2 1/2] ASoC: img: Add binding document for Pistachio audio card

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

On Tue, Jan 26, 2016 at 02:34:26PM +0000, Damien Horsley wrote:

> +  - clock-names : Includes the following entries:
> +        "audio_pll"  The audio PLL
> +        "i2s_mclk"   The i2s reference clock
> +                     Also connected to i2s_out_0_mclk output
> +        "dac_mclk"   Dac reference clock. Connected to i2s_dac_clk output

Why are these (especially the dac_mclk and i2s_mclk) properties of the
card and not of the drivers for the respective devices?

> +  - img,mute-gpio : phandle of the mute gpio
> +
> +  - img,hp-det-gpio : phandle of the headphone detect gpio

The DT maintainers would prefer those to be named -gpios.

[toc] | [next] | [standalone]

#1319059

On 27/01/16 14:57, Mark Brown wrote:
> On Tue, Jan 26, 2016 at 02:34:26PM +0000, Damien Horsley wrote:
> 
>> +  - clock-names : Includes the following entries:
>> +        "audio_pll"  The audio PLL
>> +        "i2s_mclk"   The i2s reference clock
>> +                     Also connected to i2s_out_0_mclk output
>> +        "dac_mclk"   Dac reference clock. Connected to i2s_dac_clk output
> 
> Why are these (especially the dac_mclk and i2s_mclk) properties of the
> card and not of the drivers for the respective devices?
> 

Due to the shared nature of these clocks. Individual components cannot
be responsible for controlling these as this could break configurations
for other components that are sharing the clocks. Only the card driver
has visibility of all of the components and their requirements.

audio_pll is used by spdif out, parallel out, i2s out, and i2s in if
there are codecs on the i2s in path that make use of i2s_mclk and
dac_mclk (derived from audio_pll)

i2s_mclk and dac_mclk can be used by both the i2s in and i2s out paths
on some boards

>> +  - img,mute-gpio : phandle of the mute gpio
>> +
>> +  - img,hp-det-gpio : phandle of the headphone detect gpio
> 
> The DT maintainers would prefer those to be named -gpios.
> 

Ok

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

#1319113

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

On Wed, Jan 27, 2016 at 03:18:20PM +0000, Damien Horsley wrote:
> On 27/01/16 14:57, Mark Brown wrote:
> > On Tue, Jan 26, 2016 at 02:34:26PM +0000, Damien Horsley wrote:

> >> +  - clock-names : Includes the following entries:
> >> +        "audio_pll"  The audio PLL
> >> +        "i2s_mclk"   The i2s reference clock
> >> +                     Also connected to i2s_out_0_mclk output
> >> +        "dac_mclk"   Dac reference clock. Connected to i2s_dac_clk output

> > Why are these (especially the dac_mclk and i2s_mclk) properties of the
> > card and not of the drivers for the respective devices?

> Due to the shared nature of these clocks. Individual components cannot
> be responsible for controlling these as this could break configurations
> for other components that are sharing the clocks. Only the card driver
> has visibility of all of the components and their requirements.

You're talking about the code that decides what rates to set the clock
at, not where the properties are placed in the DT.

> i2s_mclk and dac_mclk can be used by both the i2s in and i2s out paths
> on some boards

Multiple devices can reference the same clock.

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

#1319165

On 27/01/16 16:00, Mark Brown wrote:
> On Wed, Jan 27, 2016 at 03:18:20PM +0000, Damien Horsley wrote:
>> On 27/01/16 14:57, Mark Brown wrote:
>>> On Tue, Jan 26, 2016 at 02:34:26PM +0000, Damien Horsley wrote:
> 
>>>> +  - clock-names : Includes the following entries:
>>>> +        "audio_pll"  The audio PLL
>>>> +        "i2s_mclk"   The i2s reference clock
>>>> +                     Also connected to i2s_out_0_mclk output
>>>> +        "dac_mclk"   Dac reference clock. Connected to i2s_dac_clk output
> 
>>> Why are these (especially the dac_mclk and i2s_mclk) properties of the
>>> card and not of the drivers for the respective devices?
> 
>> Due to the shared nature of these clocks. Individual components cannot
>> be responsible for controlling these as this could break configurations
>> for other components that are sharing the clocks. Only the card driver
>> has visibility of all of the components and their requirements.
> 
> You're talking about the code that decides what rates to set the clock
> at, not where the properties are placed in the DT.
> 
>> i2s_mclk and dac_mclk can be used by both the i2s in and i2s out paths
>> on some boards
> 
> Multiple devices can reference the same clock.
> 

audio_pll is referenced exclusively by the card device

i2s_mclk and dac_mclk can also be referenced by other devices. The i2s
out controller references i2s_mclk, and codec devices can reference
i2s_mclk/dac_mclk dependent on their system clock requirements

without a reference to i2s_mclk and dac_mclk in the card driver, it
would not be possible to control the divisors and gates for these clocks
in the following cases:

Simplistic codecs that do not have drivers

Codec drivers that do not call clk_set_rate and clk_enable/clk_disable

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

#1319650

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

On Wed, Jan 27, 2016 at 05:13:09PM +0000, Damien Horsley wrote:

> audio_pll is referenced exclusively by the card device

That one *may* be plausible.

> i2s_mclk and dac_mclk can also be referenced by other devices. The i2s
> out controller references i2s_mclk, and codec devices can reference
> i2s_mclk/dac_mclk dependent on their system clock requirements

The clock API copes perfectly happily with this.

> without a reference to i2s_mclk and dac_mclk in the card driver, it
> would not be possible to control the divisors and gates for these clocks
> in the following cases:

> Simplistic codecs that do not have drivers

> Codec drivers that do not call clk_set_rate and clk_enable/clk_disable

Nonsense, if there is no driver or the driver doesn't do what you want
then fix that.  Don't bodge things at the wrong abstraction layer, that
just creates problems later on when someone comes along and does things
properly or tries to use the device tree outside of your particular
implementation (eg, when working with a differnet OS).

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web