[PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Yangbo Lu]

Enable interrupt mode to detect card instead of polling mode
for P1020/P4080/P5020/P5040/T1040 by removing the quirk
SDHCI_QUIRK_BROKEN_CARD_DETECTION. This could improve data
transferring performance and avoid the call trace caused by
polling card status sometime.

Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
----
Changes for v2:
	- Aligned all "of_device_is_compatibles" in same column
---
 drivers/mmc/host/sdhci-pltfm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index c5b01d6..97128f3 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -102,6 +102,13 @@ void sdhci_get_of_property(struct platform_device *pdev)
 		    of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
 			host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+		if (of_device_is_compatible(np, "fsl,p5040-esdhc") ||
+		    of_device_is_compatible(np, "fsl,p5020-esdhc") ||
+		    of_device_is_compatible(np, "fsl,p4080-esdhc") ||
+		    of_device_is_compatible(np, "fsl,p1020-esdhc") ||
+		    of_device_is_compatible(np, "fsl,t1040-esdhc"))
+			host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
 		clk = of_get_property(np, "clock-frequency", &size);
 		if (clk && size == sizeof(*clk) && *clk)
 			pltfm_host->clock = be32_to_cpup(clk);
-- 
2.1.0.27.g96db324

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Ulf Hansson]

On 15 May 2015 at 04:46, Yangbo Lu <yangbo.lu@freescale.com> wrote:
> Enable interrupt mode to detect card instead of polling mode
> for P1020/P4080/P5020/P5040/T1040 by removing the quirk
> SDHCI_QUIRK_BROKEN_CARD_DETECTION. This could improve data
> transferring performance and avoid the call trace caused by
> polling card status sometime.
>
> Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
> ----
> Changes for v2:
>         - Aligned all "of_device_is_compatibles" in same column
> ---
>  drivers/mmc/host/sdhci-pltfm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index c5b01d6..97128f3 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -102,6 +102,13 @@ void sdhci_get_of_property(struct platform_device *pdev)
>                     of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
>                         host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> +               if (of_device_is_compatible(np, "fsl,p5040-esdhc") ||
> +                   of_device_is_compatible(np, "fsl,p5020-esdhc") ||
> +                   of_device_is_compatible(np, "fsl,p4080-esdhc") ||
> +                   of_device_is_compatible(np, "fsl,p1020-esdhc") ||
> +                   of_device_is_compatible(np, "fsl,t1040-esdhc"))
> +                       host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;

This looks strange to me.

Why not just change the DT files for the relevant platforms to not
enable "broken-cd"?

Kind regards
Uffe

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Ulf Hansson]

On 18 May 2015 at 11:30, Lu Y.B. <yangbo.lu@freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>> Sent: Monday, May 18, 2015 5:04 PM
>> To: Lu Yangbo-B47093
>> Cc: linux-mmc; Chris Ball; Wood Scott-B07421
>> Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to
>> detect card
>>
>> On 15 May 2015 at 04:46, Yangbo Lu <yangbo.lu@freescale.com> wrote:
>> > Enable interrupt mode to detect card instead of polling mode for
>> > P1020/P4080/P5020/P5040/T1040 by removing the quirk
>> > SDHCI_QUIRK_BROKEN_CARD_DETECTION. This could improve data
>> > transferring performance and avoid the call trace caused by polling
>> > card status sometime.
>> >
>> > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
>> > ----
>> > Changes for v2:
>> >         - Aligned all "of_device_is_compatibles" in same column
>> > ---
>> >  drivers/mmc/host/sdhci-pltfm.c | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> >
>> > diff --git a/drivers/mmc/host/sdhci-pltfm.c
>> > b/drivers/mmc/host/sdhci-pltfm.c index c5b01d6..97128f3 100644
>> > --- a/drivers/mmc/host/sdhci-pltfm.c
>> > +++ b/drivers/mmc/host/sdhci-pltfm.c
>> > @@ -102,6 +102,13 @@ void sdhci_get_of_property(struct platform_device
>> *pdev)
>> >                     of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
>> >                         host->quirks |=
>> > SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>> >
>> > +               if (of_device_is_compatible(np, "fsl,p5040-esdhc") ||
>> > +                   of_device_is_compatible(np, "fsl,p5020-esdhc") ||
>> > +                   of_device_is_compatible(np, "fsl,p4080-esdhc") ||
>> > +                   of_device_is_compatible(np, "fsl,p1020-esdhc") ||
>> > +                   of_device_is_compatible(np, "fsl,t1040-esdhc"))
>> > +                       host->quirks &=
>> > + ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>>
>> This looks strange to me.
>>
>> Why not just change the DT files for the relevant platforms to not enable
>> "broken-cd"?
>>
> Thanks Uffe.
> Because most platforms using sdhci-of-esdhc have broken CD, we hope this quirk is selected in default.
> Only several platforms could use CD to detect and we clear this bit for them.

Hmm.

Those platforms could still update their DT files and add "broken-cd",
since that would be the proper description of the HW. Once that's
done, it would enable you to remove the
SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, right?

Kind regards
Uffe

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Scott Wood]

On Mon, 2015-05-18 at 04:55 -0500, Lu Yangbo-B47093 wrote:
> 
> 
> > -----Original Message-----
> > From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> > Sent: Monday, May 18, 2015 5:50 PM
> > To: Lu Yangbo-B47093
> > Cc: linux-mmc; Chris Ball; Wood Scott-B07421
> > Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to
> > detect card
> > 
> > On 18 May 2015 at 11:30, Lu Y.B. <yangbo.lu@freescale.com> wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> > >> Sent: Monday, May 18, 2015 5:04 PM
> > >> To: Lu Yangbo-B47093
> > >> Cc: linux-mmc; Chris Ball; Wood Scott-B07421
> > >> Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode
> > >> to detect card
> > >>
> > >> On 15 May 2015 at 04:46, Yangbo Lu <yangbo.lu@freescale.com> wrote:
> > >> > Enable interrupt mode to detect card instead of polling mode for
> > >> > P1020/P4080/P5020/P5040/T1040 by removing the quirk
> > >> > SDHCI_QUIRK_BROKEN_CARD_DETECTION. This could improve data
> > >> > transferring performance and avoid the call trace caused by polling
> > >> > card status sometime.
> > >> >
> > >> > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
> > >> > ----
> > >> > Changes for v2:
> > >> >         - Aligned all "of_device_is_compatibles" in same column
> > >> > ---
> > >> >  drivers/mmc/host/sdhci-pltfm.c | 7 +++++++
> > >> >  1 file changed, 7 insertions(+)
> > >> >
> > >> > diff --git a/drivers/mmc/host/sdhci-pltfm.c
> > >> > b/drivers/mmc/host/sdhci-pltfm.c index c5b01d6..97128f3 100644
> > >> > --- a/drivers/mmc/host/sdhci-pltfm.c
> > >> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> > >> > @@ -102,6 +102,13 @@ void sdhci_get_of_property(struct
> > >> > platform_device
> > >> *pdev)
> > >> >                     of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
> > >> >                         host->quirks |=
> > >> > SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> > >> >
> > >> > +               if (of_device_is_compatible(np, "fsl,p5040-esdhc")
> > ||
> > >> > +                   of_device_is_compatible(np, "fsl,p5020-esdhc")
> > ||
> > >> > +                   of_device_is_compatible(np, "fsl,p4080-esdhc")
> > ||
> > >> > +                   of_device_is_compatible(np, "fsl,p1020-esdhc")
> > ||
> > >> > +                   of_device_is_compatible(np, "fsl,t1040-esdhc"))
> > >> > +                       host->quirks &=
> > >> > + ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> > >>
> > >> This looks strange to me.
> > >>
> > >> Why not just change the DT files for the relevant platforms to not
> > >> enable "broken-cd"?
> > >>
> > > Thanks Uffe.
> > > Because most platforms using sdhci-of-esdhc have broken CD, we hope
> > this quirk is selected in default.
> > > Only several platforms could use CD to detect and we clear this bit for
> > them.
> > 
> > Hmm.
> > 
> > Those platforms could still update their DT files and add "broken-cd",
> > since that would be the proper description of the HW. Once that's done,
> > it would enable you to remove the SDHCI_QUIRK_BROKEN_CARD_DETECTION as
> > default, right?
> > 
> Yes, and if remove SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, 'borken-cd' would be needed to be added for most platforms using esdhc.

