diff mbox series

[06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

Message ID 20201213183759.223246-7-aford173@gmail.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series arm64: dts: renesas: Cleanup Beacon Kit and support more SoC's | expand

Commit Message

Adam Ford Dec. 13, 2020, 6:37 p.m. UTC
With the newly added configurable clock options, the audio CODEC can
configure the mclk automatically.  Add the reference to the versaclock.
Since the devices on I2C5 can communicate at 400KHz, let's also increase
that too

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Dec. 17, 2020, 11:11 a.m. UTC | #1
Hi Adam,

CC alsa-devel

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> With the newly added configurable clock options, the audio CODEC can
> configure the mclk automatically.  Add the reference to the versaclock.
> Since the devices on I2C5 can communicate at 400KHz, let's also increase
> that too
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -424,13 +424,15 @@ &i2c0 {
>
>  &i2c5 {
>         status = "okay";
> -       clock-frequency = <100000>;
> +       clock-frequency = <400000>;
>         pinctrl-0 = <&i2c5_pins>;
>         pinctrl-names = "default";
>
>         codec: wm8962@1a {
>                 compatible = "wlf,wm8962";
>                 reg = <0x1a>;
> +               clocks = <&versaclock6_bb 3>;
> +               clock-names = "mclk";

While the driver does get the (nameless) clock, the DT bindings lack any
mention of a clocks property.  It would be good to update the bindings.

Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
hand) use "xclk" instead of "mclk"?

>                 DCVDD-supply = <&reg_audio>;
>                 DBVDD-supply = <&reg_audio>;
>                 AVDD-supply = <&reg_audio>;

Gr{oetje,eeting}s,

                        Geert
Adam Ford Dec. 17, 2020, 1:33 p.m. UTC | #2
On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> CC alsa-devel
>
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > With the newly added configurable clock options, the audio CODEC can
> > configure the mclk automatically.  Add the reference to the versaclock.
> > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > that too
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > @@ -424,13 +424,15 @@ &i2c0 {
> >
> >  &i2c5 {
> >         status = "okay";
> > -       clock-frequency = <100000>;
> > +       clock-frequency = <400000>;
> >         pinctrl-0 = <&i2c5_pins>;
> >         pinctrl-names = "default";
> >
> >         codec: wm8962@1a {
> >                 compatible = "wlf,wm8962";
> >                 reg = <0x1a>;
> > +               clocks = <&versaclock6_bb 3>;
> > +               clock-names = "mclk";
>
> While the driver does get the (nameless) clock, the DT bindings lack any
> mention of a clocks property.  It would be good to update the bindings.

Agreed.  I'll push an update to add the clocks property.

>
> Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> hand) use "xclk" instead of "mclk"?

On the schematics for the two imx boards, it's labeled as xclk, so it
was named as such.  For this board, the schematic names it mclk. The
driver doesn't care about the clock-names property, so I'll just
remove them.

adam
>
> >                 DCVDD-supply = <&reg_audio>;
> >                 DBVDD-supply = <&reg_audio>;
> >                 AVDD-supply = <&reg_audio>;
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Adam Ford Dec. 17, 2020, 5:58 p.m. UTC | #3
On Thu, Dec 17, 2020 at 7:33 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Adam,
> >
> > CC alsa-devel
> >
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > With the newly added configurable clock options, the audio CODEC can
> > > configure the mclk automatically.  Add the reference to the versaclock.
> > > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > > that too
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -424,13 +424,15 @@ &i2c0 {
> > >
> > >  &i2c5 {
> > >         status = "okay";
> > > -       clock-frequency = <100000>;
> > > +       clock-frequency = <400000>;
> > >         pinctrl-0 = <&i2c5_pins>;
> > >         pinctrl-names = "default";
> > >
> > >         codec: wm8962@1a {
> > >                 compatible = "wlf,wm8962";
> > >                 reg = <0x1a>;
> > > +               clocks = <&versaclock6_bb 3>;
> > > +               clock-names = "mclk";
> >
> > While the driver does get the (nameless) clock, the DT bindings lack any
> > mention of a clocks property.  It would be good to update the bindings.
>
> Agreed.  I'll push an update to add the clocks property.
>

I pushed a change to add the optional clock information to the
bindings txt file [1].
> >
> > Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> > hand) use "xclk" instead of "mclk"?
>
> On the schematics for the two imx boards, it's labeled as xclk, so it
> was named as such.  For this board, the schematic names it mclk. The
> driver doesn't care about the clock-names property, so I'll just
> remove them.

I pushed patches to remove these nodes from the other boards [2].
I'll remove them if V2 of the patch series for the Renesas board.

adam
[1] - https://patchwork.kernel.org/project/alsa-devel/patch/20201217162740.1452000-1-aford173@gmail.com/
[2] - https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=403739

>
> adam
> >
> > >                 DCVDD-supply = <&reg_audio>;
> > >                 DBVDD-supply = <&reg_audio>;
> > >                 AVDD-supply = <&reg_audio>;
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> >
> > In personal conversations with technical people, I call myself a hacker. But
> > when I'm talking to journalists I just say "programmer" or something like that.
> >                                 -- Linus Torvalds
Geert Uytterhoeven Dec. 18, 2020, 12:57 p.m. UTC | #4
Hi Adam,

On Thu, Dec 17, 2020 at 2:33 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > CC alsa-devel
> >
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > With the newly added configurable clock options, the audio CODEC can
> > > configure the mclk automatically.  Add the reference to the versaclock.
> > > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > > that too
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -424,13 +424,15 @@ &i2c0 {
> > >
> > >  &i2c5 {
> > >         status = "okay";
> > > -       clock-frequency = <100000>;
> > > +       clock-frequency = <400000>;
> > >         pinctrl-0 = <&i2c5_pins>;
> > >         pinctrl-names = "default";
> > >
> > >         codec: wm8962@1a {
> > >                 compatible = "wlf,wm8962";
> > >                 reg = <0x1a>;
> > > +               clocks = <&versaclock6_bb 3>;
> > > +               clock-names = "mclk";
> >
> > While the driver does get the (nameless) clock, the DT bindings lack any
> > mention of a clocks property.  It would be good to update the bindings.
>
> Agreed.  I'll push an update to add the clocks property.

Thanks!

> > Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> > hand) use "xclk" instead of "mclk"?
>
> On the schematics for the two imx boards, it's labeled as xclk, so it
> was named as such.  For this board, the schematic names it mclk. The
> driver doesn't care about the clock-names property, so I'll just
> remove them.

If there's a single clock, not using clock-names is fine.
If you do use clock-names, the names should be clock-centric, not
board-centric.

BTW, looking at the WM8962 datasheet, it's called "MCLK".

Gr{oetje,eeting}s,

                        Geert
Adam Ford Dec. 18, 2020, 2:23 p.m. UTC | #5
On Fri, Dec 18, 2020 at 6:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Thu, Dec 17, 2020 at 2:33 PM Adam Ford <aford173@gmail.com> wrote:
> > On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > CC alsa-devel
> > >
> > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > With the newly added configurable clock options, the audio CODEC can
> > > > configure the mclk automatically.  Add the reference to the versaclock.
> > > > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > > > that too
> > > >
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > @@ -424,13 +424,15 @@ &i2c0 {
> > > >
> > > >  &i2c5 {
> > > >         status = "okay";
> > > > -       clock-frequency = <100000>;
> > > > +       clock-frequency = <400000>;
> > > >         pinctrl-0 = <&i2c5_pins>;
> > > >         pinctrl-names = "default";
> > > >
> > > >         codec: wm8962@1a {
> > > >                 compatible = "wlf,wm8962";
> > > >                 reg = <0x1a>;
> > > > +               clocks = <&versaclock6_bb 3>;
> > > > +               clock-names = "mclk";
> > >
> > > While the driver does get the (nameless) clock, the DT bindings lack any
> > > mention of a clocks property.  It would be good to update the bindings.
> >
> > Agreed.  I'll push an update to add the clocks property.
>
> Thanks!
>
> > > Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> > > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> > > hand) use "xclk" instead of "mclk"?
> >
> > On the schematics for the two imx boards, it's labeled as xclk, so it
> > was named as such.  For this board, the schematic names it mclk. The
> > driver doesn't care about the clock-names property, so I'll just
> > remove them.
>
> If there's a single clock, not using clock-names is fine.
> If you do use clock-names, the names should be clock-centric, not
> board-centric.

I already submitted patches to remove the clock-names reference from
the other two boards you noted [1].  I agree it should match  the
driver and not the schematic.  That line was a left-over from our
internal git repo where the decision was used to follow the schematic
and not the driver.

Thanks for bringing that to my attention.

adam

[1] - https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=403739

>
> BTW, looking at the WM8962 datasheet, it's called "MCLK".
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index ee7809e8db07..130993b1b20a 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -424,13 +424,15 @@  &i2c0 {
 
 &i2c5 {
 	status = "okay";
-	clock-frequency = <100000>;
+	clock-frequency = <400000>;
 	pinctrl-0 = <&i2c5_pins>;
 	pinctrl-names = "default";
 
 	codec: wm8962@1a {
 		compatible = "wlf,wm8962";
 		reg = <0x1a>;
+		clocks = <&versaclock6_bb 3>;
+		clock-names = "mclk";
 		DCVDD-supply = <&reg_audio>;
 		DBVDD-supply = <&reg_audio>;
 		AVDD-supply = <&reg_audio>;