RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
> -Original Message-
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Thursday, June 17, 2010 10:05 PM
> To: Varadarajan, Charulatha
> Cc: Cousson, Benoit; t...@atomide.com; davi...@pacbell.net;
> broo...@opensource.wolfsonmicro.com;
> a...@linux-foundation.org; linux-omap@vger.kernel.org;
> p...@pwsan.com; Nayak, Rajendra; Basak, Partha; Shilimkar, Santosh
> Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support
> for OMAP2PLUS chip GPIO init
>
> "Varadarajan, Charulatha" writes:
>
> [...]
>
> >> >>
> >> >>>
> >> >>> What does 'method' mean in that context? Maybe the
> name should be revisited?
> >> >>
> >> >> Agree. 'method' is used throughout OMAP GPIO code. As
> mentioned above, this
> >> field would be removed
> >> >> once the whole GPIO code is cleaned up. This patch
> series doesn't bother to
> >> clean up GPIO code as the
> >> >> changes would be huge and intended only for HWMOD FW
> adaptation. Cleaning up
> >> GPIO code would come as
> >> >> a separate series and we can address this then.
> >> >>
> >> > Sorry if my comment is not aligned but I thought we are
> addressing the
> >> > gpio clean up as well.
> >> >
> >> > If we are re-vamping the code so much, is it not the
> right time to clean up as
> >> well ??
> >>
> >> I agree with Santosh, you are already cleaning a bunch of
> things, and in
> >> that case you can easily take advantage of HWMOD to remove
> a good amount
> >> of useless code.
> >>
> >> We'd better do that right now, instead of waiting a next phase that
> >> might never happen...
> >
> > Since hwmod migration would change mainly the init part of the code,
> > I started working on hwmod migration as part of the first
> > series. Once we agree upon the final patch set for GPIO hwmod
> > migration, I can work on top of the hwmod migration patch series to
> > clean up the GPIO code and send a dependent series. This will help
> > sending the changes in smaller chunks.
> >
> > I would add a TODO section in patch description outlining the
> > cleanup to be done in the next patch series.
>
> At a minimum, a TODO describing the rest of the cleanups would be
> helpful.
Okay. Will take care in next version of patch series.
>
> > Tony,
> > Can you add your feedback?
> >
> > Please refer
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg2606
5.html for the old context.
> >
> >>
> >> And BTW, this 'method' is a IP version dependent information and
> >> not a Soc specific one. You can potentially use the HW revision
> >> field, if it is available for the GPIO.
> >
> > I agree that it is not SoC specific. But I still feel that it is
> > better not to have 'method' as part of dev_attr, considering that,
> > after clean-up, this information will no longer be needed.
>
> Considering just this 'method' issue, and not the entire cleanup, I
> don't like this extra 'user' field past to omap2_init_gpio() used for
> the method.
Okay, I will avoid passing 'method' as 'user' field in my next version of
this patch series.
Instead will use 'rev' field of omap_hwmod_class structure and will populate
'method' based on 'rev' value.
In hwmod database assign,
'rev' value for OMAP2 as '0'
'rev' value for OMAP3 as '1'
'rev' value for OMAP4 as '2'
static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
{
...
...
switch (oh->class->rev) {
case 0:
case 1:
pdata->method = METHOD_GPIO_24XX;
break;
case 2:
pdata->method = METHOD_GPIO_44XX;
break;
}
...
...
}
>
> Can you investigate whether or not this flag is even needed and if we
> can determine the method based on the REVISION register?
AFAIK 'method' is not based on REVISION register. If you see 'method'
usage for omap15xx chip, it has METHOD_MPUIO and METHOD_GPIO_1510.
Similar for omap16xx and omap 7xx.
Even after cleanup we might need a way to find out if the gpio bank falls
under MPUIO method. I would do a name change for 'method' as suggested
by Benoit during cleanup and it would be used only for identifying
MPUIO type after cleanup.
>
> Kevin
> --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
"Varadarajan, Charulatha" writes: [...] >> >> >> >>> >> >>> What does 'method' mean in that context? Maybe the name should be >> >>> revisited? >> >> >> >> Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, >> >> this >> field would be removed >> >> once the whole GPIO code is cleaned up. This patch series doesn't bother >> >> to >> clean up GPIO code as the >> >> changes would be huge and intended only for HWMOD FW adaptation. Cleaning >> >> up >> GPIO code would come as >> >> a separate series and we can address this then. >> >> >> > Sorry if my comment is not aligned but I thought we are addressing the >> > gpio clean up as well. >> > >> > If we are re-vamping the code so much, is it not the right time to clean >> > up as >> well ?? >> >> I agree with Santosh, you are already cleaning a bunch of things, and in >> that case you can easily take advantage of HWMOD to remove a good amount >> of useless code. >> >> We'd better do that right now, instead of waiting a next phase that >> might never happen... > > Since hwmod migration would change mainly the init part of the code, > I started working on hwmod migration as part of the first > series. Once we agree upon the final patch set for GPIO hwmod > migration, I can work on top of the hwmod migration patch series to > clean up the GPIO code and send a dependent series. This will help > sending the changes in smaller chunks. > > I would add a TODO section in patch description outlining the > cleanup to be done in the next patch series. At a minimum, a TODO describing the rest of the cleanups would be helpful. > Tony, > Can you add your feedback? > > Please refer > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg26065.html for the > old context. > >> >> And BTW, this 'method' is a IP version dependent information and >> not a Soc specific one. You can potentially use the HW revision >> field, if it is available for the GPIO. > > I agree that it is not SoC specific. But I still feel that it is > better not to have 'method' as part of dev_attr, considering that, > after clean-up, this information will no longer be needed. Considering just this 'method' issue, and not the entire cleanup, I don't like this extra 'user' field past to omap2_init_gpio() used for the method. Can you investigate whether or not this flag is even needed and if we can determine the method based on the REVISION register? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
> >>> On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
> From: Charulatha V
>
> This patch adds support for handling GPIO as a HWMOD FW adapted
> platform device for OMAP2PLUS chips.
>
> gpio_init needs to be done before machine_init functions access gpio
> APIs.
> Hence gpio_init is made as a postcore_initcall.
>
> Signed-off-by: Charulatha V
> ---
> arch/arm/mach-omap2/gpio.c | 104
>
> 1 files changed, 104 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-omap2/gpio.c
>
> diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
> new file mode 100644
> index 000..993995a
> --- /dev/null
> +++ b/arch/arm/mach-omap2/gpio.c
[snip]
> +static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
> +{
> +struct omap_device *od;
> +struct omap_gpio_platform_data *pdata;
> +char *name = "omap-gpio";
> +static int id;
> +struct omap_gpio_dev_attr *gpio_dev_data;
> +
> +if (!oh)
> +pr_err("Could not look up omap gpio %d\n", id + 1);
> +
> +pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
> +GFP_KERNEL);
> +if (!pdata) {
> +pr_err("Memory allocation failed gpio%d\n", id + 1);
> +return -ENOMEM;
> +}
> +
> +gpio_dev_data = (struct omap_gpio_dev_attr *)oh->dev_attr;
> +pdata->gpio_attr = gpio_dev_data;
> +pdata->method = (int)user;
> >>>
> >>> That method seems to be an IP version specific information and not a Soc
> >>> specific one. You should store that in the hwmod dev_attr.
> >>
> >> 'method' is chip ID information mapped to GPIO driver specific enum that is
> passed to the driver
> >> (eg., METHOD_GPIO_24XX, METHOD_GPIO_44XX). I think this should not be
> >> moved to
> hwmod dev_attr because
> >> this is only an info required for the driver to identify the chip ID and
> accordingly access
> >> functions/ registers.
> >>
> >> Also this 'method' would be removed once GPIO code undergoes a complete
> cleanup.
> >>
> >>>
> >>> What does 'method' mean in that context? Maybe the name should be
> >>> revisited?
> >>
> >> Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, this
> field would be removed
> >> once the whole GPIO code is cleaned up. This patch series doesn't bother to
> clean up GPIO code as the
> >> changes would be huge and intended only for HWMOD FW adaptation. Cleaning
> >> up
> GPIO code would come as
> >> a separate series and we can address this then.
> >>
> > Sorry if my comment is not aligned but I thought we are addressing the
> > gpio clean up as well.
> >
> > If we are re-vamping the code so much, is it not the right time to clean up
> > as
> well ??
>
> I agree with Santosh, you are already cleaning a bunch of things, and in
> that case you can easily take advantage of HWMOD to remove a good amount
> of useless code.
>
> We'd better do that right now, instead of waiting a next phase that
> might never happen...
Since hwmod migration would change mainly the init part of the code, I started
working on hwmod migration as part of the first series. Once we agree upon the
final patch set for GPIO hwmod migration, I can work on top of the hwmod
migration patch series to clean up the GPIO code and send a dependent series.
This will help sending the changes in smaller chunks.
I would add a TODO section in patch description outlining the cleanup to be
done in the next patch series.
Tony,
Can you add your feedback?
Please refer
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg26065.html for the
old context.
>
> And BTW, this 'method' is a IP version dependent information and not a
> Soc specific one. You can potentially use the HW revision field, if it
> is available for the GPIO.
I agree that it is not SoC specific. But I still feel that it is better not to
have 'method' as part of dev_attr, considering that, after clean-up, this
information will no longer be needed.
>
> Regards,
> Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
On 6/16/2010 10:46 AM, Varadarajan, Charulatha wrote:
-Original Message-
From: Shilimkar, Santosh
Sent: Wednesday, June 16, 2010 12:51 PM
To: Varadarajan, Charulatha; Cousson, Benoit
Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
khil...@deeprootsystems.com; t...@atomide.com; Basak, Partha
Subject: RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
GPIO init
-Original Message-
From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org]
On Behalf Of
Varadarajan, Charulatha
Sent: Wednesday, June 16, 2010 12:25 PM
To: Cousson, Benoit
Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
foundation.org; linux-
o...@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
khil...@deeprootsystems.com; t...@atomide.com;
Basak, Partha
Subject: RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
GPIO init
-Original Message-
From: Cousson, Benoit
Sent: Tuesday, June 15, 2010 10:53 PM
To: Varadarajan, Charulatha
Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
khil...@deeprootsystems.com; t...@atomide.com
Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
GPIO init
On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
From: Charulatha V
This patch adds support for handling GPIO as a HWMOD FW adapted
platform device for OMAP2PLUS chips.
gpio_init needs to be done before machine_init functions access gpio APIs.
Hence gpio_init is made as a postcore_initcall.
Signed-off-by: Charulatha V
---
arch/arm/mach-omap2/gpio.c | 104
1 files changed, 104 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/gpio.c
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
new file mode 100644
index 000..993995a
--- /dev/null
+++ b/arch/arm/mach-omap2/gpio.c
@@ -0,0 +1,104 @@
+/*
+ * gpio.c - OMAP2PLUS-specific gpio code
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Author:
+ * Charulatha V
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+static struct omap_device_pm_latency omap_gpio_latency[] = {
+ [0] = {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ },
+};
+
+static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
+{
+ struct omap_device *od;
+ struct omap_gpio_platform_data *pdata;
+ char *name = "omap-gpio";
+ static int id;
+ struct omap_gpio_dev_attr *gpio_dev_data;
+
+ if (!oh)
+ pr_err("Could not look up omap gpio %d\n", id + 1);
+
+ pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_err("Memory allocation failed gpio%d\n", id + 1);
+ return -ENOMEM;
+ }
+
+ gpio_dev_data = (struct omap_gpio_dev_attr *)oh->dev_attr;
+ pdata->gpio_attr = gpio_dev_data;
+ pdata->method = (int)user;
That method seems to be an IP version specific information and not a Soc
specific one. You should store that in the hwmod dev_attr.
'method' is chip ID information mapped to GPIO driver specific enum that is
passed to the driver
(eg., METHOD_GPIO_24XX, METHOD_GPIO_44XX). I think this should not be moved to
hwmod dev_attr because
this is only an info required for the driver to identify the chip ID and
accordingly access
functions/ registers.
Also this 'method' would be removed once GPIO code undergoes a complete cleanup.
What does 'method' mean in that context? Maybe the name should be revisited?
Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, this
field would be removed
once the whole GPIO code is cleaned up. This patch series doesn't bother to
clean up GPIO code as the
changes would be huge and intended only for HWMOD FW adaptation. Cleaning up
GPIO code would come as
a separate series and we can address this then.
Sorry if my comment is not aligned but I thought we are addressing the
gpio clean up as well.
If we are re-vamping the code so much, is it not the right time to clean up as
well ??
We are not addressing GPIO code clean up in this series. As you mentioned, we
are revamping the code so much just for doing HWMOD FW adaptation. If we
attempt to clean up the whole GPIO code along with this, we will have
Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
On 6/16/2010 9:21 AM, Shilimkar, Santosh wrote:
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of
Varadarajan, Charulatha
Sent: Wednesday, June 16, 2010 12:25 PM
To: Cousson, Benoit
Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com;
a...@linux-foundation.org; linux-
o...@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
khil...@deeprootsystems.com; t...@atomide.com;
Basak, Partha
Subject: RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
GPIO init
-Original Message-
From: Cousson, Benoit
Sent: Tuesday, June 15, 2010 10:53 PM
To: Varadarajan, Charulatha
Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
khil...@deeprootsystems.com; t...@atomide.com
Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
GPIO init
On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
From: Charulatha V
This patch adds support for handling GPIO as a HWMOD FW adapted
platform device for OMAP2PLUS chips.
gpio_init needs to be done before machine_init functions access gpio APIs.
Hence gpio_init is made as a postcore_initcall.
Signed-off-by: Charulatha V
---
arch/arm/mach-omap2/gpio.c | 104
1 files changed, 104 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/gpio.c
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
new file mode 100644
index 000..993995a
--- /dev/null
+++ b/arch/arm/mach-omap2/gpio.c
@@ -0,0 +1,104 @@
+/*
+ * gpio.c - OMAP2PLUS-specific gpio code
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Author:
+ * Charulatha V
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+static struct omap_device_pm_latency omap_gpio_latency[] = {
+ [0] = {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ },
+};
+
+static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
+{
+ struct omap_device *od;
+ struct omap_gpio_platform_data *pdata;
+ char *name = "omap-gpio";
+ static int id;
+ struct omap_gpio_dev_attr *gpio_dev_data;
+
+ if (!oh)
+ pr_err("Could not look up omap gpio %d\n", id + 1);
+
+ pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_err("Memory allocation failed gpio%d\n", id + 1);
+ return -ENOMEM;
+ }
+
+ gpio_dev_data = (struct omap_gpio_dev_attr *)oh->dev_attr;
+ pdata->gpio_attr = gpio_dev_data;
+ pdata->method = (int)user;
That method seems to be an IP version specific information and not a Soc
specific one. You should store that in the hwmod dev_attr.
'method' is chip ID information mapped to GPIO driver specific enum that is
passed to the driver
(eg., METHOD_GPIO_24XX, METHOD_GPIO_44XX). I think this should not be moved to
hwmod dev_attr because
this is only an info required for the driver to identify the chip ID and
accordingly access
functions/ registers.
Also this 'method' would be removed once GPIO code undergoes a complete cleanup.
What does 'method' mean in that context? Maybe the name should be revisited?
Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, this
field would be removed
once the whole GPIO code is cleaned up. This patch series doesn't bother to
clean up GPIO code as the
changes would be huge and intended only for HWMOD FW adaptation. Cleaning up
GPIO code would come as
a separate series and we can address this then.
Sorry if my comment is not aligned but I thought we are addressing the
gpio clean up as well.
If we are re-vamping the code so much, is it not the right time to clean up as
well ??
I agree with Santosh, you are already cleaning a bunch of things, and in
that case you can easily take advantage of HWMOD to remove a good amount
of useless code.
We'd better do that right now, instead of waiting a next phase that
might never happen...
And BTW, this 'method' is a IP version dependent information and not a
Soc specific one. You can potentially use the HW revision field, if it
is available for the GPIO.
Regards,
Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
> -Original Message-
> From: Shilimkar, Santosh
> Sent: Wednesday, June 16, 2010 12:51 PM
> To: Varadarajan, Charulatha; Cousson, Benoit
> Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
> foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
> khil...@deeprootsystems.com; t...@atomide.com; Basak, Partha
> Subject: RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
> GPIO init
>
> > -Original Message-
> > From: linux-omap-ow...@vger.kernel.org
> > [mailto:linux-omap-ow...@vger.kernel.org]
> On Behalf Of
> > Varadarajan, Charulatha
> > Sent: Wednesday, June 16, 2010 12:25 PM
> > To: Cousson, Benoit
> > Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
> foundation.org; linux-
> > o...@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
> khil...@deeprootsystems.com; t...@atomide.com;
> > Basak, Partha
> > Subject: RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS
> > chip
> GPIO init
> >
> >
> >
> > > -Original Message-
> > > From: Cousson, Benoit
> > > Sent: Tuesday, June 15, 2010 10:53 PM
> > > To: Varadarajan, Charulatha
> > > Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
> > > foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak,
> > > Rajendra;
> > > khil...@deeprootsystems.com; t...@atomide.com
> > > Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS
> > > chip
> > > GPIO init
> > >
> > > On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
> > > > From: Charulatha V
> > > >
> > > > This patch adds support for handling GPIO as a HWMOD FW adapted
> > > > platform device for OMAP2PLUS chips.
> > > >
> > > > gpio_init needs to be done before machine_init functions access gpio
> > > > APIs.
> > > > Hence gpio_init is made as a postcore_initcall.
> > > >
> > > > Signed-off-by: Charulatha V
> > > > ---
> > > > arch/arm/mach-omap2/gpio.c | 104
>
> > > > 1 files changed, 104 insertions(+), 0 deletions(-)
> > > > create mode 100644 arch/arm/mach-omap2/gpio.c
> > > >
> > > > diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
> > > > new file mode 100644
> > > > index 000..993995a
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-omap2/gpio.c
> > > > @@ -0,0 +1,104 @@
> > > > +/*
> > > > + * gpio.c - OMAP2PLUS-specific gpio code
> > > > + *
> > > > + * Copyright (C) 2010 Texas Instruments, Inc.
> > > > + *
> > > > + * Author:
> > > > + * Charulatha V
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License version 2 as
> > > > + * published by the Free Software Foundation.
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include
> > > > +#include
> > > > +
> > > > +static struct omap_device_pm_latency omap_gpio_latency[] = {
> > > > + [0] = {
> > > > + .deactivate_func = omap_device_idle_hwmods,
> > > > + .activate_func = omap_device_enable_hwmods,
> > > > + .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> > > > + },
> > > > +};
> > > > +
> > > > +static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
> > > > +{
> > > > + struct omap_device *od;
> > > > + struct omap_gpio_platform_data *pdata;
> > > > + char *name = "omap-gpio";
> > > > + static int id;
> > > > + struct omap_gpio_dev_attr *gpio_dev_data;
> > > > +
> > > > + if (!oh)
> > > > + pr_err("Could not look up omap gpio %d\n", id + 1);
> > > > +
> > > > + pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
> > > > + GFP_KERNEL);
> > > > + if (!pdata) {
> > > > + pr_err("Memory allocation failed gpio%d\n", id + 1);
> > &g
RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
> -Original Message-
> From: linux-omap-ow...@vger.kernel.org
> [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of
> Varadarajan, Charulatha
> Sent: Wednesday, June 16, 2010 12:25 PM
> To: Cousson, Benoit
> Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com;
> a...@linux-foundation.org; linux-
> o...@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
> khil...@deeprootsystems.com; t...@atomide.com;
> Basak, Partha
> Subject: RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS
> chip GPIO init
>
>
>
> > -Original Message-
> > From: Cousson, Benoit
> > Sent: Tuesday, June 15, 2010 10:53 PM
> > To: Varadarajan, Charulatha
> > Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
> > foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
> > khil...@deeprootsystems.com; t...@atomide.com
> > Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS
> > chip
> > GPIO init
> >
> > On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
> > > From: Charulatha V
> > >
> > > This patch adds support for handling GPIO as a HWMOD FW adapted
> > > platform device for OMAP2PLUS chips.
> > >
> > > gpio_init needs to be done before machine_init functions access gpio APIs.
> > > Hence gpio_init is made as a postcore_initcall.
> > >
> > > Signed-off-by: Charulatha V
> > > ---
> > > arch/arm/mach-omap2/gpio.c | 104
> > >
> > > 1 files changed, 104 insertions(+), 0 deletions(-)
> > > create mode 100644 arch/arm/mach-omap2/gpio.c
> > >
> > > diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
> > > new file mode 100644
> > > index 000..993995a
> > > --- /dev/null
> > > +++ b/arch/arm/mach-omap2/gpio.c
> > > @@ -0,0 +1,104 @@
> > > +/*
> > > + * gpio.c - OMAP2PLUS-specific gpio code
> > > + *
> > > + * Copyright (C) 2010 Texas Instruments, Inc.
> > > + *
> > > + * Author:
> > > + * Charulatha V
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#include
> > > +#include
> > > +
> > > +static struct omap_device_pm_latency omap_gpio_latency[] = {
> > > + [0] = {
> > > + .deactivate_func = omap_device_idle_hwmods,
> > > + .activate_func = omap_device_enable_hwmods,
> > > + .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> > > + },
> > > +};
> > > +
> > > +static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
> > > +{
> > > + struct omap_device *od;
> > > + struct omap_gpio_platform_data *pdata;
> > > + char *name = "omap-gpio";
> > > + static int id;
> > > + struct omap_gpio_dev_attr *gpio_dev_data;
> > > +
> > > + if (!oh)
> > > + pr_err("Could not look up omap gpio %d\n", id + 1);
> > > +
> > > + pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
> > > + GFP_KERNEL);
> > > + if (!pdata) {
> > > + pr_err("Memory allocation failed gpio%d\n", id + 1);
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + gpio_dev_data = (struct omap_gpio_dev_attr *)oh->dev_attr;
> > > + pdata->gpio_attr = gpio_dev_data;
> > > + pdata->method = (int)user;
> >
> > That method seems to be an IP version specific information and not a Soc
> > specific one. You should store that in the hwmod dev_attr.
>
> 'method' is chip ID information mapped to GPIO driver specific enum that is
> passed to the driver
> (eg., METHOD_GPIO_24XX, METHOD_GPIO_44XX). I think this should not be moved
> to hwmod dev_attr because
> this is only an info required for the driver to identify the chip ID and
> accordingly access
> functions/ registers.
>
> Also this 'method' would be removed once GPIO code undergoes a complete
> cleanup.
>
> >
> > What does 'method' mean in that context? Maybe the name should be revisited?
>
> Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, this
> field would be removed
> once the whole GPIO code is cleaned up. This patch series doesn't bother to
> clean up GPIO code as the
> changes would be huge and intended only for HWMOD FW adaptation. Cleaning up
> GPIO code would come as
> a separate series and we can address this then.
>
Sorry if my comment is not aligned but I thought we are addressing the
gpio clean up as well.
If we are re-vamping the code so much, is it not the right time to clean up as
well ??
Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
> -Original Message-
> From: Cousson, Benoit
> Sent: Tuesday, June 15, 2010 10:53 PM
> To: Varadarajan, Charulatha
> Cc: davi...@pacbell.net; broo...@opensource.wolfsonmicro.com; a...@linux-
> foundation.org; linux-omap@vger.kernel.org; p...@pwsan.com; Nayak, Rajendra;
> khil...@deeprootsystems.com; t...@atomide.com
> Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip
> GPIO init
>
> On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
> > From: Charulatha V
> >
> > This patch adds support for handling GPIO as a HWMOD FW adapted
> > platform device for OMAP2PLUS chips.
> >
> > gpio_init needs to be done before machine_init functions access gpio APIs.
> > Hence gpio_init is made as a postcore_initcall.
> >
> > Signed-off-by: Charulatha V
> > ---
> > arch/arm/mach-omap2/gpio.c | 104
> >
> > 1 files changed, 104 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-omap2/gpio.c
> >
> > diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
> > new file mode 100644
> > index 000..993995a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/gpio.c
> > @@ -0,0 +1,104 @@
> > +/*
> > + * gpio.c - OMAP2PLUS-specific gpio code
> > + *
> > + * Copyright (C) 2010 Texas Instruments, Inc.
> > + *
> > + * Author:
> > + * Charulatha V
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +static struct omap_device_pm_latency omap_gpio_latency[] = {
> > + [0] = {
> > + .deactivate_func = omap_device_idle_hwmods,
> > + .activate_func = omap_device_enable_hwmods,
> > + .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> > + },
> > +};
> > +
> > +static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
> > +{
> > + struct omap_device *od;
> > + struct omap_gpio_platform_data *pdata;
> > + char *name = "omap-gpio";
> > + static int id;
> > + struct omap_gpio_dev_attr *gpio_dev_data;
> > +
> > + if (!oh)
> > + pr_err("Could not look up omap gpio %d\n", id + 1);
> > +
> > + pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
> > + GFP_KERNEL);
> > + if (!pdata) {
> > + pr_err("Memory allocation failed gpio%d\n", id + 1);
> > + return -ENOMEM;
> > + }
> > +
> > + gpio_dev_data = (struct omap_gpio_dev_attr *)oh->dev_attr;
> > + pdata->gpio_attr = gpio_dev_data;
> > + pdata->method = (int)user;
>
> That method seems to be an IP version specific information and not a Soc
> specific one. You should store that in the hwmod dev_attr.
'method' is chip ID information mapped to GPIO driver specific enum that is
passed to the driver (eg., METHOD_GPIO_24XX, METHOD_GPIO_44XX). I think this
should not be moved to hwmod dev_attr because this is only an info required for
the driver to identify the chip ID and accordingly access functions/ registers.
Also this 'method' would be removed once GPIO code undergoes a complete cleanup.
>
> What does 'method' mean in that context? Maybe the name should be revisited?
Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, this
field would be removed once the whole GPIO code is cleaned up. This patch
series doesn't bother to clean up GPIO code as the changes would be huge and
intended only for HWMOD FW adaptation. Cleaning up GPIO code would come as a
separate series and we can address this then.
>
>
> > + pdata->virtual_irq_start = IH_GPIO_BASE + 32 * id;
> > +
> > + od = omap_device_build(name, id, oh, pdata,
> > + sizeof(*pdata), omap_gpio_latency,
> > + ARRAY_SIZE(omap_gpio_latency),
> > + false);
> > + WARN(IS_ERR(od), "Cant build omap_device for %s:%s.\n",
> > + name, oh->name);
> > +
> > + id++;
> > + return 0;
> > +}
> > +
> > +static int __init gpio_init(int method)
> > +{
> > + return omap_hwmod_for_each_by_class("gpio", omap2_init_gpio,
> > + (void *)method);
> > +}
> > +
> > +/*
> > + * gpio_init needs to be done before
> > + * machine_init functions access gpio APIs.
> > + * Hence gpio_init is a postcore_initcall.
> > + */
> > +#ifdef CONFIG_ARCH_OMAP2
> > +static int __init omap24xx_gpio_init(void)
> > +{ if (!cpu_is_omap24xx())
> > + return -EINVAL;
> > +
> > + return gpio_init(METHOD_GPIO_24XX);
> > +}
> > +postcore_initcall(omap24xx_gpio_init);
> > +#endif
> > +
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init
On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
From: Charulatha V
This patch adds support for handling GPIO as a HWMOD FW adapted
platform device for OMAP2PLUS chips.
gpio_init needs to be done before machine_init functions access gpio APIs.
Hence gpio_init is made as a postcore_initcall.
Signed-off-by: Charulatha V
---
arch/arm/mach-omap2/gpio.c | 104
1 files changed, 104 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/gpio.c
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
new file mode 100644
index 000..993995a
--- /dev/null
+++ b/arch/arm/mach-omap2/gpio.c
@@ -0,0 +1,104 @@
+/*
+ * gpio.c - OMAP2PLUS-specific gpio code
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Author:
+ * Charulatha V
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+static struct omap_device_pm_latency omap_gpio_latency[] = {
+ [0] = {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ },
+};
+
+static int omap2_init_gpio(struct omap_hwmod *oh, void *user)
+{
+ struct omap_device *od;
+ struct omap_gpio_platform_data *pdata;
+ char *name = "omap-gpio";
+ static int id;
+ struct omap_gpio_dev_attr *gpio_dev_data;
+
+ if (!oh)
+ pr_err("Could not look up omap gpio %d\n", id + 1);
+
+ pdata = kzalloc(sizeof(struct omap_gpio_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_err("Memory allocation failed gpio%d\n", id + 1);
+ return -ENOMEM;
+ }
+
+ gpio_dev_data = (struct omap_gpio_dev_attr *)oh->dev_attr;
+ pdata->gpio_attr = gpio_dev_data;
+ pdata->method = (int)user;
That method seems to be an IP version specific information and not a Soc
specific one. You should store that in the hwmod dev_attr.
What does 'method' mean in that context? Maybe the name should be revisited?
+ pdata->virtual_irq_start = IH_GPIO_BASE + 32 * id;
+
+ od = omap_device_build(name, id, oh, pdata,
+ sizeof(*pdata), omap_gpio_latency,
+ ARRAY_SIZE(omap_gpio_latency),
+ false);
+ WARN(IS_ERR(od), "Cant build omap_device for %s:%s.\n",
+ name, oh->name);
+
+ id++;
+ return 0;
+}
+
+static int __init gpio_init(int method)
+{
+ return omap_hwmod_for_each_by_class("gpio", omap2_init_gpio,
+ (void *)method);
+}
+
+/*
+ * gpio_init needs to be done before
+ * machine_init functions access gpio APIs.
+ * Hence gpio_init is a postcore_initcall.
+ */
+#ifdef CONFIG_ARCH_OMAP2
+static int __init omap24xx_gpio_init(void)
+{ if (!cpu_is_omap24xx())
+ return -EINVAL;
+
+ return gpio_init(METHOD_GPIO_24XX);
+}
+postcore_initcall(omap24xx_gpio_init);
+#endif
+
+#ifdef CONFIG_ARCH_OMAP3
+static int __init omap3xxx_gpio_init(void)
+{
+ if (!cpu_is_omap34xx())
+ return -EINVAL;
+
+ return gpio_init(METHOD_GPIO_24XX);
+}
+postcore_initcall(omap3xxx_gpio_init);
+#endif
+
+#ifdef CONFIG_ARCH_OMAP4
+static int __init omap44xx_gpio_init(void)
+{
+ if (!cpu_is_omap44xx())
+ return -EINVAL;
+
+ return gpio_init(METHOD_GPIO_44XX);
+}
You can avoid all this duplication of code by using the dev_attr instead
of using a parameter.
Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
