On 21/09/2021 12:40, Conor.Dooley@xxxxxxxxxxxxx wrote:
> On 20/09/2021 16:08, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> 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@xxxxxxxxxxxxx>
>>
>> ---
>>
>> Changes since v1:
>> 1. Move also bus-width, suggested by Geert.
>> ---
>> .../microchip/microchip-mpfs-icicle-kit.dts | 11 +++++++-
>> .../boot/dts/microchip/microchip-mpfs.dtsi | 28 +------------------
>> 2 files changed, 11 insertions(+), 28 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 07f1f3cab686..fc1e5869df1b 100644
>> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
>> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
>> @@ -51,8 +51,17 @@ &serial3 {
>> status = "okay";
>> };
>>
>> -&sdcard {
>> +&mmc {
>> status = "okay";
>> +
>> + bus-width = <4>;
>> + 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 5084b93188f0..83bc14860960 100644
>> --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
>> @@ -262,39 +262,13 @@ 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>;
>> interrupts = <88>;
>> 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
>>
> Hi Krzysztof,
> Seems I missed most of this series other than the new vendor name in the V1.
Unfortunately your name does not appear as maintainer for these files
and get_maintainers.pl brings it only sometimes as a --git fallback.
Also few addresses from that --git fallback are non working, so I am not
always Cc-ing them. Sorry for that, I'll try to Cc you on next Microchip
RISC-V submissions, however you should probably add a proper platform
maintainer entry (similarly to ARM/ARM64 subarchitectures).
>
> We have been redoing the device tree for the mpfs/icicle kit partly dye
> to some changes we made to the design. Previously SD and eMMC were
> different FPGA designs but now both are in the same design and managed
> by the bootloader, depending on where it finds the image to boot from.
> Since then we've just been using the following single entry in the .dtsi:
>
> mmc: mmc@20008000 { /* Common node entry for emmc/sd */
> compatible = "cdns,sd4hc";
> reg = <0x0 0x20008000 0x0 0x1000>;
> clocks = <&clkcfg CLK_MMC>;
> interrupt-parent = <&plic>;
> interrupts = <PLIC_INT_MMC_MAIN PLIC_INT_MMC_WAKEUP>;
I'll switch to 2 interrupts.
> bus-width = <4>;
> cap-mmc-highspeed;
> cap-sd-highspeed;
> no-1-8-v;
> disable-wp;
> max-frequency = <200000000>;
> status = "disabled";
I understand you prefer then the mmc node to be active? Before the DT
had SDHC one enable, so my patch did not introduce changes (except
mentioned interrupt).
I can change it to mmc above with your explanation.
Best regards,
Krzysztof
