Devicetree
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Enable the QMP PCIe PHY present in Qualcomm ipq5210 SoC
From: Varadarajan Narayanan @ 2026-06-16  5:04 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel,
	Varadarajan Narayanan, Dmitry Baryshkov

Document the bindings and update the driver to support
the PCIe phy present in Qualcomm ipq5210 SoC.

v4: Fix commit message for the bindings patch by removing redundant content
    and adding explanation for using specific compatible.

v3: https://lore.kernel.org/linux-arm-msm/20260610-pcie-phy-v3-0-334011b378d6@oss.qualcomm.com/
    Fix commit message for the bindings patch
    Remove unused tables from the phy driver (ipq5210_gen3x1_pcie_ep_tx_tbl
    and ipq5210_gen3x1_pcie_ep_pcs_tbl)

v2: https://lore.kernel.org/r/20260609-pcie-phy-v2-0-83bc80e79fa6@oss.qualcomm.com
    Had incorrectly made both the phys as fallback. The single
    lane phy is standalone and double lane uses ipq9574 as
    fallback.

v1: https://lore.kernel.org/linux-arm-msm/20260514-pci-phy-v1-0-482429192746@oss.qualcomm.com/

Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
Varadarajan Narayanan (2):
      dt-bindings: phy: qcom,ipq8074-qmp-pcie: Document the ipq5210 QMP PCIe PHY
      phy: qcom-qmp-pcie: Add support for ipq5210 PCIe phys

 .../bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml    |   2 +
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 129 +++++++++++++++++++++
 2 files changed, 131 insertions(+)
---
base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72
change-id: 20260609-pcie-phy-99fcf91a02fd
prerequisite-change-id: 20260514-icc-ipq5210-0ab03f3a3e83:v1
prerequisite-patch-id: 0b6145b6635b18fe79fbbff5815041b43778c5ed
prerequisite-patch-id: 924c6ff7baf4283ac7991ee94c803a00fc5cece4
prerequisite-patch-id: c2fe1800fe769dccd37f94c19860a07f979e3c4c

Best regards,
-- 
Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>


^ permalink raw reply

* Re: [PATCH 1/2] bindings: power: supply: qcom,pmic-glink: Document thermal-mitigation
From: Krzysztof Kozlowski @ 2026-06-16  4:35 UTC (permalink / raw)
  To: Dhruvin Rajpura, Daniel Lezcano, Zhang Rui, Lukasz Luba
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sebastian Reichel, linux-arm-msm, devicetree,
	linux-kernel, linux-pm, kamal.wadhwa, jishnu.prakash,
	Dhruvin Rajpura
In-Reply-To: <CAB8MRjTwN6J3oSFVeF-w7WpZQamEyyQ6Ckyd=TAB=-N22b8k1g@mail.gmail.com>

On 15/06/2026 12:46, Dhruvin Rajpura wrote:
> On Wed, Jun 10, 2026 at 2:34 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>>
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    description:
>>> +      Array of fast charge current limit values for different system
>> thermal
>>> +      mitigation levels. This should be a flat array that denotes the
>> maximum
>>> +      charging current (in uA) for each thermal level. Elements should
>> be listed
>>> +      in monotonically decreasing (non-increasing) order.
>>
>> What is a thermal level? How do you define it? How does it map to
>> thermal bindings?
>>
> 
> A thermal level corresponds to a cooling state in the Linux
> thermal framework. The driver registers a thermal cooling device
> with N states, where state 0 represents no throttling (hardware
> maximum FCC queried from firmware via BATT_CHG_CTRL_LIM_MAX)
> and states 1..N map to the array entries in decreasing current
> order.
> 
> When a thermal zone trips, the thermal framework calls
> set_cur_state(N) which sends the corresponding current value
> to the firmware via BATT_CHG_CTRL_LIM over PMIC GLink,
> limiting the battery charging current to reduce heat generation.
> 
> The array must be monotonically decreasing since higher cooling
> states represent more aggressive throttling requiring lower
> charging currents.
> 
> Will add this explanation to the binding patch commit message
> in the next version.


+Cc Daniel, Zhang and Lukasz,

This feels like broader problem, so should not be done only in this one
aspect for Qualcomm device. IMO, there should be a generic binding for
defining charging constraints and mapping them to thermal zones. That's
not only about current, but might be about voltage or charging level
speed (consider quick charging with lower amps but higher voltage).  Or
actually power is the factor here, not even current and voltage.

This should be solved in generic way. Both from the point of charger's
OPP-like data but also cooling cells for the charger.

One more thing:
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v8 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys
From: Dmitry Torokhov @ 2026-06-16  4:26 UTC (permalink / raw)
  To: Svyatoslav Ryhel
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
	Pavel Machek, Sebastian Reichel, Ion Agorria,
	Michał Mirosław, devicetree, linux-kernel, linux-input,
	linux-leds, linux-pm
In-Reply-To: <20260528053203.9339-5-clamor95@gmail.com>

Hi Svyatoslav,

On Thu, May 28, 2026 at 08:32:00AM +0300, Svyatoslav Ryhel wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Add support for multimedia top button row of ASUS Transformer's Mobile
> Dock keyboard. Driver is made that function keys (F1-F12) are used by
> default which suits average Linux use better and with pressing
> ScreenLock + AltGr function keys layout is switched to multimedia keys.
> Since this only modifies codes sent by asus-ec-keys it doesn't affect
> normal keyboards at all.

I think using input handler to intercept ScreenLock + AltGr is quite
awkward. I think this also passes the original key events (unless you
make it a filter not a regular handler).

I do not see benefit for reacting to AltGr+ScreenLock on other keyboards
to activate the special mode on this one. So given the fact that you
already mange the data stream when you split it into "serio" ports,
maybe just intercept this key combo right there and create the input
device and signal input events right there?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 1/8] dt-bindings: remoteproc: qcom,pas: add thermal mitigation properties
From: Krzysztof Kozlowski @ 2026-06-16  4:21 UTC (permalink / raw)
  To: Daniel Lezcano, Gaurav Kohli
  Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Amit Kucheria,
	Manivannan Sadhasivam, Konrad Dybcio, Kees Cook,
	Gustavo A. R. Silva, cros-qcom-dts-watchers, linux-arm-msm,
	linux-remoteproc, devicetree, linux-kernel, linux-pm,
	linux-hardening, Manaf Meethalavalappu Pallikunhi
In-Reply-To: <9a31bb29-75d7-42fa-b8a8-4155cf85cadf@oss.qualcomm.com>

On 15/06/2026 14:30, Daniel Lezcano wrote:
> Hi Gaurav,
>>>> thanks for review, shall i use driver data, which is basically pas 
>>>> data structure like below:
>>>>
>>>> static const struct qcom_pas_data {
>>>>      .crash_reason_smem = 601,
>>>>      .firmware_name = "cdsp.mdt",
>>>>      .tmd_names = (const char *[]){"xyz", NULL},
>>>>      .num_tmds = 1,
>>>>
>>>> Is something like above acceptable? and this will also help to filter 
>>>> tmd names as well?
>>>
>>>
>>> How the thermal framework will bind the thermal zone with the TMD ? 
>>> (node pointer, id) ?
>>>
>>
>> Hi Daniel,
>>
>> thanks for review.
>>
>> With id only, in this case instead of taking tmd names from device tree, 
>> qmi_tmd will take tmd name from pas_data(driver) and register with the 
>> cooling framework with id only. Please let us know if this looks fine.
> May be I'm missing something but:
> 
>   - The QMI TMD returns a list of names, not ids
>   - The QMI TMD may return the list in different order than assumed
>   - The cooling map index points to the name of the TMD in the DT
>   - This name is used to match the name in the aformentionned list
>   - The index in the list and the id in the DT can differ
> 
> Krzysztof , I don't get why having the TMD names as properties is wrong, 
> they describes the existing TMDs on the system and the cooling maps 
> index points to the one to be connected with thermal zone.


'xxx-names' have a fixed meaning in DT by convention - assign
identifiable strings to the 'xxx'. I miss the property 'tmd' in such
case - its definition and meaning. Where is it?

But maybe you just want list of strings, so I am open to discuss it - I
don't understand the need for this property and commit and property
description tell me nothing.

Really, this commit message is basically non-existing. It explains what
it did and provides that much explanation WHY:

"- tmd-names (thermal mitigation device names)"

Really? This is the explanation why this change is being made, why this
property is needed?

So sure, describe the problem being solved and WHY this problem is being
solved that way. Maybe it will fit DT.


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v5 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Dmitry Torokhov @ 2026-06-16  4:16 UTC (permalink / raw)
  To: Svyatoslav Ryhel
  Cc: linux-input, devicetree, linux-kernel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Linus Walleij
In-Reply-To: <CAPVz0n3iCSeT3xJ2XkwZ6PYofwSLkc0gfm+iYo4xbKBkAtihcQ@mail.gmail.com>

Hi Svyatoslav,

On Mon, Jun 15, 2026 at 09:19:27AM +0300, Svyatoslav Ryhel wrote:
> чт, 28 трав. 2026 р. о 08:38 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> >
> > вт, 12 трав. 2026 р. о 13:24 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> > >
> > > From: Linus Walleij <linusw@kernel.org>
> > >
> > > The ISA1200 is a haptic feedback unit from Imagis Technology using two
> > > motors for haptic feedback in mobile phones. Used in many mobile devices
> > > c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> > > GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
> > >
> > > The exact datasheet for the ISA1200 is not available; all data was modeled
> > > based on available downstream kernel sources for various devices and
> > > fragments of information scattered across the internet.
> > >
> > > Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
> > > Signed-off-by: Linus Walleij <linusw@kernel.org>
> > > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/input/misc/Kconfig   |  12 +
> > >  drivers/input/misc/Makefile  |   1 +
> > >  drivers/input/misc/isa1200.c | 524 +++++++++++++++++++++++++++++++++++
> > >  3 files changed, 537 insertions(+)
> > >  create mode 100644 drivers/input/misc/isa1200.c
> > >
> >
> > Hello Dmitry! Do I need to make any further adjustments to this driver?
> 
> Hello Dmitry! Do I need to make any further adjustments to this
> driver? This driver is hanging in LKML for some time already without
> responds from input maintainer. It is still relevant and I would like
> it to move forward.

There were valid sashiko comments on the patch regarding resetting
"level" to 0 and also potential racing conditions, as well as suggestion
to check number of gpios specified in the device tree.

Please see if the following works for you:

diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
index ff82252a08e1..c61adc4b605c 100644
--- a/drivers/input/misc/isa1200.c
+++ b/drivers/input/misc/isa1200.c
@@ -131,6 +131,7 @@ struct isa1200 {
 	struct work_struct play_work;
 	struct isa1200_config config;
 
+	bool suspended;
 	bool active;
 	int level;
 };
@@ -247,17 +248,21 @@ static void isa1200_stop(struct isa1200 *isa)
 			       isa->supplies);
 
 	isa->active = false;
-	isa->level = 0;
 }
 
 static void isa1200_play_work(struct work_struct *work)
 {
 	struct isa1200 *isa = container_of(work, struct isa1200, play_work);
-
-	if (isa->level)
-		isa1200_start(isa);
-	else
-		isa1200_stop(isa);
+	struct input_dev *input = isa->input;
+
+	scoped_guard(mutex_try, &input->mutex) {
+		if (!isa->suspended) {
+			if (isa->level)
+				isa1200_start(isa);
+			else
+				isa1200_stop(isa);
+		}
+	}
 }
 
 static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
@@ -280,7 +285,8 @@ static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
 
 	if (isa->level != level) {
 		isa->level = level;
-		schedule_work(&isa->play_work);
+		if (!READ_ONCE(isa->suspended))
+			schedule_work(&isa->play_work);
 	}
 
 	return 0;
@@ -292,6 +298,7 @@ static void isa1200_vibrator_close(struct input_dev *input)
 
 	cancel_work_sync(&isa->play_work);
 	isa1200_stop(isa);
+	isa->level = 0;
 }
 
 static int isa1200_of_probe(struct i2c_client *client)
@@ -331,6 +338,9 @@ static int isa1200_of_probe(struct i2c_client *client)
 		return dev_err_probe(dev, PTR_ERR(isa->enable_gpios),
 				     "failed to get enable gpios\n");
 
+	if (isa->enable_gpios && isa->enable_gpios->ndescs > ISA1200_EN_PINS_MAX)
+		return dev_err_probe(dev, -EINVAL, "too many enable gpios\n");
+
 	ldo_node = device_get_named_child_node(dev, "ldo");
 	if (!ldo_node)
 		return dev_err_probe(dev, -ENODEV,
@@ -479,9 +489,9 @@ static int isa1200_suspend(struct device *dev)
 	guard(mutex)(&isa->input->mutex);
 
 	if (input_device_enabled(isa->input)) {
+		WRITE_ONCE(isa->suspended, true);
 		cancel_work_sync(&isa->play_work);
-		if (isa->level)
-			isa1200_stop(isa);
+		isa1200_stop(isa);
 	}
 
 	return 0;
@@ -493,9 +503,11 @@ static int isa1200_resume(struct device *dev)
 
 	guard(mutex)(&isa->input->mutex);
 
-	if (input_device_enabled(isa->input))
+	if (input_device_enabled(isa->input)) {
+		WRITE_ONCE(isa->suspended, false);
 		if (isa->level)
-			isa1200_start(isa);
+			schedule_work(&isa->play_work);
+	}
 
 	return 0;
 }

-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH 2/3] dt-bindings: mfd: s2mu005-pmic: drop compatible property for multi-led node
From: Krzysztof Kozlowski @ 2026-06-16  4:15 UTC (permalink / raw)
  To: Kaustabh Chakraborty, André Draszik, Lee Jones, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Pavel Machek
  Cc: linux-kernel, linux-samsung-soc, devicetree, linux-leds
In-Reply-To: <20260616-s2mu005-pmic-supplement-v1-2-41e84518b711@disroot.org>

On 15/06/2026 22:26, Kaustabh Chakraborty wrote:
> The multi-led node is very trivial in description and also has no
> sub-nodes. A compatible string property for such nodes is not preferred
> by upstream. Remove said node from the schema. While at it, also add a
> description following its other sibling nodes.
> 
> Link: https://lore.kernel.org/all/d2f4cb7d-5c3e-4b9a-86ca-04262cbb9775@kernel.org
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  .../devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml       | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml b/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml
> index 8354422e39b1e..f62fe7a05147e 100644
> --- a/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml
> @@ -38,17 +38,10 @@ properties:
>        Child node describing MUIC device.
>  
>    multi-led:
> -    type: object
> +    $ref: /schemas/leds/leds-class-multicolor.yaml#
>  
> -    allOf:
> -      - $ref: /schemas/leds/leds-class-multicolor.yaml#
> -
> -    properties:
> -      compatible:
> -        const: samsung,s2mu005-rgb

It's already accepted and used in two drivers, leave it.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 1/3] dt-bindings: mfd: s2mu005-pmic: reorder reg and interrupts properties
From: Krzysztof Kozlowski @ 2026-06-16  4:14 UTC (permalink / raw)
  To: Kaustabh Chakraborty, André Draszik, Lee Jones, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Pavel Machek
  Cc: linux-kernel, linux-samsung-soc, devicetree, linux-leds
In-Reply-To: <20260616-s2mu005-pmic-supplement-v1-1-41e84518b711@disroot.org>

On 15/06/2026 22:26, Kaustabh Chakraborty wrote:
> As per convention, and as also reiterated by maintainers [1], the
> properties in schema is to be ordered similar to how its done in
> devicetree sources; starting from compatible and reg. Re-order the
> properties in this schema accordingly.
> 
> Link: https://lore.kernel.org/all/0240eb13-6c56-4879-8db7-b990a220a78f@kernel.org [1]
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>

Honestly, nah... I commented on v6 so you change the patch. But you were
posting this huge patchset faster than we can review (v6 and v7 posted
on the same day!), so v7 got applied where you did not implement the
comments. One small posting per 24h. One big posting per 2-3 days, not
more often.

There is little benefit in fixing this single file.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH] of: property: Fix of_fwnode_get_reference_args() with negative index
From: Krzysztof Kozlowski @ 2026-06-16  4:01 UTC (permalink / raw)
  To: Alban Bedel
  Cc: devicetree, Rob Herring, Saravana Kannan, driver-core,
	linux-kernel, Tommaso Merciai
In-Reply-To: <20260615134747.3cdb9599@OMT-CWNXR4TFW5-LHT>

On 15/06/2026 13:47, Alban Bedel wrote:
> On Mon, 15 Jun 2026 11:54:01 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> Indices larger than INT_MAX are valid in the fwnode API, so returning
>>> -EINVAL is not appropriate here.
>>>   
>>
>> Then neither ENOENT are.
>>
>> But really, EINVAL is correct here. This is OF implementation, so this
>> implementation decides what is EINVAL and what is right. Not fwnode API.
> 
> I think there is a missunderstanding here. The function we are talking
> about, of_fwnode_get_reference_args(), is the OF backed implementation
> of fwnode_property_get_reference_args(). As such it must follow the API
> documented by fwnode_property_get_reference_args() which list the
> following return values:
> 
>  * Return: %0 on success
>  *	    %-ENOENT when the index is out of bounds, the index has an empty
>  *		     reference or the property was not found
>  *	    %-EINVAL on parse error
>  *	    %-ENOTCONN when the remote firmware node exists but has not been
>  *		       registered yet
> 
> It is not explicitly documented what should be returned if the index is
> not representable in the backend, but considering it out of bound seems
> to be the most sensible thing to do.
> 

I agree, I did not consider the docs for
fwnode_property_get_reference_args().

Please send v2 fixing the comment style and with:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v9 1/2] dt-bindings: ufs: Document static TX Equalization settings properties
From: Krzysztof Kozlowski @ 2026-06-16  3:54 UTC (permalink / raw)
  To: Can Guo, bvanassche, beanhuo, peter.wang, martin.petersen, mani
  Cc: linux-scsi, Alim Akhtar, Avri Altman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Ram Kumar Dwivedi,
	Zhaoming Luo,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <20260615132834.2985346-2-can.guo@oss.qualcomm.com>

On 15/06/2026 15:28, Can Guo wrote:
> UFS v5.0/UFSHCI v5.0 adds HS-G6 support (46.6 Gbps/lane) via UniPro
> v3.0 and M-PHY v6.0. These specs define TX Equalization for all
> High-Speed Gears (not only HS-G6) to compensate channel loss and
> improve signal integrity at high speed.
> 
> For HS-G6, M-PHY uses PAM4 1b1b line coding. Pre-Coding may also be
> required depending on channel characteristics.
> 
> Document vendor-neutral properties in ufs-common.yaml:
> - txeq-preshoot-g[1-6]
> - txeq-deemphasis-g[1-6]
> - tx-precode-enable-g6
> 
> Values are per-lane Host/Device tuples (2 values for x1, 4 values for
> x2). PreShoot/DeEmphasis range from 0..7, and Precode is 0/1.
> 
> These are board-specific signal-integrity tuning values. They depend on
> channel SI/PHY characterization and validation (host PHY, device PHY,
> package, and board routing), and are determined by HW/PHY designers.
> 
> Although UFSHCI v5.0 supports TX Equalization Training via UniPro v3.0,
> which allows host software to determine optimal TX Equalization at
> runtime, static board-specific TX Equalization settings in the Device
> Tree are still necessary because:
> - TX Equalization Training is not supported for HS-G3 and below
> - TX Equalization Training is disabled on some platforms
> 
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/ufs/ufs-common.yaml   | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v2 7/8] dt-bindings: display: allwinner: Split H616 DE33 layer reg space
From: Krzysztof Kozlowski @ 2026-06-16  3:51 UTC (permalink / raw)
  To: Jernej Škrabec, wens
  Cc: samuel, mripard, maarten.lankhorst, tzimmermann, airlied, simona,
	robh, krzk+dt, conor+dt, mturquette, sboyd, dri-devel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel, linux-clk
In-Reply-To: <0r4us4OeRRWtJhxvps-bZw@gmail.com>

On 15/06/2026 17:47, Jernej Škrabec wrote:
> Dne ponedeljek, 15. junij 2026 ob 06:28:54 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
>> On 14/06/2026 16:08, Jernej Škrabec wrote:
>>> Dne ponedeljek, 25. maj 2026 ob 14:10:38 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
>>>> On 24/05/2026 23:33, Chen-Yu Tsai wrote:
>>>>> Hi,
>>>>>
>>>>> (resent from new email)
>>>>>
>>>>> On Thu, May 14, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>>
>>>>>> On Sat, May 09, 2026 at 09:00:14PM +0200, Jernej Skrabec wrote:
>>>>>>> From: Jernej Skrabec <jernej.skrabec@gmail.com>
>>>>>>>
>>>>>>> As it turns out, current H616 DE33 binding was written based on
>>>>>>> incomplete understanding of DE33 design. Namely, planes are shared
>>>>>>> resource and not tied to specific mixer, which was the case for previous
>>>>>>> generations of Display Engine (DE3 and earlier).
>>>>>>>
>>>>>>> This means that current DE33 binding doesn't properly reflect HW and
>>>>>>> using it would mean that second mixer (used for second display output)
>>>>>>> can't be supported.
>>>>>>>
>>>>>>> Remove layer register space, which will be represented with additional
>>>>>>> node, and replace it with phandle, which will point to that new, shared
>>>>>>> node. That way, all mixers can share same layers.
>>>>>>>
>>>>>>> There is no user of this binding yet, so changes can be made safely,
>>>>>>> without breaking any backward compatibility.
>>>>>>
>>>>>> There is user. git grep gives me:
>>>>>> drivers/gpu/drm/sun4i/sun8i_mixer.c
>>>>>>
>>>>>> which means this is a released ABI. As I understood, the old code was
>>>>>
>>>>> We held off on merging the DT changes so that we could rework this.
>>>>> I can't find the actual request though. It was probably over IRC.
>>>>>
>>>>>> working fine but just did not support all use cases. Why this cannot be
>>>>>> kept backwards compatible?
>>>>>
>>>>> AFAIK the "planes" block is shared between two display mixers. As the
>>>>> commit message explains, this prevents using the second mixer, since
>>>>> only one of them can claim and map the register space. And on the H700
>>>>> (which is the same die as the H616 discussed here but with more exposed
>>>>> interfaces), there could actually be a use case for the second mixer.
>>>>
>>>> It explains why you want to make the changes but not why you cannot keep
>>>> it backwards compatible.
>>>
>>> I guess it can be backward compatible, but I don't think it makes sense.
>>> Yes, original driver implemented original DT bindings, but there is no node
>>> which uses that binding. If there is no user of that, why would driver
>>
>> Did you check all out of tree users of the ABI? All vendor kernels,
>> forks and all of them for which the ABI was made for?
> 
> Since when do we care about out of tree users? I understand that drivers

Since always? That is the meaning of ABI. Otherwise there is no point to
discuss ABI at all. Why would it exist if you had all DTS inside kernel
always matching the code?


> must support old device tree files. Once they work, compatibility must
> be carried forward. But that's not the case here.
> 
> In any case, vendor kernels have completely different DT structure. This
> was developed independently from them. Take a look at [1] how BSP DT looks
> like, specifically Display Engine node.
> 
> Of course there are some distros which grab WIP patches from mailing lists
> soon after they are available. For example, I know that Armbian carried old
> WIP patches which used old ABI. However, such distros generally don't care
> about exact solution and ditch patches as soon as proper solution is merged
> upstream or even when better WIP patches come around. DT files in such
> distros get updated alongside kernel, they are not hidden in firmware. 
> 

I am not talking about BSP. I am talking about out of tree users for
which we defined the ABI and called it that way.


Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH] arm64: dts: qcom: install DT overlays via dtbs_install
From: Krzysztof Kozlowski @ 2026-06-16  3:49 UTC (permalink / raw)
  To: Vishwas Udupa
  Cc: andersson, conor+dt, devicetree, kbajaj, konradybcio, krzk+dt,
	linux-arm-msm, robh, snb, vudupa
In-Reply-To: <20260615162739.787779-1-vishwas.udupa@oss.qualcomm.com>

On 15/06/2026 18:27, Vishwas Udupa wrote:
> EL2 DTBOs are used at build time to construct DTBs corresponding to
> an EL2 (hypervisor-enabled) boot configuration. These DTBs are included in
> distributions [1] as complete boot configurations (e.g. EL1 and EL2).
> 
> The EL2 configuration is not enabled by default and is typically selected
> after the initial boot by updating a UEFI runtime variable from userspace.
> Once set, firmware selects the prebuilt EL2 DTB on subsequent boots.
> 
> Although EL2 DTBOs are not used directly at runtime during initial boot,
> they are required to generate and package the EL2 DTBs in the image so that
> firmware can switch to EL2 when the configuration variable is enabled. Hence, el2 dtbo's
> need to be retained.
> 
> 1: https://github.com/qualcomm-linux/qcom-dtb-metadata/blob/main/qcom-next-fitimage.its#L273

You cut entire context making any discussion very difficult.

Anyway, comment stays. EL2 is not an overlay. If I need to repeat
myself, then let's do like:

NAK

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH 1/2] dt-bindings: pinctrl: aspeed,ast2700-soc1: Add JTAGM1TRST group
From: Billy Tsai @ 2026-06-16  3:30 UTC (permalink / raw)
  To: Andrew Jeffery, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Joel Stanley
  Cc: linux-aspeed, openbmc, linux-gpio, devicetree, linux-arm-kernel,
	linux-kernel, Billy Tsai
In-Reply-To: <20260616-pinctrl-fix-v1-0-621036e45c7c@aspeedtech.com>

The TRST signal of the JTAG master is optional and may not be wired on
every design, but it is only selectable as part of the JTAGM1 group,
which forces the D12 ball to be muxed whenever the JTAG master is used.

Add a separate JTAGM1TRST group so boards can enable TRST independently
of the other JTAG master signals.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 .../devicetree/bindings/pinctrl/aspeed,ast2700-soc1-pinctrl.yaml         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2700-soc1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2700-soc1-pinctrl.yaml
index 76944fd14e2c..fe7cef4fef6a 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2700-soc1-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2700-soc1-pinctrl.yaml
@@ -356,6 +356,7 @@ patternProperties:
           - I3C8
           - I3C9
           - JTAGM1
+          - JTAGM1TRST
           - LPC0
           - LPC1
           - LTPI

-- 
2.34.1


^ permalink raw reply related

* [PATCH 2/2] pinctrl: aspeed: Split TRST out of the AST2700 SoC1 JTAGM1 group
From: Billy Tsai @ 2026-06-16  3:30 UTC (permalink / raw)
  To: Andrew Jeffery, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Joel Stanley
  Cc: linux-aspeed, openbmc, linux-gpio, devicetree, linux-arm-kernel,
	linux-kernel, Billy Tsai
In-Reply-To: <20260616-pinctrl-fix-v1-0-621036e45c7c@aspeedtech.com>

The JTAGM1 group includes the D12 ball carrying the TRST signal, but
TRST is optional for a JTAG master and the ball may be needed for other
functions on designs that do not wire it. With TRST embedded in the
group, such designs cannot use the JTAG master at all.

Move D12 into a new JTAGM1TRST group under the same JTAGM1 function so
TRST is muxed only when a board requests it. Boards that do use TRST
now need to select both the JTAGM1 and JTAGM1TRST groups.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c
index 50027d69c342..f8b4066699ce 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c
@@ -1018,7 +1018,8 @@ PIN_GROUP(I3C6, AA22, AB20);
 PIN_GROUP(I3C7, AF18, AE19);
 PIN_GROUP(I3C8, AD20, AC20);
 PIN_GROUP(I3C9, AA21, AB21);
-PIN_GROUP(JTAGM1, D12, F10, E11, F11, F13);
+PIN_GROUP(JTAGM1, F10, E11, F11, F13);
+PIN_GROUP(JTAGM1TRST, D12);
 PIN_GROUP(LPC0, AF26, AF25, B16, D14, B15, B14, C17, B13, E14, C15);
 PIN_GROUP(LPC1, C16, C14, C11, D9, F14, D10, C12, C13, AE16, AE17);
 PIN_GROUP(LTPI, U25, U26, Y26, AA24);
@@ -1263,6 +1264,7 @@ static const struct pingroup aspeed_g7_soc1_groups[] = {
 	GROUP(I3C8),
 	GROUP(I3C9),
 	GROUP(JTAGM1),
+	GROUP(JTAGM1TRST),
 	GROUP(LPC0),
 	GROUP(LPC1),
 	GROUP(LTPI),
@@ -1528,7 +1530,7 @@ static const struct aspeed_g7_soc1_function aspeed_g7_soc1_functions[] = {
 	FUNC(I3C7, (1), "I3C7"),
 	FUNC(I3C8, (1), "I3C8"),
 	FUNC(I3C9, (1), "I3C9"),
-	FUNC(JTAGM1, (1), "JTAGM1"),
+	FUNC(JTAGM1, (1, 1), "JTAGM1", "JTAGM1TRST"),
 	FUNC(LPC0, (2), "LPC0"),
 	FUNC(LPC1, (2), "LPC1"),
 	FUNC(LTPI, (2), "LTPI"),

-- 
2.34.1


^ permalink raw reply related

* [PATCH 0/2] pinctrl: aspeed: Make AST2700 SoC1 JTAG master TRST optional
From: Billy Tsai @ 2026-06-16  3:30 UTC (permalink / raw)
  To: Andrew Jeffery, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Joel Stanley
  Cc: linux-aspeed, openbmc, linux-gpio, devicetree, linux-arm-kernel,
	linux-kernel, Billy Tsai

The JTAGM1 pin group of the AST2700 SoC1 includes ball D12, which
carries the TRST signal. TRST is an optional signal for a JTAG master:
designs that do not wire it may need the D12 ball for other functions,
but with TRST embedded in the group they cannot use the JTAG master at
all.

Split D12 into a new JTAGM1TRST group under the existing JTAGM1
function, so TRST is only muxed when a board explicitly requests it.
Patch 1 adds the new group to the device tree binding and patch 2
splits the group in the driver.

Note that this changes the meaning of the existing JTAGM1 group: boards
that do use TRST now need to select both the JTAGM1 and JTAGM1TRST
groups.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
Billy Tsai (2):
      dt-bindings: pinctrl: aspeed,ast2700-soc1: Add JTAGM1TRST group
      pinctrl: aspeed: Split TRST out of the AST2700 SoC1 JTAGM1 group

 .../devicetree/bindings/pinctrl/aspeed,ast2700-soc1-pinctrl.yaml    | 1 +
 drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c                     | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)
---
base-commit: 761af93c9f1a100b8d9f71aa744b8f9abbbbbfb2
change-id: 20260612-pinctrl-fix-1c1e7c37261c

Best regards,
-- 
Billy Tsai <billy_tsai@aspeedtech.com>


^ permalink raw reply

* Re: [PATCH 1/2] power: sequencing: pcie-m2: Add PCI ID 0x1103 for WCN6855 Bluetooth
From: Wei Deng @ 2026-06-16  3:03 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Manivannan Sadhasivam, Bartosz Golaszewski,
	linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-pm,
	quic_chezhou, cheng.jiang, shuai.zhang, jinwang.li, xiuzhuo.shang,
	mengshi.wu
In-Reply-To: <hndyeomouu754rwevoigc2ai4ywtz5lhodizj4amjuyn4azjhq@f6ixgilydtg2>

Hi Dmitry,

On Tue, Jun 16, 2026 at 03:26:40AM +0300, Dmitry Baryshkov wrote:
> On Mon, Jun 08, 2026 at 02:47:01PM +0530, Wei Deng wrote:
>> WCN6855 is a Qualcomm Wi-Fi/BT combo chip that uses PCI device ID
>> 0x1103. Add it to pwrseq_m2_pci_ids[] alongside the existing 0x1107
>> (WCN7850) entry, so that the pwrseq-pcie-m2 driver creates a Bluetooth
>> serdev device for WCN6855 cards inserted into PCIe M.2 Key E connectors.
>>
>> Signed-off-by: Wei Deng <wei.deng@oss.qualcomm.com>
>> ---
>>  drivers/power/sequencing/pwrseq-pcie-m2.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
>> index efeb25ba9c79..b3af14464314 100644
>> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
>> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
>> @@ -188,6 +188,8 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
>>  static const struct pci_device_id pwrseq_m2_pci_ids[] = {
>>  	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1107),
>>  	  .driver_data = (kernel_ulong_t)"qcom,wcn7850-bt" },
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1103),
>> +	  .driver_data = (kernel_ulong_t)"qcom,wcn6855-bt" },
>>  	{ } /* Sentinel */
>
> Please keep the list sorted. I saw that Bartosz has applied the patch.
> Would you please send a followup?

