* Re: [PATCH] dmaengine: qcom: bam_dma: Avoid accessing BAM_REVISION on remote BAM
From: Bjorn Andersson @ 2025-02-12 2:43 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Bjorn Andersson, Vinod Koul, Md Sadre Alam, linux-arm-msm,
dmaengine, linux-kernel, Dmitry Baryshkov, Georgi Djakov
In-Reply-To: <Z6m8btwhJ9q4RjB6@linaro.org>
On Mon, Feb 10, 2025 at 09:44:30AM +0100, Stephan Gerhold wrote:
> On Fri, Feb 07, 2025 at 12:17:33PM -0800, Bjorn Andersson wrote:
> > Commit '57a7138d0627 ("dmaengine: qcom: bam_dma: Avoid writing
> > unavailable register")' made this read unconditional, in order to
> > identify if the instance is BAM-NDP or BAM-Lite.
> > But the BAM_REVISION register is not accessible on remotely managed BAM
> > instances and attempts to access it causes the system to crash.
> >
> > Move the access back to be conditional and expand the checks that was
> > introduced to restore the old behavior when no revision information is
> > available.
> >
> > Fixes: 57a7138d0627 ("dmaengine: qcom: bam_dma: Avoid writing unavailable register")
> > Reported-by: Georgi Djakov <djakov@kernel.org>
> > Closes: https://lore.kernel.org/lkml/9ef3daa8-cdb1-49f2-8d19-a72d6210ff3a@kernel.org/
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
>
> This patch fixes the most critical regression (the bus hang), but the
> in_range(..., BAM_NDP) checks are also wrong. They do not consider the
> plain "BAM" type where the register is apparently also available. The
> check should be !in_range(..., BAM_LITE) instead to fix this.
>
Sorry, I must have not paid sufficient attention while browsing the
replies; and just tried to restore the one case I thought the author
didn't consider...
Thanks for staying on top of it. Revert makes total sense.
Regards,
Bjorn
> I mentioned this twice to Md Sadre Alam [1, 2], but a fix was
> unfortunately never sent for that part of the regression.
>
> I think we should take Caleb's patch and revert the entire patch for the
> 6.14 cycle. There are several incorrect assumptions in the original
> patch, it will be easier to review a fixed version with the full diff,
> rather than looking at incremental fixups.
>
> On a somewhat related note, I'm working on a patch series for bam_dma to
> clean up the handling of remotely controlled BAMs. It will make it more
> clear when it's safe to access BAM registers and when not, and should
> allow reading the revision also for remotely controlled BAMs. This would
> avoid the need for all these if (!bdev->bam_revision) checks.
>
> Thanks,
> Stephan
>
> [1]: https://lore.kernel.org/linux-arm-msm/Z4D2jQNNW94qGIlv@linaro.org/
> [2]: https://lore.kernel.org/linux-arm-msm/Z4_U19_QyH2RJvKW@linaro.org/
>
> > ---
> > drivers/dma/qcom/bam_dma.c | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> > index c14557efd577..d42d913492a8 100644
> > --- a/drivers/dma/qcom/bam_dma.c
> > +++ b/drivers/dma/qcom/bam_dma.c
> > @@ -445,8 +445,8 @@ static void bam_reset(struct bam_device *bdev)
> > writel_relaxed(val, bam_addr(bdev, 0, BAM_CTRL));
> >
> > /* set descriptor threshold, start with 4 bytes */
> > - if (in_range(bdev->bam_revision, BAM_NDP_REVISION_START,
> > - BAM_NDP_REVISION_END))
> > + if (!bdev->bam_revision ||
> > + in_range(bdev->bam_revision, BAM_NDP_REVISION_START, BAM_NDP_REVISION_END))
> > writel_relaxed(DEFAULT_CNT_THRSHLD,
> > bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
> >
> > @@ -1006,8 +1006,8 @@ static void bam_apply_new_config(struct bam_chan *bchan,
> > maxburst = bchan->slave.src_maxburst;
> > else
> > maxburst = bchan->slave.dst_maxburst;
> > - if (in_range(bdev->bam_revision, BAM_NDP_REVISION_START,
> > - BAM_NDP_REVISION_END))
> > + if (!bdev->bam_revision ||
> > + in_range(bdev->bam_revision, BAM_NDP_REVISION_START, BAM_NDP_REVISION_END))
> > writel_relaxed(maxburst,
> > bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
> > }
> > @@ -1199,11 +1199,12 @@ static int bam_init(struct bam_device *bdev)
> > u32 val;
> >
> > /* read revision and configuration information */
> > - val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
> > - if (!bdev->num_ees)
> > + if (!bdev->num_ees) {
> > + val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION));
> > bdev->num_ees = (val >> NUM_EES_SHIFT) & NUM_EES_MASK;
> >
> > - bdev->bam_revision = val & REVISION_MASK;
> > + bdev->bam_revision = val & REVISION_MASK;
> > + }
> >
> > /* check that configured EE is within range */
> > if (bdev->ee >= bdev->num_ees)
> >
> > ---
> > base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> > change-id: 20250207-bam-read-fix-2b31297d3fa1
> >
> > Best regards,
> > --
> > Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> >
>
^ permalink raw reply
* Re: [PATCH 3/7] dt-bindings: dma: rz-dmac: Document RZ/V2H(P) family of SoCs
From: Rob Herring @ 2025-02-11 22:26 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Vinod Koul, Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Biju Das, dmaengine, devicetree, linux-kernel,
linux-renesas-soc, Lad Prabhakar
In-Reply-To: <20250206220308.76669-4-fabrizio.castro.jz@renesas.com>
On Thu, Feb 06, 2025 at 10:03:04PM +0000, Fabrizio Castro wrote:
> Document the Renesas RZ/V2H(P) family of SoCs DMAC block.
> The Renesas RZ/V2H(P) DMAC is very similar to the one found on the
> Renesas RZ/G2L family of SoCs, but there are some differences:
> * It only uses one register area
> * It only uses one clock
> * It only uses one reset
> * Instead of using MID/IRD it uses REQ NO/ACK NO
> * It is connected to the Interrupt Control Unit (ICU)
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> .../bindings/dma/renesas,rz-dmac.yaml | 152 +++++++++++++++---
> 1 file changed, 127 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> index 82de3b927479..d4dd22432e49 100644
> --- a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> @@ -11,19 +11,23 @@ maintainers:
>
> properties:
> compatible:
> - items:
> - - enum:
> - - renesas,r7s72100-dmac # RZ/A1H
> - - renesas,r9a07g043-dmac # RZ/G2UL and RZ/Five
> - - renesas,r9a07g044-dmac # RZ/G2{L,LC}
> - - renesas,r9a07g054-dmac # RZ/V2L
> - - renesas,r9a08g045-dmac # RZ/G3S
> - - const: renesas,rz-dmac
> + oneOf:
> + - items:
> + - enum:
> + - renesas,r7s72100-dmac # RZ/A1H
> + - renesas,r9a07g043-dmac # RZ/G2UL and RZ/Five
> + - renesas,r9a07g044-dmac # RZ/G2{L,LC}
> + - renesas,r9a07g054-dmac # RZ/V2L
> + - renesas,r9a08g045-dmac # RZ/G3S
> + - const: renesas,rz-dmac
> +
> + - const: renesas,r9a09g057-dmac # RZ/V2H(P)
>
> reg:
> items:
> - description: Control and channel register block
> - description: DMA extended resource selector block
> + minItems: 1
>
> interrupts:
> maxItems: 17
> @@ -52,6 +56,7 @@ properties:
> items:
> - description: DMA main clock
> - description: DMA register access clock
> + minItems: 1
>
> clock-names:
> items:
> @@ -61,14 +66,22 @@ properties:
> '#dma-cells':
> const: 1
> description:
> - The cell specifies the encoded MID/RID values of the DMAC port
> - connected to the DMA client and the slave channel configuration
> - parameters.
> + For the RZ/A1H, RZ/Five, RZ/G2{L,LC,UL}, RZ/V2L, and RZ/G3S SoCs, the cell
> + specifies the encoded MID/RID values of the DMAC port connected to the
> + DMA client and the slave channel configuration parameters.
> bits[0:9] - Specifies MID/RID value
> bit[10] - Specifies DMA request high enable (HIEN)
> bit[11] - Specifies DMA request detection type (LVL)
> bits[12:14] - Specifies DMAACK output mode (AM)
> bit[15] - Specifies Transfer Mode (TM)
> + For the RZ/V2H(P) SoC the cell specifies the REQ NO, the ACK NO, and the
> + slave channel configuration parameters.
> + bits[0:9] - Specifies the REQ NO
> + bits[10:16] - Specifies the ACK NO
> + bit[17] - Specifies DMA request high enable (HIEN)
> + bit[18] - Specifies DMA request detection type (LVL)
> + bits[19:21] - Specifies DMAACK output mode (AM)
> + bit[22] - Specifies Transfer Mode (TM)
>
> dma-channels:
> const: 16
> @@ -80,12 +93,29 @@ properties:
> items:
> - description: Reset for DMA ARESETN reset terminal
> - description: Reset for DMA RST_ASYNC reset terminal
> + minItems: 1
>
> reset-names:
> items:
> - const: arst
> - const: rst_async
>
> + renesas,icu:
> + description:
> + On the RZ/V2H(P) SoC configures the ICU to which the DMAC is connected to.
> + It must contain the phandle to the ICU, and the index of the DMAC as seen
> + from the ICU (e.g. parameter k from register ICU_DMkSELy).
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + - items:
> + - description: phandle to the ICU node.
> + - description: The DMAC index.
> + 4 for DMAC0
> + 0 for DMAC1
> + 1 for DMAC2
> + 2 for DMAC3
> + 3 for DMAC4
> +
> required:
> - compatible
> - reg
> @@ -98,27 +128,62 @@ allOf:
> - $ref: dma-controller.yaml#
>
> - if:
> - not:
> - properties:
> - compatible:
> - contains:
> - enum:
> - - renesas,r7s72100-dmac
> + properties:
> + compatible:
> + contains:
> + const: renesas,r9a09g057-dmac
> then:
> + properties:
> + reg:
> + maxItems: 1
> + clocks:
> + maxItems: 1
> + resets:
> + maxItems: 1
> +
> + clock-names: false
> + reset-names: false
> +
> required:
> - clocks
> - - clock-names
> - power-domains
> + - renesas,icu
> - resets
> - - reset-names
>
> else:
> - properties:
> - clocks: false
> - clock-names: false
> - power-domains: false
> - resets: false
> - reset-names: false
> + if:
Please try to avoid nesting if/then/else. Not sure that's easy or not
here. This diff is hard to read.
> + not:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,r7s72100-dmac
> + then:
> + properties:
> + reg:
> + minItems: 2
> + clocks:
> + minItems: 2
> + resets:
> + minItems: 2
> +
> + renesas,icu: false
> +
> + required:
> + - clocks
> + - clock-names
> + - power-domains
> + - resets
> + - reset-names
> +
> + else:
> + properties:
> + clocks: false
> + clock-names: false
> + power-domains: false
> + resets: false
> + reset-names: false
> + renesas,icu: false
>
> additionalProperties: false
>
> @@ -164,3 +229,40 @@ examples:
> #dma-cells = <1>;
> dma-channels = <16>;
> };
> +
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> + dmac0: dma-controller@11400000 {
> + compatible = "renesas,r9a09g057-dmac";
Is this example really different enough from the others to need it? I
would drop it.
Rob
^ permalink raw reply
* Re: [RFC PATCH 0/5] drm/panthor: Protected mode support for Mali CSF GPUs
From: Boris Brezillon @ 2025-02-11 14:32 UTC (permalink / raw)
To: Maxime Ripard
Cc: Nicolas Dufresne, Florent Tomasin, Vinod Koul, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Steven Price, Liviu Dudau,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
T . J . Mercier, Christian König, Matthias Brugger,
AngeloGioacchino Del Regno, Yong Wu, dmaengine, devicetree,
linux-kernel, dri-devel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, nd, Akash Goel
In-Reply-To: <20250211-robust-lush-skink-0dcc5b@houat>
On Tue, 11 Feb 2025 14:46:56 +0100
Maxime Ripard <mripard@kernel.org> wrote:
> Hi Boris,
>
> On Fri, Feb 07, 2025 at 04:02:53PM +0100, Boris Brezillon wrote:
> > Sorry for joining the party late, a couple of comments to back Akash
> > and Nicolas' concerns.
> >
> > On Wed, 05 Feb 2025 13:14:14 -0500
> > Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> >
> > > Le mercredi 05 février 2025 à 15:52 +0100, Maxime Ripard a écrit :
> > > > On Mon, Feb 03, 2025 at 04:43:23PM +0000, Florent Tomasin wrote:
> > > > > Hi Maxime, Nicolas
> > > > >
> > > > > On 30/01/2025 17:47, Nicolas Dufresne wrote:
> > > > > > Le jeudi 30 janvier 2025 à 17:38 +0100, Maxime Ripard a écrit :
> > > > > > > Hi Nicolas,
> > > > > > >
> > > > > > > On Thu, Jan 30, 2025 at 10:59:56AM -0500, Nicolas Dufresne wrote:
> > > > > > > > Le jeudi 30 janvier 2025 à 14:46 +0100, Maxime Ripard a écrit :
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > I started to review it, but it's probably best to discuss it here.
> > > > > > > > >
> > > > > > > > > On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > This is a patch series covering the support for protected mode execution in
> > > > > > > > > > Mali Panthor CSF kernel driver.
> > > > > > > > > >
> > > > > > > > > > The Mali CSF GPUs come with the support for protected mode execution at the
> > > > > > > > > > HW level. This feature requires two main changes in the kernel driver:
> > > > > > > > > >
> > > > > > > > > > 1) Configure the GPU with a protected buffer. The system must provide a DMA
> > > > > > > > > > heap from which the driver can allocate a protected buffer.
> > > > > > > > > > It can be a carved-out memory or dynamically allocated protected memory region.
> > > > > > > > > > Some system includes a trusted FW which is in charge of the protected memory.
> > > > > > > > > > Since this problem is integration specific, the Mali Panthor CSF kernel
> > > > > > > > > > driver must import the protected memory from a device specific exporter.
> > > > > > > > >
> > > > > > > > > Why do you need a heap for it in the first place? My understanding of
> > > > > > > > > your series is that you have a carved out memory region somewhere, and
> > > > > > > > > you want to allocate from that carved out memory region your buffers.
> > > > > > > > >
> > > > > > > > > How is that any different from using a reserved-memory region, adding
> > > > > > > > > the reserved-memory property to the GPU device and doing all your
> > > > > > > > > allocation through the usual dma_alloc_* API?
> > > > > > > >
> > > > > > > > How do you then multiplex this region so it can be shared between
> > > > > > > > GPU/Camera/Display/Codec drivers and also userspace ?
> > > > > > >
> > > > > > > You could point all the devices to the same reserved memory region, and
> > > > > > > they would all allocate from there, including for their userspace-facing
> > > > > > > allocations.
> > > > > >
> > > > > > I get that using memory region is somewhat more of an HW description, and
> > > > > > aligned with what a DT is supposed to describe. One of the challenge is that
> > > > > > Mediatek heap proposal endup calling into their TEE, meaning knowing the region
> > > > > > is not that useful. You actually need the TEE APP guid and its IPC protocol. If
> > > > > > we can dell drivers to use a head instead, we can abstract that SoC specific
> > > > > > complexity. I believe each allocated addressed has to be mapped to a zone, and
> > > > > > that can only be done in the secure application. I can imagine similar needs
> > > > > > when the protection is done using some sort of a VM / hypervisor.
> > > > > >
> > > > > > Nicolas
> > > > > >
> > > > >
> > > > > The idea in this design is to abstract the heap management from the
> > > > > Panthor kernel driver (which consumes a DMA buffer from it).
> > > > >
> > > > > In a system, an integrator would have implemented a secure heap driver,
> > > > > and could be based on TEE or a carved-out memory with restricted access,
> > > > > or else. This heap driver would be responsible of implementing the
> > > > > logic to: allocate, free, refcount, etc.
> > > > >
> > > > > The heap would be retrieved by the Panthor kernel driver in order to
> > > > > allocate protected memory to load the FW and allow the GPU to enter/exit
> > > > > protected mode. This memory would not belong to a user space process.
> > > > > The driver allocates it at the time of loading the FW and initialization
> > > > > of the GPU HW. This is a device globally owned protected memory.
> > > >
> > > > The thing is, it's really not clear why you absolutely need to have the
> > > > Panthor driver involved there. It won't be transparent to userspace,
> > > > since you'd need an extra flag at allocation time, and the buffers
> > > > behave differently. If userspace has to be aware of it, what's the
> > > > advantage to your approach compared to just exposing a heap for those
> > > > secure buffers, and letting userspace allocate its buffers from there?
> > >
> > > Unless I'm mistaken, the Panthor driver loads its own firmware. Since loading
> > > the firmware requires placing the data in a protected memory region, and that
> > > this aspect has no exposure to userspace, how can Panthor not be implicated ?
> >
> > Right, the very reason we need protected memory early is because some
> > FW sections need to be allocated from the protected pool, otherwise the
> > TEE will fault as soon at the FW enters the so-called 'protected mode'.
>
> How does that work if you don't have some way to allocate the protected
> memory? You can still submit jobs to the GPU, but you can't submit /
> execute "protected jobs"?
Exactly.
>
> > Now, it's not impossible to work around this limitation. For instance,
> > we could load the FW without this protected section by default (what we
> > do right now), and then provide a DRM_PANTHOR_ENABLE_FW_PROT_MODE
> > ioctl that would take a GEM object imported from a dmabuf allocated
> > from the protected dma-heap by userspace. We can then reset the FW and
> > allow it to operate in protected mode after that point.
>
> Urgh, I'd rather avoid that dance if possible :)
Me too.
>
> > This approach has two downsides though:
> >
> > 1. We have no way of checking that the memory we're passed is actually
> > suitable for FW execution in a protected context. If we're passed
> > random memory, this will likely hang the platform as soon as we enter
> > protected mode.
>
> It's a current limitation of dma-buf in general, and you'd have the same
> issue right now if someone imports a buffer, or misconfigure the heap
> for a !protected heap.
>
> I'd really like to have some way to store some metadata in dma_buf, if
> only to tell that the buffer is protected.
The dma_buf has a pointer to its ops, so it should be relatively easy
to add an is_dma_buf_coming_from_this_heap() helper. Of course this
implies linking the consumer driver to the heap it's supposed to take
protected buffers from, which is basically the thing being discussed
here :-).
>
> I suspect you'd also need that if you do things like do protected video
> playback through a codec, get a protected frame, and want to import that
> into the GPU. Depending on how you allocate it, either the codec or the
> GPU or both will want to make sure it's protected.
If it's all allocated from a central "protected" heap (even if that
goes through the driver calling the dma_heap_alloc_buffer()), it
shouldn't be an issue.
>
> > 2. If the driver already boot the FW and exposed a DRI node, we might
> > have GPU workloads running, and doing a FW reset might incur a slight
> > delay in GPU jobs execution.
> >
> > I think #1 is a more general issue that applies to suspend buffers
> > allocated for GPU contexts too. If we expose ioctls where we take
> > protected memory buffers that can possibly lead to crashes if they are
> > not real protected memory regions, and we have no way to ensure the
> > memory is protected, we probably want to restrict these ioctls/modes to
> > some high-privilege CAP_SYS_.
> >
> > For #2, that's probably something we can live with, since it's a
> > one-shot thing. If it becomes an issue, we can even make sure we enable
> > the FW protected-mode before the GPU starts being used for real.
> >
> > This being said, I think the problem applies outside Panthor, and it
> > might be that the video codec can't reset the FW/HW block to switch to
> > protected mode as easily as Panthor.
> >
> > Note that there's also downsides to the reserved-memory node approach,
> > where some bootloader stage would ask the secure FW to reserve a
> > portion of mem and pass this through the DT. This sort of things tend to
> > be an integration mess, where you need all the pieces of the stack (TEE,
> > u-boot, MTK dma-heap driver, gbm, ...) to be at a certain version to
> > work properly. If we go the ioctl() way, we restrict the scope to the
> > TEE, gbm/mesa and the protected-dma-heap driver, which is still a lot,
> > but we've ripped the bootloader out of the equation at least.
>
> Yeah. I also think there's two discussions in parallel here:
>
> 1) Being able to allocate protected buffers from the driver
> 2) Exposing an interface to allocate those to userspace
>
> I'm not really convinced we need 2, but 1 is obviously needed from what
> you're saying.
I suspect we need #2 for GBM, still. But that's what dma-heaps are for,
so I don't think that's a problem.
^ permalink raw reply
* Re: (subset) [PATCH 4/9] dt-bindings: mfd: Convert fsl,mcu-mpc8349emitx binding to YAML
From: Lee Jones @ 2025-02-11 14:13 UTC (permalink / raw)
To: devicetree, linuxppc-dev, J. Neuschäfer
Cc: Scott Wood, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Damien Le Moal, Niklas Cassel,
Herbert Xu, David S. Miller, Lee Jones, Vinod Koul,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Helgaas, J. Neuschäfer,
Wim Van Sebroeck, Guenter Roeck, Mark Brown, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, linux-kernel, linux-ide,
linux-crypto, dmaengine, linux-pci, linux-watchdog, linux-spi,
linux-mtd
In-Reply-To: <20250126-ppcyaml-v1-4-50649f51c3dd@posteo.net>
On Sun, 26 Jan 2025 19:58:59 +0100, J. Neuschäfer wrote:
> Convert mcu-mpc8349emitx.txt to YAML and list the compatible strings
> currently in use.
>
>
Applied, thanks!
[4/9] dt-bindings: mfd: Convert fsl,mcu-mpc8349emitx binding to YAML
commit: 252c95ec3802a5f447df58253575b52aa741c1a1
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [RFC PATCH 0/5] drm/panthor: Protected mode support for Mali CSF GPUs
From: Maxime Ripard @ 2025-02-11 13:46 UTC (permalink / raw)
To: Boris Brezillon
Cc: Nicolas Dufresne, Florent Tomasin, Vinod Koul, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Steven Price, Liviu Dudau,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
T . J . Mercier, Christian König, Matthias Brugger,
AngeloGioacchino Del Regno, Yong Wu, dmaengine, devicetree,
linux-kernel, dri-devel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, nd, Akash Goel
In-Reply-To: <20250207160253.42551fb1@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 8826 bytes --]
Hi Boris,
On Fri, Feb 07, 2025 at 04:02:53PM +0100, Boris Brezillon wrote:
> Sorry for joining the party late, a couple of comments to back Akash
> and Nicolas' concerns.
>
> On Wed, 05 Feb 2025 13:14:14 -0500
> Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> > Le mercredi 05 février 2025 à 15:52 +0100, Maxime Ripard a écrit :
> > > On Mon, Feb 03, 2025 at 04:43:23PM +0000, Florent Tomasin wrote:
> > > > Hi Maxime, Nicolas
> > > >
> > > > On 30/01/2025 17:47, Nicolas Dufresne wrote:
> > > > > Le jeudi 30 janvier 2025 à 17:38 +0100, Maxime Ripard a écrit :
> > > > > > Hi Nicolas,
> > > > > >
> > > > > > On Thu, Jan 30, 2025 at 10:59:56AM -0500, Nicolas Dufresne wrote:
> > > > > > > Le jeudi 30 janvier 2025 à 14:46 +0100, Maxime Ripard a écrit :
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > I started to review it, but it's probably best to discuss it here.
> > > > > > > >
> > > > > > > > On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > This is a patch series covering the support for protected mode execution in
> > > > > > > > > Mali Panthor CSF kernel driver.
> > > > > > > > >
> > > > > > > > > The Mali CSF GPUs come with the support for protected mode execution at the
> > > > > > > > > HW level. This feature requires two main changes in the kernel driver:
> > > > > > > > >
> > > > > > > > > 1) Configure the GPU with a protected buffer. The system must provide a DMA
> > > > > > > > > heap from which the driver can allocate a protected buffer.
> > > > > > > > > It can be a carved-out memory or dynamically allocated protected memory region.
> > > > > > > > > Some system includes a trusted FW which is in charge of the protected memory.
> > > > > > > > > Since this problem is integration specific, the Mali Panthor CSF kernel
> > > > > > > > > driver must import the protected memory from a device specific exporter.
> > > > > > > >
> > > > > > > > Why do you need a heap for it in the first place? My understanding of
> > > > > > > > your series is that you have a carved out memory region somewhere, and
> > > > > > > > you want to allocate from that carved out memory region your buffers.
> > > > > > > >
> > > > > > > > How is that any different from using a reserved-memory region, adding
> > > > > > > > the reserved-memory property to the GPU device and doing all your
> > > > > > > > allocation through the usual dma_alloc_* API?
> > > > > > >
> > > > > > > How do you then multiplex this region so it can be shared between
> > > > > > > GPU/Camera/Display/Codec drivers and also userspace ?
> > > > > >
> > > > > > You could point all the devices to the same reserved memory region, and
> > > > > > they would all allocate from there, including for their userspace-facing
> > > > > > allocations.
> > > > >
> > > > > I get that using memory region is somewhat more of an HW description, and
> > > > > aligned with what a DT is supposed to describe. One of the challenge is that
> > > > > Mediatek heap proposal endup calling into their TEE, meaning knowing the region
> > > > > is not that useful. You actually need the TEE APP guid and its IPC protocol. If
> > > > > we can dell drivers to use a head instead, we can abstract that SoC specific
> > > > > complexity. I believe each allocated addressed has to be mapped to a zone, and
> > > > > that can only be done in the secure application. I can imagine similar needs
> > > > > when the protection is done using some sort of a VM / hypervisor.
> > > > >
> > > > > Nicolas
> > > > >
> > > >
> > > > The idea in this design is to abstract the heap management from the
> > > > Panthor kernel driver (which consumes a DMA buffer from it).
> > > >
> > > > In a system, an integrator would have implemented a secure heap driver,
> > > > and could be based on TEE or a carved-out memory with restricted access,
> > > > or else. This heap driver would be responsible of implementing the
> > > > logic to: allocate, free, refcount, etc.
> > > >
> > > > The heap would be retrieved by the Panthor kernel driver in order to
> > > > allocate protected memory to load the FW and allow the GPU to enter/exit
> > > > protected mode. This memory would not belong to a user space process.
> > > > The driver allocates it at the time of loading the FW and initialization
> > > > of the GPU HW. This is a device globally owned protected memory.
> > >
> > > The thing is, it's really not clear why you absolutely need to have the
> > > Panthor driver involved there. It won't be transparent to userspace,
> > > since you'd need an extra flag at allocation time, and the buffers
> > > behave differently. If userspace has to be aware of it, what's the
> > > advantage to your approach compared to just exposing a heap for those
> > > secure buffers, and letting userspace allocate its buffers from there?
> >
> > Unless I'm mistaken, the Panthor driver loads its own firmware. Since loading
> > the firmware requires placing the data in a protected memory region, and that
> > this aspect has no exposure to userspace, how can Panthor not be implicated ?
>
> Right, the very reason we need protected memory early is because some
> FW sections need to be allocated from the protected pool, otherwise the
> TEE will fault as soon at the FW enters the so-called 'protected mode'.
How does that work if you don't have some way to allocate the protected
memory? You can still submit jobs to the GPU, but you can't submit /
execute "protected jobs"?
> Now, it's not impossible to work around this limitation. For instance,
> we could load the FW without this protected section by default (what we
> do right now), and then provide a DRM_PANTHOR_ENABLE_FW_PROT_MODE
> ioctl that would take a GEM object imported from a dmabuf allocated
> from the protected dma-heap by userspace. We can then reset the FW and
> allow it to operate in protected mode after that point.
Urgh, I'd rather avoid that dance if possible :)
> This approach has two downsides though:
>
> 1. We have no way of checking that the memory we're passed is actually
> suitable for FW execution in a protected context. If we're passed
> random memory, this will likely hang the platform as soon as we enter
> protected mode.
It's a current limitation of dma-buf in general, and you'd have the same
issue right now if someone imports a buffer, or misconfigure the heap
for a !protected heap.
I'd really like to have some way to store some metadata in dma_buf, if
only to tell that the buffer is protected.
I suspect you'd also need that if you do things like do protected video
playback through a codec, get a protected frame, and want to import that
into the GPU. Depending on how you allocate it, either the codec or the
GPU or both will want to make sure it's protected.
> 2. If the driver already boot the FW and exposed a DRI node, we might
> have GPU workloads running, and doing a FW reset might incur a slight
> delay in GPU jobs execution.
>
> I think #1 is a more general issue that applies to suspend buffers
> allocated for GPU contexts too. If we expose ioctls where we take
> protected memory buffers that can possibly lead to crashes if they are
> not real protected memory regions, and we have no way to ensure the
> memory is protected, we probably want to restrict these ioctls/modes to
> some high-privilege CAP_SYS_.
>
> For #2, that's probably something we can live with, since it's a
> one-shot thing. If it becomes an issue, we can even make sure we enable
> the FW protected-mode before the GPU starts being used for real.
>
> This being said, I think the problem applies outside Panthor, and it
> might be that the video codec can't reset the FW/HW block to switch to
> protected mode as easily as Panthor.
>
> Note that there's also downsides to the reserved-memory node approach,
> where some bootloader stage would ask the secure FW to reserve a
> portion of mem and pass this through the DT. This sort of things tend to
> be an integration mess, where you need all the pieces of the stack (TEE,
> u-boot, MTK dma-heap driver, gbm, ...) to be at a certain version to
> work properly. If we go the ioctl() way, we restrict the scope to the
> TEE, gbm/mesa and the protected-dma-heap driver, which is still a lot,
> but we've ripped the bootloader out of the equation at least.
Yeah. I also think there's two discussions in parallel here:
1) Being able to allocate protected buffers from the driver
2) Exposing an interface to allocate those to userspace
I'm not really convinced we need 2, but 1 is obviously needed from what
you're saying.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 4/5] drm/panthor: Add support for protected memory allocation in panthor
From: Boris Brezillon @ 2025-02-11 11:20 UTC (permalink / raw)
To: Florent Tomasin
Cc: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Steven Price, Liviu Dudau, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T . J . Mercier,
Christian König, Matthias Brugger,
AngeloGioacchino Del Regno, Yong Wu, dmaengine, devicetree,
linux-kernel, dri-devel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, nd, Akash Goel
In-Reply-To: <20250211120448.3e89e75f@collabora.com>
On Tue, 11 Feb 2025 12:04:48 +0100
Boris Brezillon <boris.brezillon@collabora.com> wrote:
> > --- a/drivers/gpu/drm/panthor/panthor_gem.c
> > +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> > @@ -9,10 +9,14 @@
> >
> > #include <drm/panthor_drm.h>
> >
> > +#include <uapi/linux/dma-heap.h>
> > +
> > #include "panthor_device.h"
> > #include "panthor_gem.h"
> > #include "panthor_mmu.h"
> >
> > +MODULE_IMPORT_NS(DMA_BUF);
>
> Uh, that's ugly. If the consensus is to let panthor allocate
> its protected buffers from a heap, let's just add a dependency on
> DMABUF_HEAPS instead.
My bad, that one is required for dma_buf_put(). Should be
MODULE_IMPORT_NS("DMA_BUF");
though.
^ permalink raw reply
* Re: [RFC PATCH 4/5] drm/panthor: Add support for protected memory allocation in panthor
From: Boris Brezillon @ 2025-02-11 11:04 UTC (permalink / raw)
To: Florent Tomasin
Cc: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Steven Price, Liviu Dudau, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T . J . Mercier,
Christian König, Matthias Brugger,
AngeloGioacchino Del Regno, Yong Wu, dmaengine, devicetree,
linux-kernel, dri-devel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, nd, Akash Goel
In-Reply-To: <821252c96ab2ab3e2d8ef211a09f8b171719baaa.1738228114.git.florent.tomasin@arm.com>
On Thu, 30 Jan 2025 13:09:00 +0000
Florent Tomasin <florent.tomasin@arm.com> wrote:
> 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.
>
> To help with the review of this patch, here are important information
> about the Mali GPU protected mode support:
> - 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.
>
> To summarize, Mali GPUs require allocations of protected buffer
> objects at the kernel level.
>
> * How is the protected heap accessed by the Panthor driver?
> The driver will retrieve the protected heap using the name of the
> heap provided to the driver via the DTB as attribute.
> 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 in the
> DTB node.
>
> * How is the Panthor driver allocating from the heap?
> Panthor is calling the DMA heap allocation function
> and obtains a DMA buffer from it. This buffer is then
> registered to GEM via PRIME by importing the DMA buffer.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> ---
> drivers/gpu/drm/panthor/Kconfig | 1 +
> drivers/gpu/drm/panthor/panthor_device.c | 22 ++++++++++-
> drivers/gpu/drm/panthor/panthor_device.h | 7 ++++
> drivers/gpu/drm/panthor/panthor_fw.c | 36 +++++++++++++++--
> drivers/gpu/drm/panthor/panthor_fw.h | 2 +
> drivers/gpu/drm/panthor/panthor_gem.c | 49 ++++++++++++++++++++++--
> drivers/gpu/drm/panthor/panthor_gem.h | 16 +++++++-
> drivers/gpu/drm/panthor/panthor_heap.c | 2 +
> drivers/gpu/drm/panthor/panthor_sched.c | 5 ++-
> 9 files changed, 130 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
> index 55b40ad07f3b..c0208b886d9f 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_GEM_SHMEM_HELPER
> select DRM_GPUVM
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 00f7b8ce935a..1018e5c90a0e 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -4,7 +4,9 @@
> /* Copyright 2023 Collabora ltd. */
>
> #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>
> @@ -102,6 +104,9 @@ void panthor_device_unplug(struct panthor_device *ptdev)
> panthor_mmu_unplug(ptdev);
> panthor_gpu_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);
>
> @@ -172,6 +177,7 @@ int panthor_device_init(struct panthor_device *ptdev)
> u32 *dummy_page_virt;
> struct resource *res;
> struct page *p;
> + const char *protm_heap_name;
> int ret;
>
> ret = panthor_gpu_coherency_init(ptdev);
> @@ -246,9 +252,19 @@ int panthor_device_init(struct panthor_device *ptdev)
> return ret;
> }
>
> + /* If a protected heap is specified but not found, defer the probe until created */
> + if (!of_property_read_string(ptdev->base.dev->of_node, "protected-heap-name",
> + &protm_heap_name)) {
> + ptdev->protm.heap = dma_heap_find(protm_heap_name);
> + if (!ptdev->protm.heap) {
> + ret = -EPROBE_DEFER;
> + goto err_rpm_put;
> + }
> + }
> +
> ret = panthor_gpu_init(ptdev);
> if (ret)
> - goto err_rpm_put;
> + goto err_dma_heap_put;
>
> ret = panthor_mmu_init(ptdev);
> if (ret)
> @@ -286,6 +302,10 @@ int panthor_device_init(struct panthor_device *ptdev)
> err_unplug_gpu:
> panthor_gpu_unplug(ptdev);
>
> +err_dma_heap_put:
> + if (ptdev->protm.heap)
> + dma_heap_put(ptdev->protm.heap);
> +
> err_rpm_put:
> pm_runtime_put_sync_suspend(ptdev->base.dev);
> return ret;
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 0e68f5a70d20..406de9e888e2 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/sched.h>
> @@ -190,6 +191,12 @@ struct panthor_device {
>
> /** @fast_rate: Maximum device clock frequency. Set by DVFS */
> unsigned long fast_rate;
> +
> + /** @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 4a2e36504fea..7822af1533b4 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -458,6 +458,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);
> @@ -491,6 +492,28 @@ 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);
> +}
> +
> +/**
> + * 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, NULL if no protected heap, an ERR_PTR() otherwise.
> + */
> +struct panthor_kernel_bo *
> +panthor_fw_alloc_protm_suspend_buf_mem(struct panthor_device *ptdev, size_t size)
> +{
> + if (!ptdev->protm.heap)
> + return NULL;
> +
> + 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);
> }
> @@ -503,6 +526,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;
> + bool is_protm_section = false;
> u32 section_size;
> u32 name_len;
> int ret;
> @@ -541,10 +565,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 not be 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");
> + is_protm_section = true;
> }
>
> if (hdr.va.start == CSF_MCU_SHARED_REGION_START &&
> @@ -610,9 +637,10 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED;
>
> section->mem = panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev),
> - section_size,
> - DRM_PANTHOR_BO_NO_MMAP,
> - vm_map_flags, va);
> + section_size,
> + DRM_PANTHOR_BO_NO_MMAP,
> + (is_protm_section ? DRM_PANTHOR_KBO_PROTECTED_HEAP : 0),
> + vm_map_flags, va);
> 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 22448abde992..29042d0dc60c 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.h
> +++ b/drivers/gpu/drm/panthor/panthor_fw.h
> @@ -481,6 +481,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 8244a4e6c2a2..88caf928acd0 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -9,10 +9,14 @@
>
> #include <drm/panthor_drm.h>
>
> +#include <uapi/linux/dma-heap.h>
> +
> #include "panthor_device.h"
> #include "panthor_gem.h"
> #include "panthor_mmu.h"
>
> +MODULE_IMPORT_NS(DMA_BUF);
Uh, that's ugly. If the consensus is to let panthor allocate
its protected buffers from a heap, let's just add a dependency on
DMABUF_HEAPS instead.
> +
> static void panthor_gem_free_object(struct drm_gem_object *obj)
> {
> struct panthor_gem_object *bo = to_panthor_bo(obj);
> @@ -31,6 +35,7 @@ static void panthor_gem_free_object(struct drm_gem_object *obj)
> */
> void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> {
> + struct dma_buf *dma_bo = NULL;
> struct panthor_vm *vm;
> int ret;
>
> @@ -38,6 +43,10 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> return;
>
> vm = bo->vm;
> +
> + if (bo->flags & DRM_PANTHOR_KBO_PROTECTED_HEAP)
> + dma_bo = bo->obj->import_attach->dmabuf;
> +
> panthor_kernel_bo_vunmap(bo);
>
> if (drm_WARN_ON(bo->obj->dev,
> @@ -51,6 +60,9 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> panthor_vm_free_va(vm, &bo->va_node);
> drm_gem_object_put(bo->obj);
>
> + if (dma_bo)
> + dma_buf_put(dma_bo);
> +
> out_free_bo:
> panthor_vm_put(vm);
> kfree(bo);
> @@ -62,6 +74,7 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> * @vm: VM to map the GEM to. If NULL, the kernel object is not GPU mapped.
> * @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.
> @@ -72,9 +85,11 @@ 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,
> + size_t size, u32 bo_flags, u32 kbo_flags, u32 vm_map_flags,
Hm, I'm not convinced by this kbo_flags. How about we have a dedicated
panthor_kernel_protected_bo_create() helper that takes a dmabuf object
to import, and then we simply store this dmabuf in panthor_kernel_bo to
reflect the fact this is a protected BO.
> u64 gpu_va)
> {
> + struct dma_buf *dma_bo = NULL;
> + struct drm_gem_object *gem_obj = NULL;
> struct drm_gem_shmem_object *obj;
> struct panthor_kernel_bo *kbo;
> struct panthor_gem_object *bo;
> @@ -87,14 +102,38 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
> if (!kbo)
> return ERR_PTR(-ENOMEM);
>
> - obj = drm_gem_shmem_create(&ptdev->base, size);
> + if (kbo_flags & DRM_PANTHOR_KBO_PROTECTED_HEAP) {
> + if (!ptdev->protm.heap) {
> + ret = -EINVAL;
> + goto err_free_bo;
> + }
> +
> + dma_bo = dma_heap_buffer_alloc(ptdev->protm.heap, size,
> + DMA_HEAP_VALID_FD_FLAGS, DMA_HEAP_VALID_HEAP_FLAGS);
> + if (!dma_bo) {
> + ret = -ENOMEM;
> + goto err_free_bo;
> + }
> +
> + gem_obj = drm_gem_prime_import(&ptdev->base, dma_bo);
> + if (IS_ERR(gem_obj)) {
> + ret = PTR_ERR(gem_obj);
> + goto err_free_dma_bo;
> + }
> +
> + obj = to_drm_gem_shmem_obj(gem_obj);
> + } else {
> + obj = drm_gem_shmem_create(&ptdev->base, size);
> + }
> +
> if (IS_ERR(obj)) {
> ret = PTR_ERR(obj);
> - goto err_free_bo;
> + goto err_free_dma_bo;
> }
>
> bo = to_panthor_bo(&obj->base);
> kbo->obj = &obj->base;
> + kbo->flags = kbo_flags;
> bo->flags = bo_flags;
>
> /* The system and GPU MMU page size might differ, which becomes a
> @@ -124,6 +163,10 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
> err_put_obj:
> drm_gem_object_put(&obj->base);
>
> +err_free_dma_bo:
> + if (dma_bo)
> + dma_buf_put(dma_bo);
> +
> err_free_bo:
> kfree(kbo);
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> index e43021cf6d45..d4fe8ae9f0a8 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.h
> +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> @@ -13,6 +13,17 @@
>
> struct panthor_vm;
>
> +/**
> + * enum drm_panthor_kbo_flags - Kernel buffer object flags, passed at creation time
> + */
> +enum drm_panthor_kbo_flags {
> + /**
> + * @DRM_PANTHOR_KBO_PROTECTED_HEAP: The buffer object will be allocated
> + * from a DMA-Buf protected heap.
> + */
> + DRM_PANTHOR_KBO_PROTECTED_HEAP = (1 << 0),
> +};
> +
> /**
> * struct panthor_gem_object - Driver specific GEM object.
> */
> @@ -75,6 +86,9 @@ struct panthor_kernel_bo {
> * @kmap: Kernel CPU mapping of @gem.
> */
> void *kmap;
> +
> + /** @flags: Combination of drm_panthor_kbo_flags flags. */
> + u32 flags;
> };
>
> static inline
> @@ -138,7 +152,7 @@ panthor_kernel_bo_vunmap(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,
> + size_t size, u32 bo_flags, u32 kbo_flags, u32 vm_map_flags,
> u64 gpu_va);
>
> void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo);
> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c
> index 3796a9eb22af..5395f0d90360 100644
> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> @@ -146,6 +146,7 @@ static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
>
> chunk->bo = panthor_kernel_bo_create(ptdev, vm, heap->chunk_size,
> DRM_PANTHOR_BO_NO_MMAP,
> + 0,
> DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> PANTHOR_VM_KERNEL_AUTO_VA);
> if (IS_ERR(chunk->bo)) {
> @@ -549,6 +550,7 @@ panthor_heap_pool_create(struct panthor_device *ptdev, struct panthor_vm *vm)
>
> pool->gpu_contexts = panthor_kernel_bo_create(ptdev, vm, bosize,
> DRM_PANTHOR_BO_NO_MMAP,
> + 0,
> DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> PANTHOR_VM_KERNEL_AUTO_VA);
> if (IS_ERR(pool->gpu_contexts)) {
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index ef4bec7ff9c7..e260ed8aef5b 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3298,6 +3298,7 @@ group_create_queue(struct panthor_group *group,
> queue->ringbuf = panthor_kernel_bo_create(group->ptdev, group->vm,
> args->ringbuf_size,
> 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);
> @@ -3328,6 +3329,7 @@ group_create_queue(struct panthor_group *group,
> queue->profiling.slot_count *
> sizeof(struct panthor_job_profiling_data),
> 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);
> @@ -3435,7 +3437,7 @@ int panthor_group_create(struct panthor_file *pfile,
> }
>
> suspend_size = csg_iface->control->protm_suspend_size;
> - group->protm_suspend_buf = panthor_fw_alloc_suspend_buf_mem(ptdev, suspend_size);
> + group->protm_suspend_buf = panthor_fw_alloc_protm_suspend_buf_mem(ptdev, suspend_size);
This predates your patchset, but I think we should refrain from
allocating a protm suspend buffer if the context is not flagged as
protected. This involves extending the uAPI to pass a new flag to the
GROUP_CREATE ioctl (repurposing the pad field in
drm_panthor_group_create into a flag field, and defining an
drm_panthor_group_create_flags enum).
> if (IS_ERR(group->protm_suspend_buf)) {
> ret = PTR_ERR(group->protm_suspend_buf);
> group->protm_suspend_buf = NULL;
> @@ -3446,6 +3448,7 @@ int panthor_group_create(struct panthor_file *pfile,
> group_args->queues.count *
> sizeof(struct panthor_syncobj_64b),
> 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);
^ permalink raw reply
* Re: [PATCH v2 11/12] dt-bindings: nand: Add fsl,elbc-fcm-nand
From: Rob Herring @ 2025-02-11 0:01 UTC (permalink / raw)
To: J. Neuschäfer
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Krzysztof Kozlowski, Conor Dooley,
Damien Le Moal, Niklas Cassel, Herbert Xu, David S. Miller,
Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <20250207-ppcyaml-v2-11-8137b0c42526@posteo.net>
On Fri, Feb 07, 2025 at 10:30:28PM +0100, J. Neuschäfer wrote:
> Formalize the binding already supported by the fsl_elbc_nand.c driver
> and used in several device trees in arch/powerpc/boot/dts/.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>
> V2:
> - split out from fsl,elbc binding patch
> - constrain #address-cells and #size-cells
> - add a general description
> - use unevaluatedProperties=false instead of additionalProperties=false
> - fix property order to comply with dts coding style
> - include raw-nand-chip.yaml instead of nand-chip.yaml
Why? Doesn't look like you use anything from it. I think the correct
thing to use here is just mtd.yaml to pick up partitions.
> ---
> .../devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml | 68 ++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml b/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..1de97bb24fa4a83e2ea5d94ab822dd0e37baa102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/fsl,elbc-fcm-nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NAND flash attached to Freescale eLBC
> +
> +description:
> + The Freescale Enhanced Local Bus controller (eLBC) contains logic to
> + interface with NAND flash, called the NAND Flash Control Machine (FCM).
> + This binding describes flash attached to an eLBC using the FCM.
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net>
> +
> +allOf:
> + - $ref: raw-nand-chip.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - fsl,mpc8313-fcm-nand
> + - fsl,mpc8315-fcm-nand
> + - fsl,mpc8377-fcm-nand
> + - fsl,mpc8378-fcm-nand
> + - fsl,mpc8379-fcm-nand
> + - fsl,mpc8536-fcm-nand
> + - fsl,mpc8569-fcm-nand
> + - fsl,mpc8572-fcm-nand
> + - fsl,p1020-fcm-nand
> + - fsl,p1021-fcm-nand
> + - fsl,p1025-fcm-nand
> + - fsl,p2020-fcm-nand
> + - const: fsl,elbc-fcm-nand
> + - const: fsl,elbc-fcm-nand
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + localbus {
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + nand@1,0 {
> + compatible = "fsl,mpc8315-fcm-nand",
> + "fsl,elbc-fcm-nand";
> + reg = <0x1 0x0 0x2000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + };
> + };
>
> --
> 2.48.0.rc1.219.gb6b6757d772
>
^ permalink raw reply
* [PATCH] dmaengine: Remove device_prep_dma_imm_data from struct dma_device
From: Nathan Lynch via B4 Relay @ 2025-02-10 23:29 UTC (permalink / raw)
To: Vinod Koul; +Cc: dmaengine, linux-kernel, Nathan Lynch
From: Nathan Lynch <nathan.lynch@amd.com>
The device_prep_dma_imm_data() method isn't implemented or invoked by
any code since commit 80ade22c06ca ("misc: mic: remove the MIC drivers").
Remove it, shrinking struct dma_device by a few bytes.
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
include/linux/dmaengine.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index a360e0330436470bfea17f8df567ff785e4293ea..bb146c5ac3e4ccd7bc0afbf3b28e5b3d659ad62f 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -839,7 +839,6 @@ struct dma_filter {
* The function takes a buffer of size buf_len. The callback function will
* be called after period_len bytes have been transferred.
* @device_prep_interleaved_dma: Transfer expression in a generic way.
- * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
* @device_caps: May be used to override the generic DMA slave capabilities
* with per-channel specific ones
* @device_config: Pushes a new configuration to a channel, return 0 or an error
@@ -942,9 +941,6 @@ struct dma_device {
struct dma_async_tx_descriptor *(*device_prep_interleaved_dma)(
struct dma_chan *chan, struct dma_interleaved_template *xt,
unsigned long flags);
- struct dma_async_tx_descriptor *(*device_prep_dma_imm_data)(
- struct dma_chan *chan, dma_addr_t dst, u64 data,
- unsigned long flags);
void (*device_caps)(struct dma_chan *chan, struct dma_slave_caps *caps);
int (*device_config)(struct dma_chan *chan, struct dma_slave_config *config);
---
base-commit: 2c17e9ea0caa5555e31e154fa1b06260b816f5cc
change-id: 20250210-dmaengine-drop-imm-data-f7a8e749fcef
Best regards,
--
Nathan Lynch <nathan.lynch@amd.com>
^ permalink raw reply related
* Re: [PATCH v2 09/12] dt-bindings: memory-controllers: Convert fsl,elbc to YAML
From: Rob Herring @ 2025-02-10 21:53 UTC (permalink / raw)
To: Crystal Wood
Cc: j.ne, devicetree, linuxppc-dev, Krzysztof Kozlowski, imx,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Krzysztof Kozlowski, Conor Dooley,
Damien Le Moal, Niklas Cassel, Herbert Xu, David S. Miller,
Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd, Li Yang, John Ogness
In-Reply-To: <Z6kQpuQf5m-bXTyt@buserror.net>
On Sun, Feb 09, 2025 at 02:31:34PM -0600, Crystal Wood wrote:
> On Fri, Feb 07, 2025 at 10:30:26PM +0100, J. Neuschäfer via B4 Relay wrote:
> > From: "J. Neuschäfer" <j.ne@posteo.net>
> >
> > Convert the Freescale localbus controller bindings from text form to
> > YAML. The updated list of compatible strings reflects current usage
> > in arch/powerpc/boot/dts/, except that many existing device trees
> > erroneously specify "simple-bus" in addition to fsl,*elbc.
> >
> > Changes compared to the txt version:
> > - removed the board-control (fsl,mpc8272ads-bcsr) node because it only
> > appears in this example and nowhere else
> > - added a new example with NAND flash
> > - updated list of compatible strings
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
> >
> > V2:
> > - fix order of properties in examples, according to dts coding style
> > - move to Documentation/devicetree/bindings/memory-controllers
> > - clarify the commit message a tiny bit
> > - remove unnecessary multiline markers (|)
> > - define address format in patternProperties
> > - trim subject line (remove "binding")
> > - remove use of "simple-bus", because it's technically incorrect
>
> While I admit I haven't been following recent developments in this area,
> as someone who was involved when "simple-bus" was created (and was on the
> ePAPR committee that standardized it) I'm surprised to hear simple-bus
> being called "erroneous" or "technically incorrect" here.
Erroneous because the binding did not say "simple-bus" was used. Not
uncommon with the old .txt bindings.
Generally, if a bus has control registers or resources like clocks, then
we tend not to call them 'simple-bus'. And '"specific-bus",
"simple-bus"' gives some problems around what driver if any do you
bind to.
If you have chip selects, then you have config registers for those.
Not really "simple" if you ask me. That being said, you could keep
'simple-bus' here. I would tend to err on making the schema match the
actual .dts rather than updating the .dts files on older platforms like
these.
> For non-NAND devices this bus generally meets the definition of "an
> internal I/O bus that cannot be probed for devices" where "devices on the
> bus can be accessed directly without additional configuration
> required". NAND flash is an exception, but those devices have
> compatibles that are specific to the bus controller.
NAND bindings have evolved quite a bit if you haven't been paying
attention.
> The fact that the address encoding is non-linear is irrelevant; the
> addresses can still be translated using the standard "ranges" mechanism.
> This seems to be a disconnect between the schema verification and the way
> the compatible has previously been defined and used.
>
> And as a practical matter, unless I'm missing something (which I might be
> since I haven't been in devicetree-land for nearly a decade), Linux is
> relying on simple-bus to probe these devices. There is a driver that
> binds to the bus itself but that is just for error interrupts and NAND.
>
> You'd probably need something like commit 3e25f800afb82bd9e5f8 ("memory:
> fsl_ifc: populate child devices without relying on simple-bus") and the
> subsequent fix in dd8adc713b1656 ("memory: fsl_ifc: populate child
> nodes of buses and mfd devices")...
>
> I'm curious what the reasoning was for removing simple-bus from IFC. It
> seems that the schema verification also played a role in that:
> https://www.spinics.net/lists/devicetree/msg220418.html
If a kernel change is needed to support changed .dts files, then we
shouldn't be doing that here (being mature platforms). That would mean
new DTB will not work with existing kernels.
>
> ...but there's also the comment in 985ede63a045eabf3f9d ("dt-bindings:
> memory: fsl: convert ifc binding to yaml schema") that "this will help to
> enforce the correct probe order between parent device and child devices",
> but was that really not already guaranteed by the parent/child
> relationship (and again, it should only really matter for NAND except for
> the possibility of missing error reports during early boot)?
>
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - fsl,mpc8313-elbc
> > + - fsl,mpc8315-elbc
> > + - fsl,mpc8377-elbc
> > + - fsl,mpc8378-elbc
> > + - fsl,mpc8379-elbc
> > + - fsl,mpc8536-elbc
> > + - fsl,mpc8569-elbc
> > + - fsl,mpc8572-elbc
> > + - fsl,p1020-elbc
> > + - fsl,p1021-elbc
> > + - fsl,p1023-elbc
> > + - fsl,p2020-elbc
> > + - fsl,p2041-elbc
> > + - fsl,p3041-elbc
> > + - fsl,p4080-elbc
> > + - fsl,p5020-elbc
> > + - fsl,p5040-elbc
> > + - const: fsl,elbc
>
> Is it really necessary to list every single chip?
Yes. If they exist, they have to be documented.
>
> And then it would need to be updated when new ones came out? I know this
> particular line of chips is not going to see any new members at this
> point, but as far as the general approach goes...
>
> Does the schema validation complain if it sees an extra compatible it
> doesn't recognize? If so that's obnoxious.
Yes.
More annoying is having to boot and debug typos:
compatible = "foo,bar", "simplebus";
Rob
^ permalink raw reply
* Re: [PATCH v2 06/12] dt-bindings: pci: Convert fsl,mpc83xx-pcie to YAML
From: Rob Herring @ 2025-02-10 21:25 UTC (permalink / raw)
To: J. Neuschäfer
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Krzysztof Kozlowski, Conor Dooley,
Damien Le Moal, Niklas Cassel, Herbert Xu, David S. Miller,
Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <Z6fxo4j5a6ro0f1w@probook>
On Sun, Feb 09, 2025 at 12:06:59AM +0000, J. Neuschäfer wrote:
> On Fri, Feb 07, 2025 at 10:30:23PM +0100, J. Neuschäfer via B4 Relay wrote:
> > From: "J. Neuschäfer" <j.ne@posteo.net>
> >
> > Formalise the binding for the PCI controllers in the Freescale MPC8xxx
> > chip family. Information about PCI-X-specific properties was taken from
> > fsl,pci.txt. The examples were taken from mpc8315erdb.dts and
> > xpedite5200_xmon.dts.
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
> >
> > V2:
> > - merge fsl,pci.txt into fsl,mpc8xxx-pci.yaml
> > - regroup compatible strings, list single-item values in one enum
> > - trim subject line (remove "binding")
> > - fix property order to comply with dts coding style
> > ---
> > .../devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml | 115 +++++++++++++++++++++
> > Documentation/devicetree/bindings/pci/fsl,pci.txt | 27 -----
> > 2 files changed, 115 insertions(+), 27 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml b/Documentation/devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..57c5503cec47e6e90ed2b09835bfad10309db927
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/fsl,mpc8xxx-pci.yaml
> > @@ -0,0 +1,115 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/pci/fsl,mpc8xxx-pci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale MPC83xx PCI/PCI-X/PCIe controllers
> > +
> > +description: |
> > + Binding for the PCI/PCI-X/PCIe host bridges on MPC8xxx SoCs.
> > + See also: Documentation/devicetree/bindings/pci/fsl,pci.txt
>
> This is obviously a bit wrong; I ended up putting the information from
> fsl,pci.txt entirely under the fsl,pci-agent-force-enum property, but
> forgot to remove the reference to the old txt file.
Looks fine other than that.
Rob
^ permalink raw reply
* Re: [PATCH v2 11/12] dt-bindings: nand: Add fsl,elbc-fcm-nandy
From: Frank Li @ 2025-02-10 19:47 UTC (permalink / raw)
To: j.ne
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Damien Le Moal, Niklas Cassel, Herbert Xu,
David S. Miller, Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <20250207-ppcyaml-v2-11-8137b0c42526@posteo.net>
On Fri, Feb 07, 2025 at 10:30:28PM +0100, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> Formalize the binding already supported by the fsl_elbc_nand.c driver
> and used in several device trees in arch/powerpc/boot/dts/.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>
> V2:
> - split out from fsl,elbc binding patch
> - constrain #address-cells and #size-cells
> - add a general description
> - use unevaluatedProperties=false instead of additionalProperties=false
> - fix property order to comply with dts coding style
> - include raw-nand-chip.yaml instead of nand-chip.yaml
> ---
> .../devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml | 68 ++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml b/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..1de97bb24fa4a83e2ea5d94ab822dd0e37baa102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/fsl,elbc-fcm-nand.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/fsl,elbc-fcm-nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NAND flash attached to Freescale eLBC
> +
> +description:
> + The Freescale Enhanced Local Bus controller (eLBC) contains logic to
> + interface with NAND flash, called the NAND Flash Control Machine (FCM).
> + This binding describes flash attached to an eLBC using the FCM.
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net>
> +
> +allOf:
> + - $ref: raw-nand-chip.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - fsl,mpc8313-fcm-nand
> + - fsl,mpc8315-fcm-nand
> + - fsl,mpc8377-fcm-nand
> + - fsl,mpc8378-fcm-nand
> + - fsl,mpc8379-fcm-nand
> + - fsl,mpc8536-fcm-nand
> + - fsl,mpc8569-fcm-nand
> + - fsl,mpc8572-fcm-nand
> + - fsl,p1020-fcm-nand
> + - fsl,p1021-fcm-nand
> + - fsl,p1025-fcm-nand
> + - fsl,p2020-fcm-nand
> + - const: fsl,elbc-fcm-nand
> + - const: fsl,elbc-fcm-nand
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + localbus {
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + nand@1,0 {
> + compatible = "fsl,mpc8315-fcm-nand",
> + "fsl,elbc-fcm-nand";
> + reg = <0x1 0x0 0x2000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
needn't #address-cells and #size-cells because no children nodes.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Frank
> + };
> + };
>
> --
> 2.48.0.rc1.219.gb6b6757d772
>
>
^ permalink raw reply
* Re: [PATCH v2 10/12] dt-bindings: memory-controllers: Add fsl,elbc-gpcm-uio
From: Frank Li @ 2025-02-10 19:45 UTC (permalink / raw)
To: j.ne
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Damien Le Moal, Niklas Cassel, Herbert Xu,
David S. Miller, Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <20250207-ppcyaml-v2-10-8137b0c42526@posteo.net>
On Fri, Feb 07, 2025 at 10:30:27PM +0100, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> Formalize the binding already supported by the uio_fsl_elbc_gpcm.c
> driver.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>
> V2:
> - split out from fsl,elbc patch
> - add description
> - remove "device_type" property
> - move to bindings/memory-controllers
> ---
> .../memory-controllers/fsl,elbc-gpcm-uio.yaml | 59 ++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl,elbc-gpcm-uio.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl,elbc-gpcm-uio.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..381584b400a0ad98c6d9e0b38f2877d44603ed84
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/fsl,elbc-gpcm-uio.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/fsl,elbc-gpcm-uio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Userspace I/O interface for Freescale eLBC devices
> +
> +description:
> + The Freescale Enhanced Local Bus controller (eLBC) supports flexible access
> + to memory devices, through the General-Purpose Chip-select Machine (GPCM).
> + The purpose of this binding is to designate devices attached to eLBC/GPMC for
> + use by userspace.
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net>
> +
> +properties:
> + compatible:
> + const: fsl,elbc-gpcm-uio
> +
> + reg:
> + maxItems: 1
> +
> + elbc-gpcm-br:
> + description: Base Register (BR) value to set
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + elbc-gpcm-or:
> + description: Option Register (OR) value to set
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + interrupts:
> + maxItems: 1
> +
> + uio_name:
> + $ref: /schemas/types.yaml#/definitions/string
> +
> +required:
> + - compatible
> + - reg
> + - elbc-gpcm-br
> + - elbc-gpcm-or
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + localbus {
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + simple-periph@2,0 {
> + compatible = "fsl,elbc-gpcm-uio";
> + reg = <0x2 0x0 0x10000>;
> + elbc-gpcm-br = <0xfd810800>;
> + elbc-gpcm-or = <0xffff09f7>;
> + };
> + };
>
> --
> 2.48.0.rc1.219.gb6b6757d772
>
>
^ permalink raw reply
* Re: [PATCH v2 08/12] dt-bindings: spi: Convert Freescale SPI bindings to YAML
From: Frank Li @ 2025-02-10 19:42 UTC (permalink / raw)
To: j.ne
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Damien Le Moal, Niklas Cassel, Herbert Xu,
David S. Miller, Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <20250207-ppcyaml-v2-8-8137b0c42526@posteo.net>
On Fri, Feb 07, 2025 at 10:30:25PM +0100, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> fsl-spi.txt contains the bindings for the fsl,spi and fsl,espi
> contollers. Convert them to YAML.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>
> V2:
> - add missing end-of-document ("...") markers
> - add missing constraints to interrupts, fsl,espi-num-chipselects,
> fsl,csbef and fsl,csaft properties
> - remove unnecessary type from clock-frequency property
> - fix property order to comply with dts coding style
> ---
> .../devicetree/bindings/spi/fsl,espi.yaml | 64 +++++++++++++++++++
> Documentation/devicetree/bindings/spi/fsl,spi.yaml | 73 ++++++++++++++++++++++
> Documentation/devicetree/bindings/spi/fsl-spi.txt | 62 ------------------
> 3 files changed, 137 insertions(+), 62 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/fsl,espi.yaml b/Documentation/devicetree/bindings/spi/fsl,espi.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..c504b7957dde39086ef7d7a7550d6169cf5ec407
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/fsl,espi.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/fsl,espi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale eSPI (Enhanced Serial Peripheral Interface) controller
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net>
> +
> +properties:
> + compatible:
> + const: fsl,mpc8536-espi
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + fsl,espi-num-chipselects:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 1, 4 ]
> + description: The number of the chipselect signals.
> +
> + fsl,csbef:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + description: Chip select assertion time in bits before frame starts
> +
> + fsl,csaft:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + description: Chip select negation time in bits after frame ends
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - fsl,espi-num-chipselects
> +
> +allOf:
> + - $ref: spi-controller.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + spi@110000 {
> + compatible = "fsl,mpc8536-espi";
> + reg = <0x110000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <53 0x2>;
Use predefine's irq type macro.
> + interrupt-parent = <&mpic>;
> + fsl,espi-num-chipselects = <4>;
> + fsl,csbef = <1>;
> + fsl,csaft = <1>;
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/spi/fsl,spi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..db65c0560c32f32324a2aaaf53c0044a4b56f3d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/fsl,spi.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/fsl,spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale SPI (Serial Peripheral Interface) controller
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net>
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,spi
> + - aeroflexgaisler,spictrl
> +
> + reg:
> + maxItems: 1
> +
> + cell-index:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + QE SPI subblock index.
> + 0: QE subblock SPI1
> + 1: QE subblock SPI2
> +
> + mode:
> + description: SPI operation mode
> + enum:
> + - cpu
> + - cpu-qe
> +
> + interrupts:
> + maxItems: 1
> +
> + clock-frequency:
> + description: input clock frequency to non FSL_SOC cores
> +
> + cs-gpios: true
> +
> + fsl,spisel_boot:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + For the MPC8306 and MPC8309, specifies that the SPISEL_BOOT signal is used
> + as chip select for a slave device. Use reg = <number of gpios> in the
> + corresponding child node, i.e. 0 if the cs-gpios property is not present.
> +
> +required:
> + - compatible
> + - reg
> + - mode
> + - interrupts
> +
> +allOf:
> + - $ref: spi-controller.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + spi@4c0 {
> + compatible = "fsl,spi";
> + reg = <0x4c0 0x40>;
> + cell-index = <0>;
> + interrupts = <82 0>;
> + interrupt-parent = <&intc>;
> + mode = "cpu";
> + cs-gpios = <&gpio 18 1 // device reg=<0>
> + &gpio 19 1>; // device reg=<1>
use predefine gpio level macro
Frank
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/spi/fsl-spi.txt b/Documentation/devicetree/bindings/spi/fsl-spi.txt
> deleted file mode 100644
> index 0654380eb7515d8bda80eea1486e77b939ac38d8..0000000000000000000000000000000000000000
> --- a/Documentation/devicetree/bindings/spi/fsl-spi.txt
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -* SPI (Serial Peripheral Interface)
> -
> -Required properties:
> -- cell-index : QE SPI subblock index.
> - 0: QE subblock SPI1
> - 1: QE subblock SPI2
> -- compatible : should be "fsl,spi" or "aeroflexgaisler,spictrl".
> -- mode : the SPI operation mode, it can be "cpu" or "cpu-qe".
> -- reg : Offset and length of the register set for the device
> -- interrupts : <a b> where a is the interrupt number and b is a
> - field that represents an encoding of the sense and level
> - information for the interrupt. This should be encoded based on
> - the information in section 2) depending on the type of interrupt
> - controller you have.
> -- clock-frequency : input clock frequency to non FSL_SOC cores
> -
> -Optional properties:
> -- cs-gpios : specifies the gpio pins to be used for chipselects.
> - The gpios will be referred to as reg = <index> in the SPI child nodes.
> - If unspecified, a single SPI device without a chip select can be used.
> -- fsl,spisel_boot : for the MPC8306 and MPC8309, specifies that the
> - SPISEL_BOOT signal is used as chip select for a slave device. Use
> - reg = <number of gpios> in the corresponding child node, i.e. 0 if
> - the cs-gpios property is not present.
> -
> -Example:
> - spi@4c0 {
> - cell-index = <0>;
> - compatible = "fsl,spi";
> - reg = <4c0 40>;
> - interrupts = <82 0>;
> - interrupt-parent = <700>;
> - mode = "cpu";
> - cs-gpios = <&gpio 18 1 // device reg=<0>
> - &gpio 19 1>; // device reg=<1>
> - };
> -
> -
> -* eSPI (Enhanced Serial Peripheral Interface)
> -
> -Required properties:
> -- compatible : should be "fsl,mpc8536-espi".
> -- reg : Offset and length of the register set for the device.
> -- interrupts : should contain eSPI interrupt, the device has one interrupt.
> -- fsl,espi-num-chipselects : the number of the chipselect signals.
> -
> -Optional properties:
> -- fsl,csbef: chip select assertion time in bits before frame starts
> -- fsl,csaft: chip select negation time in bits after frame ends
> -
> -Example:
> - spi@110000 {
> - #address-cells = <1>;
> - #size-cells = <0>;
> - compatible = "fsl,mpc8536-espi";
> - reg = <0x110000 0x1000>;
> - interrupts = <53 0x2>;
> - interrupt-parent = <&mpic>;
> - fsl,espi-num-chipselects = <4>;
> - fsl,csbef = <1>;
> - fsl,csaft = <1>;
> - };
>
> --
> 2.48.0.rc1.219.gb6b6757d772
>
>
^ permalink raw reply
* Re: [PATCH v2 05/12] dt-bindings: dma: Convert fsl,elo*-dma to YAML
From: Frank Li @ 2025-02-10 19:39 UTC (permalink / raw)
To: j.ne
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Damien Le Moal, Niklas Cassel, Herbert Xu,
David S. Miller, Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <20250207-ppcyaml-v2-5-8137b0c42526@posteo.net>
On Fri, Feb 07, 2025 at 10:30:22PM +0100, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> The devicetree bindings for Freescale DMA engines have so far existed as
> a text file. This patch converts them to YAML, and specifies all the
> compatible strings currently in use in arch/powerpc/boot/dts.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>
> V2:
> - remove unnecessary multiline markers
> - fix additionalProperties to always be false
> - add description/maxItems to interrupts
> - add missing #address-cells/#size-cells properties
> - convert "Note on DMA channel compatible properties" to YAML by listing
> fsl,ssi-dma-channel as a valid compatible value
> - fix property ordering in examples: compatible and reg come first
> - add missing newlines in examples
> - trim subject line (remove "bindings")
> ---
> .../devicetree/bindings/dma/fsl,elo-dma.yaml | 140 ++++++++++++++
> .../devicetree/bindings/dma/fsl,elo3-dma.yaml | 123 +++++++++++++
> .../devicetree/bindings/dma/fsl,eloplus-dma.yaml | 134 ++++++++++++++
> .../devicetree/bindings/powerpc/fsl/dma.txt | 204 ---------------------
> 4 files changed, 397 insertions(+), 204 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml b/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..3d8be9973fb98891a73cb701c1f983a63f444837
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/fsl,elo-dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Elo DMA Controller
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net>
> +
> +description:
> + This is a little-endian 4-channel DMA controller, used in Freescale mpc83xx
> + series chips such as mpc8315, mpc8349, mpc8379 etc.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - fsl,mpc8313-dma
> + - fsl,mpc8315-dma
> + - fsl,mpc8323-dma
> + - fsl,mpc8347-dma
> + - fsl,mpc8349-dma
> + - fsl,mpc8360-dma
> + - fsl,mpc8377-dma
> + - fsl,mpc8378-dma
> + - fsl,mpc8379-dma
> + - const: fsl,elo-dma
> +
> + reg:
> + maxItems: 1
> + description:
> + DMA General Status Register, i.e. DGSR which contains status for
> + all the 4 DMA channels.
needn't maxItems
items:
- description: DMA ...
> +
> + cell-index:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Controller index. 0 for controller @ 0x8100.
> +
> + ranges: true
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 1
> +
> + interrupts:
> + maxItems: 1
> + description: Controller interrupt.
Needn't description because no any additional informaiton.
> +
> +required:
> + - compatible
> + - reg
> +
> +patternProperties:
> + "^dma-channel@.*$":
> + type: object
> + additionalProperties: false
> +
> + properties:
> + compatible:
> + oneOf:
> + # native DMA channel
> + - items:
> + - enum:
> + - fsl,mpc8315-dma-channel
> + - fsl,mpc8323-dma-channel
> + - fsl,mpc8347-dma-channel
> + - fsl,mpc8349-dma-channel
> + - fsl,mpc8360-dma-channel
> + - fsl,mpc8377-dma-channel
> + - fsl,mpc8378-dma-channel
> + - fsl,mpc8379-dma-channel
> + - const: fsl,elo-dma-channel
> +
> + # audio DMA channel, see fsl,ssi.yaml
> + - const: fsl,ssi-dma-channel
> +
> + reg:
> + maxItems: 1
> +
> + cell-index:
> + description: DMA channel index starts at 0.
> +
> + interrupts:
> + maxItems: 1
> + description:
> + Per-channel interrupt. Only necessary if no controller interrupt has
> + been provided.
> +
> +additionalProperties: false
Need ref to dma-common.yaml?
> +
> +examples:
> + - |
> + dma@82a8 {
> + compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
> + reg = <0x82a8 4>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x8100 0x1a4>;
> + interrupt-parent = <&ipic>;
> + interrupts = <71 8>;
> + cell-index = <0>;
> +
> + dma-channel@0 {
> + compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
> + reg = <0 0x80>;
> + cell-index = <0>;
> + interrupt-parent = <&ipic>;
> + interrupts = <71 8>;
'8', use predefine MACRO for irq type.
Frank
> + };
> +
> + dma-channel@80 {
> + compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
> + reg = <0x80 0x80>;
> + cell-index = <1>;
> + interrupt-parent = <&ipic>;
> + interrupts = <71 8>;
> + };
> +
> + dma-channel@100 {
> + compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
> + reg = <0x100 0x80>;
> + cell-index = <2>;
> + interrupt-parent = <&ipic>;
> + interrupts = <71 8>;
> + };
> +
> + dma-channel@180 {
> + compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
> + reg = <0x180 0x80>;
> + cell-index = <3>;
> + interrupt-parent = <&ipic>;
> + interrupts = <71 8>;
> + };
> + };
> +
...
> --
> 2.48.0.rc1.219.gb6b6757d772
>
>
^ permalink raw reply
* Re: [PATCH v2 03/12] dt-bindings: crypto: Convert fsl,sec-2.0 to YAMLy
From: Frank Li @ 2025-02-10 19:30 UTC (permalink / raw)
To: j.ne
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Damien Le Moal, Niklas Cassel, Herbert Xu,
David S. Miller, Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
J. Neuschäfer, Wim Van Sebroeck, Guenter Roeck, Mark Brown,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, linux-ide, linux-crypto, dmaengine, linux-pci,
linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <20250207-ppcyaml-v2-3-8137b0c42526@posteo.net>
On Fri, Feb 07, 2025 at 10:30:20PM +0100, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> Convert the Freescale security engine (crypto accelerator) binding from
> text form to YAML. The list of compatible strings reflects what was
> previously described in prose; not all combinations occur in existing
> devicetrees.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>
> V2:
> - several improvements suggested by Rob Herring:
> - remove unnecessary multiline markers
> - constrain fsl,num-channels to enum: [1,4]
> - constrain fsl,channel-fifo-len to plausible limits
> - constrain fsl,exec-units-mask to maximum=0xfff
> - trim subject line (remove "binding")
> ---
> .../devicetree/bindings/crypto/fsl,sec2.0.yaml | 142 +++++++++++++++++++++
> .../devicetree/bindings/crypto/fsl-sec2.txt | 65 ----------
> 2 files changed, 142 insertions(+), 65 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/fsl,sec2.0.yaml b/Documentation/devicetree/bindings/crypto/fsl,sec2.0.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..0b82f3b68b5f82e7fb52d292a623d452c1cdb059
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/fsl,sec2.0.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/fsl,sec2.0.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale SoC SEC Security Engines versions 1.x-2.x-3.x
> +
> +maintainers:
> + - J. Neuschäfer <j.ne@posteo.net.
> +
> +properties:
> + compatible:
> + description:
> + Should contain entries for this and backward compatible SEC versions,
> + high to low. Warning - SEC1 and SEC2 are mutually exclusive.
> + oneOf:
> + - items:
> + - const: fsl,sec3.3
> + - const: fsl,sec3.1
> + - const: fsl,sec3.0
> + - const: fsl,sec2.4
> + - const: fsl,sec2.2
> + - const: fsl,sec2.1
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec3.1
> + - const: fsl,sec3.0
> + - const: fsl,sec2.4
> + - const: fsl,sec2.2
> + - const: fsl,sec2.1
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec3.0
> + - const: fsl,sec2.4
> + - const: fsl,sec2.2
> + - const: fsl,sec2.1
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec2.4
> + - const: fsl,sec2.2
> + - const: fsl,sec2.1
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec2.2
> + - const: fsl,sec2.1
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec2.1
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec2.0
> + - items:
> + - const: fsl,sec1.2
> + - const: fsl,sec1.0
> + - items:
> + - const: fsl,sec1.0
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + fsl,num-channels:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 1, 4 ]
> + description: An integer representing the number of channels available.
> +
> + fsl,channel-fifo-len:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 100
> + description:
> + An integer representing the number of descriptor pointers each channel
> + fetch fifo can hold.
> +
> + fsl,exec-units-mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 0xfff
> + description: |
> + The bitmask representing what execution units (EUs) are available.
> + EU information should be encoded following the SEC's Descriptor Header
> + Dword EU_SEL0 field documentation, i.e. as follows:
> +
> + bit 0 = reserved - should be 0
> + bit 1 = set if SEC has the ARC4 EU (AFEU)
> + bit 2 = set if SEC has the DES/3DES EU (DEU)
> + bit 3 = set if SEC has the message digest EU (MDEU/MDEU-A)
> + bit 4 = set if SEC has the random number generator EU (RNG)
> + bit 5 = set if SEC has the public key EU (PKEU)
> + bit 6 = set if SEC has the AES EU (AESU)
> + bit 7 = set if SEC has the Kasumi EU (KEU)
> + bit 8 = set if SEC has the CRC EU (CRCU)
> + bit 11 = set if SEC has the message digest EU extended alg set (MDEU-B)
> +
> + remaining bits are reserved for future SEC EUs.
> +
> + fsl,descriptor-types-mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + The bitmask representing what descriptors are available. Descriptor type
> + information should be encoded following the SEC's Descriptor Header Dword
> + DESC_TYPE field documentation, i.e. as follows:
> +
> + bit 0 = set if SEC supports the aesu_ctr_nonsnoop desc. type
> + bit 1 = set if SEC supports the ipsec_esp descriptor type
> + bit 2 = set if SEC supports the common_nonsnoop desc. type
> + bit 3 = set if SEC supports the 802.11i AES ccmp desc. type
> + bit 4 = set if SEC supports the hmac_snoop_no_afeu desc. type
> + bit 5 = set if SEC supports the srtp descriptor type
> + bit 6 = set if SEC supports the non_hmac_snoop_no_afeu desc.type
> + bit 7 = set if SEC supports the pkeu_assemble descriptor type
> + bit 8 = set if SEC supports the aesu_key_expand_output desc.type
> + bit 9 = set if SEC supports the pkeu_ptmul descriptor type
> + bit 10 = set if SEC supports the common_nonsnoop_afeu desc. type
> + bit 11 = set if SEC supports the pkeu_ptadd_dbl descriptor type
> +
> + ..and so on and so forth.
> +
> +required:
> + - compatible
> + - reg
> + - fsl,num-channels
> + - fsl,channel-fifo-len
> + - fsl,exec-units-mask
> + - fsl,descriptor-types-mask
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + /* MPC8548E */
> + crypto@30000 {
> + compatible = "fsl,sec2.1", "fsl,sec2.0";
> + reg = <0x30000 0x10000>;
> + interrupts = <29 2>;
> + interrupt-parent = <&mpic>;
> + fsl,num-channels = <4>;
> + fsl,channel-fifo-len = <24>;
> + fsl,exec-units-mask = <0xfe>;
> + fsl,descriptor-types-mask = <0x12b0ebf>;
> + };
> diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec2.txt b/Documentation/devicetree/bindings/crypto/fsl-sec2.txt
> deleted file mode 100644
> index 125f155d00d052eec7d5093b5c5076cbe720417f..0000000000000000000000000000000000000000
> --- a/Documentation/devicetree/bindings/crypto/fsl-sec2.txt
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -Freescale SoC SEC Security Engines versions 1.x-2.x-3.x
> -
> -Required properties:
> -
> -- compatible : Should contain entries for this and backward compatible
> - SEC versions, high to low, e.g., "fsl,sec2.1", "fsl,sec2.0" (SEC2/3)
> - e.g., "fsl,sec1.2", "fsl,sec1.0" (SEC1)
> - warning: SEC1 and SEC2 are mutually exclusive
> -- reg : Offset and length of the register set for the device
> -- interrupts : the SEC's interrupt number
> -- fsl,num-channels : An integer representing the number of channels
> - available.
> -- fsl,channel-fifo-len : An integer representing the number of
> - descriptor pointers each channel fetch fifo can hold.
> -- fsl,exec-units-mask : The bitmask representing what execution units
> - (EUs) are available. It's a single 32-bit cell. EU information
> - should be encoded following the SEC's Descriptor Header Dword
> - EU_SEL0 field documentation, i.e. as follows:
> -
> - bit 0 = reserved - should be 0
> - bit 1 = set if SEC has the ARC4 EU (AFEU)
> - bit 2 = set if SEC has the DES/3DES EU (DEU)
> - bit 3 = set if SEC has the message digest EU (MDEU/MDEU-A)
> - bit 4 = set if SEC has the random number generator EU (RNG)
> - bit 5 = set if SEC has the public key EU (PKEU)
> - bit 6 = set if SEC has the AES EU (AESU)
> - bit 7 = set if SEC has the Kasumi EU (KEU)
> - bit 8 = set if SEC has the CRC EU (CRCU)
> - bit 11 = set if SEC has the message digest EU extended alg set (MDEU-B)
> -
> -remaining bits are reserved for future SEC EUs.
> -
> -- fsl,descriptor-types-mask : The bitmask representing what descriptors
> - are available. It's a single 32-bit cell. Descriptor type information
> - should be encoded following the SEC's Descriptor Header Dword DESC_TYPE
> - field documentation, i.e. as follows:
> -
> - bit 0 = set if SEC supports the aesu_ctr_nonsnoop desc. type
> - bit 1 = set if SEC supports the ipsec_esp descriptor type
> - bit 2 = set if SEC supports the common_nonsnoop desc. type
> - bit 3 = set if SEC supports the 802.11i AES ccmp desc. type
> - bit 4 = set if SEC supports the hmac_snoop_no_afeu desc. type
> - bit 5 = set if SEC supports the srtp descriptor type
> - bit 6 = set if SEC supports the non_hmac_snoop_no_afeu desc.type
> - bit 7 = set if SEC supports the pkeu_assemble descriptor type
> - bit 8 = set if SEC supports the aesu_key_expand_output desc.type
> - bit 9 = set if SEC supports the pkeu_ptmul descriptor type
> - bit 10 = set if SEC supports the common_nonsnoop_afeu desc. type
> - bit 11 = set if SEC supports the pkeu_ptadd_dbl descriptor type
> -
> - ..and so on and so forth.
> -
> -Example:
> -
> - /* MPC8548E */
> - crypto@30000 {
> - compatible = "fsl,sec2.1", "fsl,sec2.0";
> - reg = <0x30000 0x10000>;
> - interrupts = <29 2>;
> - interrupt-parent = <&mpic>;
> - fsl,num-channels = <4>;
> - fsl,channel-fifo-len = <24>;
> - fsl,exec-units-mask = <0xfe>;
> - fsl,descriptor-types-mask = <0x12b0ebf>;
> - };
>
> --
> 2.48.0.rc1.219.gb6b6757d772
>
>
^ permalink raw reply
* Re: [PATCH v2 00/12] YAML conversion of several Freescale/PowerPC DT bindings
From: Mark Brown @ 2025-02-10 16:19 UTC (permalink / raw)
To: J. Neuschäfer
Cc: devicetree, linuxppc-dev, Krzysztof Kozlowski, imx, Scott Wood,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Damien Le Moal, Niklas Cassel, Herbert Xu,
David S. Miller, Lee Jones, Vinod Koul, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
Wim Van Sebroeck, Guenter Roeck, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, linux-kernel, linux-ide,
linux-crypto, dmaengine, linux-pci, linux-watchdog, linux-spi,
linux-mtd
In-Reply-To: <Z6oh9t2QQzz17Yt6@probook>
[-- Attachment #1: Type: text/plain, Size: 771 bytes --]
On Mon, Feb 10, 2025 at 03:57:42PM +0000, J. Neuschäfer wrote:
> On Mon, Feb 10, 2025 at 12:59:35PM +0000, Mark Brown wrote:
> > Please don't do this, it just makes it harder to merge things since it
> > makes it look like there's cross tree merges needed when that's not the
> > case, complicating merging, and puts the entire series in everyone's
> > inbox which makes things more noisy.
> How should I proceed with this series, in your opinion?
> I see potential advantages (less of the issues you describe) and
> disadvantages (somewhat harder to track patches) of splitting it up
> before sending v3.
I'd rather that at least the SPI stuff were sent separately (well,
ideally what you've done already is fine and it doesn't need a resend at
all).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 00/12] YAML conversion of several Freescale/PowerPC DT bindings
From: J. Neuschäfer @ 2025-02-10 15:57 UTC (permalink / raw)
To: Mark Brown
Cc: J. Neuschäfer, devicetree, linuxppc-dev, Krzysztof Kozlowski,
imx, Scott Wood, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Damien Le Moal, Niklas Cassel,
Herbert Xu, David S. Miller, Lee Jones, Vinod Koul,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Helgaas, Wim Van Sebroeck,
Guenter Roeck, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-kernel, linux-ide, linux-crypto,
dmaengine, linux-pci, linux-watchdog, linux-spi, linux-mtd
In-Reply-To: <0fe3416c-c3f3-44c4-a1c0-7e8262c54d4b@sirena.org.uk>
On Mon, Feb 10, 2025 at 12:59:35PM +0000, Mark Brown wrote:
> On Sat, Feb 08, 2025 at 02:20:47AM +0000, J. Neuschäfer wrote:
> > On Fri, Feb 07, 2025 at 09:38:05PM +0000, Mark Brown wrote:
>
> > > What's the story with dependencies here - why is all this stuff in one
> > > series?
>
> > The patches are independent of each other, except for the four elbc/nand
> > patches. They are in the same series because they came up during the
> > same project and achieve similar goals, but it isn't necessary.
>
> Please don't do this, it just makes it harder to merge things since it
> makes it look like there's cross tree merges needed when that's not the
> case, complicating merging, and puts the entire series in everyone's
> inbox which makes things more noisy.
How should I proceed with this series, in your opinion?
I see potential advantages (less of the issues you describe) and
disadvantages (somewhat harder to track patches) of splitting it up
before sending v3.
(Outside of this series, the conclusion is clear and simple)
J. Neuschäfer
^ permalink raw reply
* Re: [PATCH 3/3] dmaengine: ptdma: Utilize the AE4DMA engine's multi-queue functionality
From: Basavaraj Natikar @ 2025-02-10 14:37 UTC (permalink / raw)
To: Vinod Koul, Basavaraj Natikar; +Cc: dmaengine
In-Reply-To: <c568caf4-c69c-4e41-a794-4756ca8fbd93@amd.com>
On 2/10/2025 7:56 PM, Basavaraj Natikar wrote:
>
>
> On 2/10/2025 7:48 PM, Vinod Koul wrote:
>> On 03-02-25, 21:55, Basavaraj Natikar wrote:
>>> As AE4DMA offers multi-channel functionality compared to PTDMA’s single
>>> queue, utilize multi-queue, which supports higher speeds than PTDMA, to
>>> achieve higher performance using the AE4DMA workqueue based mechanism.
>>>
>>> Fixes: 69a47b16a51b ("dmaengine: ptdma: Extend ptdma to support
>>> multi-channel and version")
>> Why is this a fix, again!
>
> Yes, as AE4DMA is much faster with multi-queue. However, sometimes due
> to multi-queue
> processing, it takes longer and introduces a timeout for the
> synchronization of
> consumers and producers, which helps avoid long waits that could
> eventually cause a hang.
>
> Thanks,
> --
> Basavaraj
>
>>
>>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>>> ---
>>> drivers/dma/amd/ae4dma/ae4dma.h | 2 +
>>> drivers/dma/amd/ptdma/ptdma-dmaengine.c | 90
>>> ++++++++++++++++++++++++-
>>> 2 files changed, 89 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/dma/amd/ae4dma/ae4dma.h
>>> b/drivers/dma/amd/ae4dma/ae4dma.h
>>> index 265c5d436008..57f6048726bb 100644
>>> --- a/drivers/dma/amd/ae4dma/ae4dma.h
>>> +++ b/drivers/dma/amd/ae4dma/ae4dma.h
>>> @@ -37,6 +37,8 @@
>>> #define AE4_DMA_VERSION 4
>>> #define CMD_AE4_DESC_DW0_VAL 2
>>> +#define AE4_TIME_OUT 5000
>>> +
>>> struct ae4_msix {
>>> int msix_count;
>>> struct msix_entry msix_entry[MAX_AE4_HW_QUEUES];
>>> diff --git a/drivers/dma/amd/ptdma/ptdma-dmaengine.c
>>> b/drivers/dma/amd/ptdma/ptdma-dmaengine.c
>>> index 35c84ec9608b..715ac3ae067b 100644
>>> --- a/drivers/dma/amd/ptdma/ptdma-dmaengine.c
>>> +++ b/drivers/dma/amd/ptdma/ptdma-dmaengine.c
>>> @@ -198,8 +198,10 @@ static struct pt_dma_desc
>>> *pt_handle_active_desc(struct pt_dma_chan *chan,
>>> {
>>> struct dma_async_tx_descriptor *tx_desc;
>>> struct virt_dma_desc *vd;
>>> + struct pt_device *pt;
>>> unsigned long flags;
>>> + pt = chan->pt;
>>> /* Loop over descriptors until one is found with commands */
>>> do {
>>> if (desc) {
>>> @@ -217,7 +219,7 @@ static struct pt_dma_desc
>>> *pt_handle_active_desc(struct pt_dma_chan *chan,
>>> spin_lock_irqsave(&chan->vc.lock, flags);
>>> - if (desc) {
>>> + if (pt->ver != AE4_DMA_VERSION && desc) {
>>> if (desc->status != DMA_COMPLETE) {
>>> if (desc->status != DMA_ERROR)
>>> desc->status = DMA_COMPLETE;
>>> @@ -235,7 +237,7 @@ static struct pt_dma_desc
>>> *pt_handle_active_desc(struct pt_dma_chan *chan,
>>> spin_unlock_irqrestore(&chan->vc.lock, flags);
>>> - if (tx_desc) {
>>> + if (pt->ver != AE4_DMA_VERSION && tx_desc) {
>> Why should this handling be different for DMA_VERSION?
PTDMA always handles per command based interrupt and it is single queue while
AE4DMA we can submit multiple command each time with multiple as AE4DMA is
much faster with multi-queue.
However, sometimes due to multi-queue processing, it takes longer and introduces a
timeout for the synchronization of consumers and producers,
which helps avoid long waits that could eventually cause a hang.
>>
>>> dmaengine_desc_get_callback_invoke(tx_desc, NULL);
>>> dma_run_dependencies(tx_desc);
>>> vchan_vdesc_fini(vd);
>>> @@ -245,11 +247,25 @@ static struct pt_dma_desc
>>> *pt_handle_active_desc(struct pt_dma_chan *chan,
>>> return NULL;
>>> }
>>> +static inline bool ae4_core_queue_full(struct pt_cmd_queue *cmd_q)
>>> +{
>>> + u32 front_wi = readl(cmd_q->reg_control + AE4_WR_IDX_OFF);
>>> + u32 rear_ri = readl(cmd_q->reg_control + AE4_RD_IDX_OFF);
>>> +
>>> + if (((MAX_CMD_QLEN + front_wi - rear_ri) % MAX_CMD_QLEN) >=
>>> (MAX_CMD_QLEN - 1))
>>> + return true;
>>> +
>>> + return false;
>>> +}
>>> +
>>> static void pt_cmd_callback(void *data, int err)
>>> {
>>> struct pt_dma_desc *desc = data;
>>> + struct ae4_cmd_queue *ae4cmd_q;
>>> struct dma_chan *dma_chan;
>>> struct pt_dma_chan *chan;
>>> + struct ae4_device *ae4;
>>> + struct pt_device *pt;
>>> int ret;
>>> if (err == -EINPROGRESS)
>>> @@ -257,11 +273,32 @@ static void pt_cmd_callback(void *data, int err)
>>> dma_chan = desc->vd.tx.chan;
>>> chan = to_pt_chan(dma_chan);
>>> + pt = chan->pt;
>>> if (err)
>>> desc->status = DMA_ERROR;
>>> while (true) {
>>> + if (pt->ver == AE4_DMA_VERSION) {
>>> + ae4 = container_of(pt, struct ae4_device, pt);
>>> + ae4cmd_q = &ae4->ae4cmd_q[chan->id];
>>> +
>>> + if (ae4cmd_q->q_cmd_count >= (CMD_Q_LEN - 1) ||
>>> + ae4_core_queue_full(&ae4cmd_q->cmd_q)) {
>>> + wake_up(&ae4cmd_q->q_w);
>>> +
>>> + if (wait_for_completion_timeout(&ae4cmd_q->cmp,
>>> + msecs_to_jiffies(AE4_TIME_OUT))
>>> + == 0) {
>>> + dev_err(pt->dev, "TIMEOUT %d:\n", ae4cmd_q->id);
>>> + break;
>>> + }
>>> +
>>> + reinit_completion(&ae4cmd_q->cmp);
>>> + continue;
>>> + }
>>> + }
>>> +
>>> /* Check for DMA descriptor completion */
>>> desc = pt_handle_active_desc(chan, desc);
>>> @@ -296,6 +333,49 @@ static struct pt_dma_desc
>>> *pt_alloc_dma_desc(struct pt_dma_chan *chan,
>>> return desc;
>>> }
>>> +static void pt_cmd_callback_work(void *data, int err)
>>> +{
>>> + struct dma_async_tx_descriptor *tx_desc;
>>> + struct pt_dma_desc *desc = data;
>>> + struct dma_chan *dma_chan;
>>> + struct virt_dma_desc *vd;
>>> + struct pt_dma_chan *chan;
>>> + unsigned long flags;
>>> +
>>> + dma_chan = desc->vd.tx.chan;
>>> + chan = to_pt_chan(dma_chan);
>>> +
>>> + if (err == -EINPROGRESS)
>>> + return;
>>> +
>>> + tx_desc = &desc->vd.tx;
>>> + vd = &desc->vd;
>>> +
>>> + if (err)
>>> + desc->status = DMA_ERROR;
>>> +
>>> + spin_lock_irqsave(&chan->vc.lock, flags);
>>> + if (desc) {
>>> + if (desc->status != DMA_COMPLETE) {
>>> + if (desc->status != DMA_ERROR)
>>> + desc->status = DMA_COMPLETE;
>>> +
>>> + dma_cookie_complete(tx_desc);
>>> + dma_descriptor_unmap(tx_desc);
>>> + } else {
>>> + tx_desc = NULL;
>>> + }
>>> + }
>>> + spin_unlock_irqrestore(&chan->vc.lock, flags);
>>> +
>>> + if (tx_desc) {
>>> + dmaengine_desc_get_callback_invoke(tx_desc, NULL);
>>> + dma_run_dependencies(tx_desc);
>>> + list_del(&desc->vd.node);
>>> + vchan_vdesc_fini(vd);
>>> + }
>>> +}
>> Why do we have callback in driver...?
PTDMA also has similar callback pt_cmd_callback
hence AE4DMA also has callback for the multi-queue command ,
once command is processed to signal upper layer that processing
done for that queue.Thanks,
--
Basavaraj
>>
>>> +
>>> static struct pt_dma_desc *pt_create_desc(struct dma_chan *dma_chan,
>>> dma_addr_t dst,
>>> dma_addr_t src,
>>> @@ -327,6 +407,7 @@ static struct pt_dma_desc *pt_create_desc(struct
>>> dma_chan *dma_chan,
>>> desc->len = len;
>>> if (pt->ver == AE4_DMA_VERSION) {
>>> + pt_cmd->pt_cmd_callback = pt_cmd_callback_work;
>>> ae4 = container_of(pt, struct ae4_device, pt);
>>> ae4cmd_q = &ae4->ae4cmd_q[chan->id];
>>> mutex_lock(&ae4cmd_q->cmd_lock);
>>> @@ -367,13 +448,16 @@ static void pt_issue_pending(struct dma_chan
>>> *dma_chan)
>>> {
>>> struct pt_dma_chan *chan = to_pt_chan(dma_chan);
>>> struct pt_dma_desc *desc;
>>> + struct pt_device *pt;
>>> unsigned long flags;
>>> bool engine_is_idle = true;
>>> + pt = chan->pt;
>>> +
>>> spin_lock_irqsave(&chan->vc.lock, flags);
>>> desc = pt_next_dma_desc(chan);
>>> - if (desc)
>>> + if (desc && pt->ver != AE4_DMA_VERSION)
>>> engine_is_idle = false;
>>> vchan_issue_pending(&chan->vc);
>>> --
>>> 2.25.1
>
^ permalink raw reply
* Re: [PATCH] dmaengine: idxd: Delete unnecessary NULL check
From: Vinod Koul @ 2025-02-10 14:35 UTC (permalink / raw)
To: Fenghua Yu, Dan Carpenter
Cc: Dave Jiang, dmaengine, linux-kernel, kernel-janitors
In-Reply-To: <ec38214e-0bbb-4c5a-94ff-b2b2d4c3f245@stanley.mountain>
On Wed, 08 Jan 2025 12:13:20 +0300, Dan Carpenter wrote:
> The "saved_evl" pointer is a offset into the middle of a non-NULL struct.
> It can't be NULL and the check is slightly confusing. Delete the check.
>
>
Applied, thanks!
[1/1] dmaengine: idxd: Delete unnecessary NULL check
commit: 2c17e9ea0caa5555e31e154fa1b06260b816f5cc
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 1/2] dmaengine: Use str_enable_disable-like helpers
From: Vinod Koul @ 2025-02-10 14:35 UTC (permalink / raw)
To: Manivannan Sadhasivam, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Peter Ujfalusi, Michal Simek, dmaengine,
linux-kernel, imx, linux-arm-kernel, linux-sunxi,
Krzysztof Kozlowski
In-Reply-To: <20250114191021.854080-1-krzysztof.kozlowski@linaro.org>
On Tue, 14 Jan 2025 20:10:20 +0100, Krzysztof Kozlowski wrote:
> Replace ternary (condition ? "enable" : "disable") syntax with helpers
> from string_choices.h because:
> 1. Simple function call with one argument is easier to read. Ternary
> operator has three arguments and with wrapping might lead to quite
> long code.
> 2. Is slightly shorter thus also easier to read.
> 3. It brings uniformity in the text - same string.
> 4. Allows deduping by the linker, which results in a smaller binary
> file.
>
> [...]
Applied, thanks!
[1/2] dmaengine: Use str_enable_disable-like helpers
commit: 8e63891831f3904047d7ad8078ff52dd454b6975
[2/2] dmaengine: pxa: Enable compile test
commit: 9fc2f03e85952ee52558ab844473a8284d924a56
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Change maintainer for IDXD
From: Vinod Koul @ 2025-02-10 14:35 UTC (permalink / raw)
To: Dave Jiang, Vinicius Costa Gomes, Fenghua Yu; +Cc: dmaengine, linux-kernel
In-Reply-To: <20250131173551.3979408-1-fenghua.yu@intel.com>
On Fri, 31 Jan 2025 09:35:51 -0800, Fenghua Yu wrote:
> Due to job transition, I am stepping down as IDXD maintainer.
> Vinicius will take over maintainership responsibilities moving forward.
>
>
Applied, thanks!
[1/1] MAINTAINERS: Change maintainer for IDXD
commit: 753f324c4caa154e57b6dfff8fba7769dd76a0f5
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH v4] dt-bindings: dma: convert atmel-dma.txt to YAML
From: Vinod Koul @ 2025-02-10 14:35 UTC (permalink / raw)
To: Ludovic Desroches, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Andrei Simion,
Charan Pedumuru
Cc: linux-arm-kernel, dmaengine, devicetree, linux-kernel,
Durai Manickam KR
In-Reply-To: <20250203-test-v4-1-a9ec3eded1c7@microchip.com>
On Mon, 03 Feb 2025 11:48:09 +0530, Charan Pedumuru wrote:
> Add a description, required properties, appropriate compatibles and
> missing properties like clocks and clock-names which are not defined in
> the text binding for all the SoCs that are supported by microchip.
> Update the text binding name `atmel-dma.txt` to
> `atmel,at91sam9g45-dma.yaml` for the files which reference to
> `atmel-dma.txt`. Drop Tudor name from maintainers.
>
> [...]
Applied, thanks!
[1/1] dt-bindings: dma: convert atmel-dma.txt to YAML
commit: a54ec770396ce2b6e786acde22f4ebed8d1e9555
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH] dmaengine: fsl-edma: Add missing newlines to log messages
From: Vinod Koul @ 2025-02-10 14:35 UTC (permalink / raw)
To: Frank Li, Stefan Wahren; +Cc: imx, dmaengine, linux-arm-kernel
In-Reply-To: <20250205091455.4593-1-wahrenst@gmx.net>
On Wed, 05 Feb 2025 10:14:55 +0100, Stefan Wahren wrote:
> Not all log messages have a newline at the end. So fix it.
>
>
Applied, thanks!
[1/1] dmaengine: fsl-edma: Add missing newlines to log messages
commit: 1c4c8609d498173382913b8ef6a105f3e6ff3472
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 1/1] dmaengine: dw: Switch to LATE_SIMPLE_DEV_PM_OPS()
From: Vinod Koul @ 2025-02-10 14:35 UTC (permalink / raw)
To: Serge Semin, dmaengine, linux-kernel, Andy Shevchenko; +Cc: Viresh Kumar
In-Reply-To: <20250205150701.893083-1-andriy.shevchenko@linux.intel.com>
On Wed, 05 Feb 2025 17:06:48 +0200, Andy Shevchenko wrote:
> SET_LATE_SYSTEM_SLEEP_PM_OPS is deprecated, replace it with
> LATE_SYSTEM_SLEEP_PM_OPS() and use pm_sleep_ptr() for setting
> the driver's pm routines. We can now remove the ifdeffery
> in the suspend and resume functions.
>
>
Applied, thanks!
[1/1] dmaengine: dw: Switch to LATE_SIMPLE_DEV_PM_OPS()
commit: 1e137d53e8471b45257d937e9773b61a88807fe7
Best regards,
--
~Vinod
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox