* Re: [PATCH v4 07/27] mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour
From: Pratyush Yadav @ 2026-05-05 15:40 UTC (permalink / raw)
To: Miquel Raynal
Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-7-833dab5e7288@bootlin.com>
Hi,
On Fri, Apr 03 2026, Miquel Raynal wrote:
> Add comments about how these requests are actually handled in the SPI
> NOR core. Their behaviour was not entirely clear to me at first, and
> explaining them in plain English sounds the way to go.
>
> Reviewed-by: Michael Walle <mwalle@kernel.org>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/spi-nor/swp.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
> index 1d50db1ef1a0..64a917543928 100644
> --- a/drivers/mtd/spi-nor/swp.c
> +++ b/drivers/mtd/spi-nor/swp.c
> @@ -346,6 +346,14 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
> return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf[0]);
> }
>
> +/*
> + * These ioctls behave according to the following rules:
> + * ->lock(): Never locks more than what is requested, ie. may lock less
> + * ->unlock(): Never unlocks more than what is requested, ie. may unlock less
> + * -is_locked(): Checks if the region is *fully* locked, returns false otherwise.
> + * This feeback may be misleading because users may get an "unlocked"
> + * status even though a subpart of the region is effectively locked.
> + */
We already have some text where struct spi_nor_locking_ops is defined.
Can we move this information there?
> static const struct spi_nor_locking_ops spi_nor_sr_locking_ops = {
> .lock = spi_nor_sr_lock,
> .unlock = spi_nor_sr_unlock,
--
Regards,
Pratyush Yadav
^ permalink raw reply
* Re: [PATCH 2/8] dma-heap: Provide accessors so that in-kernel drivers can allocate dmabufs from specific heaps
From: Boris Brezillon @ 2026-05-05 15:45 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Yong Wu, Yunfei Dong,
Florent Tomasin
In-Reply-To: <20260505140516.1372388-3-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:08 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> From: John Stultz <jstultz@google.com>
>
> This allows drivers who don't want to create their own
> DMA-BUF exporter to be able to allocate DMA-BUFs directly
> from existing DMA-BUF Heaps.
>
> There is some concern that the premise of DMA-BUF heaps is
> that userland knows better about what type of heap memory
> is needed for a pipeline, so it would likely be best for
> drivers to import and fill DMA-BUFs allocated by userland
> instead of allocating one themselves, but this is still
> up for debate.
I think this commit message needs to be updated with more details
around what it's actually needed for here (driver needing protected
buffer to boot FW and expose a char device, and no clean way to pass
dmabufs around before this cdev is exposed).
>
> Signed-off-by: John Stultz <jstultz@google.com>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> [Yong: Fix the checkpatch alignment warning]
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> [Florent: Rebase]
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> [Ketil: Rebase]
> ---
> drivers/dma-buf/dma-heap.c | 80 ++++++++++++++++++++++++++++++--------
> include/linux/dma-heap.h | 6 +++
> 2 files changed, 70 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 9fd365ddbd517..854d40d789ff2 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -57,12 +57,24 @@ module_param(mem_accounting, bool, 0444);
> MODULE_PARM_DESC(mem_accounting,
> "Enable cgroup-based memory accounting for dma-buf heap allocations (default=false).");
>
> -static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> - u32 fd_flags,
> - u64 heap_flags)
> +/**
> + * dma_heap_buffer_alloc - Allocate dma-buf from a dma_heap
> + * @heap: DMA-Heap to allocate from
> + * @len: size to allocate in bytes
> + * @fd_flags: flags to set on returned dma-buf fd
> + * @heap_flags: flags to pass to the dma heap
> + *
> + * This is for internal dma-buf allocations only. Free returned buffers with dma_buf_put().
> + */
> +struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> + u32 fd_flags,
> + u64 heap_flags)
> {
> - struct dma_buf *dmabuf;
> - int fd;
> + if (fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> + return ERR_PTR(-EINVAL);
> +
> + if (heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> + return ERR_PTR(-EINVAL);
I'd probably move the flags checks to dma_heap_buffer_alloc() in a
separate patch, to keep the diff easier to read. Same for the
dma_heap_buffer_alloc()/dma_heap_bufferfd_alloc() split, though I'm not
too sure we need dma_heap_bufferfd_alloc(), we could just move the FD
allocation directly in dma_heap_ioctl_allocate().
>
> /*
> * Allocations from all heaps have to begin
> @@ -70,9 +82,20 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> */
> len = PAGE_ALIGN(len);
> if (!len)
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
> +
> + return heap->ops->allocate(heap, len, fd_flags, heap_flags);
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_heap_buffer_alloc, "DMA_BUF_HEAP");
>
> - dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
> +static int dma_heap_bufferfd_alloc(struct dma_heap *heap, size_t len,
> + u32 fd_flags,
> + u64 heap_flags)
> +{
> + struct dma_buf *dmabuf;
> + int fd;
> +
> + dmabuf = dma_heap_buffer_alloc(heap, len, fd_flags, heap_flags);
> if (IS_ERR(dmabuf))
> return PTR_ERR(dmabuf);
>
> @@ -110,15 +133,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
> if (heap_allocation->fd)
> return -EINVAL;
>
> - if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> - return -EINVAL;
> -
> - if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> - return -EINVAL;
> -
> - fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
> - heap_allocation->fd_flags,
> - heap_allocation->heap_flags);
> + fd = dma_heap_bufferfd_alloc(heap, heap_allocation->len,
> + heap_allocation->fd_flags,
> + heap_allocation->heap_flags);
> if (fd < 0)
> return fd;
>
> @@ -317,6 +334,36 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> }
> EXPORT_SYMBOL_NS_GPL(dma_heap_add, "DMA_BUF_HEAP");
>
> +/**
> + * dma_heap_find - get the heap registered with the specified name
> + * @name: Name of the DMA-Heap to find
> + *
> + * Returns:
> + * The DMA-Heap with the provided name.
> + *
> + * NOTE: DMA-Heaps returned from this function MUST be released using
> + * dma_heap_put() when the user is done to enable the heap to be unloaded.
> + */
> +struct dma_heap *dma_heap_find(const char *name)
s/dma_heap_find/dma_heap_find_by_name/gc
> +{
> + struct dma_heap *h;
> +
> + mutex_lock(&heap_list_lock);
> + list_for_each_entry(h, &heap_list, list) {
> + if (!kref_get_unless_zero(&h->refcount))
> + continue;
> +
> + if (!strcmp(h->name, name)) {
I think we should go sysfs_streq(), to make sure we don't return a heap
whose name only starts with the searched name.
> + mutex_unlock(&heap_list_lock);
> + return h;
> + }
> + dma_heap_put(h);
> + }
> + mutex_unlock(&heap_list_lock);
> + return NULL;
This could be simplified with something like:
struct dma_heap *h, *ret = NULL;
guard(mutex)(&heap_list_lock);
list_for_each_entry(h, &heap_list, list) {
if (!sysfs_streq(h->name, name))
continue;
if (kref_get_unless_zero(&h->refcount))
ret = h;
break;
}
return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_heap_find, "DMA_BUF_HEAP");
> +
> static void dma_heap_release(struct kref *ref)
> {
> struct dma_heap *heap = container_of(ref, struct dma_heap, refcount);
> @@ -341,6 +388,7 @@ void dma_heap_put(struct dma_heap *heap)
> {
> kref_put(&heap->refcount, dma_heap_release);
> }
> +EXPORT_SYMBOL_NS_GPL(dma_heap_put, "DMA_BUF_HEAP");
>
> static char *dma_heap_devnode(const struct device *dev, umode_t *mode)
> {
> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> index ff57741700f5f..c3351f8a1f8cf 100644
> --- a/include/linux/dma-heap.h
> +++ b/include/linux/dma-heap.h
> @@ -46,8 +46,14 @@ const char *dma_heap_get_name(struct dma_heap *heap);
>
> struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
>
> +struct dma_heap *dma_heap_find(const char *name);
> +
> void dma_heap_put(struct dma_heap *heap);
>
> +struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> + u32 fd_flags,
> + u64 heap_flags);
> +
> extern bool mem_accounting;
>
> #endif /* _DMA_HEAPS_H */
^ permalink raw reply
* Re: [PATCH 3/8] drm/panthor: De-duplicate FW memory section sync
From: Boris Brezillon @ 2026-05-05 15:47 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek
In-Reply-To: <20260505140516.1372388-4-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:09 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> Handle the sync to device of FW memory sections inside
> panthor_fw_init_section_mem() so that the callers do not have to.
>
> This small improvement is also critical for protected FW sections,
> so we avoid issuing memory transactions to protected memory from
> CPU running in normal mode.
>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panthor/panthor_fw.c | 22 ++++++----------------
> 1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index be0da5b1f3abf..0d07a133dc3af 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -446,6 +446,7 @@ static void panthor_fw_init_section_mem(struct panthor_device *ptdev,
> struct panthor_fw_section *section)
> {
> bool was_mapped = !!section->mem->kmap;
> + struct sg_table *sgt;
> int ret;
>
> if (!section->data.size &&
> @@ -464,6 +465,11 @@ static void panthor_fw_init_section_mem(struct panthor_device *ptdev,
>
> if (!was_mapped)
> panthor_kernel_bo_vunmap(section->mem);
> +
> + /* An sgt should have been requested when the kernel BO was GPU-mapped. */
> + sgt = to_panthor_bo(section->mem->obj)->dmap.sgt;
> + if (!drm_WARN_ON_ONCE(&ptdev->base, !sgt))
> + dma_sync_sgtable_for_device(ptdev->base.dev, sgt, DMA_TO_DEVICE);
> }
>
> /**
> @@ -626,7 +632,6 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> section_size = hdr.va.end - hdr.va.start;
> if (section_size) {
> u32 cache_mode = hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_MASK;
> - struct panthor_gem_object *bo;
> u32 vm_map_flags = 0;
> u64 va = hdr.va.start;
>
> @@ -663,14 +668,6 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> }
>
> panthor_fw_init_section_mem(ptdev, section);
> -
> - bo = to_panthor_bo(section->mem->obj);
> -
> - /* An sgt should have been requested when the kernel BO was GPU-mapped. */
> - if (drm_WARN_ON_ONCE(&ptdev->base, !bo->dmap.sgt))
> - return -EINVAL;
> -
> - dma_sync_sgtable_for_device(ptdev->base.dev, bo->dmap.sgt, DMA_TO_DEVICE);
> }
>
> if (hdr.va.start == CSF_MCU_SHARED_REGION_START)
> @@ -724,17 +721,10 @@ panthor_reload_fw_sections(struct panthor_device *ptdev, bool full_reload)
> struct panthor_fw_section *section;
>
> list_for_each_entry(section, &ptdev->fw->sections, node) {
> - struct sg_table *sgt;
> -
> if (!full_reload && !(section->flags & CSF_FW_BINARY_IFACE_ENTRY_WR))
> continue;
>
> panthor_fw_init_section_mem(ptdev, section);
> -
> - /* An sgt should have been requested when the kernel BO was GPU-mapped. */
> - sgt = to_panthor_bo(section->mem->obj)->dmap.sgt;
> - if (!drm_WARN_ON_ONCE(&ptdev->base, !sgt))
> - dma_sync_sgtable_for_device(ptdev->base.dev, sgt, DMA_TO_DEVICE);
> }
> }
>
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Shah, Tanmay @ 2026-05-05 15:52 UTC (permalink / raw)
To: Arnaud POULIQUEN, tanmay.shah, Beleswar Prasad Padhi,
Mathieu Poirier
Cc: Shenwei Wang, Andrew Lunn, Linus Walleij, Bartosz Golaszewski,
Jonathan Corbet, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Frank Li, Sascha Hauer, Shuah Khan,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, devicetree@vger.kernel.org,
linux-remoteproc@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, dl-linux-imx,
Bartosz Golaszewski
In-Reply-To: <87850f70-5275-4393-941f-d01146a9cffc@foss.st.com>
On 5/5/2026 4:28 AM, Arnaud POULIQUEN wrote:
> Hi Tanmay,
>
> On 5/4/26 21:19, Shah, Tanmay wrote:
>>
>> Hello all,
>>
>> I have started reviewing this work as well.
>> Thanks Shenwei for this work.
>>
>> I have gone through only the current revision, and would like to provide
>> idea on how to achieve GPIO number multiplexing with the RPMsg protocol.
>> Also, have some bindings related question.
>>
>> Please see below:
>>
>> On 4/30/2026 11:40 AM, Arnaud POULIQUEN wrote:
>>>
>>>
>>> On 4/30/26 14:56, Beleswar Prasad Padhi wrote:
>>>> Hello Arnaud,
>>>>
>>>> On 30/04/26 13:05, Arnaud POULIQUEN wrote:
>>>>> Hello,
>>>>>
>>>>> On 4/29/26 21:20, Mathieu Poirier wrote:
>>>>>> On Wed, 29 Apr 2026 at 12:07, Padhi, Beleswar <b-padhi@ti.com> wrote:
>>>>>>>
>>>>>>> Hi Mathieu,
>>>>>>>
>>>>>>> On 4/29/2026 11:03 PM, Mathieu Poirier wrote:
>>>>>>>> On Wed, 29 Apr 2026 at 10:53, Shenwei Wang <shenwei.wang@nxp.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>>>>>>>> Sent: Wednesday, April 29, 2026 10:42 AM
>>>>>>>>>> To: Shenwei Wang <shenwei.wang@nxp.com>
>>>>>>>>>> Cc: Andrew Lunn <andrew@lunn.ch>; Padhi, Beleswar <b-
>>>>>>>>>> padhi@ti.com>; Linus
>>>>>>>>>> Walleij <linusw@kernel.org>; Bartosz Golaszewski
>>>>>>>>>> <brgl@kernel.org>; Jonathan
>>>>>>>>>> Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>;
>>>>>>>>>> Krzysztof Kozlowski
>>>>>>>>>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn
>>>>>>>>>> Andersson
>>>>>>>>>> <andersson@kernel.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer
>>>>>>>>>> <s.hauer@pengutronix.de>; Shuah Khan
>>>>>>>>>> <skhan@linuxfoundation.org>; linux-
>>>>>>>>>> gpio@vger.kernel.org; linux-doc@vger.kernel.org; linux-
>>>>>>>>>> kernel@vger.kernel.org;
>>>>>>>>>> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>>>>>>>>>> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
>>>>>>>>>> devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org;
>>>>>>>>>> imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; dl-
>>>>>>>>>> linux-imx <linux-
>>>>>>>>>> imx@nxp.com>; Bartosz Golaszewski <brgl@bgdev.pl>
>>>>>>>>>> Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic
>>>>>>>>>> rpmsg GPIO driver
>>>>>>>>>> On Tue, Apr 28, 2026 at 03:24:59PM +0000, Shenwei Wang wrote:
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: Andrew Lunn <andrew@lunn.ch>
>>>>>>>>>>>> Sent: Monday, April 27, 2026 3:49 PM
>>>>>>>>>>>> To: Shenwei Wang <shenwei.wang@nxp.com>
>>>>>>>>>>>> Cc: Padhi, Beleswar <b-padhi@ti.com>; Linus Walleij
>>>>>>>>>>>> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
>>>>>>>>>>>> Jonathan
>>>>>>>>>>>> Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>;
>>>>>>>>>>>> Krzysztof
>>>>>>>>>>>> Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>>>>>>>>>>>> <conor+dt@kernel.org>;
>>>>>>>>>>>> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
>>>>>>>>>>>> <mathieu.poirier@linaro.org>; Frank Li <frank.li@nxp.com>;
>>>>>>>>>>>> Sascha
>>>>>>>>>>>> Hauer <s.hauer@pengutronix.de>; Shuah Khan
>>>>>>>>>>>> <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
>>>>>>>>>>>> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix
>>>>>>>>>>>> Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>>>>>>>>>>>> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
>>>>>>>>>>>> devicetree@vger.kernel.org; linux- remoteproc@vger.kernel.org;
>>>>>>>>>>>> imx@lists.linux.dev; linux-arm- kernel@lists.infradead.org;
>>>>>>>>>>>> dl-linux-imx <linux-imx@nxp.com>; Bartosz Golaszewski
>>>>>>>>>>>> <brgl@bgdev.pl>
>>>>>>>>>>>> Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic
>>>>>>>>>>>> rpmsg
>>>>>>>>>>>> GPIO driver
>>>>>>>>>>>>>> struct virtio_gpio_response {
>>>>>>>>>>>>>> __u8 status;
>>>>>>>>>>>>>> __u8 value;
>>>>>>>>>>>>>> };
>>>>>>>>>>>>> It is the same message format. Please see the message
>>>>>>>>>>>>> definition
>>>>>>>>>>>> (GET_DIRECTION) below:
>>>>>>>>>>>>
>>>>>>>>>>>>> + +-----+-----+-----+-----+-----+----+
>>>>>>>>>>>>> + |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
>>>>>>>>>>>>> + | 1 | 2 |port |line | err | dir|
>>>>>>>>>>>>> + +-----+-----+-----+-----+-----+----+
>>>>>>>>>>>> Sorry, but i don't see how two u8 vs six u8 are the same
>>>>>>>>>>>> message format.
>>>>>>>>>>>>
>>>>>>>>>>> Some changes to the message format are necessary.
>>>>>>>>>>>
>>>>>>>>>>> Virtio uses two communication channels (virtqueues): one for
>>>>>>>>>>> requests and
>>>>>>>>>> replies, and a second one for events.
>>>>>>>>>>> In contrast, rpmsg provides only a single communication
>>>>>>>>>>> channel, so a
>>>>>>>>>>> type field is required to distinguish between different kinds
>>>>>>>>>>> of messages.
>>>>>>>>>>>
>>>>>>>>>>> Since rpmsg replies and events share the same message format,
>>>>>>>>>>> an additional
>>>>>>>>>> line is introduced to handle both cases.
>>>>>>>>>>> Finally, rpmsg supports multiple GPIO controllers, so a port
>>>>>>>>>>> field is added to
>>>>>>>>>> uniquely identify the target controller.
>>>>>>>>>>
>>>>>>>>>> I have commented on this before - RPMSG is already providing
>>>>>>>>>> multiplexing
>>>>>>>>>> capability by way of endpoints. There is no need for a port
>>>>>>>>>> field. One endpoint,
>>>>>>>>>> one GPIO controller.
>>>>>>>>>>
>>>>>>>>> You still need a way to let the remote side know which port the
>>>>>>>>> endpoint maps to, either
>>>>>>>>> by embedding the port information in the message (the current
>>>>>>>>> way), or by sending it
>>>>>>>>> separately.
>>>>>>>>>
>>>>>>>> An endpoint is created with every namespace request. There
>>>>>>>> should be
>>>>>>>> one namespace request for every GPIO controller, which yields a
>>>>>>>> unique
>>>>>>>> endpoint for each controller and eliminates the need for an extra
>>>>>>>> field to identify them.
>>>>>>>
>>>>>>>
>>>>>>> Right, but this can still be done by just having one namespace
>>>>>>> request.
>>>>>>> We can create new endpoints bound to an existing namespace/
>>>>>>> channel by
>>>>>>> invoking rpmsg_create_ept(). This is what I suggested here too:
>>>>>>> https://lore.kernel.org/all/29485742-6e49-482e-
>>>>>>> b73d-228295daaeec@ti.com/
>>>>>>>
>>>>>>
>>>>>> I will look at your suggestion (i.e link above) later this week or
>>>>>> next week.
>>>>>>
>>>>>>> My mental model looks like this for the complete picture:
>>>>>>>
>>>>>>> 1. namespace/channel#1 = rpmsg-io
>>>>>>> a. ept1 -> gpio-controller@1
>>>>>>> b. ept2 -> gpio-controller@2
>>>>>>>
>>
>> If my understanding of what gpio-controller is right, than this won't
>> work. We need one rpmsg channel per gpio-controller, and in most cases
>> there will be only one GPIO-controller on the remote side. If there are
>> multiple or multiple instances of same controller, than we need separate
>> channel name for that controller just like we would have separate device
>> on the Linux.
>
> As done in ehe rpmsg_tty driver it could be instantiated several times with
> the same channel/service name. This would imply a specific rpmsg to
> retreive
> the gpio controller index from the remote side.
>>
>>>>>>
>>>>>> I've asked for one endpoint per GPIO controller since the very
>>>>>> beginning. I don't yet have a strong opinion on whether to use one
>>>>>> namespace request per GPIO controller or a single request that spins
>>>>>> off multiple endpoints. I'll have to look at your link and
>>>>>> reflect on
>>>>>> that. Regardless of how we proceed on that front, multiplexing needs
>>>>>> to happen at the endpoint level rather than the packet level.
>>>>>> This is
>>>>>> the only way this work can move forward.
>>>>>>
>>>>>
>>>>> I would be more in favor of Mathieu’s proposal: “An endpoint is
>>>>> created with every namespace request.”
>>>>>
>>>>> If the endpoint is created only on the Linux side, how do we match
>>>>> the Linux endpoint address with the local port field on the remote
>>>>> side?
>>>>
>>>>
>>>> Simply by sending a message to the remote containing the newly created
>>>> endpoint and the port idx. Note that is this done just one time, after
>>>> this
>>>> Linux need not have the port field in the message everytime its sending
>>>> a message.
>>>>
>>>>>
>>>>> With a multi-namespace approach, the namespace could be rpmsg-io-
>>>>> [addr], where [addr] corresponds to the GPIO controller address in
>>>>> the DT. This would:
>>>>
>>>>
>>>> You will face the same problem in this case also that you asked above:
>>>> "how do we match the Linux endpoint address with the local port field
>>>> on the remote side?"
>>>
>>> Sorry I probably introduced confusion here
>>> my sentence should be;
>>> With a multi-namespace approach, the namespace could be rpmsg-io-
>>> [port],
>>> where [port] corresponds to the GPIO controller port in the DT.
>>>
>>>
>>> For instance:
>>>
>>> rpmsg {
>>> rpmsg-io {
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> gpio@25 {
>>> compatible = "rpmsg-gpio";
>>> reg = <25>;
>>> gpio-controller;
>>> #gpio-cells = <2>;
>>> #interrupt-cells = <2>;
>>> interrupt-controller;
>>> };
>>>
>>> gpio@32 {
>>> compatible = "rpmsg-gpio";
>>> reg = <32>;
>>> gpio-controller;
>>> #gpio-cells = <2>;
>>> #interrupt-cells = <2>;
>>> interrupt-controller;
>>> };
>>> };
>>> };
>>>
>>> rpmsg-io-25 would match with gpio@25
>>> rpmsg-io-32 would match with gpio@32
>>>
>>
>> The problem with this approach is, we will endup creating way too many
>> RPMsg devices/channels. i.e. one channel per one GPIO. That limits how
>> many GPIOs can be handled by remote from memory perspective. At
>> somepoint we might just run-out of number ept & channels created by the
>> remote. As of now, open-amp library supports 128 epts I think.
>
> Right, I proposed a solution in my previous answer to Beleswar who has
> the same concern.
>
>>
>>>
>>>>
>>>> Because the endpoint that is created on a namespace request is also
>>>> dynamic in nature. How will the remote know which endpoint addr
>>>> Linux allocated for a namespace that it announced?
>>>>
>>>> As an example/PoC, I created a firmware example which announces
>>>> 2 name services to Linux, one is the standard "rpmsg_chrdev" and
>>>> the other is a TI specific name service "ti.ipc4.ping-pong". You can
>>>> see it created 2 different addresses (0x400 and 0x401) for each of
>>>> the name service request from the same firmware:
>>>>
>>>> root@j784s4-evm:~# dmesg | grep virtio0 | grep -i channel
>>>> [ 9.290275] virtio_rpmsg_bus virtio0: creating channel
>>>> ti.ipc4.ping-pong addr 0xd
>>>> [ 9.311230] virtio_rpmsg_bus virtio0: creating channel rpmsg_chrdev
>>>> addr 0xe
>>>> [ 9.496645] rpmsg_chrdev virtio0.rpmsg_chrdev.-1.14: DEBUG: Channel
>>>> formed from src = 0x400 to dst = 0xe
>>>> [ 9.707255] rpmsg_client_sample virtio0.ti.ipc4.ping-pong.-1.13:
>>>> new channel: 0x401 -> 0xd!
>>>>
>>>> So in this case, rpmsg-io-1 can have different ept addr than rpmsg-io-2
>>>> Back to same problem. Simple solution is to reply to remote with the
>>>> created ept addr and the index.
>>>
>>> That why I would like to suggest to use the name service field to
>>> identify the port/controller, instead of the endpoint address.
>>>>
>>>>>
>>>>> - match the RPMsg probe with the DT,
>>>>
>>>>
>>>> We can probe from all controllers with a single name service
>>>> announcement too.
>>>>
>>>>> - provide a simple mapping between the port and the endpoint on both
>>>>> sides,
>>>>
>>>>
>>>> We are trying to get rid of this mapping from Linux side to adapt
>>>> the gpio-virtio design.
>>>>
>>>>> - allow multiple endpoints on the remote side,
>>>>
>>>>
>>>> We can support this as well with single nameservice model.
>>>> There is no limitation. Remote has to send a message with
>>>> its newly created ept that's all.
>>>>
>>>>> - provide a simple discovery mechanism for remote capabilities.
>>>>
>>>>
>>>> A single announcement: "rpmsg-io" is also discovery mechanism.
>>>>
>>>> Feel free to let me know if you have concerns with any of the
>>>> suggestions!
>>>
>>> My only concern, whatever the solution, is that we find a smart
>>> solution to associate the correct endpoint with the correct GPIO
>>> port/controller defined in the DT.
>>>
>>> I may have misunderstood your solution. Could you please help me
>>> understand your proposal by explaining how you would handle three
>>> GPIO ports defined in the DT, considering that the endpoint
>>> addresses on the Linux side can be random?
>>> If I assume there is a unique endpoint on the remote side,
>>> I do not understand how you can match, on the firmware side,
>>> the Linux endpoint address to the GPIO port.
>>>
>>> Thanks and Regards,Arnaud
>>>
>>>>
>>>> Thanks,
>>>> Beleswar
>>>>
>>>>>
>>>>> Regards,
>>>>> Arnaud
>>>>>
>>>>>>> 2. namespace/channel#2 = rpmsg-i2c
>>>>>>> a. ept1 -> i2c@1
>>>>>>> b. ept2 -> i2c@2
>>>>>>> c. ept3 -> i2c@3
>>>>>>>
>>>>>>> etc...
>>>>>>>
>>
>> Just want to clear-up few terms before I jump to the solution:
>>
>> **RPMsg channel/device**:
>> - These are devices announced by the remote processor, and created by
>> linux. They are created at: /sys/bus/rpmsg/devices
>> - The channel format: <name>.<src ept>.<dst ept>
>>
>> **RPMsg endpoint**:
>> - Endpoint is differnt than channel. Single channel can have multiple
>> endpoints, and represented in the linux with: /dev/rpmsg? devices.
>>
>> To create endpoint device, we have rpmsg_create_ept API, which takes
>> channel information as input, which has src-ept, dst-ept.
>>
>> Following is proposed solution:
>>
>> 1) Assign RPMsg channel/device per rpmsg-gpio controller (Not per GPIO
>> pin/port).
>> - In our case that would be, single rpmsg-io node. (That makes me
>> question if bindings are correct or not).
>>
>> 2) Assign GPIO number as src ept.
>>
>> i.e. *rpmsg-io.<GPIO number>.<dst ept>*. Do not randomly assign src
>> endpoint.
>>
>> Now, RPMSG channel by spec reserves first 1024 endpoints [1], so we can
>> add 1024 offset to the GPIO number:
>>
>> so, when calling rpmsg_create_ept() API, we assing src_endpoint as:
>> (GPIO_NUMBER + RPMSG_RESERVED_ADDRESSES)
>>
>> Now on the remote side, there is single channel and only single-endpoint
>> is needed that is mapped to the rpmsg-io channel callback.
>>
>> That callback will receive all the payloads from the Linux, which will
>> have src-ept i.e. (RPMSG_RESERVED_ADDRESSES + GPIO_NUMBER).
>
>
> Interesting approach. I also tried to find a similar solution.
>
> The question here is: how can we guarantee continuous addresses? Given
> the static and dynamic allocation of endpoint addresses that are
> implemented, my conclusion was that it is not reliable enough.
>
> but perhaps I missed something...
>
>>
>> It can retrieve GPIO_NUMBER easily, and convert to appropriate pin based
>> on platform specific logic.
>>
>> This doesn't need PORT information at all. Also it makes sure that
>> remote is using only single-endpoint so not much memory is used.
>>
>> *Example*:
>> If only rpmsg-gpio channel is created by the remote side, than following
>> is the representation of the devices when GPIO 25, 26, 27 is assigned to
>> the rpmsg-io controller:
>>
>> Linux Remote
>>
>> rpmsg-channel: rpmsg-gpio.0x400.0x400
>>
>> /dev/rpmsg0 - GPIO25 ept (rpmsg-gpio.0x419.0x400)-|
>> |
>> /dev/rpmsg1 - GPIO26 ept (rpmsg-gpio.0x41a.0x400)-|-> rpmsg-gpio.*.0x400
>> |
>> /dev/rpmsg2 - GPIO27 ept (rpmsg-gpio.0x41b.0x400)-| 0x400 ept callback.
>>
>>
>> *On remote side*:
>>
>> ept_0x400_callback(..., int src_ept, ...,)
>> {
>> int gpio_num = src_ept - RPMSG_RESERVED_ADDRESSES;
>> // platform specific logic to convert gpio num to proper pin,
>> // just like you would convert gpio num to pin on a linux gpio
>> controller.
>> }
>>
>> My question on the binding:
>>
>> Why each GPIO is represented with the separate node? I think rpmsg-gpio
>> can be represented just any other GPIO controller? Please let me know if
>> I am missing something. So rpmsg channel/rpmsg device is not created per
>> GPIO, but per controller. GPIO number multiplexing should be done with
>> rpmsg src ept, that removes the need of having each GPIO as a separate
>> node.
>>
>>
>> rpmsg_gpio: rpmsg-gpio@0 {
>> compatible = "rpmsg-gpio";
>> reg = <0>;
>> gpio-controller;
>> #gpio-cells = <2>;
>> #interrupt-cells = <2>;
>> interrupt-controller;
>> };
>>
>> Then in DT, use like regular GPIO, but with the rpmsg-gpio controller:
>>
>> rpmsg-gpios = <&rpmsg_gpio (GPIO NUM) (flags)>;
>>
>> If the intent to create separate gpio nodes was only for the channel
>> creation, then it's not really needed.
>>
>> [1]
>> https://github.com/torvalds/linux/
>> blob/6d35786de28116ecf78797a62b84e6bf3c45aa5a/drivers/rpmsg/
>> virtio_rpmsg_bus.c#L136
>>
>
> It is already the case. bindings declare GPIO controllers, not directly
> GPIOs in:
>
> [PATCH v13 2/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode
> support
>
> The discussion is around having an unique RPmsg endpoint for all
> GPIO controller or one RPmsg endpoint per GPIO controller.
>
Endpoint where remote side or linux side?
If unique endpoint on remote side per gpio controller then it makes sense.
Unique endpoint on linux side doesn't make sense. Instead, unique
channel per gpio controller makes sense, and each channel will have
multiple endpoints on linux side. As I replied to Beleswar on the other
email, I will copy past my answer here too:
To be more specific:
Linux: remote:
ch1: rpmsg-gpio.-1.1024 -> gpio-controller@1024
- gpio-line ept1
- gpio-line ept2 -> They all map to same callback_ept_1024.
- gpio-line ept3
ch2: rpmsg-gpio.-1.1025 -> gpio-controller@1025
- gpio-line ept1
- gpio-line ept2 -> They all map to same callback_ept_1025.
- gpio-line ept3
On the remote side, we have to hardcode Which rpmsg controller is mapped
to which endpoint.
> Or did I misunderstand your questions?
>
> Thanks,
> Arnaud
>
I gave this patch more time yesterday, and I think the 'reg' property
should represent remote endpoint, instead of the gpio-controller index.
So in this approach remote implementation is expected to provide
hard-coded (static) endpoints for each gpio-controller instance, and
that same number should be represented with the 'reg' property.
On remote side:
#define RPMSG_GPIO_0_CONTROLLER_EPT (RPMSG_RESERVED_ADDRESSES + 1) // 1024
ept_1024_callback() {
// handle appropriate gpio port ()
}
On linux side:
So new representation of controller:
rpmsg_gpio_0: gpio@1024 {
compatible = "rpmsg-gpio";
reg = <1024>;
gpio-controller;
#gpio-cells = <2>;
#interrupt-cells = <2>;
interrupt-controller;
};
rpmsg_gpio_1: gpio@1025 {
compatible = "rpmsg-gpio";
reg = <1025>;
gpio-controller;
#gpio-cells = <2>;
#interrupt-cells = <2>;
interrupt-controller;
};
gpios = <&rpmsg_gpio_0 (GPIO NUM or PIN) flags>,
<&rpmsg_gpio_1 (GPIO NUM or PIN) flags>;
Now in the linux driver:
You can easily retrieve destination endpoint when we want to send the
command to the gpio controller via device's "reg" property.
This approach also provides built-in security as well. Because now
gpio-controller instance is hardcoded with the endpoint callback, it
can't be modified/addressed without changing the 'reg' property.
Just like you wouldn't change device address for the instance of the
gpio-controller right?
This approach can be easily adapted to all the other rpmsg controllers
as well.
So, dynamic endpoint allocation doesn't make sense in this case. Dynamic
endpoint allocation makes more sense for user-space apps which don't
really care about endpoints and only payloads.
But, here we are multiplexing device-addresses with endpoints, and so it
has to be fixed, and presented via 'reg' property. So, firmware can't
change device-address without Linux knowing it.
Thanks,
Tanmay
>
>>>>>>> This way device groups are isolated with each channel/namespace, and
>>>>>>> instances within each device groups are also respected with specific
>>>>>>> endpoints.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Beleswar
>>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>
>
^ permalink raw reply
* Re: (subset) [PATCH v25 0/7] firmware: imx: driver for NXP secure-enclave
From: Frank Li @ 2026-05-05 15:53 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Pankaj Gupta
Cc: linux-doc, linux-kernel, devicetree, imx, linux-arm-kernel
In-Reply-To: <20260122-imx-se-if-v25-0-5c3e3e3b69a8@nxp.com>
On Thu, 22 Jan 2026 17:19:12 +0530, Pankaj Gupta wrote:
> The NXP's i.MX EdgeLock Enclave, a HW IP creating an embedded secure
> enclave within the SoC boundary to enable features like
> - HSM
> - SHE
> - V2X
>
> Communicates via message unit with linux kernel. This driver is
> enables communication ensuring well defined message sequence protocol
> between Application Core and enclave's firmware.
>
> [...]
Applied, thanks!
[1/7] Documentation/firmware: add imx/se to other_interfaces
commit: 3b4531c6e0f4c8874f0266853a410438eda1fc24
[2/7] dt-bindings: arm: fsl: add imx-se-fw binding doc
commit: 4d7bcf0869686d7d7fbf16244453b987e5ca6d14
[3/7] firmware: imx: add driver for NXP EdgeLock Enclave
commit: 338529a73c2bf2c277013b745cfe6f19b84b70af
[4/7] firmware: imx: device context dedicated to priv
commit: 2d733ed67f608ee85abb854157011f88d7f280a8
[5/7] firmware: drivers: imx: adds miscdev
commit: 4de71839142b5f43846e3593f4eb236e1d733885
Best regards,
--
Frank Li <Frank.Li@nxp.com>
^ permalink raw reply
* Re: [PATCH v4 10/27] mtd: spi-nor: swp: Create a helper that writes SR, CR and checks
From: Pratyush Yadav @ 2026-05-05 16:05 UTC (permalink / raw)
To: Miquel Raynal
Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-10-833dab5e7288@bootlin.com>
On Fri, Apr 03 2026, Miquel Raynal wrote:
> There are many helpers already to either read and/or write SR and/or CR,
> as well as sometimes check the returned values. In order to be able to
> switch from a 1 byte status register to a 2 bytes status register while
> keeping the same level of verification, let's introduce a new helper
> that writes them both (atomically) and then reads them back (separated)
> to compare the values.
>
> In case 2 bytes registers are not supported, we still have the usual
> fallback available in the helper being exported to the rest of the core.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
I'm confused. Doesn't spi_nor_write_16bit_sr_and_check() do the same
thing? How are these two different?
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply
* Re: [PATCH 01/14] kbuild: Bump minimum version of LLVM for building the kernel to 17.0.1
From: Nicolas Schier @ 2026-05-05 15:27 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Bill Wendling, Justin Stitt, Nick Desaulniers, linux-kernel, llvm,
linux-kbuild, Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <20260428-bump-minimum-supported-llvm-version-to-17-v1-1-81d9b2e8ee75@kernel.org>
On Tue, Apr 28, 2026 at 10:59:07PM -0400, Nathan Chancellor wrote:
> The current minimum version of LLVM for building the kernel is 15.0.0.
> However, there are two deficiencies compared to GCC that were fixed in
> LLVM 17 that are starting to become more noticeable.
>
> The first was a bug in LLVM's scope checker [1], where all labels in a
> function were validated as potential targets of an asm goto statement,
> even if they were not listed in the asm goto statement as targets. This
> becomes particularly problematic when the cleanup attribute is used, as
>
> asm goto(... : label_a);
> ...
> label_a:
> ...
> int var __free(foo);
> asm goto(... : label_b);
> ...
> label_b:
> ...
>
> will trigger an error since the scope checker will complain that the
> cleanup variable would be skipped when jumping from the first asm goto
> to label_b (which obviously cannot happen). This issue was the catalyst
> for commit e2ffa15b9baa ("kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on
> clang < 17"). Unfortunately, this issue is reproducible with regular asm
> goto in addition to asm goto with outputs, so that change was not
> entirely sufficient to avoid the issue altogether. As asm goto has
> effectively been required since commit a0a12c3ed057 ("asm goto:
> eradicate CC_HAS_ASM_GOTO") and the usage of the cleanup attribute
> continues to grow across the tree, raising the minimum to a version that
> avoids this issue altogether is a better long term solution than
> attempting to workaround it at every spot where it happens.
>
> The second issue is an incompatibility with GCC 8.1+ around variables
> marked with const being valid constant expressions for _Static_assert
> and other macros [2]. With GCC 8.1 being the minimum supported version
> since commit 118c40b7b503 ("kbuild: require gcc-8 and binutils-2.30"),
> this incompatibility becomes more of a maintenance burden since only
> clang-15 and clang-16 are affected by it.
>
> Looking at the clang version of various major distributions through
> Docker images, no one should be left behind as a result of this bump, as
> the old ones cannot clear the current minimum of 15.0.0.
>
> archlinux:latest clang version 22.1.3
> debian:oldoldstable-slim Debian clang version 11.0.1-2
> debian:oldstable-slim Debian clang version 14.0.6
> debian:stable-slim Debian clang version 19.1.7 (3+b1)
> debian:testing-slim Debian clang version 21.1.8 (3+b1)
> debian:unstable-slim Debian clang version 21.1.8 (7+b1)
> fedora:42 clang version 20.1.8 (Fedora 20.1.8-4.fc42)
> fedora:latest clang version 21.1.8 (Fedora 21.1.8-4.fc43)
> fedora:44 clang version 22.1.1 (Fedora 22.1.1-2.fc44)
> fedora:rawhide clang version 22.1.3 (Fedora 22.1.3-1.fc45)
> opensuse/leap:latest clang version 17.0.6
> opensuse/tumbleweed:latest clang version 21.1.8
> ubuntu:jammy Ubuntu clang version 14.0.0-1ubuntu1.1
> ubuntu:noble Ubuntu clang version 18.1.3 (1ubuntu1)
> ubuntu:questing Ubuntu clang version 20.1.8 (0ubuntu4)
> ubuntu:resolute Ubuntu clang version 21.1.8 (6ubuntu1)
>
> 17.0.1 is chosen as the minimum instead of 17.0.0 to ensure that the
> particular version of LLVM 17 has the two aforementioned bugs fixed, as
> the second was fixed during the 17.0.0 release candidate phase and it
> was not until LLVM 18 that LLVM adopted the scheme of x.0.0 being a
> prerelease version and x.1.0 is a release version [3] to help with
> scenarios such as this.
>
> Link: https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14 [1]
> Link: https://github.com/llvm/llvm-project/commit/0b2d5b967d98375793897295d651f58f6fbd3034 [2]
> Link: https://github.com/llvm/llvm-project/commit/4532617ae420056bf32f6403dde07fb99d276a49 [3]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: linux-doc@vger.kernel.org
> ---
> Documentation/process/changes.rst | 2 +-
> scripts/min-tool-version.sh | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> index 9a99037270ff..b9afce768446 100644
> --- a/Documentation/process/changes.rst
> +++ b/Documentation/process/changes.rst
> @@ -36,7 +36,7 @@ bindgen (optional) 0.71.1 bindgen --version
> binutils 2.30 ld -v
> bison 2.0 bison --version
> btrfs-progs 0.18 btrfs --version
> -Clang/LLVM (optional) 15.0.0 clang --version
> +Clang/LLVM (optional) 17.0.1 clang --version
> e2fsprogs 1.41.4 e2fsck -V
> flex 2.5.35 flex --version
> gdb 7.2 gdb --version
FTR: The translations
Documentation/translations/{it_IT,pt_BR}/process/changes.rst become now
even more outdated.
Acked-by: Nicolas Schier <nsc@kernel.org>
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Padhi, Beleswar @ 2026-05-05 16:13 UTC (permalink / raw)
To: tanmay.shah, Arnaud POULIQUEN, Mathieu Poirier
Cc: Shenwei Wang, Andrew Lunn, Linus Walleij, Bartosz Golaszewski,
Jonathan Corbet, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Frank Li, Sascha Hauer, Shuah Khan,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, devicetree@vger.kernel.org,
linux-remoteproc@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, dl-linux-imx,
Bartosz Golaszewski
In-Reply-To: <a5012462-9953-424e-bfa0-427e7b07201e@amd.com>
On 5/5/2026 9:08 PM, Shah, Tanmay wrote:
[...]
>
>>> If there are
>>> multiple or multiple instances of same controller, than we need separate
>>> channel name for that controller just like we would have separate device
>>> on the Linux.
>>
>> Why so? I think there is some confusion in the terminology:
>>
>> GPIO controller = GPIO port (gpio@xyz) defined in the
>> Device tree = struct rpmsg_gpio_port in code
>>
>> GPIO line = Individual lines within each GPIO port (max =
>> GPIOS_PER_PORT_DEFAULT) = struct rpmsg_gpio_line in code
>>
> Okay, I understand now. So, same gpio controller has multiple instances.
>
>>>>>>>> 1. namespace/channel#1 = rpmsg-io
>>>>>>>> a. ept1 -> gpio-controller@1
>>>>>>>> b. ept2 -> gpio-controller@2
> So, In that case above mentioned approach doesn't work.
>
> Because, this approach is mapping endpoint to the gpio-controller.
And what is the problem with that?
> From
> linux's perspective, it needs to map rpmsg *channel* to the
> gpio-controller not the endpoint.
Can you explain why it *needs* to?
>
> To be more specific:
>
> Linux: remote:
>
> ch1: rpmsg-gpio.-1.1024 -> gpio-controller@1024
> - gpio-line ept1
> - gpio-line ept2 -> They all map to same callback_ept_1024.
> - gpio-line ept3
This is a weird mapping. I am not suggesting one ept per gpio-line,
rather one per gpio-controller. You can refer my detailed proposal
here:
https://lore.kernel.org/all/268f8e00-91bc-43ea-ba95-077cf859e7f3@ti.com/
>
> ch2: rpmsg-gpio.-1.1025 -> gpio-controller@1025
> - gpio-line ept1
> - gpio-line ept2 -> They all map to same callback_ept_1025.
> - gpio-line ept3
[...]
>
> On the remote side, we have to hardcode Which rpmsg controller is mapped
> to which endpoint.
Not necessarily, we can exchange this info at runtime.
Thanks,
Beleswar
^ permalink raw reply
* Re: [PATCH v4 16/27] mtd: spi-nor: Create a local SR cache
From: Pratyush Yadav @ 2026-05-05 16:14 UTC (permalink / raw)
To: Miquel Raynal
Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc
In-Reply-To: <20260403-winbond-v6-18-rc1-spi-nor-swp-v4-16-833dab5e7288@bootlin.com>
On Fri, Apr 03 2026, Miquel Raynal wrote:
> In order to be able to generate debugfs output without having to
> actually reach the flash, create a SPI NOR local cache of the status
> registers. What matters in our case are all the bits related to sector
> locking. As such, in order to make it clear that this cache is not
> intended to be used anywhere else, we zero the irrelevant bits.
>
> The cache is initialized once during the early init, and then maintained
> every time the write protection scheme is updated.
What is the reason for doing so? Do the reads have side effects?
>
> Suggested-by: Michael Walle <mwalle@kernel.org>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Boris Brezillon @ 2026-05-05 16:15 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260505140516.1372388-5-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:10 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> From: Florent Tomasin <florent.tomasin@arm.com>
>
> This patch allows Panthor to allocate buffer objects from a
> protected heap. The Panthor driver should be seen as a consumer
> of the heap and not an exporter.
>
> Protected memory buffers needed by the Panthor driver:
> - On CSF FW load, the Panthor driver must allocate a protected
> buffer object to hold data to use by the FW when in protected
> mode. This protected buffer object is owned by the device
> and does not belong to a process.
> - On CSG creation, the Panthor driver must allocate a protected
> suspend buffer object for the FW to store data when suspending
> the CSG while in protected mode. The kernel owns this allocation
> and does not allow user space mapping. The format of the data
> in this buffer is only known by the FW and does not need to be
> shared with other entities.
>
> The driver will retrieve the protected heap using the name of the
> heap provided to the driver as module parameter.
>
> If the heap is not yet available, the panthor driver will defer
> the probe until created. It is an integration error to provide
> a heap name that does not exist or is never created.
>
> Panthor is calling the DMA heap allocation function
> and obtains a DMA buffer from it. This buffer is then
> registered to GEM and imported.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> ---
> Documentation/gpu/panthor.rst | 47 +++++++++++++++
> drivers/gpu/drm/panthor/Kconfig | 1 +
> drivers/gpu/drm/panthor/panthor_device.c | 28 ++++++++-
> drivers/gpu/drm/panthor/panthor_device.h | 6 ++
> drivers/gpu/drm/panthor/panthor_fw.c | 29 ++++++++-
> drivers/gpu/drm/panthor/panthor_fw.h | 2 +
> drivers/gpu/drm/panthor/panthor_gem.c | 77 ++++++++++++++++++++++--
> drivers/gpu/drm/panthor/panthor_gem.h | 16 ++++-
> drivers/gpu/drm/panthor/panthor_heap.c | 2 +
> drivers/gpu/drm/panthor/panthor_sched.c | 11 +++-
> 10 files changed, 208 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/gpu/panthor.rst b/Documentation/gpu/panthor.rst
> index 7a841741278fb..be20eadea6dd5 100644
> --- a/Documentation/gpu/panthor.rst
> +++ b/Documentation/gpu/panthor.rst
> @@ -54,3 +54,50 @@ sync object arrays and heap chunks. Because they are all allocated and pinned
> at creation time, only `panthor-resident-memory` is necessary to tell us their
> size. `panthor-active-memory` shows the size of kernel BO's associated with
> VM's and groups currently being scheduled for execution by the GPU.
> +
> +Panthor Protected Memory Integration
> +=====================================
> +
> +Panthor requires the platform to provide a protected DMA HEAP.
> +This DMA heap must be identifiable via a string name.
> +The name is defined by the system integrator, it could be hard coded
> +in the heap driver, defined by a module parameter of the heap driver
> +or else.
> +
> +.. code-block:: none
> +
> + User
> + ┌─────────────────────────────┐
> + | Application |
> + └─────────────▲───────────────┘
> + | | |
> + | DMA-BUF | | Protected
> + | | | Job Submission
> + --------|---------|----------|---------
> + Kernel | | |
> + | | |
> + | | DMA-BUF |
> + ┌───────▼─────────────┐ ┌─▼───────┐
> + | DMA PROTECTED HEAP |◄───| Panthor |
> + | (Vendor specific) | | |
> + └─────────────────────┘ └─────────┘
> + | |
> + --------|--------------------|---------
> + HW | |
> + | |
> + ┌───────▼───────────────┐ ┌─▼───┐
> + | Trusted FW | | |
> + | Protected Memory ◄──► GPU |
> + └───────────────────────┘ └─────┘
> +
> +To configure Panthor to use the protected memory heap, pass the protected memory
> +heap string name as module parameter of the Panthor module.
> +
> +Example:
> +
> + .. code-block:: shell
> +
> + insmod panthor.ko protected_heap_name=“vendor_protected_heap"
> +
> +If `protected_heap_name` module parameter is not provided, Panthor will not support
> +protected job execution.
> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
> index 911e7f4810c39..fb0bad9a0fd2b 100644
> --- a/drivers/gpu/drm/panthor/Kconfig
> +++ b/drivers/gpu/drm/panthor/Kconfig
> @@ -7,6 +7,7 @@ config DRM_PANTHOR
> depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE
> depends on MMU
> select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select DMABUF_HEAPS
> select DRM_EXEC
> select DRM_GPUVM
> select DRM_SCHED
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index bc62a498a8a84..3a5cdfa99e5fe 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -5,7 +5,9 @@
> /* Copyright 2025 ARM Limited. All rights reserved. */
>
> #include <linux/clk.h>
> +#include <linux/dma-heap.h>
> #include <linux/mm.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> @@ -27,6 +29,10 @@
> #include "panthor_regs.h"
> #include "panthor_sched.h"
>
> +MODULE_PARM_DESC(protected_heap_name, "DMA heap name, from which to allocate protected buffers");
> +static char *protected_heap_name;
> +module_param(protected_heap_name, charp, 0444);
> +
> static int panthor_gpu_coherency_init(struct panthor_device *ptdev)
> {
> BUILD_BUG_ON(GPU_COHERENCY_NONE != DRM_PANTHOR_GPU_COHERENCY_NONE);
> @@ -127,6 +133,9 @@ void panthor_device_unplug(struct panthor_device *ptdev)
> panthor_gpu_unplug(ptdev);
> panthor_pwr_unplug(ptdev);
>
> + if (ptdev->protm.heap)
> + dma_heap_put(ptdev->protm.heap);
> +
> pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> pm_runtime_put_sync_suspend(ptdev->base.dev);
>
> @@ -277,9 +286,21 @@ int panthor_device_init(struct panthor_device *ptdev)
> return ret;
> }
>
> + /* If a protected heap name is specified but not found, defer the probe until created */
> + if (protected_heap_name && strlen(protected_heap_name)) {
Do we really need this strlen() > 0? Won't dma_heap_find() fail is the
name is "" already?
> + ptdev->protm.heap = dma_heap_find(protected_heap_name);
> + if (!ptdev->protm.heap) {
> + drm_warn(&ptdev->base,
> + "Protected heap \'%s\' not (yet) available - deferring probe",
> + protected_heap_name);
> + ret = -EPROBE_DEFER;
> + goto err_rpm_put;
If you move the heap retrieval before the rpm enablement, you can get
rid of this goto err_rpm_put.
> + }
> + }
> +
> ret = panthor_hw_init(ptdev);
> if (ret)
> - goto err_rpm_put;
> + goto err_dma_heap_put;
>
> ret = panthor_pwr_init(ptdev);
> if (ret)
> @@ -343,6 +364,11 @@ int panthor_device_init(struct panthor_device *ptdev)
>
> err_rpm_put:
> pm_runtime_put_sync_suspend(ptdev->base.dev);
> +
> +err_dma_heap_put:
> + if (ptdev->protm.heap)
> + dma_heap_put(ptdev->protm.heap);
Let's use drmm_add_action_or_reset() so we don't need this manual
dma_heap_put() here or in the panthor_device_unplug() path.
> +
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 5cba272f9b4de..d51fec97fc5fa 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -7,6 +7,7 @@
> #define __PANTHOR_DEVICE_H__
>
> #include <linux/atomic.h>
> +#include <linux/dma-heap.h>
> #include <linux/io-pgtable.h>
> #include <linux/regulator/consumer.h>
> #include <linux/pm_runtime.h>
> @@ -329,6 +330,11 @@ struct panthor_device {
> struct list_head node;
> } gems;
> #endif
> + /** @protm: Protected mode related data. */
> + struct {
> + /** @heap: Pointer to the protected heap */
> + struct dma_heap *heap;
> + } protm;
> };
>
> struct panthor_gpu_usage {
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 0d07a133dc3af..1aba29b9779b6 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -500,6 +500,7 @@ panthor_fw_alloc_queue_iface_mem(struct panthor_device *ptdev,
>
> mem = panthor_kernel_bo_create(ptdev, ptdev->fw->vm, SZ_8K,
> DRM_PANTHOR_BO_NO_MMAP,
> + 0,
> DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC |
> DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED,
> PANTHOR_VM_KERNEL_AUTO_VA,
> @@ -534,6 +535,26 @@ panthor_fw_alloc_suspend_buf_mem(struct panthor_device *ptdev, size_t size)
> {
> return panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev), size,
> DRM_PANTHOR_BO_NO_MMAP,
> + 0,
> + DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> + PANTHOR_VM_KERNEL_AUTO_VA,
> + "suspend_buf");
> +}
> +
> +/**
> + * panthor_fw_alloc_protm_suspend_buf_mem() - Allocate a protm suspend buffer
> + * for a command stream group.
> + * @ptdev: Device.
> + * @size: Size of the protm suspend buffer.
> + *
> + * Return: A valid pointer in case of success, an ERR_PTR() otherwise.
> + */
> +struct panthor_kernel_bo *
> +panthor_fw_alloc_protm_suspend_buf_mem(struct panthor_device *ptdev, size_t size)
> +{
> + return panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev), size,
> + DRM_PANTHOR_BO_NO_MMAP,
> + DRM_PANTHOR_KBO_PROTECTED_HEAP,
> DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> PANTHOR_VM_KERNEL_AUTO_VA,
> "FW suspend buffer");
> @@ -547,6 +568,7 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> ssize_t vm_pgsz = panthor_vm_page_size(ptdev->fw->vm);
> struct panthor_fw_binary_section_entry_hdr hdr;
> struct panthor_fw_section *section;
> + u32 kbo_flags = 0;
> u32 section_size;
> u32 name_len;
> int ret;
> @@ -585,10 +607,13 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> return -EINVAL;
> }
>
> - if (hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) {
> + if ((hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) && !ptdev->protm.heap) {
> drm_warn(&ptdev->base,
> "Firmware protected mode entry is not supported, ignoring");
> return 0;
> + } else if ((hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) && ptdev->protm.heap) {
> + drm_info(&ptdev->base, "Firmware protected mode entry supported");
> + kbo_flags = DRM_PANTHOR_KBO_PROTECTED_HEAP;
> }
>
> if (hdr.va.start == CSF_MCU_SHARED_REGION_START &&
> @@ -653,7 +678,7 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
>
> section->mem = panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev),
> section_size,
> - DRM_PANTHOR_BO_NO_MMAP,
> + DRM_PANTHOR_BO_NO_MMAP, kbo_flags,
> vm_map_flags, va, "FW section");
> if (IS_ERR(section->mem))
> return PTR_ERR(section->mem);
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.h b/drivers/gpu/drm/panthor/panthor_fw.h
> index fbdc21469ba32..0cf3761abf789 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.h
> +++ b/drivers/gpu/drm/panthor/panthor_fw.h
> @@ -509,6 +509,8 @@ panthor_fw_alloc_queue_iface_mem(struct panthor_device *ptdev,
> u32 *input_fw_va, u32 *output_fw_va);
> struct panthor_kernel_bo *
> panthor_fw_alloc_suspend_buf_mem(struct panthor_device *ptdev, size_t size);
> +struct panthor_kernel_bo *
> +panthor_fw_alloc_protm_suspend_buf_mem(struct panthor_device *ptdev, size_t size);
>
> struct panthor_vm *panthor_fw_vm(struct panthor_device *ptdev);
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index 13295d7a593df..08fe4a5e43817 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -20,12 +20,17 @@
> #include <drm/drm_print.h>
> #include <drm/panthor_drm.h>
>
> +#include <uapi/linux/dma-heap.h>
> +
> #include "panthor_device.h"
> #include "panthor_drv.h"
> #include "panthor_fw.h"
> #include "panthor_gem.h"
> #include "panthor_mmu.h"
>
> +MODULE_IMPORT_NS("DMA_BUF");
> +MODULE_IMPORT_NS("DMA_BUF_HEAP");
> +
> void panthor_gem_init(struct panthor_device *ptdev)
> {
> int err;
> @@ -466,7 +471,6 @@ static void panthor_gem_free_object(struct drm_gem_object *obj)
> }
>
> drm_gem_object_release(obj);
> -
> kfree(bo);
> drm_gem_object_put(vm_root_gem);
> }
> @@ -1026,6 +1030,7 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> }
>
> panthor_gem_debugfs_set_usage_flags(bo, usage_flags);
> +
> return bo;
>
> err_put:
> @@ -1033,6 +1038,54 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> return ERR_PTR(ret);
> }
>
> +static struct panthor_gem_object *
> +panthor_gem_create_protected(struct panthor_device *ptdev, size_t size,
> + uint32_t flags, struct panthor_vm *exclusive_vm,
> + u32 usage_flags)
> +{
> + struct dma_buf *dma_bo = NULL;
s/dma_bo/dmabuf/
> + struct drm_gem_object *gem_obj;
> + struct panthor_gem_object *bo;
> + int ret;
> +
> + if (!ptdev->protm.heap)
> + return ERR_PTR(-EINVAL);
> +
> + if (flags != DRM_PANTHOR_BO_NO_MMAP)
> + return ERR_PTR(-EINVAL);
> +
> + if (!exclusive_vm)
> + return ERR_PTR(-EINVAL);
> +
> + dma_bo = dma_heap_buffer_alloc(ptdev->protm.heap, size, DMA_HEAP_VALID_FD_FLAGS,
> + DMA_HEAP_VALID_HEAP_FLAGS);
> + if (IS_ERR(dma_bo))
> + return ERR_PTR(PTR_ERR(dma_bo));
> +
> + gem_obj = drm_gem_prime_import(&ptdev->base, dma_bo);
You can call
dma_buf_put(dma_bo);
here. If the prime_import worked, it should have acquired a ref on the
dmabuf, and if it failed, we want to drop the ref anyway.
> + if (IS_ERR(gem_obj)) {
> + ret = PTR_ERR(gem_obj);
return PTR_ERR(gem_obj);
> + goto err_free_dma_bo;
> + }
> +
> + bo = to_panthor_bo(gem_obj);
> + bo->flags = flags;
> +
> + panthor_gem_debugfs_set_usage_flags(bo, usage_flags);
> +
> + bo->exclusive_vm_root_gem = panthor_vm_root_gem(exclusive_vm);
> + drm_gem_object_get(bo->exclusive_vm_root_gem);
> + bo->base.resv = bo->exclusive_vm_root_gem->resv;
> +
> + return bo;
> +
> +err_free_dma_bo:
> + if (dma_bo)
> + dma_buf_put(dma_bo);
> +
> + return ERR_PTR(ret);
This error path can go away.
> +}
> +
> struct drm_gem_object *
> panthor_gem_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach,
> @@ -1242,12 +1295,17 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> {
> struct panthor_device *ptdev;
> struct panthor_vm *vm;
> + struct dma_buf *dma_bo = NULL;
>
> if (IS_ERR_OR_NULL(bo))
> return;
>
> ptdev = container_of(bo->obj->dev, struct panthor_device, base);
> vm = bo->vm;
> +
> + if (bo->flags & DRM_PANTHOR_KBO_PROTECTED_HEAP)
> + dma_bo = bo->obj->import_attach->dmabuf;
> +
> panthor_kernel_bo_vunmap(bo);
>
> drm_WARN_ON(bo->obj->dev,
> @@ -1257,6 +1315,10 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> if (vm == panthor_fw_vm(ptdev))
> panthor_gem_unpin(to_panthor_bo(bo->obj));
> drm_gem_object_put(bo->obj);
> +
> + if (dma_bo)
> + dma_buf_put(dma_bo);
With panthor_gem_create_protected() tweaked as suggested, you don't
need this extra dma_buf_put(), and the dma_bo can go away.
> +
> panthor_vm_put(vm);
> kfree(bo);
> }
> @@ -1267,6 +1329,7 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> * @vm: VM to map the GEM to.
> * @size: Size of the buffer object.
> * @bo_flags: Combination of drm_panthor_bo_flags flags.
> + * @kbo_flags: Combination of drm_panthor_kbo_flags flags.
> * @vm_map_flags: Combination of drm_panthor_vm_bind_op_flags (only those
> * that are related to map operations).
> * @gpu_va: GPU address assigned when mapping to the VM.
> @@ -1278,8 +1341,8 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> */
> struct panthor_kernel_bo *
> panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
> - size_t size, u32 bo_flags, u32 vm_map_flags,
> - u64 gpu_va, const char *name)
> + size_t size, u32 bo_flags, u32 kbo_flags,
Rather than adding new flags and having to patch all current
panthor_kernel_bo_create() users as a result, I'd just add a
panthor_kernel_bo_create_protected() helper. If you want to share the
rest of the logic in panthor_kernel_bo_create(), just add a static
panthor_kernel_bo_create_from_gem() helper taking a panthor_gem_object,
and have panthor_kernel_bo_create[_protected]() call this internal
helper.
> + u32 vm_map_flags, u64 gpu_va, const char *name)
> {
> struct panthor_kernel_bo *kbo;
> struct panthor_gem_object *bo;
^ permalink raw reply
* Re: [PATCH v9 3/6] iio: adc: ad4691: add triggered buffer support
From: Jonathan Cameron @ 2026-05-05 16:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <afoFjaUBcGXeuEzH@ashevche-desk.local>
On Tue, 5 May 2026 17:58:21 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Tue, May 05, 2026 at 02:26:40PM +0100, Jonathan Cameron wrote:
>
> ...
>
> > > > > + for (i = 0; i < ARRAY_SIZE(ad4691_gp_names); i++) {
> > > > > + irq = fwnode_irq_get_byname(dev_fwnode(dev),
> > > > > + ad4691_gp_names[i]);
> > > > > + if (irq > 0)
> > > > > + break;
> > > >
> > > > This is problematic in case the above returns EPROBE_DEFER. Can you confirm
> > > > it
> > > > may not ever happen? (Note, I don't know the answer.)
> > >
> > > You are right, thanks for this!
> > I'm missing something. Why is that a problem? Driver will return
> > the error and a dev_err_probe() is used so it won't print anything.
> > So probe will fail which is exactly what we want.
>
> If there are two IRQs and the first one is probe deferred and second returns
> an error, we return that error instead of the deferral probe.
>
> May be I missed something, but I have no idea how in this case it may return
> the first error code in such a case.
Ah. Indeed. I completely misread the code. if (irq) would do the job to fix this.
J
>
^ permalink raw reply
* Re: [PATCH 5/8] drm/panthor: Minor scheduler refactoring
From: Boris Brezillon @ 2026-05-05 16:19 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260505140516.1372388-6-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:11 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> From: Florent Tomasin <florent.tomasin@arm.com>
>
> Refactor parts of the group scheduling logic into new helper functions.
> This will simplify addition of the protected mode feature.
>
> Remove redundant assignments of csg_slot.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
Glad to see this big tick_ctx_apply() function split into smaller
pieces.
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 135 +++++++++++++++---------
> 1 file changed, 86 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5ee386338005c..987072bd867c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1934,6 +1934,12 @@ static void csgs_upd_ctx_init(struct panthor_csg_slots_upd_ctx *ctx)
> memset(ctx, 0, sizeof(*ctx));
> }
>
> +static void csgs_upd_ctx_ring_doorbell(struct panthor_csg_slots_upd_ctx *ctx,
> + u32 csg_id)
> +{
> + ctx->update_mask |= BIT(csg_id);
> +}
> +
> static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
> struct panthor_csg_slots_upd_ctx *ctx,
> u32 csg_id, u32 value, u32 mask)
> @@ -1944,7 +1950,8 @@ static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
>
> ctx->requests[csg_id].value = (ctx->requests[csg_id].value & ~mask) | (value & mask);
> ctx->requests[csg_id].mask |= mask;
> - ctx->update_mask |= BIT(csg_id);
> +
> + csgs_upd_ctx_ring_doorbell(ctx, csg_id);
> }
>
> static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> @@ -1961,8 +1968,12 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> while (update_slots) {
> struct panthor_fw_csg_iface *csg_iface;
> u32 csg_id = ffs(update_slots) - 1;
> + u32 req_mask = ctx->requests[csg_id].mask;
>
> update_slots &= ~BIT(csg_id);
> + if (!req_mask)
> + continue;
> +
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> panthor_fw_update_reqs(csg_iface, req,
> ctx->requests[csg_id].value,
> @@ -1979,6 +1990,9 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> int ret;
>
> update_slots &= ~BIT(csg_id);
> + if (!req_mask)
> + continue;
> +
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
>
> ret = panthor_fw_csg_wait_acks(ptdev, csg_id, req_mask, &acked, 100);
> @@ -2266,12 +2280,76 @@ tick_ctx_cleanup(struct panthor_scheduler *sched,
> }
> }
>
> +static void
> +tick_ctx_evict_group(struct panthor_scheduler *sched,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> +
> + if (drm_WARN_ON(&ptdev->base, group->csg_id < 0))
> + return;
> +
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, group->csg_id,
> + group_can_run(group) ?
> + CSG_STATE_SUSPEND : CSG_STATE_TERMINATE,
> + CSG_STATE_MASK);
> +}
> +
> +
> +static void
> +tick_ctx_reschedule_group(struct panthor_scheduler *sched,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group,
> + int new_csg_prio)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> + struct panthor_fw_csg_iface *csg_iface;
> + struct panthor_csg_slot *csg_slot;
> +
> + if (group->csg_id < 0)
> + return;
> +
> + csg_iface = panthor_fw_get_csg_iface(ptdev, group->csg_id);
> + csg_slot = &sched->csg_slots[group->csg_id];
> +
> + if (csg_slot->priority != new_csg_prio) {
> + panthor_fw_update_reqs(csg_iface, endpoint_req,
> + CSG_EP_REQ_PRIORITY(new_csg_prio),
> + CSG_EP_REQ_PRIORITY_MASK);
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, group->csg_id,
> + csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> + CSG_ENDPOINT_CONFIG);
> + }
> +}
> +
> +static void
> +tick_ctx_schedule_group(struct panthor_scheduler *sched,
> + struct panthor_sched_tick_ctx *ctx,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group,
> + int csg_id, int csg_prio)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> + struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> +
> + group_bind_locked(group, csg_id);
> + csg_slot_prog_locked(ptdev, csg_id, csg_prio);
> +
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
> + group->state == PANTHOR_CS_GROUP_SUSPENDED ?
> + CSG_STATE_RESUME : CSG_STATE_START,
> + CSG_STATE_MASK);
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
> + csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> + CSG_ENDPOINT_CONFIG);
> +}
> +
> static void
> tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *ctx)
> {
> struct panthor_group *group, *tmp;
> struct panthor_device *ptdev = sched->ptdev;
> - struct panthor_csg_slot *csg_slot;
> int prio, new_csg_prio = MAX_CSG_PRIO, i;
> u32 free_csg_slots = 0;
> struct panthor_csg_slots_upd_ctx upd_ctx;
> @@ -2282,42 +2360,12 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
> for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
> /* Suspend or terminate evicted groups. */
> list_for_each_entry(group, &ctx->old_groups[prio], run_node) {
> - bool term = !group_can_run(group);
> - int csg_id = group->csg_id;
> -
> - if (drm_WARN_ON(&ptdev->base, csg_id < 0))
> - continue;
> -
> - csg_slot = &sched->csg_slots[csg_id];
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - term ? CSG_STATE_TERMINATE : CSG_STATE_SUSPEND,
> - CSG_STATE_MASK);
> + tick_ctx_evict_group(sched, &upd_ctx, group);
> }
>
> /* Update priorities on already running groups. */
> list_for_each_entry(group, &ctx->groups[prio], run_node) {
> - struct panthor_fw_csg_iface *csg_iface;
> - int csg_id = group->csg_id;
> -
> - if (csg_id < 0) {
> - new_csg_prio--;
> - continue;
> - }
> -
> - csg_slot = &sched->csg_slots[csg_id];
> - csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> - if (csg_slot->priority == new_csg_prio) {
> - new_csg_prio--;
> - continue;
> - }
> -
> - panthor_fw_csg_endpoint_req_update(ptdev, csg_iface,
> - CSG_EP_REQ_PRIORITY(new_csg_prio),
> - CSG_EP_REQ_PRIORITY_MASK);
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> - CSG_ENDPOINT_CONFIG);
> - new_csg_prio--;
> + tick_ctx_reschedule_group(sched, &upd_ctx, group, new_csg_prio--);
> }
> }
>
> @@ -2354,28 +2402,17 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
> for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
> list_for_each_entry(group, &ctx->groups[prio], run_node) {
> int csg_id = group->csg_id;
> - struct panthor_fw_csg_iface *csg_iface;
> + int csg_prio = new_csg_prio--;
>
> - if (csg_id >= 0) {
> - new_csg_prio--;
> + if (csg_id >= 0)
> continue;
> - }
>
> csg_id = ffs(free_csg_slots) - 1;
> if (drm_WARN_ON(&ptdev->base, csg_id < 0))
> break;
>
> - csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> - csg_slot = &sched->csg_slots[csg_id];
> - group_bind_locked(group, csg_id);
> - csg_slot_prog_locked(ptdev, csg_id, new_csg_prio--);
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - group->state == PANTHOR_CS_GROUP_SUSPENDED ?
> - CSG_STATE_RESUME : CSG_STATE_START,
> - CSG_STATE_MASK);
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> - CSG_ENDPOINT_CONFIG);
> + tick_ctx_schedule_group(sched, ctx, &upd_ctx, group, csg_id, csg_prio);
> +
> free_csg_slots &= ~BIT(csg_id);
> }
> }
^ permalink raw reply
* Re: [PATCH 6/8] drm/panthor: Explicit expansion of locked VM region
From: Boris Brezillon @ 2026-05-05 16:32 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek
In-Reply-To: <20260505140516.1372388-7-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:12 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> Currently the panthor_vm_lock_region() function will implicitly expand
> an already locked VM region. This can be problematic because the caller
> do not reliably know if it needs to call panthor_vm_unlock_region()
> or not.
>
> Worth noting, there is currently no known issues with this as the code
> is written today.
>
> This change introduces panthor_vm_expand_region() which will only work
> if there is already a locked VM region. This again means that the
> original lock and unlock functions can work as a pair. This pairing is
> needed for subsequent protected memory changes.
>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> ---
> drivers/gpu/drm/panthor/panthor_mmu.c | 69 +++++++++++++++++++--------
> 1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index fc930ee158a52..07f54176ec1bf 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1701,15 +1701,36 @@ static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
> struct panthor_device *ptdev = vm->ptdev;
> int ret = 0;
>
> - /* sm_step_remap() can call panthor_vm_lock_region() to account for
> - * the wider unmap needed when doing a partial huge page unamp. We
> - * need to ignore the lock if it's already part of the locked region.
> - */
> - if (start >= vm->locked_region.start &&
> - start + size <= vm->locked_region.start + vm->locked_region.size)
> - return 0;
> + if (drm_WARN_ON(&ptdev->base, vm->locked_region.size))
> + return -EINVAL;
How about we have a helper called panthor_vm_apply_as_lock() that would
only take care of the AS_LOCKADDR() sequence. panthor_vm_lock_region()
would have this WARN_ON(), the pack_region_range() and a call to
panthor_vm_apply_as_lock(). Similarly,
panthor_vm_expand_locked_region() would rely on
panthor_vm_apply_as_lock() to apply the expanded lock.
> +
> + mutex_lock(&ptdev->mmu->as.slots_lock);
> + if (vm->as.id >= 0 && size) {
> + /* Lock the region that needs to be updated */
> + gpu_write64(ptdev, AS_LOCKADDR(vm->as.id),
> + pack_region_range(ptdev, &start, &size));
> +
> + /* If the lock succeeded, update the locked_region info. */
> + ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_LOCK);
> + }
>
> - /* sm_step_remap() may need a locked region that isn't a strict superset
> + if (!ret) {
> + vm->locked_region.start = start;
> + vm->locked_region.size = size;
> + }
> + mutex_unlock(&ptdev->mmu->as.slots_lock);
> +
> + return ret;
> +}
> +
> +static int panthor_vm_expand_region(struct panthor_vm *vm, u64 start, u64 size)
s/panthor_vm_expand_region/panthor_vm_expand_locked_region/
> +{
> + struct panthor_device *ptdev = vm->ptdev;
> + u64 end;
> + int ret = 0;
> +
> + /* This function is here to handle the following case:
> + * sm_step_remap() may need a locked region that isn't a strict superset
> * of the original one because of having to extend unmap boundaries beyond
> * it to deal with partial unmaps of transparent huge pages. What we want
> * in those cases is to lock the union of both regions. The new region must
> @@ -1717,16 +1738,24 @@ static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
> * boundaries in a remap operation can only shift up or down respectively,
> * but never otherwise.
> */
> - if (vm->locked_region.size) {
> - u64 end = max(vm->locked_region.start + vm->locked_region.size,
> - start + size);
>
> - drm_WARN_ON_ONCE(&vm->ptdev->base, (start + size <= vm->locked_region.start) ||
> - (start >= vm->locked_region.start + vm->locked_region.size));
> + /* This function can only expand an already locked region */
> + if (drm_WARN_ON(&ptdev->base, !vm->locked_region.size))
> + return -EINVAL;
>
> - start = min(start, vm->locked_region.start);
> - size = end - start;
> - }
> + /* Early out if requested range is already locked */
> + if (start >= vm->locked_region.start &&
> + start + size <= vm->locked_region.start + vm->locked_region.size)
> + return 0;
> +
> + end = max(vm->locked_region.start + vm->locked_region.size,
> + start + size);
> +
> + drm_WARN_ON_ONCE(&ptdev->base, (start + size <= vm->locked_region.start) ||
> + (start >= vm->locked_region.start + vm->locked_region.size));
> +
> + start = min(start, vm->locked_region.start);
> + size = end - start;
>
> mutex_lock(&ptdev->mmu->as.slots_lock);
> if (vm->as.id >= 0 && size) {
> @@ -2252,11 +2281,13 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
> unmap_hugepage_align(&op->remap, &unmap_start, &unmap_range);
>
> /* If the range changed, we might have to lock a wider region to guarantee
> - * atomicity. panthor_vm_lock_region() bails out early if the new region
> - * is already part of the locked region, so no need to do this check here.
> + * atomicity.
> */
> if (!unmap_vma->evicted) {
> - panthor_vm_lock_region(vm, unmap_start, unmap_range);
> + ret = panthor_vm_expand_region(vm, unmap_start, unmap_range);
> + if (ret)
> + return ret;
> +
> panthor_vm_unmap_pages(vm, unmap_start, unmap_range);
> }
>
^ permalink raw reply
* Re: [PATCH 1/8] dma-heap: Add proper kref handling on dma-buf heaps
From: Boris Brezillon @ 2026-05-05 16:40 UTC (permalink / raw)
To: Maxime Ripard
Cc: Ketil Johnsen, David Airlie, Simona Vetter, Maarten Lankhorst,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Yong Wu, Yunfei Dong,
Florent Tomasin
In-Reply-To: <20260505-spaniel-of-scientific-warranty-ca075e@houat>
On Tue, 5 May 2026 17:39:13 +0200
Maxime Ripard <mripard@kernel.org> wrote:
> Hi Boris,
>
> On Tue, May 05, 2026 at 05:20:48PM +0200, Boris Brezillon wrote:
> > Hi Ketil,
> >
> > On Tue, 5 May 2026 16:05:07 +0200
> > Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> >
> > > From: John Stultz <jstultz@google.com>
> > >
> > > Add proper reference counting on the dma_heap structure. While
> > > existing heaps are built-in, we may eventually have heaps loaded
> > > from modules, and we'll need to be able to properly handle the
> > > references to the heaps
> >
> > It's weird that this "heap as module" thing is mentioned here, but
> > actual robustness to make this safe is not added in the commit or any
> > of the following ones.
> >
> > >
> > > Signed-off-by: John Stultz <jstultz@google.com>
> > > Signed-off-by: T.J. Mercier <tjmercier@google.com>
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > [Yong: Just add comment for "minor" and "refcount"]
> > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > > [Yunfei: Change reviewer's comments]
> > > Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> > > [Florent: Rebase]
> > > Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> > > [Ketil: Rebase]
> > > ---
> > > drivers/dma-buf/dma-heap.c | 29 +++++++++++++++++++++++++++++
> > > include/linux/dma-heap.h | 2 ++
> > > 2 files changed, 31 insertions(+)
> > >
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> > > index ac5f8685a6494..9fd365ddbd517 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -12,6 +12,7 @@
> > > #include <linux/dma-heap.h>
> > > #include <linux/err.h>
> > > #include <linux/export.h>
> > > +#include <linux/kref.h>
> > > #include <linux/list.h>
> > > #include <linux/nospec.h>
> > > #include <linux/syscalls.h>
> > > @@ -31,6 +32,7 @@
> > > * @heap_devt: heap device node
> > > * @list: list head connecting to list of heaps
> > > * @heap_cdev: heap char device
> > > + * @refcount: reference counter for this heap device
> > > *
> > > * Represents a heap of memory from which buffers can be made.
> > > */
> > > @@ -41,6 +43,7 @@ struct dma_heap {
> > > dev_t heap_devt;
> > > struct list_head list;
> > > struct cdev heap_cdev;
> > > + struct kref refcount;
> > > };
> > >
> > > static LIST_HEAD(heap_list);
> > > @@ -248,6 +251,7 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> > > if (!heap)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > + kref_init(&heap->refcount);
> > > heap->name = exp_info->name;
> > > heap->ops = exp_info->ops;
> > > heap->priv = exp_info->priv;
> > > @@ -313,6 +317,31 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> > > }
> > > EXPORT_SYMBOL_NS_GPL(dma_heap_add, "DMA_BUF_HEAP");
> > >
> > > +static void dma_heap_release(struct kref *ref)
> > > +{
> > > + struct dma_heap *heap = container_of(ref, struct dma_heap, refcount);
> > > + unsigned int minor = MINOR(heap->heap_devt);
> > > +
> > > + mutex_lock(&heap_list_lock);
> > > + list_del(&heap->list);
> > > + mutex_unlock(&heap_list_lock);
> > > +
> > > + device_destroy(dma_heap_class, heap->heap_devt);
> > > + cdev_del(&heap->heap_cdev);
> > > + xa_erase(&dma_heap_minors, minor);
> > > +
> > > + kfree(heap);
> >
> > That's actually problematic, because cdev_del() doesn't guarantee that
> > all opened FDs have been closed [1], it just guarantees that no new ones
> > can materialize. In order to make that safe, we'd need a
> >
> > 1. kref_get_unless_zero() in dma_heap_open(), with proper locking around
> > the xa_load() to protect against the heap removal that's happening
> > here
> > 2. a dma_heap_put() in a new dma_heap_close() implementation
> > 3. a guarantee that heap implementations won't go away until the last
> > ref is dropped, which means ops and all the data needed for this heap
> > to satisfy ioctl()s (and more generally every passed at
> > dma_heap_add() time) have to stay valid until the last ref is
> > dropped. Alternatively, we could restrict this only to in-flight
> > ioctl()s, and have the ops replaced by some dummy ops using RCU or a
> > rwlock. But I guess live dmabufs allocated on this heap have to
> > retain the heap and its implementation anyway.
> >
> > For record, #3 is already not satisfied by the current tee_heap
> > implementation (tee_dma_heap objects can vanish before the dma_heap
> > object is gone). The other implementations seem to be fine because they
> > are statically linked, and they either have exp_info.priv set to NULL,
> > or something that's never released.
>
> That statement won't hold for long, see:
> https://lore.kernel.org/r/20260427-dma-buf-heaps-as-modules-v5-0-b6f5678feefc@kernel.org
>
> However, all upstream heaps can be loaded as module, but not unloaded.
> So once you get a reference to it, you can assume it will live forever.
> That's why we didn't merge that patch before, even though it was discussed:
>
> https://lore.kernel.org/all/CANDhNCqk9Uk4aXHhUsL4hR1GHNmWZnH3C9Np-A02wdi+J3D7tA@mail.gmail.com/
Hm, not too sure that makes the tee_heap implementation sane WRT
tee_heap removal though, unless we have a guarantee that
tee_device_unregister() will never be called...
^ permalink raw reply
* Re: [PATCH v10 05/11] iio: core: add decimal value formatting into 64-bit value
From: Jonathan Cameron @ 2026-05-05 16:42 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Rodrigo Alencar, linux-kernel, linux-iio, devicetree, linux-doc,
David Lechner, Andy Shevchenko, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Andrew Morton, Petr Mladek, Steven Rostedt,
Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky, Shuah Khan
In-Reply-To: <keeskqikjulxcgc36zpzzvqw2osvm437hlchgcuovh5gkvmdr3@ishisbiujrrn>
On Mon, 4 May 2026 11:42:38 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/04/15 10:51AM, Rodrigo Alencar wrote:
> > Create new format types for iio values (IIO_VAL_DECIMAL64_*), which
> > defines the representation of fixed decimal point values into a single
> > 64-bit number. This new format increases the range of represented values,
> > allowing for integer parts greater than 2^32, as bits are not "wasted"
> > in the fractional part, which can be seen in IIO_VAL_INT_PLUS_MICRO and
> > IIO_VAL_INT_PLUS_NANO. Helper macros are created to compose and decompose
> > 64-bit decimals into integer values used in IIO formatting interfaces,
> > which creates consistency and avoid error-prone manual assignments when
> > using wordpart macros. When doing the parsing, kstrtodec64() is used with
> > the scale defined by the specific decimal format type.
> >
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> ...
>
> > --- a/include/linux/iio/types.h
> > +++ b/include/linux/iio/types.h
> > @@ -7,6 +7,7 @@
> > #ifndef _IIO_TYPES_H_
> > #define _IIO_TYPES_H_
> >
> > +#include <linux/wordpart.h>
> > #include <uapi/linux/iio/types.h>
> >
> > enum iio_event_info {
> > @@ -34,6 +35,38 @@ enum iio_event_info {
> > #define IIO_VAL_FRACTIONAL_LOG2 11
> > #define IIO_VAL_CHAR 12
> >
> > +#define IIO_VAL_DECIMAL64_BASE 100
> > +#define IIO_VAL_DECIMAL64_MILLI (IIO_VAL_DECIMAL64_BASE + 3)
> > +#define IIO_VAL_DECIMAL64_MICRO (IIO_VAL_DECIMAL64_BASE + 6)
> > +#define IIO_VAL_DECIMAL64_NANO (IIO_VAL_DECIMAL64_BASE + 9)
> > +#define IIO_VAL_DECIMAL64_PICO (IIO_VAL_DECIMAL64_BASE + 12)
> > +
> > +#define iio_val_s64_compose(_val0, _val1) \
> > + ({ (s64)((((u64)(_val1)) << 32) | (u32)(_val0)); })
> > +
> > +#define iio_val_s64_from_array(_vals) \
> > + ({ \
> > + const int *_arr = (const int *)(_vals); \
> > + s64 _dec64 = iio_val_s64_compose(_arr[0], _arr[1]); \
> > + \
> > + _dec64; \
> > + })
> > +
> > +#define iio_val_s64_decompose(_dec64, _val0, _val1) \
> > + do { \
> > + s64 _tmp64 = (s64)(_dec64); \
> > + \
> > + *(_val0) = lower_32_bits(_tmp64); \
> > + *(_val1) = upper_32_bits(_tmp64); \
> > + } while (0)
> > +
> > +#define iio_val_s64_array_populate(_dec64, _vals) \
> > + do { \
> > + int *_arr = (int *)(_vals); \
> > + \
> > + iio_val_s64_decompose((_dec64), &_arr[0], &_arr[1]); \
> > + } while (0)
>
> Hi Jonathan,
>
> Are those macros ok? in terms of where they are declared or whether they
> should be static inline functions? any preferences?
My preference would be for static inline functions rather than macros given
all the types are known here.
>
> > +
> > enum iio_available_type {
> > IIO_AVAIL_LIST,
> > IIO_AVAIL_RANGE,
> >
> > --
> > 2.43.0
> >
>
^ permalink raw reply
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
From: Jonathan Cameron @ 2026-05-05 17:08 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-iio,
devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <dsppqeyqtelrt3arkgxdgkhxvgtqpxkvf6n3rbkz4wk7zgfwqb@cn4rqk7p4g6i>
On Mon, 27 Apr 2026 15:35:21 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/04/27 10:46AM, Jonathan Cameron wrote:
> > On Sun, 26 Apr 2026 21:42:15 +0100
> > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> >
> > > On 26/04/26 02:10PM, Jonathan Cameron wrote:
> > > > On Fri, 17 Apr 2026 09:17:38 +0100
> > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> > > >
> > > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > >
> > > > > Add documentation for the AD9910 DDS IIO driver, which describes channels,
> > > > > DDS modes, attributes and ABI usage examples.
> > > > >
> > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > >
> > > > Hi Rodrigo,
> > > >
> > > > I think this is getting close to something workable subject to some tweaks
> > > > to not make the priority thing visible and use rate of change parameters
> > > > so /Sec rather than steps.
> > >
> > > I am not sure about this one. Getting the value into units per seconds will
> > > increase the range of values by a lot, e.g., for the frequency case the step
> > > size can range from a few Hz up to the entire supported range (hundreds of
> > > MHz), and if you consider that one would often have the sampling_frequency
> > > at 250 MHz... an attribute frequency_roc could have an order of 10^17 Hz/s,
> > > and I am not sure how practical is that, although it can have a physical meaning,
> > > like a "chirp slope".
> >
> > That scaling is indeed a bit of a pain though it will go in a 64 bit int
> > however, seems likely we'll get higher frequency devices one day that will
> > limb even faster.
> >
> > Maybe wait and see if anyone else has input on this.
> > >
> > > >
> > > > Given this defines the ABI for a whole class of new devices that are
> > > > rather complex, one concern is whether whatever we define here is general
> > > > enough to be useful.
> > > >
> > > > Do you have any other DDS in your queue to upstream? Maybe it's worth
> > > > sanity checking the ABI against them to see if it is fit for purpose?
> > >
> > > Not really, still the only DDS. Other DDS of the same family have a similar
> > > Digital Ramp Generator with controls over ramp limits, rates and step.
> >
> > There are two in staging that have been there a very long time...
> > ad9832 and ad9834. I haven't looked at how they correspond to this.
>
> It seems they can benefit of this child channel concept, as they define
> multiple frequency and phase configuration for the same physical DAC.
> I see the following in the most complicated one:
> - out_altvoltage0_frequency0
> - out_altvoltage0_frequency1
> - out_altvoltage0_frequency_scale
> - out_altvoltage0_phase0
> - out_altvoltage0_phase1
> - out__altvoltage0_phase_scale
> - out_altvoltage0_pincontrol_en
> - out_altvoltage0_frequencysymbol
> - out_altvoltage0_phasesymbol
> - out_altvoltage0_out_enable
> - out_altvoltage0_out1_enable
> - out_altvoltage0_out0_wavetype
> - out_altvoltage0_out1_wavetype
> - out_altvoltage0_out0_wavetype_available
> - out_altvoltage0_out1_wavetype_available
>
> less complicated, no RAM, no DRG nor parallel port. In terms of common stuff I can see
> frequency_scale and pinctrl_en.
My main concern here is IIRC these are very much PSK / FSK devices with expectation
of symbol being externally controlled. (maybe I remembered that wrong ;)
So the symbol relationship would need to map to child channels in some fashion.
>
> > We should think hard about whether to bring them inline with this
> > and out of staging, or just delete them.
> >
> > >
> > > ...
> > >
> > > > > +DDS modes
> > > > > +=========
> > > > > +
> > > > > +The AD9910 supports multiple modes of operation that can be configured
> > > > > +independently or in combination. Such modes and their corresponding IIO channels
> > > > > +are described in this section. The following tables are extracted from the
> > > > > +AD9910 datasheet and summarizes the control parameters for each mode and their
> > > > > +priority when multiple sources are enabled simultaneously:
> > > >
> > > > Maybe add a bit on what priority means. Does it mean that only the highest
> > > > priority one is acted on? If so why do we need to expose that others are
> > > > enabled? Just report only the highest priority one as enabled.
> > > >
> > > > I can see the hardware needs to do priority so it knows where to go when
> > > > a given source is disabled but from a software point of view that
> > > > can be controlled by us enabling that next item (and the driver does
> > > > things in the right order to get the appropriate transition)
> > > >
> > > > That may mean that if all modes are disabled, we have to disable any output
> > > > but seems doable.
> > >
> > > That is a bit complicated, as you can see, this part has modes that target
> > > one DDS parameter or multiple (destinations: phase, frequency, amplitude).
> > > Also, multiple modes can coexist, when they target different parameters/destination.
> > > At the same time, RAM mode complicates everything because even though it targets
> > > one specific parameter, once it is enabled influences the base mode for the
> > > other parameters because single-tone is off. I have ordered the mode channels so
> > > that higher index have higher priority, so that can be a bit clearer.
> > >
> > > Right now, all the controls are provided, what might be missing is a way
> > > to query which level of those priorities is currently active, but those levels
> > > are not the same thing as the controls. If we turn the priority levels into the
> > > controls themselves it would be a different ABI and it would get a lot messy.
> > > That is why I am dumping this priority table in this document! =(
> >
> > I understand (at least some) of the hardware complexity but I don't like
> > the fact this is effectively exposing it to userspace. + I really don't want
> > more ABI to indicate whether a mode is actively doing anything or not.
> > Whilst I agree the code will be more complex, having clarity on what is
> > enabled at any given time is definitely something we ant to aim for.
>
> This part seems special, so I would not bother with the fact that userspace
> needs to figure some things out.
I'm kind of fine for that being needed for complex usecases but if I want
to do something simple I'd like that to be standard and intuitive.
Obviously gets blurry wrt to what counts as simple.
>
> Since there is this priority, there is a difference on enable vs active,
> and things are different for each DDS parameter. Take the single tone example
> when it is enabled, should that mean that it is active on frequency, phase
> or amplitude? that granularity could be exposed like that if we create dedicated
> channels for each parameter on every mode possible, which turns the ABI into
> a greater mess.
I did wonder if we should separate those but agree it's tricky.
>
> > Can we work out a transition diagram? That might make it easier to
> > tell whether it's possible to map it as single enables at a time and
> > incorporate weird corners like the RAM one. Maybe not needed if the
> > RAM one is the only real oddity and otherwise it's just going up
> > and down the priority lists.
>
> I still think it is not that simple, an enable is just a control for
> a specific mode-destination pair.
>
> > One complexity I can see with single enables is that they'd need
> > to be separate for each of frequency, phase and amplitude to reflect
> > the transitions that can occur.
>
> As mentioned above, that would be messy to interface with...
>
> > Also the fun of profiles, where those profile pins are basically picking
> > symbols - could be used for multi level PSK or FSK for example if wired
> > up to an external symbol source. I'd be a bit surprised if those are
> > always wired up to a host CPU.
>
> Yes, some users may want to control the profile pins through an FPGA.
>
> > *sigh* I'm talking my self around to needing ABI to indicate a channel
> > is active. The symbol stuff gets us some of the way there (and would
> > work for the tones) but doesn't cover the added complexity of RAM etc.
> > So 'maybe' new ABI for _isactive or something like that?
>
> active_params or active_targets? listing out DDS parameters that is currently
> active for that mode/channel? like printing "frequency phase amplitude", so
> that would mean the mode is driving all of them. That would be a read-only
> status.
Would have to be simple per channel sysfs attribute that would say
is this particular channel actually contributing to the current output.
Where it can contribute in multiple ways we'll need separate attributes.
Anything else is going to break the rules on one thing per each sysfs
attribute.
out_altvoltageXXX_phase_active
out_altvoltageXXX_freq_active
or something like that.
>
> > Perhaps the boundary we put on this is the ABI should be such that
> > simple choices such as enabling a single tone, or single RAM mode
> > setting are intuitive.
> >
> > Why do we only have one ram channel? I'd kind of expect the firmware
> > to fill all 8 RAM profiles because of that 'external' profile pins
> > use case.
>
> Most of the RAM configuration is now comming from the firmware, so there
> is no much info to display/configure in multiple channels. The per-profile
> configs require the weird ABI like operating modes and address start/end.
> By removing those, I have mostly "global" stuff.
If folk are doing profile control from external pins I'd kind of expect to
see some description of what they are controlling even if it's read only.
Maybe we don't do this now but we should make sure there is space in the ABI
by thinking what it might look like.
>
> > > > > +
> > > > > +.. flat-table:: DDS Frequency Control
> > > > > + :header-rows: 1
> > > > > +
> > > > > + * - Priority
> > > > > + - Data Source
> > > > > + - Conditions
> > > > > +
> > > > > + * - Highest Priority
> > > > > + - RAM
> > > > > + - RAM enabled and data destination is frequency
> > > > > +
> > > > > + * -
> > > > > + - DRG
> > > > > + - DRG enabled and data destination is frequency
> > > > > +
> > > > > + * -
> > > > > + - Parallel data and FTW (frequency_offset)
> > > > > + - Parallel data port enabled and data destination is frequency
> > > > > +
> > > > > + * -
> > > > > + - FTW (frequency)
> > > > > + - RAM enabled and data destination is not frequency
> > > > > +
> > > > > + * -
> > > > > + - FTW (frequency) in single tone channel for the active profile
> > > > > + - DRG enabled and data destination is not frequency
> > > > > +
> > > > > + * -
> > > > > + - FTW (frequency) in single tone channel for the active profile
> > > > > + - Parallel data port enabled and data destination is not frequency
> > > > > +
> > > > > + * - Lowest Priority
> > > > > + - FTW (frequency) in single tone channel for the active profile
> > > > > + - None
> > > >
> > > > > +
> > > > > +Single tone mode
> > > > > +----------------
> > > > > +
> > > > > +Single tone is the baseline operating mode. The ``profile[Y]`` channels
> > > > > +provides enable, frequency, phase and amplitude control:
> > > > > +
> > > > > +.. flat-table::
> > > > > + :header-rows: 1
> > > > > +
> > > > > + * - Attribute
> > > > > + - Unit
> > > > > + - Description
> > > > > +
> > > > > + * - ``en``
> > > > > + - boolean
> > > > > + - Enable/disable profile Y. Only one profile can be active at a
> > > > > + time. Then enabling a profile disables the current active profile.
> > > > > + Disabling an active profile enables the next profile in ascending order,
> > > > > + wrapping around from 7 to 0.
> > > >
> > > > That passing on to the next one seems rather non user friendly. Can we just
> > > > disable the whole unit under those conditions instead? As above that may mean
> > > > turning of the output entirely. So to change mode it would always be transition
> > > > to the one that is enabled. A disable of a given channel results in no output.
> > >
> > > Yes, I can go for the software powerdown in that case! and the powerdown attribute
> > > could be removed?
> >
> > Yes, I think that works. If all sources are disabled, then powerdown.
> > Maybe we keep the powerdown as well though as that's standard DAC ABI.
>
> and what would happen when we power up and none of the profiles are enabled?
return -EBUSY. This is beyond the 'do sensible things' in intuitive way
boundary so I don't mind being a bit obtuse.
> > >
> > > > > +Digital ramp generator (DRG)
> > > > > +----------------------------
> > > > > +
> > > > > +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> > > > > +hardware. It is controlled through three channels: a parent control channel
> > > > > +(``digital_ramp_generator``) and two child ramp channels
> > > > > +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> > > > > +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
> > > > > +sets the destination to frequency.
> > > > > +
> > > > > +Control channel attributes
> > > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > +
> > > > > +.. flat-table::
> > > > > + :header-rows: 1
> > > > > +
> > > > > + * - Attribute
> > > > > + - Unit
> > > > > + - Description
> > > > > +
> > > > > + * - ``en``
> > > > > + - boolean
> > > > > + - Enable/disable the DRG.
> > > > > +
> > > > > +Ramp channel attributes
> > > > > +^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > +
> > > > > +The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
> > > > > +attribute set but configure ascending and descending ramp parameters
> > > > > +independently:
> > > > > +
> > > > > +.. flat-table::
> > > > > + :header-rows: 1
> > > > > +
> > > > > + * - Attribute
> > > > > + - Unit
> > > > > + - Description
> > > > > +
> > > > > + * - ``en``
> > > > > + - boolean
> > > > > + - Enable/disable the ramp no-dwell behavior. Enabling both creates a
> > > > > + bidirectional continuous ramp (Triangular pattern). Other configurations
> > > > > + creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
> > > >
> > > > transition
> > > >
> > > > > + only, ramp-down only or bidirectional with dwell at the limits.
> > > >
> > > > Feels a little unintuitive to use the generic enable for this.
> > > > We might need a specific control for this one.
> > >
> > > How about dwell_en, but it might not sound that generic. I used "enable" because:
> > > - no-dwell high means a ramp-up pattern (only enabling the ramp-up channel)
> > > - no-dwell low means a ramp-down pattern (only enabling the ramp-down channel)
> > > - both no-dwell is a continuous ramp that goes up and down. (both enabled)
> > > The last case is a bit off though, when both are disabled we get the normal mode, which
> > > is also a ramps up and down, but dwelling in the limits.
> > >
> > > > > +
> > > > > + * - ``frequency``
> > > > > + - Hz
> > > > > + - Frequency ramp limit. Range [0, SYSCLK/2).
> > > > > +
> > > > > + * - ``phase``
> > > > > + - rad
> > > > > + - Phase ramp limit. Range [0, 2*pi).
> >
> > Looking at this again, how do we set the DRG mode? E.g. if it effects
> > only phase?
>
> You mean the destination? I removed the destination ABI. so now destination is
> set when we write to either frequency, phase or scale properties.
> * writing to frequency or frequency_step sets the destination to frequency
> * writing to phase or phase_step sets the destination to phase
> * writing to scale or scale_step sets the destination to amplitude
So it's last write that sets it. We definitely need a way to know which
one is active if we get multiple writes. What do the others return if they
were set but something else has been set since?
>
> The DRG mode (dwell mode) is now controlled with the enable bits in the ramp
> up/down channels
>
> > > > > +
> > > > > + * - ``scale``
> > > > > + - fractional
> > > > > + - Amplitude scale ramp limit. Range [0, 1).
> > > > > +
> > > > > + * - ``sampling_frequency``
> > > > > + - Hz
> > > > > + - Ramp clock rate: SYSCLK / (4 * divider).
> > > > > +
> > > > > + * - ``frequency_step``
> > > > > + - Hz
> > > > > + - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
> > > >
> > > > So this was the bit I referred to earlier. Normally we do
> > > > rate of change measurements for this stuff rather than what happens on
> > > > each tick (based on how we handle things like ROC events)
> > > >
> > > > So could we make these
> > > > ``frequency_roc`` units HZ/Sec
> > > > etc? Then from the mix configured would need to work out the optimum
> > > > tick to deliver it.
> > > >
> > > > I suppose it's possible that someone might want a stepped frequency
> > > > though which would break this approach? Does anyone actually do that?
> > > > If so we'd need to keep the samping_frequency but then control _roc
> > > > with that in mind.
> > >
> > > yeah... frequency steps would make sense when the user controls when to
> > > perform the updates, or when it comes from certain events.
> >
> > You've lost me here. How can they do that? Some external clocking
> > or event?
>
> That would depend on what the user does. This part has this DRHOLD pin
> which can freeze the ramp. If the user sets this HIGH and creates pulses
> it is able to control the stepping of the RAMP manually. But I assume
> that no one would do that... such application is unknown to me.
They'd have to sync that with the clock driving the ramp. I guess that's
possible - but nasty. I vote we pretend this usecase doesn't exist for now ;)
> > > > > +
> > > > > +Output shift keying (OSK)
> > > > This is a new one on me...
> > > > > +-------------------------
> > > > > +
> > > > > +OSK controls the output amplitude envelope, allowing the output to be ramped
> > > > > +on/off rather than switched abruptly.
> > > >
> > > > > +
> > > > > +.. flat-table::
> > > > > + :header-rows: 1
> > > > > +
> > > > > + * - Attribute
> > > > > + - Unit
> > > > > + - Description
> > > > > +
> > > > > + * - ``en``
> > > > > + - boolean
> > > > > + - Enable/disable OSK.
> > > > > +
> > > > > + * - ``scale``
> > > > > + - fractional
> > > > > + - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
> > > > > +
> > > > > + * - ``sampling_frequency``
> > > > > + - Hz
> > > > > + - OSK ramp rate: SYSCLK / (4 * divider).
> > > > > +
> > > > > + * - ``pinctrl_en``
> > > > > + - boolean
> > > > > + - Enable manual external pin control. When enabled, the OSK pin directly
> > > > > + gates the output on/off instead of using the automatic ramp.
> > > >
> > > > I wonder if we should split the various OSK modes into different channels given
> > > > only some properties apply to each of automatic and manual modes. Also I think
> > > > automatic mode is meaningless without pinctrl_en (so that can be replaced
> > > > by simply enabling that mode). I have no idea if anyone cares about pin ctrl
> > > > with manual mode or not? That one seems even more odd.
> > >
> > > OSK is either in manual or auto:
> > > * In manual mode the OSK pin enables and disables the output based on its level.
> > > * In auto, the OSK pin controls the direction the amplitude updates.
> > >
> > > If we enable RAM mode, and other modes do not target amplitude, the only way to
> > > manually configure the amplitude in software (i.e. without using an OSK gpio)
> > > is going manual mode (scale_step == 0), disable this pinctrl_en and then set the
> > > scale property (ASF register). That is the only reason I added this property.
> >
> > Ah. Maybe we hide that away and make the amplitude a property of RAM channel?
>
> And what if a user is in fact willing to use the OSK pin?
Sigh. I don't have a good answer, but I don't like the weird 'special' nature
of this attribute. I guess sometimes there isn't a good answer to be found.
OSK + RAM is odd enough that I'm not that bothered if we have to go weird
here.
>
> > It can do this magic under the hood. I don't mind the attributes for OSK changing
> > if this trick is in use (they won't be active anyway).
>
> OSK has the highest priority of all, but it only acts on the amplitude.
>
> > > > > +
> > > > > + * - ``scale_step``
> > > > > + - fractional
> > > > > + - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
> > > > > + and sets the per-tick increment. Writing ``0`` disables it. Rounded to
> > > > > + nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
> > > >
> > > > Similar thing about rate of change of amplitude fitting better with current ABI
> > > > than step does.
> > >
> > > ok... and this one is still missing the correspondent available attr.
> >
> > Available is a bit tricky when there is an inverse relationship involved in the maths
> > as what do we put the step as. Maybe we should add a note on that to the ABI
> > docs. [min step max] where step gives the minimum step that due to non linearity
> > may not be applicable between discrete values that may be taken away from that
> > minimum granularity base value. If that occurs the driver will round to the
> > nearest possible value.
>
^ permalink raw reply
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
From: Jonathan Cameron @ 2026-05-05 17:09 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Nuno Sá, Rodrigo Alencar via B4 Relay, rodrigo.alencar,
linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <acpq4fyekssspgche7q6g5ns57y25alalooorybqfz2fzdr2yu@wj6seookj4ip>
On Mon, 27 Apr 2026 15:54:14 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/04/27 11:31AM, Nuno Sá wrote:
> > On Mon, Apr 27, 2026 at 10:46:08AM +0100, Jonathan Cameron wrote:
> > > On Sun, 26 Apr 2026 21:42:15 +0100
> > > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> > >
> > > > On 26/04/26 02:10PM, Jonathan Cameron wrote:
> > > > > On Fri, 17 Apr 2026 09:17:38 +0100
> > > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> > > > >
> > > > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > > >
> > > > > > Add documentation for the AD9910 DDS IIO driver, which describes channels,
> > > > > > DDS modes, attributes and ABI usage examples.
> > > > > >
> > > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > >
> > > > > Hi Rodrigo,
> > > > >
> > > > > I think this is getting close to something workable subject to some tweaks
> > > > > to not make the priority thing visible and use rate of change parameters
> > > > > so /Sec rather than steps.
> > > >
> > > > I am not sure about this one. Getting the value into units per seconds will
> > > > increase the range of values by a lot, e.g., for the frequency case the step
> > > > size can range from a few Hz up to the entire supported range (hundreds of
> > > > MHz), and if you consider that one would often have the sampling_frequency
> > > > at 250 MHz... an attribute frequency_roc could have an order of 10^17 Hz/s,
> > > > and I am not sure how practical is that, although it can have a physical meaning,
> > > > like a "chirp slope".
> > >
> > > That scaling is indeed a bit of a pain though it will go in a 64 bit int
> > > however, seems likely we'll get higher frequency devices one day that will
> > > limb even faster.
> > >
> > > Maybe wait and see if anyone else has input on this.
> >
> > If we think things like RF DACs (which internally - typically - make use
> > of things like DDS), we can already go to the GHz "world".
> >
> > Not saying we already have such a device that would map the new ABI with
> > bigger values (we might have but nothing I'm aware of from the top of my
> > head) but just saying the above is, indeed, very likely to pop up at some
> > time.
> >
> > Just something to bear in mind :)
> >
> > - Nuno Sá
>
> under this Documentation/ABI/testing/sysfs-bus-iio-impedance-analyzer-ad5933,
> there is ABI: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_increment
>
> which seems to be a similar concept to the step used here.
Yeah but staging driver so let's not focus on that too much.
>
^ permalink raw reply
* Re: [PATCH 7/8] drm/panthor: Add support for entering and exiting protected mode
From: Boris Brezillon @ 2026-05-05 17:11 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin, Paul Toadere,
Samuel Percival
In-Reply-To: <20260505140516.1372388-8-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:13 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> From: Florent Tomasin <florent.tomasin@arm.com>
>
> This patch modifies the Panthor driver code to allow handling
> of the GPU HW protected mode enter and exit.
>
> The logic added by this patch includes:
> - the mechanisms needed for entering and exiting protected mode.
> - the handling of protected mode IRQs and FW interactions.
> - the scheduler changes needed to decide when to enter
> protected mode based on CSG scheduling.
>
> Note that the submission of a protected mode jobs are done
> from the user space.
>
> The following is a summary of how protected mode is entered
> and exited:
> - When the GPU detects a protected mode job needs to be
> executed, an IRQ is sent to the CPU to notify the kernel
> driver that the job is blocked until the GPU has entered
> protected mode. The entering of protected mode is controlled
> by the kernel driver.
> - The Mali Panthor CSF driver will schedule a tick and evaluate
> which CS in the CSG to schedule on slot needs protected mode.
> If the priority of the CSG is not sufficiently high, the
> protected mode job will not progress until the CSG is
> scheduled at top priority.
> - The Panthor scheduler notifies the GPU that the blocked
> protected jobs will soon be able to progress.
> - Once all CSG and CS slots are updated, the scheduler
> requests the GPU to enter protected mode and waits for
> it to be acknowledged.
> - If successful, all protected mode jobs will resume execution
> while normal mode jobs block until the GPU exits
> protected mode, or the kernel driver rotates the CSGs
> and forces the GPU to exit protected mode.
> - If unsuccessful, the scheduler will request a GPU reset.
> - When a protected mode job is suspended as a result of
> the CSGs rotation, the GPU will send an IRQ to the CPU
> to notify that the protected mode job needs to resume.
>
> This sequence will continue so long the user space is
> submitting protected mode jobs.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> Co-developed-by: Paul Toadere <paul.toadere@arm.com>
> Signed-off-by: Paul Toadere <paul.toadere@arm.com>
> Co-developed-by: Samuel Percival <samuel.percival@arm.com>
> Signed-off-by: Samuel Percival <samuel.percival@arm.com>
> Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> ---
> drivers/gpu/drm/panthor/panthor_device.c | 1 +
> drivers/gpu/drm/panthor/panthor_device.h | 9 +
> drivers/gpu/drm/panthor/panthor_fw.c | 86 ++++++++-
> drivers/gpu/drm/panthor/panthor_fw.h | 5 +
> drivers/gpu/drm/panthor/panthor_gpu.c | 14 +-
> drivers/gpu/drm/panthor/panthor_gpu.h | 6 +
> drivers/gpu/drm/panthor/panthor_mmu.c | 10 +
> drivers/gpu/drm/panthor/panthor_sched.c | 224 +++++++++++++++++++++--
> 8 files changed, 339 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 3a5cdfa99e5fe..449f17b0f4c5c 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -207,6 +207,7 @@ int panthor_device_init(struct panthor_device *ptdev)
>
> ptdev->soc_data = of_device_get_match_data(ptdev->base.dev);
>
> + init_rwsem(&ptdev->protm.lock);
> init_completion(&ptdev->unplug.done);
> ret = drmm_mutex_init(&ptdev->base, &ptdev->unplug.lock);
> if (ret)
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index d51fec97fc5fa..ebeec45cf60a1 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -334,6 +334,15 @@ struct panthor_device {
> struct {
> /** @heap: Pointer to the protected heap */
> struct dma_heap *heap;
> +
> + /**
> + * @lock: Lock to prevent VM operations during protected mode.
Here it says the lock prevents VM ops while the GPU is in protected
mode, but...
> + *
> + * The MMU will not execute commands when the GPU is in
> + * protected mode, so we use this RW lock to sync access
> + * between VM_BIND and GPU protected mode.
> + */
> + struct rw_semaphore lock;
> } protm;
> };
>
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 1aba29b9779b6..281556530ddab 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -1057,7 +1057,9 @@ static void panthor_fw_init_global_iface(struct panthor_device *ptdev)
> GLB_CFG_PROGRESS_TIMER |
> GLB_CFG_POWEROFF_TIMER |
> GLB_IDLE_EN |
> - GLB_IDLE;
> + GLB_IDLE |
> + GLB_PROTM_ENTER |
> + GLB_PROTM_EXIT;
>
> if (panthor_fw_has_glb_state(ptdev))
> glb_iface->input->ack_irq_mask |= GLB_STATE_MASK;
> @@ -1456,6 +1458,88 @@ static void panthor_fw_ping_work(struct work_struct *work)
> }
> }
>
> +int panthor_fw_protm_enter(struct panthor_device *ptdev)
> +{
> + struct panthor_fw_global_iface *glb_iface;
> + u32 acked;
> + u32 status;
> + int ret;
> +
> + down_write(&ptdev->protm.lock);
> +
> + glb_iface = panthor_fw_get_glb_iface(ptdev);
> +
> + panthor_fw_toggle_reqs(glb_iface, req, ack, GLB_PROTM_ENTER);
> + gpu_write(ptdev, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1);
> +
> + ret = panthor_fw_glb_wait_acks(ptdev, GLB_PROTM_ENTER, &acked, 4000);
> + if (ret) {
> + drm_err(&ptdev->base, "Wait for FW protected mode acknowledge timed out");
> + up_write(&ptdev->protm.lock);
> + return ret;
> + }
> +
> + /* Wait for the GPU to actually enter protected mode.
> + * There would be some time gap between FW sending the
> + * ACK for GLB_PROTM_ENTER and GPU entering protected mode.
> + */
> + if (gpu_read_poll_timeout(ptdev, GPU_STATUS, status,
> + (status & GPU_STATUS_PROTM_ACTIVE) ||
> + ((glb_iface->input->req ^ glb_iface->output->ack) &
> + GLB_PROTM_EXIT),
> + 10, 500000)) {
> + drm_err(&ptdev->base, "Wait for GPU protected mode enter timed out");
> + ret = -ETIMEDOUT;
> + }
> +
> + up_write(&ptdev->protm.lock);
... here I see the lock being released right after we've entered
protected mode. Meaning the MMU layer can proceed with any pending VM
ops even though the GPU only exists PROTM when panthor_fw_protm_exit()
is called. If this is expected, the protm::lock doc should be updated to
reflect that.
Also, I don't think a rw_semaphore alone is enough to cover the kind of
critical section you're trying to declare, because it requires that the
lock is taken/released from the same thread, and
panthor_fw_protm_enter()/panthor_fw_protm_exit() will be called from
different work items. This probably explains why the doc no longer
matches the implementation.
I guess this could be reworked to use a combination of rwlock+completion,
where the VM logic does something like:
down_read(protm.lock);
while (!try_wait_for_completion(protm.complete)) {
up_read(protm.lock);
if (!wait_for_completion_timeout(protm.complete, timeout)) {
schedule_reset();
return -ETIMEDOUT;
}
down_read(protm.lock);
}
// proceed with the VM op
up_read(protm.lock);
in panthor_fw_protm_enter(), you'd take the lock in write mode,
reinit the completion object, release the lock, and proceed with
the PROTM operation. If it fails, you call complete_all()
immediately, if it works, you defer the complete_all() to the
panthor_fw_protm_exit() path.
> +
> + return ret;
> +}
> +
> +void panthor_fw_protm_exit(struct panthor_device *ptdev)
> +{
> + struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
> +
> + /* Acknowledge the protm exit. */
> + panthor_fw_update_reqs(glb_iface, req, glb_iface->output->ack, GLB_PROTM_EXIT);
> +}
> +
> +int panthor_fw_protm_exit_wait_event_timeout(struct panthor_device *ptdev)
> +{
> + struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
> + int ret = 0;
> +
> + /* Send PING request to force an exit */
> + panthor_fw_toggle_reqs(glb_iface, req, ack, GLB_PING);
Uh, if a PING triggers a PROTM exit, we should probably pause the PING
(or reschedule it) right before entering PROTM, otherwise timings might
make it so PROTM is exited almost immediately after enter.
> + gpu_write(ptdev, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1);
> +
> + ret = wait_event_timeout(ptdev->fw->req_waitqueue,
> + !(gpu_read(ptdev, GPU_STATUS) & GPU_STATUS_PROTM_ACTIVE),
> + msecs_to_jiffies(500));
> +
> + if (!ret) {
> + drm_err(&ptdev->base, "Wait for forced protected mode exit timed out");
> + panthor_device_schedule_reset(ptdev);
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +void panthor_fw_protm_exit_sync(struct panthor_device *ptdev)
> +{
> + u32 status;
> +
> + /* Busy-wait (5ms) for FW to exit protected mode on its own */
> + if (!gpu_read_poll_timeout(ptdev, GPU_STATUS, status,
> + !(status & GPU_STATUS_PROTM_ACTIVE), 10,
> + 5000))
> + return;
> +
> + panthor_fw_protm_exit_wait_event_timeout(ptdev);
> +}
I'll stop there for now.
Regards,
Boris
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Mathieu Poirier @ 2026-05-05 17:19 UTC (permalink / raw)
To: Arnaud POULIQUEN
Cc: Padhi, Beleswar, Shenwei Wang, Andrew Lunn, Linus Walleij,
Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Frank Li,
Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
dl-linux-imx, Bartosz Golaszewski
In-Reply-To: <f7ef3417-eb84-4467-ac72-a9bc8b0c81e8@foss.st.com>
On Thu, Apr 30, 2026 at 09:35:09AM +0200, Arnaud POULIQUEN wrote:
> Hello,
>
> On 4/29/26 21:20, Mathieu Poirier wrote:
> > On Wed, 29 Apr 2026 at 12:07, Padhi, Beleswar <b-padhi@ti.com> wrote:
> > >
> > > Hi Mathieu,
> > >
> > > On 4/29/2026 11:03 PM, Mathieu Poirier wrote:
> > > > On Wed, 29 Apr 2026 at 10:53, Shenwei Wang <shenwei.wang@nxp.com> wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Mathieu Poirier <mathieu.poirier@linaro.org>
> > > > > > Sent: Wednesday, April 29, 2026 10:42 AM
> > > > > > To: Shenwei Wang <shenwei.wang@nxp.com>
> > > > > > Cc: Andrew Lunn <andrew@lunn.ch>; Padhi, Beleswar <b-padhi@ti.com>; Linus
> > > > > > Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan
> > > > > > Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > > > > > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn Andersson
> > > > > > <andersson@kernel.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer
> > > > > > <s.hauer@pengutronix.de>; Shuah Khan <skhan@linuxfoundation.org>; linux-
> > > > > > gpio@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> > > > > > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
> > > > > > devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org;
> > > > > > imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-
> > > > > > imx@nxp.com>; Bartosz Golaszewski <brgl@bgdev.pl>
> > > > > > Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > > > > > On Tue, Apr 28, 2026 at 03:24:59PM +0000, Shenwei Wang wrote:
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > > > > > Sent: Monday, April 27, 2026 3:49 PM
> > > > > > > > To: Shenwei Wang <shenwei.wang@nxp.com>
> > > > > > > > Cc: Padhi, Beleswar <b-padhi@ti.com>; Linus Walleij
> > > > > > > > <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan
> > > > > > > > Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof
> > > > > > > > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> > > > > > > > Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> > > > > > > > <mathieu.poirier@linaro.org>; Frank Li <frank.li@nxp.com>; Sascha
> > > > > > > > Hauer <s.hauer@pengutronix.de>; Shuah Khan
> > > > > > > > <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> > > > > > > > doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix
> > > > > > > > Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> > > > > > > > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
> > > > > > > > devicetree@vger.kernel.org; linux- remoteproc@vger.kernel.org;
> > > > > > > > imx@lists.linux.dev; linux-arm- kernel@lists.infradead.org;
> > > > > > > > dl-linux-imx <linux-imx@nxp.com>; Bartosz Golaszewski
> > > > > > > > <brgl@bgdev.pl>
> > > > > > > > Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg
> > > > > > > > GPIO driver
> > > > > > > > > > struct virtio_gpio_response {
> > > > > > > > > > __u8 status;
> > > > > > > > > > __u8 value;
> > > > > > > > > > };
> > > > > > > > > It is the same message format. Please see the message definition
> > > > > > > > (GET_DIRECTION) below:
> > > > > > > >
> > > > > > > > > + +-----+-----+-----+-----+-----+----+
> > > > > > > > > + |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
> > > > > > > > > + | 1 | 2 |port |line | err | dir|
> > > > > > > > > + +-----+-----+-----+-----+-----+----+
> > > > > > > > Sorry, but i don't see how two u8 vs six u8 are the same message format.
> > > > > > > >
> > > > > > > Some changes to the message format are necessary.
> > > > > > >
> > > > > > > Virtio uses two communication channels (virtqueues): one for requests and
> > > > > > replies, and a second one for events.
> > > > > > > In contrast, rpmsg provides only a single communication channel, so a
> > > > > > > type field is required to distinguish between different kinds of messages.
> > > > > > >
> > > > > > > Since rpmsg replies and events share the same message format, an additional
> > > > > > line is introduced to handle both cases.
> > > > > > > Finally, rpmsg supports multiple GPIO controllers, so a port field is added to
> > > > > > uniquely identify the target controller.
> > > > > >
> > > > > > I have commented on this before - RPMSG is already providing multiplexing
> > > > > > capability by way of endpoints. There is no need for a port field. One endpoint,
> > > > > > one GPIO controller.
> > > > > >
> > > > > You still need a way to let the remote side know which port the endpoint maps to, either
> > > > > by embedding the port information in the message (the current way), or by sending it
> > > > > separately.
> > > > >
> > > > An endpoint is created with every namespace request. There should be
> > > > one namespace request for every GPIO controller, which yields a unique
> > > > endpoint for each controller and eliminates the need for an extra
> > > > field to identify them.
> > >
> > >
> > > Right, but this can still be done by just having one namespace request.
> > > We can create new endpoints bound to an existing namespace/channel by
> > > invoking rpmsg_create_ept(). This is what I suggested here too:
> > > https://lore.kernel.org/all/29485742-6e49-482e-b73d-228295daaeec@ti.com/
> > >
> >
> > I will look at your suggestion (i.e link above) later this week or next week.
> >
> > > My mental model looks like this for the complete picture:
> > >
> > > 1. namespace/channel#1 = rpmsg-io
> > > a. ept1 -> gpio-controller@1
> > > b. ept2 -> gpio-controller@2
> > >
> >
> > I've asked for one endpoint per GPIO controller since the very
> > beginning. I don't yet have a strong opinion on whether to use one
> > namespace request per GPIO controller or a single request that spins
> > off multiple endpoints. I'll have to look at your link and reflect on
> > that. Regardless of how we proceed on that front, multiplexing needs
> > to happen at the endpoint level rather than the packet level. This is
> > the only way this work can move forward.
> >
>
> I would be more in favor of Mathieu’s proposal: “An endpoint is created with
> every namespace request.”
>
> If the endpoint is created only on the Linux side, how do we match the Linux
> endpoint address with the local port field on the remote side?
>
> With a multi-namespace approach, the namespace could be rpmsg-io-[addr],
> where [addr] corresponds to the GPIO controller address in the DT. This
> would:
>
> - match the RPMsg probe with the DT,
> - provide a simple mapping between the port and the endpoint on both sides,
> - allow multiple endpoints on the remote side,
> - provide a simple discovery mechanism for remote capabilities.
>
This is exactly what I had in mind but I'll finish reading this thread before
expressing a final point of view. That said, the namespace announcement should
be "rpmsg-gpio-[addr]" rather than "rpmsg-io-[addr]" to make sure there is no
ambiguity on the meaning of "io".
More comments to come...
> Regards,
> Arnaud
>
> > > 2. namespace/channel#2 = rpmsg-i2c
> > > a. ept1 -> i2c@1
> > > b. ept2 -> i2c@2
> > > c. ept3 -> i2c@3
> > >
> > > etc...
> > >
> > > This way device groups are isolated with each channel/namespace, and
> > > instances within each device groups are also respected with specific
> > > endpoints.
> > >
> > > Thanks,
> > > Beleswar
> > >
> >
>
^ permalink raw reply
* [PATCH v17 01/11] PCI/AER: Introduce AER-CXL Kfifo
From: Terry Bowman @ 2026-05-05 17:30 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, djbw, bhelgaas,
shiju.jose, ming.li, Smita.KoralahalliChannabasappa, rrichter,
dan.carpenter, PradeepVineshReddy.Kodamati, lukas,
Benjamin.Cheatham, sathyanarayanan.kuppuswamy, vishal.l.verma,
alucerop, ira.weiny, corbet, rafael, xueshuai, linux-cxl
Cc: linux-kernel, linux-pci, linux-acpi, linux-doc, terry.bowman
In-Reply-To: <20260505173029.2718246-1-terry.bowman@amd.com>
CXL virtual hierarchy (VH) native RAS handling for CXL Port devices will be
added soon. This requires a notification mechanism for the AER driver to
share the AER interrupt with the CXL driver. The CXL drivers use the
notification to handle and log the CXL RAS errors.
Note, 'CXL protocol error' terminology refers to CXL VH and not CXL RCH
errors unless specifically noted going forward.
Introduce a new file in the AER driver to handle the CXL protocol
errors: pci/pcie/aer_cxl_vh.c.
Add a kfifo work queue to be used by the AER and CXL drivers. Multiple
AER IRQ worker threads can be running and enqueueing concurrently, so
include write path synchronization. Pack the kfifo, the spinlock, the
rwsem, and the work pointer into a single structure. Initialize the
kfifo with INIT_KFIFO() from a subsys_initcall so its mask, esize and
data fields are valid before any producer or consumer runs.
Add CXL work queue handler registration functions in the AER driver.
Export them so the CXL driver can assign or clear the work handler.
Introduce 'struct cxl_proto_err_work_data' to serve as the kfifo work
data. It contains a reference to the PCI error source device and the
error severity. The cxl_core driver uses this when dequeuing the work.
Introduce cxl_forward_error() to add a given CXL protocol error to a
work structure and push it onto the AER-CXL kfifo. This function takes
a pci_dev_get() on the source device. The kfifo consumer is responsible
for the matching pci_dev_put() after dequeue. On enqueue failure
cxl_forward_error() does the put itself.
Synchronize accesses to the work function pointer during registration,
deregistration, enqueue, and dequeue.
handle_error_source() is intentionally not changed here. The is_cxl_error()
switch that routes errors to cxl_forward_error() is added in a later patch
together with the kfifo consumer registration. This way the producer and
consumer land in the same commit, so CXL errors are not silently dropped
during bisect.
Also add MAINTAINERS entries for both drivers/pci/pcie/aer_cxl_vh.c
(new in this patch) and drivers/pci/pcie/aer_cxl_rch.c (already in tree
but previously unlisted) under the existing CXL entry. This way the CXL
maintainers are CC'd on changes to the AER-CXL bridging code.
Co-developed-by: Dan Williams <djbw@kernel.org>
Signed-off-by: Dan Williams <djbw@kernel.org>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
Changes in v16->v17:
- Reword "kfifo semaphore" to "kfifo spinlock" to match fifo_lock.
- Defer the handle_error_source() is_cxl_error() switch to the patch that
registers the kfifo consumer to keep each commit bisect-safe.
- Rename rwsema to rwsem
- Change CPER exports to use EXPORT_SYMBOL_FOR_MODULES.
- Add work cancel function.
- Replace kfifo_put() with kfifo_in_spinlocked() for multiple producers
- Add fifo_lock spinlock for concurrent producer serialisation
- Initialize the embedded kfifo with INIT_KFIFO() in a subsys_initcall so
kfifo->mask, ->esize and ->data are set before first use.
- Clear PCI_ERR_COR_STATUS in cxl_forward_error() before enqueue so the
device is acked for correctable events even when the consumer drops the
event. Uncorrectable status is left for cxl_do_recovery() to clear after
recovery completes, mirroring the AER core convention.
- WARN on double-registration in cxl_register_proto_err_work() to make an
unintended second consumer visible at runtime.
- Add direct rwsem.h, cleanup.h and workqueue.h includes for symbols used
in aer_cxl_vh.c
- Add MAINTAINERS entries for drivers/pci/pcie/aer_cxl_*.c
- Update message
Changes in v15->v16:
- Add pci_dev_put() and comment in pci_dev_get() (Dan)
- /rw_sema/rwsema/ (Dan)
- Split validation checks in cxl_forward_error() to allow
for meaningful reason in log (Terry)
- Shorten commit title to remove wordiness (Terry)
- Remove bitfield.h include, unnecessary. (Terry)
Changes in v14->v15:
- Moved pci_dev_get() call to this patch (Dave)
Changes in v13 -> v14:
- Replaced workqueue_types.h include with 'struct work_struct'
predeclaration (Bjorn)
- Update error message (Bjorn)
- Reordered 'struct cxl_proto_err_work_data' (Bjorn)
- Remove export of cxl_error_is_native() here (Bjorn)
Changes in v12->v13:
- Added Dave Jiang's review-by
- Update error message (Ben)
Changes in v11->v12:
- None
---
MAINTAINERS | 2 +
drivers/pci/pcie/Makefile | 1 +
drivers/pci/pcie/aer.c | 10 ---
drivers/pci/pcie/aer_cxl_vh.c | 142 ++++++++++++++++++++++++++++++++++
drivers/pci/pcie/portdrv.h | 4 +
include/linux/aer.h | 28 +++++++
6 files changed, 177 insertions(+), 10 deletions(-)
create mode 100644 drivers/pci/pcie/aer_cxl_vh.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 882214b0e7db..93d4e43bb90d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6433,6 +6433,8 @@ S: Maintained
F: Documentation/driver-api/cxl
F: Documentation/userspace-api/fwctl/fwctl-cxl.rst
F: drivers/cxl/
+F: drivers/pci/pcie/aer_cxl_rch.c
+F: drivers/pci/pcie/aer_cxl_vh.c
F: include/cxl/
F: include/uapi/linux/cxl_mem.h
F: tools/testing/cxl/
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index b0b43a18c304..62d3d3c69a5d 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o bwctrl.o
obj-y += aspm.o
obj-$(CONFIG_PCIEAER) += aer.o err.o tlp.o
obj-$(CONFIG_CXL_RAS) += aer_cxl_rch.o
+obj-$(CONFIG_CXL_RAS) += aer_cxl_vh.o
obj-$(CONFIG_PCIEAER_INJECT) += aer_inject.o
obj-$(CONFIG_PCIE_PME) += pme.o
obj-$(CONFIG_PCIE_DPC) += dpc.o
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index c4fd9c0b2a54..c5bce25df51c 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1150,16 +1150,6 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
*/
EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
-#ifdef CONFIG_CXL_RAS
-bool is_aer_internal_error(struct aer_err_info *info)
-{
- if (info->severity == AER_CORRECTABLE)
- return info->status & PCI_ERR_COR_INTERNAL;
-
- return info->status & PCI_ERR_UNC_INTN;
-}
-#endif
-
/**
* pci_aer_handle_error - handle logging error into an event log
* @dev: pointer to pci_dev data structure of error source device
diff --git a/drivers/pci/pcie/aer_cxl_vh.c b/drivers/pci/pcie/aer_cxl_vh.c
new file mode 100644
index 000000000000..c0fea2c2b9bc
--- /dev/null
+++ b/drivers/pci/pcie/aer_cxl_vh.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2026 AMD Corporation. All rights reserved. */
+
+#include <linux/aer.h>
+#include <linux/cleanup.h>
+#include <linux/init.h>
+#include <linux/kfifo.h>
+#include <linux/rwsem.h>
+#include <linux/workqueue.h>
+#include "../pci.h"
+#include "portdrv.h"
+
+#define CXL_ERROR_SOURCES_MAX 128
+
+struct cxl_proto_err_kfifo {
+ struct work_struct *work;
+ struct rw_semaphore rwsem;
+ spinlock_t fifo_lock;
+ DECLARE_KFIFO(fifo, struct cxl_proto_err_work_data,
+ CXL_ERROR_SOURCES_MAX);
+};
+
+static struct cxl_proto_err_kfifo cxl_proto_err_kfifo = {
+ .rwsem = __RWSEM_INITIALIZER(cxl_proto_err_kfifo.rwsem),
+ .fifo_lock = __SPIN_LOCK_UNLOCKED(cxl_proto_err_kfifo.fifo_lock),
+};
+
+static int __init cxl_proto_err_kfifo_init(void)
+{
+ INIT_KFIFO(cxl_proto_err_kfifo.fifo);
+ return 0;
+}
+subsys_initcall(cxl_proto_err_kfifo_init);
+
+bool is_aer_internal_error(struct aer_err_info *info)
+{
+ if (info->severity == AER_CORRECTABLE)
+ return info->status & PCI_ERR_COR_INTERNAL;
+
+ return info->status & PCI_ERR_UNC_INTN;
+}
+
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info)
+{
+ if (!info || !info->is_cxl)
+ return false;
+
+ if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
+ return false;
+
+ return is_aer_internal_error(info);
+}
+
+void cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info)
+{
+ struct cxl_proto_err_work_data wd = {
+ .severity = info->severity,
+ .pdev = pdev,
+ };
+
+ if (info->severity == AER_CORRECTABLE)
+ pci_write_config_dword(pdev, pdev->aer_cap + PCI_ERR_COR_STATUS,
+ info->status);
+
+ guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
+
+ if (!cxl_proto_err_kfifo.work) {
+ dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo reader not registered\n");
+ return;
+ }
+
+ /*
+ * Reference discipline: the AER caller (handle_error_source())
+ * holds a ref on @pdev for the duration of this call and releases
+ * it on return. Take a fresh ref here so the pdev stays live while
+ * queued in the kfifo; the consumer (for_each_cxl_proto_err())
+ * drops that ref after handling. On enqueue failure below, drop
+ * the ref we just took to avoid a leak.
+ */
+ pci_dev_get(pdev);
+
+ /* Serialize concurrent kfifo writers: multiple AER threaded IRQs */
+ if (!kfifo_in_spinlocked(&cxl_proto_err_kfifo.fifo, &wd, 1,
+ &cxl_proto_err_kfifo.fifo_lock)) {
+ dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo add failed\n");
+ pci_dev_put(pdev);
+ return;
+ }
+
+ schedule_work(cxl_proto_err_kfifo.work);
+}
+
+void cxl_register_proto_err_work(struct work_struct *work)
+{
+ guard(rwsem_write)(&cxl_proto_err_kfifo.rwsem);
+ WARN_ONCE(cxl_proto_err_kfifo.work,
+ "AER-CXL kfifo consumer already registered\n");
+ cxl_proto_err_kfifo.work = work;
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_register_proto_err_work, "cxl_core");
+
+static struct work_struct *cancel_cxl_proto_err(void)
+{
+ struct work_struct *work;
+ struct cxl_proto_err_work_data wd;
+
+ guard(rwsem_write)(&cxl_proto_err_kfifo.rwsem);
+ work = cxl_proto_err_kfifo.work;
+ cxl_proto_err_kfifo.work = NULL;
+ while (kfifo_get(&cxl_proto_err_kfifo.fifo, &wd)) {
+ dev_err_ratelimited(&wd.pdev->dev,
+ "AER-CXL error report canceled\n");
+ pci_dev_put(wd.pdev);
+ }
+ return work;
+}
+
+void cxl_unregister_proto_err_work(void)
+{
+ struct work_struct *work = cancel_cxl_proto_err();
+
+ if (work)
+ cancel_work_sync(work);
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_unregister_proto_err_work, "cxl_core");
+
+int for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+ cxl_proto_err_fn_t fn)
+{
+ int rc;
+
+ guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
+ while (kfifo_get(&cxl_proto_err_kfifo.fifo, wd)) {
+ rc = fn(wd);
+ pci_dev_put(wd->pdev);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index cc58bf2f2c84..66a6b8099c96 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -130,9 +130,13 @@ struct aer_err_info;
bool is_aer_internal_error(struct aer_err_info *info);
void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info);
void cxl_rch_enable_rcec(struct pci_dev *rcec);
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info);
+void cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info);
#else
static inline bool is_aer_internal_error(struct aer_err_info *info) { return false; }
static inline void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info) { }
static inline void cxl_rch_enable_rcec(struct pci_dev *rcec) { }
+static inline bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline void cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info) { }
#endif /* CONFIG_CXL_RAS */
#endif /* _PORTDRV_H_ */
diff --git a/include/linux/aer.h b/include/linux/aer.h
index df0f5c382286..78841cf4268c 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -25,6 +25,7 @@
#define PCIE_STD_MAX_TLP_HEADERLOG (PCIE_STD_NUM_TLP_HEADERLOG + 10)
struct pci_dev;
+struct work_struct;
struct pcie_tlp_log {
union {
@@ -53,6 +54,18 @@ struct aer_capability_regs {
u16 uncor_err_source;
};
+/**
+ * struct cxl_proto_err_work_data - Error information used in CXL error handling
+ * @pdev: PCI device detecting the error
+ * @severity: AER severity
+ */
+struct cxl_proto_err_work_data {
+ struct pci_dev *pdev;
+ int severity;
+};
+
+typedef int (*cxl_proto_err_fn_t)(struct cxl_proto_err_work_data *wd);
+
#if defined(CONFIG_PCIEAER)
int pci_aer_clear_nonfatal_status(struct pci_dev *dev);
int pcie_aer_is_native(struct pci_dev *dev);
@@ -66,6 +79,21 @@ static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
#endif
+#ifdef CONFIG_CXL_RAS
+void cxl_register_proto_err_work(struct work_struct *work);
+int for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+ cxl_proto_err_fn_t fn);
+void cxl_unregister_proto_err_work(void);
+#else
+static inline void cxl_register_proto_err_work(struct work_struct *work) { }
+static inline int for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+ cxl_proto_err_fn_t fn)
+{
+ return 0;
+}
+static inline void cxl_unregister_proto_err_work(void) { }
+#endif
+
void pci_print_aer(struct pci_dev *dev, int aer_severity,
struct aer_capability_regs *aer);
int cper_severity_to_aer(int cper_severity);
--
2.34.1
^ permalink raw reply related
* [PATCH v17 00/11] Enable CXL PCIe Port Protocol Error handling and logging
From: Terry Bowman @ 2026-05-05 17:30 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, djbw, bhelgaas,
shiju.jose, ming.li, Smita.KoralahalliChannabasappa, rrichter,
dan.carpenter, PradeepVineshReddy.Kodamati, lukas,
Benjamin.Cheatham, sathyanarayanan.kuppuswamy, vishal.l.verma,
alucerop, ira.weiny, corbet, rafael, xueshuai, linux-cxl
Cc: linux-kernel, linux-pci, linux-acpi, linux-doc, terry.bowman
This patch series enables CXL protocol error handling for both CXL Ports
and CXL Endpoints (EP). The previous revision is available at:
https://lore.kernel.org/linux-cxl/20260302203648.2886956-1-terry.bowman@amd.com/
Today the kernel handles native CXL.cachemem RAS only for Endpoints and
Restricted CXL Host (RCH) Downstream Ports. Root Ports, Upstream Switch
Ports, and Downstream Switch Ports are uncovered. This series introduces
a unified CXL protocol error path for all CXL device types, in both VH
and RCH topologies.
CXL protocol errors are layered as a distinct error plane on top of PCIe
AER. CXL RAS conditions are signaled as PCIe correctable (CE) and
uncorrectable (UCE) Internal AER Errors. The AER driver classifies these
events using pcie_is_cxl() and hands them off to cxl_core through the
AER-CXL kfifo.
The cxl_core driver dequeues each event, resolves the cxl_port topology,
and dispatches to the CE or UCE handler. RCD Endpoints are handled
slightly differently: the RCH Downstream Port's RAS state is processed
first, then the Endpoint's own RAS follows the common path.
PCIe AER errors remain a separate plane and are handled independently.
This series tightens the CXL Endpoint AER UCE handler and removes the
Endpoint AER CE handler, which is now redundant since the AER driver
clears and logs CE status itself.
PCI_ERS_RESULT_PANIC, introduced in earlier revisions, has been dropped.
The panic decision is made directly in cxl_do_recovery(): the kernel
panics on any uncorrectable CXL RAS error reported by cxl_handle_ras(),
or earlier on link disconnect.
A fatal UCE on an Upstream Switch Port or Endpoint surfaces through the
AER path rather than the CXL RAS path. USP devices are bound to the PCIe
portdrv driver, so when a USP reports a fatal UCE, the PCIe error
handler provided by portdrv is invoked. PCI config reads to the source
device are expected to fail in this scenario, so the AER core never
retrieves UNCOR_STATUS, and the event cannot be classified as CXL. See
the fatal and non-fatal log excerpts for USP and EP below.
The last patch is documentation with a diagram conveying the above.
== Patch Details ==
Patch 1 - PCI/AER: Introduce AER-CXL Kfifo
Adds the AER-CXL kfifo in drivers/pci/pcie/aer_cxl_vh.c along with the
producer helper cxl_forward_error() and the consumer registration
helpers cxl_register_proto_err_work() and cxl_unregister_proto_err_work().
The kfifo is intended to deliver CXL VH protocol errors from the AER
driver to cxl_core. The producer dispatch in handle_error_source() and
the consumer registration are added together in patch 6 so the path
goes live atomically.
Patch 2 - cxl/ras: Unify Endpoint and Port AER trace events
Unifies the Endpoint and Port AER trace events into a single set used by
all CXL devices on both the CPER and native AER paths. THIS IS AN ABI
CHANGE FOR USERSPACE TOOLS THAT KEY OFF THE OLD ENDPOINT TRACE FIELD
NAMES.
Patch 3 - cxl: Use common CPER handling for all CXL devices
Folds the Port and Endpoint paths in CPER protocol error handling into a
single code path.
Patch 4 - cxl: Rename find_cxl_port() to find_cxl_port_by_dport()
Renames find_cxl_port() to find_cxl_port_by_dport() to make the lookup
method explicit and consistent with the existing find_cxl_port_by_uport().
Both helpers remain static to core/port.c; patch 6 widens their scope
when it adds the first cross-file caller.
Patch 5 - cxl: Limit CXL-CPER kfifo registration functions scope
Limits the scope of the CXL-CPER kfifo registration functions and adds
work cancellation on unregister.
Patch 6 - PCI: Establish common CXL Port protocol error flow
Dequeues work from the AER-CXL kfifo and establishes a common flow for
all CXL Port protocol error handling. It directs CXL protocol errors to
correctable or uncorrectable handlers, and panics on any uncorrectable
CXL RAS error.
Patch 7 - PCI/CXL: Add RCH support to CXL handlers
Adds CXL1.1 Restricted CXL Host (RCH) support to the new common flow.
Behavior change: an RCD uncorrectable CXL RAS error now panics, matching
the policy applied to all other CXL devices. The RCH trace events also
change to report by PCI BDF instead of memdev name.
Patch 8 - cxl: Remove Endpoint AER correctable handler
Removes the CXL PCIe AER correctable handler. The handling and logging is
already performed by the AER driver and the new common flow.
Patch 9 - cxl: Update Endpoint AER uncorrectable handler
Replaces cxl_error_detected() with cxl_pci_error_detected(). The new
handler decides solely on the pci_channel_state_t parameter and does not
access PCIe AER capability registers from .error_detected, matching the
pattern used by NVMe, ixgbe, mlx5, and other modern PCIe drivers.
CXL.cachemem-corrupting protocol errors are routed separately through
the AER-CXL kfifo to cxl_handle_proto_error(), so cxl_pci does not need
to second-guess the AER core's classification.
Patch 10 - PCI/CXL: Mask/Unmask CXL protocol errors
Enables CXL Internal Error reporting on CXL Ports and Endpoints. The
unmask is paired with the RAS register block mapping in
cxl_dport_map_ras() and devm_cxl_port_ras_setup() so it only runs when
RAS registers were successfully mapped. The matching mask is registered
as a devres action and runs when the cxl_port device's devres is
released.
Patch 11 - Documentation: cxl: Document CXL protocol error handling
Adds Documentation/driver-api/cxl/linux/protocol-error-handling.rst
describing the end-to-end CXL protocol error path: AER ingress, the
AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special
cases, severity policy, trace events, and a source code map.
== Notes ==
- @Bjorn, I kindly request your review for the following patches. Many
of the changes are to CXL-specific files in the PCI tree:
Patch 1 - PCI/AER: Introduce AER-CXL Kfifo
Patch 6 - PCI: Establish common CXL Port protocol error flow
Patch 7 - PCI/CXL: Add RCH support to CXL handlers
Patch 10 - PCI/CXL: Mask/Unmask CXL protocol errors
- USP/EP fatal UCE follows the AER path because of how the AER core
collects status. aer_get_device_error_info() only reads
PCI_ERR_UNCOR_STATUS for Root Ports/RCECs/Downstream Ports or
non-fatal severities, where config reads to the source are still
expected to succeed. For a fatal UCE signaled by an upstream
component, config reads to that device are expected to fail, so
UNCOR_STATUS is never retrieved. Without the status word,
is_cxl_error() cannot classify the event as CXL and the AER path
handles it.
- Dan's related series addressing RAS setup has more details:
https://lore.kernel.org/linux-cxl/20260131000403.2135324-1-dan.j.williams@intel.com/
- TODOs for future series:
- Add Port support to CXL CLI injection (Ben)
- Move aer_cxl_rch.c to cxl/core/ras_rch.c
- Move RCH traversing for handling from AER driver into CXL driver
- Investigate if cxl_pci UCE error handler can be removed.
- Support user-defined status masks
- Add CXL Port traversing in cxl_do_recovery()
== Testing ==
Below are the testing results while using QEMU. The QEMU testing uses a CXL
Root Port, a CXL Upstream Switch Port, four CXL Downstream Switch Ports,
and four CXL Type 3 Endpoints as given below. I've attached the QEMU
startup commandline used. This testing uses protocol error injection at one
of each device class (Root Port, USP, DSP, Endpoint).
The test setup uses a modified aer-inject that supports CE and UCE
Internal Error injection. Because the QEMU CXL devices do not provide a
way to set the CXL RAS UNCOR_STATUS / COR_STATUS registers from
userspace, a separate out-of-tree debug patch ("test/cxl: Force RAS
status in cxl_handle_cor_ras() and cxl_handle_ras()") is applied on top
of the series for these tests, unconditionally OR'ing a Cache Data ECC
bit into the RAS status read. That debug patch is intentionally NOT part
of this v17 posting; reviewers reproducing the logs below in QEMU will
need it (or equivalent QEMU support) to see the CXL RAS trace events.
The sub-topology for the QEMU testing is:
---------------------
| CXL RP - 0C:00.0 |
---------------------
|
---------------------
| CXL USP - 0D:00.0 |
---------------------
|
+-------------+---------+---------+-------------+
| | | |
---------------- ---------------- ---------------- ----------------
|CXL DSP | |CXL DSP | |CXL DSP | |CXL DSP |
| 0E:00.0 | | 0E:01.0 | | 0E:02.0 | | 0E:03.0 |
---------------- ---------------- ---------------- ----------------
| | | |
---------------- ---------------- ---------------- ----------------
| CXL EP | | CXL EP | | CXL EP | | CXL EP |
| 0F:00.0 | | 10:00.0 | | 11:00.0 | | 12:00.0 |
---------------- ---------------- ---------------- ----------------
root@tbowman-cxl:~# lspci -t
-+-[0000:00]-+-00.0
| +-01.0
| +-02.0
| +-03.0
| +-1f.0
| +-1f.2
| \-1f.3
\-[0000:0c]---00.0-[0d-12]----00.0-[0e-12]--+-00.0-[0f]----00.0
+-01.0-[10]----00.0
+-02.0-[11]----00.0
\-03.0-[12]----00.0
=== Root Port - CE ===
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00004000/00000000 into device 0000:0c:00.0
pcieport 0000:0c:00.0: AER: Correctable error message received from 0000:0c:00.0
aer_event: 0000:0c:00.0 CXL Bus Error: severity=Corrected, Corrected Internal Error, TLP Header=Not available
pcieport 0000:0c:00.0: CXL Bus Error: severity=Correctable, type=Transaction Layer, (Receiver ID)
pcieport 0000:0c:00.0: device [8086:7075] error status/mask=00004000/0000a000
pcieport 0000:0c:00.0: [14] CorrIntErr
cxl_aer_correctable_error: device=0000:0c:00.0 host=pci0000:0c serial=0 status: 'Cache Data ECC Error'
== Root Port - UCE ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00000000/00400000 into device 0000:0c:00.0
pcieport 0000:0c:00.0: AER: Uncorrectable (Fatal) error message received from 0000:0c:00.0
aer_event: 0000:0c:00.0 CXL Bus Error: severity=Fatal, Uncorrectable Internal Error, TLP Header=Not available
pcieport 0000:0c:00.0: CXL Bus Error: severity=Uncorrectable (Fatal), type=Transaction Layer, (Receiver ID)
pcieport 0000:0c:00.0: device [8086:7075] error status/mask=00400000/02000000
pcieport 0000:0c:00.0: [22] UncorrIntErr
cxl_aer_uncorrectable_error: device=0000:0c:00.0 host=pci0000:0c serial=0 status: 'Cache Data Parity Error' first_error: 'Cache Data Parity Error'
Kernel panic - not syncing: CXL cachemem error.
CPU: 7 UID: 0 PID: 199 Comm: kworker/7:1 Not tainted 7.1.0-rc2-00011-g8cbf75633e69 #26 PREEMPT(lazy)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
Workqueue: events cxl_proto_err_work_fn [cxl_core]
Call Trace:
<TASK>
vpanic+0x453/0x4b0
panic+0x56/0x60
__cxl_proto_err_work_fn+0x265/0x280 [cxl_core]
? __pfx___cxl_proto_err_work_fn+0x10/0x10 [cxl_core]
for_each_cxl_proto_err+0x50/0x90
cxl_proto_err_work_fn+0x27/0x80 [cxl_core]
process_one_work+0x16e/0x3c0
worker_thread+0x172/0x2e0
? __pfx_worker_thread+0x10/0x10
kthread+0xe5/0x120
? __pfx_kthread+0x10/0x10
ret_from_fork+0x1bd/0x220
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Kernel Offset: disabled
---[ end Kernel panic - not syncing: CXL cachemem error. ]---
== Upstream Switch Port - CE ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00004000/00000000 into device 0000:0d:00.0
pcieport 0000:0c:00.0: AER: Correctable error message received from 0000:0d:00.0
aer_event: 0000:0d:00.0 CXL Bus Error: severity=Corrected, Corrected Internal Error, TLP Header=Not available
pcieport 0000:0d:00.0: CXL Bus Error: severity=Correctable, type=Transaction Layer, (Receiver ID)
pcieport 0000:0d:00.0: device [19e5:a128] error status/mask=00004000/0000a000
pcieport 0000:0d:00.0: [14] CorrIntErr
cxl_aer_correctable_error: device=0000:0d:00.0 host=0000:0c:00.0 serial=0 status: 'Cache Data ECC Error'
== Upstream Switch Port - UCE (fatal - AER recovery) ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00000000/00400000 into device 0000:0d:00.0
pcieport 0000:0c:00.0: AER: Uncorrectable (Fatal) error message received from 0000:0d:00.0
aer_event: 0000:0d:00.0 CXL Bus Error: severity=Fatal, , TLP Header=Not available
pcieport 0000:0d:00.0: AER: CXL Bus Error: severity=Uncorrectable (Fatal), type=Inaccessible, (Unregistered Agent ID)
cxl_pci 0000:0f:00.0: mem1: frozen state error detected, disable CXL.mem
cxl_pci 0000:10:00.0: mem0: frozen state error detected, disable CXL.mem
cxl_pci 0000:11:00.0: mem3: frozen state error detected, disable CXL.mem
cxl_pci 0000:12:00.0: mem2: frozen state error detected, disable CXL.mem
pcieport 0000:0c:00.0: AER: Root Port link has been reset (0)
cxl_pci 0000:0f:00.0: mem1: restart CXL.mem after slot reset
cxl_pci 0000:10:00.0: mem0: restart CXL.mem after slot reset
cxl_pci 0000:11:00.0: mem3: restart CXL.mem after slot reset
cxl_pci 0000:12:00.0: mem2: restart CXL.mem after slot reset
cxl_pci 0000:0f:00.0: mem1: error resume successful
cxl_pci 0000:10:00.0: mem0: error resume successful
cxl_pci 0000:11:00.0: mem3: error resume successful
cxl_pci 0000:12:00.0: mem2: error resume successful
pcieport 0000:0c:00.0: AER: device recovery successful
== Upstream Switch Port - UCE (non-fatal - panic) ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00000000/00400000 into device 0000:0d:00.0
pcieport 0000:0c:00.0: AER: Uncorrectable (Non-Fatal) error message received from 0000:0d:00.0
aer_event: 0000:0d:00.0 CXL Bus Error: severity=Uncorrected, non-fatal, Uncorrectable Internal Error, TLP Header=Not available
pcieport 0000:0d:00.0: CXL Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
pcieport 0000:0d:00.0: device [19e5:a128] error status/mask=00400000/02000000
pcieport 0000:0d:00.0: [22] UncorrIntErr
cxl_aer_uncorrectable_error: device=0000:0d:00.0 host=0000:0c:00.0 serial=0 status: 'Cache Data Parity Error' first_error: 'Cache Data Parity Error'
Kernel panic - not syncing: CXL cachemem error.
CPU: 7 UID: 0 PID: 54 Comm: kworker/7:0 Not tainted 7.1.0-rc2-00011-g8cbf75633e69 #26 PREEMPT(lazy)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
Workqueue: events cxl_proto_err_work_fn [cxl_core]
Call Trace:
<TASK>
vpanic+0x453/0x4b0
panic+0x56/0x60
__cxl_proto_err_work_fn+0x265/0x280 [cxl_core]
? __pfx___cxl_proto_err_work_fn+0x10/0x10 [cxl_core]
for_each_cxl_proto_err+0x50/0x90
cxl_proto_err_work_fn+0x27/0x80 [cxl_core]
process_one_work+0x16e/0x3c0
worker_thread+0x172/0x2e0
? __pfx_worker_thread+0x10/0x10
kthread+0xe5/0x120
? __pfx_kthread+0x10/0x10
ret_from_fork+0x1bd/0x220
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Kernel Offset: disabled
---[ end Kernel panic - not syncing: CXL cachemem error. ]---
== Downstream Switch Port - CE ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00004000/00000000 into device 0000:0e:00.0
pcieport 0000:0c:00.0: AER: Correctable error message received from 0000:0e:00.0
aer_event: 0000:0e:00.0 CXL Bus Error: severity=Corrected, Corrected Internal Error, TLP Header=Not available
pcieport 0000:0e:00.0: CXL Bus Error: severity=Correctable, type=Transaction Layer, (Receiver ID)
pcieport 0000:0e:00.0: device [19e5:a129] error status/mask=00004000/0000a000
pcieport 0000:0e:00.0: [14] CorrIntErr
cxl_aer_correctable_error: device=0000:0e:00.0 host=0000:0d:00.0 serial=0 status: 'Cache Data ECC Error'
== Downstream Switch Port - UCE (fatal) ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00000000/00400000 into device 0000:0e:00.0
pcieport 0000:0c:00.0: AER: Uncorrectable (Fatal) error message received from 0000:0e:00.0
aer_event: 0000:0e:00.0 CXL Bus Error: severity=Fatal, Uncorrectable Internal Error, TLP Header=Not available
pcieport 0000:0e:00.0: CXL Bus Error: severity=Uncorrectable (Fatal), type=Transaction Layer, (Receiver ID)
pcieport 0000:0e:00.0: device [19e5:a129] error status/mask=00400000/02000000
pcieport 0000:0e:00.0: [22] UncorrIntErr
cxl_aer_uncorrectable_error: device=0000:0e:00.0 host=0000:0d:00.0 serial=0 status: 'Cache Data Parity Error' first_error: 'Cache Data Parity Error'
Kernel panic - not syncing: CXL cachemem error.
CPU: 7 UID: 0 PID: 211 Comm: kworker/7:1 Not tainted 7.1.0-rc2-00011-g8cbf75633e69 #26 PREEMPT(lazy)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
Workqueue: events cxl_proto_err_work_fn [cxl_core]
Call Trace:
<TASK>
vpanic+0x453/0x4b0
panic+0x56/0x60
__cxl_proto_err_work_fn+0x265/0x280 [cxl_core]
? __pfx___cxl_proto_err_work_fn+0x10/0x10 [cxl_core]
for_each_cxl_proto_err+0x50/0x90
cxl_proto_err_work_fn+0x27/0x80 [cxl_core]
process_one_work+0x16e/0x3c0
worker_thread+0x172/0x2e0
? __pfx_worker_thread+0x10/0x10
kthread+0xe5/0x120
? __pfx_kthread+0x10/0x10
ret_from_fork+0x1bd/0x220
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Kernel Offset: disabled
---[ end Kernel panic - not syncing: CXL cachemem error. ]---
== Endpoint - CE ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00004000/00000000 into device 0000:0f:00.0
pcieport 0000:0c:00.0: AER: Correctable error message received from 0000:0f:00.0
aer_event: 0000:0f:00.0 CXL Bus Error: severity=Corrected, Corrected Internal Error, TLP Header=Not available
cxl_pci 0000:0f:00.0: CXL Bus Error: severity=Correctable, type=Transaction Layer, (Receiver ID)
cxl_pci 0000:0f:00.0: device [8086:0d93] error status/mask=00004000/0000a000
cxl_pci 0000:0f:00.0: [14] CorrIntErr
cxl_aer_correctable_error: device=0000:0f:00.0 host=0000:0e:00.0 serial=0 status: 'Cache Data ECC Error'
== Endpoint - UCE (fatal - AER recovery) ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00000000/00400000 into device 0000:0f:00.0
pcieport 0000:0c:00.0: AER: Uncorrectable (Fatal) error message received from 0000:0f:00.0
aer_event: 0000:0f:00.0 CXL Bus Error: severity=Fatal, , TLP Header=Not available
cxl_pci 0000:0f:00.0: AER: CXL Bus Error: severity=Uncorrectable (Fatal), type=Inaccessible, (Unregistered Agent ID)
cxl_pci 0000:0f:00.0: mem0: frozen state error detected, disable CXL.mem
pcieport 0000:0e:00.0: AER: Downstream Port link has been reset (0)
cxl_pci 0000:0f:00.0: mem0: restart CXL.mem after slot reset
cxl_pci 0000:0f:00.0: mem0: error resume successful
pcieport 0000:0e:00.0: AER: device recovery successful
== Endpoint - UCE (non-fatal) ==
pcieport 0000:0c:00.0: aer_inject: Injecting errors 00000000/00400000 into device 0000:0f:00.0
pcieport 0000:0c:00.0: AER: Uncorrectable (Non-Fatal) error message received from 0000:0f:00.0
aer_event: 0000:0f:00.0 CXL Bus Error: severity=Uncorrected, non-fatal, Uncorrectable Internal Error, TLP Header=Not available
cxl_pci 0000:0f:00.0: CXL Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
cxl_pci 0000:0f:00.0: device [8086:0d93] error status/mask=00400000/02000000
cxl_pci 0000:0f:00.0: [22] UncorrIntErr
cxl_aer_uncorrectable_error: device=0000:0f:00.0 host=0000:0e:00.0 serial=0 status: 'Cache Data Parity Error' first_error: 'Cache Data Parity Error'
Kernel panic - not syncing: CXL cachemem error.
CPU: 7 UID: 0 PID: 201 Comm: kworker/7:1 Not tainted 7.1.0-rc2-00011-g8cbf75633e69 #26 PREEMPT(lazy)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
Workqueue: events cxl_proto_err_work_fn [cxl_core]
Call Trace:
<TASK>
vpanic+0x453/0x4b0
panic+0x56/0x60
__cxl_proto_err_work_fn+0x265/0x280 [cxl_core]
? __pfx___cxl_proto_err_work_fn+0x10/0x10 [cxl_core]
for_each_cxl_proto_err+0x50/0x90
cxl_proto_err_work_fn+0x27/0x80 [cxl_core]
process_one_work+0x16e/0x3c0
worker_thread+0x172/0x2e0
? __pfx_worker_thread+0x10/0x10
kthread+0xe5/0x120
? __pfx_kthread+0x10/0x10
ret_from_fork+0x1bd/0x220
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Kernel Offset: disabled
---[ end Kernel panic - not syncing: CXL cachemem error. ]---
== Changes ==
Changes in v16->v17:
PCI/AER: Introduce AER-CXL Kfifo
- Reword "kfifo semaphore" to "kfifo spinlock" to match fifo_lock.
- Defer the handle_error_source() is_cxl_error() switch to the patch that
registers the kfifo consumer to keep each commit bisect-safe.
- Rename rwsema to rwsem
- Change CPER exports to use EXPORT_SYMBOL_FOR_MODULES.
- Add work cancel function.
- Replace kfifo_put() with kfifo_in_spinlocked() for multiple producers
- Add fifo_lock spinlock for concurrent producer serialisation
- Initialize the embedded kfifo with INIT_KFIFO() in a subsys_initcall so
kfifo->mask, ->esize and ->data are set before first use.
- Clear PCI_ERR_COR_STATUS in cxl_forward_error() before enqueue so the
device is acked for correctable events even when the consumer drops the
event. Uncorrectable status is left for cxl_do_recovery() to clear after
recovery completes, mirroring the AER core convention.
- WARN on double-registration in cxl_register_proto_err_work() to make an
unintended second consumer visible at runtime.
- Add direct rwsem.h, cleanup.h and workqueue.h includes for symbols used
in aer_cxl_vh.c
- Add MAINTAINERS entries for drivers/pci/pcie/aer_cxl_*.c
- Update message
cxl/ras: Unify Endpoint and Port AER trace events
- Replace cxlds->serial with pci_get_dsn()
- Change 'memdev' to 'device' (Dan)
- Updated Commit message
cxl: Use common CPER handling for all CXL devices
- New commit
cxl: Rename find_cxl_port() to find_cxl_port_by_dport()
- New commit
- Drop the de-staticisation of find_cxl_port_by_uport() and the
core.h declarations from this prep patch; both move to the patch
that introduces the first cross-file caller.
cxl: Limit CXL-CPER kfifo registration functions scope
- Split from v16 02/10 ("Update unregistration for AER-CXL and
CPER-CXL kfifos"); AER-CXL half folded into v17 01/10.
- Convert exports to EXPORT_SYMBOL_FOR_MODULES("cxl_core").
- Change register/unregister return type from int to void.
- Drop work_struct argument from cxl_cper_unregister_prot_err_work();
it now cancels its own work.
- Remove now-redundant cancel_work_sync() from cxl_ras_exit().
- Add WARN_ONCE() in cxl_cper_register_prot_err_work() for
double-registration.
PCI: Establish common CXL Port protocol error flow
- get_cxl_port() -> find_cxl_port_by_dev()
- Simplified find_cxl_port_by_dev()
- Replace and remove cxl_serial_number() w/ pci_get_dsn()
- cxl_get_ras_base() -> to_ras_base()
- Drop dependency on PCI_ERS_RESULT_PANIC; cxl_do_recovery() panics
directly. (PANIC enum patch dropped from series.)
- Clarify panic semantics: panic on any uncorrectable CXL RAS error, not
only AER-FATAL severities.
- Drop the redundant PCI_ERR_COR_STATUS RMW in cxl_handle_proto_error();
cxl_forward_error() already acks the correctable AER status.
- Add is_cxl_error() switch in handle_error_source() here, paired with the
kfifo consumer registration, to keep each commit bisect-safe.
- Drop pcie_aer_is_native() guard in cxl_do_recovery() (always native).
- Swap order with the "Limit" patch for bisectability w/ cxl_ras_exit()
- Reword for "any uncorrectable" CXL RAS error panics.
- Restore log messages for port-not-found and port-unbound cases.
- Whitespace cleanup (Jonathan)
- Update to get_cxl_port() documentation (Terry)
- Fix __cxl_proto_err_work_fn() to return 0 for transient errors.
- Drop !port check in cxl_do_recovery(), caller already validated
- Fix kerneldoc @pdev -> @dev in find_cxl_port_by_dev()
- Fix missing space in pr_err_ratelimited()
- Add disconnect check before access
- Made pcie_clear_device_status() and pci_aer_clear_fatal_status()
EXPORT_SYMBOL_FOR_MODULES("cxl_core") (Dan)
- Move find_cxl_port_by_dport() and find_cxl_port_by_uport()
de-staticisation and core.h declarations from the rename patch to
here, where the first cross-file callers in find_cxl_port_by_dev()
land.
PCI/CXL: Add RCH support to CXL handlers
- Drop now-dead cxlds->rcd branches from cxl_{cor_,}error_detected().
- Drop duplicate subject line from commit body.
- Document panic-on-uncorrectable behavior change for RCD path.
- Document trace event device-name change (memN -> PCI BDF) for RCH path.
- Rewrite cxl_handle_proto_error() RC_END comment to clarify RCD/RCH shared
interrupt relationship
- Rewrite commit message
cxl: Remove Endpoint AER correctable handler
- Update commit message
- Add Reviewed-by from Jonathan and DaveJ
cxl: Update Endpoint AER uncorrectable handler
- Rename pci_error_handlers struct instance to cxl_pci_error_handlers to
avoid shadowing the struct type tag.
- Restore scoped_guard(device) and dev->driver check around AER read.
- NULL-check find_cxl_port_by_dev() before deref of port->uport_dev.
- Updated commit message. (Terry)
- Add scope cleanup for port variable in cxl_pci_error_detected() (Terry)
- Drop cxl_uncor_aer_present(), rely on AER state
PCI/CXL: Mask/Unmask CXL protocol errors
- Drop redundant cxl_mask_proto_interrupts() calls from unregister_port()
and cxl_dport_remove(); the devres action registered alongside the unmask
is the sole mask path.
- Update title
- Remove unnecessary check for aer_capabilities
- Gate cxl_unmask_proto_interrupts() on pcie_aer_is_native()
- Add pci_aer_mask_internal_errors() and cxl_mask_proto_interrupts()
- Only unmask on successful cxl_map_component_regs()
- NULL-check @dev in cxl_{un,}mask_proto_interrupts()
- Drop static and declare in core/core.h
Documentation: cxl: Document CXL protocol error handling
- New commit
Changes in v15->v16:
PCI/AER: Introduce AER-CXL Kfifo
- Add pci_dev_put() and comment at pci_dev_get() (Dan)
- /rw_sema/rwsema/ (Dan)
- Split validation checks in cxl_forward_error() to allow
for meaningful reason in log (Terry)
- Shortened commit title to remove wordiness (Terry)
PCI/CXL: Update unregistration for AER-CXL and CPER-CXL kfifos
- New commit
cxl: Update CXL Endpoint tracing
- Add Dan's review-by
- Incorporate Dan's comment into commit message:
"Add the serial number at the end to preserve compatibility with
libtraceevent parsing of the parameters."
PCI/ERR: Introduce PCI_ERS_RESULT_PANIC
- None
PCI: Establish common CXL Port protocol error flow
- get_ras_base(), initialize dport to NULL (Jonathan)
- Remove guard(device)(&cxlmd->dev) (Jonathan)
- Fix dev_warns() (Jonathan)
- Remove comment in cxl_port_error_detected() (Dan)
- Made pcie_clear_device_status() and pci_aer_clear_fatal_status()
"CXL" Export namespace (Dan)
- Update switch-case brackets to follow clang-format (Dan)
- Add PCI_EXP_TYPE_RC_END for cxl_get_ras_base() (Terry)
- Add NULL port check in cxl_serial_number() (Terry)
PCI/CXL: Add RCH support to CXL handlers
- New commit
cxl: Update error handlers to support CXL Port devices
- None
cxl: Update Endpoint AER uncorrectable handler
- Update commit message (DaveJ)
- s/cxl_handle_aer()/cxl_uncor_aer_present()/g (Jonathan)
- cxl_uncor_aer_present(): Leave original result calculation based on
if a UCE is present and the provided state (Terry)
- Add call to pci_print_aer(). AER fails to log because is upstream
link (Terry)
cxl: Remove Endpoint AER correctable handler
- None
cxl: Enable CXL protocol error reporting
- None
Changes in v14->v15:
PCI/AER: Introduce AER-CXL Kfifo in new file, pcie/aer_cxl_vh.c
- Move pci_dev_get() call to this patch (Dave)
cxl: Update CXL Endpoint tracing
- Update commit message
- Moved cxl_handle_ras/cxl_handle_cor_ras() changes to future patch (Terry)
PCI/ERR: Introduce PCI_ERS_RESULT_PANIC
- None
PCI/AER: Dequeue forwarded CXL error
- Move pci_dev_get() to cxl_forward_error() (Dave)
- Move in is_cxl_error() change from later patch (Terry)
PCI: Establish common CXL Port protocol error flow
- Update commit message and title. Added Bjorn's ack.
- Move CE and UCE handling logic here (Terry)
cxl: Update error handlers to support CXL Port protocol errors
- New commit (Terry)
cxl: Update Endpoint AER uncorrectable handler
- Title update (Terry)
- Change cxl_pci_error-detected() to handle & log AER (Terry)
- Update commit message (Terry)
- Moved cxl_handle_ras()/cxl_handle_cor_ras() to earlier patch (Terry)
cxl: Remove Endpoint AER correctable handler
- Remove cxl_pci_cor_error_detected(). Is not needed. AER is logged
in the AER driver. (Dan)
- Update commit message (Terry)
cxl: Enable CXL protocol error reporting
- Update commit title's prefix (Bjorn)
Changes in v13->v14:
PCI: Move CXL DVSEC definitions into uapi/linux/pci_regs.h
- Add Jonathan's and Dan's review-by
- Update commit title prefix (Bjorn)
- Revert format fix for cxl_sbr_masked() (Jonathan)
- Update 'Compute Express Link' comment block (Jonathan)
- Move PCI_DVSEC_CXL_FLEXBUS definitions to later patch where
used (Jonathan)
- Removed stray change (Bjorn)
PCI: Update CXL DVSEC definitions
- New patch. Split from previous patch such that there is now a separate
move patch and a format fix patch.
- Formatting update requested (Bjorn)
- Remove PCI_DVSEC_HEADER1_LENGTH_MASK because it duplicates
PCI_DVSEC_HEADER1_LEN() (Bjorn)
- Add Dan's review-by
PCI: Introduce pcie_is_cxl()
- Move FLEXBUS_STATUS DVSEC here (Jonathan)
- Remove check for EP and USP (Dan)
- Update commit message (Bjorn)
- Fix writing past 80 columns (Bjorn)
- Add pci_is_pcie() parent bridge check at beginning of function (Bjorn)
PCI: Replace cxl_error_is_native() with pcie_aer_is_native()
- New commit
cxl/pci: Move CXL driver's RCH error handling into core/ras_rch.c
- Add sign-off for Dan and Jonathan
- Revert inadvertent formatting of cxl_dport_map_rch_aer() (Jonathan)
- Remove default value for CXL_RCH_RAS (Dan)
- Remove unnecessary pci.h include in core.h & ras_rch.c (Jonathan)
- Add linux/types.h include in ras_rch.c (Jonathan)
- Change CONFIG_CXL_RCH_RAS -> CONFIG_CXL_RAS (Dan)
PCI/AER: Export pci_aer_unmask_internal_errors
- New commit. Bjorn requested separating out and adding immediatetly
before being used. This is called from cxl_rch_enable_rcec() in
following patch.
PCI/AER: Update is_internal_error() to be non-static is_aer_internal_error()
- New commit
PCI/AER: Move CXL RCH error handling to aer_cxl_rch.c
- Add review-by and signed-off for Dan
- Commit message fixup (Dan)
- Update commit message with use-case description (Dan, Lukas)
- Make cxl_error_is_native() static (Dan)
- Make is_internal_error() non-static, non-export (Terry)
PCI/AER: Use guard() in cxl_rch_handle_error_iter()
- Add review-by for Jonathan, Dave Jiang, Dan WIlliams, and Bjorn
- Remove cleanup.h (Jonathan)
- Reverted comment removal (Bjorn)
- Move this patch after pci/pcie/aer_cxl_rch.c creation (Bjorn)
PCI/AER: Replace PCIEAER_CXL symbol with CXL_RAS
- New commit
PCI/AER: Report CXL or PCIe bus type in AER trace logging
- Merged with Dan's commit. Changes are moving bus_type the last
parameter in function calls (Dan)
- Removed all DCOs because of changes (Terry)
- Update commit message (Bjorn)
- Add Bjorn's ack-by
PCI/AER: Update struct aer_err_info with kernel-doc formatting
- New commit
cxl/mem: Clarify @host for devm_cxl_add_nvdimm()
- New commit
cxl/port: Remove "enumerate dports" helpers
- New commit
cxl/port: Fix devm resource leaks around with dport management
- New commit
cxl/port: Move dport operations to a driver event
- New commit
cxl/port: Move dport RAS reporting to a port resource
- New commit
cxl: Map CXL Endpoint Port and CXL Switch Port RAS registers
- Correct message spelling (Terry)
cxl/port: Move endpoint component register management to cxl_port
- Correct message spelling (Terry)
cxl/port: Map Port component registers before switchport init
- Updates to use cxl_port_setup_regs() (Dan)
cxl: Change CXL handlers to use guard() instead of scoped_guard()
- Add reviewed-by for Jonathan and Dave Jiang
PCI/ERR: Introduce PCI_ERS_RESULT_PANIC
- Add review-by for Dan
- Update Title prefix (Bjorn)
- Removed merge_result. Only logging error for device reporting the
error (Dan)
- Remove PCI_ERS_RESULT_PANIC paragraph in pci-error-recovery.rst (Bjorn)
PCI/AER: Move AER driver's CXL VH handling to pcie/aer_cxl_vh.c
- Replaced workqueue_types.h include with 'struct work_struct'
predeclaration (Bjorn)
- Update error message (Bjorn)
- Reordered 'struct cxl_proto_err_work_data' (Bjorn)
- Remove export of cxl_error_is_native() here (Bjorn)
cxl/port: Unify endpoint and switch port lookup
- New patch
PCI/AER: Dequeue forwarded CXL error
- Update commit title's prefix (Bjorn)
- Add pdev ref get in AER driver before enqueue and add pdev ref put in
CXL driver after dequeue and handling (Dan)
- Removed handling to simplify patch context (Terry)
PCI: Introduce CXL Port protocol error handlers
- Add Dave Jiang's review-by
- Update commit message & headline (Bjorn)
- Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
one line (Jonathan)
- Remove cxl_walk_port(). Only log the erroring device. No port walking. (Dan)
- Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
sufficient (Dan)
- Remove device_lock_if()
- Combine CE and UCE here (Terry)
cxl: Update Endpoint uncorrectable protocol error handling
- Update commit headline (Bjorn)
- Rename pci_error_detected()/pci_cor_error_detected() ->
cxl_pci_error_detected/cxl_pci_cor_error_detected() (Jonathan)
- Remove now-invalid comment in cxl_error_detected() (Jonathan)
- Split into separate patches for UCE and CE (Terry)
cxl: Update Endpoint correctable protocol error handling
- New commit
- Change cxl_cor_error_detected() parameter to &pdev->dev device from
memdev device. (Terry)
cxl: Enable CXL protocol errors during CXL Port probe
- Update commit title's prefix (Bjorn)
Changes in v12->v13:
CXL/PCI: Move CXL DVSEC definitions into uapi/linux/pci_regs.h
- Add Dave Jiang's reviewed-by
- Remove changes to existing PCI_DVSEC_CXL_PORT* defines. Update commit
message. (Jonathan)
PCI/CXL: Introduce pcie_is_cxl()
- Add Ben's "reviewed-by"
cxl/pci: Remove unnecessary CXL Endpoint handling helper functions
- None
cxl/pci: Remove unnecessary CXL RCH handling helper functions
- None
cxl: Remove CXL VH handling in CONFIG_PCIEAER_CXL conditional blocks from core
- None
cxl: Move CXL driver's RCH error handling into core/ras_rch.c
- None
CXL/AER: Replace device_lock() in cxl_rch_handle_error_iter() with guard() lock
- New patch
CXL/AER: Move AER drivers RCH error handling into pcie/aer_cxl_rch.c
- Add forward declararation of 'struct aer_err_info' in pci/pci.h (Terry)
- Changed copyright date from 2025 to 2023 (Jonathan)
- Add David Jiang's, Jonathan's, and Ben's review-by
- Readd 'struct aer_err_info' (Bot)
PCI/AER: Report CXL or PCIe bus error type in trace logging
- Remove duplicated aer_err_info inline comments. Is already in the
kernel-doc header (Ben)
cxl/pci: Update RAS handler interfaces to also support CXL Ports
- None
cxl/pci: Log message if RAS registers are unmapped
- Added Bens review-by
cxl/pci: Unify CXL trace logging for CXL Endpoints and CXL Ports
- Added Dave Jiang's review-by
cxl/pci: Update cxl_handle_cor_ras() to return early if no RAS errors
- Add Ben's review-by
cxl/pci: Map CXL Endpoint Port and CXL Switch Port RAS registers
- Change as result of dport delay fix. No longer need switchport and
endport approach. Refactor. (Terry)
CXL/PCI: Introduce PCI_ERS_RESULT_PANIC
- Add Dave Jiang's, Jonathan's, Ben's review-by
- Typo fix (Ben)
CXL/AER: Introduce pcie/aer_cxl_vh.c in AER driver for forwarding CXL errors
- Add Dave Jiang's review-by
- Update error message (Ben)
cxl: Introduce cxl_pci_drv_bound() to check for bound driver
- Add Dave Jiang's review-by.
cxl: Change CXL handlers to use guard() instead of scoped_guard()
- New patch
cxl/pci: Introduce CXL protocol error handlers for endpoints
- Updated all the implemetnation and commit message. (Terry)
- Refactored cxl_cor_error_detected()/cxl_error_detected() to remove
pdev (Dave Jiang)
CXL/PCI: Introduce CXL Port protocol error handlers
- Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
patch (Terry)
- Remove EP case in cxl_get_ras_base(), not used. (Terry)
- Remove check for dport->dport_dev (Dave)
- Remove whitespace (Terry)
PCI/AER: Dequeue forwarded CXL error
- Rewrite cxl_handle_proto_error() and cxl_proto_err_work_fn() (Terry)
- Rename get_cxl_host dev() to be get_cxl_port() (Terry)
- Remove exporting of unused function, pci_aer_clear_fatal_status() (Dave Jiang)
- Change pr_err() calls to ratelimited. (Terry)
- Update commit message. (Terry)
- Remove namespace qualifier from pcie_clear_device_status()
export (Dave Jiang)
- Move locks into cxl_proto_err_work_fn() (Dave)
- Update log messages in cxl_forward_error() (Ben)
CXL/PCI: Export and rename merge_result() to pci_ers_merge_result()
- Renamed pci_ers_merge_result() to pcie_ers_merge_result().
pci_ers_merge_result() is already used in eeh driver. (Bot)
CXL/PCI: Introduce CXL uncorrectable protocol error recovery
- Rewrite report_error_detected() and cxl_walk_port (Terry)
- Add guard() before calling cxl_pci_drv_bound() (Dave Jiang)
- Add guard() calls for EP (cxlds->cxlmd->dev & pdev->dev) and ports
(pdev->dev & parent cxl_port) in cxl_report_error_detected() and
cxl_handle_proto_error() (Terry)
- Remove unnecessary check for endpoint port. (Dave Jiang)
- Remove check for RCIEP EP in cxl_report_error_detected() (Terry)
CXL/PCI: Enable CXL protocol errors during CXL Port probe
- Add dev and dev_is_pci() NULL checks in cxl_unmask_proto_interrupts() (Terry)
- Add Dave Jiang's and Ben's review-by
CXL/PCI: Disable CXL protocol error interrupts during CXL Port cleanup
- Added dev and dev_is_pci() checks in cxl_mask_proto_interrupts() (Terry)
Changes in v11 -> v12:
cxl/pci: Remove unnecessary CXL Endpoint handling helper functions
- Added Dave Jiang's review by
- Moved to front of series
cxl/pci: Remove unnecessary CXL RCH handling helper functions
- Add reviewed-by for Alejandro & Dave Jiang
- Moved to front of series
cxl: Remove ifdef blocks of CONFIG_PCIEAER_CXL from core/pci.c
- Update CONFIG_CXL_RAS in CXL Kconfig to have CXL_PCI dependency (Terry)
CXL/AER: Remove CONFIG_PCIEAER_CXL and replace with CONFIG_CXL_RAS
- Added review-by for Sathyanarayanan
- Changed Kconfig dependency from PCIEAER_CXL to PCIEAER. Moved
this backwards into this patch.
cxl: Move CXL driver RCH error handling into CONFIG_CXL_RCH_RAS conditio
- Moved CXL_RCH_RAS Kconfig definition here from following commit
CXL/AER: Introduce aer_cxl_rch.c into AER driver for handling CXL RCH errors
- Rename drivers/pci/pcie/cxl_rch.c to drivers/pci/pcie/aer_cxl_rch.c (Lukas)
- Removed forward declararation of 'struct aer_err_info' in pci/pci.h (Terry)
CXL/PCI: Move CXL DVSEC definitions into uapi/linux/pci_regs.h
- Change formatting to be same as existing definitions
- Change GENMASK() -> __GENMASK() and BIT() to _BITUL()
PCI/CXL: Introduce pcie_is_cxl()
- Add review-by for Alejandro
- Add comment in set_pcie_cxl() explaining why updating parent status.
PCI/AER: Report CXL or PCIe bus error type in trace logging
- Change aer_err_info::is_cxl to be bool a bitfield. Update structure padding. (Lukas)
- Add kernel-doc for 'struct aer_err_info' (Lukas)
cxl/pci: Unify CXL trace logging for CXL Endpoints and CXL Ports
- Correct parameters to call trace_cxl_aer_correctable_error() (Shiju)
- Add reviewed-by for Jonathan and Shiju
cxl/pci: Map CXL Endpoint Port and CXL Switch Port RAS registers
- Add check for dport_parent->rch before calling cxl_dport_init_ras_reporting().
- RCH dports are initialized from cxl_dport_init_ras_reporting cxl_mem_probe().
CXL/PCI: Introduce PCI_ERS_RESULT_PANIC
- Documentation requested by (Lukas)
CXL/AER: Introduce aer_cxl_vh.c in AER driver for forwarding CXL errors
- Rename drivers/pci/pcie/cxl_aer.c to drivers/pci/pcie/aer_cxl_vh.c (Lukas)
cxl: Introduce cxl_pci_drv_bound() to check for bound driver
- New patch
PCI/AER: Dequeue forwarded CXL error
- Add guard for CE case in cxl_handle_proto_error() (Dave)
- Updated commit message (Terry)
CXL/PCI: Introduce CXL Port protocol error handlers
- Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
pci_to_cxl_dev() (Lukas)
- Change cxl_error_detected() -> cxl_cor_error_detected() (Terry)
- Remove NULL variable assignments (Jonathan)
- Replace bus_find_device() with find_cxl_port_by_uport() for upstream
port searches. (Dave)
CXL/PCI: Export and rename merge_result() to pci_ers_merge_result()
- Remove static inline pci_ers_merge_result() definition for !CONFIG_PCIEAER.
Is not needed. (Lukas)
CXL/PCI: Introduce CXL uncorrectable protocol error recovery
- Clean up port discovery in cxl_do_recovery() (Dave)
- Add PCI_EXP_TYPE_RC_END to type check in cxl_report_error_detected()
Changes in v10 -> v11:
cxl: Remove ifdef blocks of CONFIG_PCIEAER_CXL from core/pci.c
- New patch
CXL/AER: Remove CONFIG_PCIEAER_CXL and replace with CONFIG_CXL_RAS
- New patch
cxl/pci: Remove unnecessary CXL RCH handling helper functions
- New patch
cxl: Move CXL driver RCH error handling into CONFIG_CXL_RCH_RAS conditional block
- New patch
CXL/AER: Introduce rch_aer.c into AER driver for handling CXL RCH errors
- Remove changes in code-split and move to earlier, new patch
- Add #include <linux/bitfield.h> to cxl_ras.c
- Move cxl_rch_handle_error() & cxl_rch_enable_rcec() declarations from pci.h
to aer.h, more localized.
- Introduce CONFIG_CXL_RCH_RAS, includes Makefile changes, ras.c ifdef changes
CXL/PCI: Move CXL DVSEC definitions into uapi/linux/pci_regs.h
- New patch
PCI/CXL: Introduce pcie_is_cxl()
- Amended set_pcie_cxl() to check for Upstream Port's and EP's parent
downstream port by calling set_pcie_cxl(). (Dan)
- Retitle patch: 'Add' -> 'Introduce'
- Add check for CXL.mem and CXL.cache (Alejandro, Dan)
PCI/AER: Report CXL or PCIe bus error type in trace logging
- Remove duplicate call to trace_aer_event() (Shiju)
- Added Dan William's and Dave Jiang's reviewed-by
CXL/AER: Update PCI class code check to use FIELD_GET()
- Add #include <linux/bitfield.h> to cxl_ras.c (Terry)
- Removed line wrapping at "(CXL 3.2, 8.1.12.1)". (Jonathan)
cxl/pci: Log message if RAS registers are unmapped
- Added Dave Jiang's review-by (Terry)
cxl/pci: Unify CXL trace logging for CXL Endpoints and CXL Ports
- Updated CE and UCE trace routines to maintian consistent TP_Struct ABI
and unchanged TP_printk() logging. (Shiju, Alison)
cxl/pci: Update cxl_handle_cor_ras() to return early if no RAS errors
- Added Dave Jiang and Jonathan Cameron's review-by
- Changes moved to core/ras.c
cxl/pci: Map CXL Endpoint Port and CXL Switch Port RAS registers
- Use local pointer for readability in cxl_switch_port_init_ras() (Jonathan Cameron)
- Rename port to be ep in cxl_endpoint_port_init_ras() (Dave Jiang)
- Rename dport to be parent_dport in cxl_endpoint_port_init_ras()
and cxl_switch_port_init_ras() (Dave Jiang)
- Port helper changes were in cxl/port.c, now in core/ras.c (Dave Jiang)
cxl/pci: Introduce CXL Endpoint protocol error handlers
- cxl_error_detected() - Change handlers' scoped_guard() to guard() (Jonathan)
- cxl_error_detected() - Remove extra line (Shiju)
- Changes moved to core/ras.c (Terry)
- cxl_error_detected(), remove 'ue' and return with function call. (Jonathan)
- Remove extra space in documentation for PCI_ERS_RESULT_PANIC definition
- Move #include "pci.h from cxl.h to core.h (Terry)
- Remove unnecessary includes of cxl.h and core.h in mem.c (Terry)
CXL/AER: Introduce cxl_aer.c into AER driver for forwarding CXL errors
- Move RCH implementation to cxl_rch.c and RCH declarations to pci/pci.h. (Terry)
- Introduce 'struct cxl_proto_err_kfifo' containing semaphore, fifo,
and work struct. (Dan)
- Remove embedded struct from cxl_proto_err_work (Dan)
- Make 'struct work_struct *cxl_proto_err_work' definition static (Jonathan)
- Add check for NULL cxl_proto_err_kfifo to determine if CXL driver is
not registered for workqueue. (Dan)
PCI/AER: Dequeue forwarded CXL error
- Reword patch commit message to remove RCiEP details (Jonathan)
- Add #include <linux/bitfield.h> (Terry)
- is_cxl_rcd() - Fix short comment message wrap (Jonathan)
- is_cxl_rcd() - Combine return calls into 1 (Jonathan)
- cxl_handle_proto_error() - Move comment earlier (Jonathan)
- Usse FIELD_GET() in discovering class code (Jonathan)
- Remove BDF from cxl_proto_err_work_data. Use 'struct pci_dev *' (Dan)
CXL/PCI: Introduce CXL Port protocol error handlers
- Removed check for PCI_EXP_TYPE_RC_END in cxl_report_error_detected() (Terry)
- Update is_cxl_error() to check for acceptable PCI EP and port types
CXL/PCI: Export and rename merge_result() to pci_ers_merge_result()
- pci_ers_merge_result() - Change export to non-namespace and rename
to be pci_ers_merge_result() (Jonathan)
- Move pci_ers_merge_result() definition to pci.h. Needs pci_ers_result (Terry)
CXL/PCI: Introduce CXL uncorrectable protocol error recovery
- pci_ers_merge_results() - Move to earlier patch
CXL/PCI: Disable CXL protocol error interrupts during CXL Port cleanup
- Remove guard() in cxl_mask_proto_interrupts(). Observed device lockup/block
during testing. (Terry)
Changes in v9 -> v10:
- Add drivers/pci/pcie/cxl_aer.c
- Add drivers/cxl/core/native_ras.c
- Change cxl_register_prot_err_work()/cxl_unregister_prot_err_work to return void
- Check for pcie_ports_native in cxl_do_recovery()
- Remove debug logging in cxl_do_recovery()
- Update PCI_ERS_RESULT_PANIC definition to indicate is CXL specific
- Revert trace logging changes: name,parent -> memdev,host.
- Use FIELD_GET() to check for EP class code (cxl_aer.c & native_ras.c).
- Change _prot_ to _proto_ everywhere
- cxl_rch_handle_error_iter(), check if driver is cxl_pci_driver
- Remove cxl_create_prot_error_info(). Move logic into forward_cxl_error()
- Remove sbdf_to_pci() and move logic into cxl_handle_proto_error()
- Simplify/refactor get_pci_cxl_host_dev()
- Simplify/refactor cxl_get_ras_base()
- Move patch 'Remove unnecessary CXL Endpoint handling helper functions' to front
- Update description for 'CXL/PCI: Introduce CXL Port protocol error
handlers' with why state is not used to determine handling
- Introduce cxl_pci_drv_bound() and call from cxl_rch_handle_error_iter()
Changes in v8 -> v9:
- Updated reference counting to use pci_get_device()/pci_put_device() in
cxl_disable_prot_errors()/cxl_enable_prot_errors
- Refactored cxl_create_prot_err_info() to fix reference counting
- Removed 'struct cxl_port' driver changes for error handler. Instead
check for CXL device type (EP or Port device) and call handler
- Make pcie_is_cxl() static inline in include/linux/linux.h
- Remove NULL check in create_prot_err_info()
- Change success return in cxl_ras_init() to use hardcoded 0
- Changed 'struct work_struct cxl_prot_err_work' declaration to static
- Change to use rate limited log with dev anchor in forward_cxl_error()
- Refactored forward-cxl_error() to remove severity auto variable
- Changed pci_aer_clear_nonfatal_status() to be static inline for
!(CONFIG_PCIEAER)
- Renamed merge_result() to be cxl_merge_result()
- Removed 'ue' condition in cxl_error_detected()
- Updated 2nd parameter in call to __cxl_handle_cor_ras()/__cxl_handle_ras()
in unify patch
- Added log message for failure while assigning interrupt disable callback
- Updated pci_aer_mask_internal_errors() to use pci_clear_and_set_config_dword()
- Simplified patch titles for clarity
- Moved CXL error interrupt disabling into cxl/core/port.c with CXL Port
teardown
- Updated 'struct cxl_port_err_info' to only contain sbdf and severity
Removed everything else.
- Added pdev and CXL device get_device()/put_device() before calling handlers
Changes in v7 -> v8:
[Dan] Use kfifo. Move handling to CXL driver. AER forwards error to CXL
driver
[Dan] Add device reference incrementors where needed throughout
[Dan] Initiate CXL Port RAS init from Switch Port and Endpoint Port init
[Dan] Combine CXL Port and CXL Endpoint trace routine
[Dan] Introduce aer_info::is_cxl. Use to indicate CXL or PCI errors
[Jonathan] Add serial number for all devices in trace
[DaveJ] Move find_cxl_port() change into patch using it
[Terry] Move CXL Port RAS init into cxl/port.c
[Terry] Moved kfifo functions into cxl/core/ras.c
Changes in v6 -> v7:
[Terry] Move updated trace routine call to later patch. Was causing build
error.
Changes in v5 -> v6:
[Ira] Move pcie_is_cxl(dev) define to a inline function
[Ira] Update returning value from pcie_is_cxl_port() to bool w/o cast
[Ira] Change cxl_report_error_detected() cleanup to return correct bool
[Ira] Introduce and use PCI_ERS_RESULT_PANIC
[Ira] Reuse comment for PCIe and CXL recovery paths
[Jonathan] Add type check in for cxl_handle_cor_ras() and cxl_handle_ras()
[Jonathan] cxl_uport/dport_init_ras_reporting(), added a mutex.
[Jonathan] Add logging example to patches updating trace output
[Jonathan] Make parameter 'const' to eliminate for cast in match_uport()
[Jonathan] Use __free() in cxl_pci_port_ras()
[Terry] Add patch to log the PCIe SBDF along with CXL device name
[Terry] Add patch to handle CXL endpoint and RCH DP errors as CXL errors
[Terry] Remove patch w USP UCE fatal support @ aer_get_device_error_info()
[Terry] Rebase to cxl/next commit 5585e342e8d3 ("cxl/memdev: Remove unused partition values")
[Gregory] Pre-initialize pointer to NULL in cxl_pci_port_ras()
[Gregory] Move AER driver bus name detection to a static function
Changes in v4 -> v5:
[Alejandro] Refactor cxl_walk_bridge to simplify 'status' variable usage
[Alejandro] Add WARN_ONCE() in __cxl_handle_ras() and cxl_handle_cor_ras()
[Ming] Remove unnecessary NULL check in cxl_pci_port_ras()
[Terry] Add failure check for call to to_cxl_port() in cxl_pci_port_ras()
[Ming] Use port->dev for call to devm_add_action_or_reset() in
cxl_dport_init_ras_reporting() and cxl_uport_init_ras_reporting()
[Jonathan] Use get_device()/put_device() to prevent race condition in
cxl_clear_port_error_handlers() and cxl_clear_port_error_handlers()
[Terry] Commit message cleanup. Capitalize keywords from CXL and PCI
specifications
Changes in v3 -> v4:
[Lukas] Capitalize PCIe and CXL device names as in specifications
[Lukas] Move call to pcie_is_cxl() into cxl_port_devsec()
[Lukas] Correct namespace spelling
[Lukas] Removed export from pcie_is_cxl_port()
[Lukas] Simplify 'if' blocks in cxl_handle_error()
[Lukas] Change panic message to remove redundant 'panic' text
[Ming] Update to call cxl_dport_init_ras_reporting() in RCH case
[lkp@intel] 'host' parameter is already removed. Remove parameter description too.
[Terry] Added field description for cxl_err_handlers in pci.h comment block
Changes in v1 -> v2:
[Jonathan] Remove extra NULL check and cleanup in cxl_pci_port_ras()
[Jonathan] Update description to DSP map patch description
[Jonathan] Update cxl_pci_port_ras() to check for NULL port
[Jonathan] Dont call handler before handler port changes are present (patch order)
[Bjorn] Fix linebreak in cover sheet URL
[Bjorn] Remove timestamps from test logs in cover sheet
[Bjorn] Retitle AER commits to use "PCI/AER:"
[Bjorn] Retitle patch#3 to use renaming instead of refactoring
[Bjorn] Fix base commit-id on cover sheet
[Bjorn] Add VH spec reference/citation
[Terry] Removed last 2 patches to enable internal errors. Is not needed
because internal errors are enabled in AER driver.
[Dan] Create cxl_do_recovery() and pci_driver::cxl_err_handlers.
[Dan] Use kernel panic in CXL recovery
[Dan] cxl_port_hndlrs -> cxl_port_error_handlers
Dan Williams (3):
cxl/ras: Unify Endpoint and Port AER trace events
cxl: Rename find_cxl_port() to find_cxl_port_by_dport()
cxl: Limit CXL-CPER kfifo registration functions scope
Terry Bowman (8):
PCI/AER: Introduce AER-CXL Kfifo
cxl: Use common CPER handling for all CXL devices
PCI: Establish common CXL Port protocol error flow
PCI/CXL: Add RCH support to CXL handlers
cxl: Remove Endpoint AER correctable handler
cxl: Update Endpoint AER uncorrectable handler
PCI/CXL: Mask/Unmask CXL protocol errors
Documentation: cxl: Document CXL protocol error handling
Documentation/driver-api/cxl/index.rst | 1 +
.../cxl/linux/protocol-error-handling.rst | 440 ++++++++++++++++++
MAINTAINERS | 2 +
drivers/acpi/apei/ghes.c | 27 +-
drivers/cxl/core/core.h | 22 +-
drivers/cxl/core/port.c | 22 +-
drivers/cxl/core/ras.c | 351 +++++++++-----
drivers/cxl/core/ras_rch.c | 10 +-
drivers/cxl/core/trace.h | 76 +--
drivers/cxl/cxlpci.h | 11 +-
drivers/cxl/pci.c | 7 +-
drivers/pci/pci.c | 1 +
drivers/pci/pci.h | 2 -
drivers/pci/pcie/Makefile | 1 +
drivers/pci/pcie/aer.c | 33 +-
drivers/pci/pcie/aer_cxl_rch.c | 17 +-
drivers/pci/pcie/aer_cxl_vh.c | 149 ++++++
drivers/pci/pcie/portdrv.h | 4 +
include/cxl/event.h | 10 +-
include/linux/aer.h | 32 ++
include/linux/pci.h | 2 +
21 files changed, 959 insertions(+), 261 deletions(-)
create mode 100644 Documentation/driver-api/cxl/linux/protocol-error-handling.rst
create mode 100644 drivers/pci/pcie/aer_cxl_vh.c
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
--
2.34.1
^ permalink raw reply
* [PATCH v17 02/11] cxl/ras: Unify Endpoint and Port AER trace events
From: Terry Bowman @ 2026-05-05 17:30 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, djbw, bhelgaas,
shiju.jose, ming.li, Smita.KoralahalliChannabasappa, rrichter,
dan.carpenter, PradeepVineshReddy.Kodamati, lukas,
Benjamin.Cheatham, sathyanarayanan.kuppuswamy, vishal.l.verma,
alucerop, ira.weiny, corbet, rafael, xueshuai, linux-cxl
Cc: linux-kernel, linux-pci, linux-acpi, linux-doc, terry.bowman
In-Reply-To: <20260505173029.2718246-1-terry.bowman@amd.com>
From: Dan Williams <djbw@kernel.org>
CXL protocol error logging uses two parallel sets of trace events. The
cxl_port_aer_correctable_error() and cxl_port_aer_uncorrectable_error()
events are used by CPER for CXL Port devices. The cxl_aer_correctable_error()
and cxl_aer_uncorrectable_error() events are used for CXL Endpoints. Update
the trace routines to use the latter for all CXL devices on both the CPER
and native AER paths.
Generalize cxl_aer_correctable_error()/cxl_aer_uncorrectable_error to
take a struct device * and a u64 serial argument supplied by the caller.
cxl_handle_ras() and cxl_handle_cor_ras() gain the new u64 serial parameter,
sourced from pci_get_dsn().
The CPER path keeps its existing Port-vs-Endpoint dispatch and passes the
new arguments to the unified trace events. The CPER path will be folded
together in a following patch.
Remove the now-unused cxl_port_aer_correctable_error() and
cxl_port_aer_uncorrectable_error().
**WARNING: ABI BREAK**
Rename the trace event field "memdev" to "device" so all CXL device types
(Ports and Endpoints) can be reported under a common field name. Note this
is an ABI break for userspace tools that key off the old "memdev" field.
Specifically, rasdaemon's ras-cxl-handler.c looks up "memdev" and bails on
NULL, so an unmodified rasdaemon will drop every CXL CE/UCE event once this
kernel ships. A rasdaemon update is needed in a separate series.
The need for the field rename was discussed in v16 review [1].
Also, for CXL Upstream Switch Port (USP) and Endpoint (EP) fatal UCE,
the cxl_aer_uncorrectable_error trace event is not emitted. The AER core
only retrieves PCI_ERR_UNCOR_STATUS for Root Ports, RCECs, and Downstream
Ports, or for non-fatal severities. PCI config reads to the source device
are expected to fail otherwise, so the AER core never reads the status
word, is_cxl_error() does not classify the event as CXL, and the AER path
handles it instead. In this case the AER handler consumes the event and
logs it as an AER error without calling the CXL RAS handlers or trace
logging.
Before this patch, Endpoint and Port devices emitted different events:
# Endpoint (cxl_aer_*):
cxl_aer_correctable_error: memdev=mem0 host=0000:0c:00.0 serial=0: status: 'CRC Threshold Hit'
cxl_aer_uncorrectable_error: memdev=mem0 host=0000:0c:00.0 serial=0: status: 'Cache Data ECC Error | Memory Data ECC Error' first_error: 'Cache Data ECC Error'
# Port (cxl_port_aer_*, no serial field):
cxl_port_aer_correctable_error: device=0000:0c:00.0 host=pci0000:0c status='CRC Threshold Hit'
cxl_port_aer_uncorrectable_error: device=0000:0c:00.0 host=pci0000:0c status: 'Cache Data ECC Error | Memory Data ECC Error' first_error: 'Cache Data ECC Error'
After this patch, all CXL devices emit the unified cxl_aer_* events
with the same field layout:
cxl_aer_correctable_error: device=0000:0c:00.0 host=pci0000:0c serial=0 status: 'CRC Threshold Hit'
cxl_aer_uncorrectable_error: device=0000:0c:00.0 host=pci0000:0c serial=0 status: 'Cache Data ECC Error | Memory Data ECC Error' first_error: 'Cache Data ECC Error'
[1] https://lore.kernel.org/linux-cxl/69cb2d5ba3111_178904100b7@dwillia2-mobl4.notmuch/
Co-developed-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Dan Williams <djbw@kernel.org>
---
Changes in v16->v17:
- Replace cxlds->serial with pci_get_dsn()
- Change 'memdev' to 'device' (Dan)
- Updated Commit message
Changes in v15->v16:
- Add Dan's review-by
- Incorporate Dan's comment into commit message:
"Add the serial number at the end to preserve compatibility with
libtraceevent parsing of the parameters."
Changes in v14->v15:
- Update commit message.
- Moved cxl_handle_ras/cxl_handle_cor_ras() changes to future patch (terry)
Changes in v13->v14:
- Update commit headline (Bjorn)
Changes in v12->v13:
- Added Dave Jiang's review-by
Changes in v11 -> v12:
- Correct parameters to call trace_cxl_aer_correctable_error()
- Add reviewed-by for Jonathan and Shiju
Changes in v10->v11:
- Updated CE and UCE trace routines to maintain consistent TP_Struct ABI
and unchanged TP_printk() logging.
---
drivers/cxl/core/core.h | 11 ++++--
drivers/cxl/core/ras.c | 39 +++++++++++--------
drivers/cxl/core/ras_rch.c | 6 ++-
drivers/cxl/core/trace.h | 76 ++++++++------------------------------
4 files changed, 49 insertions(+), 83 deletions(-)
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 82ca3a476708..132ac9c1ebf4 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -183,8 +183,9 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
#ifdef CONFIG_CXL_RAS
int cxl_ras_init(void);
void cxl_ras_exit(void);
-bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
-void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);
+bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base);
+void cxl_handle_cor_ras(struct device *dev, u64 serial,
+ void __iomem *ras_base);
void cxl_dport_map_rch_aer(struct cxl_dport *dport);
void cxl_disable_rch_root_ints(struct cxl_dport *dport);
void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds);
@@ -195,11 +196,13 @@ static inline int cxl_ras_init(void)
return 0;
}
static inline void cxl_ras_exit(void) { }
-static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
+static inline bool cxl_handle_ras(struct device *dev, u64 serial,
+ void __iomem *ras_base)
{
return false;
}
-static inline void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base) { }
+static inline void cxl_handle_cor_ras(struct device *dev, u64 serial,
+ void __iomem *ras_base) { }
static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
static inline void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) { }
diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
index 006c6ffc2f56..d7081caaf5d3 100644
--- a/drivers/cxl/core/ras.c
+++ b/drivers/cxl/core/ras.c
@@ -13,7 +13,7 @@ static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev,
{
u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
- trace_cxl_port_aer_correctable_error(&pdev->dev, status);
+ trace_cxl_aer_correctable_error(&pdev->dev, status, pci_get_dsn(pdev));
}
static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
@@ -28,20 +28,24 @@ static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
else
fe = status;
- trace_cxl_port_aer_uncorrectable_error(&pdev->dev, status, fe,
- ras_cap.header_log);
+ trace_cxl_aer_uncorrectable_error(&pdev->dev, status, fe,
+ ras_cap.header_log,
+ pci_get_dsn(pdev));
}
-static void cxl_cper_trace_corr_prot_err(struct cxl_memdev *cxlmd,
+static void cxl_cper_trace_corr_prot_err(struct pci_dev *pdev,
+ struct cxl_memdev *cxlmd,
struct cxl_ras_capability_regs ras_cap)
{
u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
- trace_cxl_aer_correctable_error(cxlmd, status);
+ trace_cxl_aer_correctable_error(&cxlmd->dev, status,
+ pci_get_dsn(pdev));
}
static void
-cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
+cxl_cper_trace_uncorr_prot_err(struct pci_dev *pdev,
+ struct cxl_memdev *cxlmd,
struct cxl_ras_capability_regs ras_cap)
{
u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
@@ -53,8 +57,9 @@ cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
else
fe = status;
- trace_cxl_aer_uncorrectable_error(cxlmd, status, fe,
- ras_cap.header_log);
+ trace_cxl_aer_uncorrectable_error(&cxlmd->dev, status, fe,
+ ras_cap.header_log,
+ pci_get_dsn(pdev));
}
static int match_memdev_by_parent(struct device *dev, const void *uport)
@@ -101,9 +106,9 @@ void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
cxlmd = to_cxl_memdev(mem_dev);
if (data->severity == AER_CORRECTABLE)
- cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
+ cxl_cper_trace_corr_prot_err(pdev, cxlmd, data->ras_cap);
else
- cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
+ cxl_cper_trace_uncorr_prot_err(pdev, cxlmd, data->ras_cap);
}
EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);
@@ -183,7 +188,7 @@ void devm_cxl_port_ras_setup(struct cxl_port *port)
}
EXPORT_SYMBOL_NS_GPL(devm_cxl_port_ras_setup, "CXL");
-void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
+void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
{
void __iomem *addr;
u32 status;
@@ -195,7 +200,7 @@ void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
status = readl(addr);
if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
- trace_cxl_aer_correctable_error(to_cxl_memdev(dev), status);
+ trace_cxl_aer_correctable_error(dev, status, serial);
}
}
@@ -220,7 +225,7 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
* Log the state of the RAS status registers and prepare them to log the
* next error status. Return 1 if reset needed.
*/
-bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
+bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
{
u32 hl[CXL_HEADERLOG_SIZE_U32];
void __iomem *addr;
@@ -247,7 +252,7 @@ bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
}
header_log_copy(ras_base, hl);
- trace_cxl_aer_uncorrectable_error(to_cxl_memdev(dev), status, fe, hl);
+ trace_cxl_aer_uncorrectable_error(dev, status, fe, hl, serial);
writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);
return true;
@@ -270,7 +275,8 @@ void cxl_cor_error_detected(struct pci_dev *pdev)
if (cxlds->rcd)
cxl_handle_rdport_errors(cxlds);
- cxl_handle_cor_ras(&cxlds->cxlmd->dev, cxlmd->endpoint->regs.ras);
+ cxl_handle_cor_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
+ cxlmd->endpoint->regs.ras);
}
}
EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, "CXL");
@@ -299,7 +305,8 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
* chance the situation is recoverable dump the status of the RAS
* capability registers and bounce the active state of the memdev.
*/
- ue = cxl_handle_ras(&cxlds->cxlmd->dev, cxlmd->endpoint->regs.ras);
+ ue = cxl_handle_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
+ cxlmd->endpoint->regs.ras);
}
switch (state) {
diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
index 0a8b3b9b6388..61835fbafc0f 100644
--- a/drivers/cxl/core/ras_rch.c
+++ b/drivers/cxl/core/ras_rch.c
@@ -115,7 +115,9 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
pci_print_aer(pdev, severity, &aer_regs);
if (severity == AER_CORRECTABLE)
- cxl_handle_cor_ras(&cxlds->cxlmd->dev, dport->regs.ras);
+ cxl_handle_cor_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
+ dport->regs.ras);
else
- cxl_handle_ras(&cxlds->cxlmd->dev, dport->regs.ras);
+ cxl_handle_ras(&cxlds->cxlmd->dev, pci_get_dsn(pdev),
+ dport->regs.ras);
}
diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index a972e4ef1936..6f3957b3c3af 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -48,49 +48,22 @@
{ CXL_RAS_UC_IDE_RX_ERR, "IDE Rx Error" } \
)
-TRACE_EVENT(cxl_port_aer_uncorrectable_error,
- TP_PROTO(struct device *dev, u32 status, u32 fe, u32 *hl),
- TP_ARGS(dev, status, fe, hl),
+TRACE_EVENT(cxl_aer_uncorrectable_error,
+ TP_PROTO(const struct device *dev, u32 status, u32 fe, u32 *hl,
+ u64 serial),
+ TP_ARGS(dev, status, fe, hl, serial),
TP_STRUCT__entry(
__string(device, dev_name(dev))
__string(host, dev_name(dev->parent))
- __field(u32, status)
- __field(u32, first_error)
- __array(u32, header_log, CXL_HEADERLOG_SIZE_U32)
- ),
- TP_fast_assign(
- __assign_str(device);
- __assign_str(host);
- __entry->status = status;
- __entry->first_error = fe;
- /*
- * Embed the 512B headerlog data for user app retrieval and
- * parsing, but no need to print this in the trace buffer.
- */
- memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
- ),
- TP_printk("device=%s host=%s status: '%s' first_error: '%s'",
- __get_str(device), __get_str(host),
- show_uc_errs(__entry->status),
- show_uc_errs(__entry->first_error)
- )
-);
-
-TRACE_EVENT(cxl_aer_uncorrectable_error,
- TP_PROTO(const struct cxl_memdev *cxlmd, u32 status, u32 fe, u32 *hl),
- TP_ARGS(cxlmd, status, fe, hl),
- TP_STRUCT__entry(
- __string(memdev, dev_name(&cxlmd->dev))
- __string(host, dev_name(cxlmd->dev.parent))
__field(u64, serial)
__field(u32, status)
__field(u32, first_error)
__array(u32, header_log, CXL_HEADERLOG_SIZE_U32)
),
TP_fast_assign(
- __assign_str(memdev);
+ __assign_str(device);
__assign_str(host);
- __entry->serial = cxlmd->cxlds->serial;
+ __entry->serial = serial;
__entry->status = status;
__entry->first_error = fe;
/*
@@ -99,8 +72,8 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
*/
memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
),
- TP_printk("memdev=%s host=%s serial=%lld: status: '%s' first_error: '%s'",
- __get_str(memdev), __get_str(host), __entry->serial,
+ TP_printk("device=%s host=%s serial=%lld status: '%s' first_error: '%s'",
+ __get_str(device), __get_str(host), __entry->serial,
show_uc_errs(__entry->status),
show_uc_errs(__entry->first_error)
)
@@ -124,42 +97,23 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
{ CXL_RAS_CE_PHYS_LAYER_ERR, "Received Error From Physical Layer" } \
)
-TRACE_EVENT(cxl_port_aer_correctable_error,
- TP_PROTO(struct device *dev, u32 status),
- TP_ARGS(dev, status),
+TRACE_EVENT(cxl_aer_correctable_error,
+ TP_PROTO(const struct device *dev, u32 status, u64 serial),
+ TP_ARGS(dev, status, serial),
TP_STRUCT__entry(
__string(device, dev_name(dev))
__string(host, dev_name(dev->parent))
- __field(u32, status)
- ),
- TP_fast_assign(
- __assign_str(device);
- __assign_str(host);
- __entry->status = status;
- ),
- TP_printk("device=%s host=%s status='%s'",
- __get_str(device), __get_str(host),
- show_ce_errs(__entry->status)
- )
-);
-
-TRACE_EVENT(cxl_aer_correctable_error,
- TP_PROTO(const struct cxl_memdev *cxlmd, u32 status),
- TP_ARGS(cxlmd, status),
- TP_STRUCT__entry(
- __string(memdev, dev_name(&cxlmd->dev))
- __string(host, dev_name(cxlmd->dev.parent))
__field(u64, serial)
__field(u32, status)
),
TP_fast_assign(
- __assign_str(memdev);
+ __assign_str(device);
__assign_str(host);
- __entry->serial = cxlmd->cxlds->serial;
+ __entry->serial = serial;
__entry->status = status;
),
- TP_printk("memdev=%s host=%s serial=%lld: status: '%s'",
- __get_str(memdev), __get_str(host), __entry->serial,
+ TP_printk("device=%s host=%s serial=%lld status: '%s'",
+ __get_str(device), __get_str(host), __entry->serial,
show_ce_errs(__entry->status)
)
);
--
2.34.1
^ permalink raw reply related
* [PATCH v17 03/11] cxl: Use common CPER handling for all CXL devices
From: Terry Bowman @ 2026-05-05 17:30 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, djbw, bhelgaas,
shiju.jose, ming.li, Smita.KoralahalliChannabasappa, rrichter,
dan.carpenter, PradeepVineshReddy.Kodamati, lukas,
Benjamin.Cheatham, sathyanarayanan.kuppuswamy, vishal.l.verma,
alucerop, ira.weiny, corbet, rafael, xueshuai, linux-cxl
Cc: linux-kernel, linux-pci, linux-acpi, linux-doc, terry.bowman
In-Reply-To: <20260505173029.2718246-1-terry.bowman@amd.com>
Fold the Port and Endpoint specific paths in cxl_cper_handle_prot_err()
into a single code path. Drop the PCI type dispatch block as both Port
and Endpoint devices now go through the same code path.
Extend the pdev->dev.driver != NULL gate to Port devices, which previously
bypassed it. This check and the existing device lock will ensure the CXL
device remains accessible while in scope.
Recent trace event changes generalize the interface to take a
struct device * for all CXL devices. Update the Endpoint CPER path
to pass &pdev->dev (the PCI device) instead of &cxlmd->dev (the
memdev). This makes the trace event's "device=" field show the PCI
BDF for all CPER callers, replacing the prior "device=memN" output
for Endpoints. Userspace consumers correlating CPER trace events to
memdev names must map the PCI BDF back via /sys/bus/cxl/devices/.
Remove the bus_find_device(&cxl_bus_type, ..., match_memdev_by_parent)
lookup along with the match_memdev_by_parent() helper.
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
Changes in v16->v17:
- New commit
---
drivers/cxl/core/ras.c | 81 +++++++-----------------------------------
1 file changed, 13 insertions(+), 68 deletions(-)
diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
index d7081caaf5d3..56611da8357a 100644
--- a/drivers/cxl/core/ras.c
+++ b/drivers/cxl/core/ras.c
@@ -8,65 +8,28 @@
#include <cxlpci.h>
#include "trace.h"
-static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev,
- struct cxl_ras_capability_regs ras_cap)
+static void cxl_cper_trace_corr_prot_err(struct pci_dev *pdev, u64 serial,
+ struct cxl_ras_capability_regs *ras_cap)
{
- u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
+ u32 status = ras_cap->cor_status & ~ras_cap->cor_mask;
- trace_cxl_aer_correctable_error(&pdev->dev, status, pci_get_dsn(pdev));
+ trace_cxl_aer_correctable_error(&pdev->dev, status, serial);
}
-static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
- struct cxl_ras_capability_regs ras_cap)
+static void cxl_cper_trace_uncorr_prot_err(struct pci_dev *pdev, u64 serial,
+ struct cxl_ras_capability_regs *ras_cap)
{
- u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
+ u32 status = ras_cap->uncor_status & ~ras_cap->uncor_mask;
u32 fe;
if (hweight32(status) > 1)
fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
- ras_cap.cap_control));
+ ras_cap->cap_control));
else
fe = status;
trace_cxl_aer_uncorrectable_error(&pdev->dev, status, fe,
- ras_cap.header_log,
- pci_get_dsn(pdev));
-}
-
-static void cxl_cper_trace_corr_prot_err(struct pci_dev *pdev,
- struct cxl_memdev *cxlmd,
- struct cxl_ras_capability_regs ras_cap)
-{
- u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
-
- trace_cxl_aer_correctable_error(&cxlmd->dev, status,
- pci_get_dsn(pdev));
-}
-
-static void
-cxl_cper_trace_uncorr_prot_err(struct pci_dev *pdev,
- struct cxl_memdev *cxlmd,
- struct cxl_ras_capability_regs ras_cap)
-{
- u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
- u32 fe;
-
- if (hweight32(status) > 1)
- fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
- ras_cap.cap_control));
- else
- fe = status;
-
- trace_cxl_aer_uncorrectable_error(&cxlmd->dev, status, fe,
- ras_cap.header_log,
- pci_get_dsn(pdev));
-}
-
-static int match_memdev_by_parent(struct device *dev, const void *uport)
-{
- if (is_cxl_memdev(dev) && dev->parent == uport)
- return 1;
- return 0;
+ ras_cap->header_log, serial);
}
void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
@@ -77,38 +40,20 @@ void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
pci_get_domain_bus_and_slot(data->prot_err.agent_addr.segment,
data->prot_err.agent_addr.bus,
devfn);
- struct cxl_memdev *cxlmd;
- int port_type;
if (!pdev)
return;
- port_type = pci_pcie_type(pdev);
- if (port_type == PCI_EXP_TYPE_ROOT_PORT ||
- port_type == PCI_EXP_TYPE_DOWNSTREAM ||
- port_type == PCI_EXP_TYPE_UPSTREAM) {
- if (data->severity == AER_CORRECTABLE)
- cxl_cper_trace_corr_port_prot_err(pdev, data->ras_cap);
- else
- cxl_cper_trace_uncorr_port_prot_err(pdev, data->ras_cap);
-
- return;
- }
-
guard(device)(&pdev->dev);
if (!pdev->dev.driver)
return;
- struct device *mem_dev __free(put_device) = bus_find_device(
- &cxl_bus_type, NULL, pdev, match_memdev_by_parent);
- if (!mem_dev)
- return;
-
- cxlmd = to_cxl_memdev(mem_dev);
if (data->severity == AER_CORRECTABLE)
- cxl_cper_trace_corr_prot_err(pdev, cxlmd, data->ras_cap);
+ cxl_cper_trace_corr_prot_err(pdev, pci_get_dsn(pdev),
+ &data->ras_cap);
else
- cxl_cper_trace_uncorr_prot_err(pdev, cxlmd, data->ras_cap);
+ cxl_cper_trace_uncorr_prot_err(pdev, pci_get_dsn(pdev),
+ &data->ras_cap);
}
EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);
--
2.34.1
^ permalink raw reply related
* [PATCH v17 04/11] cxl: Rename find_cxl_port() to find_cxl_port_by_dport()
From: Terry Bowman @ 2026-05-05 17:30 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, djbw, bhelgaas,
shiju.jose, ming.li, Smita.KoralahalliChannabasappa, rrichter,
dan.carpenter, PradeepVineshReddy.Kodamati, lukas,
Benjamin.Cheatham, sathyanarayanan.kuppuswamy, vishal.l.verma,
alucerop, ira.weiny, corbet, rafael, xueshuai, linux-cxl
Cc: linux-kernel, linux-pci, linux-acpi, linux-doc, terry.bowman
In-Reply-To: <20260505173029.2718246-1-terry.bowman@amd.com>
From: Dan Williams <djbw@kernel.org>
find_cxl_port() and find_cxl_port_by_uport() are internal port lookup
functions that search the CXL bus by dport and uport respectively, but
their names do not make the lookup method clear.
Rename find_cxl_port() to find_cxl_port_by_dport() to make the lookup
method explicit and consistent with find_cxl_port_by_uport(). Both
functions remain static to port.c; the upcoming patch that adds the
first cross-file caller will widen their scope.
Co-developed-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Dan Williams <djbw@kernel.org>
---
Changes in v16->v17:
- New commit
---
drivers/cxl/core/port.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index c5aacd7054f1..b35a9016fc81 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1377,7 +1377,7 @@ static int match_port_by_dport(struct device *dev, const void *data)
return dport != NULL;
}
-static struct cxl_port *__find_cxl_port(struct cxl_find_port_ctx *ctx)
+static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
{
struct device *dev;
@@ -1390,8 +1390,16 @@ static struct cxl_port *__find_cxl_port(struct cxl_find_port_ctx *ctx)
return NULL;
}
-static struct cxl_port *find_cxl_port(struct device *dport_dev,
- struct cxl_dport **dport)
+/**
+ * find_cxl_port_by_dport - find a cxl_port by one of its targets
+ * @dport_dev: device representing the dport target
+ * @dport: optional output of the 'struct cxl_dport' companion of the @dport_dev
+ *
+ * Return a 'struct cxl_port' with an elevated reference if found. Use
+ * __free(put_cxl_port) to release.
+ */
+static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
+ struct cxl_dport **dport)
{
struct cxl_find_port_ctx ctx = {
.dport_dev = dport_dev,
@@ -1399,7 +1407,7 @@ static struct cxl_port *find_cxl_port(struct device *dport_dev,
};
struct cxl_port *port;
- port = __find_cxl_port(&ctx);
+ port = __find_cxl_port_by_dport(&ctx);
return port;
}
@@ -1893,14 +1901,14 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, "CXL");
struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev,
struct cxl_dport **dport)
{
- return find_cxl_port(pdev->dev.parent, dport);
+ return find_cxl_port_by_dport(pdev->dev.parent, dport);
}
EXPORT_SYMBOL_NS_GPL(cxl_pci_find_port, "CXL");
struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
struct cxl_dport **dport)
{
- return find_cxl_port(grandparent(&cxlmd->dev), dport);
+ return find_cxl_port_by_dport(grandparent(&cxlmd->dev), dport);
}
EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL");
--
2.34.1
^ permalink raw reply related
* [PATCH v17 05/11] cxl: Limit CXL-CPER kfifo registration functions scope
From: Terry Bowman @ 2026-05-05 17:30 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, djbw, bhelgaas,
shiju.jose, ming.li, Smita.KoralahalliChannabasappa, rrichter,
dan.carpenter, PradeepVineshReddy.Kodamati, lukas,
Benjamin.Cheatham, sathyanarayanan.kuppuswamy, vishal.l.verma,
alucerop, ira.weiny, corbet, rafael, xueshuai, linux-cxl
Cc: linux-kernel, linux-pci, linux-acpi, linux-doc, terry.bowman
In-Reply-To: <20260505173029.2718246-1-terry.bowman@amd.com>
From: Dan Williams <djbw@kernel.org>
Some CPER functions used by CXL drivers are exported using the
EXPORT_SYMBOL_NS_GPL(fn, ns) macro. This doesn't provide compile time
enforcement or visibility of the consumers.
This can be improved by using EXPORT_SYMBOL_FOR_MODULES() instead.
EXPORT_SYMBOL_FOR_MODULES() explicitly names the modules that can access
the function. This provides more precise control and visibility of symbol
exposure than the namespace macro. It also provides compile time checking.
To improve control and clarity, update cxl_cper_register_prot_err_work(),
cxl_cper_unregister_prot_err_work(), and cxl_cper_prot_err_kfifo_get()
to use EXPORT_SYMBOL_FOR_MODULES(). Also, update the register and unregister
functions to return void type.
Update the CPER kfifo unregister to cancel work while using
synchronization.
Co-developed-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Dan Williams <djbw@kernel.org>
---
Changes in v16->v17:
- Split from v16 02/10 ("Update unregistration for AER-CXL and
CPER-CXL kfifos"); AER-CXL half folded into v17 01/10.
- Convert exports to EXPORT_SYMBOL_FOR_MODULES("cxl_core").
- Change register/unregister return type from int to void.
- Drop work_struct argument from cxl_cper_unregister_prot_err_work();
it now cancels its own work.
- Remove now-redundant cancel_work_sync() from cxl_ras_exit().
- Add WARN_ONCE() in cxl_cper_register_prot_err_work() for
double-registration.
---
drivers/acpi/apei/ghes.c | 27 ++++++++++++++-------------
drivers/cxl/core/ras.c | 6 +++---
include/cxl/event.h | 10 ++++------
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 3236a3ce79d6..dd0a073af93c 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -778,33 +778,34 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
#endif
}
-int cxl_cper_register_prot_err_work(struct work_struct *work)
+void cxl_cper_register_prot_err_work(struct work_struct *work)
{
- if (cxl_cper_prot_err_work)
- return -EINVAL;
-
guard(spinlock)(&cxl_cper_prot_err_work_lock);
+ WARN_ONCE(cxl_cper_prot_err_work,
+ "CPER-CXL kfifo consumer already registered\n");
cxl_cper_prot_err_work = work;
- return 0;
}
-EXPORT_SYMBOL_NS_GPL(cxl_cper_register_prot_err_work, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_register_prot_err_work, "cxl_core");
-int cxl_cper_unregister_prot_err_work(struct work_struct *work)
+void cxl_cper_unregister_prot_err_work(void)
{
- if (cxl_cper_prot_err_work != work)
- return -EINVAL;
+ struct work_struct *work;
- guard(spinlock)(&cxl_cper_prot_err_work_lock);
+ spin_lock(&cxl_cper_prot_err_work_lock);
+ work = cxl_cper_prot_err_work;
cxl_cper_prot_err_work = NULL;
- return 0;
+ spin_unlock(&cxl_cper_prot_err_work_lock);
+
+ if (work)
+ cancel_work_sync(work);
}
-EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_prot_err_work, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_unregister_prot_err_work, "cxl_core");
int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd)
{
return kfifo_get(&cxl_cper_prot_err_fifo, wd);
}
-EXPORT_SYMBOL_NS_GPL(cxl_cper_prot_err_kfifo_get, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_prot_err_kfifo_get, "cxl_core");
/* Room for 8 entries for each of the 4 event log queues */
#define CXL_CPER_FIFO_DEPTH 32
diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
index 56611da8357a..9193dac4e507 100644
--- a/drivers/cxl/core/ras.c
+++ b/drivers/cxl/core/ras.c
@@ -68,13 +68,13 @@ static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn);
int cxl_ras_init(void)
{
- return cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
+ cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
+ return 0;
}
void cxl_ras_exit(void)
{
- cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
- cancel_work_sync(&cxl_cper_prot_err_work);
+ cxl_cper_unregister_prot_err_work();
}
static void cxl_dport_map_ras(struct cxl_dport *dport)
diff --git a/include/cxl/event.h b/include/cxl/event.h
index ff97fea718d2..51acedb0d683 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -289,8 +289,8 @@ struct cxl_cper_prot_err_work_data {
int cxl_cper_register_work(struct work_struct *work);
int cxl_cper_unregister_work(struct work_struct *work);
int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd);
-int cxl_cper_register_prot_err_work(struct work_struct *work);
-int cxl_cper_unregister_prot_err_work(struct work_struct *work);
+void cxl_cper_register_prot_err_work(struct work_struct *work);
+void cxl_cper_unregister_prot_err_work(void);
int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd);
#else
static inline int cxl_cper_register_work(struct work_struct *work)
@@ -306,13 +306,11 @@ static inline int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)
{
return 0;
}
-static inline int cxl_cper_register_prot_err_work(struct work_struct *work)
+static inline void cxl_cper_register_prot_err_work(struct work_struct *work)
{
- return 0;
}
-static inline int cxl_cper_unregister_prot_err_work(struct work_struct *work)
+static inline void cxl_cper_unregister_prot_err_work(void)
{
- return 0;
}
static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd)
{
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox