[PATCH 0/6] s390: Unify IUCV device allocation

[PATCH 0/6] s390: Unify IUCV device allocation

[Heiko Carstens]

Unify IUCV device allocation as suggested by Arnd Bergmann in order
to get rid of code duplication in various device drivers.

This also removes various warnings caused by
-Wcast-function-type-strict as reported by Nathan Lynch.

Unless there are objections I think this whole series should go via
the s390 tree.

Heiko Carstens (6):
  s390/iucv: Provide iucv_alloc_device() / iucv_release_device()
  s390/vmlogrdr: Make use of iucv_alloc_device()
  s390/netiucv: Make use of iucv_alloc_device()
  s390/smsgiucv_app: Make use of iucv_alloc_device()
  tty: hvc-iucv: Make use of iucv_alloc_device()
  s390/iucv: Unexport iucv_root

 drivers/s390/char/vmlogrdr.c    | 20 +++--------------
 drivers/s390/net/netiucv.c      | 20 ++++-------------
 drivers/s390/net/smsgiucv_app.c | 21 +++++-------------
 drivers/tty/hvc/hvc_iucv.c      | 15 ++-----------
 include/net/iucv/iucv.h         |  7 +++++-
 net/iucv/iucv.c                 | 38 +++++++++++++++++++++++++++++++--
 6 files changed, 56 insertions(+), 65 deletions(-)

-- 
2.40.1

[PATCH 1/6] s390/iucv: Provide iucv_alloc_device() / iucv_release_device()

[Heiko Carstens]

Provide iucv_alloc_device() and iucv_release_device() helper functions,
which can be used to deduplicate more or less identical IUCV device
allocation and release code in four different drivers.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 include/net/iucv/iucv.h |  6 ++++++
 net/iucv/iucv.c         | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/net/iucv/iucv.h b/include/net/iucv/iucv.h
index 5cd7871127c9..b3736e66fe1a 100644
--- a/include/net/iucv/iucv.h
+++ b/include/net/iucv/iucv.h
@@ -84,6 +84,12 @@ struct iucv_array {
 extern const struct bus_type iucv_bus;
 extern struct device *iucv_root;
 
+struct device_driver;
+
+struct device *iucv_alloc_device(const struct attribute_group **attrs,
+				 struct device_driver *driver, void *priv,
+				 const char *fmt, ...) __printf(4, 5);
+
 /*
  * struct iucv_path
  * pathid: 16 bit path identification
diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c
index a4ab615ca3e3..9db7c2c0ae72 100644
--- a/net/iucv/iucv.c
+++ b/net/iucv/iucv.c
@@ -76,6 +76,41 @@ EXPORT_SYMBOL(iucv_bus);
 struct device *iucv_root;
 EXPORT_SYMBOL(iucv_root);
 
+static void iucv_release_device(struct device *device)
+{
+	kfree(device);
+}
+
+struct device *iucv_alloc_device(const struct attribute_group **attrs,
+				 struct device_driver *driver,
+				 void *priv, const char *fmt, ...)
+{
+	struct device *dev;
+	va_list vargs;
+	int rc;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		goto out_error;
+	va_start(vargs, fmt);
+	rc = dev_set_name(dev, fmt, vargs);
+	va_end(vargs);
+	if (rc)
+		goto out_error;
+	dev->bus = &iucv_bus;
+	dev->parent = iucv_root;
+	dev->driver = driver;
+	dev->groups = attrs;
+	dev->release = iucv_release_device;
+	dev_set_drvdata(dev, priv);
+	return dev;
+
+out_error:
+	kfree(dev);
+	return NULL;
+}
+EXPORT_SYMBOL(iucv_alloc_device);
+
 static int iucv_available;
 
 /* General IUCV interrupt structure */
-- 
2.40.1

[PATCH 2/6] s390/vmlogrdr: Make use of iucv_alloc_device()

[Heiko Carstens]

Make use of iucv_alloc_device() to get rid of quite some code. In addition
this also removes a cast to an incompatible function (clang W=1):

  drivers/s390/char/vmlogrdr.c:746:18: error: cast from 'void (*)(const void *)' to 'void (*)(struct device *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
    746 |                 dev->release = (void (*)(struct device *))kfree;
        |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/r/20240417-s390-drivers-fix-cast-function-type-v1-1-fd048c9903b0@kernel.org
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/char/vmlogrdr.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/s390/char/vmlogrdr.c b/drivers/s390/char/vmlogrdr.c
index d7e408c8d0b8..c09e1e09fb66 100644
--- a/drivers/s390/char/vmlogrdr.c
+++ b/drivers/s390/char/vmlogrdr.c
@@ -728,23 +728,9 @@ static int vmlogrdr_register_device(struct vmlogrdr_priv_t *priv)
 	struct device *dev;
 	int ret;
 
-	dev = kzalloc(sizeof(struct device), GFP_KERNEL);
-	if (dev) {
-		dev_set_name(dev, "%s", priv->internal_name);
-		dev->bus = &iucv_bus;
-		dev->parent = iucv_root;
-		dev->driver = &vmlogrdr_driver;
-		dev->groups = vmlogrdr_attr_groups;
-		dev_set_drvdata(dev, priv);
-		/*
-		 * The release function could be called after the
-		 * module has been unloaded. It's _only_ task is to
-		 * free the struct. Therefore, we specify kfree()
-		 * directly here. (Probably a little bit obfuscating
-		 * but legitime ...).
-		 */
-		dev->release = (void (*)(struct device *))kfree;
-	} else
+	dev = iucv_alloc_device(vmlogrdr_attr_groups, &vmlogrdr_driver,
+				priv, priv->internal_name);
+	if (!dev)
 		return -ENOMEM;
 	ret = device_register(dev);
 	if (ret) {
-- 
2.40.1

[PATCH 3/6] s390/netiucv: Make use of iucv_alloc_device()

[Heiko Carstens]

Make use of iucv_alloc_device() to get rid of quite some code. In addition
this also removes a cast to an incompatible function (clang W=1):

  drivers/s390/net/netiucv.c:1716:18: error: cast from 'void (*)(const void *)' to 'void (*)(struct device *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
   1716 |                 dev->release = (void (*)(struct device *))kfree;
        |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/r/20240417-s390-drivers-fix-cast-function-type-v1-3-fd048c9903b0@kernel.org
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/net/netiucv.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/s390/net/netiucv.c b/drivers/s390/net/netiucv.c
index 8852b03f943b..039e18d46f76 100644
--- a/drivers/s390/net/netiucv.c
+++ b/drivers/s390/net/netiucv.c
@@ -1696,26 +1696,14 @@ static const struct attribute_group *netiucv_attr_groups[] = {
 static int netiucv_register_device(struct net_device *ndev)
 {
 	struct netiucv_priv *priv = netdev_priv(ndev);
-	struct device *dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+	struct device *dev;
 	int ret;
 
 	IUCV_DBF_TEXT(trace, 3, __func__);
 
-	if (dev) {
-		dev_set_name(dev, "net%s", ndev->name);
-		dev->bus = &iucv_bus;
-		dev->parent = iucv_root;
-		dev->groups = netiucv_attr_groups;
-		/*
-		 * The release function could be called after the
-		 * module has been unloaded. It's _only_ task is to
-		 * free the struct. Therefore, we specify kfree()
-		 * directly here. (Probably a little bit obfuscating
-		 * but legitime ...).
-		 */
-		dev->release = (void (*)(struct device *))kfree;
-		dev->driver = &netiucv_driver;
-	} else
+	dev = iucv_alloc_device(netiucv_attr_groups, &netiucv_driver, NULL,
+				"net%s", ndev->name);
+	if (!dev)
 		return -ENOMEM;
 
 	ret = device_register(dev);
-- 
2.40.1

[PATCH 4/6] s390/smsgiucv_app: Make use of iucv_alloc_device()

[Heiko Carstens]

Make use of iucv_alloc_device() to get rid of quite some code. In addition
this also removes a cast to an incompatible function (clang W=1):

  drivers/s390/net/smsgiucv_app.c:176:26: error: cast from 'void (*)(const void *)' to 'void (*)(struct device *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
    176 |         smsg_app_dev->release = (void (*)(struct device *)) kfree;
        |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/r/20240417-s390-drivers-fix-cast-function-type-v1-2-fd048c9903b0@kernel.org
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/net/smsgiucv_app.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/s390/net/smsgiucv_app.c b/drivers/s390/net/smsgiucv_app.c
index 0a263999f7ae..cc44cbb0028b 100644
--- a/drivers/s390/net/smsgiucv_app.c
+++ b/drivers/s390/net/smsgiucv_app.c
@@ -156,25 +156,14 @@ static int __init smsgiucv_app_init(void)
 	if (!MACHINE_IS_VM)
 		return -ENODEV;
 
-	smsg_app_dev = kzalloc(sizeof(*smsg_app_dev), GFP_KERNEL);
+	smsgiucv_drv = driver_find(SMSGIUCV_DRV_NAME, &iucv_bus);
+	if (!smsgiucv_drv)
+		return -ENODEV;
+
+	smsg_app_dev = iucv_alloc_device(NULL, smsgiucv_drv, NULL, KMSG_COMPONENT);
 	if (!smsg_app_dev)
 		return -ENOMEM;
 
-	smsgiucv_drv = driver_find(SMSGIUCV_DRV_NAME, &iucv_bus);
-	if (!smsgiucv_drv) {
-		kfree(smsg_app_dev);
-		return -ENODEV;
-	}
-
-	rc = dev_set_name(smsg_app_dev, KMSG_COMPONENT);
-	if (rc) {
-		kfree(smsg_app_dev);
-		goto fail;
-	}
-	smsg_app_dev->bus = &iucv_bus;
-	smsg_app_dev->parent = iucv_root;
-	smsg_app_dev->release = (void (*)(struct device *)) kfree;
-	smsg_app_dev->driver = smsgiucv_drv;
 	rc = device_register(smsg_app_dev);
 	if (rc) {
 		put_device(smsg_app_dev);
-- 
2.40.1

[PATCH 5/6] tty: hvc-iucv: Make use of iucv_alloc_device()

[Heiko Carstens]

Make use of iucv_alloc_device() to get rid of quite some code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/tty/hvc/hvc_iucv.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/hvc/hvc_iucv.c b/drivers/tty/hvc/hvc_iucv.c
index b1149bc62ca1..ed4bf40278a7 100644
--- a/drivers/tty/hvc/hvc_iucv.c
+++ b/drivers/tty/hvc/hvc_iucv.c
@@ -1035,11 +1035,6 @@ static const struct attribute_group *hvc_iucv_dev_attr_groups[] = {
 	NULL,
 };
 
-static void hvc_iucv_free(struct device *data)
-{
-	kfree(data);
-}
-
 /**
  * hvc_iucv_alloc() - Allocates a new struct hvc_iucv_private instance
  * @id:			hvc_iucv_table index
@@ -1090,18 +1085,12 @@ static int __init hvc_iucv_alloc(int id, unsigned int is_console)
 	memcpy(priv->srv_name, name, 8);
 	ASCEBC(priv->srv_name, 8);
 
-	/* create and setup device */
-	priv->dev = kzalloc(sizeof(*priv->dev), GFP_KERNEL);
+	priv->dev = iucv_alloc_device(hvc_iucv_dev_attr_groups, NULL,
+				      priv, "hvc_iucv%d", id);
 	if (!priv->dev) {
 		rc = -ENOMEM;
 		goto out_error_dev;
 	}
-	dev_set_name(priv->dev, "hvc_iucv%d", id);
-	dev_set_drvdata(priv->dev, priv);
-	priv->dev->bus = &iucv_bus;
-	priv->dev->parent = iucv_root;
-	priv->dev->groups = hvc_iucv_dev_attr_groups;
-	priv->dev->release = hvc_iucv_free;
 	rc = device_register(priv->dev);
 	if (rc) {
 		put_device(priv->dev);
-- 
2.40.1

Re: [PATCH 5/6] tty: hvc-iucv: Make use of iucv_alloc_device()

[Greg Kroah-Hartman]

On Mon, May 06, 2024 at 09:44:53PM +0200, Heiko Carstens wrote:
> Make use of iucv_alloc_device() to get rid of quite some code.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  drivers/tty/hvc/hvc_iucv.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 6/6] s390/iucv: Unexport iucv_root

[Heiko Carstens]

There is no user of iucv_root outside of the core IUCV code left.
Therefore remove the EXPORT_SYMBOL.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 include/net/iucv/iucv.h | 1 -
 net/iucv/iucv.c         | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/net/iucv/iucv.h b/include/net/iucv/iucv.h
index b3736e66fe1a..4d114e6d6d23 100644
--- a/include/net/iucv/iucv.h
+++ b/include/net/iucv/iucv.h
@@ -82,7 +82,6 @@ struct iucv_array {
 } __attribute__ ((aligned (8)));
 
 extern const struct bus_type iucv_bus;
-extern struct device *iucv_root;
 
 struct device_driver;
 
diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c
index 9db7c2c0ae72..2e61f19621ee 100644
--- a/net/iucv/iucv.c
+++ b/net/iucv/iucv.c
@@ -73,8 +73,7 @@ const struct bus_type iucv_bus = {
 };
 EXPORT_SYMBOL(iucv_bus);
 
-struct device *iucv_root;
-EXPORT_SYMBOL(iucv_root);
+static struct device *iucv_root;
 
 static void iucv_release_device(struct device *device)
 {
-- 
2.40.1

Re: [PATCH 0/6] s390: Unify IUCV device allocation

[Alexandra Winter]

On 06.05.24 21:44, Heiko Carstens wrote:
> Unify IUCV device allocation as suggested by Arnd Bergmann in order
> to get rid of code duplication in various device drivers.
> 
> This also removes various warnings caused by
> -Wcast-function-type-strict as reported by Nathan Lynch.
> 
> Unless there are objections I think this whole series should go via
> the s390 tree.
> 
> Heiko Carstens (6):
>   s390/iucv: Provide iucv_alloc_device() / iucv_release_device()
>   s390/vmlogrdr: Make use of iucv_alloc_device()
>   s390/netiucv: Make use of iucv_alloc_device()
>   s390/smsgiucv_app: Make use of iucv_alloc_device()
>   tty: hvc-iucv: Make use of iucv_alloc_device()
>   s390/iucv: Unexport iucv_root
> 
>  drivers/s390/char/vmlogrdr.c    | 20 +++--------------
>  drivers/s390/net/netiucv.c      | 20 ++++-------------
>  drivers/s390/net/smsgiucv_app.c | 21 +++++-------------
>  drivers/tty/hvc/hvc_iucv.c      | 15 ++-----------
>  include/net/iucv/iucv.h         |  7 +++++-
>  net/iucv/iucv.c                 | 38 +++++++++++++++++++++++++++++++--
>  6 files changed, 56 insertions(+), 65 deletions(-)
> 

Thank you, Heiko.
For the series:
Acked-by: Alexandra Winter <wintera@linux.ibm.com>

Re: [PATCH 0/6] s390: Unify IUCV device allocation

[Heiko Carstens]

On Mon, May 06, 2024 at 09:44:48PM +0200, Heiko Carstens wrote:
> Unify IUCV device allocation as suggested by Arnd Bergmann in order
> to get rid of code duplication in various device drivers.
> 
> This also removes various warnings caused by
> -Wcast-function-type-strict as reported by Nathan Lynch.
                                             ^^^^^^^^^^^^

Ahem :)

This should have been Nathan Chancellor, of course. Sorry for this!

Re: [PATCH 0/6] s390: Unify IUCV device allocation

[Nathan Chancellor]

On Tue, May 07, 2024 at 02:32:20PM +0200, Heiko Carstens wrote:
> On Mon, May 06, 2024 at 09:44:48PM +0200, Heiko Carstens wrote:
> > Unify IUCV device allocation as suggested by Arnd Bergmann in order
> > to get rid of code duplication in various device drivers.
> > 
> > This also removes various warnings caused by
> > -Wcast-function-type-strict as reported by Nathan Lynch.
>                                              ^^^^^^^^^^^^
> 
> Ahem :)
> 
> This should have been Nathan Chancellor, of course. Sorry for this!

Heh, at least you got it right on the patches :)

Cheers,
Nathan