Re: [PATCH v4 4/4] nvmem: eeprom: at25: export FRAM serial num

On Thu, May 20, 2021 at 08:56:39AM +0200, Jiří Prchal wrote:

Here I'm completlly lost:

On 20. 05. 21 7:52, Greg Kroah-Hartman wrote:

quoted

On Thu, May 20, 2021 at 07:47:14AM +0200, Jiri Prchal wrote:

quoted

This exports serial number of FRAM in sysfs file named "sernum".
Formatted in hex, each byte separated by space.
Example:
$ cat /sys/class/spi_master/spi0/spi0.0/sernum

No new Documentation/ABI/ entry for this?

No, should I do and how / where?

All sysfs files need a Documentation/ABI/ entry.

quoted

quoted

+static ssize_t sernum_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct at25_data *at25;
+	int i;
+
+	at25 = dev_get_drvdata(dev);
+	for (i = 0; i < FM25_SN_LEN; i++)
+		buf += sprintf(buf, "%02x ", at25->sernum[i]);
+	sprintf(--buf, "\n");
+	return (3 * i);

No, that is not how sysfs files work, sorry.  They are "one value per
file".  This looks like multiple values in the same file, why not just
one file per "sernum"?

It's formatted by spaces. It's one long number like MAC addr, so is better
to expose it as hex string without spaces? Or like MAC separated by colon?

Why format it at all?  Just dump out the whole thing as a string.

But who is going to care about this?  What tool will be reading it and
what will it be for?  The Documentation/ABI/ entry would help explain
all of this.

quoted

quoted

+	/* Export the FM25 serial number */
+	if (at25->has_sernum) {
+		err = device_create_file(&spi->dev, &dev_attr_sernum);

You just raced with userspace and lost :(

?

quoted

Please do this correctly, by setting the driver group if you need a file
like this.

Any example, please?

Loads, see any platform driver that sets the dev_groups pointer.

thanks,

greg k-h