Re: [PATCH v2 3/3] gpio: pcf857x: Add OF support
From: Sylwester Nawrocki
Date: Sun Aug 25 2013 - 04:05:02 EST
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 size
might 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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/