2019-07-31 12:54:52

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

This patch originally started out just as a way for platform drivers to
easily add a sysfs group in a race-free way, but thanks to Dmitry's
patch, this series now is for all drivers in the kernel (hey, a unified
driver model works!!!)

I've only converted a few platform drivers here in this series to show
how it works, but other busses can be converted after the first patch
goes into the tree.

Here's the original 00 message, for people to get an idea of what is
going on here:

If a platform driver wants to add a sysfs group, it has to do so in a
racy way, adding it after the driver is bound. To resolve this issue,
have the platform driver core do this for the driver, making the
individual drivers logic smaller and simpler, and solving the race at
the same time.

All of these patches depend on the first patch. I'll take the first one
through my driver-core tree, and any subsystem maintainer can either ack
their individul patch and I will be glad to also merge it, or they can
wait until after 5.4-rc1 when the core patch hits Linus's tree and then
take it, it's up to them.

Thank to Richard Gong for the idea and the testing of the platform
driver patch and to Dmitry Torokhov for rewriting the first patch to
work well for all busses.

-----

V2 - work for all busses and not just platform drivers.


Dmitry Torokhov (1):
driver core: add dev_groups to all drivers

Greg Kroah-Hartman (9):
uio: uio_fsl_elbc_gpcm: convert platform driver to use dev_groups
input: keyboard: gpio_keys: convert platform driver to use dev_groups
input: axp20x-pek: convert platform driver to use dev_groups
firmware: arm_scpi: convert platform driver to use dev_groups
olpc: x01: convert platform driver to use dev_groups
platform: x86: hp-wmi: convert platform driver to use dev_groups
video: fbdev: wm8505fb: convert platform driver to use dev_groups
video: fbdev: w100fb: convert platform driver to use dev_groups
video: fbdev: sm501fb: convert platform driver to use dev_groups

arch/x86/platform/olpc/olpc-xo1-sci.c | 17 ++++------
drivers/base/dd.c | 14 ++++++++
drivers/firmware/arm_scpi.c | 5 +--
drivers/input/keyboard/gpio_keys.c | 13 ++------
drivers/input/misc/axp20x-pek.c | 15 ++-------
drivers/platform/x86/hp-wmi.c | 47 +++++++--------------------
drivers/uio/uio_fsl_elbc_gpcm.c | 23 +++++--------
drivers/video/fbdev/sm501fb.c | 37 +++++----------------
drivers/video/fbdev/w100fb.c | 23 ++++++-------
drivers/video/fbdev/wm8505fb.c | 13 ++++----
include/linux/device.h | 3 ++
11 files changed, 76 insertions(+), 134 deletions(-)

--
2.22.0


2019-07-31 12:55:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 01/10] driver core: add dev_groups to all drivers

From: Dmitry Torokhov <[email protected]>

Add the ability for the driver core to create and remove a list of
attribute groups automatically when the device is bound/unbound from a
specific driver.

Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/dd.c | 14 ++++++++++++++
include/linux/device.h | 3 +++
2 files changed, 17 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 994a90747420..d811e60610d3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -554,9 +554,16 @@ static int really_probe(struct device *dev, struct device_driver *drv)
goto probe_failed;
}

+ if (device_add_groups(dev, drv->dev_groups)) {
+ dev_err(dev, "device_add_groups() failed\n");
+ goto dev_groups_failed;
+ }
+
if (test_remove) {
test_remove = false;

+ device_remove_groups(dev, drv->dev_groups);
+
if (dev->bus->remove)
dev->bus->remove(dev);
else if (drv->remove)
@@ -584,6 +591,11 @@ static int really_probe(struct device *dev, struct device_driver *drv)
drv->bus->name, __func__, dev_name(dev), drv->name);
goto done;

+dev_groups_failed:
+ if (dev->bus->remove)
+ dev->bus->remove(dev);
+ else if (drv->remove)
+ drv->remove(dev);
probe_failed:
if (dev->bus)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
@@ -1114,6 +1126,8 @@ static void __device_release_driver(struct device *dev, struct device *parent)

pm_runtime_put_sync(dev);

+ device_remove_groups(dev, drv->dev_groups);
+
if (dev->bus && dev->bus->remove)
dev->bus->remove(dev);
else if (drv->remove)
diff --git a/include/linux/device.h b/include/linux/device.h
index c330b75c6c57..98c00b71b598 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -262,6 +262,8 @@ enum probe_type {
* @resume: Called to bring a device from sleep mode.
* @groups: Default attributes that get created by the driver core
* automatically.
+ * @dev_groups: Additional attributes attached to device instance once the
+ * it is bound to the driver.
* @pm: Power management operations of the device which matched
* this driver.
* @coredump: Called when sysfs entry is written to. The device driver
@@ -296,6 +298,7 @@ struct device_driver {
int (*suspend) (struct device *dev, pm_message_t state);
int (*resume) (struct device *dev);
const struct attribute_group **groups;
+ const struct attribute_group **dev_groups;

const struct dev_pm_ops *pm;
void (*coredump) (struct device *dev);
--
2.22.0

2019-07-31 12:56:18

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 06/10] olpc: x01: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a lid sysfs file.

Cc: Darren Hart <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Acked-by: Andy Shevchenko <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
arch/x86/platform/olpc/olpc-xo1-sci.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/arch/x86/platform/olpc/olpc-xo1-sci.c b/arch/x86/platform/olpc/olpc-xo1-sci.c
index 25ce1b3b0732..99a28ce2244c 100644
--- a/arch/x86/platform/olpc/olpc-xo1-sci.c
+++ b/arch/x86/platform/olpc/olpc-xo1-sci.c
@@ -157,6 +157,12 @@ static ssize_t lid_wake_mode_set(struct device *dev,
static DEVICE_ATTR(lid_wake_mode, S_IWUSR | S_IRUGO, lid_wake_mode_show,
lid_wake_mode_set);

+static struct attribute *lid_attrs[] = {
+ &dev_attr_lid_wake_mode.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(lid);
+
/*
* Process all items in the EC's SCI queue.
*
@@ -510,17 +516,8 @@ static int setup_lid_switch(struct platform_device *pdev)
goto err_register;
}

- r = device_create_file(&lid_switch_idev->dev, &dev_attr_lid_wake_mode);
- if (r) {
- dev_err(&pdev->dev, "failed to create wake mode attr: %d\n", r);
- goto err_create_attr;
- }
-
return 0;

-err_create_attr:
- input_unregister_device(lid_switch_idev);
- lid_switch_idev = NULL;
err_register:
input_free_device(lid_switch_idev);
return r;
@@ -528,7 +525,6 @@ static int setup_lid_switch(struct platform_device *pdev)

static void free_lid_switch(void)
{
- device_remove_file(&lid_switch_idev->dev, &dev_attr_lid_wake_mode);
input_unregister_device(lid_switch_idev);
}

@@ -624,6 +620,7 @@ static int xo1_sci_remove(struct platform_device *pdev)
static struct platform_driver xo1_sci_driver = {
.driver = {
.name = "olpc-xo1-sci-acpi",
+ .dev_groups = lid_groups,
},
.probe = xo1_sci_probe,
.remove = xo1_sci_remove,
--
2.22.0

2019-07-31 12:56:25

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 07/10] platform: x86: hp-wmi: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a bunch of sysfs files.

Cc: Darren Hart <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Acked-by: Andy Shevchenko <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/platform/x86/hp-wmi.c | 47 +++++++++--------------------------
1 file changed, 12 insertions(+), 35 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 2521e45280b8..6bcbbb375401 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -502,6 +502,17 @@ static DEVICE_ATTR_RO(dock);
static DEVICE_ATTR_RO(tablet);
static DEVICE_ATTR_RW(postcode);

+static struct attribute *hp_wmi_attrs[] = {
+ &dev_attr_display.attr,
+ &dev_attr_hddtemp.attr,
+ &dev_attr_als.attr,
+ &dev_attr_dock.attr,
+ &dev_attr_tablet.attr,
+ &dev_attr_postcode.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(hp_wmi);
+
static void hp_wmi_notify(u32 value, void *context)
{
struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -678,16 +689,6 @@ static void hp_wmi_input_destroy(void)
input_unregister_device(hp_wmi_input_dev);
}

-static void cleanup_sysfs(struct platform_device *device)
-{
- device_remove_file(&device->dev, &dev_attr_display);
- device_remove_file(&device->dev, &dev_attr_hddtemp);
- device_remove_file(&device->dev, &dev_attr_als);
- device_remove_file(&device->dev, &dev_attr_dock);
- device_remove_file(&device->dev, &dev_attr_tablet);
- device_remove_file(&device->dev, &dev_attr_postcode);
-}
-
static int __init hp_wmi_rfkill_setup(struct platform_device *device)
{
int err, wireless;
@@ -858,8 +859,6 @@ static int __init hp_wmi_rfkill2_setup(struct platform_device *device)

static int __init hp_wmi_bios_setup(struct platform_device *device)
{
- int err;
-
/* clear detected rfkill devices */
wifi_rfkill = NULL;
bluetooth_rfkill = NULL;
@@ -869,35 +868,12 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
if (hp_wmi_rfkill_setup(device))
hp_wmi_rfkill2_setup(device);

- err = device_create_file(&device->dev, &dev_attr_display);
- if (err)
- goto add_sysfs_error;
- err = device_create_file(&device->dev, &dev_attr_hddtemp);
- if (err)
- goto add_sysfs_error;
- err = device_create_file(&device->dev, &dev_attr_als);
- if (err)
- goto add_sysfs_error;
- err = device_create_file(&device->dev, &dev_attr_dock);
- if (err)
- goto add_sysfs_error;
- err = device_create_file(&device->dev, &dev_attr_tablet);
- if (err)
- goto add_sysfs_error;
- err = device_create_file(&device->dev, &dev_attr_postcode);
- if (err)
- goto add_sysfs_error;
return 0;
-
-add_sysfs_error:
- cleanup_sysfs(device);
- return err;
}

static int __exit hp_wmi_bios_remove(struct platform_device *device)
{
int i;
- cleanup_sysfs(device);

for (i = 0; i < rfkill2_count; i++) {
rfkill_unregister(rfkill2[i].rfkill);
@@ -966,6 +942,7 @@ static struct platform_driver hp_wmi_driver = {
.driver = {
.name = "hp-wmi",
.pm = &hp_wmi_pm_ops,
+ .dev_groups = hp_wmi_groups,
},
.remove = __exit_p(hp_wmi_bios_remove),
};
--
2.22.0

2019-07-31 12:56:40

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 09/10] video: fbdev: w100fb: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a bunch of sysfs files.

Cc: Tony Prisk <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Acked-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/video/fbdev/w100fb.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/video/fbdev/w100fb.c b/drivers/video/fbdev/w100fb.c
index 597ffaa13cd2..3be07807edcd 100644
--- a/drivers/video/fbdev/w100fb.c
+++ b/drivers/video/fbdev/w100fb.c
@@ -164,6 +164,15 @@ static ssize_t fastpllclk_store(struct device *dev, struct device_attribute *att

static DEVICE_ATTR_RW(fastpllclk);

+static struct attribute *w100fb_attrs[] = {
+ &dev_attr_fastpllclk.attr,
+ &dev_attr_reg_read.attr,
+ &dev_attr_reg_write.attr,
+ &dev_attr_flip.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(w100fb);
+
/*
* Some touchscreens need hsync information from the video driver to
* function correctly. We export it here.
@@ -752,14 +761,6 @@ int w100fb_probe(struct platform_device *pdev)
goto out;
}

- err = device_create_file(&pdev->dev, &dev_attr_fastpllclk);
- err |= device_create_file(&pdev->dev, &dev_attr_reg_read);
- err |= device_create_file(&pdev->dev, &dev_attr_reg_write);
- err |= device_create_file(&pdev->dev, &dev_attr_flip);
-
- if (err != 0)
- fb_warn(info, "failed to register attributes (%d)\n", err);
-
fb_info(info, "%s frame buffer device\n", info->fix.id);
return 0;
out:
@@ -784,11 +785,6 @@ static int w100fb_remove(struct platform_device *pdev)
struct fb_info *info = platform_get_drvdata(pdev);
struct w100fb_par *par=info->par;

- device_remove_file(&pdev->dev, &dev_attr_fastpllclk);
- device_remove_file(&pdev->dev, &dev_attr_reg_read);
- device_remove_file(&pdev->dev, &dev_attr_reg_write);
- device_remove_file(&pdev->dev, &dev_attr_flip);
-
unregister_framebuffer(info);

vfree(par->saved_intmem);
@@ -1625,6 +1621,7 @@ static struct platform_driver w100fb_driver = {
.resume = w100fb_resume,
.driver = {
.name = "w100fb",
+ .dev_groups = w100fb_groups,
},
};

--
2.22.0

2019-07-31 12:56:48

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 02/10] uio: uio_fsl_elbc_gpcm: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a sysfs group of attributes.

Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/uio/uio_fsl_elbc_gpcm.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/uio/uio_fsl_elbc_gpcm.c b/drivers/uio/uio_fsl_elbc_gpcm.c
index 450e2f5c9b43..be8a6905f507 100644
--- a/drivers/uio/uio_fsl_elbc_gpcm.c
+++ b/drivers/uio/uio_fsl_elbc_gpcm.c
@@ -71,6 +71,13 @@ static ssize_t reg_store(struct device *dev, struct device_attribute *attr,
static DEVICE_ATTR(reg_br, 0664, reg_show, reg_store);
static DEVICE_ATTR(reg_or, 0664, reg_show, reg_store);

+static struct attribute *uio_fsl_elbc_gpcm_attrs[] = {
+ &dev_attr_reg_br.attr,
+ &dev_attr_reg_or.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(uio_fsl_elbc_gpcm);
+
static ssize_t reg_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -411,25 +418,12 @@ static int uio_fsl_elbc_gpcm_probe(struct platform_device *pdev)
/* store private data */
platform_set_drvdata(pdev, info);

- /* create sysfs files */
- ret = device_create_file(priv->dev, &dev_attr_reg_br);
- if (ret)
- goto out_err3;
- ret = device_create_file(priv->dev, &dev_attr_reg_or);
- if (ret)
- goto out_err4;
-
dev_info(priv->dev,
"eLBC/GPCM device (%s) at 0x%llx, bank %d, irq=%d\n",
priv->name, (unsigned long long)res.start, priv->bank,
irq != NO_IRQ ? irq : -1);

return 0;
-out_err4:
- device_remove_file(priv->dev, &dev_attr_reg_br);
-out_err3:
- platform_set_drvdata(pdev, NULL);
- uio_unregister_device(info);
out_err2:
if (priv->shutdown)
priv->shutdown(info, true);
@@ -448,8 +442,6 @@ static int uio_fsl_elbc_gpcm_remove(struct platform_device *pdev)
struct uio_info *info = platform_get_drvdata(pdev);
struct fsl_elbc_gpcm *priv = info->priv;

- device_remove_file(priv->dev, &dev_attr_reg_or);
- device_remove_file(priv->dev, &dev_attr_reg_br);
platform_set_drvdata(pdev, NULL);
uio_unregister_device(info);
if (priv->shutdown)
@@ -474,6 +466,7 @@ static struct platform_driver uio_fsl_elbc_gpcm_driver = {
.driver = {
.name = "fsl,elbc-gpcm-uio",
.of_match_table = uio_fsl_elbc_gpcm_match,
+ .dev_groups = uio_fsl_elbc_gpcm_groups,
},
.probe = uio_fsl_elbc_gpcm_probe,
.remove = uio_fsl_elbc_gpcm_remove,
--
2.22.0

2019-07-31 12:57:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 08/10] video: fbdev: wm8505fb: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a sysfs file.

Cc: Tony Prisk <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Acked-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/video/fbdev/wm8505fb.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/wm8505fb.c b/drivers/video/fbdev/wm8505fb.c
index ff752635a31c..17c780315ca5 100644
--- a/drivers/video/fbdev/wm8505fb.c
+++ b/drivers/video/fbdev/wm8505fb.c
@@ -176,6 +176,12 @@ static ssize_t contrast_store(struct device *dev,

static DEVICE_ATTR_RW(contrast);

+static struct attribute *wm8505fb_attrs[] = {
+ &dev_attr_contrast.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(wm8505fb);
+
static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
{
chan &= 0xffff;
@@ -361,10 +367,6 @@ static int wm8505fb_probe(struct platform_device *pdev)
return ret;
}

- ret = device_create_file(&pdev->dev, &dev_attr_contrast);
- if (ret < 0)
- fb_warn(&fbi->fb, "failed to register attributes (%d)\n", ret);
-
fb_info(&fbi->fb, "%s frame buffer at 0x%lx-0x%lx\n",
fbi->fb.fix.id, fbi->fb.fix.smem_start,
fbi->fb.fix.smem_start + fbi->fb.fix.smem_len - 1);
@@ -376,8 +378,6 @@ static int wm8505fb_remove(struct platform_device *pdev)
{
struct wm8505fb_info *fbi = platform_get_drvdata(pdev);

- device_remove_file(&pdev->dev, &dev_attr_contrast);
-
unregister_framebuffer(&fbi->fb);

writel(0, fbi->regbase);
@@ -399,6 +399,7 @@ static struct platform_driver wm8505fb_driver = {
.driver = {
.name = DRIVER_NAME,
.of_match_table = wmt_dt_ids,
+ .dev_groups = wm8505fb_groups,
},
};

--
2.22.0

2019-07-31 13:00:00

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] driver core: add dev_groups to all drivers

On Wed, Jul 31, 2019 at 02:49:26PM +0200, Takashi Iwai wrote:
> On Wed, 31 Jul 2019 14:43:40 +0200,
> Greg Kroah-Hartman wrote:
> >
> > From: Dmitry Torokhov <[email protected]>
> >
> > Add the ability for the driver core to create and remove a list of
> > attribute groups automatically when the device is bound/unbound from a
> > specific driver.
> >
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> Missing sign-off from Dmitry?

He never provided it :(

Dmitry, can you please do so? I forgot to include that in the cover
leter...

thanks,

greg k-h

2019-07-31 13:09:42

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] driver core: add dev_groups to all drivers

On Wed, Jul 31, 2019 at 02:51:04PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 31, 2019 at 02:49:26PM +0200, Takashi Iwai wrote:
> > On Wed, 31 Jul 2019 14:43:40 +0200,
> > Greg Kroah-Hartman wrote:
> > >
> > > From: Dmitry Torokhov <[email protected]>
> > >
> > > Add the ability for the driver core to create and remove a list of
> > > attribute groups automatically when the device is bound/unbound from a
> > > specific driver.
> > >
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
> > Missing sign-off from Dmitry?
>
> He never provided it :(
>
> Dmitry, can you please do so? I forgot to include that in the cover
> leter...

Yeah, sorry, I thought what I sent was a draft to be used as you wish
with it; I did not expect to be put down as an author. Anyway,

Signed-off-by: Dmitry Torokhov <[email protected]>

Thanks.

--
Dmitry

2019-07-31 13:11:58

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Wed, Jul 31, 2019 at 02:43:39PM +0200, Greg Kroah-Hartman wrote:
> This patch originally started out just as a way for platform drivers to
> easily add a sysfs group in a race-free way, but thanks to Dmitry's
> patch, this series now is for all drivers in the kernel (hey, a unified
> driver model works!!!)
>
> I've only converted a few platform drivers here in this series to show
> how it works, but other busses can be converted after the first patch
> goes into the tree.
>
> Here's the original 00 message, for people to get an idea of what is
> going on here:
>
> If a platform driver wants to add a sysfs group, it has to do so in a
> racy way, adding it after the driver is bound. To resolve this issue,
> have the platform driver core do this for the driver, making the
> individual drivers logic smaller and simpler, and solving the race at
> the same time.
>
> All of these patches depend on the first patch. I'll take the first one
> through my driver-core tree, and any subsystem maintainer can either ack
> their individul patch and I will be glad to also merge it, or they can
> wait until after 5.4-rc1 when the core patch hits Linus's tree and then
> take it, it's up to them.

Maybe make an immutable branch off 5.2 with just patch 1/10 so that
subsystems (and the driver core tree itself) could pull it in at their
leisure into their "*-next" branches and did not have to wait till 5.4
or risk merge clashes?

Thanks.

--
Dmitry

2019-07-31 13:23:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Wed, Jul 31, 2019 at 06:10:45AM -0700, Dmitry Torokhov wrote:
> On Wed, Jul 31, 2019 at 02:43:39PM +0200, Greg Kroah-Hartman wrote:
> > This patch originally started out just as a way for platform drivers to
> > easily add a sysfs group in a race-free way, but thanks to Dmitry's
> > patch, this series now is for all drivers in the kernel (hey, a unified
> > driver model works!!!)
> >
> > I've only converted a few platform drivers here in this series to show
> > how it works, but other busses can be converted after the first patch
> > goes into the tree.
> >
> > Here's the original 00 message, for people to get an idea of what is
> > going on here:
> >
> > If a platform driver wants to add a sysfs group, it has to do so in a
> > racy way, adding it after the driver is bound. To resolve this issue,
> > have the platform driver core do this for the driver, making the
> > individual drivers logic smaller and simpler, and solving the race at
> > the same time.
> >
> > All of these patches depend on the first patch. I'll take the first one
> > through my driver-core tree, and any subsystem maintainer can either ack
> > their individul patch and I will be glad to also merge it, or they can
> > wait until after 5.4-rc1 when the core patch hits Linus's tree and then
> > take it, it's up to them.
>
> Maybe make an immutable branch off 5.2 with just patch 1/10 so that
> subsystems (and the driver core tree itself) could pull it in at their
> leisure into their "*-next" branches and did not have to wait till 5.4
> or risk merge clashes?

Good idea, I will do that when I apply it after a few days to give
people a chance to review it.

thanks,

greg k-h

2019-07-31 13:59:19

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 03/10] input: keyboard: gpio_keys: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a bunch of sysfs files.

Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/input/keyboard/gpio_keys.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 03f4d152f6b7..1373dc5b0765 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -351,10 +351,7 @@ static struct attribute *gpio_keys_attrs[] = {
&dev_attr_disabled_switches.attr,
NULL,
};
-
-static const struct attribute_group gpio_keys_attr_group = {
- .attrs = gpio_keys_attrs,
-};
+ATTRIBUTE_GROUPS(gpio_keys);

static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
{
@@ -851,13 +848,6 @@ static int gpio_keys_probe(struct platform_device *pdev)

fwnode_handle_put(child);

- error = devm_device_add_group(dev, &gpio_keys_attr_group);
- if (error) {
- dev_err(dev, "Unable to export keys/switches, error: %d\n",
- error);
- return error;
- }
-
error = input_register_device(input);
if (error) {
dev_err(dev, "Unable to register input device, error: %d\n",
@@ -1026,6 +1016,7 @@ static struct platform_driver gpio_keys_device_driver = {
.name = "gpio-keys",
.pm = &gpio_keys_pm_ops,
.of_match_table = gpio_keys_of_match,
+ .dev_groups = gpio_keys_groups,
}
};

--
2.22.0

2019-07-31 13:59:24

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 04/10] input: axp20x-pek: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a sysfs group of attributes.

Cc: Dmitry Torokhov <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/input/misc/axp20x-pek.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index debeeaeb8812..235925b28772 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -195,15 +195,12 @@ DEVICE_ATTR(startup, 0644, axp20x_show_attr_startup, axp20x_store_attr_startup);
DEVICE_ATTR(shutdown, 0644, axp20x_show_attr_shutdown,
axp20x_store_attr_shutdown);

-static struct attribute *axp20x_attributes[] = {
+static struct attribute *axp20x_attrs[] = {
&dev_attr_startup.attr,
&dev_attr_shutdown.attr,
NULL,
};
-
-static const struct attribute_group axp20x_attribute_group = {
- .attrs = axp20x_attributes,
-};
+ATTRIBUTE_GROUPS(axp20x);

static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
{
@@ -356,13 +353,6 @@ static int axp20x_pek_probe(struct platform_device *pdev)

axp20x_pek->info = (struct axp20x_info *)match->driver_data;

- error = devm_device_add_group(&pdev->dev, &axp20x_attribute_group);
- if (error) {
- dev_err(&pdev->dev, "Failed to create sysfs attributes: %d\n",
- error);
- return error;
- }
-
platform_set_drvdata(pdev, axp20x_pek);

return 0;
@@ -411,6 +401,7 @@ static struct platform_driver axp20x_pek_driver = {
.driver = {
.name = "axp20x-pek",
.pm = &axp20x_pek_pm_ops,
+ .dev_groups = axp20x_groups,
},
};
module_platform_driver(axp20x_pek_driver);
--
2.22.0

2019-07-31 14:00:43

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] driver core: add dev_groups to all drivers

On Wed, 31 Jul 2019 14:43:40 +0200,
Greg Kroah-Hartman wrote:
>
> From: Dmitry Torokhov <[email protected]>
>
> Add the ability for the driver core to create and remove a list of
> attribute groups automatically when the device is bound/unbound from a
> specific driver.
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Missing sign-off from Dmitry?


Takashi

2019-07-31 14:32:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 10/10] video: fbdev: sm501fb: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a bunch of sysfs files.

Cc: [email protected]
Cc: [email protected]
Acked-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/video/fbdev/sm501fb.c | 37 +++++++++--------------------------
1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index 6edb4492e675..3dd1b1d76e98 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -1271,6 +1271,14 @@ static ssize_t sm501fb_debug_show_pnl(struct device *dev,

static DEVICE_ATTR(fbregs_pnl, 0444, sm501fb_debug_show_pnl, NULL);

+static struct attribute *sm501fb_attrs[] = {
+ &dev_attr_crt_src.attr,
+ &dev_attr_fbregs_pnl.attr,
+ &dev_attr_fbregs_crt.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(sm501fb);
+
/* acceleration operations */
static int sm501fb_sync(struct fb_info *info)
{
@@ -2011,33 +2019,9 @@ static int sm501fb_probe(struct platform_device *pdev)
goto err_started_crt;
}

- /* create device files */
-
- ret = device_create_file(dev, &dev_attr_crt_src);
- if (ret)
- goto err_started_panel;
-
- ret = device_create_file(dev, &dev_attr_fbregs_pnl);
- if (ret)
- goto err_attached_crtsrc_file;
-
- ret = device_create_file(dev, &dev_attr_fbregs_crt);
- if (ret)
- goto err_attached_pnlregs_file;
-
/* we registered, return ok */
return 0;

-err_attached_pnlregs_file:
- device_remove_file(dev, &dev_attr_fbregs_pnl);
-
-err_attached_crtsrc_file:
- device_remove_file(dev, &dev_attr_crt_src);
-
-err_started_panel:
- unregister_framebuffer(info->fb[HEAD_PANEL]);
- sm501_free_init_fb(info, HEAD_PANEL);
-
err_started_crt:
unregister_framebuffer(info->fb[HEAD_CRT]);
sm501_free_init_fb(info, HEAD_CRT);
@@ -2067,10 +2051,6 @@ static int sm501fb_remove(struct platform_device *pdev)
struct fb_info *fbinfo_crt = info->fb[0];
struct fb_info *fbinfo_pnl = info->fb[1];

- device_remove_file(&pdev->dev, &dev_attr_fbregs_crt);
- device_remove_file(&pdev->dev, &dev_attr_fbregs_pnl);
- device_remove_file(&pdev->dev, &dev_attr_crt_src);
-
sm501_free_init_fb(info, HEAD_CRT);
sm501_free_init_fb(info, HEAD_PANEL);

@@ -2234,6 +2214,7 @@ static struct platform_driver sm501fb_driver = {
.resume = sm501fb_resume,
.driver = {
.name = "sm501-fb",
+ .dev_groups = sm501fb_groups,
},
};

--
2.22.0

2019-07-31 14:32:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH v2 05/10] firmware: arm_scpi: convert platform driver to use dev_groups

Platform drivers now have the option to have the platform core create
and remove any needed sysfs attribute files. So take advantage of that
and do not register "by hand" a sysfs group of attributes.

Acked-by: Sudeep Holla <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/firmware/arm_scpi.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 725164b83242..a80c331c3a6e 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -1011,10 +1011,6 @@ static int scpi_probe(struct platform_device *pdev)
scpi_info->firmware_version));
scpi_info->scpi_ops = &scpi_ops;

- ret = devm_device_add_groups(dev, versions_groups);
- if (ret)
- dev_err(dev, "unable to create sysfs version group\n");
-
return devm_of_platform_populate(dev);
}

@@ -1030,6 +1026,7 @@ static struct platform_driver scpi_driver = {
.driver = {
.name = "scpi_protocol",
.of_match_table = scpi_of_match,
+ .dev_groups = versions_groups,
},
.probe = scpi_probe,
.remove = scpi_remove,
--
2.22.0

2019-07-31 15:16:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] driver core: add dev_groups to all drivers

On Wed, Jul 31, 2019 at 06:08:00AM -0700, Dmitry Torokhov wrote:
> On Wed, Jul 31, 2019 at 02:51:04PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jul 31, 2019 at 02:49:26PM +0200, Takashi Iwai wrote:
> > > On Wed, 31 Jul 2019 14:43:40 +0200,
> > > Greg Kroah-Hartman wrote:
> > > >
> > > > From: Dmitry Torokhov <[email protected]>
> > > >
> > > > Add the ability for the driver core to create and remove a list of
> > > > attribute groups automatically when the device is bound/unbound from a
> > > > specific driver.
> > > >
> > > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > >
> > > Missing sign-off from Dmitry?
> >
> > He never provided it :(
> >
> > Dmitry, can you please do so? I forgot to include that in the cover
> > leter...
>
> Yeah, sorry, I thought what I sent was a draft to be used as you wish
> with it; I did not expect to be put down as an author. Anyway,

Your patch pretty much worked as-is, I only had to change one line. You
did a nice job :)

> Signed-off-by: Dmitry Torokhov <[email protected]>

Thanks, I'll add this.

greg k-h

2019-07-31 15:19:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Wed, Jul 31, 2019 at 06:10:45AM -0700, Dmitry Torokhov wrote:
> On Wed, Jul 31, 2019 at 02:43:39PM +0200, Greg Kroah-Hartman wrote:
> > This patch originally started out just as a way for platform drivers to
> > easily add a sysfs group in a race-free way, but thanks to Dmitry's
> > patch, this series now is for all drivers in the kernel (hey, a unified
> > driver model works!!!)
> >
> > I've only converted a few platform drivers here in this series to show
> > how it works, but other busses can be converted after the first patch
> > goes into the tree.
> >
> > Here's the original 00 message, for people to get an idea of what is
> > going on here:
> >
> > If a platform driver wants to add a sysfs group, it has to do so in a
> > racy way, adding it after the driver is bound. To resolve this issue,
> > have the platform driver core do this for the driver, making the
> > individual drivers logic smaller and simpler, and solving the race at
> > the same time.
> >
> > All of these patches depend on the first patch. I'll take the first one
> > through my driver-core tree, and any subsystem maintainer can either ack
> > their individul patch and I will be glad to also merge it, or they can
> > wait until after 5.4-rc1 when the core patch hits Linus's tree and then
> > take it, it's up to them.
>
> Maybe make an immutable branch off 5.2 with just patch 1/10 so that
> subsystems (and the driver core tree itself) could pull it in at their
> leisure into their "*-next" branches and did not have to wait till 5.4
> or risk merge clashes?

Isn't cherry-pick enough for one patch?

--
With Best Regards,
Andy Shevchenko


2019-07-31 15:40:57

by Richard Gong

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] driver core: add dev_groups to all drivers


Hi Greg,

On 7/31/19 7:43 AM, Greg Kroah-Hartman wrote:
> From: Dmitry Torokhov <[email protected]>
>
> Add the ability for the driver core to create and remove a list of
> attribute groups automatically when the device is bound/unbound from a
> specific driver.
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Tested-by: Richard Gong <[email protected]>

> ---
> drivers/base/dd.c | 14 ++++++++++++++
> include/linux/device.h | 3 +++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 994a90747420..d811e60610d3 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -554,9 +554,16 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> goto probe_failed;
> }
>
> + if (device_add_groups(dev, drv->dev_groups)) {
> + dev_err(dev, "device_add_groups() failed\n");
> + goto dev_groups_failed;
> + }
> +
> if (test_remove) {
> test_remove = false;
>
> + device_remove_groups(dev, drv->dev_groups);
> +
> if (dev->bus->remove)
> dev->bus->remove(dev);
> else if (drv->remove)
> @@ -584,6 +591,11 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> drv->bus->name, __func__, dev_name(dev), drv->name);
> goto done;
>
> +dev_groups_failed:
> + if (dev->bus->remove)
> + dev->bus->remove(dev);
> + else if (drv->remove)
> + drv->remove(dev);
> probe_failed:
> if (dev->bus)
> blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> @@ -1114,6 +1126,8 @@ static void __device_release_driver(struct device *dev, struct device *parent)
>
> pm_runtime_put_sync(dev);
>
> + device_remove_groups(dev, drv->dev_groups);
> +
> if (dev->bus && dev->bus->remove)
> dev->bus->remove(dev);
> else if (drv->remove)
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c330b75c6c57..98c00b71b598 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -262,6 +262,8 @@ enum probe_type {
> * @resume: Called to bring a device from sleep mode.
> * @groups: Default attributes that get created by the driver core
> * automatically.
> + * @dev_groups: Additional attributes attached to device instance once the
> + * it is bound to the driver.
> * @pm: Power management operations of the device which matched
> * this driver.
> * @coredump: Called when sysfs entry is written to. The device driver
> @@ -296,6 +298,7 @@ struct device_driver {
> int (*suspend) (struct device *dev, pm_message_t state);
> int (*resume) (struct device *dev);
> const struct attribute_group **groups;
> + const struct attribute_group **dev_groups;
>
> const struct dev_pm_ops *pm;
> void (*coredump) (struct device *dev);
>

Regards,
Richard

2019-07-31 16:38:09

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Wed, Jul 31, 2019 at 04:38:40PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 31, 2019 at 06:10:45AM -0700, Dmitry Torokhov wrote:
> > On Wed, Jul 31, 2019 at 02:43:39PM +0200, Greg Kroah-Hartman wrote:
> > > This patch originally started out just as a way for platform drivers to
> > > easily add a sysfs group in a race-free way, but thanks to Dmitry's
> > > patch, this series now is for all drivers in the kernel (hey, a unified
> > > driver model works!!!)
> > >
> > > I've only converted a few platform drivers here in this series to show
> > > how it works, but other busses can be converted after the first patch
> > > goes into the tree.
> > >
> > > Here's the original 00 message, for people to get an idea of what is
> > > going on here:
> > >
> > > If a platform driver wants to add a sysfs group, it has to do so in a
> > > racy way, adding it after the driver is bound. To resolve this issue,
> > > have the platform driver core do this for the driver, making the
> > > individual drivers logic smaller and simpler, and solving the race at
> > > the same time.
> > >
> > > All of these patches depend on the first patch. I'll take the first one
> > > through my driver-core tree, and any subsystem maintainer can either ack
> > > their individul patch and I will be glad to also merge it, or they can
> > > wait until after 5.4-rc1 when the core patch hits Linus's tree and then
> > > take it, it's up to them.
> >
> > Maybe make an immutable branch off 5.2 with just patch 1/10 so that
> > subsystems (and the driver core tree itself) could pull it in at their
> > leisure into their "*-next" branches and did not have to wait till 5.4
> > or risk merge clashes?
>
> Isn't cherry-pick enough for one patch?

With cherry-picking you run into potential merge conflicts if Greg
changes more code in the same area. Plus, once everything is merged into
Linus' tree, there will be N git commits adding the same thing.

With immutable branch there is a single git commit, so merges are
guaranteed to be clean, with no conflicts.

Thanks.

--
Dmitry

2019-08-02 13:48:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Wed, Jul 31, 2019 at 06:10:45AM -0700, Dmitry Torokhov wrote:
> On Wed, Jul 31, 2019 at 02:43:39PM +0200, Greg Kroah-Hartman wrote:
> > This patch originally started out just as a way for platform drivers to
> > easily add a sysfs group in a race-free way, but thanks to Dmitry's
> > patch, this series now is for all drivers in the kernel (hey, a unified
> > driver model works!!!)
> >
> > I've only converted a few platform drivers here in this series to show
> > how it works, but other busses can be converted after the first patch
> > goes into the tree.
> >
> > Here's the original 00 message, for people to get an idea of what is
> > going on here:
> >
> > If a platform driver wants to add a sysfs group, it has to do so in a
> > racy way, adding it after the driver is bound. To resolve this issue,
> > have the platform driver core do this for the driver, making the
> > individual drivers logic smaller and simpler, and solving the race at
> > the same time.
> >
> > All of these patches depend on the first patch. I'll take the first one
> > through my driver-core tree, and any subsystem maintainer can either ack
> > their individul patch and I will be glad to also merge it, or they can
> > wait until after 5.4-rc1 when the core patch hits Linus's tree and then
> > take it, it's up to them.
>
> Maybe make an immutable branch off 5.2 with just patch 1/10 so that
> subsystems (and the driver core tree itself) could pull it in at their
> leisure into their "*-next" branches and did not have to wait till 5.4
> or risk merge clashes?

I have now done this with patch 1/10. Here's the pull info if any
subsystem maintainer wants to suck this into their tree to provide the
ability for drivers to add/remove attribute groups easily.

This is part of my driver-core tree now, and will go to Linus for
5.4-rc1, along with a few platform drivers that have been acked by their
various subsystem maintainers that convert them to use this new
functionality.

If anyone has any questions about this, please let me know.

thanks,

greg k-h

-------------------

The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:

Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/dev_groups_all_drivers

for you to fetch changes up to 23b6904442d08b7dbed7622ed33b236d41a3aa8b:

driver core: add dev_groups to all drivers (2019-08-02 12:37:53 +0200)

----------------------------------------------------------------
dev_groups added to struct driver

Persistent tag for others to pull this branch from

This is the first patch in a longer series that adds the ability for the
driver core to create and remove a list of attribute groups
automatically when the device is bound/unbound from a specific driver.

See:
https://lore.kernel.org/r/[email protected]
for details on this patch, and examples of how to use it in other
drivers.

Signed-off-by: Greg Kroah-Hartman <[email protected]>

----------------------------------------------------------------
Dmitry Torokhov (1):
driver core: add dev_groups to all drivers

drivers/base/dd.c | 14 ++++++++++++++
include/linux/device.h | 3 +++
2 files changed, 17 insertions(+)

2019-08-12 07:00:14

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 03/10] input: keyboard: gpio_keys: convert platform driver to use dev_groups

On Wed, Jul 31, 2019 at 02:43:42PM +0200, Greg Kroah-Hartman wrote:
> Platform drivers now have the option to have the platform core create
> and remove any needed sysfs attribute files. So take advantage of that
> and do not register "by hand" a bunch of sysfs files.
>
> Cc: Bartlomiej Zolnierkiewicz <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Applied, thank you.

> ---
> drivers/input/keyboard/gpio_keys.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 03f4d152f6b7..1373dc5b0765 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -351,10 +351,7 @@ static struct attribute *gpio_keys_attrs[] = {
> &dev_attr_disabled_switches.attr,
> NULL,
> };
> -
> -static const struct attribute_group gpio_keys_attr_group = {
> - .attrs = gpio_keys_attrs,
> -};
> +ATTRIBUTE_GROUPS(gpio_keys);
>
> static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
> {
> @@ -851,13 +848,6 @@ static int gpio_keys_probe(struct platform_device *pdev)
>
> fwnode_handle_put(child);
>
> - error = devm_device_add_group(dev, &gpio_keys_attr_group);
> - if (error) {
> - dev_err(dev, "Unable to export keys/switches, error: %d\n",
> - error);
> - return error;
> - }
> -
> error = input_register_device(input);
> if (error) {
> dev_err(dev, "Unable to register input device, error: %d\n",
> @@ -1026,6 +1016,7 @@ static struct platform_driver gpio_keys_device_driver = {
> .name = "gpio-keys",
> .pm = &gpio_keys_pm_ops,
> .of_match_table = gpio_keys_of_match,
> + .dev_groups = gpio_keys_groups,
> }
> };
>
> --
> 2.22.0
>

--
Dmitry

2019-08-12 07:00:46

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] input: axp20x-pek: convert platform driver to use dev_groups

On Wed, Jul 31, 2019 at 02:43:43PM +0200, Greg Kroah-Hartman wrote:
> Platform drivers now have the option to have the platform core create
> and remove any needed sysfs attribute files. So take advantage of that
> and do not register "by hand" a sysfs group of attributes.
>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Applied, thank you.

> ---
> drivers/input/misc/axp20x-pek.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index debeeaeb8812..235925b28772 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -195,15 +195,12 @@ DEVICE_ATTR(startup, 0644, axp20x_show_attr_startup, axp20x_store_attr_startup);
> DEVICE_ATTR(shutdown, 0644, axp20x_show_attr_shutdown,
> axp20x_store_attr_shutdown);
>
> -static struct attribute *axp20x_attributes[] = {
> +static struct attribute *axp20x_attrs[] = {
> &dev_attr_startup.attr,
> &dev_attr_shutdown.attr,
> NULL,
> };
> -
> -static const struct attribute_group axp20x_attribute_group = {
> - .attrs = axp20x_attributes,
> -};
> +ATTRIBUTE_GROUPS(axp20x);
>
> static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
> {
> @@ -356,13 +353,6 @@ static int axp20x_pek_probe(struct platform_device *pdev)
>
> axp20x_pek->info = (struct axp20x_info *)match->driver_data;
>
> - error = devm_device_add_group(&pdev->dev, &axp20x_attribute_group);
> - if (error) {
> - dev_err(&pdev->dev, "Failed to create sysfs attributes: %d\n",
> - error);
> - return error;
> - }
> -
> platform_set_drvdata(pdev, axp20x_pek);
>
> return 0;
> @@ -411,6 +401,7 @@ static struct platform_driver axp20x_pek_driver = {
> .driver = {
> .name = "axp20x-pek",
> .pm = &axp20x_pek_pm_ops,
> + .dev_groups = axp20x_groups,
> },
> };
> module_platform_driver(axp20x_pek_driver);
> --
> 2.22.0
>

--
Dmitry

2020-05-13 22:23:16

by Emil Velikov

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

Hi Greg,

On Fri, 2 Aug 2019 at 11:46, Greg Kroah-Hartman
<[email protected]> wrote:

>
> I have now done this with patch 1/10. Here's the pull info if any
> subsystem maintainer wants to suck this into their tree to provide the
> ability for drivers to add/remove attribute groups easily.
>
> This is part of my driver-core tree now, and will go to Linus for
> 5.4-rc1, along with a few platform drivers that have been acked by their
> various subsystem maintainers that convert them to use this new
> functionality.
>
> If anyone has any questions about this, please let me know.
>
> thanks,
>
> greg k-h
>
> -------------------
>
> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>
> Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/dev_groups_all_drivers
>
> for you to fetch changes up to 23b6904442d08b7dbed7622ed33b236d41a3aa8b:
>
> driver core: add dev_groups to all drivers (2019-08-02 12:37:53 +0200)
>
> ----------------------------------------------------------------
> dev_groups added to struct driver
>
> Persistent tag for others to pull this branch from
>
> This is the first patch in a longer series that adds the ability for the
> driver core to create and remove a list of attribute groups
> automatically when the device is bound/unbound from a specific driver.
>
> See:
> https://lore.kernel.org/r/[email protected]
> for details on this patch, and examples of how to use it in other
> drivers.
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ----------------------------------------------------------------
> Dmitry Torokhov (1):
> driver core: add dev_groups to all drivers
>
> drivers/base/dd.c | 14 ++++++++++++++
> include/linux/device.h | 3 +++
> 2 files changed, 17 insertions(+)
> _______________________________________________

Was planning to re-spin DRM a series which uses .dev_groups, although
I cannot see the core patch.
Did the it get reverted or simply fell though the cracks?

-Emil

2020-05-14 07:18:24

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Wed, May 13, 2020 at 11:18:15PM +0100, Emil Velikov wrote:
> Hi Greg,
>
> On Fri, 2 Aug 2019 at 11:46, Greg Kroah-Hartman
> <[email protected]> wrote:
>
> >
> > I have now done this with patch 1/10. Here's the pull info if any
> > subsystem maintainer wants to suck this into their tree to provide the
> > ability for drivers to add/remove attribute groups easily.
> >
> > This is part of my driver-core tree now, and will go to Linus for
> > 5.4-rc1, along with a few platform drivers that have been acked by their
> > various subsystem maintainers that convert them to use this new
> > functionality.
> >
> > If anyone has any questions about this, please let me know.
> >
> > thanks,
> >
> > greg k-h
> >
> > -------------------
> >
> > The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
> >
> > Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
> >
> > are available in the Git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/dev_groups_all_drivers
> >
> > for you to fetch changes up to 23b6904442d08b7dbed7622ed33b236d41a3aa8b:
> >
> > driver core: add dev_groups to all drivers (2019-08-02 12:37:53 +0200)
> >
> > ----------------------------------------------------------------
> > dev_groups added to struct driver
> >
> > Persistent tag for others to pull this branch from
> >
> > This is the first patch in a longer series that adds the ability for the
> > driver core to create and remove a list of attribute groups
> > automatically when the device is bound/unbound from a specific driver.
> >
> > See:
> > https://lore.kernel.org/r/[email protected]
> > for details on this patch, and examples of how to use it in other
> > drivers.
> >
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
> > ----------------------------------------------------------------
> > Dmitry Torokhov (1):
> > driver core: add dev_groups to all drivers
> >
> > drivers/base/dd.c | 14 ++++++++++++++
> > include/linux/device.h | 3 +++
> > 2 files changed, 17 insertions(+)
> > _______________________________________________
>
> Was planning to re-spin DRM a series which uses .dev_groups, although
> I cannot see the core patch.
> Did the it get reverted or simply fell though the cracks?

Nope, it's in there:
23b6904442d0 ("driver core: add dev_groups to all drivers")
which showed up in the 5.4 kernel release.

Lots of other subsystems have already been converted to use this, do you
not see it in your tree?

thanks,

greg k-h

2020-05-14 11:53:19

by Emil Velikov

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] drivers, provide a way to add sysfs groups easily

On Thu, 14 May 2020 at 08:16, Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Wed, May 13, 2020 at 11:18:15PM +0100, Emil Velikov wrote:
> > Hi Greg,
> >
> > On Fri, 2 Aug 2019 at 11:46, Greg Kroah-Hartman
> > <[email protected]> wrote:
> >
> > >
> > > I have now done this with patch 1/10. Here's the pull info if any
> > > subsystem maintainer wants to suck this into their tree to provide the
> > > ability for drivers to add/remove attribute groups easily.
> > >
> > > This is part of my driver-core tree now, and will go to Linus for
> > > 5.4-rc1, along with a few platform drivers that have been acked by their
> > > various subsystem maintainers that convert them to use this new
> > > functionality.
> > >
> > > If anyone has any questions about this, please let me know.
> > >
> > > thanks,
> > >
> > > greg k-h
> > >
> > > -------------------
> > >
> > > The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
> > >
> > > Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
> > >
> > > are available in the Git repository at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/dev_groups_all_drivers
> > >
> > > for you to fetch changes up to 23b6904442d08b7dbed7622ed33b236d41a3aa8b:
> > >
> > > driver core: add dev_groups to all drivers (2019-08-02 12:37:53 +0200)
> > >
> > > ----------------------------------------------------------------
> > > dev_groups added to struct driver
> > >
> > > Persistent tag for others to pull this branch from
> > >
> > > This is the first patch in a longer series that adds the ability for the
> > > driver core to create and remove a list of attribute groups
> > > automatically when the device is bound/unbound from a specific driver.
> > >
> > > See:
> > > https://lore.kernel.org/r/[email protected]
> > > for details on this patch, and examples of how to use it in other
> > > drivers.
> > >
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > >
> > > ----------------------------------------------------------------
> > > Dmitry Torokhov (1):
> > > driver core: add dev_groups to all drivers
> > >
> > > drivers/base/dd.c | 14 ++++++++++++++
> > > include/linux/device.h | 3 +++
> > > 2 files changed, 17 insertions(+)
> > > _______________________________________________
> >
> > Was planning to re-spin DRM a series which uses .dev_groups, although
> > I cannot see the core patch.
> > Did the it get reverted or simply fell though the cracks?
>
> Nope, it's in there:
> 23b6904442d0 ("driver core: add dev_groups to all drivers")
> which showed up in the 5.4 kernel release.
>
> Lots of other subsystems have already been converted to use this, do you
> not see it in your tree?
>
A case of PEBKAC it seems - I was looking at a 5.3 checkout somehow.

Thanks for the core work. Will check/merge the fbdev patches over the
next few days and polish drm land.

-Emil