Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hi Mark Brown, On 03/20/2014 09:47 PM, Mark Brown wrote: On Thu, Mar 20, 2014 at 10:37:53AM +0800, Bo Shen wrote: For this, in my mind, I think we need add following parameters in DT. 1. sysclk_src_from_fll --> we need do nothing. No, how would this work? If nothing else the FLL needs configuration. Only configure it in machine driver. Then no DT operation. 2. sysclk_src_from_mclk 2.1 use_external_xtal --> we need clock_frequency 2.2 !use_external_xtal --> we need retrieve clock and clock_frequency. No, this is all handled in the clock binding. If there's a fixed rate clock the device tree should have a fixed rate clock provided. Does this acceptable? Or any other better suggestion for this? Just have the device tree describe the hardware and provide a way of specifying an optional MCLK. I will implement an RFC patch, please help review it. Thanks. Best Regards, Bo Shen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Thu, Mar 20, 2014 at 10:37:53AM +0800, Bo Shen wrote: > For this, in my mind, I think we need add following parameters in DT. > 1. sysclk_src_from_fll --> we need do nothing. No, how would this work? If nothing else the FLL needs configuration. > 2. sysclk_src_from_mclk >2.1 use_external_xtal --> we need clock_frequency >2.2 !use_external_xtal --> we need retrieve clock and clock_frequency. No, this is all handled in the clock binding. If there's a fixed rate clock the device tree should have a fixed rate clock provided. > Does this acceptable? Or any other better suggestion for this? Just have the device tree describe the hardware and provide a way of specifying an optional MCLK. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Thu, Mar 20, 2014 at 10:37:53AM +0800, Bo Shen wrote: For this, in my mind, I think we need add following parameters in DT. 1. sysclk_src_from_fll -- we need do nothing. No, how would this work? If nothing else the FLL needs configuration. 2. sysclk_src_from_mclk 2.1 use_external_xtal -- we need clock_frequency 2.2 !use_external_xtal -- we need retrieve clock and clock_frequency. No, this is all handled in the clock binding. If there's a fixed rate clock the device tree should have a fixed rate clock provided. Does this acceptable? Or any other better suggestion for this? Just have the device tree describe the hardware and provide a way of specifying an optional MCLK. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hi Mark Brown, On 03/20/2014 09:47 PM, Mark Brown wrote: On Thu, Mar 20, 2014 at 10:37:53AM +0800, Bo Shen wrote: For this, in my mind, I think we need add following parameters in DT. 1. sysclk_src_from_fll -- we need do nothing. No, how would this work? If nothing else the FLL needs configuration. Only configure it in machine driver. Then no DT operation. 2. sysclk_src_from_mclk 2.1 use_external_xtal -- we need clock_frequency 2.2 !use_external_xtal -- we need retrieve clock and clock_frequency. No, this is all handled in the clock binding. If there's a fixed rate clock the device tree should have a fixed rate clock provided. Does this acceptable? Or any other better suggestion for this? Just have the device tree describe the hardware and provide a way of specifying an optional MCLK. I will implement an RFC patch, please help review it. Thanks. Best Regards, Bo Shen -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hi Mark,
On 03/19/2014 06:28 PM, Mark Brown wrote:
On Wed, Mar 19, 2014 at 01:57:23PM +0800, Bo Shen wrote:
On 03/17/2014 07:55 PM, Mark Brown wrote:
If this is a clock for the CODEC it should be documented as part of the
binding for the CODEC and connected to the CODEC in the device tree
rather than being part of a machine driver binding.
This is a optional clock for CODEC which depends on hardware design. There
are 3 options for this clock, wm8904 as an example.
1. Using internal FLL, so won't use this clock.
2. Using external oscillator, no need to retrieve this clock.
3. Using SoC provide this clock (we use this case).
After considering these 3 options, if we put this into CODEC driver to do
it, I think it will be more complicate to do logic judgement. Do you think
so?
There shouldn't be any meaningful complexity from the above cases -
cases 2 and 3 are the same and if the clock isn't used at all then it
can be omitted. If the FLL is clocked from MCLK then the CODEC driver
should be able to work out how to configure it easily, the device isn't
like a digital hub CODEC with lots of clocking options.
For this, in my mind, I think we need add following parameters in DT.
1. sysclk_src_from_fll --> we need do nothing.
2. sysclk_src_from_mclk
2.1 use_external_xtal --> we need clock_frequency
2.2 !use_external_xtal --> we need retrieve clock and clock_frequency.
So, the dt may looks like:
for case 1:
wm8904: wm8904@1a {
reg = <0x1a>;
sysclk_src_from_fll;
}
for case 2.1:
wm8904: wm8904@1a {
reg = <0x1a>;
sysclk_src_from_mclk;
use_external_xtal;
clock_frequency = 1200;
}
for case 2.2:
wm8904: wm8904@1a {
reg = <0x1a>;
sysclk_src_from_mclk;
clocks = <>;
clock-names = "mclk";
clock_frequency = 32768;
}
Does this acceptable? Or any other better suggestion for this?
Best Regards,
Bo Shen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Wed, Mar 19, 2014 at 01:57:23PM +0800, Bo Shen wrote: > On 03/17/2014 07:55 PM, Mark Brown wrote: > >If this is a clock for the CODEC it should be documented as part of the > >binding for the CODEC and connected to the CODEC in the device tree > >rather than being part of a machine driver binding. > This is a optional clock for CODEC which depends on hardware design. There > are 3 options for this clock, wm8904 as an example. > 1. Using internal FLL, so won't use this clock. > 2. Using external oscillator, no need to retrieve this clock. > 3. Using SoC provide this clock (we use this case). > After considering these 3 options, if we put this into CODEC driver to do > it, I think it will be more complicate to do logic judgement. Do you think > so? There shouldn't be any meaningful complexity from the above cases - cases 2 and 3 are the same and if the clock isn't used at all then it can be omitted. If the FLL is clocked from MCLK then the CODEC driver should be able to work out how to configure it easily, the device isn't like a digital hub CODEC with lots of clocking options. > And, in machine driver, it will depends on the clock option to decide > whether to call snd_soc_dai_set_pll and snd_soc_dai_set_sysclk. Some if not all of this logic can be eaten by the CODEC driver, especially with a simple device like this. Realistically the device is either going to be clocked from MCLK (with the rate telling us if we need the FLL) or from BCLK (if it's slave and there's no MCLK). > And also the mentions the machine > drivers responsibility (one is for clocking) as following: > --->8--- > The ASoC machine (or board) driver is the code that glues together all the > component drivers (e.g. codecs, platforms and DAIs). It also describes the > relationships between each componnent which include audio paths, GPIOs, > interrupts, clocking, jacks and voltage regulators. > ---8<--- That's a Linux specific consideration which shouldn't play any part in DT design. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Wed, Mar 19, 2014 at 01:57:23PM +0800, Bo Shen wrote: On 03/17/2014 07:55 PM, Mark Brown wrote: If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. This is a optional clock for CODEC which depends on hardware design. There are 3 options for this clock, wm8904 as an example. 1. Using internal FLL, so won't use this clock. 2. Using external oscillator, no need to retrieve this clock. 3. Using SoC provide this clock (we use this case). After considering these 3 options, if we put this into CODEC driver to do it, I think it will be more complicate to do logic judgement. Do you think so? There shouldn't be any meaningful complexity from the above cases - cases 2 and 3 are the same and if the clock isn't used at all then it can be omitted. If the FLL is clocked from MCLK then the CODEC driver should be able to work out how to configure it easily, the device isn't like a digital hub CODEC with lots of clocking options. And, in machine driver, it will depends on the clock option to decide whether to call snd_soc_dai_set_pll and snd_soc_dai_set_sysclk. Some if not all of this logic can be eaten by the CODEC driver, especially with a simple device like this. Realistically the device is either going to be clocked from MCLK (with the rate telling us if we need the FLL) or from BCLK (if it's slave and there's no MCLK). And also the Documentation/soud/alsa/soc/machine.txt mentions the machine drivers responsibility (one is for clocking) as following: ---8--- The ASoC machine (or board) driver is the code that glues together all the component drivers (e.g. codecs, platforms and DAIs). It also describes the relationships between each componnent which include audio paths, GPIOs, interrupts, clocking, jacks and voltage regulators. ---8--- That's a Linux specific consideration which shouldn't play any part in DT design. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hi Mark,
On 03/19/2014 06:28 PM, Mark Brown wrote:
On Wed, Mar 19, 2014 at 01:57:23PM +0800, Bo Shen wrote:
On 03/17/2014 07:55 PM, Mark Brown wrote:
If this is a clock for the CODEC it should be documented as part of the
binding for the CODEC and connected to the CODEC in the device tree
rather than being part of a machine driver binding.
This is a optional clock for CODEC which depends on hardware design. There
are 3 options for this clock, wm8904 as an example.
1. Using internal FLL, so won't use this clock.
2. Using external oscillator, no need to retrieve this clock.
3. Using SoC provide this clock (we use this case).
After considering these 3 options, if we put this into CODEC driver to do
it, I think it will be more complicate to do logic judgement. Do you think
so?
There shouldn't be any meaningful complexity from the above cases -
cases 2 and 3 are the same and if the clock isn't used at all then it
can be omitted. If the FLL is clocked from MCLK then the CODEC driver
should be able to work out how to configure it easily, the device isn't
like a digital hub CODEC with lots of clocking options.
For this, in my mind, I think we need add following parameters in DT.
1. sysclk_src_from_fll -- we need do nothing.
2. sysclk_src_from_mclk
2.1 use_external_xtal -- we need clock_frequency
2.2 !use_external_xtal -- we need retrieve clock and clock_frequency.
So, the dt may looks like:
for case 1:
wm8904: wm8904@1a {
reg = 0x1a;
sysclk_src_from_fll;
}
for case 2.1:
wm8904: wm8904@1a {
reg = 0x1a;
sysclk_src_from_mclk;
use_external_xtal;
clock_frequency = 1200;
}
for case 2.2:
wm8904: wm8904@1a {
reg = 0x1a;
sysclk_src_from_mclk;
clocks = pck0;
clock-names = mclk;
clock_frequency = 32768;
}
Does this acceptable? Or any other better suggestion for this?
Best Regards,
Bo Shen
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hi Mark, On 03/17/2014 07:55 PM, Mark Brown wrote: On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: - compatible: "atmel,asoc-wm8904" - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain "pck0". If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. This is a optional clock for CODEC which depends on hardware design. There are 3 options for this clock, wm8904 as an example. 1. Using internal FLL, so won't use this clock. 2. Using external oscillator, no need to retrieve this clock. 3. Using SoC provide this clock (we use this case). After considering these 3 options, if we put this into CODEC driver to do it, I think it will be more complicate to do logic judgement. Do you think so? And, in machine driver, it will depends on the clock option to decide whether to call snd_soc_dai_set_pll and snd_soc_dai_set_sysclk. And also the mentions the machine drivers responsibility (one is for clocking) as following: --->8--- The ASoC machine (or board) driver is the code that glues together all the component drivers (e.g. codecs, platforms and DAIs). It also describes the relationships between each componnent which include audio paths, GPIOs, interrupts, clocking, jacks and voltage regulators. ---8<--- So, I think put this into machine driver will be better. Do you have any other idea? Or if I misunderstand something, please point it out. Thanks. Best Regards, Bo Shen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hi Mark, On 03/17/2014 07:55 PM, Mark Brown wrote: On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. This is a optional clock for CODEC which depends on hardware design. There are 3 options for this clock, wm8904 as an example. 1. Using internal FLL, so won't use this clock. 2. Using external oscillator, no need to retrieve this clock. 3. Using SoC provide this clock (we use this case). After considering these 3 options, if we put this into CODEC driver to do it, I think it will be more complicate to do logic judgement. Do you think so? And, in machine driver, it will depends on the clock option to decide whether to call snd_soc_dai_set_pll and snd_soc_dai_set_sysclk. And also the Documentation/soud/alsa/soc/machine.txt mentions the machine drivers responsibility (one is for clocking) as following: ---8--- The ASoC machine (or board) driver is the code that glues together all the component drivers (e.g. codecs, platforms and DAIs). It also describes the relationships between each componnent which include audio paths, GPIOs, interrupts, clocking, jacks and voltage regulators. ---8--- So, I think put this into machine driver will be better. Do you have any other idea? Or if I misunderstand something, please point it out. Thanks. Best Regards, Bo Shen -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 02:44:00PM +0100, Boris BREZILLON wrote: > Le 17/03/2014 12:55, Mark Brown a écrit : > >On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: > >If this is a clock for the CODEC it should be documented as part of the > >binding for the CODEC and connected to the CODEC in the device tree > >rather than being part of a machine driver binding. > Tell me if I'm mistaken, but doing this would implies a lot of changes. > The current wm8904 driver does not handle clk retrieval and configuration, > and I'm afraid that introducing these concepts would break other drivers > (those > connecting to a wm8904 codec). Currently the only mainline users of the device are the Atmel drivers. > How about adding CCF support as proposed in this series and think about a > cleaner > solution for a future release ? No, DT is an ABI so you can't do that - once something is in DT we're stuck supporting it so we can't do quick hacks so easily. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hello Mark, Le 17/03/2014 11:02, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON Signed-off-by: Boris BREZILLON Signed-off-by: Bo Shen --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: "atmel,asoc-wm8904" - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain "pck0". The word "driver" doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. Agreed. - clock-names: Should contain "pck0" After thinking a bit more about it, this can be any programmable clk (pckX). I'll fix it. Thanks. Best Regards, Boris Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hello Mark, Le 17/03/2014 12:55, Mark Brown a écrit : On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: - compatible: "atmel,asoc-wm8904" - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain "pck0". If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. Tell me if I'm mistaken, but doing this would implies a lot of changes. The current wm8904 driver does not handle clk retrieval and configuration, and I'm afraid that introducing these concepts would break other drivers (those connecting to a wm8904 codec). Do you see a simple alternative to this approach (or is there something I misunderstood in your suggestion) ? How about adding CCF support as proposed in this series and think about a cleaner solution for a future release ? Best Regards, Boris -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: >- compatible: "atmel,asoc-wm8904" >- atmel,model: The user-visible name of this sound complex. > + - clocks: A list of clocks needed by the wm8904 chip. > + - clock-output-names: Driver related clock names. Shall contain "pck0". If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 11:24:41AM +, Boris BREZILLON wrote: > Le 17/03/2014 11:48, Mark Rutland a écrit : > > On Mon, Mar 17, 2014 at 10:18:08AM +, boris brezillon wrote: > >> Hello Mark, > >> > >> Le 17/03/2014 11:02, Mark Rutland a écrit : > >>> On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: > From: Boris BREZILLON > > Signed-off-by: Boris BREZILLON > Signed-off-by: Bo Shen > --- > > Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > index 8bbe50c..aca341c 100644 > --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex > Required properties: > - compatible: "atmel,asoc-wm8904" > - atmel,model: The user-visible name of this sound complex. > + - clocks: A list of clocks needed by the wm8904 chip. > + - clock-output-names: Driver related clock names. Shall contain > "pck0". > >>> The word "driver" doesn't need to appear in biding documents, and this > >>> fails to describe what it sets out to. How about the following: > >>> > >>> - clocks: a list of phandle + clock-specifier pairs, one for each entry > >>> in clock-names. > >> Agreed. > >>> - clock-names: Should contain "pck0" > >> After thinking a bit more about it, this can be any programmable clk > >> (pckX). > > Huh? > > > > The clock-names property describes the names of the inputs to the > > device, from the view of the device, not the names of the clocks fed > > into those inputs. > > > > What are the names of the clock input lines on the wm8904? > > mclk, and you're right, this is how we should name the clk (I just got > influenced by the existing driver, which was requesting pck0). > > This gives the following: > > - clock-names: Should contain "mclk" > > Do you agree ? That sounds right to me. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Le 17/03/2014 11:48, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 10:18:08AM +, boris brezillon wrote: Hello Mark, Le 17/03/2014 11:02, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON Signed-off-by: Boris BREZILLON Signed-off-by: Bo Shen --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: "atmel,asoc-wm8904" - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain "pck0". The word "driver" doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. Agreed. - clock-names: Should contain "pck0" After thinking a bit more about it, this can be any programmable clk (pckX). Huh? The clock-names property describes the names of the inputs to the device, from the view of the device, not the names of the clocks fed into those inputs. What are the names of the clock input lines on the wm8904? mclk, and you're right, this is how we should name the clk (I just got influenced by the existing driver, which was requesting pck0). This gives the following: - clock-names: Should contain "mclk" Do you agree ? Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 10:18:08AM +, boris brezillon wrote: > Hello Mark, > > Le 17/03/2014 11:02, Mark Rutland a écrit : > > On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: > >> From: Boris BREZILLON > >> > >> Signed-off-by: Boris BREZILLON > >> Signed-off-by: Bo Shen > >> --- > >> > >> Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > >> b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > >> index 8bbe50c..aca341c 100644 > >> --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > >> +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > >> @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex > >> Required properties: > >> - compatible: "atmel,asoc-wm8904" > >> - atmel,model: The user-visible name of this sound complex. > >> + - clocks: A list of clocks needed by the wm8904 chip. > >> + - clock-output-names: Driver related clock names. Shall contain "pck0". > > The word "driver" doesn't need to appear in biding documents, and this > > fails to describe what it sets out to. How about the following: > > > > - clocks: a list of phandle + clock-specifier pairs, one for each entry > >in clock-names. > Agreed. > > - clock-names: Should contain "pck0" > After thinking a bit more about it, this can be any programmable clk (pckX). Huh? The clock-names property describes the names of the inputs to the device, from the view of the device, not the names of the clocks fed into those inputs. What are the names of the clock input lines on the wm8904? Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
From: Boris BREZILLON Signed-off-by: Boris BREZILLON Signed-off-by: Bo Shen --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: "atmel,asoc-wm8904" - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain "pck0". - atmel,audio-routing: A list of the connections between audio components. Each entry is a pair of strings, the first being the connection's sink, the second being the connection's source. Valid names for sources and -- 1.8.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: > From: Boris BREZILLON > > Signed-off-by: Boris BREZILLON > Signed-off-by: Bo Shen > --- > > Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > index 8bbe50c..aca341c 100644 > --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt > @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex > Required properties: >- compatible: "atmel,asoc-wm8904" >- atmel,model: The user-visible name of this sound complex. > + - clocks: A list of clocks needed by the wm8904 chip. > + - clock-output-names: Driver related clock names. Shall contain "pck0". The word "driver" doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. - clock-names: Should contain "pck0" Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On 03/17/2014 05:45 PM, Bo Shen wrote: From: Boris BREZILLON Signed-off-by: Boris BREZILLON Signed-off-by: Bo Shen Oh, sorry for my SOB. Please remove it when apply. --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: "atmel,asoc-wm8904" - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain "pck0". - atmel,audio-routing: A list of the connections between audio components. Each entry is a pair of strings, the first being the connection's sink, the second being the connection's source. Valid names for sources and Best Regards, Bo Shen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On 03/17/2014 05:45 PM, Bo Shen wrote: From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com Oh, sorry for my SOB. Please remove it when apply. --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. - atmel,audio-routing: A list of the connections between audio components. Each entry is a pair of strings, the first being the connection's sink, the second being the connection's source. Valid names for sources and Best Regards, Bo Shen -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. The word driver doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. - clock-names: Should contain pck0 Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. - atmel,audio-routing: A list of the connections between audio components. Each entry is a pair of strings, the first being the connection's sink, the second being the connection's source. Valid names for sources and -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 10:18:08AM +, boris brezillon wrote: Hello Mark, Le 17/03/2014 11:02, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. The word driver doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. Agreed. - clock-names: Should contain pck0 After thinking a bit more about it, this can be any programmable clk (pckX). Huh? The clock-names property describes the names of the inputs to the device, from the view of the device, not the names of the clocks fed into those inputs. What are the names of the clock input lines on the wm8904? Cheers, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Le 17/03/2014 11:48, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 10:18:08AM +, boris brezillon wrote: Hello Mark, Le 17/03/2014 11:02, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. The word driver doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. Agreed. - clock-names: Should contain pck0 After thinking a bit more about it, this can be any programmable clk (pckX). Huh? The clock-names property describes the names of the inputs to the device, from the view of the device, not the names of the clocks fed into those inputs. What are the names of the clock input lines on the wm8904? mclk, and you're right, this is how we should name the clk (I just got influenced by the existing driver, which was requesting pck0). This gives the following: - clock-names: Should contain mclk Do you agree ? Cheers, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 11:24:41AM +, Boris BREZILLON wrote: Le 17/03/2014 11:48, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 10:18:08AM +, boris brezillon wrote: Hello Mark, Le 17/03/2014 11:02, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. The word driver doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. Agreed. - clock-names: Should contain pck0 After thinking a bit more about it, this can be any programmable clk (pckX). Huh? The clock-names property describes the names of the inputs to the device, from the view of the device, not the names of the clocks fed into those inputs. What are the names of the clock input lines on the wm8904? mclk, and you're right, this is how we should name the clk (I just got influenced by the existing driver, which was requesting pck0). This gives the following: - clock-names: Should contain mclk Do you agree ? That sounds right to me. Cheers, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. signature.asc Description: Digital signature
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hello Mark, Le 17/03/2014 12:55, Mark Brown a écrit : On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. Tell me if I'm mistaken, but doing this would implies a lot of changes. The current wm8904 driver does not handle clk retrieval and configuration, and I'm afraid that introducing these concepts would break other drivers (those connecting to a wm8904 codec). Do you see a simple alternative to this approach (or is there something I misunderstood in your suggestion) ? How about adding CCF support as proposed in this series and think about a cleaner solution for a future release ? Best Regards, Boris -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
Hello Mark, Le 17/03/2014 11:02, Mark Rutland a écrit : On Mon, Mar 17, 2014 at 09:45:40AM +, Bo Shen wrote: From: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com Signed-off-by: Bo Shen voice.s...@atmel.com --- Documentation/devicetree/bindings/sound/atmel-wm8904.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt index 8bbe50c..aca341c 100644 --- a/Documentation/devicetree/bindings/sound/atmel-wm8904.txt +++ b/Documentation/devicetree/bindings/sound/atmel-wm8904.txt @@ -3,6 +3,8 @@ Atmel ASoC driver with wm8904 audio codec complex Required properties: - compatible: atmel,asoc-wm8904 - atmel,model: The user-visible name of this sound complex. + - clocks: A list of clocks needed by the wm8904 chip. + - clock-output-names: Driver related clock names. Shall contain pck0. The word driver doesn't need to appear in biding documents, and this fails to describe what it sets out to. How about the following: - clocks: a list of phandle + clock-specifier pairs, one for each entry in clock-names. Agreed. - clock-names: Should contain pck0 After thinking a bit more about it, this can be any programmable clk (pckX). I'll fix it. Thanks. Best Regards, Boris Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] ASoC: atmel: document clock properties of the wm8904 driver
On Mon, Mar 17, 2014 at 02:44:00PM +0100, Boris BREZILLON wrote: Le 17/03/2014 12:55, Mark Brown a écrit : On Mon, Mar 17, 2014 at 05:45:40PM +0800, Bo Shen wrote: If this is a clock for the CODEC it should be documented as part of the binding for the CODEC and connected to the CODEC in the device tree rather than being part of a machine driver binding. Tell me if I'm mistaken, but doing this would implies a lot of changes. The current wm8904 driver does not handle clk retrieval and configuration, and I'm afraid that introducing these concepts would break other drivers (those connecting to a wm8904 codec). Currently the only mainline users of the device are the Atmel drivers. How about adding CCF support as proposed in this series and think about a cleaner solution for a future release ? No, DT is an ABI so you can't do that - once something is in DT we're stuck supporting it so we can't do quick hacks so easily. signature.asc Description: Digital signature
