| Message ID | 20201118230929.18147-1-rentao.bupt@gmail.com |
|---|---|
| Headers | show |
| Series | hwmon: (max127) Add Maxim MAX127 hardware monitoring | expand |
On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > From: Tao Ren <rentao.bupt@gmail.com> > > The patch series adds hardware monitoring driver for the Maxim MAX127 > chip. Hi Tao Why are using sending a hwmon driver to the networking mailing list? Andrew
On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > From: Tao Ren <rentao.bupt@gmail.com> > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > chip. > > Hi Tao > > Why are using sending a hwmon driver to the networking mailing list? > > Andrew Hi Andrew, I added netdev because the mailing list is included in "get_maintainer.pl Documentation/hwmon/index.rst" output. Is it the right command to find reviewers? Could you please suggest? Thank you. Cheers, Tao
On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > chip. > > > > Hi Tao > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > Andrew > > Hi Andrew, > > I added netdev because the mailing list is included in "get_maintainer.pl > Documentation/hwmon/index.rst" output. Is it the right command to find > reviewers? Could you please suggest? Thank you. I have no idea why running get_maintainer.pl on Documentation/hwmon/index.rst returns such a large list of mailing lists and people. For some reason it includes everyone in the XDP maintainer list. If anyone has an idea how that happens, please let me know - we'll want to get this fixed to avoid the same problem in the future. Guenter
On Wed, Nov 18, 2020 at 05:01:19PM -0800, Guenter Roeck wrote: > On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > > chip. > > > > > > Hi Tao > > > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > > > Andrew > > > > Hi Andrew, > > > > I added netdev because the mailing list is included in "get_maintainer.pl > > Documentation/hwmon/index.rst" output. Is it the right command to find > > reviewers? Could you please suggest? Thank you. > > I have no idea why running get_maintainer.pl on > Documentation/hwmon/index.rst returns such a large list of mailing > lists and people. For some reason it includes everyone in the XDP > maintainer list. If anyone has an idea how that happens, please > let me know - we'll want to get this fixed to avoid the same problem > in the future. > I found it. The XDP maintainer entry has: K: xdp This matches Documentation/hwmon/index.rst. $ grep xdp Documentation/hwmon/index.rst xdpe12284 It seems to me that a context match such as "xdp" in MAINTAINERS isn't really appropriate. "xdp" matches a total of 348 files in the kernel. The large majority of those is not XDP related. The maintainers of XDP (and all the listed mailing lists) should not be surprised to get a large number of odd review requests if they want to review every single patch on files which include the term "xdp". Guenter
On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > chip. > > > > Hi Tao > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > Andrew > > Hi Andrew, > > I added netdev because the mailing list is included in "get_maintainer.pl > Documentation/hwmon/index.rst" output. Is it the right command to find > reviewers? Could you please suggest? Thank you. Hi Tae What you are doing is correct. I suspected it was a get_maintainers problem. Now we know this, we can figure out why it is adding all these extra addresses which make no sense. Maybe a bug in the MAINTAINERS file? Andrew
On Wed, Nov 18, 2020 at 05:26:53PM -0800, Guenter Roeck wrote: > On Wed, Nov 18, 2020 at 05:01:19PM -0800, Guenter Roeck wrote: > > On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > > > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > > > chip. > > > > > > > > Hi Tao > > > > > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > > > > > Andrew > > > > > > Hi Andrew, > > > > > > I added netdev because the mailing list is included in "get_maintainer.pl > > > Documentation/hwmon/index.rst" output. Is it the right command to find > > > reviewers? Could you please suggest? Thank you. > > > > I have no idea why running get_maintainer.pl on > > Documentation/hwmon/index.rst returns such a large list of mailing > > lists and people. For some reason it includes everyone in the XDP > > maintainer list. If anyone has an idea how that happens, please > > let me know - we'll want to get this fixed to avoid the same problem > > in the future. > > > > I found it. The XDP maintainer entry has: > > K: xdp > > This matches Documentation/hwmon/index.rst. > > $ grep xdp Documentation/hwmon/index.rst > xdpe12284 > > It seems to me that a context match such as "xdp" in MAINTAINERS isn't > really appropriate. "xdp" matches a total of 348 files in the kernel. > The large majority of those is not XDP related. The maintainers > of XDP (and all the listed mailing lists) should not be surprised > to get a large number of odd review requests if they want to review > every single patch on files which include the term "xdp". > > Guenter Thanks Guenter and Andrew. Given xdp maintainers were included by mistake, I will remove them from the future discussions of this hwmon patch series. Cheers, Tao
On Wed, 18 Nov 2020 17:26:53 -0800 Guenter Roeck wrote: > On Wed, Nov 18, 2020 at 05:01:19PM -0800, Guenter Roeck wrote: > > On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > > > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > > > chip. > > > > > > > > Hi Tao > > > > > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > > > > > Andrew > > > > > > Hi Andrew, > > > > > > I added netdev because the mailing list is included in "get_maintainer.pl > > > Documentation/hwmon/index.rst" output. Is it the right command to find > > > reviewers? Could you please suggest? Thank you. > > > > I have no idea why running get_maintainer.pl on > > Documentation/hwmon/index.rst returns such a large list of mailing > > lists and people. For some reason it includes everyone in the XDP > > maintainer list. If anyone has an idea how that happens, please > > let me know - we'll want to get this fixed to avoid the same problem > > in the future. > > I found it. The XDP maintainer entry has: > > K: xdp > > This matches Documentation/hwmon/index.rst. > > $ grep xdp Documentation/hwmon/index.rst > xdpe12284 > > It seems to me that a context match such as "xdp" in MAINTAINERS isn't > really appropriate. "xdp" matches a total of 348 files in the kernel. > The large majority of those is not XDP related. The maintainers > of XDP (and all the listed mailing lists) should not be surprised > to get a large number of odd review requests if they want to review > every single patch on files which include the term "xdp". Agreed, we should fix this. For maintainers with high patch volume life would be so much easier if people CCed the right folks to get reviews, so we should try our best to fix get_maintainer. XDP folks, any opposition to changing the keyword / filename to: [^a-z0-9]xdp[^a-z0-9] ?
On Thu, 19 Nov 2020 07:46:34 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > On Wed, 18 Nov 2020 17:26:53 -0800 Guenter Roeck wrote: > > On Wed, Nov 18, 2020 at 05:01:19PM -0800, Guenter Roeck wrote: > > > On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > > > > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > > > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > > > > chip. > > > > > > > > > > Hi Tao > > > > > > > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > > > > > > > Andrew > > > > > > > > Hi Andrew, > > > > > > > > I added netdev because the mailing list is included in "get_maintainer.pl > > > > Documentation/hwmon/index.rst" output. Is it the right command to find > > > > reviewers? Could you please suggest? Thank you. > > > > > > I have no idea why running get_maintainer.pl on > > > Documentation/hwmon/index.rst returns such a large list of mailing > > > lists and people. For some reason it includes everyone in the XDP > > > maintainer list. If anyone has an idea how that happens, please > > > let me know - we'll want to get this fixed to avoid the same problem > > > in the future. > > > > I found it. The XDP maintainer entry has: > > > > K: xdp > > > > This matches Documentation/hwmon/index.rst. > > > > $ grep xdp Documentation/hwmon/index.rst > > xdpe12284 > > > > It seems to me that a context match such as "xdp" in MAINTAINERS isn't > > really appropriate. "xdp" matches a total of 348 files in the kernel. > > The large majority of those is not XDP related. The maintainers > > of XDP (and all the listed mailing lists) should not be surprised > > to get a large number of odd review requests if they want to review > > every single patch on files which include the term "xdp". > > Agreed, we should fix this. For maintainers with high patch volume life > would be so much easier if people CCed the right folks to get reviews, > so we should try our best to fix get_maintainer. > > XDP folks, any opposition to changing the keyword / filename to: > > [^a-z0-9]xdp[^a-z0-9] > > ? I think it is a good idea to change the keyword (K:), but I'm not sure this catch what we want, maybe it does. The pattern match are meant to catch drivers containing XDP related bits. Previously Joe Perches <joe@perches.com> suggested this pattern match, which I don't fully understand... could you explain Joe? (?:\b|_)xdp(?:\b|_) For the filename (N:) regex match, I'm considering if we should remove it and list more files explicitly. I think normal glob * pattern works, which should be sufficient.
On Thu, Nov 19, 2020 at 7:46 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 18 Nov 2020 17:26:53 -0800 Guenter Roeck wrote: > > On Wed, Nov 18, 2020 at 05:01:19PM -0800, Guenter Roeck wrote: > > > On Wed, Nov 18, 2020 at 03:42:53PM -0800, Tao Ren wrote: > > > > On Thu, Nov 19, 2020 at 12:27:19AM +0100, Andrew Lunn wrote: > > > > > On Wed, Nov 18, 2020 at 03:09:27PM -0800, rentao.bupt@gmail.com wrote: > > > > > > From: Tao Ren <rentao.bupt@gmail.com> > > > > > > > > > > > > The patch series adds hardware monitoring driver for the Maxim MAX127 > > > > > > chip. > > > > > > > > > > Hi Tao > > > > > > > > > > Why are using sending a hwmon driver to the networking mailing list? > > > > > > > > > > Andrew > > > > > > > > Hi Andrew, > > > > > > > > I added netdev because the mailing list is included in "get_maintainer.pl > > > > Documentation/hwmon/index.rst" output. Is it the right command to find > > > > reviewers? Could you please suggest? Thank you. > > > > > > I have no idea why running get_maintainer.pl on > > > Documentation/hwmon/index.rst returns such a large list of mailing > > > lists and people. For some reason it includes everyone in the XDP > > > maintainer list. If anyone has an idea how that happens, please > > > let me know - we'll want to get this fixed to avoid the same problem > > > in the future. > > > > I found it. The XDP maintainer entry has: > > > > K: xdp > > > > This matches Documentation/hwmon/index.rst. > > > > $ grep xdp Documentation/hwmon/index.rst > > xdpe12284 > > > > It seems to me that a context match such as "xdp" in MAINTAINERS isn't > > really appropriate. "xdp" matches a total of 348 files in the kernel. > > The large majority of those is not XDP related. The maintainers > > of XDP (and all the listed mailing lists) should not be surprised > > to get a large number of odd review requests if they want to review > > every single patch on files which include the term "xdp". > > Agreed, we should fix this. For maintainers with high patch volume life > would be so much easier if people CCed the right folks to get reviews, > so we should try our best to fix get_maintainer. > > XDP folks, any opposition to changing the keyword / filename to: > > [^a-z0-9]xdp[^a-z0-9] Reducing regex makes sense. git grep -l -E "xdp"|wc -l 348 git grep -l -E "[^a-z0-9]xdp[^a-z0-9]"|wc -l 295 The false positive match was: +drivers/hwmon/pmbus/Kconfig +drivers/hwmon/pmbus/Makefile +drivers/hwmon/pmbus/xdpe12284.c +drivers/net/ethernet/natsemi/ns83820.c +drivers/net/ethernet/neterion/s2io.c +drivers/net/ethernet/neterion/s2io.h +drivers/net/ethernet/neterion/vxge/vxge-config.c +drivers/net/ethernet/neterion/vxge/vxge-config.h +drivers/net/ethernet/neterion/vxge/vxge-traffic.c +drivers/net/ethernet/sis/sis900.c +drivers/net/ethernet/sis/sis900.h +drivers/net/wireless/ath/ath5k/ath5k.h +drivers/net/wireless/ath/ath5k/base.c +drivers/net/wireless/ath/ath5k/debug.c +drivers/net/wireless/ath/ath5k/dma.c so it's pretty much hwmon and few drivers. I agree that sparing xdp from hwmon patches is a good thing :)
On Thu, 2020-11-19 at 17:35 +0100, Jesper Dangaard Brouer wrote: > On Thu, 19 Nov 2020 07:46:34 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > I think it is a good idea to change the keyword (K:), but I'm not sure > this catch what we want, maybe it does. The pattern match are meant to > catch drivers containing XDP related bits. > > Previously Joe Perches <joe@perches.com> suggested this pattern match, > which I don't fully understand... could you explain Joe? > > (?:\b|_)xdp(?:\b|_) This regex matches only: xdp xdp_<anything> <anything>_xdp_<anything> <anything>_xdp > For the filename (N:) regex match, I'm considering if we should remove > it and list more files explicitly. I think normal glob * pattern > works, which should be sufficient. Lists are generally more specific than regex globs.
On Thu, 19 Nov 2020 09:09:53 -0800 Joe Perches wrote: > On Thu, 2020-11-19 at 17:35 +0100, Jesper Dangaard Brouer wrote: > > On Thu, 19 Nov 2020 07:46:34 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > > > I think it is a good idea to change the keyword (K:), but I'm not sure > > this catch what we want, maybe it does. The pattern match are meant to > > catch drivers containing XDP related bits. > > > > Previously Joe Perches <joe@perches.com> suggested this pattern match, > > which I don't fully understand... could you explain Joe? > > > > (?:\b|_)xdp(?:\b|_) > > This regex matches only: > > xdp > xdp_<anything> > <anything>_xdp_<anything> > <anything>_xdp > > > For the filename (N:) regex match, I'm considering if we should remove > > it and list more files explicitly. I think normal glob * pattern > > works, which should be sufficient. > > Lists are generally more specific than regex globs. Checking like Alexei did it seems Joe's version is faster and better: $ git grep -l -E "[^a-z0-9]xdp[^a-z0-9]" | wc -l 295 $ git grep -l -E '(\b|_)xdp(\b|_)' | wc -l 297 $ time git grep -l -E '(\b|_)xdp(\b|_)' > /tmp/a real 0m5.171s user 0m32.657s sys 0m0.664s $ time git grep -l -E "[^a-z0-9]xdp[^a-z0-9]" > /tmp/b real 0m16.627s user 1m48.149s sys 0m0.977s 09:56 linux$ diff /tmp/a /tmp/b 4d3 < Documentation/networking/index.rst 189d187 < samples/bpf/.gitignore Joe would you like to send a patch, or should I?
On Thu, 19 Nov 2020 09:59:28 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > On Thu, 19 Nov 2020 09:09:53 -0800 Joe Perches wrote: > > On Thu, 2020-11-19 at 17:35 +0100, Jesper Dangaard Brouer wrote: > > > On Thu, 19 Nov 2020 07:46:34 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > > > > > I think it is a good idea to change the keyword (K:), but I'm not sure > > > this catch what we want, maybe it does. The pattern match are meant to > > > catch drivers containing XDP related bits. > > > > > > Previously Joe Perches <joe@perches.com> suggested this pattern match, > > > which I don't fully understand... could you explain Joe? > > > > > > (?:\b|_)xdp(?:\b|_) > > > > This regex matches only: > > > > xdp > > xdp_<anything> > > <anything>_xdp_<anything> > > <anything>_xdp > > > > > For the filename (N:) regex match, I'm considering if we should remove > > > it and list more files explicitly. I think normal glob * pattern > > > works, which should be sufficient. > > > > Lists are generally more specific than regex globs. > > Checking like Alexei did it seems Joe's version is faster and better: > > $ git grep -l -E "[^a-z0-9]xdp[^a-z0-9]" | wc -l > 295 > $ git grep -l -E '(\b|_)xdp(\b|_)' | wc -l > 297 > $ time git grep -l -E '(\b|_)xdp(\b|_)' > /tmp/a Okay, I guess this is the pattern you want: '(\b|_)xdp(\b|_)' > Joe would you like to send a patch, or should I? As you noticed I already send out a patch, I can send a new with your pattern, as it seems to be faster.
From: Tao Ren <rentao.bupt@gmail.com> The patch series adds hardware monitoring driver for the Maxim MAX127 chip. Patch #1 adds the max127 hardware monitoring driver, and patch #2 adds documentation for the driver. Tao Ren (2): hwmon: (max127) Add Maxim MAX127 hardware monitoring driver docs: hwmon: Document max127 driver Documentation/hwmon/index.rst | 1 + Documentation/hwmon/max127.rst | 45 +++++ drivers/hwmon/Kconfig | 9 + drivers/hwmon/Makefile | 1 + drivers/hwmon/max127.c | 346 +++++++++++++++++++++++++++++++++ 5 files changed, 402 insertions(+) create mode 100644 Documentation/hwmon/max127.rst create mode 100644 drivers/hwmon/max127.c