On 08:37 Tue 13 Mar , Wolfram Sang wrote: > On Thu, Mar 08, 2012 at 09:50:31AM +0100, Jean-Christophe PLAGNIOL-VILLARD > wrote: > > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[email protected]> > > Cc: Nicolas Ferre <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Your patch description says "add DT support" but you are doing a bit more. > This should at least be described here. > > > --- > > v3: > > > > update i2c binding (Rob comments) > > > > Jean can I have a ack to apply it? > > > > Best Regards, > > J. > > .../devicetree/bindings/gpio/gpio_i2c.txt | 32 +++++++ > > drivers/i2c/busses/i2c-gpio.c | 94 > > +++++++++++++++---- > > 2 files changed, 106 insertions(+), 20 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt > > > > diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt > > b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt > > new file mode 100644 > > index 0000000..4f8ec94 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt > > @@ -0,0 +1,32 @@ > > +Device-Tree bindings for i2c gpio driver > > + > > +Required properties: > > + - compatible = "i2c-gpio"; > > I assume people at "devicetree-discuss" have taken care of that, yet I wonder > that this breaks the "vendor,product" syntax I know of for compatible entries. yes but there no vendor here get ack from Grant and Rob > > > + - gpios: sda and scl gpio > > + > > + > > +Optional properties: > > + - i2c-gpio,sda-open-drain: sda as open drain > > + - i2c-gpio,scl-open-drain: scl as open drain > > + - i2c-gpio,scl-output-only: scl as output only > > + - i2c-gpio,delay-us: delay between GPIO operations (may depend on each > > platform) > > + - i2c-gpio,timeout-ms: timeout to get data > > + > > +Example nodes: > > + > > +i2c@0 { > > + compatible = "i2c-gpio"; > > + gpios = <&pioA 23 0 /* sda */ > > + &pioA 24 0 /* scl */ > > + >; > > + i2c-gpio,sda-open-drain; > > + i2c-gpio,scl-open-drain; > > + i2c-gpio,delay-us = <2>; /* ~100 kHz */ > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + rv3029c2@56 { > > + compatible = "rv3029c2"; > > + reg = <0x56>; > > + }; > > +}; > > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c > > index a651779..98ae85b 100644 > > --- a/drivers/i2c/busses/i2c-gpio.c > > +++ b/drivers/i2c/busses/i2c-gpio.c > > @@ -14,8 +14,15 @@ > > #include <linux/module.h> > > #include <linux/slab.h> > > #include <linux/platform_device.h> > > +#include <linux/gpio.h> > > +#include <linux/of_gpio.h> > > +#include <linux/of_i2c.h> > > > > -#include <asm/gpio.h> > > +struct i2c_gpio_private_data { > > + struct i2c_adapter adap; > > + struct i2c_algo_bit_data bit_data; > > + struct i2c_gpio_platform_data pdata; > > +}; > > > > /* Toggle SDA by changing the direction of the pin */ > > static void i2c_gpio_setsda_dir(void *data, int state) > > @@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data) > > return gpio_get_value(pdata->scl_pin); > > } > > > > +static int __devinit of_i2c_gpio_probe(struct device_node *np, > > + struct i2c_gpio_platform_data *pdata) > > +{ > > + u32 reg; > > + > > + if (of_gpio_count(np) < 2) > > + return -ENODEV; > > + > > + pdata->sda_pin = of_get_gpio(np, 0); > > + pdata->scl_pin = of_get_gpio(np, 1); > > + > > + if (pdata->sda_pin < 0 || pdata->scl_pin < 0) { > > gpio_is_valid()? > > > + pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n", > > + np->full_name, pdata->sda_pin, pdata->scl_pin); > > + return -ENODEV; > > + } > > + > > + of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay); > > + > > + if (of_property_read_u32(np, "i2c-gpio,timeout-ms", ®)) > > + pdata->timeout = msecs_to_jiffies(reg); > > + > > + pdata->sda_is_open_drain = > > + !!of_property_read_bool(np, "i2c-gpio,sda-open-drain"); > > + pdata->scl_is_open_drain = > > + !!of_property_read_bool(np, "i2c-gpio,scl-open-drain"); > > + pdata->scl_is_output_only = > > + !!of_property_read_bool(np, "i2c-gpio,scl-output-only"); > > + > > + return 0; > > +} > > + > > static int __devinit i2c_gpio_probe(struct platform_device *pdev) > > { > > + struct i2c_gpio_private_data *priv; > > struct i2c_gpio_platform_data *pdata; > > struct i2c_algo_bit_data *bit_data; > > struct i2c_adapter *adap; > > int ret; > > > > - pdata = pdev->dev.platform_data; > > - if (!pdata) > > - return -ENXIO; > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + adap = &priv->adap; > > + bit_data = &priv->bit_data; > > + pdata = &priv->pdata; > > > > - ret = -ENOMEM; > > - adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL); > > - if (!adap) > > - goto err_alloc_adap; > > - bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL); > > - if (!bit_data) > > - goto err_alloc_bit_data; > > + if (pdev->dev.of_node) { > > + ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata); > > + if (ret) > > + return ret; > > + } else { > > + if (!pdev->dev.platform_data) > > + return -ENXIO; > > + memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata)); > > + } > > > > ret = gpio_request(pdata->sda_pin, "sda"); > > if (ret) > > @@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct > > platform_device *pdev) > > adap->algo_data = bit_data; > > adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD; > > adap->dev.parent = &pdev->dev; > > + adap->dev.of_node = pdev->dev.of_node; > > > > /* > > * If "dev->id" is negative we consider it as zero. > > @@ -154,7 +200,9 @@ static int __devinit i2c_gpio_probe(struct > > platform_device *pdev) > > if (ret) > > goto err_add_bus; > > > > - platform_set_drvdata(pdev, adap); > > + of_i2c_register_devices(adap); > > + > > + platform_set_drvdata(pdev, priv); > > > > dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n", > > pdata->sda_pin, pdata->scl_pin, > > @@ -168,34 +216,40 @@ err_add_bus: > > err_request_scl: > > gpio_free(pdata->sda_pin); > > err_request_sda: > > - kfree(bit_data); > > -err_alloc_bit_data: > > - kfree(adap); > > -err_alloc_adap: > > return ret; > > } > > > > static int __devexit i2c_gpio_remove(struct platform_device *pdev) > > { > > + struct i2c_gpio_private_data *priv; > > struct i2c_gpio_platform_data *pdata; > > struct i2c_adapter *adap; > > > > - adap = platform_get_drvdata(pdev); > > - pdata = pdev->dev.platform_data; > > + priv = platform_get_drvdata(pdev); > > + adap = &priv->adap; > > + pdata = &priv->pdata; > > > > i2c_del_adapter(adap); > > gpio_free(pdata->scl_pin); > > gpio_free(pdata->sda_pin); > > - kfree(adap->algo_data); > > - kfree(adap); > > > > return 0; > > } > > > > +#if defined(CONFIG_OF) > > +static const struct of_device_id i2c_gpio_dt_ids[] = { > > + { .compatible = "i2c-gpio", }, > > + { /* sentinel */ } > > +}; > > + > > +MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids); > > +#endif > > + > > static struct platform_driver i2c_gpio_driver = { > > .driver = { > > .name = "i2c-gpio", > > .owner = THIS_MODULE, > > + .of_match_table = of_match_ptr(i2c_gpio_dt_ids), > > }, > > .probe = i2c_gpio_probe, > > .remove = __devexit_p(i2c_gpio_remove), > > Otherwise looks good in general. What was your test scenario? AT91 support on 3 different SoC
will update and send it via AT91 with your ack Best Regards, J. > > Thanks, > > Wolfram > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
