Hello Adam, On 28.03.22 12:47, Adam Ford wrote: > On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote: >> >> Hello Adam, >> >> On 27.03.22 14:38, Adam Ford wrote: >>> The SDHC controller in the imx8mp has the same controller >>> as the imx8mm which supports HS400-ES. Change the compatible >>> fallback to imx8mm to enable it. >> >> I believe that's a shortcoming of the Linux driver, which should explicitly list >> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it. >> >> I find dropping compatibles problematic, because like Linux matching >> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match >> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc. >> >> I'd prefer that either the kernel driver gains extra compatibles or that >> the DTS lists extra compatibles and we refrain from dropping existing >> (correct) ones. >> > > I would argue that imx7d is not correct since the IP blocks between > imx7d and imx8mm have different flags/quirks. One of which includes > HS400-ES, but there are other differences as well. The DTS currently says that an fsl,imx7d-usdhc is a subset of an fsl,imx8mm-usdhc. So a driver could treat both HW the exact same by focusing on the i.MX7D parts. Linux apparently did exactly that so far. Is this not accurate? >> What do you think? > > From my understanding of the fallback compatibility strings is to > avoid having to add more and more compatible strings to the drivers > when they do not serve a functional purpose. Based On a conversation > with Krzysztof [1], he suggested we update the YAML file based on the > fallback, but he wanted NXP to give their feedback as to what the > right fallback strings should be. Haibo from NXP sent me a hierarchy > [1] which is what I used to update the YAML file. Based on the YAML > file, the fallback in each DTSI file was updated to ensure the use of > the proper IP block. Myself I am in favor of moving to three compatibles instead of dropping one. For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible, but for existing device trees, this may introduce needless potential breakage for other software that also uses Linux device trees. Cheers, Ahmad > > adam > > [1] - https://lore.kernel.org/linux-arm-kernel/CAHCN7xLWoUGi-jfxR2a0gvEFkPT3USUEb+8U3CCqCb5wWEJ8xw@xxxxxxxxxxxxxx/T/ > >> >> Cheers, >> Ahmad >> >>> Signed-off-by: Adam Ford <aford173@xxxxxxxxx> >>> --- >>> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi >>> index 794d75173cf5..d5ee1520f1fe 100644 >>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi >>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi >>> @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 { >>> }; >>> >>> usdhc1: mmc@30b40000 { >>> - compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc"; >>> + compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc"; >>> reg = <0x30b40000 0x10000>; >>> interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>; >>> clocks = <&clk IMX8MP_CLK_DUMMY>, >>> @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 { >>> }; >>> >>> usdhc2: mmc@30b50000 { >>> - compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc"; >>> + compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc"; >>> reg = <0x30b50000 0x10000>; >>> interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>; >>> clocks = <&clk IMX8MP_CLK_DUMMY>, >>> @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 { >>> }; >>> >>> usdhc3: mmc@30b60000 { >>> - compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc"; >>> + compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc"; >>> reg = <0x30b60000 0x10000>; >>> interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>; >>> clocks = <&clk IMX8MP_CLK_DUMMY>, >> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |