* 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
* [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource
From: Enric Balletbo i Serra @ 2026-05-14 11:14 UTC (permalink / raw)
To: Russell King, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Daniel Lezcano, Thomas Gleixner
Cc: linux-arm-kernel, imx, linux-kernel, Enric Balletbo i Serra
From: Enric Balletbo i Serra <eballetb@redhat.com>
The Kconfig logic for selecting the scheduler clocksource on
NXP Vybrid (VF610) uses a `choice` block restricted to 32-bit ARM. This
prevents 64-bit architectures, such as the NXP S32 family, from enabling
the NXP Periodic Interrupt Timer (PIT) driver (CONFIG_NXP_PIT_TIMER).
Relocate the NXP clocksource selection from arch/arm/mach-imx/Kconfig to
drivers/clocksource/Kconfig. This allows the configuration to be shared
across different architectures.
Update the selection to include support for ARCH_S32 and add a "None"
option restricted to ARCH_S32, since Vybrid lacks the ARM Architected
Timer. The Vybrid Global Timer option is restricted to ARCH_MULTI_V7
SOC_VF610 platforms to prevent it from being visible on Cortex-M4 builds,
which lack the ARM Global Timer hardware.
Fixes: bee33f22d7c3 ("clocksource/drivers/nxp-pit: Add NXP Automotive s32g2 / s32g3 support")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
Changes in v3:
- Restrict VF_TIMER_NONE to ARCH_S32 to prevent selecting it on Vybrid
platforms which lack the ARM Architected Timer
- Link to v2: https://lore.kernel.org/r/20260513-fix-nxp-timer-v2-1-533b99c57b67@redhat.com
Changes in v2:
- Fix VF_USE_ARM_GLOBAL_TIMER dependency: use ARCH_MULTI_V7 instead of
ARM to prevent the option from being visible on Cortex-M4 builds
(Sashiko AI review)
- Link to v1: https://lore.kernel.org/r/20260302-fix-nxp-timer-v1-1-af4bc62d4ffa@redhat.com
---
arch/arm/mach-imx/Kconfig | 21 ---------------------
drivers/clocksource/Kconfig | 31 +++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 6ea1bd55acf8..a361840d7a04 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -227,27 +227,6 @@ config SOC_VF610
help
This enables support for Freescale Vybrid VF610 processor.
-choice
- prompt "Clocksource for scheduler clock"
- depends on SOC_VF610
- default VF_USE_ARM_GLOBAL_TIMER
-
- config VF_USE_ARM_GLOBAL_TIMER
- bool "Use ARM Global Timer"
- depends on ARCH_MULTI_V7
- select ARM_GLOBAL_TIMER
- select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
- help
- Use the ARM Global Timer as clocksource
-
- config VF_USE_PIT_TIMER
- bool "Use PIT timer"
- select NXP_PIT_TIMER
- help
- Use SoC Periodic Interrupt Timer (PIT) as clocksource
-
-endchoice
-
endif
endif
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index d1a33a231a44..d9c76dd443f8 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -793,4 +793,35 @@ config RTK_SYSTIMER
this option only when building for a Realtek platform or for compilation
testing.
+choice
+ prompt "NXP clocksource for scheduler clock"
+ depends on SOC_VF610 || ARCH_S32
+ # Default to Global Timer for Vybrid (32-bit)
+ default VF_USE_ARM_GLOBAL_TIMER if SOC_VF610
+ # Default to None for S32 (64-bit)
+ default VF_TIMER_NONE if ARCH_S32
+
+ config VF_USE_ARM_GLOBAL_TIMER
+ bool "Use NXP Vybrid Global Timer"
+ depends on ARCH_MULTI_V7 && SOC_VF610
+ select ARM_GLOBAL_TIMER
+ select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
+ help
+ Use the NXP Vybrid Global Timer as clocksource.
+
+ config VF_USE_PIT_TIMER
+ bool "Use NXP PIT timer"
+ select NXP_PIT_TIMER
+ help
+ Use NXP Periodic Interrupt Timer (PIT) as clocksource.
+
+ config VF_TIMER_NONE
+ bool "None (Use standard Arch Timer)"
+ depends on ARCH_S32
+ help
+ Do not use any specific NXP timer driver. Use the standard
+ ARM Architected Timer instead.
+
+endchoice
+
endmenu
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260302-fix-nxp-timer-9cb1fbd7afcd
Best regards,
--
Enric Balletbo i Serra <eballetb@redhat.com>
^ permalink raw reply related
* Re: [PATCH v5 1/3] firmware: smccc: coco: Manage arm-smccc platform device and CCA auxiliary drivers
From: Suzuki K Poulose @ 2026-05-14 11:04 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-coco, linux-arm-kernel,
linux-kernel
Cc: Catalin Marinas, Greg KH, Jeremy Linton, Jonathan Cameron,
Lorenzo Pieralisi, Mark Rutland, Sudeep Holla, Will Deacon,
Steven Price
In-Reply-To: <20260514094030.42495-2-aneesh.kumar@kernel.org>
Hi Aneesh
On 14/05/2026 10:40, Aneesh Kumar K.V (Arm) wrote:
> Make the SMCCC driver responsible for registering the arm-smccc platform
> device and after confirming the relevant SMCCC function IDs, create
> the arm_cca_guest auxiliary device.
>
There are a few changes squashed in to this patch. Please could we
split the patch in the following order ?
1. Add platform device for arm-smccc
2. Move TRNG to Auxilliary Device - (Even though it is a later patch,
move it before the RSI changes)
3. Move RSI dev as Auxilliary
4. Add the firmware sysfs ABI.
That way, first two could be merged while we figure out (3) and (4)
> Also update the arm-cca-guest driver to use the auxiliary device
> interface instead of the platform device (arm-cca-dev). The removal of
> the platform device registration will follow in a subsequent patch,
> allowing this change to be applied without immediately breaking existing
> userspace dependencies [1].
>
> [1] https://lore.kernel.org/all/4a7d84b2-2ec4-4773-a2d5-7b63d5c683cf@arm.com
>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> arch/arm64/include/asm/rsi.h | 2 +-
> arch/arm64/kernel/rsi.c | 2 +-
> drivers/firmware/smccc/Kconfig | 1 +
> drivers/firmware/smccc/Makefile | 1 +
> drivers/firmware/smccc/rmm.c | 24 ++++++++
> drivers/firmware/smccc/rmm.h | 17 ++++++
> drivers/firmware/smccc/smccc.c | 17 ++++++
> drivers/virt/coco/arm-cca-guest/Kconfig | 1 +
> drivers/virt/coco/arm-cca-guest/Makefile | 2 +
> .../{arm-cca-guest.c => arm-cca.c} | 59 +++++++++----------
> 10 files changed, 94 insertions(+), 32 deletions(-)
> create mode 100644 drivers/firmware/smccc/rmm.c
> create mode 100644 drivers/firmware/smccc/rmm.h
> rename drivers/virt/coco/arm-cca-guest/{arm-cca-guest.c => arm-cca.c} (84%)
>
> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
> index 88b50d660e85..2d2d363aaaee 100644
> --- a/arch/arm64/include/asm/rsi.h
> +++ b/arch/arm64/include/asm/rsi.h
> @@ -10,7 +10,7 @@
> #include <linux/jump_label.h>
> #include <asm/rsi_cmds.h>
>
> -#define RSI_PDEV_NAME "arm-cca-dev"
> +#define RSI_DEV_NAME "arm-rsi-dev"
>
> DECLARE_STATIC_KEY_FALSE(rsi_present);
>
> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
> index 9e846ce4ef9c..8380e5ba88d2 100644
> --- a/arch/arm64/kernel/rsi.c
> +++ b/arch/arm64/kernel/rsi.c
> @@ -161,7 +161,7 @@ void __init arm64_rsi_init(void)
> }
>
> static struct platform_device rsi_dev = {
> - .name = RSI_PDEV_NAME,
> + .name = "arm-cca-dev",
> .id = PLATFORM_DEVID_NONE
> };
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 15e7466179a6..2b6984757241 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -8,6 +8,7 @@ config HAVE_ARM_SMCCC
> config HAVE_ARM_SMCCC_DISCOVERY
> bool
> depends on ARM_PSCI_FW
> + select AUXILIARY_BUS
> default y
> help
> SMCCC v1.0 lacked discoverability and hence PSCI v1.0 was updated
> diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
> index 40d19144a860..146dc3c03c20 100644
> --- a/drivers/firmware/smccc/Makefile
> +++ b/drivers/firmware/smccc/Makefile
> @@ -2,3 +2,4 @@
> #
> obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
> obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
> +obj-$(CONFIG_ARM64) += rmm.o
> diff --git a/drivers/firmware/smccc/rmm.c b/drivers/firmware/smccc/rmm.c
> new file mode 100644
> index 000000000000..728338cb5a22
> --- /dev/null
> +++ b/drivers/firmware/smccc/rmm.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Arm Limited
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +
> +#include "rmm.h"
> +
> +void __init register_rsi_device(struct platform_device *pdev)
> +{
> + unsigned long ret;
> + unsigned long ver_lower, ver_higher;
> +
> + if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC)
> + return;
> +
> + ret = rsi_request_version(RSI_ABI_VERSION, &ver_lower, &ver_higher);
> + if (ret != RSI_SUCCESS)
> + return;
> +
> + __devm_auxiliary_device_create(&pdev->dev,
> + "arm_cca_guest", RSI_DEV_NAME, NULL, 0);
> +}
> diff --git a/drivers/firmware/smccc/rmm.h b/drivers/firmware/smccc/rmm.h
> new file mode 100644
> index 000000000000..a47a650d4f51
> --- /dev/null
> +++ b/drivers/firmware/smccc/rmm.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _SMCCC_RMM_H
> +#define _SMCCC_RMM_H
> +
> +#include <linux/platform_device.h>
> +
> +#ifdef CONFIG_ARM64
> +#include <asm/rsi_cmds.h>
> +void __init register_rsi_device(struct platform_device *pdev);
> +#else
> +
> +static void __init register_rsi_device(struct platform_device *pdev)
> +{
> +
> +}
> +#endif
> +#endif
> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index bdee057db2fd..eb077b9aa6da 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
> @@ -12,6 +12,8 @@
> #include <linux/platform_device.h>
> #include <asm/archrandom.h>
>
> +#include "rmm.h"
> +
> static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
> static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
>
> @@ -85,6 +87,21 @@ static int __init smccc_devices_init(void)
> {
> struct platform_device *pdev;
>
> + if (smccc_conduit == SMCCC_CONDUIT_NONE)
> + return 0;
> +
> + pdev = platform_device_register_simple("arm-smccc",
> + PLATFORM_DEVID_NONE, NULL, 0);
> + if (IS_ERR(pdev)) {
> + pr_err("arm-smccc: could not register device: %ld\n", PTR_ERR(pdev));
> + } else {
> + /*
> + * Register the RMI and RSI devices only when firmware exposes
> + * the required SMCCC function IDs at a supported revision.
> + */
> + register_rsi_device(pdev);
> + }
> +
> if (smccc_trng_available) {
> pdev = platform_device_register_simple("smccc_trng", -1,
> NULL, 0);
> diff --git a/drivers/virt/coco/arm-cca-guest/Kconfig b/drivers/virt/coco/arm-cca-guest/Kconfig
> index 3f0f013f03f1..a42359a90558 100644
> --- a/drivers/virt/coco/arm-cca-guest/Kconfig
> +++ b/drivers/virt/coco/arm-cca-guest/Kconfig
> @@ -2,6 +2,7 @@ config ARM_CCA_GUEST
> tristate "Arm CCA Guest driver"
> depends on ARM64
> select TSM_REPORTS
> + select AUXILIARY_BUS
> help
> The driver provides userspace interface to request and
> attestation report from the Realm Management Monitor(RMM).
> diff --git a/drivers/virt/coco/arm-cca-guest/Makefile b/drivers/virt/coco/arm-cca-guest/Makefile
> index 69eeba08e98a..75a120e24fda 100644
> --- a/drivers/virt/coco/arm-cca-guest/Makefile
> +++ b/drivers/virt/coco/arm-cca-guest/Makefile
> @@ -1,2 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_ARM_CCA_GUEST) += arm-cca-guest.o
> +
> +arm-cca-guest-y += arm-cca.o
> diff --git a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c b/drivers/virt/coco/arm-cca-guest/arm-cca.c
> similarity index 84%
> rename from drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> rename to drivers/virt/coco/arm-cca-guest/arm-cca.c
> index 0c9ea24a200c..7daada072cc0 100644
> --- a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> +++ b/drivers/virt/coco/arm-cca-guest/arm-cca.c
> @@ -3,6 +3,7 @@
> * Copyright (C) 2023 ARM Ltd.
> */
>
> +#include <linux/auxiliary_bus.h>
> #include <linux/arm-smccc.h>
> #include <linux/cc_platform.h>
> #include <linux/kernel.h>
> @@ -181,52 +182,50 @@ static int arm_cca_report_new(struct tsm_report *report, void *data)
> return ret;
> }
>
> -static const struct tsm_report_ops arm_cca_tsm_ops = {
> +static const struct tsm_report_ops arm_cca_tsm_report_ops = {
> .name = KBUILD_MODNAME,
> .report_new = arm_cca_report_new,
> };
>
> -/**
> - * arm_cca_guest_init - Register with the Trusted Security Module (TSM)
> - * interface.
> - *
> - * Return:
> - * * %0 - Registered successfully with the TSM interface.
> - * * %-ENODEV - The execution context is not an Arm Realm.
> - * * %-EBUSY - Already registered.
> - */
> -static int __init arm_cca_guest_init(void)
> +static void unregister_cca_tsm_report(void *data)
> +{
> + tsm_report_unregister(&arm_cca_tsm_report_ops);
> +}
> +
> +static int cca_devsec_tsm_probe(struct auxiliary_device *adev,
super minor nit: While I understand you plan to use this for DEV SEC TSM
in the future, could we retain the generic TSM name usage ?
> + const struct auxiliary_device_id *id)
> {
> int ret;
>
> if (!is_realm_world())
> return -ENODEV;
>
> - ret = tsm_report_register(&arm_cca_tsm_ops, NULL);
> - if (ret < 0)
> - pr_err("Error %d registering with TSM\n", ret);
> + ret = tsm_report_register(&arm_cca_tsm_report_ops, NULL);
> + if (ret < 0) {
> + dev_err_probe(&adev->dev, ret, "Error registering with TSM\n");
> + return ret;
> + }
>
> - return ret;
> -}
> -module_init(arm_cca_guest_init);
> + ret = devm_add_action_or_reset(&adev->dev, unregister_cca_tsm_report, NULL);
> + if (ret < 0) {
> + dev_err_probe(&adev->dev, ret, "Error registering devm action\n");
> + return ret;
> + }
>
> -/**
> - * arm_cca_guest_exit - unregister with the Trusted Security Module (TSM)
> - * interface.
> - */
> -static void __exit arm_cca_guest_exit(void)
> -{
> - tsm_report_unregister(&arm_cca_tsm_ops);
> + return 0;
> }
> -module_exit(arm_cca_guest_exit);
>
> -/* modalias, so userspace can autoload this module when RSI is available */
> -static const struct platform_device_id arm_cca_match[] __maybe_unused = {
> - { RSI_PDEV_NAME, 0},
> - { }
> +static const struct auxiliary_device_id cca_devsec_tsm_id_table[] = {
same as above, s/devsec_// ?
Suzuki
> + { .name = KBUILD_MODNAME "." RSI_DEV_NAME },
> + {}
> };
> +MODULE_DEVICE_TABLE(auxiliary, cca_devsec_tsm_id_table);
>
> -MODULE_DEVICE_TABLE(platform, arm_cca_match);
> +static struct auxiliary_driver cca_devsec_tsm_driver = {
> + .probe = cca_devsec_tsm_probe,
> + .id_table = cca_devsec_tsm_id_table,
> +};
> +module_auxiliary_driver(cca_devsec_tsm_driver);
> MODULE_AUTHOR("Sami Mujawar <sami.mujawar@arm.com>");
> MODULE_DESCRIPTION("Arm CCA Guest TSM Driver");
> MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [PATCH 4/5] ARM: dts: qcom: msm8974-oneplus-bacon: Fix sn3193 shutdown GPIO polarity
From: Konrad Dybcio @ 2026-05-14 10:58 UTC (permalink / raw)
To: Jun Yan, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
Vincent Knecht, Grant Feng, Andre Przywara, Florian Fainelli,
Heiko Stuebner, Michal Simek, Wei Xu, Robert Marko
Cc: Pavel Machek, Krzysztof Kozlowski, Alexandre TORGUE,
Jisheng Zhang, Enric Balletbo i Serra, Romain Perier, linux-leds,
devicetree, linux-kernel, linux-arm-msm, linux-arm-kernel
In-Reply-To: <20260508152435.21389-5-jerrysteve1101@gmail.com>
On 5/8/26 5:24 PM, Jun Yan wrote:
> The sn3193 shutdown pin is active-low[1]. Correct the GPIO flags
> from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.
>
> [1] https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
>
> Fixes: 724ba6751532 ("ARM: dts: Move .dts files to vendor sub-directories")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 3/5] arm64: dts: qcom: msm8916-alcatel-idol347: Fix sn3190 shutdown GPIO polarity
From: Konrad Dybcio @ 2026-05-14 10:58 UTC (permalink / raw)
To: Jun Yan, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
Vincent Knecht, Grant Feng, Andre Przywara, Heiko Stuebner,
Romain Perier, Paul Barker, Patrice Chotard, Robert Marko
Cc: Pavel Machek, Krzysztof Kozlowski, Jesper Nilsson, Peter Rosin,
linux-leds, devicetree, linux-kernel, linux-arm-msm,
linux-arm-kernel
In-Reply-To: <20260508152435.21389-4-jerrysteve1101@gmail.com>
On 5/8/26 5:24 PM, Jun Yan wrote:
> The sn3190 shutdown pin is active-low [1]. Correct the GPIO flags
> from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.
>
> [1] https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf
>
> Fixes: 1c8cc183d070 ("arm64: dts: qcom: msm8916-alcatel-idol347: add LED indicator")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
Assuming you get the other backwards compat pieces in:
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v5 4/8] mfd: khadas-mcu: Add support for VIM4 MCU variant
From: Lee Jones @ 2026-05-14 10:54 UTC (permalink / raw)
To: Ronald Claveau via B4 Relay
Cc: Neil Armstrong, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andi Shyti, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Beniamino Galvani, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Liam Girdwood, Mark Brown, linux-amlogic, devicetree,
linux-kernel, linux-i2c, linux-arm-kernel, linux-pm,
Ronald Claveau
In-Reply-To: <20260424-add-mcu-fan-khadas-vim4-v5-4-afcfa7157b23@aliel.fr>
On Fri, 24 Apr 2026, Ronald Claveau via B4 Relay wrote:
> From: Ronald Claveau <linux-kernel-dev@aliel.fr>
>
> Refactor probe() to use per-variant khadas_mcu_data
> instead of hardcoded globals.
>
> Add dedicated regmap configuration and device data for the VIM4 MCU,
> with its own volatile/writeable registers.
>
> Add the fan control register
> (0–100 levels vs 0–3 for previous supported boards).
>
> Add a new compatible string "khadas,vim4-mcu".
>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
> drivers/mfd/khadas-mcu.c | 106 ++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 95 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
> index ba981a7886921..b36b3b3ab73c0 100644
> --- a/drivers/mfd/khadas-mcu.c
> +++ b/drivers/mfd/khadas-mcu.c
> @@ -75,15 +75,91 @@ static const struct regmap_config khadas_mcu_regmap_config = {
> .cache_type = REGCACHE_MAPLE,
> };
>
> +static const struct khadas_mcu_fan_pdata khadas_mcu_fan_pdata = {
> + .fan_reg = KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
> + .max_level = 3,
> +};
What is 3?
> +
> static struct mfd_cell khadas_mcu_fan_cells[] = {
> /* VIM1/2 Rev13+ and VIM3 only */
> - { .name = "khadas-mcu-fan-ctrl", },
> + {
> + .name = "khadas-mcu-fan-ctrl",
> + .platform_data = &khadas_mcu_fan_pdata,
> + .pdata_size = sizeof(khadas_mcu_fan_pdata),
> + },
> };
Worth making this const at one point.
>
> static struct mfd_cell khadas_mcu_cells[] = {
> { .name = "khadas-mcu-user-mem", },
> };
>
> +static const struct khadas_mcu_data khadas_mcu_data = {
> + .regmap_config = &khadas_mcu_regmap_config,
> + .cells = khadas_mcu_cells,
> + .ncells = ARRAY_SIZE(khadas_mcu_cells),
> + .fan_cells = khadas_mcu_fan_cells,
> + .nfan_cells = ARRAY_SIZE(khadas_mcu_fan_cells),
> +};
This is a red flag!
> +static bool khadas_mcu_vim4_reg_volatile(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case KHADAS_MCU_PWR_OFF_CMD_REG:
> + case KHADAS_MCU_VIM4_REST_CONF_REG:
> + case KHADAS_MCU_WOL_INIT_START_REG:
> + case KHADAS_MCU_VIM4_LED_ON_RAM_REG:
> + case KHADAS_MCU_VIM4_FAN_CTRL_REG:
> + case KHADAS_MCU_VIM4_WDT_EN_REG:
> + case KHADAS_MCU_VIM4_SYS_RST_REG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool khadas_mcu_vim4_reg_writeable(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case KHADAS_MCU_VERSION_0_REG:
> + case KHADAS_MCU_VERSION_1_REG:
> + case KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG:
> + return false;
> + default:
> + return true;
> + }
> +}
> +
> +static const struct regmap_config khadas_mcu_vim4_regmap_config = {
> + .reg_bits = 8,
> + .reg_stride = 1,
> + .val_bits = 8,
> + .max_register = KHADAS_MCU_VIM4_SYS_RST_REG,
> + .volatile_reg = khadas_mcu_vim4_reg_volatile,
> + .writeable_reg = khadas_mcu_vim4_reg_writeable,
> + .cache_type = REGCACHE_MAPLE,
> +};
> +
> +static const struct khadas_mcu_fan_pdata khadas_vim4_fan_pdata = {
> + .fan_reg = KHADAS_MCU_VIM4_FAN_CTRL_REG,
> + .max_level = 0x64,
> +};
> +
> +static const struct mfd_cell khadas_mcu_vim4_cells[] = {
> + {
> + .name = "khadas-mcu-fan-ctrl",
> + .platform_data = &khadas_vim4_fan_pdata,
> + .pdata_size = sizeof(khadas_vim4_fan_pdata),
> + },
> +};
> +
> +static const struct khadas_mcu_data khadas_vim4_mcu_data = {
> + .regmap_config = &khadas_mcu_vim4_regmap_config,
> + .cells = NULL,
> + .ncells = 0,
> + .fan_cells = khadas_mcu_vim4_cells,
> + .nfan_cells = ARRAY_SIZE(khadas_mcu_vim4_cells),
> +};
> +
> static int khadas_mcu_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> @@ -94,28 +170,35 @@ static int khadas_mcu_probe(struct i2c_client *client)
> if (!ddata)
> return -ENOMEM;
>
> + ddata->data = i2c_get_match_data(client);
> + if (!ddata->data)
> + return -EINVAL;
Shouldn't this be -ENODEV?
> i2c_set_clientdata(client, ddata);
>
> ddata->dev = dev;
>
> - ddata->regmap = devm_regmap_init_i2c(client, &khadas_mcu_regmap_config);
> + ddata->regmap = devm_regmap_init_i2c(client,
> + ddata->data->regmap_config);
Use up to 100-chars to prevent this kind of wrapping.
> if (IS_ERR(ddata->regmap)) {
> ret = PTR_ERR(ddata->regmap);
> dev_err(dev, "Failed to allocate register map: %d\n", ret);
> return ret;
> }
Maybe convert this to dev_err_probe() at one point.
> - ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> - khadas_mcu_cells,
> - ARRAY_SIZE(khadas_mcu_cells),
> - NULL, 0, NULL);
> - if (ret)
> - return ret;
> + if (ddata->data->cells && ddata->data->ncells) {
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + ddata->data->cells,
> + ddata->data->ncells,
> + NULL, 0, NULL);
> + if (ret)
> + return ret;
> + }
>
> if (of_property_present(dev->of_node, "#cooling-cells"))
> return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> - khadas_mcu_fan_cells,
> - ARRAY_SIZE(khadas_mcu_fan_cells),
> + ddata->data->fan_cells,
> + ddata->data->nfan_cells,
> NULL, 0, NULL);
>
> return 0;
> @@ -123,7 +206,8 @@ static int khadas_mcu_probe(struct i2c_client *client)
>
> #ifdef CONFIG_OF
> static const struct of_device_id khadas_mcu_of_match[] = {
> - { .compatible = "khadas,mcu", },
> + { .compatible = "khadas,mcu", .data = &khadas_mcu_data },
> + { .compatible = "khadas,vim4-mcu", .data = &khadas_vim4_mcu_data },
We don't allow data from one registration API (MFD) to be shoved through
another (DT). Pass a value to match on instead, then use a switch()
statement or similar to populate or register the devices.
> {},
> };
> MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
>
> --
> 2.49.0
>
>
^ permalink raw reply
* Re: [PATCH v5 0/3] Switch Arm CCA to use an auxiliary device instead of a platform device
From: Aneesh Kumar K.V @ 2026-05-14 10:51 UTC (permalink / raw)
To: Greg KH
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: <2026051453-batting-delighted-0a57@gregkh>
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.
-aneesh
^ permalink raw reply
* Re: [PATCH 0/5] scmi: Log client subsystem entity counts
From: Sudeep Holla @ 2026-05-14 10:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Guenter Roeck, Andy Shevchenko, Alex Tran, Sudeep Holla,
Jyoti Bhayana, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Cristian Marussi, Linus Walleij,
Rafael J. Wysocki, Philipp Zabel, Viresh Kumar, linux-iio,
linux-kernel, arm-scmi, linux-arm-kernel, linux-gpio, linux-pm,
linux-hwmon
In-Reply-To: <2026051441-agreed-suffice-bbe1@gregkh>
On Thu, May 14, 2026 at 08:48:19AM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2026 at 11:27:21AM -0700, Guenter Roeck wrote:
> > On 5/13/26 11:02, Andy Shevchenko wrote:
> > > +Greg (I believe the trend is to drop such messages and not add them [back]?)
> > >
> >
> > Is there some common guidance on this ? I'd be all for dropping messages
> > instead of adding them, but there seems to be a perpetual battle between
> > people who want to log everything and people concerned about logging noise.
> > As maintainer I always seem to be stuck between those two camps.
>
> When drivers work properly, they should be quiet. This patch series
> adds a bunch of dev_info() calls, which is not ok. If a developer wants
> to see extra messages, use the dev_dbg() infrastructure, or the tracing
> infrastructure, both of which are there for this very reason.
>
I completely agree and tend to follow that. But I always assumed it was
left to maintainers taste.
> So yes, I agree with Andy, this series is not ok, don't make more noise
> please.
>
I am now thinking if [1] was the one setting example for this series. I did
ack it as I left it to the subsystem maintainer's choice(in this case author
as well).
--
Regards,
Sudeep
[1] https://lore.kernel.org/all/20260304101457.7470-1-ulf.hansson@linaro.org/
^ permalink raw reply
* [PATCH] i2c: davinci: fix division by zero on missing clock-frequency
From: Chaitanya Sabnis @ 2026-05-14 10:37 UTC (permalink / raw)
To: brgl, andi.shyti
Cc: linux-arm-kernel, linux-i2c, linux-kernel, Chaitanya Sabnis,
Sashiko
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.
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>
---
drivers/i2c/busses/i2c-davinci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index a773ba082321..bd0754abdcb7 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -760,9 +760,9 @@ static int davinci_i2c_probe(struct platform_device *pdev)
r = device_property_read_u32(&pdev->dev, "clock-frequency", &prop);
if (r)
- prop = DAVINCI_I2C_DEFAULT_BUS_FREQ;
-
- dev->bus_freq = prop / 1000;
+ dev->bus_freq = DAVINCI_I2C_DEFAULT_BUS_FREQ;
+ else
+ dev->bus_freq = prop / 1000;
dev->has_pfunc = device_property_present(&pdev->dev, "ti,has-pfunc");
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 2/4] ASoC: stm: stm32_i2s: Use guard() for spin locks
From: Bui Duc Phuc @ 2026-05-14 10:33 UTC (permalink / raw)
To: Mark Brown
Cc: Olivier Moysan, Arnaud Pouliquen, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai, Maxime Coquelin, Alexandre Torgue, linux-sound,
linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <agUknFcDIfwrOCld@sirena.co.uk>
Hi Mark,
On Thu, May 14, 2026 at 8:25 AM Mark Brown <broonie@kernel.org> wrote:
> How does scoped_guard interact with break statements - does this still
> apply to the switch? I've not looked at how they're implemented...
I checked the scoped_guard macro implementation...
and You're right the break statement inside scoped_guard only exits
the guard's implicit loop, which is not what I intended.
Since there are no further statements after the switch block in this
function, I will replace the break with return 0 to correctly exit
the function.
I'll send a v2 of the whole series with this fix.
Would that work for you?
Best regard,
Phuc
^ permalink raw reply
* Re: [RFC PATCH net-next 0/3] net: macb: candidate fixes for silent TX stall on BCM2712/RP1
From: Théo Lebrun @ 2026-05-14 10:31 UTC (permalink / raw)
To: Lukasz Raczylo, netdev
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <cover.1777064117.git.lukasz@raczylo.com>
Hello Lukasz,
On Sat Apr 25, 2026 at 12:38 AM CEST, Lukasz Raczylo wrote:
> This series proposes three candidate fixes for the silent TX stall
> observed on Raspberry Pi 5 (BCM2712 SoC, Cadence GEM via RP1 PCIe
> south bridge). The bug has been reported, with reproducers, at:
I've taken over the MACB driver maintenance following Clandiu & Nicolas'
work. I have read with curiosity your series and links attached
(though I skimmed over parts because there's been a lot of discussion).
Have you moved forward since your initial post? I've seen your
still-working-on-it message from May 8th on the rpi kernel PR. I have a
few remarks and/or questions:
- You still think the two fix patches solve it? Any clearer picture of
which of the two patches inbetween [1/3] or[2/3] fixes it? Does
[3/3] ever trigger on your targets?
- Can you clarify the exact symptoms? I've seen a few contradictory
facts. Two I remember:
- You say here it is a Tx stall but I've seen messages in the linked
threads that say explicitely broken Tx & Rx.
https://github.com/cilium/cilium/issues/43198#issue-3706713821
- You say here link down/up fixes it, but there is a comment that
says they unload/reload the module (rtheobald). They don't say
explicitely that link down/up doesn't work for them though, but
someone before in the thread recommended link down/up. Another
one says "Only power cycle recovers the node" (lexfrei).
- Also, some messages point out disabling TSO / SG / EEE helped it.
Any comment on that? It would help point fingers.
- Some comments are about DT props missing. Is that lead dead now?
- I've seen no mention of the bug having been reproduced on upstream
kernel (?). What does the rpi kernel bring to the table that makes
everyone use it?
- Anything was found to increase the reproducibility of the bug? If it
was then a bisect could be made possible, as I've seen mentions that
it didn't appear on some older kernels.
Now about the patches:
> Reading the current driver we identified three plausible races
> between driver and hardware, each of which could independently
> produce the observed behaviour. We did not determine which is the
> actual root cause -- that likely requires either BCM2712/RP1
> documentation we do not have, or dynamic tracing of the driver
> during an in-situ stall. The series therefore attempts to close
> all three, with each commit message stating which specific race
> that patch is targeting.
>
> Patch 1/3 -- flush PCIe posted write after TSTART doorbell.
> Writes to NCR are posted PCIe writes and may not reach the MAC
> before the driver returns. If the TSTART doorbell is lost, no
> TX starts, no TCOMP arrives, and the ring goes quiescent. A
> read-back of NCR after the write is a standard read-after-write
> PCIe flush.
- Makes sense, but only on MACB mounted over PCI, which is not the
majority.
- IDK if we can do better than a readl(NCR) on all platforms.
- I am surprised it is the only writel() that needs to be flushed?
> Patch 2/3 -- re-check ISR after IER re-enable in macb_tx_poll().
> An existing comment in macb_tx_poll() notes that completions
> raised while TCOMP is masked do not re-fire when IER is
> re-enabled, and mitigates the window with macb_tx_complete_pending(),
> which inspects driver-visible ring state only (after rmb()). On
> PCIe-attached parts the descriptor DMA write that sets TX_USED
> can remain in flight when that check runs; the rmb() orders CPU
> writes but does not retire peripheral DMA. Reading ISR directly
> after IER re-enable addresses this in two ways: (a) the MMIO read
> is an architected PCIe read barrier for prior DMA writes, so a
> subsequent macb_tx_complete_pending() sees up-to-date TX_USED
> state; (b) it directly observes a pending TCOMP bit if the
> hardware has one set. Either signal reschedules NAPI.
This will not fly because ISR might be read-to-clear.
See macb_queue_isr_clear() and how it is used. So we cannot re-read ISR
safely on those platforms.
> Patch 3/3 -- TX stall watchdog. Defence-in-depth. If patches
> 1 and 2 close the races we identified, this patch performs a
> single spin_lock_irqsave/unlock and a branch per queue per
> second with no other effect. If a further race remains that we
> have not identified, it invokes the driver's own existing
> macb_tx_restart(), which already verifies that TBQP is behind
> tx_head before re-asserting TSTART. We include this patch
> because we have empirically observed multi-minute stalls on this
> hardware; we are willing to drop it if the preference is for
> 1 and 2 to stand alone.
Good idea, but that is what ndo_tx_timeout() is meant for no? It is a
mechanism that is not specific to our HW so that should be implemented
at the subsystem level, and it looks like it already is. :-)
We are aware of a few software scheduling races that we plan on fixing.
If your above patches ended up not fixing the issue, you could look
into those.
https://lore.kernel.org/netdev/DHIT9TPJQJ46.21A89R5UAFXVH@bootlin.com/
Thanks!
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v4 1/7] dt-bindings: clock: qcom: Add video clock controller on Qualcomm Eliza SoC
From: Taniya Das @ 2026-05-14 10:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Maxime Coquelin,
Alexandre Torgue, Luca Weiss, Ajit Pandey, Imran Shaik,
Jagadeesh Kona, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel
In-Reply-To: <20260514-obedient-crouching-bulldog-7d63a8@quoll>
On 5/14/2026 3:48 PM, Krzysztof Kozlowski wrote:
> On Wed, May 13, 2026 at 08:57:36PM +0530, Taniya Das wrote:
>> Eliza Video clock controller is on CX and MX rails similar to Milos.
>> Add compatible string for Eliza video clock controller to the existing
>> Milos videocc binding and add the dt-bindings header for Eliza.
>>
>> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> ---
>> .../bindings/clock/qcom,milos-videocc.yaml | 9 ++++--
>> include/dt-bindings/clock/qcom,eliza-videocc.h | 37 ++++++++++++++++++++++
>> 2 files changed, 44 insertions(+), 2 deletions(-)
>
> Please allow others to actually review your patchsets. You sent v4
> immediately without waiting for v3 review.... which did happen. So now
> please address somewhere comments for v3.
>
I have put a comment to ignore the v3 patch comment as I had missed
change in clock node, but for sure will address the v3 comments as well.
--
Thanks,
Taniya Das
^ permalink raw reply
* Re: [PATCH v5 0/3] Switch Arm CCA to use an auxiliary device instead of a platform device
From: Greg KH @ 2026-05-14 10:19 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
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: <20260514094030.42495-1-aneesh.kumar@kernel.org>
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.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v4 1/7] dt-bindings: clock: qcom: Add video clock controller on Qualcomm Eliza SoC
From: Krzysztof Kozlowski @ 2026-05-14 10:18 UTC (permalink / raw)
To: Taniya Das
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, Maxime Coquelin,
Alexandre Torgue, Luca Weiss, Ajit Pandey, Imran Shaik,
Jagadeesh Kona, linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel
In-Reply-To: <20260513-eliza_mm_cc_v2-v4-1-e61b5434e8d9@oss.qualcomm.com>
On Wed, May 13, 2026 at 08:57:36PM +0530, Taniya Das wrote:
> Eliza Video clock controller is on CX and MX rails similar to Milos.
> Add compatible string for Eliza video clock controller to the existing
> Milos videocc binding and add the dt-bindings header for Eliza.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
> .../bindings/clock/qcom,milos-videocc.yaml | 9 ++++--
> include/dt-bindings/clock/qcom,eliza-videocc.h | 37 ++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 2 deletions(-)
Please allow others to actually review your patchsets. You sent v4
immediately without waiting for v3 review.... which did happen. So now
please address somewhere comments for v3.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/3] dt-bindings: PCI: imx6q-pcie: Add intr, aer and pme interrupts
From: Krzysztof Kozlowski @ 2026-05-14 10:16 UTC (permalink / raw)
To: Richard Zhu
Cc: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
kwilczynski, mani, s.hauer, kernel, festevam, linux-pci,
linux-arm-kernel, devicetree, imx, linux-kernel
In-Reply-To: <20260513025101.1498104-2-hongxing.zhu@nxp.com>
On Wed, May 13, 2026 at 10:50:59AM +0800, Richard Zhu wrote:
> Add optional interrupt entries to the i.MX6Q PCIe binding to support
Describe hardware, not "binding".
> event-based interrupt handling:
Same questions as last time.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 3/6] dt-bindings: arm-smmu: Update the description for Glymur GPU SMMU
From: Krzysztof Kozlowski @ 2026-05-14 10:14 UTC (permalink / raw)
To: Akhil P Oommen
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Rob Clark, Sean Paul, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Will Deacon, Robin Murphy, Joerg Roedel,
linux-arm-msm, devicetree, linux-kernel, dri-devel, freedreno,
linux-arm-kernel, iommu
In-Reply-To: <20260513-glymur-gpu-dt-v4-3-f83832c3bc9a@oss.qualcomm.com>
On Wed, May 13, 2026 at 12:51:20AM +0530, Akhil P Oommen wrote:
> Add the interconnects property to the common SMMU properties and extend
> the sm8750 clock description section to also cover Glymur since it uses
> the same single "hlos" vote clock.
>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 2/6] dt-bindings: display/msm: gpu: Document Adreno X2-185
From: Krzysztof Kozlowski @ 2026-05-14 10:13 UTC (permalink / raw)
To: Akhil P Oommen
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Rob Clark, Sean Paul, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Will Deacon, Robin Murphy, Joerg Roedel,
linux-arm-msm, devicetree, linux-kernel, dri-devel, freedreno,
linux-arm-kernel, iommu
In-Reply-To: <20260513-glymur-gpu-dt-v4-2-f83832c3bc9a@oss.qualcomm.com>
On Wed, May 13, 2026 at 12:51:19AM +0530, Akhil P Oommen wrote:
> Adreno X2-185 GPU found in Glymur chipsets belongs to the A8x family.
> It features a new slice architecture with 4 slices, significantly higher
> bandwidth throughput compared to mobile counterparts, raytracing support,
> and the highest GPU Fmax seen so far on an Adreno GPU (1850 Mhz), among
> other improvements. Update the dt bindings documentation to describe this
> GPU.
>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/display/msm/gpu.yaml | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
Someone needs to start applying Adreno GPU patches...
https://lore.kernel.org/all/20260301142033.88851-2-krzysztof.kozlowski@oss.qualcomm.com/
>
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> index 04b2328903ca..e67cd708dda2 100644
> --- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> @@ -411,6 +411,22 @@ allOf:
> - clocks
> - clock-names
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,adreno-44070001
> + then:
> + properties:
> + reg:
> + minItems: 2
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: kgsl_3d0_reg_memory
> + - const: cx_mem
Are you sure there is no cx_dbgc? If not, then just minItems+maxItems
like for 'reg:'.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 4/4] arm64: dts: mediatek: mt8195-cherry: Sort top level nodes correctly
From: Chen-Yu Tsai @ 2026-05-14 10:12 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-mediatek, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260514101254.2749300-1-wenst@chromium.org>
The thermistor device nodes were added before the vbus regulator and
reserved memory nodes, when they should be after them, based on
alphabetical order of the device node _name_.
Move them to the correct position. No functional changes intended.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
.../boot/dts/mediatek/mt8195-cherry.dtsi | 94 +++++++++----------
1 file changed, 47 insertions(+), 47 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
index 538c46ada32b..ef7afc436aef 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
@@ -149,6 +149,53 @@ ppvar_sys: regulator-ppvar-sys {
regulator-boot-on;
};
+ usb_vbus: regulator-5v0-usb-vbus {
+ compatible = "regulator-fixed";
+ regulator-name = "usb-vbus";
+ enable-active-high;
+ regulator-always-on;
+ vin-supply = <&pp5000_s5>;
+ };
+
+ reserved_memory: reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ scp_mem: memory@50000000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0x50000000 0 0x2900000>;
+ no-map;
+ };
+
+ adsp_mem: memory@60000000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0x60000000 0 0xd80000>;
+ no-map;
+ };
+
+ afe_mem: memory@60d80000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0x60d80000 0 0x100000>;
+ no-map;
+ };
+
+ adsp_device_mem: memory@60e80000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0x60e80000 0 0x280000>;
+ no-map;
+ };
+ };
+
+ spk_amplifier: rt1019p {
+ compatible = "realtek,rt1019p";
+ label = "rt1019p";
+ #sound-dai-cells = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&rt1019p_pins_default>;
+ sdb-gpios = <&pio 100 GPIO_ACTIVE_HIGH>;
+ };
+
/* Murata NCP03WF104F05RL */
tboard_thermistor1: thermal-sensor-t1 {
compatible = "generic-adc-thermal";
@@ -219,53 +266,6 @@ tboard_thermistor2: thermal-sensor-t2 {
120000 51
125000 44>;
};
-
- usb_vbus: regulator-5v0-usb-vbus {
- compatible = "regulator-fixed";
- regulator-name = "usb-vbus";
- enable-active-high;
- regulator-always-on;
- vin-supply = <&pp5000_s5>;
- };
-
- reserved_memory: reserved-memory {
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
-
- scp_mem: memory@50000000 {
- compatible = "shared-dma-pool";
- reg = <0 0x50000000 0 0x2900000>;
- no-map;
- };
-
- adsp_mem: memory@60000000 {
- compatible = "shared-dma-pool";
- reg = <0 0x60000000 0 0xd80000>;
- no-map;
- };
-
- afe_mem: memory@60d80000 {
- compatible = "shared-dma-pool";
- reg = <0 0x60d80000 0 0x100000>;
- no-map;
- };
-
- adsp_device_mem: memory@60e80000 {
- compatible = "shared-dma-pool";
- reg = <0 0x60e80000 0 0x280000>;
- no-map;
- };
- };
-
- spk_amplifier: rt1019p {
- compatible = "realtek,rt1019p";
- label = "rt1019p";
- #sound-dai-cells = <0>;
- pinctrl-names = "default";
- pinctrl-0 = <&rt1019p_pins_default>;
- sdb-gpios = <&pio 100 GPIO_ACTIVE_HIGH>;
- };
};
&adsp {
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 3/4] arm64: dts: mediatek: mt8195-cherry: Fix names for EC controlled regulators
From: Chen-Yu Tsai @ 2026-05-14 10:12 UTC (permalink / raw)
To: Matthias Brugger, AngeloGioacchino Del Regno
Cc: Chen-Yu Tsai, linux-mediatek, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260514101254.2749300-1-wenst@chromium.org>
The names currently given to the EC controlled regulators do not match
what is used in the hardware design.
Fix the names and the labels.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
index ca2bb367ee68..538c46ada32b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
@@ -717,8 +717,8 @@ &mmc1 {
pinctrl-1 = <&mmc1_pins_default>;
sd-uhs-sdr50;
sd-uhs-sdr104;
- vmmc-supply = <&mt_pmic_vmch_ldo_reg>;
- vqmmc-supply = <&mt_pmic_vmc_ldo_reg>;
+ vmmc-supply = <&pp3000_sd>;
+ vqmmc-supply = <&pp3000_vmc_pmu>;
};
&mt6359codec {
@@ -1436,19 +1436,19 @@ i2c_tunnel: i2c-tunnel {
#size-cells = <0>;
};
- mt_pmic_vmc_ldo_reg: regulator@0 {
+ pp3000_vmc_pmu: regulator@0 {
compatible = "google,cros-ec-regulator";
reg = <0>;
- regulator-name = "mt_pmic_vmc_ldo";
+ regulator-name = "pp3000_vmc_pmu";
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <3600000>;
vin-supply = <&pp4200_z2>;
};
- mt_pmic_vmch_ldo_reg: regulator@1 {
+ pp3000_sd: regulator@1 {
compatible = "google,cros-ec-regulator";
reg = <1>;
- regulator-name = "mt_pmic_vmch_ldo";
+ regulator-name = "pp3000_sd";
regulator-min-microvolt = <2700000>;
regulator-max-microvolt = <3600000>;
vin-supply = <&pp4200_z2>;
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
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