2022-05-12 15:25:30

by Serge Semin

[permalink] [raw]
Subject: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

Indeed in accordance with what is imeplemtned in the AHCI paltform driver
and the way the AHCI DT nodes are defined in the DT files we can add the
next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
by design.

Signed-off-by: Serge Semin <[email protected]>

Changelog v2:
- This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.
---
.../devicetree/bindings/ata/ahci-common.yaml | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/ata/ahci-common.yaml b/Documentation/devicetree/bindings/ata/ahci-common.yaml
index 620042ca12e7..a7d1a8353de3 100644
--- a/Documentation/devicetree/bindings/ata/ahci-common.yaml
+++ b/Documentation/devicetree/bindings/ata/ahci-common.yaml
@@ -31,6 +31,8 @@ properties:

reg-names:
description: CSR space IDs
+ contains:
+ const: ahci

interrupts:
description:
@@ -71,14 +73,13 @@ properties:
maxItems: 1

phy-names:
- maxItems: 1
+ const: sata-phy

ports-implemented:
$ref: '/schemas/types.yaml#/definitions/uint32'
description:
Mask that indicates which ports the HBA supports. Useful if PI is not
programmed by the BIOS, which is true for some embedded SoC's.
- maximum: 0x1f

patternProperties:
"^sata-port@[0-9a-f]+$":
@@ -89,8 +90,12 @@ patternProperties:

properties:
reg:
- description: AHCI SATA port identifier
- maxItems: 1
+ description:
+ AHCI SATA port identifier. By design AHCI controller can't have
+ more than 32 ports due to the CAP.NP fields and PI register size
+ constraints.
+ minimum: 0
+ maximum: 31

phys:
description: Individual AHCI SATA port PHY
@@ -98,7 +103,7 @@ patternProperties:

phy-names:
description: AHCI SATA port PHY ID
- maxItems: 1
+ const: sata-phy

target-supply:
description: Power regulator for SATA port target device
--
2.35.1



2022-05-13 13:19:33

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

On Thu, May 12, 2022 at 08:21:42AM +0200, Hannes Reinecke wrote:
> On 5/12/22 01:17, Serge Semin wrote:
> > Indeed in accordance with what is imeplemtned in the AHCI paltform driver
>

> Spelling; 'imeplemtned' and 'paltform'

Ok. I'll fix it in v3.

>
> > and the way the AHCI DT nodes are defined in the DT files we can add the
> > next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
> > name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
> > by design.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> >
> > Changelog v2:
> > - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.
> > ---
> > .../devicetree/bindings/ata/ahci-common.yaml | 15 ++++++++++-----
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/ata/ahci-common.yaml b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> > index 620042ca12e7..a7d1a8353de3 100644
> > --- a/Documentation/devicetree/bindings/ata/ahci-common.yaml
> > +++ b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> > @@ -31,6 +31,8 @@ properties:
> > reg-names:
> > description: CSR space IDs
> > + contains:
> > + const: ahci
> > interrupts:
> > description:
> > @@ -71,14 +73,13 @@ properties:
> > maxItems: 1
> > phy-names:
> > - maxItems: 1
> > + const: sata-phy
> > ports-implemented:
> > $ref: '/schemas/types.yaml#/definitions/uint32'
> > description:
> > Mask that indicates which ports the HBA supports. Useful if PI is not
> > programmed by the BIOS, which is true for some embedded SoC's.
> > - maximum: 0x1f
> > patternProperties:
> > "^sata-port@[0-9a-f]+$":
> > @@ -89,8 +90,12 @@ patternProperties:
> > properties:
> > reg:
> > - description: AHCI SATA port identifier
> > - maxItems: 1
> > + description:
> > + AHCI SATA port identifier. By design AHCI controller can't have
> > + more than 32 ports due to the CAP.NP fields and PI register size
> > + constraints.
> > + minimum: 0
> > + maximum: 31
> > phys:
> > description: Individual AHCI SATA port PHY
> > @@ -98,7 +103,7 @@ patternProperties:
> > phy-names:
> > description: AHCI SATA port PHY ID
> > - maxItems: 1
> > + const: sata-phy
> > target-supply:
> > description: Power regulator for SATA port target device
>

