Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* 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 net-next 1/2] net: ti: icssg: Derive stats array lengths from ARRAY_SIZE
From: MD Danish Anwar @ 2026-05-14  4:56 UTC (permalink / raw)
  To: Jacob Keller, David CARLIER
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jonathan Corbet, Shuah Khan, Roger Quadros,
	Andrew Lunn, Meghana Malladi, Kevin Hao, Vadim Fedorenko, netdev,
	linux-doc, linux-kernel, linux-arm-kernel, Vignesh Raghavendra
In-Reply-To: <9fbf6682-b521-4b7e-b5b6-af91694ed051@intel.com>

Hi Jacob,

On 14/05/26 1:30 am, Jacob Keller wrote:
> On 5/12/2026 2:40 AM, MD Danish Anwar wrote:
>> Hi David,
>>
>> On 12/05/26 1:28 pm, David CARLIER wrote:
>>> Hi MD,
>>>
>>> On Tue, 12 May 2026 at 07:06, MD Danish Anwar <danishanwar@ti.com> wrote:
>>>>
>>>> Replace the manually maintained ICSSG_NUM_MIIG_STATS and
>>>> ICSSG_NUM_PA_STATS constants with ARRAY_SIZE() expressions derived
>>>> directly from the corresponding stat descriptor arrays, so that adding
>>>> new entries to icssg_all_miig_stats[] or icssg_all_pa_stats[] no longer
>>>> requires a separate update to a numeric constant.
>>>>
>>>> To make this self-contained, break the circular include dependency
>>>> between icssg_stats.h and icssg_prueth.h:
>>>>
>>>>   - icssg_stats.h previously included icssg_prueth.h (transitively
>>>>     pulling in icssg_switch_map.h and ETH_GSTRING_LEN).  Replace that
>>>>     with direct includes of <linux/ethtool.h>, <linux/kernel.h> and
>>>>     "icssg_switch_map.h".
>>>>
>>>>   - icssg_prueth.h now includes icssg_stats.h, giving it access to
>>>>     the ARRAY_SIZE-based ICSSG_NUM_MIIG_STATS and ICSSG_NUM_PA_STATS
>>>>     before they are used in the prueth_emac struct and ICSSG_NUM_STATS.
>>>>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>>  drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +--
>>>>  drivers/net/ethernet/ti/icssg/icssg_stats.h  | 7 ++++++-
>>>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> index df93d15c5b78..e2ccecb0a0dd 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> @@ -43,6 +43,7 @@
>>>>
>>>>  #include "icssg_config.h"
>>>>  #include "icss_iep.h"
>>>> +#include "icssg_stats.h"
>>>>  #include "icssg_switch_map.h"
>>>>
>>>>  #define PRUETH_MAX_MTU          (2000 - ETH_HLEN - ETH_FCS_LEN)
>>>> @@ -57,8 +58,6 @@
>>>>
>>>>  #define ICSSG_MAX_RFLOWS       8       /* per slice */
>>>>
>>>> -#define ICSSG_NUM_PA_STATS     32
>>>> -#define ICSSG_NUM_MIIG_STATS   60
>>>>  /* Number of ICSSG related stats */
>>>>  #define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
>>>>  #define ICSSG_NUM_STANDARD_STATS 31
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> index 5ec0b38e0c67..b854eb587c1e 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> @@ -8,10 +8,15 @@
>>>>  #ifndef __NET_TI_ICSSG_STATS_H
>>>>  #define __NET_TI_ICSSG_STATS_H
>>>>
>>>> -#include "icssg_prueth.h"
>>>> +#include <linux/ethtool.h>
>>>> +#include <linux/kernel.h>
>>>> +#include "icssg_switch_map.h"
>>>>
>>>>  #define STATS_TIME_LIMIT_1G_MS    25000    /* 25 seconds @ 1G */
>>>>
>>>> +#define ICSSG_NUM_MIIG_STATS   ARRAY_SIZE(icssg_all_miig_stats)
>>>> +#define ICSSG_NUM_PA_STATS     ARRAY_SIZE(icssg_all_pa_stats)
>>>> +
>>>>  struct miig_stats_regs {
>>>>         /* Rx */
>>>>         u32 rx_packets;
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> One thing that caught my eye: icssg_all_miig_stats[] and
>>>   icssg_all_pa_stats[] are 'static const' arrays in icssg_stats.h with
>>>   ETH_GSTRING_LEN name buffers per entry. Right now only icssg_stats.c
>>>   and icssg_ethtool.c pull them in. After this patch icssg_prueth.h
>>>   includes icssg_stats.h, so every .c in the driver (classifier,
>>>   common, config, mii_cfg, queues, switchdev, ...) ends up with its own
>>>   static-const copy of both tables.
>>>
>>>   Would a static_assert() work for what you're after? Something like:
>>>
>>
>> While adding more stats manually, The ARRAY_SIZE() approach was
>> explicitly requested by maintainer [1]:
>>
>> This patch is a direct response to that feedback. static_assert() would
>> still require updating the numeric constant on every array change. The
>> goal here is to eliminate the need of manually incrementing stats count
>> whenever new stats are added
>>
>> Your concern about multiple copies of table is noted and valid. Could
>> you advise on the preferred way to reconcile these two requirements? I
>> am happy to restructure if there is an approach that satisfies both.
>>
> The way we solved this in the Intel drivers is to use a single array
> which contains both the stat name as well as the offset from the
> structure where the stat resides.
> 
> The stat string code just iterates over the stat list for the strings,
> while the stat value code iterates the array and computes the stat
> address from the offset and size and base structure pointer. Each object
> that has stats has its own stat array structure.
> 
> This is probably overkill, but the advantage is that the strings and
> their values are stored together and adding a new stat is as simple as
> adding a new entry to that list.
> 
> I.e.
> 
> struct ice_stats {
>         char stat_string[ETH_GSTRING_LEN];
>         int sizeof_stat;
>         int stat_offset;
> };
> 
> #define ICE_STAT(_type, _name, _stat) { \
>         .stat_string = _name, \
>         .sizeof_stat = sizeof_field(_type, _stat), \
>         .stat_offset = offsetof(_type, _stat) \
> }
> 
> #define ICE_VSI_STAT(_name, _stat) \
>                 ICE_STAT(struct ice_vsi, _name, _stat)
> #define ICE_PF_STAT(_name, _stat) \
>                 ICE_STAT(struct ice_pf, _name, _stat)
> 
> 
> Then the stats for the individial arrays are defined like this:
> 
> static const struct ice_stats ice_gstrings_vsi_stats[] = {
>         ICE_VSI_STAT(ICE_RX_UNICAST, eth_stats.rx_unicast),
>         ICE_VSI_STAT(ICE_TX_UNICAST, eth_stats.tx_unicast),
>         ICE_VSI_STAT(ICE_RX_MULTICAST, eth_stats.rx_multicast),
>         ICE_VSI_STAT(ICE_TX_MULTICAST, eth_stats.tx_multicast),
>         ICE_VSI_STAT(ICE_RX_BROADCAST, eth_stats.rx_broadcast),
>         ICE_VSI_STAT(ICE_TX_BROADCAST, eth_stats.tx_broadcast),
> 	...
> };
> 
> (Note, ICE_RX_UNICAST is a macro that defines the string value.. I don't
> recall who changed this to macros or why vs just having the strings be
> directly in the definition...)
> 

Thanks for sharing the ice driver pattern — that's a clean design.

> This is probably a lot bigger refactor to make work, and may not be
> exactly suitable for your driver. I've considered "upgrading" these data

Yes, I need to see if refactoring is applicable to ICSSG or not. I will
look into this and send a separate patch / series in future if
applicable. For this series I will stick with what David Carlier suggested.

> structures and logic as helpers to the core ethtool code (or perhaps
> now, to libeth) but never got around to it.


-- 
Thanks and Regards,
Danish



^ permalink raw reply

* Re: [PATCH v4 02/13] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Aneesh Kumar K.V @ 2026-05-14  5:01 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: <agSDk0QsEM0ZBCTA@google.com>

Mostafa Saleh <smostafa@google.com> writes:

