Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-08 Thread Girish KS
On Thu, Feb 7, 2013 at 5:04 PM, Girish KS girishks2...@gmail.com wrote:
 On Wed, Feb 6, 2013 at 3:48 PM, Grant Likely grant.lik...@secretlab.ca 
 wrote:
 On Wed, Feb 6, 2013 at 8:12 PM, Girish KS girishks2...@gmail.com wrote:
 On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca 
 wrote:
 On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S girishks2...@gmail.com 
 wrote:
 The status of the interrupt is available in the status register,
 so reading the clear pending register and writing back the same
 value will not actually clear the pending interrupts. This patch
 modifies the interrupt handler to read the status register and
 clear the corresponding pending bit in the clear pending register.

 Modified the hwInit function to clear all the pending interrupts.

 Signed-off-by: Girish K S ks.g...@samsung.com
 ---
  drivers/spi/spi-s3c64xx.c |   41 
 +
  1 file changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index ad93231..b770f88 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void 
 *data)
  {
   struct s3c64xx_spi_driver_data *sdd = data;
   struct spi_master *spi = sdd-master;
 - unsigned int val;
 + unsigned int val, clr = 0;

 - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + val = readl(sdd-regs + S3C64XX_SPI_STATUS);

 - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 -
 - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 -
 - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
 + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
 + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   dev_err(spi-dev, RX overrun\n);
 - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   dev_err(spi-dev, RX underrun\n);
 - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   dev_err(spi-dev, TX overrun\n);
 - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   dev_err(spi-dev, TX underrun\n);
 + }
 +
 + /* Clear the pending irq by setting and then clearing it */
 + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);

 Wait, what?  clr  ~clr == 0   Always.  What are you actually trying to do 
 here?
 The user manual says, wirting 1 to the pending clear register clears
 the interrupt (its not auto clear to 0). so i need to explicitly reset
 those bits thats what the 2nd write does

 Then write 0. That's the result of what the code does anyway, but the
 code as-written is nonsensical.
 i cannot write 0. because the 0th bit is trailing byte interrupt clear
 pending bit, which is not being handled in the handler.

Sorry, Writing 0  will still be valid after code for trainling byte is added..
will change and resubmit


 g.

--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-08 Thread Tomasz Figa
On Thursday 07 of February 2013 09:46:58 Girish KS wrote:
 On Thu, Feb 7, 2013 at 3:09 AM, Tomasz Figa t.f...@samsung.com wrote:
  Hi Girish,
  
  On Wednesday 06 of February 2013 12:12:29 Girish KS wrote:
  On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely
  grant.lik...@secretlab.ca 
  wrote:
   On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S
  
  girishks2...@gmail.com wrote:
   The status of the interrupt is available in the status register,
   so reading the clear pending register and writing back the same
   value will not actually clear the pending interrupts. This patch
   modifies the interrupt handler to read the status register and
   clear the corresponding pending bit in the clear pending register.
   
   Modified the hwInit function to clear all the pending interrupts.
   
   Signed-off-by: Girish K S ks.g...@samsung.com
   ---
   
drivers/spi/spi-s3c64xx.c |   41
+ 1 file changed, 25
insertions(+), 16 deletions(-)
   
   diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
   index ad93231..b770f88 100644
   --- a/drivers/spi/spi-s3c64xx.c
   +++ b/drivers/spi/spi-s3c64xx.c
   @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq,
   void *data)
   
{

 struct s3c64xx_spi_driver_data *sdd = data;
 struct spi_master *spi = sdd-master;
   
   - unsigned int val;
   + unsigned int val, clr = 0;
   
   - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
   + val = readl(sdd-regs + S3C64XX_SPI_STATUS);
   
   - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
   - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
   - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
   - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   -
   - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   -
   - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
   + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
   + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   
 dev_err(spi-dev, RX overrun\n);
   
   - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   
 dev_err(spi-dev, RX underrun\n);
   
   - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   
 dev_err(spi-dev, TX overrun\n);
   
   - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   
 dev_err(spi-dev, TX underrun\n);
   
   + }
   +
   + /* Clear the pending irq by setting and then clearing it */
   + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   
   Wait, what?  clr  ~clr == 0   Always.  What are you actually
   trying
   to do here?
  
  The user manual says, wirting 1 to the pending clear register clears
  the interrupt (its not auto clear to 0). so i need to explicitly
  reset
  those bits thats what the 2nd write does
  
  I have looked through user's manuals of different Samsung SoCs. All of
  them said that writing 1 to a bit clears the corresponding interrupt,
  but none of them contain any note that it must be manually cleared to
  0.
 What i meant was the clear pending bit will not clear automatically.
 When I set the
 clear pending bit, it remains set. This is a problem for the next
 interrupt cycle.

How did you check that it does not clear automatically?

  In addition the expression
  
  clr  ~clr
  
  makes no sense, because it is equal to 0.
 
 It makes sense, because we are not disturbing the interrupt pending
 bit at position 0, which is a trailing clr bit.

You either seem to misunderstand the problem I'm mentioning or not 
understanding it at all.

If you take a variable named clr, no matter what value it is set to, and 
you AND it with bitwise negation of the same variable, you will get 0.

See on this example:

Bits: 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
 ---
Values:   1 | 1 | 0 | 0 | 1 | 0 | 0 | 1
 ---
Negation: 0 | 0 | 1 | 1 | 0 | 1 | 1 | 0
 ---
AND:  0 | 0 | 0 | 0 | 0 | 0 | 0 | 0

Now, can you see that (clr  ~clr) is the same as (0)?

Best regards,
Tomasz


--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-08 Thread Girish KS
On Fri, Feb 8, 2013 at 12:33 AM, Tomasz Figa tomasz.f...@gmail.com wrote:
 On Thursday 07 of February 2013 09:46:58 Girish KS wrote:
 On Thu, Feb 7, 2013 at 3:09 AM, Tomasz Figa t.f...@samsung.com wrote:
  Hi Girish,
 
  On Wednesday 06 of February 2013 12:12:29 Girish KS wrote:
  On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely
  grant.lik...@secretlab.ca
  wrote:
   On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S
 
  girishks2...@gmail.com wrote:
   The status of the interrupt is available in the status register,
   so reading the clear pending register and writing back the same
   value will not actually clear the pending interrupts. This patch
   modifies the interrupt handler to read the status register and
   clear the corresponding pending bit in the clear pending register.
  
   Modified the hwInit function to clear all the pending interrupts.
  
   Signed-off-by: Girish K S ks.g...@samsung.com
   ---
  
drivers/spi/spi-s3c64xx.c |   41
+ 1 file changed, 25
insertions(+), 16 deletions(-)
  
   diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
   index ad93231..b770f88 100644
   --- a/drivers/spi/spi-s3c64xx.c
   +++ b/drivers/spi/spi-s3c64xx.c
   @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq,
   void *data)
  
{
  
 struct s3c64xx_spi_driver_data *sdd = data;
 struct spi_master *spi = sdd-master;
  
   - unsigned int val;
   + unsigned int val, clr = 0;
  
   - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
   + val = readl(sdd-regs + S3C64XX_SPI_STATUS);
  
   - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
   - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
   - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
   - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   -
   - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   -
   - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
   + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
   + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
  
 dev_err(spi-dev, RX overrun\n);
  
   - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
  
 dev_err(spi-dev, RX underrun\n);
  
   - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
  
 dev_err(spi-dev, TX overrun\n);
  
   - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
  
 dev_err(spi-dev, TX underrun\n);
  
   + }
   +
   + /* Clear the pending irq by setting and then clearing it */
   + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  
   Wait, what?  clr  ~clr == 0   Always.  What are you actually
   trying
   to do here?
 
  The user manual says, wirting 1 to the pending clear register clears
  the interrupt (its not auto clear to 0). so i need to explicitly
  reset
  those bits thats what the 2nd write does
 
  I have looked through user's manuals of different Samsung SoCs. All of
  them said that writing 1 to a bit clears the corresponding interrupt,
  but none of them contain any note that it must be manually cleared to
  0.
 What i meant was the clear pending bit will not clear automatically.
 When I set the
 clear pending bit, it remains set. This is a problem for the next
 interrupt cycle.

 How did you check that it does not clear automatically?
I checked it with trace32 debugger. Also  confirmed with the IP
validation engineer.

  In addition the expression
 
  clr  ~clr
 
  makes no sense, because it is equal to 0.

 It makes sense, because we are not disturbing the interrupt pending
 bit at position 0, which is a trailing clr bit.

 You either seem to misunderstand the problem I'm mentioning or not
 understanding it at all.

 If you take a variable named clr, no matter what value it is set to, and
 you AND it with bitwise negation of the same variable, you will get 0.

 See on this example:

 Bits: 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
  ---
 Values:   1 | 1 | 0 | 0 | 1 | 0 | 0 | 1
  ---
 Negation: 0 | 0 | 1 | 1 | 0 | 1 | 1 | 0
  ---
 AND:  0 | 0 | 0 | 0 | 0 | 0 | 0 | 0

 Now, can you see that (clr  ~clr) is the same as (0)?

Already apolozised for the same: will resubmit.


 Best regards,
 Tomasz


--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___

Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-08 Thread Girish KS
On Fri, Feb 8, 2013 at 12:58 AM, Girish KS girishks2...@gmail.com wrote:
 On Fri, Feb 8, 2013 at 12:33 AM, Tomasz Figa tomasz.f...@gmail.com wrote:
 On Thursday 07 of February 2013 09:46:58 Girish KS wrote:
 On Thu, Feb 7, 2013 at 3:09 AM, Tomasz Figa t.f...@samsung.com wrote:
  Hi Girish,
 
  On Wednesday 06 of February 2013 12:12:29 Girish KS wrote:
  On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely
  grant.lik...@secretlab.ca
  wrote:
   On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S
 
  girishks2...@gmail.com wrote:
   The status of the interrupt is available in the status register,
   so reading the clear pending register and writing back the same
   value will not actually clear the pending interrupts. This patch
   modifies the interrupt handler to read the status register and
   clear the corresponding pending bit in the clear pending register.
  
   Modified the hwInit function to clear all the pending interrupts.
  
   Signed-off-by: Girish K S ks.g...@samsung.com
   ---
  
drivers/spi/spi-s3c64xx.c |   41
+ 1 file changed, 25
insertions(+), 16 deletions(-)
  
   diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
   index ad93231..b770f88 100644
   --- a/drivers/spi/spi-s3c64xx.c
   +++ b/drivers/spi/spi-s3c64xx.c
   @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq,
   void *data)
  
{
  
 struct s3c64xx_spi_driver_data *sdd = data;
 struct spi_master *spi = sdd-master;
  
   - unsigned int val;
   + unsigned int val, clr = 0;
  
   - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
   + val = readl(sdd-regs + S3C64XX_SPI_STATUS);
  
   - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
   - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
   - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
   - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   -
   - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   -
   - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
   + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
   + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
  
 dev_err(spi-dev, RX overrun\n);
  
   - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
  
 dev_err(spi-dev, RX underrun\n);
  
   - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
  
 dev_err(spi-dev, TX overrun\n);
  
   - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
   + }
   + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
   + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
  
 dev_err(spi-dev, TX underrun\n);
  
   + }
   +
   + /* Clear the pending irq by setting and then clearing it */
   + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
   + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  
   Wait, what?  clr  ~clr == 0   Always.  What are you actually
   trying
   to do here?
 
  The user manual says, wirting 1 to the pending clear register clears
  the interrupt (its not auto clear to 0). so i need to explicitly
  reset
  those bits thats what the 2nd write does
 
  I have looked through user's manuals of different Samsung SoCs. All of
  them said that writing 1 to a bit clears the corresponding interrupt,
  but none of them contain any note that it must be manually cleared to
  0.
 What i meant was the clear pending bit will not clear automatically.
 When I set the
 clear pending bit, it remains set. This is a problem for the next
 interrupt cycle.

 How did you check that it does not clear automatically?
 I checked it with trace32 debugger. Also  confirmed with the IP
 validation engineer.

