linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATH v3 0/4] i2c: show and change bus frequency via sysfs
@ 2014-10-15 20:03 Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 1/4] i2c: document the existing i2c sysfs ABI Octavian Purdila
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Octavian Purdila @ 2014-10-15 20:03 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel, Octavian Purdila

This patch series adds support to show and change the bus frequency
via sysfs, by exposing files to show the minimum, maximum and current
frequency as well as allowing the frequency to be changed. This allows
the user to view or change the bus frequency on a per bus level.

Changes since v3:

 * use DEVICE_ATTR_RO and make i2c_adapter_visible_attr static
 
 * implement sysfs bus frequency support for the diolan u2c bus

Octavian Purdila (4):
  i2c: document the existing i2c sysfs ABI
  i2c: document struct i2c_adapter
  i2c: show and change bus frequency via sysfs
  i2c: i2c-diolan-u2c: sysfs bus frequency support

 Documentation/ABI/testing/sysfs-bus-i2c | 75 +++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-diolan-u2c.c     | 49 ++++++++++++------
 drivers/i2c/i2c-core.c                  | 90 +++++++++++++++++++++++++++++++++
 include/linux/i2c.h                     | 32 ++++++++++--
 4 files changed, 228 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATH v3 1/4] i2c: document the existing i2c sysfs ABI
  2014-10-15 20:03 [PATH v3 0/4] i2c: show and change bus frequency via sysfs Octavian Purdila
@ 2014-10-15 20:03 ` Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 2/4] i2c: document struct i2c_adapter Octavian Purdila
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Octavian Purdila @ 2014-10-15 20:03 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel, Octavian Purdila

This patch adds Documentation/ABI/testing/sysfs-bus-i2c which
documents the existing i2c sysfs ABI.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-i2c | 45 +++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c b/Documentation/ABI/testing/sysfs-bus-i2c
new file mode 100644
index 0000000..8075585
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c
@@ -0,0 +1,45 @@
+What:		/sys/bus/i2c/devices/i2c-X
+KernelVersion:	since at least 2.6.12
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		This entry represents a registered i2c bus. X is the
+		bus number and its format is "%d".
+
+What:		/sys/bus/i2c/devices/i2c-X/Y
+What:		/sys/bus/i2c/devices/Y
+KernelVersion:	since at least 2.6.12
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		An i2c device attached to bus X. Format of Y is
+		"%d-%04x" where the first number is the bus number (X)
+		and the second number is the device i2c address.
+
+What:		/sys/bus/i2c/devices/i2c-X/new_device
+KernelVersion:	2.6.31
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		Write only entry that allows instantiating a
+		new i2c device on bus X. This is to be used when
+		enumeration mechanism such as ACPI or DT are not
+		present or not used for this device.
+		Format: "%s %hi\n" where the first argument is the
+		device name (no spaces allowed) and the second is the
+		i2c address of the device.
+
+What:		/sys/bus/i2c/devices/i2c-X/delete_device
+KernelVersion:	2.6.31
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		Write only entry that allows the removal of an i2c
+		device from bus X.
+		Format: "%s %hi\n" where the first argument is the
+		device name (no spaces allowed) and the second is the
+		i2c address of the device.
+
+What:		/sys/bus/i2c/devices/i2c-X/i2c-Y
+What:		/sys/bus/i2c/devices/i2c-Y
+KernelVersion:	3.13
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		An i2c device attached to bus X that is enumerated via
+		ACPI. Y is the ACPI device name and its format is "%s".
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATH v3 2/4] i2c: document struct i2c_adapter
  2014-10-15 20:03 [PATH v3 0/4] i2c: show and change bus frequency via sysfs Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 1/4] i2c: document the existing i2c sysfs ABI Octavian Purdila
@ 2014-10-15 20:03 ` Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 3/4] i2c: show and change bus frequency via sysfs Octavian Purdila
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Octavian Purdila @ 2014-10-15 20:03 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel, Octavian Purdila

Document the i2c_adapter fields that must be initialized before
calling i2c_add_adapter().

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 include/linux/i2c.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a95efeb..36041e2 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -418,9 +418,19 @@ int i2c_recover_bus(struct i2c_adapter *adap);
 int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
 int i2c_generic_scl_recovery(struct i2c_adapter *adap);
 
-/*
- * i2c_adapter is the structure used to identify a physical i2c bus along
- * with the access algorithms necessary to access it.
+/**
+ * struct i2c_adapter - represents an I2C physical bus
+ *
+ * The following members must be set by the adapter driver before
+ * calling i2c_add_adapter():
+ *
+ * @owner: the module owner, usually THIS_MODULE
+ * @class: bitmask of I2C_CLASS_*
+ * @algo: see struct i2c_algorithm
+ * @dev.parent: set this to the struct device of the driver (e.g. pci_dev->dev,
+ *	usb->interface->dev, platform_device->dev etc.)
+ * @name: name of this i2c bus
+ * @bus_recovery_info: see struct i2c_bus_recovery_info. Optional.
  */
 struct i2c_adapter {
 	struct module *owner;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATH v3 3/4] i2c: show and change bus frequency via sysfs
  2014-10-15 20:03 [PATH v3 0/4] i2c: show and change bus frequency via sysfs Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 1/4] i2c: document the existing i2c sysfs ABI Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 2/4] i2c: document struct i2c_adapter Octavian Purdila
@ 2014-10-15 20:03 ` Octavian Purdila
  2014-10-15 20:03 ` [PATH v3 4/4] i2c: i2c-diolan-u2c: sysfs bus frequency support Octavian Purdila
  2014-10-16  6:53 ` [PATH v3 0/4] i2c: show and change bus frequency via sysfs Wolfram Sang
  4 siblings, 0 replies; 7+ messages in thread
From: Octavian Purdila @ 2014-10-15 20:03 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel, Octavian Purdila

This patch adds three new sysfs files: frequency, frequency_min and
frequency_max which allows the user to view or change the bus
frequency on a per bus level.

This is required for the case where multiple busses have the same
adapter driver in which case a module parameter does not allow
controlling the bus speed individually (e.g. USB I2C adapters).

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-i2c | 30 +++++++++++
 drivers/i2c/i2c-core.c                  | 90 +++++++++++++++++++++++++++++++++
 include/linux/i2c.h                     | 16 ++++++
 3 files changed, 136 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c b/Documentation/ABI/testing/sysfs-bus-i2c
index 8075585..70af81f 100644
--- a/Documentation/ABI/testing/sysfs-bus-i2c
+++ b/Documentation/ABI/testing/sysfs-bus-i2c
@@ -43,3 +43,33 @@ Contact:	linux-i2c@vger.kernel.org
 Description:
 		An i2c device attached to bus X that is enumerated via
 		ACPI. Y is the ACPI device name and its format is "%s".
+
+What:		/sys/bus/i2c/devices/i2c-X/frequency
+KernelVersion:	3.19
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		The current bus frequency for bus X. Can be updated if
+		the bus supports it. The unit is HZ and format is
+		"%d\n".
+		If the bus does not support changing the frequency
+		then this entry will be read-only.
+		If the bus does not support showing the frequency than
+		this entry will not be visible.
+		When updating the bus frequency that value must be in
+		the range defined by bus_frequency_min and
+		bus_frequency_max otherwise writing to this entry will
+		fail with -EINVAL.
+		The bus may not support all of the frequencies in the
+		min-max range and may round the frequency to the
+		closest supported one. The user must read the entry
+		after writing it to retrieve the actual set frequency.
+
+What:		/sys/bus/i2c/devices/i2c-X/frequency_min
+What:		/sys/bus/i2c/devices/i2c-X/frequency_max
+KernelVersion:	3.19
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		Minimum and maximum frequencies for bus X. The unit is
+		HZ and format is "%d\n".
+		If the bus does not support changing the frequency
+		these entries will not be visible.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 632057a..27c29eb 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -940,15 +940,97 @@ static DEVICE_ATTR(new_device, S_IWUSR, NULL, i2c_sysfs_new_device);
 static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, S_IWUSR, NULL,
 				   i2c_sysfs_delete_device);
 
+static ssize_t
+i2c_sysfs_freq_show(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", adap->freq);
+}
+
+static ssize_t
+i2c_sysfs_freq_store(struct device *dev, struct device_attribute *attr,
+		     const char *buf, size_t count)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+	unsigned int freq;
+	int ret;
+
+	if (kstrtouint(buf, 0, &freq) || freq < adap->min_freq ||
+	    freq > adap->max_freq)
+		return -EINVAL;
+
+	i2c_lock_adapter(adap);
+	ret = adap->set_freq(adap, &freq);
+	i2c_unlock_adapter(adap);
+
+	if (ret)
+		return ret;
+
+	adap->freq = freq;
+
+	return count;
+}
+
+static DEVICE_ATTR(frequency, S_IRUGO, i2c_sysfs_freq_show,
+		   i2c_sysfs_freq_store);
+
+static ssize_t frequency_min_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", adap->min_freq);
+}
+
+static DEVICE_ATTR_RO(frequency_min);
+
+static ssize_t frequency_max_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", adap->max_freq);
+}
+
+static DEVICE_ATTR_RO(frequency_max);
+
+static umode_t i2c_adapter_visible_attr(struct kobject *kobj,
+					struct attribute *attr, int idx)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_frequency_min.attr)
+		return adap->min_freq ? mode : 0;
+
+	if (attr == &dev_attr_frequency_max.attr)
+		return adap->max_freq ? mode : 0;
+
+	if (attr == &dev_attr_frequency.attr) {
+		if (adap->set_freq)
+			mode |= S_IWUSR;
+		return adap->freq ? mode : 0;
+	}
+
+	return mode;
+}
+
 static struct attribute *i2c_adapter_attrs[] = {
 	&dev_attr_name.attr,
 	&dev_attr_new_device.attr,
 	&dev_attr_delete_device.attr,
+	&dev_attr_frequency.attr,
+	&dev_attr_frequency_min.attr,
+	&dev_attr_frequency_max.attr,
 	NULL
 };
 
 static struct attribute_group i2c_adapter_attr_group = {
 	.attrs		= i2c_adapter_attrs,
+	.is_visible	= i2c_adapter_visible_attr,
 };
 
 static const struct attribute_group *i2c_adapter_attr_groups[] = {
@@ -1262,6 +1344,14 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 	struct device *dev = &adapter->dev;
 	int id;
 
+	if (adapter->set_freq) {
+		if (!adapter->freq || !adapter->min_freq || !adapter->max_freq)
+			return -EINVAL;
+	} else {
+		if (adapter->min_freq || adapter->max_freq)
+			return -EINVAL;
+	}
+
 	if (dev->of_node) {
 		id = of_alias_get_id(dev->of_node, "i2c");
 		if (id >= 0) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 36041e2..5d893f5 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -431,6 +431,17 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap);
  *	usb->interface->dev, platform_device->dev etc.)
  * @name: name of this i2c bus
  * @bus_recovery_info: see struct i2c_bus_recovery_info. Optional.
+ * @set_freq: set the bus frequency (in HZ) and returns the actual set
+ *	value. Since not all the frequencies in the min - max interval
+ *	may be valid the driver may round the frequency to the closest
+ *	supported one. Optional but must be set if min_freq or
+ *	max_freq is set.
+ * @min_freq: minimum bus frequency. Optional but must be set if
+ *	set_freq is set.
+ * @max_freq: maximum bus frequency. Optional but must be set if
+ *	set_freq is set.
+ * @freq: initial bus frequency. Optional but must be set if set_freq
+ *	is set.
  */
 struct i2c_adapter {
 	struct module *owner;
@@ -438,6 +449,11 @@ struct i2c_adapter {
 	const struct i2c_algorithm *algo; /* the algorithm to access the bus */
 	void *algo_data;
 
+	unsigned int freq;
+	unsigned int min_freq;
+	unsigned int max_freq;
+	int (*set_freq)(struct i2c_adapter *a, unsigned int *freq);
+
 	/* data fields that are valid for all devices	*/
 	struct rt_mutex bus_lock;
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATH v3 4/4] i2c: i2c-diolan-u2c: sysfs bus frequency support
  2014-10-15 20:03 [PATH v3 0/4] i2c: show and change bus frequency via sysfs Octavian Purdila
                   ` (2 preceding siblings ...)
  2014-10-15 20:03 ` [PATH v3 3/4] i2c: show and change bus frequency via sysfs Octavian Purdila
@ 2014-10-15 20:03 ` Octavian Purdila
  2014-10-16 13:24   ` Guenter Roeck
  2014-10-16  6:53 ` [PATH v3 0/4] i2c: show and change bus frequency via sysfs Wolfram Sang
  4 siblings, 1 reply; 7+ messages in thread
From: Octavian Purdila @ 2014-10-15 20:03 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel, Octavian Purdila

Add support for showing and changing the bus frequency via
sysfs.

Tested on a DLN2 adapter run in U2C-12 compatibility mode.

Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/i2c/busses/i2c-diolan-u2c.c | 49 +++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
index b19a310..ff4e120 100644
--- a/drivers/i2c/busses/i2c-diolan-u2c.c
+++ b/drivers/i2c/busses/i2c-diolan-u2c.c
@@ -71,6 +71,9 @@
 #define U2C_I2C_FREQ_STD	100000
 #define U2C_I2C_FREQ(s)		(1000000 / (2 * (s - 1) + 10))
 
+#define U2C_I2C_MIN_FREQ	U2C_I2C_FREQ(U2C_I2C_SPEED_2KHZ)
+#define U2C_I2C_MAX_FREQ	U2C_I2C_FREQ_FAST
+
 #define DIOLAN_USB_TIMEOUT	100	/* in ms */
 #define DIOLAN_SYNC_TIMEOUT	20	/* in ms */
 
@@ -298,31 +301,24 @@ static void diolan_get_serial(struct i2c_diolan_u2c *dev)
 	}
 }
 
-static int diolan_init(struct i2c_diolan_u2c *dev)
+static int diolan_set_freq(struct i2c_adapter *adapter, unsigned int *frequency)
 {
+	struct i2c_diolan_u2c *dev = i2c_get_adapdata(adapter);
 	int speed, ret;
 
-	if (frequency >= 200000) {
+	if (*frequency >= 200000) {
 		speed = U2C_I2C_SPEED_FAST;
-		frequency = U2C_I2C_FREQ_FAST;
-	} else if (frequency >= 100000 || frequency == 0) {
+		*frequency = U2C_I2C_FREQ_FAST;
+	} else if (*frequency >= 100000 || *frequency == 0) {
 		speed = U2C_I2C_SPEED_STD;
-		frequency = U2C_I2C_FREQ_STD;
+		*frequency = U2C_I2C_FREQ_STD;
 	} else {
-		speed = U2C_I2C_SPEED(frequency);
+		speed = U2C_I2C_SPEED(*frequency);
 		if (speed > U2C_I2C_SPEED_2KHZ)
 			speed = U2C_I2C_SPEED_2KHZ;
-		frequency = U2C_I2C_FREQ(speed);
+		*frequency = U2C_I2C_FREQ(speed);
 	}
 
-	dev_info(&dev->interface->dev,
-		 "Diolan U2C at USB bus %03d address %03d speed %d Hz\n",
-		 dev->usb_dev->bus->busnum, dev->usb_dev->devnum, frequency);
-
-	diolan_flush_input(dev);
-	diolan_fw_version(dev);
-	diolan_get_serial(dev);
-
 	/* Set I2C speed */
 	ret = diolan_set_speed(dev, speed);
 	if (ret < 0)
@@ -336,6 +332,23 @@ static int diolan_init(struct i2c_diolan_u2c *dev)
 	if (speed != U2C_I2C_SPEED_FAST)
 		ret = diolan_set_clock_synch_timeout(dev, DIOLAN_SYNC_TIMEOUT);
 
+	return 0;
+}
+
+static int diolan_init(struct i2c_diolan_u2c *dev)
+{
+	int ret;
+
+	diolan_flush_input(dev);
+	diolan_fw_version(dev);
+	diolan_get_serial(dev);
+
+	ret = diolan_set_freq(&dev->adapter, &frequency);
+
+	dev_info(&dev->interface->dev,
+		 "Diolan U2C at USB bus %03d address %03d speed %d Hz\n",
+		 dev->usb_dev->bus->busnum, dev->usb_dev->devnum, frequency);
+
 	return ret;
 }
 
@@ -471,6 +484,9 @@ static int diolan_u2c_probe(struct usb_interface *interface,
 	dev->adapter.owner = THIS_MODULE;
 	dev->adapter.class = I2C_CLASS_HWMON;
 	dev->adapter.algo = &diolan_usb_algorithm;
+	dev->adapter.min_freq = U2C_I2C_MIN_FREQ;
+	dev->adapter.max_freq = U2C_I2C_MAX_FREQ;
+	dev->adapter.set_freq = diolan_set_freq;
 	i2c_set_adapdata(&dev->adapter, dev);
 	snprintf(dev->adapter.name, sizeof(dev->adapter.name),
 		 DRIVER_NAME " at bus %03d device %03d",
@@ -485,6 +501,9 @@ static int diolan_u2c_probe(struct usb_interface *interface,
 		goto error_free;
 	}
 
+	/* set the current bus frequency */
+	dev->adapter.freq = frequency;
+
 	/* and finally attach to i2c layer */
 	ret = i2c_add_adapter(&dev->adapter);
 	if (ret < 0) {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATH v3 0/4] i2c: show and change bus frequency via sysfs
  2014-10-15 20:03 [PATH v3 0/4] i2c: show and change bus frequency via sysfs Octavian Purdila
                   ` (3 preceding siblings ...)
  2014-10-15 20:03 ` [PATH v3 4/4] i2c: i2c-diolan-u2c: sysfs bus frequency support Octavian Purdila
@ 2014-10-16  6:53 ` Wolfram Sang
  4 siblings, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2014-10-16  6:53 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

On Wed, Oct 15, 2014 at 11:03:27PM +0300, Octavian Purdila wrote:
> This patch series adds support to show and change the bus frequency
> via sysfs, by exposing files to show the minimum, maximum and current
> frequency as well as allowing the frequency to be changed. This allows
> the user to view or change the bus frequency on a per bus level.

To give you some early feedback.

Patches 1-2 are nice, will review them somewhen, no general issues here.
For patch 3, I am not convinced that sysfs is the right mechanism. Maybe
configfs, yet I have never dealt with it, so far. So, looking into this
might take more time since I have other core changes pending before
that.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATH v3 4/4] i2c: i2c-diolan-u2c: sysfs bus frequency support
  2014-10-15 20:03 ` [PATH v3 4/4] i2c: i2c-diolan-u2c: sysfs bus frequency support Octavian Purdila
@ 2014-10-16 13:24   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2014-10-16 13:24 UTC (permalink / raw)
  To: Octavian Purdila, wsa; +Cc: johan, linux-i2c, linux-api, linux-kernel

On 10/15/2014 01:03 PM, Octavian Purdila wrote:
> Add support for showing and changing the bus frequency via
> sysfs.
>
> Tested on a DLN2 adapter run in U2C-12 compatibility mode.
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
>   drivers/i2c/busses/i2c-diolan-u2c.c | 49 +++++++++++++++++++++++++------------
>   1 file changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
> index b19a310..ff4e120 100644
> --- a/drivers/i2c/busses/i2c-diolan-u2c.c
> +++ b/drivers/i2c/busses/i2c-diolan-u2c.c
> @@ -71,6 +71,9 @@
>   #define U2C_I2C_FREQ_STD	100000
>   #define U2C_I2C_FREQ(s)		(1000000 / (2 * (s - 1) + 10))
>
> +#define U2C_I2C_MIN_FREQ	U2C_I2C_FREQ(U2C_I2C_SPEED_2KHZ)
> +#define U2C_I2C_MAX_FREQ	U2C_I2C_FREQ_FAST
> +
>   #define DIOLAN_USB_TIMEOUT	100	/* in ms */
>   #define DIOLAN_SYNC_TIMEOUT	20	/* in ms */
>
> @@ -298,31 +301,24 @@ static void diolan_get_serial(struct i2c_diolan_u2c *dev)
>   	}
>   }
>
> -static int diolan_init(struct i2c_diolan_u2c *dev)
> +static int diolan_set_freq(struct i2c_adapter *adapter, unsigned int *frequency)

I really dislike this kind of side-effect programming, where a function
named as _set changes one of its parameters. Not my call to make here,
though, so if the i2c maintainers are fine with it

Acked-by: Guenter Roeck <linux@roeck-us.net>

>   {
> +	struct i2c_diolan_u2c *dev = i2c_get_adapdata(adapter);
>   	int speed, ret;
>
> -	if (frequency >= 200000) {
> +	if (*frequency >= 200000) {
>   		speed = U2C_I2C_SPEED_FAST;
> -		frequency = U2C_I2C_FREQ_FAST;
> -	} else if (frequency >= 100000 || frequency == 0) {
> +		*frequency = U2C_I2C_FREQ_FAST;
> +	} else if (*frequency >= 100000 || *frequency == 0) {
>   		speed = U2C_I2C_SPEED_STD;
> -		frequency = U2C_I2C_FREQ_STD;
> +		*frequency = U2C_I2C_FREQ_STD;
>   	} else {
> -		speed = U2C_I2C_SPEED(frequency);
> +		speed = U2C_I2C_SPEED(*frequency);
>   		if (speed > U2C_I2C_SPEED_2KHZ)
>   			speed = U2C_I2C_SPEED_2KHZ;
> -		frequency = U2C_I2C_FREQ(speed);
> +		*frequency = U2C_I2C_FREQ(speed);
>   	}
>
> -	dev_info(&dev->interface->dev,
> -		 "Diolan U2C at USB bus %03d address %03d speed %d Hz\n",
> -		 dev->usb_dev->bus->busnum, dev->usb_dev->devnum, frequency);
> -
> -	diolan_flush_input(dev);
> -	diolan_fw_version(dev);
> -	diolan_get_serial(dev);
> -
>   	/* Set I2C speed */
>   	ret = diolan_set_speed(dev, speed);
>   	if (ret < 0)
> @@ -336,6 +332,23 @@ static int diolan_init(struct i2c_diolan_u2c *dev)
>   	if (speed != U2C_I2C_SPEED_FAST)
>   		ret = diolan_set_clock_synch_timeout(dev, DIOLAN_SYNC_TIMEOUT);
>
> +	return 0;
> +}
> +
> +static int diolan_init(struct i2c_diolan_u2c *dev)
> +{
> +	int ret;
> +
> +	diolan_flush_input(dev);
> +	diolan_fw_version(dev);
> +	diolan_get_serial(dev);
> +
> +	ret = diolan_set_freq(&dev->adapter, &frequency);
> +
> +	dev_info(&dev->interface->dev,
> +		 "Diolan U2C at USB bus %03d address %03d speed %d Hz\n",
> +		 dev->usb_dev->bus->busnum, dev->usb_dev->devnum, frequency);
> +
>   	return ret;
>   }
>
> @@ -471,6 +484,9 @@ static int diolan_u2c_probe(struct usb_interface *interface,
>   	dev->adapter.owner = THIS_MODULE;
>   	dev->adapter.class = I2C_CLASS_HWMON;
>   	dev->adapter.algo = &diolan_usb_algorithm;
> +	dev->adapter.min_freq = U2C_I2C_MIN_FREQ;
> +	dev->adapter.max_freq = U2C_I2C_MAX_FREQ;
> +	dev->adapter.set_freq = diolan_set_freq;
>   	i2c_set_adapdata(&dev->adapter, dev);
>   	snprintf(dev->adapter.name, sizeof(dev->adapter.name),
>   		 DRIVER_NAME " at bus %03d device %03d",
> @@ -485,6 +501,9 @@ static int diolan_u2c_probe(struct usb_interface *interface,
>   		goto error_free;
>   	}
>
> +	/* set the current bus frequency */
> +	dev->adapter.freq = frequency;
> +
>   	/* and finally attach to i2c layer */
>   	ret = i2c_add_adapter(&dev->adapter);
>   	if (ret < 0) {
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-10-16 13:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-15 20:03 [PATH v3 0/4] i2c: show and change bus frequency via sysfs Octavian Purdila
2014-10-15 20:03 ` [PATH v3 1/4] i2c: document the existing i2c sysfs ABI Octavian Purdila
2014-10-15 20:03 ` [PATH v3 2/4] i2c: document struct i2c_adapter Octavian Purdila
2014-10-15 20:03 ` [PATH v3 3/4] i2c: show and change bus frequency via sysfs Octavian Purdila
2014-10-15 20:03 ` [PATH v3 4/4] i2c: i2c-diolan-u2c: sysfs bus frequency support Octavian Purdila
2014-10-16 13:24   ` Guenter Roeck
2014-10-16  6:53 ` [PATH v3 0/4] i2c: show and change bus frequency via sysfs Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).