diff mbox series

[1/3] ASoC: dt-bindings: fsl_easrc: Add support for imx8mp-easrc

Message ID 20230820175655.206723-1-aford173@gmail.com (mailing list archive)
State Superseded
Headers show
Series [1/3] ASoC: dt-bindings: fsl_easrc: Add support for imx8mp-easrc | expand

Commit Message

Adam Ford Aug. 20, 2023, 5:56 p.m. UTC
The i.MX8MP appears to have the same easrc support as the Nano, so
add imx8mp as an option with a fallback to imx8mn.

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Rob Herring (Arm) Aug. 20, 2023, 6:28 p.m. UTC | #1
On Sun, 20 Aug 2023 12:56:53 -0500, Adam Ford wrote:
> The i.MX8MP appears to have the same easrc support as the Nano, so
> add imx8mp as an option with a fallback to imx8mn.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/fsl,easrc.example.dtb: easrc@300c0000: compatible: 'oneOf' conditional failed, one must be fixed:
	['fsl,imx8mn-easrc'] is too short
	'fsl,imx8mn-easrc' is not one of ['fsl,imx8mp-easrc']
	from schema $id: http://devicetree.org/schemas/sound/fsl,easrc.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230820175655.206723-1-aford173@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski Aug. 20, 2023, 8:32 p.m. UTC | #2
On 20/08/2023 19:56, Adam Ford wrote:
> The i.MX8MP appears to have the same easrc support as the Nano, so
> add imx8mp as an option with a fallback to imx8mn.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> index bdde68a1059c..2d53b3b10f2c 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> @@ -14,7 +14,11 @@ properties:
>      pattern: "^easrc@.*"
>  
>    compatible:
> -    const: fsl,imx8mn-easrc
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,imx8mp-easrc
> +          - const: fsl,imx8mn-easrc

You need here also const for fsl,imx8mn-easrc, otherwise you do not
allow it alone. Test it for fsl,imx8mn-easrc DTS - you will notice warnings.

Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 20, 2023, 8:33 p.m. UTC | #3
On 20/08/2023 22:32, Krzysztof Kozlowski wrote:
> On 20/08/2023 19:56, Adam Ford wrote:
>> The i.MX8MP appears to have the same easrc support as the Nano, so
>> add imx8mp as an option with a fallback to imx8mn.
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>
>> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
>> index bdde68a1059c..2d53b3b10f2c 100644
>> --- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
>> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
>> @@ -14,7 +14,11 @@ properties:
>>      pattern: "^easrc@.*"
>>  
>>    compatible:
>> -    const: fsl,imx8mn-easrc
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - fsl,imx8mp-easrc
>> +          - const: fsl,imx8mn-easrc
> 
> You need here also const for fsl,imx8mn-easrc, otherwise you do not
> allow it alone. Test it for fsl,imx8mn-easrc DTS - you will notice warnings.

