* Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
From: Hans de Goede @ 2023-09-20 9:32 UTC (permalink / raw)
To: Julius Zint
Cc: Thomas Weißschuh, Lee Jones, Daniel Thompson, Jingoo Han,
Jiri Kosina, Benjamin Tissoires, Helge Deller, linux-kernel,
dri-devel, linux-input, linux-fbdev
In-Reply-To: <afed6395-8680-7e2c-d88c-8bb5f3c39346@zint.sh>
Hi,
On 9/19/23 19:46, Julius Zint wrote:
>
>
> On Wed, 6 Sep 2023, Hans de Goede wrote:
>
>> Hi Julius,
>>
>> On 9/4/23 21:02, Julius Zint wrote:
>>>
>>>
>>> On Mon, 4 Sep 2023, Thomas Weißschuh wrote:
>>>
>>>> +Cc Hans who ins involved with the backlight subsystem
>>>>
>>>> Hi Julius,
>>>>
>>>> today I stumbled upon a mail from Hans [0], which explains that the
>>>> backlight subsystem is not actually a good fit (yet?) for external
>>>> displays.
>>>>
>>>> It seems a new API is in the works that would better fit, but I'm not
>>>> sure about the state of this API. Maybe Hans can clarify.
>>>>
>>>> This also ties back to my review question how userspace can figure out
>>>> to which display a backlight devices applies. So far it can not.
>>>>
>>>> [0] https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/
>>>>
>>>
>>> Hi Thomas,
>>>
>>> thanks for the hint. I will make sure to give this a proper read and
>>> see, if it fits my use case better then the current backlight subsystem.
>>
>> Note the actual proposal for the new usespace API for display brightness
>> control is here:
>>
>> https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com/
>>
>> I have finished / stabilized the backlight code refactor which I landed
>> in 6.1, which is a prerequisite for the above proposal. But I have not
>> been able to make time to actually implement the above proposal; and
>> I don't know when I will be able to make time for this.
>>
>>> Especially since I wasnt able to properly address your other review
>>> comments for now. You are right that the name should align better with
>>> the kernel module and also, that it is possible for multiple displays to
>>> be attached.
>>>
>>> In its current state, this would mean that you could only control the
>>> backlight for the first HID device (enough for me :-).
>>>
>>> The systemd-backlight@.service uses not only the file name, but also the
>>> full bus path for storing/restoring backlights. I did not yet get around
>>> to see how the desktops handle brightness control, but since the
>>> systemd-backlight@.service already uses the name, its important to stay
>>> the same over multiple boots.
>>>
>>> I would be able to get a handle on the underlying USB device and use the
>>> serial to uniquely (and persistently) name the backlight. But it does
>>> feel hacky doing it this way.
>>
>> So mutter (gnome-shell compositor library) has a similar issue when saving
>> monitor layouts and I can tell you beforehand that monitor serial numbers
>> by themselves are not unique enough. Some models just report 123456789
>> as serial and if you have a dual-monitor setup with 2 such monitors
>> and name the backlight class device <serial>-vcp-hid or something like that
>> you will still end up with 2 identical names.
>>
>> To avoid this when saving monitor layouts mutter saves both the port
>> to which the monitor is attached (e.g. DP-1 DP-2) and the serialnumber
>> and on startup / monitor hotplug when it checks to see if it has saved
>> layout info for the monitor it checks the port+serialnr combination.
>>
>> So what I think you should do is figure out a way to map which
>> VCP HID device maps to which drm-connector and then use
>> the connector-name + serial-nr to generate the backlight device name.
>>
>> We will need the mapping the a drm-connector object anyway for
>> the new brightness API proposal from above.
>>
>> Note this does NOT solve the fact that registering a new backlight
>> class device for an external monitor on a laptop will hopelessly
>> confuse userspace, see:
>>
>> https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/
>>
>> Regards,
>>
>> Hans
>>
>
> Thank you for all this additional information. I have watched the talks
> and read up upon the mail threads you`ve linked.
Now I wonder which presentation you have watched, did you watch
the old XDC2014 presentation ? Note I gave a much more up2date
presentation on this at kernel-recipes last year:
https://kernel-recipes.org/en/2022/talks/new-userspace-api-for-display-panel-brightness-control/
> I will see if I can make the mapping to the DRM connector and plan to
> update this patchset.
Sounds good.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 0/2] Input: ilitek_ts_i2c - Fix spurious input events
From: Emanuele Ghidoli @ 2023-09-20 7:54 UTC (permalink / raw)
To: Emanuele Ghidoli, Dmitry Torokhov; +Cc: linux-kernel, linux-input, Joe Hung
In-Reply-To: <20230920074650.922292-1-ghidoliemanuele@gmail.com>
On 20/09/2023 09:46, Emanuele Ghidoli wrote:
> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
>
> A couple of fixes to prevent spurious events when the data buffer is not the expected one.
>
> Emanuele Ghidoli (2):
> Input: ilitek_ts_i2c - avoid wrong input subsystem sync
> Input: ilitek_ts_i2c - add report id message validation
I did a small mistake, V1 never existed, this series will start from V2, apologize about that ...
Emanuele
^ permalink raw reply
* [PATCH v2 2/2] Input: ilitek_ts_i2c - add report id message validation
From: Emanuele Ghidoli @ 2023-09-20 7:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Emanuele Ghidoli, linux-kernel, linux-input, Joe Hung
In-Reply-To: <20230920074650.922292-1-ghidoliemanuele@gmail.com>
From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Ilitek touch IC driver answer to plain i2c read request, after it has
generated an interrupt request, with a report id message starting
with an identifier and a series of points.
If a request is sent unsolicited by an interrupt request the answer
do not contain this identifier.
Add a test to discard these messages, making the driver robust to
spurious interrupt requests.
Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 0c3491e346f4..efce545236cd 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -37,6 +37,8 @@
#define ILITEK_TP_CMD_GET_MCU_VER 0x61
#define ILITEK_TP_CMD_GET_IC_MODE 0xC0
+#define ILITEK_TP_I2C_REPORT_ID 0x48
+
#define REPORT_COUNT_ADDRESS 61
#define ILITEK_SUPPORT_MAX_POINT 40
@@ -163,6 +165,11 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
goto err_sync_frame;
}
+ if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
+ dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
+ goto err_sync_frame;
+ }
+
report_max_point = buf[REPORT_COUNT_ADDRESS];
if (report_max_point > ts->max_tp) {
dev_err(dev, "FW report max point:%d > panel info. max:%d\n",
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] Input: ilitek_ts_i2c - avoid wrong input subsystem sync
From: Emanuele Ghidoli @ 2023-09-20 7:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Emanuele Ghidoli, linux-kernel, linux-input, Joe Hung
In-Reply-To: <20230920074650.922292-1-ghidoliemanuele@gmail.com>
From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
For different reasons i2c transaction may fail or
report id message content may be wrong.
Avoid sync the input subsystem if message cannot be parsed.
An input subsystem sync without points is interpreted as
"nothing is touching the screen" while normally this is not the case.
Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 90c4934e750a..0c3491e346f4 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -203,9 +203,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
ilitek_touch_down(ts, id, x, y);
}
-err_sync_frame:
input_mt_sync_frame(input);
input_sync(input);
+err_sync_frame:
return error;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/2] Input: ilitek_ts_i2c - Fix spurious input events
From: Emanuele Ghidoli @ 2023-09-20 7:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Emanuele Ghidoli, linux-kernel, linux-input, Joe Hung
From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
A couple of fixes to prevent spurious events when the data buffer is not the expected one.
Emanuele Ghidoli (2):
Input: ilitek_ts_i2c - avoid wrong input subsystem sync
Input: ilitek_ts_i2c - add report id message validation
drivers/input/touchscreen/ilitek_ts_i2c.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
--
2.34.1
^ permalink raw reply
* Poptávka
From: Lukas Varga @ 2023-09-20 7:31 UTC (permalink / raw)
To: linux-input
Dobré ráno,
Dovolil jsem si Vás kontaktovat, protože mám zájem ověřit možnost navázání spolupráce.
Podporujeme firmy při získávání nových obchodních zákazníků.
Můžeme si promluvit a poskytnout podrobnosti?
V případě zájmu Vás bude kontaktovat náš anglicky mluvící zástupce.
Pozdravy
Lukas Varga
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: fix handling of unpopulated devices
From: Johan Hovold @ 2023-09-20 7:26 UTC (permalink / raw)
To: Doug Anderson
Cc: Johan Hovold, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, Maxime Ripard, Dmitry Torokhov, LinusW, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:GPIO SUBSYSTEM
In-Reply-To: <CAD=FV=XBG7auVVyHn5uvahSZZxp5qBfp4+A9NwFqahdN6XrbZA@mail.gmail.com>
On Tue, Sep 19, 2023 at 11:15:46AM -0700, Doug Anderson wrote:
> On Tue, Sep 19, 2023 at 12:07 AM Johan Hovold <johan@kernel.org> wrote:
> > But regardless of what a long-term proper solution to this may look
> > like, we need to fix the regression in 6.6-rc1 by restoring the old
> > behaviour.
>
> OK, fair enough. I'll take a look at your patch, though I think the
> person that really needs to approve it is Benjamin...
>
> Style-wise, I will say that Benjamin really wanted to keep the "panel
> follower" code out of the main probe routine. Some of my initial
> patches adding "panel follower" looked more like the results after
> your patch but Benjamin really wasn't happy until there were no
> special cases for panel-followers in the main probe routine. This is
> why the code is structured as it is.
Ok, I prefer not hiding away things like that as it obscures what's
really going on, for example, in this case, that you register a device
without really having probed it.
As I alluded to in the commit message, you probably want to be able to
support second-source touchscreen panel followers as well at some point
and then deferring checking whether device is populated until the panel
is powered on is not going to work.
I skimmed the thread were you added this, but I'm not sure I saw any
reason for why powering on the panel follower temporarily during probe
would not work?
> Thinking that way, is there any reason you can't just move the
> i2c_hid_init_irq() into __do_i2c_hid_core_initial_power_up()? You
> could replace the call to enable_irq() with it and then remove the
> `IRQF_NO_AUTOEN` flag? I think that would also solve the issue if you
> wanted to use a 2nd source + the panel follower concept? Both devices
> would probe, but only one of them would actually grab the interrupt
> and only one of them would actually create real HID devices. We might
> need to do some work to keep from trying again at every poweron of the
> panel, but it would probably be workable? I think this would also be a
> smaller change...
That was my first idea as well, but conceptually it is more correct to
request resources at probe time and not at some later point when you can
no longer fail probe.
You'd also need to handle the fact that the interrupt may never have
been requested when remove() is called, which adds unnecessary
complexity.
Johan
^ permalink raw reply
* Re: (subset) [PATCH v4 00/11] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
From: Bjorn Andersson @ 2023-09-19 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Konrad Dybcio, Rob Herring,
Frank Rowand, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Douglas Anderson
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel
In-Reply-To: <20230727171750.633410-1-dianders@chromium.org>
On Thu, 27 Jul 2023 10:16:27 -0700, Douglas Anderson wrote:
> The big motivation for this patch series is mostly described in the patch
> ("drm/panel: Add a way for other devices to follow panel state"), but to
> quickly summarize here: for touchscreens that are connected to a panel we
> need the ability to power sequence the two device together. This is not a
> new need, but so far we've managed to get by through a combination of
> inefficiency, added costs, or perhaps just a little bit of brokenness.
> It's time to do better. This patch series allows us to do better.
>
> [...]
Applied, thanks!
[11/11] arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
commit: 989aac9dea7fcfc33b5eedc4ae44abbf71460a4d
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* [PATCH] hid: cp2112: Fix duplicate workqueue initialization
From: Danny Kaehn @ 2023-09-19 21:22 UTC (permalink / raw)
To: jikos, benjamin.tissoires, andriy.shevchenko
Cc: linux-input, ethan.twardy, Danny Kaehn
Previously the cp2112 driver called INIT_DELAYED_WORK within
cp2112_gpio_irq_startup, resulting in duplicate initilizations of the
workqueue on subsequent IRQ startups following an initial request. This
resulted in a warning in set_work_data in workqueue.c, as well as a rare
NULL dereference within process_one_work in workqueue.c.
Initialize the workqueue within _probe instead.
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
Note -- the warning & NULL dereference that were caused by this were
completely decoupled from the driver code, making this a fairly tricky
bug to track down. I wonder if there would be a way to add a WARN into
__INIT_WORK if an already initialized workqueue is re-initialized
without a lot of overhead...
drivers/hid/hid-cp2112.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 54c33a24f844..36f76c6dfa20 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1151,8 +1151,6 @@ static unsigned int cp2112_gpio_irq_startup(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct cp2112_device *dev = gpiochip_get_data(gc);
- INIT_DELAYED_WORK(&dev->gpio_poll_worker, cp2112_gpio_poll_callback);
-
if (!dev->gpio_poll) {
dev->gpio_poll = true;
schedule_delayed_work(&dev->gpio_poll_worker, 0);
@@ -1307,6 +1305,8 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
girq->handler = handle_simple_irq;
girq->threaded = true;
+ INIT_DELAYED_WORK(&dev->gpio_poll_worker, cp2112_gpio_poll_callback);
+
ret = gpiochip_add_data(&dev->gc, dev);
if (ret < 0) {
hid_err(hdev, "error registering gpio chip\n");
--
2.25.1
^ permalink raw reply related
* [PATCH] hid: cp2112: Fix IRQ shutdown stopping polling for all IRQs on chip
From: Danny Kaehn @ 2023-09-19 21:24 UTC (permalink / raw)
To: jikos, benjamin.tissoires, andriy.shevchenko
Cc: linux-input, ethan.twardy, Danny Kaehn
Previously cp2112_gpio_irq_shutdown always cancelled the
gpio_poll_worker, even if other IRQs were still active, and did not set
the gpio_poll flag to false. This resulted in any call to _shutdown()
resulting in interrupts no longer functioning on the chip until a
_remove occurred (a.e. the cp2112 is unplugged or system rebooted).
Only cancel polling if all IRQs are disabled/masked, and correctly set
the gpio_poll flag, allowing polling to restart when an interrupt is
next enabled.
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
drivers/hid/hid-cp2112.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 36f76c6dfa20..d589214834b9 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1166,7 +1166,12 @@ static void cp2112_gpio_irq_shutdown(struct irq_data *d)
struct cp2112_device *dev = gpiochip_get_data(gc);
cp2112_gpio_irq_mask(d);
- cancel_delayed_work_sync(&dev->gpio_poll_worker);
+
+ if (!dev->irq_mask)
+ {
+ dev->gpio_poll = false;
+ cancel_delayed_work_sync(&dev->gpio_poll_worker);
+ }
}
static int cp2112_gpio_irq_type(struct irq_data *d, unsigned int type)
--
2.25.1
^ permalink raw reply related
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Rob Herring @ 2023-09-19 18:17 UTC (permalink / raw)
To: Tylor Yang
Cc: dmitry.torokhov, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel,
poyuan_chang, jingliang, hbarnor
In-Reply-To: <20230919024943.3088916-2-tylor_yang@himax.corp-partner.google.com>
On Tue, Sep 19, 2023 at 10:49:42AM +0800, Tylor Yang wrote:
> The Himax HID-over-SPI framework support for Himax touchscreen ICs
> that report HID packet through SPI bus. The driver core need reset
> pin to meet reset timing spec. of IC. An interrupt pin to disable
> and enable interrupt when suspend/resume. An optional power control
> pin if target board needed. Panel id pins for identify panel is also
> an option.
>
> Additional optional arguments:
> ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime
> conditions.
>
> This patch also add maintainer of this driver.
>
> Signed-off-by: Tylor Yang <tylor_yang@himax.corp-partner.google.com>
> ---
> .../bindings/input/himax,hid-over-spi.yaml | 109 ++++++++++++++++++
> MAINTAINERS | 6 +
> 2 files changed, 115 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
> new file mode 100644
> index 000000000000..3ee3a89842ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/himax,hid-over-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Himax TDDI devices using SPI to send HID packets
> +
> +maintainers:
> + - Tylor Yang <tylor_yang@himax.corp-partner.google.com>
> +
> +description: |
> + Support the Himax TDDI devices which using SPI interface to acquire
> + HID packets from the device. The device needs to be initialized using
> + Himax protocol before it start sending HID packets.
> +
> +properties:
> + compatible:
> + const: himax,hid-over-spi
Doesn't look like a specific device. Compatibles are generally based on
part numbers.
'over-spi' is redundant as the parent would be a spi controller.
> +
> + reg:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
These are for child nodes, but you don't have any.
> +
> + interrupts:
> + maxItems: 1
> +
> + himax,rst-gpio:
> + maxItems: 1
> + description: Reset device, active low signal.
Use standard reset-gpios.
(-gpio is deprecated)
> +
> + himax,irq-gpio:
> + maxItems: 1
> + description: Interrupt request, active low signal.
You have the interrupt already, why do you need this?
> +
> + himax,3v3-gpio:
> + maxItems: 1
> + description: GPIO to control 3.3V power supply.
This should be a regulator supply. Then use gpio-regulator if it happens
to be GPIO controlled.
> +
> + himax,id-gpios:
> + maxItems: 8
> + description: GPIOs to read physical Panel ID. Optional.
> +
> + spi-cpha: true
> + spi-cpol: true
> +
> + himax,ic-det-delay-ms:
> + description:
> + Due to TDDI properties, the TPIC detection timing must after the
> + display panel initialized. This property is used to specify the
> + delay time when TPIC detection and display panel initialization
> + timing are overlapped. How much milliseconds to delay before TPIC
> + detection start.
> +
> + himax,ic-resume-delay-ms:
> + description:
> + Due to TDDI properties, the TPIC resume timing must after the
> + display panel resumed. This property is used to specify the
> + delay time when TPIC resume and display panel resume
> + timing are overlapped. How much milliseconds to delay before TPIC
> + resume start.
These should be implied by the compatible. Unless they are board
specific and not device specific.
Rob
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: fix handling of unpopulated devices
From: Doug Anderson @ 2023-09-19 18:15 UTC (permalink / raw)
To: Johan Hovold
Cc: Johan Hovold, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, Maxime Ripard, Dmitry Torokhov, LinusW, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:GPIO SUBSYSTEM
In-Reply-To: <ZQlIveJVdvyV2Ygy@hovoldconsulting.com>
Hi,
On Tue, Sep 19, 2023 at 12:07 AM Johan Hovold <johan@kernel.org> wrote:
>
> > c) To solve this in the core, we have to make sure we properly handle
> > (without hanging/failing) multiple partially-conflicting devices and
> > devices that might acquire resources in arbitrary orders.
> >
> > Though the above solution detecting the pinctrl conflicts sounds
> > appealing and I'm currently working on prototyping it, I'm still not
> > 100% convinced. I'm worried about the above downsides.
>
> Yes, I agree that we'd need to take a broader look at this and not just
> focus on the immediate pinctrl issue.
OK. FWIW, I got blocked on trying to solve this in the core
automatically by just using the conflicting "pinctrl" entries. There
are probably some ways to get it solved, but none of them are easy.
> > Personally, I feel like we could add information to the device tree
> > that would help us out. The question is: is this an abuse of device
> > tree for something that Linux ought to be able to figure out on its
> > own, or is it OK? To make it concrete, I was thinking about something
> > like this:
> >
> > / {
> > tp_ex_group: trackpad-exclusion-group {
> > members = <&tp1>, <&tp2>, <&tp3>;
> > };
> > };
> >
> > &i2c_bus {
> > tp1: trackpad@10 {
> > ...
> > mutual-exclusion-group = <&tp_ex_group>;
> > };
> > tp2: trackpad@20 {
> > ...
> > mutual-exclusion-group = <&tp_ex_group>;
> > };
> > tp3: trackpad@30 {
> > ...
> > mutual-exclusion-group = <&tp_ex_group>;
> > };
> > };
> >
> > Then the device core would know not to probe devices in the same
> > "mutual-exclusion-group" at the same time.
> >
> > If DT folks are OK with the "mutual-exclusion-group" idea then I'll
> > probably backburner my attempt to make this work on the pinctrl level
> > and go with that.
>
> I expressed something along these lines in the thread above:
I'm going to try coding up the above to see how it looks. Assuming
nothing comes up, I'll try to have something in the next few days.
> It seems we'd need some way to describe the devices as mutually
> exclusive...
>
> but given that we had prior art for handling simple cases and due to
> lack of time, I left it on the ever-growing todo list.
>
> But regardless of what a long-term proper solution to this may look
> like, we need to fix the regression in 6.6-rc1 by restoring the old
> behaviour.
OK, fair enough. I'll take a look at your patch, though I think the
person that really needs to approve it is Benjamin...
Style-wise, I will say that Benjamin really wanted to keep the "panel
follower" code out of the main probe routine. Some of my initial
patches adding "panel follower" looked more like the results after
your patch but Benjamin really wasn't happy until there were no
special cases for panel-followers in the main probe routine. This is
why the code is structured as it is.
Thinking that way, is there any reason you can't just move the
i2c_hid_init_irq() into __do_i2c_hid_core_initial_power_up()? You
could replace the call to enable_irq() with it and then remove the
`IRQF_NO_AUTOEN` flag? I think that would also solve the issue if you
wanted to use a 2nd source + the panel follower concept? Both devices
would probe, but only one of them would actually grab the interrupt
and only one of them would actually create real HID devices. We might
need to do some work to keep from trying again at every poweron of the
panel, but it would probably be workable? I think this would also be a
smaller change...
-Doug
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Rafael J. Wysocki @ 2023-09-19 17:59 UTC (permalink / raw)
To: srinivas pandruvada, Kai-Heng Feng
Cc: Xu, Even, jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-pm@vger.kernel.org, linux-pci@vger.kernel.org,
Lee, Jian Hui, Zhang, Lixu, Ba, Najumon,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <67c85f083201ed2cda2cab198b40141ad21912a2.camel@linux.intel.com>
On Tue, Sep 19, 2023 at 6:54 PM srinivas pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Tue, 2023-09-19 at 15:36 +0800, Kai-Heng Feng wrote:
> > On Mon, Sep 18, 2023 at 11:57 PM srinivas pandruvada
> > <srinivas.pandruvada@linux.intel.com> wrote:
> > >
> > > Hi Kai-Heng,
> > > On Mon, 2023-09-18 at 09:17 +0800, Kai-Heng Feng wrote:
> > > > Hi Even,
> > > >
> > > > On Mon, Sep 18, 2023 at 8:33 AM Xu, Even <even.xu@intel.com>
> > > > wrote:
> > > > >
> > > > > Hi, Kai-Heng,
> > > > >
> > > > > I just got feedback, for testing EHL S5 wakeup feature, you
> > > > > need
> > > > > several steps to setup and access
> > > > > "https://portal.devicewise.com/things/browse" to trigger wake.
> > > > > But currently, our test account of this website are all out of
> > > > > data.
> > > > > So maybe you need double check with the team who required you
> > > > > preparing the patch for the verification.
> > > >
> > > > The patch is to solve the GPE refcount overflow, while
> > > > maintaining S5
> > > > wakeup. I don't have any mean to test S5 wake.
> > > >
> > > The issue is not calling acpi_disable_gpe(). To reduce the scope of
> > > change can we just add that instead of a adding new callbacks. This
> > > way
> > > scope is reduced.
> >
> > This patch does exactly the same thing by letting PCI and ACPI handle
> > the PME and GPE.
> > Though the change seems to be bigger, it actually reduces the duped
> > code, while keep the S5 wakeup ability intact.
> It may be doing the same. But with long chain of calls without
> verification, I am not comfortable.
> This can be another patch by itself to use the framework.
I agree.
Let's change one thing at a time.
> But you are targeting a fix for overflow issue, which is separate from
> the use of PCI/ACPI framework.
Yes, let's fix the bug first and make things look nicer separately.
^ permalink raw reply
* [PATCH 00/49] iio: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:48 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel, Linus Walleij, linux-arm-kernel, Eugen Hristev,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Jinjie Ruan,
Rob Herring, Heiko Stuebner, Yang Yingliang, Chen-Yu Tsai,
Aidan MacDonald, Andy Shevchenko, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Hartley Sweeten,
Alexander Sverdlin, Krzysztof Kozlowski, Alim Akhtar,
linux-samsung-soc, Shawn Guo, Sascha Hauer, Fabio Estevam,
NXP Linux Team, Andreas Klinger, Cai Huoqing, Haibo Chen,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
George Stark, Andy Shevchenko, Nuno Sá, linux-amlogic,
Saravanan Sekar, Jiakai Luo, Dongliang Mu, Avi Fishman,
Tomer Maimon, Tali Perry, Patrick Venture, Nancy Yuen,
Benjamin Fair, openbmc, Andy Gross, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, Marek Vasut, Maxime Coquelin,
Alexandre Torgue, Olivier Moysan, Fabrice Gasnier, Zhang Shurong,
Yangtao Li, linux-stm32, Sean Nyekjaer, Tom Rix, Jernej Skrabec,
Samuel Holland, Rafael J. Wysocki, Damien Le Moal, Mark Brown,
Ido Schimmel, Daniel Lezcano, linux-sunxi, Dmitry Torokhov,
Andreas Kemnade, Peter Rosin, Vladimir Zapolskiy, Kevin Tsai,
Benson Leung, Guenter Roeck, chrome-platform
this series converts all platform drivers below drivers/iio to use
.remove_new(). The motivation is to get rid of an integer return code
that is (mostly) ignored by the platform driver core and error prone on
the driver side. As all platform drivers return zero unconditionally in their
remove callback up to now, the conversions are "trivial".
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
There are no interdependencies between the patches. As there are still
quite a few drivers to convert, I'm happy about every patch that makes
it in. So even if there is a merge conflict with one patch until you
apply or I picked a wrong subject prefix, please apply the remainder of
this series anyhow.
Best regards
Uwe
Uwe Kleine-König (49):
iio: accel: hid-sensor-accel-3d: Convert to platform remove callback
returning void
iio: adc: ab8500-gpadc: Convert to platform remove callback returning
void
iio: adc: at91-sama5d2: Convert to platform remove callback returning
void
iio: adc: at91: Convert to platform remove callback returning void
iio: adc: axp20x: Convert to platform remove callback returning void
iio: adc: bcm_iproc: Convert to platform remove callback returning
void
iio: adc: dln2: Convert to platform remove callback returning void
iio: adc: ep93xx: Convert to platform remove callback returning void
iio: adc: exynos: Convert to platform remove callback returning void
iio: adc: fsl-imx25-gcq: Convert to platform remove callback returning
void
iio: adc: hx711: Convert to platform remove callback returning void
iio: adc: imx8qxp: Convert to platform remove callback returning void
iio: adc: imx93: Convert to platform remove callback returning void
iio: adc: meson_saradc: Convert to platform remove callback returning
void
iio: adc: mp2629: Convert to platform remove callback returning void
iio: adc: mxs-lradc: Convert to platform remove callback returning
void
iio: adc: npcm: Convert to platform remove callback returning void
iio: adc: qcom-pm8xxx-xoadc: Convert to platform remove callback
returning void
iio: adc: rcar-gyroadc: Convert to platform remove callback returning
void
iio: adc: stm32-adc-core: Convert to platform remove callback
returning void
iio: adc: stm32-adc: Convert to platform remove callback returning
void
iio: adc: stm32-dfsdm-adc: Convert to platform remove callback
returning void
iio: adc: stm32-dfsdm-core: Convert to platform remove callback
returning void
iio: adc: sun4i-gpadc-iio: Convert to platform remove callback
returning void
iio: adc: ti_am335x_adc: Convert to platform remove callback returning
void
iio: adc: twl4030-madc: Convert to platform remove callback returning
void
iio: adc: twl6030-gpadc: Convert to platform remove callback returning
void
iio: adc: vf610_adc: Convert to platform remove callback returning
void
iio: dac: dpot-dac: Convert to platform remove callback returning void
iio: dac: lpc18xx_dac: Convert to platform remove callback returning
void
iio: dac: stm32-dac-core: Convert to platform remove callback
returning void
iio: dac: stm32-dac: Convert to platform remove callback returning
void
iio: dac: vf610: Convert to platform remove callback returning void
iio: gyro: hid-sensor-gyro-3d: Convert to platform remove callback
returning void
iio: humidity: hid-sensor-humidity: Convert to platform remove
callback returning void
iio: light: cm3605: Convert to platform remove callback returning void
iio: light: hid-sensor-als: Convert to platform remove callback
returning void
iio: light: hid-sensor-prox: Convert to platform remove callback
returning void
iio: light: lm3533-als: Convert to platform remove callback returning
void
iio: magnetometer: hid-sensor-magn-3d: Convert to platform remove
callback returning void
iio: orientation: hid-sensor-incl-3d: Convert to platform remove
callback returning void
iio: orientation: hid-sensor-rotation: Convert to platform remove
callback returning void
iio: position: hid-sensor-custom-intel-hinge: Convert to platform
remove callback returning void
iio: pressure: hid-sensor: Convert to platform remove callback
returning void
iio: proximity: cros_ec_mkbp: Convert to platform remove callback
returning void
iio: proximity: srf04: Convert to platform remove callback returning
void
iio: temperature: hid-sensor: Convert to platform remove callback
returning void
iio: trigger: iio-trig-interrupt: Convert to platform remove callback
returning void
iio: trigger: stm32-timer: Convert to platform remove callback
returning void
drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
drivers/iio/adc/ab8500-gpadc.c | 6 ++----
drivers/iio/adc/at91-sama5d2_adc.c | 6 ++----
drivers/iio/adc/at91_adc.c | 6 ++----
drivers/iio/adc/axp20x_adc.c | 6 ++----
drivers/iio/adc/bcm_iproc_adc.c | 6 ++----
drivers/iio/adc/dln2-adc.c | 5 ++---
drivers/iio/adc/ep93xx_adc.c | 6 ++----
drivers/iio/adc/exynos_adc.c | 6 ++----
drivers/iio/adc/fsl-imx25-gcq.c | 6 ++----
drivers/iio/adc/hx711.c | 6 ++----
drivers/iio/adc/imx8qxp-adc.c | 6 ++----
drivers/iio/adc/imx93_adc.c | 6 ++----
drivers/iio/adc/meson_saradc.c | 6 ++----
drivers/iio/adc/mp2629_adc.c | 6 ++----
drivers/iio/adc/mxs-lradc-adc.c | 6 ++----
drivers/iio/adc/npcm_adc.c | 6 ++----
drivers/iio/adc/qcom-pm8xxx-xoadc.c | 6 ++----
drivers/iio/adc/rcar-gyroadc.c | 6 ++----
drivers/iio/adc/stm32-adc-core.c | 6 ++----
drivers/iio/adc/stm32-adc.c | 6 ++----
drivers/iio/adc/stm32-dfsdm-adc.c | 6 ++----
drivers/iio/adc/stm32-dfsdm-core.c | 6 ++----
drivers/iio/adc/sun4i-gpadc-iio.c | 8 +++-----
drivers/iio/adc/ti_am335x_adc.c | 6 ++----
drivers/iio/adc/twl4030-madc.c | 6 ++----
drivers/iio/adc/twl6030-gpadc.c | 6 ++----
drivers/iio/adc/vf610_adc.c | 6 ++----
drivers/iio/dac/dpot-dac.c | 6 ++----
drivers/iio/dac/lpc18xx_dac.c | 6 ++----
drivers/iio/dac/stm32-dac-core.c | 6 ++----
drivers/iio/dac/stm32-dac.c | 6 ++----
drivers/iio/dac/vf610_dac.c | 6 ++----
drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 ++----
drivers/iio/humidity/hid-sensor-humidity.c | 6 ++----
drivers/iio/light/cm3605.c | 6 ++----
drivers/iio/light/hid-sensor-als.c | 6 ++----
drivers/iio/light/hid-sensor-prox.c | 6 ++----
drivers/iio/light/lm3533-als.c | 6 ++----
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 6 ++----
drivers/iio/orientation/hid-sensor-incl-3d.c | 6 ++----
drivers/iio/orientation/hid-sensor-rotation.c | 6 ++----
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 6 ++----
drivers/iio/pressure/hid-sensor-press.c | 6 ++----
drivers/iio/proximity/cros_ec_mkbp_proximity.c | 6 ++----
drivers/iio/proximity/srf04.c | 6 ++----
drivers/iio/temperature/hid-sensor-temperature.c | 6 ++----
drivers/iio/trigger/iio-trig-interrupt.c | 6 ++----
drivers/iio/trigger/stm32-timer-trigger.c | 6 ++----
49 files changed, 99 insertions(+), 196 deletions(-)
base-commit: 29e400e3ea486bf942b214769fc9778098114113
--
2.40.1
^ permalink raw reply
* [PATCH 38/49] iio: light: hid-sensor-prox: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/light/hid-sensor-prox.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index a47591e1bad9..26c481d2998c 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -313,7 +313,7 @@ static int hid_prox_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_prox_remove(struct platform_device *pdev)
+static void hid_prox_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -322,8 +322,6 @@ static int hid_prox_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, hsdev->usage);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &prox_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_prox_ids[] = {
@@ -346,7 +344,7 @@ static struct platform_driver hid_prox_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_prox_probe,
- .remove = hid_prox_remove,
+ .remove_new = hid_prox_remove,
};
module_platform_driver(hid_prox_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 44/49] iio: pressure: hid-sensor: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/pressure/hid-sensor-press.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index a9215eb32d70..956045e2db29 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -323,7 +323,7 @@ static int hid_press_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_press_remove(struct platform_device *pdev)
+static void hid_press_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -332,8 +332,6 @@ static int hid_press_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &press_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_press_ids[] = {
@@ -352,7 +350,7 @@ static struct platform_driver hid_press_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_press_probe,
- .remove = hid_press_remove,
+ .remove_new = hid_press_remove,
};
module_platform_driver(hid_press_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 43/49] iio: position: hid-sensor-custom-intel-hinge: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
index 07c30d217255..76e173850a35 100644
--- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
+++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
@@ -342,7 +342,7 @@ static int hid_hinge_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_hinge_remove(struct platform_device *pdev)
+static void hid_hinge_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -351,8 +351,6 @@ static int hid_hinge_remove(struct platform_device *pdev)
iio_device_unregister(indio_dev);
sensor_hub_remove_callback(hsdev, hsdev->usage);
hid_sensor_remove_trigger(indio_dev, &st->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_hinge_ids[] = {
@@ -371,7 +369,7 @@ static struct platform_driver hid_hinge_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_hinge_probe,
- .remove = hid_hinge_remove,
+ .remove_new = hid_hinge_remove,
};
module_platform_driver(hid_hinge_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 40/49] iio: magnetometer: hid-sensor-magn-3d: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index e85a3a8eea90..5c795a430d09 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -547,7 +547,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_magn_3d_remove(struct platform_device *pdev)
+static void hid_magn_3d_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -556,8 +556,6 @@ static int hid_magn_3d_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &magn_state->magn_flux_attributes);
-
- return 0;
}
static const struct platform_device_id hid_magn_3d_ids[] = {
@@ -576,7 +574,7 @@ static struct platform_driver hid_magn_3d_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_magn_3d_probe,
- .remove = hid_magn_3d_remove,
+ .remove_new = hid_magn_3d_remove,
};
module_platform_driver(hid_magn_3d_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 42/49] iio: orientation: hid-sensor-rotation: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/orientation/hid-sensor-rotation.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index a033699910e8..5e8cadd5177a 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -327,7 +327,7 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_dev_rot_remove(struct platform_device *pdev)
+static void hid_dev_rot_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -336,8 +336,6 @@ static int hid_dev_rot_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, hsdev->usage);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &rot_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_dev_rot_ids[] = {
@@ -364,7 +362,7 @@ static struct platform_driver hid_dev_rot_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_dev_rot_probe,
- .remove = hid_dev_rot_remove,
+ .remove_new = hid_dev_rot_remove,
};
module_platform_driver(hid_dev_rot_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 41/49] iio: orientation: hid-sensor-incl-3d: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/orientation/hid-sensor-incl-3d.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index ba5b581d5b25..8943d5c78bc0 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -383,7 +383,7 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_incl_3d_remove(struct platform_device *pdev)
+static void hid_incl_3d_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -392,8 +392,6 @@ static int hid_incl_3d_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_INCLINOMETER_3D);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &incl_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_incl_3d_ids[] = {
@@ -412,7 +410,7 @@ static struct platform_driver hid_incl_3d_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_incl_3d_probe,
- .remove = hid_incl_3d_remove,
+ .remove_new = hid_incl_3d_remove,
};
module_platform_driver(hid_incl_3d_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 47/49] iio: temperature: hid-sensor: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/temperature/hid-sensor-temperature.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index d40f235af1d4..0143fd478933 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -257,7 +257,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_temperature_remove(struct platform_device *pdev)
+static void hid_temperature_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -265,8 +265,6 @@ static int hid_temperature_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_temperature_ids[] = {
@@ -285,7 +283,7 @@ static struct platform_driver hid_temperature_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_temperature_probe,
- .remove = hid_temperature_remove,
+ .remove_new = hid_temperature_remove,
};
module_platform_driver(hid_temperature_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 35/49] iio: humidity: hid-sensor-humidity: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/humidity/hid-sensor-humidity.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index fa0fe404a70a..bf6d2636a85e 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -260,7 +260,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_humidity_remove(struct platform_device *pdev)
+static void hid_humidity_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -269,8 +269,6 @@ static int hid_humidity_remove(struct platform_device *pdev)
iio_device_unregister(indio_dev);
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY);
hid_sensor_remove_trigger(indio_dev, &humid_st->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_humidity_ids[] = {
@@ -289,7 +287,7 @@ static struct platform_driver hid_humidity_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_humidity_probe,
- .remove = hid_humidity_remove,
+ .remove_new = hid_humidity_remove,
};
module_platform_driver(hid_humidity_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 37/49] iio: light: hid-sensor-als: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/light/hid-sensor-als.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index eb1aedad7edc..cb76841ec0ce 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -347,7 +347,7 @@ static int hid_als_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_als_remove(struct platform_device *pdev)
+static void hid_als_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -356,8 +356,6 @@ static int hid_als_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, hsdev->usage);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &als_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_als_ids[] = {
@@ -380,7 +378,7 @@ static struct platform_driver hid_als_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_als_probe,
- .remove = hid_als_remove,
+ .remove_new = hid_als_remove,
};
module_platform_driver(hid_als_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 34/49] iio: gyro: hid-sensor-gyro-3d: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index 698c50da1f10..59a38bf9459b 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -359,7 +359,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_gyro_3d_remove(struct platform_device *pdev)
+static void hid_gyro_3d_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -368,8 +368,6 @@ static int hid_gyro_3d_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_gyro_3d_ids[] = {
@@ -388,7 +386,7 @@ static struct platform_driver hid_gyro_3d_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_gyro_3d_probe,
- .remove = hid_gyro_3d_remove,
+ .remove_new = hid_gyro_3d_remove,
};
module_platform_driver(hid_gyro_3d_platform_driver);
--
2.40.1
^ permalink raw reply related
* [PATCH 01/49] iio: accel: hid-sensor-accel-3d: Convert to platform remove callback returning void
From: Uwe Kleine-König @ 2023-09-19 17:48 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Lars-Peter Clausen, linux-input,
linux-iio, kernel
In-Reply-To: <20230919174931.1417681-1-u.kleine-koenig@pengutronix.de>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 5eac7ea19993..9b7a73a4c48a 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -422,7 +422,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
}
/* Function to deinitialize the processing for usage id */
-static int hid_accel_3d_remove(struct platform_device *pdev)
+static void hid_accel_3d_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -431,8 +431,6 @@ static int hid_accel_3d_remove(struct platform_device *pdev)
sensor_hub_remove_callback(hsdev, hsdev->usage);
iio_device_unregister(indio_dev);
hid_sensor_remove_trigger(indio_dev, &accel_state->common_attributes);
-
- return 0;
}
static const struct platform_device_id hid_accel_3d_ids[] = {
@@ -454,7 +452,7 @@ static struct platform_driver hid_accel_3d_platform_driver = {
.pm = &hid_sensor_pm_ops,
},
.probe = hid_accel_3d_probe,
- .remove = hid_accel_3d_remove,
+ .remove_new = hid_accel_3d_remove,
};
module_platform_driver(hid_accel_3d_platform_driver);
--
2.40.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox