diff mbox series

[1/3] dt-bindings: apple,aic: Document A7-A11 compatibles

Message ID 20240829092610.89731-2-towinchenmi@gmail.com (mailing list archive)
State Superseded, archived
Headers show
Series Add AIC support for A7-A11 SoCs | expand

Commit Message

Nick Chan Aug. 29, 2024, 9:03 a.m. UTC
Document the compatibles for Apple A7-A11 SoCs.

There are three feature levels:
- A7-A10: No fast IPI
- A11: fast IPI, global only
- M1: fast IPI with local and global support

Each feature level is an extension of the previous. For example, M1 will
also work with the A7 feature level.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 .../bindings/interrupt-controller/apple,aic.yaml  | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Rob Herring Aug. 29, 2024, 10:24 a.m. UTC | #1
On Thu, 29 Aug 2024 17:03:11 +0800, Nick Chan wrote:
> Document the compatibles for Apple A7-A11 SoCs.
> 
> There are three feature levels:
> - A7-A10: No fast IPI
> - A11: fast IPI, global only
> - M1: fast IPI with local and global support
> 
> Each feature level is an extension of the previous. For example, M1 will
> also work with the A7 feature level.
> 
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  .../bindings/interrupt-controller/apple,aic.yaml  | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/interrupt-controller/apple,aic.example.dtb: interrupt-controller@23b100000: compatible: 'oneOf' conditional failed, one must be fixed:
	['apple,t8103-aic', 'apple,aic'] is too long
	['apple,t8103-aic', 'apple,aic'] is too short
	'apple,s5l8960x-aic' was expected
	'apple,t8103-aic' is not one of ['apple,s8000-aic', 'apple,t7000-aic', 'apple,t8010-aic']
	'apple,t8015-aic' was expected
	from schema $id: http://devicetree.org/schemas/interrupt-controller/apple,aic.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240829092610.89731-2-towinchenmi@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.
Nick Chan Aug. 29, 2024, 11:09 a.m. UTC | #2
On 29/8/2024 18:24, Rob Herring (Arm) wrote:
> 
> On Thu, 29 Aug 2024 17:03:11 +0800, Nick Chan wrote:
>> Document the compatibles for Apple A7-A11 SoCs.
>>
>> There are three feature levels:
>> - A7-A10: No fast IPI
>> - A11: fast IPI, global only
>> - M1: fast IPI with local and global support
>>
>> Each feature level is an extension of the previous. For example, M1 will
>> also work with the A7 feature level.
>>
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>> ---
>>  .../bindings/interrupt-controller/apple,aic.yaml  | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/interrupt-controller/apple,aic.example.dtb: interrupt-controller@23b100000: compatible: 'oneOf' conditional failed, one must be fixed:
> 	['apple,t8103-aic', 'apple,aic'] is too long
> 	['apple,t8103-aic', 'apple,aic'] is too short
> 	'apple,s5l8960x-aic' was expected
> 	'apple,t8103-aic' is not one of ['apple,s8000-aic', 'apple,t7000-aic', 'apple,t8010-aic']
> 	'apple,t8015-aic' was expected
> 	from schema $id: http://devicetree.org/schemas/interrupt-controller/apple,aic.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240829092610.89731-2-towinchenmi@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:
Acked. yamllint was not installed on my system.

> 
> 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.
> 

Nick Chan
Krzysztof Kozlowski Aug. 29, 2024, 11:43 a.m. UTC | #3
On 29/08/2024 13:09, Nick Chan wrote:
> 
> 
> On 29/8/2024 18:24, Rob Herring (Arm) wrote:
>>
>> On Thu, 29 Aug 2024 17:03:11 +0800, Nick Chan wrote:
>>> Document the compatibles for Apple A7-A11 SoCs.
>>>
>>> There are three feature levels:
>>> - A7-A10: No fast IPI
>>> - A11: fast IPI, global only
>>> - M1: fast IPI with local and global support
>>>
>>> Each feature level is an extension of the previous. For example, M1 will
>>> also work with the A7 feature level.
>>>
>>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>>> ---
>>>  .../bindings/interrupt-controller/apple,aic.yaml  | 15 ++++++++++++---
>>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/interrupt-controller/apple,aic.example.dtb: interrupt-controller@23b100000: compatible: 'oneOf' conditional failed, one must be fixed:
>> 	['apple,t8103-aic', 'apple,aic'] is too long
>> 	['apple,t8103-aic', 'apple,aic'] is too short
>> 	'apple,s5l8960x-aic' was expected
>> 	'apple,t8103-aic' is not one of ['apple,s8000-aic', 'apple,t7000-aic', 'apple,t8010-aic']
>> 	'apple,t8015-aic' was expected
>> 	from schema $id: http://devicetree.org/schemas/interrupt-controller/apple,aic.yaml#
>>
>> doc reference errors (make refcheckdocs):
>>
>> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240829092610.89731-2-towinchenmi@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:
> Acked. yamllint was not installed on my system.
> 

