| Message ID | 1360184596-1603-3-git-send-email-jon-hunter@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/06/2013 03:03 PM, Jon Hunter wrote: > If the device-tree blob is present during boot, then register the SDMA > controller with the device-tree DMA driver so that we can use device-tree > to look-up DMA client information. > > Signed-off-by: Jon Hunter <jon-hunter@ti.com> > --- > drivers/dma/omap-dma.c | 31 ++++++++++++++++++++++++++++++- > 1 file changed, 30 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c > index 5a31264..a32d81b 100644 > --- a/drivers/dma/omap-dma.c > +++ b/drivers/dma/omap-dma.c > @@ -16,6 +16,8 @@ > #include <linux/platform_device.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > +#include <linux/of_dma.h> > +#include <linux/of_device.h> > > #include "virt-dma.h" > > @@ -67,6 +69,8 @@ static const unsigned es_bytes[] = { > [OMAP_DMA_DATA_TYPE_S32] = 4, > }; > > +static struct of_dma_filter_info info; > + > static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d) > { > return container_of(d, struct omap_dmadev, ddev); > @@ -621,10 +625,25 @@ static int omap_dma_probe(struct platform_device *pdev) > pr_warn("OMAP-DMA: failed to register slave DMA engine device: %d\n", > rc); > omap_dma_free(od); > + return rc; > } else { > platform_set_drvdata(pdev, od); > } I realise now that I could get rid of the else here and just call platform_set_drvdata(), if we don't return. Anyway, I will wait for other comments before changing. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 06 February 2013, Jon Hunter wrote: > +static struct of_dma_filter_info info; Both members of this structure are constant, so you can just initialize it here, and it would be nice to give it a more descriptive name, such as omap_dmadev_info. > static struct platform_driver omap_dma_driver = { > .probe = omap_dma_probe, > .remove = omap_dma_remove, > .driver = { > .name = "omap-dma-engine", > .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(omap_dma_match) > }, > }; please end the new line with a comma. > @@ -673,7 +702,7 @@ static int omap_dma_init(void) > { > int rc = platform_driver_register(&omap_dma_driver); > > - if (rc == 0) { > + if ((rc == 0) && (!of_have_populated_dt())) { > pdev = platform_device_register_full(&omap_dma_dev_info); > if (IS_ERR(pdev)) { > platform_driver_unregister(&omap_dma_driver); There is already a patch in linux-next that makes this obsolete. The device is now registered in arch/arm/mach-omap2/dma.c, so I guess you have to change that location now. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/07/2013 08:39 AM, Arnd Bergmann wrote: > On Wednesday 06 February 2013, Jon Hunter wrote: >> +static struct of_dma_filter_info info; > > Both members of this structure are constant, so you can just initialize it here, > and it would be nice to give it a more descriptive name, such as omap_dmadev_info. > >> static struct platform_driver omap_dma_driver = { >> .probe = omap_dma_probe, >> .remove = omap_dma_remove, >> .driver = { >> .name = "omap-dma-engine", >> .owner = THIS_MODULE, >> + .of_match_table = of_match_ptr(omap_dma_match) >> }, >> }; > > please end the new line with a comma. Ok. >> @@ -673,7 +702,7 @@ static int omap_dma_init(void) >> { >> int rc = platform_driver_register(&omap_dma_driver); >> >> - if (rc == 0) { >> + if ((rc == 0) && (!of_have_populated_dt())) { >> pdev = platform_device_register_full(&omap_dma_dev_info); >> if (IS_ERR(pdev)) { >> platform_driver_unregister(&omap_dma_driver); > > There is already a patch in linux-next that makes this obsolete. > The device is now registered in arch/arm/mach-omap2/dma.c, so > I guess you have to change that location now. Thanks. Will rebase on top of linux-next. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 07 February 2013 09:51:11 Jon Hunter wrote: > >> @@ -673,7 +702,7 @@ static int omap_dma_init(void) > >> { > >> int rc = platform_driver_register(&omap_dma_driver); > >> > >> - if (rc == 0) { > >> + if ((rc == 0) && (!of_have_populated_dt())) { > >> pdev = platform_device_register_full(&omap_dma_dev_info); > >> if (IS_ERR(pdev)) { > >> platform_driver_unregister(&omap_dma_driver); > > > > There is already a patch in linux-next that makes this obsolete. > > The device is now registered in arch/arm/mach-omap2/dma.c, so > > I guess you have to change that location now. > > Thanks. Will rebase on top of linux-next. Actually, there is another problem with that, because then you may get into the situation where nobody can apply your patch. You should never build work on top of linux-next, because then your base gets rebuilt every day. Instead, you should /test/ your branch merged with linux-next to ensure it works with all the other things in place, and when you see conflicts like this, you have to find out what they are and then decide whether you need to rebase it, and to what. Example: $ git log --oneline next/master drivers/dma/omap-dma.c | head be1f948 ARM: OMAP: Fix dmaengine init for multiplatform 45c3eb7 ARM: OMAP: Move plat-omap/dma-omap.h to include/linux/omap-dma.h 8280960 ARM: OMAP: Remove cpu_is_omap usage from plat-omap/dma.c 94c6578 Merge branch 'omap-for-v3.8/cleanup-headers-dma' into omap-for-v3.8/cleanup-headers 27615a9 ARM: OMAP: Trivial driver changes to remove include plat/cpu.h 2b6c4e7 ARM: OMAP: DMA: Move plat/dma.h to plat-omap/dma-omap.h f5a246e Merge tag 'sound-3.7' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound 2dde5b9 dmaengine: omap-dma: Add support to suppress interrupts in cyclic mode ec8b5e4 dmaengine: Pass flags via device_prep_dma_cyclic() callback 2dcdf57 dmaengine: omap: Add support for pause/resume in cyclic dma mode The topmost patch in linux-next is what has the conflict, so let's see how that got there: $ git log --oneline --ancestry-path --merges be1f948..next/master | tail 669166d Merge branch 'next/cleanup' into for-next 82fe557 Merge branch 'next/cleanup' into for-next 2f9adc9 Merge branch 'next/soc' into for-next 91ae65c Merge branch 'next/multiplatform' into for-next 2fd73eb Merge tag 'vt8500-multiplatform-3.9' of git://server.prisktech.co.nz/git/linuxwmt into next/multiplatform 33740d2 Merge branch 'fixes' into for-next b75baf8 Merge branch 'next/multiplatform' into for-next 6130133 Merge tag 'omap-for-v3.9/multiplatform-enable-signed-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap into next/multiplatform 3613f45 Merge branch 'next/multiplatform' into for-next 45f6a1d Merge tag 'omap-for-v3.9/multiplatform-enable-signed-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap into next/multiplatform At the bottom, you can see where it got merged. The commit was in Tony's omap-for-v3.9/multiplatform-enable-signed-v2, which subsequently got merged into arm-soc/next/multiplatform, arm-soc/for-next and next/master. The logical step is to base your patch on top of omap-for-v3.9/multiplatform-enable-signed-v2, and work with Tony and/or Olof and me to get that upstream. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/07/2013 10:07 AM, Arnd Bergmann wrote: > On Thursday 07 February 2013 09:51:11 Jon Hunter wrote: >>>> @@ -673,7 +702,7 @@ static int omap_dma_init(void) >>>> { >>>> int rc = platform_driver_register(&omap_dma_driver); >>>> >>>> - if (rc == 0) { >>>> + if ((rc == 0) && (!of_have_populated_dt())) { >>>> pdev = platform_device_register_full(&omap_dma_dev_info); >>>> if (IS_ERR(pdev)) { >>>> platform_driver_unregister(&omap_dma_driver); >>> >>> There is already a patch in linux-next that makes this obsolete. >>> The device is now registered in arch/arm/mach-omap2/dma.c, so >>> I guess you have to change that location now. >> >> Thanks. Will rebase on top of linux-next. > > Actually, there is another problem with that, because then > you may get into the situation where nobody can apply your > patch. Yes Tony mentioned always using a proper branch for pulls. I will base upon his omap-for-v3.9/multiplatform-v2 (per your inputs, thanks!). > You should never build work on top of linux-next, because > then your base gets rebuilt every day. Instead, you should > /test/ your branch merged with linux-next to ensure it works > with all the other things in place, and when you see conflicts > like this, you have to find out what they are and then decide > whether you need to rebase it, and to what. Ugh, looks like linux-next is broken for omap2+ boards at the moment and is panic'ing in the twl i2c code ... some else to look into ... Peter have you seen this on linux-next? I am seeing this on omap2-4 boards. [ 2.286132] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 2.294738] pgd = c0004000 [ 2.297576] [00000000] *pgd=00000000 [ 2.301361] Internal error: Oops: 5 [#1] SMP ARM [ 2.306243] Modules linked in: [ 2.309448] CPU: 0 Not tainted (3.8.0-rc6-next-20130207-00016-g735c237 #35) [ 2.317169] PC is at twl_i2c_read+0x3c/0xec [ 2.321563] LR is at twl_i2c_read+0x1c/0xec [ 2.325988] pc : [<c0333950>] lr : [<c0333930>] psr: 80000153 [ 2.325988] sp : c702fed0 ip : 00000000 fp : 00000000 [ 2.338043] r10: c702e000 r9 : c06e84e8 r8 : c06e51c8 [ 2.343536] r7 : 00000001 r6 : 00000006 r5 : c702fef6 r4 : 00000004 [ 2.350402] r3 : c129508c r2 : 00000006 r1 : c702fef6 r0 : 0000000e [ 2.357269] Flags: Nzcv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel [ 2.365051] Control: 10c5387d Table: 80004019 DAC: 00000017 [ 2.371093] Process swapper/0 (pid: 1, stack limit = 0xc702e240) [ 2.377410] Stack: (0xc702fed0 to 0xc7030000) [ 2.382019] fec0: c0d42180 c0d429d0 c70a7640 c07354c4 [ 2.390624] fee0: 00000001 c0719798 c0d42180 c06f2cc0 c04e76cc c06ee1ac 00000000 c07354c4 [ 2.399230] ff00: 00000007 c06f2d64 00000017 c06fb308 00000000 c06f07a4 c0cb8580 c06edee0 [ 2.407836] ff20: c06eded4 c06e8504 c0d42180 c0008768 0000009e c00611b4 00000001 00000000 [ 2.416442] ff40: c061994c c06b7548 00000007 00000007 00000000 c07354c4 00000007 c0719798 [ 2.425048] ff60: c0d42180 c06e51c8 c07197a0 0000009e 00000000 c06e590c 00000007 00000007 [ 2.433654] ff80: c06e51c8 00000000 00000000 c04d26ec 00000000 00000000 00000000 00000000 [ 2.442260] ffa0: 00000000 c04d26f4 00000000 c00137b0 00000000 00000000 00000000 00000000 [ 2.450866] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.459472] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 80fb6c10 71bbcd20 [ 2.468078] [<c0333950>] (twl_i2c_read+0x3c/0xec) from [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) [ 2.477722] [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) from [<c06f2d64>] (omap3_twl_init+0x2c/0x70) [ 2.487518] [<c06f2d64>] (omap3_twl_init+0x2c/0x70) from [<c06fb308>] (omap_pmic_late_init+0x18/0x24) [ 2.497222] [<c06fb308>] (omap_pmic_late_init+0x18/0x24) from [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) [ 2.507934] [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) from [<c06edee0>] (omap3_init_late+0xc/0x18) [ 2.518188] [<c06edee0>] (omap3_init_late+0xc/0x18) from [<c06e8504>] (init_machine_late+0x1c/0x28) [ 2.527740] [<c06e8504>] (init_machine_late+0x1c/0x28) from [<c0008768>] (do_one_initcall+0x100/0x16c) [ 2.537536] [<c0008768>] (do_one_initcall+0x100/0x16c) from [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) [ 2.547698] [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04d26f4>] (kernel_init+0x8/0xe4) [ 2.557250] [<c04d26f4>] (kernel_init+0x8/0xe4) from [<c00137b0>] (ret_from_fork+0x14/0x24) Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Jon Hunter <jon-hunter@ti.com> [130207 16:55]: > > Ugh, looks like linux-next is broken for omap2+ boards at the moment > and is panic'ing in the twl i2c code ... some else to look into ... > > Peter have you seen this on linux-next? I am seeing this on omap2-4 boards. Does not happen with arm-soc/for-next, probably related to the drivers/mfd/*twl* changes? Tony > [ 2.286132] Unable to handle kernel NULL pointer dereference at virtual address 00000000 > [ 2.294738] pgd = c0004000 > [ 2.297576] [00000000] *pgd=00000000 > [ 2.301361] Internal error: Oops: 5 [#1] SMP ARM > [ 2.306243] Modules linked in: > [ 2.309448] CPU: 0 Not tainted (3.8.0-rc6-next-20130207-00016-g735c237 #35) > [ 2.317169] PC is at twl_i2c_read+0x3c/0xec > [ 2.321563] LR is at twl_i2c_read+0x1c/0xec > [ 2.325988] pc : [<c0333950>] lr : [<c0333930>] psr: 80000153 > [ 2.325988] sp : c702fed0 ip : 00000000 fp : 00000000 > [ 2.338043] r10: c702e000 r9 : c06e84e8 r8 : c06e51c8 > [ 2.343536] r7 : 00000001 r6 : 00000006 r5 : c702fef6 r4 : 00000004 > [ 2.350402] r3 : c129508c r2 : 00000006 r1 : c702fef6 r0 : 0000000e > [ 2.357269] Flags: Nzcv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel > [ 2.365051] Control: 10c5387d Table: 80004019 DAC: 00000017 > [ 2.371093] Process swapper/0 (pid: 1, stack limit = 0xc702e240) > [ 2.377410] Stack: (0xc702fed0 to 0xc7030000) > [ 2.382019] fec0: c0d42180 c0d429d0 c70a7640 c07354c4 > [ 2.390624] fee0: 00000001 c0719798 c0d42180 c06f2cc0 c04e76cc c06ee1ac 00000000 c07354c4 > [ 2.399230] ff00: 00000007 c06f2d64 00000017 c06fb308 00000000 c06f07a4 c0cb8580 c06edee0 > [ 2.407836] ff20: c06eded4 c06e8504 c0d42180 c0008768 0000009e c00611b4 00000001 00000000 > [ 2.416442] ff40: c061994c c06b7548 00000007 00000007 00000000 c07354c4 00000007 c0719798 > [ 2.425048] ff60: c0d42180 c06e51c8 c07197a0 0000009e 00000000 c06e590c 00000007 00000007 > [ 2.433654] ff80: c06e51c8 00000000 00000000 c04d26ec 00000000 00000000 00000000 00000000 > [ 2.442260] ffa0: 00000000 c04d26f4 00000000 c00137b0 00000000 00000000 00000000 00000000 > [ 2.450866] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 2.459472] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 80fb6c10 71bbcd20 > [ 2.468078] [<c0333950>] (twl_i2c_read+0x3c/0xec) from [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) > [ 2.477722] [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) from [<c06f2d64>] (omap3_twl_init+0x2c/0x70) > [ 2.487518] [<c06f2d64>] (omap3_twl_init+0x2c/0x70) from [<c06fb308>] (omap_pmic_late_init+0x18/0x24) > [ 2.497222] [<c06fb308>] (omap_pmic_late_init+0x18/0x24) from [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) > [ 2.507934] [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) from [<c06edee0>] (omap3_init_late+0xc/0x18) > [ 2.518188] [<c06edee0>] (omap3_init_late+0xc/0x18) from [<c06e8504>] (init_machine_late+0x1c/0x28) > [ 2.527740] [<c06e8504>] (init_machine_late+0x1c/0x28) from [<c0008768>] (do_one_initcall+0x100/0x16c) > [ 2.537536] [<c0008768>] (do_one_initcall+0x100/0x16c) from [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) > [ 2.547698] [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04d26f4>] (kernel_init+0x8/0xe4) > [ 2.557250] [<c04d26f4>] (kernel_init+0x8/0xe4) from [<c00137b0>] (ret_from_fork+0x14/0x24) > > Cheers > Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/07/2013 08:39 AM, Arnd Bergmann wrote: > On Wednesday 06 February 2013, Jon Hunter wrote: >> +static struct of_dma_filter_info info; > > Both members of this structure are constant, so you can just initialize it here, > and it would be nice to give it a more descriptive name, such as omap_dmadev_info. No problem. By the way, I see the dma_cap_mask_t is defined as follows ... typedef struct { DECLARE_BITMAP(bits, DMA_TX_TYPE_END); } dma_cap_mask_t; Is there a good way to statically initialise this? Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 06, 2013 at 03:03:16PM -0600, Jon Hunter wrote: > @@ -673,7 +702,7 @@ static int omap_dma_init(void) > { > int rc = platform_driver_register(&omap_dma_driver); > > - if (rc == 0) { > + if ((rc == 0) && (!of_have_populated_dt())) { Looks good, the worst I can find is this over-use of parens... -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c index 5a31264..a32d81b 100644 --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -16,6 +16,8 @@ #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/of_dma.h> +#include <linux/of_device.h> #include "virt-dma.h" @@ -67,6 +69,8 @@ static const unsigned es_bytes[] = { [OMAP_DMA_DATA_TYPE_S32] = 4, }; +static struct of_dma_filter_info info; + static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d) { return container_of(d, struct omap_dmadev, ddev); @@ -621,10 +625,25 @@ static int omap_dma_probe(struct platform_device *pdev) pr_warn("OMAP-DMA: failed to register slave DMA engine device: %d\n", rc); omap_dma_free(od); + return rc; } else { platform_set_drvdata(pdev, od); } + if (pdev->dev.of_node) { + info.dma_cap = od->ddev.cap_mask; + info.filter_fn = omap_dma_filter_fn; + + /* Device-tree DMA controller registration */ + rc = of_dma_controller_register(pdev->dev.of_node, + of_dma_simple_xlate, &info); + if (rc) { + pr_warn("OMAP-DMA: failed to register DMA controller\n"); + dma_async_device_unregister(&od->ddev); + omap_dma_free(od); + } + } + dev_info(&pdev->dev, "OMAP DMA engine driver\n"); return rc; @@ -634,18 +653,28 @@ static int omap_dma_remove(struct platform_device *pdev) { struct omap_dmadev *od = platform_get_drvdata(pdev); + if (pdev->dev.of_node) + of_dma_controller_free(pdev->dev.of_node); + dma_async_device_unregister(&od->ddev); omap_dma_free(od); return 0; } +static const struct of_device_id omap_dma_match[] = { + { .compatible = "ti,omap-sdma", }, + {}, +}; +MODULE_DEVICE_TABLE(of, omap_dma_match); + static struct platform_driver omap_dma_driver = { .probe = omap_dma_probe, .remove = omap_dma_remove, .driver = { .name = "omap-dma-engine", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(omap_dma_match) }, }; @@ -673,7 +702,7 @@ static int omap_dma_init(void) { int rc = platform_driver_register(&omap_dma_driver); - if (rc == 0) { + if ((rc == 0) && (!of_have_populated_dt())) { pdev = platform_device_register_full(&omap_dma_dev_info); if (IS_ERR(pdev)) { platform_driver_unregister(&omap_dma_driver);
If the device-tree blob is present during boot, then register the SDMA controller with the device-tree DMA driver so that we can use device-tree to look-up DMA client information. Signed-off-by: Jon Hunter <jon-hunter@ti.com> --- drivers/dma/omap-dma.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)