From: Bartosz Golaszewski <[email protected]>
It's useful to have the device type information for those sub-devices
that are actually GPIO chips registered with GPIOLIB. While at it: use
the device type struct to setup the release callback which is the
preferred way to use the device API.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/gpio/gpiolib.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d50a786f8176..6b1f16c15deb 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -663,6 +663,11 @@ static void gpiodev_release(struct device *dev)
kfree(gdev);
}
+static struct device_type gpio_dev_type = {
+ .name = "gpio_chip",
+ .release = gpiodev_release,
+};
+
#ifdef CONFIG_GPIO_CDEV
#define gcdev_register(gdev, devt) gpiolib_cdev_register((gdev), (devt))
#define gcdev_unregister(gdev) gpiolib_cdev_unregister((gdev))
@@ -680,6 +685,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
int ret;
+ device_initialize(&gdev->dev);
+
/*
* If fwnode doesn't belong to another device, it's safe to clear its
* initialized flag.
@@ -691,9 +698,6 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
if (ret)
return ret;
- /* From this point, the .release() function cleans up gpio_device */
- gdev->dev.release = gpiodev_release;
-
ret = gpiochip_sysfs_register(gdev);
if (ret)
goto err_remove_device;
@@ -825,6 +829,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
gdev = kzalloc(sizeof(*gdev), GFP_KERNEL);
if (!gdev)
return -ENOMEM;
+
+ gdev->dev.type = &gpio_dev_type;
gdev->dev.bus = &gpio_bus_type;
gdev->dev.parent = gc->parent;
gdev->chip = gc;
@@ -851,7 +857,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_free_ida;
- device_initialize(&gdev->dev);
if (gc->parent && gc->parent->driver)
gdev->owner = gc->parent->driver->owner;
else if (gc->owner)
--
2.40.1
On Thu, Feb 1, 2024 at 5:28 PM Bartosz Golaszewski <[email protected]> wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> It's useful to have the device type information for those sub-devices
> that are actually GPIO chips registered with GPIOLIB. While at it: use
> the device type struct to setup the release callback which is the
> preferred way to use the device API.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
(...=
> +static struct device_type gpio_dev_type = {
> + .name = "gpio_chip",
> + .release = gpiodev_release,
> +};
> + gdev->dev.type = &gpio_dev_type;
> gdev->dev.bus = &gpio_bus_type;
> gdev->dev.parent = gc->parent;
Looks good to me (TM) but we should run this by Greg so he get the chance
to chime in, I doubt he will see it on LKML.
FWIW:
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Wed, Feb 07, 2024 at 11:45:39AM +0100, Linus Walleij wrote:
> On Thu, Feb 1, 2024 at 5:28 PM Bartosz Golaszewski <[email protected]> wrote:
>
> > From: Bartosz Golaszewski <[email protected]>
> >
> > It's useful to have the device type information for those sub-devices
> > that are actually GPIO chips registered with GPIOLIB. While at it: use
> > the device type struct to setup the release callback which is the
> > preferred way to use the device API.
> >
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
> (...=
> > +static struct device_type gpio_dev_type = {
Can you make this structure const please?
> > + .name = "gpio_chip",
> > + .release = gpiodev_release,
> > +};
> > + gdev->dev.type = &gpio_dev_type;
> > gdev->dev.bus = &gpio_bus_type;
> > gdev->dev.parent = gc->parent;
>
> Looks good to me (TM) but we should run this by Greg so he get the chance
> to chime in, I doubt he will see it on LKML.
Yes thanks, I wouldn't have caught it...
greg k-h
On Wed, Feb 7, 2024 at 11:49 AM Greg KH <[email protected]> wrote:
>
> On Wed, Feb 07, 2024 at 11:45:39AM +0100, Linus Walleij wrote:
> > On Thu, Feb 1, 2024 at 5:28 PM Bartosz Golaszewski <[email protected]> wrote:
> >
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > It's useful to have the device type information for those sub-devices
> > > that are actually GPIO chips registered with GPIOLIB. While at it: use
> > > the device type struct to setup the release callback which is the
> > > preferred way to use the device API.
> > >
> > > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > (...=
> > > +static struct device_type gpio_dev_type = {
>
> Can you make this structure const please?
>
I have, there's a v2 on the list already.
Thanks
Bart
> > > + .name = "gpio_chip",
> > > + .release = gpiodev_release,
> > > +};
> > > + gdev->dev.type = &gpio_dev_type;
> > > gdev->dev.bus = &gpio_bus_type;
> > > gdev->dev.parent = gc->parent;
> >
> > Looks good to me (TM) but we should run this by Greg so he get the chance
> > to chime in, I doubt he will see it on LKML.
>
> Yes thanks, I wouldn't have caught it...
>
> greg k-h
On Wed, Feb 07, 2024 at 01:19:37PM +0100, Bartosz Golaszewski wrote:
> On Wed, Feb 7, 2024 at 11:49 AM Greg KH <[email protected]> wrote:
> >
> > On Wed, Feb 07, 2024 at 11:45:39AM +0100, Linus Walleij wrote:
> > > On Thu, Feb 1, 2024 at 5:28 PM Bartosz Golaszewski <[email protected]> wrote:
> > >
> > > > From: Bartosz Golaszewski <[email protected]>
> > > >
> > > > It's useful to have the device type information for those sub-devices
> > > > that are actually GPIO chips registered with GPIOLIB. While at it: use
> > > > the device type struct to setup the release callback which is the
> > > > preferred way to use the device API.
> > > >
> > > > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > > (...=
> > > > +static struct device_type gpio_dev_type = {
> >
> > Can you make this structure const please?
> >
>
> I have, there's a v2 on the list already.
Great, let me go dig that up and do a review...