| 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: <20140401005001.GA3590@katana>
Date: Tue, 1 Apr 2014 02:50:01 +0200
From: Wolfram Sang <wsa@...-dreams.de>
To: Soren Brinkmann <soren.brinkmann@...inx.com>
Cc: 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>,
Rob Landley <rob@...dley.net>,
Russell King <linux@....linux.org.uk>,
Michal Simek <michal.simek@...inx.com>,
Grant Likely <grant.likely@...aro.org>,
Mike Looijmans <mike.looijmans@...ic.nl>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-doc@...r.kernel.org, devicetree@...r.kernel.org,
linux-i2c@...r.kernel.org, Harini Katakam <harinik@...inx.com>
Subject: Re: [PATCH v3 1/2] i2c: Add driver for Cadence I2C controller
On Tue, Mar 11, 2014 at 09:50:12AM -0700, Soren Brinkmann wrote:
> Add a driver for the Cadence I2C controller. This controller is for
> example found in Xilinx Zynq.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@...inx.com>
...
> +static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
> +{
> + unsigned int isr_status, avail_bytes;
> + unsigned int bytes_to_recv, bytes_to_send;
> + struct cdns_i2c *id = ptr;
> + /* Signal completion only after everything is updated */
> + int done_flag = 0;
> +
> + isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
> +
> + /* Handling nack and arbitration lost interrupt */
> + if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST))
> + done_flag = 1;
> +
> + /* Handling Data interrupt */
> + if ((isr_status & CDNS_I2C_IXR_DATA) &&
> + (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
> + /* Always read data interrupt threshold bytes */
> + bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
> + id->recv_count = id->recv_count - CDNS_I2C_DATA_INTR_DEPTH;
Use '-='
...
> +static int cdns_i2c_process_msg(struct cdns_i2c *id, struct i2c_msg *msg,
> + struct i2c_adapter *adap)
> +{
> + int ret;
> + u32 reg;
> + bool retry = false;
> + unsigned retries = adap->retries;
> +
> + id->p_msg = msg;
> + do {
> + id->err_status = 0;
> + init_completion(&id->xfer_done);
reinit_completion. And add init_completion in probe.
> +
> + /* Check for the TEN Bit mode on each msg */
> + reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
> + if (msg->flags & I2C_M_TEN) {
> + if (reg & CDNS_I2C_CR_NEA)
> + cdns_i2c_writereg(reg & ~CDNS_I2C_CR_NEA,
> + CDNS_I2C_CR_OFFSET);
> + } else {
> + if (!(reg & CDNS_I2C_CR_NEA))
> + cdns_i2c_writereg(reg | CDNS_I2C_CR_NEA,
> + CDNS_I2C_CR_OFFSET);
> + }
> +
> + /* Check for the R/W flag on each msg */
> + if (msg->flags & I2C_M_RD)
> + cdns_i2c_mrecv(id);
> + else
> + cdns_i2c_msend(id);
> +
> + /* Wait for the signal of completion */
> + ret = wait_for_completion_timeout(&id->xfer_done, HZ);
> + if (!ret) {
> + cdns_i2c_master_reset(adap);
> + dev_err(id->adap.dev.parent,
> + "timeout waiting on completion\n");
> + return -ETIMEDOUT;
> + }
> +
> + cdns_i2c_writereg(CDNS_I2C_IXR_ALL_INTR_MASK,
> + CDNS_I2C_IDR_OFFSET);
> +
> + /* If it is bus arbitration error, try again */
> + if (id->err_status & CDNS_I2C_IXR_ARB_LOST) {
Just return -EAGAIN here...
> + dev_dbg(id->adap.dev.parent,
> + "Lost ownership on bus, trying again\n");
> + if (retries--) {
> + mdelay(2);
> + retry = true;
> + } else {
> + dev_err(id->adap.dev.parent,
> + "Retries completed, exit\n");
> + return -EREMOTEIO;
> + }
> + }
... and you can skip this block since the core will do the retries.
> + } while (retry);
> +
> + return 0;
> +}
> +
> +/**
> + * cdns_i2c_master_xfer - The main i2c transfer function
> + * @adap: pointer to the i2c adapter driver instance
> + * @msgs: pointer to the i2c message structure
> + * @num: the number of messages to transfer
> + *
> + * Return: number of msgs processed on success, negative error otherwise
> + *
> + * This function waits for the bus idle condition and updates the timeout if
> + * modified by user. Then initiates the send/recv activity based on the
> + * transfer message received.
> + */
> +static int cdns_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> + int num)
> +{
> + struct cdns_i2c *id = adap->algo_data;
> + unsigned long timeout;
> + int ret, count;
> + u32 reg;
> +
> + /* Waiting for bus-ready. If bus not ready, it returns after timeout */
> + timeout = jiffies + CDNS_I2C_TIMEOUT;
> + while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) & CDNS_I2C_SR_BA) {
> + if (time_after(jiffies, timeout)) {
> + dev_warn(id->adap.dev.parent,
> + "timedout waiting for bus ready\n");
> + cdns_i2c_master_reset(adap);
> + return -ETIMEDOUT;
> + }
> + schedule_timeout(1);
> + }
> +
> + /* The bus is free. Set the new timeout value if updated */
> + if (id->adap.timeout != id->cur_timeout) {
> + cdns_i2c_writereg(id->adap.timeout & CDNS_I2C_TIME_OUT_TO_MASK,
> + CDNS_I2C_TIME_OUT_OFFSET);
> + id->cur_timeout = id->adap.timeout;
> + }
> +
> + /*
> + * Set the flag to one when multiple messages are to be
> + * processed with a repeated start.
> + */
> + if (num > 1) {
> + id->bus_hold_flag = 1;
> + reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
> + reg |= CDNS_I2C_CR_HOLD;
> + cdns_i2c_writereg(reg, CDNS_I2C_CR_OFFSET);
> + } else {
> + id->bus_hold_flag = 0;
> + }
> +
> + /* Process the msg one by one */
> + for (count = 0; count < num; count++, msgs++) {
> + if (count == (num - 1))
> + id->bus_hold_flag = 0;
> +
> + ret = cdns_i2c_process_msg(id, msgs, adap);
> + if (ret)
> + return ret;
> +
> + /* Report the other error interrupts to application as EIO */
> + if (id->err_status & 0xE4) {
Please replace this magic hex value with something readable.
> + cdns_i2c_master_reset(adap);
> + return -EIO;
> + }
> + }
> +
> + return num;
> +}
> +
...
> +/**
> + * cdns_i2c_probe - Platform registration call
> + * @pdev: Handle to the platform device structure
> + *
> + * Return: 0 on success, negative error otherwise
> + *
> + * This function does all the memory allocation and registration for the i2c
> + * device. User can modify the address mode to 10 bit address mode using the
> + * ioctl call with option I2C_TENBIT.
> + */
> +static int cdns_i2c_probe(struct platform_device *pdev)
> +{
> + struct resource *r_mem;
> + struct cdns_i2c *id;
> + int ret;
> +
> + id = devm_kzalloc(&pdev->dev, sizeof(*id), GFP_KERNEL);
> + if (!id)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, id);
> +
> + r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + id->membase = devm_ioremap_resource(&pdev->dev, r_mem);
> + if (IS_ERR(id->membase))
> + return PTR_ERR(id->membase);
> +
> + id->irq = platform_get_irq(pdev, 0);
> +
> + id->adap.nr = pdev->id;
Drop this line and use i2c_add_adapter() later to let the core handle
the numbering. This will also take aliases into account.
> + id->adap.dev.of_node = pdev->dev.of_node;
> + id->adap.algo = &cdns_i2c_algo;
> + id->adap.timeout = 0x1F; /* Default timeout value */
This value is in jiffies, so you probably want to use msecs_to_jiffies
or alike.
> + id->adap.retries = 3; /* Default retry value. */
> + id->adap.algo_data = id;
> + id->adap.dev.parent = &pdev->dev;
> + snprintf(id->adap.name, sizeof(id->adap.name),
> + "Cadence I2C at %08lx", (unsigned long)r_mem->start);
> +
> + id->cur_timeout = id->adap.timeout;
> + id->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(id->clk)) {
> + dev_err(&pdev->dev, "input clock not found.\n");
> + return PTR_ERR(id->clk);
> + }
> + ret = clk_prepare_enable(id->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to enable clock.\n");
> + return ret;
> + }
> + id->clk_rate_change_nb.notifier_call = cdns_i2c_clk_notifier_cb;
> + if (clk_notifier_register(id->clk, &id->clk_rate_change_nb))
> + dev_warn(&pdev->dev, "Unable to register clock notifier.\n");
> + id->input_clk = clk_get_rate(id->clk);
> +
> + ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> + &id->i2c_clk);
> + if (ret || (id->i2c_clk > CDNS_I2C_SPEED_MAX))
> + id->i2c_clk = CDNS_I2C_SPEED_MAX;
> +
> + cdns_i2c_writereg(0xE, CDNS_I2C_CR_OFFSET);
Remove magic value. Please check the driver for more, in case I missed
some.
> + cdns_i2c_writereg(id->adap.timeout, CDNS_I2C_TIME_OUT_OFFSET);
> +
> + ret = cdns_i2c_setclk(id->input_clk, id);
> + if (ret) {
> + dev_err(&pdev->dev, "invalid SCL clock: %u Hz\n", id->i2c_clk);
> + ret = -EINVAL;
> + goto err_clk_dis;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, id->irq, cdns_i2c_isr, 0,
> + DRIVER_NAME, id);
> + if (ret) {
> + dev_err(&pdev->dev, "cannot get irq %d\n", id->irq);
> + goto err_clk_dis;
> + }
> +
> + ret = i2c_add_numbered_adapter(&id->adap);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "reg adap failed: %d\n", ret);
> + goto err_clk_dis;
> + }
> +
> + dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
> + id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
> +
> + return 0;
> +
> +err_clk_dis:
> + clk_disable_unprepare(id->clk);
> + return ret;
> +}
> +
Thanks,
Wolfram
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists