On 03/12/2013 06:14 PM, Alexander Aring wrote:
> Hi,
>
> On Tue, Mar 12, 2013 at 05:19:31PM -0400, Alan Ott wrote:
>> On 02/23/2013 11:12 AM, Sascha Herrmann wrote:
>>> If interrupt is configured to edge type, do not disable irq in the isr.
>>> The at86rf230 resets the irq line only after the irq status register is
>>> read. Disabling the irq can lock the driver in situations where a irq is
>>> set by the radio while the driver is still reading the frame buffer.
>>> Additional the irq filter register is set to filter out all unused
>>> interrupts and the irq status register is read in the probe function to
>>> clear the irq line.
>>>
>>> Signed-off-by: Sascha Herrmann <sas...@ps.nvbi.de>
>>> ---
>>>  drivers/net/ieee802154/at86rf230.c |   29 ++++++++++++++++-------------
>>>  1 file changed, 16 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ieee802154/at86rf230.c 
>>> b/drivers/net/ieee802154/at86rf230.c
>>> index e892ec3..2abc8c7 100644
>>> --- a/drivers/net/ieee802154/at86rf230.c
>>> +++ b/drivers/net/ieee802154/at86rf230.c
>>> @@ -31,6 +31,7 @@
>>>  #include <linux/spi/spi.h>
>>>  #include <linux/spi/at86rf230.h>
>>>  #include <linux/skbuff.h>
>>> +#include <linux/irq.h>
>>>   #include <net/mac802154.h>
>>>  #include <net/wpan-phy.h>
>>> @@ -51,7 +52,7 @@ struct at86rf230_local {
>>>     struct ieee802154_dev *dev;
>>>     spinlock_t lock;
>>> -   bool irq_disabled;
>>> +   bool irq_busy;
>>>     bool is_tx;
>>>  };
>>>  @@ -544,7 +545,7 @@ at86rf230_xmit(struct ieee802154_dev *dev, struct 
>>> sk_buff *skb)
>>>     unsigned long flags;
>>>     spin_lock(&lp->lock);
>>> -   if  (lp->irq_disabled) {
>>> +   if  (lp->irq_busy) {
>>>             spin_unlock(&lp->lock);
>>>             return -EBUSY;
>>>     }
>>> @@ -658,20 +659,22 @@ static void at86rf230_irqwork(struct work_struct 
>>> *work)
>>>     }
>>>     spin_lock_irqsave(&lp->lock, flags);
>>> -   lp->irq_disabled = 0;
>>> +   lp->irq_busy = 0;
>>>     spin_unlock_irqrestore(&lp->lock, flags);
>>>  -  enable_irq(lp->spi->irq);
>>> +   if (!(lp->irq_type & IRQ_TYPE_EDGE_BOTH))
>>> +           enable_irq(lp->spi->irq);
>>>  }
>>>   static irqreturn_t at86rf230_isr(int irq, void *data)
>>>  {
>>>     struct at86rf230_local *lp = data;
>>>  -  disable_irq_nosync(irq);
>>> +   if (!(lp->irq_type & IRQ_TYPE_EDGE_BOTH))
>>> +           disable_irq_nosync(irq);
>>>     spin_lock(&lp->lock);
>>> -   lp->irq_disabled = 1;
>>> +   lp->irq_busy = 1;
>>>     spin_unlock(&lp->lock);
>>>     schedule_work(&lp->irqwork);
>>> @@ -701,12 +704,7 @@ static int at86rf230_hw_init(struct at86rf230_local 
>>> *lp)
>>>             dev_info(&lp->spi->dev, "Status: %02x\n", status);
>>>     }
>>>  -  rc = at86rf230_write_subreg(lp, SR_IRQ_MASK, 0xff); /* IRQ_TRX_UR |
>>> -                                                        * IRQ_CCA_ED |
>>> -                                                        * IRQ_TRX_END |
>>> -                                                        * IRQ_PLL_UNL |
>>> -                                                        * IRQ_PLL_LOCK
>>> -                                                        */
>>> +   rc = at86rf230_write_subreg(lp, SR_IRQ_MASK, IRQ_TRX_END);
>>>     if (rc)
>>>             return rc;
>>>  @@ -773,7 +771,7 @@ static int at86rf230_probe(struct spi_device *spi)
>>>  {
>>>     struct ieee802154_dev *dev;
>>>     struct at86rf230_local *lp;
>>> -   u8 man_id_0, man_id_1;
>>> +   u8 man_id_0, man_id_1, status;
>>>     int rc;
>>>     const char *chip;
>>>     int supported = 0;
>>> @@ -893,6 +891,11 @@ static int at86rf230_probe(struct spi_device *spi)
>>>     if (rc)
>>>             goto err_gpio_dir;
>>>  +  /* Read irq status register to reset irq line */
>>> +   rc = at86rf230_read_subreg(lp, RG_IRQ_STATUS, 0xff, 0, &status);
>>> +   if (rc)
>>> +           goto err_irq;
>>> +
>>>     rc = ieee802154_register_device(lp->dev);
>>>     if (rc)
>>>             goto err_irq;
>> There are quite a few + and - lines which lead with a stray space.
>>
>> Can anyone else here test this? Any other at86rf230 users? Alex, do you
>> have any time to give this a go?
>>
> That will be some difficult... I use a at86rf230 which is controlled by
> a microcontoller which has a spi<=>usb bridge. I modified this driver
> myself to get this working with spi<=>usb bridge(not ready for
> mainline yet).

What I mean is that it would be nice if someone else could test to make
sure that your new code doesn't break their setup. Really, it's just a
nice-to-have, since not many people are using this stuff anyway.

> I have a spi at86rf230 on a beaglebone at spi connected, but I can only
> test if breaks hardware or not. I have no equipment to debug it completely.
> I can check if I can communicate from beaglebone(with spi at86rf230
> connected) to one of my (spi<=>usb bridge) at86rf230.
> Let me know if that is okay for you.

It's not up to me really, but that's also a good test, since it's not
using any usb-spi funny-business. Are you using 3.8 on BeagleBone?

> Currently I have some other issues. For example the problem with
> addressing with a linklayer based linklocal (when SAM bit is 0x3)
> address. But I fixed that already.
>
> I did some(a huge) code cleanup in 6lowpan.c in lowpan_header_create and
> lowpan_process_data.
> I will send a RFC patch in few days. But I need to merge this patches with
> Tony's patches.


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Reply via email to