* [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify.
2012-02-10 6:40 ` [PATCH v5 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
@ 2012-02-10 6:40 ` MyungJoo Ham
2012-02-20 1:54 ` Mark Brown
2012-02-10 6:40 ` [PATCH v5 2/5] Extcon: support notification based on the state changes MyungJoo Ham
` (4 subsequent siblings)
5 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-10 6:40 UTC (permalink / raw)
To: linux-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, Donggeun Kim, Greg KH, Arnd Bergmann,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown,
John Stultz, Joerg Roedel, myungjoo.ham
External connector class (extcon) is based on and an extension of Android
kernel's switch class located at linux/drivers/switch/.
This patch provides the before-extension switch class moved to the
location where the extcon will be located (linux/drivers/extcon/) and
updates to handle class properly.
The before-extension class, switch class of Android kernel, commits
imported are:
switch: switch class and GPIO drivers.
Author: Mike Lockwood <lockwood@android.com>
switch: gpio: Don't call request_irq with interrupts disabled
Author: Arve Hjønnevåg <arve@android.com>
switch: Use device_create instead of device_create_drvdata.
Author: Arve Hjønnevåg <arve@android.com>
switch_gpio: Add missing #include <linux/interrupt.h>
Author: Mike Lockwood <lockwood@android.com>
In this patch, upon the commits of Android kernel, we have added:
- Relocated and renamed for extcon.
- Comments, module name, and author information are updated
- Code clean for successing patches
- Bugfix: enabling write access without write functions
- Class/device/sysfs create/remove handling
- Added comments about uevents
- Format changes for extcon_dev_register() to have a parent dev.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changes from v2
- Updated name_show
- Sysfs entries are handled by class itself.
- Updated the method to add/remove devices for the class
- Comments on uevent send
- Able to become a module
- Compatible with Android platform
Changes from RFC
- Renamed to extcon (external connector) from multistate switch
- Added a seperated directory (drivers/extcon)
- Added kerneldoc comments
- Removed unused variables from extcon_gpio.c
- Added ABI Documentation.
---
Documentation/ABI/testing/sysfs-class-extcon | 26 +++
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/extcon/Kconfig | 22 +++
drivers/extcon/Makefile | 6 +
drivers/extcon/extcon_class.c | 246 ++++++++++++++++++++++++++
drivers/extcon/extcon_gpio.c | 173 ++++++++++++++++++
include/linux/extcon.h | 104 +++++++++++
8 files changed, 580 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-extcon
create mode 100644 drivers/extcon/Kconfig
create mode 100644 drivers/extcon/Makefile
create mode 100644 drivers/extcon/extcon_class.c
create mode 100644 drivers/extcon/extcon_gpio.c
create mode 100644 include/linux/extcon.h
diff --git a/Documentation/ABI/testing/sysfs-class-extcon b/Documentation/ABI/testing/sysfs-class-extcon
new file mode 100644
index 0000000..59a4b1c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-extcon
@@ -0,0 +1,26 @@
+What: /sys/class/extcon/.../
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ Provide a place in sysfs for the extcon objects.
+ This allows accessing extcon specific variables.
+ The name of extcon object denoted as ... is the name given
+ with extcon_dev_register.
+
+What: /sys/class/extcon/.../name
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ The /sys/class/extcon/.../name shows the name of the extcon
+ object. If the extcon object has an optional callback
+ "show_name" defined, the callback will provide the name with
+ this sysfs node.
+
+What: /sys/class/extcon/.../state
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ The /sys/class/extcon/.../state shows the cable attach/detach
+ information of the corresponding extcon object. If the extcon
+ objecct has an optional callback "show_state" defined, the
+ callback will provide the name with this sysfs node.
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 5afe5d1..537d126 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -136,4 +136,6 @@ source "drivers/virt/Kconfig"
source "drivers/devfreq/Kconfig"
+source "drivers/extcon/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index c07be02..6faceab 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -132,3 +132,4 @@ obj-$(CONFIG_VIRT_DRIVERS) += virt/
obj-$(CONFIG_HYPERV) += hv/
obj-$(CONFIG_PM_DEVFREQ) += devfreq/
+obj-$(CONFIG_EXTCON) += extcon/
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
new file mode 100644
index 0000000..3fa3c22
--- /dev/null
+++ b/drivers/extcon/Kconfig
@@ -0,0 +1,22 @@
+menuconfig EXTCON
+ tristate "External Connector Class (extcon) support"
+ help
+ Say Y here to enable external connector class (extcon) support.
+ This allows monitoring external connectors by userspace
+ via sysfs and uevent and supports external connectors with
+ multiple states; i.e., an extcon that may have multiple
+ cables attached. For example, an external connector of a device
+ may be used to connect an HDMI cable and a AC adaptor, and to
+ host USB ports. Many of 30-pin connectors including PDMI are
+ also good examples.
+
+if EXTCON
+
+config EXTCON_GPIO
+ tristate "GPIO extcon support"
+ depends on GENERIC_GPIO
+ help
+ Say Y here to enable GPIO based extcon support. Note that GPIO
+ extcon supports single state per extcon instance.
+
+endif # MULTISTATE_SWITCH
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
new file mode 100644
index 0000000..2c46d41
--- /dev/null
+++ b/drivers/extcon/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for external connector class (extcon) devices
+#
+
+obj-$(CONFIG_EXTCON) += extcon_class.o
+obj-$(CONFIG_EXTCON_GPIO) += extcon_gpio.o
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
new file mode 100644
index 0000000..9d29c79
--- /dev/null
+++ b/drivers/extcon/extcon_class.c
@@ -0,0 +1,246 @@
+/*
+ * drivers/extcon/extcon_class.c
+ *
+ * External connector (extcon) class driver
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Author: Donggeun Kim <dg77.kim@samsung.com>
+ * Author: MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * based on android/drivers/switch/switch_class.c
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/module.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/slab.h>
+
+struct class *extcon_class;
+struct class *extcon_class_for_android;
+
+static ssize_t state_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ if (edev->print_state) {
+ int ret = edev->print_state(edev, buf);
+
+ if (ret >= 0)
+ return ret;
+ /* Use default if failed */
+ }
+ return sprintf(buf, "%u\n", edev->state);
+}
+
+static ssize_t name_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ /* Optional callback given by the user */
+ if (edev->print_name) {
+ int ret = edev->print_name(edev, buf);
+ if (ret >= 0)
+ return ret;
+ }
+
+ return sprintf(buf, "%s\n", dev_name(edev->dev));
+}
+
+/**
+ * extcon_set_state() - Set the cable attach states of the extcon device.
+ * @edev: the extcon device
+ * @state: new cable attach status for @edev
+ *
+ * Changing the state sends uevent with environment variable containing
+ * the name of extcon device (envp[0]) and the state output (envp[1]).
+ * Tizen uses this format for extcon device to get events from ports.
+ * Android uses this format as well.
+ */
+void extcon_set_state(struct extcon_dev *edev, u32 state)
+{
+ char name_buf[120];
+ char state_buf[120];
+ char *prop_buf;
+ char *envp[3];
+ int env_offset = 0;
+ int length;
+
+ if (edev->state != state) {
+ edev->state = state;
+
+ prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
+ if (prop_buf) {
+ length = name_show(edev->dev, NULL, prop_buf);
+ if (length > 0) {
+ if (prop_buf[length - 1] == '\n')
+ prop_buf[length - 1] = 0;
+ snprintf(name_buf, sizeof(name_buf),
+ "SWITCH_NAME=%s", prop_buf);
+ envp[env_offset++] = name_buf;
+ }
+ length = state_show(edev->dev, NULL, prop_buf);
+ if (length > 0) {
+ if (prop_buf[length - 1] == '\n')
+ prop_buf[length - 1] = 0;
+ snprintf(state_buf, sizeof(state_buf),
+ "SWITCH_STATE=%s", prop_buf);
+ envp[env_offset++] = state_buf;
+ }
+ envp[env_offset] = NULL;
+ kobject_uevent_env(&edev->dev->kobj, KOBJ_CHANGE, envp);
+ free_page((unsigned long)prop_buf);
+ } else {
+ dev_err(edev->dev, "out of memory in extcon_set_state\n");
+ kobject_uevent(&edev->dev->kobj, KOBJ_CHANGE);
+ }
+ }
+}
+EXPORT_SYMBOL_GPL(extcon_set_state);
+
+static struct device_attribute extcon_attrs[] = {
+ __ATTR_RO(state),
+ __ATTR_RO(name),
+};
+
+static int create_extcon_class(void)
+{
+ if (!extcon_class) {
+ extcon_class = class_create(THIS_MODULE, "extcon");
+ if (IS_ERR(extcon_class))
+ return PTR_ERR(extcon_class);
+ extcon_class->dev_attrs = extcon_attrs;
+ }
+
+ return 0;
+}
+
+static int create_extcon_class_for_android(void)
+{
+ if (!extcon_class_for_android) {
+ extcon_class_for_android = class_create(THIS_MODULE, "switch");
+ if (IS_ERR(extcon_class_for_android))
+ return PTR_ERR(extcon_class_for_android);
+ extcon_class_for_android->dev_attrs = extcon_attrs;
+ }
+ return 0;
+}
+
+static void extcon_cleanup(struct extcon_dev *edev, bool skip)
+{
+ if (!skip && get_device(edev->dev)) {
+ device_unregister(edev->dev);
+ put_device(edev->dev);
+ }
+
+ kfree(edev->dev);
+}
+
+static void extcon_dev_release(struct device *dev)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ extcon_cleanup(edev, true);
+}
+
+/**
+ * extcon_dev_register() - Register a new extcon device
+ * @edev : the new extcon device (should be allocated before calling)
+ * @dev : the parent device for this extcon device.
+ *
+ * Among the members of edev struct, please set the "user initializing data"
+ * in any case and set the "optional callbacks" if required. However, please
+ * do not set the values of "internal data", which are initialized by
+ * this function.
+ */
+int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
+{
+ int ret;
+
+ if (!extcon_class && !edev->use_class_name_switch) {
+ ret = create_extcon_class();
+ if (ret < 0)
+ return ret;
+ }
+ if (!extcon_class_for_android && edev->use_class_name_switch) {
+ ret = create_extcon_class_for_android();
+ if (ret < 0)
+ return ret;
+ }
+
+ edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+ edev->dev->parent = dev;
+ if (edev->use_class_name_switch)
+ edev->dev->class = extcon_class_for_android;
+ else
+ edev->dev->class = extcon_class;
+ edev->dev->release = extcon_dev_release;
+
+ dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+ ret = device_register(edev->dev);
+ if (ret) {
+ put_device(edev->dev);
+ goto err_dev;
+ }
+
+ dev_set_drvdata(edev->dev, edev);
+ edev->state = 0;
+ return 0;
+
+err_dev:
+ kfree(edev->dev);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(extcon_dev_register);
+
+/**
+ * extcon_dev_unregister() - Unregister the extcon device.
+ * @edev: the extcon device instance to be unregitered.
+ *
+ * Note that this does not call kfree(edev) because edev was not allocated
+ * by this class.
+ */
+void extcon_dev_unregister(struct extcon_dev *edev)
+{
+ extcon_cleanup(edev, false);
+}
+EXPORT_SYMBOL_GPL(extcon_dev_unregister);
+
+static int __init extcon_class_init(void)
+{
+ return create_extcon_class();
+}
+
+static void __exit extcon_class_exit(void)
+{
+ class_destroy(extcon_class);
+
+ if (extcon_class_for_android)
+ class_destroy(extcon_class_for_android);
+}
+
+module_init(extcon_class_init);
+module_exit(extcon_class_exit);
+
+MODULE_AUTHOR("Mike Lockwood <lockwood@android.com>");
+MODULE_AUTHOR("Donggeun Kim <dg77.kim@samsung.com>");
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_DESCRIPTION("External connector (extcon) class driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/extcon/extcon_gpio.c b/drivers/extcon/extcon_gpio.c
new file mode 100644
index 0000000..7f4bb63
--- /dev/null
+++ b/drivers/extcon/extcon_gpio.c
@@ -0,0 +1,173 @@
+/*
+ * drivers/extcon/extcon_gpio.c
+ *
+ * Single-state GPIO extcon driver based on extcon class
+ *
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * Modified by MyungJoo Ham <myungjoo.ham@samsung.com> to support extcon
+ * (originally switch class is supported)
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/extcon.h>
+#include <linux/workqueue.h>
+#include <linux/gpio.h>
+
+struct gpio_extcon_data {
+ struct extcon_dev edev;
+ unsigned gpio;
+ const char *state_on;
+ const char *state_off;
+ int irq;
+ struct work_struct work;
+};
+
+static void gpio_extcon_work(struct work_struct *work)
+{
+ int state;
+ struct gpio_extcon_data *data =
+ container_of(work, struct gpio_extcon_data, work);
+
+ state = gpio_get_value(data->gpio);
+ extcon_set_state(&data->edev, state);
+}
+
+static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
+{
+ struct gpio_extcon_data *extcon_data =
+ (struct gpio_extcon_data *)dev_id;
+
+ schedule_work(&extcon_data->work);
+ return IRQ_HANDLED;
+}
+
+static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
+{
+ struct gpio_extcon_data *extcon_data =
+ container_of(edev, struct gpio_extcon_data, edev);
+ const char *state;
+ if (extcon_get_state(edev))
+ state = extcon_data->state_on;
+ else
+ state = extcon_data->state_off;
+
+ if (state)
+ return sprintf(buf, "%s\n", state);
+ return -1;
+}
+
+static int gpio_extcon_probe(struct platform_device *pdev)
+{
+ struct gpio_extcon_platform_data *pdata = pdev->dev.platform_data;
+ struct gpio_extcon_data *extcon_data;
+ int ret = 0;
+
+ if (!pdata)
+ return -EBUSY;
+
+ extcon_data = kzalloc(sizeof(struct gpio_extcon_data), GFP_KERNEL);
+ if (!extcon_data)
+ return -ENOMEM;
+
+ extcon_data->edev.name = pdata->name;
+ extcon_data->gpio = pdata->gpio;
+ extcon_data->state_on = pdata->state_on;
+ extcon_data->state_off = pdata->state_off;
+ extcon_data->edev.print_state = extcon_gpio_print_state;
+
+ ret = extcon_dev_register(&extcon_data->edev, &pdev->dev);
+ if (ret < 0)
+ goto err_extcon_dev_register;
+
+ ret = gpio_request(extcon_data->gpio, pdev->name);
+ if (ret < 0)
+ goto err_request_gpio;
+
+ ret = gpio_direction_input(extcon_data->gpio);
+ if (ret < 0)
+ goto err_set_gpio_input;
+
+ INIT_WORK(&extcon_data->work, gpio_extcon_work);
+
+ extcon_data->irq = gpio_to_irq(extcon_data->gpio);
+ if (extcon_data->irq < 0) {
+ ret = extcon_data->irq;
+ goto err_detect_irq_num_failed;
+ }
+
+ ret = request_irq(extcon_data->irq, gpio_irq_handler,
+ IRQF_TRIGGER_LOW, pdev->name, extcon_data);
+ if (ret < 0)
+ goto err_request_irq;
+
+ /* Perform initial detection */
+ gpio_extcon_work(&extcon_data->work);
+
+ return 0;
+
+err_request_irq:
+err_detect_irq_num_failed:
+err_set_gpio_input:
+ gpio_free(extcon_data->gpio);
+err_request_gpio:
+ extcon_dev_unregister(&extcon_data->edev);
+err_extcon_dev_register:
+ kfree(extcon_data);
+
+ return ret;
+}
+
+static int __devexit gpio_extcon_remove(struct platform_device *pdev)
+{
+ struct gpio_extcon_data *extcon_data = platform_get_drvdata(pdev);
+
+ cancel_work_sync(&extcon_data->work);
+ gpio_free(extcon_data->gpio);
+ extcon_dev_unregister(&extcon_data->edev);
+ kfree(extcon_data);
+
+ return 0;
+}
+
+static struct platform_driver gpio_extcon_driver = {
+ .probe = gpio_extcon_probe,
+ .remove = __devexit_p(gpio_extcon_remove),
+ .driver = {
+ .name = "extcon-gpio",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init gpio_extcon_init(void)
+{
+ return platform_driver_register(&gpio_extcon_driver);
+}
+
+static void __exit gpio_extcon_exit(void)
+{
+ platform_driver_unregister(&gpio_extcon_driver);
+}
+
+module_init(gpio_extcon_init);
+module_exit(gpio_extcon_exit);
+
+MODULE_AUTHOR("Mike Lockwood <lockwood@android.com>");
+MODULE_DESCRIPTION("GPIO extcon driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
new file mode 100644
index 0000000..da0bbc8
--- /dev/null
+++ b/include/linux/extcon.h
@@ -0,0 +1,104 @@
+/*
+ * External connector (extcon) class driver
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Author: Donggeun Kim <dg77.kim@samsung.com>
+ * Author: MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * based on switch class driver
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+*/
+
+#ifndef __LINUX_EXTCON_H__
+#define __LINUX_EXTCON_H__
+
+/**
+ * struct extcon_dev - An extcon device represents one external connector.
+ * @name The name of this extcon device. Parent device name is used
+ * if NULL.
+ * @use_class_name_switch set true in order to be compatible with
+ * Android platform, which uses "switch"
+ * for the class name.
+ * @print_name An optional callback to override the method to print the
+ * name of the extcon device.
+ * @print_state An optional callback to override the method to print the
+ * status of the extcon device.
+ * @dev Device of this extcon. Do not provide at register-time.
+ * @state Attach/detach state of this extcon. Do not provide at
+ * register-time
+ *
+ * In most cases, users only need to provide "User initializing data" of
+ * this struct when registering an extcon. In some exceptional cases,
+ * optional callbacks may be needed. However, the values in "internal data"
+ * are overwritten by register function.
+ */
+struct extcon_dev {
+ /* --- Optional user initializing data --- */
+ const char *name;
+ bool use_class_name_switch;
+
+ /* --- Optional callbacks to override class functions --- */
+ ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
+ ssize_t (*print_state)(struct extcon_dev *edev, char *buf);
+
+ /* --- Internal data. Please do not set. --- */
+ struct device *dev;
+ u32 state;
+};
+
+/**
+ * struct gpio_extcon_platform_data - A simple GPIO-controlled extcon device.
+ * @name The name of this GPIO extcon device.
+ * @gpio Corresponding GPIO.
+ * @state_on print_state is overriden with state_on if attached. If Null,
+ * default method of extcon class is used.
+ * @state_off print_state is overriden with state_on if dettached. If Null,
+ * default method of extcon class is used.
+ */
+struct gpio_extcon_platform_data {
+ const char *name;
+ unsigned gpio;
+
+ /* if NULL, "0" or "1" will be printed */
+ const char *state_on;
+ const char *state_off;
+};
+
+#ifdef CONFIG_EXTCON
+extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
+extern void extcon_dev_unregister(struct extcon_dev *edev);
+
+static inline u32 extcon_get_state(struct extcon_dev *edev)
+{
+ return edev->state;
+}
+
+extern void extcon_set_state(struct extcon_dev *edev, u32 state);
+#else /* CONFIG_EXTCON */
+static inline int extcon_dev_register(struct extcon_dev *edev,
+ struct device *dev)
+{
+ return 0;
+}
+
+static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
+
+static inline u32 extcon_get_state(struct extcon_dev *edev)
+{
+ return 0;
+}
+
+static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+#endif /* CONFIG_EXTCON */
+#endif /* __LINUX_EXTCON_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify.
2012-02-10 6:40 ` [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
@ 2012-02-20 1:54 ` Mark Brown
2012-02-20 6:17 ` MyungJoo Ham
0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-02-20 1:54 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel, myungjoo.ham
[-- Attachment #1: Type: text/plain, Size: 4193 bytes --]
On Fri, Feb 10, 2012 at 03:40:34PM +0900, MyungJoo Ham wrote:
> External connector class (extcon) is based on and an extension of Android
> kernel's switch class located at linux/drivers/switch/.
This looks good though I've skipped some bits as it's taken me far too
long to get round to reviewing, it'd be really good if we could get it
into 3.4 at least in staging if not in fully. I don't know if arm-soc
might be a good route if there's some concerns? A few things below but
they're relatively minor.
One thing I'd suggest is splitting the GPIO implementation into a
separate patch, mostly just to reduce the size of the initial patch for
ease of review.
> + if (edev->state != state) {
> + edev->state = state;
> +
> + prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
> + if (prop_buf) {
Is the cast really needed here?
> +static int create_extcon_class(void)
> +{
> + if (!extcon_class) {
> + extcon_class = class_create(THIS_MODULE, "extcon");
> + if (IS_ERR(extcon_class))
> + return PTR_ERR(extcon_class);
> + extcon_class->dev_attrs = extcon_attrs;
I thought we were trying to remove classes, though I'm not sure if we're
actually at the point where that's happening yet? Greg?
> +static int create_extcon_class_for_android(void)
> +{
> + if (!extcon_class_for_android) {
> + extcon_class_for_android = class_create(THIS_MODULE, "switch");
> + if (IS_ERR(extcon_class_for_android))
> + return PTR_ERR(extcon_class_for_android);
> + extcon_class_for_android->dev_attrs = extcon_attrs;
> + }
> + return 0;
> +}
Might be better to put this as a separate Kconfig option or just leave
it as an out of tree patch (given how trivial it is). We're going to
end up renaming a bunch of the classes anyway I expect...
> +static int __init extcon_class_init(void)
> +{
> + return create_extcon_class();
> +}
> +
> +static void __exit extcon_class_exit(void)
> +{
> + class_destroy(extcon_class);
> +
> + if (extcon_class_for_android)
> + class_destroy(extcon_class_for_android);
> +}
> +
> +module_init(extcon_class_init);
> +module_exit(extcon_class_exit);
Ideally these should go next to the functions.
> +static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
> +{
> + struct gpio_extcon_data *extcon_data =
> + (struct gpio_extcon_data *)dev_id;
> +
> + schedule_work(&extcon_data->work);
> + return IRQ_HANDLED;
> +}
Given that all this does is schedule some work it'd seem useful to make
this a threaded IRQ and just do the work directly in the interrupt
handler. Though on the other hand we don't have any debounce here so
perhaps it's even better to allow the user to specify a debunce time in
the platform data and change this to schedule_delayed_work() to
implement it?
> +static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
> +{
> + struct gpio_extcon_data *extcon_data =
> + container_of(edev, struct gpio_extcon_data, edev);
> + const char *state;
> + if (extcon_get_state(edev))
> + state = extcon_data->state_on;
> + else
> + state = extcon_data->state_off;
> +
> + if (state)
> + return sprintf(buf, "%s\n", state);
> + return -1;
-EINVAL or something?
> + extcon_data = kzalloc(sizeof(struct gpio_extcon_data), GFP_KERNEL);
> + if (!extcon_data)
> + return -ENOMEM;
devm_kzalloc().
> + ret = request_irq(extcon_data->irq, gpio_irq_handler,
> + IRQF_TRIGGER_LOW, pdev->name, extcon_data);
> + if (ret < 0)
> + goto err_request_irq;
request_any_context_irq() would allow use with any GPIO - sometimes the
GPIOs for accessory detection are on GPIO expanders which need threaded
context and there's nothing in the code that minds. It would also be a
good idea if the user could specify the triggers, lots of circuits need
edge triggers for example.
> +static int __init gpio_extcon_init(void)
> +{
> + return platform_driver_register(&gpio_extcon_driver);
> +}
> +
> +static void __exit gpio_extcon_exit(void)
> +{
> + platform_driver_unregister(&gpio_extcon_driver);
> +}
> +
> +module_init(gpio_extcon_init);
> +module_exit(gpio_extcon_exit);
module_platform_driver().
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify.
2012-02-20 1:54 ` Mark Brown
@ 2012-02-20 6:17 ` MyungJoo Ham
2012-02-20 15:45 ` Mark Brown
0 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-20 6:17 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
On Mon, Feb 20, 2012 at 10:54 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Feb 10, 2012 at 03:40:34PM +0900, MyungJoo Ham wrote:
>> External connector class (extcon) is based on and an extension of Android
>> kernel's switch class located at linux/drivers/switch/.
>
> This looks good though I've skipped some bits as it's taken me far too
> long to get round to reviewing, it'd be really good if we could get it
> into 3.4 at least in staging if not in fully. I don't know if arm-soc
> might be a good route if there's some concerns? A few things below but
> they're relatively minor.
Yeah. I guess arm-soc would be fine. I'll send thru arm-soc as well next time.
>
> One thing I'd suggest is splitting the GPIO implementation into a
> separate patch, mostly just to reduce the size of the initial patch for
> ease of review.
Ok, I've splitted gpio implementation for the next iteration.
>
>> + if (edev->state != state) {
>> + edev->state = state;
>> +
>> + prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
>> + if (prop_buf) {
>
> Is the cast really needed here?
Unless we have that cast, we get:
drivers/extcon/extcon_class.c:89:12: warning: assignment makes pointer
from integer without a cast
>
>> +static int create_extcon_class(void)
>> +{
>> + if (!extcon_class) {
>> + extcon_class = class_create(THIS_MODULE, "extcon");
>> + if (IS_ERR(extcon_class))
>> + return PTR_ERR(extcon_class);
>> + extcon_class->dev_attrs = extcon_attrs;
>
> I thought we were trying to remove classes, though I'm not sure if we're
> actually at the point where that's happening yet? Greg?
>
Hmm.. I remember I was recommended to use classes some time ago (just
a few months ago) especially for adding sysfs entries. Things have
been changed already?
>> +static int create_extcon_class_for_android(void)
>> +{
>> + if (!extcon_class_for_android) {
>> + extcon_class_for_android = class_create(THIS_MODULE, "switch");
>> + if (IS_ERR(extcon_class_for_android))
>> + return PTR_ERR(extcon_class_for_android);
>> + extcon_class_for_android->dev_attrs = extcon_attrs;
>> + }
>> + return 0;
>> +}
>
> Might be better to put this as a separate Kconfig option or just leave
> it as an out of tree patch (given how trivial it is). We're going to
> end up renaming a bunch of the classes anyway I expect...
Then, would it be proper to put "for-android" features surrounded by
#ifdef CONFIG_ANDROID ?
>
>> +static int __init extcon_class_init(void)
>> +{
>> + return create_extcon_class();
>> +}
>> +
>> +static void __exit extcon_class_exit(void)
>> +{
>> + class_destroy(extcon_class);
>> +
>> + if (extcon_class_for_android)
>> + class_destroy(extcon_class_for_android);
>> +}
>> +
>> +module_init(extcon_class_init);
>> +module_exit(extcon_class_exit);
>
> Ideally these should go next to the functions.
Yes..
>
>> +static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
>> +{
>> + struct gpio_extcon_data *extcon_data =
>> + (struct gpio_extcon_data *)dev_id;
>> +
>> + schedule_work(&extcon_data->work);
>> + return IRQ_HANDLED;
>> +}
>
> Given that all this does is schedule some work it'd seem useful to make
> this a threaded IRQ and just do the work directly in the interrupt
> handler. Though on the other hand we don't have any debounce here so
> perhaps it's even better to allow the user to specify a debunce time in
> the platform data and change this to schedule_delayed_work() to
> implement it?
I looks like adding a debounce time would be useful. I'll let it use
delayed_work.
I'll do the same for adc_jack, too, though I'm thinking about
submitting adc_jack later seperatedly from this patchset.
>
>> +static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
>> +{
>> + struct gpio_extcon_data *extcon_data =
>> + container_of(edev, struct gpio_extcon_data, edev);
>> + const char *state;
>> + if (extcon_get_state(edev))
>> + state = extcon_data->state_on;
>> + else
>> + state = extcon_data->state_off;
>> +
>> + if (state)
>> + return sprintf(buf, "%s\n", state);
>> + return -1;
>
> -EINVAL or something?
I'll use -EINVAL and add NULL check at probe function.
>
>> + extcon_data = kzalloc(sizeof(struct gpio_extcon_data), GFP_KERNEL);
>> + if (!extcon_data)
>> + return -ENOMEM;
>
> devm_kzalloc().
I'll try devm_kzalloc and devm_kfree.
>
>> + ret = request_irq(extcon_data->irq, gpio_irq_handler,
>> + IRQF_TRIGGER_LOW, pdev->name, extcon_data);
>> + if (ret < 0)
>> + goto err_request_irq;
>
> request_any_context_irq() would allow use with any GPIO - sometimes the
> GPIOs for accessory detection are on GPIO expanders which need threaded
> context and there's nothing in the code that minds. It would also be a
> good idea if the user could specify the triggers, lots of circuits need
> edge triggers for example.
Letting users specify flags looks much better than fixing the flag as
IRQF_TRIGGER_LOW. And I'll replease request_irq with
request_any_context_irq as you've mentioned.
>
>> +static int __init gpio_extcon_init(void)
>> +{
>> + return platform_driver_register(&gpio_extcon_driver);
>> +}
>> +
>> +static void __exit gpio_extcon_exit(void)
>> +{
>> + platform_driver_unregister(&gpio_extcon_driver);
>> +}
>> +
>> +module_init(gpio_extcon_init);
>> +module_exit(gpio_extcon_exit);
>
> module_platform_driver().
Oh.. yes, another modern idiom. :)
Thanks so much!
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify.
2012-02-20 6:17 ` MyungJoo Ham
@ 2012-02-20 15:45 ` Mark Brown
0 siblings, 0 replies; 50+ messages in thread
From: Mark Brown @ 2012-02-20 15:45 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]
On Mon, Feb 20, 2012 at 03:17:39PM +0900, MyungJoo Ham wrote:
> On Mon, Feb 20, 2012 at 10:54 AM, Mark Brown
> >> + prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
> >> + if (prop_buf) {
> > Is the cast really needed here?
> Unless we have that cast, we get:
> drivers/extcon/extcon_class.c:89:12: warning: assignment makes pointer
> from integer without a cast
Oh, dear. That's a very sad API but it looks like your code is OK.
> >> + extcon_class->dev_attrs = extcon_attrs;
> > I thought we were trying to remove classes, though I'm not sure if we're
> > actually at the point where that's happening yet? Greg?
> Hmm.. I remember I was recommended to use classes some time ago (just
> a few months ago) especially for adding sysfs entries. Things have
> been changed already?
There's class like buses but they're not actually classes - I'm not sure
if the in kernel API has changed or not, though. Like I say Greg's the
expert.
> >> +static int create_extcon_class_for_android(void)
> >> +{
> >> + if (!extcon_class_for_android) {
> >> + extcon_class_for_android = class_create(THIS_MODULE, "switch");
> >> + if (IS_ERR(extcon_class_for_android))
> >> + return PTR_ERR(extcon_class_for_android);
> >> + extcon_class_for_android->dev_attrs = extcon_attrs;
> >> + }
> >> + return 0;
> >> +}
> > Might be better to put this as a separate Kconfig option or just leave
> > it as an out of tree patch (given how trivial it is). We're going to
> > end up renaming a bunch of the classes anyway I expect...
> Then, would it be proper to put "for-android" features surrounded by
> #ifdef CONFIG_ANDROID ?
That's an idea. Though I'm pretty sure that for a simple change like
this the Android userspace could just adapt and we will be changing some
of the names of the directory entries I expect (I can't imagine we're
going to keep h2w for example) so the rename would likely be useful to
them.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v5 2/5] Extcon: support notification based on the state changes.
2012-02-10 6:40 ` [PATCH v5 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
2012-02-10 6:40 ` [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
@ 2012-02-10 6:40 ` MyungJoo Ham
2012-02-20 2:20 ` Mark Brown
2012-02-10 6:40 ` [PATCH v5 3/5] Extcon: support multiple states at a device MyungJoo Ham
` (3 subsequent siblings)
5 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-10 6:40 UTC (permalink / raw)
To: linux-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, Donggeun Kim, Greg KH, Arnd Bergmann,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown,
John Stultz, Joerg Roedel, myungjoo.ham
From: Donggeun Kim <dg77.kim@samsung.com>
State changes of extcon devices have been notified via kobjet_uevent.
This patch adds notifier interfaces in order to allow device drivers to
get notified easily. Along with notifier interface,
extcon_get_extcon_dev() function is added so that device drivers may
discover a extcon_dev easily.
Signed-off-by: Donggeun Kim <dg77.kim@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changes from RFC
- Renamed switch to extcon
- Bugfix: extcon_dev_unregister()
- Bugfix: "edev->dev" is "internal" data.
- Added kerneldoc comments.
- Reworded comments.
---
drivers/extcon/extcon_class.c | 66 +++++++++++++++++++++++++++++++++++++++++
include/linux/extcon.h | 38 +++++++++++++++++++++++
2 files changed, 104 insertions(+), 0 deletions(-)
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
index 9d29c79..763cf4d 100644
--- a/drivers/extcon/extcon_class.c
+++ b/drivers/extcon/extcon_class.c
@@ -34,6 +34,9 @@
struct class *extcon_class;
struct class *extcon_class_for_android;
+static LIST_HEAD(extcon_dev_list);
+static DEFINE_MUTEX(extcon_dev_list_lock);
+
static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -73,6 +76,9 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
* the name of extcon device (envp[0]) and the state output (envp[1]).
* Tizen uses this format for extcon device to get events from ports.
* Android uses this format as well.
+ *
+ * Note that notifier provides the which bits are changes in the state
+ * variable with "val" to the callback.
*/
void extcon_set_state(struct extcon_dev *edev, u32 state)
{
@@ -82,10 +88,14 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
char *envp[3];
int env_offset = 0;
int length;
+ u32 old_state = edev->state;
if (edev->state != state) {
edev->state = state;
+ raw_notifier_call_chain(&edev->nh, old_state ^ edev->state,
+ edev);
+
prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
if (prop_buf) {
length = name_show(edev->dev, NULL, prop_buf);
@@ -115,6 +125,51 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
}
EXPORT_SYMBOL_GPL(extcon_set_state);
+/**
+ * extcon_get_extcon_dev() - Get the extcon device instance from the name
+ * @extcon_name: The extcon name provided with extcon_dev_register()
+ */
+struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
+{
+ struct extcon_dev *sd;
+
+ mutex_lock(&extcon_dev_list_lock);
+ list_for_each_entry(sd, &extcon_dev_list, entry) {
+ if (!strcmp(sd->name, extcon_name))
+ goto out;
+ }
+ sd = NULL;
+out:
+ mutex_unlock(&extcon_dev_list_lock);
+ return sd;
+}
+EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
+
+/**
+ * extcon_register_notifier() - Register a notifee to get notified by
+ * any attach status changes from the extcon.
+ * @edev: the extcon device.
+ * @nb: a notifier block to be registered.
+ */
+int extcon_register_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return raw_notifier_chain_register(&edev->nh, nb);
+}
+EXPORT_SYMBOL_GPL(extcon_register_notifier);
+
+/**
+ * extcon_unregister_notifier() - Unregister a notifee from the extcon device.
+ * @edev: the extcon device.
+ * @nb: a registered notifier block to be unregistered.
+ */
+int extcon_unregister_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return raw_notifier_chain_unregister(&edev->nh, nb);
+}
+EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+
static struct device_attribute extcon_attrs[] = {
__ATTR_RO(state),
__ATTR_RO(name),
@@ -145,6 +200,10 @@ static int create_extcon_class_for_android(void)
static void extcon_cleanup(struct extcon_dev *edev, bool skip)
{
+ mutex_lock(&extcon_dev_list_lock);
+ list_del(&edev->entry);
+ mutex_unlock(&extcon_dev_list_lock);
+
if (!skip && get_device(edev->dev)) {
device_unregister(edev->dev);
put_device(edev->dev);
@@ -200,8 +259,15 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
goto err_dev;
}
+ RAW_INIT_NOTIFIER_HEAD(&edev->nh);
+
dev_set_drvdata(edev->dev, edev);
edev->state = 0;
+
+ mutex_lock(&extcon_dev_list_lock);
+ list_add(&edev->entry, &extcon_dev_list);
+ mutex_unlock(&extcon_dev_list_lock);
+
return 0;
err_dev:
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index da0bbc8..1e47e3b 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -23,6 +23,7 @@
#ifndef __LINUX_EXTCON_H__
#define __LINUX_EXTCON_H__
+#include <linux/notifier.h>
/**
* struct extcon_dev - An extcon device represents one external connector.
* @name The name of this extcon device. Parent device name is used
@@ -37,6 +38,9 @@
* @dev Device of this extcon. Do not provide at register-time.
* @state Attach/detach state of this extcon. Do not provide at
* register-time
+ * @nh Notifier for the state change events from this extcon
+ * @entry To support list of extcon devices so that uses can search
+ * for extcon devices based on the extcon name.
*
* In most cases, users only need to provide "User initializing data" of
* this struct when registering an extcon. In some exceptional cases,
@@ -55,6 +59,8 @@ struct extcon_dev {
/* --- Internal data. Please do not set. --- */
struct device *dev;
u32 state;
+ struct raw_notifier_head nh;
+ struct list_head entry;
};
/**
@@ -76,8 +82,14 @@ struct gpio_extcon_platform_data {
};
#ifdef CONFIG_EXTCON
+
+/*
+ * Following APIs are for notifiers or configurations.
+ * Notifiers are the external port and connection devices.
+ */
extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
extern void extcon_dev_unregister(struct extcon_dev *edev);
+extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
static inline u32 extcon_get_state(struct extcon_dev *edev)
{
@@ -85,6 +97,15 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
}
extern void extcon_set_state(struct extcon_dev *edev, u32 state);
+
+/*
+ * Following APIs are to monitor every action of a notifier.
+ * Registerer gets notified for every external port of a connection device.
+ */
+extern int extcon_register_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb);
+extern int extcon_unregister_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb);
#else /* CONFIG_EXTCON */
static inline int extcon_dev_register(struct extcon_dev *edev,
struct device *dev)
@@ -100,5 +121,22 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
}
static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
+{
+ return NULL;
+}
+
+static inline int extcon_register_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int extcon_unregister_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
#endif /* CONFIG_EXTCON */
#endif /* __LINUX_EXTCON_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v5 2/5] Extcon: support notification based on the state changes.
2012-02-10 6:40 ` [PATCH v5 2/5] Extcon: support notification based on the state changes MyungJoo Ham
@ 2012-02-20 2:20 ` Mark Brown
0 siblings, 0 replies; 50+ messages in thread
From: Mark Brown @ 2012-02-20 2:20 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel, myungjoo.ham
[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]
On Fri, Feb 10, 2012 at 03:40:35PM +0900, MyungJoo Ham wrote:
> From: Donggeun Kim <dg77.kim@samsung.com>
> State changes of extcon devices have been notified via kobjet_uevent.
> This patch adds notifier interfaces in order to allow device drivers to
> get notified easily. Along with notifier interface,
> extcon_get_extcon_dev() function is added so that device drivers may
> discover a extcon_dev easily.
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
This is something I'd been going to ask for. One related thing which
needs some thought is how we combine extcon detection methods for
hardware which uses multiple unrelated detection methods. One of the
common examples for audio jacks is that you might have an ultra low
power detection method for detecting if something is physically present
in the jack and then use that to start up additional detection methods
that can tell what that thing actually is. The two provide different
pieces of the picture reported to userspace.
With the ALSA and input accessory reporting methods we've got bitmasks
so this can be implemented fairly naturally but with the ABI that we've
got from Android here things aren't so easy as we need to go down to a
single value. Perhaps some sort of bitmask to value translation layer?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v5 3/5] Extcon: support multiple states at a device.
2012-02-10 6:40 ` [PATCH v5 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
2012-02-10 6:40 ` [PATCH v5 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
2012-02-10 6:40 ` [PATCH v5 2/5] Extcon: support notification based on the state changes MyungJoo Ham
@ 2012-02-10 6:40 ` MyungJoo Ham
2012-02-20 2:24 ` Mark Brown
2012-02-22 10:07 ` Arnd Bergmann
2012-02-10 6:40 ` [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables MyungJoo Ham
` (2 subsequent siblings)
5 siblings, 2 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-10 6:40 UTC (permalink / raw)
To: linux-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, Donggeun Kim, Greg KH, Arnd Bergmann,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown,
John Stultz, Joerg Roedel, myungjoo.ham
One switch device (e.g., MUIC(MAX8997, MAX77686, ...), and some 30-pin
devices) may have multiple cables attached. For example, one
30-pin port may inhabit a USB cable, an HDMI cable, and a mic.
Thus, one switch device requires multiple state bits each representing
a type of cable.
For such purpose, we use the 32bit state variable; thus, up to 32
different type of cables may be defined for a switch device. The list of
possible cables is defined by the array of cable names in the switch_dev
struct given to the class.
Signed-off-by: Donggeun Kim <dg77.kim@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changes from V4
- Bugfixes after more testing at Exynos4412 boards with userspace
processses.
Changes from V3
- Bugfixes after more testing at Exynos4412 boards.
Changes from V2
- State can be stored by user
- Documentation updated
Changes from RFC
- Switch is renamed to extcon
- Added kerneldoc comments
- Added APIs to support "standard" cable names
- Added helper APIs to support notifier block registration with cable
name.
- Regrouped function list in the header file.
Extcon: extcon framework bugfix
This patch fixes the following issues:
1. NULL pointer error due to device_attribute
2. The message of 'BUG: sleeping function called from invalid context ...'
is shown when calling get_zeroed_page in spin locked
3. Handling old state had bug that not properly interpretting the
semantics of values.
4. Incorrect sysfs node format (state)
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
Documentation/ABI/testing/sysfs-class-extcon | 38 +++-
drivers/extcon/extcon_class.c | 310 ++++++++++++++++++++++++-
include/linux/extcon.h | 151 +++++++++++++
3 files changed, 482 insertions(+), 17 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-extcon b/Documentation/ABI/testing/sysfs-class-extcon
index 59a4b1c..31bb609 100644
--- a/Documentation/ABI/testing/sysfs-class-extcon
+++ b/Documentation/ABI/testing/sysfs-class-extcon
@@ -7,6 +7,14 @@ Description:
The name of extcon object denoted as ... is the name given
with extcon_dev_register.
+ One extcon device denotes a single external connector
+ port. An external connector may have multiple cables
+ attached simultaneously. Many of docks, cradles, and
+ accessory cables have such capability. For example,
+ the 30-pin port of Nuri board (/arch/arm/mach-exynos)
+ may have both HDMI and Charger attached, or analog audio,
+ video, and USB cables attached simulteneously.
+
What: /sys/class/extcon/.../name
Date: December 2011
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
@@ -20,7 +28,29 @@ What: /sys/class/extcon/.../state
Date: December 2011
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
Description:
- The /sys/class/extcon/.../state shows the cable attach/detach
- information of the corresponding extcon object. If the extcon
- objecct has an optional callback "show_state" defined, the
- callback will provide the name with this sysfs node.
+ The /sys/class/extcon/.../state shows and stores the cable
+ attach/detach information of the corresponding extcon object.
+ If the extcon objecct has an optional callback "show_state"
+ defined, the showing function is overriden with the optional
+ callback.
+
+ If the default callback for showing function is used, the
+ format is like this:
+ # cat state
+ USB_OTG=1
+ HDMI=0
+ TA=1
+ EAR_JACK=0
+ #
+ In this example, the extcon device have USB_OTG and TA
+ cables attached and HDMI and EAR_JACK cables detached.
+
+ There are two methods to update state of extcon.
+ Method 1.
+ echo CABLE_NAME [0|1] > state
+ Method 2.
+ echo 0xHEX > state
+
+ Method 1 updates the state (0 or 1) of the
+ corresponding cable (either the name or index of the cable).
+ Method 2 updates the whole state of the extcon dev.
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
index 763cf4d..2803bd0 100644
--- a/drivers/extcon/extcon_class.c
+++ b/drivers/extcon/extcon_class.c
@@ -31,6 +31,25 @@
#include <linux/extcon.h>
#include <linux/slab.h>
+const char *extcon_cable_name[] = {
+ [EXTCON_USB] = "USB",
+ [EXTCON_USB_HOST] = "USB-Host",
+ [EXTCON_TA] = "TA",
+ [EXTCON_FAST_CHARGER] = "Fast-charger",
+ [EXTCON_SLOW_CHARGER] = "Slow-charger",
+ [EXTCON_CHARGE_DOWNSTREAM] = "Charge-downstream",
+ [EXTCON_HDMI] = "HDMI",
+ [EXTCON_MHL] = "MHL",
+ [EXTCON_DVI] = "DVI",
+ [EXTCON_VGA] = "VGA",
+ [EXTCON_DOCK] = "Dock",
+ [EXTCON_AUDIO_IN] = "Audio-in",
+ [EXTCON_AUDIO_OUT] = "Audio-out",
+ [EXTCON_VIDEO_IN] = "Video-in",
+ [EXTCON_VIDEO_OUT] = "Video-out",
+
+ NULL,
+};
struct class *extcon_class;
struct class *extcon_class_for_android;
@@ -40,6 +59,7 @@ static DEFINE_MUTEX(extcon_dev_list_lock);
static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ int i, count = 0;
struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
if (edev->print_state) {
@@ -49,7 +69,55 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
return ret;
/* Use default if failed */
}
- return sprintf(buf, "%u\n", edev->state);
+
+ if (edev->max_supported == 0)
+ return sprintf(buf, "%u\n", edev->state);
+
+ for (i = 0; i < SUPPORTED_CABLE_MAX; i++) {
+ if (!edev->supported_cable[i])
+ break;
+ count += sprintf(buf + count, "%s=%d\n",
+ edev->supported_cable[i],
+ !!(edev->state & (1 << i)));
+ }
+
+ return count;
+}
+
+void extcon_set_state(struct extcon_dev *edev, u32 state);
+static ssize_t state_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u32 state;
+ ssize_t ret = 0;
+ int enabled;
+ char *buffer = kzalloc(count + 1, GFP_KERNEL);
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ /* Format: /Documentation/ABI/testing/sysfs-class-extcon */
+ ret = sscanf(buf, "%s %d", buffer, &enabled);
+
+ /* TODO: remove before submit */
+ pr_info("[%s][%d]\n", buffer, enabled);
+
+ if (ret == 2) {
+ /* Method 1 */
+ ret = extcon_set_cable_state(edev, buffer, enabled);
+ pr_info("Method 1\n");
+ } else if (ret == 1) {
+ /* Method 2 */
+ ret = sscanf(buf, "0x%x", &state);
+ pr_info("Method 2 with 0x%x\n", state);
+ if (ret == 0)
+ ret = -EINVAL;
+ else
+ extcon_set_state(edev, state);
+ } else {
+ ret = -EINVAL;
+ }
+
+ kfree(buffer);
+ return ret;
}
static ssize_t name_show(struct device *dev, struct device_attribute *attr,
@@ -68,8 +136,10 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
}
/**
- * extcon_set_state() - Set the cable attach states of the extcon device.
+ * extcon_update_state() - Update the cable attach states of the extcon device
+ * only for the masked bits.
* @edev: the extcon device
+ * @mask: the bit mask to designate updated bits.
* @state: new cable attach status for @edev
*
* Changing the state sends uevent with environment variable containing
@@ -77,10 +147,10 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
* Tizen uses this format for extcon device to get events from ports.
* Android uses this format as well.
*
- * Note that notifier provides the which bits are changes in the state
- * variable with "val" to the callback.
+ * Note that the notifier provides which bits are changed in the state
+ * variable with the val parameter (second) to the callback.
*/
-void extcon_set_state(struct extcon_dev *edev, u32 state)
+void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
{
char name_buf[120];
char state_buf[120];
@@ -88,15 +158,20 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
char *envp[3];
int env_offset = 0;
int length;
- u32 old_state = edev->state;
+ unsigned long flags;
- if (edev->state != state) {
- edev->state = state;
+ spin_lock_irqsave(&edev->lock, flags);
- raw_notifier_call_chain(&edev->nh, old_state ^ edev->state,
- edev);
+ if (edev->state != ((edev->state & ~mask) | (state & mask))) {
+ u32 old_state = edev->state;
- prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
+ edev->state &= ~mask;
+ edev->state |= state & mask;
+
+ raw_notifier_call_chain(&edev->nh, old_state, edev);
+
+ /* This could be in interrupt handler */
+ prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
if (prop_buf) {
length = name_show(edev->dev, NULL, prop_buf);
if (length > 0) {
@@ -115,17 +190,132 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
envp[env_offset++] = state_buf;
}
envp[env_offset] = NULL;
+ /* Unlock early before uevent */
+ spin_unlock_irqrestore(&edev->lock, flags);
+
kobject_uevent_env(&edev->dev->kobj, KOBJ_CHANGE, envp);
free_page((unsigned long)prop_buf);
} else {
+ /* Unlock early before uevent */
+ spin_unlock_irqrestore(&edev->lock, flags);
+
dev_err(edev->dev, "out of memory in extcon_set_state\n");
kobject_uevent(&edev->dev->kobj, KOBJ_CHANGE);
}
+ } else {
+ /* No changes */
+ spin_unlock_irqrestore(&edev->lock, flags);
}
}
+EXPORT_SYMBOL_GPL(extcon_update_state);
+
+/**
+ * extcon_set_state() - Set the cable attach states of the extcon device.
+ * @edev: the extcon device
+ * @state: new cable attach status for @edev
+ *
+ * Note that notifier provides which bits are changed in the state
+ * variable with the val parameter (second) to the callback.
+ */
+void extcon_set_state(struct extcon_dev *edev, u32 state)
+{
+ extcon_update_state(edev, 0xffffffff, state);
+}
EXPORT_SYMBOL_GPL(extcon_set_state);
/**
+ * extcon_find_cable_index() - Get the cable index based on the cable name.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name to be searched.
+ *
+ * Note that accessing a cable state based on cable_index is faster than
+ * cable_name because using cable_name induces a loop with strncmp().
+ * Thus, when get/set_cable_state is repeatedly used, using cable_index
+ * is recommended.
+ */
+int extcon_find_cable_index(struct extcon_dev *edev, const char *cable_name)
+{
+ int i;
+
+ if (edev->supported_cable) {
+ for (i = 0; edev->supported_cable[i]; i++) {
+ if (!strncmp(edev->supported_cable[i],
+ cable_name, CABLE_NAME_MAX))
+ return i;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(extcon_find_cable_index);
+
+/**
+ * extcon_get_cable_state_() - Get the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @index: cable index that can be retrieved by extcon_find_cable_index().
+ */
+int extcon_get_cable_state_(struct extcon_dev *edev, int index)
+{
+ if (index < 0 || (edev->max_supported && edev->max_supported <= index))
+ return -EINVAL;
+
+ return !!(edev->state & (1 << index));
+}
+EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
+
+/**
+ * extcon_get_cable_state() - Get the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name.
+ *
+ * Note that this is slower than extcon_get_cable_state_.
+ */
+int extcon_get_cable_state(struct extcon_dev *edev, const char *cable_name)
+{
+ return extcon_get_cable_state_(edev, extcon_find_cable_index
+ (edev, cable_name));
+}
+EXPORT_SYMBOL_GPL(extcon_get_cable_state);
+
+/**
+ * extcon_get_cable_state_() - Set the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @index: cable index that can be retrieved by extcon_find_cable_index().
+ * @cable_state: the new cable status. The default semantics is
+ * true: attached / false: detached.
+ */
+int extcon_set_cable_state_(struct extcon_dev *edev,
+ int index, bool cable_state)
+{
+ u32 state;
+
+ if (index < 0 || (edev->max_supported && edev->max_supported <= index))
+ return -EINVAL;
+
+ state = cable_state ? (1 << index) : 0;
+ extcon_update_state(edev, 1 << index, state);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
+
+/**
+ * extcon_get_cable_state() - Set the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name.
+ * @cable_state: the new cable status. The default semantics is
+ * true: attached / false: detached.
+ *
+ * Note that this is slower than extcon_set_cable_state_.
+ */
+int extcon_set_cable_state(struct extcon_dev *edev,
+ const char *cable_name, bool cable_state)
+{
+ return extcon_set_cable_state_(edev, extcon_find_cable_index
+ (edev, cable_name), cable_state);
+}
+EXPORT_SYMBOL_GPL(extcon_set_cable_state);
+
+/**
* extcon_get_extcon_dev() - Get the extcon device instance from the name
* @extcon_name: The extcon name provided with extcon_dev_register()
*/
@@ -145,11 +335,88 @@ out:
}
EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
+static int _call_per_cable(struct notifier_block *nb, unsigned long val,
+ void *ptr)
+{
+ struct extcon_specific_cable_nb *obj = container_of(nb,
+ struct extcon_specific_cable_nb, internal_nb);
+ struct extcon_dev *edev = ptr;
+
+ if ((val & (1 << obj->cable_index)) !=
+ (edev->state & (1 << obj->cable_index))) {
+ obj->previous_value = val;
+ return obj->user_nb->notifier_call(nb, val, ptr);
+ }
+
+ return NOTIFY_OK;
+}
+
+/**
+ * extcon_register_interest() - Register a notifier for a state change of a
+ * specific cable, not a entier set of cables of a
+ * extcon device.
+ * @obj: an empty extcon_specific_cable_nb object to be returned.
+ * @extcon_name: the name of extcon device.
+ * @cable_name: the target cable name.
+ * @nb: the notifier block to get notified.
+ *
+ * Provide an empty extcon_specific_cable_nb. extcon_register_interest() sets
+ * the struct for you.
+ *
+ * extcon_register_interest is a helper function for those who want to get
+ * notification for a single specific cable's status change. If a user wants
+ * to get notification for any changes of all cables of a extcon device,
+ * he/she should use the general extcon_register_notifier().
+ *
+ * Note that the second parameter given to the callback of nb (val) is
+ * "old_state", not the current state. The current state can be retrieved
+ * by looking at the third pameter (edev pointer)'s state value.
+ */
+int extcon_register_interest(struct extcon_specific_cable_nb *obj,
+ const char *extcon_name, const char *cable_name,
+ struct notifier_block *nb)
+{
+ if (!obj || !extcon_name || !cable_name || !nb)
+ return -EINVAL;
+
+ obj->edev = extcon_get_extcon_dev(extcon_name);
+ if (!obj->edev)
+ return -ENODEV;
+
+ obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
+ if (obj->cable_index < 0)
+ return -ENODEV;
+
+ obj->user_nb = nb;
+
+ obj->internal_nb.notifier_call = _call_per_cable;
+
+ return raw_notifier_chain_register(&obj->edev->nh, &obj->internal_nb);
+}
+
+/**
+ * extcon_unregister_interest() - Unregister the notifier registered by
+ * extcon_register_interest().
+ * @obj: the extcon_specific_cable_nb object returned by
+ * extcon_register_interest().
+ */
+int extcon_unregister_interest(struct extcon_specific_cable_nb *obj)
+{
+ if (!obj)
+ return -EINVAL;
+
+ return raw_notifier_chain_unregister(&obj->edev->nh, &obj->internal_nb);
+}
+
/**
* extcon_register_notifier() - Register a notifee to get notified by
* any attach status changes from the extcon.
* @edev: the extcon device.
* @nb: a notifier block to be registered.
+ *
+ * Note that the second parameter given to the callback of nb (val) is
+ * "old_state", not the current state. The current state can be retrieved
+ * by looking at the third pameter (edev pointer)'s state value.
*/
int extcon_register_notifier(struct extcon_dev *edev,
struct notifier_block *nb)
@@ -171,8 +438,9 @@ int extcon_unregister_notifier(struct extcon_dev *edev,
EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
static struct device_attribute extcon_attrs[] = {
- __ATTR_RO(state),
+ __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
+ __ATTR_NULL,
};
static int create_extcon_class(void)
@@ -231,7 +499,7 @@ static void extcon_dev_release(struct device *dev)
*/
int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
{
- int ret;
+ int ret, index = 0;
if (!extcon_class && !edev->use_class_name_switch) {
ret = create_extcon_class();
@@ -244,6 +512,20 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
return ret;
}
+ if (edev->supported_cable) {
+ /* Get size of array */
+ for (index = 0; edev->supported_cable[index]; index++)
+ ;
+ edev->max_supported = index;
+ } else {
+ edev->max_supported = 0;
+ }
+
+ if (index > SUPPORTED_CABLE_MAX) {
+ dev_err(edev->dev, "extcon: maximum number of supported cables exceeded.\n");
+ return -EINVAL;
+ }
+
edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
edev->dev->parent = dev;
if (edev->use_class_name_switch)
@@ -259,6 +541,8 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
goto err_dev;
}
+ spin_lock_init(&edev->lock);
+
RAW_INIT_NOTIFIER_HEAD(&edev->nh);
dev_set_drvdata(edev->dev, edev);
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 1e47e3b..4384d8f 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -24,6 +24,47 @@
#define __LINUX_EXTCON_H__
#include <linux/notifier.h>
+
+#define SUPPORTED_CABLE_MAX 32
+#define CABLE_NAME_MAX 30
+
+/*
+ * The standard cable name is to help support general notifier
+ * and notifee device drivers to share the common names.
+ * Please use standard cable names unless your notifier device has
+ * a very unique and abnormal cable or
+ * the cable type is supposed to be used with only one unique
+ * pair of notifier/notifee devices.
+ *
+ * Please add any other "standard" cables used with extcon dev.
+ *
+ * You may add a dot and number to specify version or specification
+ * of the specific cable if it is required. (e.g., "Fast-charger.18"
+ * and "Fast-charger.10" for 1.8A and 1.0A chargers)
+ * However, the notifee and notifier should be able to handle such
+ * string and if the notifee can negotiate the protocol or idenify,
+ * you don't need such convention. This convention is helpful when
+ * notifier can distinguish but notifiee cannot.
+ */
+enum extcon_cable_name {
+ EXTCON_USB = 0,
+ EXTCON_USB_HOST,
+ EXTCON_TA, /* Travel Adaptor */
+ EXTCON_FAST_CHARGER,
+ EXTCON_SLOW_CHARGER,
+ EXTCON_CHARGE_DOWNSTREAM, /* Charging an external device */
+ EXTCON_HDMI,
+ EXTCON_MHL,
+ EXTCON_DVI,
+ EXTCON_VGA,
+ EXTCON_DOCK,
+ EXTCON_AUDIO_IN,
+ EXTCON_AUDIO_OUT,
+ EXTCON_VIDEO_IN,
+ EXTCON_VIDEO_OUT,
+};
+extern const char *extcon_cable_name[];
+
/**
* struct extcon_dev - An extcon device represents one external connector.
* @name The name of this extcon device. Parent device name is used
@@ -31,6 +72,9 @@
* @use_class_name_switch set true in order to be compatible with
* Android platform, which uses "switch"
* for the class name.
+ * @supported_cable Array of supported cable name ending with NULL.
+ * If supported_cable is NULL, cable name related APIs
+ * are disabled.
* @print_name An optional callback to override the method to print the
* name of the extcon device.
* @print_state An optional callback to override the method to print the
@@ -41,6 +85,8 @@
* @nh Notifier for the state change events from this extcon
* @entry To support list of extcon devices so that uses can search
* for extcon devices based on the extcon name.
+ * @lock
+ * @max_supported Internal value to store the number of cables.
*
* In most cases, users only need to provide "User initializing data" of
* this struct when registering an extcon. In some exceptional cases,
@@ -51,6 +97,7 @@ struct extcon_dev {
/* --- Optional user initializing data --- */
const char *name;
bool use_class_name_switch;
+ const char **supported_cable;
/* --- Optional callbacks to override class functions --- */
ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
@@ -61,6 +108,8 @@ struct extcon_dev {
u32 state;
struct raw_notifier_head nh;
struct list_head entry;
+ spinlock_t lock; /* could be called by irq handler */
+ int max_supported;
};
/**
@@ -81,6 +130,23 @@ struct gpio_extcon_platform_data {
const char *state_off;
};
+/**
+ * struct extcon_specific_cable_nb - An internal data for
+ * extcon_register_interest().
+ * @internal_nb a notifier block bridging extcon notifier and cable notifier.
+ * @user_nb user provided notifier block for events from a specific cable.
+ * @cable_index the target cable.
+ * @edev the target extcon device.
+ * @previous_value the saved previous event value.
+ */
+struct extcon_specific_cable_nb {
+ struct notifier_block internal_nb;
+ struct notifier_block *user_nb;
+ int cable_index;
+ struct extcon_dev *edev;
+ unsigned long previous_value;
+};
+
#ifdef CONFIG_EXTCON
/*
@@ -91,16 +157,54 @@ extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
extern void extcon_dev_unregister(struct extcon_dev *edev);
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
+/*
+ * get/set/update_state access the 32b encoded state value, which represents
+ * states of all possible cables of the multistate port. For example, if one
+ * calls extcon_set_state(edev, 0x7), it may mean that all the three cables
+ * are attached to the port.
+ */
static inline u32 extcon_get_state(struct extcon_dev *edev)
{
return edev->state;
}
extern void extcon_set_state(struct extcon_dev *edev, u32 state);
+extern void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
+
+/*
+ * get/set_cable_state access each bit of the 32b encoded state value.
+ * They are used to access the status of each cable based on the cable_name
+ * or cable_index, which is retrived by extcon_find_cable_index
+ */
+extern int extcon_find_cable_index(struct extcon_dev *sdev,
+ const char *cable_name);
+extern int extcon_get_cable_state_(struct extcon_dev *edev, int cable_index);
+extern int extcon_set_cable_state_(struct extcon_dev *edev, int cable_index,
+ bool cable_state);
+
+extern int extcon_get_cable_state(struct extcon_dev *edev,
+ const char *cable_name);
+extern int extcon_set_cable_state(struct extcon_dev *edev,
+ const char *cable_name, bool cable_state);
+
+/*
+ * Following APIs are for notifiees (those who want to be notified)
+ * to register a callback for events from a specific cable of the extcon.
+ * Notifiees are the connected device drivers wanting to get notified by
+ * a specific external port of a connection device.
+ */
+extern int extcon_register_interest(struct extcon_specific_cable_nb *obj,
+ const char *extcon_name,
+ const char *cable_name,
+ struct notifier_block *nb);
+extern int extcon_unregister_interest(struct extcon_specific_cable_nb *nb);
/*
* Following APIs are to monitor every action of a notifier.
* Registerer gets notified for every external port of a connection device.
+ * Probably this could be used to debug an action of notifier; however,
+ * we do not recommend to use this at normal 'notifiee' device drivers who
+ * want to be notified by a specific external port of the notifier.
*/
extern int extcon_register_notifier(struct extcon_dev *edev,
struct notifier_block *nb);
@@ -121,6 +225,41 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
}
static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+
+static inline void extcon_update_state(struct extcon_dev *edev, u32 mask,
+ u32 state)
+{ }
+
+static inline int extcon_find_cable_index(struct extcon_dev *edev,
+ const char *cable_name)
+{
+ return 0;
+}
+
+static inline int extcon_get_cable_state_(struct extcon_dev *edev,
+ int cable_index)
+{
+ return 0;
+}
+
+static inline int extcon_set_cable_state_(struct extcon_dev *edev,
+ int cable_index, bool cable_state)
+{
+ return 0;
+}
+
+static inline int extcon_get_cable_state(struct extcon_dev *edev,
+ const char *cable_name)
+{
+ return 0;
+}
+
+static inline int extcon_set_cable_state(struct extcon_dev *edev,
+ const char *cable_name, int state)
+{
+ return 0;
+}
+
static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
{
return NULL;
@@ -138,5 +277,17 @@ static inline int extcon_unregister_notifier(struct extcon_dev *edev,
return 0;
}
+static inline int extcon_register_interest(struct extcon_specific_cable_nb *obj,
+ const char *extcon_name,
+ const char *cable_name,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int extcon_unregister_interest(struct extcon_specific_cable_nb *)
+{
+ return 0;
+}
#endif /* CONFIG_EXTCON */
#endif /* __LINUX_EXTCON_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v5 3/5] Extcon: support multiple states at a device.
2012-02-10 6:40 ` [PATCH v5 3/5] Extcon: support multiple states at a device MyungJoo Ham
@ 2012-02-20 2:24 ` Mark Brown
2012-02-20 7:02 ` MyungJoo Ham
2012-02-22 10:07 ` Arnd Bergmann
1 sibling, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-02-20 2:24 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel, myungjoo.ham
[-- Attachment #1: Type: text/plain, Size: 555 bytes --]
On Fri, Feb 10, 2012 at 03:40:36PM +0900, MyungJoo Ham wrote:
> + [EXTCON_AUDIO_IN] = "Audio-in",
> + [EXTCON_AUDIO_OUT] = "Audio-out",
Can I suggest making these more specific - how about
[EXTCON_LINE_IN] = "Line-in",
[EXTCON_LINE_OUT] = "Line-out",
[EXTCON_MIC_IN] = "Microphone",
[EXTCON_HEADPHONE_OUT] = "Headphone",
as a starting point? The distinction between line and the other two is
often pretty important from a UI/system interaction point of view.
> + /* TODO: remove before submit */
> + pr_info("[%s][%d]\n", buffer, enabled);
:)
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 3/5] Extcon: support multiple states at a device.
2012-02-20 2:24 ` Mark Brown
@ 2012-02-20 7:02 ` MyungJoo Ham
0 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-20 7:02 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
On Mon, Feb 20, 2012 at 11:24 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Feb 10, 2012 at 03:40:36PM +0900, MyungJoo Ham wrote:
>
>> + [EXTCON_AUDIO_IN] = "Audio-in",
>> + [EXTCON_AUDIO_OUT] = "Audio-out",
>
> Can I suggest making these more specific - how about
>
> [EXTCON_LINE_IN] = "Line-in",
> [EXTCON_LINE_OUT] = "Line-out",
> [EXTCON_MIC_IN] = "Microphone",
> [EXTCON_HEADPHONE_OUT] = "Headphone",
>
> as a starting point? The distinction between line and the other two is
> often pretty important from a UI/system interaction point of view.
Sure, this looks more generic. I'll add them along with SPDIF in/out.
>
>> + /* TODO: remove before submit */
>> + pr_info("[%s][%d]\n", buffer, enabled);
>
> :)
Whoops...
--
MyungJoo Ham, Ph.D.
System S/W Lab, Software Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v5 3/5] Extcon: support multiple states at a device.
2012-02-10 6:40 ` [PATCH v5 3/5] Extcon: support multiple states at a device MyungJoo Ham
2012-02-20 2:24 ` Mark Brown
@ 2012-02-22 10:07 ` Arnd Bergmann
1 sibling, 0 replies; 50+ messages in thread
From: Arnd Bergmann @ 2012-02-22 10:07 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown,
John Stultz, Joerg Roedel, myungjoo.ham
On Friday 10 February 2012, MyungJoo Ham wrote:
> Date: December 2011
> Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> Description:
> - The /sys/class/extcon/.../state shows the cable attach/detach
> - information of the corresponding extcon object. If the extcon
> - objecct has an optional callback "show_state" defined, the
> - callback will provide the name with this sysfs node.
> + The /sys/class/extcon/.../state shows and stores the cable
> + attach/detach information of the corresponding extcon object.
> + If the extcon objecct has an optional callback "show_state"
> + defined, the showing function is overriden with the optional
> + callback.
> +
> + If the default callback for showing function is used, the
> + format is like this:
> + # cat state
> + USB_OTG=1
> + HDMI=0
> + TA=1
> + EAR_JACK=0
> + #
> + In this example, the extcon device have USB_OTG and TA
> + cables attached and HDMI and EAR_JACK cables detached.
> +
> + There are two methods to update state of extcon.
> + Method 1.
> + echo CABLE_NAME [0|1] > state
> + Method 2.
> + echo 0xHEX > state
> +
> + Method 1 updates the state (0 or 1) of the
> + corresponding cable (either the name or index of the cable).
> + Method 2 updates the whole state of the extcon dev.
I think having two ways to do the same thing is rather confusing, it would
be nice to provide only one way and make sure that it works for everyone.
Maybe the entire thing can be turned around into one attribute group
per cable and unified with the mutually-exclusive part, like
extcon/.../usb-otg/state # 0 or 1 for disable or enable
/mutually-exclusive # list of exclusive ports
/hdmi/...
/ta/...
Arnd
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-10 6:40 ` [PATCH v5 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
` (2 preceding siblings ...)
2012-02-10 6:40 ` [PATCH v5 3/5] Extcon: support multiple states at a device MyungJoo Ham
@ 2012-02-10 6:40 ` MyungJoo Ham
2012-02-20 2:27 ` Mark Brown
2012-02-10 6:40 ` [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
5 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-10 6:40 UTC (permalink / raw)
To: linux-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, Donggeun Kim, Greg KH, Arnd Bergmann,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown,
John Stultz, Joerg Roedel, myungjoo.ham
There could be cables that cannot be attaches simulatenously. Extcon
device drivers may express such information via mutually_exclusive in
struct extcon_dev.
For example, for an extcon device with 16 cables (bits 0 to 15 are
available), if mutually_exclusive = { 0x7, 0xC0, 0x81, 0 }, then, the
following attachments are prohibitted.
{0, 1}
{0, 2}
{1, 2}
{6, 7}
{0, 7}
and every attachment set that are superset of one of the above.
For the detail, please refer to linux/include/linux/extcon.h.
The concept is suggested by NeilBrown <neilb@suse.de>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Documentation/ABI/testing/sysfs-class-extcon | 19 +++++++
drivers/extcon/extcon_class.c | 70 +++++++++++++++++++++++---
include/linux/extcon.h | 24 +++++++--
3 files changed, 101 insertions(+), 12 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-extcon b/Documentation/ABI/testing/sysfs-class-extcon
index 31bb609..5e5f9a9 100644
--- a/Documentation/ABI/testing/sysfs-class-extcon
+++ b/Documentation/ABI/testing/sysfs-class-extcon
@@ -15,6 +15,10 @@ Description:
may have both HDMI and Charger attached, or analog audio,
video, and USB cables attached simulteneously.
+ If there are cables mutually exclusive with each other,
+ such binary relations may be expressed with extcon_dev's
+ mutually_exclusive array.
+
What: /sys/class/extcon/.../name
Date: December 2011
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
@@ -54,3 +58,18 @@ Description:
Method 1 updates the state (0 or 1) of the
corresponding cable (either the name or index of the cable).
Method 2 updates the whole state of the extcon dev.
+ Inputs of all the methods are required to meet the
+ mutually_exclusive contidions if they exist.
+
+What: /sys/class/extcon/.../mutually_exclusive
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ Shows the relations of mutually exclusiveness. For example,
+ if the mutually_exclusive array of extcon_dev is
+ {0x3, 0x5, 0xC, 0x0}, the, the output is:
+ # cat mutually_exclusive
+ 0x3
+ 0x5
+ 0xC
+ #
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
index 2803bd0..768b70a 100644
--- a/drivers/extcon/extcon_class.c
+++ b/drivers/extcon/extcon_class.c
@@ -56,6 +56,39 @@ struct class *extcon_class_for_android;
static LIST_HEAD(extcon_dev_list);
static DEFINE_MUTEX(extcon_dev_list_lock);
+/**
+ * check_mutually_exclusive - Check if new_state violates mutually_exclusive
+ * condition.
+ * @edev: the extcon device
+ * @new_state: new cable attach status for @edev
+ *
+ * Returns 0 if nothing violates. Returns the index + 1 for the first
+ * violated condition.
+ */
+static int check_mutually_exclusive(struct extcon_dev *edev, u32 new_state)
+{
+ int i = 0;
+
+ if (!edev->mutually_exclusive)
+ return 0;
+
+ for (i = 0; edev->mutually_exclusive[i]; i++) {
+ int count = 0, j;
+ u32 correspondants = new_state & edev->mutually_exclusive[i];
+ u32 exp = 1;
+
+ for (j = 0; j < 32; j++) {
+ if (exp & correspondants)
+ count++;
+ if (count > 1)
+ return i + 1;
+ exp <<= 1;
+ }
+ }
+
+ return 0;
+}
+
static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -84,7 +117,7 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
return count;
}
-void extcon_set_state(struct extcon_dev *edev, u32 state);
+int extcon_set_state(struct extcon_dev *edev, u32 state);
static ssize_t state_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -111,7 +144,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
if (ret == 0)
ret = -EINVAL;
else
- extcon_set_state(edev, state);
+ ret = extcon_set_state(edev, state);
} else {
ret = -EINVAL;
}
@@ -135,6 +168,23 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
return sprintf(buf, "%s\n", dev_name(edev->dev));
}
+static ssize_t mutually_exclusive_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+ int i;
+ int count = 0;
+
+ if (!edev->mutually_exclusive || !edev->mutually_exclusive[0])
+ return sprintf(buf, "No cables are mutually exclusive.\n");
+
+ for (i = 0; edev->mutually_exclusive[i]; i++)
+ count += sprintf(buf + count, "0x%x\n",
+ edev->mutually_exclusive[i]);
+
+ return count;
+}
+
/**
* extcon_update_state() - Update the cable attach states of the extcon device
* only for the masked bits.
@@ -150,7 +200,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
* Note that the notifier provides which bits are changed in the state
* variable with the val parameter (second) to the callback.
*/
-void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
+int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
{
char name_buf[120];
char state_buf[120];
@@ -165,6 +215,10 @@ void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
if (edev->state != ((edev->state & ~mask) | (state & mask))) {
u32 old_state = edev->state;
+ if (check_mutually_exclusive(edev, (edev->state & ~mask) |
+ (state & mask)))
+ return -EPERM;
+
edev->state &= ~mask;
edev->state |= state & mask;
@@ -206,6 +260,8 @@ void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
/* No changes */
spin_unlock_irqrestore(&edev->lock, flags);
}
+
+ return 0;
}
EXPORT_SYMBOL_GPL(extcon_update_state);
@@ -217,9 +273,9 @@ EXPORT_SYMBOL_GPL(extcon_update_state);
* Note that notifier provides which bits are changed in the state
* variable with the val parameter (second) to the callback.
*/
-void extcon_set_state(struct extcon_dev *edev, u32 state)
+int extcon_set_state(struct extcon_dev *edev, u32 state)
{
- extcon_update_state(edev, 0xffffffff, state);
+ return extcon_update_state(edev, 0xffffffff, state);
}
EXPORT_SYMBOL_GPL(extcon_set_state);
@@ -293,8 +349,7 @@ int extcon_set_cable_state_(struct extcon_dev *edev,
return -EINVAL;
state = cable_state ? (1 << index) : 0;
- extcon_update_state(edev, 1 << index, state);
- return 0;
+ return extcon_update_state(edev, 1 << index, state);
}
EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
@@ -440,6 +495,7 @@ EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
static struct device_attribute extcon_attrs[] = {
__ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
+ __ATTR_RO(mutually_exclusive),
__ATTR_NULL,
};
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 4384d8f..20b8e5d 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -75,6 +75,14 @@ extern const char *extcon_cable_name[];
* @supported_cable Array of supported cable name ending with NULL.
* If supported_cable is NULL, cable name related APIs
* are disabled.
+ * @mutually_exclusive Array of mutually exclusive set of cables that cannot
+ * be attached simultaneously. The array should be
+ * ending with NULL or be NULL (no mutually exclusive
+ * cables). For example, if it is { 0x7, 0x30, 0}, then,
+ * {0, 1}, {0, 1, 2}, {0, 2}, {1, 2}, or {4, 5} cannot
+ * be attached simulataneously. {0x7, 0} is equivalent to
+ * {0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
+ * can be no simultaneous connections.
* @print_name An optional callback to override the method to print the
* name of the extcon device.
* @print_state An optional callback to override the method to print the
@@ -98,6 +106,7 @@ struct extcon_dev {
const char *name;
bool use_class_name_switch;
const char **supported_cable;
+ const u32 *mutually_exclusive;
/* --- Optional callbacks to override class functions --- */
ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
@@ -168,8 +177,8 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
return edev->state;
}
-extern void extcon_set_state(struct extcon_dev *edev, u32 state);
-extern void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
+extern int extcon_set_state(struct extcon_dev *edev, u32 state);
+extern int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
/*
* get/set_cable_state access each bit of the 32b encoded state value.
@@ -224,11 +233,16 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
return 0;
}
-static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+static inline int extcon_set_state(struct extcon_dev *edev, u32 state)
+{
+ return 0;
+}
-static inline void extcon_update_state(struct extcon_dev *edev, u32 mask,
+static inline int extcon_update_state(struct extcon_dev *edev, u32 mask,
u32 state)
-{ }
+{
+ return 0;
+}
static inline int extcon_find_cable_index(struct extcon_dev *edev,
const char *cable_name)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-10 6:40 ` [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables MyungJoo Ham
@ 2012-02-20 2:27 ` Mark Brown
2012-02-22 8:23 ` MyungJoo Ham
0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-02-20 2:27 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel, myungjoo.ham
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Fri, Feb 10, 2012 at 03:40:37PM +0900, MyungJoo Ham wrote:
> +What: /sys/class/extcon/.../mutually_exclusive
> +Date: December 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + Shows the relations of mutually exclusiveness. For example,
> + if the mutually_exclusive array of extcon_dev is
> + {0x3, 0x5, 0xC, 0x0}, the, the output is:
> + # cat mutually_exclusive
> + 0x3
> + 0x5
> + 0xC
> + #
sysfs really isn't supposed to have more than one value per file so this
should be something like a directory with one array element per file.
It'd kind of be cheating but the values could be used as the filenames,
though serial numbers would be more idiomatic.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-20 2:27 ` Mark Brown
@ 2012-02-22 8:23 ` MyungJoo Ham
2012-02-22 10:00 ` Arnd Bergmann
0 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-22 8:23 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
On Mon, Feb 20, 2012 at 11:27 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Feb 10, 2012 at 03:40:37PM +0900, MyungJoo Ham wrote:
>
>> +What: /sys/class/extcon/.../mutually_exclusive
>> +Date: December 2011
>> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> + Shows the relations of mutually exclusiveness. For example,
>> + if the mutually_exclusive array of extcon_dev is
>> + {0x3, 0x5, 0xC, 0x0}, the, the output is:
>> + # cat mutually_exclusive
>> + 0x3
>> + 0x5
>> + 0xC
>> + #
>
> sysfs really isn't supposed to have more than one value per file so this
> should be something like a directory with one array element per file.
> It'd kind of be cheating but the values could be used as the filenames,
> though serial numbers would be more idiomatic.
Hmm.. I'll either use multiple files with a sub-directory or bin-attr
to inhabit the values. Or I'll search for any other convinient way to
express such an array thru sysfs.
Thanks.
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
System S/W Lab,S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-22 8:23 ` MyungJoo Ham
@ 2012-02-22 10:00 ` Arnd Bergmann
2012-02-24 4:56 ` MyungJoo Ham
0 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2012-02-22 10:00 UTC (permalink / raw)
To: myungjoo.ham
Cc: Mark Brown, linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, John Stultz,
Joerg Roedel
On Wednesday 22 February 2012, MyungJoo Ham wrote:
>
> On Mon, Feb 20, 2012 at 11:27 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Fri, Feb 10, 2012 at 03:40:37PM +0900, MyungJoo Ham wrote:
> >
> >> +What: /sys/class/extcon/.../mutually_exclusive
> >> +Date: December 2011
> >> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> >> +Description:
> >> + Shows the relations of mutually exclusiveness. For example,
> >> + if the mutually_exclusive array of extcon_dev is
> >> + {0x3, 0x5, 0xC, 0x0}, the, the output is:
> >> + # cat mutually_exclusive
> >> + 0x3
> >> + 0x5
> >> + 0xC
> >> + #
> >
> > sysfs really isn't supposed to have more than one value per file so this
> > should be something like a directory with one array element per file.
> > It'd kind of be cheating but the values could be used as the filenames,
> > though serial numbers would be more idiomatic.
>
> Hmm.. I'll either use multiple files with a sub-directory or bin-attr
> to inhabit the values. Or I'll search for any other convinient way to
> express such an array thru sysfs.
How about having one file for each cable and list the ones that it
cannot coexist with. In the above example, that would become
mutually_exclusive/0: "0x6"
mutually_exclusive/1: "0x1"
mutually_exclusive/2: "0x9"
mutually_exclusive/3: "0x4"
The information here is a bit redundant, but this would be much easier
to understand for human readers. I also wouldn't mind having a list
of exclusive cable names in each file, while would make it slightly
harder to parse but very easy for humans:
mutually_exclusive/Video-in: "VGA"
mutually_exclusive/Video-out: "VGA"
mutually_exclusive/VGA: "Video-out Video-in"
Arnd
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-22 10:00 ` Arnd Bergmann
@ 2012-02-24 4:56 ` MyungJoo Ham
2012-02-24 12:53 ` Arnd Bergmann
0 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-24 4:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Brown, linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, gregkh,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, John Stultz,
Joerg Roedel
On Wed, Feb 22, 2012 at 7:00 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 22 February 2012, MyungJoo Ham wrote:
>>
>> On Mon, Feb 20, 2012 at 11:27 AM, Mark Brown
>> <broonie@opensource.wolfsonmicro.com> wrote:
>> > On Fri, Feb 10, 2012 at 03:40:37PM +0900, MyungJoo Ham wrote:
>> >
>> >> +What: /sys/class/extcon/.../mutually_exclusive
>> >> +Date: December 2011
>> >> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
>> >> +Description:
>> >> + Shows the relations of mutually exclusiveness. For example,
>> >> + if the mutually_exclusive array of extcon_dev is
>> >> + {0x3, 0x5, 0xC, 0x0}, the, the output is:
>> >> + # cat mutually_exclusive
>> >> + 0x3
>> >> + 0x5
>> >> + 0xC
>> >> + #
>> >
>> > sysfs really isn't supposed to have more than one value per file so this
>> > should be something like a directory with one array element per file.
>> > It'd kind of be cheating but the values could be used as the filenames,
>> > though serial numbers would be more idiomatic.
>>
>> Hmm.. I'll either use multiple files with a sub-directory or bin-attr
>> to inhabit the values. Or I'll search for any other convinient way to
>> express such an array thru sysfs.
>
> How about having one file for each cable and list the ones that it
> cannot coexist with. In the above example, that would become
>
> mutually_exclusive/0: "0x6"
> mutually_exclusive/1: "0x1"
> mutually_exclusive/2: "0x9"
> mutually_exclusive/3: "0x4"
>
> The information here is a bit redundant, but this would be much easier
> to understand for human readers. I also wouldn't mind having a list
> of exclusive cable names in each file, while would make it slightly
> harder to parse but very easy for humans:
>
> mutually_exclusive/Video-in: "VGA"
> mutually_exclusive/Video-out: "VGA"
> mutually_exclusive/VGA: "Video-out Video-in"
>
> Arnd
Ok, here goes an example how it would look like in the next revision of Extcon.
I think this addresses your comments on multistate-support patch (3/5)
/sys/class/extcon/max8997-muic.0/
name: "max8997-muic.0"
state: 0x00000009 /* RW. 0xblahblah format
for both read and write */
mutually_exclusive/
rule.0: 0x3
rule.1: 0x5
rule.2: 0xC
cable.0/
name: "VGA"
state: 1 /* RW: 0 or 1 */
mutually_exclusive/
cable.1
cable.2
cable.1/
...
How about this?
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-24 4:56 ` MyungJoo Ham
@ 2012-02-24 12:53 ` Arnd Bergmann
2012-02-27 6:47 ` MyungJoo Ham
0 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2012-02-24 12:53 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Mark Brown, linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, gregkh,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, John Stultz,
Joerg Roedel
On Friday 24 February 2012, MyungJoo Ham wrote:
> Ok, here goes an example how it would look like in the next revision of Extcon.
> I think this addresses your comments on multistate-support patch (3/5)
>
> /sys/class/extcon/max8997-muic.0/
> name: "max8997-muic.0"
> state: 0x00000009 /* RW. 0xblahblah format
> for both read and write */
> mutually_exclusive/
> rule.0: 0x3
> rule.1: 0x5
> rule.2: 0xC
> cable.0/
> name: "VGA"
> state: 1 /* RW: 0 or 1 */
> mutually_exclusive/
> cable.1
> cable.2
> cable.1/
> ...
>
>
> How about this?
>
I would leave out the global mutually_exclusive/ directory because that
is redundant and I see no benefit in it. Having the redundant "state"
files is probably a good idea because the global one lets you make
atomic changes of multiple cables and the other are very convenient.
What kind of object is /sys/class/extcon/max8997-muic.0/cable.0/mutually_exclusive?
Is that a file with two lines, a directory with two empty files or
a directory with symlinks to the other directories?
Arnd
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables.
2012-02-24 12:53 ` Arnd Bergmann
@ 2012-02-27 6:47 ` MyungJoo Ham
0 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 6:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Brown, linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, gregkh,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, John Stultz,
Joerg Roedel
On Fri, Feb 24, 2012 at 9:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 24 February 2012, MyungJoo Ham wrote:
>> Ok, here goes an example how it would look like in the next revision of Extcon.
>> I think this addresses your comments on multistate-support patch (3/5)
>>
>> /sys/class/extcon/max8997-muic.0/
>> name: "max8997-muic.0"
>> state: 0x00000009 /* RW. 0xblahblah format
>> for both read and write */
>> mutually_exclusive/
>> rule.0: 0x3
>> rule.1: 0x5
>> rule.2: 0xC
==> I'll simply add 0x3, 0x5, 0xc as the filenames insteadof rule.0 .. 2.
>> cable.0/
>> name: "VGA"
>> state: 1 /* RW: 0 or 1 */
>> mutually_exclusive/
>> cable.1
>> cable.2
==> Removing this subdirectory, "mutually_exclusive"
>> cable.1/
>> ...
>>
>>
>> How about this?
>>
>
> I would leave out the global mutually_exclusive/ directory because that
> is redundant and I see no benefit in it. Having the redundant "state"
> files is probably a good idea because the global one lets you make
> atomic changes of multiple cables and the other are very convenient.
>
> What kind of object is /sys/class/extcon/max8997-muic.0/cable.0/mutually_exclusive?
> Is that a file with two lines, a directory with two empty files or
> a directory with symlinks to the other directories?
I meant it to be a directory containing attributes (w/o show/store
callbacks) with names of exclusive cables.
However, because we cannot make 2nd-subdirectories under a kobject (I
didn't know this until some hours ago), it appears that I need to
avoid this approach until it is allowed to do so.
Because the redundant information is in the global mutually_exclusive
directory, I'll skip this one (mutually_exclusive subdirectory under
cable.x subdirectory).
>
> Arnd
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices
2012-02-10 6:40 ` [PATCH v5 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
` (3 preceding siblings ...)
2012-02-10 6:40 ` [PATCH v5 4/5] Extcon: support mutually exclusive relation between cables MyungJoo Ham
@ 2012-02-10 6:40 ` MyungJoo Ham
2012-02-10 16:25 ` Mark Brown
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
5 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-10 6:40 UTC (permalink / raw)
To: linux-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, Donggeun Kim, Greg KH, Arnd Bergmann,
Linus Walleij, Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown,
John Stultz, Joerg Roedel, myungjoo.ham
External connector devices that decides connection information based on
ADC values may use adc-jack device driver. The user simply needs to
provide a table of adc range and connection states. Then, extcon
framework will automatically notify others.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/extcon/Kconfig | 7 ++-
drivers/extcon/Makefile | 1 +
drivers/extcon/adc_jack.c | 188 +++++++++++++++++++++++++++++++++++++++
include/linux/extcon/adc_jack.h | 107 ++++++++++++++++++++++
4 files changed, 302 insertions(+), 1 deletions(-)
create mode 100644 drivers/extcon/adc_jack.c
create mode 100644 include/linux/extcon/adc_jack.h
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 3fa3c22..7487384 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -19,4 +19,9 @@ config EXTCON_GPIO
Say Y here to enable GPIO based extcon support. Note that GPIO
extcon supports single state per extcon instance.
-endif # MULTISTATE_SWITCH
+config EXTCON_ADC_JACK
+ tristate "ADC Jack extcon support"
+ help
+ Say Y here to enable extcon device driver based on ADC values.
+
+endif # EXTCON
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 2c46d41..ca558a0 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_EXTCON) += extcon_class.o
obj-$(CONFIG_EXTCON_GPIO) += extcon_gpio.o
+obj-$(CONFIG_EXTCON_ADC_JACK) += adc_jack.o
diff --git a/drivers/extcon/adc_jack.c b/drivers/extcon/adc_jack.c
new file mode 100644
index 0000000..9cad565
--- /dev/null
+++ b/drivers/extcon/adc_jack.c
@@ -0,0 +1,188 @@
+/*
+ * drivers/extcon/adc_jack.c
+ *
+ * Analog Jack extcon driver with ADC-based detection capability.
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/extcon.h>
+#include <linux/extcon/adc_jack.h>
+
+static void adc_jack_handler(struct work_struct *work)
+{
+ struct adc_jack_data *data = container_of(to_delayed_work(work),
+ struct adc_jack_data,
+ handler);
+ u32 adc, state = 0;
+ int ret;
+ int i;
+
+ if (!data->ready)
+ return;
+
+ ret = data->get_adc(&adc);
+ if (ret) {
+ dev_err(data->edev.dev, "get_adc() error: %d\n", ret);
+ return;
+ }
+
+ /* Get state from adc value with adc_condition */
+ for (i = 0; i < data->num_conditions; i++) {
+ struct adc_jack_cond *def = &data->adc_condition[i];
+ if (!def->state)
+ break;
+ if (def->min_adc <= adc && def->max_adc >= adc) {
+ state = def->state;
+ break;
+ }
+ }
+ /* if no def has met, it means state = 0 (no cables attached) */
+
+ extcon_set_state(&data->edev, state);
+}
+
+static irqreturn_t adc_jack_irq_thread(int irq, void *_data)
+{
+ struct adc_jack_data *data = _data;
+
+ schedule_delayed_work(&data->handler, data->handling_delay);
+
+ return IRQ_HANDLED;
+}
+
+static int adc_jack_probe(struct platform_device *pdev)
+{
+ struct adc_jack_data *data;
+ struct adc_jack_pdata *pdata = pdev->dev.platform_data;
+ int i, err = 0;
+
+ data = kzalloc(sizeof(struct adc_jack_data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->edev.name = pdata->name;
+
+ if (pdata->cable_names)
+ data->edev.supported_cable = pdata->cable_names;
+ else
+ data->edev.supported_cable = extcon_cable_name;
+
+ /* Check the length of array and set num_cables */
+ for (i = 0; data->edev.supported_cable[i]; i++)
+ ;
+ if (i == 0 || i > SUPPORTED_CABLE_MAX) {
+ err = -EINVAL;
+ dev_err(&pdev->dev, "error: pdata->cable_names size = %d\n",
+ i - 1);
+ goto err_alloc;
+ }
+ data->num_cables = i;
+
+ if (!pdata->adc_condition ||
+ !pdata->adc_condition[0].state) {
+ err = -EINVAL;
+ dev_err(&pdev->dev, "error: adc_condition not defined.\n");
+ goto err_alloc;
+ }
+ data->adc_condition = pdata->adc_condition;
+
+ /* Check the length of array and set num_conditions */
+ for (i = 0; data->adc_condition[i].state; i++)
+ ;
+ data->num_conditions = i;
+
+ if (!pdata->get_adc) {
+ dev_err(&pdev->dev, "error: get_adc() is mandatory.\n");
+ err = -EINVAL;
+ goto err_alloc;
+ }
+ data->get_adc = pdata->get_adc;
+ data->handling_delay = msecs_to_jiffies(pdata->handling_delay_ms);
+
+ INIT_DELAYED_WORK_DEFERRABLE(&data->handler, adc_jack_handler);
+
+ platform_set_drvdata(pdev, data);
+
+ if (pdata->irq) {
+ data->irq = pdata->irq;
+
+ err = request_threaded_irq(data->irq, NULL,
+ adc_jack_irq_thread,
+ pdata->irq_flags, pdata->name,
+ data);
+
+ if (err) {
+ dev_err(&pdev->dev, "error: irq %d\n", data->irq);
+ err = -EINVAL;
+ goto err_initwork;
+ }
+ }
+ err = extcon_dev_register(&data->edev, &pdev->dev);
+ if (err)
+ goto err_irq;
+
+ data->ready = true;
+
+ goto out;
+
+err_irq:
+ if (data->irq)
+ free_irq(data->irq, data);
+err_initwork:
+ cancel_delayed_work_sync(&data->handler);
+err_alloc:
+ kfree(data);
+out:
+ return err;
+}
+
+static int __devexit adc_jack_remove(struct platform_device *pdev)
+{
+ struct adc_jack_data *data = platform_get_drvdata(pdev);
+
+ extcon_dev_unregister(&data->edev);
+ if (data->irq)
+ free_irq(data->irq, data);
+ platform_set_drvdata(pdev, NULL);
+ kfree(data);
+
+ return 0;
+}
+
+static struct platform_driver adc_jack_driver = {
+ .probe = adc_jack_probe,
+ .remove = __devexit_p(adc_jack_remove),
+ .driver = {
+ .name = "adc-jack",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init adc_jack_init(void)
+{
+ return platform_driver_register(&adc_jack_driver);
+}
+
+static void __exit adc_jack_exit(void)
+{
+ platform_driver_unregister(&adc_jack_driver);
+}
+
+module_init(adc_jack_init);
+module_exit(adc_jack_exit);
+
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_DESCRIPTION("ADC jack extcon driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/extcon/adc_jack.h b/include/linux/extcon/adc_jack.h
new file mode 100644
index 0000000..1650b85
--- /dev/null
+++ b/include/linux/extcon/adc_jack.h
@@ -0,0 +1,107 @@
+/*
+ * include/linux/extcon/adc_jack.h
+ *
+ * Analog Jack extcon driver with ADC-based detection capability.
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef _EXTCON_ADC_JACK_H_
+#define _EXTCON_ADC_JACK_H_ __FILE__
+#include <linux/extcon.h>
+
+/**
+ * struct adc_jack_data - internal data for adc_jack device driver
+ * @edev - extcon device.
+ * @cable_names - list of supported cables.
+ * @num_cables - size of cable_names.
+ * @adc_condition - list of adc value conditions.
+ * @num_condition - size of adc_condition.
+ * @irq - irq number of attach/detach event (0 if not exist).
+ * @handling_delay - interrupt handler will schedule extcon event
+ * handling at handling_delay jiffies.
+ * @handler - extcon event handler called by interrupt handler.
+ * @get_adc - a callback to get ADC value to identify state.
+ * @ready - true if it is safe to run handler.
+ */
+struct adc_jack_data {
+ struct extcon_dev edev;
+
+ const char **cable_names;
+ int num_cables;
+ struct adc_jack_cond *adc_condition;
+ int num_conditions;
+
+ int irq;
+ unsigned long handling_delay; /* in jiffies */
+ struct delayed_work handler;
+
+ int (*get_adc)(u32 *value);
+
+ bool ready;
+};
+
+/**
+ * struct adc_jack_cond - condition to use an extcon state
+ * @state - the corresponding extcon state (if 0, this struct denotes
+ * the last adc_jack_cond element among the array)
+ * @min_adc - min adc value for this condition
+ * @max_adc - max adc value for this condition
+ *
+ * For example, if { .state = 0x3, .min_adc = 100, .max_adc = 200}, it means
+ * that if ADC value is between (inclusive) 100 and 200, than the cable 0 and
+ * 1 are attached (1<<0 | 1<<1 == 0x3)
+ *
+ * Note that you don't need to describe condition for "no cable attached"
+ * because when no adc_jack_cond is met, state = 0 is automatically chosen.
+ */
+struct adc_jack_cond {
+ u32 state; /* extcon state value. 0 if invalid */
+
+ u32 min_adc;
+ u32 max_adc;
+};
+
+/**
+ * struct adc_jack_pdata - platform data for adc jack device.
+ * @name - name of the extcon device. If null, "adc-jack" is used.
+ * @cable_names - array of cable names ending with null. If the array itself
+ * if null, extcon standard cable names are chosen.
+ * @adc_contition - array of struct adc_jack_cond conditions ending
+ * with .state = 0 entry. This describes how to decode
+ * adc values into extcon state.
+ * @irq - IRQ number that is triggerred by cable attach/detach
+ * events. If irq = 0, use should manually update extcon state
+ * with extcon APIs.
+ * @irq_flags - irq flags used for the @irq
+ * @handling_delay_ms - in some devices, we need to read ADC value some
+ * milli-seconds after the interrupt occurs. You may
+ * describe such delays with @handling_delay_ms, which
+ * is rounded-off by jiffies.
+ * @get_adc - the callback to read ADC value to identify cable states.
+ */
+struct adc_jack_pdata {
+ const char *name;
+ /*
+ * NULL if standard extcon names are used.
+ * The last entry should be NULL
+ */
+ const char **cable_names;
+ /* The last entry's state should be 0 */
+ struct adc_jack_cond *adc_condition;
+
+ int irq; /* Jack insertion/removal interrupt */
+ unsigned long irq_flags;
+ unsigned long handling_delay_ms; /* in ms */
+
+ /* When we have ADC subsystem, this can be generalized. */
+ int (*get_adc)(u32 *value);
+};
+
+#endif /* _EXTCON_ADC_JACK_H */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices
2012-02-10 6:40 ` [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices MyungJoo Ham
@ 2012-02-10 16:25 ` Mark Brown
2012-02-14 2:22 ` MyungJoo Ham
0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-02-10 16:25 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel, myungjoo.ham
[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]
On Fri, Feb 10, 2012 at 03:40:38PM +0900, MyungJoo Ham wrote:
> External connector devices that decides connection information based on
> ADC values may use adc-jack device driver. The user simply needs to
> provide a table of adc range and connection states. Then, extcon
> framework will automatically notify others.
This really should be done in terms of the IIO in-kernel framework.
> + /* Check the length of array and set num_cables */
> + for (i = 0; data->edev.supported_cable[i]; i++)
> + ;
> + if (i == 0 || i > SUPPORTED_CABLE_MAX) {
Can we not avoid the hard limit?
> + /* Check the length of array and set num_conditions */
> + for (i = 0; data->adc_condition[i].state; i++)
> + ;
> + data->num_conditions = i;
It'd seem nicer to just specify the length in the parameters, less error
prone.
> + if (pdata->irq) {
> + data->irq = pdata->irq;
> +
> + err = request_threaded_irq(data->irq, NULL,
> + adc_jack_irq_thread,
> + pdata->irq_flags, pdata->name,
> + data);
Since all this does is schedule a delayed work there's no reason to
bother with a threaded IRQ and this could be request_any_context_irq().
> + err = extcon_dev_register(&data->edev, &pdev->dev);
> + if (err)
> + goto err_irq;
What happens if the interrupt fires prior to this...
> + data->ready = true;
...ah, that's what this is about. It'd seem cleaner to just reverse the
request and registration.
> + goto out;
More idiomatic to have a return statement here.
> +static int __devexit adc_jack_remove(struct platform_device *pdev)
> +{
> + struct adc_jack_data *data = platform_get_drvdata(pdev);
> +
> + extcon_dev_unregister(&data->edev);
> + if (data->irq)
> + free_irq(data->irq, data);
The interrupt needs to be freed prior to the device unregistration
otherwise they may race.
> +static int __init adc_jack_init(void)
> +{
> + return platform_driver_register(&adc_jack_driver);
> +}
> +
> +static void __exit adc_jack_exit(void)
> +{
> + platform_driver_unregister(&adc_jack_driver);
> +}
module_platform_driver().
> + unsigned long handling_delay; /* in jiffies */
I'd suggest calling this "debounce" instead, it seems more idiomatic.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices
2012-02-10 16:25 ` Mark Brown
@ 2012-02-14 2:22 ` MyungJoo Ham
2012-02-14 5:58 ` Mark Brown
0 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-14 2:22 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
On Sat, Feb 11, 2012 at 1:25 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Feb 10, 2012 at 03:40:38PM +0900, MyungJoo Ham wrote:
>> External connector devices that decides connection information based on
>> ADC values may use adc-jack device driver. The user simply needs to
>> provide a table of adc range and connection states. Then, extcon
>> framework will automatically notify others.
>
> This really should be done in terms of the IIO in-kernel framework.
The ADC part may be done in IIO. However, the intention of this device
driver is to provide extcon interface to any ADC drivers, not
providing an ADC device driver. If we are going to implement this in
the ADC driver in IIO, we will need to write the given code in every
ADC driver used for analog ports.
>
>> + /* Check the length of array and set num_cables */
>> + for (i = 0; data->edev.supported_cable[i]; i++)
>> + ;
>> + if (i == 0 || i > SUPPORTED_CABLE_MAX) {
>
> Can we not avoid the hard limit?
Without that limit, we won't be able to easily express binary cable
status (u32) with the extcon framework. At least, we will need to
forget about setting the status with u32 values.
Anyway, I can remove the checking SUPPORT_CABLE_MAX part at probe.
>
>> + /* Check the length of array and set num_conditions */
>> + for (i = 0; data->adc_condition[i].state; i++)
>> + ;
>> + data->num_conditions = i;
>
> It'd seem nicer to just specify the length in the parameters, less error
> prone.
Whether to put a NULL at the end or to provide a size variable seems
quite confusing in Linux kernel. Some use the former, some use the
latter. Which one is sort of "standard" now? I felt that the former is
being more popular these days, anyway.
>
>> + if (pdata->irq) {
>> + data->irq = pdata->irq;
>> +
>> + err = request_threaded_irq(data->irq, NULL,
>> + adc_jack_irq_thread,
>> + pdata->irq_flags, pdata->name,
>> + data);
>
> Since all this does is schedule a delayed work there's no reason to
> bother with a threaded IRQ and this could be request_any_context_irq().
Thanks. I'll update it.
>
>> + err = extcon_dev_register(&data->edev, &pdev->dev);
>> + if (err)
>> + goto err_irq;
>
> What happens if the interrupt fires prior to this...
>
>> + data->ready = true;
>
> ...ah, that's what this is about. It'd seem cleaner to just reverse the
> request and registration.
If it goes to err_irq, it reverses the request of IRQ. data->ready is
there not to react to a fired IRQ before free-ing the IRQ.
>
>> + goto out;
>
> More idiomatic to have a return statement here.
Yup.
>
>> +static int __devexit adc_jack_remove(struct platform_device *pdev)
>> +{
>> + struct adc_jack_data *data = platform_get_drvdata(pdev);
>> +
>> + extcon_dev_unregister(&data->edev);
>> + if (data->irq)
>> + free_irq(data->irq, data);
>
> The interrupt needs to be freed prior to the device unregistration
> otherwise they may race.
Yes. Thanks.
>
>> +static int __init adc_jack_init(void)
>> +{
>> + return platform_driver_register(&adc_jack_driver);
>> +}
>> +
>> +static void __exit adc_jack_exit(void)
>> +{
>> + platform_driver_unregister(&adc_jack_driver);
>> +}
>
> module_platform_driver().
Oh.. this is good. :)
>
>> + unsigned long handling_delay; /* in jiffies */
>
> I'd suggest calling this "debounce" instead, it seems more idiomatic.
Yes, it seems so. I'll fix it.
Thank you.
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices
2012-02-14 2:22 ` MyungJoo Ham
@ 2012-02-14 5:58 ` Mark Brown
0 siblings, 0 replies; 50+ messages in thread
From: Mark Brown @ 2012-02-14 5:58 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, NeilBrown, Randy Dunlap, Mike Lockwood,
Arve Hjønnevag, Kyungmin Park, Donggeun Kim, Greg KH,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
On Tue, Feb 14, 2012 at 11:22:14AM +0900, MyungJoo Ham wrote:
> On Sat, Feb 11, 2012 at 1:25 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Fri, Feb 10, 2012 at 03:40:38PM +0900, MyungJoo Ham wrote:
> >> External connector devices that decides connection information based on
> >> ADC values may use adc-jack device driver. The user simply needs to
> >> provide a table of adc range and connection states. Then, extcon
> >> framework will automatically notify others.
> > This really should be done in terms of the IIO in-kernel framework.
> The ADC part may be done in IIO. However, the intention of this device
> driver is to provide extcon interface to any ADC drivers, not
> providing an ADC device driver. If we are going to implement this in
Right, exactly.
> the ADC driver in IIO, we will need to write the given code in every
> ADC driver used for analog ports.
No, that's not what I'm suggesting - what I'm suggesting is that rather
than having a callback for implementing the ADC read functionality this
should work as an in-kernel IIO driver so it'll just automatically work
with any ADC without needing code to hook things up. Unless I've not
understood your comment fully.
> >> + /* Check the length of array and set num_cables */
> >> + for (i = 0; data->edev.supported_cable[i]; i++)
> >> + ;
> >> + if (i == 0 || i > SUPPORTED_CABLE_MAX) {
> > Can we not avoid the hard limit?
> Without that limit, we won't be able to easily express binary cable
> status (u32) with the extcon framework. At least, we will need to
> forget about setting the status with u32 values.
> Anyway, I can remove the checking SUPPORT_CABLE_MAX part at probe.
It might be clearer to make the limit more obviously associated with
the bitmask - it looks like it's an array thing the way the code is
written but a limit due to using a bitmask seems reasonable.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 0/5] Introduce External Connector Class (extcon)
2012-02-10 6:40 ` [PATCH v5 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
` (4 preceding siblings ...)
2012-02-10 6:40 ` [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices MyungJoo Ham
@ 2012-02-27 12:15 ` MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
` (4 more replies)
5 siblings, 5 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 12:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, gregkh, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown, John Stultz,
Joerg Roedel, myungjoo.ham
For external connectors, which may have different types of cables attached
(USB, TA, HDMI, Analog A/V, and others), we often have seperated device
drivers that detect the state changes at the port and device drivers that
do something according to the state changes.
An example is a 4-pin (or more pins?) analog audio connector
(usually referred as "3.5-pi" jacks). It can be implemented with the 5/5
patch, "adc-jack", supplied with information about IRQ, a method to get
ADC values, and ADC value ranges. With a 4-pin 3.5-pi port and adc-jack
driver with proper ADC configurations, the following modes may be
detected: "standard mono cable", "standard stereo cable",
"standard stereo + remote cable", "standard stereo + mic + remote cable",
"standard stereo + different-spec mic + remote cable", and such.
Note that for 4-pin cables with mic and remote, there are multiple
standards (at least Samsung and Apple use different configurations for
those 4-pin earphones although the pins looks exactly same.)
and they often can be distinguished with ADC values.
Although we may have simply given different "cable id" for "stereo" and
"stereo + mic". However, for those userspace processes or other device
drivers that are interested only in "stereo" part of possible combinations,
having seperated mode/id for "stereo" and "mic" is more desirable.
For example, if we do not have multistate capabilities, those who want
to know "stereo" is attached or not should monitor everything:
"standard stereo cable", "standard stereo + remove cable",
"standard stereo + mic + remote cable", and others. However, with
multistate capabilities, they only need to monitor the bit associated
with "standard stero".
For another example, when MAX8997-MUIC detects a Charger cable insertion,
another device driver (such as MAX8903 charger, MAX8997 charger, Charger
Manager, or board file) needs to set charger current limit accordingly
and when MAX8997-MUIC detects a HDMI cable insertion, multimedia device
drivers need to do some operations accordingly.
This patchset supports the usage of notifier for passing such information
between device drivers.
Another issue is that at a single switch port, there might be multiple
and heterogeneous cables attached at the same time. Besides the state
(Attached or Detached) of each cable may alter independently. Such extcon
devices that support simultaneous mult-cable attachment include, bot not
limited to, docks, cradles, and 30-pin-like ports (like that of NURI board
in linux/arch/arm/mach-exynos, which resembles Samsung Galaxy Tab series).
For example, the 30-pin port of NURI board may be used to connect analog
audio cables, analog video cable, and a USB or a charger. Because such
cables coonnect part of the 30 pins to external devices/cables, the
possible combinations of external devices/cables vary significantly and
users may fabricate their own design; e.g., an 30-pin adaptor connecting
USB, AC-DC adaptor, HDMI, analog audio, analog video, USB-host, SD-card,
and so on.
In order to address such issues, Android kernel's "Switch" class seems to
be a good basis and we have implemented "Multistate Switch Class" based on
it. The "Switch" class code of Android kernel is GPL as well.
Summary of changes from v5 patchset
- Excluded adc-jack from the "default" patchset. Adc-jack will be later
added after Extcon is finalized.
- Sysfs style updated
- Bugfixes after using at Exynos4412 boards with userspace processes
- Code stype updated
Summary of changes from v4 patchset
- Bugfixes after testing at Exynos4412 boards with userspace processes
- Added adc-jack to support 3.5-pi jack and other ADC-based cable detection
ports.
Summary of changes from v3 patchset
- Bugfixes after testing at Exynos4412 boards.
Summary of changes from v2 patchset
- Support to express mutually exclusive set of cables
- Revised device/class/sysfs-entries add/removal methods
- Added sysfs entries
- Revised documentation and comments
- Some bugfixes
- Compatible with Android (for extcon devices with "use_class_name_switch" set)
Summary of changes from RFC
- ABI documentation added
- Notifees may get notified for a state change of a specific cable, not
every cable of the corresponding extcon.
- Added kerneldoc comments
- Moved to /drivers/extcon/
- Added helper functions
- Some bugfixes
Donggeun Kim (1):
Extcon: support notification based on the state changes.
MyungJoo Ham (4):
Extcon (external connector): import Android's switch class and
modify.
Extcon: support generic GPIO extcon driver
Extcon: support multiple states at a device.
Extcon: support mutually exclusive relation between cables.
Documentation/ABI/testing/sysfs-class-extcon | 97 +++
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/extcon/Kconfig | 35 ++
drivers/extcon/Makefile | 6 +
drivers/extcon/extcon_class.c | 812 ++++++++++++++++++++++++++
drivers/extcon/extcon_gpio.c | 175 ++++++
include/linux/extcon.h | 322 ++++++++++
include/linux/extcon/extcon_gpio.h | 52 ++
9 files changed, 1502 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-extcon
create mode 100644 drivers/extcon/Kconfig
create mode 100644 drivers/extcon/Makefile
create mode 100644 drivers/extcon/extcon_class.c
create mode 100644 drivers/extcon/extcon_gpio.c
create mode 100644 include/linux/extcon.h
create mode 100644 include/linux/extcon/extcon_gpio.h
--
1.7.4.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
@ 2012-02-27 12:15 ` MyungJoo Ham
2012-03-09 12:41 ` Mark Brown
2012-02-27 12:15 ` [PATCH v6 2/5] Extcon: support generic GPIO extcon driver MyungJoo Ham
` (3 subsequent siblings)
4 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 12:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, gregkh, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown, John Stultz,
Joerg Roedel, myungjoo.ham
External connector class (extcon) is based on and an extension of
Android kernel's switch class located at linux/drivers/switch/.
This patch provides the before-extension switch class moved to the
location where the extcon will be located (linux/drivers/extcon/) and
updates to handle class properly.
The before-extension class, switch class of Android kernel, commits
imported are:
switch: switch class and GPIO drivers. (splitted)
Author: Mike Lockwood <lockwood@android.com>
switch: Use device_create instead of device_create_drvdata.
Author: Arve Hjønnevåg <arve@android.com>
In this patch, upon the commits of Android kernel, we have added:
- Relocated and renamed for extcon.
- Comments, module name, and author information are updated
- Code clean for successing patches
- Bugfix: enabling write access without write functions
- Class/device/sysfs create/remove handling
- Added comments about uevents
- Format changes for extcon_dev_register() to have a parent dev.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changes from v5
- Split the patch
- Style fixes
- "Android-compatible" mode is enabled by Kconfig option.
Changes from v2
- Updated name_show
- Sysfs entries are handled by class itself.
- Updated the method to add/remove devices for the class
- Comments on uevent send
- Able to become a module
- Compatible with Android platform
Changes from RFC
- Renamed to extcon (external connector) from multistate switch
- Added a seperated directory (drivers/extcon)
- Added kerneldoc comments
- Removed unused variables from extcon_gpio.c
- Added ABI Documentation.
---
Documentation/ABI/testing/sysfs-class-extcon | 26 +++
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/extcon/Kconfig | 28 ++++
drivers/extcon/Makefile | 5 +
drivers/extcon/extcon_class.c | 226 ++++++++++++++++++++++++++
include/linux/extcon.h | 82 ++++++++++
7 files changed, 370 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-extcon
create mode 100644 drivers/extcon/Kconfig
create mode 100644 drivers/extcon/Makefile
create mode 100644 drivers/extcon/extcon_class.c
create mode 100644 include/linux/extcon.h
diff --git a/Documentation/ABI/testing/sysfs-class-extcon b/Documentation/ABI/testing/sysfs-class-extcon
new file mode 100644
index 0000000..59a4b1c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-extcon
@@ -0,0 +1,26 @@
+What: /sys/class/extcon/.../
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ Provide a place in sysfs for the extcon objects.
+ This allows accessing extcon specific variables.
+ The name of extcon object denoted as ... is the name given
+ with extcon_dev_register.
+
+What: /sys/class/extcon/.../name
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ The /sys/class/extcon/.../name shows the name of the extcon
+ object. If the extcon object has an optional callback
+ "show_name" defined, the callback will provide the name with
+ this sysfs node.
+
+What: /sys/class/extcon/.../state
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ The /sys/class/extcon/.../state shows the cable attach/detach
+ information of the corresponding extcon object. If the extcon
+ objecct has an optional callback "show_state" defined, the
+ callback will provide the name with this sysfs node.
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 5afe5d1..537d126 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -136,4 +136,6 @@ source "drivers/virt/Kconfig"
source "drivers/devfreq/Kconfig"
+source "drivers/extcon/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index c07be02..6faceab 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -132,3 +132,4 @@ obj-$(CONFIG_VIRT_DRIVERS) += virt/
obj-$(CONFIG_HYPERV) += hv/
obj-$(CONFIG_PM_DEVFREQ) += devfreq/
+obj-$(CONFIG_EXTCON) += extcon/
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
new file mode 100644
index 0000000..11db865
--- /dev/null
+++ b/drivers/extcon/Kconfig
@@ -0,0 +1,28 @@
+menuconfig EXTCON
+ tristate "External Connector Class (extcon) support"
+ help
+ Say Y here to enable external connector class (extcon) support.
+ This allows monitoring external connectors by userspace
+ via sysfs and uevent and supports external connectors with
+ multiple states; i.e., an extcon that may have multiple
+ cables attached. For example, an external connector of a device
+ may be used to connect an HDMI cable and a AC adaptor, and to
+ host USB ports. Many of 30-pin connectors including PDMI are
+ also good examples.
+
+if EXTCON
+
+comment "Extcon Class Configuration"
+
+config EXTCON_ANDROID_COMPATIBLE
+ bool "Class name uses 'switch' to be compatible with Android kernel"
+ depends on ANDROID
+ help
+ The class name of extcon device drivers are set 'switch' in order
+ to be compatible with Android kernel switch framework. With this
+ option enabled, Android userspace platform may use extcon device
+ drivers as if they are switch device drivers.
+
+comment "Extcon Device Drivers"
+
+endif # MULTISTATE_SWITCH
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
new file mode 100644
index 0000000..6bc6921
--- /dev/null
+++ b/drivers/extcon/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for external connector class (extcon) devices
+#
+
+obj-$(CONFIG_EXTCON) += extcon_class.o
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
new file mode 100644
index 0000000..9bc062b
--- /dev/null
+++ b/drivers/extcon/extcon_class.c
@@ -0,0 +1,226 @@
+/*
+ * drivers/extcon/extcon_class.c
+ *
+ * External connector (extcon) class driver
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Author: Donggeun Kim <dg77.kim@samsung.com>
+ * Author: MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * based on android/drivers/switch/switch_class.c
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/module.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/slab.h>
+
+struct class *extcon_class;
+
+static ssize_t state_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ if (edev->print_state) {
+ int ret = edev->print_state(edev, buf);
+
+ if (ret >= 0)
+ return ret;
+ /* Use default if failed */
+ }
+ return sprintf(buf, "%u\n", edev->state);
+}
+
+static ssize_t name_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ /* Optional callback given by the user */
+ if (edev->print_name) {
+ int ret = edev->print_name(edev, buf);
+ if (ret >= 0)
+ return ret;
+ }
+
+ return sprintf(buf, "%s\n", dev_name(edev->dev));
+}
+
+/**
+ * extcon_set_state() - Set the cable attach states of the extcon device.
+ * @edev: the extcon device
+ * @state: new cable attach status for @edev
+ *
+ * Changing the state sends uevent with environment variable containing
+ * the name of extcon device (envp[0]) and the state output (envp[1]).
+ * Tizen uses this format for extcon device to get events from ports.
+ * Android uses this format as well.
+ */
+void extcon_set_state(struct extcon_dev *edev, u32 state)
+{
+ char name_buf[120];
+ char state_buf[120];
+ char *prop_buf;
+ char *envp[3];
+ int env_offset = 0;
+ int length;
+
+ if (edev->state != state) {
+ edev->state = state;
+
+ prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
+ if (prop_buf) {
+ length = name_show(edev->dev, NULL, prop_buf);
+ if (length > 0) {
+ if (prop_buf[length - 1] == '\n')
+ prop_buf[length - 1] = 0;
+ snprintf(name_buf, sizeof(name_buf),
+ "SWITCH_NAME=%s", prop_buf);
+ envp[env_offset++] = name_buf;
+ }
+ length = state_show(edev->dev, NULL, prop_buf);
+ if (length > 0) {
+ if (prop_buf[length - 1] == '\n')
+ prop_buf[length - 1] = 0;
+ snprintf(state_buf, sizeof(state_buf),
+ "SWITCH_STATE=%s", prop_buf);
+ envp[env_offset++] = state_buf;
+ }
+ envp[env_offset] = NULL;
+ kobject_uevent_env(&edev->dev->kobj, KOBJ_CHANGE, envp);
+ free_page((unsigned long)prop_buf);
+ } else {
+ dev_err(edev->dev, "out of memory in extcon_set_state\n");
+ kobject_uevent(&edev->dev->kobj, KOBJ_CHANGE);
+ }
+ }
+}
+EXPORT_SYMBOL_GPL(extcon_set_state);
+
+static struct device_attribute extcon_attrs[] = {
+ __ATTR_RO(state),
+ __ATTR_RO(name),
+};
+
+static int create_extcon_class(void)
+{
+ if (!extcon_class) {
+#ifdef CONFIG_ANDROID
+ extcon_class = class_create(THIS_MODULE, "switch");
+#else /* !CONFIG_ANDROID */
+ extcon_class = class_create(THIS_MODULE, "extcon");
+#endif /* CONFIG_ANDROID */
+ if (IS_ERR(extcon_class))
+ return PTR_ERR(extcon_class);
+ extcon_class->dev_attrs = extcon_attrs;
+ }
+
+ return 0;
+}
+
+static void extcon_cleanup(struct extcon_dev *edev, bool skip)
+{
+ if (!skip && get_device(edev->dev)) {
+ device_unregister(edev->dev);
+ put_device(edev->dev);
+ }
+
+ kfree(edev->dev);
+}
+
+static void extcon_dev_release(struct device *dev)
+{
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ extcon_cleanup(edev, true);
+}
+
+/**
+ * extcon_dev_register() - Register a new extcon device
+ * @edev : the new extcon device (should be allocated before calling)
+ * @dev : the parent device for this extcon device.
+ *
+ * Among the members of edev struct, please set the "user initializing data"
+ * in any case and set the "optional callbacks" if required. However, please
+ * do not set the values of "internal data", which are initialized by
+ * this function.
+ */
+int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
+{
+ int ret;
+
+ if (!extcon_class) {
+ ret = create_extcon_class();
+ if (ret < 0)
+ return ret;
+ }
+
+ edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+ edev->dev->parent = dev;
+ edev->dev->class = extcon_class;
+ edev->dev->release = extcon_dev_release;
+
+ dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+ ret = device_register(edev->dev);
+ if (ret) {
+ put_device(edev->dev);
+ goto err_dev;
+ }
+
+ dev_set_drvdata(edev->dev, edev);
+ edev->state = 0;
+ return 0;
+
+err_dev:
+ kfree(edev->dev);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(extcon_dev_register);
+
+/**
+ * extcon_dev_unregister() - Unregister the extcon device.
+ * @edev: the extcon device instance to be unregitered.
+ *
+ * Note that this does not call kfree(edev) because edev was not allocated
+ * by this class.
+ */
+void extcon_dev_unregister(struct extcon_dev *edev)
+{
+ extcon_cleanup(edev, false);
+}
+EXPORT_SYMBOL_GPL(extcon_dev_unregister);
+
+static int __init extcon_class_init(void)
+{
+ return create_extcon_class();
+}
+module_init(extcon_class_init);
+
+static void __exit extcon_class_exit(void)
+{
+ class_destroy(extcon_class);
+}
+module_exit(extcon_class_exit);
+
+MODULE_AUTHOR("Mike Lockwood <lockwood@android.com>");
+MODULE_AUTHOR("Donggeun Kim <dg77.kim@samsung.com>");
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_DESCRIPTION("External connector (extcon) class driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
new file mode 100644
index 0000000..cd732ab
--- /dev/null
+++ b/include/linux/extcon.h
@@ -0,0 +1,82 @@
+/*
+ * External connector (extcon) class driver
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Author: Donggeun Kim <dg77.kim@samsung.com>
+ * Author: MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * based on switch class driver
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+*/
+
+#ifndef __LINUX_EXTCON_H__
+#define __LINUX_EXTCON_H__
+
+/**
+ * struct extcon_dev - An extcon device represents one external connector.
+ * @name The name of this extcon device. Parent device name is used
+ * if NULL.
+ * @print_name An optional callback to override the method to print the
+ * name of the extcon device.
+ * @print_state An optional callback to override the method to print the
+ * status of the extcon device.
+ * @dev Device of this extcon. Do not provide at register-time.
+ * @state Attach/detach state of this extcon. Do not provide at
+ * register-time
+ *
+ * In most cases, users only need to provide "User initializing data" of
+ * this struct when registering an extcon. In some exceptional cases,
+ * optional callbacks may be needed. However, the values in "internal data"
+ * are overwritten by register function.
+ */
+struct extcon_dev {
+ /* --- Optional user initializing data --- */
+ const char *name;
+
+ /* --- Optional callbacks to override class functions --- */
+ ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
+ ssize_t (*print_state)(struct extcon_dev *edev, char *buf);
+
+ /* --- Internal data. Please do not set. --- */
+ struct device *dev;
+ u32 state;
+};
+
+#ifdef CONFIG_EXTCON
+extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
+extern void extcon_dev_unregister(struct extcon_dev *edev);
+
+static inline u32 extcon_get_state(struct extcon_dev *edev)
+{
+ return edev->state;
+}
+
+extern void extcon_set_state(struct extcon_dev *edev, u32 state);
+#else /* CONFIG_EXTCON */
+static inline int extcon_dev_register(struct extcon_dev *edev,
+ struct device *dev)
+{
+ return 0;
+}
+
+static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
+
+static inline u32 extcon_get_state(struct extcon_dev *edev)
+{
+ return 0;
+}
+
+static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+#endif /* CONFIG_EXTCON */
+#endif /* __LINUX_EXTCON_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-02-27 12:15 ` [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
@ 2012-03-09 12:41 ` Mark Brown
2012-03-12 8:06 ` MyungJoo Ham
2012-03-29 22:27 ` Erik Gilling
0 siblings, 2 replies; 50+ messages in thread
From: Mark Brown @ 2012-03-09 12:41 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, linux-arm-kernel, NeilBrown, Randy Dunlap,
Mike Lockwood, Arve Hjønnevag, Kyungmin Park, gregkh,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel, myungjoo.ham
[-- Attachment #1: Type: text/plain, Size: 583 bytes --]
On Mon, Feb 27, 2012 at 09:15:35PM +0900, MyungJoo Ham wrote:
> +#ifdef CONFIG_ANDROID
> + extcon_class = class_create(THIS_MODULE, "switch");
> +#else /* !CONFIG_ANDROID */
> + extcon_class = class_create(THIS_MODULE, "extcon");
> +#endif /* CONFIG_ANDROID */
This seems somewhat sad - if ANDROID is turned on the standard ABI
vanishes. It'd be much nicer to do this with a symlink (or with
symlinks within the android directory if the driver core doesn't support
that). That way userspace code can be written to the new ABI and will
work on Android systems without ifdefery.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-09 12:41 ` Mark Brown
@ 2012-03-12 8:06 ` MyungJoo Ham
2012-03-29 22:27 ` Erik Gilling
1 sibling, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-03-12 8:06 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, linux-arm-kernel, NeilBrown, Randy Dunlap,
Mike Lockwood, Arve Hjønnevag, Kyungmin Park, gregkh,
Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Morten CHRISTIANSEN, John Stultz, Joerg Roedel
On Fri, Mar 9, 2012 at 9:41 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Feb 27, 2012 at 09:15:35PM +0900, MyungJoo Ham wrote:
>
>> +#ifdef CONFIG_ANDROID
>> + extcon_class = class_create(THIS_MODULE, "switch");
>> +#else /* !CONFIG_ANDROID */
>> + extcon_class = class_create(THIS_MODULE, "extcon");
>> +#endif /* CONFIG_ANDROID */
>
> This seems somewhat sad - if ANDROID is turned on the standard ABI
> vanishes. It'd be much nicer to do this with a symlink (or with
> symlinks within the android directory if the driver core doesn't support
> that). That way userspace code can be written to the new ABI and will
> work on Android systems without ifdefery.
Yes, this was the itching part.
I'll first look into creating symlinks (either /sys/class/switch -->
/sys/class/extcon or /sys/class/switch/* --> /sys/class/extcon/*).
However, I think this symbolic links are better created selectively w/
CONFIG_ANDROID.
Thanks.
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-09 12:41 ` Mark Brown
2012-03-12 8:06 ` MyungJoo Ham
@ 2012-03-29 22:27 ` Erik Gilling
2012-03-30 8:56 ` MyungJoo Ham
2012-03-30 10:07 ` Mark Brown
1 sibling, 2 replies; 50+ messages in thread
From: Erik Gilling @ 2012-03-29 22:27 UTC (permalink / raw)
To: Mark Brown
Cc: MyungJoo Ham, Arnd Bergmann, NeilBrown, gregkh, Linus Walleij,
Dmitry Torokhov, Arve Hjønnevag, linux-kernel, Randy Dunlap,
myungjoo.ham, John Stultz, linux-arm-kernel, Joerg Roedel,
Kyungmin Park, Morten CHRISTIANSEN, Mike Lockwood
On Fri, Mar 9, 2012 at 4:41 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Feb 27, 2012 at 09:15:35PM +0900, MyungJoo Ham wrote:
>
>> +#ifdef CONFIG_ANDROID
>> + extcon_class = class_create(THIS_MODULE, "switch");
>> +#else /* !CONFIG_ANDROID */
>> + extcon_class = class_create(THIS_MODULE, "extcon");
>> +#endif /* CONFIG_ANDROID */
>
> This seems somewhat sad - if ANDROID is turned on the standard ABI
> vanishes. It'd be much nicer to do this with a symlink (or with
> symlinks within the android directory if the driver core doesn't support
> that). That way userspace code can be written to the new ABI and will
> work on Android systems without ifdefery.
This won't work if userspace code is receiving uevents through netlink
and comparing based on the device name which it does in android. Why
change the name at all? Extcon is much more specific than "switch."
A switch can can be used for more than just external connections such
as availability of HMDI audio (which is only available once a HDMI
mode is chosen and the interface is enabled.)
-Erik
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-29 22:27 ` Erik Gilling
@ 2012-03-30 8:56 ` MyungJoo Ham
2012-03-30 10:14 ` Mark Brown
2012-03-30 10:07 ` Mark Brown
1 sibling, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-03-30 8:56 UTC (permalink / raw)
To: Erik Gilling
Cc: Mark Brown, Arnd Bergmann, NeilBrown, gregkh, Linus Walleij,
Dmitry Torokhov, Arve Hjønnevag, linux-kernel, Randy Dunlap,
John Stultz, linux-arm-kernel, Joerg Roedel, Kyungmin Park,
Morten CHRISTIANSEN, Mike Lockwood
On Fri, Mar 30, 2012 at 7:27 AM, Erik Gilling <konkers@google.com> wrote:
> On Fri, Mar 9, 2012 at 4:41 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> On Mon, Feb 27, 2012 at 09:15:35PM +0900, MyungJoo Ham wrote:
>>
>>> +#ifdef CONFIG_ANDROID
>>> + extcon_class = class_create(THIS_MODULE, "switch");
>>> +#else /* !CONFIG_ANDROID */
>>> + extcon_class = class_create(THIS_MODULE, "extcon");
>>> +#endif /* CONFIG_ANDROID */
>>
>> This seems somewhat sad - if ANDROID is turned on the standard ABI
>> vanishes. It'd be much nicer to do this with a symlink (or with
>> symlinks within the android directory if the driver core doesn't support
>> that). That way userspace code can be written to the new ABI and will
>> work on Android systems without ifdefery.
>
> This won't work if userspace code is receiving uevents through netlink
> and comparing based on the device name which it does in android. Why
> change the name at all? Extcon is much more specific than "switch."
> A switch can can be used for more than just external connections such
> as availability of HMDI audio (which is only available once a HDMI
> mode is chosen and the interface is enabled.)
>
> -Erik
With class_compat as in patchset v7, (this thread is w/ v6) does the
issue (missing link from device name directory) still exist? (I don't
think so)
In the earlier iteration of dicussion, the name "switch" was dropped
as "switch" may mislead the concept.
https://lkml.org/lkml/2011/11/29/299
For HDMI and HDMI-audio, although HDMI-audio is not physically
seperated from HDMI-cable and it is available after HDMI-cable is
"attached", HDMI-audio is still an external connection to the kernel
and its device drivers, isn' it?
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-30 8:56 ` MyungJoo Ham
@ 2012-03-30 10:14 ` Mark Brown
0 siblings, 0 replies; 50+ messages in thread
From: Mark Brown @ 2012-03-30 10:14 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Erik Gilling, Arnd Bergmann, NeilBrown, gregkh, Linus Walleij,
Dmitry Torokhov, Arve Hjønnevag, linux-kernel, Randy Dunlap,
John Stultz, linux-arm-kernel, Joerg Roedel, Kyungmin Park,
Morten CHRISTIANSEN, Mike Lockwood
[-- Attachment #1: Type: text/plain, Size: 270 bytes --]
On Fri, Mar 30, 2012 at 05:56:01PM +0900, MyungJoo Ham wrote:
> With class_compat as in patchset v7, (this thread is w/ v6) does the
> issue (missing link from device name directory) still exist? (I don't
> think so)
Not unless the uevents are issued with both names.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-29 22:27 ` Erik Gilling
2012-03-30 8:56 ` MyungJoo Ham
@ 2012-03-30 10:07 ` Mark Brown
2012-03-30 17:29 ` Erik Gilling
1 sibling, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-03-30 10:07 UTC (permalink / raw)
To: Erik Gilling
Cc: MyungJoo Ham, Arnd Bergmann, NeilBrown, gregkh, Linus Walleij,
Dmitry Torokhov, Arve Hjønnevag, linux-kernel, Randy Dunlap,
myungjoo.ham, John Stultz, linux-arm-kernel, Joerg Roedel,
Kyungmin Park, Morten CHRISTIANSEN, Mike Lockwood
[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]
On Thu, Mar 29, 2012 at 03:27:01PM -0700, Erik Gilling wrote:
> On Fri, Mar 9, 2012 at 4:41 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Mon, Feb 27, 2012 at 09:15:35PM +0900, MyungJoo Ham wrote:
> >> +#ifdef CONFIG_ANDROID
> >> + extcon_class = class_create(THIS_MODULE, "switch");
> >> +#else /* !CONFIG_ANDROID */
> >> + extcon_class = class_create(THIS_MODULE, "extcon");
> >> +#endif /* CONFIG_ANDROID */
> > This seems somewhat sad - if ANDROID is turned on the standard ABI
> > vanishes. It'd be much nicer to do this with a symlink (or with
> > symlinks within the android directory if the driver core doesn't support
> > that). That way userspace code can be written to the new ABI and will
> > work on Android systems without ifdefery.
> This won't work if userspace code is receiving uevents through netlink
> and comparing based on the device name which it does in android. Why
That's not really the point here - the point is that the new ABI
vanishes as soon as you turn on the legacy Android ABI. You're right
that the particular fix I suggested has issues but the overall problem
exists and should be dealt with more sensibly.
> change the name at all? Extcon is much more specific than "switch."
> A switch can can be used for more than just external connections such
> as availability of HMDI audio (which is only available once a HDMI
> mode is chosen and the interface is enabled.)
I don't know myself, it was called extcon when first proposed for
mainline. Renaming does have the advantage of indicating to userspace
that this is the mainline, reviewed version and that things might be a
bit different as a result of that review.
Things like the HDMI audio example you cite do seem close enough to the
term to still be useful, they're related to an external connection and
it's state. If you were really worrying about it you might expect HDMI
audio to be reported via something like the ALSA APIs rather than a
totally unrelated ABI like this.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-30 10:07 ` Mark Brown
@ 2012-03-30 17:29 ` Erik Gilling
2012-03-30 17:38 ` Dima Zavin
2012-03-31 10:19 ` Mark Brown
0 siblings, 2 replies; 50+ messages in thread
From: Erik Gilling @ 2012-03-30 17:29 UTC (permalink / raw)
To: Mark Brown
Cc: MyungJoo Ham, Arnd Bergmann, NeilBrown, gregkh, Linus Walleij,
Dmitry Torokhov, Arve Hjønnevag, linux-kernel, Randy Dunlap,
myungjoo.ham, John Stultz, linux-arm-kernel, Joerg Roedel,
Kyungmin Park, Morten CHRISTIANSEN, Mike Lockwood, Dima Zavin
On Fri, Mar 30, 2012 at 3:07 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Mar 29, 2012 at 03:27:01PM -0700, Erik Gilling wrote:
>> On Fri, Mar 9, 2012 at 4:41 AM, Mark Brown
>
>> > This seems somewhat sad - if ANDROID is turned on the standard ABI
>> > vanishes. It'd be much nicer to do this with a symlink (or with
>> > symlinks within the android directory if the driver core doesn't support
>> > that). That way userspace code can be written to the new ABI and will
>> > work on Android systems without ifdefery.
>
>> This won't work if userspace code is receiving uevents through netlink
>> and comparing based on the device name which it does in android. Why
>
> That's not really the point here - the point is that the new ABI
> vanishes as soon as you turn on the legacy Android ABI. You're right
> that the particular fix I suggested has issues but the overall problem
> exists and should be dealt with more sensibly.
I'm also not in favor of having functionality conditionally compiled
based on CONFIG_ANDROID. If fact, going through the patch stack
there's much more that changes the ABI from the switch driver than
just the name. Android asumes that there is a single "switch" for
each logical entity (called cable types in extcon) each with a binary
state (0,1). Here things have changed to have a single extcon
instance that can have a bitmask of states which are sent as strings
in the uevent.
As it stands, this patch does not solve the cases where we use switch
today and we'll probably continue to carry the switch driver in the
common android tree. If, instead, we got rid of the idea of multiple
states and mutual exclusivity and relied on the driver that uses
extcon to have multiple instances for each logical entity and deal
with mutual exclusion itself, we'd have a driver that would be pretty
easy to support in android.
-Erik
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-30 17:29 ` Erik Gilling
@ 2012-03-30 17:38 ` Dima Zavin
2012-04-02 5:09 ` MyungJoo Ham
2012-03-31 10:19 ` Mark Brown
1 sibling, 1 reply; 50+ messages in thread
From: Dima Zavin @ 2012-03-30 17:38 UTC (permalink / raw)
To: Erik Gilling
Cc: Mark Brown, MyungJoo Ham, Arnd Bergmann, NeilBrown, gregkh,
Linus Walleij, Dmitry Torokhov, Arve Hjønnevag,
linux-kernel, Randy Dunlap, myungjoo.ham, John Stultz,
linux-arm-kernel, Joerg Roedel, Kyungmin Park,
Morten CHRISTIANSEN, Mike Lockwood
On Fri, Mar 30, 2012 at 10:29 AM, Erik Gilling <konkers@google.com> wrote:
> On Fri, Mar 30, 2012 at 3:07 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> On Thu, Mar 29, 2012 at 03:27:01PM -0700, Erik Gilling wrote:
>>> On Fri, Mar 9, 2012 at 4:41 AM, Mark Brown
>>
>>> > This seems somewhat sad - if ANDROID is turned on the standard ABI
>>> > vanishes. It'd be much nicer to do this with a symlink (or with
>>> > symlinks within the android directory if the driver core doesn't support
>>> > that). That way userspace code can be written to the new ABI and will
>>> > work on Android systems without ifdefery.
>>
>>> This won't work if userspace code is receiving uevents through netlink
>>> and comparing based on the device name which it does in android. Why
>>
>> That's not really the point here - the point is that the new ABI
>> vanishes as soon as you turn on the legacy Android ABI. You're right
>> that the particular fix I suggested has issues but the overall problem
>> exists and should be dealt with more sensibly.
>
> I'm also not in favor of having functionality conditionally compiled
> based on CONFIG_ANDROID. If fact, going through the patch stack
> there's much more that changes the ABI from the switch driver than
> just the name. Android asumes that there is a single "switch" for
> each logical entity (called cable types in extcon) each with a binary
> state (0,1). Here things have changed to have a single extcon
> instance that can have a bitmask of states which are sent as strings
> in the uevent.
In addition to all this added complexity, the fact that you now have
to white-list the possible "cable types" is also undesirable. If I
wanted to expose a new cable type, I now have to modify the extcon.h
file and the extcon.c file and I don't see what that white-list buys
me. The class device itself should not care for which cable types can
be registered, nor how they relate to each other. Let the client
drivers decide, just enforce uniqueness.
--Dima
>
> As it stands, this patch does not solve the cases where we use switch
> today and we'll probably continue to carry the switch driver in the
> common android tree. If, instead, we got rid of the idea of multiple
> states and mutual exclusivity and relied on the driver that uses
> extcon to have multiple instances for each logical entity and deal
> with mutual exclusion itself, we'd have a driver that would be pretty
> easy to support in android.
>
> -Erik
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-30 17:38 ` Dima Zavin
@ 2012-04-02 5:09 ` MyungJoo Ham
0 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-04-02 5:09 UTC (permalink / raw)
To: Dima Zavin
Cc: Erik Gilling, Mark Brown, Arnd Bergmann, NeilBrown, gregkh,
Linus Walleij, Dmitry Torokhov, Arve Hjønnevag,
linux-kernel, Randy Dunlap, John Stultz, linux-arm-kernel,
Joerg Roedel, Kyungmin Park, Morten CHRISTIANSEN, Mike Lockwood
On Sat, Mar 31, 2012 at 2:38 AM, Dima Zavin <dima@android.com> wrote:
> On Fri, Mar 30, 2012 at 10:29 AM, Erik Gilling <konkers@google.com> wrote:
>> On Fri, Mar 30, 2012 at 3:07 AM, Mark Brown
>> <broonie@opensource.wolfsonmicro.com> wrote:
>>> On Thu, Mar 29, 2012 at 03:27:01PM -0700, Erik Gilling wrote:
>>>> On Fri, Mar 9, 2012 at 4:41 AM, Mark Brown
>>>
>>>> > This seems somewhat sad - if ANDROID is turned on the standard ABI
>>>> > vanishes. It'd be much nicer to do this with a symlink (or with
>>>> > symlinks within the android directory if the driver core doesn't support
>>>> > that). That way userspace code can be written to the new ABI and will
>>>> > work on Android systems without ifdefery.
>>>
>>>> This won't work if userspace code is receiving uevents through netlink
>>>> and comparing based on the device name which it does in android. Why
>>>
>>> That's not really the point here - the point is that the new ABI
>>> vanishes as soon as you turn on the legacy Android ABI. You're right
>>> that the particular fix I suggested has issues but the overall problem
>>> exists and should be dealt with more sensibly.
>>
>> I'm also not in favor of having functionality conditionally compiled
>> based on CONFIG_ANDROID. If fact, going through the patch stack
>> there's much more that changes the ABI from the switch driver than
>> just the name. Android asumes that there is a single "switch" for
>> each logical entity (called cable types in extcon) each with a binary
>> state (0,1). Here things have changed to have a single extcon
>> instance that can have a bitmask of states which are sent as strings
>> in the uevent.
>
> In addition to all this added complexity, the fact that you now have
> to white-list the possible "cable types" is also undesirable. If I
> wanted to expose a new cable type, I now have to modify the extcon.h
> file and the extcon.c file and I don't see what that white-list buys
> me. The class device itself should not care for which cable types can
> be registered, nor how they relate to each other. Let the client
> drivers decide, just enforce uniqueness.
>
> --Dima
They are not a white-list that shows the possible cable types.
It is a suggestion so that different device drivers may use the same
cable names for common cable types.
Any device drivers can still use their own "new" names without any
modifications to extcon_class.c./extcon.h. Especially as long as they
are "new", "rare", or "uncommon" cable types, the device drivers
should be able to use their own created names.
Cheers!
MyungJoo.
>
>>
>> As it stands, this patch does not solve the cases where we use switch
>> today and we'll probably continue to carry the switch driver in the
>> common android tree. If, instead, we got rid of the idea of multiple
>> states and mutual exclusivity and relied on the driver that uses
>> extcon to have multiple instances for each logical entity and deal
>> with mutual exclusion itself, we'd have a driver that would be pretty
>> easy to support in android.
>>
>> -Erik
--
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify.
2012-03-30 17:29 ` Erik Gilling
2012-03-30 17:38 ` Dima Zavin
@ 2012-03-31 10:19 ` Mark Brown
1 sibling, 0 replies; 50+ messages in thread
From: Mark Brown @ 2012-03-31 10:19 UTC (permalink / raw)
To: Erik Gilling
Cc: MyungJoo Ham, Arnd Bergmann, NeilBrown, gregkh, Linus Walleij,
Dmitry Torokhov, Arve Hjønnevag, linux-kernel, Randy Dunlap,
myungjoo.ham, John Stultz, linux-arm-kernel, Joerg Roedel,
Kyungmin Park, Morten CHRISTIANSEN, Mike Lockwood, Dima Zavin
[-- Attachment #1: Type: text/plain, Size: 2042 bytes --]
On Fri, Mar 30, 2012 at 10:29:27AM -0700, Erik Gilling wrote:
> I'm also not in favor of having functionality conditionally compiled
> based on CONFIG_ANDROID. If fact, going through the patch stack
> there's much more that changes the ABI from the switch driver than
> just the name. Android asumes that there is a single "switch" for
> each logical entity (called cable types in extcon) each with a binary
> state (0,1). Here things have changed to have a single extcon
> instance that can have a bitmask of states which are sent as strings
> in the uevent.
The binary status thing isn't true for the original Android switch API,
at least not in all cases - for headsets it's a tristate indicating if
there's nothing, a headphone or headset plugged in. This does wind up
being a bitmask because the numbers are 0, 1 and 2 but it isn't really
clear to me if this is intentional or just an effect of counting.
> As it stands, this patch does not solve the cases where we use switch
> today and we'll probably continue to carry the switch driver in the
> common android tree. If, instead, we got rid of the idea of multiple
> states and mutual exclusivity and relied on the driver that uses
> extcon to have multiple instances for each logical entity and deal
> with mutual exclusion itself, we'd have a driver that would be pretty
> easy to support in android.
What's the advantage in not having information about mutual exclusion in
the core, and why would a userspace that isn't interested in this care?
It seems like this could be used by userspace to do interesting things,
and I rather suspect you'll find some vendors have added features along
those lines in their Android devices. For example, having different
configurations for desk docks and car docks even if both end up
connecting the same things up.
Even if the userspace ABI doesn't expose the information it seems
sensible to factor out the code for handling this into the core so
there's less work to do in individual drivers.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 2/5] Extcon: support generic GPIO extcon driver
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
@ 2012-02-27 12:15 ` MyungJoo Ham
2012-03-29 22:37 ` Stephen Boyd
2012-02-27 12:15 ` [PATCH v6 3/5] Extcon: support notification based on the state changes MyungJoo Ham
` (2 subsequent siblings)
4 siblings, 1 reply; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 12:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, gregkh, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown, John Stultz,
Joerg Roedel, myungjoo.ham
The generic GPIO extcon driver (an external connector device based on
GPIO control) and imported from Android kernel.
switch: switch class and GPIO drivers. (splitted)
Author: Mike Lockwood <lockwood@android.com>
switch: gpio: Don't call request_irq with interrupts disabled
Author: Arve Hjønnevåg <arve@android.com>
switch_gpio: Add missing #include <linux/interrupt.h>
Author: Mike Lockwood <lockwood@android.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changed from v5:
- Splitted at v5 from the main extcon patch.
- Added debounce time for irq handlers.
- Use request_any_context_irq instead of request_irq
- User needs to specify irq flags for GPIO interrupts (was fixed to
IRQF_TRIGGER_LOW before)
- Use module_platform_driver().
---
drivers/extcon/Kconfig | 7 ++
drivers/extcon/Makefile | 1 +
drivers/extcon/extcon_gpio.c | 175 ++++++++++++++++++++++++++++++++++++
include/linux/extcon/extcon_gpio.h | 52 +++++++++++
4 files changed, 235 insertions(+), 0 deletions(-)
create mode 100644 drivers/extcon/extcon_gpio.c
create mode 100644 include/linux/extcon/extcon_gpio.h
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 11db865..cc3f5eb 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -25,4 +25,11 @@ config EXTCON_ANDROID_COMPATIBLE
comment "Extcon Device Drivers"
+config EXTCON_GPIO
+ tristate "GPIO extcon support"
+ depends on GENERIC_GPIO
+ help
+ Say Y here to enable GPIO based extcon support. Note that GPIO
+ extcon supports single state per extcon instance.
+
endif # MULTISTATE_SWITCH
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 6bc6921..2c46d41 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_EXTCON) += extcon_class.o
+obj-$(CONFIG_EXTCON_GPIO) += extcon_gpio.o
diff --git a/drivers/extcon/extcon_gpio.c b/drivers/extcon/extcon_gpio.c
new file mode 100644
index 0000000..f24ec67
--- /dev/null
+++ b/drivers/extcon/extcon_gpio.c
@@ -0,0 +1,175 @@
+/*
+ * drivers/extcon/extcon_gpio.c
+ *
+ * Single-state GPIO extcon driver based on extcon class
+ *
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * Modified by MyungJoo Ham <myungjoo.ham@samsung.com> to support extcon
+ * (originally switch class is supported)
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/extcon.h>
+#include <linux/workqueue.h>
+#include <linux/gpio.h>
+#include <linux/extcon.h>
+#include <linux/extcon/extcon_gpio.h>
+
+struct gpio_extcon_data {
+ struct extcon_dev edev;
+ unsigned gpio;
+ const char *state_on;
+ const char *state_off;
+ int irq;
+ struct delayed_work work;
+ unsigned long debounce_jiffies;
+};
+
+static void gpio_extcon_work(struct work_struct *work)
+{
+ int state;
+ struct gpio_extcon_data *data =
+ container_of(to_delayed_work(work), struct gpio_extcon_data,
+ work);
+
+ state = gpio_get_value(data->gpio);
+ extcon_set_state(&data->edev, state);
+}
+
+static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
+{
+ struct gpio_extcon_data *extcon_data =
+ (struct gpio_extcon_data *)dev_id;
+
+ schedule_delayed_work(&extcon_data->work,
+ extcon_data->debounce_jiffies);
+ return IRQ_HANDLED;
+}
+
+static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
+{
+ struct gpio_extcon_data *extcon_data =
+ container_of(edev, struct gpio_extcon_data, edev);
+ const char *state;
+ if (extcon_get_state(edev))
+ state = extcon_data->state_on;
+ else
+ state = extcon_data->state_off;
+
+ if (state)
+ return sprintf(buf, "%s\n", state);
+ return -EINVAL;
+}
+
+static int gpio_extcon_probe(struct platform_device *pdev)
+{
+ struct gpio_extcon_platform_data *pdata = pdev->dev.platform_data;
+ struct gpio_extcon_data *extcon_data;
+ int ret = 0;
+
+ if (!pdata)
+ return -EBUSY;
+ if (!pdata->irq_flags) {
+ dev_err(&pdev->dev, "IRQ flag is not specified.\n");
+ return -EINVAL;
+ }
+
+ extcon_data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
+ GFP_KERNEL);
+ if (!extcon_data)
+ return -ENOMEM;
+
+ extcon_data->edev.name = pdata->name;
+ extcon_data->gpio = pdata->gpio;
+ extcon_data->state_on = pdata->state_on;
+ extcon_data->state_off = pdata->state_off;
+ if (pdata->state_on && pdata->state_off)
+ extcon_data->edev.print_state = extcon_gpio_print_state;
+ extcon_data->debounce_jiffies = msecs_to_jiffies(pdata->debounce);
+
+ ret = extcon_dev_register(&extcon_data->edev, &pdev->dev);
+ if (ret < 0)
+ goto err_extcon_dev_register;
+
+ ret = gpio_request(extcon_data->gpio, pdev->name);
+ if (ret < 0)
+ goto err_request_gpio;
+
+ ret = gpio_direction_input(extcon_data->gpio);
+ if (ret < 0)
+ goto err_set_gpio_input;
+
+ INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
+
+ extcon_data->irq = gpio_to_irq(extcon_data->gpio);
+ if (extcon_data->irq < 0) {
+ ret = extcon_data->irq;
+ goto err_detect_irq_num_failed;
+ }
+
+ ret = request_any_context_irq(extcon_data->irq, gpio_irq_handler,
+ pdata->irq_flags, pdev->name,
+ extcon_data);
+ if (ret < 0)
+ goto err_request_irq;
+
+ /* Perform initial detection */
+ gpio_extcon_work(&extcon_data->work.work);
+
+ return 0;
+
+err_request_irq:
+err_detect_irq_num_failed:
+err_set_gpio_input:
+ gpio_free(extcon_data->gpio);
+err_request_gpio:
+ extcon_dev_unregister(&extcon_data->edev);
+err_extcon_dev_register:
+ devm_kfree(&pdev->dev, extcon_data);
+
+ return ret;
+}
+
+static int gpio_extcon_remove(struct platform_device *pdev)
+{
+ struct gpio_extcon_data *extcon_data = platform_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&extcon_data->work);
+ gpio_free(extcon_data->gpio);
+ extcon_dev_unregister(&extcon_data->edev);
+ devm_kfree(&pdev->dev, extcon_data);
+
+ return 0;
+}
+
+static struct platform_driver gpio_extcon = {
+ .probe = gpio_extcon_probe,
+ .remove = gpio_extcon_remove,
+ .driver = {
+ .name = "extcon-gpio",
+ .owner = THIS_MODULE,
+ },
+};
+
+module_platform_driver(gpio_extcon);
+
+MODULE_AUTHOR("Mike Lockwood <lockwood@android.com>");
+MODULE_DESCRIPTION("GPIO extcon driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/extcon/extcon_gpio.h b/include/linux/extcon/extcon_gpio.h
new file mode 100644
index 0000000..a2129b7
--- /dev/null
+++ b/include/linux/extcon/extcon_gpio.h
@@ -0,0 +1,52 @@
+/*
+ * External connector (extcon) class generic GPIO driver
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Author: MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * based on switch class driver
+ * Copyright (C) 2008 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+*/
+#ifndef __EXTCON_GPIO_H__
+#define __EXTCON_GPIO_H__ __FILE__
+
+#include <linux/extcon.h>
+
+/**
+ * struct gpio_extcon_platform_data - A simple GPIO-controlled extcon device.
+ * @name The name of this GPIO extcon device.
+ * @gpio Corresponding GPIO.
+ * @debounce Debounce time for GPIO IRQ in ms.
+ * @irq_flags IRQ Flags (e.g., IRQF_TRIGGER_LOW).
+ * @state_on print_state is overriden with state_on if attached. If Null,
+ * default method of extcon class is used.
+ * @state_off print_state is overriden with state_on if dettached. If Null,
+ * default method of extcon class is used.
+ *
+ * Note that in order for state_on or state_off to be valid, both state_on
+ * and state_off should be not NULL. If at least one of them is NULL,
+ * the print_state is not overriden.
+ */
+struct gpio_extcon_platform_data {
+ const char *name;
+ unsigned gpio;
+ unsigned long debounce;
+ unsigned long irq_flags;
+
+ /* if NULL, "0" or "1" will be printed */
+ const char *state_on;
+ const char *state_off;
+};
+
+#endif /* __EXTCON_GPIO_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v6 2/5] Extcon: support generic GPIO extcon driver
2012-02-27 12:15 ` [PATCH v6 2/5] Extcon: support generic GPIO extcon driver MyungJoo Ham
@ 2012-03-29 22:37 ` Stephen Boyd
2012-03-30 8:33 ` MyungJoo Ham
0 siblings, 1 reply; 50+ messages in thread
From: Stephen Boyd @ 2012-03-29 22:37 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-kernel, linux-arm-kernel, Mark Brown, Arnd Bergmann,
NeilBrown, gregkh, Linus Walleij, Dmitry Torokhov, Randy Dunlap,
Kyungmin Park, myungjoo.ham, John Stultz, Arve Hjønnevag,
Joerg Roedel, Morten CHRISTIANSEN, Mike Lockwood
On 02/27/12 04:15, MyungJoo Ham wrote
> +static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
> +{
> + struct gpio_extcon_data *extcon_data =
> + (struct gpio_extcon_data *)dev_id;
Casting from void is unnecessary.
> +
> +static int gpio_extcon_probe(struct platform_device *pdev)
__devinit?
> +{
> + struct gpio_extcon_platform_data *pdata = pdev->dev.platform_data;
> + struct gpio_extcon_data *extcon_data;
> + int ret = 0;
[snip]
> + return ret;
> +}
> +
> +static int gpio_extcon_remove(struct platform_device *pdev)
__devexit?
> +{
> + struct gpio_extcon_data *extcon_data = platform_get_drvdata(pdev);
[snip]
> +static struct platform_driver gpio_extcon = {
> + .probe = gpio_extcon_probe,
> + .remove = gpio_extcon_remove,
__devexit_p()?
> + .driver = {
> + .name = "extcon-gpio",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +module_platform_driver(gpio_extcon);
> +
> +MODULE_AUTHOR("Mike Lockwood <lockwood@android.com>");
> +MODULE_DESCRIPTION("GPIO extcon driver");
> +MODULE_LICENSE("GPL");
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v6 2/5] Extcon: support generic GPIO extcon driver
2012-03-29 22:37 ` Stephen Boyd
@ 2012-03-30 8:33 ` MyungJoo Ham
0 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-03-30 8:33 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-kernel, linux-arm-kernel, Mark Brown, Arnd Bergmann,
NeilBrown, gregkh, Linus Walleij, Dmitry Torokhov, Randy Dunlap,
Kyungmin Park, John Stultz, Arve Hjønnevag, Joerg Roedel,
Morten CHRISTIANSEN, Mike Lockwood
On Fri, Mar 30, 2012 at 7:37 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 02/27/12 04:15, MyungJoo Ham wrote
>> +static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
>> +{
>> + struct gpio_extcon_data *extcon_data =
>> + (struct gpio_extcon_data *)dev_id;
>
> Casting from void is unnecessary.
>
>> +
>> +static int gpio_extcon_probe(struct platform_device *pdev)
>
> __devinit?
>
>> +{
>> + struct gpio_extcon_platform_data *pdata = pdev->dev.platform_data;
>> + struct gpio_extcon_data *extcon_data;
>> + int ret = 0;
> [snip]
>> + return ret;
>> +}
>> +
>> +static int gpio_extcon_remove(struct platform_device *pdev)
>
> __devexit?
>> +{
>> + struct gpio_extcon_data *extcon_data = platform_get_drvdata(pdev);
> [snip]
>> +static struct platform_driver gpio_extcon = {
>> + .probe = gpio_extcon_probe,
>> + .remove = gpio_extcon_remove,
>
> __devexit_p()?
>> + .driver = {
>> + .name = "extcon-gpio",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +module_platform_driver(gpio_extcon);
>> +
>> +MODULE_AUTHOR("Mike Lockwood <lockwood@android.com>");
>> +MODULE_DESCRIPTION("GPIO extcon driver");
>> +MODULE_LICENSE("GPL");
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
Thank you for the comments.
I'll apply it and test on patchset v8.
Cheers!
MyungJoo.
--
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 3/5] Extcon: support notification based on the state changes.
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 1/5] Extcon (external connector): import Android's switch class and modify MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 2/5] Extcon: support generic GPIO extcon driver MyungJoo Ham
@ 2012-02-27 12:15 ` MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 4/5] Extcon: support multiple states at a device MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 5/5] Extcon: support mutually exclusive relation between cables MyungJoo Ham
4 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 12:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, gregkh, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown, John Stultz,
Joerg Roedel, myungjoo.ham
From: Donggeun Kim <dg77.kim@samsung.com>
State changes of extcon devices have been notified via kobjet_uevent.
This patch adds notifier interfaces in order to allow device drivers to
get notified easily. Along with notifier interface,
extcon_get_extcon_dev() function is added so that device drivers may
discover a extcon_dev easily.
Signed-off-by: Donggeun Kim <dg77.kim@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
--
Changes from RFC
- Renamed switch to extcon
- Bugfix: extcon_dev_unregister()
- Bugfix: "edev->dev" is "internal" data.
- Added kerneldoc comments.
- Reworded comments.
---
drivers/extcon/extcon_class.c | 66 +++++++++++++++++++++++++++++++++++++++++
include/linux/extcon.h | 38 +++++++++++++++++++++++
2 files changed, 104 insertions(+), 0 deletions(-)
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
index 9bc062b..51b14bd 100644
--- a/drivers/extcon/extcon_class.c
+++ b/drivers/extcon/extcon_class.c
@@ -33,6 +33,9 @@
struct class *extcon_class;
+static LIST_HEAD(extcon_dev_list);
+static DEFINE_MUTEX(extcon_dev_list_lock);
+
static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -72,6 +75,9 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
* the name of extcon device (envp[0]) and the state output (envp[1]).
* Tizen uses this format for extcon device to get events from ports.
* Android uses this format as well.
+ *
+ * Note that notifier provides the which bits are changes in the state
+ * variable with "val" to the callback.
*/
void extcon_set_state(struct extcon_dev *edev, u32 state)
{
@@ -81,10 +87,14 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
char *envp[3];
int env_offset = 0;
int length;
+ u32 old_state = edev->state;
if (edev->state != state) {
edev->state = state;
+ raw_notifier_call_chain(&edev->nh, old_state ^ edev->state,
+ edev);
+
prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
if (prop_buf) {
length = name_show(edev->dev, NULL, prop_buf);
@@ -114,6 +124,51 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
}
EXPORT_SYMBOL_GPL(extcon_set_state);
+/**
+ * extcon_get_extcon_dev() - Get the extcon device instance from the name
+ * @extcon_name: The extcon name provided with extcon_dev_register()
+ */
+struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
+{
+ struct extcon_dev *sd;
+
+ mutex_lock(&extcon_dev_list_lock);
+ list_for_each_entry(sd, &extcon_dev_list, entry) {
+ if (!strcmp(sd->name, extcon_name))
+ goto out;
+ }
+ sd = NULL;
+out:
+ mutex_unlock(&extcon_dev_list_lock);
+ return sd;
+}
+EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
+
+/**
+ * extcon_register_notifier() - Register a notifee to get notified by
+ * any attach status changes from the extcon.
+ * @edev: the extcon device.
+ * @nb: a notifier block to be registered.
+ */
+int extcon_register_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return raw_notifier_chain_register(&edev->nh, nb);
+}
+EXPORT_SYMBOL_GPL(extcon_register_notifier);
+
+/**
+ * extcon_unregister_notifier() - Unregister a notifee from the extcon device.
+ * @edev: the extcon device.
+ * @nb: a registered notifier block to be unregistered.
+ */
+int extcon_unregister_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return raw_notifier_chain_unregister(&edev->nh, nb);
+}
+EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
+
static struct device_attribute extcon_attrs[] = {
__ATTR_RO(state),
__ATTR_RO(name),
@@ -137,6 +192,10 @@ static int create_extcon_class(void)
static void extcon_cleanup(struct extcon_dev *edev, bool skip)
{
+ mutex_lock(&extcon_dev_list_lock);
+ list_del(&edev->entry);
+ mutex_unlock(&extcon_dev_list_lock);
+
if (!skip && get_device(edev->dev)) {
device_unregister(edev->dev);
put_device(edev->dev);
@@ -184,8 +243,15 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
goto err_dev;
}
+ RAW_INIT_NOTIFIER_HEAD(&edev->nh);
+
dev_set_drvdata(edev->dev, edev);
edev->state = 0;
+
+ mutex_lock(&extcon_dev_list_lock);
+ list_add(&edev->entry, &extcon_dev_list);
+ mutex_unlock(&extcon_dev_list_lock);
+
return 0;
err_dev:
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index cd732ab..50be845 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -23,6 +23,7 @@
#ifndef __LINUX_EXTCON_H__
#define __LINUX_EXTCON_H__
+#include <linux/notifier.h>
/**
* struct extcon_dev - An extcon device represents one external connector.
* @name The name of this extcon device. Parent device name is used
@@ -34,6 +35,9 @@
* @dev Device of this extcon. Do not provide at register-time.
* @state Attach/detach state of this extcon. Do not provide at
* register-time
+ * @nh Notifier for the state change events from this extcon
+ * @entry To support list of extcon devices so that uses can search
+ * for extcon devices based on the extcon name.
*
* In most cases, users only need to provide "User initializing data" of
* this struct when registering an extcon. In some exceptional cases,
@@ -51,11 +55,19 @@ struct extcon_dev {
/* --- Internal data. Please do not set. --- */
struct device *dev;
u32 state;
+ struct raw_notifier_head nh;
+ struct list_head entry;
};
#ifdef CONFIG_EXTCON
+
+/*
+ * Following APIs are for notifiers or configurations.
+ * Notifiers are the external port and connection devices.
+ */
extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
extern void extcon_dev_unregister(struct extcon_dev *edev);
+extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
static inline u32 extcon_get_state(struct extcon_dev *edev)
{
@@ -63,6 +75,15 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
}
extern void extcon_set_state(struct extcon_dev *edev, u32 state);
+
+/*
+ * Following APIs are to monitor every action of a notifier.
+ * Registerer gets notified for every external port of a connection device.
+ */
+extern int extcon_register_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb);
+extern int extcon_unregister_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb);
#else /* CONFIG_EXTCON */
static inline int extcon_dev_register(struct extcon_dev *edev,
struct device *dev)
@@ -78,5 +99,22 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
}
static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
+{
+ return NULL;
+}
+
+static inline int extcon_register_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int extcon_unregister_notifier(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
#endif /* CONFIG_EXTCON */
#endif /* __LINUX_EXTCON_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 4/5] Extcon: support multiple states at a device.
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
` (2 preceding siblings ...)
2012-02-27 12:15 ` [PATCH v6 3/5] Extcon: support notification based on the state changes MyungJoo Ham
@ 2012-02-27 12:15 ` MyungJoo Ham
2012-02-27 12:15 ` [PATCH v6 5/5] Extcon: support mutually exclusive relation between cables MyungJoo Ham
4 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 12:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, gregkh, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown, John Stultz,
Joerg Roedel, myungjoo.ham
One switch device (e.g., MUIC(MAX8997, MAX77686, ...), and some 30-pin
devices) may have multiple cables attached. For example, one
30-pin port may inhabit a USB cable, an HDMI cable, and a mic.
Thus, one switch device requires multiple state bits each representing
a type of cable.
For such purpose, we use the 32bit state variable; thus, up to 32
different type of cables may be defined for a switch device. The list of
possible cables is defined by the array of cable names in the switch_dev
struct given to the class.
Signed-off-by: Donggeun Kim <dg77.kim@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changes from V5
- Sysfs style reformed: subdirectory per cable.
- Updated standard cable names
- Removed unnecessary printf
- Bugfixes after testing
Changes from V4
- Bugfixes after more testing at Exynos4412 boards with userspace
processses.
Changes from V3
- Bugfixes after more testing at Exynos4412 boards.
Changes from V2
- State can be stored by user
- Documentation updated
Changes from RFC
- Switch is renamed to extcon
- Added kerneldoc comments
- Added APIs to support "standard" cable names
- Added helper APIs to support notifier block registration with cable
name.
- Regrouped function list in the header file.
---
Documentation/ABI/testing/sysfs-class-extcon | 63 ++++-
drivers/extcon/extcon_class.c | 439 +++++++++++++++++++++++++-
include/linux/extcon.h | 184 +++++++++++
3 files changed, 666 insertions(+), 20 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-extcon b/Documentation/ABI/testing/sysfs-class-extcon
index 59a4b1c..19b18e9 100644
--- a/Documentation/ABI/testing/sysfs-class-extcon
+++ b/Documentation/ABI/testing/sysfs-class-extcon
@@ -1,5 +1,5 @@
What: /sys/class/extcon/.../
-Date: December 2011
+Date: February 2012
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
Description:
Provide a place in sysfs for the extcon objects.
@@ -7,8 +7,16 @@ Description:
The name of extcon object denoted as ... is the name given
with extcon_dev_register.
+ One extcon device denotes a single external connector
+ port. An external connector may have multiple cables
+ attached simultaneously. Many of docks, cradles, and
+ accessory cables have such capability. For example,
+ the 30-pin port of Nuri board (/arch/arm/mach-exynos)
+ may have both HDMI and Charger attached, or analog audio,
+ video, and USB cables attached simulteneously.
+
What: /sys/class/extcon/.../name
-Date: December 2011
+Date: February 2012
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
Description:
The /sys/class/extcon/.../name shows the name of the extcon
@@ -17,10 +25,51 @@ Description:
this sysfs node.
What: /sys/class/extcon/.../state
-Date: December 2011
+Date: February 2012
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ The /sys/class/extcon/.../state shows and stores the cable
+ attach/detach information of the corresponding extcon object.
+ If the extcon object has an optional callback "show_state"
+ defined, the showing function is overriden with the optional
+ callback.
+
+ If the default callback for showing function is used, the
+ format is like this:
+ # cat state
+ USB_OTG=1
+ HDMI=0
+ TA=1
+ EAR_JACK=0
+ #
+ In this example, the extcon device have USB_OTG and TA
+ cables attached and HDMI and EAR_JACK cables detached.
+
+ In order to update the state of an extcon device, enter a hex
+ state number starting with 0x.
+ echo 0xHEX > state
+
+ This updates the whole state of the extcon dev.
+ Inputs of all the methods are required to meet the
+ mutually_exclusive contidions if they exist.
+
+ It is recommended to use this "global" state interface if
+ you need to enter the value atomically. The later state
+ interface associated with each cable cannot update
+ multiple cable states of an extcon device simultaneously.
+
+What: /sys/class/extcon/.../cable.x/name
+Date: February 2012
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ The /sys/class/extcon/.../cable.x/name shows the name of cable
+ "x" (integer between 0 and 31) of an extcon device.
+
+What: /sys/class/extcon/.../cable.x/state
+Date: February 2012
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
Description:
- The /sys/class/extcon/.../state shows the cable attach/detach
- information of the corresponding extcon object. If the extcon
- objecct has an optional callback "show_state" defined, the
- callback will provide the name with this sysfs node.
+ The /sys/class/extcon/.../cable.x/name shows and stores the
+ state of cable "x" (integer between 0 and 31) of an extcon
+ device. The state value is either 0 (detached) or 1
+ (attached).
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
index 51b14bd..55f19c2 100644
--- a/drivers/extcon/extcon_class.c
+++ b/drivers/extcon/extcon_class.c
@@ -31,6 +31,39 @@
#include <linux/extcon.h>
#include <linux/slab.h>
+/*
+ * extcon_cable_name suggests the standard cable names for commonly used
+ * cable types.
+ *
+ * However, please do not use extcon_cable_name directly for extcon_dev
+ * struct's supported_cable pointer unless your device really supports
+ * every single port-type of the following cable names. Please choose cable
+ * names that are actually used in your extcon device.
+ */
+const char *extcon_cable_name[] = {
+ [EXTCON_USB] = "USB",
+ [EXTCON_USB_HOST] = "USB-Host",
+ [EXTCON_TA] = "TA",
+ [EXTCON_FAST_CHARGER] = "Fast-charger",
+ [EXTCON_SLOW_CHARGER] = "Slow-charger",
+ [EXTCON_CHARGE_DOWNSTREAM] = "Charge-downstream",
+ [EXTCON_HDMI] = "HDMI",
+ [EXTCON_MHL] = "MHL",
+ [EXTCON_DVI] = "DVI",
+ [EXTCON_VGA] = "VGA",
+ [EXTCON_DOCK] = "Dock",
+ [EXTCON_LINE_IN] = "Line-in",
+ [EXTCON_LINE_OUT] = "Line-out",
+ [EXTCON_MIC_IN] = "Microphone",
+ [EXTCON_HEADPHONE_OUT] = "Headphone",
+ [EXTCON_SPDIF_IN] = "SPDIF-in",
+ [EXTCON_SPDIF_OUT] = "SPDIF-out",
+ [EXTCON_VIDEO_IN] = "Video-in",
+ [EXTCON_VIDEO_OUT] = "Video-out",
+
+ NULL,
+};
+
struct class *extcon_class;
static LIST_HEAD(extcon_dev_list);
@@ -39,6 +72,7 @@ static DEFINE_MUTEX(extcon_dev_list_lock);
static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ int i, count = 0;
struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
if (edev->print_state) {
@@ -48,7 +82,39 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
return ret;
/* Use default if failed */
}
- return sprintf(buf, "%u\n", edev->state);
+
+ if (edev->max_supported == 0)
+ return sprintf(buf, "%u\n", edev->state);
+
+ for (i = 0; i < SUPPORTED_CABLE_MAX; i++) {
+ if (!edev->supported_cable[i])
+ break;
+ count += sprintf(buf + count, "%s=%d\n",
+ edev->supported_cable[i],
+ !!(edev->state & (1 << i)));
+ }
+
+ return count;
+}
+
+void extcon_set_state(struct extcon_dev *edev, u32 state);
+static ssize_t state_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u32 state;
+ ssize_t ret = 0;
+ struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
+
+ ret = sscanf(buf, "0x%x", &state);
+ if (ret == 0)
+ ret = -EINVAL;
+ else
+ extcon_set_state(edev, state);
+
+ if (ret < 0)
+ return ret;
+
+ return count;
}
static ssize_t name_show(struct device *dev, struct device_attribute *attr,
@@ -66,9 +132,52 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
return sprintf(buf, "%s\n", dev_name(edev->dev));
}
+static ssize_t cable_name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct extcon_cable *cable = container_of(attr, struct extcon_cable,
+ attr_name);
+
+ return sprintf(buf, "%s\n",
+ cable->edev->supported_cable[cable->cable_index]);
+}
+
+static ssize_t cable_state_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct extcon_cable *cable = container_of(attr, struct extcon_cable,
+ attr_state);
+
+ return sprintf(buf, "%d\n",
+ extcon_get_cable_state_(cable->edev,
+ cable->cable_index));
+}
+
+static ssize_t cable_state_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct extcon_cable *cable = container_of(attr, struct extcon_cable,
+ attr_state);
+ int ret, state;
+
+ ret = sscanf(buf, "%d", &state);
+ if (ret == 0)
+ ret = -EINVAL;
+ else
+ ret = extcon_set_cable_state_(cable->edev, cable->cable_index,
+ state);
+
+ if (ret < 0)
+ return ret;
+ return count;
+}
+
/**
- * extcon_set_state() - Set the cable attach states of the extcon device.
+ * extcon_update_state() - Update the cable attach states of the extcon device
+ * only for the masked bits.
* @edev: the extcon device
+ * @mask: the bit mask to designate updated bits.
* @state: new cable attach status for @edev
*
* Changing the state sends uevent with environment variable containing
@@ -76,10 +185,10 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
* Tizen uses this format for extcon device to get events from ports.
* Android uses this format as well.
*
- * Note that notifier provides the which bits are changes in the state
- * variable with "val" to the callback.
+ * Note that the notifier provides which bits are changed in the state
+ * variable with the val parameter (second) to the callback.
*/
-void extcon_set_state(struct extcon_dev *edev, u32 state)
+void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
{
char name_buf[120];
char state_buf[120];
@@ -87,15 +196,20 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
char *envp[3];
int env_offset = 0;
int length;
- u32 old_state = edev->state;
+ unsigned long flags;
- if (edev->state != state) {
- edev->state = state;
+ spin_lock_irqsave(&edev->lock, flags);
- raw_notifier_call_chain(&edev->nh, old_state ^ edev->state,
- edev);
+ if (edev->state != ((edev->state & ~mask) | (state & mask))) {
+ u32 old_state = edev->state;
- prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
+ edev->state &= ~mask;
+ edev->state |= state & mask;
+
+ raw_notifier_call_chain(&edev->nh, old_state, edev);
+
+ /* This could be in interrupt handler */
+ prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
if (prop_buf) {
length = name_show(edev->dev, NULL, prop_buf);
if (length > 0) {
@@ -114,17 +228,132 @@ void extcon_set_state(struct extcon_dev *edev, u32 state)
envp[env_offset++] = state_buf;
}
envp[env_offset] = NULL;
+ /* Unlock early before uevent */
+ spin_unlock_irqrestore(&edev->lock, flags);
+
kobject_uevent_env(&edev->dev->kobj, KOBJ_CHANGE, envp);
free_page((unsigned long)prop_buf);
} else {
+ /* Unlock early before uevent */
+ spin_unlock_irqrestore(&edev->lock, flags);
+
dev_err(edev->dev, "out of memory in extcon_set_state\n");
kobject_uevent(&edev->dev->kobj, KOBJ_CHANGE);
}
+ } else {
+ /* No changes */
+ spin_unlock_irqrestore(&edev->lock, flags);
}
}
+EXPORT_SYMBOL_GPL(extcon_update_state);
+
+/**
+ * extcon_set_state() - Set the cable attach states of the extcon device.
+ * @edev: the extcon device
+ * @state: new cable attach status for @edev
+ *
+ * Note that notifier provides which bits are changed in the state
+ * variable with the val parameter (second) to the callback.
+ */
+void extcon_set_state(struct extcon_dev *edev, u32 state)
+{
+ extcon_update_state(edev, 0xffffffff, state);
+}
EXPORT_SYMBOL_GPL(extcon_set_state);
/**
+ * extcon_find_cable_index() - Get the cable index based on the cable name.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name to be searched.
+ *
+ * Note that accessing a cable state based on cable_index is faster than
+ * cable_name because using cable_name induces a loop with strncmp().
+ * Thus, when get/set_cable_state is repeatedly used, using cable_index
+ * is recommended.
+ */
+int extcon_find_cable_index(struct extcon_dev *edev, const char *cable_name)
+{
+ int i;
+
+ if (edev->supported_cable) {
+ for (i = 0; edev->supported_cable[i]; i++) {
+ if (!strncmp(edev->supported_cable[i],
+ cable_name, CABLE_NAME_MAX))
+ return i;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(extcon_find_cable_index);
+
+/**
+ * extcon_get_cable_state_() - Get the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @index: cable index that can be retrieved by extcon_find_cable_index().
+ */
+int extcon_get_cable_state_(struct extcon_dev *edev, int index)
+{
+ if (index < 0 || (edev->max_supported && edev->max_supported <= index))
+ return -EINVAL;
+
+ return !!(edev->state & (1 << index));
+}
+EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
+
+/**
+ * extcon_get_cable_state() - Get the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name.
+ *
+ * Note that this is slower than extcon_get_cable_state_.
+ */
+int extcon_get_cable_state(struct extcon_dev *edev, const char *cable_name)
+{
+ return extcon_get_cable_state_(edev, extcon_find_cable_index
+ (edev, cable_name));
+}
+EXPORT_SYMBOL_GPL(extcon_get_cable_state);
+
+/**
+ * extcon_get_cable_state_() - Set the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @index: cable index that can be retrieved by extcon_find_cable_index().
+ * @cable_state: the new cable status. The default semantics is
+ * true: attached / false: detached.
+ */
+int extcon_set_cable_state_(struct extcon_dev *edev,
+ int index, bool cable_state)
+{
+ u32 state;
+
+ if (index < 0 || (edev->max_supported && edev->max_supported <= index))
+ return -EINVAL;
+
+ state = cable_state ? (1 << index) : 0;
+ extcon_update_state(edev, 1 << index, state);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
+
+/**
+ * extcon_get_cable_state() - Set the status of a specific cable.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name.
+ * @cable_state: the new cable status. The default semantics is
+ * true: attached / false: detached.
+ *
+ * Note that this is slower than extcon_set_cable_state_.
+ */
+int extcon_set_cable_state(struct extcon_dev *edev,
+ const char *cable_name, bool cable_state)
+{
+ return extcon_set_cable_state_(edev, extcon_find_cable_index
+ (edev, cable_name), cable_state);
+}
+EXPORT_SYMBOL_GPL(extcon_set_cable_state);
+
+/**
* extcon_get_extcon_dev() - Get the extcon device instance from the name
* @extcon_name: The extcon name provided with extcon_dev_register()
*/
@@ -144,11 +373,88 @@ out:
}
EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
+static int _call_per_cable(struct notifier_block *nb, unsigned long val,
+ void *ptr)
+{
+ struct extcon_specific_cable_nb *obj = container_of(nb,
+ struct extcon_specific_cable_nb, internal_nb);
+ struct extcon_dev *edev = ptr;
+
+ if ((val & (1 << obj->cable_index)) !=
+ (edev->state & (1 << obj->cable_index))) {
+ obj->previous_value = val;
+ return obj->user_nb->notifier_call(nb, val, ptr);
+ }
+
+ return NOTIFY_OK;
+}
+
+/**
+ * extcon_register_interest() - Register a notifier for a state change of a
+ * specific cable, not a entier set of cables of a
+ * extcon device.
+ * @obj: an empty extcon_specific_cable_nb object to be returned.
+ * @extcon_name: the name of extcon device.
+ * @cable_name: the target cable name.
+ * @nb: the notifier block to get notified.
+ *
+ * Provide an empty extcon_specific_cable_nb. extcon_register_interest() sets
+ * the struct for you.
+ *
+ * extcon_register_interest is a helper function for those who want to get
+ * notification for a single specific cable's status change. If a user wants
+ * to get notification for any changes of all cables of a extcon device,
+ * he/she should use the general extcon_register_notifier().
+ *
+ * Note that the second parameter given to the callback of nb (val) is
+ * "old_state", not the current state. The current state can be retrieved
+ * by looking at the third pameter (edev pointer)'s state value.
+ */
+int extcon_register_interest(struct extcon_specific_cable_nb *obj,
+ const char *extcon_name, const char *cable_name,
+ struct notifier_block *nb)
+{
+ if (!obj || !extcon_name || !cable_name || !nb)
+ return -EINVAL;
+
+ obj->edev = extcon_get_extcon_dev(extcon_name);
+ if (!obj->edev)
+ return -ENODEV;
+
+ obj->cable_index = extcon_find_cable_index(obj->edev, cable_name);
+ if (obj->cable_index < 0)
+ return -ENODEV;
+
+ obj->user_nb = nb;
+
+ obj->internal_nb.notifier_call = _call_per_cable;
+
+ return raw_notifier_chain_register(&obj->edev->nh, &obj->internal_nb);
+}
+
+/**
+ * extcon_unregister_interest() - Unregister the notifier registered by
+ * extcon_register_interest().
+ * @obj: the extcon_specific_cable_nb object returned by
+ * extcon_register_interest().
+ */
+int extcon_unregister_interest(struct extcon_specific_cable_nb *obj)
+{
+ if (!obj)
+ return -EINVAL;
+
+ return raw_notifier_chain_unregister(&obj->edev->nh, &obj->internal_nb);
+}
+
/**
* extcon_register_notifier() - Register a notifee to get notified by
* any attach status changes from the extcon.
* @edev: the extcon device.
* @nb: a notifier block to be registered.
+ *
+ * Note that the second parameter given to the callback of nb (val) is
+ * "old_state", not the current state. The current state can be retrieved
+ * by looking at the third pameter (edev pointer)'s state value.
*/
int extcon_register_notifier(struct extcon_dev *edev,
struct notifier_block *nb)
@@ -170,8 +476,9 @@ int extcon_unregister_notifier(struct extcon_dev *edev,
EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
static struct device_attribute extcon_attrs[] = {
- __ATTR_RO(state),
+ __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
__ATTR_RO(name),
+ __ATTR_NULL,
};
static int create_extcon_class(void)
@@ -197,6 +504,16 @@ static void extcon_cleanup(struct extcon_dev *edev, bool skip)
mutex_unlock(&extcon_dev_list_lock);
if (!skip && get_device(edev->dev)) {
+ int index;
+
+ for (index = 0; index < edev->max_supported; index++)
+ kfree(edev->cables[index].attr_g.name);
+
+ if (edev->max_supported) {
+ kfree(edev->extcon_dev_type.groups);
+ kfree(edev->cables);
+ }
+
device_unregister(edev->dev);
put_device(edev->dev);
}
@@ -211,6 +528,10 @@ static void extcon_dev_release(struct device *dev)
extcon_cleanup(edev, true);
}
+static void dummy_sysfs_dev_release(struct device *dev)
+{
+}
+
/**
* extcon_dev_register() - Register a new extcon device
* @edev : the new extcon device (should be allocated before calling)
@@ -223,7 +544,7 @@ static void extcon_dev_release(struct device *dev)
*/
int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
{
- int ret;
+ int ret, index = 0;
if (!extcon_class) {
ret = create_extcon_class();
@@ -231,18 +552,101 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
return ret;
}
+ if (edev->supported_cable) {
+ /* Get size of array */
+ for (index = 0; edev->supported_cable[index]; index++)
+ ;
+ edev->max_supported = index;
+ } else {
+ edev->max_supported = 0;
+ }
+
+ if (index > SUPPORTED_CABLE_MAX) {
+ dev_err(edev->dev, "extcon: maximum number of supported cables exceeded.\n");
+ return -EINVAL;
+ }
+
edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
edev->dev->parent = dev;
edev->dev->class = extcon_class;
edev->dev->release = extcon_dev_release;
dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+
+ if (edev->max_supported) {
+ char buf[10];
+ char *str;
+ struct extcon_cable *cable;
+
+ edev->cables = kzalloc(sizeof(struct extcon_cable) *
+ edev->max_supported, GFP_KERNEL);
+ if (!edev->cables) {
+ ret = -ENOMEM;
+ goto err_sysfs_alloc;
+ }
+ for (index = 0; index < edev->max_supported; index++) {
+ cable = &edev->cables[index];
+
+ snprintf(buf, 10, "cable.%d", index);
+ str = kzalloc(sizeof(char) * (strlen(buf) + 1),
+ GFP_KERNEL);
+ if (!str) {
+ for (index--; index >= 0; index--) {
+ cable = &edev->cables[index];
+ kfree(cable->attr_g.name);
+ }
+ ret = -ENOMEM;
+
+ goto err_alloc_cables;
+ }
+ strcpy(str, buf);
+
+ cable->edev = edev;
+ cable->cable_index = index;
+ cable->attrs[0] = &cable->attr_name.attr;
+ cable->attrs[1] = &cable->attr_state.attr;
+ cable->attrs[2] = NULL;
+ cable->attr_g.name = str;
+ cable->attr_g.attrs = cable->attrs;
+
+ cable->attr_name.attr.name = "name";
+ cable->attr_name.attr.mode = 0444;
+ cable->attr_name.show = cable_name_show;
+
+ cable->attr_state.attr.name = "state";
+ cable->attr_state.attr.mode = 0644;
+ cable->attr_state.show = cable_state_show;
+ cable->attr_state.store = cable_state_store;
+ }
+ }
+
+ if (edev->max_supported) {
+ edev->extcon_dev_type.groups =
+ kzalloc(sizeof(struct attribute_group *) *
+ (edev->max_supported + 1), GFP_KERNEL);
+ if (!edev->extcon_dev_type.groups) {
+ ret = -ENOMEM;
+ goto err_alloc_groups;
+ }
+
+ edev->extcon_dev_type.name = dev_name(edev->dev);
+ edev->extcon_dev_type.release = dummy_sysfs_dev_release;
+
+ for (index = 0; index < edev->max_supported; index++)
+ edev->extcon_dev_type.groups[index] =
+ &edev->cables[index].attr_g;
+
+ edev->dev->type = &edev->extcon_dev_type;
+ }
+
ret = device_register(edev->dev);
if (ret) {
put_device(edev->dev);
goto err_dev;
}
+ spin_lock_init(&edev->lock);
+
RAW_INIT_NOTIFIER_HEAD(&edev->nh);
dev_set_drvdata(edev->dev, edev);
@@ -255,6 +659,15 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
return 0;
err_dev:
+ if (edev->max_supported)
+ kfree(edev->extcon_dev_type.groups);
+err_alloc_groups:
+ for (index = 0; index < edev->max_supported; index++)
+ kfree(edev->cables[index].attr_g.name);
+err_alloc_cables:
+ if (edev->max_supported)
+ kfree(edev->cables);
+err_sysfs_alloc:
kfree(edev->dev);
return ret;
}
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 50be845..99ff60e 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -24,10 +24,60 @@
#define __LINUX_EXTCON_H__
#include <linux/notifier.h>
+
+#define SUPPORTED_CABLE_MAX 32
+#define CABLE_NAME_MAX 30
+
+/*
+ * The standard cable name is to help support general notifier
+ * and notifee device drivers to share the common names.
+ * Please use standard cable names unless your notifier device has
+ * a very unique and abnormal cable or
+ * the cable type is supposed to be used with only one unique
+ * pair of notifier/notifee devices.
+ *
+ * Please add any other "standard" cables used with extcon dev.
+ *
+ * You may add a dot and number to specify version or specification
+ * of the specific cable if it is required. (e.g., "Fast-charger.18"
+ * and "Fast-charger.10" for 1.8A and 1.0A chargers)
+ * However, the notifee and notifier should be able to handle such
+ * string and if the notifee can negotiate the protocol or idenify,
+ * you don't need such convention. This convention is helpful when
+ * notifier can distinguish but notifiee cannot.
+ */
+enum extcon_cable_name {
+ EXTCON_USB = 0,
+ EXTCON_USB_HOST,
+ EXTCON_TA, /* Travel Adaptor */
+ EXTCON_FAST_CHARGER,
+ EXTCON_SLOW_CHARGER,
+ EXTCON_CHARGE_DOWNSTREAM, /* Charging an external device */
+ EXTCON_HDMI,
+ EXTCON_MHL,
+ EXTCON_DVI,
+ EXTCON_VGA,
+ EXTCON_DOCK,
+ EXTCON_LINE_IN,
+ EXTCON_LINE_OUT,
+ EXTCON_MIC_IN,
+ EXTCON_HEADPHONE_OUT,
+ EXTCON_SPDIF_IN,
+ EXTCON_SPDIF_OUT,
+ EXTCON_VIDEO_IN,
+ EXTCON_VIDEO_OUT,
+};
+extern const char *extcon_cable_name[];
+
+struct extcon_cable;
+
/**
* struct extcon_dev - An extcon device represents one external connector.
* @name The name of this extcon device. Parent device name is used
* if NULL.
+ * @supported_cable Array of supported cable name ending with NULL.
+ * If supported_cable is NULL, cable name related APIs
+ * are disabled.
* @print_name An optional callback to override the method to print the
* name of the extcon device.
* @print_state An optional callback to override the method to print the
@@ -38,6 +88,11 @@
* @nh Notifier for the state change events from this extcon
* @entry To support list of extcon devices so that uses can search
* for extcon devices based on the extcon name.
+ * @lock
+ * @max_supported Internal value to store the number of cables.
+ * @extcon_dev_type Device_type struct to provide attribute_groups
+ * customized for each extcon device.
+ * @cables Sysfs subdirectories. Each represents one cable.
*
* In most cases, users only need to provide "User initializing data" of
* this struct when registering an extcon. In some exceptional cases,
@@ -47,6 +102,7 @@
struct extcon_dev {
/* --- Optional user initializing data --- */
const char *name;
+ const char **supported_cable;
/* --- Optional callbacks to override class functions --- */
ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
@@ -57,6 +113,49 @@ struct extcon_dev {
u32 state;
struct raw_notifier_head nh;
struct list_head entry;
+ spinlock_t lock; /* could be called by irq handler */
+ int max_supported;
+
+ /* /sys/class/extcon/.../cable.n/... */
+ struct device_type extcon_dev_type;
+ struct extcon_cable *cables;
+};
+
+/**
+ * struct extcon_cable - An internal data for each cable of extcon device.
+ * @edev The extcon device
+ * @cable_index Index of this cable in the edev
+ * @attr_g Attribute group for the cable
+ * @attr_name "name" sysfs entry
+ * @attr_state "state" sysfs entry
+ * @attrs Array pointing to attr_name and attr_state for attr_g
+ */
+struct extcon_cable {
+ struct extcon_dev *edev;
+ int cable_index;
+
+ struct attribute_group attr_g;
+ struct device_attribute attr_name;
+ struct device_attribute attr_state;
+
+ struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
+};
+
+/**
+ * struct extcon_specific_cable_nb - An internal data for
+ * extcon_register_interest().
+ * @internal_nb a notifier block bridging extcon notifier and cable notifier.
+ * @user_nb user provided notifier block for events from a specific cable.
+ * @cable_index the target cable.
+ * @edev the target extcon device.
+ * @previous_value the saved previous event value.
+ */
+struct extcon_specific_cable_nb {
+ struct notifier_block internal_nb;
+ struct notifier_block *user_nb;
+ int cable_index;
+ struct extcon_dev *edev;
+ unsigned long previous_value;
};
#ifdef CONFIG_EXTCON
@@ -69,16 +168,54 @@ extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
extern void extcon_dev_unregister(struct extcon_dev *edev);
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
+/*
+ * get/set/update_state access the 32b encoded state value, which represents
+ * states of all possible cables of the multistate port. For example, if one
+ * calls extcon_set_state(edev, 0x7), it may mean that all the three cables
+ * are attached to the port.
+ */
static inline u32 extcon_get_state(struct extcon_dev *edev)
{
return edev->state;
}
extern void extcon_set_state(struct extcon_dev *edev, u32 state);
+extern void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
+
+/*
+ * get/set_cable_state access each bit of the 32b encoded state value.
+ * They are used to access the status of each cable based on the cable_name
+ * or cable_index, which is retrived by extcon_find_cable_index
+ */
+extern int extcon_find_cable_index(struct extcon_dev *sdev,
+ const char *cable_name);
+extern int extcon_get_cable_state_(struct extcon_dev *edev, int cable_index);
+extern int extcon_set_cable_state_(struct extcon_dev *edev, int cable_index,
+ bool cable_state);
+
+extern int extcon_get_cable_state(struct extcon_dev *edev,
+ const char *cable_name);
+extern int extcon_set_cable_state(struct extcon_dev *edev,
+ const char *cable_name, bool cable_state);
+
+/*
+ * Following APIs are for notifiees (those who want to be notified)
+ * to register a callback for events from a specific cable of the extcon.
+ * Notifiees are the connected device drivers wanting to get notified by
+ * a specific external port of a connection device.
+ */
+extern int extcon_register_interest(struct extcon_specific_cable_nb *obj,
+ const char *extcon_name,
+ const char *cable_name,
+ struct notifier_block *nb);
+extern int extcon_unregister_interest(struct extcon_specific_cable_nb *nb);
/*
* Following APIs are to monitor every action of a notifier.
* Registerer gets notified for every external port of a connection device.
+ * Probably this could be used to debug an action of notifier; however,
+ * we do not recommend to use this at normal 'notifiee' device drivers who
+ * want to be notified by a specific external port of the notifier.
*/
extern int extcon_register_notifier(struct extcon_dev *edev,
struct notifier_block *nb);
@@ -99,6 +236,41 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
}
static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+
+static inline void extcon_update_state(struct extcon_dev *edev, u32 mask,
+ u32 state)
+{ }
+
+static inline int extcon_find_cable_index(struct extcon_dev *edev,
+ const char *cable_name)
+{
+ return 0;
+}
+
+static inline int extcon_get_cable_state_(struct extcon_dev *edev,
+ int cable_index)
+{
+ return 0;
+}
+
+static inline int extcon_set_cable_state_(struct extcon_dev *edev,
+ int cable_index, bool cable_state)
+{
+ return 0;
+}
+
+static inline int extcon_get_cable_state(struct extcon_dev *edev,
+ const char *cable_name)
+{
+ return 0;
+}
+
+static inline int extcon_set_cable_state(struct extcon_dev *edev,
+ const char *cable_name, int state)
+{
+ return 0;
+}
+
static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
{
return NULL;
@@ -116,5 +288,17 @@ static inline int extcon_unregister_notifier(struct extcon_dev *edev,
return 0;
}
+static inline int extcon_register_interest(struct extcon_specific_cable_nb *obj,
+ const char *extcon_name,
+ const char *cable_name,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int extcon_unregister_interest(struct extcon_specific_cable_nb *)
+{
+ return 0;
+}
#endif /* CONFIG_EXTCON */
#endif /* __LINUX_EXTCON_H__ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 5/5] Extcon: support mutually exclusive relation between cables.
2012-02-27 12:15 ` [PATCH v6 0/5] Introduce External Connector Class (extcon) MyungJoo Ham
` (3 preceding siblings ...)
2012-02-27 12:15 ` [PATCH v6 4/5] Extcon: support multiple states at a device MyungJoo Ham
@ 2012-02-27 12:15 ` MyungJoo Ham
4 siblings, 0 replies; 50+ messages in thread
From: MyungJoo Ham @ 2012-02-27 12:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: NeilBrown, Randy Dunlap, Mike Lockwood, Arve Hjønnevag,
Kyungmin Park, gregkh, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Morten CHRISTIANSEN, Mark Brown, John Stultz,
Joerg Roedel, myungjoo.ham
There could be cables that t recannot be attaches simulatenously. Extcon
device drivers may express such information via mutually_exclusive in
struct extcon_dev.
For example, for an extcon device with 16 cables (bits 0 to 15 are
available), if mutually_exclusive = { 0x7, 0xC0, 0x81, 0 }, then, the
following attachments are prohibitted.
{0, 1}
{0, 2}
{1, 2}
{6, 7}
{0, 7}
and every attachment set that are superset of one of the above.
For the detail, please refer to linux/include/linux/extcon.h.
The concept is suggested by NeilBrown <neilb@suse.de>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Changes from V5:
- Updated sysfs format
---
Documentation/ABI/testing/sysfs-class-extcon | 22 +++++
drivers/extcon/extcon_class.c | 123 ++++++++++++++++++++++++--
include/linux/extcon.h | 28 +++++-
3 files changed, 160 insertions(+), 13 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-extcon b/Documentation/ABI/testing/sysfs-class-extcon
index 19b18e9..20ab361 100644
--- a/Documentation/ABI/testing/sysfs-class-extcon
+++ b/Documentation/ABI/testing/sysfs-class-extcon
@@ -15,6 +15,10 @@ Description:
may have both HDMI and Charger attached, or analog audio,
video, and USB cables attached simulteneously.
+ If there are cables mutually exclusive with each other,
+ such binary relations may be expressed with extcon_dev's
+ mutually_exclusive array.
+
What: /sys/class/extcon/.../name
Date: February 2012
Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
@@ -73,3 +77,21 @@ Description:
state of cable "x" (integer between 0 and 31) of an extcon
device. The state value is either 0 (detached) or 1
(attached).
+
+What: /sys/class/extcon/.../mutually_exclusive/...
+Date: December 2011
+Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+ Shows the relations of mutually exclusiveness. For example,
+ if the mutually_exclusive array of extcon_dev is
+ {0x3, 0x5, 0xC, 0x0}, the, the output is:
+ # ls mutually_exclusive/
+ 0x3
+ 0x5
+ 0xc
+ #
+
+ Note that mutually_exclusive is a sub-directory of the extcon
+ device and the file names under the mutually_exclusive
+ directory show the mutually-exclusive sets, not the contents
+ of the files.
diff --git a/drivers/extcon/extcon_class.c b/drivers/extcon/extcon_class.c
index 55f19c2..fb8d3a4 100644
--- a/drivers/extcon/extcon_class.c
+++ b/drivers/extcon/extcon_class.c
@@ -69,6 +69,39 @@ struct class *extcon_class;
static LIST_HEAD(extcon_dev_list);
static DEFINE_MUTEX(extcon_dev_list_lock);
+/**
+ * check_mutually_exclusive - Check if new_state violates mutually_exclusive
+ * condition.
+ * @edev: the extcon device
+ * @new_state: new cable attach status for @edev
+ *
+ * Returns 0 if nothing violates. Returns the index + 1 for the first
+ * violated condition.
+ */
+static int check_mutually_exclusive(struct extcon_dev *edev, u32 new_state)
+{
+ int i = 0;
+
+ if (!edev->mutually_exclusive)
+ return 0;
+
+ for (i = 0; edev->mutually_exclusive[i]; i++) {
+ int count = 0, j;
+ u32 correspondants = new_state & edev->mutually_exclusive[i];
+ u32 exp = 1;
+
+ for (j = 0; j < 32; j++) {
+ if (exp & correspondants)
+ count++;
+ if (count > 1)
+ return i + 1;
+ exp <<= 1;
+ }
+ }
+
+ return 0;
+}
+
static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -97,7 +130,7 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
return count;
}
-void extcon_set_state(struct extcon_dev *edev, u32 state);
+int extcon_set_state(struct extcon_dev *edev, u32 state);
static ssize_t state_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -109,7 +142,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
if (ret == 0)
ret = -EINVAL;
else
- extcon_set_state(edev, state);
+ ret = extcon_set_state(edev, state);
if (ret < 0)
return ret;
@@ -188,7 +221,7 @@ static ssize_t cable_state_store(struct device *dev,
* Note that the notifier provides which bits are changed in the state
* variable with the val parameter (second) to the callback.
*/
-void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
+int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
{
char name_buf[120];
char state_buf[120];
@@ -203,6 +236,12 @@ void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
if (edev->state != ((edev->state & ~mask) | (state & mask))) {
u32 old_state = edev->state;
+ if (check_mutually_exclusive(edev, (edev->state & ~mask) |
+ (state & mask))) {
+ spin_unlock_irqrestore(&edev->lock, flags);
+ return -EPERM;
+ }
+
edev->state &= ~mask;
edev->state |= state & mask;
@@ -244,6 +283,8 @@ void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
/* No changes */
spin_unlock_irqrestore(&edev->lock, flags);
}
+
+ return 0;
}
EXPORT_SYMBOL_GPL(extcon_update_state);
@@ -255,9 +296,9 @@ EXPORT_SYMBOL_GPL(extcon_update_state);
* Note that notifier provides which bits are changed in the state
* variable with the val parameter (second) to the callback.
*/
-void extcon_set_state(struct extcon_dev *edev, u32 state)
+int extcon_set_state(struct extcon_dev *edev, u32 state)
{
- extcon_update_state(edev, 0xffffffff, state);
+ return extcon_update_state(edev, 0xffffffff, state);
}
EXPORT_SYMBOL_GPL(extcon_set_state);
@@ -331,8 +372,7 @@ int extcon_set_cable_state_(struct extcon_dev *edev,
return -EINVAL;
state = cable_state ? (1 << index) : 0;
- extcon_update_state(edev, 1 << index, state);
- return 0;
+ return extcon_update_state(edev, 1 << index, state);
}
EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
@@ -506,6 +546,14 @@ static void extcon_cleanup(struct extcon_dev *edev, bool skip)
if (!skip && get_device(edev->dev)) {
int index;
+ if (edev->mutually_exclusive && edev->max_supported) {
+ for (index = 0; edev->mutually_exclusive[index];
+ index++)
+ kfree(edev->d_attrs_muex[index].attr.name);
+ kfree(edev->d_attrs_muex);
+ kfree(edev->attrs_muex);
+ }
+
for (index = 0; index < edev->max_supported; index++)
kfree(edev->cables[index].attr_g.name);
@@ -528,6 +576,7 @@ static void extcon_dev_release(struct device *dev)
extcon_cleanup(edev, true);
}
+static const char *muex_name = "mutually_exclusive";
static void dummy_sysfs_dev_release(struct device *dev)
{
}
@@ -620,10 +669,58 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
}
}
+ if (edev->max_supported && edev->mutually_exclusive) {
+ char buf[80];
+ char *name;
+
+ /* Count the size of mutually_exclusive array */
+ for (index = 0; edev->mutually_exclusive[index]; index++)
+ ;
+
+ edev->attrs_muex = kzalloc(sizeof(struct attribute *) *
+ (index + 1), GFP_KERNEL);
+ if (!edev->attrs_muex) {
+ ret = -ENOMEM;
+ goto err_muex;
+ }
+
+ edev->d_attrs_muex = kzalloc(sizeof(struct device_attribute) *
+ index, GFP_KERNEL);
+ if (!edev->d_attrs_muex) {
+ ret = -ENOMEM;
+ kfree(edev->attrs_muex);
+ goto err_muex;
+ }
+
+ for (index = 0; edev->mutually_exclusive[index]; index++) {
+ sprintf(buf, "0x%x", edev->mutually_exclusive[index]);
+ name = kzalloc(sizeof(char) * (strlen(buf) + 1),
+ GFP_KERNEL);
+ if (!name) {
+ for (index--; index >= 0; index--) {
+ kfree(edev->d_attrs_muex[index].attr.
+ name);
+ }
+ kfree(edev->d_attrs_muex);
+ kfree(edev->attrs_muex);
+ ret = -ENOMEM;
+ goto err_muex;
+ }
+ strcpy(name, buf);
+ edev->d_attrs_muex[index].attr.name = name;
+ edev->d_attrs_muex[index].attr.mode = 0000;
+ edev->attrs_muex[index] = &edev->d_attrs_muex[index]
+ .attr;
+ }
+ edev->attr_g_muex.name = muex_name;
+ edev->attr_g_muex.attrs = edev->attrs_muex;
+
+ }
+
if (edev->max_supported) {
edev->extcon_dev_type.groups =
kzalloc(sizeof(struct attribute_group *) *
- (edev->max_supported + 1), GFP_KERNEL);
+ (edev->max_supported + 2), GFP_KERNEL);
if (!edev->extcon_dev_type.groups) {
ret = -ENOMEM;
goto err_alloc_groups;
@@ -635,6 +732,9 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
for (index = 0; index < edev->max_supported; index++)
edev->extcon_dev_type.groups[index] =
&edev->cables[index].attr_g;
+ if (edev->mutually_exclusive)
+ edev->extcon_dev_type.groups[index] =
+ &edev->attr_g_muex;
edev->dev->type = &edev->extcon_dev_type;
}
@@ -662,6 +762,13 @@ err_dev:
if (edev->max_supported)
kfree(edev->extcon_dev_type.groups);
err_alloc_groups:
+ if (edev->max_supported && edev->mutually_exclusive) {
+ for (index = 0; edev->mutually_exclusive[index]; index++)
+ kfree(edev->d_attrs_muex[index].attr.name);
+ kfree(edev->d_attrs_muex);
+ kfree(edev->attrs_muex);
+ }
+err_muex:
for (index = 0; index < edev->max_supported; index++)
kfree(edev->cables[index].attr_g.name);
err_alloc_cables:
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 99ff60e..c9e2de8 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -78,6 +78,14 @@ struct extcon_cable;
* @supported_cable Array of supported cable name ending with NULL.
* If supported_cable is NULL, cable name related APIs
* are disabled.
+ * @mutually_exclusive Array of mutually exclusive set of cables that cannot
+ * be attached simultaneously. The array should be
+ * ending with NULL or be NULL (no mutually exclusive
+ * cables). For example, if it is { 0x7, 0x30, 0}, then,
+ * {0, 1}, {0, 1, 2}, {0, 2}, {1, 2}, or {4, 5} cannot
+ * be attached simulataneously. {0x7, 0} is equivalent to
+ * {0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
+ * can be no simultaneous connections.
* @print_name An optional callback to override the method to print the
* name of the extcon device.
* @print_state An optional callback to override the method to print the
@@ -103,6 +111,7 @@ struct extcon_dev {
/* --- Optional user initializing data --- */
const char *name;
const char **supported_cable;
+ const u32 *mutually_exclusive;
/* --- Optional callbacks to override class functions --- */
ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
@@ -119,6 +128,10 @@ struct extcon_dev {
/* /sys/class/extcon/.../cable.n/... */
struct device_type extcon_dev_type;
struct extcon_cable *cables;
+ /* /sys/class/extcon/.../mutually_exclusive/... */
+ struct attribute_group attr_g_muex;
+ struct attribute **attrs_muex;
+ struct device_attribute *d_attrs_muex;
};
/**
@@ -179,8 +192,8 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
return edev->state;
}
-extern void extcon_set_state(struct extcon_dev *edev, u32 state);
-extern void extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
+extern int extcon_set_state(struct extcon_dev *edev, u32 state);
+extern int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
/*
* get/set_cable_state access each bit of the 32b encoded state value.
@@ -235,11 +248,16 @@ static inline u32 extcon_get_state(struct extcon_dev *edev)
return 0;
}
-static inline void extcon_set_state(struct extcon_dev *edev, u32 state) { }
+static inline int extcon_set_state(struct extcon_dev *edev, u32 state)
+{
+ return 0;
+}
-static inline void extcon_update_state(struct extcon_dev *edev, u32 mask,
+static inline int extcon_update_state(struct extcon_dev *edev, u32 mask,
u32 state)
-{ }
+{
+ return 0;
+}
static inline int extcon_find_cable_index(struct extcon_dev *edev,
const char *cable_name)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 50+ messages in thread