2018-06-26 06:23:43

by Chiang, AlanX

[permalink] [raw]
Subject: [PATCH v2 0/2] Add a property in at24.c

From: "alanx.chiang" <[email protected]>

In at24.c, it uses 8-bit addressing by default. In this patch,
add a property address-width that provides a flexible method to
pass the information to the driver.

alanx.chiang (2):
dt-bindings: at24: Add address-width property
eeprom: at24: Add support for address-width property

Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
drivers/misc/eeprom/at24.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+)

--
2.7.4



2018-06-26 06:23:42

by Chiang, AlanX

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: at24: Add address-width property

From: "alanx.chiang" <[email protected]>

The AT24 series chips use 8-bit address by default. If some
chips would like to support more than 8 bits, should add the compatible
field for specfic chips in the driver.

Provide a flexible way to determine the addressing bits through
address-width in this patch.

Signed-off-by: Alan Chiang <[email protected]>
Signed-off-by: Andy Yeh <[email protected]>

---
since v1:
-- Remove the address-width field in the example.

---
Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/eeprom/at24.txt b/Documentation/devicetree/bindings/eeprom/at24.txt
index 61d833a..9467482 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.txt
+++ b/Documentation/devicetree/bindings/eeprom/at24.txt
@@ -72,6 +72,8 @@ Optional properties:

- wp-gpios: GPIO to which the write-protect pin of the chip is connected.

+ - address-width : number of address bits (one of 8, 16).
+
Example:

eeprom@52 {
--
2.7.4


2018-06-26 06:23:42

by Chiang, AlanX

[permalink] [raw]
Subject: [PATCH v2 2/2] eeprom: at24: Add support for address-width property

From: "alanx.chiang" <[email protected]>

Provide a flexible way to determine the addressing bits of eeprom.
Pass the addressing bits to driver through address-width property.

Signed-off-by: Alan Chiang <[email protected]>
Signed-off-by: Andy Yeh <[email protected]>

---
since v1
-- Add a warn message for 8-bit addressing.

---
drivers/misc/eeprom/at24.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 0c125f2..231afcd 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -478,6 +478,22 @@ static void at24_properties_to_pdata(struct device *dev,
if (device_property_present(dev, "no-read-rollover"))
chip->flags |= AT24_FLAG_NO_RDROL;

+ err = device_property_read_u32(dev, "address-width", &val);
+ if (!err) {
+ switch (val) {
+ case 8:
+ chip->flags &= ~AT24_FLAG_ADDR16;
+ dev_warn(dev, "address-width is 8, clear the ADD16 bit\n");
+ break;
+ case 16:
+ chip->flags |= AT24_FLAG_ADDR16;
+ break;
+ default:
+ dev_warn(dev, "Bad \"address-width\" property: %u\n",
+ val);
+ }
+ }
+
err = device_property_read_u32(dev, "size", &val);
if (!err)
chip->byte_len = val;
--
2.7.4


2018-06-26 06:48:46

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] eeprom: at24: Add support for address-width property

Hi Alan,

On Tue, Jun 26, 2018 at 02:22:08PM +0800, [email protected] wrote:
> From: "alanx.chiang" <[email protected]>
>
> Provide a flexible way to determine the addressing bits of eeprom.
> Pass the addressing bits to driver through address-width property.
>
> Signed-off-by: Alan Chiang <[email protected]>
> Signed-off-by: Andy Yeh <[email protected]>
>
> ---
> since v1
> -- Add a warn message for 8-bit addressing.
>
> ---
> drivers/misc/eeprom/at24.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 0c125f2..231afcd 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -478,6 +478,22 @@ static void at24_properties_to_pdata(struct device *dev,
> if (device_property_present(dev, "no-read-rollover"))
> chip->flags |= AT24_FLAG_NO_RDROL;
>
> + err = device_property_read_u32(dev, "address-width", &val);
> + if (!err) {
> + switch (val) {
> + case 8:
> + chip->flags &= ~AT24_FLAG_ADDR16;
> + dev_warn(dev, "address-width is 8, clear the ADD16 bit\n");

Even though the default is 8 address bits, I don't see a need to issue a
warning if the address-width property sets that to 8 explicitly. I.e. only
warn if the flag was set.

> + break;
> + case 16:
> + chip->flags |= AT24_FLAG_ADDR16;
> + break;
> + default:
> + dev_warn(dev, "Bad \"address-width\" property: %u\n",
> + val);
> + }
> + }
> +
> err = device_property_read_u32(dev, "size", &val);
> if (!err)
> chip->byte_len = val;

--
Regards,

Sakari Ailus
[email protected]

2018-06-26 06:51:04

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: at24: Add address-width property

Hi Alan,

On Tue, Jun 26, 2018 at 02:22:07PM +0800, [email protected] wrote:
> From: "alanx.chiang" <[email protected]>
>
> The AT24 series chips use 8-bit address by default. If some
> chips would like to support more than 8 bits, should add the compatible
> field for specfic chips in the driver.
>
> Provide a flexible way to determine the addressing bits through
> address-width in this patch.
>
> Signed-off-by: Alan Chiang <[email protected]>
> Signed-off-by: Andy Yeh <[email protected]>
>
> ---
> since v1:
> -- Remove the address-width field in the example.
>
> ---
> Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.txt b/Documentation/devicetree/bindings/eeprom/at24.txt
> index 61d833a..9467482 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.txt
> +++ b/Documentation/devicetree/bindings/eeprom/at24.txt
> @@ -72,6 +72,8 @@ Optional properties:
>
> - wp-gpios: GPIO to which the write-protect pin of the chip is connected.
>
> + - address-width : number of address bits (one of 8, 16).

Please remove the space before the colon; that way it looks the same as the
rest.

> +
> Example:
>
> eeprom@52 {

--
Kind regards,

Sakari Ailus
[email protected]

2018-06-26 07:12:53

by Chiang, AlanX

[permalink] [raw]
Subject: RE: [PATCH v2 2/2] eeprom: at24: Add support for address-width property

Hi Sakari,

> -----Original Message-----
> From: Sakari Ailus [mailto:[email protected]]
> Sent: Tuesday, June 26, 2018 2:48 PM
> To: Chiang, AlanX <[email protected]>
> Cc: [email protected]; Yeh, Andy <[email protected]>;
> Shevchenko, Andriy <[email protected]>; Mani, Rajmohan
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/2] eeprom: at24: Add support for address-width
> property
>
> Hi Alan,
>
> On Tue, Jun 26, 2018 at 02:22:08PM +0800, [email protected] wrote:
> > From: "alanx.chiang" <[email protected]>
> >
> > Provide a flexible way to determine the addressing bits of eeprom.
> > Pass the addressing bits to driver through address-width property.
> >
> > Signed-off-by: Alan Chiang <[email protected]>
> > Signed-off-by: Andy Yeh <[email protected]>
> >
> > ---
> > since v1
> > -- Add a warn message for 8-bit addressing.
> >
> > ---
> > drivers/misc/eeprom/at24.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> > index 0c125f2..231afcd 100644
> > --- a/drivers/misc/eeprom/at24.c
> > +++ b/drivers/misc/eeprom/at24.c
> > @@ -478,6 +478,22 @@ static void at24_properties_to_pdata(struct device
> *dev,
> > if (device_property_present(dev, "no-read-rollover"))
> > chip->flags |= AT24_FLAG_NO_RDROL;
> >
> > + err = device_property_read_u32(dev, "address-width", &val);
> > + if (!err) {
> > + switch (val) {
> > + case 8:
> > + chip->flags &= ~AT24_FLAG_ADDR16;
> > + dev_warn(dev, "address-width is 8, clear the ADD16
> bit\n");
>
> Even though the default is 8 address bits, I don't see a need to issue a
> warning if the address-width property sets that to 8 explicitly. I.e. only warn
> if the flag was set.
>

Do you mean I have to add a statement for checking if the bit has been set before?
For example:

If (chip->flags & AT24_FLAG_ADDR16)
dev_warn(dev, "address-width is 8, clear the ADD16 bit\n");

If it is, I would like to modify it as below:

case 8:
If (chip->flags & AT24_FLAG_ADDR16) {
chip->flags &= ~AT24_FLAG_ADDR16;
dev_warn(dev, "address-width is 8, clear the ADDR16 bit\n");
}
break;

> > + break;
> > + case 16:
> > + chip->flags |= AT24_FLAG_ADDR16;
> > + break;
> > + default:
> > + dev_warn(dev, "Bad \"address-width\" property:
> %u\n",
> > + val);
> > + }
> > + }
> > +
> > err = device_property_read_u32(dev, "size", &val);
> > if (!err)
> > chip->byte_len = val;
>
> --
> Regards,
>
> Sakari Ailus
> [email protected]

2018-06-26 07:27:32

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] eeprom: at24: Add support for address-width property

On Tue, Jun 26, 2018 at 07:11:54AM +0000, Chiang, AlanX wrote:
> Hi Sakari,
>
> > -----Original Message-----
> > From: Sakari Ailus [mailto:[email protected]]
> > Sent: Tuesday, June 26, 2018 2:48 PM
> > To: Chiang, AlanX <[email protected]>
> > Cc: [email protected]; Yeh, Andy <[email protected]>;
> > Shevchenko, Andriy <[email protected]>; Mani, Rajmohan
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v2 2/2] eeprom: at24: Add support for address-width
> > property
> >
> > Hi Alan,
> >
> > On Tue, Jun 26, 2018 at 02:22:08PM +0800, [email protected] wrote:
> > > From: "alanx.chiang" <[email protected]>
> > >
> > > Provide a flexible way to determine the addressing bits of eeprom.
> > > Pass the addressing bits to driver through address-width property.
> > >
> > > Signed-off-by: Alan Chiang <[email protected]>
> > > Signed-off-by: Andy Yeh <[email protected]>
> > >
> > > ---
> > > since v1
> > > -- Add a warn message for 8-bit addressing.
> > >
> > > ---
> > > drivers/misc/eeprom/at24.c | 16 ++++++++++++++++
> > > 1 file changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> > > index 0c125f2..231afcd 100644
> > > --- a/drivers/misc/eeprom/at24.c
> > > +++ b/drivers/misc/eeprom/at24.c
> > > @@ -478,6 +478,22 @@ static void at24_properties_to_pdata(struct device
> > *dev,
> > > if (device_property_present(dev, "no-read-rollover"))
> > > chip->flags |= AT24_FLAG_NO_RDROL;
> > >
> > > + err = device_property_read_u32(dev, "address-width", &val);
> > > + if (!err) {
> > > + switch (val) {
> > > + case 8:
> > > + chip->flags &= ~AT24_FLAG_ADDR16;
> > > + dev_warn(dev, "address-width is 8, clear the ADD16
> > bit\n");
> >
> > Even though the default is 8 address bits, I don't see a need to issue a
> > warning if the address-width property sets that to 8 explicitly. I.e. only warn
> > if the flag was set.
> >
>
> Do you mean I have to add a statement for checking if the bit has been set before?
> For example:
>
> If (chip->flags & AT24_FLAG_ADDR16)
> dev_warn(dev, "address-width is 8, clear the ADD16 bit\n");
>
> If it is, I would like to modify it as below:
>
> case 8:
> If (chip->flags & AT24_FLAG_ADDR16) {
> chip->flags &= ~AT24_FLAG_ADDR16;
> dev_warn(dev, "address-width is 8, clear the ADDR16 bit\n");
> }
> break;

Seems good to me.

--
Sakari Ailus
[email protected]

2018-06-26 07:42:56

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

2018-06-26 8:22 GMT+02:00 <[email protected]>:
> From: "alanx.chiang" <[email protected]>
>
> In at24.c, it uses 8-bit addressing by default. In this patch,
> add a property address-width that provides a flexible method to
> pass the information to the driver.
>
> alanx.chiang (2):
> dt-bindings: at24: Add address-width property
> eeprom: at24: Add support for address-width property
>
> Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
> drivers/misc/eeprom/at24.c | 16 ++++++++++++++++
> 2 files changed, 18 insertions(+)
>
> --
> 2.7.4
>

What is your use case exactly? Do you have an EEPROM model that's not
yet supported explicitly in the driver? Why would you need this
option?

Best regards,
Bartosz Golaszewski

2018-06-26 12:13:57

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] eeprom: at24: Add support for address-width property

On Tue, 2018-06-26 at 15:11 +0800, Chiang, AlanX wrote:

> If it is, I would like to modify it as below:
>
> case 8:
> If (chip->flags & AT24_FLAG_ADDR16) {
> chip->flags &= ~AT24_FLAG_ADDR16;
> dev_warn(dev, "address-width is 8, clear the ADDR16
> bit\n");
> }
> break;

No need to put bit clearing inside the loop, something like below would
be slightly better.

if (chip->flags & AT24_FLAG_ADDR16)
dev_warn(dev, "address-width is 8, clear the ADDR16 bit\n");
chip->flags &= ~AT24_FLAG_ADDR16;

On top of this the message would sound clearer if you put it like

"Override address width to be 8, while default is 16"

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2018-06-26 12:16:10

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:
> 2018-06-26 8:22 GMT+02:00 <[email protected]>:
> > From: "alanx.chiang" <[email protected]>
> >
> > In at24.c, it uses 8-bit addressing by default. In this patch,
> > add a property address-width that provides a flexible method to
> > pass the information to the driver.
> >
> > alanx.chiang (2):
> > dt-bindings: at24: Add address-width property
> > eeprom: at24: Add support for address-width property
> >
> > Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
> > drivers/misc/eeprom/at24.c | 16
> > ++++++++++++++++
> > 2 files changed, 18 insertions(+)
> >
> > --
> > 2.7.4
> >
>
> What is your use case exactly? Do you have an EEPROM model that's not
> yet supported explicitly in the driver? Why would you need this
> option?

The current at24 driver has no address width support, thus, reusing same
(allocated) IDs (non-DT case) is hard.

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2018-06-26 12:37:10

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

2018-06-26 14:14 GMT+02:00 Andy Shevchenko <[email protected]>:
> On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:
>> 2018-06-26 8:22 GMT+02:00 <[email protected]>:
>> > From: "alanx.chiang" <[email protected]>
>> >
>> > In at24.c, it uses 8-bit addressing by default. In this patch,
>> > add a property address-width that provides a flexible method to
>> > pass the information to the driver.
>> >
>> > alanx.chiang (2):
>> > dt-bindings: at24: Add address-width property
>> > eeprom: at24: Add support for address-width property
>> >
>> > Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
>> > drivers/misc/eeprom/at24.c | 16
>> > ++++++++++++++++
>> > 2 files changed, 18 insertions(+)
>> >
>> > --
>> > 2.7.4
>> >
>>
>> What is your use case exactly? Do you have an EEPROM model that's not
>> yet supported explicitly in the driver? Why would you need this
>> option?
>
> The current at24 driver has no address width support, thus, reusing same
> (allocated) IDs (non-DT case) is hard.
>

Every supported compatible has the width already specified in its
corresponding chip data.

Bart

2018-06-26 13:25:52

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

On Tue, 2018-06-26 at 14:36 +0200, Bartosz Golaszewski wrote:
> 2018-06-26 14:14 GMT+02:00 Andy Shevchenko
> <[email protected]>:
> > On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:

> > > What is your use case exactly? Do you have an EEPROM model that's
> > > not
> > > yet supported explicitly in the driver? Why would you need this
> > > option?
> >
> > The current at24 driver has no address width support,

> > thus, reusing same
> > (allocated) IDs (non-DT case) is hard.

^^^^^

> Every supported compatible has the width already specified in its
> corresponding chip data.


Please, read again carefully what I wrote before.

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2018-06-26 13:32:38

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

2018-06-26 15:23 GMT+02:00 Andy Shevchenko <[email protected]>:
> On Tue, 2018-06-26 at 14:36 +0200, Bartosz Golaszewski wrote:
>> 2018-06-26 14:14 GMT+02:00 Andy Shevchenko
>> <[email protected]>:
>> > On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:
>
>> > > What is your use case exactly? Do you have an EEPROM model that's
>> > > not
>> > > yet supported explicitly in the driver? Why would you need this
>> > > option?
>> >
>> > The current at24 driver has no address width support,
>
>> > thus, reusing same
>> > (allocated) IDs (non-DT case) is hard.
>
> ^^^^^
>
>> Every supported compatible has the width already specified in its
>> corresponding chip data.
>
>
> Please, read again carefully what I wrote before.
>

Ok makes sense in that case. Could you just point me towards an
example model which has the address width different than the default
for its type?

Bart

2018-06-26 15:50:16

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

On Tue, 2018-06-26 at 15:30 +0200, Bartosz Golaszewski wrote:
> 2018-06-26 15:23 GMT+02:00 Andy Shevchenko <[email protected]
> tel.com>:
> > On Tue, 2018-06-26 at 14:36 +0200, Bartosz Golaszewski wrote:
> > > 2018-06-26 14:14 GMT+02:00 Andy Shevchenko
> > > <[email protected]>:
> > > > On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:
> > > > > What is your use case exactly? Do you have an EEPROM model
> > > > > that's
> > > > > not
> > > > > yet supported explicitly in the driver? Why would you need
> > > > > this
> > > > > option?
> > > >
> > > > The current at24 driver has no address width support,
> > > > thus, reusing same
> > > > (allocated) IDs (non-DT case) is hard.
> >
> > ^^^^^
> >
> > > Every supported compatible has the width already specified in its
> > > corresponding chip data.
> >
> >
> > Please, read again carefully what I wrote before.
> >
>
> Ok makes sense in that case. Could you just point me towards an
> example model which has the address width different than the default
> for its type?

AFAIK, it's a companion device inside the camera voice coil IC, i.e.
DONGWOON DW9714.

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2018-06-26 16:23:50

by Yeh, Andy

[permalink] [raw]
Subject: RE: [PATCH v2 0/2] Add a property in at24.c

> -----Original Message-----
> From: Andy Shevchenko [mailto:[email protected]]
> Sent: Tuesday, June 26, 2018 11:48 PM
> To: Bartosz Golaszewski <[email protected]>
> Cc: Chiang, AlanX <[email protected]>; linux-i2c <linux-
> [email protected]>; Yeh, Andy <[email protected]>; Sakari Ailus
> <[email protected]>; Mani, Rajmohan <[email protected]>;
> Andy Shevchenko <[email protected]>; Rob Herring
> <[email protected]>; Mark Rutland <[email protected]>; Arnd
> Bergmann <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>
> Subject: Re: [PATCH v2 0/2] Add a property in at24.c
>
> On Tue, 2018-06-26 at 15:30 +0200, Bartosz Golaszewski wrote:
> > 2018-06-26 15:23 GMT+02:00 Andy Shevchenko
> <[email protected]
> > tel.com>:
> > > On Tue, 2018-06-26 at 14:36 +0200, Bartosz Golaszewski wrote:
> > > > 2018-06-26 14:14 GMT+02:00 Andy Shevchenko
> > > > <[email protected]>:
> > > > > On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:
> > > > > > What is your use case exactly? Do you have an EEPROM model
> > > > > > that's not yet supported explicitly in the driver? Why would
> > > > > > you need this option?
> > > > >
> > > > > The current at24 driver has no address width support, thus,
> > > > > reusing same
> > > > > (allocated) IDs (non-DT case) is hard.
> > >
> > > ^^^^^
> > >
> > > > Every supported compatible has the width already specified in its
> > > > corresponding chip data.
> > >
> > >
> > > Please, read again carefully what I wrote before.
> > >
> >
> > Ok makes sense in that case. Could you just point me towards an
> > example model which has the address width different than the default
> > for its type?
>
> AFAIK, it's a companion device inside the camera voice coil IC, i.e.
> DONGWOON DW9714.
>

Nope, actually it is DW9807 instead, which is used on a Samsung Chromebook.

> --
> Andy Shevchenko <[email protected]>
> Intel Finland Oy

2018-06-26 16:33:17

by Mani, Rajmohan

[permalink] [raw]
Subject: RE: [PATCH v2 0/2] Add a property in at24.c

Hi Bartosz,

> -----Original Message-----
> From: Yeh, Andy
> Sent: Tuesday, June 26, 2018 9:21 AM
> To: Andy Shevchenko <[email protected]>; Bartosz
> Golaszewski <[email protected]>
> Cc: Chiang, AlanX <[email protected]>; linux-i2c <linux-
> [email protected]>; Sakari Ailus <[email protected]>; Mani,
> Rajmohan <[email protected]>; Andy Shevchenko
> <[email protected]>; Rob Herring <[email protected]>; Mark
> Rutland <[email protected]>; Arnd Bergmann <[email protected]>; Greg
> Kroah-Hartman <[email protected]>; Linux Kernel Mailing List
> <[email protected]>
> Subject: RE: [PATCH v2 0/2] Add a property in at24.c
>
> > -----Original Message-----
> > From: Andy Shevchenko [mailto:[email protected]]
> > Sent: Tuesday, June 26, 2018 11:48 PM
> > To: Bartosz Golaszewski <[email protected]>
> > Cc: Chiang, AlanX <[email protected]>; linux-i2c <linux-
> > [email protected]>; Yeh, Andy <[email protected]>; Sakari Ailus
> > <[email protected]>; Mani, Rajmohan
> > <[email protected]>; Andy Shevchenko
> > <[email protected]>; Rob Herring <[email protected]>; Mark
> > Rutland <[email protected]>; Arnd Bergmann <[email protected]>; Greg
> > Kroah-Hartman <[email protected]>; Linux Kernel Mailing List
> > <linux- [email protected]>
> > Subject: Re: [PATCH v2 0/2] Add a property in at24.c
> >
> > On Tue, 2018-06-26 at 15:30 +0200, Bartosz Golaszewski wrote:
> > > 2018-06-26 15:23 GMT+02:00 Andy Shevchenko
> > <[email protected]
> > > tel.com>:
> > > > On Tue, 2018-06-26 at 14:36 +0200, Bartosz Golaszewski wrote:
> > > > > 2018-06-26 14:14 GMT+02:00 Andy Shevchenko
> > > > > <[email protected]>:
> > > > > > On Tue, 2018-06-26 at 09:41 +0200, Bartosz Golaszewski wrote:
> > > > > > > What is your use case exactly? Do you have an EEPROM model
> > > > > > > that's not yet supported explicitly in the driver? Why would
> > > > > > > you need this option?
> > > > > >
> > > > > > The current at24 driver has no address width support, thus,
> > > > > > reusing same
> > > > > > (allocated) IDs (non-DT case) is hard.
> > > >
> > > > ^^^^^
> > > >
> > > > > Every supported compatible has the width already specified in
> > > > > its corresponding chip data.
> > > >
> > > >
> > > > Please, read again carefully what I wrote before.
> > > >
> > >
> > > Ok makes sense in that case. Could you just point me towards an
> > > example model which has the address width different than the default
> > > for its type?
> >
> > AFAIK, it's a companion device inside the camera voice coil IC, i.e.
> > DONGWOON DW9714.
> >
>
> Nope, actually it is DW9807 instead, which is used on a Samsung Chromebook.

M24C64S is one example, where reusing same id (non-DT case) is not possible,
since this model uses 16 bits as address width, as the driver supports only
8 bits address width as default.

Thanks
Raj

2018-06-26 16:39:26

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Add a property in at24.c

On Tue, 2018-06-26 at 16:21 +0000, Yeh, Andy wrote:

> > > Ok makes sense in that case. Could you just point me towards an
> > > example model which has the address width different than the
> > > default
> > > for its type?
> >
> > AFAIK, it's a companion device inside the camera voice coil IC, i.e.
> > DONGWOON DW9714.
> >
>
> Nope, actually it is DW9807 instead, which is used on a Samsung
> Chromebook.

Ah, okay, good to know.

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2018-06-26 16:46:10

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: at24: Add address-width property

On Tue, Jun 26, 2018 at 12:22 AM <[email protected]> wrote:
>
> From: "alanx.chiang" <[email protected]>

Please fix your author name and send bindings to the DT list if you
want them reviewed.

>
> The AT24 series chips use 8-bit address by default. If some
> chips would like to support more than 8 bits, should add the compatible
> field for specfic chips in the driver.
>
> Provide a flexible way to determine the addressing bits through
> address-width in this patch.
>
> Signed-off-by: Alan Chiang <[email protected]>
> Signed-off-by: Andy Yeh <[email protected]>
>
> ---
> since v1:
> -- Remove the address-width field in the example.
>
> ---
> Documentation/devicetree/bindings/eeprom/at24.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.txt b/Documentation/devicetree/bindings/eeprom/at24.txt
> index 61d833a..9467482 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.txt
> +++ b/Documentation/devicetree/bindings/eeprom/at24.txt
> @@ -72,6 +72,8 @@ Optional properties:
>
> - wp-gpios: GPIO to which the write-protect pin of the chip is connected.
>
> + - address-width : number of address bits (one of 8, 16).
> +
> Example:
>
> eeprom@52 {
> --
> 2.7.4
>