mbox series

[wpan-next,v2,0/9] ieee802154: A bunch of fixes

Message ID 20220120112115.448077-1-miquel.raynal@bootlin.com (mailing list archive)
Headers show
Series ieee802154: A bunch of fixes | expand

Message

Miquel Raynal Jan. 20, 2022, 11:21 a.m. UTC
In preparation to a wider series, here are a number of small and random
fixes across the subsystem.

Changes in v2:
* Fixed the build error reported by a robot. It ended up being something
  which I fixed in a commit from a following series. I've now sorted
  this out and the patch now works on its own.

Miquel Raynal (9):
  net: ieee802154: hwsim: Ensure proper channel selection at probe time
  net: ieee802154: hwsim: Ensure frame checksum are valid
  net: ieee802154: mcr20a: Fix lifs/sifs periods
  net: ieee802154: at86rf230: Stop leaking skb's
  net: ieee802154: ca8210: Stop leaking skb's
  net: ieee802154: Use the IEEE802154_MAX_PAGE define when relevant
  net: ieee802154: Return meaningful error codes from the netlink
    helpers
  net: mac802154: Explain the use of ieee802154_wake/stop_queue()
  MAINTAINERS: Remove Harry Morris bouncing address

 MAINTAINERS                              |  3 +--
 drivers/net/ieee802154/at86rf230.c       |  1 +
 drivers/net/ieee802154/ca8210.c          |  1 +
 drivers/net/ieee802154/mac802154_hwsim.c | 12 ++----------
 drivers/net/ieee802154/mcr20a.c          |  4 ++--
 include/net/mac802154.h                  | 12 ++++++++++++
 net/ieee802154/nl-phy.c                  |  5 +++--
 net/ieee802154/nl802154.c                |  8 ++++----
 8 files changed, 26 insertions(+), 20 deletions(-)

Comments

Alexander Aring Jan. 20, 2022, 10:52 p.m. UTC | #1
Hi,

On Thu, 20 Jan 2022 at 06:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> In preparation to a wider series, here are a number of small and random
> fixes across the subsystem.
>
> Changes in v2:
> * Fixed the build error reported by a robot. It ended up being something
>   which I fixed in a commit from a following series. I've now sorted
>   this out and the patch now works on its own.
>

This patch series should be reviewed first and have all current
detected fixes, it also should be tagged "wpan" (no need to fix that
now). Then there is a following up series for a new feature which you
like to tackle, maybe the "more generic symbol duration handling"? It
should be based on this "fixes" patch series, Stefan will then get
things sorted out to queue them right for upstream.
Stefan, please correct me if I'm wrong.

Also, please give me the weekend to review this patch series.

Thanks.

- Alex
Alexander Aring Jan. 20, 2022, 11:07 p.m. UTC | #2
Hi,

On Thu, 20 Jan 2022 at 17:52, Alexander Aring <alex.aring@gmail.com> wrote:
>
> Hi,
>
> On Thu, 20 Jan 2022 at 06:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > In preparation to a wider series, here are a number of small and random
> > fixes across the subsystem.
> >
> > Changes in v2:
> > * Fixed the build error reported by a robot. It ended up being something
> >   which I fixed in a commit from a following series. I've now sorted
> >   this out and the patch now works on its own.
> >
>
> This patch series should be reviewed first and have all current
> detected fixes, it also should be tagged "wpan" (no need to fix that
> now). Then there is a following up series for a new feature which you
> like to tackle, maybe the "more generic symbol duration handling"? It
> should be based on this "fixes" patch series, Stefan will then get
> things sorted out to queue them right for upstream.
> Stefan, please correct me if I'm wrong.
>
> Also, please give me the weekend to review this patch series.

and please don't send other patch-series after this one isn't applied.
After it's applied, then send another patch series. Only one please,
then wait again for it to be applied... and so on.

- Alex
Miquel Raynal Jan. 21, 2022, 8:27 a.m. UTC | #3
Hi Alexander,

alex.aring@gmail.com wrote on Thu, 20 Jan 2022 17:52:57 -0500:

> Hi,
> 
> On Thu, 20 Jan 2022 at 06:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > In preparation to a wider series, here are a number of small and random
> > fixes across the subsystem.
> >
> > Changes in v2:
> > * Fixed the build error reported by a robot. It ended up being something
> >   which I fixed in a commit from a following series. I've now sorted
> >   this out and the patch now works on its own.
> >  
> 
> This patch series should be reviewed first and have all current
> detected fixes, it also should be tagged "wpan" (no need to fix that
> now). Then there is a following up series for a new feature which you
> like to tackle, maybe the "more generic symbol duration handling"? It
> should be based on this "fixes" patch series, Stefan will then get
> things sorted out to queue them right for upstream.
> Stefan, please correct me if I'm wrong.

Yup sorry that's not what I meant: the kernel robot detected that a
patch broke the build. This patch was part of the current series. The
issue was that I messed a copy paste error. But I didn't ran a
per-patch build test and another patch, which had nothing to do with
this fix, actually addressed the build issue. I very likely failed
something during my rebase operation.

So yes, this series should come first. Then we'll tackle the symbol
duration series, the Kconfig cleanup and after that we can start thick
topics :)

> Also, please give me the weekend to review this patch series.

Yes of course, you've been very (very) reactive so far, I try to be
also more reactive on my side but that's of course not a race!

Thanks,
Miquèl
Stefan Schmidt Jan. 21, 2022, 12:48 p.m. UTC | #4
Hello.

On 21.01.22 09:27, Miquel Raynal wrote:
> Hi Alexander,
> 
> alex.aring@gmail.com wrote on Thu, 20 Jan 2022 17:52:57 -0500:
> 
>> Hi,
>>
>> On Thu, 20 Jan 2022 at 06:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>>>
>>> In preparation to a wider series, here are a number of small and random
>>> fixes across the subsystem.
>>>
>>> Changes in v2:
>>> * Fixed the build error reported by a robot. It ended up being something
>>>    which I fixed in a commit from a following series. I've now sorted
>>>    this out and the patch now works on its own.
>>>   
>>
>> This patch series should be reviewed first and have all current
>> detected fixes, it also should be tagged "wpan" (no need to fix that
>> now). Then there is a following up series for a new feature which you
>> like to tackle, maybe the "more generic symbol duration handling"? It
>> should be based on this "fixes" patch series, Stefan will then get
>> things sorted out to queue them right for upstream.
>> Stefan, please correct me if I'm wrong.

Alex, agreed. I will take this series first and see if the patches apply 
cleanly against my wpan tree. Once in they can be feed back into net, 
net-next and finally wpan-next again.

> Yup sorry that's not what I meant: the kernel robot detected that a
> patch broke the build. This patch was part of the current series. The
> issue was that I messed a copy paste error. But I didn't ran a
> per-patch build test and another patch, which had nothing to do with
> this fix, actually addressed the build issue. I very likely failed
> something during my rebase operation. >
> So yes, this series should come first. Then we'll tackle the symbol
> duration series, the Kconfig cleanup and after that we can start thick
> topics :)

That sounds like a great plan to me. I know splitting the huge amount of 
work you do up into digestible pieces is work not much liked, so I 
appreciate that you take this without much grumble. :-)

I also finally started to start my review backlog on your work. Catched 
up on the big v3 patchset now. Will look over the newest sets over the 
weekend so we should be ready to process the fixes series and maybe more 
next week.

>> Also, please give me the weekend to review this patch series.

Alex, whenever you are ready with them please add you ack and I will doe 
my review and testing in parallel.

> Yes of course, you've been very (very) reactive so far, I try to be
> also more reactive on my side but that's of course not a race!

And being so reactive is very much appreciated. We just need to throttle 
this a bit so we can keep up with reviewer resources. :-)

regards
Stefan Schmidt
Stefan Schmidt Jan. 21, 2022, 4:09 p.m. UTC | #5
Hello Miquel

On 20.01.22 12:21, Miquel Raynal wrote:
> In preparation to a wider series, here are a number of small and random
> fixes across the subsystem.
> 
> Changes in v2:
> * Fixed the build error reported by a robot. It ended up being something
>    which I fixed in a commit from a following series. I've now sorted
>    this out and the patch now works on its own.
> 
> Miquel Raynal (9):
>    net: ieee802154: hwsim: Ensure proper channel selection at probe time
>    net: ieee802154: hwsim: Ensure frame checksum are valid
>    net: ieee802154: mcr20a: Fix lifs/sifs periods
>    net: ieee802154: at86rf230: Stop leaking skb's
>    net: ieee802154: ca8210: Stop leaking skb's
>    net: ieee802154: Use the IEEE802154_MAX_PAGE define when relevant
>    net: ieee802154: Return meaningful error codes from the netlink
>      helpers
>    net: mac802154: Explain the use of ieee802154_wake/stop_queue()
>    MAINTAINERS: Remove Harry Morris bouncing address
> 
>   MAINTAINERS                              |  3 +--
>   drivers/net/ieee802154/at86rf230.c       |  1 +
>   drivers/net/ieee802154/ca8210.c          |  1 +
>   drivers/net/ieee802154/mac802154_hwsim.c | 12 ++----------
>   drivers/net/ieee802154/mcr20a.c          |  4 ++--
>   include/net/mac802154.h                  | 12 ++++++++++++
>   net/ieee802154/nl-phy.c                  |  5 +++--
>   net/ieee802154/nl802154.c                |  8 ++++----
>   8 files changed, 26 insertions(+), 20 deletions(-)
> 

All patches apply without conflict to my wpan tree (to feed into net 
instead of net-next for these fixes) and compile testing showed no 
problems. I will do some light smoke testing on the weekend and also 
wait for Alex review. But beside one small review remark these should be 
ready to go in on the next iteration.

regards
Stefan Schmidt
Miquel Raynal Jan. 25, 2022, 9:51 a.m. UTC | #6
Hi Stefan,

stefan@datenfreihafen.org wrote on Fri, 21 Jan 2022 13:48:14 +0100:

> Hello.
> 
> On 21.01.22 09:27, Miquel Raynal wrote:
> > Hi Alexander,
> > 
> > alex.aring@gmail.com wrote on Thu, 20 Jan 2022 17:52:57 -0500:
> >   
> >> Hi,
> >>
> >> On Thu, 20 Jan 2022 at 06:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> >>>
> >>> In preparation to a wider series, here are a number of small and random
> >>> fixes across the subsystem.
> >>>
> >>> Changes in v2:
> >>> * Fixed the build error reported by a robot. It ended up being something
> >>>    which I fixed in a commit from a following series. I've now sorted
> >>>    this out and the patch now works on its own.  
> >>>   >>  
> >> This patch series should be reviewed first and have all current
> >> detected fixes, it also should be tagged "wpan" (no need to fix that
> >> now). Then there is a following up series for a new feature which you
> >> like to tackle, maybe the "more generic symbol duration handling"? It
> >> should be based on this "fixes" patch series, Stefan will then get
> >> things sorted out to queue them right for upstream.
> >> Stefan, please correct me if I'm wrong.  
> 
> Alex, agreed. I will take this series first and see if the patches apply cleanly against my wpan tree. Once in they can be feed back into net, net-next and finally wpan-next again.
> 
> > Yup sorry that's not what I meant: the kernel robot detected that a
> > patch broke the build. This patch was part of the current series. The
> > issue was that I messed a copy paste error. But I didn't ran a
> > per-patch build test and another patch, which had nothing to do with
> > this fix, actually addressed the build issue. I very likely failed
> > something during my rebase operation. >
> > So yes, this series should come first. Then we'll tackle the symbol
> > duration series, the Kconfig cleanup and after that we can start thick
> > topics :)  
> 
> That sounds like a great plan to me. I know splitting the huge amount of work you do up into digestible pieces is work not much liked, so I appreciate that you take this without much grumble. :-)

Yeah no problem, I also know what it is like to be on the reviewer
side, and while I like to have the full contribution to get the big
picture, I also find it much easier to review smaller series, so let's
got for it now that the main boundaries for the scan support have been
shared.

> I also finally started to start my review backlog on your work. Catched up on the big v3 patchset now. Will look over the newest sets over the weekend so we should be ready to process the fixes series and maybe more next week.
> 
> >> Also, please give me the weekend to review this patch series.  
> 
> Alex, whenever you are ready with them please add you ack and I will doe my review and testing in parallel.
> 
> > Yes of course, you've been very (very) reactive so far, I try to be
> > also more reactive on my side but that's of course not a race!  
> 
> And being so reactive is very much appreciated. We just need to throttle this a bit so we can keep up with reviewer resources. :-)

Yup! I'll soon send a v3 addressing your comments, this time I'll even
split the first series so that you have a series with only fixes for
the wpan branch, and another series with very small cleanups for
wpan-next. I'll send both because they are not very big.

Thanks,
Miquèl