Re: [PATCH v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote: diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 48e161c..a1c6bd9 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct omap_mbox **list) ret = PTR_ERR(mbox-dev); goto err_out; } + if (cpu_is_omap44xx()) Do not use cpu_is* checks in plat-omap/* see the previous thread. -- balbi -- 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 v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Wed, Nov 24, 2010 at 13:52, Felipe Balbi ba...@ti.com wrote: On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote: diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 48e161c..a1c6bd9 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct omap_mbox **list) ret = PTR_ERR(mbox-dev); goto err_out; } + if (cpu_is_omap44xx()) Do not use cpu_is* checks in plat-omap/* see the previous thread. Referring to [1], I do not find why cpu_is* checks is used in plat-omap and why it can't be avoided. In [1], it was suggested to create the pdata field that will be populated at init time using the cpu_is* check. But in this version, I am finding that this is done in plat-omap. This can be handled in mach-omap layer itself and can be passed as a pdata field which can be extracted during probe. [1] https://patchwork.kernel.org/patch/337131/ -- 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 v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Wed, Nov 24, 2010 at 02:20:40PM +0530, Varadarajan, Charulatha wrote: On Wed, Nov 24, 2010 at 13:52, Felipe Balbi ba...@ti.com wrote: On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote: diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 48e161c..a1c6bd9 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct omap_mbox **list) ret = PTR_ERR(mbox-dev); goto err_out; } + if (cpu_is_omap44xx()) Do not use cpu_is* checks in plat-omap/* see the previous thread. Referring to [1], I do not find why cpu_is* checks is used in plat-omap and why it can't be avoided. In [1], it was suggested to create the pdata field that will be populated at init time using the cpu_is* check. But in this version, I am finding that this is done in plat-omap. This can be handled in mach-omap layer itself and can be passed as a pdata field which can be extracted during probe. [1] https://patchwork.kernel.org/patch/337131/ true, since I saw mbox-rev, I confused that with something that was coming from pdata, sorry. You're right, this is not the right way to do it. -- balbi -- 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 v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Wed, Nov 24, 2010 at 2:50 AM, Varadarajan, Charulatha ch...@ti.com wrote: On Wed, Nov 24, 2010 at 13:52, Felipe Balbi ba...@ti.com wrote: On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote: diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 48e161c..a1c6bd9 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct omap_mbox **list) ret = PTR_ERR(mbox-dev); goto err_out; } + if (cpu_is_omap44xx()) Do not use cpu_is* checks in plat-omap/* see the previous thread. Referring to [1], I do not find why cpu_is* checks is used in plat-omap and why it can't be avoided. In [1], it was suggested to create the pdata field that will be populated at init time using the cpu_is* check. But in this version, I am finding that this is done in plat-omap. This can be handled in mach-omap layer itself and can be passed as a pdata field which can be extracted during probe. [1] https://patchwork.kernel.org/patch/337131/ Here are these reasons why I did this way. - The function omap_mbox_register is called only once during probe time, and I thought it was ok to call cpu check once during probe time. Since each mbox instance needs to be aware of the rev field this was the right place to add since this function iterates through the list during probe time. There are already calls to cpu_is_omap44xx in mailbox probe function. - platform data is not present for mailbox module. I could add it for revision sake but I would prefer not to do that since this will be a throw away code once the hwmod infrastructure is ready (note: mailbox hwmod patches are under review), and amount of mailbox driver rework might be considerable. - I could wait till the hwmod patches are ready to include this change, but don't want to put the dependency with hwmod since this is a critical fix and want to make it available in the mainline kernel. Please let me know what you suggest. Thank you, Best regards, Hari Kanigeri -- 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 v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
Hari, On Wed, Nov 24, 2010 at 18:31, Kanigeri, Hari h-kanige...@ti.com wrote: On Wed, Nov 24, 2010 at 2:50 AM, Varadarajan, Charulatha ch...@ti.com wrote: On Wed, Nov 24, 2010 at 13:52, Felipe Balbi ba...@ti.com wrote: On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote: diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 48e161c..a1c6bd9 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct omap_mbox **list) ret = PTR_ERR(mbox-dev); goto err_out; } + if (cpu_is_omap44xx()) Do not use cpu_is* checks in plat-omap/* see the previous thread. Referring to [1], I do not find why cpu_is* checks is used in plat-omap and why it can't be avoided. In [1], it was suggested to create the pdata field that will be populated at init time using the cpu_is* check. But in this version, I am finding that this is done in plat-omap. This can be handled in mach-omap layer itself and can be passed as a pdata field which can be extracted during probe. [1] https://patchwork.kernel.org/patch/337131/ Here are these reasons why I did this way. - The function omap_mbox_register is called only once during probe time, and I thought it was ok to call cpu check once during probe time. AFAIK it is not okay. Patches are not accepted by maintainers if cpu_is* checks are newly added in plat-omap/* Moreover, the plat-omap is for common code between omap1 and omap2+ If any specific info is required due to unavoidable reasons, they should be managed using pdata fld/ by some other means. Since each mbox instance needs to be aware of the rev field this was the right place to add since this function iterates through the list during probe time. There are already calls to cpu_is_omap44xx in mailbox probe function. Infact , there are some more drivers like that. But some of them are getting cleaned up slowly (work under progress). But old code having cpu_is* checks does not justify adding a new cpu_is* check. As I mentioned, it is repeatedly being insisted in LO mailing list to avoid cpu_is* checks in plat-omap and to clean up the drivers to avoid these checks. - platform data is not present for mailbox module. I could add it for revision sake but I would prefer not to do that since this will be a throw away code once the hwmod infrastructure is ready (note: mailbox hwmod patches are under review), and amount of mailbox driver rework might be considerable. I would leave this to Tony/ Felipe/ Benoit to decide on this. If agreed, I don't see any issue. But there should be atleast a TODO mentioning that this needs to be cleaned up. - I could wait till the hwmod patches are ready to include this change, but don't want to put the dependency with hwmod since this is a critical fix and want to make it available in the mainline kernel. Please let me know what you suggest. Thank you, Best regards, Hari Kanigeri -- 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
[PATCH v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
disablign rx interrupt on omap4 is different than its pre-decessors.
The bit in OMAP4_MAILBOX_IRQENABLE_CLR should be set to disable the
interrupts instead of clearing the bit.
Defined rev field in mailbox structure to differentiate the mailbox
versions.
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
arch/arm/mach-omap2/mailbox.c |7 ++-
arch/arm/plat-omap/include/plat/mailbox.h |4
arch/arm/plat-omap/mailbox.c |4
3 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 42dbfa4..c32058d 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -195,7 +195,12 @@ static void omap2_mbox_disable_irq(struct omap_mbox *mbox,
struct omap_mbox2_priv *p = (struct omap_mbox2_priv *)mbox-priv;
u32 l, bit = (irq == IRQ_TX) ? p-notfull_bit : p-newmsg_bit;
l = mbox_read_reg(p-irqdisable);
- l = ~bit;
+
+ if (mbox-rev == OMAP_MBOX_IP_VERSION_2)
+ l |= bit;
+ else
+ l = ~bit;
+
mbox_write_reg(l, p-irqdisable);
}
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index 13f2ef3..1655c61 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -20,6 +20,9 @@ typedef int __bitwise omap_mbox_type_t;
#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
+#define OMAP_MBOX_IP_LEGACY0x1
+#define OMAP_MBOX_IP_VERSION_2 0x2
+
struct omap_mbox_ops {
omap_mbox_type_ttype;
int (*startup)(struct omap_mbox *mbox);
@@ -58,6 +61,7 @@ struct omap_mbox {
struct omap_mbox_ops*ops;
struct device *dev;
void*priv;
+ int rev;
};
int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 48e161c..a1c6bd9 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct
omap_mbox **list)
ret = PTR_ERR(mbox-dev);
goto err_out;
}
+ if (cpu_is_omap44xx())
+ mbox-rev = OMAP_MBOX_IP_VERSION_2;
+ else
+ mbox-rev = OMAP_MBOX_IP_LEGACY;
}
return 0;
--
1.7.0
--
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 v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Wed, Nov 24, 2010 at 02:56, Hari Kanigeri h-kanige...@ti.com wrote:
Should the subject be like this?
OMAP4: mailbox: fix rx interrupt disable
disablign rx interrupt on omap4 is different than its pre-decessors.
Typo -disablign
The bit in OMAP4_MAILBOX_IRQENABLE_CLR should be set to disable the
interrupts instead of clearing the bit.
Defined rev field in mailbox structure to differentiate the mailbox
versions.
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
arch/arm/mach-omap2/mailbox.c | 7 ++-
arch/arm/plat-omap/include/plat/mailbox.h | 4
arch/arm/plat-omap/mailbox.c | 4
3 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 42dbfa4..c32058d 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -195,7 +195,12 @@ static void omap2_mbox_disable_irq(struct omap_mbox
*mbox,
struct omap_mbox2_priv *p = (struct omap_mbox2_priv *)mbox-priv;
u32 l, bit = (irq == IRQ_TX) ? p-notfull_bit : p-newmsg_bit;
l = mbox_read_reg(p-irqdisable);
- l = ~bit;
+
+ if (mbox-rev == OMAP_MBOX_IP_VERSION_2)
+ l |= bit;
+ else
+ l = ~bit;
+
mbox_write_reg(l, p-irqdisable);
}
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index 13f2ef3..1655c61 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -20,6 +20,9 @@ typedef int __bitwise omap_mbox_type_t;
#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
+#define OMAP_MBOX_IP_LEGACY 0x1
+#define OMAP_MBOX_IP_VERSION_2 0x2
+
struct omap_mbox_ops {
omap_mbox_type_t type;
int (*startup)(struct omap_mbox *mbox);
@@ -58,6 +61,7 @@ struct omap_mbox {
struct omap_mbox_ops *ops;
struct device *dev;
void *priv;
+ int rev;
};
int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 48e161c..a1c6bd9 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct
omap_mbox **list)
ret = PTR_ERR(mbox-dev);
goto err_out;
}
+ if (cpu_is_omap44xx())
Do not use cpu_is* checks in plat-omap/*
+ mbox-rev = OMAP_MBOX_IP_VERSION_2;
+ else
+ mbox-rev = OMAP_MBOX_IP_LEGACY;
}
return 0;
--
1.7.0
--
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
