* [PATCH 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
From: Caleb Connolly @ 2023-09-30 17:08 UTC (permalink / raw)
To: Dmitry Torokhov, Vincent Huang
Cc: methanal, linux-input, devicetree, phone-devel,
~postmarketos/upstreaming, Caleb Connolly
In-Reply-To: <20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org>
From: methanal <baclofen@tuta.io>
Some replacement displays include third-party touch ICs which do not
expose the function number and the interrupt status in its PDT entries.
OnePlus 6 (original touch IC)
rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2b 22 0d 06 01 01)
OnePlus 6 (aftermarket touch IC)
rmi4_i2c 12-0020: read 6 bytes at 0x00e3: 0 (2c 23 0d 06 00 00)
Introduce a new devicetree property `syna,pdt-desc` which can be used to
provide platform-specific fallback values for users with a replacement
display with an aftermarket touch IC.
Signed-off-by: methanal <baclofen@tuta.io>
[codeflow adjustments, checkpatch fixes, wording]
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
drivers/input/rmi4/rmi_driver.c | 67 ++++++++++++++++++++++++++++++++++++++---
drivers/input/rmi4/rmi_driver.h | 2 ++
include/linux/rmi.h | 3 ++
3 files changed, 67 insertions(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 58bf3dfbf81c..5e1e3d5dd800 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -461,9 +461,10 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
return 0;
}
-static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
- struct pdt_entry *entry, u16 pdt_address)
+static int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
+ struct pdt_scan_state *state, u16 pdt_address)
{
+ const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
u8 buf[RMI_PDT_ENTRY_SIZE];
int error;
@@ -474,6 +475,21 @@ static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
return error;
}
+ if (pdata->pdt_fallback_size > state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1) {
+ /* Use the description bytes from DT */
+ buf[5] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS];
+ buf[4] = pdata->pdt_fallback_desc[state->pdt_count * RMI_OF_PDT_DESC_CELLS + 1];
+
+ error = rmi_read_block(rmi_dev, pdt_address, buf,
+ RMI_PDT_ENTRY_SIZE - 2);
+ if (error) {
+ dev_err(&rmi_dev->dev,
+ "Read PDT entry at %#06x failed, code: %d.\n",
+ pdt_address, error);
+ return error;
+ }
+ }
+
entry->page_start = pdt_address & RMI4_PAGE_MASK;
entry->query_base_addr = buf[0];
entry->command_base_addr = buf[1];
@@ -551,7 +567,7 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int retval;
for (addr = pdt_start; addr >= pdt_end; addr -= RMI_PDT_ENTRY_SIZE) {
- error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, addr);
+ error = rmi_read_pdt_entry(rmi_dev, &pdt_entry, state, addr);
if (error)
return error;
@@ -1028,9 +1044,11 @@ static int rmi_driver_remove(struct device *dev)
}
#ifdef CONFIG_OF
-static int rmi_driver_of_probe(struct device *dev,
+static int rmi_driver_of_probe(struct rmi_device *rmi_dev,
struct rmi_device_platform_data *pdata)
{
+ struct device *dev = rmi_dev->xport->dev;
+ u8 buf[RMI_PDT_ENTRY_SIZE];
int retval;
retval = rmi_of_property_read_u32(dev, &pdata->reset_delay_ms,
@@ -1038,6 +1056,45 @@ static int rmi_driver_of_probe(struct device *dev,
if (retval)
return retval;
+ /*
+ * In some aftermerket touch ICs, the first PDT entry is empty and
+ * the function number register is 0. If so, the platform
+ * may have provided backup PDT entries in the device tree.
+ */
+
+ retval = rmi_read_block(rmi_dev, PDT_START_SCAN_LOCATION,
+ buf, RMI_PDT_ENTRY_SIZE);
+ if (retval) {
+ dev_err(dev, "Read PDT entry at %#06x failed, code: %d.\n",
+ PDT_START_SCAN_LOCATION, retval);
+ return retval;
+ }
+
+ if (!RMI4_END_OF_PDT(buf[5]))
+ return 0;
+
+ pdata->pdt_fallback_size = of_property_count_u8_elems(dev->of_node,
+ "syna,pdt-fallback-desc");
+
+ /* The rmi4 driver would fail later anyway, so just error out now. */
+ if (pdata->pdt_fallback_size == -EINVAL) {
+ dev_err(dev, "First PDT entry is empty and no backup values provided\n");
+ return -EINVAL;
+ }
+
+ if (pdata->pdt_fallback_size < 0) {
+ dev_err(dev, "syna,ptd-desc property was not specified properly: %d\n",
+ pdata->pdt_fallback_size);
+ return pdata->pdt_fallback_size;
+ }
+
+ pdata->pdt_fallback_desc = devm_kzalloc(dev, pdata->pdt_fallback_size, GFP_KERNEL);
+
+ retval = of_property_read_u8_array(dev->of_node, "syna,pdt-fallback-desc",
+ pdata->pdt_fallback_desc, pdata->pdt_fallback_size);
+ if (retval < 0)
+ return retval;
+
return 0;
}
#else
@@ -1163,7 +1220,7 @@ static int rmi_driver_probe(struct device *dev)
pdata = rmi_get_platform_data(rmi_dev);
if (rmi_dev->xport->dev->of_node) {
- retval = rmi_driver_of_probe(rmi_dev->xport->dev, pdata);
+ retval = rmi_driver_of_probe(rmi_dev, pdata);
if (retval)
return retval;
}
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index e1a5412f2f8f..2531c32d6163 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -31,6 +31,8 @@
#define RMI_PDT_FUNCTION_VERSION_MASK 0x60
#define RMI_PDT_INT_SOURCE_COUNT_MASK 0x07
+#define RMI_OF_PDT_DESC_CELLS 2
+
#define PDT_START_SCAN_LOCATION 0x00e9
#define PDT_END_SCAN_LOCATION 0x0005
#define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ab7eea01ab42..974597960b5e 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -214,6 +214,9 @@ struct rmi_device_platform_data {
int reset_delay_ms;
int irq;
+ u8 *pdt_fallback_desc;
+ int pdt_fallback_size;
+
struct rmi_device_platform_data_spi spi_data;
/* function handler pdata */
--
2.42.0
^ permalink raw reply related
* Re: [PATCH v5 RESEND] HID: nintendo: cleanup LED code
From: Daniel Ogorchock @ 2023-09-30 19:52 UTC (permalink / raw)
To: Martino Fontana; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20230924141547.11597-1-tinozzo123@gmail.com>
Hi Martino,
On Sun, Sep 24, 2023 at 10:16 AM Martino Fontana <tinozzo123@gmail.com> wrote:
>
> - Support player LED patterns up to 8 players.
> (Note that the behavior still consinsts in increasing the player number
> every time a controller is connected, never decreasing it. It should be
> as is described in https://bugzilla.kernel.org/show_bug.cgi?id=216225.
> However, any implementation here would stop making sense as soon as a
> non-Nintendo controller is connected, which is why I'm not bothering.)
>
> - Split part of `joycon_home_led_brightness_set` (which is called by hid)
> into `joycon_set_home_led` (which is what actually sets the LEDs), for
> consistency with player LEDs.
>
> - `joycon_player_led_brightness_set` won't try it to "determine which
> player led this is" anymore: it's already looking at every LED
> brightness value.
>
> - Instead of first registering the `led_classdev`, then attempting to set
> the LED and unregistering the `led_classdev` if it fails, first attempt
> to set the LED, then register the `led_classdev` only if it succeeds
> (the class is still filled up in either case).
>
> - If setting the player LEDs fails, still attempt setting the home LED.
> (I don't know there's a third party controller where this may actually
> happen, but who knows...)
>
> - Use `JC_NUM_LEDS` where appropriate instead of 4.
>
> - Print return codes in more places.
>
> - Use spinlock instead of mutex for `input_num`. Copy its value to a local
> variable, so that it can be unlocked immediately.
>
> - `input_num` starts counting from 0
>
> - Less holding of mutexes in general.
>
> Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
> ---
> Changes for v2:
>
> Applied suggestions, commit message explains more stuff, restored `return ret`
> when `devm_led_classdev_register` fails (since all other hid drivers do this).
> If setting LED fails, `hid_warn` now explicitly says "skipping registration".
>
> Changes for v3 and v4:
>
> Fixed errors and warnings from test robot.
>
> Changes for v5:
>
> I thought that when connecting the controller on an actual Nintendo Switch,
> only the nth player LED would turn on, which is true... on Wii and Wii U.
> So I reverted that, and to compensate, now this supports the LED patterns
> up to 8 players.
>
> drivers/hid/hid-nintendo.c | 133 +++++++++++++++++++++----------------
> 1 file changed, 76 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 10468f727..138f154fe 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -410,6 +410,18 @@ static const char * const joycon_player_led_names[] = {
> LED_FUNCTION_PLAYER4,
> };
> #define JC_NUM_LEDS ARRAY_SIZE(joycon_player_led_names)
> +#define JC_NUM_LED_PATTERNS 8
> +/* Taken from https://www.nintendo.com/my/support/qa/detail/33822 */
> +static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS][JC_NUM_LEDS] = {
> + { 1, 0, 0, 0 },
> + { 1, 1, 0, 0 },
> + { 1, 1, 1, 0 },
> + { 1, 1, 1, 1 },
> + { 1, 0, 0, 1 },
> + { 1, 0, 1, 0 },
> + { 1, 0, 1, 1 },
> + { 0, 1, 1, 0 },
> +};
>
> /* Each physical controller is associated with a joycon_ctlr struct */
> struct joycon_ctlr {
> @@ -699,6 +711,25 @@ static int joycon_set_player_leds(struct joycon_ctlr *ctlr, u8 flash, u8 on)
> return joycon_send_subcmd(ctlr, req, 1, HZ/4);
> }
>
> +static int joycon_set_home_led(struct joycon_ctlr *ctlr, enum led_brightness brightness)
> +{
> + struct joycon_subcmd_request *req;
> + u8 buffer[sizeof(*req) + 5] = { 0 };
> + u8 *data;
> +
> + req = (struct joycon_subcmd_request *)buffer;
> + req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
> + data = req->data;
> + data[0] = 0x01;
> + data[1] = brightness << 4;
> + data[2] = brightness | (brightness << 4);
> + data[3] = 0x11;
> + data[4] = 0x11;
> +
> + hid_dbg(ctlr->hdev, "setting home led brightness\n");
> + return joycon_send_subcmd(ctlr, req, 5, HZ/4);
> +}
> +
> static int joycon_request_spi_flash_read(struct joycon_ctlr *ctlr,
> u32 start_addr, u8 size, u8 **reply)
> {
> @@ -1840,6 +1871,7 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
> return 0;
> }
>
> +/* Because the subcommand sets all the leds at once, the brightness argument is ignored */
> static int joycon_player_led_brightness_set(struct led_classdev *led,
> enum led_brightness brightness)
> {
> @@ -1849,7 +1881,6 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
> int val = 0;
> int i;
> int ret;
> - int num;
>
> ctlr = hid_get_drvdata(hdev);
> if (!ctlr) {
> @@ -1857,21 +1888,10 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
> return -ENODEV;
> }
>
> - /* determine which player led this is */
> - for (num = 0; num < JC_NUM_LEDS; num++) {
> - if (&ctlr->leds[num] == led)
> - break;
> - }
> - if (num >= JC_NUM_LEDS)
> - return -EINVAL;
> + for (i = 0; i < JC_NUM_LEDS; i++)
> + val |= ctlr->leds[i].brightness << i;
>
> mutex_lock(&ctlr->output_mutex);
> - for (i = 0; i < JC_NUM_LEDS; i++) {
> - if (i == num)
> - val |= brightness << i;
> - else
> - val |= ctlr->leds[i].brightness << i;
> - }
> ret = joycon_set_player_leds(ctlr, 0, val);
> mutex_unlock(&ctlr->output_mutex);
>
> @@ -1884,9 +1904,6 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
> struct device *dev = led->dev->parent;
> struct hid_device *hdev = to_hid_device(dev);
> struct joycon_ctlr *ctlr;
> - struct joycon_subcmd_request *req;
> - u8 buffer[sizeof(*req) + 5] = { 0 };
> - u8 *data;
> int ret;
>
> ctlr = hid_get_drvdata(hdev);
> @@ -1894,43 +1911,35 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
> hid_err(hdev, "No controller data\n");
> return -ENODEV;
> }
> -
> - req = (struct joycon_subcmd_request *)buffer;
> - req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT;
> - data = req->data;
> - data[0] = 0x01;
> - data[1] = brightness << 4;
> - data[2] = brightness | (brightness << 4);
> - data[3] = 0x11;
> - data[4] = 0x11;
> -
> - hid_dbg(hdev, "setting home led brightness\n");
> mutex_lock(&ctlr->output_mutex);
> - ret = joycon_send_subcmd(ctlr, req, 5, HZ/4);
> + ret = joycon_set_home_led(ctlr, brightness);
> mutex_unlock(&ctlr->output_mutex);
> -
> return ret;
> }
>
> -static DEFINE_MUTEX(joycon_input_num_mutex);
> +static DEFINE_SPINLOCK(joycon_input_num_spinlock);
> static int joycon_leds_create(struct joycon_ctlr *ctlr)
> {
> struct hid_device *hdev = ctlr->hdev;
> struct device *dev = &hdev->dev;
> const char *d_name = dev_name(dev);
> struct led_classdev *led;
> + int led_val = 0;
> char *name;
> - int ret = 0;
> + int ret;
> int i;
> - static int input_num = 1;
> + unsigned long flags;
> + int player_led_pattern;
> + static int input_num;
>
> - /* Set the default controller player leds based on controller number */
> - mutex_lock(&joycon_input_num_mutex);
> - mutex_lock(&ctlr->output_mutex);
> - ret = joycon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
> - if (ret)
> - hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
> - mutex_unlock(&ctlr->output_mutex);
> + /*
> + * Set the player leds based on controller number
> + * Because there is no standard concept of "player number", the pattern
> + * number will simply increase by 1 every time a controller is connected.
> + */
> + spin_lock_irqsave(&joycon_input_num_spinlock, flags);
> + player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
> + spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
>
> /* configure the player LEDs */
> for (i = 0; i < JC_NUM_LEDS; i++) {
> @@ -1938,31 +1947,37 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
> d_name,
> "green",
> joycon_player_led_names[i]);
> - if (!name) {
> - mutex_unlock(&joycon_input_num_mutex);
> + if (!name)
> return -ENOMEM;
> - }
>
> led = &ctlr->leds[i];
> led->name = name;
> - led->brightness = ((i + 1) <= input_num) ? 1 : 0;
> + led->brightness = joycon_player_led_patterns[player_led_pattern][i];
> led->max_brightness = 1;
> led->brightness_set_blocking =
> joycon_player_led_brightness_set;
> led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
>
> + led_val |= joycon_player_led_patterns[player_led_pattern][i] << i;
> + }
> + mutex_lock(&ctlr->output_mutex);
> + ret = joycon_set_player_leds(ctlr, 0, led_val);
> + mutex_unlock(&ctlr->output_mutex);
> + if (ret) {
> + hid_warn(hdev, "Failed to set players LEDs, skipping registration; ret=%d\n", ret);
> + goto home_led;
> + }
> +
> + for (i = 0; i < JC_NUM_LEDS; i++) {
> + led = &ctlr->leds[i];
> ret = devm_led_classdev_register(&hdev->dev, led);
> if (ret) {
> - hid_err(hdev, "Failed registering %s LED\n", led->name);
> - mutex_unlock(&joycon_input_num_mutex);
> + hid_err(hdev, "Failed to register player %d LED; ret=%d\n", i + 1, ret);
> return ret;
> }
> }
>
> - if (++input_num > 4)
> - input_num = 1;
> - mutex_unlock(&joycon_input_num_mutex);
> -
> +home_led:
> /* configure the home LED */
> if (jc_type_has_right(ctlr)) {
> name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
> @@ -1978,16 +1993,20 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
> led->max_brightness = 0xF;
> led->brightness_set_blocking = joycon_home_led_brightness_set;
> led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
> - ret = devm_led_classdev_register(&hdev->dev, led);
> +
> + /* Set the home LED to 0 as default state */
> + mutex_lock(&ctlr->output_mutex);
> + ret = joycon_set_home_led(ctlr, 0);
> + mutex_unlock(&ctlr->output_mutex);
> if (ret) {
> - hid_err(hdev, "Failed registering home led\n");
> - return ret;
> + hid_warn(hdev, "Failed to set home LED, skipping registration; ret=%d\n", ret);
> + return 0;
> }
> - /* Set the home LED to 0 as default state */
> - ret = joycon_home_led_brightness_set(led, 0);
> +
> + ret = devm_led_classdev_register(&hdev->dev, led);
> if (ret) {
> - hid_warn(hdev, "Failed to set home LED default, unregistering home LED");
> - devm_led_classdev_unregister(&hdev->dev, led);
> + hid_err(hdev, "Failed to register home LED; ret=%d\n", ret);
> + return ret;
> }
> }
>
> --
> 2.42.0
>
This looks good to me. Thanks for the patch.
Reviewed-by: Daniel J. Ogorchock <djogorchock@gmail.com>
^ permalink raw reply
* Re: [PATCH v3] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend
From: Daniel Ogorchock @ 2023-09-30 19:55 UTC (permalink / raw)
To: Martino Fontana; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20230924140927.9844-2-tinozzo123@gmail.com>
Hi Martino,
On Sun, Sep 24, 2023 at 10:13 AM Martino Fontana <tinozzo123@gmail.com> wrote:
>
> When suspending the computer, a Switch Pro Controller connected via USB will
> lose its internal status. However, because the USB connection was technically
> never lost, when resuming the computer, the driver will attempt to communicate
> with the controller as if nothing happened (and fail).
> Because of this, the user was forced to manually disconnect the controller
> (or to press the sync button on the controller to power it off), so that it
> can be re-initialized.
>
> With this patch, the controller will be automatically re-initialized after
> resuming from suspend.
>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=216233
>
> Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
>
> ---
> Changes for v2 and v3: Applied suggestions
>
> drivers/hid/hid-nintendo.c | 175 ++++++++++++++++++++++---------------
> 1 file changed, 103 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 250f5d2f8..10468f727 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> struct joycon_input_report *report;
>
> req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
> + mutex_lock(&ctlr->output_mutex);
> ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
> + mutex_unlock(&ctlr->output_mutex);
> if (ret) {
> hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
> return ret;
> @@ -2117,6 +2119,85 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> return 0;
> }
>
> +static int joycon_init(struct hid_device *hdev)
> +{
> + struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> + int ret = 0;
> +
> + mutex_lock(&ctlr->output_mutex);
> + /* if handshake command fails, assume ble pro controller */
> + if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> + !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> + hid_dbg(hdev, "detected USB controller\n");
> + /* set baudrate for improved latency */
> + ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> + if (ret) {
> + hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> + goto out_unlock;
> + }
> + /* handshake */
> + ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> + if (ret) {
> + hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> + goto out_unlock;
> + }
> + /*
> + * Set no timeout (to keep controller in USB mode).
> + * This doesn't send a response, so ignore the timeout.
> + */
> + joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> + } else if (jc_type_is_chrggrip(ctlr)) {
> + hid_err(hdev, "Failed charging grip handshake\n");
> + ret = -ETIMEDOUT;
> + goto out_unlock;
> + }
> +
> + /* get controller calibration data, and parse it */
> + ret = joycon_request_calibration(ctlr);
> + if (ret) {
> + /*
> + * We can function with default calibration, but it may be
> + * inaccurate. Provide a warning, and continue on.
> + */
> + hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> + }
> +
> + /* get IMU calibration data, and parse it */
> + ret = joycon_request_imu_calibration(ctlr);
> + if (ret) {
> + /*
> + * We can function with default calibration, but it may be
> + * inaccurate. Provide a warning, and continue on.
> + */
> + hid_warn(hdev, "Unable to read IMU calibration data\n");
> + }
> +
> + /* Set the reporting mode to 0x30, which is the full report mode */
> + ret = joycon_set_report_mode(ctlr);
> + if (ret) {
> + hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> + goto out_unlock;
> + }
> +
> + /* Enable rumble */
> + ret = joycon_enable_rumble(ctlr);
> + if (ret) {
> + hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> + goto out_unlock;
> + }
> +
> + /* Enable the IMU */
> + ret = joycon_enable_imu(ctlr);
> + if (ret) {
> + hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> + goto out_unlock;
> + }
> +
> +out_unlock:
> + mutex_unlock(&ctlr->output_mutex);
> + return ret;
> +}
> +
> /* Common handler for parsing inputs */
> static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
> int size)
> @@ -2248,85 +2329,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>
> hid_device_io_start(hdev);
>
> - /* Initialize the controller */
> - mutex_lock(&ctlr->output_mutex);
> - /* if handshake command fails, assume ble pro controller */
> - if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> - !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> - hid_dbg(hdev, "detected USB controller\n");
> - /* set baudrate for improved latency */
> - ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> - if (ret) {
> - hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> - goto err_mutex;
> - }
> - /* handshake */
> - ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> - if (ret) {
> - hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> - goto err_mutex;
> - }
> - /*
> - * Set no timeout (to keep controller in USB mode).
> - * This doesn't send a response, so ignore the timeout.
> - */
> - joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> - } else if (jc_type_is_chrggrip(ctlr)) {
> - hid_err(hdev, "Failed charging grip handshake\n");
> - ret = -ETIMEDOUT;
> - goto err_mutex;
> - }
> -
> - /* get controller calibration data, and parse it */
> - ret = joycon_request_calibration(ctlr);
> + ret = joycon_init(hdev);
> if (ret) {
> - /*
> - * We can function with default calibration, but it may be
> - * inaccurate. Provide a warning, and continue on.
> - */
> - hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> - }
> -
> - /* get IMU calibration data, and parse it */
> - ret = joycon_request_imu_calibration(ctlr);
> - if (ret) {
> - /*
> - * We can function with default calibration, but it may be
> - * inaccurate. Provide a warning, and continue on.
> - */
> - hid_warn(hdev, "Unable to read IMU calibration data\n");
> - }
> -
> - /* Set the reporting mode to 0x30, which is the full report mode */
> - ret = joycon_set_report_mode(ctlr);
> - if (ret) {
> - hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> - goto err_mutex;
> - }
> -
> - /* Enable rumble */
> - ret = joycon_enable_rumble(ctlr);
> - if (ret) {
> - hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> - goto err_mutex;
> - }
> -
> - /* Enable the IMU */
> - ret = joycon_enable_imu(ctlr);
> - if (ret) {
> - hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> - goto err_mutex;
> + hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
> + goto err_close;
> }
>
> ret = joycon_read_info(ctlr);
> if (ret) {
> hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
> ret);
> - goto err_mutex;
> + goto err_close;
> }
>
> - mutex_unlock(&ctlr->output_mutex);
> -
> /* Initialize the leds */
> ret = joycon_leds_create(ctlr);
> if (ret) {
> @@ -2352,8 +2367,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> hid_dbg(hdev, "probe - success\n");
> return 0;
>
> -err_mutex:
> - mutex_unlock(&ctlr->output_mutex);
> err_close:
> hid_hw_close(hdev);
> err_stop:
> @@ -2383,6 +2396,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
> hid_hw_stop(hdev);
> }
>
> +#ifdef CONFIG_PM
> +
> +static int nintendo_hid_resume(struct hid_device *hdev)
> +{
> + int ret = joycon_init(hdev);
> +
> + if (ret)
> + hid_err(hdev, "Failed to restore controller after resume");
> +
> + return ret;
> +}
> +
> +#endif
> +
> static const struct hid_device_id nintendo_hid_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> USB_DEVICE_ID_NINTENDO_PROCON) },
> @@ -2404,6 +2431,10 @@ static struct hid_driver nintendo_hid_driver = {
> .probe = nintendo_hid_probe,
> .remove = nintendo_hid_remove,
> .raw_event = nintendo_hid_event,
> +
> +#ifdef CONFIG_PM
> + .resume = nintendo_hid_resume,
> +#endif
> };
> module_hid_driver(nintendo_hid_driver);
>
> --
> 2.42.0
>
Thanks for adding the resume hook for usb controllers. Looks good to me.
Reviewed-by: Daniel J. Ogorchock <djogorchock@gmail.com>
^ permalink raw reply
* [PATCH 1/3] Input: cap11xx - Cache hardware ID registers
From: Mark Brown @ 2023-09-30 23:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Mark Brown
In-Reply-To: <20231001-input-maple-v1-0-ed3716051431@kernel.org>
The cap11xx devices have three hardware identification registers which are
currently marked as volatile, preventing caching of those registers. This
is not ideal since the registers should never change at runtime, we should
be able to cache the value after the first read. Stop marking the registers
as volatile, we don't have register defaults specified in the driver so
this will result in reading from the hardware on first use.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/input/keyboard/cap11xx.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 1b4937dce672..39ed3b9ddc65 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -160,9 +160,6 @@ static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
case CAP11XX_REG_SENOR_DELTA(3):
case CAP11XX_REG_SENOR_DELTA(4):
case CAP11XX_REG_SENOR_DELTA(5):
- case CAP11XX_REG_PRODUCT_ID:
- case CAP11XX_REG_MANUFACTURER_ID:
- case CAP11XX_REG_REVISION:
return true;
}
--
2.39.2
^ permalink raw reply related
* [PATCH 0/3] Input: Small regmap improvements
From: Mark Brown @ 2023-09-30 23:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Mark Brown
This series has a few small improvements to the regmap usage in some of
the input drivers, there's nothing really important here but it's all
nice to have.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Mark Brown (3):
Input: cap11xx - Cache hardware ID registers
Input: cap11xx - Convert to use maple tree register cache
Input: qt1050 - Convert to use maple tree register cache
drivers/input/keyboard/cap11xx.c | 5 +----
drivers/input/keyboard/qt1050.c | 2 +-
2 files changed, 2 insertions(+), 5 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230929-input-maple-55fb28d952ea
Best regards,
--
Mark Brown <broonie@kernel.org>
^ permalink raw reply
* [PATCH 2/3] Input: cap11xx - Convert to use maple tree register cache
From: Mark Brown @ 2023-09-30 23:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Mark Brown
In-Reply-To: <20231001-input-maple-v1-0-ed3716051431@kernel.org>
The maple tree register cache is based on a much more modern data structure
than the rbtree cache and makes optimisation choices which are probably
more appropriate for modern systems than those made by the rbtree cache.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/input/keyboard/cap11xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 39ed3b9ddc65..77843ad15d4c 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -174,7 +174,7 @@ static const struct regmap_config cap11xx_regmap_config = {
.reg_defaults = cap11xx_reg_defaults,
.num_reg_defaults = ARRAY_SIZE(cap11xx_reg_defaults),
- .cache_type = REGCACHE_RBTREE,
+ .cache_type = REGCACHE_MAPLE,
.volatile_reg = cap11xx_volatile_reg,
};
--
2.39.2
^ permalink raw reply related
* [PATCH 3/3] Input: qt1050 - Convert to use maple tree register cache
From: Mark Brown @ 2023-09-30 23:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Mark Brown
In-Reply-To: <20231001-input-maple-v1-0-ed3716051431@kernel.org>
The maple tree register cache is based on a much more modern data structure
than the rbtree cache and makes optimisation choices which are probably
more appropriate for modern systems than those made by the rbtree cache.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/input/keyboard/qt1050.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
index 6953097db445..b51dfcd76038 100644
--- a/drivers/input/keyboard/qt1050.c
+++ b/drivers/input/keyboard/qt1050.c
@@ -213,7 +213,7 @@ static struct regmap_config qt1050_regmap_config = {
.val_bits = 8,
.max_register = QT1050_RES_CAL,
- .cache_type = REGCACHE_RBTREE,
+ .cache_type = REGCACHE_MAPLE,
.wr_table = &qt1050_writeable_table,
.rd_table = &qt1050_readable_table,
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] Input: powermate - fix use-after-free in powermate_config_complete
From: Javier Carrasco @ 2023-10-01 9:11 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <ZQjKwQDKmU8L9C9e@google.com>
Hi Dmitry,
On 19.09.23 00:10, Dmitry Torokhov wrote:
> On Mon, Sep 18, 2023 at 06:51:49AM +0200, Javier Carrasco Cruz wrote:
>> Hi,
>>
>> There's an obvious error in the patch I introduced when cleaningup
>> (urb->status should be used instead of just status). I will send a v2.
>
> I think what we need is call to usb_kill_urb(pm->config) in
> powermate_disconnect(), right after call to input_unregister_device().
>
> Thanks.
> That is definitely a more meaningful and elegant solution, so I will
check it out and eventually send a v2 with it if everything seems ok. On
the other hand usb_kill_urb() is already used on pm->irq before calling
input_unregister_device(), so I would move the existing usb_kill_urb to
have both calls right after the unregister_device call for code
consistency, if that is alright.
Thanks and best regards.
^ permalink raw reply
* [PATCH RFC v4 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for configuring
its two onboard LEDs.
Convert them to use the GPIO descriptor interface.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 535e2b2e997b..b6a4085e9fb0 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
* LEDs
******************************************************************************/
#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
+static struct gpiod_lookup_table spitz_led_gpio_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
+ GPIO_ACTIVE_HIGH),
+ { }
+ }
+};
+
static struct gpio_led spitz_gpio_leds[] = {
{
.name = "spitz:amber:charge",
.default_trigger = "sharpsl-charge",
- .gpio = SPITZ_GPIO_LED_ORANGE,
},
{
.name = "spitz:green:hddactivity",
.default_trigger = "disk-activity",
- .gpio = SPITZ_GPIO_LED_GREEN,
},
};
@@ -480,7 +489,12 @@ static struct platform_device spitz_led_device = {
static void __init spitz_leds_init(void)
{
+ gpiod_add_lookup_table(&spitz_led_gpio_table);
platform_device_register(&spitz_led_device);
+ spitz_gpio_leds[0].gpiod = gpiod_get_index(&spitz_led_device.dev,
+ NULL, 0, GPIOD_ASIS);
+ spitz_gpio_leds[1].gpiod = gpiod_get_index(&spitz_led_device.dev,
+ NULL, 1, GPIOD_ASIS);
}
#else
static inline void spitz_leds_init(void) {}
--
2.42.0
^ permalink raw reply related
* [PATCH RFC v4 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr>
Gumstix still uses the legacy GPIO interface for resetting the Bluetooth
device.
Convert it to use the GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/gumstix.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-pxa/gumstix.c b/arch/arm/mach-pxa/gumstix.c
index c9f0f62187bd..14e1b9274d7a 100644
--- a/arch/arm/mach-pxa/gumstix.c
+++ b/arch/arm/mach-pxa/gumstix.c
@@ -20,8 +20,8 @@
#include <linux/delay.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/gpio/consumer.h>
#include <linux/gpio/machine.h>
-#include <linux/gpio.h>
#include <linux/err.h>
#include <linux/clk.h>
@@ -129,6 +129,9 @@ static void gumstix_udc_init(void)
#endif
#ifdef CONFIG_BT
+GPIO_LOOKUP_SINGLE(gumstix_bt_gpio_table, "pxa2xx-uart.1", "pxa-gpio",
+ GPIO_GUMSTIX_BTRESET, "BTRST", GPIO_ACTIVE_LOW);
+
/* Normally, the bootloader would have enabled this 32kHz clock but many
** boards still have u-boot 1.1.4 so we check if it has been turned on and
** if not, we turn it on with a warning message. */
@@ -153,24 +156,23 @@ static void gumstix_setup_bt_clock(void)
static void __init gumstix_bluetooth_init(void)
{
- int err;
+ struct gpio_desc *desc;
+
+ gpiod_add_lookup_table(&gumstix_bt_gpio_table);
gumstix_setup_bt_clock();
- err = gpio_request(GPIO_GUMSTIX_BTRESET, "BTRST");
- if (err) {
+ desc = gpiod_get(&pxa_device_btuart.dev, "BTRST", GPIOD_OUT_HIGH);
+ if (IS_ERR(desc)) {
pr_err("gumstix: failed request gpio for bluetooth reset\n");
return;
}
- err = gpio_direction_output(GPIO_GUMSTIX_BTRESET, 1);
- if (err) {
- pr_err("gumstix: can't reset bluetooth\n");
- return;
- }
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 0);
+ gpiod_set_value(desc, 0);
udelay(100);
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 1);
+ gpiod_set_value(desc, 1);
+
+ gpiod_put(desc);
}
#else
static void gumstix_bluetooth_init(void)
--
2.42.0
^ permalink raw reply related
* [PATCH RFC v4 0/6] ARM: pxa: GPIO descriptor conversions
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
Hello,
Small series to convert some of the board files in the mach-pxa directory
to use the new GPIO descriptor interface.
Most notably, the am200epd, am300epd and Spitz matrix keypad among
others are not converted in this series.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
Changes in v4:
- Address maintainer comments:
- Move wait_for_sync() from spitz.c to driver
- Register LED platform device before getting its gpiod-s
- Add Linus' Reviewed-by
- Link to v3: https://lore.kernel.org/r/20230929-pxa-gpio-v3-0-af8d5e5d1f34@skole.hr
Changes in v3:
- Address maintainer comments:
- Use GPIO_LOOKUP_IDX for LEDs
- Drop unnecessary NULL assignments
- Don't give up on *all* SPI devices if hsync cannot be set up
- Add Linus' Acked-by
- Link to v2: https://lore.kernel.org/r/20230926-pxa-gpio-v2-0-984464d165dd@skole.hr
Changes in v2:
- Address maintainer comments:
- Change mentions of function to function()
- Drop cast in OHCI driver dev_warn() call
- Use %pe in OHCI and reset drivers
- Use GPIO _optional() API in OHCI driver
- Drop unnecessary not-null check in OHCI driver
- Use pr_err() instead of printk() in reset driver
- Rebase on v6.6-rc3
- Link to v1: https://lore.kernel.org/r/20230924-pxa-gpio-v1-0-2805b87d8894@skole.hr
---
Duje Mihanović (6):
ARM: pxa: Convert Spitz OHCI to GPIO descriptors
ARM: pxa: Convert Spitz LEDs to GPIO descriptors
ARM: pxa: Convert Spitz CF power control to GPIO descriptors
ARM: pxa: Convert reset driver to GPIO descriptors
ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
input: ads7846: Move wait_for_sync() logic to driver
arch/arm/mach-pxa/gumstix.c | 24 +++++++------
arch/arm/mach-pxa/reset.c | 39 +++++++--------------
arch/arm/mach-pxa/reset.h | 3 +-
arch/arm/mach-pxa/spitz.c | 69 +++++++++++++++++++++++++------------
drivers/input/touchscreen/ads7846.c | 22 ++++++++----
drivers/usb/host/ohci-pxa27x.c | 7 ++++
include/linux/spi/ads7846.h | 1 -
7 files changed, 96 insertions(+), 69 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230807-pxa-gpio-3ce25d574814
Best regards,
--
Duje Mihanović <duje.mihanovic@skole.hr>
^ permalink raw reply
* [PATCH RFC v4 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr>
The PXA reset driver still uses the legacy GPIO interface for
configuring and asserting the reset pin.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/reset.c | 39 +++++++++++++--------------------------
arch/arm/mach-pxa/reset.h | 3 +--
arch/arm/mach-pxa/spitz.c | 6 +++++-
3 files changed, 19 insertions(+), 29 deletions(-)
diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
index 27293549f8ad..2bfa66f99555 100644
--- a/arch/arm/mach-pxa/reset.c
+++ b/arch/arm/mach-pxa/reset.c
@@ -2,7 +2,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/io.h>
#include <asm/proc-fns.h>
#include <asm/system_misc.h>
@@ -14,33 +14,20 @@
static void do_hw_reset(void);
-static int reset_gpio = -1;
+static struct gpio_desc *reset_gpio;
-int init_gpio_reset(int gpio, int output, int level)
+int init_gpio_reset(int output, int level)
{
- int rc;
-
- rc = gpio_request(gpio, "reset generator");
- if (rc) {
- printk(KERN_ERR "Can't request reset_gpio\n");
- goto out;
+ reset_gpio = gpiod_get(NULL, "reset generator", GPIOD_ASIS);
+ if (IS_ERR(reset_gpio)) {
+ pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
+ return PTR_ERR(reset_gpio);
}
if (output)
- rc = gpio_direction_output(gpio, level);
+ return gpiod_direction_output(reset_gpio, level);
else
- rc = gpio_direction_input(gpio);
- if (rc) {
- printk(KERN_ERR "Can't configure reset_gpio\n");
- gpio_free(gpio);
- goto out;
- }
-
-out:
- if (!rc)
- reset_gpio = gpio;
-
- return rc;
+ return gpiod_direction_input(reset_gpio);
}
/*
@@ -50,16 +37,16 @@ int init_gpio_reset(int gpio, int output, int level)
*/
static void do_gpio_reset(void)
{
- BUG_ON(reset_gpio == -1);
+ BUG_ON(IS_ERR(reset_gpio));
/* drive it low */
- gpio_direction_output(reset_gpio, 0);
+ gpiod_direction_output(reset_gpio, 0);
mdelay(2);
/* rising edge or drive high */
- gpio_set_value(reset_gpio, 1);
+ gpiod_set_value(reset_gpio, 1);
mdelay(2);
/* falling edge */
- gpio_set_value(reset_gpio, 0);
+ gpiod_set_value(reset_gpio, 0);
/* give it some time */
mdelay(10);
diff --git a/arch/arm/mach-pxa/reset.h b/arch/arm/mach-pxa/reset.h
index 963dd190bc13..5864f61a0e94 100644
--- a/arch/arm/mach-pxa/reset.h
+++ b/arch/arm/mach-pxa/reset.h
@@ -13,10 +13,9 @@ extern void pxa_register_wdt(unsigned int reset_status);
/**
* init_gpio_reset() - register GPIO as reset generator
- * @gpio: gpio nr
* @output: set gpio as output instead of input during normal work
* @level: output level
*/
-extern int init_gpio_reset(int gpio, int output, int level);
+extern int init_gpio_reset(int output, int level);
#endif /* __ASM_ARCH_RESET_H */
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 965354e64c68..701fba130ac4 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -1024,9 +1024,13 @@ static void spitz_restart(enum reboot_mode mode, const char *cmd)
spitz_poweroff();
}
+GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
+ SPITZ_GPIO_ON_RESET, "reset generator", GPIO_ACTIVE_HIGH);
+
static void __init spitz_init(void)
{
- init_gpio_reset(SPITZ_GPIO_ON_RESET, 1, 0);
+ gpiod_add_lookup_table(&spitz_reset_gpio_table);
+ init_gpio_reset(1, 0);
pm_power_off = spitz_poweroff;
PMCR = 0x00;
--
2.42.0
^ permalink raw reply related
* [PATCH RFC v4 6/6] input: ads7846: Move wait_for_sync() logic to driver
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr>
If this code is left in the board file, the sync GPIO would have to be
separated into another lookup table during conversion to the GPIO
descriptor API (which is also done in this patch).
The only user of this code (Sharp Spitz) is also converted in this
patch.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 12 ++----------
drivers/input/touchscreen/ads7846.c | 22 +++++++++++++++-------
include/linux/spi/ads7846.h | 1 -
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 701fba130ac4..22d5c5645b8f 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -520,22 +520,12 @@ static inline void spitz_leds_init(void) {}
* SSP Devices
******************************************************************************/
#if defined(CONFIG_SPI_PXA2XX) || defined(CONFIG_SPI_PXA2XX_MODULE)
-static void spitz_ads7846_wait_for_hsync(void)
-{
- while (gpio_get_value(SPITZ_GPIO_HSYNC))
- cpu_relax();
-
- while (!gpio_get_value(SPITZ_GPIO_HSYNC))
- cpu_relax();
-}
-
static struct ads7846_platform_data spitz_ads7846_info = {
.model = 7846,
.vref_delay_usecs = 100,
.x_plate_ohms = 419,
.y_plate_ohms = 486,
.pressure_max = 1024,
- .wait_for_sync = spitz_ads7846_wait_for_hsync,
};
static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
@@ -543,6 +533,8 @@ static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
.table = {
GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_TP_INT,
"pendown", GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_HSYNC,
+ "sync", GPIO_ACTIVE_LOW),
{ }
},
};
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index faea40dd66d0..894f179bfa8d 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -138,8 +138,7 @@ struct ads7846 {
void *filter_data;
int (*get_pendown_state)(void);
struct gpio_desc *gpio_pendown;
-
- void (*wait_for_sync)(void);
+ struct gpio_desc *sync;
};
enum ads7846_filter {
@@ -636,9 +635,14 @@ static const struct attribute_group ads784x_attr_group = {
};
/*--------------------------------------------------------------------------*/
-
-static void null_wait_for_sync(void)
+static void ads7846_wait_for_sync(struct ads7846 *ts)
{
+ if (!ts->sync) return;
+ while (!gpiod_get_value(ts->sync))
+ cpu_relax();
+
+ while (gpiod_get_value(ts->sync))
+ cpu_relax();
}
static int ads7846_debounce_filter(void *ads, int data_idx, int *val)
@@ -803,7 +807,7 @@ static void ads7846_read_state(struct ads7846 *ts)
packet->last_cmd_idx = 0;
while (true) {
- ts->wait_for_sync();
+ ads7846_wait_for_sync(ts);
m = &ts->msg[msg_idx];
error = spi_sync(ts->spi, m);
@@ -1261,8 +1265,6 @@ static int ads7846_probe(struct spi_device *spi)
ts->penirq_recheck_delay_usecs =
pdata->penirq_recheck_delay_usecs;
- ts->wait_for_sync = pdata->wait_for_sync ? : null_wait_for_sync;
-
snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
snprintf(ts->name, sizeof(ts->name), "ADS%d Touchscreen", ts->model);
@@ -1361,6 +1363,12 @@ static int ads7846_probe(struct spi_device *spi)
if (err)
return err;
+ ts->sync = devm_gpiod_get_optional(dev, "sync", GPIOD_IN);
+ if (IS_ERR(ts->sync)) {
+ dev_err(dev, "Failed to get sync GPIO: %pe\n", ts->sync);
+ return PTR_ERR(ts->sync);
+ }
+
err = input_register_device(input_dev);
if (err)
return err;
diff --git a/include/linux/spi/ads7846.h b/include/linux/spi/ads7846.h
index a04c1c34c344..fa7c4f119023 100644
--- a/include/linux/spi/ads7846.h
+++ b/include/linux/spi/ads7846.h
@@ -38,7 +38,6 @@ struct ads7846_platform_data {
int gpio_pendown_debounce; /* platform specific debounce time for
* the gpio_pendown */
int (*get_pendown_state)(void);
- void (*wait_for_sync)(void);
bool wakeup;
unsigned long irq_flags;
};
--
2.42.0
^ permalink raw reply related
* [PATCH RFC v4 3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
the power supply to its CF and SD card slots.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index b6a4085e9fb0..965354e64c68 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -133,6 +133,10 @@ static unsigned long spitz_pin_config[] __initdata = {
* Scoop GPIO expander
******************************************************************************/
#if defined(CONFIG_SHARP_SCOOP) || defined(CONFIG_SHARP_SCOOP_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_card_pwr_ctrl_gpio_table, "pxa2xx-mci.0",
+ "sharp-scoop", SPITZ_GPIO_CF_POWER, "cf_power",
+ GPIO_ACTIVE_HIGH);
+
/* SCOOP Device #1 */
static struct resource spitz_scoop_1_resources[] = {
[0] = {
@@ -190,6 +194,7 @@ struct platform_device spitz_scoop_2_device = {
static void __init spitz_scoop_init(void)
{
platform_device_register(&spitz_scoop_1_device);
+ gpiod_add_lookup_table(&spitz_card_pwr_ctrl_gpio_table);
/* Akita doesn't have the second SCOOP chip */
if (!machine_is_akita())
@@ -201,9 +206,18 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
{
unsigned short cpr;
unsigned long flags;
+ struct gpio_desc *cf_power;
+
+ cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
+ if (IS_ERR(cf_power)) {
+ dev_err(&pxa_device_mci.dev,
+ "failed to get power control GPIO with %ld\n",
+ PTR_ERR(cf_power));
+ return;
+ }
if (new_cpr & 0x7) {
- gpio_set_value(SPITZ_GPIO_CF_POWER, 1);
+ gpiod_direction_output(cf_power, 1);
mdelay(5);
}
@@ -222,8 +236,10 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
if (!(cpr & 0x7)) {
mdelay(1);
- gpio_set_value(SPITZ_GPIO_CF_POWER, 0);
+ gpiod_direction_output(cf_power, 0);
}
+
+ gpiod_put(cf_power);
}
#else
--
2.42.0
^ permalink raw reply related
* [PATCH RFC v4 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Duje Mihanović @ 2023-10-01 14:12 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
a GPIO pin related to the USB host controller.
Convert this function to use the new GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 13 ++++++-------
drivers/usb/host/ohci-pxa27x.c | 7 +++++++
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index cc691b199429..535e2b2e997b 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
* USB Host
******************************************************************************/
#if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
+ SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
+
static int spitz_ohci_init(struct device *dev)
{
- int err;
-
- err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
- if (err)
- return err;
+ gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
/* Only Port 2 is connected, setup USB Port 2 Output Control Register */
UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
- return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
+ return 0;
}
static void spitz_ohci_exit(struct device *dev)
{
- gpio_free(SPITZ_GPIO_USB_HOST);
+ gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
}
static struct pxaohci_platform_data spitz_ohci_platform_data = {
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 357d9aee38a3..b70d452ca7c2 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -121,6 +121,7 @@ struct pxa27x_ohci {
void __iomem *mmio_base;
struct regulator *vbus[3];
bool vbus_enabled[3];
+ struct gpio_desc *usb_host;
};
#define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
@@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
pxa_ohci = to_pxa27x_ohci(hcd);
pxa_ohci->clk = usb_clk;
pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
+ pxa_ohci->usb_host = gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
+ if (IS_ERR(pxa_ohci->usb_host))
+ dev_warn(&pdev->dev, "failed to get USB host GPIO with %pe\n",
+ pxa_ohci->usb_host);
for (i = 0; i < 3; ++i) {
char name[6];
@@ -512,6 +517,8 @@ static void ohci_hcd_pxa27x_remove(struct platform_device *pdev)
for (i = 0; i < 3; ++i)
pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
+ gpiod_put(pxa_ohci->usb_host);
+
usb_put_hcd(hcd);
}
--
2.42.0
^ permalink raw reply related
* Re: [PATCH RFC v4 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Andy Shevchenko @ 2023-10-01 14:32 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <20231001-pxa-gpio-v4-1-0f3b975e6ed5@skole.hr>
On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> a GPIO pin related to the USB host controller.
>
> Convert this function to use the new GPIO descriptor interface.
...
> + pxa_ohci->usb_host = gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
> + if (IS_ERR(pxa_ohci->usb_host))
> + dev_warn(&pdev->dev, "failed to get USB host GPIO with %pe\n",
> + pxa_ohci->usb_host);
Since you are using _optional() API, you need to bail out on the error
case and replace dev_warn() by dev_err(). I guess I already commented
on this. What is the rationale to not follow my comment?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v4 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Andy Shevchenko @ 2023-10-01 14:34 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <20231001-pxa-gpio-v4-2-0f3b975e6ed5@skole.hr>
On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for configuring
> its two onboard LEDs.
>
> Convert them to use the GPIO descriptor interface.
...
> static void __init spitz_leds_init(void)
> {
> + gpiod_add_lookup_table(&spitz_led_gpio_table);
> platform_device_register(&spitz_led_device);
> + spitz_gpio_leds[0].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 0, GPIOD_ASIS);
> + spitz_gpio_leds[1].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 1, GPIOD_ASIS);
> }
What's the point of keeping a lookup table after we got descriptors out of it?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v4 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Andy Shevchenko @ 2023-10-01 14:40 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <20231001-pxa-gpio-v4-4-0f3b975e6ed5@skole.hr>
On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> The PXA reset driver still uses the legacy GPIO interface for
> configuring and asserting the reset pin.
>
> Convert it to use the GPIO descriptor interface.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
I dunno how.
...
> + reset_gpio = gpiod_get(NULL, "reset generator", GPIOD_ASIS);
> + if (IS_ERR(reset_gpio)) {
> + pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
> + return PTR_ERR(reset_gpio);
> }
Here you asked for the GPIO named as "reset generator-gpio(s)" (The
"(s)" part is for new bindings), but you must not use spaces in the
GPIO names. Moreover the string literal there is for labeling, and not
for matching.
...
> +GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
And here should be gpios. That's what you have to request, but because
of the global (device-less) nature of this, you have to be very
careful to avoid any clashes.
> + SPITZ_GPIO_ON_RESET, "reset generator", GPIO_ACTIVE_HIGH);
...
TBH, I don't know how it is supposed to work with your current code
and if Linus really was okay with this.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v4 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Andy Shevchenko @ 2023-10-01 14:50 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <CAHp75VcgajYz4XScSLTxYSKy6mbTjJ9mD7zF3j90d5+6V8NyZg@mail.gmail.com>
On Sun, Oct 1, 2023 at 5:40 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
...
> TBH, I don't know how it is supposed to work with your current code
> and if Linus really was okay with this.
Okay, it seems I have to refresh my memories about GPIO lookup tables.
First of all, we indeed require a connection ID just to match and no
matter if it has or hasn't the suffix.Second, the key is a label of
the GPIO controller according to the device driver (device tree?) and
pxa-gpio is that one. Seems it should work.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v4 6/6] input: ads7846: Move wait_for_sync() logic to driver
From: Andy Shevchenko @ 2023-10-01 14:56 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <20231001-pxa-gpio-v4-6-0f3b975e6ed5@skole.hr>
On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> If this code is left in the board file, the sync GPIO would have to be
> separated into another lookup table during conversion to the GPIO
> descriptor API (which is also done in this patch).
>
> The only user of this code (Sharp Spitz) is also converted in this
> patch.
Suggested-by: Linus... ?
...
> +static void ads7846_wait_for_sync(struct ads7846 *ts)
I would name it ..._wait_for_sync_gpio.
...
> + ts->sync = devm_gpiod_get_optional(dev, "sync", GPIOD_IN);
> + if (IS_ERR(ts->sync)) {
> + dev_err(dev, "Failed to get sync GPIO: %pe\n", ts->sync);
> + return PTR_ERR(ts->sync);
return dev_err_probe(...); ?
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2] Input: powermate - fix use-after-free in powermate_config_complete
From: Javier Carrasco @ 2023-10-01 15:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Javier Carrasco,
syzbot+0434ac83f907a1dbdd1e
syzbot has found a use-after-free bug [1] in the powermate driver. This
happens when the device is disconnected, which leads to a memory free
from the powermate_device struct.
When an asynchronous control message completes after the kfree and its
callback is invoked, the lock does not exist anymore and hence the bug.
Use usb_kill_urb() on pm->config to cancel any in-progress requests upon
device disconnection. Given that this action is already done on pm->irq,
reorder the code to have both calls after the call to
input_unregister_device(), which is the most common approach.
[1] https://syzkaller.appspot.com/bug?extid=0434ac83f907a1dbdd1e
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Reported-by: syzbot+0434ac83f907a1dbdd1e@syzkaller.appspotmail.com
---
Changes in v2:
- Use usb_kill_urb() on pm->config upon device disconnection.
- Link to v1: https://lore.kernel.org/r/20230916-topic-powermate_use_after_free-v1-1-2ffa46652869@gmail.com
---
drivers/input/misc/powermate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c
index c1c733a9cb89..c0aea75eb087 100644
--- a/drivers/input/misc/powermate.c
+++ b/drivers/input/misc/powermate.c
@@ -423,8 +423,9 @@ static void powermate_disconnect(struct usb_interface *intf)
usb_set_intfdata(intf, NULL);
if (pm) {
pm->requires_update = 0;
- usb_kill_urb(pm->irq);
input_unregister_device(pm->input);
+ usb_kill_urb(pm->irq);
+ usb_kill_urb(pm->config);
usb_free_urb(pm->irq);
usb_free_urb(pm->config);
powermate_free_buffers(interface_to_usbdev(intf), pm);
---
base-commit: cefc06e4de1477dbdc3cb2a91d4b1873b7797a5c
change-id: 20230916-topic-powermate_use_after_free-c703c7969c91
Best regards,
--
Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply related
* Re: [PATCH RFC v4 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Duje Mihanović @ 2023-10-01 18:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <CAHp75VcBY3W8aVEsRMPNMW9940yT+_=-w8J2uKfqvmUiAVjPhg@mail.gmail.com>
On 10/1/2023 4:32 PM, Andy Shevchenko wrote:
>> + pxa_ohci->usb_host = gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
>> + if (IS_ERR(pxa_ohci->usb_host))
>> + dev_warn(&pdev->dev, "failed to get USB host GPIO with %pe\n",
>> + pxa_ohci->usb_host);
>
> Since you are using _optional() API, you need to bail out on the error
> case and replace dev_warn() by dev_err(). I guess I already commented
> on this. What is the rationale to not follow my comment?
I must have missed it, sorry about that. I'll be sure to do so in v5.
Regards,
Duje
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 68ede283a1d8fe0813b218aeb498faf3b0fc0a7b
From: kernel test robot @ 2023-10-02 1:44 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 68ede283a1d8fe0813b218aeb498faf3b0fc0a7b Input: axp20x-pek - avoid needless newline removal
elapsed time: 1951m
configs tested: 105
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20231001 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm randconfig-001-20231001 gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20231001 gcc
i386 buildonly-randconfig-002-20231001 gcc
i386 buildonly-randconfig-003-20231001 gcc
i386 buildonly-randconfig-004-20231001 gcc
i386 buildonly-randconfig-005-20231001 gcc
i386 buildonly-randconfig-006-20231001 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20231001 gcc
i386 randconfig-002-20231001 gcc
i386 randconfig-003-20231001 gcc
i386 randconfig-004-20231001 gcc
i386 randconfig-005-20231001 gcc
i386 randconfig-006-20231001 gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20231001 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20231001 gcc
x86_64 randconfig-002-20231001 gcc
x86_64 randconfig-003-20231001 gcc
x86_64 randconfig-004-20231001 gcc
x86_64 randconfig-005-20231001 gcc
x86_64 randconfig-006-20231001 gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v7 0/4] Input: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-10-02 6:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-input,
linux-arm-msm, devicetree, linux-kernel, Neil Armstrong,
Rob Herring
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v7:
- rebased on v6.6-rc3
- Link to v6: https://lore.kernel.org/r/20230912-topic-goodix-berlin-upstream-initial-v6-0-b4ecfa49fb9d@linaro.org
Changes in v6:
- rebased on v6.6-rc1
- changed commit message prefix to match the other Input commits
- Link to v5: https://lore.kernel.org/r/20230801-topic-goodix-berlin-upstream-initial-v5-0-079252935593@linaro.org
Changes in v5:
- rebased on next-20230801
- Link to v4: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org
Changes in v4:
- Core updates:
- drop kconfig depends, deps will be handled by _SPI and _I2C
- change power_on() error labels
- print errors on all dev_err() prints
- remove useless default variable initialization
- switch irq touch checksum error to dev_err()
- add Jeff's review tag
- I2C changes
- change REGMAP_I2C Kconfig from depends to select
- add Jeff's review tag
- SPI changes
- add select REGMAP to Kconfig
- added GOODIX_BERLIN_ prefix to defines
- switched from ret to error
- add Jeff's review tag
- Link to v3: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v3-0-f0577cead709@linaro.org
Changes in v3:
- Another guge cleanups after Jeff's review:
- appended goodix_berlin_ before all defines
- removed some unused defines
- removed retries on most of read functions, can be added back later
- added __le to ic_info structures
- reworked and simplified irq handling, dropped enum and ts_event structs
- added struct for touch data
- simplified and cleaned goodix_berlin_check_checksum & goodix_berlin_is_dummy_data
- moved touch_data_addr to the end of the main code_data
- reworked probe to get_irq last and right before setip input device
- cleaned probe by removing the "cd->dev"
- added short paragraph to justify new driver for berlin devices
- defined all offsets & masks
- Added bindings review tag
- Link to v2: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v2-0-26bc8fe1e90e@linaro.org
Changes in v2:
- Huge cleanups after Jeff's review:
- switch to error instead of ret
- drop dummy vendor/product ids
- drop unused defined/enums
- drop unused ic_info and only keep needes values
- cleanup namings and use goodix_berlin_ everywhere
- fix regulator setup
- fix default variables value when assigned afterwars
- removed indirections
- dropped debugfs
- cleaned input_dev setup
- dropped _remove()
- sync'ed i2c and spi drivers
- fixed yaml bindings
- Link to v1: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org
---
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
Input: add core support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add I2C support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add SPI support for Goodix Berlin Touchscreen IC
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++
drivers/input/touchscreen/Kconfig | 31 ++
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 159 ++++++
drivers/input/touchscreen/goodix_berlin_core.c | 581 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 69 +++
drivers/input/touchscreen/goodix_berlin_spi.c | 173 ++++++
7 files changed, 1111 insertions(+)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* [PATCH v7 1/4] dt-bindings: input: document Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-10-02 6:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-input,
linux-arm-msm, devicetree, linux-kernel, Neil Armstrong,
Rob Herring
In-Reply-To: <20231002-topic-goodix-berlin-upstream-initial-v7-0-792fb91f5e88@linaro.org>
Document the Goodix GT9916 wich is part of the "Berlin" serie
of Touchscreen controllers IC from Goodix.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
new file mode 100644
index 000000000000..d90f045ac06c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix,gt9916.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix Berlin series touchscreen controller
+
+description: The Goodix Berlin series of touchscreen controllers
+ be connected to either I2C or SPI buses.
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - goodix,gt9916
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ avdd-supply:
+ description: Analog power supply regulator on AVDD pin
+
+ vddio-supply:
+ description: power supply regulator on VDDIO pin
+
+ spi-max-frequency: true
+ touchscreen-inverted-x: true
+ touchscreen-inverted-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - avdd-supply
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@5d {
+ compatible = "goodix,gt9916";
+ reg = <0x5d>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-cs = <1>;
+ cs-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ touchscreen@0 {
+ compatible = "goodix,gt9916";
+ reg = <0>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ spi-max-frequency = <1000000>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+
+...
--
2.34.1
^ permalink raw reply related
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