From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 15:45:19
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 <redacted>
---
Documentation/devicetree/bindings/riscv/cpus.yaml | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -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 onlydescription:Identifies that the hart uses the RISC-V instruction set
From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 15:45:23
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 <redacted>
---
Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 15:45:25
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 <redacted>
---
arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts | 5 -----
1 file changed, 5 deletions(-)
From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 15:45:27
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 <redacted>
---
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(-)
From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 15:45:29
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 <redacted>
---
.../microchip/microchip-mpfs-icicle-kit.dts | 10 ++++++-
.../boot/dts/microchip/microchip-mpfs.dtsi | 27 +------------------
2 files changed, 10 insertions(+), 27 deletions(-)
From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 15:45:38
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 <redacted>
---
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 1 -
1 file changed, 1 deletion(-)
From: Krzysztof Kozlowski <hidden> Date: 2021-08-19 16:21:51
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 <redacted>
---
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
From: Rob Herring <robh@kernel.org> Date: 2021-08-24 14:33:55
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.
quoted hunk
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 <redacted>
---
Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Rob Herring <robh@kernel.org> Date: 2021-08-24 14:34:35
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 <redacted>
---
Documentation/devicetree/bindings/riscv/cpus.yaml | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Hi Krzysztof,
On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
[off-list ref] 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 <redacted>
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
Hi Krzysztof,
On Thu, Aug 19, 2021 at 6:22 PM Krzysztof Kozlowski
[off-list ref] wrote:
On 19/08/2021 17:44, Krzysztof Kozlowski wrote:
quoted
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 <redacted>
---
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
Hi Krzysztof,
On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
[off-list ref] 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 <redacted>
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
Hi Krzysztof,
On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
[off-list ref] 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 <redacted>
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
From: Krzysztof Kozlowski <hidden> Date: 2021-08-24 19:05:14
On 24/08/2021 17:29, Geert Uytterhoeven wrote:
Hi Krzysztof,
On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
[off-list ref] wrote:
quoted
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 <redacted>
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
From: Krzysztof Kozlowski <hidden> Date: 2021-08-24 19:07:27
On 24/08/2021 17:37, Geert Uytterhoeven wrote:
Hi Krzysztof,
On Thu, Aug 19, 2021 at 5:45 PM Krzysztof Kozlowski
[off-list ref] wrote:
quoted
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 <redacted>
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
[off-list ref] wrote:
quoted
On 24/08/2021 16:33, Rob Herring wrote:
quoted
On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
quoted
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.
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
From: Krzysztof Kozlowski <hidden> Date: 2021-09-08 07:37:29
On 06/09/2021 10:38, Conor.Dooley@microchip.com wrote:
On 30/08/2021 16:09, Rob Herring wrote:
quoted
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
[off-list ref] wrote:
quoted
On 24/08/2021 16:33, Rob Herring wrote:
quoted
On Thu, Aug 19, 2021 at 05:44:32PM +0200, Krzysztof Kozlowski wrote:
quoted
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.
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