To be more clear. When i open the trace32 memory window (start with
SPI register's base address), the RX under run bit is set in the
status register. coz the debugger tries to read from the RX data
register. At that time i forcibly set the clear pending register. Once
i set it. It just remains set until i reset it. I consulted the
validation engineer after i saw this behaviour.

Thank you Thomas, Grant and Mark for you time.


  In addition the expression
 
  clr  ~clr
 
  makes no sense, because it is equal to 0.

 It makes sense, because we are not disturbing the interrupt pending
 bit at position 0, which is a trailing clr bit.

 You either seem to misunderstand the problem I'm mentioning or not
 understanding it at all.

 If you take a variable named clr, no matter what value it is set to, and
 you AND it with bitwise negation of the same variable, you will get 0.

 See on this example:

 Bits: 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
  ---
 Values:   1 | 1 | 0 | 0 | 1 | 0 | 0 | 1
  ---
 Negation: 0 | 0 | 1 | 1 | 0 | 1 | 1 | 0
  

Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-07 Thread Tomasz Figa
Hi Girish,

On Wednesday 06 of February 2013 12:12:29 Girish KS wrote:
 On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca 
wrote:
  On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S 
girishks2...@gmail.com wrote:
  The status of the interrupt is available in the status register,
  so reading the clear pending register and writing back the same
  value will not actually clear the pending interrupts. This patch
  modifies the interrupt handler to read the status register and
  clear the corresponding pending bit in the clear pending register.
  
  Modified the hwInit function to clear all the pending interrupts.
  
  Signed-off-by: Girish K S ks.g...@samsung.com
  ---
  
   drivers/spi/spi-s3c64xx.c |   41
   + 1 file changed, 25
   insertions(+), 16 deletions(-)
  
  diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
  index ad93231..b770f88 100644
  --- a/drivers/spi/spi-s3c64xx.c
  +++ b/drivers/spi/spi-s3c64xx.c
  @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq,
  void *data) 
   {
   
struct s3c64xx_spi_driver_data *sdd = data;
struct spi_master *spi = sdd-master;
  
  - unsigned int val;
  + unsigned int val, clr = 0;
  
  - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
  + val = readl(sdd-regs + S3C64XX_SPI_STATUS);
  
  - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
  - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
  - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
  - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
  -
  - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  -
  - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
  + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
  + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
  
dev_err(spi-dev, RX overrun\n);
  
  - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
  + }
  + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
  + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
  
dev_err(spi-dev, RX underrun\n);
  
  - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
  + }
  + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
  + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
  
dev_err(spi-dev, TX overrun\n);
  
  - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
  + }
  + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
  + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
  
dev_err(spi-dev, TX underrun\n);
  
  + }
  +
  + /* Clear the pending irq by setting and then clearing it */
  + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  
  Wait, what?  clr  ~clr == 0   Always.  What are you actually trying
  to do here?
 The user manual says, wirting 1 to the pending clear register clears
 the interrupt (its not auto clear to 0). so i need to explicitly reset
 those bits thats what the 2nd write does

I have looked through user's manuals of different Samsung SoCs. All of 
them said that writing 1 to a bit clears the corresponding interrupt, but 
none of them contain any note that it must be manually cleared to 0.

In addition the expression

clr  ~clr

makes no sense, because it is equal to 0.

If you really need to clear those bits manually (and I don't think so), 
you should replace this expression with 0.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center
SW Solution Development, Linux Platform


--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-07 Thread Girish KS
On Thu, Feb 7, 2013 at 3:09 AM, Tomasz Figa t.f...@samsung.com wrote:
 Hi Girish,

 On Wednesday 06 of February 2013 12:12:29 Girish KS wrote:
 On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca
 wrote:
  On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S
 girishks2...@gmail.com wrote:
  The status of the interrupt is available in the status register,
  so reading the clear pending register and writing back the same
  value will not actually clear the pending interrupts. This patch
  modifies the interrupt handler to read the status register and
  clear the corresponding pending bit in the clear pending register.
 
  Modified the hwInit function to clear all the pending interrupts.
 
  Signed-off-by: Girish K S ks.g...@samsung.com
  ---
 
   drivers/spi/spi-s3c64xx.c |   41
   + 1 file changed, 25
   insertions(+), 16 deletions(-)
 
  diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
  index ad93231..b770f88 100644
  --- a/drivers/spi/spi-s3c64xx.c
  +++ b/drivers/spi/spi-s3c64xx.c
  @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq,
  void *data)
   {
 
struct s3c64xx_spi_driver_data *sdd = data;
struct spi_master *spi = sdd-master;
 
  - unsigned int val;
  + unsigned int val, clr = 0;
 
  - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
  + val = readl(sdd-regs + S3C64XX_SPI_STATUS);
 
  - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
  - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
  - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
  - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
  -
  - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  -
  - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
  + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
  + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
 
dev_err(spi-dev, RX overrun\n);
 
  - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
  + }
  + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
  + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
 
