| Message ID | 878um178vn.wl%kuninori.morimoto.gx@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 3 September 2014 04:09, Kuninori Morimoto <kuninori.morimoto.gx@gmail.com> wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Now, omap_hsmmc can use .multi_io_quirk callback > instead of MMC_CAP2_NO_MULTI_READ flags. > let's use it. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > v2 -> v3 > > - blk_size_workaround -> multi_io_quirk > > drivers/mmc/host/omap_hsmmc.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > index ece1634..7b41f57 100644 > --- a/drivers/mmc/host/omap_hsmmc.c > +++ b/drivers/mmc/host/omap_hsmmc.c > @@ -219,6 +219,7 @@ struct omap_hsmmc_host { > #define AUTO_CMD23 (1 << 0) /* Auto CMD23 support */ > #define HSMMC_SDIO_IRQ_ENABLED (1 << 1) /* SDIO irq enabled */ > #define HSMMC_WAKE_IRQ_ENABLED (1 << 2) > +#define HSMMC_NO_MULTI_READ (1 << 3) > struct omap_hsmmc_next next_data; > struct omap_mmc_platform_data *pdata; > }; > @@ -1829,6 +1830,19 @@ static int omap_hsmmc_disable_fclk(struct mmc_host *mmc) > return 0; > } > > +static int *omap_hsmmc_multi_io_quirk(struct mmc_card *card, > + unsigned int direction, int blk_size) > +{ > + struct omap_hsmmc_host *host = mmc_priv(card->host); > + > + /* This controller can't do multiblock reads due to hw bugs */ > + if ((host->flags & HSMMC_NO_MULTI_READ) && > + (direction == MMC_DATA_READ)) > + return 1; > + > + return blk_size; > +} > + > static const struct mmc_host_ops omap_hsmmc_ops = { > .enable = omap_hsmmc_enable_fclk, > .disable = omap_hsmmc_disable_fclk, > @@ -1840,6 +1854,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = { > .get_ro = omap_hsmmc_get_ro, > .init_card = omap_hsmmc_init_card, > .enable_sdio_irq = omap_hsmmc_enable_sdio_irq, > + .multi_io_quirk = omap_hsmmc_multi_io_quirk, > }; > > #ifdef CONFIG_DEBUG_FS > @@ -2101,7 +2116,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev) > > if (host->pdata->controller_flags & OMAP_HSMMC_BROKEN_MULTIBLOCK_READ) { > dev_info(&pdev->dev, "multiblock reads disabled due to 35xx erratum 2.1.1.128; MMC read performance may suffer\n"); > - mmc->caps2 |= MMC_CAP2_NO_MULTI_READ; An option to adding yet another flag, would be to assign ->multi_io_quirk = omap_hsmmc_multi_io_quirk() here? Isn't that actually better for those variants that doesn't suffer from this HW bug? Kind regards Uffe > + host->flags |= HSMMC_NO_MULTI_READ; > } > > pm_runtime_enable(host->dev); > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > > index ece1634..7b41f57 100644 > > --- a/drivers/mmc/host/omap_hsmmc.c > > +++ b/drivers/mmc/host/omap_hsmmc.c > > @@ -219,6 +219,7 @@ struct omap_hsmmc_host { > > #define AUTO_CMD23 (1 << 0) /* Auto CMD23 support */ > > #define HSMMC_SDIO_IRQ_ENABLED (1 << 1) /* SDIO irq enabled */ > > #define HSMMC_WAKE_IRQ_ENABLED (1 << 2) > > +#define HSMMC_NO_MULTI_READ (1 << 3) > > struct omap_hsmmc_next next_data; > > struct omap_mmc_platform_data *pdata; > > }; (snip) > > #ifdef CONFIG_DEBUG_FS > > @@ -2101,7 +2116,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev) > > > > if (host->pdata->controller_flags & OMAP_HSMMC_BROKEN_MULTIBLOCK_READ) { > > dev_info(&pdev->dev, "multiblock reads disabled due to 35xx erratum 2.1.1.128; MMC read performance may suffer\n"); > > - mmc->caps2 |= MMC_CAP2_NO_MULTI_READ; > > An option to adding yet another flag, would be to assign > ->multi_io_quirk = omap_hsmmc_multi_io_quirk() here? Isn't that > actually better for those variants that doesn't suffer from this HW > bug? This means we need to copy this ops for each driver ? Otherwise, it breaks behavior if many drivers were probed. Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9 September 2014 02:09, Kuninori Morimoto <kuninori.morimoto.gx@gmail.com> wrote: > > Hi Ulf > >> > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> > index ece1634..7b41f57 100644 >> > --- a/drivers/mmc/host/omap_hsmmc.c >> > +++ b/drivers/mmc/host/omap_hsmmc.c >> > @@ -219,6 +219,7 @@ struct omap_hsmmc_host { >> > #define AUTO_CMD23 (1 << 0) /* Auto CMD23 support */ >> > #define HSMMC_SDIO_IRQ_ENABLED (1 << 1) /* SDIO irq enabled */ >> > #define HSMMC_WAKE_IRQ_ENABLED (1 << 2) >> > +#define HSMMC_NO_MULTI_READ (1 << 3) >> > struct omap_hsmmc_next next_data; >> > struct omap_mmc_platform_data *pdata; >> > }; > (snip) >> > #ifdef CONFIG_DEBUG_FS >> > @@ -2101,7 +2116,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev) >> > >> > if (host->pdata->controller_flags & OMAP_HSMMC_BROKEN_MULTIBLOCK_READ) { >> > dev_info(&pdev->dev, "multiblock reads disabled due to 35xx erratum 2.1.1.128; MMC read performance may suffer\n"); >> > - mmc->caps2 |= MMC_CAP2_NO_MULTI_READ; >> >> An option to adding yet another flag, would be to assign >> ->multi_io_quirk = omap_hsmmc_multi_io_quirk() here? Isn't that >> actually better for those variants that doesn't suffer from this HW >> bug? > > This means we need to copy this ops for each driver ? > Otherwise, it breaks behavior if many drivers were probed. The are only one driver for omap_hsmmc, this is not tmio, which certainly is a different story. What you need to do, is to make omap_hsmmc_ops non const - and assign the function pointer here. That should work. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf > > This means we need to copy this ops for each driver ? > > Otherwise, it breaks behavior if many drivers were probed. > > The are only one driver for omap_hsmmc, this is not tmio, which > certainly is a different story. > > What you need to do, is to make omap_hsmmc_ops non const - and assign > the function pointer here. That should work. I meant this case omap_hsmmc@0 0xAAAAAA irq = xxxx (MULTI read workaround is needed) omap_hsmmc@1 0xBBBBBB irq = yyyy (MULTI read workaround is not needed) Maybe current omap doesn't have 2 hsmmc on 1 SoC (I don't know) but, we don't know the future ? Of course I can follow your style if this issue never happen Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9 September 2014 08:07, Kuninori Morimoto <kuninori.morimoto.gx@gmail.com> wrote: > > Hi Ulf > >> > This means we need to copy this ops for each driver ? >> > Otherwise, it breaks behavior if many drivers were probed. >> >> The are only one driver for omap_hsmmc, this is not tmio, which >> certainly is a different story. >> >> What you need to do, is to make omap_hsmmc_ops non const - and assign >> the function pointer here. That should work. > > I meant this case > > omap_hsmmc@0 0xAAAAAA irq = xxxx (MULTI read workaround is needed) > omap_hsmmc@1 0xBBBBBB irq = yyyy (MULTI read workaround is not needed) > > Maybe current omap doesn't have 2 hsmmc on 1 SoC (I don't know) > but, we don't know the future ? > Of course I can follow your style if this issue never happen The above can be handled by having different omap variants structs, like how mmci does it. Also, I don't think we need to worry about that now. So please adopt to my proposal. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf > > omap_hsmmc@0 0xAAAAAA irq = xxxx (MULTI read workaround is needed) > > omap_hsmmc@1 0xBBBBBB irq = yyyy (MULTI read workaround is not needed) > > > > Maybe current omap doesn't have 2 hsmmc on 1 SoC (I don't know) > > but, we don't know the future ? > > Of course I can follow your style if this issue never happen > > The above can be handled by having different omap variants structs, > like how mmci does it. Also, I don't think we need to worry about that > now. So please adopt to my proposal. OK, I send patch soon Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index ece1634..7b41f57 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -219,6 +219,7 @@ struct omap_hsmmc_host { #define AUTO_CMD23 (1 << 0) /* Auto CMD23 support */ #define HSMMC_SDIO_IRQ_ENABLED (1 << 1) /* SDIO irq enabled */ #define HSMMC_WAKE_IRQ_ENABLED (1 << 2) +#define HSMMC_NO_MULTI_READ (1 << 3) struct omap_hsmmc_next next_data; struct omap_mmc_platform_data *pdata; }; @@ -1829,6 +1830,19 @@ static int omap_hsmmc_disable_fclk(struct mmc_host *mmc) return 0; } +static int *omap_hsmmc_multi_io_quirk(struct mmc_card *card, + unsigned int direction, int blk_size) +{ + struct omap_hsmmc_host *host = mmc_priv(card->host); + + /* This controller can't do multiblock reads due to hw bugs */ + if ((host->flags & HSMMC_NO_MULTI_READ) && + (direction == MMC_DATA_READ)) + return 1; + + return blk_size; +} + static const struct mmc_host_ops omap_hsmmc_ops = { .enable = omap_hsmmc_enable_fclk, .disable = omap_hsmmc_disable_fclk, @@ -1840,6 +1854,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = { .get_ro = omap_hsmmc_get_ro, .init_card = omap_hsmmc_init_card, .enable_sdio_irq = omap_hsmmc_enable_sdio_irq, + .multi_io_quirk = omap_hsmmc_multi_io_quirk, }; #ifdef CONFIG_DEBUG_FS @@ -2101,7 +2116,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev) if (host->pdata->controller_flags & OMAP_HSMMC_BROKEN_MULTIBLOCK_READ) { dev_info(&pdev->dev, "multiblock reads disabled due to 35xx erratum 2.1.1.128; MMC read performance may suffer\n"); - mmc->caps2 |= MMC_CAP2_NO_MULTI_READ; + host->flags |= HSMMC_NO_MULTI_READ; } pm_runtime_enable(host->dev);