Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v7 4/4] i2c: qcom-geni: Support multi-owner controllers in GPI mode
From: Bjorn Andersson @ 2026-05-22  3:15 UTC (permalink / raw)
  To: Mukesh Kumar Savaliya
  Cc: viken.dadhaniya, andi.shyti, robh, krzk+dt, conor+dt, vkoul,
	Frank.Li, konradybcio, dmitry.baryshkov, linmq006, quic_jseerapu,
	agross, linux-arm-msm, linux-i2c, devicetree, linux-kernel,
	dmaengine, krzysztof.kozlowski, bartosz.golaszewski,
	bjorn.andersson, konrad.dybcio
In-Reply-To: <20260423145705.545552-5-mukesh.savaliya@oss.qualcomm.com>

On Thu, Apr 23, 2026 at 08:25:51PM +0530, Mukesh Kumar Savaliya wrote:
> Some platforms use a QUP-based I2C controller in a configuration where the
> controller is shared with another system processor. In this setup the
> operating system must not assume exclusive ownership of the controller or
> its associated pins.
> 
> Add support for enabling multi-owner operation when DeviceTree specifies
> qcom,qup-multi-owner. When enabled, mark the underlying serial engine as
> shared so the common GENI resource handling avoids selecting the "sleep"
> pinctrl state, which could disrupt transfers initiated by the other
> processor.
> 
> For GPI mode transfers, request lock/unlock TRE sequencing from the GPI

"For GPI mode transfers" is there any other form?

> driver by setting a single lock_action selector per message, emitting lock
> before the first message and unlock after the last message (handling the
> single-message case as well). This serializes access to the shared
> controller without requiring message-position flags to be passed into the
> DMA engine layer.
> 
> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---
>  drivers/i2c/busses/i2c-qcom-geni.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index ae609bdd2ec4..a396ddc7d8f4 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -815,6 +815,14 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>  		if (i < num - 1)
>  			peripheral.stretch = 1;
>  
> +		peripheral.lock_action = GPI_LOCK_NONE;
> +		if (gi2c->se.multi_owner) {
> +			if (i == 0)
> +				peripheral.lock_action = GPI_LOCK_ACQUIRE;
> +			else if (i == num - 1)
> +				peripheral.lock_action = GPI_LOCK_RELEASE;

You say above that single-messages case is handled, but if num == 1 then
we will hit i == 0, set the acquire, we will not hit else, and then we
will exit the loop. What am I missing?

> +		}
> +
>  		peripheral.addr = msgs[i].addr;
>  		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
>  			peripheral.multi_msg = false;
> @@ -1014,6 +1022,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  		gi2c->clk_freq_out = I2C_MAX_STANDARD_MODE_FREQ;
>  	}
>  
> +	if (of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner")) {
> +		gi2c->se.multi_owner = true;

gi2c->se.multi_owner = of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner");

> +		dev_dbg(&pdev->dev, "I2C controller is shared with another system processor\n");
> +	}
> +
>  	if (has_acpi_companion(dev))
>  		ACPI_COMPANION_SET(&gi2c->adap.dev, ACPI_COMPANION(dev));
>  
> @@ -1089,7 +1102,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  	}
>  
>  	if (fifo_disable) {
> -		/* FIFO is disabled, so we can only use GPI DMA */
> +		/* FIFO is disabled, so we can only use GPI DMA.

That's not how we format comments outside the network subsystem.

Regards,
Bjorn

> +		 * SE can be shared in GSI mode between subsystems, each SS owns a GPII.
> +		 */
>  		gi2c->gpi_mode = true;
>  		ret = setup_gpi_dma(gi2c);
>  		if (ret)
> @@ -1098,6 +1113,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  		dev_dbg(dev, "Using GPI DMA mode for I2C\n");
>  	} else {
>  		gi2c->gpi_mode = false;
> +
> +		if (gi2c->se.multi_owner)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "I2C sharing not supported in non GSI mode\n");
> +
>  		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>  
>  		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
> -- 
> 2.43.0
> 

^ permalink raw reply

* Re: [PATCH v6 01/10] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588
From: Damon Ding @ 2026-05-22  3:02 UTC (permalink / raw)
  To: Conor Dooley
  Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
	neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
	nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
	dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
	dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <20260521-waking-case-cd709f0a7712@spud>

Hi Conor,

On 5/22/2026 3:54 AM, Conor Dooley wrote:
> On Thu, May 21, 2026 at 04:08:26PM +0800, Damon Ding wrote:
>> RK3588 eDP controller requires HCLK_VO1 to access the VO1 GRF
>> registers and enable the video datapath.
>>
>> Previously, the clock was enabled implicitly via the 'rockchip,vo-grf'
>> phandle reference, which allowed the eDP to work without explicitly
>> managing the hclk_vo1 clock. However, this is not safe or explicit.
>>
>> To make the clock dependency explicit, enforce per-SoC clock-names
>> requirements:
>>   - RK3288: 2 clocks (dp, pclk)
>>   - RK3399: 3 clocks (dp, pclk, grf)
>>   - RK3588: 3 clocks (dp, pclk, hclk)
>>
>> Do not reuse the 'grf' clock name for RK3588 because it represents
>> a different clock with distinct control logic:
>> - The 'grf' clock is only for GRF register access and is toggled
>>    dynamically during register access.
>> - The 'hclk' clock controls both GRF access and video datapath
>>    gating, and must remain enabled during probe.
>>
>> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v4:
>> - Modify the commit msg.
>>
>> Changes in v5:
>> - Enforce the correct third clock name on a per-compatible basis.
>> - Modify the commit msg simultaneously.
>>
>> Changes in v6:
>> - Expand more detail commit msg about using hclk instead of grf clock.
>> ---
>>   .../rockchip/rockchip,analogix-dp.yaml        | 37 +++++++++++++++++--
>>   1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> index d99b23b88cc5..8001c1facf98 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> @@ -23,10 +23,7 @@ properties:
>>   
>>     clock-names:
>>       minItems: 2
>> -    items:
>> -      - const: dp
>> -      - const: pclk
>> -      - const: grf
> 
> Instead of removing this, you can make it
> 
> items:
>   - const: dp
>   - const pclk
>   - enum:
>       - grf
>       - hclk
> 
>> +    maxItems: 3
> 
> And delete this.
> 

Oh I see, thanks for clarifying.

The idea is to keep all valid clock names at the top level, and use only 
minItems/maxItems in allOf to differentiate platforms.

>>   
>>     power-domains:
>>       maxItems: 1
>> @@ -60,6 +57,33 @@ required:
>>   allOf:
>>     - $ref: /schemas/display/bridge/analogix,dp.yaml#
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - rockchip,rk3288-dp
>> +    then:
>> +      properties:
>> +        clock-names:
>> +          items:
>> +            - const: dp
>> +            - const: pclk
> 
> Then this becomes
>    clock-names:
>      maxItems: 2
>    clocks:
>      maxItems: 2
> 
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - rockchip,rk3399-edp
>> +    then:
>> +      properties:
>> +        clock-names:
>> +          items:
>> +            - const: dp
>> +            - const: pclk
>> +            - const: grf
> 
> maybe this needs a minItems: 3? Depends on if the grf clock is
> mandatory. Also probably needs a
> clocks:
>    minItems: 3
> if that's the case.
> 

Yes, the minItems should be 3 for both clocks and clock-names.

>> +
>>     - if:
>>         properties:
>>           compatible:
>> @@ -68,6 +92,11 @@ allOf:
>>                 - rockchip,rk3588-edp
>>       then:
>>         properties:
>> +        clock-names:
>> +          items:
>> +            - const: dp
>> +            - const: pclk
>> +            - const: hclk
> 
> And the same here as for the 3399.
> 

Will update in v7.

Best regards,
Damon


^ permalink raw reply

* Re: [PATCH v7 3/4] soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers
From: Bjorn Andersson @ 2026-05-22  3:06 UTC (permalink / raw)
  To: Mukesh Kumar Savaliya
  Cc: viken.dadhaniya, andi.shyti, robh, krzk+dt, conor+dt, vkoul,
	Frank.Li, konradybcio, dmitry.baryshkov, linmq006, quic_jseerapu,
	agross, linux-arm-msm, linux-i2c, devicetree, linux-kernel,
	dmaengine, krzysztof.kozlowski, bartosz.golaszewski,
	bjorn.andersson, konrad.dybcio
In-Reply-To: <20260423145705.545552-4-mukesh.savaliya@oss.qualcomm.com>

On Thu, Apr 23, 2026 at 08:25:50PM +0530, Mukesh Kumar Savaliya wrote:
> On platforms where a GENI Serial Engine is shared with another system
> processor, selecting the "sleep" pinctrl state can disrupt ongoing
> transfers initiated by the other processor.
> 

Isn't it strange that the DeviceTree will define a sleep state for the
OS to select, but when this other property is set the OS should never
select this state?

> Teach geni_se_resources_off() to skip selecting the pinctrl sleep state
> when the Serial Engine is marked as shared, while still allowing the
> rest of the resource shutdown sequence to proceed.
> 
> This is required for multi-owner configurations (described via DeviceTree
> with qcom,qup-multi-owner on the protocol controller node).
> 

The requirement as such is reasonable, but you don't define in the
binding that when this property is set, the sleep state must not be
selected by the OS...

Regards,
Bjorn

> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/qcom-geni-se.c  | 15 +++++++++++----
>  include/linux/soc/qcom/geni-se.h |  2 ++
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index cd1779b6a91a..1a60832ace16 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -597,10 +597,17 @@ int geni_se_resources_off(struct geni_se *se)
>  
>  	if (has_acpi_companion(se->dev))
>  		return 0;
> -
> -	ret = pinctrl_pm_select_sleep_state(se->dev);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * Select the "sleep" pinctrl state only when the serial engine is
> +	 * exclusively owned by this system processor. For shared controller
> +	 * configurations, another system processor may still be using the pins,
> +	 * and switching them to "sleep" can disrupt ongoing transfers.
> +	 */
> +	if (!se->multi_owner) {
> +		ret = pinctrl_pm_select_sleep_state(se->dev);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	geni_se_clks_off(se);
>  	return 0;
> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
> index 0a984e2579fe..46217cac73c3 100644
> --- a/include/linux/soc/qcom/geni-se.h
> +++ b/include/linux/soc/qcom/geni-se.h
> @@ -63,6 +63,7 @@ struct geni_icc_path {
>   * @num_clk_levels:	Number of valid clock levels in clk_perf_tbl
>   * @clk_perf_tbl:	Table of clock frequency input to serial engine clock
>   * @icc_paths:		Array of ICC paths for SE
> + * @multi_owner:	True if SE is shared between multiple owners.
>   */
>  struct geni_se {
>  	void __iomem *base;
> @@ -72,6 +73,7 @@ struct geni_se {
>  	unsigned int num_clk_levels;
>  	unsigned long *clk_perf_tbl;
>  	struct geni_icc_path icc_paths[3];
> +	bool multi_owner;
>  };
>  
>  /* Common SE registers */
> -- 
> 2.43.0
> 

^ permalink raw reply

* Re: Re: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
From: 李志 @ 2026-05-22  2:52 UTC (permalink / raw)
  To: Conor Dooley; +Cc: sashiko-reviews, robh, devicetree, conor+dt
In-Reply-To: <20260519-presuming-arrest-718fc461b055@spud>




> -----Original Messages-----
> From: "Conor Dooley" <conor@kernel.org>
> Send time:Tuesday, 19/05/2026 17:55:27
> To: 李志 <lizhi2@eswincomputing.com>
> Cc: sashiko-reviews@lists.linux.dev, robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
> Subject: Re: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> 
> On Tue, May 19, 2026 at 05:51:05PM +0800, 李志 wrote:
> > 
> > 
> > 
> > > -----Original Messages-----
> > > From: "Conor Dooley" <conor@kernel.org>
> > > Send time:Tuesday, 19/05/2026 17:11:59
> > > To: sashiko-reviews@lists.linux.dev
> > > Cc: lizhi2@eswincomputing.com, robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
> > > Subject: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > > 
> > > On Tue, May 19, 2026 at 02:23:33AM +0000, sashiko-bot@kernel.org wrote:
> > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > > - [Medium] Missing `minItems: 4` constraint makes the newly added optional cells mandatory, breaking schema backward compatibility.
> > > > --
> > > > 
> > > > commit 2992483df88af2823c275fc86187f7e38acbee5c
> > > > Author: Zhi Li <lizhi2@eswincomputing.com>
> > > > 
> > > > dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > > > 
> > > > This commit documents two optional cells in the eswin,hsp-sp-csr property
> > > > for the TXD and RXD delay control register offsets.
> > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > > --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > > +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > 
> > > > [ ... ]
> > > > 
> > > > > @@ -82,6 +91,8 @@ properties:
> > > > >            - description: Offset of AXI clock controller Low-Power request
> > > > >                           register
> > > > >            - description: Offset of register controlling TX/RX clock delay
> > > > > +          - description: Optional offset of register controlling TXD delay
> > > > > +          - description: Optional offset of register controlling RXD delay
> > > > 
> > > > Will this make the two new offsets strictly required instead of optional?
> > > > 
> > > > In dt-schema, when an items list defines a tuple, minItems implicitly
> > > > defaults to the total number of items in that list. By increasing the list
> > > > from 4 to 6 items without adding a minItems: 4 constraint to the inner items
> > > > list, won't validation now strictly require exactly 6 items?
> > > > 
> > > > Could this cause dt_binding_check to fail for existing device trees that
> > > > correctly specify only the original 4 elements?
> > > 
> > > Looks like a valid complaint to me.
> > > pw-bot: changes-requested
> > > 
> > You're right, this is a valid issue.
> > 
> > I'll fix it in v3 by adding only minItems: 4 to preserve backward
> > compatibility with existing 4-cell device trees.
> > 
> > Should I drop your Acked-by in v3 due to the schema change, or should
> > it be retained?
> 
> Just fix it, and you can retain.

Hi Conor,

I noticed that the v2 series has already been applied to net.git:
https://lore.kernel.org/lkml/177935821264.4013498.4647676520552839707.git-patchwork-notify@kernel.org/

Since the missing minItems: 4 breaks backward compatibility for
existing 4-cell device trees, I plan to send a new single-patch series
targeting net to add the missing constraint.

Just wanted to confirm that this is the preferred approach.

Thanks,
Zhi

^ permalink raw reply

* Re: [PATCH 2/2] soc: qcom: Add CDSP power management driver
From: Bjorn Andersson @ 2026-05-22  2:49 UTC (permalink / raw)
  To: Vignesh Viswanathan
  Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown, linux-arm-msm, devicetree,
	linux-kernel, Praveenkumar I, Manikanta Mylavarapu
In-Reply-To: <20260520-cdsp-power-v1-2-85eb9501a1cd@oss.qualcomm.com>

On Wed, May 20, 2026 at 12:35:10AM +0530, Vignesh Viswanathan wrote:
> From: Praveenkumar I <praveenkumar.i@oss.qualcomm.com>
> 
> On platforms like IPQ9650, the CDSP subsystem cannot manage its own
> power rails and requires the APSS to handle power management on its
> behalf. Add a platform driver to fulfill this role.
> 

Why complement one incomplete driver with another for the same piece of
hardware rather than fixing the first driver?

> Handle LPM (Low Power Mode) by executing hardware isolation and restoration
> sequences via MPM register programming, coordinated with the NSP Q6 through
> a handshake protocol. Support both FULL_PC (CX+MX collapse) and LONG_APCR
> (CX-only) modes, detected dynamically at runtime.
> 
> Handle DCVS (Dynamic Clock and Voltage Scaling) requests from the NSP Q6
> received via a shared SMEM channel. Apply voltages via the regulator
> framework and write responses back to SMEM.
> 

How does this actually work?

I presume you intend to pair this with the standard PAS driver, which
will disable the voltage votes as soon as the handover IRQ comes. So how
will the subsystem be able to notify you through this SMEM interface?


You did something non-standard, now you need to document it properly.

> Expose virtual cdsp-vdd-cx and cdsp-vdd-mx regulators so that the PAS
> remoteproc driver can control the NSP power rails through the standard
> regulator framework.
> 

Write a new remoteproc driver for this non-standard subsystem. It might
resemble a standard PAS subsystem, but it clearly isn't.

> Signed-off-by: Praveenkumar I <praveenkumar.i@oss.qualcomm.com>
> Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
> Signed-off-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/Kconfig      |   17 +
>  drivers/soc/qcom/Makefile     |    1 +
>  drivers/soc/qcom/cdsp_power.c | 1065 +++++++++++++++++++++++++++++++++++++++++

We have a framework in the Linux kernel for managing subsystems, it's
called remoteproc, that's where this belongs.

>  3 files changed, 1083 insertions(+)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 2caadbbcf830..f4b9204d4e9a 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -26,6 +26,23 @@ config QCOM_COMMAND_DB
>  	  resource on a RPM-hardened platform must use this database to get
>  	  SoC specific identifier and information for the shared resources.
>  
> +config QCOM_CDSP_POWER
> +	tristate "Qualcomm CDSP Power Management driver"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on QCOM_SMEM
> +	depends on REGULATOR
> +	help
> +	  This driver manages power for the CDSP (Compute DSP) subsystem on
> +	  Qualcomm platforms. It handles DCVS (Dynamic Clock and Voltage Scaling)
> +	  for voltage scaling via SMEM/SMP2P, and LPM (Low Power Mode) for power
> +	  collapse/restore via MPM handshake.
> +
> +	  The driver coordinates with the cdsp_rproc driver for shared regulator
> +	  access and provides runtime voltage scaling and power management for
> +	  the CDSP subsystem.
> +
> +	  Say M here if you want to include support for CDSP power management.
> +
>  config QCOM_GENI_SE
>  	tristate "QCOM GENI Serial Engine Driver"
>  	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index b7f1d2a57367..f3490a437cb8 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -40,3 +40,4 @@ qcom_ice-objs			+= ice.o
>  obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)	+= qcom_ice.o
>  obj-$(CONFIG_QCOM_PBS) +=	qcom-pbs.o
>  obj-$(CONFIG_QCOM_UBWC_CONFIG) += ubwc_config.o
> +obj-$(CONFIG_QCOM_CDSP_POWER) += cdsp_power.o
> diff --git a/drivers/soc/qcom/cdsp_power.c b/drivers/soc/qcom/cdsp_power.c
> new file mode 100644
> index 000000000000..97aff858b139
> --- /dev/null
> +++ b/drivers/soc/qcom/cdsp_power.c
> @@ -0,0 +1,1065 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + *
> + * CDSP Power Management Driver for IPQ9650
> + *
> + * Regulator ownership hierarchy:
> + *   PAS remoteproc (cx-supply / mx-supply)
> + *       -> cdsp-vdd-cx / cdsp-vdd-mx  (virtual, provided by this driver)
> + *           -> vdd-cx / vdd-mx        (voltage regulators, consumed by this driver)
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/ktime.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/workqueue.h>
> +
> +/* SMEM Protocol Definitions */
> +/* NSP Q6 SMEM host ID */
> +#define CDSP_SMEM_NSP_HOST_ID	5
> +/* hdr(64) + request_area(1024) + response_area(256) */

You have those constant below, so you can write that in code.

> +#define CDSP_SMEM_SIZE		1344
> +/* Protocol magic number 'RPMH' */
> +#define CDSP_SMEM_MAGIC		0x52504D48
> +/* Protocol version v1.0 */
> +#define CDSP_SMEM_VERSION	0x00010001
> +#define CDSP_MSG_ID_REQUEST	0x01
> +#define CDSP_MSG_ID_RESPONSE	0x02
> +#define CDSP_RESP_MSG_SIZE	24

You mean sizeof(struct cdsp_smem_response)?

> +#define CDSP_MAX_KVP		64
> +
> +/* SMEM layout fixed offsets (relative to start of SMEM item) */
> +#define CDSP_SMEM_REQUEST_OFFSET	64
> +#define CDSP_SMEM_REQUEST_SIZE		1024
> +#define CDSP_SMEM_RESPONSE_OFFSET	1088
> +#define CDSP_SMEM_RESPONSE_SIZE		256
> +
> +/* NSP CX voltage rail resource ID */
> +#define CDSP_RESOURCE_ID_CX	0x03
> +/* NSP MX voltage rail resource ID */
> +#define CDSP_RESOURCE_ID_MX	0x04
> +
> +/* MPM Register Offsets */
> +#define RSC_HDSHK_IRQ_STAT		0x0004
> +#define CLIENT_RSC_HDSHK(n)		(0x0010 + (n) * 0x10)
> +#define CLIENT_RSC_IRQ_STAT(n)		(0x0014 + (n) * 0x10)
> +#define CLIENT_RSC_IRQ_CLR(n)		(0x0018 + (n) * 0x10)
> +#define VDD_RAIL_ISO_CTRL(n)		(0x0330 + (n) * 0x4)
> +
> +/* RSCC Register Offsets */
> +#define RSCC_BR_EVENT			0x0
> +#define RSCC_BR_EVENT_PC_MODE		BIT(3)
> +
> +/* MPM Handshake Bits */
> +#define MPM_SHUTDOWN_REQ		BIT(0)
> +#define MPM_SHUTDOWN_ACK		BIT(1)
> +#define MPM_BRINGUP_REQ			BIT(2)
> +#define MPM_BRINGUP_ACK			BIT(3)
> +
> +/* MPM Isolation Control Bits */
> +#define ISO_CLK_DIS			BIT(0)
> +#define ISO_CLK_DIS_ACK			BIT(1)
> +#define ISO_INPUT			BIT(8)
> +#define ISO_INPUT_DFT			BIT(9)
> +#define ISO_INPUT_CLKS			BIT(10)
> +#define ISO_CLAMP_MEM			BIT(11)
> +#define ISO_RET_0PIN			BIT(16)
> +#define ISO_SAVE_FF			BIT(17)
> +#define ISO_RESTORE_FF			BIT(18)
> +#define ISO_FREEZE_OUTPUT		BIT(24)
> +#define ISO_PWR_UP_RESET		BIT(25)
> +
> +/* MXC rail mapped to isolation control register index 0 */
> +#define VDD_RAIL_MX			0
> +/* NSP/CX rail mapped to isolation control register index 1 */
> +#define VDD_RAIL_CX			1
> +
> +/* Power States */
> +#define CDSP_POWER_OFF			0
> +#define CDSP_POWER_ON			1
> +
> +/* Timeouts */
> +#define MPM_POLL_TIMEOUT_US		10000
> +#define MPM_POLL_SLEEP_US		10
> +
> +/* Virtual regulator IDs */
> +enum cdsp_virt_reg_id {
> +	CDSP_VIRT_NSP_CX = 0,
> +	CDSP_VIRT_NSP_MX = 1,
> +	CDSP_VIRT_MAX,
> +};
> +
> +/**
> + * struct cdsp_smem_channel_hdr - SMEM channel header (64 bytes, at offset 0x000)
> + * @magic:             Protocol magic number (0x52504D48 = 'RPMH')
> + * @version:           Protocol version (0x00010001 = v1.0)
> + * @cdsp_state:        NSP Q6 readiness flag; 0 = not ready, 1 = ready (written by NSP)
> + * @apss_state:        APSS readiness flag; 0 = not ready, 1 = ready (written by APSS)
> + * @request_offset:    Byte offset of the request area from the start of the SMEM item
> + * @request_size:      Size of the request area in bytes
> + * @response_offset:   Byte offset of the response area from the start of the SMEM item
> + * @response_size:     Size of the response area in bytes
> + * @request_in_flight: Channel busy flag; 0 = idle, 1 = busy (NSP sets, APSS clears)
> + * @cdsp_to_apss_irq:  IPCC signal ID used for NSP-to-APSS interrupts
> + * @apss_to_cdsp_irq:  IPCC signal ID used for APSS-to-NSP interrupts
> + * @cdsp_tx_count:     Number of DCVS requests sent by NSP
> + * @cdsp_rx_count:     Number of DCVS responses received by NSP
> + * @apss_tx_count:     Number of DCVS responses sent by APSS
> + * @apss_rx_count:     Number of DCVS requests received by APSS
> + * @reserved:          Reserved, must be zero
> + *
> + * Fixed at offset 0x000 in the SMEM item. Matches rpmh_smem_channel_header_t
> + * on the NSP Q6 side. APSS initialises this at probe time; NSP Q6 reads it
> + * to discover the request/response area offsets and to synchronise readiness.
> + */
> +struct cdsp_smem_channel_hdr {
> +	u32 magic;

I presume these are __le32?

> +	u32 version;
> +	u32 cdsp_state;
> +	u32 apss_state;
> +	u32 request_offset;
> +	u32 request_size;
> +	u32 response_offset;
> +	u32 response_size;
> +	u32 request_in_flight;
> +	u32 cdsp_to_apss_irq;
> +	u32 apss_to_cdsp_irq;
> +	u32 cdsp_tx_count;
> +	u32 cdsp_rx_count;
> +	u32 apss_tx_count;
> +	u32 apss_rx_count;
> +	u32 reserved;
> +} __packed;
> +
> +/**
> + * struct cdsp_kvp - Key-Value-Pair entry in a DCVS request
> + * @key:    Resource identifier; upper 16 bits = CDSP_RESOURCE_ID_CX or _MX
> + * @length: Length of the value field in bytes (always 4)
> + * @value:  Requested voltage in microvolts
> + */
> +struct cdsp_kvp {
> +	u32 key;
> +	u32 length;
> +	u32 value;
> +} __packed;
> +
> +/**
> + * struct cdsp_smem_request - DCVS request message written by NSP Q6, read by APSS
> + * @msg_size:     Total message size in bytes
> + * @sequence:     Monotonically increasing sequence number
> + * @msg_id:       Message type; CDSP_MSG_ID_REQUEST (0x01) for DCVS requests
> + * @req_id:       Request identifier echoed in the response
> + * @set:          Power set being requested (ACTIVE / SLEEP / WAKE)
> + * @num_commands: Number of KVP entries that follow
> + * @timestamp_us: Request timestamp in microseconds
> + * @kvp:          Array of key-value-pair voltage commands (up to CDSP_MAX_KVP)
> + */
> +struct cdsp_smem_request {
> +	u32 msg_size;
> +	u32 sequence;
> +	u32 msg_id;
> +	u32 req_id;
> +	u32 set;
> +	u32 num_commands;
> +	u32 timestamp_us;
> +	struct cdsp_kvp kvp[CDSP_MAX_KVP];
> +} __packed;
> +
> +/**
> + * struct cdsp_smem_response - DCVS response message written by APSS, read by NSP Q6
> + * @msg_size:     Total message size in bytes (CDSP_RESP_MSG_SIZE = 24)
> + * @sequence:     Echo of the request sequence number
> + * @msg_id:       Message type; CDSP_MSG_ID_RESPONSE (0x02)
> + * @status:       Result code; 0 on success, negative errno on failure
> + * @data:         Actual voltage applied in microvolts (valid when status == 0)
> + * @timestamp_us: Completion timestamp in microseconds
> + */
> +struct cdsp_smem_response {
> +	u32 msg_size;
> +	u32 sequence;
> +	u32 msg_id;
> +	u32 status;
> +	u32 data;
> +	u32 timestamp_us;
> +} __packed;
> +
> +/**
> + * struct cdsp_smem_region - Full SMEM item layout (1344 bytes total)
> + * @hdr:       Channel header at offset 0x000 (64 bytes)
> + * @request:   DCVS request area at offset 0x040 (padded to 1024 bytes)
> + * @_req_pad:  Padding to align response area to offset 0x440
> + * @response:  DCVS response area at offset 0x440 (padded to 256 bytes)
> + * @_resp_pad: Padding to complete the 256-byte response area
> + *
> + * Layout matches the rpmh_smem_channel_header_t offsets on the NSP Q6 side.
> + */
> +struct cdsp_smem_region {
> +	struct cdsp_smem_channel_hdr hdr;
> +	struct cdsp_smem_request  request;
> +	u8 _req_pad[228];
> +	struct cdsp_smem_response response;
> +	u8 _resp_pad[232];
> +} __packed;
> +
> +/**
> + * struct cdsp_power_driver - Main driver context
> + * @dev: Device pointer
> + * @smem: Pointer to SMEM region
> + * @dcvs_mbox_client: Mailbox client for DCVS response interrupt (APSS->NSP)
> + * @dcvs_mbox_chan: Mailbox channel for DCVS response interrupt
> + * @dcvs_irq: DCVS interrupt number (NSP->APSS IPCC PING)
> + * @dcvs_work: Work structure for DCVS processing
> + * @mpm_regmap: Regmap for MPM handshake registers
> + * @lpm_irq: LPM interrupt number
> + * @lpm_work: Work structure for LPM processing
> + * @rscc_regmap: Regmap for RSCC power mode detection register
> + * @vdd_cx: CX voltage regulator (consumer handle)
> + * @vdd_mx: MX voltage regulator (consumer handle, NULL if absent on this board)
> + * @power_state: Current NSP power state (CDSP_POWER_ON / CDSP_POWER_OFF)
> + * @lock: Mutex serialising DCVS and LPM work
> + */
> +struct cdsp_power_driver {
> +	struct device *dev;
> +
> +	/* SMEM for DCVS */
> +	struct cdsp_smem_region *smem;
> +
> +	/* Mbox for DCVS response (APSS -> NSP via IPCC PING) */
> +	struct mbox_client dcvs_mbox_client;
> +	struct mbox_chan   *dcvs_mbox_chan;
> +
> +	int dcvs_irq;
> +	struct work_struct dcvs_work;
> +
> +	/* MPM for LPM */
> +	struct regmap *mpm_regmap;
> +	int lpm_irq;
> +	struct workqueue_struct *lpm_wq;
> +	struct work_struct lpm_work;
> +
> +	/* RSCC for power mode detection */
> +	struct regmap *rscc_regmap;
> +
> +	/* PMIC regulator consumer handles */
> +	struct regulator *vdd_cx;
> +	struct regulator *vdd_mx;
> +
> +	/* State tracking */
> +	atomic_t power_state;
> +	/* Mutex serialising DCVS and LPM work */
> +	struct mutex lock;
> +};
> +
> +/**
> + * cdsp_virt_reg_enable() - Enable a virtual NSP regulator
> + * @rdev: Regulator device
> + *
> + * Passes the enable request through to the underlying PMIC consumer handle
> + * (vdd_cx or vdd_mx). Returns 0 immediately if the MX rail is absent on
> + * this board.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cdsp_virt_reg_enable(struct regulator_dev *rdev)
> +{
> +	struct cdsp_power_driver *drv = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct regulator *reg = (id == CDSP_VIRT_NSP_CX) ? drv->vdd_cx : drv->vdd_mx;
> +
> +	if (!reg)
> +		return 0;
> +	return regulator_enable(reg);
> +}
> +
> +/**
> + * cdsp_virt_reg_is_enabled() - Check if a virtual NSP regulator is enabled
> + * @rdev: Regulator device
> + *
> + * Queries the enable state of the underlying PMIC consumer handle (vdd_cx
> + * or vdd_mx). Returns 1 if the MX rail is absent on this board, since the
> + * MX hardware rail is always on in that configuration.
> + *
> + * Return: 1 if enabled (or absent), 0 if disabled, negative error code on failure
> + */
> +static int cdsp_virt_reg_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct cdsp_power_driver *drv = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct regulator *reg = (id == CDSP_VIRT_NSP_CX) ? drv->vdd_cx : drv->vdd_mx;
> +
> +	if (!reg)
> +		return 1;
> +	return regulator_is_enabled(reg);
> +}
> +
> +/**
> + * cdsp_virt_reg_disable() - Disable a virtual NSP regulator
> + * @rdev: Regulator device
> + *
> + * Passes the disable request through to the underlying PMIC consumer handle
> + * (vdd_cx or vdd_mx). Returns 0 immediately if the MX rail is absent on
> + * this board.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cdsp_virt_reg_disable(struct regulator_dev *rdev)
> +{
> +	struct cdsp_power_driver *drv = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct regulator *reg = (id == CDSP_VIRT_NSP_CX) ? drv->vdd_cx : drv->vdd_mx;
> +
> +	if (!reg)
> +		return 0;
> +
> +	/* Disable the regulator if it's enabled */
> +	if (cdsp_virt_reg_is_enabled(rdev))
> +		return regulator_disable(reg);
> +
> +	return 0;
> +}
> +
> +static const struct regulator_ops cdsp_virt_reg_ops = {
> +	.enable      = cdsp_virt_reg_enable,
> +	.disable     = cdsp_virt_reg_disable,
> +	.is_enabled  = cdsp_virt_reg_is_enabled,
> +};
> +
> +static const struct regulator_desc cdsp_virt_reg_descs[CDSP_VIRT_MAX] = {
> +	[CDSP_VIRT_NSP_CX] = {
> +		.id              = CDSP_VIRT_NSP_CX,
> +		.name            = "cdsp-vdd-cx",
> +		.of_match        = "cdsp-vdd-cx",
> +		.regulators_node = "regulators",
> +		.ops             = &cdsp_virt_reg_ops,
> +		.type            = REGULATOR_VOLTAGE,
> +		.owner           = THIS_MODULE,
> +	},
> +	[CDSP_VIRT_NSP_MX] = {
> +		.id              = CDSP_VIRT_NSP_MX,
> +		.name            = "cdsp-vdd-mx",
> +		.of_match        = "cdsp-vdd-mx",
> +		.regulators_node = "regulators",
> +		.ops             = &cdsp_virt_reg_ops,
> +		.type            = REGULATOR_VOLTAGE,
> +		.owner           = THIS_MODULE,
> +	},
> +};
> +
> +/**
> + * cdsp_is_full_pc_mode() - Check if FULL_PC mode is selected
> + * @drv: Driver context
> + *
> + * Reads the CDSP RSCC_BR_EVENT register to determine power collapse mode.
> + * Bit 3: 0 = FULL_PC (collapse both CX and MX)
> + *        1 = LONG_APCR (collapse only CX)
> + *
> + * Return: true if FULL_PC mode, false if LONG_APCR mode
> + */
> +static bool cdsp_is_full_pc_mode(struct cdsp_power_driver *drv)
> +{
> +	u32 br_event;
> +	int ret;
> +
> +	ret = regmap_read(drv->rscc_regmap, RSCC_BR_EVENT, &br_event);
> +	if (ret) {
> +		dev_err(drv->dev, "Failed to read RSCC_BR_EVENT: %d\n", ret);
> +		return true; /* Default to FULL_PC on error */
> +	}
> +
> +	/* Bit 3: 0 = FULL_PC (collapse both CX and MX)
> +	 *        1 = LONG_APCR (collapse only CX)
> +	 */
> +	return !(br_event & RSCC_BR_EVENT_PC_MODE);
> +}
> +
> +/**
> + * cdsp_dcvs_irq_handler() - DCVS interrupt handler
> + * @irq: Interrupt number
> + * @data: Driver context
> + *
> + * Called when CDSP sends a DCVS request via IPCC PING interrupt.
> + * Schedules work to process the request.
> + *
> + * Return: IRQ_HANDLED
> + */
> +static irqreturn_t cdsp_dcvs_irq_handler(int irq, void *data)
> +{
> +	struct cdsp_power_driver *drv = data;
> +
> +	/* Schedule work to process DCVS request */
> +	schedule_work(&drv->dcvs_work);

Why have a threaded handler with the only purpose to schedule work?

> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * cdsp_dcvs_work_fn() - Process a DCVS voltage scaling request from the NSP Q6
> + * @work: Work structure embedded in struct cdsp_power_driver
> + *
> + * Reads the KVP request from the SMEM channel, validates the resource ID and
> + * voltage, applies the voltage via the regulator framework, writes the response
> + * back to SMEM, and signals the NSP Q6 via the IPCC PING mailbox channel.
> + */
> +static void cdsp_dcvs_work_fn(struct work_struct *work)
> +{
> +	struct cdsp_power_driver *drv = container_of(work,
> +						     struct cdsp_power_driver,
> +						     dcvs_work);
> +	struct cdsp_smem_region *smem = drv->smem;
> +	u32 sequence, num_commands;
> +	int ret = 0, actual_uv = 0;
> +	int i;
> +
> +	mutex_lock(&drv->lock);
> +
> +	/* Drop DCVS requests while the NSP Q6 is powered off */
> +	if (atomic_read(&drv->power_state) == CDSP_POWER_OFF) {
> +		dev_warn(drv->dev, "DCVS request while powered off, dropping\n");
> +		mutex_unlock(&drv->lock);
> +		return;
> +	}
> +
> +	/* Read request header from SMEM */
> +	rmb();
> +	sequence     = smem->request.sequence;
> +	num_commands = smem->request.num_commands;
> +
> +	/* Validate message type — only process DCVS requests */
> +	if (smem->request.msg_id != CDSP_MSG_ID_REQUEST) {
> +		dev_err(drv->dev, "Unexpected msg_id: 0x%x (expected 0x%x)\n",
> +			smem->request.msg_id, CDSP_MSG_ID_REQUEST);
> +		ret = -EINVAL;
> +		goto send_response;
> +	}
> +
> +	dev_dbg(drv->dev, "DCVS request: seq=%u, num_commands=%u, timestamp=%u us\n",
> +		sequence, num_commands, smem->request.timestamp_us);
> +
> +	if (num_commands > CDSP_MAX_KVP) {
> +		dev_err(drv->dev, "Too many KVP commands: %u (max %d)\n",
> +			num_commands, CDSP_MAX_KVP);
> +		ret = -EINVAL;
> +		goto send_response;
> +	}
> +
> +	/* Process each KVP: key=resource_type, value=voltage_uv */
> +	for (i = 0; i < num_commands; i++) {
> +		u32 key        = smem->request.kvp[i].key;
> +		u32 voltage_uv = smem->request.kvp[i].value;
> +		/* Upper 16 bits of the KVP key encode the resource ID */
> +		u32 resource_id = (key >> 16) & 0xFFFF;
> +		struct regulator *reg;
> +		int uv;
> +
> +		if (resource_id == CDSP_RESOURCE_ID_CX) {
> +			reg = drv->vdd_cx;
> +		} else if (resource_id == CDSP_RESOURCE_ID_MX) {
> +			if (!drv->vdd_mx) {
> +				dev_dbg(drv->dev,
> +					"KVP[%d]: MX rail not available on this board\n",
> +					i);
> +				ret = -EINVAL;
> +				goto send_response;
> +			}
> +			reg = drv->vdd_mx;
> +		} else {
> +			dev_err(drv->dev, "KVP[%d]: unknown key 0x%x\n", i, key);
> +			ret = -EINVAL;
> +			goto send_response;
> +		}
> +
> +		/* Set voltage; the regulator framework enforces DTS constraints */
> +		ret = regulator_set_voltage(reg, voltage_uv, voltage_uv);
> +		if (ret) {
> +			dev_err(drv->dev, "KVP[%d]: failed to set %s voltage %u uV: %d\n",
> +				i, resource_id == CDSP_RESOURCE_ID_CX ? "CX" : "MX",
> +				voltage_uv, ret);
> +			goto send_response;
> +		}
> +
> +		/* Read back actual voltage */
> +		uv = regulator_get_voltage(reg);
> +		if (uv < 0) {
> +			dev_warn(drv->dev, "KVP[%d]: failed to read back voltage: %d\n",
> +				 i, uv);
> +			uv = voltage_uv;
> +		}
> +		/* Track the last successfully set voltage to report in the response */
> +		actual_uv = uv;
> +
> +		dev_dbg(drv->dev, "DCVS: Set %s to %d uV (requested %u uV)\n",
> +			resource_id == CDSP_RESOURCE_ID_CX ? "CX" : "MX",
> +			actual_uv, voltage_uv);
> +	}
> +
> +send_response:
> +	/* Write response to SMEM response area */
> +	smem->response.msg_size     = CDSP_RESP_MSG_SIZE;
> +	smem->response.sequence     = sequence;
> +	smem->response.msg_id       = CDSP_MSG_ID_RESPONSE;
> +	smem->response.status       = ret;
> +	smem->response.data         = (ret == 0) ? actual_uv : 0;
> +	smem->response.timestamp_us = ktime_to_us(ktime_get());
> +
> +	/*
> +	 * Update header statistics and clear in-flight flag.
> +	 * Order: write response data -> clear request_in_flight -> wmb ->
> +	 * send interrupt. NSP Q6 checks request_in_flight == 0 to know
> +	 * the response is ready.
> +	 */
> +	smem->hdr.apss_rx_count++;
> +	smem->hdr.apss_tx_count++;
> +	/* Mark channel idle before sending the interrupt */
> +	WRITE_ONCE(smem->hdr.request_in_flight, 0);
> +	/* Ensure response data is visible before sending interrupt to NSP Q6 */
> +	wmb();
> +
> +	/* Send IPCC PING interrupt to NSP Q6 */
> +	mbox_send_message(drv->dcvs_mbox_chan, NULL);
> +	mbox_client_txdone(drv->dcvs_mbox_chan, 0);
> +
> +	mutex_unlock(&drv->lock);
> +}
> +
> +/**
> + * cdsp_execute_isolation_sequence() - Execute isolation and power-down sequence
> + * @drv: Driver context
> + *
> + * Executes the isolation sequence for power collapse as defined in
> + * the MPM power collapse specification, followed by regulator disable.
> + * Processes NSP/CX rail first (always), then MXC/MX rail (FULL_PC mode only).
> + */
> +static void cdsp_execute_isolation_sequence(struct cdsp_power_driver *drv)
> +{
> +	/*
> +	 * When vdd_mx is absent (board has no MX regulator handle), always
> +	 * execute FULL_PC isolation for both CX and MX rails regardless of
> +	 * what the Q6 requested.
> +	 */
> +	int num_rails = (!drv->vdd_mx || cdsp_is_full_pc_mode(drv)) ? 2 : 1;
> +	unsigned int iso_regs[] = {
> +		VDD_RAIL_ISO_CTRL(VDD_RAIL_CX),  /* NSP/CX rail - always processed */
> +		VDD_RAIL_ISO_CTRL(VDD_RAIL_MX),  /* MXC/MX rail - FULL_PC only */
> +	};
> +	struct regulator *regulators[] = { drv->vdd_cx, drv->vdd_mx };
> +	struct regmap *mpm_map = drv->mpm_regmap;
> +	int ret, i;
> +	u32 val;
> +
> +	for (i = 0; i < num_rails; i++) {
> +		unsigned int iso = iso_regs[i];
> +
> +		/* Step 1: Disable clocks */
> +		regmap_update_bits(mpm_map, iso, ISO_CLK_DIS, ISO_CLK_DIS);
> +
> +		/* Poll CLK_DIS_ACK only for NSP/CX rail (MXC CLK_DIS_ACK is tied to 0) */
> +		if (i == 0) {
> +			ret = regmap_read_poll_timeout(mpm_map, iso, val,
> +						       (val & ISO_CLK_DIS_ACK),
> +						       MPM_POLL_SLEEP_US, MPM_POLL_TIMEOUT_US);
> +			if (ret)
> +				dev_err(drv->dev, "Timeout waiting for NSP CLK_DIS_ACK\n");
> +		}
> +
> +		/* Step 2: Isolate inputs */
> +		regmap_update_bits(mpm_map, iso,
> +				   ISO_INPUT | ISO_INPUT_CLKS | ISO_INPUT_DFT,
> +				   ISO_INPUT | ISO_INPUT_CLKS | ISO_INPUT_DFT);
> +		/* Step 3: Clamp memories */
> +		regmap_update_bits(mpm_map, iso, ISO_CLAMP_MEM, ISO_CLAMP_MEM);
> +		/* Step 4: Enable 0-pin retention */
> +		regmap_update_bits(mpm_map, iso, ISO_RET_0PIN, ISO_RET_0PIN);
> +		/* Step 5: Save to balloon latch (low-high-low pulse) */
> +		regmap_update_bits(mpm_map, iso, ISO_SAVE_FF, 0x0);
> +		regmap_update_bits(mpm_map, iso, ISO_SAVE_FF, ISO_SAVE_FF);
> +		regmap_update_bits(mpm_map, iso, ISO_SAVE_FF, 0x0);
> +		/* Step 6: Prepare restore signal (low-high) */
> +		regmap_update_bits(mpm_map, iso, ISO_RESTORE_FF, ISO_RESTORE_FF);
> +		/* Step 7: Freeze outputs */
> +		regmap_update_bits(mpm_map, iso, ISO_FREEZE_OUTPUT, ISO_FREEZE_OUTPUT);
> +
> +		/* Step 8: Turn off regulator */
> +		if (regulators[i]) {
> +			ret = regulator_disable(regulators[i]);
> +			if (ret)
> +				dev_err(drv->dev, "Failed to disable %s rail: %d\n",
> +					i == 0 ? "CX" : "MX", ret);
> +			else
> +				usleep_range(8000, 10000);
> +		}
> +	}
> +
> +	dev_dbg(drv->dev, "Isolation sequence complete (%s mode)\n",
> +		num_rails > 1 ? "FULL_PC" : "LONG_APCR");
> +}
> +
> +/**
> + * cdsp_execute_restoration_sequence() - Execute 9-step restoration sequence
> + * @drv: Driver context
> + *
> + * Executes the restoration sequence for power restore as defined in
> + * the MPM power collapse specification.
> + * FULL_PC: Restores MXC (MX) rail first, then NSP (CX) rail
> + * LONG_APCR: Restores NSP (CX) rail only
> + */
> +static void cdsp_execute_restoration_sequence(struct cdsp_power_driver *drv)
> +{
> +	/*
> +	 * When vdd_mx is absent, always restore both CX and MX rails
> +	 * (FULL_PC), but skip regulator_enable for MX (no handle).
> +	 */
> +	int start_rail = (!drv->vdd_mx || cdsp_is_full_pc_mode(drv)) ? 0 : 1;
> +	unsigned int iso_regs[] = {
> +		VDD_RAIL_ISO_CTRL(VDD_RAIL_MX),  /* MXC/MX rail - FULL_PC only, restored first */
> +		VDD_RAIL_ISO_CTRL(VDD_RAIL_CX),  /* NSP/CX rail - always restored */
> +	};
> +	struct regulator *regulators[] = { drv->vdd_mx, drv->vdd_cx };
> +	struct regmap *mpm_map = drv->mpm_regmap;
> +	static const char * const rail_names[] = { "MX", "CX" };
> +	int ret, i;
> +
> +	for (i = start_rail; i < ARRAY_SIZE(iso_regs); i++) {
> +		unsigned int iso = iso_regs[i];
> +
> +		/* Step 1: Assert power-up reset */
> +		regmap_update_bits(mpm_map, iso, ISO_PWR_UP_RESET, ISO_PWR_UP_RESET);
> +
> +		/* Step 2: Enable power rail while reset is asserted (skip if no handle) */
> +		if (regulators[i]) {
> +			ret = regulator_enable(regulators[i]);
> +			if (ret)
> +				dev_err(drv->dev, "Failed to enable %s: %d\n", rail_names[i], ret);
> +			else
> +				usleep_range(8000, 10000);
> +		}
> +
> +		/* Step 3: Clear power-up reset */
> +		regmap_update_bits(mpm_map, iso, ISO_PWR_UP_RESET, 0);
> +		/* Step 4: Restore from balloon latch */
> +		regmap_update_bits(mpm_map, iso, ISO_RESTORE_FF, 0);
> +		/* Step 5: Remove memory clamp */
> +		regmap_update_bits(mpm_map, iso, ISO_CLAMP_MEM, 0);
> +		/* Step 6: Restore from 0-pin retention */
> +		regmap_update_bits(mpm_map, iso, ISO_RET_0PIN, 0);
> +		/* Step 7: Remove output freeze */
> +		regmap_update_bits(mpm_map, iso, ISO_FREEZE_OUTPUT, 0);
> +		/* Step 8: Remove input isolation */
> +		regmap_update_bits(mpm_map, iso, ISO_INPUT | ISO_INPUT_DFT | ISO_INPUT_CLKS, 0);
> +		/* Step 9: Enable clocks */
> +		regmap_update_bits(mpm_map, iso, ISO_CLK_DIS, 0);
> +	}
> +
> +	dev_dbg(drv->dev, "Restoration sequence complete (%s mode)\n",
> +		start_rail == 0 ? "FULL_PC" : "LONG_APCR");
> +}
> +
> +/**
> + * cdsp_lpm_shutdown_sequence() - Execute LPM shutdown sequence
> + * @drv: Driver context
> + *
> + * Handles power collapse request from CDSP:
> + * 1. Completes MPM 4-phase handshake
> + * 2. Executes isolation sequence
> + * 3. Disables regulators
> + *
> + * The regulator_disable() calls here are balanced against the
> + * regulator_enable() calls made by PAS remoteproc via the virtual
> + * vdd-cx/vdd-mx regulators (which pass through to vdd_cx/vdd_mx).
> + */
> +static void cdsp_lpm_shutdown_sequence(struct cdsp_power_driver *drv)
> +{
> +	struct regmap *mpm_map = drv->mpm_regmap;
> +	bool is_full_pc = cdsp_is_full_pc_mode(drv);
> +	u32 hdshk_status;
> +	int ret;
> +
> +	dev_dbg(drv->dev, "LPM: Starting power collapse sequence\n");
> +
> +	/* Phase 1: Acknowledge shutdown request */
> +	regmap_update_bits(mpm_map, CLIENT_RSC_HDSHK(0), MPM_SHUTDOWN_ACK, MPM_SHUTDOWN_ACK);
> +
> +	/* Phase 2: Wait for request de-assertion */
> +	ret = regmap_read_poll_timeout(mpm_map, CLIENT_RSC_HDSHK(0), hdshk_status,
> +				       !(hdshk_status & MPM_SHUTDOWN_REQ),
> +				       MPM_POLL_SLEEP_US, MPM_POLL_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(drv->dev, "Timeout waiting for shutdown_req clear\n");
> +		return;
> +	}
> +
> +	/* Phase 3: Clear acknowledgment */
> +	regmap_update_bits(mpm_map, CLIENT_RSC_HDSHK(0), MPM_SHUTDOWN_ACK, 0x0);
> +
> +	/* Phase 4: Execute isolation sequence */
> +	cdsp_execute_isolation_sequence(drv);
> +
> +	atomic_set(&drv->power_state, CDSP_POWER_OFF);
> +
> +	dev_dbg(drv->dev, "LPM: Power collapse complete (%s mode)\n",
> +		is_full_pc ? "FULL_PC" : "LONG_APCR");
> +}
> +
> +/**
> + * cdsp_lpm_restore_sequence() - Execute LPM restore sequence
> + * @drv: Driver context
> + *
> + * Handles power restore request from CDSP:
> + * 1. Executes restoration sequence (includes regulator enable)
> + * 2. Completes MPM bringup handshake
> + */
> +static void cdsp_lpm_restore_sequence(struct cdsp_power_driver *drv)
> +{
> +	struct regmap *mpm_map = drv->mpm_regmap;
> +	u32 hdshk_status;
> +	int ret;
> +
> +	dev_dbg(drv->dev, "LPM: Starting power restore sequence\n");
> +
> +	/* Execute restoration sequence (includes regulator enable) */
> +	cdsp_execute_restoration_sequence(drv);
> +
> +	/* Assert BRINGUP_ACK */
> +	regmap_update_bits(mpm_map, CLIENT_RSC_HDSHK(0), MPM_BRINGUP_ACK, MPM_BRINGUP_ACK);
> +
> +	/* Wait for BRINGUP_REQ to de-assert */
> +	ret = regmap_read_poll_timeout(mpm_map, CLIENT_RSC_HDSHK(0), hdshk_status,
> +				       !(hdshk_status & MPM_BRINGUP_REQ),
> +				       MPM_POLL_SLEEP_US, MPM_POLL_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(drv->dev, "Timeout waiting for BRINGUP_REQ clear\n");
> +		return;
> +	}
> +
> +	/* Clear BRINGUP_ACK */
> +	regmap_update_bits(mpm_map, CLIENT_RSC_HDSHK(0), MPM_BRINGUP_ACK, 0);
> +
> +	atomic_set(&drv->power_state, CDSP_POWER_ON);
> +
> +	dev_dbg(drv->dev, "LPM: Power restore complete\n");
> +}
> +
> +/**
> + * cdsp_lpm_irq_handler() - LPM interrupt handler
> + * @irq: Interrupt number
> + * @data: Driver context
> + *
> + * Called when CDSP sends an LPM request via MPM.
> + * Schedules work to process the request.
> + *
> + * Return: IRQ_HANDLED
> + */
> +static irqreturn_t cdsp_lpm_irq_handler(int irq, void *data)
> +{
> +	struct cdsp_power_driver *drv = data;
> +	unsigned int client_rsc_hdsk_irq_val;
> +	unsigned int rsc_hdsk_irq_val;
> +
> +	regmap_read(drv->mpm_regmap, RSC_HDSHK_IRQ_STAT, &rsc_hdsk_irq_val);
> +	regmap_read(drv->mpm_regmap, CLIENT_RSC_IRQ_STAT(0), &client_rsc_hdsk_irq_val);
> +	if (!client_rsc_hdsk_irq_val || !rsc_hdsk_irq_val)
> +		return IRQ_NONE;
> +
> +	/* Schedule work to process LPM request */
> +	queue_work(drv->lpm_wq, &drv->lpm_work);

Why have a threaded handler to read two registers and then schedule
work?

> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * cdsp_lpm_work_fn() - LPM work function
> + * @work: Work structure
> + *
> + * Processes LPM requests from CDSP by reading MPM handshake status
> + * and executing appropriate shutdown or restore sequence.
> + */
> +static void cdsp_lpm_work_fn(struct work_struct *work)
> +{
> +	struct cdsp_power_driver *drv = container_of(work,
> +						      struct cdsp_power_driver,
> +						      lpm_work);
> +	u32 hdshk_status;
> +	bool is_shutdown;
> +
> +	mutex_lock(&drv->lock);
> +
> +	/* Read MPM handshake status */
> +	regmap_read(drv->mpm_regmap, CLIENT_RSC_HDSHK(0), &hdshk_status);
> +
> +	/* Clear the interrupt */
> +	regmap_write(drv->mpm_regmap, CLIENT_RSC_IRQ_CLR(0), 0x0);
> +	regmap_write(drv->mpm_regmap, CLIENT_RSC_IRQ_CLR(0), 0x1);
> +	regmap_write(drv->mpm_regmap, CLIENT_RSC_IRQ_CLR(0), 0x0);
> +
> +	/* Determine if this is shutdown or bringup */
> +	is_shutdown = !!(hdshk_status & MPM_SHUTDOWN_REQ);
> +
> +	if (is_shutdown) {
> +		if (atomic_read(&drv->power_state) == CDSP_POWER_OFF) {
> +			dev_warn(drv->dev, "Spurious shutdown request, already powered off\n");
> +			goto out_unlock;
> +		}
> +		cdsp_lpm_shutdown_sequence(drv);
> +	} else {
> +		if (atomic_read(&drv->power_state) == CDSP_POWER_ON) {
> +			dev_warn(drv->dev, "Spurious bringup request, already powered on\n");
> +			goto out_unlock;
> +		}
> +		cdsp_lpm_restore_sequence(drv);
> +	}
> +
> +out_unlock:
> +	mutex_unlock(&drv->lock);
> +}
> +
> +static const struct regmap_config cdsp_rscc_regmap_config = {
> +	.reg_bits       = 32,
> +	.reg_stride     = 4,
> +	.val_bits       = 32,
> +	.max_register   = 0x4,
> +	.fast_io        = true,
> +};
> +
> +static const struct regmap_config cdsp_regmap_config = {
> +	.reg_bits       = 32,
> +	.reg_stride     = 4,
> +	.val_bits       = 32,
> +	.max_register   = 0x1000,
> +	.fast_io        = true,
> +};
> +
> +/**
> + * cdsp_power_probe() - Probe the CDSP power management driver
> + * @pdev: Platform device
> + *
> + * Acquires the PMIC regulator consumer handles, registers the virtual
> + * cdsp-vdd-cx (and optionally cdsp-vdd-mx) regulator providers, maps the
> + * MPM and RSCC register regions, and registers the DCVS and LPM interrupt
> + * handlers.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int cdsp_power_probe(struct platform_device *pdev)
> +{
> +	struct regulator_config virt_cfg = {};
> +	struct cdsp_power_driver *drv;
> +	struct regulator_dev *rdev;
> +	void __iomem *rscc_base;
> +	void __iomem *mpm_base;
> +	size_t smem_size;
> +	u32 smem_id;
> +	int ret;
> +
> +	/* Allocate driver context */
> +	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	drv->dev = &pdev->dev;
> +	mutex_init(&drv->lock);
> +	atomic_set(&drv->power_state, CDSP_POWER_ON);
> +
> +	/* Get SMEM item ID from device tree */

The next 38 lines exclusively deals with allocating and filling in the
SMEM item, sounds like a function to me.

> +	ret = of_property_read_u32(pdev->dev.of_node, "qcom,smem-item", &smem_id);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to get SMEM item ID\n");
> +
> +	/* Create SMEM entry for DCVS */
> +	ret = qcom_smem_alloc(CDSP_SMEM_NSP_HOST_ID, smem_id, CDSP_SMEM_SIZE);
> +	if (ret && ret != -EEXIST)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to allocate SMEM\n");
> +
> +	/* Get SMEM pointer and validate size */
> +	drv->smem = qcom_smem_get(CDSP_SMEM_NSP_HOST_ID, smem_id, &smem_size);
> +	if (IS_ERR(drv->smem))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->smem),
> +				     "Failed to get SMEM\n");
> +
> +	if (smem_size < CDSP_SMEM_SIZE)
> +		return dev_err_probe(&pdev->dev, -EINVAL,
> +				     "SMEM region too small: got %zu, expected %u\n",
> +				     smem_size, CDSP_SMEM_SIZE);
> +
> +	/*
> +	 * Initialise the SMEM channel header.
> +	 * Zero the entire region first so all padding and reserved fields
> +	 * are clean, then fill in the fixed protocol fields.
> +	 * apss_state is set to 1 last (after wmb) so NSP Q6 only sees a
> +	 * fully-populated header once APSS is ready.
> +	 */
> +	memset(drv->smem, 0, sizeof(*drv->smem));
> +	drv->smem->hdr.magic           = CDSP_SMEM_MAGIC;
> +	drv->smem->hdr.version         = CDSP_SMEM_VERSION;
> +	drv->smem->hdr.request_offset  = CDSP_SMEM_REQUEST_OFFSET;
> +	drv->smem->hdr.request_size    = CDSP_SMEM_REQUEST_SIZE;
> +	drv->smem->hdr.response_offset = CDSP_SMEM_RESPONSE_OFFSET;
> +	drv->smem->hdr.response_size   = CDSP_SMEM_RESPONSE_SIZE;
> +	/* Signal APSS readiness to NSP Q6 */
> +	WRITE_ONCE(drv->smem->hdr.apss_state, 1);
> +	/* Ensure SMEM header is fully written before NSP Q6 reads it */

wmb() ensures ordering, but below you have a bunch of resource requests.

Exactly which thing below this is it that you need to happen after
setting the apps_state?

> +	wmb();
> +
> +	/*
> +	 * Get voltage regulator consumer handles.
> +	 * These are the actual NSP_CX and NSP_MX voltage rails.
> +	 * The virtual regulator ops pass through to these handles.
> +	 */
> +	drv->vdd_cx = devm_regulator_get(&pdev->dev, "vdd-cx");

Why do this after setting apss_state = 1?

> +	if (IS_ERR(drv->vdd_cx))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->vdd_cx),
> +				     "Failed to get vdd-cx regulator\n");
> +
> +	drv->vdd_mx = devm_regulator_get_optional(&pdev->dev, "vdd-mx");
> +	if (IS_ERR(drv->vdd_mx)) {
> +		if (PTR_ERR(drv->vdd_mx) != -ENODEV)
> +			return dev_err_probe(&pdev->dev, PTR_ERR(drv->vdd_mx),
> +					     "Failed to get vdd-mx regulator\n");
> +		drv->vdd_mx = NULL;
> +		dev_dbg(&pdev->dev, "No vdd-mx regulator, MX rail absent on this board\n");
> +	}
> +
> +	/*
> +	 * Register virtual regulator provider.
> +	 *
> +	 * Expose vdd-cx and vdd-mx virtual regulators so that PAS remoteproc
> +	 * can consume them via cx-supply / mx-supply DTS properties.
> +	 * The enable/disable ops pass through to vdd_cx / vdd_mx above,
> +	 * making CDSP the sole hardware power manager for the NSP subsystem.
> +	 */
> +	virt_cfg.dev         = &pdev->dev;
> +	virt_cfg.driver_data = drv;
> +	virt_cfg.of_node     = pdev->dev.of_node;
> +
> +	INIT_WORK(&drv->dcvs_work, cdsp_dcvs_work_fn);
> +	INIT_WORK(&drv->lpm_work, cdsp_lpm_work_fn);
> +
> +	drv->lpm_wq = alloc_ordered_workqueue("cdsp_lpm_wq", 0);
> +	if (!drv->lpm_wq) {
> +		mbox_free_channel(drv->dcvs_mbox_chan);
> +		return dev_err_probe(&pdev->dev,
> +				     -ENOMEM,
> +				     "failed to allocate cdsp lpm workqueue\n");
> +	}
> +
> +	rdev = devm_regulator_register(&pdev->dev,
> +				       &cdsp_virt_reg_descs[CDSP_VIRT_NSP_CX],
> +				       &virt_cfg);
> +	if (IS_ERR(rdev))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
> +				     "Failed to register cdsp-vdd-cx virtual regulator\n");
> +
> +	if (drv->vdd_mx) {
> +		rdev = devm_regulator_register(&pdev->dev,
> +					       &cdsp_virt_reg_descs[CDSP_VIRT_NSP_MX],
> +					       &virt_cfg);
> +		if (IS_ERR(rdev))
> +			return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
> +					     "Failed to register cdsp-vdd-mx virtual regulator\n");
> +	}
> +
> +	/* Register DCVS interrupt */
> +	drv->dcvs_irq = platform_get_irq_byname(pdev, "dcvs");
> +	if (drv->dcvs_irq < 0)
> +		return dev_err_probe(&pdev->dev, drv->dcvs_irq,
> +				     "Failed to get DCVS IRQ\n");
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, drv->dcvs_irq,
> +					NULL, cdsp_dcvs_irq_handler,

cdsp_dcvs_irq_handler() will schedule work, which will access e.g. the
dcvs_mbox_chan which isn't requested until 43 lines further down.

Not good.

> +					IRQF_ONESHOT, "cdsp-dcvs", drv);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to request DCVS IRQ\n");
> +
> +	/* Setup MPM for LPM */
> +	mpm_base = devm_platform_ioremap_resource_byname(pdev, "mpm");
> +	if (IS_ERR(mpm_base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(mpm_base),
> +				     "Failed to map MPM registers\n");
> +
> +	drv->mpm_regmap = devm_regmap_init_mmio(&pdev->dev, mpm_base, &cdsp_regmap_config);
> +	if (IS_ERR(drv->mpm_regmap))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->mpm_regmap),
> +				     "Failed to init MPM regmap\n");
> +
> +	/* Setup RSCC for power mode detection */
> +	rscc_base = devm_platform_ioremap_resource_byname(pdev, "rscc");
> +	if (IS_ERR(rscc_base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rscc_base),
> +				     "Failed to map RSCC registers\n");
> +
> +	drv->rscc_regmap = devm_regmap_init_mmio(&pdev->dev, rscc_base, &cdsp_rscc_regmap_config);
> +	if (IS_ERR(drv->rscc_regmap))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->rscc_regmap),
> +				     "Failed to init RSCC regmap\n");
> +
> +	drv->lpm_irq = platform_get_irq_byname(pdev, "lpm");
> +	if (drv->lpm_irq < 0)
> +		return dev_err_probe(&pdev->dev, drv->lpm_irq,
> +				     "Failed to get LPM IRQ\n");
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, drv->lpm_irq,
> +					NULL, cdsp_lpm_irq_handler,
> +					IRQF_ONESHOT, "cdsp-lpm", drv);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to request LPM IRQ\n");
> +
> +	/* Setup mbox for DCVS response */
> +	drv->dcvs_mbox_client.dev = &pdev->dev;
> +	drv->dcvs_mbox_client.knows_txdone = true;
> +	drv->dcvs_mbox_chan = mbox_request_channel(&drv->dcvs_mbox_client, 0);
> +	if (IS_ERR(drv->dcvs_mbox_chan))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->dcvs_mbox_chan),
> +				     "Failed to get dcvs mbox channel\n");
> +
> +	platform_set_drvdata(pdev, drv);
> +
> +	dev_dbg(&pdev->dev, "CDSP power driver initialized\n");
> +
> +	return 0;
> +}
> +
> +static void cdsp_power_remove(struct platform_device *pdev)
> +{
> +	struct cdsp_power_driver *drv = platform_get_drvdata(pdev);
> +
> +	/* Cancel any pending work */
> +	cancel_work_sync(&drv->dcvs_work);
> +	cancel_work_sync(&drv->lpm_work);

At this point no work is pending in the two queues, but if the interrupt
fires you will queue more work.

You need to ensure your interrupts are stopped before this.

> +
> +	if (drv->lpm_wq)

How can this be NULL?

Regards,
Bjorn

> +		destroy_workqueue(drv->lpm_wq);
> +
> +	mbox_free_channel(drv->dcvs_mbox_chan);
> +}
> +
> +static const struct of_device_id cdsp_power_of_match[] = {
> +	{ .compatible = "qcom,cdsp-power" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, cdsp_power_of_match);
> +
> +static struct platform_driver cdsp_power_driver = {
> +	.probe = cdsp_power_probe,
> +	.remove = cdsp_power_remove,
> +	.driver = {
> +		.name = "cdsp-power",
> +		.of_match_table = cdsp_power_of_match,
> +	},
> +};
> +
> +module_platform_driver(cdsp_power_driver);
> +
> +MODULE_DESCRIPTION("CDSP Power Management Driver");
> +MODULE_LICENSE("GPL");
> 
> -- 
> 2.43.0
> 

^ permalink raw reply

* Re: [PATCH 0/3] Add support for qcrypto on shikra
From: Eric Biggers @ 2026-05-22  2:49 UTC (permalink / raw)
  To: Kuldeep Singh
  Cc: Thara Gopinath, Herbert Xu, David S. Miller, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Vinod Koul, Frank Li, Andy Gross, linux-arm-msm, linux-crypto,
	devicetree, linux-kernel, dmaengine
In-Reply-To: <d4d35e17-84fa-4c95-9bfb-abfd25ea7f4a@oss.qualcomm.com>

On Thu, May 21, 2026 at 12:21:41PM +0530, Kuldeep Singh wrote:
> On 15-05-2026 01:17, Eric Biggers wrote:
> > On Fri, May 15, 2026 at 12:53:35AM +0530, Kuldeep Singh wrote:
> >> Add qcrypto and cryptobam DT nodes for enabling qcrypto on kaanapali.
> >> Shikra bam dma supports 7 iommus so update dt-bindings accordingly.
> >>
> >> The patchset depends on below. There's recursive dependency so referred
> >> to base DT patch here.
> >> - https://lore.kernel.org/all/20260512-shikra-dt-v1-0-716438330dd0@oss.qualcomm.com/
> >>
> >> Validations:
> >> - make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml dt_binding_check
> >> - make ARCH=arm64 qcom/shikra-cqs-evk.dtb CHECK_DTBS=1 DT_SCHEMA_FILES=Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> >> - cryptobam and crypto driver probe
> >> - kcapi test
> >>
> >> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
> > 
> > What specific kernel features would this be useful for, and what
> > specific performance improvements are you seeing with those features?
> 
> I hope you mean 7 iommu entries.
> 
> Please note, shikra is an old platform and differs with latest platforms
> like kaanapali in terms of iommus#.
> Kaanapali is optimised(in terms of iommus#) as same pipe index/sid i.e
> 4/5 can be used for general purpose or for any other usecase like
> DRM/HDCP etc.
> Whereas for shikra, there's dedicated iommu entry for each usecase and
> same pipe index/sid cannot be used for other usecases.
> 
> The performance will be be effectively similar.

It sounds like you don't actually have an answer to my questions, then.

Performance tests (e.g.
https://lore.kernel.org/r/20250615031807.GA81869@sol/) have clearly
shown that this driver is an order of magnitude slower than the CPU.

This driver has historically been quite harmful.  People were using it
accidentally and encountering very bad performance, as well as bugs such
as crashes and filesystem hangs.  We fixed that by lowering its
cra_priority.  But for the same reason, even when enabled on a platform,
it's not actually used.  Linux would be better without this driver.

We seem to be seeing the usual drivers/crypto/ pattern here: this crypto
offload driver is being pushed by the hardware manufacturer, with no
awareness of the fact that it's actually useless in Linux.

I've had enough of this.  Please consider this series:

    Nacked-by: Eric Biggers <ebiggers@kernel.org>

FWIW: the approaches that are actually used and work well in Linux are
ICE and the CPU-accelerated crypto.

- Eric

^ permalink raw reply

* RE: [PATCH v5 3/3] PCI: imx6: Add root port reset to support link recovery
From: Hongxing Zhu @ 2026-05-22  2:41 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: devicetree@vger.kernel.org, robh@kernel.org, Frank.Li@kernel.org,
	linux-pci@vger.kernel.org, conor+dt@kernel.org,
	imx@lists.linux.dev
In-Reply-To: <20260521112122.F334C1F000E9@smtp.kernel.org>

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Thursday, May 21, 2026 7:21 PM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: devicetree@vger.kernel.org; robh@kernel.org; Frank.Li@kernel.org; linux-
> pci@vger.kernel.org; conor+dt@kernel.org; imx@lists.linux.dev
> Subject: Re: [PATCH v5 3/3] PCI: imx6: Add root port reset to support link recovery
> 
> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to
> consider:
> - [High] The `lnk_intr` member is defined as `u32`, which wraps negative error
> codes from `platform_get_irq_byname` to large positive integers, breaking driver
> probe.
> - [High] Read-modify-write (RMW) operations via `regmap_clear_bits` and
> `regmap_set_bits` unintentionally clear Write-1-to-Clear (W1C) status bits.
> - [High] Iterating over the PCI bus device list without holding the `pci_bus_sem`
> read lock causes a race condition.
> - [High] The patch references `reset_root_port` and calls
> `pci_host_handle_link_down()`, neither of which exist in the mainline kernel tree.
> - [High] Hardware LUT state is accessed in `imx_pcie_reset_root_port` without
> acquiring the protective `imx_pcie->lock` mutex.
> - [Medium] The shared interrupt handler claims the interrupt without verifying
> that its specific interrupt source is actually enabled, leading to spurious wakeups.
> - [Low] Using `platform_get_irq_byname` instead of
> `platform_get_irq_byname_optional` for an optional interrupt will cause
> unnecessary kernel error logs.
> --
> 
> commit c3df91626a3cc2df53cc4e1da0e0b1aeb4d5d8cd
> Author: Richard Zhu <hongxing.zhu@nxp.com>
> 
> PCI: imx6: Add root port reset to support link recovery
> 
> 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.
> 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> 
> [ ... ]
> 
> > @@ -158,6 +165,7 @@ struct imx_pcie {
> >  	bool			supports_clkreq;
> >  	bool			enable_ext_refclk;
> >  	struct regmap		*iomuxc_gpr;
> > +	u32			lnk_intr;
> 
> [Severity: High]
> Will platform_get_irq_byname returning a negative error code (like -ENXIO) wrap
> to a large positive integer here, causing the later if (imx_pcie->lnk_intr > 0) check
> to pass and break driver probe on systems without the optional interrupt? Should
> this be an int instead?
> 
Good catch! will update it later.
> [ ... ]
> 
> > @@ -1306,6 +1314,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;
> 
> [Severity: High]
> Does struct pci_host_bridge have a reset_root_port member? It appears this
> patch relies on out-of-tree or unmerged prerequisites and will cause build
> failures.
No worries, the dependency had been specified.
> 
> [ ... ]
> 
> > @@ -1573,6 +1588,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)
> > +		regmap_clear_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > +				  IMX95_LINK_DOWN_INT_EN);
> 
> [Severity: High]
> Does this read-modify-write operation accidentally clear Write-1-to-Clear status
> bits?
> 
> IMX95_LINK_INT_CTRL_STS contains W1C bits like
> IMX95_LINK_DOWN_INT_STS.
> When regmap_clear_bits reads the register, any pending W1C bits will be read as
> 1. Writing them back during the modify phase will inadvertently clear the status
> bits without handling the interrupt.
> 
Okay, would clear the W1C bit firstly, then do the regmap_clear_bits().
> [ ... ]
> 
> > @@ -1632,6 +1653,85 @@ 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) {
> 
> [Severity: Medium]
> Should this check verify that the interrupt is actually enabled?
> 
> Since this is an IRQF_SHARED handler sharing a line with the PME driver, if the
> hardware status bit is asserted but IMX95_LINK_DOWN_INT_EN is masked, a
> subsequent interrupt triggered by the PME driver could cause this ISR to
> incorrectly claim the interrupt and return IRQ_WAKE_THREAD.
IRQ_WAKE_THREAD only be returned when IMX95_LINK_DOWN_INT_STS is asserted.
> 
> [ ... ]
> 
> > +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)
> 
> [Severity: High]
> Is it safe to iterate over the PCI bus device list without holding the pci_bus_sem
> read lock? Traversing the bus->devices list asynchronously risks list corruption and
> use-after-free vulnerabilities if PCI devices are concurrently hot-added or
> removed. Should pci_walk_bus() or
> down_read(&pci_bus_sem) be used here?
> 
> > +		if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
> > +			pci_host_handle_link_down(port);
> 
> [Severity: High]
> Is pci_host_handle_link_down() available in the mainline kernel tree?
> This function appears to be undefined, which will cause a build failure.
> 
No worries, the dependency had been specified.
> [ ... ]
> 
> > +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);
> 
> [Severity: High]
> Does this access the hardware LUT state without acquiring the protective
> imx_pcie->lock mutex?
> 
> Since imx_pcie_reset_root_port can be invoked asynchronously from the link
> down interrupt handler, concurrent accesses to the LUT configuration from other
> execution paths (which do use the lock) could lead to a race condition and corrupt
> the hardware LUT configuration.
No worries, there are no race when do the lut save/restore.
> 
> [ ... ]
> 
> > @@ -1834,9 +1934,33 @@ 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(pdev, "intr");
> 
> [Severity: Low]
> Since this interrupt is optional, should this use
> platform_get_irq_byname_optional() to avoid printing unnecessary error
> messages to the kernel log when it is absent?
Okay, would update it later.

Best Regards
Richard Zhu

> 
> --
> Sashiko AI
> review * https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsa
> shiko.dev%2F%23%2Fpatchset%2F20260521093255.61585-1-
> hongxing.zhu%40nxp.com%3Fpart%3D3&data=05%7C02%7Chongxing.zhu%40nx
> p.com%7Ce831884f29d14575cb8408deb72b1830%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C639149592897765731%7CUnknown%7CTWFpbGZsb
> 3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIj
> oiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Ui0aRHNXO9FFLrgBoH
> ClFZFn6eYvAj1gwXiWD45uNlc%3D&reserved=0

^ permalink raw reply

* Re: [PATCH v11 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
From: sashiko-bot @ 2026-05-22  2:32 UTC (permalink / raw)
  To: syyang; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260522015735.2833-3-syyang@lontium.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing drm_panel_bridge_remove() in lt7911exc_dsi_host_detach() causes bridge list corruption and Use-After-Free.
- [Medium] The fast `ocm_lock` is held during a 420ms hardware reset sequence, causing severe latency stalls for concurrent DRM and DSI operations.
--

commit 7a7cfdfb50c450205bc811aedab79e9db1683754
Author: Sunyun Yang <syyang@lontium.com>

drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge

Add support for the Lontium LT7911EXC bridge chip, which converts
eDP input to MIPI DSI output using an internal firmware-controlled
pipeline.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt7911exc.c b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
> new file mode 100644
> index 0000000000000..1aec60f02f913
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
[ ... ]
> +static void lt7911exc_firmware_upgrade_work(struct work_struct *work)
> +{
> +	struct lt7911exc *lt7911exc = container_of(work, struct lt7911exc, work);
> +	struct device *dev = lt7911exc->dev;
[ ... ]
> +	mutex_lock(&lt7911exc->upgrade_lock);
> +
> +	mutex_lock(&lt7911exc->ocm_lock);
> +	lt7911exc_reset(lt7911exc);

[Severity: Medium]
Since lt7911exc_reset() sleeps for 420ms (due to msleep calls), does holding
the fast ocm_lock here unnecessarily stall concurrent DRM and DSI operations?

It looks like the upgrade_lock already serializes firmware upgrades. Can the
ocm_lock be acquired after the reset sequence?

> +	lt7911exc_hw_mcu_halt(lt7911exc);
> +	mutex_unlock(&lt7911exc->ocm_lock);
> +
> +	ret = lt7911exc_block_erase(lt7911exc);
[ ... ]
> +	ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write firmware crc\n");
> +		goto out_unlock;
> +	}
> +
> +	mutex_lock(&lt7911exc->ocm_lock);
> +	lt7911exc_reset(lt7911exc);

[Severity: Medium]
Similarly, does holding the ocm_lock during this second reset sequence cause
severe latency stalls for other display operations?

> +	mutex_unlock(&lt7911exc->ocm_lock);
> +
> +	ret = lt7911exc_upgrade_result(lt7911exc, crc32);
[ ... ]
> +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
> +{
> +	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
> +
> +	if (!lt7911exc->output.dev)
> +		return 0;
> +
> +	lt7911exc->output.dev = NULL;

[Severity: High]
Should drm_panel_bridge_remove() be called here?

When the panel is unbound, its device is destroyed and the devres cleanup
frees the bridge memory. If the bridge isn't properly removed here, the freed
bridge remains linked in the global DRM bridge_list, potentially causing a
use-after-free on subsequent list traversals.

> +
> +	return 0;
> +}
[ ... ]
> +static void lt7911exc_remove(struct i2c_client *client)
> +{
> +	struct lt7911exc *lt7911exc = i2c_get_clientdata(client);
[ ... ]
> +	mipi_dsi_host_unregister(&lt7911exc->dsi_host);
> +	drm_bridge_remove(&lt7911exc->bridge);
> +
> +	if (lt7911exc->output.panel && lt7911exc->output.bridge)
> +		drm_panel_bridge_remove(lt7911exc->output.bridge);

[Severity: High]
Can this dereference a freed pointer?

Since mipi_dsi_host_unregister() triggers panel detachment, and
lt7911exc_dsi_host_detach() doesn't remove the panel bridge, the bridge memory
could be freed during the detachment. Calling drm_panel_bridge_remove() here
would then lead to a use-after-free.

> +
> +	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522015735.2833-1-syyang@lontium.com?part=2

^ permalink raw reply

* Re: [PATCH v2 2/2] ASoC: codecs: nau8360: Add support for NAU83G60 amplifier
From: YLCHANG2 @ 2026-05-22  2:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: Neo Chang, lgirdwood, perex, robh, krzk+dt, linux-sound,
	devicetree, alsa-devel, kchsu0, sjlin0
In-Reply-To: <934509a9-1347-4b02-a341-0db0d1db42e7@sirena.org.uk>


On 5/21/26 23:35, Mark Brown wrote:
> On Wed, May 20, 2026 at 11:01:53AM +0800, YLCHANG2 wrote:
>> On 5/18/26 19:35, Mark Brown wrote:
>>>> +static bool nau8360_volatile_reg(struct device *dev, unsigned int reg)
>>>> +{
>>>> +	switch (reg) {
>>>> +	case NAU8360_R00_SOFTWARE_RST ... NAU8360_R02_I2C_ADDR:
>>>> +	case NAU8360_R06_INT_CLR_STATUS:
>>>> +	case NAU8360_R21_VBAT_READOUT ... NAU8360_R22_TEMP_READOUT:
>>>> +	case NAU8360_R41_CLK_CTL2:
>>>> +	case NAU8360_R46_I2C_DEVICE_ID:
>>> Are the I2C addresss and device ID volatile or do they just not have
>>> defaults?
>> For NAU8360_R02_I2C_ADDR, it requires a write operation to latch the I2C
>> device address.
>> For NAU8360_R46_I2C_DEVICE_ID, although it is read-only, its value is
>> determined by the hardware configuration
>> of the GPIO1 and GPIO2 pins at boot (e.g., 0x1a, 0x1b, 0x4a, or 0x4b).
> In that case they shouldn't be marked as volatile, just omitted from the
> defaults - they can be cached on first use.
Thanks for the suggestion. I will remove it from the volatile register list.

^ permalink raw reply

* RE: [PATCH v1 2/2] iio: adc: Add StarFive SAR-ADC driver
From: Xingyu Wu @ 2026-05-22  2:20 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org
In-Reply-To: <20260521123727.6a7d97ff@jic23-huawei>

On 2026/5/21 19:37, Jonathan Cameron wrote:
> 
> >
> > >
> > > > +
> > > > +	starfive_saradc_ch_monitor_stop(priv, ch);
> > >
> > > Why stop it?  Add a comment.  We aren't interested in future events?
> >
> > If we do not stop it and the input voltage remains constant, the ADC could
> continuously monitor the voltage and continuously trigger interrupts for the
> bound.
> > As a result, the CPU utilization rate will be high.
> 
> Can you use an edge interrupt instead of a level one?

No, it is a level interrupt in hardware.

> 
> If not, then a common solution is to disable for a period (maybe a second or so)
> then reenable. There are various more refined ways of doing this.

This is good idea. How about using timer to disable it?

> 
> Basically we don't want to be in a situation where a momentary blip disables the
> event and we miss a later condition that must be handled.
> 

Noted.

Best regards,
Xingyu Wu

^ permalink raw reply

* RE: [PATCH v4 4/6] media: synopsys: Add PHY stopstate wait for i.MX93
From: G.N. Zhou (OSS) @ 2026-05-22  1:58 UTC (permalink / raw)
  To: Alexander Stein, Michael Riesch, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Laurent Pinchart, Frank Li, Sakari Ailus, Bryan O'Donoghue,
	Mehdi Djait, Hans Verkuil, G.N. Zhou (OSS), G.N. Zhou (OSS)
  Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, G.N. Zhou (OSS)
In-Reply-To: <3606153.44csPzL39Z@steina-w>

Hi Alexander,

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Thursday, May 21, 2026 7:53 PM
> To: Michael Riesch <michael.riesch@collabora.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Heiko Stuebner
> <heiko@sntech.de>; Laurent Pinchart <laurent.pinchart@ideasonboard.com>;
> Frank Li <frank.li@nxp.com>; Sakari Ailus <sakari.ailus@linux.intel.com>; Bryan
> O'Donoghue <bryan.odonoghue@linaro.org>; Mehdi Djait
> <mehdi.djait@linux.intel.com>; Hans Verkuil <hverkuil+cisco@kernel.org>;
> G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>; G.N. Zhou (OSS)
> <guoniu.zhou@oss.nxp.com>
> Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-rockchip@lists.infradead.org; G.N. Zhou (OSS)
> <guoniu.zhou@oss.nxp.com>
> Subject: Re: [PATCH v4 4/6] media: synopsys: Add PHY stopstate wait for
> i.MX93
> 
> Hi,
> 
> thanks for the replay.
> 
> Am Donnerstag, 21. Mai 2026, 11:29:39 CEST schrieb G.N. Zhou (OSS):
> > Hi Alexander,
> >
> > > -----Original Message-----
> > > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > Sent: Wednesday, May 20, 2026 7:12 PM
> > > To: Michael Riesch <michael.riesch@collabora.com>; Mauro Carvalho
> > > Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>;
> > > Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> > > <conor+dt@kernel.org>; Heiko Stuebner <heiko@sntech.de>; Laurent
> > > Pinchart <laurent.pinchart@ideasonboard.com>;
> > > Frank Li <frank.li@nxp.com>; Sakari Ailus
> > > <sakari.ailus@linux.intel.com>; Bryan O'Donoghue
> > > <bryan.odonoghue@linaro.org>; Mehdi Djait
> > > <mehdi.djait@linux.intel.com>; Hans Verkuil
> > > <hverkuil+cisco@kernel.org>; G.N. Zhou (OSS)
> > > <guoniu.zhou@oss.nxp.com>
> > > Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> > > kernel@lists.infradead.org; linux-rockchip@lists.infradead.org; G.N.
> > > Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> > > Subject: Re: [PATCH v4 4/6] media: synopsys: Add PHY stopstate wait
> > > for
> > > i.MX93
> > >
> > > Hi,
> > >
> > > Am Dienstag, 19. Mai 2026, 04:07:41 CEST schrieb Guoniu Zhou:
> > > > Implement waiting for D-PHY lanes to enter stop state on i.MX93.
> > > > This ensures proper PHY initialization by verifying that the clock
> > > > lane and all active data lanes have entered the stop state before
> > > > proceeding with further operations.
> > > >
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> > > > ---
> > > > Changes in v2:
> > > > - Removes redundant register availability check
> > > > - Uses read_poll_timeout() with dw_mipi_csi2rx_read() instead of
> > > >   readl_poll_timeout() with direct register address
> > > > - Fixes stopstate condition logic
> > > > - Check PHY stopstate after sensor enable instead of before to ensure
> > > >   correct timing.
> > > > - Optimize PHY stopstate polling parameters (1000us->10us, 2s->1ms) to
> > > >   balance performance and responsiveness.
> > > > ---
> > > >  drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 36
> > > > ++++++++++++++++++++++++
> > > >  1 file changed, 36 insertions(+)
> > > >
> > > > diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> > > > b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> > > > index 92178a3dec5d..8a34aec550ad 100644
> > > > --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> > > > +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> > > > @@ -11,6 +11,7 @@
> > > >  #include <linux/clk.h>
> > > >  #include <linux/delay.h>
> > > >  #include <linux/io.h>
> > > > +#include <linux/iopoll.h>
> > > >  #include <linux/module.h>
> > > >  #include <linux/of.h>
> > > >  #include <linux/phy/phy.h>
> > > > @@ -35,6 +36,8 @@
> > > >  #define DW_REG_EXIST		BIT(31)
> > > >  #define DW_REG(x)		(DW_REG_EXIST | (x))
> > > >
> > > > +#define DPHY_STOPSTATE_CLK_LANE		BIT(16)
> > > > +
> > > >  #define DPHY_TEST_CTRL0_TEST_CLR	BIT(0)
> > > >
> > > >  #define IPI_VCID_VC(x)			FIELD_PREP(GENMASK(1, 0),
> > > (x))
> > > > @@ -65,6 +68,7 @@ enum dw_mipi_csi2rx_regs_index {
> > > >  	DW_MIPI_CSI2RX_PHY_TST_CTRL0,
> > > >  	DW_MIPI_CSI2RX_PHY_TST_CTRL1,
> > > >  	DW_MIPI_CSI2RX_PHY_SHUTDOWNZ,
> > > > +	DW_MIPI_CSI2RX_PHY_STOPSTATE,
> > > >  	DW_MIPI_CSI2RX_IPI_DATATYPE,
> > > >  	DW_MIPI_CSI2RX_IPI_MEM_FLUSH,
> > > >  	DW_MIPI_CSI2RX_IPI_MODE,
> > > > @@ -87,6 +91,7 @@ struct dw_mipi_csi2rx_drvdata {
> > > >  	void (*dphy_assert_reset)(struct dw_mipi_csi2rx_device *csi2);
> > > >  	void (*dphy_deassert_reset)(struct dw_mipi_csi2rx_device *csi2);
> > > >  	void (*ipi_enable)(struct dw_mipi_csi2rx_device *csi2);
> > > > +	int (*wait_for_phy_stopstate)(struct dw_mipi_csi2rx_device
> > > > +*csi2);
> > > >  };
> > > >
> > > >  struct dw_mipi_csi2rx_format {
> > > > @@ -139,6 +144,7 @@ static const u32
> > > > imx93_regs[DW_MIPI_CSI2RX_MAX]
> > > = {
> > > >  	[DW_MIPI_CSI2RX_PHY_SHUTDOWNZ] = DW_REG(0x40),
> > > >  	[DW_MIPI_CSI2RX_DPHY_RSTZ] = DW_REG(0x44),
> > > >  	[DW_MIPI_CSI2RX_PHY_STATE] = DW_REG(0x48),
> > > > +	[DW_MIPI_CSI2RX_PHY_STOPSTATE] = DW_REG(0x4c),
> > > >  	[DW_MIPI_CSI2RX_PHY_TST_CTRL0] = DW_REG(0x50),
> > > >  	[DW_MIPI_CSI2RX_PHY_TST_CTRL1] = DW_REG(0x54),
> > > >  	[DW_MIPI_CSI2RX_IPI_MODE] = DW_REG(0x80), @@ -556,10 +562,19
> > > @@
> > > > static int dw_mipi_csi2rx_enable_streams(struct v4l2_subdev *sd,
> > > >  	if (ret)
> > > >  		goto err_csi_stop;
> > > >
> > > > +	if (!csi2->enabled_streams &&
> > > > +	    csi2->drvdata->wait_for_phy_stopstate) {
> > > > +		ret = csi2->drvdata->wait_for_phy_stopstate(csi2);
> > > > +		if (ret)
> > > > +			goto err_disable_streams;
> > > > +	}
> > > > +
> > > >  	csi2->enabled_streams |= streams_mask;
> > > >
> > > >  	return 0;
> > > >
> > > > +err_disable_streams:
> > > > +	v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
> > > >  err_csi_stop:
> > > >  	/* Stop CSI hardware if no streams are enabled */
> > > >  	if (!csi2->enabled_streams)
> > > > @@ -871,11 +886,32 @@ static void
> > > > imx93_csi2rx_dphy_ipi_enable(struct
> > > dw_mipi_csi2rx_device *csi2)
> > > >  	dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_IPI_MODE, val);  }
> > > >
> > > > +static int imx93_csi2rx_wait_for_phy_stopstate(struct
> > > > +dw_mipi_csi2rx_device *csi2) {
> > > > +	struct device *dev = csi2->dev;
> > > > +	u32 stopstate_mask;
> > > > +	u32 val;
> > > > +	int ret;
> > > > +
> > > > +	stopstate_mask = DPHY_STOPSTATE_CLK_LANE | GENMASK(csi2-
> > > >lanes_num -
> > > > +1, 0);
> > > > +
> > > > +	ret = read_poll_timeout(dw_mipi_csi2rx_read, val,
> > > > +				(val & stopstate_mask) == stopstate_mask,
> > > > +				 10, 1000, true,
> > > > +				 csi2, DW_MIPI_CSI2RX_PHY_STOPSTATE);
> > > > +	if (ret)
> > > > +		dev_err(dev, "lanes are not in stop state: %#x,
> > > expected %#x\n",
> > > > +			val, stopstate_mask);
> > >
> > > Did you actually test this on imx93? I'm trying to get my imx327
> > > sensor to run, but only run into this error message:
> > > dw-mipi-csi2rx 4ae00000.mipi-csi: lanes are not in stop state: 0x0,
> > > expected
> > > 0x10003
> >
> > Thanks for testing. Regarding the lane stop state error on i.MX93 with
> imx327:
> >
> > This error indicates the CSI-2 lanes are not in LP-11 (stop) state
> > when expected. Please check:
> >
> > 1) Verify the sensor PHY is in LP-11 state before returning from the sensor's
> >    s_stream(1) call. The CSI-2 receiver expects lanes to be in stop state
> >    initially.
> 
> Well, this might be tricky as I don't have D-PHY capable scopes.
> I can successfully use this sensor on a imx8mp, so I am expecting this to be
> okay.
> 
> > 2) Check if the imx327 driver has a delay between starting the stream and
> >    returning from s_stream(). If the sensor transitions PHY out of LP-11
> >    state during this delay, the CSI driver's lane state check will fail
> >    when it runs later. The sensor should remain in LP-11 until the CSI
> >    controller completes its initialization.
> 
> In imx290_set_stream() and subsequently imx290_start_streaming() setting
> IMX290_XMSTA starts the stream. I expect this is the point when the sensors
> switches from LP-11 to HS. But again, I can't verify.

As mentioned in #2, I reviewed drivers/media/i2c/imx290.c and identified a 30ms
delay that appears to be the root cause of this issue.

The problem occurs because:

The sensor exits LP-11 state and transitions to HS mode after IMX290_XMSTA is written
The 30ms delay in s_stream() causes the function to return late
By the time the CSI controller performs its lane state check, the sensor has already switched
from LP-11 to HS mode, causing the check to fail。

Proposed fix:

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 21cbc81cb2ed..519aa336249a 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -1059,7 +1059,6 @@ static int imx290_start_streaming(struct imx290 *imx290,
 
        cci_write(imx290->regmap, IMX290_STANDBY, 0x00, &ret);
 
-       msleep(30);
 
        /* Start streaming */
        return cci_write(imx290->regmap, IMX290_XMSTA, 0x00, &ret);

Removing this delay allows s_stream() to return immediately, giving the CSI controller
time to complete its initialization before the sensor transitions PHY state.

Could you test this patch and confirm if it resolves the lane state check failure?

Best Regards
G.N Zhou

> 
> With enabling debug
> > echo "module videodev +p" > /sys/kernel/debug/dynamic_debug/control
> > echo 0xff > /sys/class/video4linux/video0/dev_debug
> 
> After I setup the media pipeline, running the command
> > v4l2-ctl -z "platform:4ae40000.isi" -d "mxc_isi.0.capture"
> > --stream-mmap --stream-count=1 --stream-to=imx93.raw
> I get the following debug output:
> 
> --8<--
> plane 0: bytesperline=3840 sizeimage=4147200 mxc-isi 4ae40000.isi:
> validating link "crossbar":2 -> "mxc_isi.0":0 mxc-isi 4ae40000.isi: validating
> stream "crossbar":2:0 -> "mxc_isi.0":0:0 mxc-isi 4ae40000.isi: validating link
> "dw-mipi-csi2rx 4ae00000.mipi-csi":1 -> "crossbar":0 mxc-isi 4ae40000.isi:
> validating stream "dw-mipi-csi2rx 4ae00000.mipi-csi":1:0 -> "crossbar":0:0
> mxc-isi 4ae40000.isi: validating link "imx327 4-001a":0 -> "dw-mipi-csi2rx
> 4ae00000.mipi-csi":0 mxc-isi 4ae40000.isi: validating stream "imx327 4-
> 001a":0:0 -> "dw-mipi-csi2rx 4ae00000.mipi-csi":0:0 mxc-isi 4ae40000.isi:
> enable streams "crossbar":2/0x1 mxc-isi 4ae40000.isi: collect_streams:
> "crossbar":2: found 0x1 enabled 0x0
> imx290 4-001a: Frame descriptor on pad 0, type CSI-2
> imx290 4-001a:   stream 0, code 0x300f, length 0, flags 0x0000, vc 0, dt 0x2b
> dw-mipi-csi2rx 4ae00000.mipi-csi: Frame descriptor on pad 1, type CSI-2
> dw-mipi-csi2rx 4ae00000.mipi-csi:        stream 0, code 0x300f, length 0, flags
> 0x0000, vc 0, dt 0x2b
> mxc-isi 4ae40000.isi: enable streams "dw-mipi-csi2rx 4ae00000.mipi-
> csi":1/0x1 dw-mipi-csi2rx 4ae00000.mipi-csi: collect_streams: "dw-mipi-csi2rx
> 4ae00000.mipi-csi":1: found 0x1 enabled 0x0 mxc-isi 4ae40000.isi: enable
> streams "imx327 4-001a":0/0x1
> imx290 4-001a: collect_streams: sub-device "imx327 4-001a" does not support
> streams dw-mipi-csi2rx 4ae00000.mipi-csi: lanes are not in stop state: 0x0,
> expected 0x10003 mxc-isi 4ae40000.isi: disable streams "imx327 4-
> 001a":0/0x1
> imx290 4-001a: collect_streams: sub-device "imx327 4-001a" does not support
> streams mxc-isi 4ae40000.isi: enable streams 1:0x1 failed: -110 mxc-isi
> 4ae40000.isi: failed to enable streams 0x1 on 'dw-mipi-csi2rx 4ae00000.mipi-
> csi':1: -110 mxc-isi 4ae40000.isi: enable streams 2:0x1 failed: -110 mxc-isi
> 4ae40000.isi: Failed to enable pipe 0
> video0: VIDIOC_STREAMON: error -110: type=vid-cap-mplane
> videodev: v4l2_release: video0: release
> --8<--
> 
> For completeness this is my media device config
> --8<--
> # media-ctl  -p
> Media controller API version 7.1.0
> 
> Media device information
> ------------------------
> driver          mxc-isi
> model           FSL Capture Media Device
> serial
> bus info        platform:4ae40000.isi
> hw revision     0x0
> driver version  7.1.0
> 
> Device topology
> - entity 1: crossbar (3 pads, 2 links, 1 route)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev0
>         routes:
>                 0/0 -> 2/0 [ACTIVE]
>         pad0: SINK,MUST_CONNECT
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw]
>                 <- "dw-mipi-csi2rx 4ae00000.mipi-cs":1 [ENABLED,IMMUTABLE]
>         pad1: SINK,MUST_CONNECT
>         pad2: SOURCE
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw]
>                 -> "mxc_isi.0":0 [ENABLED,IMMUTABLE]
> 
> - entity 5: mxc_isi.0 (2 pads, 2 links, 0 routes)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev1
>         pad0: SINK
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw
>                  compose.bounds:(0,0)/1920x1080
>                  compose:(0,0)/1920x1080]
>                 <- "crossbar":2 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:jpeg
> xfer:srgb ycbcr:601 quantization:full-range
>                  crop.bounds:(0,0)/1920x1080
>                  crop:(0,0)/1920x1080]
>                 -> "mxc_isi.0.capture":0 [ENABLED,IMMUTABLE]
> 
> - entity 8: mxc_isi.0.capture (1 pad, 1 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video0
>         pad0: SINK
>                 <- "mxc_isi.0":1 [ENABLED,IMMUTABLE]
> 
> - entity 16: dw-mipi-csi2rx 4ae00000.mipi-cs (2 pads, 2 links, 1 route)
>              type V4L2 subdev subtype Unknown flags 0
>              device node name /dev/v4l-subdev2
>         routes:
>                 0/0 -> 1/0 [ACTIVE]
>         pad0: SINK,MUST_CONNECT
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw
> xfer:none ycbcr:601 quantization:full-range]
>                 <- "imx327 4-001a":0 [ENABLED]
>         pad1: SOURCE
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw
> xfer:none ycbcr:601 quantization:full-range]
>                 -> "crossbar":0 [ENABLED,IMMUTABLE]
> 
> - entity 21: imx327 4-001a (1 pad, 1 link, 0 routes)
>              type V4L2 subdev subtype Sensor flags 0
>              device node name /dev/v4l-subdev3
>         pad0: SOURCE
>                 [stream:0 fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw
> xfer:none ycbcr:601 quantization:full-range
>                  crop.bounds:(0,0)/1945x1097
>                  crop:(12,8)/1920x1080]
>                 -> "dw-mipi-csi2rx 4ae00000.mipi-cs":0 [ENABLED]
> --8<--
> 
> Anything odd here?
> 
> > You may need to remove any delays in the imx327 s_stream
> > implementation, or ensure the sensor stays in LP-11 state until the CSI
> receiver is ready.
> >
> > If possible, could you share the imx327 driver code or check its s_stream
> implementation?
> 
> It's essentially upstream in drivers/media/i2c/imx290.c.
> I only have a dummy implementation for get_frame_desc and a small
> adjustement for my camera module regarding i2c access.
> 
> --8<--
> static int imx290_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> 				 struct v4l2_mbus_frame_desc *fd)
> {
> 	const struct v4l2_mbus_framefmt *format;
> 	struct v4l2_subdev_state *state;
> 
> 	state = v4l2_subdev_lock_and_get_active_state(sd);
> 	format = v4l2_subdev_state_get_format(state, pad);
> 	v4l2_subdev_unlock_state(state);
> 
> 	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> 	fd->num_entries = 1;
> 	fd->entry[0].pixelcode = format->code;
> 	fd->entry[0].stream = 0;
> 	fd->entry[0].bus.csi2.vc = 0;
> 	fd->entry[0].bus.csi2.dt = MIPI_CSI2_DT_RAW10; //TODO:
> get_data_type_by_code(format->code);
> 
> 	return 0;
> }
> --8<--
> 
> Another thing. I use https://lore.kernel.org/imx/20250701-95_cam-v1-2-
> c5172bab387b@nxp.com/
> for the D-PHY. Is there any update/progress on that driver?
> 
> Best regards,
> Alexander
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
> 


^ permalink raw reply related

* [PATCH v11 0/2] Add Lontium LT7911EXC eDP to MIPI DSI bridge
From: syyang @ 2026-05-22  1:57 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard
  Cc: Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng, qdchen,
	llzhang, Sunyun Yang

From: Sunyun Yang <syyang@lontium.com>

The LT7911EXC is an I2C-controlled bridge that receives eDP1.4
and output mipi dsi. This series introduces:

- A device tree binding YAML file describing the hardware
- A new DRM bridge driver implementing the basic functionality

Signed-off-by: Sunyun Yang<syyang@lontium.com>
---
Change in v11:
- dt-binding:
- drm/bridge:
 1. Removed Reviewed-by, we would appreciate it if the maintainer could help review this.
 2. Use devm_gpiod_get(,, GPIOD_OUT_HIGH) to fix the reset GPIO backpowering issue.                                       [sashiko-bot]
 3. Fixed Use-After-Free vulnerability due to dynamic DRM bridge removal in DSI detach callback. 
 4. Fixed Use-After-Free of regmap during DRM teardown because firmware upgrade worker defeats removal safeguard.
 5. Fixed firmware upgrade performs a hardware reset, causing silent loss of DRM state and a blank screen.
 6. Use of kmalloc for a 64KB software buffer is susceptible to memory fragmentation failures - Fix by using kvmalloc
 7. Use the 'lt7911exc->ocm_lock' mutex and the 'lt7911exc->upgrade' flag to resolve the race condition between
    DRM atomic modeset and firmware upgrade.
 8. Use the 'lt7911exc->upgrade_lock' mutex to prevent redundant upgrades.
 9. Use the 'lt7911exc->removed' flag to prevent the driver from being triggered to upgrade during the unload process.
- Link to v10: https://lore.kernel.org/lkml/20260519135816.26996-1-syyang@lontium.com/

Change in v10:
- dt-binding:
- drm/bridge:
 1. Fixed the firmware upgrade error paths to always clear the upgrade
    flag before returning, including firmware size validation failures
    and allocation failures.                                                                [sashiko-bot]
 2. Added proper locking in lt7911exc_atomic_pre_enable() and
    lt7911exc_atomic_post_disable() to serialize register accesses with the
    firmware upgrade flow and avoid concurrent I2C transactions.
 3. Added an exclusivity check in lt7911exc_dsi_host_attach() to reject multiple
    downstream attachments and prevent repeated drm_bridge_add() calls and panel bridge leaks.
 4. Reworked lt7911exc_firmware_store() to use mutex_trylock() so concurrent sysfs writers
    immediately return -EBUSY instead of blocking behind an active firmware upgrade.
 5. Updated the remove path to prevent new firmware upgrade work from being queued after
    device removal by setting the upgrade state before cancelling the worker.
- Link to v9: https://lore.kernel.org/lkml/20260519105019.22622-1-syyang@lontium.com/

Change in v9:
- dt-binding:
- drm/bridge:
 1. DSI transfer callback returns success for reads without populating                        [sashiko-bot]
    the receive buffer, leaking uninitialized memory. - fixed it by 
    implementing  a strict whitelist mechanism.
 2. DSI transfer callback polls for eDP video readiness before the 
    upstream encoder is enabled, guaranteeing a timeout. - removed
 3. The driver attempts I2C transfers while the hardware is held in
    physical reset. - fixed
 4. Missing DRM_MIPI_DSI Kconfig dependency causes linker errors. - fixed
 5. request_firmware is called while holding the hardware lock and
    halting the MCU, risking a system pipeline stall. - fixed
 6. Sleeping functions are called from atomic context in the DRM bridge callbacks. - fixed
 7. lt7911exc_dsi_host_transfer bypasses the required MCU hardware halt sequence. - fixed by 
    internal firmware controls the panel initialization sequence and handles all MIPI
    DSI command transmission.
- Link to v8: https://lore.kernel.org/lkml/20260515080934.9870-1-syyang@lontium.com/

Change in v8:
- dt-binding:
- drm/bridge:
 1. Protect firmware upgrade and DRM bridge callback paths with ocm_lock.          [sashiko-bot]
 2. Remove the hardware reset from the remove callback, and ensure that
    all hardware reset operations are protected by ocm_lock.
 3. crc reconstruction explicitly casts each byte to u32 before shifting
 4. The display configuration is handled by the firmware, and the MIPI
    DSI host registration issue has been fixed.
 5. The batch register read/write operations have already been updated
    to include return value checking.
 6. The dev_err_probe() used outside of probe context has been fixed.
- Link to v7: https://lore.kernel.org/lkml/20260512064013.40066-1-syyang@lontium.com/

Change in v7:
- dt-binding:
 1. fix commit message typos(Receiver、signal)                            [sashiko-bot]
 2. remove the ambiguity caused by "signal/dual".
- drm/bridge:
 1. using devm_regulator_get_enable avoids power leaks.                   [sashiko-bot]
 2. set reset gpio is low after cutting off power in lt7911exc_remove function, avoid backpowering.
 3. synchronous request_firmware() call cause a permanent probe failure if the driver is built-in,
    probe executes before the root filesystem is mounted, which would cause this to fail with -ENOENT,
    we have removed this functionality. Use trigger to upgrade.
 4. add `depends on I2C` and `select REGMAP_I2C` in Kconfig.
 5. add return value of `devm_drm_bridge_add()` in `probe()`.
 6. add directly header files (linux/slab.h, linux/delay.h, linux/regulator/consumer.h)
- Link to v6: https://lore.kernel.org/lkml/20260508134702.4713-1-syyang@lontium.com/

Change in v6:
- dt-binding:
- drm/bridge:
 1. use #define FW_FILE  "Lontium/lt7911exc_fw.bin" to match linux-firmware
- Link to v5: https://lore.kernel.org/lkml/20260506013153.2240-1-syyang@lontium.com/

Change in v5:
- dt-binding:
- drm/bridge:
 1. Change "mipi" to "mipi dsi" in the commit message.     [Dmitry]
 2. Change "eDP/MIPI" to "eDP/MIPI DSI" in Kconfig.
- Link to v4: https://lore.kernel.org/lkml/20260430094612.3408174-1-syyang@lontium.com/

Change in v4:
- dt-binding:
 1. Fix the missing spaces on the "subject".             [Krzysztof]
 2. Fix the error descriptions for port@0 and port@1.
- drm/bridge:
- Link to v3: https://lore.kernel.org/lkml/20260429040541.3404116-1-syyang@lontium.com/

Change in v3:
- dt-binding:
- drm/bridge:
 1. already submit lt7911exc_fw.bin to linux-firmware.  [Dmitry]
 2. remove lt7911exc_remove function.
 3. drop  the "lontium, "  in lt7911exc_i2c_table.
- Link to v2: https://lore.kernel.org/lkml/20260428063224.3316655-1-syyang@lontium.com/

Change in v2:
- dt-binding:
 1. reset pins use active low.                        [Dmitry]
- drm/bridge:
 1. use atomic_* callbacks.                           [Quentin]
 2. fix the incorrect formatting and spaces.
 3. add the required header files.                    [Dmitry]
 4. remove "enabled" flag.
 5. remove *fw from the lt7911exc struct.
 6. .max_register and .range_max use actual range.
 7. regulator use bulk interface.
 8. use dev_err_probe, devm_mutex_init and devm_drm_bridge_add.
 9. Replace GPL v2 with GPL.
- Link to v1: https://lore.kernel.org/lkml/20260420023354.1192642-1-syyang@lontium.com/

---
Sunyun Yang (2):
  dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
  drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge

 .../display/bridge/lontium,lt7911exc.yaml     |  89 +++
 drivers/gpu/drm/bridge/Kconfig                |  16 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/lontium-lt7911exc.c    | 698 ++++++++++++++++++
 4 files changed, 804 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml
 create mode 100644 drivers/gpu/drm/bridge/lontium-lt7911exc.c

-- 
2.34.1


^ permalink raw reply

* [PATCH v11 1/2] dt-bindings: bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
From: syyang @ 2026-05-22  1:57 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard
  Cc: Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng, qdchen,
	llzhang, Sunyun Yang, Krzysztof Kozlowski
In-Reply-To: <20260522015735.2833-1-syyang@lontium.com>

From: Sunyun Yang <syyang@lontium.com>

This commit adds the device tree binding schema for the Lontium LT7911EXC.
This device is an I2C-controlled bridge that converts eDP 1.4 input to MIPI
DSI output.

Signed-off-by: Sunyun Yang <syyang@lontium.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 .../display/bridge/lontium,lt7911exc.yaml     | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml
new file mode 100644
index 000000000000..3290b10ce883
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt7911exc.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/lontium,lt7911exc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lontium LT7911EXC eDP to MIPI DSI Bridge
+
+maintainers:
+  - Sunyun Yang <syyang@lontium.com>
+
+properties:
+  compatible:
+    enum:
+      - lontium,lt7911exc
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+    description: GPIO connected to RST_ pin.
+
+  vdd-supply:
+    description: Regulator for 1.2V MIPI phy power.
+
+  vcc-supply:
+    description: Regulator for 3.3V IO power.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for eDP input.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for MIPI DSI output.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - vdd-supply
+  - vcc-supply
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mipi-bridge@41 {
+            compatible = "lontium,lt7911exc";
+            reg = <0x41>;
+            reset-gpios = <&gpy8 8 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&lt7911exc_1v2>;
+            vcc-supply = <&lt7911exc_3v3>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    bridge_in: endpoint {
+                        remote-endpoint = <&edp_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    bridge_out: endpoint {
+                        remote-endpoint = <&panel_in>;
+                    };
+                };
+            };
+        };
+    };
-- 
2.34.1


^ permalink raw reply related

* [PATCH v11 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
From: syyang @ 2026-05-22  1:57 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, andrzej.hajda, neil.armstrong,
	dmitry.baryshkov, maarten.lankhorst, rfoss, mripard
  Cc: Laurent.pinchart, tzimmermann, jonas, jernej.skrabec, devicetree,
	dri-devel, linux-kernel, xmzhu, xmzhu, rlyu, xbpeng, qdchen,
	llzhang, Sunyun Yang
In-Reply-To: <20260522015735.2833-1-syyang@lontium.com>

From: Sunyun Yang <syyang@lontium.com>

Add support for the Lontium LT7911EXC bridge chip, which converts
eDP input to MIPI DSI output using an internal firmware-controlled
pipeline.

The driver provides:
- DRM bridge integration for eDP-to-DSI routing
- MIPI DSI host interface for downstream panel attachment
- Firmware upgrade mechanism over I2C (erase/program/verify)
- GPIO-based reset and regulator management

Display timing and MIPI DCS packet generation are handled by the chip
firmware and are not configured by the driver.

Signed-off-by: Sunyun Yang <syyang@lontium.com>
---
 drivers/gpu/drm/bridge/Kconfig             |  16 +
 drivers/gpu/drm/bridge/Makefile            |   1 +
 drivers/gpu/drm/bridge/lontium-lt7911exc.c | 698 +++++++++++++++++++++
 3 files changed, 715 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/lontium-lt7911exc.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c3209b0f4678..013e431e8871 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -132,6 +132,22 @@ config DRM_ITE_IT6505
 	help
 	  ITE IT6505 DisplayPort bridge chip driver.
 
+config DRM_LONTIUM_LT7911EXC
+	tristate "Lontium eDP/MIPI DSI bridge"
+	depends on OF
+	depends on I2C
+	select CRC32
+	select DRM_PANEL
+	select DRM_MIPI_DSI
+	select DRM_KMS_HELPER
+	select FW_LOADER
+	select REGMAP_I2C
+	help
+	  DRM driver for the Lontium LT7911EXC bridge
+	  chip.The LT7911EXC converts eDP input to MIPI
+	  DSI output.
+	  Please say Y if you have such hardware.
+
 config DRM_LONTIUM_LT8912B
 	tristate "Lontium LT8912B DSI/HDMI bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index beab5b695a6e..70ddca75dd3a 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
 obj-$(CONFIG_DRM_INNO_HDMI) += inno-hdmi.o
 obj-$(CONFIG_DRM_ITE_IT6263) += ite-it6263.o
 obj-$(CONFIG_DRM_ITE_IT6505) += ite-it6505.o
+obj-$(CONFIG_DRM_LONTIUM_LT7911EXC) += lontium-lt7911exc.o
 obj-$(CONFIG_DRM_LONTIUM_LT8912B) += lontium-lt8912b.o
 obj-$(CONFIG_DRM_LONTIUM_LT9211) += lontium-lt9211.o
 obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
diff --git a/drivers/gpu/drm/bridge/lontium-lt7911exc.c b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
new file mode 100644
index 000000000000..1aec60f02f91
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lontium-lt7911exc.c
@@ -0,0 +1,698 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 Lontium Semiconductor, Inc.
+ */
+
+#include <linux/crc32.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_probe_helper.h>
+#include <video/mipi_display.h>
+
+#define FW_SIZE (64 * 1024)
+#define LT_PAGE_SIZE 32
+#define FW_FILE  "Lontium/lt7911exc_fw.bin"
+#define LT7911EXC_PAGE_CONTROL 0xff
+
+struct lt7911exc_dsi_output {
+	struct mipi_dsi_device *dev;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
+};
+
+struct lt7911exc {
+	struct device *dev;
+	struct i2c_client *client;
+	struct drm_bridge bridge;
+	struct work_struct work;
+	struct mipi_dsi_host dsi_host;
+	struct lt7911exc_dsi_output output;
+	struct regmap *regmap;
+	/* Fast lock: guards short register r/w and status checks */
+	struct mutex ocm_lock;
+	/* Long lock: serializes firmware upgrade process to prevent DRM interference */
+	struct mutex upgrade_lock;
+	struct gpio_desc *reset_gpio;
+	int fw_version;
+	bool upgrade;
+	bool removed;
+};
+
+static const struct regmap_range_cfg lt7911exc_ranges[] = {
+	{
+		.name = "register_range",
+		.range_min =  0,
+		.range_max = 0xe8ff,
+		.selector_reg = LT7911EXC_PAGE_CONTROL,
+		.selector_mask = 0xff,
+		.selector_shift = 0,
+		.window_start = 0,
+		.window_len = 0x100,
+	},
+};
+
+static const struct regmap_config lt7911exc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xe8ff,
+	.ranges = lt7911exc_ranges,
+	.num_ranges = ARRAY_SIZE(lt7911exc_ranges),
+};
+
+static u32 cal_crc32_custom(const u8 *data, u64 length)
+{
+	u32 crc = 0xffffffff;
+	u8 buf[4];
+	u64 i;
+
+	if (!length || (length & 3))
+		return 0;
+
+	for (i = 0; i < length; i += 4) {
+		buf[0] = data[i + 3];
+		buf[1] = data[i + 2];
+		buf[2] = data[i + 1];
+		buf[3] = data[i + 0];
+		crc = crc32_be(crc, buf, 4);
+	}
+
+	return crc;
+}
+
+static inline struct lt7911exc *bridge_to_lt7911exc(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct lt7911exc, bridge);
+}
+
+static inline struct lt7911exc *dsi_host_to_lt7911exc(struct mipi_dsi_host *host)
+{
+	return container_of(host, struct lt7911exc, dsi_host);
+}
+
+static void lt7911exc_reset(struct lt7911exc *lt7911exc)
+{
+	/* Assert reset pin: logical 1 -> physical state low (Reset Active) */
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
+	msleep(20);
+
+	/* Deassert reset pin: logical 0 -> physical state high (Run state) */
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0);
+	msleep(400);
+
+	dev_dbg(lt7911exc->dev, "lt7911exc physical hardware reset sequence completed.\n");
+}
+
+static int lt7911exc_hw_mcu_halt(struct lt7911exc *lt7911exc)
+{
+	return regmap_write(lt7911exc->regmap, 0xe0ee, 0x01);
+}
+
+static int lt7911exc_hw_mcu_run(struct lt7911exc *lt7911exc)
+{
+	return regmap_write(lt7911exc->regmap, 0xe0ee, 0x00);
+}
+
+static int lt7911exc_regulator_enable(struct lt7911exc *lt7911exc)
+{
+	int ret;
+
+	ret = devm_regulator_get_enable(lt7911exc->dev, "vcc");
+	if (ret < 0)
+		return dev_err_probe(lt7911exc->dev, ret, "failed to enable vcc regulator\n");
+
+	usleep_range(5000, 10000);
+
+	ret = devm_regulator_get_enable(lt7911exc->dev, "vdd");
+	if (ret < 0)
+		return dev_err_probe(lt7911exc->dev, ret, "failed to enable vdd regulator\n");
+
+	return 0;
+}
+
+static int lt7911exc_read_version(struct lt7911exc *lt7911exc)
+{
+	u8 buf[3];
+	int ret;
+
+	/* no need to halt MCU for this register access */
+	ret = regmap_bulk_read(lt7911exc->regmap, 0xe081, buf, ARRAY_SIZE(buf));
+	if (ret)
+		return ret;
+
+	return (buf[0] << 16) | (buf[1] << 8) | buf[2];
+}
+
+static int lt7911exc_block_erase(struct lt7911exc *lt7911exc)
+{
+	struct device *dev = lt7911exc->dev;
+	const u32 addr = 0x00;
+	int ret;
+
+	const struct reg_sequence seq_write[] = {
+		REG_SEQ0(0xe0ee, 0x01),
+		REG_SEQ0(0xe054, 0x01),
+		REG_SEQ0(0xe055, 0x06),
+		REG_SEQ0(0xe051, 0x01),
+		REG_SEQ0(0xe051, 0x00),
+		REG_SEQ0(0xe054, 0x05),
+		REG_SEQ0(0xe055, 0xd8),
+		REG_SEQ0(0xe05a, (addr >> 16) & 0xff),
+		REG_SEQ0(0xe05b, (addr >> 8) & 0xff),
+		REG_SEQ0(0xe05c, addr & 0xff),
+		REG_SEQ0(0xe051, 0x01),
+		REG_SEQ0(0xe050, 0x00),
+	};
+
+	ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write));
+	if (ret)
+		return ret;
+
+	msleep(200);
+	dev_dbg(dev, "erase flash done.\n");
+
+	return 0;
+}
+
+static int lt7911exc_prog_init(struct lt7911exc *lt7911exc, u64 addr)
+{
+	int ret;
+
+	const struct reg_sequence seq_write[] = {
+		REG_SEQ0(0xe0ee, 0x01),
+		REG_SEQ0(0xe05f, 0x01),
+		REG_SEQ0(0xe05a, (addr >> 16) & 0xff),
+		REG_SEQ0(0xe05b, (addr >> 8) & 0xff),
+		REG_SEQ0(0xe05c, addr & 0xff),
+	};
+
+	ret = regmap_multi_reg_write(lt7911exc->regmap, seq_write, ARRAY_SIZE(seq_write));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int lt7911exc_write_data(struct lt7911exc *lt7911exc, const struct firmware *fw, u64 addr)
+{
+	struct device *dev = lt7911exc->dev;
+	int ret;
+	int page = 0, num = 0, page_len = 0;
+	u64 size, offset;
+	const u8 *data;
+
+	data = fw->data;
+	size = fw->size;
+	page = (size + LT_PAGE_SIZE - 1) / LT_PAGE_SIZE;
+	if (page * LT_PAGE_SIZE > FW_SIZE) {
+		dev_err(dev, "firmware size out of range\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "%u pages, total size %llu byte\n", page, size);
+
+	for (num = 0; num < page; num++) {
+		offset = num * LT_PAGE_SIZE;
+		page_len = (offset + LT_PAGE_SIZE <= size) ? LT_PAGE_SIZE : (size - offset);
+		ret = lt7911exc_prog_init(lt7911exc, addr);
+		if (ret)
+			return ret;
+
+		ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, &data[offset], page_len);
+		if (ret) {
+			dev_err(dev, "write error at page %d\n", num);
+			return ret;
+		}
+
+		if (page_len < LT_PAGE_SIZE) {
+			regmap_write(lt7911exc->regmap, 0xe05f, 0x05);
+			regmap_write(lt7911exc->regmap, 0xe05f, 0x01);
+			//hardware requires delay
+			usleep_range(1000, 2000);
+		}
+
+		regmap_write(lt7911exc->regmap, 0xe05f, 0x00);
+		addr += LT_PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static int lt7911exc_write_crc(struct lt7911exc *lt7911exc, u32 crc32, u64 addr)
+{
+	u8 crc[4];
+	int ret;
+
+	crc[0] = crc32 & 0xff;
+	crc[1] = (crc32 >> 8) & 0xff;
+	crc[2] = (crc32 >> 16) & 0xff;
+	crc[3] = (crc32 >> 24) & 0xff;
+
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x01);
+	regmap_write(lt7911exc->regmap, 0xe05a, (addr >> 16) & 0xff);
+	regmap_write(lt7911exc->regmap, 0xe05b, (addr >> 8) & 0xff);
+	regmap_write(lt7911exc->regmap, 0xe05c, addr & 0xff);
+
+	ret = regmap_raw_write(lt7911exc->regmap, 0xe05d, crc, 4);
+	if (ret)
+		return ret;
+
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x05);
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x01);
+	usleep_range(1000, 2000);
+	regmap_write(lt7911exc->regmap, 0xe05f, 0x00);
+
+	return 0;
+}
+
+static int lt7911exc_upgrade_result(struct lt7911exc *lt7911exc, u32 crc32)
+{
+	struct device *dev = lt7911exc->dev;
+	u32 read_hw_crc = 0;
+	u8 crc_tmp[4];
+	int ret;
+
+	regmap_write(lt7911exc->regmap, 0xe0ee, 0x01);
+	regmap_write(lt7911exc->regmap, 0xe07b, 0x60);
+	regmap_write(lt7911exc->regmap, 0xe07b, 0x40);
+	msleep(150);
+	ret = regmap_bulk_read(lt7911exc->regmap, 0x22, crc_tmp, ARRAY_SIZE(crc_tmp));
+	if (ret) {
+		dev_err(lt7911exc->dev, "Failed to read CRC: %d\n", ret);
+		return ret;
+	}
+	regmap_write(lt7911exc->regmap, 0xe0ee, 0x00);
+
+	read_hw_crc = ((u32)crc_tmp[0] << 24) | ((u32)crc_tmp[1] << 16) |
+				((u32)crc_tmp[2] << 8) | ((u32)crc_tmp[3]);
+
+	if (read_hw_crc != crc32) {
+		dev_err(dev, "lt7911exc firmware upgrade failed, expected CRC=0x%08x, read CRC=0x%08x\n",
+			crc32, read_hw_crc);
+		return -EIO;
+	}
+
+	dev_dbg(dev, "lt7911exc firmware upgrade success, CRC=0x%08x\n", read_hw_crc);
+	return 0;
+}
+
+static void lt7911exc_firmware_upgrade_work(struct work_struct *work)
+{
+	struct lt7911exc *lt7911exc = container_of(work, struct lt7911exc, work);
+	struct device *dev = lt7911exc->dev;
+	const struct firmware *fw;
+	u8 *buffer;
+	size_t total_size = FW_SIZE - 4;
+	u32 crc32;
+	int ret;
+
+	ret = request_firmware(&fw, FW_FILE, dev);
+	if (ret) {
+		dev_err(dev, "failed to load '%s'\n", FW_FILE);
+		goto out_clear_status;
+	}
+
+	if (fw->size > total_size) {
+		dev_err(dev, "firmware too large (%zu > %zu)\n", fw->size, total_size);
+		goto out_release_fw;
+	}
+
+	buffer = kvmalloc(total_size, GFP_KERNEL);
+	if (!buffer) {
+		ret = -ENOMEM;
+		goto out_release_fw;
+	}
+
+	memset(buffer, 0xff, total_size);
+	memcpy(buffer, fw->data, fw->size);
+	crc32 = cal_crc32_custom(buffer, total_size);
+	kvfree(buffer);
+
+	mutex_lock(&lt7911exc->upgrade_lock);
+
+	mutex_lock(&lt7911exc->ocm_lock);
+	lt7911exc_reset(lt7911exc);
+	lt7911exc_hw_mcu_halt(lt7911exc);
+	mutex_unlock(&lt7911exc->ocm_lock);
+
+	ret = lt7911exc_block_erase(lt7911exc);
+	if (ret) {
+		dev_err(dev, "failed to block erase.\n");
+		goto out_unlock;
+	}
+
+	ret = lt7911exc_write_data(lt7911exc, fw, 0);
+	if (ret < 0) {
+		dev_err(dev, "failed to write firmware data\n");
+		goto out_unlock;
+	}
+
+	ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4);
+	if (ret < 0) {
+		dev_err(dev, "failed to write firmware crc\n");
+		goto out_unlock;
+	}
+
+	mutex_lock(&lt7911exc->ocm_lock);
+	lt7911exc_reset(lt7911exc);
+	mutex_unlock(&lt7911exc->ocm_lock);
+
+	ret = lt7911exc_upgrade_result(lt7911exc, crc32);
+	if (ret)
+		dev_err(dev, "firmware verification failed\n");
+
+out_unlock:
+	mutex_lock(&lt7911exc->ocm_lock);
+	lt7911exc_hw_mcu_run(lt7911exc);
+	lt7911exc->fw_version = lt7911exc_read_version(lt7911exc);
+	mutex_unlock(&lt7911exc->ocm_lock);
+
+	mutex_unlock(&lt7911exc->upgrade_lock);
+	/* Notify DRM framework that hardware state changed/needs a modeset */
+	if (lt7911exc->bridge.dev)
+		drm_kms_helper_hotplug_event(lt7911exc->bridge.dev);
+
+out_release_fw:
+	release_firmware(fw);
+
+out_clear_status:
+	mutex_lock(&lt7911exc->ocm_lock);
+	/* Only clear status if the module is not in the process of removal */
+	if (!lt7911exc->removed)
+		lt7911exc->upgrade = false;
+	mutex_unlock(&lt7911exc->ocm_lock);
+}
+
+static void lt7911exc_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
+
+	guard(mutex)(&lt7911exc->ocm_lock);
+
+	//enable mipi stream
+	if (!lt7911exc->upgrade)
+		regmap_write(lt7911exc->regmap, 0xe0b0, 0x01);
+}
+
+static void lt7911exc_atomic_post_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
+{
+	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
+
+	guard(mutex)(&lt7911exc->ocm_lock);
+
+	//disable mipi stream
+	if (!lt7911exc->upgrade)
+		regmap_write(lt7911exc->regmap, 0xe0b0, 0x00);
+}
+
+static int lt7911exc_bridge_attach(struct drm_bridge *bridge,
+				   struct drm_encoder *encoder,
+				   enum drm_bridge_attach_flags flags)
+{
+	struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
+
+	if (!lt7911exc->output.bridge) {
+		dev_warn(lt7911exc->dev, "Next bridge/panel not attached yet, deferring\n");
+		return -EPROBE_DEFER;
+	}
+
+	return drm_bridge_attach(encoder, lt7911exc->output.bridge, bridge, flags);
+}
+
+static const struct drm_bridge_funcs lt7911exc_bridge_funcs = {
+	.attach = lt7911exc_bridge_attach,
+	.atomic_pre_enable = lt7911exc_atomic_pre_enable,
+	.atomic_post_disable = lt7911exc_atomic_post_disable,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+};
+
+static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
+{
+	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
+	struct drm_bridge *bridge;
+	struct drm_panel *panel;
+	int ret;
+
+	if (lt7911exc->output.dev)
+		return -EBUSY;
+
+	ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &panel, &bridge);
+	if (ret)
+		return ret;
+
+	if (panel) {
+		bridge = drm_panel_bridge_add_typed(panel, DRM_MODE_CONNECTOR_DSI);
+		if (IS_ERR(bridge))
+			return PTR_ERR(bridge);
+	}
+	lt7911exc->output.dev = dev;
+	lt7911exc->output.bridge = bridge;
+	lt7911exc->output.panel = panel;
+
+	return 0;
+}
+
+static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
+{
+	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
+
+	if (!lt7911exc->output.dev)
+		return 0;
+
+	lt7911exc->output.dev = NULL;
+
+	return 0;
+}
+
+/*
+ * The internal firmware controls the panel initialization
+ * sequence and handles all MIPI DSI command transmission.
+ */
+static ssize_t lt7911exc_dsi_host_transfer(struct mipi_dsi_host *host,
+					   const struct mipi_dsi_msg *msg)
+{
+	struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
+
+	if (msg->rx_len) {
+		dev_warn(lt7911exc->dev, "MIPI DSI read is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	switch (msg->type) {
+	case MIPI_DSI_DCS_SHORT_WRITE:
+	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
+	case MIPI_DSI_DCS_LONG_WRITE:
+	case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM:
+	case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM:
+	case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
+	case MIPI_DSI_GENERIC_LONG_WRITE:
+	break;
+	default:
+	return -EOPNOTSUPP;
+	}
+
+	guard(mutex)(&lt7911exc->ocm_lock);
+
+	if (lt7911exc->upgrade)
+		return -EBUSY;
+
+	return msg->tx_len;
+}
+
+static const struct mipi_dsi_host_ops lt7911exc_dsi_host_ops = {
+	.attach = lt7911exc_dsi_host_attach,
+	.detach = lt7911exc_dsi_host_detach,
+	.transfer = lt7911exc_dsi_host_transfer,
+};
+
+static ssize_t lt7911exc_firmware_store(struct device *dev, struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct lt7911exc *lt7911exc = dev_get_drvdata(dev);
+
+	if (!mutex_trylock(&lt7911exc->upgrade_lock))
+		return -EBUSY;
+
+	mutex_lock(&lt7911exc->ocm_lock);
+
+	if (lt7911exc->upgrade || lt7911exc->removed) {
+		mutex_unlock(&lt7911exc->ocm_lock);
+		mutex_unlock(&lt7911exc->upgrade_lock);
+		return -EBUSY;
+	}
+
+	lt7911exc->upgrade = true;
+	mutex_unlock(&lt7911exc->ocm_lock);
+	mutex_unlock(&lt7911exc->upgrade_lock);
+
+	schedule_work(&lt7911exc->work);
+
+	return len;
+}
+
+static ssize_t lt7911exc_firmware_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct lt7911exc *lt7911exc = dev_get_drvdata(dev);
+	int version;
+
+	mutex_lock(&lt7911exc->ocm_lock);
+	version = lt7911exc->fw_version;
+	mutex_unlock(&lt7911exc->ocm_lock);
+	return sysfs_emit(buf, "0x%04x\n", version);
+}
+
+static DEVICE_ATTR_RW(lt7911exc_firmware);
+
+static struct attribute *lt7911exc_attrs[] = {
+	&dev_attr_lt7911exc_firmware.attr,
+	NULL,
+};
+
+static const struct attribute_group lt7911exc_attr_group = {
+	.attrs = lt7911exc_attrs,
+};
+
+static const struct attribute_group *lt7911exc_attr_groups[] = {
+	&lt7911exc_attr_group,
+	NULL,
+};
+
+static int lt7911exc_probe(struct i2c_client *client)
+{
+	struct lt7911exc *lt7911exc;
+	struct device *dev = &client->dev;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return dev_err_probe(dev, -ENODEV, "device doesn't support I2C\n");
+
+	lt7911exc = devm_drm_bridge_alloc(dev, struct lt7911exc, bridge, &lt7911exc_bridge_funcs);
+	if (IS_ERR(lt7911exc))
+		return dev_err_probe(dev, PTR_ERR(lt7911exc), "drm bridge alloc failed.\n");
+
+	dev_set_drvdata(dev, lt7911exc);
+
+	lt7911exc->client = client;
+	lt7911exc->dev = dev;
+	lt7911exc->upgrade = false;
+	lt7911exc->removed = false;
+
+	ret = devm_mutex_init(dev, &lt7911exc->ocm_lock);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init mutex\n");
+
+	ret = devm_mutex_init(dev, &lt7911exc->upgrade_lock);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init upgrade_lock\n");
+
+	lt7911exc->regmap = devm_regmap_init_i2c(client, &lt7911exc_regmap_config);
+	if (IS_ERR(lt7911exc->regmap))
+		return dev_err_probe(dev, PTR_ERR(lt7911exc->regmap), "regmap i2c init failed\n");
+
+	lt7911exc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(lt7911exc->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(lt7911exc->reset_gpio),
+				     "failed to acquire reset gpio\n");
+
+	ret = lt7911exc_regulator_enable(lt7911exc);
+	if (ret)
+		return ret;
+
+	lt7911exc_reset(lt7911exc);
+
+	mutex_lock(&lt7911exc->ocm_lock);
+
+	lt7911exc->fw_version = lt7911exc_read_version(lt7911exc);
+
+	mutex_unlock(&lt7911exc->ocm_lock);
+
+	if (lt7911exc->fw_version < 0)
+		return dev_err_probe(dev, lt7911exc->fw_version, "failed read version of chip\n");
+
+	lt7911exc->dsi_host.dev = dev;
+	lt7911exc->dsi_host.ops = &lt7911exc_dsi_host_ops;
+	lt7911exc->bridge.of_node = np;
+
+	INIT_WORK(&lt7911exc->work, lt7911exc_firmware_upgrade_work);
+
+	i2c_set_clientdata(client, lt7911exc);
+
+	drm_bridge_add(&lt7911exc->bridge);
+
+	ret = mipi_dsi_host_register(&lt7911exc->dsi_host);
+	if (ret) {
+		drm_bridge_remove(&lt7911exc->bridge);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void lt7911exc_remove(struct i2c_client *client)
+{
+	struct lt7911exc *lt7911exc = i2c_get_clientdata(client);
+
+	mutex_lock(&lt7911exc->ocm_lock);
+	lt7911exc->removed = true;
+	lt7911exc->upgrade = true;
+	mutex_unlock(&lt7911exc->ocm_lock);
+
+	cancel_work_sync(&lt7911exc->work);
+
+	mipi_dsi_host_unregister(&lt7911exc->dsi_host);
+	drm_bridge_remove(&lt7911exc->bridge);
+
+	if (lt7911exc->output.panel && lt7911exc->output.bridge)
+		drm_panel_bridge_remove(lt7911exc->output.bridge);
+
+	gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
+}
+
+static const struct i2c_device_id lt7911exc_i2c_table[] = {
+	{"lt7911exc"},
+	{/* sentinel */}
+};
+
+MODULE_DEVICE_TABLE(i2c, lt7911exc_i2c_table);
+
+static const struct of_device_id lt7911exc_devices[] = {
+	{.compatible = "lontium,lt7911exc"},
+	{/* sentinel */}
+};
+MODULE_DEVICE_TABLE(of, lt7911exc_devices);
+
+static struct i2c_driver lt7911exc_driver = {
+	.id_table	= lt7911exc_i2c_table,
+	.probe		= lt7911exc_probe,
+	.remove		= lt7911exc_remove,
+	.driver		= {
+		.name	= "lt7911exc",
+		.of_match_table = lt7911exc_devices,
+		.dev_groups = lt7911exc_attr_groups,
+	},
+};
+module_i2c_driver(lt7911exc_driver);
+
+MODULE_AUTHOR("SunYun Yang <syyang@lontium.com>");
+MODULE_DESCRIPTION("Lontium LT7911EXC EDP to MIPI DSI bridge driver");
+MODULE_LICENSE("GPL");
+MODULE_FIRMWARE(FW_FILE);
-- 
2.34.1


^ permalink raw reply related

* RE: [PATCH v1 1/2] bindings: iio: adc: Add StarFive JHB100 SARADC
From: Xingyu Wu @ 2026-05-22  1:56 UTC (permalink / raw)
  To: Conor Dooley, Jonathan Cameron
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org
In-Reply-To: <20260521-bunny-ranch-77fddc80ad83@spud>

On 2026/5/22 00:42, Conor Dooley wrote:
> 
> On Thu, May 21, 2026 at 12:39:49PM +0100, Jonathan Cameron wrote:
> > On Thu, 21 May 2026 11:20:52 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> >
> > > On Thu, May 21, 2026 at 09:54:27AM +0000, Xingyu Wu wrote:
> > > > On 2026/5/20 23:15, Conor Dooley wrote:
> > > > >
> > > > > On Wed, May 20, 2026 at 09:43:02AM +0000, Xingyu Wu wrote:
> > > > > > On 2026/5/19 18:00, Conor Dooley wrote:
> > > > > > >
> > > > > > > On Tue, May 19, 2026 at 09:26:03AM +0000, Xingyu Wu wrote:
> > > > > > > > On 2026/5/19 00:24, Conor Dooley wrote:
> > > > > > > > >
> > > > > > > > > On Mon, May 18, 2026 at 04:18:51PM +0800, Xingyu Wu wrote:
> > > > > > > > > > Add the new documentation of SAR-ADC for the StarFive JHB100
> SoC.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> > > > > > > > > > ---
> > > > > > > > > >  .../iio/adc/starfive,jhb100-saradc.yaml       | 62
> +++++++++++++++++++
> > > > > > > > > >  1 file changed, 62 insertions(+)  create mode 100644
> > > > > > > > > > Documentation/devicetree/bindings/iio/adc/starfive,jhb
> > > > > > > > > > 100-sara
> > > > > > > > > > dc.y
> > > > > > > > > > aml
> > > > > > > > > >
> > > > > > > > > > diff --git
> > > > > > > > > > a/Documentation/devicetree/bindings/iio/adc/starfive,j
> > > > > > > > > > hb100-sa
> > > > > > > > > > radc
> > > > > > > > > > .yam
> > > > > > > > > > l
> > > > > > > > > > b/Documentation/devicetree/bindings/iio/adc/starfive,j
> > > > > > > > > > hb100-sa
> > > > > > > > > > radc
> > > > > > > > > > .yam
> > > > > > > > > > l
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 000000000000..ba8e19b72ad7
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/starfi
> > > > > > > > > > +++ ve,jhb10
> > > > > > > > > > +++ 0-sa
> > > > > > > > > > +++ radc
> > > > > > > > > > +++ .yaml
> > > > > > > > > > @@ -0,0 +1,62 @@
> > > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > > > > > > > +%YAML
> > > > > > > > > > +1.2
> > > > > > > > > > +---
> > > > > > > > > > +$id:
> > > > > > > > > > +http://devicetree.org/schemas/iio/adc/starfive,jhb100-saradc.
> > > > > > > > > > +yaml
> > > > > > > > > > +#
> > > > > > > > > > +$schema:
> > > > > > > > > > +http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > > > > +
> > > > > > > > > > +title: Successive Approximation Register (SAR) A/D
> > > > > > > > > > +converter for the StarFive JHB100 SoC
> > > > > > > > > > +
> > > > > > > > > > +maintainers:
> > > > > > > > > > +  - Xingyu Wu <xingyu.wu@starfivetech.com>
> > > > > > > > > > +
> > > > > > > > > > +properties:
> > > > > > > > > > +  compatible:
> > > > > > > > > > +    const: starfive,jhb100-saradc
> > > > > > > > > > +
> > > > > > > > > > +  reg:
> > > > > > > > > > +    maxItem: 1
> > > > > > > > > > +
> > > > > > > > > > +  interrupts:
> > > > > > > > > > +    maxItems: 1
> > > > > > > > > > +
> > > > > > > > > > +  clocks:
> > > > > > > > > > +    maxItems: 1
> > > > > > > > > > +
> > > > > > > > > > +  resets:
> > > > > > > > > > +    maxItems: 2
> > > > > > > > > > +
> > > > > > > > > > +  "#io-channel-cells":
> > > > > > > > > > +    const: 1
> > > > > > > > > > +
> > > > > > > > > > +  upper-bound-mv:
> > > > > > > > > > +    description: The upper bound voltage value of the monitor.
> > > > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint16
> > > > > > > > > > +
> > > > > > > > > > +  lower-bound-mv:
> > > > > > > > > > +    description: The lower bound voltage value of the monitor.
> > > > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint16
> > > > > > > > > > +
> > > > > > > > > > +  scan-freq:
> > > > > > > > > > +    description: Number of the scan cycle interval.
> > > > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint16
> > > > > > > > >
> > > > > > > > > Can you explain why any of these three properties are
> > > > > > > > > something that should be in the devicetree rather than software
> controlled?
> > > > > > > >
> > > > > > > > My intention is to be able to obtain the initial values
> > > > > > > > from the devicetree during
> > > > > > > probe and preset them.
> > > > > > > > Do I need to drop them and just set them through sysfs?
> > > > > > >
> > > > > > > Unless the hardware configuration determines the values
> > > > > > > (which I can't really see being the case for scan-freq at
> > > > > > > least) then yes, you need to drop and set them via sysfs.
> > > > > >
> > > > > > The ADC hardware can be set the scan-freq register to
> > > > > > determine how frequent it
> > > > > should scan its inputs.
> > > > > > The calculation is:
> > > > > > 	frequency = 100/((register value) + 5) MHz, The register value
> should >= 15.
> > > > > > The maximum allowable scan frequency is 5MHz.
> > > > > >
> > > > > > >
> > > > > > > > > How are the bounds calculated?
> > > > > > > >
> > > > > > > > The measurement range of this ADC hardware is from 0 to 1800 mV.
> > > > > > > > This set
> > > > > > > value cannot exceed it. This explanation will be added later.
> > > > > > >
> > > > > > > I'm asking how this is calculated so that I can tell if you
> > > > > > > the property is permitted or not.
> > > > > >
> > > > > > The calculation of bound is:
> > > > > > bound-mv = 1800mv * (register value) / 0xFFF
> > > > >
> > > > > These are the formulas, but how does someone know what the value
> > > > > for bound- mv needs to be? Why would someone not just want to always
> use 1800mv?
> > > > >
> > > >
> > > > Can I add the 'maximum' and ' minimum' to provide clarification? And the
> driver will also check.
> > >
> > > All that does is repeat the 1800 mV though, what I am interested in
> > > is how someone determines if they should use 1600 mV or 200 mV etc.
> > > What aspect of the hardware do the bounds depend on?
> >
> > There are two options here
> > 1. This is critical stuff to avoid hardware damage. (If you are relying on
> >    Linux for that you built your system wrong but if we ignore that...)
> >    Then userspace control should not be possible - or at least should
> >    only be able to move boundaries in directions that make them tighter.
> > 2. It is advisory only and not related to hardware damage - in that case
> >    generally doesn't belong in DT.
> 
> Per Xingyu's latest response
> | These bounds are just for the monitor mode. If the input voltage is outside this
> range, interrupt will be triggered to report the user space.
> | The bounds value (1600mv or 200mv) are determined by the users' applications.
> Users can freely set them according to the range they want to monitor.
> it controls the level at which interrupts occur, and is therefore not suitable for DT.
> 

OK, I will drop them in DT and use IIO event to set them in driver by user.

Best regards,
Xingyu Wu

^ permalink raw reply

* Re: [PATCH 0/9] riscv: ultrarisc: add DP1000 SoC DT and pinctrl support
From: Jia Wang @ 2026-05-22  1:41 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Jia Wang, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti, Linus Walleij, Bartosz Golaszewski,
	Samuel Holland, Paul Walmsley, Palmer Dabbelt, devicetree,
	linux-riscv, linux-kernel, linux-gpio
In-Reply-To: <20260521-trusting-busybody-15315a5955b7@spud>

On 2026-05-21 11:23 +0100, Conor Dooley wrote:
> On Thu, May 21, 2026 at 05:52:34PM +0800, Jia Wang wrote:
> > > Additionally, these pinctrl patches should be sent standalone to the
> > > pinctrl maintainers, they're likely to go through lots of revisions and
> > > a different maintainer applies them.
> > > 
> > 
> > Just to confirm — for v2, should I completely remove the pinctrl
> > binding/driver from this series and send them as a standalone pinctrl
> > series, while keeping only the pinctrl references in the DTS?
> 
> IMO yes. And cite a link to the pinctrl patches in your cover letter.
> 
> Probably you're better off submitting v2 of pinctrl as a higher priority
> anyway, since a) the biggest changes are needed there and b) the dts
> changes depend on a clock driver/binding being created.

