| Message ID | 1344527268-5964-14-git-send-email-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote: > We've done this before and it worked well last time. Here we're > duplicating a complex registration function to ease the process > of enabling it for Device Tree. As there are quite a few steps > taken during the registration process, it makes sense to break > them up into more manageable chunks. This patch will aid us. > > Signed-off-by: Lee Jones <lee.jones@linaro.org> I understand you have used this approach before so: Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Aug 14, 2012 at 01:13:49PM +0200, Linus Walleij wrote: > On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote: > > > We've done this before and it worked well last time. Here we're > > duplicating a complex registration function to ease the process > > of enabling it for Device Tree. As there are quite a few steps > > taken during the registration process, it makes sense to break > > them up into more manageable chunks. This patch will aid us. > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > I understand you have used this approach before so: > Acked-by: Linus Walleij <linus.walleij@linaro.org> Does this comment take back your previous one: NOTE: it seems this patch set contains some churn. First you add in the forked device init, put in a big chunk of code and then in the *same* patch set delete it again. It's not like we're dying to see all the development history... can this be squashed down a bit? ... hence leave the patch-set as it is?
On Mon, Aug 20, 2012 at 1:06 AM, Lee Jones <lee.jones@linaro.org> wrote: > On Tue, Aug 14, 2012 at 01:13:49PM +0200, Linus Walleij wrote: >> On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote: >> >> > We've done this before and it worked well last time. Here we're >> > duplicating a complex registration function to ease the process >> > of enabling it for Device Tree. As there are quite a few steps >> > taken during the registration process, it makes sense to break >> > them up into more manageable chunks. This patch will aid us. >> > >> > Signed-off-by: Lee Jones <lee.jones@linaro.org> >> >> I understand you have used this approach before so: >> Acked-by: Linus Walleij <linus.walleij@linaro.org> > > Does this comment take back your previous one: > > NOTE: it seems this patch set contains some churn. First you > add in the forked device init, put in a big chunk of code and > then in the *same* patch set delete it again. It's not like > we're dying to see all the development history... can this > be squashed down a bit? > > ... hence leave the patch-set as it is? No. I just meant leave it like that for the devices outside of this set. If you're adding and then removing *all* of them in this set, why add them in the first place? Yours, Linus Walleij
On Mon, Aug 27, 2012 at 04:07:58PM -0700, Linus Walleij wrote: > On Mon, Aug 20, 2012 at 1:06 AM, Lee Jones <lee.jones@linaro.org> wrote: > > On Tue, Aug 14, 2012 at 01:13:49PM +0200, Linus Walleij wrote: > >> On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> > >> > We've done this before and it worked well last time. Here we're > >> > duplicating a complex registration function to ease the process > >> > of enabling it for Device Tree. As there are quite a few steps > >> > taken during the registration process, it makes sense to break > >> > them up into more manageable chunks. This patch will aid us. > >> > > >> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > >> > >> I understand you have used this approach before so: > >> Acked-by: Linus Walleij <linus.walleij@linaro.org> > > > > Does this comment take back your previous one: > > > > NOTE: it seems this patch set contains some churn. First you > > add in the forked device init, put in a big chunk of code and > > then in the *same* patch set delete it again. It's not like > > we're dying to see all the development history... can this > > be squashed down a bit? > > > > ... hence leave the patch-set as it is? > > No. I just meant leave it like that for the devices outside of this > set. > > If you're adding and then removing *all* of them in this set, > why add them in the first place? So that there's no breakage during bisection. You should be able to roll the kernel back in between each of these patches and there to be full compatibility at each point. At least that was the intention. Is that wrong?
On Tue, Aug 28, 2012 at 12:48 AM, Lee Jones <lee.jones@linaro.org> wrote: > On Mon, Aug 27, 2012 at 04:07:58PM -0700, Linus Walleij wrote: >> If you're adding and then removing *all* of them in this set, >> why add them in the first place? > > So that there's no breakage during bisection. > > You should be able to roll the kernel back in between each of these > patches and there to be full compatibility at each point. At least > that was the intention. Is that wrong? No it's correct, the only way to do what I'm thinking on may be to squash them all into one gigantic patch, which is not good either. So go ahead with this scheme, it's the lesser of two evils. Acked-by etc. Yours, Linus Walleij
diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c index 1a67577..24a3604 100644 --- a/arch/arm/mach-ux500/board-mop500-msp.c +++ b/arch/arm/mach-ux500/board-mop500-msp.c @@ -172,6 +172,26 @@ static struct msp_i2s_platform_data msp3_platform_data = { .msp_i2s_dma_tx = NULL, }; +/* Due for removal once the MSP driver has been fully DT:ed. */ +void mop500_of_msp_init(struct device *parent) +{ + pr_info("%s: Register platform-device 'snd-soc-u8500'.\n", __func__); + platform_device_register(&snd_soc_mop500); + + pr_info("Initialize MSP I2S-devices.\n"); + db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0, + &msp0_platform_data); + db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1, + &msp1_platform_data); + db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2, + &msp2_platform_data); + db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1, + &msp3_platform_data); + + pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__); + platform_device_register(&ux500_pcm); +} + void mop500_msp_init(struct device *parent) { pr_info("%s: Register platform-device 'snd-soc-mop500'.\n", __func__); diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index 11dd962..8f419b1 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -807,7 +807,7 @@ static void __init u8500_init_machine(void) mop500_uib_init(); } else if (of_machine_is_compatible("calaosystems,snowball-a9500")) { - mop500_msp_init(parent); + mop500_of_msp_init(parent); } else if (of_machine_is_compatible("st-ericsson,hrefv60+")) { /* * The HREFv60 board removed a GPIO expander and routed
We've done this before and it worked well last time. Here we're duplicating a complex registration function to ease the process of enabling it for Device Tree. As there are quite a few steps taken during the registration process, it makes sense to break them up into more manageable chunks. This patch will aid us. Signed-off-by: Lee Jones <lee.jones@linaro.org> --- arch/arm/mach-ux500/board-mop500-msp.c | 20 ++++++++++++++++++++ arch/arm/mach-ux500/board-mop500.c | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-)