| Message ID | 20200907161141.31034-1-krzk@kernel.org |
|---|---|
| Headers | show |
| Series | ARM: dts: s5pv210: Cleanup - dtschema warnings | expand |
DMA still works for me on the Galaxy S. Tested-by: Jonathan Bakker <xc-racer2@live.ca> Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > There is no need to keep DMA controller nodes under AMBA bus node. > Remove the "amba" node to fix dtschema warnings like: > > amba: $nodename:0: 'amba' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210.dtsi | 49 +++++++++++++++------------------- > 1 file changed, 21 insertions(+), 28 deletions(-) > > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > index 1b0ee884e91d..84e4447931de 100644 > --- a/arch/arm/boot/dts/s5pv210.dtsi > +++ b/arch/arm/boot/dts/s5pv210.dtsi > @@ -128,35 +128,28 @@ > }; > }; > > - amba { > - #address-cells = <1>; > - #size-cells = <1>; > - compatible = "simple-bus"; > - ranges; > - > - pdma0: dma@e0900000 { > - compatible = "arm,pl330", "arm,primecell"; > - reg = <0xe0900000 0x1000>; > - interrupt-parent = <&vic0>; > - interrupts = <19>; > - clocks = <&clocks CLK_PDMA0>; > - clock-names = "apb_pclk"; > - #dma-cells = <1>; > - #dma-channels = <8>; > - #dma-requests = <32>; > - }; > + pdma0: dma@e0900000 { > + compatible = "arm,pl330", "arm,primecell"; > + reg = <0xe0900000 0x1000>; > + interrupt-parent = <&vic0>; > + interrupts = <19>; > + clocks = <&clocks CLK_PDMA0>; > + clock-names = "apb_pclk"; > + #dma-cells = <1>; > + #dma-channels = <8>; > + #dma-requests = <32>; > + }; > > - pdma1: dma@e0a00000 { > - compatible = "arm,pl330", "arm,primecell"; > - reg = <0xe0a00000 0x1000>; > - interrupt-parent = <&vic0>; > - interrupts = <20>; > - clocks = <&clocks CLK_PDMA1>; > - clock-names = "apb_pclk"; > - #dma-cells = <1>; > - #dma-channels = <8>; > - #dma-requests = <32>; > - }; > + pdma1: dma@e0a00000 { > + compatible = "arm,pl330", "arm,primecell"; > + reg = <0xe0a00000 0x1000>; > + interrupt-parent = <&vic0>; > + interrupts = <20>; > + clocks = <&clocks CLK_PDMA1>; > + clock-names = "apb_pclk"; > + #dma-cells = <1>; > + #dma-channels = <8>; > + #dma-requests = <32>; > }; > > adc: adc@e1700000 { >
Works for me on the Galaxy S. Tested-by: Jonathan Bakker <xc-racer2@live.ca> Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > The fixed clocks are kept under dedicated 'external-clocks' node, thus a > fake 'reg' was added. This is not correct with dtschema as fixed-clock > binding does not have a 'reg' property. Moving fixed clocks out of > 'soc' to root node fixes multiple dtbs_check warnings: > > external-clocks: $nodename:0: 'external-clocks' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > external-clocks: #size-cells:0:0: 0 is not one of [1, 2] > external-clocks: oscillator@0:reg:0: [0] is too short > external-clocks: oscillator@1:reg:0: [1] is too short > external-clocks: 'ranges' is a required property > oscillator@0: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210.dtsi | 36 +++++++++++++--------------------- > 1 file changed, 14 insertions(+), 22 deletions(-) > > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > index 84e4447931de..5c760a6d7955 100644 > --- a/arch/arm/boot/dts/s5pv210.dtsi > +++ b/arch/arm/boot/dts/s5pv210.dtsi > @@ -52,34 +52,26 @@ > }; > }; > > + xxti: oscillator-0 { > + compatible = "fixed-clock"; > + clock-frequency = <0>; > + clock-output-names = "xxti"; > + #clock-cells = <0>; > + }; > + > + xusbxti: oscillator-1 { > + compatible = "fixed-clock"; > + clock-frequency = <0>; > + clock-output-names = "xusbxti"; > + #clock-cells = <0>; > + }; > + > soc { > compatible = "simple-bus"; > #address-cells = <1>; > #size-cells = <1>; > ranges; > > - external-clocks { > - compatible = "simple-bus"; > - #address-cells = <1>; > - #size-cells = <0>; > - > - xxti: oscillator@0 { > - compatible = "fixed-clock"; > - reg = <0>; > - clock-frequency = <0>; > - clock-output-names = "xxti"; > - #clock-cells = <0>; > - }; > - > - xusbxti: oscillator@1 { > - compatible = "fixed-clock"; > - reg = <1>; > - clock-frequency = <0>; > - clock-output-names = "xusbxti"; > - #clock-cells = <0>; > - }; > - }; > - > onenand: onenand@b0600000 { > compatible = "samsung,s5pv210-onenand"; > reg = <0xb0600000 0x2000>, >
Works for me on the Galaxy S. Tested-by: Jonathan Bakker <xc-racer2@live.ca> Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > The Power Management Unit (PMU) is a separate device which has little > common with clock controller. Moving it to one level up (from clock > controller child to SoC) allows to remove fake simple-bus compatible and > dtbs_check warnings like: > > clock-controller@e0100000: $nodename:0: > 'clock-controller@e0100000' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210.dtsi | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > index 5c760a6d7955..46221a5c8ce5 100644 > --- a/arch/arm/boot/dts/s5pv210.dtsi > +++ b/arch/arm/boot/dts/s5pv210.dtsi > @@ -92,19 +92,16 @@ > }; > > clocks: clock-controller@e0100000 { > - compatible = "samsung,s5pv210-clock", "simple-bus"; > + compatible = "samsung,s5pv210-clock"; > reg = <0xe0100000 0x10000>; > clock-names = "xxti", "xusbxti"; > clocks = <&xxti>, <&xusbxti>; > #clock-cells = <1>; > - #address-cells = <1>; > - #size-cells = <1>; > - ranges; > + }; > > - pmu_syscon: syscon@e0108000 { > - compatible = "samsung-s5pv210-pmu", "syscon"; > - reg = <0xe0108000 0x8000>; > - }; > + pmu_syscon: syscon@e0108000 { > + compatible = "samsung-s5pv210-pmu", "syscon"; > + reg = <0xe0108000 0x8000>; > }; > > pinctrl0: pinctrl@e0200000 { >
Audio still works for me on the Galaxy S. Tested-by: Jonathan Bakker <xc-racer2@live.ca> Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > The 'audio-subsystem' node is an artificial creation, not representing > real hardware. The hardware is described by its nodes - AUDSS clock > controller and I2S0. > > Remove the 'audio-subsystem' node along with its undocumented compatible > to fix dtbs_check warnings like: > > audio-subsystem: $nodename:0: 'audio-subsystem' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210.dtsi | 65 +++++++++++++++------------------- > 1 file changed, 29 insertions(+), 36 deletions(-) > > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > index 46221a5c8ce5..2871351ab907 100644 > --- a/arch/arm/boot/dts/s5pv210.dtsi > +++ b/arch/arm/boot/dts/s5pv210.dtsi > @@ -223,43 +223,36 @@ > status = "disabled"; > }; > > - audio-subsystem { > - compatible = "samsung,s5pv210-audss", "simple-bus"; > - #address-cells = <1>; > - #size-cells = <1>; > - ranges; > - > - clk_audss: clock-controller@eee10000 { > - compatible = "samsung,s5pv210-audss-clock"; > - reg = <0xeee10000 0x1000>; > - clock-names = "hclk", "xxti", > - "fout_epll", > - "sclk_audio0"; > - clocks = <&clocks DOUT_HCLKP>, <&xxti>, > - <&clocks FOUT_EPLL>, > - <&clocks SCLK_AUDIO0>; > - #clock-cells = <1>; > - }; > + clk_audss: clock-controller@eee10000 { > + compatible = "samsung,s5pv210-audss-clock"; > + reg = <0xeee10000 0x1000>; > + clock-names = "hclk", "xxti", > + "fout_epll", > + "sclk_audio0"; > + clocks = <&clocks DOUT_HCLKP>, <&xxti>, > + <&clocks FOUT_EPLL>, > + <&clocks SCLK_AUDIO0>; > + #clock-cells = <1>; > + }; > > - i2s0: i2s@eee30000 { > - compatible = "samsung,s5pv210-i2s"; > - reg = <0xeee30000 0x1000>; > - interrupt-parent = <&vic2>; > - interrupts = <16>; > - dma-names = "rx", "tx", "tx-sec"; > - dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>; > - clock-names = "iis", > - "i2s_opclk0", > - "i2s_opclk1"; > - clocks = <&clk_audss CLK_I2S>, > - <&clk_audss CLK_I2S>, > - <&clk_audss CLK_DOUT_AUD_BUS>; > - samsung,idma-addr = <0xc0010000>; > - pinctrl-names = "default"; > - pinctrl-0 = <&i2s0_bus>; > - #sound-dai-cells = <0>; > - status = "disabled"; > - }; > + i2s0: i2s@eee30000 { > + compatible = "samsung,s5pv210-i2s"; > + reg = <0xeee30000 0x1000>; > + interrupt-parent = <&vic2>; > + interrupts = <16>; > + dma-names = "rx", "tx", "tx-sec"; > + dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>; > + clock-names = "iis", > + "i2s_opclk0", > + "i2s_opclk1"; > + clocks = <&clk_audss CLK_I2S>, > + <&clk_audss CLK_I2S>, > + <&clk_audss CLK_DOUT_AUD_BUS>; > + samsung,idma-addr = <0xc0010000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&i2s0_bus>; > + #sound-dai-cells = <0>; > + status = "disabled"; > }; > > i2s1: i2s@e2100000 { >
Works for me on the Galaxy S. Tested-by: Jonathan Bakker <xc-racer2@live.ca> Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > The device tree schema expects SPI controller to be named "spi", > otherwise dtbs_check complain with a warning like: > > spi-gpio-0: $nodename:0: 'spi-gpio-0' does not match '^spi(@.*|-[0-9a-f])*$' > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi > index 86c3b26fd21e..bd4450dbdcb6 100644 > --- a/arch/arm/boot/dts/s5pv210-aries.dtsi > +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi > @@ -545,7 +545,7 @@ > value = <0x5200>; > }; > > - spi_lcd: spi-gpio-0 { > + spi_lcd: spi-2 { > compatible = "spi-gpio"; > #address-cells = <1>; > #size-cells = <0>; >
Sadly, this is causing issues for me. The machine driver is no longer probing correctly on the Galaxy S. The failing call in sound/soc/samsung/aries_wm8994.c is /* Set CPU of_node for BT DAI */ aries_dai[2].cpus->of_node = of_parse_phandle(cpu, "sound-dai", 1); where cpus->of_node is not set properly. Which is definitely weird because it doesn't look like this should affect that. Let me know if there's any specific test that you want me to do. Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > The bindings describe I2S DAI has 1 cells. This makes especially sense > for i2s0 which registers two DAIs. Adjust the cells to fix dtbs_check > warnings like: > > i2s@e2100000: #sound-dai-cells:0:0: 1 was expected > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-fascinate4g.dts | 2 +- > arch/arm/boot/dts/s5pv210-galaxys.dts | 2 +- > arch/arm/boot/dts/s5pv210.dtsi | 6 +++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts > index ca064359dd30..a6dc8a173af1 100644 > --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts > +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts > @@ -102,7 +102,7 @@ > pinctrl-0 = <&headset_det &earpath_sel>; > > cpu { > - sound-dai = <&i2s0>, <&bt_codec>; > + sound-dai = <&i2s0 0>, <&bt_codec>; > }; > > codec { > diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts > index 560f830b6f6b..0eba06f56ac7 100644 > --- a/arch/arm/boot/dts/s5pv210-galaxys.dts > +++ b/arch/arm/boot/dts/s5pv210-galaxys.dts > @@ -132,7 +132,7 @@ > pinctrl-0 = <&headset_det &earpath_sel>; > > cpu { > - sound-dai = <&i2s0>, <&bt_codec>; > + sound-dai = <&i2s0 0>, <&bt_codec>; > }; > > codec { > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > index 2871351ab907..96e667ba1c3f 100644 > --- a/arch/arm/boot/dts/s5pv210.dtsi > +++ b/arch/arm/boot/dts/s5pv210.dtsi > @@ -251,7 +251,7 @@ > samsung,idma-addr = <0xc0010000>; > pinctrl-names = "default"; > pinctrl-0 = <&i2s0_bus>; > - #sound-dai-cells = <0>; > + #sound-dai-cells = <1>; > status = "disabled"; > }; > > @@ -266,7 +266,7 @@ > clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>; > pinctrl-names = "default"; > pinctrl-0 = <&i2s1_bus>; > - #sound-dai-cells = <0>; > + #sound-dai-cells = <1>; > status = "disabled"; > }; > > @@ -281,7 +281,7 @@ > clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>; > pinctrl-names = "default"; > pinctrl-0 = <&i2s2_bus>; > - #sound-dai-cells = <0>; > + #sound-dai-cells = <1>; > status = "disabled"; > }; > >
Hi Krzysztof, On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > The S3C RTC requires 32768 Hz clock as input which is provided by PMIC. > However there is no such clock provider but rather a regulator driver > which registers the clock as a regulator. This is an old driver which > will not be updated so add a workaround - a fixed-clock to fill missing > clock phandle reference in S3C RTC. > > This fixes dtbs_check warnings: > > rtc@e2800000: clocks: [[2, 145]] is too short > rtc@e2800000: clock-names: ['rtc'] is too short > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aries.dtsi | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi > index 6ba23562da46..86c3b26fd21e 100644 > --- a/arch/arm/boot/dts/s5pv210-aries.dtsi > +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi > @@ -47,6 +47,13 @@ > }; > }; > > + pmic_ap_clk: clock-0 { > + /* Workaround for missing clock on PMIC */ > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32768>; > + }; > + > bt_codec: bt_sco { > compatible = "linux,bt-sco"; > #sound-dai-cells = <0>; > @@ -825,6 +832,11 @@ > samsung,pwm-outputs = <1>; > }; > > +&rtc { > + clocks = <&clocks CLK_RTC>, <&pmic_ap_clk>; > + clock-names = "rtc", "rtc_src"; Missing a status = "okay"; here, but with that it works fine for me. Looks like it's also missing in the patches for the other devices as well. Thanks for the series of cleanups, Jonathan > +}; > + > &sdhci1 { > #address-cells = <1>; > #size-cells = <0>; >
Initial testing on both an i9000 and an SGH-T959P are showing that the audio has stopped working with this. I'm not 100% convinced as I've had DMA issues in the past. However trying to play something just results in a hang after 1.5s while it works just fine without this patch. Thanks, Jonathan On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > dtschema expects DMA channels in specific order (tx, rx and tx-sec). > The order actually should not matter because dma-names is used however > let's make it aligned with dtschema to suppress warnings like: > > i2s@eee30000: dma-names: ['rx', 'tx', 'tx-sec'] is not valid under any of the given schemas > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210.dtsi | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > index 96e667ba1c3f..72fb9d9f7ba3 100644 > --- a/arch/arm/boot/dts/s5pv210.dtsi > +++ b/arch/arm/boot/dts/s5pv210.dtsi > @@ -240,8 +240,8 @@ > reg = <0xeee30000 0x1000>; > interrupt-parent = <&vic2>; > interrupts = <16>; > - dma-names = "rx", "tx", "tx-sec"; > - dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>; > + dma-names = "tx", "rx", "tx-sec"; > + dmas = <&pdma1 10>, <&pdma1 9>, <&pdma1 11>; > clock-names = "iis", > "i2s_opclk0", > "i2s_opclk1"; > @@ -260,8 +260,8 @@ > reg = <0xe2100000 0x1000>; > interrupt-parent = <&vic2>; > interrupts = <17>; > - dma-names = "rx", "tx"; > - dmas = <&pdma1 12>, <&pdma1 13>; > + dma-names = "tx", "rx"; > + dmas = <&pdma1 13>, <&pdma1 12>; > clock-names = "iis", "i2s_opclk0"; > clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>; > pinctrl-names = "default"; > @@ -275,8 +275,8 @@ > reg = <0xe2a00000 0x1000>; > interrupt-parent = <&vic2>; > interrupts = <18>; > - dma-names = "rx", "tx"; > - dmas = <&pdma1 14>, <&pdma1 15>; > + dma-names = "tx", "rx"; > + dmas = <&pdma1 15>, <&pdma1 14>; > clock-names = "iis", "i2s_opclk0"; > clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>; > pinctrl-names = "default"; >
Ah, I figured out why the dma stopped working. In s5pv210-aries.dtsi the dma's for i2s0 are overriden to use pdma0 instead of pdma1. That also needs changing for this to work properly. Thanks, Jonathan On 2020-09-07 5:17 p.m., Jonathan Bakker wrote: > Initial testing on both an i9000 and an SGH-T959P are showing that the audio has > stopped working with this. I'm not 100% convinced as I've had DMA issues in the > past. However trying to play something just results in a hang after 1.5s while > it works just fine without this patch. > > Thanks, > Jonathan > > On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: >> dtschema expects DMA channels in specific order (tx, rx and tx-sec). >> The order actually should not matter because dma-names is used however >> let's make it aligned with dtschema to suppress warnings like: >> >> i2s@eee30000: dma-names: ['rx', 'tx', 'tx-sec'] is not valid under any of the given schemas >> >> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >> --- >> arch/arm/boot/dts/s5pv210.dtsi | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi >> index 96e667ba1c3f..72fb9d9f7ba3 100644 >> --- a/arch/arm/boot/dts/s5pv210.dtsi >> +++ b/arch/arm/boot/dts/s5pv210.dtsi >> @@ -240,8 +240,8 @@ >> reg = <0xeee30000 0x1000>; >> interrupt-parent = <&vic2>; >> interrupts = <16>; >> - dma-names = "rx", "tx", "tx-sec"; >> - dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>; >> + dma-names = "tx", "rx", "tx-sec"; >> + dmas = <&pdma1 10>, <&pdma1 9>, <&pdma1 11>; >> clock-names = "iis", >> "i2s_opclk0", >> "i2s_opclk1"; >> @@ -260,8 +260,8 @@ >> reg = <0xe2100000 0x1000>; >> interrupt-parent = <&vic2>; >> interrupts = <17>; >> - dma-names = "rx", "tx"; >> - dmas = <&pdma1 12>, <&pdma1 13>; >> + dma-names = "tx", "rx"; >> + dmas = <&pdma1 13>, <&pdma1 12>; >> clock-names = "iis", "i2s_opclk0"; >> clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>; >> pinctrl-names = "default"; >> @@ -275,8 +275,8 @@ >> reg = <0xe2a00000 0x1000>; >> interrupt-parent = <&vic2>; >> interrupts = <18>; >> - dma-names = "rx", "tx"; >> - dmas = <&pdma1 14>, <&pdma1 15>; >> + dma-names = "tx", "rx"; >> + dmas = <&pdma1 15>, <&pdma1 14>; >> clock-names = "iis", "i2s_opclk0"; >> clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>; >> pinctrl-names = "default"; >>
On Mon, Sep 07, 2020 at 04:55:26PM -0700, Jonathan Bakker wrote: > Sadly, this is causing issues for me. The machine driver is no longer probing correctly > on the Galaxy S. > > The failing call in sound/soc/samsung/aries_wm8994.c is > > /* Set CPU of_node for BT DAI */ > aries_dai[2].cpus->of_node = of_parse_phandle(cpu, > "sound-dai", 1); > > where cpus->of_node is not set properly. Which is definitely weird because it doesn't > look like this should affect that. > > Let me know if there's any specific test that you want me to do. Thanks for the tests. I wonder now if this was working before because really my change should not break it... I'll think more about it. Best regards, Krzysztof > > Thanks, > Jonathan > > > On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > > The bindings describe I2S DAI has 1 cells. This makes especially sense > > for i2s0 which registers two DAIs. Adjust the cells to fix dtbs_check > > warnings like: > > > > i2s@e2100000: #sound-dai-cells:0:0: 1 was expected > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > --- > > arch/arm/boot/dts/s5pv210-fascinate4g.dts | 2 +- > > arch/arm/boot/dts/s5pv210-galaxys.dts | 2 +- > > arch/arm/boot/dts/s5pv210.dtsi | 6 +++--- > > 3 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts > > index ca064359dd30..a6dc8a173af1 100644 > > --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts > > +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts > > @@ -102,7 +102,7 @@ > > pinctrl-0 = <&headset_det &earpath_sel>; > > > > cpu { > > - sound-dai = <&i2s0>, <&bt_codec>; > > + sound-dai = <&i2s0 0>, <&bt_codec>; > > }; > > > > codec { > > diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts > > index 560f830b6f6b..0eba06f56ac7 100644 > > --- a/arch/arm/boot/dts/s5pv210-galaxys.dts > > +++ b/arch/arm/boot/dts/s5pv210-galaxys.dts > > @@ -132,7 +132,7 @@ > > pinctrl-0 = <&headset_det &earpath_sel>; > > > > cpu { > > - sound-dai = <&i2s0>, <&bt_codec>; > > + sound-dai = <&i2s0 0>, <&bt_codec>; > > }; > > > > codec { > > diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi > > index 2871351ab907..96e667ba1c3f 100644 > > --- a/arch/arm/boot/dts/s5pv210.dtsi > > +++ b/arch/arm/boot/dts/s5pv210.dtsi > > @@ -251,7 +251,7 @@ > > samsung,idma-addr = <0xc0010000>; > > pinctrl-names = "default"; > > pinctrl-0 = <&i2s0_bus>; > > - #sound-dai-cells = <0>; > > + #sound-dai-cells = <1>; > > status = "disabled"; > > }; > > > > @@ -266,7 +266,7 @@ > > clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>; > > pinctrl-names = "default"; > > pinctrl-0 = <&i2s1_bus>; > > - #sound-dai-cells = <0>; > > + #sound-dai-cells = <1>; > > status = "disabled"; > > }; > > > > @@ -281,7 +281,7 @@ > > clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>; > > pinctrl-names = "default"; > > pinctrl-0 = <&i2s2_bus>; > > - #sound-dai-cells = <0>; > > + #sound-dai-cells = <1>; > > status = "disabled"; > > }; > > > >
On Mon, Sep 07, 2020 at 04:57:53PM -0700, Jonathan Bakker wrote: > Hi Krzysztof, > > On 2020-09-07 9:11 a.m., Krzysztof Kozlowski wrote: > > The S3C RTC requires 32768 Hz clock as input which is provided by PMIC. > > However there is no such clock provider but rather a regulator driver > > which registers the clock as a regulator. This is an old driver which > > will not be updated so add a workaround - a fixed-clock to fill missing > > clock phandle reference in S3C RTC. > > > > This fixes dtbs_check warnings: > > > > rtc@e2800000: clocks: [[2, 145]] is too short > > rtc@e2800000: clock-names: ['rtc'] is too short > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > --- > > arch/arm/boot/dts/s5pv210-aries.dtsi | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi > > index 6ba23562da46..86c3b26fd21e 100644 > > --- a/arch/arm/boot/dts/s5pv210-aries.dtsi > > +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi > > @@ -47,6 +47,13 @@ > > }; > > }; > > > > + pmic_ap_clk: clock-0 { > > + /* Workaround for missing clock on PMIC */ > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <32768>; > > + }; > > + > > bt_codec: bt_sco { > > compatible = "linux,bt-sco"; > > #sound-dai-cells = <0>; > > @@ -825,6 +832,11 @@ > > samsung,pwm-outputs = <1>; > > }; > > > > +&rtc { > > + clocks = <&clocks CLK_RTC>, <&pmic_ap_clk>; > > + clock-names = "rtc", "rtc_src"; > > Missing a > > status = "okay"; > > here, but with that it works fine for me. Looks like it's also > missing in the patches for the other devices as well. It wasn't there on purpose - I did not want to enable the RTC, just fix the DTS with the dtschema. However a separate patch could be to actually enable it. I'll add your tested-by to this patch. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 05:28:54PM -0700, Jonathan Bakker wrote: > Ah, I figured out why the dma stopped working. In s5pv210-aries.dtsi the dma's for i2s0 > are overriden to use pdma0 instead of pdma1. That also needs changing for this to > work properly. Indeed I missed this, thanks for review and tests. Best regards, Krzysztof > > Thanks, > Jonathan > > On 2020-09-07 5:17 p.m., Jonathan Bakker wrote: > > Initial testing on both an i9000 and an SGH-T959P are showing that the audio has > > stopped working with this. I'm not 100% convinced as I've had DMA issues in the > > past. However trying to play something just results in a hang after 1.5s while > > it works just fine without this patch. > > > > Thanks, > > Jonathan
On 9/8/20 08:53, Krzysztof Kozlowski wrote: > On Mon, Sep 07, 2020 at 04:55:26PM -0700, Jonathan Bakker wrote: >> Sadly, this is causing issues for me. The machine driver is no longer probing correctly >> on the Galaxy S. >> >> The failing call in sound/soc/samsung/aries_wm8994.c is >> >> /* Set CPU of_node for BT DAI */ >> aries_dai[2].cpus->of_node = of_parse_phandle(cpu, >> "sound-dai", 1); >> >> where cpus->of_node is not set properly. Which is definitely weird because it doesn't >> look like this should affect that. >> >> Let me know if there's any specific test that you want me to do. > Thanks for the tests. I wonder now if this was working before because > really my change should not break it... I'll think more about it. I think of_parse_phandle_with_args() needs to be used instead of just of_parse_phandle() for that to work, as AFAICS the latter assumes the cells count == 0. We would need first to update the driver and then dts.
On Mon, Sep 07, 2020 at 06:11:20PM +0200, Krzysztof Kozlowski wrote: > Fix typo in pinctrl property of "vibrator-en" fixed regulator in Aries > family of boards. The error caused lack of pin configuration for the > GPIO used in vibrator. > > Fixes: 04568cb58a43 ("ARM: dts: s5pv210: Disable pull for vibrator enable GPIO on Aries boards") > Cc: <stable@vger.kernel.org> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied 4-8. Thanks Jonathan for testing. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 06:11:26PM +0200, Krzysztof Kozlowski wrote: > The S3C RTC requires 32768 Hz clock as input which is provided by PMIC. > However there is no such clock provider but rather a regulator driver > which registers the clock as a regulator. This is an old driver which > will not be updated so add a workaround - a fixed-clock to fill missing > clock phandle reference in S3C RTC. > > This fixes dtbs_check warnings: > > rtc@e2800000: clocks: [[2, 145]] is too short > rtc@e2800000: clock-names: ['rtc'] is too short > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aquila.dts | 17 +++++++++++++++++ Applied 10-19. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 06:11:40PM +0200, Krzysztof Kozlowski wrote: > The device tree schema expects SPI controller to be named "spi", > otherwise dtbs_check complain with a warning like: > > spi-gpio-0: $nodename:0: 'spi-gpio-0' does not match '^spi(@.*|-[0-9a-f])*$' > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 06:11:36PM +0200, Krzysztof Kozlowski wrote: > The fixed regulators are kept under dedicated "regulators" node but this > causes multiple dtschema warnings: > > regulators: $nodename:0: 'regulators' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > regulators: #size-cells:0:0: 0 is not one of [1, 2] > regulators: fixed-regulator@0:reg:0: [0] is too short > regulators: fixed-regulator@1:reg:0: [1] is too short > regulators: fixed-regulator@2:reg:0: [2] is too short > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aquila.dts | 47 +++++++++++----------------- Applied. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 06:11:37PM +0200, Krzysztof Kozlowski wrote: > The fixed regulators are kept under dedicated "regulators" node but this > causes multiple dtschema warnings: > > regulators: $nodename:0: 'regulators' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > regulators: #size-cells:0:0: 0 is not one of [1, 2] > regulators: fixed-regulator@0:reg:0: [0] is too short > regulators: fixed-regulator@1:reg:0: [1] is too short > regulators: fixed-regulator@2:reg:0: [2] is too short > regulators: fixed-regulator@3:reg:0: [3] is too short > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-goni.dts | 64 +++++++++++++----------------- Applied. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 06:11:38PM +0200, Krzysztof Kozlowski wrote: > "gpios" property is deprecated. Update the Aquila DTS to fix > dtbs_checks warnings like: > > i2c-pmic: 'sda-gpios' is a required property > i2c-pmic: 'scl-gpios' is a required property > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-aquila.dts | 4 ++-- Applied. Best regards, Krzysztof
On Mon, Sep 07, 2020 at 06:11:39PM +0200, Krzysztof Kozlowski wrote: > "gpios" property is deprecated. Update the Goni DTS to fix > dtbs_checks warnings like: > > i2c-pmic: 'sda-gpios' is a required property > i2c-pmic: 'scl-gpios' is a required property > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm/boot/dts/s5pv210-goni.dts | 4 ++-- Applied. Best regards, Krzysztof
Hi The patchset tries to remove most of the dtschema warnings. Due to lack of hardware it was not tested. Best regards, Krzysztof Krzysztof Kozlowski (25): dt-bindings: samsung: pmu: document S5Pv210 dt-bindings: iio: adc: exynos-adc: require second interrupt with touch screen dt-bindings: iio: adc: exynos-adc: do not require syscon on S5Pv210 ARM: dts: s5pv210: fix pinctrl property of "vibrator-en" regulator in Aries ARM: dts: s5pv210: remove DMA controller bus node name to fix dtschema warnings ARM: dts: s5pv210: move fixed clocks under root node ARM: dts: s5pv210: move PMU node out of clock controller ARM: dts: s5pv210: remove dedicated 'audio-subsystem' node ARM: dts: s5pv210: fix number of I2S DAI cells ARM: dts: s5pv210: add RTC 32 KHz clock in Aquilla ARM: dts: s5pv210: add RTC 32 KHz clock in Aries family ARM: dts: s5pv210: add RTC 32 KHz clock in Goni ARM: dts: s5pv210: add RTC 32 KHz clock in SMDKC110 ARM: dts: s5pv210: add RTC 32 KHz clock in SMDKV210 ARM: dts: s5pv210: add RTC 32 KHz clock in Torbreck ARM: dts: s5pv210: use defines for GPIO flags in Aquila ARM: dts: s5pv210: use defines for GPIO flags in Goni ARM: dts: s5pv210: use defines for IRQ flags in SMDKV210 ARM: dts: s5pv210: use defines for IRQ flags in Goni ARM: dts: s5pv210: move fixed regulators under root node in Aquila ARM: dts: s5pv210: move fixed regulators under root node in Goni ARM: dts: s5pv210: replace deprecated "gpios" i2c-gpio property in Aquila ARM: dts: s5pv210: replace deprecated "gpios" i2c-gpio property in Goni ARM: dts: s5pv210: align SPI GPIO node name with dtschema in Aries ARM: dts: s5pv210: align DMA channels with dtschema .../devicetree/bindings/arm/samsung/pmu.yaml | 2 + .../bindings/iio/adc/samsung,exynos-adc.yaml | 16 +- arch/arm/boot/dts/s5pv210-aquila.dts | 73 ++++---- arch/arm/boot/dts/s5pv210-aries.dtsi | 16 +- arch/arm/boot/dts/s5pv210-fascinate4g.dts | 2 +- arch/arm/boot/dts/s5pv210-galaxys.dts | 2 +- arch/arm/boot/dts/s5pv210-goni.dts | 97 +++++----- arch/arm/boot/dts/s5pv210-smdkc110.dts | 9 + arch/arm/boot/dts/s5pv210-smdkv210.dts | 12 +- arch/arm/boot/dts/s5pv210-torbreck.dts | 9 + arch/arm/boot/dts/s5pv210.dtsi | 175 ++++++++---------- 11 files changed, 230 insertions(+), 183 deletions(-)