On Wed, Feb 04, 2015 at 11:58:53AM +0100, Takashi Iwai wrote:
> Instead of manual calls of device_create_file() and
> device_remove_file(), assign the static attribute groups to the device
> with device_create_with_groups(). The conditionally built sysfs
> entries are handled via is_visible callback.
>
> This simplifies the code and also avoids the possible races.
>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
lgtm, pulled into my drm-misc branch.
-Daniel
> ---
> drivers/gpu/drm/drm_sysfs.c | 132
> ++++++++++++++++++++++----------------------
> 1 file changed, 67 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index cc3d6d6d67e0..5c99d3773212 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -339,19 +339,51 @@ static ssize_t select_subconnector_show(struct device
> *device,
> drm_get_dvi_i_select_name((int)subconnector));
> }
>
> -static struct device_attribute connector_attrs[] = {
> - __ATTR_RO(status),
> - __ATTR_RO(enabled),
> - __ATTR_RO(dpms),
> - __ATTR_RO(modes),
> +static DEVICE_ATTR_RO(status);
> +static DEVICE_ATTR_RO(enabled);
> +static DEVICE_ATTR_RO(dpms);
> +static DEVICE_ATTR_RO(modes);
> +
> +static struct attribute *connector_dev_attrs[] = {
> + &dev_attr_status.attr,
> + &dev_attr_enabled.attr,
> + &dev_attr_dpms.attr,
> + &dev_attr_modes.attr,
> + NULL
> };
>
> /* These attributes are for both DVI-I connectors and all types of tv-out. */
> -static struct device_attribute connector_attrs_opt1[] = {
> - __ATTR_RO(subconnector),
> - __ATTR_RO(select_subconnector),
> +static DEVICE_ATTR_RO(subconnector);
> +static DEVICE_ATTR_RO(select_subconnector);
> +
> +static struct attribute *connector_opt_dev_attrs[] = {
> + &dev_attr_subconnector.attr,
> + &dev_attr_select_subconnector.attr,
> + NULL
> };
>
> +static umode_t connector_opt_dev_is_visible(struct kobject *kobj,
> + struct attribute *attr, int idx)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct drm_connector *connector = to_drm_connector(dev);
> +
> + /*
> + * In the long run it maybe a good idea to make one set of
> + * optionals per connector type.
> + */
> + switch (connector->connector_type) {
> + case DRM_MODE_CONNECTOR_DVII:
> + case DRM_MODE_CONNECTOR_Composite:
> + case DRM_MODE_CONNECTOR_SVIDEO:
> + case DRM_MODE_CONNECTOR_Component:
> + case DRM_MODE_CONNECTOR_TV:
> + return attr->mode;
> + }
> +
> + return 0;
> +}
> +
> static struct bin_attribute edid_attr = {
> .attr.name = "edid",
> .attr.mode = 0444,
> @@ -359,6 +391,27 @@ static struct bin_attribute edid_attr = {
> .read = edid_show,
> };
>
> +static struct bin_attribute *connector_bin_attrs[] = {
> + &edid_attr,
> + NULL
> +};
> +
> +static const struct attribute_group connector_dev_group = {
> + .attrs = connector_dev_attrs,
> + .bin_attrs = connector_bin_attrs,
> +};
> +
> +static const struct attribute_group connector_opt_dev_group = {
> + .attrs = connector_opt_dev_attrs,
> + .is_visible = connector_opt_dev_is_visible,
> +};
> +
> +static const struct attribute_group *connector_dev_groups[] = {
> + &connector_dev_group,
> + &connector_opt_dev_group,
> + NULL
> +};
> +
> /**
> * drm_sysfs_connector_add - add a connector to sysfs
> * @connector: connector to add
> @@ -371,73 +424,27 @@ static struct bin_attribute edid_attr = {
> int drm_sysfs_connector_add(struct drm_connector *connector)
> {
> struct drm_device *dev = connector->dev;
> - int attr_cnt = 0;
> - int opt_cnt = 0;
> - int i;
> - int ret;
>
> if (connector->kdev)
> return 0;
>
> - connector->kdev = device_create(drm_class, dev->primary->kdev,
> - 0, connector, "card%d-%s",
> - dev->primary->index, connector->name);
> + connector->kdev =
> + device_create_with_groups(drm_class, dev->primary->kdev, 0,
> + connector, connector_dev_groups,
> + "card%d-%s", dev->primary->index,
> + connector->name);
> DRM_DEBUG("adding \"%s\" to sysfs\n",
> connector->name);
>
> if (IS_ERR(connector->kdev)) {
> DRM_ERROR("failed to register connector device: %ld\n",
> PTR_ERR(connector->kdev));
> - ret = PTR_ERR(connector->kdev);
> - goto out;
> - }
> -
> - /* Standard attributes */
> -
> - for (attr_cnt = 0; attr_cnt < ARRAY_SIZE(connector_attrs); attr_cnt++) {
> - ret = device_create_file(connector->kdev,
> &connector_attrs[attr_cnt]);
> - if (ret)
> - goto err_out_files;
> + return PTR_ERR(connector->kdev);
> }
>
> - /* Optional attributes */
> - /*
> - * In the long run it maybe a good idea to make one set of
> - * optionals per connector type.
> - */
> - switch (connector->connector_type) {
> - case DRM_MODE_CONNECTOR_DVII:
> - case DRM_MODE_CONNECTOR_Composite:
> - case DRM_MODE_CONNECTOR_SVIDEO:
> - case DRM_MODE_CONNECTOR_Component:
> - case DRM_MODE_CONNECTOR_TV:
> - for (opt_cnt = 0; opt_cnt <
> ARRAY_SIZE(connector_attrs_opt1); opt_cnt++) {
> - ret = device_create_file(connector->kdev,
> &connector_attrs_opt1[opt_cnt]);
> - if (ret)
> - goto err_out_files;
> - }
> - break;
> - default:
> - break;
> - }
> -
> - ret = sysfs_create_bin_file(&connector->kdev->kobj, &edid_attr);
> - if (ret)
> - goto err_out_files;
> -
> /* Let userspace know we have a new connector */
> drm_sysfs_hotplug_event(dev);
>
> return 0;
> -
> -err_out_files:
> - for (i = 0; i < opt_cnt; i++)
> - device_remove_file(connector->kdev, &connector_attrs_opt1[i]);
> - for (i = 0; i < attr_cnt; i++)
> - device_remove_file(connector->kdev, &connector_attrs[i]);
> - device_unregister(connector->kdev);
> -
> -out:
> - return ret;
> }
>
> /**
> @@ -455,16 +462,11 @@ out:
> */
> void drm_sysfs_connector_remove(struct drm_connector *connector)
> {
> - int i;
> -
> if (!connector->kdev)
> return;
> DRM_DEBUG("removing \"%s\" from sysfs\n",
> connector->name);
>
> - for (i = 0; i < ARRAY_SIZE(connector_attrs); i++)
> - device_remove_file(connector->kdev, &connector_attrs[i]);
> - sysfs_remove_bin_file(&connector->kdev->kobj, &edid_attr);
> device_unregister(connector->kdev);
> connector->kdev = NULL;
> }
> --
> 2.2.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
