subject:"\[PATCH V2\] FIX OMAP3\:McBSP poll read and write for OMAP3"

RE: [PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

2009-11-18 Thread Varadarajan, Charu Latha

-Original Message-
From: Tony Lindgren [mailto:t...@atomide.com] 
Sent: Friday, November 13, 2009 2:41 AM
To: Varadarajan, Charu Latha
Cc: linux-omap@vger.kernel.org; Syed, Rafiuddin
Subject: Re: [PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

* ch...@ti.com ch...@ti.com [091019 23:41]:
 omap_mcbsp_pollwrite and omap_mcbsp_pollread functions access
 McBSP registers as 16-bit registers.

 In OMAP3, McBSP registers (DRR_REG and DXR_REG) are limited to
 32-bit data accesses (L4 Interconnect). 16-bit and 8-bit is
 not allowed and can corrupt register content.

 This patch adds omap3_mcbsp_pollwrite and
 omap3_mcbsp_pollread functions for OMAP3 cpu, inorder to
 perform 32 bit access of above mentioned McBSP registers.

How about making the functions generic, I don't think we want
to have separate omap1_mcbsp_pollread, omap2_mcbsp_pollread,
omap3_mcbsp_pollread..

If it's not obvious how to implement it for some omap, just
return an error based on the CPU type.

My v1 patch handles this in a single generic API. 
[http://www.spinics.net/lists/linux-omap/msg19074.html and
http://patchwork.kernel.org/patch/53631/]

As per the review comments received for v1 patch, v2 patch is 
modified to have new API omap3_mcbsp_pollread/write in 
addition to omap_mcbsp_pollread/write APIs. 

Review comments for V1 mentioned that API signartures should not be 
changed from 16bit to 32 bit input parameters. 
If the API needs to be made generic, API signature has to be 
modified as given below (as in patch v1):
-int omap_mcbsp_pollread(unsigned int id, u16 *buf)
+int omap_mcbsp_pollread(unsigned int id, u32 *buf)

If agreed for this generic API change, I shall post v3 patch
which would handle cputypes dynamically inside the API

Regards,

Tony

 Signed-off-by: Charulatha V ch...@ti.com
 Signed-off-by: Syed Rafiuddin rafiuddin.s...@ti.com
 ---
  arch/arm/plat-omap/include/mach/mcbsp.h |4 +
  arch/arm/plat-omap/mcbsp.c  |   92 
++-
  2 files changed, 95 insertions(+), 1 deletions(-)

 diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h 
b/arch/arm/plat-omap/include/mach/mcbsp.h
 index 0c5f937..b0f9260 100644
 --- a/arch/arm/plat-omap/include/mach/mcbsp.h
 +++ b/arch/arm/plat-omap/include/mach/mcbsp.h
 @@ -406,6 +406,10 @@ void omap_mcbsp_set_spi_mode(unsigned 
int id, const struct omap_mcbsp_spi_cfg *
  /* Polled read/write functions */
  int omap_mcbsp_pollread(unsigned int id, u16 * buf);
  int omap_mcbsp_pollwrite(unsigned int id, u16 buf);
 +
 +int omap3_mcbsp_pollread(unsigned int id, u32 *buf);
 +int omap3_mcbsp_pollwrite(unsigned int id, u32 buf);
 +
  int omap_mcbsp_set_io_type(unsigned int id, 
omap_mcbsp_io_type_t io_type);

  #endif
snip--
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 V2] FIX OMAP3:McBSP poll read and write for OMAP3

2009-11-18 Thread Tony Lindgren

* Varadarajan, Charu Latha ch...@ti.com [091118 00:58]:

 -Original Message-
 From: Tony Lindgren [mailto:t...@atomide.com] 
 Sent: Friday, November 13, 2009 2:41 AM
 To: Varadarajan, Charu Latha
 Cc: linux-omap@vger.kernel.org; Syed, Rafiuddin
 Subject: Re: [PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

 * ch...@ti.com ch...@ti.com [091019 23:41]:
  omap_mcbsp_pollwrite and omap_mcbsp_pollread functions access
  McBSP registers as 16-bit registers.

  In OMAP3, McBSP registers (DRR_REG and DXR_REG) are limited to
  32-bit data accesses (L4 Interconnect). 16-bit and 8-bit is
  not allowed and can corrupt register content.

  This patch adds omap3_mcbsp_pollwrite and
  omap3_mcbsp_pollread functions for OMAP3 cpu, inorder to
  perform 32 bit access of above mentioned McBSP registers.

 How about making the functions generic, I don't think we want
 to have separate omap1_mcbsp_pollread, omap2_mcbsp_pollread,
 omap3_mcbsp_pollread..

 If it's not obvious how to implement it for some omap, just
 return an error based on the CPU type.

 My v1 patch handles this in a single generic API. 
 [http://www.spinics.net/lists/linux-omap/msg19074.html and
 http://patchwork.kernel.org/patch/53631/]

Well first of all we don't have any references to this function
in our tree. Second, we have:

#define OMAP_MCBSP_REG_DRR2 0x00
#define OMAP_MCBSP_REG_DRR1 0x02
#define OMAP_MCBSP_REG_DXR2 0x04
#define OMAP_MCBSP_REG_DXR1 0x06

Meaning that both DRR and DXR are 32-bits in length. And based
on that it seems that at least omap_mcbsp_pollwrite is broken
if it supports only one 16-bit write instead of 16 + 16 bits
that the hardware seems to support.

 As per the review comments received for v1 patch, v2 patch is 
 modified to have new API omap3_mcbsp_pollread/write in 
 addition to omap_mcbsp_pollread/write APIs. 

That does not make much sense based on the fact that the
hardware has 32-bits for DRR and DXR. 

 Review comments for V1 mentioned that API signartures should not be 
 changed from 16bit to 32 bit input parameters. 
 If the API needs to be made generic, API signature has to be 
 modified as given below (as in patch v1):
 -int omap_mcbsp_pollread(unsigned int id, u16 *buf)
 +int omap_mcbsp_pollread(unsigned int id, u32 *buf)

 If agreed for this generic API change, I shall post v3 patch
 which would handle cputypes dynamically inside the API

To me it smells like the all drivers using the the
omap_mcbsp_pollread/write are broken legacy drivers.
So maybe we should just remove these two functions?

If we really want to keep these around, we should review
the drivers using these functions. And if we end up keeping
these functions around, I like the first patch better, except
it does not seem to support DRR2 and DXR2 on older hardware,
so that's broken too.

IMHO, we should rather set up this driver as a proper
bus driver as suggested by Paul in a related thread.

Regards,

Tony
--
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 V2] FIX OMAP3:McBSP poll read and write for OMAP3

2009-11-12 Thread Tony Lindgren

* ch...@ti.com ch...@ti.com [091019 23:41]:
 omap_mcbsp_pollwrite and omap_mcbsp_pollread functions access
 McBSP registers as 16-bit registers.
 
 In OMAP3, McBSP registers (DRR_REG and DXR_REG) are limited to
 32-bit data accesses (L4 Interconnect). 16-bit and 8-bit is
 not allowed and can corrupt register content.
 
 This patch adds omap3_mcbsp_pollwrite and
 omap3_mcbsp_pollread functions for OMAP3 cpu, inorder to
 perform 32 bit access of above mentioned McBSP registers.

How about making the functions generic, I don't think we want
to have separate omap1_mcbsp_pollread, omap2_mcbsp_pollread,
omap3_mcbsp_pollread..

If it's not obvious how to implement it for some omap, just
return an error based on the CPU type.

Regards,

Tony
 
 Signed-off-by: Charulatha V ch...@ti.com
 Signed-off-by: Syed Rafiuddin rafiuddin.s...@ti.com
 ---
  arch/arm/plat-omap/include/mach/mcbsp.h |4 +
  arch/arm/plat-omap/mcbsp.c  |   92 
 ++-
  2 files changed, 95 insertions(+), 1 deletions(-)
 
 diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h 
 b/arch/arm/plat-omap/include/mach/mcbsp.h
 index 0c5f937..b0f9260 100644
 --- a/arch/arm/plat-omap/include/mach/mcbsp.h
 +++ b/arch/arm/plat-omap/include/mach/mcbsp.h
 @@ -406,6 +406,10 @@ void omap_mcbsp_set_spi_mode(unsigned int id, const 
 struct omap_mcbsp_spi_cfg *
  /* Polled read/write functions */
  int omap_mcbsp_pollread(unsigned int id, u16 * buf);
  int omap_mcbsp_pollwrite(unsigned int id, u16 buf);
 +
 +int omap3_mcbsp_pollread(unsigned int id, u32 *buf);
 +int omap3_mcbsp_pollwrite(unsigned int id, u32 buf);
 +
  int omap_mcbsp_set_io_type(unsigned int id, omap_mcbsp_io_type_t io_type);
  
  #endif
 diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
 index b6b520d..1212d11 100644
 --- a/arch/arm/plat-omap/mcbsp.c
 +++ b/arch/arm/plat-omap/mcbsp.c
 @@ -425,7 +425,7 @@ void omap_mcbsp_stop(unsigned int id)
  }
  EXPORT_SYMBOL(omap_mcbsp_stop);
  
 -/* polled mcbsp i/o operations */
 +/* polled mcbsp i/o operations for omap1 and omap2420 */
  int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
  {
   struct omap_mcbsp *mcbsp;
 @@ -515,6 +515,96 @@ int omap_mcbsp_pollread(unsigned int id, u16 *buf)
  }
  EXPORT_SYMBOL(omap_mcbsp_pollread);
  
 +/* polled mcbsp i/o operations for omap3 */
 +int omap3_mcbsp_pollwrite(unsigned int id, u32 buf)
 +{
 + struct omap_mcbsp *mcbsp;
 + void __iomem *base;
 +
 + if (!omap_mcbsp_check_valid_id(id)) {
 + printk(KERN_ERR %s: Invalid id (%d)\n, __func__, id + 1);
 + return -ENODEV;
 + }
 +
 + mcbsp = id_to_mcbsp_ptr(id);
 + base = mcbsp-io_base;
 +
 + OMAP_MCBSP_WRITE(base, DXR, buf);
 + /* if frame sync error - clear the error */
 + if (OMAP_MCBSP_READ(base, SPCR2)  XSYNC_ERR) {
 + /* clear error */
 + OMAP_MCBSP_WRITE(base, SPCR2, OMAP_MCBSP_READ(base, SPCR2)
 +  (~XSYNC_ERR));
 + /* resend */
 + return -EPERM;
 + } else {
 + /* wait for transmit confirmation */
 + int attemps = 0;
 + while (!(OMAP_MCBSP_READ(base, SPCR2)  XRDY)) {
 + if (attemps++  1000) {
 + OMAP_MCBSP_WRITE(base, SPCR2,
 + OMAP_MCBSP_READ(base, SPCR2) 
 + (~XRST));
 + udelay(10);
 + OMAP_MCBSP_WRITE(base, SPCR2,
 + OMAP_MCBSP_READ(base, SPCR2) |
 + (XRST));
 + udelay(10);
 + dev_err(mcbsp-dev, Could not write to
 +  McBSP%d Register\n, mcbsp-id);
 + return -ENOENT;
 + }
 + }
 + }
 +
 + return 0;
 +}
 +EXPORT_SYMBOL(omap3_mcbsp_pollwrite);
 +
 +int omap3_mcbsp_pollread(unsigned int id, u32 *buf)
 +{
 + struct omap_mcbsp *mcbsp;
 + void __iomem *base;
 +
 + if (!omap_mcbsp_check_valid_id(id)) {
 + printk(KERN_ERR %s: Invalid id (%d)\n, __func__, id + 1);
 + return -ENODEV;
 + }
 + mcbsp = id_to_mcbsp_ptr(id);
 +
 + base = mcbsp-io_base;
 + /* if frame sync error - clear the error */
 + if (OMAP_MCBSP_READ(base, SPCR1)  RSYNC_ERR) {
 + /* clear error */
 + OMAP_MCBSP_WRITE(base, SPCR1, OMAP_MCBSP_READ(base, SPCR1)
 +  (~RSYNC_ERR));
 + /* resend */
 + return -EPERM;
 + } else {
 + /* wait for recieve confirmation */
 + int attemps = 0;
 + while (!(OMAP_MCBSP_READ(base, SPCR1)  RRDY)) {
 + if (attemps++  1000) {
 + OMAP_MCBSP_WRITE(base, SPCR1,
 +

[PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

2009-10-20 Thread charu

omap_mcbsp_pollwrite and omap_mcbsp_pollread functions access
McBSP registers as 16-bit registers.

In OMAP3, McBSP registers (DRR_REG and DXR_REG) are limited to
32-bit data accesses (L4 Interconnect). 16-bit and 8-bit is
not allowed and can corrupt register content.

This patch adds omap3_mcbsp_pollwrite and
omap3_mcbsp_pollread functions for OMAP3 cpu, inorder to
perform 32 bit access of above mentioned McBSP registers.

Signed-off-by: Charulatha V ch...@ti.com
Signed-off-by: Syed Rafiuddin rafiuddin.s...@ti.com
---
 arch/arm/plat-omap/include/mach/mcbsp.h |4 +
 arch/arm/plat-omap/mcbsp.c  |   92 ++-
 2 files changed, 95 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h 
b/arch/arm/plat-omap/include/mach/mcbsp.h
index 0c5f937..b0f9260 100644
--- a/arch/arm/plat-omap/include/mach/mcbsp.h
+++ b/arch/arm/plat-omap/include/mach/mcbsp.h
@@ -406,6 +406,10 @@ void omap_mcbsp_set_spi_mode(unsigned int id, const struct 
omap_mcbsp_spi_cfg *
 /* Polled read/write functions */
 int omap_mcbsp_pollread(unsigned int id, u16 * buf);
 int omap_mcbsp_pollwrite(unsigned int id, u16 buf);
+
+int omap3_mcbsp_pollread(unsigned int id, u32 *buf);
+int omap3_mcbsp_pollwrite(unsigned int id, u32 buf);
+
 int omap_mcbsp_set_io_type(unsigned int id, omap_mcbsp_io_type_t io_type);
 
 #endif
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index b6b520d..1212d11 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -425,7 +425,7 @@ void omap_mcbsp_stop(unsigned int id)
 }
 EXPORT_SYMBOL(omap_mcbsp_stop);
 
-/* polled mcbsp i/o operations */
+/* polled mcbsp i/o operations for omap1 and omap2420 */
 int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
 {
struct omap_mcbsp *mcbsp;
@@ -515,6 +515,96 @@ int omap_mcbsp_pollread(unsigned int id, u16 *buf)
 }
 EXPORT_SYMBOL(omap_mcbsp_pollread);
 
+/* polled mcbsp i/o operations for omap3 */
+int omap3_mcbsp_pollwrite(unsigned int id, u32 buf)
+{
+   struct omap_mcbsp *mcbsp;
+   void __iomem *base;
+
+   if (!omap_mcbsp_check_valid_id(id)) {
+   printk(KERN_ERR %s: Invalid id (%d)\n, __func__, id + 1);
+   return -ENODEV;
+   }
+
+   mcbsp = id_to_mcbsp_ptr(id);
+   base = mcbsp-io_base;
+
+   OMAP_MCBSP_WRITE(base, DXR, buf);
+   /* if frame sync error - clear the error */
+   if (OMAP_MCBSP_READ(base, SPCR2)  XSYNC_ERR) {
+   /* clear error */
+   OMAP_MCBSP_WRITE(base, SPCR2, OMAP_MCBSP_READ(base, SPCR2)
+(~XSYNC_ERR));
+   /* resend */
+   return -EPERM;
+   } else {
+   /* wait for transmit confirmation */
+   int attemps = 0;
+   while (!(OMAP_MCBSP_READ(base, SPCR2)  XRDY)) {
+   if (attemps++  1000) {
+   OMAP_MCBSP_WRITE(base, SPCR2,
+   OMAP_MCBSP_READ(base, SPCR2) 
+   (~XRST));
+   udelay(10);
+   OMAP_MCBSP_WRITE(base, SPCR2,
+   OMAP_MCBSP_READ(base, SPCR2) |
+   (XRST));
+   udelay(10);
+   dev_err(mcbsp-dev, Could not write to
+McBSP%d Register\n, mcbsp-id);
+   return -ENOENT;
+   }
+   }
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(omap3_mcbsp_pollwrite);
+
+int omap3_mcbsp_pollread(unsigned int id, u32 *buf)
+{
+   struct omap_mcbsp *mcbsp;
+   void __iomem *base;
+
+   if (!omap_mcbsp_check_valid_id(id)) {
+   printk(KERN_ERR %s: Invalid id (%d)\n, __func__, id + 1);
+   return -ENODEV;
+   }
+   mcbsp = id_to_mcbsp_ptr(id);
+
+   base = mcbsp-io_base;
+   /* if frame sync error - clear the error */
+   if (OMAP_MCBSP_READ(base, SPCR1)  RSYNC_ERR) {
+   /* clear error */
+   OMAP_MCBSP_WRITE(base, SPCR1, OMAP_MCBSP_READ(base, SPCR1)
+(~RSYNC_ERR));
+   /* resend */
+   return -EPERM;
+   } else {
+   /* wait for recieve confirmation */
+   int attemps = 0;
+   while (!(OMAP_MCBSP_READ(base, SPCR1)  RRDY)) {
+   if (attemps++  1000) {
+   OMAP_MCBSP_WRITE(base, SPCR1,
+   OMAP_MCBSP_READ(base, SPCR1) 
+   (~RRST));
+   udelay(10);
+   OMAP_MCBSP_WRITE(base, SPCR1,
+   OMAP_MCBSP_READ(base, SPCR1) |
+   (RRST));
+

RE: [PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

Re: [PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

Re: [PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

[PATCH V2] FIX OMAP3:McBSP poll read and write for OMAP3

4 matches

Site Navigation

Mail list logo

Footer information