Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] i2c: imx: Fix slave registration race and error handling
From: Liem @ 2026-06-29  2:38 UTC (permalink / raw)
  To: carlos.song
  Cc: andi.shyti, biwen.li, festevam, frank.li, frank.li, imx, kernel,
	liem16213, linux-arm-kernel, linux-i2c, linux-kernel, o.rempel,
	s.hauer, stable, wsa
In-Reply-To: <20260629023829.152651-1-liem16213@gmail.com>

In i2c_imx_reg_slave(), the slave pointer was assigned before
pm_runtime_resume_and_get().  If pm_runtime_resume_and_get() failed,
the error path returned without clearing i2c_imx->slave, leaving it
non-NULL and causing all subsequent registration attempts to fail
with -EBUSY.

Additionally, because this driver uses a shared IRQ, the interrupt
handler i2c_imx_isr() can execute concurrently and, after acquiring
slave_lock, dereference i2c_imx->slave.  The previous fix attempt
added a lockless i2c_imx->slave = NULL on the error path, but that
could race with the ISR under the lock and still cause a NULL pointer
dereference.

Fix both issues by deferring the assignment of i2c_imx->slave and
i2c_imx->last_slave_event to after a successful resume, and by
performing the assignment inside the slave_lock critical section.
This guarantees that the slave pointer is never left stale on the
error path and is always valid when observed by the interrupt handler.

Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")
Cc: stable@vger.kernel.org
Signed-off-by: Liem <liem16213@gmail.com>
---
v3 -> v4:
  - Instead of clearing the slave pointer on error, defer the
    assignment until after pm_runtime_resume_and_get() succeeds,
    and take slave_lock to avoid racing with the shared IRQ handler.
    Suggested by Sashiko and Carlos Song
---
 drivers/i2c/busses/i2c-imx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 28313d0fad37..2398c406e913 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -930,9 +930,6 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
 	if (i2c_imx->slave)
 		return -EBUSY;
 
-	i2c_imx->slave = client;
-	i2c_imx->last_slave_event = I2C_SLAVE_STOP;
-
 	/* Resume */
 	ret = pm_runtime_resume_and_get(i2c_imx->adapter.dev.parent);
 	if (ret < 0) {
@@ -940,6 +937,11 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
 		return ret;
 	}
 
+	scoped_guard(spinlock_irqsave, &i2c_imx->slave_lock) {
+		i2c_imx->slave = client;
+		i2c_imx->last_slave_event = I2C_SLAVE_STOP;
+	}
+
 	i2c_imx_slave_init(i2c_imx);
 
 	return 0;
-- 
2.34.1



^ permalink raw reply related

* [PATCH v4 2/2] i2c: imx: Cancel hrtimer before clearing slave pointer
From: Liem @ 2026-06-29  2:38 UTC (permalink / raw)
  To: carlos.song
  Cc: andi.shyti, biwen.li, festevam, frank.li, frank.li, imx, kernel,
	liem16213, linux-arm-kernel, linux-i2c, linux-kernel, o.rempel,
	s.hauer, stable, wsa, Carlos Song
In-Reply-To: <20260629023829.152651-1-liem16213@gmail.com>

In i2c_imx_unreg_slave(), the slave pointer is set to NULL after
disabling interrupts.  However, a pending interrupt might already
have started the hrtimer (i2c_imx_slave_timeout) before the pointer
was cleared.  If the hrtimer fires after i2c_imx->slave is set to
NULL, the timer callback i2c_imx_slave_finish_op() will call
i2c_imx_slave_event() with a NULL slave pointer, which results in a
use-after-free / NULL pointer dereference.

Fix by canceling the hrtimer and waiting for it to complete after
disabling interrupts, before clearing the slave pointer.

Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")
Cc: stable@vger.kernel.org
Acked-by: Carlos Song <carlos.song@nxp.com>
Signed-off-by: Liem <liem16213@gmail.com>
---
v3 -> v4: No changes, added Acked-by from Carlos Song.
---
 drivers/i2c/busses/i2c-imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 2398c406e913..b1c6581db774 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -960,6 +960,7 @@ static int i2c_imx_unreg_slave(struct i2c_client *client)
 
 	i2c_imx_reset_regs(i2c_imx);
 
+	hrtimer_cancel(&i2c_imx->slave_timer);
 	i2c_imx->slave = NULL;
 
 	/* Suspend */
-- 
2.34.1



^ permalink raw reply related

* Re: [PATCH] Bluetooth: btmtk: Disable remote wakeup for MT7922/MT7925
From: Chris Lu (陸稚泓) @ 2026-06-29  3:01 UTC (permalink / raw)
  To: luiz.dentz@gmail.com, i@rong.moe
  Cc: Will-CY Lee (李政穎), marcel@holtmann.org,
	AngeloGioacchino Del Regno, SS Wu (巫憲欣),
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-bluetooth@vger.kernel.org,
	patchwork-bot+bluetooth@kernel.org,
	linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com
In-Reply-To: <4e96132bcfb267af05c06dc277b5ba6895e3ddde.camel@rong.moe>

Hi Rong,

Thanks for your understanding. We hope to find a better way to handle
this issue.


Hi Luiz,

Regarding this issue, MediaTek Bluetooth team has already initialed an
internal investigation for problematic combination and will seek
assistance from AMD SOC team to investigate the root cause of this
issue.

However, since this change has impact on MTK products, may I submit a
patch to remove it first? 

If the investigation determines the issue lies within MTK BT
driver/firmware, we'll submit a separate solution to fix it.


On Sat, 2026-06-27 at 03:56 +0800, Rong Zhang wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Hi Chris, Luiz,
> 
> On Fri, 2026-06-26 at 17:56 +0000, Chris Lu (陸稚泓) wrote:
> > Hi Rong and Luiz,
> > 
> > On Fri, 2026-06-26 at 12:54 -0400, Luiz Augusto von Dentz wrote:
> > > Hi,
> > > 
> > > 
> > > On Fri, Jun 26, 2026 at 7:19 AM Rong Zhang <i@rong.moe> wrote:
> > > > 
> > > > Hi Chris,
> > > > 
> > > > 于 2026年6月26日 GMT+08:00 10:40:54,"Chris Lu (陸稚泓)"
> > > > <Chris.Lu@mediatek.com> 写道:
> > > > > Hi Luiz,
> > > > > 
> > > > > Could we revert this change?
> > > > 
> > > > Please don't. The fundamental goal of the patch is to make
> > > > MT7922/MT7925 *usable* on affected platforms.
> > > > 
> > > > Moreover, almost all recent AMD platforms ship with
> > > > MT7922/MT7925,
> > > > reverting this patch affects quite a lot of users. Almost every
> > > > AMD
> > > > platform I've met suffers from the issue, and there are plenty
> > > > of
> > > > bug reports.
> > > > 
> > > > Intel platforms never ship with MTK WLAN NICs, so I can't tell
> > > > if
> > > > the issue is reproducible on them.
> > > 
> > We still have non-AMD platforms with MT7922/7925, where
> > "wakeup by Bluetooth" is an essential feature. In other word, this
> > change will have a serious impact to them.
> 
> Oh, perhaps I missed something before. Your words were a bit unclear
> to
> me at first glance.
> 
> You previous reply said "wake on Bluetooth". I interpreted it as USB
> remote wakeup upon Bluetooch device connection.
> 
> "Wakeup by Bluetooth" is clearer. You meant using the Bluetooth
> interface as a wakeup source to wake up the system from sleep, right?
> 
> Hmm, it makes more sense to me.
> 
> My patch was inspired by commit f4292e2faf52 ("Bluetooth: btusb: Make
> the CSR clone chip force-suspend workaround more generic"). The
> mentioned commit is correct as the quirky chips can't perform remote
> wakeup at all.
> 
> If MT7922/MT7925 can reliably wake up the system from sleep, my patch
> could be a burden. I won't insist anymore if you'd like to revert it.
> 
> > 
> > > We cannot really remove it anymore, since it has already been
> > > pulled.
> > > Therefore, I strongly prefer that the issue is fixed somehow or
> > > if
> > > there is no other way then a proper revert patch must be sent,
> > > since
> > > I
> > > don't have any hardware with this controller I cannot really test
> > > this
> > > myself.
> > > 
> > Got  it.
> > > > > 
> > > > > Sorry, MediaTek wasn't aware someone submitted this change
> > > > > and it
> > > > > had
> > > > > already been merged.
> > > > > 
> > > > > MTK believes this issue is strongly related to the platform's
> > > > > USB
> > > > > hub
> > > > > behavior,
> > > > 
> > > > Still, I believe that there are some interoperability issues in
> > > > MT7922/MT7925's hardware or firmware, as reinitializing the
> > > > XHCI
> > > > root hub (by logically removing it from the PCI bus and probing
> > > > it
> > > > again) doesn't make it recover at all.
> > > > 
> > > > > The product lines MTK directly support haven't reported such
> > > > > issue.
> > > > 
> > > > ...or did you mean none of the existing AMD platforms are
> > > > supported
> > > > by MTK?
> > > > 
> > I'll report it to corresponding department to see if they have such
> > issue. (I'm not from Bluetooth with AMD platform support team)
> > 
> > > > > 
> > > > > Disable wake capability would severely impact wake on
> > > > > Bluetooth
> > > > > on
> > > > > MTK's official product lines.
> > > > 
> > > > Could you kindly explain "MTK's official product lines"?
> > > > 
> > Some products are module makers selling our modules to downstream
> > customers directly.
> > 
> > > > > Furthermore, this patch looks like a
> > > > > workaround. There should be a better way to handle this
> > > > > issue. We
> > > > > hope
> > > > > this change can be reverted.
> > > > 
> > > > The issue is severe on affected platforms by making the
> > > > Bluetooth
> > > > interface completely gone. IMHO, we must figure out the "better
> > > > way" before reverting it, or else it's more like burying your
> > > > head
> > > > into sand by shouting "works fine on our platforms" (TM).
> > > > 
> > > > That being said, I think we can narrow the range of the quirk
> > > > down
> > > > to AMD platforms only. Does it make sense to you? If so, I will
> > > > submit a patch for this.
> > > 
> > As I mentioned above, we have many platforms using the kernel
> > driver,
> > not just AMD,
> > 
> > If this change can be limited to specific platform , it would be
> > the
> > best way so far.
> 
> I plan to submit another patch to properly fix it next week. In
> general,
> I will:
> 
> - narrow the quirk down to AMD platforms only
> - disable USB remote wakeup for runtime autosuspend only
> - leave the wakeup source as is, so users can enable "wakeup by
> Bluetooth" even on AMD platforms if they really want it
> 
> Hopefully it will minimize the impact of the workaround.
> 
> Thanks,
> Rong
> 
> > 
> > > Ok, then you guys please coordinate. I don't want to start
> > > requiring
> > > Sign-Off-By (SOB) from driver authors because this instance shows
> > > that
> > > response time can be very long. Therefore, the best way forward
> > > in my
> > > opinion, is to have the issue fixed in a way that works for both
> > > of
> > > you.
> > 
> > Of course, Mediatek BT team also agree that any issue need to be
> > resolved, However, original issue for this change requires further
> > verification of Hardware/firmware and even USB behavior. A device
> > that
> > can reproduce the issue stably is required in order to experiment
> > and
> > identify the root cause.
> > 
> > As mentioned, I'll deliver internally to relate support team. If
> > they
> > have similar settings, MTK will attempt to reproduce it and provide
> > an
> > official solution.
> > 
> > Thanks very much for your input and understanding.
> > 
> > Best Regards,
> > Chris Lu

Thanks,
Chris Lu


^ permalink raw reply

* Re: [PATCH v14 0/7] Provide support for Trigger Generation Unit
From: Songwei.Chai @ 2026-06-29  3:03 UTC (permalink / raw)
  To: andersson, alexander.shishkin, mike.leach, konrad.dybcio,
	suzuki.poulose, james.clark, krzk+dt, conor+dt, gregkh
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
	devicetree, Songwei Chai
In-Reply-To: <48c6abce-c492-46a6-84ef-3074983e817c@oss.qualcomm.com>

Hi Greg & Alexander,

Apologies for interrupting again.

As the TGU hardware plays an important role in Qualcomm tracing design, 
I would greatly appreciate it if you could kindly take some time to 
review this at your earliest convenience.

Best regards,
Songwei

On 6/5/2026 11:14 AM, Songwei.Chai wrote:
> hi Greg & Alexander,
>
> I hope this message finds you well.
>
> We are currently working on a Qualcomm TGU (Trace Generation Unit) 
> driver and would like to seek your guidance on how best to integrate 
> it into the /hwtracing/ subsystem.
>
> TGU is a programmable hardware block that monitors signal conditions 
> and triggers debug-related actions, effectively acting as a trace 
> generation utility. Based on its functionality, placing it under 
> |"drivers/hwtracing"| appears to be a reasonable choice.
>
> We initially explored integrating it into 
> "|drivers/hwtracing/coresight"|.However, that approach did not receive 
> support, primarily because the component is not tightly coupled with 
> the CoreSight subsystem.
>
> *Chat History*: 
> https://lore.kernel.org/all/CAJ9a7ViKxHThyZfFFDV_FkNRimk4uo1NrMtQ-kcaj1qO4ZcGnA@mail.gmail.com/
>
> As an alternative, we are proposing to introduce a dedicated 
> |"drivers/hwtracing/qcom/"|directory, similar to the existing 
> "|drivers/hwtracing/intel_th"|.
> A more detailed rationale can be found in the cover letter under the 
> section /"Why we are proposing this:"/.
>
> *Current status of the patch:*
>
>  * Reviewed-by: Jie Gan
>  * Acked-by: Konrad Dybcio
>
> We would greatly appreciate it if you could take some time to review 
> this patch and share your thoughts. Your feedback would be very 
> helpful in moving this effort forward in the right direction.
>
> Thanks a lot for your time and consideration.
>
> Best regards,
> Songwei
>
> On 4/17/2026 3:33 PM, Songwei Chai wrote:
>> We propose creating a new qcom directory under drivers/hwtracing
>> to host this TGU driver, as well as additional Qualcomm-specific
>> hwtracing drivers that we plan to submit in the coming months.
>> This structure will help organize vendor-specific implementations
>> and facilitate future development and maintenance.
>>
>> Feedback from the community on this proposal is highly appreciated.
>>
>> - Why we are proposing this:
>>
>> TGU has the ability to monitor signal conditions and trigger 
>> debug-related
>> actions, serving as a programmable hardware component that enhances 
>> system
>> trace and debug capabilities. Placing it under drivers/hwtracing aligns
>> with its function as a trace generation utility.
>>
>> We previously attempted to push this driver to 
>> drivers/hwtracing/coresight,
>> but did not receive support from the maintainers of the CoreSight
>> subsystem. The reason provided was: “This component is primarily a part
>> of the Qualcomm proprietary QPMDA subsystem, and is capable of operating
>> independently from the CoreSight hardware trace generation system.”
>>
>> Chat history : 
>> https://lore.kernel.org/all/CAJ9a7ViKxHThyZfFFDV_FkNRimk4uo1NrMtQ-kcaj1qO4ZcGnA@mail.gmail.com/
>>
>> Given this, we have been considering whether it would be appropriate
>> to create a dedicated drivers/hwtracing/qcom directory for
>> Qualcomm-related hwtracing drivers. This would follow the precedent set
>> by Intel, which maintains its own directory at 
>> drivers/hwtracing/intel_th.
>> We believe this structure would significantly facilitate
>> future submissions of related Qualcomm drivers.
>>
>> - Maintenance of drivers/hwtracing/qcom:
>>
>> Bjorn, who maintains linux-arm-msm, will be the maintainer of this
>> directory — we’ve discussed this with him and he’s aware that his task
>> list may grow accordingly. Additionally, Qualcomm engineers familiar 
>> with
>> the debug hardware — such as [Tingwei Zhang, Jinlong Mao, Songwei Chai],
>> will be available to review incoming patches and support ongoing
>> development.
>>
>> - Detail for TGU:
>>
>> This component can be utilized to sense a plurality of signals and
>> create a trigger into the CTI or generate interrupts to processors
>> once the input signal meets the conditions. We can treat the TGU’s
>> workflow as a flowsheet, it has some “steps” regions for customization.
>> In each step region, we can set the signals that we want with priority
>> in priority_group, set the conditions in each step via condition_decode,
>> and set the resultant action by condition_select. Meanwhile,
>> some TGUs (not all) also provide timer/counter functionality.
>> Based on the characteristics described above, we consider the TGU as a
>> helper in the CoreSight subsystem. Its master device is the TPDM, which
>> can transmit signals from other subsystems, and we reuse the existing
>> ports mechanism to link the TPDM to the connected TGU.
>>
>> Here is a detailed example to explain how to use the TGU:
>>
>> In this example, the TGU is configured to use 2 conditions, 2 steps, and
>> the timer. The goal is to look for one of two patterns which are 
>> generated
>> from TPDM, giving priority to one, and then generate a trigger once the
>> timer reaches a certain value. In other words, two conditions are used
>> for the first step to look for the two patterns, where the one with the
>> highest priority is used in the first condition. Then, in the second 
>> step,
>> the timer is enabled and set to be compared to the given value at each
>> clock cycle. These steps are better shown below.
>>                     |-----------------|
>>                |                 |
>>                |       TPDM      |
>>                |                 |
>>                |-----------------|
>>                         |
>>                         |
>>      --- --- --- --- --- --- --- --- --- --- --- --- --- --- --- --- 
>> ------
>>      | |                                                 |
>>      |                  | |--------------------| |
>>      |    |---- --->    |                          |  Go to next 
>> steps  | |
>>      |    |             |                |--- ---> |  Enable 
>> timer      | |
>>      |    |             v                | |                    | |
>>      |    |    |-----------------|       | |--------------------| |
>>      |    |    |                 |  Yes  | |           |
>>      |    |    |   inputs==0xB   | ----->| | <-------- |
>>      |    |    |                 |       | |      No | |
>>      | No |    |-----------------|       | v         | |
>>      |    |             |                | |-----------------| | |
>>      |    |             |                | |                 | | |
>>      |    |             |                |          | timer>=3   |-- |
>>      |    |             v                | |                 |   |
>>      |    |    |-----------------|       | |-----------------|   |
>>      |    |    |                 |  Yes  | |           |
>>      |    |--- |   inputs==0xA   | ----->| | Yes       |
>>      |         |                 | |           |
>>      |         |-----------------| v           |
>>      | |-----------------|   |
>>      | |                 |   |
>>      |                                              | Trigger    |   |
>>      | |                 |   |
>>      | |-----------------|   |
>>      |  TGU |           |
>>      |--- --- --- --- --- --- --- --- --- --- --- --- --- --- |--- 
>> --- -- |
>>                                                               |
>>                                                               v
>> |-----------------|
>>                                                      |The controllers  |
>>                                                      |which will use   |
>>                                                      |triggers further |
>> |-----------------|
>>
>> steps:
>>      1. Reset TGU /*it will disable tgu and reset dataset*/
>>      - echo 1 > /sys/bus/amba/devices/<tgu-name>/reset_tgu
>>
>>      2. Set the pattern match for priority0 to 0xA = 0b1010 and for
>>         priority 1 to 0xB = 0b1011.
>>      - echo 0x11113232 > 
>> /sys/bus/amba/devices/<tgu-name>/step0_priority0/reg0
>>      - echo 0x11113233 > 
>> /sys/bus/amba/devices/<tgu-name>/step0_priority1/reg0
>>
>>      Note:
>>          Bit distribution diagram for each priority register
>> |-------------------------------------------------------------------|
>>      |   Bits          |       Field Nam   | Description              |
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |     29:28       |   SEL_BIT7_TYPE2  | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |     25:24       |   SEL_BIT6_TYPE2  | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |     21:20       |   SEL_BIT5_TYPE2  | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |     17:16       |   SEL_BIT4_TYPE2  | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |     13:12       |   SEL_BIT3_TYPE2  | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |      9:8        |   SEL_BIT2_TYPE2  | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |      5:4        |  SEL_BIT1_TYPE2   | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      |                 |                   | 00 = bypass for OR 
>> output   |
>>      |      1:0        |  SEL_BIT0_TYPE2   | 01 = bypass for AND 
>> output  |
>>      |                 |                   | 10 = sense input '0' is 
>> true|
>>      |                 |                   | 11 = sense input '1' is 
>> true|
>> |-------------------------------------------------------------------|
>>      These bits are used to identify the signals we want to sense, with
>>      a maximum signal number of 140. For example, to sense the signal
>>      0xA (binary 1010), we set the value of bits 0 to 13 to 3232, which
>>      represents 1010. The remaining bits are set to 1, as we want to use
>>      AND gate to summarize all the signals we want to sense here. For
>>      rising or falling edge detection of any input to the priority, set
>>      the remaining bits to 0 to use an OR gate.
>>
>>      3. look for the pattern for priority_i i=0,1.
>>      - echo 0x3 > 
>> /sys/bus/amba/devices/<tgu-name>/step0_condition_decode/reg0
>>      - echo 0x30 > 
>> /sys/bus/amba/devices/<tgu-name>/step0_condition_decode/reg1
>>
>> |-------------------------------------------------------------------------------|
>>      |   Bits          |    Field Nam        | 
>> Description                |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |For each decoded 
>> condition, this       |
>>      |      24         |       NOT           |inverts the output. If 
>> the condition   |
>>      |                 |                     |decodes to true, and 
>> the NOT field     |
>>      |                 |                     |is '1', then the output 
>> is NOT true.   |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from the associated|
>>      |      21         |  BC0_COMP_ACTIVE    |comparator will be 
>> actively included in|
>>      |                 |                     |the decoding of this 
>> particular        |
>>      |                 | |condition.                             |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from the associated|
>>      |                 |                     |comparator will need to 
>> be 1 to affect |
>>      |      20         |   BC0_COMP_HIGH     |the decoding of this 
>> condition.        |
>>      |                 |                     |Conversely, a '0' here 
>> requires a '0'  |
>>      |                 |                     |from the 
>> comparator                    |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from the associated|
>>      |      17         |                     |comparator will be 
>> actively included in|
>>      |                 |  TC0_COMP_ACTIVE    |the decoding of this 
>> particular        |
>>      |                 | |condition.                             |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from the associated|
>>      |                 |                     |comparator will need to 
>> be 1 to affect |
>>      |      16         |  TC0_COMP_HIGH      |the decoding of this 
>> particular        |
>>      |                 | |condition.Conversely, a 0 here         |
>>      |                 |                     |requires a '0' from the 
>> comparator     |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from Priority_n    |
>>      |                 |                     |OR logic will be 
>> actively              |
>>      |     4n+3        | Priority_n_OR_ACTIVE|included in the 
>> decoding of            |
>>      |                 |    (n=0,1,2,3)      |this particular 
>> condition.             |
>>      |                 | |                                       |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from Priority_n    |
>>      |                 |                     |will need to be '1' to 
>> affect the      |
>>      |     4n+2        |  Priority_n_OR_HIGH |decoding of this 
>> particular            |
>>      |                 |    (n=0,1,2,3)      |condition. Conversely, 
>> a '0' here      |
>>      |                 |                     |requires a '0' from 
>> Priority_n OR logic|
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from Priority_n    |
>>      |                 |                     |AND logic will be 
>> actively             |
>>      |     4n+1        |Priority_n_AND_ACTIVE|included in the 
>> decoding of this       |
>>      |                 |  (n=0,1,2,3)        |particular 
>> condition.                  |
>>      |                 | |                                       |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from Priority_n    |
>>      |                 |                     |AND logic will need to 
>> be '1' to       |
>>      |      4n         | Priority_n_AND_HIGH |affect the decoding of 
>> this            |
>>      |                 |   (n=0,1,2,3)       |particular condition. 
>> Conversely,      |
>>      |                 |                     |a '0' here requires a 
>> '0' from         |
>>      |                 |                     |Priority_n AND 
>> logic.                  |
>> |-------------------------------------------------------------------------------|
>>      Since we use `priority_0` and `priority_1` with an AND output in 
>> step 2, we set `0x3`
>>      and `0x30` here to activate them.
>>
>>      4. Set NEXT_STEP = 1 and TC0_ENABLE = 1 so that when the conditions
>>         are met then the next step will be step 1 and the timer will 
>> be enabled.
>>      - echo 0x20008 > 
>> /sys/bus/amba/devices/<tgu-name>/step0_condition_select/reg0
>>      - echo 0x20008 > 
>> /sys/bus/amba/devices/<tgu-name>/step0_condition_select/reg1
>>
>> |-----------------------------------------------------------------------------|
>>      |   Bits          |       Field Nam   | 
>> Description                |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This field defines the 
>> next step the   |
>>      |    18:17        |     NEXT_STEP     |TGU will 'goto' for the 
>> associated     |
>>      |                 |                   |Condition and 
>> Step.                    |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |For each possible output 
>> trigger       |
>>      |    13           |     TRIGGER       |available, set a '1' if 
>> you want       |
>>      |                 |                   |the trigger to go active 
>> for the       |
>>      |                 |                   |associated condition and 
>> Step.         |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will cause BC0 to 
>> increment if the|
>>      |    9            |     BC0_INC       |associated Condition is 
>> decoded for    |
>>      |                 |                   |this 
>> step.                             |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will cause BC0 to 
>> decrement if the|
>>      |    8            |     BC0_DEC       |associated Condition is 
>> decoded for    |
>>      |                 |                   |this 
>> step.                             |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will clear BC0 count 
>> value to 0 if|
>>      |    7            |     BC0_CLEAR     |the associated Condition 
>> is decoded    |
>>      |                 |                   |for this 
>> step.                         |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will cause TC0 to 
>> increment until |
>>      |    3            |     TC0_ENABLE    |paused or cleared if the 
>> associated    |
>>      |                 |                   |Condition is decoded for 
>> this step.    |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will cause TC0 to 
>> pause until     |
>>      |    2            |     TC0_PAUSE     |enabled if the associated 
>> Condition    |
>>      |                 |                   |is decoded for this 
>> step.              |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will clear TC0 count 
>> value to 0   |
>>      |    1            |     TC0_CLEAR     |if the associated 
>> Condition is         |
>>      |                 |                   |decoded for this 
>> step.                 |
>> |-----------------------------------------------------------------------------|
>>      |                 |                   |This will set the done 
>> signal to the   |
>>      |    0            |     DONE          |TGU FSM if the associated 
>> Condition    |
>>      |                 |                   |is decoded for this 
>> step.              |
>> |-----------------------------------------------------------------------------|
>>      Based on the distribution diagram, we set `0x20008` for 
>> `priority0` and `priority1` to
>>      achieve "jump to step 1 and enable TC0" once the signal is sensed.
>>
>>      5. activate the timer comparison for this step.
>>      -  echo 0x30000  > 
>> /sys/bus/amba/devices/<tgu-name>/step1_condition_decode/reg0
>>
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from the associated|
>>      |      17         |                     |comparator will be 
>> actively included in|
>>      |                 |  TC0_COMP_ACTIVE    |the decoding of this 
>> particular        |
>>      |                 | |condition.                             |
>> |-------------------------------------------------------------------------------|
>>      |                 |                     |When '1' the output 
>> from the associated|
>>      |                 |                     |comparator will need to 
>> be 1 to affect |
>>      |      16         |  TC0_COMP_HIGH      |the decoding of this 
>> particular        |
>>      |                 | |condition.Conversely, a 0 here         |
>>      |                 |                     |requires a '0' from the 
>> comparator     |
>> |-------------------------------------------------------------------------------|
>>      Accroding to the decode distribution diagram , we give 0x30000 
>> here to set 16th&17th bit
>>      to enable timer comparison.
>>
>>      6. Set the NEXT_STEP = 0 and TC0_PAUSE = 1 and TC0_CLEAR = 1 
>> once the timer
>>         has reached the given value.
>>      - echo 0x6 > 
>> /sys/bus/amba/devices/<tgu-name>/step1_condition_select/reg0
>>
>>      7. Enable Trigger 0 for TGU when the condition 0 is met in step1,
>>         i.e. when the timer reaches 3.
>>      - echo 0x2000 > 
>> /sys/bus/amba/devices/<tgu-name>/step1_condition_select/default
>>
>>      Note:
>>          1. 'default' register allows for establishing the resultant 
>> action for
>>          the default condition
>>
>>          2. Trigger:For each possible output trigger available from
>>          the Design document, there are three triggers: interrupts, CTI,
>>          and Cross-TGU mapping.All three triggers can occur, but
>>          the choice of which trigger to use depends on the user's
>>          needs.
>>
>>      8. Compare the timer to 3 in step 1.
>>      - echo 0x3 > /sys/bus/amba/devices/<tgu-name>/step1_timer/reg0
>>
>>      9. enale tgu
>>      - echo 1 > /sys/bus/amba/devices/<tgu-name>/enable_tgu
>> ---
>> Link to V13: 
>> https://lore.kernel.org/all/20260402092838.341295-1-songwei.chai@oss.qualcomm.com/
>>
>> Changes in V14:
>> - Fix some typos and formatting.
>> ---
>> Link to V12: 
>> https://lore.kernel.org/all/20260317032639.2393221-1-songwei.chai@oss.qualcomm.com/
>>
>> Changes in V13:
>> - add ":" after "KernelVersion"
>> - add an enablement check in the enable function to avoid increasing 
>> the counter each time
>> ---
>> Link to V11: 
>> https://lore.kernel.org/all/ee1ca8e6-8e5f-47d8-8a24-f904ee2fc6d0@oss.qualcomm.com/
>>
>> Changes in V12:
>> - Remove the in-ports property from the bindings, as this device is 
>> decoupled from CoreSight.
>> - Update kernel version and date.
>> ---
>> Link to V10: 
>> https://lore.kernel.org/all/20c5406d-3e9f-4fdb-84ba-4cbe629c79b5@oss.qualcomm.com/
>>
>> Changes in V11:
>> - Change the names of members in drvdata: max_xxx -> num_xxx, enable 
>> -> enabled
>> - Use "FIELD_GET" to replace "BMVAL"
>> - Use devm_kcalloc to replace devm_kzalloc once create members of 
>> value_table
>> - Keep a consistent \n above return
>> - Keep reverse-Christmas-tree style
>> - Add checks so that the enable and reset nodes only accept 0 or 1
>> ---
>> Link to V9: 
>> https://lore.kernel.org/all/20251219065902.2296896-1-songwei.chai@oss.qualcomm.com/
>>
>> Changes in V10:
>> - Modified code formatting based on Jie's feedback to improve 
>> readability.
>> - Applied inverse Christmas tree order to the variables.
>> ---
>> Link to V8: 
>> https://lore.kernel.org/all/20251203090055.2432719-1-songwei.chai@oss.qualcomm.com/
>>
>> Changes in V9:
>> - Decoupled the tgu driver from coresight header file and registered 
>> it as an amba device.
>> - Retained Rob's reviewed-by tag on patch1/7 since the file remains 
>> unchanged.
>> - Updated the sysfs node path in the Documentation directory.
>> ---
>> Link to V7: 
>> https://lore.kernel.org/all/20251104064043.88972-1-songwei.chai@oss.qualcomm.com/
>>
>> Changes in V8:
>> - Add "select" section in bindings.
>> - Update publish date in "sysfs-bus-coresight-devices-tgu".
>> ---
>> Link to V6: 
>> https://lore.kernel.org/all/20250709104114.22240-1-songchai@qti.qualcomm.com/
>>
>> Changes in V7:
>> - Move the TGU code location from 'drivers/hwtracing/coresight/' to 
>> 'drivers/hwtracing/qcom/'.
>> - Rename the spinlock used in the code from 'spinlock' to 'lock'.
>> - Perform the 'calculate_array_location' separately, instead of doing 
>> it within the function.
>> - Update the sender email address.
>> ---
>> Link to V5: 
>> https://lore.kernel.org/all/20250529081949.26493-1-quic_songchai@quicinc.com/
>>
>> Changes in V6:
>> - Replace spinlock with guard(spinlock) in tgu_enable.
>> - Remove redundant blank line.
>> - Update publish date and contact member's name in 
>> "sysfs-bus-coresight-devices-tgu".
>> ---
>> Link to V4: 
>> https://patchwork.kernel.org/project/linux-arm-msm/cover/20250423101054.954066-1-quic_songchai@quicinc.com/
>>
>> Changes in V5:
>> - Update publish date and kernel_version in 
>> "sysfs-bus-coresight-devices-tgu"
>> ---
>> Link to V3: 
>> https://lore.kernel.org/all/20250227092640.2666894-1-quic_songchai@quicinc.com/
>>
>> Changes in V4:
>> - Add changlog in coverletter.
>> - Correct 'year' in Copyright in patch1.
>> - Correct port mechansim description in patch1.
>> - Remove 'tgu-steps','tgu-regs','tgu-conditions','tgu-timer-counters' 
>> from dt-binding
>> and set them through reading DEVID register as per Mike's suggestion.
>> - Modify tgu_disable func to make it have single return point in 
>> patch2 as per
>> Mike's suggestion.
>> - Use sysfs_emit in enable_tgu_show func in ptach2.
>> - Remove redundant judgement in enable_tgu_store in patch2.
>> - Correct typo in description in patch3.
>> - Set default ret as SYSFS_GROUP_INVISIBLE, and returnret at end in 
>> pacth3 as
>> per Mike's suggestion.
>> - Remove tgu_dataset_ro definition in patch3
>> - Use #define constants with explanations of what they are rather than
>> arbitrary magic numbers in patch3 and patch4.
>> - Check -EINVAL before using 'calculate_array_location()' in array in 
>> patch4.
>> - Add 'default' in 'tgu_dataset_show''s switch part in patch4.
>> - Document the value needed to initiate the reset in pacth7.
>> - Check "value" in 'reset_tgu_store' and bail out with an error code 
>> if 0 in patch7.
>> - Remove dev_dbg in 'reset_tgu_store' in patch7.
>> ---
>> Link to V2: 
>> https://lore.kernel.org/all/20241010073917.16023-1-quic_songchai@quicinc.com/
>>
>> Changes in V3:
>> - Correct typo and format in dt-binding in patch1
>> - Rebase to the latest kernel version
>> ---
>> Link to V1: 
>> https://lore.kernel.org/all/20240830092311.14400-1-quic_songchai@quicinc.com/
>>
>> Changes in V2:
>>   - Use real name instead of login name,
>>   - Correct typo and format in dt-binding and code.
>>   - Bring order in tgu_prob(declarations with and without 
>> assignments) as per
>> Krzysztof's suggestion.
>>   - Add module device table in patch2.
>>   - Set const for tgu_common_grp and tgu_ids in patch2.
>>   - Initialize 'data' in tgu_ids to fix the warning in pacth2.
>> ---
>> Songwei Chai (7):
>>    dt-bindings: arm: Add support for Qualcomm TGU trace
>>    qcom-tgu: Add TGU driver
>>    qcom-tgu: Add signal priority support
>>    qcom-tgu: Add TGU decode support
>>    qcom-tgu: Add support to configure next action
>>    qcom-tgu: Add timer/counter functionality for TGU
>>    qcom-tgu: Add reset node to initialize
>>
>>   .../ABI/testing/sysfs-bus-amba-devices-tgu    |  51 ++
>>   .../devicetree/bindings/arm/qcom,tgu.yaml     |  71 ++
>>   drivers/Makefile                              |   1 +
>>   drivers/hwtracing/Kconfig                     |   2 +
>>   drivers/hwtracing/qcom/Kconfig                |  20 +
>>   drivers/hwtracing/qcom/Makefile               |   3 +
>>   drivers/hwtracing/qcom/tgu.c                  | 704 ++++++++++++++++++
>>   drivers/hwtracing/qcom/tgu.h                  | 275 +++++++
>>   8 files changed, 1127 insertions(+)
>>   create mode 100644 
>> Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>>   create mode 100644 Documentation/devicetree/bindings/arm/qcom,tgu.yaml
>>   create mode 100644 drivers/hwtracing/qcom/Kconfig
>>   create mode 100644 drivers/hwtracing/qcom/Makefile
>>   create mode 100644 drivers/hwtracing/qcom/tgu.c
>>   create mode 100644 drivers/hwtracing/qcom/tgu.h
>>


^ permalink raw reply

* Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants
From: Joey Lu @ 2026-06-29  3:47 UTC (permalink / raw)
  To: Icenowy Zheng, Conor Dooley
  Cc: Conor Dooley, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, robh, krzk+dt, conor+dt, ychuang3, schung, yclu4,
	dri-devel, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <9456bde5059bea3aac1ed64355e3f017dd9bd3e5.camel@iscas.ac.cn>


On 6/26/2026 5:33 PM, Icenowy Zheng wrote:
> 在 2026-06-26五的 10:26 +0100,Conor Dooley写道:
>> On Fri, Jun 26, 2026 at 05:00:35PM +0800, Icenowy Zheng wrote:
>>> 在 2026-06-26五的 08:19 +0100,Conor Dooley写道:
>>>> On Fri, Jun 26, 2026 at 01:27:21PM +0800, Icenowy Zheng wrote:
>>>>> 在 2026-06-25四的 17:33 +0100,Conor Dooley写道:
>>>>>> On Thu, Jun 25, 2026 at 05:44:43PM +0800, Joey Lu wrote:
>>>>>>> +allOf:
>>>>>>> +  - if:
>>>>>>> +      properties:
>>>>>>> +        compatible:
>>>>>>> +          contains:
>>>>>>> +            const: thead,th1520-dc8200
>>>>>>> +    then:
>>>>>>> +      properties:
>>>>>>> +        clocks:
>>>>>>> +          minItems: 5
>>>>>>> +          maxItems: 5
>>>>>>> +
>>>>>>> +        clock-names:
>>>>>>> +          minItems: 5
>>>>>>> +          maxItems: 5
>>>>>> All the maxItems here repeat the maximum constraint and do
>>>>>> nothing.
>>>>>>
>>>>>> Since you didn't change the minimum constraint at the top
>>>>>> level,
>>>>>> your
>>>>>> minItems also do nothing.
>>>>>>
>>>>>>> +
>>>>>>> +        resets:
>>>>>>> +          minItems: 3
>>>>>>> +          maxItems: 3
>>>>>>> +
>>>>>>> +        reset-names:
>>>>>>> +          minItems: 3
>>>>>>> +          maxItems: 3
>>>>>>> +
>>>>>>> +      required:
>>>>>>> +        - resets
>>>>>>> +        - reset-names
>>>>>> Both conditional sections have this, but the original binding
>>>>>> doesn't
>>>>>> require these for the thead device. This is a functional
>>>>>> change
>>>>>> therefore and shouldn't be in a patch calling itself
>>>>>> "generalise
>>>>>> for
>>>>>> single ended variants".
>>>>> Well yes they're required.
>>>>>
>>>>> Should I send a patch adding the `thead,th1520-dc8200` part of
>>>>> the
>>>>> schema?
>>>> If you mean the code above, no. Adding a conditional section when
>>>> there's only that compatible doesn't make sense.
>>>>
>>>> What you could do is just add it at the top level though, which
>>>> would
>>>> also benefit this patch since it'd not have to be conditionally
>>>> added
>>>> for the new nuvoton device.
>>>> Just note in your commit message about what the ABI impact of the
>>>> change
>>>> to required properties is (effectively nothing because it's
>>>> optional
>>>> in
>>>> the driver and the only user has the properties).
>>> Okay, I will craft such a patch and send it.
>>>
>>>>>>> +
>>>>>>> +        resets:
>>>>>>> +          minItems: 1
>>>>>>> +          maxItems: 1
>>>>>>> +
>>>>>>> +        reset-names:
>>>>>>> +          items:
>>>>>>> +            - const: core
>>>>>> This is just maxItems: 1.
>>>>> Well the implicit rules of DT binding schemas are quite
>>>>> weird...
>>>> I don't think it is that strange, as the binding has
>>>>    reset-names:
>>>>      items:
>>>>        - const: core
>>>>        - const: axi
>>>>        - const: ahb
>>> Ah does the list constraint the order of items? If it constrains
>>> the
>> It does, yes.
>> Alternatively, using an enum permits free ordering.
> Ah in this case this should be converted to an enum, I think.
>
> Should I send a patch for converting it?
>
> Thanks,
> Icenowy
Thank you all for the detailed review and discussion, it really helped
clarify the right approach.

Since I will supply all four clocks with the same phandle for core/axi/ahb,
and only one reset "core" for MA35D1, the ordering constraint in the
`items` list is not a problem, "core" is already the first entry. There
is no need to convert to an enum.

Regarding the clock situation for the MA35D1: I agree with supplying all
four clocks (core, axi, ahb, pix0) in the devicetree, even though the
MA35D1 clock controller gates core/axi/ahb with a single bit. The DT will
use the same clock phandle for core, axi, and ahb:

   clocks = <&clk X>, <&clk X>, <&clk X>, <&pix_clk Y>;
   clock-names = "core", "axi", "ahb", "pix0";

This correctly models the hardware topology. Since all three names resolve
to the same underlying clock node, the CCF's standard enable refcounting
handles the shared gate correctly without any custom implementation needed.
I will also revert the change in patch 4/7 that made axi and ahb clocks
optional, since they will now always be provided in the devicetree.

Regarding moving `resets` and `reset-names` to the top-level `required:`,
I will wait for Icenowy's patch to land before sending v6 to avoid
duplicating the work.

In v6 I will update patch 1/7 with:
- Update the subject to "dt-bindings: display: verisilicon,dc: add
   support for nuvoton,ma35d1-dcu"
- Lower `clocks`/`clock-names` `minItems` to 4 at the top level
- Remove the `thead,th1520-dc8200` conditional block entirely
- Keep only the `nuvoton,ma35d1-dcu` conditional block, using only
   `maxItems: 4` for clocks/clock-names and `maxItems: 1` for
   resets/reset-names to tighten the top-level constraints

BR,
Joey
>>> order, it partly breaks the intention of having names; if it does
>>> not
>>> constrain the order, it needs to be clarified that the required 1
>>> reset
>>> is core instead of the other two.
>> Given the discussion we're having on the clocks, I wonder if this is
>> also an oversimplification and the IP has three resets inputs hooked
>> up
>> to one output of the reset controller (or 3 outputs controlled by one
>> bit..).


^ permalink raw reply

* Re: [PATCH v5 4/7] drm/verisilicon: make axi and ahb clocks optional
From: Joey Lu @ 2026-06-29  3:48 UTC (permalink / raw)
  To: Icenowy Zheng, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, robh, krzk+dt, conor+dt
  Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <cf9af38dfe52548649b9010f298341d4064aaa21.camel@iscas.ac.cn>


On 6/26/2026 4:03 PM, Icenowy Zheng wrote:
> 在 2026-06-25四的 17:44 +0800,Joey Lu写道:
>> The Nuvoton MA35D1 SoC integrates a DCUltraLite display controller
>> whose
>> AXI and AHB bus clocks share a single gate enable bit with the
>> display
>> core clock, so the clock driver does not expose them separately. This
>> patch makes the axi and ahb clocks optional in the probe.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ```
> Reviewed-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> ```
>
> Thanks,
> Icenowy
Thank you for the review. I will revert this patch entirely. As discussed
in the binding review, axi and ahb clocks will now always be supplied in
the devicetree using the same phandle as the core clock gate, so making
them optional in the driver is no longer needed.
>> ---
>>   drivers/gpu/drm/verisilicon/vs_dc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
>> b/drivers/gpu/drm/verisilicon/vs_dc.c
>> index 9729b693d360..fd1f5fe67a68 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
>> @@ -90,13 +90,13 @@ static int vs_dc_probe(struct platform_device
>> *pdev)
>>   		return PTR_ERR(dc->core_clk);
>>   	}
>>   
>> -	dc->axi_clk = devm_clk_get_enabled(dev, "axi");
>> +	dc->axi_clk = devm_clk_get_optional_enabled(dev, "axi");
>>   	if (IS_ERR(dc->axi_clk)) {
>>   		dev_err(dev, "can't get axi clock\n");
>>   		return PTR_ERR(dc->axi_clk);
>>   	}
>>   
>> -	dc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
>> +	dc->ahb_clk = devm_clk_get_optional_enabled(dev, "ahb");
>>   	if (IS_ERR(dc->ahb_clk)) {
>>   		dev_err(dev, "can't get ahb clock\n");
>>   		return PTR_ERR(dc->ahb_clk);


^ permalink raw reply

* Re: [PATCH v5 3/7] drm/verisilicon: introduce per-variant hardware ops table
From: Joey Lu @ 2026-06-29  3:52 UTC (permalink / raw)
  To: Icenowy Zheng, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, robh, krzk+dt, conor+dt
  Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <c842f858313732bd774abd2840cc97f730ac2e9f.camel@iscas.ac.cn>


On 6/26/2026 4:02 PM, Icenowy Zheng wrote:
> 在 2026-06-25四的 17:44 +0800,Joey Lu写道:
>> The DC8200 and DCUltraLite share a broadly similar register layout
>> but
>> differ in how the bridge, CRTC, primary plane and IRQ paths are
>> driven.
>> Introduce a vs_dc_funcs vtable so each variant can supply its own
>> implementation without scattering conditionals across multiple files.
>>
>> Add a generation field to struct vs_chip_identity to distinguish
>> variants.
>> Extract the DC8200-specific hardware ops into vs_dc8200.c and add
>> unified
>> IRQ bit definitions so implementations can translate hardware-
>> specific
>> bits to a common set.  Update the shared code to dispatch through
>> dc->funcs.
>>
>> No behaviour change for existing DC8200 platforms.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>> ---
>>   drivers/gpu/drm/verisilicon/Makefile          |   2 +-
>>   drivers/gpu/drm/verisilicon/vs_bridge.c       |  20 +--
>>   drivers/gpu/drm/verisilicon/vs_crtc.c         |  38 +++++-
>>   drivers/gpu/drm/verisilicon/vs_dc.c           |   6 +-
>>   drivers/gpu/drm/verisilicon/vs_dc.h           |  32 +++++
>>   drivers/gpu/drm/verisilicon/vs_dc8200.c       | 115
>> ++++++++++++++++++
>>   drivers/gpu/drm/verisilicon/vs_drm.c          |   5 +-
>>   drivers/gpu/drm/verisilicon/vs_drm.h          |   8 ++
>>   drivers/gpu/drm/verisilicon/vs_hwdb.c         |   4 +
>>   drivers/gpu/drm/verisilicon/vs_hwdb.h         |   6 +
>>   .../gpu/drm/verisilicon/vs_primary_plane.c    |  32 +----
>>   11 files changed, 214 insertions(+), 54 deletions(-)
>>   create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8200.c
>>
>> diff --git a/drivers/gpu/drm/verisilicon/Makefile
>> b/drivers/gpu/drm/verisilicon/Makefile
>> index 426f4bcaa834..9d4cd16452fa 100644
>> --- a/drivers/gpu/drm/verisilicon/Makefile
>> +++ b/drivers/gpu/drm/verisilicon/Makefile
>> @@ -1,6 +1,6 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   
>> -verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_drm.o
>> vs_hwdb.o \
>> +verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
>> vs_drm.o vs_hwdb.o \
>>   	vs_plane.o vs_primary_plane.o vs_cursor_plane.o
>>   
>>   obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
>> diff --git a/drivers/gpu/drm/verisilicon/vs_bridge.c
>> b/drivers/gpu/drm/verisilicon/vs_bridge.c
>> index dc7c85b07fe3..3fbc8d57f8a1 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_bridge.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_bridge.c
>> @@ -162,15 +162,8 @@ static void vs_bridge_enable_common(struct
>> vs_crtc *crtc,
>>   			VSDC_DISP_PANEL_CONFIG_DE_EN |
>>   			VSDC_DISP_PANEL_CONFIG_DAT_EN |
>>   			VSDC_DISP_PANEL_CONFIG_CLK_EN);
>> -	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
>> -			VSDC_DISP_PANEL_CONFIG_RUNNING);
>> -	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
>> -			  VSDC_DISP_PANEL_START_MULTI_DISP_SYNC);
>> -	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_START,
>> -			VSDC_DISP_PANEL_START_RUNNING(output));
>> -
>> -	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(crtc-
>>> id),
>> -			VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
>> +
>> +	dc->funcs->panel_enable_ex(dc, output);
>>   }
>>   
>>   static void vs_bridge_atomic_enable_dpi(struct drm_bridge *bridge,
>> @@ -228,14 +221,7 @@ static void vs_bridge_atomic_disable(struct
>> drm_bridge *bridge,
>>   	struct vs_dc *dc = crtc->dc;
>>   	unsigned int output = crtc->id;
>>   
>> -	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
>> -			  VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
>> -			  VSDC_DISP_PANEL_START_RUNNING(output));
>> -	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
>> -			  VSDC_DISP_PANEL_CONFIG_RUNNING);
>> -
>> -	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(crtc-
>>> id),
>> -			VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
>> +	dc->funcs->panel_disable_ex(dc, output);
>>   }
>>   
>>   static const struct drm_bridge_funcs vs_dpi_bridge_funcs = {
>> diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c
>> b/drivers/gpu/drm/verisilicon/vs_crtc.c
>> index 0b8a35d09cd2..1c4aac708669 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_crtc.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
>> @@ -16,10 +16,33 @@
>>   #include "vs_crtc_regs.h"
>>   #include "vs_crtc.h"
>>   #include "vs_dc.h"
>> -#include "vs_dc_top_regs.h"
>>   #include "vs_drm.h"
>>   #include "vs_plane.h"
>>   
>> +static void vs_crtc_atomic_begin(struct drm_crtc *crtc,
>> +				  struct drm_atomic_commit *state)
>> +{
>> +	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
>> +	struct vs_dc *dc = vcrtc->dc;
>> +	unsigned int output = vcrtc->id;
>> +
>> +	if (dc->funcs->crtc_begin)
>> +		dc->funcs->crtc_begin(dc, output);
>> +}
>> +
>> +static void vs_crtc_atomic_flush(struct drm_crtc *crtc,
>> +				  struct drm_atomic_commit *state)
>> +{
>> +	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
>> +	struct vs_dc *dc = vcrtc->dc;
>> +	unsigned int output = vcrtc->id;
>> +
>> +	if (dc->funcs->crtc_flush)
>> +		dc->funcs->crtc_flush(dc, output);
>> +
>> +	drm_crtc_vblank_atomic_flush(crtc, state);
>> +}
>> +
>>   static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
>>   				   struct drm_atomic_commit *state)
>>   {
>> @@ -30,6 +53,9 @@ static void vs_crtc_atomic_disable(struct drm_crtc
>> *crtc,
>>   	drm_crtc_vblank_off(crtc);
>>   
>>   	clk_disable_unprepare(dc->pix_clk[output]);
>> +
>> +	if (dc->funcs->crtc_disable_ex)
>> +		dc->funcs->crtc_disable_ex(dc, output);
>>   }
>>   
>>   static void vs_crtc_atomic_enable(struct drm_crtc *crtc,
>> @@ -42,6 +68,9 @@ static void vs_crtc_atomic_enable(struct drm_crtc
>> *crtc,
>>   	drm_WARN_ON(&dc->drm_dev->base,
>>   		    clk_prepare_enable(dc->pix_clk[output]));
>>   
>> +	if (dc->funcs->crtc_enable_ex)
>> +		dc->funcs->crtc_enable_ex(dc, output);
>> +
>>   	drm_crtc_vblank_on(crtc);
>>   }
>>   
>> @@ -119,7 +148,8 @@ static bool vs_crtc_mode_fixup(struct drm_crtc
>> *crtc,
>>   }
>>   
>>   static const struct drm_crtc_helper_funcs vs_crtc_helper_funcs = {
>> -	.atomic_flush	= drm_crtc_vblank_atomic_flush,
>> +	.atomic_begin	= vs_crtc_atomic_begin,
>> +	.atomic_flush	= vs_crtc_atomic_flush,
>>   	.atomic_enable	= vs_crtc_atomic_enable,
>>   	.atomic_disable	= vs_crtc_atomic_disable,
>>   	.mode_set_nofb	= vs_crtc_mode_set_nofb,
>> @@ -132,7 +162,7 @@ static int vs_crtc_enable_vblank(struct drm_crtc
>> *crtc)
>>   	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
>>   	struct vs_dc *dc = vcrtc->dc;
>>   
>> -	regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
>> VSDC_TOP_IRQ_VSYNC(vcrtc->id));
>> +	dc->funcs->enable_vblank(dc, vcrtc->id);
>>   
>>   	return 0;
>>   }
>> @@ -142,7 +172,7 @@ static void vs_crtc_disable_vblank(struct
>> drm_crtc *crtc)
>>   	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
>>   	struct vs_dc *dc = vcrtc->dc;
>>   
>> -	regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
>> VSDC_TOP_IRQ_VSYNC(vcrtc->id));
>> +	dc->funcs->disable_vblank(dc, vcrtc->id);
>>   }
>>   
>>   static const struct drm_crtc_funcs vs_crtc_funcs = {
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
>> b/drivers/gpu/drm/verisilicon/vs_dc.c
>> index dad9967bc10b..9729b693d360 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
>> @@ -8,9 +8,7 @@
>>   #include <linux/of.h>
>>   #include <linux/of_graph.h>
>>   
>> -#include "vs_crtc.h"
>>   #include "vs_dc.h"
>> -#include "vs_dc_top_regs.h"
>>   #include "vs_drm.h"
>>   #include "vs_hwdb.h"
>>   
>> @@ -33,7 +31,7 @@ static irqreturn_t vs_dc_irq_handler(int irq, void
>> *private)
>>   	struct vs_dc *dc = private;
>>   	u32 irqs;
>>   
>> -	regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &irqs);
>> +	irqs = dc->funcs->irq_ack(dc);
>>   
>>   	vs_drm_handle_irq(dc, irqs);
>>   
>> @@ -136,6 +134,8 @@ static int vs_dc_probe(struct platform_device
>> *pdev)
>>   	dev_info(dev, "Found DC%x rev %x customer %x\n", dc-
>>> identity.model,
>>   		 dc->identity.revision, dc->identity.customer_id);
>>   
>> +	dc->funcs = &vs_dc8200_funcs;
>> +
>>   	if (port_count > dc->identity.display_count) {
>>   		dev_err(dev, "too many downstream ports than HW
>> capability\n");
>>   		ret = -EINVAL;
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h
>> b/drivers/gpu/drm/verisilicon/vs_dc.h
>> index ed1016f18758..825f5dd6bf17 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_dc.h
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc.h
>> @@ -14,6 +14,7 @@
>>   #include <linux/reset.h>
>>   
>>   #include <drm/drm_device.h>
>> +#include <drm/drm_plane.h>
>>   
>>   #include "vs_hwdb.h"
>>   
>> @@ -22,6 +23,34 @@
>>   
>>   struct vs_drm_dev;
>>   struct vs_crtc;
>> +struct vs_dc;
>> +
>> +struct vs_dc_funcs {
>> +	/* Bridge: atomic_enable, atomic_disable */
>> +	void (*panel_enable_ex)(struct vs_dc *dc, unsigned int
>> output);
>> +	void (*panel_disable_ex)(struct vs_dc *dc, unsigned int
>> output);
>> +
>> +	/* CRTC: atomic_begin, atomic_flush */
>> +	void (*crtc_begin)(struct vs_dc *dc, unsigned int output);
>> +	void (*crtc_flush)(struct vs_dc *dc, unsigned int output);
>> +
>> +	/* CRTC: atomic_enable, atomic_disable */
>> +	void (*crtc_enable_ex)(struct vs_dc *dc, unsigned int
>> output);
>> +	void (*crtc_disable_ex)(struct vs_dc *dc, unsigned int
>> output);
>> +
>> +	/* CRTC: enable_vblank, disable_vblank */
>> +	void (*enable_vblank)(struct vs_dc *dc, unsigned int
>> output);
>> +	void (*disable_vblank)(struct vs_dc *dc, unsigned int
>> output);
>> +
>> +	/* Primary plane: atomic_enable, atomic_disable,
>> atomic_update */
>> +	void (*primary_plane_enable_ex)(struct vs_dc *dc, unsigned
>> int output);
>> +	void (*primary_plane_disable_ex)(struct vs_dc *dc, unsigned
>> int output);
>> +	void (*primary_plane_update_ex)(struct vs_dc *dc, unsigned
>> int output,
>> +					struct drm_plane_state
>> *state);
>> +
>> +	/* IRQ acknowledge */
>> +	u32 (*irq_ack)(struct vs_dc *dc);
>> +};
>>   
>>   struct vs_dc {
>>   	struct regmap *regs;
>> @@ -33,6 +62,9 @@ struct vs_dc {
>>   
>>   	struct vs_drm_dev *drm_dev;
>>   	struct vs_chip_identity identity;
>> +	const struct vs_dc_funcs *funcs;
>>   };
>>   
>> +extern const struct vs_dc_funcs vs_dc8200_funcs;
>> +
>>   #endif /* _VS_DC_H_ */
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc8200.c
>> b/drivers/gpu/drm/verisilicon/vs_dc8200.c
>> new file mode 100644
>> index 000000000000..17378f4ef96d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc8200.c
>> @@ -0,0 +1,115 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2025 Icenowy Zheng <uwu@icenowy.me>
>> + */
>> +
>> +#include <linux/regmap.h>
>> +
>> +#include "vs_bridge_regs.h"
>> +#include "vs_dc.h"
>> +#include "vs_dc_top_regs.h"
>> +#include "vs_drm.h"
>> +#include "vs_plane.h"
>> +#include "vs_primary_plane_regs.h"
>> +
>> +static void vs_dc8200_panel_enable_ex(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
>> +			VSDC_DISP_PANEL_CONFIG_RUNNING);
>> +	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
>> +			  VSDC_DISP_PANEL_START_MULTI_DISP_SYNC);
>> +	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_START,
>> +			VSDC_DISP_PANEL_START_RUNNING(output));
>> +
>> +	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(output),
>> +			VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
>> +}
>> +
>> +static void vs_dc8200_panel_disable_ex(struct vs_dc *dc, unsigned
>> int output)
>> +{
>> +	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
>> +			  VSDC_DISP_PANEL_CONFIG_RUNNING);
>> +	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
>> +			  VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
>> +			  VSDC_DISP_PANEL_START_RUNNING(output));
>> +
>> +	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG_EX(output),
>> +			VSDC_DISP_PANEL_CONFIG_EX_COMMIT);
>> +}
>> +
>> +static void vs_dc8200_enable_vblank(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_TOP_IRQ_EN,
>> +			VSDC_TOP_IRQ_VSYNC(output));
>> +}
>> +
>> +static void vs_dc8200_disable_vblank(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_clear_bits(dc->regs, VSDC_TOP_IRQ_EN,
>> +			  VSDC_TOP_IRQ_VSYNC(output));
>> +}
>> +
>> +static void vs_dc8200_plane_commit(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
>> +			VSDC_FB_CONFIG_EX_COMMIT);
>> +}
>> +
>> +static void vs_dc8200_primary_plane_enable_ex(struct vs_dc *dc,
>> unsigned int output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
>> +			VSDC_FB_CONFIG_EX_FB_EN);
>> +	regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
>> +			   VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
>> +			   VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
>> +
>> +	vs_dc8200_plane_commit(dc, output);
>> +}
>> +
>> +static void vs_dc8200_primary_plane_disable_ex(struct vs_dc *dc,
>> unsigned int output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
>> +			VSDC_FB_CONFIG_EX_FB_EN);
>> +
>> +	vs_dc8200_plane_commit(dc, output);
>> +}
>> +
>> +static void vs_dc8200_primary_plane_update_ex(struct vs_dc *dc,
>> unsigned int output,
>> +				       struct drm_plane_state
>> *state)
>> +{
>> +	regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
>> +		     VSDC_MAKE_PLANE_POS(state->crtc_x, state-
>>> crtc_y));
>> +	regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
>> +		     VSDC_MAKE_PLANE_POS(state->crtc_x + state-
>>> crtc_w,
>> +					 state->crtc_y + state-
>>> crtc_h));
>> +	regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
>> +		     VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
>> +
>> +	vs_dc8200_plane_commit(dc, output);
>> +}
>> +
>> +static u32 vs_dc8200_irq_ack(struct vs_dc *dc)
>> +{
>> +	u32 hw_irqs, unified = 0;
>> +	unsigned int i;
>> +
>> +	regmap_read(dc->regs, VSDC_TOP_IRQ_ACK, &hw_irqs);
>> +
>> +	for (i = 0; i < VSDC_MAX_OUTPUTS; i++) {
>> +		if (hw_irqs & VSDC_TOP_IRQ_VSYNC(i))
>> +			unified |= VSDC_IRQ_VSYNC(i);
>> +	}
> Maybe add a drm_WARN_ONCE for unknown hardware IRQ bit?
>
> Well, with this addressed,
>
> ```
> Reviewed-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> ```
>
> Thanks,
> Icenowy
I will add a `drm_WARN_ONCE` after the IRQ translation loop in
`vs_dc8200_irq_ack` to catch any unknown hardware IRQ bits.



^ permalink raw reply

* Re: [PATCH v5 5/7] drm/verisilicon: add DC8000 (DCUltraLite) display controller support
From: Joey Lu @ 2026-06-29  3:54 UTC (permalink / raw)
  To: Icenowy Zheng, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, robh, krzk+dt, conor+dt
  Cc: ychuang3, schung, yclu4, dri-devel, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <39e7d0426d1690ccfc8dbcfc77e7440bc5c9a725.camel@iscas.ac.cn>


On 6/26/2026 4:03 PM, Icenowy Zheng wrote:
> 在 2026-06-25四的 17:44 +0800,Joey Lu写道:
>> The Nuvoton MA35D1 SoC integrates a Verisilicon DCUltraLite display
>> controller (DC8000 generation) whose register layout differs from
>> the DC8200 in several important ways:
>>
>> 1. No CONFIG_EX commit path: framebuffer updates use the enable (bit
>> 0)
>>     and reset (bit 4) bits in FB_CONFIG instead of the DC8200 staging
>>     registers (FB_CONFIG_EX, FB_TOP_LEFT, FB_BOTTOM_RIGHT,
>>     FB_BLEND_CONFIG, PANEL_CONFIG_EX).
>>
>> 2. No PANEL_START register: panel output starts when
>>     PANEL_CONFIG.RUNNING is set; there is no multi-display sync start
>>     register.
>>
>> 3. Different IRQ registers: DCUltraLite uses DISP_IRQ_STA (0x147C) /
>>     DISP_IRQ_EN (0x1480) versus DC8200's TOP_IRQ_ACK (0x0010) /
>>     TOP_IRQ_EN (0x0014).
>>
>> 4. Simpler clock topology: only 'core' (bus gate) and 'pix0' (pixel
>>     divider) clocks; no axi or ahb clocks required.
>>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>> ---
>>   drivers/gpu/drm/verisilicon/Makefile    |  2 +-
>>   drivers/gpu/drm/verisilicon/vs_dc.c     |  5 +-
>>   drivers/gpu/drm/verisilicon/vs_dc.h     |  1 +
>>   drivers/gpu/drm/verisilicon/vs_dc8000.c | 86
>> +++++++++++++++++++++++++
>>   4 files changed, 92 insertions(+), 2 deletions(-)
>>   create mode 100644 drivers/gpu/drm/verisilicon/vs_dc8000.c
>>
>> diff --git a/drivers/gpu/drm/verisilicon/Makefile
>> b/drivers/gpu/drm/verisilicon/Makefile
>> index 9d4cd16452fa..d2fd8e4dff24 100644
>> --- a/drivers/gpu/drm/verisilicon/Makefile
>> +++ b/drivers/gpu/drm/verisilicon/Makefile
>> @@ -1,6 +1,6 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   
>> -verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
>> vs_drm.o vs_hwdb.o \
>> +verisilicon-dc-objs := vs_bridge.o vs_crtc.o vs_dc.o vs_dc8200.o
>> vs_dc8000.o vs_drm.o vs_hwdb.o \
>>   	vs_plane.o vs_primary_plane.o vs_cursor_plane.o
>>   
>>   obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon-dc.o
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c
>> b/drivers/gpu/drm/verisilicon/vs_dc.c
>> index fd1f5fe67a68..9499fffbca58 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_dc.c
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc.c
>> @@ -134,7 +134,10 @@ static int vs_dc_probe(struct platform_device
>> *pdev)
>>   	dev_info(dev, "Found DC%x rev %x customer %x\n", dc-
>>> identity.model,
>>   		 dc->identity.revision, dc->identity.customer_id);
>>   
>> -	dc->funcs = &vs_dc8200_funcs;
>> +	if (dc->identity.generation == VSDC_GEN_DC8200)
>> +		dc->funcs = &vs_dc8200_funcs;
>> +	else
>> +		dc->funcs = &vs_dc8000_funcs;
>>   
>>   	if (port_count > dc->identity.display_count) {
>>   		dev_err(dev, "too many downstream ports than HW
>> capability\n");
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc.h
>> b/drivers/gpu/drm/verisilicon/vs_dc.h
>> index 825f5dd6bf17..ac96ad701199 100644
>> --- a/drivers/gpu/drm/verisilicon/vs_dc.h
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc.h
>> @@ -66,5 +66,6 @@ struct vs_dc {
>>   };
>>   
>>   extern const struct vs_dc_funcs vs_dc8200_funcs;
>> +extern const struct vs_dc_funcs vs_dc8000_funcs;
>>   
>>   #endif /* _VS_DC_H_ */
>> diff --git a/drivers/gpu/drm/verisilicon/vs_dc8000.c
>> b/drivers/gpu/drm/verisilicon/vs_dc8000.c
>> new file mode 100644
>> index 000000000000..fbe0fa516cac
>> --- /dev/null
>> +++ b/drivers/gpu/drm/verisilicon/vs_dc8000.c
>> @@ -0,0 +1,86 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2026 Joey Lu <yclu4@nuvoton.com>
>> + */
>> +
>> +#include <linux/regmap.h>
>> +
>> +#include "vs_crtc_regs.h"
>> +#include "vs_dc.h"
>> +#include "vs_drm.h"
>> +#include "vs_primary_plane_regs.h"
>> +
>> +static void vs_dc8000_panel_enable_ex(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
>> +			VSDC_FB_CONFIG_RESET);
>> +}
>> +
>> +static void vs_dc8000_panel_disable_ex(struct vs_dc *dc, unsigned
>> int output)
>> +{
>> +	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
>> +			  VSDC_FB_CONFIG_RESET);
>> +}
>> +
>> +static void vs_dc8000_crtc_begin(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
>> +			VSDC_FB_CONFIG_VALID);
>> +}
>> +
>> +static void vs_dc8000_crtc_flush(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
>> +			  VSDC_FB_CONFIG_VALID);
>> +}
>> +
>> +static void vs_dc8000_crtc_enable_ex(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_FB_CONFIG(output),
>> +			VSDC_FB_CONFIG_ENABLE);
>> +}
>> +
>> +static void vs_dc8000_crtc_disable_ex(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG(output),
>> +			  VSDC_FB_CONFIG_ENABLE);
>> +}
>> +
>> +static void vs_dc8000_enable_vblank(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_set_bits(dc->regs, VSDC_DISP_IRQ_EN,
>> +			VSDC_DISP_IRQ_VSYNC(output));
>> +}
>> +
>> +static void vs_dc8000_disable_vblank(struct vs_dc *dc, unsigned int
>> output)
>> +{
>> +	regmap_clear_bits(dc->regs, VSDC_DISP_IRQ_EN,
>> +			  VSDC_DISP_IRQ_VSYNC(output));
>> +}
>> +
>> +static u32 vs_dc8000_irq_ack(struct vs_dc *dc)
>> +{
>> +	u32 hw_irqs, unified = 0;
>> +	unsigned int i;
>> +
>> +	regmap_read(dc->regs, VSDC_DISP_IRQ_STA, &hw_irqs);
>> +
>> +	for (i = 0; i < VSDC_MAX_OUTPUTS; i++) {
>> +		if (hw_irqs & VSDC_DISP_IRQ_VSYNC(i))
>> +			unified |= VSDC_IRQ_VSYNC(i);
>> +	}
> Maybe a warning for unknown IRQ bits should be added here.
>
> Thanks,
> Icenowy
I will add the same `drm_WARN_ONCE` pattern in `vs_dc8000_irq_ack` for
unknown `VSDC_DISP_IRQ_STA` bits.
>> +
>> +	return unified;
>> +}
>> +
>> +const struct vs_dc_funcs vs_dc8000_funcs = {
>> +	.panel_enable_ex	= vs_dc8000_panel_enable_ex,
>> +	.panel_disable_ex	= vs_dc8000_panel_disable_ex,
>> +	.crtc_begin		= vs_dc8000_crtc_begin,
>> +	.crtc_flush		= vs_dc8000_crtc_flush,
>> +	.crtc_enable_ex		= vs_dc8000_crtc_enable_ex,
>> +	.crtc_disable_ex	= vs_dc8000_crtc_disable_ex,
>> +	.enable_vblank		= vs_dc8000_enable_vblank,
>> +	.disable_vblank		= vs_dc8000_disable_vblank,
>> +	.irq_ack		= vs_dc8000_irq_ack,
>> +};


^ permalink raw reply

* Re: [PATCH v3 00/12] arm64: Add HOTPLUG_PARALLEL support for secondary CPUs
From: Shrikanth Hegde @ 2026-06-29  4:04 UTC (permalink / raw)
  To: Jinjie Ruan, catalin.marinas, will, tsbogend, tglx, mingo, bp,
	dave.hansen, hpa, peterz, kees, nathan, linusw, ojeda,
	david.kaplan, lukas.bulwahn, ryan.roberts, maz, timothy.hayes,
	lpieralisi, thuth, menglong8.dong, oupton, yeoreum.yun,
	miko.lenczewski, broonie, kevin.brodsky, james.clark, yangyicong,
	tabba, osandov, arnd, anshuman.khandual, david, akpm, ljs,
	dev.jain, yang, chaitanyas.prakash, kprateek.nayak, chenl311,
	thorsten.blum, chang.seok.bae, tim.c.chen, x86, linux-kernel,
	linux-arm-kernel, linux-mips
In-Reply-To: <20260624092537.2916971-1-ruanjinjie@huawei.com>



On 6/24/26 2:55 PM, Jinjie Ruan wrote:
> Support for parallel secondary CPU bringup is already utilized by x86,
> MIPS, and RISC-V. This patch brings this capability to the arm64
> architecture.
> 
> Introduce CONFIG_HOTPLUG_PARALLEL_SMT to avoid primary SMT threads
> to boot first constraint.
> 
> And add a 'cpu' parameter to update_cpu_boot_status() to allow updating
> the boot status at a per-CPU granularity during parallel bringup.
> 
> Rework the global `secondary_data` and `__early_cpu_boot_status` accessed
> during early boot into per-CPU arrays to allow secondary CPUs to boot
> in parallel.
> 

Shouldn't this be called CONFIG_BRINGUP_PARALLEL instead?
Hotplug usually means disable/enable CPU at runtime.


^ permalink raw reply

* Re: [PATCH rc v6 1/7] iommu/arm-smmu-v3: Add arm_smmu_kdump_adopt_strtab() for kdump
From: Nicolin Chen @ 2026-06-29  4:01 UTC (permalink / raw)
  To: Pranjal Shrivastava
  Cc: will, robin.murphy, jgg, joro, kees, baolu.lu, kevin.tian,
	miko.lenczewski, smostafa, linux-arm-kernel, iommu, linux-kernel,
	stable, jamien
In-Reply-To: <akGnm_dzvoapjwoI@google.com>

On Sun, Jun 28, 2026 at 11:00:43PM +0000, Pranjal Shrivastava wrote:
> On Wed, May 20, 2026 at 10:03:18AM -0700, Nicolin Chen wrote:
> > +static int arm_smmu_kdump_adopt_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
> > +					  phys_addr_t base, u32 span,
> > +					  struct arm_smmu_strtab_l2 **l2table)
[...]
> > +
> > +	/*
> > +	 * Retest the span in case the L1 descriptor has been overwritten since
> > +	 * the adopt. Reject this master's insert; panic or SMMU-disable would
> > +	 * either lose the vmcore or cascade aborts. Do not try to fix it, as it
> > +	 * would break all other SIDs in the same bus (PCI case). The corruption
> > +	 * blast radius is already bounded to that bus range.
> > +	 */
> > +	if (span != STRTAB_SPLIT + 1) {
> > +		dev_err(smmu->dev,
> > +			"kdump: L1[%u] span %u changed since adopt (was %u)\n",
> > +			arm_smmu_strtab_l1_idx(sid), span, STRTAB_SPLIT + 1);
> > +		return -EINVAL;
> > +	}
> > +
> > +	size = (1UL << (span - 1)) * sizeof(struct arm_smmu_ste);
> > +
> > +	table = devm_memremap(smmu->dev, base, size, MEMREMAP_WB);
> 
> Why do we use devm_memremap() here but memremap() in the rest of the
> functions (even for the L1 ptr)?

Because it's called by arm_smmu_init_l2_strtab() that is a later
stage (arm_smmu_insert_master) than the other adopt functions.

This is deliberate. Otherwise, no one would undo memremap.

I can add a line of note here.

> > +static int arm_smmu_kdump_adopt_strtab_2lvl(struct arm_smmu_device *smmu,
> > +					    u32 cfg_reg, phys_addr_t base)
[...]
> > +
> > +	cfg->l2.l2ptrs =
> > +		kcalloc(num_l1_ents, sizeof(*cfg->l2.l2ptrs), GFP_KERNEL);
> > +	if (!cfg->l2.l2ptrs)
> > +		return -ENOMEM;
> 
> We shuold ummap cfg->l2.l1tab before returning -ENOMEM here

No. The caller would call cleanup() on the -ENOMEM.

> > +		if (span != STRTAB_SPLIT + 1) {
> > +			dev_err(smmu->dev,
> > +				"kdump: L1[%u] unsupported span %u (vs %u)\n",
> > +				i, span, STRTAB_SPLIT + 1);
> > +			return -EINVAL;
> 
> We leak kcalloc'd mem (l2.l2ptrs) here, also we should unmap cfg->l2.l1tab

Ditto.

> > +static int arm_smmu_kdump_adopt_strtab_linear(struct arm_smmu_device *smmu,
> > +					      u32 cfg_reg, phys_addr_t base)
> > +{
> > +	u32 log2size = FIELD_GET(STRTAB_BASE_CFG_LOG2SIZE, cfg_reg);
> > +	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> > +	unsigned int max_log2size;
> > +	size_t size;
> > +
> > +	/*
> > +	 * Only a coherent SMMU is supported at this moment. For a non-coherent
> > +	 * SMMU that wants to support ARM_SMMU_OPT_KDUMP_ADOPT, try MEMREMAP_WC.
> > +	 */
> > +	if (WARN_ON(!(smmu->features & ARM_SMMU_FEAT_COHERENCY)))
> > +		return -EOPNOTSUPP;
> > +
> > +	/* Cap the size at what the kdump kernel itself would have allocated */
> > +	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
> > +		max_log2size =
> > +			ilog2(STRTAB_MAX_L1_ENTRIES * STRTAB_NUM_L2_STES);
> 
> Looks like we'd never hit this if condition because we'd never support a
> "linear" strtab if the HW supports ARM_SMMU_FEAT_2_LVL_STRTAB. Please
> see arm_smmu_init_strtab:

It's a defensive check that Sashiko recommended, as we cannot be sure
that the crashed kernel was using the same driver, which I think it's
not bad to have..

> > +	size = cfg->linear.num_ents * sizeof(struct arm_smmu_ste);
> > +	cfg->linear.table = memremap(base, size, MEMREMAP_WB);
> > +	if (!cfg->linear.table)
> > +		return -ENOMEM;
> 
> We seem to skips initializing cfg->linear.ste_dma (it is populated in
> arm_smmu_init_strtab_linear)

Yes, deliberately since it's not used anywhere in kdump adopt mode.

> While the comment notes that arm_smmu_write_strtab() is bypassed in the
> kdump case, leaving cfg->linear.ste_dma uninitialized seems like a 
> ticking time bomb if any other part of the driver ever uses it.

I can't think of a reason...

> > +static void arm_smmu_kdump_adopt_cleanup(void *data)
> > +{
> > +	struct arm_smmu_device *smmu = data;
> > +	u32 cfg_reg = readl_relaxed(smmu->base + ARM_SMMU_STRTAB_BASE_CFG);
> 
> I'm worried about reading the HW register here, since this is a devres action, it
> can run after arm_smmu_device_remove() or arm_smmu_device_shutdown()
> (which would call rpm_put()). Please see __device_release_driver[1]:
> 
> 	pm_runtime_put_sync(dev); <--- HW turned off
> 
> 	device_remove(dev);
> 
> 	if (dev->bus && dev->bus->dma_cleanup)
> 		dev->bus->dma_cleanup(dev);
> 
> 	device_unbind_cleanup(dev); <--- This is where devm_release runs
> 	device_links_driver_cleanup(dev);
> 
> Thus, even if we call rpm_get() here it would make no sense as the 
> register contents would've been lost. Can we rely on some SW state
> here? smmu->features & 2LVL or maybe add an fmt in cfg? 

Yes. Sashiko pointed it out too. We can do feature bit.

> > +		 * format, enforce the same format to match the adopted table.
> > +		 */
> > +		ret = arm_smmu_kdump_adopt_strtab_linear(smmu, cfg_reg, base);
> > +		if (!ret)
> > +			smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;
> 
> IIRC, this should NOT be set if we selected the linear format.
> Looking at arm_smmu_init_strtab(), if the HW supports 2-level tables,
> the driver unconditionally selects it. What is the expected scenario
> where the previous kernel would have allocated a linear table on 2-level
> capable hardware? IMO, it is a bug if we see linear fmt with this
> feature set. Am I missing something?

Again, the crashed kernel doesn't have to use the same driver.

I will address the rest of your comments.

Thanks
Nicolin


^ permalink raw reply

* Re: [PATCH v4 0/3] iommu/arm-smmu-v3: Tegra264 invalidation workaround
From: Ashish Mhetre @ 2026-06-29  4:11 UTC (permalink / raw)
  To: Jon Hunter, Nicolin Chen, Will Deacon, Robin Murphy, Joerg Roedel,
	Jason Gunthorpe
  Cc: linux-arm-kernel, iommu, linux-kernel, linux-tegra
In-Reply-To: <96959efb-051c-411a-810f-927943d5491d@nvidia.com>



On 6/17/2026 1:51 PM, Jon Hunter wrote:
>
> On 17/06/2026 05:54, Nicolin Chen wrote:
>> On Wed, Jun 17, 2026 at 09:28:10AM +0530, Ashish Mhetre wrote:
>>> On 6/9/2026 1:02 PM, Ashish Mhetre wrote:
>>> Hi all,
>>>
>>> A gentle reminder to review the patches and share your comments.
>>
>> https://docs.kernel.org/process/maintainer-tip.html
>>
>> "
>> 4.2.9 Merge window
>>
>> Please do not expect patches to be reviewed or merged by tip
>> maintainers around or during the merge window. The trees are closed
>> to all but urgent fixes during this time. They reopen once the merge
>> window closes and a new -rc1 kernel has been released.
>> "
>>
>> I would wait for rc1 to rebase and respin.
>
> Assuming that they need to be rebased. If they apply cleanly to -next 
> after -rc1 is out, then there is no need to rebase. However, 
> definitely check.
>
> Jon
>

The patches apply cleanly on linux-next-20260626 and rebase is not
needed. Can you please let me know if the series is good for review
or should I rebase on latest linux-next and resend?

Thanks,
Ashish Mhetre


^ permalink raw reply

* Re: [PATCH v14 0/7] Provide support for Trigger Generation Unit
From: Greg KH @ 2026-06-29  4:22 UTC (permalink / raw)
  To: Songwei.Chai
  Cc: andersson, alexander.shishkin, mike.leach, konrad.dybcio,
	suzuki.poulose, james.clark, krzk+dt, conor+dt, linux-kernel,
	linux-arm-kernel, linux-arm-msm, coresight, devicetree
In-Reply-To: <a36a1ed3-5194-465c-b029-0404e0f2bcee@oss.qualcomm.com>

On Mon, Jun 29, 2026 at 11:03:33AM +0800, Songwei.Chai wrote:
> Hi Greg & Alexander,
> 
> Apologies for interrupting again.
> 
> As the TGU hardware plays an important role in Qualcomm tracing design, I
> would greatly appreciate it if you could kindly take some time to review
> this at your earliest convenience.

The merge window _just_ closed, please give us a chance to catch up.

Also, why us?  Surely you have other reviewers for this code, right?

thanks,

greg k-h


^ permalink raw reply

* Re: [PATCH v4 1/6] arm64/hugetlb: Extend batching of multiple CONT_PTE in a single PTE setup
From: Dev Jain @ 2026-06-29  5:34 UTC (permalink / raw)
  To: Wen Jiang, linux-mm, linux-arm-kernel, catalin.marinas, will,
	akpm, urezki
  Cc: baohua, Xueyuan.chen21, rppt, david, ryan.roberts,
	anshuman.khandual, ajd, linux-kernel, jiangwen6, shanghaoqiang
In-Reply-To: <20260618084726.1070022-2-jiangwen6@xiaomi.com>



On 18/06/26 2:17 pm, Wen Jiang wrote:
> From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> 
> For sizes aligned to CONT_PTE_SIZE and smaller than PMD_SIZE,
> we can handle CONT_PTE_SIZE groups together.
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  arch/arm64/mm/hugetlbpage.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index a42c05cf56408..c4d8b226126cb 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -110,6 +110,12 @@ static inline int num_contig_ptes(unsigned long size, size_t *pgsize)
>  		contig_ptes = CONT_PTES;
>  		break;
>  	default:
> +		if (size > 0 && size < PMD_SIZE &&
> +				IS_ALIGNED(size, CONT_PTE_SIZE)) {
> +			contig_ptes = size >> PAGE_SHIFT;
> +			*pgsize = PAGE_SIZE;

Nit: *pgsize update can go before contig_ptes, to keep the pattern elsewhere.

Reviewed-by: Dev Jain <dev.jain@arm.com>



> +			break;
> +		}
>  		WARN_ON(!__hugetlb_valid_size(size));
>  	}
>  
> @@ -359,6 +365,10 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>  	case CONT_PTE_SIZE:
>  		return pte_mkcont(entry);
>  	default:
> +		if (pagesize > 0 && pagesize < PMD_SIZE &&
> +				IS_ALIGNED(pagesize, CONT_PTE_SIZE))
> +			return pte_mkcont(entry);
> +
>  		break;
>  	}
>  	pr_warn("%s: unrecognized huge page size 0x%lx\n",



^ permalink raw reply

* Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants
From: Icenowy Zheng @ 2026-06-29  5:31 UTC (permalink / raw)
  To: Joey Lu, Conor Dooley
  Cc: Conor Dooley, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, robh, krzk+dt, conor+dt, ychuang3, schung, yclu4,
	dri-devel, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <b3b7a0f8-93a8-4965-a2f3-3ca1552a25d6@gmail.com>

在 2026-06-29一的 11:47 +0800,Joey Lu写道:
> 
> On 6/26/2026 5:33 PM, Icenowy Zheng wrote:
> > 在 2026-06-26五的 10:26 +0100,Conor Dooley写道:
> > > On Fri, Jun 26, 2026 at 05:00:35PM +0800, Icenowy Zheng wrote:
> > > > 在 2026-06-26五的 08:19 +0100,Conor Dooley写道:
> > > > > On Fri, Jun 26, 2026 at 01:27:21PM +0800, Icenowy Zheng
> > > > > wrote:
> > > > > > 在 2026-06-25四的 17:33 +0100,Conor Dooley写道:
> > > > > > > On Thu, Jun 25, 2026 at 05:44:43PM +0800, Joey Lu wrote:
> > > > > > > > +allOf:
> > > > > > > > +  - if:
> > > > > > > > +      properties:
> > > > > > > > +        compatible:
> > > > > > > > +          contains:
> > > > > > > > +            const: thead,th1520-dc8200
> > > > > > > > +    then:
> > > > > > > > +      properties:
> > > > > > > > +        clocks:
> > > > > > > > +          minItems: 5
> > > > > > > > +          maxItems: 5
> > > > > > > > +
> > > > > > > > +        clock-names:
> > > > > > > > +          minItems: 5
> > > > > > > > +          maxItems: 5
> > > > > > > All the maxItems here repeat the maximum constraint and
> > > > > > > do
> > > > > > > nothing.
> > > > > > > 
> > > > > > > Since you didn't change the minimum constraint at the top
> > > > > > > level,
> > > > > > > your
> > > > > > > minItems also do nothing.
> > > > > > > 
> > > > > > > > +
> > > > > > > > +        resets:
> > > > > > > > +          minItems: 3
> > > > > > > > +          maxItems: 3
> > > > > > > > +
> > > > > > > > +        reset-names:
> > > > > > > > +          minItems: 3
> > > > > > > > +          maxItems: 3
> > > > > > > > +
> > > > > > > > +      required:
> > > > > > > > +        - resets
> > > > > > > > +        - reset-names
> > > > > > > Both conditional sections have this, but the original
> > > > > > > binding
> > > > > > > doesn't
> > > > > > > require these for the thead device. This is a functional
> > > > > > > change
> > > > > > > therefore and shouldn't be in a patch calling itself
> > > > > > > "generalise
> > > > > > > for
> > > > > > > single ended variants".
> > > > > > Well yes they're required.
> > > > > > 
> > > > > > Should I send a patch adding the `thead,th1520-dc8200` part
> > > > > > of
> > > > > > the
> > > > > > schema?
> > > > > If you mean the code above, no. Adding a conditional section
> > > > > when
> > > > > there's only that compatible doesn't make sense.
> > > > > 
> > > > > What you could do is just add it at the top level though,
> > > > > which
> > > > > would
> > > > > also benefit this patch since it'd not have to be
> > > > > conditionally
> > > > > added
> > > > > for the new nuvoton device.
> > > > > Just note in your commit message about what the ABI impact of
> > > > > the
> > > > > change
> > > > > to required properties is (effectively nothing because it's
> > > > > optional
> > > > > in
> > > > > the driver and the only user has the properties).
> > > > Okay, I will craft such a patch and send it.
> > > > 
> > > > > > > > +
> > > > > > > > +        resets:
> > > > > > > > +          minItems: 1
> > > > > > > > +          maxItems: 1
> > > > > > > > +
> > > > > > > > +        reset-names:
> > > > > > > > +          items:
> > > > > > > > +            - const: core
> > > > > > > This is just maxItems: 1.
> > > > > > Well the implicit rules of DT binding schemas are quite
> > > > > > weird...
> > > > > I don't think it is that strange, as the binding has
> > > > >    reset-names:
> > > > >      items:
> > > > >        - const: core
> > > > >        - const: axi
> > > > >        - const: ahb
> > > > Ah does the list constraint the order of items? If it
> > > > constrains
> > > > the
> > > It does, yes.
> > > Alternatively, using an enum permits free ordering.
> > Ah in this case this should be converted to an enum, I think.
> > 
> > Should I send a patch for converting it?
> > 
> > Thanks,
> > Icenowy
> Thank you all for the detailed review and discussion, it really
> helped
> clarify the right approach.
> 
> Since I will supply all four clocks with the same phandle for
> core/axi/ahb,
> and only one reset "core" for MA35D1, the ordering constraint in the
> `items` list is not a problem, "core" is already the first entry.
> There
> is no need to convert to an enum.
> 
> Regarding the clock situation for the MA35D1: I agree with supplying
> all
> four clocks (core, axi, ahb, pix0) in the devicetree, even though the
> MA35D1 clock controller gates core/axi/ahb with a single bit. The DT
> will
> use the same clock phandle for core, axi, and ahb:
> 
>    clocks = <&clk X>, <&clk X>, <&clk X>, <&pix_clk Y>;
>    clock-names = "core", "axi", "ahb", "pix0";
> 
> This correctly models the hardware topology. Since all three names

No, this doesn't correctly model the hardware topology -- this will
lead to clk_get_rate() return the rate of DC core clock when checking
the AXI clock rate, which is problematic because both clocks are
limiting the performance of the DC.

> resolve
> to the same underlying clock node, the CCF's standard enable
> refcounting
> handles the shared gate correctly without any custom implementation
> needed.
> I will also revert the change in patch 4/7 that made axi and ahb
> clocks
> optional, since they will now always be provided in the devicetree.
> 
> Regarding moving `resets` and `reset-names` to the top-level
> `required:`,
> I will wait for Icenowy's patch to land before sending v6 to avoid
> duplicating the work.

The patch is sent.

> 
> In v6 I will update patch 1/7 with:
> - Update the subject to "dt-bindings: display: verisilicon,dc: add
>    support for nuvoton,ma35d1-dcu"
> - Lower `clocks`/`clock-names` `minItems` to 4 at the top level
> - Remove the `thead,th1520-dc8200` conditional block entirely

I think this conditional block will still be needed, because it will
need to constrain the minItems to ensure all clocks / resets are
populated.

Thanks,
Icenowy

> - Keep only the `nuvoton,ma35d1-dcu` conditional block, using only
>    `maxItems: 4` for clocks/clock-names and `maxItems: 1` for
>    resets/reset-names to tighten the top-level constraints
> 
> BR,
> Joey
> > > > order, it partly breaks the intention of having names; if it
> > > > does
> > > > not
> > > > constrain the order, it needs to be clarified that the required
> > > > 1
> > > > reset
> > > > is core instead of the other two.
> > > Given the discussion we're having on the clocks, I wonder if this
> > > is
> > > also an oversimplification and the IP has three resets inputs
> > > hooked
> > > up
> > > to one output of the reset controller (or 3 outputs controlled by
> > > one
> > > bit..).



^ permalink raw reply

* Re: [PATCH v4 2/6] arm64/vmalloc: Allow arch_vmap_pte_range_map_size to batch multiple CONT_PTE
From: Dev Jain @ 2026-06-29  5:34 UTC (permalink / raw)
  To: Wen Jiang, linux-mm, linux-arm-kernel, catalin.marinas, will,
	akpm, urezki
  Cc: baohua, Xueyuan.chen21, rppt, david, ryan.roberts,
	anshuman.khandual, ajd, linux-kernel, jiangwen6, shanghaoqiang
In-Reply-To: <20260618084726.1070022-3-jiangwen6@xiaomi.com>



On 18/06/26 2:17 pm, Wen Jiang wrote:
> From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> 
> Allow arch_vmap_pte_range_map_size to batch across multiple CONT_PTE
> blocks, reducing both PTE setup and TLB flush iterations.
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  arch/arm64/include/asm/vmalloc.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/vmalloc.h b/arch/arm64/include/asm/vmalloc.h
> index 4ec1acd3c1b34..787fd17b48e2c 100644
> --- a/arch/arm64/include/asm/vmalloc.h
> +++ b/arch/arm64/include/asm/vmalloc.h
> @@ -23,6 +23,8 @@ static inline unsigned long arch_vmap_pte_range_map_size(unsigned long addr,
>  						unsigned long end, u64 pfn,
>  						unsigned int max_page_shift)
>  {
> +	unsigned long size;
> +
>  	/*
>  	 * If the block is at least CONT_PTE_SIZE in size, and is naturally
>  	 * aligned in both virtual and physical space, then we can pte-map the
> @@ -40,7 +42,9 @@ static inline unsigned long arch_vmap_pte_range_map_size(unsigned long addr,
>  	if (!IS_ALIGNED(PFN_PHYS(pfn), CONT_PTE_SIZE))
>  		return PAGE_SIZE;
>  
> -	return CONT_PTE_SIZE;
> +	size = min3(end - addr, 1UL << max_page_shift, PMD_SIZE >> 1);
> +	size = 1UL << __fls(size);

Nit: I'd rather use rounddown_pow_of_two() : __fls is confusing : )

Reviewed-by: Dev Jain <dev.jain@arm.com>


> +	return size;
>  }
>  
>  #define arch_vmap_pte_range_unmap_size arch_vmap_pte_range_unmap_size



^ permalink raw reply

* Re: [PATCH v4 3/6] mm/vmalloc: Extract vmap_set_ptes() to consolidate PTE mapping logic
From: Dev Jain @ 2026-06-29  5:54 UTC (permalink / raw)
  To: Wen Jiang, linux-mm, linux-arm-kernel, catalin.marinas, will,
	akpm, urezki
  Cc: baohua, Xueyuan.chen21, rppt, david, ryan.roberts,
	anshuman.khandual, ajd, linux-kernel, jiangwen6, shanghaoqiang
In-Reply-To: <20260618084726.1070022-4-jiangwen6@xiaomi.com>



On 18/06/26 2:17 pm, Wen Jiang wrote:
> Extract the common PTE mapping logic from vmap_pte_range() into a
> shared helper vmap_set_ptes(). This handles both CONT_PTE and regular
> PTE mappings in a single function, preparing for the next patch which
> will extend vmap_pages_pte_range() to also use this helper.
> 
> The #ifdef CONFIG_HUGETLB_PAGE guard is moved inside vmap_set_ptes(),
> so callers no longer need to handle the conditional compilation.
> 
> No functional change.
> 
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  mm/vmalloc.c | 44 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 2c2f74a07f396..6660f240d27c9 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -91,6 +91,35 @@ struct vfree_deferred {
>  static DEFINE_PER_CPU(struct vfree_deferred, vfree_deferred);
>  
>  /*** Page table manipulation functions ***/
> +
> +/*
> + * Set PTE mappings for the given PFN. Try CONT_PTE mappings first when

CONT_PTE is arm64-specific, doesn't feel right to mention this in generic code.

If you really want to put a comment, just say:

"Try contiguous mappings at the PTE level for arches which support them, and if
requested by the caller. Fall back to PAGE_SIZE mappings otherwise.

Return: mapping size."
> + * supported, otherwise fall back to PAGE_SIZE mappings.
> + *
> + * Return: mapping size.
> + */
> +static __always_inline unsigned long vmap_set_ptes(pte_t *pte,
> +		unsigned long addr, unsigned long end, u64 pfn,
> +		pgprot_t prot, unsigned int max_page_shift)
> +{
> +#ifdef CONFIG_HUGETLB_PAGE
> +	if (max_page_shift > PAGE_SHIFT) {

This if block is not needed, max_page_shift > PAGE_SHIFT is handled by
arch_vmap_pte_range_map_size.

With that:

Reviewed-by: Dev Jain <dev.jain@arm.com>


> +		unsigned long size;
> +
> +		size = arch_vmap_pte_range_map_size(addr, end, pfn, max_page_shift);
> +		if (size != PAGE_SIZE) {
> +			pte_t entry = pfn_pte(pfn, prot);
> +
> +			entry = arch_make_huge_pte(entry, ilog2(size), 0);
> +			set_huge_pte_at(&init_mm, addr, pte, entry, size);
> +			return size;
> +		}
> +	}
> +#endif
> +	set_pte_at(&init_mm, addr, pte, pfn_pte(pfn, prot));
> +	return PAGE_SIZE;
> +}
> +
>  static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  			phys_addr_t phys_addr, pgprot_t prot,
>  			unsigned int max_page_shift, pgtbl_mod_mask *mask)
> @@ -119,19 +148,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  			BUG();
>  		}
>  
> -#ifdef CONFIG_HUGETLB_PAGE
> -		size = arch_vmap_pte_range_map_size(addr, end, pfn, max_page_shift);
> -		if (size != PAGE_SIZE) {
> -			pte_t entry = pfn_pte(pfn, prot);
> -
> -			entry = arch_make_huge_pte(entry, ilog2(size), 0);
> -			set_huge_pte_at(&init_mm, addr, pte, entry, size);
> -			pfn += PFN_DOWN(size);
> -			continue;
> -		}
> -#endif
> -		set_pte_at(&init_mm, addr, pte, pfn_pte(pfn, prot));
> -		pfn++;
> +		size = vmap_set_ptes(pte, addr, end, pfn, prot, max_page_shift);
> +		pfn += PFN_DOWN(size);
>  	} while (pte += PFN_DOWN(size), addr += size, addr != end);
>  
>  	lazy_mmu_mode_disable();



