On 27/01/14 20:03, Phoebe Buckheister wrote:
> The standard assigns channel 0 on page 2 to be 100kbps QPSK in the
> 868.3MHz band. Add support to the at86rf230 driver for this channel and
> page, at the moment predicated only for the RF212 chip.

I have the feeling that this be better split into two different
functions - one for rf230/rf231, one for rf212. Thus we would have
no ugly nested conditions.

> Signed-off-by: Phoebe Buckheister <phoebe.buckheis...@itwm.fraunhofer.de>
> ---
>  drivers/net/ieee802154/at86rf230.c |   26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ieee802154/at86rf230.c
b/drivers/net/ieee802154/at86rf230.c
> index 01aeaf9..10a4647 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -105,7 +105,10 @@ static inline int is_rf212(struct at86rf230_local
*local)
>  #define      SR_SFD_VALUE            0x0b, 0xff, 0
>  #define      RG_TRX_CTRL_2   (0x0c)
>  #define      SR_OQPSK_DATA_RATE      0x0c, 0x03, 0
> -#define      SR_RESERVED_0c_2        0x0c, 0x7c, 2
> +#define      SR_SUB_MODE             0x0c, 0x04, 2
> +#define      SR_BPSK_QPSK            0x0c, 0x08, 3
> +#define      SR_OQPSK_SUB1_RC_EN     0x0c, 0x10, 4
> +#define      SR_RESERVED_0c_4        0x0c, 0x60, 5
>  #define      SR_RX_SAFE_MODE         0x0c, 0x80, 7
>  #define      RG_ANT_DIV      (0x0d)
>  #define      SR_ANT_CTRL             0x0d, 0x03, 0
> @@ -532,7 +535,7 @@ at86rf230_channel(struct ieee802154_dev *dev, int
page, int channel)
>
>       might_sleep();
>
> -     if (page != 0
> +     if ((page != 0 && !(is_rf212(lp) && page == 2 && channel == 0))
>               || (!is_rf212(lp) && (channel < 11 || channel > 26))
>               || (is_rf212(lp) && (channel < 0 || channel > 10))) {
>               WARN_ON(1);

I think this is better expressed as
->channels_supported[page] & BIT(channel).
provided that 'page' is correct and safe itself.

> @@ -540,8 +543,26 @@ at86rf230_channel(struct ieee802154_dev *dev, int
page, int channel)
>       }
>
>       rc = at86rf230_write_subreg(lp, SR_CHANNEL, channel);
> +     if (rc)
> +             return rc;
> +
> +     if (page == 2) {
> +             rc = at86rf230_write_subreg(lp, SR_BPSK_QPSK, 1);
> +             if (rc)
> +                     return rc;
> +
> +             rc = at86rf230_write_subreg(lp, SR_SUB_MODE, 0);
> +             if (rc)
> +                     return rc;
> +
> +             rc = at86rf230_write_subreg(lp, SR_OQPSK_DATA_RATE, 0);
> +             if (rc)
> +                     return rc;
> +     }

Who would switch back to BPSK, when phy is asked to switch to page 0?

> +
>       msleep(1); /* Wait for PLL */
>       dev->phy->current_channel = channel;
> +     dev->phy->current_page = page;
>
>       return 0;
>  }
> @@ -988,6 +1009,7 @@ static int at86rf230_probe(struct spi_device *spi)
>
>       if (is_rf212(lp)) {
>               dev->phy->channels_supported[0] = 0x00007FF;
> +             dev->phy->channels_supported[2] = 0x0000001;

Hmm. Aren't channels 1-10 from page 2 supported (O-QPSK @915 Mhz supported?)

Also do you plan to support O-QPSK @ 750 Mhz (802.15.4c - channels 0-3
from page 5)?

--
With best wishes
Dmitry


------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Reply via email to