| Message ID | 20240501084110.4165-2-shresthprasad7@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,next] tty: sunsu: Simplify device_node cleanup by using __free | expand |
On Thu, May 02, 2024 at 10:21:16PM +0530, Shresth Prasad wrote: > On Thu, May 2, 2024 at 9:35 PM Ilpo Järvinen > <ilpo.jarvinen@linux.intel.com> wrote: > > > > On Wed, 1 May 2024, Shresth Prasad wrote: > > > > > Add `__free` function attribute to `ap` and `match` pointer > > > initialisations which ensure that the pointers are freed as soon as they > > > go out of scope, thus removing the need to manually free them using > > > `of_node_put`. > > > > > > This also removes the need for the `goto` statement and the `rc` > > > variable. > > > > > > Tested using a qemu x86_64 virtual machine. > > > > Eh, how can you test this with an x86_64 VM ??? > > > > config SERIAL_SUNSU > > tristate "Sun SU serial support" > > depends on SPARC && PCI > > > > By that, I mean that I compiled the kernel and ran the produced bzImage > on a x86_64 qemu machine. But you didn't include the driver you were testing :( > I unfortunately don't have the hardware to test it on, but I don't > think the change is complex enough to require testing on real hardware > (unless I'm assuming incorrectly). That's why I asked if you had tested this or not...
On Fri, May 3, 2024 at 11:04 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, May 02, 2024 at 10:21:16PM +0530, Shresth Prasad wrote: > > On Thu, May 2, 2024 at 9:35 PM Ilpo Järvinen > > <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > On Wed, 1 May 2024, Shresth Prasad wrote: > > > > > > > Add `__free` function attribute to `ap` and `match` pointer > > > > initialisations which ensure that the pointers are freed as soon as they > > > > go out of scope, thus removing the need to manually free them using > > > > `of_node_put`. > > > > > > > > This also removes the need for the `goto` statement and the `rc` > > > > variable. > > > > > > > > Tested using a qemu x86_64 virtual machine. > > > > > > Eh, how can you test this with an x86_64 VM ??? > > > > > > config SERIAL_SUNSU > > > tristate "Sun SU serial support" > > > depends on SPARC && PCI > > > > > > > By that, I mean that I compiled the kernel and ran the produced bzImage > > on a x86_64 qemu machine. > > But you didn't include the driver you were testing :( > > > I unfortunately don't have the hardware to test it on, but I don't > > think the change is complex enough to require testing on real hardware > > (unless I'm assuming incorrectly). > > That's why I asked if you had tested this or not... > Really sorry about that, I thought compiling and booting would qualify as testing. What should I be doing then? Regards, Shresth
On Sat, May 4, 2024 at 9:32 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, May 03, 2024 at 02:31:22PM +0530, Shresth Prasad wrote: > > On Fri, May 3, 2024 at 11:04 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Thu, May 02, 2024 at 10:21:16PM +0530, Shresth Prasad wrote: > > > > On Thu, May 2, 2024 at 9:35 PM Ilpo Järvinen > > > > <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > > > > > On Wed, 1 May 2024, Shresth Prasad wrote: > > > > > > > > > > > Add `__free` function attribute to `ap` and `match` pointer > > > > > > initialisations which ensure that the pointers are freed as soon as they > > > > > > go out of scope, thus removing the need to manually free them using > > > > > > `of_node_put`. > > > > > > > > > > > > This also removes the need for the `goto` statement and the `rc` > > > > > > variable. > > > > > > > > > > > > Tested using a qemu x86_64 virtual machine. > > > > > > > > > > Eh, how can you test this with an x86_64 VM ??? > > > > > > > > > > config SERIAL_SUNSU > > > > > tristate "Sun SU serial support" > > > > > depends on SPARC && PCI > > > > > > > > > > > > > By that, I mean that I compiled the kernel and ran the produced bzImage > > > > on a x86_64 qemu machine. > > > > > > But you didn't include the driver you were testing :( > > > > > > > I unfortunately don't have the hardware to test it on, but I don't > > > > think the change is complex enough to require testing on real hardware > > > > (unless I'm assuming incorrectly). > > > > > > That's why I asked if you had tested this or not... > > > > > > > Really sorry about that, I thought compiling and booting would qualify > > as testing. What should I be doing then? > > Compiling and booting the code you change would be a good start :) > > thanks, > > greg k-h I've managed to successfully cross compile the kernel for sparc, along with this module, but I couldn't figure out how to boot the generated kernel image. I looked around for quite a while but couldn't find any straightforward guide suggesting how I would go about booting my own kernel on qemu-system-sparc. I would really appreciate it if you could point me in the right direction. Regards, Shresth
diff --git a/drivers/tty/serial/sunsu.c b/drivers/tty/serial/sunsu.c index 67a5fc70bb4b..0f463da5e7ce 100644 --- a/drivers/tty/serial/sunsu.c +++ b/drivers/tty/serial/sunsu.c @@ -1382,44 +1382,29 @@ static inline struct console *SUNSU_CONSOLE(void) static enum su_type su_get_type(struct device_node *dp) { - struct device_node *ap = of_find_node_by_path("/aliases"); - enum su_type rc = SU_PORT_PORT; + struct device_node *ap __free(device_node) = + of_find_node_by_path("/aliases"); if (ap) { const char *keyb = of_get_property(ap, "keyboard", NULL); const char *ms = of_get_property(ap, "mouse", NULL); - struct device_node *match; if (keyb) { - match = of_find_node_by_path(keyb); + struct device_node *match __free(device_node) = + of_find_node_by_path(keyb); - /* - * The pointer is used as an identifier not - * as a pointer, we can drop the refcount on - * the of__node immediately after getting it. - */ - of_node_put(match); - - if (dp == match) { - rc = SU_PORT_KBD; - goto out; - } + if (dp == match) + return SU_PORT_KBD; } if (ms) { - match = of_find_node_by_path(ms); + struct device_node *match __free(device_node) = + of_find_node_by_path(ms); - of_node_put(match); - - if (dp == match) { - rc = SU_PORT_MS; - goto out; - } + if (dp == match) + return SU_PORT_MS; } } - -out: - of_node_put(ap); - return rc; + return SU_PORT_PORT; } static int su_probe(struct platform_device *op)
Add `__free` function attribute to `ap` and `match` pointer initialisations which ensure that the pointers are freed as soon as they go out of scope, thus removing the need to manually free them using `of_node_put`. This also removes the need for the `goto` statement and the `rc` variable. Tested using a qemu x86_64 virtual machine. Suggested-by: Julia Lawall <julia.lawall@inria.fr> Signed-off-by: Shresth Prasad <shresthprasad7@gmail.com> --- Changes in v2: - Specify how the patch was tested drivers/tty/serial/sunsu.c | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-)