^ permalink raw reply

* Re: [PATCH v2 00/12] coresight: Add CPU cluster funnel/replicator/tmc support
From: Yuanfang Zhang @ 2026-06-29  5:55 UTC (permalink / raw)
  To: Maulik Shah (mkshah), Sudeep Holla, ulfh, Suzuki K Poulose,
	Leo Yan
  Cc: Mike Leach, James Clark, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Mathieu Poirier, Alexander Shishkin,
	Bjorn Andersson, Konrad Dybcio, kernel, coresight,
	linux-arm-kernel, devicetree, linux-kernel, linux-arm-msm,
	Jie Gan
In-Reply-To: <11a01108-63da-4317-a547-5977a469a7dc@oss.qualcomm.com>

Hi Sudeep & Suzuki,

Gentle reminder on this patch. Any feedback would be appreciated.

Thanks,
Yuanfang.

On 5/25/2026 9:17 PM, Maulik Shah (mkshah) wrote:
> 
> 
> On 12/19/2025 3:51 PM, Sudeep Holla wrote:
>> On Fri, Dec 19, 2025 at 10:13:14AM +0800, yuanfang zhang wrote:
>>>
>>>
>>> On 12/18/2025 7:33 PM, Sudeep Holla wrote:
>>>> On Thu, Dec 18, 2025 at 12:09:40AM -0800, Yuanfang Zhang wrote:
>>>>> This patch series adds support for CoreSight components local to CPU clusters,
>>>>> including funnel, replicator, and TMC, which reside within CPU cluster power
>>>>> domains. These components require special handling due to power domain
>>>>> constraints.
>>>>>
>>>>
>>>> Could you clarify why PSCI-based power domains associated with clusters in
>>>> domain-idle-states cannot address these requirements, given that PSCI CPU-idle
>>>> OSI mode was originally intended to support them? My understanding of this
>>>> patch series is that OSI mode is unable to do so, which, if accurate, appears
>>>> to be a flaw that should be corrected.
>>>
>>> It is due to the particular characteristics of the CPU cluster power
>>> domain.Runtime PM for CPU devices works little different, it is mostly used
>>> to manage hierarchicalCPU topology (PSCI OSI mode) to talk with genpd
>>> framework to manage the last CPU handling in cluster.
>>
>> That is indeed the intended design. Could you clarify which specific
>> characteristics differentiate it here?
> 
> Sorry for coming very late on this.
> 
> This series is intended to handle coresight components which resides within CPU cluster.
> For the cases where cluster is in deepest idle low power mode or all CPUs belonging to cluster
> are hotplugged off, access to coresight components can not be done.
> 
> The implementation tried to address in two parts,
> 1. Using cluster power-domain to know which coresight component belongs to which cluster/CPUs
> 2. Schedule the task on intended cluster's CPU to make sure the CPU (and cluster) power is
> ON while coresight component of the cluster is being accessed (using smp_call_function_single()).
> 
> The use of power-domains in (1) will limit this to PSCI OS-Initiated mode,
> to have this support on PSCI Platform-Coordinated mode too, probably instead of power-domains,
> cpu-maps (which also defines the clusters) from device tree is a better choice which will give
> the information on which CPU belongs to which cluster.
> 
> (2) ensured that scheduling happened on intended CPU and while the access is in progress, CPU (and
> cluster) will not enter power down in between.
> 
>>
>>> It doesn’t really send IPI to wakeup CPU device (It don’t have
>>> .power_on/.power_off) callback implemented which gets invoked from
>>> .runtime_resume callback. This behavior is aligned with the upstream Kernel.
>>>
>>
>> I am quite lost here. Why is it necessary to wake up the CPU? If I understand
>> correctly, all of this complexity is meant to ensure that the cluster power
>> domain is enabled before any of the funnel registers are accessed. Is that
>> correct?
> 
> Yes, This is to ensure that CPU (and cluster) power is ON while coresight components
> for same cluster are being accessed.
> 
>>
>> If so, and if the cluster domains are already defined as the power domains for
>> these funnel devices, then they should be requested to power on automatically
>> before any register access occurs. Is that not the case?
> 
> Cluster power-domains will be only available for PSCI OS-initiated mode but also
> will not help for cases where all CPUs in cluster are hotplugged off as hotplugs are
> platform coordinated.
> 
> After discussion with our HW team to automatically request power on for coresight
> component GPR [1] can be used but they seems not working as intended on the existing
> SoCs and will be available on next generation SoC.
> 
> [1] https://developer.arm.com/documentation/ddi0480/d/Functional-Overview/Granular-Power-Requestor
> 
>>
>> What am I missing in this reasoning?
>>
>> The only explanation I can see is that the firmware does not properly honor
>> power-domain requests coming directly from the OS. I believe that may be the
>> case, but I would be glad to be proven wrong.
>>
> 
> please see below comment for more details, This seems not a firmware issue.
> 
>>>>
>>>>> Unlike system-level CoreSight devices, these components share the CPU cluster's
>>>>> power domain. When the cluster enters low-power mode (LPM), their registers
>>>>> become inaccessible. Notably, `pm_runtime_get` alone cannot bring the cluster
>>>>> out of LPM, making standard register access unreliable.
>>>>>
>>>>
>>>> Are these devices the only ones on the system that are uniquely bound to
>>>> cluster-level power domains? If not, what additional devices share this
>>>> dependency so that we can understand how they are managed in comparison?
>>>>
>>>
>>> Yes, devices like ETM and TRBE also share this power domain and access constraint.
>>> Their drivers naturally handle enablement/disablement on the specific CPU they
>>> belong to (e.g., via hotplug callbacks or existing smp_call_function paths).
>>
>> I understand many things are possible to implement, but the key question
>> remains: why doesn’t the existing OSI mechanism - added specifically to cover
>> cases like this solve the problem today?
>>
>> Especially on platforms with OSI enabled, what concrete limitation forces us
>> into this additional “wake-up” path instead of relying on OSI to manage the
>> dependency/power sequencing?
> 
> + Ulf in loop.
> 
> Current platforms with OSI enabled, Linux PSCI do not implement the power_on/power_off
> requests, as far as i know, runtime PM was never meant to implement this part and
> pm_runtime_get_sync() (from drivers/cpuidle/cpuidle-psci.c) call is only used to convey
> to cluster power domains about a child CPU/ sub domain being on after it has already
> been landed in Linux.
> 
> The standalone invoke of pm_runtime_get_sync() from another CPU do not really turn on/get
> the CPU (and cluster), as the CPUs either use CPUidle / CPU hotplug paths to enter/exit
> low power mode.
> 
> To put it other way,
> For a hot-plugged off CPU, invoking a pm_runtime_get_sync() won't get the CPU (and make
> its cluster power domain) ON. In order to turn on the CPU, one has to still request
> the online of the CPU, say via sysfs command echo 1 > /sys/devices/system/cpu/cpuX/online
> which would invoke PSCI CPU_ON function and the power domain for CPU gets marked as ON
> only after CPU already landed in Linux via psci_idle_cpuhp_up() invoking pm_runtime_get_sync().
> 
> I used specific hotplug example but same applies to idle powered down CPU (or Cluster) too.
> 
>>
>>>>> To address this, the series introduces:
>>>>> - Identifying cluster-bound devices via a new `qcom,cpu-bound-components`
>>>>>   device tree property.
>>>>
>>>> Really, no please.
>>>>
>>>
>>> Our objective is to determine which CoreSight components are physically locate
>>> within the CPU cluster power domain.
>>>
>>> Would it be acceptable to derive this relationship from the existing
>>> power-domains binding?
>>
>> In my opinion, this is not merely a possibility but an explicit expectation.
>>
>>> For example, if a Funnel or Replicator node is linked to a power-domains
>>> entry that specifies a cpumask, the driver could recognize this shared
>>> dependency and automatically apply the appropriate cluster-aware behavior.
>>>
>>
>> Sure, but for the driver to use that information, we need clear explanation
>> for all the questions above. In short, why it is not working with the existing
>> PSCI domain idle support.
>>
> 
> Explained above.
> 
> Thanks,
> Maulik



^ permalink raw reply

* Re: [PATCH v3 2/2] iio: adc: add Axiado SARADC driver
From: Andy Shevchenko @ 2026-06-29  6:05 UTC (permalink / raw)
  To: Petar Stepanovic
  Cc: David Lechner, Akhila Kavi, Prasad Bolisetty, Jonathan Cameron,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Harshit Shah, linux-iio, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <b06005e0-b7bc-4967-ac7b-cb170219f131@axiado.com>

On Mon, Jun 29, 2026 at 04:33:00AM +0200, Petar Stepanovic wrote:

...

> >> +static const struct iio_chan_spec axiado_saradc_iio_channels[] = {
> >> +     AX_SARADC_CH(0, "adc0"),   AX_SARADC_CH(1, "adc1"),
> >> +     AX_SARADC_CH(2, "adc2"),   AX_SARADC_CH(3, "adc3"),
> >> +     AX_SARADC_CH(4, "adc4"),   AX_SARADC_CH(5, "adc5"),
> >> +     AX_SARADC_CH(6, "adc6"),   AX_SARADC_CH(7, "adc7"),
> >> +     AX_SARADC_CH(8, "adc8"),   AX_SARADC_CH(9, "adc9"),
> >> +     AX_SARADC_CH(10, "adc10"), AX_SARADC_CH(11, "adc11"),
> >> +     AX_SARADC_CH(12, "adc12"), AX_SARADC_CH(13, "adc13"),
> >> +     AX_SARADC_CH(14, "adc14"), AX_SARADC_CH(15, "adc15"),
> > Two columns looks a bit odd.
> 
> I will also reformat the channel table to one entry per line.

I think with a new approach David proposed, 4 per line will be also acceptable.

> >> +};

...

> >> +static void axiado_saradc_disable(void *data)
> >> +{
> >> +     struct axiado_saradc *info = data;
> >> +
> >> +     writel(AX_SARADC_GLOBAL_CTRL_PD, info->regs + AX_SARADC_GLOBAL_CTRL_REG);
> > People usual make read and write wrappers or use regmap to avoid having
> > to write `info->regs + AX_SARADC_GLOBAL_CTRL_REG` so many times.
> 
> My understanding is that simple read/write wrappers are not always
> preferred unless they provide additional value. Would switching the
> driver to regmap be acceptable here to avoid repeating the base address
> calculation?

This is the value of transition --> having that register base to be hidden and
not repeated all the times.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH v4 4/6] mm/vmalloc: Extend page table walk to support larger page_shift sizes and eliminate page table rewalk
From: Dev Jain @ 2026-06-29  6:20 UTC (permalink / raw)
  To: Wen Jiang, linux-mm, linux-arm-kernel, catalin.marinas, will,
	akpm, urezki
  Cc: baohua, Xueyuan.chen21, rppt, david, ryan.roberts,
	anshuman.khandual, ajd, linux-kernel, jiangwen6, shanghaoqiang
In-Reply-To: <20260618084726.1070022-5-jiangwen6@xiaomi.com>



On 18/06/26 2:17 pm, Wen Jiang wrote:
> From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> 
> vmap_pages_range_noflush_walk() (formerly vmap_small_pages_range_noflush())
> provides a clean interface by taking struct page **pages and mapping them
> via direct PTE iteration. This avoids the page table rewalk seen when
> using vmap_range_noflush() for page_shift values other than PAGE_SHIFT.
> 
> Extend it to support larger page_shift values, and add PMD- and
> contiguous-PTE mappings as well. Rename it to vmap_pages_range_noflush_walk()
> since it now handles more than just small pages.
> 
> For vmalloc() allocations with VM_ALLOW_HUGE_VMAP, we no longer need to
> iterate over pages one by one via vmap_range_noflush(), which would
> otherwise lead to page table rewalk. The code is now unified with the
> PAGE_SHIFT case by simply calling vmap_pages_range_noflush_walk().
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  mm/vmalloc.c | 81 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 47 insertions(+), 34 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 6660f240d27c9..253e017130e09 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -127,7 +127,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  	pte_t *pte;
>  	u64 pfn;
>  	struct page *page;
> -	unsigned long size = PAGE_SIZE;
> +	unsigned long size;
> +	unsigned int steps;
>  
>  	if (WARN_ON_ONCE(!PAGE_ALIGNED(end - addr)))
>  		return -EINVAL;
> @@ -149,8 +150,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  		}
>  
>  		size = vmap_set_ptes(pte, addr, end, pfn, prot, max_page_shift);
> -		pfn += PFN_DOWN(size);
> -	} while (pte += PFN_DOWN(size), addr += size, addr != end);
> +		steps = PFN_DOWN(size);
> +	} while (pte += steps, pfn += steps, addr += size, addr != end);
>  

This belongs to previous patch.

Apart from that LGTM:

Reviewed-by: Dev Jain <dev.jain@arm.com>


>  	lazy_mmu_mode_disable();
>  	*mask |= PGTBL_PTE_MODIFIED;
> @@ -542,8 +543,10 @@ void vunmap_range(unsigned long addr, unsigned long end)
>  
>  static int vmap_pages_pte_range(pmd_t *pmd, unsigned long addr,
>  		unsigned long end, pgprot_t prot, struct page **pages, int *nr,
> -		pgtbl_mod_mask *mask)
> +		pgtbl_mod_mask *mask, unsigned int shift)
>  {
> +	unsigned long pfn, size;
> +	unsigned int steps;
>  	int err = 0;
>  	pte_t *pte;
>  
> @@ -574,9 +577,10 @@ static int vmap_pages_pte_range(pmd_t *pmd, unsigned long addr,
>  			break;
>  		}
>  
> -		set_pte_at(&init_mm, addr, pte, mk_pte(page, prot));
> -		(*nr)++;
> -	} while (pte++, addr += PAGE_SIZE, addr != end);
> +		pfn = page_to_pfn(page);
> +		size = vmap_set_ptes(pte, addr, end, pfn, prot, shift);
> +		steps = PFN_DOWN(size);
> +	} while (pte += steps, *nr += steps, addr += size, addr != end);
>  
>  	lazy_mmu_mode_disable();
>  	*mask |= PGTBL_PTE_MODIFIED;
> @@ -586,7 +590,7 @@ static int vmap_pages_pte_range(pmd_t *pmd, unsigned long addr,
>  
>  static int vmap_pages_pmd_range(pud_t *pud, unsigned long addr,
>  		unsigned long end, pgprot_t prot, struct page **pages, int *nr,
> -		pgtbl_mod_mask *mask)
> +		pgtbl_mod_mask *mask, unsigned int shift)
>  {
>  	pmd_t *pmd;
>  	unsigned long next;
> @@ -596,7 +600,27 @@ static int vmap_pages_pmd_range(pud_t *pud, unsigned long addr,
>  		return -ENOMEM;
>  	do {
>  		next = pmd_addr_end(addr, end);
> -		if (vmap_pages_pte_range(pmd, addr, next, prot, pages, nr, mask))
> +
> +		if (shift >= PMD_SHIFT) {
> +			struct page *page = pages[*nr];
> +			phys_addr_t phys_addr;
> +
> +			if (WARN_ON(!page))
> +				return -ENOMEM;
> +			if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> +				return -EINVAL;
> +
> +			phys_addr = page_to_phys(page);
> +
> +			if (vmap_try_huge_pmd(pmd, addr, next, phys_addr, prot,
> +						shift)) {
> +				*mask |= PGTBL_PMD_MODIFIED;
> +				*nr += 1 << (PMD_SHIFT - PAGE_SHIFT);
> +				continue;
> +			}
> +		}
> +
> +		if (vmap_pages_pte_range(pmd, addr, next, prot, pages, nr, mask, shift))
>  			return -ENOMEM;
>  	} while (pmd++, addr = next, addr != end);
>  	return 0;
> @@ -604,7 +628,7 @@ static int vmap_pages_pmd_range(pud_t *pud, unsigned long addr,
>  
>  static int vmap_pages_pud_range(p4d_t *p4d, unsigned long addr,
>  		unsigned long end, pgprot_t prot, struct page **pages, int *nr,
> -		pgtbl_mod_mask *mask)
> +		pgtbl_mod_mask *mask, unsigned int shift)
>  {
>  	pud_t *pud;
>  	unsigned long next;
> @@ -614,7 +638,7 @@ static int vmap_pages_pud_range(p4d_t *p4d, unsigned long addr,
>  		return -ENOMEM;
>  	do {
>  		next = pud_addr_end(addr, end);
> -		if (vmap_pages_pmd_range(pud, addr, next, prot, pages, nr, mask))
> +		if (vmap_pages_pmd_range(pud, addr, next, prot, pages, nr, mask, shift))
>  			return -ENOMEM;
>  	} while (pud++, addr = next, addr != end);
>  	return 0;
> @@ -622,7 +646,7 @@ static int vmap_pages_pud_range(p4d_t *p4d, unsigned long addr,
>  
>  static int vmap_pages_p4d_range(pgd_t *pgd, unsigned long addr,
>  		unsigned long end, pgprot_t prot, struct page **pages, int *nr,
> -		pgtbl_mod_mask *mask)
> +		pgtbl_mod_mask *mask, unsigned int shift)
>  {
>  	p4d_t *p4d;
>  	unsigned long next;
> @@ -632,14 +656,18 @@ static int vmap_pages_p4d_range(pgd_t *pgd, unsigned long addr,
>  		return -ENOMEM;
>  	do {
>  		next = p4d_addr_end(addr, end);
> -		if (vmap_pages_pud_range(p4d, addr, next, prot, pages, nr, mask))
> +		if (vmap_pages_pud_range(p4d, addr, next, prot, pages, nr, mask, shift))
>  			return -ENOMEM;
>  	} while (p4d++, addr = next, addr != end);
>  	return 0;
>  }
>  
> -static int vmap_small_pages_range_noflush(unsigned long addr, unsigned long end,
> -		pgprot_t prot, struct page **pages)
> +/*
> + * It can take an array of pages which are not all contiguous, but it
> + * may have contiguous chunks, as hinted by @shift.
> + */
> +static int vmap_pages_range_noflush_walk(unsigned long addr, unsigned long end,
> +		pgprot_t prot, struct page **pages, unsigned int shift)
>  {
>  	unsigned long start = addr;
>  	pgd_t *pgd;
> @@ -654,7 +682,7 @@ static int vmap_small_pages_range_noflush(unsigned long addr, unsigned long end,
>  		next = pgd_addr_end(addr, end);
>  		if (pgd_bad(*pgd))
>  			mask |= PGTBL_PGD_MODIFIED;
> -		err = vmap_pages_p4d_range(pgd, addr, next, prot, pages, &nr, &mask);
> +		err = vmap_pages_p4d_range(pgd, addr, next, prot, pages, &nr, &mask, shift);
>  		if (err)
>  			break;
>  	} while (pgd++, addr = next, addr != end);
> @@ -677,27 +705,12 @@ static int vmap_small_pages_range_noflush(unsigned long addr, unsigned long end,
>  int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  		pgprot_t prot, struct page **pages, unsigned int page_shift)
>  {
> -	unsigned int i, nr = (end - addr) >> PAGE_SHIFT;
> -
>  	WARN_ON(page_shift < PAGE_SHIFT);
>  
> -	if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC) ||
> -			page_shift == PAGE_SHIFT)
> -		return vmap_small_pages_range_noflush(addr, end, prot, pages);
> +	if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC))
> +		page_shift = PAGE_SHIFT;
>  
> -	for (i = 0; i < nr; i += 1U << (page_shift - PAGE_SHIFT)) {
> -		int err;
> -
> -		err = vmap_range_noflush(addr, addr + (1UL << page_shift),
> -					page_to_phys(pages[i]), prot,
> -					page_shift);
> -		if (err)
> -			return err;
> -
> -		addr += 1UL << page_shift;
> -	}
> -
> -	return 0;
> +	return vmap_pages_range_noflush_walk(addr, end, prot, pages, page_shift);
>  }
>  
>  int vmap_pages_range_noflush(unsigned long addr, unsigned long end,



^ permalink raw reply

* [PATCH] arm64: dts: amlogic: add some device nodes for A9
From: Xianwei Zhao via B4 Relay @ 2026-06-29  6:41 UTC (permalink / raw)
  To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel,
	Xianwei Zhao

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add pinctrl and irqchip-gpio device nodes for A9 SoC.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Add pinctrl and irqchip-gpio device node for A9.
---
 arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi | 140 ++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi
index 660c8556a864..cdc57b685cc3 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi
@@ -6,6 +6,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/amlogic,pinctrl.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -97,6 +98,95 @@ soc {
 		#size-cells = <2>;
 		ranges;
 
+		apb: bus@fe000000 {
+			compatible = "simple-bus";
+			reg = <0x0 0xfe000000 0x0 0x480000>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
+
+			periphs_pinctrl: pinctrl@4000 {
+				compatible = "amlogic,pinctrl-a9";
+				#address-cells = <2>;
+				#size-cells = <2>;
+				ranges = <0x0 0x0 0x0 0x4000 0x0 0x340>;
+
+				gpioz: gpio@c0 {
+					reg = <0 0xc0 0 0x20>, <0 0x18 0 0x8>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_Z<<8) 16>;
+				};
+
+				gpiox: gpio@100 {
+					reg = <0 0x100 0 0x24>, <0 0xc 0 0x8>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_X<<8) 18>;
+				};
+
+				gpioh: gpio@140 {
+					reg = <0 0x140 0 0x20>, <0 0x2c 0 0x4>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_H<<8) 8>;
+				};
+
+				gpiom: gpio@1a0 {
+					reg = <0 0x1a0 0 0x20>, <0 0x20 0 0x4>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_M<<8) 8>;
+				};
+
+				gpiob: gpio@240 {
+					reg = <0 0x240 0 0x20>, <0 0x0 0 0x8>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_B<<8) 14>;
+				};
+
+				gpioa: gpio@280 {
+					reg = <0 0x280 0 0x24>, <0 0x40 0 0xc>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_A<<8) 20>;
+				};
+
+				gpioy: gpio@2c0 {
+					reg = <0 0x2c0 0 0x20>, <0 0x30 0 0x8>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_Y<<8) 10>;
+				};
+
+				gpiocc: gpio@300 {
+					reg = <0 0x300 0 0x20>, <0 0x14 0 0x4>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_CC<<8) 2>;
+				};
+			};
+
+			gpio_intc: interrupt-controller@4080 {
+				compatible = "amlogic,a9-gpio-intc",
+					     "amlogic,meson-gpio-intc";
+				reg = <0x0 0x4080 0x0 0x20>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				amlogic,channel-interrupts =
+					<10 11 12 13 14 15 16 17 18 19 20 21>;
+			};
+		};
+
 		gic: interrupt-controller@ff800000 {
 			compatible = "arm,gic-v3";
 			#interrupt-cells = <3>;
@@ -123,6 +213,56 @@ uart_b: serial@1e000 {
 				clock-names = "xtal", "pclk", "baud";
 				status = "disabled";
 			};
+
+			aobus_pinctrl: pinctrl@4000 {
+				compatible = "amlogic,pinctrl-a9";
+				#address-cells = <2>;
+				#size-cells = <2>;
+				ranges = <0x0 0x0 0x0 0x4000 0x0 0x0e0>;
+
+				gpioao: gpio@1c {
+					reg = <0 0x1c 0 0x20>, <0 0x0 0 0x8>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_AO<<8) 13>;
+				};
+
+				gpioc: gpio@3c {
+					reg = <0 0x3c 0 0x20>, <0 0x10 0 0x4>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_C<<8) 7>;
+				};
+
+				gpiod: gpio@5c {
+					reg = <0 0x5c 0 0x24>, <0 0x8 0 0x8>;
+					reg-names = "gpio", "mux";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_D<<8) 18>;
+				};
+
+				test_n: gpio@c0 {
+					reg = <0 0xc0 0 0x20>;
+					reg-names = "gpio";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_TEST_N<<8) 1>;
+				};
+			};
+
+			gpio_ao_intc: interrupt-controller@4080 {
+				compatible = "amlogic,a9-gpio-ao-intc",
+					     "amlogic,meson-gpio-intc";
+				reg = <0x0 0x4080 0x0 0x34>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				amlogic,channel-interrupts =
+					<384 385 386 387 388 389 390 391 392 393
+					394 395 396 397 398 399 400 401 402 403>;
+			};
 		};
 	};
 };

---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260629-a9-node-3e6fceba7c90

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>




^ permalink raw reply related

* Re: [PATCH v2 07/16] usb: hub: Power on connected M.2 E-key connectors
From: Chen-Yu Tsai @ 2026-06-29  6:46 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Alan Stern, linux-acpi, driver-core, linux-pm, linux-usb,
	devicetree, linux-mediatek, linux-arm-kernel, linux-kernel,
	Manivannan Sadhasivam, Greg Kroah-Hartman, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
	Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno
In-Reply-To: <CAGXv+5HQa9BH5wyVwKNxjXLEZDnE0sbeQjgNxwmAG+OF8bbz=w@mail.gmail.com>

On Fri, Jun 12, 2026 at 4:55 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Thu, Jun 11, 2026 at 6:11 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
> >
> > On Wed, 10 Jun 2026 10:40:41 +0200, Chen-Yu Tsai <wenst@chromium.org> said:
> > > The new M.2 E-key connector can have a USB connection. For the USB device
> > > on this connector to work, its power must be enabled and the W_DISABLE2#
> > > signal deasserted. The connector driver handles this and provides a
> > > toggle over the power sequencing API.
> > >
> > > This feature currently only supports a directly connected (no mux in
> > > between) M.2 E-key connector. Existing USB connector types are not
> > > covered. The USB A connector was recently added to the onboard devices
> > > driver. USB B connectors have historically been managed by the USB
> > > gadget or dual-role device controller drivers. USB C connectors are
> > > handled by TCPM drivers.
> > >
> > > The power sequencing API does not know whether a power sequence provider
> > > is not needed or not available yet, so we only request it for connectors
> > > that we know need it, which at this time is just the E-key connector.
> > >
> > > On the USB side, the port firmware node (if present) is tied to the
> > > usb_port device. This device is used to acquire the power sequencing
> > > descriptor. This allows the provider to tell the different ports on one
> > > hub apart.
> > >
> > > This feature is not implemented in the onboard USB devices driver. The
> > > power sequencing API expects the consumer device to make the request,
> > > but there is no device node to instantiate a platform device to tie
> > > the driver to. The connector is not a child node of the USB host or
> > > hub, and the graph connection is from a USB port to the connector.
> > > And the connector itself already has a driver.
> > >
> > > Power sequencing is not directly enabled in the connector driver as
> > > that would completely decouple the timing of it from the USB subsystem.
> > > It would not be possible for the USB subsystem to toggle the power
> > > for a power cycle or to disable the port.
> > >
> > > This change depends on another change to make the power sequencing
> > > framework bool instead of tristate. The USB core and hub driver are
> > > bool, so if the power sequencing framework is built as a module, the
> > > kernel will fail to link.
> > >
> >
> > That bit needs to go away I suppose?
>
> Yeah, instead we need
>
>     config USB
>         depends on POWER_SEQUENCING && !POWER_SEQUENCING

FTR:

Somehow I remembered this incorrectly. It should be the following instead:

    depends on POWER_SEQUENCING || !POWER_SEQUENCING

and the dependency issue mentioned below then goes away.

ChenYu

> But I ran into a dozen or so drivers that have "select USB", mostly
> input devices:
>
>     config TOUCHSCREEN_USB_COMPOSITE
>         tristate "USB Touchscreen Driver"
>         depends on USB_ARCH_HAS_HCD
>         select USB
>
> Kconfig complains about unmet dependencies.
>
> > I see Andy has some suggestions but in general I like this approach much better
> > than adding the pwrseq_get_index() function. Thanks!
>
> Thanks!
>
> ChenYu


^ permalink raw reply

* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Aneesh Kumar K.V @ 2026-06-29  6:46 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Alexey Kardashevskiy, Catalin Marinas, iommu, linux-arm-kernel,
	linux-kernel, linux-coco, Robin Murphy, Marek Szyprowski,
	Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
	Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
	linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
	Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <20260619140616.GB1068655@ziepe.ca>

Jason Gunthorpe <jgg@ziepe.ca> writes:

> On Fri, Jun 19, 2026 at 02:36:19PM +0100, Aneesh Kumar K.V wrote:
>> >> Agreed. If the device can do encrypted DMA and requires bouncing, it
>> >> should bounce through encrypted pools. We don't support encrypted pools
>> >> now and that means, we mark the option ("mem_encrypt=on iommu=pt
>> >> swiotlb=force") not supported for now? 
>> >
>> > ?? if you don't have a CC system then the swiotlb is "encrypted"
>> > meaning ordinary struct page system memory.
>> >
>> > The hypervisor should not be triggering any CC special stuff here, it
>> > is not a CC guest.
>> >
>> > Agree we don't need to worry about swiotlb=force with a trusted device
>> > in the GUEST for now, but it should be something to fix eventually.
>> >
>> 
>> If i understand this correctly, the setup Alexey is referring to here is
>> bare metal system with memory encryption enabled and dma address doesn't
>> need C bit cleared because it is handled in iommu.
>
> This is how I understand it too, if the iommu is turned on then it can
> take the high PA with the C bit set and map it to an IOVA that matches
> the device's dma limit.
>
>> ( I consider this as memory encryption that is handled
>> transparently, device can access any address because that encryption
>> details are now managed by iommu).
>
> Compared to the guest side there are some important host side differences:
>
>  - On the host the iommu can fix it because this is only a matter of
>    IOVA range not access control. On a guest even a IOMMU cannot
>    permit access to private memory
>  - On the host the state of the device is driven by the dma limit
>    which is not set until after the driver probes. On guest the state is
>    set by the tsm and device security level before the driver
>    probes
>  - Both flows end up using pgprot_decrypted and set_memory_decrypted()
>    to create their special pools, but for completely different
>    reasons.
>  - The memory coming from the special swiotlb pool must NOT be used by
>    a trusted device on a CC guest, while there is no problem for any
>    device to use it on the host.
>

Agreed.

>> Thinking about this more, I guess we should mark the swiotlb as
>> cc_shared only with  CC_ATTR_GUEST_MEM_ENCRYPT instead of
>> CC_ATTR_MEM_ENCRYPT as we have below.
>
> The name cc_shared should be used for GUEST scenarios only.
>
> I guess there is some merit in keeping swiotlb using "decrypted" to
> mean it usinig pgprot_decrypted and set_memory_decyped() which AMD
> gives meaning to on both host and guest.
>

Are you suggesting to change the struct io_tlb_mem::cc_shared back to
struct io_tlb_mem::unencrypted?. If we want to split cc_shared and
unencrypted as two flags, I think we will add quiet a lot of code
duplication.

> IDK what AMD should do on the host by default. I guess it should setup
> a swiotlb pool of low dma addrs "unencrypted", but not "cc_shared"?
>

If by low DMA address you mean using an address with the C-bit
cleared. Currently the SME code uses force_dma_unencrypted() as the hook to
determine whether the C-bit needs to be cleared. Therefore,
force_dma_unencrypted(dev) must be true to use such a pool.

The current code already does this and uses the swiotlb pool correctly
on SME. The challenge arises when we want to force SWIOTLB
bouncing even for devices that can handle encrypted DMA addresses (more
on that below). For such a config force_dma_uencrypted(dev) will return
false and swiotlb will be marked cc_shared/decrypted = true; This trip
the new check we added.

	/* swiotlb pool is incorrect for this device */
	if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
		return (phys_addr_t)DMA_MAPPING_ERROR;

We can also do

	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
		/* swiotlb pool is incorrect for this device */
		if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
			return (phys_addr_t)DMA_MAPPING_ERROR;

		/* Force attrs to match the kind of memory in the pool */
		if (mem->cc_shared)
			*attrs |= DMA_ATTR_CC_SHARED;
		else
			*attrs &= ~DMA_ATTR_CC_SHARED;
	} else {
		/*
		 * Host memory encryption where device requires an
		 * unencrypted dma_addr_t due to dma mask limit
    		 */
		if (force_dma_unencrypted(dev))
			*attrs |= DMA_ATTR_CC_SHARED;
		else
			*attrs &= ~DMA_ATTR_CC_SHARED;
	}


Here I see value in having DMA_ATTR_UNENCRYPTED. The question is do we
need to split this into two flags and introduce the resulting code
duplication.

>
> But if we are operating on the host then this pool is not limited to
> only T=0 devices, every device can "safely" use it. (ignoring this
> destroys the security memory encryption on bare metal was supposed to
> provide)
>
>> Now we have the case of host memory encryption where the C-bit needs to
>> be cleared in dma_addr_t. That requires special handling in the kernel, and
>> I believe we need to mark swiotlb as unencrypted in this configuration.
>
> I think we need to split the two things up, they have different
> behaviors and need different flags and labels to make it all work
> right.
>
>> I am still not clear whether there is a config option or runtime check
>> we can use to identify this case.
>
> The dma api has to detect, after the driver sets the dma limit, that
> none of system memory is usable when:
>  - The direct path is being used
>  - phys to dma for 0 is outside the dma limit
>
> Then it should assume the arch has setup a swiotlb pool for it to use
> to fix the high memory problem.
>
> Similar hackery would be needed in the dma alloc path to know that
> decrypted can be used to fix the high memory problem like for GUEST.
>
> I guess some 'dev_cannot_reach_memory(dev)' sort of test in a
> few key places? Setup with a static branch to be a nop on everything
> but AMD, compiled out on every other arch.
>

If we are not able to reach the memory because of the memory encryption
bit, then isn't dev_cannot_reach_memory(dev) the same as
force_dma_unencrypted(dev)? If so, that is how it is already done.

I am wondering whether we can keep this simpler by ignoring the
swiotlb=force kernel parameter and keeping cc_shared as it is, even
though that can be confusing when looking at SME.

The three configurations we need to consider here are:

1) SEV-SNP guest
2) SME host with iommu=translated
3) SME host with iommu=passthrough

IIUC, all of the above work with the current code because we mark the
swiotlb as cc_shared/decrypted when CC_ATTR_MEM_ENCRYPT is set (i.e.,
this applies to an SME host as well).

The challenge arises when the user forces swiotlb bouncing with the
swiotlb=force command-line option. At that point, all devices, including
those whose DMA mask can handle encrypted DMA addresses, are forced to
use SWIOTLB. That becomes a problem because SWIOTLB is marked as
decrypted by default.

How about something like the following?

x86/dma: Disable forced SWIOTLB bouncing for SME IOMMU passthrough

With host memory encryption and IOMMU passthrough, DMA address handling
depends on whether a device can address the C-bit. Devices that cannot
address it need DMA addresses with the C-bit cleared, while devices that
can address encrypted memory should keep using encrypted DMA addresses.

The default swiotlb pool is marked shared when memory encryption is active.
Forcing all devices through that pool would also force devices capable of
encrypted DMA to use shared mappings. Clear the global swiotlb-force-bounce
state in this mode, and warn when this overrides an explicit swiotlb=force
command-line request.

Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

modified   arch/x86/kernel/pci-dma.c
@@ -51,8 +51,24 @@ static void __init pci_swiotlb_detect(void)
 	 * Set swiotlb to 1 so that bounce buffers are allocated and used for
 	 * devices that can't support DMA to encrypted memory.
 	 */
-	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
+	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
 		x86_swiotlb_enable = true;
+		/*
+		 * With host memory encryption and IOMMU passthrough, devices
+		 * that cannot address the C-bit need DMA addresses with the
+		 * C-bit cleared, while devices that can address encrypted
+		 * memory should keep using encrypted DMA addresses.
+		 *
+		 * The default SWIOTLB pool is marked shared when memory
+		 * encryption is active, so forcing all devices through it would
+		 * also force devices that support encrypted DMA to use shared
+		 * mappings. Disable global forced bouncing in this mode.
+		 */
+		if (iommu_default_passthrough() &&
+		    clear_swiotlb_force_bounce())
+			pr_warn("Ignoring swiotlb=force with host memory encryption and "
+				"IOMMU passthrough\n");
+	}
 
 	/*
 	 * Guest with guest memory encryption currently perform all DMA through
modified   include/linux/swiotlb.h
@@ -40,6 +40,7 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
 int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 	int (*remap)(void *tlb, unsigned long nslabs));
 extern void __init swiotlb_update_mem_attributes(void);
+bool __init clear_swiotlb_force_bounce(void);
 
 #ifdef CONFIG_SWIOTLB
 
modified   kernel/dma/swiotlb.c
@@ -208,6 +208,15 @@ unsigned long swiotlb_size_or_default(void)
 	return default_nslabs << IO_TLB_SHIFT;
 }
 
+bool __init clear_swiotlb_force_bounce(void)
+{
+	if (!swiotlb_force_bounce)
+		return false;
+
+	swiotlb_force_bounce = false;
+	return true;
+}
+
 void __init swiotlb_adjust_size(unsigned long size)
 {
 	/*


^ permalink raw reply

* Re: [PATCH v4 6/6] mm/vmalloc: align vm_area so vmap() can batch mappings
From: Dev Jain @ 2026-06-29  6:47 UTC (permalink / raw)
  To: Wen Jiang, linux-mm, linux-arm-kernel, catalin.marinas, will,
	akpm, urezki
  Cc: baohua, Xueyuan.chen21, rppt, david, ryan.roberts,
	anshuman.khandual, ajd, linux-kernel, jiangwen6, shanghaoqiang
In-Reply-To: <20260618084726.1070022-7-jiangwen6@xiaomi.com>



On 18/06/26 2:17 pm, Wen Jiang wrote:
> From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> 
> Try to align the vmap virtual address to PMD_SHIFT or a
> larger PTE mapping size hinted by the architecture, so
> contiguous pages can be batch-mapped when setting PMD or
> PTE entries.
> 
> Add __get_vm_area_node_aligned_caller() as a wrapper over
> __get_vm_area_node() to simplify repeated calls with fixed
> arguments.
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  mm/vmalloc.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index fffb885cb2158..bc9fa93e2bdc6 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3628,6 +3628,41 @@ static int vmap_batched(unsigned long addr, unsigned long end,
>  	return err;
>  }
>  
> +static struct vm_struct *__get_vm_area_node_aligned_caller(unsigned long size,
> +		unsigned long align, unsigned long flags, const void *caller)

There are 3 similar functions below __get_vm_area_node: they are __get_vm_area_caller,
get_vm_area and get_vm_area_caller. You can put this one just below them.


> +{
> +	return __get_vm_area_node(size, align, PAGE_SHIFT, flags,
> +			VMALLOC_START, VMALLOC_END,
> +			NUMA_NO_NODE, GFP_KERNEL, caller);
> +}
> +
> +static struct vm_struct *vmap_get_aligned_vm_area(unsigned long size,
> +		unsigned long flags, const void *caller)
> +{
> +	struct vm_struct *vm_area;
> +	unsigned int shift;
> +
> +	/* Try PMD alignment for large sizes */

This comment feels excessive. Can remove it.


> +	if (size >= PMD_SIZE) {

Need an arch_vmap_pmd_supported() check here.


> +		vm_area = __get_vm_area_node_aligned_caller(size, PMD_SIZE,
> +				flags, caller);
> +		if (vm_area)
> +			return vm_area;
> +	}
> +
> +	/* Try CONT_PTE alignment */

Comment feels excessive, and again no need to mention arm64 specific stuff here.
Just remove it.

> +	shift = arch_vmap_pte_supported_shift(size);
> +	if (shift > PAGE_SHIFT) {
> +		vm_area = __get_vm_area_node_aligned_caller(size, 1UL << shift,
> +				flags, caller);
> +		if (vm_area)
> +			return vm_area;
> +	}
> +
> +	/* Fall back to page alignment */

This comment can also be dropped, but I am fine either way.


> +	return __get_vm_area_node_aligned_caller(size, PAGE_SIZE, flags, caller);
> +}
> +
>  /**
>   * vmap - map an array of pages into virtually contiguous space
>   * @pages: array of page pointers
> @@ -3666,7 +3701,7 @@ void *vmap(struct page **pages, unsigned int count,
>  		return NULL;
>  
>  	size = (unsigned long)count << PAGE_SHIFT;
> -	area = get_vm_area_caller(size, flags, __builtin_return_address(0));
> +	area = vmap_get_aligned_vm_area(size, flags, __builtin_return_address(0));

So the effect is that now we search for an aligned vm_struct unconditionally. Better
to mention in the commit message that we do not expect any significant overhead for
this.


>  	if (!area)
>  		return NULL;
>  



^ permalink raw reply

* [PATCH 4/4] ipmi: bt-bmc: Read LPC address and SerIRQ from device tree
From: Yu-Che Hsieh via B4 Relay @ 2026-06-29  6:49 UTC (permalink / raw)
  To: Corey Minyard, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Joel Stanley, Andrew Jeffery
  Cc: openipmi-developer, linux-kernel, devicetree, linux-arm-kernel,
	linux-aspeed, Yu-Che Hsieh
In-Reply-To: <20260629-aspeed-bt-bmc-multichannel-v1-0-fc23ee337f7a@aspeedtech.com>

From: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>

The BT interface currently programs a fixed host LPC IO address and

SerIRQ number. This works for the existing single-channel setup, but

does not allow the host interface parameters to be described by firmware.

Read the LPC IO address from aspeed,lpc-io-reg and the SerIRQ number

and interrupt type from aspeed,lpc-interrupts. Keep the existing IO

address, SerIRQ number, and level-low interrupt type as defaults when

the properties are not present.

Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>
---
 drivers/char/ipmi/bt-bmc.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 486ecc0b6815..6e1f941e63db 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -65,6 +65,12 @@ struct bt_bmc {
 	struct timer_list	poll_timer;
 	struct mutex		mutex;
 	atomic_t		open_count;
+	u32			io_addr;
+
+	struct {
+		u32 id;
+		u32 type;
+	} sirq;
 };
 
 static u8 bt_inb(struct bt_bmc *bt_bmc, int reg)
@@ -429,6 +435,33 @@ static int bt_bmc_probe(struct platform_device *pdev)
 	mutex_init(&bt_bmc->mutex);
 	init_waitqueue_head(&bt_bmc->queue);
 
+	rc = of_property_read_u32(dev->of_node, "aspeed,lpc-io-reg",
+				  &bt_bmc->io_addr);
+	if (rc) {
+		bt_bmc->io_addr = BT_IO_BASE;
+	} else if (bt_bmc->io_addr > FIELD_MAX(BT_CR0_IO_BASE)) {
+		dev_err(dev, "invalid LPC IO address\n");
+		return -EINVAL;
+	}
+
+	rc = of_property_read_u32_array(dev->of_node, "aspeed,lpc-interrupts",
+					(u32 *)&bt_bmc->sirq, 2);
+	if (rc) {
+		bt_bmc->sirq.id = BT_IRQ;
+		bt_bmc->sirq.type = IRQ_TYPE_LEVEL_LOW;
+	} else {
+		if (bt_bmc->sirq.id > FIELD_MAX(BT_CR0_SIRQ)) {
+			dev_err(dev, "invalid SerIRQ number\n");
+			return -EINVAL;
+		}
+
+		if (bt_bmc->sirq.type != IRQ_TYPE_LEVEL_HIGH &&
+		    bt_bmc->sirq.type != IRQ_TYPE_LEVEL_LOW) {
+			dev_err(dev, "invalid SerIRQ type\n");
+			return -EINVAL;
+		}
+	}
+
 	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR;
 	bt_bmc->miscdev.name	= DEVICE_NAME;
 	bt_bmc->miscdev.fops	= &bt_bmc_fops;
@@ -450,8 +483,10 @@ static int bt_bmc_probe(struct platform_device *pdev)
 		add_timer(&bt_bmc->poll_timer);
 	}
 
-	writel(FIELD_PREP(BT_CR0_IO_BASE, BT_IO_BASE) |
-	       FIELD_PREP(BT_CR0_SIRQ, BT_IRQ) |
+	writel(FIELD_PREP(BT_CR0_IO_BASE, bt_bmc->io_addr) |
+	       FIELD_PREP(BT_CR0_SIRQ, bt_bmc->sirq.id) |
+	       FIELD_PREP(BT_CR0_SIRQ_TYPE,
+			  bt_bmc->sirq.type == IRQ_TYPE_LEVEL_LOW ? 0 : 1) |
 	       BT_CR0_EN_CLR_SLV_RDP |
 	       BT_CR0_EN_CLR_SLV_WRP |
 	       BT_CR0_ENABLE_IBT,

-- 
2.34.1




^ permalink raw reply related

* [PATCH 3/4] dt-bindings: ipmi: Add optional LPC properties to ASPEED BT devices
From: Yu-Che Hsieh via B4 Relay @ 2026-06-29  6:49 UTC (permalink / raw)
  To: Corey Minyard, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Joel Stanley, Andrew Jeffery
  Cc: openipmi-developer, linux-kernel, devicetree, linux-arm-kernel,
	linux-aspeed, Yu-Che Hsieh
In-Reply-To: <20260629-aspeed-bt-bmc-multichannel-v1-0-fc23ee337f7a@aspeedtech.com>

From: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>

Allocating IO and IRQ resources to LPC devices is in-theory an operation

for the host, however ASPEED systems describe these resources through

BMC-internal configuration, as already supported by the ASPEED KCS BMC

binding.

Add aspeed,lpc-io-reg and aspeed,lpc-interrupts to the ASPEED BT BMC

binding so firmware can describe the host LPC IO address and SerIRQ

configuration using the same properties as KCS devices.

Signed-off-by: Yu-Che Hsieh <yc_hsieh@aspeedtech.com>
---
 .../bindings/ipmi/aspeed,ast2400-ibt-bmc.yaml       | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.yaml b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.yaml
index c4f7cdbbe16b..1803c6bbae93 100644
--- a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.yaml
+++ b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.yaml
@@ -25,6 +25,24 @@ properties:
   interrupts:
     maxItems: 1
 
+  aspeed,lpc-io-reg:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    maxItems: 1
+    description: |
+      The host CPU LPC IO address for the BT device.
+
+  aspeed,lpc-interrupts:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    maxItems: 2
+    description: |
+      A 2-cell property expressing the LPC SerIRQ number and the interrupt
+      level/sense encoding (specified in the standard fashion).
+
+      Note that the generated interrupt is issued from the BMC to the host, and
+      thus the target interrupt controller is not captured by the BMC's
+      devicetree.
+
 required:
   - compatible
   - reg
@@ -35,10 +53,13 @@ additionalProperties: false
 examples:
   - |
     #include <dt-bindings/clock/aspeed-clock.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
 
     bt@1e789140 {
         compatible = "aspeed,ast2400-ibt-bmc";
         reg = <0x1e789140 0x18>;
         interrupts = <8>;
         clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
+        aspeed,lpc-io-reg = <0xe4>;
+        aspeed,lpc-interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
     };

-- 
2.34.1




^ permalink raw reply related


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