* [PATCH 01/19] amba: store owner from modules with amba_driver_register()
From: Krzysztof Kozlowski @ 2024-03-26 20:23 UTC (permalink / raw)
To: Russell King, Suzuki K Poulose, Mike Leach, James Clark,
Alexander Shishkin, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Andi Shyti, Olivia Mackall, Herbert Xu, Vinod Koul,
Dmitry Torokhov, Miquel Raynal, Michal Simek, Krzysztof Kozlowski,
Eric Auger, Alex Williamson
Cc: linux-kernel, coresight, linux-arm-kernel, linux-stm32, linux-i2c,
linux-crypto, dmaengine, linux-input, kvm
In-Reply-To: <20240326-module-owner-amba-v1-0-4517b091385b@linaro.org>
Modules registering driver with amba_driver_register() often forget to
set .owner field. The field is used by some of other kernel parts for
reference counting (try_module_get()), so it is expected that drivers
will set it.
Solve the problem by moving this task away from the drivers to the core
amba bus code, just like we did for platform_driver in
commit 9447057eaff8 ("platform_device: use a macro instead of
platform_driver_register").
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/amba/bus.c | 11 +++++++----
include/linux/amba/bus.h | 11 +++++++++--
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index a24c152bfaac..aba3aa95b224 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -488,28 +488,31 @@ static int __init amba_stub_drv_init(void)
* waiting on amba_match(). So, register a stub driver to make sure
* amba_match() is called even if no amba driver has been registered.
*/
- return amba_driver_register(&amba_proxy_drv);
+ return __amba_driver_register(&amba_proxy_drv, NULL);
}
late_initcall_sync(amba_stub_drv_init);
/**
- * amba_driver_register - register an AMBA device driver
+ * __amba_driver_register - register an AMBA device driver
* @drv: amba device driver structure
+ * @owner: owning module/driver
*
* Register an AMBA device driver with the Linux device model
* core. If devices pre-exist, the drivers probe function will
* be called.
*/
-int amba_driver_register(struct amba_driver *drv)
+int __amba_driver_register(struct amba_driver *drv,
+ struct module *owner)
{
if (!drv->probe)
return -EINVAL;
+ drv->drv.owner = owner;
drv->drv.bus = &amba_bustype;
return driver_register(&drv->drv);
}
-EXPORT_SYMBOL(amba_driver_register);
+EXPORT_SYMBOL(__amba_driver_register);
/**
* amba_driver_unregister - remove an AMBA device driver
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index c60a6a14638c..958a55bcc708 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -112,11 +112,18 @@ extern struct bus_type amba_bustype;
#define amba_get_drvdata(d) dev_get_drvdata(&d->dev)
#define amba_set_drvdata(d,p) dev_set_drvdata(&d->dev, p)
+/*
+ * use a macro to avoid include chaining to get THIS_MODULE
+ */
+#define amba_driver_register(drv) \
+ __amba_driver_register(drv, THIS_MODULE)
+
#ifdef CONFIG_ARM_AMBA
-int amba_driver_register(struct amba_driver *);
+int __amba_driver_register(struct amba_driver *, struct module *);
void amba_driver_unregister(struct amba_driver *);
#else
-static inline int amba_driver_register(struct amba_driver *drv)
+static inline int __amba_driver_register(struct amba_driver *drv,
+ struct module *owner)
{
return -EINVAL;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 00/19] amba: store owner from modules with amba_driver_register()
From: Krzysztof Kozlowski @ 2024-03-26 20:23 UTC (permalink / raw)
To: Russell King, Suzuki K Poulose, Mike Leach, James Clark,
Alexander Shishkin, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Andi Shyti, Olivia Mackall, Herbert Xu, Vinod Koul,
Dmitry Torokhov, Miquel Raynal, Michal Simek, Krzysztof Kozlowski,
Eric Auger, Alex Williamson
Cc: linux-kernel, coresight, linux-arm-kernel, linux-stm32, linux-i2c,
linux-crypto, dmaengine, linux-input, kvm
Merging
=======
All further patches depend on the first amba patch, therefore please ack
and this should go via one tree.
Description
===========
Modules registering driver with amba_driver_register() often forget to
set .owner field.
Solve the problem by moving this task away from the drivers to the core
amba bus code, just like we did for platform_driver in commit
9447057eaff8 ("platform_device: use a macro instead of
platform_driver_register").
Best regards,
Krzysztof
---
Krzysztof Kozlowski (19):
amba: store owner from modules with amba_driver_register()
coresight: cti: drop owner assignment
coresight: catu: drop owner assignment
coresight: etm3x: drop owner assignment
coresight: etm4x: drop owner assignment
coresight: funnel: drop owner assignment
coresight: replicator: drop owner assignment
coresight: etb10: drop owner assignment
coresight: stm: drop owner assignment
coresight: tmc: drop owner assignment
coresight: tpda: drop owner assignment
coresight: tpdm: drop owner assignment
coresight: tpiu: drop owner assignment
i2c: nomadik: drop owner assignment
hwrng: nomadik: drop owner assignment
dmaengine: pl330: drop owner assignment
Input: ambakmi - drop owner assignment
memory: pl353-smc: drop owner assignment
vfio: amba: drop owner assignment
drivers/amba/bus.c | 11 +++++++----
drivers/char/hw_random/nomadik-rng.c | 1 -
drivers/dma/pl330.c | 1 -
drivers/hwtracing/coresight/coresight-catu.c | 1 -
drivers/hwtracing/coresight/coresight-cti-core.c | 1 -
drivers/hwtracing/coresight/coresight-etb10.c | 1 -
drivers/hwtracing/coresight/coresight-etm3x-core.c | 1 -
drivers/hwtracing/coresight/coresight-etm4x-core.c | 1 -
drivers/hwtracing/coresight/coresight-funnel.c | 1 -
drivers/hwtracing/coresight/coresight-replicator.c | 1 -
drivers/hwtracing/coresight/coresight-stm.c | 1 -
drivers/hwtracing/coresight/coresight-tmc-core.c | 1 -
drivers/hwtracing/coresight/coresight-tpda.c | 1 -
drivers/hwtracing/coresight/coresight-tpdm.c | 1 -
drivers/hwtracing/coresight/coresight-tpiu.c | 1 -
drivers/i2c/busses/i2c-nomadik.c | 1 -
drivers/input/serio/ambakmi.c | 1 -
drivers/memory/pl353-smc.c | 1 -
drivers/vfio/platform/vfio_amba.c | 1 -
include/linux/amba/bus.h | 11 +++++++++--
20 files changed, 16 insertions(+), 24 deletions(-)
---
base-commit: 1fdad13606e104ff103ca19d2d660830cb36d43e
change-id: 20240326-module-owner-amba-3ebb65256be7
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
^ permalink raw reply
* Re: [PATCH v1 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Conor Dooley @ 2024-03-26 19:28 UTC (permalink / raw)
To: Allen Lin
Cc: Conor Dooley, Rob Herring, dmitry.torokhov,
krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires,
linux-input, devicetree, linux-kernel
In-Reply-To: <TY0PR06MB561197578717990F4BEA93D29E352@TY0PR06MB5611.apcprd06.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 2946 bytes --]
On Tue, Mar 26, 2024 at 06:40:28PM +0800, Allen Lin wrote:
> Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
> >
> > On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > > >
> > > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > > touchscreen controller.
> > > > > > >
> > > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > > >
> > > > > > note to self/Krzysztof/Rob:
> > > > > > There was a previous attempt at this kind of device. This version looks
> > > > > > better but might be incomplete given there's a bunch more properties in
> > > > > > that patchset:
> > > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > > >
> > > > > Those don't look like properties we want coming back.
> > > >
> > > > Oh, I don't want most of them coming back either. There are some
> > > > supplies in there though that I think we would like to come back, no?
> > > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > > really seem credible.
> > >
> > > We will use Firmware-name in Device Tree.
> >
> > > For power supply settings, because there may be other device using
> > > same regulator.
> >
> > If there are other devices using the same regulator is it more
> > important that you document it so that it doesn't get disabled by the
> > other users.
> >
> > > We plan to define it as an optional property for user to control in
> > > next release.
> >
> > I don't see how the regulator would not be required, the device doesn't
> > function without power.
> >
> > Thanks,
> > Conor.
>
> I will set power supply as required.
> The description of power supply as below,
>
> properties:
> vccd-supply:
> description: A phandle for the regualtor supplying IO power. Should be own
> by TPIC only.
What does "owned by TPIC" only mean? Why would the vccd supply not be
allowed to be shared with another device?
> This works for TP digital IO only, main power is
> given by display part VSP/VSN power source which is controlled
> by lcm driver.
What drivers control things doesn't really matter here, we're just
describing the hardware. If there's another supply to the controller,
then document it too please.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: In kernel virtual HID devices (was Future handling of complex RGB devices on Linux v3)
From: Werner Sembach @ 2024-03-26 16:57 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Hans de Goede, Lee Jones, jikos, linux-kernel, Jelle van der Waa,
Miguel Ojeda, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds, Pavel Machek, Gregor Riepl
In-Reply-To: <vjd5xqgd2gsyz4ubgk6eusuyqdtxpdw6vogc5u537x2a245xcj@m2twppbxea4p>
Hi all,
Am 26.03.24 um 16:39 schrieb Benjamin Tissoires:
> On Mar 26 2024, Werner Sembach wrote:
>> Hi all,
>>
>> Am 25.03.24 um 19:30 schrieb Hans de Goede:
>>
>> [snip]
>>>>> If the kernel already handles the custom protocol into generic HID, the
>>>>> work for userspace is not too hard because they can deal with a known
>>>>> protocol and can be cross-platform in their implementation.
>>>>>
>>>>> I'm mentioning that cross-platform because SDL used to rely on the
>>>>> input, LEDs, and other Linux peculiarities and eventually fell back on
>>>>> using hidraw only because it's way more easier that way.
>>>>>
>>>>> The other advantage of LampArray is that according to Microsoft's
>>>>> document, new devices are going to support it out of the box, so they'll
>>>>> be supported out of the box directly.
>>>>>
>>>>> Most of the time my stance is "do not add new kernel API, you'll regret
>>>>> it later". So in that case, given that we have a formally approved
>>>>> standard, I would suggest to use it, and consider it your API.
>>>> The only new UAPI would be the use_leds_uapi switch to turn on/off the backwards compatibility.
> I have my reserves with such a kill switch (see below).
>
>>> Actually we don't even need that. Typically there is a single HID
>>> driver handling both keys and the backlight, so userspace cannot
>>> just unbind the HID driver since then the keys stop working.
> I don't think Werner meant unbinding the HID driver, just a toggle to
> enable/disable the basic HID core processing of LampArray.
>
>>> But with a virtual LampArray HID device the only functionality
>>> for an in kernel HID driver would be to export a basic keyboard
>>> backlight control interface for simple non per key backlight control
>>> to integrate nicely with e.g. GNOME's backlight control.
>> Don't forget that in the future there will be devices that natively support
>> LampArray in their firmware, so for them it is the same device.
> Yeah, the generic LampArray support will not be able to differentiate
> "emulated" devices from native ones.
>
>> Regards,
>>
>> Werner
>>
>>> And then when OpenRGB wants to take over it can just unbind the HID
>>> driver from the HID device using existing mechanisms for that.
> Again no, it'll be too unpredicted.
>
>>> Hmm, I wonder if that will not also kill hidraw support though ...
>>> I guess getting hidraw support back might require then also manually
>>> binding the default HID input driver. Bentiss any input on this?
> To be able to talk over hidraw you need a driver to be bound, yes. But I
> had the impression that LampArray would be supported by default in
> hid-input.c, thus making this hard to remove. Having a separate driver
> will work, but as soon as the LampArray device will also export a
> multitouch touchpad, we are screwed and will have to make a choice
> between LampArray and touch...
>
>>> Background info: as discussed earlier in the thread Werner would like
>>> to have a basic driver registering a /sys/class/leds/foo::kbd_backlight/
>>> device, since those are automatically supported by GNOME (and others)
>>> and will give basic kbd backlight brightness control in the desktop
>>> environment. This could be a simple HID driver for
>>> the hid_allocate_device()-ed virtual HID device, but userspace needs
>>> to be able to move that out of the way when it wants to take over
>>> full control of the per key lighting.
> Do we really need to entirely unregister the led class device? Can't we
> snoop on the commands and get some "mean value"?
>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>> The control flow for the whole system would look something like this:
>>>>
>>>> - System boots
>>>>
>>>> - Kernel driver initializes keyboard (maybe stops rainbowpuke boot effects, sets brightness to a default value, or initializes a solid color)
>>>>
>>>> - systemd-backlight restores last keyboard backlight brightness
>>>>
>>>> - UPower sees sysfs leds entry and exposes it to DBus for DEs to do keyboard brightness handling
>>>>
>>>> - If the user wants more control they (auto-)start OpenRGB
>>>>
>>>> - OpenRGB disables sysfs leds entry via use_leds_uapi to prevent double control of the same device by UPower
>>>>
>>>> - OpenRGB directly interacts with hidraw device via LampArray API to give fine granular control of the backlight
>>>>
>>>> - When OpenRGB closes it should reenable the sysfs leds entry
> That's where your plan falls short: if OpenRGB crashes, or is killed it
> will not reset that bit.
>
> Next question: is OpenRGB supposed to keep the hidraw node opened all
> the time or not?
TBH I didn't look at the OpenRGB code yet and LampArray there is currently only
planned. I somewhat hope that until the kernel driver is ready someone else
already picked up implementing LampArray in OpenRGB.
>
> If it has to keep it open, we should be able to come up with a somewhat
> similar hack that we have with hid-steam: when the hidraw node is
> opened, we disable the kernel processing of LampArray. When the node is
> closed, we re-enable it.
>
> But that also means we have to distinguish steam/SDL from OpenRGB...
My first thought here also: What is if something else is reading hidraw devices?
Especially for hidraw devices that are not just LampArray.
>
> I just carefully read the LampArray spec. And it's simpler than what
> I expected. But the thing that caught my attention was that it's
> mentioned that there is no way for the host to query the current
> color/illumination of the LEDs when the mode is set to
> AutonomousMode=false. Which means that the kernel should be able to
> snoop into any commands sent from OpenRGB to the device, compute a mean
> value and update its internal state.
>
> Basically all we need is the "toggle" to put the led class in read-only
> mode while OpenRGB kicks in. Maybe given that we are about to snoop on
> the commands sent, we can detect that there is a LampArray command
> emitted, attach this information to the struct file * in hidraw, and
> then re-enable rw when that user closes the hidraw node.
I think a read-only mode is not part of the current led class UAPI. Also I don't
want to associate AutonomousMode=true with led class is used.
AutonomousMode=true could for example mean that it is controlled via keyboard
shortcuts that are directly handled in the keyboard firmware, aka a case where
you want neither OpenRGB nor led class make any writes to the keyboard.
Or AutonomousMode=true could mean that on a device that implements both a
LampArray interface as well as a proprietary legacy interface is currently
controlled via the proprietary legacy interface (a lot of which are supported by
OpenRGB).
Regards,
Werner
>
> Cheers,
> Benjamin
>
>>>> - System shutdown
>>>>
>>>> - Since OpenRGB reenables the sysfs leds entry, systemd-backlight can correctly store a brightness value for next boot
>>>>
>>>> Regards,
>>>>
>>>> Werner
>>>>
>>>>> Side note to self: I really need to resurrect the hidraw revoke series
>>>>> so we could export those hidraw node to userspace with uaccess through
>>>>> logind...
>>>>>
>>>>> Cheers,
>>>>> Benjamin
^ permalink raw reply
* Re: [PATCH v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
From: Rhodes, David @ 2024-03-26 16:06 UTC (permalink / raw)
To: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree
In-Reply-To: <20240320192956.2395731-6-jogletre@opensource.cirrus.com>
On 3/20/24 2:29 PM, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
>
> The ASoC driver enables I2S streaming to the device.
>
> Signed-off-by: James Ogletree<jogletre@opensource.cirrus.com>
> ---
Reviewed-by: David Rhodes <drhodes@opensource.cirrus.com>
Thanks,
David
^ permalink raw reply
* Re: In kernel virtual HID devices (was Future handling of complex RGB devices on Linux v3)
From: Benjamin Tissoires @ 2024-03-26 15:39 UTC (permalink / raw)
To: Werner Sembach
Cc: Hans de Goede, Lee Jones, jikos, linux-kernel, Jelle van der Waa,
Miguel Ojeda, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds, Pavel Machek, Gregor Riepl
In-Reply-To: <65b24776-ae1a-4290-a1d5-c7637ad0accc@tuxedocomputers.com>
On Mar 26 2024, Werner Sembach wrote:
> Hi all,
>
> Am 25.03.24 um 19:30 schrieb Hans de Goede:
>
> [snip]
> > > > If the kernel already handles the custom protocol into generic HID, the
> > > > work for userspace is not too hard because they can deal with a known
> > > > protocol and can be cross-platform in their implementation.
> > > >
> > > > I'm mentioning that cross-platform because SDL used to rely on the
> > > > input, LEDs, and other Linux peculiarities and eventually fell back on
> > > > using hidraw only because it's way more easier that way.
> > > >
> > > > The other advantage of LampArray is that according to Microsoft's
> > > > document, new devices are going to support it out of the box, so they'll
> > > > be supported out of the box directly.
> > > >
> > > > Most of the time my stance is "do not add new kernel API, you'll regret
> > > > it later". So in that case, given that we have a formally approved
> > > > standard, I would suggest to use it, and consider it your API.
> > > The only new UAPI would be the use_leds_uapi switch to turn on/off the backwards compatibility.
I have my reserves with such a kill switch (see below).
> > Actually we don't even need that. Typically there is a single HID
> > driver handling both keys and the backlight, so userspace cannot
> > just unbind the HID driver since then the keys stop working.
I don't think Werner meant unbinding the HID driver, just a toggle to
enable/disable the basic HID core processing of LampArray.
> >
> > But with a virtual LampArray HID device the only functionality
> > for an in kernel HID driver would be to export a basic keyboard
> > backlight control interface for simple non per key backlight control
> > to integrate nicely with e.g. GNOME's backlight control.
>
> Don't forget that in the future there will be devices that natively support
> LampArray in their firmware, so for them it is the same device.
Yeah, the generic LampArray support will not be able to differentiate
"emulated" devices from native ones.
>
> Regards,
>
> Werner
>
> > And then when OpenRGB wants to take over it can just unbind the HID
> > driver from the HID device using existing mechanisms for that.
Again no, it'll be too unpredicted.
> >
> > Hmm, I wonder if that will not also kill hidraw support though ...
> > I guess getting hidraw support back might require then also manually
> > binding the default HID input driver. Bentiss any input on this?
To be able to talk over hidraw you need a driver to be bound, yes. But I
had the impression that LampArray would be supported by default in
hid-input.c, thus making this hard to remove. Having a separate driver
will work, but as soon as the LampArray device will also export a
multitouch touchpad, we are screwed and will have to make a choice
between LampArray and touch...
> >
> > Background info: as discussed earlier in the thread Werner would like
> > to have a basic driver registering a /sys/class/leds/foo::kbd_backlight/
> > device, since those are automatically supported by GNOME (and others)
> > and will give basic kbd backlight brightness control in the desktop
> > environment. This could be a simple HID driver for
> > the hid_allocate_device()-ed virtual HID device, but userspace needs
> > to be able to move that out of the way when it wants to take over
> > full control of the per key lighting.
Do we really need to entirely unregister the led class device? Can't we
snoop on the commands and get some "mean value"?
> >
> > Regards,
> >
> > Hans
> >
> >
> >
> >
> >
> >
> >
> > > The control flow for the whole system would look something like this:
> > >
> > > - System boots
> > >
> > > - Kernel driver initializes keyboard (maybe stops rainbowpuke boot effects, sets brightness to a default value, or initializes a solid color)
> > >
> > > - systemd-backlight restores last keyboard backlight brightness
> > >
> > > - UPower sees sysfs leds entry and exposes it to DBus for DEs to do keyboard brightness handling
> > >
> > > - If the user wants more control they (auto-)start OpenRGB
> > >
> > > - OpenRGB disables sysfs leds entry via use_leds_uapi to prevent double control of the same device by UPower
> > >
> > > - OpenRGB directly interacts with hidraw device via LampArray API to give fine granular control of the backlight
> > >
> > > - When OpenRGB closes it should reenable the sysfs leds entry
That's where your plan falls short: if OpenRGB crashes, or is killed it
will not reset that bit.
Next question: is OpenRGB supposed to keep the hidraw node opened all
the time or not?
If it has to keep it open, we should be able to come up with a somewhat
similar hack that we have with hid-steam: when the hidraw node is
opened, we disable the kernel processing of LampArray. When the node is
closed, we re-enable it.
But that also means we have to distinguish steam/SDL from OpenRGB...
I just carefully read the LampArray spec. And it's simpler than what
I expected. But the thing that caught my attention was that it's
mentioned that there is no way for the host to query the current
color/illumination of the LEDs when the mode is set to
AutonomousMode=false. Which means that the kernel should be able to
snoop into any commands sent from OpenRGB to the device, compute a mean
value and update its internal state.
Basically all we need is the "toggle" to put the led class in read-only
mode while OpenRGB kicks in. Maybe given that we are about to snoop on
the commands sent, we can detect that there is a LampArray command
emitted, attach this information to the struct file * in hidraw, and
then re-enable rw when that user closes the hidraw node.
Cheers,
Benjamin
> > >
> > > - System shutdown
> > >
> > > - Since OpenRGB reenables the sysfs leds entry, systemd-backlight can correctly store a brightness value for next boot
> > >
> > > Regards,
> > >
> > > Werner
> > >
> > > > Side note to self: I really need to resurrect the hidraw revoke series
> > > > so we could export those hidraw node to userspace with uaccess through
> > > > logind...
> > > >
> > > > Cheers,
> > > > Benjamin
^ permalink raw reply
* Re: [PATCH v9 00/38] ep93xx device tree conversion
From: Andy Shevchenko @ 2024-03-26 14:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: nikita.shubin, Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Michael Turquette, Stephen Boyd, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Wim Van Sebroeck,
Guenter Roeck, Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Ralf Baechle, Wu, Aaron, Lee Jones,
Olof Johansson, Niklas Cassel, linux-arm-kernel, linux-kernel,
linux-gpio, linux-clk, linux-pm, devicetree, dmaengine,
linux-watchdog, linux-pwm, linux-spi, netdev, linux-mtd,
linux-ide, linux-input, linux-sound, Arnd Bergmann,
Bartosz Golaszewski, Andrew Lunn
In-Reply-To: <dc3e2cb4-f631-4611-8814-0dc04c5502f0@linaro.org>
On Tue, Mar 26, 2024 at 11:19:54AM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 10:18, Nikita Shubin via B4 Relay wrote:
> > The goal is to recieve ACKs for all patches in series to merge it via Arnd branch.
> >
> > Some changes since last version (v8):
> >
> > - Most important, fixed bug in Device Tree resulting in CS4271 not working by Alexander Sverdlin.
> > - added #interrupt-cells to gpio nodes with interrupts-controller
> > - fixed some EOF in dtsi files
> > - fixed identation and type in ep93xx-keypad thanks to Andy Shevchenko
> >
> > Stephen Boyd, Vinod Koul PLEASE! give some comments on following, couse i hadn't one for a couple of iterations already:
> >
> > Following patches require attention from Stephen Boyd, as they were converted to aux_dev as suggested:
> >
> > - ARM: ep93xx: add regmap aux_dev
> > - clk: ep93xx: add DT support for Cirrus EP93xx
> >
> > Following patches require attention from Vinod Koul:
> >
> > - dma: cirrus: Convert to DT for Cirrus EP93xx
> > - dma: cirrus: remove platform code
>
> A lot of this could have been already merged if you split it... Just
> saying...
But you able to apply DT schema patches if you wish.
Just doing? :-)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition
From: Doug Anderson @ 2024-03-26 13:43 UTC (permalink / raw)
To: Łukasz Majczak
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Hans de Goede,
Maxime Ripard, Kai-Heng Feng, Johan Hovold, linux-input,
linux-kernel, Radoslaw Biernacki
In-Reply-To: <CAE5UKNpob9dT4YXSYKsrKGQ3cZP7o3OF7y-eb925Xd-gH3BEXA@mail.gmail.com>
Hi,
On Tue, Mar 26, 2024 at 1:58 AM Łukasz Majczak <lma@chromium.org> wrote:
>
> > nit: checkpatch should have yelled at you saying that you should
> > specify a commit in the format:
> >
> > commit b3a81b6c4fc6 ("HID: i2c-hid: check if device is there before
> > really probing")
> >
> I will do it, but I did run the checkpatch (with --strict option) and
> it didn't complain about anything.
Weird that checkpatch didn't yell, but perhaps somehow your commit
message didn't trigger its regex. ;-)
> > nit: I believe your sign off should be last. It's also unclear why two
> > signoffs. Did Radoslaw author it and you changed it? ...or was it
> > Co-Developed-by, or ...? You'll probably need to adjust your tags a
> > bit depending on the answers.
> >
> Yes, we've discussed this patch together and the original
> investigation was done by Rad.
Sounds good. Probably the best way to tag is these tags in this order:
Co-developed-by: Radoslaw Biernacki <rad@chromium.org>
Signed-off-by: Radoslaw Biernacki <rad@chromium.org>
Signed-off-by: Lukasz Majczak <lma@chromium.org>
> > Having both ends of the usleep be 400 is iffy. In this case it's at
> > probe time so I wonder if udelay() is better? If not, maybe give at
> > least _some_ margin?
> >
> >
> > > + } while (tries-- > 0 && ret < 0);
> >
> According to Documentation/timers/timers-howto.rst:
> " SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
> * Use usleep_range"
> It was also pointed out by checkpath (when I initially used msleep).
> I think giving some margin (eg. 400,500) would be ok.
Yeah, usleep_range(400, 500) would be fine. udelay(400) would also be
OK. The later would be more "accurate" but also more wasteful of CPU
cycles. Given that this is at probe time and only run a small handful
of times, it probably doesn't matter lots though perhaps the sleeping
function would allow more parallelism of other running probes.
-Doug
^ permalink raw reply
* [PATCH] Input: PS/2 - add support for Lenovo Xiaoxin keyboard
From: WangYuli @ 2024-03-26 13:17 UTC (permalink / raw)
To: wangyuli, dmitry.torokhov, rrangel, shaoyang, helugang
Cc: linux-input, linux-kernel, huangbibo
Modified keyboard_ids in function ps2_is_keyboard_id
for Lenovo Xiaoxin keyboard.
Signed-off-by: yuanjianye <yuanjianye@uniontech.com>
Signed-off-by: shaoyang <shaoyang@uniontech.com>
Reviewed-by: huangbibo <huangbibo@uniontech.com>
Signed-off-by: helugang <helugang@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
drivers/input/serio/libps2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 6d78a1fe00c1..39d46526c56a 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -189,6 +189,7 @@ bool ps2_is_keyboard_id(u8 id_byte)
0x5d, /* Trust keyboard */
0x60, /* NMB SGI keyboard, translated */
0x47, /* NMB SGI keyboard */
+ 0x83, /* Lenovo Xiaoxin keyboard */
};
return memchr(keyboard_ids, id_byte, sizeof(keyboard_ids)) != NULL;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v1 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Allen Lin @ 2024-03-26 10:40 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, Rob Herring, dmitry.torokhov,
krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires,
linux-input, devicetree, linux-kernel
In-Reply-To: <20240326-whoever-spotter-1fe7ace35428@wendy>
Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
>
> On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > >
> > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > touchscreen controller.
> > > > > >
> > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > >
> > > > > note to self/Krzysztof/Rob:
> > > > > There was a previous attempt at this kind of device. This version looks
> > > > > better but might be incomplete given there's a bunch more properties in
> > > > > that patchset:
> > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > >
> > > > Those don't look like properties we want coming back.
> > >
> > > Oh, I don't want most of them coming back either. There are some
> > > supplies in there though that I think we would like to come back, no?
> > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > really seem credible.
> >
> > We will use Firmware-name in Device Tree.
>
> > For power supply settings, because there may be other device using
> > same regulator.
>
> If there are other devices using the same regulator is it more
> important that you document it so that it doesn't get disabled by the
> other users.
>
> > We plan to define it as an optional property for user to control in
> > next release.
>
> I don't see how the regulator would not be required, the device doesn't
> function without power.
>
> Thanks,
> Conor.
I will set power supply as required.
The description of power supply as below,
properties:
vccd-supply:
description: A phandle for the regualtor supplying IO power. Should be own
by TPIC only. This works for TP digital IO only, main power is
given by display part VSP/VSN power source which is controlled
by lcm driver.
required:
- compatible
- reg
- interrupts
- reset-gpios
- panel
- vccd-supply
Thanks
Allen
^ permalink raw reply
* Re: [PATCH v9 00/38] ep93xx device tree conversion
From: Krzysztof Kozlowski @ 2024-03-26 10:19 UTC (permalink / raw)
To: nikita.shubin, Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Wim Van Sebroeck, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Mark Brown, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Ralf Baechle, Wu, Aaron, Lee Jones, Olof Johansson, Niklas Cassel
Cc: linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, linux-pm,
devicetree, dmaengine, linux-watchdog, linux-pwm, linux-spi,
netdev, linux-mtd, linux-ide, linux-input, linux-sound,
Arnd Bergmann, Bartosz Golaszewski, Andy Shevchenko, Andrew Lunn,
Andy Shevchenko
In-Reply-To: <20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me>
On 26/03/2024 10:18, Nikita Shubin via B4 Relay wrote:
> The goal is to recieve ACKs for all patches in series to merge it via Arnd branch.
>
> Some changes since last version (v8):
>
> - Most important, fixed bug in Device Tree resulting in CS4271 not working by Alexander Sverdlin.
> - added #interrupt-cells to gpio nodes with interrupts-controller
> - fixed some EOF in dtsi files
> - fixed identation and type in ep93xx-keypad thanks to Andy Shevchenko
>
> Stephen Boyd, Vinod Koul PLEASE! give some comments on following, couse i hadn't one for a couple of iterations already:
>
> Following patches require attention from Stephen Boyd, as they were converted to aux_dev as suggested:
>
> - ARM: ep93xx: add regmap aux_dev
> - clk: ep93xx: add DT support for Cirrus EP93xx
>
> Following patches require attention from Vinod Koul:
>
> - dma: cirrus: Convert to DT for Cirrus EP93xx
> - dma: cirrus: remove platform code
A lot of this could have been already merged if you split it... Just
saying...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v9 00/38] ep93xx device tree conversion
From: Arnd Bergmann @ 2024-03-26 10:07 UTC (permalink / raw)
To: Nikita Shubin, Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Wim Van Sebroeck, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Mark Brown, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Ralf Baechle, Aaron Wu, Lee Jones, Olof Johansson, Niklas Cassel
Cc: linux-arm-kernel, linux-kernel, open list:GPIO SUBSYSTEM,
linux-clk, linux-pm, devicetree, dmaengine, linux-watchdog,
linux-pwm, linux-spi, Netdev, linux-mtd, linux-ide, linux-input,
linux-sound, Bartosz Golaszewski, Krzysztof Kozlowski,
Andy Shevchenko, Andrew Lunn, Andy Shevchenko
In-Reply-To: <20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me>
On Tue, Mar 26, 2024, at 10:18, Nikita Shubin via B4 Relay wrote:
> The goal is to recieve ACKs for all patches in series to merge it via
> Arnd branch.
Thank you for the continued updates, I really hope we can merge
it all for 6.10. I've looked through it again and I'm pretty much
ready to just merge it, though I admit that the process is not
working out that great, and it would probably have been quicker
to add DT support to drivers individually through the subsystem
trees.
> Stephen Boyd, Vinod Koul PLEASE! give some comments on following, couse
> i hadn't one for a couple of iterations already:
>
> Following patches require attention from Stephen Boyd, as they were
> converted to aux_dev as suggested:
>
> - ARM: ep93xx: add regmap aux_dev
> - clk: ep93xx: add DT support for Cirrus EP93xx
>
> Following patches require attention from Vinod Koul:
>
> - dma: cirrus: Convert to DT for Cirrus EP93xx
> - dma: cirrus: remove platform code
I suspect that Stephen and Vinod may be missing this, as reviewing
a 38 patch series tends to be a lot of work, and they may have
missed that they are on the critical path here. I certainly
tend to just ignore an entire thread when it looks like I'm not
immediately going to be reviewing it all and other people are
likely to have more comments first, so I'm not blaming them.
To better catch their attention, I would suggest you repost the
two smaller sets of patches as a separate series, with only the
relevant people on Cc. Please also include the respective
bindings when you send send these patches to Stephen and
Vinod.
Arnd
^ permalink raw reply
* [PATCH v9 23/38] input: keypad: ep93xx: add DT support for Cirrus EP93xx
From: Nikita Shubin via B4 Relay @ 2024-03-26 9:18 UTC (permalink / raw)
To: Hartley Sweeten, Alexander Sverdlin, Russell King,
Dmitry Torokhov, Uwe Kleine-König, Nikita Shubin,
Linus Walleij, Andy Shevchenko, Sergey Shtylyov
Cc: Damien Le Moal, linux-arm-kernel, linux-kernel, linux-input,
Arnd Bergmann
In-Reply-To: <20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me>
From: Nikita Shubin <nikita.shubin@maquefel.me>
- drop flags, they were not used anyway
- add OF ID match table
- process "autorepeat", "debounce-delay-ms", prescale from device tree
- drop platform data usage and it's header
- keymap goes from device tree now on
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
arch/arm/mach-ep93xx/core.c | 46 ---------------------
drivers/input/keyboard/ep93xx_keypad.c | 74 ++++++++++------------------------
include/linux/soc/cirrus/ep93xx.h | 4 --
3 files changed, 22 insertions(+), 102 deletions(-)
diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
index 03bce5e9d1f1..b99c46d22c4d 100644
--- a/arch/arm/mach-ep93xx/core.c
+++ b/arch/arm/mach-ep93xx/core.c
@@ -697,52 +697,6 @@ void __init ep93xx_register_keypad(struct ep93xx_keypad_platform_data *data)
platform_device_register(&ep93xx_keypad_device);
}
-int ep93xx_keypad_acquire_gpio(struct platform_device *pdev)
-{
- int err;
- int i;
-
- for (i = 0; i < 8; i++) {
- err = gpio_request(EP93XX_GPIO_LINE_C(i), dev_name(&pdev->dev));
- if (err)
- goto fail_gpio_c;
- err = gpio_request(EP93XX_GPIO_LINE_D(i), dev_name(&pdev->dev));
- if (err)
- goto fail_gpio_d;
- }
-
- /* Enable the keypad controller; GPIO ports C and D used for keypad */
- ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_KEYS |
- EP93XX_SYSCON_DEVCFG_GONK);
-
- return 0;
-
-fail_gpio_d:
- gpio_free(EP93XX_GPIO_LINE_C(i));
-fail_gpio_c:
- for (--i; i >= 0; --i) {
- gpio_free(EP93XX_GPIO_LINE_C(i));
- gpio_free(EP93XX_GPIO_LINE_D(i));
- }
- return err;
-}
-EXPORT_SYMBOL(ep93xx_keypad_acquire_gpio);
-
-void ep93xx_keypad_release_gpio(struct platform_device *pdev)
-{
- int i;
-
- for (i = 0; i < 8; i++) {
- gpio_free(EP93XX_GPIO_LINE_C(i));
- gpio_free(EP93XX_GPIO_LINE_D(i));
- }
-
- /* Disable the keypad controller; GPIO ports C and D used for GPIO */
- ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
- EP93XX_SYSCON_DEVCFG_GONK);
-}
-EXPORT_SYMBOL(ep93xx_keypad_release_gpio);
-
/*************************************************************************
* EP93xx I2S audio peripheral handling
*************************************************************************/
diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
index 6b811d6bf625..dcbc50304a5a 100644
--- a/drivers/input/keyboard/ep93xx_keypad.c
+++ b/drivers/input/keyboard/ep93xx_keypad.c
@@ -6,20 +6,13 @@
*
* Based on the pxa27x matrix keypad controller by Rodolfo Giometti.
*
- * NOTE:
- *
- * The 3-key reset is triggered by pressing the 3 keys in
- * Row 0, Columns 2, 4, and 7 at the same time. This action can
- * be disabled by setting the EP93XX_KEYPAD_DISABLE_3_KEY flag.
- *
- * Normal operation for the matrix does not autorepeat the key press.
- * This action can be enabled by setting the EP93XX_KEYPAD_AUTOREPEAT
- * flag.
*/
#include <linux/bits.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/interrupt.h>
#include <linux/clk.h>
#include <linux/io.h>
@@ -27,7 +20,6 @@
#include <linux/input/matrix_keypad.h>
#include <linux/slab.h>
#include <linux/soc/cirrus/ep93xx.h>
-#include <linux/platform_data/keypad-ep93xx.h>
#include <linux/pm_wakeirq.h>
/*
@@ -61,12 +53,16 @@
#define KEY_REG_KEY1_MASK GENMASK(5, 0)
#define KEY_REG_KEY1_SHIFT 0
+#define EP93XX_MATRIX_ROWS (8)
+#define EP93XX_MATRIX_COLS (8)
+
#define EP93XX_MATRIX_SIZE (EP93XX_MATRIX_ROWS * EP93XX_MATRIX_COLS)
struct ep93xx_keypad {
- struct ep93xx_keypad_platform_data *pdata;
struct input_dev *input_dev;
struct clk *clk;
+ unsigned int debounce;
+ u16 prescale;
void __iomem *mmio_base;
@@ -133,23 +129,11 @@ static irqreturn_t ep93xx_keypad_irq_handler(int irq, void *dev_id)
static void ep93xx_keypad_config(struct ep93xx_keypad *keypad)
{
- struct ep93xx_keypad_platform_data *pdata = keypad->pdata;
unsigned int val = 0;
- clk_set_rate(keypad->clk, pdata->clk_rate);
+ val |= (keypad->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK;
- if (pdata->flags & EP93XX_KEYPAD_DISABLE_3_KEY)
- val |= KEY_INIT_DIS3KY;
- if (pdata->flags & EP93XX_KEYPAD_DIAG_MODE)
- val |= KEY_INIT_DIAG;
- if (pdata->flags & EP93XX_KEYPAD_BACK_DRIVE)
- val |= KEY_INIT_BACK;
- if (pdata->flags & EP93XX_KEYPAD_TEST_MODE)
- val |= KEY_INIT_T2;
-
- val |= ((pdata->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK);
-
- val |= ((pdata->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK);
+ val |= (keypad->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK;
__raw_writel(val, keypad->mmio_base + KEY_INIT);
}
@@ -220,17 +204,10 @@ static int ep93xx_keypad_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(ep93xx_keypad_pm_ops,
ep93xx_keypad_suspend, ep93xx_keypad_resume);
-static void ep93xx_keypad_release_gpio_action(void *_pdev)
-{
- struct platform_device *pdev = _pdev;
-
- ep93xx_keypad_release_gpio(pdev);
-}
-
static int ep93xx_keypad_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct ep93xx_keypad *keypad;
- const struct matrix_keymap_data *keymap_data;
struct input_dev *input_dev;
int err;
@@ -238,14 +215,6 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
if (!keypad)
return -ENOMEM;
- keypad->pdata = dev_get_platdata(&pdev->dev);
- if (!keypad->pdata)
- return -EINVAL;
-
- keymap_data = keypad->pdata->keymap_data;
- if (!keymap_data)
- return -EINVAL;
-
keypad->irq = platform_get_irq(pdev, 0);
if (keypad->irq < 0)
return keypad->irq;
@@ -254,19 +223,13 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
if (IS_ERR(keypad->mmio_base))
return PTR_ERR(keypad->mmio_base);
- err = ep93xx_keypad_acquire_gpio(pdev);
- if (err)
- return err;
-
- err = devm_add_action_or_reset(&pdev->dev,
- ep93xx_keypad_release_gpio_action, pdev);
- if (err)
- return err;
-
keypad->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(keypad->clk))
return PTR_ERR(keypad->clk);
+ device_property_read_u32(dev, "debounce-delay-ms", &keypad->debounce);
+ device_property_read_u16(dev, "cirrus,prescale", &keypad->prescale);
+
input_dev = devm_input_allocate_device(&pdev->dev);
if (!input_dev)
return -ENOMEM;
@@ -278,13 +241,13 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
input_dev->open = ep93xx_keypad_open;
input_dev->close = ep93xx_keypad_close;
- err = matrix_keypad_build_keymap(keymap_data, NULL,
+ err = matrix_keypad_build_keymap(NULL, NULL,
EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS,
keypad->keycodes, input_dev);
if (err)
return err;
- if (keypad->pdata->flags & EP93XX_KEYPAD_AUTOREPEAT)
+ if (device_property_read_bool(&pdev->dev, "autorepeat"))
__set_bit(EV_REP, input_dev->evbit);
input_set_drvdata(input_dev, keypad);
@@ -313,10 +276,17 @@ static void ep93xx_keypad_remove(struct platform_device *pdev)
dev_pm_clear_wake_irq(&pdev->dev);
}
+static const struct of_device_id ep93xx_keypad_of_ids[] = {
+ { .compatible = "cirrus,ep9307-keypad" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ep93xx_keypad_of_ids);
+
static struct platform_driver ep93xx_keypad_driver = {
.driver = {
.name = "ep93xx-keypad",
.pm = pm_sleep_ptr(&ep93xx_keypad_pm_ops),
+ .of_match_table = ep93xx_keypad_of_ids,
},
.probe = ep93xx_keypad_probe,
.remove_new = ep93xx_keypad_remove,
diff --git a/include/linux/soc/cirrus/ep93xx.h b/include/linux/soc/cirrus/ep93xx.h
index a27447971302..8942bfaf1545 100644
--- a/include/linux/soc/cirrus/ep93xx.h
+++ b/include/linux/soc/cirrus/ep93xx.h
@@ -41,8 +41,6 @@ int ep93xx_pwm_acquire_gpio(struct platform_device *pdev);
void ep93xx_pwm_release_gpio(struct platform_device *pdev);
int ep93xx_ide_acquire_gpio(struct platform_device *pdev);
void ep93xx_ide_release_gpio(struct platform_device *pdev);
-int ep93xx_keypad_acquire_gpio(struct platform_device *pdev);
-void ep93xx_keypad_release_gpio(struct platform_device *pdev);
int ep93xx_i2s_acquire(void);
void ep93xx_i2s_release(void);
unsigned int ep93xx_chip_revision(void);
@@ -52,8 +50,6 @@ static inline int ep93xx_pwm_acquire_gpio(struct platform_device *pdev) { return
static inline void ep93xx_pwm_release_gpio(struct platform_device *pdev) {}
static inline int ep93xx_ide_acquire_gpio(struct platform_device *pdev) { return 0; }
static inline void ep93xx_ide_release_gpio(struct platform_device *pdev) {}
-static inline int ep93xx_keypad_acquire_gpio(struct platform_device *pdev) { return 0; }
-static inline void ep93xx_keypad_release_gpio(struct platform_device *pdev) {}
static inline int ep93xx_i2s_acquire(void) { return 0; }
static inline void ep93xx_i2s_release(void) {}
static inline unsigned int ep93xx_chip_revision(void) { return 0; }
--
2.41.0
^ permalink raw reply related
* [PATCH v9 22/38] dt-bindings: input: Add Cirrus EP93xx keypad
From: Nikita Shubin via B4 Relay @ 2024-03-26 9:18 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexander Sverdlin
Cc: linux-input, devicetree, linux-kernel, Arnd Bergmann,
Krzysztof Kozlowski
In-Reply-To: <20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me>
From: Nikita Shubin <nikita.shubin@maquefel.me>
Add YAML bindings for ep93xx SoC keypad.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
.../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
new file mode 100644
index 000000000000..a0d2460c55ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,ep9307-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus ep93xx keypad
+
+maintainers:
+ - Alexander Sverdlin <alexander.sverdlin@gmail.com>
+
+allOf:
+ - $ref: /schemas/input/matrix-keymap.yaml#
+
+description:
+ The KPP is designed to interface with a keypad matrix with 2-point contact
+ or 3-point contact keys. The KPP is designed to simplify the software task
+ of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
+ and decoding one or multiple keys pressed simultaneously on a keypad.
+
+properties:
+ compatible:
+ oneOf:
+ - const: cirrus,ep9307-keypad
+ - items:
+ - enum:
+ - cirrus,ep9312-keypad
+ - cirrus,ep9315-keypad
+ - const: cirrus,ep9307-keypad
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ debounce-delay-ms:
+ description: |
+ Time in microseconds that key must be pressed or
+ released for state change interrupt to trigger.
+
+ cirrus,prescale:
+ description: row/column counter pre-scaler load value
+ $ref: /schemas/types.yaml#/definitions/uint16
+ maximum: 1023
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/input/input.h>
+ #include <dt-bindings/clock/cirrus,ep9301-syscon.h>
+ keypad@800f0000 {
+ compatible = "cirrus,ep9307-keypad";
+ reg = <0x800f0000 0x0c>;
+ interrupt-parent = <&vic0>;
+ interrupts = <29>;
+ clocks = <&eclk EP93XX_CLK_KEYPAD>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&keypad_default_pins>;
+ linux,keymap = <KEY_UP>,
+ <KEY_DOWN>,
+ <KEY_VOLUMEDOWN>,
+ <KEY_HOME>,
+ <KEY_RIGHT>,
+ <KEY_LEFT>,
+ <KEY_ENTER>,
+ <KEY_VOLUMEUP>,
+ <KEY_F6>,
+ <KEY_F8>,
+ <KEY_F9>,
+ <KEY_F10>,
+ <KEY_F1>,
+ <KEY_F2>,
+ <KEY_F3>,
+ <KEY_POWER>;
+ };
--
2.41.0
^ permalink raw reply related
* [PATCH v9 00/38] ep93xx device tree conversion
From: Nikita Shubin via B4 Relay @ 2024-03-26 9:18 UTC (permalink / raw)
To: Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nikita Shubin, Vinod Koul, Wim Van Sebroeck, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Ralf Baechle, Wu, Aaron, Lee Jones,
Olof Johansson, Niklas Cassel
Cc: linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, linux-pm,
devicetree, dmaengine, linux-watchdog, linux-pwm, linux-spi,
netdev, linux-mtd, linux-ide, linux-input, linux-sound,
Arnd Bergmann, Bartosz Golaszewski, Krzysztof Kozlowski,
Andy Shevchenko, Andrew Lunn, Andy Shevchenko
The goal is to recieve ACKs for all patches in series to merge it via Arnd branch.
Some changes since last version (v8):
- Most important, fixed bug in Device Tree resulting in CS4271 not working by Alexander Sverdlin.
- added #interrupt-cells to gpio nodes with interrupts-controller
- fixed some EOF in dtsi files
- fixed identation and type in ep93xx-keypad thanks to Andy Shevchenko
Stephen Boyd, Vinod Koul PLEASE! give some comments on following, couse i hadn't one for a couple of iterations already:
Following patches require attention from Stephen Boyd, as they were converted to aux_dev as suggested:
- ARM: ep93xx: add regmap aux_dev
- clk: ep93xx: add DT support for Cirrus EP93xx
Following patches require attention from Vinod Koul:
- dma: cirrus: Convert to DT for Cirrus EP93xx
- dma: cirrus: remove platform code
Patches should be formated with '--histogram'
---
Changes in v9:
ARM: dts: add Cirrus EP93XX SoC .dtsi
- added #interrupt-cells to gpio nodes with interrupts-controller
- fixed EOF
ARM: dts: ep93xx: Add EDB9302 DT
- Alexander Sverdlin: fixed bug in Device Tree resulting in CS4271 not working
input: keypad: ep93xx: add DT support for Cirrus EP93xx
- fixed identation and type
- Link to v8: https://lore.kernel.org/r/20240226-ep93xx-v8-0-3136dca7238f@maquefel.me/
Changes in v8:
soc: Add SoC driver for Cirrus ep93xx
- fixed freeing adev instead of rdev
- use __free() and no_free_ptr() for rdev allocation
- s/of_device_get_match_data()/device_get_match_data()/
ata: pata_ep93xx: add device tree support
- more appropriate usage of dev_err_probe()
pinctrl: add a Cirrus ep93xx SoC pin controller
- 8 per row in ide_9312_pins
mtd: rawnand: add support for ts72xx
- fwnode_handle_put() for fwnode in ts72xx_nand_remove()
- Link to v7: https://lore.kernel.org/r/20240118-ep93xx-v7-0-d953846ae771@maquefel.me
Changes in v7:
mtd: rawnand: add support for ts72xx
- fixed KConfig description
ARM: ep93xx: Add terminator to gpiod_lookup_table
- + Reported-by, Fixes
ARM: ep93xx: add regmap aux_dev
- + trailing comma
- - #include <linux/spinlock.h>
clk: ep93xx: add DT support for Cirrus EP93xx
- dropped unused defines
- return from default in ep93xx_mux_get_parent()
- use guard() in ep93xx_mux_set_parent_lock()
- <math.h> header for abs_diff()
- fixed comments
pinctrl: add a Cirrus ep93xx SoC pin controller
- dropped comments for DEVCFG defines
- <linux/array_size.h> for ARRAY_SIZE()
- + default in ep93xx_get_group_name()
- correct cast for id->driver_data
- s/device_set_of_node_from_dev()/device_set_node()/
power: reset: Add a driver for the ep93xx reset
- Add <linux/container_of.h>, <linux/errno.h>, <linux/slab.h>
- Add <linux/module.h>, <linux/mod_devicetable.h>
- Remove <platform_device.h>
spi: ep93xx: add DT support for Cirrus EP93xx
- Replace with ret = dev_err_probe(...);
ata: pata_ep93xx: add device tree support
- fixed wrong rebase with some partes leaked in "ata: pata_ep93xx: remove legacy pinctrl use"
- fix dma_request_chan() error processing
dma: cirrus: Convert to DT for Cirrus EP93xx
- fixed commit message (dropped explicit "only")
- fixed clk_get() processing to defer probe and log spamming
- refactor ep93xx_m2p_dma_filter()
- dropped blank line in ep93xx_m2p_dma_of_xlate()
- refactor ep93xx_m2m_dma_of_xlate()
dma: cirrus: remove platform code
- s/dma/DMA/ in commit message
soc: Add SoC driver for Cirrus ep93xx
- add period
- use cleanup and guard() for spinlocking
- correct cast for device_get_match_data()
- dropped dev_info() with SoC revision - i can't find it anywhere since 2.6 :/,
don't know why i was so sured that ep93xx always printed that
ata: pata_ep93xx: remove legacy pinctrl use
- made error handling in DMA as Uwe suggested
- Link to v6: https://lore.kernel.org/r/20231212-ep93xx-v6-0-c307b8ac9aa8@maquefel.me
Changes in v6:
- clk: ep93xx: add DT support for Cirrus EP93xx
- s/spin_lock_irqsave()/guard()/
- refactor index check in ep93xx_mux_set_parent_lock() to something more readable
- use in_range in ep93xx_mux_set_parent_lock()/ep93xx_ddiv_set_rate()
- use GENMASK() in ep93xx_ddiv_recalc_rate()
- comment reserved bit in ep93xx_ddiv_set_rate()
- move out from loop ClkDiv value assigment
- some style fixes
Andy, i was i asked to set index of XTALI explicitly, i am not setting ddiv_pdata
there becouse only XTALI is jnown in advance, and i think setting them in one place is more convenient.
- pinctrl: add a Cirrus ep93xx SoC pin controller
- drop OF from Kconfig
- droped linux/of.h include
- add space to */ where it is applicable
- add coma in multiline assigment
- "return NULL" as default case in ep93xx_get_group_name()
- fixed casting id->driver_data
- use device_set_of_node_from_dev()
- use dev_err_probe()
- power: reset: Add a driver for the ep93xx reset
- drop linux/of.h include
- soc: Add SoC driver for Cirrus ep93xx
- s/GPL-2.0/GPL-2.0-only/
- drop linux/kernel.h include
- + blank line before linux/soc/cirrus/ep93xx.h
- + blank line after ep93xx_get_soc_rev()
- + coma for pinctrl_names
- valid casting to int for of_device_get_match_data() return value
- mtd: rawnand: add support for ts72xx
- return as part of switch case
- s/iowrite8/iowrite8_rep/
- net: cirrus: add DT support for Cirrus EP93xx
- fix header sorting
- dma: cirrus: Convert to DT for Cirrus EP93xx
- use devm_clk_get
- use is_slave_direction
Changes in v5:
- gpio: ep93xx: split device in multiple
- ordered headers
- use irqd_to_hwirq()
- s/platform_get_irq()/platform_get_irq_optional()/
- [PATCH v4 02/42] ARM: ep93xx: add swlocked prototypes
- replaced with ARM: ep93xx: add regmap aux_dev
- [PATCH v4 03/42] dt-bindings: clock: Add Cirrus EP93xx
- fixed identation
- removed EP93XX_CLK_END
- and dropped it
- clock bindings moved to syscon with renaming to cirrus,ep9301-syscon.h
- clk: ep93xx: add DT support for Cirrus EP93xx
- convert to auxiliary and use parent device tree node
- moved all clocks except XTALI here
- used devm version everywhere and *_parent_hw() instead of passing name where it's possible
- unfortunately devm_clk_hw_register_fixed_rate doesn't have a parent index version
- [PATCH v4 05/42] dt-bindings: pinctrl: Add Cirrus EP93xx
- "unevaluatedProperties: false" for pins
- returned "additionalProperties: false" where it was
- and dropped it
- pinctrl: add a Cirrus ep93xx SoC pin controller
- sorted includes
- convert to auxiliary and use parent device tree node
- power: reset: Add a driver for the ep93xx reset
- convert to auxiliary device
- dt-bindings: soc: Add Cirrus EP93xx
- dropped all ref to reboot, clk, pinctrl subnodes
- added pins, as it's now used for pinctrl
- added #clock-cells, as it's now used for clk
- dt-bindings: pwm: Add Cirrus EP93xx
- $ref to pwm.yaml
- fixed 'pwm-cells'
- s/additionalProperties/unevaluatedProperties/
- soc: Add SoC driver for Cirrus ep93xx
- removed clocks, they are moved to clk auxiliary driver, as we dropped the clk dt node
- removed all swlocked exported functions
- dropped static spinlock
- added instantiating auxiliary reboot, clk, pinctrl
- dt-bindings: spi: Add Cirrus EP93xx
- Document DMA support
- spi: ep93xx: add DT support for Cirrus EP93xx
- dropped CONFIG_OF and SPI/DMA platform data entirely
- s/master/host/
- reworked DMA setup so we can use probe defer
- dt-bindings: dma: Add Cirrus EP93xx
- dropped bindings header (moved ports description to YAML)
- changed '#dma-cells' to 2, we use port, direction in cells so we can drop platform code completely
- dma: cirrus: add DT support for Cirrus EP93xx
- dropped platform probing completely
- dropped struct ep93xx_dma_data replaced with internal struct ep93xx_dma_chan_cfg with port/direction
- added xlate functions for m2m/m2p
- we require filters to set dma_cfg before hw_setup
- dt-bindings: ata: Add Cirrus EP93xx
- Document DMA support
- ata: pata_ep93xx: add device tree support
- drop DMA platform header with data
- use DMA OF so we can defer probing until DMA is up
- ARM: dts: add Cirrus EP93XX SoC .dtsi
- ARM: dts: ep93xx: add ts7250 board
- ARM: dts: ep93xx: Add EDB9302 DT
- replaced "eclk: clock-controller" to syscon reference
- replaced "pinctrl: pinctrl" to syscon reference
- gpios are now "enabled" by default
- reworked i2s node
- change all dma nodes and refs
- new additions to I2S
- Document DMA
- Document Audio Port usage
- drop legacy DMA support
- Link to v4: https://lore.kernel.org/r/20230915-ep93xx-v4-0-a1d779dcec10@maquefel.me
Changes in v4:
- gpio: ep93xx: split device in multiple
- s/generic_handle_irq/generic_handle_domain_irq/
- s/int offset/irq_hw_number_t offset/ though now it looks a bit odd to me
- drop i = 0
- drop 'error'
- use dev_err_probe withour printing devname once again
dt-bindings: clock: Add Cirrus EP93xx
- renamed cirrus,ep93xx-clock.h -> cirrus,ep9301-clk.h
clk: ep93xx: add DT support for Cirrus EP93xx
- drop unused includes
- use .name only for xtali, pll1, pll2 parents
- convert // to /*
- pass clk_parent_data instead of char* clock name
dt-bindings: pinctrl: Add Cirrus EP93xx
- s/additionalProperties/unevaluatedProperties/
dt-bindings: soc: Add Cirrus EP93xx
- move syscon to soc directory
- add vendor prefix
- make reboot same style as pinctrl, clk
- use absolute path for ref
- expand example
soc: Add SoC driver for Cirrus ep93xx
- s/0xf0000000/GENMASK(31, 28)/
- s/ret/ep93xx_chip_revision(map)/
- drop symbol exports
- convert to platform driver
dt-bindings: rtc: Add Cirrus EP93xx
- allOf: with $ref to rtc.yaml
- s/additionalProperties/unevaluatedProperties/
dt-bindings: watchdog: Add Cirrus EP93x
- drop description
- reword
power: reset: Add a driver for the ep93xx reset
- lets use 'GPL-2.0+' instead of '(GPL-2.0)'
- s/of_device/of/
- drop mdelay with warning
- return 0 at the end
net: cirrus: add DT support for Cirrus EP93xx
- fix leaking np
mtd: nand: add support for ts72xx
- +bits.h
- drop comment
- ok to fwnode_get_next_child_node
- use goto to put handle and nand and report error
ARM: dts: add Cirrus EP93XX SoC .dtsi
- add simple-bus for ebi, as we don't require to setup anything
- add arm,pl011 compatible to uart nodes
- drop i2c-gpio, as it's isn't used anywhere
ARM: dts: ep93xx: add ts7250 board
- generic node name for temperature-sensor
- drop i2c
- move nand, rtc, watchdog to ebi node
- Link to v3: https://lore.kernel.org/r/20230605-ep93xx-v3-0-3d63a5f1103e@maquefel.me
---
Alexander Sverdlin (3):
ASoC: ep93xx: Drop legacy DMA support
ARM: dts: ep93xx: Add EDB9302 DT
ASoC: cirrus: edb93xx: Delete driver
Nikita Shubin (35):
gpio: ep93xx: split device in multiple
ARM: ep93xx: add regmap aux_dev
clk: ep93xx: add DT support for Cirrus EP93xx
pinctrl: add a Cirrus ep93xx SoC pin controller
power: reset: Add a driver for the ep93xx reset
dt-bindings: soc: Add Cirrus EP93xx
soc: Add SoC driver for Cirrus ep93xx
dt-bindings: dma: Add Cirrus EP93xx
dma: cirrus: Convert to DT for Cirrus EP93xx
dt-bindings: watchdog: Add Cirrus EP93x
watchdog: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: pwm: Add Cirrus EP93xx
pwm: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: spi: Add Cirrus EP93xx
spi: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: net: Add Cirrus EP93xx
net: cirrus: add DT support for Cirrus EP93xx
dt-bindings: mtd: Add ts7200 nand-controller
mtd: rawnand: add support for ts72xx
dt-bindings: ata: Add Cirrus EP93xx
ata: pata_ep93xx: add device tree support
dt-bindings: input: Add Cirrus EP93xx keypad
input: keypad: ep93xx: add DT support for Cirrus EP93xx
wdt: ts72xx: add DT support for ts72xx
gpio: ep93xx: add DT support for gpio-ep93xx
ASoC: dt-bindings: ep93xx: Document DMA support
ASoC: dt-bindings: ep93xx: Document Audio Port support
ARM: dts: add Cirrus EP93XX SoC .dtsi
ARM: dts: ep93xx: add ts7250 board
ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
pwm: ep93xx: drop legacy pinctrl
ata: pata_ep93xx: remove legacy pinctrl use
ARM: ep93xx: delete all boardfiles
ARM: ep93xx: soc: drop defines
dma: cirrus: remove platform code
.../bindings/arm/cirrus/cirrus,ep9301.yaml | 38 +
.../bindings/ata/cirrus,ep9312-pata.yaml | 42 +
.../bindings/dma/cirrus,ep9301-dma-m2m.yaml | 84 ++
.../bindings/dma/cirrus,ep9301-dma-m2p.yaml | 144 ++
.../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++
.../devicetree/bindings/mtd/technologic,nand.yaml | 45 +
.../devicetree/bindings/net/cirrus,ep9301-eth.yaml | 59 +
.../devicetree/bindings/pwm/cirrus,ep9301-pwm.yaml | 53 +
.../bindings/soc/cirrus/cirrus,ep9301-syscon.yaml | 94 ++
.../bindings/sound/cirrus,ep9301-i2s.yaml | 16 +
.../devicetree/bindings/spi/cirrus,ep9301-spi.yaml | 70 +
.../bindings/watchdog/cirrus,ep9301-wdt.yaml | 42 +
arch/arm/Makefile | 1 -
arch/arm/boot/dts/cirrus/Makefile | 4 +
arch/arm/boot/dts/cirrus/ep93xx-bk3.dts | 125 ++
arch/arm/boot/dts/cirrus/ep93xx-edb9302.dts | 181 +++
arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts | 145 ++
arch/arm/boot/dts/cirrus/ep93xx.dtsi | 444 ++++++
arch/arm/mach-ep93xx/Kconfig | 20 +-
arch/arm/mach-ep93xx/Makefile | 11 -
arch/arm/mach-ep93xx/clock.c | 733 ----------
arch/arm/mach-ep93xx/core.c | 1018 --------------
arch/arm/mach-ep93xx/dma.c | 114 --
arch/arm/mach-ep93xx/edb93xx.c | 368 -----
arch/arm/mach-ep93xx/ep93xx-regs.h | 38 -
arch/arm/mach-ep93xx/gpio-ep93xx.h | 111 --
arch/arm/mach-ep93xx/hardware.h | 25 -
arch/arm/mach-ep93xx/irqs.h | 76 --
arch/arm/mach-ep93xx/platform.h | 42 -
arch/arm/mach-ep93xx/soc.h | 212 ---
arch/arm/mach-ep93xx/timer-ep93xx.c | 143 --
arch/arm/mach-ep93xx/ts72xx.c | 422 ------
arch/arm/mach-ep93xx/ts72xx.h | 94 --
arch/arm/mach-ep93xx/vision_ep9307.c | 321 -----
drivers/ata/pata_ep93xx.c | 107 +-
drivers/clk/Kconfig | 8 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ep93xx.c | 840 ++++++++++++
drivers/dma/ep93xx_dma.c | 287 +++-
drivers/gpio/gpio-ep93xx.c | 345 ++---
drivers/input/keyboard/ep93xx_keypad.c | 74 +-
drivers/mtd/nand/raw/Kconfig | 6 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/technologic-nand-controller.c | 222 +++
drivers/net/ethernet/cirrus/ep93xx_eth.c | 63 +-
drivers/pinctrl/Kconfig | 7 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-ep93xx.c | 1434 ++++++++++++++++++++
drivers/power/reset/Kconfig | 10 +
drivers/power/reset/Makefile | 1 +
drivers/power/reset/ep93xx-restart.c | 84 ++
drivers/pwm/pwm-ep93xx.c | 26 +-
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/cirrus/Kconfig | 13 +
drivers/soc/cirrus/Makefile | 2 +
drivers/soc/cirrus/soc-ep93xx.c | 240 ++++
drivers/spi/spi-ep93xx.c | 66 +-
drivers/watchdog/ep93xx_wdt.c | 8 +
drivers/watchdog/ts72xx_wdt.c | 8 +
include/dt-bindings/clock/cirrus,ep9301-syscon.h | 46 +
include/linux/platform_data/dma-ep93xx.h | 94 --
include/linux/platform_data/eth-ep93xx.h | 10 -
include/linux/platform_data/keypad-ep93xx.h | 32 -
include/linux/platform_data/spi-ep93xx.h | 15 -
include/linux/soc/cirrus/ep93xx.h | 47 +-
sound/soc/cirrus/Kconfig | 9 -
sound/soc/cirrus/Makefile | 4 -
sound/soc/cirrus/edb93xx.c | 116 --
sound/soc/cirrus/ep93xx-i2s.c | 19 -
sound/soc/cirrus/ep93xx-pcm.c | 19 +-
71 files changed, 5139 insertions(+), 4550 deletions(-)
---
base-commit: 70293240c5ce675a67bfc48f419b093023b862b3
change-id: 20230605-ep93xx-01c76317e2d2
Best regards,
--
Nikita Shubin <nikita.shubin@maquefel.me>
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition
From: Łukasz Majczak @ 2024-03-26 8:58 UTC (permalink / raw)
To: Doug Anderson
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Hans de Goede,
Maxime Ripard, Kai-Heng Feng, Johan Hovold, linux-input,
linux-kernel, Radoslaw Biernacki
In-Reply-To: <CAD=FV=VHps2d4dqLXCqE=hNMbk4pxeN607nFA0nEkd7chNAr3A@mail.gmail.com>
> nit: checkpatch should have yelled at you saying that you should
> specify a commit in the format:
>
> commit b3a81b6c4fc6 ("HID: i2c-hid: check if device is there before
> really probing")
>
I will do it, but I did run the checkpatch (with --strict option) and
it didn't complain about anything.
>
> nit: I believe your sign off should be last. It's also unclear why two
> signoffs. Did Radoslaw author it and you changed it? ...or was it
> Co-Developed-by, or ...? You'll probably need to adjust your tags a
> bit depending on the answers.
>
Yes, we've discussed this patch together and the original
investigation was done by Rad.
> Having both ends of the usleep be 400 is iffy. In this case it's at
> probe time so I wonder if udelay() is better? If not, maybe give at
> least _some_ margin?
>
>
> > + } while (tries-- > 0 && ret < 0);
>
According to Documentation/timers/timers-howto.rst:
" SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range"
It was also pointed out by checkpath (when I initially used msleep).
I think giving some margin (eg. 400,500) would be ok.
> I'm not a huge fan of having to check "tries" and "ret" twice.
> Personally I'd rather see a "while (true)" loop and test the condition
> once to break out. AKA:
>
> while (true) {
> ret = i2c_...
> tries--;
> if (tries == 0 || ret >= 0)
> break;
> udelay(400);
> }
>
> ...if you feel very strongly about the way you have coded it, though,
> I won't stand in your way.
I don't have emotional bond to this code ;), thanks.
>
> Pretty much all my comments are just nits and, since I'm not the
> maintainer here, my opinion is just an opinion. I'd wait at least a
> little while for the maintainers to comment before posting v2. I'm
> happy to give a Reviewed-by tag when some of the nits are fixed.
>
> -Doug
Thank you Doug for all your input.
Best regards,
Lukasz
^ permalink raw reply
* Re: [PATCH v1 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Conor Dooley @ 2024-03-26 8:48 UTC (permalink / raw)
To: Allen Lin
Cc: Conor Dooley, Rob Herring, dmitry.torokhov,
krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires,
linux-input, devicetree, linux-kernel
In-Reply-To: <TY0PR06MB56110ADEA805B68BE2B887069E352@TY0PR06MB5611.apcprd06.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]
On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> >
> > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > touchscreen controller.
> > > > >
> > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > >
> > > > note to self/Krzysztof/Rob:
> > > > There was a previous attempt at this kind of device. This version looks
> > > > better but might be incomplete given there's a bunch more properties in
> > > > that patchset:
> > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > >
> > > Those don't look like properties we want coming back.
> >
> > Oh, I don't want most of them coming back either. There are some
> > supplies in there though that I think we would like to come back, no?
> > Maybe this particular device doesn't have any supplies, but that doesn't
> > really seem credible.
>
> We will use Firmware-name in Device Tree.
> For power supply settings, because there may be other device using
> same regulator.
If there are other devices using the same regulator is it more
important that you document it so that it doesn't get disabled by the
other users.
> We plan to define it as an optional property for user to control in
> next release.
I don't see how the regulator would not be required, the device doesn't
function without power.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: In kernel virtual HID devices (was Future handling of complex RGB devices on Linux v3)
From: Werner Sembach @ 2024-03-26 7:57 UTC (permalink / raw)
To: Hans de Goede, Benjamin Tissoires
Cc: Lee Jones, jikos, linux-kernel, Jelle van der Waa, Miguel Ojeda,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Pavel Machek, Gregor Riepl
In-Reply-To: <fcf4dd53-f461-4c2e-8fbe-50b50e4e6797@redhat.com>
Hi all,
Am 25.03.24 um 19:30 schrieb Hans de Goede:
[snip]
>>> If the kernel already handles the custom protocol into generic HID, the
>>> work for userspace is not too hard because they can deal with a known
>>> protocol and can be cross-platform in their implementation.
>>>
>>> I'm mentioning that cross-platform because SDL used to rely on the
>>> input, LEDs, and other Linux peculiarities and eventually fell back on
>>> using hidraw only because it's way more easier that way.
>>>
>>> The other advantage of LampArray is that according to Microsoft's
>>> document, new devices are going to support it out of the box, so they'll
>>> be supported out of the box directly.
>>>
>>> Most of the time my stance is "do not add new kernel API, you'll regret
>>> it later". So in that case, given that we have a formally approved
>>> standard, I would suggest to use it, and consider it your API.
>> The only new UAPI would be the use_leds_uapi switch to turn on/off the backwards compatibility.
> Actually we don't even need that. Typically there is a single HID
> driver handling both keys and the backlight, so userspace cannot
> just unbind the HID driver since then the keys stop working.
>
> But with a virtual LampArray HID device the only functionality
> for an in kernel HID driver would be to export a basic keyboard
> backlight control interface for simple non per key backlight control
> to integrate nicely with e.g. GNOME's backlight control.
Don't forget that in the future there will be devices that natively support
LampArray in their firmware, so for them it is the same device.
Regards,
Werner
> And then when OpenRGB wants to take over it can just unbind the HID
> driver from the HID device using existing mechanisms for that.
>
> Hmm, I wonder if that will not also kill hidraw support though ...
> I guess getting hidraw support back might require then also manually
> binding the default HID input driver. Bentiss any input on this?
>
> Background info: as discussed earlier in the thread Werner would like
> to have a basic driver registering a /sys/class/leds/foo::kbd_backlight/
> device, since those are automatically supported by GNOME (and others)
> and will give basic kbd backlight brightness control in the desktop
> environment. This could be a simple HID driver for
> the hid_allocate_device()-ed virtual HID device, but userspace needs
> to be able to move that out of the way when it wants to take over
> full control of the per key lighting.
>
> Regards,
>
> Hans
>
>
>
>
>
>
>
>> The control flow for the whole system would look something like this:
>>
>> - System boots
>>
>> - Kernel driver initializes keyboard (maybe stops rainbowpuke boot effects, sets brightness to a default value, or initializes a solid color)
>>
>> - systemd-backlight restores last keyboard backlight brightness
>>
>> - UPower sees sysfs leds entry and exposes it to DBus for DEs to do keyboard brightness handling
>>
>> - If the user wants more control they (auto-)start OpenRGB
>>
>> - OpenRGB disables sysfs leds entry via use_leds_uapi to prevent double control of the same device by UPower
>>
>> - OpenRGB directly interacts with hidraw device via LampArray API to give fine granular control of the backlight
>>
>> - When OpenRGB closes it should reenable the sysfs leds entry
>>
>> - System shutdown
>>
>> - Since OpenRGB reenables the sysfs leds entry, systemd-backlight can correctly store a brightness value for next boot
>>
>> Regards,
>>
>> Werner
>>
>>> Side note to self: I really need to resurrect the hidraw revoke series
>>> so we could export those hidraw node to userspace with uaccess through
>>> logind...
>>>
>>> Cheers,
>>> Benjamin
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: touchscreen: edt-ft5x06: Add ft5426
From: Krzysztof Kozlowski @ 2024-03-26 7:36 UTC (permalink / raw)
To: Andreas Kemnade, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, o.rempel, dario.binacchi, u.kleine-koenig, hdegoede,
p.puschmann, linux-input, devicetree, linux-kernel,
caleb.connolly
In-Reply-To: <20240325192531.256854-1-andreas@kemnade.info>
On 25/03/2024 20:25, Andreas Kemnade wrote:
> Add compatible for ft5426.
> Searches for documentation reveal neither edt nor evervision
> as some related company, only FocalTech.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> index f2808cb4d99df..17b05522eacd9 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> @@ -40,6 +40,7 @@ properties:
> - edt,edt-ft5506
> - evervision,ev-ft5726
> - focaltech,ft6236
> + - focaltech,ft5426
Please keep the order, so one line bump.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Allen Lin @ 2024-03-26 5:46 UTC (permalink / raw)
To: Conor Dooley
Cc: Rob Herring, dmitry.torokhov, krzysztof.kozlowski+dt, conor+dt,
jikos, benjamin.tissoires, linux-input, devicetree, linux-kernel
In-Reply-To: <20240322-rectified-udder-fef9102f58da@spud>
Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
>
> On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > It's power sequence should be bound with a lcm driver, thus it
> > > > needs to be a panel follower. Others are the same as normal SPI
> > > > touchscreen controller.
> > > >
> > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > >
> > > note to self/Krzysztof/Rob:
> > > There was a previous attempt at this kind of device. This version looks
> > > better but might be incomplete given there's a bunch more properties in
> > > that patchset:
> > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> >
> > Those don't look like properties we want coming back.
>
> Oh, I don't want most of them coming back either. There are some
> supplies in there though that I think we would like to come back, no?
> Maybe this particular device doesn't have any supplies, but that doesn't
> really seem credible.
We will use Firmware-name in Device Tree.
For power supply settings, because there may be other device using
same regulator.
We plan to define it as an optional property for user to control in
next release.
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition
From: Doug Anderson @ 2024-03-25 23:12 UTC (permalink / raw)
To: Lukasz Majczak
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Hans de Goede,
Maxime Ripard, Kai-Heng Feng, Johan Hovold, linux-input,
linux-kernel, Radoslaw Biernacki
In-Reply-To: <20240325105452.529921-1-lma@chromium.org>
Hi,
On Mon, Mar 25, 2024 at 3:55 AM Lukasz Majczak <lma@chromium.org> wrote:
>
> This patch extends the early bailout for probing procedure introduced in
> commit: b3a81b6c4fc6730ac49e20d789a93c0faabafc98, in order to cover devices
nit: checkpatch should have yelled at you saying that you should
specify a commit in the format:
commit b3a81b6c4fc6 ("HID: i2c-hid: check if device is there before
really probing")
Please fix and make sure that you're running checkpatch.
> based on STM microcontrollers. For touchpads based on STM uC,
> the probe sequence needs to take into account the increased response time
> for i2c transaction if the device already entered a deep power state.
> STM specify the wakeup time as 400us between positive strobe of
> the clock line. Deep sleep is controlled by Touchpad FW,
> though some devices enter it pretty early to conserve power
> in case of lack of activity on the i2c bus.
> Failing to follow this requirement will result in touchpad device not being
> detected due initial transaction being dropped by STM i2c slave controller.
> By adding additional try, first transaction will wake up the touchpad
> STM controller, and the second will produce proper detection response.
>
> Signed-off-by: Lukasz Majczak <lma@chromium.org>
> Signed-off-by: Radoslaw Biernacki <rad@chromium.org>
nit: I believe your sign off should be last. It's also unclear why two
signoffs. Did Radoslaw author it and you changed it? ...or was it
Co-Developed-by, or ...? You'll probably need to adjust your tags a
bit depending on the answers.
> ---
> drivers/hid/i2c-hid/i2c-hid-core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
Overall comment on this patch: I know that this is based on a patch
we've been carrying downstream in ChromeOS for years and some folks
considered it not upstreamable because it's too hacky. My $0.02 is
that, while it's ugly, this is needed to support real hardware that
was shipped. There's zero chance we can fix the hardware, a .00001%
chance that we could convince someone to update the firmware on the
i2c-hid device, and a .01% chance that we could convince people to try
to figure out a workaround by adding something to the main AP firmware
on this device. As I understand it, nobody has come up with a better
kernel workaround than this patch.
To me it doesn't seem terrible to have this retry here and it's not a
huge penalty for other i2c-hid users. ...so personally I'm in favor of
the idea of this landing. If I was a consumer and had one of the
affected Chromebooks I'd personally rather upstream have the support
for it.
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 2df1ab3c31cc..69af0fad4f41 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1013,9 +1013,15 @@ static int __i2c_hid_core_probe(struct i2c_hid *ihid)
> struct i2c_client *client = ihid->client;
> struct hid_device *hid = ihid->hid;
> int ret;
> + int tries = 2;
> +
> + do {
> + /* Make sure there is something at this address */
> + ret = i2c_smbus_read_byte(client);
> + if (tries > 0 && ret < 0)
> + usleep_range(400, 400);
Having both ends of the usleep be 400 is iffy. In this case it's at
probe time so I wonder if udelay() is better? If not, maybe give at
least _some_ margin?
> + } while (tries-- > 0 && ret < 0);
I'm not a huge fan of having to check "tries" and "ret" twice.
Personally I'd rather see a "while (true)" loop and test the condition
once to break out. AKA:
while (true) {
ret = i2c_...
tries--;
if (tries == 0 || ret >= 0)
break;
udelay(400);
}
...if you feel very strongly about the way you have coded it, though,
I won't stand in your way.
Pretty much all my comments are just nits and, since I'm not the
maintainer here, my opinion is just an opinion. I'd wait at least a
little while for the maintainers to comment before posting v2. I'm
happy to give a Reviewed-by tag when some of the nits are fixed.
-Doug
^ permalink raw reply
* [PATCH 1/2] dt-bindings: input: touchscreen: edt-ft5x06: Add ft5426
From: Andreas Kemnade @ 2024-03-25 19:25 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
andreas, o.rempel, dario.binacchi, u.kleine-koenig, hdegoede,
p.puschmann, linux-input, devicetree, linux-kernel,
caleb.connolly
In-Reply-To: <20240324184415.206587-1-andreas@kemnade.info>
Add compatible for ft5426.
Searches for documentation reveal neither edt nor evervision
as some related company, only FocalTech.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
.../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
index f2808cb4d99df..17b05522eacd9 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
@@ -40,6 +40,7 @@ properties:
- edt,edt-ft5506
- evervision,ev-ft5726
- focaltech,ft6236
+ - focaltech,ft5426
reg:
maxItems: 1
--
2.39.2
^ permalink raw reply related
* Re: [PATCH 0/2] Input: add ft5426
From: Conor Dooley @ 2024-03-25 19:16 UTC (permalink / raw)
To: Andreas Kemnade
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
o.rempel, dario.binacchi, u.kleine-koenig, hdegoede, p.puschmann,
linux-input, devicetree, linux-kernel, caleb.connolly
In-Reply-To: <20240324184415.206587-1-andreas@kemnade.info>
[-- Attachment #1: Type: text/plain, Size: 411 bytes --]
On Sun, Mar 24, 2024 at 07:44:13PM +0100, Andreas Kemnade wrote:
> Add ft5426 touchscreen controller and the corresponding compatible.
>
> Andreas Kemnade (2):
> dt-bindings: input: touchscreen: edt-ft5x06: Add ft5426
> Input: edt-ft5x06 - add ft5426
I think something got messed up, I only got the cover and 2/2, no 1/2:
https://lore.kernel.org/all/20240324184415.206587-1-andreas@kemnade.info/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: In kernel virtual HID devices (was Future handling of complex RGB devices on Linux v3)
From: Miguel Ojeda @ 2024-03-25 18:38 UTC (permalink / raw)
To: Hans de Goede
Cc: Werner Sembach, Lee Jones, linux-kernel, Jelle van der Waa,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Pavel Machek, Gregor Riepl, Benjamin Tissoires, Jiri Kosina,
Andy Shevchenko, Geert Uytterhoeven
In-Reply-To: <aec1d22d-9e59-4dfc-b108-5ba339b0e76a@redhat.com>
On Mon, Mar 25, 2024 at 3:25 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> +Cc: Bentiss, Jiri
Cc'ing Andy and Geert as well who recently became the
maintainers/reviewers of auxdisplay, in case they are interested in
these threads (one of the initial solutions discussed in a past thread
a while ago was to extend auxdisplay).
Cheers,
Miguel
^ permalink raw reply
* Re: In kernel virtual HID devices (was Future handling of complex RGB devices on Linux v3)
From: Hans de Goede @ 2024-03-25 18:30 UTC (permalink / raw)
To: Werner Sembach, Benjamin Tissoires
Cc: Lee Jones, jikos, linux-kernel, Jelle van der Waa, Miguel Ojeda,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Pavel Machek, Gregor Riepl
In-Reply-To: <870cca8a-1a1b-4d17-874e-a26c30aca2bf@tuxedocomputers.com>
Hi Werner,
On 3/25/24 5:48 PM, Werner Sembach wrote:
> Hi Benjamin,
>
> Am 25.03.24 um 16:56 schrieb Benjamin Tissoires:
>> On Mar 25 2024, Hans de Goede wrote:
>>> +Cc: Bentiss, Jiri
>>>
>>> Hi Werner,
>>>
>>> On 3/20/24 12:16 PM, Werner Sembach wrote:
>>>> Hi Hans and the others,
>>>>
>>>> Am 22.02.24 um 14:14 schrieb Werner Sembach:
>>>>> Hi,
>>>>>
>>>>> Thanks everyone for the exhaustive feedback. And at least this thread is a good comprehesive reference for the future ^^.
>>>>>
>>>>> To recap the hopefully final UAPI for complex RGB lighting devices:
>>>>>
>>>>> - By default there is a singular /sys/class/leds/* entry that treats the device as if it was a single zone RGB keyboard backlight with no special effects.
>>>>>
>>>>> - There is an accompanying misc device with the sysfs attributes "name", "device_type", "firmware_version_string", "serial_number" for device identification and "use_leds_uapi" that defaults to 1.
>>>>>
>>>>> - If set to 0 the /sys/class/leds/* entry disappears. The driver should keep the last state the backlight was in active if possible.
>>>>>
>>>>> - If set 1 it appears again. The driver should bring it back to a static 1 zone setting while avoiding flicker if possible.
>>>>>
>>>>> - If the device is not controllable by for example hidraw, the misc device might also implement additional ioctls or sysfs attributes to allow a more complex low level control for the keyboard backlight. This is will be a highly vendor specific UAPI.
>>>> So in the OpenRGB issue thread https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/dynamic-lighting-devices aka HID LampArray was mentioned. I did dismiss it because I thought that is only relevant for firmware, but I now stumbled upon the Virtual HID Framework (VHF) https://learn.microsoft.com/en-us/windows-hardware/drivers/hid/virtual-hid-framework--vhf- and now I wonder if an equivalent exists for Linux? A quick search did not yield any results for me.
>>> Oh, interesting. I did not know about the HID LampArray API.
>>>
>>> About your question about virtual HID devices, there is uHID,
>>> but as the name suggests this allows userspace to emulate a HID
>>> device.
>>>
>>> In your case you want to do the emulation in kernel so that you
>>> can translate the proprietary WMI calls to something HID LampArray
>>> compatible.
>>>
>>> I guess you could do this by defining your own HID transport driver,
>>> like how e.g. the i2c-hid code defines 1 i2c-hid parent + 1 HID
>>> "client" for each device which talks HID over i2c in the machine.
>>>
>>> Bentiss, Jiri, do you have any input on this. Would something like
>>> that be acceptable to you (just based on the concept at least) ?
>> I just read the thread, and I think I now understand a little bit what
>> this request is :)
>>
>> IMO working with the HID LampArray is the way forward. So I would
>> suggest to convert any non-HID RGB "LED display" that we are talking
>> about as a HID LampArray device through `hid_allocate_device()` in the
>> kernel. Basically what you are suggesting Hans. It's just that you don't
>> need a formal transport layer, just a child device that happens to be
>> HID.
>>
>> The next question IMO is: do we want the kernel to handle such
>> machinery? Wouldn't it be simpler to just export the HID device and let
>> userspace talk to it through hidraw, like what OpenRGB does?
>
> That's already part of my plan: The kernels main goal is to give devices a LampArray interface that don't have one already (e.g. because they are no HID devices to begin with).
>
> The actual handling of LampArray will happen in userspace.
>
> Exception is that maybe it could be useful to implement a small subset of LampArray in a generic leds-subsystem driver for backwards compatibility to userspace applications that only implement that (e.g. UPower). It would treat the whole keyboard as a single led.
>
>>
>> If the kernel already handles the custom protocol into generic HID, the
>> work for userspace is not too hard because they can deal with a known
>> protocol and can be cross-platform in their implementation.
>>
>> I'm mentioning that cross-platform because SDL used to rely on the
>> input, LEDs, and other Linux peculiarities and eventually fell back on
>> using hidraw only because it's way more easier that way.
>>
>> The other advantage of LampArray is that according to Microsoft's
>> document, new devices are going to support it out of the box, so they'll
>> be supported out of the box directly.
>>
>> Most of the time my stance is "do not add new kernel API, you'll regret
>> it later". So in that case, given that we have a formally approved
>> standard, I would suggest to use it, and consider it your API.
>
> The only new UAPI would be the use_leds_uapi switch to turn on/off the backwards compatibility.
Actually we don't even need that. Typically there is a single HID
driver handling both keys and the backlight, so userspace cannot
just unbind the HID driver since then the keys stop working.
But with a virtual LampArray HID device the only functionality
for an in kernel HID driver would be to export a basic keyboard
backlight control interface for simple non per key backlight control
to integrate nicely with e.g. GNOME's backlight control.
And then when OpenRGB wants to take over it can just unbind the HID
driver from the HID device using existing mechanisms for that.
Hmm, I wonder if that will not also kill hidraw support though ...
I guess getting hidraw support back might require then also manually
binding the default HID input driver. Bentiss any input on this?
Background info: as discussed earlier in the thread Werner would like
to have a basic driver registering a /sys/class/leds/foo::kbd_backlight/
device, since those are automatically supported by GNOME (and others)
and will give basic kbd backlight brightness control in the desktop
environment. This could be a simple HID driver for
the hid_allocate_device()-ed virtual HID device, but userspace needs
to be able to move that out of the way when it wants to take over
full control of the per key lighting.
Regards,
Hans
>
> The control flow for the whole system would look something like this:
>
> - System boots
>
> - Kernel driver initializes keyboard (maybe stops rainbowpuke boot effects, sets brightness to a default value, or initializes a solid color)
>
> - systemd-backlight restores last keyboard backlight brightness
>
> - UPower sees sysfs leds entry and exposes it to DBus for DEs to do keyboard brightness handling
>
> - If the user wants more control they (auto-)start OpenRGB
>
> - OpenRGB disables sysfs leds entry via use_leds_uapi to prevent double control of the same device by UPower
>
> - OpenRGB directly interacts with hidraw device via LampArray API to give fine granular control of the backlight
>
> - When OpenRGB closes it should reenable the sysfs leds entry
>
> - System shutdown
>
> - Since OpenRGB reenables the sysfs leds entry, systemd-backlight can correctly store a brightness value for next boot
>
> Regards,
>
> Werner
>
>>
>> Side note to self: I really need to resurrect the hidraw revoke series
>> so we could export those hidraw node to userspace with uaccess through
>> logind...
>>
>> Cheers,
>> Benjamin
>
^ 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