I was OK with changing the device tree if it just meant that things that
previously didn't work now work.  I'm not OK with requiring the device
trees to change in order for things that already work to stay working.

In general it is not reasonable to expect device trees to enumerate
hardware bugs since the bugs (or the need to work aronud them) are often
discovered after the device tree has been created.

Yangbo, when I asked why you couldn't use SVR you said that it was a
common SDHC file.  But, the file that sets
SDHCI_QUIRK_BROKEN_CARD_DETECTION in the first place is sdhci-of-esdhc.c
which is Freescale-specific.  Why not just test SVR in there to
determine whether the quirk should be set?

-Scott

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Scott Wood]

On Mon, 2015-05-18 at 21:16 -0500, Lu Yangbo-B47093 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, May 19, 2015 4:43 AM
> > To: Lu Yangbo-B47093
> > Cc: Ulf Hansson; linux-mmc; Chris Ball
> > Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to
> > detect card
> > 
> > On Mon, 2015-05-18 at 04:55 -0500, Lu Yangbo-B47093 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> > > > Sent: Monday, May 18, 2015 5:50 PM
> > > > To: Lu Yangbo-B47093
> > > > Cc: linux-mmc; Chris Ball; Wood Scott-B07421
> > > > Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode
> > > > to detect card
> > > >
> > > > On 18 May 2015 at 11:30, Lu Y.B. <yangbo.lu@freescale.com> wrote:
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> > > > >> Sent: Monday, May 18, 2015 5:04 PM
> > > > >> To: Lu Yangbo-B47093
> > > > >> Cc: linux-mmc; Chris Ball; Wood Scott-B07421
> > > > >> Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt
> > > > >> mode to detect card
> > > > >>
> > > > >> On 15 May 2015 at 04:46, Yangbo Lu <yangbo.lu@freescale.com> wrote:
> > > > >> > Enable interrupt mode to detect card instead of polling mode
> > > > >> > for
> > > > >> > P1020/P4080/P5020/P5040/T1040 by removing the quirk
> > > > >> > SDHCI_QUIRK_BROKEN_CARD_DETECTION. This could improve data
> > > > >> > transferring performance and avoid the call trace caused by
> > > > >> > polling card status sometime.
> > > > >> >
> > > > >> > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
> > > > >> > ----
> > > > >> > Changes for v2:
> > > > >> >         - Aligned all "of_device_is_compatibles" in same column
> > > > >> > ---
> > > > >> >  drivers/mmc/host/sdhci-pltfm.c | 7 +++++++
> > > > >> >  1 file changed, 7 insertions(+)
> > > > >> >
> > > > >> > diff --git a/drivers/mmc/host/sdhci-pltfm.c
> > > > >> > b/drivers/mmc/host/sdhci-pltfm.c index c5b01d6..97128f3 100644
> > > > >> > --- a/drivers/mmc/host/sdhci-pltfm.c
> > > > >> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> > > > >> > @@ -102,6 +102,13 @@ void sdhci_get_of_property(struct
> > > > >> > platform_device
> > > > >> *pdev)
> > > > >> >                     of_device_is_compatible(np, "fsl,mpc8536-
> > esdhc"))
> > > > >> >                         host->quirks |=
> > > > >> > SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> > > > >> >
> > > > >> > +               if (of_device_is_compatible(np,
> > > > >> > + "fsl,p5040-esdhc")
> > > > ||
> > > > >> > +                   of_device_is_compatible(np,
> > > > >> > + "fsl,p5020-esdhc")
> > > > ||
> > > > >> > +                   of_device_is_compatible(np,
> > > > >> > + "fsl,p4080-esdhc")
> > > > ||
> > > > >> > +                   of_device_is_compatible(np,
> > > > >> > + "fsl,p1020-esdhc")
> > > > ||
> > > > >> > +                   of_device_is_compatible(np, "fsl,t1040-
> > esdhc"))
> > > > >> > +                       host->quirks &=
> > > > >> > + ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> > > > >>
> > > > >> This looks strange to me.
> > > > >>
> > > > >> Why not just change the DT files for the relevant platforms to
> > > > >> not enable "broken-cd"?
> > > > >>
> > > > > Thanks Uffe.
> > > > > Because most platforms using sdhci-of-esdhc have broken CD, we
> > > > > hope
> > > > this quirk is selected in default.
> > > > > Only several platforms could use CD to detect and we clear this
> > > > > bit for
> > > > them.
> > > >
> > > > Hmm.
> > > >
> > > > Those platforms could still update their DT files and add
> > > > "broken-cd", since that would be the proper description of the HW.
> > > > Once that's done, it would enable you to remove the
> > > > SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, right?
> > > >
> > > Yes, and if remove SDHCI_QUIRK_BROKEN_CARD_DETECTION as default,
> > 'borken-cd' would be needed to be added for most platforms using esdhc.
> > 
> > I was OK with changing the device tree if it just meant that things that
> > previously didn't work now work.  I'm not OK with requiring the device
> > trees to change in order for things that already work to stay working.
> > 
> > In general it is not reasonable to expect device trees to enumerate
> > hardware bugs since the bugs (or the need to work aronud them) are often
> > discovered after the device tree has been created.
> > 
> > Yangbo, when I asked why you couldn't use SVR you said that it was a
> > common SDHC file.  But, the file that sets
> > SDHCI_QUIRK_BROKEN_CARD_DETECTION in the first place is sdhci-of-esdhc.c
> > which is Freescale-specific.  Why not just test SVR in there to determine
> > whether the quirk should be set?
> Thanks Scott.
> The SVR testing could be only used for PPC. It doesn’t support ARM that we will use.
> I think using the dts should be better way.

None of the chips that this patch looks for are ARM chips.  Are you're
saying our ARM chips don't have any form of identification?  Even if
that's the case, there's nothing wrong with looking at the device tree;
it's changing the device tree that I'm objecting to if there's an
alternative.

In any case, I don't know why these checks are being added to common
code rather than sdhci-of-esdhc.c.

-Scott

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Scott Wood]

On Mon, 2015-05-18 at 21:49 -0500, Lu Yangbo-B47093 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, May 19, 2015 10:28 AM
> > To: Lu Yangbo-B47093
> > Cc: Ulf Hansson; linux-mmc; Chris Ball
> > Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to
> > detect card
> > 
> > On Mon, 2015-05-18 at 21:16 -0500, Lu Yangbo-B47093 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Tuesday, May 19, 2015 4:43 AM
> > > > To: Lu Yangbo-B47093
> > > > Cc: Ulf Hansson; linux-mmc; Chris Ball
> > > > Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode
> > > > to detect card
> > > >
> > > > On Mon, 2015-05-18 at 04:55 -0500, Lu Yangbo-B47093 wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> > > > > > Sent: Monday, May 18, 2015 5:50 PM
> > > > > > To: Lu Yangbo-B47093
> > > > > > Cc: linux-mmc; Chris Ball; Wood Scott-B07421
> > > > > > Subject: Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt
> > > > > > mode to detect card
> > > > > >
> > > > > > Those platforms could still update their DT files and add
> > > > > > "broken-cd", since that would be the proper description of the HW.
> > > > > > Once that's done, it would enable you to remove the
> > > > > > SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, right?
> > > > > >
> > > > > Yes, and if remove SDHCI_QUIRK_BROKEN_CARD_DETECTION as default,
> > > > 'borken-cd' would be needed to be added for most platforms using
> > esdhc.
> > > >
> > > > I was OK with changing the device tree if it just meant that things
> > > > that previously didn't work now work.  I'm not OK with requiring the
> > > > device trees to change in order for things that already work to stay
> > working.
> > > >
> > > > In general it is not reasonable to expect device trees to enumerate
> > > > hardware bugs since the bugs (or the need to work aronud them) are
> > > > often discovered after the device tree has been created.
> > > >
> > > > Yangbo, when I asked why you couldn't use SVR you said that it was a
> > > > common SDHC file.  But, the file that sets
> > > > SDHCI_QUIRK_BROKEN_CARD_DETECTION in the first place is
> > > > sdhci-of-esdhc.c which is Freescale-specific.  Why not just test SVR
> > > > in there to determine whether the quirk should be set?
> > > Thanks Scott.
> > > The SVR testing could be only used for PPC. It doesn’t support ARM that
> > we will use.
> > > I think using the dts should be better way.
> > 
> > None of the chips that this patch looks for are ARM chips.  Are you're
> > saying our ARM chips don't have any form of identification?  Even if
> > that's the case, there's nothing wrong with looking at the device tree;
> > it's changing the device tree that I'm objecting to if there's an
> > alternative.
> > 
> > In any case, I don't know why these checks are being added to common code
> > rather than sdhci-of-esdhc.c.
> > 
> Scott, I know what you mean now.
> What about moving these checks to sdhci-of-esdhc and still using dts?

The only thing I'm strongly opposed to is requiring chips that are
already working to add broken-cd to their device trees to continue
working.  I'd rather see SVR be used, as we do for errata in other
devices, but assuming the chips affected by this patch have never had
this feature work I'm not insisting on it.

> I know the SVR checks is from <asm/mpc85xx.h> and we will need to check
> ARM architecture like LS2085A later although it hasn't been exist on
> upstream now.  

Again, ARM can use the device tree (ESDHC isn't described in the
upstream LS2085A device tree yet, and the internal one already has
"fsl,ls2085a-esdhc").  And looking at the manual, LS2085A *does* have
SVR.  You'd need to read it from the DCFG block rather than a core
register, but it's there.

-Scott

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Ulf Hansson]

[...]

>> > Those platforms could still update their DT files and add "broken-cd",
>> > since that would be the proper description of the HW. Once that's done,
>> > it would enable you to remove the SDHCI_QUIRK_BROKEN_CARD_DETECTION as
>> > default, right?
>> >
>> Yes, and if remove SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, 'borken-cd' would be needed to be added for most platforms using esdhc.
>
> I was OK with changing the device tree if it just meant that things that
> previously didn't work now work.  I'm not OK with requiring the device
> trees to change in order for things that already work to stay working.

I get your point, but.. considering maintaince from mmc code point of
view, I don't like the suggested approach in $subject patch.

As this patch anyway requires updating DTBs, I would like to know the
involved platforms and at what level of deployment the DTBs are in. In
principle what I am asking is, how hard is it to update the DTBs?

BTW, is it only sdhci-of-esdhc.c we are discussing here?

>
> In general it is not reasonable to expect device trees to enumerate
> hardware bugs since the bugs (or the need to work aronud them) are often
> discovered after the device tree has been created.
>
> Yangbo, when I asked why you couldn't use SVR you said that it was a
> common SDHC file.  But, the file that sets
> SDHCI_QUIRK_BROKEN_CARD_DETECTION in the first place is sdhci-of-esdhc.c
> which is Freescale-specific.  Why not just test SVR in there to
> determine whether the quirk should be set?
>
> -Scott
>
>

Kind regards
Uffe

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Scott Wood]

