[PATCH 0/7] replace cdev_by_name() with cdev_open_by_name()

[PATCH 0/7] replace cdev_by_name() with cdev_open_by_name()

[Sascha Hauer]

As Ahmad noticed cdev_by_name() doesn't increase the reference counting
on the returned cdev which causes problems further down the road. This
series replaces some of the easier occurences of cdev_by_name() with
cdev_open_by_name().

Sascha Hauer (7):
  ARM: Freescale i.MX23 evk: use cdev_open_by_name()
  ARM: tx28: use cdev_open_by_name()
  ARM: omap: use cdev_open_by_name()
  ARM: Rockchip: use cdev_open_by_name()
  commands: devlookup: use cdev_open_by_name()
  commands: findmnt: use cdev_open_by_name()
  bootm: use cdev_open_by_name()

 arch/arm/boards/freescale-mx23-evk/mx23-evk.c | 13 +++++++------
 arch/arm/boards/karo-tx28/tx28-stk5.c         | 13 ++++++++-----
 arch/arm/mach-omap/omap_generic.c             |  5 ++++-
 arch/arm/mach-rockchip/bbu.c                  |  8 +++++++-
 commands/devlookup.c                          | 15 ++++++++++-----
 commands/findmnt.c                            |  3 ++-
 common/bootm.c                                |  5 ++++-
 7 files changed, 42 insertions(+), 20 deletions(-)

-- 
2.39.2

[PATCH 1/7] ARM: Freescale i.MX23 evk: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/freescale-mx23-evk/mx23-evk.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boards/freescale-mx23-evk/mx23-evk.c b/arch/arm/boards/freescale-mx23-evk/mx23-evk.c
index d4de99eafb..b10a6e8dcd 100644
--- a/arch/arm/boards/freescale-mx23-evk/mx23-evk.c
+++ b/arch/arm/boards/freescale-mx23-evk/mx23-evk.c
@@ -52,22 +52,21 @@ static struct fsl_usb2_platform_data usb_pdata = {
  */
 static int register_persistent_environment(void)
 {
-	struct cdev *cdev;
+	struct cdev *cdev, *env;
 
 	/*
 	 * The imx23-olinuxino only has one MCI card socket.
 	 * So, we expect its name as "disk0".
 	 */
-	cdev = cdev_by_name("disk0");
+	cdev = cdev_open_by_name("disk0", O_RDONLY);
 	if (cdev == NULL) {
 		pr_err("No MCI card preset\n");
 		return -ENODEV;
 	}
-
-
+	cdev_close(cdev);
 
 	/* MCI card is present, also a useable partition on it? */
-	cdev = cdev_by_name("disk0.1");
+	cdev = cdev_open_by_name("disk0.1", O_RDONLY);
 	if (cdev == NULL) {
 		pr_err("No second partition available\n");
 		pr_info("Please create at least a second partition with"
@@ -76,8 +75,10 @@ static int register_persistent_environment(void)
 	}
 
 	/* use the full partition as our persistent environment storage */
-	cdev = devfs_add_partition("disk0.1", 0, cdev->size,
+	env = devfs_add_partition("disk0.1", 0, cdev->size,
 						DEVFS_PARTITION_FIXED, "env0");
+	cdev_close(cdev);
+
 	return PTR_ERR_OR_ZERO(cdev);
 }
 
-- 
2.39.2

[PATCH 2/7] ARM: tx28: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/karo-tx28/tx28-stk5.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boards/karo-tx28/tx28-stk5.c b/arch/arm/boards/karo-tx28/tx28-stk5.c
index d1fd526c00..01a14a33a3 100644
--- a/arch/arm/boards/karo-tx28/tx28-stk5.c
+++ b/arch/arm/boards/karo-tx28/tx28-stk5.c
@@ -310,20 +310,21 @@ static const uint32_t tx28_starterkit_pad_setup[] = {
  */
 static int register_persistent_environment(void)
 {
-	struct cdev *cdev;
+	struct cdev *cdev, *env;
 
 	/*
 	 * The TX28 STK5 has only one usable MCI card socket.
 	 * So, we expect its name as "disk0".
 	 */
-	cdev = cdev_by_name("disk0");
+	cdev = cdev_open_by_name("disk0", O_RDONLY);
 	if (cdev == NULL) {
 		pr_err("No MCI card preset\n");
 		return -ENODEV;
 	}
+	cdev_close(cdev);
 
 	/* MCI card is present, also a usable partition on it? */
-	cdev = cdev_by_name("disk0.1");
+	cdev = cdev_open_by_name("disk0.1", O_RDONLY);
 	if (cdev == NULL) {
 		pr_err("No second partition available\n");
 		pr_info("Please create at least a second partition with"
@@ -332,9 +333,11 @@ static int register_persistent_environment(void)
 	}
 
 	/* use the full partition as our persistent environment storage */
-	cdev = devfs_add_partition("disk0.1", 0, cdev->size,
+	env = devfs_add_partition("disk0.1", 0, cdev->size,
 					DEVFS_PARTITION_FIXED, "env0");
-	return PTR_ERR_OR_ZERO(cdev);
+	cdev_close(cdev);
+
+	return PTR_ERR_OR_ZERO(env);
 }
 
 static void tx28_get_ethaddr(void)
-- 
2.39.2

[PATCH 3/7] ARM: omap: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-omap/omap_generic.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap/omap_generic.c b/arch/arm/mach-omap/omap_generic.c
index 99e14fb540..112c806216 100644
--- a/arch/arm/mach-omap/omap_generic.c
+++ b/arch/arm/mach-omap/omap_generic.c
@@ -157,13 +157,16 @@ static int omap_env_init(void)
 
 	device_detect_by_name(diskdev);
 	partname = basprintf("%s.0", diskdev);
-	cdev = cdev_by_name(partname);
+	cdev = cdev_open_by_name(partname, O_RDONLY);
 	if (cdev == NULL) {
 		pr_err("Failed to get device %s\n", partname);
 		goto out;
 	}
 
 	rootpath = cdev_mount_default(cdev, NULL);
+
+	cdev_close(cdev);
+
 	if (IS_ERR(rootpath)) {
 		pr_err("Failed to load environment: mount %s failed (%ld)\n",
 						cdev->name, PTR_ERR(rootpath));
-- 
2.39.2

[PATCH 4/7] ARM: Rockchip: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-rockchip/bbu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/bbu.c b/arch/arm/mach-rockchip/bbu.c
index f15b32937c..7715540863 100644
--- a/arch/arm/mach-rockchip/bbu.c
+++ b/arch/arm/mach-rockchip/bbu.c
@@ -43,6 +43,7 @@ static int rockchip_bbu_mmc_handler(struct bbu_handler *handler,
 	int ret, fd, wr0, wr1;
 	loff_t space;
 	const char *cdevname;
+	struct cdev *cdev;
 
 	filetype = file_detect_type(data->image, data->len);
 	if (filetype != filetype_rockchip_rkns_image) {
@@ -60,7 +61,12 @@ static int rockchip_bbu_mmc_handler(struct bbu_handler *handler,
 	if (ret)
 		return ret;
 
-	space = cdev_unallocated_space(cdev_by_name(cdevname));
+	cdev = cdev_open_by_name(cdevname, O_RDONLY);
+	if (!cdev)
+		return -ENOENT;
+
+	space = cdev_unallocated_space(cdev);
+	cdev_close(cdev);
 
 	if (space < IMG_OFFSET_0 + data->len) {
 		if (!bbu_force(data, "Unallocated space on %s (%lld) is too small for one image\n",
-- 
2.39.2

[PATCH 5/7] commands: devlookup: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/devlookup.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/commands/devlookup.c b/commands/devlookup.c
index ffd6afbaba..bc9bd94614 100644
--- a/commands/devlookup.c
+++ b/commands/devlookup.c
@@ -25,7 +25,7 @@ static int do_devlookup(int argc, char *argv[])
 {
 	const char *variable = NULL, *devicefile, *paramname;
 	struct cdev *cdev;
-	int opt;
+	int opt, ret;
 
 	while ((opt = getopt(argc, argv, "v:")) > 0) {
 		switch(opt) {
@@ -43,7 +43,7 @@ static int do_devlookup(int argc, char *argv[])
 
 	devicefile = devpath_to_name(devicefile);
 
-	cdev = cdev_by_name(devicefile);
+	cdev = cdev_open_by_name(devicefile, O_RDONLY);
 	if (!cdev) {
 		printf("devlookup: cdev %s not found\n", devicefile);
 		return -ENOENT;
@@ -51,13 +51,18 @@ static int do_devlookup(int argc, char *argv[])
 
 	if (!cdev->dev) {
 		printf("devlookup: cdev %s not associated with a device\n", devicefile);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out;
 	}
 
 	if (paramname)
-		return report(variable, dev_get_param(cdev->dev, paramname));
+		ret = report(variable, dev_get_param(cdev->dev, paramname));
+	else
+		ret = report(variable, dev_name(cdev->dev));
+out:
+	cdev_close(cdev);
 
-	return report(variable, dev_name(cdev->dev));
+	return ret;
 }
 
 BAREBOX_CMD_HELP_START(devlookup)
-- 
2.39.2

[PATCH 6/7] commands: findmnt: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/findmnt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/commands/findmnt.c b/commands/findmnt.c
index da8f58835f..3c9a520b85 100644
--- a/commands/findmnt.c
+++ b/commands/findmnt.c
@@ -71,7 +71,7 @@ static int do_findmnt(int argc, char *argv[])
 			const char *backingstore;
 			struct cdev *cdev;
 
-			cdev = cdev_by_name(devpath_to_name(device));
+			cdev = cdev_open_by_name(devpath_to_name(device), O_RDONLY);
 			if (!cdev)
 				continue;
 
@@ -82,6 +82,7 @@ static int do_findmnt(int argc, char *argv[])
 				print_header(&header_printed);
 				report_findmnt(target);
 			}
+			cdev_close(cdev);
 		}
 	}
 
-- 
2.39.2

[PATCH 7/7] bootm: use cdev_open_by_name()

[Sascha Hauer]

cdev_by_name() returns a cdev without increasing its reference count. In
order to maintain a proper reference counting use cdev_open_by_name()
instead and make sure it's closed afterwards when no longer needed.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/bootm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/bootm.c b/common/bootm.c
index c851ab0456..357537c54e 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -760,7 +760,7 @@ int bootm_boot(struct bootm_data *bootm_data)
 
 		if (bootm_data->root_dev) {
 			const char *root_dev_name = devpath_to_name(bootm_data->root_dev);
-			const struct cdev *root_cdev = cdev_by_name(root_dev_name);
+			struct cdev *root_cdev = cdev_open_by_name(root_dev_name, O_RDONLY);
 
 			rootarg = cdev_get_linux_rootarg(root_cdev);
 			if (!rootarg) {
@@ -773,6 +773,9 @@ int bootm_boot(struct bootm_data *bootm_data)
 					pr_err("%s doesn't have a PARTUUID, cannot set root= option\n",
 						root_dev_name);
 			}
+
+			if (root_cdev)
+				cdev_close(root_cdev);
 		} else {
 			rootarg = path_get_linux_rootarg(data->os_file);
 		}
-- 
2.39.2

Re: [PATCH 0/7] replace cdev_by_name() with cdev_open_by_name()

[Marco Felsch]

Hi Sascha,

On 24-05-16, Sascha Hauer wrote:
> As Ahmad noticed cdev_by_name() doesn't increase the reference counting
> on the returned cdev which causes problems further down the road. This
> series replaces some of the easier occurences of cdev_by_name() with
> cdev_open_by_name().

Can we kindly inform the user with a print_once() or so that
cdev_by_name should be replaced with cdev_open_by_name()?

Regards,
  Marco

> Sascha Hauer (7):
>   ARM: Freescale i.MX23 evk: use cdev_open_by_name()
>   ARM: tx28: use cdev_open_by_name()
>   ARM: omap: use cdev_open_by_name()
>   ARM: Rockchip: use cdev_open_by_name()
>   commands: devlookup: use cdev_open_by_name()
>   commands: findmnt: use cdev_open_by_name()
>   bootm: use cdev_open_by_name()
> 
>  arch/arm/boards/freescale-mx23-evk/mx23-evk.c | 13 +++++++------
>  arch/arm/boards/karo-tx28/tx28-stk5.c         | 13 ++++++++-----
>  arch/arm/mach-omap/omap_generic.c             |  5 ++++-
>  arch/arm/mach-rockchip/bbu.c                  |  8 +++++++-
>  commands/devlookup.c                          | 15 ++++++++++-----
>  commands/findmnt.c                            |  3 ++-
>  common/bootm.c                                |  5 ++++-
>  7 files changed, 42 insertions(+), 20 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH 0/7] replace cdev_by_name() with cdev_open_by_name()

[Sascha Hauer]

On Thu, May 16, 2024 at 04:43:04PM +0200, Marco Felsch wrote:
> Hi Sascha,
> 
> On 24-05-16, Sascha Hauer wrote:
> > As Ahmad noticed cdev_by_name() doesn't increase the reference counting
> > on the returned cdev which causes problems further down the road. This
> > series replaces some of the easier occurences of cdev_by_name() with
> > cdev_open_by_name().
> 
> Can we kindly inform the user with a print_once() or so that
> cdev_by_name should be replaced with cdev_open_by_name()?

Some of the call sites of cdev_by_name() are used frequently on all
boards, so this wouldn't be practical.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/7] replace cdev_by_name() with cdev_open_by_name()

[Sascha Hauer]

On Thu, 16 May 2024 09:08:15 +0200, Sascha Hauer wrote:
> As Ahmad noticed cdev_by_name() doesn't increase the reference counting
> on the returned cdev which causes problems further down the road. This
> series replaces some of the easier occurences of cdev_by_name() with
> cdev_open_by_name().
> 
> Sascha Hauer (7):
>   ARM: Freescale i.MX23 evk: use cdev_open_by_name()
>   ARM: tx28: use cdev_open_by_name()
>   ARM: omap: use cdev_open_by_name()
>   ARM: Rockchip: use cdev_open_by_name()
>   commands: devlookup: use cdev_open_by_name()
>   commands: findmnt: use cdev_open_by_name()
>   bootm: use cdev_open_by_name()
> 
> [...]

Applied, thanks!

[1/7] ARM: Freescale i.MX23 evk: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=3111bf9b13d2 (link may not be stable)
[2/7] ARM: tx28: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=05164ce75799 (link may not be stable)
[3/7] ARM: omap: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=50788af27a89 (link may not be stable)
[4/7] ARM: Rockchip: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=9da41e4d65fc (link may not be stable)
[5/7] commands: devlookup: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=bc5868b647aa (link may not be stable)
[6/7] commands: findmnt: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=7aa92b02b84a (link may not be stable)
[7/7] bootm: use cdev_open_by_name()
      https://git.pengutronix.de/cgit/barebox/commit/?id=888a17fa7e95 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>