From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Subject: Re: [PATCH v4 5/7] ARM: davinci: i2c: add OF support Date: Sat, 26 May 2012 00:08:54 -0600 Message-ID: <20120526060854.A9BB93E2E10@localhost> References: <1337694920-8925-1-git-send-email-hs@denx.de> <1337694920-8925-6-git-send-email-hs@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1337694920-8925-6-git-send-email-hs-ynQEQJNshbs@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces+gld-davinci-linux-open-source=gmane.org-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org To: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org Cc: Wolfgang Denk , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Wolfram Sang , linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ben Dooks , Sylwester Nawrocki , Heiko Schocher , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: linux-i2c@vger.kernel.org On Tue, 22 May 2012 15:55:18 +0200, Heiko Schocher wrote: > add of support for the davinci i2c driver. > > Signed-off-by: Heiko Schocher > Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org > Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org > Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: Ben Dooks > Cc: Wolfram Sang > Cc: Grant Likely > Cc: Sekhar Nori > Cc: Wolfgang Denk > Cc: Sylwester Nawrocki > > --- > - changes for v2: > - add comments from Sylwester Nawrocki : > - use "cell-index" instead "id" > - OF_DEV_AUXDATA in the machine code, instead pre-define platform > device name > - add comment from Grant Likely: > - removed "id" resp. "cell-index" completely > - fixed documentation > - use of_match_ptr() > - use devm_kzalloc() for allocating plattform data mem > - fixed a whitespace issue > - no changes for v3 > - changes for v4 > remove "pinmux-handle" property as discussed here: > http://www.spinics.net/lists/arm-kernel/msg175701.html > with Nori Sekhar > --- > .../devicetree/bindings/arm/davinci/i2c.txt | 31 +++++++++++++++++++ > drivers/i2c/busses/i2c-davinci.c | 32 ++++++++++++++++++++ > 2 files changed, 63 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/arm/davinci/i2c.txt > > diff --git a/Documentation/devicetree/bindings/arm/davinci/i2c.txt b/Documentation/devicetree/bindings/arm/davinci/i2c.txt > new file mode 100644 > index 0000000..e98a025 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/davinci/i2c.txt > @@ -0,0 +1,31 @@ > +* Texas Instruments Davinci I2C > + > +This file provides information, what the device node for the > +davinci i2c interface contain. > + > +Required properties: > +- compatible: "ti,davinci-i2c"; > +- reg : Offset and length of the register set for the device > + > +Recommended properties : > +- interrupts : standard interrupt property. > +- clock-frequency : desired I2C bus clock frequency in Hz. > + > +Optional properties: > +- bus-delay: bus delay in usec > + > +Example (enbw_cmc board): > + i2c@1c22000 { > + compatible = "ti,davinci-i2c"; > + reg = <0x22000 0x1000>; > + clock-frequency = <100000>; > + interrupts = <15>; > + interrupt-parent = <&intc>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + dtt@48 { > + compatible = "national,lm75"; > + reg = <0x48>; > + }; > + }; > diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c > index a76d85f..c1783bb 100644 > --- a/drivers/i2c/busses/i2c-davinci.c > +++ b/drivers/i2c/busses/i2c-davinci.c > @@ -38,9 +38,12 @@ > #include > #include > #include > +#include > +#include > > #include > #include > +#include > > /* ----- global defines ----------------------------------------------- */ > > @@ -635,6 +638,12 @@ static struct i2c_algorithm i2c_davinci_algo = { > .functionality = i2c_davinci_func, > }; > > +static const struct of_device_id davinci_i2c_of_match[] = { > + {.compatible = "ti,davinci-i2c", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, davinci_i2c_of_match); > + > static int davinci_i2c_probe(struct platform_device *pdev) > { > struct davinci_i2c_dev *dev; > @@ -676,6 +685,26 @@ static int davinci_i2c_probe(struct platform_device *pdev) > dev->irq = irq->start; > platform_set_drvdata(pdev, dev); > > + if ((dev->dev->platform_data == NULL) && > + (pdev->dev.of_node)) { > + struct davinci_i2c_platform_data *pdata; > + u32 prop; > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) { > + r = -ENOMEM; > + goto err_free_mem; > + } > + memcpy(pdata, &davinci_i2c_platform_data_default, > + sizeof(*pdata)); > + dev->dev->platform_data = pdata; This is actually illegal. Drivers must not change the value of dev->dev->platform_data. It messes up unbinding/rebinding drivers. You'll need to have a copy of the pointer in davinci_i2c_dev. Other than that this patch looks good. g. > + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > + &prop)) > + pdata->bus_freq = prop / 1000; > + if (!of_property_read_u32(pdev->dev.of_node, "bus-delay", > + &prop)) > + pdata->bus_delay = prop; > + } > dev->clk = clk_get(&pdev->dev, NULL); > if (IS_ERR(dev->clk)) { > r = -ENODEV; > @@ -711,6 +740,7 @@ static int davinci_i2c_probe(struct platform_device *pdev) > adap->algo = &i2c_davinci_algo; > adap->dev.parent = &pdev->dev; > adap->timeout = DAVINCI_I2C_TIMEOUT; > + adap->dev.of_node = pdev->dev.of_node; > > adap->nr = pdev->id; > r = i2c_add_numbered_adapter(adap); > @@ -718,6 +748,7 @@ static int davinci_i2c_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "failure adding adapter\n"); > goto err_free_irq; > } > + of_i2c_register_devices(adap); > > return 0; > > @@ -809,6 +840,7 @@ static struct platform_driver davinci_i2c_driver = { > .name = "i2c_davinci", > .owner = THIS_MODULE, > .pm = davinci_i2c_pm_ops, > + .of_match_table = of_match_ptr(davinci_i2c_of_match), > }, > }; > > -- > 1.7.7.6 > -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd.