On 15-11-09 02:45 AM, Ulf Hansson wrote:
On 7 November 2015 at 00:55, Scott Branden <sbran...@broadcom.com> wrote:
Hi Ulf,

On 15-11-06 12:14 AM, Ulf Hansson wrote:

On 5 November 2015 at 23:39, Al Cooper <alcoop...@gmail.com> wrote:

Add quirk to disable SDR50 mode for controllers/boards that have
problems with this mode.


No thanks! No more quirks please!


I'm fine with not having this quirk added (I don't need this one but use
multiple of the other quirks in the driver)  But, what if I also needed it
in my driver?  When do we determine when a quirk should be added to sdhci.c
or not.  What about existing quirks - should the current ones be moved to
multiple existing drivers?

The sdhci driver should turn into a library providing generic helper
functions. Each host can then pick which functions to use and also
deal with its own "quirks", instead of managing those in generic code.

I guess we might end up getting a bit more code in total, but on the
other hand the code would be better optimized and also maintainable.


OK, if I need to add any new quirks I will look into this a bit more and see if it can be done. I don't have a need to do this right now so if anyone else wants to have a look feel free to.


Kind regards
Uffe


Signed-off-by: Al Cooper <alcoop...@gmail.com>
---
   drivers/mmc/host/sdhci.c | 3 +++
   drivers/mmc/host/sdhci.h | 2 ++
   2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b48565e..71067c7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3176,6 +3176,9 @@ int sdhci_add_host(struct sdhci_host *host)
          } else if (caps[1] & SDHCI_SUPPORT_SDR50)
                  mmc->caps |= MMC_CAP_UHS_SDR50;

+       if (host->quirks2 & SDHCI_QUIRK2_BROKEN_SDR50)
+               mmc->caps &= ~MMC_CAP_UHS_SDR50;
+

Perhaps a lot of these quirks can be solved by having a generic mechanism to
override any of the values in the caps registers rather than adding all
these quirks?

Sure, it makes sense if it can decreases the number of quirks!

I think it can in some cases. In fact - the hardware designers do not even set the correct settings in the caps register on some of the SoCs. The caps register does not appear to be used in the hardware other than for "info" purposes to read by the driver - and when the info is wrong this may lead to many of the quirks that have been added by others over the years.

          if (host->quirks2 & SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 &&
              (caps[1] & SDHCI_SUPPORT_HS400))
                  mmc->caps2 |= MMC_CAP2_HS400;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 9d4aa31..0941c94 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -412,6 +412,8 @@ struct sdhci_host {
   #define SDHCI_QUIRK2_ACMD23_BROKEN                     (1<<14)
   /* Broken Clock divider zero in controller */
   #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN             (1<<15)
+/* Controller does not support SDR50 */
+#define SDHCI_QUIRK2_BROKEN_SDR50                      (1<<16)
   /*
    * When internal clock is disabled, a delay is needed before modifying
the
    * SD clock frequency or enabling back the internal clock.
--
1.9.0.138.g2de3478


Regards,
  Scott

Kind regards
Uffe

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

Reply via email to