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.
What do you think?
Cheers,
Ahmad
> Signed-off-by: Adam Ford <[email protected]>
> ---
> 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 |
On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <[email protected]> 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.
> 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.
adam
[1] - https://lore.kernel.org/linux-arm-kernel/CAHCN7xLWoUGi-jfxR2a0gvEFkPT3USUEb+8U3CCqCb5wWEJ8xw@mail.gmail.com/T/
>
> Cheers,
> Ahmad
>
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > 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 |