On 4/17/21 3:24 PM, Gustavo A. R. Silva wrote:
>
>
> On 4/17/21 13:29, Jes Sorensen wrote:
>> On 3/10/21 3:59 PM, Kees Cook wrote:
>>> On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
>>>> On 3/10/21 2:45 PM, Kees Cook wrote:
>>>>
On 4/17/21 8:09 PM, Joe Perches wrote:
> On Sat, 2021-04-17 at 14:30 -0400, Jes Sorensen wrote:
>> On 4/17/21 1:52 PM, Kalle Valo wrote:
>>> "Gustavo A. R. Silva" wrote:
>>>
>>>> In preparation to enable -Wimplicit-fallthrough for Clang, fix
&g
On 3/23/21 3:36 PM, Pascal Terjan wrote:
> Based on 2001:3319 and 2357:0109 which I used to test the fix and
> 0bda:818b and 2357:0108 for which I found efuse dumps online.
>
> == 2357:0109 ==
> === Before ===
> Vendor: Realtek
> Product: \x03802.11n NI
> Serial:
> === After ===
> Vendor: Realtek
t;
> Patch applied to wireless-drivers-next.git, thanks.
>
> bf3365a856a1 rtl8xxxu: Fix fall-through warnings for Clang
>
Sorry this junk patch should not have been applied.
Jes
On 3/10/21 3:59 PM, Kees Cook wrote:
> On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
>> On 3/10/21 2:45 PM, Kees Cook wrote:
>>> On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
>>>> On 3/10/21 2:14 PM, Kees Cook wrote:
>>&g
On 3/10/21 2:45 PM, Kees Cook wrote:
> On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
>> On 3/10/21 2:14 PM, Kees Cook wrote:
>>> Hm, this conversation looks like a miscommunication, mainly? I see
>>> Gustavo, as requested by many others[1], replacing t
hanges broken out. Anyway, this
> change still needs to land, so what would be the preferred path? I think
> Gustavo could just carry it for Linus to merge without bothering you if
> that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
Instead of mutilating the kernel, Gustavo should invest in fixing the
broken clang compiler.
Thanks,
Jes
---
> 1 file changed, 4 insertions(+), 4 deletions(-)
While I wasn't CC'ed on the cover-letter I see Jakub also raised issues
about this unnecessary patch noise.
Quite frankly, this seems to be patch churn for the sake of patch churn.
If clang is broken, fix clang instead.
NACK
Jes
gt;rx_agg_buf_size / 512);
> if (rtl8xxxu_dma_agg_pages >= 0) {
> if (rtl8xxxu_dma_agg_pages <= page_thresh)
> - timeout = page_thresh;
> + ; /* do nothing */
Sorry this is the wrong way to do this. If the if statement is no longer
needed, then remove it, don't just make it do nothing.
Nack
Jes
On 10/6/20 3:45 AM, Arend Van Spriel wrote:
> + Jes
>
> On 10/5/2020 4:12 PM, Kalle Valo wrote:
>> Greg Kroah-Hartman writes:
>>
>>> On Fri, Oct 02, 2020 at 01:53:58PM +0200, Sebastian Andrzej Siewior
>>> wrote:
>>>> On 2020-10-02 13:37:25 [+020
nput/misc/Makefile |1 +
> drivers/input/misc/da7280.c | 1840
> +++
> 3 files changed, 1854 insertions(+)
> create mode 100644 drivers/input/misc/da7280.c
Hi Roy,
Overall the driver looks pretty good now. I did find one issue, see
below. If you
ting beyond the end of the cmd
buffer when doing the \0 termination.
If you change the code above to say
if (count > MAX_USER_INPUT_LEN)
count = MAX_USER_INPUT_LEN
memcpy(cmd, buf, count);
it should take care of it, and it will also return the actual count
written to the caller.
Cheers,
Jes
eless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 +++
3 files changed, 5 insertions(+)
Looks good to me! If this takes care of the warm boot problem, that's
pretty awesome.
Signed-off-by: Jes Sorensen
Jes
mean x86_64 since there's no such thing as an amd64
architecture :)
Cheers,
Jes
ch of tdma settings
- Reformat some lines to meet kernel coding style
v3:
- Add comment for rtl8723bu_set_coex_with_type()
Looks good to me!
Signed-off-by: Jes Sorensen
Jes
On 10/2/19 9:19 PM, Chris Chiu wrote:
On Wed, Oct 2, 2019 at 11:04 PM Jes Sorensen wrote:
In general I think it looks good! One nit below:
Sorry I have been traveling for the last three weeks, so just catching up.
+void rtl8723bu_set_coex_with_type(struct rtl8xxxu_priv *priv, u8 type
The one item here, I would prefer introducing some defined types to
avoid the hard coded type numbers. It's much easier to read and debug
when named.
If you shortened the name of the function to rtl8723bu_set_coex() you
won't have problems with line lengths at the calling point.
Cheers,
Jes
OFDM rate,
> and MCS rate. However, with Realtek's driver, the tx rate observed from
> wireshark under the same condition is almost 65Mbps or 72Mbps, which
> indicating that rtl8xxxu could still be further improved.
>
> Signed-off-by: Chris Chiu
> Reviewed-by: Daniel Drake
I
>> in the timer case. So even if you remove the timer running check, the
>> lock is still required here.
>
> It turns out you were right, all that really needs to be done is the
> del_timer_sync(). I've added your patch to my queue.
>
> Sorry for the trouble.
Awesome!
Sorry I was going to get back and look at it again, but Linux Plumbers
and playing sardine i a tin can got in the way.
Cheers,
Jes
rtl8723bu_set_ps_tdma(priv,
> 0x51, 0x21, 0x3, 0x10, 0x10);
> +
> rtl8723bu_set_coex_with_type(priv, 4);
> + } else if (btcoex->has_hid &&
> + btcoex->has_a2dp) {
> + rtl8723bu_set_ps_tdma(priv,
> 0x51, 0x21, 0x3, 0x10, 0x10);
> +
> rtl8723bu_set_coex_with_type(priv, 3);
> + } else {
> + rtl8723bu_set_ps_tdma(priv,
> 0x61, 0x35, 0x3, 0x11, 0x11);
> +
> rtl8723bu_set_coex_with_type(priv, 4);
> + }
Same here
Otherwise, thanks for digging into this, it's really great to see!
Cheers,
Jes
On 8/28/19 6:32 PM, Corey Minyard wrote:
> On Wed, Aug 28, 2019 at 04:36:24PM -0400, Jes Sorensen wrote:
>> From: Jes Sorensen
>>
>> I came across this in 4.16, but I believe the bug is still present
>> in current 5.x, even if it is less likely to trigger.
>>
From: Jes Sorensen
smi_mod_timer() enables the timer before setting timer_running. This
means the timer can be running when we get to stop_timer_and_thread()
without timer_running having been set, resulting in del_timer_sync()
not being called and the timer being left to cause havoc during
From: Jes Sorensen
I came across this in 4.16, but I believe the bug is still present
in current 5.x, even if it is less likely to trigger.
Basially stop_timer_and_thread() only calls del_timer_sync() if
timer_running == true. However smi_mod_timer enables the timer before
setting timer_running
On 8/12/19 10:32 AM, Kalle Valo wrote:
> Jes Sorensen writes:
>> Looks good to me! Nice work! I am actually very curious if this will
>> improve performance 8192eu as well.
>>
>> Ideally I'd like to figure out how to make host controlled rates work,
>> but in
---
Looks good to me! Nice work! I am actually very curious if this will
improve performance 8192eu as well.
Ideally I'd like to figure out how to make host controlled rates work,
but in all my experiments with that, I never really got it to work well.
Signed-off-by: Jes Sorensen
Jes
On 7/4/19 11:44 PM, Daniel Drake wrote:
> On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen wrote:
>> My point is this seems to be very dongle dependent :( We have to be
>> careful not breaking it for some users while fixing it for others.
>
> Do you still have your device?
&g
On 7/4/19 10:27 PM, Chris Chiu wrote:
> On Fri, Jul 5, 2019 at 12:43 AM Jes Sorensen wrote:
>>
>> On 7/4/19 6:55 AM, Chris Chiu wrote:
>>> The WiFi tx power of RTL8723BU is extremely low after booting. So
>>> the WiFi scan gives very limited AP list and it a
0x0, 0x280
> + * Antenna switch to PTA: | 0x200, 0x80
>*/
> - rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
> + rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
Per the documentation, shouldn't this be set to 0x200 then rather than 0x80?
We may need to put in place some code to detect whether we have normal
or inverse configuration of the dongle otherwise?
I really appreciate you're digging into this!
Cheers,
Jes
On 7/2/19 11:25 PM, Chris Chiu wrote:
> On Tue, Jul 2, 2019 at 8:44 PM Jes Sorensen wrote:
>>
>> On 6/27/19 5:52 AM, Chris Chiu wrote:
>>> The WiFi tx power of RTL8723BU is extremely low after booting. So
>>> the WiFi scan gives very limited AP list and it a
On 7/3/19 3:42 AM, Daniel Drake wrote:
> On Tue, Jul 2, 2019 at 8:42 PM Jes Sorensen wrote:
>> We definitely don't want to bring over the vendor code, since it's a
>> pile of spaghetti, but we probably need to get something sorted. This
>> went down the drain when
* initialized. First MAC returns 0xea, second MAC returns 0x00
>*/
> - if (val8 == 0xea)
> + if (val8 == 0xea || !(val16 & BIT(11)))
> macpower = false;
> else
> macpower = true;
This part I would like to ask you take a good look at the other chips to
make sure you don't break support for 8192cu, 8723au, 8188eu with this.
Cheers,
Jes
d by the vendor
> driver with btcoexist?
We definitely don't want to bring over the vendor code, since it's a
pile of spaghetti, but we probably need to get something sorted. This
went down the drain when the bluetooth driver was added without taking
it into account - long after this driver was merged.
Cheers,
Jes
e memory.
> So memset is not needed.
>
> Signed-off-by: Fuqian Huang
> ---
> drivers/net/hippi/rrunner.c | 2 --
> 1 file changed, 2 deletions(-)
Acked-by: Jes Sorensen
> diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
> index 7b9350dbebdd..2a6ec5394966 100
sk = (sta->supp_rates[0] & 0xfff) |
> + (sta->ht_cap.mcs.rx_mask[0] << 12) |
> + (sta->ht_cap.mcs.rx_mask[0] << 20);
> +
> + if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) {
> + if (sta->vht_cap.vht_supported)
> + network_type = WIRELESS_MODE_AC;
> + else if (sta->ht_cap.ht_supported)
> + network_type = WIRELESS_MODE_N_5G;
> +
> + network_type |= WIRELESS_MODE_A;
> + } else {
> + if (sta->vht_cap.vht_supported)
> + network_type = WIRELESS_MODE_AC;
> + else if (sta->ht_cap.ht_supported)
> + network_type = WIRELESS_MODE_N_24G;
> +
> + if (sta->supp_rates[0] <= 0xf)
> + network_type |= WIRELESS_MODE_B;
> + else if (sta->supp_rates[0] & 0xf)
> + network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
> + else
> + network_type |= WIRELESS_MODE_G;
> + }
> +
> + return network_type;
> +}
I always hated the wireless_mode nonsense in the realtek driver, but
maybe we cannot avoid it :(
Cheers,
Jes
is similar to the rtl8188cus(0x0bda:0x818a) and uses the same config.
>>
>> Signed-off-by: Aleksei Mamlin
>
> Patch applied to wireless-drivers-next.git, thanks.
>
> 514502c3a70b rtl8xxxu: Add rtl8188ctv support
Looks good to me too - assume it's been tested?
Thanks,
Jes
On 05/23/2018 10:26 AM, Matthew Wilcox wrote:
> On Wed, May 23, 2018 at 10:20:10AM -0400, Jes Sorensen wrote:
>>> +++ b/drivers/pci/pcie/aer/aerdrv_stats.c
>>> @@ -0,0 +1,64 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>
>> Fix the formatting please - th
On 05/23/2018 10:26 AM, Alex G. wrote:
> On 05/23/2018 09:20 AM, Jes Sorensen wrote:
>> On 05/22/2018 06:28 PM, Rajat Jain wrote:
>>> new file mode 100644
>>> index ..b9f251992209
>>> --- /dev/null
>>> +++ b/drivers/pci/pcie/aer/aerdrv_stat
; * @e_info: pointer to error info
> diff --git a/drivers/pci/pcie/aer/aerdrv_stats.c
> b/drivers/pci/pcie/aer/aerdrv_stats.c
> new file mode 100644
> index ..b9f251992209
> --- /dev/null
> +++ b/drivers/pci/pcie/aer/aerdrv_stats.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0
Fix the formatting please - that gross // gibberish doesn't belong there.
Jes
dev->name);
> goto drop;
> case E_FLG_SYN_ERR:
>
Fine with me! I do wonder if it's time to retire this driver and the
HIPPI code. I haven't had access to hardware for over a decade so I have
no idea if it even works anymore.
Jes
rdev-devel/2018-January/115390.html
>
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Jes Sorensen
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/r
On 10/25/2017 06:51 AM, Kees Cook wrote:
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Jes Sorensen
Cc: linux-hi...@sunsite.dk
Cc: net
On 10/11/2017 04:41 AM, Kalle Valo wrote:
Jes Sorensen writes:
On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
While this isn't harmful, to me this looks like pointless
On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
While this isn't harmful, to me this looks like pointless patch churn
for zero gain and it's just ugly.
Jes
Cc: Jes So
tate 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
On 05/16/2017 10:19 AM, Arnd Bergmann wrote:
On Tue, May 16, 2017 at 3:44 PM, Jes Sorensen wrote:
On 05/16/2017 07:55 AM, Stanislaw Gruszka wrote:
On Mon, May 15, 2017 at 10:39:51AM -0400, David Miller wrote:
Passing return values by reference is and always has been a really
poor way to
ould be massive and error prone change, which I'm not prefer either.
That's why you do it in multiple smaller patches rather than one ugly
giant patch.
The rt2x00 current register accessor functions makes the Realtek vendor
driver equivalent ones look pretty, which is saying something.
Jes
tty/serial/st-asc.c | 2 +-
net/decnet/af_decnet.c| 3 ++-
19 files changed, 31 insertions(+), 27 deletions(-)
rrunner.c changes look fine to me.
Jes
deletions(-)
Applied!
Thanks,
Jes
diff --git a/mdadm.c b/mdadm.c
index 0f32773..d6b5437 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1965,14 +1965,12 @@ static int misc_list(struct mddev_dev *devlist,
rv |= SetAction(dv->devname, c->action);
co
so I kept switch-case.
If you agree, I'll send v2 patch using if-expression.
I like this solution better, I much favor code that is more explicit in
what it does. Request less brain capacity for me to read :)
Cheers,
Jes
> if (mdfd >= 0) break;
> - case 1:
> + case '/':
> mdfd = open_mddev(dv->devname, 1);
> }
> if (mdfd>=0) {
While I agree the original code is ugly, I am not convinced your
replacement is a lot prettier.
Thanks,
Jes
mit patch sets,
which includes a short description of what the set includes. That way I
can just respond to the cover letter if all the patches are applied.
Thanks,
Jes
tions(-)
Applied!
Ideally I would like to get rid of the hardcoded values and use the bit
defines instead, but the original code did the same, so I am going to
take it.
I am leaving out the second patch for now, per Neil's comments. If you
feel strongly about it, please conveince Neil first :)
Thanks,
Jes
The conversion of notify from using a spinlock to a mutex missed
adding the call mutex_destroy().
Fixes: 986ab09 ("fsnotify: use a mutex instead of a spinlock to protect a
groups mark list")
Signed-off-by: Jes Sorensen
Reviewed-by: Josef Bacik
---
fs/notify/group.c | 1 +
1 file
Splice off the notification list while processing read events, which allows
it to be processed without taking and releasing the spinlock for each
event.
Signed-off-by: Jes Sorensen
Reviewed-by: Josef Bacik
---
fs/notify/inotify/inotify_user.c | 28
1 file changed
Preparing to switch inotify to code not taking the spinlock for each
event in read()
Signed-off-by: Jes Sorensen
Reviewed-by: Josef Bacik
---
fs/notify/inotify/inotify_user.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/notify/inotify/inotify_user.c b/fs
This splits up fsnotify_remove_first_event() and fsnotify_peek_first_event()
into a helper function with a wrapper, and introduces two new versions that
takes a list instead of the group as argument.
Signed-off-by: Jes Sorensen
Reviewed-by: Josef Bacik
---
fs/notify/notification.c | 38
Introduces mutex in the read() path to prevent a threaded client reading
from the same fd consuming events out of order.
This will matter when avoiding taking the spinlock when consuming each
event in the read() path.
Signed-off-by: Jes Sorensen
Reviewed-by: Josef Bacik
---
fs/notify/inotify
k, it does increase the overhead for the case where the
users tries to read one event at a time.
Last, the first patch of the series adds the missing mutex_destoy()
for mark_mutex.
Thoughts?
Jes
Jes Sorensen (5):
notify: Call mutex_destroy() before freeing mutex memory
inotify: Use mut
01,11 +1502,12 @@ static int add_to_super1(struct supertype *st,
> mdu_disk_info_t *dk,
> }
> }
>
> - if ((dk->state & 6) == 6) /* active, sync */
> + dk_state &= ~(1< + if ((dk_state & 6) == 6) /* active, sync */
> *rp = __cpu_to_le16(dk->raid_disk);
This does not look right - you haven't assigned a value to dk_state, but
then start masking bits out of it.
Cheers,
Jes
On 02/13/2017 12:54 AM, zhilong wrote:
On 02/13/2017 01:08 PM, zhilong wrote:
Hi, Jes;
On 01/13/2017 12:41 AM, Jes Sorensen wrote:
On 01/11/17 23:24, Guoqing Jiang wrote:
On 01/12/2017 12:59 AM, Jes Sorensen wrote:
On 01/11/17 11:52, Shaohua Li wrote:
On Tue, Jan 10, 2017 at 11:49:04AM
On 01/11/17 23:24, Guoqing Jiang wrote:
>
>
> On 01/12/2017 12:59 AM, Jes Sorensen wrote:
>> On 01/11/17 11:52, Shaohua Li wrote:
>>> On Tue, Jan 10, 2017 at 11:49:04AM -0600, Bruce Dubbs wrote:
>>>> Jes Sorensen wrote:
>>>>> I am pleased to an
On 01/11/17 11:52, Shaohua Li wrote:
> On Tue, Jan 10, 2017 at 11:49:04AM -0600, Bruce Dubbs wrote:
>> Jes Sorensen wrote:
>>> I am pleased to announce the availability of
>>> mdadm version 4.0
>>>
>>> It is available at the usual places:
>&
RAID.
This is my first release of mdadm. Please thank Neil Brown for his
previous work as maintainer and blame me for all the bugs I caused
since taking over.
Jes Sorensen, 2017-01-09
ernel versions impacted (at least): kernel-4.4.26, kernel-4.8.15,
> kernel-4.9.0
Well you have one foreign object in there that is not part of the
kernel and which shows up in the OOPS: DRDB
What happens when you remove that from the equation?
Jes
bss dec hex filename
>>20950 02095 wireless/realtek/rtlwifi/rc.o
>>
[snip]
> The content of your patch is OK; however, your subject is not. By
> convention, "net: wireless: realtek:" is assumed. We do, however,
> include "rtlwifi:" to indicate which part of
> drivers/net/wireless/realtek/ is referenced.
In addition, the first part of the description is useful and the file
size information is reasonable too, but ~20 lines of coccinelle scripts
in the commit message is rather pointless.
Jes
Arnd Bergmann writes:
> We accidentally print the rate before we know it for txdesc_v2:
Hi Arnd,
Thanks for the patch - Barry Day already posted a patch for this which
Kalle has applied to the wireless tree.
Cheers,
Jes
>
> wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In
-examine output.
>
> Signed-off-by: NeilBrown
> ---
>
> Hi Jes,
> this patch adds mdadm support for the failfast functionality that
> Shaohua recently included in his for-next.
> Hopefully the man-page additions provide all necessary context.
> If there is anything tha
' was declared here
>>> > u32 rate;
>>> > ^
>>> >
>>> > Introduced by commit
>>> >
>>> > b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have
>>> > access to retry count")
&g
s to the mac80211
> driver.
Now you are pissing on my name - do you really want to be taken
seriously here?
> Previous comments I made about enable_rf, rtl8xxxu_start,
> rtl8xxxu_init_device etc should be clarified. I will leave it for the
> moment as it currently serves no direct useful purpose.
I have made it very clear I want this issue resolved, but I want it
done right.
Jes
> 1 files changed, 35 insertions(+), 30 deletions(-)
Nothing that sticks out to me
Acked-by: Jes Sorensen
Jes
> diff --git a/drivers/net/ethernet/alteon/acenic.c
> b/drivers/net/ethernet/alteon/acenic.c
> index a5c1e29..16f0c70 100644
> --- a/drivers/net/ethernet/alteon/acen
Joe Perches writes:
> On Sun, 2016-10-30 at 19:02 -0400, Jes Sorensen wrote:
>> Code is 80 characters wide, and comments are /* */ never the ugly C++
>> crap.
>
> You might look at the recent Linus Torvalds authored commit
> 5e467652ffef (?printk: re-organize log_out
t_usb_anchor(&priv->rx_anchor);
> init_usb_anchor(&priv->tx_anchor);
> init_usb_anchor(&priv->int_anchor);
> @@ -6080,9 +6093,12 @@ static int rtl8xxxu_probe(struct usb_interface
> *interface,
> goto exit;
> }
>
> - ret = rtl8xxxu_init_device(hw);
> - if (ret)
> + if(!(id->idVendor == USB_VENDOR_ID_REALTEK
> + && id->idProduct == USB_PRODUCT_ID_RTL8723BU)) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
> goto exit;
> + }
Again, this coding style abuse will never go into this driver,
Jes
itself that is a
powerful statement!
Yes I want to see it work with as many devices as possible, but just
moving things around without balancing it and not explaining why is not
a fix. If we move more of the init sequence to _start() you also have to
move matching pieces to _stop().
Jes
tests.
>
> For rtl8xxxu-devel branch of
> git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
>
> Signed-off-by: John Heenan
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wirel
the latter in my book is easier to read
and reminds you what the type is when you review the code.
Point being this comes down to personal preference and stating that the
former is the right way or making that a rule and using checkpatch to
harrass people with patches to change it is bogus.
Jes
best
to ignore any guidance you have been given, I do reject your entire
patchset and you can consider this a NACK for this entire series.
I get the impression you obtain your response to any email from M-x
doctor.
Jes
source code line.
>>
>> If you actually rewrite the code or fix some real bug there.
>
> Do the proposed update steps 12 - 16 for the function "setup_conf"
> (in this software module) fit to your condition?
>
> Do you reject this update step?
I do - those changes do nothing to improve the code and simply hides a
lot of history.
Jes
ing more future proof - if the
code says 'struct foo' in the sizeof argument, what is the problem?
The one area where there is a higher risk is if the type is changed, but
that is outweighed by the fact the spelled out version is easier to
review.
Jes
ions from the Linux coding style become "bugs" also in
> your view of the software situation?
>
>> because these are a waste of time
>
> How do you value compliance with coding styles?
The Linux Coding Style is not a law, nor is it at all perfect. You
clearly misunderstood how Linux development work and you are doing a
great job wasting everyone's time with this patchset.
Jes
Joe Perches writes:
> On Sat, 2016-10-01 at 16:32 -0400, Jes Sorensen wrote:
>> Your output shows it moving to the text segment - if it's in a different
>> segment, eg. rodata, you should use output demonstrating that to justify
>> the change.
>
> For size, rodat
Joe Perches writes:
> On Sat, 2016-10-01 at 14:53 -0400, Jes Sorensen wrote:
>> Joe Perches writes:
>> > Make the init arrays const to reduce data.
>> > $ size drivers/net/wireless/realtek/rtl8xxxu/built-in.o*
>> > (allyesconfig: x86-32)
>> > tex
ent instead.
If any architecture replicates the text segment onto individual numa
nodes, this would actually be a real loss rather than a win. Some archs
used to do this, not sure if they are doing it anymore.
I am not against this patch, but I am not sure it's really a win either.
Jes
changed, 9 insertions(+), 9 deletions(-)
Sorry but this patch is just plain ridiculous. It does not improve the
code in any shape or form.
'out' as a label is perfectly legitimate and just as good as 'unlock'.
Jes
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
* num_pages, GFP_KERNEL);
> + store->filemap = kmalloc_array(num_pages,
> +sizeof(*store->filemap),
> +GFP_KERNEL);
If you have to make cosmetic changes for the sake of it, then at least
use the 80 characters per line that you have available.
This one makes the code uglier.
Jes
consistent
>> > and emit somewhat smaller object code.
>> I find if .. else constructs much easier to read than the cond ? :
>> form. I would reject any such patches.
>
> I think object code reduction generally a good thing
> but then again, I'm not a maintainer here.
I missed this part, but I am with Larry here - 'foo ? bar : boo' are
just obfuscating the code and far less clear than if or switch
statements.
Jes
Larry Finger writes:
> On 09/24/2016 12:32 PM, Joe Perches wrote:
>> Is there any value in that or is Jes' work going to make
>> doing any or all of this unnecessary and futile?
>
> That is not yet determined. The only driver that is to be replaced at
> this point is
)
1) Do not submit one giant patch modifying multiple drivers in one go
2) drivers/staging/rtl8723au is gone - had you followed 1), you wouldn't
necessarily have had to respin this patch.
3) Consider if your changes, even if technically correct, actually
improve the code (see
+ ret = _issue_deauth(padapter, da, reason, wait_ms > 0);
>
> i++;
While this part of the patch is technically correct, I would argue it
doesn't improve the code. It would make the code more readable to pass
in the wait_ms value and then have the decision made based on that in
the called function.
Cheers,
Jes
Kalle Valo writes:
> Colin Ian King wrote:
>> From: Colin Ian King
>>
>> Trivial fix to spelling mistakes in dev_dbg message.
>>
>> Signed-off-by: Colin Ian King
>> Reviewed-by: Julian Calaby
>
> I assume Jes will take this.
It's on my li
Joe Perches writes:
> On Tue, 2016-09-06 at 12:00 -0400, Jes Sorensen wrote:
>
>> Nothing wrong with these patches, however I intend to post a patch to
>> remove this driver soon, so it's kind of a waste of your time to spend
>> too many cycles on it.
>
> It
insertions(+), 39 deletions(-)
Nothing wrong with these patches, however I intend to post a patch to
remove this driver soon, so it's kind of a waste of your time to spend
too many cycles on it.
Best regards,
Jes
>
>> Signed-off-by: Baoyou Xie
>
> The title should be "rtl8xxxu: ". See:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject_name
>
> Also I assume Jes will take this.
Yes to both accounts!
Thanks,
Jes
sunbing writes:
> On Aug 11, 2016, at 23:25, Jes Sorensen wrote:
>
>> Bing Sun writes:
>>> Fixed sparse parse error:
>>> Expected constant expression in case statement.
>>>
>>> Signed-off-by: Bing Sun
>>> ---
>>> drivers/stagi
P)) {
> dscp = ip_hdr(skb)->tos & 0xfc;
> - break;
> - default:
> - return 0;
> + return dscp >> 5;
> }
> - return dscp >> 5;
> +
> + return 0;
> }
Pardon me here, but I find it really hard to see how this change is an
improvement over the old code in any shape or form.
Jes
Shiva Kerdel writes:
> Fixed some coding style issues that were detected as errors.
>
> Signed-off-by: Shiva Kerdel
You have already been told this once by Greg. Describe what you are
fixing in the commit message, and don't fix more than one type of bug
per commit.
Jes
&g
tions(-)
Excuse me, but why are you CC'ing everybody and their 217 cousins on
these patches?
The fact that someone had a patch applied in a directory close to this
file within the last 3 years is not justification for spamming them with
this.
Jes
Stefan Lippers-Hollmann writes:
> Hi
>
> On 2016-07-20, Arnd Bergmann wrote:
>> On Wednesday, July 20, 2016 11:33:43 AM CEST Jes Sorensen wrote:
>> > Arnd Bergmann writes:
>> > > On Wednesday, July 20, 2016 7:25:19 AM CEST Jes Sorensen wrote:
>> >
ion to clarify the types and simplify
> the check while removing the warning.
>
> Signed-off-by: Arnd Bergmann
> ---
> drivers/staging/rtl8192e/rtl819x_Qos.h| 3 ---
> drivers/staging/rtl8192e/rtl819x_TSProc.c | 5 +
> 2 files changed, 5 insertions(+), 3 deletions(-)
Lo
Arnd Bergmann writes:
> On Wednesday, July 20, 2016 7:25:19 AM CEST Jes Sorensen wrote:
>> Arnd Bergmann writes:
>> Well it really all depends on how much time I have and how much others
>> step up and help contribute to the code. For rtl8xxxu my plans are as
>> f
23au/hal/rtl8723a_bt-coexist.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I don't really know about the BT parts here, since I never did anything
with that part of the chip. Larry probably knows more.
The only question is whether fixing this bug changes behavior that has
1 - 100 of 420 matches
Mail list logo
