* Re: [PATCH v2 3/7] ASoC: codecs: ES8389: Fix the issue about mclk_src
From: sashiko-bot @ 2026-06-18 8:42 UTC (permalink / raw)
To: Zhang Yi; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260618082842.2778-4-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Modifying device tree property interpretation breaks existing configurations.
Pre-existing issues:
- [High] Incorrect BCLK (sysclk) calculation for mono streams.
--
commit 97d350d02faf890bb7862a3fb657b9cbe4613b33
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: codecs: ES8389: Fix the issue about mclk_src
Fix the issue with incorrect modifications to mclk_src.
When the system needs to be configured to use the MCLK from the SCLK pin,
the code still sets the relevant registers to use the MCLK from the MCLK
pin.
> diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
> --- a/sound/soc/codecs/es8389.h
> +++ b/sound/soc/codecs/es8389.h
[ ... ]
> @@ -116,9 +116,11 @@
> #define ES8389_TDM_SLOT (0x70 << 0)
> #define ES8389_TDM_SHIFT 4
>
> -#define ES8389_MCLK_SOURCE (1 << 6)
> -#define ES8389_MCLK_PIN (1 << 6)
> -#define ES8389_SCLK_PIN (0 << 6)
> +#define ES8389_MCLK_MASK (3 << 6)
> +#define ES8389_MCLK_FROM_SCLK (1 << 6)
> +#define ES8389_MCLK_SOURCE ES8389_MCLK_PIN
> +#define ES8389_MCLK_PIN 0
> +#define ES8389_SCLK_PIN 1
[ ... ]
> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
> @@ -607,9 +607,9 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
> regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
> regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
>
> - if (es8389->mclk_src == ES8389_SCLK_PIN) {
> + if (es8389->mclk_src) {
[Severity: Critical]
Does changing this condition and redefining the pin macros break existing
device tree configurations?
Previously in es8389.h, ES8389_SCLK_PIN was 0. If an existing device tree
explicitly selected SCLK using <0>, it would match the old condition.
Because ES8389_SCLK_PIN is now redefined as 1, an existing legacy DT passing
<0> will evaluate as false here and incorrectly fall back to MCLK.
Similarly, if a legacy configuration used <64> for MCLK, it would now
evaluate as true and incorrectly select SCLK. Is there a need for
compatibility fallback logic to prevent ABI breakage for deployed platforms?
> regmap_update_bits(es8389->regmap, ES8389_MASTER_CLK,
> - ES8389_MCLK_SOURCE, es8389->mclk_src);
> + ES8389_MCLK_MASK, ES8389_MCLK_FROM_SCLK);
> es8389->sysclk = params_channels(params) * params_width(params) * params_rate(params);
[Severity: High]
This is a pre-existing issue, but does this calculation result in an
incorrect sysclk for mono streams?
By using params_channels() in es8389_pcm_hw_params(), the physical clock
rate is derived from the logical channel count. For a mono stream, this
returns 1. However, on standard I2S buses, the physical frame always
contains 2 slots (left and right), meaning the actual physical bit clock on
the wire should be 2 * width * rate.
Could this cause the sysclk calculation to be halved, making it fail to
find a matching frequency and breaking mono playback?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=3
^ permalink raw reply
* Re: [PATCH v2 1/7] ASoC: dt-bindings: ES8389: Add members about HPF and clock
From: sashiko-bot @ 2026-06-18 8:43 UTC (permalink / raw)
To: Zhang Yi; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260618082842.2778-2-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Property `everest,mclk-src` uses an unidiomatic `uint8` type to model a boolean flag, which is highly error-prone due to DT endianness.
- [Low] Property `everest,hpf-frq` lacks the standard `-hz` unit suffix and uses an uncommon abbreviation.
--
commit 733db5a41798dd67759483d1a1aa543394f0ddf2
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: dt-bindings: ES8389: Add members about HPF and clock
Add members related to HPF and mclk_source
> diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> --- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> +++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> @@ -42,6 +42,21 @@ properties:
> description:
> Interface power supply.
>
> + everest,mclk-src:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description:
> + Indicates that SCLK is used as the internal clock.
> + minimum: 0
> + maximum: 0x01
> + default: 0x00
[Severity: Medium]
Since this functions conceptually as a boolean toggle, should this be
defined as a standard boolean property instead of a uint8?
If a developer intuitively writes "everest,mclk-src = <1>;" in the Device
Tree, standard angle-bracket integer cells evaluate to 32-bit big-endian
integers (0x00 0x00 0x00 0x01).
When device_property_read_u8() reads this in the driver:
ret = device_property_read_u8(component->dev, "everest,mclk-src", &es8389->mclk_src);
It will read the first byte (0x00) and silently misinterpret the flag as 0.
To correctly set the flag as a uint8, users would be forced to use the unwieldy
syntax "/bits/ 8 <1>;". Using a boolean property (where presence indicates true)
would prevent this silent configuration failure.
Also, prior discussions upstream noted a bitmask mismatch (0x40) related to this
property, but it was resolved in a subsequent patch in this series ("ASoC:
codecs: ES8389: Fix the issue about mclk_src").
> +
> + everest,hpf-frq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + The frequency of HPF in Hz.
> + maximum: 1020
> + default: 16
[Severity: Low]
Does this property name need the standard "-hz" unit suffix?
According to Device Tree schema guidelines, properties representing frequencies
should use the "-hz" suffix to be self-documenting (e.g., "everest,hpf-hz").
Additionally, "frq" is a non-standard abbreviation for frequency.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=1
^ permalink raw reply
* Re: [PATCH v2 2/7] ASoC: codecs: ES8389: Modify volatile_register
From: sashiko-bot @ 2026-06-18 8:45 UTC (permalink / raw)
To: Zhang Yi; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260618082842.2778-3-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The updated `es8389_volatile_register` list misses critical configuration registers, causing their state to be permanently lost upon resume.
- [High] Variables passed to `regmap_read` are used without checking for a potential error return, which can leave them uninitialized and result in non-deterministic hardware behavior.
- [High] The return value of `es8389_set_bias_level()` is ignored in `es8389_resume()`, potentially leading to silent failures.
--
commit d39c101472412561134bb4faff5fe29c9d7f7dfb
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: codecs: ES8389: Modify volatile_register
This commit updates the volatile register definitions for the ES8389 codec,
switching to an allowlist approach for cached registers. It also adjusts
the cache bypass logic during the resume sequence.
> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 3484c87853cb4..10de2143f8d8f 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
> @@ -50,10 +50,29 @@ static const char * const es8389_core_supplies[] = {
> static bool es8389_volatile_register(struct device *dev,
> unsigned int reg)
> {
> - if ((reg <= 0xff))
> - return true;
> - else
> + switch (reg) {
> + case ES8389_ADCL_VOL:
[ ... ]
> + case ES8389_ADC_MODE:
> + case ES8389_DMIC_EN:
> return false;
> + default:
> + return true;
> + }
[Severity: High]
This isn't a bug introduced by this patch, but by defaulting to true, do we
miss critical configuration registers like ES8389_MASTER_MODE and
ES8389_ADC_HPF2?
If these are left as volatile, they are bypassed during regcache_sync().
Since ASoC doesn't automatically re-invoke set_fmt on resume, and
es8389_init() overwrites these registers with reset defaults, does this
permanently lose the configuration across a suspend/resume cycle?
> }
>
> static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -9550, 50, 0);
> @@ -861,13 +880,13 @@ static int es8389_resume(struct snd_soc_component *component)
> regcache_cache_only(es8389->regmap, false);
> regcache_cache_bypass(es8389->regmap, true);
> regmap_read(es8389->regmap, ES8389_RESET, ®v);
[Severity: High]
This is a pre-existing issue, but should the return value of regmap_read()
be checked here?
If the underlying I2C/SPI read fails, regv remains uninitialized. This
could result in non-deterministic hardware behavior since the resume
sequence would evaluate stack garbage in the if (regv == 0xff) check below.
This also appears in other places, for example in es8389_init():
regmap_read(es8389->regmap, ES8389_MAX_REGISTER, ®);
es8389->version = reg;
> - regcache_cache_bypass(es8389->regmap, false);
>
> if (regv == 0xff)
> es8389_init(component);
> else
> es8389_set_bias_level(component, SND_SOC_BIAS_ON);
[Severity: High]
This is also a pre-existing issue, but what happens if es8389_set_bias_level()
encounters an error?
If clk_prepare_enable() fails inside es8389_set_bias_level(), it returns
an error code. Since that return value is ignored here, will the function
silently proceed to regcache_sync() and return success? This might leave
the hardware unpowered while the system falsely believes it resumed
correctly.
>
> + regcache_cache_bypass(es8389->regmap, false);
> regcache_sync(es8389->regmap);
>
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=2
^ permalink raw reply
* Re: [PATCH v4 resend 2/5] reset: cix: add audss support to sky1 reset driver
From: Philipp Zabel @ 2026-06-18 8:49 UTC (permalink / raw)
To: joakim.zhang, mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt,
gary.yang
Cc: cix-kernel-upstream, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20260617064100.1504617-3-joakim.zhang@cixtech.com>
On Mi, 2026-06-17 at 14:40 +0800, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> Extend the Sky1 reset controller driver for the AUDSS CRU syscon. The
> AUDSS block provides sixteen active-low software reset bits in one
> register for audio subsystem peripherals, reusing the existing
> regmap-based reset ops used by the FCH and S5 system control variants.
>
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> drivers/reset/reset-sky1.c | 86 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/reset/reset-sky1.c b/drivers/reset/reset-sky1.c
> index 78e80a533c39..af32ee005ebc 100644
> --- a/drivers/reset/reset-sky1.c
> +++ b/drivers/reset/reset-sky1.c
[...]
> @@ -343,21 +379,65 @@ static int sky1_reset_probe(struct platform_device *pdev)
> sky1src->rcdev.of_node = dev->of_node;
> sky1src->rcdev.dev = dev;
>
> - return devm_reset_controller_register(dev, &sky1src->rcdev);
> + ret = devm_reset_controller_register(dev, &sky1src->rcdev);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, sky1src);
> +
> + if (of_device_is_compatible(dev->of_node, "cix,sky1-audss-system-control")) {
The compatible was already evaluated by of_device_get_match_data(), you
could check (variant == &variant_sky1_audss) here.
regards
Philipp
^ permalink raw reply
* Re: [External] Re: [PATCH v4 2/3] riscv: track effective hardware PTE A/D updating
From: yunhui cui @ 2026-06-18 8:53 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260618070215.268ED1F000E9@smtp.kernel.org>
Hi sashiko,
On Thu, Jun 18, 2026 at 3:03 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] A hotplugged CPU lacking the Svadu extension is incorrectly allowed to come online when hardware PTE A/D updating is globally enabled but FWFT is not required, leading to silent data corruption.
> --
>
> commit e238566a69f2ecd235872518a759dd75d54f71af
> Author: Yunhui Cui <cuiyunhui@bytedance.com>
>
> riscv: track effective hardware PTE A/D updating
>
> This commit tracks the runtime state of hardware PTE A/D updating using a
> static key. It attempts to enable FWFT for all online harts, falling back
> to software A/D if that fails, and ensures hotplugged CPUs match the global
> state before coming online.
>
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
>
> [ ... ]
>
> > +int riscv_enable_hw_pte_ad_updating(void)
> > +{
> > + unsigned int cpu;
> > + int ret;
> > +
> > + if (!riscv_has_hw_pte_ad_updating() ||
> > + !riscv_hw_pte_ad_updating_requires_fwft)
> > + return 0;
>
> [Severity: High]
> Does this code allow a hotplugged CPU to come online without the Svadu
> extension when hardware A/D updating is globally enabled?
>
> If the boot CPUs have Svadu but lack Svade, the flag
> riscv_hw_pte_ad_updating_requires_fwft is set to false and the global
> static key riscv_hw_pte_ad_updating is enabled.
No, a hotplugged CPU should not be able to bypass this in the normal
RISC-V hotplug path. A CPU reaching smp_callin() must already be in
cpu_possible_mask, and the
global Svadu state is computed from the ISA intersection of all
possible CPUs. Therefore, if any possible hotplug CPU lacks Svadu,
riscv_has_extension_unlikely(SVADU)
is false and the HW A/D static key is never enabled.
>
> When a new CPU is later hotplugged, it calls this function via
> smp_callin(). Since riscv_hw_pte_ad_updating_requires_fwft is false, it
> returns 0 and bypasses any capability checks for the local CPU.
>
> If the hotplugged CPU does not actually support Svadu (e.g., in an
> asymmetric SoC or due to firmware misconfiguration), it successfully
> comes online. Because the kernel globally assumes hardware A/D updating
> is active, it skips software A/D fault handling.
>
> Could this lead to the hotplugged CPU silently failing to update
> accessed/dirty bits, causing data corruption when dirty data is dropped
> during reclaim?
>
> > +
> > + cpu = smp_processor_id();
> > + ret = riscv_enable_local_hw_pte_ad_updating();
> > + if (ret)
> > + pr_err("CPU%u failed to enable hardware PTE A/D updating: %d\n",
> > + cpu, ret);
> > +
> > + return ret;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260618064406.14508-1-cuiyunhui@bytedance.com?part=2
Thanks,
Yunhui
^ permalink raw reply
* Re: [PATCH v2 2/2] pinctrl: qcom: Add the tlmm driver for Maili platform
From: Konrad Dybcio @ 2026-06-18 9:02 UTC (permalink / raw)
To: Jingyi Wang, Bjorn Andersson, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: aiqun.yu, tingwei.zhang, trilok.soni, yijie.yang, linux-arm-msm,
linux-gpio, devicetree, linux-kernel
In-Reply-To: <20260614-maili-pinctrl-v2-2-0db5bfc23d64@oss.qualcomm.com>
On 6/15/26 8:55 AM, Jingyi Wang wrote:
> Add support for Maili TLMM configuration and control via the pinctrl
> framework.
>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> ---
[...]
> +/* Every pin is maintained as a single group, and missing or non-existing pin
> + * would be maintained as dummy group to synchronize pin group index with
> + * pin descriptor registered with pinctrl core.
> + * Clients would not be able to request these dummy pin groups.
> + */
Let's drop this comment
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 0/6] arm64: dts: qcom: sc8280xp: set GPI DMA channels according to DSDT
From: Konrad Dybcio @ 2026-06-18 9:04 UTC (permalink / raw)
To: Icenowy Zheng, Pengyu Luo
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <926a0fe9224d73e0f5e3f58f3769c6247b1cabd4.camel@iscas.ac.cn>
On 6/9/26 5:54 PM, Icenowy Zheng wrote:
> 在 2026-06-09二的 14:23 +0200，Konrad Dybcio写道：
>> On 6/7/26 10:49 AM, Icenowy Zheng wrote:
>>> 在 2026-06-06六的 21:51 +0800，Pengyu Luo写道：
>>>> On Sat, Jun 6, 2026 at 9:21 PM Icenowy Zheng
>>>> <zhengxingda@iscas.ac.cn> wrote:
>>>>>
>>>>> 在 2026-06-06六的 17:46 +0800，Pengyu Luo写道：
>>>>>> On 2026-06-06 17:28:35+08:00, Icenowy Zheng wrote:
>>>>>>> 在 2026-06-06六的 17:22 +0800，Pengyu Luo写道：
>>>>>>>
>>>>>>>> On 2026-06-02 21:21:27+08:00, Icenowy Zheng wrote:
>>>>>>>>
>>>>>>>> The magnetic keyboard (USB HID) can't be connected
>>>>>>>> somehow,
>>>>>>>> others
>>>>>>>> are
>>>>>>>> fine, such as the spi touchscreen (not upstream yet),
>>>>>>>> which
>>>>>>>> utilizes
>>>>>>>> DMA definitely. My config is here
>>>>>>>> https://pastebin.com/SdjuyJYk
>>>>>>>
>>>>>>> Is this a defconfig?
>>>>>>>
>>>>>>
>>>>>> Yes.
>>>>>>
>>>>>>> BTW it seems that CONFIG_ASYNC_TX_DMA needs to be selected
>>>>>>> too
>>>>>>> for
>>>>>>> exhibiting the problem (because there should be "public"
>>>>>>> GPI
>>>>>>> DMA
>>>>>>> consumers to trigger the stuck/reset).
>>>>>>>
>>>>>>
>>>>>> Is this still necessary? I checked the fedora discussion and
>>>>>> your
>>>>>> GPI
>>>>>> DMA fix. And GPI DMA is only for the QUP-supported
>>>>>> peripherals as
>>>>>> the
>>>>>> binding mentioned, devicetree/bindings/dma/qcom,gpi.yaml
>>>>>
>>>>> The devicetree without this fix seems to be still incorrect,
>>>>> because
>>>>> with the device tree fix even if the GPI DMA driver misbehaves
>>>>> the
>>>>> system won't be stuck (although it will iterate all GPI
>>>>> channels
>>>>> and
>>>>> then fail to function at all).
>>>>>
>>>>
>>>> Back to the start. You said some GPI interfaces aren't available
>>>> to
>>>> HLOS, your mask is 0xb(0b1011), so I use 0x4(0b100) did a quick
>>>> test,
>>>> and spi6 consumed it, no stuck or reset. Could you give me a
>>>> unavailable channel?
>>>
>>> I think channel 0b10000 of gpi_dma2 could be an example?
>>>
>>> It seems that 4 channels are tried on gpi_dma2 before hang on my
>>> gaokun3, but as gaokun3 has no known serial access, it's possible
>>> that
>>> 0b100000 or 0b1000 is problematic.
>>>
>>> (The reason gpi_dma2 is checked first is because it's the GPI DMA
>>> controller with the smallest address)
>>>
>>> BTW I just took the values from Windows DSDT, which is quite
>>> conservative.
>>
>> So, with DMA_PRIVATE set, is this series made redundant?
>
> I assume technically the trustzone is still protecting some channels,
> although the system stuck issue is fixed.
>
> This series should still be relevant, although not so emergent.
So now we're down to the case of the TZ reserving some of the GPI
channels (presumably for locked down/TZ-driven QUPs) crashing the
device on access, is that right?
Konrad
^ permalink raw reply
* Re: [PATCH 0/6] arm64: dts: qcom: sc8280xp: set GPI DMA channels according to DSDT
From: Konrad Dybcio @ 2026-06-18 9:05 UTC (permalink / raw)
To: Icenowy Zheng, Pengyu Luo
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <7ac67f89-c6b4-4e0c-8eec-1e5c757777ce@oss.qualcomm.com>
On 6/18/26 11:04 AM, Konrad Dybcio wrote:
> On 6/9/26 5:54 PM, Icenowy Zheng wrote:
>> 在 2026-06-09二的 14:23 +0200，Konrad Dybcio写道：
>>> On 6/7/26 10:49 AM, Icenowy Zheng wrote:
>>>> 在 2026-06-06六的 21:51 +0800，Pengyu Luo写道：
>>>>> On Sat, Jun 6, 2026 at 9:21 PM Icenowy Zheng
>>>>> <zhengxingda@iscas.ac.cn> wrote:
>>>>>>
>>>>>> 在 2026-06-06六的 17:46 +0800，Pengyu Luo写道：
>>>>>>> On 2026-06-06 17:28:35+08:00, Icenowy Zheng wrote:
>>>>>>>> 在 2026-06-06六的 17:22 +0800，Pengyu Luo写道：
>>>>>>>>
>>>>>>>>> On 2026-06-02 21:21:27+08:00, Icenowy Zheng wrote:
>>>>>>>>>
>>>>>>>>> The magnetic keyboard (USB HID) can't be connected
>>>>>>>>> somehow,
>>>>>>>>> others
>>>>>>>>> are
>>>>>>>>> fine, such as the spi touchscreen (not upstream yet),
>>>>>>>>> which
>>>>>>>>> utilizes
>>>>>>>>> DMA definitely. My config is here
>>>>>>>>> https://pastebin.com/SdjuyJYk
>>>>>>>>
>>>>>>>> Is this a defconfig?
>>>>>>>>
>>>>>>>
>>>>>>> Yes.
>>>>>>>
>>>>>>>> BTW it seems that CONFIG_ASYNC_TX_DMA needs to be selected
>>>>>>>> too
>>>>>>>> for
>>>>>>>> exhibiting the problem (because there should be "public"
>>>>>>>> GPI
>>>>>>>> DMA
>>>>>>>> consumers to trigger the stuck/reset).
>>>>>>>>
>>>>>>>
>>>>>>> Is this still necessary? I checked the fedora discussion and
>>>>>>> your
>>>>>>> GPI
>>>>>>> DMA fix. And GPI DMA is only for the QUP-supported
>>>>>>> peripherals as
>>>>>>> the
>>>>>>> binding mentioned, devicetree/bindings/dma/qcom,gpi.yaml
>>>>>>
>>>>>> The devicetree without this fix seems to be still incorrect,
>>>>>> because
>>>>>> with the device tree fix even if the GPI DMA driver misbehaves
>>>>>> the
>>>>>> system won't be stuck (although it will iterate all GPI
>>>>>> channels
>>>>>> and
>>>>>> then fail to function at all).
>>>>>>
>>>>>
>>>>> Back to the start. You said some GPI interfaces aren't available
>>>>> to
>>>>> HLOS, your mask is 0xb(0b1011), so I use 0x4(0b100) did a quick
>>>>> test,
>>>>> and spi6 consumed it, no stuck or reset. Could you give me a
>>>>> unavailable channel?
>>>>
>>>> I think channel 0b10000 of gpi_dma2 could be an example?
>>>>
>>>> It seems that 4 channels are tried on gpi_dma2 before hang on my
>>>> gaokun3, but as gaokun3 has no known serial access, it's possible
>>>> that
>>>> 0b100000 or 0b1000 is problematic.
>>>>
>>>> (The reason gpi_dma2 is checked first is because it's the GPI DMA
>>>> controller with the smallest address)
>>>>
>>>> BTW I just took the values from Windows DSDT, which is quite
>>>> conservative.
>>>
>>> So, with DMA_PRIVATE set, is this series made redundant?
>>
>> I assume technically the trustzone is still protecting some channels,
>> although the system stuck issue is fixed.
>>
>> This series should still be relevant, although not so emergent.
>
> So now we're down to the case of the TZ reserving some of the GPI
> channels (presumably for locked down/TZ-driven QUPs) crashing the
> device on access, is that right?
i.e. now, is requesting these channels through (wrongfully) enabling
the devices in DT the only remaining concern?
Konrad
^ permalink raw reply
* [PATCH v4 0/4] Enable SPI on SA8255p Qualcomm platforms
From: Praveen Talari @ 2026-06-18 9:06 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, bjorn.andersson, Praveen Talari
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel,
mukesh.savaliya, aniket.randive, chandana.chiluveru,
jyothi.seerapu, chiluka.harish, Nikunj Kela, Krzysztof Kozlowski
The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power
and performance protocols.
The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power states(on/off).
The SCMI performance protocol manages SPI frequency, with each
frequency rate represented by a performance level. The driver uses
geni_se_set_perf_opp() API to request the desired frequency rate.
As part of geni_se_set_perf_opp(), the OPP for the requested frequency
is obtained using dev_pm_opp_find_freq_floor() and the performance
level is set using dev_pm_opp_set_opp().
Praveen Talari (4):
spi: dt-bindings: describe SA8255p
spi: qcom-geni: Use geni_se_resources_init() for resource
initialization
spi: qcom-geni: Use resources helper APIs in runtime PM functions
spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
.../bindings/spi/qcom,sa8255p-geni-spi.yaml | 63 ++++++++++++++
drivers/spi/spi-geni-qcom.c | 83 ++++++++-----------
2 files changed, 97 insertions(+), 49 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml
---
Changes in v4:
- Added Reviewed-by tags.
- Added return check for dev_data variable.
- Link to v3: https://lore.kernel.org/all/20260604-enable-spi-on-sa8255p-v3-0-43984eac4c67@oss.qualcomm.com
Changes in v3:
- Added the OPP rate restoration in resume callback.
- Added missed dma-names in example node.
- Link to v2: https://patch.msgid.link/20260530-enable-spi-on-sa8255p-v2-0-17574601bd63@oss.qualcomm.com
Changes in v2:
- Rebased patches on latest linux-next.
- Link to v1: https://lore.kernel.org/all/20260112190134.1526646-1-praveen.talari@oss.qualcomm.com/
To: Mark Brown <broonie@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
To: bjorn.andersson@oss.qualcomm.com
To: Praveen Talari <praveen.talari@oss.qualcomm.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: mukesh.savaliya@oss.qualcomm.com
Cc: aniket.randive@oss.qualcomm.com
Cc: chandana.chiluveru@oss.qualcomm.com
Cc: jyothi.seerapu@oss.qualcomm.com
Cc: chiluka.harish@oss.qualcomm.com
---
Praveen Talari (4):
spi: dt-bindings: describe SA8255p
spi: qcom-geni: Use geni_se_resources_init() for resource initialization
spi: qcom-geni: Use resources helper APIs in runtime PM functions
spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
.../bindings/spi/qcom,sa8255p-geni-spi.yaml | 64 ++++++++++++++++
drivers/spi/spi-geni-qcom.c | 86 ++++++++++------------
2 files changed, 104 insertions(+), 46 deletions(-)
---
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
change-id: 20260529-enable-spi-on-sa8255p-8166eaa226cb
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Praveen Talari (4):
spi: dt-bindings: describe SA8255p
spi: qcom-geni: Use geni_se_resources_init() for resource initialization
spi: qcom-geni: Use resources helper APIs in runtime PM functions
spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
.../bindings/spi/qcom,sa8255p-geni-spi.yaml | 64 ++++++++++++++++
drivers/spi/spi-geni-qcom.c | 88 +++++++++++-----------
2 files changed, 106 insertions(+), 46 deletions(-)
---
base-commit: 4fa3f5fabb30bf00d7475d5a33459ea83d639bf9
change-id: 20260529-enable-spi-on-sa8255p-8166eaa226cb
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v4 1/4] spi: dt-bindings: describe SA8255p
From: Praveen Talari @ 2026-06-18 9:06 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, bjorn.andersson, Praveen Talari
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel,
mukesh.savaliya, aniket.randive, chandana.chiluveru,
jyothi.seerapu, chiluka.harish, Nikunj Kela, Krzysztof Kozlowski
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com>
Add DT bindings for the QUP GENI SPI controller on sa8255p platform.
SA8255p platform abstracts resources such as clocks, interconnect and
GPIO pins configuration in Firmware. SCMI power and perf protocols are
utilized to request resource configurations.
SA8255p platform does not require the Serial Engine (SE) common properties
as the SE firmware is loaded and managed by the TrustZone (TZ) secure
environment.
Co-developed-by: Nikunj Kela <quic_nkela@quicinc.com>
Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
.../bindings/spi/qcom,sa8255p-geni-spi.yaml | 64 ++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml b/Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml
new file mode 100644
index 000000000000..d9347d780ca4
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/qcom,sa8255p-geni-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
+
+maintainers:
+ - Praveen Talari <praveen.talari@oss.qualcomm.com>
+
+properties:
+ compatible:
+ const: qcom,sa8255p-geni-spi
+
+ reg:
+ maxItems: 1
+
+ dmas:
+ maxItems: 2
+
+ dma-names:
+ items:
+ - const: tx
+ - const: rx
+
+ interrupts:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 2
+
+ power-domain-names:
+ items:
+ - const: power
+ - const: perf
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - power-domains
+ - power-domain-names
+
+allOf:
+ - $ref: /schemas/spi/spi-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/dma/qcom-gpi.h>
+
+ spi@888000 {
+ compatible = "qcom,sa8255p-geni-spi";
+ reg = <0x888000 0x4000>;
+ interrupts = <GIC_SPI 584 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&gpi_dma0 0 0 QCOM_GPI_SPI>,
+ <&gpi_dma0 1 0 QCOM_GPI_SPI>;
+ dma-names = "tx", "rx";
+ power-domains = <&scmi0_pd 0>, <&scmi0_dvfs 0>;
+ power-domain-names = "power", "perf";
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH v4 2/4] spi: qcom-geni: Use geni_se_resources_init() for resource initialization
From: Praveen Talari @ 2026-06-18 9:06 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, bjorn.andersson, Praveen Talari
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel,
mukesh.savaliya, aniket.randive, chandana.chiluveru,
jyothi.seerapu, chiluka.harish
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com>
Replace resources initialization such as clocks, ICC path and OPP with the
common geni_se_resources_init() function to avoid code duplication across
all drivers.
The geni_se_resources_init() function handles all these resources
internally, reducing code duplication and ensuring consistent resource
management across GENI SE drivers.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/spi/spi-geni-qcom.c | 26 +-------------------------
1 file changed, 1 insertion(+), 25 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 26e723cfea61..09feb6577efc 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1045,7 +1045,6 @@ static int spi_geni_probe(struct platform_device *pdev)
struct spi_controller *spi;
struct spi_geni_master *mas;
void __iomem *base;
- struct clk *clk;
struct device *dev = &pdev->dev;
irq = platform_get_irq(pdev, 0);
@@ -1060,10 +1059,6 @@ static int spi_geni_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
- clk = devm_clk_get(dev, "se");
- if (IS_ERR(clk))
- return PTR_ERR(clk);
-
if (device_property_read_bool(dev, "spi-slave"))
spi = devm_spi_alloc_target(dev, sizeof(*mas));
else
@@ -1079,17 +1074,10 @@ static int spi_geni_probe(struct platform_device *pdev)
mas->se.dev = dev;
mas->se.wrapper = dev_get_drvdata(dev->parent);
mas->se.base = base;
- mas->se.clk = clk;
- ret = devm_pm_opp_set_clkname(&pdev->dev, "se");
+ ret = geni_se_resources_init(&mas->se);
if (ret)
return ret;
- /* OPP table is optional */
- ret = devm_pm_opp_of_add_table(&pdev->dev);
- if (ret && ret != -ENODEV) {
- dev_err(&pdev->dev, "invalid OPP table in device tree\n");
- return ret;
- }
spi->bus_num = -1;
spi->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_CS_HIGH;
@@ -1115,24 +1103,12 @@ static int spi_geni_probe(struct platform_device *pdev)
if (spi->target)
spi->target_abort = spi_geni_target_abort;
- ret = geni_icc_get(&mas->se, NULL);
- if (ret)
- return ret;
-
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
ret = devm_pm_runtime_enable(dev);
if (ret)
return ret;
- /* Set the bus quota to a reasonable value for register access */
- mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
- mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
-
- ret = geni_icc_set_bw(&mas->se);
- if (ret)
- return ret;
-
ret = spi_geni_init(mas);
if (ret)
return ret;
--
2.34.1
^ permalink raw reply related
* [PATCH v4 3/4] spi: qcom-geni: Use resources helper APIs in runtime PM functions
From: Praveen Talari @ 2026-06-18 9:06 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, bjorn.andersson, Praveen Talari
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel,
mukesh.savaliya, aniket.randive, chandana.chiluveru,
jyothi.seerapu, chiluka.harish
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com>
To manage GENI serial engine resources during runtime power management,
drivers currently need to call functions for ICC, clock, and
SE resource operations in both suspend and resume paths, resulting in
code duplication across drivers.
The new geni_se_resources_activate() and geni_se_resources_deactivate()
helper APIs addresses this issue by providing a streamlined method to
enable or disable all resources based, thereby eliminating redundancy
across drivers.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/spi/spi-geni-qcom.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 09feb6577efc..025165c4b952 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1130,16 +1130,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
{
struct spi_controller *spi = dev_get_drvdata(dev);
struct spi_geni_master *mas = spi_controller_get_devdata(spi);
- int ret;
-
- /* Drop the performance state vote */
- dev_pm_opp_set_rate(dev, 0);
-
- ret = geni_se_resources_off(&mas->se);
- if (ret)
- return ret;
- return geni_icc_disable(&mas->se);
+ return geni_se_resources_deactivate(&mas->se);
}
static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
@@ -1148,15 +1140,14 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
struct spi_geni_master *mas = spi_controller_get_devdata(spi);
int ret;
- ret = geni_icc_enable(&mas->se);
+ ret = geni_se_resources_activate(&mas->se);
if (ret)
return ret;
- ret = geni_se_resources_on(&mas->se);
- if (ret)
- return ret;
+ if (mas->se.has_opp)
+ return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
- return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
+ return 0;
}
static int __maybe_unused spi_geni_suspend(struct device *dev)
--
2.34.1
^ permalink raw reply related
* [PATCH v4 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
From: Praveen Talari @ 2026-06-18 9:06 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, bjorn.andersson, Praveen Talari
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel,
mukesh.savaliya, aniket.randive, chandana.chiluveru,
jyothi.seerapu, chiluka.harish
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com>
The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power
and performance protocols.
The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power states(on/off).
The SCMI performance protocol manages SPI frequency, with each
frequency rate represented by a performance level. The driver uses
geni_se_set_perf_opp() API to request the desired frequency rate.
As part of geni_se_set_perf_opp(), the OPP for the requested frequency
is obtained using dev_pm_opp_find_freq_floor() and the performance
level is set using dev_pm_opp_set_opp().
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v3->v4
- Added return check for dev_data.
---
drivers/spi/spi-geni-qcom.c | 49 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 025165c4b952..88ac0833351c 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -78,6 +78,13 @@
#define GSI_CPHA BIT(4)
#define GSI_CPOL BIT(5)
+struct geni_spi_desc {
+ int (*resources_init)(struct geni_se *se);
+ int (*set_rate)(struct geni_se *se, unsigned long clk_freq);
+ int (*power_on)(struct geni_se *se);
+ int (*power_off)(struct geni_se *se);
+};
+
struct spi_geni_master {
struct geni_se se;
struct device *dev;
@@ -105,6 +112,7 @@ struct spi_geni_master {
struct dma_chan *tx;
struct dma_chan *rx;
int cur_xfer_mode;
+ const struct geni_spi_desc *dev_data;
};
static void spi_slv_setup(struct spi_geni_master *mas)
@@ -305,11 +313,12 @@ static void spi_setup_word_len(struct spi_geni_master *mas, u16 mode,
writel(word_len, se->base + SE_SPI_WORD_LEN);
}
-static int geni_spi_set_clock_and_bw(struct spi_geni_master *mas,
- unsigned long clk_hz)
+static int geni_spi_set_clock_and_bw(struct geni_se *se,
+ unsigned long clk_hz)
{
+ struct spi_controller *spi = dev_get_drvdata(se->dev);
+ struct spi_geni_master *mas = spi_controller_get_devdata(spi);
u32 clk_sel, m_clk_cfg, idx, div;
- struct geni_se *se = &mas->se;
int ret;
if (clk_hz == mas->cur_speed_hz)
@@ -825,7 +834,7 @@ static int setup_se_xfer(struct spi_transfer *xfer,
}
/* Speed and bits per word can be overridden per transfer */
- ret = geni_spi_set_clock_and_bw(mas, xfer->speed_hz);
+ ret = mas->dev_data->set_rate(&mas->se, xfer->speed_hz);
if (ret)
return ret;
@@ -1075,7 +1084,11 @@ static int spi_geni_probe(struct platform_device *pdev)
mas->se.wrapper = dev_get_drvdata(dev->parent);
mas->se.base = base;
- ret = geni_se_resources_init(&mas->se);
+ mas->dev_data = device_get_match_data(&pdev->dev);
+ if (!mas->dev_data)
+ return -EINVAL;
+
+ ret = mas->dev_data->resources_init(&mas->se);
if (ret)
return ret;
@@ -1131,7 +1144,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
struct spi_controller *spi = dev_get_drvdata(dev);
struct spi_geni_master *mas = spi_controller_get_devdata(spi);
- return geni_se_resources_deactivate(&mas->se);
+ return mas->dev_data->power_off ?
+ mas->dev_data->power_off(&mas->se) : 0;
}
static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
@@ -1140,9 +1154,11 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
struct spi_geni_master *mas = spi_controller_get_devdata(spi);
int ret;
- ret = geni_se_resources_activate(&mas->se);
- if (ret)
- return ret;
+ if (mas->dev_data->power_on) {
+ ret = mas->dev_data->power_on(&mas->se);
+ if (ret)
+ return ret;
+ }
if (mas->se.has_opp)
return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
@@ -1188,8 +1204,21 @@ static const struct dev_pm_ops spi_geni_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(spi_geni_suspend, spi_geni_resume)
};
+static const struct geni_spi_desc geni_spi = {
+ .resources_init = geni_se_resources_init,
+ .set_rate = geni_spi_set_clock_and_bw,
+ .power_on = geni_se_resources_activate,
+ .power_off = geni_se_resources_deactivate,
+};
+
+static const struct geni_spi_desc sa8255p_geni_spi = {
+ .resources_init = geni_se_domain_attach,
+ .set_rate = geni_se_set_perf_opp,
+};
+
static const struct of_device_id spi_geni_dt_match[] = {
- { .compatible = "qcom,geni-spi" },
+ { .compatible = "qcom,geni-spi", .data = &geni_spi },
+ { .compatible = "qcom,sa8255p-geni-spi", .data = &sa8255p_geni_spi },
{}
};
MODULE_DEVICE_TABLE(of, spi_geni_dt_match);
--
2.34.1
^ permalink raw reply related
* [PATCH v7 0/3] Add root port reset to support link recovery
From: hongxing.zhu @ 2026-06-18 9:20 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
kwilczynski, mani, s.hauer, kernel, festevam
Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel
Based on the following patch-set[1] issued by Mani.
Add support for resetting the Root Port for i.MX PCIe to enable link recovery.
[1] [PATCH v8 0/5] PCI: Add support for resetting the Root Ports in a platform specific way
PCIe links can go down due to various unexpected circumstances. This patch series
adds root port reset support for link recovery on i.MX PCIe controllers when the
optional "intr" interrupt is present.
When a link down event is detected, the root port reset uninitializes and
reinitializes the PCIe controller, then restarts the PCIe link.
On i.MX95 platforms, link events and PME share the same interrupt line.
Link event interrupts cannot use only an IRQ thread handler because the PME
driver uses request_irq() to bind the PME interrupt directly with only the
IRQF_SHARED flag set.
To address this, we register one handler with IRQF_SHARED for link event
interrupts and manipulate the enable bits of link events to ensure the same
interrupt source is triggered only once at a time.
Additionally, this series adds 'intr', 'aer', and 'pme' interrupt entries to
the i.MX6Q PCIe binding to support PCIe event-based interrupts for general
controller events, Advanced Error Reporting, and Power Management Events
respectively.
Changes in v7:
- Remove the redundant maxItem setting of interrupt property.
- Update driver codes refer to sashiko-reviews
Changes in v6:
- Use conditional constraints (if/then schema) to specify that these three
optional interrupts are only valid for the i.MX95 variant, while other
variants like imx6q should not have them.
- Change lnk_intr data type from u32 to int to properly handle negative
error codes returned by platform_get_irq_byname_optional().
- Replace platform_get_irq_byname() with platform_get_irq_byname_optional()
to suppress unnecessary error messages when the optional link event IRQ is
not present in the device tree.
- To avoid inadvertently clear the pending W1C status bit, clear the W1C
bit firstly, then do the regmap_clear_bits().
Changes in v5:
- Update the commit message of the first dt-binding patch for clarity.
- Add explicit comment explaining that writing 1 to IMX95_LINK_DOWN_INT_STS
clears the bit
Changes in v4:
- Set these new added three interrupts as optional interrupt.
Changes in v3:
- Don't add a new if:block; Drop the maxItems constraint of the interrupts
property for i.MX95 PCIe.
- Add constraints for the interrupts property for other variants.
- Regarding the ABI break: add descriptions explaining why these new
interrupts are mandatory and required by i.MX95 PCIe.
Changes in v2:
- Constrain the new added three interrupt entries to be valid only for the
i.MX95 variant using conditional schemas
[PATCH v7 1/3] dt-bindings: imx6q-pcie: Add optional intr/aer/pme
[PATCH v7 2/3] arm64: dts: imx95: Add dma, intr, aer and pme
[PATCH v7 3/3] PCI: imx6: Add root port reset to support link
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 25 +++++++++++++++++
arch/arm64/boot/dts/freescale/imx95.dtsi | 16 ++++++++---
drivers/pci/controller/dwc/pci-imx6.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 169 insertions(+), 4 deletions(-)
^ permalink raw reply
* [PATCH v7 1/3] dt-bindings: imx6q-pcie: Add optional intr/aer/pme interrupts for i.MX95
From: hongxing.zhu @ 2026-06-18 9:20 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
kwilczynski, mani, s.hauer, kernel, festevam
Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel,
Richard Zhu, Frank Li
In-Reply-To: <20260618092100.3669556-1-hongxing.zhu@oss.nxp.com>
From: Richard Zhu <hongxing.zhu@nxp.com>
The i.MX95 PCIe controller introduces three additional dedicated hardware
interrupt lines for specific events:
- intr: general controller events
- aer: Advanced Error Reporting events
- pme: Power Management Events
These interrupts are optional on i.MX95. PCIe basic functionality
(enumeration, configuration, and data transfer) works correctly without
them, as the controller can operate using only the existing msi interrupt.
Earlier i.MX PCIe variants (imx6q, imx6sx, imx6qp, imx7d, imx8mm, imx8mp,
imx8mq, imx8q) do not have these three dedicated interrupt lines.
Update the binding to allow up to 5 interrupts for i.MX95, while
restricting earlier variants to a maximum of 2 interrupts using
conditional constraints (if/then schema). This ensures the schema
accurately reflects the hardware capabilities of each SoC variant.
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
.../bindings/pci/fsl,imx6q-pcie.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index e8b8131f5f23b..4f56e8e4f1008 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -58,12 +58,18 @@ properties:
items:
- description: builtin MSI controller.
- description: builtin DMA controller.
+ - description: PCIe event interrupt.
+ - description: builtin AER SPI standalone interrupt line.
+ - description: builtin PME SPI standalone interrupt line.
interrupt-names:
minItems: 1
items:
- const: msi
- const: dma
+ - const: intr
+ - const: aer
+ - const: pme
reset-gpio:
deprecated: true
@@ -249,6 +255,25 @@ allOf:
- const: ref
- const: extref # Optional
+ - if:
+ properties:
+ compatible:
+ enum:
+ - fsl,imx6q-pcie
+ - fsl,imx6sx-pcie
+ - fsl,imx6qp-pcie
+ - fsl,imx7d-pcie
+ - fsl,imx8mm-pcie
+ - fsl,imx8mp-pcie
+ - fsl,imx8mq-pcie
+ - fsl,imx8q-pcie
+ then:
+ properties:
+ interrupts:
+ maxItems: 2
+ interrupt-names:
+ maxItems: 2
+
unevaluatedProperties: false
examples:
--
2.34.1
^ permalink raw reply related
* [PATCH v7 3/3] PCI: imx6: Add root port reset to support link recovery
From: hongxing.zhu @ 2026-06-18 9:21 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
kwilczynski, mani, s.hauer, kernel, festevam
Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel,
Richard Zhu
In-Reply-To: <20260618092100.3669556-1-hongxing.zhu@oss.nxp.com>
From: Richard Zhu <hongxing.zhu@nxp.com>
The PCIe link can go down due to various unexpected circumstances. Add
root port reset support to enable link recovery for the i.MX PCIe
controller when the optional "intr" interrupt is present.
When a link down event occurs, reset the root port by: uninitializing the
PCIe controller, re-initializing it, and restarting the link.
On i.MX95 platforms, link events and PME share the same interrupt line.
The link event interrupt cannot use a threaded-only IRQ handler because
the PME driver uses request_irq() with only the IRQF_SHARED flag set,
which requires a primary handler.
To handle this shared interrupt scenario, register a primary interrupt
handler with IRQF_SHARED for link events and manipulate the link event
enable bits to ensure the shared interrupt source triggers only one
handler at a time.
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
drivers/pci/controller/dwc/pci-imx6.c | 132 ++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 773ab65b2afac..3de70f41b0b85 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -79,6 +79,11 @@
#define IMX95_SID_MASK GENMASK(5, 0)
#define IMX95_MAX_LUT 32
+#define IMX95_LINK_INT_CTRL_STS 0x1040
+#define IMX95_PE0_INT_STS 0x10e8
+#define IMX95_LINK_DOWN_INT_STS BIT(11)
+#define IMX95_LINK_DOWN_INT_EN BIT(10)
+
#define IMX95_PCIE_RST_CTRL 0x3010
#define IMX95_PCIE_COLD_RST BIT(0)
@@ -126,6 +131,8 @@ enum imx_pcie_variants {
#define IMX_PCIE_MAX_INSTANCES 2
struct imx_pcie;
+static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
+ struct pci_dev *pdev);
struct imx_pcie_drvdata {
enum imx_pcie_variants variant;
@@ -158,6 +165,7 @@ struct imx_pcie {
bool supports_clkreq;
bool enable_ext_refclk;
struct regmap *iomuxc_gpr;
+ int lnk_intr;
u16 msi_ctrl;
u32 controller_id;
struct reset_control *pciephy_reset;
@@ -1394,6 +1402,13 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
imx_setup_phy_mpll(imx_pcie);
+ /*
+ * Callback invoked by PCI core when link down is detected and
+ * recovery is needed.
+ */
+ if (pp->bridge)
+ pp->bridge->reset_root_port = imx_pcie_reset_root_port;
+
return 0;
err_phy_off:
@@ -1661,6 +1676,9 @@ static int imx_pcie_suspend_noirq(struct device *dev)
if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
return 0;
+ if (imx_pcie->lnk_intr > 0)
+ regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_EN);
imx_pcie_msi_save_restore(imx_pcie, true);
if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
imx_pcie_lut_save(imx_pcie);
@@ -1711,6 +1729,9 @@ static int imx_pcie_resume_noirq(struct device *dev)
if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
imx_pcie_lut_restore(imx_pcie);
imx_pcie_msi_save_restore(imx_pcie, false);
+ if (imx_pcie->lnk_intr > 0)
+ regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_EN);
return 0;
}
@@ -1720,6 +1741,86 @@ static const struct dev_pm_ops imx_pcie_pm_ops = {
imx_pcie_resume_noirq)
};
+static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv)
+{
+ struct imx_pcie *imx_pcie = priv;
+ struct dw_pcie *pci = imx_pcie->pci;
+ struct device *dev = pci->dev;
+ u32 val;
+
+ regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);
+ if (val & IMX95_LINK_DOWN_INT_STS) {
+ dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
+ /* Clear link down interrupt status by writing 1b'1 to it */
+ regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_STS);
+ if (!(val & IMX95_LINK_DOWN_INT_EN))
+ return IRQ_NONE;
+ regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_EN);
+
+ return IRQ_WAKE_THREAD;
+ }
+
+ regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_INT_STS, &val);
+ if (unlikely(val))
+ regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_INT_STS, val);
+
+ return IRQ_NONE;
+}
+
+static irqreturn_t imx_pcie_lnk_irq_thread(int irq, void *priv)
+{
+ struct imx_pcie *imx_pcie = priv;
+ struct dw_pcie *pci = imx_pcie->pci;
+ struct dw_pcie_rp *pp = &pci->pp;
+ struct pci_dev *port;
+
+ for_each_pci_bridge(port, pp->bridge->bus)
+ if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
+ pci_host_handle_link_down(port);
+
+ regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_EN);
+
+ return IRQ_HANDLED;
+}
+
+static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
+ struct pci_dev *pdev)
+{
+ struct pci_bus *bus = bridge->bus;
+ struct dw_pcie_rp *pp = bus->sysdata;
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct imx_pcie *imx_pcie = to_imx_pcie(pci);
+ int ret;
+
+ imx_pcie_msi_save_restore(imx_pcie, true);
+ if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
+ imx_pcie_lut_save(imx_pcie);
+ imx_pcie_stop_link(pci);
+ imx_pcie_host_exit(pp);
+
+ ret = imx_pcie_host_init(pp);
+ if (ret) {
+ dev_err(pci->dev, "Failed to re-init PCIe\n");
+ return ret;
+ }
+ ret = dw_pcie_setup_rc(pp);
+ if (ret)
+ return ret;
+
+ imx_pcie_start_link(pci);
+ dw_pcie_wait_for_link(pci);
+
+ if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
+ imx_pcie_lut_restore(imx_pcie);
+ imx_pcie_msi_save_restore(imx_pcie, false);
+
+ dev_dbg(pci->dev, "Root port reset completed\n");
+ return 0;
+}
+
static int imx_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1919,15 +2020,46 @@ static int imx_pcie_probe(struct platform_device *pdev)
val |= PCI_MSI_FLAGS_ENABLE;
dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
}
+
+ /* Get link event irq if it is present */
+ imx_pcie->lnk_intr = platform_get_irq_byname_optional(pdev, "intr");
+ if (imx_pcie->lnk_intr == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_host_deinit;
+ }
+ if (imx_pcie->lnk_intr > 0) {
+ ret = devm_request_threaded_irq(dev, imx_pcie->lnk_intr,
+ imx_pcie_lnk_irq_isr,
+ imx_pcie_lnk_irq_thread,
+ IRQF_SHARED,
+ "lnk", imx_pcie);
+ if (ret) {
+ dev_err_probe(dev, ret,
+ "unable to request LNK IRQ\n");
+ goto err_host_deinit;
+ }
+
+ regmap_set_bits(imx_pcie->iomuxc_gpr,
+ IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_EN);
+ }
}
return 0;
+
+err_host_deinit:
+ dw_pcie_host_deinit(&pci->pp);
+
+ return ret;
}
static void imx_pcie_shutdown(struct platform_device *pdev)
{
struct imx_pcie *imx_pcie = platform_get_drvdata(pdev);
+ if (imx_pcie->lnk_intr > 0)
+ regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+ IMX95_LINK_DOWN_INT_EN);
/* bring down link, so bootloader gets clean state in case of reboot */
imx_pcie_assert_core_reset(imx_pcie);
imx_pcie_assert_perst(imx_pcie, true);
--
2.34.1
^ permalink raw reply related
* [PATCH v7 2/3] arm64: dts: imx95: Add dma, intr, aer and pme interrupts for PCIe
From: hongxing.zhu @ 2026-06-18 9:20 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
kwilczynski, mani, s.hauer, kernel, festevam
Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel,
Richard Zhu
In-Reply-To: <20260618092100.3669556-1-hongxing.zhu@oss.nxp.com>
From: Richard Zhu <hongxing.zhu@nxp.com>
The current PCIe device tree configuration only defines the MSI
interrupt, which is sufficient for basic PCIe operation but limits
advanced functionality.
Add the following interrupt lines to pcie0 and pcie1 nodes:
- dma: DMA interrupt for PCIe DMA operations
- intr: General controller events and link state changes
- aer: Advanced Error Reporting interrupt
- pme: Power Management Event interrupt
This enables enhanced PCIe features and capabilities that were
previously unavailable due to missing interrupt definitions.
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
arch/arm64/boot/dts/freescale/imx95.dtsi | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index 3e35c956a4d7a..1a9803f967901 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -1945,8 +1945,12 @@ pcie0: pcie@4c300000 {
bus-range = <0x00 0xff>;
num-lanes = <1>;
num-viewport = <8>;
- interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "msi";
+ interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi", "dma", "intr", "aer", "pme";
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 0x7>;
interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
@@ -2020,8 +2024,12 @@ pcie1: pcie@4c380000 {
bus-range = <0x00 0xff>;
num-lanes = <1>;
num-viewport = <8>;
- interrupts = <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "msi";
+ interrupts = <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi", "dma", "intr", "aer", "pme";
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 0x7>;
interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] dt-bindings: usb: ti,tps6598x: add TPS6699x compatible
From: Pandey, Radhey Shyam @ 2026-06-18 9:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Radhey Shyam Pandey
Cc: gregkh, robh, krzk+dt, conor+dt, bryan.odonoghue, linux-usb,
devicetree, linux-kernel
In-Reply-To: <9825384b-8e36-4c16-a10c-8abe9d000b56@kernel.org>
> On 13/06/2026 21:08, Krzysztof Kozlowski wrote:
>> On Sat, Jun 13, 2026 at 09:53:35PM +0530, Radhey Shyam Pandey wrote:
>> + switch and Power Delivery controller.
>>>
>>> A variant of this controller known as Apple CD321x or Apple ACE is also
>>> present on hardware with Apple SoCs such as the M1.
>>> @@ -19,6 +20,7 @@ properties:
>>> compatible:
>>> enum:
>>> - ti,tps6598x
>>> + - ti,tps6699x
>>
>> Family compatibles are not allowed. Please use specific models,
>> expressing compatibility (my slides of DTS101 talkmight help you if
>> something is unclear).
Thanks for pointing it out and the recent DTS 101 talk at OSS India.
It's clear now , will rename the compatible to match the exact
device i.e ti,tps66993 and send out v2.
>
>
> And where is any user of this? Why do we want this binding in the first
> place?
There is a user of it and will include the driver patch as well in v2.
Thanks,
Radhey
^ permalink raw reply
* Re: [PATCH v8 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys
From: Svyatoslav Ryhel @ 2026-06-18 9:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <ajGyejCSRMhY4G2R@google.com>
ср, 17 черв. 2026 р. о 00:23 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
>
> On Tue, Jun 16, 2026 at 09:25:25AM +0300, Svyatoslav Ryhel wrote:
> > вт, 16 черв. 2026 р. о 07:26 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
> > >
> > > Hi Svyatoslav,
> > >
> > > On Thu, May 28, 2026 at 08:32:00AM +0300, Svyatoslav Ryhel wrote:
> > > > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > >
> > > > Add support for multimedia top button row of ASUS Transformer's Mobile
> > > > Dock keyboard. Driver is made that function keys (F1-F12) are used by
> > > > default which suits average Linux use better and with pressing
> > > > ScreenLock + AltGr function keys layout is switched to multimedia keys.
> > > > Since this only modifies codes sent by asus-ec-keys it doesn't affect
> > > > normal keyboards at all.
> > >
> > > I think using input handler to intercept ScreenLock + AltGr is quite
> > > awkward. I think this also passes the original key events (unless you
> > > make it a filter not a regular handler).
> > >
> > > I do not see benefit for reacting to AltGr+ScreenLock on other keyboards
> > > to activate the special mode on this one. So given the fact that you
> > > already mange the data stream when you split it into "serio" ports,
> > > maybe just intercept this key combo right there and create the input
> > > device and signal input events right there?
> > >
> >
> > Though it seems awkward at a first glance, media keys are integrated
> > with a standard keyboard in a detachable dock. It is highly unlikely
> > that media keys will be used with a different keyboard then the one
> > that is integrated with dock. Additionally, the ScreenLock key has a
> > code specific to this driver and is not in general use, so even if any
> > standard keyboard has AltGr but none has ScreenLock specific to this
> > driver except the dock itself. Handler is also set as observer so it
> > should not interfere with work of other input devices.
>
> I am not concerned about it interfering with other drivers, I am
> concerned about it unnecessarily connecting to unrelated devices
> (anything that declares EV_KEY).
>
> Again, I think having input handler is not appropriate here. I would
> fold this patch into the patch that introduces the 2 serio ports,
> enhanced the data stream analysis to detect your key combo, and then
> report through this new input device. You do not need to have the round
> trip through atkbd and the new input handler for this.
>
I will try with filtering first if you don't mind. If that will not
work I'll consider folding. Do you have any good examples of handler
with filter to use as an inspiration? Thank you.
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v4 2/4] spi: qcom-geni: Use geni_se_resources_init() for resource initialization
From: sashiko-bot @ 2026-06-18 9:20 UTC (permalink / raw)
To: Praveen Talari; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-2-f5b5067e7e1e@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The `geni_se_resources_init()` function is missing from the kernel tree.
Pre-existing issues:
- [High] Asynchronous `pm_runtime_put()` is used on error paths that immediately call `pm_runtime_disable()`, risking the hardware being left in an invalid active state.
--
commit e8e291bf8f15d73b4da4bf256d08b86fec92a093
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
spi: qcom-geni: Use geni_se_resources_init() for resource initialization
This commit replaces individual resource initializations such as clocks, ICC
paths, and OPP tables with a common geni_se_resources_init() function. This
aims to reduce code duplication and ensure consistent resource management
across GENI SE drivers.
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 26e723cfea61e..09feb6577efcb 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
[ ... ]
> @@ -1079,17 +1074,10 @@ static int spi_geni_probe(struct platform_device *pdev)
> mas->se.dev = dev;
> mas->se.wrapper = dev_get_drvdata(dev->parent);
> mas->se.base = base;
> - mas->se.clk = clk;
>
> - ret = devm_pm_opp_set_clkname(&pdev->dev, "se");
> + ret = geni_se_resources_init(&mas->se);
[Severity: Low]
Is this function defined anywhere in the kernel tree?
It doesn't appear to be declared or defined in this commit or the existing
codebase, which results in an implicit declaration error and linker failure.
> if (ret)
> return ret;
[ ... ]
> @@ -1115,24 +1103,12 @@ static int spi_geni_probe(struct platform_device *pdev)
> if (spi->target)
> spi->target_abort = spi_geni_target_abort;
>
> - ret = geni_icc_get(&mas->se, NULL);
> - if (ret)
> - return ret;
> -
> pm_runtime_use_autosuspend(&pdev->dev);
> pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
> ret = devm_pm_runtime_enable(dev);
[Severity: High]
This is a pre-existing issue, but I noticed a potential resource leak related
to how runtime PM is disabled.
If probe fails later on, the devres framework will immediately call
pm_runtime_disable() to undo devm_pm_runtime_enable().
Does this cause the asynchronous pm_runtime_put() called inside
spi_geni_init() to have its queued idle/suspend work cancelled? This could
leave the device permanently powered on after a probe failure.
> if (ret)
> return ret;
[ ... ]
> ret = spi_geni_init(mas);
> if (ret)
> return ret;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [PATCH v4 3/4] spi: qcom-geni: Use resources helper APIs in runtime PM functions
From: sashiko-bot @ 2026-06-18 9:21 UTC (permalink / raw)
To: Praveen Talari; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-3-f5b5067e7e1e@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The patch relies on APIs and structure fields that do not exist in the baseline kernel tree or the rest of the patch series, causing fatal compilation errors.
Pre-existing issues:
- [High] Hardware registers are accessed without checking if `pm_runtime_get_sync()` successfully resumed the device.
--
commit 8edc09ad870bfce8d07091d2faeae9667787e1f0
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
spi: qcom-geni: Use resources helper APIs in runtime PM functions
This commit updates the geni SPI driver's runtime power management
functions to use streamlined resource activation and deactivation
helpers. It aims to eliminate code duplication across drivers.
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 09feb6577efcb..025165c4b9527 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -1130,16 +1130,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
> {
> struct spi_controller *spi = dev_get_drvdata(dev);
> struct spi_geni_master *mas = spi_controller_get_devdata(spi);
> - int ret;
> -
> - /* Drop the performance state vote */
> - dev_pm_opp_set_rate(dev, 0);
> -
> - ret = geni_se_resources_off(&mas->se);
> - if (ret)
> - return ret;
>
> - return geni_icc_disable(&mas->se);
> + return geni_se_resources_deactivate(&mas->se);
[Severity: Low]
It appears the geni_se_resources_deactivate() and
geni_se_resources_activate() functions are not implemented in the baseline
tree, which could lead to immediate build failures.
Are these new helper APIs defined elsewhere in this patch series?
> }
>
> static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
> @@ -1148,15 +1140,14 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
> struct spi_geni_master *mas = spi_controller_get_devdata(spi);
> int ret;
>
> - ret = geni_icc_enable(&mas->se);
> + ret = geni_se_resources_activate(&mas->se);
> if (ret)
> return ret;
>
> - ret = geni_se_resources_on(&mas->se);
> - if (ret)
> - return ret;
> + if (mas->se.has_opp)
[Severity: Low]
The has_opp field appears to be missing from the definition of
struct geni_se, which would cause a compilation error.
Does the has_opp field exist in struct geni_se in another patch within
this series?
> + return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
>
> - return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
> + return 0;
> }
[ ... ]
[Severity: High]
This is a pre-existing issue, but looking at spi_geni_init() in
drivers/spi/spi-geni-qcom.c:
pm_runtime_get_sync(mas->dev);
proto = geni_se_read_proto(se);
If pm_runtime_get_sync() fails to resume the device, accessing unclocked
or powered-off registers via geni_se_read_proto() could trigger a
synchronous external abort and result in a kernel panic.
Should the return value of pm_runtime_get_sync() be checked before
accessing hardware registers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH v4 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
From: sashiko-bot @ 2026-06-18 9:23 UTC (permalink / raw)
To: Praveen Talari; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260618-enable-spi-on-sa8255p-v4-4-f5b5067e7e1e@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Incomplete abstraction in `setup_gsi_xfer()` breaks GPI DMA transfers for SA8255p.
- [High] `spi_geni_runtime_resume()` hardcodes legacy OPP clock restore, breaking performance state restoration on SA8255p.
- [Low] Compilation fails due to undeclared identifiers `geni_se_domain_attach` and `geni_se_set_perf_opp`.
Pre-existing issues:
- [High] Ignored `pm_runtime_get_sync()` return value in `spi_geni_init()` causes kernel panic on initialization failure.
--
commit 41d8ba8fbc50744c00c196e2484d690b4592cc0a
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
Subject: spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
This commit adds support for the SA8255p SoC to the qcom-geni SPI driver.
It abstracts resources initialization and rate setting into a new
geni_spi_desc structure to handle SCMI-based power and performance protocols
required by this platform.
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 025165c4b9527..88ac0833351c0 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
[ ... ]
> @@ -825,7 +834,7 @@ static int setup_se_xfer(struct spi_transfer *xfer,
> }
>
> /* Speed and bits per word can be overridden per transfer */
> - ret = geni_spi_set_clock_and_bw(mas, xfer->speed_hz);
> + ret = mas->dev_data->set_rate(&mas->se, xfer->speed_hz);
> if (ret)
> return ret;
[Severity: High]
Since setup_se_xfer() now uses the new set_rate callback, should
setup_gsi_xfer() be updated as well?
Currently, setup_gsi_xfer() appears to hardcode a call to the legacy
get_spi_clk_cfg() function, which bypasses mas->dev_data->set_rate.
Could this break GPI DMA transfers for the new SA8255p platform by leaving
the hardware misconfigured?
[ ... ]
> @@ -1075,7 +1084,11 @@ static int spi_geni_probe(struct platform_device *pdev)
> mas->se.wrapper = dev_get_drvdata(dev->parent);
> mas->se.base = base;
>
> - ret = geni_se_resources_init(&mas->se);
> + mas->dev_data = device_get_match_data(&pdev->dev);
> + if (!mas->dev_data)
> + return -EINVAL;
> +
> + ret = mas->dev_data->resources_init(&mas->se);
> if (ret)
> return ret;
[Severity: High]
This is a pre-existing issue, but does spi_geni_init() safely handle power
domain or clock enablement failures?
In spi_geni_init(), which is called shortly after this probe setup, the
return value of pm_runtime_get_sync() is ignored before accessing hardware:
pm_runtime_get_sync(mas->dev);
proto = geni_se_read_proto(se);
If pm_runtime_get_sync() fails, could the subsequent read from the
GENI_FW_REVISION_RO register via geni_se_read_proto() cause an asynchronous
external abort and panic the kernel?
[ ... ]
> @@ -1140,9 +1154,11 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
> struct spi_geni_master *mas = spi_controller_get_devdata(spi);
> int ret;
>
> - ret = geni_se_resources_activate(&mas->se);
> - if (ret)
> - return ret;
> + if (mas->dev_data->power_on) {
> + ret = mas->dev_data->power_on(&mas->se);
> + if (ret)
> + return ret;
> + }
>
> if (mas->se.has_opp)
> return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
[Severity: High]
Does this hardcoded OPP restore bypass the new set_rate abstraction for the
SA8255p platform?
For SA8255p, mas->dev_data->set_rate is geni_se_set_perf_opp(), which
does not update the legacy mas->cur_sclk_hz variable. If has_opp is true,
could this accidentally set the clock rate to 0 and stall the device?
Alternatively, if has_opp is false, could this fail to restore the
performance state entirely?
[ ... ]
> @@ -1188,8 +1204,21 @@ static const struct dev_pm_ops spi_geni_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(spi_geni_suspend, spi_geni_resume)
> };
>
> +static const struct geni_spi_desc geni_spi = {
> + .resources_init = geni_se_resources_init,
> + .set_rate = geni_spi_set_clock_and_bw,
> + .power_on = geni_se_resources_activate,
> + .power_off = geni_se_resources_deactivate,
> +};
> +
> +static const struct geni_spi_desc sa8255p_geni_spi = {
> + .resources_init = geni_se_domain_attach,
> + .set_rate = geni_se_set_perf_opp,
> +};
[Severity: Low]
Are geni_se_domain_attach and geni_se_set_perf_opp declared elsewhere?
It looks like these identifiers are not defined in this file or the included
headers, which might cause a build failure.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=4
^ permalink raw reply
* Re: [PATCH RFC v4 10/12] reset: zte: Add a zx297520v3 reset driver
From: Philipp Zabel @ 2026-06-18 9:24 UTC (permalink / raw)
To: Stefan Dösinger, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney
Cc: linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260616-zx29clk-v4-10-ca994bd22e9d@gmail.com>
On Di, 2026-06-16 at 23:26 +0300, Stefan Dösinger wrote:
> This drives the auxiliary devices created by the clock driver.
Which auxiliary devices? Which clock driver?
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
> ---
> MAINTAINERS | 1 +
> drivers/reset/Kconfig | 11 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-zte-zx297520v3.c | 224 +++++++++++++++++++++++++++++++++++
> 4 files changed, 237 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1f0459b2c72..55bf0290343a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3871,6 +3871,7 @@ F: Documentation/devicetree/zte,zx297520v3-*
> F: arch/arm/boot/dts/zte/
> F: arch/arm/mach-zte/
> F: drivers/clk/zte/
> +F: drivers/reset/reset-zte-zx297520v3.c
> F: include/dt-bindings/clock/zte,zx297520v3-clk.h
>
> ARM/ZYNQ ARCHITECTURE
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index d009eb0849a3..116dd23f1b8e 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -404,6 +404,17 @@ config RESET_UNIPHIER_GLUE
> on UniPhier SoCs. Say Y if you want to control reset signals
> provided by the glue layer.
>
> +config RESET_ZTE_ZX297520V3
> + tristate "ZTE zx297520v3 Reset Driver"
> + depends on (ARCH_ZTE || COMPILE_TEST)
> + default CLK_ZTE_ZX297520V3
> + select AUXILIARY_BUS
> + help
> + This enables the reset controller for ZTE zx297520v3 SoCs. The reset
> + controller is part of the clock controller on this SoC. This driver
> + operates on an auxiliary device exposed by the clock driver. Enable
> + this driver if you plan to boot the kernel on a zx297520v3 based SoC.
> +
> config RESET_ZYNQ
> bool "ZYNQ Reset Driver" if COMPILE_TEST
> default ARCH_ZYNQ
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 3e52569bd276..9a8a48d44dc4 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -50,5 +50,6 @@ obj-$(CONFIG_RESET_TI_TPS380X) += reset-tps380x.o
> obj-$(CONFIG_RESET_TN48M_CPLD) += reset-tn48m.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
> +obj-$(CONFIG_RESET_ZTE_ZX297520V3) += reset-zte-zx297520v3.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> obj-$(CONFIG_RESET_ZYNQMP) += reset-zynqmp.o
> diff --git a/drivers/reset/reset-zte-zx297520v3.c b/drivers/reset/reset-zte-zx297520v3.c
> new file mode 100644
> index 000000000000..2022f4df2ebd
> --- /dev/null
> +++ b/drivers/reset/reset-zte-zx297520v3.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Stefan Dösinger
> + */
> +#include <dt-bindings/clock/zte,zx297520v3-clk.h>
> +#include <linux/reset-controller.h>
> +#include <linux/platform_device.h>
What is this used for?
> +#include <linux/auxiliary_bus.h>
> +#include <linux/clk-provider.h>
What is this used for?
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/iopoll.h>
> +#include <linux/delay.h>
> +
> +struct zte_reset_reg {
> + u32 mask, wait_mask;
> + u16 reg;
> +};
> +
> +struct zte_reset_info {
> + const struct zte_reset_reg *resets;
> + unsigned int num;
> +};
> +
> +struct zte_reset {
> + struct reset_controller_dev rcdev;
> + struct regmap *map;
> + const struct zte_reset_reg *resets;
> +};
> +
> +static inline struct zte_reset *to_zte_reset(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct zte_reset, rcdev);
> +}
> +
> +static int zx29_rst_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct zte_reset *rst = to_zte_reset(rcdev);
> +
> + return regmap_clear_bits(rst->map, rst->resets[id].reg, rst->resets[id].mask);
> +}
> +
> +static int zx29_rst_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct zte_reset *rst = to_zte_reset(rcdev);
> + int res;
> + u32 val;
> +
> + res = regmap_set_bits(rst->map, rst->resets[id].reg, rst->resets[id].mask);
> + if (res)
> + return res;
> +
> + /* This is a special case used only by USB reset */
> + if (rst->resets[id].wait_mask) {
> + return regmap_read_poll_timeout(rst->map, rst->resets[id].reg + 4, val,
> + val & rst->resets[id].wait_mask, 1, 100);
> + }
> +
> + return 0;
> +}
> +
> +static int zx29_rst_status(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct zte_reset *rst = to_zte_reset(rcdev);
> + int res;
> +
> + res = regmap_test_bits(rst->map, rst->resets[id].reg, rst->resets[id].mask);
> + if (res < 0)
> + return res;
> +
> + return !res;
> +}
> +
> +static const struct reset_control_ops zx29_rst_ops = {
> + .assert = zx29_rst_assert,
> + .deassert = zx29_rst_deassert,
> + .status = zx29_rst_status,
> +};
> +
> +static const struct zte_reset_reg zx297520v3_top_resets[] = {
> + /* This bit is set by ZTE's cpko.ko blob, it looks like a reset bit for the LTE DSP
> + * coprocessor. Clocks for it are in matrixclk.
> + */
> + [ZX297520V3_ZSP_RESET] = { .reg = 0x13c, .mask = BIT(0) },
> +
> + [ZX297520V3_UART0_RESET] = { .reg = 0x78, .mask = BIT(6) | BIT(7) },
Is this a single reset line controlled by two bits (do you know what
they are)? Or might these actually be two different reset controls that
are just always set together?
> + [ZX297520V3_I2C0_RESET] = { .reg = 0x74, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_RTC_RESET] = { .reg = 0x74, .mask = BIT(4) | BIT(5) },
> + [ZX297520V3_TIMER_T08_RESET] = { .reg = 0x78, .mask = BIT(4) | BIT(5) },
> + [ZX297520V3_TIMER_T09_RESET] = { .reg = 0x78, .mask = BIT(2) | BIT(3) },
> + [ZX297520V3_PMM_RESET] = { .reg = 0x74, .mask = BIT(0) | BIT(1) },
> +
> + /* I haven't found any clocks for GPIO. It probably wouldn't make much
> + * sense anyway. Only one reset bit per controller.
> + */
> + [ZX297520V3_GPIO_RESET] = { .reg = 0x74, .mask = BIT(3) },
> + [ZX297520V3_GPIO8_RESET] = { .reg = 0x74, .mask = BIT(2) },
> +
> + [ZX297520V3_TIMER_T12_RESET] = { .reg = 0x74, .mask = BIT(6) | BIT(7) },
> + [ZX297520V3_TIMER_T13_RESET] = { .reg = 0x7c, .mask = BIT(0) | BIT(1) },
> + [ZX297520V3_TIMER_T14_RESET] = { .reg = 0x7c, .mask = BIT(2) | BIT(3) },
> + [ZX297520V3_TIMER_T15_RESET] = { .reg = 0x74, .mask = BIT(10) | BIT(11) },
> + [ZX297520V3_TIMER_T16_RESET] = { .reg = 0x7c, .mask = BIT(4) | BIT(5) },
> + [ZX297520V3_TIMER_T17_RESET] = { .reg = 0x12c, .mask = BIT(0) | BIT(1) },
> + [ZX297520V3_WDT_T18_RESET] = { .reg = 0x74, .mask = BIT(12) | BIT(13) },
> + [ZX297520V3_USIM1_RESET] = { .reg = 0x74, .mask = BIT(14) | BIT(15) },
> + [ZX297520V3_AHB_RESET] = { .reg = 0x70, .mask = BIT(0) | BIT(1) },
> +
> + /* USB reset. This is slightly special because it needs to wait for a ready bit after
> + * deasserting.
> + */
> + [ZX297520V3_USB_RESET] = { .reg = 0x80, .mask = BIT(3) | BIT(4) | BIT(5),
> + .wait_mask = BIT(1)},
Same as above, are these actually three separate reset lines?
> + [ZX297520V3_HSIC_RESET] = { .reg = 0x80, .mask = BIT(0) | BIT(1) | BIT(2),
> + .wait_mask = BIT(0)},
> +};
> +
> +static const struct zte_reset_info zx297520v3_top_info = {
> + .resets = zx297520v3_top_resets,
> + .num = ARRAY_SIZE(zx297520v3_top_resets),
> +};
> +
> +static const struct zte_reset_reg zx297520v3_matrix_resets[] = {
> + [ZX297520V3_CPU_RESET] = { .reg = 0x28, .mask = BIT(1) },
> + [ZX297520V3_EDCP_RESET] = { .reg = 0x68, .mask = BIT(0) },
> + [ZX297520V3_SD0_RESET] = { .reg = 0x58, .mask = BIT(1) },
> + [ZX297520V3_SD1_RESET] = { .reg = 0x58, .mask = BIT(0) },
> + [ZX297520V3_NAND_RESET] = { .reg = 0x58, .mask = BIT(4) },
> + [ZX297520V3_PDCFG_RESET] = { .reg = 0x94, .mask = BIT(20) },
> + [ZX297520V3_SSC_RESET] = { .reg = 0x94, .mask = BIT(24) },
> + [ZX297520V3_GMAC_RESET] = { .reg = 0x114, .mask = BIT(0) | BIT(1) },
> + [ZX297520V3_VOU_RESET] = { .reg = 0x16c, .mask = BIT(0) },
> +};
> +
> +static const struct zte_reset_info zx297520v3_matrix_info = {
> + .resets = zx297520v3_matrix_resets,
> + .num = ARRAY_SIZE(zx297520v3_matrix_resets),
> +};
> +
> +static const struct zte_reset_reg zx297520v3_lsp_resets[] = {
> + [ZX297520V3_TIMER_L1_RESET] = { .reg = 0x04, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_WDT_L2_RESET] = { .reg = 0x08, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_WDT_L3_RESET] = { .reg = 0x0c, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_PWM_RESET] = { .reg = 0x10, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_I2S0_RESET] = { .reg = 0x14, .mask = BIT(8) | BIT(9) },
> + /* 0x18: Not writeable */
> + [ZX297520V3_I2S1_RESET] = { .reg = 0x1c, .mask = BIT(8) | BIT(9) },
> + /* 0x20: Not writeable */
> + [ZX297520V3_QSPI_RESET] = { .reg = 0x24, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_UART1_RESET] = { .reg = 0x28, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_I2C1_RESET] = { .reg = 0x2c, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_SPI0_RESET] = { .reg = 0x30, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_TIMER_LB_RESET] = { .reg = 0x34, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_TIMER_LC_RESET] = { .reg = 0x38, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_UART2_RESET] = { .reg = 0x3c, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_WDT_LE_RESET] = { .reg = 0x40, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_TIMER_LF_RESET] = { .reg = 0x44, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_SPI1_RESET] = { .reg = 0x48, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_TIMER_L11_RESET] = { .reg = 0x4c, .mask = BIT(8) | BIT(9) },
> + [ZX297520V3_TDM_RESET] = { .reg = 0x50, .mask = BIT(8) | BIT(9) },
> +};
> +
> +static const struct zte_reset_info zx297520v3_lsp_info = {
> + .resets = zx297520v3_lsp_resets,
> + .num = ARRAY_SIZE(zx297520v3_lsp_resets),
> +};
> +
> +static int reset_zx297520v3_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + const struct zte_reset_info *drv_info;
> + struct device *dev = &adev->dev;
> + struct zte_reset *rst;
> +
> + drv_info = (struct zte_reset_info *)id->driver_data;
> +
> + rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> + if (!rst)
> + return -ENOMEM;
> +
> + rst->resets = drv_info->resets;
> + rst->rcdev.owner = THIS_MODULE;
> + rst->rcdev.nr_resets = drv_info->num;
> + rst->rcdev.ops = &zx29_rst_ops;
> + rst->rcdev.of_node = dev->of_node;
> + rst->rcdev.dev = dev;
> + rst->rcdev.of_reset_n_cells = 1;
No need to set of_reset_n_cells if of_xlate is not set. Here
reset_controller_register will use fwnode_n_cells and set it to 1
anyway.
> +
> + rst->map = device_node_to_regmap(dev->of_node);
> + if (IS_ERR(rst->map))
> + return dev_err_probe(rdev, PTR_ERR(rst->map), "Cannot get parent syscon regmap\n");
> +
> + return devm_reset_controller_register(dev, &rst->rcdev);
> +}
> +
> +static const struct auxiliary_device_id reset_zx297520v3_ids[] = {
> + {
> + .name = "clk_zte.zx297520v3_toprst",
> + .driver_data = (kernel_ulong_t)&zx297520v3_top_info,
> + },
> + {
> + .name = "clk_zte.zx297520v3_matrixrst",
> + .driver_data = (kernel_ulong_t)&zx297520v3_matrix_info,
> + },
> + {
> + .name = "clk_zte.zx297520v3_lsprst",
> + .driver_data = (kernel_ulong_t)&zx297520v3_lsp_info,
> + },
> + { },
> +};
> +
Drop this empty line.
> +MODULE_DEVICE_TABLE(auxiliary, reset_zx297520v3_ids);
> +
> +static struct auxiliary_driver reset_zx297520v3_drv = {
> + .name = "zx297520v3_reset",
> + .id_table = reset_zx297520v3_ids,
> + .probe = reset_zx297520v3_probe,
> +};
> +
Drop this empty line.
> +module_auxiliary_driver(reset_zx297520v3_drv);
> +
> +MODULE_AUTHOR("Stefan Dösinger <stefandoesinger@gmail.com>");
> +MODULE_DESCRIPTION("ZTE zx297520v3 reset driver");
> +MODULE_LICENSE("GPL");
regards
Philipp
^ permalink raw reply
* Re: [PATCH v2 3/6] arm64: dts: qcom: kodiak: Add GEM_NOC interconnect for adreno SMMU
From: Konrad Dybcio @ 2026-06-18 9:26 UTC (permalink / raw)
To: Bibek Kumar Patro, Dmitry Baryshkov
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <26d51cbb-2d87-4564-b3c6-cc61ab900e19@oss.qualcomm.com>
On 6/8/26 4:37 PM, Bibek Kumar Patro wrote:
>
>
> On 6/8/2026 7:27 PM, Dmitry Baryshkov wrote:
>> On Tue, May 26, 2026 at 08:12:04PM +0530, Bibek Kumar Patro wrote:
>>> On Kodiak platforms, the Adreno SMMU requires a bandwidth vote on
>>> the GEM_NOC path (MASTER_GPU_TCU -> SLAVE_EBI1) before its registers
>>> are accessible. Without this vote, the SMMU may become unreachable,
>>> leading to intermittent probe failures and runtime issues.
>>>
>>> Add the required interconnect to ensure reliable register access.
>>
>> Does it only concern the GPU SMMU? What about the APPS SMMU? Should it
>> be voting on other interconnects too? I guess so, because currently I
>> see that TBUs vote for various interconnects. BTW: should apps_smmu also
>> vote on the power domains?
>>
>
> This race mainly occurs in GPU SMMU, where the GDSC can have an
Mainly or exclusively?
Konrad
> independent vote on the Adreno SMMU. However, the GEM_NOC vote may
> already have been removed by the GPU (or any consumer of adreno_smmu,
> e.g gmu), unless it is explicitly voted by the GPU SMMU (which acts as a
> supplier for the GPU). This mismatch can lead to SHUB timeouts or NoC
> errors.
>
> Mostly this race reported in suspend/resume cycle (when gpu/gmu devices moves to slumber/suspend state before adreno_smmu powers down
> and the later doesn't have explicit interconnect voting).
>
> In the case of APPS SMMU, such a race is not expected for any known
> use case. APPS SMMU is part of a shared infrastructure block, and its
> power is typically kept enabled as long as attached master devices are
> active. Therefore, explicit power-domain voting from APPS SMMU may not
> be required.
>
> Thanks,
> Bibek
>
>
>>>
>>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>> ---
>>> arch/arm64/boot/dts/qcom/kodiak.dtsi | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>> index fa540d8c2615dc02d941eb16bc7253204c2750bd..eefa4b836a81374ff437ab4bbcbc3fecc1590ab6 100644
>>> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>> @@ -3386,6 +3386,8 @@ adreno_smmu: iommu@3da0000 {
>>> power-domains = <&gpucc GPU_CC_CX_GDSC>;
>>> dma-coherent;
>>> + interconnects = <&gem_noc MASTER_GPU_TCU QCOM_ICC_TAG_ALWAYS
>>> + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>>> };
>>> gfx_0_tbu: tbu@3dd9000 {
>>>
>>> --
>>> 2.34.1
>>>
>>
>
>
^ permalink raw reply
* RE: [PATCH v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: Joakim Zhang @ 2026-06-18 9:27 UTC (permalink / raw)
To: Philipp Zabel, Conor Dooley
Cc: mturquette@baylibre.com, sboyd@kernel.org, bmasney@redhat.com,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
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: <cd63c1ab9831fdfd39aad8d1a40c4702a9cbb158.camel@pengutronix.de>
Hello,
> -----Original Message-----
> From: Philipp Zabel <p.zabel@pengutronix.de>
> Sent: Thursday, June 18, 2026 4:30 PM
> To: Joakim Zhang <joakim.zhang@cixtech.com>; Conor Dooley
> <conor@kernel.org>
> Cc: mturquette@baylibre.com; sboyd@kernel.org; bmasney@redhat.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; 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 v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss
> clock controller
>
> EXTERNAL EMAIL
>
> CAUTION: Suspicious Email from unusual domain.
>
> On Do, 2026-06-18 at 01:43 +0000, Joakim Zhang wrote:
> > Hello,
> >
> >
> > > -----Original Message-----
> > > From: Conor Dooley <conor@kernel.org>
> > > Sent: Wednesday, June 17, 2026 11:56 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 v4 3/5] dt-bindings: clock:
> > > cix,sky1-audss-clock: add audss clock controller
> > >
> > > On Wed, Jun 17, 2026 at 02:04:35PM +0800, joakim.zhang@cixtech.com
> wrote:
> > > > From: Joakim Zhang <joakim.zhang@cixtech.com>
> > > >
> > > > The AUDSS CRU contains an internal clock tree of muxes, dividers
> > > > and gates for DSP, I2S, HDA, DMAC and related blocks. The clock
> > > > provider is a child node of the cix,sky1-audss-system-control
> > > > syscon and accesses registers through the parent MMIO region.
> > >
> > > Why can this not just be part of the parent syscon node?
> >
> > The clock and reset blocks are handled by different subsystems and
> maintainers (clk vs reset). Putting the clock provider on the parent syscon node
> would mean a single driver has to register both the reset controller and the
> clock provider on one device, which doesn't fit well.
>
> There are many examples of clock and reset drivers sharing the same node, by
> using platform_driver for one (usually clk) and auxiliary_driver for the other
> (usually reset).
OK, I will have a look. If you are also prefer to this, I will refactor the patch.
Thanks,
Joakim
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox