On Tue, Oct 7, 2014 at 7:46 PM, Johan Hovold <johan@xxxxxxxxxx> wrote: > On Thu, Sep 25, 2014 at 07:07:32PM +0300, Octavian Purdila wrote: >> From: Laurentiu Palcu <laurentiu.palcu@xxxxxxxxx> >> >> This patch adds support for the Diolan DLN-2 I2C master module. Due >> to hardware limitations it does not support SMBUS quick commands. >> >> Information about the USB protocol interface can be found in the >> Programmer's Reference Manual [1], see section 6.2.2 for the I2C >> master module commands and responses. >> >> [1] https://www.diolan.com/downloads/dln-api-manual.pdf >> >> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@xxxxxxxxx> >> Signed-off-by: Octavian Purdila <octavian.purdila@xxxxxxxxx> >> --- > > [...] > >> +#define DLN2_I2C_MAX_XFER_SIZE 256 >> +#define DLN2_I2C_BUF_SIZE (DLN2_I2C_MAX_XFER_SIZE + 16) >> + >> +struct dln2_i2c { >> + struct platform_device *pdev; >> + struct i2c_adapter adapter; >> + u32 freq; >> + u32 min_freq; >> + u32 max_freq; >> + u8 port; >> + /* >> + * 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. >> + */ >> + void *buf; >> +}; >> + >> +static int dln2_i2c_enable(struct dln2_i2c *dln2, bool enable) >> +{ >> + int ret; >> + u16 cmd; >> + >> + if (enable) >> + cmd = DLN2_I2C_ENABLE; >> + else >> + cmd = DLN2_I2C_DISABLE; >> + >> + ret = dln2_transfer(dln2->pdev, cmd, &dln2->port, sizeof(dln2->port), >> + NULL, NULL); > > Don't use the port member of dln2 directly here. Encode the protocol in > the function as you already do for most other messages (and did in > previous versions). You could even declare a > > struct { > u8 port; > } tx; > > for consistency. > Yes, I think I did this after Arnd's review on the gpio stuff where I thought he suggested to remove the structure, when in fact he asked to remove the __packed attribute. I will revert it back. >> + 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; >> + >> + tx.port = dln2->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; >> + __le32 freq; >> + unsigned len = sizeof(freq); >> + >> + ret = dln2_transfer(dln2->pdev, cmd, &dln2->port, sizeof(dln2->port), >> + &freq, &len); > > Same here. > OK. >> + if (ret < 0) >> + return ret; >> + if (len < sizeof(freq)) >> + return -EPROTO; >> + >> + *res = le32_to_cpu(freq); >> + >> + return 0; >> +} >> + > > [...] > >> +static ssize_t dln2_i2c_freq_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct dln2_i2c *dln2 = dev_get_drvdata(dev); >> + >> + return snprintf(buf, PAGE_SIZE, "%d\n", dln2->freq); >> +} >> + >> +static ssize_t dln2_i2c_freq_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct dln2_i2c *dln2 = dev_get_drvdata(dev); >> + unsigned long freq; >> + int ret; >> + >> + if (kstrtoul(buf, 0, &freq) || freq < dln2->min_freq || >> + freq > dln2->max_freq) >> + return -EINVAL; >> + >> + ret = dln2_i2c_enable(dln2, false); >> + if (ret < 0) >> + return ret; >> + >> + ret = dln2_i2c_set_frequency(dln2, freq); >> + if (ret < 0) >> + return ret; >> + >> + ret = dln2_i2c_enable(dln2, true); >> + >> + return count; >> +} >> + >> +static DEVICE_ATTR_RW(dln2_i2c_freq); > > You forgot to add the required documentation of the attribute to > Documentation/ABI as requested. > Hmm, I did add a new entry in Documentation/ABI/ at testing/sysfs-bus-i2c-busses-dln2. It should be right at the beginning at the patch. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
