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
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 <[email protected]>
Closes: https://lore.kernel.org/r/20240417-s390-drivers-fix-cast-function-type-v1-2-fd048c9903b0@kernel.org
Signed-off-by: Heiko Carstens <[email protected]>
---
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
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 <[email protected]>
Closes: https://lore.kernel.org/r/20240417-s390-drivers-fix-cast-function-type-v1-3-fd048c9903b0@kernel.org
Signed-off-by: Heiko Carstens <[email protected]>
---
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
Make use of iucv_alloc_device() to get rid of quite some code.
Signed-off-by: Heiko Carstens <[email protected]>
---
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
There is no user of iucv_root outside of the core IUCV code left.
Therefore remove the EXPORT_SYMBOL.
Signed-off-by: Heiko Carstens <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
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
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 <[email protected]>
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!
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
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 <[email protected]>
> ---
> drivers/tty/hvc/hvc_iucv.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
Acked-by: Greg Kroah-Hartman <[email protected]>