| Message ID | 1406519386-4902-9-git-send-email-emilio@elopez.com.ar |
|---|---|
| State | New |
| Headers | show |
Hi, First I suggest adding "DT: " to the commit line of all the DT patches. On Mon, Jul 28, 2014 at 11:49 AM, Emilio López <emilio@elopez.com.ar> wrote: > This commit adds all the mod1 clocks available on A20 to its device > tree. This list was created by looking at the A20 user manual. > > Not-signed-off-by: Emilio López <emilio@elopez.com.ar> > --- > > As mentioned on an earlier patch, note that this is untested, and I > only added them to sun7i. It'd be great if actual users of these clocks > could comment :) > > arch/arm/boot/dts/sun7i-a20.dtsi | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index 5d0265a..c57f7ad 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -335,6 +335,29 @@ > clock-output-names = "ir1"; > }; > > + iis0_clk: clk@01c200b8 { > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod1-clk"; > + reg = <0x01c200b8 0x4>; > + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; User manual says pll2x8, pll2x4, pll2x2, pll2 for all the the clocks. So the order here should be reversed. Same for the others. > + clock-output-names = "iis0"; > + }; > + > + ac97_clk: clk@01c200bc { > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod1-clk"; > + reg = <0x01c200bc 0x4>; > + clocks = <&pll2 3>, <&pll2 2>, <&pll2 1>, <&pll2 0>; > + clock-output-names = "ac97"; > + }; > + > + spdif_clk: clk@01c200c0 { > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod1-clk"; > + reg = <0x01c200c0 0x4>; > + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; > + clock-output-names = "spdif"; > + }; Newline. And the manuals don't have anything on the SPDIF clock, so I can't comment on it. > usb_clk: clk@01c200cc { > #clock-cells = <1>; > #reset-cells = <1>; > @@ -352,6 +375,22 @@ > clock-output-names = "spi3"; > }; > > + iis1_clk: clk@01c200d8 { > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod1-clk"; > + reg = <0x01c200d8 0x4>; > + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; > + clock-output-names = "iis1"; > + }; > + > + iis2_clk: clk@01c200dc { > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod1-clk"; > + reg = <0x01c200dc 0x4>; > + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; > + clock-output-names = "iis2"; > + }; > + > codec_clk: clk@01c20140 { > #clock-cells = <0>; > compatible = "allwinner,sun4i-a10-codec-clk"; > -- Apart from the title and the clock orders, everything looks good. Jon should test this with the above corrected. Cheers ChenYu
On Mon, Jul 28, 2014 at 12:49:46AM -0300, Emilio López wrote: > This commit adds all the mod1 clocks available on A20 to its device > tree. This list was created by looking at the A20 user manual. > > Not-signed-off-by: Emilio López <emilio@elopez.com.ar> > --- > > As mentioned on an earlier patch, note that this is untested, and I > only added them to sun7i. It'd be great if actual users of these clocks > could comment :) > > arch/arm/boot/dts/sun7i-a20.dtsi | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index 5d0265a..c57f7ad 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -335,6 +335,29 @@ > clock-output-names = "ir1"; > }; > > + iis0_clk: clk@01c200b8 { > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod1-clk"; > + reg = <0x01c200b8 0x4>; > + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; > + clock-output-names = "iis0"; Usually, it's called i2s. Otherwise, beside Chen-Yu's comment, it looks fine. Maxime
On Mon, Jul 28, 2014 at 9:25 AM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Jul 28, 2014 at 12:49:46AM -0300, Emilio López wrote: >> This commit adds all the mod1 clocks available on A20 to its device >> tree. This list was created by looking at the A20 user manual. >> >> Not-signed-off-by: Emilio López <emilio@elopez.com.ar> >> --- >> >> As mentioned on an earlier patch, note that this is untested, and I >> only added them to sun7i. It'd be great if actual users of these clocks >> could comment :) >> >> arch/arm/boot/dts/sun7i-a20.dtsi | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi >> index 5d0265a..c57f7ad 100644 >> --- a/arch/arm/boot/dts/sun7i-a20.dtsi >> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi >> @@ -335,6 +335,29 @@ >> clock-output-names = "ir1"; >> }; >> >> + iis0_clk: clk@01c200b8 { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun4i-a10-mod1-clk"; >> + reg = <0x01c200b8 0x4>; >> + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; >> + clock-output-names = "iis0"; > > Usually, it's called i2s. I'd preferred i2s, but Allwinner used iis everywhere in the manual. So which wins in the device tree? Same issue happens with twi vs i2c. > > Otherwise, beside Chen-Yu's comment, it looks fine. > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
On Mon, Jul 28, 2014 at 12:28 AM, Chen-Yu Tsai <wens@csie.org> wrote: > Hi, > > First I suggest adding "DT: " to the commit line of all the DT patches. > > On Mon, Jul 28, 2014 at 11:49 AM, Emilio López <emilio@elopez.com.ar> wrote: >> This commit adds all the mod1 clocks available on A20 to its device >> tree. This list was created by looking at the A20 user manual. >> >> Not-signed-off-by: Emilio López <emilio@elopez.com.ar> >> --- >> >> As mentioned on an earlier patch, note that this is untested, and I >> only added them to sun7i. It'd be great if actual users of these clocks >> could comment :) >> >> arch/arm/boot/dts/sun7i-a20.dtsi | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi >> index 5d0265a..c57f7ad 100644 >> --- a/arch/arm/boot/dts/sun7i-a20.dtsi >> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi >> @@ -335,6 +335,29 @@ >> clock-output-names = "ir1"; >> }; >> >> + iis0_clk: clk@01c200b8 { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun4i-a10-mod1-clk"; >> + reg = <0x01c200b8 0x4>; >> + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; > > User manual says pll2x8, pll2x4, pll2x2, pll2 for all the the clocks. > So the order here should be reversed. Same for the others. > >> + clock-output-names = "iis0"; >> + }; >> + >> + ac97_clk: clk@01c200bc { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun4i-a10-mod1-clk"; >> + reg = <0x01c200bc 0x4>; >> + clocks = <&pll2 3>, <&pll2 2>, <&pll2 1>, <&pll2 0>; >> + clock-output-names = "ac97"; >> + }; >> + >> + spdif_clk: clk@01c200c0 { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun4i-a10-mod1-clk"; >> + reg = <0x01c200c0 0x4>; >> + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; >> + clock-output-names = "spdif"; >> + }; > > Newline. And the manuals don't have anything on the SPDIF clock, > so I can't comment on it. We do have this much SPDIF doc. http://linux-sunxi.org/SPDIF Based on the Allwinner code I see no reason to think that the SPDIF clock varies from any of the other audio clocks. Who knows why they left SPDIF out of the manual. > >> usb_clk: clk@01c200cc { >> #clock-cells = <1>; >> #reset-cells = <1>; >> @@ -352,6 +375,22 @@ >> clock-output-names = "spi3"; >> }; >> >> + iis1_clk: clk@01c200d8 { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun4i-a10-mod1-clk"; >> + reg = <0x01c200d8 0x4>; >> + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; >> + clock-output-names = "iis1"; >> + }; >> + >> + iis2_clk: clk@01c200dc { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun4i-a10-mod1-clk"; >> + reg = <0x01c200dc 0x4>; >> + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; >> + clock-output-names = "iis2"; >> + }; >> + >> codec_clk: clk@01c20140 { >> #clock-cells = <0>; >> compatible = "allwinner,sun4i-a10-codec-clk"; >> -- > > Apart from the title and the clock orders, everything looks good. > Jon should test this with the above corrected. I don't have i2s hardware available yet. Maybe by end of the week. > > > Cheers > ChenYu
On Mon, Jul 28, 2014 at 10:47:16AM -0400, jonsmirl@gmail.com wrote: > On Mon, Jul 28, 2014 at 9:25 AM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Mon, Jul 28, 2014 at 12:49:46AM -0300, Emilio López wrote: > >> This commit adds all the mod1 clocks available on A20 to its device > >> tree. This list was created by looking at the A20 user manual. > >> > >> Not-signed-off-by: Emilio López <emilio@elopez.com.ar> > >> --- > >> > >> As mentioned on an earlier patch, note that this is untested, and I > >> only added them to sun7i. It'd be great if actual users of these clocks > >> could comment :) > >> > >> arch/arm/boot/dts/sun7i-a20.dtsi | 39 +++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 39 insertions(+) > >> > >> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > >> index 5d0265a..c57f7ad 100644 > >> --- a/arch/arm/boot/dts/sun7i-a20.dtsi > >> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > >> @@ -335,6 +335,29 @@ > >> clock-output-names = "ir1"; > >> }; > >> > >> + iis0_clk: clk@01c200b8 { > >> + #clock-cells = <0>; > >> + compatible = "allwinner,sun4i-a10-mod1-clk"; > >> + reg = <0x01c200b8 0x4>; > >> + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; > >> + clock-output-names = "iis0"; > > > > Usually, it's called i2s. > > I'd preferred i2s, but Allwinner used iis everywhere in the manual. So > which wins in the device tree? > > Same issue happens with twi vs i2c. I'd say whatever is the most common, this is why we ended up using i2c instead of allwinner's twi, pinctrl over port controller, etc. Maxime
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 5d0265a..c57f7ad 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -335,6 +335,29 @@ clock-output-names = "ir1"; }; + iis0_clk: clk@01c200b8 { + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod1-clk"; + reg = <0x01c200b8 0x4>; + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; + clock-output-names = "iis0"; + }; + + ac97_clk: clk@01c200bc { + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod1-clk"; + reg = <0x01c200bc 0x4>; + clocks = <&pll2 3>, <&pll2 2>, <&pll2 1>, <&pll2 0>; + clock-output-names = "ac97"; + }; + + spdif_clk: clk@01c200c0 { + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod1-clk"; + reg = <0x01c200c0 0x4>; + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; + clock-output-names = "spdif"; + }; usb_clk: clk@01c200cc { #clock-cells = <1>; #reset-cells = <1>; @@ -352,6 +375,22 @@ clock-output-names = "spi3"; }; + iis1_clk: clk@01c200d8 { + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod1-clk"; + reg = <0x01c200d8 0x4>; + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; + clock-output-names = "iis1"; + }; + + iis2_clk: clk@01c200dc { + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod1-clk"; + reg = <0x01c200dc 0x4>; + clocks = <&pll2 0>, <&pll2 1>, <&pll2 2>, <&pll2 3>; + clock-output-names = "iis2"; + }; + codec_clk: clk@01c20140 { #clock-cells = <0>; compatible = "allwinner,sun4i-a10-codec-clk";