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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2015-05-18 at 04:55 -0500, Lu Yangbo-B47093 wrote:
> 
> 
> > -----Original Message-----
> > From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx]
> > 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@xxxxxxxxxxxxx> wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx]
> > >> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > >> > ----
> > >> > 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


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux