| Message ID | 20240625205752.4007067-1-Frank.Li@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci | expand |
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
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 >
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 ?
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
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
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
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(-)