Thanks for the feedback. I have sent a followup patch to sort the list:

  <20260616025632.3697863-1-wei.deng@oss.qualcomm.com>

>>  };
>>
>> --
>> 2.34.1
>>

--
Best Regards,
Wei Deng

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: qcom: lemans-evk: Describe the PCIe M.2 Key E connector
From: Wei Deng @ 2026-06-16  3:00 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm,
	devicetree, linux-kernel, manivannan.sadhasivam,
	bartosz.golaszewski, quic_chezhou, cheng.jiang, shuai.zhang,
	jinwang.li, xiuzhuo.shang, mengshi.wu
In-Reply-To: <tspehw7yb6hrgek7rz6qghcoqr4v6cdpulbzpggii6qlmaatxk@gcb2tbb3qji2>

Hi Dmitry,

On Tue, Jun 16, 2026 at 03:28:11AM +0300, Dmitry Baryshkov wrote:
> On Mon, Jun 15, 2026 at 04:02:28PM +0530, Wei Deng wrote:
>> The lemans EVK has the PCIe M.2 Mechanical Key E connector to connect
>> wireless connectivity cards over PCIe and UART interfaces. Hence,
>> describe the connector node and link it with the PCIe 0 Root Port and
>> UART17 nodes through graph port/endpoint.
>>
>> Also add 'compatible = "pciclass,0604"' to the pcieport0 node in
>> lemans.dtsi to allow the PCI subsystem to associate the DT node with
>> the PCI-to-PCI bridge device.
>>
>> The M.2 Key E connector is powered by a 3.3V fixed regulator
>> (vreg_wcn_3p3) which is sourced from the board's 12V DC input rail
>> (vreg_dcin_12v). Both regulators are always-on and are required by the
>> pcie-m2-e-connector binding.
>>
>> Also add the serial1 = &uart17 alias, which is required for the
>> Bluetooth serdev device to be enumerated on the UART17 interface.
>>
>> Signed-off-by: Wei Deng <wei.deng@oss.qualcomm.com>
>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> ---
>> Changes in v2:
>> - Collected Reviewed-by tag and reorganized the patch
>
> Please don't send the patches as replies to the previous iterations.
> Each new revision should be in a separate thread.

Thanks for the feedback. I will make sure to start a new thread for
each new revision going forward.

>>
>> Link: https://lore.kernel.org/linux-arm-msm/20260608091702.3797437-2-wei.deng@oss.qualcomm.com/ [v1]
>>
>>  arch/arm64/boot/dts/qcom/lemans-evk.dts | 75 +++++++++++++++++++++++++
>>  arch/arm64/boot/dts/qcom/lemans.dtsi    |  1 +
>>  2 files changed, 76 insertions(+)
>>
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>
>
> --
> With best wishes
> Dmitry

--
Best Regards,
Wei Deng

^ permalink raw reply

* RE: [PATCH v6 1/3] dt-bindings: imx6q-pcie: Add optional intr/aer/pme interrupts for i.MX95
From: Hongxing Zhu @ 2026-06-16  2:56 UTC (permalink / raw)
  To: Rob Herring, Hongxing Zhu (OSS)
  Cc: krzk+dt@kernel.org, conor+dt@kernel.org, bhelgaas@google.com,
	Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-kernel@vger.kernel.org
In-Reply-To: <20260612151348.GA1040341-robh@kernel.org>

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Friday, June 12, 2026 11:14 PM
> To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> Cc: krzk+dt@kernel.org; conor+dt@kernel.org; bhelgaas@google.com; Frank Li
> <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> imx@lists.linux.dev; linux-kernel@vger.kernel.org; Hongxing Zhu
> <hongxing.zhu@nxp.com>
> Subject: Re: [PATCH v6 1/3] dt-bindings: imx6q-pcie: Add optional intr/aer/pme
> interrupts for i.MX95
> 
> On Wed, Jun 03, 2026 at 02:25:08PM +0800, hongxing.zhu@oss.nxp.com wrote:
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> >
> > The i.MX95 PCIe controller introduces three additional dedicated
> > hardware interrupt lines for specific events:
> > - intr: general controller events
> > - aer: Advanced Error Reporting events
> > - pme: Power Management Events
> >
> > These interrupts are optional on i.MX95. PCIe basic functionality
> > (enumeration, configuration, and data transfer) works correctly
> > without them, as the controller can operate using only the existing msi
> interrupt.
> >
> > Earlier i.MX PCIe variants (imx6q, imx6sx, imx6qp, imx7d, imx8mm,
> > imx8mp, imx8mq, imx8q) do not have these three dedicated interrupt lines.
> >
> > Update the binding to allow up to 5 interrupts for i.MX95, while
> > restricting earlier variants to a maximum of 2 interrupts using
> > conditional constraints (if/then schema). This ensures the schema
> > accurately reflects the hardware capabilities of each SoC variant.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../bindings/pci/fsl,imx6q-pcie.yaml          | 29 +++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > index e8b8131f5f23..9b5d4e59dfff 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > @@ -58,12 +58,18 @@ properties:
> >      items:
> >        - description: builtin MSI controller.
> >        - description: builtin DMA controller.
> > +      - description: PCIe event interrupt.
> > +      - description: builtin AER SPI standalone interrupt line.
> > +      - description: builtin PME SPI standalone interrupt line.
> >
> >    interrupt-names:
> >      minItems: 1
> >      items:
> >        - const: msi
> >        - const: dma
> > +      - const: intr
> > +      - const: aer
> > +      - const: pme
> >
> >    reset-gpio:
> >      deprecated: true
> > @@ -248,6 +254,29 @@ allOf:
> >              - const: pcie_aux
> >              - const: ref
> >              - const: extref  # Optional
> > +        interrupts:
> > +          maxItems: 5
> > +        interrupt-names:
> > +          maxItems: 5
> 
> 5 is already the max.
Thank you for the review.
You're correct. These maxItems constraints are redundant and will be removed
later.

Best Regards
Richard Zhu
> 
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,imx6q-pcie
> > +            - fsl,imx6sx-pcie
> > +            - fsl,imx6qp-pcie
> > +            - fsl,imx7d-pcie
> > +            - fsl,imx8mm-pcie
> > +            - fsl,imx8mp-pcie
> > +            - fsl,imx8mq-pcie
> > +            - fsl,imx8q-pcie
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 2
> > +        interrupt-names:
> > +          maxItems: 2
> >
> >  unevaluatedProperties: false
> >
> > --
> > 2.34.1
> >

^ permalink raw reply

* Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio irq property
From: Xianwei Zhao @ 2026-06-16  2:56 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	linux-amlogic, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260615-ultra-pagan-84de490070e1@spud>

Hi Conor,
    Thanks for your detailed review. I will drop this patch.

On 2026/6/16 00:52, Conor Dooley wrote:
> Subject:
> Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio 
> irq property
> From:
> Conor Dooley <conor@kernel.org>
> Date:
> 2026/6/16 00:52
> 
> To:
> xianwei.zhao@amlogic.com
> CC:
> Linus Walleij <linusw@kernel.org>, Rob Herring <robh@kernel.org>, 
> Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley 
> <conor+dt@kernel.org>, Neil Armstrong <neil.armstrong@linaro.org>, Kevin 
> Hilman <khilman@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>, 
> Martin Blumenstingl <martin.blumenstingl@googlemail.com>, 
> linux-amlogic@lists.infradead.org, linux-gpio@vger.kernel.org, 
> devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
> linux-arm-kernel@lists.infradead.org
> 
> 
> Given Linus' comments on the cover letter,
> pw-bot: changes-requested
> 
> Thanks,
> Conor.

^ permalink raw reply

* Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio irq property
From: Xianwei Zhao @ 2026-06-16  2:54 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Conor Dooley
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	linux-amlogic, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <542ec217-1187-4fb2-8fd3-b90a6143b84d@kernel.org>

Hi Krzysztof,
    Thanks for your detailed review. After considering the feedback, I 
think this approach is not suitable, so I will drop this patch.

On 2026/6/15 13:32, Krzysztof Kozlowski wrote:
> On 15/06/2026 04:47, Xianwei Zhao wrote:
>> Hi Conor,
>>      Thanks for your review.
>>
>> On 2026/6/12 01:39, Conor Dooley wrote:
>>> Subject:
>>> Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio
>>> irq property
>>> From:
>>> Conor Dooley<conor@kernel.org>
>>> Date:
>>> 2026/6/12 01:39
>>>
>>> To:
>>> xianwei.zhao@amlogic.com
>>> CC:
>>> Linus Walleij<linusw@kernel.org>, Rob Herring<robh@kernel.org>,
>>> Krzysztof Kozlowski<krzk+dt@kernel.org>, Conor Dooley
>>> <conor+dt@kernel.org>, Neil Armstrong<neil.armstrong@linaro.org>, Kevin
>>> Hilman<khilman@baylibre.com>, Jerome Brunet<jbrunet@baylibre.com>,
>>> Martin Blumenstingl<martin.blumenstingl@googlemail.com>,
>>> linux-amlogic@lists.infradead.org,linux-gpio@vger.kernel.org,
>>> devicetree@vger.kernel.org,linux-kernel@vger.kernel.org,
>>> linux-arm-kernel@lists.infradead.org
>>>
>>>
>>>
>>> On Thu, Jun 11, 2026 at 07:54:33AM +0000, Xianwei Zhao via B4 Relay wrote:
>>>> From: Xianwei Zhao<xianwei.zhao@amlogic.com>
>>>>
>>>> Add the hw-irq property for each GPIO bank and enable interrupt-parent
>>>> for pinctrl so that gpiod_to_irq() can translate GPIO lines to IRQs.
>>> Uhhhhh, what? Why can't you just use the normal interrupts property?
>>>
>> The interrupt cannot be used directly because the GPIO bank only
>> provides an IRQ base, which does not have a one-to-one mapping with the
>> actual hardware interrupts.
>>
>> On Amlogic SoCs, GPIO interrupts are handled through a mux. Multiple
>> GPIO pins are mapped to a limited number of real interrupt sources. The
>> implementation can be found here:
>>
>> https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-meson-gpio.c
>>
>> To use a GPIO interrupt, an unused hardware interrupt must first be
>> allocated, and then the corresponding mux register must be configured.
>> This allocation and mapping are already implemented in the existing driver.
>>
>> In that driver, the mapping is performed dynamically rather than simply
>> calculating:
>>
>> irq = irq_start + gpio_offset
>>
>> If the interrupt is used directly, only the GPIO index can be obtained.
> 
> If it is performed dynamically, then it is not suitable for DT.
> 
> You still did not explain what hardware aspect exactly is described by
> "hw-irq".
> 
> 
> 
>> The real interrupt number cannot be derived by simply adding an offset,
>> because the hardware interrupt must be allocated first. Pre-allocating
>> all interrupts during initialization would prevent later GPIOs from
>> obtaining available interrupt sources.
>>
>> Perhaps other names would be more appropriate here, such as "irq_start".
>>
>>>> Signed-off-by: Xianwei Zhao<xianwei.zhao@amlogic.com>
>>>> ---
> Best regards,
> Krzysztof

^ permalink raw reply

* Re: [PATCH RFC 0/2] pinctrl: Add support gpiod_to_irq
From: Xianwei Zhao @ 2026-06-16  2:45 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, linux-amlogic,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <CAD++jLm9+8RSbBCi-k=8S98XvVJ7taYrK=kuBp3_RqxE_bcxbg@mail.gmail.com>

Hi Linus,
    Understood. Thank you for your detailed explanation. I will drop 
this patch.

On 2026/6/15 20:59, Linus Walleij wrote:
>>> Hi Xianwei,
>>>
>>> thanks for your patches!
>>>
>>> On Thu, Jun 11, 2026 at 9:54 AM Xianwei Zhao via B4 Relay
>>> <devnull+xianwei.zhao.amlogic.com@kernel.org>   wrote:
>>>
>>>> Some users need to obtain an IRQ directly from a GPIO descriptor through gpiod_to_irq().
>>>> Add the required DT binding and implementation to support this use case.
>>>> Since this introduces a new DT property, the property is kept optional to
>>>> maintain compatibility with existing SoCs and DTS files.
>>> To me it looks like you have just re-implemented hierarchical
>>> irqs.
>>>
>>> Look into the section "Infrastructure helpers for GPIO irqchips"
>>> in Documentation/driver-api/gpio/driver.rst, especially towards
>>> the end.
>>>
>>> Solve this by using GPIOLIB_IRQCHIP and a custom
>>> child_to_parent_hwirq() callback to translate the GPIO into
>>> an IRQ.
>>>
>>> To just implement gpiod_to_irq() without any irqchip abstraction
>>> is also broken: you can't force all users to just use this way
>>> to get an IRQ it's excessively restricting.
>>>
>>> Add
>>>
>>>     interrupt-controller: true
>>>
>>>     "#interrupt-cells":
>>>       const: 2
>>>
>>> to the pinctrl node as well so that DT users can simply request
>>> the IRQ from the irqchip inside of the pin controller. It will
>>> be hierarchical and lightweight but an irqchip nevertheless.
>>>
>>> The GPIOLIB_IRQCHIP approach will help you to get this
>>> right.
>>>
>> I read the document (Documentation/driver-api/gpio/driver.rst) you
>> pointed me to and found that the corresponding implementation has
>> already been added in this file:
>>
>> https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-meson-gpio.c
> That is the parent interrupt controller to the pinctrl+gpio isn't it.
> 
> It will be even clearer once you use interrupts = <>; instead of
> the hwirq = <>; hack.
> 
>> However, it is implemented as a standalone irqchip and is not integrated
>> with the GPIO controller.
> Right, so it is the parent. Of course it it stand alone.
> 
>> In this patch, I implemented the GPIO-to-IRQ conversion through
>> gpiod_to_irq(). Users can still obtain the interrupt directly through
>> the interrupt property, for example:
>>
>> interrupts-extended = <&gpio_intc 16 1>;
>>
>> The purpose of this change is to make GPIO-to-IRQ conversion easier for
>> users who do not want to know the actual interrupt number. The interrupt
>> mapping is not fixed and varies between different SoCs, so users should
>> not need to handle the hardware interrupt allocation details.
> This is not why gpiod_to_irq() exists. It is not a convenience function
> that is voluntary to implement.
> 
> If you implement gpiod_to_irq() you implement an entire
> irqchip otherwise it is a bug.
> 
> If the pin control + GPIO driver should serve IRQ numbers in any
> shape or form, you need to go the whole way and provide a
> hierarchical irqchip.
> 
> It doesn't matter if you don't need to set a single bit in the
> pinctrl + GPIO hardware for these IRQs, the fact that they are
> routed internally on the SoC out through the pin control and
> GPIO block by definition makes it hierarchical.
> 
> Yours,
> Linus Walleij

