On 7 November 2015 at 00:55, Scott Branden <sbranden@xxxxxxxxxxxx> wrote:
> Hi Ulf,
>
> On 15-11-06 12:14 AM, Ulf Hansson wrote:
>>
>> On 5 November 2015 at 23:39, Al Cooper <alcooperx@xxxxxxxxx> 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.
>>
>> Kind regards
>> Uffe
>>
>>>
>>> Signed-off-by: Al Cooper <alcooperx@xxxxxxxxx>
>>> ---
>>> 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!
>
>>> 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
--
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
