* [PATCH 1/2] misc: at24: parse OF-data, too @ 2010-11-15 17:25 Wolfram Sang 2010-11-15 17:25 ` [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts Wolfram Sang 2010-11-15 21:41 ` [PATCH 1/2] misc: at24: parse OF-data, too Grant Likely 0 siblings, 2 replies; 12+ messages in thread From: Wolfram Sang @ 2010-11-15 17:25 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev Information about the pagesize and read-only-status may also come from the devicetree. Parse this data, too, and act accordingly. While we are here, change the initialization printout a bit. write_max is useful to know to detect performance bottlenecks, the rest is superfluous. Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> --- Grant: As mentioned at ELCE10, I could pretty much respin this old approach I tried roughly a year ago (just with archdata then). If the approach and docs are good, I am fine with the patches entering via one of your trees. Documentation/powerpc/dts-bindings/eeprom.txt | 28 ++++++++++++++++++++++ drivers/misc/eeprom/at24.c | 33 ++++++++++++++++++++----- 2 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 Documentation/powerpc/dts-bindings/eeprom.txt diff --git a/Documentation/powerpc/dts-bindings/eeprom.txt b/Documentation/powerpc/dts-bindings/eeprom.txt new file mode 100644 index 0000000..4342c10 --- /dev/null +++ b/Documentation/powerpc/dts-bindings/eeprom.txt @@ -0,0 +1,28 @@ +EEPROMs (I2C) + +Required properties: + + - compatible : should be "<manufacturer>,<type>" + 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 + + - reg : the I2C address of the EEPROM + +Optional properties: + + - pagesize : the length of the pagesize for writing. Please consult the + manual of your device, that value varies a lot. A wrong value + may result in data loss! If not specified, a safety value of + '1' is used which will be very slow. + + - read-only: this parameterless property disables writes to the eeprom + +Example: + +eeprom@52 { + compatible = "atmel,24c32"; + reg = <0x52>; + pagesize = <32>; +}; diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 559b0b3..aaf16cb 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -20,6 +20,7 @@ #include <linux/log2.h> #include <linux/bitops.h> #include <linux/jiffies.h> +#include <linux/of.h> #include <linux/i2c.h> #include <linux/i2c/at24.h> @@ -457,6 +458,27 @@ static ssize_t at24_macc_write(struct memory_accessor *macc, const char *buf, /*-------------------------------------------------------------------------*/ +#ifdef CONFIG_OF +static void at24_get_ofdata(struct i2c_client *client, + struct at24_platform_data *chip) +{ + const u32 *val; + struct device_node *node = client->dev.of_node; + + if (node) { + if (of_get_property(node, "read-only", NULL)) + chip->flags |= AT24_FLAG_READONLY; + val = of_get_property(node, "pagesize", NULL); + if (val) + chip->page_size = *val; + } +} +#else +static void at24_get_ofdata(struct i2c_client *client, + struct at24_platform_data *chip) +{ } +#endif /* CONFIG_OF */ + static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct at24_platform_data chip; @@ -485,6 +507,9 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) */ chip.page_size = 1; + /* update chipdata if OF is present */ + at24_get_ofdata(client, &chip); + chip.setup = NULL; chip.context = NULL; } @@ -597,19 +622,15 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) i2c_set_clientdata(client, at24); - dev_info(&client->dev, "%zu byte %s EEPROM %s\n", + dev_info(&client->dev, "%zu byte %s EEPROM, %s, %u bytes/write\n", at24->bin.size, client->name, - writable ? "(writable)" : "(read-only)"); + writable ? "writable" : "read-only", at24->write_max); if (use_smbus == I2C_SMBUS_WORD_DATA || use_smbus == I2C_SMBUS_BYTE_DATA) { dev_notice(&client->dev, "Falling back to %s reads, " "performance will suffer\n", use_smbus == I2C_SMBUS_WORD_DATA ? "word" : "byte"); } - dev_dbg(&client->dev, - "page_size %d, num_addresses %d, write_max %d, use_smbus %d\n", - chip.page_size, num_addresses, - at24->write_max, use_smbus); /* export data to kernel code */ if (chip.setup) -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 17:25 [PATCH 1/2] misc: at24: parse OF-data, too Wolfram Sang @ 2010-11-15 17:25 ` Wolfram Sang 2010-11-15 17:32 ` Anton Vorontsov 2010-11-15 21:41 ` [PATCH 1/2] misc: at24: parse OF-data, too Grant Likely 1 sibling, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2010-11-15 17:25 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> --- arch/powerpc/boot/dts/pcm030.dts | 1 + arch/powerpc/boot/dts/pcm032.dts | 3 ++- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts index 8a4ec30..e7c36bc 100644 --- a/arch/powerpc/boot/dts/pcm030.dts +++ b/arch/powerpc/boot/dts/pcm030.dts @@ -259,6 +259,7 @@ eeprom@52 { compatible = "catalyst,24c32"; reg = <0x52>; + pagesize = <32>; }; }; diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts index 85d857a..e175e2c 100644 --- a/arch/powerpc/boot/dts/pcm032.dts +++ b/arch/powerpc/boot/dts/pcm032.dts @@ -257,8 +257,9 @@ reg = <0x51>; }; eeprom@52 { - compatible = "at24,24c32"; + compatible = "catalyst,24c32"; reg = <0x52>; + pagesize = <32>; }; }; -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 17:25 ` [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts Wolfram Sang @ 2010-11-15 17:32 ` Anton Vorontsov 2010-11-15 21:06 ` Mitch Bradley 0 siblings, 1 reply; 12+ messages in thread From: Anton Vorontsov @ 2010-11-15 17:32 UTC (permalink / raw) To: Wolfram Sang; +Cc: linuxppc-dev, devicetree-discuss On Mon, Nov 15, 2010 at 06:25:16PM +0100, Wolfram Sang wrote: > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > --- > arch/powerpc/boot/dts/pcm030.dts | 1 + > arch/powerpc/boot/dts/pcm032.dts | 3 ++- > 2 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts > index 8a4ec30..e7c36bc 100644 > --- a/arch/powerpc/boot/dts/pcm030.dts > +++ b/arch/powerpc/boot/dts/pcm030.dts > @@ -259,6 +259,7 @@ > eeprom@52 { > compatible = "catalyst,24c32"; > reg = <0x52>; > + pagesize = <32>; I think you'd better drop the pagesize property altogether, and instead make the compatible string more specific (if needed at all. are there any 'catalyst,24c32' chips with pagesize != 32?) Thanks, -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 17:32 ` Anton Vorontsov @ 2010-11-15 21:06 ` Mitch Bradley 2010-11-15 21:24 ` Anton Vorontsov 0 siblings, 1 reply; 12+ messages in thread From: Mitch Bradley @ 2010-11-15 21:06 UTC (permalink / raw) To: Anton Vorontsov; +Cc: linuxppc-dev, devicetree-discuss On 11/15/2010 7:32 AM, Anton Vorontsov wrote: > On Mon, Nov 15, 2010 at 06:25:16PM +0100, Wolfram Sang wrote: >> Signed-off-by: Wolfram Sang<w.sang@pengutronix.de> >> --- >> arch/powerpc/boot/dts/pcm030.dts | 1 + >> arch/powerpc/boot/dts/pcm032.dts | 3 ++- >> 2 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts >> index 8a4ec30..e7c36bc 100644 >> --- a/arch/powerpc/boot/dts/pcm030.dts >> +++ b/arch/powerpc/boot/dts/pcm030.dts >> @@ -259,6 +259,7 @@ >> eeprom@52 { >> compatible = "catalyst,24c32"; >> reg =<0x52>; >> + pagesize =<32>; > > I think you'd better drop the pagesize property altogether, and > instead make the compatible string more specific (if needed at > all. are there any 'catalyst,24c32' chips with pagesize != 32?) Microchip makes a 24c32 part that looks pretty similar to the catalyst part, but Microchip's has a 64-byte page size compared to Catalyst's 32. It would probably be feasible to have a generic I2C EEPROM driver that could handle many different parts, parameterized by total size, block size, and page size. > > Thanks, > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 21:06 ` Mitch Bradley @ 2010-11-15 21:24 ` Anton Vorontsov 2010-11-15 21:58 ` Grant Likely 2010-11-15 22:11 ` Wolfram Sang 0 siblings, 2 replies; 12+ messages in thread From: Anton Vorontsov @ 2010-11-15 21:24 UTC (permalink / raw) To: Mitch Bradley; +Cc: linuxppc-dev, devicetree-discuss On Mon, Nov 15, 2010 at 11:06:44AM -1000, Mitch Bradley wrote: [...] > >> eeprom@52 { > >> compatible = "catalyst,24c32"; > >> reg =<0x52>; > >>+ pagesize =<32>; > > > >I think you'd better drop the pagesize property altogether, and > >instead make the compatible string more specific (if needed at > >all. are there any 'catalyst,24c32' chips with pagesize != 32?) > > Microchip makes a 24c32 part that looks pretty similar to the > catalyst part, but Microchip's has a 64-byte page size compared to > Catalyst's 32. Well, when using microchip part, the compatible string would be "microchip,24c32", correct? Then we have all the information already, no need for the pagesize. > It would probably be feasible to have a generic I2C EEPROM driver > that could handle many different parts, parameterized by total size, > block size, and page size. I guess it can do this already via I2C ID table. The problem is that I2C core is only using part ID (no vendor ID matching). So, the current driver may just implement quirks like this: if (of_device_is_compatible(np, "catalyst,24c32")) pagesize = 32; Or, if it's some generic pattern, something like if (of_device_is_compatible_vendor(np, "catalyst")) pagesize /= 2; Thanks, -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 21:24 ` Anton Vorontsov @ 2010-11-15 21:58 ` Grant Likely 2010-11-15 22:17 ` Mitch Bradley 2010-11-15 22:11 ` Wolfram Sang 1 sibling, 1 reply; 12+ messages in thread From: Grant Likely @ 2010-11-15 21:58 UTC (permalink / raw) To: Anton Vorontsov; +Cc: Mitch Bradley, linuxppc-dev, devicetree-discuss On Mon, Nov 15, 2010 at 2:24 PM, Anton Vorontsov <cbouatmailru@gmail.com> w= rote: > On Mon, Nov 15, 2010 at 11:06:44AM -1000, Mitch Bradley wrote: > [...] >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0eeprom@52 { >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D= "catalyst,24c32"; >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D<0x52>; >> >>+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pagesize =3D<32>= ; >> > >> >I think you'd better drop the pagesize property altogether, and >> >instead make the compatible string more specific (if needed at >> >all. are there any 'catalyst,24c32' chips with pagesize !=3D 32?) >> >> Microchip makes a 24c32 part that looks pretty similar to the >> catalyst part, but Microchip's has a 64-byte page size compared to >> Catalyst's 32. > > Well, when using microchip part, the compatible string would be > "microchip,24c32", correct? Then we have all the information > already, no need for the pagesize. > >> It would probably be feasible to have a generic I2C EEPROM driver >> that could handle many different parts, parameterized by total size, >> block size, and page size. The current at24.c driver is already parameterized; but part of the parameter data is statically linked into the board support code. > > I guess it can do this already via I2C ID table. The problem > is that I2C core is only using part ID (no vendor ID matching). This could potentially be changed for at24 devices since the i2c subsystem already matches by name. It would mean adding the vendor prefix to all instantiations of the device in the kernel, although it would mess up the current heuristic used by of_modalias_node() (not a show-stopper). > > So, the current driver may just implement quirks like this: > > if (of_device_is_compatible(np, "catalyst,24c32")) > =A0 =A0 =A0 =A0pagesize =3D 32; > > Or, if it's some generic pattern, something like > > if (of_device_is_compatible_vendor(np, "catalyst")) > =A0 =A0 =A0 =A0pagesize /=3D 2; This would get ugly in a hurry. It would be better to make it data driven by searching for a better match in an of_device_id table so that the workarounds don't grow over time and eventually achieve sentience. g. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 21:58 ` Grant Likely @ 2010-11-15 22:17 ` Mitch Bradley 2010-11-15 22:30 ` David Gibson 0 siblings, 1 reply; 12+ messages in thread From: Mitch Bradley @ 2010-11-15 22:17 UTC (permalink / raw) To: Grant Likely; +Cc: linuxppc-dev, devicetree-discuss In general I think it's better to report parameter values directly, instead of inferring them from manufacturer and part numbers. That way you at least have a fighting chance of avoiding a kernel upgrade when a part changes. Of course, that only works when the device tree is exported from the boot firmware instead of having to carry the device tree inside the kernel. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 22:17 ` Mitch Bradley @ 2010-11-15 22:30 ` David Gibson 0 siblings, 0 replies; 12+ messages in thread From: David Gibson @ 2010-11-15 22:30 UTC (permalink / raw) To: Mitch Bradley; +Cc: linuxppc-dev, devicetree-discuss On Mon, Nov 15, 2010 at 12:17:51PM -1000, Mitch Bradley wrote: > In general I think it's better to report parameter values directly, > instead of inferring them from manufacturer and part numbers. That > way you at least have a fighting chance of avoiding a kernel upgrade > when a part changes. I tend to agree. Although a fallback to deducing from the manufacturer / part id if the pagesize property is missing would be sensible. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 21:24 ` Anton Vorontsov 2010-11-15 21:58 ` Grant Likely @ 2010-11-15 22:11 ` Wolfram Sang 2010-11-16 21:45 ` Wolfram Sang 1 sibling, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2010-11-15 22:11 UTC (permalink / raw) To: Anton Vorontsov; +Cc: Mitch Bradley, linuxppc-dev, devicetree-discuss [-- Attachment #1: Type: text/plain, Size: 1352 bytes --] > > >I think you'd better drop the pagesize property altogether, and > > >instead make the compatible string more specific (if needed at > > >all. are there any 'catalyst,24c32' chips with pagesize != 32?) > > > > Microchip makes a 24c32 part that looks pretty similar to the > > catalyst part, but Microchip's has a 64-byte page size compared to > > Catalyst's 32. > > Well, when using microchip part, the compatible string would be > "microchip,24c32", correct? Then we have all the information > already, no need for the pagesize. Hmm, there are myriads of I2C eeproms out there, this table would be enourmous. Even worse, I seem to recall that I had once seen a manufacturer increasing the page-size from one charge to the next without changing the part-number, so I got this feeling "you can't map pagesize to manufacturer/type" which I still have. Sadly, this was long ago, so I can't proof it right now. Will try to dig up some datasheets when in the office tomorrow. In general, I2C EEPROMs are really a mess, the basic access method is the same, but except that everything else is possible :) Thus, this approach. Thus, this approach. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-15 22:11 ` Wolfram Sang @ 2010-11-16 21:45 ` Wolfram Sang 2010-11-16 22:03 ` Anton Vorontsov 0 siblings, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2010-11-16 21:45 UTC (permalink / raw) To: Anton Vorontsov; +Cc: linuxppc-dev, devicetree-discuss [-- Attachment #1: Type: text/plain, Size: 613 bytes --] > Even worse, I seem to recall that I had once seen a manufacturer increasing the > page-size from one charge to the next without changing the part-number, so I > got this feeling "you can't map pagesize to manufacturer/type" which I still > have. Sadly, this was long ago, so I can't proof it right now. Will try to dig > up some datasheets when in the office tomorrow. Had a look, couldn't find anything :( And now? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts 2010-11-16 21:45 ` Wolfram Sang @ 2010-11-16 22:03 ` Anton Vorontsov 0 siblings, 0 replies; 12+ messages in thread From: Anton Vorontsov @ 2010-11-16 22:03 UTC (permalink / raw) To: Wolfram Sang; +Cc: linuxppc-dev, devicetree-discuss On Tue, Nov 16, 2010 at 10:45:37PM +0100, Wolfram Sang wrote: > > > Even worse, I seem to recall that I had once seen a manufacturer increasing the > > page-size from one charge to the next without changing the part-number, so I > > got this feeling "you can't map pagesize to manufacturer/type" which I still > > have. Sadly, this was long ago, so I can't proof it right now. Will try to dig > > up some datasheets when in the office tomorrow. > > Had a look, couldn't find anything :( And now? Well, it seems that there are enough people in "pagesize" camp anyway, I'd say just go ahead with the current approach. :-) It's an additional information, so won't do any harm anyway... Thanks, -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] misc: at24: parse OF-data, too 2010-11-15 17:25 [PATCH 1/2] misc: at24: parse OF-data, too Wolfram Sang 2010-11-15 17:25 ` [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts Wolfram Sang @ 2010-11-15 21:41 ` Grant Likely 1 sibling, 0 replies; 12+ messages in thread From: Grant Likely @ 2010-11-15 21:41 UTC (permalink / raw) To: Wolfram Sang; +Cc: linuxppc-dev, devicetree-discuss On Mon, Nov 15, 2010 at 10:25 AM, Wolfram Sang <w.sang@pengutronix.de> wrot= e: > Information about the pagesize and read-only-status may also come from > the devicetree. Parse this data, too, and act accordingly. While we are > here, change the initialization printout a bit. write_max is useful to > know to detect performance bottlenecks, the rest is superfluous. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > --- > > Grant: As mentioned at ELCE10, I could pretty much respin this old approa= ch I > tried roughly a year ago (just with archdata then). If the approach and d= ocs > are good, I am fine with the patches entering via one of your trees. > > =A0Documentation/powerpc/dts-bindings/eeprom.txt | =A0 28 +++++++++++++++= +++++++ > =A0drivers/misc/eeprom/at24.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| = =A0 33 ++++++++++++++++++++----- > =A02 files changed, 53 insertions(+), 6 deletions(-) > =A0create mode 100644 Documentation/powerpc/dts-bindings/eeprom.txt > > diff --git a/Documentation/powerpc/dts-bindings/eeprom.txt b/Documentatio= n/powerpc/dts-bindings/eeprom.txt > new file mode 100644 > index 0000000..4342c10 > --- /dev/null > +++ b/Documentation/powerpc/dts-bindings/eeprom.txt > @@ -0,0 +1,28 @@ > +EEPROMs (I2C) > + > +Required properties: > + > + =A0- compatible : should be "<manufacturer>,<type>" > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0If there is no specific driver for <manu= facturer>, a generic > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0driver based on <type> is selected. Poss= ible types are: > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A024c00, 24c01, 24c02, 24c04, 24c08, 24c16= , 24c32, 24c64, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A024c128, 24c256, 24c512, 24c1024, spd > + > + =A0- reg : the I2C address of the EEPROM > + > +Optional properties: > + > + =A0- pagesize : the length of the pagesize for writing. Please consult = the > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 manual of your device, that value varies a = lot. A wrong value > + =A0 =A0 =A0 =A0 =A0 =A0 =A0may result in data loss! If not specified, a= safety value of > + =A0 =A0 =A0 =A0 =A0 =A0 =A0'1' is used which will be very slow. As Anton mentions, the compatible value should be specific enough that a pagesize property isn't needed (assuming there aren't at24 devices with differing page sizes), but that leaves the question of where to put the page size data in the driver. I think the obvious answer is to put it in the at24_ids table. This would also be a good thing for the non-OF use-cases too. Unfortunately the driver_data field is already in use and requires refactoring to turn driver_data into a pointer to a structure, and so requires more work (sorry). A refactored at24_ids table might look something like (there may be a cleaner way to go about it though, I used the macro to solve the problem of constructing an at24_platform_data structure for each entry and storing a pointer to it in the unsigned long driver_data field): #define AT24_DEVDATA(_len, _page_size, _flags) \ ((unsigned long)((struct at24_platform_data[]) \ {{.byte_len =3D len, .page_size =3D _page_size, .flags =3D _flags}})) static const struct i2c_device_id at24_ids[] =3D { /* needs 8 addresses as A0-A2 are ignored */ { "24c00", AT24_DEVDATA(128 / 8, 16, AT24_FLAG_TAKE8ADDR) }, /* old variants can't be handled with this generic entry! */ { "24c01", AT24_DEVDATA(1024 / 8, 16, 0) }, { "24c02", AT24_DEVDATA(2048 / 8, 16, 0) }, ... }; This will also make the probe code simpler. However, if I am wrong and similarly named at24 devices have different page sizes, then encoding it in a property is the right thing to do. Also, the read-only property is fine. g. > + > + =A0- read-only: this parameterless property disables writes to the eepr= om > + > +Example: > + > +eeprom@52 { > + =A0 =A0 =A0 compatible =3D "atmel,24c32"; > + =A0 =A0 =A0 reg =3D <0x52>; > + =A0 =A0 =A0 pagesize =3D <32>; > +}; > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c > index 559b0b3..aaf16cb 100644 > --- a/drivers/misc/eeprom/at24.c > +++ b/drivers/misc/eeprom/at24.c > @@ -20,6 +20,7 @@ > =A0#include <linux/log2.h> > =A0#include <linux/bitops.h> > =A0#include <linux/jiffies.h> > +#include <linux/of.h> > =A0#include <linux/i2c.h> > =A0#include <linux/i2c/at24.h> > > @@ -457,6 +458,27 @@ static ssize_t at24_macc_write(struct memory_accesso= r *macc, const char *buf, > > =A0/*--------------------------------------------------------------------= -----*/ > > +#ifdef CONFIG_OF > +static void at24_get_ofdata(struct i2c_client *client, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct at24_platform_data *chip) > +{ > + =A0 =A0 =A0 const u32 *val; > + =A0 =A0 =A0 struct device_node *node =3D client->dev.of_node; > + > + =A0 =A0 =A0 if (node) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (of_get_property(node, "read-only", NULL= )) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->flags |=3D AT24_FLAG_= READONLY; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val =3D of_get_property(node, "pagesize", N= ULL); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (val) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->page_size =3D *val; > + =A0 =A0 =A0 } > +} > +#else > +static void at24_get_ofdata(struct i2c_client *client, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct at24_platform_data *chip) > +{ } > +#endif /* CONFIG_OF */ > + > =A0static int at24_probe(struct i2c_client *client, const struct i2c_devi= ce_id *id) > =A0{ > =A0 =A0 =A0 =A0struct at24_platform_data chip; > @@ -485,6 +507,9 @@ static int at24_probe(struct i2c_client *client, cons= t struct i2c_device_id *id) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0chip.page_size =3D 1; > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* update chipdata if OF is present */ > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 at24_get_ofdata(client, &chip); > + > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0chip.setup =3D NULL; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0chip.context =3D NULL; > =A0 =A0 =A0 =A0} > @@ -597,19 +622,15 @@ static int at24_probe(struct i2c_client *client, co= nst struct i2c_device_id *id) > > =A0 =A0 =A0 =A0i2c_set_clientdata(client, at24); > > - =A0 =A0 =A0 dev_info(&client->dev, "%zu byte %s EEPROM %s\n", > + =A0 =A0 =A0 dev_info(&client->dev, "%zu byte %s EEPROM, %s, %u bytes/wr= ite\n", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0at24->bin.size, client->name, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 writable ? "(writable)" : "(read-only)"); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 writable ? "writable" : "read-only", at24->= write_max); > =A0 =A0 =A0 =A0if (use_smbus =3D=3D I2C_SMBUS_WORD_DATA || > =A0 =A0 =A0 =A0 =A0 =A0use_smbus =3D=3D I2C_SMBUS_BYTE_DATA) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_notice(&client->dev, "Falling back to = %s reads, " > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "performance will suf= fer\n", use_smbus =3D=3D > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 I2C_SMBUS_WORD_DATA ?= "word" : "byte"); > =A0 =A0 =A0 =A0} > - =A0 =A0 =A0 dev_dbg(&client->dev, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 "page_size %d, num_addresses %d, write_max = %d, use_smbus %d\n", > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip.page_size, num_addresses, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 at24->write_max, use_smbus); > > =A0 =A0 =A0 =A0/* export data to kernel code */ > =A0 =A0 =A0 =A0if (chip.setup) > -- > 1.7.2.3 > > --=20 Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-11-16 22:03 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-15 17:25 [PATCH 1/2] misc: at24: parse OF-data, too Wolfram Sang 2010-11-15 17:25 ` [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts Wolfram Sang 2010-11-15 17:32 ` Anton Vorontsov 2010-11-15 21:06 ` Mitch Bradley 2010-11-15 21:24 ` Anton Vorontsov 2010-11-15 21:58 ` Grant Likely 2010-11-15 22:17 ` Mitch Bradley 2010-11-15 22:30 ` David Gibson 2010-11-15 22:11 ` Wolfram Sang 2010-11-16 21:45 ` Wolfram Sang 2010-11-16 22:03 ` Anton Vorontsov 2010-11-15 21:41 ` [PATCH 1/2] misc: at24: parse OF-data, too Grant Likely
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).