* [PATCH v3 0/1] at24: support eeproms that do not auto-rollover reads. @ 2017-12-04 15:36 Sven Van Asbroeck 2017-12-04 15:36 ` [PATCH v3 1/1] " Sven Van Asbroeck 0 siblings, 1 reply; 7+ messages in thread From: Sven Van Asbroeck @ 2017-12-04 15:36 UTC (permalink / raw) To: svendev, robh+dt, mark.rutland, wsa, brgl, nsekhar, sakari.ailus, david, javier, divagar.mohandass Cc: devicetree, linux-kernel, linux-i2c, Sven Van Asbroeck From: Sven Van Asbroeck <svenv@arcx.com> v3: rebased against at24 maintainer's devel staging branch: git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git at24/devel clarified some of the comments and wording v2: kbuild test robot feedback: correct "warning: comparison of distinct pointer types lacks a cast" build warning on some compilers / architectures. v1: original patch Sven Van Asbroeck (1): at24: support eeproms that do not auto-rollover reads. .../devicetree/bindings/eeprom/eeprom.txt | 5 +++ drivers/misc/eeprom/at24.c | 37 +++++++++++++++------- include/linux/platform_data/at24.h | 2 ++ 3 files changed, 32 insertions(+), 12 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads. 2017-12-04 15:36 [PATCH v3 0/1] at24: support eeproms that do not auto-rollover reads Sven Van Asbroeck @ 2017-12-04 15:36 ` Sven Van Asbroeck [not found] ` <1512401778-20366-2-git-send-email-svendev-fuHqz3Nb1YI@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Sven Van Asbroeck @ 2017-12-04 15:36 UTC (permalink / raw) To: svendev, robh+dt, mark.rutland, wsa, brgl, nsekhar, sakari.ailus, david, javier, divagar.mohandass Cc: devicetree, linux-kernel, linux-i2c, Sven Van Asbroeck From: Sven Van Asbroeck <svenv@arcx.com> Some multi-address eeproms in the at24 family may not automatically roll-over reads to the next slave address. On those eeproms, reads that straddle slave boundaries will not work correctly. Solution: Mark such eeproms with a flag that prevents reads straddling slave boundaries. Add the AT24_FLAG_NO_RDROL flag to the eeprom entry in the device_id table, or add 'no-read-rollover' to the eeprom devicetree entry. Note that I have not personally enountered an at24 chip that does not support read rollovers. They may or may not exist. However, my hardware requires this functionality because of a quirk. It's up to the Linux community to decide if this patch is useful/ general enough to warrant merging. Signed-off-by: Sven Van Asbroeck <svendev@arcx.com> --- .../devicetree/bindings/eeprom/eeprom.txt | 5 +++ drivers/misc/eeprom/at24.c | 37 +++++++++++++++------- include/linux/platform_data/at24.h | 2 ++ 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt index 27f2bc1..a1764f8 100644 --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt @@ -38,6 +38,11 @@ Optional properties: - size: total eeprom size in bytes + - no-read-rollover: supported on the at24 eeprom family only. + This parameterless property indicates that the multi-address + eeprom does not automatically roll over reads to the next + slave address. Please consult the manual of your device. + Example: eeprom@52 { diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 625b001..33bca28 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -251,15 +251,6 @@ struct at24_data { * Slave address and byte offset derive from the offset. Always * set the byte address; on a multi-master board, another master * may have changed the chip's "current" address pointer. - * - * REVISIT some multi-address chips don't rollover page reads to - * the next slave address, so we may need to truncate the count. - * Those chips might need another quirk flag. - * - * If the real hardware used four adjacent 24c02 chips and that - * were misconfigured as one 24c08, that would be a similar effect: - * one "eeprom" file not four, but larger reads would fail when - * they crossed certain pages. */ static struct at24_client *at24_translate_offset(struct at24_data *at24, unsigned int *offset) @@ -277,6 +268,28 @@ static struct at24_client *at24_translate_offset(struct at24_data *at24, return &at24->client[i]; } +static size_t at24_adjust_read_count(struct at24_data *at24, + unsigned int offset, size_t count) +{ + unsigned int bits; + size_t remainder; + /* + * In case of multi-address chips that don't rollover reads to + * the next slave address: truncate the count to the slave boundary, + * so that the read never straddles slaves. + */ + if (at24->chip.flags & AT24_FLAG_NO_RDROL) { + bits = (at24->chip.flags & AT24_FLAG_ADDR16) ? 16 : 8; + remainder = BIT(bits) - offset; + if (count > remainder) + count = remainder; + } + if (count > io_limit) + count = io_limit; + + return count; +} + static ssize_t at24_regmap_read(struct at24_data *at24, char *buf, unsigned int offset, size_t count) { @@ -289,9 +302,7 @@ static ssize_t at24_regmap_read(struct at24_data *at24, char *buf, at24_client = at24_translate_offset(at24, &offset); regmap = at24_client->regmap; client = at24_client->client; - - if (count > io_limit) - count = io_limit; + count = at24_adjust_read_count(at24, offset, count); /* adjust offset for mac and serial read ops */ offset += at24->offset_adj; @@ -457,6 +468,8 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip) if (device_property_present(dev, "read-only")) chip->flags |= AT24_FLAG_READONLY; + if (device_property_present(dev, "no-read-rollover")) + chip->flags |= AT24_FLAG_NO_RDROL; err = device_property_read_u32(dev, "size", &val); if (!err) diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h index 271a4e2..841bb28 100644 --- a/include/linux/platform_data/at24.h +++ b/include/linux/platform_data/at24.h @@ -50,6 +50,8 @@ 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 */ +#define AT24_FLAG_NO_RDROL BIT(1) /* does not auto-rollover reads to */ + /* the next slave address */ void (*setup)(struct nvmem_device *nvmem, void *context); void *context; -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <1512401778-20366-2-git-send-email-svendev-fuHqz3Nb1YI@public.gmane.org>]
* Re: [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads. [not found] ` <1512401778-20366-2-git-send-email-svendev-fuHqz3Nb1YI@public.gmane.org> @ 2017-12-04 21:40 ` Sakari Ailus [not found] ` <20171204214023.ml2ujacezsop5ilb-sGAanXTfQ4777SC2UrCW1FMQynFLKtET@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Sakari Ailus @ 2017-12-04 21:40 UTC (permalink / raw) To: Sven Van Asbroeck Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, wsa-z923LK4zBo2bacvFa/9K2g, brgl-ARrdPY/1zhM, nsekhar-l0cyMroinI0, david-nq/r/kbU++upp/zk7JDF2g, javier-0uQlZySMnqxg9hUCZPvPmw, divagar.mohandass-ral2JQCrhuEAvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Sven Van Asbroeck Hi Sven, On Mon, Dec 04, 2017 at 10:36:18AM -0500, Sven Van Asbroeck wrote: > From: Sven Van Asbroeck <svenv-fuHqz3Nb1YI@public.gmane.org> > > Some multi-address eeproms in the at24 family may not automatically > roll-over reads to the next slave address. On those eeproms, reads > that straddle slave boundaries will not work correctly. > > Solution: > Mark such eeproms with a flag that prevents reads straddling > slave boundaries. Add the AT24_FLAG_NO_RDROL flag to the eeprom > entry in the device_id table, or add 'no-read-rollover' to the > eeprom devicetree entry. > > Note that I have not personally enountered an at24 chip that > does not support read rollovers. They may or may not exist. > However, my hardware requires this functionality because of > a quirk. > > It's up to the Linux community to decide if this patch is useful/ > general enough to warrant merging. > > Signed-off-by: Sven Van Asbroeck <svendev-fuHqz3Nb1YI@public.gmane.org> > --- > .../devicetree/bindings/eeprom/eeprom.txt | 5 +++ > drivers/misc/eeprom/at24.c | 37 +++++++++++++++------- > include/linux/platform_data/at24.h | 2 ++ > 3 files changed, 32 insertions(+), 12 deletions(-) > > diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt > index 27f2bc1..a1764f8 100644 > --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt > +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt > @@ -38,6 +38,11 @@ Optional properties: > > - size: total eeprom size in bytes > > + - no-read-rollover: supported on the at24 eeprom family only. If this is truly specific to at24, then vendor prefix would be appropriate, plus it'd go to an at24 specific binding file. However if it isn't I'd just remove the above sentence. I guess the latter? Binding changes would be nicer in a separate patch, too. > + This parameterless property indicates that the multi-address > + eeprom does not automatically roll over reads to the next > + slave address. Please consult the manual of your device. > + > Example: > > eeprom@52 { > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c > index 625b001..33bca28 100644 > --- a/drivers/misc/eeprom/at24.c > +++ b/drivers/misc/eeprom/at24.c > @@ -251,15 +251,6 @@ struct at24_data { > * Slave address and byte offset derive from the offset. Always > * set the byte address; on a multi-master board, another master > * may have changed the chip's "current" address pointer. > - * > - * REVISIT some multi-address chips don't rollover page reads to > - * the next slave address, so we may need to truncate the count. > - * Those chips might need another quirk flag. > - * > - * If the real hardware used four adjacent 24c02 chips and that > - * were misconfigured as one 24c08, that would be a similar effect: > - * one "eeprom" file not four, but larger reads would fail when > - * they crossed certain pages. > */ > static struct at24_client *at24_translate_offset(struct at24_data *at24, > unsigned int *offset) > @@ -277,6 +268,28 @@ static struct at24_client *at24_translate_offset(struct at24_data *at24, > return &at24->client[i]; > } > > +static size_t at24_adjust_read_count(struct at24_data *at24, > + unsigned int offset, size_t count) > +{ > + unsigned int bits; > + size_t remainder; > + /* > + * In case of multi-address chips that don't rollover reads to > + * the next slave address: truncate the count to the slave boundary, > + * so that the read never straddles slaves. > + */ > + if (at24->chip.flags & AT24_FLAG_NO_RDROL) { > + bits = (at24->chip.flags & AT24_FLAG_ADDR16) ? 16 : 8; > + remainder = BIT(bits) - offset; > + if (count > remainder) > + count = remainder; > + } > + if (count > io_limit) > + count = io_limit; > + > + return count; > +} > + > static ssize_t at24_regmap_read(struct at24_data *at24, char *buf, > unsigned int offset, size_t count) > { > @@ -289,9 +302,7 @@ static ssize_t at24_regmap_read(struct at24_data *at24, char *buf, > at24_client = at24_translate_offset(at24, &offset); > regmap = at24_client->regmap; > client = at24_client->client; > - > - if (count > io_limit) > - count = io_limit; > + count = at24_adjust_read_count(at24, offset, count); > > /* adjust offset for mac and serial read ops */ > offset += at24->offset_adj; > @@ -457,6 +468,8 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip) > > if (device_property_present(dev, "read-only")) > chip->flags |= AT24_FLAG_READONLY; > + if (device_property_present(dev, "no-read-rollover")) > + chip->flags |= AT24_FLAG_NO_RDROL; > > err = device_property_read_u32(dev, "size", &val); > if (!err) > diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h > index 271a4e2..841bb28 100644 > --- a/include/linux/platform_data/at24.h > +++ b/include/linux/platform_data/at24.h > @@ -50,6 +50,8 @@ 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 */ > +#define AT24_FLAG_NO_RDROL BIT(1) /* does not auto-rollover reads to */ > + /* the next slave address */ > > void (*setup)(struct nvmem_device *nvmem, void *context); > void *context; > -- > 1.9.1 > -- Sakari Ailus sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20171204214023.ml2ujacezsop5ilb-sGAanXTfQ4777SC2UrCW1FMQynFLKtET@public.gmane.org>]
* Re: [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads. [not found] ` <20171204214023.ml2ujacezsop5ilb-sGAanXTfQ4777SC2UrCW1FMQynFLKtET@public.gmane.org> @ 2017-12-04 22:24 ` Sven Van Asbroeck 2017-12-05 7:44 ` Sakari Ailus 0 siblings, 1 reply; 7+ messages in thread From: Sven Van Asbroeck @ 2017-12-04 22:24 UTC (permalink / raw) To: Sakari Ailus Cc: Sven Van Asbroeck, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, wsa-z923LK4zBo2bacvFa/9K2g, Bartosz Golaszewski, nsekhar-l0cyMroinI0, david-nq/r/kbU++upp/zk7JDF2g, javier-0uQlZySMnqxg9hUCZPvPmw, divagar.mohandass-ral2JQCrhuEAvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-i2c, Sven Van Asbroeck > If this is truly specific to at24, then vendor prefix would be appropriate, > plus it'd go to an at24 specific binding file. However if it isn't I'd just > remove the above sentence. I guess the latter? Yes, no-read-rollover is truly specific to at24.c, because it applies only to i2c multi-address chips. The at25 is spi based so cannot have multiple addresses. So yes, "at24,no-read-rollover" would perhaps be a better name. Regarding an at24 specific binding file. You're saying I should create Documentation/devicetree/bindings/eeprom/at24.txt ? Should I indicate that at24.txt "inherits from" eeprom.txt? Note that at25.txt does not currently do this. -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads. 2017-12-04 22:24 ` Sven Van Asbroeck @ 2017-12-05 7:44 ` Sakari Ailus 2017-12-05 8:14 ` Bartosz Golaszewski 0 siblings, 1 reply; 7+ messages in thread From: Sakari Ailus @ 2017-12-05 7:44 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Sven Van Asbroeck, robh+dt, mark.rutland, wsa, Bartosz Golaszewski, nsekhar, david, javier, divagar.mohandass, devicetree, linux-kernel, linux-i2c, Sven Van Asbroeck On Mon, Dec 04, 2017 at 05:24:33PM -0500, Sven Van Asbroeck wrote: > > If this is truly specific to at24, then vendor prefix would be appropriate, > > plus it'd go to an at24 specific binding file. However if it isn't I'd just > > remove the above sentence. I guess the latter? > > Yes, no-read-rollover is truly specific to at24.c, because it applies only > to i2c multi-address chips. The at25 is spi based so cannot have multiple > addresses. > > So yes, "at24,no-read-rollover" would perhaps be a better name. > > Regarding an at24 specific binding file. You're saying I should create > Documentation/devicetree/bindings/eeprom/at24.txt ? Should I indicate > that at24.txt "inherits from" eeprom.txt? Note that at25.txt does not > currently do this. Hmm. I actually missed we didn't have one to begin with. at25.txt exists and it documents a number of properties specific to at25, so if at24 will have an at24-specific property, then I think it should go to a separate file. Aren't there really other chips which need this? It'd be (a little bit) easier to just remove the sentence. :-) -- Regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads. 2017-12-05 7:44 ` Sakari Ailus @ 2017-12-05 8:14 ` Bartosz Golaszewski 2017-12-06 21:29 ` Rob Herring 0 siblings, 1 reply; 7+ messages in thread From: Bartosz Golaszewski @ 2017-12-05 8:14 UTC (permalink / raw) To: Sakari Ailus Cc: Sven Van Asbroeck, Sven Van Asbroeck, Rob Herring, Mark Rutland, Wolfram Sang, nsekhar, David Lechner, javier, divagar.mohandass, devicetree, linux-kernel, linux-i2c, Sven Van Asbroeck 2017-12-05 8:44 GMT+01:00 Sakari Ailus <sakari.ailus@linux.intel.com>: > On Mon, Dec 04, 2017 at 05:24:33PM -0500, Sven Van Asbroeck wrote: >> > If this is truly specific to at24, then vendor prefix would be appropriate, >> > plus it'd go to an at24 specific binding file. However if it isn't I'd just >> > remove the above sentence. I guess the latter? >> >> Yes, no-read-rollover is truly specific to at24.c, because it applies only >> to i2c multi-address chips. The at25 is spi based so cannot have multiple >> addresses. >> >> So yes, "at24,no-read-rollover" would perhaps be a better name. >> >> Regarding an at24 specific binding file. You're saying I should create >> Documentation/devicetree/bindings/eeprom/at24.txt ? Should I indicate >> that at24.txt "inherits from" eeprom.txt? Note that at25.txt does not >> currently do this. > > Hmm. I actually missed we didn't have one to begin with. at25.txt exists > and it documents a number of properties specific to at25, so if at24 will > have an at24-specific property, then I think it should go to a separate > file. The eeprom.txt file in the bindings directory actually describes the bindings for at24. There's a patch[1] from Wolfram waiting for Rob's ack that renames it to at24.txt. I hope that clears any confusion. @Sven: please split the patch into two: one for bindings and one for code. As for the name: I would change it to at24,no-read-rollover and remove the fragment saying it's only supported in at24 - as I said: this file only concerns at24 and will be renamed. > > Aren't there really other chips which need this? It'd be (a little bit) > easier to just remove the sentence. :-) > > -- > Regards, > > Sakari Ailus > sakari.ailus@linux.intel.com Thanks, Bartosz [1] http://patchwork.ozlabs.org/patch/842500/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads. 2017-12-05 8:14 ` Bartosz Golaszewski @ 2017-12-06 21:29 ` Rob Herring 0 siblings, 0 replies; 7+ messages in thread From: Rob Herring @ 2017-12-06 21:29 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Sakari Ailus, Sven Van Asbroeck, Sven Van Asbroeck, Mark Rutland, Wolfram Sang, nsekhar, David Lechner, javier, divagar.mohandass, devicetree, linux-kernel, linux-i2c, Sven Van Asbroeck On Tue, Dec 05, 2017 at 09:14:22AM +0100, Bartosz Golaszewski wrote: > 2017-12-05 8:44 GMT+01:00 Sakari Ailus <sakari.ailus@linux.intel.com>: > > On Mon, Dec 04, 2017 at 05:24:33PM -0500, Sven Van Asbroeck wrote: > >> > If this is truly specific to at24, then vendor prefix would be appropriate, > >> > plus it'd go to an at24 specific binding file. However if it isn't I'd just > >> > remove the above sentence. I guess the latter? > >> > >> Yes, no-read-rollover is truly specific to at24.c, because it applies only > >> to i2c multi-address chips. The at25 is spi based so cannot have multiple > >> addresses. > >> > >> So yes, "at24,no-read-rollover" would perhaps be a better name. > >> > >> Regarding an at24 specific binding file. You're saying I should create > >> Documentation/devicetree/bindings/eeprom/at24.txt ? Should I indicate > >> that at24.txt "inherits from" eeprom.txt? Note that at25.txt does not > >> currently do this. > > > > Hmm. I actually missed we didn't have one to begin with. at25.txt exists > > and it documents a number of properties specific to at25, so if at24 will > > have an at24-specific property, then I think it should go to a separate > > file. > > The eeprom.txt file in the bindings directory actually describes the > bindings for at24. There's a patch[1] from Wolfram waiting for Rob's > ack that renames it to at24.txt. I hope that clears any confusion. It's going to wait forever until it is sent to the DT list so patchwork picks it up and is in my queue. > @Sven: please split the patch into two: one for bindings and one for code. > > As for the name: I would change it to at24,no-read-rollover and remove at24 is not a vendor. > the fragment saying it's only supported in at24 - as I said: this file > only concerns at24 and will be renamed. > > > > > Aren't there really other chips which need this? It'd be (a little bit) > > easier to just remove the sentence. :-) > > > > -- > > Regards, > > > > Sakari Ailus > > sakari.ailus@linux.intel.com > > Thanks, > Bartosz > > [1] http://patchwork.ozlabs.org/patch/842500/ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-12-06 21:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-04 15:36 [PATCH v3 0/1] at24: support eeproms that do not auto-rollover reads Sven Van Asbroeck 2017-12-04 15:36 ` [PATCH v3 1/1] " Sven Van Asbroeck [not found] ` <1512401778-20366-2-git-send-email-svendev-fuHqz3Nb1YI@public.gmane.org> 2017-12-04 21:40 ` Sakari Ailus [not found] ` <20171204214023.ml2ujacezsop5ilb-sGAanXTfQ4777SC2UrCW1FMQynFLKtET@public.gmane.org> 2017-12-04 22:24 ` Sven Van Asbroeck 2017-12-05 7:44 ` Sakari Ailus 2017-12-05 8:14 ` Bartosz Golaszewski 2017-12-06 21:29 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).