| 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
| ||
|
Message-ID: <4F7115FA.6080507@gmail.com>
Date: Mon, 26 Mar 2012 20:20:58 -0500
From: Rob Herring <robherring2@...il.com>
To: David Daney <ddaney.cavm@...il.com>
CC: linux-mips@...ux-mips.org, ralf@...ux-mips.org,
devicetree-discuss@...ts.ozlabs.org,
Grant Likely <grant.likely@...retlab.ca>,
Rob Herring <rob.herring@...xeda.com>,
David Daney <david.daney@...ium.com>,
linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
"Ben Dooks (embedded platforms)" <ben-linux@...ff.org>,
"Jean Delvare \(PC drivers, core\)" <khali@...ux-fr.org>
Subject: Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.
On 03/26/2012 07:27 PM, David Daney wrote:
> From: David Daney <david.daney@...ium.com>
>
> There are three parts to this:
>
> 1) Remove the definitions of OCTEON_IRQ_TWSI and OCTEON_IRQ_TWSI2.
> The interrupts are specified by the device tree and these hard
> coded irq numbers block the used of the irq lines by the irq_domain
> code.
>
> 2) Remove platform device setup code from octeon-platform.c, it is
> now unused.
>
> 3) Convert i2c-octeon.c to use device tree. Part of this includes
> using the devm_* functions instead of the raw counterparts, thus
> simplifying error handling. No functionality is changed.
Shouldn't this patch go after converting the platform to DT?
> Signed-off-by: David Daney <david.daney@...ium.com>
> Cc: "Jean Delvare (PC drivers, core)" <khali@...ux-fr.org>
> Cc: "Ben Dooks (embedded platforms)" <ben-linux@...ff.org>
> Cc: "Wolfram Sang (embedded platforms)" <w.sang@...gutronix.de>
> Cc: linux-i2c@...r.kernel.org
> ---
>
> Should probably go via Ralf's linux-mips.org tree.
>
> arch/mips/cavium-octeon/octeon-irq.c | 2 -
> arch/mips/cavium-octeon/octeon-platform.c | 84 -------------------------
> arch/mips/include/asm/octeon/octeon.h | 5 --
> drivers/i2c/busses/i2c-octeon.c | 94 ++++++++++++++++-------------
> 4 files changed, 52 insertions(+), 133 deletions(-)
snip
>
> - if (i2c_data == NULL) {
> - dev_err(i2c->dev, "no I2C frequency data\n");
> + /*
> + * "clock-rate" is a legacy binding, the official binding is
> + * "clock-frequency". Try the official one first and then
> + * fall back if it doesn't exist.
> + */
> + data = of_get_property(pdev->dev.of_node, "clock-frequency", &len);
> + if (!data || len != sizeof(*data))
> + data = of_get_property(pdev->dev.of_node, "clock-rate", &len);
> + if (data && len == sizeof(*data)) {
> + i2c->twsi_freq = be32_to_cpup(data);
Can't you use of_property_read_u32?
Does the legacy binding really exist as DT support is new?
Otherwise,
Acked-by: Rob Herring <rob.herring@...xeda.com>
> + } else {
> + dev_err(i2c->dev,
> + "no I2C 'clock-rate' or 'clock-frequency' property\n");
> result = -ENXIO;
> - goto fail_region;
> + goto out;
> }
>
> - i2c->twsi_phys = res_mem->start;
> - i2c->regsize = resource_size(res_mem);
> - i2c->twsi_freq = i2c_data->i2c_freq;
> - i2c->sys_freq = i2c_data->sys_freq;
> + i2c->sys_freq = octeon_get_io_clock_rate();
>
> - if (!request_mem_region(i2c->twsi_phys, i2c->regsize, res_mem->name)) {
> + if (!devm_request_mem_region(&pdev->dev, i2c->twsi_phys, i2c->regsize,
> + res_mem->name)) {
> dev_err(i2c->dev, "request_mem_region failed\n");
> - goto fail_region;
> + goto out;
> }
> - i2c->twsi_base = ioremap(i2c->twsi_phys, i2c->regsize);
> + i2c->twsi_base = devm_ioremap(&pdev->dev, i2c->twsi_phys, i2c->regsize);
>
> init_waitqueue_head(&i2c->queue);
>
> i2c->irq = irq;
>
> - result = request_irq(i2c->irq, octeon_i2c_isr, 0, DRV_NAME, i2c);
> + result = devm_request_irq(&pdev->dev, i2c->irq,
> + octeon_i2c_isr, 0, DRV_NAME, i2c);
> if (result < 0) {
> dev_err(i2c->dev, "failed to attach interrupt\n");
> - goto fail_irq;
> + goto out;
> }
>
> result = octeon_i2c_initlowlevel(i2c);
> if (result) {
> dev_err(i2c->dev, "init low level failed\n");
> - goto fail_add;
> + goto out;
> }
>
> result = octeon_i2c_setclock(i2c);
> if (result) {
> dev_err(i2c->dev, "clock init failed\n");
> - goto fail_add;
> + goto out;
> }
>
> i2c->adap = octeon_i2c_ops;
> i2c->adap.dev.parent = &pdev->dev;
> - i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> + i2c->adap.dev.of_node = pdev->dev.of_node;
> i2c_set_adapdata(&i2c->adap, i2c);
> platform_set_drvdata(pdev, i2c);
>
> - result = i2c_add_numbered_adapter(&i2c->adap);
> + result = i2c_add_adapter(&i2c->adap);
> if (result < 0) {
> dev_err(i2c->dev, "failed to add adapter\n");
> goto fail_add;
> }
> -
> dev_info(i2c->dev, "version %s\n", DRV_VERSION);
>
> - return result;
> + of_i2c_register_devices(&i2c->adap);
> +
> + return 0;
>
> fail_add:
> platform_set_drvdata(pdev, NULL);
> - free_irq(i2c->irq, i2c);
> -fail_irq:
> - iounmap(i2c->twsi_base);
> - release_mem_region(i2c->twsi_phys, i2c->regsize);
> -fail_region:
> - kfree(i2c);
> out:
> return result;
> };
> @@ -613,19 +619,24 @@ static int __devexit octeon_i2c_remove(struct platform_device *pdev)
>
> i2c_del_adapter(&i2c->adap);
> platform_set_drvdata(pdev, NULL);
> - free_irq(i2c->irq, i2c);
> - iounmap(i2c->twsi_base);
> - release_mem_region(i2c->twsi_phys, i2c->regsize);
> - kfree(i2c);
> return 0;
> };
>
> +static struct of_device_id octeon_i2c_match[] = {
> + {
> + .compatible = "cavium,octeon-3860-twsi",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, octeon_i2c_match);
> +
> static struct platform_driver octeon_i2c_driver = {
> .probe = octeon_i2c_probe,
> .remove = __devexit_p(octeon_i2c_remove),
> .driver = {
> .owner = THIS_MODULE,
> .name = DRV_NAME,
> + .of_match_table = octeon_i2c_match,
> },
> };
>
> @@ -635,4 +646,3 @@ MODULE_AUTHOR("Michael Lawnick <michael.lawnick.ext@....com>");
> MODULE_DESCRIPTION("I2C-Bus adapter for Cavium OCTEON processors");
> MODULE_LICENSE("GPL");
> MODULE_VERSION(DRV_VERSION);
> -MODULE_ALIAS("platform:" DRV_NAME);
--
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/
Powered by blists - more mailing lists