> On Tue, May 12, 2026 at 02:33:57PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Propagate force_dma_unencrypted() into DMA_ATTR_CC_SHARED in the
>> dma-direct allocation path and use the attribute to drive the related
>> decisions.
>> 
>> This updates dma_direct_alloc(), dma_direct_free(), and
>> dma_direct_alloc_pages() to fold the forced unencrypted case into attrs.
>> 
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>>  kernel/dma/direct.c | 44 ++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 36 insertions(+), 8 deletions(-)
>> 
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index b958f150718a..0c2e1f8436ce 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -201,16 +201,31 @@ 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;
>> +	bool mark_mem_decrypt = false;
>>  	struct page *page;
>>  	void *ret;
>>  
>> +	/*
>> +	 * DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
>> +	 * attribute. The direct allocator uses it internally after it has
>> +	 * decided that the backing pages must be shared/decrypted, so the
>> +	 * rest of the allocation path can consistently select DMA addresses,
>> +	 * choose compatible pools and restore encryption on free.
>> +	 */
>> +	if (attrs & DMA_ATTR_CC_SHARED)
>> +		return NULL;
>> +
>> +	if (force_dma_unencrypted(dev)) {
>> +		attrs |= DMA_ATTR_CC_SHARED;
>> +		mark_mem_decrypt = true;
>> +	}
>> +
>>  	size = PAGE_ALIGN(size);
>>  	if (attrs & DMA_ATTR_NO_WARN)
>>  		gfp |= __GFP_NOWARN;
>>  
>> -	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> -	    !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev))
>> +	if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
>> +	     DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev))
>>  		return dma_direct_alloc_no_mapping(dev, size, dma_handle, gfp);
>>  
>>  	if (!dev_is_dma_coherent(dev)) {
>> @@ -244,7 +259,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>  	 * Remapping or decrypting memory may block, allocate the memory from
>>  	 * the atomic pools instead if we aren't allowed block.
>>  	 */
>> -	if ((remap || force_dma_unencrypted(dev)) &&
>> +	if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
>>  	    dma_direct_use_pool(dev, gfp))
>>  		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>  
>> @@ -318,11 +333,20 @@ 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;
>> +	bool mark_mem_encrypted = false;
>>  	unsigned int page_order = get_order(size);
>>  
>> -	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> -	    !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev)) {
>> +	/*
>> +	 * if the device had requested for an unencrypted buffer,
>> +	 * convert it to encrypted on free
>> +	 */
>> +	if (force_dma_unencrypted(dev)) {
>> +		attrs |= DMA_ATTR_CC_SHARED;
>> +		mark_mem_encrypted = true;
>> +	}
>> +
>> +	if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
>> +	     DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev)) {
>>  		/* cpu_addr is a struct page cookie, not a kernel address */
>>  		dma_free_contiguous(dev, cpu_addr, size);
>>  		return;
>> @@ -365,10 +389,14 @@ void dma_direct_free(struct device *dev, size_t size,
>>  struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>>  		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp)
>>  {
>> +	unsigned long attrs = 0;
>>  	struct page *page;
>>  	void *ret;
>>  
>> -	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
>> +	if (force_dma_unencrypted(dev))
>> +		attrs |= DMA_ATTR_CC_SHARED;
>> +
>> +	if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
>>  		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>
> What about dma_direct_free_pages()? Nothing inside uses attrs, but
> that’s quite similar to dma_direct_alloc_pages()
>
> Also, at this point, shouldn’t this patch also remove
> force_dma_unencrypted() calls from dma_set_decrypted() and
> dma_set_encrypted()?
>

The final change have 

void dma_direct_free_pages(struct device *dev, size_t size,
...
{
	/*
	 * if the device had requested for an unencrypted buffer,
	 * convert it to encrypted on free
	 */
	bool mark_mem_encrypted =  force_dma_unencrypted(dev);

That got added by "dma-direct: select DMA address encoding from DMA_ATTR_CC_SHARED "

I'll move that hunk into this patch. The overall goal is 

dma_direct_alloc(.. attrs)
{

	if (force_dma_unencrypted(dev))
		attrs |= DMA_ATTR_CC_SHARED;
}

dma_direct_free(.. attrs)
{
	if (force_dma_unencrypted(dev)) {
		attrs |= DMA_ATTR_CC_SHARED;
		mark_mem_encrypted = true;
	}

	if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
		mark_mem_encrypted = false;

}

dma_direct_alloc_pages()
{
	if (force_dma_unencrypted(dev))
		attrs |= DMA_ATTR_CC_SHARED;

}

dma_direct_free_pages()
{
	bool mark_mem_encrypted =  force_dma_unencrypted(dev);

	if (swiotlb_find_pool(dev, page_to_phys(page)))
		mark_mem_encrypted = false;

}

-aneesh


^ permalink raw reply

* Re: [PATCH v2] drm/bridge: imx93-mipi-dsi: Fix mode validation
From: Liu Ying @ 2026-05-14  5:55 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Frank Li,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Luca Ceresoli
  Cc: Dmitry Baryshkov, dri-devel, imx, linux-arm-kernel, linux-kernel,
	sashiko-bot
In-Reply-To: <20260512-imx93-mipi-dsi-fix-mode-validation-v2-1-7aec3be5da2c@nxp.com>

On Tue, May 12, 2026 at 05:18:49PM +0800, Liu Ying wrote:
> i.MX93 MIPI DPHY PLL has limitation for matching with some pixel clock
> rates, e.g., the best DPHY PLL frequency is 445.333333MHz for a typical
> 1920x1080p@60Hz CEA/DMT display modes with a pixel clock rate running
> at 148.5MHz with 4 data lanes + RGB888 pixel in MIPI DSI sync pulse mode,
> while the expected PLL frequency is (148.5 * 24) / 4 / 2 MHz = 445.5MHz.
> Fortunately, VESA Display Monitor Timing Standard allows +/-0.5% pixel
> clock rate deviation for timings.  So, for those display modes read
> from EDID through a bridge with DRM_BRIDGE_OP_DETECT and DRM_BRIDGE_OP_EDID
> operation bit masks set, pixel clock rate could be adjusted to match
> with the PLL frequency(for the above example, the pixel clock rate is
> adjusted to be 148.444444MHz with about -0.03% deviation from the 148.5MHz
> nominal rate so that the adjusted rate matches with the 445.333333MHz PLL
> frequency).
> 
> Instead of checking the last bridge's operation bit masks against
> DRM_BRIDGE_OP_DETECT and DRM_BRIDGE_OP_EDID to determine if allowing
> +/-0.5% pixel clock rate deviation, check any bridge after this bridge,
> because the last bridge is usually a display connector bridge without
> any operation bit mask when the clock rate deviation is allowed.
> 
> Fixes: ce62f8ea7e3f ("drm/bridge: imx: Add i.MX93 MIPI DSI support")
> Fixes: 5849eff7f067 ("drm/bridge: imx93-mipi-dsi: use drm_bridge_chain_get_last_bridge()")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> Changes in v2:
> - Collect Frank's R-b tag.
> - Add an explanation to commit message about the reason why mode validation
>   checks bridge's operation bit masks.  (Dmitry)
> - Copy Dmitry.
> - Link to v1: https://lore.kernel.org/r/20260227-imx93-mipi-dsi-fix-mode-validation-v1-1-a9cd67991280@nxp.com
> 
> To: Liu Ying <victor.liu@nxp.com>
> To: Andrzej Hajda <andrzej.hajda@intel.com>
> To: Neil Armstrong <neil.armstrong@linaro.org>
> To: Robert Foss <rfoss@kernel.org>
> To: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> To: Jonas Karlman <jonas@kwiboo.se>
> To: Jernej Skrabec <jernej.skrabec@gmail.com>
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> To: Maxime Ripard <mripard@kernel.org>
> To: Thomas Zimmermann <tzimmermann@suse.de>
> To: David Airlie <airlied@gmail.com>
> To: Simona Vetter <simona@ffwll.ch>
> To: Frank Li <Frank.Li@nxp.com>
> To: Sascha Hauer <s.hauer@pengutronix.de>
> To: Pengutronix Kernel Team <kernel@pengutronix.de>
> To: Fabio Estevam <festevam@gmail.com>
> To: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)

Just for the record, sashiko bot reported a deadlock issue[1], but didn't copy
all receivers.  I think that's a real issue, so would fix it.

[1] https://lore.kernel.org/all/20260513201715.AACA2C19425@smtp.kernel.org/

-- 
Regards,
Liu Ying


^ permalink raw reply

* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-05-14  5: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: <agSKQrSIhizCXKwx@google.com>

Mostafa Saleh <smostafa@google.com> writes:

> On Tue, May 12, 2026 at 02:33:59PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Teach swiotlb to distinguish between encrypted and decrypted bounce
>> buffer pools, and make allocation and mapping paths select a pool whose
>> state matches the requested DMA attributes.
>> 
>> Add a decrypted flag to io_tlb_mem, initialize it for the default and
>> restricted pools, and propagate DMA_ATTR_CC_SHARED into swiotlb pool
>> allocation. Reject swiotlb alloc/map requests when the selected pool does
>> not match the required encrypted/decrypted state.
>> 
>> Also return DMA addresses with the matching phys_to_dma_{encrypted,
>> unencrypted} helper so the DMA address encoding stays consistent with the
>> chosen pool.
>> 
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>>  include/linux/dma-direct.h |  10 ++++
>>  include/linux/swiotlb.h    |   8 ++-
>>  kernel/dma/direct.c        |  14 +++--
>>  kernel/dma/swiotlb.c       | 108 +++++++++++++++++++++++++++----------
>>  4 files changed, 107 insertions(+), 33 deletions(-)
>> 
>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>> index c249912456f9..94fad4e7c11e 100644
>> --- a/include/linux/dma-direct.h
>> +++ b/include/linux/dma-direct.h
>> @@ -77,6 +77,10 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
>>  #ifndef phys_to_dma_unencrypted
>>  #define phys_to_dma_unencrypted		phys_to_dma
>>  #endif
>> +
>> +#ifndef phys_to_dma_encrypted
>> +#define phys_to_dma_encrypted		phys_to_dma
>> +#endif
>>  #else
>>  static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
>>  {
>> @@ -90,6 +94,12 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
>>  {
>>  	return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
>>  }
>> +
>> +static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
>> +		phys_addr_t paddr)
>> +{
>> +	return dma_addr_encrypted(__phys_to_dma(dev, paddr));
>> +}
>>  /*
>>   * If memory encryption is supported, phys_to_dma will set the memory encryption
>>   * bit in the DMA address, and dma_to_phys will clear it.
>> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
>> index 3dae0f592063..b3fa3c6e0169 100644
>> --- a/include/linux/swiotlb.h
>> +++ b/include/linux/swiotlb.h
>> @@ -81,6 +81,7 @@ struct io_tlb_pool {
>>  	struct list_head node;
>>  	struct rcu_head rcu;
>>  	bool transient;
>> +	bool unencrypted;
>>  #endif
>>  };
>>  
>> @@ -111,6 +112,7 @@ struct io_tlb_mem {
>>  	struct dentry *debugfs;
>>  	bool force_bounce;
>>  	bool for_alloc;
>> +	bool unencrypted;
>>  #ifdef CONFIG_SWIOTLB_DYNAMIC
>>  	bool can_grow;
>>  	u64 phys_limit;
>> @@ -282,7 +284,8 @@ static inline void swiotlb_sync_single_for_cpu(struct device *dev,
>>  extern void swiotlb_print_info(void);
>>  
>>  #ifdef CONFIG_DMA_RESTRICTED_POOL
>> -struct page *swiotlb_alloc(struct device *dev, size_t size);
>> +struct page *swiotlb_alloc(struct device *dev, size_t size,
>> +		unsigned long attrs);
>>  bool swiotlb_free(struct device *dev, struct page *page, size_t size);
>>  
>>  static inline bool is_swiotlb_for_alloc(struct device *dev)
>> @@ -290,7 +293,8 @@ static inline bool is_swiotlb_for_alloc(struct device *dev)
>>  	return dev->dma_io_tlb_mem->for_alloc;
>>  }
>>  #else
>> -static inline struct page *swiotlb_alloc(struct device *dev, size_t size)
>> +static inline struct page *swiotlb_alloc(struct device *dev, size_t size,
>> +		unsigned long attrs)
>>  {
>>  	return NULL;
>>  }
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index dc2907439b3d..97ae4fa10521 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -104,9 +104,10 @@ static void __dma_direct_free_pages(struct device *dev, struct page *page,
>>  	dma_free_contiguous(dev, page, size);
>>  }
>>  
>> -static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
>> +static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size,
>> +		unsigned long attrs)
>>  {
>> -	struct page *page = swiotlb_alloc(dev, size);
>> +	struct page *page = swiotlb_alloc(dev, size, attrs);
>>  
>>  	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
>>  		swiotlb_free(dev, page, size);
>> @@ -266,8 +267,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>  						  gfp, attrs);
>>  
>>  	if (is_swiotlb_for_alloc(dev)) {
>> -		page = dma_direct_alloc_swiotlb(dev, size);
>> +		page = dma_direct_alloc_swiotlb(dev, size, attrs);
>>  		if (page) {
>> +			/*
>> +			 * swiotlb allocations comes from pool already marked
>> +			 * decrypted
>> +			 */
>>  			mark_mem_decrypt = false;
>>  			goto setup_page;
>>  		}
>> @@ -374,6 +379,7 @@ void dma_direct_free(struct device *dev, size_t size,
>>  		return;
>>  
>>  	if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
>> +		/* Swiotlb doesn't need a page attribute update on free */
>>  		mark_mem_encrypted = false;
>>  
>>  	if (is_vmalloc_addr(cpu_addr)) {
>> @@ -403,7 +409,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>>  						  gfp, attrs);
>>  
>>  	if (is_swiotlb_for_alloc(dev)) {
>> -		page = dma_direct_alloc_swiotlb(dev, size);
>> +		page = dma_direct_alloc_swiotlb(dev, size, attrs);
>>  		if (!page)
>>  			return NULL;
>>  
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index ab4eccbaa076..065663be282c 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -259,10 +259,21 @@ void __init swiotlb_update_mem_attributes(void)
>>  	struct io_tlb_pool *mem = &io_tlb_default_mem.defpool;
>>  	unsigned long bytes;
>>  
>> +	/*
>> +	 * if platform support memory encryption, swiotlb buffers are
>> +	 * decrypted by default.
>> +	 */
>> +	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> +		io_tlb_default_mem.unencrypted = true;
>> +	else
>> +		io_tlb_default_mem.unencrypted = false;
>> +
>>  	if (!mem->nslabs || mem->late_alloc)
>>  		return;
>>  	bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
>> -	set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
>> +
>> +	if (io_tlb_default_mem.unencrypted)
>> +		set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
>>  }
>>  
>>  static void swiotlb_init_io_tlb_pool(struct io_tlb_pool *mem, phys_addr_t start,
>> @@ -505,8 +516,10 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
>>  	if (!mem->slots)
>>  		goto error_slots;
>>  
>> -	set_memory_decrypted((unsigned long)vstart,
>> -			     (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
>> +	if (io_tlb_default_mem.unencrypted)
>> +		set_memory_decrypted((unsigned long)vstart,
>> +				     (nslabs << IO_TLB_SHIFT) >> PAGE_SHIFT);
>> +
>>  	swiotlb_init_io_tlb_pool(mem, virt_to_phys(vstart), nslabs, true,
>>  				 nareas);
>>  	add_mem_pool(&io_tlb_default_mem, mem);
>> @@ -539,7 +552,9 @@ void __init swiotlb_exit(void)
>>  	tbl_size = PAGE_ALIGN(mem->end - mem->start);
>>  	slots_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), mem->nslabs));
>>  
>> -	set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
>> +	if (io_tlb_default_mem.unencrypted)
>> +		set_memory_encrypted(tbl_vaddr, tbl_size >> PAGE_SHIFT);
>> +
>>  	if (mem->late_alloc) {
>>  		area_order = get_order(array_size(sizeof(*mem->areas),
>>  			mem->nareas));
>> @@ -563,6 +578,7 @@ void __init swiotlb_exit(void)
>>   * @gfp:	GFP flags for the allocation.
>>   * @bytes:	Size of the buffer.
>>   * @phys_limit:	Maximum allowed physical address of the buffer.
>> + * @unencrypted: true to allocate unencrypted memory, false for encrypted memory
>>   *
>>   * Allocate pages from the buddy allocator. If successful, make the allocated
>>   * pages decrypted that they can be used for DMA.
>> @@ -570,7 +586,8 @@ void __init swiotlb_exit(void)
>>   * Return: Decrypted pages, %NULL on allocation failure, or ERR_PTR(-EAGAIN)
>>   * if the allocated physical address was above @phys_limit.
>>   */
>> -static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
>> +static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes,
>> +		u64 phys_limit, bool unencrypted)
>>  {
>>  	unsigned int order = get_order(bytes);
>>  	struct page *page;
>> @@ -588,13 +605,13 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
>>  	}
>>  
>>  	vaddr = phys_to_virt(paddr);
>> -	if (set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> +	if (unencrypted && set_memory_decrypted((unsigned long)vaddr, PFN_UP(bytes)))
>>  		goto error;
>>  	return page;
>>  
>>  error:
>>  	/* Intentional leak if pages cannot be encrypted again. */
>> -	if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> +	if (unencrypted && !set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>>  		__free_pages(page, order);
>>  	return NULL;
>>  }
>> @@ -604,30 +621,26 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit)
>>   * @dev:	Device for which a memory pool is allocated.
>>   * @bytes:	Size of the buffer.
>>   * @phys_limit:	Maximum allowed physical address of the buffer.
>> + * @attrs:	DMA attributes for the allocation.
>>   * @gfp:	GFP flags for the allocation.
>>   *
>>   * Return: Allocated pages, or %NULL on allocation failure.
>>   */
>>  static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>> -		u64 phys_limit, gfp_t gfp)
>> +		u64 phys_limit, unsigned long attrs, gfp_t gfp)
>>  {
>>  	struct page *page;
>> -	unsigned long attrs = 0;
>>  
>>  	/*
>>  	 * Allocate from the atomic pools if memory is encrypted and
>>  	 * the allocation is atomic, because decrypting may block.
>>  	 */
>> -	if (!gfpflags_allow_blocking(gfp) && dev && force_dma_unencrypted(dev)) {
>> +	if (!gfpflags_allow_blocking(gfp) && (attrs & DMA_ATTR_CC_SHARED)) {
>>  		void *vaddr;
>>  
>>  		if (!IS_ENABLED(CONFIG_DMA_COHERENT_POOL))
>>  			return NULL;
>>  
>> -		/* swiotlb considered decrypted by default */
>> -		if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> -			attrs = DMA_ATTR_CC_SHARED;
>> -
>>  		return dma_alloc_from_pool(dev, bytes, &vaddr, gfp,
>>  					   attrs, dma_coherent_ok);
>>  	}
>> @@ -638,7 +651,8 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>>  	else if (phys_limit <= DMA_BIT_MASK(32))
>>  		gfp |= __GFP_DMA32;
>>  
>> -	while (IS_ERR(page = alloc_dma_pages(gfp, bytes, phys_limit))) {
>> +	while (IS_ERR(page = alloc_dma_pages(gfp, bytes, phys_limit,
>> +					     !!(attrs & DMA_ATTR_CC_SHARED)))) {
>>  		if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
>>  		    phys_limit < DMA_BIT_MASK(64) &&
>>  		    !(gfp & (__GFP_DMA32 | __GFP_DMA)))
>> @@ -657,15 +671,18 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>>   * swiotlb_free_tlb() - free a dynamically allocated IO TLB buffer
>>   * @vaddr:	Virtual address of the buffer.
>>   * @bytes:	Size of the buffer.
>> + * @unencrypted: true if @vaddr was allocated decrypted and must be
>> + *	re-encrypted before being freed
>>   */
>> -static void swiotlb_free_tlb(void *vaddr, size_t bytes)
>> +static void swiotlb_free_tlb(void *vaddr, size_t bytes, bool unencrypted)
>>  {
>>  	if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
>>  	    dma_free_from_pool(NULL, vaddr, bytes))
>>  		return;
>>  
>>  	/* Intentional leak if pages cannot be encrypted again. */
>> -	if (!set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>> +	if (!unencrypted ||
>> +	    !set_memory_encrypted((unsigned long)vaddr, PFN_UP(bytes)))
>>  		__free_pages(virt_to_page(vaddr), get_order(bytes));
>>  }
>>  
>> @@ -676,6 +693,7 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
>>   * @nslabs:	Desired (maximum) number of slabs.
>>   * @nareas:	Number of areas.
>>   * @phys_limit:	Maximum DMA buffer physical address.
>> + * @attrs:	DMA attributes for the allocation.
>>   * @gfp:	GFP flags for the allocations.
>>   *
>>   * Allocate and initialize a new IO TLB memory pool. The actual number of
>> @@ -686,7 +704,8 @@ static void swiotlb_free_tlb(void *vaddr, size_t bytes)
>>   */
>>  static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
>>  		unsigned long minslabs, unsigned long nslabs,
>> -		unsigned int nareas, u64 phys_limit, gfp_t gfp)
>> +		unsigned int nareas, u64 phys_limit, unsigned long attrs,
>> +		gfp_t gfp)
>>  {
>>  	struct io_tlb_pool *pool;
>>  	unsigned int slot_order;
>> @@ -704,9 +723,10 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
>>  	if (!pool)
>>  		goto error;
>>  	pool->areas = (void *)pool + sizeof(*pool);
>> +	pool->unencrypted = !!(attrs & DMA_ATTR_CC_SHARED);
>>  
>>  	tlb_size = nslabs << IO_TLB_SHIFT;
>> -	while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, gfp))) {
>> +	while (!(tlb = swiotlb_alloc_tlb(dev, tlb_size, phys_limit, attrs, gfp))) {
>>  		if (nslabs <= minslabs)
>>  			goto error_tlb;
>>  		nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
>> @@ -724,7 +744,8 @@ static struct io_tlb_pool *swiotlb_alloc_pool(struct device *dev,
>>  	return pool;
>>  
>>  error_slots:
>> -	swiotlb_free_tlb(page_address(tlb), tlb_size);
>> +	swiotlb_free_tlb(page_address(tlb), tlb_size,
>> +			 !!(attrs & DMA_ATTR_CC_SHARED));
>>  error_tlb:
>>  	kfree(pool);
>>  error:
>> @@ -742,7 +763,9 @@ static void swiotlb_dyn_alloc(struct work_struct *work)
>>  	struct io_tlb_pool *pool;
>>  
>>  	pool = swiotlb_alloc_pool(NULL, IO_TLB_MIN_SLABS, default_nslabs,
>> -				  default_nareas, mem->phys_limit, GFP_KERNEL);
>> +				  default_nareas, mem->phys_limit,
>> +				  mem->unencrypted ? DMA_ATTR_CC_SHARED : 0,
>> +				  GFP_KERNEL);
>>  	if (!pool) {
>>  		pr_warn_ratelimited("Failed to allocate new pool");
>>  		return;
>> @@ -762,7 +785,7 @@ static void swiotlb_dyn_free(struct rcu_head *rcu)
>>  	size_t tlb_size = pool->end - pool->start;
>>  
>>  	free_pages((unsigned long)pool->slots, get_order(slots_size));
>> -	swiotlb_free_tlb(pool->vaddr, tlb_size);
>> +	swiotlb_free_tlb(pool->vaddr, tlb_size, pool->unencrypted);
>>  	kfree(pool);
>>  }
>>  
>> @@ -1232,6 +1255,7 @@ static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
>>  	nslabs = nr_slots(alloc_size);
>>  	phys_limit = min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
>>  	pool = swiotlb_alloc_pool(dev, nslabs, nslabs, 1, phys_limit,
>> +				  mem->unencrypted ? DMA_ATTR_CC_SHARED : 0,
>>  				  GFP_NOWAIT);
>>  	if (!pool)
>>  		return -1;
>> @@ -1394,6 +1418,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
>>  		enum dma_data_direction dir, unsigned long attrs)
>>  {
>>  	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
>> +	bool require_decrypted = false;
>>  	unsigned int offset;
>>  	struct io_tlb_pool *pool;
>>  	unsigned int i;
>> @@ -1411,6 +1436,16 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
>>  	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>>  		pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
>>  
>> +	/*
>> +	 * if we are trying to swiotlb map a decrypted paddr or the paddr is encrypted
>> +	 * but the device is forcing decryption, use decrypted io_tlb_mem
>> +	 */
>> +	if ((attrs & DMA_ATTR_CC_SHARED) || force_dma_unencrypted(dev))
>> +		require_decrypted = true;
>> +
>> +	if (require_decrypted != mem->unencrypted)
>> +		return (phys_addr_t)DMA_MAPPING_ERROR;
>> +
>>  	/*
>>  	 * The default swiotlb memory pool is allocated with PAGE_SIZE
>>  	 * alignment. If a mapping is requested with larger alignment,
>> @@ -1608,8 +1643,14 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
>>  	if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
>>  		return DMA_MAPPING_ERROR;
>>  
>> -	/* Ensure that the address returned is DMA'ble */
>> -	dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
>> +	/*
>> +	 * Use the allocated io_tlb_mem encryption type to determine dma addr.
>> +	 */
>> +	if (dev->dma_io_tlb_mem->unencrypted)
>> +		dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
>> +	else
>> +		dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
>> +
>>  	if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
>>  		__swiotlb_tbl_unmap_single(dev, swiotlb_addr, size, dir,
>>  			attrs | DMA_ATTR_SKIP_CPU_SYNC,
>> @@ -1773,7 +1814,8 @@ static inline void swiotlb_create_debugfs_files(struct io_tlb_mem *mem,
>>  
>>  #ifdef CONFIG_DMA_RESTRICTED_POOL
>>  
>> -struct page *swiotlb_alloc(struct device *dev, size_t size)
>> +struct page *swiotlb_alloc(struct device *dev, size_t size,
>> +		unsigned long attrs)
>>  {
>>  	struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
>>  	struct io_tlb_pool *pool;
>> @@ -1784,6 +1826,9 @@ struct page *swiotlb_alloc(struct device *dev, size_t size)
>>  	if (!mem)
>>  		return NULL;
>>  
>> +	if (mem->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
>> +		return NULL;
>> +
>>  	align = (1 << (get_order(size) + PAGE_SHIFT)) - 1;
>>  	index = swiotlb_find_slots(dev, 0, size, align, &pool);
>>  	if (index == -1)
>> @@ -1853,9 +1898,18 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>>  			kfree(mem);
>>  			return -ENOMEM;
>>  		}
>> +		/*
>> +		 * if platform supports memory encryption,
>> +		 * restricted mem pool is decrypted by default
>> +		 */
>> +		if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
>> +			mem->unencrypted = true;
>> +			set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>> +					     rmem->size >> PAGE_SHIFT);
>> +		} else {
>> +			mem->unencrypted = false;
>> +		}
>
> This breaks pKVM as it doesn’t set CC_ATTR_MEM_ENCRYPT, so all virtio
> traffic now fails.
>
> Also, by design, some drivers are clueless about bouncing, so
> I believe that the pool should have a way to control it’s property
> (encrypted or decrypted) and that takes priority over whatever
> attributes comes from allocation.
> And that brings us to the same point whether it’s better to return
> the memory along with it’s state or we pass the requested state.
> I think for other cases it’s fine for the device/DMA-API to dictate
> the attrs, but not in restricted-dma case, the firmware just knows better.
>

Is it that the pKVM guest kernel does not have awareness of
encrypted/decrypted DMA allocations? Instead, the firmware attaches
hypervisor-shared pages to the device via restricted-dma-pool? The
kernel then has swiotlb->for_alloc = true, and hence all DMA allocations
go through the restricted-dma-pool?

Given that pKVM supports pkvm_set_memory_encrypted() and
pkvm_set_memory_decrypted(), can we consider adding CC_ATTR_MEM_ENCRYPT
support to pKVM? It would also be good to investigate whether we can set
force_dma_unencrypted(dev) to true where needed.

I agree that this patch, as it stands, can break pKVM because we are now
missing the set_memory_decrypted() call required for pKVM to work.

We now mark the swiotlb io_tlb_mem as unencrypted/encrypted in the guest
using struct io_tlb_mem->unencrypted. I am not clear what we can use for
pKVM to conditionalize this so that it works for both protected and
unprotected guests.

-aneesh



^ permalink raw reply

* Re: [PATCH v2 1/6] devfreq: Use mutex guard in governor_store()
From: Yaxiong Tian @ 2026-05-14  6:02 UTC (permalink / raw)
  To: Jie Zhan, cwchoi00, cw00.choi, myungjoo.ham, kyungmin.park,
	linux-pm, linux-arm-kernel
  Cc: linuxarm, zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260513093832.1645890-2-zhanjie9@hisilicon.com>


在 2026/5/13 17:38, Jie Zhan 写道:
> Use mutex guard in governor_store() so as to simplify the locking logic.
>
> No functional impact intended.
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>   drivers/devfreq/devfreq.c | 38 ++++++++++++++++----------------------
>   1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eae..7a70dd051644 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1394,23 +1394,20 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	if (ret != 1)
>   		return -EINVAL;
>   
> -	mutex_lock(&devfreq_list_lock);
> +	guard(mutex)(&devfreq_list_lock);
>   	governor = try_then_request_governor(str_governor);
> -	if (IS_ERR(governor)) {
> -		ret = PTR_ERR(governor);
> -		goto out;
> -	}
> +	if (IS_ERR(governor))
> +		return PTR_ERR(governor);
> +
>   	if (!df->governor)
>   		goto start_new_governor;
>   
> -	if (df->governor == governor) {
> -		ret = 0;
> -		goto out;
> -	} else if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)
> -		|| IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE)) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> +	if (df->governor == governor)
> +		return count;
> +
> +	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE) ||
> +	    IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE))
> +		return -EINVAL;
>   
>   	/*
>   	 * Stop the current governor and remove the specific sysfs files
> @@ -1420,7 +1417,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	if (ret) {
>   		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
>   			 __func__, df->governor->name, ret);
> -		goto out;
> +		return ret;
>   	}
>   
>   start_new_governor:
> @@ -1438,7 +1435,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   		/* Restore previous governor */
>   		df->governor = prev_governor;
>   		if (!df->governor)
> -			goto out;
> +			return ret;
>   
>   		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>   		if (ret) {
> @@ -1446,7 +1443,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   				"%s: reverting to Governor %s failed (%d)\n",
>   				__func__, prev_governor->name, ret);
>   			df->governor = NULL;
> -			goto out;
> +			return ret;
>   		}
>   	}
>   
> @@ -1455,13 +1452,10 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	 * the new governor, restore the sysfs files of previous governor.
>   	 */
>   	ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
> +	if (ret)
> +		return ret;
>   
> -out:
> -	mutex_unlock(&devfreq_list_lock);
> -
> -	if (!ret)
> -		ret = count;
> -	return ret;
> +	return count;
>   }
>   static DEVICE_ATTR_RW(governor);
>   
Reviewed-by: Yaxiong Tian <tianyaxiong@kylinos.cn>


^ permalink raw reply

* Re: [PATCH v2 2/6] devfreq: Use mutex guard in devfreq_add/remove_governor()
From: Yaxiong Tian @ 2026-05-14  6:02 UTC (permalink / raw)
  To: Jie Zhan, cwchoi00, cw00.choi, myungjoo.ham, kyungmin.park,
	linux-pm, linux-arm-kernel
  Cc: linuxarm, zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260513093832.1645890-3-zhanjie9@hisilicon.com>


在 2026/5/13 17:38, Jie Zhan 写道:
> Use mutex guard in devfreq_add/remove_governor() so as to simplify the
> locking logic.
>
> No functional impact intended.
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>   drivers/devfreq/devfreq.c | 22 +++++++---------------
>   1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 7a70dd051644..53c40d795a13 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1261,28 +1261,23 @@ void devfreq_resume(void)
>   int devfreq_add_governor(struct devfreq_governor *governor)
>   {
>   	struct devfreq_governor *g;
> -	int err = 0;
>   
>   	if (!governor) {
>   		pr_err("%s: Invalid parameters.\n", __func__);
>   		return -EINVAL;
>   	}
>   
> -	mutex_lock(&devfreq_list_lock);
> +	guard(mutex)(&devfreq_list_lock);
>   	g = find_devfreq_governor(governor->name);
>   	if (!IS_ERR(g)) {
>   		pr_err("%s: governor %s already registered\n", __func__,
>   		       g->name);
> -		err = -EINVAL;
> -		goto err_out;
> +		return -EINVAL;
>   	}
>   
>   	list_add(&governor->node, &devfreq_governor_list);
>   
> -err_out:
> -	mutex_unlock(&devfreq_list_lock);
> -
> -	return err;
> +	return 0;
>   }
>   EXPORT_SYMBOL(devfreq_add_governor);
>   
> @@ -1320,21 +1315,20 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>   {
>   	struct devfreq_governor *g;
>   	struct devfreq *devfreq;
> -	int err = 0;
>   
>   	if (!governor) {
>   		pr_err("%s: Invalid parameters.\n", __func__);
>   		return -EINVAL;
>   	}
>   
> -	mutex_lock(&devfreq_list_lock);
> +	guard(mutex)(&devfreq_list_lock);
>   	g = find_devfreq_governor(governor->name);
>   	if (IS_ERR(g)) {
>   		pr_err("%s: governor %s not registered\n", __func__,
>   		       governor->name);
> -		err = PTR_ERR(g);
> -		goto err_out;
> +		return PTR_ERR(g);
>   	}
> +
>   	list_for_each_entry(devfreq, &devfreq_list, node) {
>   		int ret;
>   		struct device *dev = devfreq->dev.parent;
> @@ -1356,10 +1350,8 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>   	}
>   
>   	list_del(&governor->node);
> -err_out:
> -	mutex_unlock(&devfreq_list_lock);
>   
> -	return err;
> +	return 0;
>   }
>   EXPORT_SYMBOL(devfreq_remove_governor);
>   
Reviewed-by: Yaxiong Tian <tianyaxiong@kylinos.cn>


^ permalink raw reply

* Re: [PATCH v2 3/6] devfreq: Factor out devfreq_set_governor()
From: Yaxiong Tian @ 2026-05-14  6:09 UTC (permalink / raw)
  To: Jie Zhan, cwchoi00, cw00.choi, myungjoo.ham, kyungmin.park,
	linux-pm, linux-arm-kernel
  Cc: linuxarm, zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260513093832.1645890-4-zhanjie9@hisilicon.com>


在 2026/5/13 17:38, Jie Zhan 写道:
> Factor out common governor setting logic into devfreq_set_governor() so as
> to consolidate the code in governor_store() and devfreq_add_device().
>
> The caller is expected to hold 'devfreq_list_lock', enforced via
> lockdep_assert_held().  This is required because devfreq_add_device() must
> hold the lock from setting governor until the device is added to
> 'devfreq_list', so that a concurrent devfreq_remove_governor() cannot free
> the governor in between.
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>

I'm a bit unclear about the purpose of this commit, or the problem it 
solves. Can you explain it more clearly?


> ---
>   drivers/devfreq/devfreq.c | 117 ++++++++++++++++++--------------------
>   1 file changed, 56 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 53c40d795a13..9e3e6a7348f8 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -318,6 +318,59 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
>   	return governor;
>   }
>   
> +static int devfreq_set_governor(struct devfreq *df,
> +				const struct devfreq_governor *new_gov)
> +{
> +	const struct devfreq_governor *old_gov;
> +	struct device *dev;
> +	int ret;
> +
> +	lockdep_assert_held(&devfreq_list_lock);
> +
> +	old_gov = df->governor;
> +	dev = &df->dev;
> +
> +	if (old_gov) {
> +		if (old_gov == new_gov)
> +			return 0;
> +
> +		if (IS_SUPPORTED_FLAG(old_gov->flags, IMMUTABLE) ||
> +		    IS_SUPPORTED_FLAG(new_gov->flags, IMMUTABLE))
> +			return -EINVAL;
> +
> +		/* Stop the current governor */
> +		ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
> +		if (ret) {
> +			dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> +				 __func__, df->governor->name, ret);
> +			return ret;
> +		}
> +	}
> +
> +	/* Start the new governor */
> +	df->governor = new_gov;
> +	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> +	if (ret) {
> +		dev_warn(dev, "%s: Governor %s not started(%d)\n",
> +			 __func__, df->governor->name, ret);
> +
> +		/* Restore previous governor */
> +		df->governor = old_gov;
> +		if (!df->governor)
> +			return ret;
> +
> +		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> +		if (ret) {
> +			dev_err(dev, "%s: restore Governor %s failed (%d)\n",
> +				__func__, df->governor->name, ret);
> +			df->governor = NULL;
> +			return ret;
> +		}
> +	}
> +
> +	return sysfs_update_group(&df->dev.kobj, &gov_attr_group);
> +}
> +
>   static int devfreq_notify_transition(struct devfreq *devfreq,
>   		struct devfreq_freqs *freqs, unsigned int state)
>   {
> @@ -942,9 +995,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>   		goto err_init;
>   	}
>   
> -	devfreq->governor = governor;
> -	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
> -						NULL);
> +	err = devfreq_set_governor(devfreq, governor);
>   	if (err) {
>   		dev_err_probe(dev, err,
>   			"%s: Unable to start governor for the device\n",
> @@ -952,10 +1003,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>   		goto err_init;
>   	}
>   
> -	err = sysfs_update_group(&devfreq->dev.kobj, &gov_attr_group);
> -	if (err)
> -		goto err_init;
> -
>   	list_add(&devfreq->node, &devfreq_list);
>   
>   	mutex_unlock(&devfreq_list_lock);
> @@ -1380,7 +1427,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	struct devfreq *df = to_devfreq(dev);
>   	int ret;
>   	char str_governor[DEVFREQ_NAME_LEN + 1];
> -	const struct devfreq_governor *governor, *prev_governor;
> +	const struct devfreq_governor *governor;
>   
>   	ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
>   	if (ret != 1)
> @@ -1391,59 +1438,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	if (IS_ERR(governor))
>   		return PTR_ERR(governor);
>   
> -	if (!df->governor)
> -		goto start_new_governor;
> -
> -	if (df->governor == governor)
> -		return count;
> -
> -	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE) ||
> -	    IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE))
> -		return -EINVAL;
> -
> -	/*
> -	 * Stop the current governor and remove the specific sysfs files
> -	 * which depend on current governor.
> -	 */
> -	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
> -	if (ret) {
> -		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> -			 __func__, df->governor->name, ret);
> -		return ret;
> -	}
> -
> -start_new_governor:
> -	/*
> -	 * Start the new governor and create the specific sysfs files
> -	 * which depend on the new governor.
> -	 */
> -	prev_governor = df->governor;
> -	df->governor = governor;
> -	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> -	if (ret) {
> -		dev_warn(dev, "%s: Governor %s not started(%d)\n",
> -			 __func__, df->governor->name, ret);
> -
> -		/* Restore previous governor */
> -		df->governor = prev_governor;
> -		if (!df->governor)
> -			return ret;
> -
> -		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> -		if (ret) {
> -			dev_err(dev,
> -				"%s: reverting to Governor %s failed (%d)\n",
> -				__func__, prev_governor->name, ret);
> -			df->governor = NULL;
> -			return ret;
> -		}
> -	}
> -
> -	/*
> -	 * Create the sysfs files for the new governor. But if failed to start
> -	 * the new governor, restore the sysfs files of previous governor.
> -	 */
> -	ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
> +	ret = devfreq_set_governor(df, governor);
>   	if (ret)
>   		return ret;
>   


