Adds support for Cypress FRAMs.
Jiri Prchal (4):
nvmem: eeprom: at25: add support for FRAM
nvmem: eeprom: at25: add support for FRAM
nvmem: eeprom: add documentation for FRAM
nvmem: eeprom: at25: export FRAM serial num
.../devicetree/bindings/eeprom/at25.yaml | 31 ++-
drivers/misc/eeprom/Kconfig | 5 +-
drivers/misc/eeprom/at25.c | 177 +++++++++++++++---
drivers/nvmem/core.c | 4 +
include/linux/nvmem-provider.h | 1 +
5 files changed, 180 insertions(+), 38 deletions(-)
--
v2: fixes in some files
v3: resend and added more recipients
v4: resend
---
2.25.1
Added enum and string for FRAM to expose it as "fram".
Signed-off-by: Jiri Prchal <[email protected]>
---
v2: no change here
v3: resend and added more recipients
v4: resend
---
drivers/nvmem/core.c | 4 ++++
include/linux/nvmem-provider.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 177f5bf27c6d..01ef9a934b0a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -180,6 +180,7 @@ static const char * const nvmem_type_str[] = {
[NVMEM_TYPE_EEPROM] = "EEPROM",
[NVMEM_TYPE_OTP] = "OTP",
[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
+ [NVMEM_TYPE_FRAM] = "FRAM",
};
#ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -359,6 +360,9 @@ static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
if (!config->base_dev)
return -EINVAL;
+ if (config->type == NVMEM_TYPE_FRAM)
+ bin_attr_nvmem_eeprom_compat.attr.name = "fram";
+
nvmem->eeprom = bin_attr_nvmem_eeprom_compat;
nvmem->eeprom.attr.mode = nvmem_bin_attr_get_umode(nvmem);
nvmem->eeprom.size = nvmem->size;
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index e162b757b6d5..890003565761 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -25,6 +25,7 @@ enum nvmem_type {
NVMEM_TYPE_EEPROM,
NVMEM_TYPE_OTP,
NVMEM_TYPE_BATTERY_BACKED,
+ NVMEM_TYPE_FRAM,
};
#define NVMEM_DEVID_NONE (-1)
--
2.25.1
Added dt binding documentation.
Signed-off-by: Jiri Prchal <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v2: fixed dt_binding_check warnings thanks to Rob Herring
v3: resend and added more recipients
v4: resend
---
.../devicetree/bindings/eeprom/at25.yaml | 31 +++++++++++++++----
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/eeprom/at25.yaml b/Documentation/devicetree/bindings/eeprom/at25.yaml
index 121a601db22e..840ee7a83a14 100644
--- a/Documentation/devicetree/bindings/eeprom/at25.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at25.yaml
@@ -4,14 +4,16 @@
$id: "http://devicetree.org/schemas/eeprom/at25.yaml#"
$schema: "http://devicetree.org/meta-schemas/core.yaml#"
-title: SPI EEPROMs compatible with Atmel's AT25
+title: SPI EEPROMs or FRAMs compatible with Atmel's AT25
maintainers:
- Christian Eggers <[email protected]>
properties:
$nodename:
- pattern: "^eeprom@[0-9a-f]{1,2}$"
+ anyOf:
+ - pattern: "^eeprom@[0-9a-f]{1,2}$"
+ - pattern: "^fram@[0-9a-f]{1,2}$"
# There are multiple known vendors who manufacture EEPROM chips compatible
# with Atmel's AT25. The compatible string requires two items where the
@@ -31,6 +33,7 @@ properties:
- microchip,25lc040
- st,m95m02
- st,m95256
+ - cypress,fm25
- const: atmel,at25
@@ -48,7 +51,7 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
enum: [1, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072]
description:
- Size of the eeprom page.
+ Size of the eeprom page. FRAMs don't have pages.
size:
$ref: /schemas/types.yaml#/definitions/uint32
@@ -101,9 +104,19 @@ required:
- compatible
- reg
- spi-max-frequency
- - pagesize
- - size
- - address-width
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ not:
+ contains:
+ const: cypress,fm25
+ then:
+ required:
+ - pagesize
+ - size
+ - address-width
additionalProperties: false
@@ -126,4 +139,10 @@ examples:
size = <32768>;
address-width = <16>;
};
+
+ fram@1 {
+ compatible = "cypress,fm25", "atmel,at25";
+ reg = <1>;
+ spi-max-frequency = <40000000>;
+ };
};
--
2.25.1
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
a4 36 44 f2 ae 6c 00 00
Signed-off-by: Jiri Prchal <[email protected]>
---
v2: no change here
v3: resend and added more recipients
v4: resend
---
drivers/misc/eeprom/at25.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 4f6e983c278b..b2cffeb3af2c 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -38,6 +38,7 @@ struct at25_data {
struct nvmem_config nvmem_config;
struct nvmem_device *nvmem;
int has_sernum;
+ char *sernum;
};
#define AT25_WREN 0x06 /* latch the write enable */
@@ -172,6 +173,19 @@ static int fm25_aux_read(struct at25_data *at25, char *buf, uint8_t command,
return status;
}
+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);
+}
+static DEVICE_ATTR_RO(sernum);
+
static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
{
struct at25_data *at25 = priv;
@@ -427,8 +441,13 @@ static int at25_probe(struct spi_device *spi)
else
at25->chip.flags |= EE_ADDR2;
- if (id[8])
+ if (id[8]) {
at25->has_sernum = 1;
+ at25->sernum = kzalloc(FM25_SN_LEN, GFP_KERNEL);
+ if (!at25->sernum)
+ return -ENOMEM;
+ fm25_aux_read(at25, at25->sernum, FM25_RDSN, FM25_SN_LEN);
+ }
else
at25->has_sernum = 0;
@@ -467,6 +486,13 @@ static int at25_probe(struct spi_device *spi)
if (IS_ERR(at25->nvmem))
return PTR_ERR(at25->nvmem);
+ /* Export the FM25 serial number */
+ if (at25->has_sernum) {
+ err = device_create_file(&spi->dev, &dev_attr_sernum);
+ if (err)
+ return err;
+ }
+
dev_info(&spi->dev, "%d %s %s %s%s, pagesize %u\n",
(chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
(chip.byte_len < 1024) ? "Byte" : "KByte",
--
2.25.1
Added support for Cypress FRAMs.
These frams have ID and some of them have serial number too.
Size of them is recognized by ID. They don't have pages, it could
be read or written at once, but it's limited in this driver to
io limit 4096.
Signed-off-by: Jiri Prchal <[email protected]>
---
v2: fixed warning at %zd at int
Reported-by: kernel test robot <[email protected]>
v3: resend and added more recipients
v4: resend
---
drivers/misc/eeprom/Kconfig | 5 +-
drivers/misc/eeprom/at25.c | 151 +++++++++++++++++++++++++++++-------
2 files changed, 124 insertions(+), 32 deletions(-)
diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index 0f791bfdc1f5..f0a7531f354c 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -32,12 +32,13 @@ config EEPROM_AT24
will be called at24.
config EEPROM_AT25
- tristate "SPI EEPROMs from most vendors"
+ tristate "SPI EEPROMs (FRAMs) from most vendors"
depends on SPI && SYSFS
select NVMEM
select NVMEM_SYSFS
help
- Enable this driver to get read/write support to most SPI EEPROMs,
+ Enable this driver to get read/write support to most SPI EEPROMs
+ and Cypress FRAMs,
after you configure the board init code to know about each eeprom
on your target board.
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index b76e4901b4a4..4f6e983c278b 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* at25.c -- support most SPI EEPROMs, such as Atmel AT25 models
+ * and Cypress FRAMs FM25 models
*
* Copyright (C) 2006 David Brownell
*/
@@ -16,6 +17,8 @@
#include <linux/spi/spi.h>
#include <linux/spi/eeprom.h>
#include <linux/property.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
/*
* NOTE: this is an *EEPROM* driver. The vagaries of product naming
@@ -34,6 +37,7 @@ struct at25_data {
unsigned addrlen;
struct nvmem_config nvmem_config;
struct nvmem_device *nvmem;
+ int has_sernum;
};
#define AT25_WREN 0x06 /* latch the write enable */
@@ -42,6 +46,9 @@ struct at25_data {
#define AT25_WRSR 0x01 /* write status register */
#define AT25_READ 0x03 /* read byte(s) */
#define AT25_WRITE 0x02 /* write byte(s)/sector */
+#define FM25_SLEEP 0xb9 /* enter sleep mode */
+#define FM25_RDID 0x9f /* read device ID */
+#define FM25_RDSN 0xc3 /* read S/N */
#define AT25_SR_nRDY 0x01 /* nRDY = write-in-progress */
#define AT25_SR_WEN 0x02 /* write enable (latched) */
@@ -51,6 +58,9 @@ struct at25_data {
#define AT25_INSTR_BIT3 0x08 /* Additional address bit in instr */
+#define FM25_ID_LEN 9 /* ID length */
+#define FM25_SN_LEN 8 /* serial number length */
+
#define EE_MAXADDRLEN 3 /* 24 bit addresses, up to 2 MBytes */
/* Specs often allow 5 msec for a page write, sometimes 20 msec;
@@ -58,6 +68,9 @@ struct at25_data {
*/
#define EE_TIMEOUT 25
+#define IS_EEPROM 0
+#define IS_FRAM 1
+
/*-------------------------------------------------------------------------*/
#define io_limit PAGE_SIZE /* bytes */
@@ -129,6 +142,36 @@ static int at25_ee_read(void *priv, unsigned int offset,
return status;
}
+/*
+ * read extra registers as ID or serial number
+ */
+static int fm25_aux_read(struct at25_data *at25, char *buf, uint8_t command,
+ int len)
+{
+ int status;
+ struct spi_transfer t[2];
+ struct spi_message m;
+
+ spi_message_init(&m);
+ memset(t, 0, sizeof(t));
+
+ t[0].tx_buf = &command;
+ t[0].len = 1;
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].rx_buf = buf;
+ t[1].len = len;
+ spi_message_add_tail(&t[1], &m);
+
+ mutex_lock(&at25->lock);
+
+ status = spi_sync(at25->spi, &m);
+ dev_dbg(&at25->spi->dev, "read %d aux bytes --> %d\n", len, status);
+
+ mutex_unlock(&at25->lock);
+ return status;
+}
+
static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
{
struct at25_data *at25 = priv;
@@ -303,34 +346,48 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
return 0;
}
+static const struct of_device_id at25_of_match[] = {
+ { .compatible = "atmel,at25", .data = (const void *)IS_EEPROM },
+ { .compatible = "cypress,fm25", .data = (const void *)IS_FRAM },
+ { }
+};
+MODULE_DEVICE_TABLE(of, at25_of_match);
+
+static int mypow(int x, int n)
+{
+ int i;
+ int res = 1;
+
+ for (i = 0; i < n; ++i)
+ res *= x;
+
+ return res;
+}
+
static int at25_probe(struct spi_device *spi)
{
struct at25_data *at25 = NULL;
struct spi_eeprom chip;
int err;
int sr;
- int addrlen;
+ char id[FM25_ID_LEN];
+ const struct of_device_id *match;
+ int is_fram = 0;
+
+ match = of_match_device(of_match_ptr(at25_of_match), &spi->dev);
+ if (match)
+ is_fram = (int)(uintptr_t)match->data;
/* Chip description */
if (!spi->dev.platform_data) {
- err = at25_fw_to_chip(&spi->dev, &chip);
- if (err)
- return err;
+ if (!is_fram) {
+ err = at25_fw_to_chip(&spi->dev, &chip);
+ if (err)
+ return err;
+ }
} else
chip = *(struct spi_eeprom *)spi->dev.platform_data;
- /* For now we only support 8/16/24 bit addressing */
- if (chip.flags & EE_ADDR1)
- addrlen = 1;
- else if (chip.flags & EE_ADDR2)
- addrlen = 2;
- else if (chip.flags & EE_ADDR3)
- addrlen = 3;
- else {
- dev_dbg(&spi->dev, "unsupported address type\n");
- return -EINVAL;
- }
-
/* Ping the chip ... the status register is pretty portable,
* unlike probing manufacturer IDs. We do expect that system
* firmware didn't write it in the past few milliseconds!
@@ -349,9 +406,49 @@ static int at25_probe(struct spi_device *spi)
at25->chip = chip;
at25->spi = spi;
spi_set_drvdata(spi, at25);
- at25->addrlen = addrlen;
- at25->nvmem_config.type = NVMEM_TYPE_EEPROM;
+ if (is_fram) {
+ /* Get ID of chip */
+ fm25_aux_read(at25, id, FM25_RDID, FM25_ID_LEN);
+ if (id[6] != 0xc2) {
+ dev_err(&spi->dev,
+ "Error: no Cypress FRAM (id %02x)\n", id[6]);
+ return -ENODEV;
+ }
+ /* set size found in ID */
+ if (id[7] < 0x21 || id[7] > 0x26) {
+ dev_err(&spi->dev, "Error: unsupported size (id %02x)\n", id[7]);
+ return -ENODEV;
+ }
+ chip.byte_len = mypow(2, id[7] - 0x21 + 4) * 1024;
+
+ if (at25->chip.byte_len > 64 * 1024)
+ at25->chip.flags |= EE_ADDR3;
+ else
+ at25->chip.flags |= EE_ADDR2;
+
+ if (id[8])
+ at25->has_sernum = 1;
+ else
+ at25->has_sernum = 0;
+
+ at25->chip.page_size = PAGE_SIZE;
+ strncpy(at25->chip.name, "fm25", sizeof(at25->chip.name));
+ }
+
+ /* For now we only support 8/16/24 bit addressing */
+ if (at25->chip.flags & EE_ADDR1)
+ at25->addrlen = 1;
+ else if (at25->chip.flags & EE_ADDR2)
+ at25->addrlen = 2;
+ else if (at25->chip.flags & EE_ADDR3)
+ at25->addrlen = 3;
+ else {
+ dev_dbg(&spi->dev, "unsupported address type\n");
+ return -EINVAL;
+ }
+
+ at25->nvmem_config.type = is_fram ? NVMEM_TYPE_FRAM : NVMEM_TYPE_EEPROM;
at25->nvmem_config.name = dev_name(&spi->dev);
at25->nvmem_config.dev = &spi->dev;
at25->nvmem_config.read_only = chip.flags & EE_READONLY;
@@ -370,23 +467,17 @@ static int at25_probe(struct spi_device *spi)
if (IS_ERR(at25->nvmem))
return PTR_ERR(at25->nvmem);
- dev_info(&spi->dev, "%d %s %s eeprom%s, pagesize %u\n",
- (chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
- (chip.byte_len < 1024) ? "Byte" : "KByte",
- at25->chip.name,
- (chip.flags & EE_READONLY) ? " (readonly)" : "",
- at25->chip.page_size);
+ dev_info(&spi->dev, "%d %s %s %s%s, pagesize %u\n",
+ (chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
+ (chip.byte_len < 1024) ? "Byte" : "KByte",
+ at25->chip.name, is_fram ? "fram" : "eeprom",
+ (chip.flags & EE_READONLY) ? " (readonly)" : "",
+ at25->chip.page_size);
return 0;
}
/*-------------------------------------------------------------------------*/
-static const struct of_device_id at25_of_match[] = {
- { .compatible = "atmel,at25", },
- { }
-};
-MODULE_DEVICE_TABLE(of, at25_of_match);
-
static struct spi_driver at25_driver = {
.driver = {
.name = "at25",
--
2.25.1
On Thu, May 20, 2021 at 07:47:14AM +0200, Jiri Prchal wrote:
> 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?
> a4 36 44 f2 ae 6c 00 00
>
> Signed-off-by: Jiri Prchal <[email protected]>
> ---
> v2: no change here
> v3: resend and added more recipients
> v4: resend
> ---
> drivers/misc/eeprom/at25.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
> index 4f6e983c278b..b2cffeb3af2c 100644
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -38,6 +38,7 @@ struct at25_data {
> struct nvmem_config nvmem_config;
> struct nvmem_device *nvmem;
> int has_sernum;
> + char *sernum;
> };
>
> #define AT25_WREN 0x06 /* latch the write enable */
> @@ -172,6 +173,19 @@ static int fm25_aux_read(struct at25_data *at25, char *buf, uint8_t command,
> return status;
> }
>
> +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"?
Also, please use sysfs_emit() in the future.
> +}
> +static DEVICE_ATTR_RO(sernum);
> +
> static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> {
> struct at25_data *at25 = priv;
> @@ -427,8 +441,13 @@ static int at25_probe(struct spi_device *spi)
> else
> at25->chip.flags |= EE_ADDR2;
>
> - if (id[8])
> + if (id[8]) {
> at25->has_sernum = 1;
> + at25->sernum = kzalloc(FM25_SN_LEN, GFP_KERNEL);
> + if (!at25->sernum)
> + return -ENOMEM;
> + fm25_aux_read(at25, at25->sernum, FM25_RDSN, FM25_SN_LEN);
> + }
> else
> at25->has_sernum = 0;
>
> @@ -467,6 +486,13 @@ static int at25_probe(struct spi_device *spi)
> if (IS_ERR(at25->nvmem))
> return PTR_ERR(at25->nvmem);
>
> + /* 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 :(
Please do this correctly, by setting the driver group if you need a file
like this.
thanks,
greg k-h
On Thu, May 20, 2021 at 07:47:12AM +0200, Jiri Prchal wrote:
> +static int mypow(int x, int n)
> +{
> + int i;
> + int res = 1;
> +
> + for (i = 0; i < n; ++i)
> + res *= x;
> +
> + return res;
> +}
We really don't have a in-kernel function for this today?
On Thu, May 20, 2021 at 07:47:11AM +0200, Jiri Prchal wrote:
> Added enum and string for FRAM to expose it as "fram".
documentation???
Here I'm completlly lost:
On 20. 05. 21 7:52, Greg Kroah-Hartman wrote:
> On Thu, May 20, 2021 at 07:47:14AM +0200, Jiri Prchal wrote:
>> 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?
>> +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?
>
> Also, please use sysfs_emit() in the future.
Will do...
>
>
>
>> +}
>> +static DEVICE_ATTR_RO(sernum);
>> +
>> static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
>> {
>> struct at25_data *at25 = priv;
>> @@ -427,8 +441,13 @@ static int at25_probe(struct spi_device *spi)
>> else
>> at25->chip.flags |= EE_ADDR2;
>>
>> - if (id[8])
>> + if (id[8]) {
>> at25->has_sernum = 1;
>> + at25->sernum = kzalloc(FM25_SN_LEN, GFP_KERNEL);
>> + if (!at25->sernum)
>> + return -ENOMEM;
>> + fm25_aux_read(at25, at25->sernum, FM25_RDSN, FM25_SN_LEN);
>> + }
>> else
>> at25->has_sernum = 0;
>>
>> @@ -467,6 +486,13 @@ static int at25_probe(struct spi_device *spi)
>> if (IS_ERR(at25->nvmem))
>> return PTR_ERR(at25->nvmem);
>>
>> + /* 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 :(
?
>
> Please do this correctly, by setting the driver group if you need a file
> like this.
Any example, please?
>
> thanks,
>
> greg k-h
>
thanks
Jiri
On 20. 05. 21 7:53, Greg Kroah-Hartman wrote:
> On Thu, May 20, 2021 at 07:47:11AM +0200, Jiri Prchal wrote:
>> Added enum and string for FRAM to expose it as "fram".
>
> documentation???
>
Hi Greg,
please help me, you mean more explanation in commit message?
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:
> > On Thu, May 20, 2021 at 07:47:14AM +0200, Jiri Prchal wrote:
> > > 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.
> > > +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.
> > > + /* 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 :(
> ?
> >
> > 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
On Thu, May 20, 2021 at 08:54:52AM +0200, Jiří Prchal wrote:
>
> On 20. 05. 21 7:53, Greg Kroah-Hartman wrote:
> > On Thu, May 20, 2021 at 07:47:11AM +0200, Jiri Prchal wrote:
> > > Added enum and string for FRAM to expose it as "fram".
> >
> > documentation???
> >
> Hi Greg,
> please help me, you mean more explanation in commit message?
No, I mean you need to document user/kernel apis that you add in
Documentation/ABI/ please.
thanks,
greg k-h