Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux 6.11 compatibility #533

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

charlie89
Copy link

@charlie89 charlie89 commented Oct 16, 2024

Hi,

This adds support for linux kernel 6.11.
This kernel has 2 breaking changes that made sudo dpkg-reconfigure pivccu-modules-dkms fail as it couldn't compile the kernel module.
With the changes it works for me on a Ubuntu Server 24.10 VM (kernel 6.11.0-8-generic) using raspberrymatic in docker with the HB-RF-ETH.

In kernel/generic_raw_uart.h the function __##__raw_uart_driver##_remove had to be changed from int to void because of this kernel commit:

commit 0edb555a65d1ef047a9805051c36922b52a38a9d
Author: Uwe Kleine-König <[email protected]>
Date:   Mon Oct 9 12:37:26 2023 +0200

    platform: Make platform_driver::remove() return void
    
    struct platform_driver::remove returning an integer made driver authors
    expect that returning an error code was proper error handling. However
    the driver core ignores the error and continues to remove the device
    because there is nothing the core could do anyhow and reentering the
    remove callback again is only calling for trouble.
    
    To prevent such wrong assumptions, change the return type of the remove
    callback to void. This was prepared by introducing an alternative remove
    callback returning void and converting all drivers to that. So .remove()
    can be changed without further changes in drivers.
    
    This corresponds to step b) of the plan outlined in commit
    5c5a7680e67b ("platform: Provide a remove callback that returns no value").
    
    Signed-off-by: Uwe Kleine-König <[email protected]>

diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 7a41c72c1959..d422db6eec63 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -237,15 +237,14 @@ struct platform_driver {
        int (*probe)(struct platform_device *);
 
        /*
-        * Traditionally the remove callback returned an int which however is
-        * ignored by the driver core. This led to wrong expectations by driver
-        * authors who thought returning an error code was a valid error
-        * handling strategy. To convert to a callback returning void, new
-        * drivers should implement .remove_new() until the conversion it done
-        * that eventually makes .remove() return void.
+        * .remove_new() is a relic from a prototype conversion of .remove().
+        * New drivers are supposed to implement .remove(). Once all drivers are
+        * converted to not use .remove_new any more, it will be dropped.
         */
-       int (*remove)(struct platform_device *);
-       void (*remove_new)(struct platform_device *);
+       union {
+               void (*remove)(struct platform_device *);
+               void (*remove_new)(struct platform_device *);
+       };
 
        void (*shutdown)(struct platform_device *);
        int (*suspend)(struct platform_device *, pm_message_t state);

In kernel/hb_rf_eth.c we need to use gpiochip_add_data as gpiochip_add was removed in this kernel commit:

commit 3ff1180a39fbc43ae69d4238e6922c57e3278910
Author: Andrew Davis <[email protected]>
Date:   Mon Jun 10 08:53:13 2024 -0500

    gpiolib: Remove data-less gpiochip_add() function
    
    GPIO chips should be added with driver-private data associated with the
    chip. If none is needed, NULL can be used. All users already do this
    except one, fix that here. With no more users of the base gpiochip_add()
    we can drop this function so no more users show up later.
    
    Signed-off-by: Andrew Davis <[email protected]>
    Acked-by: Greg Kroah-Hartman <[email protected]>
    Link: https://lore.kernel.org/r/[email protected]
    Signed-off-by: Bartosz Golaszewski <[email protected]>

diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 0032bb6e7d8f..6d31388dde0a 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -632,10 +632,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
        devm_gpiochip_add_data_with_key(dev, gc, data, NULL, NULL)
 #endif /* CONFIG_LOCKDEP */
 
-static inline int gpiochip_add(struct gpio_chip *gc)
-{
-       return gpiochip_add_data(gc, NULL);
-}
 void gpiochip_remove(struct gpio_chip *gc);
 int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip *gc,
                                    void *data, struct lock_class_key *lock_key,

The old function calls should probably be left in there so it would still compile on older kernels, but I don't know how to do that...
Also i'm not sure if those kernel changes were introduced with 6.11 or in earlier versions.

for linux commit 0edb555a65d1ef047a9805051c36922b52a38a9d: platform: Make platform_driver::remove() return void
for linux commit 3ff1180a39fbc43ae69d4238e6922c57e3278910: gpiolib: Remove data-less gpiochip_add() function
@tuxBurner
Copy link

👍 Awesome

@dkadioglu
Copy link

If you additionally add the following change, then the modules also build on kernel 6.12, which will be the new LTS kernel as far as I understand.

diff --git a/kernel/eq3_char_loop.c b/kernel/eq3_char_loop.c
index 201cde4..a71098c 100644
--- a/kernel/eq3_char_loop.c
+++ b/kernel/eq3_char_loop.c
@@ -917,7 +917,7 @@ static int eq3loop_open(struct inode *inode, struct file *filp)
 
 static struct file_operations eq3loop_fops = {
        .owner          = THIS_MODULE,
-       .llseek         = no_llseek,
+       .llseek         = noop_llseek,
        .read           = eq3loop_read,
        .write          = eq3loop_write,
        .open           = eq3loop_open,
diff --git a/kernel/fake_hmrf.c b/kernel/fake_hmrf.c
index 85f84de..cf3a040 100644
--- a/kernel/fake_hmrf.c
+++ b/kernel/fake_hmrf.c
@@ -61,7 +61,7 @@ static void fake_hmrf_add_to_buffer(char *buf, size_t len);
 static struct file_operations fake_hmrf_fops =
     {
         .owner = THIS_MODULE,
-        .llseek = no_llseek,
+        .llseek = noop_llseek,
         .read = fake_hmrf_read,
         .write = fake_hmrf_write,
         .open = fake_hmrf_open,
diff --git a/kernel/generic_raw_uart.c b/kernel/generic_raw_uart.c
index e227333..e1f9c55 100644
--- a/kernel/generic_raw_uart.c
+++ b/kernel/generic_raw_uart.c
@@ -147,7 +147,7 @@ static int generic_raw_uart_get_device_type(struct generic_raw_uart_instance *in
 static struct file_operations generic_raw_uart_fops =
 {
   .owner = THIS_MODULE,
-  .llseek = no_llseek,
+  .llseek = noop_llseek,
   .read = generic_raw_uart_read,
   .write = generic_raw_uart_write,
   .open = generic_raw_uart_open,

@df8oe
Copy link

df8oe commented Nov 28, 2024

I can confirm that modules build on Archlinux (aarch64) Kernel 6.12. But are you sure, no_llseek and noop_llseek are the same in this context? As I understood with no_llseek you can test if llseek is possible at all, while noop_llseek does not check this?

@dkadioglu
Copy link

I can confirm that modules build on Archlinux (aarch64) Kernel 6.12. But are you sure, no_llseek and noop_llseek are the same in this context? As I understood with no_llseek you can test if llseek is possible at all, while noop_llseek does not check this?

Thanks for your feedback. You're maybe right, I looked through the kernel commits and found this: torvalds/linux@cb787f4
According to this, .no_llseek has been set to NULL two years ago and has now been removed. To my understanding of torvalds/linux@868941b setting .llseek to .no_llseek is not necessary any more. However, I am absolutely no expert here. The patch should then look like:

diff --git a/kernel/eq3_char_loop.c b/kernel/eq3_char_loop.c
index 201cde4..a71098c 100644
--- a/kernel/eq3_char_loop.c
+++ b/kernel/eq3_char_loop.c
@@ -917,7 +916,6 @@ static int eq3loop_open(struct inode *inode, struct file *filp)
 
 static struct file_operations eq3loop_fops = {
        .owner          = THIS_MODULE,
-       .llseek         = no_llseek,
        .read           = eq3loop_read,
        .write          = eq3loop_write,
        .open           = eq3loop_open,
diff --git a/kernel/fake_hmrf.c b/kernel/fake_hmrf.c
index 85f84de..cf3a040 100644
--- a/kernel/fake_hmrf.c
+++ b/kernel/fake_hmrf.c
@@ -61,7 +60,6 @@ static void fake_hmrf_add_to_buffer(char *buf, size_t len);
 static struct file_operations fake_hmrf_fops =
     {
         .owner = THIS_MODULE,
-        .llseek = no_llseek,
         .read = fake_hmrf_read,
         .write = fake_hmrf_write,
         .open = fake_hmrf_open,
diff --git a/kernel/generic_raw_uart.c b/kernel/generic_raw_uart.c
index e227333..e1f9c55 100644
--- a/kernel/generic_raw_uart.c
+++ b/kernel/generic_raw_uart.c
@@ -147,7 +146,6 @@ static int generic_raw_uart_get_device_type(struct generic_raw_uart_instance *in
 static struct file_operations generic_raw_uart_fops =
 {
   .owner = THIS_MODULE,
-  .llseek = no_llseek,
   .read = generic_raw_uart_read,
   .write = generic_raw_uart_write,
   .open = generic_raw_uart_open,
diff --git a/kernel/generic_raw_uart.h b/kernel/generic_raw_uart.h
index b865e22..791e5d0 100644

@df8oe df8oe mentioned this pull request Nov 28, 2024
@df8oe
Copy link

df8oe commented Nov 28, 2024

I think alexreinert can light up the darkness here :) I will test the modules this weekend for function with my raspberrymatic in a docker and will give feedback. I also never used no_llseek but function description of the both look potentially "getting issues"... We will see.

@df8oe
Copy link

df8oe commented Nov 28, 2024

Update:
I have tested the replacement of no_llseek with noop_llseek with my raspberrymatic. I have many rf modules, connected via hb_rf_eth, and many wired devices connected via eth gateway. At a first short test of different devices everything ist working fine. I will stay at this configuration and watch if something unexpected happens. At the moment: many thanks for the work of @charlie98 and @dkadioglu

@df8oe
Copy link

df8oe commented Dec 5, 2024

I can confirm that everything is working well. I have removed all lines with .llseek = no_llseek,

@dkadioglu
Copy link

I can also confirm that it's working under kernel 6.12 without issues - at least nothing obvious. I removed the lines with .llseek = no_llseek, too.

@nijave
Copy link

nijave commented Dec 29, 2024

Thanks for this PR, I've used a similar version to get this buildroot project working https://github.com/home-assistant/operating-system/pull/3767/files#diff-bb010bc93b85377ba08d15223fc4f926fe3fe4c0d3f291ace6138cf2160c4795

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants