LinuxLists.cc - [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit

2022-04-18 11:29:17

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

From: Vignesh Raghavendra <[email protected]>

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 <[email protected]>
Signed-off-by: Aswath Govindraju <[email protected]>
---
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)
+{
+ 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

2022-04-18 12:18:31

by kernel test robot

[permalink] [raw]

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

2022-04-18 19:37:58

by kernel test robot

[permalink] [raw]

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

Hi Aswath,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master ulf-hansson-mmc-mirror/next v5.18-rc3 next-20220414]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Aswath-Govindraju/MMC-Add-quirk-to-set-the-TESTCD-bit/20220418-182325
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: riscv-randconfig-r016-20220418 (https://download.01.org/0day-ci/archive/20220418/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 429cbac0390654f90bba18a41799464adf31a5ec)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/a7d917691f55e240b1ab0abf36b0b39d1194a323
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Aswath-Govindraju/MMC-Add-quirk-to-set-the-TESTCD-bit/20220418-182325
git checkout a7d917691f55e240b1ab0abf36b0b39d1194a323
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/mmc/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from drivers/mmc/host/sdhci_am654.c:9:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/mmc/host/sdhci_am654.c:9:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/mmc/host/sdhci_am654.c:9:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> drivers/mmc/host/sdhci_am654.c:375:6: warning: no previous prototype for function 'sdhci_am654_reset' [-Wmissing-prototypes]
void sdhci_am654_reset(struct sdhci_host *host, u8 mask)
^
drivers/mmc/host/sdhci_am654.c:375:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void sdhci_am654_reset(struct sdhci_host *host, u8 mask)
^
static
8 warnings generated.

vim +/sdhci_am654_reset +375 drivers/mmc/host/sdhci_am654.c

374
> 375 void sdhci_am654_reset(struct sdhci_host *host, u8 mask)
376 {
377 u8 ctrl;
378 struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
379 struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
380
381 sdhci_reset(host, mask);
382
383 if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_FORCE_CDTEST) {
384 ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
385 ctrl |= SDHCI_CTRL_CDTEST_INS | SDHCI_CTRL_CDTEST_EN;
386 sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
387 }
388 }
389

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-04-22 19:30:04

by Ulf Hansson

[permalink] [raw]

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

On Mon, 18 Apr 2022 at 12:21, Aswath Govindraju <[email protected]> wrote:
>
> From: Vignesh Raghavendra <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Aswath Govindraju <[email protected]>
> ---
> 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.

> +{
> + 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.

Kind regards
Uffe

2022-04-25 09:51:14

by Aswath Govindraju

[permalink] [raw]

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

Hi Uffe,

On 21/04/22 17:41, Ulf Hansson wrote:
> On Mon, 18 Apr 2022 at 12:21, Aswath Govindraju <[email protected]> wrote:
>>
>> From: Vignesh Raghavendra <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Aswath Govindraju <[email protected]>
>> ---
>> 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