^ permalink raw reply

* Re: [PATCH v2 4/6] devfreq: Add module owner to devfreq governor
From: Yaxiong Tian @ 2026-05-14  6:14 UTC (permalink / raw)
  To: Jie Zhan, cwchoi00, cw00.choi, myungjoo.ham, kyungmin.park,
	linux-pm, linux-arm-kernel
  Cc: linuxarm, zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260513093832.1645890-5-zhanjie9@hisilicon.com>


在 2026/5/13 17:38, Jie Zhan 写道:
> Add an 'owner' member to struct devfreq_governor, such that we can find
> the module that holds the governor code when it's compiled as a kernel
> module.  This allows the devfreq core to properly manage the lifecycle
> of governors.
>
> Update devfreq_add_governor() and devm_devfreq_add_governor() to
> automatically set 'owner' to THIS_MODULE via helper macros.
>
> This is a prerequisite for implementing governor reference counting to
> prevent a governor module from being unloaded (except for force unload)
> while a governor is in use.
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>   drivers/devfreq/devfreq.c        | 26 +++++++++-----------------
>   include/linux/devfreq-governor.h | 26 +++++++++++++++++++++++---
>   2 files changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 9e3e6a7348f8..e1363ab69173 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1301,11 +1301,8 @@ void devfreq_resume(void)
>   	mutex_unlock(&devfreq_list_lock);
>   }
>   
> -/**
> - * devfreq_add_governor() - Add devfreq governor
> - * @governor:	the devfreq governor to be added
> - */
> -int devfreq_add_governor(struct devfreq_governor *governor)
> +int __devfreq_add_governor(struct devfreq_governor *governor,
> +			   struct module *mod)
>   {
>   	struct devfreq_governor *g;
>   
> @@ -1322,37 +1319,32 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>   		return -EINVAL;
>   	}
>   
> +	governor->owner = mod;
>   	list_add(&governor->node, &devfreq_governor_list);
>   
>   	return 0;
>   }
> -EXPORT_SYMBOL(devfreq_add_governor);
> +EXPORT_SYMBOL(__devfreq_add_governor);

It's a bit strange to use the __ symbol here. Generally speaking, 
functions exported with EXPORT_SYMBOL are public, while __ indicates 
internal.

The same applies below.

>   
>   static void devm_devfreq_remove_governor(void *governor)
>   {
>   	WARN_ON(devfreq_remove_governor(governor));
>   }
>   
> -/**
> - * devm_devfreq_add_governor() - Add devfreq governor
> - * @dev:	device which adds devfreq governor
> - * @governor:	the devfreq governor to be added
> - *
> - * This is a resource-managed variant of devfreq_add_governor().
> - */
> -int devm_devfreq_add_governor(struct device *dev,
> -			      struct devfreq_governor *governor)
> +int __devm_devfreq_add_governor(struct device *dev,
> +				struct devfreq_governor *governor,
> +				struct module *mod)
>   {
>   	int err;
>   
> -	err = devfreq_add_governor(governor);
> +	err = __devfreq_add_governor(governor, mod);
>   	if (err)
>   		return err;
>   
>   	return devm_add_action_or_reset(dev, devm_devfreq_remove_governor,
>   					governor);
>   }
> -EXPORT_SYMBOL(devm_devfreq_add_governor);
> +EXPORT_SYMBOL(__devm_devfreq_add_governor);
>   
>   /**
>    * devfreq_remove_governor() - Remove devfreq feature from a device.
> diff --git a/include/linux/devfreq-governor.h b/include/linux/devfreq-governor.h
> index dfdd0160a29f..1c4ff57e24de 100644
> --- a/include/linux/devfreq-governor.h
> +++ b/include/linux/devfreq-governor.h
> @@ -12,6 +12,7 @@
>   #define __LINUX_DEVFREQ_DEVFREQ_H__
>   
>   #include <linux/devfreq.h>
> +struct module;
>   
>   #define DEVFREQ_NAME_LEN			16
>   
> @@ -50,6 +51,7 @@
>   /**
>    * struct devfreq_governor - Devfreq policy governor
>    * @node:		list node - contains registered devfreq governors
> + * @owner:		Module that this governor belongs to
>    * @name:		Governor's name
>    * @attrs:		Governor's sysfs attribute flags
>    * @flags:		Governor's feature flags
> @@ -67,6 +69,7 @@
>   struct devfreq_governor {
>   	struct list_head node;
>   
> +	struct module *owner;
>   	const char name[DEVFREQ_NAME_LEN];
>   	const u64 attrs;
>   	const u64 flags;
> @@ -81,11 +84,28 @@ void devfreq_monitor_suspend(struct devfreq *devfreq);
>   void devfreq_monitor_resume(struct devfreq *devfreq);
>   void devfreq_update_interval(struct devfreq *devfreq, unsigned int *delay);
>   
> -int devfreq_add_governor(struct devfreq_governor *governor);
> +/**
> + * devfreq_add_governor() - Add devfreq governor
> + * @governor:	The devfreq governor to be added
> + */
> +#define devfreq_add_governor(governor) \
> +	__devfreq_add_governor((governor), THIS_MODULE)
> +int __devfreq_add_governor(struct devfreq_governor *governor,
> +			   struct module *mod);
>   int devfreq_remove_governor(struct devfreq_governor *governor);
>   
> -int devm_devfreq_add_governor(struct device *dev,
> -			      struct devfreq_governor *governor);
> +/**
> + * devm_devfreq_add_governor() - Add devfreq governor
> + * @dev:	device which adds devfreq governor
> + * @governor:	the devfreq governor to be added
> + *
> + * This is a resource-managed variant of devfreq_add_governor().
> + */
> +#define devm_devfreq_add_governor(dev, governor) \
> +	__devm_devfreq_add_governor((dev), (governor), THIS_MODULE)
> +int __devm_devfreq_add_governor(struct device *dev,
> +				struct devfreq_governor *governor,
> +				struct module *mod);
>   
>   int devfreq_update_status(struct devfreq *devfreq, unsigned long freq);
>   int devfreq_update_target(struct devfreq *devfreq, unsigned long freq);


^ permalink raw reply

* Re: [PATCH v4 04/13] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-05-14  6:24 UTC (permalink / raw)
  To: Jason Gunthorpe, 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, 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>

Jason Gunthorpe <jgg@ziepe.ca> writes:

>> 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.
>
> 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.
>

That is what this patch series aims to do. dma_direct_map_phys() will
derive the DMA address based on the attributes of the physical address:

if (attrs & DMA_ATTR_CC_SHARED)
        dma_addr = phys_to_dma_unencrypted(dev, phys);
else
        dma_addr = phys_to_dma_encrypted(dev, phys);

If that fails the dma_capable() check, we fall back to swiotlb_map():

if (unlikely(!dma_capable(dev, dma_addr, size, true, attrs)))
        return swiotlb_map(dev, phys, size, dir, attrs);

We currently do not have an encrypted SWIOTLB pool, but once that is
supported, swiotlb_map() should do the right thing and return the
correct encrypted dma_addr_t based on io_tlb_mem->unencrypted:

if (dev->dma_io_tlb_mem->unencrypted) {
        dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
        attrs |= DMA_ATTR_CC_SHARED;
} else {
        dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
}

-aneesh


^ permalink raw reply

* Re: [PATCH v6 08/10] arm64: dts: lx2160a: add labels to thermal trip-point nodes
From: Krzysztof Kozlowski @ 2026-05-14  6:41 UTC (permalink / raw)
  To: Josua Mayer
  Cc: Shawn Guo, Li Yang, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Frank Li,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Yazan Shhady, Jon Nettleton, linux-arm-kernel, devicetree,
	linux-kernel, imx
In-Reply-To: <20260512-lx2160-pci-v6-8-d0ff72d3c983@solid-run.com>

On Tue, May 12, 2026 at 04:39:03PM +0200, Josua Mayer wrote:
> LX2160A SoC dtsi defines rather conservative thermal trip points,
> alert at 85°C and critical at 95°C.
> 
> This is okay for most boards, however the SoC maximum junction
> temperature is 105°C in both commercial and industrial version.
> 
> Industrial grade boards need to change the thresholds to avoid premature
> thermal shutdown in high-temeprature environments.
> 
> Add labels to all thermal trip point nodes, enabling board dts to
> reference them and modify properties.

This is dead code or no-op. Labels should be referenced, otherwise you
are changing here nothing.

Squash the patches with the user of this label.

Best regards,
Krzysztof



^ permalink raw reply

* Re: [PATCH v6 05/10] dt-bindings: arm: fsl: Add solidrun lx2160a twins board
From: Krzysztof Kozlowski @ 2026-05-14  6:43 UTC (permalink / raw)
  To: Josua Mayer
  Cc: Shawn Guo, Li Yang, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Frank Li,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Yazan Shhady, Jon Nettleton, linux-arm-kernel, devicetree,
	linux-kernel, imx
In-Reply-To: <20260512-lx2160-pci-v6-5-d0ff72d3c983@solid-run.com>

On Tue, May 12, 2026 at 04:39:00PM +0200, Josua Mayer wrote:
> The SolidRun LX2160A Twins board supports two configurations, one with
> with a sinle CEX-7 module, and one with two (dual).
> 
> The dual configuration was not yet tested.

And how do see dual configuration? New compatible? For the same
hardware (the same because from SoC point of view it will be exactly
the same)?

You must post complete binding, otherwise this feels risky and when you
actually try running dual configuration you will see that existing
binding makes no sense.

Best regards,
Krzysztof



^ permalink raw reply

* [PATCH v2] arm64: dts: mediatek: mt7981b: Add PMU
From: Peter Collingbourne @ 2026-05-14  6:44 UTC (permalink / raw)
  To: Sjoerd Simons
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Peter Collingbourne

The interrupt number was taken from a downstream DTS of the similar MT7987
[1] and verified on my OpenWrt One.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
Link: [1] https://github.com/openwrt/openwrt/blob/e4b3d5c799aef3be20b7f6079e8e5a14b215c116/target/linux/mediatek/dts/mt7987.dtsi#L246
---
v2:
* Remove interrupt-parent

 arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
index 1bbe219380f9..ba6720031908 100644
--- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
@@ -38,6 +38,11 @@ oscillator-40m {
 		#clock-cells = <0>;
 	};
 
+	pmu {
+		compatible = "arm,cortex-a53-pmu";
+		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
+	};
+
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
-- 
2.54.0



^ permalink raw reply related

* Re: [PATCH] arm64: dts: mediatek: mt7981b: Add PMU
From: Peter Collingbourne @ 2026-05-14  6:45 UTC (permalink / raw)
  To: Sjoerd Simons
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek
In-Reply-To: <6dbefb2c80964c0394771ae11fd0f9e05486db29.camel@collabora.com>

On Mon, May 4, 2026 at 11:49 PM Sjoerd Simons <sjoerd@collabora.com> wrote:
>
> On Sat, 2026-05-02 at 00:49 -0700, Peter Collingbourne wrote:
> > The interrupt number was taken from a downstream DTS of the similar MT7987
> > [1] and verified on my OpenWrt One.
> >
> > Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
> > Link: [1]
> > https://github.com/openwrt/openwrt/blob/e4b3d5c799aef3be20b7f6079e8e5a14b215c116/target/linux/mediatek/dts/mt7987.dtsi#L246
> > ---
> >  arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> > index 4084f4dfa3e5..3c6fbb6c5333 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> > @@ -38,6 +38,12 @@ oscillator-40m {
> >               #clock-cells = <0>;
> >       };
> >
> > +     pmu {
> > +             compatible = "arm,cortex-a53-pmu";
> > +             interrupt-parent = <&gic>;
>
> The parent node already specifies the interrupt-parent, so this is redundant.
>
> > +             interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> > +     };
> > +
>
> Otherwise looks good

Thanks, sent v2 with removed interrupt-parent.

Peter


^ permalink raw reply

* Re: [PATCH 6/8] arm64: dts: qcom: kaanapali: Add GPU cooling
From: Gaurav Kohli @ 2026-05-14  6:47 UTC (permalink / raw)
  To: Dmitry Baryshkov, Akhil P Oommen
  Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sean Paul,
	linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm,
	freedreno, dri-devel
In-Reply-To: <iun4ziuei3tzvr75qbbqgxytto6vptvtd7j5mr5ol5aqviaafz@5m4yxgnqjavc>