dev_err(spi-dev, RX underrun\n);
 
  - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
  + }
  + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
  + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
 
dev_err(spi-dev, TX overrun\n);
 
  - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
  + }
  + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
  + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 
dev_err(spi-dev, TX underrun\n);
 
  + }
  +
  + /* Clear the pending irq by setting and then clearing it */
  + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
  + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 
  Wait, what?  clr  ~clr == 0   Always.  What are you actually trying
  to do here?
 The user manual says, wirting 1 to the pending clear register clears
 the interrupt (its not auto clear to 0). so i need to explicitly reset
 those bits thats what the 2nd write does

 I have looked through user's manuals of different Samsung SoCs. All of
 them said that writing 1 to a bit clears the corresponding interrupt, but
 none of them contain any note that it must be manually cleared to 0.
What i meant was the clear pending bit will not clear automatically.
When I set the
clear pending bit, it remains set. This is a problem for the next
interrupt cycle.
 In addition the expression

 clr  ~clr

 makes no sense, because it is equal to 0.
It makes sense, because we are not disturbing the interrupt pending
bit at position 0, which is a trailing clr bit.

 If you really need to clear those bits manually (and I don't think so),
 you should replace this expression with 0.
since we are not enabling (was not enabled in previous implementation)
trailing byte interrupt, and not handling the same in handler
we cannot write 0.


 Best regards,
 --
 Tomasz Figa
 Samsung Poland RD Center
 SW Solution Development, Linux Platform


--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-07 Thread Girish KS
On Wed, Feb 6, 2013 at 3:48 PM, Grant Likely grant.lik...@secretlab.ca wrote:
 On Wed, Feb 6, 2013 at 8:12 PM, Girish KS girishks2...@gmail.com wrote:
 On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca 
 wrote:
 On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S girishks2...@gmail.com 
 wrote:
 The status of the interrupt is available in the status register,
 so reading the clear pending register and writing back the same
 value will not actually clear the pending interrupts. This patch
 modifies the interrupt handler to read the status register and
 clear the corresponding pending bit in the clear pending register.

 Modified the hwInit function to clear all the pending interrupts.

 Signed-off-by: Girish K S ks.g...@samsung.com
 ---
  drivers/spi/spi-s3c64xx.c |   41 +
  1 file changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index ad93231..b770f88 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void 
 *data)
  {
   struct s3c64xx_spi_driver_data *sdd = data;
   struct spi_master *spi = sdd-master;
 - unsigned int val;
 + unsigned int val, clr = 0;

 - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + val = readl(sdd-regs + S3C64XX_SPI_STATUS);

 - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 -
 - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 -
 - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
 + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
 + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   dev_err(spi-dev, RX overrun\n);
 - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   dev_err(spi-dev, RX underrun\n);
 - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   dev_err(spi-dev, TX overrun\n);
 - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   dev_err(spi-dev, TX underrun\n);
 + }
 +
 + /* Clear the pending irq by setting and then clearing it */
 + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);

 Wait, what?  clr  ~clr == 0   Always.  What are you actually trying to do 
 here?
 The user manual says, wirting 1 to the pending clear register clears
 the interrupt (its not auto clear to 0). so i need to explicitly reset
 those bits thats what the 2nd write does

 Then write 0. That's the result of what the code does anyway, but the
 code as-written is nonsensical.
i cannot write 0. because the 0th bit is trailing byte interrupt clear
pending bit, which is not being handled in the handler.

 g.