^ permalink raw reply

* Re: (subset) [PATCH v3 0/3] Add Hawi UFS PHY and Controller support
From: Martin K. Petersen @ 2026-06-16  2:26 UTC (permalink / raw)
  To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, mani, alim.akhtar,
	bvanassche, andersson, dmitry.baryshkov, abel.vesa, luca.weiss,
	palash.kambar
  Cc: Martin K . Petersen, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, linux-scsi, nitin.rawat
In-Reply-To: <20260526090956.2340262-1-palash.kambar@oss.qualcomm.com>

On Tue, 26 May 2026 14:39:53 +0530, palash.kambar@oss.qualcomm.com wrote:

> This series introduces devicetree binding documentation and PHY
> initialization support required to enable UFS on this platform.
> 
> 1. Devicetree binding documentation for the QMP UFS PHY
>      used on Qualcomm Hawi.
>   2. Devicetree binding documentation for the UFS controller
>      instance present on the Hawi platform.
>   3. Initialization sequence tables and configuration required
>      for the QMP UFS PHY on Hawi SoC.
> 
> [...]

Applied to 7.2/scsi-queue, thanks!

[2/3] scsi: ufs: qcom: dt-bindings: Document the Hawi UFS controller
      https://git.kernel.org/mkp/scsi/c/63977ab3c6a0

-- 
Martin K. Petersen

^ permalink raw reply

* Re: [PATCH v5 09/16] media: iris: Add framework support for AR50_LITE video core
From: sashiko-bot @ 2026-06-16  2:17 UTC (permalink / raw)
  To: Dmitry Baryshkov; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260616-iris-ar50lt-v5-9-583b42770b6a@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] pm_runtime_get_sync() leaks usage count on failure in iris_enable_power_domains().
- [High] iris_disable_power_domains() bails early on OPP failure, bypassing pm_runtime_put_sync() and leaking the PM reference.
- [High] 32-bit multiplication in VPU frequency calculations silently overflows, leading to drastic underclocking at high resolutions/framerates.
- [High] The function `iris_vpu_boot_firmware()` returns success (0) even when it detects a firmware boot error.
--

commit 7269410ec2faaa10691a53dc0ab6c6efcc23fc3e
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: Add framework support for AR50_LITE video core

Add power sequence for ar5lt core. Add register handling for ar50lt by
hooking up vpu op with ar50lt specific implemtation or resue from earlier
generation wherever feasible.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
> new file mode 100644
> index 0000000000000..e084a5b49f2e3
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c

[ ... ]

> +static int iris_vpu_ar50lt_power_on_controller(struct iris_core *core)
> +{
> +	int ret;
> +
> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);

[Severity: High]
This isn't a bug introduced by this patch, but do iris_enable_power_domains()
and iris_disable_power_domains() leak runtime PM references on failure?

In drivers/media/platform/qcom/iris/iris_resources.c,
iris_enable_power_domains() calls:

	ret = pm_runtime_get_sync(pd_dev);
	if (ret < 0)
		return ret;

If the resume fails, it returns a negative error code but leaves the usage
count incremented without calling pm_runtime_put_noidle().

Similarly, iris_disable_power_domains() has an early return that bypasses
pm_runtime_put_sync():

	ret = iris_opp_set_rate(core->dev, 0);
	if (ret)
		return ret;
	
	pm_runtime_put_sync(pd_dev);

If iris_opp_set_rate() fails, the pm_runtime_put_sync() call is skipped,
permanently leaking the runtime PM reference and leaving the power domain
active.

[ ... ]

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> index 41498f94480e8..d64e7745a63dc 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> @@ -97,7 +97,8 @@ int iris_vpu_boot_firmware(struct iris_core *core)
>  	}
>  
>  	writel(HOST2XTENSA_INTR_ENABLE, core->reg_base + CPU_CS_H2XSOFTINTEN);
> -	writel(0x0, core->reg_base + CPU_CS_X2RPMH);
> +	if (!core->iris_platform_data->no_rpmh)
> +		writel(0x0, core->reg_base + CPU_CS_X2RPMH);
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does iris_vpu_boot_firmware()
return success even when it detects a firmware boot error?

Earlier in this function:

	if ((ctrl_status & CTRL_ERROR_STATUS__M) == 0x4) {
		dev_err(core->dev, "invalid setting for uc_region\n");
		break;
	}

Because the loop is exited early via break, the count does not reach
max_tries. This skips the subsequent timeout check (if (count >= max_tries)),
and the function incorrectly falls through to return 0.

Could this cause the caller to proceed assuming the firmware booted correctly,
leading to undefined behavior and hardware timeouts later?

[ ... ]

> @@ -422,6 +423,32 @@ void iris_vpu35_vpu4x_program_bootup_registers(struct iris_core *core)
>  	writel(0x1, core->reg_base + WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0);
>  }
>  
> +u64 iris_vpu2_calculate_frequency(struct iris_inst *inst, size_t data_size)
> +{
> +	struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
> +	struct v4l2_format *inp_f = inst->fmt_src;
> +	u32 mbs_per_second, mbpf, height, width;
> +	unsigned long vpp_freq, vsp_freq;
> +	u32 fps = inst->frame_rate;
> +
> +	width = max(inp_f->fmt.pix_mp.width, inst->crop.width);
> +	height = max(inp_f->fmt.pix_mp.height, inst->crop.height);
> +
> +	mbpf = NUM_MBS_PER_FRAME(height, width);
> +	mbs_per_second = mbpf * fps;
> +
> +	vpp_freq = mbs_per_second * caps->mb_cycles_vpp;

[Severity: High]
This is a pre-existing issue, but does this 32-bit multiplication silently
overflow before assignment?

Both mbs_per_second and caps->mb_cycles_vpp are u32. For high resolutions
and framerates (like 8K at 240+ FPS), mbs_per_second can reach ~33M and
mb_cycles_vpp is ~200. This results in a product greater than 6.6 billion,
exceeding the u32 maximum.

The value wraps around before being cast to unsigned long, which might cause
the requested clock frequency to be set drastically lower than required.

The same multiplication overflow also exists in
iris_vpu3x_vpu4x_calculate_frequency() where mult_frac() internally executes a
u32 * u32 operation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-iris-ar50lt-v5-0-583b42770b6a@oss.qualcomm.com?part=9

^ permalink raw reply

* Re: [PATCH 1/7] dt-bindings: display: tegra: Changes to support Tegra264
From: Mikko Perttunen @ 2026-06-16  2:00 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Thierry Reding, Jonathan Hunter, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-tegra, dri-devel,
	devicetree, linux-kernel
In-Reply-To: <20260612-iphone-racing-c2f1934f3cf1@spud>

On Saturday, June 13, 2026 1:16 AM Conor Dooley wrote:
> On Fri, Jun 12, 2026 at 03:32:29PM +0900, Mikko Perttunen wrote:
> > Add nvidia,tegra264-host1x compatible string. The Tegra264 host1x is
> > similar to Tegra234, but with a different set of engines and layout.
> > 
> > The engine register range is no longer continuous, so two range entries
> > are also needed.
> 
> Please restrict the new ranges of 2 to only the new device.

Thank you, will fix.

Mikko

> pw-bot: changes-requested
> 
> 
> Thanks,
> Conor.
> 
> > 
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> >  .../devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml     | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > index 3563378a01af..5b0e3158aa5b 100644
> > --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > @@ -25,6 +25,7 @@ properties:
> >            - nvidia,tegra186-host1x
> >            - nvidia,tegra194-host1x
> >            - nvidia,tegra234-host1x
> > +          - nvidia,tegra264-host1x
> >  
> >        - items:
> >            - const: nvidia,tegra132-host1x
> > @@ -57,7 +58,8 @@ properties:
> >      enum: [1, 2]
> >  
> >    ranges:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> >  
> >    clocks:
> >      description: Must contain one entry, for the module clock. See
> > @@ -192,6 +194,7 @@ allOf:
> >            contains:
> >              enum:
> >                - nvidia,tegra234-host1x
> > +              - nvidia,tegra264-host1x
> >      then:
> >        properties:
> >          reg-names:
> > 
> > -- 
> > 2.53.0
> > 





^ permalink raw reply

* Re: [PATCH net-next v5 0/4] net: dsa: mt7628 embedded switch initial support
From: Jakub Kicinski @ 2026-06-16  1:56 UTC (permalink / raw)
  To: Joris Vaisvila
  Cc: netdev, horms, pabeni, edumazet, davem, olteanv, Andrew Lunn,
	devicetree, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Arınç ÜNAL, Landen Chao, DENG Qingfang, Sean Wang,
	Daniel Golle
In-Reply-To: <20260613181845.111877-1-joey@tinyisr.com>

On Sat, 13 Jun 2026 21:18:41 +0300 Joris Vaisvila wrote:
> This patch series adds initial support for the MediaTek MT7628 Embedded
> Switch.
> 
> The driver implements the basic functionality required to operate the
> switch using DSA. The hardware provides five internal Fast Ethernet user
> ports and one Gigabit port connected internally to the CPU MAC.

net-next has closed, and Linus W is reworking the tagger API in
parallel:
https://lore.kernel.org/all/20260616-dsa-fix-free-skb-v1-1-fd30b35dcf66@kernel.org/
Please repost after the merge window. See
https://netdev.bots.linux.dev/net-next.html
-- 
pw-bot: defer

^ permalink raw reply

* Re: [PATCH v11 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
From: sashiko-bot @ 2026-06-16  1:48 UTC (permalink / raw)
  To: Edelweise Escala; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260616-ltc3220-driver-v11-2-6e51dd97ced6@analog.com>

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Typographical error in regmap_config uses an undefined enum value `REGCACHE_FLAT_S`, causing a build failure.
- [High] Aggregated mode (QUICK_WRITE) causes the regmap cache to desynchronize, breaking LEDs during regcache_sync().
- [Medium] regcache_sync() in the resume handler is a no-op because the regmap cache is never marked dirty.
- [Medium] User-provided brightness values in hardware patterns are incorrectly masked instead of clamped, causing wrap-around behavior.
- [Medium] The driver modifies the cached LED state (`uled_cfg->reg_value`) and hardware registers without driver-level synchronization, allowing concurrent operations to permanently desync the software cache from the hardware.
--

commit 531f73073c031952f85d8c75169d6129f992543c
Author: Edelweise Escala <edelweise.escala@analog.com>

leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
    
Add driver for the LTC3220 18-channel LED driver with I2C interface,
individual brightness control, and hardware-assisted blink/gradation
features.

> diff --git a/drivers/leds/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
> new file mode 100644
> index 0000000000000..8bb06e50d3c6a
> --- /dev/null
> +++ b/drivers/leds/leds-ltc3220.c
[ ... ]
> +static const struct regmap_config ltc3220_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = LTC3220_GRAD_BLINK_REG,
> +	.cache_type = REGCACHE_FLAT_S,

[Severity: High]
Does this cause a build failure?

The correct enum value defined by the kernel is REGCACHE_FLAT, but this uses
an undefined REGCACHE_FLAT_S.

> +};
[ ... ]
> +static int ltc3220_set_led_data(struct led_classdev *led_cdev,
> +				enum led_brightness brightness)
> +{
> +	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> +							 led_cdev);
> +	struct ltc3220 *ltc3220 = container_of(uled_cfg - uled_cfg->led_index, struct ltc3220,
> +					       uled_cfg[0]);
> +	int ret;
> +
> +	ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
> +			   brightness);

[Severity: High]
When configured in aggregated mode, the hardware's QUICK_WRITE feature is
enabled, and a single write to LED 0 implicitly updates LEDs 1-17 in
hardware.

However, the driver doesn't update the regmap cache for registers 0x02
through 0x12, leaving them at 0.

When regcache_sync() runs during resume, will it write out the cached 0s and
inadvertently turn off LEDs 1-17?

Should the driver manually sync the cache for these implicitly updated
registers?

> +	if (ret)
> +		return ret;
> +
> +	uled_cfg->reg_value = brightness;

[Severity: Medium]
Since the LED core does not strictly serialize asynchronous triggers against
all callback paths, is this unprotected assignment susceptible to a race
condition?

A concurrent call to ltc3220_set_led_data() or ltc3220_blink_set() could
cause the software reg_value to become permanently desynchronized from the
hardware register state.

Does this need a per-device or per-LED mutex to protect read-modify-write
operations?

> +
> +	/*
> +	 * When aggregated LED mode is enabled, writing to LED 1 updates all
> +	 * LEDs simultaneously via quick-write mode. Update cached values for
[ ... ]
> +static int ltc3220_pattern_set(struct led_classdev *led_cdev,
> +			       struct led_pattern *pattern,
> +			       u32 len, int repeat)
> +{
[ ... ]
> +	if (repeat != 1)
> +		return -EOPNOTSUPP;
> +
> +	start_brightness = pattern[0].brightness & LTC3220_LED_CURRENT_MASK;
> +	end_brightness = pattern[1].brightness & LTC3220_LED_CURRENT_MASK;

[Severity: Medium]
Will this bitwise masking cause wrap-around behavior if a user provides a
brightness value greater than 63 via the sysfs hw_pattern interface?

For instance, a value of 64 would wrap to 0, unexpectedly turning the LED
off.

Would it be safer to use min_t() to clamp the input instead of a
bitwise mask?

> +
> +	is_increasing = end_brightness > start_brightness;
[ ... ]
> +static int ltc3220_blink_set(struct led_classdev *led_cdev,
> +			     unsigned long *delay_on,
> +			     unsigned long *delay_off)
> +{
[ ... ]
> +	ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
> +				 LTC3220_BLINK_MASK, FIELD_PREP(LTC3220_BLINK_MASK, blink_mode));
> +	if (ret)
> +		return ret;
> +
> +	/* Ensure LED has non-zero brightness before enabling blink mode */
> +	if (uled_cfg->reg_value == 0)
> +		uled_cfg->reg_value = led_cdev->max_brightness;

[Severity: Medium]
Could this rely on a stale reg_value if concurrent operations are
modifying the state without synchronization, as noted in
ltc3220_set_led_data() above?

> +
> +	return regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
> +			    FIELD_PREP(LTC3220_LED_MODE_MASK, LTC3220_BLINK_MODE) |
> +			    uled_cfg->reg_value);
> +}
[ ... ]
> +static int ltc3220_suspend(struct device *dev)
> +{
> +	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> +
> +	for (int i = 0; i < LTC3220_NUM_LEDS; i++) {
> +		if (ltc3220->uled_cfg[i].registered)
> +			led_classdev_suspend(&ltc3220->uled_cfg[i].led_cdev);
> +	}
> +
> +	return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> +				 LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
> +}

[Severity: Medium]
Should regcache_mark_dirty(ltc3220->regmap) be called here or in resume?

Without marking the cache dirty, the regcache_sync() in ltc3220_resume()
will silently return without restoring any registers, resulting in the
permanent loss of the blink and gradation states upon resume.

> +
> +static int ltc3220_resume(struct device *dev)
> +{
> +	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> +	int ret;
> +
> +	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> +				 LTC3220_SHUTDOWN_MASK, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regcache_sync(ltc3220->regmap);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ltc3220-driver-v11-0-6e51dd97ced6@analog.com?part=2

^ 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