Thanks, that's clear.

I’ll prioritize submitting the pinctrl driver/binding and the clock
driver/binding as separate series first. The DTS updates will follow
once those are in place.

Thanks again for the guidance.

Best Regards,
Jia Wang



^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
From: Damon Ding @ 2026-05-22  1:32 UTC (permalink / raw)
  To: Conor Dooley
  Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
	neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
	nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
	dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
	dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <20260521-modify-quadrant-8d6a2e36727c@spud>

On 5/22/2026 3:50 AM, Conor Dooley wrote:
> On Thu, May 21, 2026 at 07:44:57PM +0800, Damon Ding wrote:
>> Add data-lanes property support to the port@1 endpoint for physical
>> lane mapping configuration.
>>
>> Lane mapping is mainly used for below scenarios:
>> 1. Correct PCB lane swap and differential line routing crossover
>>     without hardware changes;
>> 2. Adapt mismatched lane pin definitions between SoC and eDP panel;
>> 3. Support multiple panel hardware variants on the same board
>>     by configuring data-lanes in device tree only.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> Sashiko complaint here looks valid.
> pw-bot: changes-requested
> 

Yes, unevaluatedProperties: false should be added to both the port@1
and endpoint nodes.

Will fix in v3.

Best regards,
Damon

> 
>>
>> ---
>>
>> Changes in v2:
>> - Add lane mapping application scenarios in commit message.
>> - Remove redundant deprecated property 'data-lanes' for eDP node.
>> - Update port@1 $ref to /schemas/graph.yaml#/$defs/port-base.
>> ---
>>   .../bindings/display/bridge/analogix,dp.yaml    | 17 ++++++++++++-----
>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> index 62f0521b0924..e34fdb21adb4 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> @@ -42,13 +42,20 @@ properties:
>>       properties:
>>         port@0:
>>           $ref: /schemas/graph.yaml#/properties/port
>> -        description:
>> -          Input node to receive pixel data.
>> +        description: Input node to receive pixel data.
>>   
>>         port@1:
>> -        $ref: /schemas/graph.yaml#/properties/port
>> -        description:
>> -          Port node with one endpoint connected to a dp-connector node.
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        description: Port node with one endpoint connected to sink device node.
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            properties:
>> +              data-lanes:
>> +                minItems: 1
>> +                maxItems: 4
>> +                items:
>> +                  enum: [ 0, 1, 2, 3 ]
>>   
>>       required:
>>         - port@0
>> -- 
>> 2.34.1
>>


^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example
From: Damon Ding @ 2026-05-22  1:25 UTC (permalink / raw)
  To: Conor Dooley
  Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
	neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
	nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
	dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
	dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <20260521-powdery-tux-22a7cf4fadc3@spud>

On 5/22/2026 3:47 AM, Conor Dooley wrote:
> On Thu, May 21, 2026 at 07:44:58PM +0800, Damon Ding wrote:
>> Add data-lanes setting in endpoint example to show actual lane mapping
>> usage.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> Squash this with patch 1.
> pw-bot: changes-requested
> 

Yes, will fix in v3

Best regards,
Damon

> 
>> ---
>>   .../bindings/display/rockchip/rockchip,analogix-dp.yaml          | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> index bb75d898a5c5..cf75c926318b 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> @@ -151,6 +151,7 @@ examples:
>>             reg = <1>;
>>   
>>             edp_out_panel: endpoint {
>> +            data-lanes = <0 1>;
>>               remote-endpoint = <&panel_in_edp>;
>>             };
>>           };
>> -- 
>> 2.34.1
>>


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: soc: qcom: pmic-glink: Document batteryless property
From: Dmitry Baryshkov @ 2026-05-21 23:34 UTC (permalink / raw)
  To: Kamal Wadhwa
  Cc: Krzysztof Kozlowski, jishnu.prakash, Rakesh Kota, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Sebastian Reichel, linux-arm-msm, devicetree, linux-kernel,
	linux-pm
In-Reply-To: <20260521071341.q4efqssppvettaey@hu-kamalw-hyd.qualcomm.com>

On Thu, May 21, 2026 at 12:43:41PM +0530, Kamal Wadhwa wrote:
> On Tue, May 19, 2026 at 12:35:13PM +0200, Krzysztof Kozlowski wrote:
> > On Tue, May 19, 2026 at 01:55:26PM +0530, Rakesh Kota wrote:
> > > 
> > > > And isn't lack of monitored battery property enough to indicate that?
> > > 
> > > Regarding monitored-battery — its absence alone isn't sufficient. The
> > > BATT_ID line on debug boards is pulled to ~10kΩ, which is used during
> > > development phase where some battery properties are still present. The
> > > same ~10kΩ value is also used on some genuinely battery-less production
> > > platforms where no battery properties exist, making auto-detection
> > > unreliable. Hence the need for an explicit DT property to identify
> > > hardware platforms where no battery populated. 
> > 
> > I don't understand this logic. So you claim you have debug boards which
> > do not have battery, but define monitored-battery? Then these are wrong
> > and fix them first.
> 
> Actually our firmware treats the debug board as a "fake battery" rather then
> a "no-battery" case.
> 
> This is done to avoid triggering shutdown or trigger power/thermal related
> mitigations to kick in from the HLOS (android) that is configured mainly for
> battery-backed devices.
> 
> Note that we can know if its a debug board, just by looking at the battery
> ID resistance or the battery profile name in the power supply properties
> for `qcom-battmgr-bat` in sysfs.
> 
> However, the problem started with the boards that are battery-less and
> unfortunetely used the same debug board batt ID resistance value, so from
> the firmware side the batteryless board is also seen same as a board with
> debug-board connected.

Which devices are using this resistance value? Can this be fixed by
resoldering the devices? Can we fix this by pushing this property into
the adsp_dtb.mbn and then using it for those affected devices only?

> Since firmware does not have a way to dynamically tell if it on a
> debug-board powered device or a DCIN powered device, We are required to
> add this new DT property.

No, you are not. It's just a solution that you are proposing. One of a
plenty. Please start by describing the problem:

Device BigVendor some-EVK v1.23 has a hardware flow, the soldered in
resistance makes ADSP firmware emulate a fake battery rather than
completely ignoring the battery when reporting PSY properties. This is
confusing for the users of that EVK, which are assumed to be not able to
resolder 0203-size resistance, etc.

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v2 net-next 1/3] dt-bindings: vendor-prefixes: add SiTime Corporation
From: Ali Rouhi @ 2026-05-21 23:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: jiri, vadim.fedorenko, arkadiusz.kubalewski, robh, krzk+dt,
	conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree, netdev,
	linux-kernel, Ali Rouhi
In-Reply-To: <fa73a8fd-ee73-4a7b-b6c3-1d2edd73b45d@kernel.org>

On Thu, May 21, 2026 at 2:12 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> Do not top post.
>

Apologies, will inline-reply going forward.

Best regards,
Ali

^ permalink raw reply

* Re: [PATCH v2] dt-bindings: clock: via,vt8500: Convert to DT Schema
From: Rob Herring (Arm) @ 2026-05-21 23:32 UTC (permalink / raw)
  To: Udaya Kiran Challa
  Cc: linux-rtc, conor+dt, krzk+dt, mturquette, skhan, me, linux-kernel,
	sboyd, devicetree
In-Reply-To: <20260521170810.19702-1-challauday369@gmail.com>


On Thu, 21 May 2026 22:37:28 +0530, Udaya Kiran Challa wrote:
> Convert the VIA/Wondermedia VT8500 and Wondermedia WM8xxx series SoCs clock
> controller binding from the legacy text format to DT schema.
> 
> Signed-off-by: Udaya Kiran Challa <challauday369@gmail.com>
> ---
> Changelog:
> Changes since v1:
> - Add default value for divisor-mask
> - Add required properties compatible and model
> - Fix example node name
> - Update example size cells and reg value
> 
> Link to v1:https://lore.kernel.org/all/20260520025131.17772-1-challauday369@gmail.com/
> ---
>  .../bindings/clock/via,vt8500-clock.yaml      | 126 ++++++++++++++++++
>  .../devicetree/bindings/clock/vt8500.txt      |  74 ----------
>  2 files changed, 126 insertions(+), 74 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml
>  delete mode 100644 Documentation/devicetree/bindings/clock/vt8500.txt
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/clock/via,vt8500-clock.example.dtb: /: failed to match any schema with compatible: ['via,wm8650']

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260521170810.19702-1-challauday369@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: soc: qcom: Document CDSP Power Management
From: Bjorn Andersson @ 2026-05-21 23:29 UTC (permalink / raw)
  To: Vignesh Viswanathan
  Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown, linux-arm-msm, devicetree,
	linux-kernel
In-Reply-To: <20260520-cdsp-power-v1-1-85eb9501a1cd@oss.qualcomm.com>

On Wed, May 20, 2026 at 12:35:09AM +0530, Vignesh Viswanathan wrote:
> Add documentation for the CDSP Power Management driver, which handles

Your commit message should not describe an action, it should describe
the problem you're solving.

> Dynamic Clock and Voltage Scaling (DCVS) requests via SMEM, manages Low
> Power Mode (LPM) transitions via MPM handshake, and provides virtual
> regulators for the remoteproc driver to control CDSP power rails.
> 

You have a node describing the CDSP (remoteproc) already, but it doesn't
contain all the properties, so you're going to add this sibling node.

Why don't you describe the remoteproc properly instead?

> Signed-off-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
> ---
>  .../bindings/soc/qcom/qcom,cdsp-power.yaml         | 138 +++++++++++++++++++++
>  1 file changed, 138 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml
> new file mode 100644
> index 000000000000..f0f89fdeba4e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml
> @@ -0,0 +1,138 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/qcom/qcom,cdsp-power.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CDSP Power Management
> +
> +maintainers:
> +  - Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
> +
> +description:
> +  The CDSP Power Management driver provides power management services for the
> +  Qualcomm Compute DSP (CDSP) subsystem. It handles Dynamic Clock and Voltage
> +  Scaling (DCVS) requests via SMEM, manages Low Power Mode (LPM) transitions
> +  via MPM handshake, and provides virtual regulators that are consumed by the
> +  CDSP remoteproc driver.
> +
> +properties:
> +  compatible:
> +    const: qcom,cdsp-power
> +
> +  reg:
> +    items:
> +      - description: MPM (Modem Power Manager) register region
> +      - description: RSCC (RSC Configuration) register region
> +
> +  reg-names:
> +    items:
> +      - const: mpm
> +      - const: rscc
> +
> +  interrupts-extended:
> +    items:
> +      - description: LPM (Low Power Mode) interrupt from MPM
> +      - description: DCVS (Dynamic Clock and Voltage Scaling) interrupt from IPCC
> +
> +  interrupt-names:
> +    items:
> +      - const: lpm
> +      - const: dcvs
> +
> +  mboxes:
> +    maxItems: 1
> +    description: IPCC mailbox channel for sending DCVS responses to CDSP
> +
> +  qcom,smem-item:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      SMEM item ID used for DCVS communication channel between APSS and CDSP.
> +      This is a platform-specific value that identifies the shared memory region.
> +
> +  vdd-cx-supply:
> +    description:
> +      Phandle to the CX voltage regulator. This is the actual hardware regulator
> +      (e.g., from MP8899 PMIC) that supplies power to the CDSP CX rail.

This isn't the CX supply of the power management block, this is the CX
supply of the remoteproc - so put it there.

> +
> +  vdd-mx-supply:
> +    description:
> +      Phandle to the MX voltage regulator. This is the actual hardware regulator
> +      (e.g., from MP8899 PMIC) that supplies power to the CDSP MX rail. Optional
> +      on boards where MX rail is always-on or not present.
> +
> +  regulators:
> +    type: object
> +    description:
> +      Virtual regulators provided by this driver for consumption by the CDSP
> +      remoteproc driver. These virtual regulators pass through enable/disable
> +      requests to the actual hardware regulators (vdd-cx-supply, vdd-mx-supply).

These regulators doesn't exist in reality, they are only here because
you choose to split the description of your remoteproc implementation in
two.

> +
> +    properties:
> +      cdsp-vdd-cx:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        description: Virtual CX regulator for CDSP
> +        unevaluatedProperties: false
> +
> +      cdsp-vdd-mx:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        description: Virtual MX regulator for CDSP
> +        unevaluatedProperties: false
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts-extended
> +  - interrupt-names
> +  - mboxes
> +  - qcom,smem-item
> +  - vdd-cx-supply
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/mailbox/qcom-ipcc.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;

It's just an example, when you write bindings drop the 0x0 from base and
size in your reg and this need goes away.

> +
> +        cdsp_power: cdsp-power@4ae000 {
> +            compatible = "qcom,cdsp-power";
> +            reg = <0x0 0x004ae000 0x0 0x1000>,

For when you do this properly, please confirm that this is a dedicated
MPM register region and does not alias with any other nodes.

> +                  <0x0 0x26018018 0x0 0x4>;

No, we don't point reg = <> at a single register.

> +            reg-names = "mpm", "rscc";
> +
> +            interrupts-extended = <&intc GIC_SPI 65 IRQ_TYPE_EDGE_RISING 0>,

0?

> +                                  <&ipcc IPCC_CLIENT_CDSP
> +                                         IPCC_MPROC_SIGNAL_PING
> +                                         IRQ_TYPE_EDGE_RISING>;
> +            interrupt-names = "lpm", "dcvs";
> +
> +            mboxes = <&ipcc IPCC_CLIENT_CDSP IPCC_MPROC_SIGNAL_PING>;
> +
> +            qcom,smem-item = <503>;

Isn't this static for the given remoteproc?

Regards,
Bjorn

> +
> +            vdd-cx-supply = <&ipq9650_s2>;
> +            vdd-mx-supply = <&ipq9650_s4>;
> +
> +            regulators {
> +                cdsp_vdd_cx: cdsp-vdd-cx {
> +                    regulator-name = "cdsp-vdd-cx";
> +                };
> +
> +                cdsp_vdd_mx: cdsp-vdd-mx {
> +                    regulator-name = "cdsp-vdd-mx";
> +                };
> +            };
> +        };
> +    };
> 
> -- 
> 2.43.0
> 

^ permalink raw reply

* Re: [PATCH 0/2] qcom_battmgr: Add batteryless DC-adapter MAINS support
From: Bjorn Andersson @ 2026-05-21 22:56 UTC (permalink / raw)
  To: Rakesh Kota
  Cc: Konrad Dybcio, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sebastian Reichel, linux-arm-msm, devicetree,
	linux-kernel, linux-pm, jishnu.prakash, kamal.wadhwa
In-Reply-To: <20260519065938.4i5wot72pfxy4m3d@hu-kotarake-hyd.qualcomm.com>

On Tue, May 19, 2026 at 12:29:38PM +0530, Rakesh Kota wrote:
> On Mon, May 18, 2026 at 04:36:32PM +0200, Konrad Dybcio wrote:
> > On 5/18/26 3:49 PM, Rakesh Kota wrote:
> > > On batteryless boards powered by 12V DC adapters, registering the
> > > power supply as BATTERY causes userspace to incorrectly trigger
> > > battery power-saving sequences.
> > 
> > Does battman really offer no way of differentiating whether a battery
> > is *actually* present in such cases?
> >
> > What boards are affected?
> >
> Currently, batteryless support is only implemented for the
> qcs6490-rb3gen2(Kodiak) board.
> 

What do you mean?

Are you saying that the pmic_glink firmware in Kodiak has a one-off hack
that no other implementation of this firmware has?

My Lenovo IdeaCentre (hamoa) doesn't have battery, what should I do now?

Regards,
Bjorn

> regards
> Rakesh
> > Konrad

^ permalink raw reply

* Re: [PATCH] arm64: dts: qcom: glymur: Configure DP endpoints for 2-lane operation
From: Bjorn Andersson @ 2026-05-21 22:25 UTC (permalink / raw)
  To: Saurabh Anand
  Cc: konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, quic_riteshk, quic_vproddut, quic_mkuntuma,
	mahadevan.p
In-Reply-To: <20260521120058.2966709-1-saurabh.anand@oss.qualcomm.com>

On Thu, May 21, 2026 at 05:30:58PM +0530, Saurabh Anand wrote:
> Add explicit data-lanes to the MDSS DP output endpoints to enable

As described in
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
this is where you describe the problem that you're solving.

If you had done that, we wouldn't need to guess what you're up to.

Thanks,
Bjorn

> display port in 2 lanes configuration and disable the mode-switch
> property from the USB QMP PHY node.
> 
> Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/glymur.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index 72c7dc6e4f09..d6b3c5bfebd5 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
> @@ -2445,7 +2445,6 @@ usb_0_qmpphy: phy@fd5000 {
>  			#clock-cells = <1>;
>  			#phy-cells = <1>;
>  
> -			mode-switch;
>  			orientation-switch;
>  
>  			status = "disabled";
> @@ -2517,7 +2516,6 @@ usb_1_qmpphy: phy@fde000 {
>  			#clock-cells = <1>;
>  			#phy-cells = <1>;
>  
> -			mode-switch;
>  			orientation-switch;
>  
>  			status = "disabled";
> @@ -4550,6 +4548,7 @@ port@1 {
>  						reg = <1>;
>  
>  						mdss_dp0_out: endpoint {
> +							data-lanes = <0 1>;
>  							remote-endpoint = <&usb_dp_qmpphy_dp_in>;
>  						};
>  					};
> @@ -4641,6 +4640,7 @@ port@1 {
>  						reg = <1>;
>  
>  						mdss_dp1_out: endpoint {
> +							data-lanes = <0 1>;
>  							remote-endpoint = <&usb_1_qmpphy_dp_in>;
>  						};
>  					};
> -- 
> 2.34.1
> 

^ permalink raw reply

* Re: [PATCH v2 net-next 1/3] dt-bindings: vendor-prefixes: add SiTime Corporation
From: Krzysztof Kozlowski @ 2026-05-21 21:12 UTC (permalink / raw)
  To: Ali Rouhi
  Cc: jiri, vadim.fedorenko, arkadiusz.kubalewski, robh, krzk+dt,
	conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree, netdev,
	linux-kernel, Ali Rouhi
In-Reply-To: <CALFSGuoWkHYmtrouvLk2M7bWS1=qTSPUn5GuabFvK92cPBfuZA@mail.gmail.com>

On 21/05/2026 22:40, Ali Rouhi wrote:
>> Mismatch in From/DCO.
> 
> Acknowledged — Gmail SMTP relay issue, will fix for v3.

Do not top post.

> 
>> It looks like you received a tag and forgot to add it.
> 
> Thank you for the reminder.  Will carry Conor's tag in v3:
> 
>   Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Best regards,
> Ali
> 
> On Thu, May 21, 2026 at 3:18 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Wed, May 20, 2026 at 12:19:41PM -0700, Ali Rouhi wrote:
>>> Add vendor prefix for SiTime Corporation, manufacturer of
>>> programmable clock generators and MEMS oscillators.
>>>
>>> Signed-off-by: Ali Rouhi <arouhi@sitime.com>
>>
>> Mismatch in From/DCO.
>>
>> Best regards,
>> Krzysztof
>>


Best regards,
Krzysztof

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox