[PATCH 1/6] dt-bindings: riscv: correct e51 and u54-mc CPU bindings

[PATCH 1/6] dt-bindings: riscv: correct e51 and u54-mc CPU bindings

[Krzysztof Kozlowski]

All existing boards with sifive,e51 and sifive,u54-mc use it on top of
sifive,rocket0 compatible:

  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: cpu@0: compatible: 'oneOf' conditional failed, one must be fixed:
    ['sifive,e51', 'sifive,rocket0', 'riscv'] is too long
    Additional items are not allowed ('riscv' was unexpected)
    Additional items are not allowed ('sifive,rocket0', 'riscv' were unexpected)
    'riscv' was expected

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index e534f6a7cfa1..aa5fb64d57eb 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -31,9 +31,7 @@ properties:
               - sifive,bullet0
               - sifive,e5
               - sifive,e7
-              - sifive,e51
               - sifive,e71
-              - sifive,u54-mc
               - sifive,u74-mc
               - sifive,u54
               - sifive,u74
@@ -41,6 +39,12 @@ properties:
               - sifive,u7
               - canaan,k210
           - const: riscv
+      - items:
+          - enum:
+              - sifive,e51
+              - sifive,u54-mc
+          - const: sifive,rocket0
+          - const: riscv
       - const: riscv    # Simulator only
     description:
       Identifies that the hart uses the RISC-V instruction set
-- 
2.30.2

[PATCH 2/6] dt-bindings: mmc: cdns: match MPFS MMC/SDHCI controller

[Krzysztof Kozlowski]

The Microchip MPFS Icicle Kit uses Cadence SD/SDIO/eMMC Host Controller
without any additional vendor compatible:

  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: mmc@20008000: compatible:0: 'cdns,sd4hc' is not one of ['socionext,uniphier-sd4hc']
  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: mmc@20008000: compatible: ['cdns,sd4hc'] is too short

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
index af7442f73881..0489aa92cb54 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -15,9 +15,11 @@ allOf:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - socionext,uniphier-sd4hc
+    oneOf:
+      - items:
+          - enum:
+              - socionext,uniphier-sd4hc
+          - const: cdns,sd4hc
       - const: cdns,sd4hc
 
   reg:
-- 
2.30.2

[PATCH 3/6] riscv: microchip: mpfs: drop duplicated nodes

[Krzysztof Kozlowski]