On 5/13/2026 11:23 PM, Dmitry Baryshkov wrote:
> On Tue, May 12, 2026 at 03:53:20AM +0530, Akhil P Oommen wrote:
>> From: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
>>
>> Unlike the CPU, the GPU does not throttle its speed automatically when it
>> reaches high temperatures.
>>
>> Set up GPU cooling by throttling the GPU speed
>> when reaching 105°C.
>>
>> Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/kaanapali.dtsi | 165 ++++++++++++++++++++++++++------
>>   1 file changed, 135 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
>> index c57aea44218e..5089416ec32c 100644
>> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
>> @@ -26,6 +26,7 @@
>>   #include <dt-bindings/soc/qcom,gpr.h>
>>   #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>>   #include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
>> +#include <dt-bindings/thermal/thermal.h>
>>   
>>   #include "kaanapali-ipcc.h"
>>   
>> @@ -7045,13 +7046,15 @@ nsphmx-3-critical {
>>   		};
>>   
>>   		gpuss-0-thermal {
>> +			polling-delay-passive = <200>;
> 
> Other DT files use 10 for GPU thermal zones polling interval.
> 

Sure, let me update.

>> +
>>   			thermal-sensors = <&tsens5 0>;
>>   
>>   			trips {
>> -				gpuss-0-hot {
>> -					temperature = <120000>;
>> +				gpuss_0_alert0: gpuss-0-alert0 {
>> +					temperature = <105000>;
>>   					hysteresis = <5000>;
>> -					type = "hot";
>> +					type = "passive";
>>   				};
> 
> Why don't we keep both passive and hot trip points?
> 

Need guidance here, we are keeping passive at low temp so still hot trip 
is needed for such cases.

>>   
>>   				gpuss-0-critical {
>>
> 



^ permalink raw reply

* Re: [PATCH 0/5] scmi: Log client subsystem entity counts
From: Greg Kroah-Hartman @ 2026-05-14  6:48 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Andy Shevchenko, Alex Tran, Jyoti Bhayana, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Sudeep Holla,
	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: <1f2fb1de-ebc8-40ef-ac53-3348499295e3@roeck-us.net>

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.

So yes, I agree with Andy, this series is not ok, don't make more noise
please.

thanks,

greg k-h


^ permalink raw reply

* Re: [PATCH v2 5/6] devfreq: Get and put module refcount when switching governor
From: Yaxiong Tian @ 2026-05-14  6:50 UTC (permalink / raw)
  To: Jie Zhan, cwchoi00, cw00.choi, myungjoo.ham, kyungmin.park,
	linux-pm, linux-arm-kernel
  Cc: linuxarm, zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260513093832.1645890-6-zhanjie9@hisilicon.com>


在 2026/5/13 17:38, Jie Zhan 写道:
> A governor module can be dynamically inserted or removed if compiled as a
> kernel module.  'devfreq->governor' would become NULL if the governor
> module is removed when it's in use.
>
> To prevent the governor module from being removed (except for force
> unload) when it's in use, get and put a refcount of the governor module
> when starting and stopping the governor.
>
> As a result, unloading a governor module in use returns an error, e.g.:
>    $ cat governor
>    performance
>    $ rmmod governor_performance
>    rmmod: ERROR: Module governor_performance is in use
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>   drivers/devfreq/devfreq.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index e1363ab69173..2ea42325d030 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -345,24 +345,37 @@ static int devfreq_set_governor(struct devfreq *df,
>   				 __func__, df->governor->name, ret);
>   			return ret;
>   		}
> +		module_put(old_gov->owner);
>   	}
>   
>   	/* Start the new governor */
> +	if (!try_module_get(new_gov->owner)) {
> +		df->governor = NULL;
> +		return -EINVAL;
> +	}
> +

Here, new_gov has already been checked in try_then_request_governor() 
for whether the module can be obtained.

I think it might be more reasonable to merge try_then_request_governor() 
and devfreq_set_governor(), so that when new_gov cannot be obtained, the 
operation of the old governor is not affected.

>   	df->governor = new_gov;
>   	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>   	if (ret) {
>   		dev_warn(dev, "%s: Governor %s not started(%d)\n",
>   			 __func__, df->governor->name, ret);
> +		module_put(new_gov->owner);
>   
>   		/* Restore previous governor */
>   		df->governor = old_gov;
>   		if (!df->governor)
>   			return ret;
>   
> +		if (!try_module_get(old_gov->owner)) {
> +			df->governor = NULL;
> +			return -EINVAL;
> +		}
> +
>   		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>   		if (ret) {
>   			dev_err(dev, "%s: restore Governor %s failed (%d)\n",
>   				__func__, df->governor->name, ret);
> +			module_put(old_gov->owner);
>   			df->governor = NULL;
>   			return ret;
>   		}
> @@ -1040,9 +1053,11 @@ int devfreq_remove_device(struct devfreq *devfreq)
>   
>   	devfreq_cooling_unregister(devfreq->cdev);
>   
> -	if (devfreq->governor)
> +	if (devfreq->governor) {
>   		devfreq->governor->event_handler(devfreq,
>   						 DEVFREQ_GOV_STOP, NULL);
> +		module_put(devfreq->governor->owner);
> +	}
>   	device_unregister(&devfreq->dev);
>   
>   	return 0;


^ permalink raw reply

* Re: [PATCH v4 03/13] dma-pool: track decrypted atomic pools and select them via attrs
From: Aneesh Kumar K.V @ 2026-05-14  7:00 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: <agSED6BdSXBhbDYI@google.com>

Mostafa Saleh <smostafa@google.com> writes:

...

>>  struct page *dma_alloc_from_pool(struct device *dev, size_t size,
>> -		void **cpu_addr, gfp_t gfp,
>> +		void **cpu_addr, gfp_t gfp, unsigned long attrs,
>>  		bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t))
>>  {
>> -	struct gen_pool *pool = NULL;
>> +	struct dma_gen_pool *dma_pool = NULL;
>>  	struct page *page;
>>  	bool pool_found = false;
>>  
>> -	while ((pool = dma_guess_pool(pool, gfp))) {
>> +	while ((dma_pool = dma_guess_pool(dma_pool, gfp))) {
>> +
>> +		if (dma_pool->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED))
>> +			continue;
>> +
>
> nit: If we fail to find a matching pool, a slightly misleading message
> is printed as pool_found = false
>

The message printed is

	WARN(1, "Failed to get suitable pool for %s\n", dev_name(dev));

That is correct, isn’t it? The kernel failed to find a pool with the
correct encryption attribute. For example, the request was for an
encrypted allocation from the pool, but no encrypted pool was available.

>
>>  		pool_found = true;
>> -		page = __dma_alloc_from_pool(dev, size, pool, cpu_addr,
>> +		page = __dma_alloc_from_pool(dev, size, dma_pool->pool, cpu_addr,
>>  					     phys_addr_ok);
>>  		if (page)
>>  			return page;
>> @@ -296,12 +345,14 @@ struct page *dma_alloc_from_pool(struct device *dev, size_t size,

-aneesh


^ permalink raw reply

* [PATCH v1 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping
From: Damon Ding @ 2026-05-14  7:01 UTC (permalink / raw)
  To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
	neil.armstrong, rfoss
  Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
	cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
	luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
In-Reply-To: <20260514070133.2275069-1-damon.ding@rock-chips.com>

Parse the optional 'data-lanes' device tree property to support
custom physical lane mapping configuration.

If no valid configuration is found, fall back to the default
lane map (0, 1, 2, 3) automatically and keep the driver running.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 56 +++++++++++++++++++
 .../drm/bridge/analogix/analogix_dp_core.h    |  4 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 +++--
 .../gpu/drm/bridge/analogix/analogix_dp_reg.h |  4 ++
 4 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index c8eb3511f92a..a7d19c3be7e0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1234,6 +1234,59 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
 	.detect = analogix_dp_bridge_detect,
 };
 
+static int analogix_dp_dt_parse_lanes_map(struct analogix_dp_device *dp)
+{
+	struct video_info *video_info = &dp->video_info;
+	struct device_node *endpoint;
+	u32 tmp[LANE_COUNT4];
+	u32 map[LANE_COUNT4] = {0, 1, 2, 3};
+	bool used[LANE_COUNT4] = {false};
+	int num_lanes;
+	int ret, i;
+
+	memcpy(video_info->lane_map, map, sizeof(map));
+
+	num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
+						   video_info->max_lane_count);
+	if (num_lanes < 0)
+		return -EINVAL;
+
+	endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
+	if (!endpoint)
+		return -EINVAL;
+
+	ret = of_property_read_u32_array(endpoint, "data-lanes", tmp, num_lanes);
+	of_node_put(endpoint);
+	if (ret)
+		return -EINVAL;
+
+	for (i = 0; i < num_lanes; i++) {
+		if (tmp[i] >= LANE_COUNT4) {
+			dev_dbg(dp->dev, "data-lanes[%d] = %u is out of range\n", i, tmp[i]);
+			return -EINVAL;
+		}
+
+		if (used[tmp[i]]) {
+			dev_dbg(dp->dev, "data-lanes[%d] = %u is duplicate\n", i, tmp[i]);
+			return -EINVAL;
+		}
+
+		used[tmp[i]] = true;
+		map[i] = tmp[i];
+	}
+
+	for (i = 0; i < LANE_COUNT4 && num_lanes < LANE_COUNT4; i++) {
+		if (!used[i])
+			map[num_lanes++] = i;
+	}
+
+	dev_dbg(dp->dev, "Using parsed lane map: <%u %u %u %u>\n", map[0], map[1], map[2], map[3]);
+
+	memcpy(video_info->lane_map, map, sizeof(map));
+
+	return 0;
+}
+
 static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 {
 	struct device_node *dp_node = dp->dev->of_node;
@@ -1266,6 +1319,9 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 		break;
 	}
 
+	if (analogix_dp_dt_parse_lanes_map(dp))
+		dev_dbg(dp->dev, "No valid data-lanes found, using default lane map\n");
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 17347448c6b0..634fad241e69 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -137,6 +137,8 @@ struct video_info {
 
 	int max_link_rate;
 	enum link_lane_count_type max_lane_count;
+
+	u32 lane_map[LANE_COUNT4];
 };
 
 struct link_train {
@@ -175,7 +177,7 @@ struct analogix_dp_device {
 /* analogix_dp_reg.c */
 void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable);
 void analogix_dp_stop_video(struct analogix_dp_device *dp);
-void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable);
+void analogix_dp_lane_mapping(struct analogix_dp_device *dp);
 void analogix_dp_init_analog_param(struct analogix_dp_device *dp);
 void analogix_dp_init_interrupt(struct analogix_dp_device *dp);
 void analogix_dp_reset(struct analogix_dp_device *dp);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 6207ded7ffd5..f5f55bd25062 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -48,16 +48,15 @@ void analogix_dp_stop_video(struct analogix_dp_device *dp)
 	writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
 }
 
-void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable)
+void analogix_dp_lane_mapping(struct analogix_dp_device *dp)
 {
+	u32 *lane_map = dp->video_info.lane_map;
 	u32 reg;
 
-	if (enable)
-		reg = LANE3_MAP_LOGIC_LANE_0 | LANE2_MAP_LOGIC_LANE_1 |
-		      LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
-	else
-		reg = LANE3_MAP_LOGIC_LANE_3 | LANE2_MAP_LOGIC_LANE_2 |
-		      LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
+	reg = lane_map[0] << LANE0_MAP_SHIFT;
+	reg |= lane_map[1] << LANE1_MAP_SHIFT;
+	reg |= lane_map[2] << LANE2_MAP_SHIFT;
+	reg |= lane_map[3] << LANE3_MAP_SHIFT;
 
 	writel(reg, dp->reg_base + ANALOGIX_DP_LANE_MAP);
 }
@@ -140,7 +139,7 @@ void analogix_dp_reset(struct analogix_dp_device *dp)
 
 	usleep_range(20, 30);
 
-	analogix_dp_lane_swap(dp, 0);
+	analogix_dp_lane_mapping(dp);
 
 	writel(0x0, dp->reg_base + ANALOGIX_DP_SYS_CTL_1);
 	writel(0x40, dp->reg_base + ANALOGIX_DP_SYS_CTL_2);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
index 12735139046c..ac914e37089b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
@@ -209,6 +209,10 @@
 #define LANE0_MAP_LOGIC_LANE_1			(0x1 << 0)
 #define LANE0_MAP_LOGIC_LANE_2			(0x2 << 0)
 #define LANE0_MAP_LOGIC_LANE_3			(0x3 << 0)
+#define LANE3_MAP_SHIFT				(6)
+#define LANE2_MAP_SHIFT				(4)
+#define LANE1_MAP_SHIFT				(2)
+#define LANE0_MAP_SHIFT				(0)
 
 /* ANALOGIX_DP_ANALOG_CTL_1 */
 #define TX_TERMINAL_CTRL_50_OHM			(0x1 << 4)
-- 
2.34.1



^ permalink raw reply related

* [PATCH v1 2/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
From: Damon Ding @ 2026-05-14  7:01 UTC (permalink / raw)
  To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
	neil.armstrong, rfoss
  Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
	cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
	luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
In-Reply-To: <20260514070133.2275069-1-damon.ding@rock-chips.com>

Add data-lanes property support to the port@1 endpoint for physical
lane mapping configuration.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../bindings/display/bridge/analogix,dp.yaml  | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
index 62f0521b0924..a82f9b7776c0 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
@@ -36,19 +36,35 @@ properties:
       Hotplug detect GPIO.
       Indicates which GPIO should be used for hotplug detection
 
+  data-lanes:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    deprecated: true
+    minItems: 1
+    maxItems: 4
+    items:
+      maximum: 3
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
     properties:
       port@0:
         $ref: /schemas/graph.yaml#/properties/port
-        description:
-          Input node to receive pixel data.
+        description: Input node to receive pixel data.
 
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
-        description:
-          Port node with one endpoint connected to a dp-connector node.
+        description: Port node with one endpoint connected to sink device node.
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+                items:
+                  enum: [ 0, 1, 2, 3 ]
 
     required:
       - port@0
-- 
2.34.1



^ permalink raw reply related

* [PATCH v1 0/3] Add eDP lane mapping support
From: Damon Ding @ 2026-05-14  7:01 UTC (permalink / raw)
  To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
	neil.armstrong, rfoss
  Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
	cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
	luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding

This patch series is followed by:
https://lore.kernel.org/all/20260513074414.2053435-1-damon.ding@rock-chips.com/

This series adds configurable eDP/DP physical lane mapping support
via device tree data-lanes property.

Lane mapping is mainly used for below scenarios:
1. Correct PCB lane swap and differential line routing crossover
   without hardware changes;
2. Adapt mismatched lane pin definitions between SoC and eDP panel;
3. Support multiple panel hardware variants on the same board
   by configuring data-lanes in device tree only.

The series includes driver implementation and device tree binding
updates to support custom lane mapping configuration from endpoint
node, and keeps default linear lane order if no configuration is given.

Patch 1: Supplement inherited properties for rockchip,analogix-dp schema
Patch 2: Add endpoint data-lanes property to analogix-dp binding
Patch 3: Implement lane mapping in analogix_dp driver

Damon Ding (3):
  dt-bindings: display: rockchip: analogix-dp: Expose inherited
    properties
  dt-bindings: display: bridge: analogix-dp: Add data-lanes support for
    endpoint
  drm/bridge: analogix_dp: Add support for optional data-lanes mapping

 .../bindings/display/bridge/analogix,dp.yaml  | 24 ++++++--
 .../rockchip/rockchip,analogix-dp.yaml        |  7 +++
 .../drm/bridge/analogix/analogix_dp_core.c    | 56 +++++++++++++++++++
 .../drm/bridge/analogix/analogix_dp_core.h    |  4 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 +++--
 .../gpu/drm/bridge/analogix/analogix_dp_reg.h |  4 ++
 6 files changed, 97 insertions(+), 13 deletions(-)

-- 
2.34.1



^ permalink raw reply

* Herzlichen Glückwunsch
From: Euro Millions @ 2026-05-14  5:50 UTC (permalink / raw)
  To: linux-arm-kernel

Herzlichen Glückwunsch, Sie haben €650.000,00 bei den monatlichen 
Gewinnspielen von Euro Millions/Google Promo am 1.Mai 2026 
gewonnen.

Bitte geben Sie die folgenden Informationen ein, damit Ihr 
Gewinnbetrag an Sie überwiesen werden kann.

Vollständiger Name:
Heimatadresse:
Geschlecht:
Alter:
Telefon:

John Andrew
Online-Koordinator


^ permalink raw reply

* Re: [PATCH] arm64: dts: renesas: r8a78000: Fix GIC-720AE View 1 Redistributor description
From: Marc Zyngier @ 2026-05-14  7:04 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-arm-kernel, Conor Dooley, Geert Uytterhoeven,
	Krzysztof Kozlowski, Kuninori Morimoto, Magnus Damm, Rob Herring,
	devicetree, linux-renesas-soc
In-Reply-To: <20260513223125.43337-1-marek.vasut+renesas@mailbox.org>

On Wed, 13 May 2026 23:30:08 +0100,
Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
> 
> The Renesas R-Car X5H (R8A78000) SoC contains Arm CoreLink GIC-720AE
> Generic Interrupt Controller with Multi View capability. Firmware has
> access to configuration View 0, Linux kernel has access to View 1.

Huh. That's pretty unexpected. The usual wisdom is to give the APs
view 0 so that it looks like a "normal" machine, rather than only a
partition of the system (which is what view != 0 indicates).

I guess there is some additional fun going on there, such as other
CPUs getting a portion of the GIC for themselves, and firmware
preventing whatever is running on the APs to interact with them...

> 
> The Arm CoreLink GIC-720AE Generic Interrupt Controller Technical
> Reference Manual, currently latest r2p1 [1], chapter "Programmers model
> for GIC-720AE", subchapter "Redistributor registers for control and
> physical LPIs summary", part "GICR_TYPER, Redistributor Type Register"
> clarifies register "GICR_TYPER" bit 4 "Last" behavior in Multi View
> setup as follows:
> 
> "
> Last
> Last Redistributor:
> 
> 0 ... This Redistributor is not the last Redistributor on the chip.
> 1 ... This Redistributor is the last Redistributor on the chip.
>       When GICD_CFGID.VIEW == 1, for views 1, 2, or 3 this bit
>       always returns 1.
> "
> 
> On this SoC, GICD_CFGID.VIEW is 1 and the Linux kernel has access to
> View 1, therefore Linux kernel GICv3 driver will interpret register
> "GICR_TYPER" bit 4 "Last" = 1 in the first Redistributor in continuous
> Redistributor page as that first Redistributor being the one and only
> Redistributor and will stop processing the continuous Redistributor
> page further. This will prevent the other Redistributors from being
> recognized by the system and used for other PEs.
> 
> Because the hardware indicates that the continuous Redistributor page
> is not continuous for View 1, 2, or 3, describe every Redistributor
> separately in the DT. This makes all Redistributors for all cores
> accessible in Linux.
> 
> [1] https://developer.arm.com/documentation/102666/0201/Programmers-model-for-GIC-720AE/Redistributor-registers-for-control-and-physical-LPIs-summary/GICR-TYPER--Redistributor-Type-Register?lang=en
>

I am amazed that you managed to find anything at all on this web site.

I would refrain from adding links to any ARM web sites in a commit
message though. They tend to have a 10 minutes half-life period, and
whole specs to disappear from public view when they are not flavour of
the week anymore.

A link to the equivalent PDF has better chances to survive further
creative^WAI slop driven reorg of the documentation:

https://documentation-service.arm.com/static/69ef3c1cd35efd294e335c43

but your best bet is to download it, archive it, and refer to it
yourself.

> Fixes: 63500d12cf76 ("arm64: dts: renesas: Add R8A78000 SoC support")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply

* Re: [PATCH v2 03/11] m68k: mcf5441x: setup DAC clock name as per driver name
From: Angelo Dureghello @ 2026-05-14  7:05 UTC (permalink / raw)
  To: Greg Ungerer
  Cc: Geert Uytterhoeven, Steven King, Arnd Bergmann, Maxime Coquelin,
	Alexandre Torgue, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Greg Ungerer, linux-m68k, linux-kernel,
	linux-stm32, linux-arm-kernel, linux-iio
In-Reply-To: <734e2323-78dc-43c7-b5fc-0a35cc72e035@linux-m68k.org>

Hi Greg,

On 14.05.2026 11:27, Greg Ungerer wrote:
> 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?
> 

from a brief study, this 12bit DAC, and DAC module in general, is only on
this mcf45441x family. There are some ColdFire with ADC only, as those
mcf5249/53/82. 
The mcf51mm/ag/je are the opnly to have a 5bit dacs, but these are mcus.
So, if i don't miss any existing model, the name may be correct,
unless we want rename it to a more generic mcf_dac.

> Regards
> Greg
> 
> 

Regards,
angelo
> 
> 
> > 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

* [PATCH 0/2] add PIT DTS support for S32G2/S32G3 SoCs
From: Khristine Andreea Barbulescu @ 2026-05-14  7:06 UTC (permalink / raw)
  To: Chester Lin, Matthias Brugger, Ghennadi Procopciuc, Frank Li,
	Sascha Hauer, Fabio Estevam, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Pengutronix Kernel Team, linux-arm-kernel, imx, devicetree,
	linux-kernel, NXP S32 Linux, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo

This patchset aims to add one change to the S32G2/S32G3 dtsi support:
- Add PIT dts support for S32G SoC based boards

Khristine Andreea Barbulescu (2):
  arm64: dts: s32g: add PIT support for s32g2 and s32g3
  dt-bindings: timer: pit: add PIT node example for s32g2/3 platforms

 .../bindings/timer/fsl,vf610-pit.yaml         |  8 ++++++++
 arch/arm64/boot/dts/freescale/s32g2.dtsi      | 20 ++++++++++++++++++-
 arch/arm64/boot/dts/freescale/s32g3.dtsi      | 20 ++++++++++++++++++-
 3 files changed, 46 insertions(+), 2 deletions(-)

-- 
2.34.1



^ permalink raw reply


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