devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci
@ 2024-06-25 20:57 Frank Li
  2024-06-26  8:17 ` Krzysztof Kozlowski
  2024-07-12 10:17 ` Niklas Cassel
  0 siblings, 2 replies; 6+ messages in thread
From: Frank Li @ 2024-06-25 20:57 UTC (permalink / raw)
  To: Damien Le Moal, Niklas Cassel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
  Cc: imx

Add compatible string 'fsl,ls1046a-ahci' and 'fsl,ls1012a-ahci' compatible
string. Allow 'fsl,ls1012a-ahci' fallback to 'fsl,ls1043a-ahci'.

ls1046a ahci ecc disable bit is difference with other chips.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
index 162b3bb5427ed..a244bc603549d 100644
--- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
+++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
@@ -11,13 +11,18 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - fsl,ls1021a-ahci
-      - fsl,ls1043a-ahci
-      - fsl,ls1028a-ahci
-      - fsl,ls1088a-ahci
-      - fsl,ls2080a-ahci
-      - fsl,lx2160a-ahci
+    oneOf:
+      - items:
+          - const: fsl,ls1012a-ahci
+          - const: fsl,ls1043a-ahci
+      - enum:
+          - fsl,ls1021a-ahci
+          - fsl,ls1043a-ahci
+          - fsl,ls1046a-ahci
+          - fsl,ls1028a-ahci
+          - fsl,ls1088a-ahci
+          - fsl,ls2080a-ahci
+          - fsl,lx2160a-ahci
 
   reg:
     minItems: 1
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci
  2024-06-25 20:57 [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci Frank Li
@ 2024-06-26  8:17 ` Krzysztof Kozlowski
  2024-07-02 20:45   ` Frank Li
  2024-07-12 10:17 ` Niklas Cassel
  1 sibling, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2024-06-26  8:17 UTC (permalink / raw)
  To: Frank Li, Damien Le Moal, Niklas Cassel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
  Cc: imx

On 25/06/2024 22:57, Frank Li wrote:
> Add compatible string 'fsl,ls1046a-ahci' and 'fsl,ls1012a-ahci' compatible
> string. Allow 'fsl,ls1012a-ahci' fallback to 'fsl,ls1043a-ahci'.
> 
> ls1046a ahci ecc disable bit is difference with other chips.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> index 162b3bb5427ed..a244bc603549d 100644
> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> @@ -11,13 +11,18 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - fsl,ls1021a-ahci
> -      - fsl,ls1043a-ahci
> -      - fsl,ls1028a-ahci
> -      - fsl,ls1088a-ahci
> -      - fsl,ls2080a-ahci
> -      - fsl,lx2160a-ahci
> +    oneOf:
> +      - items:
> +          - const: fsl,ls1012a-ahci
> +          - const: fsl,ls1043a-ahci
> +      - enum:
> +          - fsl,ls1021a-ahci
> +          - fsl,ls1043a-ahci
> +          - fsl,ls1046a-ahci

Where is the driver change for this?

Your commit does not explain why you are doing it and without driver
change adding new support it is not obvious. This probably applies to
all your patches.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci
  2024-06-26  8:17 ` Krzysztof Kozlowski
@ 2024-07-02 20:45   ` Frank Li
  2024-07-05  0:20     ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Frank Li @ 2024-07-02 20:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Damien Le Moal, Niklas Cassel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, imx

On Wed, Jun 26, 2024 at 10:17:55AM +0200, Krzysztof Kozlowski wrote:
> On 25/06/2024 22:57, Frank Li wrote:
> > Add compatible string 'fsl,ls1046a-ahci' and 'fsl,ls1012a-ahci' compatible
> > string. Allow 'fsl,ls1012a-ahci' fallback to 'fsl,ls1043a-ahci'.
> > 
> > ls1046a ahci ecc disable bit is difference with other chips.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > index 162b3bb5427ed..a244bc603549d 100644
> > --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > @@ -11,13 +11,18 @@ maintainers:
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - fsl,ls1021a-ahci
> > -      - fsl,ls1043a-ahci
> > -      - fsl,ls1028a-ahci
> > -      - fsl,ls1088a-ahci
> > -      - fsl,ls2080a-ahci
> > -      - fsl,lx2160a-ahci
> > +    oneOf:
> > +      - items:
> > +          - const: fsl,ls1012a-ahci
> > +          - const: fsl,ls1043a-ahci
> > +      - enum:
> > +          - fsl,ls1021a-ahci
> > +          - fsl,ls1043a-ahci
> > +          - fsl,ls1046a-ahci
> 
> Where is the driver change for this?
> 
> Your commit does not explain why you are doing it and without driver
> change adding new support it is not obvious. This probably applies to
> all your patches.

I think I missed ls1012a and ls1021a.  Commit message is wrong. This is
for legancy platorm. 

Basic try to eliminate the CHECK_DTBS warning. ls1012a use

"fsl,ls1012a-ahci", "fsl,ls1043a-ahci". There are two methods, 
1. change binding doc to allow "fsl,ls1012a-ahci", "fsl,ls1043a-ahci"
2. remove "fsl,ls1012a-ahci".

which way do you perfer?

Frank

> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci
  2024-07-02 20:45   ` Frank Li
@ 2024-07-05  0:20     ` Damien Le Moal
  2024-07-05  6:06       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2024-07-05  0:20 UTC (permalink / raw)
  To: Frank Li, Krzysztof Kozlowski
  Cc: Niklas Cassel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, imx

On 7/3/24 05:45, Frank Li wrote:
> On Wed, Jun 26, 2024 at 10:17:55AM +0200, Krzysztof Kozlowski wrote:
>> On 25/06/2024 22:57, Frank Li wrote:
>>> Add compatible string 'fsl,ls1046a-ahci' and 'fsl,ls1012a-ahci' compatible
>>> string. Allow 'fsl,ls1012a-ahci' fallback to 'fsl,ls1043a-ahci'.
>>>
>>> ls1046a ahci ecc disable bit is difference with other chips.
>>>
>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>> ---
>>>  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
>>> index 162b3bb5427ed..a244bc603549d 100644
>>> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
>>> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
>>> @@ -11,13 +11,18 @@ maintainers:
>>>  
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - fsl,ls1021a-ahci
>>> -      - fsl,ls1043a-ahci
>>> -      - fsl,ls1028a-ahci
>>> -      - fsl,ls1088a-ahci
>>> -      - fsl,ls2080a-ahci
>>> -      - fsl,lx2160a-ahci
>>> +    oneOf:
>>> +      - items:
>>> +          - const: fsl,ls1012a-ahci
>>> +          - const: fsl,ls1043a-ahci
>>> +      - enum:
>>> +          - fsl,ls1021a-ahci
>>> +          - fsl,ls1043a-ahci
>>> +          - fsl,ls1046a-ahci
>>
>> Where is the driver change for this?
>>
>> Your commit does not explain why you are doing it and without driver
>> change adding new support it is not obvious. This probably applies to
>> all your patches.
> 
> I think I missed ls1012a and ls1021a.  Commit message is wrong. This is
> for legancy platorm. 
> 
> Basic try to eliminate the CHECK_DTBS warning. ls1012a use
> 
> "fsl,ls1012a-ahci", "fsl,ls1043a-ahci". There are two methods, 
> 1. change binding doc to allow "fsl,ls1012a-ahci", "fsl,ls1043a-ahci"

But then shouldn't you also change the drivers/ata/ahci_qoriq.c to add ls1012a
as a compatible ?

> 2. remove "fsl,ls1012a-ahci".

I am not sure if that is acceptable since there is one device tree using it out
there already.

I am no DT expert, but I think (1) with the driver change is the right approach.
Krzysztof ?

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci
  2024-07-05  0:20     ` Damien Le Moal
@ 2024-07-05  6:06       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05  6:06 UTC (permalink / raw)
  To: Damien Le Moal, Frank Li
  Cc: Niklas Cassel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, imx

On 05/07/2024 02:20, Damien Le Moal wrote:
> On 7/3/24 05:45, Frank Li wrote:
>> On Wed, Jun 26, 2024 at 10:17:55AM +0200, Krzysztof Kozlowski wrote:
>>> On 25/06/2024 22:57, Frank Li wrote:
>>>> Add compatible string 'fsl,ls1046a-ahci' and 'fsl,ls1012a-ahci' compatible
>>>> string. Allow 'fsl,ls1012a-ahci' fallback to 'fsl,ls1043a-ahci'.
>>>>
>>>> ls1046a ahci ecc disable bit is difference with other chips.
>>>>
>>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>>> ---
>>>>  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
>>>> index 162b3bb5427ed..a244bc603549d 100644
>>>> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
>>>> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
>>>> @@ -11,13 +11,18 @@ maintainers:
>>>>  
>>>>  properties:
>>>>    compatible:
>>>> -    enum:
>>>> -      - fsl,ls1021a-ahci
>>>> -      - fsl,ls1043a-ahci
>>>> -      - fsl,ls1028a-ahci
>>>> -      - fsl,ls1088a-ahci
>>>> -      - fsl,ls2080a-ahci
>>>> -      - fsl,lx2160a-ahci
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - const: fsl,ls1012a-ahci
>>>> +          - const: fsl,ls1043a-ahci
>>>> +      - enum:
>>>> +          - fsl,ls1021a-ahci
>>>> +          - fsl,ls1043a-ahci
>>>> +          - fsl,ls1046a-ahci
>>>
>>> Where is the driver change for this?
>>>
>>> Your commit does not explain why you are doing it and without driver
>>> change adding new support it is not obvious. This probably applies to
>>> all your patches.
>>
>> I think I missed ls1012a and ls1021a.  Commit message is wrong. This is
>> for legancy platorm. 
>>
>> Basic try to eliminate the CHECK_DTBS warning. ls1012a use
>>
>> "fsl,ls1012a-ahci", "fsl,ls1043a-ahci". There are two methods, 
>> 1. change binding doc to allow "fsl,ls1012a-ahci", "fsl,ls1043a-ahci"
> 
> But then shouldn't you also change the drivers/ata/ahci_qoriq.c to add ls1012a
> as a compatible ?

The fallback will be used by the driver, so there is no need to add
front compatibles to of_device_id table.

> 
>> 2. remove "fsl,ls1012a-ahci".
> 
> I am not sure if that is acceptable since there is one device tree using it out
> there already.
> 
> I am no DT expert, but I think (1) with the driver change is the right approach.
> Krzysztof ?

Yep, this cannot be removed.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci
  2024-06-25 20:57 [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci Frank Li
  2024-06-26  8:17 ` Krzysztof Kozlowski
@ 2024-07-12 10:17 ` Niklas Cassel
  1 sibling, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2024-07-12 10:17 UTC (permalink / raw)
  To: Frank Li
  Cc: Damien Le Moal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, imx

On Tue, Jun 25, 2024 at 04:57:52PM -0400, Frank Li wrote:
> Add compatible string 'fsl,ls1046a-ahci' and 'fsl,ls1012a-ahci' compatible
> string. Allow 'fsl,ls1012a-ahci' fallback to 'fsl,ls1043a-ahci'.
> 
> ls1046a ahci ecc disable bit is difference with other chips.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> index 162b3bb5427ed..a244bc603549d 100644
> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> @@ -11,13 +11,18 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - fsl,ls1021a-ahci
> -      - fsl,ls1043a-ahci
> -      - fsl,ls1028a-ahci
> -      - fsl,ls1088a-ahci
> -      - fsl,ls2080a-ahci
> -      - fsl,lx2160a-ahci
> +    oneOf:
> +      - items:
> +          - const: fsl,ls1012a-ahci
> +          - const: fsl,ls1043a-ahci
> +      - enum:
> +          - fsl,ls1021a-ahci
> +          - fsl,ls1043a-ahci
> +          - fsl,ls1046a-ahci
> +          - fsl,ls1028a-ahci
> +          - fsl,ls1088a-ahci
> +          - fsl,ls2080a-ahci
> +          - fsl,lx2160a-ahci
>  
>    reg:
>      minItems: 1
> -- 
> 2.34.1
> 

Frank,

if the check_dts warning is still there,
will you submit a new patch with a better commit message that explains that
the patch fixes the initial commit that converted the binding to yaml?

Such that DT maintainers can review your v2 patch.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-07-12 10:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-25 20:57 [PATCH 1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci Frank Li
2024-06-26  8:17 ` Krzysztof Kozlowski
2024-07-02 20:45   ` Frank Li
2024-07-05  0:20     ` Damien Le Moal
2024-07-05  6:06       ` Krzysztof Kozlowski
2024-07-12 10:17 ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).