This patch adds support for the GPIOs found on the Fintek super-I/O
chips F71882FG and F71889F.
A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
is used to connect the low-bandwidth devices. Among others functions the
F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
controller, an hardware monitoring controller and some GPIO pins.
Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
The GPIOs are used to control the LEDs and the hard drive power.
Signed-off-by: Simon Guinot <[email protected]>
---
Changes since v2:
- Remove useless NULL setters for driver data.
Changes since v1:
- Enhance the commit message by describing what is a Super-I/O.
- Use self-explanatory names for the GPIO register macros.
- Add a comment to explain the platform device and driver registration.
- Fix gpio_get when GPIO is configured in input mode. I only had
the hardware to check this mode recently...
drivers/gpio/Kconfig | 10 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-f7188x.c | 445 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 456 insertions(+)
create mode 100644 drivers/gpio/gpio-f7188x.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b2450ba..c5ae0ac 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -146,6 +146,16 @@ config GPIO_MM_LANTIQ
(EBU) found on Lantiq SoCs. The gpios are output only as they are
created by attaching a 16bit latch to the bus.
+config GPIO_F7188X
+ tristate "F71882FG and F71889F GPIO support"
+ depends on X86
+ help
+ This option enables support for GPIOs found on Fintek Super-I/O
+ chips F71882FG and F71889F.
+
+ To compile this driver as a module, choose M here: the module will
+ be called f7188x-gpio.
+
config GPIO_MPC5200
def_bool y
depends on PPC_MPC52xx
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ef3e983..bc7a677 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_GPIO_DA9055) += gpio-da9055.o
obj-$(CONFIG_ARCH_DAVINCI) += gpio-davinci.o
obj-$(CONFIG_GPIO_EM) += gpio-em.o
obj-$(CONFIG_GPIO_EP93XX) += gpio-ep93xx.o
+obj-$(CONFIG_GPIO_F7188X) += gpio-f7188x.o
obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
obj-$(CONFIG_GPIO_GRGPIO) += gpio-grgpio.o
obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c
new file mode 100644
index 0000000..4aaff6a
--- /dev/null
+++ b/drivers/gpio/gpio-f7188x.c
@@ -0,0 +1,445 @@
+/*
+ * GPIO driver for Fintek Super-I/O F71882 and F71889
+ *
+ * Copyright (C) 2010-2013 LaCie
+ *
+ * Author: Simon Guinot <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+
+#define DRVNAME "gpio-f7188x"
+
+/*
+ * Super-I/O registers
+ */
+#define SIO_LDSEL 0x07 /* Logical device select */
+#define SIO_DEVID 0x20 /* Device ID (2 bytes) */
+#define SIO_DEVREV 0x22 /* Device revision */
+#define SIO_MANID 0x23 /* Fintek ID (2 bytes) */
+
+#define SIO_LD_GPIO 0x06 /* GPIO logical device */
+#define SIO_UNLOCK_KEY 0x87 /* Key to enable Super-I/O */
+#define SIO_LOCK_KEY 0xAA /* Key to disable Super-I/O */
+
+#define SIO_FINTEK_ID 0x1934 /* Manufacturer ID */
+#define SIO_F71882_ID 0x0541 /* F71882 chipset ID */
+#define SIO_F71889_ID 0x0909 /* F71889 chipset ID */
+
+enum chips { f71882fg, f71889f };
+
+static const char * const f7188x_names[] = {
+ "f71882fg",
+ "f71889f",
+};
+
+struct f7188x_sio {
+ int addr;
+ enum chips type;
+};
+
+struct f7188x_gpio_bank {
+ struct gpio_chip chip;
+ unsigned int regbase;
+ struct f7188x_gpio_data *data;
+};
+
+struct f7188x_gpio_data {
+ struct f7188x_sio *sio;
+ int nr_bank;
+ struct f7188x_gpio_bank *bank;
+};
+
+/*
+ * Super-I/O functions.
+ */
+
+static inline int superio_inb(int base, int reg)
+{
+ outb(reg, base);
+ return inb(base + 1);
+}
+
+static int superio_inw(int base, int reg)
+{
+ int val;
+
+ outb(reg++, base);
+ val = inb(base + 1) << 8;
+ outb(reg, base);
+ val |= inb(base + 1);
+
+ return val;
+}
+
+static inline void superio_outb(int base, int reg, int val)
+{
+ outb(reg, base);
+ outb(val, base + 1);
+}
+
+static inline void superio_enter(int base)
+{
+ /* According to the datasheet the key must be send twice. */
+ outb(SIO_UNLOCK_KEY, base);
+ outb(SIO_UNLOCK_KEY, base);
+}
+
+static inline void superio_select(int base, int ld)
+{
+ outb(SIO_LDSEL, base);
+ outb(ld, base + 1);
+}
+
+static inline void superio_exit(int base)
+{
+ outb(SIO_LOCK_KEY, base);
+}
+
+/*
+ * GPIO chip.
+ */
+
+static int f7188x_gpio_direction_in(struct gpio_chip *chip, unsigned offset);
+static int f7188x_gpio_get(struct gpio_chip *chip, unsigned offset);
+static int f7188x_gpio_direction_out(struct gpio_chip *chip,
+ unsigned offset, int value);
+static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value);
+
+#define F7188X_GPIO_BANK(_base, _ngpio, _regbase) \
+ { \
+ .chip = { \
+ .label = DRVNAME, \
+ .owner = THIS_MODULE, \
+ .direction_input = f7188x_gpio_direction_in, \
+ .get = f7188x_gpio_get, \
+ .direction_output = f7188x_gpio_direction_out, \
+ .set = f7188x_gpio_set, \
+ .base = _base, \
+ .ngpio = _ngpio, \
+ }, \
+ .regbase = _regbase, \
+ }
+
+#define gpio_dir(base) (base + 0)
+#define gpio_data_out(base) (base + 1)
+#define gpio_data_in(base) (base + 2)
+/* Output mode register (0:open drain 1:push-pull). */
+#define gpio_out_mode(base) (base + 3)
+
+static struct f7188x_gpio_bank f71882_gpio_bank[] = {
+ F7188X_GPIO_BANK(0 , 8, 0xF0),
+ F7188X_GPIO_BANK(10, 8, 0xE0),
+ F7188X_GPIO_BANK(20, 8, 0xD0),
+ F7188X_GPIO_BANK(30, 4, 0xC0),
+ F7188X_GPIO_BANK(40, 4, 0xB0),
+};
+
+static struct f7188x_gpio_bank f71889_gpio_bank[] = {
+ F7188X_GPIO_BANK(0 , 7, 0xF0),
+ F7188X_GPIO_BANK(10, 7, 0xE0),
+ F7188X_GPIO_BANK(20, 8, 0xD0),
+ F7188X_GPIO_BANK(30, 8, 0xC0),
+ F7188X_GPIO_BANK(40, 8, 0xB0),
+ F7188X_GPIO_BANK(50, 5, 0xA0),
+ F7188X_GPIO_BANK(60, 8, 0x90),
+ F7188X_GPIO_BANK(70, 8, 0x80),
+};
+
+static int f7188x_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
+{
+ struct f7188x_gpio_bank *bank =
+ container_of(chip, struct f7188x_gpio_bank, chip);
+ struct f7188x_sio *sio = bank->data->sio;
+ u8 dir;
+
+ superio_enter(sio->addr);
+ superio_select(sio->addr, SIO_LD_GPIO);
+
+ dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
+ dir &= ~(1 << offset);
+ superio_outb(sio->addr, gpio_dir(bank->regbase), dir);
+
+ superio_exit(sio->addr);
+
+ return 0;
+}
+
+static int f7188x_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct f7188x_gpio_bank *bank =
+ container_of(chip, struct f7188x_gpio_bank, chip);
+ struct f7188x_sio *sio = bank->data->sio;
+ u8 dir, data;
+
+ superio_enter(sio->addr);
+ superio_select(sio->addr, SIO_LD_GPIO);
+
+ dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
+ dir = !!(dir & (1 << offset));
+ if (dir)
+ data = superio_inb(sio->addr, gpio_data_out(bank->regbase));
+ else
+ data = superio_inb(sio->addr, gpio_data_in(bank->regbase));
+
+ superio_exit(sio->addr);
+
+ return !!(data & 1 << offset);
+}
+
+static int f7188x_gpio_direction_out(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ struct f7188x_gpio_bank *bank =
+ container_of(chip, struct f7188x_gpio_bank, chip);
+ struct f7188x_sio *sio = bank->data->sio;
+ u8 dir, data_out;
+
+ superio_enter(sio->addr);
+ superio_select(sio->addr, SIO_LD_GPIO);
+
+ data_out = superio_inb(sio->addr, gpio_data_out(bank->regbase));
+ if (value)
+ data_out |= (1 << offset);
+ else
+ data_out &= ~(1 << offset);
+ superio_outb(sio->addr, gpio_data_out(bank->regbase), data_out);
+
+ dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
+ dir |= (1 << offset);
+ superio_outb(sio->addr, gpio_dir(bank->regbase), dir);
+
+ superio_exit(sio->addr);
+
+ return 0;
+}
+
+static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ struct f7188x_gpio_bank *bank =
+ container_of(chip, struct f7188x_gpio_bank, chip);
+ struct f7188x_sio *sio = bank->data->sio;
+ u8 data_out;
+
+ superio_enter(sio->addr);
+ superio_select(sio->addr, SIO_LD_GPIO);
+
+ data_out = superio_inb(sio->addr, gpio_data_out(bank->regbase));
+ if (value)
+ data_out |= (1 << offset);
+ else
+ data_out &= ~(1 << offset);
+ superio_outb(sio->addr, gpio_data_out(bank->regbase), data_out);
+
+ superio_exit(sio->addr);
+}
+
+/*
+ * Platform device and driver.
+ */
+
+static int f7188x_gpio_probe(struct platform_device *pdev)
+{
+ int err;
+ int i;
+ struct f7188x_sio *sio = pdev->dev.platform_data;
+ struct f7188x_gpio_data *data;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ switch (sio->type) {
+ case f71882fg:
+ data->nr_bank = ARRAY_SIZE(f71882_gpio_bank);
+ data->bank = f71882_gpio_bank;
+ break;
+ case f71889f:
+ data->nr_bank = ARRAY_SIZE(f71889_gpio_bank);
+ data->bank = f71889_gpio_bank;
+ break;
+ default:
+ return -ENODEV;
+ }
+ data->sio = sio;
+
+ platform_set_drvdata(pdev, data);
+
+ /* For each GPIO bank, register a GPIO chip. */
+ for (i = 0; i < data->nr_bank; i++) {
+ struct f7188x_gpio_bank *bank = &data->bank[i];
+
+ bank->chip.dev = &pdev->dev;
+ bank->data = data;
+
+ err = gpiochip_add(&bank->chip);
+ if (err) {
+ dev_err(&pdev->dev,
+ "Failed to register gpiochip %d: %d\n",
+ i, err);
+ goto err_gpiochip;
+ }
+ }
+
+ return 0;
+
+err_gpiochip:
+ for (i = i - 1; i >= 0; i--) {
+ struct f7188x_gpio_bank *bank = &data->bank[i];
+ int tmp;
+
+ tmp = gpiochip_remove(&bank->chip);
+ if (tmp < 0)
+ dev_err(&pdev->dev,
+ "Failed to remove gpiochip %d: %d\n",
+ i, tmp);
+ }
+
+ return err;
+}
+
+static int f7188x_gpio_remove(struct platform_device *pdev)
+{
+ int err;
+ int i;
+ struct f7188x_gpio_data *data = platform_get_drvdata(pdev);
+
+ for (i = 0; i < data->nr_bank; i++) {
+ struct f7188x_gpio_bank *bank = &data->bank[i];
+
+ err = gpiochip_remove(&bank->chip);
+ if (err) {
+ dev_err(&pdev->dev,
+ "Failed to remove GPIO gpiochip %d: %d\n",
+ i, err);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+static int __init f7188x_find(int addr, struct f7188x_sio *sio)
+{
+ int err = -ENODEV;
+ u16 devid;
+
+ superio_enter(addr);
+
+ devid = superio_inw(addr, SIO_MANID);
+ if (devid != SIO_FINTEK_ID) {
+ pr_debug(DRVNAME ": Not a Fintek device at 0x%08x\n", addr);
+ goto err;
+ }
+
+ devid = superio_inw(addr, SIO_DEVID);
+ switch (devid) {
+ case SIO_F71882_ID:
+ sio->type = f71882fg;
+ break;
+ case SIO_F71889_ID:
+ sio->type = f71889f;
+ break;
+ default:
+ pr_info(DRVNAME ": Unsupported Fintek device 0x%04x\n", devid);
+ goto err;
+ }
+ sio->addr = addr;
+
+ pr_info(DRVNAME ": Found %s at %#x, revision %d\n",
+ f7188x_names[sio->type],
+ (unsigned int) addr,
+ (int) superio_inb(addr, SIO_DEVREV));
+
+ err = 0;
+err:
+ superio_exit(addr);
+ return err;
+}
+
+static struct platform_device *f7188x_gpio_pdev;
+
+static int __init
+f7188x_gpio_device_add(const struct f7188x_sio *sio)
+{
+ int err;
+
+ f7188x_gpio_pdev = platform_device_alloc(DRVNAME, -1);
+ if (!f7188x_gpio_pdev)
+ return -ENOMEM;
+
+ err = platform_device_add_data(f7188x_gpio_pdev,
+ sio, sizeof(*sio));
+ if (err) {
+ pr_err(DRVNAME "Platform data allocation failed\n");
+ goto err;
+ }
+
+ err = platform_device_add(f7188x_gpio_pdev);
+ if (err) {
+ pr_err(DRVNAME "Device addition failed\n");
+ goto err;
+ }
+
+ return 0;
+
+err:
+ platform_device_put(f7188x_gpio_pdev);
+
+ return err;
+}
+
+/*
+ * Try to match a supported Fintech device by reading the (hard-wired)
+ * configuration I/O ports. If available, then register both the platform
+ * device and driver to support the GPIOs.
+ */
+
+static struct platform_driver f7188x_gpio_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = DRVNAME,
+ },
+ .probe = f7188x_gpio_probe,
+ .remove = f7188x_gpio_remove,
+};
+
+static int __init f7188x_gpio_init(void)
+{
+ int err;
+ struct f7188x_sio sio;
+
+ if (f7188x_find(0x2e, &sio) &&
+ f7188x_find(0x4e, &sio))
+ return -ENODEV;
+
+ err = platform_driver_register(&f7188x_gpio_driver);
+ if (!err) {
+ err = f7188x_gpio_device_add(&sio);
+ if (err)
+ platform_driver_unregister(&f7188x_gpio_driver);
+ }
+
+ return err;
+}
+subsys_initcall(f7188x_gpio_init);
+
+static void __exit f7188x_gpio_exit(void)
+{
+ platform_device_unregister(f7188x_gpio_pdev);
+ platform_driver_unregister(&f7188x_gpio_driver);
+}
+module_exit(f7188x_gpio_exit);
+
+MODULE_DESCRIPTION("GPIO driver for Super-I/O chips F71882FG and F71889F");
+MODULE_AUTHOR("Simon Guinot <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.7.10.4
On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
<[email protected]> wrote:
> This patch adds support for the GPIOs found on the Fintek super-I/O
> chips F71882FG and F71889F.
>
> A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> is used to connect the low-bandwidth devices. Among others functions the
> F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> controller, an hardware monitoring controller and some GPIO pins.
>
> Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> The GPIOs are used to control the LEDs and the hard drive power.
>
> Signed-off-by: Simon Guinot <[email protected]>
> ---
> Changes since v2:
> - Remove useless NULL setters for driver data.
Given the recent discussion with Rafael I want to have an
extended discussion of this patch.
It is my current understanding that:
- It is possible to define the whereabouts of the SuperIO
chips using ACPI
- It is possible for developers to influence the source
AML for the DSDT tables of these systems.
- It is the proper thing to do.
- So we should atleast support ACPI probing with the
port-based detection as a final fallback if all else fails.
Why can I not get something like:
#include <linux/acpi.h>
(...)
static const struct acpi_device_id gpio_acpi_match[] = {
{ "FOOBAR", 0 },
{ }
};
MODULE_DEVICE_TABLE(acpi, gpio_acpi_match);
static struct platform_driver gpio_driver = {
(...)
.driver = {
(...)
.acpi_match_table = ACPI_PTR(gpio_acpi_match),
},
};
?
If there is something wrong in my reasoning above, please
share it!
Yours,
Linus Walleij
On Monday, July 29, 2013 05:59:08 PM Linus Walleij wrote:
> On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
> <[email protected]> wrote:
>
> > This patch adds support for the GPIOs found on the Fintek super-I/O
> > chips F71882FG and F71889F.
> >
> > A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> > is used to connect the low-bandwidth devices. Among others functions the
> > F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> > controller, an hardware monitoring controller and some GPIO pins.
> >
> > Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> > The GPIOs are used to control the LEDs and the hard drive power.
> >
> > Signed-off-by: Simon Guinot <[email protected]>
> > ---
> > Changes since v2:
> > - Remove useless NULL setters for driver data.
>
> Given the recent discussion with Rafael I want to have an
> extended discussion of this patch.
>
> It is my current understanding that:
>
> - It is possible to define the whereabouts of the SuperIO
> chips using ACPI
> - It is possible for developers to influence the source
> AML for the DSDT tables of these systems.
> - It is the proper thing to do.
> - So we should atleast support ACPI probing with the
> port-based detection as a final fallback if all else fails.
>
> Why can I not get something like:
>
> #include <linux/acpi.h>
> (...)
> static const struct acpi_device_id gpio_acpi_match[] = {
> { "FOOBAR", 0 },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, gpio_acpi_match);
>
> static struct platform_driver gpio_driver = {
> (...)
> .driver = {
> (...)
> .acpi_match_table = ACPI_PTR(gpio_acpi_match),
> },
> };
>
> ?
>
> If there is something wrong in my reasoning above, please
> share it!
In addition to that, the "FOOBAR" PNP ID needs to be added to the
acpi_platform_device_ids[] table in drivers/acpi/acpi_platform.c.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Mon, Jul 29, 2013 at 05:59:08PM +0200, Linus Walleij wrote:
> On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
> <[email protected]> wrote:
>
> > This patch adds support for the GPIOs found on the Fintek super-I/O
> > chips F71882FG and F71889F.
> >
> > A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> > is used to connect the low-bandwidth devices. Among others functions the
> > F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> > controller, an hardware monitoring controller and some GPIO pins.
> >
> > Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> > The GPIOs are used to control the LEDs and the hard drive power.
> >
> > Signed-off-by: Simon Guinot <[email protected]>
> > ---
> > Changes since v2:
> > - Remove useless NULL setters for driver data.
Hi Linus,
>
> Given the recent discussion with Rafael I want to have an
> extended discussion of this patch.
>
> It is my current understanding that:
>
> - It is possible to define the whereabouts of the SuperIO
> chips using ACPI
Agreed.
> - It is possible for developers to influence the source
> AML for the DSDT tables of these systems.
I am not sure about that. Let's consider the LaCie x86-based boards.
LaCie only adds a few devices on the top of a motherboard provided by
an another manufacturer. In turns, this last gets a Super-I/O from an
another manufacturer. In my understanding, the Super-I/O manufacturer is
responsible for registering the PNP IDs (one per device functionality).
LaCie may have enough leverage to obtain some modifications on the ACPI
DSDT tables but about the PNP IDs registration, let's say it is less
that certain. The problem is that LaCie don't have any contacts with the
Super-I/O manufacturer.
I have to say that all this process is not as easy as adding a node in
a dts file.
> - It is the proper thing to do.
Yes, it may be.
> - So we should atleast support ACPI probing with the
> port-based detection as a final fallback if all else fails.
>
> Why can I not get something like:
>
> #include <linux/acpi.h>
> (...)
> static const struct acpi_device_id gpio_acpi_match[] = {
> { "FOOBAR", 0 },
After some checks on my boards, it appears that there is no PNP ID
available for the Super-I/O GPIO functionality (or any others). Moreover
I think this IDs don't have been registered to Microsoft by Fintech
(the super-I/O manufacturer).
How do you envisage the follow-up ?
> { }
> };
> MODULE_DEVICE_TABLE(acpi, gpio_acpi_match);
>
> static struct platform_driver gpio_driver = {
> (...)
> .driver = {
> (...)
> .acpi_match_table = ACPI_PTR(gpio_acpi_match),
> },
> };
It seems to me that the ACPI probing is the easiest part. How do you see
the ioport probing fallback ?
I can only figure out broken solutions:
1. From the init function, we could check that the PNP IDs are well
available in the ACPI DSDT tables before registering the platform
driver. If not, we could fall back on the ioport probing method.
I don't know if checking the DSDT tables is even possible. It is at
least weird and it defeats completely the purpos of acpi_match_table.
2. In a late initcall, we could check that the driver is well
registered else fall back on the ioport detection.
As GPIOs may be needed early, I don't think this method is suitable.
And I have no more ideas...
Please, help me to find a correct way to combine this probing methods.
>
> ?
>
> If there is something wrong in my reasoning above, please
> share it!
I'd rather say that your reasoning adds some difficulties :)
Thanks,
Simon
On 08/01/2013 06:46 AM, Simon Guinot wrote:
> On Mon, Jul 29, 2013 at 05:59:08PM +0200, Linus Walleij wrote:
>> On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
>> <[email protected]> wrote:
>>
>>> This patch adds support for the GPIOs found on the Fintek super-I/O
>>> chips F71882FG and F71889F.
>>>
>>> A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
>>> is used to connect the low-bandwidth devices. Among others functions the
>>> F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
>>> controller, an hardware monitoring controller and some GPIO pins.
>>>
>>> Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
>>> The GPIOs are used to control the LEDs and the hard drive power.
>>>
>>> Signed-off-by: Simon Guinot <[email protected]>
>>> ---
>>> Changes since v2:
>>> - Remove useless NULL setters for driver data.
>
> Hi Linus,
>
>>
>> Given the recent discussion with Rafael I want to have an
>> extended discussion of this patch.
>>
>> It is my current understanding that:
>>
>> - It is possible to define the whereabouts of the SuperIO
>> chips using ACPI
>
> Agreed.
>
>> - It is possible for developers to influence the source
>> AML for the DSDT tables of these systems.
>
> I am not sure about that. Let's consider the LaCie x86-based boards.
> LaCie only adds a few devices on the top of a motherboard provided by
> an another manufacturer. In turns, this last gets a Super-I/O from an
> another manufacturer. In my understanding, the Super-I/O manufacturer is
> responsible for registering the PNP IDs (one per device functionality).
>
> LaCie may have enough leverage to obtain some modifications on the ACPI
> DSDT tables but about the PNP IDs registration, let's say it is less
> that certain. The problem is that LaCie don't have any contacts with the
> Super-I/O manufacturer.
>
> I have to say that all this process is not as easy as adding a node in
> a dts file.
>
>> - It is the proper thing to do.
>
> Yes, it may be.
>
>> - So we should atleast support ACPI probing with the
>> port-based detection as a final fallback if all else fails.
>>
>> Why can I not get something like:
>>
>> #include <linux/acpi.h>
>> (...)
>> static const struct acpi_device_id gpio_acpi_match[] = {
>> { "FOOBAR", 0 },
>
> After some checks on my boards, it appears that there is no PNP ID
> available for the Super-I/O GPIO functionality (or any others). Moreover
> I think this IDs don't have been registered to Microsoft by Fintech
> (the super-I/O manufacturer).
>
> How do you envisage the follow-up ?
>
>> { }
>> };
>> MODULE_DEVICE_TABLE(acpi, gpio_acpi_match);
>>
>> static struct platform_driver gpio_driver = {
>> (...)
>> .driver = {
>> (...)
>> .acpi_match_table = ACPI_PTR(gpio_acpi_match),
>> },
>> };
>
> It seems to me that the ACPI probing is the easiest part. How do you see
> the ioport probing fallback ?
>
> I can only figure out broken solutions:
>
> 1. From the init function, we could check that the PNP IDs are well
> available in the ACPI DSDT tables before registering the platform
> driver. If not, we could fall back on the ioport probing method.
> I don't know if checking the DSDT tables is even possible. It is at
> least weird and it defeats completely the purpos of acpi_match_table.
> 2. In a late initcall, we could check that the driver is well
> registered else fall back on the ioport detection.
> As GPIOs may be needed early, I don't think this method is suitable.
>
> And I have no more ideas...
>
3. Implement Super-IO detection in the the ACPI platform driver.
If there is no ACPI device entry for a detected Super-IO chip's sub-function(s),
fake it and create the respective platform device(s).
Just as kludgy as your proposed solutions, but at least it would move Super-IO detection
to one file and let all Super-IO drivers use the ACPI match table.
Drawbacks:
- Only works for x86 (ie it would limit SuperIO drivers to x86). Not sure if that is
a real limitation - are SuperIO chips used on other platforms ?
- It would require us to define fake PNP IDs for the various SuperIO functions.
- It may fail if a firmware / chip vendor ever adds real PNP IDs for the various
sub-functions and those start showing up in ACPI tables (maybe that doesn't matter
as much as the drivers would have to be updated anyway to match the real IDs).
Guenter
On Thu, Aug 01, 2013 at 08:52:32AM -0700, Guenter Roeck wrote:
> On 08/01/2013 06:46 AM, Simon Guinot wrote:
> >On Mon, Jul 29, 2013 at 05:59:08PM +0200, Linus Walleij wrote:
> >>On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
> >><[email protected]> wrote:
> >>
> >>>This patch adds support for the GPIOs found on the Fintek super-I/O
> >>>chips F71882FG and F71889F.
> >>>
> >>>A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> >>>is used to connect the low-bandwidth devices. Among others functions the
> >>>F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> >>>controller, an hardware monitoring controller and some GPIO pins.
> >>>
> >>>Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> >>>The GPIOs are used to control the LEDs and the hard drive power.
> >>>
> >>>Signed-off-by: Simon Guinot <[email protected]>
> >>>---
> >>>Changes since v2:
> >>>- Remove useless NULL setters for driver data.
> >
> >Hi Linus,
> >
> >>
> >>Given the recent discussion with Rafael I want to have an
> >>extended discussion of this patch.
> >>
> >>It is my current understanding that:
> >>
> >>- It is possible to define the whereabouts of the SuperIO
> >> chips using ACPI
> >
> >Agreed.
> >
> >>- It is possible for developers to influence the source
> >> AML for the DSDT tables of these systems.
> >
> >I am not sure about that. Let's consider the LaCie x86-based boards.
> >LaCie only adds a few devices on the top of a motherboard provided by
> >an another manufacturer. In turns, this last gets a Super-I/O from an
> >another manufacturer. In my understanding, the Super-I/O manufacturer is
> >responsible for registering the PNP IDs (one per device functionality).
> >
> >LaCie may have enough leverage to obtain some modifications on the ACPI
> >DSDT tables but about the PNP IDs registration, let's say it is less
> >that certain. The problem is that LaCie don't have any contacts with the
> >Super-I/O manufacturer.
> >
> >I have to say that all this process is not as easy as adding a node in
> >a dts file.
> >
> >>- It is the proper thing to do.
> >
> >Yes, it may be.
> >
> >>- So we should atleast support ACPI probing with the
> >> port-based detection as a final fallback if all else fails.
> >>
> >>Why can I not get something like:
> >>
> >>#include <linux/acpi.h>
> >>(...)
> >>static const struct acpi_device_id gpio_acpi_match[] = {
> >> { "FOOBAR", 0 },
> >
> >After some checks on my boards, it appears that there is no PNP ID
> >available for the Super-I/O GPIO functionality (or any others). Moreover
> >I think this IDs don't have been registered to Microsoft by Fintech
> >(the super-I/O manufacturer).
> >
> >How do you envisage the follow-up ?
> >
> >> { }
> >>};
> >>MODULE_DEVICE_TABLE(acpi, gpio_acpi_match);
> >>
> >>static struct platform_driver gpio_driver = {
> >> (...)
> >> .driver = {
> >> (...)
> >> .acpi_match_table = ACPI_PTR(gpio_acpi_match),
> >> },
> >>};
> >
> >It seems to me that the ACPI probing is the easiest part. How do you see
> >the ioport probing fallback ?
> >
> >I can only figure out broken solutions:
> >
> >1. From the init function, we could check that the PNP IDs are well
> > available in the ACPI DSDT tables before registering the platform
> > driver. If not, we could fall back on the ioport probing method.
> > I don't know if checking the DSDT tables is even possible. It is at
> > least weird and it defeats completely the purpos of acpi_match_table.
> >2. In a late initcall, we could check that the driver is well
> > registered else fall back on the ioport detection.
> > As GPIOs may be needed early, I don't think this method is suitable.
> >
> >And I have no more ideas...
> >
> 3. Implement Super-IO detection in the the ACPI platform driver.
> If there is no ACPI device entry for a detected Super-IO chip's sub-function(s),
> fake it and create the respective platform device(s).
>
> Just as kludgy as your proposed solutions, but at least it would move Super-IO detection
> to one file and let all Super-IO drivers use the ACPI match table.
>
> Drawbacks:
> - Only works for x86 (ie it would limit SuperIO drivers to x86). Not sure if that is
> a real limitation - are SuperIO chips used on other platforms ?
> - It would require us to define fake PNP IDs for the various SuperIO functions.
> - It may fail if a firmware / chip vendor ever adds real PNP IDs for the various
> sub-functions and those start showing up in ACPI tables (maybe that doesn't matter
> as much as the drivers would have to be updated anyway to match the real IDs).
As an additional drawback, this solution involves systematic I/O port
probing from the ACPI platform driver, even if the platform don't need
it. This may increase the conflicts risk in comparison with probes from
some end drivers (which are more likely built as modules and not loaded
automatically). Except that, moving all this Super-I/O detection in a
same place would be indeed a great benefit.
Linus, how do you see the follow-up for the gpio-f7188x driver ? Let me
know your advice.
Thanks,
Simon
On Thu, Aug 1, 2013 at 3:46 PM, Simon Guinot <[email protected]> wrote:
> On Mon, Jul 29, 2013 at 05:59:08PM +0200, Linus Walleij wrote:
>> - So we should atleast support ACPI probing with the
>> port-based detection as a final fallback if all else fails.
>>
>> Why can I not get something like:
>>
>> #include <linux/acpi.h>
>> (...)
>> static const struct acpi_device_id gpio_acpi_match[] = {
>> { "FOOBAR", 0 },
>
> After some checks on my boards, it appears that there is no PNP ID
> available for the Super-I/O GPIO functionality (or any others). Moreover
> I think this IDs don't have been registered to Microsoft by Fintech
> (the super-I/O manufacturer).
>
> How do you envisage the follow-up ?
Well shall we just apply this as-is then, with ISA-style port
IO probing and all?
I think Rafael said something about it being possible for us
to register our own kernel ACPI PNP IDs (as if: there is no
road here, but if someone starts to walk here, a road will
soon become, and we take the first step then).
But overall I am a bit confused: I am hearing from one end
of the x86 community that ACPI is the way to go for
configuring platform devices on x86, yet stuff like this is
popping up from independent vendors, and get integrated
on boards with no ACPI tables in sight.
Over at ksummit-discuss we have had a thread about
whether device tree should be used in such cases, but
that is not clear either.
Basically I'm a bit confused because the x86 community
is talking with so many voices and I'm not used to it,
and I don't know if they actually have a common vision.
So I'm cc:ing some of my x86 friends to see if they
are OK with this port-probing approach before I merge
the driver.... Rafael, HPA, Matthew?
Yours,
Linus Walleij
On Thu, Aug 29, 2013 at 02:39:33PM +0200, Linus Walleij wrote:
> I think Rafael said something about it being possible for us
> to register our own kernel ACPI PNP IDs (as if: there is no
> road here, but if someone starts to walk here, a road will
> soon become, and we take the first step then).
It'd be straightforward to register the LNX PnP prefix and have someone
take responsibility for assigning numbers, but really a generic vendor
string should only be used when defining programming models rather than
specific devices.
> But overall I am a bit confused: I am hearing from one end
> of the x86 community that ACPI is the way to go for
> configuring platform devices on x86, yet stuff like this is
> popping up from independent vendors, and get integrated
> on boards with no ACPI tables in sight.
ACPI is usually used to describe systems, and the normal ACPI way of
handling GPIO devices is to expose the device at the other end of the
GPIO lines and then provide AML for toggling the lines. Attaching an
actual driver to the device would interfere with that, so nobody writes
an actual driver.
> Over at ksummit-discuss we have had a thread about
> whether device tree should be used in such cases, but
> that is not clear either.
If a vendor doesn't provide any way to autoprobe a device, there's no
way to autoprobe a device. That usually means that you're not expected
to use that device.
> Basically I'm a bit confused because the x86 community
> is talking with so many voices and I'm not used to it,
> and I don't know if they actually have a common vision.
x86 is driven by the vendors, not us. If the vendors don't provide ACPI
entries for a device then the choices are to either use port probing or
refuse to support that device. We've traditionally gone for the former.
--
Matthew Garrett | [email protected]
On 08/29/2013 05:57 AM, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 02:39:33PM +0200, Linus Walleij wrote:
>
>> I think Rafael said something about it being possible for us
>> to register our own kernel ACPI PNP IDs (as if: there is no
>> road here, but if someone starts to walk here, a road will
>> soon become, and we take the first step then).
>
> It'd be straightforward to register the LNX PnP prefix and have someone
> take responsibility for assigning numbers, but really a generic vendor
> string should only be used when defining programming models rather than
> specific devices.
>
>> But overall I am a bit confused: I am hearing from one end
>> of the x86 community that ACPI is the way to go for
>> configuring platform devices on x86, yet stuff like this is
>> popping up from independent vendors, and get integrated
>> on boards with no ACPI tables in sight.
>
> ACPI is usually used to describe systems, and the normal ACPI way of
> handling GPIO devices is to expose the device at the other end of the
> GPIO lines and then provide AML for toggling the lines. Attaching an
> actual driver to the device would interfere with that, so nobody writes
> an actual driver.
>
>> Over at ksummit-discuss we have had a thread about
>> whether device tree should be used in such cases, but
>> that is not clear either.
>
> If a vendor doesn't provide any way to autoprobe a device, there's no
> way to autoprobe a device. That usually means that you're not expected
> to use that device.
>
Pretty radical. Following your advice, should we remove all watchdog
and hwmon drivers for all SuperIO chips out there, plus any existing
gpio drivers (drivers/char/pc8736x_gpio.c might be a candidate) ?
Oh, and the parallel port driver also detects super-io chips directly,
so maybe the respective code should be removed as well. I am sure there
is more code that can be removed.
Or is the idea to say "no acpi, no new driver" ? Just wondering -
I have a GPIO driver for Nuvoton chips on my back-burner; that would be
necessary to access some fan controls connected to gpio pins on some boards.
If this is a no-go, I'll happily drop it from my list of things to do,
and just tell the user community that Linux won't support their hardware
due to policy reasons.
Guenter
On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> On 08/29/2013 05:57 AM, Matthew Garrett wrote:
> >If a vendor doesn't provide any way to autoprobe a device, there's no
> >way to autoprobe a device. That usually means that you're not expected
> >to use that device.
> >
>
> Pretty radical. Following your advice, should we remove all watchdog
> and hwmon drivers for all SuperIO chips out there, plus any existing
> gpio drivers (drivers/char/pc8736x_gpio.c might be a candidate) ?
The majority of board vendors clearly don't expect the OS to drive the
hwmon chips - they're there for the benefit of ACPI and SMM code. That
doesn't mean that there's no benefit in having drivers for them, just
that the board vendors don't care about that use case and so won't do
anything to make it easier.
--
Matthew Garrett | [email protected]
On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > On 08/29/2013 05:57 AM, Matthew Garrett wrote:
> > >If a vendor doesn't provide any way to autoprobe a device, there's no
> > >way to autoprobe a device. That usually means that you're not expected
> > >to use that device.
> > >
> >
> > Pretty radical. Following your advice, should we remove all watchdog
> > and hwmon drivers for all SuperIO chips out there, plus any existing
> > gpio drivers (drivers/char/pc8736x_gpio.c might be a candidate) ?
>
> The majority of board vendors clearly don't expect the OS to drive the
> hwmon chips - they're there for the benefit of ACPI and SMM code. That
> doesn't mean that there's no benefit in having drivers for them, just
> that the board vendors don't care about that use case and so won't do
> anything to make it easier.
>
Actually, not entirely true. Some vendors even provide software running on
Windows to access those chips (including access to fans controlled through
GPIO pins) and to provide information to the user. It might be more accurate
to say that some board vendors don't care about Linux (or about providing
access through ACPI, for that matter).
Question here is what the Linux kernel community's policy is going to be
to handle such cases. Pragmatic or dogmatic ?
Guenter
On Thu, Aug 29, 2013 at 09:08:30AM -0700, Guenter Roeck wrote:
> On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> > On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > The majority of board vendors clearly don't expect the OS to drive the
> > hwmon chips - they're there for the benefit of ACPI and SMM code. That
> > doesn't mean that there's no benefit in having drivers for them, just
> > that the board vendors don't care about that use case and so won't do
> > anything to make it easier.
> >
> Actually, not entirely true. Some vendors even provide software running on
> Windows to access those chips (including access to fans controlled through
> GPIO pins) and to provide information to the user. It might be more accurate
> to say that some board vendors don't care about Linux (or about providing
> access through ACPI, for that matter).
Eh. If they're really providing code that just assumes that hardware is
present and bangs on it, what choice do we have? You've checked that the
machines in question don't have a magic ACPI device that provides
resource information?
> Question here is what the Linux kernel community's policy is going to be
> to handle such cases. Pragmatic or dogmatic ?
I don't see any benefit in changing the status quo. Sometimes hardware
is just shit. The majority of x86 vendors certainly don't care about
anything we do, so it's not like we're in a position to force them to
change.
--
Matthew Garrett | [email protected]
On Thu, Aug 29, 2013 at 05:25:51PM +0100, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 09:08:30AM -0700, Guenter Roeck wrote:
> > On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> > > On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > > The majority of board vendors clearly don't expect the OS to drive the
> > > hwmon chips - they're there for the benefit of ACPI and SMM code. That
> > > doesn't mean that there's no benefit in having drivers for them, just
> > > that the board vendors don't care about that use case and so won't do
> > > anything to make it easier.
> > >
> > Actually, not entirely true. Some vendors even provide software running on
> > Windows to access those chips (including access to fans controlled through
> > GPIO pins) and to provide information to the user. It might be more accurate
> > to say that some board vendors don't care about Linux (or about providing
> > access through ACPI, for that matter).
>
> Eh. If they're really providing code that just assumes that hardware is
> present and bangs on it, what choice do we have? You've checked that the
> machines in question don't have a magic ACPI device that provides
> resource information?
>
Not to my knowledge. One vendor doing this extensively is ASRock. They have
a Windows based tuning aplication which lets users even set chip voltages
through SuperIO GPIO pins. Not that I would recommend supporting anything
like that in Linux ;).
I also have a couple of Intel motherboards which don't support ACPI methods
to access hardware monitoring information or the watchdog on the SuperIO chip.
Guess they don't want me to use that information ;).
There is lots of hardware monitoring tools available for Windows. Practically
none of them uses ACPI to collect the information from the hardware. Instead,
they use the same method we use in the Linux kernel to detect the SuperIO chip.
An open source tool is http://openhardwaremonitor.org/.
> > Question here is what the Linux kernel community's policy is going to be
> > to handle such cases. Pragmatic or dogmatic ?
>
> I don't see any benefit in changing the status quo. Sometimes hardware
> is just shit. The majority of x86 vendors certainly don't care about
> anything we do, so it's not like we're in a position to force them to
> change.
>
Me not either. It just sounded to me that this was the suggestion, so I was
asking for clarification. It doesn't make sense for people like me to submit
a driver into the kernel only to have it rejected for non-technical reasons.
Guenter
On Thu, Aug 29, 2013 at 6:08 PM, Guenter Roeck <[email protected]> wrote:
> On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
>> The majority of board vendors clearly don't expect the OS to drive the
>> hwmon chips - they're there for the benefit of ACPI and SMM code. That
>> doesn't mean that there's no benefit in having drivers for them, just
>> that the board vendors don't care about that use case and so won't do
>> anything to make it easier.
>>
> Actually, not entirely true. Some vendors even provide software running on
> Windows to access those chips (including access to fans controlled through
> GPIO pins) and to provide information to the user. It might be more accurate
> to say that some board vendors don't care about Linux (or about providing
> access through ACPI, for that matter).
>
> Question here is what the Linux kernel community's policy is going to be
> to handle such cases. Pragmatic or dogmatic ?
The GPIO maintainer is going to be pragmatic, because it's cool to
be able to control HWMON things on one's workstation.
The kernel is a funnier place with these drivers, so I'm applying the
patch.
Yours,
Linus Walleij
On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
<[email protected]> wrote:
> This patch adds support for the GPIOs found on the Fintek super-I/O
> chips F71882FG and F71889F.
>
> A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> is used to connect the low-bandwidth devices. Among others functions the
> F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> controller, an hardware monitoring controller and some GPIO pins.
>
> Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> The GPIOs are used to control the LEDs and the hard drive power.
>
> Signed-off-by: Simon Guinot <[email protected]>
> ---
> Changes since v2:
> - Remove useless NULL setters for driver data.
OK we had some discussion on this: there seem to be no alternatives
to port probing, and hacking these board drivers is mighty cool, so
patch applied.
Yours,
Linus Walleij
On Thu, Aug 29, 2013 at 05:25:51PM +0100, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 09:08:30AM -0700, Guenter Roeck wrote:
> > On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> > > On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > > The majority of board vendors clearly don't expect the OS to drive the
> > > hwmon chips - they're there for the benefit of ACPI and SMM code. That
> > > doesn't mean that there's no benefit in having drivers for them, just
> > > that the board vendors don't care about that use case and so won't do
> > > anything to make it easier.
> > >
> > Actually, not entirely true. Some vendors even provide software running on
> > Windows to access those chips (including access to fans controlled through
> > GPIO pins) and to provide information to the user. It might be more accurate
> > to say that some board vendors don't care about Linux (or about providing
> > access through ACPI, for that matter).
>
> Eh. If they're really providing code that just assumes that hardware is
> present and bangs on it, what choice do we have? You've checked that the
> machines in question don't have a magic ACPI device that provides
> resource information?
>
> > Question here is what the Linux kernel community's policy is going to be
> > to handle such cases. Pragmatic or dogmatic ?
>
> I don't see any benefit in changing the status quo. Sometimes hardware
> is just shit. The majority of x86 vendors certainly don't care about
> anything we do, so it's not like we're in a position to force them to
> change.
Almost all the LaCie NASes are running a Linux based OS. I can assure
you that LaCie really takes care of Linux. Now, the main reason why the
Super-I/O ACPI support is missing on this boards is a lack of knowledge.
Thanks to this discussion, I am now aware of that. Then, for the next
products, I will do my best to get AMLs or at least ACPI IDs.
Thanks,
Simon
On Fri, Aug 30, 2013 at 12:20:54AM +0200, Simon Guinot wrote:
> Almost all the LaCie NASes are running a Linux based OS. I can assure
> you that LaCie really takes care of Linux. Now, the main reason why the
> Super-I/O ACPI support is missing on this boards is a lack of knowledge.
> Thanks to this discussion, I am now aware of that. Then, for the next
> products, I will do my best to get AMLs or at least ACPI IDs.
Awesome! Let me know if you need any advice on this.
--
Matthew Garrett | [email protected]