2011-04-29 22:28:20

by Vivien Didelot

[permalink] [raw]
Subject: [RFC 0/5] Support for Technologic Systems TS-5500 board

This set of patches is a request for comments. It brings the complete support
for the TS-5500 Single Board Computer from Technologic Systems.

The first patch adds detection for TS-5xxx boards (it also works for TS-3xxx
boards).

The second patch adds GPIO support.

The third patch adds support for the RS-485 feature of the ts-5500 SBC.

The fourth patch adds support for the TS-5500 LEDs.

The fifth patch adds support for the analogic to digital converter feature of
the board.

Jerome Oufella (1):
gpio: add support for Technologic Systems TS-5500 GPIOs

Jonas Fonseca (3):
platform-drivers-x86: add support for Technologic Systems TS-5xxx
detection
leds: add support for Technologic Systems TS-5500 leds
hwmon: add support for Technologic Systems TS-5500 A-D converter

Vivien Didelot (1):
serial: add support for Technologic Systems TS-5500 RS-485 serial
port

Documentation/ts5xxx-sbcinfo.txt | 47 +++
MAINTAINERS | 11 +
drivers/gpio/Kconfig | 10 +
drivers/gpio/Makefile | 1 +
drivers/gpio/ts5500-gpio.c | 528 +++++++++++++++++++++++++++++++++
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/ts5500-adc.c | 481 ++++++++++++++++++++++++++++++
drivers/leds/Kconfig | 12 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-ts5500.c | 211 +++++++++++++
drivers/platform/x86/Kconfig | 11 +
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/ts5xxx-sbcinfo.c | 254 ++++++++++++++++
drivers/tty/serial/8250.c | 30 ++
drivers/tty/serial/Kconfig | 19 ++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/ts5500-auto485.c | 45 +++
include/linux/ts5500-gpio.h | 68 +++++
include/linux/ts5xxx-sbcinfo.h | 42 +++
20 files changed, 1784 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ts5xxx-sbcinfo.txt
create mode 100644 drivers/gpio/ts5500-gpio.c
create mode 100644 drivers/hwmon/ts5500-adc.c
create mode 100644 drivers/leds/leds-ts5500.c
create mode 100644 drivers/platform/x86/ts5xxx-sbcinfo.c
create mode 100644 drivers/tty/serial/ts5500-auto485.c
create mode 100644 include/linux/ts5500-gpio.h
create mode 100644 include/linux/ts5xxx-sbcinfo.h


2011-04-29 22:28:18

by Vivien Didelot

[permalink] [raw]
Subject: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

From: Jonas Fonseca <[email protected]>


Signed-off-by: Vivien Didelot <[email protected]>
---
Documentation/ts5xxx-sbcinfo.txt | 47 ++++++
MAINTAINERS | 6 +
drivers/platform/x86/Kconfig | 11 ++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/ts5xxx-sbcinfo.c | 254 +++++++++++++++++++++++++++++++++
include/linux/ts5xxx-sbcinfo.h | 42 ++++++
6 files changed, 361 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ts5xxx-sbcinfo.txt
create mode 100644 drivers/platform/x86/ts5xxx-sbcinfo.c
create mode 100644 include/linux/ts5xxx-sbcinfo.h

diff --git a/Documentation/ts5xxx-sbcinfo.txt b/Documentation/ts5xxx-sbcinfo.txt
new file mode 100644
index 0000000..54905b1
--- /dev/null
+++ b/Documentation/ts5xxx-sbcinfo.txt
@@ -0,0 +1,47 @@
+TS-5xxx boards detection
+========================
+
+Supported boards:
+ * Technologic Systems TS-3100
+ Manual: http://www.embeddedarm.com/documentation/ts-3100-manual.pdf
+
+ * Technologic Systems TS-3200
+ Manual: http://www.embeddedarm.com/documentation/ts-3200-manual.pdf
+
+ * Technologic Systems TS-3300
+ Manual: http://www.embeddedarm.com/documentation/ts-3300-manual.pdf
+
+ * Technologic Systems TS-3400
+ Manual: http://www.embeddedarm.com/documentation/ts-3400-manual.pdf
+
+ * Technologic Systems TS-5300
+ Manual: http://www.embeddedarm.com/documentation/ts-5300-manual.pdf
+
+ * Technologic Systems TS-5400
+ Manual: http://www.embeddedarm.com/documentation/ts-5400-manual.pdf
+
+ * Technologic Systems TS-5500
+ Manual: http://www.embeddedarm.com/documentation/ts-5500-manual.pdf
+
+ * Technologic Systems TS-5600
+ Manual: http://www.embeddedarm.com/documentation/ts-5600-manual.pdf
+
+ * Technologic Systems TS-5700
+ Manual: http://www.embeddedarm.com/documentation/ts-5700-manual.pdf
+
+Authors:
+ Liberty Young <[email protected]>
+ Jonas Fonseca <[email protected]>
+ Alexandre Savard <[email protected]>
+
+Description
+-----------
+
+The ts5xxx-sbcinfo driver provides detection for Technologic Systems TS-5xxx
+Single Board Computers. This driver also works with TS-3xxx boards.
+
+/proc filesystem
+----------------
+
+Information about the TS board is available through the /proc/ts-sbcinfo.
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 1380312..b077e6d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6053,6 +6053,12 @@ W: http://tcp-lp-mod.sourceforge.net/
S: Maintained
F: net/ipv4/tcp_lp.c

+TECHNOLOGIC SYSTEMS TS5500 MACHINE SUPPORT
+M: Savoir-faire Linux Inc. <[email protected]>
+S: Maintained
+F: drivers/platform/x86/ts5xxx-sbcinfo.c
+F: include/linux/ts5xxx-sbcinfo.h
+
TEGRA SUPPORT
M: Colin Cross <[email protected]>
M: Erik Gilling <[email protected]>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0485e39..5c25da2 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -753,4 +753,15 @@ config SAMSUNG_LAPTOP
To compile this driver as a module, choose M here: the module
will be called samsung-laptop.

+config TS5500_SBC
+ tristate "Technologic Systems TS-5500 SBC support"
+ depends on X86_ELAN
+ ---help---
+ This enables support for the Technologic Systems TS-5500 platform.
+
+ It also gives access to specific device informations in the
+ /proc/sbcinfo file.
+
+ If you have a TS-5500, say Y here.
+
endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 029e886..e47b449 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -42,3 +42,4 @@ obj-$(CONFIG_XO15_EBOOK) += xo15-ebook.o
obj-$(CONFIG_IBM_RTL) += ibm_rtl.o
obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o
obj-$(CONFIG_INTEL_MFLD_THERMAL) += intel_mid_thermal.o
+obj-$(CONFIG_TS5500_SBC) += ts5xxx-sbcinfo.o
diff --git a/drivers/platform/x86/ts5xxx-sbcinfo.c b/drivers/platform/x86/ts5xxx-sbcinfo.c
new file mode 100644
index 0000000..ea9501b
--- /dev/null
+++ b/drivers/platform/x86/ts5xxx-sbcinfo.c
@@ -0,0 +1,254 @@
+/*
+ * Technologic Systems TS-5xxx boards - SBC info layer
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Alexandre Savard <[email protected]>
+ * Jonas Fonseca <[email protected]>
+ *
+ * Portions originate from ts_sbcinfo.c (c) Technologic Systems
+ * Liberty Young <[email protected]>
+ *
+ * These functions add a proc ts-sbcinfo entry to display information
+ * about the Single Board Computer (SBC).
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/proc_fs.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/io.h>
+
+#include <linux/ts5xxx-sbcinfo.h>
+
+#define PROCFS_NAME "ts-sbcinfo"
+
+#define PROCFS_MAX_SIZE 1024
+
+#define IOADDR_SBCID 0x74
+#define IOADDR_SRAM 0x75
+#define IOADDR_RESTOP 0x76
+#define IOADDR_LEDJP 0x77
+
+/* Structure containing the Single Board Computer's info */
+static struct ts5xxx_sbcinfo ts_sbcinfo;
+
+static struct proc_dir_entry *proc_entry;
+static char procfs_buffer[PROCFS_MAX_SIZE];
+static unsigned long procfs_buffer_size;
+
+/**
+ * ts5xxx_sbcinfo_set() - set the SBC info structure with the current SBC's info
+ * @sbcinfo: structure containing SBC info to set.
+ */
+void ts5xxx_sbcinfo_set(struct ts5xxx_sbcinfo *sbcinfo)
+{
+ memcpy(sbcinfo, &ts_sbcinfo, sizeof(*sbcinfo));
+}
+EXPORT_SYMBOL(ts5xxx_sbcinfo_set);
+
+/**
+ * struct ts_sbc_config - TS SBC configuration
+ * @ref: SBC's reference.
+ * @id: ID read from the id register.
+ * @sram: Bit to indicate the existence of SRAM.
+ * @adc: Bit for analogic to digital converter.
+ * @rs485: Bit for RS485.
+ * @auto485: Bit for auto 485.
+ * @external_reset: Bit for external reset feature.
+ * @jumpers: Mask to list connected jumpers.
+ */
+struct ts_sbc_config {
+ int ref;
+ u8 id;
+ u8 sram;
+ u8 adc;
+ u8 rs485;
+ u8 auto485;
+ u8 external_reset;
+ u8 jumpers;
+};
+
+#define NONE 0x00
+#define ALWAYS 0xFF
+
+/* TS SBCs configurations */
+struct ts_sbc_config ts_sbcs_configs[] = {
+ /* Ref ID SRAM ADC RS485 Auto485 E-Reset Jprs */
+ { 3100, 0x01, NONE, NONE, NONE, NONE, NONE, NONE },
+ { 3200, 0x02, 0x01, NONE, NONE, NONE, NONE, NONE },
+ { 3300, 0x03, 0x01, 0x04, NONE, NONE, NONE, NONE },
+ { 3400, 0x04, 0x01, NONE, NONE, NONE, 0x01, NONE },
+ { 5300, 0x50, 0x01, NONE, 0x02, 0x0A, NONE, 0xFE },
+ { 5400, 0x40, NONE, NONE, 0x02, 0x02, NONE, 0xFE },
+ { 5500, 0x60, NONE, 0x04, 0x02, 0x02, 0x01, 0xFE },
+ { 5600, 0x20, 0x01, ALWAYS, 0x02, 0x02, 0x01, 0xFE },
+ { 5700, 0x70, NONE, NONE, 0x02, NONE, ALWAYS, 0xFE },
+};
+
+/**
+ * ts_find_sbc_config() - find a SBC configuration from an id
+ * @id: ID of the board to find.
+ */
+static inline struct ts_sbc_config *ts_find_sbc_config(u8 id)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ts_sbcs_configs); i++)
+ if (id == ts_sbcs_configs[i].id)
+ return &ts_sbcs_configs[i];
+
+ return NULL;
+}
+
+/**
+ * ts_sbcfeature() - detect if a feature is enabled or not
+ * @info: SBC's info structure to update.
+ * @sbc: Configuration of the SBC.
+ * @reg: Register containing the value.
+ * @feature: Structure field to update.
+ */
+#define ts_sbcfeature(info, sbc, reg, feature) do { \
+ (info)->feature = (sbc)->feature == ALWAYS || \
+ !!((reg) & (sbc)->feature); \
+ } while (0)
+
+/**
+ * ts_sbcinfo_detect() - detect the TS board
+ * @sbcinfo: structure where to store the detected board's info.
+ */
+static int ts_sbcinfo_detect(struct ts5xxx_sbcinfo *sbcinfo)
+{
+ u8 temp;
+ struct ts_sbc_config *sbc;
+ int ret = 0;
+
+ memset(sbcinfo, 0, sizeof(*sbcinfo));
+
+ if (!request_region(IOADDR_SBCID, 4, "TS-SBC"))
+ return -EBUSY;
+
+ temp = inb(IOADDR_SBCID);
+ /* If it is a 3x00 SBC only match against the first 3 bits */
+ if (temp & 0x07)
+ temp &= 0x07;
+
+ sbc = ts_find_sbc_config(temp);
+ if (!sbc) {
+ ret = -ENODEV;
+ goto error;
+ }
+
+ sbcinfo->board_id = sbc->ref;
+
+ temp = inb(IOADDR_SRAM);
+ ts_sbcfeature(sbcinfo, sbc, temp, sram);
+ ts_sbcfeature(sbcinfo, sbc, temp, adc);
+ ts_sbcfeature(sbcinfo, sbc, temp, rs485);
+ ts_sbcfeature(sbcinfo, sbc, temp, auto485);
+
+ temp = inb(IOADDR_RESTOP);
+ sbcinfo->industrial = !!(temp & 0x02);
+ ts_sbcfeature(sbcinfo, sbc, temp, external_reset);
+
+ temp = inb(IOADDR_LEDJP);
+ sbcinfo->jumpers = temp & sbc->jumpers;
+
+error:
+ release_region(IOADDR_SBCID, 4);
+ return ret;
+}
+
+#define ts_addbuf(buf, name, fmt, a...) \
+ sprintf(buf, name ":%s" fmt "\n", \
+ &" "[sizeof(name) - 1], a)
+
+static int ts_sbcinfo_init_buffer(char *buf, struct ts5xxx_sbcinfo *sbcinfo)
+{
+ char *pos = buf;
+
+ pos += ts_addbuf(pos, "Board ID", "TS-%d", sbcinfo->board_id);
+ pos += ts_addbuf(pos, "RS485", "%s", sbcinfo->rs485 ? "yes" : "no");
+ pos += ts_addbuf(pos, "AnalogToDigital", "%s",
+ sbcinfo->adc ? "yes" : "no");
+ pos += ts_addbuf(pos, "Auto485", "%s", sbcinfo->auto485 ? "yes" : "no");
+ pos += ts_addbuf(pos, "SRAM", "%s", sbcinfo->sram ? "yes" : "no");
+ pos += ts_addbuf(pos, "External Reset", "%s",
+ sbcinfo->external_reset ? "yes" : "no");
+
+ if (sbcinfo->jumpers) {
+ pos += ts_addbuf(pos, "JPS", "%s%s%s%s%s%s",
+ sbcinfo->jumpers & 0x02 ? "JP1 " : "",
+ sbcinfo->jumpers & 0x04 ? "JP2 " : "",
+ sbcinfo->jumpers & 0x08 ? "JP3 " : "",
+ sbcinfo->jumpers & 0x10 ? "JP4 " : "",
+ sbcinfo->jumpers & 0x20 ? "JP5 " : "",
+ sbcinfo->jumpers & 0x80 ?
+ (sbcinfo->board_id == 5300 ? "JP8" : "JP6")
+ : "");
+ }
+
+ return pos - buf;
+}
+
+/**
+ * ts_sbcinfo_proc_read() - function called when a read access is done on
+ * /proc/ts-sbcinfo
+ */
+static int ts_sbcinfo_proc_read(char *page, char **start, off_t off,
+ int count, int *eof, void *data)
+{
+ int to_copy = (procfs_buffer_size <= count) ?
+ procfs_buffer_size - off : count;
+
+ if (off + to_copy >= procfs_buffer_size) {
+ to_copy = procfs_buffer_size - off;
+ *eof = 1;
+ }
+
+ if (to_copy <= 0)
+ return 0;
+
+ *start = page + off;
+ memcpy(*start, procfs_buffer + off, to_copy);
+
+ return to_copy;
+}
+
+static int __init ts5xxx_sbcinfo_init(void)
+{
+ int err;
+
+ err = ts_sbcinfo_detect(&ts_sbcinfo);
+ if (err < 0) {
+ printk(KERN_ERR KBUILD_MODNAME
+ ": Failed to get SBC information\n");
+ return err;
+ }
+
+ proc_entry = create_proc_read_entry(PROCFS_NAME, S_IRUGO, NULL,
+ ts_sbcinfo_proc_read, 0);
+ if (proc_entry == NULL) {
+ printk(KERN_ERR KBUILD_MODNAME
+ ": Failed to create proc entry\n");
+ return -ENOMEM;
+ }
+
+ procfs_buffer_size = ts_sbcinfo_init_buffer(procfs_buffer, &ts_sbcinfo);
+ printk(KBUILD_MODNAME ": TS SBC's info driver loaded.\n");
+
+ return 0;
+}
+postcore_initcall(ts5xxx_sbcinfo_init);
+
+static void __exit ts5xxx_sbcinfo_exit(void)
+{
+ remove_proc_entry(proc_entry->name, proc_entry->parent);
+ proc_entry = NULL;
+}
+module_exit(ts5xxx_sbcinfo_exit);
+
+MODULE_AUTHOR("Jonas Fonseca <[email protected]>");
+MODULE_AUTHOR("Alexandre Savard <[email protected]>");
+MODULE_DESCRIPTION("Technologic Systems SingleBoardComputer /proc driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/ts5xxx-sbcinfo.h b/include/linux/ts5xxx-sbcinfo.h
new file mode 100644
index 0000000..0240e5c
--- /dev/null
+++ b/include/linux/ts5xxx-sbcinfo.h
@@ -0,0 +1,42 @@
+/*
+ * Technologic Systems TS-5xxx boards - SBC info layer
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Alexandre Savard <[email protected]>
+ *
+ * Portions originate from ts_sbcinfo.h (c) Technologic Systems
+ * Liberty Young <[email protected]>
+ */
+
+#ifndef _LINUX_TS5XXX_SBCINFO_H
+#define _LINUX_TS5XXX_SBCINFO_H
+
+/**
+ * struct ts5xxx_sbcinfo - Describes the SBC and options installed
+ * @board_id: Board name.
+ * @jumpers: Connected jumpers.
+ * @rs485: Flag to indicate the existence of RS485.
+ * @adc: Analogic to digital converter?
+ * @rs422: RS422 available?
+ * @ethernet: Ethernet port available?
+ * @auto485: Auto 485 available?
+ * @external_reset: External reset available?
+ * @sram: Presence of SRAM available?
+ * @industrial: Industrial temperature.
+ */
+struct ts5xxx_sbcinfo {
+ int board_id;
+ u8 jumpers;
+ bool rs485;
+ bool adc;
+ bool rs422;
+ bool ethernet;
+ bool auto485;
+ bool external_reset;
+ bool sram;
+ bool industrial;
+};
+
+extern void ts5xxx_sbcinfo_set(struct ts5xxx_sbcinfo *sbcinfo);
+
+#endif
--
1.7.1

2011-04-29 22:28:19

by Vivien Didelot

[permalink] [raw]
Subject: [RFC 3/5] serial: add support for Technologic Systems TS-5500 RS-485 serial port


Signed-off-by: Vivien Didelot <[email protected]>
---
MAINTAINERS | 1 +
drivers/tty/serial/8250.c | 30 +++++++++++++++++++++++
drivers/tty/serial/Kconfig | 19 ++++++++++++++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/ts5500-auto485.c | 45 +++++++++++++++++++++++++++++++++++
5 files changed, 96 insertions(+), 0 deletions(-)
create mode 100644 drivers/tty/serial/ts5500-auto485.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 32c2c57..da3b6d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6060,6 +6060,7 @@ F: drivers/platform/x86/ts5xxx-sbcinfo.c
F: include/linux/ts5xxx-sbcinfo.h
F: drivers/gpio/ts5500-gpio.c
F: include/linux/ts5500-gpio.h
+F: drivers/tty/serial/ts5500-auto485.c

TEGRA SUPPORT
M: Colin Cross <[email protected]>
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 6611535..3ed7fee 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -109,6 +109,13 @@ static unsigned int skip_txen_test; /* force skip of txen test at init time */
#define CONFIG_HUB6 1

#include <asm/serial.h>
+
+/* TS-5500 related stuff */
+#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
+#define TS5500_TIMER2_SPEED_ADDR 0x42
+#define TS5500_485_SERIAL_PORT 0x02
+#endif
+
/*
* SERIAL_PORT_DFNS tells us about built-in ports that have no
* standard enumeration mechanism. Platforms that can find all
@@ -437,6 +444,25 @@ static void au_serial_out(struct uart_port *p, int offset, int value)
__raw_writel(value, p->membase + offset);
}

+#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
+void serial8250_ts5500_set_termios(struct uart_port *port,
+ struct ktermios *new,
+ struct ktermios *old)
+{
+ u16 speed;
+
+ if (new->c_ospeed >= 9600 && port->line == TS5500_485_SERIAL_PORT) {
+ speed = ((115200 * 2) / new->c_ospeed);
+
+ /* This should be written by low byte followed by high byte */
+ spin_lock_irq(&port->lock);
+ outb((speed & 0x0F), TS5500_TIMER2_SPEED_ADDR);
+ outb((speed >> 8), TS5500_TIMER2_SPEED_ADDR);
+ spin_unlock_irq(&port->lock);
+ }
+}
+#endif
+
static unsigned int tsi_serial_in(struct uart_port *p, int offset)
{
unsigned int tmp;
@@ -2464,6 +2490,10 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);
+
+#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
+ serial8250_ts5500_set_termios(port, termios, old);
+#endif
}
EXPORT_SYMBOL(serial8250_do_set_termios);

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 80484af..cbd88c0 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -241,6 +241,25 @@ config SERIAL_8250_RSA
help
::: To be written :::

+config SERIAL_8250_TS5500_485_TIMER
+ bool "Support RS-485 mode on TS-5500"
+ depends on TS5500_SBC
+ depends on SERIAL_8250_EXTENDED
+ help
+ Say Y here to enable RS-485 timer programming for the COM3 serial
+ port on Technologic Systems TS-5500 SBCs.
+ Note: this does not affect the RS-232 behaviour on the COM3 port.
+
+config SERIAL_8250_TS5500_AUTO485
+ tristate "Support Auto485 feature on TS-5500"
+ depends on SERIAL_8250_TS5500_485_TIMER
+ help
+ Say Y or M here to enable the Auto485 feature on Technologic Systems
+ TS-5500 SBCs.
+ Warning: When loaded, the Auto485 feature is enabled
+ as long as the module is loaded, so you won't be able to disable it
+ if the module is statically linked in.
+
config SERIAL_8250_MCA
tristate "Support 8250-type ports on MCA buses"
depends on SERIAL_8250 != n && MCA
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index fee0690..3fbcbbf 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -94,3 +94,4 @@ obj-$(CONFIG_SERIAL_IFX6X60) += ifx6x60.o
obj-$(CONFIG_SERIAL_PCH_UART) += pch_uart.o
obj-$(CONFIG_SERIAL_MSM_SMD) += msm_smd_tty.o
obj-$(CONFIG_SERIAL_MXS_AUART) += mxs-auart.o
+obj-$(CONFIG_SERIAL_8250_TS5500_AUTO485) += ts5500-auto485.o
diff --git a/drivers/tty/serial/ts5500-auto485.c b/drivers/tty/serial/ts5500-auto485.c
new file mode 100644
index 0000000..fd01aa0
--- /dev/null
+++ b/drivers/tty/serial/ts5500-auto485.c
@@ -0,0 +1,45 @@
+/*
+ * ts5500-auto485.c - support for the TS-5500 auto485 feature
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Arthur Gautier <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/ts5xxx-sbcinfo.h>
+
+#define TS5500_485_CONTROLER_ADDR 0x75 /* controler register address */
+#define TS5500_485_BITS 0xC0 /* RTS485 and Auto485 bits */
+
+static int __init serial8250_init_ts5500_485(void)
+{
+ struct ts5xxx_sbcinfo sbcinfo;
+ u8 reg;
+
+ /* Validate platform and auto485 feature */
+ ts5xxx_sbcinfo_set(&sbcinfo);
+
+ if (!sbcinfo.auto485) {
+ printk(KERN_ERR ": Auto485 not available on this platform\n");
+ return -ENODEV;
+ }
+
+ /* Enable RS-485 mode */
+ reg = inb(TS5500_485_CONTROLER_ADDR) | TS5500_485_BITS;
+ outb(reg, TS5500_485_CONTROLER_ADDR);
+
+ return 0;
+}
+module_init(serial8250_init_ts5500_485);
+
+static void __exit serial8250_ts5500_485(void)
+{
+ u8 reg = inb(TS5500_485_CONTROLER_ADDR) & ~TS5500_485_BITS;
+ outb(reg, TS5500_485_CONTROLER_ADDR);
+}
+module_exit(serial8250_ts5500_485);
+
+MODULE_DESCRIPTION("Technologic Systems TS-5500, auto485 driver");
+MODULE_AUTHOR("Arthur Gautier <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.7.1

2011-04-29 22:28:55

by Vivien Didelot

[permalink] [raw]
Subject: [RFC 2/5] gpio: add support for Technologic Systems TS-5500 GPIOs

From: Jerome Oufella <[email protected]>


Signed-off-by: Vivien Didelot <[email protected]>
---
MAINTAINERS | 2 +
drivers/gpio/Kconfig | 10 +
drivers/gpio/Makefile | 1 +
drivers/gpio/ts5500-gpio.c | 528 +++++++++++++++++++++++++++++++++++++++++++
include/linux/ts5500-gpio.h | 68 ++++++
5 files changed, 609 insertions(+), 0 deletions(-)
create mode 100644 drivers/gpio/ts5500-gpio.c
create mode 100644 include/linux/ts5500-gpio.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b077e6d..32c2c57 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6058,6 +6058,8 @@ M: Savoir-faire Linux Inc. <[email protected]>
S: Maintained
F: drivers/platform/x86/ts5xxx-sbcinfo.c
F: include/linux/ts5xxx-sbcinfo.h
+F: drivers/gpio/ts5500-gpio.c
+F: include/linux/ts5500-gpio.h

TEGRA SUPPORT
M: Colin Cross <[email protected]>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d3b2953..d707e42 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -322,6 +322,16 @@ config GPIO_BT8XX

If unsure, say N.

+config GPIO_TS5500
+ tristate "TS-5500 GPIO support"
+ depends on TS5500_SBC
+ help
+ This enables support for the Technologic Systems TS-5500 DIO
+ headers for GPIO usage.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ts5500-gpio.
+
config GPIO_LANGWELL
bool "Intel Langwell/Penwell GPIO support"
depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index becef59..f5bd65f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -43,3 +43,4 @@ obj-$(CONFIG_GPIO_SX150X) += sx150x.o
obj-$(CONFIG_GPIO_VX855) += vx855_gpio.o
obj-$(CONFIG_GPIO_ML_IOH) += ml_ioh_gpio.o
obj-$(CONFIG_AB8500_GPIO) += ab8500-gpio.o
+obj-$(CONFIG_GPIO_TS5500) += ts5500-gpio.o
diff --git a/drivers/gpio/ts5500-gpio.c b/drivers/gpio/ts5500-gpio.c
new file mode 100644
index 0000000..9d75d1c
--- /dev/null
+++ b/drivers/gpio/ts5500-gpio.c
@@ -0,0 +1,528 @@
+/*
+ * GPIO (DIO) driver for Technologic Systems TS-5500
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Jerome Oufella <[email protected]>
+ *
+ * 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.
+ *
+ * The TS-5500 board has 38 GPIOs referred to as DIOs in the product's
+ * litterature.
+ *
+ */
+
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/ts5xxx-sbcinfo.h>
+#include <linux/ts5500-gpio.h>
+#include <linux/io.h>
+
+#define MODULE_NAME "ts5500-gpio"
+
+#define PORT_BIT_SET(addr, bit) \
+ do { \
+ u8 var; \
+ var = inb(addr); \
+ var |= (1 << bit); \
+ outb(var, addr); \
+ } while (0)
+
+#define PORT_BIT_CLEAR(addr, bit) \
+ do { \
+ u8 var; \
+ var = inb(addr); \
+ var &= ~(1 << bit); \
+ outb(var, addr); \
+ } while (0)
+
+/* "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[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.
+ * @pdata: GPIO platform data.
+ * @gpio_lock: Read/Write Mutex.
+ */
+struct ts5500_drvdata {
+ struct device *master;
+ struct gpio_chip gpio_chip;
+ struct ts5500_gpio_platform_data *pdata;
+ struct mutex gpio_lock;
+};
+
+static struct ts5500_gpio_platform_data gpio_pdata = {
+ .name = MODULE_NAME
+};
+
+static void ts5500_gpio_device_release(struct device *dev);
+static int __devinit ts5500_gpio_probe(struct platform_device *pdev);
+static int __devexit ts5500_gpio_remove(struct platform_device *pdev);
+
+static struct platform_device gpio_device = {
+ .name = MODULE_NAME,
+ .id = -1,
+ .dev = {
+ .platform_data = &gpio_pdata,
+ .release = ts5500_gpio_device_release,
+ }
+};
+
+static struct platform_driver ts5500_gpio_driver = {
+ .driver = {
+ .name = MODULE_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = ts5500_gpio_probe,
+ .remove = __devexit_p(ts5500_gpio_remove)
+};
+
+/**
+ * ts5500_gpio_device_release() - callback for releasing resources
+ * @dev: Device.
+ */
+static void ts5500_gpio_device_release(struct device *dev)
+{
+ /* noop */
+}
+
+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 */
+ switch (offset) {
+ case LCD_EN:
+ return -ENXIO;
+ default:
+ if (offset > chip->ngpio)
+ return -ENXIO;
+ }
+
+ ioaddr = line_to_port_map[offset];
+ bitno = line_to_bit_map[offset];
+
+ mutex_lock(&drvdata->gpio_lock);
+ byte = inb(ioaddr);
+ mutex_unlock(&drvdata->gpio_lock);
+
+ 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 DIO1_12:
+ case DIO1_13:
+ case DIO2_13:
+ case LCD_RS:
+ case 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)
+{
+ struct ts5500_drvdata *drvdata;
+
+ drvdata = container_of(chip, struct ts5500_drvdata, gpio_chip);
+
+ /* Only a few lines are IRQ-Capable */
+ switch (offset) {
+ case DIO1_13:
+ return DIO1_13_IRQ;
+ case DIO2_13:
+ return DIO2_13_IRQ;
+ case LCD_RS:
+ return 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 >= DIO1_0 && offset < DIO1_13)
+ return DIO1_13_IRQ;
+
+ if (dio2_irq && offset >= DIO2_0 && offset < DIO2_13)
+ return DIO2_13_IRQ;
+
+ if (lcd_irq && offset >= LCD_0 && offset <= LCD_WR)
+ return 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 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 DIO1_12:
+ case DIO1_13:
+ case DIO2_13:
+ case LCD_RS:
+ case 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 ts5500_gpio_platform_data *pdata;
+ struct ts5xxx_sbcinfo sbcinfo;
+ struct gpio_chip *chip;
+ int ret;
+
+ if (pdev == NULL) {
+ printk(MODULE_NAME ": Platform device not available!\n");
+ return -ENODEV;
+ }
+
+ ts5xxx_sbcinfo_set(&sbcinfo);
+ if (5500 != sbcinfo.board_id) {
+ printk(MODULE_NAME ": Incompatible TS Board.\n");
+ return -ENODEV;
+ }
+
+ pdata = (struct ts5500_gpio_platform_data *) pdev->dev.platform_data;
+ if (pdata == NULL) {
+ printk(MODULE_NAME ": Platform data 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;
+ drvdata->pdata = pdata;
+ 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 = DIO1_0;
+ chip->label = pdata->name;
+ chip->ngpio = (use_lcdio ? LCD_WR + 1 : 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:
+ printk(KERN_ERR "gpio: Failed registering 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;
+}
+
+static int __devexit ts5500_gpio_remove(struct platform_device *pdev)
+{
+ struct ts5500_gpio_platform_data *pdata;
+ struct ts5500_drvdata *drvdata;
+ int ret, i;
+
+ pdata = (struct ts5500_gpio_platform_data *) pdev->dev.platform_data;
+ 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, "gpiochip_remove() failed, retcode %d\n",
+ ret);
+
+ kfree(drvdata);
+ return ret;
+}
+
+static int __init ts5500_gpio_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&ts5500_gpio_driver);
+ if (ret)
+ return ret;
+
+ ret = platform_device_register(&gpio_device);
+ if (ret) {
+ printk(MODULE_NAME ": Failed to register platform device\n");
+ platform_driver_unregister(&ts5500_gpio_driver);
+ return ret;
+ }
+
+ printk(MODULE_NAME ": GPIO/DIO driver loaded.\n");
+ return 0;
+}
+module_init(ts5500_gpio_init);
+
+static void __exit ts5500_gpio_exit(void)
+{
+ platform_device_unregister(&gpio_device);
+ platform_driver_unregister(&ts5500_gpio_driver);
+ printk(MODULE_NAME ": unloaded.\n");
+}
+module_exit(ts5500_gpio_exit);
+
+MODULE_AUTHOR("Jerome Oufella");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Technologic Systems TS5500, GPIO/DIO driver");
diff --git a/include/linux/ts5500-gpio.h b/include/linux/ts5500-gpio.h
new file mode 100644
index 0000000..387039b
--- /dev/null
+++ b/include/linux/ts5500-gpio.h
@@ -0,0 +1,68 @@
+/*
+ * GPIO (DIO) driver for Technologic Systems TS-5500
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Jerome Oufella <[email protected]>
+ *
+ * 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 DIO1_0 0
+#define DIO1_1 1
+#define DIO1_2 2
+#define DIO1_3 3
+#define DIO1_4 4
+#define DIO1_5 5
+#define DIO1_6 6
+#define DIO1_7 7
+#define DIO1_8 8
+#define DIO1_9 9
+#define DIO1_10 10
+#define DIO1_11 11
+#define DIO1_12 12
+#define DIO1_13 13
+#define DIO2_0 14
+#define DIO2_1 15
+#define DIO2_2 16
+#define DIO2_3 17
+#define DIO2_4 18
+#define DIO2_5 19
+#define DIO2_6 20
+#define DIO2_7 21
+#define DIO2_8 22
+#define DIO2_9 23
+#define DIO2_10 24
+#define DIO2_11 25
+/* #define DIO2_12 - Leave commented out as this line simply doesn't exist */
+#define DIO2_13 26
+#define LCD_0 27
+#define LCD_1 28
+#define LCD_2 29
+#define LCD_3 30
+#define LCD_4 31
+#define LCD_5 32
+#define LCD_6 33
+#define LCD_7 34
+#define LCD_EN 35
+#define LCD_RS 36
+#define LCD_WR 37
+
+/* Lines that can trigger IRQs */
+#define DIO1_13_IRQ 7
+#define DIO2_13_IRQ 6
+#define LCD_RS_IRQ 1
+
+/**
+ * struct ts5500_gpio_platform_data - platform data for ts-5500 gpio usage
+ * @name: Name of the resource in /sys/class/gpio/gpiochip%d/label
+ */
+struct ts5500_gpio_platform_data {
+ const char *name;
+};
+
+#endif /* TS5500_GPIO_H_ */
--
1.7.1

2011-04-29 22:33:17

by Vivien Didelot

[permalink] [raw]
Subject: [RFC 4/5] leds: add support for Technologic Systems TS-5500 leds

From: Jonas Fonseca <[email protected]>


Signed-off-by: Vivien Didelot <[email protected]>
---
MAINTAINERS | 1 +
drivers/leds/Kconfig | 12 +++
drivers/leds/Makefile | 1 +
drivers/leds/leds-ts5500.c | 211 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 225 insertions(+), 0 deletions(-)
create mode 100644 drivers/leds/leds-ts5500.c

diff --git a/MAINTAINERS b/MAINTAINERS
index da3b6d3..c0a646d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6061,6 +6061,7 @@ F: include/linux/ts5xxx-sbcinfo.h
F: drivers/gpio/ts5500-gpio.c
F: include/linux/ts5500-gpio.h
F: drivers/tty/serial/ts5500-auto485.c
+F: drivers/leds/leds-ts5500.c

TEGRA SUPPORT
M: Colin Cross <[email protected]>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 9bec869..14144df 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -379,6 +379,18 @@ config LEDS_NETXBIG
and 5Big Network v2 boards. The LEDs are wired to a CPLD and are
controlled through a GPIO extension bus.

+config LEDS_TS5500
+ tristate "LED Support for TS-5500 SBCs"
+ depends on LEDS_CLASS && TS5500_SBC
+ help
+ This option enables support for on-chip LED drivers found
+ on Technologic Systems TS-5500 SBCs.
+
+ To compile this driver as a module, choose M here: the module will
+ be called leds-ts5500.
+
+comment "LED Triggers"
+
config LEDS_TRIGGERS
bool "LED Trigger support"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 39c80fc..ce5a25f 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_LEDS_DELL_NETBOOKS) += dell-led.o
obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
+obj-$(CONFIG_LEDS_TS5500) += leds-ts5500.o

# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-ts5500.c b/drivers/leds/leds-ts5500.c
new file mode 100644
index 0000000..aec6d71
--- /dev/null
+++ b/drivers/leds/leds-ts5500.c
@@ -0,0 +1,211 @@
+/*
+ * Technologic Systems TS-5500 boards - LED driver
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Jonas Fonseca <[email protected]>
+ *
+ * Portions Copyright (c) 2008 Compulab, Ltd.
+ * Mike Rapoport <[email protected]>
+ *
+ * Portions Copyright (c) 2006-2008 Marvell International Ltd.
+ * Eric Miao <[email protected]>
+ *
+ * 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/leds.h>
+#include <linux/workqueue.h>
+#include <linux/ts5xxx-sbcinfo.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME "leds-ts5500"
+
+/**
+ * struct ts5500_led - LED structure
+ * @cdev: LED class device structure.
+ * @work: Work structure.
+ * @new_brightness: LED brightness.
+ * @ioaddr: LED I/O address.
+ */
+struct ts5500_led {
+ struct led_classdev cdev;
+ struct work_struct work;
+ enum led_brightness new_brightness;
+ int ioaddr;
+ int bit;
+};
+
+static void ts5500_led_work(struct work_struct *work)
+{
+ struct ts5500_led *led = container_of(work, struct ts5500_led, work);
+ u8 val = led->new_brightness ? led->bit : 0;
+
+ outb(val, led->ioaddr);
+}
+
+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);
+
+ led->new_brightness = value;
+ schedule_work(&led->work);
+}
+
+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;
+ led->new_brightness = LED_OFF;
+
+ INIT_WORK(&led->work, ts5500_led_work);
+
+ 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 = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = ts5500_led_probe,
+ .remove = __devexit_p(ts5500_led_remove),
+};
+
+
+static struct led_info ts5500_led_info = {
+ .name = DRIVER_NAME,
+ .default_trigger = DRIVER_NAME,
+ .flags = 0x01,
+};
+
+static struct led_platform_data ts5500_led_platform_data = {
+ .num_leds = 1,
+ .leds = &ts5500_led_info,
+};
+
+static struct resource ts5500_led_resources[] = {
+ {
+ .name = DRIVER_NAME,
+ .start = 0x77,
+ .end = 0x77,
+ .flags = IORESOURCE_IO,
+ }
+};
+
+static void ts5500_led_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct platform_device ts5500_led_device = {
+ .name = DRIVER_NAME,
+ .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 __init ts5500_led_init(void)
+{
+ struct ts5xxx_sbcinfo sbcinfo;
+ int ret;
+
+ ts5xxx_sbcinfo_set(&sbcinfo);
+
+ if (sbcinfo.board_id != 5500) {
+ printk(KERN_ERR DRIVER_NAME
+ ": Expected TS5500 but TS-SBC reported TS%d\n",
+ sbcinfo.board_id);
+ return -ENODEV;
+ }
+
+ 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 TS5500");
+MODULE_AUTHOR("Jonas Fonseca <[email protected]>");
--
1.7.1

2011-04-29 22:33:28

by Vivien Didelot

[permalink] [raw]
Subject: [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

From: Jonas Fonseca <[email protected]>


Signed-off-by: Vivien Didelot <[email protected]>
---
MAINTAINERS | 1 +
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/ts5500-adc.c | 481 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 493 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/ts5500-adc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c0a646d..aace74a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6062,6 +6062,7 @@ F: drivers/gpio/ts5500-gpio.c
F: include/linux/ts5500-gpio.h
F: drivers/tty/serial/ts5500-auto485.c
F: drivers/leds/leds-ts5500.c
+F: drivers/hwmon/ts5500-adc.c

TEGRA SUPPORT
M: Colin Cross <[email protected]>
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 060ef63..5070530 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1286,6 +1286,16 @@ config SENSORS_MC13783_ADC
help
Support for the A/D converter on MC13783 PMIC.

+config SENSORS_TS5500_ADC
+ tristate "Technologic Systems TS-5500 ADC"
+ depends on TS5500_SBC
+ help
+ Support for the A/D converter on Technologic Systems TS-5500
+ SBCs.
+
+ This driver can also be built as a module. If so, the module
+ will be called ts5500-adc.
+
if ACPI

comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 967d0ea..9c8563d 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -114,6 +114,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_TS5500_ADC)+= ts5500-adc.o

# PMBus drivers
obj-$(CONFIG_PMBUS) += pmbus_core.o
diff --git a/drivers/hwmon/ts5500-adc.c b/drivers/hwmon/ts5500-adc.c
new file mode 100644
index 0000000..6568e9e
--- /dev/null
+++ b/drivers/hwmon/ts5500-adc.c
@@ -0,0 +1,481 @@
+/*
+ * Technologic Systems TS-5500 boards - MAX197 ADC driver
+ *
+ * Copyright (c) 2010 Savoir-faire Linux Inc.
+ * Jonas Fonseca <[email protected]>
+ *
+ * Portions Copyright (C) 2008 Marc Pignat <[email protected]>
+ *
+ * The driver uses direct access for communication with the ADC.
+ * 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/ts5xxx-sbcinfo.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+
+#define MODULE_NAME "ts5500-max197"
+
+/*
+ * Control bits of A/D command
+ * bits 0-2: selected channel (0 - 7)
+ * bits 3: uni/bipolar (0 = unipolar (ie 0 to +5V))
+ * (1 = bipolar (ie -5 to +5V))
+ * bit 4: selected range (0 = 5v range, 1 = 10V range)
+ * bit 5-7: padded zero (unused)
+ */
+#define TS5500_MAX197_BIPOLAR 0x08
+#define TS5500_MAX197_UNIPOLAR 0x00
+#define TS5500_MAX197_RANGE_5V 0x00 /* 0 to 5V range */
+#define TS5500_MAX197_RANGE_10V 0x10 /* 0 to 10V range */
+
+#define TS5500_MAX197_READ_DELAY 15 /* usec */
+#define TS5500_MAX197_READ_BUSY_MASK 0x01
+#define TS5500_MAX197_NAME "MAX197 (8 channels)"
+
+#define MAX197_CHANNELS_MAX 8 /* 0 to 7 channels on device */
+
+#define max197_test_bit(bit, map) (test_bit(bit, map) != 0)
+
+/**
+ * struct max197_platform_data
+ * @name: Name of the device.
+ * @ioaddr: I/O address containing:
+ * .data: Data register for conversion reading.
+ * .ctrl: A/D Control Register (bit 0 = 0 when
+ * conversion completed).
+ * @read: Information about conversion reading, with:
+ * .delay: Delay before next conversion.
+ * .busy_mask: Control register bit 0 equals 1 means
+ * conversion is not completed yet.
+ * @ctrl: Data tables addressable by [polarity][range].
+ * @ranges: Ranges.
+ * .min: Min value.
+ * .max: Max value.
+ * @scale: Polarity/Range coefficients to scale raw conversion reading.
+ */
+struct max197_platform_data {
+ const char *name;
+ struct {
+ int data;
+ int ctrl;
+ } ioaddr;
+ struct {
+ u8 delay;
+ u8 busy_mask;
+ } read;
+ u8 ctrl[2][2];
+ struct {
+ int min[2][2];
+ int max[2][2];
+ } ranges;
+ int scale[2][2];
+};
+
+/**
+ * struct max197_chip
+ * @hwmon_dev: The hwmon device.
+ * @lock: Read/Write mutex.
+ * @spec: The MAX197 platform data.
+ * @polarity: bitmap for polarity.
+ * @range: bitmap for range.
+ */
+struct max197_chip {
+ struct device *hwmon_dev;
+ struct mutex lock;
+ struct max197_platform_data spec;
+ DECLARE_BITMAP(polarity, MAX197_CHANNELS_MAX);
+ DECLARE_BITMAP(range, MAX197_CHANNELS_MAX);
+};
+
+static inline s32 max197_scale(struct max197_chip *chip, s16 raw,
+ int polarity, int range)
+{
+ s32 scaled = raw;
+
+ scaled *= chip->spec.scale[polarity][range];
+ scaled /= 10000;
+
+ return scaled;
+}
+
+static inline int max197_range(struct max197_chip *chip, int is_min,
+ int polarity, int range)
+{
+ if (is_min)
+ return chip->spec.ranges.min[polarity][range];
+ return chip->spec.ranges.max[polarity][range];
+}
+
+static inline int max197_strtol(const char *buf, long *value, int range1,
+ int range2)
+{
+ if (strict_strtol(buf, 10, value))
+ return -EINVAL;
+
+ if (range1 < range2)
+ *value = SENSORS_LIMIT(*value, range1, range2);
+ else
+ *value = SENSORS_LIMIT(*value, range2, range1);
+
+ return 0;
+}
+
+static inline struct max197_chip *max197_get_drvdata(struct device *dev)
+{
+ return platform_get_drvdata(to_platform_device(dev));
+}
+
+/**
+ * max197_show_range() - Display range on user output
+ *
+ * Function called on read access on
+ * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
+ struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+ int is_min = attr->nr != 0;
+ int polarity, range;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ polarity = max197_test_bit(attr->index, chip->polarity);
+ range = max197_test_bit(attr->index, chip->range);
+
+ mutex_unlock(&chip->lock);
+
+ return sprintf(buf, "%d\n",
+ max197_range(chip, is_min, polarity, range));
+}
+
+/**
+ * max197_store_range() - Write range from user input
+ *
+ * Function called on write access on
+ * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
+ struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+ int is_min = attr->nr != 0;
+ int range1 = max197_range(chip, is_min, 0, 0);
+ int range2 = max197_range(chip, is_min, 1, 1);
+ long value;
+
+ if (max197_strtol(buf, &value, range1, range2))
+ return -EINVAL;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ if (abs(value) > 5000)
+ set_bit(attr->index, chip->range);
+ else
+ clear_bit(attr->index, chip->range);
+
+ if (is_min) {
+ if (value < 0)
+ set_bit(attr->index, chip->polarity);
+ else
+ clear_bit(attr->index, chip->polarity);
+ }
+
+ mutex_unlock(&chip->lock);
+
+ return count;
+}
+
+/**
+ * max197_show_input() - Show channel input
+ *
+ * Function called on read access on
+ * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+ int polarity, range;
+ int ret;
+ u8 command;
+
+ if (mutex_lock_interruptible(&chip->lock))
+ return -ERESTARTSYS;
+
+ polarity = max197_test_bit(attr->index, chip->polarity);
+ range = max197_test_bit(attr->index, chip->range);
+
+ command = attr->index | chip->spec.ctrl[polarity][range];
+
+ outb(command, chip->spec.ioaddr.data);
+
+ udelay(chip->spec.read.delay);
+ ret = inb(chip->spec.ioaddr.ctrl);
+
+ if (ret & chip->spec.read.busy_mask) {
+ dev_err(dev, "device not ready (ret=0x0%x, try=%d)\n", ret,
+ range);
+ ret = -EIO;
+ } else {
+ /* LSB of conversion is at 0x196 and MSB is at 0x197 */
+ u8 lsb = inb(chip->spec.ioaddr.data);
+ u8 msb = inb(chip->spec.ioaddr.data + 1);
+ s16 raw = (msb << 8) | lsb;
+ s32 scaled = max197_scale(chip, raw, polarity, range);
+
+ ret = sprintf(buf, "%d\n", scaled);
+ }
+
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
+static ssize_t max197_show_name(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ return sprintf(buf, "%s\n", max197_get_drvdata(dev)->spec.name);
+}
+
+#define MAX197_HWMON_CHANNEL(chan) \
+ SENSOR_DEVICE_ATTR(in##chan##_input, S_IRUGO, \
+ max197_show_input, NULL, chan); \
+ SENSOR_DEVICE_ATTR_2(in##chan##_max, S_IRUGO | S_IWUSR, \
+ max197_show_range, \
+ max197_store_range, 0, chan); \
+ SENSOR_DEVICE_ATTR_2(in##chan##_min, S_IRUGO | S_IWUSR, \
+ max197_show_range, \
+ max197_store_range, 1, chan) \
+
+#define MAX197_SYSFS_CHANNEL(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);
+
+static MAX197_HWMON_CHANNEL(0);
+static MAX197_HWMON_CHANNEL(1);
+static MAX197_HWMON_CHANNEL(2);
+static MAX197_HWMON_CHANNEL(3);
+static MAX197_HWMON_CHANNEL(4);
+static MAX197_HWMON_CHANNEL(5);
+static MAX197_HWMON_CHANNEL(6);
+static MAX197_HWMON_CHANNEL(7);
+
+static const struct attribute_group max197_sysfs_group = {
+ .attrs = (struct attribute *[]) {
+ &dev_attr_name.attr,
+ MAX197_SYSFS_CHANNEL(0),
+ MAX197_SYSFS_CHANNEL(1),
+ MAX197_SYSFS_CHANNEL(2),
+ MAX197_SYSFS_CHANNEL(3),
+ MAX197_SYSFS_CHANNEL(4),
+ MAX197_SYSFS_CHANNEL(5),
+ MAX197_SYSFS_CHANNEL(6),
+ MAX197_SYSFS_CHANNEL(7),
+ NULL
+ }
+};
+
+static int __devinit max197_probe(struct platform_device *pdev)
+{
+ struct max197_platform_data *pdata = pdev->dev.platform_data;
+ struct max197_chip *chip;
+ int ret;
+
+ if (pdata == NULL)
+ return -ENODEV;
+
+ chip = kzalloc(sizeof *chip, GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->spec = *pdata;
+
+ mutex_init(&chip->lock);
+ mutex_lock(&chip->lock);
+
+ ret = sysfs_create_group(&pdev->dev.kobj, &max197_sysfs_group);
+ if (ret) {
+ dev_err(&pdev->dev, "sysfs_create_group failed.\n");
+ goto error_unlock_and_free;
+ }
+
+ 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_unregister_device;
+ }
+
+ platform_set_drvdata(pdev, chip);
+ mutex_unlock(&chip->lock);
+ return 0;
+
+error_unregister_device:
+ sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
+
+error_unlock_and_free:
+ mutex_unlock(&chip->lock);
+ kfree(chip);
+ return ret;
+}
+
+static int __devexit max197_remove(struct platform_device *pdev)
+{
+ struct max197_chip *chip = platform_get_drvdata(pdev);
+
+ mutex_lock(&chip->lock);
+ hwmon_device_unregister(chip->hwmon_dev);
+ sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
+ platform_set_drvdata(pdev, NULL);
+ mutex_unlock(&chip->lock);
+ kfree(chip);
+
+ return 0;
+}
+
+static struct platform_driver max197_driver = {
+ .driver = {
+ .name = MODULE_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = max197_probe,
+ .remove = __devexit_p(max197_remove),
+};
+
+static void ts5500_max197_release(struct device *dev)
+{
+ /* noop */
+}
+
+static struct resource ts5500_max197_resources[] = {
+ {
+ .name = MODULE_NAME "-data",
+ .start = 0x196,
+ .end = 0x197,
+ .flags = IORESOURCE_IO,
+ },
+ {
+ .name = MODULE_NAME "-ctrl",
+ .start = 0x195,
+ .end = 0x195,
+ .flags = IORESOURCE_IO,
+ }
+};
+
+static struct max197_platform_data ts5500_max197_platform_data = {
+ .name = TS5500_MAX197_NAME,
+ .ioaddr = {
+ .data = 0x196,
+ .ctrl = 0x195,
+ },
+ .read = {
+ .delay = TS5500_MAX197_READ_DELAY,
+ .busy_mask = TS5500_MAX197_READ_BUSY_MASK,
+ },
+ .ctrl = {
+ { TS5500_MAX197_UNIPOLAR | TS5500_MAX197_RANGE_5V,
+ TS5500_MAX197_UNIPOLAR | TS5500_MAX197_RANGE_10V },
+ { TS5500_MAX197_BIPOLAR | TS5500_MAX197_RANGE_5V,
+ TS5500_MAX197_BIPOLAR | TS5500_MAX197_RANGE_10V },
+ },
+ .ranges = {
+ .min = {
+ { 0, 0 },
+ { -5000, -10000 },
+ },
+ .max = {
+ { 5000, 10000 },
+ { 5000, 10000 },
+ },
+ },
+ .scale = {
+ { 12207, 24414 },
+ { 24414, 48828 },
+ },
+};
+
+static struct platform_device ts5500_max197_device = {
+ .name = MODULE_NAME,
+ .id = -1,
+ .resource = ts5500_max197_resources,
+ .num_resources = ARRAY_SIZE(ts5500_max197_resources),
+ .dev = {
+ .platform_data = &ts5500_max197_platform_data,
+ .release = ts5500_max197_release,
+ },
+};
+
+static int __init ts5500_max197_init(void)
+{
+ struct ts5xxx_sbcinfo sbcinfo;
+ int ret;
+
+ ts5xxx_sbcinfo_set(&sbcinfo);
+ if (5500 != sbcinfo.board_id && !sbcinfo.adc) {
+ printk(MODULE_NAME ": Incompatible TS Board.\n");
+ return -ENODEV;
+ }
+
+ ret = platform_driver_register(&max197_driver);
+ if (ret)
+ goto error_out;
+
+ ret = platform_device_register(&ts5500_max197_device);
+ if (ret)
+ goto error_device_register;
+
+ return 0;
+
+error_device_register:
+ platform_driver_unregister(&max197_driver);
+error_out:
+ return ret;
+}
+module_init(ts5500_max197_init);
+
+static void __exit ts5500_max197_exit(void)
+{
+ platform_device_unregister(&ts5500_max197_device);
+ platform_driver_unregister(&max197_driver);
+}
+module_exit(ts5500_max197_exit);
+
+MODULE_DESCRIPTION("TS5500 MAX197 ADC device driver");
+MODULE_AUTHOR("Jonas Fonseca <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ts5500-max197");
--
1.7.1

2011-04-29 23:34:30

by Greg KH

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

On Fri, Apr 29, 2011 at 06:21:48PM -0400, Vivien Didelot wrote:
> +/proc filesystem
> +----------------
> +
> +Information about the TS board is available through the /proc/ts-sbcinfo.

Really? Why?

As you now have added a new kernel/user ABI, it must be documented in
Documentation/ABI/

Please include that in your next patch.

But first off, why a new proc file? What is it used for?

> +static int ts_sbcinfo_init_buffer(char *buf, struct ts5xxx_sbcinfo *sbcinfo)
> +{
> + char *pos = buf;
> +
> + pos += ts_addbuf(pos, "Board ID", "TS-%d", sbcinfo->board_id);
> + pos += ts_addbuf(pos, "RS485", "%s", sbcinfo->rs485 ? "yes" : "no");
> + pos += ts_addbuf(pos, "AnalogToDigital", "%s",
> + sbcinfo->adc ? "yes" : "no");
> + pos += ts_addbuf(pos, "Auto485", "%s", sbcinfo->auto485 ? "yes" : "no");
> + pos += ts_addbuf(pos, "SRAM", "%s", sbcinfo->sram ? "yes" : "no");
> + pos += ts_addbuf(pos, "External Reset", "%s",
> + sbcinfo->external_reset ? "yes" : "no");

Most of these look like they should be simple "one value per file" sysfs
files for your system. That would make things much easier on your
userspace tools to handle parsing them properly, right?

Why not do that instead?

thanks,

greg k-h

2011-04-30 03:40:30

by Guenter Roeck

[permalink] [raw]
Subject: Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

On Fri, Apr 29, 2011 at 06:21:52PM -0400, Vivien Didelot wrote:
> From: Jonas Fonseca <[email protected]>
>
>
> Signed-off-by: Vivien Didelot <[email protected]>

Hi Vivien,

The headline and file name are a bit misleading, since the driver is really
for MAX197 on a TS-5500 board.

You should split the driver into two parts, a generic driver
for the MAX197 and a platform driver (residing somewhere in arch/
or possibly drivers/platform/) to instantiate it.

There should be a platform data include file, probably in
include/linux/platform_data/.

.ioaddr in platform data should not be necessary. The driver's probe
function should get the values using platform_get_resource().

Having said that, from reading the code it looks like the chip is not really
used for hardware monitoring, but for generic ADC functionality. A quick look
into the TS-5500 user manual confirms this. So this should not be a hwmon
driver in the first place, but a generic ADC driver. Given the ADC conversion rate
of the MAX197, the hwmon ABI is not optimal anyway. You should move this driver
into the iio subsystem, probably to drivers/staging/iio/adc. Copying the iio
mailing list for input.

Thanks,
Guenter

> ---
> MAINTAINERS | 1 +
> drivers/hwmon/Kconfig | 10 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/ts5500-adc.c | 481 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 493 insertions(+), 0 deletions(-)
> create mode 100644 drivers/hwmon/ts5500-adc.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c0a646d..aace74a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6062,6 +6062,7 @@ F: drivers/gpio/ts5500-gpio.c
> F: include/linux/ts5500-gpio.h
> F: drivers/tty/serial/ts5500-auto485.c
> F: drivers/leds/leds-ts5500.c
> +F: drivers/hwmon/ts5500-adc.c
>
> TEGRA SUPPORT
> M: Colin Cross <[email protected]>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 060ef63..5070530 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1286,6 +1286,16 @@ config SENSORS_MC13783_ADC
> help
> Support for the A/D converter on MC13783 PMIC.
>
> +config SENSORS_TS5500_ADC
> + tristate "Technologic Systems TS-5500 ADC"
> + depends on TS5500_SBC
> + help
> + Support for the A/D converter on Technologic Systems TS-5500
> + SBCs.
> +
> + This driver can also be built as a module. If so, the module
> + will be called ts5500-adc.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 967d0ea..9c8563d 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -114,6 +114,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_TS5500_ADC)+= ts5500-adc.o
>
> # PMBus drivers
> obj-$(CONFIG_PMBUS) += pmbus_core.o
> diff --git a/drivers/hwmon/ts5500-adc.c b/drivers/hwmon/ts5500-adc.c
> new file mode 100644
> index 0000000..6568e9e
> --- /dev/null
> +++ b/drivers/hwmon/ts5500-adc.c
> @@ -0,0 +1,481 @@
> +/*
> + * Technologic Systems TS-5500 boards - MAX197 ADC driver
> + *
> + * Copyright (c) 2010 Savoir-faire Linux Inc.
> + * Jonas Fonseca <[email protected]>
> + *
> + * Portions Copyright (C) 2008 Marc Pignat <[email protected]>
> + *
> + * The driver uses direct access for communication with the ADC.
> + * 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/ts5xxx-sbcinfo.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +
> +#define MODULE_NAME "ts5500-max197"
> +
> +/*
> + * Control bits of A/D command
> + * bits 0-2: selected channel (0 - 7)
> + * bits 3: uni/bipolar (0 = unipolar (ie 0 to +5V))
> + * (1 = bipolar (ie -5 to +5V))
> + * bit 4: selected range (0 = 5v range, 1 = 10V range)
> + * bit 5-7: padded zero (unused)
> + */
> +#define TS5500_MAX197_BIPOLAR 0x08
> +#define TS5500_MAX197_UNIPOLAR 0x00
> +#define TS5500_MAX197_RANGE_5V 0x00 /* 0 to 5V range */
> +#define TS5500_MAX197_RANGE_10V 0x10 /* 0 to 10V range */
> +
> +#define TS5500_MAX197_READ_DELAY 15 /* usec */
> +#define TS5500_MAX197_READ_BUSY_MASK 0x01
> +#define TS5500_MAX197_NAME "MAX197 (8 channels)"
> +
> +#define MAX197_CHANNELS_MAX 8 /* 0 to 7 channels on device */
> +
> +#define max197_test_bit(bit, map) (test_bit(bit, map) != 0)
> +
> +/**
> + * struct max197_platform_data
> + * @name: Name of the device.
> + * @ioaddr: I/O address containing:
> + * .data: Data register for conversion reading.
> + * .ctrl: A/D Control Register (bit 0 = 0 when
> + * conversion completed).
> + * @read: Information about conversion reading, with:
> + * .delay: Delay before next conversion.
> + * .busy_mask: Control register bit 0 equals 1 means
> + * conversion is not completed yet.
> + * @ctrl: Data tables addressable by [polarity][range].
> + * @ranges: Ranges.
> + * .min: Min value.
> + * .max: Max value.
> + * @scale: Polarity/Range coefficients to scale raw conversion reading.
> + */
> +struct max197_platform_data {
> + const char *name;
> + struct {
> + int data;
> + int ctrl;
> + } ioaddr;
> + struct {
> + u8 delay;
> + u8 busy_mask;
> + } read;
> + u8 ctrl[2][2];
> + struct {
> + int min[2][2];
> + int max[2][2];
> + } ranges;
> + int scale[2][2];
> +};
> +
> +/**
> + * struct max197_chip
> + * @hwmon_dev: The hwmon device.
> + * @lock: Read/Write mutex.
> + * @spec: The MAX197 platform data.
> + * @polarity: bitmap for polarity.
> + * @range: bitmap for range.
> + */
> +struct max197_chip {
> + struct device *hwmon_dev;
> + struct mutex lock;
> + struct max197_platform_data spec;
> + DECLARE_BITMAP(polarity, MAX197_CHANNELS_MAX);
> + DECLARE_BITMAP(range, MAX197_CHANNELS_MAX);
> +};
> +
> +static inline s32 max197_scale(struct max197_chip *chip, s16 raw,
> + int polarity, int range)
> +{
> + s32 scaled = raw;
> +
> + scaled *= chip->spec.scale[polarity][range];
> + scaled /= 10000;
> +
> + return scaled;
> +}
> +
> +static inline int max197_range(struct max197_chip *chip, int is_min,
> + int polarity, int range)
> +{
> + if (is_min)
> + return chip->spec.ranges.min[polarity][range];
> + return chip->spec.ranges.max[polarity][range];
> +}
> +
> +static inline int max197_strtol(const char *buf, long *value, int range1,
> + int range2)
> +{
> + if (strict_strtol(buf, 10, value))
> + return -EINVAL;
> +
> + if (range1 < range2)
> + *value = SENSORS_LIMIT(*value, range1, range2);
> + else
> + *value = SENSORS_LIMIT(*value, range2, range1);
> +
> + return 0;
> +}
> +
> +static inline struct max197_chip *max197_get_drvdata(struct device *dev)
> +{
> + return platform_get_drvdata(to_platform_device(dev));
> +}
> +
> +/**
> + * max197_show_range() - Display range on user output
> + *
> + * Function called on read access on
> + * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
> + struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> + int is_min = attr->nr != 0;
> + int polarity, range;
> +
> + if (mutex_lock_interruptible(&chip->lock))
> + return -ERESTARTSYS;
> +
> + polarity = max197_test_bit(attr->index, chip->polarity);
> + range = max197_test_bit(attr->index, chip->range);
> +
> + mutex_unlock(&chip->lock);
> +
> + return sprintf(buf, "%d\n",
> + max197_range(chip, is_min, polarity, range));
> +}
> +
> +/**
> + * max197_store_range() - Write range from user input
> + *
> + * Function called on write access on
> + * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
> + struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> + int is_min = attr->nr != 0;
> + int range1 = max197_range(chip, is_min, 0, 0);
> + int range2 = max197_range(chip, is_min, 1, 1);
> + long value;
> +
> + if (max197_strtol(buf, &value, range1, range2))
> + return -EINVAL;
> +
> + if (mutex_lock_interruptible(&chip->lock))
> + return -ERESTARTSYS;
> +
> + if (abs(value) > 5000)
> + set_bit(attr->index, chip->range);
> + else
> + clear_bit(attr->index, chip->range);
> +
> + if (is_min) {
> + if (value < 0)
> + set_bit(attr->index, chip->polarity);
> + else
> + clear_bit(attr->index, chip->polarity);
> + }
> +
> + mutex_unlock(&chip->lock);
> +
> + return count;
> +}
> +
> +/**
> + * max197_show_input() - Show channel input
> + *
> + * Function called on read access on
> + * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> + int polarity, range;
> + int ret;
> + u8 command;
> +
> + if (mutex_lock_interruptible(&chip->lock))
> + return -ERESTARTSYS;
> +
> + polarity = max197_test_bit(attr->index, chip->polarity);
> + range = max197_test_bit(attr->index, chip->range);
> +
> + command = attr->index | chip->spec.ctrl[polarity][range];
> +
> + outb(command, chip->spec.ioaddr.data);
> +
> + udelay(chip->spec.read.delay);
> + ret = inb(chip->spec.ioaddr.ctrl);
> +
> + if (ret & chip->spec.read.busy_mask) {
> + dev_err(dev, "device not ready (ret=0x0%x, try=%d)\n", ret,
> + range);
> + ret = -EIO;
> + } else {
> + /* LSB of conversion is at 0x196 and MSB is at 0x197 */
> + u8 lsb = inb(chip->spec.ioaddr.data);
> + u8 msb = inb(chip->spec.ioaddr.data + 1);
> + s16 raw = (msb << 8) | lsb;
> + s32 scaled = max197_scale(chip, raw, polarity, range);
> +
> + ret = sprintf(buf, "%d\n", scaled);
> + }
> +
> + mutex_unlock(&chip->lock);
> + return ret;
> +}
> +
> +static ssize_t max197_show_name(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> +{
> + return sprintf(buf, "%s\n", max197_get_drvdata(dev)->spec.name);
> +}
> +
> +#define MAX197_HWMON_CHANNEL(chan) \
> + SENSOR_DEVICE_ATTR(in##chan##_input, S_IRUGO, \
> + max197_show_input, NULL, chan); \
> + SENSOR_DEVICE_ATTR_2(in##chan##_max, S_IRUGO | S_IWUSR, \
> + max197_show_range, \
> + max197_store_range, 0, chan); \
> + SENSOR_DEVICE_ATTR_2(in##chan##_min, S_IRUGO | S_IWUSR, \
> + max197_show_range, \
> + max197_store_range, 1, chan) \
> +
> +#define MAX197_SYSFS_CHANNEL(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);
> +
> +static MAX197_HWMON_CHANNEL(0);
> +static MAX197_HWMON_CHANNEL(1);
> +static MAX197_HWMON_CHANNEL(2);
> +static MAX197_HWMON_CHANNEL(3);
> +static MAX197_HWMON_CHANNEL(4);
> +static MAX197_HWMON_CHANNEL(5);
> +static MAX197_HWMON_CHANNEL(6);
> +static MAX197_HWMON_CHANNEL(7);
> +
> +static const struct attribute_group max197_sysfs_group = {
> + .attrs = (struct attribute *[]) {
> + &dev_attr_name.attr,
> + MAX197_SYSFS_CHANNEL(0),
> + MAX197_SYSFS_CHANNEL(1),
> + MAX197_SYSFS_CHANNEL(2),
> + MAX197_SYSFS_CHANNEL(3),
> + MAX197_SYSFS_CHANNEL(4),
> + MAX197_SYSFS_CHANNEL(5),
> + MAX197_SYSFS_CHANNEL(6),
> + MAX197_SYSFS_CHANNEL(7),
> + NULL
> + }
> +};
> +
> +static int __devinit max197_probe(struct platform_device *pdev)
> +{
> + struct max197_platform_data *pdata = pdev->dev.platform_data;
> + struct max197_chip *chip;
> + int ret;
> +
> + if (pdata == NULL)
> + return -ENODEV;
> +
> + chip = kzalloc(sizeof *chip, GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->spec = *pdata;
> +
> + mutex_init(&chip->lock);
> + mutex_lock(&chip->lock);
> +
> + ret = sysfs_create_group(&pdev->dev.kobj, &max197_sysfs_group);
> + if (ret) {
> + dev_err(&pdev->dev, "sysfs_create_group failed.\n");
> + goto error_unlock_and_free;
> + }
> +
> + 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_unregister_device;
> + }
> +
> + platform_set_drvdata(pdev, chip);
> + mutex_unlock(&chip->lock);
> + return 0;
> +
> +error_unregister_device:
> + sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
> +
> +error_unlock_and_free:
> + mutex_unlock(&chip->lock);
> + kfree(chip);
> + return ret;
> +}
> +
> +static int __devexit max197_remove(struct platform_device *pdev)
> +{
> + struct max197_chip *chip = platform_get_drvdata(pdev);
> +
> + mutex_lock(&chip->lock);
> + hwmon_device_unregister(chip->hwmon_dev);
> + sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
> + platform_set_drvdata(pdev, NULL);
> + mutex_unlock(&chip->lock);
> + kfree(chip);
> +
> + return 0;
> +}
> +
> +static struct platform_driver max197_driver = {
> + .driver = {
> + .name = MODULE_NAME,
> + .owner = THIS_MODULE,
> + },
> + .probe = max197_probe,
> + .remove = __devexit_p(max197_remove),
> +};
> +
> +static void ts5500_max197_release(struct device *dev)
> +{
> + /* noop */
> +}
> +
> +static struct resource ts5500_max197_resources[] = {
> + {
> + .name = MODULE_NAME "-data",
> + .start = 0x196,
> + .end = 0x197,
> + .flags = IORESOURCE_IO,
> + },
> + {
> + .name = MODULE_NAME "-ctrl",
> + .start = 0x195,
> + .end = 0x195,
> + .flags = IORESOURCE_IO,
> + }
> +};
> +
> +static struct max197_platform_data ts5500_max197_platform_data = {
> + .name = TS5500_MAX197_NAME,
> + .ioaddr = {
> + .data = 0x196,
> + .ctrl = 0x195,
> + },
> + .read = {
> + .delay = TS5500_MAX197_READ_DELAY,
> + .busy_mask = TS5500_MAX197_READ_BUSY_MASK,
> + },
> + .ctrl = {
> + { TS5500_MAX197_UNIPOLAR | TS5500_MAX197_RANGE_5V,
> + TS5500_MAX197_UNIPOLAR | TS5500_MAX197_RANGE_10V },
> + { TS5500_MAX197_BIPOLAR | TS5500_MAX197_RANGE_5V,
> + TS5500_MAX197_BIPOLAR | TS5500_MAX197_RANGE_10V },
> + },
> + .ranges = {
> + .min = {
> + { 0, 0 },
> + { -5000, -10000 },
> + },
> + .max = {
> + { 5000, 10000 },
> + { 5000, 10000 },
> + },
> + },
> + .scale = {
> + { 12207, 24414 },
> + { 24414, 48828 },
> + },
> +};
> +
> +static struct platform_device ts5500_max197_device = {
> + .name = MODULE_NAME,
> + .id = -1,
> + .resource = ts5500_max197_resources,
> + .num_resources = ARRAY_SIZE(ts5500_max197_resources),
> + .dev = {
> + .platform_data = &ts5500_max197_platform_data,
> + .release = ts5500_max197_release,
> + },
> +};
> +
> +static int __init ts5500_max197_init(void)
> +{
> + struct ts5xxx_sbcinfo sbcinfo;
> + int ret;
> +
> + ts5xxx_sbcinfo_set(&sbcinfo);
> + if (5500 != sbcinfo.board_id && !sbcinfo.adc) {
> + printk(MODULE_NAME ": Incompatible TS Board.\n");
> + return -ENODEV;
> + }
> +
> + ret = platform_driver_register(&max197_driver);
> + if (ret)
> + goto error_out;
> +
> + ret = platform_device_register(&ts5500_max197_device);
> + if (ret)
> + goto error_device_register;
> +
> + return 0;
> +
> +error_device_register:
> + platform_driver_unregister(&max197_driver);
> +error_out:
> + return ret;
> +}
> +module_init(ts5500_max197_init);
> +
> +static void __exit ts5500_max197_exit(void)
> +{
> + platform_device_unregister(&ts5500_max197_device);
> + platform_driver_unregister(&max197_driver);
> +}
> +module_exit(ts5500_max197_exit);
> +
> +MODULE_DESCRIPTION("TS5500 MAX197 ADC device driver");
> +MODULE_AUTHOR("Jonas Fonseca <[email protected]>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:ts5500-max197");
> --
> 1.7.1
>
>
> _______________________________________________
> lm-sensors mailing list
> [email protected]
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

2011-04-30 09:54:29

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

On 04/30/11 04:39, Guenter Roeck wrote:
> On Fri, Apr 29, 2011 at 06:21:52PM -0400, Vivien Didelot wrote:
>> From: Jonas Fonseca <[email protected]>
>>
>>
>> Signed-off-by: Vivien Didelot <[email protected]>
>
> Hi Vivien,
>
> The headline and file name are a bit misleading, since the driver is really
> for MAX197 on a TS-5500 board.
>
> You should split the driver into two parts, a generic driver
> for the MAX197 and a platform driver (residing somewhere in arch/
> or possibly drivers/platform/) to instantiate it.
The bus looks 'interesting'. Vivien, is this a common interface to
parts? If so, perhaps we want a small layer in between the raw reads
on the ts5500 and what hits the maxim part. That would allow us
to simply support other means of talking to this chip. If it's a very
much part specific interface, then we can leave generalizing until
someone actually needs it of course!
>
> There should be a platform data include file, probably in
> include/linux/platform_data/.
>
> .ioaddr in platform data should not be necessary. The driver's probe
> function should get the values using platform_get_resource().
>
> Having said that, from reading the code it looks like the chip is not really
> used for hardware monitoring, but for generic ADC functionality. A quick look
> into the TS-5500 user manual confirms this. So this should not be a hwmon
> driver in the first place, but a generic ADC driver. Given the ADC conversion rate
> of the MAX197, the hwmon ABI is not optimal anyway. You should move this driver
> into the iio subsystem, probably to drivers/staging/iio/adc. Copying the iio
> mailing list for input.
Yup, that does make sense here as far as I can tell. Definitely wants to be broken
up in the platform bit and max197 drivers though as Guenter has said.
>
> Thanks,
> Guenter
Thanks for cc'ing us...
>
>> ---
>> MAINTAINERS | 1 +
>> drivers/hwmon/Kconfig | 10 +
>> drivers/hwmon/Makefile | 1 +
>> drivers/hwmon/ts5500-adc.c | 481 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 493 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/hwmon/ts5500-adc.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c0a646d..aace74a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6062,6 +6062,7 @@ F: drivers/gpio/ts5500-gpio.c
>> F: include/linux/ts5500-gpio.h
>> F: drivers/tty/serial/ts5500-auto485.c
>> F: drivers/leds/leds-ts5500.c
>> +F: drivers/hwmon/ts5500-adc.c
>>
>> TEGRA SUPPORT
>> M: Colin Cross <[email protected]>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 060ef63..5070530 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1286,6 +1286,16 @@ config SENSORS_MC13783_ADC
>> help
>> Support for the A/D converter on MC13783 PMIC.
>>
>> +config SENSORS_TS5500_ADC
>> + tristate "Technologic Systems TS-5500 ADC"
>> + depends on TS5500_SBC
>> + help
>> + Support for the A/D converter on Technologic Systems TS-5500
>> + SBCs.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called ts5500-adc.
>> +
>> if ACPI
>>
>> comment "ACPI drivers"
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 967d0ea..9c8563d 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -114,6 +114,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_TS5500_ADC)+= ts5500-adc.o
>>
>> # PMBus drivers
>> obj-$(CONFIG_PMBUS) += pmbus_core.o
>> diff --git a/drivers/hwmon/ts5500-adc.c b/drivers/hwmon/ts5500-adc.c
>> new file mode 100644
>> index 0000000..6568e9e
>> --- /dev/null
>> +++ b/drivers/hwmon/ts5500-adc.c
>> @@ -0,0 +1,481 @@
>> +/*
>> + * Technologic Systems TS-5500 boards - MAX197 ADC driver
>> + *
>> + * Copyright (c) 2010 Savoir-faire Linux Inc.
>> + * Jonas Fonseca <[email protected]>
>> + *
>> + * Portions Copyright (C) 2008 Marc Pignat <[email protected]>
>> + *
>> + * The driver uses direct access for communication with the ADC.
>> + * 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/ts5xxx-sbcinfo.h>
>> +#include <linux/slab.h>
>> +#include <linux/io.h>
>> +
>> +#define MODULE_NAME "ts5500-max197"
>> +
>> +/*
>> + * Control bits of A/D command
>> + * bits 0-2: selected channel (0 - 7)
>> + * bits 3: uni/bipolar (0 = unipolar (ie 0 to +5V))
>> + * (1 = bipolar (ie -5 to +5V))
>> + * bit 4: selected range (0 = 5v range, 1 = 10V range)
>> + * bit 5-7: padded zero (unused)
>> + */
>> +#define TS5500_MAX197_BIPOLAR 0x08
>> +#define TS5500_MAX197_UNIPOLAR 0x00
>> +#define TS5500_MAX197_RANGE_5V 0x00 /* 0 to 5V range */
>> +#define TS5500_MAX197_RANGE_10V 0x10 /* 0 to 10V range */
>> +
>> +#define TS5500_MAX197_READ_DELAY 15 /* usec */
>> +#define TS5500_MAX197_READ_BUSY_MASK 0x01
>> +#define TS5500_MAX197_NAME "MAX197 (8 channels)"
>> +
>> +#define MAX197_CHANNELS_MAX 8 /* 0 to 7 channels on device */
>> +
>> +#define max197_test_bit(bit, map) (test_bit(bit, map) != 0)
>> +
>> +/**
>> + * struct max197_platform_data
>> + * @name: Name of the device.
>> + * @ioaddr: I/O address containing:
>> + * .data: Data register for conversion reading.
>> + * .ctrl: A/D Control Register (bit 0 = 0 when
>> + * conversion completed).
>> + * @read: Information about conversion reading, with:
>> + * .delay: Delay before next conversion.
>> + * .busy_mask: Control register bit 0 equals 1 means
>> + * conversion is not completed yet.
>> + * @ctrl: Data tables addressable by [polarity][range].
>> + * @ranges: Ranges.
>> + * .min: Min value.
>> + * .max: Max value.
>> + * @scale: Polarity/Range coefficients to scale raw conversion reading.
>> + */
>> +struct max197_platform_data {
>> + const char *name;
>> + struct {
>> + int data;
>> + int ctrl;
>> + } ioaddr;
>> + struct {
>> + u8 delay;
>> + u8 busy_mask;
>> + } read;
>> + u8 ctrl[2][2];
>> + struct {
>> + int min[2][2];
>> + int max[2][2];
>> + } ranges;
>> + int scale[2][2];
>> +};
>> +
>> +/**
>> + * struct max197_chip
>> + * @hwmon_dev: The hwmon device.
>> + * @lock: Read/Write mutex.
>> + * @spec: The MAX197 platform data.
>> + * @polarity: bitmap for polarity.
>> + * @range: bitmap for range.
>> + */
>> +struct max197_chip {
>> + struct device *hwmon_dev;
>> + struct mutex lock;
>> + struct max197_platform_data spec;
>> + DECLARE_BITMAP(polarity, MAX197_CHANNELS_MAX);
>> + DECLARE_BITMAP(range, MAX197_CHANNELS_MAX);
>> +};
>> +
>> +static inline s32 max197_scale(struct max197_chip *chip, s16 raw,
>> + int polarity, int range)
>> +{
>> + s32 scaled = raw;
>> +
>> + scaled *= chip->spec.scale[polarity][range];
>> + scaled /= 10000;
>> +
>> + return scaled;
>> +}
>> +
>> +static inline int max197_range(struct max197_chip *chip, int is_min,
>> + int polarity, int range)
>> +{
>> + if (is_min)
>> + return chip->spec.ranges.min[polarity][range];
>> + return chip->spec.ranges.max[polarity][range];
>> +}
>> +
>> +static inline int max197_strtol(const char *buf, long *value, int range1,
>> + int range2)
>> +{
>> + if (strict_strtol(buf, 10, value))
>> + return -EINVAL;
>> +
>> + if (range1 < range2)
>> + *value = SENSORS_LIMIT(*value, range1, range2);
>> + else
>> + *value = SENSORS_LIMIT(*value, range2, range1);
>> +
>> + return 0;
>> +}
>> +
>> +static inline struct max197_chip *max197_get_drvdata(struct device *dev)
>> +{
>> + return platform_get_drvdata(to_platform_device(dev));
>> +}
>> +
>> +/**
>> + * max197_show_range() - Display range on user output
>> + *
>> + * Function called on read access on
>> + * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
>> + struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
>> + int is_min = attr->nr != 0;
>> + int polarity, range;
>> +
>> + if (mutex_lock_interruptible(&chip->lock))
>> + return -ERESTARTSYS;
>> +
>> + polarity = max197_test_bit(attr->index, chip->polarity);
>> + range = max197_test_bit(attr->index, chip->range);
>> +
>> + mutex_unlock(&chip->lock);
>> +
>> + return sprintf(buf, "%d\n",
>> + max197_range(chip, is_min, polarity, range));
>> +}
>> +
>> +/**
>> + * max197_store_range() - Write range from user input
>> + *
>> + * Function called on write access on
>> + * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
>> + struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
>> + int is_min = attr->nr != 0;
>> + int range1 = max197_range(chip, is_min, 0, 0);
>> + int range2 = max197_range(chip, is_min, 1, 1);
>> + long value;
>> +
>> + if (max197_strtol(buf, &value, range1, range2))
>> + return -EINVAL;
>> +
>> + if (mutex_lock_interruptible(&chip->lock))
>> + return -ERESTARTSYS;
>> +
>> + if (abs(value) > 5000)
>> + set_bit(attr->index, chip->range);
>> + else
>> + clear_bit(attr->index, chip->range);
>> +
>> + if (is_min) {
>> + if (value < 0)
>> + set_bit(attr->index, chip->polarity);
>> + else
>> + clear_bit(attr->index, chip->polarity);
>> + }
>> +
>> + mutex_unlock(&chip->lock);
>> +
>> + return count;
>> +}
>> +
>> +/**
>> + * max197_show_input() - Show channel input
>> + *
>> + * Function called on read access on
>> + * /sys/devices/platform/ts5500-max197/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 = max197_get_drvdata(dev);
>> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>> + int polarity, range;
>> + int ret;
>> + u8 command;
>> +
>> + if (mutex_lock_interruptible(&chip->lock))
>> + return -ERESTARTSYS;
>> +
>> + polarity = max197_test_bit(attr->index, chip->polarity);
>> + range = max197_test_bit(attr->index, chip->range);
>> +
>> + command = attr->index | chip->spec.ctrl[polarity][range];
>> +
>> + outb(command, chip->spec.ioaddr.data);
>> +
>> + udelay(chip->spec.read.delay);
>> + ret = inb(chip->spec.ioaddr.ctrl);
>> +
>> + if (ret & chip->spec.read.busy_mask) {
>> + dev_err(dev, "device not ready (ret=0x0%x, try=%d)\n", ret,
>> + range);
>> + ret = -EIO;
>> + } else {
>> + /* LSB of conversion is at 0x196 and MSB is at 0x197 */
>> + u8 lsb = inb(chip->spec.ioaddr.data);
>> + u8 msb = inb(chip->spec.ioaddr.data + 1);
>> + s16 raw = (msb << 8) | lsb;
>> + s32 scaled = max197_scale(chip, raw, polarity, range);
>> +
>> + ret = sprintf(buf, "%d\n", scaled);
>> + }
>> +
>> + mutex_unlock(&chip->lock);
>> + return ret;
>> +}
>> +
>> +static ssize_t max197_show_name(struct device *dev,
>> + struct device_attribute *devattr, char *buf)
>> +{
>> + return sprintf(buf, "%s\n", max197_get_drvdata(dev)->spec.name);
>> +}
>> +
>> +#define MAX197_HWMON_CHANNEL(chan) \
>> + SENSOR_DEVICE_ATTR(in##chan##_input, S_IRUGO, \
>> + max197_show_input, NULL, chan); \
>> + SENSOR_DEVICE_ATTR_2(in##chan##_max, S_IRUGO | S_IWUSR, \
>> + max197_show_range, \
>> + max197_store_range, 0, chan); \
>> + SENSOR_DEVICE_ATTR_2(in##chan##_min, S_IRUGO | S_IWUSR, \
>> + max197_show_range, \
>> + max197_store_range, 1, chan) \
>> +
>> +#define MAX197_SYSFS_CHANNEL(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);
>> +
>> +static MAX197_HWMON_CHANNEL(0);
>> +static MAX197_HWMON_CHANNEL(1);
>> +static MAX197_HWMON_CHANNEL(2);
>> +static MAX197_HWMON_CHANNEL(3);
>> +static MAX197_HWMON_CHANNEL(4);
>> +static MAX197_HWMON_CHANNEL(5);
>> +static MAX197_HWMON_CHANNEL(6);
>> +static MAX197_HWMON_CHANNEL(7);
>> +
>> +static const struct attribute_group max197_sysfs_group = {
>> + .attrs = (struct attribute *[]) {
>> + &dev_attr_name.attr,
>> + MAX197_SYSFS_CHANNEL(0),
>> + MAX197_SYSFS_CHANNEL(1),
>> + MAX197_SYSFS_CHANNEL(2),
>> + MAX197_SYSFS_CHANNEL(3),
>> + MAX197_SYSFS_CHANNEL(4),
>> + MAX197_SYSFS_CHANNEL(5),
>> + MAX197_SYSFS_CHANNEL(6),
>> + MAX197_SYSFS_CHANNEL(7),
>> + NULL
>> + }
>> +};
>> +
>> +static int __devinit max197_probe(struct platform_device *pdev)
>> +{
>> + struct max197_platform_data *pdata = pdev->dev.platform_data;
>> + struct max197_chip *chip;
>> + int ret;
>> +
>> + if (pdata == NULL)
>> + return -ENODEV;
>> +
>> + chip = kzalloc(sizeof *chip, GFP_KERNEL);
>> + if (!chip)
>> + return -ENOMEM;
>> +
>> + chip->spec = *pdata;
>> +
>> + mutex_init(&chip->lock);
>> + mutex_lock(&chip->lock);
>> +
>> + ret = sysfs_create_group(&pdev->dev.kobj, &max197_sysfs_group);
>> + if (ret) {
>> + dev_err(&pdev->dev, "sysfs_create_group failed.\n");
>> + goto error_unlock_and_free;
>> + }
>> +
>> + 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_unregister_device;
>> + }
>> +
>> + platform_set_drvdata(pdev, chip);
>> + mutex_unlock(&chip->lock);
>> + return 0;
>> +
>> +error_unregister_device:
>> + sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
>> +
>> +error_unlock_and_free:
>> + mutex_unlock(&chip->lock);
>> + kfree(chip);
>> + return ret;
>> +}
>> +
>> +static int __devexit max197_remove(struct platform_device *pdev)
>> +{
>> + struct max197_chip *chip = platform_get_drvdata(pdev);
>> +
>> + mutex_lock(&chip->lock);
>> + hwmon_device_unregister(chip->hwmon_dev);
>> + sysfs_remove_group(&pdev->dev.kobj, &max197_sysfs_group);
>> + platform_set_drvdata(pdev, NULL);
>> + mutex_unlock(&chip->lock);
>> + kfree(chip);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver max197_driver = {
>> + .driver = {
>> + .name = MODULE_NAME,
>> + .owner = THIS_MODULE,
>> + },
>> + .probe = max197_probe,
>> + .remove = __devexit_p(max197_remove),
>> +};
>> +
>> +static void ts5500_max197_release(struct device *dev)
>> +{
>> + /* noop */
>> +}
>> +
>> +static struct resource ts5500_max197_resources[] = {
>> + {
>> + .name = MODULE_NAME "-data",
>> + .start = 0x196,
>> + .end = 0x197,
>> + .flags = IORESOURCE_IO,
>> + },
>> + {
>> + .name = MODULE_NAME "-ctrl",
>> + .start = 0x195,
>> + .end = 0x195,
>> + .flags = IORESOURCE_IO,
>> + }
>> +};
>> +
>> +static struct max197_platform_data ts5500_max197_platform_data = {
>> + .name = TS5500_MAX197_NAME,
>> + .ioaddr = {
>> + .data = 0x196,
>> + .ctrl = 0x195,
>> + },
>> + .read = {
>> + .delay = TS5500_MAX197_READ_DELAY,
>> + .busy_mask = TS5500_MAX197_READ_BUSY_MASK,
>> + },
>> + .ctrl = {
>> + { TS5500_MAX197_UNIPOLAR | TS5500_MAX197_RANGE_5V,
>> + TS5500_MAX197_UNIPOLAR | TS5500_MAX197_RANGE_10V },
>> + { TS5500_MAX197_BIPOLAR | TS5500_MAX197_RANGE_5V,
>> + TS5500_MAX197_BIPOLAR | TS5500_MAX197_RANGE_10V },
>> + },
>> + .ranges = {
>> + .min = {
>> + { 0, 0 },
>> + { -5000, -10000 },
>> + },
>> + .max = {
>> + { 5000, 10000 },
>> + { 5000, 10000 },
>> + },
>> + },
>> + .scale = {
>> + { 12207, 24414 },
>> + { 24414, 48828 },
>> + },
>> +};
>> +
>> +static struct platform_device ts5500_max197_device = {
>> + .name = MODULE_NAME,
>> + .id = -1,
>> + .resource = ts5500_max197_resources,
>> + .num_resources = ARRAY_SIZE(ts5500_max197_resources),
>> + .dev = {
>> + .platform_data = &ts5500_max197_platform_data,
>> + .release = ts5500_max197_release,
>> + },
>> +};
>> +
>> +static int __init ts5500_max197_init(void)
>> +{
>> + struct ts5xxx_sbcinfo sbcinfo;
>> + int ret;
>> +
>> + ts5xxx_sbcinfo_set(&sbcinfo);
>> + if (5500 != sbcinfo.board_id && !sbcinfo.adc) {
>> + printk(MODULE_NAME ": Incompatible TS Board.\n");
>> + return -ENODEV;
>> + }
>> +
>> + ret = platform_driver_register(&max197_driver);
>> + if (ret)
>> + goto error_out;
>> +
>> + ret = platform_device_register(&ts5500_max197_device);
>> + if (ret)
>> + goto error_device_register;
>> +
>> + return 0;
>> +
>> +error_device_register:
>> + platform_driver_unregister(&max197_driver);
>> +error_out:
>> + return ret;
>> +}
>> +module_init(ts5500_max197_init);
>> +
>> +static void __exit ts5500_max197_exit(void)
>> +{
>> + platform_device_unregister(&ts5500_max197_device);
>> + platform_driver_unregister(&max197_driver);
>> +}
>> +module_exit(ts5500_max197_exit);
>> +
>> +MODULE_DESCRIPTION("TS5500 MAX197 ADC device driver");
>> +MODULE_AUTHOR("Jonas Fonseca <[email protected]>");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:ts5500-max197");
>> --
>> 1.7.1
>>
>>
>> _______________________________________________
>> lm-sensors mailing list
>> [email protected]
>> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2011-04-30 10:06:47

by Alan

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

> + * ts5xxx_sbcinfo_set() - set the SBC info structure with the current SBC's info
> + * @sbcinfo: structure containing SBC info to set.
> + */
> +void ts5xxx_sbcinfo_set(struct ts5xxx_sbcinfo *sbcinfo)
> +{
> + memcpy(sbcinfo, &ts_sbcinfo, sizeof(*sbcinfo));
> +}
> +EXPORT_SYMBOL(ts5xxx_sbcinfo_set);

You export this but it's not clear who for or why - and there is also no
locking, so it seems to be an internal interface ?


> +/**
> + * ts_find_sbc_config() - find a SBC configuration from an id
> + * @id: ID of the board to find.
> + */
> +static inline struct ts_sbc_config *ts_find_sbc_config(u8 id)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(ts_sbcs_configs); i++)
> + if (id == ts_sbcs_configs[i].id)
> + return &ts_sbcs_configs[i];
> +
> + return NULL;
> +}

gcc will figure out inlines for you in static stuff and normally very well

> +
> +/**
> + * ts_sbcinfo_detect() - detect the TS board
> + * @sbcinfo: structure where to store the detected board's info.
> + */
> +static int ts_sbcinfo_detect(struct ts5xxx_sbcinfo *sbcinfo)
> +{
> + u8 temp;
> + struct ts_sbc_config *sbc;
> + int ret = 0;
> +
> + memset(sbcinfo, 0, sizeof(*sbcinfo));
> +
> + if (!request_region(IOADDR_SBCID, 4, "TS-SBC"))
> + return -EBUSY;
> +
> + temp = inb(IOADDR_SBCID);
> + /* If it is a 3x00 SBC only match against the first 3 bits */
> + if (temp & 0x07)
> + temp &= 0x07;

So if this is compiled into a kernel we blindly inb this address and some
platform just crashed on boot. Is this board like other embedded ones in
that there is some safe way to check if its such a board first ?

> +
> + sbc = ts_find_sbc_config(temp);
> + if (!sbc) {
> + ret = -ENODEV;

Assuming you've indentified the board properly this case might be worth a
pr_err giving the fact it seemed to be a TS-5xxx, the config number and
that it is unknown - that will help anyone with future boards realise
their config isn't supported.


> +
> +/**
> + * ts_sbcinfo_proc_read() - function called when a read access is done on
> + * /proc/ts-sbcinfo
> + */
> +static int ts_sbcinfo_proc_read(char *page, char **start, off_t off,
> + int count, int *eof, void *data)
> +{
> + int to_copy = (procfs_buffer_size <= count) ?
> + procfs_buffer_size - off : count;

> +
> + if (off + to_copy >= procfs_buffer_size) {
> + to_copy = procfs_buffer_size - off;
> + *eof = 1;
> + }

Suppose off is 0x7FFFFFFF and count = 100

to_copy = 100
off + to_copy >= procfs_buffer_size [off_t may be 32bit]

100 + 0x7FFFFFFF (wraps) < buffer size


> +static int __init ts5xxx_sbcinfo_init(void)
> +{
> + int err;
> +
> + err = ts_sbcinfo_detect(&ts_sbcinfo);
> + if (err < 0) {
> + printk(KERN_ERR KBUILD_MODNAME
> + ": Failed to get SBC information\n");
> + return err;
> + }

Well that will depend why and probably the caller can give the best
information including silence if the board is just not a ts-5xxx

> + proc_entry = create_proc_read_entry(PROCFS_NAME, S_IRUGO, NULL,
> + ts_sbcinfo_proc_read, 0);
> + if (proc_entry == NULL) {
> + printk(KERN_ERR KBUILD_MODNAME
> + ": Failed to create proc entry\n");

pr_err

> + return -ENOMEM;
> + }
> +
> + procfs_buffer_size = ts_sbcinfo_init_buffer(procfs_buffer, &ts_sbcinfo);
> + printk(KBUILD_MODNAME ": TS SBC's info driver loaded.\n");

Printk not needed

Alan

2011-04-30 10:15:22

by Alan

[permalink] [raw]
Subject: Re: [RFC 2/5] gpio: add support for Technologic Systems TS-5500 GPIOs

> + * The TS-5500 board has 38 GPIOs referred to as DIOs in the product's
> + * litterature.

literature

> +#define MODULE_NAME "ts5500-gpio"

see dev_dbg etc and the fmt support in them - they will do all this for
you nicely and the various printks can become dev_* which will tidy it up
and give more info

> +
> +#define PORT_BIT_SET(addr, bit) \
> + do { \
> + u8 var; \
> + var = inb(addr); \
> + var |= (1 << bit); \
> + outb(var, addr); \
> + } while (0)
> +
> +#define PORT_BIT_CLEAR(addr, bit) \
> + do { \
> + u8 var; \
> + var = inb(addr); \
> + var &= ~(1 << bit); \
> + outb(var, addr); \
> + } while (0)

These shouldn't be magic macros especially if creating variables and give
the multiple references to addr will break on things like ++ - use a
function so it is typechecked and variables behave as espected.


> + /* 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;
> + }

I'd have expected these to be resources of the platform device but I
guess that if they are entirely fixed it doesn't really matter much.

> + /* 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 */
> + }

What happens if an IRQ occurs at this point, you have no handler for it ?

> +err_gpiochip_add:
> + printk(KERN_ERR "gpio: Failed registering gpio chip.\n");
> + kfree(drvdata);

pr_err/dev_err etc - in general


> +
> +static int __init ts5500_gpio_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&ts5500_gpio_driver);
> + if (ret)
> + return ret;
> +
> + ret = platform_device_register(&gpio_device);
> + if (ret) {
> + printk(MODULE_NAME ": Failed to register platform device\n");
> + platform_driver_unregister(&ts5500_gpio_driver);
> + return ret;
> + }
> +
> + printk(MODULE_NAME ": GPIO/DIO driver loaded.\n");

prinkt isn't needed

Also you've done no checks to see if it's being loaded on the right
board. I'd actually have expected a single chunk of code in
arch/x86/platform/ts5500 or similar to do the board check and create the
platform devices, and this code to just register them.

2011-04-30 10:17:26

by Alan

[permalink] [raw]
Subject: Re: [RFC 3/5] serial: add support for Technologic Systems TS-5500 RS-485 serial port

> --- a/drivers/tty/serial/8250.c
> +++ b/drivers/tty/serial/8250.c
> @@ -109,6 +109,13 @@ static unsigned int skip_txen_test; /* force skip of txen test at init time */
> #define CONFIG_HUB6 1
>
> #include <asm/serial.h>
> +
> +/* TS-5500 related stuff */
> +#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
> +#define TS5500_TIMER2_SPEED_ADDR 0x42
> +#define TS5500_485_SERIAL_PORT 0x02
> +#endif
> +
> /*
> * SERIAL_PORT_DFNS tells us about built-in ports that have no
> * standard enumeration mechanism. Platforms that can find all
> @@ -437,6 +444,25 @@ static void au_serial_out(struct uart_port *p, int offset, int value)
> __raw_writel(value, p->membase + offset);
> }
>
> +#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
> +void serial8250_ts5500_set_termios(struct uart_port *port,
> + struct ktermios *new,
> + struct ktermios *old)
> +{
> + u16 speed;
> +
> + if (new->c_ospeed >= 9600 && port->line == TS5500_485_SERIAL_PORT) {
> + speed = ((115200 * 2) / new->c_ospeed);
> +
> + /* This should be written by low byte followed by high byte */
> + spin_lock_irq(&port->lock);
> + outb((speed & 0x0F), TS5500_TIMER2_SPEED_ADDR);
> + outb((speed >> 8), TS5500_TIMER2_SPEED_ADDR);
> + spin_unlock_irq(&port->lock);
> + }
> +}
> +#endif

Please don't put board specific magic in this file, we are desperately
trying to get rid of it.

> +
> static unsigned int tsi_serial_in(struct uart_port *p, int offset)
> {
> unsigned int tmp;
> @@ -2464,6 +2490,10 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
> /* Don't rewrite B0 */
> if (tty_termios_baud_rate(termios))
> tty_termios_encode_baud_rate(termios, baud, baud);
> +
> +#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
> + serial8250_ts5500_set_termios(port, termios, old);
> +#endif
> }
> EXPORT_SYMBOL(serial8250_do_set_termios);

Provide your own set_termios method and then call into this one (which is
why it is exported)


> diff --git a/drivers/tty/serial/ts5500-auto485.c b/drivers/tty/serial/ts5500-auto485.c
> new file mode 100644
> index 0000000..fd01aa0
> --- /dev/null
> +++ b/drivers/tty/serial/ts5500-auto485.c
> @@ -0,0 +1,45 @@
> +/*
> + * ts5500-auto485.c - support for the TS-5500 auto485 feature

This needs documentation and description. Also we have runtime 485
switching ioctls so any code should really be using those and extending
them as needed.

2011-04-30 10:20:08

by Alan

[permalink] [raw]
Subject: Re: [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

> + ts5xxx_sbcinfo_set(&sbcinfo);
> + if (5500 != sbcinfo.board_id && !sbcinfo.adc) {
> + printk(MODULE_NAME ": Incompatible TS Board.\n");
> + return -ENODEV

This is a symptom of creating the devices in the wrong place. If you put
all the ident/creation of boards in one place with the device creation
the drivers will cleanup and these ugly export things will go away

2011-05-02 21:07:34

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

Hi,

Thanks you all for your comments, I'll consider them carefully.

Excerpts from Greg KH's message of 2011-04-29 19:32:30 -0400:
> On Fri, Apr 29, 2011 at 06:21:48PM -0400, Vivien Didelot wrote:
> > +/proc filesystem
> > +----------------
> > +
> > +Information about the TS board is available through the /proc/ts-sbcinfo.
>
> Really? Why?
>
> As you now have added a new kernel/user ABI, it must be documented in
> Documentation/ABI/
>
> Please include that in your next patch.
>
> But first off, why a new proc file? What is it used for?

I think it could be useful for the user to know what option is available
or not on the board.

>
> > +static int ts_sbcinfo_init_buffer(char *buf, struct ts5xxx_sbcinfo *sbcinfo)
> > +{
> > + char *pos = buf;
> > +
> > + pos += ts_addbuf(pos, "Board ID", "TS-%d", sbcinfo->board_id);
> > + pos += ts_addbuf(pos, "RS485", "%s", sbcinfo->rs485 ? "yes" : "no");
> > + pos += ts_addbuf(pos, "AnalogToDigital", "%s",
> > + sbcinfo->adc ? "yes" : "no");
> > + pos += ts_addbuf(pos, "Auto485", "%s", sbcinfo->auto485 ? "yes" : "no");
> > + pos += ts_addbuf(pos, "SRAM", "%s", sbcinfo->sram ? "yes" : "no");
> > + pos += ts_addbuf(pos, "External Reset", "%s",
> > + sbcinfo->external_reset ? "yes" : "no");
>
> Most of these look like they should be simple "one value per file" sysfs
> files for your system. That would make things much easier on your
> userspace tools to handle parsing them properly, right?
>
> Why not do that instead?

I had a look at other drivers, and /sys/devices/platform/ts/ seems to be
a good place to hold those entries (one per options as you suggested).
Do you agree with that?

By the way, as they would be sysfs attributes, is Documentation/ABI the
best place to add the documentation file?

>
> thanks,
>
> greg k-h

Regards,
Vivien.

2011-05-02 21:55:45

by Greg KH

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

On Mon, May 02, 2011 at 05:07:29PM -0400, Vivien Didelot wrote:
> Hi,
>
> Thanks you all for your comments, I'll consider them carefully.
>
> Excerpts from Greg KH's message of 2011-04-29 19:32:30 -0400:
> > On Fri, Apr 29, 2011 at 06:21:48PM -0400, Vivien Didelot wrote:
> > > +/proc filesystem
> > > +----------------
> > > +
> > > +Information about the TS board is available through the /proc/ts-sbcinfo.
> >
> > Really? Why?
> >
> > As you now have added a new kernel/user ABI, it must be documented in
> > Documentation/ABI/
> >
> > Please include that in your next patch.
> >
> > But first off, why a new proc file? What is it used for?
>
> I think it could be useful for the user to know what option is available
> or not on the board.

But that's not how we export data to userspace anymore, sorry. We try
to learn from our past mistakes.

Almost all of this data can be gotten from other methods anyway, you
need to justify why you wish to repeat this here.

> > > +static int ts_sbcinfo_init_buffer(char *buf, struct ts5xxx_sbcinfo *sbcinfo)
> > > +{
> > > + char *pos = buf;
> > > +
> > > + pos += ts_addbuf(pos, "Board ID", "TS-%d", sbcinfo->board_id);
> > > + pos += ts_addbuf(pos, "RS485", "%s", sbcinfo->rs485 ? "yes" : "no");
> > > + pos += ts_addbuf(pos, "AnalogToDigital", "%s",
> > > + sbcinfo->adc ? "yes" : "no");
> > > + pos += ts_addbuf(pos, "Auto485", "%s", sbcinfo->auto485 ? "yes" : "no");
> > > + pos += ts_addbuf(pos, "SRAM", "%s", sbcinfo->sram ? "yes" : "no");
> > > + pos += ts_addbuf(pos, "External Reset", "%s",
> > > + sbcinfo->external_reset ? "yes" : "no");
> >
> > Most of these look like they should be simple "one value per file" sysfs
> > files for your system. That would make things much easier on your
> > userspace tools to handle parsing them properly, right?
> >
> > Why not do that instead?
>
> I had a look at other drivers, and /sys/devices/platform/ts/ seems to be
> a good place to hold those entries (one per options as you suggested).
> Do you agree with that?

No, this is not a platform device, is it? Only platform devices go
there, and I really doubt you have many of them for this board.

> By the way, as they would be sysfs attributes, is Documentation/ABI the
> best place to add the documentation file?

Yes.

thanks,

greg k-h

2011-05-03 06:05:00

by Govindraj

[permalink] [raw]
Subject: Re: [RFC 4/5] leds: add support for Technologic Systems TS-5500 leds

Looks fine some minor comments.


On Sat, Apr 30, 2011 at 3:51 AM, Vivien Didelot
<[email protected]> wrote:
> From: Jonas Fonseca <[email protected]>
>

Missing Patch description.

>
> Signed-off-by: Vivien Didelot <[email protected]>
> ---
> ?MAINTAINERS ? ? ? ? ? ? ? ?| ? ?1 +
> ?drivers/leds/Kconfig ? ? ? | ? 12 +++
> ?drivers/leds/Makefile ? ? ?| ? ?1 +
> ?drivers/leds/leds-ts5500.c | ?211 ++++++++++++++++++++++++++++++++++++++++++++
> ?4 files changed, 225 insertions(+), 0 deletions(-)
> ?create mode 100644 drivers/leds/leds-ts5500.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index da3b6d3..c0a646d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6061,6 +6061,7 @@ F: ? ? ? ?include/linux/ts5xxx-sbcinfo.h
> ?F: ? ? drivers/gpio/ts5500-gpio.c
> ?F: ? ? include/linux/ts5500-gpio.h
> ?F: ? ? drivers/tty/serial/ts5500-auto485.c
> +F: ? ? drivers/leds/leds-ts5500.c
>
> ?TEGRA SUPPORT
> ?M: ? ? Colin Cross <[email protected]>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9bec869..14144df 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -379,6 +379,18 @@ config LEDS_NETXBIG
> ? ? ? ? ?and 5Big Network v2 boards. The LEDs are wired to a CPLD and are
> ? ? ? ? ?controlled through a GPIO extension bus.
>
> +config LEDS_TS5500
> + ? ? ? tristate "LED Support for TS-5500 SBCs"
> + ? ? ? depends on LEDS_CLASS && TS5500_SBC
> + ? ? ? help
> + ? ? ? ? This option enables support for on-chip LED drivers found
> + ? ? ? ? on Technologic Systems TS-5500 SBCs.
> +
> + ? ? ? ? To compile this driver as a module, choose M here: the module will
> + ? ? ? ? be called leds-ts5500.
> +
> +comment "LED Triggers"
> +
> ?config LEDS_TRIGGERS
> ? ? ? ?bool "LED Trigger support"
> ? ? ? ?depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 39c80fc..ce5a25f 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_LEDS_DELL_NETBOOKS) ? ? ?+= dell-led.o
> ?obj-$(CONFIG_LEDS_MC13783) ? ? ? ? ? ? += leds-mc13783.o
> ?obj-$(CONFIG_LEDS_NS2) ? ? ? ? ? ? ? ? += leds-ns2.o
> ?obj-$(CONFIG_LEDS_NETXBIG) ? ? ? ? ? ? += leds-netxbig.o
> +obj-$(CONFIG_LEDS_TS5500) ? ? ? ? ? ? ?+= leds-ts5500.o
>
> ?# LED SPI Drivers
> ?obj-$(CONFIG_LEDS_DAC124S085) ? ? ? ? ?+= leds-dac124s085.o
> diff --git a/drivers/leds/leds-ts5500.c b/drivers/leds/leds-ts5500.c
> new file mode 100644
> index 0000000..aec6d71
> --- /dev/null
> +++ b/drivers/leds/leds-ts5500.c
> @@ -0,0 +1,211 @@
> +/*
> + * Technologic Systems TS-5500 boards - LED driver
> + *
> + * Copyright (c) 2010 Savoir-faire Linux Inc.
> + * ? ? Jonas Fonseca <[email protected]>
> + *
> + * Portions Copyright (c) 2008 Compulab, Ltd.
> + * ? ? Mike Rapoport <[email protected]>
> + *
> + * Portions Copyright (c) 2006-2008 Marvell International Ltd.
> + * ? ? Eric Miao <[email protected]>
> + *
> + * 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/leds.h>
> +#include <linux/workqueue.h>
> +#include <linux/ts5xxx-sbcinfo.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME "leds-ts5500"
> +
> +/**
> + * struct ts5500_led - LED structure
> + * @cdev: ? ? ? ? ? ? ?LED class device structure.
> + * @work: ? ? ? ? ? ? ?Work structure.
> + * @new_brightness: ? ?LED brightness.
> + * @ioaddr: ? ? ? ? ? ?LED I/O address.
> + */
> +struct ts5500_led {
> + ? ? ? struct led_classdev ? ? cdev;
> + ? ? ? struct work_struct ? ? ?work;
> + ? ? ? enum led_brightness ? ? new_brightness;
> + ? ? ? int ? ? ? ? ? ? ? ? ? ? ioaddr;
> + ? ? ? int ? ? ? ? ? ? ? ? ? ? bit;
> +};
> +
> +static void ts5500_led_work(struct work_struct *work)
> +{
> + ? ? ? struct ts5500_led *led = container_of(work, struct ts5500_led, work);
> + ? ? ? u8 val = led->new_brightness ? led->bit : 0;
> +
> + ? ? ? outb(val, led->ioaddr);
> +}
> +
> +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);
> +
> + ? ? ? led->new_brightness = value;
> + ? ? ? schedule_work(&led->work);
> +}
> +
> +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;
> + ? ? ? led->new_brightness = LED_OFF;
> +
> + ? ? ? INIT_WORK(&led->work, ts5500_led_work);
> +
> + ? ? ? 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 ? = DRIVER_NAME,
> + ? ? ? ? ? ? ? .owner ?= THIS_MODULE,
> + ? ? ? },
> + ? ? ? .probe ? ? ? ? ?= ts5500_led_probe,
> + ? ? ? .remove ? ? ? ? = __devexit_p(ts5500_led_remove),
> +};
> +
> +

Extra Line here

> +static struct led_info ts5500_led_info = {
> + ? ? ? .name = DRIVER_NAME,
> + ? ? ? .default_trigger = DRIVER_NAME,
> + ? ? ? .flags = 0x01,

Magic value for flag.

A descriptive macro will be better.

> +};
> +
> +static struct led_platform_data ts5500_led_platform_data = {
> + ? ? ? .num_leds ? ? ? = 1,
> + ? ? ? .leds ? ? ? ? ? = &ts5500_led_info,
> +};
> +
> +static struct resource ts5500_led_resources[] = {
> + ? ? ? {
> + ? ? ? ? ? ? ? .name ? = DRIVER_NAME,
> + ? ? ? ? ? ? ? .start ?= 0x77,
> + ? ? ? ? ? ? ? .end ? ?= 0x77,
> + ? ? ? ? ? ? ? .flags ?= IORESOURCE_IO,
> + ? ? ? }
> +};
> +
> +static void ts5500_led_release(struct device *dev)
> +{
> + ? ? ? /* noop */
> +}
> +
> +static struct platform_device ts5500_led_device = {
> + ? ? ? .name = DRIVER_NAME,
> + ? ? ? .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 __init ts5500_led_init(void)
> +{
> + ? ? ? struct ts5xxx_sbcinfo sbcinfo;
> + ? ? ? int ret;
> +
> + ? ? ? ts5xxx_sbcinfo_set(&sbcinfo);
> +
> + ? ? ? if (sbcinfo.board_id != 5500) {
> + ? ? ? ? ? ? ? printk(KERN_ERR DRIVER_NAME
> + ? ? ? ? ? ? ? ? ? ? ?": Expected TS5500 but TS-SBC reported TS%d\n",
> + ? ? ? ? ? ? ? ? ? ? ?sbcinfo.board_id);

You also consider using pr_err.

Feel free to add.

Reviewed-by: Govindraj.R <[email protected]>


> + ? ? ? ? ? ? ? return -ENODEV;
> + ? ? ? }
> +
> + ? ? ? 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 TS5500");
> +MODULE_AUTHOR("Jonas Fonseca <[email protected]>");
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

2011-05-03 09:38:57

by Alan

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

> > I had a look at other drivers, and /sys/devices/platform/ts/ seems to be
> > a good place to hold those entries (one per options as you suggested).
> > Do you agree with that?
>
> No, this is not a platform device, is it? Only platform devices go
> there, and I really doubt you have many of them for this board.

It seems analogous to some of the laptop helpers so I don't think it is
that unreasonable to make the board config a platform device of its own -
we don't really have a general solution to the board as a device, so if
you look at equivalents on laptops and the like we have assorted platform
control interfaces that are platform devices.

Alan

2011-05-03 14:13:00

by Greg KH

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

On Tue, May 03, 2011 at 10:39:19AM +0100, Alan Cox wrote:
> > > I had a look at other drivers, and /sys/devices/platform/ts/ seems to be
> > > a good place to hold those entries (one per options as you suggested).
> > > Do you agree with that?
> >
> > No, this is not a platform device, is it? Only platform devices go
> > there, and I really doubt you have many of them for this board.
>
> It seems analogous to some of the laptop helpers so I don't think it is
> that unreasonable to make the board config a platform device of its own -
> we don't really have a general solution to the board as a device, so if
> you look at equivalents on laptops and the like we have assorted platform
> control interfaces that are platform devices.

Platform control devices are great, I have no objection to that. I do
object to random proc files to try to describe the fact that this board
has a rs232 port on it, when that is able to be determined elsewhere.

thanks,

greg k-h

2011-05-03 15:55:42

by Vivien Didelot

[permalink] [raw]
Subject: Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

Hi Guenter,

Excerpts from Guenter Roeck's message of 2011-04-29 23:39:38 -0400:
> Hi Vivien,
>
> The headline and file name are a bit misleading, since the driver is really
> for MAX197 on a TS-5500 board.
>
> You should split the driver into two parts, a generic driver
> for the MAX197 and a platform driver (residing somewhere in arch/
> or possibly drivers/platform/) to instantiate it.
>
> There should be a platform data include file, probably in
> include/linux/platform_data/.
>
> .ioaddr in platform data should not be necessary. The driver's probe
> function should get the values using platform_get_resource().
>
> Having said that, from reading the code it looks like the chip is not really
> used for hardware monitoring, but for generic ADC functionality. A quick look
> into the TS-5500 user manual confirms this. So this should not be a hwmon
> driver in the first place, but a generic ADC driver. Given the ADC conversion rate
> of the MAX197, the hwmon ABI is not optimal anyway. You should move this driver
> into the iio subsystem, probably to drivers/staging/iio/adc. Copying the iio
> mailing list for input.
>
> Thanks,
> Guenter

I've took a closer look to the manual and that's right, in fact the
driver doesn't talk to the MAX197 directly. The board uses a CPLD to
abstract the interface to the MAX197. So all the MAX197 logic is hidden
by the CPLD. Therefore, the driver files should probably not have
function and structure names with a "max197_" prefix. I'll make the code
a bit clearer. What do you think?

Ok for iio subsystem, several ADC drivers are in drivers/hwmon/ but
drivers/staging/iio/adc seems to be the good place now.

Regards,
Vivien.

2011-05-03 17:33:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

On Tue, 2011-05-03 at 11:55 -0400, Vivien Didelot wrote:
> Hi Guenter,
>
> Excerpts from Guenter Roeck's message of 2011-04-29 23:39:38 -0400:
> > Hi Vivien,
> >
> > The headline and file name are a bit misleading, since the driver is really
> > for MAX197 on a TS-5500 board.
> >
> > You should split the driver into two parts, a generic driver
> > for the MAX197 and a platform driver (residing somewhere in arch/
> > or possibly drivers/platform/) to instantiate it.
> >
> > There should be a platform data include file, probably in
> > include/linux/platform_data/.
> >
> > .ioaddr in platform data should not be necessary. The driver's probe
> > function should get the values using platform_get_resource().
> >
> > Having said that, from reading the code it looks like the chip is not really
> > used for hardware monitoring, but for generic ADC functionality. A quick look
> > into the TS-5500 user manual confirms this. So this should not be a hwmon
> > driver in the first place, but a generic ADC driver. Given the ADC conversion rate
> > of the MAX197, the hwmon ABI is not optimal anyway. You should move this driver
> > into the iio subsystem, probably to drivers/staging/iio/adc. Copying the iio
> > mailing list for input.
> >
> > Thanks,
> > Guenter
>
> I've took a closer look to the manual and that's right, in fact the
> driver doesn't talk to the MAX197 directly. The board uses a CPLD to
> abstract the interface to the MAX197. So all the MAX197 logic is hidden
> by the CPLD. Therefore, the driver files should probably not have
> function and structure names with a "max197_" prefix. I'll make the code
> a bit clearer. What do you think?
>
The original driver for the chip on http://mcrapet.free.fr/ has a
platform dependent and a platform independent part. Other than coding
style issues, that approach seems to be cleaner to me.

Thanks,
Guenter

2011-05-04 09:01:07

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

On 05/03/11 16:55, Vivien Didelot wrote:
> Hi Guenter,
>
> Excerpts from Guenter Roeck's message of 2011-04-29 23:39:38 -0400:
>> Hi Vivien,
>>
>> The headline and file name are a bit misleading, since the driver is really
>> for MAX197 on a TS-5500 board.
>>
>> You should split the driver into two parts, a generic driver
>> for the MAX197 and a platform driver (residing somewhere in arch/
>> or possibly drivers/platform/) to instantiate it.
>>
>> There should be a platform data include file, probably in
>> include/linux/platform_data/.
>>
>> .ioaddr in platform data should not be necessary. The driver's probe
>> function should get the values using platform_get_resource().
>>
>> Having said that, from reading the code it looks like the chip is not really
>> used for hardware monitoring, but for generic ADC functionality. A quick look
>> into the TS-5500 user manual confirms this. So this should not be a hwmon
>> driver in the first place, but a generic ADC driver. Given the ADC conversion rate
>> of the MAX197, the hwmon ABI is not optimal anyway. You should move this driver
>> into the iio subsystem, probably to drivers/staging/iio/adc. Copying the iio
>> mailing list for input.
>>
>> Thanks,
>> Guenter
>
> I've took a closer look to the manual and that's right, in fact the
> driver doesn't talk to the MAX197 directly. The board uses a CPLD to
> abstract the interface to the MAX197. So all the MAX197 logic is hidden
> by the CPLD. Therefore, the driver files should probably not have
> function and structure names with a "max197_" prefix. I'll make the code
> a bit clearer. What do you think?
>
> Ok for iio subsystem, several ADC drivers are in drivers/hwmon/ but
> drivers/staging/iio/adc seems to be the good place now.
Just as a heads up, beware there are a few abi changes (and a lot of core
ones) working their way through review /already in staging-next.
I only mention it because merges against that tree will go through staging-next
and as it's name suggests is sometimes a fast moving target!

Jonathan
> Regards,
> Vivien.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2011-05-04 15:15:55

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

Excerpts from Alan Cox's message of 2011-04-30 06:07:05 -0400:
> > +
> > +/**
> > + * ts_sbcinfo_detect() - detect the TS board
> > + * @sbcinfo: structure where to store the detected board's info.
> > + */
> > +static int ts_sbcinfo_detect(struct ts5xxx_sbcinfo *sbcinfo)
> > +{
> > + u8 temp;
> > + struct ts_sbc_config *sbc;
> > + int ret = 0;
> > +
> > + memset(sbcinfo, 0, sizeof(*sbcinfo));
> > +
> > + if (!request_region(IOADDR_SBCID, 4, "TS-SBC"))
> > + return -EBUSY;
> > +
> > + temp = inb(IOADDR_SBCID);
> > + /* If it is a 3x00 SBC only match against the first 3 bits */
> > + if (temp & 0x07)
> > + temp &= 0x07;
>
> So if this is compiled into a kernel we blindly inb this address and some
> platform just crashed on boot. Is this board like other embedded ones in
> that there is some safe way to check if its such a board first ?
>

It is possible to make a BIOS call (Int 15h / Function B000h) to get
board information, like the manufacturer ("TS"). It should be a safer
way to check if we are on a Technologic Systems board. a intcall(0x15,
&ireg, &oreg) should do the trick to check the platform, before checking
the SBC ID. What do you think?

Thanks,
Vivien.

2011-05-04 15:29:26

by Alan

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

> It is possible to make a BIOS call (Int 15h / Function B000h) to get
> board information, like the manufacturer ("TS"). It should be a safer
> way to check if we are on a Technologic Systems board. a intcall(0x15,
> &ireg, &oreg) should do the trick to check the platform, before checking
> the SBC ID. What do you think?

And that goes to 'How you do yo know it's safe to call INT 15 0xB000 -
especially as we are in 32bit mode and the BIOS is 16bit so you have to
call it very early meaning its a great way to make something else not
boot.

Does the board not support anything sane like DMI ?

Alan

2011-05-04 16:29:19

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC 2/5] gpio: add support for Technologic Systems TS-5500 GPIOs

On Saturday 30 April 2011, Vivien Didelot wrote:
> + ts5xxx_sbcinfo_set(&sbcinfo);
> + if (5500 != sbcinfo.board_id) {
> + printk(MODULE_NAME ": Incompatible TS Board.\n");
> + return -ENODEV;
> + }

The above should not be necessary:

> +static int __init ts5500_gpio_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&ts5500_gpio_driver);
> + if (ret)
> + return ret;
> +
> + ret = platform_device_register(&gpio_device);
> + if (ret) {
> + printk(MODULE_NAME ": Failed to register platform device\n");
> + platform_driver_unregister(&ts5500_gpio_driver);
> + return ret;
> + }
> +
> + printk(MODULE_NAME ": GPIO/DIO driver loaded.\n");
> + return 0;
> +}
> +module_init(ts5500_gpio_init);

Doing it this way requires you know that you have to load the driver,
which is not good if you want to have the same kernel running on
different machines.

Better move the device registration into your platform code, in the
place where you know what device you have. The add a MODULE_DEVICE_TABLE()
entry to the module so the driver gets automatically loaded (if it's
a module), and match the platform device to the driver you register
from the module_init function.

Same for the other devices where you follow the same pattern.

Another way to do the same in a more modern fashion would be
to provide a flattened device tree that describes all your devices
and pass that into the kernel. Take a look at the CE4100 and OLPC
platforms to see how that is done.

Arnd

2011-05-04 20:34:40

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

Excerpts from Alan Cox's message of 2011-05-04 11:29:54 -0400:
> > It is possible to make a BIOS call (Int 15h / Function B000h) to get
> > board information, like the manufacturer ("TS"). It should be a safer
> > way to check if we are on a Technologic Systems board. a intcall(0x15,
> > &ireg, &oreg) should do the trick to check the platform, before checking
> > the SBC ID. What do you think?
>
> And that goes to 'How you do yo know it's safe to call INT 15 0xB000 -
> especially as we are in 32bit mode and the BIOS is 16bit so you have to
> call it very early meaning its a great way to make something else not
> boot.
>
> Does the board not support anything sane like DMI ?
>
> Alan

