Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20231122060215.2074799-1-vigneshr@ti.com>
In-Reply-To: <20231122060215.2074799-1-vigneshr@ti.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Thu, 23 Nov 2023 18:18:41 +0100
Message-ID: <CAPDyKFrsXSwyU-VsPEt1tYCPWk99XaNZ7kJGGWPEDEYBWaPktg@mail.gmail.com>
Subject: Re: [PATCH v2] mmc: sdhci_am654: Drop lookup for deprecated ti,otap-del-sel
To:     Vignesh Raghavendra <vigneshr@ti.com>
Cc:     Adrian Hunter <adrian.hunter@intel.com>, linux-mmc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 22 Nov 2023 at 07:05, Vignesh Raghavendra <vigneshr@ti.com> wrote:
>
> ti,otap-del-sel has been deprecated since v5.7 and there are no users of
> this property and no documentation in the DT bindings either.
> Drop the fallback code looking for this property, this makes
> sdhci_am654_get_otap_delay() much easier to read as all the TAP values
> can be handled via a single iterator loop.
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>
> v2:
> * Drop legacy_otapdly as pointed out by Adrian
>
> v1: https://lore.kernel.org/r/20231110072535.2695134-1-vigneshr@ti.com
>
>  drivers/mmc/host/sdhci_am654.c | 37 ++++++----------------------------
>  1 file changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index 967bd2dfcda1..d659c59422e1 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -141,7 +141,6 @@ static const struct timing_data td[] = {
>
>  struct sdhci_am654_data {
>         struct regmap *base;
> -       bool legacy_otapdly;
>         int otap_del_sel[ARRAY_SIZE(td)];
>         int itap_del_sel[ARRAY_SIZE(td)];
>         int clkbuf_sel;
> @@ -272,11 +271,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>         sdhci_set_clock(host, clock);
>
>         /* Setup DLL Output TAP delay */
> -       if (sdhci_am654->legacy_otapdly)
> -               otap_del_sel = sdhci_am654->otap_del_sel[0];
> -       else
> -               otap_del_sel = sdhci_am654->otap_del_sel[timing];
> -
> +       otap_del_sel = sdhci_am654->otap_del_sel[timing];
>         otap_del_ena = (timing > MMC_TIMING_UHS_SDR25) ? 1 : 0;
>
>         mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> @@ -314,10 +309,7 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
>         u32 mask, val;
>
>         /* Setup DLL Output TAP delay */
> -       if (sdhci_am654->legacy_otapdly)
> -               otap_del_sel = sdhci_am654->otap_del_sel[0];
> -       else
> -               otap_del_sel = sdhci_am654->otap_del_sel[timing];
> +       otap_del_sel = sdhci_am654->otap_del_sel[timing];
>
>         mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
>         val = (0x1 << OTAPDLYENA_SHIFT) |
> @@ -577,32 +569,15 @@ static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
>         int i;
>         int ret;
>
> -       ret = device_property_read_u32(dev, td[MMC_TIMING_LEGACY].otap_binding,
> -                                &sdhci_am654->otap_del_sel[MMC_TIMING_LEGACY]);
> -       if (ret) {
> -               /*
> -                * ti,otap-del-sel-legacy is mandatory, look for old binding
> -                * if not found.
> -                */
> -               ret = device_property_read_u32(dev, "ti,otap-del-sel",
> -                                              &sdhci_am654->otap_del_sel[0]);
> -               if (ret) {
> -                       dev_err(dev, "Couldn't find otap-del-sel\n");
> -
> -                       return ret;
> -               }
> -
> -               dev_info(dev, "Using legacy binding ti,otap-del-sel\n");
> -               sdhci_am654->legacy_otapdly = true;
> -
> -               return 0;
> -       }
> -
>         for (i = MMC_TIMING_LEGACY; i <= MMC_TIMING_MMC_HS400; i++) {
>
>                 ret = device_property_read_u32(dev, td[i].otap_binding,
>                                                &sdhci_am654->otap_del_sel[i]);
>                 if (ret) {
> +                       if (i == MMC_TIMING_LEGACY) {
> +                               dev_err(dev, "Couldn't find mandatory ti,otap-del-sel-legacy\n");
> +                               return ret;
> +                       }
>                         dev_dbg(dev, "Couldn't find %s\n",
>                                 td[i].otap_binding);
>                         /*
> --
> 2.42.0
>