* [PATCH 1/2 V3] dt: add Atmel Captouch bindings
@ 2016-05-16 20:54 Grant Grundler
2016-05-16 20:54 ` [PATCH 2/2 V3] Input: add Atmel Captouch Button driver Grant Grundler
2016-05-18 16:44 ` [PATCH 1/2 V3] dt: add Atmel Captouch bindings Rob Herring
0 siblings, 2 replies; 9+ messages in thread
From: Grant Grundler @ 2016-05-16 20:54 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Dmitry Torokhov
Cc: devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala,
Ian Campbell, Grant Likely, Grant Grundler
From: Daniel Hung-yu Wu <hywu@google.com>
Add binding for Atmel Capacitive Touch Button device.
Signed-off-by: Daniel Hung-yu Wu <hywu@google.com>
Signed-off-by: Grant Grundler <grundler@chromium.org>
---
.../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt
V3: doh! use dash, not underscore in node names. Wordsmith description.
V2: split Documentation/devicetree/bindings from driver patch
rename "atmel,atmegaxx_captouch" to "atmel,captouch" since it's irrelevant
which controller implements the register set/controller.
V1: posted https://lkml.org/lkml/2016/5/3/613
diff --git a/Documentation/devicetree/bindings/input/atmel,captouch.txt b/Documentation/devicetree/bindings/input/atmel,captouch.txt
new file mode 100644
index 0000000..405d287
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/atmel,captouch.txt
@@ -0,0 +1,36 @@
+Device tree bindings for Atmel capacitive touch device, typically
+an Atmel touch sensor connected to AtmegaXX MCU running firmware
+based on Qtouch library.
+
+The node for this device must be a child of a I2C controller node, as the
+device communicates via I2C.
+
+Required properties:
+
+ compatible: Must be "atmel,captouch".
+ reg: The I2C slave address of the device.
+ interrupts: Property describing the interrupt line the device
+ is connected to. The device only has one interrupt
+ source.
+ linux,keycodes: Specifies an array of numeric keycode values to
+ be used for reporting button presses. The array can
+ contain up to 8 entries.
+
+Optional properties:
+
+ autorepeat: Enables the Linux input system's autorepeat
+ feature on the input device.
+
+Example:
+
+ atmel-captouch@51 {
+ compatible = "atmel,captouch";
+ reg = <0x51>;
+ interrupt-parent = <&tlmm>;
+ interrupts = <67 IRQ_TYPE_EDGE_FALLING>;
+ linux,keycodes = <BTN_0>, <BTN_1>,
+ <BTN_2>, <BTN_3>,
+ <BTN_4>, <BTN_5>,
+ <BTN_6>, <BTN_7>;
+ autorepeat;
+ };
--
2.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/2 V3] Input: add Atmel Captouch Button driver 2016-05-16 20:54 [PATCH 1/2 V3] dt: add Atmel Captouch bindings Grant Grundler @ 2016-05-16 20:54 ` Grant Grundler 2016-05-18 16:44 ` [PATCH 1/2 V3] dt: add Atmel Captouch bindings Rob Herring 1 sibling, 0 replies; 9+ messages in thread From: Grant Grundler @ 2016-05-16 20:54 UTC (permalink / raw) To: Rob Herring, Pawel Moll, Mark Rutland, Dmitry Torokhov Cc: devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely, Grant Grundler From: Daniel Hung-yu Wu <hywu@google.com> Add I2C driver for Atmel Capacitive Touch Button device. Signed-off-by: Hung-yu Wu <hywu@google.com> Signed-off-by: Grant Grundler <grundler@chromium.org> --- drivers/input/misc/Kconfig | 13 ++ drivers/input/misc/Makefile | 1 + drivers/input/misc/atmel_captouch.c | 287 ++++++++++++++++++++++++++++++++++++ 3 files changed, 299 insertions(+) create mode 100644 drivers/input/misc/atmel_captouch.c V3: use dash, not underscore in nodename. Wordsmith Kconfig help text. V2: split Documentation/devicetree/bindings from driver patch. rename "atmel,atmegaxx_captouch" to "atmel,captouch" since it's irrelevant which controller implements the register set/controller. V1: posted https://lkml.org/lkml/2016/5/3/613 diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 838824b..d8b6620 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -82,6 +82,19 @@ config INPUT_ARIZONA_HAPTICS To compile this driver as a module, choose M here: the module will be called arizona-haptics. +config INPUT_ATMEL_CAPTOUCH + tristate "Atmel Capacitive Touch Button Driver" + depends on OF || COMPILE_TEST + help + Say Y here if an Atmel Capacitive Touch Button device which + implements "captouch" protocol is connected to I2C bus. Typically + this device consists of Atmel Touch sensor controlled by AtMegaXX + MCU running firmware based on Qtouch library. + One should find "atmel,captouch" node in the board specific DTS. + + To compile this driver as a module, choose M here: the + module will be called atmel_captouch. + config INPUT_BMA150 tristate "BMA150/SMB380 acceleration sensor support" depends on I2C diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index f77268e..b5ec626 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_INPUT_APANEL) += apanel.o obj-$(CONFIG_INPUT_ARIZONA_HAPTICS) += arizona-haptics.o obj-$(CONFIG_INPUT_ATI_REMOTE2) += ati_remote2.o obj-$(CONFIG_INPUT_ATLAS_BTNS) += atlas_btns.o +obj-$(CONFIG_INPUT_ATMEL_CAPTOUCH) += atmel_captouch.o obj-$(CONFIG_INPUT_BFIN_ROTARY) += bfin_rotary.o obj-$(CONFIG_INPUT_BMA150) += bma150.o obj-$(CONFIG_INPUT_CM109) += cm109.o diff --git a/drivers/input/misc/atmel_captouch.c b/drivers/input/misc/atmel_captouch.c new file mode 100644 index 0000000..91bfc73 --- /dev/null +++ b/drivers/input/misc/atmel_captouch.c @@ -0,0 +1,287 @@ +/* + * Atmel Atmegaxx Capacitive Touch Button Driver + * + * Copyright (C) 2016 Google, inc. + * + * 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. + */ + +/* + * It's irrelevant the HW used to develop captouch driver is based + * on Atmega88PA part and uses QtouchADC parts for sensing touch. + * Calling this driver "captouch" is an arbitrary way to distinguish + * the protocol this driver supported by other atmel/qtouch drivers. + * + * captouch driver supports a newer/different version of the I2C + * registers/commands than the qt1070.c driver. + * Don't let the similarity of the general driver structure fool you. :) + * + * For raw i2c access from userspace, use i2cset/i2cget + * to poke at /dev/i2c-N devices. + */ + +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/i2c.h> +#include <linux/input.h> +#include <linux/interrupt.h> +#include <linux/slab.h> + +/* Maximum number of buttons supported */ +#define MAX_NUM_OF_BUTTONS 8 + +/* Registers */ +#define REG_KEY1_THRESHOLD 0x02 +#define REG_KEY2_THRESHOLD 0x03 +#define REG_KEY3_THRESHOLD 0x04 +#define REG_KEY4_THRESHOLD 0x05 + +#define REG_KEY1_REF_H 0x20 +#define REG_KEY1_REF_L 0x21 +#define REG_KEY2_REF_H 0x22 +#define REG_KEY2_REF_L 0x23 +#define REG_KEY3_REF_H 0x24 +#define REG_KEY3_REF_L 0x25 +#define REG_KEY4_REF_H 0x26 +#define REG_KEY4_REF_L 0x27 + +#define REG_KEY1_DLT_H 0x30 +#define REG_KEY1_DLT_L 0x31 +#define REG_KEY2_DLT_H 0x32 +#define REG_KEY2_DLT_L 0x33 +#define REG_KEY3_DLT_H 0x34 +#define REG_KEY3_DLT_L 0x35 +#define REG_KEY4_DLT_H 0x36 +#define REG_KEY4_DLT_L 0x37 + +#define REG_KEY_STATE 0x3C + +/* + * @i2c_client: I2C slave device client pointer + * @input: Input device pointer + * @num_btn: Number of buttons + * @keycodes: map of button# to KeyCode + * @prev_btn: Previous key state to detect button "press" or "release" + * @xfer_buf: I2C transfer buffer + */ +struct atmel_captouch_device { + struct i2c_client *client; + struct input_dev *input; + u32 num_btn; + u32 keycodes[MAX_NUM_OF_BUTTONS]; + u8 prev_btn; + u8 xfer_buf[8] ____cacheline_aligned; +}; + +/* + * Read from I2C slave device + * The protocol is that the client has to provide both the register address + * and the length, and while reading back the device would prepend the data + * with address and length for verification. + */ +static int atmel_read(struct atmel_captouch_device *capdev, + u8 reg, u8 *data, size_t len) +{ + struct i2c_client *client = capdev->client; + struct device *dev = &client->dev; + struct i2c_msg msg[2]; + int err; + + if (len > sizeof(capdev->xfer_buf) - 2) + return -EINVAL; + + capdev->xfer_buf[0] = reg; + capdev->xfer_buf[1] = len; + + msg[0].addr = client->addr; + msg[0].flags = 0; + msg[0].buf = capdev->xfer_buf; + msg[0].len = 2; + + msg[1].addr = client->addr; + msg[1].flags = I2C_M_RD; + msg[1].buf = capdev->xfer_buf; + msg[1].len = len + 2; + + err = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); + if (err != ARRAY_SIZE(msg)) + return err < 0 ? err : -EIO; + + if (capdev->xfer_buf[0] != reg) { + dev_err(dev, "I2C read error: register address does not match\n"); + return -ECOMM; + } + + memcpy(data, &capdev->xfer_buf[2], len); + + return 0; +} + +/* + * Handle interrupt and report the key changes to the input system. + * Multi-touch can be supported; however, it really depends on whether + * the device can multi-touch. + */ +static irqreturn_t atmel_captouch_isr(int irq, void *data) +{ + struct atmel_captouch_device *capdev = data; + struct device *dev = &capdev->client->dev; + int error; + int i; + u8 new_btn; + u8 changed_btn; + + error = atmel_read(capdev, REG_KEY_STATE, &new_btn, 1); + if (error) { + dev_err(dev, "failed to read button state: %d\n", error); + goto out; + } + + dev_dbg(dev, "%s: button state %#02x\n", __func__, new_btn); + + changed_btn = new_btn ^ capdev->prev_btn; + capdev->prev_btn = new_btn; + + for (i = 0; i < capdev->num_btn; i++) { + if (changed_btn & BIT(i)) + input_report_key(capdev->input, + capdev->keycodes[i], + new_btn & BIT(i)); + } + + input_sync(capdev->input); + +out: + return IRQ_HANDLED; +} + +/* + * Probe function to setup the device, input system and interrupt + */ +static int atmel_captouch_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct atmel_captouch_device *capdev; + struct device *dev = &client->dev; + struct device_node *node; + int i; + int err; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_BYTE_DATA | + I2C_FUNC_SMBUS_WORD_DATA | + I2C_FUNC_SMBUS_I2C_BLOCK)) { + dev_err(dev, "needed i2c functionality is not supported\n"); + return -EINVAL; + } + + capdev = devm_kzalloc(dev, sizeof(*capdev), GFP_KERNEL); + if (!capdev) + return -ENOMEM; + + capdev->client = client; + i2c_set_clientdata(client, capdev); + + err = atmel_read(capdev, REG_KEY_STATE, + &capdev->prev_btn, sizeof(capdev->prev_btn)); + if (err) { + dev_err(dev, "failed to read initial button state: %d\n", err); + return err; + } + + capdev->input = devm_input_allocate_device(dev); + if (!capdev->input) { + dev_err(dev, "failed to allocate input device\n"); + return -ENOMEM; + } + + capdev->input->id.bustype = BUS_I2C; + capdev->input->id.product = 0x880A; + capdev->input->id.version = 0; + capdev->input->name = "ATMegaXX Capacitive Button Controller"; + __set_bit(EV_KEY, capdev->input->evbit); + + node = dev->of_node; + if (!node) { + dev_err(dev, "failed to find matching node in device tree\n"); + return -EINVAL; + } + + if (of_property_read_bool(node, "autorepeat")) + __set_bit(EV_REP, capdev->input->evbit); + + capdev->num_btn = of_property_count_u32_elems(node, "linux,keymap"); + if (capdev->num_btn > MAX_NUM_OF_BUTTONS) + capdev->num_btn = MAX_NUM_OF_BUTTONS; + + err = of_property_read_u32_array(node, "linux,keycodes", + capdev->keycodes, + capdev->num_btn); + if (err) { + dev_err(dev, + "failed to read linux,keycode property: %d\n", err); + return err; + } + + for (i = 0; i < capdev->num_btn; i++) + __set_bit(capdev->keycodes[i], capdev->input->keybit); + + capdev->input->keycode = capdev->keycodes; + capdev->input->keycodesize = sizeof(capdev->keycodes[0]); + capdev->input->keycodemax = capdev->num_btn; + + err = input_register_device(capdev->input); + if (err) + return err; + + err = devm_request_threaded_irq(dev, client->irq, NULL, + atmel_captouch_isr, IRQF_ONESHOT, + "atmel_captouch", capdev); + if (err) { + dev_err(dev, "failed to request irq %d: %d\n", + client->irq, err); + return err; + } + + return 0; +} + +#ifdef CONFIG_OF +static const struct of_device_id atmel_captouch_of_id[] = { + { + .compatible = "atmel,captouch", + }, + { /* sentinel */ } +} +MODULE_DEVICE_TABLE(of, atmel_captouch_of_id); +#endif + +static const struct i2c_device_id atmel_captouch_id[] = { + { "atmel_captouch", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, atmel_captouch_id); + +static struct i2c_driver atmel_captouch_driver = { + .probe = atmel_captouch_probe, + .id_table = atmel_captouch_id, + .driver = { + .name = "atmel_captouch", + .of_match_table = of_match_ptr(atmel_captouch_of_id), + }, +}; +module_i2c_driver(atmel_captouch_driver); + +/* Module information */ +MODULE_AUTHOR("Hung-yu Wu <hywu@google.com>"); +MODULE_DESCRIPTION("Atmel ATmegaXX Capacitance Touch Sensor I2C Driver"); +MODULE_LICENSE("GPL v2"); -- 2.1.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-16 20:54 [PATCH 1/2 V3] dt: add Atmel Captouch bindings Grant Grundler 2016-05-16 20:54 ` [PATCH 2/2 V3] Input: add Atmel Captouch Button driver Grant Grundler @ 2016-05-18 16:44 ` Rob Herring 2016-05-18 21:59 ` Dmitry Torokhov 1 sibling, 1 reply; 9+ messages in thread From: Rob Herring @ 2016-05-18 16:44 UTC (permalink / raw) To: Grant Grundler Cc: Pawel Moll, Mark Rutland, Dmitry Torokhov, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: > From: Daniel Hung-yu Wu <hywu@google.com> > > Add binding for Atmel Capacitive Touch Button device. > > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> > Signed-off-by: Grant Grundler <grundler@chromium.org> > --- > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-18 16:44 ` [PATCH 1/2 V3] dt: add Atmel Captouch bindings Rob Herring @ 2016-05-18 21:59 ` Dmitry Torokhov 2016-05-18 22:26 ` Grant Grundler 2016-05-20 12:56 ` Rob Herring 0 siblings, 2 replies; 9+ messages in thread From: Dmitry Torokhov @ 2016-05-18 21:59 UTC (permalink / raw) To: Rob Herring Cc: Grant Grundler, Pawel Moll, Mark Rutland, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote: > On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: > > From: Daniel Hung-yu Wu <hywu@google.com> > > > > Add binding for Atmel Capacitive Touch Button device. > > > > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> > > Signed-off-by: Grant Grundler <grundler@chromium.org> > > --- > > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ > > 1 file changed, 34 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt > > Acked-by: Rob Herring <robh@kernel.org> Folded into the driver patch and applied. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-18 21:59 ` Dmitry Torokhov @ 2016-05-18 22:26 ` Grant Grundler 2016-05-20 12:56 ` Rob Herring 1 sibling, 0 replies; 9+ messages in thread From: Grant Grundler @ 2016-05-18 22:26 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rob Herring, Grant Grundler, Pawel Moll, Mark Rutland, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Wed, May 18, 2016 at 2:59 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote: >> On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: >> > From: Daniel Hung-yu Wu <hywu@google.com> >> > >> > Add binding for Atmel Capacitive Touch Button device. >> > >> > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> >> > Signed-off-by: Grant Grundler <grundler@chromium.org> >> > --- >> > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ >> > 1 file changed, 34 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt >> >> Acked-by: Rob Herring <robh@kernel.org> > > Folded into the driver patch and applied. > > Thanks. Thanks Rob! Thanks Dmitry! :) grant > > -- > Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-18 21:59 ` Dmitry Torokhov 2016-05-18 22:26 ` Grant Grundler @ 2016-05-20 12:56 ` Rob Herring 2016-05-20 16:16 ` Dmitry Torokhov 1 sibling, 1 reply; 9+ messages in thread From: Rob Herring @ 2016-05-20 12:56 UTC (permalink / raw) To: Dmitry Torokhov Cc: Grant Grundler, Pawel Moll, Mark Rutland, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Wed, May 18, 2016 at 4:59 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote: >> On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: >> > From: Daniel Hung-yu Wu <hywu@google.com> >> > >> > Add binding for Atmel Capacitive Touch Button device. >> > >> > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> >> > Signed-off-by: Grant Grundler <grundler@chromium.org> >> > --- >> > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ >> > 1 file changed, 34 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt >> >> Acked-by: Rob Herring <robh@kernel.org> > > Folded into the driver patch and applied. Folded why? Please don't do that. You should be committing what is posted as is for the most part. We specifically ask that binding changes are kept separate commits. It also messes up the ability to correlate git commits to patchworks or mail searches. Rob ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-20 12:56 ` Rob Herring @ 2016-05-20 16:16 ` Dmitry Torokhov 2016-05-20 22:12 ` Rob Herring 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Torokhov @ 2016-05-20 16:16 UTC (permalink / raw) To: Rob Herring Cc: Grant Grundler, Pawel Moll, Mark Rutland, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Fri, May 20, 2016 at 07:56:51AM -0500, Rob Herring wrote: > On Wed, May 18, 2016 at 4:59 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote: > >> On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: > >> > From: Daniel Hung-yu Wu <hywu@google.com> > >> > > >> > Add binding for Atmel Capacitive Touch Button device. > >> > > >> > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> > >> > Signed-off-by: Grant Grundler <grundler@chromium.org> > >> > --- > >> > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ > >> > 1 file changed, 34 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt > >> > >> Acked-by: Rob Herring <robh@kernel.org> > > > > Folded into the driver patch and applied. > > Folded why? Please don't do that. You should be committing what is > posted as is for the most part. We specifically ask that binding > changes are kept separate commits. It also messes up the ability to I know that you ask for binding docs to be posted separately (I guess so that devicetree list is not overrun with driver code mails), but logically driver patch and binding doc patch are a single change and should be committed together, so that when I am researching the history I can easily see what was introduced and when. You do not require header changes to be submitted separately form .c files, do you? > correlate git commits to patchworks or mail searches. The fact that it was applied can be found in mail archives. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-20 16:16 ` Dmitry Torokhov @ 2016-05-20 22:12 ` Rob Herring 2016-05-20 22:21 ` Dmitry Torokhov 0 siblings, 1 reply; 9+ messages in thread From: Rob Herring @ 2016-05-20 22:12 UTC (permalink / raw) To: Dmitry Torokhov Cc: Grant Grundler, Pawel Moll, Mark Rutland, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Fri, May 20, 2016 at 11:16 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Fri, May 20, 2016 at 07:56:51AM -0500, Rob Herring wrote: >> On Wed, May 18, 2016 at 4:59 PM, Dmitry Torokhov >> <dmitry.torokhov@gmail.com> wrote: >> > On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote: >> >> On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: >> >> > From: Daniel Hung-yu Wu <hywu@google.com> >> >> > >> >> > Add binding for Atmel Capacitive Touch Button device. >> >> > >> >> > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> >> >> > Signed-off-by: Grant Grundler <grundler@chromium.org> >> >> > --- >> >> > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ >> >> > 1 file changed, 34 insertions(+) >> >> > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt >> >> >> >> Acked-by: Rob Herring <robh@kernel.org> >> > >> > Folded into the driver patch and applied. >> >> Folded why? Please don't do that. You should be committing what is >> posted as is for the most part. We specifically ask that binding >> changes are kept separate commits. It also messes up the ability to > > I know that you ask for binding docs to be posted separately (I guess > so that devicetree list is not overrun with driver code mails), And because we're really only reviewing the binding, so putting my ack on the driver is not really correct. > but > logically driver patch and binding doc patch are a single change and > should be committed together, so that when I am researching the history > I can easily see what was introduced and when. You do not require header > changes to be submitted separately form .c files, do you? Yes, for include/dt-bindings we ask that they are part of the binding doc, not the driver even though both use it. You can also certainly have bindings without drivers though generally we require them. I would not if they had a driver in BSD or u-boot for example. You can already easily see when things are introduced because they will be next to each other in the git history. >> correlate git commits to patchworks or mail searches. > > The fact that it was applied can be found in mail archives. Yes, with extra effort reading the history you can, but not with a script. There's a patchwork script to add commit hashes to patchwork which works all based on the subject. Regardless of one commit or two, you simply shouldn't be changing what you commit. Either commit what was posted or require the author to combine things and repost. That's our job as patch monkeys. Rob ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings 2016-05-20 22:12 ` Rob Herring @ 2016-05-20 22:21 ` Dmitry Torokhov 0 siblings, 0 replies; 9+ messages in thread From: Dmitry Torokhov @ 2016-05-20 22:21 UTC (permalink / raw) To: Rob Herring Cc: Grant Grundler, Pawel Moll, Mark Rutland, devicetree, linux-input, LKML, Hung-yu Wu, Kumar Gala, Ian Campbell, Grant Likely On Fri, May 20, 2016 at 05:12:55PM -0500, Rob Herring wrote: > On Fri, May 20, 2016 at 11:16 AM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Fri, May 20, 2016 at 07:56:51AM -0500, Rob Herring wrote: > >> On Wed, May 18, 2016 at 4:59 PM, Dmitry Torokhov > >> <dmitry.torokhov@gmail.com> wrote: > >> > On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote: > >> >> On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote: > >> >> > From: Daniel Hung-yu Wu <hywu@google.com> > >> >> > > >> >> > Add binding for Atmel Capacitive Touch Button device. > >> >> > > >> >> > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com> > >> >> > Signed-off-by: Grant Grundler <grundler@chromium.org> > >> >> > --- > >> >> > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++ > >> >> > 1 file changed, 34 insertions(+) > >> >> > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt > >> >> > >> >> Acked-by: Rob Herring <robh@kernel.org> > >> > > >> > Folded into the driver patch and applied. > >> > >> Folded why? Please don't do that. You should be committing what is > >> posted as is for the most part. We specifically ask that binding > >> changes are kept separate commits. It also messes up the ability to > > > > I know that you ask for binding docs to be posted separately (I guess > > so that devicetree list is not overrun with driver code mails), > > And because we're really only reviewing the binding, so putting my ack > on the driver is not really correct. If you'd like I can annotate your Acks to state that they are for bindings only when I fold everything together. > > > but > > logically driver patch and binding doc patch are a single change and > > should be committed together, so that when I am researching the history > > I can easily see what was introduced and when. You do not require header > > changes to be submitted separately form .c files, do you? > > Yes, for include/dt-bindings we ask that they are part of the binding > doc, not the driver even though both use it. You can also certainly > have bindings without drivers though generally we require them. I > would not if they had a driver in BSD or u-boot for example. Would they be in linux kernel sources then? I can see rules changed if ever DTS/bindings are split from kernel, but while they are kept together I do not see why we'd want to keep commits separate. > > You can already easily see when things are introduced because they > will be next to each other in the git history. > > >> correlate git commits to patchworks or mail searches. > > > > The fact that it was applied can be found in mail archives. > > Yes, with extra effort reading the history you can, but not with a > script. There's a patchwork script to add commit hashes to patchwork > which works all based on the subject. If you'd like I can teach my scripts to update patches in your patchwork instance when I do folds. > > Regardless of one commit or two, you simply shouldn't be changing what > you commit. Either commit what was posted or require the author to > combine things and repost. That's our job as patch monkeys. Or I do a bit of work on my end (i.e. add a few "depends on" that were missed in case I notice them) and not ask for yet another respin. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-05-20 22:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-16 20:54 [PATCH 1/2 V3] dt: add Atmel Captouch bindings Grant Grundler 2016-05-16 20:54 ` [PATCH 2/2 V3] Input: add Atmel Captouch Button driver Grant Grundler 2016-05-18 16:44 ` [PATCH 1/2 V3] dt: add Atmel Captouch bindings Rob Herring 2016-05-18 21:59 ` Dmitry Torokhov 2016-05-18 22:26 ` Grant Grundler 2016-05-20 12:56 ` Rob Herring 2016-05-20 16:16 ` Dmitry Torokhov 2016-05-20 22:12 ` Rob Herring 2016-05-20 22:21 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).