Commit 2158091d authored by Linus Torvalds's avatar Linus Torvalds

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input

Pull input updates from Dmitry Torokhov:

 - a new driver to ChipOne icn8505 based touchscreens

 - on certain systems with Elan touch controllers they will be switched
   away form PS/2 emulation and over to native SMbus mode

 - assorted driver fixups and improvements

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input: (24 commits)
  Input: elan_i2c - add ELAN0612 (Lenovo v330 14IKB) ACPI ID
  Input: goodix - add new ACPI id for GPD Win 2 touch screen
  Input: xpad - add GPD Win 2 Controller USB IDs
  Input: ti_am335x_tsc - prevent system suspend when TSC is in use
  Input: ti_am335x_tsc - ack pending IRQs at probe and before suspend
  Input: cros_ec_keyb - mark cros_ec_keyb driver as wake enabled device.
  Input: mk712 - update documentation web link
  Input: atmel_mxt_ts - fix reset-gpio for level based irqs
  Input: atmel_mxt_ts - require device properties present when probing
  Input: psmouse-smbus - allow to control psmouse_deactivate
  Input: elantech - detect new ICs and setup Host Notify for them
  Input: elantech - add support for SMBus devices
  Input: elantech - query the resolution in query_info
  Input: elantech - split device info into a separate structure
  Input: elan_i2c - add trackstick report
  Input: usbtouchscreen - add sysfs attribute for 3M MTouch firmware rev
  Input: ati_remote2 - fix typo 'can by' to 'can be'
  Input: replace hard coded string with __func__ in pr_err()
  Input: add support for ChipOne icn8505 based touchscreens
  Input: gamecon - avoid using __set_bit() for capabilities
  ...
parents 3e1a29b3 e6e7e9cd
......@@ -14,6 +14,7 @@ Optional properties:
- pinctrl-0: a phandle pointing to the pin settings for the device (see
pinctrl binding [1]).
- vcc-supply: a phandle for the regulator supplying 3.3V power.
- elan,trackpoint: touchpad can support a trackpoint (boolean)
[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
[1]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
......
......@@ -3431,6 +3431,12 @@ S: Maintained
F: Documentation/devicetree/bindings/input/touchscreen/chipone_icn8318.txt
F: drivers/input/touchscreen/chipone_icn8318.c
CHIPONE ICN8505 I2C TOUCHSCREEN DRIVER
M: Hans de Goede <hdegoede@redhat.com>
L: linux-input@vger.kernel.org
S: Maintained
F: drivers/input/touchscreen/chipone_icn8505.c
CHROME HARDWARE PLATFORM SUPPORT
M: Benson Leung <bleung@chromium.org>
M: Olof Johansson <olof@lixom.net>
......
......@@ -1943,8 +1943,7 @@ void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int
break;
default:
pr_err("input_set_capability: unknown type %u (code %u)\n",
type, code);
pr_err("%s: unknown type %u (code %u)\n", __func__, type, code);
dump_stack();
return;
}
......
......@@ -269,9 +269,7 @@ static int as5011_probe(struct i2c_client *client,
input_dev->id.bustype = BUS_I2C;
input_dev->dev.parent = &client->dev;
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(EV_ABS, input_dev->evbit);
__set_bit(BTN_JOYSTICK, input_dev->keybit);
input_set_capability(input_dev, EV_KEY, BTN_JOYSTICK);
input_set_abs_params(input_dev, ABS_X,
AS5011_MIN_AXIS, AS5011_MAX_AXIS, AS5011_FUZZ, AS5011_FLAT);
......
......@@ -862,7 +862,7 @@ static int gc_setup_pad(struct gc *gc, int idx, int pad_type)
case GC_N64:
for (i = 0; i < 10; i++)
__set_bit(gc_n64_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, gc_n64_btn[i]);
for (i = 0; i < 2; i++) {
input_set_abs_params(input_dev, ABS_X + i, -127, 126, 0, 2);
......@@ -879,26 +879,27 @@ static int gc_setup_pad(struct gc *gc, int idx, int pad_type)
break;
case GC_SNESMOUSE:
__set_bit(BTN_LEFT, input_dev->keybit);
__set_bit(BTN_RIGHT, input_dev->keybit);
__set_bit(REL_X, input_dev->relbit);
__set_bit(REL_Y, input_dev->relbit);
input_set_capability(input_dev, EV_KEY, BTN_LEFT);
input_set_capability(input_dev, EV_KEY, BTN_RIGHT);
input_set_capability(input_dev, EV_REL, REL_X);
input_set_capability(input_dev, EV_REL, REL_Y);
break;
case GC_SNES:
for (i = 4; i < 8; i++)
__set_bit(gc_snes_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, gc_snes_btn[i]);
/* fall through */
case GC_NES:
for (i = 0; i < 4; i++)
__set_bit(gc_snes_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, gc_snes_btn[i]);
break;
case GC_MULTI2:
__set_bit(BTN_THUMB, input_dev->keybit);
input_set_capability(input_dev, EV_KEY, BTN_THUMB);
/* fall through */
case GC_MULTI:
__set_bit(BTN_TRIGGER, input_dev->keybit);
input_set_capability(input_dev, EV_KEY, BTN_TRIGGER);
/* fall through */
break;
case GC_PSX:
......@@ -906,15 +907,17 @@ static int gc_setup_pad(struct gc *gc, int idx, int pad_type)
input_set_abs_params(input_dev,
gc_psx_abs[i], 4, 252, 0, 2);
for (i = 0; i < 12; i++)
__set_bit(gc_psx_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, gc_psx_btn[i]);
break;
break;
case GC_DDR:
for (i = 0; i < 4; i++)
__set_bit(gc_psx_ddr_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY,
gc_psx_ddr_btn[i]);
for (i = 0; i < 12; i++)
__set_bit(gc_psx_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, gc_psx_btn[i]);
break;
}
......
......@@ -86,8 +86,10 @@
#define XPAD_PKT_LEN 64
/* xbox d-pads should map to buttons, as is required for DDR pads
but we map them to axes when possible to simplify things */
/*
* xbox d-pads should map to buttons, as is required for DDR pads
* but we map them to axes when possible to simplify things
*/
#define MAP_DPAD_TO_BUTTONS (1 << 0)
#define MAP_TRIGGERS_TO_BUTTONS (1 << 1)
#define MAP_STICKS_TO_NULL (1 << 2)
......@@ -123,6 +125,7 @@ static const struct xpad_device {
u8 mapping;
u8 xtype;
} xpad_device[] = {
{ 0x0079, 0x18d4, "GPD Win 2 Controller", 0, XTYPE_XBOX360 },
{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
......@@ -387,15 +390,15 @@ static const signed short xpad_abs_triggers[] = {
* match against vendor id as well. Wired Xbox 360 devices have protocol 1,
* wireless controllers have protocol 129.
*/
#define XPAD_XBOX360_VENDOR_PROTOCOL(vend,pr) \
#define XPAD_XBOX360_VENDOR_PROTOCOL(vend, pr) \
.match_flags = USB_DEVICE_ID_MATCH_VENDOR | USB_DEVICE_ID_MATCH_INT_INFO, \
.idVendor = (vend), \
.bInterfaceClass = USB_CLASS_VENDOR_SPEC, \
.bInterfaceSubClass = 93, \
.bInterfaceProtocol = (pr)
#define XPAD_XBOX360_VENDOR(vend) \
{ XPAD_XBOX360_VENDOR_PROTOCOL(vend,1) }, \
{ XPAD_XBOX360_VENDOR_PROTOCOL(vend,129) }
{ XPAD_XBOX360_VENDOR_PROTOCOL((vend), 1) }, \
{ XPAD_XBOX360_VENDOR_PROTOCOL((vend), 129) }
/* The Xbox One controller uses subclass 71 and protocol 208. */
#define XPAD_XBOXONE_VENDOR_PROTOCOL(vend, pr) \
......@@ -405,10 +408,11 @@ static const signed short xpad_abs_triggers[] = {
.bInterfaceSubClass = 71, \
.bInterfaceProtocol = (pr)
#define XPAD_XBOXONE_VENDOR(vend) \
{ XPAD_XBOXONE_VENDOR_PROTOCOL(vend, 208) }
{ XPAD_XBOXONE_VENDOR_PROTOCOL((vend), 208) }
static const struct usb_device_id xpad_table[] = {
{ USB_INTERFACE_INFO('X', 'B', 0) }, /* X-Box USB-IF not approved class */
XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 Controller */
XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster X-Box 360 controllers */
XPAD_XBOX360_VENDOR(0x045e), /* Microsoft X-Box 360 controllers */
XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft X-Box One controllers */
......@@ -1573,7 +1577,6 @@ static void xpad_close(struct input_dev *dev)
static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
{
struct usb_xpad *xpad = input_get_drvdata(input_dev);
set_bit(abs, input_dev->absbit);
switch (abs) {
case ABS_X:
......@@ -1593,6 +1596,9 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
case ABS_HAT0Y: /* the d-pad (only if dpad is mapped to axes */
input_set_abs_params(input_dev, abs, -1, 1, 0, 0);
break;
default:
input_set_abs_params(input_dev, abs, 0, 0, 0, 0);
break;
}
}
......@@ -1633,10 +1639,7 @@ static int xpad_init_input(struct usb_xpad *xpad)
input_dev->close = xpad_close;
}
__set_bit(EV_KEY, input_dev->evbit);
if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
__set_bit(EV_ABS, input_dev->evbit);
/* set up axes */
for (i = 0; xpad_abs[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs[i]);
......@@ -1644,21 +1647,22 @@ static int xpad_init_input(struct usb_xpad *xpad)
/* set up standard buttons */
for (i = 0; xpad_common_btn[i] >= 0; i++)
__set_bit(xpad_common_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, xpad_common_btn[i]);
/* set up model-specific ones */
if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
xpad->xtype == XTYPE_XBOXONE) {
for (i = 0; xpad360_btn[i] >= 0; i++)
__set_bit(xpad360_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, xpad360_btn[i]);
} else {
for (i = 0; xpad_btn[i] >= 0; i++)
__set_bit(xpad_btn[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY, xpad_btn[i]);
}
if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
for (i = 0; xpad_btn_pad[i] >= 0; i++)
__set_bit(xpad_btn_pad[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY,
xpad_btn_pad[i]);
}
/*
......@@ -1675,7 +1679,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
for (i = 0; xpad_btn_triggers[i] >= 0; i++)
__set_bit(xpad_btn_triggers[i], input_dev->keybit);
input_set_capability(input_dev, EV_KEY,
xpad_btn_triggers[i]);
} else {
for (i = 0; xpad_abs_triggers[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
......
......@@ -244,24 +244,35 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
switch (ckdev->ec->event_data.event_type) {
case EC_MKBP_EVENT_KEY_MATRIX:
if (device_may_wakeup(ckdev->dev)) {
pm_wakeup_event(ckdev->dev, 0);
} else {
/*
* If EC is not the wake source, discard key state changes
* during suspend.
* If keyboard is not wake enabled, discard key state
* changes during suspend. Switches will be re-checked
* in cros_ec_keyb_resume() to be sure nothing is lost.
*/
if (queued_during_suspend)
return NOTIFY_OK;
}
if (ckdev->ec->event_size != ckdev->cols) {
dev_err(ckdev->dev,
"Discarded incomplete key matrix event.\n");
return NOTIFY_OK;
}
cros_ec_keyb_process(ckdev,
ckdev->ec->event_data.data.key_matrix,
ckdev->ec->event_size);
break;
case EC_MKBP_EVENT_SYSRQ:
if (device_may_wakeup(ckdev->dev))
pm_wakeup_event(ckdev->dev, 0);
else if (queued_during_suspend)
return NOTIFY_OK;
val = get_unaligned_le32(&ckdev->ec->event_data.data.sysrq);
dev_dbg(ckdev->dev, "sysrq code from EC: %#x\n", val);
handle_sysrq(val);
......@@ -269,12 +280,9 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
case EC_MKBP_EVENT_BUTTON:
case EC_MKBP_EVENT_SWITCH:
/*
* If EC is not the wake source, discard key state
* changes during suspend. Switches will be re-checked in
* cros_ec_keyb_resume() to be sure nothing is lost.
*/
if (queued_during_suspend)
if (device_may_wakeup(ckdev->dev))
pm_wakeup_event(ckdev->dev, 0);
else if (queued_during_suspend)
return NOTIFY_OK;
if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
......@@ -639,6 +647,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
return err;
}
device_init_wakeup(ckdev->dev, true);
return 0;
}
......
......@@ -22,7 +22,7 @@ MODULE_LICENSE("GPL");
/*
* ATI Remote Wonder II Channel Configuration
*
* The remote control can by assigned one of sixteen "channels" in order to facilitate
* The remote control can be assigned one of sixteen "channels" in order to facilitate
* the use of multiple remote controls within range of each other.
* A remote's "channel" may be altered by pressing and holding the "PC" button for
* approximately 3 seconds, after which the button will slowly flash the count of the
......
......@@ -133,6 +133,18 @@ config MOUSE_PS2_ELANTECH
If unsure, say N.
config MOUSE_PS2_ELANTECH_SMBUS
bool "Elantech PS/2 SMbus companion" if EXPERT
default y
depends on MOUSE_PS2 && MOUSE_PS2_ELANTECH
depends on I2C=y || I2C=MOUSE_PS2
select MOUSE_PS2_SMBUS
help
Say Y here if you have a Elantech touchpad connected to
to an SMBus, but enumerated through PS/2.
If unsure, say Y.
config MOUSE_PS2_SENTELIC
bool "Sentelic Finger Sensing Pad PS/2 protocol extension"
depends on MOUSE_PS2
......
......@@ -2049,14 +2049,11 @@ static int alps_hw_init_v1_v2(struct psmouse *psmouse)
return 0;
}
static int alps_hw_init_v6(struct psmouse *psmouse)
/* Must be in passthrough mode when calling this function */
static int alps_trackstick_enter_extended_mode_v3_v6(struct psmouse *psmouse)
{
unsigned char param[2] = {0xC8, 0x14};
/* Enter passthrough mode to let trackpoint enter 6byte raw mode */
if (alps_passthrough_mode_v2(psmouse, true))
return -1;
if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
......@@ -2064,9 +2061,25 @@ static int alps_hw_init_v6(struct psmouse *psmouse)
ps2_command(&psmouse->ps2dev, &param[1], PSMOUSE_CMD_SETRATE))
return -1;
return 0;
}
static int alps_hw_init_v6(struct psmouse *psmouse)
{
int ret;
/* Enter passthrough mode to let trackpoint enter 6byte raw mode */
if (alps_passthrough_mode_v2(psmouse, true))
return -1;
ret = alps_trackstick_enter_extended_mode_v3_v6(psmouse);
if (alps_passthrough_mode_v2(psmouse, false))
return -1;
if (ret)
return ret;
if (alps_absolute_mode_v6(psmouse)) {
psmouse_err(psmouse, "Failed to enable absolute mode\n");
return -1;
......@@ -2140,10 +2153,18 @@ static int alps_probe_trackstick_v3_v7(struct psmouse *psmouse, int reg_base)
static int alps_setup_trackstick_v3(struct psmouse *psmouse, int reg_base)
{
struct ps2dev *ps2dev = &psmouse->ps2dev;
int ret = 0;
int reg_val;
unsigned char param[4];
/*
* We need to configure trackstick to report data for touchpad in
* extended format. And also we need to tell touchpad to expect data
* from trackstick in extended format. Without this configuration
* trackstick packets sent from touchpad are in basic format which is
* different from what we expect.
*/
if (alps_passthrough_mode_v3(psmouse, reg_base, true))
return -EIO;
......@@ -2161,39 +2182,36 @@ static int alps_setup_trackstick_v3(struct psmouse *psmouse, int reg_base)
ret = -ENODEV;
} else {
psmouse_dbg(psmouse, "trackstick E7 report: %3ph\n", param);
/*
* Not sure what this does, but it is absolutely
* essential. Without it, the touchpad does not
* work at all and the trackstick just emits normal
* PS/2 packets.
*/
if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
alps_command_mode_send_nibble(psmouse, 0x9) ||
alps_command_mode_send_nibble(psmouse, 0x4)) {
psmouse_err(psmouse,
"Error sending magic E6 sequence\n");
if (alps_trackstick_enter_extended_mode_v3_v6(psmouse)) {
psmouse_err(psmouse, "Failed to enter into trackstick extended mode\n");
ret = -EIO;
goto error;
}
}
if (alps_passthrough_mode_v3(psmouse, reg_base, false))
return -EIO;
if (ret)
return ret;
if (alps_enter_command_mode(psmouse))
return -EIO;
reg_val = alps_command_mode_read_reg(psmouse, reg_base + 0x08);
if (reg_val == -1) {
ret = -EIO;
} else {
/*
* This ensures the trackstick packets are in the format
* supported by this driver. If bit 1 isn't set the packet
* format is different.
* Tell touchpad that trackstick is now in extended mode.
* If bit 1 isn't set the packet format is different.
*/
if (alps_enter_command_mode(psmouse) ||
alps_command_mode_write_reg(psmouse,
reg_base + 0x08, 0x82) ||
alps_exit_command_mode(psmouse))
reg_val |= BIT(1);
if (__alps_command_mode_write_reg(psmouse, reg_val))
ret = -EIO;
}
error:
if (alps_passthrough_mode_v3(psmouse, reg_base, false))
ret = -EIO;
if (alps_exit_command_mode(psmouse))
return -EIO;
return ret;
}
......
......@@ -36,6 +36,7 @@
#include <linux/jiffies.h>
#include <linux/completion.h>
#include <linux/of.h>
#include <linux/property.h>
#include <linux/regulator/consumer.h>
#include <asm/unaligned.h>
......@@ -51,6 +52,7 @@
#define ETP_MAX_FINGERS 5
#define ETP_FINGER_DATA_LEN 5
#define ETP_REPORT_ID 0x5D
#define ETP_TP_REPORT_ID 0x5E
#define ETP_REPORT_ID_OFFSET 2
#define ETP_TOUCH_INFO_OFFSET 3
#define ETP_FINGER_DATA_OFFSET 4
......@@ -61,6 +63,7 @@
struct elan_tp_data {
struct i2c_client *client;
struct input_dev *input;
struct input_dev *tp_input; /* trackpoint input node */
struct regulator *vcc;
const struct elan_transport_ops *ops;
......@@ -930,6 +933,33 @@ static void elan_report_absolute(struct elan_tp_data *data, u8 *packet)
input_sync(input);
}
static void elan_report_trackpoint(struct elan_tp_data *data, u8 *report)
{
struct input_dev *input = data->tp_input;
u8 *packet = &report[ETP_REPORT_ID_OFFSET + 1];
int x, y;
if (!data->tp_input) {
dev_warn_once(&data->client->dev,
"received a trackpoint report while no trackpoint device has been created. Please report upstream.\n");
return;
}
input_report_key(input, BTN_LEFT, packet[0] & 0x01);
input_report_key(input, BTN_RIGHT, packet[0] & 0x02);
input_report_key(input, BTN_MIDDLE, packet[0] & 0x04);
if ((packet[3] & 0x0F) == 0x06) {
x = packet[4] - (int)((packet[1] ^ 0x80) << 1);
y = (int)((packet[2] ^ 0x80) << 1) - packet[5];
input_report_rel(input, REL_X, x);
input_report_rel(input, REL_Y, y);
}
input_sync(input);
}
static irqreturn_t elan_isr(int irq, void *dev_id)
{
struct elan_tp_data *data = dev_id;
......@@ -951,11 +981,17 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
if (error)
goto out;
if (report[ETP_REPORT_ID_OFFSET] != ETP_REPORT_ID)
switch (report[ETP_REPORT_ID_OFFSET]) {
case ETP_REPORT_ID:
elan_report_absolute(data, report);
break;
case ETP_TP_REPORT_ID:
elan_report_trackpoint(data, report);
break;
default:
dev_err(dev, "invalid report id data (%x)\n",
report[ETP_REPORT_ID_OFFSET]);
else
elan_report_absolute(data, report);
}
out:
return IRQ_HANDLED;
......@@ -966,6 +1002,36 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
* Elan initialization functions
******************************************************************
*/
static int elan_setup_trackpoint_input_device(struct elan_tp_data *data)
{
struct device *dev = &data->client->dev;
struct input_dev *input;
input = devm_input_allocate_device(dev);
if (!input)
return -ENOMEM;
input->name = "Elan TrackPoint";
input->id.bustype = BUS_I2C;
input->id.vendor = ELAN_VENDOR_ID;
input->id.product = data->product_id;
input_set_drvdata(input, data);
input_set_capability(input, EV_REL, REL_X);
input_set_capability(input, EV_REL, REL_Y);
input_set_capability(input, EV_KEY, BTN_LEFT);
input_set_capability(input, EV_KEY, BTN_RIGHT);
input_set_capability(input, EV_KEY, BTN_MIDDLE);
__set_bit(INPUT_PROP_POINTER, input->propbit);
__set_bit(INPUT_PROP_POINTING_STICK, input->propbit);
data->tp_input = input;
return 0;
}
static int elan_setup_input_device(struct elan_tp_data *data)
{
struct device *dev = &data->client->dev;
......@@ -1140,6 +1206,12 @@ static int elan_probe(struct i2c_client *client,
if (error)
return error;
if (device_property_read_bool(&client->dev, "elan,trackpoint")) {
error = elan_setup_trackpoint_input_device(data);
if (error)
return error;
}
/*
* Platform code (ACPI, DTS) should normally set up interrupt
* for us, but in case it did not let's fall back to using falling
......@@ -1177,6 +1249,16 @@ static int elan_probe(struct i2c_client *client,
return error;
}
if (data->tp_input) {
error = input_register_device(data->tp_input);
if (error) {
dev_err(&client->dev,
"failed to register TrackPoint input device: %d\n",
error);
return error;
}
}
/*
* Systems using device tree should set up wakeup via DTS,
* the rest will configure device as wakeup source by default.
......@@ -1262,6 +1344,7 @@ static const struct acpi_device_id elan_acpi_id[] = {
{ "ELAN060B", 0 },
{ "ELAN060C", 0 },
{ "ELAN0611", 0 },
{ "ELAN0612", 0 },
{ "ELAN1000", 0 },
{ }
};
......
This diff is collapsed.
......@@ -106,6 +106,30 @@
*/
#define ETP_WEIGHT_VALUE 5
/*
* Bus information on 3rd byte of query ETP_RESOLUTION_QUERY(0x04)
*/
#define ETP_BUS_PS2_ONLY 0
#define ETP_BUS_SMB_ALERT_ONLY 1
#define ETP_BUS_SMB_HST_NTFY_ONLY 2
#define ETP_BUS_PS2_SMB_ALERT 3
#define ETP_BUS_PS2_SMB_HST_NTFY 4
/*
* New ICs are either using SMBus Host Notify or just plain PS2.
*
* ETP_FW_VERSION_QUERY is:
* Byte 1:
* - bit 0..3: IC BODY
* Byte 2:
* - bit 4: HiddenButton
* - bit 5: PS2_SMBUS_NOTIFY
* - bit 6: PS2CRCCheck
*/
#define ETP_NEW_IC_SMBUS_HOST_NOTIFY(fw_version) \
((((fw_version) & 0x0f2000) == 0x0f2000) && \
((fw_version) & 0x0000ff) > 0)
/*
* The base position for one finger, v4 hardware
*/
......@@ -114,6 +138,25 @@ struct finger_pos {
unsigned int y;
};
struct elantech_device_info {
unsigned char capabilities[3];
unsigned char samples[3];
unsigned char debug;
unsigned char hw_version;
unsigned int fw_version;
unsigned int x_res;
unsigned int y_res;
unsigned int bus;
bool paritycheck;
bool jumpy_cursor;
bool reports_pressure;
bool crc_enabled;
bool set_hw_resolution;
bool has_trackpoint;
int (*send_cmd)(struct psmouse *psmouse, unsigned char c,
unsigned char *param);
};
struct elantech_data {
struct input_dev *tp_dev; /* Relative device for trackpoint */
char tp_phys[32];
......@@ -127,27 +170,18 @@ struct elantech_data {
unsigned char reg_24;
unsigned char reg_25;
unsigned char reg_26;
unsigned char debug;
unsigned char capabilities[3];
unsigned char samples[3];
bool paritycheck;
bool jumpy_cursor;
bool reports_pressure;
bool crc_enabled;
bool set_hw_resolution;
unsigned char hw_version;
unsigned int fw_version;
unsigned int single_finger_reports;
unsigned int y_max;
unsigned int width;
struct finger_pos mt[ETP_MAX_FINGERS];
unsigned char parity[256];
int (*send_cmd)(struct psmouse *psmouse, unsigned char c, unsigned char *param);
struct elantech_device_info info;
void (*original_set_rate)(struct psmouse *psmouse, unsigned int rate);
};
#ifdef CONFIG_MOUSE_PS2_ELANTECH
int elantech_detect(struct psmouse *psmouse, bool set_properties);
int elantech_init_ps2(struct psmouse *psmouse);
int elantech_init(struct psmouse *psmouse);
#else
static inline int elantech_detect(struct psmouse *psmouse, bool set_properties)
......@@ -158,6 +192,19 @@ static inline int elantech_init(struct psmouse *psmouse)
{
return -ENOSYS;
}
static inline int elantech_init_ps2(struct psmouse *psmouse)
{
return -ENOSYS;
}
#endif /* CONFIG_MOUSE_PS2_ELANTECH */
#if defined(CONFIG_MOUSE_PS2_ELANTECH_SMBUS)
int elantech_init_smbus(struct psmouse *psmouse);
#else
static inline int elantech_init_smbus(struct psmouse *psmouse)
{
return -ENOSYS;
}
#endif /* CONFIG_MOUSE_PS2_ELANTECH_SMBUS */
#endif
......@@ -856,7 +856,17 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.name = "ETPS/2",
.alias = "elantech",
.detect = elantech_detect,
.init = elantech_init,
.init = elantech_init_ps2,
},
#endif
#ifdef CONFIG_MOUSE_PS2_ELANTECH_SMBUS
{
.type = PSMOUSE_ELANTECH_SMBUS,
.name = "ETSMBus",
.alias = "elantech-smbus",
.detect = elantech_detect,
.init = elantech_init_smbus,
.smbus_companion = true,
},
#endif
#ifdef CONFIG_MOUSE_PS2_SENTELIC
......@@ -1158,8 +1168,13 @@ static int psmouse_extensions(struct psmouse *psmouse,
/* Try Elantech touchpad */
if (max_proto > PSMOUSE_IMEX &&
psmouse_try_protocol(psmouse, PSMOUSE_ELANTECH,
&max_proto, set_properties, true)) {
&max_proto, set_properties, false)) {
if (!set_properties)
return PSMOUSE_ELANTECH;
ret = elantech_init(psmouse);
if (ret >= 0)
return ret;
}
if (max_proto > PSMOUSE_IMEX) {
......
......@@ -23,6 +23,7 @@ struct psmouse_smbus_dev {
struct i2c_client *client;
struct list_head node;
bool dead;
bool need_deactivate;
};
static LIST_HEAD(psmouse_smbus_list);
......@@ -118,6 +119,9 @@ static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse)
static int psmouse_smbus_reconnect(struct psmouse *psmouse)
{
struct psmouse_smbus_dev *smbdev = psmouse->private;
if (smbdev->need_deactivate)
psmouse_deactivate(psmouse);
return 0;
......@@ -225,6 +229,7 @@ void psmouse_smbus_cleanup(struct psmouse *psmouse)
int psmouse_smbus_init(struct psmouse *psmouse,
const struct i2c_board_info *board,
const void *pdata, size_t pdata_size,
bool need_deactivate,
bool leave_breadcrumbs)
{
struct psmouse_smbus_dev *smbdev;
......@@ -236,12 +241,19 @@ int psmouse_smbus_init(struct psmouse *psmouse,
smbdev->psmouse = psmouse;
smbdev->board = *board;
smbdev->need_deactivate = need_deactivate;
smbdev->board.platform_data = kmemdup(pdata, pdata_size, GFP_KERNEL);
if (pdata) {
smbdev->board.platform_data = kmemdup(pdata, pdata_size,
GFP_KERNEL);
if (!smbdev->board.platform_data) {
kfree(smbdev);
return -ENOMEM;
}
}
if (need_deactivate)
psmouse_deactivate(psmouse);
psmouse->private = smbdev;
psmouse->protocol_handler = psmouse_smbus_process_byte;
......@@ -250,8 +262,6 @@ int psmouse_smbus_init(struct psmouse *psmouse,
psmouse->disconnect = psmouse_smbus_disconnect;
psmouse->resync_time = 0;
psmouse_deactivate(psmouse);
mutex_lock(&psmouse_smbus_mutex);
list_add_tail(&smbdev->node, &psmouse_smbus_list);
mutex_unlock(&psmouse_smbus_mutex);
......
......@@ -68,6 +68,7 @@ enum psmouse_type {
PSMOUSE_VMMOUSE,
PSMOUSE_BYD,
PSMOUSE_SYNAPTICS_SMBUS,
PSMOUSE_ELANTECH_SMBUS,
PSMOUSE_AUTO /* This one should always be last */
};
......@@ -224,6 +225,7 @@ struct i2c_board_info;
int psmouse_smbus_init(struct psmouse *psmouse,
const struct i2c_board_info *board,
const void *pdata, size_t pdata_size,
bool need_deactivate,
bool leave_breadcrumbs);
void psmouse_smbus_cleanup(struct psmouse *psmouse);
......
......@@ -1754,7 +1754,7 @@ static int synaptics_create_intertouch(struct psmouse *psmouse,
};
return psmouse_smbus_init(psmouse, &intertouch_board,
&pdata, sizeof(pdata),
&pdata, sizeof(pdata), true,
leave_breadcrumbs);
}
......
......@@ -164,6 +164,17 @@ config TOUCHSCREEN_CHIPONE_ICN8318
To compile this driver as a module, choose M here: the
module will be called chipone_icn8318.
config TOUCHSCREEN_CHIPONE_ICN8505
tristate "chipone icn8505 touchscreen controller"
depends on I2C && ACPI
help
Say Y here if you have a ChipOne icn8505 based I2C touchscreen.
If unsure, say N.
To compile this driver as a module, choose M here: the
module will be called chipone_icn8505.
config TOUCHSCREEN_CY8CTMG110
tristate "cy8ctmg110 touchscreen"
depends on I2C
......
......@@ -19,6 +19,7 @@ obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8505) += chipone_icn8505.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C) += cyttsp_i2c.o cyttsp_i2c_common.o
......
......@@ -194,6 +194,8 @@ enum t100_type {
/* Delay times */
#define MXT_BACKUP_TIME 50 /* msec */
#define MXT_RESET_GPIO_TIME 20 /* msec */
#define MXT_RESET_INVALID_CHG 100 /* msec */
#define MXT_RESET_TIME 200 /* msec */
#define MXT_RESET_TIMEOUT 3000 /* msec */
#define MXT_CRC_TIMEOUT 1000 /* msec */
......@@ -1208,7 +1210,7 @@ static int mxt_soft_reset(struct mxt_data *data)
return ret;
/* Ignore CHG line for 100ms after reset */
msleep(100);
msleep(MXT_RESET_INVALID_CHG);
mxt_acquire_irq(data);
......@@ -2999,142 +3001,6 @@ static int mxt_parse_device_properties(struct mxt_data *data)
return 0;
}
#ifdef CONFIG_ACPI
struct mxt_acpi_platform_data {
const char *hid;
const struct property_entry *props;
};
static unsigned int samus_touchpad_buttons[] = {
KEY_RESERVED,
KEY_RESERVED,
KEY_RESERVED,
BTN_LEFT
};
static const struct property_entry samus_touchpad_props[] = {
PROPERTY_ENTRY_U32_ARRAY("linux,gpio-keymap", samus_touchpad_buttons),
{ }
};
static struct mxt_acpi_platform_data samus_platform_data[] = {
{
/* Touchpad */
.hid = "ATML0000",
.props = samus_touchpad_props,
},
{
/* Touchscreen */
.hid = "ATML0001",
},
{ }
};
static unsigned int chromebook_tp_buttons[] = {
KEY_RESERVED,
KEY_RESERVED,
KEY_RESERVED,
KEY_RESERVED,
KEY_RESERVED,
BTN_LEFT
};
static const struct property_entry chromebook_tp_props[] = {
PROPERTY_ENTRY_U32_ARRAY("linux,gpio-keymap", chromebook_tp_buttons),
{ }
};
static struct mxt_acpi_platform_data chromebook_platform_data[] = {
{
/* Touchpad */
.hid = "ATML0000",
.props = chromebook_tp_props,
},
{
/* Touchscreen */
.hid = "ATML0001",
},
{ }
};
static const struct dmi_system_id mxt_dmi_table[] = {
{
/* 2015 Google Pixel */
.ident = "Chromebook Pixel 2",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
DMI_MATCH(DMI_PRODUCT_NAME, "Samus"),
},
.driver_data = samus_platform_data,
},
{
/* Samsung Chromebook Pro */
.ident = "Samsung Chromebook Pro",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Google"),
DMI_MATCH(DMI_PRODUCT_NAME, "Caroline"),
},
.driver_data = samus_platform_data,
},
{
/* Other Google Chromebooks */
.ident = "Chromebook",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
},
.driver_data = chromebook_platform_data,
},
{ }
};
static int mxt_prepare_acpi_properties(struct i2c_client *client)
{
struct acpi_device *adev;
const struct dmi_system_id *system_id;
const struct mxt_acpi_platform_data *acpi_pdata;
adev = ACPI_COMPANION(&client->dev);
if (!adev)
return -ENOENT;
system_id = dmi_first_match(mxt_dmi_table);
if (!system_id)
return -ENOENT;
acpi_pdata = system_id->driver_data;
if (!acpi_pdata)
return -ENOENT;
while (acpi_pdata->hid) {
if (!strcmp(acpi_device_hid(adev), acpi_pdata->hid)) {
/*
* Remove previously installed properties if we
* are probing this device not for the very first
* time.
*/
device_remove_properties(&client->dev);
/*
* Now install the platform-specific properties
* that are missing from ACPI.
*/
device_add_properties(&client->dev, acpi_pdata->props);
break;
}
acpi_pdata++;
}
return 0;
}
#else
static int mxt_prepare_acpi_properties(struct i2c_client *client)
{
return -ENOENT;
}
#endif
static const struct dmi_system_id chromebook_T9_suspend_dmi[] = {
{
.matches = {
......@@ -3155,6 +3021,18 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
struct mxt_data *data;
int error;
/*
* Ignore devices that do not have device properties attached to
* them, as we need help determining whether we are dealing with
* touch screen or touchpad.
*
* So far on x86 the only users of Atmel touch controllers are
* Chromebooks, and chromeos_laptop driver will ensure that
* necessary properties are provided (if firmware does not do that).
*/
if (!device_property_present(&client->dev, "compatible"))
return -ENXIO;
/*
* Ignore ACPI devices representing bootloader mode.
*
......@@ -3186,10 +3064,6 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ?
MXT_SUSPEND_T9_CTRL : MXT_SUSPEND_DEEP_SLEEP;
error = mxt_prepare_acpi_properties(client);
if (error && error != -ENOENT)
return error;
error = mxt_parse_device_properties(data);
if (error)
return error;
......@@ -3210,20 +3084,14 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
return error;
}
disable_irq(client->irq);
if (data->reset_gpio) {
data->in_bootloader = true;
msleep(MXT_RESET_TIME);
reinit_completion(&data->bl_completion);
msleep(MXT_RESET_GPIO_TIME);
gpiod_set_value(data->reset_gpio, 1);
error = mxt_wait_for_completion(data, &data->bl_completion,
MXT_RESET_TIMEOUT);
if (error)
return error;
data->in_bootloader = false;
msleep(MXT_RESET_INVALID_CHG);
}
disable_irq(client->irq);
error = mxt_initialize(data);
if (error)
return error;
......
This diff is collapsed.
......@@ -933,6 +933,7 @@ MODULE_DEVICE_TABLE(i2c, goodix_ts_id);
#ifdef CONFIG_ACPI
static const struct acpi_device_id goodix_acpi_match[] = {
{ "GDIX1001", 0 },
{ "GDIX1002", 0 },
{ }
};
MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
......
......@@ -17,7 +17,7 @@
* found in Gateway AOL Connected Touchpad computers.
*
* Documentation for ICS MK712 can be found at:
* http://www.idt.com/products/getDoc.cfm?docID=18713923
* https://www.idt.com/general-parts/mk712-touch-screen-controller
*/
/*
......
......@@ -34,6 +34,8 @@
#define SEQ_SETTLE 275
#define MAX_12BIT ((1 << 12) - 1)
#define TSC_IRQENB_MASK (IRQENB_FIFO0THRES | IRQENB_EOS | IRQENB_HW_PEN)
static const int config_pins[] = {
STEPCONFIG_XPP,
STEPCONFIG_XNN,
......@@ -274,6 +276,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
if (status & IRQENB_HW_PEN) {
ts_dev->pen_down = true;
irqclr |= IRQENB_HW_PEN;
pm_stay_awake(ts_dev->mfd_tscadc->dev);
}
if (status & IRQENB_PENUP) {
......@@ -283,6 +286,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
input_report_key(input_dev, BTN_TOUCH, 0);
input_report_abs(input_dev, ABS_PRESSURE, 0);
input_sync(input_dev);
pm_relax(ts_dev->mfd_tscadc->dev);
} else {
ts_dev->pen_down = true;
}
......@@ -432,6 +436,7 @@ static int titsc_probe(struct platform_device *pdev)
goto err_free_mem;
}
titsc_writel(ts_dev, REG_IRQSTATUS, TSC_IRQENB_MASK);
titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
err = titsc_config_wires(ts_dev);
......@@ -495,6 +500,7 @@ static int __maybe_unused titsc_suspend(struct device *dev)
tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
if (device_may_wakeup(tscadc_dev->dev)) {
titsc_writel(ts_dev, REG_IRQSTATUS, TSC_IRQENB_MASK);
idle = titsc_readl(ts_dev, REG_IRQENABLE);
titsc_writel(ts_dev, REG_IRQENABLE,
(idle | IRQENB_HW_PEN));
......@@ -513,6 +519,7 @@ static int __maybe_unused titsc_resume(struct device *dev)
titsc_writel(ts_dev, REG_IRQWAKEUP,
0x00);
titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
pm_relax(ts_dev->mfd_tscadc->dev);
}
titsc_step_config(ts_dev);
titsc_writel(ts_dev, REG_FIFO0THR,
......
......@@ -440,6 +440,8 @@ static int panjit_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
#define MTOUCHUSB_RESET 7
#define MTOUCHUSB_REQ_CTRLLR_ID 10
#define MTOUCHUSB_REQ_CTRLLR_ID_LEN 16
static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
{
if (hwcalib_xy) {
......@@ -454,11 +456,93 @@ static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
return 1;
}
struct mtouch_priv {
u8 fw_rev_major;
u8 fw_rev_minor;
};
static ssize_t mtouch_firmware_rev_show(struct device *dev,
struct device_attribute *attr, char *output)
{
struct usb_interface *intf = to_usb_interface(dev);
struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
struct mtouch_priv *priv = usbtouch->priv;
return scnprintf(output, PAGE_SIZE, "%1x.%1x\n",
priv->fw_rev_major, priv->fw_rev_minor);
}
static DEVICE_ATTR(firmware_rev, 0444, mtouch_firmware_rev_show, NULL);
static struct attribute *mtouch_attrs[] = {
&dev_attr_firmware_rev.attr,
NULL
};
static const struct attribute_group mtouch_attr_group = {
.attrs = mtouch_attrs,
};
static int mtouch_get_fw_revision(struct usbtouch_usb *usbtouch)
{
struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
struct mtouch_priv *priv = usbtouch->priv;
u8 *buf;
int ret;
buf = kzalloc(MTOUCHUSB_REQ_CTRLLR_ID_LEN, GFP_NOIO);
if (!buf)
return -ENOMEM;
ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
MTOUCHUSB_REQ_CTRLLR_ID,
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
0, 0, buf, MTOUCHUSB_REQ_CTRLLR_ID_LEN,
USB_CTRL_SET_TIMEOUT);
if (ret != MTOUCHUSB_REQ_CTRLLR_ID_LEN) {
dev_warn(&usbtouch->interface->dev,
"Failed to read FW rev: %d\n", ret);
ret = ret < 0 ? ret : -EIO;
goto free;
}
priv->fw_rev_major = buf[3];
priv->fw_rev_minor = buf[4];
ret = 0;
free:
kfree(buf);
return ret;
}
static int mtouch_alloc(struct usbtouch_usb *usbtouch)
{
int ret;
usbtouch->priv = kmalloc(sizeof(struct mtouch_priv), GFP_KERNEL);
if (!usbtouch->priv)
return -ENOMEM;
ret = sysfs_create_group(&usbtouch->interface->dev.kobj,
&mtouch_attr_group);
if (ret) {
kfree(usbtouch->priv);
usbtouch->priv = NULL;
return ret;
}
return 0;
}
static int mtouch_init(struct usbtouch_usb *usbtouch)
{
int ret, i;
struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
ret = mtouch_get_fw_revision(usbtouch);
if (ret)
return ret;
ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
MTOUCHUSB_RESET,
USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
......@@ -492,6 +576,14 @@ static int mtouch_init(struct usbtouch_usb *usbtouch)
return 0;
}
static void mtouch_exit(struct usbtouch_usb *usbtouch)
{
struct mtouch_priv *priv = usbtouch->priv;
sysfs_remove_group(&usbtouch->interface->dev.kobj, &mtouch_attr_group);
kfree(priv);
}
#endif
......@@ -1119,7 +1211,9 @@ static struct usbtouch_device_info usbtouch_dev_info[] = {
.max_yc = 0x4000,
.rept_size = 11,
.read_data = mtouch_read_data,
.alloc = mtouch_alloc,
.init = mtouch_init,
.exit = mtouch_exit,
},
#endif
......
......@@ -229,7 +229,7 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
}
EXPORT_SYMBOL(cros_ec_suspend);
static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
static void cros_ec_report_events_during_suspend(struct cros_ec_device *ec_dev)
{
while (cros_ec_get_next_event(ec_dev, NULL) > 0)
blocking_notifier_call_chain(&ec_dev->event_notifier,
......@@ -253,21 +253,16 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
ret);
/*
* In some cases, we need to distinguish between events that occur
* during suspend if the EC is not a wake source. For example,
* keypresses during suspend should be discarded if it does not wake
* the system.
*
* If the EC is not a wake source, drain the event queue and mark them
* as "queued during suspend".
*/
if (ec_dev->wake_enabled) {
disable_irq_wake(ec_dev->irq);
ec_dev->wake_enabled = 0;
} else {
cros_ec_drain_events(ec_dev);
}
/*
* Let the mfd devices know about events that occur during
* suspend. This way the clients know what to do with them.
*/
cros_ec_report_events_during_suspend(ec_dev);
return 0;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment