On 6/16/2010 8:54 AM, Varadarajan, Charulatha wrote:

From: Cousson, Benoit
Sent: Tuesday, June 15, 2010 10:10 PM

Hi Charu,

On 6/15/2010 5:05 PM, Varadarajan, Charulatha wrote:
From: Charulatha V<ch...@ti.com>

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<ch...@ti.com>
---
   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.

It is still a duplication of an information we already have indirectly. The number of GPIO bank is exactly the number of hwmod gpio instances. You do not need any other parameters to get that.

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.

I'm not OK with that. If we start mixing the pure IP specific information with the global one it will be a mess, and it will prevent the easy reuse accross SoC familly. hwmod structure is an IP information. The number of GPIO is an integration information that has nothing to do inside the hwmod. If we want to add an extra instance of GPIO in OMAP4440 for example, we will not be able to reuse the current 4430 data.

So please, don't do that.

BTW, you didn't answer the first answer, do you really need that?


+       .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

No it is not a coincidence, it is because we are re-using the same IP implementation. In that case, it is not a big deal, but you should try to reuse as most as you can already existing data.

Benoit



+
   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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to