On Tue, 2015-05-19 at 09:46 +0200, Ulf Hansson wrote:
> [...]
> 
> >> > Those platforms could still update their DT files and add "broken-cd",
> >> > since that would be the proper description of the HW. Once that's done,
> >> > it would enable you to remove the SDHCI_QUIRK_BROKEN_CARD_DETECTION as
> >> > default, right?
> >> >
> >> Yes, and if remove SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, 'borken-cd' would be needed to be added for most platforms using esdhc.
> >
> > I was OK with changing the device tree if it just meant that things that
> > previously didn't work now work.  I'm not OK with requiring the device
> > trees to change in order for things that already work to stay working.
> 
> I get your point, but.. considering maintaince from mmc code point of
> view, I don't like the suggested approach in $subject patch.
> 
> As this patch anyway requires updating DTBs,

Not in a way that causes things that used to work to stop working if
someone uses a new kernel with an old device tree.

> I would like to know the involved platforms and at what level of
> deployment the DTBs are in. In principle what I am asking is, how hard
> is it to update the DTBs?

The change you asked for would affect all platforms with esdhc that are
not on the list being excluded by this patch.  I counted 29 files in
arch/powerpc/boot/dts with "esdhc".  The oldest patch I see introducing
esdhc to a device tree is 5761bc5dae6 from 2008.  This is not new
stuff.

The patch adding the quirk to the esdhc files is 3bb2a9f6a7c08 from
2011, so the driver behavior is also not new.

> BTW, is it only sdhci-of-esdhc.c we are discussing here?

It's only sdhci-of-esdhc.c that would be affected by this patch (though
sdhci-esdhc-imx also sets SDHCI_QUIRK_BROKEN_CARD_DETECTION by default).
If the tests are moved into that file, would you be OK with it?

-Scott

Re: [PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Ulf Hansson]

On 21 May 2015 at 04:59, Scott Wood <scottwood@freescale.com> wrote:
> On Tue, 2015-05-19 at 09:46 +0200, Ulf Hansson wrote:
>> [...]
>>
>> >> > Those platforms could still update their DT files and add "broken-cd",
>> >> > since that would be the proper description of the HW. Once that's done,
>> >> > it would enable you to remove the SDHCI_QUIRK_BROKEN_CARD_DETECTION as
>> >> > default, right?
>> >> >
>> >> Yes, and if remove SDHCI_QUIRK_BROKEN_CARD_DETECTION as default, 'borken-cd' would be needed to be added for most platforms using esdhc.
>> >
>> > I was OK with changing the device tree if it just meant that things that
>> > previously didn't work now work.  I'm not OK with requiring the device
>> > trees to change in order for things that already work to stay working.
>>
>> I get your point, but.. considering maintaince from mmc code point of
>> view, I don't like the suggested approach in $subject patch.
>>
>> As this patch anyway requires updating DTBs,
>
> Not in a way that causes things that used to work to stop working if
> someone uses a new kernel with an old device tree.
>
>> I would like to know the involved platforms and at what level of
>> deployment the DTBs are in. In principle what I am asking is, how hard
>> is it to update the DTBs?
>
> The change you asked for would affect all platforms with esdhc that are
> not on the list being excluded by this patch.  I counted 29 files in
> arch/powerpc/boot/dts with "esdhc".  The oldest patch I see introducing
> esdhc to a device tree is 5761bc5dae6 from 2008.  This is not new
> stuff.
>
> The patch adding the quirk to the esdhc files is 3bb2a9f6a7c08 from
> 2011, so the driver behavior is also not new.
>
>> BTW, is it only sdhci-of-esdhc.c we are discussing here?
>
> It's only sdhci-of-esdhc.c that would be affected by this patch (though
> sdhci-esdhc-imx also sets SDHCI_QUIRK_BROKEN_CARD_DETECTION by default).
> If the tests are moved into that file, would you be OK with it?

