* [PATCH 1/2] misc: add nirtfeatures driver
@ 2016-03-14 21:54 Kyle Roeschley
2016-03-14 22:05 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Kyle Roeschley @ 2016-03-14 21:54 UTC (permalink / raw)
To: gregkh, arnd
Cc: linux-kernel, jeff.westfahl, joshc, nathan.sullivan, xander.huff
From: Jeff Westfahl <jeff.westfahl@ni.com>
This driver introduces support for hardware features of National
Instruments real-time controllers. This is an ACPI device that exposes
LEDs, switches, and watchdogs.
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com>
---
drivers/misc/Kconfig | 9 +
drivers/misc/Makefile | 1 +
drivers/misc/nirtfeatures.c | 575 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 585 insertions(+)
create mode 100644 drivers/misc/nirtfeatures.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1557951..d97f71e 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -526,6 +526,15 @@ config VEXPRESS_SYSCFG
bus. System Configuration interface is one of the possible means
of generating transactions on this bus.
+config NI_RT_FEATURES
+ bool "NI 903x/913x support"
+ depends on X86 && ACPI
+ help
+ This driver exposes LEDs and other features of NI 903x/913x Real-Time
+ controllers.
+
+ If unsure, say N (but it's safe to say "Y").
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 537d7f3..539127d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/
obj-$(CONFIG_ECHO) += echo/
obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
obj-$(CONFIG_CXL_BASE) += cxl/
+obj-$(CONFIG_NI_RT_FEATURES) += nirtfeatures.o
diff --git a/drivers/misc/nirtfeatures.c b/drivers/misc/nirtfeatures.c
new file mode 100644
index 0000000..37298f2
--- /dev/null
+++ b/drivers/misc/nirtfeatures.c
@@ -0,0 +1,575 @@
+/*
+ * Copyright (C) 2016 National Instruments Corp.
+ *
+ * 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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+
+#define MODULE_NAME "nirtfeatures"
+
+/* Register addresses */
+
+#define NIRTF_YEAR 0x01
+#define NIRTF_MONTH 0x02
+#define NIRTF_DAY 0x03
+#define NIRTF_HOUR 0x04
+#define NIRTF_MINUTE 0x05
+#define NIRTF_SCRATCH 0x06
+#define NIRTF_PLATFORM_MISC 0x07
+#define NIRTF_PROC_RESET_SOURCE 0x11
+#define NIRTF_CONTROLLER_MODE 0x12
+#define NIRTF_SYSTEM_LEDS 0x20
+#define NIRTF_STATUS_LED_SHIFT1 0x21
+#define NIRTF_STATUS_LED_SHIFT0 0x22
+#define NIRTF_RT_LEDS 0x23
+
+#define NIRTF_IO_SIZE 0x40
+
+/* Register values */
+
+#define NIRTF_PLATFORM_MISC_ID_MASK 0x07
+#define NIRTF_PLATFORM_MISC_ID_MANHATTAN 0
+#define NIRTF_PLATFORM_MISC_ID_HAMMERHEAD 4
+#define NIRTF_PLATFORM_MISC_ID_WINGHEAD 5
+
+#define NIRTF_CONTROLLER_MODE_NO_FPGA_SW 0x40
+#define NIRTF_CONTROLLER_MODE_HARD_BOOT_N 0x20
+#define NIRTF_CONTROLLER_MODE_NO_FPGA 0x10
+#define NIRTF_CONTROLLER_MODE_RECOVERY 0x08
+#define NIRTF_CONTROLLER_MODE_CONSOLE_OUT 0x04
+#define NIRTF_CONTROLLER_MODE_IP_RESET 0x02
+#define NIRTF_CONTROLLER_MODE_SAFE 0x01
+
+#define NIRTF_SYSTEM_LEDS_STATUS_RED 0x08
+#define NIRTF_SYSTEM_LEDS_STATUS_YELLOW 0x04
+#define NIRTF_SYSTEM_LEDS_POWER_GREEN 0x02
+#define NIRTF_SYSTEM_LEDS_POWER_YELLOW 0x01
+
+#define NIRTF_RT_LEDS_USER2_GREEN 0x08
+#define NIRTF_RT_LEDS_USER2_YELLOW 0x04
+#define NIRTF_RT_LEDS_USER1_GREEN 0x02
+#define NIRTF_RT_LEDS_USER1_YELLOW 0x01
+
+#define to_nirtfeatures(dev) acpi_driver_data(to_acpi_device(dev))
+
+/* Structures */
+
+struct nirtfeatures {
+ struct acpi_device *acpi_device;
+ u16 io_base;
+ spinlock_t lock;
+ u8 revision[5];
+ const char *bpstring;
+ struct nirtfeatures_led *extra_leds;
+ unsigned num_extra_leds;
+};
+
+struct nirtfeatures_led {
+ struct led_classdev cdev;
+ struct nirtfeatures *nirtfeatures;
+ u8 address;
+ u8 mask;
+ u8 pattern_hi_addr;
+ u8 pattern_lo_addr;
+};
+
+/* sysfs files */
+
+static ssize_t revision_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+
+ return sprintf(buf, "20%02X/%02X/%02X %02X:%02X\n",
+ nirtfeatures->revision[0], nirtfeatures->revision[1],
+ nirtfeatures->revision[2], nirtfeatures->revision[3],
+ nirtfeatures->revision[4]);
+}
+static DEVICE_ATTR_RO(revision);
+
+static ssize_t scratch_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+ u8 data;
+
+ spin_lock(&nirtfeatures->lock);
+
+ data = inb(nirtfeatures->io_base + NIRTF_SCRATCH);
+
+ spin_unlock(&nirtfeatures->lock);
+
+ return sprintf(buf, "%02x\n", data);
+}
+
+static ssize_t scratch_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+ unsigned long tmp;
+
+ if (kstrtoul(buf, 0, &tmp) || (tmp > 0xFF))
+ return -EINVAL;
+
+ spin_lock(&nirtfeatures->lock);
+
+ outb((u8)tmp, nirtfeatures->io_base + NIRTF_SCRATCH);
+
+ spin_unlock(&nirtfeatures->lock);
+
+ return count;
+}
+static DEVICE_ATTR_RW(scratch);
+
+static ssize_t backplane_id_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+
+ return sprintf(buf, "%s\n", nirtfeatures->bpstring);
+}
+static DEVICE_ATTR_RO(backplane_id);
+
+static const char *const nirtfeatures_reset_source_strings[] = {
+ "button", "processor", "fpga", "watchdog", "software",
+};
+
+static ssize_t reset_source_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+ u8 data;
+ int i;
+
+ data = inb(nirtfeatures->io_base + NIRTF_PROC_RESET_SOURCE);
+
+ for (i = 0; i < ARRAY_SIZE(nirtfeatures_reset_source_strings); i++)
+ if ((1 << i) & data)
+ return sprintf(buf, "%s\n",
+ nirtfeatures_reset_source_strings[i]);
+
+ return sprintf(buf, "poweron\n");
+}
+static DEVICE_ATTR_RO(reset_source);
+
+static ssize_t mode_show(struct device *dev, char *buf, unsigned int mask)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+ u8 data;
+
+ data = inb(nirtfeatures->io_base + NIRTF_CONTROLLER_MODE);
+ data &= mask;
+
+ return sprintf(buf, "%u\n", !!data);
+}
+
+static ssize_t no_fpga_sw_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+ int ret;
+
+ spin_lock(&nirtfeatures->lock);
+
+ ret = mode_show(dev, buf, NIRTF_CONTROLLER_MODE_NO_FPGA_SW);
+
+ spin_unlock(&nirtfeatures->lock);
+
+ return ret;
+}
+
+static ssize_t no_fpga_sw_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+ unsigned long tmp;
+ u8 data;
+
+ if (kstrtoul(buf, 0, &tmp) || (tmp > 1))
+ return -EINVAL;
+
+ spin_lock(&nirtfeatures->lock);
+
+ data = inb(nirtfeatures->io_base + NIRTF_CONTROLLER_MODE);
+
+ if (tmp)
+ data |= NIRTF_CONTROLLER_MODE_NO_FPGA_SW;
+ else
+ data &= ~NIRTF_CONTROLLER_MODE_NO_FPGA_SW;
+
+ outb(data, nirtfeatures->io_base + NIRTF_CONTROLLER_MODE);
+
+ spin_unlock(&nirtfeatures->lock);
+
+ return count;
+}
+static DEVICE_ATTR_RW(no_fpga_sw);
+
+static ssize_t soft_reset_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return mode_show(dev, buf, NIRTF_CONTROLLER_MODE_HARD_BOOT_N);
+}
+static DEVICE_ATTR_RO(soft_reset);
+
+static ssize_t no_fpga_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return mode_show(dev, buf, NIRTF_CONTROLLER_MODE_NO_FPGA);
+}
+static DEVICE_ATTR_RO(no_fpga);
+
+static ssize_t recovery_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return mode_show(dev, buf, NIRTF_CONTROLLER_MODE_RECOVERY);
+}
+static DEVICE_ATTR_RO(recovery_mode);
+
+static ssize_t console_out_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return mode_show(dev, buf, NIRTF_CONTROLLER_MODE_CONSOLE_OUT);
+}
+static DEVICE_ATTR_RO(console_out);
+
+static ssize_t ip_reset_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return mode_show(dev, buf, NIRTF_CONTROLLER_MODE_IP_RESET);
+}
+static DEVICE_ATTR_RO(ip_reset);
+
+static ssize_t safe_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return mode_show(dev, buf, NIRTF_CONTROLLER_MODE_SAFE);
+}
+static DEVICE_ATTR_RO(safe_mode);
+
+static const struct attribute *nirtfeatures_attrs[] = {
+ &dev_attr_revision.attr,
+ &dev_attr_scratch.attr,
+ &dev_attr_backplane_id.attr,
+ &dev_attr_reset_source.attr,
+ &dev_attr_no_fpga_sw.attr,
+ &dev_attr_soft_reset.attr,
+ &dev_attr_no_fpga.attr,
+ &dev_attr_recovery_mode.attr,
+ &dev_attr_console_out.attr,
+ &dev_attr_ip_reset.attr,
+ &dev_attr_safe_mode.attr,
+ NULL
+};
+
+/* LEDs */
+
+static void nirtfeatures_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct nirtfeatures_led *led = (struct nirtfeatures_led *)led_cdev;
+ u8 data;
+
+ spin_lock(&led->nirtfeatures->lock);
+
+ data = inb(led->nirtfeatures->io_base + led->address);
+ data &= ~led->mask;
+ if (!!brightness)
+ data |= led->mask;
+ outb(data, led->nirtfeatures->io_base + led->address);
+
+ if (led->pattern_hi_addr && led->pattern_lo_addr) {
+ /* Write the high byte first. */
+ outb(brightness >> 8,
+ led->nirtfeatures->io_base + led->pattern_hi_addr);
+ outb(brightness & 0xFF,
+ led->nirtfeatures->io_base + led->pattern_lo_addr);
+ }
+
+ spin_unlock(&led->nirtfeatures->lock);
+}
+
+static enum led_brightness
+nirtfeatures_led_brightness_get(struct led_classdev *led_cdev)
+{
+ struct nirtfeatures_led *led = (struct nirtfeatures_led *)led_cdev;
+ u8 data;
+
+ data = inb(led->nirtfeatures->io_base + led->address);
+
+ /*
+ * For the yellow status LED, the blink pattern used for brightness
+ * on write is write-only, so we just return on/off for all LEDs.
+ */
+ return (data & led->mask) ? led_cdev->max_brightness : 0;
+}
+
+static struct nirtfeatures_led nirtfeatures_leds_common[] = {
+ {
+ {
+ .name = "nilrt:user1:green",
+ },
+ .address = NIRTF_RT_LEDS,
+ .mask = NIRTF_RT_LEDS_USER1_GREEN,
+ },
+ {
+ {
+ .name = "nilrt:user1:yellow",
+ },
+ .address = NIRTF_RT_LEDS,
+ .mask = NIRTF_RT_LEDS_USER1_YELLOW,
+ },
+ {
+ {
+ .name = "nilrt:status:red",
+ },
+ .address = NIRTF_SYSTEM_LEDS,
+ .mask = NIRTF_SYSTEM_LEDS_STATUS_RED,
+ },
+ {
+ {
+ .name = "nilrt:status:yellow",
+ .max_brightness = 0xFFFF,
+ },
+ .address = NIRTF_SYSTEM_LEDS,
+ .mask = NIRTF_SYSTEM_LEDS_STATUS_YELLOW,
+ .pattern_hi_addr = NIRTF_STATUS_LED_SHIFT1,
+ .pattern_lo_addr = NIRTF_STATUS_LED_SHIFT0,
+ },
+ {
+ {
+ .name = "nilrt:power:green",
+ },
+ .address = NIRTF_SYSTEM_LEDS,
+ .mask = NIRTF_SYSTEM_LEDS_POWER_GREEN,
+ },
+ {
+ {
+ .name = "nilrt:power:yellow",
+ },
+ .address = NIRTF_SYSTEM_LEDS,
+ .mask = NIRTF_SYSTEM_LEDS_POWER_YELLOW,
+ },
+};
+
+static struct nirtfeatures_led nirtfeatures_leds_cdaq[] = {
+ {
+ {
+ .name = "nilrt:user2:green",
+ },
+ .address = NIRTF_RT_LEDS,
+ .mask = NIRTF_RT_LEDS_USER2_GREEN,
+ },
+ {
+ {
+ .name = "nilrt:user2:yellow",
+ },
+ .address = NIRTF_RT_LEDS,
+ .mask = NIRTF_RT_LEDS_USER2_YELLOW,
+ },
+};
+
+static int nirtfeatures_create_leds(struct nirtfeatures *nirtfeatures)
+{
+ int i;
+ int err;
+
+ struct nirtfeatures_led *leds = nirtfeatures_leds_common;
+
+ for (i = 0; i < ARRAY_SIZE(nirtfeatures_leds_common); i++) {
+
+ leds[i].nirtfeatures = nirtfeatures;
+
+ if (leds[i].cdev.max_brightness == 0)
+ leds[i].cdev.max_brightness = 1;
+
+ leds[i].cdev.brightness_set = nirtfeatures_led_brightness_set;
+ leds[i].cdev.brightness_get = nirtfeatures_led_brightness_get;
+
+ err =
+ devm_led_classdev_register(&nirtfeatures->acpi_device->dev,
+ &leds[i].cdev);
+ if (err)
+ return err;
+ }
+
+ for (i = 0; i < nirtfeatures->num_extra_leds; ++i) {
+
+ nirtfeatures->extra_leds[i].nirtfeatures = nirtfeatures;
+
+ if (nirtfeatures->extra_leds[i].cdev.max_brightness == 0)
+ nirtfeatures->extra_leds[i].cdev.max_brightness = 1;
+
+ nirtfeatures->extra_leds[i].cdev.brightness_set =
+ nirtfeatures_led_brightness_set;
+
+ nirtfeatures->extra_leds[i].cdev.brightness_get =
+ nirtfeatures_led_brightness_get;
+
+ err = devm_led_classdev_register(
+ &nirtfeatures->acpi_device->dev,
+ &nirtfeatures->extra_leds[i].cdev);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+/* ACPI driver */
+
+static acpi_status nirtfeatures_resources(struct acpi_resource *res,
+ void *data)
+{
+ struct nirtfeatures *nirtfeatures = data;
+ u8 io_size;
+
+ if (res->type == ACPI_RESOURCE_TYPE_IO) {
+ if (nirtfeatures->io_base != 0) {
+ dev_err(&nirtfeatures->acpi_device->dev,
+ "too many IO resources\n");
+ return AE_ALREADY_EXISTS;
+ }
+
+ nirtfeatures->io_base = res->data.io.minimum;
+ io_size = res->data.io.address_length;
+
+ if (io_size != NIRTF_IO_SIZE) {
+ dev_err(&nirtfeatures->acpi_device->dev,
+ "invalid IO size 0x%02x\n", io_size);
+ return AE_ERROR;
+ }
+
+ if (!devm_request_region(&nirtfeatures->acpi_device->dev,
+ nirtfeatures->io_base, io_size,
+ MODULE_NAME)) {
+ dev_err(&nirtfeatures->acpi_device->dev,
+ "failed to get memory region\n");
+ return AE_NO_MEMORY;
+ }
+ }
+
+ return AE_OK;
+}
+
+static int nirtfeatures_acpi_add(struct acpi_device *device)
+{
+ struct nirtfeatures *nirtfeatures;
+ acpi_status acpi_ret;
+ u8 bpinfo;
+ int err;
+
+ nirtfeatures = devm_kzalloc(&device->dev, sizeof(*nirtfeatures),
+ GFP_KERNEL);
+ if (!nirtfeatures)
+ return -ENOMEM;
+
+ device->driver_data = nirtfeatures;
+ nirtfeatures->acpi_device = device;
+
+ acpi_ret = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
+ nirtfeatures_resources, nirtfeatures);
+ if (ACPI_FAILURE(acpi_ret) || nirtfeatures->io_base == 0) {
+ dev_err(&device->dev, "failed to get resources\n");
+ return -ENODEV;
+ }
+
+ bpinfo = inb(nirtfeatures->io_base + NIRTF_PLATFORM_MISC);
+ bpinfo &= NIRTF_PLATFORM_MISC_ID_MASK;
+
+ switch (bpinfo) {
+ case NIRTF_PLATFORM_MISC_ID_MANHATTAN:
+ nirtfeatures->bpstring = "Manhattan";
+ break;
+ case NIRTF_PLATFORM_MISC_ID_HAMMERHEAD:
+ nirtfeatures->bpstring = "Hammerhead";
+ nirtfeatures->extra_leds = nirtfeatures_leds_cdaq;
+ nirtfeatures->num_extra_leds =
+ ARRAY_SIZE(nirtfeatures_leds_cdaq);
+ break;
+ case NIRTF_PLATFORM_MISC_ID_WINGHEAD:
+ nirtfeatures->bpstring = "Winghead";
+ nirtfeatures->extra_leds = nirtfeatures_leds_cdaq;
+ nirtfeatures->num_extra_leds =
+ ARRAY_SIZE(nirtfeatures_leds_cdaq);
+ break;
+ default:
+ dev_err(&nirtfeatures->acpi_device->dev,
+ "Unrecognized backplane type %u\n", bpinfo);
+ nirtfeatures->bpstring = "Unknown";
+ break;
+ }
+
+ spin_lock_init(&nirtfeatures->lock);
+
+ nirtfeatures->revision[0] = inb(nirtfeatures->io_base + NIRTF_YEAR);
+ nirtfeatures->revision[1] = inb(nirtfeatures->io_base + NIRTF_MONTH);
+ nirtfeatures->revision[2] = inb(nirtfeatures->io_base + NIRTF_DAY);
+ nirtfeatures->revision[3] = inb(nirtfeatures->io_base + NIRTF_HOUR);
+ nirtfeatures->revision[4] = inb(nirtfeatures->io_base + NIRTF_MINUTE);
+
+ err = nirtfeatures_create_leds(nirtfeatures);
+ if (err) {
+ dev_err(&device->dev, "could not create LEDs\n");
+ return err;
+ }
+
+ err = sysfs_create_files(&device->dev.kobj, nirtfeatures_attrs);
+ if (err) {
+ dev_err(&device->dev, "could not create sysfs attributes\n");
+ return err;
+ }
+
+ dev_dbg(&nirtfeatures->acpi_device->dev,
+ "%s backplane, revision 20%02X/%02X/%02X %02X:%02X, io_base 0x%04X\n",
+ nirtfeatures->bpstring, nirtfeatures->revision[0],
+ nirtfeatures->revision[1], nirtfeatures->revision[2],
+ nirtfeatures->revision[3], nirtfeatures->revision[4],
+ nirtfeatures->io_base);
+
+ return 0;
+}
+
+static int nirtfeatures_acpi_remove(struct acpi_device *device)
+{
+ sysfs_remove_files(&device->dev.kobj, nirtfeatures_attrs);
+
+ return 0;
+}
+
+static const struct acpi_device_id nirtfeatures_device_ids[] = {
+ {"NIC775D", 0},
+ {"", 0},
+};
+
+static struct acpi_driver nirtfeatures_acpi_driver = {
+ .name = MODULE_NAME,
+ .ids = nirtfeatures_device_ids,
+ .ops = {
+ .add = nirtfeatures_acpi_add,
+ .remove = nirtfeatures_acpi_remove,
+ },
+};
+
+module_acpi_driver(nirtfeatures_acpi_driver);
+
+MODULE_DEVICE_TABLE(acpi, nirtfeatures_device_ids);
+MODULE_DESCRIPTION("NI RT Features");
+MODULE_AUTHOR("Jeff Westfahl <jeff.westfahl@ni.com>");
+MODULE_LICENSE("GPL");
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: add nirtfeatures driver
2016-03-14 21:54 [PATCH 1/2] misc: add nirtfeatures driver Kyle Roeschley
@ 2016-03-14 22:05 ` Greg KH
2016-03-14 22:30 ` Josh Cartwright
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2016-03-14 22:05 UTC (permalink / raw)
To: Kyle Roeschley
Cc: arnd, linux-kernel, jeff.westfahl, joshc, nathan.sullivan,
xander.huff
On Mon, Mar 14, 2016 at 04:54:32PM -0500, Kyle Roeschley wrote:
> From: Jeff Westfahl <jeff.westfahl@ni.com>
>
> This driver introduces support for hardware features of National
> Instruments real-time controllers. This is an ACPI device that exposes
> LEDs, switches, and watchdogs.
If it's an acpi driver, why not put it in drivers/acpi? why misc?
Also, please get the acpi driver developers to review this.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: add nirtfeatures driver
2016-03-14 22:05 ` Greg KH
@ 2016-03-14 22:30 ` Josh Cartwright
2016-03-14 22:36 ` Kyle Roeschley
2016-03-16 7:48 ` Lee Jones
0 siblings, 2 replies; 6+ messages in thread
From: Josh Cartwright @ 2016-03-14 22:30 UTC (permalink / raw)
To: Greg KH
Cc: Kyle Roeschley, arnd, linux-kernel, jeff.westfahl,
nathan.sullivan, xander.huff, Lee Jones
[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]
On Mon, Mar 14, 2016 at 03:05:59PM -0700, Greg KH wrote:
> On Mon, Mar 14, 2016 at 04:54:32PM -0500, Kyle Roeschley wrote:
> > From: Jeff Westfahl <jeff.westfahl@ni.com>
> >
> > This driver introduces support for hardware features of National
> > Instruments real-time controllers. This is an ACPI device that exposes
> > LEDs, switches, and watchdogs.
>
> If it's an acpi driver, why not put it in drivers/acpi?
For the same reason we don't move all drivers for devices-on-a-PCI-bus
into drivers/pci?
Drivers typically exist in the sourcetree with other drivers which
implement similar functionality, which works great for devices with
clear functional boundaries (GPIO controller drivers in drivers/gpio,
led drivers in drivers/leds, etc. etc.); but for devices which are a
hodgepodge of functionality, there isn't really a good fit anywhere
except maybe in misc or mfd.
We could move it to mfd, but drivers in drivers/mfd which don't make use
of MFD_CORE seems equally strange (although, I suppose there is
precedent). Maybe Lee has some thoughts.
Josh
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: add nirtfeatures driver
2016-03-14 22:30 ` Josh Cartwright
@ 2016-03-14 22:36 ` Kyle Roeschley
2016-03-16 7:48 ` Lee Jones
1 sibling, 0 replies; 6+ messages in thread
From: Kyle Roeschley @ 2016-03-14 22:36 UTC (permalink / raw)
To: Josh Cartwright
Cc: Greg KH, arnd, linux-kernel, jeff.westfahl, nathan.sullivan,
xander.huff, Lee Jones
On Mon, Mar 14, 2016 at 05:30:22PM -0500, Josh Cartwright wrote:
> On Mon, Mar 14, 2016 at 03:05:59PM -0700, Greg KH wrote:
> > On Mon, Mar 14, 2016 at 04:54:32PM -0500, Kyle Roeschley wrote:
> > > From: Jeff Westfahl <jeff.westfahl@ni.com>
> > >
> > > This driver introduces support for hardware features of National
> > > Instruments real-time controllers. This is an ACPI device that exposes
> > > LEDs, switches, and watchdogs.
> >
> > If it's an acpi driver, why not put it in drivers/acpi?
>
> For the same reason we don't move all drivers for devices-on-a-PCI-bus
> into drivers/pci?
>
> Drivers typically exist in the sourcetree with other drivers which
> implement similar functionality, which works great for devices with
> clear functional boundaries (GPIO controller drivers in drivers/gpio,
> led drivers in drivers/leds, etc. etc.); but for devices which are a
> hodgepodge of functionality, there isn't really a good fit anywhere
> except maybe in misc or mfd.
>
> We could move it to mfd, but drivers in drivers/mfd which don't make use
> of MFD_CORE seems equally strange (although, I suppose there is
> precedent). Maybe Lee has some thoughts.
Maybe drivers/platform? I notice several ACPI drivers there that also have grab
bags of functionality.
Regards,
Kyle Roeschley
National Instruments
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: add nirtfeatures driver
2016-03-14 22:30 ` Josh Cartwright
2016-03-14 22:36 ` Kyle Roeschley
@ 2016-03-16 7:48 ` Lee Jones
2016-03-16 22:30 ` Kyle Roeschley
1 sibling, 1 reply; 6+ messages in thread
From: Lee Jones @ 2016-03-16 7:48 UTC (permalink / raw)
To: Josh Cartwright
Cc: Greg KH, Kyle Roeschley, arnd, linux-kernel, jeff.westfahl,
nathan.sullivan, xander.huff
On Mon, 14 Mar 2016, Josh Cartwright wrote:
> On Mon, Mar 14, 2016 at 03:05:59PM -0700, Greg KH wrote:
> > On Mon, Mar 14, 2016 at 04:54:32PM -0500, Kyle Roeschley wrote:
> > > From: Jeff Westfahl <jeff.westfahl@ni.com>
> > >
> > > This driver introduces support for hardware features of National
> > > Instruments real-time controllers. This is an ACPI device that exposes
> > > LEDs, switches, and watchdogs.
> >
> > If it's an acpi driver, why not put it in drivers/acpi?
>
> For the same reason we don't move all drivers for devices-on-a-PCI-bus
> into drivers/pci?
>
> Drivers typically exist in the sourcetree with other drivers which
> implement similar functionality, which works great for devices with
> clear functional boundaries (GPIO controller drivers in drivers/gpio,
> led drivers in drivers/leds, etc. etc.); but for devices which are a
> hodgepodge of functionality, there isn't really a good fit anywhere
> except maybe in misc or mfd.
>
> We could move it to mfd, but drivers in drivers/mfd which don't make use
> of MFD_CORE seems equally strange (although, I suppose there is
> precedent). Maybe Lee has some thoughts.
Is there any reason why the functionality can't be split up into
different source files? Create an LED driver, a Switch (whatever that
is) driver and a Watchdog driver, place them in drivers/{appropriate},
then register from each of them using the MFD API.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: add nirtfeatures driver
2016-03-16 7:48 ` Lee Jones
@ 2016-03-16 22:30 ` Kyle Roeschley
0 siblings, 0 replies; 6+ messages in thread
From: Kyle Roeschley @ 2016-03-16 22:30 UTC (permalink / raw)
To: Lee Jones
Cc: Josh Cartwright, Greg KH, arnd, linux-kernel, jeff.westfahl,
nathan.sullivan, xander.huff, dvhart
Hi Lee,
On Wed, Mar 16, 2016 at 07:48:24AM +0000, Lee Jones wrote:
> Is there any reason why the functionality can't be split up into
> different source files? Create an LED driver, a Switch (whatever that
> is) driver and a Watchdog driver, place them in drivers/{appropriate},
> then register from each of them using the MFD API.
I goofed a bit in the commit messages; this driver exposes LEDs and buttons
("switches") and gives access to information such as the reason for last system
reset, FPGA boot mode, and hardware revision among other things. The watchdog
was split into another driver and shouldn't have been mentioned.
As for splitting this driver up, I'm not sure I see the necessity. I'm also
unsure as to the distinction between a platform driver and an MFD driver. In
the case of this driver, the CPLD is always and only present on a specific
series of embedded controllers so I think it might be more appropriate to put
it in drivers/platform. CC-ing Darren Hart, x86 platform driver maintainer, for
input and because I forgot to copy him earlier in this chain.
Kyle Roeschley
National Instruments
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-16 22:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-14 21:54 [PATCH 1/2] misc: add nirtfeatures driver Kyle Roeschley
2016-03-14 22:05 ` Greg KH
2016-03-14 22:30 ` Josh Cartwright
2016-03-14 22:36 ` Kyle Roeschley
2016-03-16 7:48 ` Lee Jones
2016-03-16 22:30 ` Kyle Roeschley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox