On 08/25/2013 02:15 AM, Laurent Pinchart wrote:
On Saturday 24 August 2013 16:13:11 Tomasz Figa wrote:On Saturday 24 of August 2013 02:54:07 Laurent Pinchart wrote:On Saturday 24 August 2013 02:41:59 Tomasz Figa wrote:On Tuesday 20 of August 2013 01:04:54 Laurent Pinchart wrote:Add DT bindings for the pcf857x-compatible chips and parse the device tree node in the driver. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> --- .../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 +++++++++++++ drivers/gpio/gpio-pcf857x.c | 57 ++++++++++--- 2 files changed, 119 insertions(+), 9 deletions(-) create mode 100644[snip]diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c index 070e81f..50a90f1 100644 --- a/drivers/gpio/gpio-pcf857x.c +++ b/drivers/gpio/gpio-pcf857x.c[snip]@@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = { }; MODULE_DEVICE_TABLE(i2c, pcf857x_id); +#ifdef CONFIG_OF +static const struct of_device_id pcf857x_of_table[] = { + { .compatible = "nxp,pcf8574", .data = (void *)8 }, + { .compatible = "nxp,pcf8574a", .data = (void *)8 }, + { .compatible = "nxp,pca8574", .data = (void *)8 }, + { .compatible = "nxp,pca9670", .data = (void *)8 }, + { .compatible = "nxp,pca9672", .data = (void *)8 }, + { .compatible = "nxp,pca9674", .data = (void *)8 }, + { .compatible = "nxp,pcf8575", .data = (void *)16 }, + { .compatible = "nxp,pca8575", .data = (void *)16 }, + { .compatible = "nxp,pca9671", .data = (void *)16 }, + { .compatible = "nxp,pca9673", .data = (void *)16 }, + { .compatible = "nxp,pca9675", .data = (void *)16 }, + { .compatible = "maxim,max7328", .data = (void *)8 }, + { .compatible = "maxim,max7329", .data = (void *)8 }, + { .compatible = "ti,tca9554", .data = (void *)8 }, + { } +}; +MODULE_DEVICE_TABLE(of, pcf857x_of_table); +#endif + /* * The pcf857x, pca857x, and pca967x chips only expose one read and * one write register. Writing a "one" bit (to match the reset @@ -257,14 +280,29 @@ fail: static int pcf857x_probe(struct i2c_client *client, const struct i2c_device_id *id) { - struct pcf857x_platform_data *pdata; + struct pcf857x_platform_data *pdata = client->dev.platform_data; + struct device_node *np = client->dev.of_node;> > > struct pcf857x *gpio;+ unsigned int n_latch = 0; + unsigned int ngpio; int status; - pdata = client->dev.platform_data; - if (!pdata) { +#ifdef CONFIG_OF + if (np) {Wouldn't if (IS_ENABLED(CONFIG_OF)&& np) be sufficient here, without the #ifdef? You would have to move the match table out of the #ifdef in this case, though...That's the exact reason why I've used #ifdef CONFIG_OF here, I didn't want to add the overhead of the pcf857x_of_table when CONFIG_OF isn't defined.I'm not sure if I remember correctly, but I think there was something said in one of discussions some time ago, that we should be moving away from ifdef'ing such things, in favour of just having them compiled unconditionally.There seems to be a general consensus to favor if (IS_ENABLED(CONFIG_OF)) instead of #ifdef CONFIG_OF when possible. I'm not sure what the opinion is regarding using conditional compilation to avoid compiling unnecessary data tables in. I would vote for using it (there's no need to bloat the kernel unnecessarily on non-OF platforms), but I'll conform to whatever is decided to be best.[Adding DT maintainers on Cc for more opinions.]I'll resubmit the patch with the DT bindings documentation fixed, and will submit yet another version if I need to remove the #ifdef.
I think it makes sense to keep this table compiled in conditionally, size of
struct of_device_id is relatively large. While absolute increase in sizemight not be that significant the relative increase is quite large - appr. 130%.
Before $subject patch: $ size drivers/gpio/gpio-pcf857x.o text data bss dec hex filename 2228 140 0 2368 940 drivers/gpio/gpio-pcf857x.o After applying the patch: $ size drivers/gpio/gpio-pcf857x.o text data bss dec hex filename 5284 140 0 5424 1530 drivers/gpio/gpio-pcf857x.o -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
