Re: [PATCH 09/11] mmc: sdhci-cadence: Add Pensando Elba SoC support

[Brad Larson <brad@xxxxxxxxxxx>]

Hi Arnd,

On Thu, Apr 7, 2022 at 1:38 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Thu, Apr 7, 2022 at 7:06 PM Brad Larson <brad@xxxxxxxxxxx> wrote:
> > On Thu, Apr 7, 2022 at 12:13 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> > >
> > > What is the relationship between cadence and pensando elba?
> >
> > Pensando licensed the cadence controller, its 100% cadence IP.  The
> > integration issue we ran into was with the accessors where we have the
> > workaround.  The initial patch added a separate Elba driver file but
> > the feedback was the Elba support didn't justify doing that and to add
> > to sdhci-cacence.c.
>
> I looked back at the earlier reviews now, I think the main problem with
> versions 1 and 2 was that it had the abstraction the wrong way around,
> so you added the complexity of having multiple files, without the benefits.
>
> I still think that the cleanest approach would be to have it the way I
> suggested in my reply to v1, with an elba specific platform driver
> that calls into the generic cadence code, but the generic code knowing
> nothing about the front-end.
>
> Then again, it sounds like there was already an agreement about
> the approach you took here, so let's stay with that and hope we don't
> get any other chips with the same IP block in the future.

Thanks for looking this over.  I won't change for now in the patch
update in process.  This will likely get another look as I've added a
node to the device tree to enable an added reset driver which can
hardware reset the emmc.  The current cadence sdhci driver does not
include this and I've currently added filling out
mmc_host_ops.card_hw_reset like below, yet one more thing different
for our platforms not in the common driver.

@@ -404,6 +550,17 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
        if (ret)
                goto free;

+       if (host->mmc->caps & MMC_CAP_HW_RESET) {
+               priv->rst_hw =
devm_reset_control_get_optional_exclusive(dev, "hw");
+               if (IS_ERR(priv->rst_hw)) {
+                       ret = PTR_ERR(priv->rst_hw);
+                       if (ret == -ENOENT)
+                               priv->rst_hw = NULL;
+               } else {
+                       host->mmc_host_ops.card_hw_reset = sdhci_mmc_hw_reset;
+               }
+       }
+
        ret = sdhci_add_host(host);

Regards,
Brad