Re: [PATCH v5 1/5] mmc: core: Extend mmc_of_parse() to parse CQE bindings

[Chun-Hung Wu <chun-hung.wu@mediatek.com>]

On Fri, 2020-05-08 at 13:05 +0800, Ulf Hansson wrote:
> On Thu, 7 May 2020 at 18:33, Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org> wrote:
> >
> >
> > On 5/6/2020 10:06 PM, Ulf Hansson wrote:
> > > On Wed, 6 May 2020 at 15:01, Veerabhadrarao Badiganti
> > > <vbadigan@codeaurora.org> wrote:
> > >>
> > >> On 4/28/2020 5:26 AM, Chun-Hung Wu wrote:
> > >>> Parse CQE bindings "supports-cqe" and "disable-cqe-dcmd"
> > >>> in mmc_of_parse().
> > >>>
> > >>> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> > >>> ---
> > >>>    drivers/mmc/core/host.c | 5 +++++
> > >>>    1 file changed, 5 insertions(+)
> > >>>
> > >>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > >>> index c876872..47521c6 100644
> > >>> --- a/drivers/mmc/core/host.c
> > >>> +++ b/drivers/mmc/core/host.c
> > >>> @@ -302,6 +302,11 @@ int mmc_of_parse(struct mmc_host *host)
> > >>>                host->caps2 |= MMC_CAP2_NO_SD;
> > >>>        if (device_property_read_bool(dev, "no-mmc"))
> > >>>                host->caps2 |= MMC_CAP2_NO_MMC;
> > >>> +     if (device_property_read_bool(dev, "supports-cqe"))
> > >>> +             host->caps2 |= MMC_CAP2_CQE;
> > >> This change is breaking emmc driver on qcom platforms where this dt
> > >> property is defined.
> > >>
> > >> [    1.543453]  cqhci_deactivate+0xc/0x38
> > >> [    1.545627]  sdhci_msm_reset+0x40/0x58
> > >> [    1.549447]  sdhci_do_reset+0x48/0x7c
> > >> [    1.553180]  __sdhci_read_caps+0x7c/0x214
> > >> [    1.556913]  sdhci_setup_host+0x58/0xce8
> > >> [    1.560905]  sdhci_msm_probe+0x588/0x8a4
> > >> [    1.564900]  platform_drv_probe+0x4c/0xb0
> > >>
> > >> So, we cant have this flag defined before sdhci_setup_host().
> > >>
> > >> I will have to clear this cap and re-enable it in our initialization.
> > > Thanks for reporting! I have dropped all the four patches from
> > > Chun-Hung, so we can figure out how to fix this.
> > >
> > > Please help to review the next version of the series.
> >
> > Thanks Ulf.
> >
> > Hi Chun-Hung,
> >
> > On qcom controller CQE also gets reset when SDHC is reset. So we have to
> > explicitly disable CQE
> > by invoking  cqhci_deactivate() during sdhc reset
> >
> > SDHC gets reset in sdhci_setup_host() even before cqe is initialized.
> > With MMC_CAP2_CQE_DCMD cap set even before sdhci_set_host(), we are
> > getting null pointer access with cqhci_deactivate().
> >
> > If CQE getting reset with SDHC reset is generic (applicable to other
> > controllers) then you have revisit your logic.
> > If its not the case then only qcom driver would get affected.
> 
> Thanks for clarifying the problem, much appreciated.
> 
> To me, it looks like the DT parsing of the CQE properties are better
> suited to be managed by each sdhci variant, to continue to leave some
> room for flexibility.
> 
> Chun-Hung, can you please drop patch 1 and patch2 from the series and
> adapt to this change in the mediatek variant?
Ok, I will rollback patch1 and patch2 from series and keep DT parsing
of the CQE properties by each sdhci/msdc variant.
CQE properties will move to mediatek variant.
> 
> [...]
> 
> Kind regards
> Uffe

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek