diff mbox series

[01/10] dt-bindings: arm: samsung: pmu: Use unevaluatedProperties

Message ID 20200829142501.31478-1-krzk@kernel.org
State Changes Requested, archived
Headers show
Series [01/10] dt-bindings: arm: samsung: pmu: Use unevaluatedProperties | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 8 lines checked

Commit Message

Krzysztof Kozlowski Aug. 29, 2020, 2:24 p.m. UTC
Additional properties actually might appear (e.g. assigned-clocks) so
use unevaluatedProperties to fix dtbs_check warnings like:

  arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: system-controller@105c0000:
    'assigned-clock-parents', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sylwester Nawrocki Aug. 31, 2020, 12:14 p.m. UTC | #1
On 29.08.2020 16:24, Krzysztof Kozlowski wrote:
> Additional properties actually might appear (e.g. assigned-clocks) so
> use unevaluatedProperties to fix dtbs_check warnings like:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: system-controller@105c0000:
>     'assigned-clock-parents', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Marek Szyprowski Aug. 31, 2020, 12:50 p.m. UTC | #2
Hi Krzysztof,

On 29.08.2020 16:25, Krzysztof Kozlowski wrote:
> The USB-C connector bindings require port@0.  Such port was already
> described in DTS but outside of the connector itself.  Put it into
> proper place to fix dtbs_check warnings like:
>
>    arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: musb_connector: ports: 'port@0' is a required property
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

I'm not sure if topic should be about USB-C, I will call it simply USB 
connector node. TM2(e) uses Samsung's 11-pin micro USB 2.0 connector, 
which has nothing in common with USB Type-C.

Anyway, this patch breaks DWC3 (tested in Device mode) driver operation, 
so something has to be somehow adjusted or fixed. Added CC Andrzej 
Hajda, who actually worked on this.

