* [PATCH v6 0/3] TS-5500 platform support
@ 2012-04-13 0:28 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij
Hi,
This patchset brings support for the Technologic Systems TS-5500 platform.
The code is rebased on v3.4-rc2.
The first patch adds support to hwmon for the Maxim MAX197 A/D converter,
which is embedded on the TS-5500 platform, as suggested by Guenter Roeck.
The second patch adds basic support for the platform.
The third patch adds support for the board GPIO interface.
According to comments from Alan Cox and Mark Brown, the driver has been
reworked to use libgpio and moved to the GPIO subsystem.
-Vivien
Jerome Oufella (1):
gpio: TS-5500 GPIO support
Vivien Didelot (2):
hwmon: Maxim MAX197 support
x86/platform: TS-5500 basic platform support
Documentation/ABI/testing/sysfs-platform-ts5500 | 46 +++
Documentation/hwmon/max197 | 59 ++++
MAINTAINERS | 7 +
arch/x86/Kconfig | 8 +
arch/x86/include/asm/ts5500.h | 62 ++++
arch/x86/platform/Makefile | 1 +
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500.c | 400 +++++++++++++++++++++++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ts5500.c | 347 ++++++++++++++++++++
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max197.c | 398 ++++++++++++++++++++++
include/linux/max197.h | 20 ++
15 files changed, 1367 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-ts5500
create mode 100644 Documentation/hwmon/max197
create mode 100644 arch/x86/include/asm/ts5500.h
create mode 100644 arch/x86/platform/ts5500/Makefile
create mode 100644 arch/x86/platform/ts5500/ts5500.c
create mode 100644 drivers/gpio/gpio-ts5500.c
create mode 100644 drivers/hwmon/max197.c
create mode 100644 include/linux/max197.h
--
1.7.9.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* [lm-sensors] [PATCH v6 0/3] TS-5500 platform support
@ 2012-04-13 0:28 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij
Hi,
This patchset brings support for the Technologic Systems TS-5500 platform.
The code is rebased on v3.4-rc2.
The first patch adds support to hwmon for the Maxim MAX197 A/D converter,
which is embedded on the TS-5500 platform, as suggested by Guenter Roeck.
The second patch adds basic support for the platform.
The third patch adds support for the board GPIO interface.
According to comments from Alan Cox and Mark Brown, the driver has been
reworked to use libgpio and moved to the GPIO subsystem.
-Vivien
Jerome Oufella (1):
gpio: TS-5500 GPIO support
Vivien Didelot (2):
hwmon: Maxim MAX197 support
x86/platform: TS-5500 basic platform support
Documentation/ABI/testing/sysfs-platform-ts5500 | 46 +++
Documentation/hwmon/max197 | 59 ++++
MAINTAINERS | 7 +
arch/x86/Kconfig | 8 +
arch/x86/include/asm/ts5500.h | 62 ++++
arch/x86/platform/Makefile | 1 +
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500.c | 400 +++++++++++++++++++++++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ts5500.c | 347 ++++++++++++++++++++
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max197.c | 398 ++++++++++++++++++++++
include/linux/max197.h | 20 ++
15 files changed, 1367 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-ts5500
create mode 100644 Documentation/hwmon/max197
create mode 100644 arch/x86/include/asm/ts5500.h
create mode 100644 arch/x86/platform/ts5500/Makefile
create mode 100644 arch/x86/platform/ts5500/ts5500.c
create mode 100644 drivers/gpio/gpio-ts5500.c
create mode 100644 drivers/hwmon/max197.c
create mode 100644 include/linux/max197.h
--
1.7.9.2
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 1/3] hwmon: Maxim MAX197 support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-04-13 0:28 ` Vivien Didelot
-1 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
Documentation/hwmon/max197 | 59 +++++++
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max197.c | 398 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/max197.h | 20 +++
5 files changed, 487 insertions(+)
create mode 100644 Documentation/hwmon/max197
create mode 100644 drivers/hwmon/max197.c
create mode 100644 include/linux/max197.h
diff --git a/Documentation/hwmon/max197 b/Documentation/hwmon/max197
new file mode 100644
index 0000000..318f6d1
--- /dev/null
+++ b/Documentation/hwmon/max197
@@ -0,0 +1,59 @@
+Maxim MAX197 driver
+===================
+
+Author:
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+
+Supported chips:
+ * Maxim MAX197
+ Prefix: 'max197'
+ Datasheet: http://datasheets.maxim-ic.com/en/ds/MAX197.pdf
+
+ * Maxim MAX199
+ Prefix: 'max199'
+ Datasheet: http://datasheets.maxim-ic.com/en/ds/MAX199.pdf
+
+Description
+-----------
+
+The A/D converters MAX197 and MAX199 are both 8-Channel, Multi-Range, 5V,
+12-Bit DAS with 8+4 Bus Interface and Fault Protection.
+
+The available ranges for the MAX197 are {0,-5V} to 5V, and {0,-10V} to 10V,
+while they are {0,-2V} to 2V, and {0,-4V} to 4V on the MAX199.
+
+Platform data
+-------------
+
+The MAX197 platform data (defined in linux/max197.h) should be filled with a
+pointer to a conversion function, defined like:
+
+ int convert(u8 ctrl, u16 *raw);
+
+ctrl is the control byte to write to start a new conversion, and raw is a
+pointer to the raw value read from the chip.
+
+Control byte format:
+
+Bit Name Description
+7,6 PD1,PD0 Clock and Power-Down modes
+5 ACQMOD Internal or External Controlled Acquisition
+4 RNG Full-scale voltage magnitude at the input
+3 BIP Unipolar or Bipolar conversion mode
+2,1,0 A2,A1,A0 Channel
+
+Sysfs interface
+---------------
+
+* in[0-7]_input: The conversion value for the corresponding channel.
+ RO
+
+* in[0-7]_min: The lower limit (in mV) for the corresponding channel.
+ For the MAX197, it will be adjusted to -10000, -5000, or 0.
+ For the MAX199, it will be adjusted to -4000, -2000, or 0.
+ RW
+
+* in[0-7]_max: The higher limit (in mV) for the corresponding channel.
+ For the MAX197, it will be adjusted to 0, 5000, or 10000.
+ For the MAX199, it will be adjusted to 0, 2000, or 4000.
+ RW
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8deedc1..f5eefe6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1376,6 +1376,15 @@ config SENSORS_MC13783_ADC
help
Support for the A/D converter on MC13783 and MC13892 PMIC.
+config SENSORS_MAX197
+ tristate "Maxim MAX197 and compatibles."
+ help
+ If you say yes here you get support for the Maxim MAX197,
+ and MAX199 A/D converters.
+
+ This driver can also be built as a module. If so, the module
+ will be called max197.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 6d3f11f..a69b8c6 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -126,6 +126,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_MAX197) += max197.o
obj-$(CONFIG_PMBUS) += pmbus/
diff --git a/drivers/hwmon/max197.c b/drivers/hwmon/max197.c
new file mode 100644
index 0000000..e2317da
--- /dev/null
+++ b/drivers/hwmon/max197.c
@@ -0,0 +1,398 @@
+/*
+ * Maxim MAX197 A/D Converter Driver
+ *
+ * Copyright (c) 2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * For further information, see the Documentation/hwmon/max197 file.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/sysfs.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/max197.h>
+
+#define MAX199_LIMIT 4000 /* 4V */
+#define MAX197_LIMIT 10000 /* 10V */
+
+#define MAX197_NUM_CH 8 /* 8 Analog Input Channels */
+
+/* Control byte format */
+#define MAX197_A0 0x01 /* Channel bit 0 */
+#define MAX197_A1 0x02 /* Channel bit 1 */
+#define MAX197_A2 0x04 /* Channel bit 2 */
+#define MAX197_BIP 0x08 /* Bipolarity */
+#define MAX197_RNG 0x10 /* Full range */
+#define MAX197_ACQMOD 0x20 /* Internal/External controlled acquisition */
+#define MAX197_PD0 0x40 /* Clock/Power-Down modes bit 1 */
+#define MAX197_PD1 0x80 /* Clock/Power-Down modes bit 2 */
+
+#define MAX197_SCALE 12207 /* Scale coefficient for raw data */
+
+/**
+ * struct max197_chip - device instance specific data
+ * @pdata: Platform data.
+ * @hwmon_dev: The hwmon device.
+ * @lock: Read/Write mutex.
+ * @limit: Max range value (10V for MAX197, 4V for MAX199).
+ * @scale: Need to scale.
+ * @ctrl_bytes: Channels control byte.
+ */
+struct max197_chip {
+ struct max197_platform_data *pdata;
+ struct device *hwmon_dev;
+ struct mutex lock;
+ int limit;
+ bool scale;
+ u8 ctrl_bytes[MAX197_NUM_CH];
+};
+
+static inline void max197_set_unipolarity(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] &= ~MAX197_BIP;
+}
+
+static inline void max197_set_bipolarity(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] |= MAX197_BIP;
+}
+
+static inline void max197_set_half_range(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] &= ~MAX197_RNG;
+}
+
+static inline void max197_set_full_range(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] |= MAX197_RNG;
+}
+
+static inline bool max197_is_bipolar(struct max197_chip *chip, int channel)
+{
+ return chip->ctrl_bytes[channel] & MAX197_BIP;
+}
+
+static inline bool max197_is_full_range(struct max197_chip *chip, int channel)
+{
+ return chip->ctrl_bytes[channel] & MAX197_RNG;
+}
+
+/**
+ * max197_show_range() - Display range on user output
+ *
+ * Function called on read access on in{0,1,2,3,4,5,6,7}_{min,max}
+ */
+static ssize_t max197_show_range(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct max197_chip *chip = dev_get_drvdata(dev);
+ struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+ int channel = attr->index;
+ bool is_min = attr->nr;
+ int range;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ range = max197_is_full_range(chip, channel) ?
+ chip->limit : chip->limit / 2;
+ if (is_min) {
+ if (max197_is_bipolar(chip, channel))
+ range = -range;
+ else
+ range = 0;
+ }
+
+ mutex_unlock(&chip->lock);
+
+ return sprintf(buf, "%d\n", range);
+}
+
+/**
+ * max197_store_range() - Write range from user input
+ *
+ * Function called on write access on in{0,1,2,3,4,5,6,7}_{min,max}
+ */
+static ssize_t max197_store_range(struct device *dev,
+ struct device_attribute *devattr,
+ const char *buf, size_t count)
+{
+ struct max197_chip *chip = dev_get_drvdata(dev);
+ struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+ int channel = attr->index;
+ bool is_min = attr->nr;
+ long value;
+ int half = chip->limit / 2;
+ int full = chip->limit;
+
+ if (kstrtol(buf, 10, &value))
+ return -EINVAL;
+
+ if (is_min) {
+ if (value <= -full)
+ value = -full;
+ else if (value < 0)
+ value = -half;
+ else
+ value = 0;
+ } else {
+ if (value >= full)
+ value = full;
+ else
+ value = half;
+ }
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ if (value == 0) {
+ /* We can deduce only the polarity */
+ max197_set_unipolarity(chip, channel);
+ } else if (value == -half) {
+ max197_set_bipolarity(chip, channel);
+ max197_set_half_range(chip, channel);
+ } else if (value == -full) {
+ max197_set_bipolarity(chip, channel);
+ max197_set_full_range(chip, channel);
+ } else if (value == half) {
+ /* We can deduce only the range */
+ max197_set_half_range(chip, channel);
+ } else if (value == full) {
+ /* We can deduce only the range */
+ max197_set_full_range(chip, channel);
+ }
+
+ mutex_unlock(&chip->lock);
+
+ return count;
+}
+
+/**
+ * max197_show_input() - Show channel input
+ *
+ * Function called on read access on in{0,1,2,3,4,5,6,7}_input
+ */
+static ssize_t max197_show_input(struct device *dev,
+ struct device_attribute *devattr,
+ char *buf)
+{
+ struct max197_chip *chip = dev_get_drvdata(dev);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+ int channel = attr->index;
+ u16 raw;
+ s32 scaled;
+ int ret;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ ret = chip->pdata->convert(chip->ctrl_bytes[channel], &raw);
+ if (ret) {
+ dev_err(dev, "conversion failed\n");
+ goto unlock;
+ }
+
+ /*
+ * Coefficient to apply on raw value.
+ * See Table 1. Full Scale and Zero Scale in the MAX197 datasheet.
+ */
+ scaled = raw;
+ if (chip->scale) {
+ scaled *= MAX197_SCALE;
+ if (max197_is_full_range(chip, channel))
+ scaled *= 2;
+ scaled /= 10000;
+ }
+
+ ret = sprintf(buf, "%d\n", scaled);
+
+unlock:
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
+static ssize_t max197_show_name(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ return sprintf(buf, "%s\n", pdev->name);
+}
+
+#define MAX197_SENSOR_DEVICE_ATTR_CH(chan) \
+ static SENSOR_DEVICE_ATTR(in##chan##_input, S_IRUGO, \
+ max197_show_input, NULL, chan); \
+ static SENSOR_DEVICE_ATTR_2(in##chan##_min, S_IRUGO | S_IWUSR, \
+ max197_show_range, \
+ max197_store_range, \
+ true, chan); \
+ static SENSOR_DEVICE_ATTR_2(in##chan##_max, S_IRUGO | S_IWUSR, \
+ max197_show_range, \
+ max197_store_range, \
+ false, chan)
+
+#define MAX197_SENSOR_DEV_ATTR_IN(chan) \
+ &sensor_dev_attr_in##chan##_input.dev_attr.attr, \
+ &sensor_dev_attr_in##chan##_max.dev_attr.attr, \
+ &sensor_dev_attr_in##chan##_min.dev_attr.attr
+
+static DEVICE_ATTR(name, S_IRUGO, max197_show_name, NULL);
+
+MAX197_SENSOR_DEVICE_ATTR_CH(0);
+MAX197_SENSOR_DEVICE_ATTR_CH(1);
+MAX197_SENSOR_DEVICE_ATTR_CH(2);
+MAX197_SENSOR_DEVICE_ATTR_CH(3);
+MAX197_SENSOR_DEVICE_ATTR_CH(4);
+MAX197_SENSOR_DEVICE_ATTR_CH(5);
+MAX197_SENSOR_DEVICE_ATTR_CH(6);
+MAX197_SENSOR_DEVICE_ATTR_CH(7);
+
+static const struct attribute_group max197_sysfs_group = {
+ .attrs = (struct attribute *[]) {
+ &dev_attr_name.attr,
+ MAX197_SENSOR_DEV_ATTR_IN(0),
+ MAX197_SENSOR_DEV_ATTR_IN(1),
+ MAX197_SENSOR_DEV_ATTR_IN(2),
+ MAX197_SENSOR_DEV_ATTR_IN(3),
+ MAX197_SENSOR_DEV_ATTR_IN(4),
+ MAX197_SENSOR_DEV_ATTR_IN(5),
+ MAX197_SENSOR_DEV_ATTR_IN(6),
+ MAX197_SENSOR_DEV_ATTR_IN(7),
+ NULL
+ }
+};
+
+static int __devinit max197_probe(struct platform_device *pdev)
+{
+ struct max197_chip *chip = kzalloc(sizeof *chip, GFP_KERNEL);
+ int ch, ret;
+
+ if (chip == NULL) {
+ ret = -ENOMEM;
+ goto error_ret;
+ }
+ platform_set_drvdata(pdev, chip);
+
+ mutex_init(&chip->lock);
+
+ if (pdev->dev.platform_data == NULL) {
+ dev_err(&pdev->dev, "no platform data supplied\n");
+ ret = -EINVAL;
+ goto error_free_chip;
+ }
+ chip->pdata = pdev->dev.platform_data;
+
+ if (chip->pdata->convert == NULL) {
+ dev_err(&pdev->dev, "no convert function supplied\n");
+ ret = -EINVAL;
+ goto error_free_chip;
+ }
+
+ if (strcmp("max197", pdev->name) == 0) {
+ chip->limit = MAX197_LIMIT;
+ chip->scale = true;
+ } else {
+ chip->limit = MAX199_LIMIT;
+ chip->scale = false;
+ }
+
+ for (ch = 0; ch < MAX197_NUM_CH; ch++)
+ chip->ctrl_bytes[ch] = (u8) ch;
+
+ ret = sysfs_create_group(&pdev->dev.kobj, &max197_sysfs_group);
+ if (ret) {
+ dev_err(&pdev->dev, "sysfs create group failed\n");
+ goto error_free_chip;
+ }
+
+ chip->hwmon_dev = hwmon_device_register(&pdev->dev);
+ if (IS_ERR(chip->hwmon_dev)) {
+ dev_err(&pdev->dev, "hwmon device register failed\n");
+ ret = PTR_ERR(chip->hwmon_dev);
+ goto error_release_sysfs_group;
+ }
+
+ return 0;
+
+error_release_sysfs_group:
+ sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
+error_free_chip:
+ kfree(chip);
+error_ret:
+ return ret;
+}
+
+static int __devexit max197_remove(struct platform_device *pdev)
+{
+ struct max197_chip *chip = platform_get_drvdata(pdev);
+
+ hwmon_device_unregister(chip->hwmon_dev);
+ sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
+ kfree(chip);
+
+ return 0;
+}
+
+static struct platform_driver __refdata maxim_drivers[] = {
+ {
+ .driver = {
+ .name = "max197",
+ .owner = THIS_MODULE,
+ },
+ .probe = max197_probe,
+ .remove = __devexit_p(max197_remove),
+ }, {
+ .driver = {
+ .name = "max199",
+ .owner = THIS_MODULE,
+ },
+ .probe = max197_probe,
+ .remove = __devexit_p(max197_remove),
+ }
+};
+
+static int __init max197_init(void)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(maxim_drivers); i++) {
+ ret = platform_driver_register(&maxim_drivers[i]);
+ if (ret)
+ goto error_unregister;
+ }
+
+ return 0;
+
+error_unregister:
+ while (--i >= 0)
+ platform_driver_unregister(&maxim_drivers[i]);
+
+ return ret;
+}
+module_init(max197_init);
+
+static void __exit max197_exit(void)
+{
+ int i;
+ for (i = ARRAY_SIZE(maxim_drivers) - 1; i >= 0; i--)
+ platform_driver_unregister(&maxim_drivers[i]);
+}
+module_exit(max197_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>");
+MODULE_DESCRIPTION("Maxim MAX197 ADC driver");
diff --git a/include/linux/max197.h b/include/linux/max197.h
new file mode 100644
index 0000000..0ed5399
--- /dev/null
+++ b/include/linux/max197.h
@@ -0,0 +1,20 @@
+/*
+ * Maxim MAX197 A/D Converter Driver
+ *
+ * Copyright (c) 2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * For further information, see the Documentation/hwmon/max197 file.
+ */
+
+/**
+ * struct max197_platform_data - MAX197 connectivity info
+ * @convert: start a conversion, and read the raw data from the chip.
+ */
+struct max197_platform_data {
+ int (*convert)(u8 ctrl, u16 *raw);
+};
--
1.7.9.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [lm-sensors] [PATCH v6 1/3] hwmon: Maxim MAX197 support
@ 2012-04-13 0:28 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
Documentation/hwmon/max197 | 59 +++++++
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max197.c | 398 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/max197.h | 20 +++
5 files changed, 487 insertions(+)
create mode 100644 Documentation/hwmon/max197
create mode 100644 drivers/hwmon/max197.c
create mode 100644 include/linux/max197.h
diff --git a/Documentation/hwmon/max197 b/Documentation/hwmon/max197
new file mode 100644
index 0000000..318f6d1
--- /dev/null
+++ b/Documentation/hwmon/max197
@@ -0,0 +1,59 @@
+Maxim MAX197 driver
+=========+
+Author:
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+
+Supported chips:
+ * Maxim MAX197
+ Prefix: 'max197'
+ Datasheet: http://datasheets.maxim-ic.com/en/ds/MAX197.pdf
+
+ * Maxim MAX199
+ Prefix: 'max199'
+ Datasheet: http://datasheets.maxim-ic.com/en/ds/MAX199.pdf
+
+Description
+-----------
+
+The A/D converters MAX197 and MAX199 are both 8-Channel, Multi-Range, 5V,
+12-Bit DAS with 8+4 Bus Interface and Fault Protection.
+
+The available ranges for the MAX197 are {0,-5V} to 5V, and {0,-10V} to 10V,
+while they are {0,-2V} to 2V, and {0,-4V} to 4V on the MAX199.
+
+Platform data
+-------------
+
+The MAX197 platform data (defined in linux/max197.h) should be filled with a
+pointer to a conversion function, defined like:
+
+ int convert(u8 ctrl, u16 *raw);
+
+ctrl is the control byte to write to start a new conversion, and raw is a
+pointer to the raw value read from the chip.
+
+Control byte format:
+
+Bit Name Description
+7,6 PD1,PD0 Clock and Power-Down modes
+5 ACQMOD Internal or External Controlled Acquisition
+4 RNG Full-scale voltage magnitude at the input
+3 BIP Unipolar or Bipolar conversion mode
+2,1,0 A2,A1,A0 Channel
+
+Sysfs interface
+---------------
+
+* in[0-7]_input: The conversion value for the corresponding channel.
+ RO
+
+* in[0-7]_min: The lower limit (in mV) for the corresponding channel.
+ For the MAX197, it will be adjusted to -10000, -5000, or 0.
+ For the MAX199, it will be adjusted to -4000, -2000, or 0.
+ RW
+
+* in[0-7]_max: The higher limit (in mV) for the corresponding channel.
+ For the MAX197, it will be adjusted to 0, 5000, or 10000.
+ For the MAX199, it will be adjusted to 0, 2000, or 4000.
+ RW
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8deedc1..f5eefe6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1376,6 +1376,15 @@ config SENSORS_MC13783_ADC
help
Support for the A/D converter on MC13783 and MC13892 PMIC.
+config SENSORS_MAX197
+ tristate "Maxim MAX197 and compatibles."
+ help
+ If you say yes here you get support for the Maxim MAX197,
+ and MAX199 A/D converters.
+
+ This driver can also be built as a module. If so, the module
+ will be called max197.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 6d3f11f..a69b8c6 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -126,6 +126,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_MAX197) += max197.o
obj-$(CONFIG_PMBUS) += pmbus/
diff --git a/drivers/hwmon/max197.c b/drivers/hwmon/max197.c
new file mode 100644
index 0000000..e2317da
--- /dev/null
+++ b/drivers/hwmon/max197.c
@@ -0,0 +1,398 @@
+/*
+ * Maxim MAX197 A/D Converter Driver
+ *
+ * Copyright (c) 2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * For further information, see the Documentation/hwmon/max197 file.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/sysfs.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/max197.h>
+
+#define MAX199_LIMIT 4000 /* 4V */
+#define MAX197_LIMIT 10000 /* 10V */
+
+#define MAX197_NUM_CH 8 /* 8 Analog Input Channels */
+
+/* Control byte format */
+#define MAX197_A0 0x01 /* Channel bit 0 */
+#define MAX197_A1 0x02 /* Channel bit 1 */
+#define MAX197_A2 0x04 /* Channel bit 2 */
+#define MAX197_BIP 0x08 /* Bipolarity */
+#define MAX197_RNG 0x10 /* Full range */
+#define MAX197_ACQMOD 0x20 /* Internal/External controlled acquisition */
+#define MAX197_PD0 0x40 /* Clock/Power-Down modes bit 1 */
+#define MAX197_PD1 0x80 /* Clock/Power-Down modes bit 2 */
+
+#define MAX197_SCALE 12207 /* Scale coefficient for raw data */
+
+/**
+ * struct max197_chip - device instance specific data
+ * @pdata: Platform data.
+ * @hwmon_dev: The hwmon device.
+ * @lock: Read/Write mutex.
+ * @limit: Max range value (10V for MAX197, 4V for MAX199).
+ * @scale: Need to scale.
+ * @ctrl_bytes: Channels control byte.
+ */
+struct max197_chip {
+ struct max197_platform_data *pdata;
+ struct device *hwmon_dev;
+ struct mutex lock;
+ int limit;
+ bool scale;
+ u8 ctrl_bytes[MAX197_NUM_CH];
+};
+
+static inline void max197_set_unipolarity(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] &= ~MAX197_BIP;
+}
+
+static inline void max197_set_bipolarity(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] |= MAX197_BIP;
+}
+
+static inline void max197_set_half_range(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] &= ~MAX197_RNG;
+}
+
+static inline void max197_set_full_range(struct max197_chip *chip, int channel)
+{
+ chip->ctrl_bytes[channel] |= MAX197_RNG;
+}
+
+static inline bool max197_is_bipolar(struct max197_chip *chip, int channel)
+{
+ return chip->ctrl_bytes[channel] & MAX197_BIP;
+}
+
+static inline bool max197_is_full_range(struct max197_chip *chip, int channel)
+{
+ return chip->ctrl_bytes[channel] & MAX197_RNG;
+}
+
+/**
+ * max197_show_range() - Display range on user output
+ *
+ * Function called on read access on in{0,1,2,3,4,5,6,7}_{min,max}
+ */
+static ssize_t max197_show_range(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct max197_chip *chip = dev_get_drvdata(dev);
+ struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+ int channel = attr->index;
+ bool is_min = attr->nr;
+ int range;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ range = max197_is_full_range(chip, channel) ?
+ chip->limit : chip->limit / 2;
+ if (is_min) {
+ if (max197_is_bipolar(chip, channel))
+ range = -range;
+ else
+ range = 0;
+ }
+
+ mutex_unlock(&chip->lock);
+
+ return sprintf(buf, "%d\n", range);
+}
+
+/**
+ * max197_store_range() - Write range from user input
+ *
+ * Function called on write access on in{0,1,2,3,4,5,6,7}_{min,max}
+ */
+static ssize_t max197_store_range(struct device *dev,
+ struct device_attribute *devattr,
+ const char *buf, size_t count)
+{
+ struct max197_chip *chip = dev_get_drvdata(dev);
+ struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+ int channel = attr->index;
+ bool is_min = attr->nr;
+ long value;
+ int half = chip->limit / 2;
+ int full = chip->limit;
+
+ if (kstrtol(buf, 10, &value))
+ return -EINVAL;
+
+ if (is_min) {
+ if (value <= -full)
+ value = -full;
+ else if (value < 0)
+ value = -half;
+ else
+ value = 0;
+ } else {
+ if (value >= full)
+ value = full;
+ else
+ value = half;
+ }
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ if (value = 0) {
+ /* We can deduce only the polarity */
+ max197_set_unipolarity(chip, channel);
+ } else if (value = -half) {
+ max197_set_bipolarity(chip, channel);
+ max197_set_half_range(chip, channel);
+ } else if (value = -full) {
+ max197_set_bipolarity(chip, channel);
+ max197_set_full_range(chip, channel);
+ } else if (value = half) {
+ /* We can deduce only the range */
+ max197_set_half_range(chip, channel);
+ } else if (value = full) {
+ /* We can deduce only the range */
+ max197_set_full_range(chip, channel);
+ }
+
+ mutex_unlock(&chip->lock);
+
+ return count;
+}
+
+/**
+ * max197_show_input() - Show channel input
+ *
+ * Function called on read access on in{0,1,2,3,4,5,6,7}_input
+ */
+static ssize_t max197_show_input(struct device *dev,
+ struct device_attribute *devattr,
+ char *buf)
+{
+ struct max197_chip *chip = dev_get_drvdata(dev);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+ int channel = attr->index;
+ u16 raw;
+ s32 scaled;
+ int ret;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ ret = chip->pdata->convert(chip->ctrl_bytes[channel], &raw);
+ if (ret) {
+ dev_err(dev, "conversion failed\n");
+ goto unlock;
+ }
+
+ /*
+ * Coefficient to apply on raw value.
+ * See Table 1. Full Scale and Zero Scale in the MAX197 datasheet.
+ */
+ scaled = raw;
+ if (chip->scale) {
+ scaled *= MAX197_SCALE;
+ if (max197_is_full_range(chip, channel))
+ scaled *= 2;
+ scaled /= 10000;
+ }
+
+ ret = sprintf(buf, "%d\n", scaled);
+
+unlock:
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
+static ssize_t max197_show_name(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ return sprintf(buf, "%s\n", pdev->name);
+}
+
+#define MAX197_SENSOR_DEVICE_ATTR_CH(chan) \
+ static SENSOR_DEVICE_ATTR(in##chan##_input, S_IRUGO, \
+ max197_show_input, NULL, chan); \
+ static SENSOR_DEVICE_ATTR_2(in##chan##_min, S_IRUGO | S_IWUSR, \
+ max197_show_range, \
+ max197_store_range, \
+ true, chan); \
+ static SENSOR_DEVICE_ATTR_2(in##chan##_max, S_IRUGO | S_IWUSR, \
+ max197_show_range, \
+ max197_store_range, \
+ false, chan)
+
+#define MAX197_SENSOR_DEV_ATTR_IN(chan) \
+ &sensor_dev_attr_in##chan##_input.dev_attr.attr, \
+ &sensor_dev_attr_in##chan##_max.dev_attr.attr, \
+ &sensor_dev_attr_in##chan##_min.dev_attr.attr
+
+static DEVICE_ATTR(name, S_IRUGO, max197_show_name, NULL);
+
+MAX197_SENSOR_DEVICE_ATTR_CH(0);
+MAX197_SENSOR_DEVICE_ATTR_CH(1);
+MAX197_SENSOR_DEVICE_ATTR_CH(2);
+MAX197_SENSOR_DEVICE_ATTR_CH(3);
+MAX197_SENSOR_DEVICE_ATTR_CH(4);
+MAX197_SENSOR_DEVICE_ATTR_CH(5);
+MAX197_SENSOR_DEVICE_ATTR_CH(6);
+MAX197_SENSOR_DEVICE_ATTR_CH(7);
+
+static const struct attribute_group max197_sysfs_group = {
+ .attrs = (struct attribute *[]) {
+ &dev_attr_name.attr,
+ MAX197_SENSOR_DEV_ATTR_IN(0),
+ MAX197_SENSOR_DEV_ATTR_IN(1),
+ MAX197_SENSOR_DEV_ATTR_IN(2),
+ MAX197_SENSOR_DEV_ATTR_IN(3),
+ MAX197_SENSOR_DEV_ATTR_IN(4),
+ MAX197_SENSOR_DEV_ATTR_IN(5),
+ MAX197_SENSOR_DEV_ATTR_IN(6),
+ MAX197_SENSOR_DEV_ATTR_IN(7),
+ NULL
+ }
+};
+
+static int __devinit max197_probe(struct platform_device *pdev)
+{
+ struct max197_chip *chip = kzalloc(sizeof *chip, GFP_KERNEL);
+ int ch, ret;
+
+ if (chip = NULL) {
+ ret = -ENOMEM;
+ goto error_ret;
+ }
+ platform_set_drvdata(pdev, chip);
+
+ mutex_init(&chip->lock);
+
+ if (pdev->dev.platform_data = NULL) {
+ dev_err(&pdev->dev, "no platform data supplied\n");
+ ret = -EINVAL;
+ goto error_free_chip;
+ }
+ chip->pdata = pdev->dev.platform_data;
+
+ if (chip->pdata->convert = NULL) {
+ dev_err(&pdev->dev, "no convert function supplied\n");
+ ret = -EINVAL;
+ goto error_free_chip;
+ }
+
+ if (strcmp("max197", pdev->name) = 0) {
+ chip->limit = MAX197_LIMIT;
+ chip->scale = true;
+ } else {
+ chip->limit = MAX199_LIMIT;
+ chip->scale = false;
+ }
+
+ for (ch = 0; ch < MAX197_NUM_CH; ch++)
+ chip->ctrl_bytes[ch] = (u8) ch;
+
+ ret = sysfs_create_group(&pdev->dev.kobj, &max197_sysfs_group);
+ if (ret) {
+ dev_err(&pdev->dev, "sysfs create group failed\n");
+ goto error_free_chip;
+ }
+
+ chip->hwmon_dev = hwmon_device_register(&pdev->dev);
+ if (IS_ERR(chip->hwmon_dev)) {
+ dev_err(&pdev->dev, "hwmon device register failed\n");
+ ret = PTR_ERR(chip->hwmon_dev);
+ goto error_release_sysfs_group;
+ }
+
+ return 0;
+
+error_release_sysfs_group:
+ sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
+error_free_chip:
+ kfree(chip);
+error_ret:
+ return ret;
+}
+
+static int __devexit max197_remove(struct platform_device *pdev)
+{
+ struct max197_chip *chip = platform_get_drvdata(pdev);
+
+ hwmon_device_unregister(chip->hwmon_dev);
+ sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
+ kfree(chip);
+
+ return 0;
+}
+
+static struct platform_driver __refdata maxim_drivers[] = {
+ {
+ .driver = {
+ .name = "max197",
+ .owner = THIS_MODULE,
+ },
+ .probe = max197_probe,
+ .remove = __devexit_p(max197_remove),
+ }, {
+ .driver = {
+ .name = "max199",
+ .owner = THIS_MODULE,
+ },
+ .probe = max197_probe,
+ .remove = __devexit_p(max197_remove),
+ }
+};
+
+static int __init max197_init(void)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(maxim_drivers); i++) {
+ ret = platform_driver_register(&maxim_drivers[i]);
+ if (ret)
+ goto error_unregister;
+ }
+
+ return 0;
+
+error_unregister:
+ while (--i >= 0)
+ platform_driver_unregister(&maxim_drivers[i]);
+
+ return ret;
+}
+module_init(max197_init);
+
+static void __exit max197_exit(void)
+{
+ int i;
+ for (i = ARRAY_SIZE(maxim_drivers) - 1; i >= 0; i--)
+ platform_driver_unregister(&maxim_drivers[i]);
+}
+module_exit(max197_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>");
+MODULE_DESCRIPTION("Maxim MAX197 ADC driver");
diff --git a/include/linux/max197.h b/include/linux/max197.h
new file mode 100644
index 0000000..0ed5399
--- /dev/null
+++ b/include/linux/max197.h
@@ -0,0 +1,20 @@
+/*
+ * Maxim MAX197 A/D Converter Driver
+ *
+ * Copyright (c) 2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * For further information, see the Documentation/hwmon/max197 file.
+ */
+
+/**
+ * struct max197_platform_data - MAX197 connectivity info
+ * @convert: start a conversion, and read the raw data from the chip.
+ */
+struct max197_platform_data {
+ int (*convert)(u8 ctrl, u16 *raw);
+};
--
1.7.9.2
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v6 2/3] x86/platform: TS-5500 basic platform support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-04-13 0:28 ` Vivien Didelot
-1 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
Documentation/ABI/testing/sysfs-platform-ts5500 | 46 +++
MAINTAINERS | 5 +
arch/x86/Kconfig | 8 +
arch/x86/platform/Makefile | 1 +
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500.c | 400 +++++++++++++++++++++++
6 files changed, 461 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-ts5500
create mode 100644 arch/x86/platform/ts5500/Makefile
create mode 100644 arch/x86/platform/ts5500/ts5500.c
diff --git a/Documentation/ABI/testing/sysfs-platform-ts5500 b/Documentation/ABI/testing/sysfs-platform-ts5500
new file mode 100644
index 0000000..bc29bb8
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-ts5500
@@ -0,0 +1,46 @@
+What: /sys/devices/platform/ts5500/id
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Product ID of the TS board. TS-5500 ID is 0x60.
+
+What: /sys/devices/platform/ts5500/sram
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if the SRAM feature is present. If it is enabled, it will
+ display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/ereset
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if an external reset is present. If it is present, it will
+ display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/jp{1,2,3,4,5,6}
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Show the state of a plugged jumper. If it is present, it will
+ display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/adc
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if the A/D Converter is present. If it is enabled,
+ it will display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/rs485
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if the RS485 feature is present. If it is enabled, it
+ will display "1", otherwise "0".
diff --git a/MAINTAINERS b/MAINTAINERS
index 2dcfca8..b126129 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6620,6 +6620,11 @@ S: Supported
F: drivers/net/team/
F: include/linux/if_team.h
+TECHNOLOGIC SYSTEMS TS-5500 MACHINE SUPPORT
+M: Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>
+S: Maintained
+F: arch/x86/platform/ts5500/
+
TEGRA SUPPORT
M: Colin Cross <ccross@android.com>
M: Olof Johansson <olof@lixom.net>
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1d14cc6..55f32b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2131,6 +2131,14 @@ config GEOS
---help---
This option enables system support for the Traverse Technologies GEOS.
+config TS5500
+ bool "Technologic Systems TS-5500 Single Board Computer support"
+ depends on MELAN
+ select CHECK_SIGNATURE
+ ---help---
+ Add support for the Technologic Systems TS-5500 SBC.
+ If you have a TS-5500 platform, say Y here.
+
endif # X86_32
config AMD_NB
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 8d87439..f50911b 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -7,5 +7,6 @@ obj-y += mrst/
obj-y += olpc/
obj-y += scx200/
obj-y += sfi/
+obj-y += ts5500/
obj-y += visws/
obj-y += uv/
diff --git a/arch/x86/platform/ts5500/Makefile b/arch/x86/platform/ts5500/Makefile
new file mode 100644
index 0000000..c54e348
--- /dev/null
+++ b/arch/x86/platform/ts5500/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TS5500) += ts5500.o
diff --git a/arch/x86/platform/ts5500/ts5500.c b/arch/x86/platform/ts5500/ts5500.c
new file mode 100644
index 0000000..96f76c0
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500.c
@@ -0,0 +1,400 @@
+/*
+ * Technologic Systems TS-5500 board - SBC info layer
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ * Alexandre Savard <alexandre.savard@savoirfairelinux.com>
+ * Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
+ *
+ * Portions originate from ts_sbcinfo.c (c) Technologic Systems
+ * Liberty Young <liberty@embeddedx86.com>
+ *
+ * These functions add sysfs platform entries to display information about
+ * the Technologic Systems TS-5500 Single Board Computer (SBC).
+ *
+ * For further information about sysfs entries, see
+ * Documentation/ABI/testing/sysfs-platform-ts5500
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <asm/processor.h>
+#include <linux/leds.h>
+#include <linux/delay.h>
+#include <linux/max197.h>
+
+/* Hardware info for pre-detection */
+#define AMD_ELAN_FAMILY 4
+#define AMD_ELAN_SC520 9
+
+/* Product code register */
+#define TS5500_PRODUCT_CODE_ADDR 0x74
+#define TS5500_PRODUCT_CODE 0x60 /* TS-5500 product code */
+
+/* SRAM/RS-485/ADC options, and RS-485 RTS/Automatic RS-485 flags register */
+#define TS5500_SRAM_RS485_ADC_ADDR 0x75
+#define TS5500_SRAM 0x01 /* SRAM option */
+#define TS5500_RS485 0x02 /* RS-485 option */
+#define TS5500_ADC 0x04 /* A/D converter option */
+#define TS5500_RS485_RTS 0x40 /* RTS for RS-485 */
+#define TS5500_RS485_AUTO 0x80 /* Automatic RS-485 */
+
+/* External Reset/Industrial Temperature Range options register */
+#define TS5500_ERESET_ITR_ADDR 0x76
+#define TS5500_ERESET 0x01 /* External Reset option */
+#define TS5500_ITR 0x02 /* Indust. Temp. Range option */
+
+/* LED/Jumpers register */
+#define TS5500_LED_JP_ADDR 0x77
+#define TS5500_LED 0x01 /* LED flag */
+#define TS5500_JP1 0x02 /* Automatic CMOS */
+#define TS5500_JP2 0x04 /* Enable Serial Console */
+#define TS5500_JP3 0x08 /* Write Enable Drive A */
+#define TS5500_JP4 0x10 /* Fast Console (115K baud) */
+#define TS5500_JP5 0x20 /* User Jumper */
+#define TS5500_JP6 0x40 /* Console on COM1 (req. JP2) */
+#define TS5500_JP7 0x80 /* Undocumented (Unused) */
+
+/* A/D Converter registers */
+#define TS5500_ADC_CONV_BUSY_ADDR 0x195 /* Conversion state register */
+#define TS5500_ADC_CONV_BUSY 0x01
+#define TS5500_ADC_CONV_INIT_LSB_ADDR 0x196 /* Start conv. / LSB register */
+#define TS5500_ADC_CONV_MSB_ADDR 0x197 /* MSB register */
+#define TS5500_ADC_CONV_DELAY 12 /* usec */
+
+/**
+ * struct ts5500_sbc - TS-5500 SBC main structure
+ * @lock: Read/Write mutex.
+ * @board_id: Board name.
+ * @sram: Check SRAM option.
+ * @rs485: Check RS-485 option.
+ * @adc: Check Analog/Digital converter option.
+ * @ereset: Check External Reset option.
+ * @itr: Check Industrial Temperature Range option.
+ * @jumpers: States of jumpers 1-7.
+ */
+struct ts5500_sbc {
+ struct mutex lock;
+ int board_id;
+ bool sram;
+ bool rs485;
+ bool adc;
+ bool ereset;
+ bool itr;
+ u8 jumpers;
+};
+
+/* Current platform */
+struct ts5500_sbc *ts5500;
+
+/**
+ * ts5500_pre_detect_hw() - check for TS-5500 specific hardware
+ */
+static int __init ts5500_pre_detect_hw(void)
+{
+ /* Check for AMD ElanSC520 Microcontroller */
+ if (cpu_info.x86_vendor != X86_VENDOR_AMD ||
+ cpu_info.x86 != AMD_ELAN_FAMILY ||
+ cpu_info.x86_model != AMD_ELAN_SC520)
+ return -ENODEV;
+
+ return 0;
+}
+
+/* BIOS signatures */
+static struct {
+ const unsigned char *string;
+ const ssize_t offset;
+} signatures[] __initdata = {
+ {"TS-5x00 AMD Elan", 0xb14}
+};
+
+/**
+ * ts5500_bios_signature() - find board signature in BIOS shadow RAM.
+ */
+static int __init ts5500_bios_signature(void)
+{
+ void __iomem *bios = ioremap(0xF0000, 0x10000);
+ int i, ret = 0;
+
+ for (i = 0; i < ARRAY_SIZE(signatures); i++)
+ if (check_signature(bios + signatures[i].offset,
+ signatures[i].string,
+ strlen(signatures[i].string)))
+ goto found;
+ else
+ pr_notice("Technologic Systems BIOS signature "
+ "'%s' not found at offset %zd\n",
+ signatures[i].string, signatures[i].offset);
+ ret = -ENODEV;
+found:
+ iounmap(bios);
+ return ret;
+}
+
+/**
+ * ts5500_detect_config() - detect the TS board
+ * @sbc: Structure in which the detected board's details will be stored.
+ */
+static int __init ts5500_detect_config(struct ts5500_sbc *sbc)
+{
+ u8 tmp;
+ int ret = 0;
+
+ if (!request_region(TS5500_PRODUCT_CODE_ADDR, 4, "ts5500"))
+ return -EBUSY;
+
+ mutex_lock(&ts5500->lock);
+ tmp = inb(TS5500_PRODUCT_CODE_ADDR);
+ if (tmp != TS5500_PRODUCT_CODE) {
+ pr_err("This platform is not a TS-5500 (found ID 0x%x)\n", tmp);
+ ret = -ENODEV;
+ goto cleanup;
+ }
+ sbc->board_id = tmp;
+
+ tmp = inb(TS5500_SRAM_RS485_ADC_ADDR);
+ ts5500->sram = tmp & TS5500_SRAM;
+ ts5500->rs485 = tmp & TS5500_RS485;
+ ts5500->adc = tmp & TS5500_ADC;
+
+ tmp = inb(TS5500_ERESET_ITR_ADDR);
+ ts5500->ereset = tmp & TS5500_ERESET;
+ ts5500->itr = tmp & TS5500_ITR;
+
+ tmp = inb(TS5500_LED_JP_ADDR);
+ sbc->jumpers = tmp & ~TS5500_LED;
+
+cleanup:
+ mutex_unlock(&ts5500->lock);
+ release_region(TS5500_PRODUCT_CODE_ADDR, 4);
+ return ret;
+}
+
+#define TS5500_IS_JP_SET(sbc, jmp) (!!(sbc->jumpers & TS5500_JP##jmp))
+
+static ssize_t ts5500_show_id(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "0x%x\n", sbc->board_id);
+}
+
+static ssize_t ts5500_show_sram(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->sram);
+}
+
+static ssize_t ts5500_show_rs485(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->rs485);
+}
+
+static ssize_t ts5500_show_adc(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->adc);
+}
+
+static ssize_t ts5500_show_ereset(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->ereset);
+}
+
+static ssize_t ts5500_show_itr(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->itr);
+}
+
+#define TS5500_SHOW_JP(jp) \
+ static ssize_t ts5500_show_jp##jp(struct device *dev, \
+ struct device_attribute *attr,\
+ char *buf) \
+ { \
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev); \
+ return sprintf(buf, "%d\n", TS5500_IS_JP_SET(sbc, jp)); \
+ }
+
+TS5500_SHOW_JP(1)
+TS5500_SHOW_JP(2)
+TS5500_SHOW_JP(3)
+TS5500_SHOW_JP(4)
+TS5500_SHOW_JP(5)
+TS5500_SHOW_JP(6)
+
+static DEVICE_ATTR(id, S_IRUGO, ts5500_show_id, NULL);
+static DEVICE_ATTR(sram, S_IRUGO, ts5500_show_sram, NULL);
+static DEVICE_ATTR(rs485, S_IRUGO, ts5500_show_rs485, NULL);
+static DEVICE_ATTR(adc, S_IRUGO, ts5500_show_adc, NULL);
+static DEVICE_ATTR(ereset, S_IRUGO, ts5500_show_ereset, NULL);
+static DEVICE_ATTR(itr, S_IRUGO, ts5500_show_itr, NULL);
+static DEVICE_ATTR(jp1, S_IRUGO, ts5500_show_jp1, NULL);
+static DEVICE_ATTR(jp2, S_IRUGO, ts5500_show_jp2, NULL);
+static DEVICE_ATTR(jp3, S_IRUGO, ts5500_show_jp3, NULL);
+static DEVICE_ATTR(jp4, S_IRUGO, ts5500_show_jp4, NULL);
+static DEVICE_ATTR(jp5, S_IRUGO, ts5500_show_jp5, NULL);
+static DEVICE_ATTR(jp6, S_IRUGO, ts5500_show_jp6, NULL);
+
+static struct attribute *ts5500_attributes[] = {
+ &dev_attr_id.attr,
+ &dev_attr_sram.attr,
+ &dev_attr_rs485.attr,
+ &dev_attr_adc.attr,
+ &dev_attr_ereset.attr,
+ &dev_attr_itr.attr,
+ &dev_attr_jp1.attr,
+ &dev_attr_jp2.attr,
+ &dev_attr_jp3.attr,
+ &dev_attr_jp4.attr,
+ &dev_attr_jp5.attr,
+ &dev_attr_jp6.attr,
+ NULL
+};
+
+static const struct attribute_group ts5500_attr_group = {
+ .attrs = ts5500_attributes,
+};
+
+/* LED platform device */
+
+static void ts5500_led_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ outb(!!brightness, TS5500_LED_JP_ADDR);
+}
+
+static enum led_brightness ts5500_led_get(struct led_classdev *led_cdev)
+{
+ return (inb(TS5500_LED_JP_ADDR) & TS5500_LED) ? LED_FULL : LED_OFF;
+}
+
+static struct led_classdev ts5500_led_cdev = {
+ .name = "ts5500:green:activity",
+ .brightness_set = ts5500_led_set,
+ .brightness_get = ts5500_led_get,
+};
+
+/* A/D Converter platform device */
+
+static int ts5500_adc_convert(u8 ctrl, u16 *raw)
+{
+ u8 lsb, msb;
+
+ /* Start convertion (ensure the 3 MSB are set to 0) */
+ outb(ctrl & 0x1F, TS5500_ADC_CONV_INIT_LSB_ADDR);
+
+ udelay(TS5500_ADC_CONV_DELAY);
+ if (inb(TS5500_ADC_CONV_BUSY_ADDR) & TS5500_ADC_CONV_BUSY)
+ return -EBUSY;
+
+ /* Read the raw data */
+ lsb = inb(TS5500_ADC_CONV_INIT_LSB_ADDR);
+ msb = inb(TS5500_ADC_CONV_MSB_ADDR);
+ *raw = (msb << 8) | lsb;
+
+ return 0;
+}
+
+static struct max197_platform_data ts5500_adc_pdata = {
+ .convert = ts5500_adc_convert,
+};
+
+static void ts5500_adc_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_adc_pdev = {
+ .name = "max197",
+ .id = -1,
+ .dev = {
+ .platform_data = &ts5500_adc_pdata,
+ .release = ts5500_adc_release,
+ },
+};
+
+static int __init ts5500_init(void)
+{
+ int ret;
+ struct platform_device *pdev;
+
+ /*
+ * There is no DMI available, or PCI bridge subvendor info,
+ * only the BIOS provides a 16-bit identification call.
+ * It is safer to check for a TS-5500 specific hardware
+ * such as the processor, then find a signature in the BIOS.
+ */
+ ret = ts5500_pre_detect_hw();
+ if (ret)
+ return ret;
+
+ ret = ts5500_bios_signature();
+ if (ret)
+ return ret;
+
+ ts5500 = kzalloc(sizeof(struct ts5500_sbc), GFP_KERNEL);
+ if (!ts5500)
+ return -ENOMEM;
+ mutex_init(&ts5500->lock);
+
+ ret = ts5500_detect_config(ts5500);
+ if (ret)
+ goto release_mem;
+
+ pdev = platform_device_register_simple("ts5500", -1, NULL, 0);
+ if (IS_ERR(pdev)) {
+ ret = PTR_ERR(pdev);
+ goto release_mem;
+ }
+ platform_set_drvdata(pdev, ts5500);
+
+ ret = sysfs_create_group(&pdev->dev.kobj,
+ &ts5500_attr_group);
+ if (ret)
+ goto release_pdev;
+
+ if (led_classdev_register(&pdev->dev, &ts5500_led_cdev))
+ dev_warn(ts5500_led_cdev.dev, "failed to register the LED\n");
+ if (ts5500->adc) {
+ ts5500_adc_pdev.dev.parent = &pdev->dev;
+ if (platform_device_register(&ts5500_adc_pdev))
+ dev_warn(&ts5500_adc_pdev.dev,
+ "failed to register the A/D converter\n");
+ }
+
+ return 0;
+
+release_pdev:
+ platform_device_unregister(pdev);
+release_mem:
+ kfree(ts5500);
+
+ return ret;
+}
+device_initcall(ts5500_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>");
+MODULE_DESCRIPTION("Technologic Systems TS-5500 Board's platform driver");
--
1.7.9.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [lm-sensors] [PATCH v6 2/3] x86/platform: TS-5500 basic platform support
@ 2012-04-13 0:28 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
Documentation/ABI/testing/sysfs-platform-ts5500 | 46 +++
MAINTAINERS | 5 +
arch/x86/Kconfig | 8 +
arch/x86/platform/Makefile | 1 +
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500.c | 400 +++++++++++++++++++++++
6 files changed, 461 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-ts5500
create mode 100644 arch/x86/platform/ts5500/Makefile
create mode 100644 arch/x86/platform/ts5500/ts5500.c
diff --git a/Documentation/ABI/testing/sysfs-platform-ts5500 b/Documentation/ABI/testing/sysfs-platform-ts5500
new file mode 100644
index 0000000..bc29bb8
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-ts5500
@@ -0,0 +1,46 @@
+What: /sys/devices/platform/ts5500/id
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Product ID of the TS board. TS-5500 ID is 0x60.
+
+What: /sys/devices/platform/ts5500/sram
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if the SRAM feature is present. If it is enabled, it will
+ display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/ereset
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if an external reset is present. If it is present, it will
+ display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/jp{1,2,3,4,5,6}
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Show the state of a plugged jumper. If it is present, it will
+ display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/adc
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if the A/D Converter is present. If it is enabled,
+ it will display "1", otherwise "0".
+
+What: /sys/devices/platform/ts5500/rs485
+Date: April 2012
+KernelVersion: 3.3
+Contact: "Savoir-faire Linux Inc." <kernel@savoirfairelinux.com>
+Description:
+ Say if the RS485 feature is present. If it is enabled, it
+ will display "1", otherwise "0".
diff --git a/MAINTAINERS b/MAINTAINERS
index 2dcfca8..b126129 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6620,6 +6620,11 @@ S: Supported
F: drivers/net/team/
F: include/linux/if_team.h
+TECHNOLOGIC SYSTEMS TS-5500 MACHINE SUPPORT
+M: Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>
+S: Maintained
+F: arch/x86/platform/ts5500/
+
TEGRA SUPPORT
M: Colin Cross <ccross@android.com>
M: Olof Johansson <olof@lixom.net>
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1d14cc6..55f32b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2131,6 +2131,14 @@ config GEOS
---help---
This option enables system support for the Traverse Technologies GEOS.
+config TS5500
+ bool "Technologic Systems TS-5500 Single Board Computer support"
+ depends on MELAN
+ select CHECK_SIGNATURE
+ ---help---
+ Add support for the Technologic Systems TS-5500 SBC.
+ If you have a TS-5500 platform, say Y here.
+
endif # X86_32
config AMD_NB
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 8d87439..f50911b 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -7,5 +7,6 @@ obj-y += mrst/
obj-y += olpc/
obj-y += scx200/
obj-y += sfi/
+obj-y += ts5500/
obj-y += visws/
obj-y += uv/
diff --git a/arch/x86/platform/ts5500/Makefile b/arch/x86/platform/ts5500/Makefile
new file mode 100644
index 0000000..c54e348
--- /dev/null
+++ b/arch/x86/platform/ts5500/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TS5500) += ts5500.o
diff --git a/arch/x86/platform/ts5500/ts5500.c b/arch/x86/platform/ts5500/ts5500.c
new file mode 100644
index 0000000..96f76c0
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500.c
@@ -0,0 +1,400 @@
+/*
+ * Technologic Systems TS-5500 board - SBC info layer
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ * Alexandre Savard <alexandre.savard@savoirfairelinux.com>
+ * Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
+ *
+ * Portions originate from ts_sbcinfo.c (c) Technologic Systems
+ * Liberty Young <liberty@embeddedx86.com>
+ *
+ * These functions add sysfs platform entries to display information about
+ * the Technologic Systems TS-5500 Single Board Computer (SBC).
+ *
+ * For further information about sysfs entries, see
+ * Documentation/ABI/testing/sysfs-platform-ts5500
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <asm/processor.h>
+#include <linux/leds.h>
+#include <linux/delay.h>
+#include <linux/max197.h>
+
+/* Hardware info for pre-detection */
+#define AMD_ELAN_FAMILY 4
+#define AMD_ELAN_SC520 9
+
+/* Product code register */
+#define TS5500_PRODUCT_CODE_ADDR 0x74
+#define TS5500_PRODUCT_CODE 0x60 /* TS-5500 product code */
+
+/* SRAM/RS-485/ADC options, and RS-485 RTS/Automatic RS-485 flags register */
+#define TS5500_SRAM_RS485_ADC_ADDR 0x75
+#define TS5500_SRAM 0x01 /* SRAM option */
+#define TS5500_RS485 0x02 /* RS-485 option */
+#define TS5500_ADC 0x04 /* A/D converter option */
+#define TS5500_RS485_RTS 0x40 /* RTS for RS-485 */
+#define TS5500_RS485_AUTO 0x80 /* Automatic RS-485 */
+
+/* External Reset/Industrial Temperature Range options register */
+#define TS5500_ERESET_ITR_ADDR 0x76
+#define TS5500_ERESET 0x01 /* External Reset option */
+#define TS5500_ITR 0x02 /* Indust. Temp. Range option */
+
+/* LED/Jumpers register */
+#define TS5500_LED_JP_ADDR 0x77
+#define TS5500_LED 0x01 /* LED flag */
+#define TS5500_JP1 0x02 /* Automatic CMOS */
+#define TS5500_JP2 0x04 /* Enable Serial Console */
+#define TS5500_JP3 0x08 /* Write Enable Drive A */
+#define TS5500_JP4 0x10 /* Fast Console (115K baud) */
+#define TS5500_JP5 0x20 /* User Jumper */
+#define TS5500_JP6 0x40 /* Console on COM1 (req. JP2) */
+#define TS5500_JP7 0x80 /* Undocumented (Unused) */
+
+/* A/D Converter registers */
+#define TS5500_ADC_CONV_BUSY_ADDR 0x195 /* Conversion state register */
+#define TS5500_ADC_CONV_BUSY 0x01
+#define TS5500_ADC_CONV_INIT_LSB_ADDR 0x196 /* Start conv. / LSB register */
+#define TS5500_ADC_CONV_MSB_ADDR 0x197 /* MSB register */
+#define TS5500_ADC_CONV_DELAY 12 /* usec */
+
+/**
+ * struct ts5500_sbc - TS-5500 SBC main structure
+ * @lock: Read/Write mutex.
+ * @board_id: Board name.
+ * @sram: Check SRAM option.
+ * @rs485: Check RS-485 option.
+ * @adc: Check Analog/Digital converter option.
+ * @ereset: Check External Reset option.
+ * @itr: Check Industrial Temperature Range option.
+ * @jumpers: States of jumpers 1-7.
+ */
+struct ts5500_sbc {
+ struct mutex lock;
+ int board_id;
+ bool sram;
+ bool rs485;
+ bool adc;
+ bool ereset;
+ bool itr;
+ u8 jumpers;
+};
+
+/* Current platform */
+struct ts5500_sbc *ts5500;
+
+/**
+ * ts5500_pre_detect_hw() - check for TS-5500 specific hardware
+ */
+static int __init ts5500_pre_detect_hw(void)
+{
+ /* Check for AMD ElanSC520 Microcontroller */
+ if (cpu_info.x86_vendor != X86_VENDOR_AMD ||
+ cpu_info.x86 != AMD_ELAN_FAMILY ||
+ cpu_info.x86_model != AMD_ELAN_SC520)
+ return -ENODEV;
+
+ return 0;
+}
+
+/* BIOS signatures */
+static struct {
+ const unsigned char *string;
+ const ssize_t offset;
+} signatures[] __initdata = {
+ {"TS-5x00 AMD Elan", 0xb14}
+};
+
+/**
+ * ts5500_bios_signature() - find board signature in BIOS shadow RAM.
+ */
+static int __init ts5500_bios_signature(void)
+{
+ void __iomem *bios = ioremap(0xF0000, 0x10000);
+ int i, ret = 0;
+
+ for (i = 0; i < ARRAY_SIZE(signatures); i++)
+ if (check_signature(bios + signatures[i].offset,
+ signatures[i].string,
+ strlen(signatures[i].string)))
+ goto found;
+ else
+ pr_notice("Technologic Systems BIOS signature "
+ "'%s' not found at offset %zd\n",
+ signatures[i].string, signatures[i].offset);
+ ret = -ENODEV;
+found:
+ iounmap(bios);
+ return ret;
+}
+
+/**
+ * ts5500_detect_config() - detect the TS board
+ * @sbc: Structure in which the detected board's details will be stored.
+ */
+static int __init ts5500_detect_config(struct ts5500_sbc *sbc)
+{
+ u8 tmp;
+ int ret = 0;
+
+ if (!request_region(TS5500_PRODUCT_CODE_ADDR, 4, "ts5500"))
+ return -EBUSY;
+
+ mutex_lock(&ts5500->lock);
+ tmp = inb(TS5500_PRODUCT_CODE_ADDR);
+ if (tmp != TS5500_PRODUCT_CODE) {
+ pr_err("This platform is not a TS-5500 (found ID 0x%x)\n", tmp);
+ ret = -ENODEV;
+ goto cleanup;
+ }
+ sbc->board_id = tmp;
+
+ tmp = inb(TS5500_SRAM_RS485_ADC_ADDR);
+ ts5500->sram = tmp & TS5500_SRAM;
+ ts5500->rs485 = tmp & TS5500_RS485;
+ ts5500->adc = tmp & TS5500_ADC;
+
+ tmp = inb(TS5500_ERESET_ITR_ADDR);
+ ts5500->ereset = tmp & TS5500_ERESET;
+ ts5500->itr = tmp & TS5500_ITR;
+
+ tmp = inb(TS5500_LED_JP_ADDR);
+ sbc->jumpers = tmp & ~TS5500_LED;
+
+cleanup:
+ mutex_unlock(&ts5500->lock);
+ release_region(TS5500_PRODUCT_CODE_ADDR, 4);
+ return ret;
+}
+
+#define TS5500_IS_JP_SET(sbc, jmp) (!!(sbc->jumpers & TS5500_JP##jmp))
+
+static ssize_t ts5500_show_id(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "0x%x\n", sbc->board_id);
+}
+
+static ssize_t ts5500_show_sram(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->sram);
+}
+
+static ssize_t ts5500_show_rs485(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->rs485);
+}
+
+static ssize_t ts5500_show_adc(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->adc);
+}
+
+static ssize_t ts5500_show_ereset(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->ereset);
+}
+
+static ssize_t ts5500_show_itr(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%d\n", sbc->itr);
+}
+
+#define TS5500_SHOW_JP(jp) \
+ static ssize_t ts5500_show_jp##jp(struct device *dev, \
+ struct device_attribute *attr,\
+ char *buf) \
+ { \
+ struct ts5500_sbc *sbc = dev_get_drvdata(dev); \
+ return sprintf(buf, "%d\n", TS5500_IS_JP_SET(sbc, jp)); \
+ }
+
+TS5500_SHOW_JP(1)
+TS5500_SHOW_JP(2)
+TS5500_SHOW_JP(3)
+TS5500_SHOW_JP(4)
+TS5500_SHOW_JP(5)
+TS5500_SHOW_JP(6)
+
+static DEVICE_ATTR(id, S_IRUGO, ts5500_show_id, NULL);
+static DEVICE_ATTR(sram, S_IRUGO, ts5500_show_sram, NULL);
+static DEVICE_ATTR(rs485, S_IRUGO, ts5500_show_rs485, NULL);
+static DEVICE_ATTR(adc, S_IRUGO, ts5500_show_adc, NULL);
+static DEVICE_ATTR(ereset, S_IRUGO, ts5500_show_ereset, NULL);
+static DEVICE_ATTR(itr, S_IRUGO, ts5500_show_itr, NULL);
+static DEVICE_ATTR(jp1, S_IRUGO, ts5500_show_jp1, NULL);
+static DEVICE_ATTR(jp2, S_IRUGO, ts5500_show_jp2, NULL);
+static DEVICE_ATTR(jp3, S_IRUGO, ts5500_show_jp3, NULL);
+static DEVICE_ATTR(jp4, S_IRUGO, ts5500_show_jp4, NULL);
+static DEVICE_ATTR(jp5, S_IRUGO, ts5500_show_jp5, NULL);
+static DEVICE_ATTR(jp6, S_IRUGO, ts5500_show_jp6, NULL);
+
+static struct attribute *ts5500_attributes[] = {
+ &dev_attr_id.attr,
+ &dev_attr_sram.attr,
+ &dev_attr_rs485.attr,
+ &dev_attr_adc.attr,
+ &dev_attr_ereset.attr,
+ &dev_attr_itr.attr,
+ &dev_attr_jp1.attr,
+ &dev_attr_jp2.attr,
+ &dev_attr_jp3.attr,
+ &dev_attr_jp4.attr,
+ &dev_attr_jp5.attr,
+ &dev_attr_jp6.attr,
+ NULL
+};
+
+static const struct attribute_group ts5500_attr_group = {
+ .attrs = ts5500_attributes,
+};
+
+/* LED platform device */
+
+static void ts5500_led_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ outb(!!brightness, TS5500_LED_JP_ADDR);
+}
+
+static enum led_brightness ts5500_led_get(struct led_classdev *led_cdev)
+{
+ return (inb(TS5500_LED_JP_ADDR) & TS5500_LED) ? LED_FULL : LED_OFF;
+}
+
+static struct led_classdev ts5500_led_cdev = {
+ .name = "ts5500:green:activity",
+ .brightness_set = ts5500_led_set,
+ .brightness_get = ts5500_led_get,
+};
+
+/* A/D Converter platform device */
+
+static int ts5500_adc_convert(u8 ctrl, u16 *raw)
+{
+ u8 lsb, msb;
+
+ /* Start convertion (ensure the 3 MSB are set to 0) */
+ outb(ctrl & 0x1F, TS5500_ADC_CONV_INIT_LSB_ADDR);
+
+ udelay(TS5500_ADC_CONV_DELAY);
+ if (inb(TS5500_ADC_CONV_BUSY_ADDR) & TS5500_ADC_CONV_BUSY)
+ return -EBUSY;
+
+ /* Read the raw data */
+ lsb = inb(TS5500_ADC_CONV_INIT_LSB_ADDR);
+ msb = inb(TS5500_ADC_CONV_MSB_ADDR);
+ *raw = (msb << 8) | lsb;
+
+ return 0;
+}
+
+static struct max197_platform_data ts5500_adc_pdata = {
+ .convert = ts5500_adc_convert,
+};
+
+static void ts5500_adc_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_adc_pdev = {
+ .name = "max197",
+ .id = -1,
+ .dev = {
+ .platform_data = &ts5500_adc_pdata,
+ .release = ts5500_adc_release,
+ },
+};
+
+static int __init ts5500_init(void)
+{
+ int ret;
+ struct platform_device *pdev;
+
+ /*
+ * There is no DMI available, or PCI bridge subvendor info,
+ * only the BIOS provides a 16-bit identification call.
+ * It is safer to check for a TS-5500 specific hardware
+ * such as the processor, then find a signature in the BIOS.
+ */
+ ret = ts5500_pre_detect_hw();
+ if (ret)
+ return ret;
+
+ ret = ts5500_bios_signature();
+ if (ret)
+ return ret;
+
+ ts5500 = kzalloc(sizeof(struct ts5500_sbc), GFP_KERNEL);
+ if (!ts5500)
+ return -ENOMEM;
+ mutex_init(&ts5500->lock);
+
+ ret = ts5500_detect_config(ts5500);
+ if (ret)
+ goto release_mem;
+
+ pdev = platform_device_register_simple("ts5500", -1, NULL, 0);
+ if (IS_ERR(pdev)) {
+ ret = PTR_ERR(pdev);
+ goto release_mem;
+ }
+ platform_set_drvdata(pdev, ts5500);
+
+ ret = sysfs_create_group(&pdev->dev.kobj,
+ &ts5500_attr_group);
+ if (ret)
+ goto release_pdev;
+
+ if (led_classdev_register(&pdev->dev, &ts5500_led_cdev))
+ dev_warn(ts5500_led_cdev.dev, "failed to register the LED\n");
+ if (ts5500->adc) {
+ ts5500_adc_pdev.dev.parent = &pdev->dev;
+ if (platform_device_register(&ts5500_adc_pdev))
+ dev_warn(&ts5500_adc_pdev.dev,
+ "failed to register the A/D converter\n");
+ }
+
+ return 0;
+
+release_pdev:
+ platform_device_unregister(pdev);
+release_mem:
+ kfree(ts5500);
+
+ return ret;
+}
+device_initcall(ts5500_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>");
+MODULE_DESCRIPTION("Technologic Systems TS-5500 Board's platform driver");
--
1.7.9.2
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-04-13 0:28 ` Vivien Didelot
-1 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Jerome Oufella, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij, Vivien Didelot
From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
Signed-off-by: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
MAINTAINERS | 2 +
arch/x86/include/asm/ts5500.h | 62 ++++++++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ts5500.c | 347 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 419 insertions(+)
create mode 100644 arch/x86/include/asm/ts5500.h
create mode 100644 drivers/gpio/gpio-ts5500.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b126129..b7b5d95 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6624,6 +6624,8 @@ TECHNOLOGIC SYSTEMS TS-5500 MACHINE SUPPORT
M: Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>
S: Maintained
F: arch/x86/platform/ts5500/
+F: arch/x86/include/asm/ts5500.h
+F: drivers/gpio/gpio-ts5500.c
TEGRA SUPPORT
M: Colin Cross <ccross@android.com>
diff --git a/arch/x86/include/asm/ts5500.h b/arch/x86/include/asm/ts5500.h
new file mode 100644
index 0000000..baf136c
--- /dev/null
+++ b/arch/x86/include/asm/ts5500.h
@@ -0,0 +1,62 @@
+/*
+ * Technologic Systems TS-5500 GPIO (DIO) definitions
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _TS5500_H
+#define _TS5500_H
+
+#define TS5500_DIO1_0 0
+#define TS5500_DIO1_1 1
+#define TS5500_DIO1_2 2
+#define TS5500_DIO1_3 3
+#define TS5500_DIO1_4 4
+#define TS5500_DIO1_5 5
+#define TS5500_DIO1_6 6
+#define TS5500_DIO1_7 7
+#define TS5500_DIO1_8 8
+#define TS5500_DIO1_9 9
+#define TS5500_DIO1_10 10
+#define TS5500_DIO1_11 11
+#define TS5500_DIO1_12 12
+#define TS5500_DIO1_13 13
+
+#define TS5500_DIO2_0 14
+#define TS5500_DIO2_1 15
+#define TS5500_DIO2_2 16
+#define TS5500_DIO2_3 17
+#define TS5500_DIO2_4 18
+#define TS5500_DIO2_5 19
+#define TS5500_DIO2_6 20
+#define TS5500_DIO2_7 21
+#define TS5500_DIO2_8 22
+#define TS5500_DIO2_9 23
+#define TS5500_DIO2_10 24
+#define TS5500_DIO2_11 25
+/* #define TS5500_DIO2_12 - Keep commented out as it simply doesn't exist. */
+#define TS5500_DIO2_13 26
+
+#define TS5500_LCD_0 27
+#define TS5500_LCD_1 28
+#define TS5500_LCD_2 29
+#define TS5500_LCD_3 30
+#define TS5500_LCD_4 31
+#define TS5500_LCD_5 32
+#define TS5500_LCD_6 33
+#define TS5500_LCD_7 34
+#define TS5500_LCD_EN 35
+#define TS5500_LCD_RS 36
+#define TS5500_LCD_WR 37
+
+/* Lines that can trigger IRQs */
+#define TS5500_DIO1_13_IRQ 7
+#define TS5500_DIO2_13_IRQ 6
+#define TS5500_LCD_RS_IRQ 1
+
+#endif /* _TS5500_H */
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index edadbda..a19b0f3 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -319,6 +319,13 @@ config GPIO_TPS65912
help
This driver supports TPS65912 gpio chip
+config GPIO_TS5500
+ tristate "TS-5500 GPIOs"
+ depends on TS5500
+ help
+ This driver supports the DIO headers for GPIO usage on the Technologic
+ Systems TS-5500 platform.
+
config GPIO_TWL4030
tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
depends on TWL4030_CORE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 007f54b..30cbd03 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_TIMBERDALE) += gpio-timberdale.o
obj-$(CONFIG_ARCH_DAVINCI_TNETV107X) += gpio-tnetv107x.o
obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
+obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
obj-$(CONFIG_GPIO_TWL4030) += gpio-twl4030.o
obj-$(CONFIG_GPIO_UCB1400) += gpio-ucb1400.o
obj-$(CONFIG_GPIO_VR41XX) += gpio-vr41xx.o
diff --git a/drivers/gpio/gpio-ts5500.c b/drivers/gpio/gpio-ts5500.c
new file mode 100644
index 0000000..01f34d8
--- /dev/null
+++ b/drivers/gpio/gpio-ts5500.c
@@ -0,0 +1,347 @@
+/*
+ * GPIO (DIO) driver for Technologic Systems TS-5500
+ *
+ * TS-5500 board has 38 GPIOs referred to as DIOs in the product's literature.
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <asm/ts5500.h>
+
+static int dio1_irq = 1;
+module_param(dio1_irq, int, 0644);
+MODULE_PARM_DESC(dio1_irq, "Enable usage of IRQ7 for any DIO1 line"
+ " (default 1)");
+
+static int dio2_irq;
+module_param(dio2_irq, int, 0644);
+MODULE_PARM_DESC(dio2_irq, "Enable usage of IRQ6 for any DIO2 line"
+ " (default 0)");
+
+static int lcd_irq;
+module_param(lcd_irq, int, 0644);
+MODULE_PARM_DESC(lcd_irq, "Enable usage of IRQ1 for any LCD line"
+ " (default 0)");
+
+static int use_lcdio;
+module_param(use_lcdio, int, 0644);
+MODULE_PARM_DESC(use_lcdio, "Enable usage of the LCD header for DIO operation"
+ " (default 0)");
+
+static DEFINE_SPINLOCK(gpio_lock);
+
+static inline void port_bit_set(u8 addr, int bit)
+{
+ u8 var;
+
+ var = inb(addr);
+ var |= (1 << bit);
+ outb(var, addr);
+}
+
+static inline void port_bit_clear(u8 addr, int bit)
+{
+ u8 var;
+
+ var = inb(addr);
+ var &= ~(1 << bit);
+ outb(var, addr);
+}
+
+/* "DIO" line to IO port mapping table for line's value */
+static const unsigned long line_to_port_map[] = {
+ 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, /* DIO1_[0-7] */
+ 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, /* DIO1_[8-13] */
+ 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, /* DIO2_[0-7] */
+ 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, /* DIO2_[8-13] */
+ 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, /* LCD_[0-7] */
+ 0x73, 0x73, 0x73 /* LCD_{EN,RS,WR} */
+};
+
+/* "DIO" line to IO port's bit map for line's value */
+static const int line_to_bit_map[] = {
+ 0, 1, 2, 3, 4, 5, 6, 7, /* DIO1_[0-7] */
+ 0, 1, 2, 3, 4, 5, /* DIO1_[8-13] */
+ 0, 1, 2, 3, 4, 5, 6, 7, /* DIO2_[0-7] */
+ 0, 1, 2, 3, 4, 5, /* DIO2_[8-13] */
+ 0, 1, 2, 3, 4, 5, 6, 7, /* LCD_[0-7] */
+ 0, 7, 6 /* LCD_{EN,RS,WR} */
+};
+
+/* "DIO" line's direction control mapping table */
+static const unsigned long line_to_dir_map[] = {
+ 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, /* DIO1_[0-7] */
+ 0x7A, 0x7A, 0x7A, 0x7A, 0, 0, /* DIO1_[8-13] */
+ 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* DIO2_[0-7] */
+ 0x7D, 0x7D, 0x7D, 0x7D, 0, 0, /* DIO2_[8-13] */
+ 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* LCD_[0-7] */
+ 0, 0, 0 /* LCD_{EN,RS,WR} */
+};
+
+/* "DIO" line's direction control bit-mapping table */
+static const int line_to_dir_bit_map[] = {
+ 0, 0, 0, 0, 1, 1, 1, 1, /* DIO1_[0-7] */
+ 5, 5, 5, 5, -1, -1, /* DIO1_[8-13] */
+ 0, 0, 0, 0, 1, 1, 1, 1, /* DIO2_[0-7] */
+ 5, 5, 5, 5, -1, -1, /* DIO2_[8-13] */
+ 2, 2, 2, 2, 3, 3, 3, 3, /* LCD_[0-7] */
+ -1, -1, -1 /* LCD_{EN,RS,WR} */
+};
+
+static int ts5500_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long dir_reg, flags;
+ int dir_bit;
+
+ /* Some lines cannot be configured as input */
+ if (offset == TS5500_LCD_EN)
+ return -ENXIO;
+
+ dir_reg = line_to_dir_map[offset];
+ dir_bit = line_to_dir_bit_map[offset];
+
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_clear(dir_reg, dir_bit);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+}
+
+static int ts5500_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long ioaddr;
+ int bitno;
+
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ return (inb(ioaddr) >> bitno) & 0x1;
+}
+
+static int ts5500_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
+ int val)
+{
+ unsigned long dir_reg, ioaddr, flags;
+ int dir_bit, bitno;
+
+ /* Some lines cannot be configured as output */
+ switch (offset) {
+ case TS5500_DIO1_12:
+ case TS5500_DIO1_13:
+ case TS5500_DIO2_13:
+ case TS5500_LCD_RS:
+ case TS5500_LCD_WR:
+ return -ENXIO;
+ default:
+ break;
+ }
+
+ dir_reg = line_to_dir_map[offset];
+ dir_bit = line_to_dir_bit_map[offset];
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_set(dir_reg, dir_bit);
+ if (val)
+ port_bit_set(ioaddr, bitno);
+ else
+ port_bit_clear(ioaddr, bitno);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+}
+
+static void ts5500_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+ int bitno;
+ unsigned long ioaddr, flags;
+
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ spin_lock_irqsave(&gpio_lock, flags);
+ if (val)
+ port_bit_set(ioaddr, bitno);
+ else
+ port_bit_clear(ioaddr, bitno);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+}
+
+static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ /* Only a few lines are IRQ-capable */
+ if (offset == TS5500_DIO1_13)
+ return TS5500_DIO1_13_IRQ;
+ if (offset == TS5500_DIO2_13)
+ return TS5500_DIO2_13_IRQ;
+ if (offset == TS5500_LCD_RS)
+ return TS5500_LCD_RS_IRQ;
+
+ /* IRQ line may be bridged with another DIO line from the same header */
+ if (dio1_irq && offset >= TS5500_DIO1_0 && offset < TS5500_DIO1_13)
+ return TS5500_DIO1_13_IRQ;
+ if (dio2_irq && offset >= TS5500_DIO2_0 && offset < TS5500_DIO2_13)
+ return TS5500_DIO2_13_IRQ;
+ if (lcd_irq && offset >= TS5500_LCD_0 && offset <= TS5500_LCD_WR)
+ return TS5500_LCD_RS_IRQ;
+
+ return -ENXIO;
+}
+
+static struct gpio_chip ts5500_gpio_chip = {
+ .label = "ts5500-gpio",
+ .owner = THIS_MODULE,
+ .direction_input = ts5500_gpio_direction_input,
+ .get = ts5500_gpio_get,
+ .direction_output = ts5500_gpio_direction_output,
+ .set = ts5500_gpio_set,
+ .to_irq = ts5500_gpio_to_irq,
+ .base = TS5500_DIO1_0,
+};
+
+static void ts5500_gpio_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_gpio_pdev = {
+ .name = "ts5500-gpio",
+ .id = -1,
+ .dev = {
+ .release = ts5500_gpio_release,
+ },
+};
+
+static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
+{
+ int ret;
+ unsigned long flags;
+
+ if (pdev == NULL)
+ return -ENODEV;
+
+ if (!request_region(0x7A, 3, "ts5500-gpio-DIO1")) {
+ dev_err(&pdev->dev, "failed to request I/O port 0x7A-7C\n");
+ return -EBUSY;
+ }
+
+ if (!request_region(0x7D, 3, "ts5500-gpio-DIO2")) {
+ dev_err(&pdev->dev, "failed to request I/O port 0x7D-7F\n");
+ ret = -EBUSY;
+ goto release_dio1;
+ }
+
+ if (use_lcdio && !request_region(0x72, 2, "ts5500-gpio-LCD")) {
+ dev_err(&pdev->dev, "failed to request I/O port 0x72-73\n");
+ ret = -EBUSY;
+ goto release_dio2;
+ }
+
+ if (use_lcdio)
+ ts5500_gpio_chip.ngpio = TS5500_LCD_WR + 1;
+ else
+ ts5500_gpio_chip.ngpio = TS5500_DIO2_13 + 1;
+
+ ret = gpiochip_add(&ts5500_gpio_chip);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register the gpio chip\n");
+ goto release_lcd;
+ }
+
+ /* Enable IRQ generation */
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_set(0x7A, 7); /* DIO1_13 on IRQ7 */
+ port_bit_set(0x7D, 7); /* DIO2_13 on IRQ6 */
+ if (use_lcdio) {
+ port_bit_clear(0x7D, 4); /* LCD header usage as DIO */
+ port_bit_set(0x7D, 6); /* LCD_RS on IRQ1 */
+ }
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+
+release_lcd:
+ if (use_lcdio)
+ release_region(0x72, 2);
+release_dio2:
+ release_region(0x7D, 3);
+release_dio1:
+ release_region(0x7A, 3);
+
+ return ret;
+}
+
+static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
+{
+ int ret;
+ unsigned long flags;
+
+ /* Disable IRQ generation */
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_clear(0x7A, 7);
+ port_bit_clear(0x7D, 7);
+ if (use_lcdio)
+ port_bit_clear(0x7D, 6);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ release_region(0x7A, 3);
+ release_region(0x7D, 3);
+ if (use_lcdio)
+ release_region(0x72, 2);
+
+ ret = gpiochip_remove(&ts5500_gpio_chip);
+ if (ret)
+ dev_err(&pdev->dev, "failed to remove the gpio chip\n");
+
+ return ret;
+}
+
+static struct platform_driver ts5500_gpio_driver = {
+ .driver = {
+ .name = "ts5500-gpio",
+ .owner = THIS_MODULE,
+ },
+ .probe = ts5500_gpio_probe,
+ .remove = __devexit_p(ts5500_gpio_remove),
+};
+
+static int __init ts5500_gpio_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&ts5500_gpio_driver);
+ if (ret)
+ return ret;
+
+ ret = platform_device_register(&ts5500_gpio_pdev);
+ if (ret) {
+ platform_driver_unregister(&ts5500_gpio_driver);
+ return ret;
+ }
+
+ return 0;
+}
+module_init(ts5500_gpio_init);
+
+static void __exit ts5500_gpio_exit(void)
+{
+ platform_driver_unregister(&ts5500_gpio_driver);
+ platform_device_unregister(&ts5500_gpio_pdev);
+}
+module_exit(ts5500_gpio_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>");
+MODULE_DESCRIPTION("Technologic Systems TS-5500, GPIO/DIO driver");
--
1.7.9.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-04-13 0:28 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 0:28 UTC (permalink / raw)
To: x86
Cc: Jerome Oufella, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Grant Likely, Linus Walleij, Vivien Didelot
From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
Signed-off-by: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
MAINTAINERS | 2 +
arch/x86/include/asm/ts5500.h | 62 ++++++++
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ts5500.c | 347 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 419 insertions(+)
create mode 100644 arch/x86/include/asm/ts5500.h
create mode 100644 drivers/gpio/gpio-ts5500.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b126129..b7b5d95 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6624,6 +6624,8 @@ TECHNOLOGIC SYSTEMS TS-5500 MACHINE SUPPORT
M: Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>
S: Maintained
F: arch/x86/platform/ts5500/
+F: arch/x86/include/asm/ts5500.h
+F: drivers/gpio/gpio-ts5500.c
TEGRA SUPPORT
M: Colin Cross <ccross@android.com>
diff --git a/arch/x86/include/asm/ts5500.h b/arch/x86/include/asm/ts5500.h
new file mode 100644
index 0000000..baf136c
--- /dev/null
+++ b/arch/x86/include/asm/ts5500.h
@@ -0,0 +1,62 @@
+/*
+ * Technologic Systems TS-5500 GPIO (DIO) definitions
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _TS5500_H
+#define _TS5500_H
+
+#define TS5500_DIO1_0 0
+#define TS5500_DIO1_1 1
+#define TS5500_DIO1_2 2
+#define TS5500_DIO1_3 3
+#define TS5500_DIO1_4 4
+#define TS5500_DIO1_5 5
+#define TS5500_DIO1_6 6
+#define TS5500_DIO1_7 7
+#define TS5500_DIO1_8 8
+#define TS5500_DIO1_9 9
+#define TS5500_DIO1_10 10
+#define TS5500_DIO1_11 11
+#define TS5500_DIO1_12 12
+#define TS5500_DIO1_13 13
+
+#define TS5500_DIO2_0 14
+#define TS5500_DIO2_1 15
+#define TS5500_DIO2_2 16
+#define TS5500_DIO2_3 17
+#define TS5500_DIO2_4 18
+#define TS5500_DIO2_5 19
+#define TS5500_DIO2_6 20
+#define TS5500_DIO2_7 21
+#define TS5500_DIO2_8 22
+#define TS5500_DIO2_9 23
+#define TS5500_DIO2_10 24
+#define TS5500_DIO2_11 25
+/* #define TS5500_DIO2_12 - Keep commented out as it simply doesn't exist. */
+#define TS5500_DIO2_13 26
+
+#define TS5500_LCD_0 27
+#define TS5500_LCD_1 28
+#define TS5500_LCD_2 29
+#define TS5500_LCD_3 30
+#define TS5500_LCD_4 31
+#define TS5500_LCD_5 32
+#define TS5500_LCD_6 33
+#define TS5500_LCD_7 34
+#define TS5500_LCD_EN 35
+#define TS5500_LCD_RS 36
+#define TS5500_LCD_WR 37
+
+/* Lines that can trigger IRQs */
+#define TS5500_DIO1_13_IRQ 7
+#define TS5500_DIO2_13_IRQ 6
+#define TS5500_LCD_RS_IRQ 1
+
+#endif /* _TS5500_H */
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index edadbda..a19b0f3 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -319,6 +319,13 @@ config GPIO_TPS65912
help
This driver supports TPS65912 gpio chip
+config GPIO_TS5500
+ tristate "TS-5500 GPIOs"
+ depends on TS5500
+ help
+ This driver supports the DIO headers for GPIO usage on the Technologic
+ Systems TS-5500 platform.
+
config GPIO_TWL4030
tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
depends on TWL4030_CORE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 007f54b..30cbd03 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_TIMBERDALE) += gpio-timberdale.o
obj-$(CONFIG_ARCH_DAVINCI_TNETV107X) += gpio-tnetv107x.o
obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
+obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
obj-$(CONFIG_GPIO_TWL4030) += gpio-twl4030.o
obj-$(CONFIG_GPIO_UCB1400) += gpio-ucb1400.o
obj-$(CONFIG_GPIO_VR41XX) += gpio-vr41xx.o
diff --git a/drivers/gpio/gpio-ts5500.c b/drivers/gpio/gpio-ts5500.c
new file mode 100644
index 0000000..01f34d8
--- /dev/null
+++ b/drivers/gpio/gpio-ts5500.c
@@ -0,0 +1,347 @@
+/*
+ * GPIO (DIO) driver for Technologic Systems TS-5500
+ *
+ * TS-5500 board has 38 GPIOs referred to as DIOs in the product's literature.
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <asm/ts5500.h>
+
+static int dio1_irq = 1;
+module_param(dio1_irq, int, 0644);
+MODULE_PARM_DESC(dio1_irq, "Enable usage of IRQ7 for any DIO1 line"
+ " (default 1)");
+
+static int dio2_irq;
+module_param(dio2_irq, int, 0644);
+MODULE_PARM_DESC(dio2_irq, "Enable usage of IRQ6 for any DIO2 line"
+ " (default 0)");
+
+static int lcd_irq;
+module_param(lcd_irq, int, 0644);
+MODULE_PARM_DESC(lcd_irq, "Enable usage of IRQ1 for any LCD line"
+ " (default 0)");
+
+static int use_lcdio;
+module_param(use_lcdio, int, 0644);
+MODULE_PARM_DESC(use_lcdio, "Enable usage of the LCD header for DIO operation"
+ " (default 0)");
+
+static DEFINE_SPINLOCK(gpio_lock);
+
+static inline void port_bit_set(u8 addr, int bit)
+{
+ u8 var;
+
+ var = inb(addr);
+ var |= (1 << bit);
+ outb(var, addr);
+}
+
+static inline void port_bit_clear(u8 addr, int bit)
+{
+ u8 var;
+
+ var = inb(addr);
+ var &= ~(1 << bit);
+ outb(var, addr);
+}
+
+/* "DIO" line to IO port mapping table for line's value */
+static const unsigned long line_to_port_map[] = {
+ 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, /* DIO1_[0-7] */
+ 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, /* DIO1_[8-13] */
+ 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, /* DIO2_[0-7] */
+ 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, /* DIO2_[8-13] */
+ 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, /* LCD_[0-7] */
+ 0x73, 0x73, 0x73 /* LCD_{EN,RS,WR} */
+};
+
+/* "DIO" line to IO port's bit map for line's value */
+static const int line_to_bit_map[] = {
+ 0, 1, 2, 3, 4, 5, 6, 7, /* DIO1_[0-7] */
+ 0, 1, 2, 3, 4, 5, /* DIO1_[8-13] */
+ 0, 1, 2, 3, 4, 5, 6, 7, /* DIO2_[0-7] */
+ 0, 1, 2, 3, 4, 5, /* DIO2_[8-13] */
+ 0, 1, 2, 3, 4, 5, 6, 7, /* LCD_[0-7] */
+ 0, 7, 6 /* LCD_{EN,RS,WR} */
+};
+
+/* "DIO" line's direction control mapping table */
+static const unsigned long line_to_dir_map[] = {
+ 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, /* DIO1_[0-7] */
+ 0x7A, 0x7A, 0x7A, 0x7A, 0, 0, /* DIO1_[8-13] */
+ 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* DIO2_[0-7] */
+ 0x7D, 0x7D, 0x7D, 0x7D, 0, 0, /* DIO2_[8-13] */
+ 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* LCD_[0-7] */
+ 0, 0, 0 /* LCD_{EN,RS,WR} */
+};
+
+/* "DIO" line's direction control bit-mapping table */
+static const int line_to_dir_bit_map[] = {
+ 0, 0, 0, 0, 1, 1, 1, 1, /* DIO1_[0-7] */
+ 5, 5, 5, 5, -1, -1, /* DIO1_[8-13] */
+ 0, 0, 0, 0, 1, 1, 1, 1, /* DIO2_[0-7] */
+ 5, 5, 5, 5, -1, -1, /* DIO2_[8-13] */
+ 2, 2, 2, 2, 3, 3, 3, 3, /* LCD_[0-7] */
+ -1, -1, -1 /* LCD_{EN,RS,WR} */
+};
+
+static int ts5500_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long dir_reg, flags;
+ int dir_bit;
+
+ /* Some lines cannot be configured as input */
+ if (offset = TS5500_LCD_EN)
+ return -ENXIO;
+
+ dir_reg = line_to_dir_map[offset];
+ dir_bit = line_to_dir_bit_map[offset];
+
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_clear(dir_reg, dir_bit);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+}
+
+static int ts5500_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long ioaddr;
+ int bitno;
+
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ return (inb(ioaddr) >> bitno) & 0x1;
+}
+
+static int ts5500_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
+ int val)
+{
+ unsigned long dir_reg, ioaddr, flags;
+ int dir_bit, bitno;
+
+ /* Some lines cannot be configured as output */
+ switch (offset) {
+ case TS5500_DIO1_12:
+ case TS5500_DIO1_13:
+ case TS5500_DIO2_13:
+ case TS5500_LCD_RS:
+ case TS5500_LCD_WR:
+ return -ENXIO;
+ default:
+ break;
+ }
+
+ dir_reg = line_to_dir_map[offset];
+ dir_bit = line_to_dir_bit_map[offset];
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_set(dir_reg, dir_bit);
+ if (val)
+ port_bit_set(ioaddr, bitno);
+ else
+ port_bit_clear(ioaddr, bitno);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+}
+
+static void ts5500_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+ int bitno;
+ unsigned long ioaddr, flags;
+
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ spin_lock_irqsave(&gpio_lock, flags);
+ if (val)
+ port_bit_set(ioaddr, bitno);
+ else
+ port_bit_clear(ioaddr, bitno);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+}
+
+static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ /* Only a few lines are IRQ-capable */
+ if (offset = TS5500_DIO1_13)
+ return TS5500_DIO1_13_IRQ;
+ if (offset = TS5500_DIO2_13)
+ return TS5500_DIO2_13_IRQ;
+ if (offset = TS5500_LCD_RS)
+ return TS5500_LCD_RS_IRQ;
+
+ /* IRQ line may be bridged with another DIO line from the same header */
+ if (dio1_irq && offset >= TS5500_DIO1_0 && offset < TS5500_DIO1_13)
+ return TS5500_DIO1_13_IRQ;
+ if (dio2_irq && offset >= TS5500_DIO2_0 && offset < TS5500_DIO2_13)
+ return TS5500_DIO2_13_IRQ;
+ if (lcd_irq && offset >= TS5500_LCD_0 && offset <= TS5500_LCD_WR)
+ return TS5500_LCD_RS_IRQ;
+
+ return -ENXIO;
+}
+
+static struct gpio_chip ts5500_gpio_chip = {
+ .label = "ts5500-gpio",
+ .owner = THIS_MODULE,
+ .direction_input = ts5500_gpio_direction_input,
+ .get = ts5500_gpio_get,
+ .direction_output = ts5500_gpio_direction_output,
+ .set = ts5500_gpio_set,
+ .to_irq = ts5500_gpio_to_irq,
+ .base = TS5500_DIO1_0,
+};
+
+static void ts5500_gpio_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_gpio_pdev = {
+ .name = "ts5500-gpio",
+ .id = -1,
+ .dev = {
+ .release = ts5500_gpio_release,
+ },
+};
+
+static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
+{
+ int ret;
+ unsigned long flags;
+
+ if (pdev = NULL)
+ return -ENODEV;
+
+ if (!request_region(0x7A, 3, "ts5500-gpio-DIO1")) {
+ dev_err(&pdev->dev, "failed to request I/O port 0x7A-7C\n");
+ return -EBUSY;
+ }
+
+ if (!request_region(0x7D, 3, "ts5500-gpio-DIO2")) {
+ dev_err(&pdev->dev, "failed to request I/O port 0x7D-7F\n");
+ ret = -EBUSY;
+ goto release_dio1;
+ }
+
+ if (use_lcdio && !request_region(0x72, 2, "ts5500-gpio-LCD")) {
+ dev_err(&pdev->dev, "failed to request I/O port 0x72-73\n");
+ ret = -EBUSY;
+ goto release_dio2;
+ }
+
+ if (use_lcdio)
+ ts5500_gpio_chip.ngpio = TS5500_LCD_WR + 1;
+ else
+ ts5500_gpio_chip.ngpio = TS5500_DIO2_13 + 1;
+
+ ret = gpiochip_add(&ts5500_gpio_chip);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register the gpio chip\n");
+ goto release_lcd;
+ }
+
+ /* Enable IRQ generation */
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_set(0x7A, 7); /* DIO1_13 on IRQ7 */
+ port_bit_set(0x7D, 7); /* DIO2_13 on IRQ6 */
+ if (use_lcdio) {
+ port_bit_clear(0x7D, 4); /* LCD header usage as DIO */
+ port_bit_set(0x7D, 6); /* LCD_RS on IRQ1 */
+ }
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ return 0;
+
+release_lcd:
+ if (use_lcdio)
+ release_region(0x72, 2);
+release_dio2:
+ release_region(0x7D, 3);
+release_dio1:
+ release_region(0x7A, 3);
+
+ return ret;
+}
+
+static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
+{
+ int ret;
+ unsigned long flags;
+
+ /* Disable IRQ generation */
+ spin_lock_irqsave(&gpio_lock, flags);
+ port_bit_clear(0x7A, 7);
+ port_bit_clear(0x7D, 7);
+ if (use_lcdio)
+ port_bit_clear(0x7D, 6);
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ release_region(0x7A, 3);
+ release_region(0x7D, 3);
+ if (use_lcdio)
+ release_region(0x72, 2);
+
+ ret = gpiochip_remove(&ts5500_gpio_chip);
+ if (ret)
+ dev_err(&pdev->dev, "failed to remove the gpio chip\n");
+
+ return ret;
+}
+
+static struct platform_driver ts5500_gpio_driver = {
+ .driver = {
+ .name = "ts5500-gpio",
+ .owner = THIS_MODULE,
+ },
+ .probe = ts5500_gpio_probe,
+ .remove = __devexit_p(ts5500_gpio_remove),
+};
+
+static int __init ts5500_gpio_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&ts5500_gpio_driver);
+ if (ret)
+ return ret;
+
+ ret = platform_device_register(&ts5500_gpio_pdev);
+ if (ret) {
+ platform_driver_unregister(&ts5500_gpio_driver);
+ return ret;
+ }
+
+ return 0;
+}
+module_init(ts5500_gpio_init);
+
+static void __exit ts5500_gpio_exit(void)
+{
+ platform_driver_unregister(&ts5500_gpio_driver);
+ platform_device_unregister(&ts5500_gpio_pdev);
+}
+module_exit(ts5500_gpio_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>");
+MODULE_DESCRIPTION("Technologic Systems TS-5500, GPIO/DIO driver");
--
1.7.9.2
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v6 2/3] x86/platform: TS-5500 basic platform support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-04-13 10:37 ` Thomas Gleixner
-1 siblings, 0 replies; 28+ messages in thread
From: Thomas Gleixner @ 2012-04-13 10:37 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, H. Peter Anvin, linux-kernel, lm-sensors,
Guenter Roeck, Jean Delvare, Grant Likely, Linus Walleij
On Thu, 12 Apr 2012, Vivien Didelot wrote:
> +/**
> + * struct ts5500_sbc - TS-5500 SBC main structure
> + * @lock: Read/Write mutex.
What's the point of this mutex ?
AFAICT, it's only ever used in the init path which is serialized by
itself.
> + * @board_id: Board name.
name in an integer ?
> + * @sram: Check SRAM option.
> + * @rs485: Check RS-485 option.
> + * @adc: Check Analog/Digital converter option.
> + * @ereset: Check External Reset option.
> + * @itr: Check Industrial Temperature Range option.
> + * @jumpers: States of jumpers 1-7.
> + */
> +struct ts5500_sbc {
> + struct mutex lock;
> + int board_id;
> + bool sram;
> + bool rs485;
> + bool adc;
> + bool ereset;
> + bool itr;
> + u8 jumpers;
> +};
> +/**
> + * ts5500_bios_signature() - find board signature in BIOS shadow RAM.
> + */
> +static int __init ts5500_bios_signature(void)
> +{
> + void __iomem *bios = ioremap(0xF0000, 0x10000);
> + int i, ret = 0;
ioremaps can fail.
> + for (i = 0; i < ARRAY_SIZE(signatures); i++)
> + if (check_signature(bios + signatures[i].offset,
+ signatures[i].string,
> + strlen(signatures[i].string)))
> + goto found;
> + else
> + pr_notice("Technologic Systems BIOS signature "
> + "'%s' not found at offset %zd\n",
> + signatures[i].string, signatures[i].offset);
> + ret = -ENODEV;
> +found:
> + iounmap(bios);
Uurg, this is convoluted. What's wrong with doing:
int ret = -ENODEV;
for (....) {
if (check_signature()) {
ret = 0;
break;
}
}
That way the code becomes readable and we really do not need a
printout when the kernel is configured for multiple platforms and
runs on a !TS board. Also you would print out that nonsense if your
signature array has more than one entry for each non matching
one. Pretty pointless.
> + tmp = inb(TS5500_PRODUCT_CODE_ADDR);
> + if (tmp != TS5500_PRODUCT_CODE) {
> + pr_err("This platform is not a TS-5500 (found ID 0x%x)\n", tmp);
> + ret = -ENODEV;
> + goto cleanup;
> + }
> + sbc->board_id = tmp;
So we store a constant value in a data structure and the sole purpose
is to display that constant value in sysfs file. Interesting feature.
> +static struct attribute *ts5500_attributes[] = {
> + &dev_attr_id.attr,
> + &dev_attr_sram.attr,
> + &dev_attr_rs485.attr,
> + &dev_attr_adc.attr,
> + &dev_attr_ereset.attr,
> + &dev_attr_itr.attr,
> + &dev_attr_jp1.attr,
> + &dev_attr_jp2.attr,
> + &dev_attr_jp3.attr,
> + &dev_attr_jp4.attr,
> + &dev_attr_jp5.attr,
> + &dev_attr_jp6.attr,
So you create 12 sysfs entries to export boolean features and a
constant value. I don't care much, but this looks like massive
overkill.
> +/* A/D Converter platform device */
> +
> +static int ts5500_adc_convert(u8 ctrl, u16 *raw)
> +{
> + u8 lsb, msb;
> +
> + /* Start convertion (ensure the 3 MSB are set to 0) */
> + outb(ctrl & 0x1F, TS5500_ADC_CONV_INIT_LSB_ADDR);
> +
> + udelay(TS5500_ADC_CONV_DELAY);
> + if (inb(TS5500_ADC_CONV_BUSY_ADDR) & TS5500_ADC_CONV_BUSY)
> + return -EBUSY;
Shouldn't you check the busy bit _BEFORE_ writing into the converter?
Also initiating the conversion and then bailing out if it did not
complete in some micro seconds is kinda strange. What's wrong with
that hardware? And how does it ever recover?
> +static void ts5500_adc_release(struct device *dev)
> +{
> + /* noop */
Very helpful comment.
> +}
> +
> +static struct platform_device ts5500_adc_pdev = {
> + .name = "max197",
> + .id = -1,
> + .dev = {
> + .platform_data = &ts5500_adc_pdata,
> + .release = ts5500_adc_release,
What's the point of this empty release function ? The device is never
released.
Thanks,
tglx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 2/3] x86/platform: TS-5500 basic platform support
@ 2012-04-13 10:37 ` Thomas Gleixner
0 siblings, 0 replies; 28+ messages in thread
From: Thomas Gleixner @ 2012-04-13 10:37 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, H. Peter Anvin, linux-kernel, lm-sensors,
Guenter Roeck, Jean Delvare, Grant Likely, Linus Walleij
On Thu, 12 Apr 2012, Vivien Didelot wrote:
> +/**
> + * struct ts5500_sbc - TS-5500 SBC main structure
> + * @lock: Read/Write mutex.
What's the point of this mutex ?
AFAICT, it's only ever used in the init path which is serialized by
itself.
> + * @board_id: Board name.
name in an integer ?
> + * @sram: Check SRAM option.
> + * @rs485: Check RS-485 option.
> + * @adc: Check Analog/Digital converter option.
> + * @ereset: Check External Reset option.
> + * @itr: Check Industrial Temperature Range option.
> + * @jumpers: States of jumpers 1-7.
> + */
> +struct ts5500_sbc {
> + struct mutex lock;
> + int board_id;
> + bool sram;
> + bool rs485;
> + bool adc;
> + bool ereset;
> + bool itr;
> + u8 jumpers;
> +};
> +/**
> + * ts5500_bios_signature() - find board signature in BIOS shadow RAM.
> + */
> +static int __init ts5500_bios_signature(void)
> +{
> + void __iomem *bios = ioremap(0xF0000, 0x10000);
> + int i, ret = 0;
ioremaps can fail.
> + for (i = 0; i < ARRAY_SIZE(signatures); i++)
> + if (check_signature(bios + signatures[i].offset,
+ signatures[i].string,
> + strlen(signatures[i].string)))
> + goto found;
> + else
> + pr_notice("Technologic Systems BIOS signature "
> + "'%s' not found at offset %zd\n",
> + signatures[i].string, signatures[i].offset);
> + ret = -ENODEV;
> +found:
> + iounmap(bios);
Uurg, this is convoluted. What's wrong with doing:
int ret = -ENODEV;
for (....) {
if (check_signature()) {
ret = 0;
break;
}
}
That way the code becomes readable and we really do not need a
printout when the kernel is configured for multiple platforms and
runs on a !TS board. Also you would print out that nonsense if your
signature array has more than one entry for each non matching
one. Pretty pointless.
> + tmp = inb(TS5500_PRODUCT_CODE_ADDR);
> + if (tmp != TS5500_PRODUCT_CODE) {
> + pr_err("This platform is not a TS-5500 (found ID 0x%x)\n", tmp);
> + ret = -ENODEV;
> + goto cleanup;
> + }
> + sbc->board_id = tmp;
So we store a constant value in a data structure and the sole purpose
is to display that constant value in sysfs file. Interesting feature.
> +static struct attribute *ts5500_attributes[] = {
> + &dev_attr_id.attr,
> + &dev_attr_sram.attr,
> + &dev_attr_rs485.attr,
> + &dev_attr_adc.attr,
> + &dev_attr_ereset.attr,
> + &dev_attr_itr.attr,
> + &dev_attr_jp1.attr,
> + &dev_attr_jp2.attr,
> + &dev_attr_jp3.attr,
> + &dev_attr_jp4.attr,
> + &dev_attr_jp5.attr,
> + &dev_attr_jp6.attr,
So you create 12 sysfs entries to export boolean features and a
constant value. I don't care much, but this looks like massive
overkill.
> +/* A/D Converter platform device */
> +
> +static int ts5500_adc_convert(u8 ctrl, u16 *raw)
> +{
> + u8 lsb, msb;
> +
> + /* Start convertion (ensure the 3 MSB are set to 0) */
> + outb(ctrl & 0x1F, TS5500_ADC_CONV_INIT_LSB_ADDR);
> +
> + udelay(TS5500_ADC_CONV_DELAY);
> + if (inb(TS5500_ADC_CONV_BUSY_ADDR) & TS5500_ADC_CONV_BUSY)
> + return -EBUSY;
Shouldn't you check the busy bit _BEFORE_ writing into the converter?
Also initiating the conversion and then bailing out if it did not
complete in some micro seconds is kinda strange. What's wrong with
that hardware? And how does it ever recover?
> +static void ts5500_adc_release(struct device *dev)
> +{
> + /* noop */
Very helpful comment.
> +}
> +
> +static struct platform_device ts5500_adc_pdev = {
> + .name = "max197",
> + .id = -1,
> + .dev = {
> + .platform_data = &ts5500_adc_pdata,
> + .release = ts5500_adc_release,
What's the point of this empty release function ? The device is never
released.
Thanks,
tglx
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-04-13 19:04 ` Mark Brown
-1 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2012-04-13 19:04 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Grant Likely, Linus Walleij
On Thu, Apr 12, 2012 at 08:28:55PM -0400, Vivien Didelot wrote:
Guess I won't wait for a more thorough off list review :)
> +static void ts5500_gpio_release(struct device *dev)
> +{
> + /* noop */
> +}
So, this really shouldn't be here...
> +static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long flags;
> +
> + if (pdev == NULL)
> + return -ENODEV;
Don't bother, the kernel got seriously confused if this happens.
> + ret = platform_device_register(&ts5500_gpio_pdev);
> + if (ret) {
> + platform_driver_unregister(&ts5500_gpio_driver);
> + return ret;
> + }
...probably what your release function should do is free the device
which should be dynamically allocated here, platform_device_alloc() will
do the right thing for you. This isn't usually an issue for static
platform devices as they are registered from board files which can't be
unloaded.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-04-13 19:04 ` Mark Brown
0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2012-04-13 19:04 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Grant Likely, Linus Walleij
On Thu, Apr 12, 2012 at 08:28:55PM -0400, Vivien Didelot wrote:
Guess I won't wait for a more thorough off list review :)
> +static void ts5500_gpio_release(struct device *dev)
> +{
> + /* noop */
> +}
So, this really shouldn't be here...
> +static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long flags;
> +
> + if (pdev = NULL)
> + return -ENODEV;
Don't bother, the kernel got seriously confused if this happens.
> + ret = platform_device_register(&ts5500_gpio_pdev);
> + if (ret) {
> + platform_driver_unregister(&ts5500_gpio_driver);
> + return ret;
> + }
...probably what your release function should do is free the device
which should be dynamically allocated here, platform_device_alloc() will
do the right thing for you. This isn't usually an issue for static
platform devices as they are registered from board files which can't be
unloaded.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 2/3] x86/platform: TS-5500 basic platform support
2012-04-13 10:37 ` [lm-sensors] " Thomas Gleixner
@ 2012-04-13 20:46 ` Vivien Didelot
-1 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 20:46 UTC (permalink / raw)
To: Thomas Gleixner
Cc: x86, Ingo Molnar, H. Peter Anvin, linux-kernel, lm-sensors,
Guenter Roeck, Jean Delvare, Grant Likely, Linus Walleij
Hi,
Thanks for the comments.
On Fri, 2012-04-13 at 12:37 +0200, Thomas Gleixner wrote:
> On Thu, 12 Apr 2012, Vivien Didelot wrote:
> > +/**
> > + * struct ts5500_sbc - TS-5500 SBC main structure
> > + * @lock: Read/Write mutex.
>
> What's the point of this mutex ?
>
> AFAICT, it's only ever used in the init path which is serialized by
> itself.
You're right, I'll remove it.
>
> > + * @board_id: Board name.
>
> name in an integer ?
>
> > + * @sram: Check SRAM option.
> > + * @rs485: Check RS-485 option.
> > + * @adc: Check Analog/Digital converter option.
> > + * @ereset: Check External Reset option.
> > + * @itr: Check Industrial Temperature Range option.
> > + * @jumpers: States of jumpers 1-7.
> > + */
> > +struct ts5500_sbc {
> > + struct mutex lock;
> > + int board_id;
> > + bool sram;
> > + bool rs485;
> > + bool adc;
> > + bool ereset;
> > + bool itr;
> > + u8 jumpers;
> > +};
>
>
> > +/**
> > + * ts5500_bios_signature() - find board signature in BIOS shadow RAM.
> > + */
> > +static int __init ts5500_bios_signature(void)
> > +{
> > + void __iomem *bios = ioremap(0xF0000, 0x10000);
> > + int i, ret = 0;
>
> ioremaps can fail.
>
> > + for (i = 0; i < ARRAY_SIZE(signatures); i++)
> > + if (check_signature(bios + signatures[i].offset,
> + signatures[i].string,
> > + strlen(signatures[i].string)))
> > + goto found;
> > + else
> > + pr_notice("Technologic Systems BIOS signature "
> > + "'%s' not found at offset %zd\n",
> > + signatures[i].string, signatures[i].offset);
> > + ret = -ENODEV;
> > +found:
> > + iounmap(bios);
>
> Uurg, this is convoluted. What's wrong with doing:
>
> int ret = -ENODEV;
>
> for (....) {
> if (check_signature()) {
> ret = 0;
> break;
> }
> }
>
> That way the code becomes readable and we really do not need a
> printout when the kernel is configured for multiple platforms and
> runs on a !TS board. Also you would print out that nonsense if your
> signature array has more than one entry for each non matching
> one. Pretty pointless.
Got it.
>
> > + tmp = inb(TS5500_PRODUCT_CODE_ADDR);
> > + if (tmp != TS5500_PRODUCT_CODE) {
> > + pr_err("This platform is not a TS-5500 (found ID 0x%x)\n", tmp);
> > + ret = -ENODEV;
> > + goto cleanup;
> > + }
> > + sbc->board_id = tmp;
>
> So we store a constant value in a data structure and the sole purpose
> is to display that constant value in sysfs file. Interesting feature.
I plan to add support for a few variants of this board which differ at
this register level.
>
> > +static struct attribute *ts5500_attributes[] = {
> > + &dev_attr_id.attr,
> > + &dev_attr_sram.attr,
> > + &dev_attr_rs485.attr,
> > + &dev_attr_adc.attr,
> > + &dev_attr_ereset.attr,
> > + &dev_attr_itr.attr,
> > + &dev_attr_jp1.attr,
> > + &dev_attr_jp2.attr,
> > + &dev_attr_jp3.attr,
> > + &dev_attr_jp4.attr,
> > + &dev_attr_jp5.attr,
> > + &dev_attr_jp6.attr,
>
> So you create 12 sysfs entries to export boolean features and a
> constant value. I don't care much, but this looks like massive
> overkill.
Ok, I'll go for a simple "settings" sysfs attribute.
>
> > +/* A/D Converter platform device */
> > +
> > +static int ts5500_adc_convert(u8 ctrl, u16 *raw)
> > +{
> > + u8 lsb, msb;
> > +
> > + /* Start convertion (ensure the 3 MSB are set to 0) */
> > + outb(ctrl & 0x1F, TS5500_ADC_CONV_INIT_LSB_ADDR);
> > +
> > + udelay(TS5500_ADC_CONV_DELAY);
> > + if (inb(TS5500_ADC_CONV_BUSY_ADDR) & TS5500_ADC_CONV_BUSY)
> > + return -EBUSY;
>
> Shouldn't you check the busy bit _BEFORE_ writing into the converter?
>
> Also initiating the conversion and then bailing out if it did not
> complete in some micro seconds is kinda strange. What's wrong with
> that hardware? And how does it ever recover?
The manufacturer has CPLD logic driving the actual A/D converter. The
documentation says they guarantee it must complete within x microseconds
otherwise you have to re-initiate a conversion.
I'll add a proper comment explaining this.
>
> > +static void ts5500_adc_release(struct device *dev)
> > +{
> > + /* noop */
>
> Very helpful comment.
>
> > +}
> > +
> > +static struct platform_device ts5500_adc_pdev = {
> > + .name = "max197",
> > + .id = -1,
> > + .dev = {
> > + .platform_data = &ts5500_adc_pdata,
> > + .release = ts5500_adc_release,
>
> What's the point of this empty release function ? The device is never
> released.
Ok.
Thanks,
Vivien
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 2/3] x86/platform: TS-5500 basic platform support
@ 2012-04-13 20:46 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-04-13 20:46 UTC (permalink / raw)
To: Thomas Gleixner
Cc: x86, Ingo Molnar, H. Peter Anvin, linux-kernel, lm-sensors,
Guenter Roeck, Jean Delvare, Grant Likely, Linus Walleij
Hi,
Thanks for the comments.
On Fri, 2012-04-13 at 12:37 +0200, Thomas Gleixner wrote:
> On Thu, 12 Apr 2012, Vivien Didelot wrote:
> > +/**
> > + * struct ts5500_sbc - TS-5500 SBC main structure
> > + * @lock: Read/Write mutex.
>
> What's the point of this mutex ?
>
> AFAICT, it's only ever used in the init path which is serialized by
> itself.
You're right, I'll remove it.
>
> > + * @board_id: Board name.
>
> name in an integer ?
>
> > + * @sram: Check SRAM option.
> > + * @rs485: Check RS-485 option.
> > + * @adc: Check Analog/Digital converter option.
> > + * @ereset: Check External Reset option.
> > + * @itr: Check Industrial Temperature Range option.
> > + * @jumpers: States of jumpers 1-7.
> > + */
> > +struct ts5500_sbc {
> > + struct mutex lock;
> > + int board_id;
> > + bool sram;
> > + bool rs485;
> > + bool adc;
> > + bool ereset;
> > + bool itr;
> > + u8 jumpers;
> > +};
>
>
> > +/**
> > + * ts5500_bios_signature() - find board signature in BIOS shadow RAM.
> > + */
> > +static int __init ts5500_bios_signature(void)
> > +{
> > + void __iomem *bios = ioremap(0xF0000, 0x10000);
> > + int i, ret = 0;
>
> ioremaps can fail.
>
> > + for (i = 0; i < ARRAY_SIZE(signatures); i++)
> > + if (check_signature(bios + signatures[i].offset,
> + signatures[i].string,
> > + strlen(signatures[i].string)))
> > + goto found;
> > + else
> > + pr_notice("Technologic Systems BIOS signature "
> > + "'%s' not found at offset %zd\n",
> > + signatures[i].string, signatures[i].offset);
> > + ret = -ENODEV;
> > +found:
> > + iounmap(bios);
>
> Uurg, this is convoluted. What's wrong with doing:
>
> int ret = -ENODEV;
>
> for (....) {
> if (check_signature()) {
> ret = 0;
> break;
> }
> }
>
> That way the code becomes readable and we really do not need a
> printout when the kernel is configured for multiple platforms and
> runs on a !TS board. Also you would print out that nonsense if your
> signature array has more than one entry for each non matching
> one. Pretty pointless.
Got it.
>
> > + tmp = inb(TS5500_PRODUCT_CODE_ADDR);
> > + if (tmp != TS5500_PRODUCT_CODE) {
> > + pr_err("This platform is not a TS-5500 (found ID 0x%x)\n", tmp);
> > + ret = -ENODEV;
> > + goto cleanup;
> > + }
> > + sbc->board_id = tmp;
>
> So we store a constant value in a data structure and the sole purpose
> is to display that constant value in sysfs file. Interesting feature.
I plan to add support for a few variants of this board which differ at
this register level.
>
> > +static struct attribute *ts5500_attributes[] = {
> > + &dev_attr_id.attr,
> > + &dev_attr_sram.attr,
> > + &dev_attr_rs485.attr,
> > + &dev_attr_adc.attr,
> > + &dev_attr_ereset.attr,
> > + &dev_attr_itr.attr,
> > + &dev_attr_jp1.attr,
> > + &dev_attr_jp2.attr,
> > + &dev_attr_jp3.attr,
> > + &dev_attr_jp4.attr,
> > + &dev_attr_jp5.attr,
> > + &dev_attr_jp6.attr,
>
> So you create 12 sysfs entries to export boolean features and a
> constant value. I don't care much, but this looks like massive
> overkill.
Ok, I'll go for a simple "settings" sysfs attribute.
>
> > +/* A/D Converter platform device */
> > +
> > +static int ts5500_adc_convert(u8 ctrl, u16 *raw)
> > +{
> > + u8 lsb, msb;
> > +
> > + /* Start convertion (ensure the 3 MSB are set to 0) */
> > + outb(ctrl & 0x1F, TS5500_ADC_CONV_INIT_LSB_ADDR);
> > +
> > + udelay(TS5500_ADC_CONV_DELAY);
> > + if (inb(TS5500_ADC_CONV_BUSY_ADDR) & TS5500_ADC_CONV_BUSY)
> > + return -EBUSY;
>
> Shouldn't you check the busy bit _BEFORE_ writing into the converter?
>
> Also initiating the conversion and then bailing out if it did not
> complete in some micro seconds is kinda strange. What's wrong with
> that hardware? And how does it ever recover?
The manufacturer has CPLD logic driving the actual A/D converter. The
documentation says they guarantee it must complete within x microseconds
otherwise you have to re-initiate a conversion.
I'll add a proper comment explaining this.
>
> > +static void ts5500_adc_release(struct device *dev)
> > +{
> > + /* noop */
>
> Very helpful comment.
>
> > +}
> > +
> > +static struct platform_device ts5500_adc_pdev = {
> > + .name = "max197",
> > + .id = -1,
> > + .dev = {
> > + .platform_data = &ts5500_adc_pdata,
> > + .release = ts5500_adc_release,
>
> What's the point of this empty release function ? The device is never
> released.
Ok.
Thanks,
Vivien
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 1/3] hwmon: Maxim MAX197 support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-04-14 0:46 ` Guenter Roeck
-1 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2012-04-14 0:46 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare, Grant Likely, Linus Walleij
On Thu, Apr 12, 2012 at 08:28:53PM -0400, Vivien Didelot wrote:
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
[ ... ]
> +
> +/**
> + * max197_show_input() - Show channel input
> + *
> + * Function called on read access on in{0,1,2,3,4,5,6,7}_input
> + */
> +static ssize_t max197_show_input(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + struct max197_chip *chip = dev_get_drvdata(dev);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> + int channel = attr->index;
> + u16 raw;
> + s32 scaled;
> + int ret;
> +
> + if (mutex_lock_interruptible(&chip->lock))
> + return -ERESTARTSYS;
> +
> + ret = chip->pdata->convert(chip->ctrl_bytes[channel], &raw);
> + if (ret) {
> + dev_err(dev, "conversion failed\n");
> + goto unlock;
> + }
Hi Vivien,
thinking about this, you can simplify the API to
ret = chip->pdata->convert(chip->ctrl_bytes[channel]);
if (ret < 0) {
dev_err(dev, "conversion failed\n");
goto unlock;
}
scaled = ret;
This overloads the return value with both error code (< 0) and return value (unsigned, >= 0).
This would make the code a bit simpler and smaller.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 1/3] hwmon: Maxim MAX197 support
@ 2012-04-14 0:46 ` Guenter Roeck
0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2012-04-14 0:46 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare, Grant Likely, Linus Walleij
On Thu, Apr 12, 2012 at 08:28:53PM -0400, Vivien Didelot wrote:
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
[ ... ]
> +
> +/**
> + * max197_show_input() - Show channel input
> + *
> + * Function called on read access on in{0,1,2,3,4,5,6,7}_input
> + */
> +static ssize_t max197_show_input(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + struct max197_chip *chip = dev_get_drvdata(dev);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> + int channel = attr->index;
> + u16 raw;
> + s32 scaled;
> + int ret;
> +
> + if (mutex_lock_interruptible(&chip->lock))
> + return -ERESTARTSYS;
> +
> + ret = chip->pdata->convert(chip->ctrl_bytes[channel], &raw);
> + if (ret) {
> + dev_err(dev, "conversion failed\n");
> + goto unlock;
> + }
Hi Vivien,
thinking about this, you can simplify the API to
ret = chip->pdata->convert(chip->ctrl_bytes[channel]);
if (ret < 0) {
dev_err(dev, "conversion failed\n");
goto unlock;
}
scaled = ret;
This overloads the return value with both error code (< 0) and return value (unsigned, >= 0).
This would make the code a bit simpler and smaller.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
@ 2012-05-17 21:06 ` Grant Likely
-1 siblings, 0 replies; 28+ messages in thread
From: Grant Likely @ 2012-05-17 21:06 UTC (permalink / raw)
To: Vivien Didelot, x86
Cc: Jerome Oufella, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Linus Walleij, Vivien Didelot
On Thu, 12 Apr 2012 20:28:55 -0400, Vivien Didelot <vivien.didelot@savoirfairelinux.com> wrote:
> From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
>
> Signed-off-by: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> MAINTAINERS | 2 +
> arch/x86/include/asm/ts5500.h | 62 ++++++++
Why the separate header file? What will use these defines? I
normally expect driver-specific defines to be in the driver .c file
directly; particularly for things like gpio drivers which should be a
generic interface that doesn't need to export symbols.
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-ts5500.c | 347 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 419 insertions(+)
> create mode 100644 arch/x86/include/asm/ts5500.h
> create mode 100644 drivers/gpio/gpio-ts5500.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b126129..b7b5d95 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6624,6 +6624,8 @@ TECHNOLOGIC SYSTEMS TS-5500 MACHINE SUPPORT
> M: Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>
> S: Maintained
> F: arch/x86/platform/ts5500/
> +F: arch/x86/include/asm/ts5500.h
> +F: drivers/gpio/gpio-ts5500.c
>
> TEGRA SUPPORT
> M: Colin Cross <ccross@android.com>
> diff --git a/arch/x86/include/asm/ts5500.h b/arch/x86/include/asm/ts5500.h
> new file mode 100644
> index 0000000..baf136c
> --- /dev/null
> +++ b/arch/x86/include/asm/ts5500.h
> @@ -0,0 +1,62 @@
> +/*
> + * Technologic Systems TS-5500 GPIO (DIO) definitions
> + *
> + * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
> + * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _TS5500_H
> +#define _TS5500_H
> +
> +#define TS5500_DIO1_0 0
> +#define TS5500_DIO1_1 1
> +#define TS5500_DIO1_2 2
> +#define TS5500_DIO1_3 3
> +#define TS5500_DIO1_4 4
> +#define TS5500_DIO1_5 5
> +#define TS5500_DIO1_6 6
> +#define TS5500_DIO1_7 7
> +#define TS5500_DIO1_8 8
> +#define TS5500_DIO1_9 9
> +#define TS5500_DIO1_10 10
> +#define TS5500_DIO1_11 11
> +#define TS5500_DIO1_12 12
> +#define TS5500_DIO1_13 13
> +
> +#define TS5500_DIO2_0 14
> +#define TS5500_DIO2_1 15
> +#define TS5500_DIO2_2 16
> +#define TS5500_DIO2_3 17
> +#define TS5500_DIO2_4 18
> +#define TS5500_DIO2_5 19
> +#define TS5500_DIO2_6 20
> +#define TS5500_DIO2_7 21
> +#define TS5500_DIO2_8 22
> +#define TS5500_DIO2_9 23
> +#define TS5500_DIO2_10 24
> +#define TS5500_DIO2_11 25
> +/* #define TS5500_DIO2_12 - Keep commented out as it simply doesn't exist. */
> +#define TS5500_DIO2_13 26
> +
> +#define TS5500_LCD_0 27
> +#define TS5500_LCD_1 28
> +#define TS5500_LCD_2 29
> +#define TS5500_LCD_3 30
> +#define TS5500_LCD_4 31
> +#define TS5500_LCD_5 32
> +#define TS5500_LCD_6 33
> +#define TS5500_LCD_7 34
> +#define TS5500_LCD_EN 35
> +#define TS5500_LCD_RS 36
> +#define TS5500_LCD_WR 37
> +
> +/* Lines that can trigger IRQs */
> +#define TS5500_DIO1_13_IRQ 7
> +#define TS5500_DIO2_13_IRQ 6
> +#define TS5500_LCD_RS_IRQ 1
> +
> +#endif /* _TS5500_H */
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index edadbda..a19b0f3 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -319,6 +319,13 @@ config GPIO_TPS65912
> help
> This driver supports TPS65912 gpio chip
>
> +config GPIO_TS5500
> + tristate "TS-5500 GPIOs"
> + depends on TS5500
> + help
> + This driver supports the DIO headers for GPIO usage on the Technologic
> + Systems TS-5500 platform.
> +
> config GPIO_TWL4030
> tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
> depends on TWL4030_CORE
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 007f54b..30cbd03 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_TIMBERDALE) += gpio-timberdale.o
> obj-$(CONFIG_ARCH_DAVINCI_TNETV107X) += gpio-tnetv107x.o
> obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
> obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
> +obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
> obj-$(CONFIG_GPIO_TWL4030) += gpio-twl4030.o
> obj-$(CONFIG_GPIO_UCB1400) += gpio-ucb1400.o
> obj-$(CONFIG_GPIO_VR41XX) += gpio-vr41xx.o
> diff --git a/drivers/gpio/gpio-ts5500.c b/drivers/gpio/gpio-ts5500.c
> new file mode 100644
> index 0000000..01f34d8
> --- /dev/null
> +++ b/drivers/gpio/gpio-ts5500.c
> @@ -0,0 +1,347 @@
> +/*
> + * GPIO (DIO) driver for Technologic Systems TS-5500
> + *
> + * TS-5500 board has 38 GPIOs referred to as DIOs in the product's literature.
> + *
> + * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
> + * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
> + * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/gpio.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <asm/ts5500.h>
> +
> +static int dio1_irq = 1;
> +module_param(dio1_irq, int, 0644);
> +MODULE_PARM_DESC(dio1_irq, "Enable usage of IRQ7 for any DIO1 line"
> + " (default 1)");
> +
> +static int dio2_irq;
> +module_param(dio2_irq, int, 0644);
> +MODULE_PARM_DESC(dio2_irq, "Enable usage of IRQ6 for any DIO2 line"
> + " (default 0)");
> +
> +static int lcd_irq;
> +module_param(lcd_irq, int, 0644);
> +MODULE_PARM_DESC(lcd_irq, "Enable usage of IRQ1 for any LCD line"
> + " (default 0)");
> +
> +static int use_lcdio;
> +module_param(use_lcdio, int, 0644);
> +MODULE_PARM_DESC(use_lcdio, "Enable usage of the LCD header for DIO operation"
> + " (default 0)");
> +
> +static DEFINE_SPINLOCK(gpio_lock);
> +
> +static inline void port_bit_set(u8 addr, int bit)
> +{
> + u8 var;
> +
> + var = inb(addr);
> + var |= (1 << bit);
> + outb(var, addr);
> +}
> +
> +static inline void port_bit_clear(u8 addr, int bit)
> +{
> + u8 var;
> +
> + var = inb(addr);
> + var &= ~(1 << bit);
> + outb(var, addr);
> +}
> +
> +/* "DIO" line to IO port mapping table for line's value */
> +static const unsigned long line_to_port_map[] = {
> + 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, /* DIO1_[0-7] */
> + 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, /* DIO1_[8-13] */
> + 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, /* DIO2_[0-7] */
> + 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, /* DIO2_[8-13] */
> + 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, /* LCD_[0-7] */
> + 0x73, 0x73, 0x73 /* LCD_{EN,RS,WR} */
> +};
> +
> +/* "DIO" line to IO port's bit map for line's value */
> +static const int line_to_bit_map[] = {
> + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO1_[0-7] */
> + 0, 1, 2, 3, 4, 5, /* DIO1_[8-13] */
> + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO2_[0-7] */
> + 0, 1, 2, 3, 4, 5, /* DIO2_[8-13] */
> + 0, 1, 2, 3, 4, 5, 6, 7, /* LCD_[0-7] */
> + 0, 7, 6 /* LCD_{EN,RS,WR} */
> +};
> +
> +/* "DIO" line's direction control mapping table */
> +static const unsigned long line_to_dir_map[] = {
> + 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, /* DIO1_[0-7] */
> + 0x7A, 0x7A, 0x7A, 0x7A, 0, 0, /* DIO1_[8-13] */
> + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* DIO2_[0-7] */
> + 0x7D, 0x7D, 0x7D, 0x7D, 0, 0, /* DIO2_[8-13] */
> + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* LCD_[0-7] */
> + 0, 0, 0 /* LCD_{EN,RS,WR} */
> +};
> +
> +/* "DIO" line's direction control bit-mapping table */
> +static const int line_to_dir_bit_map[] = {
> + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO1_[0-7] */
> + 5, 5, 5, 5, -1, -1, /* DIO1_[8-13] */
> + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO2_[0-7] */
> + 5, 5, 5, 5, -1, -1, /* DIO2_[8-13] */
> + 2, 2, 2, 2, 3, 3, 3, 3, /* LCD_[0-7] */
> + -1, -1, -1 /* LCD_{EN,RS,WR} */
> +};
Splitting up this data into 4 arrays seems odd. I think it would be
better to define a single table with a tuple for each line.
> +
> +static int ts5500_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> + unsigned long dir_reg, flags;
> + int dir_bit;
> +
> + /* Some lines cannot be configured as input */
> + if (offset == TS5500_LCD_EN)
> + return -ENXIO;
> +
> + dir_reg = line_to_dir_map[offset];
> + dir_bit = line_to_dir_bit_map[offset];
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_clear(dir_reg, dir_bit);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int ts5500_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + unsigned long ioaddr;
> + int bitno;
> +
> + ioaddr = line_to_port_map[offset];
> + bitno = line_to_bit_map[offset];
> +
> + return (inb(ioaddr) >> bitno) & 0x1;
> +}
> +
> +static int ts5500_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
> + int val)
> +{
> + unsigned long dir_reg, ioaddr, flags;
> + int dir_bit, bitno;
> +
> + /* Some lines cannot be configured as output */
> + switch (offset) {
> + case TS5500_DIO1_12:
> + case TS5500_DIO1_13:
> + case TS5500_DIO2_13:
> + case TS5500_LCD_RS:
> + case TS5500_LCD_WR:
> + return -ENXIO;
This would also do well in a 'flags' field in the per-line data table
as I described above.
> + default:
> + break;
> + }
> +
> + dir_reg = line_to_dir_map[offset];
> + dir_bit = line_to_dir_bit_map[offset];
> + ioaddr = line_to_port_map[offset];
> + bitno = line_to_bit_map[offset];
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_set(dir_reg, dir_bit);
> + if (val)
> + port_bit_set(ioaddr, bitno);
> + else
> + port_bit_clear(ioaddr, bitno);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static void ts5500_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> +{
> + int bitno;
> + unsigned long ioaddr, flags;
> +
> + ioaddr = line_to_port_map[offset];
> + bitno = line_to_bit_map[offset];
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + if (val)
> + port_bit_set(ioaddr, bitno);
> + else
> + port_bit_clear(ioaddr, bitno);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +}
> +
> +static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + /* Only a few lines are IRQ-capable */
> + if (offset == TS5500_DIO1_13)
> + return TS5500_DIO1_13_IRQ;
> + if (offset == TS5500_DIO2_13)
> + return TS5500_DIO2_13_IRQ;
> + if (offset == TS5500_LCD_RS)
> + return TS5500_LCD_RS_IRQ;
> +
> + /* IRQ line may be bridged with another DIO line from the same header */
> + if (dio1_irq && offset >= TS5500_DIO1_0 && offset < TS5500_DIO1_13)
> + return TS5500_DIO1_13_IRQ;
> + if (dio2_irq && offset >= TS5500_DIO2_0 && offset < TS5500_DIO2_13)
> + return TS5500_DIO2_13_IRQ;
> + if (lcd_irq && offset >= TS5500_LCD_0 && offset <= TS5500_LCD_WR)
> + return TS5500_LCD_RS_IRQ;
Also candidate for putting in the table.
> +
> + return -ENXIO;
> +}
> +
> +static struct gpio_chip ts5500_gpio_chip = {
> + .label = "ts5500-gpio",
> + .owner = THIS_MODULE,
> + .direction_input = ts5500_gpio_direction_input,
> + .get = ts5500_gpio_get,
> + .direction_output = ts5500_gpio_direction_output,
> + .set = ts5500_gpio_set,
> + .to_irq = ts5500_gpio_to_irq,
> + .base = TS5500_DIO1_0,
Don't hard code the GPIO base. It should be dynamically assigned by
using '-1' here.
> +};
> +
> +static void ts5500_gpio_release(struct device *dev)
> +{
> + /* noop */
> +}
> +
> +static struct platform_device ts5500_gpio_pdev = {
> + .name = "ts5500-gpio",
> + .id = -1,
> + .dev = {
> + .release = ts5500_gpio_release,
> + },
> +};
> +
> +static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long flags;
> +
> + if (pdev == NULL)
> + return -ENODEV;
> +
> + if (!request_region(0x7A, 3, "ts5500-gpio-DIO1")) {
> + dev_err(&pdev->dev, "failed to request I/O port 0x7A-7C\n");
> + return -EBUSY;
> + }
> +
> + if (!request_region(0x7D, 3, "ts5500-gpio-DIO2")) {
> + dev_err(&pdev->dev, "failed to request I/O port 0x7D-7F\n");
> + ret = -EBUSY;
> + goto release_dio1;
> + }
> +
> + if (use_lcdio && !request_region(0x72, 2, "ts5500-gpio-LCD")) {
> + dev_err(&pdev->dev, "failed to request I/O port 0x72-73\n");
> + ret = -EBUSY;
> + goto release_dio2;
> + }
> +
> + if (use_lcdio)
> + ts5500_gpio_chip.ngpio = TS5500_LCD_WR + 1;
> + else
> + ts5500_gpio_chip.ngpio = TS5500_DIO2_13 + 1;
> +
> + ret = gpiochip_add(&ts5500_gpio_chip);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register the gpio chip\n");
> + goto release_lcd;
> + }
> +
> + /* Enable IRQ generation */
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_set(0x7A, 7); /* DIO1_13 on IRQ7 */
> + port_bit_set(0x7D, 7); /* DIO2_13 on IRQ6 */
> + if (use_lcdio) {
> + port_bit_clear(0x7D, 4); /* LCD header usage as DIO */
> + port_bit_set(0x7D, 6); /* LCD_RS on IRQ1 */
> + }
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +
> +release_lcd:
> + if (use_lcdio)
> + release_region(0x72, 2);
> +release_dio2:
> + release_region(0x7D, 3);
> +release_dio1:
> + release_region(0x7A, 3);
> +
> + return ret;
> +}
> +
> +static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long flags;
> +
> + /* Disable IRQ generation */
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_clear(0x7A, 7);
> + port_bit_clear(0x7D, 7);
> + if (use_lcdio)
> + port_bit_clear(0x7D, 6);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + release_region(0x7A, 3);
> + release_region(0x7D, 3);
> + if (use_lcdio)
> + release_region(0x72, 2);
> +
> + ret = gpiochip_remove(&ts5500_gpio_chip);
> + if (ret)
> + dev_err(&pdev->dev, "failed to remove the gpio chip\n");
> +
> + return ret;
> +}
> +
> +static struct platform_driver ts5500_gpio_driver = {
> + .driver = {
> + .name = "ts5500-gpio",
> + .owner = THIS_MODULE,
> + },
> + .probe = ts5500_gpio_probe,
> + .remove = __devexit_p(ts5500_gpio_remove),
> +};
> +
> +static int __init ts5500_gpio_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&ts5500_gpio_driver);
> + if (ret)
> + return ret;
> +
> + ret = platform_device_register(&ts5500_gpio_pdev);
> + if (ret) {
> + platform_driver_unregister(&ts5500_gpio_driver);
> + return ret;
> + }
As already discussed.... Bleah!! :-)
Please move the device registration to platform setup code.
> +
> + return 0;
> +}
> +module_init(ts5500_gpio_init);
> +
> +static void __exit ts5500_gpio_exit(void)
> +{
> + platform_driver_unregister(&ts5500_gpio_driver);
> + platform_device_unregister(&ts5500_gpio_pdev);
> +}
> +module_exit(ts5500_gpio_exit);
module_platform_driver() will replace some of the boilerplate.
g.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-05-17 21:06 ` Grant Likely
0 siblings, 0 replies; 28+ messages in thread
From: Grant Likely @ 2012-05-17 21:06 UTC (permalink / raw)
To: Vivien Didelot, x86
Cc: Jerome Oufella, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Linus Walleij, Vivien Didelot
On Thu, 12 Apr 2012 20:28:55 -0400, Vivien Didelot <vivien.didelot@savoirfairelinux.com> wrote:
> From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
>
> Signed-off-by: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> MAINTAINERS | 2 +
> arch/x86/include/asm/ts5500.h | 62 ++++++++
Why the separate header file? What will use these defines? I
normally expect driver-specific defines to be in the driver .c file
directly; particularly for things like gpio drivers which should be a
generic interface that doesn't need to export symbols.
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-ts5500.c | 347 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 419 insertions(+)
> create mode 100644 arch/x86/include/asm/ts5500.h
> create mode 100644 drivers/gpio/gpio-ts5500.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b126129..b7b5d95 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6624,6 +6624,8 @@ TECHNOLOGIC SYSTEMS TS-5500 MACHINE SUPPORT
> M: Savoir-faire Linux Inc. <kernel@savoirfairelinux.com>
> S: Maintained
> F: arch/x86/platform/ts5500/
> +F: arch/x86/include/asm/ts5500.h
> +F: drivers/gpio/gpio-ts5500.c
>
> TEGRA SUPPORT
> M: Colin Cross <ccross@android.com>
> diff --git a/arch/x86/include/asm/ts5500.h b/arch/x86/include/asm/ts5500.h
> new file mode 100644
> index 0000000..baf136c
> --- /dev/null
> +++ b/arch/x86/include/asm/ts5500.h
> @@ -0,0 +1,62 @@
> +/*
> + * Technologic Systems TS-5500 GPIO (DIO) definitions
> + *
> + * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
> + * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _TS5500_H
> +#define _TS5500_H
> +
> +#define TS5500_DIO1_0 0
> +#define TS5500_DIO1_1 1
> +#define TS5500_DIO1_2 2
> +#define TS5500_DIO1_3 3
> +#define TS5500_DIO1_4 4
> +#define TS5500_DIO1_5 5
> +#define TS5500_DIO1_6 6
> +#define TS5500_DIO1_7 7
> +#define TS5500_DIO1_8 8
> +#define TS5500_DIO1_9 9
> +#define TS5500_DIO1_10 10
> +#define TS5500_DIO1_11 11
> +#define TS5500_DIO1_12 12
> +#define TS5500_DIO1_13 13
> +
> +#define TS5500_DIO2_0 14
> +#define TS5500_DIO2_1 15
> +#define TS5500_DIO2_2 16
> +#define TS5500_DIO2_3 17
> +#define TS5500_DIO2_4 18
> +#define TS5500_DIO2_5 19
> +#define TS5500_DIO2_6 20
> +#define TS5500_DIO2_7 21
> +#define TS5500_DIO2_8 22
> +#define TS5500_DIO2_9 23
> +#define TS5500_DIO2_10 24
> +#define TS5500_DIO2_11 25
> +/* #define TS5500_DIO2_12 - Keep commented out as it simply doesn't exist. */
> +#define TS5500_DIO2_13 26
> +
> +#define TS5500_LCD_0 27
> +#define TS5500_LCD_1 28
> +#define TS5500_LCD_2 29
> +#define TS5500_LCD_3 30
> +#define TS5500_LCD_4 31
> +#define TS5500_LCD_5 32
> +#define TS5500_LCD_6 33
> +#define TS5500_LCD_7 34
> +#define TS5500_LCD_EN 35
> +#define TS5500_LCD_RS 36
> +#define TS5500_LCD_WR 37
> +
> +/* Lines that can trigger IRQs */
> +#define TS5500_DIO1_13_IRQ 7
> +#define TS5500_DIO2_13_IRQ 6
> +#define TS5500_LCD_RS_IRQ 1
> +
> +#endif /* _TS5500_H */
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index edadbda..a19b0f3 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -319,6 +319,13 @@ config GPIO_TPS65912
> help
> This driver supports TPS65912 gpio chip
>
> +config GPIO_TS5500
> + tristate "TS-5500 GPIOs"
> + depends on TS5500
> + help
> + This driver supports the DIO headers for GPIO usage on the Technologic
> + Systems TS-5500 platform.
> +
> config GPIO_TWL4030
> tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
> depends on TWL4030_CORE
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 007f54b..30cbd03 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_TIMBERDALE) += gpio-timberdale.o
> obj-$(CONFIG_ARCH_DAVINCI_TNETV107X) += gpio-tnetv107x.o
> obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
> obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
> +obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
> obj-$(CONFIG_GPIO_TWL4030) += gpio-twl4030.o
> obj-$(CONFIG_GPIO_UCB1400) += gpio-ucb1400.o
> obj-$(CONFIG_GPIO_VR41XX) += gpio-vr41xx.o
> diff --git a/drivers/gpio/gpio-ts5500.c b/drivers/gpio/gpio-ts5500.c
> new file mode 100644
> index 0000000..01f34d8
> --- /dev/null
> +++ b/drivers/gpio/gpio-ts5500.c
> @@ -0,0 +1,347 @@
> +/*
> + * GPIO (DIO) driver for Technologic Systems TS-5500
> + *
> + * TS-5500 board has 38 GPIOs referred to as DIOs in the product's literature.
> + *
> + * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
> + * Jerome Oufella <jerome.oufella@savoirfairelinux.com>
> + * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/gpio.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <asm/ts5500.h>
> +
> +static int dio1_irq = 1;
> +module_param(dio1_irq, int, 0644);
> +MODULE_PARM_DESC(dio1_irq, "Enable usage of IRQ7 for any DIO1 line"
> + " (default 1)");
> +
> +static int dio2_irq;
> +module_param(dio2_irq, int, 0644);
> +MODULE_PARM_DESC(dio2_irq, "Enable usage of IRQ6 for any DIO2 line"
> + " (default 0)");
> +
> +static int lcd_irq;
> +module_param(lcd_irq, int, 0644);
> +MODULE_PARM_DESC(lcd_irq, "Enable usage of IRQ1 for any LCD line"
> + " (default 0)");
> +
> +static int use_lcdio;
> +module_param(use_lcdio, int, 0644);
> +MODULE_PARM_DESC(use_lcdio, "Enable usage of the LCD header for DIO operation"
> + " (default 0)");
> +
> +static DEFINE_SPINLOCK(gpio_lock);
> +
> +static inline void port_bit_set(u8 addr, int bit)
> +{
> + u8 var;
> +
> + var = inb(addr);
> + var |= (1 << bit);
> + outb(var, addr);
> +}
> +
> +static inline void port_bit_clear(u8 addr, int bit)
> +{
> + u8 var;
> +
> + var = inb(addr);
> + var &= ~(1 << bit);
> + outb(var, addr);
> +}
> +
> +/* "DIO" line to IO port mapping table for line's value */
> +static const unsigned long line_to_port_map[] = {
> + 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, /* DIO1_[0-7] */
> + 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, /* DIO1_[8-13] */
> + 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, /* DIO2_[0-7] */
> + 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, /* DIO2_[8-13] */
> + 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, /* LCD_[0-7] */
> + 0x73, 0x73, 0x73 /* LCD_{EN,RS,WR} */
> +};
> +
> +/* "DIO" line to IO port's bit map for line's value */
> +static const int line_to_bit_map[] = {
> + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO1_[0-7] */
> + 0, 1, 2, 3, 4, 5, /* DIO1_[8-13] */
> + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO2_[0-7] */
> + 0, 1, 2, 3, 4, 5, /* DIO2_[8-13] */
> + 0, 1, 2, 3, 4, 5, 6, 7, /* LCD_[0-7] */
> + 0, 7, 6 /* LCD_{EN,RS,WR} */
> +};
> +
> +/* "DIO" line's direction control mapping table */
> +static const unsigned long line_to_dir_map[] = {
> + 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, /* DIO1_[0-7] */
> + 0x7A, 0x7A, 0x7A, 0x7A, 0, 0, /* DIO1_[8-13] */
> + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* DIO2_[0-7] */
> + 0x7D, 0x7D, 0x7D, 0x7D, 0, 0, /* DIO2_[8-13] */
> + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* LCD_[0-7] */
> + 0, 0, 0 /* LCD_{EN,RS,WR} */
> +};
> +
> +/* "DIO" line's direction control bit-mapping table */
> +static const int line_to_dir_bit_map[] = {
> + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO1_[0-7] */
> + 5, 5, 5, 5, -1, -1, /* DIO1_[8-13] */
> + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO2_[0-7] */
> + 5, 5, 5, 5, -1, -1, /* DIO2_[8-13] */
> + 2, 2, 2, 2, 3, 3, 3, 3, /* LCD_[0-7] */
> + -1, -1, -1 /* LCD_{EN,RS,WR} */
> +};
Splitting up this data into 4 arrays seems odd. I think it would be
better to define a single table with a tuple for each line.
> +
> +static int ts5500_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> + unsigned long dir_reg, flags;
> + int dir_bit;
> +
> + /* Some lines cannot be configured as input */
> + if (offset = TS5500_LCD_EN)
> + return -ENXIO;
> +
> + dir_reg = line_to_dir_map[offset];
> + dir_bit = line_to_dir_bit_map[offset];
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_clear(dir_reg, dir_bit);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int ts5500_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + unsigned long ioaddr;
> + int bitno;
> +
> + ioaddr = line_to_port_map[offset];
> + bitno = line_to_bit_map[offset];
> +
> + return (inb(ioaddr) >> bitno) & 0x1;
> +}
> +
> +static int ts5500_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
> + int val)
> +{
> + unsigned long dir_reg, ioaddr, flags;
> + int dir_bit, bitno;
> +
> + /* Some lines cannot be configured as output */
> + switch (offset) {
> + case TS5500_DIO1_12:
> + case TS5500_DIO1_13:
> + case TS5500_DIO2_13:
> + case TS5500_LCD_RS:
> + case TS5500_LCD_WR:
> + return -ENXIO;
This would also do well in a 'flags' field in the per-line data table
as I described above.
> + default:
> + break;
> + }
> +
> + dir_reg = line_to_dir_map[offset];
> + dir_bit = line_to_dir_bit_map[offset];
> + ioaddr = line_to_port_map[offset];
> + bitno = line_to_bit_map[offset];
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_set(dir_reg, dir_bit);
> + if (val)
> + port_bit_set(ioaddr, bitno);
> + else
> + port_bit_clear(ioaddr, bitno);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static void ts5500_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> +{
> + int bitno;
> + unsigned long ioaddr, flags;
> +
> + ioaddr = line_to_port_map[offset];
> + bitno = line_to_bit_map[offset];
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + if (val)
> + port_bit_set(ioaddr, bitno);
> + else
> + port_bit_clear(ioaddr, bitno);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +}
> +
> +static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + /* Only a few lines are IRQ-capable */
> + if (offset = TS5500_DIO1_13)
> + return TS5500_DIO1_13_IRQ;
> + if (offset = TS5500_DIO2_13)
> + return TS5500_DIO2_13_IRQ;
> + if (offset = TS5500_LCD_RS)
> + return TS5500_LCD_RS_IRQ;
> +
> + /* IRQ line may be bridged with another DIO line from the same header */
> + if (dio1_irq && offset >= TS5500_DIO1_0 && offset < TS5500_DIO1_13)
> + return TS5500_DIO1_13_IRQ;
> + if (dio2_irq && offset >= TS5500_DIO2_0 && offset < TS5500_DIO2_13)
> + return TS5500_DIO2_13_IRQ;
> + if (lcd_irq && offset >= TS5500_LCD_0 && offset <= TS5500_LCD_WR)
> + return TS5500_LCD_RS_IRQ;
Also candidate for putting in the table.
> +
> + return -ENXIO;
> +}
> +
> +static struct gpio_chip ts5500_gpio_chip = {
> + .label = "ts5500-gpio",
> + .owner = THIS_MODULE,
> + .direction_input = ts5500_gpio_direction_input,
> + .get = ts5500_gpio_get,
> + .direction_output = ts5500_gpio_direction_output,
> + .set = ts5500_gpio_set,
> + .to_irq = ts5500_gpio_to_irq,
> + .base = TS5500_DIO1_0,
Don't hard code the GPIO base. It should be dynamically assigned by
using '-1' here.
> +};
> +
> +static void ts5500_gpio_release(struct device *dev)
> +{
> + /* noop */
> +}
> +
> +static struct platform_device ts5500_gpio_pdev = {
> + .name = "ts5500-gpio",
> + .id = -1,
> + .dev = {
> + .release = ts5500_gpio_release,
> + },
> +};
> +
> +static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long flags;
> +
> + if (pdev = NULL)
> + return -ENODEV;
> +
> + if (!request_region(0x7A, 3, "ts5500-gpio-DIO1")) {
> + dev_err(&pdev->dev, "failed to request I/O port 0x7A-7C\n");
> + return -EBUSY;
> + }
> +
> + if (!request_region(0x7D, 3, "ts5500-gpio-DIO2")) {
> + dev_err(&pdev->dev, "failed to request I/O port 0x7D-7F\n");
> + ret = -EBUSY;
> + goto release_dio1;
> + }
> +
> + if (use_lcdio && !request_region(0x72, 2, "ts5500-gpio-LCD")) {
> + dev_err(&pdev->dev, "failed to request I/O port 0x72-73\n");
> + ret = -EBUSY;
> + goto release_dio2;
> + }
> +
> + if (use_lcdio)
> + ts5500_gpio_chip.ngpio = TS5500_LCD_WR + 1;
> + else
> + ts5500_gpio_chip.ngpio = TS5500_DIO2_13 + 1;
> +
> + ret = gpiochip_add(&ts5500_gpio_chip);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register the gpio chip\n");
> + goto release_lcd;
> + }
> +
> + /* Enable IRQ generation */
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_set(0x7A, 7); /* DIO1_13 on IRQ7 */
> + port_bit_set(0x7D, 7); /* DIO2_13 on IRQ6 */
> + if (use_lcdio) {
> + port_bit_clear(0x7D, 4); /* LCD header usage as DIO */
> + port_bit_set(0x7D, 6); /* LCD_RS on IRQ1 */
> + }
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return 0;
> +
> +release_lcd:
> + if (use_lcdio)
> + release_region(0x72, 2);
> +release_dio2:
> + release_region(0x7D, 3);
> +release_dio1:
> + release_region(0x7A, 3);
> +
> + return ret;
> +}
> +
> +static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long flags;
> +
> + /* Disable IRQ generation */
> + spin_lock_irqsave(&gpio_lock, flags);
> + port_bit_clear(0x7A, 7);
> + port_bit_clear(0x7D, 7);
> + if (use_lcdio)
> + port_bit_clear(0x7D, 6);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + release_region(0x7A, 3);
> + release_region(0x7D, 3);
> + if (use_lcdio)
> + release_region(0x72, 2);
> +
> + ret = gpiochip_remove(&ts5500_gpio_chip);
> + if (ret)
> + dev_err(&pdev->dev, "failed to remove the gpio chip\n");
> +
> + return ret;
> +}
> +
> +static struct platform_driver ts5500_gpio_driver = {
> + .driver = {
> + .name = "ts5500-gpio",
> + .owner = THIS_MODULE,
> + },
> + .probe = ts5500_gpio_probe,
> + .remove = __devexit_p(ts5500_gpio_remove),
> +};
> +
> +static int __init ts5500_gpio_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&ts5500_gpio_driver);
> + if (ret)
> + return ret;
> +
> + ret = platform_device_register(&ts5500_gpio_pdev);
> + if (ret) {
> + platform_driver_unregister(&ts5500_gpio_driver);
> + return ret;
> + }
As already discussed.... Bleah!! :-)
Please move the device registration to platform setup code.
> +
> + return 0;
> +}
> +module_init(ts5500_gpio_init);
> +
> +static void __exit ts5500_gpio_exit(void)
> +{
> + platform_driver_unregister(&ts5500_gpio_driver);
> + platform_device_unregister(&ts5500_gpio_pdev);
> +}
> +module_exit(ts5500_gpio_exit);
module_platform_driver() will replace some of the boilerplate.
g.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-05-17 21:06 ` [lm-sensors] " Grant Likely
@ 2012-05-17 21:14 ` Joe Perches
-1 siblings, 0 replies; 28+ messages in thread
From: Joe Perches @ 2012-05-17 21:14 UTC (permalink / raw)
To: Grant Likely
Cc: Vivien Didelot, x86, Jerome Oufella, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, linux-kernel, lm-sensors,
Guenter Roeck, Jean Delvare, Linus Walleij
On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> On Thu, 12 Apr 2012 20:28:55 -0400, Vivien Didelot <vivien.didelot@savoirfairelinux.com> wrote:
> > From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
[]
> > +/* "DIO" line to IO port mapping table for line's value */
> > +static const unsigned long line_to_port_map[] = {
> > + 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, /* DIO1_[0-7] */
> > + 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, /* DIO1_[8-13] */
> > + 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, /* DIO2_[0-7] */
> > + 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, /* DIO2_[8-13] */
> > + 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, /* LCD_[0-7] */
> > + 0x73, 0x73, 0x73 /* LCD_{EN,RS,WR} */
> > +};
There doesn't seem to be a reason to make this a ulong.
uchar would suffice.
> > +
> > +/* "DIO" line to IO port's bit map for line's value */
> > +static const int line_to_bit_map[] = {
> > + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO1_[0-7] */
> > + 0, 1, 2, 3, 4, 5, /* DIO1_[8-13] */
> > + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO2_[0-7] */
> > + 0, 1, 2, 3, 4, 5, /* DIO2_[8-13] */
> > + 0, 1, 2, 3, 4, 5, 6, 7, /* LCD_[0-7] */
> > + 0, 7, 6 /* LCD_{EN,RS,WR} */
Nor this an int, s8 maybe.
> > +/* "DIO" line's direction control mapping table */
> > +static const unsigned long line_to_dir_map[] = {
> > + 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, /* DIO1_[0-7] */
> > + 0x7A, 0x7A, 0x7A, 0x7A, 0, 0, /* DIO1_[8-13] */
> > + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* DIO2_[0-7] */
> > + 0x7D, 0x7D, 0x7D, 0x7D, 0, 0, /* DIO2_[8-13] */
> > + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* LCD_[0-7] */
> > + 0, 0, 0 /* LCD_{EN,RS,WR} */
uchar
> > +/* "DIO" line's direction control bit-mapping table */
> > +static const int line_to_dir_bit_map[] = {
> > + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO1_[0-7] */
> > + 5, 5, 5, 5, -1, -1, /* DIO1_[8-13] */
> > + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO2_[0-7] */
> > + 5, 5, 5, 5, -1, -1, /* DIO2_[8-13] */
> > + 2, 2, 2, 2, 3, 3, 3, 3, /* LCD_[0-7] */
> > + -1, -1, -1 /* LCD_{EN,RS,WR} */
> > +};
s8?
> Splitting up this data into 4 arrays seems odd. I think it would be
> better to define a single table with a tuple for each line.
yup.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-05-17 21:14 ` Joe Perches
0 siblings, 0 replies; 28+ messages in thread
From: Joe Perches @ 2012-05-17 21:14 UTC (permalink / raw)
To: Grant Likely
Cc: Vivien Didelot, x86, Jerome Oufella, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, linux-kernel, lm-sensors,
Guenter Roeck, Jean Delvare, Linus Walleij
On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> On Thu, 12 Apr 2012 20:28:55 -0400, Vivien Didelot <vivien.didelot@savoirfairelinux.com> wrote:
> > From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
[]
> > +/* "DIO" line to IO port mapping table for line's value */
> > +static const unsigned long line_to_port_map[] = {
> > + 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, 0x7B, /* DIO1_[0-7] */
> > + 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, 0x7C, /* DIO1_[8-13] */
> > + 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, 0x7E, /* DIO2_[0-7] */
> > + 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, /* DIO2_[8-13] */
> > + 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, 0x72, /* LCD_[0-7] */
> > + 0x73, 0x73, 0x73 /* LCD_{EN,RS,WR} */
> > +};
There doesn't seem to be a reason to make this a ulong.
uchar would suffice.
> > +
> > +/* "DIO" line to IO port's bit map for line's value */
> > +static const int line_to_bit_map[] = {
> > + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO1_[0-7] */
> > + 0, 1, 2, 3, 4, 5, /* DIO1_[8-13] */
> > + 0, 1, 2, 3, 4, 5, 6, 7, /* DIO2_[0-7] */
> > + 0, 1, 2, 3, 4, 5, /* DIO2_[8-13] */
> > + 0, 1, 2, 3, 4, 5, 6, 7, /* LCD_[0-7] */
> > + 0, 7, 6 /* LCD_{EN,RS,WR} */
Nor this an int, s8 maybe.
> > +/* "DIO" line's direction control mapping table */
> > +static const unsigned long line_to_dir_map[] = {
> > + 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, 0x7A, /* DIO1_[0-7] */
> > + 0x7A, 0x7A, 0x7A, 0x7A, 0, 0, /* DIO1_[8-13] */
> > + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* DIO2_[0-7] */
> > + 0x7D, 0x7D, 0x7D, 0x7D, 0, 0, /* DIO2_[8-13] */
> > + 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, 0x7D, /* LCD_[0-7] */
> > + 0, 0, 0 /* LCD_{EN,RS,WR} */
uchar
> > +/* "DIO" line's direction control bit-mapping table */
> > +static const int line_to_dir_bit_map[] = {
> > + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO1_[0-7] */
> > + 5, 5, 5, 5, -1, -1, /* DIO1_[8-13] */
> > + 0, 0, 0, 0, 1, 1, 1, 1, /* DIO2_[0-7] */
> > + 5, 5, 5, 5, -1, -1, /* DIO2_[8-13] */
> > + 2, 2, 2, 2, 3, 3, 3, 3, /* LCD_[0-7] */
> > + -1, -1, -1 /* LCD_{EN,RS,WR} */
> > +};
s8?
> Splitting up this data into 4 arrays seems odd. I think it would be
> better to define a single table with a tuple for each line.
yup.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-05-17 21:06 ` [lm-sensors] " Grant Likely
@ 2012-05-17 21:40 ` Vivien Didelot
-1 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-05-17 21:40 UTC (permalink / raw)
To: Grant Likely
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> > arch/x86/include/asm/ts5500.h | 62 ++++++++
>
> Why the separate header file? What will use these defines? I
> normally expect driver-specific defines to be in the driver .c file
> directly; particularly for things like gpio drivers which should be a
> generic interface that doesn't need to export symbols.
Should an intermediate driver directly use values for GPIOs instead of
these symbols? For example, how should a temperature sensor plugged on
this platform refer to inputs and outputs?
Thanks,
Vivien
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-05-17 21:40 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-05-17 21:40 UTC (permalink / raw)
To: Grant Likely
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> > arch/x86/include/asm/ts5500.h | 62 ++++++++
>
> Why the separate header file? What will use these defines? I
> normally expect driver-specific defines to be in the driver .c file
> directly; particularly for things like gpio drivers which should be a
> generic interface that doesn't need to export symbols.
Should an intermediate driver directly use values for GPIOs instead of
these symbols? For example, how should a temperature sensor plugged on
this platform refer to inputs and outputs?
Thanks,
Vivien
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-05-17 21:40 ` [lm-sensors] " Vivien Didelot
@ 2012-05-17 22:59 ` Grant Likely
-1 siblings, 0 replies; 28+ messages in thread
From: Grant Likely @ 2012-05-17 22:59 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
>> > arch/x86/include/asm/ts5500.h | 62 ++++++++
>>
>> Why the separate header file? What will use these defines? I
>> normally expect driver-specific defines to be in the driver .c file
>> directly; particularly for things like gpio drivers which should be a
>> generic interface that doesn't need to export symbols.
>
> Should an intermediate driver directly use values for GPIOs instead of
> these symbols? For example, how should a temperature sensor plugged on
> this platform refer to inputs and outputs?
Tell me more about this platform. Where does the data about
connections come from? Is it a purpose-built embedded system? Is the
GPIO controller described in ACPI? (probably not since GPIOs were only
added to ACPI in v5) Does the end-user attach her own hardware to the
board like the temperature sensor you describe? If so, is that
hardware driven by kernel drivers or user-space drivers?
For userspace drivers you can get information about the GPIO number
assignments from /sys, but it isn't well documented and can probably
be improved.
If it is kernelspace, then you really need a way to add data about the
platform to the kernel at runtime. Having it statically compiled in
isn't a very good solution. I would recommend injecting configuration
data into the kernel from userspace. You could invent something, but
that wouldn't be very portable. Xilinx has done some work on this
using Flattened Device Tree and the firmware loading infrastructure.
The kernel requests a .dtb (device tree blob) from userspace and uses
that to configure devices. That may do the job for you. GPIO and
platform device infrastructure already have FDT support which will
help you here. I expect it could be done with an ACPI fragment too,
but I just don't know of anybody having done any work in this area.
That probably isn't the answer you want though since I assume you just
need to get something that works rather than investing a whole bunch
of time on generic infrastructure. What I would recommend is for your
platform setup code to use a notifier to wait for the
BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
with the correct gpio number at that time (because once you have a
reference to the gpio controller you can calculate the assigned gpio
numbers).
g.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-05-17 22:59 ` Grant Likely
0 siblings, 0 replies; 28+ messages in thread
From: Grant Likely @ 2012-05-17 22:59 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
>> > arch/x86/include/asm/ts5500.h | 62 ++++++++
>>
>> Why the separate header file? What will use these defines? I
>> normally expect driver-specific defines to be in the driver .c file
>> directly; particularly for things like gpio drivers which should be a
>> generic interface that doesn't need to export symbols.
>
> Should an intermediate driver directly use values for GPIOs instead of
> these symbols? For example, how should a temperature sensor plugged on
> this platform refer to inputs and outputs?
Tell me more about this platform. Where does the data about
connections come from? Is it a purpose-built embedded system? Is the
GPIO controller described in ACPI? (probably not since GPIOs were only
added to ACPI in v5) Does the end-user attach her own hardware to the
board like the temperature sensor you describe? If so, is that
hardware driven by kernel drivers or user-space drivers?
For userspace drivers you can get information about the GPIO number
assignments from /sys, but it isn't well documented and can probably
be improved.
If it is kernelspace, then you really need a way to add data about the
platform to the kernel at runtime. Having it statically compiled in
isn't a very good solution. I would recommend injecting configuration
data into the kernel from userspace. You could invent something, but
that wouldn't be very portable. Xilinx has done some work on this
using Flattened Device Tree and the firmware loading infrastructure.
The kernel requests a .dtb (device tree blob) from userspace and uses
that to configure devices. That may do the job for you. GPIO and
platform device infrastructure already have FDT support which will
help you here. I expect it could be done with an ACPI fragment too,
but I just don't know of anybody having done any work in this area.
That probably isn't the answer you want though since I assume you just
need to get something that works rather than investing a whole bunch
of time on generic infrastructure. What I would recommend is for your
platform setup code to use a notifier to wait for the
BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
with the correct gpio number at that time (because once you have a
reference to the gpio controller you can calculate the assigned gpio
numbers).
g.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-05-17 22:59 ` [lm-sensors] " Grant Likely
@ 2012-05-18 14:37 ` Vivien Didelot
-1 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-05-18 14:37 UTC (permalink / raw)
To: Grant Likely
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
Hi Grant,
On Thu, 2012-05-17 at 16:59 -0600, Grant Likely wrote:
> On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
> <vivien.didelot@savoirfairelinux.com> wrote:
> > On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> >> > arch/x86/include/asm/ts5500.h | 62 ++++++++
> >>
> >> Why the separate header file? What will use these defines? I
> >> normally expect driver-specific defines to be in the driver .c file
> >> directly; particularly for things like gpio drivers which should be a
> >> generic interface that doesn't need to export symbols.
> >
> > Should an intermediate driver directly use values for GPIOs instead of
> > these symbols? For example, how should a temperature sensor plugged on
> > this platform refer to inputs and outputs?
>
> Tell me more about this platform. Where does the data about
> connections come from? Is it a purpose-built embedded system?
This is a generic purpose platform, the GPIO bus is exposed on an
external DB25 connector. End-users can use it however they like.
> Is the
> GPIO controller described in ACPI? (probably not since GPIOs were only
> added to ACPI in v5) Does the end-user attach her own hardware to the
> board like the temperature sensor you describe? If so, is that
> hardware driven by kernel drivers or user-space drivers?
Both in fact. For instance, we are connecting an SHT15
humidity/temperature sensor, which already has support in the kernel.
>
> For userspace drivers you can get information about the GPIO number
> assignments from /sys, but it isn't well documented and can probably
> be improved.
>
> If it is kernelspace, then you really need a way to add data about the
> platform to the kernel at runtime. Having it statically compiled in
> isn't a very good solution. I would recommend injecting configuration
> data into the kernel from userspace. You could invent something, but
> that wouldn't be very portable. Xilinx has done some work on this
> using Flattened Device Tree and the firmware loading infrastructure.
> The kernel requests a .dtb (device tree blob) from userspace and uses
> that to configure devices. That may do the job for you. GPIO and
> platform device infrastructure already have FDT support which will
> help you here. I expect it could be done with an ACPI fragment too,
> but I just don't know of anybody having done any work in this area.
>
> That probably isn't the answer you want though since I assume you just
> need to get something that works rather than investing a whole bunch
> of time on generic infrastructure.
Exactly.
> What I would recommend is for your
> platform setup code to use a notifier to wait for the
> BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
> with the correct gpio number at that time (because once you have a
> reference to the gpio controller you can calculate the assigned gpio
> numbers).
Thanks.
I've been working on pushing this code mainline for a while. To
summarize, for you to accept this code, you'd prefer me to move every
symbol into the driver itself (in addition to addressing your and Joe's
other requests), and then we're good?
>
> g.
Thanks,
Vivien.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-05-18 14:37 ` Vivien Didelot
0 siblings, 0 replies; 28+ messages in thread
From: Vivien Didelot @ 2012-05-18 14:37 UTC (permalink / raw)
To: Grant Likely
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
Hi Grant,
On Thu, 2012-05-17 at 16:59 -0600, Grant Likely wrote:
> On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
> <vivien.didelot@savoirfairelinux.com> wrote:
> > On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> >> > arch/x86/include/asm/ts5500.h | 62 ++++++++
> >>
> >> Why the separate header file? What will use these defines? I
> >> normally expect driver-specific defines to be in the driver .c file
> >> directly; particularly for things like gpio drivers which should be a
> >> generic interface that doesn't need to export symbols.
> >
> > Should an intermediate driver directly use values for GPIOs instead of
> > these symbols? For example, how should a temperature sensor plugged on
> > this platform refer to inputs and outputs?
>
> Tell me more about this platform. Where does the data about
> connections come from? Is it a purpose-built embedded system?
This is a generic purpose platform, the GPIO bus is exposed on an
external DB25 connector. End-users can use it however they like.
> Is the
> GPIO controller described in ACPI? (probably not since GPIOs were only
> added to ACPI in v5) Does the end-user attach her own hardware to the
> board like the temperature sensor you describe? If so, is that
> hardware driven by kernel drivers or user-space drivers?
Both in fact. For instance, we are connecting an SHT15
humidity/temperature sensor, which already has support in the kernel.
>
> For userspace drivers you can get information about the GPIO number
> assignments from /sys, but it isn't well documented and can probably
> be improved.
>
> If it is kernelspace, then you really need a way to add data about the
> platform to the kernel at runtime. Having it statically compiled in
> isn't a very good solution. I would recommend injecting configuration
> data into the kernel from userspace. You could invent something, but
> that wouldn't be very portable. Xilinx has done some work on this
> using Flattened Device Tree and the firmware loading infrastructure.
> The kernel requests a .dtb (device tree blob) from userspace and uses
> that to configure devices. That may do the job for you. GPIO and
> platform device infrastructure already have FDT support which will
> help you here. I expect it could be done with an ACPI fragment too,
> but I just don't know of anybody having done any work in this area.
>
> That probably isn't the answer you want though since I assume you just
> need to get something that works rather than investing a whole bunch
> of time on generic infrastructure.
Exactly.
> What I would recommend is for your
> platform setup code to use a notifier to wait for the
> BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
> with the correct gpio number at that time (because once you have a
> reference to the gpio controller you can calculate the assigned gpio
> numbers).
Thanks.
I've been working on pushing this code mainline for a while. To
summarize, for you to accept this code, you'd prefer me to move every
symbol into the driver itself (in addition to addressing your and Joe's
other requests), and then we're good?
>
> g.
Thanks,
Vivien.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support
2012-05-18 14:37 ` [lm-sensors] " Vivien Didelot
@ 2012-05-18 19:59 ` Grant Likely
-1 siblings, 0 replies; 28+ messages in thread
From: Grant Likely @ 2012-05-18 19:59 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
On Fri, May 18, 2012 at 8:37 AM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Hi Grant,
>
> On Thu, 2012-05-17 at 16:59 -0600, Grant Likely wrote:
>> On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
>> <vivien.didelot@savoirfairelinux.com> wrote:
>> > On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
>> >> > arch/x86/include/asm/ts5500.h | 62 ++++++++
>> >>
>> >> Why the separate header file? What will use these defines? I
>> >> normally expect driver-specific defines to be in the driver .c file
>> >> directly; particularly for things like gpio drivers which should be a
>> >> generic interface that doesn't need to export symbols.
>> >
>> > Should an intermediate driver directly use values for GPIOs instead of
>> > these symbols? For example, how should a temperature sensor plugged on
>> > this platform refer to inputs and outputs?
>>
>> Tell me more about this platform. Where does the data about
>> connections come from? Is it a purpose-built embedded system?
>
> This is a generic purpose platform, the GPIO bus is exposed on an
> external DB25 connector. End-users can use it however they like.
>
>> Is the
>> GPIO controller described in ACPI? (probably not since GPIOs were only
>> added to ACPI in v5) Does the end-user attach her own hardware to the
>> board like the temperature sensor you describe? If so, is that
>> hardware driven by kernel drivers or user-space drivers?
>
> Both in fact. For instance, we are connecting an SHT15
> humidity/temperature sensor, which already has support in the kernel.
>
>>
>> For userspace drivers you can get information about the GPIO number
>> assignments from /sys, but it isn't well documented and can probably
>> be improved.
>>
>> If it is kernelspace, then you really need a way to add data about the
>> platform to the kernel at runtime. Having it statically compiled in
>> isn't a very good solution. I would recommend injecting configuration
>> data into the kernel from userspace. You could invent something, but
>> that wouldn't be very portable. Xilinx has done some work on this
>> using Flattened Device Tree and the firmware loading infrastructure.
>> The kernel requests a .dtb (device tree blob) from userspace and uses
>> that to configure devices. That may do the job for you. GPIO and
>> platform device infrastructure already have FDT support which will
>> help you here. I expect it could be done with an ACPI fragment too,
>> but I just don't know of anybody having done any work in this area.
>>
>> That probably isn't the answer you want though since I assume you just
>> need to get something that works rather than investing a whole bunch
>> of time on generic infrastructure.
>
> Exactly.
>
>> What I would recommend is for your
>> platform setup code to use a notifier to wait for the
>> BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
>> with the correct gpio number at that time (because once you have a
>> reference to the gpio controller you can calculate the assigned gpio
>> numbers).
>
> Thanks.
>
> I've been working on pushing this code mainline for a while. To
> summarize, for you to accept this code, you'd prefer me to move every
> symbol into the driver itself (in addition to addressing your and Joe's
> other requests), and then we're good?
Either that or have a *really good* argument why it should be
exposed/exported. It is already a tough-fought battle to move away
from driver-specific hacks and custom platform code, so I don't want
to be adding yet more if at all avoided.
g.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH v6 3/3] gpio: TS-5500 GPIO support
@ 2012-05-18 19:59 ` Grant Likely
0 siblings, 0 replies; 28+ messages in thread
From: Grant Likely @ 2012-05-18 19:59 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Jerome Oufella, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, linux-kernel, lm-sensors, Guenter Roeck,
Jean Delvare, Linus Walleij
On Fri, May 18, 2012 at 8:37 AM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Hi Grant,
>
> On Thu, 2012-05-17 at 16:59 -0600, Grant Likely wrote:
>> On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
>> <vivien.didelot@savoirfairelinux.com> wrote:
>> > On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
>> >> > arch/x86/include/asm/ts5500.h | 62 ++++++++
>> >>
>> >> Why the separate header file? What will use these defines? I
>> >> normally expect driver-specific defines to be in the driver .c file
>> >> directly; particularly for things like gpio drivers which should be a
>> >> generic interface that doesn't need to export symbols.
>> >
>> > Should an intermediate driver directly use values for GPIOs instead of
>> > these symbols? For example, how should a temperature sensor plugged on
>> > this platform refer to inputs and outputs?
>>
>> Tell me more about this platform. Where does the data about
>> connections come from? Is it a purpose-built embedded system?
>
> This is a generic purpose platform, the GPIO bus is exposed on an
> external DB25 connector. End-users can use it however they like.
>
>> Is the
>> GPIO controller described in ACPI? (probably not since GPIOs were only
>> added to ACPI in v5) Does the end-user attach her own hardware to the
>> board like the temperature sensor you describe? If so, is that
>> hardware driven by kernel drivers or user-space drivers?
>
> Both in fact. For instance, we are connecting an SHT15
> humidity/temperature sensor, which already has support in the kernel.
>
>>
>> For userspace drivers you can get information about the GPIO number
>> assignments from /sys, but it isn't well documented and can probably
>> be improved.
>>
>> If it is kernelspace, then you really need a way to add data about the
>> platform to the kernel at runtime. Having it statically compiled in
>> isn't a very good solution. I would recommend injecting configuration
>> data into the kernel from userspace. You could invent something, but
>> that wouldn't be very portable. Xilinx has done some work on this
>> using Flattened Device Tree and the firmware loading infrastructure.
>> The kernel requests a .dtb (device tree blob) from userspace and uses
>> that to configure devices. That may do the job for you. GPIO and
>> platform device infrastructure already have FDT support which will
>> help you here. I expect it could be done with an ACPI fragment too,
>> but I just don't know of anybody having done any work in this area.
>>
>> That probably isn't the answer you want though since I assume you just
>> need to get something that works rather than investing a whole bunch
>> of time on generic infrastructure.
>
> Exactly.
>
>> What I would recommend is for your
>> platform setup code to use a notifier to wait for the
>> BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
>> with the correct gpio number at that time (because once you have a
>> reference to the gpio controller you can calculate the assigned gpio
>> numbers).
>
> Thanks.
>
> I've been working on pushing this code mainline for a while. To
> summarize, for you to accept this code, you'd prefer me to move every
> symbol into the driver itself (in addition to addressing your and Joe's
> other requests), and then we're good?
Either that or have a *really good* argument why it should be
exposed/exported. It is already a tough-fought battle to move away
from driver-specific hacks and custom platform code, so I don't want
to be adding yet more if at all avoided.
g.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2012-05-18 19:59 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-13 0:28 [PATCH v6 0/3] TS-5500 platform support Vivien Didelot
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
2012-04-13 0:28 ` [PATCH v6 1/3] hwmon: Maxim MAX197 support Vivien Didelot
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
2012-04-14 0:46 ` Guenter Roeck
2012-04-14 0:46 ` [lm-sensors] " Guenter Roeck
2012-04-13 0:28 ` [PATCH v6 2/3] x86/platform: TS-5500 basic platform support Vivien Didelot
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
2012-04-13 10:37 ` Thomas Gleixner
2012-04-13 10:37 ` [lm-sensors] " Thomas Gleixner
2012-04-13 20:46 ` Vivien Didelot
2012-04-13 20:46 ` [lm-sensors] " Vivien Didelot
2012-04-13 0:28 ` [PATCH v6 3/3] gpio: TS-5500 GPIO support Vivien Didelot
2012-04-13 0:28 ` [lm-sensors] " Vivien Didelot
2012-04-13 19:04 ` Mark Brown
2012-04-13 19:04 ` [lm-sensors] " Mark Brown
2012-05-17 21:06 ` Grant Likely
2012-05-17 21:06 ` [lm-sensors] " Grant Likely
2012-05-17 21:14 ` Joe Perches
2012-05-17 21:14 ` [lm-sensors] " Joe Perches
2012-05-17 21:40 ` Vivien Didelot
2012-05-17 21:40 ` [lm-sensors] " Vivien Didelot
2012-05-17 22:59 ` Grant Likely
2012-05-17 22:59 ` [lm-sensors] " Grant Likely
2012-05-18 14:37 ` Vivien Didelot
2012-05-18 14:37 ` [lm-sensors] " Vivien Didelot
2012-05-18 19:59 ` Grant Likely
2012-05-18 19:59 ` [lm-sensors] " Grant Likely
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.