| Message ID | 20250411172410.25406-1-wentongw@amazon.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] input: misc: Add as1895 hall sensor driver | expand |
Hi Wentong, On Fri, Apr 11, 2025 at 05:23:56PM +0000, Wentong Wu wrote: > The as1895 is designed for omnipolar detection hall-effect > application, such as cover switch, contactless switch, solid > state switch and lid close sensor etc battery operation. > > When the magnetic flux density (B) is larger than operate > point (BOP), the output will be turned on (low), the output > is held until the magnetic flux density (B) is lower than > release point (BRP), then turn off (high). And the connected > GPIO will trigger interrupts to CPU according to the output > change of as1895. > > This driver reports the magnetic field change of as1895 via > input subsystem to notify user space system suspend/resume > status. It can alse report the state change to the external > connectors via the Extcon (External Connector) subsystem. > > Tested-by: Calvin Fang <fangtianwen@huaqin.com> > Signed-off-by: Wentong Wu <wentongw@amazon.com> Thank you for your patch! It seems like this device can be supported using the existing gpio_keys driver, which allows a GPIO to be represented as any key or switch. It does not seem necessary to write an entirely new driver for this device. Please note that while this device does seem very interesting, this driver is also not suitable for mainline because it doesn't follow the style guide (i.e. does not pass checkpatch) and makes heavy use of the legacy GPIO API, among other problems. Please refer to other drivers in mainline for some examples of what is considered acceptable. Kind regards, Jeff LaBundy
Hi Wentong, On 11-Apr-25 19:23, Wentong Wu wrote: > The as1895 is designed for omnipolar detection hall-effect > application, such as cover switch, contactless switch, solid > state switch and lid close sensor etc battery operation. > > When the magnetic flux density (B) is larger than operate > point (BOP), the output will be turned on (low), the output > is held until the magnetic flux density (B) is lower than > release point (BRP), then turn off (high). And the connected > GPIO will trigger interrupts to CPU according to the output > change of as1895. > > This driver reports the magnetic field change of as1895 via > input subsystem to notify user space system suspend/resume > status. It can alse report the state change to the external > connectors via the Extcon (External Connector) subsystem. > > Tested-by: Calvin Fang <fangtianwen@huaqin.com> > Signed-off-by: Wentong Wu <wentongw@amazon.com> Using an extcon device here feels weird/wrong. In an offlist discussion about this you mentioned that: > Could you please share more about the extcon? I just use extcon > to notify other drivers the status change. "extcon" stands for external connector, it is mainly designed for use with micro-usb and/or 3.5 mm jack *connectors* to indicate if there is something or nothing plugged in and if something is plugged in what it is (e.g. charger/ USB-device / USB-host connected, or microphone/ headphones/headset/line-in/line-out/rs232-over-jack connected). Since your LID switch is not a connector of any kind using extcon for this is clearly wrong. Also why do other drivers need to respond to the LID switch, typically the LID switch state is reported to userspace and userspace will then decide to suspend the whole system, or if an external display is connected and active, to ignore the LID switch. So other drivers will get their suspend callback called if they need to act on the LID switch. If other drivers really need to respond to the LID switch for some reason they can install an in kernel input-listener, see: net/rfkill/input.c for an example, especially how that code calls input_register_handler(). As already mentioned by Jeff in his review just to turn a GPIO into an input-device reporting LID_SW you do not need a new driver, you can do this with the proper devicetree description of the switch in combination with the existing gpio-keys driver. So all in all it seems that you do not need this driver at all. Regards, Hans > --- > drivers/input/misc/Kconfig | 9 ++ > drivers/input/misc/Makefile | 1 + > drivers/input/misc/as1895.c | 193 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 203 insertions(+) > create mode 100644 drivers/input/misc/as1895.c > > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig > index 6a852c76331b..6ba26052354b 100644 > --- a/drivers/input/misc/Kconfig > +++ b/drivers/input/misc/Kconfig > @@ -956,4 +956,13 @@ config INPUT_STPMIC1_ONKEY > To compile this driver as a module, choose M here: the > module will be called stpmic1_onkey. > > +config INPUT_AS1895 > + tristate "AS1895 hall sensor support" > + depends on GPIOLIB || COMPILE_TEST > + help > + Say Y here if you have a as1895 hall sensor connected to a GPIO pin. > + > + To compile this driver as a module, choose M here: the > + module will be called as1895. > + > endif > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile > index 4f7f736831ba..38b364a6c8c1 100644 > --- a/drivers/input/misc/Makefile > +++ b/drivers/input/misc/Makefile > @@ -92,3 +92,4 @@ obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o > obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o > obj-$(CONFIG_INPUT_YEALINK) += yealink.o > obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR) += ideapad_slidebar.o > +obj-$(CONFIG_INPUT_AS1895) += as1895.o > diff --git a/drivers/input/misc/as1895.c b/drivers/input/misc/as1895.c > new file mode 100644 > index 000000000000..d87318f1bda4 > --- /dev/null > +++ b/drivers/input/misc/as1895.c > @@ -0,0 +1,193 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. > + */ > + > +#include <linux/extcon-provider.h> > +#include <linux/gpio.h> > +#include <linux/input.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/module.h> > +#include <linux/of_gpio.h> > +#include <linux/platform_device.h> > +#include <linux/pm.h> > +#include <linux/pm_wakeup.h> > +#include <linux/slab.h> > +#include <linux/types.h> > + > +/* Timeout value for processing the wakeup event */ > +#define AS1895_WAKEUP_TIMEOUT_MS 100 > + > +#define AS1895_DRV_NAME "AMZN-AS1895" > + > +struct as1895_dev { > + struct input_dev *input; > + struct extcon_dev *edev; > + struct wakeup_source *ws; > + > + /* the connected gpio pin number */ > + int gpio_pin; > + int irq; > +}; > + > +static const unsigned int as1895_cable[] = { > + EXTCON_MECHANICAL, > + EXTCON_NONE, > +}; > + > +static irqreturn_t as1895_irq_thread_handler(int irq, void *data) > +{ > + struct as1895_dev *as1895 = (struct as1895_dev *)data; > + struct input_dev *input = as1895->input; > + int gpio_val; > + > + /* allow user space to consume the event */ > + __pm_wakeup_event(as1895->ws, AS1895_WAKEUP_TIMEOUT_MS); > + > + gpio_val = gpio_get_value(as1895->gpio_pin); > + > + extcon_set_state_sync(as1895->edev, EXTCON_MECHANICAL, > + gpio_val ? false : true); > + > + input_report_switch(input, SW_LID, !gpio_val); > + > + input_sync(input); > + > + return IRQ_HANDLED; > +} > + > +static int as1895_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct device *dev = &pdev->dev; > + struct as1895_dev *as1895; > + unsigned long irqflags; > + int ret, gpio_pin; > + const char *name; > + > + ret = of_property_read_string(node, "name", &name); > + if (ret) > + return ret; > + > + gpio_pin = of_get_named_gpio(node, "int-gpio", 0); > + if (!gpio_is_valid(gpio_pin)) > + return -EINVAL; > + > + as1895 = devm_kzalloc(dev, sizeof(struct as1895_dev), GFP_KERNEL); > + if (!as1895) > + return -ENOMEM; > + > + as1895->edev = devm_extcon_dev_allocate(dev, as1895_cable); > + if (IS_ERR(as1895->edev)) > + return -ENOMEM; > + > + /* register extcon device */ > + ret = devm_extcon_dev_register(dev, as1895->edev); > + if (ret) { > + dev_err(dev, "can't register extcon device: %d\n", ret); > + return ret; > + } > + > + as1895->input = devm_input_allocate_device(dev); > + if (!as1895->input) > + return -ENOMEM; > + > + as1895->input->name = name; > + set_bit(EV_SW, as1895->input->evbit); > + set_bit(SW_LID, as1895->input->swbit); > + > + /* register input device */ > + ret = input_register_device(as1895->input); > + if (ret) { > + dev_err(dev, "can't register input device: %d\n", ret); > + return ret; > + } > + > + as1895->ws = wakeup_source_register(NULL, name); > + if (!as1895->ws) > + return -ENOMEM; > + > + as1895->gpio_pin = gpio_pin; > + as1895->irq = gpio_to_irq(gpio_pin); > + > + irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT; > + > + ret = request_threaded_irq(as1895->irq, NULL, > + as1895_irq_thread_handler, > + irqflags, name, as1895); > + if (ret) > + goto err_unregister_ws; > + > + platform_set_drvdata(pdev, as1895); > + > + device_init_wakeup(dev, true); > + > + return 0; > + > +err_unregister_ws: > + wakeup_source_unregister(as1895->ws); > + > + return ret; > +} > + > +static void as1895_remove(struct platform_device *pdev) > +{ > + struct as1895_dev *as1895 = platform_get_drvdata(pdev); > + > + device_init_wakeup(&pdev->dev, false); > + > + free_irq(as1895->irq, as1895); > + > + wakeup_source_unregister(as1895->ws); > +} > + > +static int as1895_suspend(struct device *dev) > +{ > + struct as1895_dev *as1895 = dev_get_drvdata(dev); > + > + if (device_may_wakeup(dev)) > + enable_irq_wake(as1895->irq); > + > + return 0; > +} > + > +static int as1895_resume(struct device *dev) > +{ > + struct as1895_dev *as1895 = dev_get_drvdata(dev); > + > + if (device_may_wakeup(dev)) > + disable_irq_wake(as1895->irq); > + > + return 0; > +} > + > +static DEFINE_SIMPLE_DEV_PM_OPS(as1895_pm_ops, as1895_suspend, as1895_resume); > + > +#if IS_ENABLED(CONFIG_OF) > +static const struct of_device_id as1895_of_match[] = { > + { .compatible = "amazon,as1895-hall-sensor" }, > + { /* sentinel */ } > +}; > + > +MODULE_DEVICE_TABLE(of, as1895_of_match); > +#endif > + > +static struct platform_driver as1895_driver = { > + .probe = as1895_probe, > + .remove = as1895_remove, > + .driver = { > + .name = AS1895_DRV_NAME, > + .pm = pm_sleep_ptr(&as1895_pm_ops), > + .of_match_table = of_match_ptr(as1895_of_match), > + } > +}; > + > +module_platform_driver(as1895_driver); > + > +MODULE_AUTHOR("Wentong Wu <wentongw@amazon.com>"); > +MODULE_AUTHOR("Zengjin Zhao <zzengjin@amazon.com>"); > +MODULE_AUTHOR("Xinkai Liu <xinka@amazon.com>"); > +MODULE_AUTHOR("Weihua Wu <wuweihua@amazon.com>"); > +MODULE_DESCRIPTION("Amazon as1895 hall sensor driver"); > +MODULE_LICENSE("GPL");
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 6a852c76331b..6ba26052354b 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -956,4 +956,13 @@ config INPUT_STPMIC1_ONKEY To compile this driver as a module, choose M here: the module will be called stpmic1_onkey. +config INPUT_AS1895 + tristate "AS1895 hall sensor support" + depends on GPIOLIB || COMPILE_TEST + help + Say Y here if you have a as1895 hall sensor connected to a GPIO pin. + + To compile this driver as a module, choose M here: the + module will be called as1895. + endif diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 4f7f736831ba..38b364a6c8c1 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -92,3 +92,4 @@ obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o obj-$(CONFIG_INPUT_YEALINK) += yealink.o obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR) += ideapad_slidebar.o +obj-$(CONFIG_INPUT_AS1895) += as1895.o diff --git a/drivers/input/misc/as1895.c b/drivers/input/misc/as1895.c new file mode 100644 index 000000000000..d87318f1bda4 --- /dev/null +++ b/drivers/input/misc/as1895.c @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. + */ + +#include <linux/extcon-provider.h> +#include <linux/gpio.h> +#include <linux/input.h> +#include <linux/interrupt.h> +#include <linux/irq.h> +#include <linux/module.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/pm.h> +#include <linux/pm_wakeup.h> +#include <linux/slab.h> +#include <linux/types.h> + +/* Timeout value for processing the wakeup event */ +#define AS1895_WAKEUP_TIMEOUT_MS 100 + +#define AS1895_DRV_NAME "AMZN-AS1895" + +struct as1895_dev { + struct input_dev *input; + struct extcon_dev *edev; + struct wakeup_source *ws; + + /* the connected gpio pin number */ + int gpio_pin; + int irq; +}; + +static const unsigned int as1895_cable[] = { + EXTCON_MECHANICAL, + EXTCON_NONE, +}; + +static irqreturn_t as1895_irq_thread_handler(int irq, void *data) +{ + struct as1895_dev *as1895 = (struct as1895_dev *)data; + struct input_dev *input = as1895->input; + int gpio_val; + + /* allow user space to consume the event */ + __pm_wakeup_event(as1895->ws, AS1895_WAKEUP_TIMEOUT_MS); + + gpio_val = gpio_get_value(as1895->gpio_pin); + + extcon_set_state_sync(as1895->edev, EXTCON_MECHANICAL, + gpio_val ? false : true); + + input_report_switch(input, SW_LID, !gpio_val); + + input_sync(input); + + return IRQ_HANDLED; +} + +static int as1895_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct device *dev = &pdev->dev; + struct as1895_dev *as1895; + unsigned long irqflags; + int ret, gpio_pin; + const char *name; + + ret = of_property_read_string(node, "name", &name); + if (ret) + return ret; + + gpio_pin = of_get_named_gpio(node, "int-gpio", 0); + if (!gpio_is_valid(gpio_pin)) + return -EINVAL; + + as1895 = devm_kzalloc(dev, sizeof(struct as1895_dev), GFP_KERNEL); + if (!as1895) + return -ENOMEM; + + as1895->edev = devm_extcon_dev_allocate(dev, as1895_cable); + if (IS_ERR(as1895->edev)) + return -ENOMEM; + + /* register extcon device */ + ret = devm_extcon_dev_register(dev, as1895->edev); + if (ret) { + dev_err(dev, "can't register extcon device: %d\n", ret); + return ret; + } + + as1895->input = devm_input_allocate_device(dev); + if (!as1895->input) + return -ENOMEM; + + as1895->input->name = name; + set_bit(EV_SW, as1895->input->evbit); + set_bit(SW_LID, as1895->input->swbit); + + /* register input device */ + ret = input_register_device(as1895->input); + if (ret) { + dev_err(dev, "can't register input device: %d\n", ret); + return ret; + } + + as1895->ws = wakeup_source_register(NULL, name); + if (!as1895->ws) + return -ENOMEM; + + as1895->gpio_pin = gpio_pin; + as1895->irq = gpio_to_irq(gpio_pin); + + irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT; + + ret = request_threaded_irq(as1895->irq, NULL, + as1895_irq_thread_handler, + irqflags, name, as1895); + if (ret) + goto err_unregister_ws; + + platform_set_drvdata(pdev, as1895); + + device_init_wakeup(dev, true); + + return 0; + +err_unregister_ws: + wakeup_source_unregister(as1895->ws); + + return ret; +} + +static void as1895_remove(struct platform_device *pdev) +{ + struct as1895_dev *as1895 = platform_get_drvdata(pdev); + + device_init_wakeup(&pdev->dev, false); + + free_irq(as1895->irq, as1895); + + wakeup_source_unregister(as1895->ws); +} + +static int as1895_suspend(struct device *dev) +{ + struct as1895_dev *as1895 = dev_get_drvdata(dev); + + if (device_may_wakeup(dev)) + enable_irq_wake(as1895->irq); + + return 0; +} + +static int as1895_resume(struct device *dev) +{ + struct as1895_dev *as1895 = dev_get_drvdata(dev); + + if (device_may_wakeup(dev)) + disable_irq_wake(as1895->irq); + + return 0; +} + +static DEFINE_SIMPLE_DEV_PM_OPS(as1895_pm_ops, as1895_suspend, as1895_resume); + +#if IS_ENABLED(CONFIG_OF) +static const struct of_device_id as1895_of_match[] = { + { .compatible = "amazon,as1895-hall-sensor" }, + { /* sentinel */ } +}; + +MODULE_DEVICE_TABLE(of, as1895_of_match); +#endif + +static struct platform_driver as1895_driver = { + .probe = as1895_probe, + .remove = as1895_remove, + .driver = { + .name = AS1895_DRV_NAME, + .pm = pm_sleep_ptr(&as1895_pm_ops), + .of_match_table = of_match_ptr(as1895_of_match), + } +}; + +module_platform_driver(as1895_driver); + +MODULE_AUTHOR("Wentong Wu <wentongw@amazon.com>"); +MODULE_AUTHOR("Zengjin Zhao <zzengjin@amazon.com>"); +MODULE_AUTHOR("Xinkai Liu <xinka@amazon.com>"); +MODULE_AUTHOR("Weihua Wu <wuweihua@amazon.com>"); +MODULE_DESCRIPTION("Amazon as1895 hall sensor driver"); +MODULE_LICENSE("GPL");