2021-05-19 20:18:12

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH 0/2] eeprom: ee1004: Let device core handle attribute eeprom

Instead of creating/removing the attribute ourselves, just declare the
attribute and let the device core handle it. This allows to simplify
the code.

Heiner Kallweit (2):
sysfs: Add helper BIN_ATTRIBUTE_GROUPS
eeprom: ee1004: Let device core handle attribute eeprom

drivers/misc/eeprom/ee1004.c | 26 +++++++++-----------------
include/linux/sysfs.h | 6 ++++++
2 files changed, 15 insertions(+), 17 deletions(-)

--
2.31.1



2021-05-19 20:19:43

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH 2/2] eeprom: ee1004: Let device core handle attribute eeprom

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 <[email protected]>
---
drivers/misc/eeprom/ee1004.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 252e15ba6..0950d4d9d 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,15 @@ 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
};

+BIN_ATTRIBUTE_GROUPS(ee1004);
+
static int ee1004_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -222,11 +222,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 +232,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 +248,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 +266,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,
--
2.31.1



2021-05-21 07:18:10

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v2] eeprom: ee1004: Let device core handle attribute eeprom

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 <[email protected]>
---
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,
--
2.31.1

2021-05-21 20:44:20

by Heiner Kallweit

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

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 <[email protected]>
> ---
> 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,
>