* Re: [PATCH] ARM: enable interrupts when arm_notify_die() is handling user mode errors
From: Sebastian Andrzej Siewior @ 2026-06-25 8:50 UTC (permalink / raw)
To: Xie Yuanbin
Cc: linux, clrkwllms, rostedt, rmk+kernel, linusw, arnd,
linux-arm-kernel, linux-kernel, linux-rt-devel, liaohua4,
lilinjie8
In-Reply-To: <20260625073522.182503-1-xieyuanbin1@huawei.com>
On 2026-06-25 15:35:22 [+0800], Xie Yuanbin wrote:
> For lastest linux-next kernel, with default multi_v7_defconfig, and
> setting CONFIG_PREEMPT_RT=y, CONFIG_DEBUG_ATOMIC_SLEEP=y, and
> CONFIG_PERF_EVENTS=n. When the user program executes bkpt
> instruction, the following WARN will be triggered:
> ```log
> [ 3.677825] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> [ 3.678002] in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 84, name: test
> [ 3.678036] preempt_count: 0, expected: 0
> [ 3.678078] RCU nest depth: 0, expected: 0
> [ 3.678864] CPU: 0 UID: 0 PID: 84 Comm: test Tainted: G W 7.1.0-next-20260623 #45 PREEMPT_RT
> [ 3.679067] Tainted: [W]=WARN
> [ 3.679088] Hardware name: Generic DT based system
> [ 3.679198] Call trace:
> [ 3.679695] unwind_backtrace from show_stack+0x10/0x14
> [ 3.680363] show_stack from dump_stack_lvl+0x50/0x5c
> [ 3.680377] dump_stack_lvl from __might_resched+0x160/0x174
> [ 3.680393] __might_resched from rt_spin_lock+0x38/0x138
> [ 3.680425] rt_spin_lock from force_sig_info_to_task+0x1c/0x11c
> [ 3.680438] force_sig_info_to_task from force_sig_fault+0x44/0x64
> [ 3.680450] force_sig_fault from do_PrefetchAbort+0x94/0x9c
> [ 3.680461] do_PrefetchAbort from ret_from_exception+0x0/0x20
> [ 3.680513] Exception stack(0xf0ab5fb0 to 0xf0ab5ff8)
> [ 3.680653] 5fa0: 00000000 bed32e94 bed32e9c 00037954
> [ 3.680672] 5fc0: 00000002 00000001 bed32e94 0009d590 00000000 bed32e9c 00000002 00000000
> [ 3.680682] 5fe0: bed32d48 bed32d38 00037a00 00037958 60000010 ffffffff
> ```
I don't think this required information as it is obvious. At the very
least you could trim it the needed parts if considered needed.
> When PREEMPT_RT is enabled, force_sig_info() requires interrupts to be
> enabled. Enable interrupts when arm_notify_die() is handling user mode
> errors to fix the issue.
>
> Fixes: c6e61c06d606 ("ARM: 9463/1: Allow to enable RT")
>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>
So I did test the 32bit case on arm64 while testing/ backporting the
breakpoint handling there but apparently did not try it on real
arm32.
For "asm("BKPT #0");" the SIGTRAP is not raised instead I get just
| 8<--- cut here ---
| Unhandled prefetch abort: debug event (0x222) at 0x00000000
on the kernel side and a "Bus error" on userland side.
So
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
for this but actual breakpoint handling might be broken or is it just
me? But then your stack trace looks like mine so :/
Sebastian
^ permalink raw reply
* Re: [PATCH v2 2/6] iommu/arm-smmu: Add interconnect bandwidth voting support
From: Konrad Dybcio @ 2026-06-25 8:47 UTC (permalink / raw)
To: Bibek Kumar Patro, Dmitry Baryshkov
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <aaf4daee-4886-4214-a761-80545d2565ee@oss.qualcomm.com>
On 6/19/26 12:54 PM, Bibek Kumar Patro wrote:
>
>
> On 6/18/2026 2:58 PM, Konrad Dybcio wrote:
>> On 6/17/26 4:26 PM, Bibek Kumar Patro wrote:
>>>
>>>
>>> On 6/16/2026 5:51 AM, Dmitry Baryshkov wrote:
>>>> On Mon, Jun 15, 2026 at 06:36:51PM +0530, Bibek Kumar Patro wrote:
>>>>>
>>>>>
>>>>> On 6/8/2026 7:25 PM, Dmitry Baryshkov wrote:
>>>>>> On Tue, May 26, 2026 at 08:12:03PM +0530, Bibek Kumar Patro wrote:
>>>>>>> On some SoCs the SMMU registers require an active interconnect
>>>>>>> bandwidth vote to be accessible. While other clients typically
>>>>>>> satisfy this requirement implicitly, certain corner cases (e.g.
>>>>>>> during sleep/wakeup transitions) can leave the SMMU without a
>>>>>>> vote, causing intermittent register access failures.
>>>>>>>
>>>>>>> Add support for an optional interconnect path to the arm-smmu
>>>>>>> driver and vote for bandwidth while the SMMU is active. The path
>>>>>>> is acquired from DT if present and ignored otherwise.
>>>>>>>
>>>>>>> The bandwidth vote is enabled before accessing SMMU registers
>>>>>>> during probe and runtime resume, and released during runtime
>>>>>>> suspend and on error paths.
>>>>>>>
>>>>>>> Generally, from an architectural perspective, GEM_NOC and DDR are
>>>>>>> expected to have an active vote whenever the adreno_smmu block is
>>>>>>> powered on. In most common use cases, this requirement is implicitly
>>>>>>> satisfied because other GPU-related clients (for example, the GMU
>>>>>>> device) already hold a GEM_NOC vote when adreno_smmu is enabled.
>>>>>>>
>>>>>>> However, there are certain corner cases, such as during sleep/wakeup
>>>>>>> transitions, where the GEM_NOC vote can be removed before adreno_smmu
>>>>>>> is powered down. If adreno_smmu is then accessed while the interconnect
>>>>>>> vote is missing, it can lead to the observed failures. Because of the
>>>>>>> precise ordering involved, this scenario is difficult to reproduce
>>>>>>> consistently.
>>>>>>> (also GDSC is involved in adreno usecases can have an independent vote)
>>>>>>>
>>>>>>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>>>>>> ---
>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 57 +++++++++++++++++++++++++++++++++--
>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 ++
>>>>>>> 2 files changed, 57 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>>>>>> index 0bd21d206eb3e75c3b9fb1364cdc92e82c5aa499..07c7e44ec6a5bd1488f00f87d859a20495e46601 100644
>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>>>>>> @@ -53,6 +53,11 @@
>>>>>>> #define MSI_IOVA_BASE 0x8000000
>>>>>>> #define MSI_IOVA_LENGTH 0x100000
>>>>>>> +/* Interconnect bandwidth vote values for the SMMU register access path */
>>>>>>> +#define ARM_SMMU_ICC_AVG_BW 0
>>>>>>> +#define ARM_SMMU_ICC_PEAK_BW_HIGH 1000
>>>>>>
>>>>>> totally random numbers, which might be different for non-Qualcomm platform.
>>>>>>
>>>>>
>>>>> Ideally, any non-zero value would be enough to keep the path active.
>>>>
>>>> This is true for Qualcomm devices. However, you are adding this to a
>>>> generic code.
>>>>
>>>>> Here 1 Would be enough to keep the path active, but might be too small to
>>>>> reliably keep the bus active.
>>>>> Other is UINT_MAX, which will reliably keep the bus active but might cause a
>>>>> power penalty.
>>>>>
>>>>> #define ARM_SMMU_ICC_PEAK_BW_HIGH UINT_MAX
>>>>>
>>>>> seems to be suitable here to reliably keep the bus active by BCM
>>>>> for both Qualcomm and non-Qualcomm platforms (with some power penalty).
>>>>>
>>>>> LMK, if you feel otherwise.
>>>>
>>>> Shift it to the qcom instance or provide platform-specific values? (My
>>>> preference would be towards the first solution).
>>>>
>>>
>>>
>>> To support platform-specific values, we may need to introduce a LUT-based approach in the driver. (Bandwidth voting values cannot be placed in device-tree property IIRC ?)
>>>
>>> Currently, all Qualcomm platforms use 0x1000 for SMMU ICC voting. I
>>
>> (you used decimal 1000)
>>
>
> It's my bad, i meant 1000 only
> (I'll check on the icc_bw calculation to get clarity on the values)
>
>>> can evaluate if this could be moved to a Qualcomm-specific
>>> implementation.
>>
>> Add a vendor hook to arm_smmu_runtime_suspend/resume and handle it within
>> the QC driver
>>
>
> Just curious, wouldn't this apply for all the arm-smmu users in addition to Qualcomm devices as i mentioned here [1].
> Vendor hook would make it Qualcomm specific.
You're proposing to use a Qualcomm-specific bandwidth value so that
fits
Konrad
>
> [1]: https://lore.kernel.org/all/984ff9c7-3eef-463c-a330-bf7acd063667@oss.qualcomm.com/
>
> Thanks & regards,
> Bibek
>
>> Konrad
>
^ permalink raw reply
* Re: [PATCH 0/6] treewide: remove unnecessary invalid range checks in memblock iteration loops
From: Mike Rapoport @ 2026-06-25 8:46 UTC (permalink / raw)
To: Sang-Heon Jeon
Cc: Albert Ou, Andrew Morton, Andrey Ryabinin, Catalin Marinas,
Huacai Chen, Mike Rapoport, Muchun Song, Oscar Salvador,
Palmer Dabbelt, Paul Walmsley, Will Deacon, Alexander Potapenko,
Alexandre Ghiti, Andrey Konovalov, David Hildenbrand,
Dmitry Vyukov, kasan-dev, linux-arm-kernel, linux-mm, linux-riscv,
loongarch, Vincenzo Frascino, WANG Xuerui
In-Reply-To: <20260621145919.1453-1-ekffu200098@gmail.com>
On Sun, 21 Jun 2026 23:59:10 +0900, Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> treewide: remove unnecessary invalid range checks in memblock iteration loops
>
> The memblock API guarantees that for_each_mem_range() and
> for_each_mem_pfn_range() never return an invalid range, meaning start is
> always less than end.
>
> Several memblock callers still have unnecessary invalid range checks in
> their loop bodies, so remove them.
>
> [...]
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC v2 PATCH] reserve_mem: add support for static memory
From: Mike Rapoport @ 2026-06-25 8:37 UTC (permalink / raw)
To: Shyam Saini
Cc: linux-mm, linux-doc, linux-kernel, akpm, tgopinath, bboscaccy,
kees, tony.luck, gpiccoli, bp, rdunlap, peterz, feng.tang,
dapeng1.mi, elver, enelsonmoore, kuba, lirongqing, ebiggers,
Catalin Marinas, Will Deacon, Ard Biesheuvel, David Hildenbrand,
linux-arm-kernel
In-Reply-To: <ajyC2eX9MKSU84Z8@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Hi Shyam,
On Wed, Jun 24, 2026 at 06:22:33PM -0700, Shyam Saini wrote:
> On 21 Jun 2026 13:36, Mike Rapoport wrote:
> > On Thu, Jun 18, 2026 at 11:23:31PM -0700, Shyam Saini wrote:
> > > reserve_mem relies on dynamic memory allocation, this limits the
> > > usecase where memory is required to be preserved across the boots.
> > > Eg: ramoops memory reservation on ACPI platforms
> > >
> > > So add support to pass a pre-determined static address and reserve
> > > memory at a specified location. This enables use case like ramoops
> > > on ACPI platforms to reliably access ramoops region with previous
> > > boot logs.
> > >
> > > Also skip the parsing of <align> when static address is passed.
> > >
> > > Example syntax for static address
> > > reserve_mem=4M@0x1E0000000:oops
> >
> > reserve_mem is best effort by design because such hacks as well as memmap=
> > cannot guarantee this memory is actually free.
> >
> > If you want to preserve ramoops reliably, use KHO with reserve_mem.
> > The first kernel will allocate memory, this memory will be preserved by KHO
> > and could be picked up by the second kernel.
>
> ok, On ARM64 DTS systems, we can reserve ramoops memory in the device tree during
> the warm reboot.
The cc list actually implied x86 ;-)
Added arm64 folks now.
> For an equivalent ARM64 ACPI platform, what is the recommended way to reserve
> and preserve that memory across the boots?
I don't think it exists, but a command line option (be it memmap= or
reserve_mem=) does not seem the right way to me.
Most of the arguments that were made against adding memmap= to arm64 [1]
apply here.
If kexec is an option, KHO provides a reliable way to preserve memory
across boots.
If kexec is not an option, we should look for a generic way to specify
something like DT's reserved_mem for ACPI/EFI systems.
[1] https://lkml.kernel.org/lkml/20201118063314.22940-1-song.bao.hua@hisilicon.com/T/
> Thanks,
> Shyam
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v7 04/30] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers
From: Cristian Ciocaltea @ 2026-06-25 8:27 UTC (permalink / raw)
To: Maxime Ripard
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260625-screeching-uptight-elephant-5bdaed@houat>
On 6/25/26 11:05 AM, Maxime Ripard wrote:
> On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
>> +static int drm_scdc_reset_crtc(struct drm_connector *connector,
>> + struct drm_modeset_acquire_ctx *ctx)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + struct drm_crtc *crtc;
>> + u8 config;
>> + int ret;
>> +
>> + if (!ctx)
>> + return 0;
>> +
>> + /*
>> + * This is normally part of .detect_ctx() call path, which already holds
>> + * connection_mutex through @ctx. However, re-acquiring it with the
>> + * same context is a no-op and makes the helper safe under any caller.
>> + */
>> + ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
>> + if (ret)
>> + return ret;
>> +
>> + if (!connector->state)
>> + return 0;
>> +
>> + crtc = connector->state->crtc;
>> + if (!crtc)
>> + return 0;
>> +
>> + ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
>> + if (ret) {
>> + drm_scdc_dbg(connector, "Failed to read TMDS config: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + if ((config & SCDC_SCRAMBLING_ENABLE) &&
>> + (config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40))
>> + return 0;
>
> I don't think we should care about the bit clock ratio here. From a
> technical standpoint, and since you ignore low rates for the scrambler
> for now, they are indeed equivalents. But semantically, scrambler can be
> enabled for any rate, and the bit clock ratio should be changed only for
> rates higher than 340MHz.
>
> This function makes sure to trigger a modeset when the scrambler is
> enabled or not. From a spec point of view, it's somewhat orthogonal to
> the bit clock ratio. From a function name point of view, it's
> misleading.
>
> So yeah, I'd just drop the check on the bit clock ratio here. Drivers
> will then get to enable either when the commit actually happens.
Ack.
Thanks,
Cristian
^ permalink raw reply
* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
From: Paul Elder @ 2026-06-25 8:23 UTC (permalink / raw)
To: Kieran Bingham, laurent.pinchart
Cc: michael.riesch, xuhf, stefan.klug, dan.scally, jacopo.mondi,
linux-media, linux-arm-kernel, linux-rockchip, linux-kernel,
hverkuil+cisco, nicolas.dufresne, ribalda, sakari.ailus
In-Reply-To: <178229758404.3075020.12553514371020830845@ping.linuxembedded.co.uk>
Hi Kieran,
Thanks for the not-necessarily-a-full review.
Quoting Kieran Bingham (2026-06-24 19:39:44)
> Hi Paul,
>
> I'm taking a first read through, some comments inline, but not
> necessarily a full review:
>
> Quoting Paul Elder (2026-06-19 06:26:28)
> > Currently, a media graph contains a main device whose driver is
> > responsible for creating the media device. We have however recently run
> > into devices that have multiple devices that can quality as a main
>
> s/quality/qualify/
>
> > device. Examples are the RK3588 which has a VICAP and two ISP
> > instances, and another example is the i.MX8MP which has an ISI and two
> > ISP instances. As there is currently no way to reconcile who the main
> > device is in the media device, these setups simple cannot be used
> > simultaneously.
>
> I'm very excited to see how we could apply this to the NXP i.MX8MP to
> resolve the ISI+ISP issue!
\o/
>
>
> > This patch extends the media controller API with a "shared media graph"
> > framework. This allows drivers to share a media device, thus enabling
> > the setups mentioned above. Instead of owning and creating a media
> > device, drivers can join-or-create a shared media device via the shared
> > media graph API. The matching is done automatically based on the
> > detected endpoints in the device tree.
>
> Sounds great! I'll read on...
>
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> > drivers/media/mc/Makefile | 2 +-
> > drivers/media/mc/mc-shared-graph.c | 335 +++++++++++++++++++++++++++++
> > include/media/mc-shared-graph.h | 92 ++++++++
> > 3 files changed, 428 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/media/mc/mc-shared-graph.c
> > create mode 100644 include/media/mc-shared-graph.h
> >
> > diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
> > index 2b7af42ba59c..1d502fdc52ad 100644
> > --- a/drivers/media/mc/Makefile
> > +++ b/drivers/media/mc/Makefile
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > mc-objs := mc-device.o mc-devnode.o mc-entity.o \
> > - mc-request.o
> > + mc-request.o mc-shared-graph.o
> >
> > ifneq ($(CONFIG_USB),)
> > mc-objs += mc-dev-allocator.o
> > diff --git a/drivers/media/mc/mc-shared-graph.c b/drivers/media/mc/mc-shared-graph.c
> > new file mode 100644
> > index 000000000000..c4067e5b861d
> > --- /dev/null
> > +++ b/drivers/media/mc/mc-shared-graph.c
> > @@ -0,0 +1,335 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mc-shared-graph.c - Media Controller Shared Graph API
> > + *
> > + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> > + */
> > +
> > +/*
> > + * This file adds the Media Controller Shared Graph API. This allows drivers
> > + * to create shared media graphs or join existing media graphs from other
> > + * drivers, so that they can all be in the same media graph. This allows us to
> > + * have more complex media graphs chaining more complex hardware together,
> > + * instead of simple async subdevs.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/fwnode.h>
> > +#include <linux/kref.h>
> > +#include <linux/property.h>
> > +
> > +#include <media/media-device.h>
> > +
> > +#include <media/mc-shared-graph.h>
> > +
> > +static LIST_HEAD(media_device_shared_list);
> > +static DEFINE_MUTEX(media_device_shared_lock);
> > +
> > +struct media_device_shared_member {
> > + struct device *dev;
> > + struct fwnode_handle *fwnode;
> > + struct list_head list;
> > +};
> > +
> > +struct media_device_shared_link {
> > + struct media_entity *source;
> > + u16 source_pad;
> > + struct media_entity *sink;
> > + u16 sink_pad;
> > + u32 flags;
> > + struct list_head list;
> > +};
> > +
> > +// TODO figure out locking for when multiple drivers touch the media graph;
> > +// maybe macros for shared versions?
>
> Do you mean for when drivers are trying to change link state directly?
I meant for all the operations that act on media device. I'm not sure what
there is because I didn't really find anything significant, and I found some
action point from some meeting notes somewhere that said "deprecate media_ops"
(not assigned to me) so...
If there aren't any then it's a non-issue, but if there are then I was
wondering if we need to return the shared media device to the driver (as
opposed to a non-shared regular media device) and use shared versions of media
device functions that have locking.
>
> > +struct media_device_shared {
> > + struct media_device mdev;
> > + struct list_head members;
> > + struct list_head links;
> > +
> > + struct list_head list;
> > + struct kref refcount;
> > +
> > + struct device *removed_device;
> > +};
> > +
> > +static inline struct media_device_shared *
> > +to_media_device_shared(struct media_device *mdev)
> > +{
> > + return container_of(mdev, struct media_device_shared, mdev);
> > +}
> > +
> > +static void media_device_shared_release(struct kref *kref)
> > +{
> > + struct media_device_shared *mds =
> > + container_of(kref, struct media_device_shared, refcount);
> > +
> > + dev_dbg(mds->removed_device, "%s: releasing Media Device\n", __func__);
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + media_device_unregister(&mds->mdev);
> > + media_device_cleanup(&mds->mdev);
> > +
> > + list_del(&mds->list);
> > + mutex_unlock(&media_device_shared_lock);
> > +
> > + kfree(mds);
> > +}
> > +
> > +/* Callers should hold media_device_shared_lock when calling this function */
>
> Lets add a lockdep_assert then?
I learned something new today :)
>
>
> > +static bool __media_device_shared_find_match(struct media_device_shared *mds,
> > + struct fwnode_handle *fwnode)
> > +{
> > + struct media_device_shared_member *member;
> > + struct fwnode_handle *ep;
> > + struct fwnode_handle *remote_ep;
> > + bool match = false;
> > +
>
> is it just as easy as:
>
> lockdep_assert_held(&media_device_shared_lock);
> ?
If I understand correctly how lockdep_assert works, yes.
>
> > + // TODO: parse the device tree endpoints graph instead of finding just the
> > + // first-level neighbours
> > + fwnode_graph_for_each_endpoint(fwnode, ep) {
> > + list_for_each_entry(member, &mds->members, list) {
> > + remote_ep = fwnode_graph_get_remote_port_parent(ep);
> > + match = (member->fwnode == remote_ep);
> > + fwnode_handle_put(remote_ep);
> > +
> > + if (!match)
> > + continue;
> > +
> > + goto match_complete;
> > + }
> > + }
> > +
> > +match_complete:
> > + fwnode_handle_put(ep);
> > + return match;
> > +}
> > +
> > +/* Callers should hold media_device_shared_lock when calling this function */
>
> Lets add a lockdep check too then :D (same everywhere/anywhere it's
> needed)
>
>
> > +static struct media_device *__media_device_shared_get(struct device *dev)
> > +{
> > + struct media_device_shared *mds;
> > + struct media_device_shared_member *member;
> > + struct fwnode_handle *fwnode = dev_fwnode(dev);
> > + bool ret;
> > +
> > + dev_dbg(dev, "%s: searching for media device for %pfwf", __func__, fwnode);
> > +
> > + list_for_each_entry(mds, &media_device_shared_list, list) {
> > + ret = __media_device_shared_find_match(mds, fwnode);
> > + if (ret)
> > + break;
> > + }
> > +
> > + if (!ret)
> > + return NULL;
> > +
> > + member = kzalloc_obj(*member);
> > + if (!member)
> > + return NULL;
> > +
> > + member->dev = dev;
> > + member->fwnode = fwnode;
> > + list_add_tail(&member->list, &mds->members);
> > + kref_get(&mds->refcount);
> > +
> > + dev_dbg(dev, "%s: %pfwf joined media device of %pfwf",
> > + __func__, fwnode,
> > + list_first_entry(&mds->members, struct media_device_shared_member, list)->fwnode);
> > +
> > + return &mds->mdev;
> > +}
> > +
> > +/* Callers should hold media_device_shared_lock when calling this function */
> > +static struct media_device *__media_device_shared_create(struct device *dev)
> > +{
> > + struct media_device_shared *mds;
> > + struct media_device_shared_member *member;
> > + struct fwnode_handle *fwnode = dev_fwnode(dev);
> > + int ret;
> > +
> > + mds = kzalloc_obj(*mds);
> > + if (!mds)
> > + return NULL;
> > +
> > + member = kzalloc_obj(*member);
> > + if (!member)
> > + goto err_free_mds;
> > +
> > + media_device_init(&mds->mdev);
> > +
> > + ret = media_device_register(&mds->mdev);
> > + if (ret)
> > + goto err_free_member;
> > +
> > + INIT_LIST_HEAD(&mds->members);
> > + member->dev = dev;
> > + member->fwnode = fwnode;
> > + list_add_tail(&member->list, &mds->members);
> > +
> > + INIT_LIST_HEAD(&mds->links);
> > +
> > + kref_init(&mds->refcount);
> > + list_add_tail(&mds->list, &media_device_shared_list);
> > +
> > + // TODO figure out how to reconcile this with multiple members
>
> Aha, right - becuse the 'media_device dev' becomes whoever registers first.
> I wonder where it's actually used, and maybe that's ok ?
Yes, exactly. So far all I could find it used for was dev_err etc. In my
testing it hasn't caused any problems, since I tested both loading orders
between the rkcif and rkisp2. Maybe I missed something important and somebody
else knows better.
Although it's not very nice to have prints from the other device. Is making a
thin device an option?
>
> > + mds->mdev.dev = dev;
> > +
> > + devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
> > + __func__, fwnode, &mds->mdev);
> > + return &mds->mdev;
> > +
> > +err_free_member:
> > + kfree(member);
> > +err_free_mds:
> > + kfree(mds);
> > + return NULL;
> > +}
> > +
> > +// TODO figure out how to resolve the identifiers (model, driver name, etc);
> > +// atm it's racy and whoever gets it last wins
>
> Maybe that's something we need to pass or set up explicitly then, so
> it's clear it's a 'specific graph'
Yes. I have no clue where that name would come from though.
Although I thought userspace matches on the entity names of the media device?
Does the name of the graph matter? It needs to be consistent though imo.
I considered having a list of names of members, but I'm not sure how useful
that would be. We don't have a userspace API to check it, plus it can already
just list the member entities of the graph. And the driver doesn't really care
who else is in the graph, since the shared media graph machinery already
automatches based on dt endpoints.
Also imo it's also not too nice for userspace when {driver_name} refers to a
driver that doesn't exist.
>
> Would this give us a way to represent the hardware in a new form - i.e.
> thinking about IMX8MP - the existing graph wouldn't be available - so it
> wouldn't be any worry from a UABI perspective because it's just a whole
> new interface/media-device name....
Hm, maybe the media device name could just be something along the lines of
"shared-{platform_name}-media"? Although I suppose that assumes that each
platform only has one media graph and we'll run into problems when we have
multiple. Or we append the youngest register block address in the shared media
graph to the name?
>
>
>
> > +struct media_device *media_device_shared_join(struct device *dev)
> > +{
> > + struct media_device *mdev;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + mdev = __media_device_shared_get(dev);
> > + if (!!mdev) {
> > + dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
> > + mutex_unlock(&media_device_shared_lock);
> > + return mdev;
> > + }
> > +
> > + mdev = __media_device_shared_create(dev);
> > + if (!mdev) {
> > + dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
> > + mutex_unlock(&media_device_shared_lock);
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > + dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
> > + mutex_unlock(&media_device_shared_lock);
> > + return mdev;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_join);
> > +
> > +void media_device_shared_leave(struct media_device *mdev, struct device *dev)
> > +{
> > + struct media_device_shared *mds = to_media_device_shared(mdev);
> > + struct media_device_shared_member *member;
> > + struct media_device_shared_member *member_tmp;
> > + bool removed = false;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + list_for_each_entry_safe(member, member_tmp, &mds->members, list) {
> > + if (member->dev == dev) {
> > + list_del(&member->list);
> > + kfree(member);
> > + removed = true;
> > + }
> > + }
> > +
> > + if (!removed)
> > + dev_err(dev, "%s: %pfwf trying to leave from graph in which not a member",
> > + __func__, dev_fwnode(dev));
> > +
> > + mds->removed_device = dev;
> > + mutex_unlock(&media_device_shared_lock);
> > + kref_put(&mds->refcount, media_device_shared_release);
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_leave);
> > +
> > +int media_device_shared_join_link_source(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *source,
> > + u16 source_pad, u32 flags)
> > +{
> > + struct media_device_shared *mds = to_media_device_shared(mdev);
> > + struct media_device_shared_link *link;
> > + struct media_device_shared_link *link_tmp;
> > + int ret = 0;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + /*
> > + * TODO Figure out flags. Should we use greatest common denominator? Or
> > + * prioritize sink? Or whoever wins the race? For now we just take the flags
> > + * from the sink.
> > + *
> > + * TODO Figure out how to actually do the matching. For now we just match
> > + * whoever comes in first. This works with the simple example we're running
> > + * with now (rkcif + one rkisp2) but with setups with multiple copies of
> > + * hardware this will cause problems, like with rkcif + two rkisp2 and
> > + * imx8-isi + two rkisp1.
> > + */
> > + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> > + if (link->sink) {
> > + ret = media_create_pad_link(source, source_pad,
> > + link->sink, link->sink_pad,
> > + link->flags);
> > + list_del(&link->list);
> > + kfree(link);
> > + goto exit_join_link_source;
> > + }
> > + }
> > +
> > + link = kzalloc_obj(*link);
> > + if (!link) {
> > + ret = -ENOMEM;
> > + goto exit_join_link_source;
> > + }
> > +
> > + link->source = source;
> > + link->source_pad = source_pad;
> > + link->flags = flags;
> > + list_add_tail(&link->list, &mds->links);
> > +
> > +exit_join_link_source:
> > + mutex_unlock(&media_device_shared_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
> > +
> > +// TODO deduplicate from above
>
> Indeed, it does look like an opportunity.
Ok this is the one exception that was an actual todo and not a topic for
discussion :)
(Unless someone has the opinion "actually we don't need to deduplicate this")
>
> > +int media_device_shared_join_link_sink(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *sink,
> > + u16 sink_pad, u32 flags)
> > +{
> > + struct media_device_shared *mds = to_media_device_shared(mdev);
> > + struct media_device_shared_link *link;
> > + struct media_device_shared_link *link_tmp;
> > + int ret = 0;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> > + if (link->source) {
> > + ret = media_create_pad_link(link->source, link->source_pad,
> > + sink, sink_pad,
> > + flags);
> > + list_del(&link->list);
> > + kfree(link);
> > + goto exit_join_link_sink;
> > + }
> > + }
> > +
> > + link = kzalloc_obj(*link);
> > + if (!link) {
> > + ret = -ENOMEM;
> > + goto exit_join_link_sink;
> > + }
> > +
> > + link->sink = sink;
> > + link->sink_pad = sink_pad;
> > + link->flags = flags;
> > + list_add_tail(&link->list, &mds->links);
> > +
> > +exit_join_link_sink:
> > + mutex_unlock(&media_device_shared_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_join_link_sink);
> > diff --git a/include/media/mc-shared-graph.h b/include/media/mc-shared-graph.h
> > new file mode 100644
> > index 000000000000..487325163f84
> > --- /dev/null
> > +++ b/include/media/mc-shared-graph.h
> > @@ -0,0 +1,92 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mc-shared-graph.h - Media Controller Shared Graph API
> > + *
> > + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> > + */
> > +
> > +/*
> > + * This file adds the Media Controller Shared Graph API. This allows drivers
> > + * to create shared media graphs or join existing media graphs from other
> > + * drivers, so that they can all be in the same media graph. This allows us to
> > + * have more complex media graphs chaining more complex hardware together,
> > + * instead of simple async subdevs.
> > + */
> > +
> > +#include <linux/types.h>
> > +
> > +#ifndef _MEDIA_SHARED_GRAPH_H
> > +#define _MEDIA_SHARED_GRAPH_H
> > +
> > +struct device;
> > +struct media_device;
> > +struct media_entity;
> > +
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +/**
> > + * media_device_shared_join() - Join or create a new shared media device
> > + *
> > + * @dev: struct &device pointer
> > + *
> > + * This is the entrance function for a device to join or create a new shared
> > + * media device. It searches for an existing shared media device based on the
> > + * neighbours in the device's device tree ports node. If found, then this
> > + * functions returns the existing shared media device and joins it. If one is
> > + * not found then one is created and initialized and returned.
> > + */
> > +struct media_device *media_device_shared_join(struct device *dev);
> > +
> > +/**
> > + * media_device_shared_leave() - Leave the shared media device.
> > + *
> > + * @mdev: struct &media_device pointer
> > + * @dev: struct &device pointer
> > + *
> > + * This function makes the device leave the shared media device. When all
> > + * members have left the media device it will be freed.
> > + */
> > +void media_device_shared_leave(struct media_device *mdev, struct device *dev);
> > +
> > +/**
> > + * media_device_shared_join_link_source() - Register a link source in the shared media device
> > + *
> > + * @mdev: The struct &media_device pointer that is part of a shared media device
> > + * @dev: struct &device pointer
> > + * @source: The link source
> > + * @source_pad: The pad
> > + * @flags: The flags
> > + *
> > + * This function registers with the shared media device the source part of a
> > + * link. When the shared media device receives the matching sink part of a link
> > + * via media_device_shared_join_link_sink() then the link will be fully created.
> > + */
> > +int media_device_shared_join_link_source(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *source,
> > + u16 source_pad, u32 flags);
> > +
> > +/**
> > + * media_device_shared_join_link_sink() - Register a link sink in the shared media device
> > + *
> > + * Same as media_device_shared_join_link_source() but for sink instead of
> > + * source.
> > + */
> > +int media_device_shared_join_link_sink(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *sink,
> > + u16 sink_pad, u32 flags);
> > +#else
> > +static inline struct media_device *media_device_shared_join(struct device *dev)
> > +{ return NULL; }
> > +static inline void media_device_shared_leave(struct media_device *mdev,
> > + struct device *dev) { }
> > +static inline int media_device_shared_join_link_source(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *source,
> > + u16 source_pad, u32 flags) { }
> > +static inline int media_device_shared_join_link_sink(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *sink,
> > + u16 sink_pad, u32 flags) { }
>
> Should these two stubs which return an int return an error code? -ENODEV or such ?
Ah yes they should. I missed that.
Thanks,
Paul
>
>
> > +#endif /* CONFIG_MEDIA_CONTROLLER */
> > +#endif /* _MEDIA_DEV_SHARED_GRAPH_H */
> > --
> > 2.47.2
> >
^ permalink raw reply
* Re: [PATCH v7 25/30] drm/vc4: hdmi: Convert to common HDMI 2.0 SCDC scrambling helpers
From: Cristian Ciocaltea @ 2026-06-25 8:19 UTC (permalink / raw)
To: Maxime Ripard
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260625-busy-ultra-oryx-ffb3c9@houat>
On 6/25/26 10:44 AM, Maxime Ripard wrote:
> On Sat, Jun 13, 2026 at 03:41:20AM +0300, Cristian Ciocaltea wrote:
>> Hi Maxime,
>>
>> On 6/12/26 3:04 PM, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Tue, Jun 02, 2026 at 01:44:25AM +0300, Cristian Ciocaltea wrote:
>>>> Replace the vc4-local scrambling implementation with the newly
>>>> introduced DRM common SCDC scrambling infrastructure:
>>>>
>>>> - Advertise source-side scrambling support by setting
>>>> connector->hdmi.scrambling_supported based on the variant's
>>>> max_pixel_clock before drmm_connector_hdmi_init().
>>>>
>>>> - Provide minimal .scrambler_{enable|disable} connector callbacks that
>>>> only toggle the VC5 HDMI_SCRAMBLER_CTL register. Sink-side SCDC
>>>> programming and periodic status monitoring are now delegated to
>>>> drm_scdc_{start|stop}_scrambling().
>>>>
>>>> - Replace vc4_hdmi_enable_scrambling() with a conditional call to
>>>> drm_scdc_start_scrambling() in post_crtc_enable, gated on
>>>> conn_state->hdmi.scrambler_needed (computed by the HDMI state helper).
>>>>
>>>> - Replace vc4_hdmi_disable_scrambling() with drm_scdc_stop_scrambling()
>>>> in post_crtc_disable.
>>>>
>>>> - Drop vc4_hdmi_reset_link() and vc4_hdmi_handle_hotplug(), switching
>>>> the .detect_ctx() path to
>>>> drm_atomic_helper_connector_hdmi_hotplug_ctx() which internally calls
>>>> drm_scdc_sync_status() to trigger a CRTC reset on reconnection.
>>>>
>>>> - Drop the local scrambling_work delayed workqueue and scdc_enabled
>>>> flag, now tracked by the common drm_connector_hdmi layer.
>>>>
>>>> - Drop vc4_hdmi_supports_scrambling() and
>>>> vc4_hdmi_mode_needs_scrambling() helpers, inlining the remaining 4KP60
>>>> warning with an explicit drm_hdmi_compute_mode_clock() check.
>>>>
>>>> - Seed connector->hdmi.scrambler_enabled = true in connector_init() to
>>>> ensure drm_scdc_stop_scrambling() runs at boot and disables any stale
>>>> scrambling state left by the bootloader.
>>>>
>>>> No functional change is expected for the supported modes.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>
>>> I'd really like it to be broken down into several patches:
>>>
>>>> ---
>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 265 ++++++-----------------------------------
>>>> drivers/gpu/drm/vc4/vc4_hdmi.h | 8 --
>>>> 2 files changed, 35 insertions(+), 238 deletions(-)
>>>>
[...]
>>>> -
>>>> - /*
>>>> - * HDMI 2.0 says that one should not send scrambled data
>>>> - * prior to configuring the sink scrambling, and that
>>>> - * TMDS clock/data transmission should be suspended when
>>>> - * changing the TMDS clock rate in the sink. So let's
>>>> - * just do a full modeset here, even though some sinks
>>>> - * would be perfectly happy if were to just reconfigure
>>>> - * the SCDC settings on the fly.
>>>> - */
>>>> - return drm_atomic_helper_reset_crtc(crtc, ctx);
>>>> -}
>>>
>>> This one doesn't look functionally equivalent to me to
>>> drm_scdc_reset_crtc: this part was, in part, making sure we would only
>>> reset the scrambler if it was enabled in the first place.
>>> drm_scdc_reset_crtc() doesn't and will always trigger a modeset on
>>> hotplug. That's unnecessary and a significant functional different.
>>
>> drm_scdc_reset_crtc() alone was not meant to be an equivalent of
>> vc4_hdmi_reset_link(), as it only checks the sink side and it serves as an
>> internal helper used exclusively by drm_scdc_sync_status().
>>
>> As a matter of fact, the latter is the one responsible for verifying if the
>> scrambler was enabled on the controller side before attempting to invoke the
>> reset logic, hence we should get the same behavior. But we don't invoke it
>> directly either, it's part of the drm_atomic_helper_connector_hdmi_hotplug_ctx()
>> call path.
>
> Oh, right, sorry.
>
>>> I'd argue that it's drm_scdc_reset_crtc() that needs to align to what
>>> vc4 was doing, not the opposite.
>>
>> The only difference consists in dropping the crtc state check:
>>
>> ret = drm_modeset_lock(&crtc->mutex, ctx);
>> if (ret)
>> return ret;
>>
>> crtc_state = crtc->state;
>> if (!crtc_state->active)
>> return 0;
>>
>> The rationale was that when CRTC is inactive, drm_atomic_helper_reset_crtc()
>> should result in a no-op commit anyway.
>
> A commit is expensive, so I'd skip it if we can.
>
>> And the one for the in-flight commit:
>>
>> if (conn_state->commit &&
>> !try_wait_for_completion(&conn_state->commit->hw_done)) {
>> mutex_unlock(&vc4_hdmi->mutex);
>> return 0;
>> }
>
> And yeah, we'll need this one too.
>
>> Both checks are also missing in drm_bridge_helper_reset_crtc(), taken as an
>> initial reference. Should we still keep any/both and sync the bridge helper
>> accordingly?
>
> Yes, but I'd expect the bridge helpers to converge / reuse your helpers
> eventually anyway?
Ack.
[...]
>>>> - vc4_hdmi_disable_scrambling(encoder);
>>>> + drm_scdc_stop_scrambling(&vc4_hdmi->connector);
>>>
>>> I don't think the names here are right. It's not *only* related to scdc
>>> but also to the HDMI controller. I'm fine with using a scdc prefix but
>>> only for the things related to scdc. This is related (in part) to the
>>> HDMI controller, so it should use a drm_connector_hdmi prefix.
>>
>> Ack. I guess we should also move these helpers out of drm_scdc_helper.c, but
>> unsure where. FWIW I'm currently working on adding HDMI 2.1 FRL support, and
>> implemented the link training in a dedicated drm_hdmi_frl_helper.c.
>>
>> Should we create drm_hdmi_scrambler_helper.c? Or maybe have a common one to
>> hold both - any suggestion for the naming?
>
> display/drm_hdmi_helper.c sounds like a great place for both?
Indeed, let's move all the stuff there.
>>>> drm_dev_exit(idx);
>>>>
>>>> @@ -1625,6 +1434,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
>>>> struct drm_display_info *display = &vc4_hdmi->connector.display_info;
>>>> bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
>>>> bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
>>>> + struct drm_connector_state *conn_state;
>>>> unsigned long flags;
>>>> int ret;
>>>> int idx;
>>>> @@ -1693,7 +1503,10 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
>>>> }
>>>>
>>>> vc4_hdmi_recenter_fifo(vc4_hdmi);
>>>> - vc4_hdmi_enable_scrambling(encoder);
>>>> +
>>>> + conn_state = drm_atomic_get_new_connector_state(state, connector);
>>>> + if (conn_state && conn_state->hdmi.scrambler_needed)
>>>> + drm_scdc_start_scrambling(connector);
>>>
>>> And the nice thing with a drm_connector_hdmi_* prefix is that you don't
>>> have to shoehorn it into SCDC support anymore, so you can take a state
>>> as an argument, and do the check in the helper instead of doing it in
>>> every driver and hoping they get it right.
>>
>> I was about to consider a similar approach before deciding to let drivers manage
>> the logic, i.e. to prevent loosing flexibility later when dealing with HDMI 2.1.
>> That was mostly in the context of supporting drivers to define if/when a display
>> mode that fits in TMDS should be sent over FRL.
>>
>> Thinking again, that's not really a valid concern right now, e.g. will use TMDS
>> by default for all supported modes, and switch to FRL only when absolutely
>> required. Later we might consider extending the infrastructure to support
>> dynamic control if required.
>
> Thanks for working on FRL as well :)
>
> I agree, let's focus on getting HDMI 2.0 right, and then we'll try to
> make 2.1 the easiest to work with for drivers.
Thanks for clarifying the remaining details — I should now have everything I
need to prepare a new revision. :)
Regards,
Cristian
^ permalink raw reply
* Re: [PATCH RFC v4 10/12] reset: zte: Add a zx297520v3 reset driver
From: Philipp Zabel @ 2026-06-25 8:17 UTC (permalink / raw)
To: Stefan Dösinger, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney
Cc: linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <7_XKp5ZWTJeJfxJieymOJA@gmail.com>
On Mi, 2026-06-24 at 23:00 +0300, Stefan Dösinger wrote:
> Hi Philipp,
>
> Am Donnerstag, 18. Juni 2026, 12:24:26 Ostafrikanische Zeit schrieb Philipp
> Zabel:
>
> > > + [ZX297520V3_UART0_RESET] = { .reg = 0x78, .mask = BIT(6) |
> BIT(7)
> > > },
> > Is this a single reset line controlled by two bits (do you know what
> > they are)? Or might these actually be two different reset controls that
> > are just always set together?
>
> I suppose I could expose both bits as separate reset controls in the binding.
> The lower bit is usually the one that actually resets the device, while the
> higher one works similarly to PCLK - it disconnects the device from the bus,
> if asserted. Depending on the device it may or may not leave any residual
> effect behind after deassert.
So it's not a separate reset.
Whether bus isolation should be controlled together with the reset or
not could be argued, but exposing this as a separate reset control via
the reset API would not be correct.
> The stumbling block is the dwc2 USB driver. It only takes one reset, so I'd
> have to add another one (or abuse the dwc2-ecc reset) and presumably add a PHY
> driver for the 3rd reset or add a dwc2-phy reset.
This on the other hand sounds like there is a separate PHY reset line?
If so, I think that should be modeled as a separate control.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v7 04/30] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers
From: Maxime Ripard @ 2026-06-25 8:05 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260602-dw-hdmi-qp-scramb-v7-4-445eb54ee1ed@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]
On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
> +static int drm_scdc_reset_crtc(struct drm_connector *connector,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_crtc *crtc;
> + u8 config;
> + int ret;
> +
> + if (!ctx)
> + return 0;
> +
> + /*
> + * This is normally part of .detect_ctx() call path, which already holds
> + * connection_mutex through @ctx. However, re-acquiring it with the
> + * same context is a no-op and makes the helper safe under any caller.
> + */
> + ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
> + if (ret)
> + return ret;
> +
> + if (!connector->state)
> + return 0;
> +
> + crtc = connector->state->crtc;
> + if (!crtc)
> + return 0;
> +
> + ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> + if (ret) {
> + drm_scdc_dbg(connector, "Failed to read TMDS config: %d\n", ret);
> + return ret;
> + }
> +
> + if ((config & SCDC_SCRAMBLING_ENABLE) &&
> + (config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40))
> + return 0;
I don't think we should care about the bit clock ratio here. From a
technical standpoint, and since you ignore low rates for the scrambler
for now, they are indeed equivalents. But semantically, scrambler can be
enabled for any rate, and the bit clock ratio should be changed only for
rates higher than 340MHz.
This function makes sure to trigger a modeset when the scrambler is
enabled or not. From a spec point of view, it's somewhat orthogonal to
the bit clock ratio. From a function name point of view, it's
misleading.
So yeah, I'd just drop the check on the bit clock ratio here. Drivers
will then get to enable either when the commit actually happens.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH] pinctrl: imx1: fix device_node leak in dt_is_flat_functions()
From: Linus Walleij @ 2026-06-25 8:04 UTC (permalink / raw)
To: Felix Gu
Cc: Dong Aisheng, Fabio Estevam, Frank Li, Jacky Bai,
Pengutronix Kernel Team, NXP S32 Linux Team, Sascha Hauer,
linux-gpio, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20260523-pinctrl-imx-v1-1-73b7cb731351@gmail.com>
On Sat, May 23, 2026 at 12:27 PM Felix Gu <ustc.gu@gmail.com> wrote:
> for_each_child_of_node() holds a reference on the iterator node that
> must be released on early return. imx1_pinctrl_dt_is_flat_functions()
> has two early return paths inside the loop that skip this cleanup.
>
> Replace both loops with the scoped variant so that the reference is
> automatically dropped when the iterator goes out of scope.
>
> Fixes: 63d2059cd665 ("pinctrl: imx1: Allow parsing DT without function nodes")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
Patch applied for v7.3!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support
From: Krzysztof Kozlowski @ 2026-06-25 7:58 UTC (permalink / raw)
To: Joey Lu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Arnd Bergmann, Catalin Marinas, Jacky Huang,
Shan-Chun Hung, Hui-Ping Chen, Joey Lu, linux-phy, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260625023958.569299-3-a0987203069@gmail.com>
On Thu, Jun 25, 2026 at 10:39:56AM +0800, Joey Lu wrote:
> properties:
> compatible:
> enum:
> - nuvoton,ma35d1-usb2-phy
>
> + reg:
> + maxItems: 1
> +
> "#phy-cells":
> - const: 0
> + const: 1
> + description:
> + The single cell selects the PHY port. 0 selects the OTG port (USB0,
> + shared with DWC2 gadget controller) and 1 selects the host-only port
> + (USB1).
>
> - clocks:
> - maxItems: 1
This is odd, considering that parent does not have clocks. So explain me
this:
1. USB PHY needed clocks.
2. You extend USB PHY to cover second part.
3. That extension for second part means that clocks are not needed.
Really, how? How is it possible in hardware?
> + nuvoton,rcalcode:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
You should require two values. I understand that any PHY is optional,
thus you skip the entry, so how would you provide value for PHY1 only?
> + items:
> + minimum: 0
> + maximum: 15
> + description:
> + Resistor calibration trim codes for PHY0 and PHY1 respectively.
> + Each 4-bit value is written to the RCALCODE field in USBPMISCR and
> + adjusts the PHY's internal termination resistance. Both entries are
> + optional; when absent the hardware reset default is used.
>
> - nuvoton,sys:
> - $ref: /schemas/types.yaml#/definitions/phandle
> + nuvoton,oc-active-high:
> + type: boolean
> description:
> - phandle to syscon for checking the PHY clock status.
> + When present, the over-current detect input from the VBUS power switch
> + is treated as active-high. The default (property absent) is active-low.
> + This setting is shared by both USB host ports.
>
> required:
> - compatible
> + - reg
That's ABI break which was not explained in the commit msg - neither
specifying impact nor actually providing reasons why you break ABI.
And honestly, you have no resources here except the address, so now it
is clear that this should be folded into parent. See DTS101 talk slides.
> - "#phy-cells"
> - - clocks
> - - nuvoton,sys
>
> additionalProperties: false
>
> examples:
> - |
> - #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> + system-management@40460000 {
> + compatible = "nuvoton,ma35d1-reset", "syscon", "simple-mfd";
> + reg = <0x40460000 0x200>;
> + #reset-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
Drop. Keep only child node and make parent binding example complete.
>
> - usb_phy: usb-phy {
> - compatible = "nuvoton,ma35d1-usb2-phy";
> - clocks = <&clk USBD_GATE>;
> - nuvoton,sys = <&sys>;
> - #phy-cells = <0>;
> + usb-phy@60 {
> + compatible = "nuvoton,ma35d1-usb2-phy";
> + reg = <0x60 0x14>;
> + #phy-cells = <1>;
> + };
> };
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v7 04/30] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers
From: Maxime Ripard @ 2026-06-25 7:53 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <2f5ce8eb-ef47-4238-ad68-ce62981a1845@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 3076 bytes --]
On Sat, Jun 13, 2026 at 04:30:02AM +0300, Cristian Ciocaltea wrote:
> On 6/12/26 4:43 PM, Maxime Ripard wrote:
> > On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
> >> +/**
> >> + * drm_scdc_start_scrambling - activate scrambling and monitor SCDC status
> >> + * @connector: connector
> >> + *
> >> + * Enables scrambling and high TMDS clock ratio on both source and sink sides.
> >> + * Additionally, use a delayed work item to monitor the scrambling status on
> >> + * the sink side and retry the operation, as some displays refuse to set the
> >> + * scrambling bit right away.
> >> + *
> >> + * Returns:
> >> + * Zero if scrambling is set successfully, an error code otherwise.
> >> + */
> >> +int drm_scdc_start_scrambling(struct drm_connector *connector)
> >> +{
> >> + struct drm_display_info *info = &connector->display_info;
> >> + struct drm_connector_hdmi *hdmi = &connector->hdmi;
> >> + int ret;
> >> + u8 ver;
> >> +
> >> + if (!hdmi->scrambler_supported) {
> >> + drm_scdc_dbg(connector, "Scrambler not supported, bailing.\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!info->is_hdmi ||
> >> + !info->hdmi.scdc.supported ||
> >> + !info->hdmi.scdc.scrambling.supported) {
> >> + drm_scdc_dbg(connector, "Sink doesn't support scrambling.\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + drm_scdc_dbg(connector, "Enabling scrambling\n");
> >> +
> >> + ret = drm_scdc_readb(connector->ddc, SCDC_SINK_VERSION, &ver);
> >> + if (ret) {
> >> + drm_scdc_dbg(connector, "Failed to read SCDC_SINK_VERSION: %d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + ret = drm_scdc_writeb(connector->ddc, SCDC_SOURCE_VERSION,
> >> + min_t(u8, ver, SCDC_MAX_SOURCE_VERSION));
> >> + if (ret) {
> >> + drm_scdc_dbg(connector, "Failed to write SCDC_SOURCE_VERSION: %d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + hdmi->scdc_cb = drm_scdc_monitor_scrambler;
> >> + WRITE_ONCE(hdmi->scrambler_enabled, true);
> >> +
> >> + ret = drm_scdc_try_scrambling_setup(connector);
> >> + if (!ret)
> >> + ret = hdmi->funcs->scrambler_enable(connector);
> >> +
> >> + if (ret) {
> >> + WRITE_ONCE(hdmi->scrambler_enabled, false);
> >> + cancel_delayed_work_sync(&hdmi->scdc_work);
> >> + hdmi->scdc_cb = NULL;
> >> +
> >> + drm_scdc_set_scrambling(connector, false);
> >> + drm_scdc_set_high_tmds_clock_ratio(connector, false);
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL(drm_scdc_start_scrambling);
> >
> > This is also pretty significantly different than what vc4 implemented. I
> > don't necessarily mind, but claiming that it's functionally equivalent
> > isn't true. I think it would be a much better strategy to turn the vc4
> > code into helpers as is because it's been merged 4 years ago and we know
> > it's solid.
>
> As explained in patch 25, I think there's been a slight misunderstanding of how
> the CRTC reset logic was actually implemented.
You're absolutely right, let me review it again. sorry.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: reset: nuvoton,ma35d1-reset: add simple-mfd and child node support
From: Krzysztof Kozlowski @ 2026-06-25 7:51 UTC (permalink / raw)
To: Joey Lu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Arnd Bergmann, Catalin Marinas, Jacky Huang,
Shan-Chun Hung, Hui-Ping Chen, Joey Lu, linux-phy, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260625023958.569299-2-a0987203069@gmail.com>
On Thu, Jun 25, 2026 at 10:39:55AM +0800, Joey Lu wrote:
> The MA35D1 system-management syscon node hosts the USB PHY register
> block at offset 0x60. To model usb-phy@60 as a DT child of the syscon
> node the binding must allow:
Explain why do you need child node. If you have fixed device @0x60, you do
not need DT child node at all. Compatible implies that child existence.
>
> - simple-mfd as an optional third compatible so the MFD core can
> instantiate child platform devices.
>
> - #address-cells and #size-cells (each const: 1) so child nodes can
> carry a reg property.
>
> - An open child-node pattern (patternProperties "^.*@[0-9a-f]+$")
> to pass dt-schema validation.
No. Do not explain what you did - we can read the diff. You must explain
WHY you are doing that.
>
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
> .../bindings/reset/nuvoton,ma35d1-reset.yaml | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> index 3ce7dcecd87a..1fda7e8f4b5d 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> @@ -19,6 +19,8 @@ properties:
> items:
> - const: nuvoton,ma35d1-reset
> - const: syscon
> + - const: simple-mfd
> + minItems: 2
>
> reg:
> maxItems: 1
> @@ -26,6 +28,16 @@ properties:
> '#reset-cells':
> const: 1
>
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 1
> +
> +patternProperties:
> + "^.*@[0-9a-f]+$":
This must be specific.
> + type: object
Missing ref and additionalProps. Please look at other simple-mfd.
> +
> required:
> - compatible
> - reg
> @@ -43,4 +55,3 @@ examples:
> #reset-cells = <1>;
> };
> ...
> -
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH 0/3] KVM: arm64: nv: Shadow ptdump fixes
From: Wei-Lin Chang @ 2026-06-25 7:47 UTC (permalink / raw)
To: Itaru Kitayama
Cc: linux-arm-kernel, kvmarm, linux-kernel, Marc Zyngier,
Oliver Upton, Joey Gouly, Steffen Eiden, Suzuki K Poulose,
Zenghui Yu, Catalin Marinas, Will Deacon
In-Reply-To: <ajty6I7ZqodP4ous@sm-arm-grace07>
Hi Itaru,
On Wed, Jun 24, 2026 at 03:02:16PM +0900, Itaru Kitayama wrote:
> Hi Wei-Lin,
>
> On Tue, Jun 23, 2026 at 03:24:40PM +0100, Wei-Lin Chang wrote:
> > Hi,
> >
> > This series fixes two bugs regarding the shadow ptdump debugfs files.
> > It is based on kvmarm/fixes + [1] ("KVM: arm64: Reassign nested_mmus
> > array behind mmu_lock").
> >
> > The first is a UAF. A nested mmu can still be accessed when the debugfs
> > file is being closed, after the nested mmus are freed. I can observe
> > this by turning on CONFIG_KASAN and closing the file after the VM is
> > destroyed. To fix this, mmu access is avoided in the .release()
> > callback.
> >
> > The second is sleeping in atomic context, found by Itaru [2] (thanks).
> > Originally the code creates a debugfs file whenever a context gets bound
> > to an s2 mmu instance, and deletes it when it gets unbound. Problem is
> > the bind/unbind is done with the mmu_lock held, and debugfs file
> > creation and deletion can sleep. This is observable by using
> > CONFIG_DEBUG_ATOMIC_SLEEP. The new approach is just have one debugfs
> > file for each s2 mmu instance, and show their state + information when
> > requested, which can be invalid, or VTCR + VTTBR + whether s2 enabled +
> > ptdump.
> >
> > The fixes are tested with CONFIG_PROVE_LOCKING,
> > CONFIG_DEBUG_ATOMIC_SLEEP, and CONFIG_KASAN.
> >
> > Thanks!
> > Wei-Lin Chang
> >
> > [1]: https://lore.kernel.org/kvmarm/aiKIVVeIr1aAB1yp@v4bel/
> > [2]: https://lore.kernel.org/kvmarm/aiuF0KSvvv-ZozI1@sm-arm-grace07/
> >
> > Wei-Lin Chang (3):
> > KVM: arm64: nv: Print nested mmu info in kvm_ptdump_guest_show()
> > KVM: arm64: ptdump: Store both mmu and kvm pointers in
> > kvm_ptdump_guest_state
> > KVM: arm64: nv: Move to per nested mmu ptdump files
> >
> > arch/arm64/kvm/nested.c | 16 +++++++++++-----
> > arch/arm64/kvm/ptdump.c | 29 +++++++++++++++++++----------
> > 2 files changed, 30 insertions(+), 15 deletions(-)
> >
> > --
> > 2.43.0
>
> At end of the execution of the shadow stage 2 selftest I see:
>
> [ 569.228448] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000098
> [ 569.228712] Mem abort info:
> [ 569.229091] ESR = 0x0000000096000046
> [ 569.229165] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 569.229213] SET = 0, FnV = 0
> [ 569.229244] EA = 0, S1PTW = 0
> [ 569.229276] FSC = 0x06: level 2 translation fault
> [ 569.229312] Data abort info:
> [ 569.229341] ISV = 0, ISS = 0x00000046, ISS2 = 0x00000000
> [ 569.229369] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> [ 569.229397] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 569.229458] user pgtable: 4k pages, 48-bit VAs, pgdp=000000006dce3000
> [ 569.229545] [0000000000000098] pgd=0800000048b63403, p4d=0800000048b63403, pud=0800000048b7f403, pmd=0000000000000
> ** replaying previous printk message **
> [ 569.229545] [0000000000000098] pgd=0800000048b63403, p4d=0800000048b63403, pud=0800000048b7f403, pmd=0000000000000000
> [ 569.236428] Internal error: Oops: 0000000096000046 [#1] SMP
> [ 569.237974] Modules linked in:
> [ 569.238644] CPU: 1 UID: 0 PID: 824 Comm: shadow_stage2 Not tainted 7.1.0-rc4+ #59 PREEMPT(full)
> [ 569.239139] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2ubuntu0.7 11/27/2025
> [ 569.239632] pstate: 61402009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [ 569.240004] pc : down_write+0x50/0xe8
> [ 569.240450] lr : down_write+0x34/0xe8
> [ 569.240696] sp : ffff80008252ba20
> [ 569.240965] x29: ffff80008252ba20 x28: 0000000000000000 x27: 0000000040000200
> [ 569.241346] x26: 0000000000000200 x25: ffffd1bf542891a0 x24: 0000000000000001
> [ 569.241625] x23: 0000000000000098 x22: ffff000000637480 x21: ffffd1bf57abc518
> [ 569.241985] x20: 0000000000000000 x19: 0000000000000098 x18: ffff80008253d090
> [ 569.242261] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 569.242568] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [ 569.242904] x11: 0000000000000000 x10: 0000000000000000 x9 : ffffd1bf5532388c
> [ 569.243335] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> [ 569.243638] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [ 569.244056] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000000000
> [ 569.244507] Call trace:
> [ 569.244778] down_write+0x50/0xe8 (P)
> [ 569.245094] __simple_recursive_removal+0x68/0x230
> [ 569.245322] simple_recursive_removal+0x20/0x50
> [ 569.245498] debugfs_remove+0x64/0xc0
> [ 569.245655] kvm_nested_s2_ptdump_remove_debugfs+0x20/0x48
> [ 569.245960] kvm_arch_flush_shadow_all+0x4c/0xc0
> [ 569.246100] kvm_mmu_notifier_release+0x3c/0x90
> [ 569.246344] mmu_notifier_unregister+0x68/0x148
> [ 569.246594] kvm_destroy_vm+0x130/0x2d8
> [ 569.246829] kvm_device_release+0xf8/0x170
> [ 569.246969] __fput+0xf4/0x350
> [ 569.247147] fput_close_sync+0x4c/0x138
> [ 569.247291] __arm64_sys_close+0x44/0xa0
> [ 569.247493] invoke_syscall+0xa8/0x138
> [ 569.247727] el0_svc_common.constprop.0+0x4c/0x140
> [ 569.248059] do_el0_svc+0x28/0x58
> [ 569.248236] el0_svc+0x48/0x218
> [ 569.248420] el0t_64_sync_handler+0xc0/0x108
> [ 569.248690] el0t_64_sync+0x1b4/0x1b8
> [ 569.249737] Code: b9000820 d503201f d2800000 d2800021 (c8e07e61)
> [ 569.250624] ---[ end trace 0000000000000000 ]---
> [ 569.251589] note: shadow_stage2[824] exited with preempt_count 1
> [ 569.253677] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000098
> [ 569.254391] Mem abort info:
> [ 569.254416] ESR = 0x0000000096000046
> [ 569.254436] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 569.254479] SET = 0, FnV = 0
> [ 569.254493] EA = 0, S1PTW = 0
> [ 569.254506] FSC = 0x06: level 2 translation fault
> [ 569.254522] Data abort info:
> [ 569.254530] ISV = 0, ISS = 0x00000046, ISS2 = 0x00000000
> [ 569.254544] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> [ 569.254559] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 569.254574] user pgtable: 4k pages, 48-bit VAs, pgdp=000000006dce3000
> [ 569.254602] [0000000000000098] pgd=0800000048b63403, p4d=0800000048b63403, pud=0800000048b7f403, pmd=0000000000000000
> [ 569.254709] Internal error: Oops: 0000000096000046 [#2] SMP
> [ 569.257747] Modules linked in:
> [ 569.258124] CPU: 1 UID: 0 PID: 824 Comm: shadow_stage2 Tainted: G D 7.1.0-rc4+ #59 PREEMPT(full)
> [ 569.258642] Tainted: [D]=DIE
> [ 569.258862] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2ubuntu0.7 11/27/2025
> [ 569.259232] pstate: 60402009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 569.259549] pc : down_write+0x50/0xe8
> [ 569.259814] lr : down_write+0x34/0xe8
> [ 569.259960] sp : ffff80008252b310
> [ 569.260175] x29: ffff80008252b310 x28: 0000000000000000 x27: 0000000040000200
> [ 569.260507] x26: 0000000000000200 x25: ffffd1bf542891a0 x24: 0000000000000001
> [ 569.260891] x23: 0000000000000098 x22: ffff000000637480 x21: ffffd1bf57abc518
> [ 569.261278] x20: 0000000000000000 x19: 0000000000000098 x18: ffff80008253d138
> [ 569.261652] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 569.262180] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [ 569.262572] x11: 0000000000000000 x10: 0000000000000000 x9 : ffffd1bf5532388c
> [ 569.263299] x8 : ffff80008252b508 x7 : 0000000000000000 x6 : 0000000000000000
> [ 569.263950] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [ 569.264428] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000000000
> [ 569.264799] Call trace:
> [ 569.265039] down_write+0x50/0xe8 (P)
> [ 569.265441] __simple_recursive_removal+0x68/0x230
> [ 569.265817] simple_recursive_removal+0x20/0x50
> [ 569.266132] debugfs_remove+0x64/0xc0
> [ 569.266411] kvm_nested_s2_ptdump_remove_debugfs+0x20/0x48
> [ 569.266782] kvm_arch_flush_shadow_all+0x4c/0xc0
> [ 569.267059] kvm_mmu_notifier_release+0x3c/0x90
> [ 569.267564] __mmu_notifier_release+0x88/0x2a0
> [ 569.267736] exit_mmap+0x430/0x490
> [ 569.267943] __mmput+0x3c/0x178
> [ 569.268068] mmput+0xa4/0xd8
> [ 569.268221] do_exit+0x274/0xb00
> [ 569.268335] make_task_dead+0x98/0x1f0
> [ 569.268634] die+0x194/0x1a0
> [ 569.268893] die_kernel_fault+0x1d0/0x3c0
> [ 569.269139] __do_kernel_fault+0x280/0x290
> [ 569.269348] do_page_fault+0x128/0x7d8
> [ 569.269550] do_translation_fault+0x74/0xc0
> [ 569.269767] do_mem_abort+0x50/0xd0
> [ 569.269945] el1_abort+0x44/0x80
> [ 569.270122] el1h_64_sync_handler+0x54/0xd0
> [ 569.270306] el1h_64_sync+0x80/0x88
> [ 569.270683] down_write+0x50/0xe8 (P)
> [ 569.270997] __simple_recursive_removal+0x68/0x230
> [ 569.271217] simple_recursive_removal+0x20/0x50
> [ 569.271704] debugfs_remove+0x64/0xc0
> [ 569.271948] kvm_nested_s2_ptdump_remove_debugfs+0x20/0x48
> [ 569.272212] kvm_arch_flush_shadow_all+0x4c/0xc0
> [ 569.272510] kvm_mmu_notifier_release+0x3c/0x90
> [ 569.272731] mmu_notifier_unregister+0x68/0x148
> [ 569.272960] kvm_destroy_vm+0x130/0x2d8
> [ 569.273210] kvm_device_release+0xf8/0x170
> [ 569.273490] __fput+0xf4/0x350
> [ 569.273748] fput_close_sync+0x4c/0x138
> [ 569.274023] __arm64_sys_close+0x44/0xa0
> [ 569.274289] invoke_syscall+0xa8/0x138
> [ 569.274560] el0_svc_common.constprop.0+0x4c/0x140
> [ 569.274838] do_el0_svc+0x28/0x58
> [ 569.275066] el0_svc+0x48/0x218
> [ 569.275321] el0t_64_sync_handler+0xc0/0x108
> [ 569.275556] el0t_64_sync+0x1b4/0x1b8
> [ 569.275844] Code: b9000820 d503201f d2800000 d2800021 (c8e07e61)
> [ 569.276068] ---[ end trace 0000000000000000 ]---
> [ 569.277042] note: shadow_stage2[824] exited with preempt_count 1
> [ 569.277234] Fixing recursive fault but reboot is needed!
>
> the kernel is based off of kvmarm/fixes, applied your series and
> Hyunwoo's patch as well. Could you take a look at this?
Thanks once more!
This is caused by kvm_destroy_vm_debugfs() being called before
mmu_notifier_unregister() in kvm_destroy_vm(). In mmu notifier release I
remove each nested mmu's debugfs file, but all is removed priorly, so of
course UAF and bad dereferences happen.
I didn't catch this because mmu notifier release can also be called
independently before kvm_destroy_vm(). It looks like in my case kvmtool
doesn't close the VM fd on normal exit, so at process exit mm_struct
goes away before kvm, triggering mmu notifier release to free the nested
mmus and the shadow ptdump files before VM destruction. Hence when
kvm_destroy_vm(), the bug is avoided.
I don't see a way out with this per-mmu file scheme. The core issue is
mmus have a different lifetime than the VM's debugfs directory, and
both's removal can happen in parallel, i.e. the VM debugfs directory
can be removed anytime we are in mmu notifier release freeing the mmus
and their shadow ptdump files.
The original idea of just having one "nested_mmus" file could be sound,
we'll just have to take the mmu_lock to check if mmu->pgt is still alive
when getting information.
Thanks,
Wei-Lin Chang
>
> Thanks,
> Itaru.
>
> >
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: usb: Add Rockchip RK3568 compatible for EHCI and OHCI
From: Krzysztof Kozlowski @ 2026-06-25 7:46 UTC (permalink / raw)
To: Jonas Karlman
Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Diederik de Haas, devicetree, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel
In-Reply-To: <20260624192726.781864-2-jonas@kwiboo.se>
On Wed, Jun 24, 2026 at 07:27:24PM +0000, Jonas Karlman wrote:
> The Rockchip RK3568 EHCI/OHCI controller depends on clk_usbphy1_480m
> being enabled, or the system may freeze when registers are accessed.
>
> Add Rockchip RK3568 EHCI and OHCI compatibles with a similar four-clock
> constraint as RK3588, also extend the EHCI constraint to include RK3588
> to match similar requirements of RK3588.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> Existing DTs for RK3568 use the plain generic-ehci/ohci compatible,
> next patch make use of these new compatibles and adds the missing
> clk_usbphy1_480m clock references.
>
> Existing DTs for RK3588 have contained the required four clocks since
> the initial addition of the EHCI/OHCI nodes.
>
> Changes in v2:
> - Include rockchip,rk3588-ehci in the EHCI constraint
> - Make clocks prop required for EHCI and OHCI
> ---
> .../devicetree/bindings/usb/generic-ehci.yaml | 14 ++++++++++++++
> .../devicetree/bindings/usb/generic-ohci.yaml | 7 ++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> index 55a5aa7d7a54..a39f01e740b1 100644
> --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> @@ -52,6 +52,7 @@ properties:
> - ibm,476gtr-ehci
> - nxp,lpc1850-ehci
> - qca,ar7100-ehci
> + - rockchip,rk3568-ehci
> - rockchip,rk3588-ehci
> - snps,hsdk-v1.0-ehci
> - socionext,uniphier-ehci
> @@ -186,6 +187,19 @@ allOf:
> required:
> - clocks
> - clock-names
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3568-ehci
> + - rockchip,rk3588-ehci
> + then:
> + properties:
> + clocks:
> + minItems: 4
> + required:
> + - clocks
This is ABI break for RK3588, so your commit msg should also mention
that RK3588 is not working for example. Otherwise you provided rationale
only for breaking RK3568 ABI.
Same for OHCI part.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v7 25/30] drm/vc4: hdmi: Convert to common HDMI 2.0 SCDC scrambling helpers
From: Maxime Ripard @ 2026-06-25 7:44 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <8937d785-4daa-4246-9553-24aed7d279b5@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 19934 bytes --]
On Sat, Jun 13, 2026 at 03:41:20AM +0300, Cristian Ciocaltea wrote:
> Hi Maxime,
>
> On 6/12/26 3:04 PM, Maxime Ripard wrote:
> > Hi,
> >
> > On Tue, Jun 02, 2026 at 01:44:25AM +0300, Cristian Ciocaltea wrote:
> >> Replace the vc4-local scrambling implementation with the newly
> >> introduced DRM common SCDC scrambling infrastructure:
> >>
> >> - Advertise source-side scrambling support by setting
> >> connector->hdmi.scrambling_supported based on the variant's
> >> max_pixel_clock before drmm_connector_hdmi_init().
> >>
> >> - Provide minimal .scrambler_{enable|disable} connector callbacks that
> >> only toggle the VC5 HDMI_SCRAMBLER_CTL register. Sink-side SCDC
> >> programming and periodic status monitoring are now delegated to
> >> drm_scdc_{start|stop}_scrambling().
> >>
> >> - Replace vc4_hdmi_enable_scrambling() with a conditional call to
> >> drm_scdc_start_scrambling() in post_crtc_enable, gated on
> >> conn_state->hdmi.scrambler_needed (computed by the HDMI state helper).
> >>
> >> - Replace vc4_hdmi_disable_scrambling() with drm_scdc_stop_scrambling()
> >> in post_crtc_disable.
> >>
> >> - Drop vc4_hdmi_reset_link() and vc4_hdmi_handle_hotplug(), switching
> >> the .detect_ctx() path to
> >> drm_atomic_helper_connector_hdmi_hotplug_ctx() which internally calls
> >> drm_scdc_sync_status() to trigger a CRTC reset on reconnection.
> >>
> >> - Drop the local scrambling_work delayed workqueue and scdc_enabled
> >> flag, now tracked by the common drm_connector_hdmi layer.
> >>
> >> - Drop vc4_hdmi_supports_scrambling() and
> >> vc4_hdmi_mode_needs_scrambling() helpers, inlining the remaining 4KP60
> >> warning with an explicit drm_hdmi_compute_mode_clock() check.
> >>
> >> - Seed connector->hdmi.scrambler_enabled = true in connector_init() to
> >> ensure drm_scdc_stop_scrambling() runs at boot and disables any stale
> >> scrambling state left by the bootloader.
> >>
> >> No functional change is expected for the supported modes.
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >
> > I'd really like it to be broken down into several patches:
> >
> >> ---
> >> drivers/gpu/drm/vc4/vc4_hdmi.c | 265 ++++++-----------------------------------
> >> drivers/gpu/drm/vc4/vc4_hdmi.h | 8 --
> >> 2 files changed, 35 insertions(+), 238 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> >> index 046ac4f43ba8..02f6ca6ab52b 100644
> >> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> >> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> >> @@ -114,31 +114,6 @@
> >> #define HSM_MIN_CLOCK_FREQ 120000000
> >> #define CEC_CLOCK_FREQ 40000
> >>
> >> -static bool vc4_hdmi_supports_scrambling(struct vc4_hdmi *vc4_hdmi)
> >> -{
> >> - struct drm_display_info *display = &vc4_hdmi->connector.display_info;
> >> -
> >> - lockdep_assert_held(&vc4_hdmi->mutex);
> >> -
> >> - if (!display->is_hdmi)
> >> - return false;
> >> -
> >> - if (!display->hdmi.scdc.supported ||
> >> - !display->hdmi.scdc.scrambling.supported)
> >> - return false;
> >> -
> >> - return true;
> >> -}
> >> -
> >> -static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
> >> - unsigned int bpc,
> >> - enum drm_output_color_format fmt)
> >> -{
> >> - unsigned long long clock = drm_hdmi_compute_mode_clock(mode, bpc, fmt);
> >> -
> >> - return clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> >> -}
> >> -
> >> static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
> >> {
> >> struct drm_debugfs_entry *entry = m->private;
> >> @@ -272,124 +247,6 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
> >> static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
> >> #endif
> >>
> >> -static int vc4_hdmi_reset_link(struct drm_connector *connector,
> >> - struct drm_modeset_acquire_ctx *ctx)
> >> -{
> >> - struct drm_device *drm;
> >> - struct vc4_hdmi *vc4_hdmi;
> >> - struct drm_connector_state *conn_state;
> >> - struct drm_crtc_state *crtc_state;
> >> - struct drm_crtc *crtc;
> >> - bool scrambling_needed;
> >> - u8 config;
> >> - int ret;
> >> -
> >> - if (!connector)
> >> - return 0;
> >> -
> >> - drm = connector->dev;
> >> - ret = drm_modeset_lock(&drm->mode_config.connection_mutex, ctx);
> >> - if (ret)
> >> - return ret;
> >> -
> >> - conn_state = connector->state;
> >> - crtc = conn_state->crtc;
> >> - if (!crtc)
> >> - return 0;
> >> -
> >> - ret = drm_modeset_lock(&crtc->mutex, ctx);
> >> - if (ret)
> >> - return ret;
> >> -
> >> - crtc_state = crtc->state;
> >> - if (!crtc_state->active)
> >> - return 0;
> >> -
> >> - vc4_hdmi = connector_to_vc4_hdmi(connector);
> >> - mutex_lock(&vc4_hdmi->mutex);
> >> -
> >> - if (!vc4_hdmi_supports_scrambling(vc4_hdmi)) {
> >> - mutex_unlock(&vc4_hdmi->mutex);
> >> - return 0;
> >> - }
> >> -
> >> - scrambling_needed = vc4_hdmi_mode_needs_scrambling(&vc4_hdmi->saved_adjusted_mode,
> >> - vc4_hdmi->output_bpc,
> >> - vc4_hdmi->output_format);
> >> - if (!scrambling_needed) {
> >> - mutex_unlock(&vc4_hdmi->mutex);
> >> - return 0;
> >> - }
> >> -
> >> - if (conn_state->commit &&
> >> - !try_wait_for_completion(&conn_state->commit->hw_done)) {
> >> - mutex_unlock(&vc4_hdmi->mutex);
> >> - return 0;
> >> - }
> >> -
> >> - ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> >> - if (ret < 0) {
> >> - drm_err(drm, "Failed to read TMDS config: %d\n", ret);
> >> - mutex_unlock(&vc4_hdmi->mutex);
> >> - return 0;
> >> - }
> >> -
> >> - if (!!(config & SCDC_SCRAMBLING_ENABLE) == scrambling_needed) {
> >> - mutex_unlock(&vc4_hdmi->mutex);
> >> - return 0;
> >> - }
> >> -
> >> - mutex_unlock(&vc4_hdmi->mutex);
> >> -
> >> - /*
> >> - * HDMI 2.0 says that one should not send scrambled data
> >> - * prior to configuring the sink scrambling, and that
> >> - * TMDS clock/data transmission should be suspended when
> >> - * changing the TMDS clock rate in the sink. So let's
> >> - * just do a full modeset here, even though some sinks
> >> - * would be perfectly happy if were to just reconfigure
> >> - * the SCDC settings on the fly.
> >> - */
> >> - return drm_atomic_helper_reset_crtc(crtc, ctx);
> >> -}
> >
> > This one doesn't look functionally equivalent to me to
> > drm_scdc_reset_crtc: this part was, in part, making sure we would only
> > reset the scrambler if it was enabled in the first place.
> > drm_scdc_reset_crtc() doesn't and will always trigger a modeset on
> > hotplug. That's unnecessary and a significant functional different.
>
> drm_scdc_reset_crtc() alone was not meant to be an equivalent of
> vc4_hdmi_reset_link(), as it only checks the sink side and it serves as an
> internal helper used exclusively by drm_scdc_sync_status().
>
> As a matter of fact, the latter is the one responsible for verifying if the
> scrambler was enabled on the controller side before attempting to invoke the
> reset logic, hence we should get the same behavior. But we don't invoke it
> directly either, it's part of the drm_atomic_helper_connector_hdmi_hotplug_ctx()
> call path.
Oh, right, sorry.
> > I'd argue that it's drm_scdc_reset_crtc() that needs to align to what
> > vc4 was doing, not the opposite.
>
> The only difference consists in dropping the crtc state check:
>
> ret = drm_modeset_lock(&crtc->mutex, ctx);
> if (ret)
> return ret;
>
> crtc_state = crtc->state;
> if (!crtc_state->active)
> return 0;
>
> The rationale was that when CRTC is inactive, drm_atomic_helper_reset_crtc()
> should result in a no-op commit anyway.
A commit is expensive, so I'd skip it if we can.
> And the one for the in-flight commit:
>
> if (conn_state->commit &&
> !try_wait_for_completion(&conn_state->commit->hw_done)) {
> mutex_unlock(&vc4_hdmi->mutex);
> return 0;
> }
And yeah, we'll need this one too.
> Both checks are also missing in drm_bridge_helper_reset_crtc(), taken as an
> initial reference. Should we still keep any/both and sync the bridge helper
> accordingly?
Yes, but I'd expect the bridge helpers to converge / reuse your helpers
eventually anyway?
> >> -static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> >> - struct drm_modeset_acquire_ctx *ctx,
> >> - enum drm_connector_status status)
> >> -{
> >> - struct drm_connector *connector = &vc4_hdmi->connector;
> >> - int ret;
> >> -
> >> - /*
> >> - * NOTE: This function should really be called with vc4_hdmi->mutex
> >> - * held, but doing so results in reentrancy issues since
> >> - * cec_s_phys_addr() might call .adap_enable, which leads to that
> >> - * funtion being called with our mutex held.
> >> - *
> >> - * A similar situation occurs with vc4_hdmi_reset_link() that
> >> - * will call into our KMS hooks if the scrambling was enabled.
> >> - *
> >> - * Concurrency isn't an issue at the moment since we don't share
> >> - * any state with any of the other frameworks so we can ignore
> >> - * the lock for now.
> >> - */
> >> -
> >> - drm_atomic_helper_connector_hdmi_hotplug(connector, status);
> >> -
> >> - if (status != connector_status_connected)
> >> - return;
> >> -
> >> - for (;;) {
> >> - ret = vc4_hdmi_reset_link(connector, ctx);
> >> - if (ret == -EDEADLK) {
> >> - drm_modeset_backoff(ctx);
> >> - continue;
> >> - }
> >> -
> >> - break;
> >> - }
> >> -}
> >> -
> >> static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
> >> struct drm_modeset_acquire_ctx *ctx,
> >> bool force)
> >> @@ -401,8 +258,8 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
> >> /*
> >> * NOTE: This function should really take vc4_hdmi->mutex, but
> >> * doing so results in reentrancy issues since
> >> - * vc4_hdmi_handle_hotplug() can call into other functions that
> >> - * would take the mutex while it's held here.
> >> + * drm_atomic_helper_connector_hdmi_hotplug_ctx() can call into other
> >> + * functions that would take the mutex while it's held here.
> >> *
> >> * Concurrency isn't an issue at the moment since we don't share
> >> * any state with any of the other frameworks so we can ignore
> >> @@ -425,10 +282,11 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
> >> status = connector_status_connected;
> >> }
> >>
> >> - vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
> >> + ret = drm_atomic_helper_connector_hdmi_hotplug_ctx(connector, status, ctx);
> >> +
> >> pm_runtime_put(&vc4_hdmi->pdev->dev);
> >>
> >> - return status;
> >> + return ret == -EDEADLK ? ret : status;
> >> }
> >>
> >> static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
> >> @@ -441,9 +299,12 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
> >> if (!vc4->hvs->vc5_hdmi_enable_hdmi_20) {
> >> struct drm_device *drm = connector->dev;
> >> const struct drm_display_mode *mode;
> >> + unsigned long long clock;
> >>
> >> list_for_each_entry(mode, &connector->probed_modes, head) {
> >> - if (vc4_hdmi_mode_needs_scrambling(mode, 8, DRM_OUTPUT_COLOR_FORMAT_RGB444)) {
> >> + clock = drm_hdmi_compute_mode_clock(mode, 8,
> >> + DRM_OUTPUT_COLOR_FORMAT_RGB444);
> >> + if (clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ) {
> >
> > This should be a patch of its own, but I think we should turn
> > vc4_hdmi_mode_needs_scrambling() into a helper, instead of checking the
> > clock rate in every driver that might need it. From a logical standpoint
> > it's equivalent, but not from a semantic one.
>
> Ack.
>
> >
> >> drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz.");
> >> drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60.");
> >> }
> >> @@ -540,6 +401,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
> >> if (vc4_hdmi->variant->supports_hdr)
> >> max_bpc = 12;
> >>
> >> + connector->hdmi.scrambler_supported =
> >> + vc4_hdmi->variant->max_pixel_clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> >> +
> >> ret = drmm_connector_hdmi_init(dev, connector,
> >> "Broadcom", "Videocore",
> >> &vc4_hdmi_connector_funcs,
> >> @@ -561,6 +425,14 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
> >>
> >> drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
> >>
> >> + /*
> >> + * Since we don't know the state of the controller and its
> >> + * display (if any), let's assume it's always enabled.
> >> + * drm_scdc_stop_scrambling() will thus run at boot, make
> >> + * sure it's disabled, and avoid any inconsistency.
> >> + */
> >> + connector->hdmi.scrambler_enabled = connector->hdmi.scrambler_supported;
> >> +
> >> /*
> >> * Some of the properties below require access to state, like bpc.
> >> * Allocate some default initial connector state with our reset helper.
> >> @@ -786,93 +658,30 @@ static int vc4_hdmi_write_spd_infoframe(struct drm_connector *connector,
> >> buffer, len);
> >> }
> >>
> >> -#define SCRAMBLING_POLLING_DELAY_MS 1000
> >> -
> >> -static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
> >> +static int vc4_hdmi_scrambler_enable(struct drm_connector *connector)
> >> {
> >> - struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> >> - struct drm_connector *connector = &vc4_hdmi->connector;
> >> - struct drm_device *drm = connector->dev;
> >> - const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
> >> + struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> >> unsigned long flags;
> >> - int idx;
> >> -
> >> - lockdep_assert_held(&vc4_hdmi->mutex);
> >> -
> >> - if (!vc4_hdmi_supports_scrambling(vc4_hdmi))
> >> - return;
> >> -
> >> - if (!vc4_hdmi_mode_needs_scrambling(mode,
> >> - vc4_hdmi->output_bpc,
> >> - vc4_hdmi->output_format))
> >> - return;
> >> -
> >> - if (!drm_dev_enter(drm, &idx))
> >> - return;
> >
> > drm_dev_enter should be kept here
>
> Sorry, somehow I missed to realize when I prepared the patches that I
> accidentally dropped these during my initial driver rework.
>
> >
> >> - drm_scdc_set_high_tmds_clock_ratio(connector, true);
> >> - drm_scdc_set_scrambling(connector, true);
> >>
> >> spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
> >> HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |
> >> VC5_HDMI_SCRAMBLER_CTL_ENABLE);
> >> spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
> >>
> >> - drm_dev_exit(idx);
> >
> > And exit here.
> >
> >> -static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
> >> +static int vc4_hdmi_scrambler_disable(struct drm_connector *connector)
> >> {
> >> - struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> >> - struct drm_connector *connector = &vc4_hdmi->connector;
> >> - struct drm_device *drm = connector->dev;
> >> + struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> >> unsigned long flags;
> >> - int idx;
> >> -
> >> - lockdep_assert_held(&vc4_hdmi->mutex);
> >> -
> >> - if (!vc4_hdmi->scdc_enabled)
> >> - return;
> >> -
> >> - vc4_hdmi->scdc_enabled = false;
> >> -
> >> - if (delayed_work_pending(&vc4_hdmi->scrambling_work))
> >> - cancel_delayed_work_sync(&vc4_hdmi->scrambling_work);
> >> -
> >> - if (!drm_dev_enter(drm, &idx))
> >> - return;
> >
> > Ditto
> >
> >> spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
> >> HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) &
> >> ~VC5_HDMI_SCRAMBLER_CTL_ENABLE);
> >> spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
> >>
> >> - drm_scdc_set_scrambling(connector, false);
> >> - drm_scdc_set_high_tmds_clock_ratio(connector, false);
> >> -
> >> - drm_dev_exit(idx);
> >> -}
> >> -
> >> -static void vc4_hdmi_scrambling_wq(struct work_struct *work)
> >> -{
> >> - struct vc4_hdmi *vc4_hdmi = container_of(to_delayed_work(work),
> >> - struct vc4_hdmi,
> >> - scrambling_work);
> >> - struct drm_connector *connector = &vc4_hdmi->connector;
> >> -
> >> - if (drm_scdc_get_scrambling_status(connector))
> >> - return;
> >> -
> >> - drm_scdc_set_high_tmds_clock_ratio(connector, true);
> >> - drm_scdc_set_scrambling(connector, true);
> >> -
> >> - queue_delayed_work(system_percpu_wq, &vc4_hdmi->scrambling_work,
> >> - msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
> >> + return 0;
> >> }
> >>
> >> static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
> >> @@ -917,7 +726,7 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
> >> spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
> >> }
> >>
> >> - vc4_hdmi_disable_scrambling(encoder);
> >> + drm_scdc_stop_scrambling(&vc4_hdmi->connector);
> >
> > I don't think the names here are right. It's not *only* related to scdc
> > but also to the HDMI controller. I'm fine with using a scdc prefix but
> > only for the things related to scdc. This is related (in part) to the
> > HDMI controller, so it should use a drm_connector_hdmi prefix.
>
> Ack. I guess we should also move these helpers out of drm_scdc_helper.c, but
> unsure where. FWIW I'm currently working on adding HDMI 2.1 FRL support, and
> implemented the link training in a dedicated drm_hdmi_frl_helper.c.
>
> Should we create drm_hdmi_scrambler_helper.c? Or maybe have a common one to
> hold both - any suggestion for the naming?
display/drm_hdmi_helper.c sounds like a great place for both?
> >
> >> drm_dev_exit(idx);
> >>
> >> @@ -1625,6 +1434,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
> >> struct drm_display_info *display = &vc4_hdmi->connector.display_info;
> >> bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
> >> bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
> >> + struct drm_connector_state *conn_state;
> >> unsigned long flags;
> >> int ret;
> >> int idx;
> >> @@ -1693,7 +1503,10 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
> >> }
> >>
> >> vc4_hdmi_recenter_fifo(vc4_hdmi);
> >> - vc4_hdmi_enable_scrambling(encoder);
> >> +
> >> + conn_state = drm_atomic_get_new_connector_state(state, connector);
> >> + if (conn_state && conn_state->hdmi.scrambler_needed)
> >> + drm_scdc_start_scrambling(connector);
> >
> > And the nice thing with a drm_connector_hdmi_* prefix is that you don't
> > have to shoehorn it into SCDC support anymore, so you can take a state
> > as an argument, and do the check in the helper instead of doing it in
> > every driver and hoping they get it right.
>
> I was about to consider a similar approach before deciding to let drivers manage
> the logic, i.e. to prevent loosing flexibility later when dealing with HDMI 2.1.
> That was mostly in the context of supporting drivers to define if/when a display
> mode that fits in TMDS should be sent over FRL.
>
> Thinking again, that's not really a valid concern right now, e.g. will use TMDS
> by default for all supported modes, and switch to FRL only when absolutely
> required. Later we might consider extending the infrastructure to support
> dynamic control if required.
Thanks for working on FRL as well :)
I agree, let's focus on getting HDMI 2.0 right, and then we'll try to
make 2.1 the easiest to work with for drivers.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH 1/8] clk: qcom: dispcc-sm8450: Fix mdss clocks
From: Konrad Dybcio @ 2026-06-25 7:38 UTC (permalink / raw)
To: Esteban Urrutia, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Rob Clark, Will Deacon, Robin Murphy,
Joerg Roedel (AMD), Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy
In-Reply-To: <9bc524b9-c6da-47a1-a7cf-abeb131416a7@proton.me>
On 6/25/26 4:22 AM, Esteban Urrutia wrote:
> On 6/23/26 11:50 AM, Konrad Dybcio wrote:
>> This can also be fixed by migrating to use qcom_cc_driver_data,
>> which takes a list of alpha PLLs to be configured, and thenthere's
>> a switch-statement in clk-alpha-pll.c that always assigns the
>> correct function
>
> If this is done, should a patch that migrates to qcom_cc_driver_data and a
> patch that fixes the issue be sent, or should only a single patch be sent?
It's fine to just have one patch, but please mention that this
actually happens to fix the issue in the commit message
Konrad
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: arm: qcom,coresight-tnoc: allow arm,primecell-periphid
From: Jie Gan @ 2026-06-25 7:36 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Tingwei Zhang, Jingyi Wang, Abel Vesa,
Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
Yuanfang Zhang, Konrad Dybcio, linux-arm-msm, devicetree,
linux-kernel, coresight, linux-arm-kernel
In-Reply-To: <20260625-strong-daft-pudu-21471f@quoll>
On 6/25/2026 3:24 PM, Krzysztof Kozlowski wrote:
> On Wed, Jun 24, 2026 at 05:49:25PM +0800, Jie Gan wrote:
>> The TNOC device is an AMBA primecell and may carry the standard
>> arm,primecell-periphid property, which is used to supply the
>> peripheral ID when it cannot be read from the device registers.
>>
>> Reference primecell.yaml and set additionalProperties to true so the
>> binding accepts arm,primecell-periphid along with the other common
>> primecell properties.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>> Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
>> index ef648a15b806..9624fc0adfdc 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
>> @@ -32,6 +32,9 @@ select:
>> required:
>> - compatible
>>
>> +allOf:
>> + - $ref: /schemas/arm/primecell.yaml#
>> +
>> properties:
>> $nodename:
>> pattern: "^tn(@[0-9a-f]+)$"
>> @@ -78,7 +81,7 @@ required:
>> - in-ports
>> - out-ports
>>
>> -additionalProperties: false
>> +additionalProperties: true
>
> Nope, it is not allowed. Explicitly mentioned in writing bindings and
> all DT introductory talks by me.
Yes, I am totally wrong with this and I should add:
unevaluatedProperties: false
and remove
additionalProperties: false
Thanks,
Jie
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* [PATCH] ARM: enable interrupts when arm_notify_die() is handling user mode errors
From: Xie Yuanbin @ 2026-06-25 7:35 UTC (permalink / raw)
To: linux, bigeasy, clrkwllms, rostedt, rmk+kernel, linusw, arnd
Cc: linux-arm-kernel, linux-kernel, linux-rt-devel, liaohua4,
lilinjie8, Xie Yuanbin
For lastest linux-next kernel, with default multi_v7_defconfig, and
setting CONFIG_PREEMPT_RT=y, CONFIG_DEBUG_ATOMIC_SLEEP=y, and
CONFIG_PERF_EVENTS=n. When the user program executes bkpt
instruction, the following WARN will be triggered:
```log
[ 3.677825] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
[ 3.678002] in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 84, name: test
[ 3.678036] preempt_count: 0, expected: 0
[ 3.678078] RCU nest depth: 0, expected: 0
[ 3.678864] CPU: 0 UID: 0 PID: 84 Comm: test Tainted: G W 7.1.0-next-20260623 #45 PREEMPT_RT
[ 3.679067] Tainted: [W]=WARN
[ 3.679088] Hardware name: Generic DT based system
[ 3.679198] Call trace:
[ 3.679695] unwind_backtrace from show_stack+0x10/0x14
[ 3.680363] show_stack from dump_stack_lvl+0x50/0x5c
[ 3.680377] dump_stack_lvl from __might_resched+0x160/0x174
[ 3.680393] __might_resched from rt_spin_lock+0x38/0x138
[ 3.680425] rt_spin_lock from force_sig_info_to_task+0x1c/0x11c
[ 3.680438] force_sig_info_to_task from force_sig_fault+0x44/0x64
[ 3.680450] force_sig_fault from do_PrefetchAbort+0x94/0x9c
[ 3.680461] do_PrefetchAbort from ret_from_exception+0x0/0x20
[ 3.680513] Exception stack(0xf0ab5fb0 to 0xf0ab5ff8)
[ 3.680653] 5fa0: 00000000 bed32e94 bed32e9c 00037954
[ 3.680672] 5fc0: 00000002 00000001 bed32e94 0009d590 00000000 bed32e9c 00000002 00000000
[ 3.680682] 5fe0: bed32d48 bed32d38 00037a00 00037958 60000010 ffffffff
```
When PREEMPT_RT is enabled, force_sig_info() requires interrupts to be
enabled. Enable interrupts when arm_notify_die() is handling user mode
errors to fix the issue.
Fixes: c6e61c06d606 ("ARM: 9463/1: Allow to enable RT")
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>
---
arch/arm/kernel/traps.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index afbd2ebe5c39..6aa205a92920 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -375,6 +375,7 @@ void arm_notify_die(const char *str, struct pt_regs *regs,
unsigned long err, unsigned long trap)
{
if (user_mode(regs)) {
+ local_irq_enable();
current->thread.error_code = err;
current->thread.trap_no = trap;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] dt-bindings: mediatek: cec: Correct the compatibles for mt7623-mt8167
From: Krzysztof Kozlowski @ 2026-06-25 7:32 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Chun-Kuang Hu, Philipp Zabel, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, CK Hu, Jitao shi,
dri-devel, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260624173627.19785-1-l.scorcia@gmail.com>
On Wed, Jun 24, 2026 at 07:36:15PM +0200, Luca Leonardo Scorcia wrote:
> The HDMI CEC driver for both mt7623 and mt8167 is actually the same as
> mt8173-cec and the mt7623n.dtsi board include file already uses mt8173-cec
> compatible as a fallback, but the documentation lists them as separate
> entries. Correct the binding by adding the correct fallback.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Sakari Ailus @ 2026-06-25 7:24 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Frank Li, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
driver-core, linux-acpi, linux-kernel, linux-media,
linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
Frank Li, Guoniu Zhou
In-Reply-To: <20260624222042.GN851255@killaraus.ideasonboard.com>
Hi Laurent,
On Thu, Jun 25, 2026 at 01:20:42AM +0300, Laurent Pinchart wrote:
> On Wed, Jun 24, 2026 at 03:46:48PM -0500, Frank Li wrote:
> > On Wed, Jun 24, 2026 at 11:02:37PM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> > > > On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > > > > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > > > > simplify media code.
> > > > > >
> > > > > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > > > > rkisp1-dev.c only silience improvement.
> > > > > >
> > > > > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > > > > >
> > > > > > Build test only.
> > > > > >
> > > > > > Sakari Ailus:
> > > > > > when I try to improve the patch
> > > > > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > > > > camss.c pattern, so I create this small improvement firstly.
> > > > >
> > > > > Those are nice cleanups, thank you.
> > > > >
> > > > > After applying this series, the only left users of the
> > > > > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> > > >
> > > > I already checked previously, two place use it.
> > > >
> > > > fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> > > > ep is NULL, which totally equial to scoped() version.
> > > >
> > > > another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> > > > caller to call put().
> > > >
> > > > if use scoped() version, need use no_free_ptr() at return, which make think
> > > > a little bit complex.
> > >
> > > It would introduce a tiny bit of extra complexity there, but the
> > > advantage (in my opinion) is that we'll be able to remove the less safe
> > > fwnode_graph_for_each_endpoint() macro.
> > >
> > > Now one may argue that the risk of
> > > fwnode_graph_for_each_endpoint_scoped() is returning the iterator
> > > without using no_free_ptr(). I wonder if that would be easier to catch
> > > in static analysis tools than the current pattern that leaks a reference
> > > when exiting the loop early.
> >
> > It's not big deal, if everyone prefer drop fwnode_graph_for_each_endpoint(),
> > I can do it.
>
> Let's see what others think. If people prefer keeping both versions,
> I'll be OK with that.
I'd prefer to keep both: it depends on the use case which one is better.
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* RE: [PATCH V2 1/8] PCI: imx6: Add skip_pwrctrl_off flag support
From: Sherry Sun @ 2026-06-25 7:25 UTC (permalink / raw)
To: Frank Li (OSS)
Cc: Sherry Sun (OSS), robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, Frank Li, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, Amitkumar Karwar,
Neeraj Sanjay Kale, marcel@holtmann.org, luiz.dentz@gmail.com,
Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com,
brgl@kernel.org, imx@lists.linux.dev, linux-pci@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <ajwOvZUlOEQzmjsu@SMW015318>
> Subject: Re: [PATCH V2 1/8] PCI: imx6: Add skip_pwrctrl_off flag support
>
> On Wed, Jun 24, 2026 at 07:09:26AM +0000, Sherry Sun wrote:
> > > Subject: Re: [PATCH V2 1/8] PCI: imx6: Add skip_pwrctrl_off flag
> > > support
> > >
> > > On Tue, Jun 23, 2026 at 11:07:28AM +0800, Sherry Sun (OSS) wrote:
> > > > From: Sherry Sun <sherry.sun@nxp.com>
> > > >
> > > > Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices
> > > > during suspend to preserve wakeup capability of the devices and
> > > > also not to power on the devices in the init path.
> > > > This allows controller power-off to be skipped when some devices(e.g.
> > > > M.2 cards key E without auxiliary power) required to support PCIe
> > > > L2 link state and wake-up mechanisms.
> > > >
> > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > ---
> > > > drivers/pci/controller/dwc/pci-imx6.c | 36
> > > > +++++++++++++++++----------
> > > > 1 file changed, 23 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 0fa716d1ed75..ff5a9565dbbf 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -1382,16 +1382,20 @@ static int imx_pcie_host_init(struct
> > > > dw_pcie_rp
> > > *pp)
> > > > }
> > > > }
> > > >
> > > > - ret = pci_pwrctrl_create_devices(dev);
> > > > - if (ret) {
> > > > - dev_err(dev, "failed to create pwrctrl devices\n");
> > > > - goto err_reg_disable;
> > > > + if (!pci->suspended) {
> > > > + ret = pci_pwrctrl_create_devices(dev);
> > >
> > > Is possible move pci_pwrctrl_create_devices() of
> > > pci_pwrctrl_create_devices
> > >
> > > and call it direct at probe() function, like other regulator_get function.
> > >
> >
> > Hi Frank,
> > That makes sense. However, if we move pci_pwrctrl_create_devices () to
> > probe(), we may need to add the following goto err_pwrctrl_destroy
> > path in imx_pcie_probe() to properly handle errors from
> > pci_pwrctrl_power_on_devices(), is that acceptable?
>
> Can you add a API devm_pci_pwrctrl_create_devices() ?
>
Hi Frank, we cannot unconditionally destroy the pwrctrl devices
when probing fails by using devm API.
Since we need to check the return value of
pci_pwrctrl_power_on_devices() for example EPROBE_DEFER to decide
whether to destroy the pwrctrl devices to avoid the deferred probe loop.
You can find more related discussion here.
https://lore.kernel.org/all/tutxwjciedqoje5wxvtin4h637auni5zzpvb7rtfg4uticxoux@yfl6xg7oht7t/
Best Regards
Sherry
>
> >
> > @@ -1960,11 +1949,15 @@ static int imx_pcie_probe(struct
> platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > + ret = pci_pwrctrl_create_devices(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to create
> > + pwrctrl devices\n");
> > +
> > pci->use_parent_dt_ranges = true;
> > if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
> > ret = imx_add_pcie_ep(imx_pcie, pdev);
> > if (ret < 0)
> > - return ret;
> > + goto err_pwrctrl_destroy;
> >
> > /*
> > * FIXME: Only single Device (EPF) is supported due to
> > the @@ -1979,7 +1972,7 @@ static int imx_pcie_probe(struct
> platform_device *pdev)
> > pci->pp.use_atu_msg = true;
> > ret = dw_pcie_host_init(&pci->pp);
> > if (ret < 0)
> > - return ret;
> > + goto err_pwrctrl_destroy;
> >
> > if (pci_msi_enabled()) {
> > u8 offset = dw_pcie_find_capability(pci,
> > PCI_CAP_ID_MSI); @@ -1991,6 +1984,11 @@ static int
> imx_pcie_probe(struct platform_device *pdev)
> > }
> >
> > return 0;
> > +
> > +err_pwrctrl_destroy:
> > + if (ret != -EPROBE_DEFER)
> > + pci_pwrctrl_destroy_devices(dev);
> > + return ret;
> > }
> >
> > Best Regards
> > Sherry
> >
> > >
> > > > + if (ret) {
> > > > + dev_err(dev, "failed to create pwrctrl devices\n");
> > > > + goto err_reg_disable;
> > > > + }
> > > > }
> > > >
> > > > - ret = pci_pwrctrl_power_on_devices(dev);
> > > > - if (ret) {
> > > > - dev_err(dev, "failed to power on pwrctrl devices\n");
> > > > - goto err_pwrctrl_destroy;
> > > > + if (!pp->skip_pwrctrl_off) {
> > > > + ret = pci_pwrctrl_power_on_devices(dev);
> > > > + if (ret) {
> > > > + dev_err(dev, "failed to power on pwrctrl devices\n");
> > > > + goto err_pwrctrl_destroy;
> > > > + }
> > > > }
> > > >
> > > > ret = imx_pcie_clk_enable(imx_pcie); @@ -1460,9 +1464,10 @@
> > > static
> > > > int imx_pcie_host_init(struct dw_pcie_rp *pp)
> > > > err_clk_disable:
> > > > imx_pcie_clk_disable(imx_pcie);
> > > > err_pwrctrl_power_off:
> > > > - pci_pwrctrl_power_off_devices(dev);
> > > > + if (!pp->skip_pwrctrl_off)
> > > > + pci_pwrctrl_power_off_devices(dev);
> > > > err_pwrctrl_destroy:
> > > > - if (ret != -EPROBE_DEFER)
> > > > + if (ret != -EPROBE_DEFER && !pci->suspended)
> > > > pci_pwrctrl_destroy_devices(dev);
> > > > err_reg_disable:
> > > > if (imx_pcie->vpcie)
> > > > @@ -1482,7 +1487,8 @@ static void imx_pcie_host_exit(struct
> > > > dw_pcie_rp
> > > *pp)
> > > > }
> > > > imx_pcie_clk_disable(imx_pcie);
> > > >
> > > > - pci_pwrctrl_power_off_devices(pci->dev);
> > > > + if (!pci->pp.skip_pwrctrl_off)
> > > > + pci_pwrctrl_power_off_devices(pci->dev);
> > > > if (imx_pcie->vpcie)
> > > > regulator_disable(imx_pcie->vpcie);
> > > > }
> > > > @@ -1990,12 +1996,16 @@ static int imx_pcie_probe(struct
> > > > platform_device *pdev) static void imx_pcie_shutdown(struct
> > > > platform_device *pdev) {
> > > > struct imx_pcie *imx_pcie = platform_get_drvdata(pdev);
> > > > + struct dw_pcie *pci = imx_pcie->pci;
> > > > + struct dw_pcie_rp *pp = &pci->pp;
> > > >
> > > > /* bring down link, so bootloader gets clean state in case of reboot */
> > > > imx_pcie_assert_core_reset(imx_pcie);
> > > > imx_pcie_assert_perst(imx_pcie, true);
> > > > - pci_pwrctrl_power_off_devices(&pdev->dev);
> > > > - pci_pwrctrl_destroy_devices(&pdev->dev);
> > > > + if (!pp->skip_pwrctrl_off)
> > > > + pci_pwrctrl_power_off_devices(&pdev->dev);
> > > > + if (!pci->suspended)
> > > > + pci_pwrctrl_destroy_devices(&pdev->dev);
> > > > }
> > > >
> > > > static const struct imx_pcie_drvdata drvdata[] = {
> > > > --
> > > > 2.50.1
> > > >
> > > >
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: arm: qcom,coresight-tnoc: allow arm,primecell-periphid
From: Krzysztof Kozlowski @ 2026-06-25 7:24 UTC (permalink / raw)
To: Jie Gan
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Tingwei Zhang, Jingyi Wang, Abel Vesa,
Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
Yuanfang Zhang, Konrad Dybcio, linux-arm-msm, devicetree,
linux-kernel, coresight, linux-arm-kernel
In-Reply-To: <20260624-fix-tracenoc-probe-issue-v2-1-786520f62f21@oss.qualcomm.com>
On Wed, Jun 24, 2026 at 05:49:25PM +0800, Jie Gan wrote:
> The TNOC device is an AMBA primecell and may carry the standard
> arm,primecell-periphid property, which is used to supply the
> peripheral ID when it cannot be read from the device registers.
>
> Reference primecell.yaml and set additionalProperties to true so the
> binding accepts arm,primecell-periphid along with the other common
> primecell properties.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> index ef648a15b806..9624fc0adfdc 100644
> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> @@ -32,6 +32,9 @@ select:
> required:
> - compatible
>
> +allOf:
> + - $ref: /schemas/arm/primecell.yaml#
> +
> properties:
> $nodename:
> pattern: "^tn(@[0-9a-f]+)$"
> @@ -78,7 +81,7 @@ required:
> - in-ports
> - out-ports
>
> -additionalProperties: false
> +additionalProperties: true
Nope, it is not allowed. Explicitly mentioned in writing bindings and
all DT introductory talks by me.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
From: Oleksij Rempel @ 2026-06-25 7:18 UTC (permalink / raw)
To: Vincent Jardin
Cc: Pengutronix Kernel Team, Andi Shyti, Frank Li, Sascha Hauer,
Fabio Estevam, Wolfram Sang, Kaushal Butala, Shawn Guo,
Stefan Eichenberger, linux-i2c, imx, linux-arm-kernel,
linux-kernel, stable
In-Reply-To: <20260525-for-upstream-i2c-lx2160-fix-v1-v2-0-26a3cc8cd055@free.fr>
On Mon, May 25, 2026 at 06:43:14PM +0200, Vincent Jardin wrote:
> i2c-imx rejects a SMBus Block Read byte count of 0 (valid per SMBus 3.1
> 6.5.7) and it returns without a NACK+STOP, leaving the target
> holding SDA so the bus is stuck until a power cycle occur.
>
> The same bug is occuring with two independently introduced spots, so the
> fix is two patches with their respective Fixes: tags and backport ranges:
>
> 1/2 atomic/polling path Fixes: 8e8782c71595 v3.16+
> 2/2 IRQ-driven state machine Fixes: 5f5c2d4579ca v6.13+
>
> Signed-off-by: Vincent Jardin <vjardin@free.fr>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Thank you!
Best Regards,
Oleksij
-
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: mark I2C adapter when hardware is powered down
From: Oleksij Rempel @ 2026-06-25 7:15 UTC (permalink / raw)
To: Carlos Song (OSS)
Cc: kernel, andi.shyti, s.hauer, festevam, carlos.song, haibo.chen,
linux-i2c, imx, linux-arm-kernel, linux-kernel, stable
In-Reply-To: <20260525030400.3182911-1-carlos.song@oss.nxp.com>
On Mon, May 25, 2026 at 11:04:00AM +0800, Carlos Song (OSS) wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> On some i.MX platforms, certain I2C client drivers keep a periodic
> workqueue which continues to trigger I2C transfers.
>
> During system suspend/resume, there exists a time window between:
> - suspend_noirq and the system entering suspend
> - the system starting to resume and resume_noirq
>
> In this window, the I2C controller resources such as clock and pinctrl
> may already be disabled or not yet restored.
>
> If a workqueue triggers an I2C transfer in this period, the driver
> attempts to access I2C registers while the hardware resources are
> unavailable, which may lead to system hang.
>
> Mark the I2C adapter as suspended during noirq suspend and block new
> transfers until resume, ensuring that I2C transfers are only issued
> when hardware resources are available.
>
> Fixes: 358025ac091e ("i2c: imx: make controller available until system suspend_noirq() and from resume_noirq()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Thank you!
Best Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox