Re: [PATCH v2] driver core: move device->knode_class to device_private

[Rafael J. Wysocki]

On Fri, Jan 18, 2019 at 3:35 AM Wei Yang <richardw.yang@xxxxxxxxxxxxxxx> wrote:
>
> As the description of struct device_private says, it stores data which
> is private to driver core. And it already has similar fields like:
> knode_parent, knode_driver, knode_driver and knode_bus. This look it is
> more proper to put knode_class together with those fields to make it
> private to driver core.
>
> This patch move device->knode_class to device_private to make it comply
> with code convention.
>
> Signed-off-by: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx>
>
> ---
> v2: add a helper to get dev from klist_class

Looks better now.

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

> ---
>  drivers/base/base.h    |  4 ++++
>  drivers/base/class.c   | 14 ++++++++++----
>  drivers/base/core.c    |  4 ++--
>  include/linux/device.h |  1 -
>  4 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 7a419a7a6235..37329a668935 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -60,6 +60,7 @@ struct driver_private {
>   * @knode_parent - node in sibling list
>   * @knode_driver - node in driver list
>   * @knode_bus - node in bus list
> + * @knode_class - node in class list
>   * @deferred_probe - entry in deferred_probe_list which is used to retry the
>   *     binding of drivers which were unable to get all the resources needed by
>   *     the device; typically because it depends on another driver getting
> @@ -74,6 +75,7 @@ struct device_private {
>         struct klist_node knode_parent;
>         struct klist_node knode_driver;
>         struct klist_node knode_bus;
> +       struct klist_node knode_class;
>         struct list_head deferred_probe;
>         struct device *device;
>  };
> @@ -83,6 +85,8 @@ struct device_private {
>         container_of(obj, struct device_private, knode_driver)
>  #define to_device_private_bus(obj)     \
>         container_of(obj, struct device_private, knode_bus)
> +#define to_device_private_class(obj)   \
> +       container_of(obj, struct device_private, knode_class)
>
>  /* initialisation functions */
>  extern int devices_init(void);
> diff --git a/drivers/base/class.c b/drivers/base/class.c
> index 54def4e02f00..d8a6a5864c2e 100644
> --- a/drivers/base/class.c
> +++ b/drivers/base/class.c
> @@ -117,16 +117,22 @@ static void class_put(struct class *cls)
>                 kset_put(&cls->p->subsys);
>  }
>
> +static struct device *klist_class_to_dev(struct klist_node *n)
> +{
> +       struct device_private *p = to_device_private_class(n);
> +       return p->device;
> +}
> +
>  static void klist_class_dev_get(struct klist_node *n)
>  {
> -       struct device *dev = container_of(n, struct device, knode_class);
> +       struct device *dev = klist_class_to_dev(n);
>
>         get_device(dev);
>  }
>
>  static void klist_class_dev_put(struct klist_node *n)
>  {
> -       struct device *dev = container_of(n, struct device, knode_class);
> +       struct device *dev = klist_class_to_dev(n);
>
>         put_device(dev);
>  }
> @@ -277,7 +283,7 @@ void class_dev_iter_init(struct class_dev_iter *iter, struct class *class,
>         struct klist_node *start_knode = NULL;
>
>         if (start)
> -               start_knode = &start->knode_class;
> +               start_knode = &start->p->knode_class;
>         klist_iter_init_node(&class->p->klist_devices, &iter->ki, start_knode);
>         iter->type = type;
>  }
> @@ -304,7 +310,7 @@ struct device *class_dev_iter_next(struct class_dev_iter *iter)
>                 knode = klist_next(&iter->ki);
>                 if (!knode)
>                         return NULL;
> -               dev = container_of(knode, struct device, knode_class);
> +               dev = klist_class_to_dev(knode);
>                 if (!iter->type || iter->type == dev->type)
>                         return dev;
>         }
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 04bbcd779e11..a624d27d11c0 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1932,7 +1932,7 @@ int device_add(struct device *dev)
>         if (dev->class) {
>                 mutex_lock(&dev->class->p->mutex);
>                 /* tie the class to the device */
> -               klist_add_tail(&dev->knode_class,
> +               klist_add_tail(&dev->p->knode_class,
>                                &dev->class->p->klist_devices);
>
>                 /* notify any interfaces that the device is here */
> @@ -2069,7 +2069,7 @@ void device_del(struct device *dev)
>                         if (class_intf->remove_dev)
>                                 class_intf->remove_dev(dev, class_intf);
>                 /* remove the device from the class list */
> -               klist_del(&dev->knode_class);
> +               klist_del(&dev->p->knode_class);
>                 mutex_unlock(&dev->class->p->mutex);
>         }
>         device_remove_file(dev, &dev_attr_uevent);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 1b25c7a43f4c..040e97669f94 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1035,7 +1035,6 @@ struct device {
>         spinlock_t              devres_lock;
>         struct list_head        devres_head;
>
> -       struct klist_node       knode_class;
>         struct class            *class;
>         const struct attribute_group **groups;  /* optional groups */
>
> --
> 2.19.1
>