Message ID | 20191114125435.27756-3-i.mikhaylov@yadro.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | add inversion signal presence support | expand |
On 14/11/19 2:54 PM, Ivan Mikhaylov wrote: > Change the default .get_cd callback. Add inverted signal card detection > check. > > Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com> > > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c > index 8962f6664381..186559ee8fcc 100644 > --- a/drivers/mmc/host/sdhci-of-aspeed.c > +++ b/drivers/mmc/host/sdhci-of-aspeed.c > @@ -143,6 +143,19 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev, > return (delta / 0x100) - 1; > } > > +static int aspeed_get_cd(struct mmc_host *mmc) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + > + int present = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) > + & SDHCI_CARD_PRESENT); > + > + if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) > + present = !present; Perhaps safer to flip the bit using CONFIG_MMC_SDHCI_IO_ACCESSORS and ->readl() callback > + > + return present; > +} > + > static int aspeed_sdhci_probe(struct platform_device *pdev) > { > struct sdhci_pltfm_host *pltfm_host; > @@ -183,6 +196,10 @@ static int aspeed_sdhci_probe(struct platform_device *pdev) > goto err_pltfm_free; > } > > + host->mmc_host_ops.get_cd = aspeed_get_cd; > + if (of_property_read_bool(pdev->dev.of_node, "cd-inverted")) > + dev_info(&pdev->dev, "aspeed: sdhci: presence signal inversion enabled\n"); Is this print really needed? > + > ret = mmc_of_parse(host->mmc); > if (ret) > goto err_sdhci_add; >
On Thu, 2019-11-14 at 15:10 +0200, Adrian Hunter wrote: > On 14/11/19 2:54 PM, Ivan Mikhaylov wrote: > > Change the default .get_cd callback. Add inverted signal card detection > > check. > > > > Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com> > > > > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of- > > aspeed.c > > index 8962f6664381..186559ee8fcc 100644 > > --- a/drivers/mmc/host/sdhci-of-aspeed.c > > +++ b/drivers/mmc/host/sdhci-of-aspeed.c > > @@ -143,6 +143,19 @@ static inline int aspeed_sdhci_calculate_slot(struct > > aspeed_sdhci *dev, > > return (delta / 0x100) - 1; > > } > > > > +static int aspeed_get_cd(struct mmc_host *mmc) > > +{ > > + struct sdhci_host *host = mmc_priv(mmc); > > + > > + int present = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) > > + & SDHCI_CARD_PRESENT); > > + > > + if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) > > + present = !present; > > Perhaps safer to flip the bit using CONFIG_MMC_SDHCI_IO_ACCESSORS and > ->readl() callback > Sorry, don't quite understand what you're saying. You want to instantiate '.read_l' callback instead of '.get_cd' in sdhci_ops and substitute the real value? res = readl(base, reg); if (reg == SDHCI_PRESENT_STATE) if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) return !res; return res; Something like this? > > > + host->mmc_host_ops.get_cd = aspeed_get_cd; > > + if (of_property_read_bool(pdev->dev.of_node, "cd-inverted")) > > + dev_info(&pdev->dev, "aspeed: sdhci: presence signal inversion > > enabled\n"); > > Is this print really needed? > I can remove it if you think it's redundant. Thanks.
On 14/11/19 7:19 PM, Ivan Mikhaylov wrote: > On Thu, 2019-11-14 at 15:10 +0200, Adrian Hunter wrote: >> On 14/11/19 2:54 PM, Ivan Mikhaylov wrote: >>> Change the default .get_cd callback. Add inverted signal card detection >>> check. >>> >>> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com> >>> >>> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of- >>> aspeed.c >>> index 8962f6664381..186559ee8fcc 100644 >>> --- a/drivers/mmc/host/sdhci-of-aspeed.c >>> +++ b/drivers/mmc/host/sdhci-of-aspeed.c >>> @@ -143,6 +143,19 @@ static inline int aspeed_sdhci_calculate_slot(struct >>> aspeed_sdhci *dev, >>> return (delta / 0x100) - 1; >>> } >>> >>> +static int aspeed_get_cd(struct mmc_host *mmc) >>> +{ >>> + struct sdhci_host *host = mmc_priv(mmc); >>> + >>> + int present = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) >>> + & SDHCI_CARD_PRESENT); >>> + >>> + if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) >>> + present = !present; >> >> Perhaps safer to flip the bit using CONFIG_MMC_SDHCI_IO_ACCESSORS and >> ->readl() callback >> > > Sorry, don't quite understand what you're saying. You want to instantiate > '.read_l' callback instead of '.get_cd' in sdhci_ops and substitute the real > value? > > res = readl(base, reg); > if (reg == SDHCI_PRESENT_STATE) > if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) > return !res; Presumably just flip the SDHCI_CARD_PRESENT bit i.e. return res ^ SDHCI_CARD_PRESENT; > return res; > > Something like this? Yes > >> >>> + host->mmc_host_ops.get_cd = aspeed_get_cd; >>> + if (of_property_read_bool(pdev->dev.of_node, "cd-inverted")) >>> + dev_info(&pdev->dev, "aspeed: sdhci: presence signal inversion >>> enabled\n"); >> >> Is this print really needed? >> > I can remove it if you think it's redundant. > > Thanks. > >
On Fri, 2019-11-15 at 09:20 +0200, Adrian Hunter wrote: > On 14/11/19 7:19 PM, Ivan Mikhaylov wrote: > > On Thu, 2019-11-14 at 15:10 +0200, Adrian Hunter wrote: > > On 14/11/19 2:54 PM, Ivan Mikhaylov wrote: > > > > Change the default .get_cd callback. Add inverted signal card detection > > > > check. > > > > > > > > Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com> > > > > > > > > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c > > > > b/drivers/mmc/host/sdhci-of- > > > > aspeed.c > > > > index 8962f6664381..186559ee8fcc 100644 > > > > --- a/drivers/mmc/host/sdhci-of-aspeed.c > > > > +++ b/drivers/mmc/host/sdhci-of-aspeed.c > > > > @@ -143,6 +143,19 @@ static inline int > > > > aspeed_sdhci_calculate_slot(struct > > > > aspeed_sdhci *dev, > > > > return (delta / 0x100) - 1; > > > > } > > > > > > > > +static int aspeed_get_cd(struct mmc_host *mmc) > > > > +{ > > > > + struct sdhci_host *host = mmc_priv(mmc); > > > > + > > > > + int present = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) > > > > + & SDHCI_CARD_PRESENT); > > > > + > > > > + if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) > > > > + present = !present; > > > > > > Perhaps safer to flip the bit using CONFIG_MMC_SDHCI_IO_ACCESSORS and > > > ->readl() callback > > > > Sorry, don't quite understand what you're saying. You want to instantiate > > '.read_l' callback instead of '.get_cd' in sdhci_ops and substitute the real > > value? > > > > res = readl(base, reg); > > if (reg == SDHCI_PRESENT_STATE) > > if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) > > return !res; > > Presumably just flip the SDHCI_CARD_PRESENT bit i.e. > > return res ^ SDHCI_CARD_PRESENT; > > > return res; > > > > Something like this? > > Yes > Don't you think it will bring a little overhead on any sdhci_readl plus sdhci_readl will not get the real value in case of inverted signal which seems is not right from communication fairness between hw and sw? I took that approach with .get_cd from variety of drivers in host/mmc but if you think it will be better and safer with .read_l - I'll do that way. Sorry for the link in subject, didn't notice that I put it in previous message somehow. Thanks.
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c index 8962f6664381..186559ee8fcc 100644 --- a/drivers/mmc/host/sdhci-of-aspeed.c +++ b/drivers/mmc/host/sdhci-of-aspeed.c @@ -143,6 +143,19 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev, return (delta / 0x100) - 1; } +static int aspeed_get_cd(struct mmc_host *mmc) +{ + struct sdhci_host *host = mmc_priv(mmc); + + int present = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) + & SDHCI_CARD_PRESENT); + + if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH) + present = !present; + + return present; +} + static int aspeed_sdhci_probe(struct platform_device *pdev) { struct sdhci_pltfm_host *pltfm_host; @@ -183,6 +196,10 @@ static int aspeed_sdhci_probe(struct platform_device *pdev) goto err_pltfm_free; } + host->mmc_host_ops.get_cd = aspeed_get_cd; + if (of_property_read_bool(pdev->dev.of_node, "cd-inverted")) + dev_info(&pdev->dev, "aspeed: sdhci: presence signal inversion enabled\n"); + ret = mmc_of_parse(host->mmc); if (ret) goto err_sdhci_add;
Change the default .get_cd callback. Add inverted signal card detection check. Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>