| 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: <1389704580.2794.17.camel@iivanov-dev.int.mm-sol.com>
Date: Tue, 14 Jan 2014 15:03:00 +0200
From: "Ivan T. Ivanov" <iivanov@...sol.com>
To: Bjorn Andersson <bjorn.andersson@...ymobile.com>
Cc: Rob Herring <rob.herring@...xeda.com>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Rob Landley <rob@...dley.net>,
Wolfram Sang <wsa@...-dreams.de>,
Grant Likely <grant.likely@...aro.org>,
Jean Delvare <khali@...ux-fr.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Martin Schwidefsky <schwidefsky@...ibm.com>,
James Ralston <james.d.ralston@...el.com>,
Bill Brown <bill.e.brown@...el.com>,
Matt Porter <matt.porter@...aro.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] i2c: New bus driver for the QUP I2C controller
Hi Bjorn,
Just two comments.
On Mon, 2014-01-13 at 16:30 -0800, Bjorn Andersson wrote:
> From: "Ivan T. Ivanov" <iivanov@...sol.com>
>
> This bus driver supports the QUP i2c hardware controller in the Qualcomm
> MSM SOCs. The Qualcomm Universal Peripheral Engine (QUP) is a general
> purpose data path engine with input/output FIFOs and an embedded i2c
> mini-core. The driver supports FIFO mode (for low bandwidth applications)
> and block mode (interrupt generated for each block-size data transfer).
> The driver currently does not support DMA transfers.
>
> Shamelessly based on codeaurora version of the driver.
>
> Signed-off-by: Ivan T. Ivanov <iivanov@...sol.com>
> [bjorn: updated to reflect i2c framework changes
> splited up qup_i2c_enable() in enable/disable
> don't overwrite ret value on error in xfer functions
> initilize core for each transfer
> remove explicit pinctrl selection
> use existing clock instead of setting new core clock]
> Signed-off-by: Bjorn Andersson <bjorn.andersson@...ymobile.com>
> ---
<snip>
> +
> +static int qup_i2c_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct qup_i2c_dev *qup;
> + struct resource *res;
> + u32 val, io_mode, hw_ver, size;
> + int ret, fs_div, hs_div;
> + int src_clk_freq;
> +
> + qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
> + if (!qup)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, qup);
> +
> + ret = of_alias_get_id(node, "i2c");
> + if (ret >= 0)
> + pdev->id = ret;
This is part of the i2c_add_adapter() and have to be dropped.
> +
> + qup->dev = &pdev->dev;
> +
> + qup->clk_freq = 100000;
> + if (!of_property_read_u32(node, "clock-frequency", &val))
> + qup->clk_freq = val;
> +
> + /* We support frequencies up to FAST Mode(400KHz) */
> + if (qup->clk_freq <= 0 || qup->clk_freq > 400000) {
> + dev_err(qup->dev, "clock frequency not supported %d\n",
> + qup->clk_freq);
> + return -EIO;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + qup->base = devm_ioremap_resource(qup->dev, res);
> + if (IS_ERR(qup->base))
> + return PTR_ERR(qup->base);
> +
> + qup->irq = platform_get_irq(pdev, 0);
> + if (qup->irq < 0) {
> + dev_err(qup->dev, "No IRQ defined\n");
> + return qup->irq;
> + }
> +
> + qup->clk = devm_clk_get(qup->dev, "core");
> + if (IS_ERR(qup->clk)) {
> + dev_err(qup->dev, "Could not get core clock\n");
> + return PTR_ERR(qup->clk);
> + }
> +
> + qup->pclk = devm_clk_get(qup->dev, "iface");
> + if (IS_ERR(qup->pclk)) {
> + dev_err(qup->dev, "Could not get iface clock\n");
> + return PTR_ERR(qup->pclk);
> + }
> +
> + init_completion(&qup->xfer);
> +
> + qup_i2c_enable_clocks(qup);
> +
> + /*
> + * Bootloaders might leave a pending interrupt on certain QUP's,
> + * so we reset the core before registering for interrupts.
> + */
> + writel(1, qup->base + QUP_SW_RESET);
> + ret = qup_i2c_poll_state(qup, 0, true);
> + if (ret)
> + goto fail;
> +
> + ret = devm_request_irq(qup->dev, qup->irq, qup_i2c_interrupt,
> + IRQF_TRIGGER_HIGH, "i2c_qup", qup);
> + if (ret) {
> + dev_err(qup->dev, "Request %d IRQ failed\n", qup->irq);
> + goto fail;
> + }
> + disable_irq(qup->irq);
> +
> + hw_ver = readl(qup->base + QUP_HW_VERSION);
> + dev_dbg(qup->dev, "%d Revision %x\n", pdev->id, hw_ver);
> +
> + src_clk_freq = clk_get_rate(qup->clk);
> + fs_div = ((src_clk_freq / qup->clk_freq) / 2) - 3;
> + hs_div = 3;
> + qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
> + qup->one_bit_t = (USEC_PER_SEC / qup->clk_freq) + 1;
> +
> + io_mode = readl(qup->base + QUP_IO_MODE);
> +
> + size = QUP_OUTPUT_BLOCK_SIZE(io_mode);
> + if (size)
> + qup->out_blk_sz = size * 16;
> + else
> + qup->out_blk_sz = 16;
> +
> + size = QUP_INPUT_BLOCK_SIZE(io_mode);
> + if (size)
> + qup->in_blk_sz = size * 16;
> + else
> + qup->in_blk_sz = 16;
> +
> + qup->xfer_time = msecs_to_jiffies(qup->out_fifo_sz);
> +
> + /*
> + * The block/fifo size w.r.t. 'actual data' is 1/2 due to 'tag'
> + * associated with each byte written/received
> + */
> + qup->out_blk_sz /= 2;
> + qup->in_blk_sz /= 2;
> +
> + size = QUP_OUTPUT_FIFO_SIZE(io_mode);
> + qup->out_fifo_sz = qup->out_blk_sz * (2 << size);
> +
> + size = QUP_INPUT_FIFO_SIZE(io_mode);
> + qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
> +
> + /*
> + * Wait for FIFO number of bytes to be absolutely sure
> + * that I2C write state machine is not idle. Each byte
> + * takes 9 clock cycles. (8 bits + 1 ack)
> + */
> + qup->wait_idle = qup->one_bit_t * 9;
> + qup->wait_idle *= qup->out_fifo_sz;
> +
> + dev_info(qup->dev, "IN:block:%d, fifo:%d, OUT:block:%d, fifo:%d\n",
> + qup->in_blk_sz, qup->in_fifo_sz,
> + qup->out_blk_sz, qup->out_fifo_sz);
This could be lowered to dbg, I think.
> +
> + i2c_set_adapdata(&qup->adap, qup);
> + qup->adap.algo = &qup_i2c_algo;
> + qup->adap.nr = pdev->id;
> + qup->adap.dev.parent = qup->dev;
> + qup->adap.dev.of_node = pdev->dev.of_node;
> + strlcpy(qup->adap.name, "QUP I2C adapter", sizeof(qup->adap.name));
> +
> + ret = i2c_add_numbered_adapter(&qup->adap);
> + if (!ret) {
> + pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
> + pm_runtime_use_autosuspend(qup->dev);
> + pm_runtime_enable(qup->dev);
> + return 0;
> + }
> +fail:
> + qup_i2c_disable_clocks(qup);
> + return ret;
> +}
> +
Thanks,
Ivan
--
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