Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Jes Sorensen]

On 05/17/2017 10:46 AM, Arnd Bergmann wrote:

I've managed to split up my long patch into a series of reasonble
steps now.

The first two are required to fix a regression from commit 41977e86c984
("rt2x00: add support for MT7620"), the rest are just cleanups to
have a consistent state across all the register access functions.

  Arnd


Nice work! This is a textbook example of how to do this the right way!

Reviewed-by: Jes Sorensen 

Jes

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Arnd Bergmann]

On Fri, May 19, 2017 at 2:15 PM, Tom Psyborg  wrote:
>
>
> On 19 May 2017 at 08:55, Arnd Bergmann  wrote:
>>
>>
>> On which base version did you apply my patches? There may be a conflict
>> against patches that are in your tree but not yet in linux-next, as I
>> don't see
>> the warning and also see no reference to rt2800_bbp_read in rt2800lib.h
>>
>>   Arnd
>
>
>
> it's not exactly base version, it is patched comapt-wireless in openwrt that
> i applied your patches to (i had to fix some things manually) but these
> warnings might appear because of recent mt7620 commit:
> https://git.lede-project.org/?p=lede/dangole/staging.git;a=blob_plain;f=package/kernel/mac80211/patches/999-0001-rt2800mmio-use-BBP-register-21-to-reset-MT7620-BBP.patch;hb=f4f0d8efa2d55ada111ddcd502a51041364bd7e5

I suspect you just did an incorrect merge, and left an extra 'static'
in front of the declaration in the header file when you modified the
function prototype.

  Arnd

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Arnd Bergmann]

On Fri, May 19, 2017 at 9:15 AM, Kalle Valo  wrote:
> Arnd Bergmann  writes:
>
>> On Fri, May 19, 2017 at 7:18 AM, Kalle Valo  wrote:
>>> Arnd Bergmann  writes:
>>>
 I've managed to split up my long patch into a series of reasonble
 steps now.

 The first two are required to fix a regression from commit 41977e86c984
 ("rt2x00: add support for MT7620"), the rest are just cleanups to
 have a consistent state across all the register access functions.
>>>
>>> Can these all go to 4.13 or would you prefer me to push the first two
>>> 4.12? Or what?
>>
>> I think you can reasonably argue either way: the second patch does
>> fix a real bug that may or may not lead to an exploitable stack overflow
>> when CONFIG_KASAN is enabled, which would be a reason to put it
>> into 4.12. On the other hand, I have another 20 patches for similar
>> (or worse) stack overflow issues with KASAN that I'm hoping to all
>> get into 4.13 and backported into stable kernel later if necessary,
>> so we could treat this one like the others.
>>
>> The only difference between this and the others is that in rt2x00 it
>> is a regression against 4.11, while the others have all been present
>> for a long time.
>
> Having all of these in 4.12 sounds a bit excessive and splitting the set
> (the first two into 4.12 and the rest into 4.13) sounds too much work.
> So I would prefer to queue these to 4.13, if it's ok for everyone?

Ok, sounds fine. Thanks,

  Arnd

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Kalle Valo]

Arnd Bergmann  writes:

> On Fri, May 19, 2017 at 7:18 AM, Kalle Valo  wrote:
>> Arnd Bergmann  writes:
>>
>>> I've managed to split up my long patch into a series of reasonble
>>> steps now.
>>>
>>> The first two are required to fix a regression from commit 41977e86c984
>>> ("rt2x00: add support for MT7620"), the rest are just cleanups to
>>> have a consistent state across all the register access functions.
>>
>> Can these all go to 4.13 or would you prefer me to push the first two
>> 4.12? Or what?
>
> I think you can reasonably argue either way: the second patch does
> fix a real bug that may or may not lead to an exploitable stack overflow
> when CONFIG_KASAN is enabled, which would be a reason to put it
> into 4.12. On the other hand, I have another 20 patches for similar
> (or worse) stack overflow issues with KASAN that I'm hoping to all
> get into 4.13 and backported into stable kernel later if necessary,
> so we could treat this one like the others.
>
> The only difference between this and the others is that in rt2x00 it
> is a regression against 4.11, while the others have all been present
> for a long time.

Having all of these in 4.12 sounds a bit excessive and splitting the set
(the first two into 4.12 and the rest into 4.13) sounds too much work.
So I would prefer to queue these to 4.13, if it's ok for everyone?

-- 
Kalle Valo

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Kalle Valo]

Arnd Bergmann  writes:

> On Fri, May 19, 2017 at 8:44 AM, Tom Psyborg  
> wrote:
>> warning: 'rt2800_bbp_read' used but never defined
>>  static u8 rt2800_bbp_read(struct rt2x00_dev *rt2x00dev,
>>^
>> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800lib.h:262:13:
>> warning: 'rt2800_bbp_write' used but never defined
>>  static void rt2800_bbp_write(struct rt2x00_dev *rt2x00dev,
>>  ^
>>   CC [M]
>> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800pci.o
>> In file included from
>> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800pci.c:43:0:
>> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800lib.h:259:11:
>> warning: 'rt2800_bbp_read' declared 'static' but never defined
>> [-Wunused-function]
>>  static u8 rt2800_bbp_read(struct rt2x00_dev *rt2x00dev,
>>^
>
> On which base version did you apply my patches? There may be a conflict
> against patches that are in your tree but not yet in linux-next, as I don't 
> see
> the warning and also see no reference to rt2800_bbp_read in rt2800lib.h

I did a test build with current wireless-drivers-next and I also don't
see any warnings.

-- 
Kalle Valo

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Arnd Bergmann]

On Fri, May 19, 2017 at 8:44 AM, Tom Psyborg  wrote:
> warning: 'rt2800_bbp_read' used but never defined
>  static u8 rt2800_bbp_read(struct rt2x00_dev *rt2x00dev,
>^
> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800lib.h:262:13:
> warning: 'rt2800_bbp_write' used but never defined
>  static void rt2800_bbp_write(struct rt2x00_dev *rt2x00dev,
>  ^
>   CC [M]
> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800pci.o
> In file included from
> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800pci.c:43:0:
> /home/ubuntu/Music/openwrt/build_dir/target-mipsel_24kc_musl-1.1.16/linux-ramips_mt7620/compat-wireless-2016-05-12/drivers/net/wireless/ralink/rt2x00/rt2800lib.h:259:11:
> warning: 'rt2800_bbp_read' declared 'static' but never defined
> [-Wunused-function]
>  static u8 rt2800_bbp_read(struct rt2x00_dev *rt2x00dev,
>^

On which base version did you apply my patches? There may be a conflict
against patches that are in your tree but not yet in linux-next, as I don't see
the warning and also see no reference to rt2800_bbp_read in rt2800lib.h

  Arnd

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Arnd Bergmann]

On Fri, May 19, 2017 at 7:18 AM, Kalle Valo  wrote:
> Arnd Bergmann  writes:
>
>> I've managed to split up my long patch into a series of reasonble
>> steps now.
>>
>> The first two are required to fix a regression from commit 41977e86c984
>> ("rt2x00: add support for MT7620"), the rest are just cleanups to
>> have a consistent state across all the register access functions.
>
> Can these all go to 4.13 or would you prefer me to push the first two
> 4.12? Or what?

I think you can reasonably argue either way: the second patch does
fix a real bug that may or may not lead to an exploitable stack overflow
when CONFIG_KASAN is enabled, which would be a reason to put it
into 4.12. On the other hand, I have another 20 patches for similar
(or worse) stack overflow issues with KASAN that I'm hoping to all
get into 4.13 and backported into stable kernel later if necessary,
so we could treat this one like the others.

The only difference between this and the others is that in rt2x00 it
is a regression against 4.11, while the others have all been present
for a long time.

  Arnd

Re: [PATCH v2 00/10] rt2x00: rt2x00: improve calling conventions for register accessors

[Kalle Valo]

Arnd Bergmann  writes:

> I've managed to split up my long patch into a series of reasonble
> steps now.
>
> The first two are required to fix a regression from commit 41977e86c984
> ("rt2x00: add support for MT7620"), the rest are just cleanups to
> have a consistent state across all the register access functions.

Can these all go to 4.13 or would you prefer me to push the first two
4.12? Or what?

-- 
Kalle Valo