The error are not coming from yamllint. It's not about yamllint. You
just never tested the binding.

Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 29, 2024, 11:44 a.m. UTC | #4
On 29/08/2024 11:03, Nick Chan wrote:
> Document the compatibles for Apple A7-A11 SoCs.
> 
> There are three feature levels:
> - A7-A10: No fast IPI
> - A11: fast IPI, global only
> - M1: fast IPI with local and global support
> 
> Each feature level is an extension of the previous. For example, M1 will
> also work with the A7 feature level.

It's hard for me to map above to compatibles. Extend the commit msg to
include names used in the bindings.

> 
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  .../bindings/interrupt-controller/apple,aic.yaml  | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> index 698588e9aa86..28e09b933087 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> @@ -36,9 +36,18 @@ allOf:
>  
>  properties:
>    compatible:
> -    items:
> -      - const: apple,t8103-aic
> -      - const: apple,aic
> +    oneOf:
> +      - const: apple,s5l8960x-aic

Which one is this?

> +      - items:
> +          - enum:
> +              - apple,s8000-aic
> +              - apple,t7000-aic
> +              - apple,t8010-aic
> +          - const: apple,s5l8960x-aic
> +      - items:
> +          - const: apple,t8103-aic
> +          - const: apple,t8015-aic

Why are you changing all existing devices? Test your change, you would
see here errors.



Best regards,
Krzysztof
Nick Chan Aug. 29, 2024, 12:42 p.m. UTC | #5
On 29/8/2024 19:44, Krzysztof Kozlowski wrote:
> On 29/08/2024 11:03, Nick Chan wrote:
>> Document the compatibles for Apple A7-A11 SoCs.
>>
>> There are three feature levels:
>> - A7-A10: No fast IPI
>> - A11: fast IPI, global only
>> - M1: fast IPI with local and global support
>>
>> Each feature level is an extension of the previous. For example, M1 will
>> also work with the A7 feature level.
> 
> It's hard for me to map above to compatibles. Extend the commit msg to
> include names used in the bindings.
Acked. Will be in v2. A description of the feature levels will also be
added to
the description of AIC.

> 
>>
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>> ---
>>  .../bindings/interrupt-controller/apple,aic.yaml  | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>> index 698588e9aa86..28e09b933087 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>> @@ -36,9 +36,18 @@ allOf:
>>  
>>  properties:
>>    compatible:
>> -    items:
>> -      - const: apple,t8103-aic
>> -      - const: apple,aic
>> +    oneOf:
>> +      - const: apple,s5l8960x-aic
> 
> Which one is this?
> 
>> +      - items:
>> +          - enum:
>> +              - apple,s8000-aic
>> +              - apple,t7000-aic
>> +              - apple,t8010-aic
>> +          - const: apple,s5l8960x-aic
>> +      - items:
>> +          - const: apple,t8103-aic
>> +          - const: apple,t8015-aic
> 
> Why are you changing all existing devices? Test your change, you would
> see here errors.
This part is a bit of a mess from when A11 was supposed to get the M1
compatible. However, now that A11 is found out to be different from M1,
in v2 all of A7-M1 will get its own
SoC-specific compatible, in addition to the "apple,aic" generic
fallback. (you should have already seen the changes in the improper
"resend")

> 
> 
> 
> Best regards,
> Krzysztof
> 

Nick Chan
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
index 698588e9aa86..28e09b933087 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
@@ -36,9 +36,18 @@  allOf:
 
 properties:
   compatible:
-    items:
-      - const: apple,t8103-aic
-      - const: apple,aic
+    oneOf:
+      - const: apple,s5l8960x-aic
+      - items:
+          - enum:
+              - apple,s8000-aic
+              - apple,t7000-aic
+              - apple,t8010-aic
+          - const: apple,s5l8960x-aic
+      - items:
+          - const: apple,t8103-aic
+          - const: apple,t8015-aic
+          - const: apple,aic
 
   interrupt-controller: true