Re: [PATCH 5.15,5.10,5.4,4.19 0/2] Fix warning when tracing with large filenames
On Wed, Apr 24, 2024 at 07:20:07PM -0300, Thadeu Lima de Souza Cascardo wrote: > The warning described on patch "tracing: Increase PERF_MAX_TRACE_SIZE to > handle Sentinel1 and docker together" can be triggered with a perf probe on > do_execve with a large path. As PATH_MAX is larger than PERF_MAX_TRACE_SIZE > (2048 before the patch), the warning will trigger. > > The fix was included in 5.16, so backporting to 5.15 and earlier LTS > kernels. Also included is a patch that better describes the attempted > allocation size. All now queued up, thanks. greg k-h
Re: Please create the email alias do-not-apply-to-sta...@kernel.org -> /dev/null
On Mon, Apr 22, 2024 at 10:46:37PM +0100, Mauro Carvalho Chehab wrote: > Em Mon, 22 Apr 2024 15:25:18 -0400 > Konstantin Ryabitsev escreveu: > > > On Mon, Apr 22, 2024 at 05:49:29PM +0200, Thorsten Leemhuis wrote: > > > @Greg, BTW: should this be stable+noauto...@kernel.org or have a > > > 'vger.' > > > > No vger, just stable+whate...@kernel.org. > > > > > in it, e.g. stable+noauto...@vger.kernel.org? I assume without 'vger.' > > > is fine, just wanted to be sure, as > > > Documentation/process/stable-kernel-rules.rst in all other cases > > > specifies sta...@vger.kernel.org, so people are likely to get confused. > > > :-/ #sigh > > > > These serve two different purposes: > > > > sta...@kernel.org (goes into devnull) > > sta...@vger.kernel.org (actual mailing list) > > > > Confusion happens all the time, unfortunately. > > Yeah, I did already used sta...@kernel.org a few times in the > past. > > IMO, the best would be either for stable to also accept it or for > kernel.org mail server to return an error message (only to the > submitter) warning about the invalid address, eventually with a > hint message pointing to the correct value. sta...@kernel.org is there to route to /dev/null on purpose so that developers/maintainers who only want their patches to get picked up when they hit Linus's tree, will have happen and not notify anyone else. This is especially good when dealing with security-related things as we have had MANY people accidentally leak patches way too early by having cc: sta...@vger.kernel.org in their signed-off-by areas, and forgetting to tell git send-email to suppress cc: when sending them out for internal review. Having that bounce would just be noisy for the developers involved. thanks, greg k-h
Re: Please create the email alias do-not-apply-to-sta...@kernel.org -> /dev/null
On Thu, Apr 18, 2024 at 09:04:53AM +0200, Thorsten Leemhuis wrote: > On 17.04.24 15:38, Greg KH wrote: > > On Wed, Apr 17, 2024 at 03:21:12PM +0200, Thorsten Leemhuis wrote: > >> On 17.04.24 14:52, Konstantin Ryabitsev wrote: > >>> On Wed, Apr 17, 2024 at 09:48:18AM +0200, Thorsten Leemhuis wrote: > >>>> Could you please create the email alias > >>>> do-not-apply-to-sta...@kernel.org which redirects all mail to /dev/null, > >>>> just like sta...@kernel.org does? > >>>> > >>>> To quote: > >>>> > >>>>> How about: > >>>>> cc: # Reason goes here, and > >>>>> must be present > >>>>> > >>>>> and we can make that address be routed to /dev/null just like > >>>>> is? > >> > >> FWIW, we could go back to what I initially proposed: use the existing > >> stable tag with a pre-defined comment to mark patches that AUTOSEL et. > >> al. should not pick up: > >> https://lore.kernel.org/all/c0a08b160b286e8c98549eedb37404c6e784cf8a.1712812895.git.li...@leemhuis.info/ > > > > If you can pick a better string, possibly, yes. > > What did you think of Konstantin's > > Cc: stable+noauto...@kernel.org # Reason > > That looked like a good solution -- and I wondered why I did not come up > with that idea myself. Sure, "autosel" would also imply/mean "the > scripts/tools that look out for Fixes: tags", but does that matter? We can live with this, sure. That way no need to change anything on any kernel.org backend. thanks, greg k-h
Re: Please create the email alias do-not-apply-to-sta...@kernel.org -> /dev/null
On Wed, Apr 17, 2024 at 03:21:12PM +0200, Thorsten Leemhuis wrote: > On 17.04.24 14:52, Konstantin Ryabitsev wrote: > > On Wed, Apr 17, 2024 at 09:48:18AM +0200, Thorsten Leemhuis wrote: > >> > >> Could you please create the email alias > >> do-not-apply-to-sta...@kernel.org which redirects all mail to /dev/null, > >> just like sta...@kernel.org does? > >> > >> That's an idea GregKH brought up a few days ago here: > >> https://lore.kernel.org/all/2024041123-earthling-primarily-4656@gregkh/ > >> > >> To quote: > >> > >>> How about: > >>> cc: # Reason goes here, and must be > >>> present > >>> > >>> and we can make that address be routed to /dev/null just like > >>> is? > > First, many thx for your feedback. > > > That would make it into actual commits and probably irk maintainers and > > Linus, no? > > Yup. > > > I also don't really love the idea of overloading email > > addresses with additional semantics. Using Cc: stable kinda makes sense, > > even if it's not a real email address (but it could become at some > > point), but this feels different. > > Okay. > > > In general, I feel this information belongs in the patch basement (the > > place where change-id, base-commit, etc goes). E.g.: > > > > stable-autosel: ignore > > [This fix requires a feature that is only present in mainline] > > > > This allows passing along structured information that can be parsed by > > automated tooling without putting it into the commit. > > That afaics makes them useless for the stable team (Greg may correct me > if I'm wrong here), as they deal with the commits and have no easy, > fast, and reliable way to look up the patch posting to query this. Or is > the "patch basement" available somehow in git for each commit and I just > missed that? You are correct, as-is, that would make it useless for my tools. BUT I could, if it's possible, just look up the original in lore somehow and parse that. If it's there, does anyone have a "simple" way to map a git commit back to a lore message if it does NOT have a Link: line in it? I guess I should look at setting up a local copy of lei to dig through the git record of lkml? But what about patches that aren't cc: to lkml, I don't want to have to hit lore.kernel.org for each query, that's going to not be nice. > Guess in a better world we might use "git notes" for this, but we > currently do not use them because they iirc are somewhat tricky (they > are easily lost on rebases iirc is one of the reasons ) -- and starting > to use them just for this is not worth it. git notes never works for anything, and everyone always mentions it :) > >> There was some discussion about using something shorter, but in the end > >> there was no strong opposition and the thread ended a a few days ago. > > I feel this is a significant change to the workflow, so I would like the > > workflows list to have another go at this topic. :) > > FWIW, we could go back to what I initially proposed: use the existing > stable tag with a pre-defined comment to mark patches that AUTOSEL et. > al. should not pick up: > https://lore.kernel.org/all/c0a08b160b286e8c98549eedb37404c6e784cf8a.1712812895.git.li...@leemhuis.info/ If you can pick a better string, possibly, yes. But in the end, your proposal seems to imply: cc: sta...@kernel.org # Psych! Just kidding, never backport this! but really, that's just mean, and again, this is a VERY rare case you are trying to automate here. We have MUCH better and simpler ways for maintainers to not have their subsystems scanned for stuff like this, why are we spending all of our time on this topic? thanks, greg k-h
Re: Please create the email alias do-not-apply-to-sta...@kernel.org -> /dev/null
On Wed, Apr 17, 2024 at 09:09:26AM +0100, Mauro Carvalho Chehab wrote: > Em Wed, 17 Apr 2024 09:48:18 +0200 > Thorsten Leemhuis escreveu: > > > Hi kernel.org helpdesk! > > > > Could you please create the email alias > > do-not-apply-to-sta...@kernel.org which redirects all mail to /dev/null, > > just like sta...@kernel.org does? > > > > That's an idea GregKH brought up a few days ago here: > > https://lore.kernel.org/all/2024041123-earthling-primarily-4656@gregkh/ > > > > To quote: > > > > > How about: > > > cc: # Reason goes here, and must be > > > present > > > > > > and we can make that address be routed to /dev/null just like > > > is? > > > > There was some discussion about using something shorter, but in the end > > there was no strong opposition and the thread ended a a few days ago. > > Heh, a shorter name would make it a lot easier to remember, specially > since not wanting a patch to go to stable is an exception... I bet > I'll never remember the right syntax, needing to look at the docs > every time it would be used. > > IMO, something like: > no-stable > or > nostable > > would do the trick and would be a lot easier to remember. > > Btw, IMO, it won't hurt accepting more than one variant that > could be allowed, e. g. using a regular expression like: > > (do)?[-_]?(nt|not?).*stable That's not going to work at the mailing list server, or with my scripts, sorry. > at the scripts used by stable developers - and maybe at the ML server - to > catch different variations won't hurt, as it sounds likely that people will > end messing up with a big name like "do-not-apply-to-stable", typing > instead things like: > > do_not_apply_to_stable > dont-apply-to-stable > > and other variants. I want this very explicit that someone does not want this applied, and that it has a reason to do so. And if getting the email right to do so is the issue with that, that's fine. This is a very rare case that almost no one should normally hit. And again, if maintainers don't want their tree to have Fixes: and AUTOBOT run on them, we can easily add that to our lists. That's the simpler and more explicit thing to do for those that do not want to have anything backported they do not explicitly mark as such (some subsystems do this already, like kvm and -mm and xfs, it's fine!). This all is here because of maintainers who do NOT want to do that. thanks, greg k-h
Re: Please create the email alias do-not-apply-to-sta...@kernel.org -> /dev/null
On Wed, Apr 17, 2024 at 09:48:18AM +0200, Thorsten Leemhuis wrote: > Hi kernel.org helpdesk! > > Could you please create the email alias > do-not-apply-to-sta...@kernel.org which redirects all mail to /dev/null, > just like sta...@kernel.org does? > > That's an idea GregKH brought up a few days ago here: > https://lore.kernel.org/all/2024041123-earthling-primarily-4656@gregkh/ > > To quote: > > > How about: > > cc: # Reason goes here, and must be > > present > > > > and we can make that address be routed to /dev/null just like > > is? > > There was some discussion about using something shorter, but in the end > there was no strong opposition and the thread ended a a few days ago. > > The goal is to have a tag that developers can use in Linux kenrel > commits that have a Fixes: tag, but nevertheless should not be > backported by the stable-team without explicit request. The thread > linked above explains this in more detail. Once the address exists I'll > resubmit the patches in question that will document the tag. > > Is asking for this here like this the right way? If I need to file a > ticket somewhere or some ack from a higher authority, just let me know! I approve this message :) thanks, greg k-h
Re: [PATCH v6.1.y-v4.19.y] vhost: use kzalloc() instead of kmalloc() followed by memset()
On Mon, Feb 05, 2024 at 10:49:37AM +0530, Ajay Kaher wrote:
> From: Prathu Baronia
>
> From: Prathu Baronia
>
> commit 4d8df0f5f79f747d75a7d356d9b9ea40a4e4c8a9 upstream
>
> Use kzalloc() to allocate new zeroed out msg node instead of
> memsetting a node allocated with kmalloc().
>
> Signed-off-by: Prathu Baronia
> Message-Id: <20230522085019.42914-1-prathubaronia2...@gmail.com>
> Signed-off-by: Michael S. Tsirkin
> Reviewed-by: Stefano Garzarella
> [Ajay: This is a security fix as per CVE-2024-0340]
And this is why I am so grumpy about Red Hat and CVEs, they know how to
let us know about stuff like this, but no. Hopefully, someday soon,
they will soon not be allowed to do this anymore.
{sigh}
Now queued up, thanks.
greg k-h
Re: [PATCH] tracing/timerlat: Move hrtimer_init to timerlat_fd open()
On Thu, Feb 01, 2024 at 01:08:23PM -0500, Steven Rostedt wrote:
> On Thu, 1 Feb 2024 10:05:59 -0800
> Greg KH wrote:
>
> > > timerlat_fd =
> > > open("/sys/kernel/tracing/osnoise/per_cpu/cpu0/timerlat_fd", 'r')
> > > timerlat_fd.close();
> > >
> > > # ./taskset -c 0 ./timerlat_load.py
> > >
> >
> > Then obviously, this is a real, functional, change, so say so in the
> > kernel changelog :)
>
> I've updated the change log to remove that comment and add the reproducer.
And cc: stable properly? thanks!
Re: [PATCH] tracing/timerlat: Move hrtimer_init to timerlat_fd open()
On Thu, Feb 01, 2024 at 05:02:56PM +0100, Daniel Bristot de Oliveira wrote:
> On 2/1/24 16:44, Greg KH wrote:
> > On Thu, Feb 01, 2024 at 04:13:39PM +0100, Daniel Bristot de Oliveira wrote:
> >> Currently, the timerlat's hrtimer is initialized at the first read of
> >> timerlat_fd, and destroyed at close(). It works, but it causes an error
> >> if the user program open() and close() the file without reading.
> >
> > What error exactly happens? Userspace, or the kernel crashes?
>
> sorry, kernel crash:
>
> # echo NO_OSNOISE_WORKLOAD > /sys/kernel/debug/tracing/osnoise/options
> # echo timerlat > /sys/kernel/debug/tracing/current_tracer
>
> # cat ./timerlat_load.py
> #!/usr/bin/env python3
>
> timerlat_fd = open("/sys/kernel/tracing/osnoise/per_cpu/cpu0/timerlat_fd",
> 'r')
> timerlat_fd.close();
>
> # ./taskset -c 0 ./timerlat_load.py
>
Then obviously, this is a real, functional, change, so say so in the
kernel changelog :)
thanks,
greg k-h
Re: [PATCH] tracing/timerlat: Move hrtimer_init to timerlat_fd open()
On Thu, Feb 01, 2024 at 04:13:39PM +0100, Daniel Bristot de Oliveira wrote: > Currently, the timerlat's hrtimer is initialized at the first read of > timerlat_fd, and destroyed at close(). It works, but it causes an error > if the user program open() and close() the file without reading. What error exactly happens? Userspace, or the kernel crashes? thanks, greg k-h
Re: [PATCH] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
On Thu, Feb 01, 2024 at 11:41:56AM +0100, Pavel Machek wrote:
> +#define DEBUG
Please don't. This is dynamic, use the dynamic debugging and set it
from userspace if you want to debug the driver.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* firmware regs */
> +
> +#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
> +#define ANX7688_REG_FEATURE_CTRL0x27
> +#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
> +#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
> +#define ANX7688_REG_FW_VERSION1 0x15
> +#define ANX7688_REG_FW_VERSION0 0x16
> +
> +#define ANX7688_EEPROM_FW_LOADED 0x01
Mix of tabs and spaces, please just use tabs.
> +static const char * const anx7688_supply_names[] = {
> +"avdd33",
> +"avdd18",
> +"dvdd18",
> +"avdd10",
> +"dvdd10",
> + "i2c",
> +"hdmi_vt",
> +
> +"vconn", // power for VCONN1/VCONN2 switches
> +"vbus", // vbus power
> +};
Again, tabs vs. spaces, please use checkpatch.
> +#define ANX7688_NUM_SUPPLIES ARRAY_SIZE(anx7688_supply_names)
> +#define ANX7688_NUM_ALWAYS_ON_SUPPLIES (ANX7688_NUM_SUPPLIES - 1)
> +
> +#define ANX7688_I2C_INDEX (ANX7688_NUM_SUPPLIES - 4)
> +#define ANX7688_VCONN_INDEX (ANX7688_NUM_SUPPLIES - 2)
> +#define ANX7688_VBUS_INDEX (ANX7688_NUM_SUPPLIES - 1)
> +
> +enum {
> + ANX7688_F_POWERED,
> + ANX7688_F_CONNECTED,
> + ANX7688_F_FW_FAILED,
> + ANX7688_F_PWRSUPPLY_CHANGE,
> + ANX7688_F_CURRENT_UPDATE,
> +};
> +
> +struct anx7688 {
> +struct device *dev;
> +struct i2c_client *client;
> +struct i2c_client *client_tcpc;
> +struct regulator_bulk_data supplies[ANX7688_NUM_SUPPLIES];
> + struct power_supply *vbus_in_supply;
> + struct notifier_block vbus_in_nb;
> + int input_current_limit; // mA
> +struct gpio_desc *gpio_enable;
> +struct gpio_desc *gpio_reset;
> +struct gpio_desc *gpio_cabledet;
I'm stopping here, again, tabs, you know this :(
greg k-h
Re: [PATCH] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
On Thu, Feb 01, 2024 at 11:41:56AM +0100, Pavel Machek wrote: > --- /dev/null > +++ b/drivers/usb/typec/anx7688.c > @@ -0,0 +1,1866 @@ > +/* > + * ANX7688 USB-C HDMI bridge/PD driver > + * Did this pass checkpatch? I need a spdx line for new files please, don't force us to go back and guess in the future, that isn't nice. thanks, greg k-h
Re: [PATCH RFC 0/4] Introduce uts_release
On Wed, Jan 31, 2024 at 05:16:09PM +, John Garry wrote: > On 31/01/2024 16:22, Greg KH wrote: > > > before: > > > real0m53.591s > > > user1m1.842s > > > sys 0m9.161s > > > > > > after: > > > real0m37.481s > > > user0m46.461s > > > sys 0m7.199s > > > > > > Sending as an RFC as I need to test more of the conversions and I would > > > like to also convert more UTS_RELEASE users to prove this is proper > > > approach. > > I like it, I also think that v4l2 includes this as well as all of those > > drivers seem to rebuild when this changes, does that not happen for you > > too? > > I didn't see that. Were you were building for arm64? I can see some v4l2 > configs enabled there for the vanilla defconfig (but none for x86-64). Building for x86, maybe it's one of the other LINUX_VERSION type defines we have, sorry, can't remember, it's been a long time since I looked into it. thanks, greg k-h
Re: [PATCH RFC 0/4] Introduce uts_release
On Wed, Jan 31, 2024 at 10:48:47AM +, John Garry wrote: > When hacking it is a waste of time and compute energy that we need to > rebuild much kernel code just for changing the head git commit, like this: > > > touch include/generated/utsrelease.h > > time make -j3 > mkdir -p /home/john/mnt_sda4/john/kernel-dev2/tools/objtool && make > O=/home/john/mnt_sda4/john/kernel-dev2 subdir=tools/objtool > --no-print-directory -C objtool > INSTALL libsubcmd_headers > CALLscripts/checksyscalls.sh > CC init/version.o > AR init/built-in.a > CC kernel/sys.o > CC kernel/module/main.o > AR kernel/module/built-in.a > CC drivers/base/firmware_loader/main.o > CC kernel/trace/trace.o > AR drivers/base/firmware_loader/built-in.a > AR drivers/base/built-in.a > CC net/ethtool/ioctl.o > AR kernel/trace/built-in.a > AR kernel/built-in.a > AR net/ethtool/built-in.a > AR net/built-in.a > AR drivers/built-in.a > AR built-in.a > ... > > Files like drivers/base/firmware_loader/main.c needs to be recompiled as > it includes generated/utsrelease.h for UTS_RELEASE macro, and utsrelease.h > is regenerated when the head commit changes. > > Introduce global char uts_release[] in init/version.c, which this > mentioned code can use instead of UTS_RELEASE, meaning that we don't need > to rebuild for changing the head commit - only init/version.c needs to be > rebuilt. Whether all the references to UTS_RELEASE in the codebase are > proper is a different matter. > > For an x86_64 defconfig build for this series on my old laptop, here is > before and after rebuild time: > > before: > real0m53.591s > user1m1.842s > sys 0m9.161s > > after: > real0m37.481s > user0m46.461s > sys 0m7.199s > > Sending as an RFC as I need to test more of the conversions and I would > like to also convert more UTS_RELEASE users to prove this is proper > approach. I like it, I also think that v4l2 includes this as well as all of those drivers seem to rebuild when this changes, does that not happen for you too? Anyway, if the firmware changes work, I'm all for this, thanks for taking it on! thanks, greg k-h
Re: [PATCH] driver core: Add a guard() definition for the device_lock()
On Wed, Dec 13, 2023 at 03:02:35PM -0800, Dan Williams wrote: > At present there are ~200 usages of device_lock() in the kernel. Some of > those usages lead to "goto unlock;" patterns which have proven to be > error prone. Define a "device" guard() definition to allow for those to > be cleaned up and prevent new ones from appearing. > > Link: > http://lore.kernel.org/r/657897453dda8_269bd29...@dwillia2-mobl3.amr.corp.intel.com.notmuch > Link: > http://lore.kernel.org/r/6577b0c2a02df_a04c529...@dwillia2-xfh.jf.intel.com.notmuch > Cc: Vishal Verma > Cc: Ira Weiny > Cc: Peter Zijlstra > Cc: Greg Kroah-Hartman > Cc: Andrew Morton > Signed-off-by: Dan Williams > --- > Hi Greg, > > I wonder if you might include this change in v6.7-rc to ease some patch > sets alternately going through my tree and Andrew's tree. Those > discussions are linked above. Alternately I can can just take it through > my tree with your ack and the other use case can circle back to it in > the v6.9 cycle. Sure, I'll queue it up now for 6.7-final, makes sense to have it now for others to build off of, and for me to fix up some places in the driver core to use it as well. > I considered also defining a __free() helper similar to __free(mutex), > but I think "__free(device)" would be a surprising name for something > that drops a lock. Also, I like the syntax of guard(device) over > something like guard(device_lock) since a 'struct device *' is the > argument, not a lock type, but I'm open to your or Peter's thoughts on > the naming. guard(device); makes sense to me, as that's what you are doing here, so I'm good with it. thanks, greg k-h
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
On Tue, Nov 28, 2023 at 08:05:26AM +, Greg KH wrote:
> On Mon, Nov 27, 2023 at 11:27:07AM -0800, Matthew Maurer wrote:
> > > > >
> > > > > > With regards to future directions that likely won't work for
> > > > > > loosening it:
> > > > > > Unfortunately, the .rmeta format itself is not stable, so I
> > > > > > wouldn't want to
> > > > > > teach genksyms to open it up and split out the pieces for specific
> > > > > > functions.
> > > > > > Extending genksyms to parse Rust would also not solve the situation
> > > > > > -
> > > > > > layouts are allowed to differ across compiler versions or even (in
> > > > > > rare
> > > > > > cases) seemingly unrelated code changes.
> > > > >
> > > > > What do you mean by "layout" here? Yes, the crcs can be different
> > > > > across compiler versions and seemingly unrelated code changes
> > > > > (genksyms
> > > > > is VERY fragile) but that's ok, that's not what you are checking here.
> > > > > You want to know if the rust function signature changes or not from
> > > > > the
> > > > > last time you built the code, with the same compiler and options,
> > > > > that's
> > > > > all you are verifying.
> > What I mean by layout here is that if you write in Rust:
> > struct Foo {
> > x: i32,
> > y: i32,
> > }
> > it is not guaranteed to have the same layout across different compilations,
> > even
> > within the same compiler. See
> > https://doc.rust-lang.org/reference/type-layout.html#the-rust-representation
>
> Then you are going to have big problems, sorry.
>
> > Specifically, the compiler is allowed to arbitrarily insert padding,
> > reorder fields, etc.
> > on the same code as long as the overall alignment of the struct and
> > individual
> > alignment of the fields remains correct and non-overlapping.
> >
> > This means the compiler is *explicitly* allowed to, for example, permute x
> > and y
> > as an optimization. In the above example this is unlikely, but if you
> > instead consider
> > struct Bar {
> > x: i8,
> > y: i64,
> > z: i8,
> > }
> > It's easy to see why the compiler might decide to structure this as
> > y,x,z to reduce the
> > size of the struct. Those optimization decisions may be affected by
> > any other part of
> > the code, PGO, etc.
>
> Then you all need to figure out some way to determine how the compiler
> layed out the structure after it compiled/optimized it and be able to
> compare it to previous builds (or just generate a crc based on the
> layout it chose.)
>
> > > > > > Future directions that might work for loosening it:
> > > > > > * Generating crcs from debuginfo + compiler + flags
> > > > > > * Adding a feature to the rust compiler to dump this information.
> > > > > > This
> > > > > > is likely to
> > > > > > get pushback because Rust's current stance is that there is no
> > > > > > ability to load
> > > > > > object code built against a different library.
> > > > >
> > > > > Why not parse the function signature like we do for C?
> > Because the function signature is insufficient to check the ABI, see above.
> > > > >
> > > > > > Would setting up Rust symbols so that they have a crc built out of
> > > > > > .rmeta be
> > > > > > sufficient for you to consider this useful? If not, can you help me
> > > > > > understand
> > > > > > what level of precision would be required?
> > > > >
> > > > > What exactly does .rmeta have to do with the function signature?
> > > > > That's
> > > > > all you care about here.
> > The .rmeta file contains the decisions the compiler made about layout
> > in the crate
> > you're interfacing with. For example, the choice to encode Bar
> > with a yxz field order would be written into the .rmeta file.
>
> Ok, then yes, can you parse the .rmeta file to get that information?
>
> > > > rmeta is generated per crate.
> > > >
> > > > CRC is computed per symbol.
> > > >
> > > > They have different granularity.
> > > > It is weird to refuse a module for incompatibility
>
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
On Mon, Nov 27, 2023 at 11:27:07AM -0800, Matthew Maurer wrote:
> > > >
> > > > > With regards to future directions that likely won't work for
> > > > > loosening it:
> > > > > Unfortunately, the .rmeta format itself is not stable, so I wouldn't
> > > > > want to
> > > > > teach genksyms to open it up and split out the pieces for specific
> > > > > functions.
> > > > > Extending genksyms to parse Rust would also not solve the situation -
> > > > > layouts are allowed to differ across compiler versions or even (in
> > > > > rare
> > > > > cases) seemingly unrelated code changes.
> > > >
> > > > What do you mean by "layout" here? Yes, the crcs can be different
> > > > across compiler versions and seemingly unrelated code changes (genksyms
> > > > is VERY fragile) but that's ok, that's not what you are checking here.
> > > > You want to know if the rust function signature changes or not from the
> > > > last time you built the code, with the same compiler and options, that's
> > > > all you are verifying.
> What I mean by layout here is that if you write in Rust:
> struct Foo {
> x: i32,
> y: i32,
> }
> it is not guaranteed to have the same layout across different compilations,
> even
> within the same compiler. See
> https://doc.rust-lang.org/reference/type-layout.html#the-rust-representation
Then you are going to have big problems, sorry.
> Specifically, the compiler is allowed to arbitrarily insert padding,
> reorder fields, etc.
> on the same code as long as the overall alignment of the struct and individual
> alignment of the fields remains correct and non-overlapping.
>
> This means the compiler is *explicitly* allowed to, for example, permute x
> and y
> as an optimization. In the above example this is unlikely, but if you
> instead consider
> struct Bar {
> x: i8,
> y: i64,
> z: i8,
> }
> It's easy to see why the compiler might decide to structure this as
> y,x,z to reduce the
> size of the struct. Those optimization decisions may be affected by
> any other part of
> the code, PGO, etc.
Then you all need to figure out some way to determine how the compiler
layed out the structure after it compiled/optimized it and be able to
compare it to previous builds (or just generate a crc based on the
layout it chose.)
> > > > > Future directions that might work for loosening it:
> > > > > * Generating crcs from debuginfo + compiler + flags
> > > > > * Adding a feature to the rust compiler to dump this information. This
> > > > > is likely to
> > > > > get pushback because Rust's current stance is that there is no
> > > > > ability to load
> > > > > object code built against a different library.
> > > >
> > > > Why not parse the function signature like we do for C?
> Because the function signature is insufficient to check the ABI, see above.
> > > >
> > > > > Would setting up Rust symbols so that they have a crc built out of
> > > > > .rmeta be
> > > > > sufficient for you to consider this useful? If not, can you help me
> > > > > understand
> > > > > what level of precision would be required?
> > > >
> > > > What exactly does .rmeta have to do with the function signature? That's
> > > > all you care about here.
> The .rmeta file contains the decisions the compiler made about layout
> in the crate
> you're interfacing with. For example, the choice to encode Bar
> with a yxz field order would be written into the .rmeta file.
Ok, then yes, can you parse the .rmeta file to get that information?
> > > rmeta is generated per crate.
> > >
> > > CRC is computed per symbol.
> > >
> > > They have different granularity.
> > > It is weird to refuse a module for incompatibility
> > > of a symbol that it is not using at all.
> >
> > I agree, this should be on a per-symbol basis, so the Rust
> > infrastructure in the kernel needs to be fixed up to support this
> > properly, not just ignored like this patchset does.
> I agree there is a divergence here, I tried to point it out so that it
> wouldn't be
> a surprise later. The .rmeta file itself (which is the only way we
> could know that
> the ABI actually matches, because layout decisions are in there) is an
> unstable
> format, which is why I would be reluctant to try to parse it and find only the
> relevant portions to hash. This isn't just a "technically unstable"
> format, but one
> in which the compiler essentially just serializes out relevant internal data
> structures, so any parser for it will involve significant alterations
> on compiler
> updates, which doesn't seem like a good plan.
> >
> > thanks,
> >
> > greg k-h
> Given the above additional information, would you be interested in a patchset
> which either:
>
> A. Computes the CRC off the Rust type signature, knowing the compiler is
> allowed to change the ABI based on information not contained in the CRC.
No.
> B. Uses the CRC of the .rmeta file, knowing, as was pointed out, that this
> effectively contains the ABI of every symbol in the compilation unit, as well
> as inline
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
On Thu, Nov 23, 2023 at 08:38:45PM +0900, Masahiro Yamada wrote: > On Thu, Nov 23, 2023 at 6:05 PM Greg KH wrote: > > > > On Wed, Nov 22, 2023 at 01:04:09PM -0800, Matthew Maurer wrote: > > > > So, even if you enable CONFIG_MODVERSIONS, > > > > nothing is checked for Rust. > > > > Genksyms computes a CRC from "int foo", and > > > > the module subsystem confirms it is a "int" > > > > variable. > > > > > > > > We know this check always succeeds. > > > > > > > > Why is this useful? > > > The reason this is immediately useful is that it allows us to have Rust > > > in use with a kernel where C modules are able to benefit from MODVERSIONS > > > checking. The check would effectively be a no-op for now, as you have > > > correctly > > > determined, but we could refine it to make it more restrictive later. > > > Since the > > > existing C approach errs on the side of "it could work" rather than "it > > > will > > > work", I thought being more permissive was the correct initial solution. > > > > But it's just providing "fake" information to the CRC checker, which > > means that the guarantee of a ABI check is not true at all. > > > > So the ask for the user of "ensure that the ABI checking is correct" is > > being circumvented here, and any change in the rust side can not be > > detected at all. > > > > The kernel is a "whole", either an option works for it, or it doesn't, > > and you are splitting that guarantee here by saying "modversions will > > only work for a portion of the kernel, not the whole thing" which is > > going to cause problems for when people expect it to actually work > > properly. > > > > So, I'd strongly recommend fixing this for the rust code if you wish to > > allow modversions to be enabled at all. > > > > > With regards to future directions that likely won't work for loosening it: > > > Unfortunately, the .rmeta format itself is not stable, so I wouldn't want > > > to > > > teach genksyms to open it up and split out the pieces for specific > > > functions. > > > Extending genksyms to parse Rust would also not solve the situation - > > > layouts are allowed to differ across compiler versions or even (in rare > > > cases) seemingly unrelated code changes. > > > > What do you mean by "layout" here? Yes, the crcs can be different > > across compiler versions and seemingly unrelated code changes (genksyms > > is VERY fragile) but that's ok, that's not what you are checking here. > > You want to know if the rust function signature changes or not from the > > last time you built the code, with the same compiler and options, that's > > all you are verifying. > > > > > Future directions that might work for loosening it: > > > * Generating crcs from debuginfo + compiler + flags > > > * Adding a feature to the rust compiler to dump this information. This > > > is likely to > > > get pushback because Rust's current stance is that there is no ability > > > to load > > > object code built against a different library. > > > > Why not parse the function signature like we do for C? > > > > > Would setting up Rust symbols so that they have a crc built out of .rmeta > > > be > > > sufficient for you to consider this useful? If not, can you help me > > > understand > > > what level of precision would be required? > > > > What exactly does .rmeta have to do with the function signature? That's > > all you care about here. > > > > > rmeta is generated per crate. > > CRC is computed per symbol. > > They have different granularity. > It is weird to refuse a module for incompatibility > of a symbol that it is not using at all. I agree, this should be on a per-symbol basis, so the Rust infrastructure in the kernel needs to be fixed up to support this properly, not just ignored like this patchset does. thanks, greg k-h
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
On Wed, Nov 22, 2023 at 01:04:09PM -0800, Matthew Maurer wrote: > > So, even if you enable CONFIG_MODVERSIONS, > > nothing is checked for Rust. > > Genksyms computes a CRC from "int foo", and > > the module subsystem confirms it is a "int" > > variable. > > > > We know this check always succeeds. > > > > Why is this useful? > The reason this is immediately useful is that it allows us to have Rust > in use with a kernel where C modules are able to benefit from MODVERSIONS > checking. The check would effectively be a no-op for now, as you have > correctly > determined, but we could refine it to make it more restrictive later. > Since the > existing C approach errs on the side of "it could work" rather than "it will > work", I thought being more permissive was the correct initial solution. But it's just providing "fake" information to the CRC checker, which means that the guarantee of a ABI check is not true at all. So the ask for the user of "ensure that the ABI checking is correct" is being circumvented here, and any change in the rust side can not be detected at all. The kernel is a "whole", either an option works for it, or it doesn't, and you are splitting that guarantee here by saying "modversions will only work for a portion of the kernel, not the whole thing" which is going to cause problems for when people expect it to actually work properly. So, I'd strongly recommend fixing this for the rust code if you wish to allow modversions to be enabled at all. > With regards to future directions that likely won't work for loosening it: > Unfortunately, the .rmeta format itself is not stable, so I wouldn't want to > teach genksyms to open it up and split out the pieces for specific functions. > Extending genksyms to parse Rust would also not solve the situation - > layouts are allowed to differ across compiler versions or even (in rare > cases) seemingly unrelated code changes. What do you mean by "layout" here? Yes, the crcs can be different across compiler versions and seemingly unrelated code changes (genksyms is VERY fragile) but that's ok, that's not what you are checking here. You want to know if the rust function signature changes or not from the last time you built the code, with the same compiler and options, that's all you are verifying. > Future directions that might work for loosening it: > * Generating crcs from debuginfo + compiler + flags > * Adding a feature to the rust compiler to dump this information. This > is likely to > get pushback because Rust's current stance is that there is no ability to > load > object code built against a different library. Why not parse the function signature like we do for C? > Would setting up Rust symbols so that they have a crc built out of .rmeta be > sufficient for you to consider this useful? If not, can you help me understand > what level of precision would be required? What exactly does .rmeta have to do with the function signature? That's all you care about here. thanks, greg k-h
Re: [PATCH v2 2/5] modules: Refactor + kdoc elf_validity_cached_copy
On Sat, Nov 18, 2023 at 02:54:43AM +, Matthew Maurer wrote: > Functionality is almost identical, just structured for better > documentation and readability. Changes: > > * Section names are checked for *all* non-SHT_NULL sections, not just > SHF_ALLOC sections. We have code that accesses section names of > non-SHF_ALLOC sections (see find_any_sec for example) > * The section name check occurs *before* strcmping on section names. > Previously, it was possible to use an out-of-bounds offset to strcmp > against ".modinfo" or ".gnu.linkonce.this_module" > * strtab is checked for NUL lead+termination and nonzero size > * The symbol table is swept to ensure offsets are inbounds of strtab > > While some of these oversights would normally be worrying, all of the > potentially unverified accesses occur after signature check, and only in > response to a user who can load a kernel module. > > Signed-off-by: Matthew Maurer > --- > kernel/module/internal.h | 7 +- > kernel/module/main.c | 585 +-- > 2 files changed, 444 insertions(+), 148 deletions(-) Again, this needs to be broken into much smaller pieces before we can even review it. Would you want to review this? thanks, greg "think of the reviewers" k-h
Re: [PATCH v2 1/5] export_report: Rehabilitate script
On Sat, Nov 18, 2023 at 02:54:42AM +, Matthew Maurer wrote: > * modules.order has .o files when in a build dir, support this > * .mod.c source layout has changed, update regexes to match > * Add a stage 3, to be more robust against additional .mod.c content When you have to list different things you do in a patch, that is a huge hint that you need to break up your patch into smaller pieces. Remember, each patch can only do one logical thing. I know it feels odd, but it makes it easier to review. This patch, as-is, is nothing that I would be able to take, please make it a series. thanks, greg k-h
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 03:46:30PM +0900, Justin Stitt wrote: > On Wed, Sep 27, 2023 at 3:14 PM Greg KH wrote: > > > > On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > > > Note that folks really shouldn't be using get_maintainer on tree files > > > anyways [1]. > > > > That's not true, Linus and I use it on a daily basis this way, it's part > > of our normal workflow, AND the workflow of the kernel security team. > > > > So please don't take that valid use-case away from us. > > Fair. I'm on the side of keeping the "K:'' behavior the way it is and > that's why I'm proposing adding "D:" to provide a more granular > content matching type operating strictly on patches. It's purely > opt-in. > > The patch I linked mentioned steering folks away from using > tree files but not necessarily removing the behavior. Please don't steer folks away from it, it is a valid use case of the tool, and I would argue, one of the most important ones given how often I use it that way. Hence my objection to this verbage in the changelog, it's not correct. thanks, greg k-h
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > Note that folks really shouldn't be using get_maintainer on tree files > anyways [1]. That's not true, Linus and I use it on a daily basis this way, it's part of our normal workflow, AND the workflow of the kernel security team. So please don't take that valid use-case away from us. thanks, greg k-h
Re: SPDX: Appletalk FW license in the kernel
On Tue, Sep 26, 2023 at 12:34:03AM -0700, Christoph Hellwig wrote: > On Fri, Sep 15, 2023 at 09:39:05AM -0400, Prarit Bhargava wrote: > > To be clear, I am not asking for their removal, however, I do think a better > > license should be issued for these files. The files were trivially modified > > in 2006. It could be that the code in question is now unused and it is just > > easier to remove them. > > > > Is there anyone you know of that we could approach to determine a proper > > SPDX License for these files? > > The code contains firmware that is downloaded to the device. The proper > thing would be to convert them to separate binary files in the > linux-firmware packages. But given that the driver has seen nothing > but tree wide cleanups since the dawn of git I suspect there is no > maintainer and probably no user left. The best might be to indeed just > remove it and see if anyone screams, in which case we could bring it > back after doing the above. > We should just remove them for now, I have no objection to that at all. Want me to send the patch? thanks, greg k-h
Re: [PATCH] printk: add cpu id information to printk() output
On Fri, Sep 15, 2023 at 04:46:02PM +0800, Enlin Mu wrote: > John Ogness 于2023年9月15日周五 16:34写道： > > > > On 2023-09-15, Enlin Mu wrote: > > > Sometimes we want to print cpu id of printk() messages to consoles > > > > > > diff --git a/include/linux/threads.h b/include/linux/threads.h > > > index c34173e6c5f1..6700bd9a174f 100644 > > > --- a/include/linux/threads.h > > > +++ b/include/linux/threads.h > > > @@ -34,6 +34,9 @@ > > > #define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ > > > (sizeof(long) > 4 ? 4 * 1024 * 1024 : PID_MAX_DEFAULT)) > > > > > > +#define CPU_ID_SHIFT 23 > > > +#define CPU_ID_MASK 0xff80 > > > > This only supports 256 CPUs. I think it doesn't make sense to try to > > squish CPU and Task IDs into 32 bits. > Yes, it is not good way, > > > > What about introducing a caller_id option to always only print the CPU > > ID? Or do you really need Task _and_ CPU? >Yes, I need it.Because I need to know which CPU is printing the > log, so that I can identify the current system operation, such as load > situation and CPU busy/idle status The cpu that is printing the log isn't the one that added the log message, so I think you will have incorrect data here, right? thanks, greg k-h
Re: [PATCH] printk: add cpu id information to printk() output
On Fri, Sep 15, 2023 at 03:40:34PM +0800, Enlin Mu wrote: > From: Enlin Mu > > Sometimes we want to print cpu id of printk() messages to consoles This is rejected every few years. What has changes from the previous times this was sent? And why can't you use trace_printk()? thanks, greg k-h
Re: [PATCH 5/5] misc: zynqmp: Add afi config driver
On Tue, Apr 20, 2021 at 01:47:17PM +, Nava kishore Manne wrote:
> Hi Greg,
>
> Please find my response inline.
>
> > -Original Message-----
> > From: Greg KH
> > Sent: Tuesday, April 20, 2021 2:21 PM
> > To: Nava kishore Manne
> > Cc: robh...@kernel.org; Michal Simek ; Derek Kiernan
> > ; Dragan Cvetic ;
> > a...@arndb.de; Rajan Vaja ; Jolly Shah
> > ; Tejas Patel ; Amit Sunil
> > Dhamne ; devicet...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > chinnikishore...@gmail.com; git
> > Subject: Re: [PATCH 5/5] misc: zynqmp: Add afi config driver
> >
> > On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote:
> > > This patch adds zynqmp afi config driver.This is useful for the
> > > configuration of the PS-PL interface on Zynq US+ MPSoC platform.
> > >
> > > Signed-off-by: Nava kishore Manne
> > > ---
> > > drivers/misc/Kconfig | 11 ++
> > > drivers/misc/Makefile | 1 +
> > > drivers/misc/zynqmp-afi.c | 83
> > > +++
> > > 3 files changed, 95 insertions(+)
> > > create mode 100644 drivers/misc/zynqmp-afi.c
> > >
> > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> > > 877b43b3377d..d1ea1eeb3ac1 100644
> > > --- a/drivers/misc/Kconfig
> > > +++ b/drivers/misc/Kconfig
> > > @@ -456,6 +456,17 @@ config ZYNQ_AFI
> > > between PS and PL, the AXI port data path should be configured
> > > with the proper Bus-width values
> > >
> > > +config ZYNQMP_AFI
> > > +tristate "Xilinx ZYNQMP AFI support"
> > > +help
> > > + ZynqMP AFI driver support for writing to the AFI registers for
> > > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the
> > > + PS to the programmable logic (PL) through the AXI port. This AXI
> > > + port helps to establish the data path between the PS and PL.
> > > + In-order to establish the proper communication path between
> > > + PS and PL, the AXI port data path should be configured with
> > > + the proper Bus-width values
> > > +
> > > source "drivers/misc/c2port/Kconfig"
> > > source "drivers/misc/eeprom/Kconfig"
> > > source "drivers/misc/cb710/Kconfig"
> > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index
> > > e9b03843100f..54bd0edc511e 100644
> > > --- a/drivers/misc/Makefile
> > > +++ b/drivers/misc/Makefile
> > > @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/
> > > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> > > obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> > > obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o
> > > +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o
> > > diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c new
> > > file mode 100644 index ..a318652576d2
> > > --- /dev/null
> > > +++ b/drivers/misc/zynqmp-afi.c
> > > @@ -0,0 +1,83 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Xilinx FPGA AFI bridge.
> > > + * Copyright (c) 2018-2021 Xilinx Inc.
> > > + */
> > > +
> > > +#include
> > > +#include #include
> > > +#include #include #include
> > > + #include
> > > +
> > > +/**
> > > + * struct zynqmp_afi_fpga - AFI register description
> > > + * @value: value to be written to the register
> > > + * @regid: Register id for the register to be written
> > > + */
> > > +struct zynqmp_afi_fpga {
> > > + u32 value;
> > > + u32 regid;
> > > +};
> > > +
> > > +static int zynqmp_afi_fpga_probe(struct platform_device *pdev)
> > > +{
> > > + struct zynqmp_afi_fpga *zynqmp_afi_fpga;
> > > + struct device_node *np = pdev->dev.of_node;
> > > + int i, entries, ret;
> > > + u32 reg, val;
> > > +
> > > + zynqmp_afi_fpga = devm_kzalloc(>dev,
> > > +sizeof(*zynqmp_afi_fpga), GFP_KERNEL);
> > > + if (!zynqmp_afi_fpga)
> > > + return -ENOMEM;
> > > + platform_set_drvdata(pdev, zynqmp_afi_fpga);
> > > +
> > > + entries = of_property_count_u32_elems(np, "config-afi");
> > > + if (!entries || (entries % 2)) {
> > > + dev_err(>dev, "Invalid number of
Re: [PATCH 2/5] misc: zynq: Add afi config driver
On Tue, Apr 20, 2021 at 01:36:51PM +, Nava kishore Manne wrote:
> Hi Greg,
>
> Please find my response inline.
>
> > -Original Message-----
> > From: Greg KH
> > Sent: Tuesday, April 20, 2021 2:17 PM
> > To: Nava kishore Manne
> > Cc: robh...@kernel.org; Michal Simek ; Derek Kiernan
> > ; Dragan Cvetic ;
> > a...@arndb.de; Rajan Vaja ; Jolly Shah
> > ; Tejas Patel ; Amit Sunil
> > Dhamne ; devicet...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > chinnikishore...@gmail.com; git
> > Subject: Re: [PATCH 2/5] misc: zynq: Add afi config driver
> >
> > On Tue, Apr 20, 2021 at 01:41:50PM +0530, Nava kishore Manne wrote:
> > > This patch adds zynq afi config driver. This is useful for the
> > > configuration of the PS-PL interface on zynq platform.
> >
> > What is "PS-PL"? Can you describe it better please?
> >
> PS-PL interface is nothing but the interface between processing system(PS)
> that contains arm cores and Programmable Logic(PL) i.e FPGA.
> Will update the description in v2.
>
> > >
> > > Signed-off-by: Nava kishore Manne
> > > ---
> > > drivers/misc/Kconfig| 11 ++
> > > drivers/misc/Makefile | 1 +
> > > drivers/misc/zynq-afi.c | 81
> > > +
> > > 3 files changed, 93 insertions(+)
> > > create mode 100644 drivers/misc/zynq-afi.c
> > >
> > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> > > f532c59bb59b..877b43b3377d 100644
> > > --- a/drivers/misc/Kconfig
> > > +++ b/drivers/misc/Kconfig
> > > @@ -445,6 +445,17 @@ config HISI_HIKEY_USB
> > > switching between the dual-role USB-C port and the USB-A host
> > ports
> > > using only one USB controller.
> > >
> > > +config ZYNQ_AFI
> > > + tristate "Xilinx ZYNQ AFI support"
> > > + help
> > > + Zynq AFI driver support for writing to the AFI registers
> > > + for configuring PS_PL Bus-width. Xilinx Zynq SoC connect
> > > + the PS to the programmable logic (PL) through the AXI port.
> > > + This AXI port helps to establish the data path between the
> > > + PS and PL.In-order to establish the proper communication path
> > > + between PS and PL, the AXI port data path should be configured
> > > + with the proper Bus-width values
> > > +
> > > source "drivers/misc/c2port/Kconfig"
> > > source "drivers/misc/eeprom/Kconfig"
> > > source "drivers/misc/cb710/Kconfig"
> > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index
> > > 99b6f15a3c70..e9b03843100f 100644
> > > --- a/drivers/misc/Makefile
> > > +++ b/drivers/misc/Makefile
> > > @@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI) +=
> > habanalabs/
> > > obj-$(CONFIG_UACCE) += uacce/
> > > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> > > obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> > > +obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o
> > > diff --git a/drivers/misc/zynq-afi.c b/drivers/misc/zynq-afi.c new
> > > file mode 100644 index ..04317d1bdb98
> > > --- /dev/null
> > > +++ b/drivers/misc/zynq-afi.c
> > > @@ -0,0 +1,81 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Xilinx ZYNQ AFI driver.
> > > + * Copyright (c) 2018-2021 Xilinx Inc.
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +/* Registers and special values for doing register-based operations */
> > > +#define AFI_RDCHAN_CTRL_OFFSET 0x00
> > > +#define AFI_WRCHAN_CTRL_OFFSET 0x14
> > > +
> > > +#define AFI_BUSWIDTH_MASK0x01
> > > +
> > > +/**
> > > + * struct afi_fpga - AFI register description
> > > + * @membase: pointer to register struct
> > > + * @afi_width: AFI bus width to be written
> > > + */
> > > +struct zynq_afi_fpga {
> > > + void __iomem*membase;
> > > + u32 afi_width;
> > > +};
> > > +
> > > +static int zynq_afi_fpga_probe(struct platform_device *pdev) {
> > > + struct zynq_afi_fpga *afi_fpga;
> > > + struct resource *res;
> > > + u32 reg_val;
> > > + u32 val;
> > > +
> > &
Re: [PATCH 5/5] misc: zynqmp: Add afi config driver
On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote:
> This patch adds zynqmp afi config driver.This is useful for
> the configuration of the PS-PL interface on Zynq US+ MPSoC
> platform.
>
> Signed-off-by: Nava kishore Manne
> ---
> drivers/misc/Kconfig | 11 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/zynqmp-afi.c | 83 +++
> 3 files changed, 95 insertions(+)
> create mode 100644 drivers/misc/zynqmp-afi.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 877b43b3377d..d1ea1eeb3ac1 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -456,6 +456,17 @@ config ZYNQ_AFI
> between PS and PL, the AXI port data path should be configured
> with the proper Bus-width values
>
> +config ZYNQMP_AFI
> +tristate "Xilinx ZYNQMP AFI support"
> +help
> + ZynqMP AFI driver support for writing to the AFI registers for
> + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the
> + PS to the programmable logic (PL) through the AXI port. This AXI
> + port helps to establish the data path between the PS and PL.
> + In-order to establish the proper communication path between
> + PS and PL, the AXI port data path should be configured with
> + the proper Bus-width values
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e9b03843100f..54bd0edc511e 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/
> obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o
> +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o
> diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c
> new file mode 100644
> index ..a318652576d2
> --- /dev/null
> +++ b/drivers/misc/zynqmp-afi.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx FPGA AFI bridge.
> + * Copyright (c) 2018-2021 Xilinx Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * struct zynqmp_afi_fpga - AFI register description
> + * @value: value to be written to the register
> + * @regid: Register id for the register to be written
> + */
> +struct zynqmp_afi_fpga {
> + u32 value;
> + u32 regid;
> +};
> +
> +static int zynqmp_afi_fpga_probe(struct platform_device *pdev)
> +{
> + struct zynqmp_afi_fpga *zynqmp_afi_fpga;
> + struct device_node *np = pdev->dev.of_node;
> + int i, entries, ret;
> + u32 reg, val;
> +
> + zynqmp_afi_fpga = devm_kzalloc(>dev,
> +sizeof(*zynqmp_afi_fpga), GFP_KERNEL);
> + if (!zynqmp_afi_fpga)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, zynqmp_afi_fpga);
> +
> + entries = of_property_count_u32_elems(np, "config-afi");
> + if (!entries || (entries % 2)) {
> + dev_err(>dev, "Invalid number of registers\n");
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < entries / 2; i++) {
> + ret = of_property_read_u32_index(np, "config-afi", i * 2, );
> + if (ret) {
> + dev_err(>dev, "failed to read register\n");
> + return -EINVAL;
> + }
> + ret = of_property_read_u32_index(np, "config-afi", i * 2 + 1,
> + );
> + if (ret) {
> + dev_err(>dev, "failed to read value\n");
> + return -EINVAL;
> + }
> + ret = zynqmp_pm_afi(reg, val);
> + if (ret < 0) {
> + dev_err(>dev, "AFI register write error %d\n",
> + ret);
> + return ret;
> + }
> + }
> + return 0;
> +}
Again, why does this have to be in the kernel? All it does is make a
single call to the hardware based on some values read from the device
tree. Can't you do this from userspace?
thanks,
greg k-h
Re: [PATCH 5/5] misc: zynqmp: Add afi config driver
On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > This patch adds zynqmp afi config driver.This is useful for > the configuration of the PS-PL interface on Zynq US+ MPSoC > platform. Again, please spell out what those terms mean, as I have no idea :( > > Signed-off-by: Nava kishore Manne > --- > drivers/misc/Kconfig | 11 ++ > drivers/misc/Makefile | 1 + > drivers/misc/zynqmp-afi.c | 83 +++ > 3 files changed, 95 insertions(+) > create mode 100644 drivers/misc/zynqmp-afi.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 877b43b3377d..d1ea1eeb3ac1 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -456,6 +456,17 @@ config ZYNQ_AFI > between PS and PL, the AXI port data path should be configured > with the proper Bus-width values > > +config ZYNQMP_AFI > +tristate "Xilinx ZYNQMP AFI support" > +help > + ZynqMP AFI driver support for writing to the AFI registers for > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > + PS to the programmable logic (PL) through the AXI port. This AXI > + port helps to establish the data path between the PS and PL. > + In-order to establish the proper communication path between > + PS and PL, the AXI port data path should be configured with > + the proper Bus-width values Please use tabs properly, you mix them above, checkpatch should have caught that. thanks, greg k-h
Re: [PATCH 2/5] misc: zynq: Add afi config driver
On Tue, Apr 20, 2021 at 01:41:50PM +0530, Nava kishore Manne wrote:
> This patch adds zynq afi config driver. This is useful for
> the configuration of the PS-PL interface on zynq platform.
What is "PS-PL"? Can you describe it better please?
>
> Signed-off-by: Nava kishore Manne
> ---
> drivers/misc/Kconfig| 11 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/zynq-afi.c | 81 +
> 3 files changed, 93 insertions(+)
> create mode 100644 drivers/misc/zynq-afi.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f532c59bb59b..877b43b3377d 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -445,6 +445,17 @@ config HISI_HIKEY_USB
> switching between the dual-role USB-C port and the USB-A host ports
> using only one USB controller.
>
> +config ZYNQ_AFI
> + tristate "Xilinx ZYNQ AFI support"
> + help
> + Zynq AFI driver support for writing to the AFI registers
> + for configuring PS_PL Bus-width. Xilinx Zynq SoC connect
> + the PS to the programmable logic (PL) through the AXI port.
> + This AXI port helps to establish the data path between the
> + PS and PL.In-order to establish the proper communication path
> + between PS and PL, the AXI port data path should be configured
> + with the proper Bus-width values
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 99b6f15a3c70..e9b03843100f 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI) += habanalabs/
> obj-$(CONFIG_UACCE) += uacce/
> obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> +obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o
> diff --git a/drivers/misc/zynq-afi.c b/drivers/misc/zynq-afi.c
> new file mode 100644
> index ..04317d1bdb98
> --- /dev/null
> +++ b/drivers/misc/zynq-afi.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx ZYNQ AFI driver.
> + * Copyright (c) 2018-2021 Xilinx Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* Registers and special values for doing register-based operations */
> +#define AFI_RDCHAN_CTRL_OFFSET 0x00
> +#define AFI_WRCHAN_CTRL_OFFSET 0x14
> +
> +#define AFI_BUSWIDTH_MASK0x01
> +
> +/**
> + * struct afi_fpga - AFI register description
> + * @membase: pointer to register struct
> + * @afi_width: AFI bus width to be written
> + */
> +struct zynq_afi_fpga {
> + void __iomem*membase;
> + u32 afi_width;
> +};
> +
> +static int zynq_afi_fpga_probe(struct platform_device *pdev)
> +{
> + struct zynq_afi_fpga *afi_fpga;
> + struct resource *res;
> + u32 reg_val;
> + u32 val;
> +
> + afi_fpga = devm_kzalloc(>dev, sizeof(*afi_fpga), GFP_KERNEL);
> + if (!afi_fpga)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + afi_fpga->membase = devm_ioremap_resource(>dev, res);
> + if (IS_ERR(afi_fpga->membase))
> + return PTR_ERR(afi_fpga->membase);
> +
> + val = device_property_read_u32(>dev, "xlnx,afi-width",
> +_fpga->afi_width);
> + if (val) {
> + dev_err(>dev, "failed to get the afi bus width\n");
> + return -EINVAL;
> + }
> +
> + reg_val = readl(afi_fpga->membase + AFI_RDCHAN_CTRL_OFFSET);
> + reg_val &= ~AFI_BUSWIDTH_MASK;
> + writel(reg_val | afi_fpga->afi_width,
> +afi_fpga->membase + AFI_RDCHAN_CTRL_OFFSET);
> + reg_val = readl(afi_fpga->membase + AFI_WRCHAN_CTRL_OFFSET);
> + reg_val &= ~AFI_BUSWIDTH_MASK;
> + writel(reg_val | afi_fpga->afi_width,
> +afi_fpga->membase + AFI_WRCHAN_CTRL_OFFSET);
> +
> + return 0;
> +}
I do not understand, why is this driver needed at all? Why can't you do
the above from userspace?
All this does is write some values to the hardware at probe time, who
needs this?
thanks,
greg k-h
Re: [PATCH] video: fbdev: sm501fb: Fix deallocation of buffers order
On Tue, Apr 06, 2021 at 06:35:17PM -0500, Aditya Pakki wrote: > The resource release in sm501fb_remove() is not in the inverse order of > sm501fb_probe(), for the buffers. Release the info object after > deallocating the buffers. > > Signed-off-by: Aditya Pakki > --- > drivers/video/fbdev/sm501fb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c > index 6a52eba64559..4c32c9e88850 100644 > --- a/drivers/video/fbdev/sm501fb.c > +++ b/drivers/video/fbdev/sm501fb.c > @@ -2060,11 +2060,11 @@ static int sm501fb_remove(struct platform_device > *pdev) > unregister_framebuffer(fbinfo_pnl); > > sm501fb_stop(info); > - kfree(info); > > framebuffer_release(fbinfo_pnl); > framebuffer_release(fbinfo_crt); > > + kfree(info); > return 0; > } > > -- > 2.25.1 > There is no function change here at all, please stop it with pointless patches. greg k-h
Re: [PATCH] scsi: be2iscsi: Reset the address passed in beiscsi_iface_create_default
On Tue, Apr 06, 2021 at 07:24:45PM -0500, Aditya Pakki wrote: > if_info is a local variable that is passed to beiscsi_if_get_info. In > case of failure, the variable is free'd but not reset to NULL. The patch > avoids security issue by passing NULL to if_info. That is just not true at all. Stop submitting patches that you know are invalid. Your experiment is not ethical, and not welcome or appreciated. greg k-h
Re: [PATCH] SUNRPC: Add a check for gss_release_msg
On Tue, Apr 06, 2021 at 07:16:56PM -0500, Aditya Pakki wrote: > In gss_pipe_destroy_msg(), in case of error in msg, gss_release_msg > deletes gss_msg. The patch adds a check to avoid a potential double > free. > > Signed-off-by: Aditya Pakki > --- > net/sunrpc/auth_gss/auth_gss.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c > index 5f42aa5fc612..eb52eebb3923 100644 > --- a/net/sunrpc/auth_gss/auth_gss.c > +++ b/net/sunrpc/auth_gss/auth_gss.c > @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) > warn_gssd(); > gss_release_msg(gss_msg); > } > - gss_release_msg(gss_msg); > + if (gss_msg) > + gss_release_msg(gss_msg);a If you look at the code, this is impossible to have happen. Please stop submitting known-invalid patches. Your professor is playing around with the review process in order to achieve a paper in some strange and bizarre way. This is not ok, it is wasting our time, and we will have to report this, AGAIN, to your university... greg k-h
Re: linux-next: manual merge of the rust tree with the char-misc tree
On Fri, Apr 16, 2021 at 05:58:06PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the rust tree got a conflict in:
>
> include/uapi/linux/android/binder.h
>
> between commits:
>
> 432ff1e91694 ("binder: BINDER_FREEZE ioctl")
> ae28c1be1e54 ("binder: BINDER_GET_FROZEN_INFO ioctl")
> a7dc1e6f99df ("binder: tell userspace to dump current backtrace when
> detected oneway spamming")
>
> from the char-misc tree and commit:
>
> 1fed5dee5fbb ("Android: Binder IPC in Rust (WIP)")
>
> from the rust tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
Hah, yeah, it is going to hit this and needs to be fixed up in the rust
code. If they really want this as an enum, we can talk about it :)
thanks,
greg k-h
Re: [PATCH 0/1] coresight: Fix for v5.12-rc7
On Fri, Apr 16, 2021 at 08:54:00AM +0200, Greg KH wrote: > On Thu, Apr 15, 2021 at 02:24:03PM -0600, Mathieu Poirier wrote: > > Hi Greg, > > > > Please consider this patch as a fix for v5.12-rc7. Applies cleanly > > to your char-misc-linus branch (e49d033bddf5). > > It's too late for 5.12-final, and really my tree should be closed for > 5.13-rc1 now. I can sneak this in for the merge window, is that ok? I've just taken it for my 5.13-rc1 set of patches and added a cc: stable to get it backported to 5.12.1. thanks, greg k-h
Re: [PATCH 0/1] coresight: Fix for v5.12-rc7
On Thu, Apr 15, 2021 at 02:24:03PM -0600, Mathieu Poirier wrote: > Hi Greg, > > Please consider this patch as a fix for v5.12-rc7. Applies cleanly > to your char-misc-linus branch (e49d033bddf5). It's too late for 5.12-final, and really my tree should be closed for 5.13-rc1 now. I can sneak this in for the merge window, is that ok? thanks, greg k-h
Re: [RFC PATCH] USB:XHCI:skip hub registration
On Fri, Apr 16, 2021 at 10:43:34AM +0800, liulongfang wrote:
> On 2021/4/15 20:34, Greg KH wrote:
> > On Thu, Apr 15, 2021 at 08:22:38PM +0800, Longfang Liu wrote:
> >> When the number of ports on the USB hub is 0, skip the registration
> >> operation of the USB hub.
> >
> > That's crazy. Why not fix the hardware? How has this hub passed the
> > USB certification process?
> >
> >> The current Kunpeng930's XHCI hardware controller is defective. The number
> >> of ports on its USB3.0 bus controller is 0, and the number of ports on
> >> the USB2.0 bus controller is 1.
> >>
> >> In order to solve this problem that the USB3.0 controller does not have
> >> a port which causes the registration of the hub to fail, this patch passes
> >> the defect information by adding flags in the quirks of xhci and usb_hcd,
> >> and finally skips the registration process of the hub directly according
> >> to the results of these flags when the hub is initialized.
> >>
> >> Signed-off-by: Longfang Liu
> >> ---
> >> drivers/usb/core/hub.c | 6 ++
> >> drivers/usb/host/xhci-pci.c | 4
> >> drivers/usb/host/xhci.c | 5 +
> >> drivers/usb/host/xhci.h | 1 +
> >> include/linux/usb/hcd.h | 1 +
> >> 5 files changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index b1e14be..2d6869d 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -1769,9 +1769,15 @@ static int hub_probe(struct usb_interface *intf,
> >> const struct usb_device_id *id)
> >>struct usb_host_interface *desc;
> >>struct usb_device *hdev;
> >>struct usb_hub *hub;
> >> + struct usb_hcd *hcd;
> >>
> >>desc = intf->cur_altsetting;
> >>hdev = interface_to_usbdev(intf);
> >> + hcd = bus_to_hcd(hdev->bus);
> >> + if (hcd->usb3_no_port) {
> >> + dev_warn(>dev, "USB hub has no port\n");
> >> + return -ENODEV;
> >> + }
> >>
> >>/*
> >> * Set default autosuspend delay as 0 to speedup bus suspend,
> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> >> index ef513c2..63b89a4 100644
> >> --- a/drivers/usb/host/xhci-pci.c
> >> +++ b/drivers/usb/host/xhci-pci.c
> >> @@ -281,6 +281,10 @@ static void xhci_pci_quirks(struct device *dev,
> >> struct xhci_hcd *xhci)
> >>if (xhci->quirks & XHCI_RESET_ON_RESUME)
> >>xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
> >>"QUIRK: Resetting on resume");
> >> +
> >> + if (pdev->vendor == PCI_VENDOR_ID_HUAWEI &&
> >> + pdev->device == 0xa23c)
> >> + xhci->quirks |= XHCI_USB3_NOPORT;
> >
> > Can't we just detect this normally that there are no ports for this
> > device? Why is the device lying about how many ports it has such that
> > we have to "override" this?
> >
>
> The hub driver will check the port number in prob(). If there is no port,
> the driver will report an error log. But we hope this defective device
> does not print error log.
Defective devices deserve to have errors sent to the error log,
otherwise how will people know to tell the companies to fix them?
Again, this device can not pass USB certification, so there's not much
we should do to work around that, right?
thanks,
greg k-h
Re: [PATCH v2 4/5] staging: rtl8192e: rectified spelling mistake and replace memcmp with ether_oui_equal
On Thu, Apr 15, 2021 at 07:12:47PM +0530, Mitali Borkar wrote:
> On Wed, Apr 14, 2021 at 10:16:59AM +0200, Greg KH wrote:
> > On Wed, Apr 14, 2021 at 12:26:01PM +0530, Mitali Borkar wrote:
> > > Added a generic function of static inline bool in
> > > include/linux/etherdevice.h to replace memcmp with
> > > ether_oui_equal throughout the execution.
> > > Corrected the misspelled words in this file.
> > >
> > > Signed-off-by: Mitali Borkar
> > > ---
> > >
> > > Changes from v1:- Rectified spelling mistake and replaced memcmp with
> > > ether_oui_equal.
> > >
> > > drivers/staging/rtl8192e/rtl819x_HTProc.c | 48 +++
> > > include/linux/etherdevice.h | 5 +++
> > > 2 files changed, 29 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> > > b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> > > index ec6b46166e84..ce58feb2af9a 100644
> > > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> > > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> > > @@ -43,7 +43,7 @@ u16 MCS_DATA_RATE[2][2][77] = {
> > >810, 720, 810, 900, 900, 990} }
> > > };
> > >
> > > -static u8 UNKNOWN_BORADCOM[3] = {0x00, 0x14, 0xbf};
> > > +static u8 UNKNOWN_BROADCOM[3] = {0x00, 0x14, 0xbf};
> > >
> > > static u8 LINKSYSWRT330_LINKSYSWRT300_BROADCOM[3] = {0x00, 0x1a, 0x70};
> > >
> > > @@ -146,16 +146,16 @@ bool IsHTHalfNmodeAPs(struct rtllib_device *ieee)
> > > boolretValue = false;
> > > struct rtllib_network *net = >current_network;
> > >
> > > - if ((memcmp(net->bssid, BELKINF5D8233V1_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, BELKINF5D82334V3_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, PCI_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, EDIMAX_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, AIRLINK_RALINK, 3) == 0) ||
> > > + if ((ether_oui_equal(net->bssid, BELKINF5D8233V1_RALINK) == 0) ||
> > > + (ether_oui_equal(net->bssid, BELKINF5D82334V3_RALINK) == 0) ||
> > > + (ether_oui_equal(net->bssid, PCI_RALINK) == 0) ||
> > > + (ether_oui_equal(net->bssid, EDIMAX_RALINK) == 0) ||
> > > + (ether_oui_equal(net->bssid, AIRLINK_RALINK) == 0) ||
> > > (net->ralink_cap_exist))
> > > retValue = true;
> > > - else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) ||
> > > - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) ||
> > > - !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3) ||
> > > + else if (ether_oui_equal(net->bssid, UNKNOWN_BROADCOM) ||
> > > + ether_oui_equal(net->bssid,
> > > LINKSYSWRT330_LINKSYSWRT300_BROADCOM) ||
> > > + ether_oui_equal(net->bssid,
> > > LINKSYSWRT350_LINKSYSWRT150_BROADCOM) ||
> > >(net->broadcom_cap_exist))
> > > retValue = true;
> > > else if (net->bssht.bd_rt2rt_aggregation)
> > > @@ -179,26 +179,26 @@ static void HTIOTPeerDetermine(struct rtllib_device
> > > *ieee)
> > > pHTInfo->IOTPeer = HT_IOT_PEER_92U_SOFTAP;
> > > } else if (net->broadcom_cap_exist) {
> > > pHTInfo->IOTPeer = HT_IOT_PEER_BROADCOM;
> > > - } else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) ||
> > > - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) ||
> > > - !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3)) {
> > > + } else if (ether_oui_equal(net->bssid, UNKNOWN_BROADCOM) ||
> > > +ether_oui_equal(net->bssid,
> > > LINKSYSWRT330_LINKSYSWRT300_BROADCOM) ||
> > > +ether_oui_equal(net->bssid,
> > > LINKSYSWRT350_LINKSYSWRT150_BROADCOM)) {
> > > pHTInfo->IOTPeer = HT_IOT_PEER_BROADCOM;
> > > - } else if ((memcmp(net->bssid, BELKINF5D8233V1_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, BELKINF5D82334V3_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, PCI_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, EDIMAX_RALINK, 3) == 0) ||
> > > - (memcmp(net->bssid, AIRLINK_RALINK, 3) == 0) ||
> > > - net->ralink_cap_exist) {
> > > + } else if ((ether_oui_equal(net->bssid, BELKINF5D8233V1_RALINK) == 0) ||
> > > +(eth
Re: backport patches to linux-5.4.y
On Tue, Apr 13, 2021 at 02:11:54PM +0200, Anders Roxell wrote:
> Hi,
>
> Can these patches be backported to linux-5.4.y, I've tried to build
> perf on arm and it failed without these patches.
> fc8c0a992233 ("perf tools: Use %define api.pure full instead of %pure-parser")
> 20befbb10803 ("perf tools: Use %zd for size_t printf formats on 32-bit")
> 77d02bd00cea ("perf map: Tighten snprintf() string precision to pass
> gcc check on some 32-bit arches")
>
>
>
> Commit fc8c0a992233 ("perf tools: Use %define api.pure full instead of
> %pure-parser") fixes:
>
> util/parse-events.y:1.1-12: warning: deprecated directive:
> '%pure-parser', use '%define api.pure' [-Wdeprecated]
> 1 | %pure-parser
> | ^~~~
> | %define api.pure
>
> Commit 20befbb10803 ("perf tools: Use %zd for size_t printf formats on
> 32-bit") fixes:
>
> In file included from util/session.c:17:
> util/session.c: In function 'perf_session__process_compressed_event':
> util/session.c:91:11: error: format '%ld' expects argument of type
> 'long int', but argument 4 has type 'size_t' {aka 'unsigned int'}
> [-Werror=format=]
>91 | pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
> | ^~
> util/debug.h:16:21: note: in definition of macro 'pr_fmt'
>16 | #define pr_fmt(fmt) fmt
> | ^~~
> util/session.c:91:2: note: in expansion of macro 'pr_debug'
>91 | pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
> | ^~~~
>
> Commit 77d02bd00cea ("perf map: Tighten snprintf() string precision to
> pass gcc check on some 32-bit arches") fixes:
>
> util/map.c: In function 'map__new':
> util/map.c:125:5: error: '%s' directive output may be truncated
> writing between 1 and 2147483645 bytes into a region of size 4096
> [-Werror=format-truncation=]
> 125 |"%s/platforms/%s/arch-%s/usr/lib/%s",
> | ^~
> In file included from /usr/arm-linux-gnueabihf/include/stdio.h:867,
> from util/symbol.h:11,
> from util/map.c:2:
> /usr/arm-linux-gnueabihf/include/bits/stdio2.h:67:10: note:
> '__builtin___snprintf_chk' output 32 or more bytes (assuming
> 4294967321) into a destination of size 4096
>67 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> | ^~~~
>68 |__bos (__s), __fmt, __va_arg_pack ());
> |~
Now queued up, thanks.
greg k-h
Re: [RFC PATCH] USB:XHCI:skip hub registration
On Thu, Apr 15, 2021 at 08:22:38PM +0800, Longfang Liu wrote:
> When the number of ports on the USB hub is 0, skip the registration
> operation of the USB hub.
That's crazy. Why not fix the hardware? How has this hub passed the
USB certification process?
> The current Kunpeng930's XHCI hardware controller is defective. The number
> of ports on its USB3.0 bus controller is 0, and the number of ports on
> the USB2.0 bus controller is 1.
>
> In order to solve this problem that the USB3.0 controller does not have
> a port which causes the registration of the hub to fail, this patch passes
> the defect information by adding flags in the quirks of xhci and usb_hcd,
> and finally skips the registration process of the hub directly according
> to the results of these flags when the hub is initialized.
>
> Signed-off-by: Longfang Liu
> ---
> drivers/usb/core/hub.c | 6 ++
> drivers/usb/host/xhci-pci.c | 4
> drivers/usb/host/xhci.c | 5 +
> drivers/usb/host/xhci.h | 1 +
> include/linux/usb/hcd.h | 1 +
> 5 files changed, 17 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index b1e14be..2d6869d 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1769,9 +1769,15 @@ static int hub_probe(struct usb_interface *intf, const
> struct usb_device_id *id)
> struct usb_host_interface *desc;
> struct usb_device *hdev;
> struct usb_hub *hub;
> + struct usb_hcd *hcd;
>
> desc = intf->cur_altsetting;
> hdev = interface_to_usbdev(intf);
> + hcd = bus_to_hcd(hdev->bus);
> + if (hcd->usb3_no_port) {
> + dev_warn(>dev, "USB hub has no port\n");
> + return -ENODEV;
> + }
>
> /*
>* Set default autosuspend delay as 0 to speedup bus suspend,
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index ef513c2..63b89a4 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -281,6 +281,10 @@ static void xhci_pci_quirks(struct device *dev, struct
> xhci_hcd *xhci)
> if (xhci->quirks & XHCI_RESET_ON_RESUME)
> xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
> "QUIRK: Resetting on resume");
> +
> + if (pdev->vendor == PCI_VENDOR_ID_HUAWEI &&
> + pdev->device == 0xa23c)
> + xhci->quirks |= XHCI_USB3_NOPORT;
Can't we just detect this normally that there are no ports for this
device? Why is the device lying about how many ports it has such that
we have to "override" this?
And again, why not fix this broken hardware?
thanks,
greg k-h
Re: [PATCH v3] USB: Don't set USB_PORT_FEAT_SUSPEND on WD19's Realtek Hub
On Thu, Apr 15, 2021 at 07:48:56PM +0800, chris.c...@canonical.com wrote: > From: Chris Chiu > > Realtek Hub (0bda:5487) in Dell Dock WD19 sometimes fails to work > after the system resumes from suspend with remote wakeup enabled > device connected: > [ 1947.640907] hub 5-2.3:1.0: hub_ext_port_status failed (err = -71) > [ 1947.641208] usb 5-2.3-port5: cannot disable (err = -71) > [ 1947.641401] hub 5-2.3:1.0: hub_ext_port_status failed (err = -71) > [ 1947.641450] usb 5-2.3-port4: cannot reset (err = -71) How does other operating systems handle this? The hardware seems like it does not follow the USB spec, how did it get "certified"? > Information of this hub: > T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 10 Spd=480 MxCh= 5 > D: Ver= 2.10 Cls=09(hub ) Sub=00 Prot=02 MxPS=64 #Cfgs= 1 > P: Vendor=0bda ProdID=5487 Rev= 1.47 > S: Manufacturer=Dell Inc. > S: Product=Dell dock > C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA > I: If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=01 Driver=hub > E: Ad=81(I) Atr=03(Int.) MxPS= 1 Ivl=256ms > I:* If#= 0 Alt= 1 #EPs= 1 Cls=09(hub ) Sub=00 Prot=02 Driver=hub > E: Ad=81(I) Atr=03(Int.) MxPS= 1 Ivl=256ms > > The failure results from the ETIMEDOUT by chance when turning on > the suspend feature for the target port of the hub. The port > will be unresponsive and placed in unknown state. The hub_activate > invoked during resume will fail to get the port stautus with -EPROTO. > Then all devices connected to the hub will never be found and probed. > > The USB_PORT_FEAT_SUSPEND is not really necessary due to the > "global suspend" in USB 2.0 spec. It's only for many hub devices > which don't relay wakeup requests from the devices connected to > downstream ports. For this realtek hub, there's no problem waking > up the system from connected keyboard. > > This commit bypasses the USB_PORT_FEAT_SUSPEND for the quirky hub. Can you make this only be allowed for hubs? But why doesn't the nomal "this can not suspend properly" bit work for this? We have that for other USB devices already. thanks, greg k-h
Re: [GIT PULL] interconnect changes for 5.13
On Thu, Apr 15, 2021 at 11:09:48AM +0300, Georgi Djakov wrote: > Hello Greg, > > This is the pull request with the interconnect changes for the 5.13-rc1 > merge window. These include two new drivers. > > Patches have been in linux-next without any reported issues. Please pull > into char-misc-next. > > Thanks, > Georgi > > The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15: > > Linux 5.12-rc2 (2021-03-05 17:33:41 -0800) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/djakov/icc.git > tags/icc-5.13-rc1 Pulled and pushed out, thanks. greg k-h
Re: [GIT PULL]: Generic phy updates for v5.13 -second round
On Wed, Apr 14, 2021 at 08:47:33PM +0530, Vinod Koul wrote: > Hi Greg, > > As promised, here are some minor fixes for earlier pull request. This > includes fixes which came in after the request was sent > > The following changes since commit cbc336c09b6d6dfb24d20c955599123308fa2fe2: > > phy: fix resource_size.cocci warnings (2021-04-06 10:39:20 +0530) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git > tags/phy-for-5.13-second Pulled and pushed out, thanks. greg k-h
Re: [Resend RFC PATCH V2 08/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Wed, Apr 14, 2021 at 10:49:41AM -0400, Tianyu Lan wrote:
> From: Tianyu Lan
>
> UIO HV driver should not load in the isolation VM for security reason.
> Return ENOTSUPP in the hv_uio_probe() in the isolation VM.
>
> Signed-off-by: Tianyu Lan
> ---
> drivers/uio/uio_hv_generic.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 0330ba99730e..678b021d66f8 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
>
> #include "../hv/hyperv_vmbus.h"
>
> @@ -241,6 +242,10 @@ hv_uio_probe(struct hv_device *dev,
> void *ring_buffer;
> int ret;
>
> + /* UIO driver should not be loaded in the isolation VM.*/
> + if (hv_is_isolation_supported())
> + return -ENOTSUPP;
> +
> /* Communicating with host has to be via shared memory not hypercall */
> if (!channel->offermsg.monitor_allocated) {
> dev_err(>device, "vmbus channel requires hypercall\n");
> --
> 2.25.1
>
Again you send out known-wrong patches?
:(
Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Wed, Apr 14, 2021 at 11:20:19PM +0800, Tianyu Lan wrote: > Hi Greg: > Thanks for your review. > > On 4/14/2021 12:00 AM, Greg KH wrote: > > On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote: > > > From: Tianyu Lan > > > > > > UIO HV driver should not load in the isolation VM for security reason. > > > > Why? I need a lot more excuse than that. > > The reason is that ring buffers have been marked as visible to host. > UIO driver will expose these buffers to user space and user space > driver hasn't done some secure check for data from host. This > is considered as insecure in isolation VM. But as this is a VM choice, why did the VM mark those as visible in the first place? thanks, greg k-h
Re: [PATCH 4/7] staging: rtl8723bs: replace DBG_871X_SEL_NL with netdev_dbg()
On Wed, Apr 14, 2021 at 11:43:41AM +0200, Fabio Aiuto wrote: > On Wed, Apr 14, 2021 at 10:26:01AM +0200, Greg KH wrote: > > > - DBG_871X_SEL_NL(sel, "%10s %16s %8s %10s %11s %14s\n", > > > - "TH_L2H_ini", "TH_EDCCA_HL_diff", "IGI_Base", > > > - "ForceEDCCA", "AdapEn_RSSI", "IGI_LowerBound"); > > > - DBG_871X_SEL_NL(sel, "0x%-8x %-16d 0x%-6x %-10d %-11u %-14u\n", > > > - (u8)odm->TH_L2H_ini, > > > - odm->TH_EDCCA_HL_diff, > > > - odm->IGI_Base, > > > - odm->ForceEDCCA, > > > - odm->AdapEn_RSSI, > > > - odm->IGI_LowerBound > > > - ); > > > + netdev_dbg(adapter->pnetdev, "%10s %16s %8s %10s %11s %14s\n", > > > +"TH_L2H_ini", "TH_EDCCA_HL_diff", "IGI_Base", "ForceEDCCA", > > > +"AdapEn_RSSI", "IGI_LowerBound");netdev_dbg(adapter->pnetdev, > > > +"0x%-8x %-16d > > > 0x%-6x %-10d %-11u %-14u\n", > > > + > > > (u8)odm->TH_L2H_ini, > > > + > > > odm->TH_EDCCA_HL_diff, > > > +odm->IGI_Base, > > > +odm->ForceEDCCA, > > > +odm->AdapEn_RSSI, > > > + > > > odm->IGI_LowerBound); > > > > Something went wrong with this change :( > > thanks Greg, I sent an example to Julia, that reproduce the problem. > As soon as it gets fixed I will send you a v2. You can fix it up yourself "by hand" for now, no need to wait for a tool fix :)
Re: [PATCH v6 1/2] staging: rtl8192e: remove parentheses around boolean expression
On Wed, Apr 14, 2021 at 12:51:55PM +0530, Mitali Borkar wrote: > Removed unnecessary parentheses around '!xyz' boolean expression as '!' > has higher precedance than '||' > > Signed-off-by: Mitali Borkar > --- This series does not apply to my tree :(
Re: [PATCH 4/7] staging: rtl8723bs: replace DBG_871X_SEL_NL with netdev_dbg()
On Tue, Apr 13, 2021 at 04:56:32PM +0200, Fabio Aiuto wrote:
> replace DGB_871X_SEL_NL macro with netdev_dbg().
>
> DBG_871X_SEL_NL macro expands to a raw prink call or a
> seq_printf if selected stream _is not_ a local
> debug symbol set to null.
> This second scenario never occurs so replace
> all macro usages with netdev_dbg().
>
> This is done with the following coccinelle script:
>
> @@
> expression sel;
> expression list args;
> identifier padapter;
> identifier func;
> @@
>
> func(..., struct adapter *padapter, ...) {
> <...
> - DBG_871X_SEL_NL(sel, args);
> + netdev_dbg(padapter->pnetdev, args);
> ...>
> }
>
> Signed-off-by: Fabio Aiuto
> ---
> drivers/staging/rtl8723bs/core/rtw_debug.c | 16 +++
> drivers/staging/rtl8723bs/core/rtw_odm.c | 49 +++---
> drivers/staging/rtl8723bs/hal/hal_com.c| 31 ++
> 3 files changed, 46 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
> b/drivers/staging/rtl8723bs/core/rtw_debug.c
> index 324c7e5248f8..79fd968bb147 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
> @@ -20,7 +20,7 @@ void sd_f0_reg_dump(void *sel, struct adapter *adapter)
>
> for (i = 0x0; i <= 0xff; i++) {
> if (i%16 == 0)
> - DBG_871X_SEL_NL(sel, "0x%02x ", i);
> + netdev_dbg(adapter->pnetdev, "0x%02x ", i);
>
> DBG_871X_SEL(sel, "%02x ", rtw_sd_f0_read8(adapter, i));
>
> @@ -35,11 +35,11 @@ void mac_reg_dump(void *sel, struct adapter *adapter)
> {
> int i, j = 1;
>
> - DBG_871X_SEL_NL(sel, "=== MAC REG ===\n");
> + netdev_dbg(adapter->pnetdev, "=== MAC REG ===\n");
>
> for (i = 0x0; i < 0x800; i += 4) {
> if (j%4 == 1)
> - DBG_871X_SEL_NL(sel, "0x%03x", i);
> + netdev_dbg(adapter->pnetdev, "0x%03x", i);
> DBG_871X_SEL(sel, " 0x%08x ", rtw_read32(adapter, i));
> if ((j++)%4 == 0)
> DBG_871X_SEL(sel, "\n");
> @@ -50,10 +50,10 @@ void bb_reg_dump(void *sel, struct adapter *adapter)
> {
> int i, j = 1;
>
> - DBG_871X_SEL_NL(sel, "=== BB REG ===\n");
> + netdev_dbg(adapter->pnetdev, "=== BB REG ===\n");
> for (i = 0x800; i < 0x1000 ; i += 4) {
> if (j%4 == 1)
> - DBG_871X_SEL_NL(sel, "0x%03x", i);
> + netdev_dbg(adapter->pnetdev, "0x%03x", i);
> DBG_871X_SEL(sel, " 0x%08x ", rtw_read32(adapter, i));
> if ((j++)%4 == 0)
> DBG_871X_SEL(sel, "\n");
> @@ -73,14 +73,14 @@ void rf_reg_dump(void *sel, struct adapter *adapter)
> else
> path_nums = 2;
>
> - DBG_871X_SEL_NL(sel, "=== RF REG ===\n");
> + netdev_dbg(adapter->pnetdev, "=== RF REG ===\n");
>
> for (path = 0; path < path_nums; path++) {
> - DBG_871X_SEL_NL(sel, "RF_Path(%x)\n", path);
> + netdev_dbg(adapter->pnetdev, "RF_Path(%x)\n", path);
> for (i = 0; i < 0x100; i++) {
> value = rtw_hal_read_rfreg(adapter, path, i,
> 0x);
> if (j%4 == 1)
> - DBG_871X_SEL_NL(sel, "0x%02x ", i);
> + netdev_dbg(adapter->pnetdev, "0x%02x ", i);
> DBG_871X_SEL(sel, " 0x%08x ", value);
> if ((j++)%4 == 0)
> DBG_871X_SEL(sel, "\n");
> diff --git a/drivers/staging/rtl8723bs/core/rtw_odm.c
> b/drivers/staging/rtl8723bs/core/rtw_odm.c
> index 53f7cc0444ba..084f6ae040ee 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_odm.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_odm.c
> @@ -96,12 +96,13 @@ void rtw_odm_dbg_comp_msg(void *sel, struct adapter
> *adapter)
> int i;
>
> rtw_hal_get_def_var(adapter, HW_DEF_ODM_DBG_FLAG, _comp);
> - DBG_871X_SEL_NL(sel, "odm.DebugComponents = 0x%016llx\n", dbg_comp);
> + netdev_dbg(adapter->pnetdev, "odm.DebugComponents = 0x%016llx\n",
> +dbg_comp);
> for (i = 0; i < RTW_ODM_COMP_MAX; i++) {
> if (odm_comp_str[i])
> - DBG_871X_SEL_NL(sel, "%cBIT%-2d %s\n",
> - (BIT0 << i) & dbg_comp ? '+' : ' ',
> - i, odm_comp_str[i]);
> + netdev_dbg(adapter->pnetdev, "%cBIT%-2d %s\n",
> +(BIT0 << i) & dbg_comp ? '+' : ' ', i,
> +odm_comp_str[i]);
> }
> }
>
> @@ -116,11 +117,11 @@ void rtw_odm_dbg_level_msg(void *sel, struct adapter
> *adapter)
> int i;
>
> rtw_hal_get_def_var(adapter, HW_DEF_ODM_DBG_LEVEL, _level);
> - DBG_871X_SEL_NL(sel, "odm.DebugLevel = %u\n", dbg_level);
> +
Re: [PATCH 5/7] staging: rtl8723bs: put a new line after ';'
On Tue, Apr 13, 2021 at 04:56:33PM +0200, Fabio Aiuto wrote: > fix the following post commit hook checkpatch issue: > > ERROR: space required after that ';' (ctx:VxV) > 232: FILE: drivers/staging/rtl8723bs/core/rtw_odm.c:160: > +"AdapEn_RSSI", "IGI_LowerBound");netdev_dbg > (adapter->pnetdev, > > This was coccinelle script output coding style issue. > > Signed-off-by: Fabio Aiuto > --- > drivers/staging/rtl8723bs/core/rtw_odm.c | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/core/rtw_odm.c > b/drivers/staging/rtl8723bs/core/rtw_odm.c > index 084f6ae040ee..f4a0ef428564 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_odm.c > +++ b/drivers/staging/rtl8723bs/core/rtw_odm.c > @@ -157,14 +157,15 @@ void rtw_odm_adaptivity_parm_msg(void *sel, struct > adapter *adapter) > > netdev_dbg(adapter->pnetdev, "%10s %16s %8s %10s %11s %14s\n", > "TH_L2H_ini", "TH_EDCCA_HL_diff", "IGI_Base", "ForceEDCCA", > -"AdapEn_RSSI", "IGI_LowerBound");netdev_dbg(adapter->pnetdev, > -"0x%-8x %-16d > 0x%-6x %-10d %-11u %-14u\n", > - > (u8)odm->TH_L2H_ini, > - > odm->TH_EDCCA_HL_diff, > -odm->IGI_Base, > -odm->ForceEDCCA, > -odm->AdapEn_RSSI, > - > odm->IGI_LowerBound); > +"AdapEn_RSSI", "IGI_LowerBound"); > + netdev_dbg(adapter->pnetdev, > +"0x%-8x %-16d 0x%-6x %-10d %-11u %-14u\n", > +(u8)odm->TH_L2H_ini, > +odm->TH_EDCCA_HL_diff, > +odm->IGI_Base, > +odm->ForceEDCCA, > +odm->AdapEn_RSSI, > +odm->IGI_LowerBound); > } > > void rtw_odm_adaptivity_parm_set(struct adapter *adapter, s8 TH_L2H_ini, > -- > 2.20.1 This patch should not be needed, your commit that caused this error should not have done so. Please fix that instead. thanks, greg k-h
Re: Subject: [PATCH v2 0/5] staging: rtl8192e: CLeanup patchset for style issues in rtl819x_Y.c files
On Wed, Apr 14, 2021 at 12:24:44PM +0530, Mitali Borkar wrote: > Changes from v1:- Dropped 6/6 from and made this as a patchset of 5 as > alignment of code is done in following patches. Why is "Subject:" listed in your subject line? Do not manually add it after git format-patch has created it... thanks, greg k-h
Re: [PATCH v2 4/5] staging: rtl8192e: rectified spelling mistake and replace memcmp with ether_oui_equal
On Wed, Apr 14, 2021 at 12:26:01PM +0530, Mitali Borkar wrote:
> Added a generic function of static inline bool in
> include/linux/etherdevice.h to replace memcmp with
> ether_oui_equal throughout the execution.
> Corrected the misspelled words in this file.
>
> Signed-off-by: Mitali Borkar
> ---
>
> Changes from v1:- Rectified spelling mistake and replaced memcmp with
> ether_oui_equal.
>
> drivers/staging/rtl8192e/rtl819x_HTProc.c | 48 +++
> include/linux/etherdevice.h | 5 +++
> 2 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> index ec6b46166e84..ce58feb2af9a 100644
> --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> @@ -43,7 +43,7 @@ u16 MCS_DATA_RATE[2][2][77] = {
>810, 720, 810, 900, 900, 990} }
> };
>
> -static u8 UNKNOWN_BORADCOM[3] = {0x00, 0x14, 0xbf};
> +static u8 UNKNOWN_BROADCOM[3] = {0x00, 0x14, 0xbf};
>
> static u8 LINKSYSWRT330_LINKSYSWRT300_BROADCOM[3] = {0x00, 0x1a, 0x70};
>
> @@ -146,16 +146,16 @@ bool IsHTHalfNmodeAPs(struct rtllib_device *ieee)
> boolretValue = false;
> struct rtllib_network *net = >current_network;
>
> - if ((memcmp(net->bssid, BELKINF5D8233V1_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, BELKINF5D82334V3_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, PCI_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, EDIMAX_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, AIRLINK_RALINK, 3) == 0) ||
> + if ((ether_oui_equal(net->bssid, BELKINF5D8233V1_RALINK) == 0) ||
> + (ether_oui_equal(net->bssid, BELKINF5D82334V3_RALINK) == 0) ||
> + (ether_oui_equal(net->bssid, PCI_RALINK) == 0) ||
> + (ether_oui_equal(net->bssid, EDIMAX_RALINK) == 0) ||
> + (ether_oui_equal(net->bssid, AIRLINK_RALINK) == 0) ||
> (net->ralink_cap_exist))
> retValue = true;
> - else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) ||
> - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) ||
> - !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3) ||
> + else if (ether_oui_equal(net->bssid, UNKNOWN_BROADCOM) ||
> + ether_oui_equal(net->bssid,
> LINKSYSWRT330_LINKSYSWRT300_BROADCOM) ||
> + ether_oui_equal(net->bssid,
> LINKSYSWRT350_LINKSYSWRT150_BROADCOM) ||
>(net->broadcom_cap_exist))
> retValue = true;
> else if (net->bssht.bd_rt2rt_aggregation)
> @@ -179,26 +179,26 @@ static void HTIOTPeerDetermine(struct rtllib_device
> *ieee)
> pHTInfo->IOTPeer = HT_IOT_PEER_92U_SOFTAP;
> } else if (net->broadcom_cap_exist) {
> pHTInfo->IOTPeer = HT_IOT_PEER_BROADCOM;
> - } else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) ||
> - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) ||
> - !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3)) {
> + } else if (ether_oui_equal(net->bssid, UNKNOWN_BROADCOM) ||
> +ether_oui_equal(net->bssid,
> LINKSYSWRT330_LINKSYSWRT300_BROADCOM) ||
> +ether_oui_equal(net->bssid,
> LINKSYSWRT350_LINKSYSWRT150_BROADCOM)) {
> pHTInfo->IOTPeer = HT_IOT_PEER_BROADCOM;
> - } else if ((memcmp(net->bssid, BELKINF5D8233V1_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, BELKINF5D82334V3_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, PCI_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, EDIMAX_RALINK, 3) == 0) ||
> - (memcmp(net->bssid, AIRLINK_RALINK, 3) == 0) ||
> - net->ralink_cap_exist) {
> + } else if ((ether_oui_equal(net->bssid, BELKINF5D8233V1_RALINK) == 0) ||
> +(ether_oui_equal(net->bssid, BELKINF5D82334V3_RALINK) == 0)
> ||
> +(ether_oui_equal(net->bssid, PCI_RALINK) == 0) ||
> +(ether_oui_equal(net->bssid, EDIMAX_RALINK) == 0) ||
> +(ether_oui_equal(net->bssid, AIRLINK_RALINK) == 0) ||
> +net->ralink_cap_exist) {
> pHTInfo->IOTPeer = HT_IOT_PEER_RALINK;
> } else if ((net->atheros_cap_exist) ||
> - (memcmp(net->bssid, DLINK_ATHEROS_1, 3) == 0) ||
> - (memcmp(net->bssid, DLINK_ATHEROS_2, 3) == 0)) {
> +(ether_oui_equal(net->bssid, DLINK_ATHEROS_1) == 0) ||
> +(ether_oui_equal(net->bssid, DLINK_ATHEROS_2) == 0)) {
> pHTInfo->IOTPeer = HT_IOT_PEER_ATHEROS;
> - } else if ((memcmp(net->bssid, CISCO_BROADCOM, 3) == 0) ||
> - net->cisco_cap_exist) {
> + } else if ((ether_oui_equal(net->bssid, CISCO_BROADCOM) == 0) ||
> +net->cisco_cap_exist) {
> pHTInfo->IOTPeer = HT_IOT_PEER_CISCO;
> - } else if ((memcmp(net->bssid, LINKSYS_MARVELL_4400N,
Re: [PATCH 5.4 v3 1/1] iommu/vt-d: Fix agaw for a supported 48 bit guest address width
On Mon, Apr 12, 2021 at 01:27:35PM -0700, Saeed Mirzamohammadi wrote: > The IOMMU driver calculates the guest addressability for a DMA request > based on the value of the mgaw reported from the IOMMU. However, this > is a fused value and as mentioned in the spec, the guest width > should be calculated based on the minimum of supported adjusted guest > address width (SAGAW) and MGAW. > > This is from specification: > "Guest addressability for a given DMA request is limited to the > minimum of the value reported through this field and the adjusted > guest address width of the corresponding page-table structure. > (Adjusted guest address widths supported by hardware are reported > through the SAGAW field)." > > This causes domain initialization to fail and following > errors appear for EHCI PCI driver: > > [2.486393] ehci-pci :01:00.4: EHCI Host Controller > [2.486624] ehci-pci :01:00.4: new USB bus registered, assigned bus > number 1 > [2.489127] ehci-pci :01:00.4: DMAR: Allocating domain failed > [2.489350] ehci-pci :01:00.4: DMAR: 32bit DMA uses non-identity > mapping > [2.489359] ehci-pci :01:00.4: can't setup: -12 > [2.489531] ehci-pci :01:00.4: USB bus 1 deregistered > [2.490023] ehci-pci :01:00.4: init :01:00.4 fail, -12 > [2.490358] ehci-pci: probe of :01:00.4 failed with error -12 > > This issue happens when the value of the sagaw corresponds to a > 48-bit agaw. This fix updates the calculation of the agaw based on > the minimum of IOMMU's sagaw value and MGAW. > > Cc: sta...@vger.kernel.org > Signed-off-by: Saeed Mirzamohammadi > Tested-by: Camille Lu > Reviewed-by: Lu Baolu > > --- Also when you resend this, please state the commit that this fixes, as this must be a regression, right? What kernel version did this previous work for? thanks, greg k-h
Re: [PATCH] usb: core: remove unused including
On Wed, Apr 14, 2021 at 02:05:40PM +0800, Yang Li wrote: > Fix the following versioncheck warning: > ./drivers/usb/core/hcd.c: 14 linux/version.h not needed. > > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > drivers/usb/core/hcd.c | 1 - > 1 file changed, 1 deletion(-) I am now adding any patch sent to me from the "Abaci Robot" to my local blacklist and they will be ignored as you have constantly kept ignoring my simple request to do basic build testing of your patches before sending them out. Because you have not done that, you are obviously trying to waste developer and reviewer's time with stuff like this, which is not acceptable at all. *plonk*
Re: [PATCH 1/2] kunit: Fix formatting of KUNIT tests to meet the standard
On Wed, Apr 14, 2021 at 12:33:02AM -0400, Nico Pache wrote: > There are few instances of KUNIT tests that are not properly defined. > This commit focuses on correcting these issues to match the standard > defined in the Documentation. > > Issues Fixed: > - Tests should default to KUNIT_ALL_TESTS > - Tests configs tristate should have `if !KUNIT_ALL_TESTS` > - Tests should end in KUNIT_TEST, some fixes have been applied to > correct issues were KUNIT_TESTS is used or KUNIT is not mentioned. You are changing lots of different things all in one patch, please break this up into "one config option change per patch" to make it easier to review and get merged through the proper subsystem. thanks, greg k-h
Re: [PATCH 1/2] kunit: Fix formatting of KUNIT tests to meet the standard
On Wed, Apr 14, 2021 at 12:33:02AM -0400, Nico Pache wrote: > There are few instances of KUNIT tests that are not properly defined. > This commit focuses on correcting these issues to match the standard > defined in the Documentation. > > Issues Fixed: > - Tests should default to KUNIT_ALL_TESTS > - Tests configs tristate should have `if !KUNIT_ALL_TESTS` > - Tests should end in KUNIT_TEST, some fixes have been applied to > correct issues were KUNIT_TESTS is used or KUNIT is not mentioned. > > No functional changes other than CONFIG name changes > Signed-off-by: Nico Pache Please put a blank line before your signed-off-by, as is required by the tools :( thanks, greg k-h
Re: [PATCH 5.4 v3 1/1] iommu/vt-d: Fix agaw for a supported 48 bit guest address width
On Tue, Apr 13, 2021 at 11:05:34AM -0700, Saeed Mirzamohammadi wrote: > Hi Greg, > > I don’t have any commit ID since the fix is not in mainline or any Linus’ > tree yet. The driver has completely changed for newer stable versions (and > also mainline) and the fix only applies for 5.4, 4.19, and 4.14 stable > kernels. Why can we not just take what is in mainline? And if not, then you need to document the heck out of this in the changelog text, and get all of the related maintainers in the area to sign off on this. Diverging from Linus's tree creates a big burden over time, you have to make this really really obvious why you are doing this, and what you are doing here so that everyone agrees with it. Remember, 90% of all of these types of "do it differently than Linus's tree" are buggy and cause problems, be very careful. Please fix up and resend. thanks, greg k-h
Re: [PATCH] hrtimer: Update softirq_expires_next correctly after __hrtimer_get_next_event()
On Tue, Apr 13, 2021 at 09:55:08AM -0700, Maciej Żenczykowski wrote:
> This patch (or at least the version of it that showed up in 5.10.24
> LTS when combined with Android Common Kernel and some arm phone
> platform drivers) causes a roughly 60% regression (from ~5.3-6 gbps
> down to ~2.2gbps) on running pktgen when egressing via ncm gadget on a
> SuperSpeed+ 10gbps USB3 connection.
>
> The regression is not there in 5.10.23, and is present in 5.10.24 and
> 5.10.26. Reverting just this one patch is confirmed to restore
> performance (both on 5.10.24 and 5.10.26).
>
> We don't know the cause, as we know nothing about hrtimers... but we
> lightly suspect that the ncm->task_timer in f_ncm.c is perhaps not
> firing as often as it should...
>
> Unfortunately I have no idea how to replicate this on commonly
> available hardware (or with a pure stable or 5.11/5.12 Linus tree)
> since it requires a gadget capable usb3.1 10gbps controller (which I
> only have access to in combination with a 5.10-based arm+dwc3 soc).
>
> (the regression is visible with just usb3.0, but it's smaller due to
> usb3.0 topping out at just under 4gbps, though it still drops to
> 2.2gbps -- and this still doesn't help since usb3 gadget capable
> controllers are nearly as rare)
>
To give context, the commit is now 46eb1701c046 ("hrtimer: Update
softirq_expires_next correctly after __hrtimer_get_next_event()") and is
attached below.
The f_ncm.c driver is doing a lot of calls to hrtimer_start() with mode
HRTIMER_MODE_REL_SOFT for I think every packet it gets. If that should
not be happening, we can easily fix it but I guess no one has actually
had fast USB devices that noticed this until now :)
thanks,
greg k-h
--
commit 46eb1701c046cc18c032fa68f3c8ccbf24483ee4
Author: Anna-Maria Behnsen
Date: Tue Feb 23 17:02:40 2021 +0100
hrtimer: Update softirq_expires_next correctly after
__hrtimer_get_next_event()
hrtimer_force_reprogram() and hrtimer_interrupt() invokes
__hrtimer_get_next_event() to find the earliest expiry time of hrtimer
bases. __hrtimer_get_next_event() does not update
cpu_base::[softirq_]_expires_next to preserve reprogramming logic. That
needs to be done at the callsites.
hrtimer_force_reprogram() updates cpu_base::softirq_expires_next only when
the first expiring timer is a softirq timer and the soft interrupt is not
activated. That's wrong because cpu_base::softirq_expires_next is left
stale when the first expiring timer of all bases is a timer which expires
in hard interrupt context. hrtimer_interrupt() does never update
cpu_base::softirq_expires_next which is wrong too.
That becomes a problem when clock_settime() sets CLOCK_REALTIME forward and
the first soft expiring timer is in the CLOCK_REALTIME_SOFT base. Setting
CLOCK_REALTIME forward moves the clock MONOTONIC based expiry time of that
timer before the stale cpu_base::softirq_expires_next.
cpu_base::softirq_expires_next is cached to make the check for raising the
soft interrupt fast. In the above case the soft interrupt won't be raised
until clock monotonic reaches the stale cpu_base::softirq_expires_next
value. That's incorrect, but what's worse it that if the softirq timer
becomes the first expiring timer of all clock bases after the hard expiry
timer has been handled the reprogramming of the clockevent from
hrtimer_interrupt() will result in an interrupt storm. That happens because
the reprogramming does not use cpu_base::softirq_expires_next, it uses
__hrtimer_get_next_event() which returns the actual expiry time. Once clock
MONOTONIC reaches cpu_base::softirq_expires_next the soft interrupt is
raised and the storm subsides.
Change the logic in hrtimer_force_reprogram() to evaluate the soft and hard
bases seperately, update softirq_expires_next and handle the case when a
soft expiring timer is the first of all bases by comparing the expiry times
and updating the required cpu base fields. Split this functionality into a
separate function to be able to use it in hrtimer_interrupt() as well
without copy paste.
Fixes: 5da70160462e ("hrtimer: Implement support for softirq based
hrtimers")
Reported-by: Mikael Beckius
Suggested-by: Thomas Gleixner
Tested-by: Mikael Beckius
Signed-off-by: Anna-Maria Behnsen
Signed-off-by: Thomas Gleixner
Signed-off-by: Ingo Molnar
Link:
https://lore.kernel.org/r/20210223160240.27518-1-anna-ma...@linutronix.de
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 743c852e10f2..788b9d137de4 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -546,8 +546,11 @@ static ktime_t __hrtimer_next_event_base(struct
hrtimer_cpu_base *cpu_base,
}
/*
- * Recomputes cpu_base::*next_timer and returns the earliest expires_next but
- * does not set cpu_base::*expires_next, that is done by hrtimer_reprogram.
Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote: > From: Tianyu Lan > > UIO HV driver should not load in the isolation VM for security reason. Why? I need a lot more excuse than that. Why would the vm allow UIO devices to bind to it if it was not possible? Shouldn't the VM be handling this type of logic and not forcing all individual hyperv drivers to do this? This feels wrong... thanks, greg k-h
Re: [RFC V2 PATCH 8/12] UIO/Hyper-V: Not load UIO HV driver in the isolation VM.
On Tue, Apr 13, 2021 at 11:22:13AM -0400, Tianyu Lan wrote:
> From: Tianyu Lan
>
> UIO HV driver should not load in the isolation VM for security reason.
> Return ENOTSUPP in the hv_uio_probe() in the isolation VM.
>
> Signed-off-by: Tianyu Lan
> ---
> drivers/uio/uio_hv_generic.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 0330ba99730e..678b021d66f8 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
No driver should be having to add "asm" include files.
>
> #include "../hv/hyperv_vmbus.h"
>
> @@ -241,6 +242,10 @@ hv_uio_probe(struct hv_device *dev,
> void *ring_buffer;
> int ret;
>
> + /* UIO driver should not be loaded in the isolation VM.*/
> + if (hv_is_isolation_supported())
> + return -ENOTSUPP;
> +
> /* Communicating with host has to be via shared memory not hypercall */
> if (!channel->offermsg.monitor_allocated) {
> dev_err(>device, "vmbus channel requires hypercall\n");
> --
> 2.25.1
>
Always run checkpatch.pl on your patches so you do not get grumpy
maintainers telling you to run checkpatch.pl on your patch :(
{sigh}
Re: [PATCH v5 0/3] staging: rtl8192e: Cleanup patchset for style issues in rtl819x_HTProc.c
On Tue, Apr 13, 2021 at 04:15:03PM +0530, Mitali Borkar wrote: > On Tue, Apr 13, 2021 at 09:52:48AM +0200, Greg KH wrote: > > On Tue, Apr 13, 2021 at 08:55:03AM +0530, Mitali Borkar wrote: > > > Changes from v4:- > > > [PATCH v4 1/3]:- No changes. > > > [PATCH v4 2/3]:- No changes. > > > [PATCH V4 3/3]:- Removed casts and parentheses. > > > > This series does not apply cleanly, please rebase and resend. > > > Resend as v6? Does not apply cleanly as in? Were mails not threaded > properly? Yes, send a v6, after rebasing on my tree and the staging-testing branch and resend. They were threaded properly, it looks like others have made changes to the same files that you were, which is quite common when doing small cleanup changes like this. thanks, greg k-h
Re: cocci script hints request
On Tue, Apr 13, 2021 at 11:24:56AM +0200, Fabio Aiuto wrote:
> On Tue, Apr 13, 2021 at 11:11:38AM +0200, Greg KH wrote:
> > On Tue, Apr 13, 2021 at 11:04:01AM +0200, Fabio Aiuto wrote:
> > > Hi,
> > >
> > > I would like to improve the following coccinelle script:
> > >
> > > @@
> > > expression a, fmt;
> > > expression list var_args;
> > > @@
> > >
> > > - DBG_871X_LEVEL(a, fmt, var_args);
> > > + printk(fmt, var_args);
> > >
> > > I would replace the DBG_871X_LEVEL macro with printk,
> >
> > No you really do not, you want to change that to a dev_*() call instead
> > depending on the "level" of the message.
> >
> > No "raw" printk() calls please, I will just reject them :)
> >
> > thanks,
> >
> > greg k-h
>
> but there are very few occurences of DBG_871X_LEVEL in module init functions:
Then do those "by hand", if they really are needed.
Drivers, when they are working properly, are totally quiet.
>
> static int __init rtw_drv_entry(void)
> {
> int ret;
>
> DBG_871X_LEVEL(_drv_always_, "module init start\n");
Horrible, please remove.
> dump_drv_version(RTW_DBGDUMP);
> #ifdef BTCOEXVERSION
> DBG_871X_LEVEL(_drv_always_, "rtl8723bs BT-Coex version = %s\n",
> BTCOEXVERSION);
Not needed at all.
> #endif /* BTCOEXVERSION */
>
> sdio_drvpriv.drv_registered = true;
>
> ret = sdio_register_driver(_drvpriv.r871xs_drv);
> if (ret != 0) {
> sdio_drvpriv.drv_registered = false;
> rtw_ndev_notifier_unregister();
> }
>
> DBG_871X_LEVEL(_drv_always_, "module init ret =%d\n", ret);
Again, not needed this is noise and if someone really needs to debug
this, they can use the built-in kernel ftrace logic instead.
> return ret;
> }
>
> where I don't have a device available... shall I pass NULL to
> first argument?
No, that would be a mess :)
I bet almost all of these can be removed if they are like the above
examples as we do not need a lot of "look, the code got here!" type of
messages at all.
> Another question: may I use netdev_dbg in case of rtl8723bs?
Yes please, that is even better and recommended.
thanks,
greg k-h
Re: cocci script hints request
On Tue, Apr 13, 2021 at 11:04:01AM +0200, Fabio Aiuto wrote: > Hi, > > I would like to improve the following coccinelle script: > > @@ > expression a, fmt; > expression list var_args; > @@ > > - DBG_871X_LEVEL(a, fmt, var_args); > + printk(fmt, var_args); > > I would replace the DBG_871X_LEVEL macro with printk, No you really do not, you want to change that to a dev_*() call instead depending on the "level" of the message. No "raw" printk() calls please, I will just reject them :) thanks, greg k-h
Re: [PATCH v5 0/3] staging: rtl8192e: Cleanup patchset for style issues in rtl819x_HTProc.c
On Tue, Apr 13, 2021 at 08:55:03AM +0530, Mitali Borkar wrote: > Changes from v4:- > [PATCH v4 1/3]:- No changes. > [PATCH v4 2/3]:- No changes. > [PATCH V4 3/3]:- Removed casts and parentheses. This series does not apply cleanly, please rebase and resend. thanks, greg k-h
Re: [PATCH v7 3/3] bio: add limit_bio_size sysfs
On Tue, Apr 13, 2021 at 11:55:02AM +0900, Changheun Lee wrote: > Add limit_bio_size block sysfs node to limit bio size. > Queue flag QUEUE_FLAG_LIMIT_BIO_SIZE will be set if limit_bio_size is set. > And bio max size will be limited by queue max sectors via > QUEUE_FLAG_LIMIT_BIO_SIZE set. > > Signed-off-by: Changheun Lee > --- > Documentation/ABI/testing/sysfs-block | 10 ++ > Documentation/block/queue-sysfs.rst | 7 +++ > block/blk-sysfs.c | 3 +++ > 3 files changed, 20 insertions(+) Isn't it too late to change the sysfs entry after the device has been probed and initialized by the kernel as the kernel does not look at this value after that? Why do you need a userspace knob for this? What tool is going to ever change this, and what logic is it going to use to change it? Why can't the kernel also just "do the right thing" and properly detect this option as well as userspace can? thanks, greg k-h
Re: [PATCH 5.4 v3 1/1] iommu/vt-d: Fix agaw for a supported 48 bit guest address width
On Mon, Apr 12, 2021 at 01:27:35PM -0700, Saeed Mirzamohammadi wrote: > The IOMMU driver calculates the guest addressability for a DMA request > based on the value of the mgaw reported from the IOMMU. However, this > is a fused value and as mentioned in the spec, the guest width > should be calculated based on the minimum of supported adjusted guest > address width (SAGAW) and MGAW. > > This is from specification: > "Guest addressability for a given DMA request is limited to the > minimum of the value reported through this field and the adjusted > guest address width of the corresponding page-table structure. > (Adjusted guest address widths supported by hardware are reported > through the SAGAW field)." > > This causes domain initialization to fail and following > errors appear for EHCI PCI driver: > > [2.486393] ehci-pci :01:00.4: EHCI Host Controller > [2.486624] ehci-pci :01:00.4: new USB bus registered, assigned bus > number 1 > [2.489127] ehci-pci :01:00.4: DMAR: Allocating domain failed > [2.489350] ehci-pci :01:00.4: DMAR: 32bit DMA uses non-identity > mapping > [2.489359] ehci-pci :01:00.4: can't setup: -12 > [2.489531] ehci-pci :01:00.4: USB bus 1 deregistered > [2.490023] ehci-pci :01:00.4: init :01:00.4 fail, -12 > [2.490358] ehci-pci: probe of :01:00.4 failed with error -12 > > This issue happens when the value of the sagaw corresponds to a > 48-bit agaw. This fix updates the calculation of the agaw based on > the minimum of IOMMU's sagaw value and MGAW. > > Cc: sta...@vger.kernel.org > Signed-off-by: Saeed Mirzamohammadi > Tested-by: Camille Lu > Reviewed-by: Lu Baolu What is the git commit id of this patch in Linus's tree? thanks, greg k-h
Re: Subject: [PATCH v2] staging: media: meson: vdec: declare u32 as static const appropriately
On Tue, Apr 13, 2021 at 11:57:52AM +0530, Mitali Borkar wrote: > Declared 32 bit unsigned int as static constant inside a function > appropriately. > > Reported-by: kernel test robot > Signed-off-by: Mitali Borkar Why does your Subject: line have "Subject:" twice?
Re: linux-next: build failure after merge of the usb tree
On Mon, Apr 12, 2021 at 09:36:55PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the usb tree, today's linux-next build (x86_64 almodconfig
> modules_install) failed like this:
>
> depmod: ERROR: Cycle detected: usbcore -> typec -> usbcore
> depmod: ERROR: Found 2 modules in dependency cycles!
>
> Caused by commit
>
> 63cd78617350 ("usb: Link the ports to the connectors they are attached to")
>
> I have reverted that commit for today.
Ugh, good catch, odd that 0-day didn't catch that :(
I'll go revert it in my tree as well. Heikki, can you send a fixed up
version when you get a chance?
thanks,
greg k-h
Re: [PATCH 12/12] USB: cdc-acm: add more Maxlinear/Exar models to ignore list
On Mon, Apr 12, 2021 at 11:55:57AM +0200, Johan Hovold wrote: > From: Mauro Carvalho Chehab > > Now that the xr_serial got support for other models, add their USB IDs > as well. > > The Maxlinear/Exar USB UARTs can be used in either ACM mode using the > cdc-acm driver or in "custom driver" mode in which further features such > as hardware and software flow control, GPIO control and in-band > line-status reporting are available. > > In ACM mode the device always enables RTS/CTS flow control, something > which could prevent transmission in case the CTS input isn't wired up > correctly. > > Ensure that cdc_acm will not bind to these devices if the custom > USB-serial driver is enabled. > > Signed-off-by: Mauro Carvalho Chehab > Link: > https://lore.kernel.org/r/5155887a764cbc11f8da0217fe08a24a77d120b4.1616571453.git.mchehab+hua...@kernel.org > [ johan: rewrite commit message, clean up entries ] > Cc: Oliver Neukum > Signed-off-by: Johan Hovold Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 1/6] staging: media: intel-ipu3: replace bit shifts with BIT() macro
On Mon, Apr 12, 2021 at 01:44:35PM +0300, Sakari Ailus wrote: > Hi Greg, > > On Mon, Apr 12, 2021 at 11:49:43AM +0200, Greg KH wrote: > > On Mon, Apr 12, 2021 at 12:42:30PM +0300, Sakari Ailus wrote: > > > Hi Mitali, > > > > > > On Mon, Apr 12, 2021 at 04:38:39AM +0530, Mitali Borkar wrote: > > > > Added #include and replaced bit shifts by BIT() macro. > > > > This BIT() macro from linux/bitops.h is used to define > > > > IPU3_UAPI_GRID_Y_START_EN > > > > and IPU3_UAPI_AWB_RGBS_THR_B_* bitmask. > > > > Use of macro is better and neater. It maintains consistency. > > > > Reported by checkpatch. > > > > > > > > Signed-off-by: Mitali Borkar > > > > --- > > > > drivers/staging/media/ipu3/include/intel-ipu3.h | 7 --- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > index edd8edda0647..589d5ccee3a7 100644 > > > > --- a/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h > > > > @@ -5,6 +5,7 @@ > > > > #define __IPU3_UAPI_H > > > > > > > > #include > > > > +#include > > > > > > > > /* from /drivers/staging/media/ipu3/include/videodev2.h */ > > > > > > > > @@ -22,11 +23,11 @@ > > > > #define IPU3_UAPI_MAX_BUBBLE_SIZE 10 > > > > > > > > #define IPU3_UAPI_GRID_START_MASK ((1 << 12) - 1) > > > > -#define IPU3_UAPI_GRID_Y_START_EN (1 << 15) > > > > +#define IPU3_UAPI_GRID_Y_START_EN BIT(15) > > > > > > This header is used in user space where you don't have the BIT() macro. > > > > If that is true, why is it not in a "uapi" subdir within this driver? > > > > Otherwise it is not obvious at all that this is the case :( > > It defines an interface towards the user space and the argument has been a > staging driver shouldn't be doing that (for the lack of ABI stability), > hence leaving it where it is currently. I think we are talking past each other here... If you have a userspace api, then put that in a .h file that has a "uapi" in the path. Just because you have this in a staging driver does not mean you should not have such a thing, otherwise you are going to constantly fight against people sending you patches like this as there is no obvious way to determine this. So how about moving this file to: drivers/staging/media/ipu3/include/uapi/intel-ipu3.h thanks, greg k-h
Re: [Outreachy kernel] [PATCH] staging: rtl8192u: Remove function
On Mon, Apr 12, 2021 at 12:12:34PM +0200, Fabio M. De Francesco wrote:
> On Monday, April 12, 2021 11:42:51 AM CEST Greg KH wrote:
> > On Sun, Apr 11, 2021 at 08:48:13PM +0200, Fabio M. De Francesco wrote:
> > > Remove cmpk_handle_query_config_rx() because it just initializes a
> > > local
> > > variable and then returns "void".
> > >
> > > Signed-off-by: Fabio M. De Francesco
> > > ---
> > >
> > > drivers/staging/rtl8192u/r819xU_cmdpkt.c | 40
> > > 1 file changed, 40 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> > > b/drivers/staging/rtl8192u/r819xU_cmdpkt.c index
> > > 4cece40a92f6..d5a54c2d3086 100644
> > > --- a/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> > > +++ b/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> > > @@ -249,46 +249,6 @@ static void cmpk_handle_interrupt_status(struct
> > > net_device *dev, u8 *pmsg)>
> > > DMESG("< cmpk_handle_interrupt_status()\n");
> > >
> > > }
> > >
> > > -/*
> > > - - * Function:cmpk_handle_query_config_rx()
> > > - *
> > > - * Overview:The function is responsible for extract the message
> > > from - * firmware. It will contain dedicated info in
> > > - * ws-06-0063-rtl8190-command-packet-specification.
> Please
> > > - * refer to chapter "Beacon State Element".
> > > - *
> > > - * Input: u8*pmsg - Message Pointer of the
> command packet.
> > > - *
> > > - * Output: NONE
> > > - *
> > > - * Return: NONE
> > > - *
> > > - * Revised History:
> > > - * When Who Remark
> > > - * 05/12/2008 amy Create Version 0 porting from
> windows code.
> > > - *
> > > -
> > > *-
> > > -- - */
> > > -static void cmpk_handle_query_config_rx(struct net_device *dev, u8
> > > *pmsg) -{
> > > - struct cmpk_query_cfg rx_query_cfg;
> > > -
> > > - /* 1. Extract TX feedback info from RFD to temp structure
> buffer. */
> > > - /* It seems that FW use big endian(MIPS) and DRV use little
> endian in
> > > - * windows OS. So we have to read the content byte by byte or
> > > transfer
> > > - * endian type before copy the message copy.
> > > - */
> > > - rx_query_cfg.cfg_action = (pmsg[4] & 0x80) >> 7;
> > > - rx_query_cfg.cfg_type = (pmsg[4] & 0x60) >> 5;
> > > - rx_query_cfg.cfg_size = (pmsg[4] & 0x18) >> 3;
> > > - rx_query_cfg.cfg_page = (pmsg[6] & 0x0F) >> 0;
> > > - rx_query_cfg.cfg_offset = pmsg[7];
> > > - rx_query_cfg.value = (pmsg[8] << 24) |
> (pmsg[9] << 16) |
> > > - (pmsg[10] << 8)
> | (pmsg[11] << 0);
> > > - rx_query_cfg.mask = (pmsg[12] << 24) |
> (pmsg[13] << 16) |
> > > - (pmsg[14] << 8)
> | (pmsg[15] << 0);
> > > -}
> > > -
> > >
> > > /*
> > > ->
> > > * Function: cmpk_count_tx_status()
> > > *
> >
> > Always test-build your patches as they can not break the build. You
> > obviously did not do that here, why not?
> >
> I can't see that where the build of rtl8192u is broken.
> The following lines are from the compilation log:
>
> git/kernels/staging> make -j8 drivers/staging/rtl8192u/
> [...]
> CC [M] drivers/staging/rtl8192u/r819xU_cmdpkt.o
> CC [M] drivers/staging/rtl8192u/r819xU_cmdpkt.o
> [...]
> LD [M] drivers/staging/rtl8192u/r8192u_usb.o
>
> No errors are reported.
>
> What am I missing?
The function is used elsewhere in this file :(
Re: [PATCH 0/4] USB: serial: closing-wait fixes and cleanups
On Mon, Apr 12, 2021 at 11:38:11AM +0200, Johan Hovold wrote: > The port drain_delay parameter is used to add a time-based delay when > closing the port in order to allow the transmit FIFO to drain in cases > where we don't know how to tell if the FIFO is empty. > > This series removes a redundant time-based delay which is no longer > needed, and documents the reason for two other uses where such a delay > is needed to let the transmitter shift register clear. As it turns out, > this is really only needed for one of the two device types handled by > the ti_usb_3410_5052 driver. Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 1/6] staging: media: intel-ipu3: replace bit shifts with BIT() macro
On Mon, Apr 12, 2021 at 12:42:30PM +0300, Sakari Ailus wrote: > Hi Mitali, > > On Mon, Apr 12, 2021 at 04:38:39AM +0530, Mitali Borkar wrote: > > Added #include and replaced bit shifts by BIT() macro. > > This BIT() macro from linux/bitops.h is used to define > > IPU3_UAPI_GRID_Y_START_EN > > and IPU3_UAPI_AWB_RGBS_THR_B_* bitmask. > > Use of macro is better and neater. It maintains consistency. > > Reported by checkpatch. > > > > Signed-off-by: Mitali Borkar > > --- > > drivers/staging/media/ipu3/include/intel-ipu3.h | 7 --- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h > > b/drivers/staging/media/ipu3/include/intel-ipu3.h > > index edd8edda0647..589d5ccee3a7 100644 > > --- a/drivers/staging/media/ipu3/include/intel-ipu3.h > > +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h > > @@ -5,6 +5,7 @@ > > #define __IPU3_UAPI_H > > > > #include > > +#include > > > > /* from /drivers/staging/media/ipu3/include/videodev2.h */ > > > > @@ -22,11 +23,11 @@ > > #define IPU3_UAPI_MAX_BUBBLE_SIZE 10 > > > > #define IPU3_UAPI_GRID_START_MASK ((1 << 12) - 1) > > -#define IPU3_UAPI_GRID_Y_START_EN (1 << 15) > > +#define IPU3_UAPI_GRID_Y_START_EN BIT(15) > > This header is used in user space where you don't have the BIT() macro. If that is true, why is it not in a "uapi" subdir within this driver? Otherwise it is not obvious at all that this is the case :( thanks, greg k-h
Re: [Outreachy kernel] [PATCH v2] staging: rtl8192u: Remove variable set but not used
On Sun, Apr 11, 2021 at 08:36:34PM +0200, Fabio M. De Francesco wrote:
> Remove variable "int ret", declared but not used.
>
> Signed-off-by: Fabio M. De Francesco
> ---
>
> Changes from v1: Change the text of the subject and log.
>
> drivers/staging/rtl8192u/r8192U_core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index f48186a89fa1..30055de66239 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -902,7 +902,6 @@ static void rtl8192_hard_data_xmit(struct sk_buff *skb,
> struct net_device *dev,
> int rate)
> {
> struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> - int ret;
> unsigned long flags;
> struct cb_desc *tcb_desc = (struct cb_desc *)(skb->cb +
> MAX_DEV_ADDR_SIZE);
> u8 queue_index = tcb_desc->queue_index;
> --
> 2.31.1
>
Breaks the build, why did you not test this patch?
thanks,
greg k-h
Re: [PATCH v4 0/3] staging: rtl8192e: modified log message
On Sun, Apr 11, 2021 at 05:04:46PM +0530, Mitali Borkar wrote: > This patch fixes style issues > Changes from v3:- > [PATCH v3 1/3]:- Modified log message. > [PATCH V3 2/3]:- No changes. > [PATCH v3 3/3]:- No changes Your subject line here is odd :( Please fix up. thanks, greg k-h
Re: [Outreachy kernel] [PATCH] staging: rtl8192u: Remove function
On Sun, Apr 11, 2021 at 08:48:13PM +0200, Fabio M. De Francesco wrote:
> Remove cmpk_handle_query_config_rx() because it just initializes a local
> variable and then returns "void".
>
> Signed-off-by: Fabio M. De Francesco
> ---
> drivers/staging/rtl8192u/r819xU_cmdpkt.c | 40
> 1 file changed, 40 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> b/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> index 4cece40a92f6..d5a54c2d3086 100644
> --- a/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> +++ b/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> @@ -249,46 +249,6 @@ static void cmpk_handle_interrupt_status(struct
> net_device *dev, u8 *pmsg)
> DMESG("< cmpk_handle_interrupt_status()\n");
> }
>
> -/*-
> - * Function:cmpk_handle_query_config_rx()
> - *
> - * Overview:The function is responsible for extract the message from
> - * firmware. It will contain dedicated info in
> - * ws-06-0063-rtl8190-command-packet-specification. Please
> - * refer to chapter "Beacon State Element".
> - *
> - * Input: u8*pmsg - Message Pointer of the command packet.
> - *
> - * Output: NONE
> - *
> - * Return: NONE
> - *
> - * Revised History:
> - * When Who Remark
> - * 05/12/2008 amy Create Version 0 porting from windows
> code.
> - *
> - *---
> - */
> -static void cmpk_handle_query_config_rx(struct net_device *dev, u8 *pmsg)
> -{
> - struct cmpk_query_cfg rx_query_cfg;
> -
> - /* 1. Extract TX feedback info from RFD to temp structure buffer. */
> - /* It seems that FW use big endian(MIPS) and DRV use little endian in
> - * windows OS. So we have to read the content byte by byte or transfer
> - * endian type before copy the message copy.
> - */
> - rx_query_cfg.cfg_action = (pmsg[4] & 0x80) >> 7;
> - rx_query_cfg.cfg_type = (pmsg[4] & 0x60) >> 5;
> - rx_query_cfg.cfg_size = (pmsg[4] & 0x18) >> 3;
> - rx_query_cfg.cfg_page = (pmsg[6] & 0x0F) >> 0;
> - rx_query_cfg.cfg_offset = pmsg[7];
> - rx_query_cfg.value = (pmsg[8] << 24) | (pmsg[9] << 16) |
> - (pmsg[10] << 8) | (pmsg[11] << 0);
> - rx_query_cfg.mask = (pmsg[12] << 24) | (pmsg[13] << 16) |
> - (pmsg[14] << 8) | (pmsg[15] << 0);
> -}
> -
>
> /*-
> * Function: cmpk_count_tx_status()
> *
> --
> 2.31.1
>
>
Always test-build your patches as they can not break the build. You
obviously did not do that here, why not?
thanks,
greg k-h
Re: [PATCH 2/3] staging rtl8723bs: split long lines
On Sun, Apr 11, 2021 at 02:57:36PM +0200, Fabio Aiuto wrote: > fix following post-commit hook checkpatch issues: > > WARNING: line length of 116 exceeds 100 columns > 30: FILE: drivers/staging/rtl8723bs/hal/sdio_halinit.c:883: > + HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, > PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, rtl8723B_enter_lps_flow); > > WARNING: line length of 119 exceeds 100 columns > 41: FILE: drivers/staging/rtl8723bs/hal/sdio_halinit.c:912: > + HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, > PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, rtl8723B_card_disable_flow); > > Signed-off-by: Fabio Aiuto > --- > drivers/staging/rtl8723bs/hal/sdio_halinit.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) You forgot a ':' in your subject line :(
Re: [PATCH] staging: emxx_udc: remove useless variable
On Mon, Apr 12, 2021 at 10:53:10AM +0800, Jiapeng Chong wrote: > Fix the following gcc warning: > > vers/staging/emxx_udc/emxx_udc.c:41:19: warning: ‘driver_desc’ defined > but not used [-Wunused-const-variable=]. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/staging/emxx_udc/emxx_udc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/emxx_udc/emxx_udc.c > b/drivers/staging/emxx_udc/emxx_udc.c > index 3536c03..741147a 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.c > +++ b/drivers/staging/emxx_udc/emxx_udc.c > @@ -38,7 +38,6 @@ > static int vbus_irq; > > static const chardriver_name[] = "emxx_udc"; > -static const chardriver_desc[] = DRIVER_DESC; > > > /*===*/ > /* Prototype */ > -- > 1.8.3.1 > > Does not apply to my tree at all :(
Re: [PATCH 1/3] staging: rtl8723bs: remove unused variable ret in hal/sdio_halinit.c
On Sun, Apr 11, 2021 at 02:57:35PM +0200, Fabio Aiuto wrote:
> fix following compiler warning issue:
>
>drivers/staging/rtl8723bs/hal/sdio_halinit.c:
> In function 'CardDisableRTL8723BSdio':
> >> drivers/staging/rtl8723bs/hal/sdio_halinit.c:881:5:
> warning: variable 'ret' set but not used [-Wunused-but-set-variable]
> 881 | u8 ret = _FAIL;
> | ^~~
>
> Reported-by: kernel test robot
> Signed-off-by: Fabio Aiuto
> ---
> drivers/staging/rtl8723bs/hal/sdio_halinit.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> index 2098384efe92..936181a73d73 100644
> --- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> +++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> @@ -878,10 +878,9 @@ static void CardDisableRTL8723BSdio(struct adapter
> *padapter)
> {
> u8 u1bTmp;
> u8 bMacPwrCtrlOn;
> - u8 ret = _FAIL;
>
> /* Run LPS WL RFOFF flow */
> - ret = HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK,
> PWR_INTF_SDIO_MSK, rtl8723B_enter_lps_flow);
> + HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK,
> PWR_INTF_SDIO_MSK, rtl8723B_enter_lps_flow);
>
> /* Reset digital sequence == */
>
> @@ -909,9 +908,8 @@ static void CardDisableRTL8723BSdio(struct adapter
> *padapter)
> /* Reset digital sequence end == */
>
> bMacPwrCtrlOn = false; /* Disable CMD53 R/W */
> - ret = false;
> rtw_hal_set_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
> - ret = HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK,
> PWR_INTF_SDIO_MSK, rtl8723B_card_disable_flow);
> + HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK,
> PWR_INTF_SDIO_MSK, rtl8723B_card_disable_flow);
> }
Why isn't the return value being checked and returned to the caller if
something goes wrong? Ignoring it feels wrong to me.
thanks,
greg k-h
Re: [Outreachy kernel] [PATCH v5 0/4] staging: rtl8723bs: Change several files of the driver
On Sun, Apr 11, 2021 at 01:04:54PM +0200, Fabio M. De Francesco wrote: > Remove camelcase, correct misspelled words in comments, change > the type of a variable and its use, change comparisons with 'true' > in controlling expressions. > > Changes from v4: Write patch version number in 2/4. > Changes from v3: Move changes of controlling expressions in patch 4/4. > Changes from v2: Rewrite subject in patch 0/4; remove a patch from the > series because it had alreay been applied (rtl8723bs: core: Remove an unused > variable). > Changes from v1: Fix a typo in subject of patch 1/5, add patch 5/5. I'll take this, but the subject here is a bit odd, and obvious :) thanks, greg k-h
Re: [PATCH v3 0/3] Modify subject description and rectify spelling mistake.
On Sun, Apr 11, 2021 at 03:25:05PM +0530, Mitali Borkar wrote: > This patch fixes style issues "this" is not a patch :( And the subject line here also needs fixed up. thanks, greg k-h
Re: [PATCH 2/3] staging: rtl8723bs: Use existing arc4 implementation
On Sat, Apr 10, 2021 at 03:35:52PM +0200, Christophe JAILLET wrote:
> Use functions provided by instead of hand writing them.
>
> The implementations are slightly different, but are equivalent. It has
> been checked with a test program which compares the output of the 2 sets of
> functions.
>
> Signed-off-by: Christophe JAILLET
> ---
> drivers/staging/rtl8723bs/core/rtw_security.c | 101 --
> 1 file changed, 21 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c
> b/drivers/staging/rtl8723bs/core/rtw_security.c
> index 9587d89a6b24..86949a65e96f 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -6,6 +6,7 @@
>
> **/
> #define _RTW_SECURITY_C_
>
> +#include
> #include
> #include
> #include
> @@ -31,66 +32,6 @@ const char *security_type_str(u8 value)
>
> /* WEP related = */
>
> -struct arc4context {
> - u32 x;
> - u32 y;
> - u8 state[256];
> -};
> -
> -
> -static void arcfour_init(struct arc4context *parc4ctx, u8 *key, u32 key_len)
> -{
> - u32 t, u;
> - u32 keyindex;
> - u32 stateindex;
> - u8 *state;
> - u32 counter;
> -
> - state = parc4ctx->state;
> - parc4ctx->x = 0;
> - parc4ctx->y = 0;
> - for (counter = 0; counter < 256; counter++)
> - state[counter] = (u8)counter;
> - keyindex = 0;
> - stateindex = 0;
> - for (counter = 0; counter < 256; counter++) {
> - t = state[counter];
> - stateindex = (stateindex + key[keyindex] + t) & 0xff;
> - u = state[stateindex];
> - state[stateindex] = (u8)t;
> - state[counter] = (u8)u;
> - if (++keyindex >= key_len)
> - keyindex = 0;
> - }
> -}
> -
> -static u32 arcfour_byte(struct arc4context *parc4ctx)
> -{
> - u32 x;
> - u32 y;
> - u32 sx, sy;
> - u8 *state;
> -
> - state = parc4ctx->state;
> - x = (parc4ctx->x + 1) & 0xff;
> - sx = state[x];
> - y = (sx + parc4ctx->y) & 0xff;
> - sy = state[y];
> - parc4ctx->x = x;
> - parc4ctx->y = y;
> - state[y] = (u8)sx;
> - state[x] = (u8)sy;
> - return state[(sx + sy) & 0xff];
> -}
> -
> -static void arcfour_encrypt(struct arc4context *parc4ctx, u8 *dest, u8 *src,
> u32 len)
> -{
> - u32 i;
> -
> - for (i = 0; i < len; i++)
> - dest[i] = src[i] ^ (unsigned char)arcfour_byte(parc4ctx);
> -}
> -
> static signed int bcrc32initialized;
> static u32 crc32_table[256];
>
> @@ -150,7 +91,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8
> *pxmitframe)
> {
> /* exclude ICV */
>
> unsigned char crc[4];
> - struct arc4context mycontext;
> + struct arc4_ctx mycontext;
Are you sure you can declare that much space on the stack? Is that what
other users of this api do? In looking at the in-kernel users, they do
not :(
Can you fix up this series to not take up so much stack memory?
thanks,
greg k-h
Re: [PATCH 0/2] remove whitespace and rectify spelling error
On Mon, Apr 12, 2021 at 01:15:52AM +0530, Mitali Borkar wrote: > This patch fixes whitespace and spelling mistake issue. "this" is not a patch, it is a 0/X email. Also your subject line does not match the prefix on the patches, please fix up. thanks, greg k-h
Re: [Outreachy kernel] [PATCH] staging: rtl8192u: Remove variable set but not used
On Sun, Apr 11, 2021 at 08:12:05PM +0200, Fabio M. De Francesco wrote: > On Sunday, April 11, 2021 7:43:57 PM CEST Julia Lawall wrote: > > On Sun, 11 Apr 2021, Fabio M. De Francesco wrote: > > > Remove variable "int ret" which is instantiated but not used. > > > > instantiated -> declared? I thought instantiated could mean initialized, > > but that doesn't seem to be the case. > > > > julia > Please, help me to remind... > > If a local variable is declared but not set to any value, does it take > space on the stack? Maybe, maybe not, doesn't matter either way. > If I understand your message, it does not. Therefore it is only declared > but no memory is allocated for it (i.e., it is not instantiated). Right? > > If you confirm I've understood this topic, I can send a v2 patch. That's not the issue here at all... confused, greg k-h
Re: [Outreachy kernel] [PATCH] staging: rtl8192u: Remove variable set but not used
On Sun, Apr 11, 2021 at 07:41:43PM +0200, Fabio M. De Francesco wrote:
> Remove variable "int ret" which is instantiated but not used.
>
> Signed-off-by: Fabio M. De Francesco
> ---
> drivers/staging/rtl8192u/r8192U_core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index f48186a89fa1..30055de66239 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -902,7 +902,6 @@ static void rtl8192_hard_data_xmit(struct sk_buff *skb,
> struct net_device *dev,
> int rate)
> {
> struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> - int ret;
> unsigned long flags;
> struct cb_desc *tcb_desc = (struct cb_desc *)(skb->cb +
> MAX_DEV_ADDR_SIZE);
> u8 queue_index = tcb_desc->queue_index;
> --
> 2.31.1
>
>
Did you test-build this patch?
Re: [PATCH] serial: stm32: optimize spin lock usage
On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
> Hi Greg，
>
> Thanks for the quick response, please ignore the last private mail.
>
> On Mon, Apr 12, 2021 at 1:52 PM Greg KH wrote:
> >
> > On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.min...@gmail.com wrote:
> > > From: dillon min
> > >
> > > To avoid potential deadlock in spin_lock usage, change to use
> > > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn)
> > > context.
> > > spin_lock(), spin_unlock() under handler context.
> > >
> > > remove unused local_irq_save/restore call.
> > >
> > > Signed-off-by: dillon min
> > > ---
> > > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> > >
> > > drivers/tty/serial/stm32-usart.c | 27 +--
> > > 1 file changed, 17 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/stm32-usart.c
> > > b/drivers/tty/serial/stm32-usart.c
> > > index b3675cf25a69..c4c859b34367 100644
> > > --- a/drivers/tty/serial/stm32-usart.c
> > > +++ b/drivers/tty/serial/stm32-usart.c
> > > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct
> > > uart_port *port, bool threaded)
> > > struct tty_port *tport = >state->port;
> > > struct stm32_port *stm32_port = to_stm32_port(port);
> > > const struct stm32_usart_offsets *ofs = _port->info->ofs;
> > > - unsigned long c;
> > > + unsigned long c, flags;
> > > u32 sr;
> > > char flag;
> > >
> > > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct
> > > uart_port *port, bool threaded)
> > > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > > }
> > >
> > > - spin_unlock(>lock);
> > > + if (threaded)
> > > + spin_unlock_irqrestore(>lock, flags);
> > > + else
> > > + spin_unlock(>lock);
> >
> > You shouldn't have to check for this, see the other patches on the list
> > recently that fixed this up to not be an issue for irq handlers.
> Can you help to give more hints, or the commit id of the patch which
> fixed this. thanks.
>
> I'm still confused with this.
>
> The stm32_usart_threaded_interrupt() is a kthread context, once
> port->lock holds by this function, another serial interrupts raised,
> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
> there will be a deadlock. isn't it?
>
> So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
handlers") for when threaded irq handlers have irqs disabled, isn't that
the case you are trying to "protect" from here?
Why is the "threaded" flag used at all? The driver should not care.
Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
threading") in linux-next for an example of how this was fixed up in a
serial driver.
does that help?
thanks,
greg k-h
Re: [PATCH] staging: rtl8188eu: replaced msleep() by usleep_range()
On Mon, Apr 12, 2021 at 11:40:53AM +0530, Mitali Borkar wrote:
> Fixed the warning:-msleep < 20ms can sleep for up to 20ms by replacing
> msleep(unsigned long msecs) by usleep_range(unsigned long min, unsigned long
> max)
> in usecs as msleep(1ms~20ms) can sleep for upto 20 ms.
>
> Signed-off-by: Mitali Borkar
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> index 50d3c3631be0..6afbb5bf8118 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> @@ -5396,7 +5396,7 @@ u8 tx_beacon_hdl(struct adapter *padapter, unsigned
> char *pbuf)
> return H2C_SUCCESS;
>
> if ((pstapriv->tim_bitmap & BIT(0)) && (psta_bmc->sleepq_len >
> 0)) {
> - msleep(10);/* 10ms, ATIM(HIQ) Windows */
> + usleep_range(1 , 2);/* 10ms, ATIM(HIQ) Windows
> */
How do you know you can sleep for 2 here?
And given that you just changed how the sleep works, are you sure the
functionality is the same now?
Only change this type of warning if you have the hardware and can test
the change properly.
thanks,
greg k-h
Re: [PATCH] serial: stm32: optimize spin lock usage
On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.min...@gmail.com wrote: > From: dillon min > > To avoid potential deadlock in spin_lock usage, change to use > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context. > spin_lock(), spin_unlock() under handler context. > > remove unused local_irq_save/restore call. > > Signed-off-by: dillon min > --- > Was verified on stm32f469-disco board. need more test on stm32mp platform. > > drivers/tty/serial/stm32-usart.c | 27 +-- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/tty/serial/stm32-usart.c > b/drivers/tty/serial/stm32-usart.c > index b3675cf25a69..c4c859b34367 100644 > --- a/drivers/tty/serial/stm32-usart.c > +++ b/drivers/tty/serial/stm32-usart.c > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port > *port, bool threaded) > struct tty_port *tport = >state->port; > struct stm32_port *stm32_port = to_stm32_port(port); > const struct stm32_usart_offsets *ofs = _port->info->ofs; > - unsigned long c; > + unsigned long c, flags; > u32 sr; > char flag; > > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port > *port, bool threaded) > uart_insert_char(port, sr, USART_SR_ORE, c, flag); > } > > - spin_unlock(>lock); > + if (threaded) > + spin_unlock_irqrestore(>lock, flags); > + else > + spin_unlock(>lock); You shouldn't have to check for this, see the other patches on the list recently that fixed this up to not be an issue for irq handlers. thanks, greg k-h
Re: [Outreachy kernel] [PATCH v3 3/4] staging: rtl8723bs: Change the type and use of a variable
On Sun, Apr 11, 2021 at 10:29:07AM +0200, Fabio M. De Francesco wrote: > Change the type of fw_current_in_ps_mode from u8 to bool, because > it is used everywhere as a bool and, accordingly, it should be > declared as a bool. Shorten the controlling expression of an > 'if' statement. The "shorten" portion should be in patch 4/4 as it does the same thing there, right? Don't mix the two here, the changing of the type is fine to do alone as a single patch and the if () change has nothing to do with that. Also, you have trailing whitespace in your changelog text :( thanks, greg k-h
Re: [Outreachy kernel] [PATCH v2 0/5] staging: rtl8723bs: Change
On Sun, Apr 11, 2021 at 09:11:43AM +0200, Fabio M. De Francesco wrote: > On Sunday, April 11, 2021 8:39:18 AM CEST Greg KH wrote: > > On Sat, Apr 10, 2021 at 05:00:03PM +0200, Fabio M. De Francesco wrote: > > > Remove camelcase, correct misspelled words in comments, remove an > > > unused > > > variable, change the type of a variable and its use, change comparisons > > > with 'true' in controlling expressions. > > > > > > Changes from v1: Fix a typo in subject of patch 1/5, add patch 5/5. > > > > The subject line above is very odd :( > > > True. I've just read the output of git format in /tmp and noticed that the > line with the "subject" was broken in two different one. I think I had > pressed return while editing. > > I'm about to send that series again with v3. > > In the while I noticed you sent a note to let me know that the you have > added the patch titled "staging: rtl8723bs: core: Remove an unused > variable" to your staging git tree. > > I think this could be a potential issue because the same patch is in the > series that I have to send anew. I put that patch in the series because > yesterday you wrote that the message with subject "Outreachy patches caught > up on.", asking to rebase and resend. > > However, I'm about to send v3 of this patchset. I have no idea whether or > not you will have problems in applying that. If you have problems with it, > please let me know. I will, please rebase your tree against mine, which should remove the one patch that I did apply already as you do not need to send it again. thanks, greg k-h
Re: MHI changes for v5.13
On Sun, Apr 11, 2021 at 11:25:59AM +0530, Manivannan Sadhasivam wrote: > Hi Greg, > > Here is the MHI Pull request for the v5.13 cycle. I stayed with the PR as the > number patches got increased. > > Details are in the signed tag, please consider merging! A suggestion, please put [GIT PULL] in your subject line, so that I know what this is for, that's a semi-standard thing these days. Also, you have a number of patches in this set that look like they should be backported to stable kernels. Any reason why you did not tag them with a "Cc: stable@..." tag in the commit? After they are merged into Linus's tree, please send the git ids of the commits that should go to stable kernels to sta...@vger.kernel.org so that we can add them properly. Now pulled and pushed out, thanks. thanks, greg k-h
Re: [git pull] habanalabs pull request for kernel 5.13
On Sat, Apr 10, 2021 at 11:01:42PM +0300, Oded Gabbay wrote: > Hi Greg, > > This is habanalabs pull request for the merge window of kernel 5.13. > It contains changes and new features, support for new firmware. > Details are in the tag. > > Thanks, > Oded > > The following changes since commit b195b20b7145bcae22ad261abc52d68336f5e913: > > Merge tag 'extcon-next-for-5.13' of > git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon into > char-misc-next (2021-04-08 08:45:30 +0200) Pulled and pushed out, thanks. greg k-h
Re: [PATCH] Staging: rtl8192u: ieee80211: remove odd backslash.
On Sat, Apr 10, 2021 at 10:29:12PM +0300, dev.dra...@bk.ru wrote:
> From: Dmitrii Wolf
>
> This backslash should be deleted - looks like leftover and not needed.
> ---
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> index 690b664df8fa..25ea8e1b6b65 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> @@ -2052,7 +2052,7 @@ void ieee80211_softmac_xmit(struct ieee80211_txb *txb,
> struct ieee80211_device *
> #else
> if ((skb_queue_len(>skb_waitQ[queue_index]) != 0) ||
> #endif
> - (!ieee->check_nic_enough_desc(ieee->dev, queue_index)) || \
> + (!ieee->check_nic_enough_desc(ieee->dev, queue_index)) ||
> (ieee->queue_stop)) {
> /* insert the skb packet to the wait queue */
> /* as for the completion function, it does not need
> --
> 2.25.1
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch does not have a Signed-off-by: line. Please read the
kernel file, Documentation/SubmittingPatches and resend it after
adding that line. Note, the line needs to be in the body of the
email, before the patch, not at the bottom of the patch or in the
email signature.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
Re: [Outreachy kernel] [PATCH v2 2/5] staging: rtl8723bs: include: Fix misspelled words in comments
On Sat, Apr 10, 2021 at 05:00:05PM +0200, Fabio M. De Francesco wrote: > Signed-off-by: Fabio M. De Francesco > --- I can not take patches without any changelog text at all, sorry.
Re: [PATCH v2 0/3] staging: rtl8192e: remove unnecessary parentheses
On Sun, Apr 11, 2021 at 10:03:40AM +0530, Mitali Borkar wrote: > This patch fixes style issues > Changes from v1:- > [Patch 1/3]:- Removed unnecessary parentheses around boolean expressions > [Patch 2/3]:- No changes > [Patch 3/3]:- No changes > > Mitali Borkar (3): > staging: rtl8192e: remove unnecessary parentheses > staging: rtl8192e: remove unnecessary ftrace-like logging > staging: rtl8192e: remove unncessary parentheses You have two patches in this series that do different things, yet have the same subject line (with a misspelling in one). Please make them unique. thanks, greg k-h
Re: [Outreachy kernel] [PATCH v2 0/5] staging: rtl8723bs: Change
On Sat, Apr 10, 2021 at 05:00:03PM +0200, Fabio M. De Francesco wrote: > Remove camelcase, correct misspelled words in comments, remove an unused > variable, change the type of a variable and its use, change comparisons > with 'true' in controlling expressions. > > Changes from v1: Fix a typo in subject of patch 1/5, add patch 5/5. The subject line above is very odd :(
Re: [Outreachy kernel] [PATCH v4] staging: rtl8192e: fixed pointer error by adding '*'
On Sat, Apr 10, 2021 at 08:26:40PM +0530, Mitali Borkar wrote: > Fixed pointer error by adding '*' to the function. That really does not describe this at all. I'll go fix this up... greg k-h
Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8723bs: Change the type and use of a variable
On Sat, Apr 10, 2021 at 03:53:38PM +0200, Fabio M. De Francesco wrote:
> On Saturday, April 10, 2021 3:24:43 PM CEST Julia Lawall wrote:
> > On Sat, 10 Apr 2021, Fabio M. De Francesco wrote:
> > > On Saturday, April 10, 2021 2:12:28 PM CEST Julia Lawall wrote:
> > > > On Sat, 10 Apr 2021, Fabio M. De Francesco wrote:
> > > > > On Saturday, April 10, 2021 1:37:30 PM CEST Julia Lawall wrote:
> > > > > > > That variable has global scope and is assigned at least in:
> > > > > > What do you mean by global scope? None of the following look
> > > > > > like
> > > > > > references to global variables.
> > > > > >
> > > > > > julia
> > > > >
> > > > > I just mean that fw_current_in_ps_mode is a field of a struct in a
> > > > > .h
> > > > > file included everywhere in this driver and that the functions whom
> > > > > the following assignments belong to have not the "static" type
> > > > > modifier.
> > > >
> > > > OK, but a field in a structure is not a variable, and this is not
> > > > what
> > > > scope means.
> > >
> > > You're right, a field in a structure is not a variable.
> > >
> > > > int x;
> > > >
> > > > outside of anything is a global variable (global scope).
> > > >
> > > > int foo() {
> > > >
> > > > int x;
> > > > ...
> > > >
> > > > }
> > > >
> > > > Here x is a local variable. Its scope is the body of the function.
> > > >
> > > > int foo() {
> > > >
> > > > if (abc) {
> > > >
> > > > int x;
> > > > ...
> > > >
> > > > }
> > > >
> > > > }
> > > >
> > > > Here x is a local variable, but its scope is only in the if branch.
> > >
> > > And you're right again: I needed a little refresh of my knowledge of C.
> > >
> > > I've searched again in the code for the purpose of finding out if that
> > > struct is initialized with global scope but I didn't find anything. I
> > > didn't even find any dynamic allocation within functions that returns
> > > pointers to that struct.
> > >
> > > Therefore, according to Greg's request, I'll delete that stupid 'if'
> > > statement in the patch series v2 that I'm about to submit.
> >
> > I'm really not clear on why the if should be deleted.
> >
> > julia
> >
> I'm supposed to delete it because of the review made by Greg. In a couple
> of previous messages he wrote:
>
> "If this is only checked, how can it ever be true? Who ever sets this
> value?"
>
> and then:
>
> "Just delete the variable from the structure entirely and when it is
> used.".
>
> However, like you, I'm not sure yet.
I don't think any of us are, try fixing up what you think is right and
resend and we can go from there :)
Re: [PATCH v2] staging: rtl8192e: fixed pointer error by adding '*'
On Sat, Apr 10, 2021 at 07:07:08PM +0530, Mitali Borkar wrote: > On Sat, Apr 10, 2021 at 03:14:24PM +0200, Greg KH wrote: > > On Sat, Apr 10, 2021 at 06:30:38PM +0530, Mitali Borkar wrote: > > > Fixed Comparison to NULL can be written as '!...' by replacing it with > > > simpler form i.e. boolean expression. This makes code more readable > > > alternative. > > > Reported by checkpatch. > > > > Checkpatch did not report this specific problem, Julia did :) > > > > And this changelog text does not reflect the commit you made here. > > > Making the changes now. > > > > > > > Signed-off-by: Mitali Borkar > > > > We need a "Reported-by:" line here to reflect that someone reported the > > problem as well. > > > I am resending this as patch v3. I have to add reprted by Julia, right? Yes please.
Re: [Outreachy kernel] [PATCH v2] staging: rtl8192e: fixed pointer error by adding '*'
On Sat, Apr 10, 2021 at 03:23:35PM +0200, Julia Lawall wrote: > > > On Sat, 10 Apr 2021, Mitali Borkar wrote: > > > Fixed Comparison to NULL can be written as '!...' by replacing it with > > simpler form i.e. boolean expression. This makes code more readable > > alternative. > > Reported by checkpatch. > > > > Signed-off-by: Mitali Borkar > > --- > > Changes from v1:- added pointer to the function, which was missed during > > fixing v1. > > > > drivers/staging/rtl8192e/rtl819x_TSProc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8192e/rtl819x_TSProc.c > > b/drivers/staging/rtl8192e/rtl819x_TSProc.c > > index 4457c1acfbf6..78b5b4eaec5f 100644 > > --- a/drivers/staging/rtl8192e/rtl819x_TSProc.c > > +++ b/drivers/staging/rtl8192e/rtl819x_TSProc.c > > @@ -327,7 +327,7 @@ bool GetTs(struct rtllib_device *ieee, struct > > ts_common_info **ppTS, > > } > > > > *ppTS = SearchAdmitTRStream(ieee, Addr, UP, TxRxSelect); > > - if (ppTS) > > + if (*ppTS) > > This looks like a patch against your previous patch, and not against the > original code. That's fine as I already accepted the previous patch. thanks, greg k-h
