| Message ID | 1511540697-27387-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| Headers | show |
| Series | mmc: tmio: various fixes and cleanups | expand |
On Sat, Nov 25, 2017 at 01:24:48AM +0900, Masahiro Yamada wrote: > struct tmio_mmc_host has "dma_dataend" and "dma_complete", but in fact, > they are Renesas private data. Move them to renesas_sdhi.h > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> +static int tmio_mmc_get_cd(struct mmc_host *mmc) > +{ > + struct tmio_mmc_host *host = mmc_priv(mmc); > + int ret; > + > + ret = mmc_gpio_get_cd(mmc); > + if (ret >= 0) > + return ret; > + > + return !!(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) & > + TMIO_STAT_SIGSTATE); > +} I wonder if we shouldn't do something like: if (mmc_can_gpio_cd()) return mmc_gpio_get_cd() else return !!(sd_ctrl_read16_and_16_as_32...) If we have a GPIO CD defined, I think we want the value of mmc_gpio_get_cd() in all cases. It makes clearer that this is an 'either-or' case and not a fallback mechanism.
On Sat, Nov 25, 2017 at 01:24:44AM +0900, Masahiro Yamada wrote: > To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set > by a legacy board (arch/sh/boards/mach-ecovec24). > > For DT platforms, the "cd-gpios" property is a legitimate way for that > in case the IP-builtin card detection can not be used for some reason. > mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if > the "cd-gpios" property is specified. > > To cater to both cases, mmc_can_gpio_cd() is a correct way to check > which card detection logic is used. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> My gut feeling is that your patch is correct, but I need to have another look at this native_hotplug code with a fresh brain and take your other patches into account as well then, too.
On 24 November 2017 at 17:24, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > I am working on this IP for Socionext SoCs. > > I was hit by several issues, and noticed various > clean-up candidates. > > - Fix and clean-up Kconfig > - Fix various card detection problems > - Move Renesas private data out of TMIO core > - Allow to perform platform-specific settings before MMC host starts > - Fix weird IRQ handling > > I am getting more and more patches for TMIO. > I put all in a single series to clarify the patch order. > > 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang. > > > Masahiro Yamada (22): > mmc: renesas_sdhi: consolidate DMAC CONFIG options > mmc: renesas_sdhi: remove wrong depends on to enable compile test > mmc: renesas_sdhi: remove eprobe jump label > mmc: tmio: set tmio_mmc_host to driver data > mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap() > mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data > mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly > mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host() > mmc: tmio: use mmc_can_gpio_cd() instead of checking > TMIO_MMC_USE_GPIO_CD > mmc: tmio: support IP-builtin card detection logic > mmc: renesas_sdhi: remove always false condition > mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h > mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to > renesas_sdhi.h > mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h > mmc: tmio: change bus_shift to unsigned int > mmc: tmio: fix never-detected card insertion bug > mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place > mmc: tmio: remove useless TMIO_MASK_CMD handling in > tmio_mmc_host_probe() > mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc() > mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe() > mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc() > mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument > > drivers/mmc/host/Kconfig | 5 +- > drivers/mmc/host/Makefile | 8 +- > drivers/mmc/host/renesas_sdhi.h | 22 ++++ > drivers/mmc/host/renesas_sdhi_core.c | 49 ++++----- > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 14 ++- > drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++--- > drivers/mmc/host/tmio_mmc.c | 23 ++-- > drivers/mmc/host/tmio_mmc.h | 23 +--- > drivers/mmc/host/tmio_mmc_core.c | 149 +++++++++++++------------- > 9 files changed, 170 insertions(+), 158 deletions(-) > > -- > 2.7.4 > To get this moving, I have applied patch 1->8 for next, thanks! Kind regards Uffe
2017-12-15 18:18 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>: > On 24 November 2017 at 17:24, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> >> I am working on this IP for Socionext SoCs. >> >> I was hit by several issues, and noticed various >> clean-up candidates. >> >> - Fix and clean-up Kconfig >> - Fix various card detection problems >> - Move Renesas private data out of TMIO core >> - Allow to perform platform-specific settings before MMC host starts >> - Fix weird IRQ handling >> >> I am getting more and more patches for TMIO. >> I put all in a single series to clarify the patch order. >> >> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang. >> >> >> Masahiro Yamada (22): >> mmc: renesas_sdhi: consolidate DMAC CONFIG options >> mmc: renesas_sdhi: remove wrong depends on to enable compile test >> mmc: renesas_sdhi: remove eprobe jump label >> mmc: tmio: set tmio_mmc_host to driver data >> mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap() >> mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data >> mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly >> mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host() >> mmc: tmio: use mmc_can_gpio_cd() instead of checking >> TMIO_MMC_USE_GPIO_CD >> mmc: tmio: support IP-builtin card detection logic >> mmc: renesas_sdhi: remove always false condition >> mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h >> mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to >> renesas_sdhi.h >> mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h >> mmc: tmio: change bus_shift to unsigned int >> mmc: tmio: fix never-detected card insertion bug >> mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place >> mmc: tmio: remove useless TMIO_MASK_CMD handling in >> tmio_mmc_host_probe() >> mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc() >> mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe() >> mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc() >> mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument >> >> drivers/mmc/host/Kconfig | 5 +- >> drivers/mmc/host/Makefile | 8 +- >> drivers/mmc/host/renesas_sdhi.h | 22 ++++ >> drivers/mmc/host/renesas_sdhi_core.c | 49 ++++----- >> drivers/mmc/host/renesas_sdhi_internal_dmac.c | 14 ++- >> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++--- >> drivers/mmc/host/tmio_mmc.c | 23 ++-- >> drivers/mmc/host/tmio_mmc.h | 23 +--- >> drivers/mmc/host/tmio_mmc_core.c | 149 +++++++++++++------------- >> 9 files changed, 170 insertions(+), 158 deletions(-) >> >> -- >> 2.7.4 >> > > To get this moving, I have applied patch 1->8 for next, thanks! > > Kind regards > Uffe After 2, COMPILE_TEST will work correctly. Then, Wolfram mentioned we would need to include <linux/io.h> from tmio_mmc.h https://patchwork.kernel.org/patch/10074333/ I was waiting for a patch from him. -- Best Regards Masahiro Yamada
On 15 December 2017 at 17:30, Wolfram Sang <wsa@the-dreams.de> wrote: > >> > Ulf, this patch then in deed should ideally be applied before 1-8 here. >> >> Okay, once you post it to linux-mmc I will pick it up, and put it in front. > > Bad news, that patch didn't help. The problem is that sparc64 doesn't > include 'asm-generic/io.h' and also has no own 'readsw'. No surprise > that just adding this include will cause lots of redefines which are way > too much to handle as a side-task. > > So, the best option I see here is to drop COMPILE_TEST for now, report > this to the sparc64 maintainers (I assume they know already), and see if > they can fix it. Okay! If some of you send a patch on top, I can fold it into the offending commit. > > Other thoughts? Nope. Kind regards Uffe
On Sat, Nov 25, 2017 at 01:24:45AM +0900, Masahiro Yamada wrote: > A card detect GPIO is set up only for platforms with "cd-gpios" > DT property or TMIO_MMC_USE_GPIO_CD flag. However, the driver > core always uses mmc_gpio_get_cd, which just fails with -ENOSYS > if ctx->cd_gpio is unset. > > The bit 5 of the status register provides the current signal level > of the CD line. Allow to use it if the GPIO is unused. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> As mentioned before (sorry, I lost this thread :( ), I like the refactoring to select in probe() which function to call depending on GPIO usage or not. If you like, we can do the same for read_only, too. Thanks!
On 24 November 2017 at 17:24, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set > by a legacy board (arch/sh/boards/mach-ecovec24). > > For DT platforms, the "cd-gpios" property is a legitimate way for that > in case the IP-builtin card detection can not be used for some reason. > mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if > the "cd-gpios" property is specified. > > To cater to both cases, mmc_can_gpio_cd() is a correct way to check > which card detection logic is used. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Thanks, applied for next! For the rest of the series, please re-post those changes. Kind regards Uffe > --- > > Changes in v2: None > > drivers/mmc/host/tmio_mmc_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c > index efffb04..610f26f 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -1232,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host, > } > mmc->max_seg_size = mmc->max_req_size; > > - _host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD || > + _host->native_hotplug = !(mmc_can_gpio_cd(mmc) || > mmc->caps & MMC_CAP_NEEDS_POLL || > !mmc_card_is_removable(mmc)); > > -- > 2.7.4 >
On Sat, Nov 25, 2017 at 01:24:57AM +0900, Masahiro Yamada wrote: > Drivers need to set up various struct members for tmio_mmc_host before > calling tmio_mmc_host_probe(). Do likewise for host->dma_ops instead > of passing it as a function argument. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> I think I have done all the review by now. I'll do some more regression testing tomorrow and then give Tested-by as well if everything works fine (but it looks like it so far).
I am working on this IP for Socionext SoCs. I was hit by several issues, and noticed various clean-up candidates. - Fix and clean-up Kconfig - Fix various card detection problems - Move Renesas private data out of TMIO core - Allow to perform platform-specific settings before MMC host starts - Fix weird IRQ handling I am getting more and more patches for TMIO. I put all in a single series to clarify the patch order. 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang. Masahiro Yamada (22): mmc: renesas_sdhi: consolidate DMAC CONFIG options mmc: renesas_sdhi: remove wrong depends on to enable compile test mmc: renesas_sdhi: remove eprobe jump label mmc: tmio: set tmio_mmc_host to driver data mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap() mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host() mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD mmc: tmio: support IP-builtin card detection logic mmc: renesas_sdhi: remove always false condition mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to renesas_sdhi.h mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h mmc: tmio: change bus_shift to unsigned int mmc: tmio: fix never-detected card insertion bug mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place mmc: tmio: remove useless TMIO_MASK_CMD handling in tmio_mmc_host_probe() mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc() mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe() mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc() mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument drivers/mmc/host/Kconfig | 5 +- drivers/mmc/host/Makefile | 8 +- drivers/mmc/host/renesas_sdhi.h | 22 ++++ drivers/mmc/host/renesas_sdhi_core.c | 49 ++++----- drivers/mmc/host/renesas_sdhi_internal_dmac.c | 14 ++- drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++--- drivers/mmc/host/tmio_mmc.c | 23 ++-- drivers/mmc/host/tmio_mmc.h | 23 +--- drivers/mmc/host/tmio_mmc_core.c | 149 +++++++++++++------------- 9 files changed, 170 insertions(+), 158 deletions(-) -- 2.7.4