* Re: [PATCH v2] drm/bridge: sil_sii8620: make remote control optional.
From: Andrzej Hajda @ 2019-03-04 7:13 UTC (permalink / raw)
To: Inki Dae, Laurent Pinchart, Dmitry Torokhov, Lukas Wunner,
dri-devel, linux-input, linux-kernel
In-Reply-To: <20190304021325.GA30785@innovation.ch>
On 04.03.2019 03:13, Life is hard, and then you die wrote:
> On Thu, Jan 24, 2019 at 05:33:55PM -0800, Ronald Tschalär wrote:
>> commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency
>> of RC_CORE) changed the driver to select both RC_CORE and INPUT.
>> However, this causes problems with other drivers, in particular an input
>> driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate
>> commit):
>>
>> drivers/clk/Kconfig:9:error: recursive dependency detected!
>> drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
>> drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
>> drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
>> drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
>> drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
>> drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
>> drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
>> drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
>>
>> According to the docs and general consensus, select should only be used
>> for non user-visible symbols, but both RC_CORE and INPUT are
>> user-visible. Furthermore almost all other references to INPUT
>> throughout the kernel config are depends, not selects. For this reason
>> the first part of this change reverts commit d6abe6df706c.
>>
>> In order to address the original reason for commit d6abe6df706c, namely
>> that not all boards use the remote controller functionality and hence
>> should not need have to deal with RC_CORE, the second part of this
>> change now makes the remote control support in the driver optional and
>> contingent on RC_CORE being defined. And with this the hard dependency
>> on INPUT also goes away as that is only needed if RC_CORE is defined
>> (which in turn already depends on INPUT).
>>
>> CC: Inki Dae <inki.dae@samsung.com>
>> CC: Andrzej Hajda <a.hajda@samsung.com>
>> CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
>> ---
>> Resending this, as I somehow managed to forget to cc dri-devel.
>> Apologies for the duplication.
>>
>> Changes in v2:
>> - completely remove dependencies on both RC_CORE and INPUT in Kconfig,
>> - make remote control functionality in driver contingent on RC_CORE
>> being defined
>>
>> drivers/gpu/drm/bridge/Kconfig | 2 --
>> drivers/gpu/drm/bridge/sil-sii8620.c | 17 +++++++++++++++++
>> 2 files changed, 17 insertions(+), 2 deletions(-)
> [snip]
>
> Is there anything I can do to help get this reviewed and moved forward?
Addressing my comments [1] ? :)
Ah I see, for some reasons (my mistake apparently) my response was not
sent to you, sorry.
[1]: https://lkml.org/lkml/2019/1/28/258
Regards
Andrzej
>
>
> Cheers,
>
> Ronald
>
>
>
^ permalink raw reply
* Re: [PATCH v2] drm/bridge: sil_sii8620: make remote control optional.
From: Life is hard, and then you die @ 2019-03-04 2:13 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae, Laurent Pinchart, Dmitry Torokhov
Cc: Lukas Wunner, dri-devel, linux-input, linux-kernel
In-Reply-To: <20190125013355.GA6722@innovation.ch>
On Thu, Jan 24, 2019 at 05:33:55PM -0800, Ronald Tschalär wrote:
> commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency
> of RC_CORE) changed the driver to select both RC_CORE and INPUT.
> However, this causes problems with other drivers, in particular an input
> driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate
> commit):
>
> drivers/clk/Kconfig:9:error: recursive dependency detected!
> drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
> drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
> drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
> drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
> drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
> drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
> drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
>
> According to the docs and general consensus, select should only be used
> for non user-visible symbols, but both RC_CORE and INPUT are
> user-visible. Furthermore almost all other references to INPUT
> throughout the kernel config are depends, not selects. For this reason
> the first part of this change reverts commit d6abe6df706c.
>
> In order to address the original reason for commit d6abe6df706c, namely
> that not all boards use the remote controller functionality and hence
> should not need have to deal with RC_CORE, the second part of this
> change now makes the remote control support in the driver optional and
> contingent on RC_CORE being defined. And with this the hard dependency
> on INPUT also goes away as that is only needed if RC_CORE is defined
> (which in turn already depends on INPUT).
>
> CC: Inki Dae <inki.dae@samsung.com>
> CC: Andrzej Hajda <a.hajda@samsung.com>
> CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> ---
> Resending this, as I somehow managed to forget to cc dri-devel.
> Apologies for the duplication.
>
> Changes in v2:
> - completely remove dependencies on both RC_CORE and INPUT in Kconfig,
> - make remote control functionality in driver contingent on RC_CORE
> being defined
>
> drivers/gpu/drm/bridge/Kconfig | 2 --
> drivers/gpu/drm/bridge/sil-sii8620.c | 17 +++++++++++++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
[snip]
Is there anything I can do to help get this reviewed and moved forward?
Cheers,
Ronald
^ permalink raw reply
* [PATCH 9/9] HID: intel-ish-hid: Use the new interface functions in HID ish client
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Only include intel-ish-client-if.h, which has all interfaces required to
implment ISHTP client. There is no longer any direct field access from
core ISHTP only include files.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 68 +++++++++-----------
drivers/hid/intel-ish-hid/ishtp-hid.c | 2 -
drivers/hid/intel-ish-hid/ishtp-hid.h | 6 +-
3 files changed, 35 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 9084fea4c247..5b98e36e3a81 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -17,8 +17,6 @@
#include <linux/hid.h>
#include <linux/intel-ish-client-if.h>
#include <linux/sched.h>
-#include "ishtp/ishtp-dev.h"
-#include "ishtp/client.h"
#include "ishtp-hid.h"
/* Rx ring buffer pool size */
@@ -40,7 +38,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
size_t cur_pos, size_t payload_len)
{
struct hostif_msg *recv_msg = recv_buf;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
"total_bad=%u cur_pos=%u\n"
@@ -77,7 +75,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
struct report_list *reports_list;
char *reports;
size_t report_len;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
int curr_hid_dev = client_data->cur_hid_dev;
payload = recv_buf + sizeof(struct hostif_msg_hdr);
@@ -91,7 +89,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
(unsigned int)data_len,
(unsigned int)sizeof(struct hostif_msg_hdr));
++client_data->bad_recv_cnt;
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
@@ -104,7 +102,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg, cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
@@ -119,7 +117,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
client_data->hid_dev_count = (unsigned int)*payload;
@@ -168,7 +166,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
if (!client_data->hid_descr[curr_hid_dev])
@@ -193,7 +191,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
if (!client_data->report_descr[curr_hid_dev])
@@ -298,7 +296,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg, cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
@@ -478,7 +476,7 @@ int ishtp_hid_link_ready_wait(struct ishtp_cl_data *client_data)
static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
{
struct hostif_msg msg;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
int retry_count;
int rv;
@@ -515,7 +513,7 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
}
client_data->num_hid_devices = client_data->hid_dev_count;
- dev_info(&hid_ishtp_cl->device->dev,
+ dev_info(ishtp_device(client_data->cl_device),
"[hid-ish]: enum_devices_done OK, num_hid_devices=%d\n",
client_data->num_hid_devices);
@@ -534,7 +532,7 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
static int ishtp_get_hid_descriptor(struct ishtp_cl *hid_ishtp_cl, int index)
{
struct hostif_msg msg;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
int rv;
/* Get HID descriptor */
@@ -581,7 +579,7 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
int index)
{
struct hostif_msg msg;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
int rv;
/* Get report descriptor */
@@ -629,7 +627,7 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
{
struct ishtp_device *dev;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
struct ishtp_fw_client *fw_client;
int i;
int rv;
@@ -646,11 +644,11 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
client_data->init_done = 0;
- dev = hid_ishtp_cl->dev;
+ dev = ishtp_get_ishtp_device(hid_ishtp_cl);
/* Connect to FW client */
- hid_ishtp_cl->rx_ring_size = HID_CL_RX_RING_SIZE;
- hid_ishtp_cl->tx_ring_size = HID_CL_TX_RING_SIZE;
+ ishtp_set_tx_ring_size(hid_ishtp_cl, HID_CL_TX_RING_SIZE);
+ ishtp_set_rx_ring_size(hid_ishtp_cl, HID_CL_RX_RING_SIZE);
fw_client = ishtp_fw_cl_get_client(dev, &hid_ishtp_guid);
if (!fw_client) {
@@ -658,9 +656,9 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
"ish client uuid not found\n");
return -ENOENT;
}
-
- hid_ishtp_cl->fw_client_id = fw_client->client_id;
- hid_ishtp_cl->state = ISHTP_CL_CONNECTING;
+ ishtp_cl_set_fw_client_id(hid_ishtp_cl,
+ ishtp_get_fw_client_id(fw_client));
+ ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_CONNECTING);
rv = ishtp_cl_connect(hid_ishtp_cl);
if (rv) {
@@ -672,7 +670,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
hid_ishtp_trace(client_data, "%s client connected\n", __func__);
/* Register read callback */
- ishtp_register_event_cb(hid_ishtp_cl->device, ish_cl_event_cb);
+ ishtp_register_event_cb(client_data->cl_device, ish_cl_event_cb);
rv = ishtp_enum_enum_devices(hid_ishtp_cl);
if (rv)
@@ -710,7 +708,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
return 0;
err_cl_disconnect:
- hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
+ ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_DISCONNECTING);
ishtp_cl_disconnect(hid_ishtp_cl);
err_cl_unlink:
ishtp_cl_unlink(hid_ishtp_cl);
@@ -747,7 +745,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
- dev_dbg(&cl_device->dev, "%s\n", __func__);
+ dev_dbg(ishtp_device(client_data->cl_device), "%s\n", __func__);
hid_ishtp_cl_deinit(hid_ishtp_cl);
@@ -756,7 +754,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
return;
ishtp_set_drvdata(cl_device, hid_ishtp_cl);
- hid_ishtp_cl->client_data = client_data;
+ ishtp_set_client_data(hid_ishtp_cl, client_data);
client_data->hid_ishtp_cl = hid_ishtp_cl;
client_data->num_hid_devices = 0;
@@ -774,7 +772,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
}
}
-void (*hid_print_trace)(void *dev, const char *format, ...);
+void (*hid_print_trace)(void *unused, const char *format, ...);
/**
* hid_ishtp_cl_probe() - ISHTP client driver probe
@@ -804,7 +802,7 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
return -ENOMEM;
ishtp_set_drvdata(cl_device, hid_ishtp_cl);
- hid_ishtp_cl->client_data = client_data;
+ ishtp_set_client_data(hid_ishtp_cl, client_data);
client_data->hid_ishtp_cl = hid_ishtp_cl;
client_data->cl_device = cl_device;
@@ -836,13 +834,13 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
{
struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
dev_dbg(ishtp_device(cl_device), "%s\n", __func__);
- hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
+ ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_DISCONNECTING);
ishtp_cl_disconnect(hid_ishtp_cl);
ishtp_put_device(cl_device);
ishtp_hid_remove(client_data);
@@ -866,7 +864,7 @@ static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
{
struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
@@ -876,8 +874,6 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
return 0;
}
-#define to_ishtp_cl_device(d) container_of(d, struct ishtp_cl_device, dev)
-
/**
* hid_ishtp_cl_suspend() - ISHTP client driver suspend
* @device: device instance
@@ -888,9 +884,9 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
*/
static int hid_ishtp_cl_suspend(struct device *device)
{
- struct ishtp_cl_device *cl_device = to_ishtp_cl_device(device);
+ struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
@@ -909,9 +905,9 @@ static int hid_ishtp_cl_suspend(struct device *device)
*/
static int hid_ishtp_cl_resume(struct device *device)
{
- struct ishtp_cl_device *cl_device = to_ishtp_cl_device(device);
+ struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.c b/drivers/hid/intel-ish-hid/ishtp-hid.c
index cb71ad43ea3b..7fac8fa593b9 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.c
@@ -16,7 +16,6 @@
#include <linux/hid.h>
#include <linux/intel-ish-client-if.h>
#include <uapi/linux/input.h>
-#include "ishtp/client.h"
#include "ishtp-hid.h"
/**
@@ -117,7 +116,6 @@ static void ishtp_hid_request(struct hid_device *hid, struct hid_report *rep,
static int ishtp_wait_for_response(struct hid_device *hid)
{
struct ishtp_hid_data *hid_data = hid->driver_data;
- struct ishtp_cl_data *client_data = hid_data->client_data;
int rv;
hid_ishtp_trace(client_data, "%s hid %p\n", __func__, hid);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h
index e1b00f5125c7..5e38345e5ee6 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.h
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
@@ -24,9 +24,9 @@
#define IS_RESPONSE 0x80
/* Used to dump to Linux trace buffer, if enabled */
-#define hid_ishtp_trace(client, ...) \
- client->cl_device->ishtp_dev->print_log(\
- client->cl_device->ishtp_dev, __VA_ARGS__)
+extern void (*hid_print_trace)(void *unused, const char *format, ...);
+#define hid_ishtp_trace(client, ...) \
+ (hid_print_trace)(NULL, __VA_ARGS__)
/* ISH Transport protocol (ISHTP in short) GUID */
static const guid_t hid_ishtp_guid =
--
2.17.2
^ permalink raw reply related
* [PATCH 8/9] HID: intel-ish-hid: Move functions related to bus and device
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Move function idefinitions related to bus and device to common header file.
Also create new function to get fw client id and move ish_hw_reset() from
inline to exported function.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp/bus.c | 26 +++++++++++++++++++++
drivers/hid/intel-ish-hid/ishtp/bus.h | 11 ---------
drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 5 ----
include/linux/intel-ish-client-if.h | 12 ++++++++++
4 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 2ca65864192f..66a485a41481 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -170,6 +170,19 @@ struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
}
EXPORT_SYMBOL(ishtp_fw_cl_get_client);
+/**
+ * ishtp_get_fw_client_id() - Get fw client id
+ *
+ * This interface is used to reset HW get FW client id.
+ *
+ * Return: firmware client id.
+ */
+int ishtp_get_fw_client_id(struct ishtp_fw_client *fw_client)
+{
+ return fw_client->client_id;
+}
+EXPORT_SYMBOL(ishtp_get_fw_client_id);
+
/**
* ishtp_fw_cl_by_id() - return index to fw_clients for client_id
* @dev: the ishtp device structure
@@ -855,6 +868,19 @@ void *ishtp_trace_callback(struct ishtp_cl_device *cl_device)
}
EXPORT_SYMBOL(ishtp_trace_callback);
+/**
+ * ish_hw_reset() - Call HW reset IPC callback
+ *
+ * This interface is used to reset HW in case of error.
+ *
+ * Return: value from IPC hw_reset callback
+ */
+int ish_hw_reset(struct ishtp_device *dev)
+{
+ return dev->ops->hw_reset(dev);
+}
+EXPORT_SYMBOL(ish_hw_reset);
+
/**
* ishtp_bus_register() - Function to register bus
*
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index 4aed195719de..93d516f5a853 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -80,16 +80,5 @@ void ishtp_recv(struct ishtp_device *dev);
void ishtp_reset_handler(struct ishtp_device *dev);
void ishtp_reset_compl_handler(struct ishtp_device *dev);
-void ishtp_put_device(struct ishtp_cl_device *);
-void ishtp_get_device(struct ishtp_cl_device *);
-
-void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
-void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
-
-int ishtp_register_event_cb(struct ishtp_cl_device *device,
- void (*read_cb)(struct ishtp_cl_device *));
int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
-struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
- const guid_t *uuid);
-
#endif /* _LINUX_ISHTP_CL_BUS_H */
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index e0a320e67a41..3cfef084b9fc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -238,11 +238,6 @@ static inline int ish_ipc_reset(struct ishtp_device *dev)
return dev->ops->ipc_reset(dev);
}
-static inline int ish_hw_reset(struct ishtp_device *dev)
-{
- return dev->ops->hw_reset(dev);
-}
-
/* Exported function */
void ishtp_device_init(struct ishtp_device *dev);
int ishtp_start(struct ishtp_device *dev);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 526e3048e09f..e98bfbb1e07e 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -9,7 +9,9 @@
#define _INTEL_ISH_CLIENT_IF_H_
struct ishtp_cl_device;
+struct ishtp_device;
struct ishtp_cl;
+struct ishtp_fw_client;
/* Client state */
enum cl_state {
@@ -95,4 +97,14 @@ void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size);
void ishtp_set_connection_state(struct ishtp_cl *cl, int state);
void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id);
+void ishtp_put_device(struct ishtp_cl_device *cl_dev);
+void ishtp_get_device(struct ishtp_cl_device *cl_dev);
+void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
+void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
+int ishtp_register_event_cb(struct ishtp_cl_device *device,
+ void (*read_cb)(struct ishtp_cl_device *));
+struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
+ const guid_t *uuid);
+int ishtp_get_fw_client_id(struct ishtp_fw_client *fw_client);
+int ish_hw_reset(struct ishtp_device *dev);
#endif /* _INTEL_ISH_CLIENT_IF_H_ */
--
2.17.2
^ permalink raw reply related
* [PATCH 7/9] HID: intel-ish-hid: Add interface functions for struct ishtp_cl
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Instead of directly accessing members of struct ishtp_cl, create interface
functions to access them.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp/client.c | 42 ++++++++++++++++++++++++
include/linux/intel-ish-client-if.h | 7 ++++
2 files changed, 49 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 657b46dcefa6..b7ac5e3b1e82 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -1063,3 +1063,45 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
eoi:
return;
}
+
+void *ishtp_get_client_data(struct ishtp_cl *cl)
+{
+ return cl->client_data;
+}
+EXPORT_SYMBOL(ishtp_get_client_data);
+
+void ishtp_set_client_data(struct ishtp_cl *cl, void *data)
+{
+ cl->client_data = data;
+}
+EXPORT_SYMBOL(ishtp_set_client_data);
+
+struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl)
+{
+ return cl->dev;
+}
+EXPORT_SYMBOL(ishtp_get_ishtp_device);
+
+void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size)
+{
+ cl->tx_ring_size = size;
+}
+EXPORT_SYMBOL(ishtp_set_tx_ring_size);
+
+void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size)
+{
+ cl->rx_ring_size = size;
+}
+EXPORT_SYMBOL(ishtp_set_rx_ring_size);
+
+void ishtp_set_connection_state(struct ishtp_cl *cl, int state)
+{
+ cl->state = state;
+}
+EXPORT_SYMBOL(ishtp_set_connection_state);
+
+void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id)
+{
+ cl->fw_client_id = fw_client_id;
+}
+EXPORT_SYMBOL(ishtp_cl_set_fw_client_id);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 7ce172f656f8..526e3048e09f 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -87,5 +87,12 @@ int ishtp_cl_flush_queues(struct ishtp_cl *cl);
int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
+void *ishtp_get_client_data(struct ishtp_cl *cl);
+void ishtp_set_client_data(struct ishtp_cl *cl, void *data);
+struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl);
+void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size);
+void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size);
+void ishtp_set_connection_state(struct ishtp_cl *cl, int state);
+void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id);
#endif /* _INTEL_ISH_CLIENT_IF_H_ */
--
2.17.2
^ permalink raw reply related
* [PATCH 6/9] HID: intel-ish-hid: Move the common functions from client.h
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Move the interface functions in client.h to common include. These are
already abstracted well to use as is. Also move any associated structures
used by these functions.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp/client.h | 24 -----------
drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 26 -----------
include/linux/intel-ish-client-if.h | 48 +++++++++++++++++++++
3 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index afa8b1f521d0..6ed00947d6bc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -19,15 +19,6 @@
#include <linux/types.h>
#include "ishtp-dev.h"
-/* Client state */
-enum cl_state {
- ISHTP_CL_INITIALIZING = 0,
- ISHTP_CL_CONNECTING,
- ISHTP_CL_CONNECTED,
- ISHTP_CL_DISCONNECTING,
- ISHTP_CL_DISCONNECTED
-};
-
/* Tx and Rx ring size */
#define CL_DEF_RX_RING_SIZE 2
#define CL_DEF_TX_RING_SIZE 2
@@ -169,19 +160,4 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
(cl1->fw_client_id == cl2->fw_client_id);
}
-/* exported functions from ISHTP under client management scope */
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
-void ishtp_cl_free(struct ishtp_cl *cl);
-int ishtp_cl_link(struct ishtp_cl *cl);
-void ishtp_cl_unlink(struct ishtp_cl *cl);
-int ishtp_cl_disconnect(struct ishtp_cl *cl);
-int ishtp_cl_connect(struct ishtp_cl *cl);
-int ishtp_cl_send(struct ishtp_cl *cl, uint8_t *buf, size_t length);
-int ishtp_cl_flush_queues(struct ishtp_cl *cl);
-
-/* exported functions from ISHTP client buffer management scope */
-int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
-bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
-struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
-
#endif /* _ISHTP_CLIENT_H_ */
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index e54ce1ef27dd..e0a320e67a41 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -79,32 +79,6 @@ struct ishtp_fw_client {
uint8_t client_id;
};
-/**
- * struct ishtp_msg_data - ISHTP message data struct
- * @size: Size of data in the *data
- * @data: Pointer to data
- */
-struct ishtp_msg_data {
- uint32_t size;
- unsigned char *data;
-};
-
-/*
- * struct ishtp_cl_rb - request block structure
- * @list: Link to list members
- * @cl: ISHTP client instance
- * @buffer: message header
- * @buf_idx: Index into buffer
- * @read_time: unused at this time
- */
-struct ishtp_cl_rb {
- struct list_head list;
- struct ishtp_cl *cl;
- struct ishtp_msg_data buffer;
- unsigned long buf_idx;
- unsigned long read_time;
-};
-
/*
* Control info for IPC messages ISHTP/IPC sending FIFO -
* list with inline data buffer
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index abc0b8122f07..7ce172f656f8 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -9,6 +9,16 @@
#define _INTEL_ISH_CLIENT_IF_H_
struct ishtp_cl_device;
+struct ishtp_cl;
+
+/* Client state */
+enum cl_state {
+ ISHTP_CL_INITIALIZING = 0,
+ ISHTP_CL_CONNECTING,
+ ISHTP_CL_CONNECTED,
+ ISHTP_CL_DISCONNECTING,
+ ISHTP_CL_DISCONNECTED
+};
/**
* struct ishtp_cl_device - ISHTP device handle
@@ -29,6 +39,32 @@ struct ishtp_cl_driver {
const struct dev_pm_ops *pm;
};
+/**
+ * struct ishtp_msg_data - ISHTP message data struct
+ * @size: Size of data in the *data
+ * @data: Pointer to data
+ */
+struct ishtp_msg_data {
+ uint32_t size;
+ unsigned char *data;
+};
+
+/*
+ * struct ishtp_cl_rb - request block structure
+ * @list: Link to list members
+ * @cl: ISHTP client instance
+ * @buffer: message header
+ * @buf_idx: Index into buffer
+ * @read_time: unused at this time
+ */
+struct ishtp_cl_rb {
+ struct list_head list;
+ struct ishtp_cl *cl;
+ struct ishtp_msg_data buffer;
+ unsigned long buf_idx;
+ unsigned long read_time;
+};
+
int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
struct module *owner);
void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
@@ -40,4 +76,16 @@ struct device *ishtp_device(struct ishtp_cl_device *cl_device);
/* Trace interface for clients */
void *ishtp_trace_callback(struct ishtp_cl_device *cl_device);
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
+void ishtp_cl_free(struct ishtp_cl *cl);
+int ishtp_cl_link(struct ishtp_cl *cl);
+void ishtp_cl_unlink(struct ishtp_cl *cl);
+int ishtp_cl_disconnect(struct ishtp_cl *cl);
+int ishtp_cl_connect(struct ishtp_cl *cl);
+int ishtp_cl_send(struct ishtp_cl *cl, uint8_t *buf, size_t length);
+int ishtp_cl_flush_queues(struct ishtp_cl *cl);
+int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
+bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
+struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
+
#endif /* _INTEL_ISH_CLIENT_IF_H_ */
--
2.17.2
^ permalink raw reply related
* [PATCH 5/9] HID: intel-ish-hid: Store ishtp_cl_device instance in device
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Store ishtp_cl_device pointer in device struct private data. In this
way we can get ishtp_cl_device * from device struct pointer.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp/bus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 95a97534fcda..2ca65864192f 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -466,6 +466,7 @@ static struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device *dev,
}
ishtp_device_ready = true;
+ dev_set_drvdata(&device->dev, device);
return device;
}
--
2.17.2
^ permalink raw reply related
* [PATCH 4/9] HID: intel-ish-hid: Move driver registry functions
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Move the driver registry with the ishtp bus to the common interface
file, which clients can include.
Also rename __ishtp_cl_driver_register() to ishtp_cl_driver_register()
and removed define for ishtp_cl_driver_register.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 2 +-
drivers/hid/intel-ish-hid/ishtp/bus.c | 8 +++---
drivers/hid/intel-ish-hid/ishtp/bus.h | 27 +-------------------
include/linux/intel-ish-client-if.h | 25 ++++++++++++++++++
4 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 287d71338b91..9084fea4c247 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -938,7 +938,7 @@ static int __init ish_hid_init(void)
int rv;
/* Register ISHTP client device driver with ISHTP Bus */
- rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver);
+ rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver, THIS_MODULE);
return rv;
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 308853eab91c..95a97534fcda 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -485,7 +485,7 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device *device)
}
/**
- * __ishtp_cl_driver_register() - Client driver register
+ * ishtp_cl_driver_register() - Client driver register
* @driver: the client driver instance
* @owner: Owner of this driver module
*
@@ -494,8 +494,8 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device *device)
*
* Return: Return value of driver_register or -ENODEV if not ready
*/
-int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
- struct module *owner)
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+ struct module *owner)
{
int err;
@@ -512,7 +512,7 @@ int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
return 0;
}
-EXPORT_SYMBOL(__ishtp_cl_driver_register);
+EXPORT_SYMBOL(ishtp_cl_driver_register);
/**
* ishtp_cl_driver_unregister() - Client driver unregister
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index c96e7fb42f01..4aed195719de 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -17,6 +17,7 @@
#include <linux/device.h>
#include <linux/mod_devicetable.h>
+#include <linux/intel-ish-client-if.h>
struct ishtp_cl;
struct ishtp_cl_device;
@@ -52,26 +53,6 @@ struct ishtp_cl_device {
void (*event_cb)(struct ishtp_cl_device *device);
};
-/**
- * struct ishtp_cl_device - ISHTP device handle
- * @driver: driver instance on a bus
- * @name: Name of the device for probe
- * @probe: driver callback for device probe
- * @remove: driver callback on device removal
- *
- * Client drivers defines to get probed/removed for ISHTP client device.
- */
-struct ishtp_cl_driver {
- struct device_driver driver;
- const char *name;
- const guid_t *guid;
- int (*probe)(struct ishtp_cl_device *dev);
- int (*remove)(struct ishtp_cl_device *dev);
- int (*reset)(struct ishtp_cl_device *dev);
- const struct dev_pm_ops *pm;
-};
-
-
int ishtp_bus_new_client(struct ishtp_device *dev);
void ishtp_remove_all_clients(struct ishtp_device *dev);
int ishtp_cl_device_bind(struct ishtp_cl *cl);
@@ -105,12 +86,6 @@ void ishtp_get_device(struct ishtp_cl_device *);
void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
-int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
- struct module *owner);
-#define ishtp_cl_driver_register(driver) \
- __ishtp_cl_driver_register(driver, THIS_MODULE)
-void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
-
int ishtp_register_event_cb(struct ishtp_cl_device *device,
void (*read_cb)(struct ishtp_cl_device *));
int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 11e285172735..abc0b8122f07 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -10,6 +10,31 @@
struct ishtp_cl_device;
+/**
+ * struct ishtp_cl_device - ISHTP device handle
+ * @driver: driver instance on a bus
+ * @name: Name of the device for probe
+ * @probe: driver callback for device probe
+ * @remove: driver callback on device removal
+ *
+ * Client drivers defines to get probed/removed for ISHTP client device.
+ */
+struct ishtp_cl_driver {
+ struct device_driver driver;
+ const char *name;
+ const guid_t *guid;
+ int (*probe)(struct ishtp_cl_device *dev);
+ int (*remove)(struct ishtp_cl_device *dev);
+ int (*reset)(struct ishtp_cl_device *dev);
+ const struct dev_pm_ops *pm;
+};
+
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+ struct module *owner);
+void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
+int ishtp_register_event_cb(struct ishtp_cl_device *device,
+ void (*read_cb)(struct ishtp_cl_device *));
+
/* Get the device * from ishtp device instance */
struct device *ishtp_device(struct ishtp_cl_device *cl_device);
/* Trace interface for clients */
--
2.17.2
^ permalink raw reply related
* [PATCH 3/9] HID: intel-ish-hid: Simplify ishtp_cl_link()
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
All callers will only use ISHTP_HOST_CLIENT_ID_ANY, so get rid of
option to pass this additional id.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 2 +-
drivers/hid/intel-ish-hid/ishtp/client.c | 14 ++++----------
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index ffed7c91bebf..287d71338b91 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -637,7 +637,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
hid_ishtp_trace(client_data, "%s reset flag: %d\n", __func__, reset);
- rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
+ rv = ishtp_cl_link(hid_ishtp_cl);
if (rv) {
dev_err(cl_data_to_dev(client_data),
"ishtp_cl_link failed\n");
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 760e48a368a8..657b46dcefa6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -168,9 +168,6 @@ EXPORT_SYMBOL(ishtp_cl_free);
/**
* ishtp_cl_link() - Reserve a host id and link the client instance
* @cl: client device instance
- * @id: host client id to use. It can be ISHTP_HOST_CLIENT_ID_ANY if any
- * id from the available can be used
- *
*
* This allocates a single bit in the hostmap. This function will make sure
* that not many client sessions are opened at the same time. Once allocated
@@ -179,11 +176,11 @@ EXPORT_SYMBOL(ishtp_cl_free);
*
* Return: 0 or error code on failure
*/
-int ishtp_cl_link(struct ishtp_cl *cl, int id)
+int ishtp_cl_link(struct ishtp_cl *cl)
{
struct ishtp_device *dev;
- unsigned long flags, flags_cl;
- int ret = 0;
+ unsigned long flags, flags_cl;
+ int id, ret = 0;
if (WARN_ON(!cl || !cl->dev))
return -EINVAL;
@@ -197,10 +194,7 @@ int ishtp_cl_link(struct ishtp_cl *cl, int id)
goto unlock_dev;
}
- /* If Id is not assigned get one*/
- if (id == ISHTP_HOST_CLIENT_ID_ANY)
- id = find_first_zero_bit(dev->host_clients_map,
- ISHTP_CLIENTS_MAX);
+ id = find_first_zero_bit(dev->host_clients_map, ISHTP_CLIENTS_MAX);
if (id >= ISHTP_CLIENTS_MAX) {
spin_unlock_irqrestore(&dev->device_lock, flags);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index 992625891a6c..afa8b1f521d0 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -172,7 +172,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
/* exported functions from ISHTP under client management scope */
struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
void ishtp_cl_free(struct ishtp_cl *cl);
-int ishtp_cl_link(struct ishtp_cl *cl, int id);
+int ishtp_cl_link(struct ishtp_cl *cl);
void ishtp_cl_unlink(struct ishtp_cl *cl);
int ishtp_cl_disconnect(struct ishtp_cl *cl);
int ishtp_cl_connect(struct ishtp_cl *cl);
--
2.17.2
^ permalink raw reply related
* [PATCH 2/9] HID: intel-ish-hid: Hide members of struct ishtp_cl_device
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
ISH clients don't need to access any field of struct ishtp_cl_device. To
avoid this create an interface functions instead where it is required.
In the case of ishtp_cl_allocate(), modify the parameters so that the
clients don't have to dereference.
Clients can also use tracing, here a new interface is added to get the
common trace function pointer, instead of direct call.
The new interface functions defined in one external header file, named
intel-ish-client-if.h. This is the only header files all ISHTP clients
must include.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 56 +++++++++++---------
drivers/hid/intel-ish-hid/ishtp-hid.c | 4 +-
drivers/hid/intel-ish-hid/ishtp-hid.h | 2 +-
drivers/hid/intel-ish-hid/ishtp/bus.c | 27 ++++++++++
drivers/hid/intel-ish-hid/ishtp/client.c | 4 +-
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
include/linux/intel-ish-client-if.h | 18 +++++++
7 files changed, 84 insertions(+), 29 deletions(-)
create mode 100644 include/linux/intel-ish-client-if.h
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index fbb9d85ba6df..ffed7c91bebf 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/hid.h>
+#include <linux/intel-ish-client-if.h>
#include <linux/sched.h>
#include "ishtp/ishtp-dev.h"
#include "ishtp/client.h"
@@ -24,6 +25,8 @@
#define HID_CL_RX_RING_SIZE 32
#define HID_CL_TX_RING_SIZE 16
+#define cl_data_to_dev(client_data) ishtp_device(client_data->cl_device)
+
/**
* report_bad_packets() - Report bad packets
* @hid_ishtp_cl: Client instance to get stats
@@ -39,7 +42,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
struct hostif_msg *recv_msg = recv_buf;
struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
- dev_err(&client_data->cl_device->dev, "[hid-ish]: BAD packet %02X\n"
+ dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
"total_bad=%u cur_pos=%u\n"
"[%02X %02X %02X %02X]\n"
"payload_len=%u\n"
@@ -83,7 +86,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
do {
if (cur_pos + sizeof(struct hostif_msg) > total_len) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: error, received %u which is less than data header %u\n",
(unsigned int)data_len,
(unsigned int)sizeof(struct hostif_msg_hdr));
@@ -122,12 +125,12 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(
- &client_data->cl_device->dev,
+ cl_data_to_dev(client_data),
client_data->hid_dev_count,
sizeof(struct device_info),
GFP_KERNEL);
if (!client_data->hid_devices) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"Mem alloc failed for hid device info\n");
wake_up_interruptible(&client_data->init_wait);
break;
@@ -135,7 +138,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
for (i = 0; i < client_data->hid_dev_count; ++i) {
if (1 + sizeof(struct device_info) * i >=
payload_len) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: [ENUM_DEVICES]: content size %zu is bigger than payload_len %zu\n",
1 + sizeof(struct device_info)
* i, payload_len);
@@ -170,7 +173,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
}
if (!client_data->hid_descr[curr_hid_dev])
client_data->hid_descr[curr_hid_dev] =
- devm_kmalloc(&client_data->cl_device->dev,
+ devm_kmalloc(cl_data_to_dev(client_data),
payload_len, GFP_KERNEL);
if (client_data->hid_descr[curr_hid_dev]) {
memcpy(client_data->hid_descr[curr_hid_dev],
@@ -195,7 +198,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
}
if (!client_data->report_descr[curr_hid_dev])
client_data->report_descr[curr_hid_dev] =
- devm_kmalloc(&client_data->cl_device->dev,
+ devm_kmalloc(cl_data_to_dev(client_data),
payload_len, GFP_KERNEL);
if (client_data->report_descr[curr_hid_dev]) {
memcpy(client_data->report_descr[curr_hid_dev],
@@ -501,12 +504,12 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
sizeof(struct hostif_msg));
}
if (!client_data->enum_devices_done) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: timed out waiting for enum_devices\n");
return -ETIMEDOUT;
}
if (!client_data->hid_devices) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: failed to allocate HID dev structures\n");
return -ENOMEM;
}
@@ -549,13 +552,13 @@ static int ishtp_get_hid_descriptor(struct ishtp_cl *hid_ishtp_cl, int index)
client_data->hid_descr_done,
3 * HZ);
if (!client_data->hid_descr_done) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: timed out for hid_descr_done\n");
return -EIO;
}
if (!client_data->hid_descr[index]) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: allocation HID desc fail\n");
return -ENOMEM;
}
@@ -596,12 +599,12 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
client_data->report_descr_done,
3 * HZ);
if (!client_data->report_descr_done) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: timed out for report descr\n");
return -EIO;
}
if (!client_data->report_descr[index]) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: failed to alloc report descr\n");
return -ENOMEM;
}
@@ -631,12 +634,12 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
int i;
int rv;
- dev_dbg(&client_data->cl_device->dev, "%s\n", __func__);
+ dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
hid_ishtp_trace(client_data, "%s reset flag: %d\n", __func__, reset);
rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
if (rv) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"ishtp_cl_link failed\n");
return -ENOMEM;
}
@@ -651,7 +654,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
fw_client = ishtp_fw_cl_get_client(dev, &hid_ishtp_guid);
if (!fw_client) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"ish client uuid not found\n");
return -ENOENT;
}
@@ -661,7 +664,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
rv = ishtp_cl_connect(hid_ishtp_cl);
if (rv) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"client connect fail\n");
goto err_cl_unlink;
}
@@ -692,7 +695,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
if (!reset) {
rv = ishtp_hid_probe(i, client_data);
if (rv) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: HID probe for #%u failed: %d\n",
i, rv);
goto err_cl_disconnect;
@@ -748,7 +751,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
hid_ishtp_cl_deinit(hid_ishtp_cl);
- hid_ishtp_cl = ishtp_cl_allocate(cl_device->ishtp_dev);
+ hid_ishtp_cl = ishtp_cl_allocate(cl_device);
if (!hid_ishtp_cl)
return;
@@ -762,15 +765,17 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
rv = hid_ishtp_cl_init(hid_ishtp_cl, 1);
if (!rv)
break;
- dev_err(&client_data->cl_device->dev, "Retry reset init\n");
+ dev_err(cl_data_to_dev(client_data), "Retry reset init\n");
}
if (rv) {
- dev_err(&client_data->cl_device->dev, "Reset Failed\n");
+ dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
hid_ishtp_trace(client_data, "%s Failed hid_ishtp_cl %p\n",
__func__, hid_ishtp_cl);
}
}
+void (*hid_print_trace)(void *dev, const char *format, ...);
+
/**
* hid_ishtp_cl_probe() - ISHTP client driver probe
* @cl_device: ISHTP client device instance
@@ -788,12 +793,13 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
if (!cl_device)
return -ENODEV;
- client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
+ client_data = devm_kzalloc(ishtp_device(cl_device),
+ sizeof(*client_data),
GFP_KERNEL);
if (!client_data)
return -ENOMEM;
- hid_ishtp_cl = ishtp_cl_allocate(cl_device->ishtp_dev);
+ hid_ishtp_cl = ishtp_cl_allocate(cl_device);
if (!hid_ishtp_cl)
return -ENOMEM;
@@ -807,6 +813,8 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
INIT_WORK(&client_data->work, hid_ishtp_cl_reset_handler);
+ hid_print_trace = ishtp_trace_callback(cl_device);
+
rv = hid_ishtp_cl_init(hid_ishtp_cl, 0);
if (rv) {
ishtp_cl_free(hid_ishtp_cl);
@@ -833,7 +841,7 @@ static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
- dev_dbg(&cl_device->dev, "%s\n", __func__);
+ dev_dbg(ishtp_device(cl_device), "%s\n", __func__);
hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
ishtp_cl_disconnect(hid_ishtp_cl);
ishtp_put_device(cl_device);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.c b/drivers/hid/intel-ish-hid/ishtp-hid.c
index bc4c536f3c0d..cb71ad43ea3b 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.c
@@ -14,6 +14,7 @@
*/
#include <linux/hid.h>
+#include <linux/intel-ish-client-if.h>
#include <uapi/linux/input.h>
#include "ishtp/client.h"
#include "ishtp-hid.h"
@@ -204,7 +205,8 @@ int ishtp_hid_probe(unsigned int cur_hid_dev,
hid->ll_driver = &ishtp_hid_ll_driver;
hid->bus = BUS_INTEL_ISHTP;
- hid->dev.parent = &client_data->cl_device->dev;
+ hid->dev.parent = ishtp_device(client_data->cl_device);
+
hid->version = le16_to_cpu(ISH_HID_VERSION);
hid->vendor = le16_to_cpu(client_data->hid_devices[cur_hid_dev].vid);
hid->product = le16_to_cpu(client_data->hid_devices[cur_hid_dev].pid);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h
index 1cd07a441cd4..e1b00f5125c7 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.h
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
@@ -24,7 +24,7 @@
#define IS_RESPONSE 0x80
/* Used to dump to Linux trace buffer, if enabled */
-#define hid_ishtp_trace(client, ...) \
+#define hid_ishtp_trace(client, ...) \
client->cl_device->ishtp_dev->print_log(\
client->cl_device->ishtp_dev, __VA_ARGS__)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 6348fee8aadc..308853eab91c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -827,6 +827,33 @@ int ishtp_use_dma_transfer(void)
return ishtp_use_dma;
}
+/**
+ * ishtp_device() - Return device pointer
+ *
+ * This interface is used to return device pointer from ishtp_cl_device
+ * instance.
+ *
+ * Return: device *.
+ */
+struct device *ishtp_device(struct ishtp_cl_device *device)
+{
+ return &device->dev;
+}
+EXPORT_SYMBOL(ishtp_device);
+
+/**
+ * ishtp_trace_callback() - Return trace callback
+ *
+ * This interface is used to return trace callback function pointer.
+ *
+ * Return: void *.
+ */
+void *ishtp_trace_callback(struct ishtp_cl_device *cl_device)
+{
+ return cl_device->ishtp_dev->print_log;
+}
+EXPORT_SYMBOL(ishtp_trace_callback);
+
/**
* ishtp_bus_register() - Function to register bus
*
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index faeccdb1475b..760e48a368a8 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -126,7 +126,7 @@ static void ishtp_cl_init(struct ishtp_cl *cl, struct ishtp_device *dev)
*
* Return: The allocated client instance or NULL on failure
*/
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev)
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device)
{
struct ishtp_cl *cl;
@@ -134,7 +134,7 @@ struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev)
if (!cl)
return NULL;
- ishtp_cl_init(cl, dev);
+ ishtp_cl_init(cl, cl_device->ishtp_dev);
return cl;
}
EXPORT_SYMBOL(ishtp_cl_allocate);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index e0df3eb611e6..992625891a6c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -170,7 +170,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
}
/* exported functions from ISHTP under client management scope */
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev);
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
void ishtp_cl_free(struct ishtp_cl *cl);
int ishtp_cl_link(struct ishtp_cl *cl, int id);
void ishtp_cl_unlink(struct ishtp_cl *cl);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
new file mode 100644
index 000000000000..11e285172735
--- /dev/null
+++ b/include/linux/intel-ish-client-if.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel ISH client Interface definitions
+ *
+ * Copyright (c) 2019, Intel Corporation.
+ */
+
+#ifndef _INTEL_ISH_CLIENT_IF_H_
+#define _INTEL_ISH_CLIENT_IF_H_
+
+struct ishtp_cl_device;
+
+/* Get the device * from ishtp device instance */
+struct device *ishtp_device(struct ishtp_cl_device *cl_device);
+/* Trace interface for clients */
+void *ishtp_trace_callback(struct ishtp_cl_device *cl_device);
+
+#endif /* _INTEL_ISH_CLIENT_IF_H_ */
--
2.17.2
^ permalink raw reply related
* [PATCH 1/9] HID: intel-ish-hid: Add match callback to ishtp bus type
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Hong Liu
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
From: Hong Liu <hong.liu@intel.com>
Currently we depend on the guid check in ishtp_cl_driver.probe to match
the device and driver. However Linux device core first calls the match()
callback to decide the matching of driver and device, and then does some
preparation before calling the driver probe function. If we return error
in the driver probe, it needs to tear down all the preparation work and
retry with next driver.
Adding the match callback can avoid the unnecessary entry into unmatched
driver probe function for ishtp clients reported by FW.
Signed-off-by: Hong Liu <hong.liu@intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +----
drivers/hid/intel-ish-hid/ishtp/bus.c | 21 ++++++++++++++++++++
drivers/hid/intel-ish-hid/ishtp/bus.h | 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 30fe0c5e6fad..fbb9d85ba6df 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -788,10 +788,6 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
if (!cl_device)
return -ENODEV;
- if (!guid_equal(&hid_ishtp_guid,
- &cl_device->fw_client->props.protocol_name))
- return -ENODEV;
-
client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
GFP_KERNEL);
if (!client_data)
@@ -922,6 +918,7 @@ static const struct dev_pm_ops hid_ishtp_pm_ops = {
static struct ishtp_cl_driver hid_ishtp_cl_driver = {
.name = "ish-hid",
+ .guid = &hid_ishtp_guid,
.probe = hid_ishtp_cl_probe,
.remove = hid_ishtp_cl_remove,
.reset = hid_ishtp_cl_reset,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index d5f4b6438d86..6348fee8aadc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -219,6 +219,26 @@ static int ishtp_cl_device_probe(struct device *dev)
return driver->probe(device);
}
+/**
+ * ishtp_cl_bus_match() - Bus match() callback
+ * @dev: the device structure
+ * @drv: the driver structure
+ *
+ * This is a bus match callback, called when a new ishtp_cl_device is
+ * registered during ishtp bus client enumeration. Use the guid_t in
+ * drv and dev to decide whether they match or not.
+ *
+ * Return: 1 if dev & drv matches, 0 otherwise.
+ */
+static int ishtp_cl_bus_match(struct device *dev, struct device_driver *drv)
+{
+ struct ishtp_cl_device *device = to_ishtp_cl_device(dev);
+ struct ishtp_cl_driver *driver = to_ishtp_cl_driver(drv);
+
+ return guid_equal(driver->guid,
+ &device->fw_client->props.protocol_name);
+}
+
/**
* ishtp_cl_device_remove() - Bus remove() callback
* @dev: the device structure
@@ -372,6 +392,7 @@ static struct bus_type ishtp_cl_bus_type = {
.name = "ishtp",
.dev_groups = ishtp_cl_dev_groups,
.probe = ishtp_cl_device_probe,
+ .match = ishtp_cl_bus_match,
.remove = ishtp_cl_device_remove,
.pm = &ishtp_cl_bus_dev_pm_ops,
.uevent = ishtp_cl_uevent,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index 4cf7ad586c37..c96e7fb42f01 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -64,6 +64,7 @@ struct ishtp_cl_device {
struct ishtp_cl_driver {
struct device_driver driver;
const char *name;
+ const guid_t *guid;
int (*probe)(struct ishtp_cl_device *dev);
int (*remove)(struct ishtp_cl_device *dev);
int (*reset)(struct ishtp_cl_device *dev);
--
2.17.2
^ permalink raw reply related
* [PATCH 0/9] HID: intel-ish-hid: Clean up external interfaces
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
FOR kernel v5.2+.
No functional changes are expected with this series. I am posting this now
because of usage of ISH in ChromeOS Embedded Controller.
https://lkml.org/lkml/2019/2/24/26
I want to make sure that API is restricted before more development and posting
there.
Currently only one ISH client is using ISH transport. But it is changing now
with the development of other clients using ISH transport. Some of these
clients which are targeted for Linux based OS only laptops, are not using
HID to export sensors. As more clients are getting developed it is important
that the external interface for ISH transport only allows what clients need.
Currently the header files used by clients "client.h" and "ishtp-dev.h"
include other ISH header files. Also clients access fields from structure
which also has other fields which are only used by ISH transort.
So this series introduces one header file "linux/intel-ish-client-if.h".
This header files doesn't include any other ISH transport header files.
There are interface functions defined so that clients never have to directly
access any ISH transort structures.
Also clients don't have to match there GUID in probe. They will be only
probbed if their GUID matches, which is passed as driver registry.
Hong Liu (1):
HID: intel-ish-hid: Add match callback to ishtp bus type
Srinivas Pandruvada (8):
HID: intel-ish-hid: Hide members of struct ishtp_cl_device
HID: intel-ish-hid: Simplify ishtp_cl_link()
HID: intel-ish-hid: Move driver registry functions
HID: intel-ish-hid: Store ishtp_cl_device instance in device
HID: intel-ish-hid: Move the common functions from client.h
HID: intel-ish-hid: Add interface functions for struct ishtp_cl
HID: intel-ish-hid: Move functions related to bus and device
HID: intel-ish-hid: Use the new interface functions in HID ish client
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 131 ++++++++++---------
drivers/hid/intel-ish-hid/ishtp-hid.c | 6 +-
drivers/hid/intel-ish-hid/ishtp-hid.h | 6 +-
drivers/hid/intel-ish-hid/ishtp/bus.c | 83 +++++++++++-
drivers/hid/intel-ish-hid/ishtp/bus.h | 37 +-----
drivers/hid/intel-ish-hid/ishtp/client.c | 60 +++++++--
drivers/hid/intel-ish-hid/ishtp/client.h | 24 ----
drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 31 -----
include/linux/intel-ish-client-if.h | 110 ++++++++++++++++
9 files changed, 310 insertions(+), 178 deletions(-)
create mode 100644 include/linux/intel-ish-client-if.h
--
2.17.2
^ permalink raw reply
* Re: [PATCH v2] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: Hans de Goede @ 2019-03-03 13:59 UTC (permalink / raw)
To: Christian Oder
Cc: andy, dvhart, linux-input, linux-kernel, platform-driver-x86
In-Reply-To: <20190303134727.6680-1-me@myself5.de>
Hi Christian,
On 03-03-19 14:47, Christian Oder wrote:
> Add touchscreen info for the CHUWUI Hi10 Air tablet.
>
> Signed-off-by: Christian Oder <me@myself5.de>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - added review tag
A quick note for any patches you may submit in the future, there
is no need to send a v2 just to add someones Reviewed-by or Acked-by,
these will be picked up automatically by the subsys-maintainer when
they add the patch to their tree.
Regards,
Hans
>
> drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index 167a156e3cc7..2d56ff7c8230 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
> .properties = chuwi_hi8_pro_props,
> };
>
> +static const struct property_entry chuwi_hi10_air_props[] = {
> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
> + PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
> + PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
> + PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
> + PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> + PROPERTY_ENTRY_BOOL("silead,home-button"),
> + { }
> +};
> +
> +static const struct ts_dmi_data chuwi_hi10_air_data = {
> + .acpi_name = "MSSL1680:00",
> + .properties = chuwi_hi10_air_props,
> +};
> +
> static const struct property_entry chuwi_vi8_props[] = {
> PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
> PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
> @@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
> },
> },
> + {
> + /* Chuwi Hi10 Air */
> + .driver_data = (void *)&chuwi_hi10_air_data,
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
> + },
> + },
> {
> /* Chuwi Vi8 (CWI506) */
> .driver_data = (void *)&chuwi_vi8_data,
>
^ permalink raw reply
* [PATCH v2] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: Christian Oder @ 2019-03-03 13:47 UTC (permalink / raw)
To: hdegoede; +Cc: andy, dvhart, linux-input, linux-kernel, me, platform-driver-x86
In-Reply-To: <c6b0c2ef-eed9-ffae-e974-92260d5356aa@redhat.com>
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder <me@myself5.de>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- added review tag
drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: Hans de Goede @ 2019-03-03 10:55 UTC (permalink / raw)
To: me
Cc: Darren Hart, Andy Shevchenko, linux-input, platform-driver-x86,
linux-kernel
In-Reply-To: <20190301164002.6031-1-me@myself5.de>
Hi,
On 01-03-19 17:40, me@myself5.de wrote:
> From: Christian Oder <me@myself5.de>
>
> Add touchscreen info for the CHUWUI Hi10 Air tablet.
>
> Signed-off-by: Christian Oder <me@myself5.de>
Thank you for the patch, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index 167a156e3cc7..2d56ff7c8230 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
> .properties = chuwi_hi8_pro_props,
> };
>
> +static const struct property_entry chuwi_hi10_air_props[] = {
> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
> + PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
> + PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
> + PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
> + PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> + PROPERTY_ENTRY_BOOL("silead,home-button"),
> + { }
> +};
> +
> +static const struct ts_dmi_data chuwi_hi10_air_data = {
> + .acpi_name = "MSSL1680:00",
> + .properties = chuwi_hi10_air_props,
> +};
> +
> static const struct property_entry chuwi_vi8_props[] = {
> PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
> PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
> @@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
> },
> },
> + {
> + /* Chuwi Hi10 Air */
> + .driver_data = (void *)&chuwi_hi10_air_data,
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
> + },
> + },
> {
> /* Chuwi Vi8 (CWI506) */
> .driver_data = (void *)&chuwi_vi8_data,
>
^ permalink raw reply
* [PATCH][next] HID: uclogic: remove redudant duplicated null check on ver_ptr
From: Colin King @ 2019-03-02 22:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently ver_ptr is being null checked twice, once before calling
usb_string and once afterwards. The second null check is redundant
and can be removed, remove it.
Detected by CoverityScan, CID#1477308 ("Logically dead code")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/hid/hid-uclogic-params.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 7710d9f957da..0187c9f8fc22 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -735,10 +735,6 @@ static int uclogic_params_huion_init(struct uclogic_params *params,
goto cleanup;
}
rc = usb_string(udev, 201, ver_ptr, ver_len);
- if (ver_ptr == NULL) {
- rc = -ENOMEM;
- goto cleanup;
- }
if (rc == -EPIPE) {
*ver_ptr = '\0';
} else if (rc < 0) {
--
2.20.1
^ permalink raw reply related
* [PATCH 2/2] Input: add a driver for GPIO controllable vibrators
From: Luca Weiss @ 2019-03-02 14:11 UTC (permalink / raw)
Cc: Luca Weiss, Dmitry Torokhov, Xiaotong Lu, Coly Li, Stephen Boyd,
Lucas Stach, Mauro Carvalho Chehab, Andrey Smirnov, Arnd Bergmann,
Aaron Wu, Rob Herring, open list,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...
In-Reply-To: <20190302141132.21160-1-luca@z3ntu.xyz>
Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
drivers/input/misc/Kconfig | 12 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
3 files changed, 227 insertions(+)
create mode 100644 drivers/input/misc/gpio-vibra.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ca59a2be9bc5..77480268fef2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -280,6 +280,18 @@ config INPUT_GPIO_DECODER
To compile this driver as a module, choose M here: the module
will be called gpio_decoder.
+config INPUT_GPIO_VIBRA
+ tristate "GPIO vibrator support"
+ depends on GPIOLIB || COMPILE_TEST
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here to get support for GPIO based vibrator devices.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the module will be
+ called gpio-vibra.
+
config INPUT_IXP4XX_BEEPER
tristate "IXP4XX Beeper support"
depends on ARCH_IXP4XX
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 9d0f9d1ff68f..79edbad44cf3 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS) += drv2667.o
obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
+obj-$(CONFIG_INPUT_GPIO_VIBRA) += gpio-vibra.o
obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
new file mode 100644
index 000000000000..14f9534668c8
--- /dev/null
+++ b/drivers/input/misc/gpio-vibra.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * GPIO vibrator driver
+ *
+ * Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
+ *
+ * Based on PWM vibrator driver:
+ * Copyright (C) 2017 Collabora Ltd.
+ *
+ * Based on previous work from:
+ * Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
+ *
+ * Based on PWM beeper driver:
+ * Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+enum vibrator_state {
+ VIBRATOR_OFF,
+ VIBRATOR_ON
+};
+
+struct gpio_vibrator {
+ struct input_dev *input;
+ struct gpio_desc *gpio;
+ struct regulator *vcc;
+
+ struct work_struct play_work;
+ enum vibrator_state state;
+ bool vcc_on;
+};
+
+static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
+{
+ struct device *pdev = vibrator->input->dev.parent;
+ int err;
+
+ if (!vibrator->vcc_on) {
+ err = regulator_enable(vibrator->vcc);
+ if (err) {
+ dev_err(pdev, "failed to enable regulator: %d", err);
+ return err;
+ }
+ vibrator->vcc_on = true;
+ }
+
+ gpiod_set_value(vibrator->gpio, 1);
+
+ return 0;
+}
+
+static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
+{
+ gpiod_set_value(vibrator->gpio, 0);
+
+ if (vibrator->vcc_on) {
+ regulator_disable(vibrator->vcc);
+ vibrator->vcc_on = false;
+ }
+}
+
+static void gpio_vibrator_play_work(struct work_struct *work)
+{
+ struct gpio_vibrator *vibrator = container_of(work,
+ struct gpio_vibrator, play_work);
+
+ if (vibrator->state == VIBRATOR_ON)
+ gpio_vibrator_start(vibrator);
+ else
+ gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct gpio_vibrator *vibrator = input_get_drvdata(dev);
+
+ int level = effect->u.rumble.strong_magnitude;
+
+ if (!level)
+ level = effect->u.rumble.weak_magnitude;
+
+ if (level)
+ vibrator->state = VIBRATOR_ON;
+ else
+ vibrator->state = VIBRATOR_OFF;
+
+ schedule_work(&vibrator->play_work);
+
+ return 0;
+}
+
+static void gpio_vibrator_close(struct input_dev *input)
+{
+ struct gpio_vibrator *vibrator = input_get_drvdata(input);
+
+ cancel_work_sync(&vibrator->play_work);
+ gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_probe(struct platform_device *pdev)
+{
+ struct gpio_vibrator *vibrator;
+ int err;
+
+ vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
+ if (!vibrator)
+ return -ENOMEM;
+
+ vibrator->input = devm_input_allocate_device(&pdev->dev);
+ if (!vibrator->input)
+ return -ENOMEM;
+
+ vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
+ err = PTR_ERR_OR_ZERO(vibrator->vcc);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to request regulator: %d",
+ err);
+ return err;
+ }
+
+ vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
+ err = PTR_ERR_OR_ZERO(vibrator->gpio);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to request main gpio: %d",
+ err);
+ return err;
+ }
+
+ INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
+
+ vibrator->input->name = "gpio-vibrator";
+ vibrator->input->id.bustype = BUS_HOST;
+ vibrator->input->dev.parent = &pdev->dev;
+ vibrator->input->close = gpio_vibrator_close;
+
+ input_set_drvdata(vibrator->input, vibrator);
+ input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
+
+ err = input_ff_create_memless(vibrator->input, NULL,
+ gpio_vibrator_play_effect);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
+ return err;
+ }
+
+ err = input_register_device(vibrator->input);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't register input dev: %d", err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, vibrator);
+
+ return 0;
+}
+
+static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
+{
+ struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+ cancel_work_sync(&vibrator->play_work);
+ if (vibrator->state == VIBRATOR_ON)
+ gpio_vibrator_stop(vibrator);
+
+ return 0;
+}
+
+static int __maybe_unused gpio_vibrator_resume(struct device *dev)
+{
+ struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+ if (vibrator->state == VIBRATOR_ON)
+ gpio_vibrator_start(vibrator);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
+ gpio_vibrator_suspend, gpio_vibrator_resume);
+
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_vibra_dt_match_table[] = {
+ { .compatible = "gpio-vibrator" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
+#endif
+
+static struct platform_driver gpio_vibrator_driver = {
+ .probe = gpio_vibrator_probe,
+ .driver = {
+ .name = "gpio-vibrator",
+ .pm = &gpio_vibrator_pm_ops,
+ .of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
+ },
+};
+module_platform_driver(gpio_vibrator_driver);
+
+MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
+MODULE_DESCRIPTION("GPIO vibrator driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio-vibrator");
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: input: add GPIO controllable vibrator
From: Luca Weiss @ 2019-03-02 14:11 UTC (permalink / raw)
Cc: Luca Weiss, Dmitry Torokhov, Rob Herring, Mark Rutland,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
.../devicetree/bindings/input/gpio-vibrator.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-vibrator.txt
diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.txt b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
new file mode 100644
index 000000000000..9e2e9acf497b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
@@ -0,0 +1,16 @@
+* GPIO vibrator device tree bindings
+
+Registers a GPIO device as vibrator, where the vibration motor just has the capability to turn on or off. If the device is connected to a pwm, you should use the pwm-vibrator driver instead.
+
+Required properties:
+- compatible: should contain "gpio-vibrator"
+- enable-gpios: Should contain a GPIO handle
+- vcc-supply: Phandle for the regulator supplying power
+
+Example from Fairphone 2:
+
+vibrator {
+ compatible = "gpio-vibrator";
+ enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+ vcc-supply = <&pm8941_l18>;
+};
--
2.21.0
^ permalink raw reply related
* [PATCH] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: me @ 2019-03-01 16:40 UTC (permalink / raw)
Cc: Christian Oder, Hans de Goede, Darren Hart, Andy Shevchenko,
linux-input, platform-driver-x86, linux-kernel
From: Christian Oder <me@myself5.de>
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder <me@myself5.de>
---
drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
--
2.20.1
^ permalink raw reply related
* Re: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Benjamin Tissoires @ 2019-03-01 9:48 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Junge, Terry, Jiri Kosina, oneukum@suse.de,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <a6ecd59f99aa64355ac7c176aa4310d32ce8969e.camel@suse.de>
On Thu, Feb 28, 2019 at 7:01 PM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Thu, 2019-02-28 at 17:02 +0000, Junge, Terry wrote:
> > This could also be a parser error. In the HID specification section 6.2.2.8 it
> > states that the last declared Usage Page is applied to Usages when the Main
> > item is encountered.
> >
> > "If the bSize field = 1 or 2 then the Usage is interpreted as an unsigned
> > value
> > that selects a Usage ID on the currently defined Usage Page. When the parser
> > encounters a main item it concatenates the last declared Usage Page with a
> > Usage to form a complete usage value. Extended usages can be used to
> > override the currently defined Usage Page for individual usages."
> >
>
> Hi Terry, thanks for the comment.
> Just for the record the paragraph I cited on my patch is the following:
>
> 6.2.2.7 Global Items
>
> [...]
>
> Usage Page: Unsigned integer specifying the current Usage Page. Since a
> usage are 32 bit values, Usage Page items can be used to conserve space
> in a report descriptor by setting the high order 16 bits of a
> subsequent usages. Any usage that follows which is defines* 16 bits or
> less is interpreted as a Usage ID and concatenated with the Usage Page
> to form a 32 bit Usage.
>
> * This is a spec errata, I belive it should say "defined"
>
> As you can see they use the word "follows" which in my opinion contradicts the
> paragraph you pointed out. That said I may be wrong, I'm not too good at
> reading specs :).
I think you are both right (that is some decision making skills :-P).
I never saw a case where the Usage Page was after the Usage. And I
would lean towards Nicolas interpretation.
However, Terry's point is valid too, and by re-reading the two
paragraphs, one could argue that the "follows" of 6.2.2.7 could be
applied when the Main item is processed as in 6.2.2.8.
So I am going to base my decision on the "reference" driver. How well
behaves Windows with this particular keyboard?
If it behaves properly, then we better use the 6.2.2.8 version where
the Usage Page is only appended when there is a Main item. This means
we need to remember what size was the last Usage (and Min/Max), to
know if we should concatenate the 2.
Note that for every change that involves the generic parser, I'd like
a few tests added to `tests/test_keyboard.py in
https://gitlab.freedesktop.org/libevdev/hid-tools
Also note that the HID parser implementation in hid-tools follows the
kernel, so we might need to adjust it there too if we want to add high
level events. If you directly call `.call_input_event()` with raw
events, it should be fine.
>
> I checked the HID parser and it's indeed written assuming local items are
> preceded by a Usage Page. I'd be glad to fix it there, but it would be nice to
> have the mantainer's opinion on the matter first.
>
You've now got the opinion of one of the 2 maintainers, hope this helps :)
Cheers,
Benjamin
^ permalink raw reply
* [PATCH] Input: fix oops in input_to_handler
From: Zhipeng Xie @ 2019-03-01 4:13 UTC (permalink / raw)
To: dmitry.torokhov, rydberg
Cc: linux-input, linux-kernel, kernel.openeuler, euleros,
huawei.libin, yangyingliang, xiezhipeng1
we got the following boot crash:
[ 36.086344] Internal error: Oops: 96000004 [#1] SMP
[ 36.091371] CPU: 32 PID: 0 Comm: swapper/32 Tainted: G OE 4.19.25-vhulk1901.1.0.h111.aarch64 #1
[ 36.101444] Hardware name: Huawei TaiShan 2280 /BC11SPCD, BIOS 1.58 10/24/2018
[ 36.108860] pstate: 20000085 (nzCv daIf -PAN -UAO)
[ 36.113727] pc : input_to_handler+0x2c/0x148
[ 36.118058] lr : input_pass_values.part.2+0x148/0x168
[ 36.123177] sp : ffff000100103ba0
[ 36.126535] x29: ffff000100103ba0 x28: ffff801fb2ac5958
[ 36.131924] x27: ffff000009799000 x26: 0000000000000001
[ 36.137375] x25: 0000000000000000 x24: ffff801fb5b57c00
[ 36.142822] x23: 000000020987f3d0 x22: ffff801faf427e00
[ 36.148211] x21: 0000000000000003 x20: 0000000000000003
[ 36.153599] x19: ffff801faf427e00 x18: 000000000000000e
[ 36.158986] x17: 000000000000000e x16: 0000000000000007
[ 36.164374] x15: 0000000000000001 x14: 0000000000000019
[ 36.169762] x13: 0000000000000033 x12: 000000000000004c
[ 36.175150] x11: 0000000000000068 x10: ffff000008dfa290
[ 36.180538] x9 : 000000000000007d x8 : 0000000000000000
[ 36.185925] x7 : 0000000000000053 x6 : 0000000000000000
[ 36.191313] x5 : 0000000000000000 x4 : 0000000000000000
[ 36.196700] x3 : 0000000000000010 x2 : 0000000000000003
[ 36.202088] x1 : ffff801fb5b57c00 x0 : ffff000008a120a0
[ 36.207477] Process swapper/32 (pid: 0, stack limit = 0x0000000032f86b58)
[ 36.214361] Call trace:
[ 36.216840] input_to_handler+0x2c/0x148
[ 36.220816] input_pass_values.part.2+0x148/0x168
[ 36.225582] input_handle_event+0x130/0x5b8
[ 36.229823] i6.242013] hid_input_report+0x128/0x1b0
[ 36.246076] hid_irq_in+0x240/0x298
[ 36.249613] __usb_hcd_giveback_urb+0x9c/0x130
[ 36.257460] usb_giveback_urb_bh+0xf4/0x198
[ 36.265127] tasklet_action_common.isra.6+0x94/0x160
[ 36.273458] tasklet_hi_action+0x2c/0x38
[ 36.280597] __do_softirq+0x118/0x314
[ 36.287494] irq_exit+0xa4/0xe8
[ 36.293682] __handle_domain_irq+0x6c/0xc0
[ 36.300783] gic_handle_irq+0x6c/0x170
[ 36.307296] el1_irq+0xb8/0x140
[ 36.313283] arch_cpu_idle+0x38/0x1c0
[ 36.319776] default_idle_call+0x24/0x44
[ 36.326527] do_idle+0x1ec/0x2d0
[ 36.332503] cpu_startup_entry+0x28/0x30
[ 36.339161] secondary_start_kernel+0x194/0x1d8
We need to check the input_handler before referencing its members.
Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
---
drivers/input/input.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 3304aaa..b768d14 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -100,6 +100,9 @@ static unsigned int input_to_handler(struct input_handle *handle,
struct input_value *end = vals;
struct input_value *v;
+ if (!handler)
+ return 0;
+
if (handler->filter) {
for (v = vals; v != vals + count; v++) {
if (handler->filter(handle, v->type, v->code, v->value))
--
1.7.12.4
^ permalink raw reply related
* Re: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Nicolas Saenz Julienne @ 2019-02-28 18:01 UTC (permalink / raw)
To: Junge, Terry, Jiri Kosina, Benjamin Tissoires
Cc: oneukum@suse.de, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <BYAPR02MB56078ADD429292DF89F8C694ED750@BYAPR02MB5607.namprd02.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]
On Thu, 2019-02-28 at 17:02 +0000, Junge, Terry wrote:
> This could also be a parser error. In the HID specification section 6.2.2.8 it
> states that the last declared Usage Page is applied to Usages when the Main
> item is encountered.
>
> "If the bSize field = 1 or 2 then the Usage is interpreted as an unsigned
> value
> that selects a Usage ID on the currently defined Usage Page. When the parser
> encounters a main item it concatenates the last declared Usage Page with a
> Usage to form a complete usage value. Extended usages can be used to
> override the currently defined Usage Page for individual usages."
>
Hi Terry, thanks for the comment.
Just for the record the paragraph I cited on my patch is the following:
6.2.2.7 Global Items
[...]
Usage Page: Unsigned integer specifying the current Usage Page. Since a
usage are 32 bit values, Usage Page items can be used to conserve space
in a report descriptor by setting the high order 16 bits of a
subsequent usages. Any usage that follows which is defines* 16 bits or
less is interpreted as a Usage ID and concatenated with the Usage Page
to form a 32 bit Usage.
* This is a spec errata, I belive it should say "defined"
As you can see they use the word "follows" which in my opinion contradicts the
paragraph you pointed out. That said I may be wrong, I'm not too good at
reading specs :).
I checked the HID parser and it's indeed written assuming local items are
preceded by a Usage Page. I'd be glad to fix it there, but it would be nice to
have the mantainer's opinion on the matter first.
Regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Junge, Terry @ 2019-02-28 17:02 UTC (permalink / raw)
To: Nicolas Saenz Julienne, Jiri Kosina, Benjamin Tissoires
Cc: oneukum@suse.de, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190228135556.14713-1-nsaenzjulienne@suse.de>
This could also be a parser error. In the HID specification section 6.2.2.8 it states that the last declared Usage Page is applied to Usages when the Main item is encountered.
"If the bSize field = 1 or 2 then the Usage is interpreted as an unsigned value
that selects a Usage ID on the currently defined Usage Page. When the parser
encounters a main item it concatenates the last declared Usage Page with a
Usage to form a complete usage value. Extended usages can be used to
override the currently defined Usage Page for individual usages."
-----Original Message-----
From: linux-input-owner@vger.kernel.org <linux-input-owner@vger.kernel.org> On Behalf Of Nicolas Saenz Julienne
Sent: Thursday, February 28, 2019 5:56 AM
To: Jiri Kosina <jikos@kernel.org>; Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: oneukum@suse.de; Nicolas Saenz Julienne <nsaenzjulienne@suse.de>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
A whole set of Primax manufactured wireless keyboards won't work out of the box as they provide wrong HID descriptors. In this case the offense being defining the "Usage Maximum/Minimum" local items before the "Usage Page". This will make the parser use the previous "Usage Page" on those local items, generating a wrong HID report. This is not a parser error as the spec clearly states that "Usage Page" applies to any item that follows it (see 6.2.2.7).
In other words, we get this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
Yet they meant this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
This patch fixes the issue by overwriting the offending report with a correct one.
This was made thanks to the user's answers provided here, also full HID descriptors are available there:
https://unix.stackexchange.com/questions/377830/linux-hid-driver-for-primax-wireless-keyboards/
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
NOTE: This is an RFC as I'm unable to test my fix. Apart from the
devices mentioned in the patch there is a whole list of them, yet I
didn't include them as I was unable to access their HID descriptors.
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-primax.c | 30 +++++++++++++++++++++++++++++- drivers/hid/hid-quirks.c | 3 +++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 24f846d67478..b39f9f36d41f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -714,6 +714,7 @@
#define USB_DEVICE_ID_LENOVO_TPPRODOCK 0x6067
#define USB_DEVICE_ID_LENOVO_X1_COVER 0x6085
#define USB_DEVICE_ID_LENOVO_X1_TAB 0x60a3
+#define USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL 0x609b
#define USB_VENDOR_ID_LG 0x1fd2
#define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
@@ -1225,6 +1226,8 @@
#define USB_DEVICE_ID_PRIMAX_REZEL 0x4e72
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D0F 0x4d0f
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22 0x4e22
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63 0x4e63
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80 0x4e80
#define USB_VENDOR_ID_RISO_KAGAKU 0x1294 /* Riso Kagaku Corp. */
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c index 3a1c3c4c50dc..033f6660e8b6 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -2,9 +2,11 @@
* HID driver for primax and similar keyboards with in-band modifiers
*
* Copyright 2011 Google Inc. All Rights Reserved
+ * Copyright 2019 Nicolas Saenz Julienne
*
- * Author:
+ * Authors:
* Terry Lambert <tlambert@google.com>
+ * Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and @@ -27,6 +29,9 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report, {
int idx = size;
+ if (hid->product != USB_DEVICE_ID_PRIMAX_KEYBOARD)
+ return 0;
+
switch (report->id) {
case 0: /* keyboard input */
/*
@@ -64,8 +69,29 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
return 0;
}
+/*
+ * Some Primax manufactured keyboards set "Usage Minimum/Maximum"
+values before
+ * setting the "Usage Page". This is not supported as per spec 6.2.2.7.
+ */
+static __u8 *px_fixup(struct hid_device *hid, __u8 *rdesc, unsigned int
+*rsize) {
+ __u8 wrong_report[] = { 0x19, 0x00, 0x2a, 0xff, 0x05, 0x07 };
+ __u8 fixed_report[] = { 0x05, 0x07, 0x19, 0x00, 0x2a, 0xff };
+
+ if (*rsize > 57 &&
+ !memcmp(&rdesc[51], wrong_report, ARRAY_SIZE(wrong_report))) {
+ hid_info(hid, "fixing up Primax report descriptor\n");
+ memcpy(&rdesc[51], fixed_report, ARRAY_SIZE(fixed_report));
+ }
+
+ return rdesc;
+}
+
static const struct hid_device_id px_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO,
+USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
{ }
};
MODULE_DEVICE_TABLE(hid, px_devices);
@@ -74,8 +100,10 @@ static struct hid_driver px_driver = {
.name = "primax",
.id_table = px_devices,
.raw_event = px_raw_event,
+ .report_fixup = px_fixup,
};
module_hid_driver(px_driver);
MODULE_AUTHOR("Terry Lambert <tlambert@google.com>");
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 94088c0ed68a..9fd256fea231 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -563,6 +563,9 @@ static const struct hid_device_id hid_have_special_driver[] = { #endif #if IS_ENABLED(CONFIG_HID_PRIMAX)
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO,
+USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
#endif
#if IS_ENABLED(CONFIG_HID_PRODIKEYS)
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
--
2.20.1
^ permalink raw reply
* [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Nicolas Saenz Julienne @ 2019-02-28 13:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: oneukum, Nicolas Saenz Julienne, linux-input, linux-kernel
A whole set of Primax manufactured wireless keyboards won't work out of
the box as they provide wrong HID descriptors. In this case the offense
being defining the "Usage Maximum/Minimum" local items before the "Usage
Page". This will make the parser use the previous "Usage Page" on those
local items, generating a wrong HID report. This is not a parser error
as the spec clearly states that "Usage Page" applies to any item that
follows it (see 6.2.2.7).
In other words, we get this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
Yet they meant this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
This patch fixes the issue by overwriting the offending report with a
correct one.
This was made thanks to the user's answers provided here, also full HID
descriptors are available there:
https://unix.stackexchange.com/questions/377830/linux-hid-driver-for-primax-wireless-keyboards/
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
NOTE: This is an RFC as I'm unable to test my fix. Apart from the
devices mentioned in the patch there is a whole list of them, yet I
didn't include them as I was unable to access their HID descriptors.
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-primax.c | 30 +++++++++++++++++++++++++++++-
drivers/hid/hid-quirks.c | 3 +++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 24f846d67478..b39f9f36d41f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -714,6 +714,7 @@
#define USB_DEVICE_ID_LENOVO_TPPRODOCK 0x6067
#define USB_DEVICE_ID_LENOVO_X1_COVER 0x6085
#define USB_DEVICE_ID_LENOVO_X1_TAB 0x60a3
+#define USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL 0x609b
#define USB_VENDOR_ID_LG 0x1fd2
#define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
@@ -1225,6 +1226,8 @@
#define USB_DEVICE_ID_PRIMAX_REZEL 0x4e72
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D0F 0x4d0f
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22 0x4e22
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63 0x4e63
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80 0x4e80
#define USB_VENDOR_ID_RISO_KAGAKU 0x1294 /* Riso Kagaku Corp. */
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c
index 3a1c3c4c50dc..033f6660e8b6 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -2,9 +2,11 @@
* HID driver for primax and similar keyboards with in-band modifiers
*
* Copyright 2011 Google Inc. All Rights Reserved
+ * Copyright 2019 Nicolas Saenz Julienne
*
- * Author:
+ * Authors:
* Terry Lambert <tlambert@google.com>
+ * Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and
@@ -27,6 +29,9 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
{
int idx = size;
+ if (hid->product != USB_DEVICE_ID_PRIMAX_KEYBOARD)
+ return 0;
+
switch (report->id) {
case 0: /* keyboard input */
/*
@@ -64,8 +69,29 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
return 0;
}
+/*
+ * Some Primax manufactured keyboards set "Usage Minimum/Maximum" values before
+ * setting the "Usage Page". This is not supported as per spec 6.2.2.7.
+ */
+static __u8 *px_fixup(struct hid_device *hid, __u8 *rdesc, unsigned int *rsize)
+{
+ __u8 wrong_report[] = { 0x19, 0x00, 0x2a, 0xff, 0x05, 0x07 };
+ __u8 fixed_report[] = { 0x05, 0x07, 0x19, 0x00, 0x2a, 0xff };
+
+ if (*rsize > 57 &&
+ !memcmp(&rdesc[51], wrong_report, ARRAY_SIZE(wrong_report))) {
+ hid_info(hid, "fixing up Primax report descriptor\n");
+ memcpy(&rdesc[51], fixed_report, ARRAY_SIZE(fixed_report));
+ }
+
+ return rdesc;
+}
+
static const struct hid_device_id px_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
{ }
};
MODULE_DEVICE_TABLE(hid, px_devices);
@@ -74,8 +100,10 @@ static struct hid_driver px_driver = {
.name = "primax",
.id_table = px_devices,
.raw_event = px_raw_event,
+ .report_fixup = px_fixup,
};
module_hid_driver(px_driver);
MODULE_AUTHOR("Terry Lambert <tlambert@google.com>");
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 94088c0ed68a..9fd256fea231 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -563,6 +563,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
#endif
#if IS_ENABLED(CONFIG_HID_PRIMAX)
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
#endif
#if IS_ENABLED(CONFIG_HID_PRODIKEYS)
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] HID: quirks: use correct format chars in dbg_hid
From: Nick Desaulniers @ 2019-02-27 19:42 UTC (permalink / raw)
To: Louis Taylor
Cc: Jiri Kosina, benjamin.tissoires, linux-input, LKML,
clang-built-linux
In-Reply-To: <20190227110720.27329-1-louis@kragniz.eu>
On Wed, Feb 27, 2019 at 3:08 AM Louis Taylor <louis@kragniz.eu> wrote:
>
> When building with -Wformat, clang warns:
>
> drivers/hid/hid-quirks.c:1075:27: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->driver_data, bl_entry->vendor,
> ^~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1076:4: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->product);
> ^~~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:12: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:26: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> 4 warnings generated.
>
> This patch fixes the format strings to use the correct format type for unsigned
> ints.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Signed-off-by: Louis Taylor <louis@kragniz.eu>
Thanks for following up on the feedback.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> v2: change format string to use %04x instead of %x
>
> drivers/hid/hid-quirks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 94088c0ed68a..b608a57b7908 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -1071,7 +1071,7 @@ static struct hid_device_id *hid_exists_dquirk(const struct hid_device *hdev)
> }
>
> if (bl_entry != NULL)
> - dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%04x:0x%04x\n",
> bl_entry->driver_data, bl_entry->vendor,
> bl_entry->product);
>
> @@ -1238,7 +1238,7 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev)
> quirks |= bl_entry->driver_data;
>
> if (quirks)
> - dbg_hid("Found squirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found squirk 0x%lx for HID device 0x%04x:0x%04x\n",
> quirks, hdev->vendor, hdev->product);
> return quirks;
> }
> --
> 2.20.1
>
--
Thanks,
~Nick Desaulniers
^ 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