| Message ID | 1372257656-9944-6-git-send-email-g.liakhovetski@gmx.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Guennadi, On Wed, Jun 26, 2013 at 11:40 PM, Guennadi Liakhovetski <g.liakhovetski@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@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? Cheers, / magnus -- 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 Magnus On Wed, 3 Jul 2013, Magnus Damm wrote: > Hi Guennadi, > > On Wed, Jun 26, 2013 at 11:40 PM, Guennadi Liakhovetski > <g.liakhovetski@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@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. 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. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 Guennadi, On Wed, Jul 3, 2013 at 3:23 PM, Guennadi Liakhovetski <g.liakhovetski@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@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@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 -- 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/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi index 25dc930..683751c 100644 --- a/arch/arm/boot/dts/r8a7740.dtsi +++ b/arch/arm/boot/dts/r8a7740.dtsi @@ -112,6 +112,68 @@ 0 149 0x4>; }; + dmac: dma-mux0 { + compatible = "renesas,shdma-mux"; + #dma-cells = <1>; + dma-channels = <6>; + dma-requests = <256>; + reg = <0 0>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + dma0: shdma@fe008020 { + compatible = "renesas,shdma"; + reg = <0xfe008020 0x270>, + <0xfe009000 0xc>; + interrupt-parent = <&gic>; + interrupts = <0 34 4 + 0 28 4 + 0 29 4 + 0 30 4 + 0 31 4 + 0 32 4 + 0 33 4>; + interrupt-names = "error", + "ch0", "ch1", "ch2", "ch3", + "ch4", "ch5"; + }; + + dma1: shdma@fe018020 { + compatible = "renesas,shdma"; + reg = <0xfe018020 0x270>, + <0xfe019000 0xc>; + interrupt-parent = <&gic>; + interrupts = <0 41 4 + 0 35 4 + 0 36 4 + 0 37 4 + 0 38 4 + 0 39 4 + 0 40 4>; + interrupt-names = "error", + "ch0", "ch1", "ch2", "ch3", + "ch4", "ch5"; + }; + + dma2: shdma@fe028020 { + compatible = "renesas,shdma"; + reg = <0xfe028020 0x270>, + <0xfe029000 0xc>; + interrupt-parent = <&gic>; + interrupts = <0 48 4 + 0 42 4 + 0 43 4 + 0 44 4 + 0 45 4 + 0 46 4 + 0 47 4>; + interrupt-names = "error", + "ch0", "ch1", "ch2", "ch3", + "ch4", "ch5"; + }; + }; + i2c0: i2c@fff20000 { #address-cells = <1>; #size-cells = <0>; diff --git a/arch/arm/mach-shmobile/setup-r8a7740.c b/arch/arm/mach-shmobile/setup-r8a7740.c index 6b3ed42..de55161 100644 --- 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), { } };
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@gmail.com> --- arch/arm/boot/dts/r8a7740.dtsi | 62 ++++++++++++++++++++++++++++++++ arch/arm/mach-shmobile/setup-r8a7740.c | 12 ++++++ 2 files changed, 74 insertions(+), 0 deletions(-)