> ---
>
> Not tested on HQ. Please kindly review and test.
>
> Best regards,
> Krzysztof
> ---
>   .../boot/dts/exynos/exynos5433-tm2-common.dtsi    | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index 6246cce2a15e..bab6c1addd5f 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -871,6 +871,13 @@
>   					#address-cells = <1>;
>   					#size-cells = <0>;
>   
> +					port@0 {
> +						reg = <0>;
> +						muic_to_usb: endpoint {
> +							remote-endpoint = <&usb_to_muic>;
> +						};
> +					};
> +
>   					port@3 {
>   						reg = <3>;
>   						musb_con_to_mhl: endpoint {
> @@ -879,14 +886,6 @@
>   					};
>   				};
>   			};
> -
> -			ports {
> -				port {
> -					muic_to_usb: endpoint {
> -						remote-endpoint = <&usb_to_muic>;
> -					};
> -				};
> -			};
>   		};
>   
>   		regulators {

Best regards
Marek Szyprowski Aug. 31, 2020, 12:52 p.m. UTC | #3
On 29.08.2020 16:24, Krzysztof Kozlowski wrote:
> "gpios" property is deprecated.  Update the Exynos5433 DTS to fix
> dtbs_checks warnings like:
>
>    arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: i2c-gpio-0: 'sda-gpios' is a required property
>    arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: i2c-gpio-0: 'scl-gpios' is a required property
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index 250fc01de78d..6246cce2a15e 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -87,8 +87,8 @@
>   
>   	i2c_max98504: i2c-gpio-0 {
>   		compatible = "i2c-gpio";
> -		gpios = <&gpd0 1 GPIO_ACTIVE_HIGH /* SPK_AMP_SDA */
> -			 &gpd0 0 GPIO_ACTIVE_HIGH /* SPK_AMP_SCL */ >;
> +		sda-gpios = <&gpd0 1 GPIO_ACTIVE_HIGH>;
> +		scl-gpios = <&gpd0 0 GPIO_ACTIVE_HIGH>;
>   		i2c-gpio,delay-us = <2>;
>   		#address-cells = <1>;
>   		#size-cells = <0>;

Best regards
Sylwester Nawrocki Aug. 31, 2020, 12:58 p.m. UTC | #4
On 29.08.2020 16:24, Krzysztof Kozlowski wrote:
> "gpios" property is deprecated.  Update the Exynos5433 DTS to fix
> dtbs_checks warnings like:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: i2c-gpio-0: 'sda-gpios' is a required property
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: i2c-gpio-0: 'scl-gpios' is a required property
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Sylwester Nawrocki Aug. 31, 2020, 1 p.m. UTC | #5
On 29.08.2020 16:24, Krzysztof Kozlowski wrote:
> System register nodes, implementing syscon binding, should use
> appropriate compatible.  This fixes dtbs_check warnings:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: syscon@13b80000:
>     compatible: ['syscon'] is not valid under any of the given schemas
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Marek Szyprowski Aug. 31, 2020, 1:11 p.m. UTC | #6
On 29.08.2020 16:25, Krzysztof Kozlowski wrote:
> The Wolfson Arizona codec is interrupt controller which is required by
> bindings.  This fixes dtbs_check warnings like:
>
>    arch/arm64/boot/dts/exynos/exynos5433-tm2e.dt.yaml: wm5110-codec@0: 'interrupt-controller' is a required property
>    arch/arm64/boot/dts/exynos/exynos5433-tm2e.dt.yaml: wm5110-codec@0: '#interrupt-cells' is a required property
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

However I really wonder if it makes sense to expose this to DTS. Indeed, 
the main MFD device of the WM5110 chip is interrupt controller, but its 
interrupts are requested internally by the respective drivers.

> ---
>
> Not tested on HQ. Please kindly review and test.
>
> Best regards,
> Krzysztof
> ---
>   arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index bab6c1addd5f..49cd55d6891c 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -1242,6 +1242,8 @@
>   
>   		gpio-controller;
>   		#gpio-cells = <2>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
>   
>   		wlf,micd-detect-debounce = <300>;
>   		wlf,micd-bias-start-time = <0x1>;

Best regards
Krzysztof Kozlowski Aug. 31, 2020, 1:49 p.m. UTC | #7
On Mon, 31 Aug 2020 at 15:12, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
>
> On 29.08.2020 16:25, Krzysztof Kozlowski wrote:
> > The Wolfson Arizona codec is interrupt controller which is required by
> > bindings.  This fixes dtbs_check warnings like:
> >
> >    arch/arm64/boot/dts/exynos/exynos5433-tm2e.dt.yaml: wm5110-codec@0: 'interrupt-controller' is a required property
> >    arch/arm64/boot/dts/exynos/exynos5433-tm2e.dt.yaml: wm5110-codec@0: '#interrupt-cells' is a required property
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> However I really wonder if it makes sense to expose this to DTS. Indeed,
> the main MFD device of the WM5110 chip is interrupt controller, but its
> interrupts are requested internally by the respective drivers.

In such case maybe the schema should be updated? Feel free to send a
follow up or a replacement patch for this one.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 1, 2020, 10:11 a.m. UTC | #8
On Sat, Aug 29, 2020 at 04:24:52PM +0200, Krzysztof Kozlowski wrote:
> Additional properties actually might appear (e.g. assigned-clocks) so
> use unevaluatedProperties to fix dtbs_check warnings like:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: system-controller@105c0000:
>     'assigned-clock-parents', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hi Rob,

Could you pick all of my dt-bindings schema patches if they are ok? From
this and other series. You already have few of them in your tree so it
will help to avoid conflicts.

I am afraid that subsystem maintainers can leave them to you and vice
versa :)

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 1, 2020, 10:12 a.m. UTC | #9
On Sat, Aug 29, 2020 at 04:24:58PM +0200, Krzysztof Kozlowski wrote:
> "gpios" property is deprecated.  Update the Exynos5433 DTS to fix
> dtbs_checks warnings like:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: i2c-gpio-0: 'sda-gpios' is a required property
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: i2c-gpio-0: 'scl-gpios' is a required property
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++--

Applied.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 1, 2020, 10:13 a.m. UTC | #10
On Sat, Aug 29, 2020 at 04:24:59PM +0200, Krzysztof Kozlowski wrote:
> System register nodes, implementing syscon binding, should use
> appropriate compatible.  This fixes dtbs_check warnings:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: syscon@13b80000:
>     compatible: ['syscon'] is not valid under any of the given schemas
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 6 +++---

Applied.

Best regards,
Krzysztof
Mark Brown Sept. 1, 2020, 2:50 p.m. UTC | #11
On Sat, 29 Aug 2020 16:24:52 +0200, Krzysztof Kozlowski wrote:
> Additional properties actually might appear (e.g. assigned-clocks) so
> use unevaluatedProperties to fix dtbs_check warnings like:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: system-controller@105c0000:
>     'assigned-clock-parents', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: samsung-i2s: Use unevaluatedProperties
      commit: 8187d8300251a99e40e288be80bef6a15b7b22e4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Andrzej Hajda Sept. 2, 2020, 11:23 a.m. UTC | #12
On 31.08.2020 14:50, Marek Szyprowski wrote:
> Hi Krzysztof,
>
> On 29.08.2020 16:25, Krzysztof Kozlowski wrote:
>> The USB-C connector bindings require port@0.  Such port was already
>> described in DTS but outside of the connector itself.  Put it into
>> proper place to fix dtbs_check warnings like:
>>
>>     arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: musb_connector: ports: 'port@0' is a required property
>>
>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> I'm not sure if topic should be about USB-C, I will call it simply USB
> connector node. TM2(e) uses Samsung's 11-pin micro USB 2.0 connector,
> which has nothing in common with USB Type-C.
>
> Anyway, this patch breaks DWC3 (tested in Device mode) driver operation,
> so something has to be somehow adjusted or fixed. Added CC Andrzej
> Hajda, who actually worked on this.
>
>> ---
>>
>> Not tested on HQ. Please kindly review and test.
>>
>> Best regards,
>> Krzysztof
>> ---
>>    .../boot/dts/exynos/exynos5433-tm2-common.dtsi    | 15 +++++++--------
>>    1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> index 6246cce2a15e..bab6c1addd5f 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> @@ -871,6 +871,13 @@
>>    					#address-cells = <1>;
>>    					#size-cells = <0>;
>>    
>> +					port@0 {
>> +						reg = <0>;
>> +						muic_to_usb: endpoint {
>> +							remote-endpoint = <&usb_to_muic>;
>> +						};
>> +					};
>> +


According to not-yet-yaml documentation of dt-bindings (patch 05/10):
> -Required nodes:
> -- any data bus to the connector should be modeled using the OF graph bindings
> -  specified in bindings/graph.txt, unless the bus is between parent node and
> -  the connector.

This is 'unless' case - muic is parent of the connector, so the port 0 is not necessary.


>>    					port@3 {
>>    						reg = <3>;
>>    						musb_con_to_mhl: endpoint {
>> @@ -879,14 +886,6 @@
>>    					};
>>    				};
>>    			};
>> -
>> -			ports {
>> -				port {
>> -					muic_to_usb: endpoint {
>> -						remote-endpoint = <&usb_to_muic>;
>> -					};
>> -				};


And this port belongs to MUIC - it describes connection between USB-HOST 
and MUIC, it has nothing to do with the connector, and is necessary.


Regards

Andrzej


>> -			};
>>    		};
>>    
>>    		regulators {
> Best regards
Rob Herring Sept. 3, 2020, 4:41 p.m. UTC | #13
On Sat, Aug 29, 2020 at 04:24:52PM +0200, Krzysztof Kozlowski wrote:
> Additional properties actually might appear (e.g. assigned-clocks) so
> use unevaluatedProperties to fix dtbs_check warnings like:
> 
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: system-controller@105c0000:
>     'assigned-clock-parents', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

NAK. See https://lore.kernel.org/r/CAL_JsqKPXJxsHPS34_TCf9bwgKxZNSV4mvQR-WKRnknQVtGGxQ@mail.gmail.com/
Rob Herring Sept. 3, 2020, 4:45 p.m. UTC | #14
On Tue, Sep 01, 2020 at 03:50:00PM +0100, Mark Brown wrote:
> On Sat, 29 Aug 2020 16:24:52 +0200, Krzysztof Kozlowski wrote:
> > Additional properties actually might appear (e.g. assigned-clocks) so
> > use unevaluatedProperties to fix dtbs_check warnings like:
> > 
> >   arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: system-controller@105c0000:
> >     'assigned-clock-parents', 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
> 
> Thanks!
> 
> [1/1] ASoC: samsung-i2s: Use unevaluatedProperties
>       commit: 8187d8300251a99e40e288be80bef6a15b7b22e4

Please revert or drop. All these 'unevaluatedProperties' changes are 
wrong.

Rob
Krzysztof Kozlowski Sept. 16, 2020, 7:59 a.m. UTC | #15
On Wed, Sep 02, 2020 at 01:23:50PM +0200, Andrzej Hajda wrote:
> 
> On 31.08.2020 14:50, Marek Szyprowski wrote:
> > Hi Krzysztof,
> >
> > On 29.08.2020 16:25, Krzysztof Kozlowski wrote:
> >> The USB-C connector bindings require port@0.  Such port was already
> >> described in DTS but outside of the connector itself.  Put it into
> >> proper place to fix dtbs_check warnings like:
> >>
> >>     arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: musb_connector: ports: 'port@0' is a required property
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > I'm not sure if topic should be about USB-C, I will call it simply USB
> > connector node. TM2(e) uses Samsung's 11-pin micro USB 2.0 connector,
> > which has nothing in common with USB Type-C.
> >
> > Anyway, this patch breaks DWC3 (tested in Device mode) driver operation,
> > so something has to be somehow adjusted or fixed. Added CC Andrzej
> > Hajda, who actually worked on this.
> >
> >> ---
> >>
> >> Not tested on HQ. Please kindly review and test.
> >>
> >> Best regards,
> >> Krzysztof
> >> ---
> >>    .../boot/dts/exynos/exynos5433-tm2-common.dtsi    | 15 +++++++--------
> >>    1 file changed, 7 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> >> index 6246cce2a15e..bab6c1addd5f 100644
> >> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> >> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> >> @@ -871,6 +871,13 @@
> >>    					#address-cells = <1>;
> >>    					#size-cells = <0>;
> >>    
> >> +					port@0 {
> >> +						reg = <0>;
> >> +						muic_to_usb: endpoint {
> >> +							remote-endpoint = <&usb_to_muic>;
> >> +						};
> >> +					};
> >> +
> 
> 
> According to not-yet-yaml documentation of dt-bindings (patch 05/10):
> > -Required nodes:
> > -- any data bus to the connector should be modeled using the OF graph bindings
> > -  specified in bindings/graph.txt, unless the bus is between parent node and
> > -  the connector.
> 
> This is 'unless' case - muic is parent of the connector, so the port 0 is not necessary.
> 
> 
> >>    					port@3 {
> >>    						reg = <3>;
> >>    						musb_con_to_mhl: endpoint {
> >> @@ -879,14 +886,6 @@
> >>    					};
> >>    				};
> >>    			};
> >> -
> >> -			ports {
> >> -				port {
> >> -					muic_to_usb: endpoint {
> >> -						remote-endpoint = <&usb_to_muic>;
> >> -					};
> >> -				};
> 
> 
> And this port belongs to MUIC - it describes connection between USB-HOST 
> and MUIC, it has nothing to do with the connector, and is necessary.

Thanks for checking this. It's really appreciated!

I'll work on v2 later to address the schema warning, hopefully without
breaking things...

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
index 686c13c14e32..30ff2da81416 100644
--- a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
+++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
@@ -86,7 +86,7 @@  required:
   - compatible
   - reg
 
-additionalProperties: false
+unevaluatedProperties: false
 
 allOf:
   - if: