* Re: [PATCH v4 01/13] dma-direct: swiotlb: handle swiotlb alloc/free outside __dma_direct_alloc_pages
From: Aneesh Kumar K.V @ 2026-05-14 4:54 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <agSDPJMZkcI-uH8f@google.com>
Mostafa Saleh <smostafa@google.com> writes:
> On Tue, May 12, 2026 at 02:33:56PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Move swiotlb allocation out of __dma_direct_alloc_pages() and handle it in
>> dma_direct_alloc() / dma_direct_alloc_pages().
>>
>> This is needed for follow-up changes that simplify the handling of
>> memory encryption/decryption based on the DMA attribute flags.
>>
>> swiotlb backing pages are already mapped decrypted by
>> swiotlb_update_mem_attributes() and rmem_swiotlb_device_init(), so
>> dma-direct should not call dma_set_decrypted() on allocation nor
>> dma_set_encrypted() on free for swiotlb-backed memory.
>>
>> Update alloc/free paths to detect swiotlb-backed pages and skip
>> encrypt/decrypt transitions for those paths. Keep the existing highmem
>> rejection in dma_direct_alloc_pages() for swiotlb allocations.
>>
>> Only for "restricted-dma-pool", we currently set `for_alloc = true`, while
>> rmem_swiotlb_device_init() decrypts the whole pool up front. This pool is
>> typically used together with "shared-dma-pool", where the shared region is
>> accessed after remap/ioremap and the returned address is suitable for
>> decrypted memory access. So existing code paths remain valid.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> kernel/dma/direct.c | 44 +++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index ec887f443741..b958f150718a 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -125,9 +125,6 @@ static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
>>
>> WARN_ON_ONCE(!PAGE_ALIGNED(size));
>>
>> - if (is_swiotlb_for_alloc(dev))
>> - return dma_direct_alloc_swiotlb(dev, size);
>> -
>> gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
>> page = dma_alloc_contiguous(dev, size, gfp);
>> if (page) {
>> @@ -204,6 +201,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>> {
>> bool remap = false, set_uncached = false;
>> + bool mark_mem_decrypt = true;
>> struct page *page;
>> void *ret;
>>
>> @@ -250,11 +248,21 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> + if (is_swiotlb_for_alloc(dev)) {
>> + page = dma_direct_alloc_swiotlb(dev, size);
>> + if (page) {
>> + mark_mem_decrypt = false;
>> + goto setup_page;
>> + }
>> + return NULL;
>> + }
>> +
>> /* we always manually zero the memory once we are done */
>> page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
>> if (!page)
>> return NULL;
>>
>> +setup_page:
>> /*
>> * dma_alloc_contiguous can return highmem pages depending on a
>> * combination the cma= arguments and per-arch setup. These need to be
>> @@ -281,7 +289,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> goto out_free_pages;
>> } else {
>> ret = page_address(page);
>> - if (dma_set_decrypted(dev, ret, size))
>> + if (mark_mem_decrypt && dma_set_decrypted(dev, ret, size))
>
> I am ok with that approach, but Jason was mentioning we shouldn’t
> special case swiotlb and make the allocator return the memory state
> (similar to the dma_page [1]) . I am also OK if you want to merge that
> part of my series with is.
>
> [1] https://lore.kernel.org/linux-iommu/20260408194750.2280873-1-smostafa@google.com/
>
I was not sure whether we need dma_page. As shown in this series, we can
simplify the allocation and free paths without adding new abstractions
like dma_page.
>
>> goto out_leak_pages;
>> }
>>
>> @@ -298,7 +306,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> return ret;
>>
>> out_encrypt_pages:
>> - if (dma_set_encrypted(dev, page_address(page), size))
>> + if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page), size))
>> return NULL;
>> out_free_pages:
>> __dma_direct_free_pages(dev, page, size);
>> @@ -310,6 +318,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> void dma_direct_free(struct device *dev, size_t size,
>> void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
>> {
>> + bool mark_mem_encrypted = true;
>> unsigned int page_order = get_order(size);
>>
>> if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> @@ -338,12 +347,15 @@ void dma_direct_free(struct device *dev, size_t size,
>> dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
>> return;
>>
>> + if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
>> + mark_mem_encrypted = false;
>> +
>> if (is_vmalloc_addr(cpu_addr)) {
>> vunmap(cpu_addr);
>> } else {
>> if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
>> arch_dma_clear_uncached(cpu_addr, size);
>> - if (dma_set_encrypted(dev, cpu_addr, size))
>> + if (mark_mem_encrypted && dma_set_encrypted(dev, cpu_addr, size))
>> return;
>> }
>>
>> @@ -359,6 +371,19 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>> if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> + if (is_swiotlb_for_alloc(dev)) {
>> + page = dma_direct_alloc_swiotlb(dev, size);
>> + if (!page)
>> + return NULL;
>> +
>> + if (PageHighMem(page)) {
>
> My understanding is that rmem_swiotlb_device_init() asserts that there
> is no PageHighMem()? Also a similar check doesn’t exist in
> dma_direct_alloc().
>
The reason I added that HighMem check is that __dma_direct_alloc_pages()
already has that check.
page = dma_alloc_contiguous(dev, size, gfp);
if (page) {
if (dma_coherent_ok(dev, page_to_phys(page), size) &&
(allow_highmem || !PageHighMem(page)))
return page;
dma_free_contiguous(dev, page, size);
}
I understand that the current usage of swiotlb alloc is restricted to
restricted memory, and it will not return HighMem pages. I will drop
this hunk from the patch.
-aneesh
^ permalink raw reply
* Re: [PATCH RFC] iommu: Enable per-device SSID space for SVA
From: Joonwon Kang @ 2026-05-14 4:12 UTC (permalink / raw)
To: jgg
Cc: Alexander.Grest, amhetre, baolu.lu, easwar.hariharan, iommu,
jacob.jun.pan, joonwonkang, joro, jpb, kees, kevin.tian,
linux-arm-kernel, linux-kernel, nicolinc, praan, robin.murphy,
smostafa, will
In-Reply-To: <20260513171059.GP7702@ziepe.ca>
> On Wed, May 13, 2026 at 05:03:33PM +0000, Joonwon Kang wrote:
> > > On Tue, May 12, 2026 at 02:51:38PM +0000, Joonwon Kang wrote:
> > >
> > > > Appreciate all your clarifications here. So, my understanding is that if
> > > > our system does not support ST64BV and ST64BV0 or if our device does not
> > > > distinguish between the posted write and the non-posted write regarding
> > > > PASID, then we can lift the use of the global PASID space. Can I say this?
> > >
> > > You should do what Robin said - just have your driver use a per-device
> > > PASID that it allocates and never use the global pasid allocator.
> > >
> > > To do this lightly re-organize the SVA code so the driver can supply
> > > its own PASID, and in this mode we wouldn't activate the ENQCMD
> > > features in the mm.
> >
> > Ah, we could actively disallow EL0 to execute ENQCMD-like instructions
> > when the device driver explicitly shows the intention via a new API like
> > `iommu_sva_bind_device_pasid()` that Tian mentioned earlier.
>
> You shouldn't need to do anything like this.
>
> All you need is to ensure that mm_get_enqcmd_pasid() returns
> IOMMU_PASID_INVALID so long as a the normal iommu_sva_bind_device()
> hasn't been called. Once it is called it is fine to allow the ENQCMD.
>
> Your new iommu_sva_bind_device_pasid() needs to establish the SVA and
> attach it without triggering mm_get_enqcmd_pasid().
>
> The arch code is required to block the ENQCMD like instructions when
> IOMMU_PASID_INVALID.
>
> Devices that can mmap an ENQCMD sensitive BAR region must not do so
> unless iommu_sva_bind_device() has been called.
>
Yes, the basic idea I meant is the same as this: use `mm->iommu` to
deactivate the instructions. The arch code for ARM may block them later
as it does not seem to support FEAT_LS64_ACCDATA for now. Thanks for the
details. I think I am pretty much aligned. Let me try it.
> > To allocate a per-device PASID, I think we should do it using
> > `dev->iommu_group->pasid_array` instead of making the device driver
>
> No, make the driver manage this, don't mess with the core code. PASID
> isn't supported with multi-device groups already.
Understood. Thanks for this info.
Thanks,
Joonwon Kang
^ permalink raw reply
* Re: [PATCH] drivers: altera_edac: Guard SDRAM irq2 retrieval for Arria10 only
From: Nazle Asmade, Muhammad Nazim Amirul @ 2026-05-14 3:42 UTC (permalink / raw)
To: Dinh Nguyen, bp@alien8.de, tony.luck@intel.com
Cc: linux-edac@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <b88eb1df-6841-43b8-990d-506c8c4f2d4f@kernel.org>
On 13/5/2026 6:45 pm, Dinh Nguyen wrote:
>
>
> On 5/12/26 06:51, Nazle Asmade, Muhammad Nazim Amirul wrote:
>> On 12/5/2026 7:25 pm, Dinh Nguyen wrote:
>>>
>>>
>>> On 5/11/26 20:37, Nazle Asmade, Muhammad Nazim Amirul wrote:
>>>> On 11/5/2026 7:54 pm, Dinh Nguyen wrote:
>>>>>
>>>>>
>>>>> On 5/8/26 02:52, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
>>>>>> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>>>>>>
>>>>>> Guard the irq2 retrieval with an of_machine_is_compatible() check so
>>>>>> that platform_get_irq(pdev, 1) is only called on Arria10 platforms.
>>>>>>
>>>>>> Signed-off-by: Nazim Amirul
>>>>>> <muhammad.nazim.amirul.nazle.asmade@altera.com>
>>>>>> Signed-off-by: Niravkumar L Rabara <nirav.rabara@altera.com>
>>>>>> ---
>>>>>> drivers/edac/altera_edac.c | 3 ++-
>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
>>>>>> index 4edd2088c2db..b30302198cd4 100644
>>>>>> --- a/drivers/edac/altera_edac.c
>>>>>> +++ b/drivers/edac/altera_edac.c
>>>>>> @@ -348,7 +348,8 @@ static int altr_sdram_probe(struct
>>>>>> platform_device
>>>>>> *pdev)
>>>>>> }
>>>>>> /* Arria10 has a 2nd IRQ */
>>>>>> - irq2 = platform_get_irq(pdev, 1);
>>>>>> + if (of_machine_is_compatible("altr,socfpga-arria10"))
>>>>>> + irq2 = platform_get_irq(pdev, 1);
>>>>>> layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
>>>>>> layers[0].size = 1;
>>>>>
>>>>> Why? We already switch on arria10 later in the same function.
>>>>>
>>>>> Sorry, but NAK.
>>>>>
>>>>> Dinh
>>>> This driver were used by cyclone5 and arria10. Cyclone5 only has one
>>>> interrupt whereby arria10 has 2 interrupt. That is the reason why the
>>>> interrupt was guard by (of_machine_is_compatible("altr,socfpga-
>>>> arria10"))
>>>>
>>>
>>> Yes, but look at line 397,
>>>
>>> /* Only the Arria10 has separate IRQs */
>>> if (of_machine_is_compatible("altr,socfpga-arria10")) {
>>> /* Arria10 specific initialization */
>>>
>>> Dinh
>>>
>>>
>> Hi Dinh, That is true, but the one that we looking at now is at line 352
>> which enabling the second interrupt and it is not required by cyclone5.
>> Perhaps are you saying we should move the irq2 at line 352 under this
>> line 397?
>
> Yes, that would be fine.
>
> Dinh
>
changes applied on v2!
https://lore.kernel.org/all/20260514034007.11541-1-muhammad.nazim.amirul.nazle.asmade@altera.com/
regards,
Nazim
^ permalink raw reply
* [PATCH v2] drivers: altera_edac: Guard SDRAM irq2 retrieval for Arria10 only
From: muhammad.nazim.amirul.nazle.asmade @ 2026-05-14 3:40 UTC (permalink / raw)
To: dinguyen, bp, tony.luck; +Cc: linux-edac, linux-arm-kernel, linux-kernel
From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Guard the irq2 retrieval with an of_machine_is_compatible() check so
that platform_get_irq(pdev, 1) is only called on Arria10 platforms.
Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
v2: Move irq2 = platform_get_irq(pdev, 1) inside the existing
of_machine_is_compatible("altr,socfpga-arria10") block instead of
adding a separate duplicate guard around it.
---
drivers/edac/altera_edac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 4edd2088c2db..ee6ced033f2c 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -347,9 +347,6 @@ static int altr_sdram_probe(struct platform_device *pdev)
return irq;
}
- /* Arria10 has a 2nd IRQ */
- irq2 = platform_get_irq(pdev, 1);
-
layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
layers[0].size = 1;
layers[0].is_virt_csrow = true;
@@ -395,6 +392,9 @@ static int altr_sdram_probe(struct platform_device *pdev)
/* Only the Arria10 has separate IRQs */
if (of_machine_is_compatible("altr,socfpga-arria10")) {
+ /* Arria10 has a 2nd IRQ */
+ irq2 = platform_get_irq(pdev, 1);
+
/* Arria10 specific initialization */
res = a10_init(mc_vbase);
if (res < 0)
--
2.43.7
^ permalink raw reply related
* [PATCH v1] ARM: imx3: Fix CCM node reference leak
From: Yuho Choi @ 2026-05-14 3:40 UTC (permalink / raw)
To: Russell King, Frank Li
Cc: Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
linux-arm-kernel, imx, linux-kernel, Yuho Choi
of_find_compatible_node() returns a referenced device node. The i.MX31
and i.MX35 early init paths use the node to map the CCM registers with
of_iomap(), but never drop the node reference.
Release the node after the mapping is created.
Fixes: 2cf98d12958c ("ARM: imx3: Retrieve the CCM base address from devicetree")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
arch/arm/mach-imx/mm-imx3.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-imx/mm-imx3.c b/arch/arm/mach-imx/mm-imx3.c
index 0788c5cc7f9e..9b0b014d7fe2 100644
--- a/arch/arm/mach-imx/mm-imx3.c
+++ b/arch/arm/mach-imx/mm-imx3.c
@@ -106,6 +106,7 @@ void __init imx31_init_early(void)
arm_pm_idle = imx31_idle;
np = of_find_compatible_node(NULL, NULL, "fsl,imx31-ccm");
mx3_ccm_base = of_iomap(np, 0);
+ of_node_put(np);
BUG_ON(!mx3_ccm_base);
}
#endif /* ifdef CONFIG_SOC_IMX31 */
@@ -143,6 +144,7 @@ void __init imx35_init_early(void)
arch_ioremap_caller = imx3_ioremap_caller;
np = of_find_compatible_node(NULL, NULL, "fsl,imx35-ccm");
mx3_ccm_base = of_iomap(np, 0);
+ of_node_put(np);
BUG_ON(!mx3_ccm_base);
}
#endif /* ifdef CONFIG_SOC_IMX35 */
--
2.43.0
^ permalink raw reply related
* [PATCH v2] iio: adc: sun20i-gpadc: support non-contiguous channel lookups
From: Michal Piekos @ 2026-05-14 3:19 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
Cc: linux-iio, linux-arm-kernel, linux-sunxi, linux-kernel,
Michal Piekos, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, llvm
Using consumer driver like iio-hwmon which resolve channels thorugh
io-channels phandles will fail for sparse channels because IIO core by
default threats phandle argument as index into channel array.
eg. <&gpadc 1> will fail if there is only channel@1 specified
Add .fwnode_xlate() which maps DT phandle to the registered channel
whose chan->channel matches the hardware channel number. It allows
sparse channel maps to be consumed by drivers like iio-hwmon.
Tested on Radxa Cubie A5E.
Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
Changes in v2:
- Move loop variable declaration into the for statement
- Fix indentation using clang-format
- Correct commit wording
- Link to v1: https://patch.msgid.link/20260513-fix-sunxi-gpadc-sparse-channels-v1-1-6c21e290bcee@mmpsystems.pl
To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Chen-Yu Tsai <wens@kernel.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Nathan Chancellor <nathan@kernel.org>
To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
To: Bill Wendling <morbo@google.com>
To: Justin Stitt <justinstitt@google.com>
Cc: linux-iio@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: llvm@lists.linux.dev
---
drivers/iio/adc/sun20i-gpadc-iio.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
index 861c14da75ad..78c9a52f38df 100644
--- a/drivers/iio/adc/sun20i-gpadc-iio.c
+++ b/drivers/iio/adc/sun20i-gpadc-iio.c
@@ -139,8 +139,20 @@ static irqreturn_t sun20i_gpadc_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+static int
+sun20i_gpadc_fwnode_xlate(struct iio_dev *indio_dev,
+ const struct fwnode_reference_args *iiospec)
+{
+ for (unsigned int i = 0; i < indio_dev->num_channels; i++)
+ if (indio_dev->channels[i].channel == iiospec->args[0])
+ return i;
+
+ return -EINVAL;
+}
+
static const struct iio_info sun20i_gpadc_iio_info = {
.read_raw = sun20i_gpadc_read_raw,
+ .fwnode_xlate = sun20i_gpadc_fwnode_xlate,
};
static void sun20i_gpadc_reset_assert(void *data)
---
base-commit: 1d5dcaa3bd65f2e8c9baa14a393d3a2dc5db7524
change-id: 20260513-fix-sunxi-gpadc-sparse-channels-2b6b2063bd49
Best regards,
--
Michal Piekos <michal.piekos@mmpsystems.pl>
^ permalink raw reply related
* [PATCH v2 2/2] ARM: dts: aspeed: Add ASRock Rack B650D4U BMC
From: Prasanth Kumar Padarthi @ 2026-05-14 3:16 UTC (permalink / raw)
To: joel, andrew
Cc: robh, krzk+dt, conor+dt, devicetree, linux-aspeed,
linux-arm-kernel, Prasanth Kumar Padarthi
In-Reply-To: <20260514031622.1416922-1-prasanth.padarthi10@gmail.com>
Add initial device tree support for the ASRock Rack B650D4U BMC.
The B650D4U is a server motherboard utilizing the ASPEED AST2600
SoC for management.
Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
arch/arm/boot/dts/aspeed/Makefile | 1 +
.../dts/aspeed/aspeed-bmc-asrock-b650d4u.dts | 71 +++++++++++++++++++
2 files changed, 72 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts
diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
index c4f064e4b..124d4f8f8 100644
--- a/arch/arm/boot/dts/aspeed/Makefile
+++ b/arch/arm/boot/dts/aspeed/Makefile
@@ -13,6 +13,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-asrock-romed8hm3.dtb \
aspeed-bmc-asrock-spc621d8hm3.dtb \
aspeed-bmc-asrock-x570d4u.dtb \
+ aspeed-bmc-asrock-b650d4u.dtb \
aspeed-bmc-asus-x4tf.dtb \
aspeed-bmc-bytedance-g220a.dtb \
aspeed-bmc-delta-ahe50dc.dtb \
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts
new file mode 100644
index 000000000..130b7f3e0
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "aspeed-g6.dtsi"
+
+/ {
+ model = "ASRock Rack B650D4U BMC";
+ compatible = "asrock,b650d4u-bmc", "aspeed,ast2600";
+
+ aliases {
+ serial4 = &uart5;
+ };
+
+ chosen {
+ stdout-path = "serial4:115200n8";
+ };
+
+ memory@80000000 {
+ device_type = "memory";
+ reg = <0x80000000 0x40000000>;
+ };
+};
+
+/* BMC Console UART */
+&uart5 {
+ status = "okay";
+};
+
+/* SPI Flash Management */
+&fmc {
+ status = "okay";
+ flash@0 {
+ status = "okay";
+ m25p,fast-read;
+ label = "bmc";
+ };
+};
+
+/* Dedicated Management LAN */
+&mdio0 {
+ status = "okay";
+
+ ethphy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+};
+
+&mac0 {
+ status = "okay";
+ phy-mode = "rgmii-rxid";
+ phy-handle = <ðphy0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
+};
+
+/* I2C Bus for FRU/EEPROM Storage */
+&i2c7 {
+ status = "okay";
+ eeprom@57 {
+ compatible = "atmel,24c02";
+ reg = <0x57>;
+ pagesize = <16>;
+ };
+};
+
+/* System Watchdog */
+&wdt1 {
+ status = "okay";
+ aspeed,reset-type = "soc";
+};
--
2.47.3
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: arm: aspeed: Add ASRock Rack B650D4U
From: Prasanth Kumar Padarthi @ 2026-05-14 3:16 UTC (permalink / raw)
To: joel, andrew
Cc: robh, krzk+dt, conor+dt, devicetree, linux-aspeed,
linux-arm-kernel, Prasanth Kumar Padarthi
In-Reply-To: <20260514031622.1416922-1-prasanth.padarthi10@gmail.com>
Add the compatible string for the ASRock Rack B650D4U BMC,
which is an AST2600-based server motherboard.
Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml b/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml
index 2f92b8ab0..656397850 100644
--- a/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml
+++ b/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml
@@ -77,6 +77,7 @@ properties:
- ampere,mtmitchell-bmc
- aspeed,ast2600-evb
- aspeed,ast2600-evb-a1
+ - asrock,b650d4u-bmc
- asus,x4tf-bmc
- facebook,bletchley-bmc
- facebook,catalina-bmc
--
2.47.3
^ permalink raw reply related
* [PATCH v2 0/2] ARM: dts: aspeed: Add ASRock Rack B650D4U support
From: Prasanth Kumar Padarthi @ 2026-05-14 3:16 UTC (permalink / raw)
To: joel, andrew
Cc: robh, krzk+dt, conor+dt, devicetree, linux-aspeed,
linux-arm-kernel, Prasanth Kumar Padarthi
This series adds initial device tree support for the ASRock Rack
B650D4U BMC, which is based on the ASPEED AST2600 SoC.
The B650D4U is a server motherboard for AMD Ryzen processors. The initial
DTS describes the BMC console, FMC flash, dedicated management Ethernet,
FRU EEPROM, and watchdog.
The dedicated management port is described as MAC0 connected to an external
Clause 22 PHY on MDIO0 at address 0. The MAC uses RGMII with RX internal
delay, matching the vendor DTB description.
Tested:
- Built aspeed-bmc-asrock-b650d4u.dtb
- Ran dtbs_check for the board/aspeed binding
- QEMU boot smoke-tested with ast2600-evb
Changes in v2:
- Added MDIO0 PHY node for the dedicated management port.
- Set mac0 phy-mode to rgmii-rxid and added phy-handle.
- Removed unused GPIO include from the DTS.
Prasanth Kumar Padarthi (2):
dt-bindings: arm: aspeed: Add ASRock Rack B650D4U
ARM: dts: aspeed: Add ASRock Rack B650D4U BMC
.../bindings/arm/aspeed/aspeed.yaml | 1 +
arch/arm/boot/dts/aspeed/Makefile | 1 +
.../dts/aspeed/aspeed-bmc-asrock-b650d4u.dts | 71 +++++++++++++++++++
3 files changed, 73 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts
--
2.47.3
^ permalink raw reply
* Re: [PATCH RFC 8/8] clk: sunxi-ng: a733: Add reset lines
From: Junhui Liu @ 2026-05-14 2:41 UTC (permalink / raw)
To: Andre Przywara, Junhui Liu
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Richard Cochran, linux-clk, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, linux-riscv, netdev
In-Reply-To: <20260514012226.691ae185@ryzen.lan>
Hi Andre,
Thanks for your review.
On Thu May 14, 2026 at 7:22 AM CST, Andre Przywara wrote:
> On Tue, 10 Mar 2026 16:34:01 +0800
> Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>
> Hi,
>
> compare the list below against my version of the manual. You list more
> than shown there, can you say where those extra reset bits come from?
They are from the vendor BSP, I will add comments to explain them in the
next version.
>
>> Add the reset lines for the Allwinner A733 SoC. These reset control bits
>> are integrated into the Bus Gate Reset (BGR) registers, typically
>> sharing the same register address with their corresponding bus clock
>> gates. Integrate them into the main CCU driver using the existing
>> sunxi-ng ccu_reset framework, allowing the CCU to also function as a
>> reset controller for the SoC.
>>
>> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> ---
>> drivers/clk/sunxi-ng/ccu-sun60i-a733.c | 128 +++++++++++++++++++++++++++++++++
>> 1 file changed, 128 insertions(+)
>>
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> index c0b09f9197d1..7d1ee9235436 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> @@ -17,6 +17,7 @@
>> #include "../clk.h"
>>
>> #include "ccu_common.h"
>> +#include "ccu_reset.h"
>>
>> #include "ccu_div.h"
>> #include "ccu_gate.h"
>> @@ -2169,11 +2170,138 @@ static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
>> .num = CLK_FANOUT3 + 1,
>> };
>>
>> +static struct ccu_reset_map sun60i_a733_ccu_resets[] = {
>> + [RST_BUS_ITS_PCIE] = { 0x574, BIT(16) },
>> + [RST_BUS_NSI] = { 0x580, BIT(30) },
>
> What is this NSI device? Some interconnect? Do we really want to handle
> this reset and clock? Is there some device referencing this? Otherwise
> the kernel will turn at least the clock off, which is probably fatal.
> Also the manual says this is a secure register, so I feel like the
> kernel must not mess with this.
Yes, NSI appears to be a high-speed interconnect. The vendor kernel
provides a sunxi-nsi driver for it accross sun8i, sun55i, and sun60i.
Since the mainline kernel doesn't manage this interconnect for the
already mainlined sun8i and sun55i platforms, I will drop the NSI clocks
and resets in the next version.
>
>> + [RST_BUS_NSI_CFG] = { 0x584, BIT(16) },
>
> Similar here, I guess, though this might not be secure only.
>
>> + [RST_BUS_IOMMU0_SYS] = { 0x58c, BIT(16) },
>> + [RST_BUS_MSI_LITE0_AHB] = { 0x594, BIT(16) },
>> + [RST_BUS_MSI_LITE0_MBUS] = { 0x594, BIT(17) },
>> + [RST_BUS_MSI_LITE1_AHB] = { 0x59c, BIT(16) },
>> + [RST_BUS_MSI_LITE1_MBUS] = { 0x59c, BIT(17) },
>> + [RST_BUS_MSI_LITE2_AHB] = { 0x5a4, BIT(16) },
>> + [RST_BUS_MSI_LITE2_MBUS] = { 0x5a4, BIT(17) },
>> + [RST_BUS_IOMMU1_SYS] = { 0x5b4, BIT(16) },
>> + [RST_BUS_DMA0] = { 0x704, BIT(16) },
>> + [RST_BUS_DMA1] = { 0x70c, BIT(16) },
>> + [RST_BUS_SPINLOCK] = { 0x724, BIT(16) },
>> + [RST_BUS_MSGBOX] = { 0x744, BIT(16) },
>> + [RST_BUS_PWM0] = { 0x784, BIT(16) },
>> + [RST_BUS_PWM1] = { 0x78c, BIT(16) },
>> + [RST_BUS_DBG] = { 0x7a4, BIT(16) },
>> + [RST_BUS_SYSDAP] = { 0x7ac, BIT(16) },
>> + [RST_BUS_TIMER0] = { 0x850, BIT(16) },
>> + [RST_BUS_DE] = { 0xa04, BIT(16) },
>> + [RST_BUS_DI] = { 0xa24, BIT(16) },
>> + [RST_BUS_G2D] = { 0xa44, BIT(16) },
>> + [RST_BUS_EINK] = { 0xa6c, BIT(16) },
>> + [RST_BUS_DE_SYS] = { 0xa74, BIT(16) },
>> + [RST_BUS_VE_ENC] = { 0xa8c, BIT(16) },
>
> The manual calls this ENC0, and since bit 17 is not documented, I
> wonder if there is an ENC1 used with some other packaging, maybe? So
> maybe calling it ENC0 would be safer here.
Okay, I will rename it to ENC0.
>
>> + [RST_BUS_VE_DEC] = { 0xa8c, BIT(18) },
>> + [RST_BUS_CE] = { 0xac4, BIT(16) },
>> + [RST_BUS_CE_SYS] = { 0xac4, BIT(17) },
>> + [RST_BUS_NPU_CORE] = { 0xb04, BIT(16) },
>> + [RST_BUS_NPU_AXI] = { 0xb04, BIT(17) },
>> + [RST_BUS_NPU_AHB] = { 0xb04, BIT(18) },
>> + [RST_BUS_NPU_SRAM] = { 0xb04, BIT(19) },
>> + [RST_BUS_GPU] = { 0xb24, BIT(16) },
>> + [RST_BUS_DRAM] = { 0xc0c, BIT(16) },
>> + [RST_BUS_NAND] = { 0xc8c, BIT(16) },
>> + [RST_BUS_MMC0] = { 0xd0c, BIT(16) },
>> + [RST_BUS_MMC1] = { 0xd1c, BIT(16) },
>> + [RST_BUS_MMC2] = { 0xd2c, BIT(16) },
>> + [RST_BUS_MMC3] = { 0xd3c, BIT(16) },
>> + [RST_BUS_UFS_AHB] = { 0xd8c, BIT(16) },
>> + [RST_BUS_UFS_AXI] = { 0xd8c, BIT(17) },
>> + [RST_BUS_UFS_PHY] = { 0xd8c, BIT(18) },
>> + [RST_BUS_UFS_CORE] = { 0xd8c, BIT(19) },
>> + [RST_BUS_UART0] = { 0xe00, BIT(16) },
>> + [RST_BUS_UART1] = { 0xe04, BIT(16) },
>> + [RST_BUS_UART2] = { 0xe08, BIT(16) },
>> + [RST_BUS_UART3] = { 0xe0c, BIT(16) },
>> + [RST_BUS_UART4] = { 0xe10, BIT(16) },
>> + [RST_BUS_UART5] = { 0xe14, BIT(16) },
>> + [RST_BUS_UART6] = { 0xe18, BIT(16) },
>> + [RST_BUS_I2C0] = { 0xe80, BIT(16) },
>> + [RST_BUS_I2C1] = { 0xe84, BIT(16) },
>> + [RST_BUS_I2C2] = { 0xe88, BIT(16) },
>> + [RST_BUS_I2C3] = { 0xe8c, BIT(16) },
>> + [RST_BUS_I2C4] = { 0xe90, BIT(16) },
>> + [RST_BUS_I2C5] = { 0xe94, BIT(16) },
>> + [RST_BUS_I2C6] = { 0xe98, BIT(16) },
>> + [RST_BUS_I2C7] = { 0xe9c, BIT(16) },
>> + [RST_BUS_I2C8] = { 0xea0, BIT(16) },
>> + [RST_BUS_I2C9] = { 0xea4, BIT(16) },
>> + [RST_BUS_I2C10] = { 0xea8, BIT(16) },
>> + [RST_BUS_I2C11] = { 0xeac, BIT(16) },
>> + [RST_BUS_I2C12] = { 0xeb0, BIT(16) },
>> + [RST_BUS_SPI0] = { 0xf04, BIT(16) },
>> + [RST_BUS_SPI1] = { 0xf0c, BIT(16) },
>> + [RST_BUS_SPI2] = { 0xf14, BIT(16) },
>> + [RST_BUS_SPIF] = { 0xf1c, BIT(16) },
>> + [RST_BUS_SPI3] = { 0xf24, BIT(16) },
>> + [RST_BUS_SPI4] = { 0xf2c, BIT(16) },
>
> SPI4 isn't mentioned in my version of the manual.
>
>> + [RST_BUS_GPADC] = { 0xfc4, BIT(16) },
>> + [RST_BUS_THS] = { 0xfe4, BIT(16) },
>> + [RST_BUS_IRRX] = { 0x1004, BIT(16) },
>> + [RST_BUS_IRTX] = { 0x100c, BIT(16) },
>> + [RST_BUS_LRADC] = { 0x1024, BIT(16) },
>> + [RST_BUS_SGPIO] = { 0x1064, BIT(16) },
>> + [RST_BUS_LPC] = { 0x1084, BIT(16) },
>
> Where do those two come from? There are not in my version of the manual.
>
>> + [RST_BUS_I2SPCM0] = { 0x120c, BIT(16) },
>> + [RST_BUS_I2SPCM1] = { 0x121c, BIT(16) },
>> + [RST_BUS_I2SPCM2] = { 0x122c, BIT(16) },
>> + [RST_BUS_I2SPCM3] = { 0x123c, BIT(16) },
>> + [RST_BUS_I2SPCM4] = { 0x124c, BIT(16) },
>> + [RST_BUS_OWA] = { 0x128c, BIT(16) },
>> + [RST_BUS_DMIC] = { 0x12cc, BIT(16) },
>> + [RST_USB_PHY0] = { 0x1300, BIT(30) },
>> + [RST_BUS_OHCI0] = { 0x1304, BIT(16) },
>> + [RST_BUS_EHCI0] = { 0x1304, BIT(20) },
>> + [RST_BUS_OTG] = { 0x1304, BIT(24) },
>> + [RST_USB_PHY1] = { 0x1308, BIT(30) },
>> + [RST_BUS_OHCI1] = { 0x130c, BIT(16) },
>> + [RST_BUS_EHCI1] = { 0x130c, BIT(20) },
>> + [RST_BUS_USB2] = { 0x135c, BIT(16) },
>> + [RST_BUS_PCIE] = { 0x138c, BIT(17) },
>> + [RST_BUS_PCIE_PWRUP] = { 0x138c, BIT(16) },
>
> Just a nit, but those two are ordered wrongly.
Will do.
>
>> + [RST_BUS_SERDES] = { 0x13c4, BIT(16) },
>> + [RST_BUS_GMAC0] = { 0x141c, BIT(16) },
>> + [RST_BUS_GMAC0_AXI] = { 0x141c, BIT(17) },
>> + [RST_BUS_GMAC1] = { 0x142c, BIT(16) },
>> + [RST_BUS_GMAC1_AXI] = { 0x142c, BIT(17) },
>
> GMAC1 is not listed in my manual, where does this come from?
>
>> + [RST_BUS_TCON_LCD0] = { 0x1504, BIT(16) },
>> + [RST_BUS_TCON_LCD1] = { 0x150c, BIT(16) },
>> + [RST_BUS_TCON_LCD2] = { 0x1514, BIT(16) },
>
> No LCD2 in my manual.
>
> The rest looks alright.
>
> Cheers,
> Andre
>
>> + [RST_BUS_LVDS0] = { 0x1544, BIT(16) },
>> + [RST_BUS_LVDS1] = { 0x154c, BIT(16) },
>> + [RST_BUS_DSI0] = { 0x1584, BIT(16) },
>> + [RST_BUS_DSI1] = { 0x158c, BIT(16) },
>> + [RST_BUS_TCON_TV0] = { 0x1604, BIT(16) },
>> + [RST_BUS_TCON_TV1] = { 0x160c, BIT(16) },
>> + [RST_BUS_EDP] = { 0x164c, BIT(16) },
>> + [RST_BUS_HDMI_MAIN] = { 0x168c, BIT(16) },
>> + [RST_BUS_HDMI_SUB] = { 0x168c, BIT(17) },
>> + [RST_BUS_HDMI_HDCP] = { 0x168c, BIT(18) },
>> + [RST_BUS_DPSS_TOP0] = { 0x16c4, BIT(16) },
>> + [RST_BUS_DPSS_TOP1] = { 0x16cc, BIT(16) },
>> + [RST_BUS_VIDEO_OUT0] = { 0x16e4, BIT(16) },
>> + [RST_BUS_VIDEO_OUT1] = { 0x16ec, BIT(16) },
>> + [RST_BUS_LEDC] = { 0x1704, BIT(16) },
>> + [RST_BUS_DSC] = { 0x1744, BIT(16) },
>> + [RST_BUS_CSI] = { 0x1844, BIT(16) },
>> + [RST_BUS_VIDEO_IN] = { 0x1884, BIT(16) },
>> + [RST_BUS_APB2JTAG] = { 0x1c04, BIT(16) },
>> +};
>> +
>> static const struct sunxi_ccu_desc sun60i_a733_ccu_desc = {
>> .ccu_clks = sun60i_a733_ccu_clks,
>> .num_ccu_clks = ARRAY_SIZE(sun60i_a733_ccu_clks),
>>
>> .hw_clks = &sun60i_a733_hw_clks,
>> +
>> + .resets = sun60i_a733_ccu_resets,
>> + .num_resets = ARRAY_SIZE(sun60i_a733_ccu_resets),
>> };
>>
>> static const u32 pll_regs[] = {
>>
--
Best regards,
Junhui Liu
^ permalink raw reply
* Re: Re: [PATCH net-next v7 2/4] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
From: 李志 @ 2026-05-14 2:25 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, devicetree, davem, edumazet, robh, krzk+dt,
conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
rmk+kernel, pjw, palmer, aou, alex, linux-riscv, linux-stm32,
linux-arm-kernel, linux-kernel, maxime.chevallier, ningyu, linmin,
pinkesh.vaghela, pritesh.patel, weishangjuan, horms
In-Reply-To: <20260512161315.141aba88@kernel.org>
> -----Original Messages-----
> From: "Jakub Kicinski" <kuba@kernel.org>
> Send time:Wednesday, 13/05/2026 07:13:15
> To: 李志 <lizhi2@eswincomputing.com>
> Cc: andrew+netdev@lunn.ch, devicetree@vger.kernel.org, davem@davemloft.net, edumazet@google.com, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, netdev@vger.kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr, linux-riscv@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maxime.chevallier@bootlin.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, pritesh.patel@einfochips.com, weishangjuan@eswincomputing.com, horms@kernel.org
> Subject: Re: [PATCH net-next v7 2/4] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
>
> On Tue, 12 May 2026 13:39:12 +0800 (GMT+08:00) 李志 wrote:
> > For the eth1 enablement part, my current understanding is that it
> > should be treated as a new independent v1 series for net-next,
> > since the scope and target tree have changed after the split.
> >
> > Would you prefer this eth1 series to start as v1, or should it
> > continue as v8 for continuity with the original series?
>
> v8 is better, but is the fix in net-next already?
>
> If this is the posting you're referring to:
> https://lore.kernel.org/all/20260507083214.192-1-lizhi2@eswincomputing.com/
> it has been dropped based on feedback from Maxime and I don't see a v2.
Yes, the eth0 fixes have now been reposted as a dedicated series targeting net:
https://lore.kernel.org/lkml/20260507083037.152-1-lizhi2@eswincomputing.com/
The previous net-next eth1-related posting was dropped following your guidance
to first complete the eth0 fixes in net before continuing with eth1. I will
keep eth1 as v8 as previously suggested.
For the eth0 net series, I will split the remaining changes in v2 as suggested
by Maxime to improve reviewability and bisectability, and resend shortly.
Once that is completed, I will proceed with the eth1 enablement series (v8)
based on the updated context.
Thanks,
Zhi
^ permalink raw reply
* Re: [net-next v8 2/3] net: ethernet: mtk_eth_soc: Add RSS support
From: Jakub Kicinski @ 2026-05-14 1:56 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Felix Fietkau, Lorenzo Bianconi, Andrew Lunn, David S. Miller,
Eric Dumazet, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Frank Wunderlich,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
Mason Chang, Daniel Golle
In-Reply-To: <20260509190938.169290-3-linux@fw-web.de>
On Sat, 9 May 2026 21:09:31 +0200 Frank Wunderlich wrote:
> From: Mason Chang <mason-cw.chang@mediatek.com>
>
> Add support for Receive Side Scaling.
>
> We can adjust SMP affinity with the following command:
> echo [CPU bitmap num] > /proc/irq/[virtual IRQ ID]/smp_affinity,
> with interrupts evenly assigned to 4 CPUs, we were able to measure
> an RX throughput of 7.3Gbps using iperf3 on the MT7988. Further
> optimizations will be carried out in the future.
Would be great to split this up a little more for ease of review.
> +static int mtk_rss_init(struct mtk_eth *eth)
> +{
> + const struct mtk_soc_data *soc = eth->soc;
> + const struct mtk_reg_map *reg_map = eth->soc->reg_map;
> + struct mtk_rss_params *rss_params = ð->rss_params;
reverse xmas tree should be followed, please fix everywhere in this
submission
> + u32 val;
> + int i;
> +
> + netdev_rss_key_fill(rss_params->hash_key, MTK_RSS_HASH_KEYSIZE);
> +
> + for (i = 0; i < MTK_RSS_MAX_INDIRECTION_TABLE; i++)
> + rss_params->indirection_table[i] = ethtool_rxfh_indir_default(i, eth->soc->rss_num);
> +
> + if (soc->rx.desc_size == sizeof(struct mtk_rx_dma)) {
> + /* Set RSS rings to PSE modes */
> + for (i = 1; i <= MTK_HW_LRO_RING_NUM(eth); i++) {
> + val = mtk_r32(eth, MTK_LRO_CTRL_DW2_CFG(reg_map, i));
> + val |= MTK_RING_PSE_MODE;
> + mtk_w32(eth, val, MTK_LRO_CTRL_DW2_CFG(reg_map, i));
> + }
> +
> + /* Enable non-lro multiple rx */
> + val = mtk_r32(eth, reg_map->pdma.lro_ctrl_dw0);
> + val |= MTK_NON_LRO_MULTI_EN;
> + mtk_w32(eth, val, reg_map->pdma.lro_ctrl_dw0);
> +
> + /* Enable RSS dly int supoort */
> + val |= MTK_LRO_DLY_INT_EN;
> + mtk_w32(eth, val, reg_map->pdma.lro_ctrl_dw0);
> + }
> +
> + /* Hash Type */
> + val = mtk_r32(eth, reg_map->pdma.rss_glo_cfg);
> + val |= MTK_RSS_IPV4_STATIC_HASH;
> + val |= MTK_RSS_IPV6_STATIC_HASH;
> + mtk_w32(eth, val, reg_map->pdma.rss_glo_cfg);
> +
> + /* Hash Key */
> + for (i = 0; i < MTK_RSS_HASH_KEYSIZE / sizeof(u32); i++)
> + mtk_w32(eth, rss_params->hash_key[i], MTK_RSS_HASH_KEY_DW(reg_map, i));
> +
> + /* Select the size of indirection table */
> + for (i = 0; i < MTK_RSS_MAX_INDIRECTION_TABLE / 16; i++)
> + mtk_w32(eth, mtk_rss_indr_table(rss_params, i),
> + MTK_RSS_INDR_TABLE_DW(reg_map, i));
> +
> + /* Pause */
> + val |= MTK_RSS_CFG_REQ;
> + mtk_w32(eth, val, reg_map->pdma.rss_glo_cfg);
> +
> + /* Enable RSS */
> + val |= MTK_RSS_EN;
> + mtk_w32(eth, val, reg_map->pdma.rss_glo_cfg);
> +
> + /* Release pause */
> + val &= ~(MTK_RSS_CFG_REQ);
> + mtk_w32(eth, val, reg_map->pdma.rss_glo_cfg);
> +
> + /* Set perRSS GRP INT */
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_RSS_RING(1)),
> + MTK_RX_DONE_INT(eth, MTK_RSS_RING(1)), reg_map->pdma.int_grp);
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_RSS_RING(2)),
> + MTK_RX_DONE_INT(eth, MTK_RSS_RING(2)), reg_map->pdma.int_grp + 0x4);
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_RSS_RING(3)),
> + MTK_RX_DONE_INT(eth, MTK_RSS_RING(3)), reg_map->pdma.int_grp3);
> +
> + return 0;
> +}
> +/* struct mtk_rss_params - This is the structure holding parameters
> + * for the RSS ring
> + * @hash_key The element is used to record the
> + * secret key for the RSS ring
> + * indirection_table The element is used to record the
> + * indirection table for the RSS ring
> + */
Quite odd looking comment. Having the right side aligned like that
makes it header to correlate where doc for fields start.
And there's @ missing for indirection_table.
^ permalink raw reply
* Re: [net-next v8 3/3] net: ethernet: mtk_eth_soc: Add LRO support
From: Jakub Kicinski @ 2026-05-14 1:53 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Felix Fietkau, Lorenzo Bianconi, Andrew Lunn, David S. Miller,
Eric Dumazet, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Frank Wunderlich,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
Mason Chang, Daniel Golle
In-Reply-To: <20260509190938.169290-4-linux@fw-web.de>
On Sat, 9 May 2026 21:09:32 +0200 Frank Wunderlich wrote:
> + mtk_rx_irq_disable(eth, MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i)));
> + napi_synchronize(ð->rx_napi[MTK_HW_LRO_RING(eth, i)].napi);
> + napi_disable(ð->rx_napi[MTK_HW_LRO_RING(eth, i)].napi);
What purpose that napi_synchronize() serve?
Also we don't charge for temporary variables, maybe save that
MTK_HW_LRO_RING(eth, i) to make this slightly more readable.
^ permalink raw reply
* Re: [net-next v8 3/3] net: ethernet: mtk_eth_soc: Add LRO support
From: Jakub Kicinski @ 2026-05-14 1:52 UTC (permalink / raw)
To: linux
Cc: Jakub Kicinski, nbd, lorenzo, andrew+netdev, davem, edumazet,
pabeni, matthias.bgg, angelogioacchino.delregno, linux,
mason-cw.chang, netdev, linux-kernel, daniel, linux-mediatek,
linux-arm-kernel
In-Reply-To: <20260509190938.169290-4-linux@fw-web.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: ethernet: mtk_eth_soc: Add LRO support
This patch adds Hardware Large Receive Offload support to the
mediatek ethernet driver and enables it on MT7988 by adding
MTK_HWLRO to MT7988_CAPS. The patch parameterizes the LRO
register accessors and ring counts on the SoC version, and adds
LRO-specific NAPI/IRQ wiring.
A few questions on the changes below.
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 908fd88287ac..8035fc2557de 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2806,7 +2806,7 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
> if (!ring->data)
> return -ENOMEM;
>
> - if (mtk_page_pool_enabled(eth)) {
> + if (mtk_page_pool_enabled(eth) && rcu_access_pointer(eth->prog)) {
> struct page_pool *pp;
>
> pp = mtk_create_page_pool(eth, &ring->xdp_q, ring_no,
[High]
Could the rationale for this gating change be explained in the commit
message? The subject is "Add LRO support" but this hunk changes the
page_pool creation policy for every netsys v2+ chip.
Before the patch, every v2+ RX ring was backed by a page_pool (since
mtk_page_pool_enabled() returns mtk_is_netsys_v2_or_greater(eth)).
After the patch, a page_pool is only created when an XDP program is
already attached at ring allocation time.
mtk_xdp_setup() also rejects XDP attach with -EOPNOTSUPP when
eth->hwlro is true:
if (eth->hwlro) {
NL_SET_ERR_MSG_MOD(extack, "HW LRO and XDP are mutually exclusive");
return -EOPNOTSUPP;
}
So on MT7988 (where this patch enables MTK_HWLRO) eth->prog is
permanently NULL and mtk_create_page_pool() is now never reached
for any RX ring, including the non-LRO ring 0.
Does this intentionally disable page_pool/DMA buffer recycling on
MT7988, and on every other v2+ chip until a BPF program is loaded?
mtk_ethtool_pp_stats() depends on ring->page_pool, so the page-pool
counters reported through ethtool would also become empty.
The driver still advertises NETDEV_XDP_ACT_BASIC|REDIRECT|NDO_XMIT|
NDO_XMIT_SG via xdp_features for v2+, which appears to diverge from
the buffer-pool capability after this change.
> @@ -2974,30 +2975,50 @@ static int mtk_hwlro_rx_init(struct mtk_eth *eth)
> ring_ctrl_dw2 |= MTK_RING_MAX_AGG_CNT_L;
> ring_ctrl_dw3 |= MTK_RING_MAX_AGG_CNT_H;
>
> - for (i = 1; i < MTK_MAX_RX_RING_NUM; i++) {
> + for (i = 1; i <= MTK_HW_LRO_RING_NUM(eth); i++) {
> mtk_w32(eth, ring_ctrl_dw1, MTK_LRO_CTRL_DW1_CFG(reg_map, i));
[ ... ]
> /* set refresh timer for altering flows to 1 sec. (unit: 20us) */
> mtk_w32(eth, (MTK_HW_LRO_TIMER_UNIT << 16) | MTK_HW_LRO_REFRESH_TIME,
> MTK_PDMA_LRO_ALT_REFRESH_TIMER);
[Medium]
Should MTK_PDMA_LRO_ALT_REFRESH_TIMER also have been converted to
go through reg_map like every other LRO register touched by this
patch? Looking at the header, it is still defined as a hardcoded
absolute offset 0x1c that has lived in the v1 PDMA window since
2016. On MT7988 (newly added to MTK_HWLRO via MT7988_CAPS) the
LRO block lives at 0x6c08-0x6c44 per mt7988_reg_map, while offset
0x1c on v3 falls in the Frame Engine register area between
MTK_FE_INT_ENABLE and MTK_FE_INT_GRP.
Every other LRO register access in this function was switched to
MTK_PDMA_LRO_CTRL_DW0(reg_map), MTK_PDMA_LRO_ALT_SCORE_DELTA(reg_map),
MTK_LRO_DIP_DW0_CFG(reg_map, x) and so on. Was the omission of
this one register intentional, or should it be a similar
reg_map-based accessor so that on MT7988 the auto-learn refresh
timer is programmed in the LRO block instead of in the FE engine
region?
> - /* set HW LRO mode & the max aggregation count for rx packets */
> - lro_ctrl_dw3 |= MTK_ADMA_MODE | (MTK_HW_LRO_MAX_AGG_CNT & 0xff);
> + if (mtk_is_netsys_v3_or_greater(eth)) {
[ ... ]
> /* enable HW LRO */
> lro_ctrl_dw0 |= MTK_LRO_EN;
>
> - mtk_w32(eth, lro_ctrl_dw3, MTK_PDMA_LRO_CTRL_DW3);
> - mtk_w32(eth, lro_ctrl_dw0, MTK_PDMA_LRO_CTRL_DW0);
> + mtk_w32(eth, lro_ctrl_dw3, MTK_PDMA_LRO_CTRL_DW3(reg_map));
> + mtk_w32(eth, lro_ctrl_dw0, MTK_PDMA_LRO_CTRL_DW0(reg_map));
> +
> + if (mtk_is_netsys_v2_or_greater(eth)) {
> + i = (soc->rx.desc_size == sizeof(struct mtk_rx_dma_v2)) ? 1 : 0;
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i)),
> + MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i)), reg_map->pdma.int_grp);
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i + 1)),
> + MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i + 1)),
> + reg_map->pdma.int_grp + 0x4);
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i + 2)),
> + MTK_RX_DONE_INT(eth, MTK_HW_LRO_RING(eth, i + 2)), reg_map->pdma.int_grp3);
> + }
[Medium]
On v3 (the only chip with both MTK_HWLRO and v2+ enabled by this
patch), the desc_size check yields i = 1, so this block routes
MTK_HW_LRO_RING(eth, 1..3) — physical rings 5, 6, 7 with done bits
BIT(29..31) — into int_grp, int_grp+0x4 and int_grp3 respectively.
What routes MTK_HW_LRO_RING(eth, 0), i.e. physical ring 4 with done
bit BIT(28)?
mtk_probe() registers an IRQ handler for all MTK_HW_LRO_RING_NUM(eth)
= 4 LRO rings using IRQF_SHARED, so a NAPI is attached for ring 4 on
eth->irq_pdma[MTK_HW_LRO_IRQ(eth, 0)] (= irq_pdma[0] on v3). Without
ring 4's BIT(28) being OR'd into any int_grp register, does ring 4's
RX interrupt actually reach NAPI, or does that depend on hardware
reset defaults of int_grp on MT7988?
> return 0;
> }
> @@ -3080,6 +3112,65 @@ static int mtk_hwlro_get_ip_cnt(struct mtk_mac *mac)
> return cnt;
> }
>
> +static int mtk_hwlro_add_ipaddr_idx(struct net_device *dev, u32 ip4dst)
> +{
> + struct mtk_mac *mac = netdev_priv(dev);
> + const struct mtk_reg_map *reg_map;
> + struct mtk_eth *eth = mac->hw;
> + u32 reg_val;
> + int i;
> +
> + reg_map = eth->soc->reg_map;
> +
> + /* check for duplicate IP address in the current DIP list */
> + for (i = 1; i <= MTK_HW_LRO_DIP_NUM(eth); i++) {
> + reg_val = mtk_r32(eth, MTK_LRO_DIP_DW0_CFG(reg_map, i));
> + if (reg_val == ip4dst)
> + break;
> + }
> +
> + if (i < MTK_HW_LRO_DIP_NUM(eth) + 1) {
> + netdev_warn(dev, "Duplicate IP address at DIP(%d)!\n", i);
> + return -EEXIST;
> + }
> +
> + /* find out available DIP index */
> + for (i = 1; i <= MTK_HW_LRO_DIP_NUM(eth); i++) {
> + reg_val = mtk_r32(eth, MTK_LRO_DIP_DW0_CFG(reg_map, i));
> + if (reg_val == 0UL)
> + break;
> + }
> +
> + if (i >= MTK_HW_LRO_DIP_NUM(eth) + 1) {
> + netdev_warn(dev, "DIP index is currently out of resource!\n");
> + return -EBUSY;
> + }
> +
> + return i;
> +}
[High]
Is it intentional that the DIP-table allocation policy is changed
from a per-MAC partition (previously hwlro_idx = mac->id *
MTK_MAX_LRO_IP_CNT + fsp->location) to a single global pool searched
across all MACs? The duplicate check and free-slot search above
both iterate every DIP register without scoping by mac->id.
Two observations:
1. Two netdevs that both want HW LRO on the same destination IP
collide on the duplicate-check and the second one fails with
-EEXIST. A single MAC can also fill all DIP slots and starve
the other; non-v3 silicon has only 3 slots while the uAPI
advertises 2 MACs * MTK_MAX_LRO_IP_CNT = 4 rules.
2. mtk_hwlro_add_ipaddr() (below) does not invalidate any
pre-existing entry at mac->hwlro_ip[fsp->location] before
calling this helper, so adding a new IP at a location that
already had one finds a fresh slot for the new IP and then
overwrites the software state. The old slot still has the
original IP with MYIP_VLD set in hardware, and a later del at
the same location only finds and invalidates the new entry.
Together they look like a CAP_NET_ADMIN-reachable way to leak DIP
slots until device reset, while mac->hwlro_ip[] looks consistent.
> +static int mtk_hwlro_get_ipaddr_idx(struct net_device *dev, u32 ip4dst)
> +{
> + struct mtk_mac *mac = netdev_priv(dev);
> + struct mtk_eth *eth = mac->hw;
> + u32 reg_val;
> + int i;
> +
> + /* find out DIP index that matches the given IP address */
> + for (i = 1; i <= MTK_HW_LRO_DIP_NUM(eth); i++) {
> + reg_val = mtk_r32(eth, MTK_LRO_DIP_DW0_CFG(eth->soc->reg_map, i));
> + if (reg_val == ip4dst)
> + break;
> + }
> +
> + if (i >= MTK_HW_LRO_DIP_NUM(eth) + 1) {
> + netdev_warn(dev, "DIP address is not exist!\n");
> + return -ENOENT;
> + }
> +
> + return i;
> +}
> +
> static int mtk_hwlro_add_ipaddr(struct net_device *dev,
> struct ethtool_rxnfc *cmd)
> {
> @@ -3112,13 +3207,17 @@ static int mtk_hwlro_del_ipaddr(struct net_device *dev,
> struct mtk_mac *mac = netdev_priv(dev);
> struct mtk_eth *eth = mac->hw;
> int hwlro_idx;
> + u32 ip4dst;
>
> if (fsp->location > 1)
> return -EINVAL;
>
> - mac->hwlro_ip[fsp->location] = 0;
> - hwlro_idx = (mac->id * MTK_MAX_LRO_IP_CNT) + fsp->location;
> + ip4dst = mac->hwlro_ip[fsp->location];
> + hwlro_idx = mtk_hwlro_get_ipaddr_idx(dev, ip4dst);
> + if (hwlro_idx < 0)
> + return hwlro_idx;
>
> + mac->hwlro_ip[fsp->location] = 0;
> mac->hwlro_ip_cnt = mtk_hwlro_get_ip_cnt(mac);
>
> mtk_hwlro_inval_ipaddr(eth, hwlro_idx);
[Medium]
Should this path validate that mac->hwlro_ip[fsp->location] is
non-zero before passing it to mtk_hwlro_get_ipaddr_idx()? When the
location was never populated (or was already deleted), ip4dst is 0.
mtk_hwlro_get_ipaddr_idx() then matches the first slot whose register
reads 0 — i.e. an arbitrary unrelated empty slot — and returns a
positive index, after which mtk_hwlro_inval_ipaddr() clears MYIP_VLD
on that unrelated slot and the call returns 0 to userspace.
Either rejecting the call with -ENOENT here when
mac->hwlro_ip[fsp->location] == 0, or having mtk_hwlro_get_ipaddr_idx()
refuse ip4dst == 0, would avoid that.
> @@ -3126,6 +3225,24 @@ static int mtk_hwlro_del_ipaddr(struct net_device *dev,
> return 0;
> }
>
> +static void mtk_hwlro_netdev_enable(struct net_device *dev)
> +{
> + struct mtk_mac *mac = netdev_priv(dev);
> + struct mtk_eth *eth = mac->hw;
> + int i, hwlro_idx;
> +
> + for (i = 0; i < MTK_MAX_LRO_IP_CNT; i++) {
> + if (mac->hwlro_ip[i] == 0)
> + continue;
> +
> + hwlro_idx = mtk_hwlro_get_ipaddr_idx(dev, mac->hwlro_ip[i]);
> + if (hwlro_idx < 0)
> + continue;
> +
> + mtk_hwlro_val_ipaddr(eth, hwlro_idx, mac->hwlro_ip[i]);
> + }
> +}
> +
[Low]
This isn't a bug, but is mtk_hwlro_netdev_enable() reachable in
practice? It uses mtk_hwlro_get_ipaddr_idx(), which only succeeds
when the IP is already programmed in hardware. The only path that
disables LRO is mtk_hwlro_netdev_disable() below, which clears
mac->hwlro_ip[i] = 0 (so the first guard always continues) and also
invalidates the DIP entries. After ethtool -K lro off the IPs are
gone from both software and hardware, so the subsequent ethtool -K
lro on appears to do nothing.
If the intent is restore-on-re-enable, should the disable preserve
mac->hwlro_ip[] and/or should this enable path call
mtk_hwlro_add_ipaddr_idx() to allocate fresh slots?
> static void mtk_hwlro_netdev_disable(struct net_device *dev)
> {
> struct mtk_mac *mac = netdev_priv(dev);
> @@ -3133,8 +3250,14 @@ static void mtk_hwlro_netdev_disable(struct net_device *dev)
> int i, hwlro_idx;
>
> for (i = 0; i < MTK_MAX_LRO_IP_CNT; i++) {
> + if (mac->hwlro_ip[i] == 0)
> + continue;
> +
> + hwlro_idx = mtk_hwlro_get_ipaddr_idx(dev, mac->hwlro_ip[i]);
> + if (hwlro_idx < 0)
> + continue;
> +
> mac->hwlro_ip[i] = 0;
> - hwlro_idx = (mac->id * MTK_MAX_LRO_IP_CNT) + i;
>
> mtk_hwlro_inval_ipaddr(eth, hwlro_idx);
> }
[Medium]
Can this leave mac->hwlro_ip[i] populated while mac->hwlro_ip_cnt is
reset to 0? When mtk_hwlro_get_ipaddr_idx() returns < 0 (for example
because the DIP table was already cleared by a prior
mtk_hwlro_rx_uninit, by a parallel teardown on another MAC, or by a
hardware reset), the continue skips the mac->hwlro_ip[i] = 0
assignment. After the loop, mac->hwlro_ip_cnt = 0 still runs
unconditionally.
The previous code unconditionally cleared mac->hwlro_ip[i]. In the
new shape, ETHTOOL_GRXCLSRLCNT would report 0 while
ETHTOOL_GRXCLSRLALL via mtk_hwlro_get_fdir_all() still enumerates
the stale entries, and mtk_hwlro_get_ip_cnt() called from a later
add would recount them.
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 378cf47913ef..f7e7299fef6b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -35,7 +35,7 @@
> #define MTK_DMA_SIZE(x) (SZ_##x)
> #define MTK_FQ_DMA_HEAD 32
> #define MTK_FQ_DMA_LENGTH 2048
> -#define MTK_RX_ETH_HLEN (ETH_HLEN + ETH_FCS_LEN)
> +#define MTK_RX_ETH_HLEN (VLAN_ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN)
> #define MTK_RX_HLEN (NET_SKB_PAD + MTK_RX_ETH_HLEN + NET_IP_ALIGN)
[High]
Could this change be called out in the commit message? It widens
MTK_RX_ETH_HLEN from 18 to 26 globally for every supported chip,
not just MT7988/LRO.
mtk_add_mac() sets:
eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH_2K - MTK_RX_ETH_HLEN;
(and MTK_MAX_RX_LENGTH - MTK_RX_ETH_HLEN for MT7628), so the
maximum MTU advertised by every supported MediaTek SoC drops by 8
bytes (e.g. 2030 -> 2022 on the 2K path, 1518 -> 1510 on MT7628).
Userspace configuration that previously set MTU values up to the
prior limit will now be rejected with -EINVAL.
If headroom for stacked VLANs is needed only on the LRO path, would
it be possible to confine the +8 to MTK_MAX_LRO_RX_LENGTH instead
of changing MTK_RX_ETH_HLEN for every chip?
^ permalink raw reply
* Re: [net-next v8 2/3] net: ethernet: mtk_eth_soc: Add RSS support
From: Jakub Kicinski @ 2026-05-14 1:52 UTC (permalink / raw)
To: linux
Cc: Jakub Kicinski, nbd, lorenzo, andrew+netdev, davem, edumazet,
pabeni, matthias.bgg, angelogioacchino.delregno, linux,
mason-cw.chang, netdev, linux-kernel, daniel, linux-mediatek,
linux-arm-kernel
In-Reply-To: <20260509190938.169290-3-linux@fw-web.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: ethernet: mtk_eth_soc: Add RSS support
This commit adds Receive Side Scaling support for the mtk_eth_soc driver,
enabling up to four RX rings on MT7988 with ethtool -x and
--set-rxfh-indir support. It also reworks the RX NAPI / IRQ handling
into per-ring instances and renames eth->irq[] to eth->irq_fe[] /
eth->irq_pdma[].
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
[ ... ]
> @@ -2235,16 +2199,15 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
[ ... ]
> @@ -2436,7 +2399,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
> * we continue
> */
> wmb();
> - mtk_update_rx_cpu_idx(eth);
> + mtk_update_rx_cpu_idx(eth, ring);
> }
>
> eth->rx_packets += done;
[High]
With RSS enabled, up to MTK_RX_NAPI_NUM (=4) NAPI instances can run
concurrently on different CPUs. eth->rx_packets, eth->rx_bytes (here)
and eth->rx_events (incremented in mtk_handle_irq_rx with IRQF_SHARED)
are plain u32 fields with no atomic, no spinlock, and no per-CPU
storage:
eth->rx_packets += done;
eth->rx_bytes += bytes;
dim_update_sample(eth->rx_events, eth->rx_packets, eth->rx_bytes,
&dim_sample);
net_dim(ð->rx_dim, &dim_sample);
Pre-patch a single rx_napi serialised these accesses. Are these
unsynchronised cross-CPU updates intentional? The corrupted values
feed dim_update_sample() / net_dim() which drives interrupt
moderation, and eth->rx_dim is a single struct dim shared across all
RX NAPIs.
> @@ -2645,7 +2608,9 @@ static int mtk_napi_tx(struct napi_struct *napi, int budget)
>
> static int mtk_napi_rx(struct napi_struct *napi, int budget)
> {
> - struct mtk_eth *eth = container_of(napi, struct mtk_eth, rx_napi);
> + struct mtk_napi *rx_napi = container_of(napi, struct mtk_napi, napi);
> + struct mtk_eth *eth = rx_napi->eth;
> + struct mtk_rx_ring *ring = rx_napi->rx_ring;
> const struct mtk_reg_map *reg_map = eth->soc->reg_map;
> int rx_done_total = 0;
[Medium]
mtk_napi_rx() calls mtk_handle_status_irq(eth) on entry of every poll.
mtk_handle_status_irq() does an unprotected read / clear of
MTK_INT_STATUS2 and calls mtk_stats_update():
status2 = mtk_r32(eth, MTK_INT_STATUS2);
if (unlikely(status2 & (MTK_GDM1_AF | MTK_GDM2_AF))) {
mtk_stats_update(eth);
mtk_w32(eth, (MTK_GDM1_AF | MTK_GDM2_AF), MTK_INT_STATUS2);
}
With multiple rx_napi instances polling on different CPUs, can a fresh
AF event arriving between the read and the write be silently dropped,
or stats be double-counted? The per-MAC stats_lock taken inside
mtk_stats_update_mac does not synchronise the status read or the
clear.
[ ... ]
> @@ -3222,6 +3192,105 @@ static int mtk_hwlro_get_fdir_all(struct net_device *dev,
[ ... ]
> +static int mtk_rss_init(struct mtk_eth *eth)
> +{
> + const struct mtk_soc_data *soc = eth->soc;
> + const struct mtk_reg_map *reg_map = eth->soc->reg_map;
> + struct mtk_rss_params *rss_params = ð->rss_params;
> + u32 val;
> + int i;
> +
> + netdev_rss_key_fill(rss_params->hash_key, MTK_RSS_HASH_KEYSIZE);
> +
> + for (i = 0; i < MTK_RSS_MAX_INDIRECTION_TABLE; i++)
> + rss_params->indirection_table[i] = ethtool_rxfh_indir_default(i, eth->soc->rss_num);
> +
> + if (soc->rx.desc_size == sizeof(struct mtk_rx_dma)) {
> + /* Set RSS rings to PSE modes */
> + for (i = 1; i <= MTK_HW_LRO_RING_NUM(eth); i++) {
> + val = mtk_r32(eth, MTK_LRO_CTRL_DW2_CFG(reg_map, i));
> + val |= MTK_RING_PSE_MODE;
> + mtk_w32(eth, val, MTK_LRO_CTRL_DW2_CFG(reg_map, i));
> + }
> +
> + /* Enable non-lro multiple rx */
> + val = mtk_r32(eth, reg_map->pdma.lro_ctrl_dw0);
> + val |= MTK_NON_LRO_MULTI_EN;
> + mtk_w32(eth, val, reg_map->pdma.lro_ctrl_dw0);
> +
> + /* Enable RSS dly int supoort */
> + val |= MTK_LRO_DLY_INT_EN;
> + mtk_w32(eth, val, reg_map->pdma.lro_ctrl_dw0);
> + }
[Medium]
Is this block reachable on any SoC? mtk_rss_init() is only invoked
from mtk_dma_init() under MTK_HAS_CAPS(MTK_RSS). Only mt7988_data
sets MTK_RSS in caps, and mt7988_data uses
rx.desc_size = sizeof(struct mtk_rx_dma_v2), not
sizeof(struct mtk_rx_dma).
If the PSE-mode / MTK_NON_LRO_MULTI_EN / MTK_LRO_DLY_INT_EN
programming is needed on MT7988, is it being silently skipped? If it
was intended for mt7981 / mt7986, those SoCs never reach this path
because their caps lack MTK_RSS.
There is also a typo above: "RSS dly int supoort" -> "support".
[ ... ]
> + /* Set perRSS GRP INT */
> + mtk_m32(eth, MTK_RX_DONE_INT(eth, MTK_RSS_RING(1)),
> + MTK_RX_DONE_INT(eth, MTK_RSS_RING(1)), reg_map->pdma.int_grp);
[ ... ]
> +
> + return 0;
> +}
[ ... ]
> @@ -3312,6 +3381,17 @@ static int mtk_dma_init(struct mtk_eth *eth)
[ ... ]
> + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSS)) {
> + for (i = 1; i < MTK_RX_RSS_NUM(eth); i++) {
> + err = mtk_rx_alloc(eth, MTK_RSS_RING(i), MTK_RX_FLAGS_NORMAL);
> + if (err)
> + return err;
> + }
> + err = mtk_rss_init(eth);
> + if (err)
> + return err;
> + }
> +
[ ... ]
> @@ -3437,14 +3523,36 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
> return 0;
> }
>
> -static irqreturn_t mtk_handle_irq_rx(int irq, void *_eth)
> +static int mtk_get_irqs_pdma(struct platform_device *pdev, struct mtk_eth *eth)
> {
> - struct mtk_eth *eth = _eth;
> + char rxring[] = "pdma0";
> + int i;
> +
> + for (i = 0; i < MTK_PDMA_IRQ_NUM; i++) {
> + rxring[4] = '0' + i;
> + eth->irq_pdma[i] = platform_get_irq_byname(pdev, rxring);
> + if (eth->irq_pdma[i] < 0)
> + return eth->irq_pdma[i];
> + }
> +
> + return 0;
> +}
[High]
This loop demands all four pdma0..pdma3 IRQs whenever
MTK_HAS_CAPS(MTK_PDMA_INT) is set, but the same patch adds
MTK_PDMA_INT to MT7981_CAPS and MT7986_CAPS — SoCs which do not
have MTK_RSS. On those SoCs only rx_napi[0] / pdma0 is ever
referenced (every pdma1..pdma3 use site is gated on
MTK_HAS_CAPS(MTK_RSS) in probe / open / stop / dma_init / dma_free).
Existing MT7981 / MT7986 device trees declare only fe1 / fe2. Does
this turn into a probe-time -ENXIO failure for those boards? Should
the call to mtk_get_irqs_pdma(), or at least the requests for
pdma1..pdma3, be folded under MTK_HAS_CAPS(MTK_RSS) so older DTs
continue to probe?
> +
> +static irqreturn_t mtk_handle_irq_rx(int irq, void *priv)
> +{
> + struct mtk_napi *rx_napi = priv;
> + struct mtk_eth *eth = rx_napi->eth;
> + struct mtk_rx_ring *ring = rx_napi->rx_ring;
>
> eth->rx_events++;
> - if (likely(napi_schedule_prep(ð->rx_napi))) {
> - mtk_rx_irq_disable(eth, eth->soc->rx.irq_done_mask);
> - __napi_schedule(ð->rx_napi);
> + if (unlikely(!(mtk_r32(eth, eth->soc->reg_map->pdma.irq_status) &
> + mtk_r32(eth, eth->soc->reg_map->pdma.irq_mask) &
> + MTK_RX_DONE_INT(eth, ring->ring_no))))
> + return IRQ_NONE;
[High]
Pre-patch mt7988_data set rx.irq_done_mask = MTK_RX_DONE_INT_V2 =
BIT(14). Post-patch the per-soc field is removed and replaced with
MTK_RX_DONE_INT(eth, ring_no), which for V3 (mt7988) expands to
BIT(24 + ring_no), so MTK_RX_DONE_INT(eth, 0) is now BIT(24) instead
of BIT(14).
Is the bit number change intentional? If MT7988 RX-done is actually
signalled on BIT(14), the new mask never matches, mtk_handle_irq_rx
returns IRQ_NONE, and RX traffic stalls. If BIT(14) was wrong all
along, could the commit message describe the silent fix? The patch
otherwise advertises only RSS.
[ ... ]
> @@ -3888,9 +4011,7 @@ static void mtk_dim_rx(struct work_struct *work)
[ ... ]
> - val = mtk_r32(eth, reg_map->pdma.delay_irq);
> - val &= MTK_PDMA_DELAY_TX_MASK;
> - val |= MTK_PDMA_DELAY_RX_EN;
> + val = MTK_PDMA_DELAY_RX_EN;
[ ... ]
> - mtk_w32(eth, val, reg_map->pdma.delay_irq);
> if (MTK_HAS_CAPS(eth->soc->caps, MTK_QDMA))
> - mtk_w32(eth, val, reg_map->qdma.delay_irq);
> + mtk_m32(eth, MTK_PDMA_DELAY_TX_MASK,
> + val << MTK_PDMA_DELAY_TX_PTIME_SHIFT, reg_map->qdma.delay_irq);
> +
> + if (eth->soc->rx.desc_size == sizeof(struct mtk_rx_dma)) {
> + mtk_m32(eth, MTK_PDMA_DELAY_RX_MASK, val, reg_map->pdma.delay_irq);
> + mtk_w32(eth, val, reg_map->pdma.lro_rx1_dly_int);
> + mtk_w32(eth, val, reg_map->pdma.lro_rx1_dly_int + 0x4);
> + mtk_w32(eth, val, reg_map->pdma.lro_rx1_dly_int + 0x8);
> + } else {
> + val = val | (val << MTK_PDMA_DELAY_RX_RING_SHIFT);
> + mtk_w32(eth, val, reg_map->pdma.rx_delay_irq);
> + }
[Critical]
The new branch is gated on rx.desc_size, not on a capability bit.
rt5350_data (MT7628) has rx.desc_size = sizeof(struct mtk_rx_dma) and
uses mt7628_reg_map, which does not initialise lro_rx1_dly_int (it
defaults to 0).
mtk_hw_init() calls mtk_dim_rx(ð->rx_dim.work) inside the MT7628
branch, and net_dim invokes it again on every moderation update. Each
call therefore performs:
mtk_w32(eth, val, 0);
mtk_w32(eth, val, 0x4);
mtk_w32(eth, val, 0x8);
These offsets land in the Frame Engine global config region
(MTK_FE_GLO_CFG(0) = 0x00 etc.) instead of any LRO/RSS register, with
val containing MTK_PDMA_DELAY_RX_EN = BIT(15). Can this corrupt
MT7628 frame-engine state on every boot and every NAPI moderation
update? Should the branch be gated on MTK_HWLRO or MTK_RSS rather
than on rx.desc_size?
[Medium]
On V3 hardware (the only chip enabling RSS) only two ring slots are
programmed here:
val = val | (val << MTK_PDMA_DELAY_RX_RING_SHIFT);
mtk_w32(eth, val, reg_map->pdma.rx_delay_irq);
With MTK_PDMA_DELAY_RX_RING_SHIFT = 16 the register holds two rings,
but RSS uses MTK_RX_RSS_NUM(eth) = 4 rings. Do rings 2 and 3 ever
receive DIM-driven moderation updates after probe, or do they stay at
the hardware default?
[ ... ]
> @@ -4149,6 +4285,25 @@ static void mtk_hw_reset_monitor_work(struct work_struct *work)
[ ... ]
> +static int mtk_napi_init(struct mtk_eth *eth)
> +{
[ ... ]
> +}
[ ... ]
> @@ -4238,12 +4393,11 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
[ ... ]
> @@ -4700,8 +4860,13 @@ static void mtk_get_ethtool_stats(struct net_device *dev,
>
> static u32 mtk_get_rx_ring_count(struct net_device *dev)
> {
> + struct mtk_mac *mac = netdev_priv(dev);
> + struct mtk_eth *eth = mac->hw;
> +
> if (dev->hw_features & NETIF_F_LRO)
> return MTK_MAX_RX_RING_NUM;
> + else if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSS))
> + return MTK_RX_RSS_NUM(eth);
>
> return 0;
> }
[ ... ]
> @@ -4784,6 +4949,70 @@ static int mtk_set_eee(struct net_device *dev, struct ethtool_keee *eee)
[ ... ]
> +static int mtk_set_rxfh(struct net_device *dev, struct ethtool_rxfh_param *rxfh,
> + struct netlink_ext_ack *extack)
> +{
> + struct mtk_mac *mac = netdev_priv(dev);
> + struct mtk_eth *eth = mac->hw;
> + struct mtk_rss_params *rss_params = ð->rss_params;
> + const struct mtk_reg_map *reg_map = eth->soc->reg_map;
> + int i;
> +
> + if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
> + rxfh->hfunc != ETH_RSS_HASH_TOP)
> + return -EOPNOTSUPP;
> +
> + if (rxfh->key) {
> + memcpy(rss_params->hash_key, rxfh->key,
> + sizeof(rss_params->hash_key));
> + for (i = 0; i < MTK_RSS_HASH_KEYSIZE / sizeof(u32); i++)
> + mtk_w32(eth, rss_params->hash_key[i],
> + MTK_RSS_HASH_KEY_DW(reg_map, i));
> + }
> +
> + if (rxfh->indir) {
> + for (i = 0; i < MTK_RSS_MAX_INDIRECTION_TABLE; i++)
> + rss_params->indirection_table[i] = rxfh->indir[i];
> + for (i = 0; i < MTK_RSS_MAX_INDIRECTION_TABLE / 16; i++)
> + mtk_w32(eth, mtk_rss_indr_table(rss_params, i),
> + MTK_RSS_INDR_TABLE_DW(reg_map, i));
> + }
> +
> + return 0;
> +}
[High]
Should this op (and .get_rxfh / .get_rxfh_key_size /
.get_rxfh_indir_size) be gated on MTK_HAS_CAPS(MTK_RSS)? As written,
mtk_ethtool_ops exposes them unconditionally for every SoC the driver
supports.
MTK_RSS_HASH_KEY_DW(reg_map, x) and MTK_RSS_INDR_TABLE_DW(reg_map, x)
are computed as (reg_map)->pdma.rss_glo_cfg + 0x20 + ... and + 0x50 +
... . mtk_reg_map (mt2701/mt7621/mt7622/mt7623/mt7629) and
mt7628_reg_map do not initialise rss_glo_cfg, so it defaults to 0.
A CAP_NET_ADMIN user invoking ethtool -X dev hkey ... on those SoCs
would write user-controlled u32 values into absolute frame-engine
offsets 0x20..0x44 (and indirection writes to 0x50..0x6c). Can these
ops be made conditional on MTK_RSS, or filled in only for the RSS-
capable SoC?
[Medium]
mtk_set_rxfh() does not validate user-supplied indir[i] entries
against MTK_RX_RSS_NUM(eth). The packing helper allots only 2 bits
per entry:
val |= (rss_params->indirection_table[i] << (2 * (i % 16)));
ethtool core only enforces indir[i] < num_rx_rings when
num_rx_rings is non-zero, and MTK_RSS hardware has at most 4 rings.
Should the driver reject values >= MTK_RX_RSS_NUM(eth) so they don't
silently bleed across the 2-bit slots in the packed register? The
indirection table also stores the raw u32 into a u8 field, which
truncates 4..255 instead of catching them.
[Medium]
mtk_rss_init() wraps key / indirection writes with an
MTK_RSS_CFG_REQ pause / release handshake:
/* Pause */
val |= MTK_RSS_CFG_REQ;
mtk_w32(eth, val, reg_map->pdma.rss_glo_cfg);
...
/* Release pause */
val &= ~(MTK_RSS_CFG_REQ);
mtk_set_rxfh() writes the same registers at runtime without that
handshake. Can the hardware sample partially-updated key /
indirection state and steer packets to incorrect or out-of-range
rings while the write loop is in flight?
[ ... ]
> @@ -5424,6 +5708,7 @@ static const struct mtk_soc_data mt2701_data = {
[ ... entries for mt2701/mt7621/mt7622/mt7623/mt7629/rt5350 ... ]
> @@ -5567,16 +5851,16 @@ static const struct mtk_soc_data mt7981_data = {
[ ... ]
> .foe_entry_size = MTK_FOE_ENTRY_V2_SIZE,
> + .rss_num = 4,
[ ... ]
> @@ -5597,6 +5881,7 @@ static const struct mtk_soc_data mt7986_data = {
[ ... ]
> .foe_entry_size = MTK_FOE_ENTRY_V2_SIZE,
> + .rss_num = 4,
[Low]
mt7981_data and mt7986_data set rss_num = 4, but MT7981_CAPS and
MT7986_CAPS do not include MTK_RSS (only MT7988_CAPS does). Every
RSS code path is gated on MTK_HAS_CAPS(MTK_RSS) before consulting
rss_num, so this has no current effect, but the SoC capability table
becomes internally inconsistent. Should rss_num be 0 on these SoCs
until MTK_RSS is actually wired up and tested for them?
> @@ -5627,20 +5911,20 @@ static const struct mtk_soc_data mt7988_data = {
[ ... ]
> .foe_entry_size = MTK_FOE_ENTRY_V3_SIZE,
> + .rss_num = 4,
[ ... ]
> .rx = {
> .desc_size = sizeof(struct mtk_rx_dma_v2),
> - .irq_done_mask = MTK_RX_DONE_INT_V2,
> .dma_l4_valid = RX_DMA_L4_VALID_V2,
> .dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
> .dma_len_offset = 8,
> - .dma_size = MTK_DMA_SIZE(2K),
> + .dma_size = MTK_DMA_SIZE(1K),
> },
> };
[ ... ]
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH 3/3] usb: dwc3: xilinx: fix error handling in zynqmp init error paths
From: Thinh Nguyen @ 2026-05-14 1:43 UTC (permalink / raw)
To: Radhey Shyam Pandey
Cc: Thinh Nguyen, gregkh@linuxfoundation.org, michal.simek@amd.com,
p.zabel@pengutronix.de, linux-usb@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git@amd.com, stable@vger.kernel.org
In-Reply-To: <20260511160814.2904882-4-radhey.shyam.pandey@amd.com>
On Mon, May 11, 2026, Radhey Shyam Pandey wrote:
> Fix error handling and resource cleanup i.e remove invalid
> phy_exit() after failed phy_init(), route failures through
> proper cleanup paths and return 0 explicitly on success.
>
> Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
> Cc: stable@vger.kernel.org
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> drivers/usb/dwc3/dwc3-xilinx.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index 94458b3da1a0..b832505e1b04 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -176,15 +176,13 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
> }
>
> ret = phy_init(priv_data->usb3_phy);
> - if (ret < 0) {
> - phy_exit(priv_data->usb3_phy);
> + if (ret < 0)
> goto err;
> - }
>
> ret = reset_control_deassert(apbrst);
> if (ret < 0) {
> dev_err(dev, "Failed to release APB reset\n");
> - goto err;
> + goto err_phy_exit;
> }
>
> if (priv_data->usb3_phy) {
> @@ -200,26 +198,24 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
> ret = reset_control_deassert(crst);
> if (ret < 0) {
> dev_err(dev, "Failed to release core reset\n");
> - goto err;
> + goto err_phy_exit;
> }
>
> ret = reset_control_deassert(hibrst);
> if (ret < 0) {
> dev_err(dev, "Failed to release hibernation reset\n");
> - goto err;
> + goto err_phy_exit;
> }
>
> ret = phy_power_on(priv_data->usb3_phy);
> - if (ret < 0) {
> - phy_exit(priv_data->usb3_phy);
> - goto err;
> - }
> + if (ret < 0)
> + goto err_phy_exit;
>
> /* ulpi reset via gpio-modepin or gpio-framework driver */
> reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> if (IS_ERR(reset_gpio)) {
> - return dev_err_probe(dev, PTR_ERR(reset_gpio),
> - "Failed to request reset GPIO\n");
> + ret = PTR_ERR(reset_gpio);
> + goto err_phy_power_off;
> }
>
> if (reset_gpio) {
> @@ -229,6 +225,13 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
> }
>
> dwc3_xlnx_set_coherency(priv_data, XLNX_USB_TRAFFIC_ROUTE_CONFIG);
> +
> + return 0;
> +
> +err_phy_power_off:
> + phy_power_off(priv_data->usb3_phy);
> +err_phy_exit:
> + phy_exit(priv_data->usb3_phy);
> err:
> return ret;
> }
> --
> 2.44.4
>
This fix should be a separate patch from this cleanup series.
Thanks,
Thinh
^ permalink raw reply
* Re: [PATCH 2/3] usb: dwc3: xilinx: use reset_control_reset() in versal init
From: Thinh Nguyen @ 2026-05-14 1:34 UTC (permalink / raw)
To: Radhey Shyam Pandey
Cc: Thinh Nguyen, gregkh@linuxfoundation.org, michal.simek@amd.com,
p.zabel@pengutronix.de, linux-usb@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git@amd.com
In-Reply-To: <20260511160814.2904882-3-radhey.shyam.pandey@amd.com>
On Mon, May 11, 2026, Radhey Shyam Pandey wrote:
> Replace separate reset_control_assert() and reset_control_deassert() calls
> with reset_control_reset(), which pulses the reset in one step. Report
> failures with dev_err_probe() and a single message. No functional change.
>
The behavior of reset_control_reset() is a little different. I wouldn't
call this "No functional change". However, I assumed this was tested.
Please provide a proper reason for this change in the change log.
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> drivers/usb/dwc3/dwc3-xilinx.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index a3c7dc258c7d..94458b3da1a0 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -98,18 +98,10 @@ static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
>
> dwc3_xlnx_mask_phy_rst(priv_data, false);
>
> - /* Assert and De-assert reset */
> - ret = reset_control_assert(crst);
> - if (ret < 0) {
> - dev_err_probe(dev, ret, "failed to assert Reset\n");
> - return ret;
> - }
> -
> - ret = reset_control_deassert(crst);
> - if (ret < 0) {
> - dev_err_probe(dev, ret, "failed to De-assert Reset\n");
> - return ret;
> - }
> + /* assert and deassert reset */
> + ret = reset_control_reset(crst);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to assert and deassert reset\n");
>
> dwc3_xlnx_mask_phy_rst(priv_data, true);
> dwc3_xlnx_set_coherency(priv_data, XLNX_USB2_TRAFFIC_ROUTE_CONFIG);
> --
> 2.44.4
>
^ permalink raw reply
* Re: [PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
From: Salih Erim @ 2026-05-14 1:33 UTC (permalink / raw)
To: Maxwell Doose, conall.ogriofa, jic23, michal.simek
Cc: David Lechner, Nuno Sá, Andy Shevchenko,
open list:XILINX AMS DRIVER, moderated list:ARM/ZYNQ ARCHITECTURE,
open list, git
In-Reply-To: <20260508124513.17752-1-m32285159@gmail.com>
Hi Maxwell,
On 5/8/2026 1:45 PM, Maxwell Doose wrote:
> Include linux/cleanup.h to take advantage of RAII macros.
>
> Replace spin_lock() and unlock() calls with their RAII macro
> counterparts, which modernizes the code and increases readability.
>
> Remove "flags" variables where spin_lock_irqsave() has been replaced
> with guard(spinlock_irqsave)().
>
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
> v2:
> - Replace guard(spinlock_irq)() in ams_unmask_worker() with
> scoped_guard() per Jonathan's suggestion.
Nice cleanup, thanks for taking care of this.
Reviewed-by: Salih Erim <salih.erim@amd.com>
Salih.
^ permalink raw reply
* Re: [PATCH v2 03/11] m68k: mcf5441x: setup DAC clock name as per driver name
From: Greg Ungerer @ 2026-05-14 1:27 UTC (permalink / raw)
To: Angelo Dureghello, Geert Uytterhoeven, Steven King, Arnd Bergmann,
Maxime Coquelin, Alexandre Torgue, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko
Cc: Greg Ungerer, linux-m68k, linux-kernel, linux-stm32,
linux-arm-kernel, linux-iio
In-Reply-To: <20260513-wip-stmark2-dac-v2-3-fcdae50cf51a@baylibre.com>
Hi Angelo,
On 13/5/26 19:14, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Later in this patchset, the mcf54415 DAC driver is added.
> Considering some other different ColdFire cpu DACs exists, the DAC driver
> is named as "mcf54415_dac", related to the mcf5441x family SoCs with
> DACs (mcf54415/6/7/8).
>
> So updating DAC clock names to bind with proper driver name.
I am not sure I like naming the clocks here with a prefix for the
specific SoC part number this is in. It might be unlikely now, but
what if another ColdFire family SoC member uses this same hardware block?
That is very common amongst other hardware blocks within the ColdFire
family. Can we come up with a name more specific to just this type
of DAC hardware block?
Regards
Greg
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> arch/m68k/coldfire/m5441x.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/m68k/coldfire/m5441x.c b/arch/m68k/coldfire/m5441x.c
> index 5b5e09ecf487..b724d7fc1a08 100644
> --- a/arch/m68k/coldfire/m5441x.c
> +++ b/arch/m68k/coldfire/m5441x.c
> @@ -43,8 +43,8 @@ DEFINE_CLK(0, "mcfpit.2", 34, MCF_BUSCLK);
> DEFINE_CLK(0, "mcfpit.3", 35, MCF_BUSCLK);
> DEFINE_CLK(0, "mcfeport.0", 36, MCF_CLK);
> DEFINE_CLK(0, "mcfadc.0", 37, MCF_CLK);
> -DEFINE_CLK(0, "mcfdac.0", 38, MCF_CLK);
> -DEFINE_CLK(0, "mcfdac.1", 39, MCF_CLK);
> +DEFINE_CLK(0, "mcf54415_dac.0", 38, MCF_CLK);
> +DEFINE_CLK(0, "mcf54415_dac.1", 39, MCF_CLK);
> DEFINE_CLK(0, "mcfrtc.0", 42, MCF_CLK);
> DEFINE_CLK(0, "mcfsim.0", 43, MCF_CLK);
> DEFINE_CLK(0, "mcfusb-otg.0", 44, MCF_CLK);
> @@ -106,8 +106,8 @@ static struct clk_lookup m5411x_clk_lookup[] = {
> CLKDEV_INIT("mcfpit.3", NULL, &__clk_0_35),
> CLKDEV_INIT("mcfeport.0", NULL, &__clk_0_36),
> CLKDEV_INIT("mcfadc.0", NULL, &__clk_0_37),
> - CLKDEV_INIT("mcfdac.0", NULL, &__clk_0_38),
> - CLKDEV_INIT("mcfdac.1", NULL, &__clk_0_39),
> + CLKDEV_INIT("mcf54415_dac.0", NULL, &__clk_0_38),
> + CLKDEV_INIT("mcf54415_dac.1", NULL, &__clk_0_39),
> CLKDEV_INIT("mcfrtc.0", NULL, &__clk_0_42),
> CLKDEV_INIT("mcfsim.0", NULL, &__clk_0_43),
> CLKDEV_INIT("mcfusb-otg.0", NULL, &__clk_0_44),
>
^ permalink raw reply
* Re: [PATCH 2/4] ASoC: stm: stm32_i2s: Use guard() for spin locks
From: Mark Brown @ 2026-05-14 1:25 UTC (permalink / raw)
To: phucduc.bui
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: <20260513104329.81592-3-phucduc.bui@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 644 bytes --]
On Wed, May 13, 2026 at 05:43:27PM +0700, phucduc.bui@gmail.com wrote:
> @@ -1016,21 +1014,18 @@ static int stm32_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> I2S_IER_OVRIE,
> (unsigned int)~I2S_IER_OVRIE);
>
> - spin_lock(&i2s->lock_fd);
> - i2s->refcount--;
> - if (i2s->refcount) {
> - spin_unlock(&i2s->lock_fd);
> - break;
> - }
> + scoped_guard(spinlock, &i2s->lock_fd) {
> + i2s->refcount--;
> + if (i2s->refcount)
> + break;
How does scoped_guard interact with break statements - does this still
apply to the switch? I've not looked at how they're implemented...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] usb: dwc3: xilinx: fix missing space before closing comment delimiter
From: Thinh Nguyen @ 2026-05-14 1:23 UTC (permalink / raw)
To: Radhey Shyam Pandey
Cc: Thinh Nguyen, gregkh@linuxfoundation.org, michal.simek@amd.com,
p.zabel@pengutronix.de, linux-usb@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git@amd.com
In-Reply-To: <20260511160814.2904882-2-radhey.shyam.pandey@amd.com>
On Mon, May 11, 2026, Radhey Shyam Pandey wrote:
> Add missing space before '*/' in an inline comment to follow
> the kernel coding style.
>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> drivers/usb/dwc3/dwc3-xilinx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index f41b0da5e89d..a3c7dc258c7d 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -196,7 +196,7 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
> }
>
> if (priv_data->usb3_phy) {
> - /* Set PIPE Power Present signal in FPD Power Present Register*/
> + /* Set PIPE Power Present signal in FPD Power Present Register */
> writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
> /* Set the PIPE Clock Select bit in FPD PIPE Clock register */
> writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
> --
> 2.44.4
>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
BR,
Thinh
^ permalink raw reply
* Re: [PATCH v4 0/3] iio: adc: xilinx-ams: refactor alarm handling to table-driven design
From: Salih Erim @ 2026-05-14 1:21 UTC (permalink / raw)
To: Guilherme Ivo Bozi, anand.ashok.dumbre, andy, conall.ogriofa,
dlechner, jic23, manish.narani, michal.simek, nuno.sa
Cc: Jonathan.Cameron, linux-arm-kernel, linux-iio
In-Reply-To: <20260513003503.339418-1-guilherme.bozi@usp.br>
Hi,
On 5/13/2026 1:31 AM, Guilherme Ivo Bozi wrote:
> v3 -> v4:
> - Removed unnecessary 'event < 0' check for type e32
Thanks for addressing the feedback.
Tested on ZCU102 Rev1.0. Driver probes successfully with no errors.
For the series:
Reviewed-by: Salih Erim <salih.erim@amd.com>
Tested-by: Salih Erim <salih.erim@amd.com>
Salih
^ permalink raw reply
* [PATCH 4/4] perf: nvidia_t410_c2c: handle PERF_EF_UPDATE in stop
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
In-Reply-To: <20260514010629.76558-1-sauravsc@amazon.com>
nv_c2c_pmu_stop() does not read the final counter value when called
with PERF_EF_UPDATE. The last counter delta is lost when perf core
removes the event.
Add the standard PMU stop pattern: bail out if already stopped, call
nv_c2c_pmu_event_update() when PERF_EF_UPDATE is requested, then mark
the event stopped.
Fixes: 2f89b7f78c50 ("perf: add NVIDIA Tegra410 C2C PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_c2c_pmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/perf/nvidia_t410_c2c_pmu.c b/drivers/perf/nvidia_t410_c2c_pmu.c
index 662fa1bc833a5..39d0e661e4c5c 100644
--- a/drivers/perf/nvidia_t410_c2c_pmu.c
+++ b/drivers/perf/nvidia_t410_c2c_pmu.c
@@ -394,6 +394,12 @@ static void nv_c2c_pmu_start(struct perf_event *event, int pmu_flags)
static void nv_c2c_pmu_stop(struct perf_event *event, int pmu_flags)
{
+ if (event->hw.state & PERF_HES_STOPPED)
+ return;
+
+ if (pmu_flags & PERF_EF_UPDATE)
+ nv_c2c_pmu_event_update(event);
+
event->hw.state |= PERF_HES_STOPPED;
}
--
2.47.3
^ permalink raw reply related
* [PATCH 3/4] perf: nvidia_t410_c2c: fix cpuhp state leak on init failure
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
In-Reply-To: <20260514010629.76558-1-sauravsc@amazon.com>
If platform_driver_register() fails, the cpuhp multi-state registered
by cpuhp_setup_state_multi() is never cleaned up. Add
cpuhp_remove_multi_state() on the error path, mirroring the cleanup
in nv_c2c_pmu_exit().
Fixes: 2f89b7f78c50 ("perf: add NVIDIA Tegra410 C2C PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_c2c_pmu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/nvidia_t410_c2c_pmu.c b/drivers/perf/nvidia_t410_c2c_pmu.c
index 411987153ff3f..662fa1bc833a5 100644
--- a/drivers/perf/nvidia_t410_c2c_pmu.c
+++ b/drivers/perf/nvidia_t410_c2c_pmu.c
@@ -1034,7 +1034,12 @@ static int __init nv_c2c_pmu_init(void)
return ret;
nv_c2c_pmu_cpuhp_state = ret;
- return platform_driver_register(&nv_c2c_pmu_driver);
+
+ ret = platform_driver_register(&nv_c2c_pmu_driver);
+ if (ret)
+ cpuhp_remove_multi_state(nv_c2c_pmu_cpuhp_state);
+
+ return ret;
}
static void __exit nv_c2c_pmu_exit(void)
--
2.47.3
^ permalink raw reply related
* [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
In-Reply-To: <20260514010629.76558-1-sauravsc@amazon.com>
cmem_lat_pmu_stop() does not read the final counter value when called
with PERF_EF_UPDATE. When perf core calls pmu->del() -> pmu->stop()
with PERF_EF_UPDATE, the last counter delta is lost because the event
is marked stopped without reading hardware.
Add the standard PMU stop pattern: bail out if already stopped, call
the event update function when PERF_EF_UPDATE is requested, then mark
the event stopped.
Fixes: 429b7638b2df ("perf: add NVIDIA Tegra410 CPU Memory Latency PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_cmem_latency_pmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/perf/nvidia_t410_cmem_latency_pmu.c b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
index e27bf31b2b366..c7fa54c7a7c9e 100644
--- a/drivers/perf/nvidia_t410_cmem_latency_pmu.c
+++ b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
@@ -303,6 +303,12 @@ static void cmem_lat_pmu_start(struct perf_event *event, int pmu_flags)
static void cmem_lat_pmu_stop(struct perf_event *event, int pmu_flags)
{
+ if (event->hw.state & PERF_HES_STOPPED)
+ return;
+
+ if (pmu_flags & PERF_EF_UPDATE)
+ cmem_lat_pmu_event_update(event);
+
event->hw.state |= PERF_HES_STOPPED;
}
--
2.47.3
^ 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