Re: [PATCH v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Mon, Nov 22, 2010 at 03:55:47PM +0100, Cousson, Benoit wrote: So far we are using a artificial SW IP revision that reflect the difference we'd like to highlight v1, v2. but what's the big deal in reading that from HW itself ? The only change will in the comparisson operator. Instead of doing: if (rev == REV_V1) you'll be doing: if (rev >= REV_V1_X_Y_Z) no ?? -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On 11/22/2010 12:51 PM, Felipe Balbi wrote: Hi, On Mon, Nov 22, 2010 at 05:46:53AM -0600, Kanigeri, Hari wrote: > From the dmtimer stuff, it looks like the driver defines the version types, which hwmod uses to define the rev field. /* timer ip constants */ #define OMAP_TIMER_IP_LEGACY0x1 #define OMAP_TIMER_IP_VERSION_2 0x2 in that case, it's the same as if you pass in a flag via platform_data. You might as well call it rev, then the diff when converting to hwmod will be smaller maybe :-p Yes, this is exactly what I was proposing to Hari. For the moment create the pdata field that will be populated at init time using the cpu_is. During the hwmod migration, the value will be extracted from the hwmod rev field. 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On 11/22/2010 11:08 AM, Balbi, Felipe wrote: On Fri, Nov 19, 2010 at 03:50:02PM +0100, Cousson, Benoit wrote: Most of the time, we do not want to use the IP revision because it is un-accurate and does not reflect the change we'd like to track. For example some time a minor change in the RTL that will not impact the SW at all might trigger a change in the IP revision, whereas on the other hand a major bug fix that will impact the SW is not capture in the IP revision... yeah, that's bad, but this can happen. That's why we are relying on a rev field in the hwmod. But then, what's inside this rev field ? Is it some internal revision of hwmod or do you read from the hw ? So far we are using a artificial SW IP revision that reflect the difference we'd like to highlight v1, v2. 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
Felipe, On Mon, Nov 22, 2010 at 5:51 AM, Felipe Balbi wrote: > Hi, > > On Mon, Nov 22, 2010 at 05:46:53AM -0600, Kanigeri, Hari wrote: >>> >>> From the dmtimer stuff, it looks like the driver defines the version >> >> types, which hwmod uses to define the rev field. >> >> /* timer ip constants */ >> #define OMAP_TIMER_IP_LEGACY 0x1 >> #define OMAP_TIMER_IP_VERSION_2 0x2 > > in that case, it's the same as if you pass in a flag via platform_data. > You might as well call it rev, then the diff when converting to hwmod > will be smaller maybe :-p I agree with you, the changes should be minimal converting to hwmod. 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
Hi, On Mon, Nov 22, 2010 at 05:46:53AM -0600, Kanigeri, Hari wrote: From the dmtimer stuff, it looks like the driver defines the version types, which hwmod uses to define the rev field. /* timer ip constants */ #define OMAP_TIMER_IP_LEGACY0x1 #define OMAP_TIMER_IP_VERSION_2 0x2 in that case, it's the same as if you pass in a flag via platform_data. You might as well call it rev, then the diff when converting to hwmod will be smaller maybe :-p -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Mon, Nov 22, 2010 at 4:08 AM, Felipe Balbi wrote: > On Fri, Nov 19, 2010 at 03:50:02PM +0100, Cousson, Benoit wrote: >> >> Most of the time, we do not want to use the IP revision because it is >> un-accurate and does not reflect the change we'd like to track. >> For example some time a minor change in the RTL that will not impact the >> SW at all might trigger a change in the IP revision, whereas on the other >> hand a major bug fix that will impact the SW is not capture in the IP >> revision... yeah, that's bad, but this can happen. >> >> That's why we are relying on a rev field in the hwmod. > > But then, what's inside this rev field ? Is it some internal revision of > hwmod or do you read from the hw ? > >From the dmtimer stuff, it looks like the driver defines the version types, which hwmod uses to define the rev field. /* timer ip constants */ #define OMAP_TIMER_IP_LEGACY0x1 #define OMAP_TIMER_IP_VERSION_2 0x2 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Fri, Nov 19, 2010 at 03:50:02PM +0100, Cousson, Benoit wrote: Most of the time, we do not want to use the IP revision because it is un-accurate and does not reflect the change we'd like to track. For example some time a minor change in the RTL that will not impact the SW at all might trigger a change in the IP revision, whereas on the other hand a major bug fix that will impact the SW is not capture in the IP revision... yeah, that's bad, but this can happen. That's why we are relying on a rev field in the hwmod. But then, what's inside this rev field ? Is it some internal revision of hwmod or do you read from the hw ? -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On 11/19/2010 3:22 PM, Kanigeri, Hari wrote: Felipe, On Fri, Nov 19, 2010 at 2:32 AM, Felipe Balbi wrote: On Thu, Nov 18, 2010 at 06:07:40PM -0600, Kanigeri, Hari wrote: Benoit, On Thu, Nov 18, 2010 at 5:28 PM, Cousson, Benoit wrote: On 11/18/2010 8:15 PM, Hari Kanigeri wrote: disabling 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. Signed-off-by: Hari Kanigeri --- arch/arm/mach-omap2/mailbox.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 42dbfa4..82b5ced 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) Since it is not omap version specific but IP version specific, you should not use cpu_is_ to do that. Moreover cpu_is calls should be used during init only. You can use the rev field in hwmod_class in order to detect the IP version. Smartreflex series for 3630 is already using that kind of mechanism. You will have to copy that revision information into pdata struct and then use that here. I see your point, but since mailbox hwmod patches from Omar are still under review I didn't find any other option than to enable this This is critical functionality that I want to include in and not wait till the hwmod patches are accepted. OK, I didn't see that your series was supposed to be done before Omar's one. Please let me know if there is any other way of approaching this problem ? how about you read the IP revision yourself during probe ? Or pass in a flag like I said on the other email ? I like your proposal of reading the IP revision in probe. I will send a revised patch for this. Most of the time, we do not want to use the IP revision because it is un-accurate and does not reflect the change we'd like to track. For example some time a minor change in the RTL that will not impact the SW at all might trigger a change in the IP revision, whereas on the other hand a major bug fix that will impact the SW is not capture in the IP revision... yeah, that's bad, but this can happen. That's why we are relying on a rev field in the hwmod. Meanwhile, you can do the cpu_is check at init time, then fill a pdata attribute with a revision parameter, and use that revision parameter in the omap2_mbox_disable_irq. When hwmod will be there you will just extract that information from the hwmod rev field. 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
Felipe, On Fri, Nov 19, 2010 at 2:32 AM, Felipe Balbi wrote: > On Thu, Nov 18, 2010 at 06:07:40PM -0600, Kanigeri, Hari wrote: >> >> Benoit, >> >> On Thu, Nov 18, 2010 at 5:28 PM, Cousson, Benoit wrote: >>> >>> On 11/18/2010 8:15 PM, Hari Kanigeri wrote: disabling 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. Signed-off-by: Hari Kanigeri --- arch/arm/mach-omap2/mailbox.c | 5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 42dbfa4..82b5ced 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) >>> >>> Since it is not omap version specific but IP version specific, you should >>> not use cpu_is_ to do that. Moreover cpu_is calls should be used during >>> init >>> only. >>> You can use the rev field in hwmod_class in order to detect the IP >>> version. >>> Smartreflex series for 3630 is already using that kind of mechanism. >>> You will have to copy that revision information into pdata struct and >>> then >>> use that here. >> >> I see your point, but since mailbox hwmod patches from Omar are still >> under review I didn't find any other option than to enable this >> This is critical functionality that I want to include in and not wait >> till the hwmod patches are accepted. >> Please let me know if there is any other way of approaching this problem ? > > how about you read the IP revision yourself during probe ? Or pass in a > flag like I said on the other email ? > I like your proposal of reading the IP revision in probe. I will send a revised patch for this. Benoit, is this ok until we move to hwmod implemenation ? 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Thu, Nov 18, 2010 at 06:07:40PM -0600, Kanigeri, Hari wrote: Benoit, On Thu, Nov 18, 2010 at 5:28 PM, Cousson, Benoit wrote: On 11/18/2010 8:15 PM, Hari Kanigeri wrote: disabling 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. Signed-off-by: Hari Kanigeri --- arch/arm/mach-omap2/mailbox.c | 5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 42dbfa4..82b5ced 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) Since it is not omap version specific but IP version specific, you should not use cpu_is_ to do that. Moreover cpu_is calls should be used during init only. You can use the rev field in hwmod_class in order to detect the IP version. Smartreflex series for 3630 is already using that kind of mechanism. You will have to copy that revision information into pdata struct and then use that here. I see your point, but since mailbox hwmod patches from Omar are still under review I didn't find any other option than to enable this This is critical functionality that I want to include in and not wait till the hwmod patches are accepted. Please let me know if there is any other way of approaching this problem ? how about you read the IP revision yourself during probe ? Or pass in a flag like I said on the other email ? -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On Thu, Nov 18, 2010 at 01:15:39PM -0600, Hari Kanigeri wrote: disabling 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. How nasty :-p Signed-off-by: Hari Kanigeri --- arch/arm/mach-omap2/mailbox.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 42dbfa4..82b5ced 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) + l |= bit; + else + l &= ~bit; you should not use cpu_is_* checks on drivers. Even though this is located under mach-omap2, it's still a normal driver and you should not use those checks. Pass a flag like "has_twisted_rx_irq_disable" via platform_data and use that instead. -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
Benoit, On Thu, Nov 18, 2010 at 5:28 PM, Cousson, Benoit wrote: > On 11/18/2010 8:15 PM, Hari Kanigeri wrote: >> >> disabling 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. >> >> Signed-off-by: Hari Kanigeri >> --- >> arch/arm/mach-omap2/mailbox.c | 5 - >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c >> index 42dbfa4..82b5ced 100644 >> --- a/arch/arm/mach-omap2/mailbox.c >> +++ b/arch/arm/mach-omap2/mailbox.c >> @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) > > Since it is not omap version specific but IP version specific, you should > not use cpu_is_ to do that. Moreover cpu_is calls should be used during init > only. > You can use the rev field in hwmod_class in order to detect the IP version. > Smartreflex series for 3630 is already using that kind of mechanism. > You will have to copy that revision information into pdata struct and then > use that here. I see your point, but since mailbox hwmod patches from Omar are still under review I didn't find any other option than to enable this This is critical functionality that I want to include in and not wait till the hwmod patches are accepted. Please let me know if there is any other way of approaching this problem ? Thank you, Best regards, Hari -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
On 11/18/2010 8:15 PM, Hari Kanigeri wrote: disabling 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. Signed-off-by: Hari Kanigeri --- arch/arm/mach-omap2/mailbox.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 42dbfa4..82b5ced 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) Since it is not omap version specific but IP version specific, you should not use cpu_is_ to do that. Moreover cpu_is calls should be used during init only. You can use the rev field in hwmod_class in order to detect the IP version. Smartreflex series for 3630 is already using that kind of mechanism. You will have to copy that revision information into pdata struct and then use that here. Benoit + l |= bit; + else + l&= ~bit; mbox_write_reg(l, p->irqdisable); } -- 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 v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4
disabling 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. Signed-off-by: Hari Kanigeri --- arch/arm/mach-omap2/mailbox.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 42dbfa4..82b5ced 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -195,7 +195,10 @@ 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 (cpu_is_omap44xx()) + l |= bit; + else + l &= ~bit; mbox_write_reg(l, p->irqdisable); } -- 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
