diff mbox series

[01/19] dt-bindings: sunxi: Fix the pinecube compatible

Message ID 20210114113538.1233933-1-maxime@cerno.tech
State Accepted
Commit e0ab5bf98208294f7fb39d8ce3746eaf39d38dfe
Headers show
Series [01/19] dt-bindings: sunxi: Fix the pinecube compatible | expand

Commit Message

Maxime Ripard Jan. 14, 2021, 11:35 a.m. UTC
Commit 6ab48105aae7 ("ARM: dts: s3: pinecube: align compatible property
to other S3 boards") changed the pinecube compatible to make it similar
to the other S3 boards we have, but failed to update the bindings
documentation.

Fixes: 6ab48105aae7 ("ARM: dts: s3: pinecube: align compatible property to other S3 boards")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jernej Škrabec Jan. 14, 2021, 8:14 p.m. UTC | #1
Dne četrtek, 14. januar 2021 ob 12:35:23 CET je Maxime Ripard napisal(a):
> The corpro,gm7123 was in use in a DT but was never properly documented,
> let's add it.
> 
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej
Jernej Škrabec Jan. 14, 2021, 8:14 p.m. UTC | #2
Dne četrtek, 14. januar 2021 ob 12:35:24 CET je Maxime Ripard napisal(a):
> additionalProperties prevent any property not explicitly defined in the
> binding to be used. Yet, some serial properties like max-speed are valid
> and validated through the serial/serial.yaml binding.
> 
> Let's change additionalProperties to unevaluatedProperties to avoid
> spurious warnings.
> 
> Cc: Alistair Francis <alistair@alistair23.me>
> Cc: Vasily Khoruzhick <anarsoul@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej
Jernej Škrabec Jan. 14, 2021, 8:33 p.m. UTC | #3
Dne četrtek, 14. januar 2021 ob 12:35:25 CET je Maxime Ripard napisal(a):
> According to the LED bindings, the LED node names are supposed to be led
> plus an optional suffix. Let's fix our users to use that new scheme.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---

Acked-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej
Jernej Škrabec Jan. 14, 2021, 8:37 p.m. UTC | #4
Dne četrtek, 14. januar 2021 ob 12:35:29 CET je Maxime Ripard napisal(a):
> The mma8452 binding doesn't expect an io-channel-cells property, let's
> remove it.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---

Acked-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej
Jernej Škrabec Jan. 14, 2021, 8:40 p.m. UTC | #5
Dne četrtek, 14. januar 2021 ob 12:35:33 CET je Maxime Ripard napisal(a):
> The empty CSI port triggers a dt-validate warning. Let's align with the
> other DTSI and remove it entirely, expecting the DTS to fill it.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej
Jernej Škrabec Jan. 14, 2021, 8:46 p.m. UTC | #6
Dne četrtek, 14. januar 2021 ob 12:35:37 CET je Maxime Ripard napisal(a):
> The commit 7fa40ca7ef61 ("arm64: allwinner: dts: a64: add DT for Early
> Adopter's PineTab") introduced an ili9881-based panel device node but
> didn't conform to the binding. Fix this.
> 
> Fixes: 7fa40ca7ef61 ("arm64: allwinner: dts: a64: add DT for Early Adopter's 
PineTab")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej
Rob Herring (Arm) Jan. 15, 2021, 1:37 a.m. UTC | #7
On Thu, Jan 14, 2021 at 12:35:24PM +0100, Maxime Ripard wrote:
> additionalProperties prevent any property not explicitly defined in the

> binding to be used. Yet, some serial properties like max-speed are valid

> and validated through the serial/serial.yaml binding.

> 

> Let's change additionalProperties to unevaluatedProperties to avoid

> spurious warnings.

> 

> Cc: Alistair Francis <alistair@alistair23.me>

> Cc: Vasily Khoruzhick <anarsoul@gmail.com>

> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> ---

>  Documentation/devicetree/bindings/net/realtek-bluetooth.yaml | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> index 4f485df69ac3..f4d4969d87f4 100644

> --- a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> @@ -37,7 +37,7 @@ properties:

>  required:

>    - compatible

>  

> -additionalProperties: false

> +unevaluatedProperties: false


This would still fail because the serial schema is applied to the parent 
and this schema is applied to the child node. It's a common problem for 
how we've done bus schemas. We'd need to split them into 2 schemas and 
reference the child schema here. I'd rather not do that here because 
then we'd apply the schema twice assuming we keep bus schemas also 
applying the child schemas (which is good at least until we have schemas 
for *all* possible child devices).

We've handled this in other cases by just doing 'max-speed: true' here.

Rob
Chen-Yu Tsai Jan. 15, 2021, 2:41 a.m. UTC | #8
On Thu, Jan 14, 2021 at 7:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
>

> Commit 6ab48105aae7 ("ARM: dts: s3: pinecube: align compatible property

> to other S3 boards") changed the pinecube compatible to make it similar

> to the other S3 boards we have, but failed to update the bindings

> documentation.

>

> Fixes: 6ab48105aae7 ("ARM: dts: s3: pinecube: align compatible property to other S3 boards")

> Signed-off-by: Maxime Ripard <maxime@cerno.tech>


Acked-by: Chen-Yu Tsai <wens@csie.org>


for the whole series.
Laurent Pinchart Jan. 15, 2021, 7:16 a.m. UTC | #9
Hi Maxime,

Thank you for the patch.

On Thu, Jan 14, 2021 at 12:35:23PM +0100, Maxime Ripard wrote:
> The corpro,gm7123 was in use in a DT but was never properly documented,

> let's add it.

> 

> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Cc: Sam Ravnborg <sam@ravnborg.org>

> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> ---

>  .../devicetree/bindings/display/bridge/simple-bridge.yaml     | 4 ++++

>  1 file changed, 4 insertions(+)

> 

> diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> index 64e8a1c24b40..97ca07b56cbc 100644

> --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> @@ -22,6 +22,10 @@ properties:

>                - ti,ths8134a

>                - ti,ths8134b

>            - const: ti,ths8134

> +      - items:

> +          - const: corpro,gm7123

> +          - const: adi,adv7123


Do we need adi,adv7123 ?

> +          - const: dumb-vga-dac

>        - enum:

>            - adi,adv7123

>            - dumb-vga-dac


-- 
Regards,

Laurent Pinchart
Maxime Ripard Jan. 15, 2021, 9:31 a.m. UTC | #10
On Fri, Jan 15, 2021 at 10:41:04AM +0800, Chen-Yu Tsai wrote:
> On Thu, Jan 14, 2021 at 7:35 PM Maxime Ripard <maxime@cerno.tech> wrote:

> >

> > Commit 6ab48105aae7 ("ARM: dts: s3: pinecube: align compatible property

> > to other S3 boards") changed the pinecube compatible to make it similar

> > to the other S3 boards we have, but failed to update the bindings

> > documentation.

> >

> > Fixes: 6ab48105aae7 ("ARM: dts: s3: pinecube: align compatible property to other S3 boards")

> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> 

> Acked-by: Chen-Yu Tsai <wens@csie.org>

> 

> for the whole series.


Applied all the patches but the 4 and 5 that had comments

Thanks!
Maxime
Maxime Ripard Jan. 18, 2021, 10:03 a.m. UTC | #11
Hi Laurent,

On Fri, Jan 15, 2021 at 09:16:30AM +0200, Laurent Pinchart wrote:
> Thank you for the patch.

> 

> On Thu, Jan 14, 2021 at 12:35:23PM +0100, Maxime Ripard wrote:

> > The corpro,gm7123 was in use in a DT but was never properly documented,

> > let's add it.

> > 

> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> > Cc: Sam Ravnborg <sam@ravnborg.org>

> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> > ---

> >  .../devicetree/bindings/display/bridge/simple-bridge.yaml     | 4 ++++

> >  1 file changed, 4 insertions(+)

> > 

> > diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > index 64e8a1c24b40..97ca07b56cbc 100644

> > --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > @@ -22,6 +22,10 @@ properties:

> >                - ti,ths8134a

> >                - ti,ths8134b

> >            - const: ti,ths8134

> > +      - items:

> > +          - const: corpro,gm7123

> > +          - const: adi,adv7123

> 

> Do we need adi,adv7123 ?


I'm not sure: it looks like having both the adv7123 and the dumb-vga-dac
compatible was never an option, so we can also change the DT to have
corpro,gm7123, dumb-vga-dac

Would that work?

Maxime
Maxime Ripard Jan. 18, 2021, 10:15 a.m. UTC | #12
Hi Rob,

On Thu, Jan 14, 2021 at 07:37:14PM -0600, Rob Herring wrote:
> On Thu, Jan 14, 2021 at 12:35:24PM +0100, Maxime Ripard wrote:

> > additionalProperties prevent any property not explicitly defined in the

> > binding to be used. Yet, some serial properties like max-speed are valid

> > and validated through the serial/serial.yaml binding.

> > 

> > Let's change additionalProperties to unevaluatedProperties to avoid

> > spurious warnings.

> > 

> > Cc: Alistair Francis <alistair@alistair23.me>

> > Cc: Vasily Khoruzhick <anarsoul@gmail.com>

> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> > ---

> >  Documentation/devicetree/bindings/net/realtek-bluetooth.yaml | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > 

> > diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> > index 4f485df69ac3..f4d4969d87f4 100644

> > --- a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> > +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> > @@ -37,7 +37,7 @@ properties:

> >  required:

> >    - compatible

> >  

> > -additionalProperties: false

> > +unevaluatedProperties: false

> 

> This would still fail because the serial schema is applied to the parent 

> and this schema is applied to the child node. It's a common problem for 

> how we've done bus schemas. We'd need to split them into 2 schemas and 

> reference the child schema here. I'd rather not do that here because 

> then we'd apply the schema twice assuming we keep bus schemas also 

> applying the child schemas (which is good at least until we have schemas 

> for *all* possible child devices).


I couldn't find what requires additionalProperties in the meta-schemas,
could you point me to what enforces it?

> We've handled this in other cases by just doing 'max-speed: true' here.


I will do that then, thanks!
Maxime
Chen-Yu Tsai Jan. 18, 2021, 10:22 a.m. UTC | #13
On Mon, Jan 18, 2021 at 6:03 PM Maxime Ripard <maxime@cerno.tech> wrote:
>

> Hi Laurent,

>

> On Fri, Jan 15, 2021 at 09:16:30AM +0200, Laurent Pinchart wrote:

> > Thank you for the patch.

> >

> > On Thu, Jan 14, 2021 at 12:35:23PM +0100, Maxime Ripard wrote:

> > > The corpro,gm7123 was in use in a DT but was never properly documented,

> > > let's add it.

> > >

> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> > > Cc: Sam Ravnborg <sam@ravnborg.org>

> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> > > ---

> > >  .../devicetree/bindings/display/bridge/simple-bridge.yaml     | 4 ++++

> > >  1 file changed, 4 insertions(+)

> > >

> > > diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > index 64e8a1c24b40..97ca07b56cbc 100644

> > > --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > @@ -22,6 +22,10 @@ properties:

> > >                - ti,ths8134a

> > >                - ti,ths8134b

> > >            - const: ti,ths8134

> > > +      - items:

> > > +          - const: corpro,gm7123

> > > +          - const: adi,adv7123

> >

> > Do we need adi,adv7123 ?

>

> I'm not sure: it looks like having both the adv7123 and the dumb-vga-dac

> compatible was never an option, so we can also change the DT to have

> corpro,gm7123, dumb-vga-dac


FYI I used the adi,adv7123 fallback because the gm7123 datasheet compares
the two, with the gm7123 having lower standby current, but only limited to
3.3V power supply. Maximum resolution, conversion rate, and the range of
output current are all the same. The package is the same as well. I
believe the gm7123 is intended as a drop-in clone of the adv7123.

ChenYu
Laurent Pinchart Jan. 18, 2021, 10:38 a.m. UTC | #14
Hi Maxime,

On Mon, Jan 18, 2021 at 11:03:21AM +0100, Maxime Ripard wrote:
> On Fri, Jan 15, 2021 at 09:16:30AM +0200, Laurent Pinchart wrote:

> > On Thu, Jan 14, 2021 at 12:35:23PM +0100, Maxime Ripard wrote:

> > > The corpro,gm7123 was in use in a DT but was never properly documented,

> > > let's add it.

> > > 

> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> > > Cc: Sam Ravnborg <sam@ravnborg.org>

> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> > > ---

> > >  .../devicetree/bindings/display/bridge/simple-bridge.yaml     | 4 ++++

> > >  1 file changed, 4 insertions(+)

> > > 

> > > diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > index 64e8a1c24b40..97ca07b56cbc 100644

> > > --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > @@ -22,6 +22,10 @@ properties:

> > >                - ti,ths8134a

> > >                - ti,ths8134b

> > >            - const: ti,ths8134

> > > +      - items:

> > > +          - const: corpro,gm7123

> > > +          - const: adi,adv7123

> > 

> > Do we need adi,adv7123 ?

> 

> I'm not sure: it looks like having both the adv7123 and the dumb-vga-dac

> compatible was never an option, so we can also change the DT to have

> corpro,gm7123, dumb-vga-dac

> 

> Would that work?


With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


-- 
Regards,

Laurent Pinchart
Laurent Pinchart Jan. 18, 2021, 10:39 a.m. UTC | #15
Hi Chen-Yu,

On Mon, Jan 18, 2021 at 06:22:15PM +0800, Chen-Yu Tsai wrote:
> On Mon, Jan 18, 2021 at 6:03 PM Maxime Ripard <maxime@cerno.tech> wrote:

> > On Fri, Jan 15, 2021 at 09:16:30AM +0200, Laurent Pinchart wrote:

> > > On Thu, Jan 14, 2021 at 12:35:23PM +0100, Maxime Ripard wrote:

> > > > The corpro,gm7123 was in use in a DT but was never properly documented,

> > > > let's add it.

> > > >

> > > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> > > > Cc: Sam Ravnborg <sam@ravnborg.org>

> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> > > > ---

> > > >  .../devicetree/bindings/display/bridge/simple-bridge.yaml     | 4 ++++

> > > >  1 file changed, 4 insertions(+)

> > > >

> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > > index 64e8a1c24b40..97ca07b56cbc 100644

> > > > --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > > +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml

> > > > @@ -22,6 +22,10 @@ properties:

> > > >                - ti,ths8134a

> > > >                - ti,ths8134b

> > > >            - const: ti,ths8134

> > > > +      - items:

> > > > +          - const: corpro,gm7123

> > > > +          - const: adi,adv7123

> > >

> > > Do we need adi,adv7123 ?

> >

> > I'm not sure: it looks like having both the adv7123 and the dumb-vga-dac

> > compatible was never an option, so we can also change the DT to have

> > corpro,gm7123, dumb-vga-dac

> 

> FYI I used the adi,adv7123 fallback because the gm7123 datasheet compares

> the two, with the gm7123 having lower standby current, but only limited to

> 3.3V power supply. Maximum resolution, conversion rate, and the range of

> output current are all the same. The package is the same as well. I

> believe the gm7123 is intended as a drop-in clone of the adv7123.


That's a good point. I don't mind either way.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


-- 
Regards,

Laurent Pinchart
Rob Herring (Arm) Jan. 22, 2021, 2:40 p.m. UTC | #16
On Mon, Jan 18, 2021 at 4:15 AM Maxime Ripard <maxime@cerno.tech> wrote:
>

> Hi Rob,

>

> On Thu, Jan 14, 2021 at 07:37:14PM -0600, Rob Herring wrote:

> > On Thu, Jan 14, 2021 at 12:35:24PM +0100, Maxime Ripard wrote:

> > > additionalProperties prevent any property not explicitly defined in the

> > > binding to be used. Yet, some serial properties like max-speed are valid

> > > and validated through the serial/serial.yaml binding.

> > >

> > > Let's change additionalProperties to unevaluatedProperties to avoid

> > > spurious warnings.

> > >

> > > Cc: Alistair Francis <alistair@alistair23.me>

> > > Cc: Vasily Khoruzhick <anarsoul@gmail.com>

> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> > > ---

> > >  Documentation/devicetree/bindings/net/realtek-bluetooth.yaml | 2 +-

> > >  1 file changed, 1 insertion(+), 1 deletion(-)

> > >

> > > diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> > > index 4f485df69ac3..f4d4969d87f4 100644

> > > --- a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> > > +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml

> > > @@ -37,7 +37,7 @@ properties:

> > >  required:

> > >    - compatible

> > >

> > > -additionalProperties: false

> > > +unevaluatedProperties: false

> >

> > This would still fail because the serial schema is applied to the parent

> > and this schema is applied to the child node. It's a common problem for

> > how we've done bus schemas. We'd need to split them into 2 schemas and

> > reference the child schema here. I'd rather not do that here because

> > then we'd apply the schema twice assuming we keep bus schemas also

> > applying the child schemas (which is good at least until we have schemas

> > for *all* possible child devices).

>

> I couldn't find what requires additionalProperties in the meta-schemas,

> could you point me to what enforces it?


https://github.com/devicetree-org/dt-schema/commit/14db51af82b0ab52f4a69f9aaa86726087b3022a

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index 7ea4d9645e93..08607c7ec1bf 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -657,7 +657,8 @@  properties:
       - description: Pine64 PineCube
         items:
           - const: pine64,pinecube
-          - const: allwinner,sun8i-s3
+          - const: sochip,s3
+          - const: allwinner,sun8i-v3
 
       - description: Pine64 PineH64 model A
         items: