| Message ID | 504A3339.1020506@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 07, 2012 at 23:17:37, Cousson, Benoit wrote: > Hi Vaibhav, > > The following patch is hacing some checkpatch issues. > > CHECK: Alignment should match open parenthesis > #169: FILE: arch/arm/plat-omap/omap_device.c:591: > + dev_dbg(&pdev->dev, "%s(): resources already allocated %d\n", > + __func__, res_count); > > CHECK: Alignment should match open parenthesis > #171: FILE: arch/arm/plat-omap/omap_device.c:593: > + memcpy(res, pdev->resource, > + sizeof(struct resource) * pdev->num_resources); > > CHECK: Alignment should match open parenthesis > #173: FILE: arch/arm/plat-omap/omap_device.c:595: > + omap_device_fill_dma_resources(od, > + &res[pdev->num_resources]); > > CHECK: Alignment should match open parenthesis > #176: FILE: arch/arm/plat-omap/omap_device.c:598: > + dev_dbg(&pdev->dev, "%s(): using resources from hwmod %d\n", > + __func__, res_count); > > total: 0 errors, 0 warnings, 4 checks, 130 lines checked > > > Since I was in a nice mood, because the week-end is almost there, I fixed them myself. > My bad...I usually do check for checkpatch.pl and sparse warnings, not sure how did I miss this one. I will be more careful here onwards. > Please note that I had to rename the API becasue it was way too long to do a proper alignement. > > omap_device_fill_dma_resources -> _od_fill_dma_resources > > In fact I realized that some private APIs should probably be renamed as well like that for > consistency. > > Just let me know if you have any issue with that version. > Looks ok to me. Thanks, Vaibhav > Regards, > Benoit > > --- > From b82b04e8eb27abe0cfe9cd7bf4fee8bb1bb9b013 Mon Sep 17 00:00:00 2001 > From: Vaibhav Hiremath <hvaibhav@ti.com> > Date: Wed, 29 Aug 2012 15:18:11 +0530 > Subject: [PATCH] ARM: OMAP: omap_device: Do not overwrite resources allocated by OF layer > > With the new devices (like, AM33XX and OMAP5) we now only support > DT boot mode of operation and now it is the time to start killing > slowly the dependency on hwmod, so with this patch, we are starting > with device resources. > The idea here is implemented considering to both boot modes - > - DT boot mode > OF framework will construct the resource structure (currently > does for MEM & IRQ resource) and we should respect/use these > resources, killing hwmod dependency. > If pdev->num_resources > 0, we assume that MEM & IRQ resources > have been allocated by OF layer already (through DTB). > > Once DMA resource is available from OF layer, we should > kill filling any resources from hwmod. > > - Non-DT boot mode > Here, pdev->num_resources = 0, and we should get all the > resources from hwmod (following existing steps) > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > Cc: Paul Walmsley <paul@pwsan.com> > Cc: Kevin Hilman <khilman@ti.com> > [b-cousson@ti.com: Fix some checkpatch CHECK issues] > Signed-off-by: Benoit Cousson <b-cousson@ti.com> > --- > arch/arm/mach-omap2/omap_hwmod.c | 27 ++++++++++ > arch/arm/plat-omap/include/plat/omap_hwmod.h | 1 + > arch/arm/plat-omap/omap_device.c | 71 +++++++++++++++++++++---- > 3 files changed, 87 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c > index 6ca8e51..7768804 100644 > --- a/arch/arm/mach-omap2/omap_hwmod.c > +++ b/arch/arm/mach-omap2/omap_hwmod.c > @@ -3158,6 +3158,33 @@ int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res) > } > > /** > + * omap_hwmod_fill_dma_resources - fill struct resource array with dma data > + * @oh: struct omap_hwmod * > + * @res: pointer to the array of struct resource to fill > + * > + * Fill the struct resource array @res with dma resource data from the > + * omap_hwmod @oh. Intended to be called by code that registers > + * omap_devices. See also omap_hwmod_count_resources(). Returns the > + * number of array elements filled. > + */ > +int omap_hwmod_fill_dma_resources(struct omap_hwmod *oh, struct resource *res) > +{ > + int i, sdma_reqs_cnt; > + int r = 0; > + > + sdma_reqs_cnt = _count_sdma_reqs(oh); > + for (i = 0; i < sdma_reqs_cnt; i++) { > + (res + r)->name = (oh->sdma_reqs + i)->name; > + (res + r)->start = (oh->sdma_reqs + i)->dma_req; > + (res + r)->end = (oh->sdma_reqs + i)->dma_req; > + (res + r)->flags = IORESOURCE_DMA; > + r++; > + } > + > + return r; > +} > + > +/** > * omap_hwmod_get_resource_byname - fetch IP block integration data by name > * @oh: struct omap_hwmod * to operate on > * @type: one of the IORESOURCE_* constants from include/linux/ioport.h > diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h > index 6132972..5857b9c 100644 > --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h > +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h > @@ -615,6 +615,7 @@ int omap_hwmod_softreset(struct omap_hwmod *oh); > > int omap_hwmod_count_resources(struct omap_hwmod *oh); > int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res); > +int omap_hwmod_fill_dma_resources(struct omap_hwmod *oh, struct resource *res); > int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type, > const char *name, struct resource *res); > > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c > index ff57b5a..6f5c580 100644 > --- a/arch/arm/plat-omap/omap_device.c > +++ b/arch/arm/plat-omap/omap_device.c > @@ -494,6 +494,33 @@ static int omap_device_fill_resources(struct omap_device *od, > } > > /** > + * _od_fill_dma_resources - fill in array of struct resource with dma resources > + * @od: struct omap_device * > + * @res: pointer to an array of struct resource to be filled in > + * > + * Populate one or more empty struct resource pointed to by @res with > + * the dma resource data for this omap_device @od. Used by > + * omap_device_alloc() after calling omap_device_count_resources(). > + * > + * Ideally this function would not be needed at all. If we have > + * mechanism to get dma resources from DT. > + * > + * Returns 0. > + */ > +static int _od_fill_dma_resources(struct omap_device *od, > + struct resource *res) > +{ > + int i, r; > + > + for (i = 0; i < od->hwmods_cnt; i++) { > + r = omap_hwmod_fill_dma_resources(od->hwmods[i], res); > + res += r; > + } > + > + return 0; > +} > + > +/** > * omap_device_alloc - allocate an omap_device > * @pdev: platform_device that will be included in this omap_device > * @oh: ptr to the single omap_hwmod that backs this omap_device > @@ -532,24 +559,44 @@ struct omap_device *omap_device_alloc(struct platform_device *pdev, > od->hwmods = hwmods; > od->pdev = pdev; > > + res_count = omap_device_count_resources(od); > /* > - * HACK: Ideally the resources from DT should match, and hwmod > - * should just add the missing ones. Since the name is not > - * properly populated by DT, stick to hwmod resources only. > + * DT Boot: > + * OF framework will construct the resource structure (currently > + * does for MEM & IRQ resource) and we should respect/use these > + * resources, killing hwmod dependency. > + * If pdev->num_resources > 0, we assume that MEM & IRQ resources > + * have been allocated by OF layer already (through DTB). > + * > + * Non-DT Boot: > + * Here, pdev->num_resources = 0, and we should get all the > + * resources from hwmod. > + * > + * TODO: Once DMA resource is available from OF layer, we should > + * kill filling any resources from hwmod. > */ > - if (pdev->num_resources && pdev->resource) > - dev_warn(&pdev->dev, "%s(): resources already allocated %d\n", > - __func__, pdev->num_resources); > - > - res_count = omap_device_count_resources(od); > - if (res_count > 0) { > - dev_dbg(&pdev->dev, "%s(): resources allocated from hwmod %d\n", > - __func__, res_count); > + if (res_count > pdev->num_resources) { > + /* Allocate resources memory to account for new resources */ > res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL); > if (!res) > goto oda_exit3; > > - omap_device_fill_resources(od, res); > + /* > + * If pdev->num_resources > 0, then assume that, > + * MEM and IRQ resources will only come from DT and only > + * fill DMA resource from hwmod layer. > + */ > + if (pdev->num_resources && pdev->resource) { > + dev_dbg(&pdev->dev, "%s(): resources already allocated %d\n", > + __func__, res_count); > + memcpy(res, pdev->resource, > + sizeof(struct resource) * pdev->num_resources); > + _od_fill_dma_resources(od, &res[pdev->num_resources]); > + } else { > + dev_dbg(&pdev->dev, "%s(): using resources from hwmod %d\n", > + __func__, res_count); > + omap_device_fill_resources(od, res); > + } > > ret = platform_device_add_resources(pdev, res, res_count); > kfree(res); > -- > 1.7.0.4 > > > > On 08/29/2012 11:48 AM, Vaibhav Hiremath wrote: > > With the new devices (like, AM33XX and OMAP5) we now only support > > DT boot mode of operation and now it is the time to start killing > > slowly the dependency on hwmod, so with this patch, we are starting > > with device resources. > > The idea here is implemented considering to both boot modes - > > - DT boot mode > > OF framework will construct the resource structure (currently > > does for MEM & IRQ resource) and we should respect/use these > > resources, killing hwmod dependency. > > If pdev->num_resources > 0, we assume that MEM & IRQ resources > > have been allocated by OF layer already (through DTB). > > > > Once DMA resource is available from OF layer, we should > > kill filling any resources from hwmod. > > > > - Non-DT boot mode > > Here, pdev->num_resources = 0, and we should get all the > > resources from hwmod (following existing steps) > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > Cc: Benoit Cousson <b-cousson@ti.com> > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Paul Walmsley <paul@pwsan.com> > > Cc: Kevin Hilman <khilman@ti.com> > > --- > > This patch is tested on BeagleBone and AM37xEVM. > > > > Why RFC? > > Still we have function duplication omap_device_fill_resources() and > > omap_device_fill_dma_resources(), we can actually split the function > > into 3 resources and avoid duplication - > > - omap_device_fill_dma_resources() > > - omap_device_fill_mem_resources() > > - omap_device_fill_irq_resources() > > > > Actually I wanted to clean it further but thought of getting > > feedback first and then proceed further. > > > > arch/arm/mach-omap2/omap_hwmod.c | 27 ++++++++++ > > arch/arm/plat-omap/include/plat/omap_hwmod.h | 1 + > > arch/arm/plat-omap/omap_device.c | 72 +++++++++++++++++++++---- > > 3 files changed, 88 insertions(+), 12 deletions(-) > > > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c > > index 31ec283..edabfb3 100644 > > --- a/arch/arm/mach-omap2/omap_hwmod.c > > +++ b/arch/arm/mach-omap2/omap_hwmod.c > > @@ -3330,6 +3330,33 @@ int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res) > > } > > > > /** > > + * omap_hwmod_fill_dma_resources - fill struct resource array with dma data > > + * @oh: struct omap_hwmod * > > + * @res: pointer to the array of struct resource to fill > > + * > > + * Fill the struct resource array @res with dma resource data from the > > + * omap_hwmod @oh. Intended to be called by code that registers > > + * omap_devices. See also omap_hwmod_count_resources(). Returns the > > + * number of array elements filled. > > + */ > > +int omap_hwmod_fill_dma_resources(struct omap_hwmod *oh, struct resource *res) > > +{ > > + int i, sdma_reqs_cnt; > > + int r = 0; > > + > > + sdma_reqs_cnt = _count_sdma_reqs(oh); > > + for (i = 0; i < sdma_reqs_cnt; i++) { > > + (res + r)->name = (oh->sdma_reqs + i)->name; > > + (res + r)->start = (oh->sdma_reqs + i)->dma_req; > > + (res + r)->end = (oh->sdma_reqs + i)->dma_req; > > + (res + r)->flags = IORESOURCE_DMA; > > + r++; > > + } > > + > > + return r; > > +} > > + > > +/** > > * omap_hwmod_get_resource_byname - fetch IP block integration data by name > > * @oh: struct omap_hwmod * to operate on > > * @type: one of the IORESOURCE_* constants from include/linux/ioport.h > > diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h > > index 9b9646c..0533073 100644 > > --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h > > +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h > > @@ -615,6 +615,7 @@ int omap_hwmod_softreset(struct omap_hwmod *oh); > > > > int omap_hwmod_count_resources(struct omap_hwmod *oh); > > int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res); > > +int omap_hwmod_fill_dma_resources(struct omap_hwmod *oh, struct resource *res); > > int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type, > > const char *name, struct resource *res); > > > > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c > > index c490240..fd15a3a 100644 > > --- a/arch/arm/plat-omap/omap_device.c > > +++ b/arch/arm/plat-omap/omap_device.c > > @@ -486,6 +486,33 @@ static int omap_device_fill_resources(struct omap_device *od, > > } > > > > /** > > + * omap_device_fill_dma_resources - fill in array of struct resource with dma resources > > + * @od: struct omap_device * > > + * @res: pointer to an array of struct resource to be filled in > > + * > > + * Populate one or more empty struct resource pointed to by @res with > > + * the dma resource data for this omap_device @od. Used by > > + * omap_device_alloc() after calling omap_device_count_resources(). > > + * > > + * Ideally this function would not be needed at all. If we have > > + * mechanism to get dma resources from DT. > > + * > > + * Returns 0. > > + */ > > +static int omap_device_fill_dma_resources(struct omap_device *od, > > + struct resource *res) > > +{ > > + int i, r; > > + > > + for (i = 0; i < od->hwmods_cnt; i++) { > > + r = omap_hwmod_fill_dma_resources(od->hwmods[i], res); > > + res += r; > > + } > > + > > + return 0; > > +} > > + > > +/** > > * omap_device_alloc - allocate an omap_device > > * @pdev: platform_device that will be included in this omap_device > > * @oh: ptr to the single omap_hwmod that backs this omap_device > > @@ -524,24 +551,45 @@ struct omap_device *omap_device_alloc(struct platform_device *pdev, > > od->hwmods = hwmods; > > od->pdev = pdev; > > > > + res_count = omap_device_count_resources(od); > > /* > > - * HACK: Ideally the resources from DT should match, and hwmod > > - * should just add the missing ones. Since the name is not > > - * properly populated by DT, stick to hwmod resources only. > > + * DT Boot: > > + * OF framework will construct the resource structure (currently > > + * does for MEM & IRQ resource) and we should respect/use these > > + * resources, killing hwmod dependency. > > + * If pdev->num_resources > 0, we assume that MEM & IRQ resources > > + * have been allocated by OF layer already (through DTB). > > + * > > + * Non-DT Boot: > > + * Here, pdev->num_resources = 0, and we should get all the > > + * resources from hwmod. > > + * > > + * TODO: Once DMA resource is available from OF layer, we should > > + * kill filling any resources from hwmod. > > */ > > - if (pdev->num_resources && pdev->resource) > > - dev_warn(&pdev->dev, "%s(): resources already allocated %d\n", > > - __func__, pdev->num_resources); > > - > > - res_count = omap_device_count_resources(od); > > - if (res_count > 0) { > > - dev_dbg(&pdev->dev, "%s(): resources allocated from hwmod %d\n", > > - __func__, res_count); > > + if (res_count > pdev->num_resources) { > > + /* Allocate resources memory to account for new resources */ > > res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL); > > if (!res) > > goto oda_exit3; > > > > - omap_device_fill_resources(od, res); > > + /* > > + * If pdev->num_resources > 0, then assume that, > > + * MEM and IRQ resources will only come from DT and only > > + * fill DMA resource from hwmod layer. > > + */ > > + if (pdev->num_resources && pdev->resource) { > > + dev_dbg(&pdev->dev, "%s(): resources already allocated %d\n", > > + __func__, res_count); > > + memcpy(res, pdev->resource, > > + sizeof(struct resource) * pdev->num_resources); > > + omap_device_fill_dma_resources(od, > > + &res[pdev->num_resources]); > > + } else { > > + dev_dbg(&pdev->dev, "%s(): using resources from hwmod %d\n", > > + __func__, res_count); > > + omap_device_fill_resources(od, res); > > + } > > > > ret = platform_device_add_resources(pdev, res, res_count); > > kfree(res); > > -- > > 1.7.0.4 > > > >
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 6ca8e51..7768804 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -3158,6 +3158,33 @@ int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res) } /** + * omap_hwmod_fill_dma_resources - fill struct resource array with dma data + * @oh: struct omap_hwmod * + * @res: pointer to the array of struct resource to fill + * + * Fill the struct resource array @res with dma resource data from the + * omap_hwmod @oh. Intended to be called by code that registers + * omap_devices. See also omap_hwmod_count_resources(). Returns the + * number of array elements filled. + */ +int omap_hwmod_fill_dma_resources(struct omap_hwmod *oh, struct resource *res) +{ + int i, sdma_reqs_cnt; + int r = 0; + + sdma_reqs_cnt = _count_sdma_reqs(oh); + for (i = 0; i < sdma_reqs_cnt; i++) { + (res + r)->name = (oh->sdma_reqs + i)->name; + (res + r)->start = (oh->sdma_reqs + i)->dma_req; + (res + r)->end = (oh->sdma_reqs + i)->dma_req; + (res + r)->flags = IORESOURCE_DMA; + r++; + } + + return r; +} + +/** * omap_hwmod_get_resource_byname - fetch IP block integration data by name * @oh: struct omap_hwmod * to operate on * @type: one of the IORESOURCE_* constants from include/linux/ioport.h diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h index 6132972..5857b9c 100644 --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h @@ -615,6 +615,7 @@ int omap_hwmod_softreset(struct omap_hwmod *oh); int omap_hwmod_count_resources(struct omap_hwmod *oh); int omap_hwmod_fill_resources(struct omap_hwmod *oh, struct resource *res); +int omap_hwmod_fill_dma_resources(struct omap_hwmod *oh, struct resource *res); int omap_hwmod_get_resource_byname(struct omap_hwmod *oh, unsigned int type, const char *name, struct resource *res); diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index ff57b5a..6f5c580 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -494,6 +494,33 @@ static int omap_device_fill_resources(struct omap_device *od, } /** + * _od_fill_dma_resources - fill in array of struct resource with dma resources + * @od: struct omap_device * + * @res: pointer to an array of struct resource to be filled in + * + * Populate one or more empty struct resource pointed to by @res with + * the dma resource data for this omap_device @od. Used by + * omap_device_alloc() after calling omap_device_count_resources(). + * + * Ideally this function would not be needed at all. If we have + * mechanism to get dma resources from DT. + * + * Returns 0. + */ +static int _od_fill_dma_resources(struct omap_device *od, + struct resource *res) +{ + int i, r; + + for (i = 0; i < od->hwmods_cnt; i++) { + r = omap_hwmod_fill_dma_resources(od->hwmods[i], res); + res += r; + } + + return 0; +} + +/** * omap_device_alloc - allocate an omap_device * @pdev: platform_device that will be included in this omap_device * @oh: ptr to the single omap_hwmod that backs this omap_device @@ -532,24 +559,44 @@ struct omap_device *omap_device_alloc(struct platform_device *pdev, od->hwmods = hwmods; od->pdev = pdev; + res_count = omap_device_count_resources(od); /* - * HACK: Ideally the resources from DT should match, and hwmod - * should just add the missing ones. Since the name is not - * properly populated by DT, stick to hwmod resources only. + * DT Boot: + * OF framework will construct the resource structure (currently + * does for MEM & IRQ resource) and we should respect/use these + * resources, killing hwmod dependency. + * If pdev->num_resources > 0, we assume that MEM & IRQ resources + * have been allocated by OF layer already (through DTB). + * + * Non-DT Boot: + * Here, pdev->num_resources = 0, and we should get all the + * resources from hwmod. + * + * TODO: Once DMA resource is available from OF layer, we should + * kill filling any resources from hwmod. */ - if (pdev->num_resources && pdev->resource) - dev_warn(&pdev->dev, "%s(): resources already allocated %d\n", - __func__, pdev->num_resources); - - res_count = omap_device_count_resources(od); - if (res_count > 0) { - dev_dbg(&pdev->dev, "%s(): resources allocated from hwmod %d\n", - __func__, res_count); + if (res_count > pdev->num_resources) { + /* Allocate resources memory to account for new resources */ res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL); if (!res) goto oda_exit3; - omap_device_fill_resources(od, res); + /* + * If pdev->num_resources > 0, then assume that, + * MEM and IRQ resources will only come from DT and only + * fill DMA resource from hwmod layer. + */ + if (pdev->num_resources && pdev->resource) { + dev_dbg(&pdev->dev, "%s(): resources already allocated %d\n", + __func__, res_count); + memcpy(res, pdev->resource, + sizeof(struct resource) * pdev->num_resources); + _od_fill_dma_resources(od, &res[pdev->num_resources]); + } else { + dev_dbg(&pdev->dev, "%s(): using resources from hwmod %d\n", + __func__, res_count); + omap_device_fill_resources(od, res); + } ret = platform_device_add_resources(pdev, res, res_count); kfree(res);