| 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: <20140924105828.GB17019@localhost>
Date: Wed, 24 Sep 2014 12:58:28 +0200
From: Johan Hovold <johan@...nel.org>
To: Octavian Purdila <octavian.purdila@...el.com>
Cc: gregkh@...uxfoundation.org, linus.walleij@...aro.org,
gnurou@...il.com, wsa@...-dreams.de, sameo@...ux.intel.com,
lee.jones@...aro.org, arnd@...db.de, johan@...nel.org,
daniel.baluta@...el.com, laurentiu.palcu@...el.com,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH v5 2/4] i2c: add support for Diolan DLN-2 USB-I2C adapter
On Fri, Sep 19, 2014 at 11:22:43PM +0300, Octavian Purdila wrote:
> +struct dln2_i2c {
> + struct platform_device *pdev;
> + struct i2c_adapter adapter;
> + u32 freq;
> + u32 min_freq;
> + u32 max_freq;
> + /*
> + * Buffer to hold the packet for read or write transfers. One
> + * is enough since we can't have multiple transfers in
> + * parallel on the i2c adapter.
> + */
> + union {
> + struct {
> + u8 port;
> + u8 addr;
> + u8 mem_addr_len;
> + __le32 mem_addr;
> + __le16 buf_len;
> + u8 buf[DLN2_I2C_MAX_XFER_SIZE];
> + } __packed tx;
> + struct {
> + __le16 buf_len;
> + u8 buf[DLN2_I2C_MAX_XFER_SIZE];
> + } __packed rx;
> + } *buf;
Please declare these structs separately from the dln2_i2c struct so you
can use temporary pointers below.
Perhaps just allocate a large enough buffer (or page) here and keep you
transfer-buffer declarations in the two functions where they are used if
you prefer.
> +};
>
> +static int dln2_i2c_enable(struct dln2_i2c *dln2, bool enable)
> +{
> + struct dln2_platform_data *pdata = dev_get_platdata(&dln2->pdev->dev);
> + int ret;
> + u8 port;
> + u16 cmd;
> +
> + port = pdata->port;
Store the port number in the dln2_i2c struct rather than access it
through the platform data for every I/O operation.
> +
> + if (enable)
> + cmd = DLN2_I2C_ENABLE;
> + else
> + cmd = DLN2_I2C_DISABLE;
> +
> + ret = dln2_transfer(dln2->pdev, cmd, &port, sizeof(port), NULL, NULL);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int dln2_i2c_set_frequency(struct dln2_i2c *dln2, u32 freq)
> +{
> + int ret;
> + struct tx_data {
> + u8 port;
> + __le32 freq;
> + } __packed tx;
> + struct dln2_platform_data *pdata = dev_get_platdata(&dln2->pdev->dev);
> +
> + tx.port = pdata->port;
> + tx.freq = cpu_to_le32(freq);
> +
> + ret = dln2_transfer(dln2->pdev, DLN2_I2C_SET_FREQUENCY, &tx, sizeof(tx),
> + NULL, NULL);
> + if (ret < 0)
> + return ret;
> +
> + dln2->freq = freq;
> +
> + return 0;
> +}
> +
> +static int dln2_i2c_get_freq(struct dln2_i2c *dln2, u16 cmd, u32 *res)
> +{
> + int ret;
> + struct dln2_platform_data *pdata = dev_get_platdata(&dln2->pdev->dev);
> + u8 port = pdata->port;
> + __le32 freq;
> + unsigned len = sizeof(freq);
> +
> + ret = dln2_transfer(dln2->pdev, cmd, &port, sizeof(port),
> + &freq, &len);
> + if (ret < 0)
> + return ret;
> + if (len < sizeof(freq))
> + return -EPROTO;
> +
> + *res = le32_to_cpu(freq);
> +
> + return 0;
> +}
> +
> +static int dln2_i2c_setup(struct dln2_i2c *dln2)
> +{
> + int ret;
> +
> + ret = dln2_i2c_enable(dln2, false);
> + if (ret < 0)
> + return ret;
> +
> + ret = dln2_i2c_get_freq(dln2, DLN2_I2C_GET_MIN_FREQUENCY,
> + &dln2->min_freq);
> + if (ret < 0)
> + return 0;
return ret
> +
> + ret = dln2_i2c_get_freq(dln2, DLN2_I2C_GET_MAX_FREQUENCY,
> + &dln2->max_freq);
> + if (ret < 0)
> + return 0;
return ret
> +
> + ret = dln2_i2c_set_frequency(dln2, DLN2_I2C_FREQ_STD);
> + if (ret < 0)
> + return ret;
> +
> + ret = dln2_i2c_enable(dln2, true);
> +
> + return ret;
> +}
> +
> +static int dln2_i2c_write(struct dln2_i2c *dln2, u8 addr,
> + u8 *data, u16 data_len)
> +{
> + struct dln2_platform_data *pdata = dev_get_platdata(&dln2->pdev->dev);
> + int ret;
> + unsigned len;
> +
> + dln2->buf->tx.port = pdata->port;
> + dln2->buf->tx.addr = addr;
> + dln2->buf->tx.mem_addr_len = 0;
> + dln2->buf->tx.mem_addr = 0;
> + dln2->buf->tx.buf_len = cpu_to_le16(data_len);
> + memcpy(dln2->buf->tx.buf, data, data_len);
> +
> + len = sizeof(dln2->buf->tx) + data_len - DLN2_I2C_MAX_XFER_SIZE;
> + ret = dln2_transfer(dln2->pdev, DLN2_I2C_WRITE, &dln2->buf->tx, len,
> + NULL, NULL);
Use a temporary pointer to the tx struct above for readability.
> + if (ret < 0)
> + return ret;
> +
> + return data_len;
> +}
> +
> +static int dln2_i2c_read(struct dln2_i2c *dln2, u16 addr, u8 *data,
> + u16 data_len)
> +{
> + struct dln2_platform_data *pdata = dev_get_platdata(&dln2->pdev->dev);
> + int ret;
> + struct {
> + u8 port;
> + u8 addr;
> + u8 mem_addr_len;
> + __le32 mem_addr;
> + __le16 buf_len;
> + } __packed tx;
> + unsigned rx_len;
> +
> + tx.port = pdata->port;
> + tx.addr = addr;
> + tx.mem_addr_len = 0;
> + tx.mem_addr = 0;
> + tx.buf_len = cpu_to_le16(data_len);
> +
> + rx_len = sizeof(dln2->buf->rx);
> +
> + ret = dln2_transfer(dln2->pdev, DLN2_I2C_READ, &tx, sizeof(tx),
> + &dln2->buf->rx, &rx_len);
> + if (ret < 0)
> + return ret;
> + if (rx_len < sizeof(dln2->buf->rx.buf_len) + data_len)
> + return -EPROTO;
> + if (le16_to_cpu(dln2->buf->rx.buf_len) != data_len)
> + return -EPROTO;
> +
> + memcpy(data, dln2->buf->rx.buf, data_len);'
Use a temporary pointer to the rx struct above.
> +
> + return data_len;
> +}
[...]
> +static DEVICE_ATTR_RW(dln2_i2c_freq);
Any new sysfs attribute must to be documented under Documentation/ABI.
[...]
> +static struct platform_driver dln2_i2c_driver = {
> + .driver.name = "dln2-i2c",
> + .driver.owner = THIS_MODULE,
No need to set the owner field when using the module_platform_driver
macro.
> + .probe = dln2_i2c_probe,
> + .remove = dln2_i2c_remove,
> +};
> +
> +module_platform_driver(dln2_i2c_driver);
> +
> +MODULE_AUTHOR("Laurentiu Palcu <laurentiu.palcu@...el.com>");
> +MODULE_DESCRIPTION("Driver for the Diolan DLN2 I2C master interface");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:dln2-i2c");
Johan
--
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