Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* 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

* [PATCH] i2c: imx: Fix slave registration error path and missing NULL check
From: Liem @ 2026-06-25  7:11 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Andi Shyti, Pengutronix Kernel Team, Frank Li, Sascha Hauer,
	Fabio Estevam, Biwen Li, Wolfram Sang, linux-i2c, imx,
	linux-arm-kernel, linux-kernel, stable, Liem

There are two issues that affect the i2c-imx slave handling:

1. In i2c_imx_reg_slave(), i2c_imx->slave is checked at the beginning
   and the function returns -EBUSY if it is non-NULL.  If
   pm_runtime_resume_and_get() fails later, the error path returns
   without clearing i2c_imx->slave, leaving it non-NULL.  Subsequent
   attempts to register a slave will then immediately fail with
   -EBUSY, making it impossible to register the slave again.  Fix
   by setting i2c_imx->slave = NULL on the error path.

2. In i2c_imx_unreg_slave(), the slave pointer is set to NULL after
   disabling interrupts.  However, a pending interrupt might already
   have started a timer (e.g. for slave event processing) before
   the pointer was cleared.  The timer callback
   i2c_imx_slave_event() dereferences i2c_imx->slave without a
   NULL check, which results in a use-after-free / NULL pointer
   dereference.  Prevent this by checking that i2c_imx->slave is
   valid before calling i2c_slave_event() and updating the
   last_slave_event field.

Both issues can trigger a kernel oops or permanent slave
registration failure under certain race conditions.  Add the
missing NULL assignment and the missing NULL check to harden
the slave path.

Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")
Cc: stable@vger.kernel.org
Signed-off-by: Liem <liem16213@gmail.com>
---
 drivers/i2c/busses/i2c-imx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 28313d0fad37..4f7bcbeecfd0 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -775,8 +775,10 @@ static void i2c_imx_enable_bus_idle(struct imx_i2c_struct *i2c_imx)
 static void i2c_imx_slave_event(struct imx_i2c_struct *i2c_imx,
 				enum i2c_slave_event event, u8 *val)
 {
-	i2c_slave_event(i2c_imx->slave, event, val);
-	i2c_imx->last_slave_event = event;
+	if (i2c_imx->slave) {
+		i2c_slave_event(i2c_imx->slave, event, val);
+		i2c_imx->last_slave_event = event;
+	}
 }
 
 static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
@@ -936,6 +938,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
 	/* Resume */
 	ret = pm_runtime_resume_and_get(i2c_imx->adapter.dev.parent);
 	if (ret < 0) {
+		i2c_imx->slave = NULL;
 		dev_err(&i2c_imx->adapter.dev, "failed to resume i2c controller");
 		return ret;
 	}
-- 
2.34.1



^ permalink raw reply related

* Re: [PATCH] net: stmmac: fix missed le32_to_cpu()
From: Maxime Chevallier @ 2026-06-25  7:07 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Ben Dooks, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Russell King (Oracle), netdev, linux-stm32, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20260624192205.4485cd61@kernel.org>



On 6/25/26 04:22, Jakub Kicinski wrote:
> On Mon, 22 Jun 2026 19:51:39 +0200 Maxime Chevallier wrote:
>> Hi Ben,
>>
>> On 6/22/26 16:37, Ben Dooks wrote:
>>> The print in ndesc_display_ring() sends the des2 and des3
>>> to the pr_info() without passing them through the relevant
>>> conversion to cpu order.
>>>
>>> Fix the (prototype) sparse warnings by using le32_to_cpu():
>>> drivers/net/ethernet/stmicro/stmmac/norm_desc.c:258:17: warning: incorrect type in argument 6 (different base types)
>>> drivers/net/ethernet/stmicro/stmmac/norm_desc.c:258:17:    expected unsigned int
>>> drivers/net/ethernet/stmicro/stmmac/norm_desc.c:258:17:    got restricted __le32 [usertype] des2
>>> drivers/net/ethernet/stmicro/stmmac/norm_desc.c:258:17: warning: incorrect type in argument 7 (different base types)
>>> drivers/net/ethernet/stmicro/stmmac/norm_desc.c:258:17:    expected unsigned int
>>> drivers/net/ethernet/stmicro/stmmac/norm_desc.c:258:17:    got restricted __le32 [usertype] des3
>>>
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>  
>>
>> I agree on the principle, but this isn't a fix so this'll have to wait
>> until net-next re-opens :)
> 
> Humpf, why are we not seeing this on x86 allmodconfig ? 🤔️
> 
> $ make C=1 W=1 drivers/net/ethernet/stmicro/stmmac/norm_desc.o 
>   DESCEND objtool
>   CC [M]  drivers/net/ethernet/stmicro/stmmac/norm_desc.o
>   CHECK   drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> $

Heh good point indeed !
  
>>> Fix the (prototype) sparse warnings by using le32_to_cpu():

Ben, what's this "prototype" sparse ? a custom tool of yours that
you used to find that ?

Maxime




^ permalink raw reply

* Re: [PATCH v6 2/9] dt-bindings: media: nxp: Add Wave6 video codec device
From: Krzysztof Kozlowski @ 2026-06-25  6:28 UTC (permalink / raw)
  To: Nas Chung
  Cc: Conor Dooley, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim,
	marek.vasut@mailbox.org
In-Reply-To: <SL2P216MB2441BB9DC91CCBE494F2B45BFBEC2@SL2P216MB2441.KORP216.PROD.OUTLOOK.COM>

On Thu, Jun 25, 2026 at 01:43:33AM +0000, Nas Chung wrote:
> >> +  sram:
> >> +    $ref: /schemas/types.yaml#/definitions/phandle
> >> +    description:
> >> +      phandle to the SRAM node used to store reference data, reducing DMA
> >> +      memory bandwidth.
> >> +
> >> +  iommus:
> >> +    maxItems: 1
> >> +
> >> +  "#cooling-cells":
> >> +    const: 2
> >> +
> >> +  "#address-cells":
> >> +    const: 2
> >> +
> >> +  "#size-cells":
> >> +    const: 2
> >> +
> >> +  ranges: true
> >> +
> >> +patternProperties:
> >> +  "^interface@[0-9a-f]+$":
> >
> >I have to wonder if this interface business is required at all.
> >Why can this not go into the parent, with each region fetchable via
> >reg-names, interrupt-names and iommu-names?
> 
> Thanks for your feedback.
> 
> I did try the flat model, but the blocker is the IOMMU.
> 
> The control region and four interface regions are independent DMA requesters
> with distinct stream IDs, and each interface can be assigned to a different VM,
> driving the video core with its own isolated memory.
> 
> If all stream IDs are listed under the parent's iommus, they bind to a
> single device and share one domain, so the isolation is lost.
> This is the main reason I added the interface nodes.

Feels similar to issue Qualcomm has. I rejected such subnodes and
Qualcomm came with a solution in DMA IOMMU code, but that solution was
rejected by DMA folks:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/

I don't have proper arguments to convince DMA folks, thus I agree for
Qualcomm for the subnodes. It should be fine here as well, in such case.

Best regards,
Krzysztof



^ permalink raw reply

* [PATCH net] net: airoha: fix max receive size configuration
From: Lorenzo Bianconi @ 2026-06-25  6:49 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Lorenzo Bianconi
  Cc: linux-arm-kernel, linux-mediatek, netdev, Madhur Agrawal

Set the GDM maximum receive size to AIROHA_MAX_RX_SIZE unconditionally
during hardware initialization instead of updating it according to the
configured MTU. This avoids dropping incoming frames that exceed the
current MTU but could still be processed by the networking stack, which
is able to fragment the reply on the TX side (e.g. ICMP echo requests).
Move the per-port MTU configuration to the PPE egress path where it
belongs, and set the tx frame size running airoha_ppe_set_xmit_frame_size()
to dynamically track the maximum MTU across running interfaces sharing
the same PPE instance.
Fix the PPE MTU register addressing to pack two port entries per
register word and add WAN_MTU0 configuration for non-LAN GDM devices.

Fixes: 54d989d58d2a ("net: airoha: Move min/max packet len configuration in airoha_dev_open()")
Tested-by: Madhur Agrawal <madhur.agrawal@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c  | 68 ++++++++++---------------------
 drivers/net/ethernet/airoha/airoha_eth.h  |  2 +
 drivers/net/ethernet/airoha/airoha_ppe.c  | 39 +++++++++++++-----
 drivers/net/ethernet/airoha/airoha_regs.h |  9 ++--
 4 files changed, 58 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 932b3a3df2e5..3f451c2d4c24 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -178,10 +178,15 @@ static void airoha_fe_maccr_init(struct airoha_eth *eth)
 {
 	int p;
 
-	for (p = 1; p <= ARRAY_SIZE(eth->ports); p++)
+	for (p = 1; p <= ARRAY_SIZE(eth->ports); p++) {
 		airoha_fe_set(eth, REG_GDM_FWD_CFG(p),
 			      GDM_TCP_CKSUM_MASK | GDM_UDP_CKSUM_MASK |
 			      GDM_IP4_CKSUM_MASK | GDM_DROP_CRC_ERR_MASK);
+		airoha_fe_rmw(eth, REG_GDM_LEN_CFG(p),
+			      GDM_SHORT_LEN_MASK | GDM_LONG_LEN_MASK,
+			      FIELD_PREP(GDM_SHORT_LEN_MASK, 60) |
+			      FIELD_PREP(GDM_LONG_LEN_MASK, AIROHA_MAX_RX_SIZE));
+	}
 
 	airoha_fe_rmw(eth, REG_CDM_VLAN_CTRL(1), CDM_VLAN_MASK,
 		      FIELD_PREP(CDM_VLAN_MASK, 0x8100));
@@ -1831,13 +1836,24 @@ static void airoha_update_hw_stats(struct airoha_gdm_dev *dev)
 	spin_unlock(&port->stats_lock);
 }
 
+static void airoha_dev_set_xmit_frame_size(struct net_device *netdev)
+{
+	struct airoha_gdm_dev *dev = netdev_priv(netdev);
+
+	airoha_ppe_set_xmit_frame_size(dev);
+	if (!airoha_is_lan_gdm_dev(dev))
+		airoha_fe_rmw(dev->eth, REG_WAN_MTU0, WAN_MTU0_MASK,
+			      FIELD_PREP(WAN_MTU0_MASK,
+					 VLAN_ETH_HLEN + netdev->mtu));
+}
+
 static int airoha_dev_open(struct net_device *netdev)
 {
-	int err, len = ETH_HLEN + netdev->mtu + ETH_FCS_LEN;
 	struct airoha_gdm_dev *dev = netdev_priv(netdev);
 	struct airoha_gdm_port *port = dev->port;
-	u32 cur_len, pse_port = FE_PSE_PORT_PPE1;
 	struct airoha_qdma *qdma = dev->qdma;
+	u32 pse_port = FE_PSE_PORT_PPE1;
+	int err;
 
 	netif_tx_start_all_queues(netdev);
 	err = airoha_set_vip_for_gdm_port(dev, true);
@@ -1851,19 +1867,7 @@ static int airoha_dev_open(struct net_device *netdev)
 		airoha_fe_clear(qdma->eth, REG_GDM_INGRESS_CFG(port->id),
 				GDM_STAG_EN_MASK);
 
-	cur_len = airoha_fe_get(qdma->eth, REG_GDM_LEN_CFG(port->id),
-				GDM_LONG_LEN_MASK);
-	if (!port->users || len > cur_len) {
-		/* Opening a sibling net_device with a larger MTU updates the
-		 * MTU of already running devices. This is required to allow
-		 * multiple net_devices with different MTUs to share the same
-		 * GDM port.
-		 */
-		airoha_fe_rmw(qdma->eth, REG_GDM_LEN_CFG(port->id),
-			      GDM_SHORT_LEN_MASK | GDM_LONG_LEN_MASK,
-			      FIELD_PREP(GDM_SHORT_LEN_MASK, 60) |
-			      FIELD_PREP(GDM_LONG_LEN_MASK, len));
-	}
+	airoha_dev_set_xmit_frame_size(netdev);
 	port->users++;
 
 	if (!airoha_is_lan_gdm_dev(dev) &&
@@ -1875,30 +1879,6 @@ static int airoha_dev_open(struct net_device *netdev)
 	return 0;
 }
 
-static void airoha_set_port_mtu(struct airoha_eth *eth,
-				struct airoha_gdm_port *port)
-{
-	u32 len = 0;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(port->devs); i++) {
-		struct airoha_gdm_dev *dev = port->devs[i];
-		struct net_device *netdev;
-
-		if (!dev)
-			continue;
-
-		netdev = netdev_from_priv(dev);
-		if (netif_running(netdev))
-			len = max_t(u32, len, netdev->mtu);
-	}
-	len += ETH_HLEN + ETH_FCS_LEN;
-
-	airoha_fe_rmw(eth, REG_GDM_LEN_CFG(port->id),
-		      GDM_LONG_LEN_MASK,
-		      FIELD_PREP(GDM_LONG_LEN_MASK, len));
-}
-
 static int airoha_dev_stop(struct net_device *netdev)
 {
 	struct airoha_gdm_dev *dev = netdev_priv(netdev);
@@ -1909,7 +1889,7 @@ static int airoha_dev_stop(struct net_device *netdev)
 	airoha_set_vip_for_gdm_port(dev, false);
 
 	if (--port->users)
-		airoha_set_port_mtu(dev->eth, port);
+		airoha_ppe_set_xmit_frame_size(dev);
 	else
 		airoha_set_gdm_port_fwd_cfg(qdma->eth,
 					    REG_GDM_FWD_CFG(port->id),
@@ -1962,10 +1942,6 @@ static int airoha_enable_gdm2_loopback(struct airoha_gdm_dev *dev)
 		      FIELD_PREP(LPBK_CHAN_MASK, chan) |
 		      LBK_GAP_MODE_MASK | LBK_LEN_MODE_MASK |
 		      LBK_CHAN_MODE_MASK | LPBK_EN_MASK);
-	airoha_fe_rmw(eth, REG_GDM_LEN_CFG(AIROHA_GDM2_IDX),
-		      GDM_SHORT_LEN_MASK | GDM_LONG_LEN_MASK,
-		      FIELD_PREP(GDM_SHORT_LEN_MASK, 60) |
-		      FIELD_PREP(GDM_LONG_LEN_MASK, AIROHA_MAX_MTU));
 	/* Forward the traffic to the proper GDM port */
 	pse_port = port->id == AIROHA_GDM3_IDX ? FE_PSE_PORT_GDM3
 					       : FE_PSE_PORT_GDM4;
@@ -2098,7 +2074,7 @@ static int airoha_dev_change_mtu(struct net_device *netdev, int mtu)
 
 	WRITE_ONCE(netdev->mtu, mtu);
 	if (port->users)
-		airoha_set_port_mtu(dev->eth, port);
+		airoha_dev_set_xmit_frame_size(netdev);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index d7ff8c5200e2..0c3fb6e5d7f1 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h
@@ -23,6 +23,7 @@
 #define AIROHA_MAX_DSA_PORTS		7
 #define AIROHA_MAX_NUM_RSTS		3
 #define AIROHA_MAX_MTU			9220
+#define AIROHA_MAX_RX_SIZE		16128
 #define AIROHA_MAX_PACKET_SIZE		2048
 #define AIROHA_NUM_QOS_CHANNELS		4
 #define AIROHA_NUM_QOS_QUEUES		8
@@ -676,6 +677,7 @@ int airoha_get_fe_port(struct airoha_gdm_dev *dev);
 bool airoha_is_valid_gdm_dev(struct airoha_eth *eth,
 			     struct airoha_gdm_dev *dev);
 
+void airoha_ppe_set_xmit_frame_size(struct airoha_gdm_dev *dev);
 void airoha_ppe_set_cpu_port(struct airoha_gdm_dev *dev, u8 ppe_id, u8 fport);
 bool airoha_ppe_is_enabled(struct airoha_eth *eth, int index);
 void airoha_ppe_check_skb(struct airoha_ppe_dev *dev, struct sk_buff *skb,
diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index 42f4b0f21d17..e7c78293002a 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c
@@ -97,6 +97,33 @@ void airoha_ppe_set_cpu_port(struct airoha_gdm_dev *dev, u8 ppe_id, u8 fport)
 		      __field_prep(DFT_CPORT_MASK(fport), fe_cpu_port));
 }
 
+void airoha_ppe_set_xmit_frame_size(struct airoha_gdm_dev *dev)
+{
+	struct airoha_gdm_port *port = dev->port;
+	struct airoha_eth *eth = dev->eth;
+	int i, ppe_id, index;
+	u32 len = 0;
+
+	for (i = 0; i < ARRAY_SIZE(port->devs); i++) {
+		struct airoha_gdm_dev *d = port->devs[i];
+		struct net_device *netdev;
+
+		if (!d)
+			continue;
+
+		netdev = netdev_from_priv(d);
+		if (netif_running(netdev))
+			len = max_t(u32, len, netdev->mtu);
+	}
+	len += VLAN_ETH_HLEN;
+
+	ppe_id = !airoha_is_lan_gdm_dev(dev) && airoha_ppe_is_enabled(eth, 1);
+	index = port->id == AIROHA_GDM4_IDX ? 7 : port->id;
+	airoha_fe_rmw(eth, REG_PPE_MTU(ppe_id, index),
+		      FP_EGRESS_MTU_MASK(index),
+		      __field_prep(FP_EGRESS_MTU_MASK(index), len));
+}
+
 static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 {
 	u32 sram_ppe_num_data_entries = PPE_SRAM_NUM_ENTRIES, sram_num_entries;
@@ -115,8 +142,6 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 		PPE_RAM_NUM_ENTRIES_SHIFT(sram_ppe_num_data_entries);
 
 	for (i = 0; i < eth->soc->num_ppe; i++) {
-		int p;
-
 		airoha_fe_wr(eth, REG_PPE_TB_BASE(i),
 			     ppe->foe_dma + sram_tb_size);
 
@@ -166,15 +191,6 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 		airoha_fe_wr(eth, REG_PPE_HASH_SEED(i), PPE_HASH_SEED);
 		airoha_fe_clear(eth, REG_PPE_PPE_FLOW_CFG(i),
 				PPE_FLOW_CFG_IP6_6RD_MASK);
-
-		for (p = 0; p < ARRAY_SIZE(eth->ports); p++)
-			airoha_fe_rmw(eth, REG_PPE_MTU(i, p),
-				      FP0_EGRESS_MTU_MASK |
-				      FP1_EGRESS_MTU_MASK,
-				      FIELD_PREP(FP0_EGRESS_MTU_MASK,
-						 AIROHA_MAX_MTU) |
-				      FIELD_PREP(FP1_EGRESS_MTU_MASK,
-						 AIROHA_MAX_MTU));
 	}
 
 	for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
@@ -196,6 +212,7 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 				 airoha_ppe_is_enabled(eth, 1);
 			fport = airoha_get_fe_port(dev);
 			airoha_ppe_set_cpu_port(dev, ppe_id, fport);
+			airoha_ppe_set_xmit_frame_size(dev);
 		}
 	}
 }
diff --git a/drivers/net/ethernet/airoha/airoha_regs.h b/drivers/net/ethernet/airoha/airoha_regs.h
index 436f3c8779c1..6fed63d013b4 100644
--- a/drivers/net/ethernet/airoha/airoha_regs.h
+++ b/drivers/net/ethernet/airoha/airoha_regs.h
@@ -327,9 +327,8 @@
 #define PPE_SRAM_TABLE_EN_MASK			BIT(0)
 
 #define REG_PPE_MTU_BASE(_n)			(((_n) ? PPE2_BASE : PPE1_BASE) + 0x304)
-#define REG_PPE_MTU(_m, _n)			(REG_PPE_MTU_BASE(_m) + ((_n) << 2))
-#define FP1_EGRESS_MTU_MASK			GENMASK(29, 16)
-#define FP0_EGRESS_MTU_MASK			GENMASK(13, 0)
+#define REG_PPE_MTU(_m, _n)			(REG_PPE_MTU_BASE(_m) + (((_n) / 2) << 2))
+#define FP_EGRESS_MTU_MASK(_n)			GENMASK(13 + (((_n) % 2) << 4), ((_n) % 2) << 4)
 
 #define REG_PPE_RAM_CTRL(_n)			(((_n) ? PPE2_BASE : PPE1_BASE) + 0x31c)
 #define PPE_SRAM_CTRL_ACK_MASK			BIT(31)
@@ -377,6 +376,10 @@
 #define REG_SRC_PORT_FC_MAP6		0x2298
 #define FC_ID_OF_SRC_PORT_MASK(_n)	GENMASK(4 + ((_n) << 3), ((_n) << 3))
 
+#define REG_WAN_MTU0			0x2300
+#define WAN_MTU1_MASK			GENMASK(29, 16)
+#define WAN_MTU0_MASK			GENMASK(13, 0)
+
 #define REG_CDM5_RX_OQ1_DROP_CNT	0x29d4
 
 /* QDMA */

---
base-commit: fd1269e454089abda0e4f9e5e25ecd02a90ab009
change-id: 20260618-airoha-fix-rx-max-len-57654b661646

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>



^ permalink raw reply related

* Re: [PATCH v2 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Krzysztof Kozlowski @ 2026-06-25  6:43 UTC (permalink / raw)
  To: Gerald Loacker
  Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-phy, linux-arm-kernel,
	linux-rockchip, linux-kernel, devicetree
In-Reply-To: <20260619-feature-mipi-csi-dphy-4k60-v2-2-323356c2cc2e@wolfvision.net>

On Fri, Jun 19, 2026 at 11:13:40AM +0200, Gerald Loacker wrote:
> Add support for the optional rockchip,clk-lane-phase device tree property
> to allow board-specific tuning of the clock lane sampling phase for
> improved signal integrity across supported data rates.
> 
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> ---
>  .../devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml          | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 03950b3cad08c..010950a8a8856 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -56,6 +56,15 @@ properties:
>      description:
>        Some additional phy settings are access through GRF regs.
>  
> +  rockchip,clk-lane-phase:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 7

Missing default here. If default is unknown, explain that in commit msg.

Best regards,
Krzysztof



^ permalink raw reply

* Re: [PATCH v4 0/6] mm/vmalloc: Speed up ioremap, vmalloc and vmap with contiguous memory
From: Dev Jain @ 2026-06-25  6:37 UTC (permalink / raw)
  To: Wen Jiang, linux-mm, linux-arm-kernel, catalin.marinas, will,
	akpm, urezki
  Cc: baohua, Xueyuan.chen21, rppt, david, ryan.roberts,
	anshuman.khandual, ajd, linux-kernel, jiangwen6, shanghaoqiang,
	Ard Biesheuvel
In-Reply-To: <20260618084726.1070022-1-jiangwen6@xiaomi.com>



On 18/06/26 2:17 pm, Wen Jiang wrote:
> This patchset accelerates ioremap, vmalloc, and vmap when the memory
> is physically fully or partially contiguous. Two techniques are used:
> 
> 1. Avoid page table rewalk when setting PTEs/PMDs for multiple memory
>    segments
> 2. Use batched mappings wherever possible in both vmalloc and ARM64
>    layers
> 
> Besides accelerating the mapping path, this also enables large
> mappings (PMD and cont-PTE) for vmap, which are currently not
> supported.
> 
> Patches 1-2 extend ARM64 vmalloc CONT-PTE mapping to support multiple
> CONT-PTE regions instead of just one.
> 
> Patch 3 extracts a common helper vmap_set_ptes() that consolidates PTE
> mapping logic between the ioremap and vmalloc/vmap paths, handling both
> CONT_PTE and regular PTE mappings. This prepares for the next patch.
> 
> Patch 4 extends the page table walk path to support page shifts other
> than PAGE_SHIFT and eliminates the page table rewalk for huge vmalloc
> mappings. The function is renamed from vmap_small_pages_range_noflush()
> to vmap_pages_range_noflush_walk().
> 
> Patches 5-6 add huge vmap support for contiguous pages, including
> support for non-compound pages with pfn alignment verification.
> 
> On the RK3588 8-core ARM64 SoC, with tasks pinned to a little core and
> the performance CPUfreq policy enabled, benchmark results:
> 
> * ioremap(1 MB): 1.35x faster (3407 ns -> 2526 ns)
> * vmalloc(1 MB) mapping time (excluding allocation) with
>   VM_ALLOW_HUGE_VMAP: 1.42x faster (5.00 us -> 3.53us)
> * vmap(100MB) with order-8 pages: 8.3x faster (1235 us -> 149 us)
> 
> Many thanks to Xueyuan Chen for his testing efforts on RK3588 boards.
> 

I am still a little nervous about doing vmap-huge by default.

We can play set_memory_* games on a vmap huge mapping partially, thus
forcing a pgtable split, and not all arches can handle a kernel pgtable
split.

For arm64, we can handle that with BBML2_NOABORT, but interestingly, in
change_memory_common, arch/arm64/mm/pageattr.c:

	area = find_vm_area((void *)addr);
	if (!area ||
	    ((unsigned long)kasan_reset_tag((void *)end) >
	     (unsigned long)kasan_reset_tag(area->addr) + area->size) ||
	    ((area->flags & (VM_ALLOC | VM_ALLOW_HUGE_VMAP)) != VM_ALLOC))
		return -EINVAL;

Even before my change fcf8dda8cc48, we were bailing out on

!(area->flags & VM_ALLOC))

So on arm64 we haven't been supporting set_memory_* for vmap memory at all, because
it has VM_MAP set and not VM_ALLOC. Although we have a contradictory comment above
this code so not sure if this was intentional:

"Let's restrict ourselves to mappings created by vmalloc (or vmap)."


So either there is no user in the kernel doing vmap + set_memory_* (looks like it
by doing an LLM scan), or it is not fatal for set_memory_* to fail.

But even if no one does it now, technically the API allows it.

> 



^ permalink raw reply

* Re: [PATCH v3 2/7] dt-bindings: serial: 8250: aspeed: add aspeed,vuart-over-pci bool prop
From: Krzysztof Kozlowski @ 2026-06-25  6:36 UTC (permalink / raw)
  To: Grégoire Layet
  Cc: joel, andrew, lkundrak, devicetree, gregkh, jirislaby, robh,
	krzk+dt, conor+dt, andrew, jacky_chou, yh_chung, ninad,
	anirudhsriniv, linux-serial, linux-aspeed, linux-arm-kernel,
	linux-kernel
In-Reply-To: <CAFi2wKbKr8FMcJeGWA5e1UZUTh2=LwYNkLEj6exd2as7=AcvVQ@mail.gmail.com>

On 24/06/2026 14:48, Grégoire Layet wrote:
> Hi Krzysztof,
> 
>> What does that mean? How UART can be accessible over PCI bus?
> 
> It's a Virtual UART. Internally, it's two FIFOs accessible via
> 8250-compatible register sets on both ends.

I do not know what is Virtual UART...

> There is 4 Virtuals UARTs on the LPC bus of the AST2600 and 2 of them
> are bridged over the PCI bus.
> So, from the host, you can access the 8250 register set on the PCI bus.

You mean these appear (or are) as PCI devices?

> 
Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/4] device property: Introduce fwnode_graph_for_each_endpoint_scoped()
From: Andy Shevchenko @ 2026-06-25  6:33 UTC (permalink / raw)
  To: Frank.Li
  Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Mauro Carvalho Chehab,
	Dafna Hirschfeld, Laurent Pinchart, 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: <20260624-fw_scoped-v2-1-0a8db472af4a@nxp.com>

On Wed, Jun 24, 2026 at 01:00:09PM -0400, Frank.Li@oss.nxp.com wrote:

> Similar to recently propose for_each_child_of_node_scoped() this new
> version of the loop macro instantiates a new local struct fwnode_handle *
> that uses the __free(fwnode_handle) auto cleanup handling so that if a
> reference to a node is held on early exit from the loop the reference will
> be released. If the loop runs to completion, the child pointer will be NULL
> and no action will be taken.
> 
> The reason this is useful is that it removes the need for
> fwnode_handle_put() on early loop exits. If there is a need to retain the
> reference, then return_ptr(child) or no_free_ptr(child) may be used to
> safely disable the auto cleanup.

...

> +#define fwnode_graph_for_each_endpoint_scoped(fwnode, child)			\
> +	for (struct fwnode_handle *child __free(fwnode_handle) =		\
> +		fwnode_graph_get_next_endpoint(fwnode, NULL);		\

Now there is a misindentation of the \, id est an additional tab is missing.

> +	     child; child = fwnode_graph_get_next_endpoint(fwnode, child))

Collect more tags and send a v3 :-)

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* [PATCH net v3] net: ti: icssg-prueth: fix XDP_TX from the AF_XDP zero-copy RX path
From: David Carlier @ 2026-06-25  6:31 UTC (permalink / raw)
  To: danishanwar, rogerq, andrew+netdev, netdev
  Cc: davem, edumazet, kuba, pabeni, horms, m-malladi, hawk,
	john.fastabend, sdf, ast, daniel, bpf, linux-arm-kernel,
	linux-kernel, stable, David Carlier

On XDP_TX from the zero-copy RX path, emac_run_xdp() converts the xsk
buffer via xdp_convert_zc_to_xdp_frame(), which clones the data into a
fresh MEM_TYPE_PAGE_ORDER0 page that is not DMA mapped. Transmitting it
as PRUETH_TX_BUFF_TYPE_XDP_TX derives the DMA address with
page_pool_get_dma_addr(), reading an uninitialized page->dma_addr, so
the device DMAs from a bogus address (corrupt TX, or an IOMMU fault).

Pick the TX buffer type from the frame's memory type: keep
PRUETH_TX_BUFF_TYPE_XDP_TX for page_pool frames and use
PRUETH_TX_BUFF_TYPE_XDP_NDO for the cloned zero-copy frame, which is then
DMA mapped through the NDO path and unmapped on completion.

While at it, fix the page_pool XDP_TX completion path. A
PRUETH_TX_BUFF_TYPE_XDP_TX frame carries a page_pool-owned DMA mapping
(established against rx_chn->dma_dev), yet prueth_xmit_free()
unconditionally calls dma_unmap_single() on it with tx_chn->dma_dev,
tearing down a mapping the driver does not own; xdp_return_frame()
already recycles the page back to the pool. Tag such frames with a
dedicated PRUETH_SWDATA_XDPF_TX type so the completion path skips the
unmap, the same way PRUETH_SWDATA_XSK buffers are handled.

Fixes: 7a64bb388df3 ("net: ti: icssg-prueth: Add AF_XDP zero copy for RX")
Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
Reviewed-by: Meghana Malladi <m-malladi@ti.com>
---
v3:
 - address Meghana Malladi review nits: split the prueth_xmit_free()
   guard to stay under 80 columns, parenthesize the swdata->type
   ternary (and the matching tx_buff_type one for consistency).
 - no functional change; carry Reviewed-by.
v2: https://lore.kernel.org/netdev/20260623112225.303930-1-devnexen@gmail.com
v1: https://lore.kernel.org/netdev/20260620213756.87499-1-devnexen@gmail.com
 drivers/net/ethernet/ti/icssg/icssg_common.c | 21 +++++++++++++++++---
 drivers/net/ethernet/ti/icssg/icssg_prueth.h |  1 +
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 82ddef9c17d5..64ae3704481e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -185,7 +185,8 @@ void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
 	first_desc = desc;
 	next_desc = first_desc;
 	swdata = cppi5_hdesc_get_swdata(first_desc);
-	if (swdata->type == PRUETH_SWDATA_XSK)
+	if (swdata->type == PRUETH_SWDATA_XSK ||
+	    swdata->type == PRUETH_SWDATA_XDPF_TX)
 		goto free_pool;
 
 	cppi5_hdesc_get_obuf(first_desc, &buf_dma, &buf_dma_len);
@@ -259,6 +260,7 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 			napi_consume_skb(skb, budget);
 			break;
 		case PRUETH_SWDATA_XDPF:
+		case PRUETH_SWDATA_XDPF_TX:
 			xdpf = swdata->data.xdpf;
 			dev_sw_netstats_tx_add(ndev, 1, xdpf->len);
 			total_bytes += xdpf->len;
@@ -769,7 +771,8 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
 	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
 	cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
 	swdata = cppi5_hdesc_get_swdata(first_desc);
-	swdata->type = PRUETH_SWDATA_XDPF;
+	swdata->type = (buff_type == PRUETH_TX_BUFF_TYPE_XDP_TX ?
+		PRUETH_SWDATA_XDPF_TX : PRUETH_SWDATA_XDPF);
 	swdata->data.xdpf = xdpf;
 
 	/* Report BQL before sending the packet */
@@ -804,6 +807,7 @@ EXPORT_SYMBOL_GPL(emac_xmit_xdp_frame);
  */
 static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len)
 {
+	enum prueth_tx_buff_type tx_buff_type;
 	struct net_device *ndev = emac->ndev;
 	struct netdev_queue *netif_txq;
 	int cpu = smp_processor_id();
@@ -826,11 +830,21 @@ static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *len
 			goto drop;
 		}
 
+		/* In AF_XDP zero-copy mode xdp_convert_buff_to_frame()
+		 * clones the xsk buffer into a fresh MEM_TYPE_PAGE_ORDER0
+		 * page that is not DMA mapped. Such a frame must be mapped
+		 * via the NDO path; only a page pool-backed frame already
+		 * carries a usable page_pool DMA address.
+		 */
+		tx_buff_type = (xdpf->mem_type == MEM_TYPE_PAGE_POOL ?
+				PRUETH_TX_BUFF_TYPE_XDP_TX :
+				PRUETH_TX_BUFF_TYPE_XDP_NDO);
+
 		q_idx = cpu % emac->tx_ch_num;
 		netif_txq = netdev_get_tx_queue(ndev, q_idx);
 		__netif_tx_lock(netif_txq, cpu);
 		result = emac_xmit_xdp_frame(emac, xdpf, q_idx,
-					     PRUETH_TX_BUFF_TYPE_XDP_TX);
+					     tx_buff_type);
 		__netif_tx_unlock(netif_txq);
 		if (result == ICSSG_XDP_CONSUMED) {
 			ndev->stats.tx_dropped++;
@@ -1395,6 +1409,7 @@ void prueth_tx_cleanup(void *data, dma_addr_t desc_dma)
 		dev_kfree_skb_any(skb);
 		break;
 	case PRUETH_SWDATA_XDPF:
+	case PRUETH_SWDATA_XDPF_TX:
 		xdpf = swdata->data.xdpf;
 		xdp_return_frame(xdpf);
 		break;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index df93d15c5b78..00bb760d68a9 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -153,6 +153,7 @@ enum prueth_swdata_type {
 	PRUETH_SWDATA_CMD,
 	PRUETH_SWDATA_XDPF,
 	PRUETH_SWDATA_XSK,
+	PRUETH_SWDATA_XDPF_TX,
 };
 
 enum prueth_tx_buff_type {
-- 
2.53.0



^ permalink raw reply related

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Andy Shevchenko @ 2026-06-25  6:31 UTC (permalink / raw)
  To: Frank Li
  Cc: Laurent Pinchart, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	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: <ajxCOE3avXXLlrfT@SMW015318>

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.

I slightly tend to the safest option (see below), but as a compromise I can
suggest to inline the fwnode_graph_for_each_endpoint() into that single user
that doesn't need a put. However, this may uglify the code and rise a question
of the consistency. So, consider that suggestion with grain of salt and apply
only if we have wider agreement with it.

> > > It'd better leave these as it.

TL;DR:
This is the safest option, of course. And as mentioned above I slightly
prefer this way. Another argument is that in some cases we might want to
have it in the future and since we have an existing user, let it live.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply

* Re: [PATCH v2 03/12] iio: adc: at91-sama5d2_adc: adapt the driver for sama7d65
From: Varshini.Rajendran @ 2026-06-25  5:53 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: ehristev, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
	Nicolas.Ferre, alexandre.belloni, claudiu.beznea, srini,
	linux-iio, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <ajrO3-buCfS0vx1L@ashevche-desk.local>

Hi Andy,

On 23/06/26 11:52 pm, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, Jun 23, 2026 at 04:29:35PM +0530, Varshini Rajendran wrote:
>> Add support for sama7d65 ADC. The differences are highlighted with the
>> compatible. The calibration data layout is the main difference.
> 
> Do you need to update a Kconfig help text?

Yes. I will update the supported SoC specifics in the Kconfig help text. 
I will also address the rest of your review comments in the v3 patchset. 
Thanks for your time.

> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 


-- 
Thanks,
Varshini Rajendran.

^ permalink raw reply

* Re: [PATCH] iommu/io-pgtable-arm: Add support for contiguous hint bit
From: Vijayanand Jitta @ 2026-06-25  5:47 UTC (permalink / raw)
  To: Daniel Mentz
  Cc: Joerg Roedel (AMD), Will Deacon, Robin Murphy, linux-arm-msm,
	iommu, linux-kernel, linux-arm-kernel, Prakash Gupta
In-Reply-To: <CAE2F3rABU2_d+e+NuFJ3ngDCEnGuVCsmE612V9RAKmyAX-R9Mw@mail.gmail.com>



On 6/20/2026 1:10 AM, Daniel Mentz wrote:
> On Thu, Jun 18, 2026 at 2:06 AM Vijayanand Jitta
> <vijayanand.jitta@oss.qualcomm.com> wrote:
>> Support is gated behind CONFIG_IOMMU_IO_PGTABLE_CONTIG_HINT, which
>> provides a compile-time opt-out for hardware affected by SMMU errata
>> related to the contiguous bit.
> 
> Have you considered making this a runtime option? Compare this with
> arm_smmu_device_iidr_probe() where the smmuv3 driver disables certain
> features based on the identified implementation and the errata
> affecting that implementation.
> 

Thanks for the review comments.

Good point. I’ll drop the Kconfig switch and make this runtime-controlled
via an io-pgtable quirk, so SMMU drivers can disable CONT based on errata.

>> On the mapping side, __arm_lpae_map() detects when the requested size
>> matches a contiguous range at the next level, sets the CONT bit on all
>> PTEs in the group, then recurses with the base block size and an
>> adjusted pgcount.
> 
> I would perform this check at the current level not the previous
> level. See comments below.
> 

Sure, will update this check at current level.

>>
>> On the unmapping side, the CONT bit is cleared from all PTEs in the
>> affected contiguous group before any individual entry is invalidated,
>> following the Break-Before-Make requirement of the architecture.
> 
> My understanding is that for unmap operations, the following rule applies:
> 
> The IOVA range targeted by an unmap operation must exactly match the
> IOVA range of a previous map operation. Partial unmap operations are
> not allowed.
> 
> The iopgtable code previously had a function named
> arm_lpae_split_blk_unmap() which allowed a block mapping to be split
> up. However, that function has since been removed, which aligns with
> prohibiting partial unmaps.
> The other concern I have is a potential race condition: While one
> thread clears the contiguous bit, another thread could try to unmap
> the same descriptor.
> 
> Consider dropping support for partial unmap and just triggering a
> WARN_ON() if you detect that a contiguous group is partially unmapped.
> 

Sure, will drop partial unmap support and  I'll update with WARN_ON()
as suggested.

>> +static inline int arm_lpae_cont_pmds(unsigned long size)
> 
> PMD is not a term that is used in this file. I advise against
> introducing this term.
> 

Agreed, I’ll avoid PMD terminology here and rename those helpers/comments
to use block-level wording.

>> +static u32 arm_lpae_find_num_cont(struct arm_lpae_io_pgtable *data, int lvl)
>> +{
>> +       if (lvl == ARM_LPAE_MAX_LEVELS - 2)
>> +               return arm_lpae_cont_pmds(ARM_LPAE_BLOCK_SIZE(lvl, data));
>> +       else if (lvl == ARM_LPAE_MAX_LEVELS - 1)
>> +               return arm_lpae_cont_ptes(ARM_LPAE_BLOCK_SIZE(lvl, data));
> 
> Consider supporting the contiguous bit at lookup level 1.
> 

Sure.

>>  static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
>>                           phys_addr_t paddr, size_t size, size_t pgcount,
>>                           arm_lpae_iopte prot, int lvl, arm_lpae_iopte *ptep,
>> @@ -463,6 +583,7 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
>>         size_t tblsz = ARM_LPAE_GRANULE(data);
>>         struct io_pgtable_cfg *cfg = &data->iop.cfg;
>>         int ret = 0, num_entries, max_entries, map_idx_start;
>> +       u32 num_cont = 1;
>>
>>         /* Find our entry at the current level */
>>         map_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
>> @@ -505,6 +626,24 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
>>                 return -EEXIST;
>>         }
>>
>> +       if (arm_lpae_pte_is_contiguous_range(data, size, lvl + 1, &num_cont)) {
> 
> I would recommend performing this check at the actual level not at the
> previous lookup level i.e. not at the (lvl - 1) level. Imagine the
> following situation: The granule size is 4KB, the initial lookup level
> is 2, and size is 32MB. I'm wondering if in that case, it'll just keep
> recursing until it hits (WARN_ON(lvl >= ARM_LPAE_MAX_LEVELS - 1)).
> 

Right, I see your point. The contiguous-size check should be done against the
current level, I’ll fix that in v2.

>> +#ifdef CONFIG_IOMMU_IO_PGTABLE_CONTIG_HINT
>> +static void arm_lpae_cont_clear(struct arm_lpae_io_pgtable *data,
>> +                               unsigned long iova, int lvl,
>> +                               arm_lpae_iopte *ptep, size_t num_entries)
>> +{
>> +       struct io_pgtable_cfg *cfg = &data->iop.cfg;
>> +       u32 num_cont = arm_lpae_find_num_cont(data, lvl);
>> +       arm_lpae_iopte *cont_ptep;
>> +       arm_lpae_iopte *cont_ptep_start;
>> +       unsigned long cont_iova;
>> +       int offset, itr;
>> +
>> +       cont_ptep = ptep - ARM_LPAE_LVL_IDX(iova, lvl, data);
>> +       cont_iova = round_down(iova,
>> +                              ARM_LPAE_BLOCK_SIZE(lvl, data) * num_cont);
> 
> As a result of this round_down() function, you are accessing a
> descriptor that describes an IOVA outside the range targeted by the
> iommu_unmap call. Consequently, you might race against another thread
> accessing the same descriptor.
> 

Agreed. I’m going to drop partial-unmap handling for contiguous groups,
so we will only operate on an exact aligned contiguous range and
reject partial unmaps with WARN_ON(). That also removes the need for
the current round_down()-based logic.

>> +       cont_ptep += ARM_LPAE_LVL_IDX(cont_iova, lvl, data);
>> +       cont_ptep_start = cont_ptep;
>> +
>> +       /*
>> +        * iova may not be aligned to the contiguous group boundary; include
>> +        * any leading entries so round_up() covers all overlapping groups.
>> +        */
>> +       offset = ARM_LPAE_LVL_IDX(iova, lvl, data) -
>> +                ARM_LPAE_LVL_IDX(cont_iova, lvl, data);
>> +       num_entries = round_up(offset + num_entries, num_cont);
>> +
>> +       for (itr = 0; itr < num_entries; itr++) {
>> +               WRITE_ONCE(*cont_ptep, READ_ONCE(*cont_ptep) & ~ARM_LPAE_PTE_CONT);
> 
> This read-modify-write operation is not safe due to the potential race
> described above.
> 

With partial unmap support removed, I suppose this should be fine now.

>> +               cont_ptep++;
>> +       }
>> +
>> +       if (!cfg->coherent_walk)
>> +               __arm_lpae_sync_pte(cont_ptep_start, num_entries, cfg);
>> +}
>> +#else
>> +static void arm_lpae_cont_clear(struct arm_lpae_io_pgtable *data,
>> +                               unsigned long iova, int lvl,
>> +                               arm_lpae_iopte *ptep, size_t num_entries)
>> +{
>> +}
>> +#endif
>> +
>>  static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>                                struct iommu_iotlb_gather *gather,
>>                                unsigned long iova, size_t size, size_t pgcount,
>> @@ -660,7 +841,7 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>  {
>>         arm_lpae_iopte pte;
>>         struct io_pgtable *iop = &data->iop;
>> -       int i = 0, num_entries, max_entries, unmap_idx_start;
>> +       int i = 0, num_cont = 1, num_entries, max_entries, unmap_idx_start;
>>
>>         /* Something went horribly wrong and we ran out of page table */
>>         if (WARN_ON(lvl == ARM_LPAE_MAX_LEVELS))
>> @@ -675,9 +856,15 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>         }
>>
>>         /* If the size matches this level, we're in the right place */
>> -       if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) {
>> +       if (size == ARM_LPAE_BLOCK_SIZE(lvl, data) ||
>> +           (size == arm_lpae_find_num_cont(data, lvl) *
>> +                    ARM_LPAE_BLOCK_SIZE(lvl, data))) {
>> +               size_t pte_size;
>> +
>>                 max_entries = arm_lpae_max_entries(unmap_idx_start, data);
>> -               num_entries = min_t(int, pgcount, max_entries);
>> +               num_cont = arm_lpae_check_num_cont(data, size, lvl);
>> +               num_entries = min_t(int, num_cont * pgcount, max_entries);
>> +               pte_size = size / num_cont;
>>
>>                 /* Find and handle non-leaf entries */
>>                 for (i = 0; i < num_entries; i++) {
>> @@ -687,11 +874,27 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>                                 break;
>>                         }
>>
>> +                       /*
>> +                        * Break-Before-Make: before invalidating any leaf
>> +                        * entry, clear the CONT bit from every entry in the
>> +                        * contiguous group(s) and flush the TLB, as required
>> +                        * by the architecture.  arm_lpae_cont_clear() covers
>> +                        * the full [iova, iova + num_entries * pte_size) range
>> +                        * via round_up(), so subsequent entries read back
>> +                        * CONT=0 and skip this block.
>> +                        */
>> +                       if (pte & ARM_LPAE_PTE_CONT) {
>> +                               arm_lpae_cont_clear(data, iova, lvl, ptep, num_entries);
>> +                               io_pgtable_tlb_flush_walk(iop, iova,
>> +                                                         num_entries * pte_size,
>> +                                                         ARM_LPAE_GRANULE(data));
> 
> I believe this is inefficient. Consider the case where we unmap 2MB
> worth of IOVA space mapped by 512 4KB page descriptors with the
> contiguous bit set. If I'm not mistaken, you're running CMOs
> (__arm_lpae_sync_pte) twice for every page descriptor. In addition,
> io_pgtable_tlb_flush_walk() will submit an extra CMD_SYNC and wait for
> it's completion.
> 
> Additionally, you perform rounding in arm_lpae_cont_clear(). However,
> io_pgtable_tlb_flush_walk() is called on the original, potentially
> unaligned range. Can this lead to under invalidation? Again, my
> preference would be to drop support for partial unmaps which would
> also remove the requirement for calling io_pgtable_tlb_flush_walk()
> here.
> 

Agreed. The current unmap path is more complex and expensive than necessary.
Since partial unmap of contiguous groups should not be supported, I will remove
the rounding-based handling and only permit unmaps that exactly match an
aligned contiguous group. That also eliminates the need for the 
extra io_pgtable_tlb_flush_walk() here.

Thanks,
Vijay

>> +                       }
>> +
>>                         if (!iopte_leaf(pte, lvl, iop->fmt)) {
>>                                 __arm_lpae_clear_pte(&ptep[i], &iop->cfg, 1);
>>
>>                                 /* Also flush any partial walks */
>> -                               io_pgtable_tlb_flush_walk(iop, iova + i * size, size,
>> +                               io_pgtable_tlb_flush_walk(iop, iova + i * pte_size, pte_size,
>>                                                           ARM_LPAE_GRANULE(data));
>>                                 __arm_lpae_free_pgtable(data, lvl + 1, iopte_deref(pte, data));
>>                         }



^ permalink raw reply


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