I've looked at that, and the board doesn't support DMI. Elements we have
are the processor (i.e. AMD ElanSC520), the BIOS, maybe the memory and
PCI bus layouts...

Have you got another idea how to get information in a safe way?

Regards,
Vivien.

2011-05-05 13:38:08

by Alan

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

> I've looked at that, and the board doesn't support DMI. Elements we have
> are the processor (i.e. AMD ElanSC520), the BIOS, maybe the memory and
> PCI bus layouts...

Well the CPU is a big help that cuts it down a lot. PCI bridge
subvendor/dev can be a good way to identify boards as well if they
manufacturers have actually bothered to set them properly.

2011-05-11 22:24:48

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

Excerpts from Alan Cox's message of jeu mai 05 09:38:37 -0400 2011:
> > I've looked at that, and the board doesn't support DMI. Elements we have
> > are the processor (i.e. AMD ElanSC520), the BIOS, maybe the memory and
> > PCI bus layouts...
>
> Well the CPU is a big help that cuts it down a lot. PCI bridge
> subvendor/dev can be a good way to identify boards as well if they
> manufacturers have actually bothered to set them properly.

In a first step, I now check the CPU and the Ethernet chip which are
relevant devices of TS-5xxx boards. The only problem comes from the
TS-5300 board which has a different Ethernet chip, using the ISA bus.
I can't find other relevant device information about this board. So, is
the CPU detection (AMD Elan SC520) enough? Or should I add a module
parameter to specify the board model and avoid this pre-detection?

