eeprom: at24: Add OF device ID table
Subsystems (2)
PATCH v6 1/1 eeprom: at24: Add OF device ID table
The driver doesn't have a struct of_device_id table but supported devices are registered via Device Trees. This is working on the assumption that a I2C device registered via OF will always match a legacy I2C device ID and that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have an OF device ID table if the devices are registered via OF.
To maintain backward compatibility with old Device Trees, only use the OF device ID table .data if the device was registered via OF and the OF node compatible matches an entry in the OF device ID table.
---
Wolfram,
This version shouldn't cause a regression if is applied before the DT changes, that's why I'm not posting as a part of a series like in the previous versions.
It also maintains backward compatibility, since as pointed out by Geert, old DTB should be supported. The driver will only attempt to use the OF entry's .data if the device was registered via OF *and* was matched using the OF device ID table.
Best regards,
Javier
Changes in v6:
- Check if the OF node matches an OF device ID table entry or if it was matched against the I2C device ID table to know from where to get the data. This will maintain backward copatibility for old DTBs (Geert Uytterhoeven).
Changes in v5:
- None
Changes in v4:
- None
Changes in v3:
- Fix wrong .data values for "atmel,24c02" and "atmel,24c64" entries.
Changes in v2:
- Only add a single OF device ID entry for each device type (Wolfram Sang).
drivers/misc/eeprom/at24.c | 71 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-)
drivers/misc/eeprom/at24.c +70 -1
@@ -12,6 +12,7 @@#include <linux/kernel.h> #include <linux/init.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/slab.h> #include <linux/delay.h> #include <linux/mutex.h>
@@ -175,6 +176,64 @@ static const struct i2c_device_id at24_ids[] = { };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 ) + }, + { }, +}; +MODULE_DEVICE_TABLE (of, at24_of_match); +static const struct acpi_device_id at24_acpi_ids[] = { {"INT3499" ,AT24_DEVICE_MAGIC (8192 /8 ,0 ) }, { }
@@ -598,7 +657,16 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)if (client->dev.platform_data) { chip = *(struct at24_platform_data *)client->dev.platform_data; }else { -if (id) { +/* +* The I2C core allows OF nodes compatibles to match against the +* I2C device ID table as a fallback, so check not only if an OF +* node is present but also if it matches an OF device ID entry. +*/ +if (client->dev.of_node && +of_match_device (at24_of_match, &client->dev)) { + magic = (kernel_ulong_t ) +of_device_get_match_data (&client->dev); + }else if (id) { magic = id->driver_data; }else {const struct acpi_device_id *aid;
@@ -814,6 +882,7 @@ static int at24_remove(struct i2c_client *client)static struct i2c_driver at24_driver = { .driver = { .name ="at24" , + .of_match_table = at24_of_match, .acpi_match_table =ACPI_PTR (at24_acpi_ids), }, .probe = at24_probe,
Signature · 1 line
2.14.1
Hi,
drivers/misc/eeprom/at24.c static const struct i2c_device_id at24_ids[] = { · line 196
Quoted 5 lines
drivers/misc/eeprom/at24.c +5
-0+ { + .compatible = "atmel,spd", + .data = (void *)AT24_DEVICE_MAGIC (2048 /8 , +AT24_FLAG_READONLY |AT24_FLAG_IRUGO ) + },
checkpatch reported this one as un-documented. And come to think of it, since this is solely for EEPROMs on RAM modules, I think we can drop a DT binding for it. Could you agree? I can do it locally, no need to resend. I'll do HW testing later, but wanted to check on your opinion already.
Thanks,
Wolfram
Quoted 8 lines · Wolfram Sang
Hi,
drivers/misc/eeprom/at24.c static const struct i2c_device_id at24_ids[] = { · line 196
Quoted 5 lines
drivers/misc/eeprom/at24.c +5
-0+ { + .compatible = "atmel,spd", + .data = (void *)AT24_DEVICE_MAGIC (2048 /8 , +AT24_FLAG_READONLY |AT24_FLAG_IRUGO ) + },
Quoted 5 lines
checkpatch reported this one as un-documented. And come to think of it, since this is solely for EEPROMs on RAM modules, I think we can drop a DT binding for it. Could you agree? I can do it locally, no need to resend. I'll do HW testing later, but wanted to check on your opinion already.
It is used by arch/powerpc/boot/dts/mpc8349emitx.dts.
Rob
Quoted 9 lines · Rob Herring
Quoted 8 lines · Wolfram Sang
Hi,
Quoted 5 lines
drivers/misc/eeprom/at24.c +5
-0+ { + .compatible = "atmel,spd", + .data = (void *)AT24_DEVICE_MAGIC (2048 /8 , +AT24_FLAG_READONLY |AT24_FLAG_IRUGO ) + },
Quoted 7 lines
Quoted 5 lines
checkpatch reported this one as un-documented. And come to think of it, since this is solely for EEPROMs on RAM modules, I think we can drop a DT binding for it. Could you agree? I can do it locally, no need to resend. I'll do HW testing later, but wanted to check on your opinion already.
It is used by
arch/powerpc/boot/dts/mpc8349emitx.dts.
Thanks!
Hi Wolfram,
Quoted 11 lines · Wolfram Sang
drivers/misc/eeprom/at24.c static const struct i2c_device_id at24_ids[] = { · line 196
Quoted 5 lines
drivers/misc/eeprom/at24.c +5
-0+ { + .compatible = "atmel,spd", + .data = (void *)AT24_DEVICE_MAGIC (2048 /8 , +AT24_FLAG_READONLY |AT24_FLAG_IRUGO ) + },checkpatch reported this one as un-documented. And come to think of it, since this is solely for EEPROMs on RAM modules, I think we can drop a DT binding for it. Could you agree? I can do it locally, no need to resend. I'll do HW testing later, but wanted to check on your opinion already.
Why?
No one has placeholders for memory modules in DT?
In addition, I can imagine a DT overlay for a memory module, adding the memory node and the EEPROM.
Gr{oetje,eeting}s,
Geert
Quoted 11 lines · Wolfram Sang
Hi,
drivers/misc/eeprom/at24.c static const struct i2c_device_id at24_ids[] = { · line 196
Quoted 5 lines
drivers/misc/eeprom/at24.c +5
-0+ { + .compatible = "atmel,spd", + .data = (void *)AT24_DEVICE_MAGIC (2048 /8 , +AT24_FLAG_READONLY |AT24_FLAG_IRUGO ) + },checkpatch reported this one as un-documented. And come to think of it, since this is solely for EEPROMs on RAM modules, I think we can drop a DT binding for it. Could you agree? I can do it locally, no need to
As mentioned by Rob, it's already used by a DTS in mainline.
I think the problem is in how the DT binding for this driver is described.
Most DT bindings describes the complete list of compatible strings supported by a driver and that's what checkpatch expects. But Documentation/devicetree/bindings/eeprom/eeprom.txt describes it as the cartesian product of the atmel (and also the deprecated at and at24) manufacturer and a list of devices. To save you a lookup:
If there is no specific driver for <manufacturer>, a generic device with <type> and manufacturer "atmel" should be used. Possible types are:
"24c00", "24c01", "24c02", "24c04", "24c08", "24c16", "24c32", "24c64", "24c128", "24c256", "24c512", "24c1024", "spd"
Quoted 3 lines
resend. I'll do HW testing later, but wanted to check on your opinion already.
Great, thanks! I hope I got it right this time. Adding a DTS snippet I can see that the entry .data is correctly used but I don't a device to check if is working correctly after $SUBJECT.
Quoted 4 lines
Thanks,
Wolfram
Wolfram Sang
AppliedQuoted 8 lines · Javier Martinez Canillas
The driver doesn't have a struct
of_device_idtable but supported devices are registered via Device Trees. This is working on the assumption that a I2C device registered via OF will always match a legacy I2C device ID and that the MODALIAS reported will always be of the form i2c:<device>.But this could change in the future so the correct approach is to have an OF device ID table if the devices are registered via OF.
Quoted 7 lines
To maintain backward compatibility with old Device Trees, only use the OF device ID table .data if the device was registered via OF and the OF node compatible matches an entry in the OF device ID table.
Suggested-by Wolfram Sang <[email protected]>Signed-off-by Javier Martinez Canillas <[email protected]>
Applied to for-next, thanks for keeping at it!
Hello Wolfram,
Quoted 13 lines · Wolfram Sang
Quoted 12 lines · Javier Martinez Canillas
The driver doesn't have a struct
of_device_idtable but supported devices are registered via Device Trees. This is working on the assumption that a I2C device registered via OF will always match a legacy I2C device ID and that the MODALIAS reported will always be of the form i2c:<device>.But this could change in the future so the correct approach is to have an OF device ID table if the devices are registered via OF.
To maintain backward compatibility with old Device Trees, only use the OF device ID table .data if the device was registered via OF and the OF node compatible matches an entry in the OF device ID table.
Quoted 6 lines
Quoted 3 lines
Suggested-by Wolfram Sang <[email protected]>Signed-off-by Javier Martinez Canillas <[email protected]>Applied to for-next, thanks for keeping at it!
Great, thanks a lot for your feedback and suggestions on this series.
Best regards,
Signature · 3 lines
Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat