Re: [PATCH v2] eeprom: ee1004: Let device core handle attribute eeprom

From: Heiner Kallweit
Date: Fri May 21 2021 - 14:44:13 EST


Greg added his Reviewed-by to v1, so you may disregard v2.

On 20.05.2021 21:25, Heiner Kallweit wrote:
> Instead of creating/removing the attribute ourselves, just declare the
> attribute and let the device core handle it. This allows to simplify
> the code.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> v2:
> - open-code the proposed BIN_ATTRIBUTES_GROUPS macro
> I leave it to you which version you prefer.
> ---
> drivers/misc/eeprom/ee1004.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
> index 252e15ba6..d61acdaba 100644
> --- a/drivers/misc/eeprom/ee1004.c
> +++ b/drivers/misc/eeprom/ee1004.c
> @@ -89,7 +89,7 @@ static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
> return status;
> }
>
> -static ssize_t ee1004_read(struct file *filp, struct kobject *kobj,
> +static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
> struct bin_attribute *bin_attr,
> char *buf, loff_t off, size_t count)
> {
> @@ -160,15 +160,19 @@ static ssize_t ee1004_read(struct file *filp, struct kobject *kobj,
> return requested;
> }
>
> -static const struct bin_attribute eeprom_attr = {
> - .attr = {
> - .name = "eeprom",
> - .mode = 0444,
> - },
> - .size = EE1004_EEPROM_SIZE,
> - .read = ee1004_read,
> +static BIN_ATTR_RO(eeprom, EE1004_EEPROM_SIZE);
> +
> +static struct bin_attribute *ee1004_attrs[] = {
> + &bin_attr_eeprom,
> + NULL
> +};
> +
> +static const struct attribute_group ee1004_group = {
> + .bin_attrs = ee1004_attrs,
> };
>
> +__ATTRIBUTE_GROUPS(ee1004);
> +
> static int ee1004_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -222,11 +226,6 @@ static int ee1004_probe(struct i2c_client *client,
> ee1004_current_page);
> mutex_unlock(&ee1004_bus_lock);
>
> - /* Create the sysfs eeprom file */
> - err = sysfs_create_bin_file(&client->dev.kobj, &eeprom_attr);
> - if (err)
> - goto err_clients_lock;
> -
> dev_info(&client->dev,
> "%u byte EE1004-compliant SPD EEPROM, read-only\n",
> EE1004_EEPROM_SIZE);
> @@ -237,8 +236,6 @@ static int ee1004_probe(struct i2c_client *client,
>
> return 0;
>
> - err_clients_lock:
> - mutex_lock(&ee1004_bus_lock);
> err_clients:
> if (--ee1004_dev_count == 0) {
> for (cnr--; cnr >= 0; cnr--) {
> @@ -255,8 +252,6 @@ static int ee1004_remove(struct i2c_client *client)
> {
> int i;
>
> - sysfs_remove_bin_file(&client->dev.kobj, &eeprom_attr);
> -
> /* Remove page select clients if this is the last device */
> mutex_lock(&ee1004_bus_lock);
> if (--ee1004_dev_count == 0) {
> @@ -275,6 +270,7 @@ static int ee1004_remove(struct i2c_client *client)
> static struct i2c_driver ee1004_driver = {
> .driver = {
> .name = "ee1004",
> + .dev_groups = ee1004_groups,
> },
> .probe = ee1004_probe,
> .remove = ee1004_remove,
>