[PATCH v2 0/4] cd/wp code clean up for i.MX USDHC

[PATCH v2 0/4] cd/wp code clean up for i.MX USDHC

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

Now i.MX USDHC driver convert to a DT-only platform driver, the legency
code for non-DT is reduncant, like the cd_types and wp_types. So clean
it up here.

V2:
-PATCH 3, fix the dt-binding-check error issue.
-no changes for other PATCH.

Haibo Chen (4):
  mmc: sdhci-esdhc-imx: remove unused enum cd_types
  mmc: sdhci-esdhc-imx: remove redundant write protect code
  dt-bindings: mmc: fsl-imx-esdhc: remove property "fsl,wp-controller"
  arm: dts: imx: remove "fsl,wp-controller" since it is dropped

 .../bindings/mmc/fsl-imx-esdhc.yaml           |  6 --
 arch/arm/boot/dts/imx53-sk-imx53.dts          |  1 -
 arch/arm/boot/dts/imx53-tx53.dtsi             |  2 -
 arch/arm/boot/dts/imx6dl-tx6s-8035.dts        |  1 -
 arch/arm/boot/dts/imx6dl-tx6u-8033.dts        |  1 -
 arch/arm/boot/dts/imx6q-bosch-acc.dts         |  2 -
 arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts  |  1 -
 arch/arm/boot/dts/imx6q-tx6q-1020.dts         |  1 -
 arch/arm/boot/dts/imx6q-tx6q-1036.dts         |  1 -
 arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi      |  1 -
 arch/arm/boot/dts/imx6qdl-emcon.dtsi          |  2 -
 arch/arm/boot/dts/imx6qdl-ts4900.dtsi         |  1 -
 arch/arm/boot/dts/imx6qdl-ts7970.dtsi         |  1 -
 arch/arm/boot/dts/imx6qdl-tx6.dtsi            |  2 -
 arch/arm/boot/dts/imx6qp-tx6qp-8037.dts       |  1 -
 arch/arm/boot/dts/imx6qp-tx6qp-8137.dts       |  1 -
 arch/arm/boot/dts/imx6ul-tx6ul-0011.dts       |  1 -
 arch/arm/boot/dts/imx6ul-tx6ul.dtsi           |  1 -
 arch/arm/boot/dts/imxrt1050.dtsi              |  1 -
 drivers/mmc/host/sdhci-esdhc-imx.c            | 56 +------------------
 20 files changed, 1 insertion(+), 83 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] mmc: sdhci-esdhc-imx: remove unused enum cd_types

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

enum cd_types is no more used, so remove it here.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index eebf94604a7f..c7db742f729c 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -207,25 +207,16 @@ enum wp_types {
 	ESDHC_WP_GPIO,		/* external gpio pin for WP */
 };
 
-enum cd_types {
-	ESDHC_CD_NONE,		/* no CD, neither controller nor gpio */
-	ESDHC_CD_CONTROLLER,	/* mmc controller internal CD */
-	ESDHC_CD_GPIO,		/* external gpio pin for CD */
-	ESDHC_CD_PERMANENT,	/* no CD, card permanently wired to host */
-};
-
 /*
  * struct esdhc_platform_data - platform data for esdhc on i.MX
  *
  * ESDHC_WP(CD)_CONTROLLER type is not available on i.MX25/35.
  *
  * @wp_type:	type of write_protect method (see wp_types enum above)
- * @cd_type:	type of card_detect method (see cd_types enum above)
  */
 
 struct esdhc_platform_data {
 	enum wp_types wp_type;
-	enum cd_types cd_type;
 	int max_bus_width;
 	unsigned int delay_line;
 	unsigned int tuning_step;       /* The delay cell steps in tuning procedure */
-- 
2.34.1

Re: [PATCH v2 1/4] mmc: sdhci-esdhc-imx: remove unused enum cd_types

[Adrian Hunter]

On 6/05/23 10:40, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> enum cd_types is no more used, so remove it here.

FWIW it doesn't look like max_bus_width is being used either?

> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index eebf94604a7f..c7db742f729c 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -207,25 +207,16 @@ enum wp_types {
>  	ESDHC_WP_GPIO,		/* external gpio pin for WP */
>  };
>  
> -enum cd_types {
> -	ESDHC_CD_NONE,		/* no CD, neither controller nor gpio */
> -	ESDHC_CD_CONTROLLER,	/* mmc controller internal CD */
> -	ESDHC_CD_GPIO,		/* external gpio pin for CD */
> -	ESDHC_CD_PERMANENT,	/* no CD, card permanently wired to host */
> -};
> -
>  /*
>   * struct esdhc_platform_data - platform data for esdhc on i.MX
>   *
>   * ESDHC_WP(CD)_CONTROLLER type is not available on i.MX25/35.
>   *
>   * @wp_type:	type of write_protect method (see wp_types enum above)
> - * @cd_type:	type of card_detect method (see cd_types enum above)
>   */
>  
>  struct esdhc_platform_data {
>  	enum wp_types wp_type;
> -	enum cd_types cd_type;
>  	int max_bus_width;
>  	unsigned int delay_line;
>  	unsigned int tuning_step;       /* The delay cell steps in tuning procedure */

[PATCH v2 2/4] mmc: sdhci-esdhc-imx: remove redundant write protect code

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The logic of the esdhc_pltfm_get_ro() is just the same with common
code sdhci_check_ro(). So remove this redundant code, depends on
mmc_of_parse() and sdhci_check_ro() to cover the write protect
logic.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 47 +-----------------------------
 1 file changed, 1 insertion(+), 46 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index c7db742f729c..54531aab70f0 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -201,22 +201,8 @@
 /* ERR004536 is not applicable for the IP  */
 #define ESDHC_FLAG_SKIP_ERR004536	BIT(17)
 
-enum wp_types {
-	ESDHC_WP_NONE,		/* no WP, neither controller nor gpio */
-	ESDHC_WP_CONTROLLER,	/* mmc controller internal WP */
-	ESDHC_WP_GPIO,		/* external gpio pin for WP */
-};
-
-/*
- * struct esdhc_platform_data - platform data for esdhc on i.MX
- *
- * ESDHC_WP(CD)_CONTROLLER type is not available on i.MX25/35.
- *
- * @wp_type:	type of write_protect method (see wp_types enum above)
- */
-
+/* struct esdhc_platform_data - platform data for esdhc on i.MX */
 struct esdhc_platform_data {
-	enum wp_types wp_type;
 	int max_bus_width;
 	unsigned int delay_line;
 	unsigned int tuning_step;       /* The delay cell steps in tuning procedure */
@@ -994,25 +980,6 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 
 }
 
-static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
-
-	switch (boarddata->wp_type) {
-	case ESDHC_WP_GPIO:
-		return mmc_gpio_get_ro(host->mmc);
-	case ESDHC_WP_CONTROLLER:
-		return !(readl(host->ioaddr + SDHCI_PRESENT_STATE) &
-			       SDHCI_WRITE_PROTECT);
-	case ESDHC_WP_NONE:
-		break;
-	}
-
-	return -ENOSYS;
-}
-
 static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width)
 {
 	u32 ctrl;
@@ -1380,7 +1347,6 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.get_max_clock = esdhc_pltfm_get_max_clock,
 	.get_min_clock = esdhc_pltfm_get_min_clock,
 	.get_max_timeout_count = esdhc_get_max_timeout_count,
-	.get_ro = esdhc_pltfm_get_ro,
 	.set_timeout = esdhc_set_timeout,
 	.set_bus_width = esdhc_pltfm_set_bus_width,
 	.set_uhs_signaling = esdhc_set_uhs_signaling,
@@ -1588,17 +1554,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 	int ret;
 
-	if (of_property_read_bool(np, "fsl,wp-controller"))
-		boarddata->wp_type = ESDHC_WP_CONTROLLER;
-
-	/*
-	 * If we have this property, then activate WP check.
-	 * Retrieveing and requesting the actual WP GPIO will happen
-	 * in the call to mmc_of_parse().
-	 */
-	if (of_property_read_bool(np, "wp-gpios"))
-		boarddata->wp_type = ESDHC_WP_GPIO;
-
 	of_property_read_u32(np, "fsl,tuning-step", &boarddata->tuning_step);
 	of_property_read_u32(np, "fsl,tuning-start-tap",
 			     &boarddata->tuning_start_tap);
-- 
2.34.1

Re: [PATCH v2 2/4] mmc: sdhci-esdhc-imx: remove redundant write protect code

[Adrian Hunter]

On 6/05/23 10:40, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The logic of the esdhc_pltfm_get_ro() is just the same with common
> code sdhci_check_ro(). So remove this redundant code, depends on
> mmc_of_parse() and sdhci_check_ro() to cover the write protect
> logic.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 47 +-----------------------------
>  1 file changed, 1 insertion(+), 46 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c7db742f729c..54531aab70f0 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -201,22 +201,8 @@
>  /* ERR004536 is not applicable for the IP  */
>  #define ESDHC_FLAG_SKIP_ERR004536	BIT(17)
>  
> -enum wp_types {
> -	ESDHC_WP_NONE,		/* no WP, neither controller nor gpio */
> -	ESDHC_WP_CONTROLLER,	/* mmc controller internal WP */
> -	ESDHC_WP_GPIO,		/* external gpio pin for WP */
> -};
> -
> -/*
> - * struct esdhc_platform_data - platform data for esdhc on i.MX
> - *
> - * ESDHC_WP(CD)_CONTROLLER type is not available on i.MX25/35.
> - *
> - * @wp_type:	type of write_protect method (see wp_types enum above)
> - */
> -
> +/* struct esdhc_platform_data - platform data for esdhc on i.MX */
>  struct esdhc_platform_data {
> -	enum wp_types wp_type;
>  	int max_bus_width;
>  	unsigned int delay_line;
>  	unsigned int tuning_step;       /* The delay cell steps in tuning procedure */
> @@ -994,25 +980,6 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
>  
>  }
>  
> -static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
> -{
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> -	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
> -
> -	switch (boarddata->wp_type) {
> -	case ESDHC_WP_GPIO:
> -		return mmc_gpio_get_ro(host->mmc);
> -	case ESDHC_WP_CONTROLLER:
> -		return !(readl(host->ioaddr + SDHCI_PRESENT_STATE) &
> -			       SDHCI_WRITE_PROTECT);
> -	case ESDHC_WP_NONE:
> -		break;
> -	}
> -
> -	return -ENOSYS;
> -}
> -
>  static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width)
>  {
>  	u32 ctrl;
> @@ -1380,7 +1347,6 @@ static struct sdhci_ops sdhci_esdhc_ops = {
>  	.get_max_clock = esdhc_pltfm_get_max_clock,
>  	.get_min_clock = esdhc_pltfm_get_min_clock,
>  	.get_max_timeout_count = esdhc_get_max_timeout_count,
> -	.get_ro = esdhc_pltfm_get_ro,
>  	.set_timeout = esdhc_set_timeout,
>  	.set_bus_width = esdhc_pltfm_set_bus_width,
>  	.set_uhs_signaling = esdhc_set_uhs_signaling,
> @@ -1588,17 +1554,6 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
>  	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
>  	int ret;
>  
> -	if (of_property_read_bool(np, "fsl,wp-controller"))
> -		boarddata->wp_type = ESDHC_WP_CONTROLLER;

esdhc_pltfm_get_ro() differs from sdhci_check_ro() by defaulting
to -ENOSYS if there is neither "fsl,wp-controller" nor "wp-gpios",
which would result in write-protect off (not supported, e.g. microsd).
So more explanation is needed for why removing "fsl,wp-controller"
will give the same result.

> -
> -	/*
> -	 * If we have this property, then activate WP check.
> -	 * Retrieveing and requesting the actual WP GPIO will happen
> -	 * in the call to mmc_of_parse().
> -	 */
> -	if (of_property_read_bool(np, "wp-gpios"))
> -		boarddata->wp_type = ESDHC_WP_GPIO;
> -
>  	of_property_read_u32(np, "fsl,tuning-step", &boarddata->tuning_step);
>  	of_property_read_u32(np, "fsl,tuning-start-tap",
>  			     &boarddata->tuning_start_tap);

RE: [PATCH v2 2/4] mmc: sdhci-esdhc-imx: remove redundant write protect code

[Bough Chen]

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: 2023年5月19日 18:37
> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de
> Cc: dl-linux-imx <linux-imx@nxp.com>; kernel@pengutronix.de;
> festevam@gmail.com; cniedermaier@dh-electronics.com;
> devicetree@vger.kernel.org; kernel@dh-electronics.com
> Subject: Re: [PATCH v2 2/4] mmc: sdhci-esdhc-imx: remove redundant write
> protect code
> 
> On 6/05/23 10:40, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The logic of the esdhc_pltfm_get_ro() is just the same with common
> > code sdhci_check_ro(). So remove this redundant code, depends on
> > mmc_of_parse() and sdhci_check_ro() to cover the write protect logic.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 47
> > +-----------------------------
> >  1 file changed, 1 insertion(+), 46 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index c7db742f729c..54531aab70f0 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -201,22 +201,8 @@
> >  /* ERR004536 is not applicable for the IP  */
> >  #define ESDHC_FLAG_SKIP_ERR004536	BIT(17)
> >
> > -enum wp_types {
> > -	ESDHC_WP_NONE,		/* no WP, neither controller nor gpio */
> > -	ESDHC_WP_CONTROLLER,	/* mmc controller internal WP */
> > -	ESDHC_WP_GPIO,		/* external gpio pin for WP */
> > -};
> > -
> > -/*
> > - * struct esdhc_platform_data - platform data for esdhc on i.MX
> > - *
> > - * ESDHC_WP(CD)_CONTROLLER type is not available on i.MX25/35.
> > - *
> > - * @wp_type:	type of write_protect method (see wp_types enum above)
> > - */
> > -
> > +/* struct esdhc_platform_data - platform data for esdhc on i.MX */
> >  struct esdhc_platform_data {
> > -	enum wp_types wp_type;
> >  	int max_bus_width;
> >  	unsigned int delay_line;
> >  	unsigned int tuning_step;       /* The delay cell steps in tuning
> procedure */
> > @@ -994,25 +980,6 @@ static inline void esdhc_pltfm_set_clock(struct
> > sdhci_host *host,
> >
> >  }
> >
> > -static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host) -{
> > -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > -	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> > -	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
> > -
> > -	switch (boarddata->wp_type) {
> > -	case ESDHC_WP_GPIO:
> > -		return mmc_gpio_get_ro(host->mmc);
> > -	case ESDHC_WP_CONTROLLER:
> > -		return !(readl(host->ioaddr + SDHCI_PRESENT_STATE) &
> > -			       SDHCI_WRITE_PROTECT);
> > -	case ESDHC_WP_NONE:
> > -		break;
> > -	}
> > -
> > -	return -ENOSYS;
> > -}
> > -
> >  static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int
> > width)  {
> >  	u32 ctrl;
> > @@ -1380,7 +1347,6 @@ static struct sdhci_ops sdhci_esdhc_ops = {
> >  	.get_max_clock = esdhc_pltfm_get_max_clock,
> >  	.get_min_clock = esdhc_pltfm_get_min_clock,
> >  	.get_max_timeout_count = esdhc_get_max_timeout_count,
> > -	.get_ro = esdhc_pltfm_get_ro,
> >  	.set_timeout = esdhc_set_timeout,
> >  	.set_bus_width = esdhc_pltfm_set_bus_width,
> >  	.set_uhs_signaling = esdhc_set_uhs_signaling, @@ -1588,17 +1554,6 @@
> > sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
> >  	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
> >  	int ret;
> >
> > -	if (of_property_read_bool(np, "fsl,wp-controller"))
> > -		boarddata->wp_type = ESDHC_WP_CONTROLLER;
> 
> esdhc_pltfm_get_ro() differs from sdhci_check_ro() by defaulting to -ENOSYS if
> there is neither "fsl,wp-controller" nor "wp-gpios", which would result in
> write-protect off (not supported, e.g. microsd).
> So more explanation is needed for why removing "fsl,wp-controller"
> will give the same result.

Okay, will add. Thanks.

Best Regards
Haibo Chen
> 
> > -
> > -	/*
> > -	 * If we have this property, then activate WP check.
> > -	 * Retrieveing and requesting the actual WP GPIO will happen
> > -	 * in the call to mmc_of_parse().
> > -	 */
> > -	if (of_property_read_bool(np, "wp-gpios"))
> > -		boarddata->wp_type = ESDHC_WP_GPIO;
> > -
> >  	of_property_read_u32(np, "fsl,tuning-step", &boarddata->tuning_step);
> >  	of_property_read_u32(np, "fsl,tuning-start-tap",
> >  			     &boarddata->tuning_start_tap);

[PATCH v2 3/4] dt-bindings: mmc: fsl-imx-esdhc: remove property "fsl,wp-controller"

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

Driver do not use this property, so remove it here.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

---
v2:
-fix dt-binding-check issue, also remove "fsl,wp-controller" in examples.
---
 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index fbfd822b9270..e34b4fb4778a 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -96,11 +96,6 @@ properties:
   interrupts:
     maxItems: 1
 
-  fsl,wp-controller:
-    description: |
-      boolean, if present, indicate to use controller internal write protection.
-    type: boolean
-
   fsl,delay-line:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: |
@@ -190,7 +185,6 @@ examples:
         compatible = "fsl,imx51-esdhc";
         reg = <0x70004000 0x4000>;
         interrupts = <1>;
-        fsl,wp-controller;
     };
 
     mmc@70008000 {
-- 
2.34.1

Re: [PATCH v2 3/4] dt-bindings: mmc: fsl-imx-esdhc: remove property "fsl,wp-controller"

[Krzysztof Kozlowski]

On 06/05/2023 09:40, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Driver do not use this property, so remove it here.

You still need to expand the commit msg and justify why this property is
being removed. "Driver" is vague, as it can be BSD driver which also
might use this binding. Instead describe why this property should be
removed, so we can understand also ABI implications.

> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> 
> ---
> v2:
> -fix dt-binding-check issue, also remove "fsl,wp-controller" in examples.
> ---
>  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 6 ------
>  1 file changed, 6 deletions(-)

Best regards,
Krzysztof

Re: [PATCH v2 3/4] dt-bindings: mmc: fsl-imx-esdhc: remove property "fsl,wp-controller"

[Peng Fan]

On 5/6/2023 6:59 PM, Krzysztof Kozlowski wrote:
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
> 
> 
> On 06/05/2023 09:40, haibo.chen@nxp.com wrote:
>> From: Haibo Chen <haibo.chen@nxp.com>
>>
>> Driver do not use this property, so remove it here.
> 
> You still need to expand the commit msg and justify why this property is
> being removed. "Driver" is vague, as it can be BSD driver which also
> might use this binding. Instead describe why this property should be
> removed, so we can understand also ABI implications.

The better approach would be mark it as deprecated.

Regards,
Peng.

> 
>>
>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>
>> ---
>> v2:
>> -fix dt-binding-check issue, also remove "fsl,wp-controller" in examples.
>> ---
>>   Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 6 ------
>>   1 file changed, 6 deletions(-)
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 3/4] dt-bindings: mmc: fsl-imx-esdhc: remove property "fsl,wp-controller"

[Rob Herring]

On Tue, May 23, 2023 at 10:03:08AM +0800, Peng Fan wrote:
> 
> 
> On 5/6/2023 6:59 PM, Krzysztof Kozlowski wrote:
> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
> > 
> > 
> > On 06/05/2023 09:40, haibo.chen@nxp.com wrote:
> > > From: Haibo Chen <haibo.chen@nxp.com>
> > > 
> > > Driver do not use this property, so remove it here.
> > 
> > You still need to expand the commit msg and justify why this property is
> > being removed. "Driver" is vague, as it can be BSD driver which also
> > might use this binding. Instead describe why this property should be
> > removed, so we can understand also ABI implications.
> 
> The better approach would be mark it as deprecated.

Sure, but still need to say why.

Rob

[PATCH v2 4/4] arm: dts: imx: remove "fsl,wp-controller" since it is dropped

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The driver sdhci-esdhc-imx.c drop the "fsl,wp-controller", so do
a clean up in the dts file.

Now the logic of write protect is:
if want to use gpio method to implement the write protect, need
to add "wp-gpios" in dts file, otherwise will use the controller
internal logic to implement the write protect. This controller
method need to config one pin as a specific write protect function
used for the controller. If not do this config or the board do not
has this pin connect to card socket, then the write is enabled.

So now remove "fsl,wp-controller" will not involve any logic change.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm/boot/dts/imx53-sk-imx53.dts         | 1 -
 arch/arm/boot/dts/imx53-tx53.dtsi            | 2 --
 arch/arm/boot/dts/imx6dl-tx6s-8035.dts       | 1 -
 arch/arm/boot/dts/imx6dl-tx6u-8033.dts       | 1 -
 arch/arm/boot/dts/imx6q-bosch-acc.dts        | 2 --
 arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts | 1 -
 arch/arm/boot/dts/imx6q-tx6q-1020.dts        | 1 -
 arch/arm/boot/dts/imx6q-tx6q-1036.dts        | 1 -
 arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi     | 1 -
 arch/arm/boot/dts/imx6qdl-emcon.dtsi         | 2 --
 arch/arm/boot/dts/imx6qdl-ts4900.dtsi        | 1 -
 arch/arm/boot/dts/imx6qdl-ts7970.dtsi        | 1 -
 arch/arm/boot/dts/imx6qdl-tx6.dtsi           | 2 --
 arch/arm/boot/dts/imx6qp-tx6qp-8037.dts      | 1 -
 arch/arm/boot/dts/imx6qp-tx6qp-8137.dts      | 1 -
 arch/arm/boot/dts/imx6ul-tx6ul-0011.dts      | 1 -
 arch/arm/boot/dts/imx6ul-tx6ul.dtsi          | 1 -
 arch/arm/boot/dts/imxrt1050.dtsi             | 1 -
 18 files changed, 22 deletions(-)

diff --git a/arch/arm/boot/dts/imx53-sk-imx53.dts b/arch/arm/boot/dts/imx53-sk-imx53.dts
index 103e73176e47..071e8b4b0a7e 100644
--- a/arch/arm/boot/dts/imx53-sk-imx53.dts
+++ b/arch/arm/boot/dts/imx53-sk-imx53.dts
@@ -76,7 +76,6 @@ &ecspi2 {
 
 &esdhc1 {
 	cd-gpios = <&gpio3 14 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx53-tx53.dtsi b/arch/arm/boot/dts/imx53-tx53.dtsi
index a439a47fb65a..4bef504008cc 100644
--- a/arch/arm/boot/dts/imx53-tx53.dtsi
+++ b/arch/arm/boot/dts/imx53-tx53.dtsi
@@ -196,7 +196,6 @@ &gpio3 19 GPIO_ACTIVE_HIGH
 
 &esdhc1 {
 	cd-gpios = <&gpio3 24 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
 	status = "okay";
@@ -204,7 +203,6 @@ &esdhc1 {
 
 &esdhc2 {
 	cd-gpios = <&gpio3 25 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc2>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx6dl-tx6s-8035.dts b/arch/arm/boot/dts/imx6dl-tx6s-8035.dts
index a5532ecc18c5..63f043372931 100644
--- a/arch/arm/boot/dts/imx6dl-tx6s-8035.dts
+++ b/arch/arm/boot/dts/imx6dl-tx6s-8035.dts
@@ -67,7 +67,6 @@ &usdhc4 {
 	bus-width = <4>;
 	non-removable;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6dl-tx6u-8033.dts b/arch/arm/boot/dts/imx6dl-tx6u-8033.dts
index 7030b2654bbd..2d24dae7dbfc 100644
--- a/arch/arm/boot/dts/imx6dl-tx6u-8033.dts
+++ b/arch/arm/boot/dts/imx6dl-tx6u-8033.dts
@@ -63,7 +63,6 @@ &usdhc4 {
 	bus-width = <4>;
 	non-removable;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-bosch-acc.dts b/arch/arm/boot/dts/imx6q-bosch-acc.dts
index 8263bfef9bf8..26794f425a59 100644
--- a/arch/arm/boot/dts/imx6q-bosch-acc.dts
+++ b/arch/arm/boot/dts/imx6q-bosch-acc.dts
@@ -573,7 +573,6 @@ &usdhc2 {
 	wakeup-source;
 	voltage-ranges = <3300 3300>;
 	vmmc-supply = <&reg_sw4>;
-	fsl,wp-controller;
 	status = "okay";
 };
 
@@ -586,7 +585,6 @@ &usdhc4 {
 	keep-power-in-suspend;
 	voltage-ranges = <3300 3300>;
 	vmmc-supply = <&reg_sw4>;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts b/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
index a773f252816c..15956a92fe8b 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1020-comtft.dts
@@ -91,7 +91,6 @@ &usdhc4 {
 	pinctrl-0 = <&pinctrl_usdhc4>;
 	bus-width = <4>;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1020.dts b/arch/arm/boot/dts/imx6q-tx6q-1020.dts
index 0a4daec8d3ad..9dc92773c2f1 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1020.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1020.dts
@@ -67,7 +67,6 @@ &usdhc4 {
 	bus-width = <4>;
 	non-removable;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6q-tx6q-1036.dts b/arch/arm/boot/dts/imx6q-tx6q-1036.dts
index cb2fcb4896c6..24eba1d2afd9 100644
--- a/arch/arm/boot/dts/imx6q-tx6q-1036.dts
+++ b/arch/arm/boot/dts/imx6q-tx6q-1036.dts
@@ -67,7 +67,6 @@ &usdhc4 {
 	bus-width = <4>;
 	non-removable;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi
index eaa87b333164..f8b985029f51 100644
--- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi
@@ -412,7 +412,6 @@ &usdhc2 { /* External SD card via DHCOM */
 
 &usdhc3 { /* Micro SD card on module */
 	cd-gpios = <&gpio7 8 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	keep-power-in-suspend;
 	pinctrl-0 = <&pinctrl_usdhc3>;
 	pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/imx6qdl-emcon.dtsi b/arch/arm/boot/dts/imx6qdl-emcon.dtsi
index ee2dd75cead6..61306e6da6bd 100644
--- a/arch/arm/boot/dts/imx6qdl-emcon.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-emcon.dtsi
@@ -792,13 +792,11 @@ &usbotg {
 &usdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usdhc1>;
-	fsl,wp-controller;
 };
 
 &usdhc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usdhc2>;
-	fsl,wp-controller;
 };
 
 &usdhc3 {
diff --git a/arch/arm/boot/dts/imx6qdl-ts4900.dtsi b/arch/arm/boot/dts/imx6qdl-ts4900.dtsi
index f88da757edda..6d2f04434d6c 100644
--- a/arch/arm/boot/dts/imx6qdl-ts4900.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-ts4900.dtsi
@@ -464,7 +464,6 @@ &usdhc2 {
 	pinctrl-0 = <&pinctrl_usdhc2>;
 	vmmc-supply = <&reg_3p3v>;
 	bus-width = <4>;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-ts7970.dtsi b/arch/arm/boot/dts/imx6qdl-ts7970.dtsi
index 1e0a041e9f60..d4c967971f6a 100644
--- a/arch/arm/boot/dts/imx6qdl-ts7970.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-ts7970.dtsi
@@ -579,7 +579,6 @@ &usdhc2 {
 	pinctrl-0 = <&pinctrl_usdhc2>;
 	vmmc-supply = <&reg_3p3v>;
 	bus-width = <4>;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-tx6.dtsi b/arch/arm/boot/dts/imx6qdl-tx6.dtsi
index a197bac95cba..e74fb3171656 100644
--- a/arch/arm/boot/dts/imx6qdl-tx6.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tx6.dtsi
@@ -777,7 +777,6 @@ &usdhc1 {
 	bus-width = <4>;
 	no-1-8-v;
 	cd-gpios = <&gpio7 2 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	status = "okay";
 };
 
@@ -787,6 +786,5 @@ &usdhc2 {
 	bus-width = <4>;
 	no-1-8-v;
 	cd-gpios = <&gpio7 3 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6qp-tx6qp-8037.dts b/arch/arm/boot/dts/imx6qp-tx6qp-8037.dts
index ffc0f2ee11d2..18cda0d42826 100644
--- a/arch/arm/boot/dts/imx6qp-tx6qp-8037.dts
+++ b/arch/arm/boot/dts/imx6qp-tx6qp-8037.dts
@@ -67,7 +67,6 @@ &usdhc4 {
 	bus-width = <4>;
 	non-removable;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qp-tx6qp-8137.dts b/arch/arm/boot/dts/imx6qp-tx6qp-8137.dts
index dd494d587014..7478ae7535f4 100644
--- a/arch/arm/boot/dts/imx6qp-tx6qp-8137.dts
+++ b/arch/arm/boot/dts/imx6qp-tx6qp-8137.dts
@@ -71,7 +71,6 @@ &usdhc4 {
 	bus-width = <4>;
 	non-removable;
 	no-1-8-v;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6ul-tx6ul-0011.dts b/arch/arm/boot/dts/imx6ul-tx6ul-0011.dts
index d82698e7d50f..5a2e8edf149e 100644
--- a/arch/arm/boot/dts/imx6ul-tx6ul-0011.dts
+++ b/arch/arm/boot/dts/imx6ul-tx6ul-0011.dts
@@ -63,6 +63,5 @@ &usdhc2 {
 	bus-width = <4>;
 	no-1-8-v;
 	non-removable;
-	fsl,wp-controller;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6ul-tx6ul.dtsi b/arch/arm/boot/dts/imx6ul-tx6ul.dtsi
index 70cef5e817bd..62c96884bfda 100644
--- a/arch/arm/boot/dts/imx6ul-tx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul-tx6ul.dtsi
@@ -575,7 +575,6 @@ &usdhc1 {
 	bus-width = <4>;
 	no-1-8-v;
 	cd-gpios = <&gpio4 14 GPIO_ACTIVE_LOW>;
-	fsl,wp-controller;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imxrt1050.dtsi b/arch/arm/boot/dts/imxrt1050.dtsi
index 852861558b47..7e983c52ebcd 100644
--- a/arch/arm/boot/dts/imxrt1050.dtsi
+++ b/arch/arm/boot/dts/imxrt1050.dtsi
@@ -91,7 +91,6 @@ usdhc1: mmc@402c0000 {
 				<&clks IMXRT1050_CLK_USDHC1>;
 			clock-names = "ipg", "ahb", "per";
 			bus-width = <4>;
-			fsl,wp-controller;
 			no-1-8-v;
 			max-frequency = <200000000>;
 			fsl,tuning-start-tap = <20>;
-- 
2.34.1