Hello.
On Sun, 2013-03-24 at 20:19, Alan Ott wrote:
> On 03/24/2013 09:40 AM, Stefan Schmidt wrote:
> > Implement the filter function to update short address, pan id and ieee
> > address on change. Allowing for hardware address filtering needed for
> > auto ACK.
> >
> > Signed-off-by: Stefan Schmidt <ste...@datenfreihafen.org>
> > ---
> > drivers/net/ieee802154/at86rf230.c | 45
> > ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 45 insertions(+)
> >
> > diff --git a/drivers/net/ieee802154/at86rf230.c
> > b/drivers/net/ieee802154/at86rf230.c
> > index fc1687e..10e79a4 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -619,6 +619,50 @@ err:
> > return -EINVAL;
> > }
> >
> > +static int
> > +at86rf230_set_hw_addr_filt(struct ieee802154_dev *dev,
> > + struct ieee802154_hw_addr_filt
> > *filt,
> > + unsigned long changed)
> > +{
> > + struct at86rf230_local *lp = dev->priv;
> > +
> > + might_sleep();
>
> There's nothing in this function that _explicitly_ sleeps. The SPI
> functions will sleep, but those functions will themselves call
> might_sleep(), so I don't think it's appropriate to do here.
Good catch. Indeed the spi write functions do their own might_sleep.
Will remove this when submitting the next time.
> > +
> > + if (changed & IEEE802515_AFILT_SADDR_CHANGED) {
> > + dev_info(&lp->spi->dev, "at86rf230_set_hw_addr_filt called for
> > saddr\n");
> > + __at86rf230_write(lp, RG_SHORT_ADDR_0, filt->short_addr &
> > 0xff); /* LSB */
> > + __at86rf230_write(lp, RG_SHORT_ADDR_1, (filt->short_addr >> 8)
> > & 0xff); /* MSB */
> > + }
> > +
> > + if (changed & IEEE802515_AFILT_PANID_CHANGED) {
> > + dev_info(&lp->spi->dev, "at86rf230_set_hw_addr_filt called for
> > pan id\n");
> > + __at86rf230_write(lp, RG_PAN_ID_0, filt->pan_id & 0xff); /* LSB
> > */
> > + __at86rf230_write(lp, RG_PAN_ID_1, (filt->pan_id >> 8) & 0xff);
> > /* MSB */
> > + }
> > +
> > + if (changed & IEEE802515_AFILT_IEEEADDR_CHANGED) {
> > + dev_info(&lp->spi->dev, "at86rf230_set_hw_addr_filt called ieee
> > addr\n");
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_0, filt->ieee_addr[7]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_1, filt->ieee_addr[6]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_2, filt->ieee_addr[5]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_3, filt->ieee_addr[4]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_4, filt->ieee_addr[3]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_5, filt->ieee_addr[2]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_6, filt->ieee_addr[1]);
> > + at86rf230_write_subreg(lp, SR_IEEE_ADDR_7, filt->ieee_addr[0]);
> > + }
> > +
> > + if (changed & IEEE802515_AFILT_PANC_CHANGED) {
> > + dev_info(&lp->spi->dev, "at86rf230_set_hw_addr_filt called panc
> > change\n");
> > + if (filt->pan_coord)
> > + at86rf230_write_subreg(lp, SR_AACK_I_AM_COORD, 1);
> > + else
> > + at86rf230_write_subreg(lp, SR_AACK_I_AM_COORD, 0);
> > + }
> > +
> > + return 0;
> > +}
>
> Some of those lines break the 80 column rule. You might get away with
> it. It's up to you if you want to try.
If the rule of thumb has not changed lately it depends on how ugly the
code would look when forcing the 80 char rule. It was always a nice
point for discussion. :)
I think the MSB and LSB comments can go easily. Breaking up the debug
print into two lines would be to ugly in my opnion. Maybe I can short
them. Will think about it before the next submit.
regards
Stefan Schmidt
------------------------------------------------------------------------------
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