Actually, I see now Rob's report... you did not have to test DTS even.
It was enough to test your change and this test was missing :(. Please
test your changes before sending.

Best regards,
Krzysztof
Adam Ford Aug. 20, 2023, 9:05 p.m. UTC | #4
On Sun, Aug 20, 2023 at 3:33 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/08/2023 22:32, Krzysztof Kozlowski wrote:
> > On 20/08/2023 19:56, Adam Ford wrote:
> >> The i.MX8MP appears to have the same easrc support as the Nano, so
> >> add imx8mp as an option with a fallback to imx8mn.
> >>
> >> Signed-off-by: Adam Ford <aford173@gmail.com>
> >>
> >> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> >> index bdde68a1059c..2d53b3b10f2c 100644
> >> --- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> >> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> >> @@ -14,7 +14,11 @@ properties:
> >>      pattern: "^easrc@.*"
> >>
> >>    compatible:
> >> -    const: fsl,imx8mn-easrc
> >> +    oneOf:
> >> +      - items:
> >> +          - enum:
> >> +              - fsl,imx8mp-easrc
> >> +          - const: fsl,imx8mn-easrc
> >
> > You need here also const for fsl,imx8mn-easrc, otherwise you do not
> > allow it alone. Test it for fsl,imx8mn-easrc DTS - you will notice warnings.
>
> Actually, I see now Rob's report... you did not have to test DTS even.
> It was enough to test your change and this test was missing :(. Please
> test your changes before sending.

For what it's worth, I did run 'make dt_binding_check', but I didn't
run it with the extra flags from Rob's e-mail.  The tool didn't return
any errors.

adam

>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Aug. 21, 2023, 6:18 a.m. UTC | #5
On 20/08/2023 23:05, Adam Ford wrote:
> On Sun, Aug 20, 2023 at 3:33 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 20/08/2023 22:32, Krzysztof Kozlowski wrote:
>>> On 20/08/2023 19:56, Adam Ford wrote:
>>>> The i.MX8MP appears to have the same easrc support as the Nano, so
>>>> add imx8mp as an option with a fallback to imx8mn.
>>>>
>>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
>>>> index bdde68a1059c..2d53b3b10f2c 100644
>>>> --- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
>>>> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
>>>> @@ -14,7 +14,11 @@ properties:
>>>>      pattern: "^easrc@.*"
>>>>
>>>>    compatible:
>>>> -    const: fsl,imx8mn-easrc
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - enum:
>>>> +              - fsl,imx8mp-easrc
>>>> +          - const: fsl,imx8mn-easrc
>>>
>>> You need here also const for fsl,imx8mn-easrc, otherwise you do not
>>> allow it alone. Test it for fsl,imx8mn-easrc DTS - you will notice warnings.
>>
>> Actually, I see now Rob's report... you did not have to test DTS even.
>> It was enough to test your change and this test was missing :(. Please
>> test your changes before sending.
> 
> For what it's worth, I did run 'make dt_binding_check', but I didn't
> run it with the extra flags from Rob's e-mail.  The tool didn't return
> any errors.

OK, indeed without the additional flags the example from that binding
won't be reported as undocumented compatible.

Best regards,
Krzysztof
Rob Herring (Arm) Aug. 21, 2023, 2:06 p.m. UTC | #6
On Sun, Aug 20, 2023 at 04:05:16PM -0500, Adam Ford wrote:
> On Sun, Aug 20, 2023 at 3:33 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 20/08/2023 22:32, Krzysztof Kozlowski wrote:
> > > On 20/08/2023 19:56, Adam Ford wrote:
> > >> The i.MX8MP appears to have the same easrc support as the Nano, so
> > >> add imx8mp as an option with a fallback to imx8mn.
> > >>
> > >> Signed-off-by: Adam Ford <aford173@gmail.com>
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > >> index bdde68a1059c..2d53b3b10f2c 100644
> > >> --- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > >> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > >> @@ -14,7 +14,11 @@ properties:
> > >>      pattern: "^easrc@.*"
> > >>
> > >>    compatible:
> > >> -    const: fsl,imx8mn-easrc
> > >> +    oneOf:
> > >> +      - items:
> > >> +          - enum:
> > >> +              - fsl,imx8mp-easrc
> > >> +          - const: fsl,imx8mn-easrc
> > >
> > > You need here also const for fsl,imx8mn-easrc, otherwise you do not
> > > allow it alone. Test it for fsl,imx8mn-easrc DTS - you will notice warnings.
> >
> > Actually, I see now Rob's report... you did not have to test DTS even.
> > It was enough to test your change and this test was missing :(. Please
> > test your changes before sending.
> 
> For what it's worth, I did run 'make dt_binding_check', but I didn't
> run it with the extra flags from Rob's e-mail.  The tool didn't return
> any errors.

The error is not related to the '-m' (undocumented compatible) warning. 
It is as Krzysztof said.

Rob
Adam Ford Aug. 21, 2023, 3:03 p.m. UTC | #7
On Mon, Aug 21, 2023 at 9:06 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Aug 20, 2023 at 04:05:16PM -0500, Adam Ford wrote:
> > On Sun, Aug 20, 2023 at 3:33 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 20/08/2023 22:32, Krzysztof Kozlowski wrote:
> > > > On 20/08/2023 19:56, Adam Ford wrote:
> > > >> The i.MX8MP appears to have the same easrc support as the Nano, so
> > > >> add imx8mp as an option with a fallback to imx8mn.
> > > >>
> > > >> Signed-off-by: Adam Ford <aford173@gmail.com>
> > > >>
> > > >> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > > >> index bdde68a1059c..2d53b3b10f2c 100644
> > > >> --- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > > >> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > > >> @@ -14,7 +14,11 @@ properties:
> > > >>      pattern: "^easrc@.*"
> > > >>
> > > >>    compatible:
> > > >> -    const: fsl,imx8mn-easrc
> > > >> +    oneOf:
> > > >> +      - items:
> > > >> +          - enum:
> > > >> +              - fsl,imx8mp-easrc
> > > >> +          - const: fsl,imx8mn-easrc
> > > >
> > > > You need here also const for fsl,imx8mn-easrc, otherwise you do not
> > > > allow it alone. Test it for fsl,imx8mn-easrc DTS - you will notice warnings.
> > >
> > > Actually, I see now Rob's report... you did not have to test DTS even.
> > > It was enough to test your change and this test was missing :(. Please
> > > test your changes before sending.
> >
> > For what it's worth, I did run 'make dt_binding_check', but I didn't
> > run it with the extra flags from Rob's e-mail.  The tool didn't return
> > any errors.
>
> The error is not related to the '-m' (undocumented compatible) warning.
> It is as Krzysztof said.

I was able to replicate the message after I updated the schema.  Is
there any way we can add the dt_binding_check to the 'make help' menu?
 I do this so infrequently that I don't necessarily know what the
proper flags are, and I sometimes forget to update the schema.  I have
seen others run into similar issues, so it seems like having it in the
help menu might be beneficial to more people.

adam

>
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
index bdde68a1059c..2d53b3b10f2c 100644
--- a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
@@ -14,7 +14,11 @@  properties:
     pattern: "^easrc@.*"
 
   compatible:
-    const: fsl,imx8mn-easrc
+    oneOf:
+      - items:
+          - enum:
+              - fsl,imx8mp-easrc
+          - const: fsl,imx8mn-easrc
 
   reg:
     maxItems: 1