All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Adam Ford <aford173@gmail.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: linux-mmc <linux-mmc@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Fabio Estevam <festevam@gmail.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Adam Ford-BE <aford@beaconembedded.com>,
	Haibo Chen <haibo.chen@nxp.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	arm-soc <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
Date: Mon, 28 Mar 2022 13:49:18 +0200	[thread overview]
Message-ID: <5c24c12b-3a12-1e18-9f03-2c54cad30bf9@kernel.org> (raw)
In-Reply-To: <74e74ea8-1554-bf08-b0ea-36e77259cb18@pengutronix.de>

On 28/03/2022 13:09, Ahmad Fatoum wrote:
> Hello Adam,
> 
> On 28.03.22 12:47, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> 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.
> 

Affecting existing users is indeed a concern with this approach, because
in-kernel DTS might be used in other projects as well.

I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
compatible with fsl,imx7d-usdhc. It's not about driver, but about
hardware and programming model. imx8mm can support additional features
and still be compatible with imx7d. However if any flags of imx7d are
actually not valid for imx8mm, then it's different case.


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Adam Ford <aford173@gmail.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: linux-mmc <linux-mmc@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Fabio Estevam <festevam@gmail.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Adam Ford-BE <aford@beaconembedded.com>,
	Haibo Chen <haibo.chen@nxp.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	arm-soc <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
Date: Mon, 28 Mar 2022 13:49:18 +0200	[thread overview]
Message-ID: <5c24c12b-3a12-1e18-9f03-2c54cad30bf9@kernel.org> (raw)
In-Reply-To: <74e74ea8-1554-bf08-b0ea-36e77259cb18@pengutronix.de>

On 28/03/2022 13:09, Ahmad Fatoum wrote:
> Hello Adam,
> 
> On 28.03.22 12:47, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> 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.
> 

Affecting existing users is indeed a concern with this approach, because
in-kernel DTS might be used in other projects as well.

I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
compatible with fsl,imx7d-usdhc. It's not about driver, but about
hardware and programming model. imx8mm can support additional features
and still be compatible with imx7d. However if any flags of imx7d are
actually not valid for imx8mm, then it's different case.


Best regards,
Krzysztof

  reply	other threads:[~2022-03-28 11:50 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-27 12:38 [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks Adam Ford
2022-03-27 12:38 ` Adam Ford
2022-03-27 12:38 ` [PATCH 2/5] arm64: dts: imx8mn: Enable HS400-ES Adam Ford
2022-03-27 12:38   ` Adam Ford
2022-03-27 12:38 ` [PATCH 3/5] arm64: dts: imx8mp: " Adam Ford
2022-03-27 12:38   ` Adam Ford
2022-03-28  7:20   ` Ahmad Fatoum
2022-03-28  7:20     ` Ahmad Fatoum
2022-03-28 10:47     ` Adam Ford
2022-03-28 10:47       ` Adam Ford
2022-03-28 11:09       ` Ahmad Fatoum
2022-03-28 11:09         ` Ahmad Fatoum
2022-03-28 11:49         ` Krzysztof Kozlowski [this message]
2022-03-28 11:49           ` Krzysztof Kozlowski
2022-03-28 12:45           ` Adam Ford
2022-03-28 12:45             ` Adam Ford
2022-03-28 12:56             ` Krzysztof Kozlowski
2022-03-28 12:56               ` Krzysztof Kozlowski
2022-03-28 13:06               ` Adam Ford
2022-03-28 13:06                 ` Adam Ford
2022-03-28 13:12                 ` Ahmad Fatoum
2022-03-28 13:12                   ` Ahmad Fatoum
2022-03-28 13:15                   ` Krzysztof Kozlowski
2022-03-28 13:15                     ` Krzysztof Kozlowski
2022-03-28 13:17                 ` Krzysztof Kozlowski
2022-03-28 13:17                   ` Krzysztof Kozlowski
2022-03-28 13:07               ` Ahmad Fatoum
2022-03-28 13:07                 ` Ahmad Fatoum
2022-03-28 13:14                 ` Krzysztof Kozlowski
2022-03-28 13:14                   ` Krzysztof Kozlowski
2022-03-28 13:42                   ` Ahmad Fatoum
2022-03-28 13:42                     ` Ahmad Fatoum
2022-03-28 17:35                     ` Krzysztof Kozlowski
2022-03-28 17:35                       ` Krzysztof Kozlowski
2022-03-27 12:38 ` [PATCH 4/5] arm64: dts: imx8qxp: Remove imx7d-usdhc compatible fallback Adam Ford
2022-03-27 12:38   ` Adam Ford
2022-03-27 12:38 ` [PATCH 5/5] arm64: dts: imx8qm: Remove fsl,imx7d-usdhc " Adam Ford
2022-03-27 12:38   ` [PATCH 5/5] arm64: dts: imx8qm: Remove fsl, imx7d-usdhc " Adam Ford
2022-03-27 19:26 ` [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks Krzysztof Kozlowski
2022-03-27 19:26   ` Krzysztof Kozlowski
2022-03-31 13:04 ` Ulf Hansson
2022-03-31 13:04   ` Ulf Hansson
2022-03-31 16:00   ` Adam Ford
2022-03-31 16:00     ` Adam Ford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5c24c12b-3a12-1e18-9f03-2c54cad30bf9@kernel.org \
    --to=krzk@kernel.org \
    --cc=a.fatoum@pengutronix.de \
    --cc=aford173@gmail.com \
    --cc=aford@beaconembedded.com \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=haibo.chen@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.