[PATCH v3 5/7] ARM: shmobile: r8a7740: add DT nodes for three DMAC instance

[Magnus Damm]

Hi Guennadi,

On Wed, Jul 3, 2013 at 3:23 PM, Guennadi Liakhovetski
<g.liakhovetski at gmx.de> wrote:
> Hi Magnus
>
> On Wed, 3 Jul 2013, Magnus Damm wrote:
>
>> Hi Guennadi,
>>
>> On Wed, Jun 26, 2013 at 11:40 PM, Guennadi Liakhovetski
>> <g.liakhovetski at gmx.de> wrote:
>> > This patch adds Device Tree support for the three DMA controller instances
>> > on r8a7740 in a DMA multiplexer node.
>> >
>> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas at gmail.com>
>> > ---
>> >  arch/arm/boot/dts/r8a7740.dtsi         |   62 ++++++++++++++++++++++++++++++++
>> >  arch/arm/mach-shmobile/setup-r8a7740.c |   12 ++++++
>> >  2 files changed, 74 insertions(+), 0 deletions(-)
>>
>> Thanks for your work on this.
>>
>> > --- a/arch/arm/mach-shmobile/setup-r8a7740.c
>> > +++ b/arch/arm/mach-shmobile/setup-r8a7740.c
>> > @@ -996,7 +996,19 @@ void __init r8a7740_add_early_devices(void)
>> >
>> >  #ifdef CONFIG_USE_OF
>> >
>> > +static struct of_dev_auxdata r8a7740_dmac_auxdata[] = {
>> > +       OF_DEV_AUXDATA("renesas,shdma", 0xfe008020, "sh-dma-engine.0",
>> > +                      &dma_platform_data),
>> > +       OF_DEV_AUXDATA("renesas,shdma", 0xfe018020, "sh-dma-engine.1",
>> > +                      &dma_platform_data),
>> > +       OF_DEV_AUXDATA("renesas,shdma", 0xfe028020, "sh-dma-engine.2",
>> > +                      &dma_platform_data),
>> > +       { }
>> > +};
>> > +
>> >  static const struct of_dev_auxdata r8a7740_auxdata_lookup[] __initconst = {
>> > +       OF_DEV_AUXDATA("renesas,shdma-mux", 0, "shdma-of.0",
>> > +                      r8a7740_dmac_auxdata),
>> >         { }
>> >  };
>>
>> Uhm, what is the reason for adding AUXDATA? For all other cases we
>> have clearly separated the DT reference bits from the C version
>> without using AUXDATA. With that approach we can use the default NULL
>> callbacks for ->init_machine(). Now with this patch we're going in the
>> totally different direction...
>>
>> Why can't you use DT only for these?
>
> Short answer - because that's how the SH DMA driver, currently in the
> "next" tree ready for Linus to be pulled, is implemented. We had enough
> time to discuss this back then, I think, it is a pity this question is
> raised only now.

Yes, indeed, in the same way it is a pity that this turns out to be
the only user of AUXDATA.

> A while ago I read, that it is ok to pass SoC-specific device properties
> via auxdata, as opposed to board-specific ones, that have to go via DT. On
> shmobile this is also easy to implement - all devices, initialised from
> setup-<soc>.c, i.e. having no board-specific configuration, and defined in
> <soc>.dtsi can just take their configuration from the same platform data,
> as in .c case. Another motivation for this is, that we want to get rid of
> board-<name>.c files, but we'll always have setup-<soc>.c files. With that
> in mind I implemented DMAC DT bindings and that also allowed us to so
> quickly get them accepted into the mainline. If we needed to initialise
> DMAC completely from DT, mainlining would certainly take much longer.

Neither DMAC nor DT have ever been rushing, so I'm not sure why you
are doing that now. Both those are about correctness. We are fine
without DMAC and without DT for a majority of the cases. So let's get
it right instead of half-right.

Sorry to say this, but from my perspective the AUXDATA solution above
seems to be a rushed short term solution to a more long term problem.
Actually, I'm not sure how you ever came to think that was a good
idea.

Please show how you plan on going from AUXDATA to DT only. I am fine
with something similar to the PFC implementation that has SoC-specifc
bits written in C outside arch/arm, but I'd rather not use AUXDATA for
future code.

Thanks,

/ magnus