Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 19 December 2017 at 04:56, Masahiro Yamada
 wrote:
> Hi Ulf,
>
>
> 2017-12-15 18:18 GMT+09:00 Ulf Hansson :
>> On 24 November 2017 at 17:24, Masahiro Yamada
>>  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!
>
>
> Could you apply 11->15 as well?
> They were reviewed by Wolfram.

Applied for next!

>
> We can skip 9, 10.

Okay, thought there were a dependency.

Thanks and kind regards
Uffe

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

Hi Ulf,


2017-12-15 18:18 GMT+09:00 Ulf Hansson :
> On 24 November 2017 at 17:24, Masahiro Yamada
>  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!


Could you apply 11->15 as well?
They were reviewed by Wolfram.

We can skip 9, 10.





-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

> > Other thoughts?
> 
> Nope.

I have :) Another solution seems to be to switch to ioread16_rep and
friends. Those are defined on sparc64. Compiles locally here, now sent
it to buildbot.



signature.asc
Description: PGP signature

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

> > 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.

Other thoughts?



signature.asc
Description: PGP signature

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 15 December 2017 at 17:30, Wolfram Sang  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

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 15 December 2017 at 12:12, Wolfram Sang  wrote:
>
>> After 2, COMPILE_TEST will work correctly.
>>
>> Then, Wolfram mentioned we would need to include  from tmio_mmc.h
>>
>> https://patchwork.kernel.org/patch/10074333/
>>
>>
>> I was waiting for a patch from him.
>
> Yes, I am sorry. I am currently swamped with I2C work, not so much time
> for SDHI. However, I did this patch now and pushed the branch out to see
> if buildbot is happy.

Great!

>
> 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.

Kind regards
Uffe

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

> After 2, COMPILE_TEST will work correctly.
> 
> Then, Wolfram mentioned we would need to include  from tmio_mmc.h
> 
> https://patchwork.kernel.org/patch/10074333/
> 
> 
> I was waiting for a patch from him.

Yes, I am sorry. I am currently swamped with I2C work, not so much time
for SDHI. However, I did this patch now and pushed the branch out to see
if buildbot is happy.

Ulf, this patch then in deed should ideally be applied before 1-8 here.



signature.asc
Description: PGP signature

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

2017-12-15 18:18 GMT+09:00 Ulf Hansson :
> On 24 November 2017 at 17:24, Masahiro Yamada
>  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  from tmio_mmc.h

https://patchwork.kernel.org/patch/10074333/


I was waiting for a patch from him.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 24 November 2017 at 17:24, Masahiro Yamada
 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

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

Hi Wolfram,


2017-11-28 2:13 GMT+09:00 Wolfram Sang :
>
>> I am getting more and more patches for TMIO.
>> I put all in a single series to clarify the patch order.
>
> Hmmm, that makes reviewing harder in my opinion.


I think this depends on people.

You are the maintainer of TMIO MMC, so
I will try my best to adjust to your opinion next time.



> I really liked the small serieses which could be improved, acked, and
> applied. This big one will take much longer to review and resending will
> cause quite some noise.
>
> For clarifying patch order in small series, it should be enough to state
> the dependencies in the cover letter. Super helpful is providing a
> public git branch where ppl can pull from.
>
> Also, can you state what kind of testing you did on what platforms?

I have no Renesas boards.

I tested this series on UniPhier boards.
(arch/arm/boot/dts/uniphier*.dts,  arch/arm64/boot/dts/socionext/uniphier*.dts)


> My
> gut feeling says that the card detect changes might be easy to catch
> regressions on old platforms, so we'd need to be extra cautious. I/we
> can do testing on some Renesas platforms, too.
>
> Other changes like moving stuff from tmio_core to renesas_sdhi is less
> critical. Either it works or not, so that is easy to spot :)
>
> Which might be another argument for multiple smaller patch series.
>
> Nonetheless, I'll try to get this reviewed in the next days...

Thanks!


> All the best,
>
>Wolfram
>
>
-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

> I am getting more and more patches for TMIO.
> I put all in a single series to clarify the patch order.

Hmmm, that makes reviewing harder in my opinion.

I really liked the small serieses which could be improved, acked, and
applied. This big one will take much longer to review and resending will
cause quite some noise.

For clarifying patch order in small series, it should be enough to state
the dependencies in the cover letter. Super helpful is providing a
public git branch where ppl can pull from.

Also, can you state what kind of testing you did on what platforms? My
gut feeling says that the card detect changes might be easy to catch
regressions on old platforms, so we'd need to be extra cautious. I/we
can do testing on some Renesas platforms, too.

Other changes like moving stuff from tmio_core to renesas_sdhi is less
critical. Either it works or not, so that is easy to spot :)

Which might be another argument for multiple smaller patch series.

Nonetheless, I'll try to get this reviewed in the next days...

All the best,

   Wolfram


> 
> 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
> 


signature.asc
Description: PGP signature