* [PATCH v5 0/5] Support for the TS-5500 platform
@ 2012-02-01 21:05 Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 1/5] x86/platform: (TS-5500) add platform base support Vivien Didelot
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Vivien Didelot @ 2012-02-01 21:05 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare
This patchset brings the support for the TS-5500 platform.
The first patch adds the base support for the board in
/arch/x86/platform/ts5500, and a documentation file in
Documentation/ABI/testing/sysfs-platform-ts5500.
The second patch adds support for GPIO.
The third patch adds support for the on-board LED.
The fourth 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 fifth patch brings support for the A/D converter.
Jerome Oufella (1):
x86/platform: (TS-5500) add GPIO support
Jonas Fonseca (1):
x86/platform: (TS-5500) add LED support
Vivien Didelot (3):
x86/platform: (TS-5500) add platform base support
hwmon: add MAX197 support
x86/platform: (TS-5500) add ADC support
Documentation/ABI/testing/sysfs-platform-ts5500 | 46 +++
Documentation/hwmon/max197 | 54 +++
MAINTAINERS | 5 +
arch/x86/Kconfig | 2 +
arch/x86/platform/Makefile | 1 +
arch/x86/platform/ts5500/Kconfig | 29 ++
arch/x86/platform/ts5500/Makefile | 4 +
arch/x86/platform/ts5500/ts5500.c | 299 ++++++++++++++
arch/x86/platform/ts5500/ts5500.h | 35 ++
arch/x86/platform/ts5500/ts5500_adc.c | 104 +++++
arch/x86/platform/ts5500/ts5500_gpio.c | 478 +++++++++++++++++++++++
arch/x86/platform/ts5500/ts5500_gpio.h | 60 +++
arch/x86/platform/ts5500/ts5500_led.c | 179 +++++++++
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max197.c | 403 +++++++++++++++++++
include/linux/max197.h | 22 +
17 files changed, 1731 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-platform-ts5500
create mode 100644 Documentation/hwmon/max197
create mode 100644 arch/x86/platform/ts5500/Kconfig
create mode 100644 arch/x86/platform/ts5500/Makefile
create mode 100644 arch/x86/platform/ts5500/ts5500.c
create mode 100644 arch/x86/platform/ts5500/ts5500.h
create mode 100644 arch/x86/platform/ts5500/ts5500_adc.c
create mode 100644 arch/x86/platform/ts5500/ts5500_gpio.c
create mode 100644 arch/x86/platform/ts5500/ts5500_gpio.h
create mode 100644 arch/x86/platform/ts5500/ts5500_led.c
create mode 100644 drivers/hwmon/max197.c
create mode 100644 include/linux/max197.h
--
1.7.6.5
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 1/5] x86/platform: (TS-5500) add platform base support
2012-02-01 21:05 [PATCH v5 0/5] Support for the TS-5500 platform Vivien Didelot
@ 2012-02-01 21:05 ` Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support Vivien Didelot
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Vivien Didelot @ 2012-02-01 21:05 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
Documentation/ABI/testing/sysfs-platform-ts5500 | 46 ++++
MAINTAINERS | 5 +
arch/x86/Kconfig | 2 +
arch/x86/platform/Makefile | 1 +
arch/x86/platform/ts5500/Kconfig | 8 +
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500.c | 299 +++++++++++++++++++++++
arch/x86/platform/ts5500/ts5500.h | 35 +++
8 files changed, 397 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-platform-ts5500
create mode 100644 arch/x86/platform/ts5500/Kconfig
create mode 100644 arch/x86/platform/ts5500/Makefile
create mode 100644 arch/x86/platform/ts5500/ts5500.c
create mode 100644 arch/x86/platform/ts5500/ts5500.h
diff --git a/Documentation/ABI/testing/sysfs-platform-ts5500 b/Documentation/ABI/testing/sysfs-platform-ts5500
new file mode 100644
index 0000000..4b5c2dc
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-ts5500
@@ -0,0 +1,46 @@
+What: /sys/devices/platform/ts5500/id
+Date: February 2012
+KernelVersion: 3.2
+Contact: "Savoir-faire Linux, Inc." <kernel-ts@savoirfairelinux.com>
+Description:
+ Product ID of the TS board. TS-5500 ID is 0x60.
+
+What: /sys/devices/platform/ts5500/sram
+Date: February 2012
+KernelVersion: 3.2
+Contact: "Savoir-faire Linux, Inc." <kernel-ts@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: February 2012
+KernelVersion: 3.2
+Contact: "Savoir-faire Linux, Inc." <kernel-ts@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: February 2012
+KernelVersion: 3.2
+Contact: "Savoir-faire Linux, Inc." <kernel-ts@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: February 2012
+KernelVersion: 3.2
+Contact: "Savoir-faire Linux, Inc." <kernel-ts@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: February 2012
+KernelVersion: 3.2
+Contact: "Savoir-faire Linux, Inc." <kernel-ts@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 62f1cd3..c494156 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6500,6 +6500,11 @@ W: http://tcp-lp-mod.sourceforge.net/
S: Maintained
F: net/ipv4/tcp_lp.c
+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 efb4294..5204f62 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2090,6 +2090,8 @@ config ALIX
Note: You have to set alix.force=1 for boards with Award BIOS.
+source "arch/x86/platform/ts5500/Kconfig"
+
endif # X86_32
config AMD_NB
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 8d87439..2d17fd3 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -9,3 +9,4 @@ obj-y += scx200/
obj-y += sfi/
obj-y += visws/
obj-y += uv/
+obj-y += ts5500/
diff --git a/arch/x86/platform/ts5500/Kconfig b/arch/x86/platform/ts5500/Kconfig
new file mode 100644
index 0000000..929b617
--- /dev/null
+++ b/arch/x86/platform/ts5500/Kconfig
@@ -0,0 +1,8 @@
+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 platform.
+
+ If you have a TS-5500, say Y here.
diff --git a/arch/x86/platform/ts5500/Makefile b/arch/x86/platform/ts5500/Makefile
new file mode 100644
index 0000000..0a689a7
--- /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..7c01c46
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500.c
@@ -0,0 +1,299 @@
+/*
+ * Technologic Systems TS-5500 board - SBC info layer
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Alexandre Savard <alexandre.savard@savoirfairelinux.com>
+ * Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
+ * Vivien Didelot <vivien.didelot@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 "ts5500.h"
+
+/* Hardware info for pre-detection */
+#define AMD_ELAN_FAMILY 4
+#define AMD_ELAN_SC520 9
+
+/* Product code register */
+#define TS5500_PRODUCT_CODE_REG 0x74
+#define TS5500_PRODUCT_CODE 0x60 /* TS-5500 product code */
+
+/**
+ * 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 __init int 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 __init int 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 ts5500_detect_config(struct ts5500_sbc *sbc)
+{
+ u8 tmp;
+ int ret = 0;
+
+ if (!request_region(TS5500_PRODUCT_CODE_REG, 4, "ts5500"))
+ return -EBUSY;
+
+ mutex_lock(&ts5500->lock);
+ tmp = inb(TS5500_PRODUCT_CODE_REG);
+ 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_REG);
+ ts5500->sram = !!(tmp & TS5500_SRAM_OPT);
+ ts5500->rs485 = !!(tmp & TS5500_RS485_OPT);
+ ts5500->adc = !!(tmp & TS5500_ADC_OPT);
+
+ tmp = inb(TS5500_ERESET_ITR_REG);
+ ts5500->ereset = !!(tmp & TS5500_ERESET_OPT);
+ ts5500->itr = !!(tmp & TS5500_ITR_OPT);
+
+ tmp = inb(TS5500_LED_JMPRS_REG);
+ sbc->jumpers = tmp & 0xFE; /* All bits except the first (LED) */
+
+cleanup:
+ mutex_unlock(&ts5500->lock);
+ release_region(TS5500_PRODUCT_CODE_REG, 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
+};
+
+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;
+
+ return 0;
+
+release_pdev:
+ platform_device_unregister(pdev);
+release_mem:
+ kfree(ts5500);
+
+ return ret;
+}
+postcore_initcall(ts5500_init);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Technologic Systems TS-5500 Board's platform driver");
diff --git a/arch/x86/platform/ts5500/ts5500.h b/arch/x86/platform/ts5500/ts5500.h
new file mode 100644
index 0000000..f4f1bc4
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500.h
@@ -0,0 +1,35 @@
+/*
+ * Technologic Systems TS-5500 board
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ */
+
+#ifndef _TS5500_H
+#define _TS5500_H
+
+/* SRAM/RS-485/ADC options, and RS-485 RTS/Automatic RS-485 flags register */
+#define TS5500_SRAM_RS485_ADC_REG 0x75
+#define TS5500_SRAM_OPT 0x01 /* SRAM option */
+#define TS5500_RS485_OPT 0x02 /* RS-485 option */
+#define TS5500_ADC_OPT 0x04 /* A/D converter option */
+#define TS5500_RS485_RTS_MASK 0x40 /* RTS for RS-485 */
+#define TS5500_RS485_AUTO_MASK 0x80 /* Automatic RS-485 */
+
+/* External Reset/Industrial Temperature Range options register */
+#define TS5500_ERESET_ITR_REG 0x76
+#define TS5500_ERESET_OPT 0x01 /* External Reset option */
+#define TS5500_ITR_OPT 0x02 /* Indust. Temp. Range option */
+
+/* LED/Jumpers register */
+#define TS5500_LED_JMPRS_REG 0x77
+#define TS5500_LED_MASK 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) */
+
+#endif
--
1.7.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-01 21:05 [PATCH v5 0/5] Support for the TS-5500 platform Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 1/5] x86/platform: (TS-5500) add platform base support Vivien Didelot
@ 2012-02-01 21:05 ` Vivien Didelot
2012-02-01 21:30 ` Alan Cox
2012-02-06 15:16 ` Mark Brown
2012-02-01 21:05 ` [PATCH v5 3/5] x86/platform: (TS-5500) add LED support Vivien Didelot
` (2 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Vivien Didelot @ 2012-02-01 21:05 UTC (permalink / raw)
To: x86
Cc: Jerome Oufella, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
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>
---
arch/x86/platform/ts5500/Kconfig | 7 +
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500_gpio.c | 478 ++++++++++++++++++++++++++++++++
arch/x86/platform/ts5500/ts5500_gpio.h | 60 ++++
4 files changed, 546 insertions(+), 0 deletions(-)
create mode 100644 arch/x86/platform/ts5500/ts5500_gpio.c
create mode 100644 arch/x86/platform/ts5500/ts5500_gpio.h
diff --git a/arch/x86/platform/ts5500/Kconfig b/arch/x86/platform/ts5500/Kconfig
index 929b617..62095c9 100644
--- a/arch/x86/platform/ts5500/Kconfig
+++ b/arch/x86/platform/ts5500/Kconfig
@@ -6,3 +6,10 @@ config TS5500
Add support for the Technologic Systems TS-5500 platform.
If you have a TS-5500, say Y here.
+
+config TS5500_GPIO
+ tristate "TS-5500 GPIO support"
+ depends on TS5500 && GENERIC_GPIO
+ default y
+ help
+ This enables support for the DIO headers for GPIO usage.
diff --git a/arch/x86/platform/ts5500/Makefile b/arch/x86/platform/ts5500/Makefile
index 0a689a7..71c1398 100644
--- a/arch/x86/platform/ts5500/Makefile
+++ b/arch/x86/platform/ts5500/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_TS5500) += ts5500.o
+obj-$(CONFIG_TS5500_GPIO) += ts5500_gpio.o
diff --git a/arch/x86/platform/ts5500/ts5500_gpio.c b/arch/x86/platform/ts5500/ts5500_gpio.c
new file mode 100644
index 0000000..241ac80
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500_gpio.c
@@ -0,0 +1,478 @@
+/*
+ * GPIO (DIO) driver for Technologic Systems TS-5500
+ *
+ * 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.
+ *
+ * TS-5500 board has 38 GPIOs referred to as DIOs in the product's literature.
+ */
+
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include "ts5500_gpio.h"
+
+static void port_bit_set(u8 addr, int bit)
+{
+ u8 var;
+ var = inb(addr);
+ var |= (1 << bit);
+ outb(var, addr);
+}
+
+static 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, LCD_RS, LCD_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, LCD_RS, LCD_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, LCD_RS, LCD_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, LCD_RS, LCD_WR */
+};
+
+/* This array is used to track requests for our GPIO lines */
+static int requested_gpios[TS5500_LCD_WR + 1];
+
+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 = 0;
+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 = 0;
+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 = 0;
+module_param(use_lcdio, int, 0644);
+MODULE_PARM_DESC(use_lcdio, "Enable usage of the LCD header for DIO operation"
+ " (default 0).");
+
+/**
+ * struct ts5500_drvdata - Driver data
+ * @master: Device.
+ * @gpio_chip: GPIO chip.
+ * @gpio_lock: Read/Write Mutex.
+ */
+struct ts5500_drvdata {
+ struct device *master;
+ struct gpio_chip gpio_chip;
+ struct mutex gpio_lock;
+};
+
+static int ts5500_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ mutex_lock(&drvdata->gpio_lock);
+ if (requested_gpios[offset]) {
+ mutex_unlock(&drvdata->gpio_lock);
+ return -EBUSY;
+ }
+ requested_gpios[offset] = 1;
+ mutex_unlock(&drvdata->gpio_lock);
+
+ return 0;
+}
+
+static void ts5500_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ mutex_lock(&drvdata->gpio_lock);
+ requested_gpios[offset] = 0;
+ mutex_unlock(&drvdata->gpio_lock);
+}
+
+static int ts5500_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long ioaddr;
+ u8 byte;
+ int bitno;
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ /* Some lines are output-only and cannot be read */
+ if (offset == TS5500_LCD_EN || offset > chip->ngpio)
+ return -ENXIO;
+
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+ byte = inb(ioaddr);
+
+ return (byte >> bitno) & 0x1;
+}
+
+static void ts5500_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+ int bitno;
+ unsigned long ioaddr;
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ /* Some lines just can't be set */
+ switch (offset) {
+ case TS5500_DIO1_12:
+ case TS5500_DIO1_13:
+ case TS5500_DIO2_13:
+ case TS5500_LCD_RS:
+ case TS5500_LCD_WR:
+ return;
+ default:
+ if (offset > chip->ngpio)
+ return;
+ break;
+ }
+
+ /* Get io port and bit for 'offset' */
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ mutex_lock(&drvdata->gpio_lock);
+ if (val == 0)
+ port_bit_clear(ioaddr, bitno);
+ else
+ port_bit_set(ioaddr, bitno);
+ mutex_unlock(&drvdata->gpio_lock);
+}
+
+static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ /* Only a few lines are IRQ-Capable */
+ switch (offset) {
+ case TS5500_DIO1_13:
+ return TS5500_DIO1_13_IRQ;
+ case TS5500_DIO2_13:
+ return TS5500_DIO2_13_IRQ;
+ case TS5500_LCD_RS:
+ return TS5500_LCD_RS_IRQ;
+ default:
+ break;
+ }
+
+ /*
+ * Handle the case where the user bridged the IRQ line 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 int ts5500_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long dir_reg;
+ int dir_bit;
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ /* Some lines cannot be set as inputs */
+ switch (offset) {
+ case TS5500_LCD_EN:
+ return -ENXIO;
+ default:
+ if (offset > chip->ngpio)
+ return -ENXIO;
+ break;
+ }
+
+ dir_reg = line_to_dir_map[offset];
+ dir_bit = line_to_dir_bit_map[offset];
+
+ mutex_lock(&drvdata->gpio_lock);
+ port_bit_clear(dir_reg, dir_bit);
+ mutex_unlock(&drvdata->gpio_lock);
+
+ return 0;
+}
+
+static int ts5500_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
+ int val)
+{
+ unsigned long dir_reg, ioaddr;
+ int dir_bit, bitno;
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ /* Some lines cannot be set as outputs */
+ 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:
+ if (offset > chip->ngpio)
+ return -ENXIO;
+ break;
+ }
+
+ /* Get direction and value registers infos */
+ 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];
+
+ mutex_lock(&drvdata->gpio_lock);
+ if (val == 0)
+ port_bit_clear(ioaddr, bitno); /* Set initial line value */
+ else
+ port_bit_set(ioaddr, bitno);
+ port_bit_set(dir_reg, dir_bit); /* Set output direction for line */
+
+ /*
+ * Confirm initial line output value
+ * (might have been changed by input)
+ */
+ if (val == 0)
+ port_bit_clear(ioaddr, bitno);
+ else
+ port_bit_set(ioaddr, bitno);
+ mutex_unlock(&drvdata->gpio_lock);
+
+ return 0;
+}
+
+static int __devinit ts5500_gpio_probe(struct platform_device *pdev)
+{
+ struct ts5500_drvdata *drvdata;
+ struct gpio_chip *chip;
+ int ret;
+
+ if (pdev == NULL) {
+ dev_err(&pdev->dev, "Platform device not available!\n");
+ return -ENODEV;
+ }
+
+ /* Request DIO1 */
+ if (!request_region(0x7A, 3, "ts5500-gpio-DIO1")) {
+ dev_err(&pdev->dev, "Cannot request I/O port 0x7A-7C\n");
+ goto err_req_dio1;
+ }
+
+ /* Request DIO2 */
+ if (!request_region(0x7D, 3, "ts5500-gpio-DIO2")) {
+ dev_err(&pdev->dev, "Cannot request I/O port 0x7D-7F\n");
+ goto err_req_dio2;
+ }
+
+ /* Request LCDIO if wanted */
+ if (use_lcdio && !request_region(0x72, 2, "ts5500-gpio-LCD")) {
+ dev_err(&pdev->dev, "Cannot request I/O port 0x72-73\n");
+ goto err_req_lcdio;
+ }
+
+ /* Setup the gpio_chip structure */
+ drvdata = kzalloc(sizeof(struct ts5500_drvdata), GFP_KERNEL);
+ if (drvdata == NULL)
+ goto err_alloc_dev;
+
+ memset(requested_gpios, 0, sizeof(requested_gpios));
+ mutex_init(&drvdata->gpio_lock);
+
+ drvdata->master = pdev->dev.parent;
+ chip = &drvdata->gpio_chip;
+ chip->request = ts5500_gpio_request;
+ chip->free = ts5500_gpio_free;
+ chip->to_irq = ts5500_gpio_to_irq;
+ chip->direction_input = ts5500_gpio_direction_input;
+ chip->direction_output = ts5500_gpio_direction_output;
+ chip->get = ts5500_gpio_get;
+ chip->set = ts5500_gpio_set;
+ chip->can_sleep = 0;
+ chip->base = TS5500_DIO1_0;
+ chip->label = pdev->name;
+ chip->ngpio = (use_lcdio ? TS5500_LCD_WR + 1 : TS5500_DIO2_13 + 1);
+
+ /* Enable IRQ generation */
+ mutex_lock(&drvdata->gpio_lock);
+ 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); /* Enable LCD header usage as DIO */
+ port_bit_set(0x7D, 6); /* LCD_RS on IRQ1 */
+ }
+ mutex_unlock(&drvdata->gpio_lock);
+
+ /* Register chip */
+ ret = gpiochip_add(&drvdata->gpio_chip);
+ if (ret)
+ goto err_gpiochip_add;
+
+ platform_set_drvdata(pdev, drvdata);
+
+ return 0;
+
+err_gpiochip_add:
+ dev_err(&pdev->dev, "Failed to register the gpio chip.\n");
+ kfree(drvdata);
+
+err_alloc_dev:
+ if (use_lcdio)
+ release_region(0x72, 2); /* Release LCD's region */
+
+err_req_lcdio:
+ release_region(0x7D, 3); /* Release DIO2's region */
+
+err_req_dio2:
+ release_region(0x7A, 3); /* Release DIO1's region */
+
+err_req_dio1:
+ ret = -EBUSY;
+
+ return ret;
+}
+
+/* Callback for releasing resources */
+static void ts5500_gpio_device_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_gpio_device = {
+ .name = "ts5500_gpio",
+ .id = -1,
+ .dev = {
+ .release = ts5500_gpio_device_release,
+ }
+};
+
+static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
+{
+ struct ts5500_drvdata *drvdata;
+ int ret, i;
+
+ drvdata = platform_get_drvdata(pdev);
+
+ /* Release GPIO lines */
+ for (i = 0; i < ARRAY_SIZE(requested_gpios); i++) {
+ if (requested_gpios[i])
+ gpio_free(i);
+ }
+
+ mutex_lock(&drvdata->gpio_lock);
+ /* Disable IRQs generation */
+ port_bit_clear(0x7A, 7);
+ port_bit_clear(0x7D, 7);
+ if (use_lcdio)
+ port_bit_clear(0x7D, 6);
+
+ /* Release IO regions */
+ release_region(0x7A, 3);
+ release_region(0x7D, 3);
+ if (use_lcdio)
+ release_region(0x72, 2);
+ mutex_unlock(&drvdata->gpio_lock);
+
+ ret = gpiochip_remove(&drvdata->gpio_chip);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to remove the gpio chip\n");
+
+ kfree(drvdata);
+ 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)
+ goto error_out;
+
+ ret = platform_device_register(&ts5500_gpio_device);
+ if (ret)
+ goto error_device_register;
+
+ return 0;
+
+error_device_register:
+ platform_driver_unregister(&ts5500_gpio_driver);
+error_out:
+ return ret;
+}
+module_init(ts5500_gpio_init);
+
+static void __exit ts5500_gpio_exit(void)
+{
+ platform_driver_unregister(&ts5500_gpio_driver);
+ platform_device_unregister(&ts5500_gpio_device);
+}
+module_exit(ts5500_gpio_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Technologic Systems TS-5500, GPIO/DIO driver");
diff --git a/arch/x86/platform/ts5500/ts5500_gpio.h b/arch/x86/platform/ts5500/ts5500_gpio.h
new file mode 100644
index 0000000..eecd4fc
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500_gpio.h
@@ -0,0 +1,60 @@
+/*
+ * GPIO (DIO) driver for Technologic Systems TS-5500
+ *
+ * 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_GPIO_H
+#define _TS5500_GPIO_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_GPIO_H */
--
1.7.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 3/5] x86/platform: (TS-5500) add LED support
2012-02-01 21:05 [PATCH v5 0/5] Support for the TS-5500 platform Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 1/5] x86/platform: (TS-5500) add platform base support Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support Vivien Didelot
@ 2012-02-01 21:05 ` Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 4/5] hwmon: add MAX197 support Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 5/5] x86/platform: (TS-5500) add ADC support Vivien Didelot
4 siblings, 0 replies; 20+ messages in thread
From: Vivien Didelot @ 2012-02-01 21:05 UTC (permalink / raw)
To: x86
Cc: Jonas Fonseca, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare,
Vivien Didelot
From: Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
Signed-off-by: Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
arch/x86/platform/ts5500/Kconfig | 7 ++
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500_led.c | 179 +++++++++++++++++++++++++++++++++
3 files changed, 187 insertions(+), 0 deletions(-)
create mode 100644 arch/x86/platform/ts5500/ts5500_led.c
diff --git a/arch/x86/platform/ts5500/Kconfig b/arch/x86/platform/ts5500/Kconfig
index 62095c9..76f777f 100644
--- a/arch/x86/platform/ts5500/Kconfig
+++ b/arch/x86/platform/ts5500/Kconfig
@@ -13,3 +13,10 @@ config TS5500_GPIO
default y
help
This enables support for the DIO headers for GPIO usage.
+
+config TS5500_LED
+ tristate "TS-5500 LED Support"
+ depends on TS5500 && LEDS_CLASS
+ default y
+ help
+ This option enables support for the on-chip LED.
diff --git a/arch/x86/platform/ts5500/Makefile b/arch/x86/platform/ts5500/Makefile
index 71c1398..88eccc9 100644
--- a/arch/x86/platform/ts5500/Makefile
+++ b/arch/x86/platform/ts5500/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_TS5500) += ts5500.o
obj-$(CONFIG_TS5500_GPIO) += ts5500_gpio.o
+obj-$(CONFIG_TS5500_LED) += ts5500_led.o
diff --git a/arch/x86/platform/ts5500/ts5500_led.c b/arch/x86/platform/ts5500/ts5500_led.c
new file mode 100644
index 0000000..f4562fb
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500_led.c
@@ -0,0 +1,179 @@
+/*
+ * Technologic Systems TS-5500 boards - LED driver
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
+ *
+ * Portions Copyright (c) 2008 Compulab, Ltd.
+ * Mike Rapoport <mike@compulab.co.il>
+ *
+ * Portions Copyright (c) 2006-2008 Marvell International Ltd.
+ * Eric Miao <eric.miao@marvell.com>
+ *
+ * Based on drivers/leds/leds-da903x.c from linux-2.6.32.8.
+ *
+ * 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/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/leds.h>
+#include "ts5500.h"
+
+/**
+ * struct ts5500_led - LED structure
+ * @cdev: LED class device structure.
+ * @ioaddr: LED I/O address.
+ */
+struct ts5500_led {
+ struct led_classdev cdev;
+ int ioaddr;
+ int bit;
+};
+
+static void ts5500_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct ts5500_led *led = container_of(led_cdev, struct ts5500_led,
+ cdev);
+ outb(!!value, led->ioaddr);
+}
+
+static struct led_info ts5500_led_info = {
+ .name = "ts5500_led",
+ .default_trigger = "ts5500_led",
+ .flags = TS5500_LED_MASK
+};
+
+static struct led_platform_data ts5500_led_platform_data = {
+ .num_leds = 1,
+ .leds = &ts5500_led_info
+};
+
+static struct resource ts5500_led_resources[] = {
+ {
+ .name = "ts5500_led",
+ .start = TS5500_LED_JMPRS_REG,
+ .end = TS5500_LED_JMPRS_REG,
+ .flags = IORESOURCE_IO
+ }
+};
+
+static void ts5500_led_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_led_device = {
+ .name = "ts5500_led",
+ .resource = ts5500_led_resources,
+ .num_resources = ARRAY_SIZE(ts5500_led_resources),
+ .id = -1,
+ .dev = {
+ .platform_data = &ts5500_led_platform_data,
+ .release = ts5500_led_release
+ }
+};
+
+static int __devinit ts5500_led_probe(struct platform_device *pdev)
+{
+ struct led_platform_data *pdata = pdev->dev.platform_data;
+ struct ts5500_led *led;
+ struct resource *res;
+ int ret;
+
+ if (pdata == NULL || !pdata->num_leds) {
+ dev_err(&pdev->dev, "No platform data available\n");
+ return -ENODEV;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "Failed to get I/O resource\n");
+ return -EBUSY;
+ }
+
+ led = kzalloc(sizeof(struct ts5500_led), GFP_KERNEL);
+ if (led == NULL) {
+ dev_err(&pdev->dev, "Failed to alloc memory for LED device\n");
+ return -ENOMEM;
+ }
+
+ led->cdev.name = pdata->leds[0].name;
+ led->cdev.default_trigger = pdata->leds[0].default_trigger;
+ led->cdev.brightness_set = ts5500_led_set;
+ led->cdev.brightness = LED_OFF;
+
+ led->ioaddr = res->start;
+ led->bit = pdata->leds[0].flags;
+
+ ret = led_classdev_register(pdev->dev.parent, &led->cdev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register LED\n");
+ goto err;
+ }
+
+ platform_set_drvdata(pdev, led);
+ return 0;
+
+err:
+ kfree(led);
+ return ret;
+}
+
+static int __devexit ts5500_led_remove(struct platform_device *pdev)
+{
+ struct ts5500_led *led = platform_get_drvdata(pdev);
+
+ platform_set_drvdata(pdev, NULL);
+ led_classdev_unregister(&led->cdev);
+ kfree(led);
+
+ return 0;
+}
+
+static struct platform_driver ts5500_led_driver = {
+ .driver = {
+ .name = "ts5500_led",
+ .owner = THIS_MODULE
+ },
+ .probe = ts5500_led_probe,
+ .remove = __devexit_p(ts5500_led_remove)
+};
+
+static int __init ts5500_led_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&ts5500_led_driver);
+ if (ret)
+ goto error_out;
+
+ ret = platform_device_register(&ts5500_led_device);
+ if (ret)
+ goto error_device_register;
+
+ return 0;
+
+error_device_register:
+ platform_driver_unregister(&ts5500_led_driver);
+error_out:
+ return ret;
+}
+module_init(ts5500_led_init);
+
+static void __exit ts5500_led_exit(void)
+{
+ platform_driver_unregister(&ts5500_led_driver);
+ platform_device_unregister(&ts5500_led_device);
+}
+module_exit(ts5500_led_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LED driver for Technologic Systems TS-5500");
--
1.7.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-01 21:05 [PATCH v5 0/5] Support for the TS-5500 platform Vivien Didelot
` (2 preceding siblings ...)
2012-02-01 21:05 ` [PATCH v5 3/5] x86/platform: (TS-5500) add LED support Vivien Didelot
@ 2012-02-01 21:05 ` Vivien Didelot
2012-02-01 21:35 ` Guenter Roeck
2012-02-01 21:05 ` [PATCH v5 5/5] x86/platform: (TS-5500) add ADC support Vivien Didelot
4 siblings, 1 reply; 20+ messages in thread
From: Vivien Didelot @ 2012-02-01 21:05 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
Documentation/hwmon/max197 | 54 ++++++
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max197.c | 403 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/max197.h | 22 +++
5 files changed, 489 insertions(+), 0 deletions(-)
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..aa0934f
--- /dev/null
+++ b/Documentation/hwmon/max197
@@ -0,0 +1,54 @@
+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 two
+function pointers:
+
+* start:
+ The function which writes the control byte to start a new conversion.
+* read:
+ The function used to read the raw value 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.
+* in[0-7]_min: The lower limit (in mV) for the corresponding channel.
+ It can be one value in -10000, -5000, -4000, -2000, 0,
+ depending on the chip.
+* in[0-7]_max: The higher limit (in mV) for the corresponding channel.
+ It can be one value in 2000, 4000, 5000, 10000,
+ depending on the chip.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 91be41f..ccdf59b 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1360,6 +1360,15 @@ config SENSORS_MC13783_ADC
help
Support for the A/D converter on MC13783 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 8251ce8..dfb73d9 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -125,6 +125,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..985615c
--- /dev/null
+++ b/drivers/hwmon/max197.c
@@ -0,0 +1,403 @@
+/*
+ * 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 != 0) && (value != -half) && (value != -full))
+ return -EINVAL;
+ } else {
+ if ((value != half) && (value != full))
+ return -EINVAL;
+ }
+
+ 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->start(chip->ctrl_bytes[channel]);
+ if (ret) {
+ dev_err(dev, "start conversion failed\n");
+ goto unlock;
+ }
+
+ ret = chip->pdata->read(&raw);
+ if (ret) {
+ dev_err(dev, "raw read 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->start == NULL) {
+ dev_err(&pdev->dev, "no start function supplied\n");
+ ret = -EINVAL;
+ goto error_free_chip;
+ }
+
+ if (chip->pdata->read == NULL) {
+ dev_err(&pdev->dev, "no read 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_DESCRIPTION("Maxim MAX197 ADC driver");
diff --git a/include/linux/max197.h b/include/linux/max197.h
new file mode 100644
index 0000000..25ec203
--- /dev/null
+++ b/include/linux/max197.h
@@ -0,0 +1,22 @@
+/*
+ * 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
+ * @start: function pointer to start a conversion.
+ * @read: function pointer to read a raw conversion result.
+ */
+struct max197_platform_data {
+ int (*start)(u8 ctrl_byte);
+ int (*read)(u16 *raw);
+};
--
1.7.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 5/5] x86/platform: (TS-5500) add ADC support
2012-02-01 21:05 [PATCH v5 0/5] Support for the TS-5500 platform Vivien Didelot
` (3 preceding siblings ...)
2012-02-01 21:05 ` [PATCH v5 4/5] hwmon: add MAX197 support Vivien Didelot
@ 2012-02-01 21:05 ` Vivien Didelot
4 siblings, 0 replies; 20+ messages in thread
From: Vivien Didelot @ 2012-02-01 21:05 UTC (permalink / raw)
To: x86
Cc: Vivien Didelot, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
linux-kernel, lm-sensors, Guenter Roeck, Jean Delvare
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
arch/x86/platform/ts5500/Kconfig | 7 ++
arch/x86/platform/ts5500/Makefile | 1 +
arch/x86/platform/ts5500/ts5500_adc.c | 104 +++++++++++++++++++++++++++++++++
3 files changed, 112 insertions(+), 0 deletions(-)
create mode 100644 arch/x86/platform/ts5500/ts5500_adc.c
diff --git a/arch/x86/platform/ts5500/Kconfig b/arch/x86/platform/ts5500/Kconfig
index 76f777f..d6f5a8a 100644
--- a/arch/x86/platform/ts5500/Kconfig
+++ b/arch/x86/platform/ts5500/Kconfig
@@ -20,3 +20,10 @@ config TS5500_LED
default y
help
This option enables support for the on-chip LED.
+
+config TS5500_ADC
+ tristate "TS-5500 ADC"
+ depends on TS5500 && SENSORS_MAX197
+ default y
+ help
+ Support for the A/D converter on Technologic Systems TS-5500 SBCs.
diff --git a/arch/x86/platform/ts5500/Makefile b/arch/x86/platform/ts5500/Makefile
index 88eccc9..dcf46d8 100644
--- a/arch/x86/platform/ts5500/Makefile
+++ b/arch/x86/platform/ts5500/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_TS5500) += ts5500.o
obj-$(CONFIG_TS5500_GPIO) += ts5500_gpio.o
obj-$(CONFIG_TS5500_LED) += ts5500_led.o
+obj-$(CONFIG_TS5500_ADC) += ts5500_adc.o
diff --git a/arch/x86/platform/ts5500/ts5500_adc.c b/arch/x86/platform/ts5500/ts5500_adc.c
new file mode 100644
index 0000000..c55a283
--- /dev/null
+++ b/arch/x86/platform/ts5500/ts5500_adc.c
@@ -0,0 +1,104 @@
+/*
+ * Technologic Systems TS-5500 boards - Mapped MAX197 ADC driver
+ *
+ * Copyright (c) 2010-2012 Savoir-faire Linux Inc.
+ * Jonas Fonseca <jonas.fonseca@savoirfairelinux.com>
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * The TS-5500 uses a CPLD to abstract the interface to the MAX197.
+ * This driver should work unchanged with the MAX199 chip.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#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 TS5500_ADC_CTRL_REG 0x195 /* Conversion state register */
+#define TS5500_ADC_INIT_LSB_REG 0x196 /* Init conv. / LSB register */
+#define TS5500_ADC_MSB_REG 0x197 /* MSB register */
+#define TS5500_ADC_READ_DELAY 12 /* usec */
+#define TS5500_ADC_READ_BUSY_MASK 0x01
+
+static int ts5500_adc_start(u8 ctrl)
+{
+ /* Ensure the 3 MSB are set to 0 */
+ outb(ctrl & 0x1F, TS5500_ADC_INIT_LSB_REG);
+
+ return 0;
+}
+
+static int ts5500_adc_read(u16 *raw)
+{
+ u8 lsb, msb;
+
+ udelay(TS5500_ADC_READ_DELAY);
+ lsb = inb(TS5500_ADC_CTRL_REG);
+
+ if (lsb & TS5500_ADC_READ_BUSY_MASK)
+ return -EBUSY;
+
+ lsb = inb(TS5500_ADC_INIT_LSB_REG);
+ msb = inb(TS5500_ADC_MSB_REG);
+ *raw = (msb << 8) | lsb;
+
+ return 0;
+}
+
+static struct max197_platform_data ts5500_adc_platform_data = {
+ .start = ts5500_adc_start,
+ .read = ts5500_adc_read
+};
+
+static void ts5500_adc_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_adc_device = {
+ .name = "max197",
+ .id = -1,
+ .dev = {
+ .platform_data = &ts5500_adc_platform_data,
+ .release = ts5500_adc_release
+ }
+};
+
+static int __init ts5500_adc_init(void)
+{
+ return platform_device_register(&ts5500_adc_device);
+}
+module_init(ts5500_adc_init);
+
+static void __exit ts5500_adc_exit(void)
+{
+ platform_device_unregister(&ts5500_adc_device);
+}
+module_exit(ts5500_adc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("TS-5500 mapped MAX197 ADC device driver");
--
1.7.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-01 21:05 ` [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support Vivien Didelot
@ 2012-02-01 21:30 ` Alan Cox
2012-02-02 19:33 ` Guenter Roeck
2012-02-06 15:16 ` Mark Brown
1 sibling, 1 reply; 20+ messages in thread
From: Alan Cox @ 2012-02-01 21:30 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
> +obj-$(CONFIG_TS5500_GPIO) += ts5500_gpio.o
Wants to be gpi-ts5500 and in the drivers/gpio directory.
Nothing else particularly strikes me abou tit
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-01 21:05 ` [PATCH v5 4/5] hwmon: add MAX197 support Vivien Didelot
@ 2012-02-01 21:35 ` Guenter Roeck
2012-02-06 20:15 ` Vivien Didelot
0 siblings, 1 reply; 20+ messages in thread
From: Guenter Roeck @ 2012-02-01 21:35 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
Hi Vivien,
On Wed, 2012-02-01 at 16:05 -0500, Vivien Didelot wrote:
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Nicely done. Looks pretty good. Couple of minor comments below.
> ---
> Documentation/hwmon/max197 | 54 ++++++
> drivers/hwmon/Kconfig | 9 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/max197.c | 403 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/max197.h | 22 +++
> 5 files changed, 489 insertions(+), 0 deletions(-)
> 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..aa0934f
> --- /dev/null
> +++ b/Documentation/hwmon/max197
> @@ -0,0 +1,54 @@
> +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 two
> +function pointers:
> +
> +* start:
> + The function which writes the control byte to start a new conversion.
> +* read:
> + The function used to read the raw value 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.
> +* in[0-7]_min: The lower limit (in mV) for the corresponding channel.
> + It can be one value in -10000, -5000, -4000, -2000, 0,
> + depending on the chip.
> +* in[0-7]_max: The higher limit (in mV) for the corresponding channel.
> + It can be one value in 2000, 4000, 5000, 10000,
> + depending on the chip.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 91be41f..ccdf59b 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1360,6 +1360,15 @@ config SENSORS_MC13783_ADC
> help
> Support for the A/D converter on MC13783 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 8251ce8..dfb73d9 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -125,6 +125,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..985615c
> --- /dev/null
> +++ b/drivers/hwmon/max197.c
> @@ -0,0 +1,403 @@
> +/*
> + * 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);
> +}
> +
Interestingly enough, you don't need the !!() above. The C language
defines that the result of an operation assigned to a bool is always
converted to either 0 or 1. Not that it matters, really - I personally
actually prefer you style.
> +/**
> + * 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 != 0) && (value != -half) && (value != -full))
> + return -EINVAL;
> + } else {
> + if ((value != half) && (value != full))
> + return -EINVAL;
> + }
> +
Technically ok, except for the unnecessary ( ). Would be great if you
could remove those.
Since the actual limits are chip dependent, and not easily to figure out
for the ordinary user, it would be nice to either accept any number and
adjust it to one of the accepted values, or to explicitly spell out the
accepted the per-chip accepted values in the documentation (and not just
say "depending on the chip"). I'll leave it up to you to decide which
way to go.
Not too difficult - if you change the code, something like
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;
}
would be good enough.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-01 21:30 ` Alan Cox
@ 2012-02-02 19:33 ` Guenter Roeck
2012-02-06 15:37 ` Mark Brown
0 siblings, 1 reply; 20+ messages in thread
From: Guenter Roeck @ 2012-02-02 19:33 UTC (permalink / raw)
To: Alan Cox
Cc: Vivien Didelot, x86, Jerome Oufella, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, linux-kernel, lm-sensors,
Jean Delvare
On Wed, 2012-02-01 at 16:30 -0500, Alan Cox wrote:
> > +obj-$(CONFIG_TS5500_GPIO) += ts5500_gpio.o
>
> Wants to be gpi-ts5500 and in the drivers/gpio directory.
>
I would agree, though there seem to be some disagreements about where
such platform specific drivers should be located. From an earlier
exchange I had with Vivien about this matter:
> > > would be the appropriate location for a driver like this. As
> > > mentioned before, my strong preference is drivers/hwmon, but I
> > > would like to hear from others.
> >
> > We should either split every driver into corresponding
> > subdirectories, or put everything in a common platform directory.
> > My first RFC patches set has every driver separated. As they are
> > really specific to the platform, people seem to agree with grouping
> > them, mainly because they won't be shared. I changed that in the
> > following patches sets, and X86 maintainers seemed to be ok with
> > that.
> >
> > I'm ok with both solutions, but we should all agree on one.
> > Maybe we should have other maintainers view on this?
> >
> That is what I had asked for. I thought the whole point of per-module
> directories was to have all drivers there. If that is no longer true,
> fine with me; who am I to argue about something like that.
> I'd just like to know.
>
It looks like things are going back and forth a bit. If I were Vivien, I
would be a bit frustrated by now and be close to giving up (Vivien, I
really commend you for your patience).
Is there a written guideline or policy people can look and point to
if/when something like this comes up ? Otherwise we may have the
LED/GPIO/xxx maintainers point one way, the X86 maintainers point the
other way, and thus may have reached a complete deadlock.
Guenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-01 21:05 ` [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support Vivien Didelot
2012-02-01 21:30 ` Alan Cox
@ 2012-02-06 15:16 ` Mark Brown
1 sibling, 0 replies; 20+ messages in thread
From: Mark Brown @ 2012-02-06 15:16 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
On Wed, Feb 01, 2012 at 04:05:41PM -0500, Vivien Didelot wrote:
> arch/x86/platform/ts5500/Kconfig | 7 +
> arch/x86/platform/ts5500/Makefile | 1 +
> arch/x86/platform/ts5500/ts5500_gpio.c | 478 ++++++++++++++++++++++++++++++++
> arch/x86/platform/ts5500/ts5500_gpio.h | 60 ++++
As previously pointed out rather than hiding these drivers in arch/
directories there's a push to ship them in drivers/ where they benefit
from better subsystem review.
> +static int ts5500_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> + struct ts5500_drvdata *drvdata;
> +
> + drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
> +
> + mutex_lock(&drvdata->gpio_lock);
> + if (requested_gpios[offset]) {
> + mutex_unlock(&drvdata->gpio_lock);
> + return -EBUSY;
> + }
> + requested_gpios[offset] = 1;
> + mutex_unlock(&drvdata->gpio_lock);
This is all redundant, gpiolib will check this for you.
> +static void ts5500_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> +{
This (and get()) should be a _cansleep() operation because...
> + mutex_lock(&drvdata->gpio_lock);
> + if (val == 0)
> + port_bit_clear(ioaddr, bitno);
> + else
> + port_bit_set(ioaddr, bitno);
> + mutex_unlock(&drvdata->gpio_lock);
...you take a mutex which needs process context.
> +static int ts5500_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + /* Only a few lines are IRQ-Capable */
> + switch (offset) {
> + case TS5500_DIO1_13:
> + return TS5500_DIO1_13_IRQ;
> + case TS5500_DIO2_13:
> + return TS5500_DIO2_13_IRQ;
> + case TS5500_LCD_RS:
> + return TS5500_LCD_RS_IRQ;
Why are these numbers compile time constants?
> + /* Setup the gpio_chip structure */
> + drvdata = kzalloc(sizeof(struct ts5500_drvdata), GFP_KERNEL);
> + if (drvdata == NULL)
> + goto err_alloc_dev;
Looks like you'd benefit from using devm_ functions for most if not all
of the allocation.
> +/* Callback for releasing resources */
> +static void ts5500_gpio_device_release(struct device *dev)
> +{
> + /* noop */
> +}
> +
> +static struct platform_device ts5500_gpio_device = {
> + .name = "ts5500_gpio",
> + .id = -1,
> + .dev = {
> + .release = ts5500_gpio_device_release,
> + }
> +};
I'm not sure what this is all about but it looks fairly obviously wrong.
> +static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
> +{
> + struct ts5500_drvdata *drvdata;
> + int ret, i;
> +
> + drvdata = platform_get_drvdata(pdev);
> +
> + /* Release GPIO lines */
> + for (i = 0; i < ARRAY_SIZE(requested_gpios); i++) {
> + if (requested_gpios[i])
> + gpio_free(i);
> + }
You shouldn't be doing this, if it was sensible then gpiolib ought to be
doing it for you.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-02 19:33 ` Guenter Roeck
@ 2012-02-06 15:37 ` Mark Brown
2012-02-06 20:23 ` Vivien Didelot
0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-02-06 15:37 UTC (permalink / raw)
To: Guenter Roeck
Cc: Alan Cox, Vivien Didelot, x86, Jerome Oufella, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, linux-kernel, lm-sensors,
Jean Delvare
On Thu, Feb 02, 2012 at 11:33:56AM -0800, Guenter Roeck wrote:
> On Wed, 2012-02-01 at 16:30 -0500, Alan Cox wrote:
> > > My first RFC patches set has every driver separated. As they are
> > > really specific to the platform, people seem to agree with grouping
> > > them, mainly because they won't be shared. I changed that in the
> > > following patches sets, and X86 maintainers seemed to be ok with
> > > that.
> It looks like things are going back and forth a bit. If I were Vivien, I
> would be a bit frustrated by now and be close to giving up (Vivien, I
> really commend you for your patience).
OTOH I just looked back and saw that some of the review comments I just
made were also made against the first version of this driver I noticed
(v2) but appear to have been ignored, the request tracking stands out.
> Is there a written guideline or policy people can look and point to
> if/when something like this comes up ? Otherwise we may have the
> LED/GPIO/xxx maintainers point one way, the X86 maintainers point the
> other way, and thus may have reached a complete deadlock.
I'm not sure I'm seeing much conflict here TBH, looking at the above and
the history I have to hand I'd say it's reading like the x86 maintainers
aren't fussed either way and the people doing kernel wide work on things
like this prefer getting stuff integrated into the frameworks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-01 21:35 ` Guenter Roeck
@ 2012-02-06 20:15 ` Vivien Didelot
2012-02-06 20:46 ` Guenter Roeck
0 siblings, 1 reply; 20+ messages in thread
From: Vivien Didelot @ 2012-02-06 20:15 UTC (permalink / raw)
To: guenter.roeck
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
Le Wed, 1 Feb 2012 13:35:38 -0800,
Guenter Roeck <guenter.roeck@ericsson.com> a écrit :
> Hi Vivien,
>
> On Wed, 2012-02-01 at 16:05 -0500, Vivien Didelot wrote:
> > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>
> Nicely done. Looks pretty good. Couple of minor comments below.
>
> > ---
> > Documentation/hwmon/max197 | 54 ++++++
> > drivers/hwmon/Kconfig | 9 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/max197.c | 403
> > ++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/max197.h | 22 +++ 5 files changed, 489
> > insertions(+), 0 deletions(-) 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..aa0934f
> > --- /dev/null
> > +++ b/Documentation/hwmon/max197
> > @@ -0,0 +1,54 @@
> > +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 two +function pointers:
> > +
> > +* start:
> > + The function which writes the control byte to start a new
> > conversion. +* read:
> > + The function used to read the raw value 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. +* in[0-7]_min: The lower limit (in mV) for the
> > corresponding channel.
> > + It can be one value in -10000, -5000, -4000,
> > -2000, 0,
> > + depending on the chip.
> > +* in[0-7]_max: The higher limit (in mV) for the corresponding
> > channel.
> > + It can be one value in 2000, 4000, 5000, 10000,
> > + depending on the chip.
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 91be41f..ccdf59b 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -1360,6 +1360,15 @@ config SENSORS_MC13783_ADC
> > help
> > Support for the A/D converter on MC13783 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 8251ce8..dfb73d9 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -125,6 +125,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..985615c
> > --- /dev/null
> > +++ b/drivers/hwmon/max197.c
> > @@ -0,0 +1,403 @@
> > +/*
> > + * 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);
> > +}
> > +
> Interestingly enough, you don't need the !!() above. The C language
> defines that the result of an operation assigned to a bool is always
> converted to either 0 or 1. Not that it matters, really - I personally
> actually prefer you style.
Thanks for the tip, I'll get rid of it.
>
> > +/**
> > + * 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 != 0) && (value != -half) && (value !=
> > -full))
> > + return -EINVAL;
> > + } else {
> > + if ((value != half) && (value != full))
> > + return -EINVAL;
> > + }
> > +
> Technically ok, except for the unnecessary ( ). Would be great if you
> could remove those.
>
> Since the actual limits are chip dependent, and not easily to figure
> out for the ordinary user, it would be nice to either accept any
> number and adjust it to one of the accepted values, or to explicitly
> spell out the accepted the per-chip accepted values in the
> documentation (and not just say "depending on the chip"). I'll leave
> it up to you to decide which way to go.
>
> Not too difficult - if you change the code, something like
>
> 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;
> }
It sounds better to accept any value and adjust it depending on the chip.
>
> would be good enough.
>
> Thanks,
> Guenter
>
>
BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c file the
better solution, or should the device be declared in the ts5500.c
platform code?
Thanks!
--
Vivien Didelot
Savoir-faire Linux Inc.
Tel: (514) 276-5468 #149
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-06 15:37 ` Mark Brown
@ 2012-02-06 20:23 ` Vivien Didelot
2012-02-07 11:13 ` Mark Brown
0 siblings, 1 reply; 20+ messages in thread
From: Vivien Didelot @ 2012-02-06 20:23 UTC (permalink / raw)
To: Mark Brown
Cc: Guenter Roeck, Alan Cox, x86, Jerome Oufella, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, linux-kernel, lm-sensors,
Jean Delvare, Richard Purdie
Le Mon, 6 Feb 2012 15:37:42 +0000,
Mark Brown <broonie@opensource.wolfsonmicro.com> a écrit :
> On Thu, Feb 02, 2012 at 11:33:56AM -0800, Guenter Roeck wrote:
> > On Wed, 2012-02-01 at 16:30 -0500, Alan Cox wrote:
>
> > > > My first RFC patches set has every driver separated. As they are
> > > > really specific to the platform, people seem to agree with
> > > > grouping them, mainly because they won't be shared. I changed
> > > > that in the following patches sets, and X86 maintainers seemed
> > > > to be ok with that.
>
> > It looks like things are going back and forth a bit. If I were
> > Vivien, I would be a bit frustrated by now and be close to giving
> > up (Vivien, I really commend you for your patience).
>
> OTOH I just looked back and saw that some of the review comments I
> just made were also made against the first version of this driver I
> noticed (v2) but appear to have been ignored, the request tracking
> stands out.
>
> > Is there a written guideline or policy people can look and point to
> > if/when something like this comes up ? Otherwise we may have the
> > LED/GPIO/xxx maintainers point one way, the X86 maintainers point
> > the other way, and thus may have reached a complete deadlock.
>
> I'm not sure I'm seeing much conflict here TBH, looking at the above
> and the history I have to hand I'd say it's reading like the x86
> maintainers aren't fussed either way and the people doing kernel wide
> work on things like this prefer getting stuff integrated into the
> frameworks.
Thanks for the comments. I'll then move the GPIO driver back to
drivers/gpio and fix what Mark pointed out.
I Cc Richard Purdie, to have his maintainer view on the platform LED
declaration. Is it ok in the ts5500_led.c platform file, or should it
better be moved into drivers/leds/leds-ts5500.c, or maybe directly
declared in the main ts5500.c platform code?
Thanks,
Vivien.
--
Vivien Didelot
Savoir-faire Linux Inc.
Tel: (514) 276-5468 #149
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-06 20:15 ` Vivien Didelot
@ 2012-02-06 20:46 ` Guenter Roeck
2012-02-10 16:07 ` Vivien Didelot
0 siblings, 1 reply; 20+ messages in thread
From: Guenter Roeck @ 2012-02-06 20:46 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
Hi Vivien,
On Mon, 2012-02-06 at 15:15 -0500, Vivien Didelot wrote:
[ ... ]
>
> BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c file the
> better solution, or should the device be declared in the ts5500.c
> platform code?
>
I would suggest to declare it in the ts5500.c platform code. That seems
to be the common approach as far as I can see.
platform_add_devices() works pretty well for this. It saves you from
having to call platform_device_register() for each device separately.
Obviously that only works if all devices are declared in a single file.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support
2012-02-06 20:23 ` Vivien Didelot
@ 2012-02-07 11:13 ` Mark Brown
0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2012-02-07 11:13 UTC (permalink / raw)
To: Vivien Didelot
Cc: Guenter Roeck, Alan Cox, x86, Jerome Oufella, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, linux-kernel, lm-sensors,
Jean Delvare, Richard Purdie
[-- Attachment #1: Type: text/plain, Size: 473 bytes --]
On Mon, Feb 06, 2012 at 03:23:07PM -0500, Vivien Didelot wrote:
> I Cc Richard Purdie, to have his maintainer view on the platform LED
> declaration. Is it ok in the ts5500_led.c platform file, or should it
> better be moved into drivers/leds/leds-ts5500.c, or maybe directly
> declared in the main ts5500.c platform code?
It would normally be done in arch/ code. I'm not sure if Richard will
get back to you, he's been inactive lately (working on Yocto stuff
instead).
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-06 20:46 ` Guenter Roeck
@ 2012-02-10 16:07 ` Vivien Didelot
2012-02-10 16:15 ` Guenter Roeck
0 siblings, 1 reply; 20+ messages in thread
From: Vivien Didelot @ 2012-02-10 16:07 UTC (permalink / raw)
To: guenter.roeck
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
Hi,
On Mon, 6 Feb 2012 12:46:04 -0800,
Guenter Roeck <guenter.roeck@ericsson.com> wrote:
> On Mon, 2012-02-06 at 15:15 -0500, Vivien Didelot wrote:
> [ ... ]
> >
> > BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c file the
> > better solution, or should the device be declared in the ts5500.c
> > platform code?
> >
> I would suggest to declare it in the ts5500.c platform code. That
> seems to be the common approach as far as I can see.
>
> platform_add_devices() works pretty well for this. It saves you from
> having to call platform_device_register() for each device separately.
> Obviously that only works if all devices are declared in a single
> file.
As the LED is registered using the leds_class, I think
platform_add_devices() couldn't be used here.
Lots of platform codes don't check the returned
value of platform_add_devices(). Should we care about a LED or ADC
registration failure (is the following snippet OK?)?
static int __init ts5500_init(void)
{
[...]
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;
led_classdev_register(&pdev->dev, &ts5500_led_cdev);
if (ts5500->adc) {
ts5500_adc_pdev.dev.parent = &pdev->dev;
platform_device_register(&ts5500_adc_pdev);
}
return 0;
release_pdev:
platform_device_unregister(pdev);
release_mem:
kfree(ts5500);
return ret;
}
device_initcall(ts5500_init);
Thanks,
--
Vivien Didelot
Savoir-faire Linux Inc.
Tel: (514) 276-5468 #149
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-10 16:07 ` Vivien Didelot
@ 2012-02-10 16:15 ` Guenter Roeck
2012-02-10 18:14 ` Vivien Didelot
0 siblings, 1 reply; 20+ messages in thread
From: Guenter Roeck @ 2012-02-10 16:15 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
Vivien,
On Fri, Feb 10, 2012 at 11:07:55AM -0500, Vivien Didelot wrote:
> Hi,
>
> On Mon, 6 Feb 2012 12:46:04 -0800,
> Guenter Roeck <guenter.roeck@ericsson.com> wrote:
>
> > On Mon, 2012-02-06 at 15:15 -0500, Vivien Didelot wrote:
> > [ ... ]
> > >
> > > BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c file the
> > > better solution, or should the device be declared in the ts5500.c
> > > platform code?
> > >
> > I would suggest to declare it in the ts5500.c platform code. That
> > seems to be the common approach as far as I can see.
> >
> > platform_add_devices() works pretty well for this. It saves you from
> > having to call platform_device_register() for each device separately.
> > Obviously that only works if all devices are declared in a single
> > file.
>
> As the LED is registered using the leds_class, I think
> platform_add_devices() couldn't be used here.
>
> Lots of platform codes don't check the returned
> value of platform_add_devices(). Should we care about a LED or ADC
> registration failure (is the following snippet OK?)?
>
> static int __init ts5500_init(void)
> {
> [...]
> 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;
>
> led_classdev_register(&pdev->dev, &ts5500_led_cdev);
> if (ts5500->adc) {
> ts5500_adc_pdev.dev.parent = &pdev->dev;
> platform_device_register(&ts5500_adc_pdev);
> }
>
I didn't look at other code, but personally I try to be consistent.
Why do you check the return value from platform_device_register_simple() above,
but not the return code from platform_device_register() ?
That does not seem to be very consistent to me.
Thanks,
Guenter
> return 0;
>
> release_pdev:
> platform_device_unregister(pdev);
> release_mem:
> kfree(ts5500);
>
> return ret;
> }
> device_initcall(ts5500_init);
>
>
> Thanks,
>
>
> --
> Vivien Didelot
> Savoir-faire Linux Inc.
> Tel: (514) 276-5468 #149
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-10 16:15 ` Guenter Roeck
@ 2012-02-10 18:14 ` Vivien Didelot
2012-02-20 18:27 ` Guenter Roeck
0 siblings, 1 reply; 20+ messages in thread
From: Vivien Didelot @ 2012-02-10 18:14 UTC (permalink / raw)
To: Guenter Roeck
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
Le Fri, 10 Feb 2012 08:15:38 -0800,
Guenter Roeck <guenter.roeck@ericsson.com> a écrit :
> Vivien,
>
> On Fri, Feb 10, 2012 at 11:07:55AM -0500, Vivien Didelot wrote:
> > Hi,
> >
> > On Mon, 6 Feb 2012 12:46:04 -0800,
> > Guenter Roeck <guenter.roeck@ericsson.com> wrote:
> >
> > > On Mon, 2012-02-06 at 15:15 -0500, Vivien Didelot wrote:
> > > [ ... ]
> > > >
> > > > BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c
> > > > file the better solution, or should the device be declared in
> > > > the ts5500.c platform code?
> > > >
> > > I would suggest to declare it in the ts5500.c platform code. That
> > > seems to be the common approach as far as I can see.
> > >
> > > platform_add_devices() works pretty well for this. It saves you
> > > from having to call platform_device_register() for each device
> > > separately. Obviously that only works if all devices are declared
> > > in a single file.
> >
> > As the LED is registered using the leds_class, I think
> > platform_add_devices() couldn't be used here.
> >
> > Lots of platform codes don't check the returned
> > value of platform_add_devices(). Should we care about a LED or ADC
> > registration failure (is the following snippet OK?)?
> >
> > static int __init ts5500_init(void)
> > {
> > [...]
> > 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;
> >
> > led_classdev_register(&pdev->dev, &ts5500_led_cdev);
> > if (ts5500->adc) {
> > ts5500_adc_pdev.dev.parent = &pdev->dev;
> > platform_device_register(&ts5500_adc_pdev);
> > }
> >
> I didn't look at other code, but personally I try to be consistent.
> Why do you check the return value from
> platform_device_register_simple() above, but not the return code from
> platform_device_register() ? That does not seem to be very consistent
> to me.
I check the platform_device_register_simple() returned value because it
is the platform itself, while the others are on-board devices. I
thought that it is not a big deal if their registrations failed but the
platform registration succeeded. Maybe I'm wrong and I should check
everything.
Thanks,
Vivien.
>
> Thanks,
> Guenter
>
> > return 0;
> >
> > release_pdev:
> > platform_device_unregister(pdev);
> > release_mem:
> > kfree(ts5500);
> >
> > return ret;
> > }
> > device_initcall(ts5500_init);
> >
> >
> > Thanks,
> >
> >
> > --
> > Vivien Didelot
> > Savoir-faire Linux Inc.
> > Tel: (514) 276-5468 #149
--
Vivien Didelot
Savoir-faire Linux Inc.
Tel: (514) 276-5468 #149
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-10 18:14 ` Vivien Didelot
@ 2012-02-20 18:27 ` Guenter Roeck
2012-02-20 19:56 ` Vivien Didelot
0 siblings, 1 reply; 20+ messages in thread
From: Guenter Roeck @ 2012-02-20 18:27 UTC (permalink / raw)
To: Vivien Didelot
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
On Fri, Feb 10, 2012 at 01:14:06PM -0500, Vivien Didelot wrote:
> Le Fri, 10 Feb 2012 08:15:38 -0800,
> Guenter Roeck <guenter.roeck@ericsson.com> a écrit :
>
> > Vivien,
> >
> > On Fri, Feb 10, 2012 at 11:07:55AM -0500, Vivien Didelot wrote:
> > > Hi,
> > >
> > > On Mon, 6 Feb 2012 12:46:04 -0800,
> > > Guenter Roeck <guenter.roeck@ericsson.com> wrote:
> > >
> > > > On Mon, 2012-02-06 at 15:15 -0500, Vivien Didelot wrote:
> > > > [ ... ]
> > > > >
> > > > > BTW, about the TS-5500 ADC part, is a platform ts5500_adc.c
> > > > > file the better solution, or should the device be declared in
> > > > > the ts5500.c platform code?
> > > > >
> > > > I would suggest to declare it in the ts5500.c platform code. That
> > > > seems to be the common approach as far as I can see.
> > > >
> > > > platform_add_devices() works pretty well for this. It saves you
> > > > from having to call platform_device_register() for each device
> > > > separately. Obviously that only works if all devices are declared
> > > > in a single file.
> > >
> > > As the LED is registered using the leds_class, I think
> > > platform_add_devices() couldn't be used here.
> > >
> > > Lots of platform codes don't check the returned
> > > value of platform_add_devices(). Should we care about a LED or ADC
> > > registration failure (is the following snippet OK?)?
> > >
> > > static int __init ts5500_init(void)
> > > {
> > > [...]
> > > 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;
> > >
> > > led_classdev_register(&pdev->dev, &ts5500_led_cdev);
> > > if (ts5500->adc) {
> > > ts5500_adc_pdev.dev.parent = &pdev->dev;
> > > platform_device_register(&ts5500_adc_pdev);
> > > }
> > >
> > I didn't look at other code, but personally I try to be consistent.
> > Why do you check the return value from
> > platform_device_register_simple() above, but not the return code from
> > platform_device_register() ? That does not seem to be very consistent
> > to me.
>
> I check the platform_device_register_simple() returned value because it
> is the platform itself, while the others are on-board devices. I
> thought that it is not a big deal if their registrations failed but the
> platform registration succeeded. Maybe I'm wrong and I should check
> everything.
>
Hmm .. seems to make sense. Ok with me. Only question is if you would want
to have it fail silently or issue a log message (possibly debug) to report
the failure.
Guenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] hwmon: add MAX197 support
2012-02-20 18:27 ` Guenter Roeck
@ 2012-02-20 19:56 ` Vivien Didelot
0 siblings, 0 replies; 20+ messages in thread
From: Vivien Didelot @ 2012-02-20 19:56 UTC (permalink / raw)
To: Guenter Roeck
Cc: x86, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel,
lm-sensors, Jean Delvare
On Mon, 20 Feb 2012 10:27:02 -0800,
Guenter Roeck <guenter.roeck@ericsson.com> wrote:
> > I check the platform_device_register_simple() returned value
> > because it is the platform itself, while the others are on-board
> > devices. I thought that it is not a big deal if their registrations
> > failed but the platform registration succeeded. Maybe I'm wrong and
> > I should check everything.
> >
> Hmm .. seems to make sense. Ok with me. Only question is if you would
> want to have it fail silently or issue a log message (possibly debug)
> to report the failure.
Ok, I'll go for something like the following:
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");
}
Thanks,
--
Vivien Didelot
Savoir-faire Linux Inc.
Tel: (514) 276-5468 #149
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2012-02-20 19:56 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-01 21:05 [PATCH v5 0/5] Support for the TS-5500 platform Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 1/5] x86/platform: (TS-5500) add platform base support Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 2/5] x86/platform: (TS-5500) add GPIO support Vivien Didelot
2012-02-01 21:30 ` Alan Cox
2012-02-02 19:33 ` Guenter Roeck
2012-02-06 15:37 ` Mark Brown
2012-02-06 20:23 ` Vivien Didelot
2012-02-07 11:13 ` Mark Brown
2012-02-06 15:16 ` Mark Brown
2012-02-01 21:05 ` [PATCH v5 3/5] x86/platform: (TS-5500) add LED support Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 4/5] hwmon: add MAX197 support Vivien Didelot
2012-02-01 21:35 ` Guenter Roeck
2012-02-06 20:15 ` Vivien Didelot
2012-02-06 20:46 ` Guenter Roeck
2012-02-10 16:07 ` Vivien Didelot
2012-02-10 16:15 ` Guenter Roeck
2012-02-10 18:14 ` Vivien Didelot
2012-02-20 18:27 ` Guenter Roeck
2012-02-20 19:56 ` Vivien Didelot
2012-02-01 21:05 ` [PATCH v5 5/5] x86/platform: (TS-5500) add ADC support Vivien Didelot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).