| Message ID | 20200907161141.31034-1-krzk@kernel.org |
|---|---|
| Headers | show |
| Series | ARM: dts: s5pv210: Cleanup - dtschema warnings | expand |
On Mon, 7 Sep 2020 18:11:19 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote: > The ADC in S5Pv210 does not have ADC phy registers in separate block for > which syscon would be needed. Remove this requirement to fix dtbs_check > warnings like: > > arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: adc@e1700000: 'samsung,syscon-phandle' is a required property > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > index 89b4f9c252a6..75174af72288 100644 > --- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > @@ -81,7 +81,6 @@ allOf: > - samsung,exynos-adc-v2 > - samsung,exynos3250-adc > - samsung,exynos4212-adc > - - samsung,s5pv210-adc > then: > required: > - samsung,syscon-phandle
On Mon, 7 Sep 2020 at 18:39, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > On Mon, 7 Sep 2020 18:11:18 +0200 > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch > > screen. In such case the second interrupt is required. This second > > interrupt can be anyway provided, even without touch screens. This > > fixes dtbs_check warnings like: > > > > arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > Acked-by: Jonathan Cameron <Jonathan.Cameron@huwei.com> > > Or I can pick this up through the IIO tree if that makes sense. > I doubt anything else will touch this binding this cycle, so either > way works for me. Let's wait for Rob's review and then if you could, please pick it up. Best regards, Krzysztof
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 { >
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>; >
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 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 Mon, Sep 07, 2020 at 06:11:18PM +0200, Krzysztof Kozlowski wrote: > The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch > screen. In such case the second interrupt is required. This second > interrupt can be anyway provided, even without touch screens. This > fixes dtbs_check warnings like: > > arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > .../bindings/iio/adc/samsung,exynos-adc.yaml | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > index cc3c8ea6a894..89b4f9c252a6 100644 > --- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > @@ -41,7 +41,10 @@ properties: > maxItems: 2 > > interrupts: > - maxItems: 1 > + description: > + ADC interrupt followed by optional touchscreen interrupt. > + minItems: 1 > + maxItems: 2 > > "#io-channel-cells": > const: 1 > @@ -107,6 +110,16 @@ allOf: > items: > - const: adc > > + - if: > + properties: > + has-touchscreen: > + true This evaluates as true if 'has-touchscreen' is not present too. You should use 'required' here. > + then: > + properties: > + interrupts: > + minItems: 2 > + maxItems: 2 > + > examples: > - | > adc: adc@12d10000 { > -- > 2.17.1 >
On Tue, 8 Sep 2020 at 22:25, Rob Herring <robh@kernel.org> wrote: > > On Mon, Sep 07, 2020 at 06:11:18PM +0200, Krzysztof Kozlowski wrote: > > The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch > > screen. In such case the second interrupt is required. This second > > interrupt can be anyway provided, even without touch screens. This > > fixes dtbs_check warnings like: > > > > arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > --- > > .../bindings/iio/adc/samsung,exynos-adc.yaml | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > > index cc3c8ea6a894..89b4f9c252a6 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > > @@ -41,7 +41,10 @@ properties: > > maxItems: 2 > > > > interrupts: > > - maxItems: 1 > > + description: > > + ADC interrupt followed by optional touchscreen interrupt. > > + minItems: 1 > > + maxItems: 2 > > > > "#io-channel-cells": > > const: 1 > > @@ -107,6 +110,16 @@ allOf: > > items: > > - const: adc > > > > + - if: > > + properties: > > + has-touchscreen: > > + true > > This evaluates as true if 'has-touchscreen' is not present too. You > should use 'required' here. I see, thanks. I'll send a v2. 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: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(-)