Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit

[Aswath Govindraju <a-govindraju@ti.com>]

Hi Uffe,

On 21/04/22 17:41, Ulf Hansson wrote:
> On Mon, 18 Apr 2022 at 12:21, Aswath Govindraju <a-govindraju@ti.com> wrote:
>>
>> From: Vignesh Raghavendra <vigneshr@ti.com>
>>
>> The ARASAN MMC controller on Keystone 3 class of devices need the SDCD
>> line to be connected for proper functioning. Similar to the issue pointed
>> out in sdhci-of-arasan.c driver, commit 3794c542641f ("mmc:
>> sdhci-of-arasan: Set controller to test mode when no CD bit").
>>
>> In cases where this can't be connected, add a quirk to force the
>> controller into test mode and set the TESTCD bit. Use the flag
>> "ti,fails-without-test-cd", to implement this above quirk when required.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> ---
>>  drivers/mmc/host/sdhci_am654.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
>> index e54fe24d47e7..c36b969ed1b6 100644
>> --- a/drivers/mmc/host/sdhci_am654.c
>> +++ b/drivers/mmc/host/sdhci_am654.c
>> @@ -147,6 +147,9 @@ struct sdhci_am654_data {
>>         int drv_strength;
>>         int strb_sel;
>>         u32 flags;
>> +       u32 quirks;
>> +
>> +#define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
>>  };
>>
>>  struct sdhci_am654_driver_data {
>> @@ -369,6 +372,21 @@ static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)
>>         }
>>  }
>>
>> +void sdhci_am654_reset(struct sdhci_host *host, u8 mask)
> 
> This should be a static function.
> 

Fixed this in the respin.

>> +{
>> +       u8 ctrl;
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
>> +
>> +       sdhci_reset(host, mask);
>> +
>> +       if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_FORCE_CDTEST) {
>> +               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>> +               ctrl |= SDHCI_CTRL_CDTEST_INS | SDHCI_CTRL_CDTEST_EN;
>> +               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>> +       }
>> +}
>> +
>>  static int sdhci_am654_execute_tuning(struct mmc_host *mmc, u32 opcode)
>>  {
>>         struct sdhci_host *host = mmc_priv(mmc);
>> @@ -500,7 +518,7 @@ static struct sdhci_ops sdhci_j721e_4bit_ops = {
>>         .set_clock = sdhci_j721e_4bit_set_clock,
>>         .write_b = sdhci_am654_write_b,
>>         .irq = sdhci_am654_cqhci_irq,
>> -       .reset = sdhci_reset,
>> +       .reset = sdhci_am654_reset,
>>  };
>>
>>  static const struct sdhci_pltfm_data sdhci_j721e_4bit_pdata = {
>> @@ -719,6 +737,9 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
>>         device_property_read_u32(dev, "ti,clkbuf-sel",
>>                                  &sdhci_am654->clkbuf_sel);
>>
>> +       if (device_property_read_bool(dev, "ti,fails-without-test-cd"))
>> +               sdhci_am654->quirks |= SDHCI_AM654_QUIRK_FORCE_CDTEST;
>> +
>>         sdhci_get_of_property(pdev);
>>
>>         return 0;
>> --
>> 2.17.1
>>
> 
> Other than the minor thing above, this looks good to me.
> 

Thank you for the review.

Link to the v2 of this patch series,

https://patchwork.kernel.org/project/linux-mmc/list/?series=635179

-- 
Thanks,
Aswath

> Kind regards
> Uffe