[RFC PATCH 0/3] Add at24 functionality to read form different EEPROM offsets

[RFC PATCH 0/3] Add at24 functionality to read form different EEPROM offsets

[Claudiu Beznea]

Hi all,

Please give feedback on the following approach to extends at24 EEPROM
driver to support reading MAC from different addresses in EEPROM (patch
2 and 3 from this series). This will allow the at24 driver to be used
as a generic driver by different hardwares.

This series also contains a patch (patch 1 from this series) which document
all at24 EEPROM memories and it was pushed with this series because patch
3 from the series is based on it.

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  dt-bindings: i2c: eeprom: document all at24 bindings
  drivers: misc: eeprom: at24: support reading mac eeprom from
    different addresses
  dt-bindings: i2c: eeprom: document "start-offset" binding

 Documentation/devicetree/bindings/eeprom/eeprom.txt | 20 +++++++++++++-------
 drivers/misc/eeprom/at24.c                          |  5 ++++-
 include/linux/platform_data/at24.h                  |  1 +
 3 files changed, 18 insertions(+), 8 deletions(-)

-- 
2.7.4

[RFC PATCH 0/3] Add at24 functionality to read form different EEPROM offsets

[Claudiu Beznea]

Hi all,

Please give feedback on the following approach to extends at24 EEPROM
driver to support reading MAC from different addresses in EEPROM (patch
2 and 3 from this series). This will allow the at24 driver to be used
as a generic driver by different hardwares.

This series also contains a patch (patch 1 from this series) which document
all at24 EEPROM memories and it was pushed with this series because patch
3 from the series is based on it.

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  dt-bindings: i2c: eeprom: document all at24 bindings
  drivers: misc: eeprom: at24: support reading mac eeprom from
    different addresses
  dt-bindings: i2c: eeprom: document "start-offset" binding

 Documentation/devicetree/bindings/eeprom/eeprom.txt | 20 +++++++++++++-------
 drivers/misc/eeprom/at24.c                          |  5 ++++-
 include/linux/platform_data/at24.h                  |  1 +
 3 files changed, 18 insertions(+), 8 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] dt-bindings: i2c: eeprom: document all at24 bindings

[Claudiu Beznea]

Document all at24 memories specific bindings.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/eeprom/eeprom.txt | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt
index 5696eb5..a50dc01 100644
--- a/Documentation/devicetree/bindings/eeprom/eeprom.txt
+++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt
@@ -4,9 +4,11 @@ Required properties:
 
   - compatible : should be "<manufacturer>,<type>", like these:
 
-	"atmel,24c00", "atmel,24c01", "atmel,24c02", "atmel,24c04",
-	"atmel,24c08", "atmel,24c16", "atmel,24c32", "atmel,24c64",
-	"atmel,24c128", "atmel,24c256", "atmel,24c512", "atmel,24c1024"
+	"atmel,24c00", "atmel,24c01", "atmel,24cs01", "atmel,24c02", "atmel,24cs02",
+	"atmel,24mac402", "atmel,24mac602", "atmel,spd", "atmel,24c04",
+	"atmel,24cs04", "atmel,24c08", "atmel,24cs08", "atmel,24c16", "atmel,24cs16",
+	"atmel,24c32", "atmel,24cs32", "atmel,24c64", "atmel,24cs64", "atmel,24c128",
+	"atmel,24c256", "atmel,24c512", "atmel,24c1024"
 
 	"catalyst,24c32"
 
@@ -16,10 +18,11 @@ Required properties:
 
 	"renesas,r1ex24002"
 
-	 If there is no specific driver for <manufacturer>, a generic
-	 driver based on <type> is selected. Possible types are:
-	 "24c00", "24c01", "24c02", "24c04", "24c08", "24c16", "24c32", "24c64",
-	 "24c128", "24c256", "24c512", "24c1024", "spd"
+	If there is no specific driver for <manufacturer>, a generic
+	driver based on <type> is selected. Possible types are:
+	"24c00", "24c01", "24cs01", "24c02", "24cs02", "24mac402", "24mac602", "spd",
+	"24c04", "24cs04", "24c08", "24cs08", "24c16", "24cs16", "24c32", "24cs32",
+	"24c64", "24cs64", "24c128", "24c256", "24c512", "24c1024"
 
   - reg : the I2C address of the EEPROM
 
-- 
2.7.4

[PATCH 1/3] dt-bindings: i2c: eeprom: document all at24 bindings

[Claudiu Beznea]

Document all at24 memories specific bindings.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/eeprom/eeprom.txt | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt
index 5696eb5..a50dc01 100644
--- a/Documentation/devicetree/bindings/eeprom/eeprom.txt
+++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt
@@ -4,9 +4,11 @@ Required properties:
 
   - compatible : should be "<manufacturer>,<type>", like these:
 
-	"atmel,24c00", "atmel,24c01", "atmel,24c02", "atmel,24c04",
-	"atmel,24c08", "atmel,24c16", "atmel,24c32", "atmel,24c64",
-	"atmel,24c128", "atmel,24c256", "atmel,24c512", "atmel,24c1024"
+	"atmel,24c00", "atmel,24c01", "atmel,24cs01", "atmel,24c02", "atmel,24cs02",
+	"atmel,24mac402", "atmel,24mac602", "atmel,spd", "atmel,24c04",
+	"atmel,24cs04", "atmel,24c08", "atmel,24cs08", "atmel,24c16", "atmel,24cs16",
+	"atmel,24c32", "atmel,24cs32", "atmel,24c64", "atmel,24cs64", "atmel,24c128",
+	"atmel,24c256", "atmel,24c512", "atmel,24c1024"
 
 	"catalyst,24c32"
 
@@ -16,10 +18,11 @@ Required properties:
 
 	"renesas,r1ex24002"
 
-	 If there is no specific driver for <manufacturer>, a generic
-	 driver based on <type> is selected. Possible types are:
-	 "24c00", "24c01", "24c02", "24c04", "24c08", "24c16", "24c32", "24c64",
-	 "24c128", "24c256", "24c512", "24c1024", "spd"
+	If there is no specific driver for <manufacturer>, a generic
+	driver based on <type> is selected. Possible types are:
+	"24c00", "24c01", "24cs01", "24c02", "24cs02", "24mac402", "24mac602", "spd",
+	"24c04", "24cs04", "24c08", "24cs08", "24c16", "24cs16", "24c32", "24cs32",
+	"24c64", "24cs64", "24c128", "24c256", "24c512", "24c1024"
 
   - reg : the I2C address of the EEPROM
 
-- 
2.7.4

[RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[Claudiu Beznea]

Add support for reading from different offsets of EEPROM.
The offset is initialized via device tree. If nothing is
given as input the old value, 0x90, is used. In this way
the driver could be used as generic driver for different
vendor memories by only changing the reading offset via
device tree.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/misc/eeprom/at24.c         | 5 ++++-
 include/linux/platform_data/at24.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 764ff5df..53c5fb8 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
 	memset(msg, 0, sizeof(msg));
 	msg[0].addr = client->addr;
 	msg[0].buf = addrbuf;
-	addrbuf[0] = 0x90 + offset;
+	addrbuf[0] = at24->chip.offset + offset;
 	msg[0].len = 1;
 	msg[1].addr = client->addr;
 	msg[1].flags = I2C_M_RD;
@@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 		 */
 		chip->page_size = 1;
 	}
+	err = device_property_read_u8(dev, "start-offset", &chip->offset);
+	if (err)
+		chip->offset = 0x90;
 }
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
index 271a4e2..d55c454 100644
--- a/include/linux/platform_data/at24.h
+++ b/include/linux/platform_data/at24.h
@@ -50,6 +50,7 @@ struct at24_platform_data {
 #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
 #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
 #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
+	u8		offset;
 
 	void		(*setup)(struct nvmem_device *nvmem, void *context);
 	void		*context;
-- 
2.7.4

[RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[Claudiu Beznea]

Add support for reading from different offsets of EEPROM.
The offset is initialized via device tree. If nothing is
given as input the old value, 0x90, is used. In this way
the driver could be used as generic driver for different
vendor memories by only changing the reading offset via
device tree.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/misc/eeprom/at24.c         | 5 ++++-
 include/linux/platform_data/at24.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 764ff5df..53c5fb8 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
 	memset(msg, 0, sizeof(msg));
 	msg[0].addr = client->addr;
 	msg[0].buf = addrbuf;
-	addrbuf[0] = 0x90 + offset;
+	addrbuf[0] = at24->chip.offset + offset;
 	msg[0].len = 1;
 	msg[1].addr = client->addr;
 	msg[1].flags = I2C_M_RD;
@@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 		 */
 		chip->page_size = 1;
 	}
+	err = device_property_read_u8(dev, "start-offset", &chip->offset);
+	if (err)
+		chip->offset = 0x90;
 }
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
index 271a4e2..d55c454 100644
--- a/include/linux/platform_data/at24.h
+++ b/include/linux/platform_data/at24.h
@@ -50,6 +50,7 @@ struct at24_platform_data {
 #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
 #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
 #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
+	u8		offset;
 
 	void		(*setup)(struct nvmem_device *nvmem, void *context);
 	void		*context;
-- 
2.7.4

[RFC PATCH 3/3] dt-bindings: i2c: eeprom: document "start-offset" binding

[Claudiu Beznea]

Document "start-offset" binding that will be used by at24
EEPROM driver.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt
index a50dc01..e0bec50 100644
--- a/Documentation/devicetree/bindings/eeprom/eeprom.txt
+++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt
@@ -35,10 +35,13 @@ Optional properties:
 
   - read-only: this parameterless property disables writes to the eeprom
 
+  - start-offset: offset in EEPROM to start reading from
+
 Example:
 
 eeprom@52 {
 	compatible = "atmel,24c32";
 	reg = <0x52>;
 	pagesize = <32>;
+	start-offset = /bits/ 8 <0xf8>;
 };
-- 
2.7.4

[RFC PATCH 3/3] dt-bindings: i2c: eeprom: document "start-offset" binding

[Claudiu Beznea]

Document "start-offset" binding that will be used by at24
EEPROM driver.

Signed-off-by: Claudiu Beznea <claudiu.beznea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
---
 Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt
index a50dc01..e0bec50 100644
--- a/Documentation/devicetree/bindings/eeprom/eeprom.txt
+++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt
@@ -35,10 +35,13 @@ Optional properties:
 
   - read-only: this parameterless property disables writes to the eeprom
 
+  - start-offset: offset in EEPROM to start reading from
+
 Example:
 
 eeprom@52 {
 	compatible = "atmel,24c32";
 	reg = <0x52>;
 	pagesize = <32>;
+	start-offset = /bits/ 8 <0xf8>;
 };
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[Andy Shevchenko]

On Thu, Jun 29, 2017 at 2:39 PM, Claudiu Beznea
<claudiu.beznea@microchip.com> wrote:
> Add support for reading from different offsets of EEPROM.
> The offset is initialized via device tree. If nothing is
> given as input the old value, 0x90, is used. In this way
> the driver could be used as generic driver for different
> vendor memories by only changing the reading offset via
> device tree.

> +       err = device_property_read_u8(dev, "start-offset", &chip->offset);

This property has to be documented.

Moreover, the name looks too broad (I checked current use of
"*-offset" type of properties and they either vendor specific, or data
type specific.

Taking above into consideration mac-offset sounds better to me.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[Ludovic Desroches]

On Thu, Jun 29, 2017 at 02:56:03PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 29, 2017 at 2:39 PM, Claudiu Beznea
> <claudiu.beznea@microchip.com> wrote:
> > Add support for reading from different offsets of EEPROM.
> > The offset is initialized via device tree. If nothing is
> > given as input the old value, 0x90, is used. In this way
> > the driver could be used as generic driver for different
> > vendor memories by only changing the reading offset via
> > device tree.
> 
> > +       err = device_property_read_u8(dev, "start-offset", &chip->offset);
> 
> This property has to be documented.
> 
> Moreover, the name looks too broad (I checked current use of
> "*-offset" type of properties and they either vendor specific, or data
> type specific.
> 
> Taking above into consideration mac-offset sounds better to me.

I agree, it sounds better to me too.

Regards

Ludovic

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[David Lechner]

On 06/29/2017 06:39 AM, Claudiu Beznea wrote:
> Add support for reading from different offsets of EEPROM.
> The offset is initialized via device tree. If nothing is
> given as input the old value, 0x90, is used. In this way
> the driver could be used as generic driver for different
> vendor memories by only changing the reading offset via
> device tree.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>   drivers/misc/eeprom/at24.c         | 5 ++++-
>   include/linux/platform_data/at24.h | 1 +
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 764ff5df..53c5fb8 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
>   	memset(msg, 0, sizeof(msg));
>   	msg[0].addr = client->addr;
>   	msg[0].buf = addrbuf;
> -	addrbuf[0] = 0x90 + offset;
> +	addrbuf[0] = at24->chip.offset + offset;
>   	msg[0].len = 1;
>   	msg[1].addr = client->addr;
>   	msg[1].flags = I2C_M_RD;
> @@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
>   		 */
>   		chip->page_size = 1;
>   	}
> +	err = device_property_read_u8(dev, "start-offset", &chip->offset);

Why use u8 here? I have an at24 EEPROM that uses 16-bit addressing where 
the MAC address is stored at 0x3F06. Seems like it would be better to 
just use u32, then you don't have to mess with /bits/ in the device tree 
binding.

> +	if (err)
> +		chip->offset = 0x90;
>   }
>   
>   static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
> index 271a4e2..d55c454 100644
> --- a/include/linux/platform_data/at24.h
> +++ b/include/linux/platform_data/at24.h
> @@ -50,6 +50,7 @@ struct at24_platform_data {
>   #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
>   #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
>   #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
> +	u8		offset;
>   
>   	void		(*setup)(struct nvmem_device *nvmem, void *context);
>   	void		*context;
> 

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[David Lechner]

On 06/29/2017 06:39 AM, Claudiu Beznea wrote:
> Add support for reading from different offsets of EEPROM.
> The offset is initialized via device tree. If nothing is
> given as input the old value, 0x90, is used. In this way
> the driver could be used as generic driver for different
> vendor memories by only changing the reading offset via
> device tree.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> ---
>   drivers/misc/eeprom/at24.c         | 5 ++++-
>   include/linux/platform_data/at24.h | 1 +
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 764ff5df..53c5fb8 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
>   	memset(msg, 0, sizeof(msg));
>   	msg[0].addr = client->addr;
>   	msg[0].buf = addrbuf;
> -	addrbuf[0] = 0x90 + offset;
> +	addrbuf[0] = at24->chip.offset + offset;
>   	msg[0].len = 1;
>   	msg[1].addr = client->addr;
>   	msg[1].flags = I2C_M_RD;
> @@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
>   		 */
>   		chip->page_size = 1;
>   	}
> +	err = device_property_read_u8(dev, "start-offset", &chip->offset);

Why use u8 here? I have an at24 EEPROM that uses 16-bit addressing where 
the MAC address is stored at 0x3F06. Seems like it would be better to 
just use u32, then you don't have to mess with /bits/ in the device tree 
binding.

> +	if (err)
> +		chip->offset = 0x90;
>   }
>   
>   static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
> index 271a4e2..d55c454 100644
> --- a/include/linux/platform_data/at24.h
> +++ b/include/linux/platform_data/at24.h
> @@ -50,6 +50,7 @@ struct at24_platform_data {
>   #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
>   #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
>   #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
> +	u8		offset;
>   
>   	void		(*setup)(struct nvmem_device *nvmem, void *context);
>   	void		*context;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[Uwe Kleine-König]

Hallo,

On Thu, Jun 29, 2017 at 02:39:10PM +0300, Claudiu Beznea wrote:
> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
> index 271a4e2..d55c454 100644
> --- a/include/linux/platform_data/at24.h
> +++ b/include/linux/platform_data/at24.h
> @@ -50,6 +50,7 @@ struct at24_platform_data {
>  #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
>  #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
>  #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
> +	u8		offset;

Assuming you rename the property, please adapt the member name here
accordingly.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[Uwe Kleine-König]

Hallo,

On Thu, Jun 29, 2017 at 02:39:10PM +0300, Claudiu Beznea wrote:
> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
> index 271a4e2..d55c454 100644
> --- a/include/linux/platform_data/at24.h
> +++ b/include/linux/platform_data/at24.h
> @@ -50,6 +50,7 @@ struct at24_platform_data {
>  #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
>  #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
>  #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
> +	u8		offset;

Assuming you rename the property, please adapt the member name here
accordingly.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[m18063]

On 29.06.2017 16:30, Ludovic Desroches wrote:
> On Thu, Jun 29, 2017 at 02:56:03PM +0300, Andy Shevchenko wrote:
>> On Thu, Jun 29, 2017 at 2:39 PM, Claudiu Beznea
>> <claudiu.beznea@microchip.com> wrote:
>>> Add support for reading from different offsets of EEPROM.
>>> The offset is initialized via device tree. If nothing is
>>> given as input the old value, 0x90, is used. In this way
>>> the driver could be used as generic driver for different
>>> vendor memories by only changing the reading offset via
>>> device tree.
>>
>>> +       err = device_property_read_u8(dev, "start-offset", &chip->offset);
>>
>> This property has to be documented.
Sure! In this patch series it was documented in patch 2 from series.
Are you talking about documenting it in another place?

>>
>> Moreover, the name looks too broad (I checked current use of
>> "*-offset" type of properties and they either vendor specific, or data
>> type specific.
>>
>> Taking above into consideration mac-offset sounds better to me.
Ok, I will use that name.
> 
> I agree, it sounds better to me too.
Ok, I will use that name.
Thanks,
Claudiu
> 
> Regards
> 
> Ludovic
> 

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[m18063]

On 29.06.2017 18:32, David Lechner wrote:
> On 06/29/2017 06:39 AM, Claudiu Beznea wrote:
>> Add support for reading from different offsets of EEPROM.
>> The offset is initialized via device tree. If nothing is
>> given as input the old value, 0x90, is used. In this way
>> the driver could be used as generic driver for different
>> vendor memories by only changing the reading offset via
>> device tree.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>   drivers/misc/eeprom/at24.c         | 5 ++++-
>>   include/linux/platform_data/at24.h | 1 +
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
>> index 764ff5df..53c5fb8 100644
>> --- a/drivers/misc/eeprom/at24.c
>> +++ b/drivers/misc/eeprom/at24.c
>> @@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
>>       memset(msg, 0, sizeof(msg));
>>       msg[0].addr = client->addr;
>>       msg[0].buf = addrbuf;
>> -    addrbuf[0] = 0x90 + offset;
>> +    addrbuf[0] = at24->chip.offset + offset;
>>       msg[0].len = 1;
>>       msg[1].addr = client->addr;
>>       msg[1].flags = I2C_M_RD;
>> @@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
>>            */
>>           chip->page_size = 1;
>>       }
>> +    err = device_property_read_u8(dev, "start-offset", &chip->offset);
> 
> Why use u8 here? I have an at24 EEPROM that uses 16-bit addressing where the MAC address is stored at 0x3F06. Seems like it would be better to just use u32, then you don't have to mess with /bits/ in the device tree binding.
Agree! I will use u32 instead. It is better that way.

Thanks,
Claudiu
> 
>> +    if (err)
>> +        chip->offset = 0x90;
>>   }
>>     static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
>> index 271a4e2..d55c454 100644
>> --- a/include/linux/platform_data/at24.h
>> +++ b/include/linux/platform_data/at24.h
>> @@ -50,6 +50,7 @@ struct at24_platform_data {
>>   #define AT24_FLAG_TAKE8ADDR    BIT(4)    /* take always 8 addresses (24c00) */
>>   #define AT24_FLAG_SERIAL    BIT(3)    /* factory-programmed serial number */
>>   #define AT24_FLAG_MAC        BIT(2)    /* factory-programmed mac address */
>> +    u8        offset;
>>         void        (*setup)(struct nvmem_device *nvmem, void *context);
>>       void        *context;
>>
> 

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[m18063]

On 29.06.2017 18:32, David Lechner wrote:
> On 06/29/2017 06:39 AM, Claudiu Beznea wrote:
>> Add support for reading from different offsets of EEPROM.
>> The offset is initialized via device tree. If nothing is
>> given as input the old value, 0x90, is used. In this way
>> the driver could be used as generic driver for different
>> vendor memories by only changing the reading offset via
>> device tree.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
>> ---
>>   drivers/misc/eeprom/at24.c         | 5 ++++-
>>   include/linux/platform_data/at24.h | 1 +
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
>> index 764ff5df..53c5fb8 100644
>> --- a/drivers/misc/eeprom/at24.c
>> +++ b/drivers/misc/eeprom/at24.c
>> @@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
>>       memset(msg, 0, sizeof(msg));
>>       msg[0].addr = client->addr;
>>       msg[0].buf = addrbuf;
>> -    addrbuf[0] = 0x90 + offset;
>> +    addrbuf[0] = at24->chip.offset + offset;
>>       msg[0].len = 1;
>>       msg[1].addr = client->addr;
>>       msg[1].flags = I2C_M_RD;
>> @@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
>>            */
>>           chip->page_size = 1;
>>       }
>> +    err = device_property_read_u8(dev, "start-offset", &chip->offset);
> 
> Why use u8 here? I have an at24 EEPROM that uses 16-bit addressing where the MAC address is stored at 0x3F06. Seems like it would be better to just use u32, then you don't have to mess with /bits/ in the device tree binding.
Agree! I will use u32 instead. It is better that way.

Thanks,
Claudiu
> 
>> +    if (err)
>> +        chip->offset = 0x90;
>>   }
>>     static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
>> index 271a4e2..d55c454 100644
>> --- a/include/linux/platform_data/at24.h
>> +++ b/include/linux/platform_data/at24.h
>> @@ -50,6 +50,7 @@ struct at24_platform_data {
>>   #define AT24_FLAG_TAKE8ADDR    BIT(4)    /* take always 8 addresses (24c00) */
>>   #define AT24_FLAG_SERIAL    BIT(3)    /* factory-programmed serial number */
>>   #define AT24_FLAG_MAC        BIT(2)    /* factory-programmed mac address */
>> +    u8        offset;
>>         void        (*setup)(struct nvmem_device *nvmem, void *context);
>>       void        *context;
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[m18063]

On 30.06.2017 10:20, Uwe Kleine-König wrote:
> Hallo,
> 
> On Thu, Jun 29, 2017 at 02:39:10PM +0300, Claudiu Beznea wrote:
>> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
>> index 271a4e2..d55c454 100644
>> --- a/include/linux/platform_data/at24.h
>> +++ b/include/linux/platform_data/at24.h
>> @@ -50,6 +50,7 @@ struct at24_platform_data {
>>  #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
>>  #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
>>  #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
>> +	u8		offset;
> 
> Assuming you rename the property, please adapt the member name here
> accordingly.
Sure!

Thanks,
Claudiu
> 
> Best regards
> Uwe
> 

Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[m18063]

On 30.06.2017 10:20, Uwe Kleine-König wrote:
> Hallo,
> 
> On Thu, Jun 29, 2017 at 02:39:10PM +0300, Claudiu Beznea wrote:
>> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
>> index 271a4e2..d55c454 100644
>> --- a/include/linux/platform_data/at24.h
>> +++ b/include/linux/platform_data/at24.h
>> @@ -50,6 +50,7 @@ struct at24_platform_data {
>>  #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
>>  #define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
>>  #define AT24_FLAG_MAC		BIT(2)	/* factory-programmed mac address */
>> +	u8		offset;
> 
> Assuming you rename the property, please adapt the member name here
> accordingly.
Sure!

Thanks,
Claudiu
> 
> Best regards
> Uwe
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/3] dt-bindings: i2c: eeprom: document "start-offset" binding

[Rob Herring]

On Thu, Jun 29, 2017 at 02:39:11PM +0300, Claudiu Beznea wrote:
> Document "start-offset" binding that will be used by at24
> EEPROM driver.

Why do we need this property?

Seems like this should be handled by defining regions of the EEPROM that 
you care about (or perhaps are reserved and don't want to touch). Just a 
single offset value is not very flexible. 

> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt
> index a50dc01..e0bec50 100644
> --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt
> +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt
> @@ -35,10 +35,13 @@ Optional properties:
>  
>    - read-only: this parameterless property disables writes to the eeprom
>  
> +  - start-offset: offset in EEPROM to start reading from
> +
>  Example:
>  
>  eeprom@52 {
>  	compatible = "atmel,24c32";
>  	reg = <0x52>;
>  	pagesize = <32>;
> +	start-offset = /bits/ 8 <0xf8>;

The size of the property is part of the ABI. Unless you *never* need 
offset of more than 256, then this should be 32-bit.

Rob