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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux