[PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[Dmitry Antipov]

Since 'symbol_duration' of 'struct wpan_phy' is in nanoseconds but
'lifs_period' and 'sifs_period' are both in microseconds, fix time
calculation in 'ieee802154_configure_durations()' and use convenient
'NSEC_PER_USEC' in 'ieee802154_setup_wpan_phy_pib()' as well.
Compile tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 net/mac802154/main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 9ab7396668d2..21b7c3b280b4 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -161,8 +161,10 @@ void ieee802154_configure_durations(struct wpan_phy *phy,
 	}
 
 	phy->symbol_duration = duration;
-	phy->lifs_period = (IEEE802154_LIFS_PERIOD * phy->symbol_duration) / NSEC_PER_SEC;
-	phy->sifs_period = (IEEE802154_SIFS_PERIOD * phy->symbol_duration) / NSEC_PER_SEC;
+	phy->lifs_period =
+		(IEEE802154_LIFS_PERIOD * phy->symbol_duration) / NSEC_PER_USEC;
+	phy->sifs_period =
+		(IEEE802154_SIFS_PERIOD * phy->symbol_duration) / NSEC_PER_USEC;
 }
 EXPORT_SYMBOL(ieee802154_configure_durations);
 
@@ -184,10 +186,10 @@ static void ieee802154_setup_wpan_phy_pib(struct wpan_phy *wpan_phy)
 	 * Should be done when all drivers sets this value.
 	 */
 
-	wpan_phy->lifs_period =
-		(IEEE802154_LIFS_PERIOD * wpan_phy->symbol_duration) / 1000;
-	wpan_phy->sifs_period =
-		(IEEE802154_SIFS_PERIOD * wpan_phy->symbol_duration) / 1000;
+	wpan_phy->lifs_period =	(IEEE802154_LIFS_PERIOD *
+				 wpan_phy->symbol_duration) / NSEC_PER_USEC;
+	wpan_phy->sifs_period =	(IEEE802154_SIFS_PERIOD *
+				 wpan_phy->symbol_duration) / NSEC_PER_USEC;
 }
 
 int ieee802154_register_hw(struct ieee802154_hw *hw)
-- 
2.45.0

Re: [PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[Miquel Raynal]

Hi Dmitry,

+ Stefan

dmantipov@yandex.ru wrote on Wed,  8 May 2024 14:40:10 +0300:

> Since 'symbol_duration' of 'struct wpan_phy' is in nanoseconds but
> 'lifs_period' and 'sifs_period' are both in microseconds, fix time
> calculation in 'ieee802154_configure_durations()' and use convenient
> 'NSEC_PER_USEC' in 'ieee802154_setup_wpan_phy_pib()' as well.
> Compile tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 

Requires Fixes and Cc: stable I guess.

Otherwise LGTM,

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

This is also a candidate for wpan, so [PATCH wpan]. Stefan, Alex, who's
handling wpan this release?

Cheers,
Miquèl

Re: [PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[Alexander Aring]

Hi,

On Mon, May 13, 2024 at 3:09 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Dmitry,
>
> + Stefan
>
> dmantipov@yandex.ru wrote on Wed,  8 May 2024 14:40:10 +0300:
>
> > Since 'symbol_duration' of 'struct wpan_phy' is in nanoseconds but
> > 'lifs_period' and 'sifs_period' are both in microseconds, fix time
> > calculation in 'ieee802154_configure_durations()' and use convenient
> > 'NSEC_PER_USEC' in 'ieee802154_setup_wpan_phy_pib()' as well.
> > Compile tested only.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >
>
> Requires Fixes and Cc: stable I guess.
>
> Otherwise LGTM,
>
> Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> This is also a candidate for wpan, so [PATCH wpan]. Stefan, Alex, who's
> handling wpan this release?
>

thought Stefan handles still wpan, -next we rotate?

- Alex

Re: [PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[Stefan Schmidt]

Hello Miquel,

On 13.05.24 09:09, Miquel Raynal wrote:
> Hi Dmitry,
> 
> + Stefan
> 
> dmantipov@yandex.ru wrote on Wed,  8 May 2024 14:40:10 +0300:
> 
>> Since 'symbol_duration' of 'struct wpan_phy' is in nanoseconds but
>> 'lifs_period' and 'sifs_period' are both in microseconds, fix time
>> calculation in 'ieee802154_configure_durations()' and use convenient
>> 'NSEC_PER_USEC' in 'ieee802154_setup_wpan_phy_pib()' as well.
>> Compile tested only.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
> 
> Requires Fixes and Cc: stable I guess.

Fixes should be enough to get picked up. Added before pushing.

regards
Stefan Schmidt

Re: [PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[Miquel Raynal]

Hi Stefan,

stefan@datenfreihafen.org wrote on Sat, 18 May 2024 23:49:41 +0200:

> Hello Miquel,
> 
> On 13.05.24 09:09, Miquel Raynal wrote:
> > Hi Dmitry,
> > 
> > + Stefan
> > 
> > dmantipov@yandex.ru wrote on Wed,  8 May 2024 14:40:10 +0300:
> >   
> >> Since 'symbol_duration' of 'struct wpan_phy' is in nanoseconds but
> >> 'lifs_period' and 'sifs_period' are both in microseconds, fix time
> >> calculation in 'ieee802154_configure_durations()' and use convenient
> >> 'NSEC_PER_USEC' in 'ieee802154_setup_wpan_phy_pib()' as well.
> >> Compile tested only.
> >>
> >> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >>  
> > 
> > Requires Fixes and Cc: stable I guess.  
> 
> Fixes should be enough to get picked up. Added before pushing.

Actually, unless the process changed recently, IIRC we should add Cc:
stable when we think it should be backported (like this patch). The
stable team indeed introduced an algorithm to collect patches not
flagged with this tag "by mistake", but they ask us to continue Cc'ing
them. Of course a Fixes tag is a good hint for the algorithm that the
patch should be backported, and most of the time the patch will be
backported anyway. Also rules being often different in net/, I think
even net maintainers now comply with this and not longer do the
cherry-picks themselves.

But maybe this changed recently again and my just lost :-)

Thanks,
Miquèl

Re: [PATCH] mac802154: fix time calculation in ieee802154_configure_durations()

[Stefan Schmidt]

Hello Dmitry,

On 08.05.24 13:40, Dmitry Antipov wrote:
> Since 'symbol_duration' of 'struct wpan_phy' is in nanoseconds but
> 'lifs_period' and 'sifs_period' are both in microseconds, fix time
> calculation in 'ieee802154_configure_durations()' and use convenient
> 'NSEC_PER_USEC' in 'ieee802154_setup_wpan_phy_pib()' as well.
> Compile tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>   net/mac802154/main.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/net/mac802154/main.c b/net/mac802154/main.c
> index 9ab7396668d2..21b7c3b280b4 100644
> --- a/net/mac802154/main.c
> +++ b/net/mac802154/main.c
> @@ -161,8 +161,10 @@ void ieee802154_configure_durations(struct wpan_phy *phy,
>   	}
>   
>   	phy->symbol_duration = duration;
> -	phy->lifs_period = (IEEE802154_LIFS_PERIOD * phy->symbol_duration) / NSEC_PER_SEC;
> -	phy->sifs_period = (IEEE802154_SIFS_PERIOD * phy->symbol_duration) / NSEC_PER_SEC;
> +	phy->lifs_period =
> +		(IEEE802154_LIFS_PERIOD * phy->symbol_duration) / NSEC_PER_USEC;
> +	phy->sifs_period =
> +		(IEEE802154_SIFS_PERIOD * phy->symbol_duration) / NSEC_PER_USEC;
>   }
>   EXPORT_SYMBOL(ieee802154_configure_durations);
>   
> @@ -184,10 +186,10 @@ static void ieee802154_setup_wpan_phy_pib(struct wpan_phy *wpan_phy)
>   	 * Should be done when all drivers sets this value.
>   	 */
>   
> -	wpan_phy->lifs_period =
> -		(IEEE802154_LIFS_PERIOD * wpan_phy->symbol_duration) / 1000;
> -	wpan_phy->sifs_period =
> -		(IEEE802154_SIFS_PERIOD * wpan_phy->symbol_duration) / 1000;
> +	wpan_phy->lifs_period =	(IEEE802154_LIFS_PERIOD *
> +				 wpan_phy->symbol_duration) / NSEC_PER_USEC;
> +	wpan_phy->sifs_period =	(IEEE802154_SIFS_PERIOD *
> +				 wpan_phy->symbol_duration) / NSEC_PER_USEC;
>   }
>   
>   int ieee802154_register_hw(struct ieee802154_hw *hw)

I added a Fixes tag for you here before pushing so stable can decide if 
they want to pick this.

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

regards
Stefan Schmidt