Looking at this again ...

On 3 September 2012 12:07, Linus Walleij [off-list ref] wrote:

On Mon, Sep 3, 2012 at 12:07 PM, Lee Jones [off-list ref] wrote:

(...)

quoted

+       if (np) {
+               if (!pdata) {
+                       pdata = devm_kzalloc(&adev->dev, sizeof(*pdata),

GFP_KERNEL);

quoted

+                       if (!pdata) {
+                               ret = -ENOMEM;
+                               goto err_no_mem;
+                       }
+               }
+               /* Provide the default configuration as a base. */
+               memcpy(pdata, &u8500_i2c, sizeof(struct

nmk_i2c_controller));

Here you blank out any pdata passed from say a board file or
whatever if pdata != NULL.

Right, this should be in 'if (!pdata) {}' . I'll correct this now.

quoted

+
+               nmk_i2c_of_probe(np, pdata);
+       }
+
        if (!pdata)
                /* No i2c configuration found, using the default. */
                pdata = &u8500_i2c;

So this is still wrong, if pdata is passed to the driver it will
not override the DT, you have the semantics the other way
around, DT overrides pdata.

This is correct, however. If there is DT, it should override any platform
data.

Actually, if there is DT then no platform data should be passed in the
first place.

Look at the switch statement in my previous comment,
just add the allocations and a memcpy() and it still holds:

if (!pdata) {
    if (np) {
              pdata = devm_kzalloc(&adev->dev, sizeof(*pdata), GFP_KERNEL);
                      if (!pdata) {
                              ret = -ENOMEM;
                              goto err_no_mem;
                      }
              }
              /* Provide the default configuration as a base. */
              memcpy(pdata, &u8500_i2c, sizeof(struct nmk_i2c_controller));
              nmk_i2c_of_probe(np, pdata);
    } else
         /* Just use the static pdata */
         pdata = &u8500_i2c;
}

No, this is wrong. Platform data should not override DT.

If DT is enabled and passed, it should have highest priority.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120903/8487d887/attachment-0001.html>