[PATCH 7/7] i2c: pxa: support to parse property

[grant.likely@secretlab.ca (Grant Likely)]

On Tue, Jul 19, 2011 at 06:17:21PM +0800, Eric Miao wrote:
> On Tue, Jul 19, 2011 at 10:24 AM, Haojian Zhuang
> <haojian.zhuang@marvell.com> wrote:
> > Support to parse some optional properties. These three properties are
> > i2c-polling, i2c-frequency, i2c-class.
> >
> > After supporting these property, i2c-pxa driver can avoid to use platform
> > data except for slave mode.
> >
> > Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> > ---
> > ?.../devicetree/bindings/i2c/pxa255-i2c.txt ? ? ? ? | ? 36 ++++++++++++++++++++
> > ?drivers/i2c/busses/i2c-pxa.c ? ? ? ? ? ? ? ? ? ? ? | ? 22 ++++++++----
> > ?2 files changed, 51 insertions(+), 7 deletions(-)
> > ?create mode 100644 Documentation/devicetree/bindings/i2c/pxa255-i2c.txt
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/pxa255-i2c.txt b/Documentation/devicetree/bindings/i2c/pxa255-i2c.txt
> > new file mode 100644
> > index 0000000..bf34236
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/pxa255-i2c.txt
> > @@ -0,0 +1,36 @@
> > +PXA255 I2C
> > +
> > +The I2C-Controller is first used in PXA255. It's widely used in Intel/Marvell
> > +silicons.
> > +
> > +Optional Property:
> > + ? ? ? - i2c-polling: Specifies whether I2C-Controller is used in polling
> > + ? ? ? ? mode or interrupt mode. The type of property should be <u32>.
> > +
> > + ? ? ? - i2c-frequency: Specifies the frequency that the I2C-Controller
> > + ? ? ? ? is working. The type of property should be <string>.
> > +
> > + ? ? ? - i2c-class: Specifies the class of I2C-Controller. The type of
> > + ? ? ? ? property should be <u32>.
> > +
> > +Example:
> > + ? ? ? i2c0: i2c at d4011000 {
> > + ? ? ? ? ? ? ? compatible = "pxa2xx-i2c";
> > + ? ? ? ? ? ? ? #address-cells = <1>;
> > + ? ? ? ? ? ? ? #size-cells = <0>;
> > + ? ? ? ? ? ? ? reg = <0xd4011000 0x60>;
> > + ? ? ? ? ? ? ? /* I2C-Controller works in interrupt mode. */
> > + ? ? ? ? ? ? ? i2c-polling = <0>;
> > + ? ? ? ? ? ? ? /* I2C-Controller's frequency is FAST. */
> > + ? ? ? ? ? ? ? i2c-frequency = "fast";
> > + ? ? ? ? ? ? ? /* interrupt of I2C-Controller */
> > + ? ? ? ? ? ? ? interrupts = <7>;
> > + ? ? ? ? ? ? ? interrupt-parent = <&mmp_intc>;
> > +
> > + ? ? ? ? ? ? ? pm860x: pmic at 34 {
> > + ? ? ? ? ? ? ? ? ? ? ? interrupt-controller;
> > + ? ? ? ? ? ? ? ? ? ? ? /* interrupt of pm860x */
> > + ? ? ? ? ? ? ? ? ? ? ? interrupts = <4>;
> > + ? ? ? ? ? ? ? ? ? ? ? interrupt-parent = <&mmp_intc>;
> > + ? ? ? ? ? ? ? };
> > + ? ? ? };
> > diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> > index adac74a..4b9fa71 100644
> > --- a/drivers/i2c/busses/i2c-pxa.c
> > +++ b/drivers/i2c/busses/i2c-pxa.c
> > @@ -1060,7 +1060,8 @@ static int i2c_pxa_probe(struct platform_device *dev)
> > ? ? ? ?const struct platform_device_id *id = platform_get_device_id(dev);
> > ? ? ? ?enum pxa_i2c_types i2c_type;
> > ? ? ? ?struct resource *res;
> > - ? ? ? int irq, ret;
> > + ? ? ? int irq, ret, poll;
> > + ? ? ? char *p = NULL;
> > ? ? ? ?static int idx = 0;
> >
> > ? ? ? ?if (np) {
> > @@ -1093,11 +1094,24 @@ static int i2c_pxa_probe(struct platform_device *dev)
> >
> >
> > ? ? ? ?if (np) {
> > + ? ? ? ? ? ? ? of_property_read_u32(np, "i2c-polling", &poll);
> > + ? ? ? ? ? ? ? i2c->use_pio = (poll) ? 1 : 0;
> > + ? ? ? ? ? ? ? of_property_read_string(np, "i2c-frequency", &p);
> > + ? ? ? ? ? ? ? if (p && !strncmp(p, "fast", 4))
> > + ? ? ? ? ? ? ? ? ? ? ? i2c->fast_mode = 1;
> > + ? ? ? ? ? ? ? of_property_read_u32(np, "i2c-class", &i2c->adap.class);
> > +
> > ? ? ? ? ? ? ? ?i2c->adap.nr = idx++;
> > ? ? ? ? ? ? ? ?snprintf(i2c->adap.name, sizeof(i2c->adap.name),
> > ? ? ? ? ? ? ? ? ? ? ? ?"pxa2xx-i2c.%u", i2c->adap.nr);
> > ? ? ? ? ? ? ? ?i2c->clk = clk_get_sys(i2c->adap.name, NULL);
> > ? ? ? ?} else {
> > + ? ? ? ? ? ? ? if (plat) {
> > + ? ? ? ? ? ? ? ? ? ? ? i2c->adap.class = plat->class;
> > + ? ? ? ? ? ? ? ? ? ? ? i2c->use_pio = plat->use_pio;
> > + ? ? ? ? ? ? ? ? ? ? ? i2c->fast_mode = plat->fast_mode;
> > + ? ? ? ? ? ? ? }
> > +
> 
> One concern of DT's impact to existing drivers is _every_ driver has to be
> modified. While I'm not sure if it's a right way to go in a long run. Or can
> we have a generic way to automatically map the DT properties to the
> specific platform data structure?

Not really, since there is no such thing as a generic platform_data
structure.  We can certainly have helper functions, but any
driver-specific data must by definition have driver-specific code to
decode it from the device tree.

It does help though when similar drivers use the same binding.

g.

> 
> Grant, any idea on this?
> 
> > ? ? ? ? ? ? ? ?/*
> > ? ? ? ? ? ? ? ? * If "dev->id" is negative we consider it as zero.
> > ? ? ? ? ? ? ? ? * The reason to do so is to avoid sysfs names that only make
> > @@ -1142,12 +1156,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
> >
> > ? ? ? ?clk_enable(i2c->clk);
> >
> > - ? ? ? if (plat) {
> > - ? ? ? ? ? ? ? i2c->adap.class = plat->class;
> > - ? ? ? ? ? ? ? i2c->use_pio = plat->use_pio;
> > - ? ? ? ? ? ? ? i2c->fast_mode = plat->fast_mode;
> > - ? ? ? }
> > -
> > ? ? ? ?if (i2c->use_pio) {
> > ? ? ? ? ? ? ? ?i2c->adap.algo = &i2c_pxa_pio_algorithm;
> > ? ? ? ?} else {
> > --
> > 1.5.6.5
> >
> >