* RE: [PATCH v3 1/5] dt-bindings: soc: cix,sky1-system-control: add audss system control
From: Joakim Zhang @ 2026-06-11 11:56 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: mturquette@baylibre.com, sboyd@kernel.org, bmasney@redhat.com,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
p.zabel@pengutronix.de, Gary Yang, cix-kernel-upstream,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20260611-gorgeous-macho-cricket-f1b78c@quoll>
Hi,
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, June 11, 2026 3:40 PM
> To: Joakim Zhang <joakim.zhang@cixtech.com>
> Cc: mturquette@baylibre.com; sboyd@kernel.org; bmasney@redhat.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; Gary Yang <gary.yang@cixtech.com>; cix-kernel-
> upstream <cix-kernel-upstream@cixtech.com>; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3 1/5] dt-bindings: soc: cix,sky1-system-control: add audss
> system control
>
> EXTERNAL EMAIL
>
> On Wed, Jun 10, 2026 at 03:56:41PM +0800, joakim.zhang@cixtech.com wrote:
> > From: Joakim Zhang <joakim.zhang@cixtech.com>
> >
> > The Cix Sky1 Audio Subsystem (AUDSS) groups audio-related clock, reset
> > and control registers in a dedicated CRU block. Software reset lines
> > are exposed on the syscon parent via #reset-cells, following the same
> > model as the existing Sky1 FCH and S5 system control bindings.
> >
> > Add the cix,sky1-audss-system-control compatible to
> > cix,sky1-system-control.yaml for the MFD/syscon parent node, and
> > define AUDSS software reset indices in
> > include/dt-bindings/reset/cix,sky1-audss-system-control.h for I2S,
> > HDA, DMAC, mailbox, watchdog and timer blocks.
>
> All this is pretty pointless - you explained the binding, which answers nothing
> why you did it that way. Instead you must explain the hardware design.
>
> >
> > Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> > ---
> > .../soc/cix/cix,sky1-system-control.yaml | 52 +++++++++++++++++--
> > .../reset/cix,sky1-audss-system-control.h | 25 +++++++++
> > 2 files changed, 72 insertions(+), 5 deletions(-) create mode 100644
> > include/dt-bindings/reset/cix,sky1-audss-system-control.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.ya
> > ml
> > b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.ya
> > ml index a01a515222c6..61d26a69fd44 100644
> > ---
> > a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.ya
> > ml
> > +++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-contro
> > +++ l.yaml
> > @@ -15,11 +15,16 @@ description:
> >
> > properties:
> > compatible:
> > - items:
> > - - enum:
> > - - cix,sky1-system-control
> > - - cix,sky1-s5-system-control
> > - - const: syscon
> > + oneOf:
> > + - items:
> > + - enum:
> > + - cix,sky1-system-control
> > + - cix,sky1-s5-system-control
> > + - const: syscon
> > + - items:
> > + - const: cix,sky1-audss-system-control
> > + - const: simple-mfd
>
> Just so you are aware - this means children do not depend on the parent for
> operation. You will not be able to fix it later, if it turns out that children do
> depend...
Understood. simple-mfd is intentional: the clock child only accesses the parent MMIO via syscon and external resets/clocks via phandles. No parent driver coordination is needed today. We attached all resources audss needed from child node now.
> > + - const: syscon
> >
> > reg:
> > maxItems: 1
> > @@ -27,6 +32,28 @@ properties:
> > '#reset-cells':
> > const: 1
> >
> > + clock-controller:
> > + type: object
> > + properties:
> > + compatible:
> > + const: cix,sky1-audss-clock
> > + required:
> > + - compatible
> > + additionalProperties: true
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: cix,sky1-audss-system-control
> > + then:
> > + required:
> > + - clock-controller
> > + else:
> > + properties:
> > + clock-controller: false
> > +
> > required:
> > - compatible
> > - reg
> > @@ -40,3 +67,18 @@ examples:
> > reg = <0x4160000 0x100>;
> > #reset-cells = <1>;
> > };
> > + - |
> > + audss_syscon: system-controller@7110000 {
> > + compatible = "cix,sky1-audss-system-control", "simple-mfd", "syscon";
> > + reg = <0x7110000 0x10000>;
> > + #reset-cells = <1>;
> > +
> > + clock-controller {
> > + compatible = "cix,sky1-audss-clock";
> > + power-domains = <&smc_devpd 0>;
>
> My questions from v2 from the other patch are still valid - why audss system
> clock controller is outside of the power domain? Why the audss reset is outside,
> but audss clock not?
>
> This does not feel like correct hardware representation.
Yes, I agree with your point. This does not really reflect the hardware well. Both noc reset and power-domain takes effect on audss, should move to parent node.
Thanks,
Joakim
^ permalink raw reply
* Re: [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY
From: Konrad Dybcio @ 2026-06-11 11:54 UTC (permalink / raw)
To: Andrew Lunn, George Moussalem, Kathiravan Thirumoorthy
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Florian Fainelli, Bjorn Andersson, Konrad Dybcio,
netdev, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <afdced5b-73b9-4214-a94a-c13fadd39dce@lunn.ch>
On 6/5/26 8:14 PM, Andrew Lunn wrote:
>>>> This PHY is integrated into the IPQ5018 SoC, connected to the first GMAC
>>>> (GMAC0) and probed upon boot. However, this PHY is not used on all
>>>> boards because an external PHY or switch can be wired to the SoC's
>>>> second GMAC instead (through a PCS). So from a power management
>>>> perspective, it would be better if we can disable the clocks if there's
>>>> no link detected.
>>>
>>> Humm, is link the correct criteria? If the PHY is not used,
>>> .config_aneg should not be called. Why not have the probe method get
>>> the optional clocks, but leave them off. When .config_aneg is called
>>> for the first time, enable the clocks?
>>
>> Will check if config_aneg is called and test accordingly.
>>
>> ip link set eth0 up/down and cable (un)plug do trigger
>> link_change_notify, and based on the link state the RX/TX clocks are
>> turned off/on properly.
>
> You are talking about something else here. You say the device is not
> used. If it is not used, .config_aneg should not be called.
>
> This is a second use case, the device is used, and you want to limit
> the power it consumes, when there is no link. Do you have any numbers?
> How much power is actually saved?
If the PHY is part of the SoC, keeping it online would require some
more hw to be online, so it probably sums up.. I don't know for sure,
I don't really work with the router SoCs
Is there any prior art wrt enabling/disabling the PHYs (not necessarily
clocks specifically, but say power supplies) at runtime?
A quick grep only points to this very driver, which gets the regulator
during probe, enables it and never turns it off
Maybe +Kathiravan knows more?
Konrad
^ permalink raw reply
* Re: [PATCH v8 1/3] dt-bindings: timer: mips,p8700-gcru
From: Aleksa Paunovic @ 2026-06-11 11:51 UTC (permalink / raw)
To: krzk@kernel.org
Cc: Djordje Todorovic, Aleksa Paunovic, alex@ghiti.fr,
aou@eecs.berkeley.edu, cfu@mips.com, conor+dt@kernel.org,
conor.dooley@microchip.com, daniel.lezcano@linaro.org,
devicetree@vger.kernel.org, jstultz@google.com,
krzk+dt@kernel.org, linux-kernel@vger.kernel.org,
linux-riscv@lists.infradead.org, palmer@dabbelt.com,
paul.walmsley@sifive.com, pjw@kernel.org, robh@kernel.org,
sboyd@kernel.org, tglx@linutronix.de, wangruikang@iscas.ac.cn
In-Reply-To: <a2b5c9b4-5fc0-4507-a221-8695ed9574d8@kernel.org>
On 6/11/26 09:50, Krzysztof Kozlowski wrote:
> On 11/06/2026 09:39, Aleksa Paunovic wrote:
>> Hi Krzysztof,
>>
>>
>> On 6/11/26 08:54, Krzysztof Kozlowski wrote:
>>> On 11/06/2026 08:51, Krzysztof Kozlowski wrote:
>>>> On 10/06/2026 10:22, Aleksa Paunovic via B4 Relay wrote:
>>>>> From: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
>>>>>
>>>>> Add dt-bindings for the GCR.U memory mapped timer device for RISC-V
>>>>> platforms. The GCR.U memory region contains shadow copies of the RISC-V
>>>>> mtime register and the hrtime Global Configuration Register.
>>>>>
>>>>> Signed-off-by: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
>>>> You keep ignoring reviews you received (14th May!) and sending same mistake.
>>>>
>>>> Can you address the emails?
>> I wasn't really sure what the etiquette was for replying to Sashiko reviews, so I decided to
>> address the comments for other patches and send a v8 without replying.
>>
>> As for this patch, the GCR.U itself does start at 0x7F000, but the first
>> actual register (mtime) is at 0x7F050 [1].
>> I'm not seeing any warnings when running dt_binding_check.
> It's still a warning which you can easily reproduce on W=1 on dts. You
> CANNOT have mismatch. If block starts at 0x7f000, first register CANNOT
> start at different address or it completely does not matter where the
> register is. It's contradictory. The block start address defines
> where... does it start.
You are right: I just checked by manually running dtc and the warning's clearly there.
Will fix the alignment in v9. Thanks!
Best regards,
Aleksa
^ permalink raw reply
* Re: [PATCH v2 0/3] pinctrl: sunxi: a523: fix GPIO IRQ operation
From: Linus Walleij @ 2026-06-11 11:49 UTC (permalink / raw)
To: Andre Przywara
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, linux-gpio, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel
In-Reply-To: <20260327113006.3135663-1-andre.przywara@arm.com>
On Fri, Mar 27, 2026 at 12:30 PM Andre Przywara <andre.przywara@arm.com> wrote:
> this is the minimal fix version for the GPIO IRQ operation on the
> Allwinner A523/A527/T527 SoCs. SD card detection is broken as a result,
> which is a major annoyance. Those patches here fix that problem, and
> should go into v7.0 still, if possible.
Patches 1 & 2 applied to the pinctrl tree, please send patch 3 to
the SoC tree.
Sorry for missing this, dunno what happened. Probably it got
lost by me trying to use korgalore and screwing up.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v9 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
From: Lee Jones @ 2026-06-11 11:47 UTC (permalink / raw)
To: Edelweise Escala
Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-leds, devicetree, linux-kernel
In-Reply-To: <20260528-ltc3220-driver-v9-2-69450fc213cb@analog.com>
/* Sashiko Automation: Issues Found (10 Findings) */
Would you mind taking care of these before I conduct my next review please?
On Thu, 28 May 2026, Edelweise Escala wrote:
> Add driver for the LTC3220 18-channel LED driver
> with I2C interface, individual brightness control, and hardware-assisted
> blink/gradation features.
>
> Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
> ---
> MAINTAINERS | 1 +
> drivers/leds/Kconfig | 13 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-ltc3220.c | 440 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 455 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c8a242577d2f..0f553ada61d9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15229,6 +15229,7 @@ L: linux-leds@vger.kernel.org
> S: Maintained
> W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> +F: drivers/leds/leds-ltc3220.c
>
> LTC4282 HARDWARE MONITOR DRIVER
> M: Nuno Sa <nuno.sa@analog.com>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f4a0a3c8c870..31b1e3ff094c 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -1000,6 +1000,19 @@ config LEDS_ST1202
> Say Y to enable support for LEDs connected to LED1202
> LED driver chips accessed via the I2C bus.
>
> +config LEDS_LTC3220
> + tristate "LED Driver for Analog Devices Inc. LTC3220"
> + depends on I2C && LEDS_CLASS
> + select REGMAP_I2C
> + help
> + Say Y to enable support for the Analog Devices LTC3220
> + 18-channel LED controller with I2C interface.
> + The driver supports individual LED brightness control (64 steps),
> + hardware-assisted blinking and gradation effects.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called leds-ltc3220.
> +
> config LEDS_TPS6105X
> tristate "LED support for TI TPS6105X"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 7db3768912ca..a68244bd50fb 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_LP8788) += leds-lp8788.o
> obj-$(CONFIG_LEDS_LP8860) += leds-lp8860.o
> obj-$(CONFIG_LEDS_LP8864) += leds-lp8864.o
> obj-$(CONFIG_LEDS_LT3593) += leds-lt3593.o
> +obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
> obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
> obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
> obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
> diff --git a/drivers/leds/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
> new file mode 100644
> index 000000000000..a633ac5ca6ae
> --- /dev/null
> +++ b/drivers/leds/leds-ltc3220.c
> @@ -0,0 +1,440 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LTC3220 18-Channel LED Driver
> + *
> + * Copyright 2026 Analog Devices Inc.
> + *
> + * Author: Edelweise Escala <edelweise.escala@analog.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
[Severity: Low]
Is this include needed?
It doesn't appear that any delay functions like msleep() or usleep_range()
are used in the driver implementation.
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
[Severity: Low]
Should this file include <linux/property.h>?
The driver uses fwnode property APIs such as device_for_each_child_node_scoped(),
fwnode_property_read_u32(), and fwnode_property_present(), which are defined
in <linux/property.h>.
> +
> +/* LTC3220 Registers */
> +#define LTC3220_COMMAND_REG 0x00
> +#define LTC3220_QUICK_WRITE_MASK BIT(0)
> +#define LTC3220_SHUTDOWN_MASK BIT(3)
> +
> +#define LTC3220_ULED_REG(x) (0x01 + (x))
> +#define LTC3220_LED_CURRENT_MASK GENMASK(5, 0)
> +#define LTC3220_LED_MODE_MASK GENMASK(7, 6)
> +
> +#define LTC3220_GRAD_BLINK_REG 0x13
> +#define LTC3220_GRADATION_MASK GENMASK(2, 0)
> +#define LTC3220_GRADATION_DIRECTION_MASK BIT(0)
> +#define LTC3220_GRADATION_PERIOD_MASK GENMASK(2, 1)
> +#define LTC3220_BLINK_MASK GENMASK(4, 3)
> +
> +#define LTC3220_NUM_LEDS 18
> +
> +#define LTC3220_GRADATION_START_VALUE 128
[Severity: Low]
Is this macro used anywhere in the driver?
It appears to be dead code as it is never referenced.
> +#define LTC3220_GRADATION_RAMP_TIME_240MS 240
> +#define LTC3220_GRADATION_RAMP_TIME_480MS 480
> +
> +#define LTC3220_BLINK_ON_156MS 156
> +#define LTC3220_BLINK_ON_625MS 625
> +#define LTC3220_BLINK_PERIOD_1250MS 1250
> +#define LTC3220_BLINK_PERIOD_2500MS 2500
> +
> +#define LTC3220_BLINK_SHORT_ON_TIME BIT(0)
> +#define LTC3220_BLINK_LONG_PERIOD BIT(1)
> +
> +enum ltc3220_led_mode {
> + LTC3220_NORMAL_MODE,
> + LTC3220_BLINK_MODE,
> + LTC3220_GRADATION_MODE,
> +};
> +
> +enum ltc3220_blink_mode {
> + LTC3220_BLINK_MODE_625MS_1250MS,
> + LTC3220_BLINK_MODE_156MS_1250MS,
> + LTC3220_BLINK_MODE_625MS_2500MS,
> + LTC3220_BLINK_MODE_156MS_2500MS
> +};
> +
> +enum ltc3220_gradation_mode {
> + LTC3220_GRADATION_MODE_DISABLED,
> + LTC3220_GRADATION_MODE_240MS_RAMP_TIME,
> + LTC3220_GRADATION_MODE_480MS_RAMP_TIME,
> + LTC3220_GRADATION_MODE_960MS_RAMP_TIME
> +};
> +
> +static bool ltc3220_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + return reg == LTC3220_GRAD_BLINK_REG;
> +}
[Severity: High]
Does this register need to be marked volatile?
Marking a register as volatile tells regmap to bypass the cache and read from
the physical hardware. If the LTC3220 is a write-only I2C device, the physical
read will fail, causing regmap_update_bits() to fail entirely in
ltc3220_pattern_set() and ltc3220_blink_set().
Even if the device supports reads, this register is exclusively controlled by
software and does not change autonomously, so could it be cached normally to
avoid unnecessary I2C reads?
> +
> +static const struct regmap_config ltc3220_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = LTC3220_GRAD_BLINK_REG,
> + .cache_type = REGCACHE_FLAT_S,
> + .volatile_reg = ltc3220_volatile_reg,
> +};
> +
> +struct ltc3220_uled_cfg {
> + struct led_classdev led_cdev;
> + u8 reg_value;
> + u8 led_index;
> +};
> +
> +struct ltc3220 {
> + struct ltc3220_uled_cfg uled_cfg[LTC3220_NUM_LEDS];
> + struct regmap *regmap;
> + bool is_aggregated;
> +};
> +
> +/*
> + * Set LED brightness. Hardware supports 0-63 brightness levels.
> + * Mode switching (blink/gradation) is handled through dedicated callbacks.
> + */
> +static int ltc3220_set_led_data(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> + led_cdev);
> + struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
> + uled_cfg[uled_cfg->led_index]);
[Severity: Low]
Will this dynamic variable index inside container_of() cause a build failure?
The driver attempts to retrieve the parent structure using
uled_cfg[uled_cfg->led_index]. The container_of macro relies on offsetof(),
which strictly requires the member argument to be a constant expression.
Since uled_cfg->led_index is a runtime variable, this violates the C standard
and can abort compilation on compilers like Clang with an error about a
non-constant integer expression.
> + int ret;
> +
> + ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
> + brightness);
> + if (ret)
> + return ret;
> +
> + uled_cfg->reg_value = brightness;
> +
> + /*
> + * When aggregated LED mode is enabled, writing to LED 1 updates all
> + * LEDs simultaneously via quick-write mode. Update cached values for
> + * all LEDs to reflect the synchronized state.
> + * See Documentation/devicetree/bindings/leds/adi,ltc3220.yaml for how
> + * to configure aggregated LED mode.
> + */
> + if (ltc3220->is_aggregated && uled_cfg->led_index == 0) {
> + for (int i = 0; i < LTC3220_NUM_LEDS; i++)
> + ltc3220->uled_cfg[i].reg_value = brightness;
> + }
> +
> + return 0;
> +}
> +
> +static enum led_brightness ltc3220_get_led_data(struct led_classdev *led_cdev)
> +{
> + struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> + led_cdev);
> +
> + return uled_cfg->reg_value;
> +}
> +
> +/*
> + * LTC3220 pattern support for hardware-assisted breathing/gradation.
> + * The hardware supports 3 gradation ramp time 240ms, 480ms, 960ms)
> + * and can ramp up or down.
> + *
> + * Pattern array interpretation:
> + * pattern[0].brightness = start brightness (0-63)
> + * pattern[0].delta_t = ramp time in milliseconds
> + * pattern[1].brightness = end brightness (0-63)
> + * pattern[1].delta_t = (optional, can be 0 or same as pattern[0].delta_t)
> + */
> +static int ltc3220_pattern_set(struct led_classdev *led_cdev,
> + struct led_pattern *pattern,
> + u32 len, int repeat)
> +{
> + struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> + led_cdev);
> + struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
> + uled_cfg[uled_cfg->led_index]);
[Severity: Low]
Will this dynamic variable index inside container_of() cause a build failure?
The driver attempts to retrieve the parent structure using
uled_cfg[uled_cfg->led_index]. The container_of macro relies on offsetof(),
which strictly requires the member argument to be a constant expression.
Since uled_cfg->led_index is a runtime variable, this violates the C standard
and can abort compilation on compilers like Clang with an error about a
non-constant integer expression.
> + u8 gradation_period;
> + u8 start_brightness;
> + u8 end_brightness;
> + u8 gradation_val;
> + bool is_increasing;
> + int ret;
> +
> + if (len != 2)
> + return -EINVAL;
> +
> + start_brightness = pattern[0].brightness & LTC3220_LED_CURRENT_MASK;
> + end_brightness = pattern[1].brightness & LTC3220_LED_CURRENT_MASK;
> +
> + is_increasing = end_brightness > start_brightness;
> +
> + if (pattern[0].delta_t == 0)
> + gradation_period = LTC3220_GRADATION_MODE_DISABLED;
> + else if (pattern[0].delta_t <= LTC3220_GRADATION_RAMP_TIME_240MS)
> + gradation_period = LTC3220_GRADATION_MODE_240MS_RAMP_TIME;
> + else if (pattern[0].delta_t <= LTC3220_GRADATION_RAMP_TIME_480MS)
> + gradation_period = LTC3220_GRADATION_MODE_480MS_RAMP_TIME;
> + else
> + gradation_period = LTC3220_GRADATION_MODE_960MS_RAMP_TIME;
> +
> + gradation_val = FIELD_PREP(LTC3220_GRADATION_PERIOD_MASK, gradation_period);
> + gradation_val |= FIELD_PREP(LTC3220_GRADATION_DIRECTION_MASK, is_increasing);
> +
> + ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
> + LTC3220_GRADATION_MASK, gradation_val);
> + if (ret)
> + return ret;
> +
> + ret = ltc3220_set_led_data(led_cdev, start_brightness);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
> + FIELD_PREP(LTC3220_LED_MODE_MASK, LTC3220_GRADATION_MODE) |
> + end_brightness);
> + if (ret)
> + return ret;
> +
> + uled_cfg->reg_value = end_brightness;
> +
> + return 0;
> +}
> +
> +static int ltc3220_pattern_clear(struct led_classdev *led_cdev)
> +{
> + struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> + led_cdev);
> + struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
> + uled_cfg[uled_cfg->led_index]);
[Severity: Low]
Will this dynamic variable index inside container_of() cause a build failure?
The driver attempts to retrieve the parent structure using
uled_cfg[uled_cfg->led_index]. The container_of macro relies on offsetof(),
which strictly requires the member argument to be a constant expression.
Since uled_cfg->led_index is a runtime variable, this violates the C standard
and can abort compilation on compilers like Clang with an error about a
non-constant integer expression.
> + int ret;
> +
> + ret = regmap_update_bits(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
> + LTC3220_LED_MODE_MASK, LTC3220_NORMAL_MODE);
> + if (ret)
> + return ret;
> +
> + return ltc3220_set_led_data(led_cdev, LED_OFF);
> +}
> +
> +/*
> + * LTC3220 has a global blink configuration that affects all LEDs.
> + * This implementation allows per-LED blink requests, but the blink timing
> + * will be shared across all LEDs. The delay values are mapped to the
> + * hardware's discrete blink rates.
> + */
> +static int ltc3220_blink_set(struct led_classdev *led_cdev,
> + unsigned long *delay_on,
> + unsigned long *delay_off)
> +{
> + struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> + led_cdev);
> + struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
> + uled_cfg[uled_cfg->led_index]);
[Severity: Low]
Will this dynamic variable index inside container_of() cause a build failure?
The driver attempts to retrieve the parent structure using
uled_cfg[uled_cfg->led_index]. The container_of macro relies on offsetof(),
which strictly requires the member argument to be a constant expression.
Since uled_cfg->led_index is a runtime variable, this violates the C standard
and can abort compilation on compilers like Clang with an error about a
non-constant integer expression.
> + u8 blink_mode = 0;
> + int ret;
> +
> + if (*delay_on <= LTC3220_BLINK_ON_156MS)
> + blink_mode = LTC3220_BLINK_SHORT_ON_TIME;
> +
> + if (*delay_on + *delay_off > LTC3220_BLINK_PERIOD_1250MS)
> + blink_mode |= LTC3220_BLINK_LONG_PERIOD;
> +
> + switch (blink_mode) {
> + case LTC3220_BLINK_MODE_625MS_1250MS:
> + *delay_on = LTC3220_BLINK_ON_625MS;
> + *delay_off = LTC3220_BLINK_PERIOD_1250MS - LTC3220_BLINK_ON_625MS;
> + break;
> + case LTC3220_BLINK_MODE_156MS_1250MS:
> + *delay_on = LTC3220_BLINK_ON_156MS;
> + *delay_off = LTC3220_BLINK_PERIOD_1250MS - LTC3220_BLINK_ON_156MS;
> + break;
> + case LTC3220_BLINK_MODE_625MS_2500MS:
> + *delay_on = LTC3220_BLINK_ON_625MS;
> + *delay_off = LTC3220_BLINK_PERIOD_2500MS - LTC3220_BLINK_ON_625MS;
> + break;
> + case LTC3220_BLINK_MODE_156MS_2500MS:
> + *delay_on = LTC3220_BLINK_ON_156MS;
> + *delay_off = LTC3220_BLINK_PERIOD_2500MS - LTC3220_BLINK_ON_156MS;
> + break;
> + }
> +
> + ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
> + LTC3220_BLINK_MASK, FIELD_PREP(LTC3220_BLINK_MASK, blink_mode));
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
> + LTC3220_LED_MODE_MASK,
> + FIELD_PREP(LTC3220_LED_MODE_MASK, LTC3220_BLINK_MODE));
> +}
[Severity: Medium]
Will the hardware blink activate at 0 brightness if the LED was previously off?
When the LED is off, its current bits in LTC3220_ULED_REG are 0. The driver
updates the mode bits to LTC3220_BLINK_MODE using regmap_update_bits() but
does not update the brightness bits.
Because the core expects the driver to handle brightness configuration for
hardware blinking, could this hardware blink execute at 0 brightness, leaving
the LED unlit?
> +
> +static void ltc3220_reset_gpio_action(void *data)
> +{
> + struct gpio_desc *reset_gpio = data;
> +
> + gpiod_set_value_cansleep(reset_gpio, 1);
> +}
> +
> +static int ltc3220_reset(struct ltc3220 *ltc3220, struct i2c_client *client)
> +{
> + struct gpio_desc *reset_gpio;
> + int ret;
> +
> + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(reset_gpio))
> + return dev_err_probe(&client->dev, PTR_ERR(reset_gpio), "Failed on reset GPIO\n");
> +
> + if (reset_gpio) {
> + gpiod_set_value_cansleep(reset_gpio, 0);
[Severity: High]
Does this sequence lack the necessary delays to ensure a successful reset?
The driver asserts the reset GPIO by requesting it with GPIOD_OUT_HIGH and
immediately de-asserts it with gpiod_set_value_cansleep(reset_gpio, 0). There
is no delay between these calls, creating a near-zero width pulse that may fail
to meet the IC's minimum reset pulse width requirement.
It also lacks a wakeup delay before subsequent I2C commands might be sent.
> +
> + return devm_add_action_or_reset(&client->dev, ltc3220_reset_gpio_action,
> + reset_gpio);
[Severity: High]
Does returning early here skip software initialization and regmap cache setup?
If the reset_gpio is successfully acquired, this function immediately returns
0, entirely skipping the subsequent regmap_write() calls intended to initialize
the hardware registers.
Because the driver does not define a .reg_defaults table, skipping these writes
leaves the regmap cache uninitialized, which can break subsequent
read-modify-write logic via regmap_update_bits().
> + }
> +
> + ret = regmap_write(ltc3220->regmap, LTC3220_COMMAND_REG, 0);
> + if (ret)
> + return ret;
> +
> + for (int i = 0; i < LTC3220_NUM_LEDS; i++) {
> + ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(i), 0);
> + if (ret)
> + return ret;
> + }
> +
> + return regmap_write(ltc3220->regmap, LTC3220_GRAD_BLINK_REG, 0);
> +}
> +
> +static int ltc3220_suspend(struct device *dev)
> +{
> + struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> +
> + return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> + LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
> +}
> +
> +static int ltc3220_resume(struct device *dev)
> +{
> + struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> +
> + return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> + LTC3220_SHUTDOWN_MASK, 0);
> +}
[Severity: High]
Are the hardware registers left permanently off after resume?
In ltc3220_suspend(), the driver sets the LTC3220_SHUTDOWN_MASK bit. On many
such chips, asserting the shutdown bit resets internal registers. Additionally,
power to the IC may be lost during system suspend.
In ltc3220_resume(), the driver clears the shutdown bit but does not call
regcache_mark_dirty() and regcache_sync(). Could this leave the physical LED
outputs permanently off, while the kernel's regmap cache and LED classdev state
incorrectly reflect that the LEDs are still configured and illuminated?
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ltc3220_pm_ops, ltc3220_suspend, ltc3220_resume);
> +
> +static int ltc3220_probe(struct i2c_client *client)
> +{
> + struct ltc3220 *ltc3220;
> + bool aggregated_led_found = false;
> + int num_leds = 0;
> + u8 led_index = 0;
> + int ret;
> +
> + ltc3220 = devm_kzalloc(&client->dev, sizeof(*ltc3220), GFP_KERNEL);
> + if (!ltc3220)
> + return -ENOMEM;
> +
> + ltc3220->regmap = devm_regmap_init_i2c(client, <c3220_regmap_config);
> + if (IS_ERR(ltc3220->regmap))
> + return dev_err_probe(&client->dev, PTR_ERR(ltc3220->regmap),
> + "Failed to initialize regmap\n");
> +
> + i2c_set_clientdata(client, ltc3220);
> +
> + ret = ltc3220_reset(ltc3220, client);
> + if (ret)
> + return dev_err_probe(&client->dev, ret, "Failed to reset device\n");
> +
> + device_for_each_child_node_scoped(&client->dev, child) {
> + struct led_init_data init_data = {};
> + struct ltc3220_uled_cfg *led;
> + u32 source;
> +
> + ret = fwnode_property_read_u32(child, "reg", &source);
> + if (ret)
> + return dev_err_probe(&client->dev, ret, "Couldn't read LED address\n");
> +
> + if (!source || source > LTC3220_NUM_LEDS)
> + return dev_err_probe(&client->dev, -EINVAL, "LED address out of range\n");
> +
> + init_data.fwnode = child;
> + init_data.devicename = "ltc3220";
> +
> + if (fwnode_property_present(child, "led-sources")) {
> + if (source != 1)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "Aggregated LED out of range\n");
> +
> + if (aggregated_led_found)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "One Aggregated LED only\n");
> +
> + aggregated_led_found = true;
> + ltc3220->is_aggregated = true;
> +
> + ret = regmap_update_bits(ltc3220->regmap,
> + LTC3220_COMMAND_REG,
> + LTC3220_QUICK_WRITE_MASK,
> + LTC3220_QUICK_WRITE_MASK);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Failed to set quick write mode\n");
> + }
> +
> + num_leds++;
> +
> + /* LED node reg/index/address goes from 1 to 18 */
> + led_index = source - 1;
> + led = <c3220->uled_cfg[led_index];
> +
> + if (led->led_cdev.dev)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "Duplicate LED reg %u found", source);
> +
> + led->led_index = led_index;
> + led->reg_value = 0;
> + led->led_cdev.brightness_set_blocking = ltc3220_set_led_data;
> + led->led_cdev.brightness_get = ltc3220_get_led_data;
> + led->led_cdev.max_brightness = 63;
> + led->led_cdev.blink_set = ltc3220_blink_set;
> + led->led_cdev.pattern_set = ltc3220_pattern_set;
> + led->led_cdev.pattern_clear = ltc3220_pattern_clear;
> +
> + ret = devm_led_classdev_register_ext(&client->dev, &led->led_cdev, &init_data);
> + if (ret)
> + return dev_err_probe(&client->dev, ret, "Failed to register LED class\n");
> + }
> +
> + /*
> + * Aggregated LED mode uses hardware quick-write to control all 18 LEDs
> + * simultaneously. This is mutually exclusive with individual LED control.
> + * See Documentation/devicetree/bindings/leds/adi,ltc3220.yaml for details
> + * on how to configure aggregated LED mode.
> + */
> + if (aggregated_led_found && num_leds > 1)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "Aggregated LED must be the only LED node\n");
[Severity: Medium]
Are the LED sysfs interfaces exposed to userspace before the driver probe is
fully validated?
Inside the loop above, devm_led_classdev_register_ext() is called, which
immediately exposes sysfs interfaces. If the validation check here fails and
returns an error, userspace could have already discovered and interacted with
the LED devices before they are unexpectedly torn down by devres.
Could these validation checks be completed before registering the interfaces?
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ltc3220_of_match[] = {
> + { .compatible = "adi,ltc3220" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ltc3220_of_match);
> +
> +static struct i2c_driver ltc3220_led_driver = {
> + .driver = {
> + .name = "ltc3220",
> + .of_match_table = ltc3220_of_match,
> + .pm = pm_sleep_ptr(<c3220_pm_ops),
> + },
> + .probe = ltc3220_probe,
> +};
> +module_i2c_driver(ltc3220_led_driver);
> +
> +MODULE_AUTHOR("Edelweise Escala <edelweise.escala@analog.com>");
> +MODULE_DESCRIPTION("LED driver for LTC3220 controllers");
> +MODULE_LICENSE("GPL");
>
> --
> 2.43.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v3 04/18] firmware: qcom: scm: Add minidump SRAM support
From: Konrad Dybcio @ 2026-06-11 11:45 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-5-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:49 PM, Mukesh Ojha wrote:
> On most Qualcomm SoCs where minidump is supported, a word in always-on
> SRAM is shared between the kernel and boot firmware. Before DDR is
> initialised on the warm reset following a crash, firmware reads this
> word to decide if minidump is enabled and collect a minidump and where
> to deliver it (USB upload to a host, or save to local storage).
>
> The SRAM region is described by a 'sram' phandle on the SCM DT node.
> If the property is absent the feature is silently disabled, keeping
> existing SoCs unaffected.
>
> Expose a 'minidump_dest' module parameter (default: usb) so the user can
> select the destination. Only the string names "usb" or "storage" are
> acceptable values.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
[...]
> + for (i = 0; i < ARRAY_SIZE(minidump_dest_map); i++)
> + if (sysfs_streq(val, minidump_dest_map[i].name))
I'm not sure about sysfs_streq() specifically, but otherwise this lgtm
Konrad
^ permalink raw reply
* Re: [PATCH v3 17/18] arm64: dts: qcom: qcs615: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:37 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-18-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 07/18] arm64: dts: qcom: sa8775p: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:37 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-8-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:49 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 08/18] arm64: dts: qcom: qcs8300: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:37 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-9-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:49 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 04/21] pinctrl: starfive: Add StarFive JHB100 sys0 controller driver
From: Linus Walleij @ 2026-06-11 11:37 UTC (permalink / raw)
To: Changhuang Liang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Emil Renner Berthing, Paul Walmsley, Albert Ou, Palmer Dabbelt,
Alexandre Ghiti, Philipp Zabel, Bartosz Golaszewski, linux-gpio,
linux-kernel, devicetree, linux-riscv, Lianfeng Ouyang
In-Reply-To: <20260603055347.66845-5-changhuang.liang@starfivetech.com>
Hi Changhuang,
thanks for your patch!
On Wed, Jun 3, 2026 at 7:54 AM Changhuang Liang
<changhuang.liang@starfivetech.com> wrote:
> Add pinctrl driver for StarFive JHB100 SoC System-0(sys0) pinctrl
> controller.
>
> Co-developed-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
This patch adds generic infrastructure "JHB100" that is then used
by several drivers does it not?
Write something about that and some about the design in the
commit message.
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jhb100-sys0.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Pinctrl / GPIO driver for StarFive JHB100 SoC System-0 domain
> + *
> + * Copyright (C) 2024 StarFive Technology Co., Ltd.
> + * Author: Alex Soo <yuklin.soo@starfivetech.com>
Shouldn't this person be in the Signed-off-by?
I guess it's not legally necessary but feels appropriate.
> +static struct config_reg_layout_desc jhb100_sys0_pinctrl_crl_desc[] = {
> + {
> + .pin_start = 0,
> + .pin_cnt = 4,
> + .drive_strength_2bit = { .shift = 0, .width = 2 },
> + .input_enable = { .shift = 2, .width = 1 },
> + .pull_down = { .shift = 3, .width = 1 },
> + .pull_up = { .shift = 4, .width = 1 },
> + .slew_rate = { .shift = 5, .width = 1 },
> + .schmitt_trigger_select = { .shift = 6, .width = 1 },
> + .reserved = { .shift = 7, .width = 8 },
> + .debounce_width = { .shift = 15, .width = 17 },
> + },
> + {
> + .pin_start = 4,
> + .pin_cnt = 5,
> + .schmitt_trigger_select = { .shift = 0, .width = 1 },
> + .reserved = { .shift = 1, .width = 31 },
> + },
> + {
> + .pin_start = 9,
> + .pin_cnt = 1,
> + .drive_strength_2bit = { .shift = 0, .width = 2 },
> + .slew_rate = { .shift = 2, .width = 1 },
> + .reserved = { .shift = 3, .width = 29 },
> + },
> + {
> + .pin_start = 10,
> + .pin_cnt = 1,
> + .drive_strength_2bit = { .shift = 0, .width = 2 },
> + .input_enable = { .shift = 2, .width = 1 },
> + .pull_down = { .shift = 3, .width = 1 },
> + .pull_up = { .shift = 4, .width = 1 },
> + .slew_rate = { .shift = 5, .width = 1 },
> + .schmitt_trigger_select = { .shift = 6, .width = 1 },
> + .reserved = { .shift = 7, .width = 25 },
> + },
> + { 0xff },
> +};
Would it be appropriate to index the different register variants with
a enum with a good name so it is easy to understand which
variant each entry in the array is?
> +#include <linux/string.h>
> +#include <linux/sort.h>
Hm why... I guess I will see.
> +#define JHB100_DEBOUNCE_WIDTH_STAGES_MAX 0x1FFFFU
Is that a GENMASK(16,0)?
Since it seems to have something to do with bitfield widths.
> +/* i2c open-drain pull-up select */
> +#define JHB100_I2C_OPEN_DRAIN_PU_600_OHMS 0
> +#define JHB100_I2C_OPEN_DRAIN_PU_900_OHMS 1
> +#define JHB100_I2C_OPEN_DRAIN_PU_1200_OHMS 2
> +#define JHB100_I2C_OPEN_DRAIN_PU_2000_OHMS 3
Very nice and to the point! It's easy to read and understand drivers
that are writing things out explicitly like this!
> +#define JHB100_NR_GPIOS_PER_BANK 32
(...)
> +static inline struct jhb100_gpio_bank *jhb100_gc_to_bank(struct gpio_chip *gc)
> +{
> + return container_of(gc, struct jhb100_gpio_bank, gc);
> +}
> +
> +static unsigned int jhb100_gpio_to_pin(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct jhb100_gpio_bank *bank = jhb100_gc_to_bank(gc);
> +
> + return bank->id * JHB100_NR_GPIOS_PER_BANK + gpio;
> +}
This usually tells me that GPIO_GENERIC can be used but maybe
this has been discussed before...
> +static const struct pinctrl_ops jhb100_pinctrl_ops = {
> + .get_groups_count = pinctrl_generic_get_group_count,
> + .get_group_name = pinctrl_generic_get_group_name,
> + .get_group_pins = pinctrl_generic_get_group_pins,
> + .dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map,
> + .dt_free_map = pinctrl_utils_free_map,
> +};
Nice use of the generic helpers!
> +static void jhb100_set_gpioval(struct jhb100_pinctrl *sfp, unsigned int pin,
> + unsigned int val)
> +{
> + const struct jhb100_pinctrl_domain_info *info = sfp->info;
> + unsigned int offset = 4 * (pin / 32);
> + unsigned int shift = 1 * (pin % 32);
> + unsigned int fs_offset = 4 * (pin / 16);
> + unsigned int fs_shift = 2 * (pin % 16);
> + u32 func_sel_mask;
> + u32 dout, doen, fs;
> + void __iomem *reg_gpio_o;
> + void __iomem *reg_gpio_oen;
> + void __iomem *reg_gpio_func_sel;
> + unsigned long flags;
> +
> + reg_gpio_o = sfp->base + info->regs->output + offset;
> + reg_gpio_oen = sfp->base + info->regs->output_en + offset;
> + reg_gpio_func_sel = sfp->base + info->regs->func_sel.reg + fs_offset;
The part from here:
> + func_sel_mask = GENMASK(info->regs->func_sel.width_per_pin - 1, 0) << fs_shift;
(...)
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + fs = readl_relaxed(reg_gpio_func_sel);
> + if (fs & func_sel_mask) {
> + fs &= ~func_sel_mask;
> + writel_relaxed(fs, reg_gpio_func_sel);
> + }
..to here seems to reimplement the shortcut
.gpio_request_enable() in struct pinmux_ops.
Then this:
> + dout = val << shift;
> + doen = 0;
> + dout |= readl_relaxed(reg_gpio_o) & ~BIT(shift);
> + writel_relaxed(dout, reg_gpio_o);
> + doen |= readl_relaxed(reg_gpio_oen) & ~BIT(shift);
> + writel_relaxed(doen, reg_gpio_oen);
Seems more like the actual code that should be here.
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
Please use guards for these spinlocks. They make for less
bugs.
guard(raw_spinlock_irqsave)(&sfp->lock);
> +static const struct pinmux_ops jhb100_pinmux_ops = {
> + .get_functions_count = pinmux_generic_get_function_count,
> + .get_function_name = pinmux_generic_get_function_name,
> + .get_function_groups = pinmux_generic_get_function_groups,
> + .set_mux = jhb100_set_mux,
> +};
Implement .gpio_request_enable() (see above) and
.gpio_set_direction() see below.
Maybe also .gpio_disable_free() if you need to deconfigure
stuff when a pin is release from GPIO.
> +static const struct pinconf_ops jhb100_pinconf_ops = {
> + .pin_config_get = jhb100_pinconf_get,
> + .pin_config_set = jhb100_pinconf_set,
> + .pin_config_group_get = jhb100_pinconf_group_get,
> + .pin_config_group_set = jhb100_pinconf_group_set,
> + .is_generic = true,
> +};
Overall this looks nice, good use of the group config!
> +static int jhb100_gpio_get_direction(struct gpio_chip *gc,
> + unsigned int gpio)
> +{
> + struct jhb100_gpio_bank *bank = jhb100_gc_to_bank(gc);
> + struct jhb100_pinctrl *sfp = gpiochip_get_data(gc);
> + const struct jhb100_pinctrl_domain_info *info = sfp->info;
> + unsigned int offset = 4 * bank->id;
> + u32 doen;
> + void __iomem *reg_gpio_oen;
> +
> + reg_gpio_oen = sfp->base + info->regs->output_en + offset;
> +
> + doen = (readl_relaxed(reg_gpio_oen) & BIT(gpio)) >> gpio;
> +
> + return doen == GPOEN_ENABLE ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int jhb100_gpio_direction_input(struct gpio_chip *gc,
> + unsigned int gpio)
> +{
> + struct jhb100_pinctrl *sfp = gpiochip_get_data(gc);
> + struct device *dev = sfp->dev;
> + struct config_reg_layout_desc *crl_desc;
> + unsigned int pin = jhb100_gpio_to_pin(gc, gpio);
> +
> + crl_desc = get_crl_desc_by_pin(sfp, pin);
> + if (!crl_desc) {
> + dev_err(dev, "pin %d can't not found reg layout descriptor\n",
> + pin);
> + return -EINVAL;
> + }
> +
> + jhb100_padcfg_rmw(sfp, pin,
> + RL_DESC_GENMASK(crl_desc, input_enable) |
> + RL_DESC_GENMASK(crl_desc, schmitt_trigger_select),
> + RL_DESC_GENMASK(crl_desc, input_enable) |
> + RL_DESC_GENMASK(crl_desc, schmitt_trigger_select));
Instead of doing these writes directly into the config registers, implement
.gpio_set_direction() in struct pinmux_ops and call the pinmux
generic back-end.
> +static int jhb100_gpio_direction_output(struct gpio_chip *gc,
> + unsigned int gpio, int value)
> +{
> + struct jhb100_pinctrl *sfp = gpiochip_get_data(gc);
> + struct device *dev = sfp->dev;
> + struct config_reg_layout_desc *crl_desc;
> + unsigned int pin = jhb100_gpio_to_pin(gc, gpio);
> +
> + jhb100_set_one_pin_mux(sfp, pin, 0,
> + value ? GPOUT_HIGH : GPOUT_LOW);
> +
> + crl_desc = get_crl_desc_by_pin(sfp, pin);
> + if (!crl_desc) {
> + dev_err(dev, "pin %d can't not found reg layout descriptor\n",
> + pin);
> + return -EINVAL;
> + }
> +
> + jhb100_padcfg_rmw(sfp, pin,
> + RL_DESC_GENMASK(crl_desc, input_enable) |
> + RL_DESC_GENMASK(crl_desc, schmitt_trigger_select) |
> + RL_DESC_GENMASK(crl_desc, pull_down) |
> + RL_DESC_GENMASK(crl_desc, pull_up),
> + 0);
Dito.
> +static int jhb100_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct jhb100_gpio_bank *bank = jhb100_gc_to_bank(gc);
> + struct jhb100_pinctrl *sfp = gpiochip_get_data(gc);
> + const struct jhb100_pinctrl_domain_info *info = sfp->info;
> + unsigned int offset = 4 * bank->id;
> + u32 doen = 0;
> + void __iomem *reg_gpio_oen;
> + void __iomem *reg;
> + unsigned long flags;
> +
> + reg_gpio_oen = sfp->base + info->regs->output_en + offset;
> + reg = sfp->base + info->regs->gpio_status + offset;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + doen = readl_relaxed(reg_gpio_oen) | BIT(gpio);
> + writel_relaxed(doen, reg_gpio_oen);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
Why *on* *earth* are you read-modify-writing the output enable
register in the *get* function? Is this a copy-on-paste error??
> + return !!(readl_relaxed(reg) & BIT(gpio % 32));
Also you never actuall read reg .... ehhhh this is a glaring bug.
> +static int jhb100_gpio_set(struct gpio_chip *gc, unsigned int gpio, int value)
> +{
> + struct jhb100_gpio_bank *bank = jhb100_gc_to_bank(gc);
> + struct jhb100_pinctrl *sfp = gpiochip_get_data(gc);
> + const struct jhb100_pinctrl_domain_info *info = sfp->info;
> + unsigned int offset = 4 * bank->id;
> + void __iomem *reg_dout;
> + u32 dout;
> + unsigned long flags;
> +
> + reg_dout = sfp->base + info->regs->output + offset;
> + dout = (value ? GPOUT_HIGH : GPOUT_LOW) << gpio;
> +
> + raw_spin_lock_irqsave(&sfp->lock, flags);
> + dout |= readl_relaxed(reg_dout) & ~BIT(gpio);
> + writel_relaxed(dout, reg_dout);
> + raw_spin_unlock_irqrestore(&sfp->lock, flags);
> +
> + return 0;
> +}
This looks right, did you only test output and not input..?
> +static const struct irq_chip jhb100_irq_chip = {
> + .irq_ack = jhb100_irq_ack,
> + .irq_mask = jhb100_irq_mask,
> + .irq_mask_ack = jhb100_irq_mask_ack,
> + .irq_unmask = jhb100_irq_unmask,
> + .irq_set_type = jhb100_irq_set_type,
> + .irq_set_wake = jhb100_irq_set_wake,
> + .irq_print_chip = jhb100_irq_print_chip,
> + .flags = IRQCHIP_SET_TYPE_MASKED |
> + IRQCHIP_IMMUTABLE |
> + IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND |
> + IRQCHIP_MASK_ON_SUSPEND |
> + IRQCHIP_SKIP_SET_WAKE,
> + GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
The irqchip looks good!
> +static int field_compare(const void *a, const void *b)
> +{
> + const struct field_info *fa = (const struct field_info *)a;
> + const struct field_info *fb = (const struct field_info *)b;
> +
> + if (fa->shift < fb->shift)
> + return -1;
> +
> + if (fa->shift > fb->shift)
> + return 1;
> +
> + return 0;
> +}
Are you sure the kernel doesn't already have a helper like this...
> + sfp->num_banks = DIV_ROUND_UP(sfp->ngpios, JHB100_NR_GPIOS_PER_BANK);
> +
> + for (unsigned int i = 0; i < sfp->num_banks; i++) {
> + if (sfp->ngpios > (i + 1) * JHB100_NR_GPIOS_PER_BANK)
> + sfp->banks[i].gc.ngpio = (i + 1) * JHB100_NR_GPIOS_PER_BANK;
> + else
> + sfp->banks[i].gc.ngpio = sfp->ngpios - i * JHB100_NR_GPIOS_PER_BANK;
This looks completely bananas, shouldn't this be simply:
sfp->banks[i].gc.ngpio = JHB100_NR_GPIOS_PER_BANK;
???
What is getting assigned to ngpios looks like a gpiochip base, and have
all the signs of a real bad AI hallucination.
> +
> + sfp->banks[i].id = i;
> +
> + sfp->banks[i].gc.parent = dev;
> + sfp->banks[i].gc.label = dev_name(dev);
> + sfp->banks[i].gc.owner = THIS_MODULE;
> + sfp->banks[i].gc.request = pinctrl_gpio_request;
Use
gpiochip_generic_request
> + sfp->banks[i].gc.free = pinctrl_gpio_free;
Use
gpiochip_generic_free
These calls will do what you want, and also check that the
right gpio ranges are available.
Make sure you add GPIO ranges (the mapping between pin control
pins and corresponding GPIO offsets) for this to work properly.
I'm pretty sure you can have a generic pin config backend as well.
sfp->banks[i].gc.set_config = gpiochip_generic_config;
This will make config calls to the gpiochip call into the pinctrl
backend = what you want.
> + sfp->banks[i].gc.get_direction = jhb100_gpio_get_direction;
> + sfp->banks[i].gc.direction_input = jhb100_gpio_direction_input;
> + sfp->banks[i].gc.direction_output = jhb100_gpio_direction_output;
> + sfp->banks[i].gc.get = jhb100_gpio_get;
> + sfp->banks[i].gc.set = jhb100_gpio_set;
> + sfp->banks[i].gc.set_config = gpiochip_generic_config;
> + sfp->banks[i].gc.base = -1;
> + sfp->banks[i].gc.of_gpio_n_cells = 3;
> + sfp->banks[i].gc.of_node_instance_match = starfive_of_node_instance_match;
Since you have a threecell scheme with 32 gpios
(JHB100_NR_GPIOS_PER_BANK) per instance (right? the ngpios
code above made me really confused....)
you should be able so select GPIO_GENERIC,
#include <linux/gpio/generic.h> and use
the generic GPIO pretty much the same way
drivers/gpio/gpio-spacemit-k1.c does it, check that driver
out (especially spacemit_gpio_add_bank()).
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 09/18] arm64: dts: qcom: qdu1000: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:37 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-10-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 10/18] arm64: dts: qcom: sm8550: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:37 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-11-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 11/18] arm64: dts: qcom: sm8650: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:36 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-12-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 12/18] arm64: dts: qcom: sc7280: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:36 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-13-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 14/18] arm64: dts: qcom: sc7180: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:36 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-15-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 15/18] arm64: dts: qcom: sm6350: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:35 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-16-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 16/18] arm64: dts: qcom: sm6375: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:35 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-17-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 18/18] arm64: dts: qcom: sdm845: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:34 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-19-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 13/18] arm64: dts: qcom: sm8350: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:34 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-14-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:50 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 06/18] arm64: dts: qcom: sm8450: Add minidump SRAM config to SCM node
From: Konrad Dybcio @ 2026-06-11 11:34 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko,
Guru Das Srinagesh
Cc: cros-qcom-dts-watchers, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260522195009.2961022-7-mukesh.ojha@oss.qualcomm.com>
On 5/22/26 9:49 PM, Mukesh Ojha wrote:
> Point the SCM node at the minidump config slot in the always-on SRAM.
> Boot firmware reads this word before DDR is initialised on a warm reset
> to decide where to deliver the minidump.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 3/8] pinctrl: qcom: Register functions before enabling pinctrl
From: Konrad Dybcio @ 2026-06-11 11:32 UTC (permalink / raw)
To: contact, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, MyungJoo Ham, Chanwoo Choi,
Guru Das Srinagesh, Linus Walleij, Rob Clark, Joerg Roedel,
Will Deacon, Robin Murphy, Kees Cook, Tony Luck,
Guilherme G. Piccoli
Cc: linux-arm-msm, devicetree, linux-kernel, linux-gpio, iommu,
phone-devel
In-Reply-To: <20260519-mainline-send-v1-sending-v3-3-3dd7aa125353@alex-min.fr>
On 5/19/26 9:16 AM, Alexandre MINETTE via B4 Relay wrote:
> From: Alexandre MINETTE <contact@alex-min.fr>
>
> pinctrl consumers can request states while the pinctrl core enables the
> controller. On Qualcomm pinctrl drivers this can happen before the SoC
> function list has been registered, which leaves the function table
> incomplete during state lookup.
>
> On APQ8064 this can fail while claiming pinctrl hogs:
>
> apq8064-pinctrl 800000.pinctrl: invalid function ps_hold in map table
> apq8064-pinctrl 800000.pinctrl: error claiming hogs: -22
> apq8064-pinctrl 800000.pinctrl: could not claim hogs: -22
>
> Register Qualcomm pinctrl with devm_pinctrl_register_and_init(), add the
> SoC pin functions, and only then enable the pinctrl device.
>
> Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH V11 7/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607
From: Jonathan Cameron @ 2026-06-11 11:31 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan
In-Reply-To: <20260610175455.19006-8-macroalpha82@gmail.com>
On Wed, 10 Jun 2026 12:54:51 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add icm42607 accelerometer sensor for icm42607.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Minor things inline.
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> new file mode 100644
> index 000000000000..cb60bb5ecc14
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> @@ -0,0 +1,379 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/iio/iio.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +#include "inv_icm42607.h"
> +#include "inv_icm42607_temp.h"
> +
> +#define INV_ICM42607_ACCEL_CHAN(_modifier, _index, _ext_info) \
> +{ \
> + .type = IIO_ACCEL, \
> + .modified = 1, \
> + .channel2 = _modifier, \
> + .info_mask_separate = \
> + BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_type_available = \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_all = \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .info_mask_shared_by_all_available = \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
See other comments on this is is 'all' and perhaps applies to temperature.
If it doesn't apply to temperature then it may need splitting up.
Little less clear cut for mount matrix, so I don't mind that one just
> + .scan_index = _index, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 16, \
> + .storagebits = 16, \
> + .endianness = IIO_BE, \
> + }, \
> + .ext_info = _ext_info, \
> +}
> +static int inv_icm42607_accel_write_odr(struct iio_dev *indio_dev,
> + int val, int val2)
> +{
> + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> + struct device *dev = regmap_get_device(st->map);
> + unsigned int idx;
> + struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> + int ret;
> +
> + for (idx = 5; idx < ARRAY_SIZE(inv_icm42607_accel_odr); ++idx) {
> + if (val == inv_icm42607_accel_odr[idx][0] &&
> + val2 == inv_icm42607_accel_odr[idx][1])
> + break;
> + }
> + if (idx >= ARRAY_SIZE(inv_icm42607_accel_odr))
> + return -EINVAL;
> +
> + conf.odr = idx;
> +
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&st->lock);
> +
> + ret = inv_icm42607_set_accel_conf(st, &conf, NULL);
> + if (ret)
> + return ret;
> +
> + return 0;
If this isn't getting more complex later,
return inv_...
> +}
> +
> +static int inv_icm42607_accel_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> + s16 data;
> + int ret;
> +
> + switch (chan->type) {
> + case IIO_ACCEL:
> + break;
> + case IIO_TEMP:
> + return inv_icm42607_temp_read_raw(indio_dev, chan, val, val2, mask);
I commented on this in previous patch, but once the shared_by_all is
added to the temp channel, this needs modifying to ensure we only call the
temp handler for cases that aren't shared_by_all.
> + default:
> + return -EINVAL;
> + }
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
As below - drop these mode claims as for now there is only one mode.
> + ret = inv_icm42607_accel_read_sensor(indio_dev, chan, &data);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> + *val = data;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + return inv_icm42607_accel_read_scale(indio_dev, val, val2);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return inv_icm42607_accel_read_odr(st, val, val2);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int inv_icm42607_accel_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals,
> + int *type, int *length, long mask)
> +{
> + if (chan->type != IIO_ACCEL)
> + return -EINVAL;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (const int *)inv_icm42607_accel_scale_nano;
> + *type = IIO_VAL_INT_PLUS_NANO;
> + *length = ARRAY_SIZE(inv_icm42607_accel_scale_nano) * 2;
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *vals = (const int *)inv_icm42607_accel_odr[5];
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + *length = (ARRAY_SIZE(inv_icm42607_accel_odr) - 5) * 2;
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int inv_icm42607_accel_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + int ret;
> +
> + if (chan->type != IIO_ACCEL)
> + return -EINVAL;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + if (!iio_device_claim_direct(indio_dev))
As in previous, this stuff doesn't belong in a driver that is always in direct
mode.
> + return -EBUSY;
> + ret = inv_icm42607_accel_write_scale(indio_dev, val, val2);
> + iio_device_release_direct(indio_dev);
> + return ret;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return inv_icm42607_accel_write_odr(indio_dev, val, val2);
> + default:
> + return -EINVAL;
> + }
> +struct iio_dev *inv_icm42607_accel_init(struct inv_icm42607_state *st)
> +{
> + struct device *dev = regmap_get_device(st->map);
> + const char *name;
where no other strong reason for ordering, reverse xmas tree.
> + struct inv_icm42607_sensor_state *accel_st;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + name = devm_kasprintf(dev, GFP_KERNEL, "%s-accel", st->hw->name);
> + if (!name)
> + return ERR_PTR(-ENOMEM);
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*accel_st));
> + if (!indio_dev)
> + return ERR_PTR(-ENOMEM);
> + accel_st = iio_priv(indio_dev);
> +
> + accel_st->power_mode = INV_ICM42607_SENSOR_MODE_LOW_NOISE;
> + accel_st->filter = INV_ICM42607_FILTER_BW_73HZ;
> +
> + iio_device_set_drvdata(indio_dev, st);
> + indio_dev->name = name;
> + indio_dev->info = &inv_icm42607_accel_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = inv_icm42607_accel_channels;
> + indio_dev->num_channels = ARRAY_SIZE(inv_icm42607_accel_channels);
> +
> + ret = devm_iio_device_register(dev, indio_dev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return indio_dev;
> +}
^ permalink raw reply
* Re: [PATCH v8 5/7] leds: Add driver for ASUS Transformer LEDs
From: Lee Jones @ 2026-06-11 11:30 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260528053203.9339-6-clamor95@gmail.com>
On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> ASUS Transformer tablets have a green and an amber LED on both the Pad
> and the Dock. If both LEDs are enabled simultaneously, the emitted light
> will be yellow.
>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/leds/Kconfig | 11 +++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-asus-transformer-ec.c | 125 ++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 drivers/leds/leds-asus-transformer-ec.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f4a0a3c8c870..f637d23400a8 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -120,6 +120,17 @@ config LEDS_OSRAM_AMS_AS3668
> To compile this driver as a module, choose M here: the module
> will be called leds-as3668.
>
> +config LEDS_ASUS_TRANSFORMER_EC
> + tristate "LED Support for Asus Transformer charging LED"
> + depends on LEDS_CLASS
> + depends on MFD_ASUS_TRANSFORMER_EC
> + help
> + This option enables support for charging indicator on
> + Asus Transformer's Pad and it's Dock.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called leds-asus-transformer-ec.
> +
> config LEDS_AW200XX
> tristate "LED support for Awinic AW20036/AW20054/AW20072/AW20108"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 8fdb45d5b439..d5395c3f1124 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o
> obj-$(CONFIG_LEDS_APU) += leds-apu.o
> obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o
> obj-$(CONFIG_LEDS_AS3668) += leds-as3668.o
> +obj-$(CONFIG_LEDS_ASUS_TRANSFORMER_EC) += leds-asus-transformer-ec.o
> obj-$(CONFIG_LEDS_AW200XX) += leds-aw200xx.o
> obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o
> obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o
> diff --git a/drivers/leds/leds-asus-transformer-ec.c b/drivers/leds/leds-asus-transformer-ec.c
> new file mode 100644
> index 000000000000..09503e76331c
> --- /dev/null
> +++ b/drivers/leds/leds-asus-transformer-ec.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/err.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/asus-transformer-ec.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +enum {
> + ASUSEC_LED_AMBER,
> + ASUSEC_LED_GREEN,
> + ASUSEC_LED_MAX
> +};
> +
> +struct asus_ec_led_config {
> + const char *name;
> + unsigned int color;
> + unsigned long long ctrl_bit;
Should we use 'u64' here instead of 'unsigned long long' to align with standard
kernel integer types?
> +};
> +
> +struct asus_ec_led {
> + struct asus_ec_leds_data *ddata;
> + struct led_classdev cdev;
> + unsigned long long ctrl_bit;
Should we use 'u64' here as well to keep it consistent?
> +};
> +
> +struct asus_ec_leds_data {
> + const struct asusec_core *ec;
> + struct asus_ec_led leds[ASUSEC_LED_MAX];
> +};
> +
> +static const struct asus_ec_led_config asus_ec_leds[] = {
> + [ASUSEC_LED_AMBER] = {
> + .name = "amber",
> + .color = LED_COLOR_ID_AMBER,
> + .ctrl_bit = ASUSEC_CTL_LED_AMBER,
> + },
> + [ASUSEC_LED_GREEN] = {
> + .name = "green",
> + .color = LED_COLOR_ID_GREEN,
> + .ctrl_bit = ASUSEC_CTL_LED_GREEN,
> + },
> +};
> +
> +static enum led_brightness asus_ec_led_get_brightness(struct led_classdev *cdev)
> +{
> + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> + const struct asusec_core *ec = led->ddata->ec;
I'm getting confused here.
ddata is what I'd be calling the device data struct passed by the parent?
In fact, ddata is a little known concept in Leds. Any reason to go for
this over the standard nomenclature?
> + u64 ctl;
> + int ret;
> +
> + ret = asus_dockram_access_ctl(ec->dockram, &ctl, 0, 0);
Did we discuss preferring regmap already?
> + if (ret)
> + return LED_OFF;
> +
> + return ctl & led->ctrl_bit ? LED_ON : LED_OFF;
> +}
> +
> +static int asus_ec_led_set_brightness(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> + const struct asusec_core *ec = led->ddata->ec;
> +
> + if (brightness)
> + return asus_dockram_access_ctl(ec->dockram, NULL,
> + led->ctrl_bit, led->ctrl_bit);
> +
> + return asus_dockram_access_ctl(ec->dockram, NULL, led->ctrl_bit, 0);
> +}
> +
> +static int asus_ec_led_probe(struct platform_device *pdev)
> +{
> + const struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> + struct asus_ec_leds_data *ddata;
> + struct device *dev = &pdev->dev;
> + int i, ret;
Could we declare the loop counter 'i' directly within the 'for' statement's
scope to keep its scope limited? For example, 'for (int i = 0; ...)'.
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, ddata);
> + ddata->ec = ec;
> +
> + for (i = 0; i < ASUSEC_LED_MAX; i++) {
Nit: for (int i = ...
> + const struct asus_ec_led_config *cfg = &asus_ec_leds[i];
> + struct asus_ec_led *led = &ddata->leds[i];
> +
> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s::%s",
> + ddata->ec->name, cfg->name);
> + if (!led->cdev.name)
> + return -ENOMEM;
> +
> + led->cdev.max_brightness = 1;
> + led->cdev.color = cfg->color;
> + led->cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> + led->cdev.brightness_get = asus_ec_led_get_brightness;
> + led->cdev.brightness_set_blocking = asus_ec_led_set_brightness;
> +
> + led->ddata = ddata;
> + led->ctrl_bit = cfg->ctrl_bit;
> +
> + ret = devm_led_classdev_register(dev, &led->cdev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to register %s LED\n",
> + cfg->name);
Should we capitalise the error message here to match our style guidelines
(e.g. 'Failed to register...')?
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver asus_ec_led_driver = {
> + .driver.name = "asus-transformer-ec-led",
> + .probe = asus_ec_led_probe,
> +};
> +module_platform_driver(asus_ec_led_driver);
> +
> +MODULE_ALIAS("platform:asus-transformer-ec-led");
> +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> +MODULE_DESCRIPTION("ASUS Transformer's charging LED driver");
> +MODULE_LICENSE("GPL");
> --
> 2.51.0
>
>
--
Lee Jones
^ permalink raw reply
* RE: [PATCH v1 1/7] ASoC: dt-bindings: ES8389: Add members about HPF and clock
From: Zhang Yi @ 2026-06-11 11:30 UTC (permalink / raw)
To: krzk
Cc: broonie, conor+dt, devicetree, krzk+dt, linux-sound, robh, tiwai,
zhangyi
In-Reply-To: <20260611-mamba-of-legendary-anger-af76a9@quoll>
> So this is like third time you send the same.
>
> You never responded to feedback, I don't see improvements and you keep
> sending the same v1.
>
> Version your patches correctly - read help of git format-patch or just
> use b4.
>
> NAK again because you just ignore us.
Sorry, I was wondering if you received the following two emails
https://lore.kernel.org/all/20260610100637.25568-1-zhangyi@everest-semi.com/
https://lore.kernel.org/all/20260610095820.25386-1-zhangyi@everest-semi.com/
^ permalink raw reply
* [PATCH v1 2/2] ASoC: qcom: sc8280xp: add Shikra EVK machine variants
From: Ajay Kumar Nandam @ 2026-06-11 11:29 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-sound, linux-arm-msm, devicetree, linux-kernel, ajay.nandam,
Mohammad Rafi Shaik
In-Reply-To: <20260611112946.954172-1-ajay.nandam@oss.qualcomm.com>
Add machine-driver support for Qualcomm Shikra EVK variants by matching
dedicated compatible strings and applying board-specific audio behavior.
Shikra platforms are available as CQM, CQS, and IQS variants with
different audio components and processing architectures:
- CQM/CQS use an I2S-based path with WSA885x amplifiers and
PM4125 + Rouleur codec components.
- CQM runs in DSP-bypass mode, where the complete audio pipeline runs
on CPU only and no DSP is involved.
- CQS uses modem-DSP based audio processing.
- IQS uses a third-party MAX98091 codec with modem-DSP support.
Introduce variant-specific handling so codec controls, DAPM widgets, and
clock programming match the board design and avoid invalid clock setup on
variants where it is not applicable.
Co-developed-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
sound/soc/qcom/common.c | 2 ++
sound/soc/qcom/sc8280xp.c | 58 +++++++++++++++++++++++++++++++++++++--
sound/soc/qcom/sdw.c | 3 ++
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
index f42c98ded..32d6c09b2 100644
--- a/sound/soc/qcom/common.c
+++ b/sound/soc/qcom/common.c
@@ -3,6 +3,7 @@
// Copyright (c) 2018, The Linux Foundation. All rights reserved.
#include <dt-bindings/sound/qcom,q6afe.h>
+#include <dt-bindings/sound/qcom,qaif.h>
#include <linux/module.h>
#include <sound/jack.h>
#include <linux/input-event-codes.h>
@@ -430,6 +431,7 @@ int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd,
}
switch (cpu_dai->id) {
+ case QAIF_CDC_DMA_RX0:
case TX_CODEC_DMA_TX_0:
case TX_CODEC_DMA_TX_1:
case TX_CODEC_DMA_TX_2:
diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
index e5d23e244..4c985f81c 100644
--- a/sound/soc/qcom/sc8280xp.c
+++ b/sound/soc/qcom/sc8280xp.c
@@ -37,6 +37,25 @@ static struct snd_soc_dapm_widget sc8280xp_dapm_widgets[] = {
SND_SOC_DAPM_SPK("DP7 Jack", NULL),
};
+static struct snd_soc_dapm_widget shikra_cqm_dapm_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone Jack", NULL),
+ SND_SOC_DAPM_MIC("Mic Jack", NULL),
+};
+
+static const struct snd_soc_dapm_widget shikra_iqs_dapm_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone", NULL),
+ SND_SOC_DAPM_MIC("Headset Mic", NULL),
+ SND_SOC_DAPM_MIC("Int Mic", NULL),
+ SND_SOC_DAPM_SPK("Speaker", NULL),
+};
+
+static const struct snd_kcontrol_new shikra_iqs_controls[] = {
+ SOC_DAPM_PIN_SWITCH("Headset Mic"),
+ SOC_DAPM_PIN_SWITCH("Headphone"),
+ SOC_DAPM_PIN_SWITCH("Int Mic"),
+ SOC_DAPM_PIN_SWITCH("Speaker"),
+};
+
struct snd_soc_common {
const char *driver_name;
const struct snd_soc_dapm_widget *dapm_widgets;
@@ -49,6 +68,7 @@ struct snd_soc_common {
bool codec_sysclk_set;
bool mi2s_mclk_enable;
bool mi2s_bclk_enable;
+ bool dsp_bypass;
};
struct sc8280xp_snd_data {
@@ -219,6 +239,10 @@ static int sc8280xp_snd_hw_params(struct snd_pcm_substream *substream,
int mclk_freq = sc8280xp_get_mclk_freq(params);
int bclk_freq = sc8280xp_get_bclk_freq(params);
+ /* Skip DSP configuration when operating in CPU-only (bypass) mode */
+ if (data->snd_soc_common_priv->dsp_bypass)
+ return 0;
+
switch (cpu_dai->id) {
case PRIMARY_MI2S_RX ... QUATERNARY_MI2S_TX:
case QUINARY_MI2S_RX ... QUINARY_MI2S_TX:
@@ -239,7 +263,7 @@ static int sc8280xp_snd_hw_params(struct snd_pcm_substream *substream,
SND_SOC_CLOCK_IN);
if (data->snd_soc_common_priv->codec_sysclk_set)
- snd_soc_dai_set_sysclk(cpu_dai,
+ snd_soc_dai_set_sysclk(codec_dai,
0, mclk_freq,
SND_SOC_CLOCK_IN);
break;
@@ -284,7 +308,7 @@ static void sc8280xp_add_be_ops(struct snd_soc_card *card)
int i;
for_each_card_prelinks(card, i, link) {
- if (link->no_pcm == 1) {
+ if (link->no_pcm == 1 || link->num_codecs > 0) {
link->init = sc8280xp_snd_init;
link->be_hw_params_fixup = sc8280xp_be_hw_params_fixup;
link->ops = &sc8280xp_be_ops;
@@ -375,6 +399,33 @@ static struct snd_soc_common sc8280xp_priv_data = {
.num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
};
+static const struct snd_soc_common shikra_cqm_priv_data = {
+ .driver_name = "shikra",
+ .dapm_widgets = shikra_cqm_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(shikra_cqm_dapm_widgets),
+ .dsp_bypass = true,
+};
+
+static const struct snd_soc_common shikra_cqs_priv_data = {
+ .driver_name = "shikra",
+ .dapm_widgets = shikra_cqm_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(shikra_cqm_dapm_widgets),
+ .mi2s_bclk_enable = true,
+ .codec_sysclk_set = true,
+};
+
+static const struct snd_soc_common shikra_iqs_priv_data = {
+ .driver_name = "shikra",
+ .dapm_widgets = shikra_iqs_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(shikra_iqs_dapm_widgets),
+ .controls = shikra_iqs_controls,
+ .num_controls = ARRAY_SIZE(shikra_iqs_controls),
+ .codec_dai_fmt = SND_SOC_DAIFMT_CBP_CFP |
+ SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_I2S,
+ .codec_sysclk_set = true,
+ .mi2s_bclk_enable = true,
+};
+
static struct snd_soc_common sm8450_priv_data = {
.driver_name = "sm8450",
.dapm_widgets = sc8280xp_dapm_widgets,
@@ -408,6 +459,9 @@ static const struct of_device_id snd_sc8280xp_dt_match[] = {
{.compatible = "qcom,qcs9075-sndcard", .data = &qcs9100_priv_data},
{.compatible = "qcom,qcs9100-sndcard", .data = &qcs9100_priv_data},
{.compatible = "qcom,sc8280xp-sndcard", .data = &sc8280xp_priv_data},
+ {.compatible = "qcom,shikra-cqm-sndcard", .data = &shikra_cqm_priv_data},
+ {.compatible = "qcom,shikra-cqs-sndcard", .data = &shikra_cqs_priv_data},
+ {.compatible = "qcom,shikra-iqs-sndcard", .data = &shikra_iqs_priv_data},
{.compatible = "qcom,sm8450-sndcard", .data = &sm8450_priv_data},
{.compatible = "qcom,sm8550-sndcard", .data = &sm8550_priv_data},
{.compatible = "qcom,sm8650-sndcard", .data = &sm8650_priv_data},
diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c
index 6576b47a4..0be743cec 100644
--- a/sound/soc/qcom/sdw.c
+++ b/sound/soc/qcom/sdw.c
@@ -4,6 +4,7 @@
#include <dt-bindings/sound/qcom,lpass.h>
#include <dt-bindings/sound/qcom,q6afe.h>
+#include <dt-bindings/sound/qcom,qaif.h>
#include <linux/module.h>
#include <sound/soc.h>
#include "sdw.h"
@@ -41,6 +42,8 @@ static bool qcom_snd_is_sdw_dai(int id)
switch (id) {
case LPASS_CDC_DMA_TX3:
case LPASS_CDC_DMA_RX0:
+ case QAIF_CDC_DMA_VA_TX0:
+ case QAIF_CDC_DMA_RX0:
return true;
default:
break;
--
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