Thanks,
Vivien.

2011-05-13 21:33:56

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 2/5] gpio: add support for Technologic Systems TS-5500 GPIOs

Excerpts from Alan Cox's message of sam avr 30 06:15:36 -0400 2011:
> > + /* 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 */
> > + }
>
> What happens if an IRQ occurs at this point, you have no handler for it ?

The IRQ is just not handled. What would be the proper way to handle
that? Would it be possible to write those registers when the IRQ is
requested?

Thanks,
Vivien.

2011-05-13 22:02:27

by Alan

[permalink] [raw]
Subject: Re: [RFC 2/5] gpio: add support for Technologic Systems TS-5500 GPIOs

On Fri, 13 May 2011 17:33:52 -0400
Vivien Didelot <[email protected]> wrote:

> Excerpts from Alan Cox's message of sam avr 30 06:15:36 -0400 2011:
> > > + /* 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 */
> > > + }
> >
> > What happens if an IRQ occurs at this point, you have no handler for it ?
>
> The IRQ is just not handled. What would be the proper way to handle
> that? Would it be possible to write those registers when the IRQ is
> requested?

The underlying rules are
- The moment you request_irq your IRQ handler may be called
- If you have a level triggered IRQ you must have a handler
before it is ever enabled (or you can get stuck)

Likewise mask it before free_irq

The only other deeply horrid and subtle going on with some PC hardware
especially is that your IRQ handler may be called *after* you mask the
IRQ on the hardware, but not after free_irq. This is because IRQ delivery
is effectively asynchronous to other bus traffic on some systems. [1]

You need things to happen in the following order ideally

Ensure IRQ cannot be generated
(most hardware default this way)
Register IRQ handler
Initialize all data structures to handle an IRQ
Enable IRQ

Sometimes when you just can't get that to occur you find drivers have to
do

Init some minimal data structures
foo->ready = 0;
Register IRQ handler
Enable IRQ
Initialize the rest

and the irq handler does

if (foo->ready == 0) {
clear IRQ source
return IRQ_HANDLED;
}

Alan
[1] An alternate view is that it's because hardware engineers have a very
black sense of humour

2011-05-17 12:37:22

by Sean Young

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

On 29 April 2011 23:21, Vivien Didelot
<[email protected]> wrote:
> From: Jonas Fonseca <[email protected]>
>
>
> Signed-off-by: Vivien Didelot <[email protected]>
> ---
> ?Documentation/ts5xxx-sbcinfo.txt ? ? ?| ? 47 ++++++
> ?MAINTAINERS ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?6 +
> ?drivers/platform/x86/Kconfig ? ? ? ? ?| ? 11 ++
> ?drivers/platform/x86/Makefile ? ? ? ? | ? ?1 +
> ?drivers/platform/x86/ts5xxx-sbcinfo.c | ?254 +++++++++++++++++++++++++++++++++
> ?include/linux/ts5xxx-sbcinfo.h ? ? ? ?| ? 42 ++++++
> ?6 files changed, 361 insertions(+), 0 deletions(-)
> ?create mode 100644 Documentation/ts5xxx-sbcinfo.txt
> ?create mode 100644 drivers/platform/x86/ts5xxx-sbcinfo.c
> ?create mode 100644 include/linux/ts5xxx-sbcinfo.h

Please note that the 2MiB on board flash is not supported by your
platform driver, which is handled by drivers/mtd/maps/ts5500_flash.c. I
guess it would be best to either merge that file into
drivers/platform/x86/ts5xxx-sbcinfo.c or select CONFIG_MTD_TS5500 in the
Kconfig.

In addition, the flash is used by the BIOS as RFD FTL, so possibly
CONFIG_RFD_FTL should be selected too.

Thanks,
Sean

2011-05-18 15:03:06

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

Hi Sean,
Excerpts from Sean Young's message of mar mai 17 08:37:19 -0400 2011:
> Please note that the 2MiB on board flash is not supported by your
> platform driver, which is handled by drivers/mtd/maps/ts5500_flash.c. I
> guess it would be best to either merge that file into
> drivers/platform/x86/ts5xxx-sbcinfo.c or select CONFIG_MTD_TS5500 in the
> Kconfig.
>
> In addition, the flash is used by the BIOS as RFD FTL, so possibly
> CONFIG_RFD_FTL should be selected too.
>
> Thanks,
> Sean
Good point. That's what I've noticed a few days ago, I'm actually
working on.

Thanks,
Vivien.

2011-06-06 20:48:51

by Vivien Didelot

[permalink] [raw]
Subject: Re: [RFC 3/5] serial: add support for Technologic Systems TS-5500 RS-485 serial port

Hi,
Sorry for the late reply to this comment.

On Sat, 30 Apr 2011 11:17:53
+0100, Alan Cox <[email protected]> wrote :
> > --- a/drivers/tty/serial/8250.c
> > +++ b/drivers/tty/serial/8250.c
> > @@ -109,6 +109,13 @@ static unsigned int skip_txen_test; /* force
> > skip of txen test at init time */
> > #define CONFIG_HUB6 1
> >
> > #include <asm/serial.h>
> > +
> > +/* TS-5500 related stuff */
> > +#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
> > +#define TS5500_TIMER2_SPEED_ADDR 0x42
> > +#define TS5500_485_SERIAL_PORT 0x02
> > +#endif
> > +
> > /*
> > * SERIAL_PORT_DFNS tells us about built-in ports that have no
> > * standard enumeration mechanism. Platforms that can find all
> > @@ -437,6 +444,25 @@ static void au_serial_out(struct uart_port *p,
> > int offset, int value)
> > __raw_writel(value, p->membase + offset);
> > }
> >
> > +#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
> > +void serial8250_ts5500_set_termios(struct uart_port *port,
> > + struct ktermios *new,
> > + struct ktermios *old)
> > +{
> > + u16 speed;
> > +
> > + if (new->c_ospeed >= 9600 && port->line ==
> > TS5500_485_SERIAL_PORT) {
> > + speed = ((115200 * 2) / new->c_ospeed);
> > +
> > + /* This should be written by low byte followed by
> > high byte */
> > + spin_lock_irq(&port->lock);
> > + outb((speed & 0x0F), TS5500_TIMER2_SPEED_ADDR);
> > + outb((speed >> 8), TS5500_TIMER2_SPEED_ADDR);
> > + spin_unlock_irq(&port->lock);
> > + }
> > +}
> > +#endif
>
> Please don't put board specific magic in this file, we are desperately
> trying to get rid of it.
>
> > +
> > static unsigned int tsi_serial_in(struct uart_port *p, int offset)
> > {
> > unsigned int tmp;
> > @@ -2464,6 +2490,10 @@ serial8250_do_set_termios(struct uart_port
> > *port, struct ktermios *termios,
> > /* Don't rewrite B0 */
> > if (tty_termios_baud_rate(termios))
> > tty_termios_encode_baud_rate(termios, baud, baud);
> > +
> > +#ifdef CONFIG_SERIAL_8250_TS5500_485_TIMER
> > + serial8250_ts5500_set_termios(port, termios, old);
> > +#endif
> > }
> > EXPORT_SYMBOL(serial8250_do_set_termios);
>
> Provide your own set_termios method and then call into this one (which
> is why it is exported)

I don't really understand how to do this. I've written my own
ts5500_serial8250_set_termios() function, which first makes a call to
serial8250_do_set_termios(), but I don't know how to make the platform
uses this one now. I've seen the plat_serial8250_port structure which
has a set_termios field, but I didn't find an example of usage of this
structure with this field. Maybe that's not the good way to "hook" the
serial8250_do_set_termios() function.
Could you please explain how to do this?

Regards,
Vivien.