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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <5487BE86.30900@broadcom.com>
Date:	Tue, 9 Dec 2014 19:31:18 -0800
From:	Ray Jui <rjui@...adcom.com>
To:	Varka Bhadram <varkabhadram@...il.com>
CC:	Wolfram Sang <wsa@...-dreams.de>, Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	"Ian Campbell" <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Grant Likely <grant.likely@...aro.org>,
	Christian Daudt <bcm@...thebug.org>,
	Matt Porter <mporter@...aro.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	Russell King <linux@....linux.org.uk>,
	"Scott Branden" <sbranden@...adcom.com>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	<bcm-kernel-feedback-list@...adcom.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 2/4] i2c: iproc: Add Broadcom iProc I2C Driver



On 12/9/2014 7:28 PM, Varka Bhadram wrote:
>
>
> On Wed, Dec 10, 2014 at 8:51 AM, Varka Bhadram <varkabhadram@...il.com
> <mailto:varkabhadram@...il.com>> wrote:
>
>     On Wed, Dec 10, 2014 at 7:11 AM, Ray Jui <rjui@...adcom.com
>     <mailto:rjui@...adcom.com>> 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@...adcom.com
>                 <mailto:rjui@...adcom.com>>
>                 Reviewed-by: Scott Branden <sbranden@...adcom.com
>                 <mailto:sbranden@...adcom.com>>
>                 ---
>                    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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/