| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250730180417.GC21430@pendragon.ideasonboard.com>
Date: Wed, 30 Jul 2025 21:04:17 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Frank Li <Frank.li@....com>
Cc: Konrad Dybcio <konradybcio@...nel.org>, Vinod Koul <vkoul@...nel.org>,
Sven Peter <sven@...nel.org>, Janne Grunau <j@...nau.net>,
Alyssa Rosenzweig <alyssa@...enzweig.io>,
Neal Gompa <neal@...pa.dev>,
Ludovic Desroches <ludovic.desroches@...rochip.com>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Ray Jui <rjui@...adcom.com>, Scott Branden <sbranden@...adcom.com>,
Paul Cercueil <paul@...pouillou.net>,
Eugeniy Paltsev <Eugeniy.Paltsev@...opsys.com>,
Viresh Kumar <vireshk@...nel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Taichi Sugaya <sugaya.taichi@...ionext.com>,
Takao Orito <orito.takao@...ionext.com>,
Andreas Färber <afaerber@...e.de>,
Manivannan Sadhasivam <mani@...nel.org>,
Daniel Mack <daniel@...que.org>,
Haojian Zhuang <haojian.zhuang@...il.com>,
Robert Jarzmik <robert.jarzmik@...e.fr>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Magnus Damm <magnus.damm@...il.com>,
Patrice Chotard <patrice.chotard@...s.st.com>,
Linus Walleij <linus.walleij@...aro.org>,
Amélie Delaunay <amelie.delaunay@...s.st.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Samuel Holland <samuel@...lland.org>,
Laxman Dewangan <ldewangan@...dia.com>,
Jon Hunter <jonathanh@...dia.com>,
Thierry Reding <thierry.reding@...il.com>,
Peter Ujfalusi <peter.ujfalusi@...il.com>,
Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Michal Simek <michal.simek@....com>, Rob Herring <robh@...nel.org>,
Saravana Kannan <saravanak@...gle.com>,
Martin Povišer <povik+lin@...ebit.org>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>,
Viken Dadhaniya <quic_vdadhani@...cinc.com>,
Andi Shyti <andi.shyti@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Marijn Suijten <marijn.suijten@...ainline.org>,
dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
asahi@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-rpi-kernel@...ts.infradead.org, linux-mips@...r.kernel.org,
imx@...ts.linux.dev, linux-actions@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-sunxi@...ts.linux.dev, linux-tegra@...r.kernel.org,
devicetree@...r.kernel.org, linux-sound@...r.kernel.org,
linux-i2c@...r.kernel.org, linux-spi@...r.kernel.org,
Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH RFC 2/6] dmaengine: Make of_dma_request_slave_channel
pass a cookie to of_xlate
On Wed, Jul 30, 2025 at 12:39:43PM -0400, Frank Li wrote:
> On Wed, Jul 30, 2025 at 11:33:29AM +0200, Konrad Dybcio wrote:
> > From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> >
> > The DMA subsystem attempts to make it theoretically possible to pair
> > any DMA block with any user. While that's convenient from a
> > codebase sanity perspective, some blocks are more intertwined.
> >
> > One such case is the Qualcomm GENI, where each wrapper contains a
> > number of Serial Engine instances, each one of which can be programmed
> > to support a different protocol (such as I2C, I3C, SPI, UART, etc.).
> >
> > The GPI DMA it's designed together with, needs to receive the ID of the
> > protocol that's in use, to adjust its behavior accordingly. Currently,
> > that's done through passing that ID through device tree, with each
> > Serial Engine expressed NUM_PROTOCOL times, resulting in terrible
> > dt-bindings that are full of useless copypasta.
> >
> > In a step to cut down on that, let the DMA user give the engine driver
> > a hint at request time.
> >
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> > ---
> > drivers/dma/amba-pl08x.c | 3 ++-
> > drivers/dma/apple-admac.c | 3 ++-
> > drivers/dma/at_hdmac.c | 6 ++++--
> > drivers/dma/at_xdmac.c | 3 ++-
> > drivers/dma/bcm2835-dma.c | 3 ++-
> > drivers/dma/dma-jz4780.c | 3 ++-
> > drivers/dma/dmaengine.c | 20 +++++++++++++++++---
> > drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 3 ++-
> > drivers/dma/dw/of.c | 3 ++-
> > drivers/dma/ep93xx_dma.c | 6 ++++--
> > drivers/dma/fsl-edma-main.c | 6 ++++--
> > drivers/dma/img-mdc-dma.c | 3 ++-
> > drivers/dma/imx-dma.c | 3 ++-
> > drivers/dma/imx-sdma.c | 3 ++-
> > drivers/dma/lgm/lgm-dma.c | 3 ++-
> > drivers/dma/milbeaut-hdmac.c | 4 +++-
> > drivers/dma/mmp_pdma.c | 3 ++-
> > drivers/dma/mmp_tdma.c | 3 ++-
> > drivers/dma/moxart-dma.c | 3 ++-
> > drivers/dma/mxs-dma.c | 3 ++-
> > drivers/dma/nbpfaxi.c | 3 ++-
> > drivers/dma/of-dma.c | 18 +++++++++++-------
> > drivers/dma/owl-dma.c | 3 ++-
> > drivers/dma/pl330.c | 3 ++-
> > drivers/dma/pxa_dma.c | 3 ++-
> > drivers/dma/qcom/bam_dma.c | 3 ++-
> > drivers/dma/qcom/gpi.c | 3 ++-
> > drivers/dma/qcom/qcom_adm.c | 3 ++-
> > drivers/dma/sh/rcar-dmac.c | 3 ++-
> > drivers/dma/sh/rz-dmac.c | 3 ++-
> > drivers/dma/sh/usb-dmac.c | 3 ++-
> > drivers/dma/st_fdma.c | 3 ++-
> > drivers/dma/ste_dma40.c | 3 ++-
> > drivers/dma/stm32/stm32-dma.c | 3 ++-
> > drivers/dma/stm32/stm32-dma3.c | 4 +++-
> > drivers/dma/stm32/stm32-mdma.c | 3 ++-
> > drivers/dma/sun4i-dma.c | 3 ++-
> > drivers/dma/sun6i-dma.c | 3 ++-
> > drivers/dma/tegra186-gpc-dma.c | 3 ++-
> > drivers/dma/tegra20-apb-dma.c | 3 ++-
> > drivers/dma/tegra210-adma.c | 3 ++-
> > drivers/dma/ti/cppi41.c | 3 ++-
> > drivers/dma/ti/edma.c | 3 ++-
> > drivers/dma/ti/k3-udma.c | 3 ++-
> > drivers/dma/uniphier-xdmac.c | 3 ++-
> > drivers/dma/xilinx/xilinx_dma.c | 3 ++-
> > drivers/dma/xilinx/xilinx_dpdma.c | 3 ++-
> > drivers/dma/xilinx/zynqmp_dma.c | 3 ++-
> > include/linux/dmaengine.h | 7 +++++++
> > include/linux/of_dma.h | 16 +++++++++-------
> > sound/soc/apple/mca.c | 2 +-
> > sound/soc/renesas/rcar/dma.c | 2 +-
> > 52 files changed, 146 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
>
> ...
>
> > const char *name)
> > {
> > diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> > index fd706cdf255c61c82ce30ef9a2c44930bef34bc8..9f9bc4207b85d48d73c25aad4b362e7c84c01756 100644
> > --- a/include/linux/of_dma.h
> > +++ b/include/linux/of_dma.h
> > @@ -19,7 +19,7 @@ struct of_dma {
> > struct list_head of_dma_controllers;
> > struct device_node *of_node;
> > struct dma_chan *(*of_dma_xlate)
> > - (struct of_phandle_args *, struct of_dma *);
> > + (struct of_phandle_args *, struct of_dma *, void *);
>
> I suggest pass down more informaiton, like client's dev point. So we can
> auto create device link between client's dev and dma chan's device.
Is .of_dma_xlate() really the right place to do that ? If you want to
create a device link for PM reasons, isn't it better created when the
channel is requested ? It should also be removed when the channel is
freed.
>
> DMA Engineer device
> DMA chan device
> consumer clients' device.
>
> If consumer device runtime pm suspend can auto trigger DMA chan's device's
> runtime pm function.
>
> It will simplifly DMA engine's run time pm manage. Currently many DMA run
> time pm implement as, runtime_pm_get() when alloc and runtime_pm_put() at
> free channel. But many devices request dma channel at probe, which make
> dma engine work at always 'on' state.
>
> But ideally, dma chan should be resume only when it is used to transfer.
This is exactly what I was going to mention after reading the last
paragraph. Is there anything that prevents a DMA engine driver to
perform a rutime PM get() when a transfer is submitted and a put() when
it completes ? (Logically speaking, the actual implementation would
likely be a bit different in drivers, but the result would be similar.)
> > void *(*of_dma_route_allocate)
> > (struct of_phandle_args *, struct of_dma *);
> > struct dma_router *dma_router;
> > @@ -34,7 +34,7 @@ struct of_dma_filter_info {
> > #ifdef CONFIG_DMA_OF
> > extern int of_dma_controller_register(struct device_node *np,
> > struct dma_chan *(*of_dma_xlate)
> > - (struct of_phandle_args *, struct of_dma *),
> > + (struct of_phandle_args *, struct of_dma *, void *),
> > void *data);
> > extern void of_dma_controller_free(struct device_node *np);
> >
> > @@ -45,16 +45,17 @@ extern int of_dma_router_register(struct device_node *np,
> > #define of_dma_router_free of_dma_controller_free
> >
> > extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> > - const char *name);
> > + const char *name,
> > + void *data);
> > extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
> > - struct of_dma *ofdma);
> > + struct of_dma *ofdma, void *data);
> > extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
> > - struct of_dma *ofdma);
> > + struct of_dma *ofdma, void *data);
> >
> > #else
> > static inline int of_dma_controller_register(struct device_node *np,
> > struct dma_chan *(*of_dma_xlate)
> > - (struct of_phandle_args *, struct of_dma *),
> > + (struct of_phandle_args *, struct of_dma *, void *),
> > void *data)
> > {
> > return -ENODEV;
> > @@ -75,7 +76,8 @@ static inline int of_dma_router_register(struct device_node *np,
> > #define of_dma_router_free of_dma_controller_free
> >
> > static inline struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> > - const char *name)
> > + const char *name,
> > + void *data)
> > {
> > return ERR_PTR(-ENODEV);
> > }
> > diff --git a/sound/soc/apple/mca.c b/sound/soc/apple/mca.c
> > index 5dd24ab90d0f052bb48f451cf009dc2e9128014d..43d48e4ac8161ee9955120fe64f7b911bfdfe1ca 100644
> > --- a/sound/soc/apple/mca.c
> > +++ b/sound/soc/apple/mca.c
> > @@ -926,7 +926,7 @@ static struct dma_chan *mca_request_dma_channel(struct mca_cluster *cl, unsigned
> > char *name = devm_kasprintf(cl->host->dev, GFP_KERNEL,
> > is_tx ? "tx%da" : "rx%db", cl->no);
> > #endif
> > - return of_dma_request_slave_channel(cl->host->dev->of_node, name);
> > + return of_dma_request_slave_channel(cl->host->dev->of_node, name, NULL);
> >
> > }
> >
> > diff --git a/sound/soc/renesas/rcar/dma.c b/sound/soc/renesas/rcar/dma.c
> > index 2035ce06fe4c4aeaa8620d817910a5319732fa58..dcbff2fc61a0472adea226371016a128563b3cd0 100644
> > --- a/sound/soc/renesas/rcar/dma.c
> > +++ b/sound/soc/renesas/rcar/dma.c
> > @@ -204,7 +204,7 @@ struct dma_chan *rsnd_dma_request_channel(struct device_node *of_node, char *nam
> > }
> >
> > if (i == rsnd_mod_id_raw(mod) && (!chan))
> > - chan = of_dma_request_slave_channel(np, x);
> > + chan = of_dma_request_slave_channel(np, x, NULL);
> > i++;
> > }
> >
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists