* [PATCH v2 2/3] Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
From: Vignesh R @ 2018-04-14 9:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
linux-omap, Tony Lindgren
In-Reply-To: <20180414095153.32060-1-vigneshr@ti.com>
From: Grygorii Strashko <grygorii.strashko@ti.com>
It is seen that just enabling the TSC module triggers a HW_PEN IRQ
without any interaction with touchscreen by user. This results in first
suspend/resume sequence to fail as system immediately wakes up from
suspend as soon as HW_PEN IRQ is enabled in suspend handler due to the
pending IRQ. Therefore clear all IRQs at probe and also in suspend
callback for sanity.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
v2: Add Acks from v1.
drivers/input/touchscreen/ti_am335x_tsc.c | 2 ++
include/linux/mfd/ti_am335x_tscadc.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 810e05c9c4f5..dcd9db768169 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -439,6 +439,7 @@ static int titsc_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "irq wake enable failed.\n");
}
+ titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
err = titsc_config_wires(ts_dev);
@@ -504,6 +505,7 @@ static int __maybe_unused titsc_suspend(struct device *dev)
tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
if (device_may_wakeup(tscadc_dev->dev)) {
+ titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
idle = titsc_readl(ts_dev, REG_IRQENABLE);
titsc_writel(ts_dev, REG_IRQENABLE,
(idle | IRQENB_HW_PEN));
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index b9a53e013bff..1a6a34f726cc 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -63,6 +63,7 @@
#define IRQENB_FIFO1OVRRUN BIT(6)
#define IRQENB_FIFO1UNDRFLW BIT(7)
#define IRQENB_PENUP BIT(9)
+#define IRQENB_MASK (0x7FF)
/* Step Configuration */
#define STEPCONFIG_MODE_MASK (3 << 0)
--
2.17.0
^ permalink raw reply related
* [PATCH v2 3/3] Input: ti_am335x_tsc - Prevent system suspend when TSC is in use
From: Vignesh R @ 2018-04-14 9:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
linux-omap, Tony Lindgren
In-Reply-To: <20180414095153.32060-1-vigneshr@ti.com>
From: Grygorii Strashko <grygorii.strashko@ti.com>
Prevent system suspend while user has finger on touch screen,
because TSC is wakeup source and suspending device while in use will
result in failure to disable the module.
This patch uses pm_stay_awake() and pm_relax() APIs to prevent and
resume system suspend as required.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
v2: No changes.
drivers/input/touchscreen/ti_am335x_tsc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index dcd9db768169..43b22e071842 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -275,6 +275,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
if (status & IRQENB_HW_PEN) {
ts_dev->pen_down = true;
irqclr |= IRQENB_HW_PEN;
+ pm_stay_awake(ts_dev->mfd_tscadc->dev);
}
if (status & IRQENB_PENUP) {
@@ -284,6 +285,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
input_report_key(input_dev, BTN_TOUCH, 0);
input_report_abs(input_dev, ABS_PRESSURE, 0);
input_sync(input_dev);
+ pm_relax(ts_dev->mfd_tscadc->dev);
} else {
ts_dev->pen_down = true;
}
@@ -524,6 +526,7 @@ static int __maybe_unused titsc_resume(struct device *dev)
titsc_writel(ts_dev, REG_IRQWAKEUP,
0x00);
titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
+ pm_relax(ts_dev->mfd_tscadc->dev);
}
titsc_step_config(ts_dev);
titsc_writel(ts_dev, REG_FIFO0THR,
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v3 01/11] iio: adc: at91-sama5d2_adc: fix channel configuration for differential channels
From: Jonathan Cameron @ 2018-04-15 19:23 UTC (permalink / raw)
To: Eugen Hristev
Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov, robh
In-Reply-To: <1523350677-27106-2-git-send-email-eugen.hristev@microchip.com>
On Tue, 10 Apr 2018 11:57:47 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> When iterating through the channels, the index in the array is not the
> scan index. Added an xlate function to translate to the proper index.
> The result of the bug is that the channel array is indexed with a wrong index,
> thus instead of the proper channel, we access invalid memory, which may
> lead to invalid results and/or corruption.
> This will be used also for devicetree channel xlate.
>
> Fixes: 5e1a1da0f ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support")
> Fixes: 073c66201 ("iio: adc: at91-sama5d2_adc: add support for DMA")
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
This fix obviously stands on it's own so I have applied it to the
fixes-togreg branch of iio.git and will send out a pull request with it in
after the merge window is closed and rc1 is out.
Marked for stable.
Note this will delay the rest of the series being applied but that is no
bad thing as I would like a little more time for people to consider it anyway
and we are obviously in no rush given the time in the cycle!
Thanks,
Jonathan
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 41 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 4eff835..8729d65 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -333,6 +333,27 @@ static const struct iio_chan_spec at91_adc_channels[] = {
> + AT91_SAMA5D2_DIFF_CHAN_CNT + 1),
> };
>
> +static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan)
> +{
> + int i;
> +
> + for (i = 0; i < indio_dev->num_channels; i++) {
> + if (indio_dev->channels[i].scan_index == chan)
> + return i;
> + }
> + return -EINVAL;
> +}
> +
> +static inline struct iio_chan_spec const *
> +at91_adc_chan_get(struct iio_dev *indio_dev, int chan)
> +{
> + int index = at91_adc_chan_xlate(indio_dev, chan);
> +
> + if (index < 0)
> + return NULL;
> + return indio_dev->channels + index;
> +}
> +
> static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> {
> struct iio_dev *indio = iio_trigger_get_drvdata(trig);
> @@ -350,8 +371,10 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> at91_adc_writel(st, AT91_SAMA5D2_TRGR, status);
>
> for_each_set_bit(bit, indio->active_scan_mask, indio->num_channels) {
> - struct iio_chan_spec const *chan = indio->channels + bit;
> + struct iio_chan_spec const *chan = at91_adc_chan_get(indio, bit);
>
> + if (!chan)
> + continue;
> if (state) {
> at91_adc_writel(st, AT91_SAMA5D2_CHER,
> BIT(chan->channel));
> @@ -448,7 +471,11 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev)
>
> for_each_set_bit(bit, indio_dev->active_scan_mask,
> indio_dev->num_channels) {
> - struct iio_chan_spec const *chan = indio_dev->channels + bit;
> + struct iio_chan_spec const *chan =
> + at91_adc_chan_get(indio_dev, bit);
> +
> + if (!chan)
> + continue;
>
> st->dma_st.rx_buf_sz += chan->scan_type.storagebits / 8;
> }
> @@ -526,8 +553,11 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> */
> for_each_set_bit(bit, indio_dev->active_scan_mask,
> indio_dev->num_channels) {
> - struct iio_chan_spec const *chan = indio_dev->channels + bit;
> + struct iio_chan_spec const *chan =
> + at91_adc_chan_get(indio_dev, bit);
>
> + if (!chan)
> + continue;
> if (st->dma_st.dma_chan)
> at91_adc_readl(st, chan->address);
> }
> @@ -587,8 +617,11 @@ static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
>
> for_each_set_bit(bit, indio_dev->active_scan_mask,
> indio_dev->num_channels) {
> - struct iio_chan_spec const *chan = indio_dev->channels + bit;
> + struct iio_chan_spec const *chan =
> + at91_adc_chan_get(indio_dev, bit);
>
> + if (!chan)
> + continue;
> st->buffer[i] = at91_adc_readl(st, chan->address);
> i++;
> }
^ permalink raw reply
* Re: [PATCH v3 03/11] iio: Add channel for Position Relative
From: Jonathan Cameron @ 2018-04-15 19:29 UTC (permalink / raw)
To: Eugen Hristev
Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov, robh
In-Reply-To: <1523350677-27106-4-git-send-email-eugen.hristev@microchip.com>
On Tue, 10 Apr 2018 11:57:49 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> Add new channel type for relative position on a pad.
>
> These type of analog sensor offers the position of a pen
> on a touchpad, and is represented as a voltage, which can be
> converted to a position on X and Y axis on the pad.
> The channel will hand the relative position on the pad in both directions.
>
> The channel can then be consumed by a touchscreen driver or
> read as-is for a raw indication of the touchpen on a touchpad.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> Changes in v2:
> - modified channel name to relative position as suggested.
> - modified kernel version to 4.18 (presumable)
>
> Documentation/ABI/testing/sysfs-bus-iio | 12 ++++++++++++
> drivers/iio/industrialio-core.c | 1 +
> include/uapi/linux/iio/types.h | 1 +
> tools/iio/iio_event_monitor.c | 2 ++
> 4 files changed, 16 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 6a5f34b..42a9287 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -190,6 +190,18 @@ Description:
> but should match other such assignments on device).
> Units after application of scale and offset are m/s^2.
>
> +What: /sys/bus/iio/devices/iio:deviceX/in_positionrelative_x_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_positionrelative_y_raw
> +KernelVersion: 4.18
> +Contact: linux-iio@vger.kernel.org
> +Description:
> + Relative position in direction x or y on a pad (may be
> + arbitrarily assigned but should match other such assignments on
> + device).
> + Units after application of scale and offset are milli percents
> + from the pad's size in both directions. Should be calibrated by
> + the consumer.
I know the milli percent comes form the humidity equivalent, but I wonder
if we are right to follow that. 10^-5 is a pretty random base unit (though
I got argued into it being a standard choice for humidity sensors IIRC...
What do people think? We could go with 1 for full range or just percent perhaps?
I'm not that fussed about staying consistent with humidity - we are unlikely
to end up with sensors doing both anytime soon so there shouldn't be
any confusion...
Jonathan
> +
> What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 19bdf3d..14bf3d24 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
> [IIO_COUNT] = "count",
> [IIO_INDEX] = "index",
> [IIO_GRAVITY] = "gravity",
> + [IIO_POSITIONRELATIVE] = "positionrelative",
> };
>
> static const char * const iio_modifier_names[] = {
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 4213cdf..033c7d2 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -44,6 +44,7 @@ enum iio_chan_type {
> IIO_COUNT,
> IIO_INDEX,
> IIO_GRAVITY,
> + IIO_POSITIONRELATIVE,
> };
>
> enum iio_modifier {
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index b61245e..148f69d 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
> [IIO_PH] = "ph",
> [IIO_UVINDEX] = "uvindex",
> [IIO_GRAVITY] = "gravity",
> + [IIO_POSITIONRELATIVE] = "positionrelative",
> };
>
> static const char * const iio_ev_type_text[] = {
> @@ -151,6 +152,7 @@ static bool event_is_known(struct iio_event_data *event)
> case IIO_PH:
> case IIO_UVINDEX:
> case IIO_GRAVITY:
> + case IIO_POSITIONRELATIVE:
> break;
> default:
> return false;
^ permalink raw reply
* Re: [PATCH v3 06/11] iio: inkern: add module put/get on iio dev module when requesting channels
From: Jonathan Cameron @ 2018-04-15 19:33 UTC (permalink / raw)
To: Eugen Hristev
Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov, robh
In-Reply-To: <1523350677-27106-7-git-send-email-eugen.hristev@microchip.com>
On Tue, 10 Apr 2018 11:57:52 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> When requesting channels for a particular consumer device,
> besides requesting the device (incrementing the reference counter), also
> do it for the driver module of the iio dev. This will avoid the situation
> where the producer IIO device can be removed and the consumer is still
> present in the kernel.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> drivers/iio/inkern.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index ec98790..68d9b87 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -11,6 +11,7 @@
> #include <linux/slab.h>
> #include <linux/mutex.h>
> #include <linux/of.h>
> +#include <linux/module.h>
>
> #include <linux/iio/iio.h>
> #include "iio_core.h"
> @@ -152,6 +153,7 @@ static int __of_iio_channel_get(struct iio_channel *channel,
> if (index < 0)
> goto err_put;
> channel->channel = &indio_dev->channels[index];
> + try_module_get(channel->indio_dev->driver_module);
And if it fails? (the module we are trying to get is going away...)
We should try and handle it I think. Be it by just erroring out of here.
>
> return 0;
>
> @@ -256,8 +258,10 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
> return chans;
>
> error_free_chans:
> - for (i = 0; i < mapind; i++)
> + for (i = 0; i < mapind; i++) {
> + module_put(chans[i].indio_dev->driver_module);
> iio_device_put(chans[i].indio_dev);
> + }
> kfree(chans);
> return ERR_PTR(ret);
> }
> @@ -351,6 +355,7 @@ void iio_channel_release(struct iio_channel *channel)
> {
> if (!channel)
> return;
> + module_put(channel->indio_dev->driver_module);
> iio_device_put(channel->indio_dev);
> kfree(channel);
> }
> @@ -482,6 +487,7 @@ void iio_channel_release_all(struct iio_channel *channels)
> struct iio_channel *chan = &channels[0];
>
> while (chan->indio_dev) {
> + module_put(chan->indio_dev->driver_module);
> iio_device_put(chan->indio_dev);
> chan++;
> }
^ permalink raw reply
* Re: [PATCH v3 07/11] iio: adc: at91-sama5d2_adc: add support for position and pressure channels
From: Jonathan Cameron @ 2018-04-15 19:45 UTC (permalink / raw)
To: Eugen Hristev
Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov, robh
In-Reply-To: <1523350677-27106-8-git-send-email-eugen.hristev@microchip.com>
On Tue, 10 Apr 2018 11:57:53 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> This implements the support for position and pressure for the included
> touchscreen support in the SAMA5D2 SOC ADC block.
> Two position channels are added and one for pressure.
> They can be read in raw format, or through a buffer.
> A normal use case is for a consumer driver to register a callback buffer
> for these channels.
> When the touchscreen channels are in the active scan mask,
> the driver will start the touchscreen sampling and push the data to the
> buffer.
>
> Some parts of this patch are based on initial original work by
> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
A few additional minor things I picked up on in this read.
Just trivial tidy ups or things I think could make the code
a little easier to read.
Looks good in general.
Jonathan
> ---
> Changes in v3:
> - prefix macros with AT91_SAMA5D2
> - reworked the x_pos and y_pos functions into a single one with two
> additional wrappers
> - reworked pressure report to have it grow naturally and not top down
> - fixed some checks regarding IIO_VOLTAGE as suggested
> - added a comment explaining some code in trigger handling
> - reworked the frequency get handler to use the saved value instead of
> reading it from the hardware.
> - added comment on deffered work queueing
> - pulled out INFO_RAW function into a separate utility function as suggested
> - added iio_dev ops structure at all times . The functions are needed in
> case we do not have a hardware trigger attached, but we want to use the
> consumer touchscreen driver, thus a callback buffer is attached. Then we still
> need to have buffer preenable and postdisable to configure the touch IRQs (etc.)
>
> Changes in v2:
> - the support is now based on callback buffer.
> drivers/iio/adc/at91-sama5d2_adc.c | 601 +++++++++++++++++++++++++++++++++----
> 1 file changed, 539 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 8729d65..5368464 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -102,14 +102,26 @@
> #define AT91_SAMA5D2_LCDR 0x20
> /* Interrupt Enable Register */
> #define AT91_SAMA5D2_IER 0x24
> +/* Interrupt Enable Register - TS X measurement ready */
> +#define AT91_SAMA5D2_IER_XRDY BIT(20)
> +/* Interrupt Enable Register - TS Y measurement ready */
> +#define AT91_SAMA5D2_IER_YRDY BIT(21)
> +/* Interrupt Enable Register - TS pressure measurement ready */
> +#define AT91_SAMA5D2_IER_PRDY BIT(22)
> /* Interrupt Enable Register - general overrun error */
> #define AT91_SAMA5D2_IER_GOVRE BIT(25)
> +/* Interrupt Enable Register - Pen detect */
> +#define AT91_SAMA5D2_IER_PEN BIT(29)
> +/* Interrupt Enable Register - No pen detect */
> +#define AT91_SAMA5D2_IER_NOPEN BIT(30)
> /* Interrupt Disable Register */
> #define AT91_SAMA5D2_IDR 0x28
> /* Interrupt Mask Register */
> #define AT91_SAMA5D2_IMR 0x2c
> /* Interrupt Status Register */
> #define AT91_SAMA5D2_ISR 0x30
> +/* Interrupt Status Register - Pen touching sense status */
> +#define AT91_SAMA5D2_ISR_PENS BIT(31)
> /* Last Channel Trigger Mode Register */
> #define AT91_SAMA5D2_LCTMR 0x34
> /* Last Channel Compare Window Register */
> @@ -131,8 +143,38 @@
> #define AT91_SAMA5D2_CDR0 0x50
> /* Analog Control Register */
> #define AT91_SAMA5D2_ACR 0x94
> +/* Analog Control Register - Pen detect sensitivity mask */
> +#define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(1, 0)
> +
> /* Touchscreen Mode Register */
> #define AT91_SAMA5D2_TSMR 0xb0
> +/* Touchscreen Mode Register - No touch mode */
> +#define AT91_SAMA5D2_TSMR_TSMODE_NONE 0
> +/* Touchscreen Mode Register - 4 wire screen, no pressure measurement */
> +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_NO_PRESS 1
> +/* Touchscreen Mode Register - 4 wire screen, pressure measurement */
> +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS 2
> +/* Touchscreen Mode Register - 5 wire screen */
> +#define AT91_SAMA5D2_TSMR_TSMODE_5WIRE 3
> +/* Touchscreen Mode Register - Average samples mask */
> +#define AT91_SAMA5D2_TSMR_TSAV_MASK GENMASK(5, 4)
> +/* Touchscreen Mode Register - Average samples */
> +#define AT91_SAMA5D2_TSMR_TSAV(x) ((x) << 4)
> +/* Touchscreen Mode Register - Touch/trigger frequency ratio mask */
> +#define AT91_SAMA5D2_TSMR_TSFREQ_MASK GENMASK(11, 8)
> +/* Touchscreen Mode Register - Touch/trigger frequency ratio */
> +#define AT91_SAMA5D2_TSMR_TSFREQ(x) ((x) << 8)
> +/* Touchscreen Mode Register - Pen Debounce Time mask */
> +#define AT91_SAMA5D2_TSMR_PENDBC_MASK GENMASK(31, 28)
> +/* Touchscreen Mode Register - Pen Debounce Time */
> +#define AT91_SAMA5D2_TSMR_PENDBC(x) ((x) << 28)
> +/* Touchscreen Mode Register - No DMA for touch measurements */
> +#define AT91_SAMA5D2_TSMR_NOTSDMA BIT(22)
> +/* Touchscreen Mode Register - Disable pen detection */
> +#define AT91_SAMA5D2_TSMR_PENDET_DIS (0 << 24)
> +/* Touchscreen Mode Register - Enable pen detection */
> +#define AT91_SAMA5D2_TSMR_PENDET_ENA BIT(24)
> +
> /* Touchscreen X Position Register */
> #define AT91_SAMA5D2_XPOSR 0xb4
> /* Touchscreen Y Position Register */
> @@ -151,6 +193,12 @@
> #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_FALL 2
> /* Trigger Mode external trigger any edge */
> #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_ANY 3
> +/* Trigger Mode internal periodic */
> +#define AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC 5
> +/* Trigger Mode - trigger period mask */
> +#define AT91_SAMA5D2_TRGR_TRGPER_MASK GENMASK(31, 16)
> +/* Trigger Mode - trigger period */
> +#define AT91_SAMA5D2_TRGR_TRGPER(x) ((x) << 16)
>
> /* Correction Select Register */
> #define AT91_SAMA5D2_COSR 0xd0
> @@ -169,6 +217,22 @@
> #define AT91_SAMA5D2_SINGLE_CHAN_CNT 12
> #define AT91_SAMA5D2_DIFF_CHAN_CNT 6
>
> +#define AT91_SAMA5D2_TIMESTAMP_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> + AT91_SAMA5D2_DIFF_CHAN_CNT + 1)
> +
> +#define AT91_SAMA5D2_TOUCH_X_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> + AT91_SAMA5D2_DIFF_CHAN_CNT * 2)
> +#define AT91_SAMA5D2_TOUCH_Y_CHAN_IDX (AT91_SAMA5D2_TOUCH_X_CHAN_IDX + 1)
> +#define AT91_SAMA5D2_TOUCH_P_CHAN_IDX (AT91_SAMA5D2_TOUCH_Y_CHAN_IDX + 1)
> +#define AT91_SAMA5D2_MAX_CHAN_IDX AT91_SAMA5D2_TOUCH_P_CHAN_IDX
> +
> +#define AT91_SAMA5D2_TOUCH_SAMPLE_PERIOD_US 2000 /* 2ms */
> +#define AT91_SAMA5D2_TOUCH_PEN_DETECT_DEBOUNCE_US 200
> +
> +#define AT91_SAMA5D2_XYZ_MASK GENMASK(11, 0)
> +
> +#define AT91_SAMA5D2_MAX_POS_BITS 12
> +
> /*
> * Maximum number of bytes to hold conversion from all channels
> * without the timestamp.
> @@ -222,6 +286,37 @@
> .indexed = 1, \
> }
>
> +#define AT91_SAMA5D2_CHAN_TOUCH(num, name, mod) \
> + { \
> + .type = IIO_POSITIONRELATIVE, \
> + .modified = 1, \
> + .channel = num, \
> + .channel2 = mod, \
> + .scan_index = num, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> + .datasheet_name = name, \
> + }
> +#define AT91_SAMA5D2_CHAN_PRESSURE(num, name) \
> + { \
> + .type = IIO_PRESSURE, \
> + .channel = num, \
> + .scan_index = num, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> + .datasheet_name = name, \
> + }
> +
> #define at91_adc_readl(st, reg) readl_relaxed(st->base + reg)
> #define at91_adc_writel(st, reg, val) writel_relaxed(val, st->base + reg)
>
> @@ -260,6 +355,22 @@ struct at91_adc_dma {
> s64 dma_ts;
> };
>
> +/**
> + * at91_adc_touch - at91-sama5d2 touchscreen information struct
> + * @sample_period_val: the value for periodic trigger interval
> + * @touching: is the pen touching the screen or not
> + * @x_pos: temporary placeholder for pressure computation
> + * @channels_bitmask: bitmask with the touchscreen channels enabled
> + * @workq: workqueue for buffer data pushing
> + */
> +struct at91_adc_touch {
> + u16 sample_period_val;
> + bool touching;
> + u16 x_pos;
> + unsigned long channels_bitmask;
> + struct work_struct workq;
> +};
> +
> struct at91_adc_state {
> void __iomem *base;
> int irq;
> @@ -267,6 +378,7 @@ struct at91_adc_state {
> struct regulator *reg;
> struct regulator *vref;
> int vref_uv;
> + unsigned int current_sample_rate;
> struct iio_trigger *trig;
> const struct at91_adc_trigger *selected_trig;
> const struct iio_chan_spec *chan;
> @@ -275,6 +387,7 @@ struct at91_adc_state {
> struct at91_adc_soc_info soc_info;
> wait_queue_head_t wq_data_available;
> struct at91_adc_dma dma_st;
> + struct at91_adc_touch touch_st;
> u16 buffer[AT91_BUFFER_MAX_HWORDS];
> /*
> * lock to prevent concurrent 'single conversion' requests through
> @@ -329,8 +442,10 @@ static const struct iio_chan_spec at91_adc_channels[] = {
> AT91_SAMA5D2_CHAN_DIFF(6, 7, 0x68),
> AT91_SAMA5D2_CHAN_DIFF(8, 9, 0x70),
> AT91_SAMA5D2_CHAN_DIFF(10, 11, 0x78),
> - IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_SINGLE_CHAN_CNT
> - + AT91_SAMA5D2_DIFF_CHAN_CNT + 1),
> + IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_TIMESTAMP_CHAN_IDX),
> + AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_X_CHAN_IDX, "x", IIO_MOD_X),
> + AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, "y", IIO_MOD_Y),
> + AT91_SAMA5D2_CHAN_PRESSURE(AT91_SAMA5D2_TOUCH_P_CHAN_IDX, "pressure"),
> };
>
> static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan)
> @@ -354,6 +469,160 @@ at91_adc_chan_get(struct iio_dev *indio_dev, int chan)
> return indio_dev->channels + index;
> }
>
> +static inline int at91_adc_of_xlate(struct iio_dev *indio_dev,
> + const struct of_phandle_args *iiospec)
> +{
> + return at91_adc_chan_xlate(indio_dev, iiospec->args[0]);
> +}
> +
> +static int at91_adc_configure_touch(struct at91_adc_state *st, bool state)
> +{
> + u32 clk_khz = st->current_sample_rate / 1000;
> + int i = 0;
> + u16 pendbc;
> + u32 tsmr, acr;
> +
> + if (!state) {
> + /* disabling touch IRQs and setting mode to no touch enabled */
> + at91_adc_writel(st, AT91_SAMA5D2_IDR,
> + AT91_SAMA5D2_IER_PEN | AT91_SAMA5D2_IER_NOPEN);
> + at91_adc_writel(st, AT91_SAMA5D2_TSMR, 0);
> + return 0;
> + }
> + /*
> + * debounce time is in microseconds, we need it in milliseconds to
> + * multiply with kilohertz, so, divide by 1000, but after the multiply.
> + * round up to make sure pendbc is at least 1
> + */
> + pendbc = round_up(AT91_SAMA5D2_TOUCH_PEN_DETECT_DEBOUNCE_US *
> + clk_khz / 1000, 1);
> +
> + /* get the required exponent */
> + while (pendbc >> i++)
> + ;
> +
> + pendbc = i;
> +
> + tsmr = AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS;
> +
> + tsmr |= AT91_SAMA5D2_TSMR_TSAV(2) & AT91_SAMA5D2_TSMR_TSAV_MASK;
> + tsmr |= AT91_SAMA5D2_TSMR_PENDBC(pendbc) &
> + AT91_SAMA5D2_TSMR_PENDBC_MASK;
> + tsmr |= AT91_SAMA5D2_TSMR_NOTSDMA;
> + tsmr |= AT91_SAMA5D2_TSMR_PENDET_ENA;
> + tsmr |= AT91_SAMA5D2_TSMR_TSFREQ(2) & AT91_SAMA5D2_TSMR_TSFREQ_MASK;
> +
> + at91_adc_writel(st, AT91_SAMA5D2_TSMR, tsmr);
> +
> + acr = at91_adc_readl(st, AT91_SAMA5D2_ACR);
> + acr &= ~AT91_SAMA5D2_ACR_PENDETSENS_MASK;
> + acr |= 0x02 & AT91_SAMA5D2_ACR_PENDETSENS_MASK;
> + at91_adc_writel(st, AT91_SAMA5D2_ACR, acr);
> +
> + /* Sample Period Time = (TRGPER + 1) / ADCClock */
> + st->touch_st.sample_period_val =
> + round_up((AT91_SAMA5D2_TOUCH_SAMPLE_PERIOD_US *
> + clk_khz / 1000) - 1, 1);
> + /* enable pen detect IRQ */
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
> +
> + return 0;
> +}
> +
> +static u16 at91_adc_touch_pos(struct at91_adc_state *st, int reg)
> +{
> + u32 val;
> + u32 scale, result, pos;
> +
> + /*
> + * to obtain the actual position we must divide by scale
> + * and multiply with max, where
> + * max = 2^AT91_SAMA5D2_MAX_POS_BITS - 1
> + */
> + /* first half of register is the x or y, second half is the scale */
> + val = at91_adc_readl(st, reg);
> + if (!val)
> + dev_dbg(&iio_priv_to_dev(st)->dev, "pos is 0\n");
> +
> + pos = val & AT91_SAMA5D2_XYZ_MASK;
> + result = (pos << AT91_SAMA5D2_MAX_POS_BITS) - pos;
> + scale = (val >> 16) & AT91_SAMA5D2_XYZ_MASK;
> + if (scale == 0) {
> + dev_err(&iio_priv_to_dev(st)->dev, "scale is 0\n");
> + return 0;
> + }
> + result /= scale;
> +
> + return result;
> +}
> +
> +static u16 at91_adc_touch_x_pos(struct at91_adc_state *st)
> +{
> + st->touch_st.x_pos = at91_adc_touch_pos(st, AT91_SAMA5D2_XPOSR);
> + return st->touch_st.x_pos;
> +}
> +
> +static u16 at91_adc_touch_y_pos(struct at91_adc_state *st)
> +{
> + return at91_adc_touch_pos(st, AT91_SAMA5D2_YPOSR);
> +}
> +
> +static u16 at91_adc_touch_pressure(struct at91_adc_state *st)
> +{
> + u32 val;
> + u32 z1, z2;
> + u32 pres;
> + u32 rxp = 1;
> + u32 factor = 1000;
> +
> + /* calculate the pressure */
> + val = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
> + z1 = val & AT91_SAMA5D2_XYZ_MASK;
> + z2 = (val >> 16) & AT91_SAMA5D2_XYZ_MASK;
> +
> + if (z1 != 0)
> + pres = rxp * (st->touch_st.x_pos * factor / 1024) *
> + (z2 * factor / z1 - factor) /
> + factor;
> + else
> + pres = 0xFFFF; /* no pen contact */
> +
> + /*
> + * The pressure from device grows down, minimum is 0xFFFF, maximum 0x0.
> + * We compute it this way, but let's return it in the expected way,
> + * growing from 0 to 0xFFFF.
> + */
> + return 0xFFFF - pres;
> +}
> +
> +static int at91_adc_read_position(struct at91_adc_state *st, int chan, u16 *val)
> +{
> + *val = 0;
> + if (!st->touch_st.touching)
> + return -ENODATA;
> + if (chan == AT91_SAMA5D2_TOUCH_X_CHAN_IDX)
> + *val = at91_adc_touch_x_pos(st);
> + else if (chan == AT91_SAMA5D2_TOUCH_Y_CHAN_IDX)
> + *val = at91_adc_touch_y_pos(st);
> + else
> + return -ENODATA;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int at91_adc_read_pressure(struct at91_adc_state *st, int chan, u16 *val)
> +{
> + *val = 0;
> + if (!st->touch_st.touching)
> + return -ENODATA;
> + if (chan == AT91_SAMA5D2_TOUCH_P_CHAN_IDX)
> + *val = at91_adc_touch_pressure(st);
> + else
> + return -ENODATA;
> +
> + return IIO_VAL_INT;
> +}
> +
> static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> {
> struct iio_dev *indio = iio_trigger_get_drvdata(trig);
> @@ -375,6 +644,10 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
>
> if (!chan)
> continue;
> + /* these channel types cannot be handled by this trigger */
> + if (chan->type == IIO_POSITIONRELATIVE ||
> + chan->type == IIO_PRESSURE)
> + continue;
A blank line here might help readability.
> if (state) {
> at91_adc_writel(st, AT91_SAMA5D2_CHER,
> BIT(chan->channel));
> @@ -520,7 +793,21 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev)
> static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> {
> int ret;
> + struct at91_adc_state *st = iio_priv(indio_dev);
>
> + /* check if we are enabling triggered buffer or the touchscreen */
> + if (bitmap_subset(indio_dev->active_scan_mask,
> + &st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) {
> + /* touchscreen enabling */
> + at91_adc_configure_touch(st, true);
> + return 0;
> + }
> + /* if trigger is not hardware, nothing to do here */
> + if (!st->selected_trig->hw_trig)
> + return 0;
> +
> + /* we continue with the triggered buffer */
> ret = at91_adc_dma_start(indio_dev);
> if (ret) {
> dev_err(&indio_dev->dev, "buffer postenable failed\n");
> @@ -536,6 +823,19 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> int ret;
> u8 bit;
>
> + /* check if we are disabling triggered buffer or the touchscreen */
> + if (bitmap_subset(indio_dev->active_scan_mask,
> + &st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) {
> + /* touchscreen disable */
> + at91_adc_configure_touch(st, false);
> + return 0;
> + }
> + /* if trigger is not hardware, nothing to do here */
> + if (!st->selected_trig->hw_trig)
> + return 0;
> +
> + /* continue with the triggered buffer */
> ret = iio_triggered_buffer_predisable(indio_dev);
> if (ret < 0)
> dev_err(&indio_dev->dev, "buffer predisable failed\n");
> @@ -558,6 +858,10 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
>
> if (!chan)
> continue;
> + /* these channel types are virtual, no need to do anything */
> + if (chan->type == IIO_POSITIONRELATIVE ||
> + chan->type == IIO_PRESSURE)
> + continue;
> if (st->dma_st.dma_chan)
> at91_adc_readl(st, chan->address);
> }
> @@ -622,7 +926,22 @@ static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
>
> if (!chan)
> continue;
> - st->buffer[i] = at91_adc_readl(st, chan->address);
> + /*
> + * Our external trigger only supports the voltage channels.
> + * In case someone requested a different type of channel
> + * just put zeroes to buffer.
> + * This should not happen because we check the scan mode
> + * and scan mask when we enable the buffer, and we don't allow
> + * the buffer to start with a mixed mask (voltage and something
> + * else).
> + * Thus, emit a warning.
> + */
> + if (chan->type == IIO_VOLTAGE) {
> + st->buffer[i] = at91_adc_readl(st, chan->address);
> + } else {
> + st->buffer[i] = 0;
> + WARN(true, "This trigger cannot handle this type of channel");
> + }
> i++;
> }
> iio_push_to_buffers_with_timestamp(indio_dev, st->buffer,
> @@ -688,9 +1007,20 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>
> static int at91_adc_buffer_init(struct iio_dev *indio)
> {
> - return devm_iio_triggered_buffer_setup(&indio->dev, indio,
> + struct at91_adc_state *st = iio_priv(indio);
> +
> + if (st->selected_trig->hw_trig) {
> + return devm_iio_triggered_buffer_setup(&indio->dev, indio,
> &iio_pollfunc_store_time,
> &at91_adc_trigger_handler, &at91_buffer_setup_ops);
> + }
> + /*
> + * we need to prepare the buffer ops in case we will get
> + * another buffer attached (like a callback buffer for the touchscreen)
> + */
> + indio->setup_ops = &at91_buffer_setup_ops;
> +
> + return 0;
> }
>
> static unsigned at91_adc_startup_time(unsigned startup_time_min,
> @@ -736,19 +1066,83 @@ static void at91_adc_setup_samp_freq(struct at91_adc_state *st, unsigned freq)
>
> dev_dbg(&indio_dev->dev, "freq: %u, startup: %u, prescal: %u\n",
> freq, startup, prescal);
> + st->current_sample_rate = freq;
> }
>
> -static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
> +static inline unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
> {
> - unsigned f_adc, f_per = clk_get_rate(st->per_clk);
> - unsigned mr, prescal;
> + return st->current_sample_rate;
> +}
>
> - mr = at91_adc_readl(st, AT91_SAMA5D2_MR);
> - prescal = (mr >> AT91_SAMA5D2_MR_PRESCAL_OFFSET)
> - & AT91_SAMA5D2_MR_PRESCAL_MAX;
> - f_adc = f_per / (2 * (prescal + 1));
> +static void at91_adc_touch_data_handler(struct iio_dev *indio_dev)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + u8 bit;
> + u16 val;
> + int i = 0;
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + AT91_SAMA5D2_MAX_CHAN_IDX + 1) {
> + struct iio_chan_spec const *chan =
> + at91_adc_chan_get(indio_dev, bit);
>
> - return f_adc;
> + if (chan->type == IIO_POSITIONRELATIVE)
> + at91_adc_read_position(st, chan->channel, &val);
> + else if (chan->type == IIO_PRESSURE)
> + at91_adc_read_pressure(st, chan->channel, &val);
> + else
> + continue;
> + st->buffer[i] = val;
> + i++;
> + }
> + /*
> + * Schedule work to push to buffers.
> + * This is intended to push to the callback buffer that another driver
> + * registered. We are still in a handler from our IRQ. If we push
> + * directly, it means the other driver has it's callback called
> + * from our IRQ context. Which is something we better avoid.
> + * Let's schedule it after our IRQ is completed.
> + */
> + schedule_work(&st->touch_st.workq);
> +}
> +
> +static void at91_adc_pen_detect_interrupt(struct at91_adc_state *st)
> +{
> + at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_PEN);
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_NOPEN |
> + AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> + AT91_SAMA5D2_IER_PRDY);
> + at91_adc_writel(st, AT91_SAMA5D2_TRGR,
> + AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC |
> + AT91_SAMA5D2_TRGR_TRGPER(st->touch_st.sample_period_val));
> + st->touch_st.touching = true;
> +}
> +
> +static void at91_adc_no_pen_detect_interrupt(struct at91_adc_state *st)
> +{
> + struct iio_dev *indio_dev = iio_priv_to_dev(st);
> +
> + at91_adc_writel(st, AT91_SAMA5D2_TRGR,
> + AT91_SAMA5D2_TRGR_TRGMOD_NO_TRIGGER);
> + at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_NOPEN |
> + AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> + AT91_SAMA5D2_IER_PRDY);
> + st->touch_st.touching = false;
> +
> + at91_adc_touch_data_handler(indio_dev);
> +
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
> +}
> +
> +static void at91_adc_workq_handler(struct work_struct *workq)
> +{
> + struct at91_adc_touch *touch_st = container_of(workq,
> + struct at91_adc_touch, workq);
> + struct at91_adc_state *st = container_of(touch_st,
> + struct at91_adc_state, touch_st);
> + struct iio_dev *indio_dev = iio_priv_to_dev(st);
> +
> + iio_push_to_buffers(indio_dev, st->buffer);
> }
>
> static irqreturn_t at91_adc_interrupt(int irq, void *private)
> @@ -757,17 +1151,39 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
> struct at91_adc_state *st = iio_priv(indio);
> u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR);
> u32 imr = at91_adc_readl(st, AT91_SAMA5D2_IMR);
> + u32 rdy_mask = AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> + AT91_SAMA5D2_IER_PRDY;
>
> if (!(status & imr))
> return IRQ_NONE;
> -
> - if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> + if (status & AT91_SAMA5D2_IER_PEN) {
> + /* pen detected IRQ */
> + at91_adc_pen_detect_interrupt(st);
> + } else if ((status & AT91_SAMA5D2_IER_NOPEN)) {
> + /* nopen detected IRQ */
> + at91_adc_no_pen_detect_interrupt(st);
> + } else if ((status & AT91_SAMA5D2_ISR_PENS) &&
> + ((status & rdy_mask) == rdy_mask)) {
> + /* periodic trigger IRQ - during pen sense */
> + at91_adc_touch_data_handler(indio);
> + } else if (status & AT91_SAMA5D2_ISR_PENS) {
> + /*
> + * touching, but the measurements are not ready yet.
> + * read and ignore.
> + */
> + status = at91_adc_readl(st, AT91_SAMA5D2_XPOSR);
> + status = at91_adc_readl(st, AT91_SAMA5D2_YPOSR);
> + status = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
> + } else if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> + /* triggered buffer without DMA */
> disable_irq_nosync(irq);
> iio_trigger_poll(indio->trig);
> } else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) {
> + /* triggered buffer with DMA - should not happen */
> disable_irq_nosync(irq);
> WARN(true, "Unexpected irq occurred\n");
> } else if (!iio_buffer_enabled(indio)) {
> + /* software requested conversion */
> st->conversion_value = at91_adc_readl(st, st->chan->address);
> st->conversion_done = true;
> wake_up_interruptible(&st->wq_data_available);
> @@ -775,58 +1191,81 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> -static int at91_adc_read_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int *val, int *val2, long mask)
> +static int at91_adc_read_info_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val)
> {
> struct at91_adc_state *st = iio_priv(indio_dev);
> u32 cor = 0;
> int ret;
>
> - switch (mask) {
> - case IIO_CHAN_INFO_RAW:
> - /* we cannot use software trigger if hw trigger enabled */
> - ret = iio_device_claim_direct_mode(indio_dev);
> - if (ret)
> - return ret;
> - mutex_lock(&st->lock);
> + /*
> + * we cannot use software trigger or touchscreen
> + * if external trigger is enabled
> + */
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> + mutex_lock(&st->lock);
>
> - st->chan = chan;
> + if (chan->type == IIO_POSITIONRELATIVE) {
> + ret = at91_adc_read_position(st, chan->channel,
> + (u16 *)val);
> + goto at91_adc_read_info_raw_exit;
I know it is more lines of code, but I think I'd prefer to
get rid of this long goto as it's not an error case.
I would suggest moving the mutex inside this if statement
(and the next one) and do the lock / unlock / return all
within the if. For the other path, take the mutex after
these if statements.
> + }
> + if (chan->type == IIO_PRESSURE) {
> + ret = at91_adc_read_pressure(st, chan->channel,
> + (u16 *)val);
> + goto at91_adc_read_info_raw_exit;
> + }
>
> - if (chan->differential)
> - cor = (BIT(chan->channel) | BIT(chan->channel2)) <<
> - AT91_SAMA5D2_COR_DIFF_OFFSET;
> -
> - at91_adc_writel(st, AT91_SAMA5D2_COR, cor);
> - at91_adc_writel(st, AT91_SAMA5D2_CHER, BIT(chan->channel));
> - at91_adc_writel(st, AT91_SAMA5D2_IER, BIT(chan->channel));
> - at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_START);
> -
> - ret = wait_event_interruptible_timeout(st->wq_data_available,
> - st->conversion_done,
> - msecs_to_jiffies(1000));
> - if (ret == 0)
> - ret = -ETIMEDOUT;
> -
> - if (ret > 0) {
> - *val = st->conversion_value;
> - if (chan->scan_type.sign == 's')
> - *val = sign_extend32(*val, 11);
> - ret = IIO_VAL_INT;
> - st->conversion_done = false;
> - }
> + st->chan = chan;
> +
> + if (chan->differential)
> + cor = (BIT(chan->channel) | BIT(chan->channel2)) <<
> + AT91_SAMA5D2_COR_DIFF_OFFSET;
> +
> + at91_adc_writel(st, AT91_SAMA5D2_COR, cor);
> + at91_adc_writel(st, AT91_SAMA5D2_CHER, BIT(chan->channel));
> + at91_adc_writel(st, AT91_SAMA5D2_IER, BIT(chan->channel));
> + at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_START);
> +
> + ret = wait_event_interruptible_timeout(st->wq_data_available,
> + st->conversion_done,
> + msecs_to_jiffies(1000));
> + if (ret == 0)
> + ret = -ETIMEDOUT;
> +
> + if (ret > 0) {
> + *val = st->conversion_value;
> + if (chan->scan_type.sign == 's')
> + *val = sign_extend32(*val, 11);
> + ret = IIO_VAL_INT;
> + st->conversion_done = false;
> + }
> +
> + at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
> + at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
>
> - at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
> - at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
> + /* Needed to ACK the DRDY interruption */
> + at91_adc_readl(st, AT91_SAMA5D2_LCDR);
>
> - /* Needed to ACK the DRDY interruption */
> - at91_adc_readl(st, AT91_SAMA5D2_LCDR);
> +at91_adc_read_info_raw_exit:
>
> - mutex_unlock(&st->lock);
> + mutex_unlock(&st->lock);
>
> - iio_device_release_direct_mode(indio_dev);
> - return ret;
> + iio_device_release_direct_mode(indio_dev);
> + return ret;
> +}
>
> +static int at91_adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return at91_adc_read_info_raw(indio_dev, chan, val);
> case IIO_CHAN_INFO_SCALE:
> *val = st->vref_uv / 1000;
> if (chan->differential)
> @@ -974,9 +1413,29 @@ static int at91_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> return 0;
> }
>
> +static int at91_adc_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> +
> + if (bitmap_subset(scan_mask, &st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_MAX_CHAN_IDX + 1))
> + return 0;
> + /*
> + * if the new bitmap is a combination of touchscreen and regular
> + * channels, then we are not fine
> + */
> + if (bitmap_intersects(&st->touch_st.channels_bitmask, scan_mask,
> + AT91_SAMA5D2_MAX_CHAN_IDX + 1))
> + return -EBUSY;
> + return 0;
> +}
> +
> static const struct iio_info at91_adc_info = {
> .read_raw = &at91_adc_read_raw,
> .write_raw = &at91_adc_write_raw,
> + .update_scan_mode = &at91_adc_update_scan_mode,
> + .of_xlate = &at91_adc_of_xlate,
> .hwfifo_set_watermark = &at91_adc_set_watermark,
> };
>
> @@ -1044,13 +1503,20 @@ static int at91_adc_probe(struct platform_device *pdev)
>
> indio_dev->dev.parent = &pdev->dev;
> indio_dev->name = dev_name(&pdev->dev);
> - indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> indio_dev->info = &at91_adc_info;
> indio_dev->channels = at91_adc_channels;
> indio_dev->num_channels = ARRAY_SIZE(at91_adc_channels);
>
> st = iio_priv(indio_dev);
>
> + bitmap_set(&st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_TOUCH_X_CHAN_IDX, 1);
> + bitmap_set(&st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, 1);
> + bitmap_set(&st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_TOUCH_P_CHAN_IDX, 1);
> +
> ret = of_property_read_u32(pdev->dev.of_node,
> "atmel,min-sample-rate-hz",
> &st->soc_info.min_sample_rate);
> @@ -1100,6 +1566,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>
> init_waitqueue_head(&st->wq_data_available);
> mutex_init(&st->lock);
> + INIT_WORK(&st->touch_st.workq, at91_adc_workq_handler);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res)
> @@ -1159,13 +1626,13 @@ static int at91_adc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> - if (st->selected_trig->hw_trig) {
> - ret = at91_adc_buffer_init(indio_dev);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
> - goto per_clk_disable_unprepare;
> - }
> + ret = at91_adc_buffer_init(indio_dev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
> + goto per_clk_disable_unprepare;
> + }
>
> + if (st->selected_trig->hw_trig) {
> ret = at91_adc_trigger_init(indio_dev);
> if (ret < 0) {
> dev_err(&pdev->dev, "couldn't setup the triggers.\n");
> @@ -1272,8 +1739,18 @@ static __maybe_unused int at91_adc_resume(struct device *dev)
> at91_adc_hw_init(st);
>
> /* reconfiguring trigger hardware state */
> - if (iio_buffer_enabled(indio_dev))
> + if (!iio_buffer_enabled(indio_dev))
> + return 0;
> +
> + /* check if we are enabling triggered buffer or the touchscreen */
> + if (bitmap_subset(indio_dev->active_scan_mask,
> + &st->touch_st.channels_bitmask,
> + AT91_SAMA5D2_MAX_CHAN_IDX + 1)) {
> + /* touchscreen enabling */
> + at91_adc_configure_touch(st, true);
> + } else {
> at91_adc_configure_trigger(st->trig, true);
> + }
>
> return 0;
>
^ permalink raw reply
* Re: [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
From: Andrzej Hajda @ 2018-04-16 7:19 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input@vger.kernel.org, orjan.eide, wxt, jeffy.chen,
linux-arm-kernel, mark.yao, wzz, hl, jingoohan1, Dmitry Torokhov,
sw0312.kim, linux-kernel, kyungmin.park, Kristian H. Kristensen,
kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-25-enric.balletbo@collabora.com>
+CC: linux-input list and maintainer
On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: "Kristian H. Kristensen" <hoegsberg@google.com>
>
> To improve PSR exit latency, we speculatively start exiting when we
> receive input events. Occasionally, this may lead to false positives,
> but most of the time we get a head start on coming out of PSR. Depending
> on how userspace takes to produce a new frame in response to the event,
> this can completely hide the exit latency. In case of Chrome OS, we
> typically get the input notifier 50ms or more before the dirty_fb
> triggered exit.
As I see from the code below, you just need notification from input
subsystem on user activity.
Maybe there is some simpler notification mechanism for such things?
If not, maybe such helper should be created in input subsystem, I
suppose it could be reused in other places as well.
Regards
Andrzej
>
> Signed-off-by: Kristian H. Kristensen <hoegsberg@google.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 134 ++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index 9376f4396b6b..a107845ba97c 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -12,6 +12,8 @@
> * GNU General Public License for more details.
> */
>
> +#include <linux/input.h>
> +
> #include <drm/drmP.h>
> #include <drm/drm_crtc_helper.h>
>
> @@ -35,6 +37,9 @@ struct psr_drv {
> enum psr_state state;
>
> struct delayed_work flush_work;
> + struct work_struct disable_work;
> +
> + struct input_handler input_handler;
>
> int (*set)(struct drm_encoder *encoder, bool enable);
> };
> @@ -133,6 +138,18 @@ static void psr_flush_handler(struct work_struct *work)
> mutex_unlock(&psr->lock);
> }
>
> +static void psr_disable_handler(struct work_struct *work)
> +{
> + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work);
> +
> + /* If the state has changed since we initiated the flush, do nothing */
> + mutex_lock(&psr->lock);
> + if (psr->state == PSR_ENABLE)
> + psr_set_state_locked(psr, PSR_FLUSH);
> + mutex_unlock(&psr->lock);
> + mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
> +}
> +
> /**
> * rockchip_drm_psr_activate - activate PSR on the given pipe
> * @encoder: encoder to obtain the PSR encoder
> @@ -173,6 +190,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
> psr->active = false;
> mutex_unlock(&psr->lock);
> cancel_delayed_work_sync(&psr->flush_work);
> + cancel_work_sync(&psr->disable_work);
>
> return 0;
> }
> @@ -226,6 +244,95 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev)
> }
> EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
>
> +static void psr_input_event(struct input_handle *handle,
> + unsigned int type, unsigned int code,
> + int value)
> +{
> + struct psr_drv *psr = handle->handler->private;
> +
> + schedule_work(&psr->disable_work);
> +}
> +
> +static int psr_input_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id)
> +{
> + struct input_handle *handle;
> + int error;
> +
> + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> + if (!handle)
> + return -ENOMEM;
> +
> + handle->dev = dev;
> + handle->handler = handler;
> + handle->name = "rockchip-psr";
> +
> + error = input_register_handle(handle);
> + if (error)
> + goto err2;
> +
> + error = input_open_device(handle);
> + if (error)
> + goto err1;
> +
> + return 0;
> +
> +err1:
> + input_unregister_handle(handle);
> +err2:
> + kfree(handle);
> + return error;
> +}
> +
> +static void psr_input_disconnect(struct input_handle *handle)
> +{
> + input_close_device(handle);
> + input_unregister_handle(handle);
> + kfree(handle);
> +}
> +
> +/* Same device ids as cpu-boost */
> +static const struct input_device_id psr_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> + INPUT_DEVICE_ID_MATCH_ABSBIT,
> + .evbit = { BIT_MASK(EV_ABS) },
> + .absbit = { [BIT_WORD(ABS_MT_POSITION_X)] =
> + BIT_MASK(ABS_MT_POSITION_X) |
> + BIT_MASK(ABS_MT_POSITION_Y) },
> + }, /* multi-touch touchscreen */
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> + .evbit = { BIT_MASK(EV_ABS) },
> + .absbit = { [BIT_WORD(ABS_X)] = BIT_MASK(ABS_X) }
> +
> + }, /* stylus or joystick device */
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> + .evbit = { BIT_MASK(EV_KEY) },
> + .keybit = { [BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) },
> + }, /* pointer (e.g. trackpad, mouse) */
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> + .evbit = { BIT_MASK(EV_KEY) },
> + .keybit = { [BIT_WORD(KEY_ESC)] = BIT_MASK(KEY_ESC) },
> + }, /* keyboard */
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> + INPUT_DEVICE_ID_MATCH_KEYBIT,
> + .evbit = { BIT_MASK(EV_KEY) },
> + .keybit = {[BIT_WORD(BTN_JOYSTICK)] = BIT_MASK(BTN_JOYSTICK) },
> + }, /* joysticks not caught by ABS_X above */
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> + INPUT_DEVICE_ID_MATCH_KEYBIT,
> + .evbit = { BIT_MASK(EV_KEY) },
> + .keybit = { [BIT_WORD(BTN_GAMEPAD)] = BIT_MASK(BTN_GAMEPAD) },
> + }, /* gamepad */
> + { },
> +};
> +
> /**
> * rockchip_drm_psr_register - register encoder to psr driver
> * @encoder: encoder that obtain the PSR function
> @@ -239,6 +346,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> {
> struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
> struct psr_drv *psr;
> + int error;
>
> if (!encoder || !psr_set)
> return -EINVAL;
> @@ -248,6 +356,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> return -ENOMEM;
>
> INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler);
> + INIT_WORK(&psr->disable_work, psr_disable_handler);
> mutex_init(&psr->lock);
>
> psr->active = true;
> @@ -255,11 +364,33 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> psr->encoder = encoder;
> psr->set = psr_set;
>
> + psr->input_handler.event = psr_input_event;
> + psr->input_handler.connect = psr_input_connect;
> + psr->input_handler.disconnect = psr_input_disconnect;
> + psr->input_handler.name =
> + kasprintf(GFP_KERNEL, "rockchip-psr-%s", encoder->name);
> + if (!psr->input_handler.name) {
> + error = -ENOMEM;
> + goto err2;
> + }
> + psr->input_handler.id_table = psr_ids;
> + psr->input_handler.private = psr;
> +
> + error = input_register_handler(&psr->input_handler);
> + if (error)
> + goto err1;
> +
> mutex_lock(&drm_drv->psr_list_lock);
> list_add_tail(&psr->list, &drm_drv->psr_list);
> mutex_unlock(&drm_drv->psr_list_lock);
>
> return 0;
> +
> + err1:
> + kfree(psr->input_handler.name);
> + err2:
> + kfree(psr);
> + return error;
> }
> EXPORT_SYMBOL(rockchip_drm_psr_register);
>
> @@ -279,8 +410,11 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder)
> mutex_lock(&drm_drv->psr_list_lock);
> list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) {
> if (psr->encoder == encoder) {
> + input_unregister_handler(&psr->input_handler);
> cancel_delayed_work_sync(&psr->flush_work);
> + cancel_work_sync(&psr->disable_work);
> list_del(&psr->list);
> + kfree(psr->input_handler.name);
> kfree(psr);
> }
> }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v6 25/30] drm/rockchip: Cancel PSR enable work before changing the state
From: Andrzej Hajda @ 2018-04-16 8:55 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-26-enric.balletbo@collabora.com>
On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> If we change the state first and reschedule later, we might have the
> work executed according to previous scheduled time and end up with PSR
> re-enabled instantly. Let's cancel the work before changing the state.
>
> While at it, consolidate psr_disable_handler() to just call
> rockchip_drm_do_flush(), as they are both supposed to do the same.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index a107845ba97c..c8655e625ba2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -138,18 +138,6 @@ static void psr_flush_handler(struct work_struct *work)
> mutex_unlock(&psr->lock);
> }
>
> -static void psr_disable_handler(struct work_struct *work)
> -{
> - struct psr_drv *psr = container_of(work, struct psr_drv, disable_work);
> -
> - /* If the state has changed since we initiated the flush, do nothing */
> - mutex_lock(&psr->lock);
> - if (psr->state == PSR_ENABLE)
> - psr_set_state_locked(psr, PSR_FLUSH);
> - mutex_unlock(&psr->lock);
> - mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
> -}
> -
> /**
> * rockchip_drm_psr_activate - activate PSR on the given pipe
> * @encoder: encoder to obtain the PSR encoder
> @@ -198,6 +186,7 @@ EXPORT_SYMBOL(rockchip_drm_psr_deactivate);
>
> static void rockchip_drm_do_flush(struct psr_drv *psr)
> {
> + cancel_delayed_work_sync(&psr->flush_work);
> psr_set_state(psr, PSR_FLUSH);
> mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
I guess you can change it to schedule_delayed_work then.
Anyway:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> }
> @@ -244,6 +233,13 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev)
> }
> EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
>
> +static void psr_disable_handler(struct work_struct *work)
> +{
> + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work);
> +
> + rockchip_drm_do_flush(psr);
> +}
> +
> static void psr_input_event(struct input_handle *handle,
> unsigned int type, unsigned int code,
> int value)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v6 26/30] drm/rockchip: psr: Avoid redundant calls to .set() callback
From: Andrzej Hajda @ 2018-04-16 9:06 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-27-enric.balletbo@collabora.com>
On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> The first time after we call rockchip_drm_do_flush() after
> rockchip_drm_psr_register(), we go from PSR_DISABLE to PSR_FLUSH. The
> difference between PSR_DISABLE and PSR_FLUSH is whether or not we have a
> delayed work pending - PSR is off in either state. However
> psr_set_state() only catches the transition from PSR_FLUSH to
> PSR_DISABLE (which never happens), while going from PSR_DISABLE to
> PSR_FLUSH triggers a call to psr->set() to disable PSR while it's
> already disabled. This triggers the eDP PHY power-on sequence without
> being shut down first and this seems to occasionally leave the encoder
> unable to later enable PSR. Let's just simplify the state machine and
> simply consider PSR_DISABLE and PSR_FLUSH the same state. This lets us
> represent the hardware state by a simple boolean called "enabled" and,
> while at it, rename the misleading "active" boolean to "inhibit", which
> represents the purpose much better.
This phrase has no corresponding part in the patch.
>
> Also, userspace can (and does) trigger the rockchip_drm_do_flush() path
> from drmModeDirtyFB() at any time, whether or the encoder is active. If
> no mode is set, we call into analogix_dp_psr_set() which returns -EINVAL
> because encoder->crtc is NULL. Avoid this by starting out with
> psr->allowed set to false.
ditto
>
> Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Original author should be first, 1st line of the patch "From:...."
suggests it should be Tomasz.
Regards
Andrzej
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 79 +++++++++--------------------
> 1 file changed, 23 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index c8655e625ba2..448c5fde241c 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -22,19 +22,13 @@
>
> #define PSR_FLUSH_TIMEOUT_MS 100
>
> -enum psr_state {
> - PSR_FLUSH,
> - PSR_ENABLE,
> - PSR_DISABLE,
> -};
> -
> struct psr_drv {
> struct list_head list;
> struct drm_encoder *encoder;
>
> struct mutex lock;
> bool active;
> - enum psr_state state;
> + bool enabled;
>
> struct delayed_work flush_work;
> struct work_struct disable_work;
> @@ -78,52 +72,22 @@ static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder)
> return psr;
> }
>
> -static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state)
> +static int psr_set_state_locked(struct psr_drv *psr, bool enable)
> {
> - /*
> - * Allowed finite state machine:
> - *
> - * PSR_ENABLE < = = = = = > PSR_FLUSH
> - * | ^ |
> - * | | |
> - * v | |
> - * PSR_DISABLE < - - - - - - - - -
> - */
> - if (state == psr->state || !psr->active)
> - return;
> -
> - /* Already disabled in flush, change the state, but not the hardware */
> - if (state == PSR_DISABLE && psr->state == PSR_FLUSH) {
> - psr->state = state;
> - return;
> - }
> + int ret;
>
> - /* Actually commit the state change to hardware */
> - switch (state) {
> - case PSR_ENABLE:
> - if (psr->set(psr->encoder, true))
> - return;
> - break;
> -
> - case PSR_DISABLE:
> - case PSR_FLUSH:
> - if (psr->set(psr->encoder, false))
> - return;
> - break;
> -
> - default:
> - pr_err("%s: Unknown state %d\n", __func__, state);
> - return;
> - }
> + if (!psr->active)
> + return -EINVAL;
>
> - psr->state = state;
> -}
> + if (enable == psr->enabled)
> + return 0;
>
> -static void psr_set_state(struct psr_drv *psr, enum psr_state state)
> -{
> - mutex_lock(&psr->lock);
> - psr_set_state_locked(psr, state);
> - mutex_unlock(&psr->lock);
> + ret = psr->set(psr->encoder, enable);
> + if (ret)
> + return ret;
> +
> + psr->enabled = enable;
> + return 0;
> }
>
> static void psr_flush_handler(struct work_struct *work)
> @@ -131,10 +95,8 @@ static void psr_flush_handler(struct work_struct *work)
> struct psr_drv *psr = container_of(to_delayed_work(work),
> struct psr_drv, flush_work);
>
> - /* If the state has changed since we initiated the flush, do nothing */
> mutex_lock(&psr->lock);
> - if (psr->state == PSR_FLUSH)
> - psr_set_state_locked(psr, PSR_ENABLE);
> + psr_set_state_locked(psr, true);
> mutex_unlock(&psr->lock);
> }
>
> @@ -176,6 +138,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
>
> mutex_lock(&psr->lock);
> psr->active = false;
> + psr->enabled = false;
> mutex_unlock(&psr->lock);
> cancel_delayed_work_sync(&psr->flush_work);
> cancel_work_sync(&psr->disable_work);
> @@ -187,8 +150,12 @@ EXPORT_SYMBOL(rockchip_drm_psr_deactivate);
> static void rockchip_drm_do_flush(struct psr_drv *psr)
> {
> cancel_delayed_work_sync(&psr->flush_work);
> - psr_set_state(psr, PSR_FLUSH);
> - mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
> +
> + mutex_lock(&psr->lock);
> + if (!psr_set_state_locked(psr, false))
> + mod_delayed_work(system_wq, &psr->flush_work,
> + PSR_FLUSH_TIMEOUT_MS);
> + mutex_unlock(&psr->lock);
> }
>
> /**
> @@ -355,8 +322,8 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> INIT_WORK(&psr->disable_work, psr_disable_handler);
> mutex_init(&psr->lock);
>
> - psr->active = true;
> - psr->state = PSR_DISABLE;
> + psr->active = false;
> + psr->enabled = false;
> psr->encoder = encoder;
> psr->set = psr_set;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v6 27/30] drm/rockchip: psr: Sanitize semantics of allow/disallow API
From: Andrzej Hajda @ 2018-04-16 9:26 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-28-enric.balletbo@collabora.com>
On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> Currently both rockchip_drm_psr_activate() and _deactivate() only set the
> boolean "active" flag without actually making sure that hardware state
> complies with it.
>
> Since we are going to extend the usage of this API to properly lock PSR
> for the duration of atomic commits, we change the semantics in following
> way:
> - a counter is used to track the number of disallow requests,
> - PSR is actually disabled in hardware on first disallow request,
> - PSR enable work is scheduled on last disallow request.
I guess you meant "on last allow request".
I think It would be more readable if you replace disallow with inhibit.
>
> The above allows using the API as a way to deterministically synchronize
> PSR state changes with other DRM events, i.e. atomic commits and cursor
> updates. As a nice side effect, the naming is sorted out and we have
> "inhibit" for stopping the software logic and "enable" for hardware
> state.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 4 +-
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 57 ++++++++++++++++++-------
> drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 4 +-
> 3 files changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 6d45d62466b3..080f05352195 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -134,7 +134,7 @@ static int rockchip_dp_poweron_end(struct analogix_dp_plat_data *plat_data)
> {
> struct rockchip_dp_device *dp = to_dp(plat_data);
>
> - return rockchip_drm_psr_activate(&dp->encoder);
> + return rockchip_drm_psr_inhibit_put(&dp->encoder);
> }
>
> static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
> @@ -142,7 +142,7 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
> struct rockchip_dp_device *dp = to_dp(plat_data);
> int ret;
>
> - ret = rockchip_drm_psr_deactivate(&dp->encoder);
> + ret = rockchip_drm_psr_inhibit_get(&dp->encoder);
> if (ret != 0)
> return ret;
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index 448c5fde241c..e7e16d92d5a1 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -27,7 +27,7 @@ struct psr_drv {
> struct drm_encoder *encoder;
>
> struct mutex lock;
> - bool active;
> + int inhibit_count;
> bool enabled;
>
> struct delayed_work flush_work;
> @@ -76,7 +76,7 @@ static int psr_set_state_locked(struct psr_drv *psr, bool enable)
> {
> int ret;
>
> - if (!psr->active)
> + if (psr->inhibit_count > 0)
> return -EINVAL;
>
> if (enable == psr->enabled)
> @@ -101,13 +101,18 @@ static void psr_flush_handler(struct work_struct *work)
> }
>
> /**
> - * rockchip_drm_psr_activate - activate PSR on the given pipe
> + * rockchip_drm_psr_inhibit_put - release PSR inhibit on given encoder
> * @encoder: encoder to obtain the PSR encoder
> *
> + * Decrements PSR inhibit count on given encoder. Should be called only
> + * for a PSR inhibit count increment done before. If PSR inhibit counter
> + * reaches zero, PSR flush work is scheduled to make the hardware enter
> + * PSR mode in PSR_FLUSH_TIMEOUT_MS.
> + *
> * Returns:
> * Zero on success, negative errno on failure.
> */
> -int rockchip_drm_psr_activate(struct drm_encoder *encoder)
> +int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder)
> {
> struct psr_drv *psr = find_psr_by_encoder(encoder);
>
> @@ -115,21 +120,29 @@ int rockchip_drm_psr_activate(struct drm_encoder *encoder)
> return PTR_ERR(psr);
>
> mutex_lock(&psr->lock);
> - psr->active = true;
> + --psr->inhibit_count;
Maybe some WARN on negative value?
With doc fixes:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> + if (!psr->inhibit_count)
> + mod_delayed_work(system_wq, &psr->flush_work,
> + PSR_FLUSH_TIMEOUT_MS);
> mutex_unlock(&psr->lock);
>
> return 0;
> }
> -EXPORT_SYMBOL(rockchip_drm_psr_activate);
> +EXPORT_SYMBOL(rockchip_drm_psr_inhibit_put);
>
> /**
> - * rockchip_drm_psr_deactivate - deactivate PSR on the given pipe
> + * rockchip_drm_psr_inhibit_get - acquire PSR inhibit on given encoder
> * @encoder: encoder to obtain the PSR encoder
> *
> + * Increments PSR inhibit count on given encoder. This function guarantees
> + * that after it returns PSR is turned off on given encoder and no PSR-related
> + * hardware state change occurs at least until a matching call to
> + * rockchip_drm_psr_inhibit_put() is done.
> + *
> * Returns:
> * Zero on success, negative errno on failure.
> */
> -int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
> +int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder)
> {
> struct psr_drv *psr = find_psr_by_encoder(encoder);
>
> @@ -137,15 +150,15 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
> return PTR_ERR(psr);
>
> mutex_lock(&psr->lock);
> - psr->active = false;
> - psr->enabled = false;
> + psr_set_state_locked(psr, false);
> + ++psr->inhibit_count;
> mutex_unlock(&psr->lock);
> cancel_delayed_work_sync(&psr->flush_work);
> cancel_work_sync(&psr->disable_work);
>
> return 0;
> }
> -EXPORT_SYMBOL(rockchip_drm_psr_deactivate);
> +EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get);
>
> static void rockchip_drm_do_flush(struct psr_drv *psr)
> {
> @@ -301,6 +314,11 @@ static const struct input_device_id psr_ids[] = {
> * @encoder: encoder that obtain the PSR function
> * @psr_set: call back to set PSR state
> *
> + * The function returns with PSR inhibit counter initialized with one
> + * and the caller (typically encoder driver) needs to call
> + * rockchip_drm_psr_inhibit_put() when it becomes ready to accept PSR
> + * enable request.
> + *
> * Returns:
> * Zero on success, negative errno on failure.
> */
> @@ -322,7 +340,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> INIT_WORK(&psr->disable_work, psr_disable_handler);
> mutex_init(&psr->lock);
>
> - psr->active = false;
> + psr->inhibit_count = 1;
> psr->enabled = false;
> psr->encoder = encoder;
> psr->set = psr_set;
> @@ -362,6 +380,11 @@ EXPORT_SYMBOL(rockchip_drm_psr_register);
> * @encoder: encoder that obtain the PSR function
> * @psr_set: call back to set PSR state
> *
> + * It is expected that the PSR inhibit counter is 1 when this function is
> + * called, which corresponds to a state when related encoder has been
> + * disconnected from any CRTCs and its driver called
> + * rockchip_drm_psr_inhibit_get() to stop the PSR logic.
> + *
> * Returns:
> * Zero on success, negative errno on failure.
> */
> @@ -373,10 +396,14 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder)
> mutex_lock(&drm_drv->psr_list_lock);
> list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) {
> if (psr->encoder == encoder) {
> - input_unregister_handler(&psr->input_handler);
> - cancel_delayed_work_sync(&psr->flush_work);
> - cancel_work_sync(&psr->disable_work);
> + /*
> + * Any other value would mean that the encoder
> + * is still in use.
> + */
> + WARN_ON(psr->inhibit_count != 1);
> +
> list_del(&psr->list);
> + input_unregister_handler(&psr->input_handler);
> kfree(psr->input_handler.name);
> kfree(psr);
> }
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> index 06537ee27565..40e026c14168 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> @@ -18,8 +18,8 @@
> void rockchip_drm_psr_flush_all(struct drm_device *dev);
> int rockchip_drm_psr_flush(struct drm_crtc *crtc);
>
> -int rockchip_drm_psr_activate(struct drm_encoder *encoder);
> -int rockchip_drm_psr_deactivate(struct drm_encoder *encoder);
> +int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder);
> +int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder);
>
> int rockchip_drm_psr_register(struct drm_encoder *encoder,
> int (*psr_set)(struct drm_encoder *, bool enable));
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Jiri Kosina @ 2018-04-16 9:49 UTC (permalink / raw)
To: Robert Munteanu; +Cc: Benjamin Tissoires, linux-kernel, linux-input
In-Reply-To: <20180411094953.12053-1-rombert@apache.org>
On Wed, 11 Apr 2018, Robert Munteanu wrote:
> Changelog:
>
> - v2: modifier keys work, some combinations are still troublesome
> - v3: style cleanup, rebase on top of 4.14
> - v4: remove most debugging calls, make init info useful for user,
> rebased on top of 4.15
> - v5: fix the HID descriptor as suggested by Benjamin Tissoires,
> use existing USB vendor id, update comment style, add SPDX
> license identifier, rename to hid-redragon, stop registering
> two input devices, rebased on top of 4.16
This should contain the text that goes directly into the git commit
changelog, therefore should instead contain something like "This adds a
news driver for HW XX, that needs a specialized driver bacuse .... and
things to specifically point out in implementation are ....".
Could you please provide that, so that I can commit it?
Thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v6 29/30] drm/rockchip: Disallow PSR for the whole atomic commit
From: Andrzej Hajda @ 2018-04-16 9:51 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk,
Kristian H . Kristensen, kernel, m.szyprowski, linux-samsung-soc,
rydberg, krzk, linux-rockchip, kgene, linux-input, orjan.eide,
wxt, jeffy.chen, linux-arm-kernel, mark.yao, wzz, hl, jingoohan1,
sw0312.kim, linux-kernel, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-30-enric.balletbo@collabora.com>
On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> Currently PSR flush is triggered from CRTC's .atomic_begin() callback,
> which is executed after modeset disables and enables and before plane
> updates are committed. Since PSR flush and re-enable can be triggered
> asynchronously by external sources (input event, delayed work), it can
> race with hardware programming done in the aforementioned stages.
>
> This patch blocks the PSR completely before hardware programming part
> begins and unblock after it ends. This relies on reference counted PSR
> disable introduced with previous patch.
>
> Cc: Kristian H. Kristensen <hoegsberg@chromium.org>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 61 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ----
> 2 files changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index e266539e04e5..d4f4118b482d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -167,8 +167,67 @@ rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> return ERR_PTR(ret);
> }
>
> +static void
> +rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct drm_encoder *encoder;
> + u32 encoder_mask = 0;
> + int i;
> +
> + for_each_old_crtc_in_state(state, crtc, crtc_state, i) {
> + encoder_mask |= crtc_state->encoder_mask;
> + encoder_mask |= crtc->state->encoder_mask;
Looks clever and cryptic. More readable would be with
for_each_oldnew_crtc_in_state.
Anyway:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> + }
> +
> + drm_for_each_encoder_mask(encoder, state->dev, encoder_mask)
> + rockchip_drm_psr_inhibit_get(encoder);
> +}
> +
> +static void
> +rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct drm_encoder *encoder;
> + u32 encoder_mask = 0;
> + int i;
> +
> + for_each_old_crtc_in_state(state, crtc, crtc_state, i) {
> + encoder_mask |= crtc_state->encoder_mask;
> + encoder_mask |= crtc->state->encoder_mask;
> + }
> +
> + drm_for_each_encoder_mask(encoder, state->dev, encoder_mask)
> + rockchip_drm_psr_inhibit_put(encoder);
> +}
> +
> +static void
> +rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
> +{
> + struct drm_device *dev = old_state->dev;
> +
> + rockchip_drm_psr_inhibit_get_state(old_state);
> +
> + drm_atomic_helper_commit_modeset_disables(dev, old_state);
> +
> + drm_atomic_helper_commit_modeset_enables(dev, old_state);
> +
> + drm_atomic_helper_commit_planes(dev, old_state,
> + DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +
> + rockchip_drm_psr_inhibit_put_state(old_state);
> +
> + drm_atomic_helper_commit_hw_done(old_state);
> +
> + drm_atomic_helper_wait_for_vblanks(dev, old_state);
> +
> + drm_atomic_helper_cleanup_planes(dev, old_state);
> +}
> +
> static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = {
> - .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> + .atomic_commit_tail = rockchip_atomic_helper_commit_tail_rpm,
> };
>
> static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 00f7f3441cf6..f14a10ca4792 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1029,16 +1029,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> }
> }
>
> -static void vop_crtc_atomic_begin(struct drm_crtc *crtc,
> - struct drm_crtc_state *old_crtc_state)
> -{
> - rockchip_drm_psr_flush(crtc);
> -}
> -
> static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> .mode_fixup = vop_crtc_mode_fixup,
> .atomic_flush = vop_crtc_atomic_flush,
> - .atomic_begin = vop_crtc_atomic_begin,
> .atomic_enable = vop_crtc_atomic_enable,
> .atomic_disable = vop_crtc_atomic_disable,
> };
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v6 30/30] drm/rockchip: psr: Remove flush by CRTC
From: Andrzej Hajda @ 2018-04-16 9:53 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-31-enric.balletbo@collabora.com>
On 05.04.2018 11:50, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> It is not used anymore after last changes and it was not even correct to
> begin with as it assumed a 1:1 relation between a CRTC and encoder,
> while in fact a CRTC can be attached to multiple encoders.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 35 -----------------------------
> drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 1 -
> 2 files changed, 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index 1bf5cba9a64d..b1988ac758d5 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -40,23 +40,6 @@ struct psr_drv {
> int (*set)(struct drm_encoder *encoder, bool enable);
> };
>
> -static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc)
> -{
> - struct rockchip_drm_private *drm_drv = crtc->dev->dev_private;
> - struct psr_drv *psr;
> -
> - mutex_lock(&drm_drv->psr_list_lock);
> - list_for_each_entry(psr, &drm_drv->psr_list, list) {
> - if (psr->encoder->crtc == crtc)
> - goto out;
> - }
> - psr = ERR_PTR(-ENODEV);
> -
> -out:
> - mutex_unlock(&drm_drv->psr_list_lock);
> - return psr;
> -}
> -
> static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder)
> {
> struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
> @@ -173,24 +156,6 @@ static void rockchip_drm_do_flush(struct psr_drv *psr)
> mutex_unlock(&psr->lock);
> }
>
> -/**
> - * rockchip_drm_psr_flush - flush a single pipe
> - * @crtc: CRTC of the pipe to flush
> - *
> - * Returns:
> - * 0 on success, -errno on fail
> - */
> -int rockchip_drm_psr_flush(struct drm_crtc *crtc)
> -{
> - struct psr_drv *psr = find_psr_by_crtc(crtc);
> - if (IS_ERR(psr))
> - return PTR_ERR(psr);
> -
> - rockchip_drm_do_flush(psr);
> - return 0;
> -}
> -EXPORT_SYMBOL(rockchip_drm_psr_flush);
> -
> /**
> * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders
> * @dev: drm device
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> index 40e026c14168..860c62494496 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> @@ -16,7 +16,6 @@
> #define __ROCKCHIP_DRM_PSR___
>
> void rockchip_drm_psr_flush_all(struct drm_device *dev);
> -int rockchip_drm_psr_flush(struct drm_crtc *crtc);
>
> int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder);
> int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Benjamin Tissoires @ 2018-04-16 9:56 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Robert Munteanu, lkml, open list:HID CORE LAYER
In-Reply-To: <nycvar.YFH.7.76.1804161148370.28129@cbobk.fhfr.pm>
On Mon, Apr 16, 2018 at 11:49 AM, Jiri Kosina <jikos@kernel.org> wrote:
> On Wed, 11 Apr 2018, Robert Munteanu wrote:
>
>> Changelog:
>>
>> - v2: modifier keys work, some combinations are still troublesome
>> - v3: style cleanup, rebase on top of 4.14
>> - v4: remove most debugging calls, make init info useful for user,
>> rebased on top of 4.15
>> - v5: fix the HID descriptor as suggested by Benjamin Tissoires,
>> use existing USB vendor id, update comment style, add SPDX
>> license identifier, rename to hid-redragon, stop registering
>> two input devices, rebased on top of 4.16
>
> This should contain the text that goes directly into the git commit
> changelog, therefore should instead contain something like "This adds a
> news driver for HW XX, that needs a specialized driver bacuse .... and
> things to specifically point out in implementation are ....".
>
> Could you please provide that, so that I can commit it?
Jiri, please hold off, I have a few comments while quickly reviewing the patch.
Cheers,
Benjamin
>
> Thanks.
>
> --
> Jiri Kosina
> SUSE Labs
>
^ permalink raw reply
* Re: [PATCH v6 28/30] drm/rockchip: Disable PSR from reboot notifier
From: Andrzej Hajda @ 2018-04-16 9:57 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-29-enric.balletbo@collabora.com>
On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> It looks like the driver subsystem detaches devices from power domains
> at shutdown without consent of the drivers.
It looks bit strange. Could you elaborate more on it. Could you show the
code performing the detach?
Regards
Andrzej
> This means that we might have
> our power domain turned off behind our back and the only way to avoid
> problems is to stop doing any hardware programming at some point before
> the power is cut. A reboot notifier, despite being a misnomer and
> handling shutdowns as well, is a good place to do it.
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index e7e16d92d5a1..1bf5cba9a64d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -13,6 +13,7 @@
> */
>
> #include <linux/input.h>
> +#include <linux/reboot.h>
>
> #include <drm/drmP.h>
> #include <drm/drm_crtc_helper.h>
> @@ -33,6 +34,7 @@ struct psr_drv {
> struct delayed_work flush_work;
> struct work_struct disable_work;
>
> + struct notifier_block reboot_nb;
> struct input_handler input_handler;
>
> int (*set)(struct drm_encoder *encoder, bool enable);
> @@ -309,6 +311,24 @@ static const struct input_device_id psr_ids[] = {
> { },
> };
>
> +static int rockchip_drm_psr_reboot_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct psr_drv *psr = container_of(nb, struct psr_drv, reboot_nb);
> +
> + /*
> + * It looks like the driver subsystem detaches devices from power
> + * domains at shutdown without consent of the drivers. This means
> + * that we might have our power domain turned off behind our back
> + * and the only way to avoid problems is to stop doing any hardware
> + * programming after this point, which is achieved by the unbalanced
> + * call below.
> + */
> + rockchip_drm_psr_inhibit_get(psr->encoder);
> +
> + return 0;
> +}
> +
> /**
> * rockchip_drm_psr_register - register encoder to psr driver
> * @encoder: encoder that obtain the PSR function
> @@ -361,6 +381,9 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> if (error)
> goto err1;
>
> + psr->reboot_nb.notifier_call = rockchip_drm_psr_reboot_notifier;
> + register_reboot_notifier(&psr->reboot_nb);
> +
> mutex_lock(&drm_drv->psr_list_lock);
> list_add_tail(&psr->list, &drm_drv->psr_list);
> mutex_unlock(&drm_drv->psr_list_lock);
> @@ -403,6 +426,7 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder)
> WARN_ON(psr->inhibit_count != 1);
>
> list_del(&psr->list);
> + unregister_reboot_notifier(&psr->reboot_nb);
> input_unregister_handler(&psr->input_handler);
> kfree(psr->input_handler.name);
> kfree(psr);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Benjamin Tissoires @ 2018-04-16 10:02 UTC (permalink / raw)
To: Robert Munteanu; +Cc: Jiri Kosina, lkml, open list:HID CORE LAYER
In-Reply-To: <20180411094953.12053-1-rombert@apache.org>
Hi Robert,
On Wed, Apr 11, 2018 at 11:49 AM, Robert Munteanu <rombert@apache.org> wrote:
> Changelog:
>
> - v2: modifier keys work, some combinations are still troublesome
> - v3: style cleanup, rebase on top of 4.14
> - v4: remove most debugging calls, make init info useful for user,
> rebased on top of 4.15
> - v5: fix the HID descriptor as suggested by Benjamin Tissoires,
> use existing USB vendor id, update comment style, add SPDX
> license identifier, rename to hid-redragon, stop registering
> two input devices, rebased on top of 4.16
As Jiri said, please provide a correct commit message.
I have a few nitpicks in the driver, v6 should be fine:
>
> Signed-off-by: Robert Munteanu <rombert@apache.org>
> ---
> drivers/hid/Kconfig | 7 ++++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-quirks.c | 3 ++
> drivers/hid/hid-redragon.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 101 insertions(+)
> create mode 100644 drivers/hid/hid-redragon.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 19c499f5623d..1125e4813716 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -560,6 +560,13 @@ config HID_MAYFLASH
> Say Y here if you have HJZ Mayflash PS3 game controller adapters
> and want to enable force feedback support.
>
> +config HID_REDRAGON
> + tristate "Redragon keyboards"
> + depends on HID
> + default !EXPERT
> + ---help---
> + Support for Redragon keyboards that need fix-ups to work properly.
> +
> config HID_MICROSOFT
> tristate "Microsoft non-fully HID-compliant devices"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index eb13b9e92d85..a36f3f40ba63 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += hid-picolcd_debugfs.o
>
> obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o
> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o
> obj-$(CONFIG_HID_RETRODE) += hid-retrode.o
> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 9454ac134ce2..41a64d0e91f9 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -599,6 +599,7 @@
> #define USB_VENDOR_ID_JESS 0x0c45
> #define USB_DEVICE_ID_JESS_YUREX 0x1010
> #define USB_DEVICE_ID_ASUS_MD_5112 0x5112
> +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b
>
> #define USB_VENDOR_ID_JESS2 0x0f30
> #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index e92b77fa574a..5f1253f1739a 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -557,6 +557,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
> #if IS_ENABLED(CONFIG_HID_PRODIKEYS)
> { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
> #endif
> +#if IS_ENABLED(CONFIG_HID_REDRAGON)
> + { HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA) },
> +#endif
Please drop this hunk. v4.16 should work without changing
hid_have_special_driver. This way, you will be sure that an initramfs
that doesn't include hid-redragon.ko will allo people to type their
LUKS password.
> #if IS_ENABLED(CONFIG_HID_RETRODE)
> { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY, USB_DEVICE_ID_RETRODE2) },
> #endif
> diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-redragon.c
> new file mode 100644
> index 000000000000..ff98a5dbb8e2
> --- /dev/null
> +++ b/drivers/hid/hid-redragon.c
> @@ -0,0 +1,89 @@
> +/*
> + * HID driver for Redragon keyboards
> + *
> + * Copyright (c) 2017 Robert Munteanu
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
you probably don't need input.h
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/log2.h>
log2.h? I am not sure you need it either
> +#include <linux/input-event-codes.h>
probably drop this one too.
> +
> +#include "hid-ids.h"
> +
> +
> +/*
> + * The Redragon Asura keyboard sends an incorrect HID descriptor.
> + * At byte 100 it contains
> + *
> + * 0x81, 0x00
> + *
> + * which is Input (Data, Arr, Abs), but it should be
> + *
> + * 0x81, 0x02
> + *
> + * which is Input (Data, Var, Abs), which is consistent with the way
> + * key codes are generated.
> + */
> +
> +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == 0x00) {
> + dev_info(&hdev->dev, "Fixing Redragon ASURA report descriptor.\n");
> + rdesc[101] = 0x02;
> + }
> +
> + return rdesc;
> +}
> +
> +static int redragon_probe(struct hid_device *dev,
> + const struct hid_device_id *id)
> +{
> + int ret;
> +
> + ret = hid_parse(dev);
> + if (ret) {
> + hid_err(dev, "parse failed\n");
> + return ret;
> + }
> +
> + /* do not register unused input device */
> + if (dev->maxapplication == 1)
> + return 0;
> +
> + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(dev, "hw start failed\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +static const struct hid_device_id redragon_devices[] = {
> + {HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(hid, redragon_devices);
> +
> +static struct hid_driver redragon_driver = {
> + .name = "redragon",
> + .id_table = redragon_devices,
> + .report_fixup = redragon_report_fixup,
> + .probe = redragon_probe
> +};
> +
> +module_hid_driver(redragon_driver);
> +
> +MODULE_LICENSE("GPL");
The SPDX header says GPL-v2. And IIRC if there is the SPDX header you
can drop the MODULE_LICENSE (not entirely sure though).
Cheers,
Benjamin
> --
> 2.16.3
>
^ permalink raw reply
* Re: [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Robert Munteanu @ 2018-04-16 10:14 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, lkml, open list:HID CORE LAYER
In-Reply-To: <CAO-hwJKbYyZdtq=zg_ZeJOiL=5cTbD4oKuWmpF-P_7TgBkqVYA@mail.gmail.com>
Hi Benjamin and Jiri,
On Mon, 2018-04-16 at 12:02 +0200, Benjamin Tissoires wrote:
> Hi Robert,
>
> On Wed, Apr 11, 2018 at 11:49 AM, Robert Munteanu <rombert@apache.org
> > wrote:
> > Changelog:
> >
> > - v2: modifier keys work, some combinations are still troublesome
> > - v3: style cleanup, rebase on top of 4.14
> > - v4: remove most debugging calls, make init info useful for user,
> > rebased on top of 4.15
> > - v5: fix the HID descriptor as suggested by Benjamin Tissoires,
> > use existing USB vendor id, update comment style, add SPDX
> > license identifier, rename to hid-redragon, stop registering
> > two input devices, rebased on top of 4.16
>
> As Jiri said, please provide a correct commit message.
Will do.
>
> I have a few nitpicks in the driver, v6 should be fine:
>
> >
> > Signed-off-by: Robert Munteanu <rombert@apache.org>
> > ---
> > drivers/hid/Kconfig | 7 ++++
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-quirks.c | 3 ++
> > drivers/hid/hid-redragon.c | 89
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 101 insertions(+)
> > create mode 100644 drivers/hid/hid-redragon.c
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 19c499f5623d..1125e4813716 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -560,6 +560,13 @@ config HID_MAYFLASH
> > Say Y here if you have HJZ Mayflash PS3 game controller
> > adapters
> > and want to enable force feedback support.
> >
> > +config HID_REDRAGON
> > + tristate "Redragon keyboards"
> > + depends on HID
> > + default !EXPERT
> > + ---help---
> > + Support for Redragon keyboards that need fix-ups to work
> > properly.
> > +
> > config HID_MICROSOFT
> > tristate "Microsoft non-fully HID-compliant devices"
> > depends on HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index eb13b9e92d85..a36f3f40ba63 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) +=
> > hid-picolcd_debugfs.o
> >
> > obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o
> > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> > +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o
> > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o
> > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o
> > \
> > hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 9454ac134ce2..41a64d0e91f9 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -599,6 +599,7 @@
> > #define USB_VENDOR_ID_JESS 0x0c45
> > #define USB_DEVICE_ID_JESS_YUREX 0x1010
> > #define USB_DEVICE_ID_ASUS_MD_5112 0x5112
> > +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b
> >
> > #define USB_VENDOR_ID_JESS2 0x0f30
> > #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111
> > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> > index e92b77fa574a..5f1253f1739a 100644
> > --- a/drivers/hid/hid-quirks.c
> > +++ b/drivers/hid/hid-quirks.c
> > @@ -557,6 +557,9 @@ static const struct hid_device_id
> > hid_have_special_driver[] = {
> > #if IS_ENABLED(CONFIG_HID_PRODIKEYS)
> > { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS,
> > USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
> > #endif
> > +#if IS_ENABLED(CONFIG_HID_REDRAGON)
> > + {
> > HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)
> > },
> > +#endif
>
> Please drop this hunk. v4.16 should work without changing
> hid_have_special_driver. This way, you will be sure that an initramfs
> that doesn't include hid-redragon.ko will allo people to type their
> LUKS password.
I recall testing without this change resulted in the driver not being
picked up. I will retest ( running 4.16.0 here, can update to 4.16.1
soon ). In case the driver is not picked up, where should I start
looking?
>
> > #if IS_ENABLED(CONFIG_HID_RETRODE)
> > { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
> > USB_DEVICE_ID_RETRODE2) },
> > #endif
> > diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-
> > redragon.c
> > new file mode 100644
> > index 000000000000..ff98a5dbb8e2
> > --- /dev/null
> > +++ b/drivers/hid/hid-redragon.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * HID driver for Redragon keyboards
> > + *
> > + * Copyright (c) 2017 Robert Munteanu
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
> > +
> > +/*
> > + * This program is free software; you can redistribute it and/or
> > modify it
> > + * under the terms of the GNU General Public License as published
> > by the Free
> > + * Software Foundation; either version 2 of the License, or (at
> > your option)
> > + * any later version.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/input.h>
>
> you probably don't need input.h
>
> > +#include <linux/hid.h>
> > +#include <linux/module.h>
> > +#include <linux/log2.h>
>
> log2.h? I am not sure you need it either
>
> > +#include <linux/input-event-codes.h>
>
> probably drop this one too.
I'll drop the missing imports, those are leftovers from my initial
work.
>
> > +
> > +#include "hid-ids.h"
> > +
> > +
> > +/*
> > + * The Redragon Asura keyboard sends an incorrect HID descriptor.
> > + * At byte 100 it contains
> > + *
> > + * 0x81, 0x00
> > + *
> > + * which is Input (Data, Arr, Abs), but it should be
> > + *
> > + * 0x81, 0x02
> > + *
> > + * which is Input (Data, Var, Abs), which is consistent with the
> > way
> > + * key codes are generated.
> > + */
> > +
> > +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8
> > *rdesc,
> > + unsigned int *rsize)
> > +{
> > + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] ==
> > 0x00) {
> > + dev_info(&hdev->dev, "Fixing Redragon ASURA report
> > descriptor.\n");
> > + rdesc[101] = 0x02;
> > + }
> > +
> > + return rdesc;
> > +}
> > +
> > +static int redragon_probe(struct hid_device *dev,
> > + const struct hid_device_id *id)
> > +{
> > + int ret;
> > +
> > + ret = hid_parse(dev);
> > + if (ret) {
> > + hid_err(dev, "parse failed\n");
> > + return ret;
> > + }
> > +
> > + /* do not register unused input device */
> > + if (dev->maxapplication == 1)
> > + return 0;
> > +
> > + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
> > + if (ret) {
> > + hid_err(dev, "hw start failed\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +static const struct hid_device_id redragon_devices[] = {
> > + {HID_USB_DEVICE(USB_VENDOR_ID_JESS,
> > USB_DEVICE_ID_REDRAGON_ASURA)},
> > + {}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(hid, redragon_devices);
> > +
> > +static struct hid_driver redragon_driver = {
> > + .name = "redragon",
> > + .id_table = redragon_devices,
> > + .report_fixup = redragon_report_fixup,
> > + .probe = redragon_probe
> > +};
> > +
> > +module_hid_driver(redragon_driver);
> > +
> > +MODULE_LICENSE("GPL");
>
> The SPDX header says GPL-v2. And IIRC if there is the SPDX header you
> can drop the MODULE_LICENSE (not entirely sure though).
Ack, will adjust and re-test.
Thanks for the review,
Robert
^ permalink raw reply
* [PATCH v9 0/2] hid-steam driver with user mode client dection
From: Rodrigo Rivas Costa @ 2018-04-16 12:27 UTC (permalink / raw)
To: Benjamin Tissoires, Pierre-Loup A. Griffais,
Clément VUCHENER, Jiri Kosina, Cameron Gutman, lkml,
linux-input
Cc: Rodrigo Rivas Costa
Hello! This is reroll v9 of the Steam Controller driver.
@Pierre-Loup, @Clément, could you please check if this driver is acceptable by
Valve? I think this could be the one ;-).
I've rolled back the synthetic LPAD diagonals. It happens that the actual
coordinates of the pads are rotated about 15 degrees to the center, but the
marks in the pad are aligned to 0/90 degrees. That, and my poor's man
trigonometry, makes clicking the diagonals quite unreliable.
Sorry for the noise, but now I think that we are better limiting ourselves to
the events emitted by the hardware, as is.
Other than that, I've doing some torture testing, with and without the Steam
Client running. I've changed:
1. The way EPIPE is handled when sending a report: I've noticed that Steam
Client retries up to 50 times, with little or no delay between them, so I do
the same.
2. I've added a fallback in case this driver is unable to get the serial number
of the controller. Failures on any other report can be ignored safely. But
failing to get the serial number was preventing the creating of the hidraw
node (wired controller only), and that could prevent Steam Client from
working.
3. I've received a mail from Valve with a bunch of constants for the
protocol. Most are related to the setting of new mappings (that we do not
need) and a few of the rest are deprecated. Anyway, I've renamed the
constants STEAM_CMD_* to align them with the official names, and added a
few more that were unknown to me, just for future reference. Still, commands
used for enabling/disabling lizard mode are the same.
Best regards.
Changes in v9:
* Remove synthetic diagonals for the DPAD (bad idea).
* Rename protocol constants to follow Valve names.
* Rewrite the retry fo steam_send_report() when EPIPE.
* Add a fallback for failure getting the serial number.
Changes in v8:
* Add constants for the protocol major magic numbers.
* Disable/Enable margin of the rpad, together with the lizard-mode.
* Synthesize diagonals for the DPAD.
* Fix overflow with Y=-32768.
* Make lizard_mode parameter a bool, dynamically updatable.
Changes in v7:
* All the automatic lizard_mode stuff.
* Added the lizard_mode parameter.
* The patchset is reduced to 2 commits. The separation of the
steam_get_serial command no longer makes sense, since I need the
steam_send_cmd in the first commit to implement the lizard mode.
* Change the input mapping to conform to Documentation/gamepad.rst.
(v6 was a RFC, it does not count).
Changes in v5:
* Fix license SPDX to GPL-2.0+.
* Minor stylistic changes (BIT(3) instead 0x08 and so on).
Changes in v4:
* Add command to check the wireless connection status on probe, without
waiting for a message (thanks to Clément Vuchener for the tip).
* Removed the error code on redundant connection/disconnection messages. That
was harmless but polluted dmesg.
* Added buttons for touching the left-pad and right-pad.
* Fixed a misplaced #include from 2/4 to 1/4.
Changes in v3:
* Use RCU to do the dynamic connec/disconnect of wireless devices.
* Remove entries in hid-quirks.c as they are no longer needed. This allows
this module to be blacklisted without side effects.
* Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
existing use cases (lizard mode). A user-space tool to do that is
linked.
* Fully separated axes for joystick and left-pad. As it happens.
* Add fuzz values for left/right pad axes, they are a little wiggly.
Changes in v2:
* Remove references to USB. Now the interesting interfaces are selected by
looking for the ones with feature reports.
* Feature reports buffers are allocated with hid_alloc_report_buf().
* Feature report length is checked, to avoid overflows in case of
corrupt/malicius USB devices.
* Resolution added to the ABS axes.
* A lot of minor cleanups.
Rodrigo Rivas Costa (2):
HID: add driver for Valve Steam Controller
HID: steam: add battery device.
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-steam.c | 1112 +++++++++++++++++++++++++++++++++++++++
include/linux/hid.h | 1 +
5 files changed, 1126 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
--
2.17.0
^ permalink raw reply
* [PATCH v9 1/2] HID: add driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-04-16 12:27 UTC (permalink / raw)
To: Benjamin Tissoires, Pierre-Loup A. Griffais,
Clément VUCHENER, Jiri Kosina, Cameron Gutman, lkml,
linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180416122703.22306-1-rodrigorivascosta@gmail.com>
There are two ways to connect the Steam Controller: directly to the USB
or with the USB wireless adapter. Both methods are similar, but the
wireless adapter can connect up to 4 devices at the same time.
The wired device will appear as 3 interfaces: a virtual mouse, a virtual
keyboard and a custom HID device.
The wireless device will appear as 5 interfaces: a virtual keyboard and
4 custom HID devices, that will remain silent until a device is actually
connected.
The custom HID device has a report descriptor with all vendor specific
usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
Steam Client provices a software translation by using hidraw and a
creates a uinput virtual gamepad and XTest keyboard/mouse.
This driver intercepts the hidraw usage, so it can get out of the way
when the Steam Client is in use.
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-steam.c | 973 ++++++++++++++++++++++++++++++++++++++++
include/linux/hid.h | 1 +
5 files changed, 987 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 779c5ae47f36..de5f4849bfe4 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -811,6 +811,14 @@ config HID_SPEEDLINK
---help---
Support for Speedlink Vicious and Divine Cezanne mouse.
+config HID_STEAM
+ tristate "Steam Controller support"
+ depends on HID
+ ---help---
+ Say Y here if you have a Steam Controller if you want to use it
+ without running the Steam Client. It supports both the wired and
+ the wireless adaptor.
+
config HID_STEELSERIES
tristate "Steelseries SRW-S1 steering wheel support"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 235bd2a7b333..e146c257285a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
+obj-$(CONFIG_HID_STEAM) += hid-steam.o
obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index a0baa5ba5b84..3014991e5d4b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -987,6 +987,10 @@
#define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
#define USB_DEVICE_ID_MTP_SITRONIX 0x5001
+#define USB_VENDOR_ID_VALVE 0x28de
+#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
+#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS 0x1142
+
#define USB_VENDOR_ID_STEELSERIES 0x1038
#define USB_DEVICE_ID_STEELSERIES_SRWS1 0x1410
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index 000000000000..36fc85714ea5
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,973 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * This controller has a builtin emulation of mouse and keyboard: the right pad
+ * can be used as a mouse, the shoulder buttons are mouse buttons, A and B
+ * buttons are ENTER and ESCAPE, and so on. This is implemented as additional
+ * HID interfaces.
+ *
+ * This is known as the "lizard mode", because apparently lizards like to use
+ * the computer from the coach, without a proper mouse and keyboard.
+ *
+ * This driver will disable the lizard mode when the input device is opened
+ * and re-enable it when the input device is closed, so as not to break user
+ * mode behaviour. The lizard_mode parameter can be used to change that.
+ *
+ * There are a few user space applications (notably Steam Client) that use
+ * the hidraw interface directly to create input devices (XTest, uinput...).
+ * In order to avoid breaking them this driver creates a layered hidraw device,
+ * so it can detect when the client is running and then:
+ * - it will not send any command to the controller.
+ * - this input device will be disabled, to avoid double input of the same
+ * user action.
+ *
+ * For additional functions, such as changing the right-pad margin or switching
+ * the led, you can use the user-space tool at:
+ *
+ * https://github.com/rodrigorc/steamctrl
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+#include <linux/mutex.h>
+#include <linux/rcupdate.h>
+#include <linux/delay.h>
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
+
+static bool lizard_mode = true;
+
+static DEFINE_MUTEX(steam_devices_lock);
+static LIST_HEAD(steam_devices);
+
+#define STEAM_QUIRK_WIRELESS BIT(0)
+
+/* Touch pads are 40 mm in diameter and 65535 units */
+#define STEAM_PAD_RESOLUTION 1638
+/* Trigger runs are about 5 mm and 256 units */
+#define STEAM_TRIGGER_RESOLUTION 51
+/* Joystick runs are about 5 mm and 256 units */
+#define STEAM_JOYSTICK_RESOLUTION 51
+
+#define STEAM_PAD_FUZZ 256
+
+/*
+ * Commands that can be sent in a feature report.
+ * Thanks to Valve for some valuable hints.
+ */
+#define STEAM_CMD_SET_MAPPINGS 0x80
+#define STEAM_CMD_CLEAR_MAPPINGS 0x81
+#define STEAM_CMD_GET_MAPPINGS 0x82
+#define STEAM_CMD_GET_ATTRIB 0x83
+#define STEAM_CMD_GET_ATTRIB_LABEL 0x84
+#define STEAM_CMD_DEFAULT_MAPPINGS 0x85
+#define STEAM_CMD_FACTORY_RESET 0x86
+#define STEAM_CMD_WRITE_REGISTER 0x87
+#define STEAM_CMD_CLEAR_REGISTER 0x88
+#define STEAM_CMD_READ_REGISTER 0x89
+#define STEAM_CMD_GET_REGISTER_LABEL 0x8a
+#define STEAM_CMD_GET_REGISTER_MAX 0x8b
+#define STEAM_CMD_GET_REGISTER_DEFAULT 0x8c
+#define STEAM_CMD_SET_MODE 0x8d
+#define STEAM_CMD_DEFAULT_MOUSE 0x8e
+#define STEAM_CMD_FORCEFEEDBAK 0x8f
+#define STEAM_CMD_REQUEST_COMM_STATUS 0xb4
+#define STEAM_CMD_GET_SERIAL 0xae
+
+/* Some useful register ids */
+#define STEAM_REG_LPAD_MODE 0x07
+#define STEAM_REG_RPAD_MODE 0x08
+#define STEAM_REG_RPAD_MARGIN 0x18
+#define STEAM_REG_LED 0x2d
+#define STEAM_REG_GYRO_MODE 0x30
+
+/* Raw event identifiers */
+#define STEAM_EV_INPUT_DATA 0x01
+#define STEAM_EV_CONNECT 0x03
+#define STEAM_EV_BATTERY 0x04
+
+/* Values for GYRO_MODE (bitmask) */
+#define STEAM_GYRO_MODE_OFF 0x0000
+#define STEAM_GYRO_MODE_STEERING 0x0001
+#define STEAM_GYRO_MODE_TILT 0x0002
+#define STEAM_GYRO_MODE_SEND_ORIENTATION 0x0004
+#define STEAM_GYRO_MODE_SEND_RAW_ACCEL 0x0008
+#define STEAM_GYRO_MODE_SEND_RAW_GYRO 0x0010
+
+/* Other random constants */
+#define STEAM_SERIAL_LEN 10
+
+struct steam_device {
+ struct list_head list;
+ spinlock_t lock;
+ struct hid_device *hdev, *client_hdev;
+ struct mutex mutex;
+ bool client_opened, input_opened;
+ struct input_dev __rcu *input;
+ unsigned long quirks;
+ struct work_struct work_connect;
+ bool connected;
+ char serial_no[STEAM_SERIAL_LEN + 1];
+};
+
+static int steam_recv_report(struct steam_device *steam,
+ u8 *data, int size)
+{
+ struct hid_report *r;
+ u8 *buf;
+ int ret;
+
+ r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+ if (hid_report_len(r) < 64)
+ return -EINVAL;
+
+ buf = hid_alloc_report_buf(r, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /*
+ * The report ID is always 0, so strip the first byte from the output.
+ * hid_report_len() is not counting the report ID, so +1 to the length
+ * or else we get a EOVERFLOW. We are safe from a buffer overflow
+ * because hid_alloc_report_buf() allocates +7 bytes.
+ */
+ ret = hid_hw_raw_request(steam->hdev, 0x00,
+ buf, hid_report_len(r) + 1,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret > 0)
+ memcpy(data, buf + 1, min(size, ret - 1));
+ kfree(buf);
+ return ret;
+}
+
+static int steam_send_report(struct steam_device *steam,
+ u8 *cmd, int size)
+{
+ struct hid_report *r;
+ u8 *buf;
+ unsigned int retries = 50;
+ int ret;
+
+ r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+ if (hid_report_len(r) < 64)
+ return -EINVAL;
+
+ buf = hid_alloc_report_buf(r, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* The report ID is always 0 */
+ memcpy(buf + 1, cmd, size);
+
+ /*
+ * Sometimes the wireless controller fails with EPIPE
+ * when sending a feature report.
+ * Doing a HID_REQ_GET_REPORT and waiting for a while
+ * seems to fix that.
+ */
+ do {
+ ret = hid_hw_raw_request(steam->hdev, 0,
+ buf, size + 1,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != -EPIPE)
+ break;
+ msleep(20);
+ } while (--retries);
+
+ kfree(buf);
+ if (ret < 0)
+ hid_err(steam->hdev, "%s: error %d (%*ph)\n", __func__,
+ ret, size, cmd);
+ return ret;
+}
+
+static inline int steam_send_report_byte(struct steam_device *steam, u8 cmd)
+{
+ return steam_send_report(steam, &cmd, 1);
+}
+
+static int steam_write_registers(struct steam_device *steam,
+ /* u8 reg, u16 val */...)
+{
+ /* Send: 0x87 len (reg valLo valHi)* */
+ u8 reg;
+ u16 val;
+ u8 cmd[64] = {STEAM_CMD_WRITE_REGISTER, 0x00};
+ va_list args;
+
+ va_start(args, steam);
+ for (;;) {
+ reg = va_arg(args, int);
+ if (reg == 0)
+ break;
+ val = va_arg(args, int);
+ cmd[cmd[1] + 2] = reg;
+ cmd[cmd[1] + 3] = val & 0xff;
+ cmd[cmd[1] + 4] = val >> 8;
+ cmd[1] += 3;
+ }
+ va_end(args);
+
+ return steam_send_report(steam, cmd, 2 + cmd[1]);
+}
+
+static int steam_get_serial(struct steam_device *steam)
+{
+ /*
+ * Send: 0xae 0x15 0x01
+ * Recv: 0xae 0x15 0x01 serialnumber (10 chars)
+ */
+ int ret;
+ u8 cmd[] = {STEAM_CMD_GET_SERIAL, 0x15, 0x01};
+ u8 reply[3 + STEAM_SERIAL_LEN + 1];
+
+ ret = steam_send_report(steam, cmd, sizeof(cmd));
+ if (ret < 0)
+ return ret;
+ ret = steam_recv_report(steam, reply, sizeof(reply));
+ if (ret < 0)
+ return ret;
+ if (reply[0] != 0xae || reply[1] != 0x15 || reply[2] != 0x01)
+ return -EIO;
+ reply[3 + STEAM_SERIAL_LEN] = 0;
+ strlcpy(steam->serial_no, reply + 3, sizeof(steam->serial_no));
+ return 0;
+}
+
+/*
+ * This command requests the wireless adaptor to post an event
+ * with the connection status. Useful if this driver is loaded when
+ * the controller is already connected.
+ */
+static inline int steam_request_conn_status(struct steam_device *steam)
+{
+ return steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS);
+}
+
+static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
+{
+ if (enable) {
+ /* enable esc, enter, cursors */
+ steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MAPPINGS);
+ /* enable mouse */
+ steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MOUSE);
+ steam_write_registers(steam,
+ STEAM_REG_RPAD_MARGIN, 0x01, /* enable margin */
+ 0);
+ } else {
+ /* disable esc, enter, cursor */
+ steam_send_report_byte(steam, STEAM_CMD_CLEAR_MAPPINGS);
+ steam_write_registers(steam,
+ STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
+ STEAM_REG_RPAD_MARGIN, 0x00, /* disable margin */
+ 0);
+ }
+}
+
+static void steam_update_lizard_mode(struct steam_device *steam)
+{
+ mutex_lock(&steam->mutex);
+ if (!steam->client_opened) {
+ if (steam->input_opened)
+ steam_set_lizard_mode(steam, false);
+ else
+ steam_set_lizard_mode(steam, lizard_mode);
+ }
+ mutex_unlock(&steam->mutex);
+}
+
+static int steam_input_open(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+ int ret;
+
+ ret = hid_hw_open(steam->hdev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&steam->mutex);
+ steam->input_opened = true;
+ if (!steam->client_opened && lizard_mode)
+ steam_set_lizard_mode(steam, false);
+ mutex_unlock(&steam->mutex);
+ return 0;
+}
+
+static void steam_input_close(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+
+ mutex_lock(&steam->mutex);
+ steam->input_opened = false;
+ if (!steam->client_opened && lizard_mode)
+ steam_set_lizard_mode(steam, true);
+ mutex_unlock(&steam->mutex);
+
+ hid_hw_close(steam->hdev);
+}
+
+static int steam_register(struct steam_device *steam)
+{
+ struct hid_device *hdev = steam->hdev;
+ struct input_dev *input;
+ int ret;
+
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ rcu_read_unlock();
+ if (input) {
+ dbg_hid("%s: already connected\n", __func__);
+ return 0;
+ }
+
+ /*
+ * Unlikely, but getting the serial could fail, and it is not so
+ * important, so make up a serial number and go on.
+ */
+ if (steam_get_serial(steam) < 0)
+ strlcpy(steam->serial_no, "XXXXXXXXXX",
+ sizeof(steam->serial_no));
+
+ hid_info(hdev, "Steam Controller '%s' connected",
+ steam->serial_no);
+
+ input = input_allocate_device();
+ if (!input)
+ return -ENOMEM;
+
+ input_set_drvdata(input, steam);
+ input->dev.parent = &hdev->dev;
+ input->open = steam_input_open;
+ input->close = steam_input_close;
+
+ input->name = (steam->quirks & STEAM_QUIRK_WIRELESS) ?
+ "Wireless Steam Controller" :
+ "Steam Controller";
+ input->phys = hdev->phys;
+ input->uniq = steam->serial_no;
+ input->id.bustype = hdev->bus;
+ input->id.vendor = hdev->vendor;
+ input->id.product = hdev->product;
+ input->id.version = hdev->version;
+
+ input_set_capability(input, EV_KEY, BTN_TR2);
+ input_set_capability(input, EV_KEY, BTN_TL2);
+ input_set_capability(input, EV_KEY, BTN_TR);
+ input_set_capability(input, EV_KEY, BTN_TL);
+ input_set_capability(input, EV_KEY, BTN_Y);
+ input_set_capability(input, EV_KEY, BTN_B);
+ input_set_capability(input, EV_KEY, BTN_X);
+ input_set_capability(input, EV_KEY, BTN_A);
+ input_set_capability(input, EV_KEY, BTN_DPAD_UP);
+ input_set_capability(input, EV_KEY, BTN_DPAD_RIGHT);
+ input_set_capability(input, EV_KEY, BTN_DPAD_LEFT);
+ input_set_capability(input, EV_KEY, BTN_DPAD_DOWN);
+ input_set_capability(input, EV_KEY, BTN_SELECT);
+ input_set_capability(input, EV_KEY, BTN_MODE);
+ input_set_capability(input, EV_KEY, BTN_START);
+ input_set_capability(input, EV_KEY, BTN_GEAR_DOWN);
+ input_set_capability(input, EV_KEY, BTN_GEAR_UP);
+ input_set_capability(input, EV_KEY, BTN_THUMBR);
+ input_set_capability(input, EV_KEY, BTN_THUMBL);
+ input_set_capability(input, EV_KEY, BTN_THUMB);
+ input_set_capability(input, EV_KEY, BTN_THUMB2);
+
+ input_set_abs_params(input, ABS_HAT2Y, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_HAT2X, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_RX, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_RY, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT0X, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT0Y, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_abs_set_res(input, ABS_X, STEAM_JOYSTICK_RESOLUTION);
+ input_abs_set_res(input, ABS_Y, STEAM_JOYSTICK_RESOLUTION);
+ input_abs_set_res(input, ABS_RX, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_RY, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT2Y, STEAM_TRIGGER_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT2X, STEAM_TRIGGER_RESOLUTION);
+
+ ret = input_register_device(input);
+ if (ret)
+ goto input_register_fail;
+
+ rcu_assign_pointer(steam->input, input);
+
+ return 0;
+
+input_register_fail:
+ input_free_device(input);
+ return ret;
+}
+
+static void steam_unregister(struct steam_device *steam)
+{
+ struct input_dev *input;
+
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ rcu_read_unlock();
+
+ if (input) {
+ RCU_INIT_POINTER(steam->input, NULL);
+ synchronize_rcu();
+ hid_info(steam->hdev, "Steam Controller '%s' disconnected",
+ steam->serial_no);
+ input_unregister_device(input);
+ }
+}
+
+static void steam_work_connect_cb(struct work_struct *work)
+{
+ struct steam_device *steam = container_of(work, struct steam_device,
+ work_connect);
+ unsigned long flags;
+ bool connected;
+ int ret;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ connected = steam->connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (connected) {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(steam->hdev,
+ "%s:steam_register failed with error %d\n",
+ __func__, ret);
+ }
+ } else {
+ steam_unregister(steam);
+ }
+}
+
+static bool steam_is_valve_interface(struct hid_device *hdev)
+{
+ struct hid_report_enum *rep_enum;
+
+ /*
+ * The wired device creates 3 interfaces:
+ * 0: emulated mouse.
+ * 1: emulated keyboard.
+ * 2: the real game pad.
+ * The wireless device creates 5 interfaces:
+ * 0: emulated keyboard.
+ * 1-4: slots where up to 4 real game pads will be connected to.
+ * We know which one is the real gamepad interface because they are the
+ * only ones with a feature report.
+ */
+ rep_enum = &hdev->report_enum[HID_FEATURE_REPORT];
+ return !list_empty(&rep_enum->report_list);
+}
+
+static int steam_client_ll_parse(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ return hid_parse_report(hdev, steam->hdev->dev_rdesc,
+ steam->hdev->dev_rsize);
+}
+
+static int steam_client_ll_start(struct hid_device *hdev)
+{
+ return 0;
+}
+
+static void steam_client_ll_stop(struct hid_device *hdev)
+{
+}
+
+static int steam_client_ll_open(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+ int ret;
+
+ ret = hid_hw_open(steam->hdev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&steam->mutex);
+ steam->client_opened = true;
+ mutex_unlock(&steam->mutex);
+ return ret;
+}
+
+static void steam_client_ll_close(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ mutex_lock(&steam->mutex);
+ steam->client_opened = false;
+ if (steam->input_opened)
+ steam_set_lizard_mode(steam, false);
+ else
+ steam_set_lizard_mode(steam, lizard_mode);
+ mutex_unlock(&steam->mutex);
+
+ hid_hw_close(steam->hdev);
+}
+
+static int steam_client_ll_raw_request(struct hid_device *hdev,
+ unsigned char reportnum, u8 *buf,
+ size_t count, unsigned char report_type,
+ int reqtype)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ return hid_hw_raw_request(steam->hdev, reportnum, buf, count,
+ report_type, reqtype);
+}
+
+static struct hid_ll_driver steam_client_ll_driver = {
+ .parse = steam_client_ll_parse,
+ .start = steam_client_ll_start,
+ .stop = steam_client_ll_stop,
+ .open = steam_client_ll_open,
+ .close = steam_client_ll_close,
+ .raw_request = steam_client_ll_raw_request,
+};
+
+static struct hid_device *steam_create_client_hid(struct hid_device *hdev)
+{
+ struct hid_device *client_hdev;
+
+ client_hdev = hid_allocate_device();
+ if (IS_ERR(client_hdev))
+ return client_hdev;
+
+ client_hdev->ll_driver = &steam_client_ll_driver;
+ client_hdev->dev.parent = hdev->dev.parent;
+ client_hdev->bus = hdev->bus;
+ client_hdev->vendor = hdev->vendor;
+ client_hdev->product = hdev->product;
+ strlcpy(client_hdev->name, hdev->name,
+ sizeof(client_hdev->name));
+ strlcpy(client_hdev->phys, hdev->phys,
+ sizeof(client_hdev->phys));
+ /*
+ * Since we use the same device info than the real interface to
+ * trick userspace, we will be calling steam_probe recursively.
+ * We need to recognize the client interface somehow.
+ */
+ client_hdev->group = HID_GROUP_STEAM;
+ return client_hdev;
+}
+
+static int steam_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct steam_device *steam;
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:parse of hid interface failed\n", __func__);
+ return ret;
+ }
+
+ /*
+ * The virtual client_dev is only used for hidraw.
+ * Also avoid the recursive probe.
+ */
+ if (hdev->group == HID_GROUP_STEAM)
+ return hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ /*
+ * The non-valve interfaces (mouse and keyboard emulation) are
+ * connected without changes.
+ */
+ if (!steam_is_valve_interface(hdev))
+ return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+
+ steam = devm_kzalloc(&hdev->dev, sizeof(*steam), GFP_KERNEL);
+ if (!steam) {
+ ret = -ENOMEM;
+ goto steam_alloc_fail;
+ }
+ steam->hdev = hdev;
+ hid_set_drvdata(hdev, steam);
+ spin_lock_init(&steam->lock);
+ mutex_init(&steam->mutex);
+ steam->quirks = id->driver_data;
+ INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+
+ steam->client_hdev = steam_create_client_hid(hdev);
+ if (IS_ERR(steam->client_hdev)) {
+ ret = PTR_ERR(steam->client_hdev);
+ goto client_hdev_fail;
+ }
+ hid_set_drvdata(steam->client_hdev, steam);
+
+ /*
+ * With the real steam controller interface, do not connect hidraw.
+ * Instead, create the client_hid and connect that.
+ */
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
+ if (ret)
+ goto hid_hw_start_fail;
+
+ ret = hid_add_device(steam->client_hdev);
+ if (ret)
+ goto client_hdev_add_fail;
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:hid_hw_open for wireless\n",
+ __func__);
+ goto hid_hw_open_fail;
+ }
+ hid_info(hdev, "Steam wireless receiver connected");
+ steam_request_conn_status(steam);
+ } else {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(hdev,
+ "%s:steam_register failed with error %d\n",
+ __func__, ret);
+ goto input_register_fail;
+ }
+ }
+
+ mutex_lock(&steam_devices_lock);
+ steam_update_lizard_mode(steam);
+ list_add(&steam->list, &steam_devices);
+ mutex_unlock(&steam_devices_lock);
+
+ return 0;
+
+hid_hw_open_fail:
+input_register_fail:
+client_hdev_add_fail:
+ hid_hw_stop(hdev);
+hid_hw_start_fail:
+ hid_destroy_device(steam->client_hdev);
+client_hdev_fail:
+ cancel_work_sync(&steam->work_connect);
+steam_alloc_fail:
+ hid_err(hdev, "%s: failed with error %d\n",
+ __func__, ret);
+ return ret;
+}
+
+static void steam_remove(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ if (!steam || hdev->group == HID_GROUP_STEAM) {
+ hid_hw_stop(hdev);
+ return;
+ }
+
+ mutex_lock(&steam_devices_lock);
+ list_del(&steam->list);
+ mutex_unlock(&steam_devices_lock);
+
+ hid_destroy_device(steam->client_hdev);
+ steam->client_opened = false;
+ cancel_work_sync(&steam->work_connect);
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ hid_info(hdev, "Steam wireless receiver disconnected");
+ hid_hw_close(hdev);
+ }
+ hid_hw_stop(hdev);
+ steam_unregister(steam);
+}
+
+static void steam_do_connect_event(struct steam_device *steam, bool connected)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->connected = connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (schedule_work(&steam->work_connect) == 0)
+ dbg_hid("%s: connected=%d event already queued\n",
+ __func__, connected);
+}
+
+/*
+ * Some input data in the protocol has the opposite sign.
+ * Clamp the values to 32767..-32767 so that the range is
+ * symmetrical and can be negated safely.
+ */
+static inline s16 steam_le16(u8 *data)
+{
+ s16 x = (s16) le16_to_cpup((__le16 *)data);
+
+ return x == -32768 ? -32767 : x;
+}
+
+/*
+ * The size for this message payload is 60.
+ * The known values are:
+ * (* values are not sent through wireless)
+ * (* accelerator/gyro is disabled by default)
+ * Offset| Type | Mapped to |Meaning
+ * -------+-------+-----------+--------------------------
+ * 4-7 | u32 | -- | sequence number
+ * 8-10 | 24bit | see below | buttons
+ * 11 | u8 | ABS_HAT2Y | left trigger
+ * 12 | u8 | ABS_HAT2X | right trigger
+ * 13-15 | -- | -- | always 0
+ * 16-17 | s16 | ABS_X/ABS_HAT0X | X value
+ * 18-19 | s16 | ABS_Y/ABS_HAT0Y | Y value
+ * 20-21 | s16 | ABS_RX | right-pad X value
+ * 22-23 | s16 | ABS_RY | right-pad Y value
+ * 24-25 | s16 | -- | * left trigger
+ * 26-27 | s16 | -- | * right trigger
+ * 28-29 | s16 | -- | * accelerometer X value
+ * 30-31 | s16 | -- | * accelerometer Y value
+ * 32-33 | s16 | -- | * accelerometer Z value
+ * 34-35 | s16 | -- | gyro X value
+ * 36-36 | s16 | -- | gyro Y value
+ * 38-39 | s16 | -- | gyro Z value
+ * 40-41 | s16 | -- | quaternion W value
+ * 42-43 | s16 | -- | quaternion X value
+ * 44-45 | s16 | -- | quaternion Y value
+ * 46-47 | s16 | -- | quaternion Z value
+ * 48-49 | -- | -- | always 0
+ * 50-51 | s16 | -- | * left trigger (uncalibrated)
+ * 52-53 | s16 | -- | * right trigger (uncalibrated)
+ * 54-55 | s16 | -- | * joystick X value (uncalibrated)
+ * 56-57 | s16 | -- | * joystick Y value (uncalibrated)
+ * 58-59 | s16 | -- | * left-pad X value
+ * 60-61 | s16 | -- | * left-pad Y value
+ * 62-63 | u16 | -- | * battery voltage
+ *
+ * The buttons are:
+ * Bit | Mapped to | Description
+ * ------+------------+--------------------------------
+ * 8.0 | BTN_TR2 | right trigger fully pressed
+ * 8.1 | BTN_TL2 | left trigger fully pressed
+ * 8.2 | BTN_TR | right shoulder
+ * 8.3 | BTN_TL | left shoulder
+ * 8.4 | BTN_Y | button Y
+ * 8.5 | BTN_B | button B
+ * 8.6 | BTN_X | button X
+ * 8.7 | BTN_A | button A
+ * 9.0 | BTN_DPAD_UP | lef-pad up
+ * 9.1 | BTN_DPAD_RIGHT | lef-pad right
+ * 9.2 | BTN_DPAD_LEFT | lef-pad left
+ * 9.3 | BTN_DPAD_DOWN | lef-pad down
+ * 9.4 | BTN_SELECT | menu left
+ * 9.5 | BTN_MODE | steam logo
+ * 9.6 | BTN_START | menu right
+ * 9.7 | BTN_GEAR_DOWN | left back lever
+ * 10.0 | BTN_GEAR_UP | right back lever
+ * 10.1 | -- | left-pad clicked
+ * 10.2 | BTN_THUMBR | right-pad clicked
+ * 10.3 | BTN_THUMB | left-pad touched (but see explanation below)
+ * 10.4 | BTN_THUMB2 | right-pad touched
+ * 10.5 | -- | unknown
+ * 10.6 | BTN_THUMBL | joystick clicked
+ * 10.7 | -- | lpad_and_joy
+ */
+
+static void steam_do_input_event(struct steam_device *steam,
+ struct input_dev *input, u8 *data)
+{
+ /* 24 bits of buttons */
+ u8 b8, b9, b10;
+ s16 x, y;
+ bool lpad_touched, lpad_and_joy;
+
+ b8 = data[8];
+ b9 = data[9];
+ b10 = data[10];
+
+ input_report_abs(input, ABS_HAT2Y, data[11]);
+ input_report_abs(input, ABS_HAT2X, data[12]);
+
+ /*
+ * These two bits tells how to interpret the values X and Y.
+ * lpad_and_joy tells that the joystick and the lpad are used at the
+ * same time.
+ * lpad_touched tells whether X/Y are to be read as lpad coord or
+ * joystick values.
+ * (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
+ */
+ lpad_touched = b10 & BIT(3);
+ lpad_and_joy = b10 & BIT(7);
+ x = steam_le16(data + 16);
+ y = -steam_le16(data + 18);
+
+ input_report_abs(input, lpad_touched ? ABS_HAT0X : ABS_X, x);
+ input_report_abs(input, lpad_touched ? ABS_HAT0Y : ABS_Y, y);
+ /* Check if joystick is centered */
+ if (lpad_touched && !lpad_and_joy) {
+ input_report_abs(input, ABS_X, 0);
+ input_report_abs(input, ABS_Y, 0);
+ }
+ /* Check if lpad is untouched */
+ if (!(lpad_touched || lpad_and_joy)) {
+ input_report_abs(input, ABS_HAT0X, 0);
+ input_report_abs(input, ABS_HAT0Y, 0);
+ }
+
+ input_report_abs(input, ABS_RX, steam_le16(data + 20));
+ input_report_abs(input, ABS_RY, -steam_le16(data + 22));
+
+ input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
+ input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
+ input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
+ input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
+ input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
+ input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
+ input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
+ input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
+ input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
+ input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
+ input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
+ input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & BIT(7)));
+ input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & BIT(0)));
+ input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & BIT(2)));
+ input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
+ input_event(input, EV_KEY, BTN_THUMB, lpad_touched || lpad_and_joy);
+ input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(4)));
+ input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
+ input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
+ input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
+ input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
+
+ input_sync(input);
+}
+
+static int steam_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+ struct input_dev *input;
+
+ if (!steam)
+ return 0;
+
+ if (steam->client_opened)
+ hid_input_report(steam->client_hdev, HID_FEATURE_REPORT,
+ data, size, 0);
+ /*
+ * All messages are size=64, all values little-endian.
+ * The format is:
+ * Offset| Meaning
+ * -------+--------------------------------------------
+ * 0-1 | always 0x01, 0x00, maybe protocol version?
+ * 2 | type of message
+ * 3 | length of the real payload (not checked)
+ * 4-n | payload data, depends on the type
+ *
+ * There are these known types of message:
+ * 0x01: input data (60 bytes)
+ * 0x03: wireless connect/disconnect (1 byte)
+ * 0x04: battery status (11 bytes)
+ */
+
+ if (size != 64 || data[0] != 1 || data[1] != 0)
+ return 0;
+
+ switch (data[2]) {
+ case STEAM_EV_INPUT_DATA:
+ if (steam->client_opened)
+ return 0;
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ if (likely(input)) {
+ steam_do_input_event(steam, input, data);
+ } else {
+ dbg_hid("%s: input data without connect event\n",
+ __func__);
+ steam_do_connect_event(steam, true);
+ }
+ rcu_read_unlock();
+ break;
+ case STEAM_EV_CONNECT:
+ /*
+ * The payload of this event is a single byte:
+ * 0x01: disconnected.
+ * 0x02: connected.
+ */
+ switch (data[4]) {
+ case 0x01:
+ steam_do_connect_event(steam, false);
+ break;
+ case 0x02:
+ steam_do_connect_event(steam, true);
+ break;
+ }
+ break;
+ case STEAM_EV_BATTERY:
+ /* TODO: battery info */
+ break;
+ }
+ return 0;
+}
+
+static int steam_param_set_lizard_mode(const char *val,
+ const struct kernel_param *kp)
+{
+ struct steam_device *steam;
+ int ret;
+
+ ret = param_set_bool(val, kp);
+ if (ret)
+ return ret;
+
+ mutex_lock(&steam_devices_lock);
+ list_for_each_entry(steam, &steam_devices, list) {
+ steam_update_lizard_mode(steam);
+ }
+ mutex_unlock(&steam_devices_lock);
+ return 0;
+}
+
+static const struct kernel_param_ops steam_lizard_mode_ops = {
+ .set = steam_param_set_lizard_mode,
+ .get = param_get_bool,
+};
+
+module_param_cb(lizard_mode, &steam_lizard_mode_ops, &lizard_mode, 0644);
+MODULE_PARM_DESC(lizard_mode,
+ "Enable mouse and keyboard emulation (lizard mode) when the gamepad is not in use");
+
+static const struct hid_device_id steam_controllers[] = {
+ { /* Wired Steam Controller */
+ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+ USB_DEVICE_ID_STEAM_CONTROLLER)
+ },
+ { /* Wireless Steam Controller */
+ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+ USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS),
+ .driver_data = STEAM_QUIRK_WIRELESS
+ },
+ {}
+};
+
+MODULE_DEVICE_TABLE(hid, steam_controllers);
+
+static struct hid_driver steam_controller_driver = {
+ .name = "hid-steam",
+ .id_table = steam_controllers,
+ .probe = steam_probe,
+ .remove = steam_remove,
+ .raw_event = steam_raw_event,
+};
+
+module_hid_driver(steam_controller_driver);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d491027a7c22..5e5d76589954 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -364,6 +364,7 @@ struct hid_item {
#define HID_GROUP_RMI 0x0100
#define HID_GROUP_WACOM 0x0101
#define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102
+#define HID_GROUP_STEAM 0x0103
/*
* HID protocol status
--
2.17.0
^ permalink raw reply related
* [PATCH v9 2/2] HID: steam: add battery device.
From: Rodrigo Rivas Costa @ 2018-04-16 12:27 UTC (permalink / raw)
To: Benjamin Tissoires, Pierre-Loup A. Griffais,
Clément VUCHENER, Jiri Kosina, Cameron Gutman, lkml,
linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180416122703.22306-1-rodrigorivascosta@gmail.com>
The wireless Steam Controller is battery operated, so add the battery
device and power information.
---
drivers/hid/hid-steam.c | 141 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 36fc85714ea5..af7ebb618867 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -40,6 +40,7 @@
#include <linux/mutex.h>
#include <linux/rcupdate.h>
#include <linux/delay.h>
+#include <linux/power_supply.h>
#include "hid-ids.h"
MODULE_LICENSE("GPL");
@@ -118,6 +119,10 @@ struct steam_device {
struct work_struct work_connect;
bool connected;
char serial_no[STEAM_SERIAL_LEN + 1];
+ struct power_supply_desc battery_desc;
+ struct power_supply __rcu *battery;
+ u8 battery_charge;
+ u16 voltage;
};
static int steam_recv_report(struct steam_device *steam,
@@ -316,6 +321,85 @@ static void steam_input_close(struct input_dev *dev)
hid_hw_close(steam->hdev);
}
+static enum power_supply_property steam_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steam_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct steam_device *steam = power_supply_get_drvdata(psy);
+ unsigned long flags;
+ s16 volts;
+ u8 batt;
+ int ret = 0;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ volts = steam->voltage;
+ batt = steam->battery_charge;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = volts * 1000; /* mV -> uV */
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = batt;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int steam_battery_register(struct steam_device *steam)
+{
+ struct power_supply *battery;
+ struct power_supply_config battery_cfg = { .drv_data = steam, };
+ unsigned long flags;
+ int ret;
+
+ steam->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ steam->battery_desc.properties = steam_battery_props;
+ steam->battery_desc.num_properties = ARRAY_SIZE(steam_battery_props);
+ steam->battery_desc.get_property = steam_battery_get_property;
+ steam->battery_desc.name = devm_kasprintf(&steam->hdev->dev,
+ GFP_KERNEL, "steam-controller-%s-battery",
+ steam->serial_no);
+ if (!steam->battery_desc.name)
+ return -ENOMEM;
+
+ /* avoid the warning of 0% battery while waiting for the first info */
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = 3000;
+ steam->battery_charge = 100;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ battery = power_supply_register(&steam->hdev->dev,
+ &steam->battery_desc, &battery_cfg);
+ if (IS_ERR(battery)) {
+ ret = PTR_ERR(battery);
+ hid_err(steam->hdev,
+ "%s:power_supply_register failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+ rcu_assign_pointer(steam->battery, battery);
+ power_supply_powers(battery, &steam->hdev->dev);
+ return 0;
+}
+
static int steam_register(struct steam_device *steam)
{
struct hid_device *hdev = steam->hdev;
@@ -409,6 +493,10 @@ static int steam_register(struct steam_device *steam)
rcu_assign_pointer(steam->input, input);
+ /* ignore battery errors, we can live without it */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS)
+ steam_battery_register(steam);
+
return 0;
input_register_fail:
@@ -419,11 +507,18 @@ static int steam_register(struct steam_device *steam)
static void steam_unregister(struct steam_device *steam)
{
struct input_dev *input;
+ struct power_supply *battery;
rcu_read_lock();
input = rcu_dereference(steam->input);
+ battery = rcu_dereference(steam->battery);
rcu_read_unlock();
+ if (battery) {
+ RCU_INIT_POINTER(steam->battery, NULL);
+ synchronize_rcu();
+ power_supply_unregister(battery);
+ }
if (input) {
RCU_INIT_POINTER(steam->input, NULL);
synchronize_rcu();
@@ -851,12 +946,44 @@ static void steam_do_input_event(struct steam_device *steam,
input_sync(input);
}
+/*
+ * The size for this message payload is 11.
+ * The known values are:
+ * Offset| Type | Meaning
+ * -------+-------+---------------------------
+ * 4-7 | u32 | sequence number
+ * 8-11 | -- | always 0
+ * 12-13 | u16 | voltage (mV)
+ * 14 | u8 | battery percent
+ */
+static void steam_do_battery_event(struct steam_device *steam,
+ struct power_supply *battery, u8 *data)
+{
+ unsigned long flags;
+
+ s16 volts = steam_le16(data + 12);
+ u8 batt = data[14];
+
+ /* Creating the battery may have failed */
+ rcu_read_lock();
+ battery = rcu_dereference(steam->battery);
+ if (likely(battery)) {
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = volts;
+ steam->battery_charge = batt;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ power_supply_changed(battery);
+ }
+ rcu_read_unlock();
+}
+
static int steam_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data,
int size)
{
struct steam_device *steam = hid_get_drvdata(hdev);
struct input_dev *input;
+ struct power_supply *battery;
if (!steam)
return 0;
@@ -914,7 +1041,19 @@ static int steam_raw_event(struct hid_device *hdev,
}
break;
case STEAM_EV_BATTERY:
- /* TODO: battery info */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ rcu_read_lock();
+ battery = rcu_dereference(steam->battery);
+ if (likely(battery)) {
+ steam_do_battery_event(steam, battery, data);
+ } else {
+ dbg_hid(
+ "%s: battery data without connect event\n",
+ __func__);
+ steam_do_connect_event(steam, true);
+ }
+ rcu_read_unlock();
+ }
break;
}
return 0;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v6 28/30] drm/rockchip: Disable PSR from reboot notifier
From: Tomasz Figa @ 2018-04-16 13:12 UTC (permalink / raw)
To: a.hajda
Cc: David Airlie, dri-devel, Douglas Anderson, Thierry Reding,
Laurent Pinchart, Yakir Yang, kernel, Marek Szyprowski,
linux-samsung-soc, rydberg, Krzysztof Kozlowski,
open list:ARM/Rockchip SoC..., kgene, linux-input,
Ørjan Eide, Caesar Wang, Jeffy,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Mark yao, 王征增
In-Reply-To: <a9e436f8-34ec-123a-9c69-efabb4576bea@samsung.com>
Hi Andrzej,
On Mon, Apr 16, 2018 at 6:57 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> > From: Tomasz Figa <tfiga@chromium.org>
> >
> > It looks like the driver subsystem detaches devices from power domains
> > at shutdown without consent of the drivers.
> It looks bit strange. Could you elaborate more on it. Could you show the
> code performing the detach?
It not only looks strange, but it is strange. The code was present in 4.4:
https://elixir.bootlin.com/linux/v4.4.128/source/drivers/base/platform.c#L553
but was apparently removed in 4.5:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/base/platform.c?h=next-20180416&id=2d30bb0b3889adf09b342722b2ce596c0763bc93
So we might not need this patch anymore.
Best regards,
Tomasz
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Benjamin Tissoires @ 2018-04-16 13:36 UTC (permalink / raw)
To: Robert Munteanu; +Cc: Jiri Kosina, lkml, open list:HID CORE LAYER
In-Reply-To: <149a44d0456b2f0b48be13ba51aeaccc33651073.camel@apache.org>
On Mon, Apr 16, 2018 at 12:14 PM, Robert Munteanu <rombert@apache.org> wrote:
> Hi Benjamin and Jiri,
>
> On Mon, 2018-04-16 at 12:02 +0200, Benjamin Tissoires wrote:
>> Hi Robert,
>>
>> On Wed, Apr 11, 2018 at 11:49 AM, Robert Munteanu <rombert@apache.org
>> > wrote:
>> > Changelog:
>> >
>> > - v2: modifier keys work, some combinations are still troublesome
>> > - v3: style cleanup, rebase on top of 4.14
>> > - v4: remove most debugging calls, make init info useful for user,
>> > rebased on top of 4.15
>> > - v5: fix the HID descriptor as suggested by Benjamin Tissoires,
>> > use existing USB vendor id, update comment style, add SPDX
>> > license identifier, rename to hid-redragon, stop registering
>> > two input devices, rebased on top of 4.16
>>
>> As Jiri said, please provide a correct commit message.
>
> Will do.
>
>>
>> I have a few nitpicks in the driver, v6 should be fine:
>>
>> >
>> > Signed-off-by: Robert Munteanu <rombert@apache.org>
>> > ---
>> > drivers/hid/Kconfig | 7 ++++
>> > drivers/hid/Makefile | 1 +
>> > drivers/hid/hid-ids.h | 1 +
>> > drivers/hid/hid-quirks.c | 3 ++
>> > drivers/hid/hid-redragon.c | 89
>> > ++++++++++++++++++++++++++++++++++++++++++++++
>> > 5 files changed, 101 insertions(+)
>> > create mode 100644 drivers/hid/hid-redragon.c
>> >
>> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> > index 19c499f5623d..1125e4813716 100644
>> > --- a/drivers/hid/Kconfig
>> > +++ b/drivers/hid/Kconfig
>> > @@ -560,6 +560,13 @@ config HID_MAYFLASH
>> > Say Y here if you have HJZ Mayflash PS3 game controller
>> > adapters
>> > and want to enable force feedback support.
>> >
>> > +config HID_REDRAGON
>> > + tristate "Redragon keyboards"
>> > + depends on HID
>> > + default !EXPERT
>> > + ---help---
>> > + Support for Redragon keyboards that need fix-ups to work
>> > properly.
>> > +
>> > config HID_MICROSOFT
>> > tristate "Microsoft non-fully HID-compliant devices"
>> > depends on HID
>> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> > index eb13b9e92d85..a36f3f40ba63 100644
>> > --- a/drivers/hid/Makefile
>> > +++ b/drivers/hid/Makefile
>> > @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) +=
>> > hid-picolcd_debugfs.o
>> >
>> > obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o
>> > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
>> > +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o
>> > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o
>> > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o
>> > \
>> > hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
>> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> > index 9454ac134ce2..41a64d0e91f9 100644
>> > --- a/drivers/hid/hid-ids.h
>> > +++ b/drivers/hid/hid-ids.h
>> > @@ -599,6 +599,7 @@
>> > #define USB_VENDOR_ID_JESS 0x0c45
>> > #define USB_DEVICE_ID_JESS_YUREX 0x1010
>> > #define USB_DEVICE_ID_ASUS_MD_5112 0x5112
>> > +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b
>> >
>> > #define USB_VENDOR_ID_JESS2 0x0f30
>> > #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111
>> > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> > index e92b77fa574a..5f1253f1739a 100644
>> > --- a/drivers/hid/hid-quirks.c
>> > +++ b/drivers/hid/hid-quirks.c
>> > @@ -557,6 +557,9 @@ static const struct hid_device_id
>> > hid_have_special_driver[] = {
>> > #if IS_ENABLED(CONFIG_HID_PRODIKEYS)
>> > { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS,
>> > USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
>> > #endif
>> > +#if IS_ENABLED(CONFIG_HID_REDRAGON)
>> > + {
>> > HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)
>> > },
>> > +#endif
>>
>> Please drop this hunk. v4.16 should work without changing
>> hid_have_special_driver. This way, you will be sure that an initramfs
>> that doesn't include hid-redragon.ko will allo people to type their
>> LUKS password.
>
> I recall testing without this change resulted in the driver not being
> picked up. I will retest ( running 4.16.0 here, can update to 4.16.1
> soon ). In case the driver is not picked up, where should I start
> looking?
>
>>
>> > #if IS_ENABLED(CONFIG_HID_RETRODE)
>> > { HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
>> > USB_DEVICE_ID_RETRODE2) },
>> > #endif
>> > diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-
>> > redragon.c
>> > new file mode 100644
>> > index 000000000000..ff98a5dbb8e2
>> > --- /dev/null
>> > +++ b/drivers/hid/hid-redragon.c
>> > @@ -0,0 +1,89 @@
>> > +/*
>> > + * HID driver for Redragon keyboards
>> > + *
>> > + * Copyright (c) 2017 Robert Munteanu
>> > + * SPDX-License-Identifier: GPL-2.0
>> > + */
>> > +
>> > +/*
>> > + * This program is free software; you can redistribute it and/or
>> > modify it
>> > + * under the terms of the GNU General Public License as published
>> > by the Free
>> > + * Software Foundation; either version 2 of the License, or (at
>> > your option)
>> > + * any later version.
>> > + */
>> > +
>> > +#include <linux/device.h>
>> > +#include <linux/input.h>
>>
>> you probably don't need input.h
>>
>> > +#include <linux/hid.h>
>> > +#include <linux/module.h>
>> > +#include <linux/log2.h>
>>
>> log2.h? I am not sure you need it either
>>
>> > +#include <linux/input-event-codes.h>
>>
>> probably drop this one too.
>
> I'll drop the missing imports, those are leftovers from my initial
> work.
>
>>
>> > +
>> > +#include "hid-ids.h"
>> > +
>> > +
>> > +/*
>> > + * The Redragon Asura keyboard sends an incorrect HID descriptor.
>> > + * At byte 100 it contains
>> > + *
>> > + * 0x81, 0x00
>> > + *
>> > + * which is Input (Data, Arr, Abs), but it should be
>> > + *
>> > + * 0x81, 0x02
>> > + *
>> > + * which is Input (Data, Var, Abs), which is consistent with the
>> > way
>> > + * key codes are generated.
>> > + */
>> > +
>> > +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8
>> > *rdesc,
>> > + unsigned int *rsize)
>> > +{
>> > + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] ==
>> > 0x00) {
>> > + dev_info(&hdev->dev, "Fixing Redragon ASURA report
>> > descriptor.\n");
>> > + rdesc[101] = 0x02;
>> > + }
>> > +
>> > + return rdesc;
>> > +}
>> > +
>> > +static int redragon_probe(struct hid_device *dev,
>> > + const struct hid_device_id *id)
>> > +{
>> > + int ret;
>> > +
>> > + ret = hid_parse(dev);
>> > + if (ret) {
>> > + hid_err(dev, "parse failed\n");
>> > + return ret;
>> > + }
>> > +
>> > + /* do not register unused input device */
>> > + if (dev->maxapplication == 1)
>> > + return 0;
>> > +
>> > + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
>> > + if (ret) {
>> > + hid_err(dev, "hw start failed\n");
>> > + return ret;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +static const struct hid_device_id redragon_devices[] = {
>> > + {HID_USB_DEVICE(USB_VENDOR_ID_JESS,
>> > USB_DEVICE_ID_REDRAGON_ASURA)},
>> > + {}
>> > +};
>> > +
>> > +MODULE_DEVICE_TABLE(hid, redragon_devices);
>> > +
>> > +static struct hid_driver redragon_driver = {
>> > + .name = "redragon",
>> > + .id_table = redragon_devices,
>> > + .report_fixup = redragon_report_fixup,
>> > + .probe = redragon_probe
>> > +};
>> > +
>> > +module_hid_driver(redragon_driver);
>> > +
>> > +MODULE_LICENSE("GPL");
>>
>> The SPDX header says GPL-v2. And IIRC if there is the SPDX header you
>> can drop the MODULE_LICENSE (not entirely sure though).
>
> Ack, will adjust and re-test.
Actually, you might be correct. I just read Mauro's blog:
https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx/
and it says "Types of licenses for MODULE_LICENSE() macro -> 'GPL' ->
GNU Public License v2 or later" (in the second table).
Cheers,
Benjamin
>
> Thanks for the review,
>
> Robert
^ permalink raw reply
* [GIT PULL] Immutable branch between MFD, Input and RTC due for the v4.18 merge window
From: Lee Jones @ 2018-04-16 14:18 UTC (permalink / raw)
To: Chen Zhong
Cc: Dmitry Torokhov, Rob Herring, Alexandre Belloni, Mark Rutland,
Matthias Brugger, Eddie Huang, Alessandro Zummo, Linus Walleij,
Beomho Seo, Javier Martinez Canillas, Jaechul Lee,
Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, linux-rtc
In-Reply-To: <1508937364-30054-1-git-send-email-chen.zhong@mediatek.com>
Enjoy!
The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:
Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-input-rtc-v4.18
for you to fetch changes up to 55d1d1547ab5e52269988af5e7c553796ff68e97:
mfd: mt6397: Add PMIC keys support to MT6397 driver (2018-04-16 15:16:11 +0100)
----------------------------------------------------------------
Immutable branch between MFD, Input and RTC due for the v4.18 merge window
----------------------------------------------------------------
Chen Zhong (5):
mfd: mt6397: Create irq mappings in mfd core driver
dt-bindings: input: Add document bindings for mtk-pmic-keys
dt-bindings: mfd: Add bindings for the keys as subnode of PMIC
input: Add MediaTek PMIC keys support
mfd: mt6397: Add PMIC keys support to MT6397 driver
.../devicetree/bindings/input/mtk-pmic-keys.txt | 43 +++
Documentation/devicetree/bindings/mfd/mt6397.txt | 6 +
drivers/input/keyboard/Kconfig | 9 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-pmic-keys.c | 339 +++++++++++++++++++++
drivers/mfd/mt6397-core.c | 26 +-
drivers/rtc/rtc-mt6397.c | 7 +-
7 files changed, 424 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
create mode 100644 drivers/input/keyboard/mtk-pmic-keys.c
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v2] Input: add bu21029 touch driver
From: Mark Jonas @ 2018-04-16 16:49 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland
Cc: linux-input, devicetree, linux-kernel, hs, Zhu Yi, Mark Jonas
In-Reply-To: <1521651874-15379-1-git-send-email-mark.jonas@de.bosch.com>
From: Zhu Yi <yi.zhu5@cn.bosch.com>
Add Rohm BU21029 resistive touch panel controller support with I2C
interface.
Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
Reviewed-by: Heiko Schocher <hs@denx.de>
---
Changes in v2:
- make ABS_PRESSURE proportionally rising with finger pressure
- fix race between interrupt and timer during shutdown
- use infrastructure from include/linux/input/touchscreen.h
- add SPDX tag for the driver
- improve binding documentation
- fix multi-line comments
---
.../bindings/input/touchscreen/bu21029.txt | 34 ++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bu21029_ts.c | 485 +++++++++++++++++++++
4 files changed, 532 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
create mode 100644 drivers/input/touchscreen/bu21029_ts.c
diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
new file mode 100644
index 0000000..030a888
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
@@ -0,0 +1,34 @@
+* Rohm BU21029 Touch Screen Controller
+
+Required properties:
+ - compatible : must be "rohm,bu21029"
+ - reg : i2c device address of the chip (0x40 or 0x41)
+ - interrupt-parent : the phandle for the gpio controller
+ - interrupts : (gpio) interrupt to which the chip is connected
+ - reset-gpios : gpio pin to reset the chip (active low)
+ - rohm,x-plate-ohms : x-plate resistance in Ohm
+
+Optional properties:
+ - touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y : vertical resolution of touchscreen (in pixels)
+ - touchscreen-max-pressure: maximum pressure value
+
+Example:
+
+ &i2c1 {
+ /* ... */
+
+ bu21029: bu21029@40 {
+ compatible = "rohm,bu21029";
+ reg = <0x40>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
+ rohm,x-plate-ohms = <600>;
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ touchscreen-max-pressure = <4095>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4f15496..e09fe8f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -151,6 +151,18 @@ config TOUCHSCREEN_BU21013
To compile this driver as a module, choose M here: the
module will be called bu21013_ts.
+config TOUCHSCREEN_BU21029
+ tristate "Rohm BU21029 based touch panel controllers"
+ depends on I2C
+ help
+ Say Y here if you have a Rohm BU21029 touchscreen controller
+ connected to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21029_ts.
+
config TOUCHSCREEN_CHIPONE_ICN8318
tristate "chipone icn8318 touchscreen controller"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index dddae79..f50624c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110) += cy8ctmg110_ts.o
obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE) += cyttsp_core.o
diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c
new file mode 100644
index 0000000..8d0cbe51
--- /dev/null
+++ b/drivers/input/touchscreen/bu21029_ts.c
@@ -0,0 +1,485 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rohm BU21029 touchscreen controller driver
+ *
+ * Copyright (C) 2015-2018 Bosch Sicherheitssysteme GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/timer.h>
+
+/*
+ * HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDH |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_ID2 Register (PAGE=0, ADDR=0x0F, Reset value=0x29, Read only)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | HW_IDL |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * HW_IDH: high 8bits of IC's ID
+ * HW_IDL: low 8bits of IC's ID
+ */
+#define BU21029_HWID_REG (0x0E << 3)
+#define SUPPORTED_HWID 0x0229
+
+/*
+ * CFR0 Register (PAGE=0, ADDR=0x00, Reset value=0x20)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | 0 | CALIB | INTRM | 0 | 0 | 0 | 0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CALIB: 0 = not to use calibration result (*)
+ * 1 = use calibration result
+ * INTRM: 0 = INT output depend on "pen down" (*)
+ * 1 = INT output always "0"
+ */
+#define BU21029_CFR0_REG (0x00 << 3)
+#define CFR0_VALUE 0x00
+
+/*
+ * CFR1 Register (PAGE=0, ADDR=0x01, Reset value=0xA6)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | MAV | AVE[2:0] | 0 | SMPL[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * MAV: 0 = median average filter off
+ * 1 = median average filter on (*)
+ * AVE: AVE+1 = number of average samples for MAV,
+ * if AVE>SMPL, then AVE=SMPL (=3)
+ * SMPL: SMPL+1 = number of conversion samples for MAV (=7)
+ */
+#define BU21029_CFR1_REG (0x01 << 3)
+#define CFR1_VALUE 0xA6
+
+/*
+ * CFR2 Register (PAGE=0, ADDR=0x02, Reset value=0x04)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | INTVL_TIME[3:0] | TIME_ST_ADC[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * INTVL_TIME: waiting time between completion of conversion
+ * and start of next conversion, only usable in
+ * autoscan mode (=20.480ms)
+ * TIME_ST_ADC: waiting time between application of voltage
+ * to panel and start of A/D conversion (=100us)
+ */
+#define BU21029_CFR2_REG (0x02 << 3)
+#define CFR2_VALUE 0xC9
+
+/*
+ * CFR3 Register (PAGE=0, ADDR=0x0B, Reset value=0x72)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | RM8 | STRETCH| PU90K | DUAL | PIDAC_OFS[3:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * RM8: 0 = coordinate resolution is 12bit (*)
+ * 1 = coordinate resolution is 8bit
+ * STRETCH: 0 = SCL_STRETCH function off
+ * 1 = SCL_STRETCH function on (*)
+ * PU90K: 0 = internal pull-up resistance for touch detection is ~50kohms (*)
+ * 1 = internal pull-up resistance for touch detection is ~90kohms
+ * DUAL: 0 = dual touch detection off (*)
+ * 1 = dual touch detection on
+ * PIDAC_OFS: dual touch detection circuit adjustment, it is not necessary
+ * to change this from initial value
+ */
+#define BU21029_CFR3_REG (0x0B << 3)
+#define CFR3_VALUE 0x42
+
+/*
+ * LDO Register (PAGE=0, ADDR=0x0C, Reset value=0x00)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 0 | PVDD[2:0] | 0 | AVDD[2:0] |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * PVDD: output voltage of panel output regulator (=2.000V)
+ * AVDD: output voltage of analog circuit regulator (=2.000V)
+ */
+#define BU21029_LDO_REG (0x0C << 3)
+#define LDO_VALUE 0x77
+
+/*
+ * Serial Interface Command Byte 1 (CID=1)
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * | 1 | CF | CMSK | PDM | STP |
+ * +--------+--------+--------+--------+--------+--------+--------+--------+
+ * CF: conversion function, see table 3 in datasheet p6 (=0000, automatic scan)
+ * CMSK: 0 = executes convert function (*)
+ * 1 = reads the convert result
+ * PDM: 0 = power down after convert function stops (*)
+ * 1 = keep power on after convert function stops
+ * STP: 1 = abort current conversion and power down, set to "0" automatically
+ */
+#define BU21029_AUTOSCAN 0x80
+
+/*
+ * The timeout value needs to be larger than INTVL_TIME + tConv4 (sample and
+ * conversion time), where tConv4 is calculated by formula:
+ * tPON + tDLY1 + (tTIME_ST_ADC + (tADC * tSMPL) * 2 + tDLY2) * 3
+ * see figure 8 in datasheet p15 for details of each field.
+ */
+#define PEN_UP_TIMEOUT msecs_to_jiffies(50)
+
+#define STOP_DELAY_US 50L
+#define START_DELAY_MS 2L
+#define BUF_LEN 8L
+#define SCALE_12BIT (1 << 12)
+#define MAX_12BIT ((1 << 12) - 1)
+#define DRIVER_NAME "bu21029"
+
+struct bu21029_ts_data {
+ struct i2c_client *client;
+ struct input_dev *in_dev;
+ struct timer_list timer;
+ struct gpio_desc *reset_gpios;
+ u32 x_plate_ohms;
+ struct touchscreen_properties prop;
+};
+
+static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
+{
+ struct i2c_client *i2c = bu21029->client;
+ u8 buf[BUF_LEN];
+ u16 x, y, z1, z2;
+ u32 rz;
+ s32 max_pressure = bu21029->in_dev->absinfo[ABS_PRESSURE].maximum;
+
+ /* read touch data and deassert INT (by restarting the autoscan mode) */
+ int error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_AUTOSCAN,
+ BUF_LEN,
+ buf);
+ if (error < 0)
+ return error;
+
+ /*
+ * compose upper 8 and lower 4 bits into a 12bit value:
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * | ByteH | ByteL |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |b07|b06|b05|b04|b03|b02|b01|b00|b07|b06|b05|b04|b03|b02|b01|b00|
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ * |v11|v10|v09|v08|v07|v06|v05|v04|v03|v02|v01|v00| 0 | 0 | 0 | 0 |
+ * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+ */
+ x = (buf[0] << 4) | (buf[1] >> 4);
+ y = (buf[2] << 4) | (buf[3] >> 4);
+ z1 = (buf[4] << 4) | (buf[5] >> 4);
+ z2 = (buf[6] << 4) | (buf[7] >> 4);
+
+ if (z1 == 0 || z2 == 0)
+ return 0;
+
+ /*
+ * calculate Rz (pressure resistance value) by equation:
+ * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
+ * Rx is x-plate resistance,
+ * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
+ * x, z1, z2 are the measured positions.
+ */
+ rz = z2 - z1;
+ rz *= x;
+ rz *= bu21029->x_plate_ohms;
+ rz /= z1;
+ rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
+ if (rz <= max_pressure) {
+ touchscreen_report_pos(bu21029->in_dev, &bu21029->prop,
+ x, y, false);
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE,
+ max_pressure - rz);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 1);
+ input_sync(bu21029->in_dev);
+ }
+
+ return 0;
+}
+
+static void bu21029_touch_release(struct timer_list *t)
+{
+ struct bu21029_ts_data *bu21029 = from_timer(bu21029, t, timer);
+
+ input_report_abs(bu21029->in_dev, ABS_PRESSURE, 0);
+ input_report_key(bu21029->in_dev, BTN_TOUCH, 0);
+ input_sync(bu21029->in_dev);
+}
+
+static irqreturn_t bu21029_touch_soft_irq(int irq, void *data)
+{
+ struct bu21029_ts_data *bu21029 = data;
+ struct i2c_client *i2c = bu21029->client;
+
+ /*
+ * report touch and deassert interrupt (will assert again after
+ * INTVL_TIME + tConv4 for continuous touch)
+ */
+ int error = bu21029_touch_report(bu21029);
+
+ if (error) {
+ dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
+ return IRQ_NONE;
+ }
+
+ /* reset timer for pen up detection */
+ mod_timer(&bu21029->timer, jiffies + PEN_UP_TIMEOUT);
+
+ return IRQ_HANDLED;
+}
+
+static void bu21029_stop_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+
+ disable_irq(bu21029->client->irq);
+ del_timer_sync(&bu21029->timer);
+
+ /* put chip into reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
+ udelay(STOP_DELAY_US);
+}
+
+static int bu21029_start_chip(struct input_dev *dev)
+{
+ struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
+ struct i2c_client *i2c = bu21029->client;
+ struct {
+ u8 reg;
+ u8 value;
+ } init_table[] = {
+ {BU21029_CFR0_REG, CFR0_VALUE},
+ {BU21029_CFR1_REG, CFR1_VALUE},
+ {BU21029_CFR2_REG, CFR2_VALUE},
+ {BU21029_CFR3_REG, CFR3_VALUE},
+ {BU21029_LDO_REG, LDO_VALUE}
+ };
+ int error, i;
+ u16 hwid;
+
+ /* take chip out of reset */
+ gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
+ mdelay(START_DELAY_MS);
+
+ error = i2c_smbus_read_i2c_block_data(i2c,
+ BU21029_HWID_REG,
+ 2,
+ (u8 *)&hwid);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to read HW ID\n");
+ goto out;
+ }
+
+ if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
+ dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
+ error = -ENODEV;
+ goto out;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(init_table); ++i) {
+ error = i2c_smbus_write_byte_data(i2c,
+ init_table[i].reg,
+ init_table[i].value);
+ if (error < 0) {
+ dev_err(&i2c->dev,
+ "failed to write 0x%x to register 0x%x\n",
+ init_table[i].value,
+ init_table[i].reg);
+ goto out;
+ }
+ }
+
+ error = i2c_smbus_write_byte(i2c, BU21029_AUTOSCAN);
+ if (error < 0) {
+ dev_err(&i2c->dev, "failed to start autoscan\n");
+ goto out;
+ }
+
+ enable_irq(bu21029->client->irq);
+ return 0;
+
+out:
+ bu21029_stop_chip(dev);
+ return error;
+}
+
+static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
+{
+ struct device *dev = &bu21029->client->dev;
+ struct device_node *np = dev->of_node;
+ u32 val32;
+ int error;
+
+ if (!np) {
+ dev_err(dev, "no device tree data\n");
+ return -EINVAL;
+ }
+
+ bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(bu21029->reset_gpios)) {
+ error = PTR_ERR(bu21029->reset_gpios);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev, "invalid 'reset-gpios':%d\n", error);
+ return error;
+ }
+
+ if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
+ dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
+ return -EINVAL;
+ }
+ bu21029->x_plate_ohms = val32;
+
+ touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
+
+ return 0;
+}
+
+static int bu21029_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct bu21029_ts_data *bu21029;
+ struct input_dev *in_dev;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ dev_err(&client->dev,
+ "i2c functionality support is not sufficient\n");
+ return -EIO;
+ }
+
+ bu21029 = devm_kzalloc(&client->dev, sizeof(*bu21029), GFP_KERNEL);
+ if (!bu21029)
+ return -ENOMEM;
+
+ in_dev = devm_input_allocate_device(&client->dev);
+ if (!in_dev) {
+ dev_err(&client->dev, "unable to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ bu21029->client = client;
+ bu21029->in_dev = in_dev;
+ timer_setup(&bu21029->timer, bu21029_touch_release, 0);
+
+ in_dev->name = DRIVER_NAME;
+ in_dev->id.bustype = BUS_I2C;
+ in_dev->open = bu21029_start_chip;
+ in_dev->close = bu21029_stop_chip;
+
+ input_set_capability(in_dev, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(in_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+ input_set_abs_params(in_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
+
+ error = bu21029_parse_dt(bu21029);
+ if (error)
+ return error;
+
+ input_set_drvdata(in_dev, bu21029);
+
+ error = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL,
+ bu21029_touch_soft_irq,
+ IRQF_ONESHOT,
+ DRIVER_NAME,
+ bu21029);
+ if (error) {
+ dev_err(&client->dev, "unable to request touch irq\n");
+ return error;
+ }
+
+ bu21029_stop_chip(in_dev);
+
+ error = input_register_device(in_dev);
+ if (error) {
+ dev_err(&client->dev, "unable to register input device\n");
+ return error;
+ }
+
+ i2c_set_clientdata(client, bu21029);
+
+ return 0;
+}
+
+static int bu21029_remove(struct i2c_client *client)
+{
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
+
+ bu21029_stop_chip(bu21029->in_dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bu21029_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_stop_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+
+static int bu21029_resume(struct device *dev)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct bu21029_ts_data *bu21029 = i2c_get_clientdata(i2c);
+
+ mutex_lock(&bu21029->in_dev->mutex);
+ if (bu21029->in_dev->users)
+ bu21029_start_chip(bu21029->in_dev);
+ mutex_unlock(&bu21029->in_dev->mutex);
+
+ return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(bu21029_pm_ops, bu21029_suspend, bu21029_resume);
+
+static const struct i2c_device_id bu21029_ids[] = {
+ {DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bu21029_ids);
+
+static struct i2c_driver bu21029_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .pm = &bu21029_pm_ops,
+ },
+ .id_table = bu21029_ids,
+ .probe = bu21029_probe,
+ .remove = bu21029_remove,
+};
+module_i2c_driver(bu21029_driver);
+
+MODULE_AUTHOR("Zhu Yi <yi.zhu5@cn.bosch.com>");
+MODULE_DESCRIPTION("Rohm BU21029 touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
From: Dmitry Torokhov @ 2018-04-16 17:41 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko, dri-devel, dianders, ykk, kernel,
m.szyprowski, linux-samsung-soc, jy0922.shim, rydberg, krzk,
linux-rockchip, kgene, linux-input, orjan.eide, wxt, jeffy.chen,
linux-arm-kernel, mark.yao, wzz, hl, jingoohan1, sw0312.kim,
linux-kernel, kyungmin.
In-Reply-To: <20d2aa03-63e3-0072-5ca7-5f3ca42c3c9e@samsung.com>
On Mon, Apr 16, 2018 at 09:19:21AM +0200, Andrzej Hajda wrote:
>
> +CC: linux-input list and maintainer
>
>
> On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> > From: "Kristian H. Kristensen" <hoegsberg@google.com>
> >
> > To improve PSR exit latency, we speculatively start exiting when we
> > receive input events. Occasionally, this may lead to false positives,
> > but most of the time we get a head start on coming out of PSR. Depending
> > on how userspace takes to produce a new frame in response to the event,
> > this can completely hide the exit latency. In case of Chrome OS, we
> > typically get the input notifier 50ms or more before the dirty_fb
> > triggered exit.
>
> As I see from the code below, you just need notification from input
> subsystem on user activity.
> Maybe there is some simpler notification mechanism for such things?
> If not, maybe such helper should be created in input subsystem, I
> suppose it could be reused in other places as well.
Creating an input_handler is how you get user activity. It allows you to
decide what devices you are interested in and you get events so you
decide what to do with them.
I am pretty sure using something like atomic notifier is not that much
simpler, but much less flexible.
I wonder though we we really need to open devices when we connect to
them, or if we can leave it as is and only receive events if someone
else opened the device and is consuming events.
Thanks.
>
> Regards
> Andrzej
>
> >
> > Signed-off-by: Kristian H. Kristensen <hoegsberg@google.com>
> > Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >
> > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 134 ++++++++++++++++++++++++++++
> > 1 file changed, 134 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > index 9376f4396b6b..a107845ba97c 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > @@ -12,6 +12,8 @@
> > * GNU General Public License for more details.
> > */
> >
> > +#include <linux/input.h>
> > +
> > #include <drm/drmP.h>
> > #include <drm/drm_crtc_helper.h>
> >
> > @@ -35,6 +37,9 @@ struct psr_drv {
> > enum psr_state state;
> >
> > struct delayed_work flush_work;
> > + struct work_struct disable_work;
> > +
> > + struct input_handler input_handler;
> >
> > int (*set)(struct drm_encoder *encoder, bool enable);
> > };
> > @@ -133,6 +138,18 @@ static void psr_flush_handler(struct work_struct *work)
> > mutex_unlock(&psr->lock);
> > }
> >
> > +static void psr_disable_handler(struct work_struct *work)
> > +{
> > + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work);
> > +
> > + /* If the state has changed since we initiated the flush, do nothing */
> > + mutex_lock(&psr->lock);
> > + if (psr->state == PSR_ENABLE)
> > + psr_set_state_locked(psr, PSR_FLUSH);
> > + mutex_unlock(&psr->lock);
> > + mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
> > +}
> > +
> > /**
> > * rockchip_drm_psr_activate - activate PSR on the given pipe
> > * @encoder: encoder to obtain the PSR encoder
> > @@ -173,6 +190,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
> > psr->active = false;
> > mutex_unlock(&psr->lock);
> > cancel_delayed_work_sync(&psr->flush_work);
> > + cancel_work_sync(&psr->disable_work);
> >
> > return 0;
> > }
> > @@ -226,6 +244,95 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev)
> > }
> > EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
> >
> > +static void psr_input_event(struct input_handle *handle,
> > + unsigned int type, unsigned int code,
> > + int value)
> > +{
> > + struct psr_drv *psr = handle->handler->private;
> > +
> > + schedule_work(&psr->disable_work);
> > +}
> > +
> > +static int psr_input_connect(struct input_handler *handler,
> > + struct input_dev *dev,
> > + const struct input_device_id *id)
> > +{
> > + struct input_handle *handle;
> > + int error;
> > +
> > + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> > + if (!handle)
> > + return -ENOMEM;
> > +
> > + handle->dev = dev;
> > + handle->handler = handler;
> > + handle->name = "rockchip-psr";
> > +
> > + error = input_register_handle(handle);
> > + if (error)
> > + goto err2;
> > +
> > + error = input_open_device(handle);
> > + if (error)
> > + goto err1;
> > +
> > + return 0;
> > +
> > +err1:
> > + input_unregister_handle(handle);
> > +err2:
> > + kfree(handle);
> > + return error;
> > +}
> > +
> > +static void psr_input_disconnect(struct input_handle *handle)
> > +{
> > + input_close_device(handle);
> > + input_unregister_handle(handle);
> > + kfree(handle);
> > +}
> > +
> > +/* Same device ids as cpu-boost */
> > +static const struct input_device_id psr_ids[] = {
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> > + INPUT_DEVICE_ID_MATCH_ABSBIT,
> > + .evbit = { BIT_MASK(EV_ABS) },
> > + .absbit = { [BIT_WORD(ABS_MT_POSITION_X)] =
> > + BIT_MASK(ABS_MT_POSITION_X) |
> > + BIT_MASK(ABS_MT_POSITION_Y) },
> > + }, /* multi-touch touchscreen */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > + .evbit = { BIT_MASK(EV_ABS) },
> > + .absbit = { [BIT_WORD(ABS_X)] = BIT_MASK(ABS_X) }
> > +
> > + }, /* stylus or joystick device */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = { [BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) },
> > + }, /* pointer (e.g. trackpad, mouse) */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = { [BIT_WORD(KEY_ESC)] = BIT_MASK(KEY_ESC) },
> > + }, /* keyboard */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> > + INPUT_DEVICE_ID_MATCH_KEYBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = {[BIT_WORD(BTN_JOYSTICK)] = BIT_MASK(BTN_JOYSTICK) },
> > + }, /* joysticks not caught by ABS_X above */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> > + INPUT_DEVICE_ID_MATCH_KEYBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = { [BIT_WORD(BTN_GAMEPAD)] = BIT_MASK(BTN_GAMEPAD) },
> > + }, /* gamepad */
> > + { },
> > +};
> > +
> > /**
> > * rockchip_drm_psr_register - register encoder to psr driver
> > * @encoder: encoder that obtain the PSR function
> > @@ -239,6 +346,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> > {
> > struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
> > struct psr_drv *psr;
> > + int error;
> >
> > if (!encoder || !psr_set)
> > return -EINVAL;
> > @@ -248,6 +356,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> > return -ENOMEM;
> >
> > INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler);
> > + INIT_WORK(&psr->disable_work, psr_disable_handler);
> > mutex_init(&psr->lock);
> >
> > psr->active = true;
> > @@ -255,11 +364,33 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> > psr->encoder = encoder;
> > psr->set = psr_set;
> >
> > + psr->input_handler.event = psr_input_event;
> > + psr->input_handler.connect = psr_input_connect;
> > + psr->input_handler.disconnect = psr_input_disconnect;
> > + psr->input_handler.name =
> > + kasprintf(GFP_KERNEL, "rockchip-psr-%s", encoder->name);
> > + if (!psr->input_handler.name) {
> > + error = -ENOMEM;
> > + goto err2;
> > + }
> > + psr->input_handler.id_table = psr_ids;
> > + psr->input_handler.private = psr;
> > +
> > + error = input_register_handler(&psr->input_handler);
> > + if (error)
> > + goto err1;
> > +
> > mutex_lock(&drm_drv->psr_list_lock);
> > list_add_tail(&psr->list, &drm_drv->psr_list);
> > mutex_unlock(&drm_drv->psr_list_lock);
> >
> > return 0;
> > +
> > + err1:
> > + kfree(psr->input_handler.name);
> > + err2:
> > + kfree(psr);
> > + return error;
> > }
> > EXPORT_SYMBOL(rockchip_drm_psr_register);
> >
> > @@ -279,8 +410,11 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder)
> > mutex_lock(&drm_drv->psr_list_lock);
> > list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) {
> > if (psr->encoder == encoder) {
> > + input_unregister_handler(&psr->input_handler);
> > cancel_delayed_work_sync(&psr->flush_work);
> > + cancel_work_sync(&psr->disable_work);
> > list_del(&psr->list);
> > + kfree(psr->input_handler.name);
> > kfree(psr);
> > }
> > }
>
>
--
Dmitry
^ 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