* [PATCH v4 07/12] HID: input: enable Totem on the Dell Canvas 27
From: Benjamin Tissoires @ 2018-07-13 14:13 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180713141354.7286-1-benjamin.tissoires@redhat.com>
The Dell Canvas 27 has a tool that can be put on the surface and acts
as a dial. The firmware processes the detection of the tool and forward
regular HID reports with X, Y, Azimuth, rotation, width/height.
The firmware also exports Contact ID, Countact Count which may hint that
several totems can be used at the same time (the FW only supports one).
We can tell that MT_TOOL_DIAL will be reported by setting the min/max
of ABS_MT_TOOL_TYPE to MT_TOOL_DIAL.
This tool is aimed at being used by the system and not the applications,
so the user space processing should not go through the regular touch
inputs.
We set INPUT_PROP_DIRECT which applies ID_INPUT_TOUCHSCREEN to this new
type of devices, but we will counter this for the time being with the
special udev hwdb entry mentioned above.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1511846
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v4
no changes in v3
changes in v2:
- manually declare ABS_MT_TOOL_TYPE and set its min/max to MT_TOOL_DIAL
to explain which tool it will set
---
drivers/hid/hid-input.c | 3 +++
drivers/hid/hid-multitouch.c | 48 +++++++++++++++++++++++++++++++-------------
include/linux/hid.h | 6 ++++++
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index ab93dd5927c3..4e94ea3e280a 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1550,6 +1550,9 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
case HID_GD_WIRELESS_RADIO_CTLS:
suffix = "Wireless Radio Control";
break;
+ case HID_GD_SYSTEM_MULTIAXIS:
+ suffix = "System Multi Axis";
+ break;
default:
break;
}
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 346e9caef6f3..a4a274ebfbef 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -665,7 +665,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
/*
* Model touchscreens providing buttons as touchpads.
*/
- if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
+ if (field->application == HID_DG_TOUCHSCREEN &&
+ (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
app->mt_flags |= INPUT_MT_POINTER;
td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
}
@@ -692,6 +693,19 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
set_abs(hi->input, code, field, cls->sn_move);
+ /*
+ * A system multi-axis that exports X and Y has a high
+ * chance of being used directly on a surface
+ */
+ if (field->application == HID_GD_SYSTEM_MULTIAXIS) {
+ __set_bit(INPUT_PROP_DIRECT,
+ hi->input->propbit);
+ input_set_abs_params(hi->input,
+ ABS_MT_TOOL_TYPE,
+ MT_TOOL_DIAL,
+ MT_TOOL_DIAL, 0, 0);
+ }
+
return 1;
case HID_GD_Y:
if (prev_usage && (prev_usage->hid == usage->hid)) {
@@ -725,7 +739,9 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
MT_STORE_FIELD(confidence_state);
return 1;
case HID_DG_TIPSWITCH:
- input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
+ if (field->application != HID_GD_SYSTEM_MULTIAXIS)
+ input_set_capability(hi->input,
+ EV_KEY, BTN_TOUCH);
MT_STORE_FIELD(tip_state);
return 1;
case HID_DG_CONTACTID:
@@ -802,6 +818,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
field->application == HID_DG_TOUCHPAD &&
(usage->hid & HID_USAGE) > 1)
code--;
+
+ if (field->application == HID_GD_SYSTEM_MULTIAXIS)
+ code = BTN_0 + ((usage->hid - 1) & HID_USAGE);
+
hid_map_usage(hi, usage, bit, max, EV_KEY, code);
input_set_capability(hi->input, EV_KEY, code);
return 1;
@@ -899,6 +919,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
bool inrange_state = false;
int active;
int slotnum;
+ int tool = MT_TOOL_FINGER;
if (!slot)
return -EINVAL;
@@ -939,8 +960,11 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
active = (*slot->tip_state || inrange_state) && confidence_state;
+ if (app->application == HID_GD_SYSTEM_MULTIAXIS)
+ tool = MT_TOOL_DIAL;
+
input_mt_slot(input, slotnum);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
+ input_mt_report_slot_state(input, tool, active);
if (active) {
/* this finger is in proximity of the sensor */
int wide = (*slot->w > *slot->h);
@@ -1203,6 +1227,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
field->application != HID_GD_SYSTEM_CONTROL &&
field->application != HID_CP_CONSUMER_CONTROL &&
field->application != HID_GD_WIRELESS_RADIO_CTLS &&
+ field->application != HID_GD_SYSTEM_MULTIAXIS &&
!(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS &&
application->quirks & MT_QUIRK_ASUS_CUSTOM_UP))
return -1;
@@ -1230,9 +1255,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
return 1;
}
- if (rdata->is_mt_collection &&
- (field->application == HID_DG_TOUCHSCREEN ||
- field->application == HID_DG_TOUCHPAD))
+ if (rdata->is_mt_collection)
return mt_touch_input_mapping(hdev, hi, field, usage, bit, max,
application);
@@ -1244,15 +1267,11 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
unsigned long **bit, int *max)
{
- /*
- * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
- * for the stylus.
- */
- if (field->physical == HID_DG_STYLUS)
- return 0;
+ struct mt_device *td = hid_get_drvdata(hdev);
+ struct mt_report_data *rdata;
- if (field->application == HID_DG_TOUCHSCREEN ||
- field->application == HID_DG_TOUCHPAD) {
+ rdata = mt_find_report_data(td, field->report);
+ if (rdata && rdata->is_mt_collection) {
/* We own these mappings, tell hid-input to ignore them */
return -1;
}
@@ -1460,6 +1479,7 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
case HID_GD_SYSTEM_CONTROL:
case HID_CP_CONSUMER_CONTROL:
case HID_GD_WIRELESS_RADIO_CTLS:
+ case HID_GD_SYSTEM_MULTIAXIS:
/* already handled by hid core */
break;
case HID_DG_TOUCHSCREEN:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 773bcb1d4044..2e4498d52a2f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -190,6 +190,12 @@ struct hid_item {
* http://www.usb.org/developers/hidpage/HUTRR40RadioHIDUsagesFinal.pdf
*/
#define HID_GD_WIRELESS_RADIO_CTLS 0x0001000c
+/*
+ * System Multi-Axis, see:
+ * http://www.usb.org/developers/hidpage/HUTRR62_-_Generic_Desktop_CA_for_System_Multi-Axis_Controllers.txt
+ */
+#define HID_GD_SYSTEM_MULTIAXIS 0x0001000e
+
#define HID_GD_X 0x00010030
#define HID_GD_Y 0x00010031
#define HID_GD_Z 0x00010032
--
2.14.3
^ permalink raw reply related
* [PATCH v4 08/12] HID: core: do not upper bound the collection stack
From: Benjamin Tissoires @ 2018-07-13 14:13 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180713141354.7286-1-benjamin.tissoires@redhat.com>
Looks like 4 was sufficient until now. However, the Surface Dial needs
a stack of 5 and simply fails at probing.
Dynamically add HID_COLLECTION_STACK_SIZE to the size of the stack if
we hit the upper bound.
Checkpatch complains about bare unsigned, so converting those to
'unsigned int' in struct hid_parser
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v4
no changes in v3
no changes in v2
---
drivers/hid/hid-core.c | 17 ++++++++++++++---
include/linux/hid.h | 9 +++++----
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3942ee61bd1c..5de6f18c9bf7 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -128,9 +128,19 @@ static int open_collection(struct hid_parser *parser, unsigned type)
usage = parser->local.usage[0];
- if (parser->collection_stack_ptr == HID_COLLECTION_STACK_SIZE) {
- hid_err(parser->device, "collection stack overflow\n");
- return -EINVAL;
+ if (parser->collection_stack_ptr == parser->collection_stack_size) {
+ unsigned int *collection_stack;
+ unsigned int new_size = parser->collection_stack_size +
+ HID_COLLECTION_STACK_SIZE;
+
+ collection_stack = krealloc(parser->collection_stack,
+ new_size * sizeof(unsigned int),
+ GFP_KERNEL);
+ if (!collection_stack)
+ return -ENOMEM;
+
+ parser->collection_stack = collection_stack;
+ parser->collection_stack_size = new_size;
}
if (parser->device->maxcollection == parser->device->collection_size) {
@@ -840,6 +850,7 @@ static int hid_scan_report(struct hid_device *hid)
break;
}
+ kfree(parser->collection_stack);
vfree(parser);
return 0;
}
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 2e4498d52a2f..aee281522c6d 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -644,12 +644,13 @@ static inline void hid_set_drvdata(struct hid_device *hdev, void *data)
struct hid_parser {
struct hid_global global;
struct hid_global global_stack[HID_GLOBAL_STACK_SIZE];
- unsigned global_stack_ptr;
+ unsigned int global_stack_ptr;
struct hid_local local;
- unsigned collection_stack[HID_COLLECTION_STACK_SIZE];
- unsigned collection_stack_ptr;
+ unsigned int *collection_stack;
+ unsigned int collection_stack_ptr;
+ unsigned int collection_stack_size;
struct hid_device *device;
- unsigned scan_flags;
+ unsigned int scan_flags;
};
struct hid_class_descriptor {
--
2.14.3
^ permalink raw reply related
* [PATCH v4 09/12] HID: microsoft: support the Surface Dial
From: Benjamin Tissoires @ 2018-07-13 14:13 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180713141354.7286-1-benjamin.tissoires@redhat.com>
The tool works nicely with hid-generic, but it ends up creating 9
different input nodes with most of them only having ABS_MISC set.
Filter the axis out, which reduces the amount of devices to 2. One is
the proper System Multi-axis collection, the other exported device
seems to provide SLEEP and POWER Key, not sure how one can trigger
those events though.
Filtering the ABS_X and ABS_Y axes also prevents udev to detect this as
a touchscreen.
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v4
no changes in v3
no changes in v2
---
drivers/hid/hid-microsoft.c | 49 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 96e7d3231d2f..72d983626afd 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -22,12 +22,13 @@
#include "hid-ids.h"
-#define MS_HIDINPUT 0x01
-#define MS_ERGONOMY 0x02
-#define MS_PRESENTER 0x04
-#define MS_RDESC 0x08
-#define MS_NOGET 0x10
-#define MS_DUPLICATE_USAGES 0x20
+#define MS_HIDINPUT BIT(0)
+#define MS_ERGONOMY BIT(1)
+#define MS_PRESENTER BIT(2)
+#define MS_RDESC BIT(3)
+#define MS_NOGET BIT(4)
+#define MS_DUPLICATE_USAGES BIT(5)
+#define MS_SURFACE_DIAL BIT(6)
static __u8 *ms_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
@@ -130,6 +131,30 @@ static int ms_presenter_8k_quirk(struct hid_input *hi, struct hid_usage *usage,
return 1;
}
+static int ms_surface_dial_quirk(struct hid_input *hi, struct hid_field *field,
+ struct hid_usage *usage, unsigned long **bit, int *max)
+{
+ switch (usage->hid & HID_USAGE_PAGE) {
+ case 0xff070000:
+ /* fall-through */
+ case HID_UP_DIGITIZER:
+ /* ignore those axis */
+ return -1;
+ case HID_UP_GENDESK:
+ switch (usage->hid) {
+ case HID_GD_X:
+ /* fall-through */
+ case HID_GD_Y:
+ /* fall-through */
+ case HID_GD_RFKILL_BTN:
+ /* ignore those axis */
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
unsigned long **bit, int *max)
@@ -146,6 +171,13 @@ static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi,
ms_presenter_8k_quirk(hi, usage, bit, max))
return 1;
+ if (quirks & MS_SURFACE_DIAL) {
+ int ret = ms_surface_dial_quirk(hi, field, usage, bit, max);
+
+ if (ret)
+ return ret;
+ }
+
return 0;
}
@@ -229,6 +261,9 @@ static int ms_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (quirks & MS_NOGET)
hdev->quirks |= HID_QUIRK_NOGET;
+ if (quirks & MS_SURFACE_DIAL)
+ hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
+
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
@@ -281,6 +316,8 @@ static const struct hid_device_id ms_devices[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_BT),
.driver_data = MS_PRESENTER },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B),
+ .driver_data = MS_SURFACE_DIAL },
{ }
};
MODULE_DEVICE_TABLE(hid, ms_devices);
--
2.14.3
^ permalink raw reply related
* [PATCH v4 10/12] HID: multitouch: report MT_TOOL_PALM for non-confident touches
From: Benjamin Tissoires @ 2018-07-13 14:13 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180713141354.7286-1-benjamin.tissoires@redhat.com>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
According to Microsoft specification [1] for Precision Touchpads (and
Touchscreens) the devices use "confidence" reports to signal accidental
touches, or contacts that are "too large to be a finger". Instead of
simply marking contact inactive in this case (which causes issues if
contact was originally proper and we lost confidence in it later, as
this results in accidental clicks, drags, etc), let's report such
contacts as MT_TOOL_PALM and let userspace decide what to do.
[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[splitted and rebased]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
changes in v4:
- dropped the touch major/minor modifications for palm events
no changes in v3
changes in v2:
- dropped the delayed release in case of palm detection. This is
a FW bug for PTP devices, and should only happen with touchscreens
- move input_set_abs_params(hi->input, ABS_MT_TOOL_TYPE,...) into
its own test so other devices can add MT_QUIRK_CONFIDENCE to their
quirks.
---
drivers/hid/hid-multitouch.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a4a274ebfbef..587369443f50 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -736,6 +736,13 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
cls->name == MT_CLS_WIN_8_DUAL) &&
field->application == HID_DG_TOUCHPAD)
app->quirks |= MT_QUIRK_CONFIDENCE;
+
+ if (app->quirks & MT_QUIRK_CONFIDENCE)
+ input_set_abs_params(hi->input,
+ ABS_MT_TOOL_TYPE,
+ MT_TOOL_FINGER,
+ MT_TOOL_PALM, 0, 0);
+
MT_STORE_FIELD(confidence_state);
return 1;
case HID_DG_TIPSWITCH:
@@ -958,10 +965,12 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
if (quirks & MT_QUIRK_HOVERING)
inrange_state = *slot->inrange_state;
- active = (*slot->tip_state || inrange_state) && confidence_state;
+ active = *slot->tip_state || inrange_state;
if (app->application == HID_GD_SYSTEM_MULTIAXIS)
tool = MT_TOOL_DIAL;
+ else if (unlikely(!confidence_state))
+ tool = MT_TOOL_PALM;
input_mt_slot(input, slotnum);
input_mt_report_slot_state(input, tool, active);
@@ -993,11 +1002,11 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
orientation = -azimuth;
}
- /*
- * divided by two to match visual scale of touch
- * for devices with this quirk
- */
if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+ /*
+ * divided by two to match visual scale of touch
+ * for devices with this quirk
+ */
major = major >> 1;
minor = minor >> 1;
}
--
2.14.3
^ permalink raw reply related
* [PATCH v4 11/12] HID: multitouch: touchscreens also use confidence reports
From: Benjamin Tissoires @ 2018-07-13 14:13 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180713141354.7286-1-benjamin.tissoires@redhat.com>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
According to [1] the confidence is used not only by touchpad devices,
but also by touchscreens.
[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchscreen-required-hid-top-level-collections
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[rebased]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v4
no changes in v3
no changes in v2 (compared to Dmitry's initial submission)
---
drivers/hid/hid-multitouch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 587369443f50..3464adab0085 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -734,7 +734,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_DG_CONFIDENCE:
if ((cls->name == MT_CLS_WIN_8 ||
cls->name == MT_CLS_WIN_8_DUAL) &&
- field->application == HID_DG_TOUCHPAD)
+ (field->application == HID_DG_TOUCHPAD ||
+ field->application == HID_DG_TOUCHSCREEN))
app->quirks |= MT_QUIRK_CONFIDENCE;
if (app->quirks & MT_QUIRK_CONFIDENCE)
--
2.14.3
^ permalink raw reply related
* [PATCH v4 12/12] HID: multitouch: handle palm for touchscreens
From: Benjamin Tissoires @ 2018-07-13 14:13 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180713141354.7286-1-benjamin.tissoires@redhat.com>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Usually, there is no palm rejection for touchscreens. You don't rest
your palm on the touchscreen while interacting with it.
However, some wacom devices do so because you can rest your palm while
interacting with the stylus.
Unfortunately, the spec for touchscreens[1] is less precise than the one
for touchpads[2]. This leads to a situation where it's 'legitimate'
for a touchscreen to provide both tipswitch off and confidence off in the
same report.
Work around that by keeping the slot active for one frame where we report
MT_TOOL_PALM, and then synthesizing the release event in a separate frame.
frame
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[rebased and new commit message]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v4
no changes in v3
changes in v2 (compared to Dmitry's initial submission):
- extracted from the initial submission in a separate patch
- rebased on top of my current series
- add an extra input_mt_sync_frame(input); to release the single touch
emulation
---
drivers/hid/hid-multitouch.c | 52 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3464adab0085..40fbb7c52723 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -118,6 +118,9 @@ struct mt_application {
int left_button_state; /* left button state */
unsigned int mt_flags; /* flags to pass to input-mt */
+ unsigned long *pending_palm_slots; /* slots where we reported palm
+ * and need to release */
+
__u8 num_received; /* how many contacts we received */
__u8 num_expected; /* expected last contact index */
__u8 buttons_count; /* number of physical buttons per touchpad */
@@ -863,6 +866,28 @@ static int mt_compute_slot(struct mt_device *td, struct mt_application *app,
return input_mt_get_slot_by_key(input, *slot->contactid);
}
+static void mt_release_pending_palms(struct mt_device *td,
+ struct mt_application *app,
+ struct input_dev *input)
+{
+ int slotnum;
+ bool need_sync = false;
+
+ for_each_set_bit(slotnum, app->pending_palm_slots, td->maxcontacts) {
+ clear_bit(slotnum, app->pending_palm_slots);
+
+ input_mt_slot(input, slotnum);
+ input_mt_report_slot_state(input, MT_TOOL_PALM, false);
+
+ need_sync = true;
+ }
+
+ if (need_sync) {
+ input_mt_sync_frame(input);
+ input_sync(input);
+ }
+}
+
/*
* this function is called when a whole packet has been received and processed,
* so that it can decide what to send to the input layer.
@@ -876,6 +901,9 @@ static void mt_sync_frame(struct mt_device *td, struct mt_application *app,
input_mt_sync_frame(input);
input_event(input, EV_MSC, MSC_TIMESTAMP, app->timestamp);
input_sync(input);
+
+ mt_release_pending_palms(td, app, input);
+
app->num_received = 0;
app->left_button_state = 0;
@@ -970,8 +998,23 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
if (app->application == HID_GD_SYSTEM_MULTIAXIS)
tool = MT_TOOL_DIAL;
- else if (unlikely(!confidence_state))
+ else if (unlikely(!confidence_state)) {
tool = MT_TOOL_PALM;
+ if (!active &&
+ input_mt_is_active(&mt->slots[slotnum])) {
+ /*
+ * The non-confidence was reported for
+ * previously valid contact that is also no
+ * longer valid. We can't simply report
+ * lift-off as userspace will not be aware
+ * of non-confidence, so we need to split
+ * it into 2 events: active MT_TOOL_PALM
+ * and a separate liftoff.
+ */
+ active = true;
+ set_bit(slotnum, app->pending_palm_slots);
+ }
+ }
input_mt_slot(input, slotnum);
input_mt_report_slot_state(input, tool, active);
@@ -1197,6 +1240,13 @@ static int mt_touch_input_configured(struct hid_device *hdev,
if (td->is_buttonpad)
__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+ app->pending_palm_slots = devm_kcalloc(&hi->input->dev,
+ BITS_TO_LONGS(td->maxcontacts),
+ sizeof(long),
+ GFP_KERNEL);
+ if (!app->pending_palm_slots)
+ return -ENOMEM;
+
ret = input_mt_init_slots(input, td->maxcontacts, app->mt_flags);
if (ret)
return ret;
--
2.14.3
^ permalink raw reply related
* [PATCH 0/2] Input: atmel_mxt_ts: Add support for optional regulators
From: Paweł Chmiel @ 2018-07-13 18:30 UTC (permalink / raw)
To: nick
Cc: dmitry.torokhov, robh+dt, mark.rutland, nicolas.ferre,
alexandre.belloni, linux-input, devicetree, linux-arm-kernel,
linux-kernel, Paweł Chmiel
This two patches add optional regulator support to atmel_mxt_ts.
First patch adds regulators to driver.
Second patch updates documentation.
Paweł Chmiel (2):
Input: atmel_mxt_ts: Add support for optional regulators.
Input: atmel_mxt_ts: Document optional voltage regulators
.../devicetree/bindings/input/atmel,maxtouch.txt | 8 ++++
drivers/input/touchscreen/atmel_mxt_ts.c | 45 ++++++++++++++++++++++
2 files changed, 53 insertions(+)
--
2.7.4
^ permalink raw reply
* [PATCH 1/2] Input: atmel_mxt_ts: Add support for optional regulators.
From: Paweł Chmiel @ 2018-07-13 18:30 UTC (permalink / raw)
To: nick
Cc: dmitry.torokhov, robh+dt, mark.rutland, nicolas.ferre,
alexandre.belloni, linux-input, devicetree, linux-arm-kernel,
linux-kernel, Paweł Chmiel
In-Reply-To: <1531506608-24479-1-git-send-email-pawel.mikolaj.chmiel@gmail.com>
This patch adds optional regulators, which can be used to power
up touchscreen. After enabling regulators, we need to wait 150msec.
This value is taken from official driver.
It was tested on Samsung Galaxy i9000 (based on Samsung S5PV210 SOC).
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 45 ++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 54fe190fd4bc..a7625ec8fb9f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/of.h>
#include <linux/property.h>
+#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/gpio/consumer.h>
#include <linux/property.h>
@@ -198,6 +199,7 @@ enum t100_type {
#define MXT_RESET_INVALID_CHG 100 /* msec */
#define MXT_RESET_TIME 200 /* msec */
#define MXT_RESET_TIMEOUT 3000 /* msec */
+#define MXT_REGULATOR_DELAY 150 /* msec */
#define MXT_CRC_TIMEOUT 1000 /* msec */
#define MXT_FW_RESET_TIME 3000 /* msec */
#define MXT_FW_CHG_TIMEOUT 300 /* msec */
@@ -310,6 +312,8 @@ struct mxt_data {
struct t7_config t7_cfg;
struct mxt_dbg dbg;
struct gpio_desc *reset_gpio;
+ struct regulator *vdd_reg;
+ struct regulator *avdd_reg;
/* Cached parameters from object table */
u16 T5_address;
@@ -3076,6 +3080,40 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
return error;
}
+ data->vdd_reg = devm_regulator_get_optional(&client->dev, "vdd");
+ if (IS_ERR(data->vdd_reg)) {
+ error = PTR_ERR(data->vdd_reg);
+ dev_err(&client->dev, "Failed to get vdd regulator: %d\n",
+ error);
+ return error;
+ }
+
+ if (data->vdd_reg) {
+ error = regulator_enable(data->vdd_reg);
+ if (error) {
+ dev_err(&client->dev, "Failed to enable vdd regulator: %d\n",
+ error);
+ return error;
+ }
+ }
+
+ data->avdd_reg = devm_regulator_get_optional(&client->dev, "avdd");
+ if (IS_ERR(data->avdd_reg)) {
+ error = PTR_ERR(data->avdd_reg);
+ dev_err(&client->dev, "Failed to get avdd regulator: %d\n",
+ error);
+ return error;
+ }
+
+ if (data->avdd_reg) {
+ error = regulator_enable(data->avdd_reg);
+ if (error) {
+ dev_err(&client->dev, "Failed to enable avdd regulator: %d\n",
+ error);
+ return error;
+ }
+ }
+
error = devm_request_threaded_irq(&client->dev, client->irq,
NULL, mxt_interrupt, IRQF_ONESHOT,
client->name, data);
@@ -3086,6 +3124,9 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
disable_irq(client->irq);
+ if (!IS_ERR(data->vdd_reg) || !IS_ERR(data->avdd_reg))
+ msleep(MXT_REGULATOR_DELAY);
+
if (data->reset_gpio) {
msleep(MXT_RESET_GPIO_TIME);
gpiod_set_value(data->reset_gpio, 1);
@@ -3116,6 +3157,10 @@ static int mxt_remove(struct i2c_client *client)
struct mxt_data *data = i2c_get_clientdata(client);
disable_irq(data->irq);
+ if (!IS_ERR(data->avdd_reg))
+ regulator_disable(data->avdd_reg);
+ if (!IS_ERR(data->vdd_reg))
+ regulator_disable(data->vdd_reg);
sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
mxt_free_input_device(data);
mxt_free_object_table(data);
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] Input: atmel_mxt_ts: Document optional voltage regulators
From: Paweł Chmiel @ 2018-07-13 18:30 UTC (permalink / raw)
To: nick
Cc: dmitry.torokhov, robh+dt, mark.rutland, nicolas.ferre,
alexandre.belloni, linux-input, devicetree, linux-arm-kernel,
linux-kernel, Paweł Chmiel
In-Reply-To: <1531506608-24479-1-git-send-email-pawel.mikolaj.chmiel@gmail.com>
Document new optional voltage regulators, which can be used
to power down/up touchscreen.
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
index c88919480d37..17930ecadad3 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
@@ -31,6 +31,12 @@ Optional properties for main touchpad device:
- reset-gpios: GPIO specifier for the touchscreen's reset pin (active low)
+- avdd-supply: Analog power supply. It powers up the analog channel block
+ of the controller to detect the touches.
+
+- vdd-supply: Digital power supply. It powers up the digital block
+ of the controller to enable i2c communication.
+
Example:
touch@4b {
@@ -38,4 +44,6 @@ Example:
reg = <0x4b>;
interrupt-parent = <&gpio>;
interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
+ avdd-supply = <&atsp_reg>;
+ vdd-supply = <&tsp_reg>;
};
--
2.7.4
^ permalink raw reply related
* [PATCH] Input: pxrc - fix leak of usb_device
From: Alexey Khoroshilov @ 2018-07-13 20:07 UTC (permalink / raw)
To: Dmitry Torokhov, Marcus Folkesson
Cc: Alexey Khoroshilov, linux-input, linux-kernel, ldv-project
pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
anywhere in the driver.
The patch adds one to error handling code and to pxrc_disconnect().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/input/joystick/pxrc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
index 07a0dbd3ced2..0a31de63ac8e 100644
--- a/drivers/input/joystick/pxrc.c
+++ b/drivers/input/joystick/pxrc.c
@@ -221,6 +221,7 @@ static int pxrc_probe(struct usb_interface *intf,
usb_free_urb(pxrc->urb);
error:
+ usb_put_dev(pxrc->udev);
return retval;
}
@@ -229,6 +230,7 @@ static void pxrc_disconnect(struct usb_interface *intf)
struct pxrc *pxrc = usb_get_intfdata(intf);
usb_free_urb(pxrc->urb);
+ usb_put_dev(pxrc->udev);
usb_set_intfdata(intf, NULL);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] Input: synaptics - Lenovo ThinkPad T25 and T480 devices should use RMI
From: Teika Kazura @ 2018-07-14 6:35 UTC (permalink / raw)
To: kitsunyan; +Cc: dmitry.torokhov, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20180707051913.24035-1-kitsunyan@inbox.ru>
Hi, kitsunyan. (Привет, perhaps?) Thanks for you patch, but the "signed-off" tag needs your real name. You can re-submit your patch conforming to that rule. Or if you're ok, the kernel maintainer can instead sign it off, adding "reported-by: kitsunyan <kitsunyan@inbox.ru>". (I'm not sure about the latter case. Sorry if I'm wrong.)
You can find the instruction in <kernell source tree>/Documentation/process/submitting-patches.rst .
Best regards / Пока,
Teika (Teika kazura)
From: kitsunyan <kitsunyan@inbox.ru>
Subject: [PATCH] Input: synaptics - Lenovo ThinkPad T25 and T480 devices should use RMI
Date: Sat, 7 Jul 2018 08:19:12 +0300
> The touchpads on both T25 and T480 are accessible over SMBUS/RMI.
>
> Signed-off-by: kitsunyan <kitsunyan@inbox.ru>
> ---
> drivers/input/mouse/synaptics.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 55d33500d55e..be934a082424 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -175,7 +175,9 @@ static const char * const smbus_pnp_ids[] = {
> "LEN0071", /* T480 */
> "LEN0072", /* X1 Carbon Gen 5 (2017) - Elan/ALPS trackpoint */
> "LEN0073", /* X1 Carbon G5 (Elantech) */
> + "LEN008e", /* T25 */
> "LEN0092", /* X1 Carbon 6 */
> + "LEN0093", /* T480 */
> "LEN0096", /* X280 */
> "LEN0097", /* X280 -> ALPS trackpoint */
> "LEN200f", /* T450s */
> --
> 2.18.0
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Marcus Folkesson @ 2018-07-14 8:09 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Dmitry Torokhov, linux-input, linux-kernel, ldv-project
In-Reply-To: <1531512477-31981-1-git-send-email-khoroshilov@ispras.ru>
Hi Alexey,
Good catch!
On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> anywhere in the driver.
>
> The patch adds one to error handling code and to pxrc_disconnect().
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
> drivers/input/joystick/pxrc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
> index 07a0dbd3ced2..0a31de63ac8e 100644
> --- a/drivers/input/joystick/pxrc.c
> +++ b/drivers/input/joystick/pxrc.c
> @@ -221,6 +221,7 @@ static int pxrc_probe(struct usb_interface *intf,
> usb_free_urb(pxrc->urb);
>
> error:
> + usb_put_dev(pxrc->udev);
> return retval;
> }
>
> @@ -229,6 +230,7 @@ static void pxrc_disconnect(struct usb_interface *intf)
> struct pxrc *pxrc = usb_get_intfdata(intf);
>
> usb_free_urb(pxrc->urb);
> + usb_put_dev(pxrc->udev);
> usb_set_intfdata(intf, NULL);
> }
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Dmitry Torokhov @ 2018-07-14 8:51 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Alexey Khoroshilov, linux-input, linux-kernel, ldv-project
In-Reply-To: <20180714080920.GA15123@gmail.com>
On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
> Hi Alexey,
>
> Good catch!
>
> On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> > anywhere in the driver.
> >
> > The patch adds one to error handling code and to pxrc_disconnect().
> >
> > Found by Linux Driver Verification project (linuxtesting.org).
> >
> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
>
> Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Hmm, the biggest question however if we need to "take" the device, as I
do not think interface can outlive the device, and whether we actually
need to store it in pxrc, as we only need it during set up, as far as I
can see.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics - Lenovo ThinkPad T25 and T480 devices should use RMI
From: kitsunyan @ 2018-07-14 13:20 UTC (permalink / raw)
To: Teika Kazura
Cc: dmitry.torokhov, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20180714.153501.1002564188152837590.teika@gmx.com>
Hi, Teika. (Yes, you are right.)
I should've worried about real name earlier since I already sent the patch from pseudonym. I don't mind if the patch will be authored by maintainer, replacing my sign with "reported-by". (Hope it's possible.)
Regards,
kitsunyan
On 14/07/18 09:35, Teika Kazura wrote:
> Hi, kitsunyan. (Привет, perhaps?) Thanks for you patch, but the "signed-off" tag needs your real name. You can re-submit your patch conforming to that rule. Or if you're ok, the kernel maintainer can instead sign it off, adding "reported-by: kitsunyan <kitsunyan@inbox.ru>". (I'm not sure about the latter case. Sorry if I'm wrong.)
>
> You can find the instruction in <kernell source tree>/Documentation/process/submitting-patches.rst .
>
> Best regards / Пока,
> Teika (Teika kazura)
>
>
> From: kitsunyan <kitsunyan@inbox.ru>
> Subject: [PATCH] Input: synaptics - Lenovo ThinkPad T25 and T480 devices should use RMI
> Date: Sat, 7 Jul 2018 08:19:12 +0300
>
>> The touchpads on both T25 and T480 are accessible over SMBUS/RMI.
>>
>> Signed-off-by: kitsunyan <kitsunyan@inbox.ru>
>> ---
>> drivers/input/mouse/synaptics.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index 55d33500d55e..be934a082424 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -175,7 +175,9 @@ static const char * const smbus_pnp_ids[] = {
>> "LEN0071", /* T480 */
>> "LEN0072", /* X1 Carbon Gen 5 (2017) - Elan/ALPS trackpoint */
>> "LEN0073", /* X1 Carbon G5 (Elantech) */
>> + "LEN008e", /* T25 */
>> "LEN0092", /* X1 Carbon 6 */
>> + "LEN0093", /* T480 */
>> "LEN0096", /* X280 */
>> "LEN0097", /* X280 -> ALPS trackpoint */
>> "LEN200f", /* T450s */
>> --
>> 2.18.0
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Marcus Folkesson @ 2018-07-15 7:42 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Alexey Khoroshilov, linux-input, linux-kernel, ldv-project
In-Reply-To: <20180714085109.mbhk6aybpjsbuq6l@penguin>
On Sat, Jul 14, 2018 at 08:51:09AM +0000, Dmitry Torokhov wrote:
> On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
> > Hi Alexey,
> >
> > Good catch!
> >
> > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> > > anywhere in the driver.
> > >
> > > The patch adds one to error handling code and to pxrc_disconnect().
> > >
> > > Found by Linux Driver Verification project (linuxtesting.org).
> > >
> > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> >
> > Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>
> Hmm, the biggest question however if we need to "take" the device, as I
> do not think interface can outlive the device, and whether we actually
> need to store it in pxrc, as we only need it during set up, as far as I
> can see.
Yep, the device is only used during setup.
I interpret the comments for usb_get_dev() as you should take a
reference count on the device even if you only use the interface, but I
could be wrong.
>From usb_get_dev()::
* usb_get_dev - increments the reference count of the usb device structure
* @dev: the device being referenced
*
* Each live reference to a device should be refcounted.
*
* Drivers for USB interfaces should normally record such references in
* their probe() methods, when they bind to an interface, and release
* them by calling usb_put_dev(), in their disconnect() methods.
I can fix the driver to not take the device if that is what we want.
If not Alexey want to fix it of course, it is his catch :-)
>
> Thanks.
>
> --
> Dmitry
Best regards
Marcus Folkesson
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Dmitry Torokhov @ 2018-07-15 10:06 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Alexey Khoroshilov, linux-input@vger.kernel.org, lkml,
ldv-project, Greg Kroah-Hartman
In-Reply-To: <20180715074225.GB15123@gmail.com>
On Sun, Jul 15, 2018 at 10:42 AM Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:
>
> On Sat, Jul 14, 2018 at 08:51:09AM +0000, Dmitry Torokhov wrote:
> > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
> > > Hi Alexey,
> > >
> > > Good catch!
> > >
> > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> > > > anywhere in the driver.
> > > >
> > > > The patch adds one to error handling code and to pxrc_disconnect().
> > > >
> > > > Found by Linux Driver Verification project (linuxtesting.org).
> > > >
> > > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> > >
> > > Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> >
> > Hmm, the biggest question however if we need to "take" the device, as I
> > do not think interface can outlive the device, and whether we actually
> > need to store it in pxrc, as we only need it during set up, as far as I
> > can see.
>
> Yep, the device is only used during setup.
> I interpret the comments for usb_get_dev() as you should take a
> reference count on the device even if you only use the interface, but I
> could be wrong.
>
> From usb_get_dev()::
>
> * usb_get_dev - increments the reference count of the usb device structure
> * @dev: the device being referenced
> *
> * Each live reference to a device should be refcounted.
> *
> * Drivers for USB interfaces should normally record such references in
> * their probe() methods, when they bind to an interface, and release
> * them by calling usb_put_dev(), in their disconnect() methods.
Hmm, usb device is a parent of usb interface so our driver model rules
ensure that usb device should not disappear while interface device is
still there. Greg, is this comment still valid?
>
> I can fix the driver to not take the device if that is what we want.
> If not Alexey want to fix it of course, it is his catch :-)
Yeah, I'd prefer doing this if possible.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Greg Kroah-Hartman @ 2018-07-15 10:12 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Marcus Folkesson, Alexey Khoroshilov, linux-input@vger.kernel.org,
lkml, ldv-project
In-Reply-To: <CAKdAkRS0NBOo7thkevZAQXBWt_ReppjdPz6a6PQ6LtJHbPYgVg@mail.gmail.com>
On Sun, Jul 15, 2018 at 01:06:32PM +0300, Dmitry Torokhov wrote:
> On Sun, Jul 15, 2018 at 10:42 AM Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> >
> > On Sat, Jul 14, 2018 at 08:51:09AM +0000, Dmitry Torokhov wrote:
> > > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
> > > > Hi Alexey,
> > > >
> > > > Good catch!
> > > >
> > > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> > > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> > > > > anywhere in the driver.
> > > > >
> > > > > The patch adds one to error handling code and to pxrc_disconnect().
> > > > >
> > > > > Found by Linux Driver Verification project (linuxtesting.org).
> > > > >
> > > > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> > > >
> > > > Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > >
> > > Hmm, the biggest question however if we need to "take" the device, as I
> > > do not think interface can outlive the device, and whether we actually
> > > need to store it in pxrc, as we only need it during set up, as far as I
> > > can see.
> >
> > Yep, the device is only used during setup.
> > I interpret the comments for usb_get_dev() as you should take a
> > reference count on the device even if you only use the interface, but I
> > could be wrong.
> >
> > From usb_get_dev()::
> >
> > * usb_get_dev - increments the reference count of the usb device structure
> > * @dev: the device being referenced
> > *
> > * Each live reference to a device should be refcounted.
> > *
> > * Drivers for USB interfaces should normally record such references in
> > * their probe() methods, when they bind to an interface, and release
> > * them by calling usb_put_dev(), in their disconnect() methods.
>
> Hmm, usb device is a parent of usb interface so our driver model rules
> ensure that usb device should not disappear while interface device is
> still there. Greg, is this comment still valid?
Yes, that is true. But remember that interface devices can go away
while the parent is still present, so if you need the interface pointer,
you have to grab a reference on it.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Dmitry Torokhov @ 2018-07-15 10:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Marcus Folkesson, Alexey Khoroshilov, linux-input@vger.kernel.org,
lkml, ldv-project
In-Reply-To: <20180715101244.GA32410@kroah.com>
On Sun, Jul 15, 2018 at 12:12:44PM +0200, Greg Kroah-Hartman wrote:
> On Sun, Jul 15, 2018 at 01:06:32PM +0300, Dmitry Torokhov wrote:
> > On Sun, Jul 15, 2018 at 10:42 AM Marcus Folkesson
> > <marcus.folkesson@gmail.com> wrote:
> > >
> > > On Sat, Jul 14, 2018 at 08:51:09AM +0000, Dmitry Torokhov wrote:
> > > > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
> > > > > Hi Alexey,
> > > > >
> > > > > Good catch!
> > > > >
> > > > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> > > > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> > > > > > anywhere in the driver.
> > > > > >
> > > > > > The patch adds one to error handling code and to pxrc_disconnect().
> > > > > >
> > > > > > Found by Linux Driver Verification project (linuxtesting.org).
> > > > > >
> > > > > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> > > > >
> > > > > Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > > >
> > > > Hmm, the biggest question however if we need to "take" the device, as I
> > > > do not think interface can outlive the device, and whether we actually
> > > > need to store it in pxrc, as we only need it during set up, as far as I
> > > > can see.
> > >
> > > Yep, the device is only used during setup.
> > > I interpret the comments for usb_get_dev() as you should take a
> > > reference count on the device even if you only use the interface, but I
> > > could be wrong.
> > >
> > > From usb_get_dev()::
> > >
> > > * usb_get_dev - increments the reference count of the usb device structure
> > > * @dev: the device being referenced
> > > *
> > > * Each live reference to a device should be refcounted.
> > > *
> > > * Drivers for USB interfaces should normally record such references in
> > > * their probe() methods, when they bind to an interface, and release
> > > * them by calling usb_put_dev(), in their disconnect() methods.
> >
> > Hmm, usb device is a parent of usb interface so our driver model rules
> > ensure that usb device should not disappear while interface device is
> > still there. Greg, is this comment still valid?
>
> Yes, that is true. But remember that interface devices can go away
> while the parent is still present, so if you need the interface pointer,
> you have to grab a reference on it.
But not in a simple interface driver, as we'll unbind the driver before
destroying the interface... IOW we need to record the reference only if
we are doing something unusual.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] Input: atmel_mxt_ts: Add support for optional regulators.
From: Dmitry Torokhov @ 2018-07-15 10:25 UTC (permalink / raw)
To: Paweł Chmiel
Cc: nick, robh+dt, mark.rutland, nicolas.ferre, alexandre.belloni,
linux-input, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1531506608-24479-2-git-send-email-pawel.mikolaj.chmiel@gmail.com>
Hi Paweł,
On Fri, Jul 13, 2018 at 08:30:07PM +0200, Paweł Chmiel wrote:
> This patch adds optional regulators, which can be used to power
> up touchscreen. After enabling regulators, we need to wait 150msec.
> This value is taken from official driver.
>
> It was tested on Samsung Galaxy i9000 (based on Samsung S5PV210 SOC).
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 45 ++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 54fe190fd4bc..a7625ec8fb9f 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -27,6 +27,7 @@
> #include <linux/interrupt.h>
> #include <linux/of.h>
> #include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> #include <linux/gpio/consumer.h>
> #include <linux/property.h>
> @@ -198,6 +199,7 @@ enum t100_type {
> #define MXT_RESET_INVALID_CHG 100 /* msec */
> #define MXT_RESET_TIME 200 /* msec */
> #define MXT_RESET_TIMEOUT 3000 /* msec */
> +#define MXT_REGULATOR_DELAY 150 /* msec */
> #define MXT_CRC_TIMEOUT 1000 /* msec */
> #define MXT_FW_RESET_TIME 3000 /* msec */
> #define MXT_FW_CHG_TIMEOUT 300 /* msec */
> @@ -310,6 +312,8 @@ struct mxt_data {
> struct t7_config t7_cfg;
> struct mxt_dbg dbg;
> struct gpio_desc *reset_gpio;
> + struct regulator *vdd_reg;
> + struct regulator *avdd_reg;
>
> /* Cached parameters from object table */
> u16 T5_address;
> @@ -3076,6 +3080,40 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> return error;
> }
>
> + data->vdd_reg = devm_regulator_get_optional(&client->dev, "vdd");
> + if (IS_ERR(data->vdd_reg)) {
> + error = PTR_ERR(data->vdd_reg);
> + dev_err(&client->dev, "Failed to get vdd regulator: %d\n",
> + error);
> + return error;
> + }
> +
> + if (data->vdd_reg) {
> + error = regulator_enable(data->vdd_reg);
> + if (error) {
> + dev_err(&client->dev, "Failed to enable vdd regulator: %d\n",
> + error);
> + return error;
> + }
> + }
> +
> + data->avdd_reg = devm_regulator_get_optional(&client->dev, "avdd");
> + if (IS_ERR(data->avdd_reg)) {
> + error = PTR_ERR(data->avdd_reg);
> + dev_err(&client->dev, "Failed to get avdd regulator: %d\n",
> + error);
> + return error;
> + }
> +
> + if (data->avdd_reg) {
devm_regulator_get_optional() does not return NULL for regulators
not present in device tree, but rather -ENOENT, so this code is not
correct; neither is the simple IS_ERR() check above.
Moreover, the optional regulators should be used when there is a part of
IP block that can be optionally powered up, but the device can work
(with limited functionality) even when it is powered down. They should
not be used in cases when supplies are mandatory (such as vdd/avdd) but
may not be exposed to the kernel by the firmware.
Simply use devm_regulator_get() and rely on the fact that on fully
constrained system you will get a dummy regulator.
> + error = regulator_enable(data->avdd_reg);
> + if (error) {
> + dev_err(&client->dev, "Failed to enable avdd regulator: %d\n",
> + error);
> + return error;
> + }
> + }
> +
> error = devm_request_threaded_irq(&client->dev, client->irq,
> NULL, mxt_interrupt, IRQF_ONESHOT,
> client->name, data);
> @@ -3086,6 +3124,9 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
>
> disable_irq(client->irq);
>
> + if (!IS_ERR(data->vdd_reg) || !IS_ERR(data->avdd_reg))
> + msleep(MXT_REGULATOR_DELAY);
> +
> if (data->reset_gpio) {
I think you should require that if regulators are exposed then reset
gpio should also be present to ensure proper power up timings.
> msleep(MXT_RESET_GPIO_TIME);
> gpiod_set_value(data->reset_gpio, 1);
> @@ -3116,6 +3157,10 @@ static int mxt_remove(struct i2c_client *client)
> struct mxt_data *data = i2c_get_clientdata(client);
>
> disable_irq(data->irq);
> + if (!IS_ERR(data->avdd_reg))
> + regulator_disable(data->avdd_reg);
> + if (!IS_ERR(data->vdd_reg))
> + regulator_disable(data->vdd_reg);
> sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
> mxt_free_input_device(data);
> mxt_free_object_table(data);
> --
> 2.7.4
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: pxrc - fix leak of usb_device
From: Alexey Khoroshilov @ 2018-07-15 19:58 UTC (permalink / raw)
To: Marcus Folkesson, Dmitry Torokhov; +Cc: linux-input, linux-kernel, ldv-project
In-Reply-To: <20180715074225.GB15123@gmail.com>
Dear Marcus,
On 15.07.2018 10:42, Marcus Folkesson wrote:
> On Sat, Jul 14, 2018 at 08:51:09AM +0000, Dmitry Torokhov wrote:
>> On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
>>> Hi Alexey,
>>>
>>> Good catch!
>>>
>>> On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
>>>> pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
>>>> anywhere in the driver.
>>>>
>>>> The patch adds one to error handling code and to pxrc_isconnect().
>>>>
>>>> Found by Linux Driver Verification project (linuxtesting.org).
>>>>
>>>> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
>>>
>>> Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>>
>> Hmm, the biggest question however if we need to "take" the device, as I
>> do not think interface can outlive the device, and whether we actually
>> need to store it in pxrc, as we only need it during set up, as far as I
>> can see.
>
> Yep, the device is only used during setup.
> I interpret the comments for usb_get_dev() as you should take a
> reference count on the device even if you only use the interface, but I
> could be wrong.
>
>>From usb_get_dev()::
>
> * usb_get_dev - increments the reference count of the usb device structure
> * @dev: the device being referenced
> *
> * Each live reference to a device should be refcounted.
> *
> * Drivers for USB interfaces should normally record such references in
> * their probe() methods, when they bind to an interface, and release
> * them by calling usb_put_dev(), in their disconnect() methods.
>
> I can fix the driver to not take the device if that is what we want.
> If not Alexey want to fix it of course, it is his catch :-)
As far as I can see the proposed solution requires some refactoring of
the init code. So, I believe the author is in the better position to do
that.
Best regards,
Alexey
^ permalink raw reply
* [PATCH] Input: elan_i2c - Add antoher ACPI ID for Lenovo Ideapad 330-15AST
From: KT Liao @ 2018-07-16 11:26 UTC (permalink / raw)
To: linux-kernel, linux-input, dmitry.torokhov, ulrik.debie-os
Cc: anantshende, kt.liao
Add ELAN0622 in ACPI mapping table to support Ideapad 330-15AST ELAN touchpad
Signed-off-by: KT Liao <kt.liao@emc.com.tw>
Reported-by: Anant Shende <anantshende@gmail.com>
---
drivers/input/mouse/elan_i2c_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 2690a4b..f7e75f0 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -1344,6 +1344,7 @@ static const struct acpi_device_id elan_acpi_id[] = {
{ "ELAN060B", 0 },
{ "ELAN060C", 0 },
{ "ELAN0611", 0 },
+ { "ELAN0622", 0 },
{ "ELAN1000", 0 },
{ }
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] Input: elan_i2c - Add antoher ACPI ID for Lenovo Ideapad 330-15AST
From: Dmitry Torokhov @ 2018-07-16 12:20 UTC (permalink / raw)
To: KT Liao; +Cc: linux-kernel, linux-input, ulrik.debie-os, anantshende
In-Reply-To: <1531740389-4040-1-git-send-email-kt.liao@emc.com.tw>
On Mon, Jul 16, 2018 at 07:26:29PM +0800, KT Liao wrote:
> Add ELAN0622 in ACPI mapping table to support Ideapad 330-15AST ELAN touchpad
>
> Signed-off-by: KT Liao <kt.liao@emc.com.tw>
> Reported-by: Anant Shende <anantshende@gmail.com>
Applied, thank you.
> ---
> drivers/input/mouse/elan_i2c_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 2690a4b..f7e75f0 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1344,6 +1344,7 @@ static const struct acpi_device_id elan_acpi_id[] = {
> { "ELAN060B", 0 },
> { "ELAN060C", 0 },
> { "ELAN0611", 0 },
> + { "ELAN0622", 0 },
> { "ELAN1000", 0 },
> { }
> };
> --
> 2.7.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 3/5] Input: hilkbd - Add casts to HP9000/300 I/O accessors
From: Dmitry Torokhov @ 2018-07-16 12:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Ungerer, Helge Deller, linux-m68k, netdev, linux-input,
linux-kernel
In-Reply-To: <20180709093040.23597-4-geert@linux-m68k.org>
On Mon, Jul 09, 2018 at 11:30:38AM +0200, Geert Uytterhoeven wrote:
> Internally, hilkbd uses "unsigned long" I/O addresses everywhere.
> This works fine as:
> - On PA-RISC, hilkbd uses the gsc_{read,write}b() I/O accessors, which
> take "unsigned long" addresses,
> - On m68k, hilkbd uses {read,write}b(), which are currently mapped to
> {in,out}_8(), and convert the passed addresses to pointers
> internally.
>
> However, the asm-generic version of {read,write}b() does not perform
> such conversions, and requires passing pointers instead. Hence add
> casts to prepare for switching m68k to the asm-generic version.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> This is a dependency for "m68k/io: Switch mmu variant to
> <asm-generic/io.h>".
>
> v2:
> - No changes.
> ---
> drivers/input/keyboard/hilkbd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/hilkbd.c b/drivers/input/keyboard/hilkbd.c
> index a4e404aaf64bdb82..5c7afdec192c139b 100644
> --- a/drivers/input/keyboard/hilkbd.c
> +++ b/drivers/input/keyboard/hilkbd.c
> @@ -57,8 +57,8 @@ MODULE_LICENSE("GPL v2");
> #define HIL_DATA 0x1
> #define HIL_CMD 0x3
> #define HIL_IRQ 2
> - #define hil_readb(p) readb(p)
> - #define hil_writeb(v,p) writeb((v),(p))
> + #define hil_readb(p) readb((const volatile void __iomem *)(p))
> + #define hil_writeb(v, p) writeb((v), (volatile void __iomem *)(p))
>
> #else
> #error "HIL is not supported on this platform"
> --
> 2.17.1
>
--
Dmitry
^ permalink raw reply
* [PATCH] input: pxrc - do not store USB device in private struct
From: Marcus Folkesson @ 2018-07-16 14:40 UTC (permalink / raw)
To: Dmitry Torokhov, Marcus Folkesson, Alexey Khoroshilov
Cc: linux-input, linux-kernel
The USB device is only needed during setup, so put it back after
initialization and do not store it in our private struct.
Also, the USB device is a parent of USB interface so our driver
model rules ensure that USB device should not disappear while
interface device is still there.
So not keep a refcount on the device is safe.
Reported-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/joystick/pxrc.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
index 07a0dbd3ced2..46a7acb747bf 100644
--- a/drivers/input/joystick/pxrc.c
+++ b/drivers/input/joystick/pxrc.c
@@ -27,7 +27,6 @@ MODULE_DEVICE_TABLE(usb, pxrc_table);
struct pxrc {
struct input_dev *input;
- struct usb_device *udev;
struct usb_interface *intf;
struct urb *urb;
struct mutex pm_mutex;
@@ -120,7 +119,7 @@ static void pxrc_close(struct input_dev *input)
mutex_unlock(&pxrc->pm_mutex);
}
-static int pxrc_usb_init(struct pxrc *pxrc)
+static int pxrc_usb_init(struct pxrc *pxrc, struct usb_device *udev)
{
struct usb_endpoint_descriptor *epirq;
unsigned int pipe;
@@ -145,7 +144,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
usb_set_intfdata(pxrc->intf, pxrc);
- usb_make_path(pxrc->udev, pxrc->phys, sizeof(pxrc->phys));
+ usb_make_path(udev, pxrc->phys, sizeof(pxrc->phys));
strlcat(pxrc->phys, "/input0", sizeof(pxrc->phys));
pxrc->urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -154,8 +153,8 @@ static int pxrc_usb_init(struct pxrc *pxrc)
goto error;
}
- pipe = usb_rcvintpipe(pxrc->udev, pxrc->epaddr),
- usb_fill_int_urb(pxrc->urb, pxrc->udev, pipe, pxrc->data, pxrc->bsize,
+ pipe = usb_rcvintpipe(udev, pxrc->epaddr),
+ usb_fill_int_urb(pxrc->urb, udev, pipe, pxrc->data, pxrc->bsize,
pxrc_usb_irq, pxrc, 1);
error:
@@ -164,7 +163,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
-static int pxrc_input_init(struct pxrc *pxrc)
+static int pxrc_input_init(struct pxrc *pxrc, struct usb_device *udev)
{
pxrc->input = devm_input_allocate_device(&pxrc->intf->dev);
if (pxrc->input == NULL) {
@@ -174,7 +173,7 @@ static int pxrc_input_init(struct pxrc *pxrc)
pxrc->input->name = "PXRC Flight Controller Adapter";
pxrc->input->phys = pxrc->phys;
- usb_to_input_id(pxrc->udev, &pxrc->input->id);
+ usb_to_input_id(udev, &pxrc->input->id);
pxrc->input->open = pxrc_open;
pxrc->input->close = pxrc_close;
@@ -197,6 +196,7 @@ static int pxrc_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct pxrc *pxrc;
+ struct usb_device *udev;
int retval;
pxrc = devm_kzalloc(&intf->dev, sizeof(*pxrc), GFP_KERNEL);
@@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf,
return -ENOMEM;
mutex_init(&pxrc->pm_mutex);
- pxrc->udev = usb_get_dev(interface_to_usbdev(intf));
+ udev = usb_get_dev(interface_to_usbdev(intf));
pxrc->intf = intf;
- retval = pxrc_usb_init(pxrc);
+ retval = pxrc_usb_init(pxrc, udev);
if (retval)
goto error;
- retval = pxrc_input_init(pxrc);
+ retval = pxrc_input_init(pxrc, udev);
if (retval)
goto err_free_urb;
+ usb_put_dev(udev);
return 0;
err_free_urb:
usb_free_urb(pxrc->urb);
error:
+ usb_put_dev(udev);
return retval;
}
--
2.18.0
^ permalink raw reply related
* Re: [PATCH 1/8] dt-bindings: mfd: document stpmu1 pmic
From: Rob Herring @ 2018-07-16 22:14 UTC (permalink / raw)
To: Pascal PAILLET-LME
Cc: dmitry.torokhov@gmail.com, mark.rutland@arm.com,
lee.jones@linaro.org, lgirdwood@gmail.com, broonie@kernel.org,
wim@linux-watchdog.org, linux@roeck-us.net,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
benjamin.gaignard@linaro.org
In-Reply-To: <1530803657-17684-2-git-send-email-p.paillet@st.com>
On Thu, Jul 05, 2018 at 03:14:22PM +0000, Pascal PAILLET-LME wrote:
> From: pascal paillet <p.paillet@st.com>
>
> stpmu1 is a pmic from STMicroelectronics. The stpmu1 integrates 10
> regulators and 3 switches with various capabilities.
>
> Signed-off-by: pascal paillet <p.paillet@st.com>
> ---
> .../devicetree/bindings/mfd/st,stpmu1.txt | 138 +++++++++++++++++++++
> 1 file changed, 138 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/st,stpmu1.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/st,stpmu1.txt b/Documentation/devicetree/bindings/mfd/st,stpmu1.txt
> new file mode 100644
> index 0000000..53bdab4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/st,stpmu1.txt
> @@ -0,0 +1,138 @@
> +* STMicroelectronics STPMU1 Power Management IC
> +
> +Required parent device properties:
> +- compatible: "st,stpmu1"
> +- reg: the I2C slave address for the stpmu1 chip
> +- interrupts-extended: interrupt lines to use: second irq is for wakeup.
> +- #interrupt-cells: should be 2.
> +- interrupt-controller: describes the STPMU1 as an interrupt
> + controller (has its own domain). interrupt number are the following:
> + /* Interrupt Register 1 (0x50 for latch) */
> + IT_SWOUT_R=0
> + IT_SWOUT_F=1
> + IT_VBUS_OTG_R=2
> + IT_VBUS_OTG_F=3
> + IT_WAKEUP_R=4
> + IT_WAKEUP_F=5
> + IT_PONKEY_R=6
> + IT_PONKEY_F=7
> + /* Interrupt Register 2 (0x51 for latch) */
> + IT_OVP_BOOST=8
> + IT_OCP_BOOST=9
> + IT_OCP_SWOUT=10
> + IT_OCP_OTG=11
> + IT_CURLIM_BUCK4=12
> + IT_CURLIM_BUCK3=13
> + IT_CURLIM_BUCK2=14
> + IT_CURLIM_BUCK1=15
> + /* Interrupt Register 3 (0x52 for latch) */
> + IT_SHORT_SWOUT=16
> + IT_SHORT_SWOTG=17
> + IT_CURLIM_LDO6=18
> + IT_CURLIM_LDO5=19
> + IT_CURLIM_LDO4=20
> + IT_CURLIM_LDO3=21
> + IT_CURLIM_LDO2=22
> + IT_CURLIM_LDO1=23
> + /* Interrupt Register 3 (0x52 for latch) */
> + IT_SWIN_R=24
> + IT_SWIN_F=25
> + IT_RESERVED_1=26
> + IT_RESERVED_2=27
> + IT_VINLOW_R=28
> + IT_VINLOW_F=29
> + IT_TWARN_R=30
> + IT_TWARN_F=31
> +
> +Optional parent device properties:
> +- st,main_control_register:
s/_/-/
And elsewhere...
> + -bit 1: Power cycling will be performed on turn OFF condition
> + -bit 2: PWRCTRL is functional
> + -bit 3: PWRCTRL active high
> +- st,pads_pull_register:
> + -bit 1: WAKEUP pull down is not active
> + -bit 2: PWRCTRL pull up is active
> + -bit 3: PWRCTRL pull down is active
> + -bit 4: WAKEUP detector is disabled
> +- st,vin_control_register:
> + -bit 0: VINLOW monitoring is enabled
> + -bit [1...3]: VINLOW rising threshold
> + 000 VINOK_f + 50mV
> + 001 VINOK_f + 100mV
> + 010 VINOK_f + 150mV
> + 011 VINOK_f + 200mV
> + 100 VINOK_f + 250mV
> + 101 VINOK_f + 300mV
> + 110 VINOK_f + 350mV
> + 111 VINOK_f + 400mV
> + -bit [4...5]: VINLOW hyst
> + 00 100mV
> + 01 200mV
> + 10 300mV
> + 11 400mV
> + -bit 6: SW_OUT detector is disabled
> + -bit 7: SW_IN detector is enabled.
> +- st,usb_control_register:
> + -bit 3: SW_OUT current limit
> + 0: 600mA
> + 1: 1.1A
> + -bit 4: VBUS_OTG discharge is enabled
> + -bit 5: SW_OUT discharge is enabled
> + -bit 6: VBUS_OTG detection is enabled
> + -bit 7: BOOST_OVP is disabled
Just dumping register values into DT is not the greatest design.
> +
> +
> +stpmu1 consists is a varied group of sub-devices:
> +
> +Device Description
> +------ ------------
> +stpmu1-onkey : On key
> +stpmu1-regulators : Regulators
> +stpmu1-wdt : Watchdog
These should match the node name below.
> +
> +each sub-device bindings is be described in associated driver
> +documentation section.
> +
> +Example:
> +
> +pmic: stpmu1@33 {
> + compatible = "st,stpmu1";
> + reg = <0x33>;
> + interrupts = <0 2>;
> + interrupts-extended = <&intc GIC_SPI 149 IRQ_TYPE_NONE>,
> + <&exti 55 1>;
> + st,version_status = <0x10>;
> + st,main_control_register=<0x0c>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + onkey {
> + compatible = "st,stpmu1-onkey";
> + interrupt-parent = <&pmic>;
> + interrupts = <7 0>,<6 1>;
> + st,onkey-pwroff-enabled;
> + st,onkey-press-seconds = <10>;
IIRC, we have some standard properties for these.
> + };
> +
> + watchdog {
> + compatible = "st,stpmu1-wdt";
> + };
> +
> + regulators {
> + compatible = "st,stpmu1-regulators";
> +
> + vdd_core: regulator@0 {
unit-address without reg is not valid. regulator-buck1, etc. instead.
> + regulator-compatible = "buck1";
> + regulator-name = "vdd_core";
> + regulator-boot-on;
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1200000>;
> + };
> + vdd: regulator@1 {
> + regulator-compatible = "buck3";
> + regulator-name = "vdd";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-boot-on;
> + regulator-pull-down;
> + };
> + };
> --
> 1.9.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox