[PATCH] [v4] module: don't ignore sysfs_create_link() failures
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
Reviewed-by: Greg Kroah-Hartman
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
Luis, can you merge this through the modules-next tree?
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: "Rafael J. Wysocki"
---
v4: minor style changes, based on feedback from Andy Shevchenko
v3: make error handling stricter, add unwinding,
fix build fail with CONFIG_MODULES=n
v2: rework to actually handle the error. I have not tested the
error handling beyond build testing, so please review carefully.
---
drivers/base/base.h | 9 ++---
drivers/base/bus.c| 9 -
drivers/base/module.c | 42 +++---
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..db4f910e8e36 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -192,11 +192,14 @@ extern struct kset *devices_kset;
void devices_kset_move_last(struct device *dev);
#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
-void module_add_driver(struct module *mod, struct device_driver *drv);
+int module_add_driver(struct module *mod, struct device_driver *drv);
void module_remove_driver(struct device_driver *drv);
#else
-static inline void module_add_driver(struct module *mod,
-struct device_driver *drv) { }
+static inline int module_add_driver(struct module *mod,
+ struct device_driver *drv)
+{
+ return 0;
+}
static inline void module_remove_driver(struct device_driver *drv) { }
#endif
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..ffea0728b8b2 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
if (error)
goto out_del_list;
}
- module_add_driver(drv->owner, drv);
+ error = module_add_driver(drv->owner, drv);
+ if (error) {
+ printk(KERN_ERR "%s: failed to create module links for %s\n",
+ __func__, drv->name);
+ goto out_detach;
+ }
error = driver_create_file(drv, _attr_uevent);
if (error) {
@@ -699,6 +704,8 @@ int bus_add_driver(struct device_driver *drv)
return 0;
+out_detach:
+ driver_detach(drv);
out_del_list:
klist_del(>knode_bus);
out_unregister:
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..a1b55da07127 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct module_kobject
*mk)
mutex_unlock(_dir_mutex);
}
-void module_add_driver(struct module *mod, struct device_driver *drv)
+int module_add_driver(struct module *mod, struct device_driver *drv)
{
char *driver_name;
- int no_warn;
struct module_kobject *mk = NULL;
+ int ret;
if (!drv)
- return;
+ return 0;
if (mod)
mk = >mkobj;
@@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
}
if (!mk)
- return;
+ return 0;
+
+ ret = sysfs_create_link(>p->kobj, >kobj, "module");
+ if (ret)
+ return ret;
- /* Don't check return codes; these calls are idempotent */
- no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
driver_name = make_driver_name(drv);
- if (driver_name) {
- module_create_drivers_dir(mk);
- no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
- driver_name);
- kfree(driver_name);
+ if (!driver_name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ module_create_drivers_dir(mk);
+ if (!mk->drivers_dir) {
+ ret = -EINVAL;
+ goto out;
}
+
+ ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
+ if (ret)
+
Re: [PATCH] [v3] module: don't ignore sysfs_create_link() failures
On Tue, Mar 26, 2024, at 16:29, Andy Shevchenko wrote:
> On Tue, Mar 26, 2024 at 03:57:18PM +0100, Arnd Bergmann wrote:
>> From: Arnd Bergmann
>>
>> The sysfs_create_link() return code is marked as __must_check, but the
>> module_add_driver() function tries hard to not care, by assigning the
>> return code to a variable. When building with 'make W=1', gcc still
>> warns because this variable is only assigned but not used:
>>
>> drivers/base/module.c: In function 'module_add_driver':
>> drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
>> [-Wunused-but-set-variable]
>>
>> Rework the code to properly unwind and return the error code to the
>> caller. My reading of the original code was that it tries to
>> not fail when the links already exist, so keep ignoring -EEXIST
>> errors.
>
>> Cc: Luis Chamberlain
>> Cc: linux-modu...@vger.kernel.org
>> Cc: Greg Kroah-Hartman
>> Cc: "Rafael J. Wysocki"
>
> Wondering if you can move these to be after --- to avoid polluting commit
> message. This will have the same effect and be archived on lore. But on
> pros side it will unload the commit message(s) from unneeded noise.
Done
>
>> +error = module_add_driver(drv->owner, drv);
>> +if (error) {
>> +printk(KERN_ERR "%s: failed to create module links for %s\n",
>> +__func__, drv->name);
>
> What's wrong with pr_err()? Even if it's not a style used, in a new pieces of
> code this can be improved beforehand. So, we will reduce a technical debt, and
> not adding to it.
I think that would be more confusing, and would rather keep the
style consistent. There is no practical difference here.
>> +int module_add_driver(struct module *mod, struct device_driver *drv)
>> {
>> char *driver_name;
>> -int no_warn;
>> +int ret;
>
> I would move it...
>
>> struct module_kobject *mk = NULL;
>
> ...to be here.
Done
Arnd
[PATCH] [v5] kallsyms: rework symbol lookup return codes
From: Arnd Bergmann
Building with W=1 in some configurations produces a false positive
warning for kallsyms:
kernel/kallsyms.c: In function '__sprint_symbol.isra':
kernel/kallsyms.c:503:17: error: 'strcpy' source argument is the same as
destination [-Werror=restrict]
503 | strcpy(buffer, name);
| ^~~~
This originally showed up while building with -O3, but later started
happening in other configurations as well, depending on inlining
decisions. The underlying issue is that the local 'name' variable is
always initialized to the be the same as 'buffer' in the called functions
that fill the buffer, which gcc notices while inlining, though it could
see that the address check always skips the copy.
The calling conventions here are rather unusual, as all of the internal
lookup functions (bpf_address_lookup, ftrace_mod_address_lookup,
ftrace_func_address_lookup, module_address_lookup and
kallsyms_lookup_buildid) already use the provided buffer and either return
the address of that buffer to indicate success, or NULL for failure,
but the callers are written to also expect an arbitrary other buffer
to be returned.
Rework the calling conventions to return the length of the filled buffer
instead of its address, which is simpler and easier to follow as well
as avoiding the warning. Leave only the kallsyms_lookup() calling conventions
unchanged, since that is called from 16 different functions and
adapting this would be a much bigger change.
Link: https://lore.kernel.org/all/20200107214042.855757-1-a...@arndb.de/
Link: https://lore.kernel.org/lkml/20240326130647.7bfb1...@gandalf.local.home/
Reviewed-by: Luis Chamberlain
Acked-by: Steven Rostedt (Google)
Signed-off-by: Arnd Bergmann
---
v5: fix ftrace_mod_address_lookup return value,
rebased on top of 2e114248e086 ("bpf: Replace deprecated strncpy with
strscpy")
v4: fix string length
v3: use strscpy() instead of strlcpy()
v2: complete rewrite after the first patch was rejected (in 2020). This
is now one of only two warnings that are in the way of enabling
-Wextra/-Wrestrict by default.
Signed-off-by: Arnd Bergmann
---
include/linux/filter.h | 14 +++---
include/linux/ftrace.h | 6 +++---
include/linux/module.h | 14 +++---
kernel/bpf/core.c| 7 +++
kernel/kallsyms.c| 23 ---
kernel/module/kallsyms.c | 26 +-
kernel/trace/ftrace.c| 13 +
7 files changed, 50 insertions(+), 53 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 161d5f7b64ed..e3a8f51fdf84 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1202,18 +1202,18 @@ static inline bool bpf_jit_kallsyms_enabled(void)
return false;
}
-const char *__bpf_address_lookup(unsigned long addr, unsigned long *size,
+int __bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char *sym);
bool is_bpf_text_address(unsigned long addr);
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym);
struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
-static inline const char *
+static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym)
{
- const char *ret = __bpf_address_lookup(addr, size, off, sym);
+ int ret = __bpf_address_lookup(addr, size, off, sym);
if (ret && modname)
*modname = NULL;
@@ -1257,11 +1257,11 @@ static inline bool bpf_jit_kallsyms_enabled(void)
return false;
}
-static inline const char *
+static inline int
__bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char *sym)
{
- return NULL;
+ return 0;
}
static inline bool is_bpf_text_address(unsigned long addr)
@@ -1280,11 +1280,11 @@ static inline struct bpf_prog
*bpf_prog_ksym_find(unsigned long addr)
return NULL;
}
-static inline const char *
+static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym)
{
- return NULL;
+ return 0;
}
static inline void bpf_prog_kallsyms_add(struct bpf_prog *fp)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 54d53f345d14..56834a3fa9be 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,15 +87,15 @@ struct ftrace_direct_func;
#if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_MODULES) && \
defined(CONFIG_DYNAMIC_FTRACE)
-const char *
+int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym);
#else
-static inline const char *
+static inline int
ftrace_mod_address_lookup(unsig
[PATCH 06/34] tracing: hide unused ftrace_event_id_fops
From: Arnd Bergmann
When CONFIG_PERF_EVENTS, a 'make W=1' build produces a warning about the
unused ftrace_event_id_fops variable:
kernel/trace/trace_events.c:2155:37: error: 'ftrace_event_id_fops' defined but
not used [-Werror=unused-const-variable=]
2155 | static const struct file_operations ftrace_event_id_fops = {
Hide this in the same #ifdef as the reference to it.
Fixes: 620a30e97feb ("tracing: Don't pass file_operations array to
event_create_dir()")
Signed-off-by: Arnd Bergmann
---
kernel/trace/trace_events.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 7c364b87352e..52f75c36bbca 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1670,6 +1670,7 @@ static int trace_format_open(struct inode *inode, struct
file *file)
return 0;
}
+#ifdef CONFIG_PERF_EVENTS
static ssize_t
event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
{
@@ -1684,6 +1685,7 @@ event_id_read(struct file *filp, char __user *ubuf,
size_t cnt, loff_t *ppos)
return simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
}
+#endif
static ssize_t
event_filter_read(struct file *filp, char __user *ubuf, size_t cnt,
@@ -2152,10 +2154,12 @@ static const struct file_operations
ftrace_event_format_fops = {
.release = seq_release,
};
+#ifdef CONFIG_PERF_EVENTS
static const struct file_operations ftrace_event_id_fops = {
.read = event_id_read,
.llseek = default_llseek,
};
+#endif
static const struct file_operations ftrace_event_filter_fops = {
.open = tracing_open_file_tr,
--
2.39.2
[PATCH 00/11] address remaining stringop-truncation warnings
From: Arnd Bergmann We are close to being able to turn on -Wstringop-truncation unconditionally instead of only at the 'make W=1' level, these ten warnings are all that I saw in randconfig testing across compiler versions on arm, arm64 and x86. The final patch is only there for reference at the moment, I hope we can merge the other ones through the subsystem trees first, as there are no dependencies between them. Arnd Arnd Bergmann (11): staging: vc04_services: changen strncpy() to strscpy_pad() scsi: devinfo: rework scsi_strcpy_devinfo() staging: replace weird strncpy() with memcpy() orangefs: convert strncpy() to strscpy() test_hexdump: avoid string truncation warning acpi: avoid warning for truncated string copy block/partitions/ldm: convert strncpy() to strscpy() blktrace: convert strncpy() to strscpy_pad() staging: rtl8723bs: convert strncpy to strscpy staging: greybus: change strncpy() to strscpy() kbuild: enable -Wstringop-truncation globally block/partitions/ldm.c| 6 ++-- drivers/acpi/acpica/tbfind.c | 19 +-- drivers/scsi/scsi_devinfo.c | 30 +++-- drivers/staging/greybus/fw-management.c | 4 +-- .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 5 ++- drivers/staging/rts5208/rtsx_scsi.c | 2 +- .../vc04_services/vchiq-mmal/mmal-vchiq.c | 4 +-- fs/orangefs/dcache.c | 4 +-- fs/orangefs/namei.c | 33 +-- fs/orangefs/super.c | 16 - kernel/trace/blktrace.c | 3 +- lib/test_hexdump.c| 2 +- scripts/Makefile.extrawarn| 1 - 13 files changed, 64 insertions(+), 65 deletions(-) -- 2.39.2 Cc: "Richard Russon Cc: Jens Axboe Cc: Robert Moore Cc: "Rafael J. Wysocki" Cc: Len Brown Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: Viresh Kumar Cc: Johan Hovold Cc: Alex Elder Cc: Greg Kroah-Hartman Cc: Florian Fainelli Cc: Broadcom internal kernel review list Cc: Mike Marshall Cc: Martin Brandenburg Cc: Steven Rostedt Cc: Masami Hiramatsu Cc: Mathieu Desnoyers Cc: Andrew Morton Cc: Masahiro Yamada Cc: Nathan Chancellor Cc: Nicolas Schier Cc: Arnd Bergmann Cc: Kees Cook Cc: Alexey Starikovskiy Cc: linux-ntfs-...@lists.sourceforge.net Cc: linux-bl...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-a...@vger.kernel.org Cc: acpica-de...@lists.linux.dev Cc: linux-s...@vger.kernel.org Cc: greybus-...@lists.linaro.org Cc: linux-stag...@lists.linux.dev Cc: linux-rpi-ker...@lists.infradead.org Cc: linux-arm-ker...@lists.infradead.org Cc: de...@lists.orangefs.org Cc: linux-trace-kernel@vger.kernel.org Cc: linux-kbu...@vger.kernel.org
Re: [PATCH 11/12] [v4] kallsyms: rework symbol lookup return codes
On Tue, Mar 26, 2024, at 18:06, Steven Rostedt wrote:
> On Tue, 26 Mar 2024 15:53:38 +0100
> Arnd Bergmann wrote:
>
>> -const char *
>> +int
>> ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
>> unsigned long *off, char **modname, char *sym)
>> {
>> struct ftrace_mod_map *mod_map;
>> -const char *ret = NULL;
>> +int ret;
>
> This needs to be ret = 0;
Fixed now, thanks!
I'll send a v5 in a few days
Arnd
[PATCH] [v3] module: don't ignore sysfs_create_link() failures
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: "Rafael J. Wysocki"
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
v3: make error handling stricter, add unwinding,
fix build fail with CONFIG_MODULES=n
v2: rework to actually handle the error. I have not tested the
error handling beyond build testing, so please review carefully.
---
drivers/base/base.h | 9 ++---
drivers/base/bus.c| 9 -
drivers/base/module.c | 42 +++---
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..db4f910e8e36 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -192,11 +192,14 @@ extern struct kset *devices_kset;
void devices_kset_move_last(struct device *dev);
#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
-void module_add_driver(struct module *mod, struct device_driver *drv);
+int module_add_driver(struct module *mod, struct device_driver *drv);
void module_remove_driver(struct device_driver *drv);
#else
-static inline void module_add_driver(struct module *mod,
-struct device_driver *drv) { }
+static inline int module_add_driver(struct module *mod,
+ struct device_driver *drv)
+{
+ return 0;
+}
static inline void module_remove_driver(struct device_driver *drv) { }
#endif
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..ffea0728b8b2 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
if (error)
goto out_del_list;
}
- module_add_driver(drv->owner, drv);
+ error = module_add_driver(drv->owner, drv);
+ if (error) {
+ printk(KERN_ERR "%s: failed to create module links for %s\n",
+ __func__, drv->name);
+ goto out_detach;
+ }
error = driver_create_file(drv, _attr_uevent);
if (error) {
@@ -699,6 +704,8 @@ int bus_add_driver(struct device_driver *drv)
return 0;
+out_detach:
+ driver_detach(drv);
out_del_list:
klist_del(>knode_bus);
out_unregister:
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..d16b5c8e5473 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct module_kobject
*mk)
mutex_unlock(_dir_mutex);
}
-void module_add_driver(struct module *mod, struct device_driver *drv)
+int module_add_driver(struct module *mod, struct device_driver *drv)
{
char *driver_name;
- int no_warn;
+ int ret;
struct module_kobject *mk = NULL;
if (!drv)
- return;
+ return 0;
if (mod)
mk = >mkobj;
@@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
}
if (!mk)
- return;
+ return 0;
+
+ ret = sysfs_create_link(>p->kobj, >kobj, "module");
+ if (ret)
+ return ret;
- /* Don't check return codes; these calls are idempotent */
- no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
driver_name = make_driver_name(drv);
- if (driver_name) {
- module_create_drivers_dir(mk);
- no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
- driver_name);
- kfree(driver_name);
+ if (!driver_name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ module_create_drivers_dir(mk);
+ if (!mk->drivers_dir) {
+ ret = -EINVAL;
+ goto out;
}
+
+ ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
+ if (ret)
+ goto out;
+
+ kfree(driver_name);
+
+ return 0;
+out:
+ sysfs_remove_link(>p->kobj,
[PATCH 11/12] [v4] kallsyms: rework symbol lookup return codes
From: Arnd Bergmann
Building with W=1 in some configurations produces a false positive
warning for kallsyms:
kernel/kallsyms.c: In function '__sprint_symbol.isra':
kernel/kallsyms.c:503:17: error: 'strcpy' source argument is the same as
destination [-Werror=restrict]
503 | strcpy(buffer, name);
| ^~~~
This originally showed up while building with -O3, but later started
happening in other configurations as well, depending on inlining
decisions. The underlying issue is that the local 'name' variable is
always initialized to the be the same as 'buffer' in the called functions
that fill the buffer, which gcc notices while inlining, though it could
see that the address check always skips the copy.
The calling conventions here are rather unusual, as all of the internal
lookup functions (bpf_address_lookup, ftrace_mod_address_lookup,
ftrace_func_address_lookup, module_address_lookup and
kallsyms_lookup_buildid) already use the provided buffer and either return
the address of that buffer to indicate success, or NULL for failure,
but the callers are written to also expect an arbitrary other buffer
to be returned.
Rework the calling conventions to return the length of the filled buffer
instead of its address, which is simpler and easier to follow as well
as avoiding the warning. Leave only the kallsyms_lookup() calling conventions
unchanged, since that is called from 16 different functions and
adapting this would be a much bigger change.
Link: https://lore.kernel.org/all/20200107214042.855757-1-a...@arndb.de/
Reviewed-by: Luis Chamberlain
Acked-by: Steven Rostedt (Google)
Signed-off-by: Arnd Bergmann
---
v4: fix string length
v3: use strscpy() instead of strlcpy()
v2: complete rewrite after the first patch was rejected (in 2020). This
is now one of only two warnings that are in the way of enabling
-Wextra/-Wrestrict by default.
---
include/linux/filter.h | 14 +++---
include/linux/ftrace.h | 6 +++---
include/linux/module.h | 14 +++---
kernel/bpf/core.c| 7 +++
kernel/kallsyms.c| 23 ---
kernel/module/kallsyms.c | 26 +-
kernel/trace/ftrace.c| 13 +
7 files changed, 50 insertions(+), 53 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index c99bc3df2d28..9d4a7c6f023e 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1168,18 +1168,18 @@ static inline bool bpf_jit_kallsyms_enabled(void)
return false;
}
-const char *__bpf_address_lookup(unsigned long addr, unsigned long *size,
+int __bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char *sym);
bool is_bpf_text_address(unsigned long addr);
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym);
struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
-static inline const char *
+static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym)
{
- const char *ret = __bpf_address_lookup(addr, size, off, sym);
+ int ret = __bpf_address_lookup(addr, size, off, sym);
if (ret && modname)
*modname = NULL;
@@ -1223,11 +1223,11 @@ static inline bool bpf_jit_kallsyms_enabled(void)
return false;
}
-static inline const char *
+static inline int
__bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char *sym)
{
- return NULL;
+ return 0;
}
static inline bool is_bpf_text_address(unsigned long addr)
@@ -1246,11 +1246,11 @@ static inline struct bpf_prog
*bpf_prog_ksym_find(unsigned long addr)
return NULL;
}
-static inline const char *
+static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym)
{
- return NULL;
+ return 0;
}
static inline void bpf_prog_kallsyms_add(struct bpf_prog *fp)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 54d53f345d14..56834a3fa9be 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,15 +87,15 @@ struct ftrace_direct_func;
#if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_MODULES) && \
defined(CONFIG_DYNAMIC_FTRACE)
-const char *
+int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym);
#else
-static inline const char *
+static inline int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
unsigned long *off, char **modname, char *sym)
{
- return NULL;
+ return 0;
}
#endif
diff --git a/include/linux/module.h b/include/linux/module.h
index 1153b0d99a80..118c36366b35 100644
--- a/include/linu
Re: [PATCH] [v2] module: don't ignore sysfs_create_link() failures
On Sat, Mar 23, 2024, at 17:50, Greg Kroah-Hartman wrote:
> On Fri, Mar 22, 2024 at 06:39:11PM +0100, Arnd Bergmann wrote:
>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>> index daee55c9b2d9..7ef75b60d331 100644
>> --- a/drivers/base/bus.c
>> +++ b/drivers/base/bus.c
>> @@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
>> if (error)
>> goto out_del_list;
>> }
>> -module_add_driver(drv->owner, drv);
>> +error = module_add_driver(drv->owner, drv);
>> +if (error) {
>> +printk(KERN_ERR "%s: failed to create module links for %s\n",
>> +__func__, drv->name);
>> +goto out_del_list;
>
> Don't we need to walk back the driver_attach() call here if this fails?
Yes, fixed now. There are still some other calls right after
it that print an error but don't cause bus_add_driver() to fail
though. We may want to add similar unwinding there, but that
feels like it should be a separate patch.
>>
>> if (!mk)
>> -return;
>> +return 0;
>> +
>> +ret = sysfs_create_link(>p->kobj, >kobj, "module");
>> +if (ret && ret != -EEXIST)
>
> Why would EEXIST happen here? How can this be called twice?
>
My impression was that the lack of error handling and the
comment was ab out a case where that might happen
intentionally. I've removed it now as I couldn't find any
evidence that this is really needed. I suppose we would
find out in testing if we do.
Arnd
[PATCH] [v2] module: don't ignore sysfs_create_link() failures
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: "Rafael J. Wysocki"
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
v2: rework to actually handle the error. I have not tested the
error handling beyond build testing, so please review carefully.
---
drivers/base/base.h | 2 +-
drivers/base/bus.c| 7 ++-
drivers/base/module.c | 42 +++---
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..0e04bfe02943 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -192,7 +192,7 @@ extern struct kset *devices_kset;
void devices_kset_move_last(struct device *dev);
#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
-void module_add_driver(struct module *mod, struct device_driver *drv);
+int module_add_driver(struct module *mod, struct device_driver *drv);
void module_remove_driver(struct device_driver *drv);
#else
static inline void module_add_driver(struct module *mod,
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..7ef75b60d331 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
if (error)
goto out_del_list;
}
- module_add_driver(drv->owner, drv);
+ error = module_add_driver(drv->owner, drv);
+ if (error) {
+ printk(KERN_ERR "%s: failed to create module links for %s\n",
+ __func__, drv->name);
+ goto out_del_list;
+ }
error = driver_create_file(drv, _attr_uevent);
if (error) {
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..61282eaed670 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct module_kobject
*mk)
mutex_unlock(_dir_mutex);
}
-void module_add_driver(struct module *mod, struct device_driver *drv)
+int module_add_driver(struct module *mod, struct device_driver *drv)
{
char *driver_name;
- int no_warn;
+ int ret;
struct module_kobject *mk = NULL;
if (!drv)
- return;
+ return 0;
if (mod)
mk = >mkobj;
@@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
}
if (!mk)
- return;
+ return 0;
+
+ ret = sysfs_create_link(>p->kobj, >kobj, "module");
+ if (ret && ret != -EEXIST)
+ return ret;
- /* Don't check return codes; these calls are idempotent */
- no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
driver_name = make_driver_name(drv);
- if (driver_name) {
- module_create_drivers_dir(mk);
- no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
- driver_name);
- kfree(driver_name);
+ if (!driver_name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ module_create_drivers_dir(mk);
+ if (!mk->drivers_dir) {
+ ret = -EINVAL;
+ goto out;
}
+
+ ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
+ if (ret && ret != -EEXIST)
+ goto out;
+
+ kfree(driver_name);
+
+ return 0;
+out:
+ sysfs_remove_link(>p->kobj, "module");
+ sysfs_remove_link(mk->drivers_dir, driver_name);
+ kfree(driver_name);
+
+ return ret;
}
void module_remove_driver(struct device_driver *drv)
--
2.39.2
[PATCH] module: silence warning about unused 'no_warn' variable
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Add an explicit cast to void to prevent this check as well.
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: "Rafael J. Wysocki"
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
I'm not entirely sure what bug the __must_check on sysfs_create_link()
is trying to prevent, or why the module loader code is allowed to
ignore this. It would be nice to have an Ack from the sysfs maintainers
on this.
---
drivers/base/module.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..0180dbcf2240 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -67,6 +67,8 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
driver_name);
kfree(driver_name);
}
+
+ (void)no_warn;
}
void module_remove_driver(struct device_driver *drv)
--
2.39.2
[PATCH] virtiofs: don't mark virtio_fs_sysfs_exit as __exit
From: Arnd Bergmann
Calling an __exit function from an __init function is not allowed
and will result in undefined behavior when the code is built-in:
WARNING: modpost: vmlinux: section mismatch in reference: virtio_fs_init+0x50
(section: .init.text) -> virtio_fs_sysfs_exit (section: .exit.text)
Remove the incorrect annotation.
Fixes: a8f62f50b4e4 ("virtiofs: export filesystem tags through sysfs")
Signed-off-by: Arnd Bergmann
---
fs/fuse/virtio_fs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 3a7dd48b534f..36d87dd3cb48 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1595,7 +1595,7 @@ static int __init virtio_fs_sysfs_init(void)
return 0;
}
-static void __exit virtio_fs_sysfs_exit(void)
+static void virtio_fs_sysfs_exit(void)
{
kset_unregister(virtio_fs_kset);
virtio_fs_kset = NULL;
--
2.39.2
Re: [PATCH v2 3/3] media: mediatek: vcodedc: Fix Wcast-function-type-strict warnings
On Tue, Feb 27, 2024, at 12:35, Ricardo Ribalda wrote: > On Tue, 27 Feb 2024 at 12:17, Hans Verkuil wrote: >> >> Ricardo, >> >> First of all, note the typo in theo subject line: vcodedc -> vcodec. >> >> There is also a similar (but not identical!) patch from Arnd: >> >> https://patchwork.linuxtv.org/project/linux-media/patch/20240224121059.1806691-1-a...@kernel.org/ >> >> That patch and yours share the change to common/mtk_vcodec_fw_vpu.c but >> otherwise >> they are different, which is a bit odd. >> >> Can you take a look at Arnd's patch and see if you need to incorporate his >> changes >> into your patch? > > We went separate paths :), I tried to make everything const (and > therefore the remoteproc changes) and he removed the const. I had the same patch 3 originally but there was still a warning or a cast from const to non-const pointer, so I went with the simpler approach that I posted. Without that regression, your patch would be nicer, but I think the version you sent has the same regression that I ran into. > His patch looks good to me. Shall I resend the series without this > patch or you can ignore 3/3 and take 1 and 2? I also sent your patches 1 and 2 with minor differences in whitespace. Both of these are already in linux-next, so I think you don't have to resend anything here. I sent a whole lot of -Wcast-function-type-strict warning fixes to address all randconfig builds on arm64, arm and x86, all but three are now merged. Aside from the mediatek vcodec, the only missing ones are https://lore.kernel.org/all/20240213095631.454543-1-a...@kernel.org/ https://lore.kernel.org/all/20240213100238.456912-1-a...@kernel.org/ I also had one patch for s390 but none of the other architectures. I think once Hans applies the vcodec patch, I'll resend the last two patches together with a patch to enable the warning by default. Arnd
[PATCH] dax: add set_dax_nomc() and set_dax_nocache() stub helpers
From: Arnd Bergmann
In some randconfig builds, the IS_ERR() check appears to not get completely
eliminated, resulting in the compiler to insert references to these two
functions that cause a link failure:
ERROR: modpost: "set_dax_nocache" [drivers/md/dm-mod.ko] undefined!
ERROR: modpost: "set_dax_nomc" [drivers/md/dm-mod.ko] undefined!
Add more stub functions for the dax-disabled case here to make it build again.
Fixes: d888f6b0a766 ("dm: treat alloc_dax() -EOPNOTSUPP failure as non-fatal")
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202402160420.e4qkwogo-...@intel.com/
Signed-off-by: Arnd Bergmann
---
include/linux/dax.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/linux/dax.h b/include/linux/dax.h
index df2d52b8a245..4527c10016fb 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -64,6 +64,9 @@ void dax_write_cache(struct dax_device *dax_dev, bool wc);
bool dax_write_cache_enabled(struct dax_device *dax_dev);
bool dax_synchronous(struct dax_device *dax_dev);
void set_dax_synchronous(struct dax_device *dax_dev);
+void set_dax_nocache(struct dax_device *dax_dev);
+void set_dax_nomc(struct dax_device *dax_dev);
+
size_t dax_recovery_write(struct dax_device *dax_dev, pgoff_t pgoff,
void *addr, size_t bytes, struct iov_iter *i);
/*
@@ -108,6 +111,12 @@ static inline bool dax_synchronous(struct dax_device
*dax_dev)
static inline void set_dax_synchronous(struct dax_device *dax_dev)
{
}
+static inline void set_dax_nocache(struct dax_device *dax_dev)
+{
+}
+static inline void set_dax_nomc(struct dax_device *dax_dev)
+{
+}
static inline bool daxdev_mapping_supported(struct vm_area_struct *vma,
struct dax_device *dax_dev)
{
@@ -120,9 +129,6 @@ static inline size_t dax_recovery_write(struct dax_device
*dax_dev,
}
#endif
-void set_dax_nocache(struct dax_device *dax_dev);
-void set_dax_nomc(struct dax_device *dax_dev);
-
struct writeback_control;
#if defined(CONFIG_BLOCK) && defined(CONFIG_FS_DAX)
int dax_add_host(struct dax_device *dax_dev, struct gendisk *disk);
--
2.39.2
Re: [PATCH] mm: Remove broken pfn_to_virt() on arch csky/hexagon/openrisc
On Fri, Feb 2, 2024, at 15:05, Yan Zhao wrote: > Remove the broken pfn_to_virt() on architectures csky/hexagon/openrisc. > > The pfn_to_virt() on those architectures takes PFN instead of PA as the > input to macro __va(), with PAGE_SHIFT applying to the converted VA, which > is not right, especially when there's a non-zero offset in __va(). [1] > > The broken pfn_to_virt() was noticed when checking how page_to_virt() is > unfolded on x86. It turns out that the one in linux/mm.h instead of in > asm-generic/page.h is compiled for x86. However, page_to_virt() in > asm-generic/page.h is found out expanding to pfn_to_virt() with a bug > described above. The pfn_to_virt() is found out not right as well on > architectures csky/hexagon/openrisc. > > Since there's no single caller on csky/hexagon/openrisc to pfn_to_virt() > and there are functions doing similar things as pfn_to_virt() -- > - pfn_to_kaddr() in asm/page.h for csky, > - page_to_virt() in asm/page.h for hexagon, and > - page_to_virt() in linux/mm.h for openrisc, > just delete the pfn_to_virt() on those architectures. > > The pfn_to_virt() in asm-generic/page.h is not touched in this patch as > it's referenced by page_to_virt() in that header while the whole header is > going to be removed as a whole due to no one including it. > > Link:https://lore.kernel.org/all/20240131055159.2506-1-yan.y.z...@intel.com > [1] > Cc: Linus Walleij > Suggested-by: Arnd Bergmann > Signed-off-by: Yan Zhao Nice changelog text! Applied it to the asm-generic tree now, thanks, Arnd
Re: [PATCH 0/4] apply page shift to PFN instead of VA in pfn_to_virt
On Thu, Feb 1, 2024, at 11:46, Geert Uytterhoeven wrote: > Hi Arnd, > > On Thu, Feb 1, 2024 at 11:11 AM Arnd Bergmann wrote: >> I think it's fair to assume we won't need asm-generic/page.h any >> more, as we likely won't be adding new NOMMU architectures. > > So you think riscv-nommu (k210) was the last one we will ever see? Yes. We've already removed half of the nommu architectures (blackfin, avr32, h8300, m32r, microblaze-nommu) over the past couple of years, and the remaining ones are pretty much only there to support existing users. The only platform one that I see getting real work is esp32 [1], but that is not a new architecture. Arnd [1] https://github.com/jcmvbkbc/linux-xtensa/tree/xtensa-6.8-rc2-esp32
Re: [PATCH 0/4] apply page shift to PFN instead of VA in pfn_to_virt
On Fri, Feb 2, 2024, at 02:02, Yan Zhao wrote: > On Thu, Feb 01, 2024 at 06:46:46AM +0100, Arnd Bergmann wrote: >> >> I think it's fair to assume we won't need asm-generic/page.h any >> more, as we likely won't be adding new NOMMU architectures. >> I can have a look myself at removing any such unused headers in >> include/asm-generic/, it's probably not the only one. >> >> Can you just send a patch to remove the unused pfn_to_virt() >> functions? > Ok. I'll do it! > BTW: do you think it's also good to keep this fixing series though we'll > remove the unused function later? > So if people want to revert the removal some day, they can get right one. > > Or maybe I'm just paranoid, and explanation about the fix in the commit > message of patch for function removal is enough. > > What's your preference? :) I would just remove it, there is no point in having both pfn_to_kaddr() and pfn_to_virt() when they do the exact same thing aside from this bug. Just do a single patch for all architectures, no need to have three or four identical ones when I'm going to merge them all through the same tree anyway. Just make sure you explain in the changelog what the bug was and how you noticed it, in case anyone is ever tempted to bring the function back. Arnd
Re: [PATCH 0/4] apply page shift to PFN instead of VA in pfn_to_virt
On Thu, Feb 1, 2024, at 01:01, Yan Zhao wrote: > On Wed, Jan 31, 2024 at 12:48:38PM +0100, Arnd Bergmann wrote: >> On Wed, Jan 31, 2024, at 06:51, Yan Zhao wrote: >> >> How exactly did you notice the function being wrong, >> did you try to add a user somewhere, or just read through >> the code? > I came across them when I was debugging an unexpected kernel page fault > on x86, and I was not sure whether page_to_virt() was compiled in > asm-generic/page.h or linux/mm.h. > Though finally, it turned out that the one in linux/mm.h was used, which > yielded the right result and the unexpected kernel page fault in my case > was not related to page_to_virt(), it did lead me to noticing that the > pfn_to_virt() in asm-generic/page.h and other 3 archs did not look right. > > Yes, unlike virt_to_pfn() which still has a caller in openrisc (among > csky, Hexagon, openrisc), pfn_to_virt() now does not have a caller in > the 3 archs. Though both virt_to_pfn() and pfn_to_virt() are referenced > in asm-generic/page.h, I also not sure if we need to remove the > asm-generic/page.h which may serve as a template to future archs ? > > So, either way looks good to me :) I think it's fair to assume we won't need asm-generic/page.h any more, as we likely won't be adding new NOMMU architectures. I can have a look myself at removing any such unused headers in include/asm-generic/, it's probably not the only one. Can you just send a patch to remove the unused pfn_to_virt() functions? Arnd
Re: [PATCH 0/4] apply page shift to PFN instead of VA in pfn_to_virt
On Wed, Jan 31, 2024, at 06:51, Yan Zhao wrote: > This is a tiny fix to pfn_to_virt() for some platforms. > > The original implementaion of pfn_to_virt() takes PFN instead of PA as the > input to macro __va, with PAGE_SHIFT applying to the converted VA, which > is not right under most conditions, especially when there's an offset in > __va. > > > Yan Zhao (4): > asm-generic/page.h: apply page shift to PFN instead of VA in > pfn_to_virt > csky: apply page shift to PFN instead of VA in pfn_to_virt > Hexagon: apply page shift to PFN instead of VA in pfn_to_virt > openrisc: apply page shift to PFN instead of VA in pfn_to_virt Nice catch, this is clearly a correct fix, and I can take the series through the asm-generic tree if we want to take this approach. I made a couple of interesting observations looking at this patch though: - this function is only used in architecture specific code on m68k, riscv and s390, though a couple of other architectures have the same definition. - There is another function that does the same thing called pfn_to_kaddr(), which is defined on arm, arm64, csky, loongarch, mips, nios2, powerpc, s390, sh, sparc and x86, as well as yet another pfn_va() on parisc. - the asm-generic/page.h file used to be included by h8300, c6x and blackfin, all of which are now gone. It has no users left and can just as well get removed, unless we find a new use for it. Since it looks like the four broken functions you fix don't have a single caller, maybe it would be better to just remove them all? How exactly did you notice the function being wrong, did you try to add a user somewhere, or just read through the code? Arnd
Re: [REGRESSION] v5.13: FS_DAX unavailable on 32-bit ARM
On Fri, Jan 26, 2024, at 20:33, Mathieu Desnoyers wrote: > > A) I have prepared a patch series introducing cache_is_aliasing() with > new Kconfig > options: > >* ARCH_HAS_CACHE_ALIASING >* ARCH_HAS_CACHE_ALIASING_DYNAMIC > > and implemented it for all architectures. The "DYNAMIC" implementation > implements cache_is_aliasing() as a runtime check, which is what is needed > on architectures like 32-bit ARM. > > With this we can basically narrow down the list of architectures which are > unsupported by DAX to those which are really affected, without actually > solving > the issue for architectures with virtually aliased dcaches. The dynamic option should only be required when building for ARMv6, which is really rare. On an ARMv7-only configuration, we know that the dcache is non-aliasing, so the compile-time check should be sufficient. Even on ARMv6, this could be done as a compile-time choice by platform, since we mostly know what the chips can do: bcm2835, imx3, wm8750 and s3c64xx are non-aliasing because they are limited to 16KB L1 caches, while omap2 and as2500 are aliasing with 32KB caches. With realview/integrator it depends on the exact CPU that was installed. Arnd
[PATCH] vfio: fix virtio-pci dependency
From: Arnd Bergmann
The new vfio-virtio driver already has a dependency on VIRTIO_PCI_ADMIN_LEGACY,
but that is a bool symbol and allows vfio-virtio to be built-in even if
virtio-pci itself is a loadable module. This leads to a link failure:
aarch64-linux-ld: drivers/vfio/pci/virtio/main.o: in function
`virtiovf_pci_probe':
main.c:(.text+0xec): undefined reference to `virtio_pci_admin_has_legacy_io'
aarch64-linux-ld: drivers/vfio/pci/virtio/main.o: in function
`virtiovf_pci_init_device':
main.c:(.text+0x260): undefined reference to
`virtio_pci_admin_legacy_io_notify_info'
aarch64-linux-ld: drivers/vfio/pci/virtio/main.o: in function
`virtiovf_pci_bar0_rw':
main.c:(.text+0x6ec): undefined reference to
`virtio_pci_admin_legacy_common_io_read'
aarch64-linux-ld: main.c:(.text+0x6f4): undefined reference to
`virtio_pci_admin_legacy_device_io_read'
aarch64-linux-ld: main.c:(.text+0x7f0): undefined reference to
`virtio_pci_admin_legacy_common_io_write'
aarch64-linux-ld: main.c:(.text+0x7f8): undefined reference to
`virtio_pci_admin_legacy_device_io_write'
Add another explicit dependency on the tristate symbol.
Fixes: eb61eca0e8c3 ("vfio/virtio: Introduce a vfio driver over virtio devices")
Signed-off-by: Arnd Bergmann
---
drivers/vfio/pci/virtio/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vfio/pci/virtio/Kconfig b/drivers/vfio/pci/virtio/Kconfig
index fc3a0be9d8d4..bd80eca4a196 100644
--- a/drivers/vfio/pci/virtio/Kconfig
+++ b/drivers/vfio/pci/virtio/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config VIRTIO_VFIO_PCI
tristate "VFIO support for VIRTIO NET PCI devices"
-depends on VIRTIO_PCI_ADMIN_LEGACY
+depends on VIRTIO_PCI && VIRTIO_PCI_ADMIN_LEGACY
select VFIO_PCI_CORE
help
This provides support for exposing VIRTIO NET VF devices which
support
--
2.39.2
Re: [PATCH 3/3] locking/atomic: openrisc: use generic_cmpxchg[64]_local for arch_cmpxchg[64]_local
On Thu, Oct 26, 2023, at 09:39, wuqiang.matt wrote: > arch_cmpxchg[64]_local() are not defined for openrisc. So implement > them with generci_cmpxchg[64]_local, advised by Masami Hiramatsu. > > Closes: > https://lore.kernel.org/linux-trace-kernel/169824660459.24340.14614817132696360531.stgit@devnote2 > Closes: > https://lore.kernel.org/oe-kbuild-all/202310241310.ir5uukog-...@intel.com > > Signed-off-by: wuqiang.matt I think on architectures that have actual atomics, you generally want to define this to be the same as arch_cmpxchg() rather than the generic version. It depends on the relative cost of doing one atomic compared to an irq-disable/enable pair, but everyone else went with the former if they could. The exceptions are armv4/armv5, sparc32 and parisc, which don't have a generic cmpxchg() or similar operation. You could do the thing that sparc64 and xtensa do, which use the native cmpxchg for supported word sizes but the generic version for 1- and 2-byte swaps, but that has its own set of problems if you end up doing operations on both the entire word and a sub-unit of the same thing. Arnd
Re: [PATCH 09/10] wireless: hostap: remove unused ioctl function
On Tue, Oct 10, 2023, at 09:00, Kalle Valo wrote: > Arnd Bergmann writes: > >> From: Arnd Bergmann >> >> The ioctl handler has no actual callers in the kernel and is useless. >> All the functionality should be reachable through the regualar interfaces. >> >> Signed-off-by: Arnd Bergmann > > In the title we prefer "wifi:" over "wireless:" but that's nitpicking. I > assume this goes via a net tree so: Changed for v2 > Acked-by: Kalle Valo Thanks > Let me know if I should take this to wireless-next instead. I think it's better to keep the series together and merge it through net-next directly, since the last patch depends on all the ones before it. Arnd
Re: [PATCH 01/10] appletalk: remove localtalk and ppp support
On Tue, Oct 10, 2023, at 09:10, Rodolfo Zitellini wrote: >> Il giorno 9 ott 2023, alle ore 19:29, Arnd Bergmann ha >> scritto: >> On Mon, Oct 9, 2023, at 18:49, Rodolfo Zitellini wrote: >> I can see a few ways this could work out: >> >> - add a custom callback pointer to struct atalk_iface to >> get and set the address for phase1 probing instead of going >> through the ioctl > > This was my initial thought, at least for the moment, mostly to keep > netatalk happy and make sure I don’t break other stuff that makes > assumptions on how the address probing worked. There are other bits I > would like to improve, for example tcpdump (which parses correctly > appetalk packets!) is broken in the current implementation. > >> - rewrite the probing logic in aarp.c more widely, and improve >> the userspace interface in the process by introducing a netlink >> interface > > This is sorta the “second step” I was planning, I think the logic for > probing could be redesigned and simplified (it also does not work 100% > correctly), and it could be a good chance to improve the interface with > netatalk too. Ok, I've adapted my patch now to not actually drop the localtalk code for now, and sent that out, I hope that works for you. Even if we go with the v1 patch that removes it all, you could just as well start with a revert of my patch when you add your driver, so in the end it shouldn't make much of a difference. >> - Move your entire driver into userspace and go to the kernel >> using tun/tap. This has the added benefit of avoiding a lot >> of the complexity of the tty line discipline code you have. > > We had some discussion too if to just make the lt an userspace stack, I > personally like how it is currently implemented because existing code > can run basically without modification. > > I would propose at this stage to change the TashTalk driver to remove > ndo_do_ioctl and to use a custom callback, if this ok. It looks like you still need a custom userspace tool to set up the uart for your new driver, so my feeling would be that having a userspace bridge to implement the localtalk/uart to ethertalk/tap driver would actually be nicer for both usability and maintenance. It's not something we need to decide now though, and is up to you in the end. Arnd
[PATCH v2] appletalk: make localtalk and ppp support conditional
From: Arnd Bergmann
The last localtalk driver is gone now, and ppp support was never fully
merged, but the code to support them for phase1 networking still calls
the deprecated .ndo_do_ioctl() helper.
In order to better isolate the localtalk and ppp portions of appletalk,
guard all of the corresponding code with CONFIG_DEV_APPLETALK checks,
including a preprocessor conditional that guards the internal ioctl calls.
This is currently all dead code and will now be left out of the
module since this Kconfig symbol is always undefined, but there are
plans to add a new driver for localtalk again in the future. When
that happens, the logic can be cleaned up to work properly without
the need for the ioctl.
Link:
https://lore.kernel.org/lkml/790ba488-b6f6-41ed-96ef-2089ef1c0...@xhero.org/
Signed-off-by: Arnd Bergmann
---
v2: don't actually remove the code, just make it conditional since we
are likely to need it again.
include/linux/atalk.h | 1 -
net/appletalk/Makefile | 3 ++-
net/appletalk/aarp.c | 24 +++-
net/appletalk/ddp.c| 24 +---
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/include/linux/atalk.h b/include/linux/atalk.h
index a55bfc6567d01..2896f2ac9568e 100644
--- a/include/linux/atalk.h
+++ b/include/linux/atalk.h
@@ -121,7 +121,6 @@ static inline struct atalk_iface *atalk_find_dev(struct
net_device *dev)
#endif
extern struct atalk_addr *atalk_find_dev_addr(struct net_device *dev);
-extern struct net_device *atrtr_get_dev(struct atalk_addr *sa);
extern int aarp_send_ddp(struct net_device *dev,
struct sk_buff *skb,
struct atalk_addr *sa, void *hwaddr);
diff --git a/net/appletalk/Makefile b/net/appletalk/Makefile
index 33164d972d379..410d52f9113e2 100644
--- a/net/appletalk/Makefile
+++ b/net/appletalk/Makefile
@@ -5,6 +5,7 @@
obj-$(CONFIG_ATALK) += appletalk.o
-appletalk-y:= aarp.o ddp.o dev.o
+appletalk-y:= aarp.o ddp.o
appletalk-$(CONFIG_PROC_FS)+= atalk_proc.o
appletalk-$(CONFIG_SYSCTL) += sysctl_net_atalk.o
+appletalk-$(CONFIG_DEV_APPLETALK) += dev.o
diff --git a/net/appletalk/aarp.c b/net/appletalk/aarp.c
index 9fa0b246902be..b15f67293ac4c 100644
--- a/net/appletalk/aarp.c
+++ b/net/appletalk/aarp.c
@@ -438,14 +438,17 @@ static struct atalk_addr *__aarp_proxy_find(struct
net_device *dev,
*/
static void aarp_send_probe_phase1(struct atalk_iface *iface)
{
+#if IS_ENABLED(CONFIG_DEV_APPLETALK)
struct ifreq atreq;
struct sockaddr_at *sa = (struct sockaddr_at *)_addr;
const struct net_device_ops *ops = iface->dev->netdev_ops;
sa->sat_addr.s_node = iface->address.s_node;
sa->sat_addr.s_net = ntohs(iface->address.s_net);
-
- /* We pass the Net:Node to the drivers/cards by a Device ioctl. */
+ /*
+* We used to pass the address via device ioctl, this has to
+* be rewritten if we bring back localtalk.
+*/
if (!(ops->ndo_do_ioctl(iface->dev, , SIOCSIFADDR))) {
ops->ndo_do_ioctl(iface->dev, , SIOCGIFADDR);
if (iface->address.s_net != htons(sa->sat_addr.s_net) ||
@@ -455,13 +458,15 @@ static void aarp_send_probe_phase1(struct atalk_iface
*iface)
iface->address.s_net = htons(sa->sat_addr.s_net);
iface->address.s_node = sa->sat_addr.s_node;
}
+#endif
}
void aarp_probe_network(struct atalk_iface *atif)
{
- if (atif->dev->type == ARPHRD_LOCALTLK ||
- atif->dev->type == ARPHRD_PPP)
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) &&
+ (atif->dev->type == ARPHRD_LOCALTLK ||
+atif->dev->type == ARPHRD_PPP))
aarp_send_probe_phase1(atif);
else {
unsigned int count;
@@ -488,8 +493,9 @@ int aarp_proxy_probe_network(struct atalk_iface *atif,
struct atalk_addr *sa)
* we don't currently support LocalTalk or PPP for proxy AARP;
* if someone wants to try and add it, have fun
*/
- if (atif->dev->type == ARPHRD_LOCALTLK ||
- atif->dev->type == ARPHRD_PPP)
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) &&
+ (atif->dev->type == ARPHRD_LOCALTLK ||
+atif->dev->type == ARPHRD_PPP))
goto out;
/*
@@ -550,7 +556,8 @@ int aarp_send_ddp(struct net_device *dev, struct sk_buff
*skb,
skb_reset_network_header(skb);
/* Check for LocalTalk first */
- if (dev->type == ARPHRD_LOCALTLK) {
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) &&
+ dev->type == ARPHRD_LOCALTLK) {
struct atalk_addr *at = atalk_find_dev_addr(dev);
struct ddpehdr *ddp = (struct ddpehdr *)skb->data;
int ft = 2;
@@
Re: [PATCH 01/10] appletalk: remove localtalk and ppp support
On Mon, Oct 9, 2023, at 18:49, Rodolfo Zitellini wrote: >> From: Arnd Bergmann >> >> The last localtalk driver is gone now, and ppp support was never fully >> merged, so clean up the appletalk code by removing the obvious dead >> code paths. >> >> Notably, this removes one of the two callers of the old .ndo_do_ioctl() >> callback that was abused for getting device addresses and is now >> only used in the ieee802154 subsystem, which still uses the same trick. >> >> The include/uapi/linux/if_ltalk.h header might still be required >> for building userspace programs, but I made sure that debian code >> search and the netatalk upstream have no references it it, so it >> should be fine to remove. >> >> Signed-off-by: Arnd Bergmann > > Hi! > I’ve been working on a new LocalTalk interface driver for the last > couple months, do you think it would be possible to at least postpone > the removal of LT a bit? > > It is a driver for an open source device called TashTalk > (https://github.com/lampmerchant/tashtalk), which runs on a PIC micro > that does all the LT interfacing, and communicates back via serial to > the host system. My driver is relatively simple and works very well > with netatalk 2.2 (which is still maintained and still has support for > AppleTalk). The driver is basically complete and trsted and I was > preparing to submit a patch. > > Still having LocalTalk in my view has many advantages for us > enthusiasts that still want to bridge old machines to the current world > without modifications, for example for printing on modern printers, > netbooting, sharing files and even tcp/ip. All this basically works out > of the box via the driver, Linux and available userspace tools > (netatalk, macipgw). > > The old ISA cards supported by COPS were basically unobtanium even 20 > years ago, but the solution of using a PIC and a serial port is very > robust and much more furure-proof. We also already have a device that > can interface a modern machine directly via USB to LocalTalk. > > The development of the TashTalk has been also extensively discussed on > thr 68KMLA forum > (https://68kmla.org/bb/index.php?threads/modtashtalk-lt0-driver-for-linux.45031/) > > I hope the decision to remove LocalTalk can be reconsidered at least > for the time being so there is a chance to submit a new, modern device > making use of this stack. Nothing is decided, I'm just proposing my patch as a cleanup for now. It would be nice to still drop the ndo_do_ioctl function though, at least in some form. When your driver actually makes it into the kernel, you can find a different method of communicating the address between the socket interface and the device driver. I can see a few ways this could work out: - add a custom callback pointer to struct atalk_iface to get and set the address for phase1 probing instead of going through the ioctl - rewrite the probing logic in aarp.c more widely, and improve the userspace interface in the process by introducing a netlink interface - Move your entire driver into userspace and go to the kernel using tun/tap. This has the added benefit of avoiding a lot of the complexity of the tty line discipline code you have. Arnd
[PATCH 10/10] net: remove ndo_do_ioctl handler
From: Arnd Bergmann
All of the references to the callback pointer are gone, so remove the
pointer itself before we grow new references to it.
Signed-off-by: Arnd Bergmann
---
Documentation/networking/netdevices.rst | 8
include/linux/netdevice.h | 7 ---
2 files changed, 15 deletions(-)
diff --git a/Documentation/networking/netdevices.rst
b/Documentation/networking/netdevices.rst
index 9e4cccb90b870..6f9b71c5d37b8 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -218,14 +218,6 @@ ndo_stop:
Context: process
Note: netif_running() is guaranteed false
-ndo_do_ioctl:
- Synchronization: rtnl_lock() semaphore.
- Context: process
-
-This is only called by network subsystems internally,
-not by user space calling ioctl as it was in before
-linux-5.14.
-
ndo_siocbond:
Synchronization: rtnl_lock() semaphore.
Context: process
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e070a4540fbaf..8d1cc8f195cb6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1121,11 +1121,6 @@ struct netdev_net_notifier {
* int (*ndo_validate_addr)(struct net_device *dev);
* Test if Media Access Control address is valid for the device.
*
- * int (*ndo_do_ioctl)(struct net_device *dev, struct ifreq *ifr, int cmd);
- * Old-style ioctl entry point. This is used internally by the
- * appletalk and ieee802154 subsystems but is no longer called by
- * the device ioctl handler.
- *
* int (*ndo_siocbond)(struct net_device *dev, struct ifreq *ifr, int cmd);
* Used by the bonding driver for its device specific ioctls:
* SIOCBONDENSLAVE, SIOCBONDRELEASE, SIOCBONDSETHWADDR,
SIOCBONDCHANGEACTIVE,
@@ -1429,8 +1424,6 @@ struct net_device_ops {
int (*ndo_set_mac_address)(struct net_device *dev,
void *addr);
int (*ndo_validate_addr)(struct net_device *dev);
- int (*ndo_do_ioctl)(struct net_device *dev,
- struct ifreq *ifr, int cmd);
int (*ndo_eth_ioctl)(struct net_device *dev,
struct ifreq *ifr, int cmd);
int (*ndo_siocbond)(struct net_device *dev,
--
2.39.2
[PATCH 08/10] wireless: atmel: remove unused ioctl function
From: Arnd Bergmann
This function has no callers, and for the past 20 years, the request_firmware
interface has been in place instead of the custom firmware loader.
Signed-off-by: Arnd Bergmann
---
drivers/net/wireless/atmel/atmel.c | 72 --
1 file changed, 72 deletions(-)
diff --git a/drivers/net/wireless/atmel/atmel.c
b/drivers/net/wireless/atmel/atmel.c
index 7c2d1c588156d..461dce21de2b0 100644
--- a/drivers/net/wireless/atmel/atmel.c
+++ b/drivers/net/wireless/atmel/atmel.c
@@ -571,7 +571,6 @@ static const struct {
{ REG_DOMAIN_ISRAEL, 3, 9, "Israel"} };
static void build_wpa_mib(struct atmel_private *priv);
-static int atmel_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
static void atmel_copy_to_card(struct net_device *dev, u16 dest,
const unsigned char *src, u16 len);
static void atmel_copy_to_host(struct net_device *dev, unsigned char *dest,
@@ -1487,7 +1486,6 @@ static const struct net_device_ops atmel_netdev_ops = {
.ndo_stop = atmel_close,
.ndo_set_mac_address= atmel_set_mac_address,
.ndo_start_xmit = start_tx,
- .ndo_do_ioctl = atmel_ioctl,
.ndo_validate_addr = eth_validate_addr,
};
@@ -2616,76 +2614,6 @@ static const struct iw_handler_def atmel_handler_def = {
.get_wireless_stats = atmel_get_wireless_stats
};
-static int atmel_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
- int i, rc = 0;
- struct atmel_private *priv = netdev_priv(dev);
- struct atmel_priv_ioctl com;
- struct iwreq *wrq = (struct iwreq *) rq;
- unsigned char *new_firmware;
- char domain[REGDOMAINSZ + 1];
-
- switch (cmd) {
- case ATMELIDIFC:
- wrq->u.param.value = ATMELMAGIC;
- break;
-
- case ATMELFWL:
- if (copy_from_user(, rq->ifr_data, sizeof(com))) {
- rc = -EFAULT;
- break;
- }
-
- if (!capable(CAP_NET_ADMIN)) {
- rc = -EPERM;
- break;
- }
-
- new_firmware = memdup_user(com.data, com.len);
- if (IS_ERR(new_firmware)) {
- rc = PTR_ERR(new_firmware);
- break;
- }
-
- kfree(priv->firmware);
-
- priv->firmware = new_firmware;
- priv->firmware_length = com.len;
- strncpy(priv->firmware_id, com.id, 31);
- priv->firmware_id[31] = '\0';
- break;
-
- case ATMELRD:
- if (copy_from_user(domain, rq->ifr_data, REGDOMAINSZ)) {
- rc = -EFAULT;
- break;
- }
-
- if (!capable(CAP_NET_ADMIN)) {
- rc = -EPERM;
- break;
- }
-
- domain[REGDOMAINSZ] = 0;
- rc = -EINVAL;
- for (i = 0; i < ARRAY_SIZE(channel_table); i++) {
- if (!strcasecmp(channel_table[i].name, domain)) {
- priv->config_reg_domain =
channel_table[i].reg_domain;
- rc = 0;
- }
- }
-
- if (rc == 0 && priv->station_state != STATION_STATE_DOWN)
- rc = atmel_open(dev);
- break;
-
- default:
- rc = -EOPNOTSUPP;
- }
-
- return rc;
-}
-
struct auth_body {
__le16 alg;
__le16 trans_seq;
--
2.39.2
[PATCH 09/10] wireless: hostap: remove unused ioctl function
From: Arnd Bergmann
The ioctl handler has no actual callers in the kernel and is useless.
All the functionality should be reachable through the regualar interfaces.
Signed-off-by: Arnd Bergmann
---
drivers/net/wireless/intersil/hostap/hostap.h | 1 -
.../wireless/intersil/hostap/hostap_ioctl.c | 228 --
.../wireless/intersil/hostap/hostap_main.c| 3 -
3 files changed, 232 deletions(-)
diff --git a/drivers/net/wireless/intersil/hostap/hostap.h
b/drivers/net/wireless/intersil/hostap/hostap.h
index c17ab6dbbb538..552ae33d78751 100644
--- a/drivers/net/wireless/intersil/hostap/hostap.h
+++ b/drivers/net/wireless/intersil/hostap/hostap.h
@@ -92,7 +92,6 @@ void hostap_info_process(local_info_t *local, struct sk_buff
*skb);
extern const struct iw_handler_def hostap_iw_handler_def;
extern const struct ethtool_ops prism2_ethtool_ops;
-int hostap_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd);
int hostap_siocdevprivate(struct net_device *dev, struct ifreq *ifr,
void __user *data, int cmd);
diff --git a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
index b4adfc190ae87..26162f92e3c3d 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
@@ -2316,21 +2316,6 @@ static const struct iw_priv_args prism2_priv[] = {
};
-static int prism2_ioctl_priv_inquire(struct net_device *dev, int *i)
-{
- struct hostap_interface *iface;
- local_info_t *local;
-
- iface = netdev_priv(dev);
- local = iface->local;
-
- if (local->func->cmd(dev, HFA384X_CMDCODE_INQUIRE, *i, NULL, NULL))
- return -EOPNOTSUPP;
-
- return 0;
-}
-
-
static int prism2_ioctl_priv_prism2_param(struct net_device *dev,
struct iw_request_info *info,
union iwreq_data *uwrq, char *extra)
@@ -2910,146 +2895,6 @@ static int prism2_ioctl_priv_writemif(struct net_device
*dev,
}
-static int prism2_ioctl_priv_monitor(struct net_device *dev, int *i)
-{
- struct hostap_interface *iface;
- local_info_t *local;
- int ret = 0;
- union iwreq_data wrqu;
-
- iface = netdev_priv(dev);
- local = iface->local;
-
- printk(KERN_DEBUG "%s: process %d (%s) used deprecated iwpriv monitor "
- "- update software to use iwconfig mode monitor\n",
- dev->name, task_pid_nr(current), current->comm);
-
- /* Backward compatibility code - this can be removed at some point */
-
- if (*i == 0) {
- /* Disable monitor mode - old mode was not saved, so go to
-* Master mode */
- wrqu.mode = IW_MODE_MASTER;
- ret = prism2_ioctl_siwmode(dev, NULL, , NULL);
- } else if (*i == 1) {
- /* netlink socket mode is not supported anymore since it did
-* not separate different devices from each other and was not
-* best method for delivering large amount of packets to
-* user space */
- ret = -EOPNOTSUPP;
- } else if (*i == 2 || *i == 3) {
- switch (*i) {
- case 2:
- local->monitor_type = PRISM2_MONITOR_80211;
- break;
- case 3:
- local->monitor_type = PRISM2_MONITOR_PRISM;
- break;
- }
- wrqu.mode = IW_MODE_MONITOR;
- ret = prism2_ioctl_siwmode(dev, NULL, , NULL);
- hostap_monitor_mode_enable(local);
- } else
- ret = -EINVAL;
-
- return ret;
-}
-
-
-static int prism2_ioctl_priv_reset(struct net_device *dev, int *i)
-{
- struct hostap_interface *iface;
- local_info_t *local;
-
- iface = netdev_priv(dev);
- local = iface->local;
-
- printk(KERN_DEBUG "%s: manual reset request(%d)\n", dev->name, *i);
- switch (*i) {
- case 0:
- /* Disable and enable card */
- local->func->hw_shutdown(dev, 1);
- local->func->hw_config(dev, 0);
- break;
-
- case 1:
- /* COR sreset */
- local->func->hw_reset(dev);
- break;
-
- case 2:
- /* Disable and enable port 0 */
- local->func->reset_port(dev);
- break;
-
- case 3:
- prism2_sta_deauth(local, WLAN_REASON_DEAUTH_LEAVING);
- if (local->func->cmd(dev, HFA384X_CMDCODE_DISABLE, 0, NULL,
-NULL))
- return -EINVAL;
- break;
-
- case 4:
- if (local->func->cmd(dev, HFA384X_CMDCODE_ENABLE, 0, NULL,
-
[PATCH 07/10] staging: rtl8723bs: remove dead code
From: Arnd Bergmann
The .ndo_do_ioctl functions are never called, so the three implementation here
is useless but only works as a way to identify the device in the notifiers,
which can really be removed as well.
Looking through the exported functions, I found a bunch more that have
no callers, so just drop all of those.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8723bs/Makefile|1 -
.../staging/rtl8723bs/include/osdep_intf.h| 32 -
drivers/staging/rtl8723bs/include/rtw_io.h|1 -
.../staging/rtl8723bs/os_dep/ioctl_linux.c| 1300 -
drivers/staging/rtl8723bs/os_dep/os_intfs.c | 29 -
drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 23 +-
6 files changed, 1 insertion(+), 1385 deletions(-)
delete mode 100644 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
diff --git a/drivers/staging/rtl8723bs/Makefile
b/drivers/staging/rtl8723bs/Makefile
index 590bde02058c7..0f3f6dea4955e 100644
--- a/drivers/staging/rtl8723bs/Makefile
+++ b/drivers/staging/rtl8723bs/Makefile
@@ -50,7 +50,6 @@ r8723bs-y = \
hal/HalHWImg8723B_RF.o \
hal/HalPhyRf_8723B.o \
os_dep/ioctl_cfg80211.o \
- os_dep/ioctl_linux.o \
os_dep/mlme_linux.o \
os_dep/osdep_service.o \
os_dep/os_intfs.o \
diff --git a/drivers/staging/rtl8723bs/include/osdep_intf.h
b/drivers/staging/rtl8723bs/include/osdep_intf.h
index 111e0179712ac..83a25598e9627 100644
--- a/drivers/staging/rtl8723bs/include/osdep_intf.h
+++ b/drivers/staging/rtl8723bs/include/osdep_intf.h
@@ -8,33 +8,6 @@
#ifndef __OSDEP_INTF_H_
#define __OSDEP_INTF_H_
-
-struct intf_priv {
-
- u8 *intf_dev;
- u32 max_iosz; /* USB2.0: 128, USB1.1: 64, SDIO:64 */
- u32 max_xmitsz; /* USB2.0: unlimited, SDIO:512 */
- u32 max_recvsz; /* USB2.0: unlimited, SDIO:512 */
-
- volatile u8 *io_rwmem;
- volatile u8 *allocated_io_rwmem;
- u32 io_wsz; /* unit: 4bytes */
- u32 io_rsz;/* unit: 4bytes */
- u8 intf_status;
-
- void (*_bus_io)(u8 *priv);
-
-/*
-Under Sync. IRP (SDIO/USB)
-A protection mechanism is necessary for the io_rwmem(read/write protocol)
-
-Under Async. IRP (SDIO/USB)
-The protection mechanism is through the pending queue.
-*/
-
- struct mutex ioctl_mutex;
-};
-
struct dvobj_priv *devobj_init(void);
void devobj_deinit(struct dvobj_priv *pdvobj);
@@ -47,17 +20,12 @@ u32 rtw_start_drv_threads(struct adapter *padapter);
void rtw_stop_drv_threads(struct adapter *padapter);
void rtw_cancel_all_timer(struct adapter *padapter);
-int rtw_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
-
int rtw_init_netdev_name(struct net_device *pnetdev, const char *ifname);
struct net_device *rtw_init_netdev(struct adapter *padapter);
void rtw_unregister_netdevs(struct dvobj_priv *dvobj);
u16 rtw_recv_select_queue(struct sk_buff *skb);
-int rtw_ndev_notifier_register(void);
-void rtw_ndev_notifier_unregister(void);
-
void rtw_ips_dev_unload(struct adapter *padapter);
int rtw_ips_pwr_up(struct adapter *padapter);
diff --git a/drivers/staging/rtl8723bs/include/rtw_io.h
b/drivers/staging/rtl8723bs/include/rtw_io.h
index e98083a07a660..f92093e73fe67 100644
--- a/drivers/staging/rtl8723bs/include/rtw_io.h
+++ b/drivers/staging/rtl8723bs/include/rtw_io.h
@@ -72,7 +72,6 @@
#define _INTF_ASYNC_ BIT(0) /* support async io */
-struct intf_priv;
struct intf_hdl;
struct io_queue;
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
deleted file mode 100644
index c81b30f1f1b05..0
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ /dev/null
@@ -1,1300 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/**
- *
- * Copyright(c) 2007 - 2012 Realtek Corporation. All rights reserved.
- *
-
**/
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#define RTL_IOCTL_WPA_SUPPLICANT (SIOCIWFIRSTPRIV + 30)
-
-static int wpa_set_auth_algs(struct net_device *dev, u32 value)
-{
- struct adapter *padapter = rtw_netdev_priv(dev);
- int ret = 0;
-
- if ((value & IW_AUTH_ALG_SHARED_KEY) && (value &
IW_AUTH_ALG_OPEN_SYSTEM)) {
- padapter->securitypriv.ndisencryptstatus =
Ndis802_11Encryption1Enabled;
- padapter->securitypriv.ndisauthtype =
Ndis802_11AuthModeAutoSwitch;
- padapter->securitypriv.dot11AuthAlgrthm = dot11AuthAlgrthm_Auto;
- } else if (value & IW_AUTH_ALG_SHARED_KEY) {
- padapter->securitypriv.ndisencryptstatus =
Ndis802_11Encryption1Enabled;
-
- padapter->securitypriv.ndisauthtype = Ndis802_11AuthModeShared;
- padapter->securitypriv.dot11
[PATCH 06/10] staging: rtl8712: remove unused legacy ioctl handlers
From: Arnd Bergmann
The .ndo_do_ioctl functions are never called, and can just be removed,
especially since this is a staging driver.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8712/os_intfs.c| 1 -
drivers/staging/rtl8712/osdep_intf.h | 2 -
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 124 --
3 files changed, 127 deletions(-)
diff --git a/drivers/staging/rtl8712/os_intfs.c
b/drivers/staging/rtl8712/os_intfs.c
index b18e6d9c832b8..121edffbd2507 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -191,7 +191,6 @@ static const struct net_device_ops rtl8712_netdev_ops = {
.ndo_start_xmit = r8712_xmit_entry,
.ndo_set_mac_address = r871x_net_set_mac_address,
.ndo_get_stats = r871x_net_get_stats,
- .ndo_do_ioctl = r871x_ioctl,
};
struct net_device *r8712_init_netdev(void)
diff --git a/drivers/staging/rtl8712/osdep_intf.h
b/drivers/staging/rtl8712/osdep_intf.h
index 9e75116c987ec..ce823030bfec2 100644
--- a/drivers/staging/rtl8712/osdep_intf.h
+++ b/drivers/staging/rtl8712/osdep_intf.h
@@ -27,6 +27,4 @@ struct intf_priv {
struct completion io_retevt_comp;
};
-int r871x_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
-
#endif /*_OSDEP_INTF_H_*/
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 36f6904d25abc..a4a34c9f00b84 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -36,8 +36,6 @@
#include
#include
-#define RTL_IOCTL_WPA_SUPPLICANT (SIOCIWFIRSTPRIV + 0x1E)
-
#define SCAN_ITEM_SIZE 768
#define MAX_CUSTOM_LEN 64
#define RATE_COUNT 4
@@ -2066,128 +2064,6 @@ static int r871x_wps_start(struct net_device *dev,
return 0;
}
-static int wpa_set_param(struct net_device *dev, u8 name, u32 value)
-{
- struct _adapter *padapter = netdev_priv(dev);
-
- switch (name) {
- case IEEE_PARAM_WPA_ENABLED:
- padapter->securitypriv.AuthAlgrthm = 2; /* 802.1x */
- switch ((value) & 0xff) {
- case 1: /* WPA */
- padapter->securitypriv.ndisauthtype =
- Ndis802_11AuthModeWPAPSK; /* WPA_PSK */
- padapter->securitypriv.ndisencryptstatus =
- Ndis802_11Encryption2Enabled;
- break;
- case 2: /* WPA2 */
- padapter->securitypriv.ndisauthtype =
- Ndis802_11AuthModeWPA2PSK; /* WPA2_PSK */
- padapter->securitypriv.ndisencryptstatus =
- Ndis802_11Encryption3Enabled;
- break;
- }
- break;
- case IEEE_PARAM_TKIP_COUNTERMEASURES:
- break;
- case IEEE_PARAM_DROP_UNENCRYPTED:
- /* HACK:
-*
-* wpa_supplicant calls set_wpa_enabled when the driver
-* is loaded and unloaded, regardless of if WPA is being
-* used. No other calls are made which can be used to
-* determine if encryption will be used or not prior to
-* association being expected. If encryption is not being
-* used, drop_unencrypted is set to false, else true -- we
-* can use this to determine if the CAP_PRIVACY_ON bit should
-* be set.
-*/
- break;
- case IEEE_PARAM_PRIVACY_INVOKED:
- break;
- case IEEE_PARAM_AUTH_ALGS:
- return wpa_set_auth_algs(dev, value);
- case IEEE_PARAM_IEEE_802_1X:
- break;
- case IEEE_PARAM_WPAX_SELECT:
- /* added for WPA2 mixed mode */
- break;
- default:
- return -EOPNOTSUPP;
- }
- return 0;
-}
-
-static int wpa_mlme(struct net_device *dev, u32 command, u32 reason)
-{
- struct _adapter *padapter = netdev_priv(dev);
-
- switch (command) {
- case IEEE_MLME_STA_DEAUTH:
- if (!r8712_set_802_11_disassociate(padapter))
- return -1;
- break;
- case IEEE_MLME_STA_DISASSOC:
- if (!r8712_set_802_11_disassociate(padapter))
- return -1;
- break;
- default:
- return -EOPNOTSUPP;
- }
- return 0;
-}
-
-static int wpa_supplicant_ioctl(struct net_device *dev, struct iw_point *p)
-{
- struct ieee_param *param;
- int ret = 0;
- struct _adapter *padapter = netdev_priv(dev);
-
- if (p->length < sizeof(struct ieee_param) || !p->pointer)
- return -EINVAL;
- param = memdup_user(p->pointer, p->length);
- if (IS_ERR(param))
- return PTR_ERR(
[PATCH 05/10] staging: rtl8192: remove unused legacy ioctl handlers
From: Arnd Bergmann
The .ndo_do_ioctl functions are never called, and can just be removed,
especially since this is a staging driver.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8192u/ieee80211/dot11d.c | 41 --
drivers/staging/rtl8192u/ieee80211/dot11d.h | 2 -
.../staging/rtl8192u/ieee80211/ieee80211.h| 12 -
.../rtl8192u/ieee80211/ieee80211_softmac.c| 563 --
drivers/staging/rtl8192u/r8192U.h | 2 -
drivers/staging/rtl8192u/r8192U_core.c| 109
6 files changed, 729 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c
b/drivers/staging/rtl8192u/ieee80211/dot11d.c
index ddaf66fa0f936..8a72c1e9eb1e1 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.c
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c
@@ -97,22 +97,6 @@ void dot11d_update_country_ie(struct ieee80211_device *dev,
u8 *pTaddr,
}
EXPORT_SYMBOL(dot11d_update_country_ie);
-u8 dot11d_get_max_tx_pwr_in_dbm(struct ieee80211_device *dev, u8 Channel)
-{
- struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
- u8 MaxTxPwrInDbm = 255;
-
- if (Channel > MAX_CHANNEL_NUMBER) {
- netdev_err(dev->dev, "%s: Invalid Channel\n", __func__);
- return MaxTxPwrInDbm;
- }
- if (dot11d_info->channel_map[Channel])
- MaxTxPwrInDbm = dot11d_info->max_tx_pwr_dbm_list[Channel];
-
- return MaxTxPwrInDbm;
-}
-EXPORT_SYMBOL(dot11d_get_max_tx_pwr_in_dbm);
-
void dot11d_scan_complete(struct ieee80211_device *dev)
{
struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
@@ -147,28 +131,3 @@ int is_legal_channel(struct ieee80211_device *dev, u8
channel)
return 0;
}
EXPORT_SYMBOL(is_legal_channel);
-
-int to_legal_channel(struct ieee80211_device *dev, u8 channel)
-{
- struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
- u8 default_chn = 0;
- u32 i = 0;
-
- for (i = 1; i <= MAX_CHANNEL_NUMBER; i++) {
- if (dot11d_info->channel_map[i] > 0) {
- default_chn = i;
- break;
- }
- }
-
- if (channel > MAX_CHANNEL_NUMBER) {
- netdev_err(dev->dev, "%s: Invalid Channel\n", __func__);
- return default_chn;
- }
-
- if (dot11d_info->channel_map[channel] > 0)
- return channel;
-
- return default_chn;
-}
-EXPORT_SYMBOL(to_legal_channel);
diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.h
b/drivers/staging/rtl8192u/ieee80211/dot11d.h
index 8b485fa180898..fd774265211a5 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.h
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.h
@@ -49,9 +49,7 @@ void dot11d_update_country_ie(struct ieee80211_device *dev,
u8 *addr,
u16 coutry_ie_len,
u8 *coutry_ie);
-u8 dot11d_get_max_tx_pwr_in_dbm(struct ieee80211_device *dev, u8 channel);
void dot11d_scan_complete(struct ieee80211_device *dev);
int is_legal_channel(struct ieee80211_device *dev, u8 channel);
-int to_legal_channel(struct ieee80211_device *dev, u8 channel);
#endif /* #ifndef __INC_DOT11D_H */
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 694d1b18f81c7..fc4201757c408 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -223,11 +223,7 @@ struct cb_desc {
#define MAX_IE_LEN 0xff
// added for kernel conflict
-#define ieee80211_wake_queue ieee80211_wake_queue_rsl
-#define ieee80211_stop_queue ieee80211_stop_queue_rsl
#define notify_wx_assoc_event notify_wx_assoc_event_rsl
-#define SendDisassociation SendDisassociation_rsl
-
struct ieee_param {
u32 cmd;
@@ -2152,7 +2148,6 @@ int ieee80211_wx_set_gen_ie(struct ieee80211_device
*ieee, u8 *ie, size_t len);
/* ieee80211_softmac.c */
short ieee80211_is_54g(const struct ieee80211_network *net);
-short ieee80211_is_shortslot(const struct ieee80211_network *net);
int ieee80211_rx_frame_softmac(struct ieee80211_device *ieee,
struct sk_buff *skb,
struct ieee80211_rx_stats *rx_stats,
@@ -2160,7 +2155,6 @@ int ieee80211_rx_frame_softmac(struct ieee80211_device
*ieee,
void ieee80211_softmac_new_net(struct ieee80211_device *ieee,
struct ieee80211_network *net);
-void SendDisassociation(struct ieee80211_device *ieee, u8 *asSta, u8 asRsn);
void ieee80211_softmac_xmit(struct ieee80211_txb *txb,
struct ieee80211_device *ieee);
@@ -2182,13 +2176,7 @@ void ieee80211_stop_protocol(struct ieee80211_device
*ieee);
void ieee80211_softmac_start_protocol(struct ieee80211_device *ieee);
void ieee80211_softmac_stop_protocol(struct
[PATCH 04/10] staging: ks7010: remove unused ioctl handler
From: Arnd Bergmann
The ndo_do_ioctl function has no actual callers, and doesn't do much here,
so just remove it entirely as preparation for removing the callback pointer
from net_device_ops.
Signed-off-by: Arnd Bergmann
---
drivers/staging/ks7010/ks_wlan_net.c | 21 -
1 file changed, 21 deletions(-)
diff --git a/drivers/staging/ks7010/ks_wlan_net.c
b/drivers/staging/ks7010/ks_wlan_net.c
index 0fb97a79ad0b3..ab7463bb25169 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -51,8 +51,6 @@ static int ks_wlan_close(struct net_device *dev);
static void ks_wlan_set_rx_mode(struct net_device *dev);
static struct net_device_stats *ks_wlan_get_stats(struct net_device *dev);
static int ks_wlan_set_mac_address(struct net_device *dev, void *addr);
-static int ks_wlan_netdev_ioctl(struct net_device *dev, struct ifreq *rq,
- int cmd);
static atomic_t update_phyinfo;
static struct timer_list update_phyinfo_timer;
@@ -2458,24 +2456,6 @@ static const struct iw_handler_def ks_wlan_handler_def =
{
.get_wireless_stats = ks_get_wireless_stats,
};
-static int ks_wlan_netdev_ioctl(struct net_device *dev, struct ifreq *rq,
- int cmd)
-{
- int ret;
- struct iwreq *wrq = (struct iwreq *)rq;
-
- switch (cmd) {
- case SIOCIWFIRSTPRIV + 20: /* KS_WLAN_SET_STOP_REQ */
- ret = ks_wlan_set_stop_request(dev, NULL, >u, NULL);
- break;
- // All other calls are currently unsupported
- default:
- ret = -EOPNOTSUPP;
- }
-
- return ret;
-}
-
static
struct net_device_stats *ks_wlan_get_stats(struct net_device *dev)
{
@@ -2608,7 +2588,6 @@ static const struct net_device_ops ks_wlan_netdev_ops = {
.ndo_start_xmit = ks_wlan_start_xmit,
.ndo_open = ks_wlan_open,
.ndo_stop = ks_wlan_close,
- .ndo_do_ioctl = ks_wlan_netdev_ioctl,
.ndo_set_mac_address = ks_wlan_set_mac_address,
.ndo_get_stats = ks_wlan_get_stats,
.ndo_tx_timeout = ks_wlan_tx_timeout,
--
2.39.2
[PATCH 03/10] ethernet: sp7021: fix ioctl callback pointer
From: Arnd Bergmann
The old .ndo_do_ioctl() callback is never called any more, instead the
driver should set .ndo_eth_ioctl() for the phy operations.
Fixes: fd3040b9394c5 ("net: ethernet: Add driver for Sunplus SP7021")
Signed-off-by: Arnd Bergmann
---
drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c
b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 391a1bc7f4463..bb4514f4e8157 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
.ndo_start_xmit = spl2sw_ethernet_start_xmit,
.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
- .ndo_do_ioctl = phy_do_ioctl,
+ .ndo_eth_ioctl = phy_do_ioctl,
.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
};
--
2.39.2
[PATCH 02/10] ieee802154: avoid deprecated .ndo_do_ioctl callback
From: Arnd Bergmann
The ieee802154 socket implementation is the last remaining caller of the
netdevice ioctl callback. In order to completely remove this, add a custom
pointer to the existing wpan_dev specific operations structure. Since that
structure is currently only used to wrap the 'create' header operation,
adjust the naming slightly to make this more generic.
It would be a good idea to adjust the calling conventions and split the
get/set operations into separate functions, but that can be a follow-up
cleanup. For the moment, I kept the actual changes to a minimum to
avoid regressions.
Signed-off-by: Arnd Bergmann
---
include/net/cfg802154.h | 9 +
net/ieee802154/socket.c | 5 +++--
net/mac802154/iface.c | 8
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index f79ce133e51a7..e604df98e2ee9 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -433,15 +433,16 @@ struct ieee802154_llsec_device_key {
u32 frame_counter;
};
-struct wpan_dev_header_ops {
+struct wpan_dev_ops {
/* TODO create callback currently assumes ieee802154_mac_cb inside
* skb->cb. This should be changed to give these information as
* parameter.
*/
- int (*create)(struct sk_buff *skb, struct net_device *dev,
+ int (*header_create)(struct sk_buff *skb, struct net_device *dev,
const struct ieee802154_addr *daddr,
const struct ieee802154_addr *saddr,
unsigned int len);
+ int (*ioctl)(struct net_device *dev, struct ifreq *ifr, int cmd);
};
struct wpan_dev {
@@ -452,7 +453,7 @@ struct wpan_dev {
struct list_head list;
struct net_device *netdev;
- const struct wpan_dev_header_ops *header_ops;
+ const struct wpan_dev_ops *ops;
/* lowpan interface, set when the wpan_dev belongs to one lowpan_dev */
struct net_device *lowpan_dev;
@@ -491,7 +492,7 @@ wpan_dev_hard_header(struct sk_buff *skb, struct net_device
*dev,
{
struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
- return wpan_dev->header_ops->create(skb, dev, daddr, saddr, len);
+ return wpan_dev->ops->header_create(skb, dev, daddr, saddr, len);
}
#endif
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 00302e8b9615b..27e58237091ca 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -139,8 +139,9 @@ static int ieee802154_dev_ioctl(struct sock *sk, struct
ifreq __user *arg,
if (!dev)
return -ENODEV;
- if (dev->type == ARPHRD_IEEE802154 && dev->netdev_ops->ndo_do_ioctl)
- ret = dev->netdev_ops->ndo_do_ioctl(dev, , cmd);
+ if (dev->type == ARPHRD_IEEE802154 && dev->ieee802154_ptr &&
+ dev->ieee802154_ptr->ops)
+ ret = dev->ieee802154_ptr->ops->ioctl(dev, , cmd);
if (!ret && put_user_ifreq(, arg))
ret = -EFAULT;
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index c0e2da5072bea..4937f8c2fb4cc 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -406,8 +406,9 @@ static int ieee802154_header_create(struct sk_buff *skb,
return hlen;
}
-static const struct wpan_dev_header_ops ieee802154_header_ops = {
- .create = ieee802154_header_create,
+static const struct wpan_dev_ops ieee802154_ops = {
+ .header_create = ieee802154_header_create,
+ .ioctl = mac802154_wpan_ioctl,
};
/* This header create functionality assumes a 8 byte array for
@@ -495,7 +496,6 @@ static const struct net_device_ops mac802154_wpan_ops = {
.ndo_open = mac802154_wpan_open,
.ndo_stop = mac802154_slave_close,
.ndo_start_xmit = ieee802154_subif_start_xmit,
- .ndo_do_ioctl = mac802154_wpan_ioctl,
.ndo_set_mac_address= mac802154_wpan_mac_addr,
};
@@ -581,7 +581,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
sdata->dev->netdev_ops = _wpan_ops;
sdata->dev->ml_priv = _mlme_wpan;
sdata->iface_default_filtering =
IEEE802154_FILTERING_4_FRAME_FIELDS;
- wpan_dev->header_ops = _header_ops;
+ wpan_dev->ops = _ops;
mutex_init(>sec_mtx);
--
2.39.2
[PATCH 01/10] appletalk: remove localtalk and ppp support
From: Arnd Bergmann
The last localtalk driver is gone now, and ppp support was never fully
merged, so clean up the appletalk code by removing the obvious dead
code paths.
Notably, this removes one of the two callers of the old .ndo_do_ioctl()
callback that was abused for getting device addresses and is now
only used in the ieee802154 subsystem, which still uses the same trick.
The include/uapi/linux/if_ltalk.h header might still be required
for building userspace programs, but I made sure that debian code
search and the netatalk upstream have no references it it, so it
should be fine to remove.
Signed-off-by: Arnd Bergmann
---
drivers/net/tun.c | 3 -
include/linux/atalk.h | 1 -
include/linux/if_ltalk.h | 8 ---
include/uapi/linux/if_ltalk.h | 10
net/appletalk/Makefile| 2 +-
net/appletalk/aarp.c | 108 +++---
net/appletalk/ddp.c | 97 +-
net/appletalk/dev.c | 46 ---
8 files changed, 11 insertions(+), 264 deletions(-)
delete mode 100644 include/linux/if_ltalk.h
delete mode 100644 include/uapi/linux/if_ltalk.h
delete mode 100644 net/appletalk/dev.c
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 89ab9efe522c3..e11476296e253 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -70,7 +70,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -3059,8 +3058,6 @@ static unsigned char tun_get_addr_len(unsigned short type)
return ROSE_ADDR_LEN;
case ARPHRD_NETROM:
return AX25_ADDR_LEN;
- case ARPHRD_LOCALTLK:
- return LTALK_ALEN;
default:
return 0;
}
diff --git a/include/linux/atalk.h b/include/linux/atalk.h
index a55bfc6567d01..2896f2ac9568e 100644
--- a/include/linux/atalk.h
+++ b/include/linux/atalk.h
@@ -121,7 +121,6 @@ static inline struct atalk_iface *atalk_find_dev(struct
net_device *dev)
#endif
extern struct atalk_addr *atalk_find_dev_addr(struct net_device *dev);
-extern struct net_device *atrtr_get_dev(struct atalk_addr *sa);
extern int aarp_send_ddp(struct net_device *dev,
struct sk_buff *skb,
struct atalk_addr *sa, void *hwaddr);
diff --git a/include/linux/if_ltalk.h b/include/linux/if_ltalk.h
deleted file mode 100644
index 4cc1c0b778700..0
--- a/include/linux/if_ltalk.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __LINUX_LTALK_H
-#define __LINUX_LTALK_H
-
-#include
-
-extern struct net_device *alloc_ltalkdev(int sizeof_priv);
-#endif
diff --git a/include/uapi/linux/if_ltalk.h b/include/uapi/linux/if_ltalk.h
deleted file mode 100644
index fa61e776f598d..0
--- a/include/uapi/linux/if_ltalk.h
+++ /dev/null
@@ -1,10 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI__LINUX_LTALK_H
-#define _UAPI__LINUX_LTALK_H
-
-#define LTALK_HLEN 1
-#define LTALK_MTU 600
-#define LTALK_ALEN 1
-
-
-#endif /* _UAPI__LINUX_LTALK_H */
diff --git a/net/appletalk/Makefile b/net/appletalk/Makefile
index 33164d972d379..152312a151800 100644
--- a/net/appletalk/Makefile
+++ b/net/appletalk/Makefile
@@ -5,6 +5,6 @@
obj-$(CONFIG_ATALK) += appletalk.o
-appletalk-y:= aarp.o ddp.o dev.o
+appletalk-y:= aarp.o ddp.o
appletalk-$(CONFIG_PROC_FS)+= atalk_proc.o
appletalk-$(CONFIG_SYSCTL) += sysctl_net_atalk.o
diff --git a/net/appletalk/aarp.c b/net/appletalk/aarp.c
index 9fa0b246902be..dfcd9f46cb3a6 100644
--- a/net/appletalk/aarp.c
+++ b/net/appletalk/aarp.c
@@ -432,49 +432,18 @@ static struct atalk_addr *__aarp_proxy_find(struct
net_device *dev,
return a ? sa : NULL;
}
-/*
- * Probe a Phase 1 device or a device that requires its Net:Node to
- * be set via an ioctl.
- */
-static void aarp_send_probe_phase1(struct atalk_iface *iface)
-{
- struct ifreq atreq;
- struct sockaddr_at *sa = (struct sockaddr_at *)_addr;
- const struct net_device_ops *ops = iface->dev->netdev_ops;
-
- sa->sat_addr.s_node = iface->address.s_node;
- sa->sat_addr.s_net = ntohs(iface->address.s_net);
-
- /* We pass the Net:Node to the drivers/cards by a Device ioctl. */
- if (!(ops->ndo_do_ioctl(iface->dev, , SIOCSIFADDR))) {
- ops->ndo_do_ioctl(iface->dev, , SIOCGIFADDR);
- if (iface->address.s_net != htons(sa->sat_addr.s_net) ||
- iface->address.s_node != sa->sat_addr.s_node)
- iface->status |= ATIF_PROBE_FAIL;
-
- iface->address.s_net = htons(sa->sat_addr.s_net);
- iface->address.s_node = sa->sat_addr.s_node;
- }
-}
-
-
void aarp_probe_network(struct atalk_iface *atif)
{
- i
Re: [PATCH] net: appletalk: remove cops support
On Wed, Oct 4, 2023, at 20:52, Jakub Kicinski wrote: > On Wed, 27 Sep 2023 11:00:30 +0200 Greg Kroah-Hartman wrote: >> The COPS Appletalk support is very old, never said to actually work >> properly, and the firmware code for the devices are under a very suspect >> license. Remove it all to clear up the license issue, if it is still >> needed and actually used by anyone, we can add it back later once the >> license is cleared up. > > Nice, Doug and Arnd also mentioned this in the past so let me add > them to the CC as I apply this... Yes, definitely, thanks Greg for getting this done. I think every time this came up we concluded that it can be removed, we just never finished the job. Acked-by: Arnd Bergmann Link: https://lore.kernel.org/netdev/e490dd0c-a65d-4acf-89c6-c06cb48ec...@app.fastmail.com/ Link: https://lore.kernel.org/netdev/9cac4fbd-9557-b0b8-54fa-93f0290a6...@schmorgal.com/ Semi-related: Since this removes one of the two callers of the .ndo_do_ioctl() callback, I've had a new look at that bit as well and ended up with a refresh of the missing bits of [1], which I'll submit next. Arnd [1] https://lore.kernel.org/lkml/20201106221743.3271965-1-a...@kernel.org/
Re: [PATCH v3 10/13] arch: make execmem setup available regardless of CONFIG_MODULES
On Mon, Sep 18, 2023, at 09:29, Mike Rapoport wrote:
> index a42e4cd11db2..c0b536e398b4 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> +#ifdef CONFIG_XIP_KERNEL
> +/*
> + * The XIP kernel text is mapped in the module area for modules and
> + * some other stuff to work without any indirect relocations.
> + * MODULES_VADDR is redefined here and not in asm/memory.h to avoid
> + * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned
> on/off.
> + */
> +#undef MODULES_VADDR
> +#define MODULES_VADDR(((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> +#endif
> +
> +#if defined(CONFIG_MMU) && defined(CONFIG_EXECMEM)
> +static struct execmem_params execmem_params __ro_after_init = {
> + .ranges = {
> + [EXECMEM_DEFAULT] = {
> + .start = MODULES_VADDR,
> + .end = MODULES_END,
> + .alignment = 1,
> + },
This causes a randconfig build failure for me on linux-next now:
arch/arm/mm/init.c:499:25: error: initializer element is not constant
499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
PMD_MASK)
| ^
arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
506 | .start = MODULES_VADDR,
| ^
arch/arm/mm/init.c:499:25: note: (near initialization for
'execmem_params.ranges[0].start')
499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
PMD_MASK)
| ^
arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
506 | .start = MODULES_VADDR,
| ^
I have not done any analysis on the issue so far, I hope
you can see the problem directly. See
https://pastebin.com/raw/xVqAyakH for a .config that runs into
this problem with gcc-13.2.0.
Arnd
[PATCH] dax: fix missing-prototype warnings
From: Arnd Bergmann
dev_dax_probe declaration for this function was removed with the only
caller outside of device.c. Mark it static to avoid a W=1
warning:
drivers/dax/device.c:399:5: error: no previous prototype for 'dev_dax_probe'
Similarly, run_dax() causes a warning, but this one is because the
declaration needs to be included:
drivers/dax/super.c:337:6: error: no previous prototype for 'run_dax'
Fixes: 83762cb5c7c4 ("dax: Kill DEV_DAX_PMEM_COMPAT")
Signed-off-by: Arnd Bergmann
---
drivers/dax/device.c | 3 +--
drivers/dax/super.c | 1 +
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index af9930c03c9c..30665a3ff6ea 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -396,7 +396,7 @@ static void dev_dax_kill(void *dev_dax)
kill_dev_dax(dev_dax);
}
-int dev_dax_probe(struct dev_dax *dev_dax)
+static int dev_dax_probe(struct dev_dax *dev_dax)
{
struct dax_device *dax_dev = dev_dax->dax_dev;
struct device *dev = _dax->dev;
@@ -471,7 +471,6 @@ int dev_dax_probe(struct dev_dax *dev_dax)
run_dax(dax_dev);
return devm_add_action_or_reset(dev, dev_dax_kill, dev_dax);
}
-EXPORT_SYMBOL_GPL(dev_dax_probe);
static struct dax_device_driver device_dax_driver = {
.probe = dev_dax_probe,
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index c4c4728a36e4..8c05dae19bfe 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -14,6 +14,7 @@
#include
#include
#include "dax-private.h"
+#include "bus.h"
/**
* struct dax_device - anchor object for dax services
--
2.39.2
[PATCH 3/3] libnvdimm: mark 'security_show' static again
From: Arnd Bergmann
The security_show() function was made global and __weak at some
point to allow overriding it. The override was removed later, but
it remains global, which causes a warning about the missing
declaration:
drivers/nvdimm/dimm_devs.c:352:9: error: no previous prototype for
'security_show'
This is also not an appropriate name for a global symbol in the
kernel, so just make it static again.
Fixes: 15a8348707ff ("libnvdimm: Introduce CONFIG_NVDIMM_SECURITY_TEST flag")
Signed-off-by: Arnd Bergmann
---
drivers/nvdimm/dimm_devs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 957f7c3d17ba..10c3cb6a574a 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -349,7 +349,7 @@ static ssize_t available_slots_show(struct device *dev,
}
static DEVICE_ATTR_RO(available_slots);
-ssize_t security_show(struct device *dev,
+static ssize_t security_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct nvdimm *nvdimm = to_nvdimm(dev);
--
2.39.2
[PATCH 2/3] testing: nvdimm: add missing prototypes for wrapped functions
From: Arnd Bergmann The nvdimm test wraps a number of API functions, but these functions don't have a prototype in a header because they are all called by a different name: drivers/nvdimm/../../tools/testing/nvdimm/test/iomap.c:74:15: error: no previous prototype for '__wrap_devm_ioremap' [-Werror=missing-prototypes] 74 | void __iomem *__wrap_devm_ioremap(struct device *dev, | ^~~ drivers/nvdimm/../../tools/testing/nvdimm/test/iomap.c:86:7: error: no previous prototype for '__wrap_devm_memremap' [-Werror=missing-prototypes] 86 | void *__wrap_devm_memremap(struct device *dev, resource_size_t offset, | ^~~~ ... Add prototypes to avoid the warning. Signed-off-by: Arnd Bergmann --- tools/testing/nvdimm/test/nfit_test.h | 29 +++ 1 file changed, 29 insertions(+) diff --git a/tools/testing/nvdimm/test/nfit_test.h b/tools/testing/nvdimm/test/nfit_test.h index b5f7a996c4d0..b00583d1eace 100644 --- a/tools/testing/nvdimm/test/nfit_test.h +++ b/tools/testing/nvdimm/test/nfit_test.h @@ -207,7 +207,36 @@ typedef struct nfit_test_resource *(*nfit_test_lookup_fn)(resource_size_t); typedef union acpi_object *(*nfit_test_evaluate_dsm_fn)(acpi_handle handle, const guid_t *guid, u64 rev, u64 func, union acpi_object *argv4); +void __iomem *__wrap_devm_ioremap(struct device *dev, + resource_size_t offset, unsigned long size); +void *__wrap_devm_memremap(struct device *dev, resource_size_t offset, + size_t size, unsigned long flags); +void *__wrap_devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap); +pfn_t __wrap_phys_to_pfn_t(phys_addr_t addr, unsigned long flags); +void *__wrap_memremap(resource_size_t offset, size_t size, + unsigned long flags); +void __wrap_devm_memunmap(struct device *dev, void *addr); +void __iomem *__wrap_ioremap(resource_size_t offset, unsigned long size); +void __iomem *__wrap_ioremap_wc(resource_size_t offset, unsigned long size); void __wrap_iounmap(volatile void __iomem *addr); +void __wrap_memunmap(void *addr); +struct resource *__wrap___request_region(struct resource *parent, + resource_size_t start, resource_size_t n, const char *name, + int flags); +int __wrap_insert_resource(struct resource *parent, struct resource *res); +int __wrap_remove_resource(struct resource *res); +struct resource *__wrap___devm_request_region(struct device *dev, + struct resource *parent, resource_size_t start, + resource_size_t n, const char *name); +void __wrap___release_region(struct resource *parent, resource_size_t start, + resource_size_t n); +void __wrap___devm_release_region(struct device *dev, struct resource *parent, + resource_size_t start, resource_size_t n); +acpi_status __wrap_acpi_evaluate_object(acpi_handle handle, acpi_string path, + struct acpi_object_list *p, struct acpi_buffer *buf); +union acpi_object * __wrap_acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, + u64 rev, u64 func, union acpi_object *argv4); + void nfit_test_setup(nfit_test_lookup_fn lookup, nfit_test_evaluate_dsm_fn evaluate); void nfit_test_teardown(void); -- 2.39.2
[PATCH 1/3] acpi: nfit: add declaration in a local header
From: Arnd Bergmann The nfit_intel_shutdown_status() function has a __weak defintion in nfit.c and an override in acpi_nfit_test.c for testing purposes. This works without an extern declaration, but causes a W=1 build warning: drivers/acpi/nfit/core.c:1717:13: error: no previous prototype for 'nfit_intel_shutdown_status' [-Werror=missing-prototypes] Add a declaration in a header that gets included from both sides to shut up the warning and ensure that the prototypes actually match. Signed-off-by: Arnd Bergmann --- drivers/acpi/nfit/nfit.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h index 6023ad61831a..573bc0de2990 100644 --- a/drivers/acpi/nfit/nfit.h +++ b/drivers/acpi/nfit/nfit.h @@ -347,4 +347,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, void acpi_nfit_desc_init(struct acpi_nfit_desc *acpi_desc, struct device *dev); bool intel_fwa_supported(struct nvdimm_bus *nvdimm_bus); extern struct device_attribute dev_attr_firmware_activate_noidle; +void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem); + #endif /* __NFIT_H__ */ -- 2.39.2
[PATCH] dax: clx: add CXL_REGION dependency
From: Arnd Bergmann
There is already a dependency on CXL_REGION, which depends on CXL_BUS,
but since CXL_REGION is a 'bool' symbol, it's possible to configure
DAX as built-in even though CXL itself is a loadable module:
x86_64-linux-ld: drivers/dax/cxl.o: in function `cxl_dax_region_probe':
cxl.c:(.text+0xb): undefined reference to `to_cxl_dax_region'
x86_64-linux-ld: drivers/dax/cxl.o: in function `cxl_dax_region_driver_init':
cxl.c:(.init.text+0x10): undefined reference to `__cxl_driver_register'
x86_64-linux-ld: drivers/dax/cxl.o: in function `cxl_dax_region_driver_exit':
cxl.c:(.exit.text+0x9): undefined reference to `cxl_driver_unregister'
Prevent this with another depndency on the tristate symbol.
Fixes: 09d09e04d2fc ("cxl/dax: Create dax devices for CXL RAM regions")
Signed-off-by: Arnd Bergmann
---
drivers/dax/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
index 8c67a24592e3..a88744244149 100644
--- a/drivers/dax/Kconfig
+++ b/drivers/dax/Kconfig
@@ -46,7 +46,7 @@ config DEV_DAX_HMEM
config DEV_DAX_CXL
tristate "CXL DAX: direct access to CXL RAM regions"
- depends on CXL_REGION && DEV_DAX
+ depends on CXL_BUS && CXL_REGION && DEV_DAX
default CXL_REGION && DEV_DAX
help
CXL RAM regions are either mapped by platform-firmware
--
2.39.1
Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
On Tue, Apr 20, 2021 at 11:14 PM Vineet Gupta
wrote:
> On 4/20/21 12:07 AM, Arnd Bergmann wrote:
> >
> > which means that half the 32-bit architectures do this. This may
> > cause more problems when arc and/or microblaze want to support
> > 64-bit kernels and compat mode in the future on their latest hardware,
> > as that means duplicating the x86 specific hacks we have for compat.
> >
> > What is alignof(u64) on 64-bit arc?
>
> $ echo 'int a = __alignof__(long long);' | arc64-linux-gnu-gcc -xc -
> -Wall -S -o - | grep -A1 a: | tail -n 1 | cut -f 3
> 8
Ok, good.
> Yeah ARCv2 alignment of 4 for 64-bit data was a bit of surprise finding
> for me as well. When 64-bit load/stores were initially targeted by the
> internal Metaware compiler (llvm based) they decided to keep alignment
> to 4 still (granted hardware allowed this) and then gcc guys decided to
> follow the same ABI. I only found this by accident :-)
>
> Can you point me to some specifics on the compat issue. For better of
> worse, arc64 does''t have a compat 32-bit mode, so everything is
> 64-on-64 or 32-on-32 (ARC32 flavor of ARCv3)
In that case, there should be no problem for you.
The main issue is with system calls and ioctls that contain a misaligned
struct member like
struct s {
u32 a;
u64 b;
};
Passing this structure by reference from a 32-bit user space application
to a 64-bit kernel with different alignment constraints means that the
kernel has to convert the structure layout. See
compat_ioctl_preallocate() in fs/ioctl.c for one such example.
Arnd
Re: [PATCH 2/3] nios2: Cleanup deprecated function strlen_user
On Tue, Apr 20, 2021 at 3:37 PM wrote: > > From: Guo Ren > > $ grep strlen_user * -r > arch/csky/include/asm/uaccess.h:#define strlen_user(str) strnlen_user(str, > 32767) > arch/csky/lib/usercopy.c: * strlen_user: - Get the size of a string in user > space. > arch/ia64/lib/strlen.S: // Please note that in the case of strlen() as > opposed to strlen_user() > arch/mips/lib/strnlen_user.S: * make strlen_user and strnlen_user access the > first few KSEG0 > arch/nds32/include/asm/uaccess.h:extern __must_check long strlen_user(const > char __user * str); > arch/nios2/include/asm/uaccess.h:extern __must_check long strlen_user(const > char __user *str); > arch/riscv/include/asm/uaccess.h:extern long __must_check strlen_user(const > char __user *str); > kernel/trace/trace_probe_tmpl.h:static nokprobe_inline int > fetch_store_strlen_user(unsigned long addr); > kernel/trace/trace_probe_tmpl.h:ret += > fetch_store_strlen_user(val + code->offset); > kernel/trace/trace_uprobe.c:fetch_store_strlen_user(unsigned long addr) > kernel/trace/trace_kprobe.c:fetch_store_strlen_user(unsigned long addr) > kernel/trace/trace_kprobe.c:return fetch_store_strlen_user(addr); I would suggest using "grep strlen_user * -rw", to let the whole-word match filter out the irrelevant ones for the changelog. > See grep result, nobody uses it. > > Signed-off-by: Guo Ren > Cc: Arnd Bergmann All three patches Reviewed-by: Arnd Bergmann Do you want me to pick them up in the asm-generic tree?
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
On Tue, Apr 20, 2021 at 1:44 AM Dominique MARTINET wrote: > Arnd Bergmann wrote on Mon, Apr 19, 2021 at 02:16:36PM +0200: > > For built-in drivers, load order depends on the initcall level and > > link order (how things are lined listed in the Makefile hierarchy). > > > > For loadable modules, this is up to user space in the end. > > > > Which of the drivers in this scenario are loadable modules? > > All the drivers involved in my case are built-in (nvmem, soc and final > soc_device_match consumer e.g. caam_jr that crashes the kernel if soc is > not identified properly). Ok, in that case you may have a chance to just adapt the initcall levels. This is somewhat fragile if someone else already relies on a particular order, but it's an easy one-line change to change a driver e.g. from module_init() or device_initcall() to arch_initcall(). > I frankly don't like the idea of moving nvmem/ above soc/ in > drivers/Makefile as a "solution" to this (especially as there is one > that seems to care about what soc they run on...), so I'll have a look > at links first, hopefully that will work out. Right, that would be way more fragile. I think the main problem in this case is the caam driver that really should not look into the particular SoC type or even machine compatible string. This is something we can do as a last resort for compatibility with busted devicetree files, but it appears that this driver does it as the primary method for identifying different hardware revisions. I would suggest fixing the binding so that each SoC that includes one of these devices has a soc specific compatible string associated with the device that the driver can use as the primary way of identifying the device. We probably need to keep the old logic around for old dtb files, but there can at least be a comment next to that table that discourages people from adding more entries there. Arnd
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
On Tue, Apr 20, 2021 at 1:44 AM Dominique MARTINET wrote: > Arnd Bergmann wrote on Mon, Apr 19, 2021 at 02:16:36PM +0200: > > For built-in drivers, load order depends on the initcall level and > > link order (how things are lined listed in the Makefile hierarchy). > > > > For loadable modules, this is up to user space in the end. > > > > Which of the drivers in this scenario are loadable modules? > > All the drivers involved in my case are built-in (nvmem, soc and final > soc_device_match consumer e.g. caam_jr that crashes the kernel if soc is > not identified properly). Ok, in that case you may have a chance to just adapt the initcall levels. This is somewhat fragile if someone else already relies on a particular order, but it's an easy one-line change to change a driver e.g. from module_init() or device_initcall() to arch_initcall(). > I frankly don't like the idea of moving nvmem/ above soc/ in > drivers/Makefile as a "solution" to this (especially as there is one > that seems to care about what soc they run on...), so I'll have a look > at links first, hopefully that will work out. Right, that would be way more fragile. I think the main problem in this case is the caam driver that really should not look into the particular SoC type or even machine compatible string. This is something we can do as a last resort for compatibility with busted devicetree files, but it appears that this driver does it as the primary method for identifying different hardware revisions. I would suggest fixing the binding so that each SoC that includes one of these devices has a soc specific compatible string associated with the device that the driver can use as the primary way of identifying the device. We probably need to keep the old logic around for old dtb files, but there can at least be a comment next to that table that discourages people from adding more entries there. Arnd
Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
On Tue, Apr 20, 2021 at 5:10 AM Matthew Wilcox wrote:
>
> On Tue, Apr 20, 2021 at 02:48:17AM +, Vineet Gupta wrote:
> > > 32-bit architectures which expect 8-byte alignment for 8-byte integers
> > > and need 64-bit DMA addresses (arc, arm, mips, ppc) had their struct
> > > page inadvertently expanded in 2019.
> >
> > FWIW, ARC doesn't require 8 byte alignment for 8 byte integers. This is
> > only needed for 8-byte atomics due to the requirements of LLOCKD/SCOND
> > instructions.
>
> Ah, like x86? OK, great, I'll drop your arch from the list of
> affected. Thanks!
I mistakenly assumed that i386 and m68k were the only supported
architectures with 32-bit alignment on u64. I checked it now and found
$ for i in /home/arnd/cross/x86_64/gcc-10.1.0-nolibc/*/bin/*-gcc ; do
echo `echo 'int a = __alignof__(long long);' | $i -xc - -Wall -S -o- |
grep -A1 a: | tail -n 1 | cut -f 3 -d\ `
${i#/home/arnd/cross/x86_64/gcc-10.1.0-nolibc/*/bin/} ; done
8 aarch64-linux-gcc
8 alpha-linux-gcc
4 arc-linux-gcc
8 arm-linux-gnueabi-gcc
8 c6x-elf-gcc
4 csky-linux-gcc
4 h8300-linux-gcc
8 hppa-linux-gcc
8 hppa64-linux-gcc
8 i386-linux-gcc
8 ia64-linux-gcc
2 m68k-linux-gcc
4 microblaze-linux-gcc
8 mips-linux-gcc
8 mips64-linux-gcc
8 nds32le-linux-gcc
4 nios2-linux-gcc
4 or1k-linux-gcc
8 powerpc-linux-gcc
8 powerpc64-linux-gcc
8 riscv32-linux-gcc
8 riscv64-linux-gcc
8 s390-linux-gcc
4 sh2-linux-gcc
4 sh4-linux-gcc
8 sparc-linux-gcc
8 sparc64-linux-gcc
8 x86_64-linux-gcc
8 xtensa-linux-gcc
which means that half the 32-bit architectures do this. This may
cause more problems when arc and/or microblaze want to support
64-bit kernels and compat mode in the future on their latest hardware,
as that means duplicating the x86 specific hacks we have for compat.
What is alignof(u64) on 64-bit arc?
Arnd
Re: [PATCH][next] wlcore: Fix buffer overrun by snprintf due to incorrect buffer size Content-Type: text/plain; charset="utf-8"
On Mon, Apr 19, 2021 at 4:01 PM Colin King wrote:
>
> From: Colin Ian King
>
> The size of the buffer than can be written to is currently incorrect, it is
> always the size of the entire buffer even though the snprintf is writing
> as position pos into the buffer. Fix this by setting the buffer size to be
> the number of bytes left in the buffer, namely sizeof(buf) - pos.
>
> Addresses-Coverity: ("Out-of-bounds access")
> Fixes: 7b0e2c4f6be3 ("wlcore: fix overlapping snprintf arguments in debugfs")
> Signed-off-by: Colin Ian King
Acked-by: Arnd Bergmann
Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap
On Thu, Apr 15, 2021 at 7:29 AM Dan Carpenter wrote:
>
> On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> > On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > > ---
> > > drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > index c95ae4d6a3b6b..cc14f00947781 100644
> > > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > > /* parsing WPA/WPA2 IE */
> > > {
> > > u8 *buf;
> > > - u8 wpa_ie[255], rsn_ie[255];
> > > + u8 *wpa_ie, *rsn_ie;
> > > u16 wpa_len = 0, rsn_len = 0;
> > > u8 *p;
> > >
> > > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > > if (!buf)
> > > return start;
>
> Arnd, added this return... I don't understand why we aren't returning
> -ENOMEM here.
I don't remember my patch, but I see that the function returns a pointer
that gets dereferenced afterwards. Changing this is probably a good idea
(the caller does return an error code), but it requires a few extra changes.
If there is a larger rework, I'd suggest using a single kzalloc to get all
three arrays at once to get simpler error handling.
Arnd
Re: [PATCH 12/13] ARM: dts: stm32: fix DSI port node on STM32MP15
On Thu, Apr 15, 2021 at 2:23 PM Alexandre TORGUE
wrote:
> On 4/15/21 12:43 PM, Ahmad Fatoum wrote:
> > On 15.04.21 12:10, Alexandre Torgue wrote:
> >> Running "make dtbs_check W=1", some warnings are reported concerning
> >> DSI. This patch reorder DSI nodes to avoid:
> >>
> >> soc/dsi@5a00: unnecessary #address-cells/#size-cells without
> >> "ranges" or child "reg" property
> >
> > This reverts parts of commit 9c32f980d9 ("ARM: dts: stm32: preset
> > stm32mp15x video #address- and #size-cells"):
> >
> > The cell count for address and size is defined by the binding and not
> > something a board would change. Avoid each board adding this
> > boilerplate by having the cell size specification in the SoC DTSI.
> >
> >
> > The DSI can have child nodes with a unit address (e.g. a panel) and ones
> > without (ports { } container). ports is described in the dtsi, panels are
> > described in the dts if available.
> >
> > Apparently, the checker is fine with
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > };
> >
> > I think my rationale for the patch above was sound, so I think the checker
> > taking offense at the DSI cells here should be considered a false positive.
>
> If it's a "false positive" warning then we need to find a way to not
> print it out. Else, it'll be difficult to distinguish which warnings are
> "normal" and which are not. This question could also be applied to patch[3].
>
> Arnd, Rob what is your feeling about this case ?
I don't have a strong opinion on this either way, but I would just
not apply this one for 5.13 in this case. Rob, Alexandre, please
let me know if I should apply the other patches before the
merge window, I usually don't mind taking bugfixes late before the
merge window, but I still want some level of confidence that they
are actually correct.
Ahmad, if you feel strongly about this particular issue, would you like
to suggest a patch for the checker?
Arnd
Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
On Mon, Apr 19, 2021 at 11:33 AM Dominique MARTINET
wrote:
> Geert Uytterhoeven wrote on Mon, Apr 19, 2021 at 11:03:24AM +0200:
>
> > soc_device_match() should only be used as a last resort, to identify
> > systems that cannot be identified otherwise. Typically this is used for
> > quirks, which should only be enabled on a very specific subset of
> > systems. IMHO such systems should make sure soc_device_match()
> > is available early, by registering their SoC device early.
>
> I definitely agree there, my suggestion to defer was only because I know
> of no other way to influence the ordering of drivers loading reliably
> and gave up on soc being init'd early.
In some cases, you can use the device_link infrastructure to deal
with dependencies between devices. Not sure if this would help
in your case, but have a look at device_link_add() etc in drivers/base/core.c
> In this particular case the problem is that since 7d981405d0fd ("soc:
> imx8m: change to use platform driver") the soc probe tries to use the
> nvmem driver for ocotp fuses for imx8m devices, which isn't ready yet.
> So soc loading gets pushed back to the end of the list because it gets
> defered and other drivers relying on soc_device_match get confused
> because they wrongly think a device doesn't match a quirk when it
> actually does.
>
> If there is a way to ensure the nvmem driver gets loaded before the soc,
> that would also solve the problem nicely, and avoid the need to mess
> with all the ~50 drivers which use it.
>
> Is there a way to control in what order drivers get loaded? Something in
> the dtb perhaps?
For built-in drivers, load order depends on the initcall level and
link order (how things are lined listed in the Makefile hierarchy).
For loadable modules, this is up to user space in the end.
Which of the drivers in this scenario are loadable modules?
Arnd
Re: [PATCH v2 (RESEND) 2/2] riscv: atomic: Using ARCH_ATOMIC in asm/atomic.h
On Sat, Apr 17, 2021 at 6:45 AM wrote: > +#define arch_atomic_read(v)__READ_ONCE((v)->counter) > +#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), > (i)) > +#define ATOMIC64_INIT ATOMIC_INIT > +#define arch_atomic64_read arch_atomic_read > +#define arch_atomic64_set arch_atomic_set > #endif I think it's a bit confusing to define arch_atomic64_read() etc in terms of arch_atomic_read(), given that they operate on different types. IMHO the clearest would be to define both in terms of the open-coded version you have for the 32-bit atomics. Also, given that all three architectures (x86, arm64, riscv) use the same definitions for the six macros above, maybe those can just get moved into a common file with a possible override? x86 uses an inline function here instead of the macro. This would also be my preference, but it may add complexity to avoid circular header dependencies. The rest of this patch looks good to me. Arnd
Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
On Sat, Apr 17, 2021 at 3:58 PM Matthew Wilcox wrote: > I wouldn't like to make that assumption. I've come across IOMMUs (maybe > on parisc? powerpc?) that like to encode fun information in the top > few bits. So we could get it down to 52 bits, but I don't think we can > get all the way down to 32 bits. Also, we need to keep the bottom bit > clear for PageTail, so that further constrains us. I'd be surprised to find such an IOMMU on a 32-bit machine, given that the main reason for using an IOMMU on these is to avoid the 32-bit address limit in DMA masters. I see that parisc32 does not enable 64-bit dma_addr_t, while powerpc32 does not support any IOMMU, so it wouldn't be either of those two. I do remember some powerpc systems that encode additional flags (transaction ordering, caching, ...) into the high bits of the physical address in the IOTLB, but not the virtual address used for looking them up. > Anyway, I like the "two unsigned longs" approach I posted yesterday, > but thanks for the suggestion. Ok, fair enough. As long as there are enough bits in this branch of 'struct page', I suppose it is the safe choice. Arnd
Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
On Fri, Apr 16, 2021 at 5:27 PM Matthew Wilcox wrote:
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index b5b195305346..db7c7020746a 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -198,7 +198,17 @@ static inline void page_pool_recycle_direct(struct
> page_pool *pool,
>
> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> {
> - return page->dma_addr;
> + dma_addr_t ret = page->dma_addr[0];
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + ret |= (dma_addr_t)page->dma_addr[1] << 32;
> + return ret;
> +}
Have you considered using a PFN type address here? I suspect you
can prove that shifting the DMA address by PAGE_BITS would
make it fit into an 'unsigned long' on all 32-bit architectures with
64-bit dma_addr_t. This requires that page->dma_addr to be
page aligned, as well as fit into 44 bits. I recently went through the
maximum address space per architecture to define a
MAX_POSSIBLE_PHYSMEM_BITS, and none of them have more than
40 here, presumably the same is true for dma address space.
Arnd
Re: Bogus struct page layout on 32-bit
On Fri, Apr 16, 2021 at 11:27 AM 'Grygorii Strashko' via Clang Built
Linux wrote:
> On 10/04/2021 11:52, Ilias Apalodimas wrote:
> > +CC Grygorii for the cpsw part as Ivan's email is not valid anymore
> The TI platforms am3/4/5 (cpsw) and Keystone 2 (netcp) can do only 32bit DMA
> even in case of LPAE (dma-ranges are used).
> Originally, as I remember, CONFIG_ARCH_DMA_ADDR_T_64BIT has not been selected
> for the LPAE case
> on TI platforms and the fact that it became set is the result of
> multi-paltform/allXXXconfig/DMA
> optimizations and unification.
> (just checked - not set in 4.14)
>
> Probable commit 4965a68780c5 ("arch: define the ARCH_DMA_ADDR_T_64BIT config
> symbol in lib/Kconfig").
I completely missed this change in the past, and I don't really agree
with it either.
Most 32-bit Arm platforms are in fact limited to 32-bit DMA, even when they have
MMIO or RAM areas above the 4GB boundary that require LPAE.
> The TI drivers have been updated, finally to accept ARCH_DMA_ADDR_T_64BIT=y
> by using
> things like (__force u32) for example.
>
> Honestly, I've done sanity check of CPSW with LPAE=y
> (ARCH_DMA_ADDR_T_64BIT=y) very long time ago.
This is of course a good idea, drivers should work with any
combination of 32-bit
or 64-bit phys_addr_t and dma_addr_t.
Arnd
Re: [PATCH net-next] net: Space: remove hp100 probe
On Wed, Apr 14, 2021, 00:42 Stephen Hemminger
wrote:
>
> On Tue, 13 Apr 2021 16:16:17 +0200 Arnd Bergmann wrote:
>
> > */
> > static struct devprobe2 isa_probes[] __initdata = {
> > -#if defined(CONFIG_HP100) && defined(CONFIG_ISA) /* ISA, EISA */
> > - {hp100_probe, 0},
> > -#endif
> > #ifdef CONFIG_3C515
> > {tc515_probe, 0},
> > #endif
>
> Thanks, do we even need to have the static initialization anymore?
I actually did some more cleanups after I sent the above patch when
I found out that this code still exists. It turned out that above half of
the static initializations are completely pointless because the
drivers never rely on the netdev= command line arguments and
can simply be changed to always using module_init() instead of
relying on net_olddevs_init() for the built-in case.
The remaining ones are all ISA drivers: 3c515, Ultra, WD80x3,
NE2000, Lance, SMC9194, CS89x0, NI65 and COPS.
With my cleanups, I move the netdev_boot_setup infrastructure
into drivers/net/Space.c and only compile it when at least one of
these eight drivers is enabled.
All these drivers also support being built as loadable modules, but
in that configuration they only support a single device (back in the
day you could copy the module and just load it twice to support
more than one instance, not sure we still want to support that).
None of these drivers have a maintainer listed, but I suppose
there are still some PC/104 machines with NE2000 network
cards that could theoretically run a modern kernel.
Arnd
[PATCH net-next] net: Space: remove hp100 probe
From: Arnd Bergmann
The driver was removed last year, but the static initialization got left
behind by accident.
Fixes: a10079c66290 ("staging: remove hp100 driver")
Signed-off-by: Arnd Bergmann
---
drivers/net/Space.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/Space.c b/drivers/net/Space.c
index 7bb699d7c422..a61cc7b26a87 100644
--- a/drivers/net/Space.c
+++ b/drivers/net/Space.c
@@ -59,9 +59,6 @@ static int __init probe_list2(int unit, struct devprobe2 *p,
int autoprobe)
* look for EISA/PCI cards in addition to ISA cards).
*/
static struct devprobe2 isa_probes[] __initdata = {
-#if defined(CONFIG_HP100) && defined(CONFIG_ISA) /* ISA, EISA */
- {hp100_probe, 0},
-#endif
#ifdef CONFIG_3C515
{tc515_probe, 0},
#endif
--
2.29.2
Re: [PATCH] asm-generic/io.h: Silence -Wnull-pointer-arithmetic warning on PCI_IOBASE
On Tue, Apr 13, 2021 at 3:06 PM David Laight wrote: > > From: Arnd Bergmann > > Sent: 13 April 2021 13:58 > ... > > The remaining ones (csky, m68k, sparc32) need to be inspected > > manually to see if they currently support PCI I/O space but in > > fact use address zero as the base (with large resources) or they > > should also turn the operations into a NOP. > > I'd expect sparc32 to use an ASI to access PCI IO space. > I can't quite remember whether IO space was supported at all. I see this bit in arch/sparc/kernel/leon_pci.c * PCI Memory and Prefetchable Memory is direct-mapped. However I/O Space is * accessed through a Window which is translated to low 64KB in PCI space, the * first 4KB is not used so 60KB is available. ... pci_add_resource_offset(, >io_space, info->io_space.start - 0x1000); which means that there is I/O space, which gets accessed through whichever method readb() uses. Having the offset equal to the resource means that the '(void *)0' start is correct. As this leaves only two others, I checked those as well: csky does not actually have a PCI host bridge driver at the moment, so we don't care about breaking port access on it it, and I would suggest leaving I/O port access disabled. (Added Guo Ren to Cc for confirmation). m68k only supports PCI on coldfire M54xx, and this variant does set a PCI_IOBASE after all. The normal MMU based m68k have no PCI and do define their out inb/outb/..., so nothing changes for them. To summarize: only sparc32 needs to set PCI_IOBASE to zero, everyone else should just WARN_ONCE() or return 0xff/0x/0x. Arnd
Re: [PATCH] asm-generic/io.h: Silence -Wnull-pointer-arithmetic warning on PCI_IOBASE
On Tue, Apr 13, 2021 at 2:38 PM Niklas Schnelle wrote:
> On Tue, 2021-04-13 at 14:26 +0200, Arnd Bergmann wrote:
> > I think the real underlying problem is that '0' is a particularly bad
> > default value,
> > we should not have used this one in asm-generic, or maybe have left it as
> > mandatory to be defined by an architecture to a sane value. Note that
> > architectures that don't actually support I/O ports will cause a NULL
> > pointer dereference when loading a legacy driver, which is exactly what
> > clang
> > is warning about here. Architectures that to support I/O ports in PCI
> > typically
> > map them at a fixed location in the virtual address space and should put
> > that
> > location here, rather than adding the pointer value to the port resources.
> >
> > What we might do instead here would be move the definition into those
> > architectures that actually define the base to be at address zero, with a
> > comment explaining why this is a bad location, and then changing the
> > inb/outb style helpers to either an empty function or a WARN_ONCE().
> >
> > On which architectures do you see the problem? How is the I/O port
> > actually mapped there?
>
> I'm seeing this on s390 which indeed has no I/O port support at all.
> I'm not sure how many others there are but I feel like us having to
> define these functions as empty is also kind of awkward. Maybe we could
> put them into the asm-generic/io.h for the case that PCI_IOBASE is not
> defined? Then at least every platform not supporting I/O ports would
> share them.
Yes, I meant doing this in the asm-generic version, something like
#if !defined(inb) && !defined(_inb)
#define _inb _inb
static inline u8 _inb(unsigned long addr)
{
#ifdef PCI_IOBASE
u8 val;
__io_pbr();
val = __raw_readb(PCI_IOBASE + addr);
__io_par(val);
return val;
#else
WARN_ONCE(1, "No I/O port support");
return 0xff;
#endif
}
#endif
And follow up with a separate patch that moves the
"#define PCI_IOBASE ((void __iomem *)0)" into the architectures
that would currently see it, i.e. those that include asm-generic/io.h
but define neither inb/_inb nor PCI_IOBASE:
$ git grep -l asm-generic/io.h | xargs grep -wL inb | xargs grep -L PCI_IOBASE
arch/arc/include/asm/io.h
arch/csky/include/asm/io.h
arch/h8300/include/asm/io.h
arch/m68k/include/asm/io.h
arch/nds32/include/asm/io.h
arch/nios2/include/asm/io.h
arch/openrisc/include/asm/io.h
arch/s390/include/asm/io.h
arch/sparc/include/asm/io_32.h
arch/um/include/asm/io.h
Out of these, I see that arc, h8300, nds32, nios2, openrisc, and um
never set CONFIG_HAVE_PCI, so these would all be better off
leaving PCI_IOBASE undefined and getting the WARN_ONCE.
The remaining ones (csky, m68k, sparc32) need to be inspected
manually to see if they currently support PCI I/O space but in
fact use address zero as the base (with large resources) or they
should also turn the operations into a NOP.
Arnd
Re: [PATCH] asm-generic/io.h: Silence -Wnull-pointer-arithmetic warning on PCI_IOBASE
On Tue, Apr 13, 2021 at 1:54 PM Niklas Schnelle wrote: > > When PCI_IOBASE is not defined, it is set to 0 such that it is ignored > in calls to the readX/writeX primitives. While mathematically obvious > this triggers clang's -Wnull-pointer-arithmetic warning. Indeed, this is an annoying warning. > An additional complication is that PCI_IOBASE is explicitly typed as > "void __iomem *" which causes the type conversion that converts the > "unsigned long" port/addr parameters to the appropriate pointer type. > As non pointer types are used by drivers at the callsite since these are > dealing with I/O port numbers, changing the parameter type would cause > further warnings in drivers. Instead use "uintptr_t" for PCI_IOBASE > 0 and explicitly cast to "void __iomem *" when calling readX/writeX. > > Signed-off-by: Niklas Schnelle > --- > include/asm-generic/io.h | 26 +- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h > index c6af40ce03be..8eb00bdef7ad 100644 > --- a/include/asm-generic/io.h > +++ b/include/asm-generic/io.h > @@ -441,7 +441,7 @@ static inline void writesq(volatile void __iomem *addr, > const void *buffer, > #endif /* CONFIG_64BIT */ > > #ifndef PCI_IOBASE > -#define PCI_IOBASE ((void __iomem *)0) > +#define PCI_IOBASE ((uintptr_t)0) > #endif > > #ifndef IO_SPACE_LIMIT Your patch looks wrong in the way it changes the type of one of the definitions, but not the type of any of the architecture specific ones. It's also awkward since 'void __iomem *' is really the correct type, while 'uintptr_t' is not! I think the real underlying problem is that '0' is a particularly bad default value, we should not have used this one in asm-generic, or maybe have left it as mandatory to be defined by an architecture to a sane value. Note that architectures that don't actually support I/O ports will cause a NULL pointer dereference when loading a legacy driver, which is exactly what clang is warning about here. Architectures that to support I/O ports in PCI typically map them at a fixed location in the virtual address space and should put that location here, rather than adding the pointer value to the port resources. What we might do instead here would be move the definition into those architectures that actually define the base to be at address zero, with a comment explaining why this is a bad location, and then changing the inb/outb style helpers to either an empty function or a WARN_ONCE(). On which architectures do you see the problem? How is the I/O port actually mapped there? Arnd
Re: [PATCH v2 16/21] ipmi: kcs_bmc: Add a "raw" character device interface
On Tue, Apr 13, 2021 at 1:45 AM Andrew Jeffery wrote: > On Mon, 12 Apr 2021, at 18:18, Arnd Bergmann wrote: > > On Mon, Apr 12, 2021 at 3:33 AM Andrew Jeffery wrote: > > > On Fri, 9 Apr 2021, at 17:25, Arnd Bergmann wrote: > > > > On Fri, Mar 19, 2021 at 7:31 AM Andrew Jeffery wrote: > > > > > > > > > > The existing IPMI chardev encodes IPMI behaviours as the name > > > > > suggests. > > > > > However, KCS devices are useful beyond IPMI (or keyboards), as they > > > > > provide a means to generate IRQs and exchange arbitrary data between a > > > > > BMC and its host system. > > > > > > > > I only noticed the series after Joel asked about the DT changes on the > > > > arm > > > > side. One question though: > > > > > > > > How does this related to the drivers/input/serio/ framework that also > > > > talks > > > > to the keyboard controller for things that are not keyboards? > > > > > > I've taken a brief look and I feel they're somewhat closely related. > > > > > > It's plausible that we could wrangle the code so the Aspeed and Nuvoton > > > KCS drivers move under drivers/input/serio. If you squint, the i8042 > > > serio device driver has similarities with what the Aspeed and Nuvoton > > > device drivers are providing to the KCS IPMI stack. > > > > After looking some more into it, I finally understood that the two are > > rather complementary. While the drivers/char/ipmi/kcs_bmc.c > > is the other (bmc) end of drivers/char/ipmi/ipmi_kcs_sm.c, it seems > > that the proposed kcs_bmc_cdev_raw.c interface would be > > what corresponds to the other side of > > drivers/input/serio/i8042.c+userio.c. > > Right. I guess the question is should we be splitting kernel subsystems > along host/bmc lines? Doesn't feel intuitive, it's all Linux, but maybe > we can consolidate in the future if it makes sense? We actually have a number of subsystems with somewhat overlapping functionality. I brought up serio, because it has an abstraction for multiple things that communicate over the keyboard controller and I thought the problem you were trying to solve was also related to the keyboard controller. It is also one of multiple abstractions that allow you to connect a device to a uart (along with serdev and tty_ldisc, probably at least one more that you can nest above or below these). Consolidating the kcs_bmc.c interface into something that already exists would obviously be best, but it's not clear which of these that should be, that depends on the fundamental properties of the hardware interface. > > Then again, these are also on > > separate ports (0x60 for the keyboard controller, 0xca2 for the BMC > > KCS), so they would never actually talk to one another. > > Well, sort of I guess. On Power systems we don't use the keyboard > controller for IPMI or keyboards, so we're just kinda exploiting the > hardware for our own purposes. Can you describe in an abstract form what the hardware interface can do here and what you want from it? I wonder if it could be part of a higher-level interface such as drivers/mailbox/ instead. Arnd
Re: [PATCH][next] habanalabs/gaudi: Fix uninitialized return code rc when read size is zero
On Mon, Apr 12, 2021 at 6:11 PM Colin King wrote:
>
> From: Colin Ian King
>
> In the case where size is zero the while loop never assigns rc and the
> return value is uninitialized. Fix this by initializing rc to zero.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 639781dcab82 ("habanalabs/gaudi: add debugfs to DMA from the device")
> Signed-off-by: Colin Ian King
> ---
> drivers/misc/habanalabs/gaudi/gaudi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c
> b/drivers/misc/habanalabs/gaudi/gaudi.c
> index 8730b691ec61..b751652f80a8 100644
> --- a/drivers/misc/habanalabs/gaudi/gaudi.c
> +++ b/drivers/misc/habanalabs/gaudi/gaudi.c
> @@ -6252,7 +6252,7 @@ static int gaudi_debugfs_read_dma(struct hl_device
> *hdev, u64 addr, u32 size,
> dma_addr_t dma_addr;
> void *kernel_addr;
> bool is_eng_idle;
> - int rc, dma_id;
> + int rc = 0, dma_id;
>
> kernel_addr = hdev->asic_funcs->asic_dma_alloc_coherent(
> hdev, SZ_2M,
In general, I don't like adding initializations during the declaration as that
tends to hide warnings for the cases where a later initialization is
missing. In this case it looks correct though.
Acked-by: Arnd Bergmann
Re: [Linux-stm32] [RESEND PATCH 0/3] MAINTAINERS: update STMicroelectronics email
On Mon, Apr 12, 2021 at 12:19 PM Patrice CHOTARD wrote: > > Hi > > I think this series has been forgotten, any chance to see it merged into > v5.13 ? It's in -rc7, but it appears that my email reply went missing when I merged it. Arnd
Re: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
On Mon, Apr 12, 2021 at 12:54 PM David Laight wrote: > From: David Laight > Sent: 12 April 2021 10:37 > ... > > I'm guessing that compat_pid_t is 16 bits? > > So the native 32bit version has an unnamed 2 byte structure pad. > > The 'packed' removes this pad from the compat structure. > > > > AFAICT (apart from mips) the __ARCH_COMPAT_FLOCK_PAD is just > > adding an explicit pad for the implicit pad the compiler > > would generate because compat_pid_t is 16 bits. > > I've just looked at the header. > compat_pid_t is 32 bits. > So Linux must have gained 32bit pids at some earlier time. > (Historically Unix pids were 16 bit - even on 32bit systems.) > > Which makes the explicit pad in 'sparc' rather 'interesting'. I saw it was there since the sparc kernel support got merged in linux-1.3, possibly copied from an older sunos version. > oh - compat_loff_t is only used in a couple of other places. > neither care in any way about the alignment. > (Provided get_user() doesn't fault on a 8n+4 aligned address.) Ah right, I also see that after this series it's only used in to other places: compat_resume_swap_area, which could also lose the __packed annotation, and in the declaration of compat_sys_sendfile64, where it makes no difference. Arnd
Re: consolidate the flock uapi definitions
On Mon, Apr 12, 2021 at 12:22 PM David Laight wrote: > > From: Arnd Bergmann > > Sent: 12 April 2021 11:04 > > > > On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig wrote: > > > > > > Hi all, > > > > > > currently we deal with the slight differents in the various architecture > > > variants of the flock and flock64 stuctures in a very cruft way. This > > > series switches to just use small arch hooks and define the rest in > > > asm-generic and linux/compat.h instead. > > > > Nice cleanup. I can merge it through the asm-generic tree if you like, > > though it's a little late just ahead of the merge window. > > > > I would not want to change the compat_loff_t definition to compat_s64 > > to avoid the padding at this time, though that might be a useful cleanup > > for a future cycle. > > Is x86 the only architecture that has 32bit and 64bit variants where > the 32bit variant aligns 64bit items on 32bit boundaries? Yes. > ISTM that fixing compat_loff_t shouldn't have any fallout. That is my assumption as well, but I still wouldn't take the risk one week before the merge window. Arnd
Re: consolidate the flock uapi definitions
On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig wrote: > > Hi all, > > currently we deal with the slight differents in the various architecture > variants of the flock and flock64 stuctures in a very cruft way. This > series switches to just use small arch hooks and define the rest in > asm-generic and linux/compat.h instead. Nice cleanup. I can merge it through the asm-generic tree if you like, though it's a little late just ahead of the merge window. I would not want to change the compat_loff_t definition to compat_s64 to avoid the padding at this time, though that might be a useful cleanup for a future cycle. Arnd
Re: [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define
On Mon, Apr 12, 2021 at 10:55 AM Christoph Hellwig wrote: > > Signed-off-by: Christoph Hellwig The patch looks good, but I'd like to see a description for each one. How about: | The check was added when Stephen Rothwell created the file, but | no architecture ever defined it. Arnd
Re: [PATCH v2 16/21] ipmi: kcs_bmc: Add a "raw" character device interface
On Mon, Apr 12, 2021 at 3:33 AM Andrew Jeffery wrote: > On Fri, 9 Apr 2021, at 17:25, Arnd Bergmann wrote: > > On Fri, Mar 19, 2021 at 7:31 AM Andrew Jeffery wrote: > > > > > > The existing IPMI chardev encodes IPMI behaviours as the name suggests. > > > However, KCS devices are useful beyond IPMI (or keyboards), as they > > > provide a means to generate IRQs and exchange arbitrary data between a > > > BMC and its host system. > > > > I only noticed the series after Joel asked about the DT changes on the arm > > side. One question though: > > > > How does this related to the drivers/input/serio/ framework that also talks > > to the keyboard controller for things that are not keyboards? > > I've taken a brief look and I feel they're somewhat closely related. > > It's plausible that we could wrangle the code so the Aspeed and Nuvoton > KCS drivers move under drivers/input/serio. If you squint, the i8042 > serio device driver has similarities with what the Aspeed and Nuvoton > device drivers are providing to the KCS IPMI stack. After looking some more into it, I finally understood that the two are rather complementary. While the drivers/char/ipmi/kcs_bmc.c is the other (bmc) end of drivers/char/ipmi/ipmi_kcs_sm.c, it seems that the proposed kcs_bmc_cdev_raw.c interface would be what corresponds to the other side of drivers/input/serio/i8042.c+userio.c. Then again, these are also on separate ports (0x60 for the keyboard controller, 0xca2 for the BMC KCS), so they would never actually talk to one another. > Both the KCS IPMI and raw chardev I've implemented in this patch need > both read and write access to the status register (STR). serio could > potentially expose its value through serio_interrupt() using the > SERIO_OOB_DATA flag, but I haven't put any thought into it beyond this > sentence. We'd need some extra support for writing STR via the serio > API. I'm not sure that fits into the abstraction (unless we make > serio_write() take a flags argument?). > > In that vein, the serio_raw interface is close to the functionality > that the raw chardev provides in this patch, though again serio_raw > lacks userspace access to STR. Flags are ignored in the ->interrupt() > callback so all values received via ->interrupt() are exposed as data. > The result is there's no way to take care of SERIO_OOB_DATA in the > read() path. Given that, I think we'd have to expose an ioctl() to > access the STR value after taking care of SERIO_OOB_DATA in > ->interrupt(). > > I'm not sure where that lands us. Based on what I looked up, I think you can just forget about my original question. We have two separate interfaces that use an Intel 8042-style protocol, but they don't really interact. Arnd
Re: Bogus struct page layout on 32-bit
On Sat, Apr 10, 2021 at 4:44 AM Matthew Wilcox wrote:
> + dma_addr_t dma_addr __packed;
> };
> struct {/* slab, slob and slub */
> union {
>
> but I don't know if GCC is smart enough to realise that dma_addr is now
> on an 8 byte boundary and it can use a normal instruction to access it,
> or whether it'll do something daft like use byte loads to access it.
>
> We could also do:
>
> + dma_addr_t dma_addr __packed __aligned(sizeof(void
> *));
>
> and I see pahole, at least sees this correctly:
>
> struct {
> long unsigned int _page_pool_pad; /* 4 4 */
> dma_addr_t dma_addr __attribute__((__aligned__(4)));
> /* 8 8 */
> } __attribute__((__packed__)) __attribute__((__aligned__(4)));
>
> This presumably affects any 32-bit architecture with a 64-bit phys_addr_t
> / dma_addr_t. Advice, please?
I've tried out what gcc would make of this: https://godbolt.org/z/aTEbxxbG3
struct page {
short a;
struct {
short b;
long long c __attribute__((packed, aligned(2)));
} __attribute__((packed));
} __attribute__((aligned(8)));
In this structure, 'c' is clearly aligned to eight bytes, and gcc does
realize that
it is safe to use the 'ldrd' instruction for 32-bit arm, which is forbidden on
struct members with less than 4 byte alignment. However, it also complains
that passing a pointer to 'c' into a function that expects a 'long long' is not
allowed because alignof(c) is only '2' here.
(I used 'short' here because I having a 64-bit member misaligned by four
bytes wouldn't make a difference to the instructions on Arm, or any other
32-bit architecture I can think of, regardless of the ABI requirements).
Arnd
[PATCH] ext4: fix debug format string warning
From: Arnd Bergmann
Using no_printk() for jbd_debug() revealed two warnings:
fs/jbd2/recovery.c: In function 'fc_do_one_pass':
fs/jbd2/recovery.c:256:30: error: format '%d' expects a matching 'int' argument
[-Werror=format=]
256 | jbd_debug(3, "Processing fast commit blk with seq %d");
| ^~~~
fs/ext4/fast_commit.c: In function 'ext4_fc_replay_add_range':
fs/ext4/fast_commit.c:1732:30: error: format '%d' expects argument of type
'int', but argument 2 has type 'long unsigned int' [-Werror=format=]
1732 | jbd_debug(1, "Converting from %d to %d %lld",
The first one was added incorrectly, and was also missing a few newlines
in debug output, and the second one happened when the type of an
argument changed.
Reported-by: kernel test robot
Fixes: d556435156b7 ("jbd2: avoid -Wempty-body warnings")
Fixes: 6db074618969 ("ext4: use BIT() macro for BH_** state bits")
Fixes: 5b849b5f96b4 ("jbd2: fast commit recovery path")
Signed-off-by: Arnd Bergmann
---
fs/ext4/fast_commit.c | 2 +-
fs/jbd2/recovery.c| 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 6c4f19b0a556..feec2f3f13e9 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1729,7 +1729,7 @@ static int ext4_fc_replay_add_range(struct super_block
*sb,
}
/* Range is mapped and needs a state change */
- jbd_debug(1, "Converting from %d to %d %lld",
+ jbd_debug(1, "Converting from %ld to %d %lld",
map.m_flags & EXT4_MAP_UNWRITTEN,
ext4_ext_is_unwritten(ex), map.m_pblk);
ret = ext4_ext_replay_update_ex(inode, cur, map.m_len,
diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 69f18fe20923..60601c5779f1 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -245,15 +245,15 @@ static int fc_do_one_pass(journal_t *journal,
return 0;
while (next_fc_block <= journal->j_fc_last) {
- jbd_debug(3, "Fast commit replay: next block %ld",
+ jbd_debug(3, "Fast commit replay: next block %ld\n",
next_fc_block);
err = jread(, journal, next_fc_block);
if (err) {
- jbd_debug(3, "Fast commit replay: read error");
+ jbd_debug(3, "Fast commit replay: read error\n");
break;
}
- jbd_debug(3, "Processing fast commit blk with seq %d");
+ jbd_debug(3, "Processing fast commit blk with seq\n");
err = journal->j_fc_replay_callback(journal, bh, pass,
next_fc_block - journal->j_fc_first,
expected_commit_id);
--
2.29.2
Re: [PATCH] pwm: raspberrypi-poe: Fix mailbox message initialization
On Fri, Apr 9, 2021 at 11:08 AM Nicolas Saenz Julienne
wrote:
>
> For testing purposes this driver might be built on big-endian
> architectures. So make sure we take that into account when populating
> structures that are to be passed to RPi4's mailbox.
>
> Reported-by: kernel test robot
> Fixes: 79caa362eab6 ("pwm: Add Raspberry Pi Firmware based PWM bus")
> Signed-off-by: Nicolas Saenz Julienne
> ---
>
> @arndb: This was just meged into the arm-soc tree some days ago. Should I
> prepare a second PR once it's been reviewed?
Yes, that would be good. If you have no other driver patches that I should
pick up, just forward the patch (with the Ack) to s...@kernel.org and I
can just apply it.
Arnd
Re: [PATCH v2 1/3] iommu: io-pgtable: add DART pagetable format
On Fri, Apr 9, 2021 at 6:56 PM Sven Peter wrote: > On Wed, Apr 7, 2021, at 12:44, Will Deacon wrote: > > On Sun, Mar 28, 2021 at 09:40:07AM +0200, Sven Peter wrote: > > > > > + cfg->pgsize_bitmap &= SZ_16K; > > > + if (!cfg->pgsize_bitmap) > > > + return NULL; > > > > This is worrying (and again, I don't think this belongs here). How is this > > thing supposed to work if the CPU is using 4k pages? > > This SoC is just full of fun surprises! > I didn't even think about that case since I've always been using 16k pages so > far. > > I've checked again and wasn't able to find any way to configure the pagesize > of the IOMMU. There seem to be variants of this IP in older iPhones which > support a 4k pagesize but to the best of my knowledge this is hard wired > and not configurable in software. > > When booting with 4k pages I hit the BUG_ON in iova.c that ensures that the > iommu pagesize has to be <= the cpu page size. > > I see two options here and I'm not sure I like either of them: > > 1) Just don't support 4k CPU pages together with IOMMU translations and only >allow full bypass mode there. >This would however mean that PCIe (i.e. ethernet, usb ports on the Mac >mini) and possibly Thunderbolt support would not be possible since these >devices don't seem to like iommu bypass mode at all. It should be possible to do a fake bypass mode by just programming a static page table for as much address space as you can, and then use swiotlb to address any memory beyond that. This won't perform well because it requires bounce buffers for any high memory, but it can be a last resort if a dart instance cannot do normal bypass mode. > 2) I've had a brief discussion on IRC with Arnd about this [1] and he pointed >out that the dma_map_sg API doesn't make any guarantees about the returned >iovas and that it might be possible to make this work at least for devices >that go through the normal DMA API. > >I've then replaced the page size check with a WARN_ON in iova.c just to see >what happens. At least normal devices that go through the DMA API seem to >work with my configuration. iommu_dma_alloc took the iommu_dma_alloc_remap >path which was called with the cpu page size but then used >domain->pgsize_bitmap to increase that to 16k. So this kinda works out, but >there are other functions in dma-iommu.c that I believe rely on the fact > that >the iommu can map single cpu pages. This feels very fragile right now and >would probably require some rather invasive changes. The other second-to-last resort here would be to duplicate the code from the dma-iommu code and implement the dma-mapping API directly on top of the dart hardware instead of the iommu layer. This would probably be much faster than the swiotlb on top of a bypass or a linear map, but it's a really awful abstraction that would require adding special cases into a lot of generic code. >Any driver that tries to use the iommu API directly could be trouble >as well if they make similar assumptions. I think pretty much all drivers using the iommu API directly already depends on having a matching page size. I don't see any way to use e.g. PCI device assignment using vfio, or a GPU driver with per-process contexts when the iotlb page size is larger than the CPU's. >Is this something you would even want to support in the iommu subsytem >and is it even possible to do this in a sane way? I don't know how hard it is to do adjust the dma-iommu implementation to allow this, but as long as we can work out the DT binding to support both normal dma-iommu mode with 16KB pages and some kind of passthrough mode (emulated or not) with 4KB pages, it can be left as a possible optimization for later. Arnd
Re: [PATCH v3] drivers: introduce and use WANT_DMA_CMA for soft dependencies on DMA_CMA
On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand wrote:
>
> Random drivers should not override a user configuration of core knobs
> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
> which depends on CMA, if possible; however, these drivers also have to
> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
> driver cannot do it's job properly in some configurations.
>
> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
> available") documents
> While this is no build dependency, etnaviv will only work correctly
> on most systems if CMA and DMA_CMA are enabled. Select both options
> if available to avoid users ending up with a non-working GPU due to
> a lacking kernel config.
> So etnaviv really wants to have DMA_CMA, however, can deal with some cases
> where it is not available.
>
> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
> of recursive dependency issues).
>
> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>
> With this change, distributions can disable CONFIG_CMA or
> CONFIG_DMA_CMA, without it silently getting enabled again by random
> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER.
>
> For example, if any driver selects WANT_DMA_CMA and we do a
> "make olddefconfig":
>
> 1. With "# CONFIG_CMA is not set" and no specification of
>"CONFIG_DMA_CMA"
>
> -> CONFIG_DMA_CMA won't be part of .config
>
> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>
> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>
> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>
> -> CONFIG_DMA_CMA will be removed from .config
>
> Note: drivers/remoteproc seems to be special; commit c51e882cd711
> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
> there is a real dependency to DMA_CMA for it to work; leave that dependency
> in place and don't convert it to a soft dependency.
I don't think this dependency is fundamentally different from the others,
though davinci machines tend to have less memory than a lot of the
other machines, so it's more likely to fail without CMA.
Regardless of this,
Reviewed-by: Arnd Bergmann
Re: [PATCH v2 00/21] ipmi: Allow raw access to KCS devices
On Fri, Apr 9, 2021 at 6:09 AM Joel Stanley wrote: > On Thu, 8 Apr 2021 at 23:47, Andrew Jeffery wrote: > > On Thu, 8 Apr 2021, at 21:44, Corey Minyard wrote: > > > On Thu, Apr 08, 2021 at 10:27:46AM +0930, Andrew Jeffery wrote: > > > There were some minor concerns that were unanswered, and there really > > > was no review by others for many of the patches. > > > > Right; I was planning to clean up the minor concerns once I'd received > > some more feedback. I could have done a better job of communicating > > that :) > > I'll merge the first five through the aspeed tree this coming merge > window. We have acks from the relevant maintainers. > > Arnd: would you prefer that this come as it's own pull request, or as > part of the device tree branch? When you are unsure, it's almost never wrong to go for a separate branch, which gives you a chance to have a concise description of the contents in the tag. This would be particularly helpful if there are incompatible changes to the DT binding that require a justification. If you are only adding a few DT nodes to existing files, then merging these through the regular branch is probably easier. Arnd
Re: [PATCH v2 00/10] Initial support for Nuvoton WPCM450 BMC SoC
On Fri, Apr 9, 2021 at 9:58 AM Jonathan Neuschäfer wrote: > On Fri, Apr 09, 2021 at 04:37:34AM +, Joel Stanley wrote: > > On Tue, 6 Apr 2021 at 21:59, Jonathan Neuschäfer > > wrote: > > I've had a look at the series and it looks good to me: > > > > Reviewed-by: Joel Stanley > > > > Nice work Jonathan. > > > > I'll put this in it's own branch along with the bindings change it > > depends on and send a pull request to Arnd for v5.13. > > Thanks a bunch! > > A few patches are going through other branches (IRQ bindings+driver; > watchdog bindings+driver probably, I guess). That leaves the following > patches to go into your branch, AFAIUI: > > [PATCH v2 01/10] dt-bindings: vendor-prefixes: Add Supermicro > [PATCH v2 02/10] dt-bindings: arm: npcm: Add nuvoton,wpcm450 compatible string > [PATCH v2 05/10] ARM: npcm: Introduce Nuvoton WPCM450 SoC > [PATCH v2 08/10] ARM: dts: Add devicetree for Nuvoton WPCM450 BMC chip > [PATCH v2 09/10] ARM: dts: Add devicetree for Supermicro X9SCi-LN4F based on > WPCM450 > [PATCH v2 10/10] MAINTAINERS: Add entry for Nuvoton WPCM450 Actually for an initial merge, we sometimes just put all the patches into one branch in the soc tree to avoid conflicts. Unfortunately we already have a (trivial) conflict now anyway since I merged the irqchip driver for the Apple M1 SoC through the soc tree but the wpcm irqchip through the irqchip tree. You did nothing wrong here, this would have just been a way to make the initial merge a bit easier, and have a tree that is more easily bisectible when one branch in the merge window contains all the code that is needed for booting. Arnd
Re: [PATCH v2 16/21] ipmi: kcs_bmc: Add a "raw" character device interface
On Fri, Mar 19, 2021 at 7:31 AM Andrew Jeffery wrote: > > The existing IPMI chardev encodes IPMI behaviours as the name suggests. > However, KCS devices are useful beyond IPMI (or keyboards), as they > provide a means to generate IRQs and exchange arbitrary data between a > BMC and its host system. I only noticed the series after Joel asked about the DT changes on the arm side. One question though: How does this related to the drivers/input/serio/ framework that also talks to the keyboard controller for things that are not keyboards? Are these separate communication channels on adjacent I/O ports, or does there need to be some arbitration? Arnd
[PATCH] ARM: dts: clps711x: fix missing interrupt parent
From: Arnd Bergmann
The kernelci.org bot reports a build time regression:
arch/arm/boot/dts/ep7209.dtsi:187.17-192.4: Warning (interrupts_property):
/keypad: Missing interrupt-parent
There is only one interrupt controller in this SoC, so I assume this
is the parent.
Fixes: 2bd86203acf3 ("ARM: dts: clps711x: Add keypad node")
Reported-by: kernelci.org bot
Signed-off-by: Arnd Bergmann
---
I applied this fixup on top of the arm/dt branch, so it will be part of
linux-next and the v5.13 pull requests.
arch/arm/boot/dts/ep7209.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/ep7209.dtsi b/arch/arm/boot/dts/ep7209.dtsi
index 40a277370fd0..57bdad2c1994 100644
--- a/arch/arm/boot/dts/ep7209.dtsi
+++ b/arch/arm/boot/dts/ep7209.dtsi
@@ -186,6 +186,7 @@ syscon3: syscon@80002200 {
keypad: keypad {
compatible = "cirrus,ep7209-keypad";
+ interrupt-parent = <>;
interrupts = <16>;
syscon = <>;
status = "disabled";
--
2.29.2
[PATCH] ARM: dts: mvebu: fix SPI device node
From: Arnd Bergmann
dtc warns about a mismatched address:
arch/arm/boot/dts/armada-385-atl-x530.dts:171.14-199.4: Warning (spi_bus_reg):
/soc/spi@10680/spi-flash@0: SPI bus unit address format error, expected "1"
I assume the "reg" property is correct here, so adjust the unit address
accordingly.
Fixes: c6dfc019c239 ("ARM: dts: mvebu: Add device tree for ATL-x530 Board")
Reported-by: Stephen Rothwell
Reported-by: kernelci.org bot
Signed-off-by: Arnd Bergmann
---
I applied this fixup to the arm/dt branch on top of the new file, this
will be part of linux-next and the v5.13 pull request
arch/arm/boot/dts/armada-385-atl-x530.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/armada-385-atl-x530.dts
b/arch/arm/boot/dts/armada-385-atl-x530.dts
index 2041bf09c578..ed3f41c7df71 100644
--- a/arch/arm/boot/dts/armada-385-atl-x530.dts
+++ b/arch/arm/boot/dts/armada-385-atl-x530.dts
@@ -168,7 +168,7 @@ {
pinctrl-0 = <_pins>;
status = "okay";
- spi-flash@0 {
+ spi-flash@1 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "jedec,spi-nor";
--
2.29.2
Re: [PATCH v2] clk: socfpga: fix iomem pointer cast on 64-bit
From: Arnd Bergmann On Sun, 14 Mar 2021 12:07:09 +0100, Krzysztof Kozlowski wrote: > Pointers should be cast with uintptr_t instead of integer. This fixes > warning when compile testing on ARM64: > > drivers/clk/socfpga/clk-gate.c: In function ‘socfpga_clk_recalc_rate’: > drivers/clk/socfpga/clk-gate.c:102:7: warning: cast from pointer to integer > of different size [-Wpointer-to-int-cast] I decided to pull that into the arm/drivers branch as well, to avoid the build regression. Since the same fix is in the clk tree, there will now be a duplicated commit in the git history, but I prefer that over introducing warnings. [1/1] clk: socfpga: fix iomem pointer cast on 64-bit commit: 36841008059caec9667459a7e126efac6379676b Arnd
Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
On Thu, Apr 8, 2021 at 11:00 PM Masahiro Yamada wrote: > > Drivers typically surround suspend and resume callbacks with #ifdef > CONFIG_PM(_SLEEP) or mark them as __maybe_unused in order to avoid > -Wunused-const-variable warnings. > > With this commit, drivers will be able to remove #ifdef CONFIG_PM(_SLEEP) > and __maybe_unsed because unused functions are dropped by the compiler > instead of the preprocessor. > > Signed-off-by: Masahiro Yamada I tried this before and could not get it to work right. > > -#ifdef CONFIG_PM_SLEEP > +#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), _ptr) > +#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), _ptr) > + > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > - .suspend = suspend_fn, \ > - .resume = resume_fn, \ > - .freeze = suspend_fn, \ > - .thaw = resume_fn, \ > - .poweroff = suspend_fn, \ > - .restore = resume_fn, > -#else > -#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > -#endif > + .suspend = pm_sleep_ptr(suspend_fn), \ > + .resume = pm_sleep_ptr(resume_fn), \ > + .freeze = pm_sleep_ptr(suspend_fn), \ > + .thaw = pm_sleep_ptr(resume_fn), \ > + .poweroff = pm_sleep_ptr(suspend_fn), \ > + .restore = pm_sleep_ptr(resume_fn), The problem that I think you inevitably hit is that you run into a missing declaration for any driver that still uses an #ifdef around a static function. The only way I can see us doing this is to create a new set of macros that behave like the version you propose here but leave the old macros in place until the last such #ifdef has been removed. Arnd
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 6:45 PM David Hildenbrand wrote: > On 08.04.21 14:49, Linus Walleij wrote: > > On Thu, Apr 8, 2021 at 2:01 PM David Hildenbrand wrote: > > > >>> This is something you could do using a hidden helper symbol like > >>> > >>> config DRMA_ASPEED_GFX > >>> bool "Aspeed display driver" > >>> select DRM_WANT_CMA > >>> > >>> config DRM_WANT_CMA > >>> bool > >>> help > >>> Select this from any driver that benefits from CMA being > >>> enabled > >>> > >>> config DMA_CMA > >>> bool "Use CMA helpers for DRM" > >>> default DRM_WANT_CMA > >>> > >>>Arnd > >>> > >> > >> That's precisely what I had first, with an additional "WANT_CMA" -- but > >> looking at the number of such existing options (I was able to spot 1 !) > > > > If you do this it probably makes sense to fix a few other drivers > > Kconfig in the process. It's not just a problem with your driver. > > "my" drivers: > > > > :) I actually wanted to convert them to "depends on DMA_CMA" but ran > into recursive dependencies ... > > > drivers/gpu/drm/mcde/Kconfig > > drivers/gpu/drm/pl111/Kconfig > > drivers/gpu/drm/tve200/Kconfig Right, this is the main problem caused by using 'select' to force-enable symbols that other drivers depend on. Usually, the answer is to be consistent about the use of 'select' and 'depends on', using the former only to enable symbols that are hidden, while using 'depends on' for anything that is an actual build time dependency. > I was assuming these are "real" dependencies. Will it also work without > DMA_CMA? I think in this case, it is fairly likely to work without DMA_CMA when the probe function gets called during a fresh boot, but fairly likely to fail if it gets called after the system has run for long enough to fragment the free memory. The point of DMA_CMA is to make it work reliably. Arnd
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 2:50 PM Linus Walleij wrote: > > On Thu, Apr 8, 2021 at 2:01 PM David Hildenbrand wrote: > > > > This is something you could do using a hidden helper symbol like > > > > > > config DRMA_ASPEED_GFX > > > bool "Aspeed display driver" > > > select DRM_WANT_CMA > > > > > > config DRM_WANT_CMA > > > bool > > > help > > >Select this from any driver that benefits from CMA being > > > enabled > > > > > > config DMA_CMA > > > bool "Use CMA helpers for DRM" > > > default DRM_WANT_CMA > > > > > > Arnd > > > > > > > That's precisely what I had first, with an additional "WANT_CMA" -- but > > looking at the number of such existing options (I was able to spot 1 !) > > If you do this it probably makes sense to fix a few other drivers > Kconfig in the process. It's not just a problem with your driver. > "my" drivers: > > drivers/gpu/drm/mcde/Kconfig > drivers/gpu/drm/pl111/Kconfig > drivers/gpu/drm/tve200/Kconfig > > certainly needs this as well, and pretty much anything that is > selecting DRM_KMS_CMA_HELPER or > DRM_GEM_CMA_HELPER "wants" DMA_CMA. Are there any that don't select either of the helpers and still want CMA? If not, it would be easy to just add default DRM_KMS_CMA_HELPER || DRM_GEM_CMA_HELPER and skipt the extra symbol. Arnd
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 2:00 PM David Hildenbrand wrote: > > On 08.04.21 13:44, Arnd Bergmann wrote: > > On Thu, Apr 8, 2021 at 1:00 PM David Hildenbrand wrote: > >>> > >>> It is a somewhat awkward way to say "prevent this symbol from > >>> being =y if the dependency is =m". > >> > >> What would be the right thing to do in the case here then to achieve the > >> "if DRMA_ASPEED_GFX is enabled, also enable DMA_CMA id possible"? > >> > >> One approach could be to have for DMA_CMA > >> > >> default y if DRMA_ASPEED_GFX > >> > >> but it feels like the wrong way to tackle this. > > > > I'm still not sure what you are trying to achieve. Is the idea only to > > provide > > a useful default for DMA_CMA depending on which drivers are enabled? > > "Random drivers should not override a user configuration of core knobs > (e.g., CONFIG_DMA_CMA=n)." > > Let's assume I'm a distribution and want to set CONFIG_CMA=n or want to > set CONFIG_DMA_CMA=n with CONFIG_CMA=y; there is no way to do that with > e.g., DRMA_ASPEED_GFX=y because it will always override my (user!) > setting -- even though it doesn't really always need it. Using "select" > is the problem here. I agree on the part of removing the 'select' if we don't need it. The part I couldn't figure out was what the 'imply' is supposed to help with. Most other users that added imply tried (and failed) to fix a build problem. > > This is something you could do using a hidden helper symbol like > > > > config DRMA_ASPEED_GFX > > bool "Aspeed display driver" > > select DRM_WANT_CMA > > > > config DRM_WANT_CMA > > bool > > help > >Select this from any driver that benefits from CMA being enabled > > > > config DMA_CMA > > bool "Use CMA helpers for DRM" > > default DRM_WANT_CMA > > > > Arnd > > > > That's precisely what I had first, with an additional "WANT_CMA" -- but > looking at the number of such existing options (I was able to spot 1 !) > I wondered if there is a better approach to achieve the same; "imply" > sounded like a good candidate. I can probably find a couple more, but regardless of how many others exist, this would be a much clearer way of doing it than 'imply' since it has none of the ambiguity and misuse problems. I think the reason we don't see more is that generally speaking, those defaults are widely ignored anyway. You almost always start out with a defconfig file that contains everything you know you need, and then you add bits to that. Having the default in any form only helps to make that defconfig file one line shorter, while requiring other users to add another line to turn it off when they do not want it. Arnd
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 1:00 PM David Hildenbrand wrote: > > > > It is a somewhat awkward way to say "prevent this symbol from > > being =y if the dependency is =m". > > What would be the right thing to do in the case here then to achieve the > "if DRMA_ASPEED_GFX is enabled, also enable DMA_CMA id possible"? > > One approach could be to have for DMA_CMA > > default y if DRMA_ASPEED_GFX > > but it feels like the wrong way to tackle this. I'm still not sure what you are trying to achieve. Is the idea only to provide a useful default for DMA_CMA depending on which drivers are enabled? This is something you could do using a hidden helper symbol like config DRMA_ASPEED_GFX bool "Aspeed display driver" select DRM_WANT_CMA config DRM_WANT_CMA bool help Select this from any driver that benefits from CMA being enabled config DMA_CMA bool "Use CMA helpers for DRM" default DRM_WANT_CMA Arnd
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 12:29 PM David Hildenbrand wrote: > On 08.04.21 12:20, Arnd Bergmann wrote: > > On Thu, Apr 8, 2021 at 11:22 AM David Hildenbrand wrote: > >> > >> Random drivers should not override a user configuration of core knobs > >> (e.g., CONFIG_DMA_CMA=n). Use "imply" instead, to still respect > >> dependencies and manual overrides. > >> > >> "This is similar to "select" as it enforces a lower limit on another > >> symbol except that the "implied" symbol's value may still be set to n > >> from a direct dependency or with a visible prompt." > >> > >> Implying DRM_CMA should be sufficient, as that depends on CMA. > >> > >> Note: If this is a real dependency, we should use "depends on DMA_CMA" > >> instead - but I assume the driver can work without CMA just fine -- > >> esp. when we wouldn't have HAVE_DMA_CONTIGUOUS right now. > > > > 'imply' is almost never the right solution, and it tends to cause more > > problems than it solves. > > I thought that was the case with "select" :) Yes, but that's a different set of problems > > > > In particular, it does not prevent a configuration with 'DRM_CMA=m' > > I assume you meant "DRM_CMA=n" ? DRM_CMA cannot be built as a module. Ok, at least that makes it easier. > > and 'DRMA_ASPEED_GFX=y', or any build failures from such > > a configuration. > > I don't follow. "DRM_CMA=n" and 'DRMA_ASPEED_GFX=y' is supposed to work > just fine (e.g., without HAVE_DMA_CONTIGUOUS) or what am I missing? I thought you were trying to solve the problem where DRMA_ASPEED_GFX can optionally link against CMA but would fail to build when the CMA code is in a loadable module. If the problem you are trying to solve is a different one, you need a different solution, not what I posted above. > > If you want this kind of soft dependency, you need > > 'depends on DRM_CMA || !DRM_CMA'. > > Seriously? I think the point of imply is "please enable if possible and > not prevented by someone else". That used to be the meaning, but it changed a few years ago. Now it means "when a used manually turns on this symbol, turn on the implied one as well, but let them turn it off again if they choose". This is pretty much a NOP. > Your example looks more like a NOP - no? > Or will it have the same effect? The example I gave is only meaningful if both are tristate, which is not the case here as you explain. It is a somewhat awkward way to say "prevent this symbol from being =y if the dependency is =m". Arnd
Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv
On Thu, Apr 8, 2021 at 11:22 AM David Hildenbrand wrote: > > Random drivers should not override a user configuration of core knobs > (e.g., CONFIG_DMA_CMA=n). Use "imply" instead, to still respect > dependencies and manual overrides. > > "This is similar to "select" as it enforces a lower limit on another > symbol except that the "implied" symbol's value may still be set to n > from a direct dependency or with a visible prompt." > > Implying DRM_CMA should be sufficient, as that depends on CMA. > > Note: If this is a real dependency, we should use "depends on DMA_CMA" > instead - but I assume the driver can work without CMA just fine -- > esp. when we wouldn't have HAVE_DMA_CONTIGUOUS right now. 'imply' is almost never the right solution, and it tends to cause more problems than it solves. In particular, it does not prevent a configuration with 'DRM_CMA=m' and 'DRMA_ASPEED_GFX=y', or any build failures from such a configuration. If you want this kind of soft dependency, you need 'depends on DRM_CMA || !DRM_CMA'. Arnd
Re: arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section `__cpuidle_method_of_table'
On Wed, Apr 7, 2021 at 8:07 PM Nick Desaulniers wrote: > > On Wed, Apr 7, 2021 at 3:52 AM kernel test robot wrote: > > > > Hi Kees, > > > > FYI, the error/warning still remains. > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > master > > head: 2d743660786ec51f5c1fefd5782bbdee7b227db0 > > commit: 5a17850e251a55bba6d65aefbfeacfa9888cd2cd arm/build: Warn on orphan > > section placement > > date: 7 months ago > > config: arm-randconfig-r033-20210407 (attached as .config) > > compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 > > reproduce (this is a W=1 build): > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5a17850e251a55bba6d65aefbfeacfa9888cd2cd > > git remote add linus > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > git fetch --no-tags linus master > > git checkout 5a17850e251a55bba6d65aefbfeacfa9888cd2cd > > # save the attached .config to linux build tree > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > > ARCH=arm > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot > > > > All warnings (new ones prefixed by >>): > > > > >> arm-linux-gnueabi-ld: warning: orphan section > > >> `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' > > >> being placed in section `__cpuidle_method_of_table' > > >> arm-linux-gnueabi-ld: warning: orphan section > > >> `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' > > >> being placed in section `__cpuidle_method_of_table' > > >> arm-linux-gnueabi-ld: warning: orphan section > > >> `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' > > >> being placed in section `__cpuidle_method_of_table' > > Looks like arch/arm/include/asm/cpuidle.h defines > `CPUIDLE_METHOD_OF_DECLARE` to create a static struct in such a > section. Only arch/arm/mach-omap2/pm33xx-core.c uses that macro. Nick, Kees, Should I resend my patch, or are you taking care of it? https://lore.kernel.org/lkml/20201230155506.1085689-1-a...@kernel.org/T/#u Arnd
[irqchip: irq/irqchip-next] irqchip/gic-v3: Fix OF_BAD_ADDR error handling
The following commit has been merged into the irq/irqchip-next branch of
irqchip:
Commit-ID: 8e13d96670a4c050d4883e6743a9e9858e5cfe10
Gitweb:
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/8e13d96670a4c050d4883e6743a9e9858e5cfe10
Author:Arnd Bergmann
AuthorDate:Tue, 23 Mar 2021 14:18:35 +01:00
Committer: Marc Zyngier
CommitterDate: Wed, 07 Apr 2021 13:25:52 +01:00
irqchip/gic-v3: Fix OF_BAD_ADDR error handling
When building with extra warnings enabled, clang points out a
mistake in the error handling:
drivers/irqchip/irq-gic-v3-mbi.c:306:21: error: result of comparison of
constant 18446744073709551615 with expression of type 'phys_addr_t' (aka
'unsigned int') is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if (mbi_phys_base == OF_BAD_ADDR) {
Truncate the constant to the same type as the variable it gets compared
to, to shut make the check work and void the warning.
Fixes: 505287525c24 ("irqchip/gic-v3: Add support for Message Based Interrupts
as an MSI controller")
Signed-off-by: Arnd Bergmann
Signed-off-by: Marc Zyngier
Link: https://lore.kernel.org/r/20210323131842.2773094-1-a...@kernel.org
---
drivers/irqchip/irq-gic-v3-mbi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-mbi.c b/drivers/irqchip/irq-gic-v3-mbi.c
index 563a9b3..e81e89a 100644
--- a/drivers/irqchip/irq-gic-v3-mbi.c
+++ b/drivers/irqchip/irq-gic-v3-mbi.c
@@ -303,7 +303,7 @@ int __init mbi_init(struct fwnode_handle *fwnode, struct
irq_domain *parent)
reg = of_get_property(np, "mbi-alias", NULL);
if (reg) {
mbi_phys_base = of_translate_address(np, reg);
- if (mbi_phys_base == OF_BAD_ADDR) {
+ if (mbi_phys_base == (phys_addr_t)OF_BAD_ADDR) {
ret = -ENXIO;
goto err_free_mbi;
}
Re: [PATCH v4 3/4] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
On Wed, Apr 7, 2021 at 1:36 PM Peter Zijlstra wrote:
>
> On Wed, Apr 07, 2021 at 10:42:50AM +0200, Arnd Bergmann wrote:
> > Since there are really only a handful of instances in the kernel
> > that use the cmpxchg() or xchg() on u8/u16 variables, it would seem
> > best to just disallow those completely
>
> Not going to happen. xchg16 is optimal for qspinlock and if we replace
> that with a cmpxchg loop on x86 we're regressing.
I did not mean to remove the possibility of doing a 16-bit compare-exchange
operation altogether, just removing it from the regular macros.
We already do this for the 64-bit xchg64()/cmpxchg64(), which only some
of the 32-bit architectures provide, so I think having an explicit
xchg8()/xchg16()/cmpxchg8()/cmpxchg16() interface while tightening the
type checking would be more consistent here.
On any 32-bit architecture, cmpxchg()/xchg() macros then only have
to deal with word-sized operations.
> > Interestingly, the s390 version using __sync_val_compare_and_swap()
> > seems to produce nice output on all architectures that have atomic
> > instructions, with any supported compiler, to the point where I think
> > we could just use that to replace most of the inline-asm versions except
> > for arm64:
> >
> > #define cmpxchg(ptr, o, n) \
> > ({ \
> > __typeof__(*(ptr)) __o = (o); \
> > __typeof__(*(ptr)) __n = (n); \
> > (__typeof__(*(ptr))) __sync_val_compare_and_swap((ptr),__o,__n);\
> > })
>
> It generates the LL/SC loop, but doesn't do sensible optimizations when
> it's again used in a loop itself. That is, it generates a loop of a
> loop, just like what you'd expect, which is sub-optimal for LL/SC.
One thing that it does though is to generate an ll/sc loop for xchg16(),
while mips, openrisc, xtensa and sparc64 currently open-code the
nested loop in their respective xchg16() wrappers. I have not seen
any case in which it produces object code that is worse than the
architecture specific code does today, except for those that rely on
runtime patching (i486+smp, arm64+lse).
> > Not how gcc's acquire/release behavior of __sync_val_compare_and_swap()
> > relates to what the kernel wants here.
> >
> > The gcc documentation also recommends using the standard
> > __atomic_compare_exchange_n() builtin instead, which would allow
> > constructing release/acquire/relaxed versions as well, but I could not
> > get it to produce equally good output. (possibly I was using it wrong)
>
> I'm scared to death of the C11 crap, the compiler will 'optimize' them
> when it feels like it and use the C11 memory model rules for it, which
> are not compatible with the kernel rules.
>
> But the same thing applies, it won't do the right thing for composites.
Makes sense. As I said, I could not even get it to produce optimal code
for the simple case.
Arnd
Re: [PATCH v4 3/4] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
On Tue, Apr 6, 2021 at 10:56 AM Stafford Horne wrote:
> On Tue, Apr 06, 2021 at 11:50:38AM +0800, Guo Ren wrote:
> > On Wed, Mar 31, 2021 at 3:23 PM Arnd Bergmann wrote:
> > > On Wed, Mar 31, 2021 at 12:35 AM Stafford Horne wrote:
> >
> > We shouldn't export xchg16/cmpxchg16(emulated by lr.w/sc.w) in riscv,
> > We should forbid these sub-word atomic primitive and lets the
> > programmers consider their atomic design.
>
> Fair enough, having the generic sub-word emulation would be something that
> an architecture can select to use/export.
I still have the feeling that we should generalize and unify the exact behavior
across architectures as much as possible here, while possibly also trying to
simplify the interface a little.
Looking through the various xchg()/cmpxchg() implementations, I find eight
distinct ways to do 8-bit and 16-bit atomics:
Full support:
ia64, m68k (Atari only), x86, arm32 (v6k+), arm64
gcc/clang __sync_{val,bool}_compare_and_swap:
s390
Emulated through ll/sc:
alpha, powerpc
Emulated through cmpxchg loop:
mips, openrisc, xtensa (xchg but not cmpxchg), sparc64 (cmpxchg_u8,
xchg_u16 but not cmpxchg_u16 and xchg_u8!)
Emulated through local_irq_save (non SMP only):
h8300, m68k (most), microblaze, mips, nds32, nios2
Emulated through hashed spinlock:
parisc (8-bit only added in 2020, 16-bit still missing)
Forced compile-time error:
arm32 (v4/v5/v6 non-SMP), arc, csky, riscv, parisc (16 bit), sparc32,
sparc64, xtensa (cmpxchg)
Silently broken:
hexagon
Since there are really only a handful of instances in the kernel
that use the cmpxchg() or xchg() on u8/u16 variables, it would seem
best to just disallow those completely and have a separate set of
functions here, with only 64-bit architectures using any variable-type
wrapper to handle both 32-bit and 64-bit arguments.
Interestingly, the s390 version using __sync_val_compare_and_swap()
seems to produce nice output on all architectures that have atomic
instructions, with any supported compiler, to the point where I think
we could just use that to replace most of the inline-asm versions except
for arm64:
#define cmpxchg(ptr, o, n) \
({ \
__typeof__(*(ptr)) __o = (o); \
__typeof__(*(ptr)) __n = (n); \
(__typeof__(*(ptr))) __sync_val_compare_and_swap((ptr),__o,__n);\
})
Not how gcc's acquire/release behavior of __sync_val_compare_and_swap()
relates to what the kernel wants here.
The gcc documentation also recommends using the standard
__atomic_compare_exchange_n() builtin instead, which would allow
constructing release/acquire/relaxed versions as well, but I could not
get it to produce equally good output. (possibly I was using it wrong)
Arnd
Re: linux-next: Signed-off-by missing for commit in the arm-soc tree
On Tue, Apr 6, 2021 at 12:11 AM Stephen Rothwell wrote:
>
> Hi all,
>
> On Tue, 6 Apr 2021 08:11:00 +1000 Stephen Rothwell
> wrote:
> >
> > Hi all,
> >
> > Commit
> >
> > 3b493ac0ac04 ("arm64: dts: allwinner: h6: Switch to macros for RSB
> > clock/reset indices")
> >
> > is missing a Signed-off-by from its committer.
>
> Sorry, that commit is in the arm-soc-fixes tree.
Thanks for the report, I've temporarily removed the sunx-fixes branch merge
from the arm/fixes branch and will send the pull request without it.
Maxime, can you fix it up and resend the pull request?
Feel free to add any other fixes that have come up since then.
Arnd
Re: [PATCH v2 00/10] Initial support for Nuvoton WPCM450 BMC SoC
On Tue, Apr 6, 2021 at 2:09 PM Jonathan Neuschäfer wrote: > > This series adds basic support for the Nuvoton WPCM450 BMC SoC. It's an older > SoC but still commonly found on eBay, mostly in Supermicro X9 server boards. > > Third-party documentation is available at: > https://github.com/neuschaefer/wpcm450/wiki > > Patches 1-4 add devicetree bindings for the WPCM450 SoC and its various parts. > Patches 5-7 add arch and driver support. Patches 8 and 9 add a devicetree for > the SoC and a board based on it. Patch 10 finally updates the MAINTAINERS > file. > > Patch 2 requires "dt-bindings: arm: Convert nuvoton,npcm750 binding to YAML" > (https://lore.kernel.org/lkml/20210320164023.614059-1-j.neuschae...@gmx.net/) Hi Jonathan, It appears these patches are doing roughly the right thing, and we may still be able to get them into v5.13, but I'm not sure what your plan for maintaining them is. The two options are that you either send your patches to be picked up by Joel, or you send everything directly to s...@kernel.org once it's fully reviewed. I only noticed your series when patch 9/10 made it into the s...@kernel.org patchwork because of the Cc, but none of the other ones did. If you end up with the second option, we can go through what this involves off-list. Regarding the Cc:s...@kernel.org, please add that only for patches that are already reviewed and ready to be picked up, ideally with a cover letter that describes what the plan is for merging. If you need me to review the platform code, use my a...@arndb.de or a...@kernel.org addresses. Arnd
Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
On Tue, Apr 6, 2021 at 3:31 PM Andy Shevchenko wrote: > > kernel.h is being used as a dump for all kinds of stuff for a long time. > Here is the attempt to start cleaning it up by splitting out panic and > oops helpers. > > At the same time convert users in header and lib folder to use new header. > Though for time being include new header back to kernel.h to avoid twisted > indirected includes for existing users. > > Signed-off-by: Andy Shevchenko Nice! Acked-by: Arnd Bergmann
Re:
On Mon, Apr 5, 2021 at 2:03 AM Mitali Borkar wrote:
>
> outreachy-ker...@googlegroups.com, mitaliborkar...@gmail.com
> Bcc:
> Subject: [PATCH] staging: qlge:remove else after break
> Reply-To:
>
> Fixed Warning:- else is not needed after break
> break terminates the loop if encountered. else is unnecessary and
> increases indenatation
>
> Signed-off-by: Mitali Borkar
> ---
> drivers/staging/qlge/qlge_mpi.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge_mpi.c b/drivers/staging/qlge/qlge_mpi.c
> index 2630ebf50341..3a49f187203b 100644
> --- a/drivers/staging/qlge/qlge_mpi.c
> +++ b/drivers/staging/qlge/qlge_mpi.c
> @@ -935,13 +935,11 @@ static int qlge_idc_wait(struct qlge_adapter *qdev)
> netif_err(qdev, drv, qdev->ndev, "IDC Success.\n");
> status = 0;
> break;
> - } else {
> - netif_err(qdev, drv, qdev->ndev,
> + } netif_err(qdev, drv, qdev->ndev,
> "IDC: Invalid State 0x%.04x.\n",
> mbcp->mbox_out[0]);
> status = -EIO;
> break;
> - }
> }
It looks like you got this one wrong in multiple ways:
- This is not an equivalent transformation, since the errror is now
printed in the first part of the 'if()' block as well.
- The indentation is wrong now, with the netif_err() starting in the
same line as the '}'.
- The description mentions a change in indentation, but you did not
actually change it.
- The changelog text appears mangled.
Arnd
Re: [PATCH 1/8] dt-bindings: clk: mstar msc313 cpupll binding description
On Fri, Feb 26, 2021 at 12:31 PM Daniel Palmer wrote: > > Hi Rob's bot > > On Wed, 24 Feb 2021 at 04:34, Rob Herring wrote: > > dtschema/dtc warnings/errors: > > Documentation/devicetree/bindings/clock/mstar,msc313-cpupll.example.dts:19:18: > > fatal error: dt-bindings/clock/mstar-msc313-mpll.h: No such file or > > directory > >19 | #include > > | ^~~ > > compilation terminated. > > make[1]: *** [scripts/Makefile.lib:344: > > Documentation/devicetree/bindings/clock/mstar,msc313-cpupll.example.dt.yaml] > > Error 1 > > make[1]: *** Waiting for unfinished jobs > > make: *** [Makefile:1370: dt_binding_check] Error 2 > > Looks like I sent this too early. I will try again later. I found this is still in patchwork as not merged, and I have not seen a replacement. Marking all eight patches as 'changes requested' now, please resend. Arnd
Re: [PATCH v6 00/12] lib/find_bit: fast path for small bitmaps
On Thu, Apr 1, 2021 at 11:16 AM Andy Shevchenko wrote: > > On Thu, Apr 1, 2021 at 3:36 AM Yury Norov wrote: > > > > Bitmap operations are much simpler and faster in case of small bitmaps > > which fit into a single word. In linux/bitmap.c we have a machinery that > > allows compiler to replace actual function call with a few instructions > > if bitmaps passed into the function are small and their size is known at > > compile time. > > > > find_*_bit() API lacks this functionality; but users will benefit from it > > a lot. One important example is cpumask subsystem when > > NR_CPUS <= BITS_PER_LONG. > > Cool, thanks! > > I guess it's assumed to go via Andrew's tree. > > But after that since you are about to be a maintainer of this, I think > it would make sense to send PRs directly to Linus. I would recommend > creating an official tree (followed by an update in the MAINTAINERS) > and connecting it to Linux next (usually done by email to Stephen). It depends on how often we expect to see updates to this. I have not followed the changes as closely as I should have, but I can also merge them through the asm-generic tree for this time so Andrew has to carry fewer patches for this. I normally don't have a lot of material for asm-generic either, half the time there are no pull requests at all for a given release. I would expect future changes to the bitmap implementation to only need an occasional bugfix, which could go through either the asm-generic tree or through mm and doesn't need another separate pull request. If it turns out to be a tree that needs regular updates every time, then having a top level repository in linux-next would be appropriate. Arnd
Re: [PATCH] arm64: Add support for cisco craw64 ARMv8 SoCs
On Wed, Mar 31, 2021 at 7:57 PM Daniel Walker wrote: > > On Wed, Mar 31, 2021 at 09:04:15AM +0200, Arnd Bergmann wrote: > > On Wed, Mar 31, 2021 at 3:46 AM Daniel Walker wrote: > > > From: Ofer Licht > > > > Thanks for the submission, it's always nice to see a new platform > > > > > Define craw64 config, dts and Makefile for Cisco > > > SoCs known as Craw. > > > > I'd like some more information about the platform, e.g. the target > > market and maybe a link to the product information. > > Our SoC is produced as an internal product. So SoC specifications aren't > widely available. > > Here is an example of a Cisco product which uses this SoC, > > https://www.cisco.com/c/en/us/products/collateral/switches/catalyst-9200-series-switches/nb-06-cat9200-ser-data-sheet-cte-en.html > > I suspect that's not really what your looking for tho. No, that's pretty much exactly what I was looking for. Just put this one sentence and the link into the patch description and it will be fine. > > > > The memory size is usually filled by the boot loader, just put an > > empty node into the .dtsi file > > Arnd, I must regretfully inform you that Cisco has a deep dark addiction to > bootloaders which, are, um, how do I say this diplomatically, um , brain dead. > > You have some other comments below related to moving things into the > bootloader, > and I can look into it, but bootloader inflexibility is wide spread inside > Cisco. Ok, no worries. If the bootloader can do it right, you should do it, but this part is not essential. Arnd
Re: [PATCH v4 3/4] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
On Wed, Mar 31, 2021 at 12:35 AM Stafford Horne wrote: > > I just want to chime in here, there may be a better spot in the thread to > mention this but, for OpenRISC I did implement some generic 8/16-bit xchg code > which I have on my todo list somwhere to replace the other generic > implementations like that in mips. > > arch/openrisc/include/asm/cmpxchg.h > > The idea would be that architectures just implement these methods: > > long cmpxchg_u32(*ptr,old,new) > long xchg_u32(*ptr,val) > > Then the rest of the generic header would implement cmpxchg. I like the idea of generalizing it a little further. I'd suggest staying a little closer to the existing naming here though, as we already have cmpxchg() for the type-agnostic version, and cmpxchg64() for the fixed-length 64-bit version. I think a nice interface between architecture-specific and architecture independent code would be to have architectures provide arch_cmpxchg32()/arch_xchg32() as the most basic version, as well as arch_cmpxchg8()/arch_cmpxchg16()/arch_xchg8()/arch_xchg16() if they have instructions for those. The common code can then build cmpxchg16()/xchg16() on top of either the 16-bit or the 32-bit primitives, and build the cmpxchg()/xchg() wrapper around those (or alternatively we can decide to have them only deal with fixed-32-bit and long/pointer sized atomics). Arnd
Re: [PATCH v4 3/4] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
On Wed, Mar 31, 2021 at 8:44 AM Guo Ren wrote: > On Wed, Mar 31, 2021 at 12:18 PM Guo Ren wrote: > > On Tue, Mar 30, 2021 at 3:12 PM Arnd Bergmann wrote: > > > On Tue, Mar 30, 2021 at 4:26 AM Guo Ren wrote: > > > As I understand, this example must not cause a deadlock on > > > a compliant hardware implementation when the underlying memory > > > has RsrvEventual behavior, but could deadlock in case of > > > RsrvNonEventual > > Thx for the nice explanation: > > - RsrvNonEventual - depends on software fall-back mechanisms, and > > just I'm worried about. > > - RsrvEventual - HW would provide the eventual success guarantee. > In riscv-spec 8.3 Eventual Success of Store-Conditional Instructions > > I found: > "As a consequence of the eventuality guarantee, if some harts in an > execution environment are > executing constrained LR/SC loops, and no other harts or devices in > the execution environment > execute an unconditional store or AMO to that reservation set, then at > least one hart will > eventually exit its constrained LR/SC loop. *** By contrast, if other > harts or devices continue to > write to that reservation set, it ***is not guaranteed*** that any > hart will exit its LR/SC loop.*** " > > Seems RsrvEventual couldn't solve the code's problem I've mentioned. Ok, got it. Arnd
