Re: [PATCH 2/4] i2c: iproc: Add Broadcom iProc I2C Driver

From: Ray Jui
Date: Tue Dec 09 2014 - 22:31:33 EST




On 12/9/2014 7:28 PM, Varka Bhadram wrote:


On Wed, Dec 10, 2014 at 8:51 AM, Varka Bhadram <varkabhadram@xxxxxxxxx
<mailto:varkabhadram@xxxxxxxxx>> wrote:

On Wed, Dec 10, 2014 at 7:11 AM, Ray Jui <rjui@xxxxxxxxxxxx
<mailto:rjui@xxxxxxxxxxxx>> wrote:



On 12/9/2014 5:33 PM, Varka Bhadram wrote:


On Wednesday 10 December 2014 06:24 AM, Ray Jui wrote:

Add initial support to the Broadcom iProc I2C controller
found in the
iProc family of SoCs.

The iProc I2C controller has separate internal TX and RX
FIFOs, each has
a size of 64 bytes. The iProc I2C controller supports
two bus speeds
including standard mode (100kHz) and fast mode (400kHz)

Signed-off-by: Ray Jui <rjui@xxxxxxxxxxxx
<mailto:rjui@xxxxxxxxxxxx>>
Reviewed-by: Scott Branden <sbranden@xxxxxxxxxxxx
<mailto:sbranden@xxxxxxxxxxxx>>
---
drivers/i2c/busses/Kconfig | 9 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-bcm-iproc.c | 503
++++++++++++++++++++++++++++++++++++
3 files changed, 513 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-bcm-iproc.c

(...)

+static int bcm_iproc_i2c_probe(struct platform_device
*pdev)
+{
+ int irq, ret = 0;
+ struct bcm_iproc_i2c_dev *dev;
+ struct i2c_adapter *adap;
+ struct resource *res;
+
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev),
GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, dev);
+ dev->device = &pdev->dev;
+ init_completion(&dev->done);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;


We can remove this resource check. This checking will happen
with
devm_ioremap_resource()

Don't you need to obtain a valid resource and pass it into
devm_ioremap_resource? Without 'res' being assigned a valid
resource, devm_ioremap_resource will reject with "invalid resource".

platform_get_resource() will return a resource, checking on this
resource is happening at
http://lxr.free-electrons.com/source/lib/devres.c#L115. So no need
to check it explicitly.

If you check here it will be duplication of check with resource. Two
times we are checking on
the resource. No point of doing like that.

Thanks.
Sorry I misunderstood what you meant. Okay I'll get rid of if (!res) check there. Thanks.



See this: http://lxr.free-electrons.com/source/lib/devres.c#L102

--
Thanks and Regards,
Varka Bhadram.
--
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/