net: ieee802154: mrf24j40: avoid uninitialized byte in SPI transfer to radio.
diff mbox

Message ID 20160712032051.F2D441C012F@csi-office.componentsw.com
State Under Review
Headers show

Commit Message

Walter Mack July 12, 2016, 3:02 a.m. UTC
isr function issues SPI read command to mrf to obtain INTSTAT.
SPI transfer is 2 bytes, but value of 2nd byte is not defined.
This had the effect that only the first ISR worked as intended. The
second ISR read incorrect INTSTAT values. Observed on Raspberry PI B+.
Signed-off-by: Walter Mack <wmack@componentsw.com>
---
 drivers/net/ieee802154/mrf24j40.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Alexander Aring July 12, 2016, 9:45 p.m. UTC | #1
Hi,

On 07/12/2016 05:02 AM, Walter Mack wrote:
> isr function issues SPI read command to mrf to obtain INTSTAT.
> SPI transfer is 2 bytes, but value of 2nd byte is not defined.
> This had the effect that only the first ISR worked as intended. The
> second ISR read incorrect INTSTAT values. Observed on Raspberry PI B+.
> Signed-off-by: Walter Mack <wmack@componentsw.com>
> ---
>  drivers/net/ieee802154/mrf24j40.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index f446db8..7b131f8 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -1054,6 +1054,8 @@ static irqreturn_t mrf24j40_isr(int irq, void *data)
>  	disable_irq_nosync(irq);
>  
>  	devrec->irq_buf[0] = MRF24J40_READSHORT(REG_INTSTAT);
> +	devrec->irq_buf[1] = 0;
> +

thanks.

I think it would be better to not use global resources for hard irq
here. We should change that code to similar stuff in at86rf230 driver,
see [0].

So each irq as their own resource data. Note you still need to disable
irq stuff because LEVEL triggered irq's and irqline will be cleared
until INSTAT is readed out.

- Alex

[0] http://lxr.free-electrons.com/source/drivers/net/ieee802154/at86rf230.c#L828
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index f446db8..7b131f8 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -1054,6 +1054,8 @@  static irqreturn_t mrf24j40_isr(int irq, void *data)
 	disable_irq_nosync(irq);
 
 	devrec->irq_buf[0] = MRF24J40_READSHORT(REG_INTSTAT);
+	devrec->irq_buf[1] = 0;
+
 	/* Read the interrupt status */
 	ret = spi_async(devrec->spi, &devrec->irq_msg);
 	if (ret) {