* [PATCH v4 00/42] ep93xx device tree conversion
From: Nikita Shubin via B4 Relay @ 2023-09-15 8:10 UTC (permalink / raw)
To: Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Nikita Shubin,
Sebastian Reichel, Daniel Lezcano, Thomas Gleixner,
Alessandro Zummo, Alexandre Belloni, Wim Van Sebroeck,
Guenter Roeck, Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Vinod Koul, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, devicetree,
linux-pm, linux-rtc, linux-watchdog, linux-pwm, linux-spi, netdev,
dmaengine, linux-mtd, linux-ide, linux-input, alsa-devel,
Arnd Bergmann, Bartosz Golaszewski, Krzysztof Kozlowski,
Andy Shevchenko, Andrew Lunn
This series aims to convert ep93xx from platform to full device tree support.
The main goal is to receive ACK's to take it via Arnd's arm-soc branch.
Major changes:
- drop newline at the end from each YAML files
- rename dma and clk bindings headers to match first compatible
- shrink SoC exported functions number to only 2
- dropped some ep93xx_pata fixes from these series
- dropped m48t86 stuff from these series
Bit thanks to Andy Shevchenko for thorough review.
Some parts are were sent in advance, so these series depends on the
following:
Link: https://lore.kernel.org/all/20230823-m48t86_device_tree-v2-0-21ff275f949d@maquefel.me/
Link: https://lore.kernel.org/all/20230823-ep93xx_pata_fixes-v1-0-d7e7229be148@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 (2):
ARM: dts: ep93xx: Add EDB9302 DT
ASoC: cirrus: edb93xx: Delete driver
Nikita Shubin (40):
gpio: ep93xx: split device in multiple
ARM: ep93xx: add swlocked prototypes
dt-bindings: clock: Add Cirrus EP93xx
clk: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: pinctrl: Add Cirrus EP93xx
pinctrl: add a Cirrus ep93xx SoC pin controller
dt-bindings: power: reset: Add ep93xx reset
power: reset: Add a driver for the ep93xx reset
dt-bindings: soc: Add Cirrus EP93xx
soc: Add SoC driver for Cirrus ep93xx
dt-bindings: timers: Add Cirrus EP93xx
clocksource: ep93xx: Add driver for Cirrus Logic EP93xx
dt-bindings: rtc: Add Cirrus EP93xx
rtc: ep93xx: add DT support 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: dma: Add Cirrus EP93xx
dma: cirrus: add DT support for Cirrus EP93xx
dt-bindings: mtd: Add ts7200 nand-controller
mtd: nand: 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
dt-bindings: wdt: Add ts72xx
wdt: ts72xx: add DT support for ts72xx
gpio: ep93xx: add DT support for gpio-ep93xx
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
.../bindings/arm/cirrus/cirrus,ep9301.yaml | 38 +
.../bindings/ata/cirrus,ep9312-pata.yaml | 42 +
.../bindings/clock/cirrus,ep9301-clk.yaml | 46 +
.../bindings/dma/cirrus,ep9301-dma-m2m.yaml | 69 +
.../bindings/dma/cirrus,ep9301-dma-m2p.yaml | 121 ++
.../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++
.../devicetree/bindings/mtd/technologic,nand.yaml | 45 +
.../devicetree/bindings/net/cirrus,ep9301-eth.yaml | 59 +
.../bindings/pinctrl/cirrus,ep9301-pinctrl.yaml | 57 +
.../bindings/power/reset/cirrus,ep9301-reboot.yaml | 34 +
.../devicetree/bindings/pwm/cirrus,ep9301-pwm.yaml | 46 +
.../devicetree/bindings/rtc/cirrus,ep9301-rtc.yaml | 42 +
.../bindings/soc/cirrus/cirrus,ep9301-syscon.yaml | 71 +
.../devicetree/bindings/spi/cirrus,ep9301-spi.yaml | 61 +
.../bindings/timer/cirrus,ep9301-timer.yaml | 49 +
.../bindings/watchdog/cirrus,ep9301-wdt.yaml | 42 +
.../bindings/watchdog/technologic,ts7200-wdt.yaml | 45 +
arch/arm/Makefile | 1 -
arch/arm/boot/dts/cirrus/Makefile | 4 +
arch/arm/boot/dts/cirrus/ep93xx-bk3.dts | 124 ++
arch/arm/boot/dts/cirrus/ep93xx-edb9302.dts | 180 +++
arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts | 143 ++
arch/arm/boot/dts/cirrus/ep93xx.dtsi | 454 +++++++
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 | 1017 --------------
arch/arm/mach-ep93xx/dma.c | 114 --
arch/arm/mach-ep93xx/edb93xx.c | 344 -----
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/ts72xx.c | 422 ------
arch/arm/mach-ep93xx/ts72xx.h | 94 --
arch/arm/mach-ep93xx/vision_ep9307.c | 311 -----
drivers/ata/pata_ep93xx.c | 33 +-
drivers/clk/Kconfig | 8 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ep93xx.c | 753 +++++++++++
drivers/clocksource/Kconfig | 11 +
drivers/clocksource/Makefile | 1 +
.../clocksource}/timer-ep93xx.c | 155 ++-
drivers/dma/ep93xx_dma.c | 125 +-
drivers/gpio/gpio-ep93xx.c | 331 ++---
drivers/input/keyboard/ep93xx_keypad.c | 74 +-
drivers/mtd/nand/raw/Kconfig | 7 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/technologic-nand-controller.c | 166 +++
drivers/net/ethernet/cirrus/ep93xx_eth.c | 63 +-
drivers/pinctrl/Kconfig | 7 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-ep93xx.c | 1429 ++++++++++++++++++++
drivers/power/reset/Kconfig | 10 +
drivers/power/reset/Makefile | 1 +
drivers/power/reset/ep93xx-restart.c | 85 ++
drivers/pwm/pwm-ep93xx.c | 26 +-
drivers/rtc/rtc-ep93xx.c | 8 +
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/cirrus/Kconfig | 12 +
drivers/soc/cirrus/Makefile | 2 +
drivers/soc/cirrus/soc-ep93xx.c | 215 +++
drivers/spi/spi-ep93xx.c | 28 +-
drivers/watchdog/ep93xx_wdt.c | 8 +
drivers/watchdog/ts72xx_wdt.c | 8 +
include/dt-bindings/clock/cirrus,ep9301-clk.h | 41 +
include/dt-bindings/dma/cirrus,ep93xx-dma.h | 26 +
include/linux/platform_data/dma-ep93xx.h | 25 +-
include/linux/platform_data/eth-ep93xx.h | 10 -
include/linux/platform_data/keypad-ep93xx.h | 32 -
include/linux/soc/cirrus/ep93xx.h | 29 +-
sound/soc/cirrus/Kconfig | 9 -
sound/soc/cirrus/Makefile | 4 -
sound/soc/cirrus/edb93xx.c | 117 --
77 files changed, 5126 insertions(+), 4168 deletions(-)
---
base-commit: bdc09c8e8b16d494ccd1c56e903e78dd76455a35
change-id: 20230605-ep93xx-01c76317e2d2
Best regards,
--
Nikita Shubin <nikita.shubin@maquefel.me>
^ permalink raw reply
* Re: [PATCH] HID: uhid: refactor deprecated strncpy
From: David Rheinsberg @ 2023-09-15 7:36 UTC (permalink / raw)
To: Kees Cook, Justin Stitt
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
linux-hardening, David Herrmann
In-Reply-To: <202309142206.60836CE@keescook>
Hi
On Fri, Sep 15, 2023, at 7:13 AM, Kees Cook wrote:
>> - /* @hid is zero-initialized, strncpy() is correct, strlcpy() not */
>> - len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1;
>> - strncpy(hid->name, ev->u.create2.name, len);
>> - len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1;
>> - strncpy(hid->phys, ev->u.create2.phys, len);
>> - len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1;
>> - strncpy(hid->uniq, ev->u.create2.uniq, len);
>
> ev->u.create2 is:
> struct uhid_create2_req {
> __u8 name[128];
> __u8 phys[64];
> __u8 uniq[64];
> ...
>
> hid is:
> struct hid_device { /* device report descriptor */
> ...
> char name[128]; /* Device name */
> char phys[64]; /* Device physical location */
> char uniq[64]; /* Device unique identifier (serial #) */
>
> So these "min" calls are redundant -- it wants to copy at most 1 less so
> it can be %NUL terminated. Which is what strscpy() already does. And
> source and dest are the same size, so we can't over-read source if it
> weren't terminated (since strscpy won't overread like strlcpy).
I *really* think we should keep the `min` calls. The compiler should already optimize them away, as both arguments are compile-time constants. There is no inherent reason why source and target are equal in size. Yes, it is unlikely to change, but I don't understand why we would want to implicitly rely on it, rather than make the compiler verify it for us. And `struct hid_device` is very much allowed to change in the future.
As an alternative, you can use BUILD_BUG_ON() and verify both are equal in length.
Thanks
David
^ permalink raw reply
* RE: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Xu, Even @ 2023-09-15 7:26 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: srinivas pandruvada, jikos@kernel.org,
benjamin.tissoires@redhat.com, linux-pm@vger.kernel.org,
linux-pci@vger.kernel.org, Lee, Jian Hui, Zhang, Lixu,
Ba, Najumon, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAAd53p5ywMVKWzhn0nYzvBnW_Bc=sntgBttJdcVUuf_a4AnX5w@mail.gmail.com>
Hi, Kai-Heng,
I am also not familiar with this S5 wakeup test case.
I already sent out mails to ask for help on it.
Will come back to you once I get feedback.
Thanks!
Best Regards,
Even Xu
-----Original Message-----
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Sent: Friday, September 15, 2023 2:01 PM
To: Xu, Even <even.xu@intel.com>
Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; jikos@kernel.org; benjamin.tissoires@redhat.com; linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee, Jian Hui <jianhui.lee@canonical.com>; Zhang, Lixu <lixu.zhang@intel.com>; Ba, Najumon <najumon.ba@intel.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
Hi Even,
On Fri, Sep 15, 2023 at 1:31 PM Xu, Even <even.xu@intel.com> wrote:
>
> Hi, Srinivas,
>
> Sure, I will test it.
> As long term not working on EHL, I doesn't have EHL board on hand right now, I can test this patch on other ISH related platforms.
> From the patch, it's focus on EHL platform, I assume Kai-Heng already verified the function on EHL board.
I only made sure the GPE overflow issue is fixed by the patch, but I didn't test the S5 wakeup.
That's because I don't know how to test it on the EHL system I have.
I'll test it if you can let me know how to test the S5 wakeup.
Kai-Heng
> I don't think it will take effect on other platforms, anyway, I will test it on the platforms I have to provide cross platform verification.
>
> Thanks!
>
> Best Regards,
> Even Xu
>
> -----Original Message-----
> From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
> Sent: Friday, September 15, 2023 12:11 AM
> To: Kai-Heng Feng <kai.heng.feng@canonical.com>; jikos@kernel.org;
> benjamin.tissoires@redhat.com
> Cc: linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee, Jian Hui
> <jianhui.lee@canonical.com>; Xu, Even <even.xu@intel.com>; Zhang, Lixu
> <lixu.zhang@intel.com>; Ba, Najumon <najumon.ba@intel.com>;
> linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
>
> Hi Even,
>
> On Thu, 2023-09-14 at 12:18 +0800, Kai-Heng Feng wrote:
> > System cannot suspend more than 255 times because the driver doesn't
> > have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the
> > GPE refcount overflows.
> >
> > Since PCI core and ACPI core already handles PCI PME wake and GPE
> > wake when the device has wakeup capability, use device_init_wakeup()
> > to let them do the wakeup setting work.
> >
> > Also add a shutdown callback which uses pci_prepare_to_sleep() to
> > let PCI and ACPI set OOB wakeup for S5.
> >
> Please test this change.
>
> Thanks,
> Srinivas
>
> > Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for
> > EHL OOB")
> > Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59
> > +++++++----------------
> > --
> > 1 file changed, 15 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > index 55cb25038e63..65e7eeb2fa64 100644
> > --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct
> > pci_dev *pdev)
> > return !pm_resume_via_firmware() || pdev->device ==
> > CHV_DEVICE_ID; }
> >
> > -static int enable_gpe(struct device *dev) -{ -#ifdef CONFIG_ACPI
> > - acpi_status acpi_sts;
> > - struct acpi_device *adev;
> > - struct acpi_device_wakeup *wakeup;
> > -
> > - adev = ACPI_COMPANION(dev);
> > - if (!adev) {
> > - dev_err(dev, "get acpi handle failed\n");
> > - return -ENODEV;
> > - }
> > - wakeup = &adev->wakeup;
> > -
> > - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> > >gpe_number);
> > - if (ACPI_FAILURE(acpi_sts)) {
> > - dev_err(dev, "enable ose_gpe failed\n");
> > - return -EIO;
> > - }
> > -
> > - return 0;
> > -#else
> > - return -ENODEV;
> > -#endif
> > -}
> > -
> > -static void enable_pme_wake(struct pci_dev *pdev) -{
> > - if ((pci_pme_capable(pdev, PCI_D0) ||
> > - pci_pme_capable(pdev, PCI_D3hot) ||
> > - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev-
> > >dev)) {
> > - pci_pme_active(pdev, true);
> > - dev_dbg(&pdev->dev, "ish ipc driver pme wake
> > enabled\n");
> > - }
> > -}
> > -
> > /**
> > * ish_probe() - PCI driver probe callback
> > * @pdev: pci device
> > @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >
> > /* Enable PME for EHL */
> > if (pdev->device == EHL_Ax_DEVICE_ID)
> > - enable_pme_wake(pdev);
> > + device_init_wakeup(dev, true);
> >
> > ret = ish_init(ishtp);
> > if (ret)
> > @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> > ish_device_disable(ishtp_dev); }
> >
> > +
> > +/**
> > + * ish_shutdown() - PCI driver shutdown callback
> > + * @pdev: pci device
> > + *
> > + * This function sets up wakeup for S5 */ static void
> > +ish_shutdown(struct pci_dev *pdev) {
> > + if (pdev->device == EHL_Ax_DEVICE_ID)
> > + pci_prepare_to_sleep(pdev); }
> > +
> > static struct device __maybe_unused *ish_resume_device;
> >
> > /* 50ms to get resume response */
> > @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> > device *device)
> > struct pci_dev *pdev = to_pci_dev(device);
> > struct ishtp_device *dev = pci_get_drvdata(pdev);
> >
> > - /* add this to finish power flow for EHL */
> > - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> > - pci_set_power_state(pdev, PCI_D0);
> > - enable_pme_wake(pdev);
> > - dev_dbg(dev->devc, "set power state to D0 for ehl\n");
> > - }
> > -
> > ish_resume_device = device;
> > dev->resume_flag = 1;
> >
> > @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> > .id_table = ish_pci_tbl,
> > .probe = ish_probe,
> > .remove = ish_remove,
> > + .shutdown = ish_shutdown,
> > .driver.pm = &ish_pm_ops,
> > };
> >
>
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Kai-Heng Feng @ 2023-09-15 6:00 UTC (permalink / raw)
To: Xu, Even
Cc: srinivas pandruvada, jikos@kernel.org,
benjamin.tissoires@redhat.com, linux-pm@vger.kernel.org,
linux-pci@vger.kernel.org, Lee, Jian Hui, Zhang, Lixu,
Ba, Najumon, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR11MB26245C44E84C37C1B551260EF4F6A@SN6PR11MB2624.namprd11.prod.outlook.com>
Hi Even,
On Fri, Sep 15, 2023 at 1:31 PM Xu, Even <even.xu@intel.com> wrote:
>
> Hi, Srinivas,
>
> Sure, I will test it.
> As long term not working on EHL, I doesn't have EHL board on hand right now, I can test this patch on other ISH related platforms.
> From the patch, it's focus on EHL platform, I assume Kai-Heng already verified the function on EHL board.
I only made sure the GPE overflow issue is fixed by the patch, but I
didn't test the S5 wakeup.
That's because I don't know how to test it on the EHL system I have.
I'll test it if you can let me know how to test the S5 wakeup.
Kai-Heng
> I don't think it will take effect on other platforms, anyway, I will test it on the platforms I have to provide cross platform verification.
>
> Thanks!
>
> Best Regards,
> Even Xu
>
> -----Original Message-----
> From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
> Sent: Friday, September 15, 2023 12:11 AM
> To: Kai-Heng Feng <kai.heng.feng@canonical.com>; jikos@kernel.org; benjamin.tissoires@redhat.com
> Cc: linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee, Jian Hui <jianhui.lee@canonical.com>; Xu, Even <even.xu@intel.com>; Zhang, Lixu <lixu.zhang@intel.com>; Ba, Najumon <najumon.ba@intel.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
>
> Hi Even,
>
> On Thu, 2023-09-14 at 12:18 +0800, Kai-Heng Feng wrote:
> > System cannot suspend more than 255 times because the driver doesn't
> > have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the
> > GPE refcount overflows.
> >
> > Since PCI core and ACPI core already handles PCI PME wake and GPE wake
> > when the device has wakeup capability, use device_init_wakeup() to let
> > them do the wakeup setting work.
> >
> > Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> > PCI and ACPI set OOB wakeup for S5.
> >
> Please test this change.
>
> Thanks,
> Srinivas
>
> > Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for
> > EHL OOB")
> > Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 +++++++----------------
> > --
> > 1 file changed, 15 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > index 55cb25038e63..65e7eeb2fa64 100644
> > --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct
> > pci_dev *pdev)
> > return !pm_resume_via_firmware() || pdev->device ==
> > CHV_DEVICE_ID;
> > }
> >
> > -static int enable_gpe(struct device *dev) -{ -#ifdef CONFIG_ACPI
> > - acpi_status acpi_sts;
> > - struct acpi_device *adev;
> > - struct acpi_device_wakeup *wakeup;
> > -
> > - adev = ACPI_COMPANION(dev);
> > - if (!adev) {
> > - dev_err(dev, "get acpi handle failed\n");
> > - return -ENODEV;
> > - }
> > - wakeup = &adev->wakeup;
> > -
> > - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> > >gpe_number);
> > - if (ACPI_FAILURE(acpi_sts)) {
> > - dev_err(dev, "enable ose_gpe failed\n");
> > - return -EIO;
> > - }
> > -
> > - return 0;
> > -#else
> > - return -ENODEV;
> > -#endif
> > -}
> > -
> > -static void enable_pme_wake(struct pci_dev *pdev) -{
> > - if ((pci_pme_capable(pdev, PCI_D0) ||
> > - pci_pme_capable(pdev, PCI_D3hot) ||
> > - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev-
> > >dev)) {
> > - pci_pme_active(pdev, true);
> > - dev_dbg(&pdev->dev, "ish ipc driver pme wake
> > enabled\n");
> > - }
> > -}
> > -
> > /**
> > * ish_probe() - PCI driver probe callback
> > * @pdev: pci device
> > @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >
> > /* Enable PME for EHL */
> > if (pdev->device == EHL_Ax_DEVICE_ID)
> > - enable_pme_wake(pdev);
> > + device_init_wakeup(dev, true);
> >
> > ret = ish_init(ishtp);
> > if (ret)
> > @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> > ish_device_disable(ishtp_dev);
> > }
> >
> > +
> > +/**
> > + * ish_shutdown() - PCI driver shutdown callback
> > + * @pdev: pci device
> > + *
> > + * This function sets up wakeup for S5 */ static void
> > +ish_shutdown(struct pci_dev *pdev) {
> > + if (pdev->device == EHL_Ax_DEVICE_ID)
> > + pci_prepare_to_sleep(pdev); }
> > +
> > static struct device __maybe_unused *ish_resume_device;
> >
> > /* 50ms to get resume response */
> > @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> > device *device)
> > struct pci_dev *pdev = to_pci_dev(device);
> > struct ishtp_device *dev = pci_get_drvdata(pdev);
> >
> > - /* add this to finish power flow for EHL */
> > - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> > - pci_set_power_state(pdev, PCI_D0);
> > - enable_pme_wake(pdev);
> > - dev_dbg(dev->devc, "set power state to D0 for ehl\n");
> > - }
> > -
> > ish_resume_device = device;
> > dev->resume_flag = 1;
> >
> > @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> > .id_table = ish_pci_tbl,
> > .probe = ish_probe,
> > .remove = ish_remove,
> > + .shutdown = ish_shutdown,
> > .driver.pm = &ish_pm_ops,
> > };
> >
>
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Kai-Heng Feng @ 2023-09-15 5:57 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: srinivas.pandruvada, jikos, benjamin.tissoires, linux-pm,
linux-pci, Jian Hui Lee, Even Xu, Zhang Lixu, Najumon Ba,
linux-input, linux-kernel
In-Reply-To: <20230914184249.GA74069@bhelgaas>
On Fri, Sep 15, 2023 at 2:42 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Sep 14, 2023 at 12:18:05PM +0800, Kai-Heng Feng wrote:
> > System cannot suspend more than 255 times because the driver doesn't
> > have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the GPE
> > refcount overflows.
>
> How can a user know they are seeing this problem? Is there a public
> bug report for it?
Here's the error when it happens:
[46307.532037] intel_ish_ipc 0000:00:1d.0: enable ose_gpe failed
The ticket was filed privately.
>
> > Since PCI core and ACPI core already handles PCI PME wake and GPE wake
> > when the device has wakeup capability, use device_init_wakeup() to let
> > them do the wakeup setting work.
> >
> > Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> > PCI and ACPI set OOB wakeup for S5.
>
> Is this logically required to be part of this patch, or could it be a
> separate patch?
Because that's what 2e23a70edabe ("HID: intel-ish-hid: ipc: finish
power flow for EHL OOB") intended to do, to enable wakeup from S5.
>
> > Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB")
> > Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 +++++++------------------
> > 1 file changed, 15 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > index 55cb25038e63..65e7eeb2fa64 100644
> > --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct pci_dev *pdev)
> > return !pm_resume_via_firmware() || pdev->device == CHV_DEVICE_ID;
> > }
> >
> > -static int enable_gpe(struct device *dev)
> > -{
> > -#ifdef CONFIG_ACPI
> > - acpi_status acpi_sts;
> > - struct acpi_device *adev;
> > - struct acpi_device_wakeup *wakeup;
> > -
> > - adev = ACPI_COMPANION(dev);
> > - if (!adev) {
> > - dev_err(dev, "get acpi handle failed\n");
> > - return -ENODEV;
> > - }
> > - wakeup = &adev->wakeup;
> > -
> > - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
> > - if (ACPI_FAILURE(acpi_sts)) {
> > - dev_err(dev, "enable ose_gpe failed\n");
> > - return -EIO;
> > - }
> > -
> > - return 0;
> > -#else
> > - return -ENODEV;
> > -#endif
> > -}
> > -
> > -static void enable_pme_wake(struct pci_dev *pdev)
> > -{
> > - if ((pci_pme_capable(pdev, PCI_D0) ||
> > - pci_pme_capable(pdev, PCI_D3hot) ||
> > - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev->dev)) {
> > - pci_pme_active(pdev, true);
> > - dev_dbg(&pdev->dev, "ish ipc driver pme wake enabled\n");
> > - }
> > -}
>
> I LOVE the removal of all this code. Thanks for doing it!
Thanks :)
>
> > /**
> > * ish_probe() - PCI driver probe callback
> > * @pdev: pci device
> > @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >
> > /* Enable PME for EHL */
> > if (pdev->device == EHL_Ax_DEVICE_ID)
> > - enable_pme_wake(pdev);
> > + device_init_wakeup(dev, true);
> >
> > ret = ish_init(ishtp);
> > if (ret)
> > @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> > ish_device_disable(ishtp_dev);
> > }
> >
> > +
> > +/**
> > + * ish_shutdown() - PCI driver shutdown callback
> > + * @pdev: pci device
> > + *
> > + * This function sets up wakeup for S5
> > + */
> > +static void ish_shutdown(struct pci_dev *pdev)
> > +{
> > + if (pdev->device == EHL_Ax_DEVICE_ID)
> > + pci_prepare_to_sleep(pdev);
>
> There are only five drivers that use pci_prepare_to_sleep(), so I have
> to ask what is special about this device that makes it necessary here?
The idea is that use pci_enable_wake() and pci_set_power_state() in
pci_prepare_to_sleep() to support S5 wakeup, by replacing the original
enable_pme_wake().
>
> It doesn't seem to match any of the scenarios mentioned in
> Documentation/power/pci.rst for using pci_prepare_to_sleep().
>
> Previously EHL_Ax_DEVICE_ID was used only in ish_probe(),
> ish_resume(), and _dma_no_cache_snooping(). None of those look like
> this, so this *looks* like new functionality that could/should be in a
> separate patch.
Because it's for S5 wakeup, so it's not a new functionality.
Kai-Heng
>
> > +}
> > +
> > static struct device __maybe_unused *ish_resume_device;
> >
> > /* 50ms to get resume response */
> > @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct device *device)
> > struct pci_dev *pdev = to_pci_dev(device);
> > struct ishtp_device *dev = pci_get_drvdata(pdev);
> >
> > - /* add this to finish power flow for EHL */
> > - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> > - pci_set_power_state(pdev, PCI_D0);
> > - enable_pme_wake(pdev);
> > - dev_dbg(dev->devc, "set power state to D0 for ehl\n");
> > - }
> > -
> > ish_resume_device = device;
> > dev->resume_flag = 1;
> >
> > @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> > .id_table = ish_pci_tbl,
> > .probe = ish_probe,
> > .remove = ish_remove,
> > + .shutdown = ish_shutdown,
> > .driver.pm = &ish_pm_ops,
> > };
> >
> > --
> > 2.34.1
> >
^ permalink raw reply
* RE: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Xu, Even @ 2023-09-15 5:31 UTC (permalink / raw)
To: srinivas pandruvada, Kai-Heng Feng, jikos@kernel.org,
benjamin.tissoires@redhat.com
Cc: linux-pm@vger.kernel.org, linux-pci@vger.kernel.org,
Lee, Jian Hui, Zhang, Lixu, Ba, Najumon,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <7b45ac2ed091497b4e21a6a5c19956161175ba16.camel@linux.intel.com>
Hi, Srinivas,
Sure, I will test it.
As long term not working on EHL, I doesn't have EHL board on hand right now, I can test this patch on other ISH related platforms.
From the patch, it's focus on EHL platform, I assume Kai-Heng already verified the function on EHL board.
I don't think it will take effect on other platforms, anyway, I will test it on the platforms I have to provide cross platform verification.
Thanks!
Best Regards,
Even Xu
-----Original Message-----
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Sent: Friday, September 15, 2023 12:11 AM
To: Kai-Heng Feng <kai.heng.feng@canonical.com>; jikos@kernel.org; benjamin.tissoires@redhat.com
Cc: linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee, Jian Hui <jianhui.lee@canonical.com>; Xu, Even <even.xu@intel.com>; Zhang, Lixu <lixu.zhang@intel.com>; Ba, Najumon <najumon.ba@intel.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
Hi Even,
On Thu, 2023-09-14 at 12:18 +0800, Kai-Heng Feng wrote:
> System cannot suspend more than 255 times because the driver doesn't
> have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the
> GPE refcount overflows.
>
> Since PCI core and ACPI core already handles PCI PME wake and GPE wake
> when the device has wakeup capability, use device_init_wakeup() to let
> them do the wakeup setting work.
>
> Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> PCI and ACPI set OOB wakeup for S5.
>
Please test this change.
Thanks,
Srinivas
> Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for
> EHL OOB")
> Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 +++++++----------------
> --
> 1 file changed, 15 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index 55cb25038e63..65e7eeb2fa64 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct
> pci_dev *pdev)
> return !pm_resume_via_firmware() || pdev->device ==
> CHV_DEVICE_ID;
> }
>
> -static int enable_gpe(struct device *dev) -{ -#ifdef CONFIG_ACPI
> - acpi_status acpi_sts;
> - struct acpi_device *adev;
> - struct acpi_device_wakeup *wakeup;
> -
> - adev = ACPI_COMPANION(dev);
> - if (!adev) {
> - dev_err(dev, "get acpi handle failed\n");
> - return -ENODEV;
> - }
> - wakeup = &adev->wakeup;
> -
> - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> >gpe_number);
> - if (ACPI_FAILURE(acpi_sts)) {
> - dev_err(dev, "enable ose_gpe failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -#else
> - return -ENODEV;
> -#endif
> -}
> -
> -static void enable_pme_wake(struct pci_dev *pdev) -{
> - if ((pci_pme_capable(pdev, PCI_D0) ||
> - pci_pme_capable(pdev, PCI_D3hot) ||
> - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev-
> >dev)) {
> - pci_pme_active(pdev, true);
> - dev_dbg(&pdev->dev, "ish ipc driver pme wake
> enabled\n");
> - }
> -}
> -
> /**
> * ish_probe() - PCI driver probe callback
> * @pdev: pci device
> @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>
> /* Enable PME for EHL */
> if (pdev->device == EHL_Ax_DEVICE_ID)
> - enable_pme_wake(pdev);
> + device_init_wakeup(dev, true);
>
> ret = ish_init(ishtp);
> if (ret)
> @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> ish_device_disable(ishtp_dev);
> }
>
> +
> +/**
> + * ish_shutdown() - PCI driver shutdown callback
> + * @pdev: pci device
> + *
> + * This function sets up wakeup for S5 */ static void
> +ish_shutdown(struct pci_dev *pdev) {
> + if (pdev->device == EHL_Ax_DEVICE_ID)
> + pci_prepare_to_sleep(pdev); }
> +
> static struct device __maybe_unused *ish_resume_device;
>
> /* 50ms to get resume response */
> @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> device *device)
> struct pci_dev *pdev = to_pci_dev(device);
> struct ishtp_device *dev = pci_get_drvdata(pdev);
>
> - /* add this to finish power flow for EHL */
> - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> - pci_set_power_state(pdev, PCI_D0);
> - enable_pme_wake(pdev);
> - dev_dbg(dev->devc, "set power state to D0 for ehl\n");
> - }
> -
> ish_resume_device = device;
> dev->resume_flag = 1;
>
> @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> .id_table = ish_pci_tbl,
> .probe = ish_probe,
> .remove = ish_remove,
> + .shutdown = ish_shutdown,
> .driver.pm = &ish_pm_ops,
> };
>
^ permalink raw reply
* [PATCH 8/8] HID: amd_sfh: Add light chromaticity for SFH1.1
From: Basavaraj Natikar @ 2023-09-15 5:17 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y for SFH1.1.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 5 ++++-
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index f100aaafa167..8a037de08e92 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -238,8 +238,11 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
als_input.illuminance_value = float_to_int(als_data.lux);
memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
- if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2)
+ if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2) {
als_input.light_color_temp = als_data.light_color_temp;
+ als_input.chromaticity_x_value = float_to_int(als_data.chromaticity_x);
+ als_input.chromaticity_y_value = float_to_int(als_data.chromaticity_y);
+ }
report_size = sizeof(als_input);
memcpy(input_report, &als_input, sizeof(als_input));
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 6f6f5db150c3..656c3e95ef8c 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -147,6 +147,8 @@ struct sfh_als_data {
struct sfh_common_data commondata;
u32 lux;
u32 light_color_temp;
+ u32 chromaticity_x;
+ u32 chromaticity_y;
};
struct hpd_status {
--
2.25.1
^ permalink raw reply related
* [PATCH 7/8] HID: amd_sfh: Add light chromaticity support
From: Basavaraj Natikar @ 2023-09-15 5:17 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
.../amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c | 5 ++++-
.../amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h | 2 ++
.../hid_descriptor/amd_sfh_hid_report_desc.h | 14 ++++++++++++++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
index b7e732ec4806..ef1f9be8b893 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
@@ -258,8 +258,11 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
als_input.illuminance_value =
(int)sensor_virt_addr[0] / AMD_SFH_FW_MULTIPLIER;
- if (sensor_idx == ACS_IDX)
+ if (sensor_idx == ACS_IDX) {
als_input.light_color_temp = sensor_virt_addr[1];
+ als_input.chromaticity_x_value = sensor_virt_addr[2];
+ als_input.chromaticity_y_value = sensor_virt_addr[3];
+ }
report_size = sizeof(als_input);
memcpy(input_report, &als_input, sizeof(als_input));
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
index a7fc50deca4d..882434b1501f 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
@@ -100,6 +100,8 @@ struct als_input_report {
/* values specific to this sensor */
int illuminance_value;
int light_color_temp;
+ int chromaticity_x_value;
+ int chromaticity_y_value;
} __packed;
struct hpd_feature_report {
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
index 26e994e54ded..67ec2d6a417d 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
@@ -648,6 +648,20 @@ static const u8 als_report_descriptor[] = {
0x75, 32, /* HID report size(32) */
0x95, 1, /* HID report count (1) */
0X81, 0x02, /* HID Input (Data_Arr_Abs) */
+0x0A, 0xD4, 0x04, /* HID usage sensor data light chromaticity_x */
+0x17, 0x00, 0x00, 0x01, 0x80, /* HID logical Min_32 */
+0x27, 0xFF, 0xFF, 0xFF, 0x7F, /* HID logical Max_32 */
+0x55, 0x0, /* HID unit exponent(0x0) */
+0x75, 32, /* HID report size(32) */
+0x95, 1, /* HID report count(1) */
+0X81, 0x02, /* HID Input (Data_Var_Abs) */
+0x0A, 0xD5, 0x04, /* HID usage sensor data light chromaticity_y */
+0x17, 0x00, 0x00, 0x01, 0x80, /* HID logical Min_32 */
+0x27, 0xFF, 0xFF, 0xFF, 0x7F, /* HID logical Max_32 */
+0x55, 0x0, /* HID unit exponent(0x0) */
+0x75, 32, /* HID report size(32) */
+0x95, 1, /* HID report count (1) */
+0X81, 0x02, /* HID Input (Data_Var_Abs) */
0xC0 /* HID end collection */
};
--
2.25.1
^ permalink raw reply related
* [PATCH 6/8] iio: hid-sensor-als: Add light chromaticity support
From: Basavaraj Natikar @ 2023-09-15 5:17 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/iio/light/hid-sensor-als.c | 61 ++++++++++++++++++++++++++++++
include/linux/hid-sensor-ids.h | 3 ++
2 files changed, 64 insertions(+)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 220fb93fea6d..e1a2cc83b892 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -17,6 +17,8 @@ enum {
CHANNEL_SCAN_INDEX_INTENSITY = 0,
CHANNEL_SCAN_INDEX_ILLUM = 1,
CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
CHANNEL_SCAN_INDEX_MAX
};
@@ -78,6 +80,30 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
},
+ {
+ .type = IIO_CHROMATICITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_X,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+ },
+ {
+ .type = IIO_CHROMATICITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_Y,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
+ },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
@@ -121,6 +147,16 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als_illum[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
break;
+ case CHANNEL_SCAN_INDEX_CHROMATICITY_X:
+ report_id = als_state->als_illum[chan->scan_index].report_id;
+ min = als_state->als_illum[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X;
+ break;
+ case CHANNEL_SCAN_INDEX_CHROMATICITY_Y:
+ report_id = als_state->als_illum[chan->scan_index].report_id;
+ min = als_state->als_illum[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y;
+ break;
default:
report_id = -1;
break;
@@ -245,6 +281,14 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
ret = 0;
break;
+ case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X:
+ als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_X] = sample_data;
+ ret = 0;
+ break;
+ case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y:
+ als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_Y] = sample_data;
+ ret = 0;
+ break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
@@ -289,6 +333,23 @@ static int als_parse_report(struct platform_device *pdev,
dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+ for (i = 0; i < 2; i++) {
+ int next_scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X + i;
+
+ ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
+ HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X + i,
+ &st->als_illum[next_scan_index]);
+ if (ret < 0)
+ return ret;
+
+ als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
+ st->als_illum[next_scan_index].size);
+
+ dev_dbg(&pdev->dev, "als %x:%x\n",
+ st->als_illum[next_scan_index].index,
+ st->als_illum[next_scan_index].report_id);
+ }
+
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als_illum[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 8af4fb3e0254..6730ee900ee1 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -22,6 +22,9 @@
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY 0x2004d3
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X 0x2004d4
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y 0x2004d5
/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.25.1
^ permalink raw reply related
* [PATCH 5/8] iio: Add channel for chromaticity
From: Basavaraj Natikar @ 2023-09-15 5:17 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add channel for chromaticity.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++
drivers/iio/industrialio-core.c | 1 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 ++
4 files changed, 12 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a2854dc9a839..6a810771f5e4 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2179,3 +2179,11 @@ Contact: linux-iio@vger.kernel.org
Description:
Number of conditions that must occur, during a running
period, before an event is generated.
+
+What: /sys/bus/iio/devices/iio:deviceX/in_chromaticity_x_raw
+What: /sys/bus/iio/devices/iio:deviceX/in_chromaticity_y_raw
+KernelVersion: 6.6
+Contact: linux-iio@vger.kernel.org
+Description:
+ The x and y light color coordinate on the CIE 1931 chromaticity
+ diagram.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d752e9c0499b..c1df66cdfdf6 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -90,6 +90,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_POSITIONRELATIVE] = "positionrelative",
[IIO_PHASE] = "phase",
[IIO_MASSCONCENTRATION] = "massconcentration",
+ [IIO_CHROMATICITY] = "chromaticity",
};
static const char * const iio_modifier_names[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index c79f2f046a0b..8952d48cfc64 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -47,6 +47,7 @@ enum iio_chan_type {
IIO_POSITIONRELATIVE,
IIO_PHASE,
IIO_MASSCONCENTRATION,
+ IIO_CHROMATICITY,
};
enum iio_modifier {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index 0a5c2bb60030..115ba1fbf3ac 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -59,6 +59,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_POSITIONRELATIVE] = "positionrelative",
[IIO_PHASE] = "phase",
[IIO_MASSCONCENTRATION] = "massconcentration",
+ [IIO_CHROMATICITY] = "chromaticity",
};
static const char * const iio_ev_type_text[] = {
@@ -173,6 +174,7 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_POSITIONRELATIVE:
case IIO_PHASE:
case IIO_MASSCONCENTRATION:
+ case IIO_CHROMATICITY:
break;
default:
return false;
--
2.25.1
^ permalink raw reply related
* [PATCH 4/8] HID: amd_sfh: Add support for SFH1.1 light color temperature
From: Basavaraj Natikar @ 2023-09-15 5:16 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature for SFH1.1.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 6 ++++++
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 13 +++++++++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 06bdcf072d10..f100aaafa167 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -188,6 +188,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
struct sfh_mag_data mag_data;
struct sfh_als_data als_data;
struct hpd_status hpdstatus;
+ struct sfh_base_info binfo;
void __iomem *sensoraddr;
u8 report_size = 0;
@@ -235,6 +236,11 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
get_common_inputs(&als_input.common_property, report_id);
als_input.illuminance_value = float_to_int(als_data.lux);
+
+ memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
+ if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2)
+ als_input.light_color_temp = als_data.light_color_temp;
+
report_size = sizeof(als_input);
memcpy(input_report, &als_input, sizeof(als_input));
break;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 9d31d5b510eb..6f6f5db150c3 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -88,6 +88,16 @@ struct sfh_sensor_list {
};
};
+struct sfh_sensor_prop {
+ union {
+ u32 sprop;
+ struct {
+ u32 elist : 16;
+ u32 feat : 16;
+ } sf;
+ };
+};
+
struct sfh_base_info {
union {
u32 sfh_base[24];
@@ -95,6 +105,8 @@ struct sfh_base_info {
struct sfh_platform_info plat_info;
struct sfh_firmware_info fw_info;
struct sfh_sensor_list s_list;
+ u32 rsvd;
+ struct sfh_sensor_prop s_prop[16];
} sbase;
};
};
@@ -134,6 +146,7 @@ struct sfh_mag_data {
struct sfh_als_data {
struct sfh_common_data commondata;
u32 lux;
+ u32 light_color_temp;
};
struct hpd_status {
--
2.25.1
^ permalink raw reply related
* [PATCH 3/8] HID: amd_sfh: Add support for light color temperature
From: Basavaraj Natikar @ 2023-09-15 5:16 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c | 4 ++++
drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h | 1 +
.../amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h | 7 +++++++
3 files changed, 12 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
index 8716a05950c8..b7e732ec4806 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
@@ -257,6 +257,10 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
else
als_input.illuminance_value =
(int)sensor_virt_addr[0] / AMD_SFH_FW_MULTIPLIER;
+
+ if (sensor_idx == ACS_IDX)
+ als_input.light_color_temp = sensor_virt_addr[1];
+
report_size = sizeof(als_input);
memcpy(input_report, &als_input, sizeof(als_input));
break;
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
index ebd55675eb62..a7fc50deca4d 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
@@ -99,6 +99,7 @@ struct als_input_report {
struct common_input_property common_property;
/* values specific to this sensor */
int illuminance_value;
+ int light_color_temp;
} __packed;
struct hpd_feature_report {
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
index 697f2791ea9c..26e994e54ded 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
@@ -641,6 +641,13 @@ static const u8 als_report_descriptor[] = {
0x75, 32, /* HID report size(32) */
0x95, 1, /* HID report count (1) */
0X81, 0x02, /* HID Input (Data_Arr_Abs) */
+0x0A, 0xD2, 0x04, /* HID usage sensor data light temperature */
+0x17, 0x00, 0x00, 0x01, 0x80, /* HID logical Min_32 */
+0x27, 0xFF, 0xFF, 0xFF, 0x7F, /* HID logical Max_32 */
+0x55, 0x0, /* HID unit exponent(0x0) */
+0x75, 32, /* HID report size(32) */
+0x95, 1, /* HID report count (1) */
+0X81, 0x02, /* HID Input (Data_Arr_Abs) */
0xC0 /* HID end collection */
};
--
2.25.1
^ permalink raw reply related
* [PATCH 2/8] iio: hid-sensor-als: Add light color temperature support
From: Basavaraj Natikar @ 2023-09-15 5:16 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/iio/light/hid-sensor-als.c | 33 ++++++++++++++++++++++++++++++
include/linux/hid-sensor-ids.h | 1 +
2 files changed, 34 insertions(+)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 48879e233aec..220fb93fea6d 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -16,6 +16,7 @@
enum {
CHANNEL_SCAN_INDEX_INTENSITY = 0,
CHANNEL_SCAN_INDEX_ILLUM = 1,
+ CHANNEL_SCAN_INDEX_COLOR_TEMP,
CHANNEL_SCAN_INDEX_MAX
};
@@ -65,6 +66,18 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
},
+ {
+ .type = IIO_TEMP,
+ .modified = 1,
+ .channel2 = IIO_MOD_TEMP_AMBIENT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
@@ -103,6 +116,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als_illum[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_ILLUM;
break;
+ case CHANNEL_SCAN_INDEX_COLOR_TEMP:
+ report_id = als_state->als_illum[chan->scan_index].report_id;
+ min = als_state->als_illum[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
+ break;
default:
report_id = -1;
break;
@@ -223,6 +241,10 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[CHANNEL_SCAN_INDEX_ILLUM] = sample_data;
ret = 0;
break;
+ case HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE:
+ als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
+ ret = 0;
+ break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
@@ -256,6 +278,17 @@ static int als_parse_report(struct platform_device *pdev,
st->als_illum[i].report_id);
}
+ ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
+ HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
+ &st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
+ if (ret < 0)
+ return ret;
+ als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
+
+ dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
+ st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als_illum[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 13b1e65fbdcc..8af4fb3e0254 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -21,6 +21,7 @@
#define HID_USAGE_SENSOR_ALS 0x200041
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
+#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.25.1
^ permalink raw reply related
* [PATCH 1/8] iio: hid-sensor-als: Use channel index to support more hub attributes
From: Basavaraj Natikar @ 2023-09-15 5:16 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
In-Reply-To: <20230915051703.1689578-1-Basavaraj.Natikar@amd.com>
Sensor hub attributes can be extended to support more channels. So in
order to support more sensor hub attributes for ALS use channel index to
get specific sensor hub attributes.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/iio/light/hid-sensor-als.c | 38 +++++++++++++++---------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index eb1aedad7edc..48879e233aec 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -24,7 +24,7 @@ enum {
struct als_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
- struct hid_sensor_hub_attribute_info als_illum;
+ struct hid_sensor_hub_attribute_info als_illum[CHANNEL_SCAN_INDEX_MAX];
struct {
u32 illum[CHANNEL_SCAN_INDEX_MAX];
u64 timestamp __aligned(8);
@@ -99,8 +99,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
switch (chan->scan_index) {
case CHANNEL_SCAN_INDEX_INTENSITY:
case CHANNEL_SCAN_INDEX_ILLUM:
- report_id = als_state->als_illum.report_id;
- min = als_state->als_illum.logical_minimum;
+ report_id = als_state->als_illum[chan->scan_index].report_id;
+ min = als_state->als_illum[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_ILLUM;
break;
default:
@@ -242,23 +242,23 @@ static int als_parse_report(struct platform_device *pdev,
struct als_state *st)
{
int ret;
+ int i;
+
+ for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
+ ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
+ HID_USAGE_SENSOR_LIGHT_ILLUM,
+ &st->als_illum[i]);
+ if (ret < 0)
+ return ret;
+ als_adjust_channel_bit_mask(channels, i, st->als_illum[i].size);
+
+ dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[i].index,
+ st->als_illum[i].report_id);
+ }
- ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
- usage_id,
- HID_USAGE_SENSOR_LIGHT_ILLUM,
- &st->als_illum);
- if (ret < 0)
- return ret;
- als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_INTENSITY,
- st->als_illum.size);
- als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_ILLUM,
- st->als_illum.size);
-
- dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum.index,
- st->als_illum.report_id);
-
- st->scale_precision = hid_sensor_format_scale(usage_id, &st->als_illum,
- &st->scale_pre_decml, &st->scale_post_decml);
+ st->scale_precision = hid_sensor_format_scale(usage_id,
+ &st->als_illum[CHANNEL_SCAN_INDEX_INTENSITY],
+ &st->scale_pre_decml, &st->scale_post_decml);
return ret;
}
--
2.25.1
^ permalink raw reply related
* [PATCH 0/8] Multiple light sensor support
From: Basavaraj Natikar @ 2023-09-15 5:16 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio
Cc: Basavaraj Natikar
This series adds support for light color temperature and chromaticity.
Basavaraj Natikar (8):
iio: hid-sensor-als: Use channel index to support more hub attributes
iio: hid-sensor-als: Add light color temperature support
HID: amd_sfh: Add support for light color temperature
HID: amd_sfh: Add support for SFH1.1 light color temperature
iio: Add channel for chromaticity
iio: hid-sensor-als: Add light chromaticity support
HID: amd_sfh: Add light chromaticity support
HID: amd_sfh: Add light chromaticity for SFH1.1
Documentation/ABI/testing/sysfs-bus-iio | 8 ++
.../hid_descriptor/amd_sfh_hid_desc.c | 7 +
.../hid_descriptor/amd_sfh_hid_desc.h | 3 +
.../hid_descriptor/amd_sfh_hid_report_desc.h | 21 +++
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 9 ++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 15 +++
drivers/iio/industrialio-core.c | 1 +
drivers/iio/light/hid-sensor-als.c | 124 +++++++++++++++---
include/linux/hid-sensor-ids.h | 4 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 +
11 files changed, 180 insertions(+), 15 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH] HID: uhid: refactor deprecated strncpy
From: Kees Cook @ 2023-09-15 5:13 UTC (permalink / raw)
To: Justin Stitt
Cc: David Rheinsberg, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, linux-hardening, David Herrmann
In-Reply-To: <20230914-strncpy-drivers-hid-uhid-c-v1-1-18a190060d8d@google.com>
On Thu, Sep 14, 2023 at 10:47:21PM +0000, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings [1].
>
> We should prefer more robust and less ambiguous string interfaces.
>
> A suitable replacement is `strscpy` [2] due to the fact that it
> guarantees NUL-termination on the destination buffer without
> unnecessarily NUL-padding.
>
> Looking at: Commit 4d26d1d1e806 ("Revert "HID: uhid: use strlcpy() instead of strncpy()"")
> we see referenced the fact that many attempts have been made to change
> these strncpy's into strlcpy to no success. I think strscpy is an
> objectively better interface here as it doesn't unnecessarily NUL-pad
> the destination buffer whilst allowing us to drop the `len = min(...)`
> pattern as strscpy will implicitly limit the number of bytes copied by
> the smaller of its dest and src arguments.
I think the worry here is that ev->u.create2.name may not be %NUL
terminated. But I think that doesn't matter, see below...
Also, note, this should CC David Herrmann <dh.herrmann@gmail.com>
(now added).
> So while the existing code may not have a bug (i.e: overread problems)
> we should still favor strscpy due to readability (plus a very slight
> performance boost).
>
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> drivers/hid/uhid.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 4588d2cd4ea4..00e1566ad37b 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -490,7 +490,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
> const struct uhid_event *ev)
> {
> struct hid_device *hid;
> - size_t rd_size, len;
> + size_t rd_size;
> void *rd_data;
> int ret;
>
> @@ -514,13 +514,9 @@ static int uhid_dev_create2(struct uhid_device *uhid,
> goto err_free;
> }
>
> - /* @hid is zero-initialized, strncpy() is correct, strlcpy() not */
> - len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1;
> - strncpy(hid->name, ev->u.create2.name, len);
> - len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1;
> - strncpy(hid->phys, ev->u.create2.phys, len);
> - len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1;
> - strncpy(hid->uniq, ev->u.create2.uniq, len);
ev->u.create2 is:
struct uhid_create2_req {
__u8 name[128];
__u8 phys[64];
__u8 uniq[64];
...
hid is:
struct hid_device { /* device report descriptor */
...
char name[128]; /* Device name */
char phys[64]; /* Device physical location */
char uniq[64]; /* Device unique identifier (serial #) */
So these "min" calls are redundant -- it wants to copy at most 1 less so
it can be %NUL terminated. Which is what strscpy() already does. And
source and dest are the same size, so we can't over-read source if it
weren't terminated (since strscpy won't overread like strlcpy).
> + strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
> + strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
> + strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
>
> hid->ll_driver = &uhid_hid_driver;
> hid->bus = ev->u.create2.bus;
>
> ---
> base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
> change-id: 20230914-strncpy-drivers-hid-uhid-c-a465f3e784dd
>
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] HID: prodikeys: refactor deprecated strncpy
From: Kees Cook @ 2023-09-15 5:02 UTC (permalink / raw)
To: Justin Stitt
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
linux-hardening
In-Reply-To: <20230914-strncpy-drivers-hid-hid-prodikeys-c-v1-1-10c00550f2c2@google.com>
On Thu, Sep 14, 2023 at 10:20:55PM +0000, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings [1].
>
> We should prefer more robust and less ambiguous string interfaces.
>
> A suitable replacement is `strscpy` [2] due to the fact that it guarantees
> NUL-termination on the destination buffer without unnecessarily NUL-padding.
>
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Note: for some reason if NUL-padding is needed let's opt for `strscpy_pad()`
> ---
> drivers/hid/hid-prodikeys.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
> index e4e9471d0f1e..c16d2ba6ea16 100644
> --- a/drivers/hid/hid-prodikeys.c
> +++ b/drivers/hid/hid-prodikeys.c
> @@ -639,9 +639,9 @@ static int pcmidi_snd_initialise(struct pcmidi_snd *pm)
> goto fail;
> }
>
> - strncpy(card->driver, shortname, sizeof(card->driver));
> - strncpy(card->shortname, shortname, sizeof(card->shortname));
> - strncpy(card->longname, longname, sizeof(card->longname));
> + strscpy(card->driver, shortname, sizeof(card->driver));
> + strscpy(card->shortname, shortname, sizeof(card->shortname));
> + strscpy(card->longname, longname, sizeof(card->longname));
"card" is already kzalloc()ed so no _pad() is needed, good.
>
> /* Set up rawmidi */
> err = snd_rawmidi_new(card, card->shortname, 0,
> @@ -652,7 +652,7 @@ static int pcmidi_snd_initialise(struct pcmidi_snd *pm)
> goto fail;
> }
> pm->rwmidi = rwmidi;
> - strncpy(rwmidi->name, card->shortname, sizeof(rwmidi->name));
> + strscpy(rwmidi->name, card->shortname, sizeof(rwmidi->name));
> rwmidi->info_flags = SNDRV_RAWMIDI_INFO_INPUT;
> rwmidi->private_data = pm;
Same here.
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
>
>
> ---
> base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
> change-id: 20230914-strncpy-drivers-hid-hid-prodikeys-c-cf42614a21d4
>
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v4 11/11] arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
From: Doug Anderson @ 2023-09-14 23:42 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, yangcong5, devicetree, Daniel Vetter, hsinyi,
Chris Morgan, linux-input, cros-qcom-dts-watchers,
Dmitry Torokhov, linux-kernel, dri-devel
In-Reply-To: <20230727101636.v4.11.Ia06c340e3482563e6bfd3106ecd0d3139f173ca4@changeid>
Hi,
On Thu, Jul 27, 2023 at 10:19 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> Let's provide the proper link from the touchscreen to the panel on
> trogdor devices where the touchscreen support it. This allows the OS
> to power sequence the touchscreen more properly.
>
> For the most part, this is just expected to marginally improve power
> consumption while the screen is off. However, in at least one trogdor
> model (wormdingler) it's suspected that this will fix some behavorial
> corner cases when the panel power cycles (like for a modeset) without
> the touchscreen power cycling.
>
> NOTE: some trogdor variants use touchscreens that don't (yet) support
> linking the touchscreen and the panel. Those variants are left alone.
>
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> (no changes since v1)
>
> arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 1 +
> arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 1 +
> arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 1 +
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 1 +
> arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 1 +
> arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 1 +
> 6 files changed, 6 insertions(+)
FWIW, this old patch could land any time. All the earlier patches in
the series have landed.
-Doug
^ permalink raw reply
* [PATCH] HID: uhid: refactor deprecated strncpy
From: Justin Stitt @ 2023-09-14 22:47 UTC (permalink / raw)
To: David Rheinsberg, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-hardening, Kees Cook,
Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it
guarantees NUL-termination on the destination buffer without
unnecessarily NUL-padding.
Looking at: Commit 4d26d1d1e806 ("Revert "HID: uhid: use strlcpy() instead of strncpy()"")
we see referenced the fact that many attempts have been made to change
these strncpy's into strlcpy to no success. I think strscpy is an
objectively better interface here as it doesn't unnecessarily NUL-pad
the destination buffer whilst allowing us to drop the `len = min(...)`
pattern as strscpy will implicitly limit the number of bytes copied by
the smaller of its dest and src arguments.
So while the existing code may not have a bug (i.e: overread problems)
we should still favor strscpy due to readability (plus a very slight
performance boost).
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
drivers/hid/uhid.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 4588d2cd4ea4..00e1566ad37b 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -490,7 +490,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
const struct uhid_event *ev)
{
struct hid_device *hid;
- size_t rd_size, len;
+ size_t rd_size;
void *rd_data;
int ret;
@@ -514,13 +514,9 @@ static int uhid_dev_create2(struct uhid_device *uhid,
goto err_free;
}
- /* @hid is zero-initialized, strncpy() is correct, strlcpy() not */
- len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1;
- strncpy(hid->name, ev->u.create2.name, len);
- len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1;
- strncpy(hid->phys, ev->u.create2.phys, len);
- len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1;
- strncpy(hid->uniq, ev->u.create2.uniq, len);
+ strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
+ strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
+ strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
hid->ll_driver = &uhid_hid_driver;
hid->bus = ev->u.create2.bus;
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 20230914-strncpy-drivers-hid-uhid-c-a465f3e784dd
Best regards,
--
Justin Stitt <justinstitt@google.com>
^ permalink raw reply related
* [PATCH] HID: prodikeys: refactor deprecated strncpy
From: Justin Stitt @ 2023-09-14 22:20 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-hardening, Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: for some reason if NUL-padding is needed let's opt for `strscpy_pad()`
---
drivers/hid/hid-prodikeys.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
index e4e9471d0f1e..c16d2ba6ea16 100644
--- a/drivers/hid/hid-prodikeys.c
+++ b/drivers/hid/hid-prodikeys.c
@@ -639,9 +639,9 @@ static int pcmidi_snd_initialise(struct pcmidi_snd *pm)
goto fail;
}
- strncpy(card->driver, shortname, sizeof(card->driver));
- strncpy(card->shortname, shortname, sizeof(card->shortname));
- strncpy(card->longname, longname, sizeof(card->longname));
+ strscpy(card->driver, shortname, sizeof(card->driver));
+ strscpy(card->shortname, shortname, sizeof(card->shortname));
+ strscpy(card->longname, longname, sizeof(card->longname));
/* Set up rawmidi */
err = snd_rawmidi_new(card, card->shortname, 0,
@@ -652,7 +652,7 @@ static int pcmidi_snd_initialise(struct pcmidi_snd *pm)
goto fail;
}
pm->rwmidi = rwmidi;
- strncpy(rwmidi->name, card->shortname, sizeof(rwmidi->name));
+ strscpy(rwmidi->name, card->shortname, sizeof(rwmidi->name));
rwmidi->info_flags = SNDRV_RAWMIDI_INFO_INPUT;
rwmidi->private_data = pm;
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 20230914-strncpy-drivers-hid-hid-prodikeys-c-cf42614a21d4
Best regards,
--
Justin Stitt <justinstitt@google.com>
^ permalink raw reply related
* Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Bjorn Helgaas @ 2023-09-14 18:42 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: srinivas.pandruvada, jikos, benjamin.tissoires, linux-pm,
linux-pci, Jian Hui Lee, Even Xu, Zhang Lixu, Najumon Ba,
linux-input, linux-kernel
In-Reply-To: <20230914041806.816741-1-kai.heng.feng@canonical.com>
On Thu, Sep 14, 2023 at 12:18:05PM +0800, Kai-Heng Feng wrote:
> System cannot suspend more than 255 times because the driver doesn't
> have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the GPE
> refcount overflows.
How can a user know they are seeing this problem? Is there a public
bug report for it?
> Since PCI core and ACPI core already handles PCI PME wake and GPE wake
> when the device has wakeup capability, use device_init_wakeup() to let
> them do the wakeup setting work.
>
> Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> PCI and ACPI set OOB wakeup for S5.
Is this logically required to be part of this patch, or could it be a
separate patch?
> Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB")
> Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 +++++++------------------
> 1 file changed, 15 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index 55cb25038e63..65e7eeb2fa64 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct pci_dev *pdev)
> return !pm_resume_via_firmware() || pdev->device == CHV_DEVICE_ID;
> }
>
> -static int enable_gpe(struct device *dev)
> -{
> -#ifdef CONFIG_ACPI
> - acpi_status acpi_sts;
> - struct acpi_device *adev;
> - struct acpi_device_wakeup *wakeup;
> -
> - adev = ACPI_COMPANION(dev);
> - if (!adev) {
> - dev_err(dev, "get acpi handle failed\n");
> - return -ENODEV;
> - }
> - wakeup = &adev->wakeup;
> -
> - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
> - if (ACPI_FAILURE(acpi_sts)) {
> - dev_err(dev, "enable ose_gpe failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -#else
> - return -ENODEV;
> -#endif
> -}
> -
> -static void enable_pme_wake(struct pci_dev *pdev)
> -{
> - if ((pci_pme_capable(pdev, PCI_D0) ||
> - pci_pme_capable(pdev, PCI_D3hot) ||
> - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev->dev)) {
> - pci_pme_active(pdev, true);
> - dev_dbg(&pdev->dev, "ish ipc driver pme wake enabled\n");
> - }
> -}
I LOVE the removal of all this code. Thanks for doing it!
> /**
> * ish_probe() - PCI driver probe callback
> * @pdev: pci device
> @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> /* Enable PME for EHL */
> if (pdev->device == EHL_Ax_DEVICE_ID)
> - enable_pme_wake(pdev);
> + device_init_wakeup(dev, true);
>
> ret = ish_init(ishtp);
> if (ret)
> @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> ish_device_disable(ishtp_dev);
> }
>
> +
> +/**
> + * ish_shutdown() - PCI driver shutdown callback
> + * @pdev: pci device
> + *
> + * This function sets up wakeup for S5
> + */
> +static void ish_shutdown(struct pci_dev *pdev)
> +{
> + if (pdev->device == EHL_Ax_DEVICE_ID)
> + pci_prepare_to_sleep(pdev);
There are only five drivers that use pci_prepare_to_sleep(), so I have
to ask what is special about this device that makes it necessary here?
It doesn't seem to match any of the scenarios mentioned in
Documentation/power/pci.rst for using pci_prepare_to_sleep().
Previously EHL_Ax_DEVICE_ID was used only in ish_probe(),
ish_resume(), and _dma_no_cache_snooping(). None of those look like
this, so this *looks* like new functionality that could/should be in a
separate patch.
> +}
> +
> static struct device __maybe_unused *ish_resume_device;
>
> /* 50ms to get resume response */
> @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct device *device)
> struct pci_dev *pdev = to_pci_dev(device);
> struct ishtp_device *dev = pci_get_drvdata(pdev);
>
> - /* add this to finish power flow for EHL */
> - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> - pci_set_power_state(pdev, PCI_D0);
> - enable_pme_wake(pdev);
> - dev_dbg(dev->devc, "set power state to D0 for ehl\n");
> - }
> -
> ish_resume_device = device;
> dev->resume_flag = 1;
>
> @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> .id_table = ish_pci_tbl,
> .probe = ish_probe,
> .remove = ish_remove,
> + .shutdown = ish_shutdown,
> .driver.pm = &ish_pm_ops,
> };
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: srinivas pandruvada @ 2023-09-14 16:11 UTC (permalink / raw)
To: Kai-Heng Feng, jikos, benjamin.tissoires
Cc: linux-pm, linux-pci, Jian Hui Lee, Even Xu, Zhang Lixu,
Najumon Ba, linux-input, linux-kernel
In-Reply-To: <20230914041806.816741-1-kai.heng.feng@canonical.com>
Hi Even,
On Thu, 2023-09-14 at 12:18 +0800, Kai-Heng Feng wrote:
> System cannot suspend more than 255 times because the driver doesn't
> have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the
> GPE
> refcount overflows.
>
> Since PCI core and ACPI core already handles PCI PME wake and GPE
> wake
> when the device has wakeup capability, use device_init_wakeup() to
> let
> them do the wakeup setting work.
>
> Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> PCI and ACPI set OOB wakeup for S5.
>
Please test this change.
Thanks,
Srinivas
> Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for
> EHL OOB")
> Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 +++++++----------------
> --
> 1 file changed, 15 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index 55cb25038e63..65e7eeb2fa64 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct
> pci_dev *pdev)
> return !pm_resume_via_firmware() || pdev->device ==
> CHV_DEVICE_ID;
> }
>
> -static int enable_gpe(struct device *dev)
> -{
> -#ifdef CONFIG_ACPI
> - acpi_status acpi_sts;
> - struct acpi_device *adev;
> - struct acpi_device_wakeup *wakeup;
> -
> - adev = ACPI_COMPANION(dev);
> - if (!adev) {
> - dev_err(dev, "get acpi handle failed\n");
> - return -ENODEV;
> - }
> - wakeup = &adev->wakeup;
> -
> - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> >gpe_number);
> - if (ACPI_FAILURE(acpi_sts)) {
> - dev_err(dev, "enable ose_gpe failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -#else
> - return -ENODEV;
> -#endif
> -}
> -
> -static void enable_pme_wake(struct pci_dev *pdev)
> -{
> - if ((pci_pme_capable(pdev, PCI_D0) ||
> - pci_pme_capable(pdev, PCI_D3hot) ||
> - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev-
> >dev)) {
> - pci_pme_active(pdev, true);
> - dev_dbg(&pdev->dev, "ish ipc driver pme wake
> enabled\n");
> - }
> -}
> -
> /**
> * ish_probe() - PCI driver probe callback
> * @pdev: pci device
> @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>
> /* Enable PME for EHL */
> if (pdev->device == EHL_Ax_DEVICE_ID)
> - enable_pme_wake(pdev);
> + device_init_wakeup(dev, true);
>
> ret = ish_init(ishtp);
> if (ret)
> @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> ish_device_disable(ishtp_dev);
> }
>
> +
> +/**
> + * ish_shutdown() - PCI driver shutdown callback
> + * @pdev: pci device
> + *
> + * This function sets up wakeup for S5
> + */
> +static void ish_shutdown(struct pci_dev *pdev)
> +{
> + if (pdev->device == EHL_Ax_DEVICE_ID)
> + pci_prepare_to_sleep(pdev);
> +}
> +
> static struct device __maybe_unused *ish_resume_device;
>
> /* 50ms to get resume response */
> @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> device *device)
> struct pci_dev *pdev = to_pci_dev(device);
> struct ishtp_device *dev = pci_get_drvdata(pdev);
>
> - /* add this to finish power flow for EHL */
> - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> - pci_set_power_state(pdev, PCI_D0);
> - enable_pme_wake(pdev);
> - dev_dbg(dev->devc, "set power state to D0 for
> ehl\n");
> - }
> -
> ish_resume_device = device;
> dev->resume_flag = 1;
>
> @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> .id_table = ish_pci_tbl,
> .probe = ish_probe,
> .remove = ish_remove,
> + .shutdown = ish_shutdown,
> .driver.pm = &ish_pm_ops,
> };
>
^ permalink raw reply
* [syzbot] Monthly input report (Sep 2023)
From: syzbot @ 2023-09-14 9:56 UTC (permalink / raw)
To: linux-input, linux-kernel, syzkaller-bugs
Hello input maintainers/developers,
This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input
During the period, 1 new issues were detected and 2 were fixed.
In total, 12 issues are still open and 50 have been fixed so far.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 3172 Yes WARNING in input_mt_init_slots
https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
<2> 105 Yes WARNING in cm109_urb_irq_callback/usb_submit_urb
https://syzkaller.appspot.com/bug?extid=2d6d691af5ab4b7e66df
<3> 35 Yes INFO: task hung in uhid_char_release
https://syzkaller.appspot.com/bug?extid=8fe2d362af0e1cba8735
<4> 34 Yes WARNING in bcm5974_start_traffic/usb_submit_urb
https://syzkaller.appspot.com/bug?extid=348331f63b034f89b622
<5> 14 Yes KASAN: use-after-free Read in powermate_config_complete (4)
https://syzkaller.appspot.com/bug?extid=0434ac83f907a1dbdd1e
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: touchscreen: document Himax HX852x(ES)
From: Krzysztof Kozlowski @ 2023-09-14 6:22 UTC (permalink / raw)
To: Stephan Gerhold, Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
linux-input, devicetree, linux-kernel, Jeff LaBundy,
Jonathan Albrieux
In-Reply-To: <20230913-hx852x-v1-1-9c1ebff536eb@gerhold.net>
On 13/09/2023 15:25, Stephan Gerhold wrote:
> Himax HX852x(ES) is a touch panel controller with optional support
> for capacitive touch keys.
>
> Unfortunately, the model naming is quite unclear and confusing. There
> seems to be a distinction between models (e.g. HX8526) and the "series"
> suffix (e.g. -A, -B, -C, -D, -E, -ES). But this doesn't seem to be
> applied very consistently because e.g. HX8527-E(44) actually seems to
> belong to the -ES series.
...
> + touchscreen-inverted-x: true
> + touchscreen-inverted-y: true
> + touchscreen-size-x: true
> + touchscreen-size-y: true
> + touchscreen-swapped-x-y: true
> +
> + linux,keycodes:
> + minItems: 1
> + maxItems: 4
> +
> +additionalProperties: false
> +
> +required:
If there is going to be resend, required: goes before additionalProperties:.
In any case:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Kai-Heng Feng @ 2023-09-14 4:18 UTC (permalink / raw)
To: srinivas.pandruvada, jikos, benjamin.tissoires
Cc: linux-pm, linux-pci, Kai-Heng Feng, Jian Hui Lee, Even Xu,
Zhang Lixu, Najumon Ba, linux-input, linux-kernel
System cannot suspend more than 255 times because the driver doesn't
have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the GPE
refcount overflows.
Since PCI core and ACPI core already handles PCI PME wake and GPE wake
when the device has wakeup capability, use device_init_wakeup() to let
them do the wakeup setting work.
Also add a shutdown callback which uses pci_prepare_to_sleep() to let
PCI and ACPI set OOB wakeup for S5.
Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB")
Cc: Jian Hui Lee <jianhui.lee@canonical.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 +++++++------------------
1 file changed, 15 insertions(+), 44 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 55cb25038e63..65e7eeb2fa64 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct pci_dev *pdev)
return !pm_resume_via_firmware() || pdev->device == CHV_DEVICE_ID;
}
-static int enable_gpe(struct device *dev)
-{
-#ifdef CONFIG_ACPI
- acpi_status acpi_sts;
- struct acpi_device *adev;
- struct acpi_device_wakeup *wakeup;
-
- adev = ACPI_COMPANION(dev);
- if (!adev) {
- dev_err(dev, "get acpi handle failed\n");
- return -ENODEV;
- }
- wakeup = &adev->wakeup;
-
- acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
- if (ACPI_FAILURE(acpi_sts)) {
- dev_err(dev, "enable ose_gpe failed\n");
- return -EIO;
- }
-
- return 0;
-#else
- return -ENODEV;
-#endif
-}
-
-static void enable_pme_wake(struct pci_dev *pdev)
-{
- if ((pci_pme_capable(pdev, PCI_D0) ||
- pci_pme_capable(pdev, PCI_D3hot) ||
- pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev->dev)) {
- pci_pme_active(pdev, true);
- dev_dbg(&pdev->dev, "ish ipc driver pme wake enabled\n");
- }
-}
-
/**
* ish_probe() - PCI driver probe callback
* @pdev: pci device
@@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Enable PME for EHL */
if (pdev->device == EHL_Ax_DEVICE_ID)
- enable_pme_wake(pdev);
+ device_init_wakeup(dev, true);
ret = ish_init(ishtp);
if (ret)
@@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
ish_device_disable(ishtp_dev);
}
+
+/**
+ * ish_shutdown() - PCI driver shutdown callback
+ * @pdev: pci device
+ *
+ * This function sets up wakeup for S5
+ */
+static void ish_shutdown(struct pci_dev *pdev)
+{
+ if (pdev->device == EHL_Ax_DEVICE_ID)
+ pci_prepare_to_sleep(pdev);
+}
+
static struct device __maybe_unused *ish_resume_device;
/* 50ms to get resume response */
@@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct device *device)
struct pci_dev *pdev = to_pci_dev(device);
struct ishtp_device *dev = pci_get_drvdata(pdev);
- /* add this to finish power flow for EHL */
- if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
- pci_set_power_state(pdev, PCI_D0);
- enable_pme_wake(pdev);
- dev_dbg(dev->devc, "set power state to D0 for ehl\n");
- }
-
ish_resume_device = device;
dev->resume_flag = 1;
@@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
.id_table = ish_pci_tbl,
.probe = ish_probe,
.remove = ish_remove,
+ .shutdown = ish_shutdown,
.driver.pm = &ish_pm_ops,
};
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox