| Message ID | 20230506074037.522961-3-haibo.chen@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cd/wp code clean up for i.MX USDHC | expand |
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);
> -----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);
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);