> -----Original Message-----
> From: Cousson, Benoit
> Sent: Tuesday, June 15, 2010 10:10 PM
> To: Varadarajan, Charulatha
> Cc: david-b@xxxxxxxxxxx; broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx; akpm@linux-
> foundation.org; linux-omap@xxxxxxxxxxxxxxx; paul@xxxxxxxxx; Nayak, Rajendra;
> khilman@xxxxxxxxxxxxxxxxxxx; tony@xxxxxxxxxxx
> Subject: Re: [PATCH 10/13 v3] OMAP: GPIO: Add gpio dev_attr and correct clks in
> OMAP4 hwmod struct
>
> Hi Charu,
>
> On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
> > From: Charulatha V<charu@xxxxxx>
> >
> > This patch adds gpio_dev_attr to OMAP4 gpio hwmod structure. This patch
> > also corrects the gpio .main_clk and .clk fields in gpio hwmod structures.
> >
> > Signed-off-by: Charulatha V<charu@xxxxxx>
> > ---
> > arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 37 +++++++++++++++++++---------
> > 1 files changed, 25 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-
> omap2/omap_hwmod_44xx_data.c
> > index 20f5f8c..b4c0878 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > @@ -22,6 +22,7 @@
> >
> > #include<plat/omap_hwmod.h>
> > #include<plat/cpu.h>
> > +#include<plat/gpio.h>
> >
> > #include "omap_hwmod_common_data.h"
> >
> > @@ -1272,6 +1273,12 @@ static struct omap_hwmod omap44xx_fdif_hwmod = {
> > * general purpose io module
> > */
> >
> > +static struct omap_gpio_dev_attr gpio_dev_attr = {
> > + .gpio_bank_count = 6,
>
> Why do you need that information?
> The point is that this struct is in theory a per instance data not a
> global one. If needed, you should be able to get that from the number of
> iteration done during the init of hwmods.
Even though it is possible to get it through number of iterations, more efficient approach would be to keep this information here. My point is that this information can be auto-generated and hence it is better to keep it in hwmod database itself.
FYI, in a meeting we recently had with Kevin, we discussed about "global data" information getting duplicated for each instance and it was agreed that it is okay to have pointers duplicated for each instance.
>
> > + .gpio_bank_bits = 32,
> > + .dbck_flag = true,
> > +};
>
> Since this structure is the same than OMAP3, you should maybe consider
> sharing it.
They are in different _data.c files. I feel that it will be good if they are kept decoupled as we need not have to mix up different SoC data. Here it's only a coincidence that OMAP3 and OMAP4 have same data.
>
> > +
> > static struct omap_hwmod_class_sysconfig omap44xx_gpio_sysc = {
> > .rev_offs = 0x0000,
> > .sysc_offs = 0x0010,
> > @@ -1305,7 +1312,7 @@ static struct omap_hwmod_addr_space omap44xx_gpio1_addrs[]
> = {
> > static struct omap_hwmod_ocp_if omap44xx_l4_wkup__gpio1 = {
> > .master =&omap44xx_l4_wkup_hwmod,
> > .slave =&omap44xx_gpio1_hwmod,
> > - .clk = "l4_wkup_clk_mux_ck",
> > + .clk = "gpio1_ick",
> > .addr = omap44xx_gpio1_addrs,
> > .addr_cnt = ARRAY_SIZE(omap44xx_gpio1_addrs),
> > .user = OCP_USER_MPU | OCP_USER_SDMA,
> > @@ -1325,7 +1332,7 @@ static struct omap_hwmod omap44xx_gpio1_hwmod = {
> > .class =&omap44xx_gpio_hwmod_class,
> > .mpu_irqs = omap44xx_gpio1_irqs,
> > .mpu_irqs_cnt = ARRAY_SIZE(omap44xx_gpio1_irqs),
> > - .main_clk = "gpio1_ick",
> > + .main_clk = NULL,
>
> Removing the line is enough. It will be null by default.
Agreed.
> I'm still not 100% sure that this is the good way to control the GPIO
> module mode, but at least this is cleaner than the previous clock mapping.
>
> Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