Considering your reasoning, yes!

Kind regards
Uffe

[PATCH v2, 2/2] mmc: sdhci-pltfm: enable interrupt mode to detect card

[Yangbo Lu]

Enable interrupt mode to detect card instead of polling mode
for P1020/P4080/P5020/P5040/T1040 by removing the quirk
SDHCI_QUIRK_BROKEN_CARD_DETECTION. This could improve data
transferring performance and avoid the call trace caused by
polling card status sometime.

Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
----
Changes for v2:
	- Aligned all "of_device_is_compatibles" in same column
---
 drivers/mmc/host/sdhci-pltfm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index c5b01d6..97128f3 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -102,6 +102,13 @@ void sdhci_get_of_property(struct platform_device *pdev)
 		    of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
 			host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+		if (of_device_is_compatible(np, "fsl,p5040-esdhc") ||
+		    of_device_is_compatible(np, "fsl,p5020-esdhc") ||
+		    of_device_is_compatible(np, "fsl,p4080-esdhc") ||
+		    of_device_is_compatible(np, "fsl,p1020-esdhc") ||
+		    of_device_is_compatible(np, "fsl,t1040-esdhc"))
+			host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
 		clk = of_get_property(np, "clock-frequency", &size);
 		if (clk && size == sizeof(*clk) && *clk)
 			pltfm_host->clock = be32_to_cpup(clk);
-- 
2.1.0.27.g96db324