diff mbox

net: ieee802154: adf7242: Fix erroneous RX enable

Message ID 20180625134951.14227-1-alexandru.ardelean@analog.com (mailing list archive)
State Accepted
Headers show

Commit Message

Alexandru Ardelean June 25, 2018, 1:49 p.m. UTC 
From: Michael Hennerich <michael.hennerich@analog.com>

Only enable RX mode if the netdev is opened.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/net/ieee802154/adf7242.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Stefan Schmidt July 5, 2018, 9:30 a.m. UTC | #1 
Hello.

[Just back from a a few weeks leave and catching up]

On 25.06.2018 15:49, Alexandru Ardelean wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Only enable RX mode if the netdev is opened.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  drivers/net/ieee802154/adf7242.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> index 3e85e9500233..0a240a3f7817 100644
> --- a/drivers/net/ieee802154/adf7242.c
> +++ b/drivers/net/ieee802154/adf7242.c
> @@ -739,7 +739,10 @@ static int adf7242_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
>  	adf7242_write_reg(lp, REG_CH_FREQ1, freq >> 8);
>  	adf7242_write_reg(lp, REG_CH_FREQ2, freq >> 16);
>  
> -	return adf7242_cmd(lp, CMD_RC_RX);
> +	if (test_bit(FLAG_START, &lp->flags))
> +		return adf7242_cmd_rx(lp);
> +	else
> +		return adf7242_cmd(lp, CMD_RC_PHY_RDY);

What is the difference between the CMD_RC_PHY_RDY and the CMD_RC_RX
command here. From the commit log I would have expected the if check but
not a change in the issued command?

regards
Stefan Schmidt
--
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
Hennerich, Michael July 9, 2018, 8:51 a.m. UTC | #2 
On 05.07.2018 11:30, Stefan Schmidt wrote:
> Hello.
> 
> [Just back from a a few weeks leave and catching up]
> 
> On 25.06.2018 15:49, Alexandru Ardelean wrote:
>> From: Michael Hennerich <michael.hennerich@analog.com>
>>
>> Only enable RX mode if the netdev is opened.
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>> ---
>>   drivers/net/ieee802154/adf7242.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
>> index 3e85e9500233..0a240a3f7817 100644
>> --- a/drivers/net/ieee802154/adf7242.c
>> +++ b/drivers/net/ieee802154/adf7242.c
>> @@ -739,7 +739,10 @@ static int adf7242_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
>>   	adf7242_write_reg(lp, REG_CH_FREQ1, freq >> 8);
>>   	adf7242_write_reg(lp, REG_CH_FREQ2, freq >> 16);
>>   
>> -	return adf7242_cmd(lp, CMD_RC_RX);
>> +	if (test_bit(FLAG_START, &lp->flags))
>> +		return adf7242_cmd_rx(lp);
>> +	else
>> +		return adf7242_cmd(lp, CMD_RC_PHY_RDY);
> 
> What is the difference between the CMD_RC_PHY_RDY and the CMD_RC_RX
> command here. From the commit log I would have expected the if check but
> not a change in the issued command?


The device must be in PHY_RDY state while changing the channel.
The difference is that in CMD_RC_RX the device is in RX mode,
and it will generate receive interrupts. The issue was that the device 
erroneously entered this state even when the netdev was stopped.
This patch makes sure that the device stays in PHY_RDY state if the 
device isn't started.
Stefan Schmidt July 9, 2018, 8:52 a.m. UTC | #3 
Hello Michael.

On 09.07.2018 10:51, Michael Hennerich wrote:
> On 05.07.2018 11:30, Stefan Schmidt wrote:
>> Hello.
>>
>> [Just back from a a few weeks leave and catching up]
>>
>> On 25.06.2018 15:49, Alexandru Ardelean wrote:
>>> From: Michael Hennerich <michael.hennerich@analog.com>
>>>
>>> Only enable RX mode if the netdev is opened.
>>>
>>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>>> ---
>>>   drivers/net/ieee802154/adf7242.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ieee802154/adf7242.c
>>> b/drivers/net/ieee802154/adf7242.c
>>> index 3e85e9500233..0a240a3f7817 100644
>>> --- a/drivers/net/ieee802154/adf7242.c
>>> +++ b/drivers/net/ieee802154/adf7242.c
>>> @@ -739,7 +739,10 @@ static int adf7242_channel(struct ieee802154_hw
>>> *hw, u8 page, u8 channel)
>>>       adf7242_write_reg(lp, REG_CH_FREQ1, freq >> 8);
>>>       adf7242_write_reg(lp, REG_CH_FREQ2, freq >> 16);
>>>   -    return adf7242_cmd(lp, CMD_RC_RX);
>>> +    if (test_bit(FLAG_START, &lp->flags))
>>> +        return adf7242_cmd_rx(lp);
>>> +    else
>>> +        return adf7242_cmd(lp, CMD_RC_PHY_RDY);
>>
>> What is the difference between the CMD_RC_PHY_RDY and the CMD_RC_RX
>> command here. From the commit log I would have expected the if check but
>> not a change in the issued command?
> 
> 
> The device must be in PHY_RDY state while changing the channel.
> The difference is that in CMD_RC_RX the device is in RX mode,
> and it will generate receive interrupts. The issue was that the device
> erroneously entered this state even when the netdev was stopped.
> This patch makes sure that the device stays in PHY_RDY state if the
> device isn't started.

Thanks. That was not clear from the short commit message. Makes more
sense now.

regards
Stefan Schmidt
--
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
Stefan Schmidt July 9, 2018, 9:01 a.m. UTC | #4 
Hello Alexandru.

On 25.06.2018 15:49, Alexandru Ardelean wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Only enable RX mode if the netdev is opened.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  drivers/net/ieee802154/adf7242.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> index 3e85e9500233..0a240a3f7817 100644
> --- a/drivers/net/ieee802154/adf7242.c
> +++ b/drivers/net/ieee802154/adf7242.c
> @@ -739,7 +739,10 @@ static int adf7242_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
>  	adf7242_write_reg(lp, REG_CH_FREQ1, freq >> 8);
>  	adf7242_write_reg(lp, REG_CH_FREQ2, freq >> 16);
>  
> -	return adf7242_cmd(lp, CMD_RC_RX);
> +	if (test_bit(FLAG_START, &lp->flags))
> +		return adf7242_cmd_rx(lp);
> +	else
> +		return adf7242_cmd(lp, CMD_RC_PHY_RDY);
>  }
>  
>  static int adf7242_set_hw_addr_filt(struct ieee802154_hw *hw,
> 

This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!

regards
Stefan Schmidt
--
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
Alexandru Ardelean July 10, 2018, 6:57 a.m. UTC | #5 
On Mon, 2018-07-09 at 11:01 +0200, Stefan Schmidt wrote:
> Hello Alexandru.

> 

> On 25.06.2018 15:49, Alexandru Ardelean wrote:

> > From: Michael Hennerich <michael.hennerich@analog.com>

> > 

> > Only enable RX mode if the netdev is opened.

> > 

> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>

> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> > ---

> >  drivers/net/ieee802154/adf7242.c | 5 ++++-

> >  1 file changed, 4 insertions(+), 1 deletion(-)

> > 

> > diff --git a/drivers/net/ieee802154/adf7242.c

> > b/drivers/net/ieee802154/adf7242.c

> > index 3e85e9500233..0a240a3f7817 100644

> > --- a/drivers/net/ieee802154/adf7242.c

> > +++ b/drivers/net/ieee802154/adf7242.c

> > @@ -739,7 +739,10 @@ static int adf7242_channel(struct ieee802154_hw

> > *hw, u8 page, u8 channel)

> >  	adf7242_write_reg(lp, REG_CH_FREQ1, freq >> 8);

> >  	adf7242_write_reg(lp, REG_CH_FREQ2, freq >> 16);

> >  

> > -	return adf7242_cmd(lp, CMD_RC_RX);

> > +	if (test_bit(FLAG_START, &lp->flags))

> > +		return adf7242_cmd_rx(lp);

> > +	else

> > +		return adf7242_cmd(lp, CMD_RC_PHY_RDY);

> >  }

> >  

> >  static int adf7242_set_hw_addr_filt(struct ieee802154_hw *hw,

> > 

> 

> This patch has been applied to the wpan tree and will be

> part of the next pull request to net. Thanks!


Thanks

Sorry for the slowness in replying.
I meant to reply sooner to your question, but I needed to take a look
first.

Thanks again
Alex

> 

> regards

> Stefan Schmidt

>
diff mbox

Patch

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index 3e85e9500233..0a240a3f7817 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -739,7 +739,10 @@  static int adf7242_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 	adf7242_write_reg(lp, REG_CH_FREQ1, freq >> 8);
 	adf7242_write_reg(lp, REG_CH_FREQ2, freq >> 16);
 
-	return adf7242_cmd(lp, CMD_RC_RX);
+	if (test_bit(FLAG_START, &lp->flags))
+		return adf7242_cmd_rx(lp);
+	else
+		return adf7242_cmd(lp, CMD_RC_PHY_RDY);
 }
 
 static int adf7242_set_hw_addr_filt(struct ieee802154_hw *hw,