The DTSI file defines soc node and address/size cells, so there is no
point in duplicating it in DTS file.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
index ec79944065c9..237830583514 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
@@ -9,8 +9,6 @@
 #define RTCCLK_FREQ		1000000
 
 / {
-	#address-cells = <2>;
-	#size-cells = <2>;
 	model = "Microchip PolarFire-SoC Icicle Kit";
 	compatible = "microchip,mpfs-icicle-kit";
 
@@ -27,9 +25,6 @@ memory@80000000 {
 		reg = <0x0 0x80000000 0x0 0x40000000>;
 		clocks = <&clkcfg 26>;
 	};
-
-	soc {
-	};
 };
 
 &serial0 {
-- 
2.30.2

[PATCH 4/6] riscv: microchip: mpfs: fix board compatible

[Krzysztof Kozlowski]

According to bindings, the compatible must include microchip,mpfs.  This
fixes dtbs_check warning:

  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: /: compatible: ['microchip,mpfs-icicle-kit'] is too short

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts | 2 +-
 arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
index 237830583514..62f7651de538 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
@@ -10,7 +10,7 @@
 
 / {
 	model = "Microchip PolarFire-SoC Icicle Kit";
-	compatible = "microchip,mpfs-icicle-kit";
+	compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 
 	chosen {
 		stdout-path = &serial0;
diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
index b9819570a7d1..cb54da0cc3c4 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
@@ -7,7 +7,7 @@ / {
 	#address-cells = <2>;
 	#size-cells = <2>;
 	model = "Microchip MPFS Icicle Kit";
-	compatible = "microchip,mpfs-icicle-kit";
+	compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
 
 	chosen {
 	};
-- 
2.30.2

[PATCH 5/6] riscv: microchip: mpfs: drop duplicated MMC/SDHC node

[Krzysztof Kozlowski]

Devicetree source is a description of hardware and hardware has only one
block @20008000 which can be configured either as eMMC or SDHC.  Having
two node for different modes is an obscure, unusual and confusing way to
configure it.  Instead the board file is supposed to customize the block
to its needs, e.g. to SDHC mode.

This fixes dtbs_check warning:
  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: sdhc@20008000: $nodename:0: 'sdhc@20008000' does not match '^mmc(@.*)?$'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 .../microchip/microchip-mpfs-icicle-kit.dts   | 10 ++++++-
 .../boot/dts/microchip/microchip-mpfs.dtsi    | 27 +------------------
 2 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
index 62f7651de538..ac6083c76083 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
@@ -43,8 +43,16 @@ &serial3 {
 	status = "okay";
 };
 
-&sdcard {
+&mmc {
 	status = "okay";
+
+	disable-wp;
+	cap-sd-highspeed;
+	card-detect-delay = <200>;
+	sd-uhs-sdr12;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 };
 
 &emac0 {
diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
index cb54da0cc3c4..c4ccd7e4d3eb 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
@@ -262,25 +262,7 @@ serial3: serial@20104000 {
 			status = "disabled";
 		};
 
-		emmc: mmc@20008000 {
-			compatible = "cdns,sd4hc";
-			reg = <0x0 0x20008000 0x0 0x1000>;
-			interrupt-parent = <&plic>;
-			interrupts = <88 89>;
-			pinctrl-names = "default";
-			clocks = <&clkcfg 6>;
-			bus-width = <4>;
-			cap-mmc-highspeed;
-			mmc-ddr-3_3v;
-			max-frequency = <200000000>;
-			non-removable;
-			no-sd;
-			no-sdio;
-			voltage-ranges = <3300 3300>;
-			status = "disabled";
-		};
-
-		sdcard: sdhc@20008000 {
+		mmc: mmc@20008000 {
 			compatible = "cdns,sd4hc";
 			reg = <0x0 0x20008000 0x0 0x1000>;
 			interrupt-parent = <&plic>;
@@ -288,13 +270,6 @@ sdcard: sdhc@20008000 {
 			pinctrl-names = "default";
 			clocks = <&clkcfg 6>;
 			bus-width = <4>;
-			disable-wp;
-			cap-sd-highspeed;
-			card-detect-delay = <200>;
-			sd-uhs-sdr12;
-			sd-uhs-sdr25;
-			sd-uhs-sdr50;
-			sd-uhs-sdr104;
 			max-frequency = <200000000>;
 			status = "disabled";
 		};
-- 
2.30.2

[PATCH 6/6] riscv: microchip: mpfs: drop unused pinctrl-names

[Krzysztof Kozlowski]

pinctrl-names without pinctrl-0 does not have any sense:

  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: sdhc@20008000: 'pinctrl-0' is a dependency of 'pinctrl-names'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
index c4ccd7e4d3eb..d9f7ee747d0d 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
@@ -267,7 +267,6 @@ mmc: mmc@20008000 {
 			reg = <0x0 0x20008000 0x0 0x1000>;
 			interrupt-parent = <&plic>;
 			interrupts = <88>;
-			pinctrl-names = "default";
 			clocks = <&clkcfg 6>;
 			bus-width = <4>;
 			max-frequency = <200000000>;
-- 
2.30.2

Re: [PATCH 3/6] riscv: microchip: mpfs: drop duplicated nodes

[Krzysztof Kozlowski]

On 19/08/2021 17:44, Krzysztof Kozlowski wrote:
> The DTSI file defines soc node and address/size cells, so there is no
> point in duplicating it in DTS file.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts | 5 -----
>  1 file changed, 5 deletions(-)
> 

Now I wonder whether the subject prefix should be "riscv: dts:
microchip:" instead?


Best regards,
Krzysztof

Re: [PATCH 2/6] dt-bindings: mmc: cdns: match MPFS MMC/SDHCI controller

[Rob Herring]

On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
> The Microchip MPFS Icicle Kit uses Cadence SD/SDIO/eMMC Host Controller
> without any additional vendor compatible:

I think the lack of vendor compatible is the error here. Experience has 
shown that vendor specific compatibles are needed for licensed IP.

> 
>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: mmc@20008000: compatible:0: 'cdns,sd4hc' is not one of ['socionext,uniphier-sd4hc']
>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: mmc@20008000: compatible: ['cdns,sd4hc'] is too short
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index af7442f73881..0489aa92cb54 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -15,9 +15,11 @@ allOf:
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - socionext,uniphier-sd4hc
> +    oneOf:
> +      - items:
> +          - enum:
> +              - socionext,uniphier-sd4hc
> +          - const: cdns,sd4hc
>        - const: cdns,sd4hc
>  
>    reg:
> -- 
> 2.30.2
> 
> 

Re: [PATCH 1/6] dt-bindings: riscv: correct e51 and u54-mc CPU bindings

[Rob Herring]

On Thu, 19 Aug 2021 17:44:31 +0200, Krzysztof Kozlowski wrote:
> All existing boards with sifive,e51 and sifive,u54-mc use it on top of
> sifive,rocket0 compatible:
> 
>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: cpu@0: compatible: 'oneOf' conditional failed, one must be fixed:
>     ['sifive,e51', 'sifive,rocket0', 'riscv'] is too long
>     Additional items are not allowed ('riscv' was unexpected)
>     Additional items are not allowed ('sifive,rocket0', 'riscv' were unexpected)
>     'riscv' was expected
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 4/6] riscv: microchip: mpfs: fix board compatible

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
> According to bindings, the compatible must include microchip,mpfs.  This
> fixes dtbs_check warning:
>
>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: /: compatible: ['microchip,mpfs-icicle-kit'] is too short
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Thanks for your patch!

> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
> @@ -10,7 +10,7 @@
>
>  / {
>         model = "Microchip PolarFire-SoC Icicle Kit";
> -       compatible = "microchip,mpfs-icicle-kit";
> +       compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";

I have the same change in my local tree, but didn't get to submit it
yet, so this part is fine ;-)

>
>         chosen {
>                 stdout-path = &serial0;
> diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
> index b9819570a7d1..cb54da0cc3c4 100644
> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
> @@ -7,7 +7,7 @@ / {
>         #address-cells = <2>;
>         #size-cells = <2>;
>         model = "Microchip MPFS Icicle Kit";
> -       compatible = "microchip,mpfs-icicle-kit";
> +       compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";

As this file is the SoC .dtsi, not the board DTS for the full
Icicle Kit, the compatible value should be just "microchip,mpfs"
(to be augmented by the board DTS).
And "model" should be "Microchip PolarFire SoC".

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/6] riscv: microchip: mpfs: drop duplicated nodes

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Aug 19, 2021 at 6:22 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
> On 19/08/2021 17:44, Krzysztof Kozlowski wrote:
> > The DTSI file defines soc node and address/size cells, so there is no
> > point in duplicating it in DTS file.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > ---
> >  arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts | 5 -----
> >  1 file changed, 5 deletions(-)
> >
>
> Now I wonder whether the subject prefix should be "riscv: dts:
> microchip:" instead?

Agreed.

For the actual patch contents:
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 6/6] riscv: microchip: mpfs: drop unused pinctrl-names

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
> pinctrl-names without pinctrl-0 does not have any sense:
>
>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: sdhc@20008000: 'pinctrl-0' is a dependency of 'pinctrl-names'

Indeed, and pin control properties should be in the board .dts, not in the
SoC .dtsi.

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/6] riscv: microchip: mpfs: drop duplicated MMC/SDHC node

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
> Devicetree source is a description of hardware and hardware has only one
> block @20008000 which can be configured either as eMMC or SDHC.  Having
> two node for different modes is an obscure, unusual and confusing way to
> configure it.  Instead the board file is supposed to customize the block
> to its needs, e.g. to SDHC mode.
>
> This fixes dtbs_check warning:
>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: sdhc@20008000: $nodename:0: 'sdhc@20008000' does not match '^mmc(@.*)?$'
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Thanks for your patch!

> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
> @@ -43,8 +43,16 @@ &serial3 {
>         status = "okay";
>  };
>
> -&sdcard {
> +&mmc {
>         status = "okay";
> +
> +       disable-wp;
> +       cap-sd-highspeed;
> +       card-detect-delay = <200>;
> +       sd-uhs-sdr12;
> +       sd-uhs-sdr25;
> +       sd-uhs-sdr50;
> +       sd-uhs-sdr104;
>  };
>
>  &emac0 {
> diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
> index cb54da0cc3c4..c4ccd7e4d3eb 100644
> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
> @@ -262,25 +262,7 @@ serial3: serial@20104000 {
>                         status = "disabled";
>                 };
>
> -               emmc: mmc@20008000 {
> -                       compatible = "cdns,sd4hc";
> -                       reg = <0x0 0x20008000 0x0 0x1000>;
> -                       interrupt-parent = <&plic>;
> -                       interrupts = <88 89>;

Note that the other node has only a single interrupt.
Which one is correct?

> -                       pinctrl-names = "default";
> -                       clocks = <&clkcfg 6>;
> -                       bus-width = <4>;
> -                       cap-mmc-highspeed;
> -                       mmc-ddr-3_3v;
> -                       max-frequency = <200000000>;
> -                       non-removable;
> -                       no-sd;
> -                       no-sdio;
> -                       voltage-ranges = <3300 3300>;
> -                       status = "disabled";
> -               };
> -
> -               sdcard: sdhc@20008000 {
> +               mmc: mmc@20008000 {
>                         compatible = "cdns,sd4hc";
>                         reg = <0x0 0x20008000 0x0 0x1000>;
>                         interrupt-parent = <&plic>;
> @@ -288,13 +270,6 @@ sdcard: sdhc@20008000 {
>                         pinctrl-names = "default";
>                         clocks = <&clkcfg 6>;
>                         bus-width = <4>;

I think bus-width should be moved to the board .dts, too.

> -                       disable-wp;
> -                       cap-sd-highspeed;
> -                       card-detect-delay = <200>;
> -                       sd-uhs-sdr12;
> -                       sd-uhs-sdr25;
> -                       sd-uhs-sdr50;
> -                       sd-uhs-sdr104;
>                         max-frequency = <200000000>;
>                         status = "disabled";
>                 };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/6] dt-bindings: mmc: cdns: match MPFS MMC/SDHCI controller

[Krzysztof Kozlowski]

On 24/08/2021 16:33, Rob Herring wrote:
> On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
>> The Microchip MPFS Icicle Kit uses Cadence SD/SDIO/eMMC Host Controller
>> without any additional vendor compatible:
> 
> I think the lack of vendor compatible is the error here. Experience has 
> shown that vendor specific compatibles are needed for licensed IP.
> 

In such case this could be:
1. a specific "microchip,mpfs250t-sd4hc", which
seems to be on MPFS Icicle Kit:
https://www.digikey.co.uk/en/product-highlight/m/microchip-technology/mpfs-icicle-kit-es--polarfire-soc-fpga-icicle-kit

2. or a generic "microchip,mpfs-sd4hc"

Any hints here?

Best regards,
Krzysztof

Re: [PATCH 4/6] riscv: microchip: mpfs: fix board compatible

[Krzysztof Kozlowski]

On 24/08/2021 17:29, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
>> According to bindings, the compatible must include microchip,mpfs.  This
>> fixes dtbs_check warning:
>>
>>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: /: compatible: ['microchip,mpfs-icicle-kit'] is too short
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 
> Thanks for your patch!
> 
>> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
>> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
>> @@ -10,7 +10,7 @@
>>
>>  / {
>>         model = "Microchip PolarFire-SoC Icicle Kit";
>> -       compatible = "microchip,mpfs-icicle-kit";
>> +       compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
> 
> I have the same change in my local tree, but didn't get to submit it
> yet, so this part is fine ;-)
> 
>>
>>         chosen {
>>                 stdout-path = &serial0;
>> diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> index b9819570a7d1..cb54da0cc3c4 100644
>> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> @@ -7,7 +7,7 @@ / {
>>         #address-cells = <2>;
>>         #size-cells = <2>;
>>         model = "Microchip MPFS Icicle Kit";
>> -       compatible = "microchip,mpfs-icicle-kit";
>> +       compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";
> 
> As this file is the SoC .dtsi, not the board DTS for the full
> Icicle Kit, the compatible value should be just "microchip,mpfs"
> (to be augmented by the board DTS).
> And "model" should be "Microchip PolarFire SoC".


Sounds good, I'll fix it in v2.


Best regards,
Krzysztof

Re: [PATCH 5/6] riscv: microchip: mpfs: drop duplicated MMC/SDHC node

[Krzysztof Kozlowski]

On 24/08/2021 17:37, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
>> Devicetree source is a description of hardware and hardware has only one
>> block @20008000 which can be configured either as eMMC or SDHC.  Having
>> two node for different modes is an obscure, unusual and confusing way to
>> configure it.  Instead the board file is supposed to customize the block
>> to its needs, e.g. to SDHC mode.
>>
>> This fixes dtbs_check warning:
>>   arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: sdhc@20008000: $nodename:0: 'sdhc@20008000' does not match '^mmc(@.*)?$'
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 
> Thanks for your patch!
> 
>> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
>> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
>> @@ -43,8 +43,16 @@ &serial3 {
>>         status = "okay";
>>  };
>>
>> -&sdcard {
>> +&mmc {
>>         status = "okay";
>> +
>> +       disable-wp;
>> +       cap-sd-highspeed;
>> +       card-detect-delay = <200>;
>> +       sd-uhs-sdr12;
>> +       sd-uhs-sdr25;
>> +       sd-uhs-sdr50;
>> +       sd-uhs-sdr104;
>>  };
>>
>>  &emac0 {
>> diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> index cb54da0cc3c4..c4ccd7e4d3eb 100644
>> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> @@ -262,25 +262,7 @@ serial3: serial@20104000 {
>>                         status = "disabled";
>>                 };
>>
>> -               emmc: mmc@20008000 {
>> -                       compatible = "cdns,sd4hc";
>> -                       reg = <0x0 0x20008000 0x0 0x1000>;
>> -                       interrupt-parent = <&plic>;
>> -                       interrupts = <88 89>;
> 
> Note that the other node has only a single interrupt.
> Which one is correct?

I assume the one put there initially, since it was tested (sdcard wsas
enabled, emmc was disabled).

> 
>> -                       pinctrl-names = "default";
>> -                       clocks = <&clkcfg 6>;
>> -                       bus-width = <4>;
>> -                       cap-mmc-highspeed;
>> -                       mmc-ddr-3_3v;
>> -                       max-frequency = <200000000>;
>> -                       non-removable;
>> -                       no-sd;
>> -                       no-sdio;
>> -                       voltage-ranges = <3300 3300>;
>> -                       status = "disabled";
>> -               };
>> -
>> -               sdcard: sdhc@20008000 {
>> +               mmc: mmc@20008000 {
>>                         compatible = "cdns,sd4hc";
>>                         reg = <0x0 0x20008000 0x0 0x1000>;
>>                         interrupt-parent = <&plic>;
>> @@ -288,13 +270,6 @@ sdcard: sdhc@20008000 {
>>                         pinctrl-names = "default";
>>                         clocks = <&clkcfg 6>;
>>                         bus-width = <4>;
> 
> I think bus-width should be moved to the board .dts, too.

Makes sense. Thanks for review!


Best regards,
Krzysztof

Re: [PATCH 2/6] dt-bindings: mmc: cdns: match MPFS MMC/SDHCI controller

[Rob Herring]

On Tue, Aug 24, 2021 at 2:02 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 24/08/2021 16:33, Rob Herring wrote:
> > On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
> >> The Microchip MPFS Icicle Kit uses Cadence SD/SDIO/eMMC Host Controller
> >> without any additional vendor compatible:
> >
> > I think the lack of vendor compatible is the error here. Experience has
> > shown that vendor specific compatibles are needed for licensed IP.
> >
>
> In such case this could be:
> 1. a specific "microchip,mpfs250t-sd4hc", which
> seems to be on MPFS Icicle Kit:
> https://www.digikey.co.uk/en/product-highlight/m/microchip-technology/mpfs-icicle-kit-es--polarfire-soc-fpga-icicle-kit
>
> 2. or a generic "microchip,mpfs-sd4hc"
>
> Any hints here?

Best for a Microchip person to answer, but sure there's some existing
compatible strings for other blocks on this chip to follow the same
pattern.

Rob

Re: [PATCH 2/6] dt-bindings: mmc: cdns: match MPFS MMC/SDHCI controller

[Conor.Dooley]

On 30/08/2021 16:09, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Tue, Aug 24, 2021 at 2:02 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
>> On 24/08/2021 16:33, Rob Herring wrote:
>>> On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
>>>> The Microchip MPFS Icicle Kit uses Cadence SD/SDIO/eMMC Host Controller
>>>> without any additional vendor compatible:
>>> I think the lack of vendor compatible is the error here. Experience has
>>> shown that vendor specific compatibles are needed for licensed IP.
>>>
>> In such case this could be:
>> 1. a specific "microchip,mpfs250t-sd4hc", which
>> seems to be on MPFS Icicle Kit:
>> https://www.digikey.co.uk/en/product-highlight/m/microchip-technology/mpfs-icicle-kit-es--polarfire-soc-fpga-icicle-kit
>>
>> 2. or a generic "microchip,mpfs-sd4hc"
>>
>> Any hints here?
> Best for a Microchip person to answer, but sure there's some existing
> compatible strings for other blocks on this chip to follow the same
> pattern.
>
> Rob

#2 would be ideal since the controller doesn't change across the part 
range, the 250t bit in the part name just covers the size/configuration 
of the FPGA.

Conor

>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/6] dt-bindings: mmc: cdns: match MPFS MMC/SDHCI controller

[Krzysztof Kozlowski]

On 06/09/2021 10:38, Conor.Dooley@microchip.com wrote:
> On 30/08/2021 16:09, Rob Herring wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On Tue, Aug 24, 2021 at 2:02 PM Krzysztof Kozlowski
>> <krzysztof.kozlowski@canonical.com> wrote:
>>> On 24/08/2021 16:33, Rob Herring wrote:
>>>> On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
>>>>> The Microchip MPFS Icicle Kit uses Cadence SD/SDIO/eMMC Host Controller
>>>>> without any additional vendor compatible:
>>>> I think the lack of vendor compatible is the error here. Experience has
>>>> shown that vendor specific compatibles are needed for licensed IP.
>>>>
>>> In such case this could be:
>>> 1. a specific "microchip,mpfs250t-sd4hc", which
>>> seems to be on MPFS Icicle Kit:
>>> https://www.digikey.co.uk/en/product-highlight/m/microchip-technology/mpfs-icicle-kit-es--polarfire-soc-fpga-icicle-kit
>>>
>>> 2. or a generic "microchip,mpfs-sd4hc"
>>>
>>> Any hints here?
>> Best for a Microchip person to answer, but sure there's some existing
>> compatible strings for other blocks on this chip to follow the same
>> pattern.
>>
>> Rob
> 
> #2 would be ideal since the controller doesn't change across the part 
> range, the 250t bit in the part name just covers the size/configuration 
> of the FPGA.


Thanks! I'll go with the microchip,mpfs-sd4hc.


Best regards,
Krzysztof