All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] MMC: Add quirk to set the TESTCD bit
@ 2022-04-18 10:20 Aswath Govindraju
  2022-04-18 10:20 ` [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to " Aswath Govindraju
  2022-04-18 10:20 ` [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set " Aswath Govindraju
  0 siblings, 2 replies; 9+ messages in thread
From: Aswath Govindraju @ 2022-04-18 10:20 UTC (permalink / raw)
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Aswath Govindraju,
	Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Adrian Hunter,
	linux-mmc, devicetree, linux-kernel

The following series of patches add support for setting TESTCD bit
when SDCD line is connected to the controller, in the sdhci_am654.c
driver.

Aswath Govindraju (1):
  dt-bindings: mmc: sdhci-am654: Add flag to force setting to TESTCD bit

Vignesh Raghavendra (1):
  drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit

 .../devicetree/bindings/mmc/sdhci-am654.yaml  |  7 ++++++
 drivers/mmc/host/sdhci_am654.c                | 23 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to TESTCD bit
  2022-04-18 10:20 [PATCH 0/2] MMC: Add quirk to set the TESTCD bit Aswath Govindraju
@ 2022-04-18 10:20 ` Aswath Govindraju
  2022-04-21 12:10   ` Ulf Hansson
  2022-04-18 10:20 ` [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set " Aswath Govindraju
  1 sibling, 1 reply; 9+ messages in thread
From: Aswath Govindraju @ 2022-04-18 10:20 UTC (permalink / raw)
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Aswath Govindraju,
	Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Adrian Hunter,
	linux-mmc, devicetree, linux-kernel

The ARASAN MMC controller on Keystone 3 class of devices needs 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 SDCD line is not connected, driver support has been added to
force the controller into test mode and set the TESTCD bit. In order to
implement this quirk the driver uses "ti,fails-without-test-cd" flag from
the device tree node. Therefore, update the bindings to document the above.

Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
 Documentation/devicetree/bindings/mmc/sdhci-am654.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
index 0566493c4def..0ab07759b472 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
@@ -186,6 +186,13 @@ properties:
     description: Clock Delay Buffer Select
     $ref: "/schemas/types.yaml#/definitions/uint32"
 
+  ti,fails-without-test-cd:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      When present, indicates that the CD line is not connected
+      and the controller is required to be forced into Test mode
+      to set the TESTCD bit.
+
 required:
   - compatible
   - reg
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit
  2022-04-18 10:20 [PATCH 0/2] MMC: Add quirk to set the TESTCD bit Aswath Govindraju
  2022-04-18 10:20 ` [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to " Aswath Govindraju
@ 2022-04-18 10:20 ` Aswath Govindraju
  2022-04-18 11:55   ` kernel test robot
                     ` (2 more replies)
  1 sibling, 3 replies; 9+ messages in thread
From: Aswath Govindraju @ 2022-04-18 10:20 UTC (permalink / raw)
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Aswath Govindraju,
	Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Adrian Hunter,
	linux-mmc, devicetree, linux-kernel

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit
  2022-04-18 10:20 ` [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set " Aswath Govindraju
@ 2022-04-18 11:55   ` kernel test robot
  2022-04-18 14:49   ` kernel test robot
  2022-04-21 12:11   ` Ulf Hansson
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-04-18 11:55 UTC (permalink / raw)
  To: Aswath Govindraju
  Cc: kbuild-all, Vignesh Raghavendra, Kishon Vijay Abraham I,
	Aswath Govindraju, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Adrian Hunter, linux-mmc, devicetree, linux-kernel

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: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220418/202204181920.iopT8zQA-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
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
        # 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=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/mmc/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/mmc/host/sdhci_am654.c:375:6: warning: no previous prototype for 'sdhci_am654_reset' [-Wmissing-prototypes]
     375 | void sdhci_am654_reset(struct sdhci_host *host, u8 mask)
         |      ^~~~~~~~~~~~~~~~~


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit
  2022-04-18 10:20 ` [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set " Aswath Govindraju
  2022-04-18 11:55   ` kernel test robot
@ 2022-04-18 14:49   ` kernel test robot
  2022-04-21 12:11   ` Ulf Hansson
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-04-18 14:49 UTC (permalink / raw)
  To: Aswath Govindraju
  Cc: llvm, kbuild-all, Vignesh Raghavendra, Kishon Vijay Abraham I,
	Aswath Govindraju, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Adrian Hunter, linux-mmc, devicetree, linux-kernel

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/202204182202.yPPV6YZI-lkp@intel.com/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 <lkp@intel.com>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to TESTCD bit
  2022-04-18 10:20 ` [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to " Aswath Govindraju
@ 2022-04-21 12:10   ` Ulf Hansson
  2022-04-25  6:19     ` Aswath Govindraju
  0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2022-04-21 12:10 UTC (permalink / raw)
  To: Aswath Govindraju
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Adrian Hunter, linux-mmc, devicetree,
	linux-kernel

On Mon, 18 Apr 2022 at 12:21, Aswath Govindraju <a-govindraju@ti.com> wrote:
>
> The ARASAN MMC controller on Keystone 3 class of devices needs 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 SDCD line is not connected, driver support has been added to
> force the controller into test mode and set the TESTCD bit. In order to
> implement this quirk the driver uses "ti,fails-without-test-cd" flag from
> the device tree node. Therefore, update the bindings to document the above.

Would you mind rephrasing this a bit. DT bindings is about describing
the HW, not about what the software should do.

Otherwise, this looks good to me.

Kind regards
Uffe

>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-am654.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
> index 0566493c4def..0ab07759b472 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
> @@ -186,6 +186,13 @@ properties:
>      description: Clock Delay Buffer Select
>      $ref: "/schemas/types.yaml#/definitions/uint32"
>
> +  ti,fails-without-test-cd:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      When present, indicates that the CD line is not connected
> +      and the controller is required to be forced into Test mode
> +      to set the TESTCD bit.
> +
>  required:
>    - compatible
>    - reg
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit
  2022-04-18 10:20 ` [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set " Aswath Govindraju
  2022-04-18 11:55   ` kernel test robot
  2022-04-18 14:49   ` kernel test robot
@ 2022-04-21 12:11   ` Ulf Hansson
  2022-04-25  6:44     ` Aswath Govindraju
  2 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2022-04-21 12:11 UTC (permalink / raw)
  To: Aswath Govindraju
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Adrian Hunter, linux-mmc, devicetree,
	linux-kernel

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to TESTCD bit
  2022-04-21 12:10   ` Ulf Hansson
@ 2022-04-25  6:19     ` Aswath Govindraju
  0 siblings, 0 replies; 9+ messages in thread
From: Aswath Govindraju @ 2022-04-25  6:19 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Adrian Hunter, linux-mmc, devicetree,
	linux-kernel

Hi Uffe,

On 21/04/22 17:40, Ulf Hansson wrote:
> On Mon, 18 Apr 2022 at 12:21, Aswath Govindraju <a-govindraju@ti.com> wrote:
>>
>> The ARASAN MMC controller on Keystone 3 class of devices needs 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 SDCD line is not connected, driver support has been added to
>> force the controller into test mode and set the TESTCD bit. In order to
>> implement this quirk the driver uses "ti,fails-without-test-cd" flag from
>> the device tree node. Therefore, update the bindings to document the above.
> 
> Would you mind rephrasing this a bit. DT bindings is about describing
> the HW, not about what the software should do.
> 

Sure, will rephrase it in the respin to remove the aspects that indicate
the sw support added.

> Otherwise, this looks good to me.
> 

Thank you for the review.

Regards,
Aswath

> Kind regards
> Uffe
> 
>>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> ---
>>  Documentation/devicetree/bindings/mmc/sdhci-am654.yaml | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
>> index 0566493c4def..0ab07759b472 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
>> @@ -186,6 +186,13 @@ properties:
>>      description: Clock Delay Buffer Select
>>      $ref: "/schemas/types.yaml#/definitions/uint32"
>>
>> +  ti,fails-without-test-cd:
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +    description:
>> +      When present, indicates that the CD line is not connected
>> +      and the controller is required to be forced into Test mode
>> +      to set the TESTCD bit.
>> +
>>  required:
>>    - compatible
>>    - reg
>> --
>> 2.17.1
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit
  2022-04-21 12:11   ` Ulf Hansson
@ 2022-04-25  6:44     ` Aswath Govindraju
  0 siblings, 0 replies; 9+ messages in thread
From: Aswath Govindraju @ 2022-04-25  6:44 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Vignesh Raghavendra, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Adrian Hunter, linux-mmc, devicetree,
	linux-kernel

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




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-04-25  6:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 10:20 [PATCH 0/2] MMC: Add quirk to set the TESTCD bit Aswath Govindraju
2022-04-18 10:20 ` [PATCH 1/2] dt-bindings: mmc: sdhci-am654: Add flag to force setting to " Aswath Govindraju
2022-04-21 12:10   ` Ulf Hansson
2022-04-25  6:19     ` Aswath Govindraju
2022-04-18 10:20 ` [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set " Aswath Govindraju
2022-04-18 11:55   ` kernel test robot
2022-04-18 14:49   ` kernel test robot
2022-04-21 12:11   ` Ulf Hansson
2022-04-25  6:44     ` Aswath Govindraju

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.