diff mbox series

[v2,next] tty: sunsu: Simplify device_node cleanup by using __free

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

Commit Message

Shresth Prasad May 1, 2024, 8:41 a.m. UTC
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(-)

Comments

Greg KH May 3, 2024, 5:34 a.m. UTC | #1
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...
Shresth Prasad May 3, 2024, 9:01 a.m. UTC | #2
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
Shresth Prasad May 14, 2024, 7:37 p.m. UTC | #3
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 mbox series

Patch

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)