Hi Sergei, On Tuesday 29 October 2013 23:47:36 Sergei Shtylyov wrote: > On 10/29/2013 04:15 PM, Laurent Pinchart wrote: > >>>>> Renesas ARM platforms are transitioning from single-platform to > >>>>> multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the > >>>>> driver available on all ARM platforms to enable it on both > >>>>> ARCH_SHMOBILE and ARCH_SHMOBILE_MULTI and increase build testing > >>>>> coverage. > >>>>> > >>>>> Cc: Chris Ball <cjb@xxxxxxxxxx> > >>>>> Cc: Guennadi Liakhovetski <g.liakhovetski@xxxxxx> > >>>>> Cc: Ian Molton <ian@xxxxxxxxxxxxxx> > >>>>> Cc: linux-mmc@xxxxxxxxxxxxxxx > >>>>> Signed-off-by: Laurent Pinchart > >>>>> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > >> > >> [...] > >> > >>>>> diff --git a/drivers/mmc/host/tmio_mmc_dma.c > >>>>> b/drivers/mmc/host/tmio_mmc_dma.c index 65edb4a..535bc35 100644 > >>>>> --- a/drivers/mmc/host/tmio_mmc_dma.c > >>>>> +++ b/drivers/mmc/host/tmio_mmc_dma.c > >>>>> @@ -28,7 +28,7 @@ void tmio_mmc_enable_dma(struct tmio_mmc_host *host, > >>>>> bool enable)> > >>>>> > >>>>> if (!host->chan_tx || !host->chan_rx) > >>>>> > >>>>> return; > >>>>> > >>>>> -#if defined(CONFIG_SUPERH) || defined(CONFIG_ARCH_SHMOBILE) > >>>>> +#if defined(CONFIG_SUPERH) || defined(CONFIG_ARM) > >>>> > >>>> I'm not sure about this one. In principle DMA so far is only used on > >>>> SDHI with TMIO. But this #if was for a theoretical case, when a TMIO > >>>> MMC IP inside an MFD is also used with DMA. Bus since I had no > >>>> indormation about whether the CTL_DMA_ENABLE register was SDHI specific > >>>> or not, I put it under an #if for the time being. In any case, I don't > >>>> think making it #ifdef CONFIG_ARM makes much sense. We can either > >>>> remove the #if completely, or keep it to limit the scope of this write > >>>> to sh-mobile arches. > >>> > >>> SUPERH || ARCH_SHMOBILE covers all the platforms this driver currently > >>> builds on. Now that we're adding ARCH_SHMOBILE_MULTI the #if needs to > >>> cover that case as well. I'm fine with limiting the CTL_DMA_ENABLE write > >>> to SUPERH only (which would modify the driver's behaviour) > >> > >> I'm afraid that would break the driver's ability to work in DMA mode on > >> SH-Mobile SoCs. > > > > So can you confirm that writing the CTL_DMA_ENABLE register is required on > > all Renesas ARM SoCs (including SH-Mobile, R-Mobile and R-Car) ? > > I can't actually say anything about all SH-Mobile and R-Mobile SoCs, only > about the R-Car SoCs: though the datasheets don't contain the SDHI register > info, from my experience the register exists on R8A777x (I had to fix up the > broken PIO fallback in the SDHI driver). OK. > >>> or enabling it unconditionally, but I don't think adding a > >>> defined(CONFIG_ARCH_SHMOBILE_MULTI) to the above #if makes sense, unless > >>> there's a plan to add support for DMA for non-SH (including both SUPERH > >>> and ARCH_SHMOBILE) platforms. > >> > >> Not only the plan -- I have already posted the patches to do it for > >> R8A777x, and Simon has queued them for 3.13. > > > > Could you please point me to those patches ? > > https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/commit/?h=nex > t&id=1eb6b5a0e55bfcfb0852b7d0f9442841ff807345 > https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/commit/?h=ne > xt&id=a43e5bd76a4a3df58167d85e8020a1c9e566ad75 > https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/commit/?h=ne > xt&id=5d6aa3435275a5308684f90c17424b055ef7f572 > https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/commit/?h=ne > xt&id=e6a8b11b82fdeaa78dad52552f945b772ee1a5c9 But that's still an ARCH_SHMOBILE platform. Guennadi's point, if I understood it correctly, was that whether the CTL_DMA_ENABLE register is part of the standard TMIO controller, or is Renesas-specific, is unknown. As we have no current or planned TMIO DMA users other than SUPERH and ARCH_SHMOBILE this is impossible to test. That is why the code is conditionally compiled. We could either get rid of the #if completely and always write to the register, or add ARCH_SHMOBILE_MULTI to the condition. I now agree that adding ARM as a dependency makes no sense. Given that this will break on multiplatform kernels anyway, I'm enclined to write to the CTL_DMA_ENABLE register unconditionally. I'll update the patch accordingly. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