--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-06 Thread Grant Likely
On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S girishks2...@gmail.com wrote:
 The status of the interrupt is available in the status register,
 so reading the clear pending register and writing back the same
 value will not actually clear the pending interrupts. This patch
 modifies the interrupt handler to read the status register and
 clear the corresponding pending bit in the clear pending register.
 
 Modified the hwInit function to clear all the pending interrupts.
 
 Signed-off-by: Girish K S ks.g...@samsung.com
 ---
  drivers/spi/spi-s3c64xx.c |   41 +
  1 file changed, 25 insertions(+), 16 deletions(-)
 
 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index ad93231..b770f88 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
  {
   struct s3c64xx_spi_driver_data *sdd = data;
   struct spi_master *spi = sdd-master;
 - unsigned int val;
 + unsigned int val, clr = 0;
  
 - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + val = readl(sdd-regs + S3C64XX_SPI_STATUS);
  
 - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 -
 - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 -
 - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
 + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
 + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   dev_err(spi-dev, RX overrun\n);
 - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   dev_err(spi-dev, RX underrun\n);
 - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   dev_err(spi-dev, TX overrun\n);
 - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   dev_err(spi-dev, TX underrun\n);
 + }
 +
 + /* Clear the pending irq by setting and then clearing it */
 + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);

Wait, what?  clr  ~clr == 0   Always.  What are you actually trying to do here?

  
   return IRQ_HANDLED;
  }
 @@ -1039,9 +1044,13 @@ static void s3c64xx_spi_hwinit(struct 
 s3c64xx_spi_driver_data *sdd, int channel)
   writel(0, regs + S3C64XX_SPI_MODE_CFG);
   writel(0, regs + S3C64XX_SPI_PACKET_CNT);
  
 - /* Clear any irq pending bits */
 - writel(readl(regs + S3C64XX_SPI_PENDING_CLR),
 - regs + S3C64XX_SPI_PENDING_CLR);
 + /* Clear any irq pending bits, should set and clear the bits */
 + val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 + S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 + S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 + S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 + writel(val, regs + S3C64XX_SPI_PENDING_CLR);
 + writel(val  ~val, regs + S3C64XX_SPI_PENDING_CLR);

Ditto.

g.

--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-06 Thread Girish KS
On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca wrote:
 On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S girishks2...@gmail.com wrote:
 The status of the interrupt is available in the status register,
 so reading the clear pending register and writing back the same
 value will not actually clear the pending interrupts. This patch
 modifies the interrupt handler to read the status register and
 clear the corresponding pending bit in the clear pending register.

 Modified the hwInit function to clear all the pending interrupts.

 Signed-off-by: Girish K S ks.g...@samsung.com
 ---
  drivers/spi/spi-s3c64xx.c |   41 +
  1 file changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index ad93231..b770f88 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
  {
   struct s3c64xx_spi_driver_data *sdd = data;
   struct spi_master *spi = sdd-master;
 - unsigned int val;
 + unsigned int val, clr = 0;

 - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + val = readl(sdd-regs + S3C64XX_SPI_STATUS);

 - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 -
 - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 -
 - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
 + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
 + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   dev_err(spi-dev, RX overrun\n);
 - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   dev_err(spi-dev, RX underrun\n);
 - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   dev_err(spi-dev, TX overrun\n);
 - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   dev_err(spi-dev, TX underrun\n);
 + }
 +
 + /* Clear the pending irq by setting and then clearing it */
 + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);

 Wait, what?  clr  ~clr == 0   Always.  What are you actually trying to do 
 here?
The user manual says, wirting 1 to the pending clear register clears
the interrupt (its not auto clear to 0). so i need to explicitly reset
those bits thats what the 2nd write does


   return IRQ_HANDLED;
  }
 @@ -1039,9 +1044,13 @@ static void s3c64xx_spi_hwinit(struct 
 s3c64xx_spi_driver_data *sdd, int channel)
   writel(0, regs + S3C64XX_SPI_MODE_CFG);
   writel(0, regs + S3C64XX_SPI_PACKET_CNT);

 - /* Clear any irq pending bits */
 - writel(readl(regs + S3C64XX_SPI_PENDING_CLR),
 - regs + S3C64XX_SPI_PENDING_CLR);
 + /* Clear any irq pending bits, should set and clear the bits */
 + val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 + S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 + S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 + S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 + writel(val, regs + S3C64XX_SPI_PENDING_CLR);
 + writel(val  ~val, regs + S3C64XX_SPI_PENDING_CLR);

 Ditto.