> Other than that it looks okay.
>
> Reviewed-by: Hannes Reinecke <[email protected]>

Thanks.

-Sergey

>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke Kernel Storage Architect
> [email protected] +49 911 74053 688
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 N?rnberg
> HRB 36809 (AG N?rnberg), GF: Felix Imend?rffer

2022-05-14 00:13:00

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

On Thu, May 12, 2022 at 11:11:22AM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 5/12/22 2:17 AM, Serge Semin wrote:
>
> > Indeed in accordance with what is imeplemtned in the AHCI paltform driver
>

> Implemented? :-)

The "paltform" word is misspelled too. Thanks for noting.

>
> > and the way the AHCI DT nodes are defined in the DT files we can add the
> > next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
> > name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
> > by design.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> >
> > Changelog v2:
> > - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.
>

> This normally goes after ---...

Right. I've got it noticed too on taking the @Hannes comments into
account.

All of the denoted problems will be fixed in v3.

-Sergey

>
> [...]
>
> MBR, Sergey

2022-05-14 02:49:38

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

On 5/12/22 01:17, Serge Semin wrote:
> Indeed in accordance with what is imeplemtned in the AHCI paltform driver

Spelling; 'imeplemtned' and 'paltform'

> and the way the AHCI DT nodes are defined in the DT files we can add the
> next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
> name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
> by design.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> Changelog v2:
> - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.
> ---
> .../devicetree/bindings/ata/ahci-common.yaml | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-common.yaml b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> index 620042ca12e7..a7d1a8353de3 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-common.yaml
> +++ b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> @@ -31,6 +31,8 @@ properties:
>
> reg-names:
> description: CSR space IDs
> + contains:
> + const: ahci
>
> interrupts:
> description:
> @@ -71,14 +73,13 @@ properties:
> maxItems: 1
>
> phy-names:
> - maxItems: 1
> + const: sata-phy
>
> ports-implemented:
> $ref: '/schemas/types.yaml#/definitions/uint32'
> description:
> Mask that indicates which ports the HBA supports. Useful if PI is not
> programmed by the BIOS, which is true for some embedded SoC's.
> - maximum: 0x1f
>
> patternProperties:
> "^sata-port@[0-9a-f]+$":
> @@ -89,8 +90,12 @@ patternProperties:
>
> properties:
> reg:
> - description: AHCI SATA port identifier
> - maxItems: 1
> + description:
> + AHCI SATA port identifier. By design AHCI controller can't have
> + more than 32 ports due to the CAP.NP fields and PI register size
> + constraints.
> + minimum: 0
> + maximum: 31
>
> phys:
> description: Individual AHCI SATA port PHY
> @@ -98,7 +103,7 @@ patternProperties:
>
> phy-names:
> description: AHCI SATA port PHY ID
> - maxItems: 1
> + const: sata-phy
>
> target-supply:
> description: Power regulator for SATA port target device

Other than that it looks okay.

Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

2022-05-14 03:36:39

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

Hello!

On 5/12/22 2:17 AM, Serge Semin wrote:

> Indeed in accordance with what is imeplemtned in the AHCI paltform driver

Implemented? :-)

> and the way the AHCI DT nodes are defined in the DT files we can add the
> next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
> name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
> by design.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> Changelog v2:
> - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.

This normally goes after ---...

[...]

MBR, Sergey

2022-05-18 04:45:31

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

On Thu, May 12, 2022 at 02:17:50AM +0300, Serge Semin wrote:
> Indeed in accordance with what is imeplemtned in the AHCI paltform driver
> and the way the AHCI DT nodes are defined in the DT files we can add the
> next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
> name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
> by design.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> Changelog v2:
> - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.
> ---
> .../devicetree/bindings/ata/ahci-common.yaml | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-common.yaml b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> index 620042ca12e7..a7d1a8353de3 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-common.yaml
> +++ b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> @@ -31,6 +31,8 @@ properties:
>
> reg-names:
> description: CSR space IDs
> + contains:
> + const: ahci

Okay, with this it makes sense to keep. The others still should go.

>
> interrupts:
> description:
> @@ -71,14 +73,13 @@ properties:
> maxItems: 1
>
> phy-names:
> - maxItems: 1
> + const: sata-phy
>
> ports-implemented:
> $ref: '/schemas/types.yaml#/definitions/uint32'
> description:
> Mask that indicates which ports the HBA supports. Useful if PI is not
> programmed by the BIOS, which is true for some embedded SoC's.
> - maximum: 0x1f
>
> patternProperties:
> "^sata-port@[0-9a-f]+$":
> @@ -89,8 +90,12 @@ patternProperties:
>
> properties:
> reg:
> - description: AHCI SATA port identifier
> - maxItems: 1
> + description:
> + AHCI SATA port identifier. By design AHCI controller can't have
> + more than 32 ports due to the CAP.NP fields and PI register size
> + constraints.
> + minimum: 0
> + maximum: 31
>
> phys:
> description: Individual AHCI SATA port PHY
> @@ -98,7 +103,7 @@ patternProperties:
>
> phy-names:
> description: AHCI SATA port PHY ID
> - maxItems: 1
> + const: sata-phy
>
> target-supply:
> description: Power regulator for SATA port target device
> --
> 2.35.1
>
>

2022-05-22 21:03:01

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v3 03/23] dt-bindings: ata: ahci-platform: Clarify common AHCI props constraints

On Tue, May 17, 2022 at 02:14:09PM -0500, Rob Herring wrote:
> On Thu, May 12, 2022 at 02:17:50AM +0300, Serge Semin wrote:
> > Indeed in accordance with what is imeplemtned in the AHCI paltform driver
> > and the way the AHCI DT nodes are defined in the DT files we can add the
> > next AHCI DT properties constraints: AHCI CSR ID is fixed to 'ahci', PHY
> > name is fixed to 'sata-phy', AHCI controller can't have more than 32 ports
> > by design.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> >
> > Changelog v2:
> > - This is a new patch created after rebasing v1 onto the 5.18-rc3 kernel.
> > ---
> > .../devicetree/bindings/ata/ahci-common.yaml | 15 ++++++++++-----
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/ata/ahci-common.yaml b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> > index 620042ca12e7..a7d1a8353de3 100644
> > --- a/Documentation/devicetree/bindings/ata/ahci-common.yaml
> > +++ b/Documentation/devicetree/bindings/ata/ahci-common.yaml
> > @@ -31,6 +31,8 @@ properties:
> >
> > reg-names:
> > description: CSR space IDs
> > + contains:
> > + const: ahci
>

> Okay, with this it makes sense to keep. The others still should go.

Ok.

-Sergey

>
> >
> > interrupts:
> > description:
> > @@ -71,14 +73,13 @@ properties:
> > maxItems: 1
> >
> > phy-names:
> > - maxItems: 1
> > + const: sata-phy
> >
> > ports-implemented:
> > $ref: '/schemas/types.yaml#/definitions/uint32'
> > description:
> > Mask that indicates which ports the HBA supports. Useful if PI is not
> > programmed by the BIOS, which is true for some embedded SoC's.
> > - maximum: 0x1f
> >
> > patternProperties:
> > "^sata-port@[0-9a-f]+$":
> > @@ -89,8 +90,12 @@ patternProperties:
> >
> > properties:
> > reg:
> > - description: AHCI SATA port identifier
> > - maxItems: 1
> > + description:
> > + AHCI SATA port identifier. By design AHCI controller can't have
> > + more than 32 ports due to the CAP.NP fields and PI register size
> > + constraints.
> > + minimum: 0
> > + maximum: 31
> >
> > phys:
> > description: Individual AHCI SATA port PHY
> > @@ -98,7 +103,7 @@ patternProperties:
> >
> > phy-names:
> > description: AHCI SATA port PHY ID
> > - maxItems: 1
> > + const: sata-phy
> >
> > target-supply:
> > description: Power regulator for SATA port target device
> > --
> > 2.35.1
> >
> >