| Message ID | 20180426140728.43155-3-icenowy@aosc.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 26/04/18 15:07, Icenowy Zheng wrote: > The Allwinner H6 SoC have 3 MMC controllers. > > Add device tree nodes for them. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > --- > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 ++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > index 4debc3962830..3cbfc035c979 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > @@ -124,12 +124,68 @@ > interrupt-controller; > #interrupt-cells = <3>; > > + mmc0_pins: mmc0-pins { > + pins = "PF0", "PF1", "PF2", "PF3", > + "PF4", "PF5"; > + function = "mmc0"; > + drive-strength = <30>; > + bias-pull-up; > + }; > + > + mmc2_pins: mmc2-pins { > + pins = "PC1", "PC4", "PC5", "PC6", > + "PC7", "PC8", "PC9", "PC10", > + "PC11", "PC12", "PC13", "PC14"; > + function = "mmc2"; > + drive-strength = <30>; > + bias-pull-up; > + }; > + > uart0_ph_pins: uart0-ph { > pins = "PH0", "PH1"; > function = "uart0"; > }; > }; > > + mmc0: mmc@4020000 { > + compatible = "allwinner,sun50i-h6-mmc"; This should be: compatible = "allwinner,sun50i-h6-mmc", "allwinner,sun50i-a64-mmc"; > + reg = <0x04020000 0x1000>; > + clocks = <&ccu CLK_BUS_MMC0>, <&ccu CLK_MMC0>; > + clock-names = "ahb", "mmc"; > + resets = <&ccu RST_BUS_MMC0>; > + reset-names = "ahb"; > + interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + mmc1: mmc@4021000 { > + compatible = "allwinner,sun50i-h6-mmc"; same here > + reg = <0x04021000 0x1000>; > + clocks = <&ccu CLK_BUS_MMC1>, <&ccu CLK_MMC1>; > + clock-names = "ahb", "mmc"; > + resets = <&ccu RST_BUS_MMC1>; > + reset-names = "ahb"; > + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + mmc2: mmc@4022000 { > + compatible = "allwinner,sun50i-h6-emmc"; and here: compatible = "allwinner,sun50i-h6-emmc", "allwinner,sun50i-a64-emmc"; The rest looks correct to me. Cheers, Andre. > + reg = <0x04022000 0x1000>; > + clocks = <&ccu CLK_BUS_MMC2>, <&ccu CLK_MMC2>; > + clock-names = "ahb", "mmc"; > + resets = <&ccu RST_BUS_MMC2>; > + reset-names = "ahb"; > + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > uart0: serial@5000000 { > compatible = "snps,dw-apb-uart"; > reg = <0x05000000 0x400>; >
于 2018年4月27日 GMT+08:00 上午12:45:38, Andre Przywara <andre.przywara@arm.com> 写到: >Hi, > >On 26/04/18 15:07, Icenowy Zheng wrote: >> The Allwinner H6 SoC have 3 MMC controllers. >> >> Add device tree nodes for them. >> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >> --- >> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 >++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >> index 4debc3962830..3cbfc035c979 100644 >> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >> @@ -124,12 +124,68 @@ >> interrupt-controller; >> #interrupt-cells = <3>; >> >> + mmc0_pins: mmc0-pins { >> + pins = "PF0", "PF1", "PF2", "PF3", >> + "PF4", "PF5"; >> + function = "mmc0"; >> + drive-strength = <30>; >> + bias-pull-up; >> + }; >> + >> + mmc2_pins: mmc2-pins { >> + pins = "PC1", "PC4", "PC5", "PC6", >> + "PC7", "PC8", "PC9", "PC10", >> + "PC11", "PC12", "PC13", "PC14"; >> + function = "mmc2"; >> + drive-strength = <30>; >> + bias-pull-up; >> + }; >> + >> uart0_ph_pins: uart0-ph { >> pins = "PH0", "PH1"; >> function = "uart0"; >> }; >> }; >> >> + mmc0: mmc@4020000 { >> + compatible = "allwinner,sun50i-h6-mmc"; > >This should be: > compatible = "allwinner,sun50i-h6-mmc", > "allwinner,sun50i-a64-mmc"; I'm intended to not add A64 compatible, as H6 is a quite new design (new process) and there might be different behavior, even on mmc0/1. > >> + reg = <0x04020000 0x1000>; >> + clocks = <&ccu CLK_BUS_MMC0>, <&ccu CLK_MMC0>; >> + clock-names = "ahb", "mmc"; >> + resets = <&ccu RST_BUS_MMC0>; >> + reset-names = "ahb"; >> + interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; >> + status = "disabled"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + }; >> + >> + mmc1: mmc@4021000 { >> + compatible = "allwinner,sun50i-h6-mmc"; > >same here > >> + reg = <0x04021000 0x1000>; >> + clocks = <&ccu CLK_BUS_MMC1>, <&ccu CLK_MMC1>; >> + clock-names = "ahb", "mmc"; >> + resets = <&ccu RST_BUS_MMC1>; >> + reset-names = "ahb"; >> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; >> + status = "disabled"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + }; >> + >> + mmc2: mmc@4022000 { >> + compatible = "allwinner,sun50i-h6-emmc"; > >and here: > compatible = "allwinner,sun50i-h6-emmc", > "allwinner,sun50i-a64-emmc"; MMC2 on H6 has EMCE capability, so surely there should only be H6 compatible, and no A64 one. > >The rest looks correct to me. > >Cheers, >Andre. > >> + reg = <0x04022000 0x1000>; >> + clocks = <&ccu CLK_BUS_MMC2>, <&ccu CLK_MMC2>; >> + clock-names = "ahb", "mmc"; >> + resets = <&ccu RST_BUS_MMC2>; >> + reset-names = "ahb"; >> + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; >> + status = "disabled"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + }; >> + >> uart0: serial@5000000 { >> compatible = "snps,dw-apb-uart"; >> reg = <0x05000000 0x400>; >>
Hi, On 27/04/18 09:36, Icenowy Zheng wrote: > > > 于 2018年4月27日 GMT+08:00 上午12:45:38, Andre Przywara <andre.przywara@arm.com> 写到: >> Hi, >> >> On 26/04/18 15:07, Icenowy Zheng wrote: >>> The Allwinner H6 SoC have 3 MMC controllers. >>> >>> Add device tree nodes for them. >>> >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>> --- >>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 >> ++++++++++++++++++++++++++++ >>> 1 file changed, 56 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >> b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>> index 4debc3962830..3cbfc035c979 100644 >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>> @@ -124,12 +124,68 @@ >>> interrupt-controller; >>> #interrupt-cells = <3>; >>> >>> + mmc0_pins: mmc0-pins { >>> + pins = "PF0", "PF1", "PF2", "PF3", >>> + "PF4", "PF5"; >>> + function = "mmc0"; >>> + drive-strength = <30>; >>> + bias-pull-up; >>> + }; >>> + >>> + mmc2_pins: mmc2-pins { >>> + pins = "PC1", "PC4", "PC5", "PC6", >>> + "PC7", "PC8", "PC9", "PC10", >>> + "PC11", "PC12", "PC13", "PC14"; >>> + function = "mmc2"; >>> + drive-strength = <30>; >>> + bias-pull-up; >>> + }; >>> + >>> uart0_ph_pins: uart0-ph { >>> pins = "PH0", "PH1"; >>> function = "uart0"; >>> }; >>> }; >>> >>> + mmc0: mmc@4020000 { >>> + compatible = "allwinner,sun50i-h6-mmc"; >> >> This should be: >> compatible = "allwinner,sun50i-h6-mmc", >> "allwinner,sun50i-a64-mmc"; > > I'm intended to not add A64 compatible, as > H6 is a quite new design > (new process) and there might be different behavior, even on mmc0/1. But as your patch proves, it is fully backwards compatible: An A64 driver works with this device. And this is what this compatible string list says: If your system does not have a specific H6 driver, you can use an A64 driver. You might not get all the (potentially) new features, but it covers everything the A64 has. And a new silicon process doesn't matter here, since the software interface is unchanged. *If* we find bugs, we can add quirks matching on the H6 compatible string - that's why we put it here already, despite having a matching string in the kernel at the moment. >>> + reg = <0x04020000 0x1000>; >>> + clocks = <&ccu CLK_BUS_MMC0>, <&ccu CLK_MMC0>; >>> + clock-names = "ahb", "mmc"; >>> + resets = <&ccu RST_BUS_MMC0>; >>> + reset-names = "ahb"; >>> + interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; >>> + status = "disabled"; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + }; >>> + >>> + mmc1: mmc@4021000 { >>> + compatible = "allwinner,sun50i-h6-mmc"; >> >> same here >> >>> + reg = <0x04021000 0x1000>; >>> + clocks = <&ccu CLK_BUS_MMC1>, <&ccu CLK_MMC1>; >>> + clock-names = "ahb", "mmc"; >>> + resets = <&ccu RST_BUS_MMC1>; >>> + reset-names = "ahb"; >>> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; >>> + status = "disabled"; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + }; >>> + >>> + mmc2: mmc@4022000 { >>> + compatible = "allwinner,sun50i-h6-emmc"; >> >> and here: >> compatible = "allwinner,sun50i-h6-emmc", >> "allwinner,sun50i-a64-emmc"; > > MMC2 on H6 has EMCE capability, so surely there should > only be H6 compatible, and no A64 one. Same as above, the A64 eMMC is a subset of the H6 eMMC, so the A64 eMMC driver can drive the H6 as well. And again your code proves that, because it behaves exactly the same as for the A64. In case we ever get support for the EMCE, we add the new compatible string to the driver and tie it to the new feature. So newer kernels can use this feature, older kernel will just not, but can happily use the eMMC anyway. Cheers, Andre.
于 2018年4月27日 GMT+08:00 下午5:18:23, Andre Przywara <andre.przywara@arm.com> 写到: >Hi, > >On 27/04/18 09:36, Icenowy Zheng wrote: >> >> >> 于 2018年4月27日 GMT+08:00 上午12:45:38, Andre Przywara ><andre.przywara@arm.com> 写到: >>> Hi, >>> >>> On 26/04/18 15:07, Icenowy Zheng wrote: >>>> The Allwinner H6 SoC have 3 MMC controllers. >>>> >>>> Add device tree nodes for them. >>>> >>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>>> --- >>>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 >>> ++++++++++++++++++++++++++++ >>>> 1 file changed, 56 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>> b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>> index 4debc3962830..3cbfc035c979 100644 >>>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>> @@ -124,12 +124,68 @@ >>>> interrupt-controller; >>>> #interrupt-cells = <3>; >>>> >>>> + mmc0_pins: mmc0-pins { >>>> + pins = "PF0", "PF1", "PF2", "PF3", >>>> + "PF4", "PF5"; >>>> + function = "mmc0"; >>>> + drive-strength = <30>; >>>> + bias-pull-up; >>>> + }; >>>> + >>>> + mmc2_pins: mmc2-pins { >>>> + pins = "PC1", "PC4", "PC5", "PC6", >>>> + "PC7", "PC8", "PC9", "PC10", >>>> + "PC11", "PC12", "PC13", "PC14"; >>>> + function = "mmc2"; >>>> + drive-strength = <30>; >>>> + bias-pull-up; >>>> + }; >>>> + >>>> uart0_ph_pins: uart0-ph { >>>> pins = "PH0", "PH1"; >>>> function = "uart0"; >>>> }; >>>> }; >>>> >>>> + mmc0: mmc@4020000 { >>>> + compatible = "allwinner,sun50i-h6-mmc"; >>> >>> This should be: >>> compatible = "allwinner,sun50i-h6-mmc", >>> "allwinner,sun50i-a64-mmc"; >> >> I'm intended to not add A64 compatible, as >> H6 is a quite new design >> (new process) and there might be different behavior, even on mmc0/1. > >But as your patch proves, it is fully backwards compatible: An A64 >driver works with this device. No, my patch only proves "the current A64 driver works with this device", not "Any A64 driver works with device", as the current driver doesn't fully use the capability provided by A64 MMC cobtrollers. >And this is what this compatible string list says: If your system does >not have a specific H6 driver, you can use an A64 driver. >You might not get all the (potentially) new features, but it covers >everything the A64 has. > >And a new silicon process doesn't matter here, since the software >interface is unchanged. *If* we find bugs, we can add quirks matching I think there's timing parameters for higher speed bins which are different among chips. As we have currently no support for speed bins higher than DDR50, they're not added yet. >on >the H6 compatible string - that's why we put it here already, despite >having a matching string in the kernel at the moment. Device tree is not driver data but hardware description, so it should follow "how the device is formed" rather than "how the device works". > >>>> + reg = <0x04020000 0x1000>; >>>> + clocks = <&ccu CLK_BUS_MMC0>, <&ccu CLK_MMC0>; >>>> + clock-names = "ahb", "mmc"; >>>> + resets = <&ccu RST_BUS_MMC0>; >>>> + reset-names = "ahb"; >>>> + interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; >>>> + status = "disabled"; >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + }; >>>> + >>>> + mmc1: mmc@4021000 { >>>> + compatible = "allwinner,sun50i-h6-mmc"; >>> >>> same here >>> >>>> + reg = <0x04021000 0x1000>; >>>> + clocks = <&ccu CLK_BUS_MMC1>, <&ccu CLK_MMC1>; >>>> + clock-names = "ahb", "mmc"; >>>> + resets = <&ccu RST_BUS_MMC1>; >>>> + reset-names = "ahb"; >>>> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; >>>> + status = "disabled"; >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + }; >>>> + >>>> + mmc2: mmc@4022000 { >>>> + compatible = "allwinner,sun50i-h6-emmc"; >>> >>> and here: >>> compatible = "allwinner,sun50i-h6-emmc", >>> "allwinner,sun50i-a64-emmc"; >> >> MMC2 on H6 has EMCE capability, so surely there should >> only be H6 compatible, and no A64 one. > >Same as above, the A64 eMMC is a subset of the H6 eMMC, so the A64 eMMC >driver can drive the H6 as well. And again your code proves that, >because it behaves exactly the same as for the A64. >In case we ever get support for the EMCE, we add the new compatible >string to the driver and tie it to the new feature. So newer kernels >can >use this feature, older kernel will just not, but can happily use the >eMMC anyway. > >Cheers, >Andre.
On 27/04/18 10:23, Icenowy Zheng wrote: > > > 于 2018年4月27日 GMT+08:00 下午5:18:23, Andre Przywara <andre.przywara@arm.com> 写到: >> Hi, >> >> On 27/04/18 09:36, Icenowy Zheng wrote: >>> >>> >>> 于 2018年4月27日 GMT+08:00 上午12:45:38, Andre Przywara >> <andre.przywara@arm.com> 写到: >>>> Hi, >>>> >>>> On 26/04/18 15:07, Icenowy Zheng wrote: >>>>> The Allwinner H6 SoC have 3 MMC controllers. >>>>> >>>>> Add device tree nodes for them. >>>>> >>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>>>> --- >>>>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 >>>> ++++++++++++++++++++++++++++ >>>>> 1 file changed, 56 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>> b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>>> index 4debc3962830..3cbfc035c979 100644 >>>>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi >>>>> @@ -124,12 +124,68 @@ >>>>> interrupt-controller; >>>>> #interrupt-cells = <3>; >>>>> >>>>> + mmc0_pins: mmc0-pins { >>>>> + pins = "PF0", "PF1", "PF2", "PF3", >>>>> + "PF4", "PF5"; >>>>> + function = "mmc0"; >>>>> + drive-strength = <30>; >>>>> + bias-pull-up; >>>>> + }; >>>>> + >>>>> + mmc2_pins: mmc2-pins { >>>>> + pins = "PC1", "PC4", "PC5", "PC6", >>>>> + "PC7", "PC8", "PC9", "PC10", >>>>> + "PC11", "PC12", "PC13", "PC14"; >>>>> + function = "mmc2"; >>>>> + drive-strength = <30>; >>>>> + bias-pull-up; >>>>> + }; >>>>> + >>>>> uart0_ph_pins: uart0-ph { >>>>> pins = "PH0", "PH1"; >>>>> function = "uart0"; >>>>> }; >>>>> }; >>>>> >>>>> + mmc0: mmc@4020000 { >>>>> + compatible = "allwinner,sun50i-h6-mmc"; >>>> >>>> This should be: >>>> compatible = "allwinner,sun50i-h6-mmc", >>>> "allwinner,sun50i-a64-mmc"; >>> >>> I'm intended to not add A64 compatible, as >>> H6 is a quite new design >>> (new process) and there might be different behavior, even on mmc0/1. >> >> But as your patch proves, it is fully backwards compatible: An A64 >> driver works with this device. > > No, my patch only proves "the current A64 driver works > with this device", not "Any A64 driver works with device", as > the current driver doesn't fully use the capability provided > by A64 MMC cobtrollers. Good point, but I still believe every A64 driver would be capable of driving an H6 MMC controller, .... >> And this is what this compatible string list says: If your system does >> not have a specific H6 driver, you can use an A64 driver. >> You might not get all the (potentially) new features, but it covers >> everything the A64 has. >> >> And a new silicon process doesn't matter here, since the software >> interface is unchanged. *If* we find bugs, we can add quirks matching > > I think there's timing parameters for higher speed bins which > are different among chips. As we have currently no support > for speed bins higher than DDR50, they're not added yet. True, but what are those differences? I compared the A64 and H6 manuals side by side, the differences I found are: SMHC_FIFOTH[+0x40]: BSIZE_OF_TRANS[30:28]: - H6 supports 16 transfers for SMHC0 also. other parameters: - H6 recommends better values for SMHC0 also SMHC_CSDC[+0x54]: - H6 doesn't mention restriction to SMHC2 (though this might be a mistake) SMHC_NTSR_REG[+0x5C]: - H6 defines fields for bits[24:8] SMHC_EMCE[+0x64] and SMHC_EMCE_DBG[+0x68]: - H6 adds, for EMCE support EMMC_DDR_SBIT_DET_REG[0x10c]: - A64 doesn't mention restriction to SMHC2, but I believe this is a mistake SMHC_EMCE_BMn[0x150 + 0x4 * 0..31] - H6 adds, for EMCE support All those pieces are only *additions* to the H6 over the A64, so don't affect backwards compatibility. >> on >> the H6 compatible string - that's why we put it here already, despite >> having a matching string in the kernel at the moment. > > Device tree is not driver data but hardware description, so > it should follow "how the device is formed" rather than > "how the device works". True, but as shown above, the compatibility is really at the device level. Unless you have any other information ... Cheers, Andre.
在 2018-04-27五的 22:25 +0100，André Przywara写道： > On 27/04/18 10:23, Icenowy Zheng wrote: > > > > > > 于 2018年4月27日 GMT+08:00 下午5:18:23, Andre Przywara <andre.przywara@ar > > m.com> 写到: > > > Hi, > > > > > > On 27/04/18 09:36, Icenowy Zheng wrote: > > > > > > > > > > > > 于 2018年4月27日 GMT+08:00 上午12:45:38, Andre Przywara > > > > > > <andre.przywara@arm.com> 写到: > > > > > Hi, > > > > > > > > > > On 26/04/18 15:07, Icenowy Zheng wrote: > > > > > > The Allwinner H6 SoC have 3 MMC controllers. > > > > > > > > > > > > Add device tree nodes for them. > > > > > > > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > > > > --- > > > > > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 > > > > > > > > > > ++++++++++++++++++++++++++++ > > > > > > 1 file changed, 56 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > > > > > > > > > b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > > > > > index 4debc3962830..3cbfc035c979 100644 > > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > > > > > @@ -124,12 +124,68 @@ > > > > > > interrupt-controller; > > > > > > #interrupt-cells = <3>; > > > > > > > > > > > > + mmc0_pins: mmc0-pins { > > > > > > + pins = "PF0", "PF1", > > > > > > "PF2", "PF3", > > > > > > + "PF4", "PF5"; > > > > > > + function = "mmc0"; > > > > > > + drive-strength = <30>; > > > > > > + bias-pull-up; > > > > > > + }; > > > > > > + > > > > > > + mmc2_pins: mmc2-pins { > > > > > > + pins = "PC1", "PC4", > > > > > > "PC5", "PC6", > > > > > > + "PC7", "PC8", > > > > > > "PC9", "PC10", > > > > > > + "PC11", "PC12", > > > > > > "PC13", "PC14"; > > > > > > + function = "mmc2"; > > > > > > + drive-strength = <30>; > > > > > > + bias-pull-up; > > > > > > + }; > > > > > > + > > > > > > uart0_ph_pins: uart0-ph { > > > > > > pins = "PH0", "PH1"; > > > > > > function = "uart0"; > > > > > > }; > > > > > > }; > > > > > > > > > > > > + mmc0: mmc@4020000 { > > > > > > + compatible = "allwinner,sun50i-h6- > > > > > > mmc"; > > > > > > > > > > This should be: > > > > > compatible = "allwinner,sun50i-h6-mmc", > > > > > "allwinner,sun50i-a64- > > > > > mmc"; > > > > > > > > I'm intended to not add A64 compatible, as > > > > H6 is a quite new design > > > > (new process) and there might be different behavior, even on > > > > mmc0/1. > > > > > > But as your patch proves, it is fully backwards compatible: An > > > A64 > > > driver works with this device. > > > > No, my patch only proves "the current A64 driver works > > with this device", not "Any A64 driver works with device", as > > the current driver doesn't fully use the capability provided > > by A64 MMC cobtrollers. > > Good point, but I still believe every A64 driver would be capable of > driving an H6 MMC controller, .... > > > > And this is what this compatible string list says: If your system > > > does > > > not have a specific H6 driver, you can use an A64 driver. > > > You might not get all the (potentially) new features, but it > > > covers > > > everything the A64 has. > > > > > > And a new silicon process doesn't matter here, since the software > > > interface is unchanged. *If* we find bugs, we can add quirks > > > matching > > > > I think there's timing parameters for higher speed bins which > > are different among chips. As we have currently no support > > for speed bins higher than DDR50, they're not added yet. > > True, but what are those differences? I compared the A64 and H6 > manuals > side by side, the differences I found are: > SMHC_FIFOTH[+0x40]: > BSIZE_OF_TRANS[30:28]: > - H6 supports 16 transfers for SMHC0 also. > other parameters: > - H6 recommends better values for SMHC0 also > SMHC_CSDC[+0x54]: > - H6 doesn't mention restriction to SMHC2 > (though this might be a mistake) > SMHC_NTSR_REG[+0x5C]: > - H6 defines fields for bits[24:8] > SMHC_EMCE[+0x64] and SMHC_EMCE_DBG[+0x68]: > - H6 adds, for EMCE support > EMMC_DDR_SBIT_DET_REG[0x10c]: > - A64 doesn't mention restriction to SMHC2, > but I believe this is a mistake > SMHC_EMCE_BMn[0x150 + 0x4 * 0..31] > - H6 adds, for EMCE support > > All those pieces are only *additions* to the H6 over the A64, so > don't > affect backwards compatibility. > > > > on > > > the H6 compatible string - that's why we put it here already, > > > despite > > > having a matching string in the kernel at the moment. > > > > Device tree is not driver data but hardware description, so > > it should follow "how the device is formed" rather than > > "how the device works". > > True, but as shown above, the compatibility is really at the device > level. > Unless you have any other information ... Rob, could you answer whether we should add the A64 compatible or not? > > Cheers, > Andre. > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi index 4debc3962830..3cbfc035c979 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi @@ -124,12 +124,68 @@ interrupt-controller; #interrupt-cells = <3>; + mmc0_pins: mmc0-pins { + pins = "PF0", "PF1", "PF2", "PF3", + "PF4", "PF5"; + function = "mmc0"; + drive-strength = <30>; + bias-pull-up; + }; + + mmc2_pins: mmc2-pins { + pins = "PC1", "PC4", "PC5", "PC6", + "PC7", "PC8", "PC9", "PC10", + "PC11", "PC12", "PC13", "PC14"; + function = "mmc2"; + drive-strength = <30>; + bias-pull-up; + }; + uart0_ph_pins: uart0-ph { pins = "PH0", "PH1"; function = "uart0"; }; }; + mmc0: mmc@4020000 { + compatible = "allwinner,sun50i-h6-mmc"; + reg = <0x04020000 0x1000>; + clocks = <&ccu CLK_BUS_MMC0>, <&ccu CLK_MMC0>; + clock-names = "ahb", "mmc"; + resets = <&ccu RST_BUS_MMC0>; + reset-names = "ahb"; + interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + #address-cells = <1>; + #size-cells = <0>; + }; + + mmc1: mmc@4021000 { + compatible = "allwinner,sun50i-h6-mmc"; + reg = <0x04021000 0x1000>; + clocks = <&ccu CLK_BUS_MMC1>, <&ccu CLK_MMC1>; + clock-names = "ahb", "mmc"; + resets = <&ccu RST_BUS_MMC1>; + reset-names = "ahb"; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + #address-cells = <1>; + #size-cells = <0>; + }; + + mmc2: mmc@4022000 { + compatible = "allwinner,sun50i-h6-emmc"; + reg = <0x04022000 0x1000>; + clocks = <&ccu CLK_BUS_MMC2>, <&ccu CLK_MMC2>; + clock-names = "ahb", "mmc"; + resets = <&ccu RST_BUS_MMC2>; + reset-names = "ahb"; + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + #address-cells = <1>; + #size-cells = <0>; + }; + uart0: serial@5000000 { compatible = "snps,dw-apb-uart"; reg = <0x05000000 0x400>;
The Allwinner H6 SoC have 3 MMC controllers. Add device tree nodes for them. Signed-off-by: Icenowy Zheng <icenowy@aosc.io> --- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56 ++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)