[PATCH bluetooth-next] at86rf230: increase sleep to off timings

[PATCH bluetooth-next] at86rf230: increase sleep to off timings

[Alexander Aring]

I expierenced when setting channel while sleep mode it didn't changed
the channel inside the hardware registers. Then I got another report of
an user which has similar issues.

I increased the sleep to off state change timing, which is according
at86rf233 at maximum 1000 us. After this change I got no similar effects
again.

I tried another option to wait on AWAKE_END irq, which can be used to
wait until the transceiver is awaked. I tested it and the IRQ took 4
seconds after starting state change. I don't believe it takes 4 seconds
to go into the TRX_OFF state from SLEEP state. The alternative is to
increase the timings which seems to work.

Cc: Oleg Hahm <oliver.hahm@inria.fr>
Signed-off-by: Alexander Aring <aar@pengutronix.de>
---
 drivers/net/ieee802154/at86rf230.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index cb9e9fe..9f10da6 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1340,7 +1340,7 @@ static struct at86rf2xx_chip_data at86rf233_data = {
 	.t_off_to_aack = 80,
 	.t_off_to_tx_on = 80,
 	.t_off_to_sleep = 35,
-	.t_sleep_to_off = 210,
+	.t_sleep_to_off = 1000,
 	.t_frame = 4096,
 	.t_p_ack = 545,
 	.rssi_base_val = -91,
@@ -1355,7 +1355,7 @@ static struct at86rf2xx_chip_data at86rf231_data = {
 	.t_off_to_aack = 110,
 	.t_off_to_tx_on = 110,
 	.t_off_to_sleep = 35,
-	.t_sleep_to_off = 380,
+	.t_sleep_to_off = 1000,
 	.t_frame = 4096,
 	.t_p_ack = 545,
 	.rssi_base_val = -91,
@@ -1370,7 +1370,7 @@ static struct at86rf2xx_chip_data at86rf212_data = {
 	.t_off_to_aack = 200,
 	.t_off_to_tx_on = 200,
 	.t_off_to_sleep = 35,
-	.t_sleep_to_off = 380,
+	.t_sleep_to_off = 1000,
 	.t_frame = 4096,
 	.t_p_ack = 545,
 	.rssi_base_val = -100,
-- 
2.8.0

Re: [PATCH bluetooth-next] at86rf230: increase sleep to off timings

[Stefan Schmidt]

Hello.

On 19/04/16 15:34, Alexander Aring wrote:
> I expierenced when setting channel while sleep mode it didn't changed
> the channel inside the hardware registers. Then I got another report of
> an user which has similar issues.
>
> I increased the sleep to off state change timing, which is according
> at86rf233 at maximum 1000 us. After this change I got no similar effects
> again.
>
> I tried another option to wait on AWAKE_END irq, which can be used to
> wait until the transceiver is awaked. I tested it and the IRQ took 4
> seconds after starting state change. I don't believe it takes 4 seconds
> to go into the TRX_OFF state from SLEEP state. The alternative is to
> increase the timings which seems to work.

Nicely spotted. Increasing the timing is fine here I think. Given the 
datasheet actually assumes a maximum of 1000 us we should allow for it 
as well.

Did you also check the maximum timing for 231 and 212 in the datasheet? 
Just to make sure they are not different (for example higher).
> Cc: Oleg Hahm <oliver.hahm@inria.fr>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> ---
>   drivers/net/ieee802154/at86rf230.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index cb9e9fe..9f10da6 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -1340,7 +1340,7 @@ static struct at86rf2xx_chip_data at86rf233_data = {
>   	.t_off_to_aack = 80,
>   	.t_off_to_tx_on = 80,
>   	.t_off_to_sleep = 35,
> -	.t_sleep_to_off = 210,
> +	.t_sleep_to_off = 1000,
>   	.t_frame = 4096,
>   	.t_p_ack = 545,
>   	.rssi_base_val = -91,
> @@ -1355,7 +1355,7 @@ static struct at86rf2xx_chip_data at86rf231_data = {
>   	.t_off_to_aack = 110,
>   	.t_off_to_tx_on = 110,
>   	.t_off_to_sleep = 35,
> -	.t_sleep_to_off = 380,
> +	.t_sleep_to_off = 1000,
>   	.t_frame = 4096,
>   	.t_p_ack = 545,
>   	.rssi_base_val = -91,
> @@ -1370,7 +1370,7 @@ static struct at86rf2xx_chip_data at86rf212_data = {
>   	.t_off_to_aack = 200,
>   	.t_off_to_tx_on = 200,
>   	.t_off_to_sleep = 35,
> -	.t_sleep_to_off = 380,
> +	.t_sleep_to_off = 1000,
>   	.t_frame = 4096,
>   	.t_p_ack = 545,
>   	.rssi_base_val = -100,

Reviewed-by: Stefan Schmidt<stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [PATCH bluetooth-next] at86rf230: increase sleep to off timings

[Alexander Aring]

Hi,

Am 04/19/2016 um 04:41 PM schrieb Stefan Schmidt:
> Hello.
>
> On 19/04/16 15:34, Alexander Aring wrote:
>> I expierenced when setting channel while sleep mode it didn't changed
>> the channel inside the hardware registers. Then I got another report of
>> an user which has similar issues.
>>
>> I increased the sleep to off state change timing, which is according
>> at86rf233 at maximum 1000 us. After this change I got no similar effects
>> again.
>>
>> I tried another option to wait on AWAKE_END irq, which can be used to
>> wait until the transceiver is awaked. I tested it and the IRQ took 4
>> seconds after starting state change. I don't believe it takes 4 seconds
>> to go into the TRX_OFF state from SLEEP state. The alternative is to
>> increase the timings which seems to work.
>
> Nicely spotted. Increasing the timing is fine here I think. Given the datasheet actually assumes a maximum of 1000 us we should allow for it as well.
>
> Did you also check the maximum timing for 231 and 212 in the datasheet? Just to make sure they are not different (for example higher).

No the datasheets differs much here, there are 380 us only. I set it now to 1 ms,
because it's not really a hotpath yet where this is used.

Alternative would be to use IRQ_CCA_ED, which is multi-funcional irq. That means
it can also used to indicate (IRQ_AWAKE) to be sure the transceiver is _really_
awake when doing spi cmds. I tested it and saw it took 4 seconds to get such IRQ
after state switch from SLEEP to TRX_OFF.

This occurs that commands like link up commands for the first time on one phy
for one interface can took above ~10 seconds. It's better we wait 1ms to be sure
and I think the IRQ_AWAKE mechanism isn't good to use in practice.

btw: I expierence similar issues with recalibrate PLL with registers to recalibrate the
PLL. But doing a state change to OFF and RX_ON/TX_ON will do the same and works
better.

- Alex

Re: [PATCH bluetooth-next] at86rf230: increase sleep to off timings

[Alexander Aring]

Am 04/19/2016 um 08:04 PM schrieb Alexander Aring:
> Hi,
>
> Am 04/19/2016 um 04:41 PM schrieb Stefan Schmidt:
>> Hello.
>>
>> On 19/04/16 15:34, Alexander Aring wrote:
>>> I expierenced when setting channel while sleep mode it didn't changed
>>> the channel inside the hardware registers. Then I got another report of
>>> an user which has similar issues.
>>>
>>> I increased the sleep to off state change timing, which is according
>>> at86rf233 at maximum 1000 us. After this change I got no similar effects
>>> again.
>>>
>>> I tried another option to wait on AWAKE_END irq, which can be used to
>>> wait until the transceiver is awaked. I tested it and the IRQ took 4
>>> seconds after starting state change. I don't believe it takes 4 seconds
>>> to go into the TRX_OFF state from SLEEP state. The alternative is to
>>> increase the timings which seems to work.
>> Nicely spotted. Increasing the timing is fine here I think. Given the datasheet actually assumes a maximum of 1000 us we should allow for it as well.
>>
>> Did you also check the maximum timing for 231 and 212 in the datasheet? Just to make sure they are not different (for example higher).
> No the datasheets differs much here, there are 380 us only. I set it now to 1 ms,
> because it's not really a hotpath yet where this is used.
>
> Alternative would be to use IRQ_CCA_ED, which is multi-funcional irq. That means
> it can also used to indicate (IRQ_AWAKE) to be sure the transceiver is _really_
> awake when doing spi cmds. I tested it and saw it took 4 seconds to get such IRQ

Also I saw a discussion on netdev that it's usual you cannot set PHY (link)
settings (in this case it was etherner, with ethtool)  when the interface is
down. [0]

I think they have similar hardware where the registers are not accessable
when the interface is down (means hardware is in sleep state).

Also a colleague means we should switch to pm calls and use dev_get/dev_put,
but for networking this differs, our sleep/awake states depends on interface is
down or up. If up it's in receive mode and is in sleep mode until all interfaces
for this phy are down.

Maybe we should discuss "when going into sleep mode" and "when allow
phy (link) settings". It is also maybe very transceiver specific because the
at86rf2xx simple doesn't allow register manipulating while sleep mode.
So far I know the mrf24j40 allows it. And [0] describes the issue for some
hardware such at86rf2xx.

- Alex

[0] http://marc.info/?l=systemd-devel&m=146042512722479&w=2

Re: [PATCH bluetooth-next] at86rf230: increase sleep to off timings

[Stefan Schmidt]

Hello.

On 19/04/16 20:04, Alexander Aring wrote:
> Hi,
>
> Am 04/19/2016 um 04:41 PM schrieb Stefan Schmidt:
>> Hello.
>>
>> On 19/04/16 15:34, Alexander Aring wrote:
>>> I expierenced when setting channel while sleep mode it didn't changed
>>> the channel inside the hardware registers. Then I got another report of
>>> an user which has similar issues.
>>>
>>> I increased the sleep to off state change timing, which is according
>>> at86rf233 at maximum 1000 us. After this change I got no similar effects
>>> again.
>>>
>>> I tried another option to wait on AWAKE_END irq, which can be used to
>>> wait until the transceiver is awaked. I tested it and the IRQ took 4
>>> seconds after starting state change. I don't believe it takes 4 seconds
>>> to go into the TRX_OFF state from SLEEP state. The alternative is to
>>> increase the timings which seems to work.
>> Nicely spotted. Increasing the timing is fine here I think. Given the datasheet actually assumes a maximum of 1000 us we should allow for it as well.
>>
>> Did you also check the maximum timing for 231 and 212 in the datasheet? Just to make sure they are not different (for example higher).
> No the datasheets differs much here, there are 380 us only. I set it now to 1 ms,
> because it's not really a hotpath yet where this is used.
>

I only was worried that the 231 or 212 need _longer_ than the 1ms for 
this. If the datasheet mentions a shorter maximum time this patch is 
fine as is.
As you said, its not a hotpath and we are ok with this 1ms for setting 
the channel.

I already gave my reviewed by and I think the patch is fine as is to be 
applied.

regards
Stefan Schmidt

Re: [PATCH bluetooth-next] at86rf230: increase sleep to off timings

[Marcel Holtmann]

Hi Alex,

> I expierenced when setting channel while sleep mode it didn't changed
> the channel inside the hardware registers. Then I got another report of
> an user which has similar issues.
> 
> I increased the sleep to off state change timing, which is according
> at86rf233 at maximum 1000 us. After this change I got no similar effects
> again.
> 
> I tried another option to wait on AWAKE_END irq, which can be used to
> wait until the transceiver is awaked. I tested it and the IRQ took 4
> seconds after starting state change. I don't believe it takes 4 seconds
> to go into the TRX_OFF state from SLEEP state. The alternative is to
> increase the timings which seems to work.
> 
> Cc: Oleg Hahm <oliver.hahm@inria.fr>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> ---
> drivers/net/ieee802154/at86rf230.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel