* Re: [PATCH v5 0/3] Switch Arm CCA to use an auxiliary device instead of a platform device
From: Greg KH @ 2026-05-14 12:45 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: linux-coco, linux-arm-kernel, linux-kernel, Catalin Marinas,
Jeremy Linton, Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland,
Sudeep Holla, Will Deacon, Steven Price, Suzuki K Poulose
In-Reply-To: <yq5ase7u5kmz.fsf@kernel.org>
On Thu, May 14, 2026 at 04:21:48PM +0530, Aneesh Kumar K.V wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
>
> > On Thu, May 14, 2026 at 03:10:27PM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> As discussed here:
> >> https://lore.kernel.org/all/20250728135216.48084-12-aneesh.kumar@kernel.org
> >>
> >> The general feedback was that a platform device should not be used when
> >> there is no underlying platform resource to represent. The existing CCA
> >> support uses a platform device solely to anchor the TSM interface in the
> >> device hierarchy, which is not an appropriate use of a platform device.
> >> Use an auxiliary device instead to track CCA support.
> >
> > Why an aux device? If this has no platform resources, please use the
> > faux bus support instead, that is what it is there for. aux devices are
> > used when you are sharing a real resource among different "child"
> > drivers, and need some way to coordinate that sharing. If you have no
> > resources, there's nothing to share, so no need for the complexity that
> > aux gives you, just use faux instead.
> >
>
> We did discuss between faux an auxiliary devices early here
> https://lore.kernel.org/all/20251010135922.GC3833649@ziepe.ca
>
> To summarize auxiliary device was choosen so that we can do module
> autoloading.
That's not a valid reason to use the aux driver, sorry. If you have
hardware that triggers an auto-module-load, then this is really a
hardware driver. If it is a "virtual" driver like this, then you need
to explicitly load it on your own. Don't abuse apis for reasons that
they are not designed for.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] arm64: dts: renesas: r8a78000: Fix GIC-720AE View 1 Redistributor description
From: Marek Vasut @ 2026-05-14 12:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-arm-kernel, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Kuninori Morimoto, Magnus Damm, Rob Herring,
devicetree, linux-renesas-soc
In-Reply-To: <86bjeixyi1.wl-maz@kernel.org>
On 5/14/26 9:04 AM, Marc Zyngier wrote:
> On Wed, 13 May 2026 23:30:08 +0100,
> Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
>>
>> The Renesas R-Car X5H (R8A78000) SoC contains Arm CoreLink GIC-720AE
>> Generic Interrupt Controller with Multi View capability. Firmware has
>> access to configuration View 0, Linux kernel has access to View 1.
>
> Huh. That's pretty unexpected. The usual wisdom is to give the APs
> view 0 so that it looks like a "normal" machine, rather than only a
> partition of the system (which is what view != 0 indicates).
>
> I guess there is some additional fun going on there, such as other
> CPUs getting a portion of the GIC for themselves, and firmware
> preventing whatever is running on the APs to interact with them...
This is my understanding as well, partitioning of the system is an
intended use case. On the development hardware I have access to, no
partitioning is applied, all cores are available to Linux.
>> The Arm CoreLink GIC-720AE Generic Interrupt Controller Technical
>> Reference Manual, currently latest r2p1 [1], chapter "Programmers model
>> for GIC-720AE", subchapter "Redistributor registers for control and
>> physical LPIs summary", part "GICR_TYPER, Redistributor Type Register"
>> clarifies register "GICR_TYPER" bit 4 "Last" behavior in Multi View
>> setup as follows:
>>
>> "
>> Last
>> Last Redistributor:
>>
>> 0 ... This Redistributor is not the last Redistributor on the chip.
>> 1 ... This Redistributor is the last Redistributor on the chip.
>> When GICD_CFGID.VIEW == 1, for views 1, 2, or 3 this bit
>> always returns 1.
>> "
>>
>> On this SoC, GICD_CFGID.VIEW is 1 and the Linux kernel has access to
>> View 1, therefore Linux kernel GICv3 driver will interpret register
>> "GICR_TYPER" bit 4 "Last" = 1 in the first Redistributor in continuous
>> Redistributor page as that first Redistributor being the one and only
>> Redistributor and will stop processing the continuous Redistributor
>> page further. This will prevent the other Redistributors from being
>> recognized by the system and used for other PEs.
>>
>> Because the hardware indicates that the continuous Redistributor page
>> is not continuous for View 1, 2, or 3, describe every Redistributor
>> separately in the DT. This makes all Redistributors for all cores
>> accessible in Linux.
>>
>> [1] https://developer.arm.com/documentation/102666/0201/Programmers-model-for-GIC-720AE/Redistributor-registers-for-control-and-physical-LPIs-summary/GICR-TYPER--Redistributor-Type-Register?lang=en
>>
>
> I am amazed that you managed to find anything at all on this web site.
>
> I would refrain from adding links to any ARM web sites in a commit
> message though. They tend to have a 10 minutes half-life period, and
> whole specs to disappear from public view when they are not flavour of
> the week anymore.
>
> A link to the equivalent PDF has better chances to survive further
> creative^WAI slop driven reorg of the documentation:
>
> https://documentation-service.arm.com/static/69ef3c1cd35efd294e335c43
>
> but your best bet is to download it, archive it, and refer to it
> yourself.
Thank you for that. That is the one thing I did not manage to find on
that website, a PDF of the GIC-720AE manual. I will include that link in
a V2.
>> Fixes: 63500d12cf76 ("arm64: dts: renesas: Add R8A78000 SoC support")
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>
> Acked-by: Marc Zyngier <maz@kernel.org>
>
> M.
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH 4/8] arm64: dts: qcom: kaanapali: Add qfprom node
From: Konrad Dybcio @ 2026-05-14 12:44 UTC (permalink / raw)
To: Akhil P Oommen, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Sean Paul, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm, freedreno, dri-devel, Jingyi Wang
In-Reply-To: <20260512-kaana-gpu-dt-v1-4-13e1c07c2050@oss.qualcomm.com>
On 5/12/26 12:23 AM, Akhil P Oommen wrote:
> From: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>
> Add the qfprom node and gpu related subnodes on Kaanapali SoC.
>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 5/8] arm64: dts: qcom: Add GPU support for Kaanapali
From: Konrad Dybcio @ 2026-05-14 12:43 UTC (permalink / raw)
To: Akhil P Oommen, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Sean Paul, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm, freedreno, dri-devel
In-Reply-To: <20260512-kaana-gpu-dt-v1-5-13e1c07c2050@oss.qualcomm.com>
On 5/12/26 12:23 AM, Akhil P Oommen wrote:
> Adreno 840 present in Kaanapali SoC is the second generation GPU in
> A8x family. It is based on the new slice architecture with 3 slices,
> higher GMEM/caches etc.
>
> There is some re-arrangement in the reglist to properly cover maximum
> register region. Other than this, the DT description is mostly similar
> to the existing chipsets except the OPP tables.
>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
[...]
> + gpu_opp_table: opp-table {
> + compatible = "operating-points-v2-adreno",
> + "operating-points-v2";
> +
> + opp-222000000 {
> + opp-hz = /bits/ 64 <222000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D2>;
> + opp-peak-kBps = <2136718>;
> + opp-supported-hw = <0x0f>;
> + /* ACD is disabled */
> + };
The clock plan also has a 160 MHz OPP @ LOWSVS_D3 and there's a couple of
interim OPPs that you have that aren't part of it (but maybe you have
better docs)
Otherwise lgtm but the size of the GPU region and the GMU base look
slightly confusing when I'm comparing them against the reg map
Konrad
^ permalink raw reply
* Re: [PATCH] ima: debugging late_initcall_sync measurements
From: Yeoreum Yun @ 2026-05-14 12:42 UTC (permalink / raw)
To: Mimi Zohar
Cc: David Safford, Jonathan McDowell, linux-security-module,
linux-kernel, linux-integrity, linux-arm-kernel, kvmarm, paul,
jmorris, serge, roberto.sassu, dmitry.kasatkin, eric.snowberg,
jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <af3oFfdSMUglZVHa@e129823.arm.com>
Hi Mimi,
> > On Fri, 2026-05-08 at 10:06 +0100, Yeoreum Yun wrote:
> >
> > > > The kernel selftests caused the measurements between late_initcall and
> > > > late_initcall_sync. After disabling all of the kernel selftests, there weren't
> > > > any measurements. Re-enabling the FIPS selftests on PowerVM LPAR resulted in
> > > > measurements. (I didn't try re-enabling any of the other selftests.)
> > > >
> > > > CONFIG_FIPS_SIGNATURE_SELFTEST=y
> > > > CONFIG_FIPS_SIGNATURE_SELFTEST_RSA=y
> > > > CONFIG_FIPS_SIGNATURE_SELFTEST_ECDSA=y
> > >
> > > Thanks for shraring this ;)
> > >
> > > I found the reason for those mesaurements. Those come from the
> > > request_module() and usermode-thread generates them while handling module
> > > loading request for crypto-x962(ecdsa-nist-p256).
> > > Since it's not a real kernel module,
> > > I confirmed file measurements between late_initcall and
> > > late_initcall_sync are gone for modeprobe with below change:
> > >
> > > @@ -1246,9 +1250,14 @@ EXPORT_SYMBOL_GPL(ima_measure_critical_data);
> > > */
> > > static int ima_kernel_module_request(char *kmod_name)
> > > {
> > > if (strncmp(kmod_name, "crypto-pkcs1(rsa,", 17) == 0)
> > > return -EINVAL;
> > >
> > > + if (IS_BUILTIN(CONFIG_CRYPTO_ECDSA) &&
> > > + (strncmp(kmod_name, "crypto-x962(ecdsa", 17) == 0))
> > > + return -EINVAL;
> > > +
> > > return 0;
> > > }
> > >
> > > Though this is the only request_module() call between
> > > late_initcall and late_initcall_sync, but I also confirmed there're
> > > request_modules() call before ima initalisation before "late_initcall":
> > >
> > > /*
> > > * NOTE: kmod_name is printed on ima_kernel_module_request()
> > > */
> > >
> > > // This is called from module_init(stm_core_init) -> device_initcall()
> > > // which is in driver/hwtracing/stm/core.c (built-in)
> > > [ 1.421986] ima: kmod_name: stm_p_basic
> > > ...
> > > [ 1.444900] ima: kmod_name: crypto-pkcs1(rsa,sha512)
> > > [ 1.444903] ima: kmod_name: crypto-pkcs1(rsa,sha512)-all
> > > ...
> > > [ 1.452029] ima: kmod_name: crypto-cbc(aes)
> > > [ 1.465321] ima: kmod_name: crypto-cbc(aes)-all
> > > ...
> > > [ 1.467845] Key type encrypted registered
> > > [ 1.467848] AppArmor: AppArmor sha256 policy hashing enabled
> > >
> > > // IMA is initailised at late_initcall level.
> > > [ 1.467850] ima: [init_ima_late:1336]
> > >
> > > If IMA should care request_module() from kernel before IMA init,
> > > I think there is no way to solve except queuing those events
> > > (kernel_load_data/kernel_load_post_data and open for module binary etc.)
> > > though it breaks "measure before use" principle since IMA couldn't
> > > measure at that time.
> > >
> > > But if you don't care about those things -- some events happend before
> > > IMA init, I think your suggestion -- controlling the init time of ima_init()
> > > via a Kconfig option is good and ignoring some usermodehelper request
> > > including request_module() before IMA initialisation upto user by that option.
> >
> > Thank you for the complete analysis. The early measurements before the TPM is
> > initialized is a problem that needs to be addressed. As to whether the solution
> > will require queueing is yet to be determined. (Roberto has some thoughts on
> > addressing it.) This discussion makes it clear that simply delaying IMA
> > initialization by moving it from late_initcall to late_initcall_sync could miss
> > measurements. That said, exposing it as an opt-in Kconfig for those who accept
> > the risk is a sensible pragmatic compromise.
>
> I think once we address ealry measurements before intialising TPM,
> It doesn't matter when IMA is initialissed since they're considered as
> ealry measurements anyway.
>
> BTW, I'm not sure whether we should take pragmatic compromise first to
> support deferred TPM initialisation or solving it together via solution
> of ealry measurements (whatever it is) in now.
I wonder what's going on for discussion to resolve these problem:
1) measurement event (via file operation) before IMA initialisation.
2) deferred TPM device initailisation and IMA.
Might someone could think it wouldn't be a problem since initrd is
measuared in PCR9 by boot loader (e.x) grub, but it still has a problem
for the case uses root= boot option where it doesn't use initrd
but use specified block dev with a filesystem.
I think soluation would be determined whether IMA neglects the
measurement event before its initialisation or not in current state:
a) Case for neglecting measurement event before IMA initailisation.
In this case, As you suggeested, IMA initialisation should be
determined by build config whether it initialises at late_initcall
or late_initcall_sync so that make user can choice upto their
platform.
b) Case for considering measurement event event before IMA
initialisation.
I couldn't image any other solution except queuing those event
and extend them after generating boot_aggregate log and if those
event can be queued, it wouldn't a problem to move IMA initialisation
to late_initcall_sync.
But you mention there are some thoughts from Roberto, might there was
some discussion with him. If you don't mind, would you let me know
how the discussion is going on and your thought to fix this all?
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH 8/8] arm64: dts: qcom: kaanapali-qrd: Enable GPU
From: Konrad Dybcio @ 2026-05-14 12:37 UTC (permalink / raw)
To: Akhil P Oommen, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Sean Paul, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm, freedreno, dri-devel
In-Reply-To: <20260512-kaana-gpu-dt-v1-8-13e1c07c2050@oss.qualcomm.com>
On 5/12/26 12:23 AM, Akhil P Oommen wrote:
> Add the secure firmware name property and enable GPU support on
> Kaanapali QRD device.
>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 7/8] arm64: dts: qcom: kaanapali-mtp: Enable GPU
From: Konrad Dybcio @ 2026-05-14 12:36 UTC (permalink / raw)
To: Akhil P Oommen, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Sean Paul, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm, freedreno, dri-devel
In-Reply-To: <20260512-kaana-gpu-dt-v1-7-13e1c07c2050@oss.qualcomm.com>
On 5/12/26 12:23 AM, Akhil P Oommen wrote:
> Add the secure firmware name property and enable GPU support on
> Kaanapali MTP device.
>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 3/8] arm64: dts: qcom: kaanapali: add the GPU SMMU node
From: Konrad Dybcio @ 2026-05-14 12:36 UTC (permalink / raw)
To: Akhil P Oommen, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Sean Paul, linux-arm-kernel, iommu, devicetree, linux-kernel,
linux-arm-msm, freedreno, dri-devel, Qingqing Zhou
In-Reply-To: <20260512-kaana-gpu-dt-v1-3-13e1c07c2050@oss.qualcomm.com>
On 5/12/26 12:23 AM, Akhil P Oommen wrote:
> From: Qingqing Zhou <quic_qqzhou@quicinc.com>
>
> Add the Adreno GPU SMMU node for kaanapali platform.
>
> Signed-off-by: Qingqing Zhou <quic_qqzhou@quicinc.com>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/kaanapali.dtsi | 41 +++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index bab654bbd6d0..26a4de9c8d45 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> @@ -2597,6 +2597,47 @@ gpucc: clock-controller@3d90000 {
> #power-domain-cells = <1>;
> };
>
> + adreno_smmu: iommu@3da0000 {
> + compatible = "qcom,kaanapali-smmu-500", "qcom,adreno-smmu",
> + "qcom,smmu-500", "arm,mmu-500";
The lines are misaligned, but please reshuffle this to be 1 a line
> + reg = <0x0 0x3da0000 0x0 0x40000>;
Please keep the 8-hex-digit padding for addr
> + #iommu-cells = <2>;
> + #global-interrupts = <1>;
> + dma-coherent;
> +
> + power-domains = <&gpucc GPU_CC_CX_GDSC>;
> +
> + clocks = <&gpucc GPU_CC_GPU_SMMU_VOTE_CLK>;
> + clock-names = "hlos";
> +
> + interrupts = <GIC_SPI 674 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 678 IRQ_TYPE_LEVEL_HIGH>,
likewise, please align the '<'s and align the property order in this node
with Glymur
Konrad
^ permalink raw reply
* Re: [PATCH] ARM: Do not select HAVE_RUST when KASAN is enabled
From: Nathan Chancellor @ 2026-05-14 12:35 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Alice Ryhl, Russell King, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Christian Schrrefl,
linux-arm-kernel, linux-kernel, rust-for-linux, stable
In-Reply-To: <CANiq72nm_3KM4gMnb0x34oJk1+_8XrUz-43zwW58Mr1UHG8qtQ@mail.gmail.com>
On Mon, May 11, 2026 at 11:58:42AM +0200, Miguel Ojeda wrote:
> On Mon, May 11, 2026 at 11:09 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Sure, I kept it simple for backporting purposes but I don't mind
> > breaking out the dependencies into their own symbol, even though it
> > feels like that could be done when support for the sanitizer is
> > re-enabled, which would truly mirror what you did. No strong opinion
> > though, so I will send a v2 after giving some time for other comments.
>
> I think it is fine either way, especially for a fix, but up to the
> KASAN/arm maintainers of course.
>
> Thanks for the patch!
>
> If KASAN or arm maintainers want to pick it up:
>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>
> Otherwise I can send it in a fixes PR I will likely need to send later
> this cycle, so please let me know!
FWIW, I think Russell has been away dealing with personal stuff
recently:
https://lore.kernel.org/aeDSTIS9-TDSihbX@shell.armlinux.org.uk/
So I doubt he would fight you taking it, given that it is Rust related.
I am rather selfishly motivated to have it picked up and merged because
I have to remember to pass KCONFIG_ALLCONFIG=<(echo CONFIG_RUST=n) every
time that I have to test arm allmodconfig. But don't feel rushed to pick
it up if you want to wait for a formal agreement on the path forward.
--
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Jason Gunthorpe @ 2026-05-14 12:35 UTC (permalink / raw)
To: Mostafa Saleh
Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
Jiri Pirko, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agW2lzJI-20DyJVe@google.com>
> > How will pKVM signal what kind of memory the DMA needs then?
> >
> > Does it use set_memory_decrypted()? How can it use
> > set_memory_decrypted() without offering CC_ATTR_MEM_ENCRYPT ?
>
> pKVM (hypervisor) doesn’t signal anything.
> The VMM when running protected guests will use restricted dma-pools
> for emulated vritio devices in the guest, which gets decrypted by
> the guest kernel and hence shared with the host kernel, and then
> traffic is bounced via the pool.
That really does sound like CC and set_memory_decrypted() to me..
> It’s also worth noting that bouncing here isn't just about visibility.
> Because memory sharing operates at page granularity, bouncing sub-page
> allocations through the restricted pool prevents adjacent, sensitive
> guest data from being exposed to the untrusted host.
That's a somewhat different problem, we have the dev->trusted stuff
that is supposed to deal with this kind of security. We need it for
IOMMU based systems too, eg hot plug thunderbolt should have it.
Then CC issue is more that the DMA API can't decrypt random passed in
memory because doing so often requires changing the PTEs pointing at
the page so it would break everything if done transparently.
> > > I believe that the pool should have a way to control it’s property
> > > (encrypted or decrypted) and that takes priority over whatever
> > > attributes comes from allocation.
> >
> > We should get here because dma_capable() fails, and then swiotlb needs
> > to return something that makes dma_capable() succeed. Yes, it should
> > return details about the thing it decided, but it shouldn't have been
> > pre-created with some idea how to make dma_capable() work.
>
> That sounds neat, but at the end we have force_dma_unencrypted() in
> dma_capable() which is just hardcoded to true/false by the platform.
For now, the next step is it becomes per-device and dynamic during the
device lifecycle.
> How is that different from having the state static by the pool?
statically attached pools to the device are not so flexible when
devices have dynamically changing capabilities..
> > If dma_capable() can fail, then swiotlb should know exactly what to do
> > to fix it.
>
> dma_capable() returns a bool, I don’t think it can know what exactly
> went wrong (based on address, size, attrs, dev...)
Yes, but I think the design is swiotlb is supposed to re-inspect what
is going on against the limits dma_capable checks and then select the
correct remedy..
> While we can debate the aesthetics of the setup , this is
> the exisitng behaviour for Linux, which existed for years
> and pKVM relies on and is used extensively.
> And, this patch alters that long-standing logic and introduces
> a functional regression.
Yeah, Aneesh needs to do something here, I'm pointing out it is
entirely seperate thing from the CC path we are working on which is
decoupling CC from reylying on force swiotlb.
> We can address this by either adjusting this patch or by changing
> pKVM guests to be more aligned with other CCA guests which is
> something I have been wondering about if it would help reduce
> bouncing.
Every time I look at pkvm I think it is just ARM CCA with a different
design and no access to the unique HW features..
> > If we can make that work then maybe the flows are designed correctly.
>
> Mmm, I am not sure I understand this one, shouldn’t the device also be
> notified about the switch in memory state, if it expects to read/write
> decrypted memory, how would that work if the kernel changes it to an
> encrypted one?
Nothing on the device changes. In a CC world we put the device in a
T=0 or T=1 state before the driver loads and the expectation from the
DMA API is that the device will only use that T=x DMA type during
operation.
A T=1 state device can access all of memory, private or shared. Any
information the platform may need is encoded in the dma_addr_t or in
the S1 IOPTEs.
So we never need to tell the device driver what kind of memory the DMA
is targetting, and we NEVER expect a device in T=1 mode to have to
issue a T=0 DMA to use the DMA API.
In a pkvm world it should be the same, the S2 table for the SMMU will
control what the device can access, and if the SMMU points to a
"private" or "shared" page is not something the device needs to know
or care about.
Jason
^ permalink raw reply
* Re: [PATCH v3 1/1] dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for MT8188
From: Krzysztof Kozlowski @ 2026-05-14 12:32 UTC (permalink / raw)
To: Arnab Layek
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, robh,
krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
andersson, mathieu.poirier, linux-remoteproc,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260514-poised-green-beagle-79cb9c@quoll>
On 14/05/2026 14:30, Krzysztof Kozlowski wrote:
> On Thu, May 14, 2026 at 07:45:33PM +0800, Arnab Layek wrote:
>> The MT8188 SCP requires support for 1-2 reserved memory regions, while
>> other MediaTek SoCs use only a single memory region.
>>
>> The schema uses a permissive base with restrictive conditionals:
>> 1) Base schema allows all devices minItems: 1, maxItems: 2
>> 2) Non-MT8188 devices (mt8183, mt8186, mt8192, mt8195, mt8195-dual) are
>> restricted to maxItems: 1, overriding the base
>> 3) MT8188 devices (mt8188, mt8188-dual) set minItems: 1 with item
>> descriptions, inheriting maxItems: 2 from base, making the second
>> L1TCM region optional
>>
>> This follows the same pattern as other MediaTek dt-bindings such as
>> mediatek,jpeg-encoder.yaml which uses conditional schemas to support
>> different numbers of iommus per device variant.
>>
>
> So I just reviewed v2, because it appeared in patchwork thread...
> because you just threaded v3 there.
>
> No, really, this was repeated also to Mediatek so many times.
>
> Implement v2 review.
>
Heh, and now I noticed that you just merged THREE versions in one thread
leading to complete mess in this entire thread.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/1] dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for MT8188
From: Krzysztof Kozlowski @ 2026-05-14 12:30 UTC (permalink / raw)
To: Arnab Layek
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, robh,
krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
andersson, mathieu.poirier, linux-remoteproc,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260514114533.174008-2-arnab.layek@mediatek.com>
On Thu, May 14, 2026 at 07:45:33PM +0800, Arnab Layek wrote:
> The MT8188 SCP requires support for 1-2 reserved memory regions, while
> other MediaTek SoCs use only a single memory region.
>
> The schema uses a permissive base with restrictive conditionals:
> 1) Base schema allows all devices minItems: 1, maxItems: 2
> 2) Non-MT8188 devices (mt8183, mt8186, mt8192, mt8195, mt8195-dual) are
> restricted to maxItems: 1, overriding the base
> 3) MT8188 devices (mt8188, mt8188-dual) set minItems: 1 with item
> descriptions, inheriting maxItems: 2 from base, making the second
> L1TCM region optional
>
> This follows the same pattern as other MediaTek dt-bindings such as
> mediatek,jpeg-encoder.yaml which uses conditional schemas to support
> different numbers of iommus per device variant.
>
So I just reviewed v2, because it appeared in patchwork thread...
because you just threaded v3 there.
No, really, this was repeated also to Mediatek so many times.
Implement v2 review.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/1] dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for MT8188
From: Krzysztof Kozlowski @ 2026-05-14 12:29 UTC (permalink / raw)
To: Arnab Layek
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, linux-remoteproc, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260511121004.2984149-2-arnab.layek@mediatek.com>
On Mon, May 11, 2026 at 08:10:04PM +0800, Arnab Layek wrote:
> The MT8188 SCP requires two reserved memory regions:
> 1. Main SCP SRAM memory region (required)
> 2. SCP L1TCM memory region (optional, for additional memory)
>
> Some other MediaTek SoCs only use a single memory region. This patch adds
Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94
> a conditional schema using if/then to allow 1-2 memory regions
> specifically for mediatek,mt8188-scp and mediatek,mt8188-scp-dual
> compatibles, while keeping the default maxItems: 1 for other
> SoCs.
Stop explaining what you did. Explain WHY. Why second entry is optional?
Why are you changing existing binding? Was it working? Not? Why not? Why
yes?
>
> Each memory region is documented with descriptions to
> clarify their purpose, following the pattern used in other bindings.
Redundant. We can read the diff.
>
> Signed-off-by: Arnab Layek <arnab.layek@mediatek.com>
> ---
> .../bindings/remoteproc/mtk,scp.yaml | 21 +++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index bdbb12118da4..df13be2026a6 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -205,6 +205,27 @@ allOf:
> items:
> - const: cfg
> - const: l1tcm
> + - if:
> + properties:
> + compatible:
> + enum:
> + - mediatek,mt8188-scp
> + - mediatek,mt8188-scp-dual
> + then:
> + properties:
> + memory-region:
> + minItems: 1
> + items:
> + - description: Main SCP SRAM memory region
> + - description: Optional SCP L1TCM memory region
Conflicts top level.
> + patternProperties:
> + "^scp@[a-f0-9]+$":
> + properties:
> + memory-region:
> + minItems: 1
> + items:
> + - description: Main SCP SRAM memory region
> + - description: Optional SCP L1TCM memory region
>
> additionalProperties: false
>
> --
> 2.45.2
>
^ permalink raw reply
* Re: [PATCH v2 8/8] PCI: rzg3s-host: Add 100 ms delay after link training
From: kernel test robot @ 2026-05-14 12:19 UTC (permalink / raw)
To: Hans Zhang, bhelgaas, lpieralisi, kwilczynski, mani, vigneshr,
jingoohan1, thomas.petazzoni, pali, ryder.lee, jianjun.wang,
claudiu.beznea.uj, mpillai
Cc: llvm, oe-kbuild-all, robh, s-vadapalli, linux-omap,
linux-arm-kernel, linux-mediatek, linux-renesas-soc, linux-pci,
linux-kernel, Hans Zhang
In-Reply-To: <20260506152346.166056-9-18255117159@163.com>
Hi Hans,
kernel test robot noticed the following build errors:
[auto build test ERROR on a293ec25d59dd96309058c70df5a4dd0f889a1e4]
url: https://github.com/intel-lab-lkp/linux/commits/Hans-Zhang/PCI-Add-pcie_wait_after_link_train-helper/20260514-132815
base: a293ec25d59dd96309058c70df5a4dd0f889a1e4
patch link: https://lore.kernel.org/r/20260506152346.166056-9-18255117159%40163.com
patch subject: [PATCH v2 8/8] PCI: rzg3s-host: Add 100 ms delay after link training
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260514/202605141426.2RPW8nvf-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260514/202605141426.2RPW8nvf-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605141426.2RPW8nvf-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/pci/access.c:8:
>> drivers/pci/pci.h:73:3: error: call to undeclared function 'msleep'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
73 | msleep(PCIE_RESET_CONFIG_WAIT_MS);
| ^
1 error generated.
--
In file included from drivers/pci/rebar.c:17:
>> drivers/pci/pci.h:73:3: error: call to undeclared function 'msleep'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
73 | msleep(PCIE_RESET_CONFIG_WAIT_MS);
| ^
>> drivers/pci/rebar.c:142:31: warning: implicit conversion from 'unsigned long long' to 'u32' (aka 'unsigned int') changes value from 140737488355328 to 0 [-Wconstant-conversion]
142 | if (size < 0 || size > ilog2(SZ_128T) - ilog2(PCI_REBAR_MIN_SIZE))
| ~~~~~~^~~~~~~~
include/linux/sizes.h:70:20: note: expanded from macro 'SZ_128T'
70 | #define SZ_128T _AC(0x800000000000, ULL)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/uapi/linux/const.h:21:18: note: expanded from macro '_AC'
21 | #define _AC(X,Y) __AC(X,Y)
| ^~~~~~~~~
include/uapi/linux/const.h:20:20: note: expanded from macro '__AC'
20 | #define __AC(X,Y) (X##Y)
| ^~~~
<scratch space>:37:1: note: expanded from here
37 | 0x800000000000ULL
| ^~~~~~~~~~~~~~~~~
include/linux/log2.h:162:14: note: expanded from macro 'ilog2'
162 | __ilog2_u32(n) : \
| ~~~~~~~~~~~ ^
1 warning and 1 error generated.
--
In file included from drivers/pci/msi/pcidev_msi.c:5:
>> drivers/pci/msi/../pci.h:73:3: error: call to undeclared function 'msleep'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
73 | msleep(PCIE_RESET_CONFIG_WAIT_MS);
| ^
1 error generated.
--
In file included from drivers/pci/pcie/portdrv.c:22:
>> drivers/pci/pcie/../pci.h:73:3: error: call to undeclared function 'msleep'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
73 | msleep(PCIE_RESET_CONFIG_WAIT_MS);
| ^
1 error generated.
vim +/msleep +73 drivers/pci/pci.h
62
63 /**
64 * pcie_wait_after_link_train - Wait 100 ms if link speed > 5 GT/s
65 * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, ...)
66 *
67 * Must be called after Link training completes and before the first
68 * Configuration Request is sent.
69 */
70 static inline void pcie_wait_after_link_train(int max_link_speed)
71 {
72 if (max_link_speed > 2)
> 73 msleep(PCIE_RESET_CONFIG_WAIT_MS);
74 }
75
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] i2c: davinci: fix division by zero on missing clock-frequency
From: Andrew Lunn @ 2026-05-14 12:16 UTC (permalink / raw)
To: Chaitanya Sabnis
Cc: brgl, andi.shyti, linux-arm-kernel, linux-i2c, linux-kernel,
Sashiko
In-Reply-To: <20260514103740.5416-1-chaitanya.msabnis@gmail.com>
On Thu, May 14, 2026 at 04:07:40PM +0530, Chaitanya Sabnis wrote:
> When the 'clock-frequency' property is missing from the device tree,
> the driver falls back to DAVINCI_I2C_DEFAULT_BUS_FREQ. However, this
> macro is defined in kHz (100), whereas the device tree property is
> expected in Hz.
>
> The probe function blindly divided the fallback value by 1000, causing
> integer truncation that resulted in dev->bus_freq = 0. This triggered
> a deterministic division-by-zero kernel panic when calculating clock
> dividers later in the probe sequence.
>
> Fix this by isolating the division so it only applies to the Hz value
> read from the device tree, cleanly assigning the kHz default otherwise.
Why not keep the patch simple and just change the value of
DAVINCI_I2C_DEFAULT_BUS_FREQ to Hz?
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260514044726.57297C2BCB7@smtp.kernel.org/
> Signed-off-by: Chaitanya Sabnis <chaitanya.msabnis@gmail.com>
Please also added a Fixes: tag.
Andrew
^ permalink raw reply
* Re: [PATCH v2 2/2] ARM: dts: aspeed: Add ASRock Rack B650D4U BMC
From: Andrew Lunn @ 2026-05-14 12:08 UTC (permalink / raw)
To: Prasanth Kumar Padarthi
Cc: joel, andrew, robh, krzk+dt, conor+dt, devicetree, linux-aspeed,
linux-arm-kernel
In-Reply-To: <20260514031622.1416922-3-prasanth.padarthi10@gmail.com>
> +&mac0 {
> + status = "okay";
> + phy-mode = "rgmii-rxid";
https://elixir.bootlin.com/linux/v6.15/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287
Please could you explain the choice of rgmii-rxid.
Andrew
^ permalink raw reply
* Re: [PATCH v2 7/8] dt-bindings: display: allwinner: Split H616 DE33 layer reg space
From: Krzysztof Kozlowski @ 2026-05-14 12:04 UTC (permalink / raw)
To: Jernej Skrabec
Cc: wens, samuel, mripard, maarten.lankhorst, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt, mturquette, sboyd, dri-devel,
devicetree, linux-arm-kernel, linux-sunxi, linux-kernel,
linux-clk
In-Reply-To: <20260509190015.79086-8-jernej.skrabec@siol.net>
On Sat, May 09, 2026 at 09:00:14PM +0200, Jernej Skrabec wrote:
> From: Jernej Skrabec <jernej.skrabec@gmail.com>
>
> As it turns out, current H616 DE33 binding was written based on
> incomplete understanding of DE33 design. Namely, planes are shared
> resource and not tied to specific mixer, which was the case for previous
> generations of Display Engine (DE3 and earlier).
>
> This means that current DE33 binding doesn't properly reflect HW and
> using it would mean that second mixer (used for second display output)
> can't be supported.
>
> Remove layer register space, which will be represented with additional
> node, and replace it with phandle, which will point to that new, shared
> node. That way, all mixers can share same layers.
>
> There is no user of this binding yet, so changes can be made safely,
> without breaking any backward compatibility.
There is user. git grep gives me:
drivers/gpu/drm/sun4i/sun8i_mixer.c
which means this is a released ABI. As I understood, the old code was
working fine but just did not support all use cases. Why this cannot be
kept backwards compatible?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Mostafa Saleh @ 2026-05-14 12:02 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <yq5ah5oaa63h.fsf@kernel.org>
On Thu, May 14, 2026 at 11:24:42AM +0530, Aneesh Kumar K.V wrote:
> Mostafa Saleh <smostafa@google.com> writes:
>
> > On Tue, May 12, 2026 at 02:33:59PM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> Teach swiotlb to distinguish between encrypted and decrypted bounce
> >> buffer pools, and make allocation and mapping paths select a pool whose
> >> state matches the requested DMA attributes.
> >>
> >> Add a decrypted flag to io_tlb_mem, initialize it for the default and
> >> restricted pools, and propagate DMA_ATTR_CC_SHARED into swiotlb pool
> >> allocation. Reject swiotlb alloc/map requests when the selected pool does
> >> not match the required encrypted/decrypted state.
> >>
> >> Also return DMA addresses with the matching phys_to_dma_{encrypted,
> >> unencrypted} helper so the DMA address encoding stays consistent with the
> >> chosen pool.
> >>
> >> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> >> ---
> >> include/linux/dma-direct.h | 10 ++++
> >> include/linux/swiotlb.h | 8 ++-
> >> kernel/dma/direct.c | 14 +++--
> >> kernel/dma/swiotlb.c | 108 +++++++++++++++++++++++++++----------
> >> 4 files changed, 107 insertions(+), 33 deletions(-)
> >>
> >> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> >> index c249912456f9..94fad4e7c11e 100644
> >> --- a/include/linux/dma-direct.h
> >> +++ b/include/linux/dma-direct.h
> >> @@ -77,6 +77,10 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
> >> #ifndef phys_to_dma_unencrypted
> >> #define phys_to_dma_unencrypted phys_to_dma
> >> #endif
> >> +
> >> +#ifndef phys_to_dma_encrypted
> >> +#define phys_to_dma_encrypted phys_to_dma
> >> +#endif
> >> #else
> >> static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
> >> {
> >> @@ -90,6 +94,12 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
> >> {
> >> return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
> >> }
> >> +
> >> +static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
> >> + phys_addr_t paddr)
> >> +{
> >> + return dma_addr_encrypted(__phys_to_dma(dev, paddr));
> >> +}
> >> /*
> >> * If memory encryption is supported, phys_to_dma will set the memory encryption
> >> * bit in the DMA address, and dma_to_phys will clear it.
> >> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> >> index 3dae0f592063..b3fa3c6e0169 100644
> >> --- a/include/linux/swiotlb.h
> >> +++ b/include/linux/swiotlb.h
> >> @@ -81,6 +81,7 @@ struct io_tlb_pool {
> >> struct list_head node;
> >> struct rcu_head rcu;
> >> bool transient;
> >> + bool unencrypted;
> >> #endif
> >> };
> >>
> >> @@ -111,6 +112,7 @@ struct io_tlb_mem {
> >> struct dentry *debugfs;
> >> bool force_bounce;
> >> bool for_alloc;
> >> + bool unencrypted;
> >> #ifdef CONFIG_SWIOTLB_DYNAMIC
> >> bool can_grow;
> >> u64 phys_limit;
> >> @@ -282,7 +284,8 @@ static inline void swiotlb_sync_single_for_cpu(struct device *dev,
> >> extern void swiotlb_print_info(void);
> >>
> >> #ifdef CONFIG_DMA_RESTRICTED_POOL
> >> -struct page *swiotlb_alloc(struct device *dev, size_t size);
> >> +struct page *swiotlb_alloc(struct device *dev, size_t size,
> >> + unsigned long attrs);
> >> bool swiotlb_free(struct device *dev, struct page *page, size_t size);
> >>
> >> static inline bool is_swiotlb_for_alloc(struct device *dev)
> >> @@ -290,7 +293,8 @@ static inline bool is_swiotlb_for_alloc(struct device *dev)
> >> return dev->dma_io_tlb_mem->for_alloc;
> >> }
> >> #else
> >> -static inline struct page *swiotlb_alloc(struct device *dev, size_t size)
> >> +static inline struct page *swiotlb_alloc(struct device *dev, size_t size,
> >> + unsigned long attrs)
> >> {
> >> return NULL;
> >> }
> >> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> >> index dc2907439b3d..97ae4fa10521 100644
> >> --- a/kernel/dma/direct.c
> >> +++ b/kernel/dma/direct.c
> >> @@ -104,9 +104,10 @@ static void __dma_direct_free_pages(struct device *dev, struct page *page,
> >> dma_free_contiguous(dev, page, size);
> >> }
> >>
> >> -static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
> >> +static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size,
> >> + unsigned long attrs)
> >> {
> >> - struct page *page = swiotlb_alloc(dev, size);
> >> + struct page *page = swiotlb_alloc(dev, size, attrs);
> >>
> >> if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
> >> swiotlb_free(dev, page, size);
> >> @@ -266,8 +267,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >> gfp, attrs);
> >>
> >> if (is_swiotlb_for_alloc(dev)) {
> >> - page = dma_direct_alloc_swiotlb(dev, size);
> >> + page = dma_direct_alloc_swiotlb(dev, size, attrs);
> >> if (page) {
> >> + /*
> >> + * swiotlb allocations comes from pool already marked
> >> + * decrypted
> >> + */
> >> mark_mem_decrypt = false;
> >> goto setup_page;
> >> }
> >> @@ -374,6 +379,7 @@ void dma_direct_free(struct device *dev, size_t size,
> >> return;
> >>
> >> if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
> >> + /* Swiotlb doesn't need a page attribute update on free */
> >> mark_mem_encrypted = false;
> >>
> >> if (is_vmalloc_addr(cpu_addr)) {
> >> @@ -403,7 +409,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
> >> gfp, attrs);
> >>
> >> if (is_swiotlb_for_alloc(dev)) {
> >> - page = dma_direct_alloc_swiotlb(dev, size);
> >> + page = dma_direct_alloc_swiotlb(dev, size, attrs);
> >> if (!page)
> >> return NULL;
> >>
> >> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> >> index ab4eccbaa076..065663be282c 100644
> >> --- a/kernel/dma/swiotlb.c
> >> +++ b/kernel/dma/swiotlb.c
> >> @@ -259,10 +259,21 @@ void __init swiotlb_update_mem_attributes(void)
> >> struct io_tlb_pool *mem = &io_tlb_default_mem.defpool;
> >> unsigned long bytes;
> >>
> >> + /*
> >> + * if platform support memory encryption, swiotlb buffers are
> >> + * decrypted by default.
> >> + */
> >> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> >> + io_tlb_default_mem.unencrypted = true;
> >> + else
> >> + io_tlb_default_mem.unencrypted = false;
> >> +
> >> if (!mem->nslabs || mem->late_alloc)
> >> return;
> >> bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
> >> - set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
> >> +
> >> + if (io_tlb_default_mem.unencrypted)
> >> + set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
> >> }
> >>
> >> static void swiotlb_init_io_tlb_pool(struct io_tlb_pool *mem, phys_addr_t start,
> >> @@ -505,8 +516,10 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
> >> if (!mem->slots)
> >> goto error_slots;
> >>
> >> - set_memory_decrypted((unsigned long)vstart,
> >> - (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
> >> + if (io_tlb_default_mem.unencrypted)
> >> + set_memory_decrypted((unsigned long)vstart,
> >> + (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
> >> +
> >> swiotlb_init_io_tlb_pool(mem, virt_to_phys(vstart), nslabs, true,
> >> nareas);
> >> add_mem_pool(&io_tlb_default_mem, mem);
> >> @@ -539,7 +552,9 @@ void __init swiotlb_exit(void)
> >> tbl_size = PAGE_ALIGN(mem->end - mem->start);
> >> slots_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), mem->nslabs));
> >>
> >> - set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
> >> + if (io_tlb_default_mem.unencrypted)
> >> + set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
> >> +
> >> if (mem->late_alloc) {
> >> area_order = get_order(array_size(sizeof(*mem->areas),
> >> mem->nareas));
> >> @@ -563,6 +578,7 @@ void __init swiotlb_exit(void)
> >> * @gfp: GFP flags for the allocation.
> >> * @bytes: Size of the buffer.
> >> * @phys_limit: Maximum allowed physical address of the buffer.
> >> + * @unencrypted: true to allocate unencrypted memory, false for encrypted memory
> >> *
> >> * Allocate pages from the buddy allocator. If successful, make the allocated
> >> * pages decrypted that they can be used for DMA.
> >> @@ -570,7 +586,8 @@ void __init swiotlb_exit(void)
> >> * Return: Decrypted pages, %NULL on allocation failure, or ERR_PTR(-EAGAIN)
> >> * if the allocated physical address was above @phys_limit.
> >> */
> >> -static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
> >> +static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes,
> >> + u64 phys_limit, bool unencrypted)
> >> {
> >> unsigned int order = get_order(bytes);
> >> struct page *page;
> >> @@ -588,13 +605,13 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
> >> }
> >>
> >> vaddr = phys_to_virt(paddr);
> >> - if (set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
> >> + if (unencrypted && set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
> >> goto error;
> >> return page;
> >>
> >> error:
> >> /* Intentional leak if pages cannot be encrypted again. */
> >> - if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
> >> + if (unencrypted && !set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
> >> __free_pages(page, order);
> >> return NULL;
> >> }
> >> @@ -604,30 +621,26 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
> >> * @dev: Device for which a memory pool is allocated.
> >> * @bytes: Size of the buffer.
> >> * @phys_limit: Maximum allowed physical address of the buffer.
> >> + * @attrs: DMA attributes for the allocation.
> >> * @gfp: GFP flags for the allocation.
> >> *
> >> * Return: Allocated pages, or %NULL on allocation failure.
> >> */
> >> static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
> >> - u64 phys_limit, gfp_t gfp)
> >> + u64 phys_limit, unsigned long attrs, gfp_t gfp)
> >> {
> >> struct page *page;
> >> - unsigned long attrs = 0;
> >>
> >> /*
> >> * Allocate from the atomic pools if memory is encrypted and
> >> * the allocation is atomic, because decrypting may block.
> >> */
> >> - if (!gfpflags_allow_blocking(gfp) && dev && force_dma_unencrypted(dev)) {
> >> + if (!gfpflags_allow_blocking(gfp) && (attrs & DMA_ATTR_CC_SHARED)) {
> >> void *vaddr;
> >>
> >> if (!IS_ENABLED(CONFIG_DMA_COHERENT_POOL))
> >> return NULL;
> >>
> >> - /* swiotlb considered decrypted by default */
> >> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> >> - attrs = DMA_ATTR_CC_SHARED;
> >> -
> >> return dma_alloc_from_pool(dev, bytes, &vaddr, gfp,
> >> attrs, dma_coherent_ok);
> >> }
> >> @@ -638,7 +651,8 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
> >> else if (phys_limit <= DMA_BIT_MASK(32))
> >> gfp |= __GFP_DMA32;
> >>
> >> - while (IS_ERR(page = alloc_dma_pages(gfp, bytes, phys_limit))) {
> >> + while (IS_ERR(page = alloc_dma_pages(gfp, bytes, phys_limit,
> >> + !!(attrs & DMA_ATTR_CC_SHARED)))) {
> >> if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
> >> phys_limit < DMA_BIT_MASK(64) &&
> >> !(gfp & (__GFP_DMA32 | __GFP_DMA)))
> >> @@ -657,15 +671,18 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
> >> * swiotlb_free_tlb() - free a dynamically allocated IO TLB buffer
> >> * @vaddr: Virtual address of the buffer.
> >> * @bytes: Size of the buffer.
> >> + * @unencrypted: true if @vaddr was allocated decrypted and must be
> >> + * re-encrypted before being freed
> >> */
> >> -static void swiotlb_free_tlb(void *vaddr, size_t bytes)
> >> +static void swiotlb_free_tlb(void *vaddr, size_t bytes, bool unencrypted)
> >> {
> >> if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
> >> dma_free_from_pool(NULL, vaddr, bytes))
> >> return;
> >>
> >> /* Intentional leak if pages cannot be encrypted again. */
> >> - if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
> >> + if (!unencrypted ||
> >> + !set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
> >> __free_pages(virt_to_page(vaddr), get_order(bytes));
> >> }
> >>
> >> @@ -676,6 +693,7 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
> >> * @nslabs: Desired (maximum) number of slabs.
> >> * @nareas: Number of areas.
> >> * @phys_limit: Maximum DMA buffer physical address.
> >> + * @attrs: DMA attributes for the allocation.
> >> * @gfp: GFP flags for the allocations.
> >> *
> >> * Allocate and initialize a new IO TLB memory pool. The actual number of
> >> @@ -686,7 +704,8 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
> >> */
> >> static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
> >> unsigned long minslabs, unsigned long nslabs,
> >> - unsigned int nareas, u64 phys_limit, gfp_t gfp)
> >> + unsigned int nareas, u64 phys_limit, unsigned long attrs,
> >> + gfp_t gfp)
> >> {
> >> struct io_tlb_pool *pool;
> >> unsigned int slot_order;
> >> @@ -704,9 +723,10 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
> >> if (!pool)
> >> goto error;
> >> pool->areas = (void *)pool + sizeof(*pool);
> >> + pool->unencrypted = !!(attrs & DMA_ATTR_CC_SHARED);
> >>
> >> tlb_size = nslabs << IO_TLB_SHIFT;
> >> - while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, gfp))) {
> >> + while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, attrs, gfp))) {
> >> if (nslabs <= minslabs)
> >> goto error_tlb;
> >> nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
> >> @@ -724,7 +744,8 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
> >> return pool;
> >>
> >> error_slots:
> >> - swiotlb_free_tlb(page_address(tlb), tlb_size);
> >> + swiotlb_free_tlb(page_address(tlb), tlb_size,
> >> + !!(attrs & DMA_ATTR_CC_SHARED));
> >> error_tlb:
> >> kfree(pool);
> >> error:
> >> @@ -742,7 +763,9 @@ static void swiotlb_dyn_alloc(struct work_struct *work)
> >> struct io_tlb_pool *pool;
> >>
> >> pool = swiotlb_alloc_pool(NULL, IO_TLB_MIN_SLABS, default_nslabs,
> >> - default_nareas, mem->phys_limit, GFP_KERNEL);
> >> + default_nareas, mem->phys_limit,
> >> + mem->unencrypted ? DMA_ATTR_CC_SHARED : 0,
> >> + GFP_KERNEL);
> >> if (!pool) {
> >> pr_warn_ratelimited("Failed to allocate new pool");
> >> return;
> >> @@ -762,7 +785,7 @@ static void swiotlb_dyn_free(struct rcu_head *rcu)
> >> size_t tlb_size = pool->end - pool->start;
> >>
> >> free_pages((unsigned long)pool->slots, get_order(slots_size));
> >> - swiotlb_free_tlb(pool->vaddr, tlb_size);
> >> + swiotlb_free_tlb(pool->vaddr, tlb_size, pool->unencrypted);
> >> kfree(pool);
> >> }
> >>
> >> @@ -1232,6 +1255,7 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
> >> nslabs = nr_slots(alloc_size);
> >> phys_limit = min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
> >> pool = swiotlb_alloc_pool(dev, nslabs, nslabs, 1, phys_limit,
> >> + mem->unencrypted ? DMA_ATTR_CC_SHARED : 0,
> >> GFP_NOWAIT);
> >> if (!pool)
> >> return -1;
> >> @@ -1394,6 +1418,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
> >> enum dma_data_direction dir, unsigned long attrs)
> >> {
> >> struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> >> + bool require_decrypted = false;
> >> unsigned int offset;
> >> struct io_tlb_pool *pool;
> >> unsigned int i;
> >> @@ -1411,6 +1436,16 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
> >> if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> >> pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
> >>
> >> + /*
> >> + * if we are trying to swiotlb map a decrypted paddr or the paddr is encrypted
> >> + * but the device is forcing decryption, use decrypted io_tlb_mem
> >> + */
> >> + if ((attrs & DMA_ATTR_CC_SHARED) || force_dma_unencrypted(dev))
> >> + require_decrypted = true;
> >> +
> >> + if (require_decrypted != mem->unencrypted)
> >> + return (phys_addr_t)DMA_MAPPING_ERROR;
> >> +
> >> /*
> >> * The default swiotlb memory pool is allocated with PAGE_SIZE
> >> * alignment. If a mapping is requested with larger alignment,
> >> @@ -1608,8 +1643,14 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
> >> if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
> >> return DMA_MAPPING_ERROR;
> >>
> >> - /* Ensure that the address returned is DMA'ble */
> >> - dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
> >> + /*
> >> + * Use the allocated io_tlb_mem encryption type to determine dma addr.
> >> + */
> >> + if (dev->dma_io_tlb_mem->unencrypted)
> >> + dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
> >> + else
> >> + dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
> >> +
> >> if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
> >> __swiotlb_tbl_unmap_single(dev, swiotlb_addr, size, dir,
> >> attrs | DMA_ATTR_SKIP_CPU_SYNC,
> >> @@ -1773,7 +1814,8 @@ static inline void swiotlb_create_debugfs_files(struct io_tlb_mem *mem,
> >>
> >> #ifdef CONFIG_DMA_RESTRICTED_POOL
> >>
> >> -struct page *swiotlb_alloc(struct device *dev, size_t size)
> >> +struct page *swiotlb_alloc(struct device *dev, size_t size,
> >> + unsigned long attrs)
> >> {
> >> struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> >> struct io_tlb_pool *pool;
> >> @@ -1784,6 +1826,9 @@ struct page *swiotlb_alloc(struct device *dev, size_t size)
> >> if (!mem)
> >> return NULL;
> >>
> >> + if (mem->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
> >> + return NULL;
> >> +
> >> align = (1 << (get_order(size) + PAGE_SHIFT)) - 1;
> >> index = swiotlb_find_slots(dev, 0, size, align, &pool);
> >> if (index == -1)
> >> @@ -1853,9 +1898,18 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> >> kfree(mem);
> >> return -ENOMEM;
> >> }
> >> + /*
> >> + * if platform supports memory encryption,
> >> + * restricted mem pool is decrypted by default
> >> + */
> >> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> >> + mem->unencrypted = true;
> >> + set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
> >> + rmem->size >> PAGE_SHIFT);
> >> + } else {
> >> + mem->unencrypted = false;
> >> + }
> >
> > This breaks pKVM as it doesn’t set CC_ATTR_MEM_ENCRYPT, so all virtio
> > traffic now fails.
> >
> > Also, by design, some drivers are clueless about bouncing, so
> > I believe that the pool should have a way to control it’s property
> > (encrypted or decrypted) and that takes priority over whatever
> > attributes comes from allocation.
> > And that brings us to the same point whether it’s better to return
> > the memory along with it’s state or we pass the requested state.
> > I think for other cases it’s fine for the device/DMA-API to dictate
> > the attrs, but not in restricted-dma case, the firmware just knows better.
> >
>
> Is it that the pKVM guest kernel does not have awareness of
> encrypted/decrypted DMA allocations? Instead, the firmware attaches
> hypervisor-shared pages to the device via restricted-dma-pool? The
> kernel then has swiotlb->for_alloc = true, and hence all DMA allocations
> go through the restricted-dma-pool?
Yes.
>
> Given that pKVM supports pkvm_set_memory_encrypted() and
> pkvm_set_memory_decrypted(), can we consider adding CC_ATTR_MEM_ENCRYPT
> support to pKVM? It would also be good to investigate whether we can set
> force_dma_unencrypted(dev) to true where needed.
I was looking in to that, but it didn't work because
force_dma_unencrypted() is broken with restricted-dma due to the
double decryption issue, that's when I sent my first series [1]
May be we should land some basic fixes for that path so we can
convert pKVM, then we do the full rework.
I will revive my old work and see if I can send a RFC.
[1] https://lore.kernel.org/all/20260305170335.963568-1-smostafa@google.com/
>
> I agree that this patch, as it stands, can break pKVM because we are now
> missing the set_memory_decrypted() call required for pKVM to work.
>
> We now mark the swiotlb io_tlb_mem as unencrypted/encrypted in the guest
> using struct io_tlb_mem->unencrypted. I am not clear what we can use for
> pKVM to conditionalize this so that it works for both protected and
> unprotected guests.
There is no problem with non-protected guests as they don't use memory
encryption, my initial thought was that th encrpyted/decrypted is
per-pool property which is decided by FW (device-tree).
Thanks,
Mostafa
>
> -aneesh
>
^ permalink raw reply
* Re: [PATCH v2 5/8] dt-bindings: display: allwinner: Add DE33 planes
From: Krzysztof Kozlowski @ 2026-05-14 12:01 UTC (permalink / raw)
To: Jernej Skrabec
Cc: wens, samuel, mripard, maarten.lankhorst, tzimmermann, airlied,
simona, robh, krzk+dt, conor+dt, mturquette, sboyd, dri-devel,
devicetree, linux-arm-kernel, linux-sunxi, linux-kernel,
linux-clk
In-Reply-To: <20260509190015.79086-6-jernej.skrabec@siol.net>
On Sat, May 09, 2026 at 09:00:12PM +0200, Jernej Skrabec wrote:
> From: Jernej Skrabec <jernej.skrabec@gmail.com>
>
> Allwinner Display Engine 3.3 contains planes, which are shared resources
> between all mixers present in SoC. They can be assigned to specific
> mixer by using registers which reside in display clocks MMIO.
>
> Add a binding for them.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
> Changes from v1:
> - dropped | for description
> - better description for allwinner,plane-mapping
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 1/1] dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for MT8188
From: Arnab Layek @ 2026-05-14 11:45 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
Cc: arnab.layek, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, andersson, mathieu.poirier,
linux-remoteproc, Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260514114533.174008-1-arnab.layek@mediatek.com>
The MT8188 SCP requires support for 1-2 reserved memory regions, while
other MediaTek SoCs use only a single memory region.
The schema uses a permissive base with restrictive conditionals:
1) Base schema allows all devices minItems: 1, maxItems: 2
2) Non-MT8188 devices (mt8183, mt8186, mt8192, mt8195, mt8195-dual) are
restricted to maxItems: 1, overriding the base
3) MT8188 devices (mt8188, mt8188-dual) set minItems: 1 with item
descriptions, inheriting maxItems: 2 from base, making the second
L1TCM region optional
This follows the same pattern as other MediaTek dt-bindings such as
mediatek,jpeg-encoder.yaml which uses conditional schemas to support
different numbers of iommus per device variant.
Signed-off-by: Arnab Layek <arnab.layek@mediatek.com>
---
.../bindings/remoteproc/mtk,scp.yaml | 45 ++++++++++++++++++-
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index bdbb12118da4..fca9b0675eae 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -55,7 +55,8 @@ properties:
initializing SCP.
memory-region:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
cros-ec-rpmsg:
$ref: /schemas/embedded-controller/google,cros-ec.yaml
@@ -123,7 +124,8 @@ patternProperties:
initializing sub cores of multi-core SCP.
memory-region:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
cros-ec-rpmsg:
$ref: /schemas/embedded-controller/google,cros-ec.yaml
@@ -205,6 +207,45 @@ allOf:
items:
- const: cfg
- const: l1tcm
+ - if:
+ properties:
+ compatible:
+ enum:
+ - mediatek,mt8183-scp
+ - mediatek,mt8186-scp
+ - mediatek,mt8192-scp
+ - mediatek,mt8195-scp
+ - mediatek,mt8195-scp-dual
+ then:
+ properties:
+ memory-region:
+ maxItems: 1
+ patternProperties:
+ "^scp@[a-f0-9]+$":
+ properties:
+ memory-region:
+ maxItems: 1
+ - if:
+ properties:
+ compatible:
+ enum:
+ - mediatek,mt8188-scp
+ - mediatek,mt8188-scp-dual
+ then:
+ properties:
+ memory-region:
+ minItems: 1
+ items:
+ - description: Main SCP SRAM memory region
+ - description: Optional SCP L1TCM memory region
+ patternProperties:
+ "^scp@[a-f0-9]+$":
+ properties:
+ memory-region:
+ minItems: 1
+ items:
+ - description: Main SCP SRAM memory region
+ - description: Optional SCP L1TCM memory region
additionalProperties: false
--
2.45.2
^ permalink raw reply related
* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Mostafa Saleh @ 2026-05-14 11:48 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
Jiri Pirko, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <20260513172450.GR7702@ziepe.ca>
On Wed, May 13, 2026 at 02:24:50PM -0300, Jason Gunthorpe wrote:
> On Wed, May 13, 2026 at 02:27:14PM +0000, Mostafa Saleh wrote:
>
> > > + /*
> > > + * if platform supports memory encryption,
> > > + * restricted mem pool is decrypted by default
> > > + */
> > > + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> > > + mem->unencrypted = true;
> > > + set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
> > > + rmem->size >> PAGE_SHIFT);
> > > + } else {
> > > + mem->unencrypted = false;
> > > + }
> >
> > This breaks pKVM as it doesn’t set CC_ATTR_MEM_ENCRYPT, so all virtio
> > traffic now fails.
>
> How will pKVM signal what kind of memory the DMA needs then?
>
> Does it use set_memory_decrypted()? How can it use
> set_memory_decrypted() without offering CC_ATTR_MEM_ENCRYPT ?
pKVM (hypervisor) doesn’t signal anything.
The VMM when running protected guests will use restricted dma-pools
for emulated vritio devices in the guest, which gets decrypted by
the guest kernel and hence shared with the host kernel, and then
traffic is bounced via the pool.
It’s also worth noting that bouncing here isn't just about visibility.
Because memory sharing operates at page granularity, bouncing sub-page
allocations through the restricted pool prevents adjacent, sensitive
guest data from being exposed to the untrusted host.
>
> > Also, by design, some drivers are clueless about bouncing, so
>
> Oh? What does this mean? We take quite a dim view of drivers mis-using
> the DMA API..
Maybe clueless is not the right word, I mean when virtio drivers use
the DMA API they don’t know whether it’s going to bounce or not as
that is decided by dma-direct (and in other cases by dma-iommu,
but not for pKVM).
>
> > I believe that the pool should have a way to control it’s property
> > (encrypted or decrypted) and that takes priority over whatever
> > attributes comes from allocation.
>
> We should get here because dma_capable() fails, and then swiotlb needs
> to return something that makes dma_capable() succeed. Yes, it should
> return details about the thing it decided, but it shouldn't have been
> pre-created with some idea how to make dma_capable() work.
That sounds neat, but at the end we have force_dma_unencrypted() in
dma_capable() which is just hardcoded to true/false by the platform.
How is that different from having the state static by the pool?
>
> If dma_capable() can fail, then swiotlb should know exactly what to do
> to fix it.
dma_capable() returns a bool, I don’t think it can know what exactly
went wrong (based on address, size, attrs, dev...)
>
> If pkvm wants to use the hacky scheme where you force a swiotlb pool
> configuration during arch init with force swiotlb that's a somewhat
> different flow and, sure the forced pool should force do whatever it
> is forced to.
>
> But lets try to keep them seperated in the discussion..
While we can debate the aesthetics of the setup , this is
the exisitng behaviour for Linux, which existed for years
and pKVM relies on and is used extensively.
And, this patch alters that long-standing logic and introduces
a functional regression.
We can address this by either adjusting this patch or by changing
pKVM guests to be more aligned with other CCA guests which is
something I have been wondering about if it would help reduce
bouncing.
>
> > And that brings us to the same point whether it’s better to return
> > the memory along with it’s state or we pass the requested state.
> > I think for other cases it’s fine for the device/DMA-API to dictate
> > the attrs, but not in restricted-dma case, the firmware just knows better.
>
> The memory type must be returned back at some level so downstream
> things can do the right transformation of the phys_addr_t.
Agreed, I believe that will be needed at least for
SWIOTLB/restricted-dma -> dma-API interactions.
>
> One of the aspirational CC things that should work is a T=1 device
> tries to DMA from a decrypted page, finds the address is above the dma
> limit of the device, so it bounces it with SWIOTLB to an encrypted low
> address page and then the DMA API internal flow switiches from working
> with decrypted to encrypted phys_addr_t.
>
> If we can make that work then maybe the flows are designed correctly.
Mmm, I am not sure I understand this one, shouldn’t the device also be
notified about the switch in memory state, if it expects to read/write
decrypted memory, how would that work if the kernel changes it to an
encrypted one?
Thanks,
Mostafa
>
> Jason
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: arm: ti: Add am62l3-beaglebadge
From: Krzysztof Kozlowski @ 2026-05-14 11:45 UTC (permalink / raw)
To: Judith Mendez
Cc: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel, Andrew Davis, Bryan Brattlof
In-Reply-To: <20260513233447.2713737-2-jm@ti.com>
On Wed, May 13, 2026 at 06:34:45PM -0500, Judith Mendez wrote:
> This board is based on ti,am62l3.
>
> https://beagleboard.org
Drop, it's just vendor website.
> https://github.com/beagleboard/BeagleBadge
Missing blank line. Please follow standard patch format.
Maybe that was supposed to be tag?
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 0/1] dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for MT8188
From: Arnab Layek @ 2026-05-14 11:45 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
Cc: arnab.layek, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, andersson, mathieu.poirier,
linux-remoteproc, Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260506133157.3283204-1-arnab.layek@mediatek.com>
This series updates the mtk,scp dt-binding schema to support MT8188's
requirement for two memory regions while maintaining backward
compatibility for other MediaTek SoCs.
The schema uses a permissive base with restrictive conditionals:
1) Base schema allows all devices minItems: 1, maxItems: 2
2) Non-MT8188 devices (mt8183, mt8186, mt8192, mt8195, mt8195-dual) are
restricted to maxItems: 1, overriding the base
3) MT8188 devices (mt8188, mt8188-dual) set minItems: 1 with item
descriptions, inheriting maxItems: 2 from base, making the second
L1TCM region optional
Changes in v3:
- Removed "Tested on..." line per Krzysztof's feedback (bindings cannot be tested)
- Added minItems: 1 to MT8188 conditional to make L1TCM region truly optional
- Clarified commit message to specifically reference mediatek,jpeg-encoder.yaml pattern
- Restructured schema per Conor's feedback: base allows maxItems: 2, conditionals restrict
- Added explicit restrictions for non-MT8188 devices (maxItems: 1)
- Added technical explanation of the permissive base + restrictive conditionals pattern
Changes in v2:
- Added conditional schema for MT8188 to allow 1-2 memory regions
- Added descriptions for each memory region
- Did not work: base maxItems: 1 conflicted with conditional trying to allow 2
Arnab Layek (1):
dt-bindings: remoteproc: mtk,scp: Allow multiple memory regions for
MT8188
.../bindings/remoteproc/mtk,scp.yaml | 45 ++++++++++++++++++-
1 file changed, 43 insertions(+), 2 deletions(-)
--
2.45.2
^ permalink raw reply
* Re: [PATCH v5 2/6] mfd: Add Rockchip mfpwm driver
From: Lee Jones @ 2026-05-14 11:41 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, William Breathitt Gray, Damon Ding,
kernel, Jonas Karlman, Alexey Charkov, linux-rockchip, linux-pwm,
devicetree, linux-arm-kernel, linux-kernel, linux-iio
In-Reply-To: <20260420-rk3576-pwm-v5-2-ae7cfbbe5427@collabora.com>
On Mon, 20 Apr 2026, Nicolas Frattaroli wrote:
> With the Rockchip RK3576, the PWM IP used by Rockchip has changed
> substantially. Looking at both the downstream pwm-rockchip driver as
> well as the mainline pwm-rockchip driver made it clear that with all its
> additional features and its differences from previous IP revisions, it
> is best supported in a new driver.
>
> This brings us to the question as to what such a new driver should be.
> To me, it soon became clear that it should actually be several new
> drivers, most prominently when Uwe Kleine-König let me know that I
> should not implement the pwm subsystem's capture callback, but instead
> write a counter driver for this functionality.
>
> Combined with the other as-of-yet unimplemented functionality of this
> new IP, it became apparent that it needs to be spread across several
> subsystems.
>
> For this reason, we add a new MFD core driver, called mfpwm (short for
> "Multi-function PWM"). This "parent" driver makes sure that only one
> device function driver is using the device at a time, and is in charge
> of registering the MFD cell devices for the individual device functions
> offered by the device.
>
> An acquire/release pattern is used to guarantee that device function
> drivers don't step on each other's toes.
The whys, whos and wherefors should not be included in the commit
message. We want to know what you're trying to achieve, why you're
trying to achieve it and how you're going about it. This should be
purely technical. Leave all of the conversation history out of it.
I'll be honest. All of this bespoke acquisition handling is freaking me
out. It's almost certainly not going to accepted like this, but in
order to help suggest an alternative I need to understand exactly what
the specifications are.
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> MAINTAINERS | 2 +
> drivers/mfd/Kconfig | 16 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rockchip-mfpwm.c | 357 ++++++++++++++++++++++++++++
> include/linux/mfd/rockchip-mfpwm.h | 470 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 846 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 86f20cb563c6..d52731242a33 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23178,6 +23178,8 @@ L: linux-rockchip@lists.infradead.org
> L: linux-pwm@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/pwm/rockchip,rk3576-pwm.yaml
> +F: drivers/mfd/rockchip-mfpwm.c
> +F: include/linux/mfd/rockchip-mfpwm.h
>
> ROCKCHIP RK3568 RANDOM NUMBER GENERATOR SUPPORT
> M: Daniel Golle <daniel@makrotopia.org>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7192c9d1d268..80b4e82c4937 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1378,6 +1378,22 @@ config MFD_RC5T583
> Additional drivers must be enabled in order to use the
> different functionality of the device.
>
> +config MFD_ROCKCHIP_MFPWM
> + tristate "Rockchip multi-function PWM controller"
> + depends on ARCH_ROCKCHIP || COMPILE_TEST
> + depends on OF
> + depends on HAS_IOMEM
> + depends on COMMON_CLK
> + select MFD_CORE
> + help
> + Some Rockchip SoCs, such as the RK3576, use a PWM controller that has
> + several different functions, such as generating PWM waveforms but also
> + counting waveforms.
> +
> + This driver manages the overall device, and selects between different
> + functionalities at runtime as needed. Drivers for them are implemented
> + in their respective subsystems.
> +
> config MFD_RK8XX
> tristate
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28a..ebadbaea9e4a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -231,6 +231,7 @@ obj-$(CONFIG_MFD_PALMAS) += palmas.o
> obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o
> obj-$(CONFIG_MFD_NTXEC) += ntxec.o
> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
> +obj-$(CONFIG_MFD_ROCKCHIP_MFPWM) += rockchip-mfpwm.o
> obj-$(CONFIG_MFD_RK8XX) += rk8xx-core.o
> obj-$(CONFIG_MFD_RK8XX_I2C) += rk8xx-i2c.o
> obj-$(CONFIG_MFD_RK8XX_SPI) += rk8xx-spi.o
> diff --git a/drivers/mfd/rockchip-mfpwm.c b/drivers/mfd/rockchip-mfpwm.c
> new file mode 100644
> index 000000000000..72d04982b961
> --- /dev/null
> +++ b/drivers/mfd/rockchip-mfpwm.c
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Collabora Ltd.
> + *
> + * A driver to manage all the different functionalities exposed by Rockchip's
> + * PWMv4 hardware.
> + *
> + * This driver is chiefly focused on guaranteeing non-concurrent operation
> + * between the different device functions, as well as setting the clocks.
> + * It registers the device function platform devices, e.g. PWM output or
> + * PWM capture.
> + *
> + * Authors:
> + * Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/rockchip-mfpwm.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +/**
> + * struct rockchip_mfpwm - private mfpwm driver instance state struct
> + * @pdev: pointer to this instance's &struct platform_device
> + * @base: pointer to the memory mapped registers of this device
> + * @pwm_clk: pointer to the PLL clock the PWM signal may be derived from
> + * @osc_clk: pointer to the fixed crystal the PWM signal may be derived from
> + * @rc_clk: pointer to the RC oscillator the PWM signal may be derived from
> + * @chosen_clk: a clk-mux of pwm_clk, osc_clk and rc_clk
> + * @pclk: pointer to the APB bus clock needed for mmio register access
> + * @active_func: pointer to the currently active device function, or %NULL if no
> + * device function is currently actively using any of the shared
> + * resources. May only be checked/modified with @state_lock held.
> + * @acquire_cnt: number of times @active_func has currently mfpwm_acquire()'d
> + * it. Must only be checked or modified while holding @state_lock.
> + * @state_lock: this lock is held while either the active device function, the
> + * enable register, or the chosen clock is being changed.
> + * @irq: the IRQ number of this device
> + */
> +struct rockchip_mfpwm {
> + struct platform_device *pdev;
It's more common to store 'struct device *'.
> + void __iomem *base;
> + struct clk *pwm_clk;
> + struct clk *osc_clk;
> + struct clk *rc_clk;
> + struct clk *chosen_clk;
> + struct clk *pclk;
> + struct rockchip_mfpwm_func *active_func;
> + unsigned int acquire_cnt;
> + spinlock_t state_lock;
> + int irq;
> +};
> +
> +static atomic_t subdev_id = ATOMIC_INIT(0);
> +
> +static inline struct rockchip_mfpwm *to_rockchip_mfpwm(struct platform_device *pdev)
> +{
> + return platform_get_drvdata(pdev);
> +}
No pointless abstractions please. Just use the call directly.
> +
> +static int mfpwm_check_pwmf(const struct rockchip_mfpwm_func *pwmf,
> + const char *fname)
> +{
> + struct device *dev = &pwmf->parent->pdev->dev;
> +
> + if (IS_ERR_OR_NULL(pwmf)) {
> + dev_warn(dev, "called %s with an erroneous handle, no effect\n",
> + fname);
> + return -EINVAL;
> + }
> +
> + if (IS_ERR_OR_NULL(pwmf->parent)) {
> + dev_warn(dev, "called %s with an erroneous mfpwm_func parent, no effect\n",
> + fname);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +__attribute__((nonnull))
> +static int mfpwm_do_acquire(struct rockchip_mfpwm_func *pwmf)
> +{
> + struct rockchip_mfpwm *mfpwm = pwmf->parent;
> + unsigned int cnt;
> +
> + if (mfpwm->active_func && pwmf->id != mfpwm->active_func->id)
Comments throughout please.
> + return -EBUSY;
> +
> + if (!mfpwm->active_func)
> + mfpwm->active_func = pwmf;
> +
> + if (!check_add_overflow(mfpwm->acquire_cnt, 1, &cnt)) {
> + mfpwm->acquire_cnt = cnt;
> + } else {
> + dev_warn(&mfpwm->pdev->dev, "prevented acquire counter overflow in %s\n",
> + __func__);
__func__s are not user friendly. The user does not care about internals.
Keep them in your local BSP if you need them.
> + return -EOVERFLOW;
How many are you planning to allow?
> + }
> +
> + dev_dbg(&mfpwm->pdev->dev, "%d acquired mfpwm, acquires now at %u\n",
> + pwmf->id, mfpwm->acquire_cnt);
Drop the debug prints when upstreaming.
> +
> + return clk_enable(mfpwm->pclk);
> +}
> +
> +int mfpwm_acquire(struct rockchip_mfpwm_func *pwmf)
> +{
> + struct rockchip_mfpwm *mfpwm;
> + unsigned long flags;
> + int ret = 0;
> +
> + ret = mfpwm_check_pwmf(pwmf, "mfpwm_acquire");
> + if (ret)
> + return ret;
> +
> + mfpwm = pwmf->parent;
> + dev_dbg(&mfpwm->pdev->dev, "%d is attempting to acquire\n", pwmf->id);
> +
> + if (!spin_trylock_irqsave(&mfpwm->state_lock, flags))
> + return -EBUSY;
> +
> + ret = mfpwm_do_acquire(pwmf);
> +
> + spin_unlock_irqrestore(&mfpwm->state_lock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(mfpwm_acquire, "ROCKCHIP_MFPWM");
> +
> +__attribute__((nonnull))
> +static void mfpwm_do_release(const struct rockchip_mfpwm_func *pwmf)
> +{
> + struct rockchip_mfpwm *mfpwm = pwmf->parent;
> +
> + if (!mfpwm->active_func)
> + return;
> +
> + if (mfpwm->active_func->id != pwmf->id)
> + return;
> +
> + /*
> + * No need to check_sub_overflow here, !mfpwm->active_func above catches
> + * this type of problem already.
> + */
> + mfpwm->acquire_cnt--;
> +
> + if (!mfpwm->acquire_cnt)
> + mfpwm->active_func = NULL;
> +
> + clk_disable(mfpwm->pclk);
> +}
> +
> +void mfpwm_release(const struct rockchip_mfpwm_func *pwmf)
> +{
> + struct rockchip_mfpwm *mfpwm;
> + unsigned long flags;
> +
> + if (mfpwm_check_pwmf(pwmf, "mfpwm_release"))
> + return;
> +
> + mfpwm = pwmf->parent;
> +
> + spin_lock_irqsave(&mfpwm->state_lock, flags);
> + mfpwm_do_release(pwmf);
> + dev_dbg(&mfpwm->pdev->dev, "%d released mfpwm, acquires now at %u\n",
> + pwmf->id, mfpwm->acquire_cnt);
> + spin_unlock_irqrestore(&mfpwm->state_lock, flags);
> +}
> +EXPORT_SYMBOL_NS_GPL(mfpwm_release, "ROCKCHIP_MFPWM");
> +
> +int mfpwm_get_mode(const struct rockchip_mfpwm_func *pwmf)
> +{
> + struct rockchip_mfpwm *mfpwm;
> + int ret;
> +
> + ret = mfpwm_check_pwmf(pwmf, "mfpwm_acquire");
> + if (ret)
> + return ret;
> +
> + mfpwm = pwmf->parent;
> +
> + guard(spinlock_irqsave)(&mfpwm->state_lock);
> +
> + if (!rockchip_pwm_v4_is_enabled(mfpwm_reg_read(mfpwm->base, PWMV4_REG_ENABLE)))
Don't embed function names like this.
> + return -1;
-1 is not a real error code.
> +
> + return mfpwm_reg_read(mfpwm->base, PWMV4_REG_CTRL) & PWMV4_MODE_MASK;
> +}
> +EXPORT_SYMBOL_NS_GPL(mfpwm_get_mode, "ROCKCHIP_MFPWM");
> +
> +/**
> + * mfpwm_register_subdev - register a single mfpwm_func
> + * @mfpwm: pointer to the parent &struct rockchip_mfpwm
> + * @name: sub-device name string
> + *
> + * Allocate a single &struct mfpwm_func, fill its members with appropriate data,
> + * and register a new mfd cell.
> + *
> + * Returns: 0 on success, negative errno on error
> + */
> +static int mfpwm_register_subdev(struct rockchip_mfpwm *mfpwm,
> + const char *name)
> +{
> + struct rockchip_mfpwm_func *func;
> + struct mfd_cell cell = {};
> +
> + func = devm_kzalloc(&mfpwm->pdev->dev, sizeof(*func), GFP_KERNEL);
> + if (IS_ERR(func))
> + return PTR_ERR(func);
> + func->irq = mfpwm->irq;
> + func->parent = mfpwm;
Suggest you use the 'struct device' hierarchy instead of hand rolling
your own.
> + func->id = atomic_inc_return(&subdev_id);
Why dosen't PLATFORM_DEVID_AUTO work for you?
> + func->base = mfpwm->base;
> + func->core = mfpwm->chosen_clk;
> + cell.name = name;
> + cell.platform_data = func;
> + cell.pdata_size = sizeof(*func);
> +
> + return devm_mfd_add_devices(&mfpwm->pdev->dev, func->id, &cell, 1, NULL,
> + 0, NULL);
> +}
> +
> +static int mfpwm_register_subdevs(struct rockchip_mfpwm *mfpwm)
> +{
> + int ret;
> +
> + ret = mfpwm_register_subdev(mfpwm, "rockchip-pwm-v4");
> + if (ret)
> + return ret;
> +
> + ret = mfpwm_register_subdev(mfpwm, "rockchip-pwm-capture");
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
Place all of your devices in static (const if they are immutable)
structs. Literally no one else does MFD registration like this - do not
reinvent the wheel.
> +static int rockchip_mfpwm_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct rockchip_mfpwm *mfpwm;
Could we use 'ddata' for the variable instance name instead of 'mfpwm'
to follow standard naming conventions for private data structures?
> + char *clk_mux_name;
> + const char *mux_p_names[3];
> + int ret = 0;
> +
> + mfpwm = devm_kzalloc(&pdev->dev, sizeof(*mfpwm), GFP_KERNEL);
> + if (IS_ERR(mfpwm))
> + return PTR_ERR(mfpwm);
> +
> + mfpwm->pdev = pdev;
> +
> + spin_lock_init(&mfpwm->state_lock);
> +
> + mfpwm->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(mfpwm->base))
> + return dev_err_probe(dev, PTR_ERR(mfpwm->base),
> + "failed to ioremap address\n");
Doesn't devm_platform_ioremap_resource() already have its own error messages?
> +
> + mfpwm->pclk = devm_clk_get_prepared(dev, "pclk");
> + if (IS_ERR(mfpwm->pclk))
> + return dev_err_probe(dev, PTR_ERR(mfpwm->pclk),
> + "couldn't get and prepare 'pclk' clock\n");
> +
> + mfpwm->irq = platform_get_irq(pdev, 0);
> + if (mfpwm->irq < 0)
> + return dev_err_probe(dev, mfpwm->irq, "couldn't get irq 0\n");
> +
> + mfpwm->pwm_clk = devm_clk_get_prepared(dev, "pwm");
> + if (IS_ERR(mfpwm->pwm_clk))
> + return dev_err_probe(dev, PTR_ERR(mfpwm->pwm_clk),
> + "couldn't get and prepare 'pwm' clock\n");
> +
> + mfpwm->osc_clk = devm_clk_get_prepared(dev, "osc");
> + if (IS_ERR(mfpwm->osc_clk))
> + return dev_err_probe(dev, PTR_ERR(mfpwm->osc_clk),
> + "couldn't get and prepare 'osc' clock\n");
> +
> + mfpwm->rc_clk = devm_clk_get_prepared(dev, "rc");
> + if (IS_ERR(mfpwm->rc_clk))
> + return dev_err_probe(dev, PTR_ERR(mfpwm->rc_clk),
> + "couldn't get and prepare 'rc' clock\n");
> +
I'd do these in a loop.
> + clk_mux_name = devm_kasprintf(dev, GFP_KERNEL, "%s_chosen", dev_name(dev));
> + if (!clk_mux_name)
> + return -ENOMEM;
> +
> + mux_p_names[0] = __clk_get_name(mfpwm->pwm_clk);
> + mux_p_names[1] = __clk_get_name(mfpwm->osc_clk);
> + mux_p_names[2] = __clk_get_name(mfpwm->rc_clk);
Didn't you already request these by name?
> + mfpwm->chosen_clk = clk_register_mux(dev, clk_mux_name, mux_p_names,
devm_clk_hw_register_mux()?
> + ARRAY_SIZE(mux_p_names),
> + CLK_SET_RATE_PARENT,
> + mfpwm->base + PWMV4_REG_CLK_CTRL,
> + PWMV4_CLK_SRC_SHIFT, PWMV4_CLK_SRC_WIDTH,
> + CLK_MUX_HIWORD_MASK, NULL);
> + ret = clk_prepare(mfpwm->chosen_clk);
> + if (ret) {
> + dev_err(dev, "failed to prepare PWM clock mux: %pe\n",
> + ERR_PTR(ret));
dev_err_probe()
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, mfpwm);
> +
> + ret = mfpwm_register_subdevs(mfpwm);
> + if (ret) {
> + dev_err(dev, "failed to register sub-devices: %pe\n",
> + ERR_PTR(ret));
* Should we use 'dev_err_probe()' for this error path as well to correctly
handle deferred probing?
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static void rockchip_mfpwm_remove(struct platform_device *pdev)
> +{
> + struct rockchip_mfpwm *mfpwm = to_rockchip_mfpwm(pdev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&mfpwm->state_lock, flags);
> +
> + if (mfpwm->chosen_clk) {
> + clk_unprepare(mfpwm->chosen_clk);
No devm_* version available?
> + clk_unregister_mux(mfpwm->chosen_clk);
> + }
> +
> + spin_unlock_irqrestore(&mfpwm->state_lock, flags);
> +}
> +
> +static const struct of_device_id rockchip_mfpwm_of_match[] = {
> + {
> + .compatible = "rockchip,rk3576-pwm",
> + },
Single line.
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_mfpwm_of_match);
> +
> +static struct platform_driver rockchip_mfpwm_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
Use the raw string instead - this makes debugging challenging.
> + .of_match_table = rockchip_mfpwm_of_match,
> + },
> + .probe = rockchip_mfpwm_probe,
> + .remove = rockchip_mfpwm_remove,
> +};
> +module_platform_driver(rockchip_mfpwm_driver);
> +
> +MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
> +MODULE_DESCRIPTION("Rockchip MFPWM Driver");
FWIW, I don't like the name.
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/rockchip-mfpwm.h b/include/linux/mfd/rockchip-mfpwm.h
How much of this file is applicable to the core driver?
> new file mode 100644
> index 000000000000..dbf1588a4382
> --- /dev/null
> +++ b/include/linux/mfd/rockchip-mfpwm.h
> @@ -0,0 +1,470 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2025 Collabora Ltd.
> + *
> + * Common header file for all the Rockchip Multi-function PWM controller
> + * drivers that are spread across subsystems.
> + *
> + * Authors:
> + * Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> + */
> +
> +#ifndef __SOC_ROCKCHIP_MFPWM_H__
> +#define __SOC_ROCKCHIP_MFPWM_H__
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/hw_bitfield.h>
> +#include <linux/io.h>
> +#include <linux/spinlock.h>
> +
> +struct rockchip_mfpwm;
> +
> +/**
> + * struct rockchip_mfpwm_func - struct representing a single function driver
> + *
> + * @id: unique id for this function driver instance
> + * @base: pointer to start of MMIO registers
> + * @parent: a pointer to the parent mfpwm struct
> + * @irq: the shared IRQ gotten from the parent mfpwm device
> + * @core: a pointer to the clk mux that drives this channel's PWM
> + */
> +struct rockchip_mfpwm_func {
> + int id;
> + void __iomem *base;
> + struct rockchip_mfpwm *parent;
> + int irq;
> + struct clk *core;
> +};
> +
> +/*
> + * PWMV4 Register Definitions
> + * --------------------------
> + *
> + * Attributes:
> + * RW - Read-Write
> + * RO - Read-Only
> + * WO - Write-Only
> + * W1T - Write high, Self-clearing
> + * W1C - Write high to clear interrupt
> + *
> + * Bit ranges to be understood with Verilog-like semantics,
> + * e.g. [03:00] is 4 bits: 0, 1, 2 and 3.
> + *
> + * All registers must be accessed with 32-bit width accesses only
> + */
> +
> +#define PWMV4_REG_VERSION 0x000
> +/*
> + * VERSION Register Description
> + * [31:24] RO | Hardware Major Version
> + * [23:16] RO | Hardware Minor Version
> + * [15:15] RO | Reserved
> + * [14:14] RO | Hardware supports biphasic counters
> + * [13:13] RO | Hardware supports filters
> + * [12:12] RO | Hardware supports waveform generation
> + * [11:11] RO | Hardware supports counter
> + * [10:10] RO | Hardware supports frequency metering
> + * [09:09] RO | Hardware supports power key functionality
> + * [08:08] RO | Hardware supports infrared transmissions
> + * [07:04] RO | Channel index of this instance
> + * [03:00] RO | Number of channels the base instance supports
> + */
> +
> +#define PWMV4_REG_ENABLE 0x004
> +/*
> + * ENABLE Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:06] RO | Reserved
> + * [05:05] RW | PWM Channel Counter Read Enable, 1 = enabled
> + */
> +#define PWMV4_CHN_CNT_RD_EN(v) FIELD_PREP_WM16(BIT(5), (v))
> +/*
> + * [04:04] W1T | PWM Globally Joined Control Enable
> + * 1 = this PWM channel will be enabled by a global pwm enable
> + * bit instead of the PWM Enable bit.
> + */
> +#define PWMV4_GLOBAL_CTRL_EN(v) FIELD_PREP_WM16(BIT(4), (v))
> +/*
> + * [03:03] RW | Force Clock Enable
> + * 0 = disabled, if the PWM channel is inactive then so is the
> + * clock prescale module
> + */
> +#define PWMV4_FORCE_CLK_EN(v) FIELD_PREP_WM16(BIT(3), (v))
> +/*
> + * [02:02] W1T | PWM Control Update Enable
> + * 1 = enabled, commits modifications of _CTRL, _PERIOD, _DUTY and
> + * _OFFSET registers once 1 is written to it
> + */
> +#define PWMV4_CTRL_UPDATE_EN FIELD_PREP_WM16_CONST(BIT(2), 1)
> +/*
> + * [01:01] RW | PWM Enable, 1 = enabled
> + * If in one-shot mode, clears after end of operation
> + */
> +#define PWMV4_EN_MASK BIT(1)
> +#define PWMV4_EN(v) FIELD_PREP_WM16(PWMV4_EN_MASK, \
> + ((v) ? 1 : 0))
> +/*
> + * [00:00] RW | PWM Clock Enable, 1 = enabled
> + * If in one-shot mode, clears after end of operation
> + */
> +#define PWMV4_CLK_EN_MASK BIT(0)
> +#define PWMV4_CLK_EN(v) FIELD_PREP_WM16(PWMV4_CLK_EN_MASK, \
> + ((v) ? 1 : 0))
> +#define PWMV4_EN_BOTH_MASK (PWMV4_EN_MASK | PWMV4_CLK_EN_MASK)
> +static inline __pure bool rockchip_pwm_v4_is_enabled(unsigned int val)
> +{
> + return (val & PWMV4_EN_BOTH_MASK);
> +}
> +
> +#define PWMV4_REG_CLK_CTRL 0x008
> +/*
> + * CLK_CTRL Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:15] RW | Clock Global Selection
> + * 0 = current channel scale clock
> + * 1 = global channel scale clock
> + */
> +#define PWMV4_CLK_GLOBAL(v) FIELD_PREP_WM16(BIT(15), (v))
> +/*
> + * [14:13] RW | Clock Source Selection
> + * 0 = Clock from PLL, frequency can be configured
> + * 1 = Clock from crystal oscillator, frequency is fixed
> + * 2 = Clock from RC oscillator, frequency is fixed
> + * 3 = Reserved
> + * NOTE: The purpose for this clock-mux-outside-CRU construct is
> + * to let the SoC go into a sleep state with the PWM
> + * hardware still having a clock signal for IR input, which
> + * can then wake up the SoC.
> + */
> +#define PWMV4_CLK_SRC_PLL 0x0U
> +#define PWMV4_CLK_SRC_CRYSTAL 0x1U
> +#define PWMV4_CLK_SRC_RC 0x2U
> +#define PWMV4_CLK_SRC_SHIFT 13
> +#define PWMV4_CLK_SRC_WIDTH 2
> +/*
> + * [12:04] RW | Scale Factor to apply to pre-scaled clock
> + * 1 <= v <= 256, v means clock divided by 2*v
> + */
> +#define PWMV4_CLK_SCALE_F(v) FIELD_PREP_WM16(GENMASK(12, 4), (v))
> +/*
> + * [03:03] RO | Reserved
> + * [02:00] RW | Prescale Factor
> + * v here means the input clock is divided by pow(2, v)
> + */
> +#define PWMV4_CLK_PRESCALE_F(v) FIELD_PREP_WM16(GENMASK(2, 0), (v))
> +
> +#define PWMV4_REG_CTRL 0x00C
> +/*
> + * CTRL Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:09] RO | Reserved
> + * [08:06] RW | PWM Input Channel Selection
> + * By default, the channel selects its own input, but writing v
> + * here selects PWM input from channel v instead.
> + */
> +#define PWMV4_CTRL_IN_SEL(v) FIELD_PREP_WM16(GENMASK(8, 6), (v))
> +/* [05:05] RW | Aligned Mode, 0 = Valid, 1 = Invalid */
> +#define PWMV4_CTRL_UNALIGNED(v) FIELD_PREP_WM16(BIT(5), (v))
> +/* [04:04] RW | Output Mode, 0 = Left Aligned, 1 = Centre Aligned */
> +#define PWMV4_LEFT_ALIGNED 0x0U
> +#define PWMV4_CENTRE_ALIGNED 0x1U
> +#define PWMV4_CTRL_OUT_MODE(v) FIELD_PREP_WM16(BIT(4), (v))
> +/*
> + * [03:03] RW | Inactive Polarity for when the channel is either disabled or
> + * has completed outputting the entire waveform in one-shot mode.
> + * 0 = Negative, 1 = Positive
> + */
> +#define PWMV4_POLARITY_N 0x0U
> +#define PWMV4_POLARITY_P 0x1U
> +#define PWMV4_INACTIVE_POL(v) FIELD_PREP_WM16(BIT(3), (v))
> +/*
> + * [02:02] RW | Duty Cycle Polarity to use at the start of the waveform.
> + * 0 = Negative, 1 = Positive
> + */
> +#define PWMV4_DUTY_POL_SHIFT 2
> +#define PWMV4_DUTY_POL_MASK BIT(PWMV4_DUTY_POL_SHIFT)
> +#define PWMV4_DUTY_POL(v) FIELD_PREP_WM16(PWMV4_DUTY_POL_MASK, \
> + (v))
> +/*
> + * [01:00] RW | PWM Mode
> + * 0 = One-shot mode, PWM generates waveform RPT times
> + * 1 = Continuous mode
> + * 2 = Capture mode, PWM measures cycles of input waveform
> + * 3 = Reserved
> + */
> +#define PWMV4_MODE_ONESHOT 0x0U
> +#define PWMV4_MODE_CONT 0x1U
> +#define PWMV4_MODE_CAPTURE 0x2U
> +#define PWMV4_MODE_MASK GENMASK(1, 0)
> +#define PWMV4_MODE(v) FIELD_PREP_WM16(PWMV4_MODE_MASK, (v))
> +#define PWMV4_CTRL_COM_FLAGS (PWMV4_INACTIVE_POL(PWMV4_POLARITY_N) | \
> + PWMV4_DUTY_POL(PWMV4_POLARITY_P) | \
> + PWMV4_CTRL_OUT_MODE(PWMV4_LEFT_ALIGNED) | \
> + PWMV4_CTRL_UNALIGNED(true))
> +#define PWMV4_CTRL_CONT_FLAGS (PWMV4_MODE(PWMV4_MODE_CONT) | \
> + PWMV4_CTRL_COM_FLAGS)
> +#define PWMV4_CTRL_CAP_FLAGS (PWMV4_MODE(PWMV4_MODE_CAPTURE) | \
> + PWMV4_CTRL_COM_FLAGS)
> +
> +#define PWMV4_REG_PERIOD 0x010
> +/*
> + * PERIOD Register Description
> + * [31:00] RW | Period of the output waveform
> + * Constraints: should be even if CTRL_OUT_MODE is CENTRE_ALIGNED
> + */
> +
> +#define PWMV4_REG_DUTY 0x014
> +/*
> + * DUTY Register Description
> + * [31:00] RW | Duty cycle of the output waveform
> + * Constraints: should be even if CTRL_OUT_MODE is CENTRE_ALIGNED
> + */
> +
> +#define PWMV4_REG_OFFSET 0x018
> +/*
> + * OFFSET Register Description
> + * [31:00] RW | Offset of the output waveform, based on the PWM clock
> + * Constraints: 0 <= v <= (PERIOD - DUTY)
> + */
> +
> +#define PWMV4_REG_RPT 0x01C
> +/*
> + * RPT Register Description
> + * [31:16] RW | Second dimensional of the effective number of waveform
> + * repetitions. Increases by one every first dimensional times.
> + * Value `n` means `n + 1` repetitions. The final number of
> + * repetitions of the waveform in one-shot mode is:
> + * `(first_dimensional + 1) * (second_dimensional + 1)`
> + * [15:00] RW | First dimensional of the effective number of waveform
> + * repetitions. Value `n` means `n + 1` repetitions.
> + */
> +
> +#define PWMV4_REG_FILTER_CTRL 0x020
> +/*
> + * FILTER_CTRL Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:10] RO | Reserved
> + * [09:04] RW | Filter window number
> + * [03:01] RO | Reserved
> + * [00:00] RW | Filter Enable, 0 = disabled, 1 = enabled
> + */
> +
> +#define PWMV4_REG_CNT 0x024
> +/*
> + * CNT Register Description
> + * [31:00] RO | Current value of the PWM Channel 0 counter in pwm clock cycles,
> + * 0 <= v <= 2^32-1
> + */
> +
> +#define PWMV4_REG_ENABLE_DELAY 0x028
> +/*
> + * ENABLE_DELAY Register Description
> + * [31:16] RO | Reserved
> + * [15:00] RW | PWM enable delay, in an unknown unit but probably cycles
> + */
> +
> +#define PWMV4_REG_HPC 0x02C
> +/*
> + * HPC Register Description
> + * [31:00] RW | Number of effective high polarity cycles of the input waveform
> + * in capture mode. Based on the PWM clock. 0 <= v <= 2^32-1
> + */
> +
> +#define PWMV4_REG_LPC 0x030
> +/*
> + * LPC Register Description
> + * [31:00] RW | Number of effective low polarity cycles of the input waveform
> + * in capture mode. Based on the PWM clock. 0 <= v <= 2^32-1
> + */
> +
> +#define PWMV4_REG_BIPHASIC_CNT_CTRL0 0x040
> +/*
> + * BIPHASIC_CNT_CTRL0 Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:10] RO | Reserved
> + * [09:09] RW | Biphasic Counter Phase Edge Selection for mode 0,
> + * 0 = rising edge (posedge), 1 = falling edge (negedge)
> + * [08:08] RW | Biphasic Counter Clock force enable, 1 = force enable
> + * [07:07] W1T | Synchronous Enable
> + * [06:06] W1T | Mode Switch
> + * 0 = Normal Mode, 1 = Switch timer clock and measured clock
> + * Constraints: "Biphasic Counter Mode" must be 0 if this is 1
> + * [05:03] RW | Biphasic Counter Mode
> + * 0x0 = Mode 0, 0x1 = Mode 1, 0x2 = Mode 2, 0x3 = Mode 3,
> + * 0x4 = Mode 4, 0x5 = Reserved
> + * [02:02] RW | Biphasic Counter Clock Selection
> + * 0 = clock is from PLL and frequency can be configured
> + * 1 = clock is from crystal oscillator and frequency is fixed
> + * [01:01] RW | Biphasic Counter Continuous Mode
> + * [00:00] W1T | Biphasic Counter Enable
> + */
> +
> +#define PWMV4_REG_BIPHASIC_CNT_CTRL1 0x044
> +/*
> + * BIPHASIC_CNT_CTRL1 Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:11] RO | Reserved
> + * [10:04] RW | Biphasic Counter Filter Window Number
> + * [03:01] RO | Reserved
> + * [00:00] RW | Biphasic Counter Filter Enable
> + */
> +
> +#define PWMV4_REG_BIPHASIC_CNT_TIMER 0x048
> +/*
> + * BIPHASIC_CNT_TIMER Register Description
> + * [31:00] RW | Biphasic Counter Timer Value, in number of biphasic counter
> + * timer clock cycles
> + */
> +
> +#define PWMV4_REG_BIPHASIC_CNT_RES 0x04C
> +/*
> + * BIPHASIC_CNT_RES Register Description
> + * [31:00] RO | Biphasic Counter Result Value
> + * Constraints: Can only be read after INTSTS[9] is asserted
> + */
> +
> +#define PWMV4_REG_BIPHASIC_CNT_RES_S 0x050
> +/*
> + * BIPHASIC_CNT_RES_S Register Description
> + * [31:00] RO | Biphasic Counter Result Value with synchronised processing
> + * Can be read in real-time if BIPHASIC_CNT_CTRL0[7] was set to 1
> + */
> +
> +#define PWMV4_REG_INTSTS 0x070
> +/*
> + * INTSTS Register Description
> + * [31:10] RO | Reserved
> + * [09:09] W1C | Biphasic Counter Interrupt Status, 1 = interrupt asserted
> + * [08:08] W1C | Waveform Middle Interrupt Status, 1 = interrupt asserted
> + * [07:07] W1C | Waveform Max Interrupt Status, 1 = interrupt asserted
> + * [06:06] W1C | IR Transmission End Interrupt Status, 1 = interrupt asserted
> + * [05:05] W1C | Power Key Match Interrupt Status, 1 = interrupt asserted
> + * [04:04] W1C | Frequency Meter Interrupt Status, 1 = interrupt asserted
> + * [03:03] W1C | Reload Interrupt Status, 1 = interrupt asserted
> + * [02:02] W1C | Oneshot End Interrupt Status, 1 = interrupt asserted
> + * [01:01] W1C | HPC Capture Interrupt Status, 1 = interrupt asserted
> + * [00:00] W1C | LPC Capture Interrupt Status, 1 = interrupt asserted
> + */
> +#define PWMV4_INT_LPC BIT(0)
> +#define PWMV4_INT_HPC BIT(1)
> +#define PWMV4_INT_LPC_W(v) FIELD_PREP_WM16(PWMV4_INT_LPC, \
> + ((v) ? 1 : 0))
> +#define PWMV4_INT_HPC_W(v) FIELD_PREP_WM16(PWMV4_INT_HPC, \
> + ((v) ? 1 : 0))
> +
> +#define PWMV4_REG_INT_EN 0x074
> +/*
> + * INT_EN Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:10] RO | Reserved
> + * [09:09] RW | Biphasic Counter Interrupt Enable, 1 = enabled
> + * [08:08] W1C | Waveform Middle Interrupt Enable, 1 = enabled
> + * [07:07] W1C | Waveform Max Interrupt Enable, 1 = enabled
> + * [06:06] W1C | IR Transmission End Interrupt Enable, 1 = enabled
> + * [05:05] W1C | Power Key Match Interrupt Enable, 1 = enabled
> + * [04:04] W1C | Frequency Meter Interrupt Enable, 1 = enabled
> + * [03:03] W1C | Reload Interrupt Enable, 1 = enabled
> + * [02:02] W1C | Oneshot End Interrupt Enable, 1 = enabled
> + * [01:01] W1C | HPC Capture Interrupt Enable, 1 = enabled
> + * [00:00] W1C | LPC Capture Interrupt Enable, 1 = enabled
> + */
> +
> +#define PWMV4_REG_INT_MASK 0x078
> +/*
> + * INT_MASK Register Description
> + * [31:16] WO | Write Enable Mask for the lower half of the register
> + * Set bit `n` here to 1 if you wish to modify bit `n >> 16` in
> + * the same write operation
> + * [15:10] RO | Reserved
> + * [09:09] RW | Biphasic Counter Interrupt Masked, 1 = masked
> + * [08:08] W1C | Waveform Middle Interrupt Masked, 1 = masked
> + * [07:07] W1C | Waveform Max Interrupt Masked, 1 = masked
> + * [06:06] W1C | IR Transmission End Interrupt Masked, 1 = masked
> + * [05:05] W1C | Power Key Match Interrupt Masked, 1 = masked
> + * [04:04] W1C | Frequency Meter Interrupt Masked, 1 = masked
> + * [03:03] W1C | Reload Interrupt Masked, 1 = masked
> + * [02:02] W1C | Oneshot End Interrupt Masked, 1 = masked
> + * [01:01] W1C | HPC Capture Interrupt Masked, 1 = masked
> + * [00:00] W1C | LPC Capture Interrupt Masked, 1 = masked
> + */
> +
> +static inline u32 mfpwm_reg_read(void __iomem *base, u32 reg)
> +{
> + return readl(base + reg);
> +}
> +
> +static inline void mfpwm_reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> + writel(val, base + reg);
> +}
a) Please do not abstract for the sake of it.
b) Please use Regmap instead.
> +
> +/**
> + * mfpwm_acquire - try becoming the active mfpwm function device
> + * @pwmf: pointer to the calling driver instance's &struct rockchip_mfpwm_func
> + *
> + * mfpwm device "function" drivers must call this function before doing anything
> + * that either modifies or relies on the parent device's state, such as clocks,
> + * enabling/disabling outputs, modifying shared regs etc.
> + *
> + * The return statues should always be checked.
> + *
> + * All mfpwm_acquire() calls must be balanced with corresponding mfpwm_release()
> + * calls once the device is no longer making changes that affect other devices,
> + * or stops producing user-visible effects that depend on the current device
> + * state being kept as-is. (e.g. after the PWM output signal is stopped)
> + *
> + * The same device function may mfpwm_acquire() multiple times while it already
> + * is active, i.e. it is re-entrant, though it needs to balance this with the
> + * same number of mfpwm_release() calls.
> + *
> + * Context: This function does not sleep.
> + *
> + * Return:
> + * * %0 - success
> + * * %-EBUSY - a different device function is active
> + * * %-EOVERFLOW - the acquire counter is at its maximum
> + */
> +extern int __must_check mfpwm_acquire(struct rockchip_mfpwm_func *pwmf);
> +
> +/**
> + * mfpwm_release - drop usage of active mfpwm device function by 1
> + * @pwmf: pointer to the calling driver instance's &struct rockchip_mfpwm_func
> + *
> + * This is the balancing call to mfpwm_acquire(). If no users of the device
> + * function remain, set the mfpwm device to have no active device function,
> + * allowing other device functions to claim it.
> + */
> +extern void mfpwm_release(const struct rockchip_mfpwm_func *pwmf);
> +
> +/**
> + * mfpwm_get_mode - get the current mode the hardware is in
> + * @pwmf: pointer to a &struct rockchip_mfpwm_func
> + *
> + * Check the hardware registers of the PWM hardware to determine which mode it
> + * is currently operating in, if any.
> + *
> + * Returns:
> + * - %-EINVAL if @pwmf is %NULL or an error pointer
> + * - %-1 if the PWM hardware is off, regardless of operating mode
> + * - %PWMV4_MODE_ONESHOT if PWM hardware is in one-shot output mode
> + * - %PWMV4_MODE_CONT if PWM hardware is in continuous output mode
> + * - %PWMV4_MODE_CAPTURE if PWM hardware is in capture mode
> + */
> +extern int mfpwm_get_mode(const struct rockchip_mfpwm_func *pwmf);
> +
> +#endif /* __SOC_ROCKCHIP_MFPWM_H__ */
>
> --
> 2.53.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: remoteproc: add imx-rproc-psci
From: Krzysztof Kozlowski @ 2026-05-14 11:38 UTC (permalink / raw)
To: Jiafei Pan
Cc: andersson, mathieu.poirier, peng.fan, Frank.Li, s.hauer, kernel,
festevam, imx, linux-arm-kernel, linux-kernel, Zhiqiang.Hou,
mingkai.hu, linux-remoteproc, devicetree
In-Reply-To: <20260511023928.39640-2-Jiafei.Pan@nxp.com>
On Mon, May 11, 2026 at 10:39:25AM +0800, Jiafei Pan wrote:
> Add compatible string "fsl,imx-rproc-psci" for i.MX Cortex-A Core's
> remoteproc support.
>
> Signed-off-by: Jiafei Pan <Jiafei.Pan@nxp.com>
>
> ---
> Changes in v3:
> - Fixed dt_binding_check warnings
>
> ---
> .../remoteproc/fsl,imx-rproc-psci.yaml | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc-psci.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc-psci.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc-psci.yaml
> new file mode 100644
> index 000000000000..28d00dbf8bc7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc-psci.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/fsl,imx-rproc-psci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX Cortex-A Core Remote Processor via PSCI
> +
> +maintainers:
> + - Jiafei Pan <Jiafei.Pan@nxp.com>
> +
> +description:
> + This binding provides support for managing Cortex-A cores as remote
Describe the hardware, not the binding.
> + processors on i.MX platforms using the PSCI (Power State Coordination
> + Interface) for CPU power management operations. This allows single
> + Cortex-A core or multiple Cortex-A cores to be controlled by Linux as
> + a remote processor, enabling them to run RTOS or bare-metal applications.
> +
> +properties:
> + compatible:
> + const: fsl,imx-rproc-psci
Why isn't the compatible specific?
> +
> + fsl,cpus-mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Bitmask indicating which CPU cores are assigned to this remote
> + processor instance. Each bit represents a CPU core, where bit N
> + corresponds to CPU N. For example, 0x2 (0b10) assigns CPU core 1,
> + while 0x6 (0b110) assigns CPU cores 1 and 2.
So you partition existing Cortex-A cores? Or how exactly? Why isn't this
deducible from the compatible (I assume you read carefully writing
bindings)?
> +
> + memory-region:
> + maxItems: 1
> + description:
> + Phandle to a reserved memory region to be used for the remote
> + processor's code and data.
> +
> +required:
> + - compatible
> + - fsl,cpus-mask
> + - memory-region
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + remoteproc-ca55-1 {
Implement previous comments.
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox