Hi,
This enables SATA support for RK3588.
Changes since PATCHv2:
* https://lore.kernel.org/all/[email protected]/
* Drop patch 1 (applied by Heiko)
* Update SATA DT binding to split Rockchip into its own file
* Enforce correct resets numbers for the rk3568/rk3588 combo PHY
Changes since PATCHv1:
* https://lore.kernel.org/all/[email protected]/
* Rebase to v6.4-rc1
* Collect Acked-by for syscon DT binding update
* Use ASIC clock description suggested by Serge Semin
* Also add RBC clock (not used by RK3588)
* Add extra patch narrowing down the allowed clocks for RK356x and RK3588
-- Sebastian
Sebastian Reichel (5):
dt-bindings: ata: dwc-ahci: add PHY clocks
dt-bindings: ata: dwc-ahci: add Rockchip RK3588
dt-bindings: phy: rockchip: rk3588 has two reset lines
arm64: dts: rockchip: rk3588: add combo PHYs
arm64: dts: rockchip: rk3588: add SATA support
.../bindings/ata/rockchip,dwc-ahci.yaml | 114 ++++++++++++++++++
.../bindings/ata/snps,dwc-ahci-common.yaml | 8 +-
.../bindings/ata/snps,dwc-ahci.yaml | 17 ++-
.../phy/phy-rockchip-naneng-combphy.yaml | 34 +++++-
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 44 +++++++
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 90 ++++++++++++++
6 files changed, 298 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
--
2.39.2
Add PHY transmit and receive clocks as described by the
DW SATA AHCI HW manual.
Suggested-by: Serge Semin <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../devicetree/bindings/ata/snps,dwc-ahci-common.yaml | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
index c1457910520b..34c5bf65b02d 100644
--- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
+++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
@@ -31,11 +31,11 @@ properties:
PM-alive clock, RxOOB detection clock, embedded PHYs reference (Rx/Tx)
clock, etc.
minItems: 1
- maxItems: 4
+ maxItems: 6
clock-names:
minItems: 1
- maxItems: 4
+ maxItems: 6
items:
oneOf:
- description: Application APB/AHB/AXI BIU clock
@@ -48,6 +48,10 @@ properties:
const: pmalive
- description: RxOOB detection clock
const: rxoob
+ - description: PHY Transmit Clock
+ const: asic
+ - description: PHY Receive Clock
+ const: rbc
- description: SATA Ports reference clock
const: ref
--
2.39.2
The RK3588 has two reset lines for the combphy. One for the
APB interface and one for the actual PHY.
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../phy/phy-rockchip-naneng-combphy.yaml | 34 ++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
index 9ae514fa7533..d3cd7997879f 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
@@ -31,8 +31,14 @@ properties:
- const: pipe
resets:
+ minItems: 1
+ maxItems: 2
+
+ reset-names:
+ minItems: 1
items:
- - description: exclusive PHY reset line
+ - const: phy
+ - const: apb
rockchip,enable-ssc:
type: boolean
@@ -78,6 +84,32 @@ required:
- rockchip,pipe-phy-grf
- "#phy-cells"
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: rockchip,rk3568-naneng-combphy
+ then:
+ properties:
+ resets:
+ maxItems: 1
+ reset-names:
+ maxItems: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: rockchip,rk3588-naneng-combphy
+ then:
+ properties:
+ resets:
+ minItems: 2
+ reset-names:
+ minItems: 2
+ required:
+ - reset-names
+
additionalProperties: false
examples:
--
2.39.2
Add all 3 combo PHYs that can be found in RK3588.
They are used for SATA, PCIe or USB3.
Signed-off-by: Sebastian Reichel <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 21 ++++++++++++
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 42 +++++++++++++++++++++++
2 files changed, 63 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 8be75556af8f..9d8539b5309b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -7,6 +7,11 @@
#include "rk3588-pinctrl.dtsi"
/ {
+ pipe_phy1_grf: syscon@fd5c0000 {
+ compatible = "rockchip,rk3588-pipe-phy-grf", "syscon";
+ reg = <0x0 0xfd5c0000 0x0 0x100>;
+ };
+
i2s8_8ch: i2s@fddc8000 {
compatible = "rockchip,rk3588-i2s-tdm";
reg = <0x0 0xfddc8000 0x0 0x1000>;
@@ -123,4 +128,20 @@ gmac0_mtl_tx_setup: tx-queues-config {
queue1 {};
};
};
+
+ combphy1_ps: phy@fee10000 {
+ compatible = "rockchip,rk3588-naneng-combphy";
+ reg = <0x0 0xfee10000 0x0 0x100>;
+ #phy-cells = <1>;
+ clocks = <&cru CLK_REF_PIPE_PHY1>, <&cru PCLK_PCIE_COMBO_PIPE_PHY1>,
+ <&cru PCLK_PHP_ROOT>;
+ clock-names = "ref", "apb", "pipe";
+ assigned-clocks = <&cru CLK_REF_PIPE_PHY1>;
+ assigned-clock-rates = <100000000>;
+ resets = <&cru SRST_REF_PIPE_PHY1>, <&cru SRST_P_PCIE2_PHY1>;
+ reset-names = "phy", "apb";
+ rockchip,pipe-grf = <&php_grf>;
+ rockchip,pipe-phy-grf = <&pipe_phy1_grf>;
+ status = "disabled";
+ };
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 01058fed8f96..45ae457a22a4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -944,6 +944,16 @@ php_grf: syscon@fd5b0000 {
reg = <0x0 0xfd5b0000 0x0 0x1000>;
};
+ pipe_phy0_grf: syscon@fd5bc000 {
+ compatible = "rockchip,rk3588-pipe-phy-grf", "syscon";
+ reg = <0x0 0xfd5bc000 0x0 0x100>;
+ };
+
+ pipe_phy2_grf: syscon@fd5c4000 {
+ compatible = "rockchip,rk3588-pipe-phy-grf", "syscon";
+ reg = <0x0 0xfd5c4000 0x0 0x100>;
+ };
+
ioc: syscon@fd5f0000 {
compatible = "rockchip,rk3588-ioc", "syscon";
reg = <0x0 0xfd5f0000 0x0 0x10000>;
@@ -2371,6 +2381,38 @@ dmac2: dma-controller@fed10000 {
#dma-cells = <1>;
};
+ combphy0_ps: phy@fee00000 {
+ compatible = "rockchip,rk3588-naneng-combphy";
+ reg = <0x0 0xfee00000 0x0 0x100>;
+ #phy-cells = <1>;
+ clocks = <&cru CLK_REF_PIPE_PHY0>, <&cru PCLK_PCIE_COMBO_PIPE_PHY0>,
+ <&cru PCLK_PHP_ROOT>;
+ clock-names = "ref", "apb", "pipe";
+ assigned-clocks = <&cru CLK_REF_PIPE_PHY0>;
+ assigned-clock-rates = <100000000>;
+ resets = <&cru SRST_REF_PIPE_PHY0>, <&cru SRST_P_PCIE2_PHY0>;
+ reset-names = "phy", "apb";
+ rockchip,pipe-grf = <&php_grf>;
+ rockchip,pipe-phy-grf = <&pipe_phy0_grf>;
+ status = "disabled";
+ };
+
+ combphy2_psu: phy@fee20000 {
+ compatible = "rockchip,rk3588-naneng-combphy";
+ reg = <0x0 0xfee20000 0x0 0x100>;
+ #phy-cells = <1>;
+ clocks = <&cru CLK_REF_PIPE_PHY2>, <&cru PCLK_PCIE_COMBO_PIPE_PHY2>,
+ <&cru PCLK_PHP_ROOT>;
+ clock-names = "ref", "apb", "pipe";
+ assigned-clocks = <&cru CLK_REF_PIPE_PHY2>;
+ assigned-clock-rates = <100000000>;
+ resets = <&cru SRST_REF_PIPE_PHY2>, <&cru SRST_P_PCIE2_PHY2>;
+ reset-names = "phy", "apb";
+ rockchip,pipe-grf = <&php_grf>;
+ rockchip,pipe-phy-grf = <&pipe_phy2_grf>;
+ status = "disabled";
+ };
+
system_sram2: sram@ff001000 {
compatible = "mmio-sram";
reg = <0x0 0xff001000 0x0 0xef000>;
--
2.39.2
Add all three SATA IP blocks to the RK3588 DT.
Signed-off-by: Sebastian Reichel <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 23 +++++++++++
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 48 +++++++++++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 9d8539b5309b..b9508cea34f1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -129,6 +129,29 @@ gmac0_mtl_tx_setup: tx-queues-config {
};
};
+ sata1: sata@fe220000 {
+ compatible = "rockchip,rk3588-dwc-ahci", "snps,dwc-ahci";
+ reg = <0 0xfe220000 0 0x1000>;
+ clocks = <&cru ACLK_SATA1>, <&cru CLK_PMALIVE1>,
+ <&cru CLK_RXOOB1>, <&cru CLK_PIPEPHY1_REF>,
+ <&cru CLK_PIPEPHY1_PIPE_ASIC_G>;
+ clock-names = "sata", "pmalive", "rxoob", "ref", "asic";
+ interrupts = <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH 0>;
+ ports-implemented = <0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ sata-port@0 {
+ reg = <0>;
+ hba-port-cap = <HBA_PORT_FBSCP>;
+ phys = <&combphy1_ps PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ snps,rx-ts-max = <32>;
+ snps,tx-ts-max = <32>;
+ };
+ };
+
combphy1_ps: phy@fee10000 {
compatible = "rockchip,rk3588-naneng-combphy";
reg = <0x0 0xfee10000 0x0 0x100>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 45ae457a22a4..00a91b08e3bb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -9,6 +9,8 @@
#include <dt-bindings/power/rk3588-power.h>
#include <dt-bindings/reset/rockchip,rk3588-cru.h>
#include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/phy/phy.h>
+#include <dt-bindings/ata/ahci.h>
/ {
compatible = "rockchip,rk3588";
@@ -1717,6 +1719,52 @@ gmac1_mtl_tx_setup: tx-queues-config {
};
};
+ sata0: sata@fe210000 {
+ compatible = "rockchip,rk3588-dwc-ahci", "snps,dwc-ahci";
+ reg = <0 0xfe210000 0 0x1000>;
+ clocks = <&cru ACLK_SATA0>, <&cru CLK_PMALIVE0>,
+ <&cru CLK_RXOOB0>, <&cru CLK_PIPEPHY0_REF>,
+ <&cru CLK_PIPEPHY0_PIPE_ASIC_G>;
+ clock-names = "sata", "pmalive", "rxoob", "ref", "asic";
+ interrupts = <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH 0>;
+ ports-implemented = <0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ sata-port@0 {
+ reg = <0>;
+ hba-port-cap = <HBA_PORT_FBSCP>;
+ phys = <&combphy0_ps PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ snps,rx-ts-max = <32>;
+ snps,tx-ts-max = <32>;
+ };
+ };
+
+ sata2: sata@fe230000 {
+ compatible = "rockchip,rk3588-dwc-ahci", "snps,dwc-ahci";
+ reg = <0 0xfe230000 0 0x1000>;
+ clocks = <&cru ACLK_SATA2>, <&cru CLK_PMALIVE2>,
+ <&cru CLK_RXOOB2>, <&cru CLK_PIPEPHY2_REF>,
+ <&cru CLK_PIPEPHY2_PIPE_ASIC_G>;
+ clock-names = "sata", "pmalive", "rxoob", "ref", "asic";
+ interrupts = <GIC_SPI 275 IRQ_TYPE_LEVEL_HIGH 0>;
+ ports-implemented = <0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ sata-port@0 {
+ reg = <0>;
+ hba-port-cap = <HBA_PORT_FBSCP>;
+ phys = <&combphy2_psu PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ snps,rx-ts-max = <32>;
+ snps,tx-ts-max = <32>;
+ };
+ };
+
sdmmc: mmc@fe2c0000 {
compatible = "rockchip,rk3588-dw-mshc", "rockchip,rk3288-dw-mshc";
reg = <0x0 0xfe2c0000 0x0 0x4000>;
--
2.39.2
This adds Rockchip RK3588 AHCI binding. In order to narrow down the
allowed clocks without bloating the generic binding, the description
of Rockchip's AHCI controllers has been moved to its own file.
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../bindings/ata/rockchip,dwc-ahci.yaml | 114 ++++++++++++++++++
.../bindings/ata/snps,dwc-ahci.yaml | 17 ++-
2 files changed, 125 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
diff --git a/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
new file mode 100644
index 000000000000..86da9bd594a7
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ata/rockchip,dwc-ahci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DWC AHCI SATA controller for Rockchip devices
+
+maintainers:
+ - Serge Semin <[email protected]>
+
+description:
+ This document defines device tree bindings for the Synopsys DWC
+ implementation of the AHCI SATA controller found in Rockchip
+ devices.
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - rockchip,rk3568-dwc-ahci
+ - rockchip,rk3588-dwc-ahci
+ required:
+ - compatible
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - rockchip,rk3568-dwc-ahci
+ - rockchip,rk3588-dwc-ahci
+ - const: snps,dwc-ahci
+
+ ports-implemented:
+ const: 1
+
+patternProperties:
+ "^sata-port@[0-9a-e]$":
+ $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - ports-implemented
+
+allOf:
+ - $ref: snps,dwc-ahci-common.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - rockchip,rk3588-dwc-ahci
+ then:
+ properties:
+ clock-names:
+ items:
+ - const: sata
+ - const: pmalive
+ - const: rxoob
+ - const: ref
+ - const: asic
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - rockchip,rk3568-dwc-ahci
+ then:
+ properties:
+ clock-names:
+ items:
+ - const: sata
+ - const: pmalive
+ - const: rxoob
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/ata/ahci.h>
+ #include <dt-bindings/phy/phy.h>
+
+ sata@fe210000 {
+ compatible = "rockchip,rk3588-dwc-ahci", "snps,dwc-ahci";
+ reg = <0xfe210000 0x1000>;
+ clocks = <&cru ACLK_SATA0>, <&cru CLK_PMALIVE0>,
+ <&cru CLK_RXOOB0>, <&cru CLK_PIPEPHY0_REF>,
+ <&cru CLK_PIPEPHY0_PIPE_ASIC_G>;
+ clock-names = "sata", "pmalive", "rxoob", "ref", "asic";
+ interrupts = <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH 0>;
+ ports-implemented = <0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sata-port@0 {
+ reg = <0>;
+ hba-port-cap = <HBA_PORT_FBSCP>;
+ phys = <&combphy0_ps PHY_TYPE_SATA>;
+ phy-names = "sata-phy";
+ snps,rx-ts-max = <32>;
+ snps,tx-ts-max = <32>;
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
index 5afa4b57ce20..55a4bdfa3d9a 100644
--- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
+++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
@@ -13,8 +13,14 @@ description:
This document defines device tree bindings for the generic Synopsys DWC
implementation of the AHCI SATA controller.
-allOf:
- - $ref: snps,dwc-ahci-common.yaml#
+select:
+ properties:
+ compatible:
+ enum:
+ - snps,dwc-ahci
+ - snps,spear-ahci
+ required:
+ - compatible
properties:
compatible:
@@ -23,10 +29,6 @@ properties:
const: snps,dwc-ahci
- description: SPEAr1340 AHCI SATA device
const: snps,spear-ahci
- - description: Rockhip RK3568 AHCI controller
- items:
- - const: rockchip,rk3568-dwc-ahci
- - const: snps,dwc-ahci
patternProperties:
"^sata-port@[0-9a-e]$":
@@ -39,6 +41,9 @@ required:
- reg
- interrupts
+allOf:
+ - $ref: snps,dwc-ahci-common.yaml#
+
unevaluatedProperties: false
examples:
--
2.39.2
On Thu, Jun 08, 2023 at 06:22:34PM +0200, Sebastian Reichel wrote:
> Add PHY transmit and receive clocks as described by the
> DW SATA AHCI HW manual.
>
> Suggested-by: Serge Semin <[email protected]>
> Signed-off-by: Sebastian Reichel <[email protected]>
Looks good. Thanks.
Reviewed-by: Serge Semin <[email protected]>
-Serge(y)
> ---
> .../devicetree/bindings/ata/snps,dwc-ahci-common.yaml | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
> index c1457910520b..34c5bf65b02d 100644
> --- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
> +++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
> @@ -31,11 +31,11 @@ properties:
> PM-alive clock, RxOOB detection clock, embedded PHYs reference (Rx/Tx)
> clock, etc.
> minItems: 1
> - maxItems: 4
> + maxItems: 6
>
> clock-names:
> minItems: 1
> - maxItems: 4
> + maxItems: 6
> items:
> oneOf:
> - description: Application APB/AHB/AXI BIU clock
> @@ -48,6 +48,10 @@ properties:
> const: pmalive
> - description: RxOOB detection clock
> const: rxoob
> + - description: PHY Transmit Clock
> + const: asic
> + - description: PHY Receive Clock
> + const: rbc
> - description: SATA Ports reference clock
> const: ref
>
> --
> 2.39.2
>
On Thu, Jun 08, 2023 at 06:22:35PM +0200, Sebastian Reichel wrote:
> This adds Rockchip RK3588 AHCI binding. In order to narrow down the
> allowed clocks without bloating the generic binding, the description
> of Rockchip's AHCI controllers has been moved to its own file.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../bindings/ata/rockchip,dwc-ahci.yaml | 114 ++++++++++++++++++
> .../bindings/ata/snps,dwc-ahci.yaml | 17 ++-
> 2 files changed, 125 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
>
> diff --git a/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
> new file mode 100644
> index 000000000000..86da9bd594a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/rockchip,dwc-ahci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DWC AHCI SATA controller for Rockchip devices
> +
> +maintainers:
> + - Serge Semin <[email protected]>
> +
> +description:
> + This document defines device tree bindings for the Synopsys DWC
> + implementation of the AHCI SATA controller found in Rockchip
> + devices.
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3568-dwc-ahci
> + - rockchip,rk3588-dwc-ahci
> + required:
> + - compatible
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - rockchip,rk3568-dwc-ahci
> + - rockchip,rk3588-dwc-ahci
> + - const: snps,dwc-ahci
> +
> + ports-implemented:
> + const: 1
> +
> +patternProperties:
> + "^sata-port@[0-9a-e]$":
> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
> +
What about adding the "reg" property constraints? Seeing the
"ports-implemented" property is supposed to be "1" the "reg" property
should be zero for all the Rockchip AHCI SATA controllers. Right?
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - ports-implemented
> +
> +allOf:
> + - $ref: snps,dwc-ahci-common.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3588-dwc-ahci
> + then:
> + properties:
> + clock-names:
> + items:
> + - const: sata
> + - const: pmalive
> + - const: rxoob
> + - const: ref
> + - const: asic
The "clocks" property constraints could have been added too. Right?
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3568-dwc-ahci
> + then:
> + properties:
> + clock-names:
> + items:
> + - const: sata
> + - const: pmalive
> + - const: rxoob
ditto
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/ata/ahci.h>
> + #include <dt-bindings/phy/phy.h>
> +
> + sata@fe210000 {
> + compatible = "rockchip,rk3588-dwc-ahci", "snps,dwc-ahci";
> + reg = <0xfe210000 0x1000>;
> + clocks = <&cru ACLK_SATA0>, <&cru CLK_PMALIVE0>,
> + <&cru CLK_RXOOB0>, <&cru CLK_PIPEPHY0_REF>,
> + <&cru CLK_PIPEPHY0_PIPE_ASIC_G>;
> + clock-names = "sata", "pmalive", "rxoob", "ref", "asic";
> + interrupts = <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH 0>;
> + ports-implemented = <0x1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sata-port@0 {
> + reg = <0>;
> + hba-port-cap = <HBA_PORT_FBSCP>;
> + phys = <&combphy0_ps PHY_TYPE_SATA>;
> + phy-names = "sata-phy";
> + snps,rx-ts-max = <32>;
> + snps,tx-ts-max = <32>;
> + };
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
> index 5afa4b57ce20..55a4bdfa3d9a 100644
> --- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci.yaml
> @@ -13,8 +13,14 @@ description:
> This document defines device tree bindings for the generic Synopsys DWC
> implementation of the AHCI SATA controller.
>
> -allOf:
> - - $ref: snps,dwc-ahci-common.yaml#
> +select:
> + properties:
> + compatible:
> + enum:
> + - snps,dwc-ahci
> + - snps,spear-ahci
> + required:
> + - compatible
Please don't move the allOf property. It's ok to have it defined above
the object properties clause if it has just a few ref-statements.
* Also make sure please that the "select" property is defined above
the "allOf" schema composition.
-Serge(y)
>
> properties:
> compatible:
> @@ -23,10 +29,6 @@ properties:
> const: snps,dwc-ahci
> - description: SPEAr1340 AHCI SATA device
> const: snps,spear-ahci
> - - description: Rockhip RK3568 AHCI controller
> - items:
> - - const: rockchip,rk3568-dwc-ahci
> - - const: snps,dwc-ahci
>
> patternProperties:
> "^sata-port@[0-9a-e]$":
> @@ -39,6 +41,9 @@ required:
> - reg
> - interrupts
>
> +allOf:
> + - $ref: snps,dwc-ahci-common.yaml#
> +
> unevaluatedProperties: false
>
> examples:
> --
> 2.39.2
>
On 12/06/2023 10:18, Krzysztof Kozlowski wrote:
> On 08/06/2023 18:22, Sebastian Reichel wrote:
>> Add PHY transmit and receive clocks as described by the
>> DW SATA AHCI HW manual.
>>
>> Suggested-by: Serge Semin <[email protected]>
>> Signed-off-by: Sebastian Reichel <[email protected]>
>> ---
>> .../devicetree/bindings/ata/snps,dwc-ahci-common.yaml | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
>> index c1457910520b..34c5bf65b02d 100644
>> --- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
>> +++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
>> @@ -31,11 +31,11 @@ properties:
>> PM-alive clock, RxOOB detection clock, embedded PHYs reference (Rx/Tx)
>> clock, etc.
>> minItems: 1
>> - maxItems: 4
>> + maxItems: 6
>>
>> clock-names:
>> minItems: 1
>> - maxItems: 4
>> + maxItems: 6
>> items:
>> oneOf:
>> - description: Application APB/AHB/AXI BIU clock
>> @@ -48,6 +48,10 @@ properties:
>> const: pmalive
>> - description: RxOOB detection clock
>> const: rxoob
>> + - description: PHY Transmit Clock
>> + const: asic
>> + - description: PHY Receive Clock
>> + const: rbc
>
> Conor's comment was not resolved. Adding entries in the middle breaks
> existing users and commit msg does not explain this.
Wait, this is oneOf, not a list. Damn context.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 08/06/2023 18:22, Sebastian Reichel wrote:
> Add PHY transmit and receive clocks as described by the
> DW SATA AHCI HW manual.
>
> Suggested-by: Serge Semin <[email protected]>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../devicetree/bindings/ata/snps,dwc-ahci-common.yaml | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
> index c1457910520b..34c5bf65b02d 100644
> --- a/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
> +++ b/Documentation/devicetree/bindings/ata/snps,dwc-ahci-common.yaml
> @@ -31,11 +31,11 @@ properties:
> PM-alive clock, RxOOB detection clock, embedded PHYs reference (Rx/Tx)
> clock, etc.
> minItems: 1
> - maxItems: 4
> + maxItems: 6
>
> clock-names:
> minItems: 1
> - maxItems: 4
> + maxItems: 6
> items:
> oneOf:
> - description: Application APB/AHB/AXI BIU clock
> @@ -48,6 +48,10 @@ properties:
> const: pmalive
> - description: RxOOB detection clock
> const: rxoob
> + - description: PHY Transmit Clock
> + const: asic
> + - description: PHY Receive Clock
> + const: rbc
Conor's comment was not resolved. Adding entries in the middle breaks
existing users and commit msg does not explain this.
Best regards,
Krzysztof
On 12/06/2023 10:35, Serge Semin wrote:
> On Mon, Jun 12, 2023 at 10:24:06AM +0200, Krzysztof Kozlowski wrote:
>> On 08/06/2023 18:22, Sebastian Reichel wrote:
>>> This adds Rockchip RK3588 AHCI binding. In order to narrow down the
>>> allowed clocks without bloating the generic binding, the description
>>> of Rockchip's AHCI controllers has been moved to its own file.
>>>
>>> Signed-off-by: Sebastian Reichel <[email protected]>
>>> ---
>>
>> ...
>>
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - rockchip,rk3568-dwc-ahci
>>> + - rockchip,rk3588-dwc-ahci
>>> + - const: snps,dwc-ahci
>>> +
>>> + ports-implemented:
>>> + const: 1
>>> +
>>> +patternProperties:
>>> + "^sata-port@[0-9a-e]$":
>>> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
>>> +
>>> + unevaluatedProperties: false
>>
>
>> You should be able to skip this patternProperties entirely, because it
>> comes from dwc-ahci-common -> ahci-common schema. Did you try the patch
>> without it?
>
> Please see my message about this. The "sata-port@[0-9a-e]$" sub-node
> bindings could be updated with the "reg" property constraint which,
> based on the "ports-implemented" property value, most likely is
> supposed to be always set to const: 1.
Then anyway the pattern is wrong as it should be @1 always.
Best regards,
Krzysztof
On 08/06/2023 18:22, Sebastian Reichel wrote:
> The RK3588 has two reset lines for the combphy. One for the
> APB interface and one for the actual PHY.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Mon, Jun 12, 2023 at 10:24:06AM +0200, Krzysztof Kozlowski wrote:
> On 08/06/2023 18:22, Sebastian Reichel wrote:
> > This adds Rockchip RK3588 AHCI binding. In order to narrow down the
> > allowed clocks without bloating the generic binding, the description
> > of Rockchip's AHCI controllers has been moved to its own file.
> >
> > Signed-off-by: Sebastian Reichel <[email protected]>
> > ---
>
> ...
>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - rockchip,rk3568-dwc-ahci
> > + - rockchip,rk3588-dwc-ahci
> > + - const: snps,dwc-ahci
> > +
> > + ports-implemented:
> > + const: 1
> > +
> > +patternProperties:
> > + "^sata-port@[0-9a-e]$":
> > + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
> > +
> > + unevaluatedProperties: false
>
> You should be able to skip this patternProperties entirely, because it
> comes from dwc-ahci-common -> ahci-common schema. Did you try the patch
> without it?
Please see my message about this. The "sata-port@[0-9a-e]$" sub-node
bindings could be updated with the "reg" property constraint which,
based on the "ports-implemented" property value, most likely is
supposed to be always set to const: 1.
-Serge(y)
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - ports-implemented
> > +
>
>
> Best regards,
> Krzysztof
>
On 08/06/2023 18:22, Sebastian Reichel wrote:
> This adds Rockchip RK3588 AHCI binding. In order to narrow down the
> allowed clocks without bloating the generic binding, the description
> of Rockchip's AHCI controllers has been moved to its own file.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
...
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - rockchip,rk3568-dwc-ahci
> + - rockchip,rk3588-dwc-ahci
> + - const: snps,dwc-ahci
> +
> + ports-implemented:
> + const: 1
> +
> +patternProperties:
> + "^sata-port@[0-9a-e]$":
> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
> +
> + unevaluatedProperties: false
You should be able to skip this patternProperties entirely, because it
comes from dwc-ahci-common -> ahci-common schema. Did you try the patch
without it?
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - ports-implemented
> +
Best regards,
Krzysztof
On Mon, Jun 12, 2023 at 10:39:57AM +0200, Krzysztof Kozlowski wrote:
> On 12/06/2023 10:35, Serge Semin wrote:
> > On Mon, Jun 12, 2023 at 10:24:06AM +0200, Krzysztof Kozlowski wrote:
> >> On 08/06/2023 18:22, Sebastian Reichel wrote:
> >>> This adds Rockchip RK3588 AHCI binding. In order to narrow down the
> >>> allowed clocks without bloating the generic binding, the description
> >>> of Rockchip's AHCI controllers has been moved to its own file.
> >>>
> >>> Signed-off-by: Sebastian Reichel <[email protected]>
> >>> ---
> >>
> >> ...
> >>
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + items:
> >>> + - enum:
> >>> + - rockchip,rk3568-dwc-ahci
> >>> + - rockchip,rk3588-dwc-ahci
> >>> + - const: snps,dwc-ahci
> >>> +
> >>> + ports-implemented:
> >>> + const: 1
> >>> +
> >>> +patternProperties:
> >>> + "^sata-port@[0-9a-e]$":
> >>> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
> >>> +
> >>> + unevaluatedProperties: false
> >>
> >
> >> You should be able to skip this patternProperties entirely, because it
> >> comes from dwc-ahci-common -> ahci-common schema. Did you try the patch
> >> without it?
> >
> > Please see my message about this. The "sata-port@[0-9a-e]$" sub-node
> > bindings could be updated with the "reg" property constraint which,
> > based on the "ports-implemented" property value, most likely is
> > supposed to be always set to const: 1.
>
> Then anyway the pattern is wrong as it should be @1 always.
* I miscalculated a bit, it should have been zero but in general
the pattern-property is indeed redundant.
As a conclusion the change should look like this:
+properties:
+ compatible:
+ items:
+ - enum:
+ - rockchip,rk3568-dwc-ahci
+ - rockchip,rk3588-dwc-ahci
+ - const: snps,dwc-ahci
+
+ ports-implemented:
+ const: 1
+
+ "sata-port@0":
+ $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
+
+ properties:
+ reg:
+ const: 0
+
+ unevaluatedProperties: false
+
+ ...
Right?
-Serge(y)
>
> Best regards,
> Krzysztof
>
On 12/06/2023 10:52, Serge Semin wrote:
> On Mon, Jun 12, 2023 at 10:39:57AM +0200, Krzysztof Kozlowski wrote:
>> On 12/06/2023 10:35, Serge Semin wrote:
>>> On Mon, Jun 12, 2023 at 10:24:06AM +0200, Krzysztof Kozlowski wrote:
>>>> On 08/06/2023 18:22, Sebastian Reichel wrote:
>>>>> This adds Rockchip RK3588 AHCI binding. In order to narrow down the
>>>>> allowed clocks without bloating the generic binding, the description
>>>>> of Rockchip's AHCI controllers has been moved to its own file.
>>>>>
>>>>> Signed-off-by: Sebastian Reichel <[email protected]>
>>>>> ---
>>>>
>>>> ...
>>>>
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + items:
>>>>> + - enum:
>>>>> + - rockchip,rk3568-dwc-ahci
>>>>> + - rockchip,rk3588-dwc-ahci
>>>>> + - const: snps,dwc-ahci
>>>>> +
>>>>> + ports-implemented:
>>>>> + const: 1
>>>>> +
>>>>> +patternProperties:
>>>>> + "^sata-port@[0-9a-e]$":
>>>>> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
>>>>> +
>>>>> + unevaluatedProperties: false
>>>>
>>>
>>>> You should be able to skip this patternProperties entirely, because it
>>>> comes from dwc-ahci-common -> ahci-common schema. Did you try the patch
>>>> without it?
>>>
>
>>> Please see my message about this. The "sata-port@[0-9a-e]$" sub-node
>>> bindings could be updated with the "reg" property constraint which,
>>> based on the "ports-implemented" property value, most likely is
>>> supposed to be always set to const: 1.
>>
>> Then anyway the pattern is wrong as it should be @1 always.
>
> * I miscalculated a bit, it should have been zero but in general
> the pattern-property is indeed redundant.
>
> As a conclusion the change should look like this:
>
> +properties:
> + compatible:
> + items:
> + - enum:
> + - rockchip,rk3568-dwc-ahci
> + - rockchip,rk3588-dwc-ahci
> + - const: snps,dwc-ahci
> +
> + ports-implemented:
> + const: 1
> +
> + "sata-port@0":
> + $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
> +
> + properties:
> + reg:
> + const: 0
> +
> + unevaluatedProperties: false
> +
> + ...
>
> Right?
Code is correct but as I said - probably meaningless. All other ports
are also accepted via referenced schema. You would need to disallow
other ports or switch to additionalProperties: false in top-level.
Best regards,
Krzysztof