same as above

 g.

--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-06 Thread Grant Likely
On Wed, Feb 6, 2013 at 8:12 PM, Girish KS girishks2...@gmail.com wrote:
 On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca 
 wrote:
 On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S girishks2...@gmail.com 
 wrote:
 The status of the interrupt is available in the status register,
 so reading the clear pending register and writing back the same
 value will not actually clear the pending interrupts. This patch
 modifies the interrupt handler to read the status register and
 clear the corresponding pending bit in the clear pending register.

 Modified the hwInit function to clear all the pending interrupts.

 Signed-off-by: Girish K S ks.g...@samsung.com
 ---
  drivers/spi/spi-s3c64xx.c |   41 +
  1 file changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index ad93231..b770f88 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void 
 *data)
  {
   struct s3c64xx_spi_driver_data *sdd = data;
   struct spi_master *spi = sdd-master;
 - unsigned int val;
 + unsigned int val, clr = 0;

 - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + val = readl(sdd-regs + S3C64XX_SPI_STATUS);

 - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 -
 - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 -
 - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
 + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
 + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   dev_err(spi-dev, RX overrun\n);
 - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   dev_err(spi-dev, RX underrun\n);
 - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   dev_err(spi-dev, TX overrun\n);
 - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   dev_err(spi-dev, TX underrun\n);
 + }
 +
 + /* Clear the pending irq by setting and then clearing it */
 + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);

 Wait, what?  clr  ~clr == 0   Always.  What are you actually trying to do 
 here?
 The user manual says, wirting 1 to the pending clear register clears
 the interrupt (its not auto clear to 0). so i need to explicitly reset
 those bits thats what the 2nd write does

Then write 0. That's the result of what the code does anyway, but the
code as-written is nonsensical.

g.

--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-06 Thread Girish KS
On Wed, Feb 6, 2013 at 3:48 PM, Grant Likely grant.lik...@secretlab.ca wrote:
 On Wed, Feb 6, 2013 at 8:12 PM, Girish KS girishks2...@gmail.com wrote:
 On Wed, Feb 6, 2013 at 2:26 AM, Grant Likely grant.lik...@secretlab.ca 
 wrote:
 On Tue,  5 Feb 2013 15:09:41 -0800, Girish K S girishks2...@gmail.com 
 wrote:
 The status of the interrupt is available in the status register,
 so reading the clear pending register and writing back the same
 value will not actually clear the pending interrupts. This patch
 modifies the interrupt handler to read the status register and
 clear the corresponding pending bit in the clear pending register.

 Modified the hwInit function to clear all the pending interrupts.

 Signed-off-by: Girish K S ks.g...@samsung.com
 ---
  drivers/spi/spi-s3c64xx.c |   41 +
  1 file changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
 index ad93231..b770f88 100644
 --- a/drivers/spi/spi-s3c64xx.c
 +++ b/drivers/spi/spi-s3c64xx.c
 @@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void 
 *data)
  {
   struct s3c64xx_spi_driver_data *sdd = data;
   struct spi_master *spi = sdd-master;
 - unsigned int val;
 + unsigned int val, clr = 0;

 - val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + val = readl(sdd-regs + S3C64XX_SPI_STATUS);

 - val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
 - S3C64XX_SPI_PND_TX_OVERRUN_CLR |
 - S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
 -
 - writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 -
 - if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
 + if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
 + clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
   dev_err(spi-dev, RX overrun\n);
 - if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
   dev_err(spi-dev, RX underrun\n);
 - if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
   dev_err(spi-dev, TX overrun\n);
 - if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
 + }
 + if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
 + clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
   dev_err(spi-dev, TX underrun\n);
 + }
 +
 + /* Clear the pending irq by setting and then clearing it */
 + writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 + writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);

 Wait, what?  clr  ~clr == 0   Always.  What are you actually trying to do 
 here?
 The user manual says, wirting 1 to the pending clear register clears
 the interrupt (its not auto clear to 0). so i need to explicitly reset
 those bits thats what the 2nd write does

 Then write 0. That's the result of what the code does anyway, but the
 code as-written is nonsensical.
Just writing 0 doest clear the status bit. The status bit in the
status register is cleared only when the corresponding bit in clear
pending register is set.
If not cleared after setting. On the next Interrupt, writing 1 to a
previously set bit will not clear the status bit.
Hope its clear

 g.

--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


[PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init

2013-02-05 Thread Girish K S
The status of the interrupt is available in the status register,
so reading the clear pending register and writing back the same
value will not actually clear the pending interrupts. This patch
modifies the interrupt handler to read the status register and
clear the corresponding pending bit in the clear pending register.

Modified the hwInit function to clear all the pending interrupts.

Signed-off-by: Girish K S ks.g...@samsung.com
---
 drivers/spi/spi-s3c64xx.c |   41 +
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index ad93231..b770f88 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -997,25 +997,30 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
 {
struct s3c64xx_spi_driver_data *sdd = data;
struct spi_master *spi = sdd-master;
-   unsigned int val;
+   unsigned int val, clr = 0;
 
-   val = readl(sdd-regs + S3C64XX_SPI_PENDING_CLR);
+   val = readl(sdd-regs + S3C64XX_SPI_STATUS);
 
-   val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
-   S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
-   S3C64XX_SPI_PND_TX_OVERRUN_CLR |
-   S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
-
-   writel(val, sdd-regs + S3C64XX_SPI_PENDING_CLR);
-
-   if (val  S3C64XX_SPI_PND_RX_OVERRUN_CLR)
+   if (val  S3C64XX_SPI_ST_RX_OVERRUN_ERR) {
+   clr = S3C64XX_SPI_PND_RX_OVERRUN_CLR;
dev_err(spi-dev, RX overrun\n);
-   if (val  S3C64XX_SPI_PND_RX_UNDERRUN_CLR)
+   }
+   if (val  S3C64XX_SPI_ST_RX_UNDERRUN_ERR) {
+   clr |= S3C64XX_SPI_PND_RX_UNDERRUN_CLR;
dev_err(spi-dev, RX underrun\n);
-   if (val  S3C64XX_SPI_PND_TX_OVERRUN_CLR)
+   }
+   if (val  S3C64XX_SPI_ST_TX_OVERRUN_ERR) {
+   clr |= S3C64XX_SPI_PND_TX_OVERRUN_CLR;
dev_err(spi-dev, TX overrun\n);
-   if (val  S3C64XX_SPI_PND_TX_UNDERRUN_CLR)
+   }
+   if (val  S3C64XX_SPI_ST_TX_UNDERRUN_ERR) {
+   clr |= S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
dev_err(spi-dev, TX underrun\n);
+   }
+
+   /* Clear the pending irq by setting and then clearing it */
+   writel(clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
+   writel(clr  ~clr, sdd-regs + S3C64XX_SPI_PENDING_CLR);
 
return IRQ_HANDLED;
 }
@@ -1039,9 +1044,13 @@ static void s3c64xx_spi_hwinit(struct 
s3c64xx_spi_driver_data *sdd, int channel)
writel(0, regs + S3C64XX_SPI_MODE_CFG);
writel(0, regs + S3C64XX_SPI_PACKET_CNT);
 
-   /* Clear any irq pending bits */
-   writel(readl(regs + S3C64XX_SPI_PENDING_CLR),
-   regs + S3C64XX_SPI_PENDING_CLR);
+   /* Clear any irq pending bits, should set and clear the bits */
+   val = S3C64XX_SPI_PND_RX_OVERRUN_CLR |
+   S3C64XX_SPI_PND_RX_UNDERRUN_CLR |
+   S3C64XX_SPI_PND_TX_OVERRUN_CLR |
+   S3C64XX_SPI_PND_TX_UNDERRUN_CLR;
+   writel(val, regs + S3C64XX_SPI_PENDING_CLR);
+   writel(val  ~val, regs + S3C64XX_SPI_PENDING_CLR);
 
writel(0, regs + S3C64XX_SPI_SWAP_CFG);
 
-- 
1.7.10.4


--
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general