| Message ID | 20220125121426.848337-2-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ieee802154: A bunch of fixes | expand |
Hello. On 25.01.22 13:14, Miquel Raynal wrote: > Drivers are expected to set the PHY current_channel and current_page > according to their default state. The hwsim driver is advertising being > configured on channel 13 by default but that is not reflected in its own > internal pib structure. In order to ensure that this driver consider the > current channel as being 13 internally, we at least need to set the > pib->channel field to 13. > > Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/net/ieee802154/mac802154_hwsim.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c > index 8caa61ec718f..00ec188a3257 100644 > --- a/drivers/net/ieee802154/mac802154_hwsim.c > +++ b/drivers/net/ieee802154/mac802154_hwsim.c > @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, > goto err_pib; > } > > + pib->page = 13; You want to set channel not page here. > rcu_assign_pointer(phy->pib, pib); > phy->idx = idx; > INIT_LIST_HEAD(&phy->edges); > regards Stefan Schmidt
Hi Stefan, stefan@datenfreihafen.org wrote on Tue, 25 Jan 2022 15:28:11 +0100: > Hello. > > On 25.01.22 13:14, Miquel Raynal wrote: > > Drivers are expected to set the PHY current_channel and current_page > > according to their default state. The hwsim driver is advertising being > > configured on channel 13 by default but that is not reflected in its own > > internal pib structure. In order to ensure that this driver consider the > > current channel as being 13 internally, we at least need to set the > > pib->channel field to 13. > > > > Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/net/ieee802154/mac802154_hwsim.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c > > index 8caa61ec718f..00ec188a3257 100644 > > --- a/drivers/net/ieee802154/mac802154_hwsim.c > > +++ b/drivers/net/ieee802154/mac802154_hwsim.c > > @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, > > goto err_pib; > > } > > > + pib->page = 13; > > You want to set channel not page here. Oh crap /o\ I've messed that update badly. Of course I meant pib->channel here, as it is in the commit log. I'll wait for Alexander's feedback before re-spinning. Unless the rest looks good for you both, I don't know if your policy allows you to fix it when applying, anyhow I'll do what is necessary. Thanks, Miquèl
Hello. On 25.01.22 17:48, Miquel Raynal wrote: > Hi Stefan, > > stefan@datenfreihafen.org wrote on Tue, 25 Jan 2022 15:28:11 +0100: > >> Hello. >> >> On 25.01.22 13:14, Miquel Raynal wrote: >>> Drivers are expected to set the PHY current_channel and current_page >>> according to their default state. The hwsim driver is advertising being >>> configured on channel 13 by default but that is not reflected in its own >>> internal pib structure. In order to ensure that this driver consider the >>> current channel as being 13 internally, we at least need to set the >>> pib->channel field to 13. >>> >>> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") >>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> >>> --- >>> drivers/net/ieee802154/mac802154_hwsim.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c >>> index 8caa61ec718f..00ec188a3257 100644 >>> --- a/drivers/net/ieee802154/mac802154_hwsim.c >>> +++ b/drivers/net/ieee802154/mac802154_hwsim.c >>> @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, >>> goto err_pib; >>> } >>> > + pib->page = 13; >> >> You want to set channel not page here. > > Oh crap /o\ I've messed that update badly. Of course I meant > pib->channel here, as it is in the commit log. > > I'll wait for Alexander's feedback before re-spinning. Unless the rest > looks good for you both, I don't know if your policy allows you to fix > it when applying, anyhow I'll do what is necessary. If Alex has nothing else and there is no re-spin I fix this when applying, no worries. regards Stefan Schmidt
Hi, On Wed, Jan 26, 2022 at 8:38 AM Stefan Schmidt <stefan@datenfreihafen.org> wrote: > > > Hello. > > On 25.01.22 17:48, Miquel Raynal wrote: > > Hi Stefan, > > > > stefan@datenfreihafen.org wrote on Tue, 25 Jan 2022 15:28:11 +0100: > > > >> Hello. > >> > >> On 25.01.22 13:14, Miquel Raynal wrote: > >>> Drivers are expected to set the PHY current_channel and current_page > >>> according to their default state. The hwsim driver is advertising being > >>> configured on channel 13 by default but that is not reflected in its own > >>> internal pib structure. In order to ensure that this driver consider the > >>> current channel as being 13 internally, we at least need to set the > >>> pib->channel field to 13. > >>> > >>> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") > >>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > >>> --- > >>> drivers/net/ieee802154/mac802154_hwsim.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c > >>> index 8caa61ec718f..00ec188a3257 100644 > >>> --- a/drivers/net/ieee802154/mac802154_hwsim.c > >>> +++ b/drivers/net/ieee802154/mac802154_hwsim.c > >>> @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, > >>> goto err_pib; > >>> } > >>> > + pib->page = 13; > >> > >> You want to set channel not page here. > > > > Oh crap /o\ I've messed that update badly. Of course I meant > > pib->channel here, as it is in the commit log. > > > > I'll wait for Alexander's feedback before re-spinning. Unless the rest > > looks good for you both, I don't know if your policy allows you to fix > > it when applying, anyhow I'll do what is necessary. > > If Alex has nothing else and there is no re-spin I fix this when > applying, no worries. Everything is fine. Acked-by: Alexander Aring <aahringo@redhat.com> On the whole series. Thanks. - Alex
Hello. On 26.01.22 23:54, Alexander Aring wrote: > Hi, > > On Wed, Jan 26, 2022 at 8:38 AM Stefan Schmidt > <stefan@datenfreihafen.org> wrote: >> >> >> Hello. >> >> On 25.01.22 17:48, Miquel Raynal wrote: >>> Hi Stefan, >>> >>> stefan@datenfreihafen.org wrote on Tue, 25 Jan 2022 15:28:11 +0100: >>> >>>> Hello. >>>> >>>> On 25.01.22 13:14, Miquel Raynal wrote: >>>>> Drivers are expected to set the PHY current_channel and current_page >>>>> according to their default state. The hwsim driver is advertising being >>>>> configured on channel 13 by default but that is not reflected in its own >>>>> internal pib structure. In order to ensure that this driver consider the >>>>> current channel as being 13 internally, we at least need to set the >>>>> pib->channel field to 13. >>>>> >>>>> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") >>>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> >>>>> --- >>>>> drivers/net/ieee802154/mac802154_hwsim.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c >>>>> index 8caa61ec718f..00ec188a3257 100644 >>>>> --- a/drivers/net/ieee802154/mac802154_hwsim.c >>>>> +++ b/drivers/net/ieee802154/mac802154_hwsim.c >>>>> @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, >>>>> goto err_pib; >>>>> } >>>>> > + pib->page = 13; >>>> >>>> You want to set channel not page here. >>> >>> Oh crap /o\ I've messed that update badly. Of course I meant >>> pib->channel here, as it is in the commit log. >>> >>> I'll wait for Alexander's feedback before re-spinning. Unless the rest >>> looks good for you both, I don't know if your policy allows you to fix >>> it when applying, anyhow I'll do what is necessary. >> >> If Alex has nothing else and there is no re-spin I fix this when >> applying, no worries. > > Everything is fine. > > Acked-by: Alexander Aring <aahringo@redhat.com> > > On the whole series. Thanks. Fixed up this commit and applied the whole patchset. Next one we should look at would be the 3 split out cleanup patches. regards Stefan Schmidt
Hi Stefan, stefan@datenfreihafen.org wrote on Thu, 27 Jan 2022 08:33:15 +0100: > Hello. > > On 26.01.22 23:54, Alexander Aring wrote: > > Hi, > > > > On Wed, Jan 26, 2022 at 8:38 AM Stefan Schmidt > > <stefan@datenfreihafen.org> wrote: > >> > >> > >> Hello. > >> > >> On 25.01.22 17:48, Miquel Raynal wrote: > >>> Hi Stefan, > >>> > >>> stefan@datenfreihafen.org wrote on Tue, 25 Jan 2022 15:28:11 +0100: > >>> > >>>> Hello. > >>>> > >>>> On 25.01.22 13:14, Miquel Raynal wrote: > >>>>> Drivers are expected to set the PHY current_channel and current_page > >>>>> according to their default state. The hwsim driver is advertising being > >>>>> configured on channel 13 by default but that is not reflected in its own > >>>>> internal pib structure. In order to ensure that this driver consider the > >>>>> current channel as being 13 internally, we at least need to set the > >>>>> pib->channel field to 13. > >>>>> > >>>>> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") > >>>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > >>>>> --- > >>>>> drivers/net/ieee802154/mac802154_hwsim.c | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c > >>>>> index 8caa61ec718f..00ec188a3257 100644 > >>>>> --- a/drivers/net/ieee802154/mac802154_hwsim.c > >>>>> +++ b/drivers/net/ieee802154/mac802154_hwsim.c > >>>>> @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, > >>>>> goto err_pib; > >>>>> } > >>>>> > + pib->page = 13; > >>>> > >>>> You want to set channel not page here. > >>> > >>> Oh crap /o\ I've messed that update badly. Of course I meant > >>> pib->channel here, as it is in the commit log. > >>> > >>> I'll wait for Alexander's feedback before re-spinning. Unless the rest > >>> looks good for you both, I don't know if your policy allows you to fix > >>> it when applying, anyhow I'll do what is necessary. > >> > >> If Alex has nothing else and there is no re-spin I fix this when > >> applying, no worries. > > > > Everything is fine. > > > > Acked-by: Alexander Aring <aahringo@redhat.com> > > > > On the whole series. Thanks. > > Fixed up this commit and applied the whole patchset. > > Next one we should look at would be the 3 split out cleanup patches. Great! Thanks, Miquèl
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index 8caa61ec718f..00ec188a3257 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -786,6 +786,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, goto err_pib; } + pib->page = 13; rcu_assign_pointer(phy->pib, pib); phy->idx = idx; INIT_LIST_HEAD(&phy->edges);
Drivers are expected to set the PHY current_channel and current_page according to their default state. The hwsim driver is advertising being configured on channel 13 by default but that is not reflected in its own internal pib structure. In order to ensure that this driver consider the current channel as being 13 internally, we at least need to set the pib->channel field to 13. Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/net/ieee802154/mac802154_hwsim.c | 1 + 1 file changed, 1 insertion(+)