* [PATCH v3 0/1] eeprom: at24: convert magic numbers to structs.
@ 2017-12-18 18:30 Sven Van Asbroeck
2017-12-18 18:30 ` [PATCH v3 1/1] " Sven Van Asbroeck
0 siblings, 1 reply; 6+ messages in thread
From: Sven Van Asbroeck @ 2017-12-18 18:30 UTC (permalink / raw)
To: svendev, wsa, brgl, nsekhar, sakari.ailus, javier, divagar.mohandass
Cc: devicetree, linux-kernel, linux-i2c
v3:
rebase against maintainer's latest devel branch
removed macros from struct {i2c,of,acpi}_device_id for easy grepping
applied newline, formatting, tab suggestions
v2:
use struct at24_chip_data instead of struct at24_platform_data
(which decreases code size)
explicitly write out of compatible nodes, e.g. "atmel,24c04"
(which allows grepping for of compatible nodes)
v1:
first shot
Sven Van Asbroeck (1):
eeprom: at24: convert magic numbers to structs.
drivers/misc/eeprom/at24.c | 219 ++++++++++++++++++++-------------------------
1 file changed, 99 insertions(+), 120 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 1/1] eeprom: at24: convert magic numbers to structs. 2017-12-18 18:30 [PATCH v3 0/1] eeprom: at24: convert magic numbers to structs Sven Van Asbroeck @ 2017-12-18 18:30 ` Sven Van Asbroeck 0 siblings, 0 replies; 6+ messages in thread From: Sven Van Asbroeck @ 2017-12-18 18:30 UTC (permalink / raw) To: svendev, wsa, brgl, nsekhar, sakari.ailus, javier, divagar.mohandass Cc: devicetree, linux-kernel, linux-i2c Fundamental properties such as capacity and page size differ among at24-type chips. But these chips do not have an id register, so this can't be discovered at runtime. Traditionally, at24-type eeprom properties were determined in two ways: - by passing a 'struct at24_platform_data' via platform_data, or - by naming the chip type in the devicetree, which passes a 'magic number' to probe(), which is then converted to a 'struct at24_platform_data'. Recently a bug was discovered because the magic number rounds down all chip sizes to the lowest power of two. This was addressed by a work-around, with the wish that magic numbers should over time be converted to structs. commit 5478e478eee3 ("eeprom: at24: correctly set the size for at24mac402") This patch replaces the magic numbers with 'struct at24_chip_data'. Signed-off-by: Sven Van Asbroeck <svendev@arcx.com> --- drivers/misc/eeprom/at24.c | 219 ++++++++++++++++++++------------------------- 1 file changed, 99 insertions(+), 120 deletions(-) diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index b44a3d2b..afa0f93 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -105,16 +105,6 @@ struct at24_data { module_param_named(write_timeout, at24_write_timeout, uint, 0); MODULE_PARM_DESC(at24_write_timeout, "Time (in ms) to try writes (default 25)"); -#define AT24_SIZE_BYTELEN 5 -#define AT24_SIZE_FLAGS 8 - -#define AT24_BITMASK(x) (BIT(x) - 1) - -/* create non-zero magic value for given eeprom parameters */ -#define AT24_DEVICE_MAGIC(_len, _flags) \ - ((1 << AT24_SIZE_FLAGS | (_flags)) \ - << AT24_SIZE_BYTELEN | ilog2(_len)) - /* * Both reads and writes fail if the previous write didn't complete yet. This * macro loops a few times waiting at least long enough for one entire page @@ -132,113 +122,108 @@ struct at24_data { op_time ? time_before(op_time, tout) : true; \ usleep_range(1000, 1500), op_time = jiffies) +struct at24_chip_data { + /* + * these fields mirror their equivalents in + * struct at24_platform_data + */ + u32 byte_len; + u8 flags; +}; + +#define AT24_CHIP_DATA(_name, _len, _flags) \ + static const struct at24_chip_data _name = { \ + .byte_len = _len, .flags = _flags, \ + } + +/* needs 8 addresses as A0-A2 are ignored */ +AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR); +/* old variants can't be handled with this generic entry! */ +AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0); +AT24_CHIP_DATA(at24_data_24cs01, 16, AT24_FLAG_SERIAL | + AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0); +AT24_CHIP_DATA(at24_data_24cs02, 16, AT24_FLAG_SERIAL | + AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24mac402, 48 / 8, AT24_FLAG_MAC | + AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24mac602, 64 / 8, AT24_FLAG_MAC | + AT24_FLAG_READONLY); +/* spd is a 24c02 in memory DIMMs */ +AT24_CHIP_DATA(at24_data_spd, 2048 / 8, AT24_FLAG_READONLY | + AT24_FLAG_IRUGO); +AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0); +AT24_CHIP_DATA(at24_data_24cs04, 16, AT24_FLAG_SERIAL | + AT24_FLAG_READONLY); +/* 24rf08 quirk is handled at i2c-core */ +AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0); +AT24_CHIP_DATA(at24_data_24cs08, 16, AT24_FLAG_SERIAL | + AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0); +AT24_CHIP_DATA(at24_data_24cs16, 16, AT24_FLAG_SERIAL | + AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16); +AT24_CHIP_DATA(at24_data_24cs32, 16, AT24_FLAG_ADDR16 | + AT24_FLAG_SERIAL | AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16); +AT24_CHIP_DATA(at24_data_24cs64, 16, AT24_FLAG_ADDR16 | + AT24_FLAG_SERIAL | AT24_FLAG_READONLY); +AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16); +AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16); +AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16); +AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16); +/* identical to 24c08 ? */ +AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0); + static const struct i2c_device_id at24_ids[] = { - /* needs 8 addresses as A0-A2 are ignored */ - { "24c00", AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR) }, - /* old variants can't be handled with this generic entry! */ - { "24c01", AT24_DEVICE_MAGIC(1024 / 8, 0) }, - { "24cs01", AT24_DEVICE_MAGIC(16, - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, - { "24c02", AT24_DEVICE_MAGIC(2048 / 8, 0) }, - { "24cs02", AT24_DEVICE_MAGIC(16, - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, - { "24mac402", AT24_DEVICE_MAGIC(48 / 8, - AT24_FLAG_MAC | AT24_FLAG_READONLY) }, - { "24mac602", AT24_DEVICE_MAGIC(64 / 8, - AT24_FLAG_MAC | AT24_FLAG_READONLY) }, - /* spd is a 24c02 in memory DIMMs */ - { "spd", AT24_DEVICE_MAGIC(2048 / 8, - AT24_FLAG_READONLY | AT24_FLAG_IRUGO) }, - { "24c04", AT24_DEVICE_MAGIC(4096 / 8, 0) }, - { "24cs04", AT24_DEVICE_MAGIC(16, - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, - /* 24rf08 quirk is handled at i2c-core */ - { "24c08", AT24_DEVICE_MAGIC(8192 / 8, 0) }, - { "24cs08", AT24_DEVICE_MAGIC(16, - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, - { "24c16", AT24_DEVICE_MAGIC(16384 / 8, 0) }, - { "24cs16", AT24_DEVICE_MAGIC(16, - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, - { "24c32", AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16) }, - { "24cs32", AT24_DEVICE_MAGIC(16, - AT24_FLAG_ADDR16 | - AT24_FLAG_SERIAL | - AT24_FLAG_READONLY) }, - { "24c64", AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16) }, - { "24cs64", AT24_DEVICE_MAGIC(16, - AT24_FLAG_ADDR16 | - AT24_FLAG_SERIAL | - AT24_FLAG_READONLY) }, - { "24c128", AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16) }, - { "24c256", AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) }, - { "24c512", AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) }, - { "24c1024", AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) }, + { "24c00", (kernel_ulong_t)&at24_data_24c00 }, + { "24c01", (kernel_ulong_t)&at24_data_24c01 }, + { "24cs01", (kernel_ulong_t)&at24_data_24cs01 }, + { "24c02", (kernel_ulong_t)&at24_data_24c02 }, + { "24cs02", (kernel_ulong_t)&at24_data_24cs02 }, + { "24mac402", (kernel_ulong_t)&at24_data_24mac402 }, + { "24mac602", (kernel_ulong_t)&at24_data_24mac602 }, + { "spd", (kernel_ulong_t)&at24_data_spd }, + { "24c04", (kernel_ulong_t)&at24_data_24c04 }, + { "24cs04", (kernel_ulong_t)&at24_data_24cs04 }, + { "24c08", (kernel_ulong_t)&at24_data_24c08 }, + { "24cs08", (kernel_ulong_t)&at24_data_24cs08 }, + { "24c16", (kernel_ulong_t)&at24_data_24c16 }, + { "24cs16", (kernel_ulong_t)&at24_data_24cs16 }, + { "24c32", (kernel_ulong_t)&at24_data_24c32 }, + { "24cs32", (kernel_ulong_t)&at24_data_24cs32 }, + { "24c64", (kernel_ulong_t)&at24_data_24c64 }, + { "24cs64", (kernel_ulong_t)&at24_data_24cs64 }, + { "24c128", (kernel_ulong_t)&at24_data_24c128 }, + { "24c256", (kernel_ulong_t)&at24_data_24c256 }, + { "24c512", (kernel_ulong_t)&at24_data_24c512 }, + { "24c1024", (kernel_ulong_t)&at24_data_24c1024 }, { "at24", 0 }, { /* END OF LIST */ } }; MODULE_DEVICE_TABLE(i2c, at24_ids); static const struct of_device_id at24_of_match[] = { - { - .compatible = "atmel,24c00", - .data = (void *)AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR) - }, - { - .compatible = "atmel,24c01", - .data = (void *)AT24_DEVICE_MAGIC(1024 / 8, 0) - }, - { - .compatible = "atmel,24c02", - .data = (void *)AT24_DEVICE_MAGIC(2048 / 8, 0) - }, - { - .compatible = "atmel,spd", - .data = (void *)AT24_DEVICE_MAGIC(2048 / 8, - AT24_FLAG_READONLY | AT24_FLAG_IRUGO) - }, - { - .compatible = "atmel,24c04", - .data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0) - }, - { - .compatible = "atmel,24c08", - .data = (void *)AT24_DEVICE_MAGIC(8192 / 8, 0) - }, - { - .compatible = "atmel,24c16", - .data = (void *)AT24_DEVICE_MAGIC(16384 / 8, 0) - }, - { - .compatible = "atmel,24c32", - .data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16) - }, - { - .compatible = "atmel,24c64", - .data = (void *)AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16) - }, - { - .compatible = "atmel,24c128", - .data = (void *)AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16) - }, - { - .compatible = "atmel,24c256", - .data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) - }, - { - .compatible = "atmel,24c512", - .data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) - }, - { - .compatible = "atmel,24c1024", - .data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) - }, - { }, + { .compatible = "atmel,24c00", .data = &at24_data_24c00 }, + { .compatible = "atmel,24c01", .data = &at24_data_24c01 }, + { .compatible = "atmel,24c02", .data = &at24_data_24c02 }, + { .compatible = "atmel,spd", .data = &at24_data_spd }, + { .compatible = "atmel,24c04", .data = &at24_data_24c04 }, + { .compatible = "atmel,24c08", .data = &at24_data_24c08 }, + { .compatible = "atmel,24c16", .data = &at24_data_24c16 }, + { .compatible = "atmel,24c32", .data = &at24_data_24c32 }, + { .compatible = "atmel,24c64", .data = &at24_data_24c64 }, + { .compatible = "atmel,24c128", .data = &at24_data_24c128 }, + { .compatible = "atmel,24c256", .data = &at24_data_24c256 }, + { .compatible = "atmel,24c512", .data = &at24_data_24c512 }, + { .compatible = "atmel,24c1024", .data = &at24_data_24c1024 }, + { /* END OF LIST */ }, }; MODULE_DEVICE_TABLE(of, at24_of_match); static const struct acpi_device_id at24_acpi_ids[] = { - { "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) }, - { } + { "INT3499", (kernel_ulong_t)&at24_data_INT3499 }, + { /* END OF LIST */ } }; MODULE_DEVICE_TABLE(acpi, at24_acpi_ids); @@ -516,8 +501,8 @@ static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len) static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) { - struct at24_platform_data chip; - kernel_ulong_t magic = 0; + struct at24_platform_data chip = { 0 }; + const struct at24_chip_data *cd = NULL; bool writable; struct at24_data *at24; int err; @@ -535,28 +520,22 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) */ if (client->dev.of_node && of_match_device(at24_of_match, &client->dev)) { - magic = (kernel_ulong_t) - of_device_get_match_data(&client->dev); + cd = of_device_get_match_data(&client->dev); } else if (id) { - magic = id->driver_data; + cd = (void *)id->driver_data; } else { const struct acpi_device_id *aid; aid = acpi_match_device(at24_acpi_ids, &client->dev); if (aid) - magic = aid->driver_data; + cd = (void *)aid->driver_data; } - if (!magic) + if (!cd) return -ENODEV; - chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN)); - magic >>= AT24_SIZE_BYTELEN; - chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS); - + chip.byte_len = cd->byte_len; + chip.flags = cd->flags; at24_get_pdata(&client->dev, &chip); - - chip.setup = NULL; - chip.context = NULL; } if (!is_power_of_2(chip.byte_len)) -- 1.9.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1513621702-6565-1-git-send-email-svenv@arcx.com>]
[parent not found: <1513621702-6565-2-git-send-email-svenv@arcx.com>]
* Re: [PATCH v3 1/1] eeprom: at24: convert magic numbers to structs. [not found] ` <1513621702-6565-2-git-send-email-svenv@arcx.com> @ 2017-12-19 15:55 ` Bartosz Golaszewski 2017-12-20 16:32 ` Sven Van Asbroeck 0 siblings, 1 reply; 6+ messages in thread From: Bartosz Golaszewski @ 2017-12-19 15:55 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Sven Van Asbroeck, Wolfram Sang, nsekhar, Sakari Ailus, Javier Martinez Canillas, Divagar Mohandass, devicetree, Linux Kernel Mailing List, linux-i2c 2017-12-18 19:28 GMT+01:00 Sven Van Asbroeck <svenv@arcx.com>: > From: Sven Van Asbroeck <svendev@arcx.com> > > Fundamental properties such as capacity and page size differ > among at24-type chips. But these chips do not have an id register, > so this can't be discovered at runtime. > > Traditionally, at24-type eeprom properties were determined in two ways: > - by passing a 'struct at24_platform_data' via platform_data, or > - by naming the chip type in the devicetree, which passes a > 'magic number' to probe(), which is then converted to > a 'struct at24_platform_data'. > > Recently a bug was discovered because the magic number rounds down > all chip sizes to the lowest power of two. This was addressed by > a work-around, with the wish that magic numbers should over time > be converted to structs. > commit 5478e478eee3 ("eeprom: at24: correctly set the size for at24mac402") > > This patch replaces the magic numbers with 'struct at24_chip_data'. > > Signed-off-by: Sven Van Asbroeck <svendev@arcx.com> OK, this looks better. We still need to fix some formatting issues though. Please find some remarks below. > --- > drivers/misc/eeprom/at24.c | 219 ++++++++++++++++++++------------------------- > 1 file changed, 99 insertions(+), 120 deletions(-) > > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c > index b44a3d2b..afa0f93 100644 > --- a/drivers/misc/eeprom/at24.c > +++ b/drivers/misc/eeprom/at24.c > @@ -105,16 +105,6 @@ struct at24_data { > module_param_named(write_timeout, at24_write_timeout, uint, 0); > MODULE_PARM_DESC(at24_write_timeout, "Time (in ms) to try writes (default 25)"); > > -#define AT24_SIZE_BYTELEN 5 > -#define AT24_SIZE_FLAGS 8 > - > -#define AT24_BITMASK(x) (BIT(x) - 1) > - > -/* create non-zero magic value for given eeprom parameters */ > -#define AT24_DEVICE_MAGIC(_len, _flags) \ > - ((1 << AT24_SIZE_FLAGS | (_flags)) \ > - << AT24_SIZE_BYTELEN | ilog2(_len)) > - > /* > * Both reads and writes fail if the previous write didn't complete yet. This > * macro loops a few times waiting at least long enough for one entire page > @@ -132,113 +122,108 @@ struct at24_data { > op_time ? time_before(op_time, tout) : true; \ > usleep_range(1000, 1500), op_time = jiffies) > > +struct at24_chip_data { > + /* > + * these fields mirror their equivalents in > + * struct at24_platform_data > + */ > + u32 byte_len; > + u8 flags; > +}; > + > +#define AT24_CHIP_DATA(_name, _len, _flags) \ > + static const struct at24_chip_data _name = { \ > + .byte_len = _len, .flags = _flags, \ > + } > + > +/* needs 8 addresses as A0-A2 are ignored */ > +AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR); > +/* old variants can't be handled with this generic entry! */ > +AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0); > +AT24_CHIP_DATA(at24_data_24cs01, 16, AT24_FLAG_SERIAL | > + AT24_FLAG_READONLY); When breaking the lines like this, prefer to break after a comma and having longer lines below - it's more readable that way. In this case it would be: AT24_CHIP_DATA(at24_data_24cs01, AT24_FLAG_SERIAL | AT24_FLAG_READONLY); The same applies elsewhere as long as it fits the line length of 80 chars. > +AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0); > +AT24_CHIP_DATA(at24_data_24cs02, 16, AT24_FLAG_SERIAL | > + AT24_FLAG_READONLY); > +AT24_CHIP_DATA(at24_data_24mac402, 48 / 8, AT24_FLAG_MAC | > + AT24_FLAG_READONLY); > +AT24_CHIP_DATA(at24_data_24mac602, 64 / 8, AT24_FLAG_MAC | > + AT24_FLAG_READONLY); > +/* spd is a 24c02 in memory DIMMs */ > +AT24_CHIP_DATA(at24_data_spd, 2048 / 8, AT24_FLAG_READONLY | > + AT24_FLAG_IRUGO); > +AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0); > +AT24_CHIP_DATA(at24_data_24cs04, 16, AT24_FLAG_SERIAL | > + AT24_FLAG_READONLY); > +/* 24rf08 quirk is handled at i2c-core */ > +AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0); > +AT24_CHIP_DATA(at24_data_24cs08, 16, AT24_FLAG_SERIAL | > + AT24_FLAG_READONLY); > +AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0); > +AT24_CHIP_DATA(at24_data_24cs16, 16, AT24_FLAG_SERIAL | > + AT24_FLAG_READONLY); > +AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16); > +AT24_CHIP_DATA(at24_data_24cs32, 16, AT24_FLAG_ADDR16 | > + AT24_FLAG_SERIAL | AT24_FLAG_READONLY); > +AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16); > +AT24_CHIP_DATA(at24_data_24cs64, 16, AT24_FLAG_ADDR16 | > + AT24_FLAG_SERIAL | AT24_FLAG_READONLY); > +AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16); > +AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16); > +AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16); > +AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16); > +/* identical to 24c08 ? */ > +AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0); > + > static const struct i2c_device_id at24_ids[] = { > - /* needs 8 addresses as A0-A2 are ignored */ > - { "24c00", AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR) }, > - /* old variants can't be handled with this generic entry! */ > - { "24c01", AT24_DEVICE_MAGIC(1024 / 8, 0) }, > - { "24cs01", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, > - { "24c02", AT24_DEVICE_MAGIC(2048 / 8, 0) }, > - { "24cs02", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, > - { "24mac402", AT24_DEVICE_MAGIC(48 / 8, > - AT24_FLAG_MAC | AT24_FLAG_READONLY) }, > - { "24mac602", AT24_DEVICE_MAGIC(64 / 8, > - AT24_FLAG_MAC | AT24_FLAG_READONLY) }, > - /* spd is a 24c02 in memory DIMMs */ > - { "spd", AT24_DEVICE_MAGIC(2048 / 8, > - AT24_FLAG_READONLY | AT24_FLAG_IRUGO) }, > - { "24c04", AT24_DEVICE_MAGIC(4096 / 8, 0) }, > - { "24cs04", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, > - /* 24rf08 quirk is handled at i2c-core */ > - { "24c08", AT24_DEVICE_MAGIC(8192 / 8, 0) }, > - { "24cs08", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, > - { "24c16", AT24_DEVICE_MAGIC(16384 / 8, 0) }, > - { "24cs16", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_SERIAL | AT24_FLAG_READONLY) }, > - { "24c32", AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16) }, > - { "24cs32", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_ADDR16 | > - AT24_FLAG_SERIAL | > - AT24_FLAG_READONLY) }, > - { "24c64", AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16) }, > - { "24cs64", AT24_DEVICE_MAGIC(16, > - AT24_FLAG_ADDR16 | > - AT24_FLAG_SERIAL | > - AT24_FLAG_READONLY) }, > - { "24c128", AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16) }, > - { "24c256", AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) }, > - { "24c512", AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) }, > - { "24c1024", AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) }, > + { "24c00", (kernel_ulong_t)&at24_data_24c00 }, Please separate the two arguments with tabs and align them nicely in the entire array. > + { "24c01", (kernel_ulong_t)&at24_data_24c01 }, > + { "24cs01", (kernel_ulong_t)&at24_data_24cs01 }, > + { "24c02", (kernel_ulong_t)&at24_data_24c02 }, > + { "24cs02", (kernel_ulong_t)&at24_data_24cs02 }, > + { "24mac402", (kernel_ulong_t)&at24_data_24mac402 }, > + { "24mac602", (kernel_ulong_t)&at24_data_24mac602 }, > + { "spd", (kernel_ulong_t)&at24_data_spd }, > + { "24c04", (kernel_ulong_t)&at24_data_24c04 }, > + { "24cs04", (kernel_ulong_t)&at24_data_24cs04 }, > + { "24c08", (kernel_ulong_t)&at24_data_24c08 }, > + { "24cs08", (kernel_ulong_t)&at24_data_24cs08 }, > + { "24c16", (kernel_ulong_t)&at24_data_24c16 }, > + { "24cs16", (kernel_ulong_t)&at24_data_24cs16 }, > + { "24c32", (kernel_ulong_t)&at24_data_24c32 }, > + { "24cs32", (kernel_ulong_t)&at24_data_24cs32 }, > + { "24c64", (kernel_ulong_t)&at24_data_24c64 }, > + { "24cs64", (kernel_ulong_t)&at24_data_24cs64 }, > + { "24c128", (kernel_ulong_t)&at24_data_24c128 }, > + { "24c256", (kernel_ulong_t)&at24_data_24c256 }, > + { "24c512", (kernel_ulong_t)&at24_data_24c512 }, > + { "24c1024", (kernel_ulong_t)&at24_data_24c1024 }, > { "at24", 0 }, > { /* END OF LIST */ } > }; > MODULE_DEVICE_TABLE(i2c, at24_ids); > > static const struct of_device_id at24_of_match[] = { > - { > - .compatible = "atmel,24c00", > - .data = (void *)AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR) > - }, > - { > - .compatible = "atmel,24c01", > - .data = (void *)AT24_DEVICE_MAGIC(1024 / 8, 0) > - }, > - { > - .compatible = "atmel,24c02", > - .data = (void *)AT24_DEVICE_MAGIC(2048 / 8, 0) > - }, > - { > - .compatible = "atmel,spd", > - .data = (void *)AT24_DEVICE_MAGIC(2048 / 8, > - AT24_FLAG_READONLY | AT24_FLAG_IRUGO) > - }, > - { > - .compatible = "atmel,24c04", > - .data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0) > - }, > - { > - .compatible = "atmel,24c08", > - .data = (void *)AT24_DEVICE_MAGIC(8192 / 8, 0) > - }, > - { > - .compatible = "atmel,24c16", > - .data = (void *)AT24_DEVICE_MAGIC(16384 / 8, 0) > - }, > - { > - .compatible = "atmel,24c32", > - .data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16) > - }, > - { > - .compatible = "atmel,24c64", > - .data = (void *)AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16) > - }, > - { > - .compatible = "atmel,24c128", > - .data = (void *)AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16) > - }, > - { > - .compatible = "atmel,24c256", > - .data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) > - }, > - { > - .compatible = "atmel,24c512", > - .data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) > - }, > - { > - .compatible = "atmel,24c1024", > - .data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) > - }, > - { }, > + { .compatible = "atmel,24c00", .data = &at24_data_24c00 }, Add tabs here as well, seems like there's enough space in the line. > + { .compatible = "atmel,24c01", .data = &at24_data_24c01 }, > + { .compatible = "atmel,24c02", .data = &at24_data_24c02 }, > + { .compatible = "atmel,spd", .data = &at24_data_spd }, > + { .compatible = "atmel,24c04", .data = &at24_data_24c04 }, > + { .compatible = "atmel,24c08", .data = &at24_data_24c08 }, > + { .compatible = "atmel,24c16", .data = &at24_data_24c16 }, > + { .compatible = "atmel,24c32", .data = &at24_data_24c32 }, > + { .compatible = "atmel,24c64", .data = &at24_data_24c64 }, > + { .compatible = "atmel,24c128", .data = &at24_data_24c128 }, > + { .compatible = "atmel,24c256", .data = &at24_data_24c256 }, > + { .compatible = "atmel,24c512", .data = &at24_data_24c512 }, > + { .compatible = "atmel,24c1024", .data = &at24_data_24c1024 }, > + { /* END OF LIST */ }, > }; > MODULE_DEVICE_TABLE(of, at24_of_match); > > static const struct acpi_device_id at24_acpi_ids[] = { > - { "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) }, > - { } > + { "INT3499", (kernel_ulong_t)&at24_data_INT3499 }, Same spacing as in the i2c device list. > + { /* END OF LIST */ } > }; > MODULE_DEVICE_TABLE(acpi, at24_acpi_ids); > > @@ -516,8 +501,8 @@ static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len) > > static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) > { > - struct at24_platform_data chip; > - kernel_ulong_t magic = 0; > + struct at24_platform_data chip = { 0 }; > + const struct at24_chip_data *cd = NULL; > bool writable; > struct at24_data *at24; > int err; > @@ -535,28 +520,22 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) > */ > if (client->dev.of_node && > of_match_device(at24_of_match, &client->dev)) { > - magic = (kernel_ulong_t) > - of_device_get_match_data(&client->dev); > + cd = of_device_get_match_data(&client->dev); > } else if (id) { > - magic = id->driver_data; > + cd = (void *)id->driver_data; > } else { > const struct acpi_device_id *aid; > > aid = acpi_match_device(at24_acpi_ids, &client->dev); > if (aid) > - magic = aid->driver_data; > + cd = (void *)aid->driver_data; > } > - if (!magic) > + if (!cd) > return -ENODEV; > > - chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN)); > - magic >>= AT24_SIZE_BYTELEN; > - chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS); > - > + chip.byte_len = cd->byte_len; > + chip.flags = cd->flags; > at24_get_pdata(&client->dev, &chip); > - > - chip.setup = NULL; > - chip.context = NULL; > } > > if (!is_power_of_2(chip.byte_len)) > -- > 1.9.1 > Thanks, Bartosz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] eeprom: at24: convert magic numbers to structs. 2017-12-19 15:55 ` Bartosz Golaszewski @ 2017-12-20 16:32 ` Sven Van Asbroeck 2017-12-20 16:37 ` Bartosz Golaszewski 0 siblings, 1 reply; 6+ messages in thread From: Sven Van Asbroeck @ 2017-12-20 16:32 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Sven Van Asbroeck, Sven Van Asbroeck, Wolfram Sang, nsekhar, Sakari Ailus, Javier Martinez Canillas, Divagar Mohandass, devicetree, Linux Kernel Mailing List, linux-i2c >> + { .compatible = "atmel,24c00", .data = &at24_data_24c00 }, > > Add tabs here as well, seems like there's enough space in the line. There's not enough space in the line to line up the 24c1024 entry with the others. I'll revert to the formatting style that was there before. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] eeprom: at24: convert magic numbers to structs. 2017-12-20 16:32 ` Sven Van Asbroeck @ 2017-12-20 16:37 ` Bartosz Golaszewski 2017-12-20 16:42 ` Sven Van Asbroeck 0 siblings, 1 reply; 6+ messages in thread From: Bartosz Golaszewski @ 2017-12-20 16:37 UTC (permalink / raw) To: Sven Van Asbroeck Cc: Sven Van Asbroeck, Sven Van Asbroeck, Wolfram Sang, nsekhar, Sakari Ailus, Javier Martinez Canillas, Divagar Mohandass, devicetree, Linux Kernel Mailing List, linux-i2c 2017-12-20 17:32 GMT+01:00 Sven Van Asbroeck <thesven73@gmail.com>: >>> + { .compatible = "atmel,24c00", .data = &at24_data_24c00 }, >> >> Add tabs here as well, seems like there's enough space in the line. > > There's not enough space in the line to line up the 24c1024 entry with > the others. I'll revert to the formatting style that was there before. I'm at 77 columns if I add a single tab after '.compatible = "atmel,24c1024"'. It looks nice like this, so I prefer to keep it. Thanks, Bartosz Golaszewski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] eeprom: at24: convert magic numbers to structs. 2017-12-20 16:37 ` Bartosz Golaszewski @ 2017-12-20 16:42 ` Sven Van Asbroeck 0 siblings, 0 replies; 6+ messages in thread From: Sven Van Asbroeck @ 2017-12-20 16:42 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Sven Van Asbroeck, Sven Van Asbroeck, Wolfram Sang, nsekhar, Sakari Ailus, Javier Martinez Canillas, Divagar Mohandass, devicetree, Linux Kernel Mailing List, linux-i2c > I'm at 77 columns if I add a single tab after '.compatible = "atmel,24c1024"'. You're right, I had to knock some sense into my editor first :) > > It looks nice like this, so I prefer to keep it. Ok I'll post a v5. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-20 16:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-18 18:30 [PATCH v3 0/1] eeprom: at24: convert magic numbers to structs Sven Van Asbroeck
2017-12-18 18:30 ` [PATCH v3 1/1] " Sven Van Asbroeck
[not found] <1513621702-6565-1-git-send-email-svenv@arcx.com>
[not found] ` <1513621702-6565-2-git-send-email-svenv@arcx.com>
2017-12-19 15:55 ` Bartosz Golaszewski
2017-12-20 16:32 ` Sven Van Asbroeck
2017-12-20 16:37 ` Bartosz Golaszewski
2017-12-20 16:42 ` Sven Van Asbroeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®