[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Michal Simek]

Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
fixed-partition are using reg property instead of offset/size pair.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Bootcount tested on zynqmp zcu104
- Add missing address/size cells
- Use dev_read_addr_size_index
- Check parameters

Just build tested - ge_bx50v3_defconfig
Definitely please retest on hardware.

---
 arch/arm/dts/imx53-ppd-uboot.dtsi    | 15 +++++++++------
 arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
 drivers/misc/i2c_eeprom.c            | 20 ++++++++++----------
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/arch/arm/dts/imx53-ppd-uboot.dtsi b/arch/arm/dts/imx53-ppd-uboot.dtsi
index d38a1bc264c9..b308a517a73c 100644
--- a/arch/arm/dts/imx53-ppd-uboot.dtsi
+++ b/arch/arm/dts/imx53-ppd-uboot.dtsi
@@ -22,17 +22,20 @@
 };
 
 &eeprom {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
 	partitions {
 		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
 
-		vpd {
-			offset = <0>;
-			size = <1022>;
+		vpd at 0 {
+			reg = <0 1022>;
 		};
 
-		bootcount: bootcount {
-			offset = <1022>;
-			size = <2>;
+		bootcount: bootcount at 1022 {
+			reg = <1022 2>;
 		};
 	};
 };
diff --git a/arch/arm/dts/imx6q-bx50v3-uboot.dtsi b/arch/arm/dts/imx6q-bx50v3-uboot.dtsi
index df446e0ed149..996eb18046c7 100644
--- a/arch/arm/dts/imx6q-bx50v3-uboot.dtsi
+++ b/arch/arm/dts/imx6q-bx50v3-uboot.dtsi
@@ -21,17 +21,19 @@
 };
 
 &eeprom {
+	#address-cells = <1>;
+	#size-cells = <0>;
 	partitions {
 		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
 
-		vpd {
-			offset = <0>;
-			size = <1022>;
+		vpd at 0 {
+			reg = <0 1022>;
 		};
 
 		bootcount: bootcount {
-			offset = <1022>;
-			size = <2>;
+			reg = <1022 2>;
 		};
 	};
 };
diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
index 45c34d388c8a..335cf70e8b7e 100644
--- a/drivers/misc/i2c_eeprom.c
+++ b/drivers/misc/i2c_eeprom.c
@@ -301,19 +301,19 @@ static int i2c_eeprom_partition_probe(struct udevice *dev)
 static int i2c_eeprom_partition_ofdata_to_platdata(struct udevice *dev)
 {
 	struct i2c_eeprom_partition *priv = dev_get_priv(dev);
-	u32 offset, size;
-	int ret;
+	fdt_size_t addr, size;
 
-	ret = dev_read_u32(dev, "offset", &offset);
-	if (ret)
-		return ret;
+	addr = dev_read_addr_size_index(dev, 0, &size);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
 
-	ret = dev_read_u32(dev, "size", &size);
-	if (ret)
-		return ret;
+	if (!size)
+		return -EINVAL;
+
+	priv->offset = (u32)addr;
+	priv->size = (u32)size;
 
-	priv->offset = offset;
-	priv->size = size;
+	debug("%s: base %x, size %x\n", __func__, priv->offset, priv->size);
 
 	return 0;
 }
-- 
2.27.0

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Simon Glass]

On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com> wrote:
>
> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
> fixed-partition are using reg property instead of offset/size pair.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Changes in v2:
> - Bootcount tested on zynqmp zcu104
> - Add missing address/size cells
> - Use dev_read_addr_size_index
> - Check parameters
>
> Just build tested - ge_bx50v3_defconfig
> Definitely please retest on hardware.
>
> ---
>  arch/arm/dts/imx53-ppd-uboot.dtsi    | 15 +++++++++------
>  arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
>  drivers/misc/i2c_eeprom.c            | 20 ++++++++++----------
>  3 files changed, 26 insertions(+), 21 deletions(-)
>

We have a sandbox I2C EEPROM, so you should be able to use the
existing test, right?

REgards,
Simon

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Michal Simek]

On 16. 06. 20 15:43, Simon Glass wrote:
> On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
>> fixed-partition are using reg property instead of offset/size pair.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Changes in v2:
>> - Bootcount tested on zynqmp zcu104
>> - Add missing address/size cells
>> - Use dev_read_addr_size_index
>> - Check parameters
>>
>> Just build tested - ge_bx50v3_defconfig
>> Definitely please retest on hardware.
>>
>> ---
>>  arch/arm/dts/imx53-ppd-uboot.dtsi    | 15 +++++++++------
>>  arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
>>  drivers/misc/i2c_eeprom.c            | 20 ++++++++++----------
>>  3 files changed, 26 insertions(+), 21 deletions(-)
>>
> 
> We have a sandbox I2C EEPROM, so you should be able to use the
> existing test, right?

The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
driver which define which eeprom stores it.
Do you have any existing tests for bootcount done via sandbox?

If bootcount is not the right way to go then doing this code should be
better way. It means just define some partitions (0 size - for failure,
then proper range, proper write, write behind size for failure).

Thanks,
Michal

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Simon Glass]

Hi Michal,

On Tue, 16 Jun 2020 at 07:53, Michal Simek <michal.simek@xilinx.com> wrote:
>
>
>
> On 16. 06. 20 15:43, Simon Glass wrote:
> > On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com> wrote:
> >>
> >> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
> >> fixed-partition are using reg property instead of offset/size pair.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >> Changes in v2:
> >> - Bootcount tested on zynqmp zcu104
> >> - Add missing address/size cells
> >> - Use dev_read_addr_size_index
> >> - Check parameters
> >>
> >> Just build tested - ge_bx50v3_defconfig
> >> Definitely please retest on hardware.
> >>
> >> ---
> >>  arch/arm/dts/imx53-ppd-uboot.dtsi    | 15 +++++++++------
> >>  arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
> >>  drivers/misc/i2c_eeprom.c            | 20 ++++++++++----------
> >>  3 files changed, 26 insertions(+), 21 deletions(-)
> >>
> >
> > We have a sandbox I2C EEPROM, so you should be able to use the
> > existing test, right?
>
> The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
> driver which define which eeprom stores it.
> Do you have any existing tests for bootcount done via sandbox?
>
> If bootcount is not the right way to go then doing this code should be
> better way. It means just define some partitions (0 size - for failure,
> then proper range, proper write, write behind size for failure).

Can you use drivers/misc/i2c_eeprom.c?

See test/dm/bootcount.c for the sandbox tests for bootcount.

Regards,
Simon
>

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Heiko Schocher]

Hi Michal,

Am 17.06.2020 um 05:12 schrieb Simon Glass:
> Hi Michal,
> 
> On Tue, 16 Jun 2020 at 07:53, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>>
>>
>> On 16. 06. 20 15:43, Simon Glass wrote:
>>> On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>
>>>> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
>>>> fixed-partition are using reg property instead of offset/size pair.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - Bootcount tested on zynqmp zcu104
>>>> - Add missing address/size cells
>>>> - Use dev_read_addr_size_index
>>>> - Check parameters
>>>>
>>>> Just build tested - ge_bx50v3_defconfig
>>>> Definitely please retest on hardware.
>>>>
>>>> ---
>>>>   arch/arm/dts/imx53-ppd-uboot.dtsi    | 15 +++++++++------
>>>>   arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
>>>>   drivers/misc/i2c_eeprom.c            | 20 ++++++++++----------
>>>>   3 files changed, 26 insertions(+), 21 deletions(-)
>>>>
>>>
>>> We have a sandbox I2C EEPROM, so you should be able to use the
>>> existing test, right?
>>
>> The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
>> driver which define which eeprom stores it.
>> Do you have any existing tests for bootcount done via sandbox?
>>
>> If bootcount is not the right way to go then doing this code should be
>> better way. It means just define some partitions (0 size - for failure,
>> then proper range, proper write, write behind size for failure).
> 
> Can you use drivers/misc/i2c_eeprom.c?
> 
> See test/dm/bootcount.c for the sandbox tests for bootcount.

Any updates on this?

@Robert: May you find time to test this change and give us feedback?

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Michal Simek]

On 06. 07. 20 7:56, Heiko Schocher wrote:
> Hi Michal,
> 
> Am 17.06.2020 um 05:12 schrieb Simon Glass:
>> Hi Michal,
>>
>> On Tue, 16 Jun 2020 at 07:53, Michal Simek <michal.simek@xilinx.com>
>> wrote:
>>>
>>>
>>>
>>> On 16. 06. 20 15:43, Simon Glass wrote:
>>>> On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com>
>>>> wrote:
>>>>>
>>>>> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
>>>>> fixed-partition are using reg property instead of offset/size pair.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - Bootcount tested on zynqmp zcu104
>>>>> - Add missing address/size cells
>>>>> - Use dev_read_addr_size_index
>>>>> - Check parameters
>>>>>
>>>>> Just build tested - ge_bx50v3_defconfig
>>>>> Definitely please retest on hardware.
>>>>>
>>>>> ---
>>>>> ? arch/arm/dts/imx53-ppd-uboot.dtsi??? | 15 +++++++++------
>>>>> ? arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
>>>>> ? drivers/misc/i2c_eeprom.c??????????? | 20 ++++++++++----------
>>>>> ? 3 files changed, 26 insertions(+), 21 deletions(-)
>>>>>
>>>>
>>>> We have a sandbox I2C EEPROM, so you should be able to use the
>>>> existing test, right?
>>>
>>> The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
>>> driver which define which eeprom stores it.
>>> Do you have any existing tests for bootcount done via sandbox?
>>>
>>> If bootcount is not the right way to go then doing this code should be
>>> better way. It means just define some partitions (0 size - for failure,
>>> then proper range, proper write, write behind size for failure).
>>
>> Can you use drivers/misc/i2c_eeprom.c?
>>
>> See test/dm/bootcount.c for the sandbox tests for bootcount.
> 
> Any updates on this?
> 
> @Robert: May you find time to test this change and give us feedback?

Just came from my vacation and it will take some time to get to this.
If anybody wants to create that test feel free to do it.
Definitely testing on Robert side would be good.

Thanks,
Michal

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Michal Simek]

On 08. 07. 20 12:55, Michal Simek wrote:
> 
> 
> On 06. 07. 20 7:56, Heiko Schocher wrote:
>> Hi Michal,
>>
>> Am 17.06.2020 um 05:12 schrieb Simon Glass:
>>> Hi Michal,
>>>
>>> On Tue, 16 Jun 2020 at 07:53, Michal Simek <michal.simek@xilinx.com>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On 16. 06. 20 15:43, Simon Glass wrote:
>>>>> On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com>
>>>>> wrote:
>>>>>>
>>>>>> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
>>>>>> fixed-partition are using reg property instead of offset/size pair.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Bootcount tested on zynqmp zcu104
>>>>>> - Add missing address/size cells
>>>>>> - Use dev_read_addr_size_index
>>>>>> - Check parameters
>>>>>>
>>>>>> Just build tested - ge_bx50v3_defconfig
>>>>>> Definitely please retest on hardware.
>>>>>>
>>>>>> ---
>>>>>> ? arch/arm/dts/imx53-ppd-uboot.dtsi??? | 15 +++++++++------
>>>>>> ? arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
>>>>>> ? drivers/misc/i2c_eeprom.c??????????? | 20 ++++++++++----------
>>>>>> ? 3 files changed, 26 insertions(+), 21 deletions(-)
>>>>>>
>>>>>
>>>>> We have a sandbox I2C EEPROM, so you should be able to use the
>>>>> existing test, right?
>>>>
>>>> The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
>>>> driver which define which eeprom stores it.
>>>> Do you have any existing tests for bootcount done via sandbox?
>>>>
>>>> If bootcount is not the right way to go then doing this code should be
>>>> better way. It means just define some partitions (0 size - for failure,
>>>> then proper range, proper write, write behind size for failure).
>>>
>>> Can you use drivers/misc/i2c_eeprom.c?
>>>
>>> See test/dm/bootcount.c for the sandbox tests for bootcount.
>>
>> Any updates on this?
>>
>> @Robert: May you find time to test this change and give us feedback?
> 
> Just came from my vacation and it will take some time to get to this.
> If anybody wants to create that test feel free to do it.
> Definitely testing on Robert side would be good.

I have sent v3 of this patch with adding testcase for i2c eeprom based
bootcount as Simon mentioned. Also tested it on xilinx zynqmp zcu104
board. Please take a look at it.

Thanks,
Michal

[PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Heiko Schocher]

Hi Michel,

Am 22.07.2020 um 12:23 schrieb Michal Simek:
> 
> 
> On 08. 07. 20 12:55, Michal Simek wrote:
>>
>>
>> On 06. 07. 20 7:56, Heiko Schocher wrote:
>>> Hi Michal,
>>>
>>> Am 17.06.2020 um 05:12 schrieb Simon Glass:
>>>> Hi Michal,
>>>>
>>>> On Tue, 16 Jun 2020 at 07:53, Michal Simek <michal.simek@xilinx.com>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 16. 06. 20 15:43, Simon Glass wrote:
>>>>>> On Mon, 15 Jun 2020 at 07:41, Michal Simek <michal.simek@xilinx.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
>>>>>>> fixed-partition are using reg property instead of offset/size pair.
>>>>>>>
>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>> - Bootcount tested on zynqmp zcu104
>>>>>>> - Add missing address/size cells
>>>>>>> - Use dev_read_addr_size_index
>>>>>>> - Check parameters
>>>>>>>
>>>>>>> Just build tested - ge_bx50v3_defconfig
>>>>>>> Definitely please retest on hardware.
>>>>>>>
>>>>>>> ---
>>>>>>>  ? arch/arm/dts/imx53-ppd-uboot.dtsi??? | 15 +++++++++------
>>>>>>>  ? arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++++++-----
>>>>>>>  ? drivers/misc/i2c_eeprom.c??????????? | 20 ++++++++++----------
>>>>>>>  ? 3 files changed, 26 insertions(+), 21 deletions(-)
>>>>>>>
>>>>>>
>>>>>> We have a sandbox I2C EEPROM, so you should be able to use the
>>>>>> existing test, right?
>>>>>
>>>>> The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
>>>>> driver which define which eeprom stores it.
>>>>> Do you have any existing tests for bootcount done via sandbox?
>>>>>
>>>>> If bootcount is not the right way to go then doing this code should be
>>>>> better way. It means just define some partitions (0 size - for failure,
>>>>> then proper range, proper write, write behind size for failure).
>>>>
>>>> Can you use drivers/misc/i2c_eeprom.c?
>>>>
>>>> See test/dm/bootcount.c for the sandbox tests for bootcount.
>>>
>>> Any updates on this?
>>>
>>> @Robert: May you find time to test this change and give us feedback?
>>
>> Just came from my vacation and it will take some time to get to this.
>> If anybody wants to create that test feel free to do it.
>> Definitely testing on Robert side would be good.
> 
> I have sent v3 of this patch with adding testcase for i2c eeprom based
> bootcount as Simon mentioned. Also tested it on xilinx zynqmp zcu104
> board. Please take a look at it.

Thanks!

Already superseeded this patch in patchwork.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de