Message-ID: <b4863d37-2182-479d-8ca2-79951badfb8d@sifive.com>
Date: Mon, 18 Mar 2024 23:27:37 -0500
From: Samuel Holland <samuel.holland@...ive.com>
To: Inochi Amaoto <inochiama@...look.com>, Vinod Koul <vkoul@...nel.org>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: Jisheng Zhang <jszhang@...nel.org>, Liu Gui <kenneth.liu@...hgo.com>,
 Jingbao Qiu <qiujingbao.dlmu@...il.com>, dlan@...too.org,
 dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 Chen Wang <unicorn_wang@...look.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Subject: Re: [PATCH v4 1/4] dt-bindings: dmaengine: Add dmamux for
 CV18XX/SG200X series SoC

Hi Inochi,

On 2024-03-18 11:03 PM, Inochi Amaoto wrote:
> On Mon, Mar 18, 2024 at 10:22:47PM -0500, Samuel Holland wrote:
>> On 2024-03-18 1:38 AM, Inochi Amaoto wrote:
>>> The DMA IP of Sophgo CV18XX/SG200X is based on a DW AXI CORE, with
>>> an additional channel remap register located in the top system control
>>> area. The DMA channel is exclusive to each core.
>>>
>>> Add the dmamux binding for CV18XX/SG200X series SoC
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@...look.com>
>>> Reviewed-by: Rob Herring <robh@...nel.org>
>>> ---
>>>  .../bindings/dma/sophgo,cv1800-dmamux.yaml    | 47 ++++++++++++++++
>>>  include/dt-bindings/dma/cv1800-dma.h          | 55 +++++++++++++++++++
>>>  2 files changed, 102 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/dma/sophgo,cv1800-dmamux.yaml
>>>  create mode 100644 include/dt-bindings/dma/cv1800-dma.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/sophgo,cv1800-dmamux.yaml b/Documentation/devicetree/bindings/dma/sophgo,cv1800-dmamux.yaml
>>> new file mode 100644
>>> index 000000000000..c813c66737ba
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/dma/sophgo,cv1800-dmamux.yaml
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/dma/sophgo,cv1800-dmamux.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Sophgo CV1800/SG200 Series DMA mux
>>> +
>>> +maintainers:
>>> +  - Inochi Amaoto <inochiama@...look.com>
>>> +
>>> +allOf:
>>> +  - $ref: dma-router.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: sophgo,cv1800-dmamux
>>> +
>>> +  reg:
>>> +    maxItems: 2
>>> +
>>> +  '#dma-cells':
>>> +    const: 3
>>> +    description:
>>> +      The first cells is DMA channel. The second one is device id.
>>> +      The third one is the cpu id.
>>
>> There are 43 devices, but only 8 channels. Since the channel is statically
>> specified in the devicetree as the first cell here, that means the SoC DT author
>> must pre-select which 8 of the 43 devices are usable, right? 
> 
> Yes, you are right.
> 
>> And then the rest
>> would have to omit their dma properties. Wouldn't it be better to leave out the
>> channel number here and dynamically allocate channels at runtime?
>>
> 
> You mean defining all the dma channel in the device and allocation channel
> selectively? This is workable, but it still needs a hint to allocate channel.

I mean allocating hardware channels only when a channel is requested by a client
driver. The dmamux driver could maintain a counter and allocate the channels
sequentially -- then the first 8 calls to cv1800_dmamux_route_allocate() would
succeed and later calls from other devices would fail.

> Also, according to the information from sophgo, it does not support dynamic 
> channel allocation, so all channel can only be initialize once.

That's important to know. In that case, the driver should probably leave the
registers alone in cv1800_dmamux_free(), and then scan to see if a device is
already mapped to a channel before allocating a new one. (Or it should have some
other way of remembering the mapping.) That way a single client can repeatedly
allocate/free its DMA channel without consuming all of the hardware channels.

> There is another problem, since we defined all the dmas property in the device,
> How to mask the devices if we do not want to use dma on them? I have see SPI
> device will disable DMA when allocation failed, I guess this is this mechanism
> is the same for all devices?

I2C/SPI/UART controller drivers generally still work after failing to acquire a
DMA channel. For audio-related drivers, DMA is generally a hard dependency.

If each board has 8 or fewer DMA-capable devices enabled in its DT, there is no
problem. If some board enables more than 8 DMA-capable devices, then it should
use "/delete-property/ dmas;" on the devices that would be least impacted by
missing DMA. Otherwise, which devices get functional DMA depends on driver probe
order.

Normally you wouldn't need to do "/delete-property/ dmas;", because many drivers
only request the DMA channel when actively being used (e.g. userspace has the
TTY/spidev/ALSA device file open), but this doesn't help if you can only assign
each channel once.

Regards,
Samuel

>>> +
>>> +  dma-masters:
>>> +    maxItems: 1
>>> +
>>> +  dma-requests:
>>> +    const: 8
>>> +
>>> +required:
>>> +  - '#dma-cells'
>>> +  - dma-masters
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    dma-router {
>>> +      compatible = "sophgo,cv1800-dmamux";
>>> +      #dma-cells = <3>;
>>> +      dma-masters = <&dmac>;
>>> +      dma-requests = <8>;
>>> +    };
>>> diff --git a/include/dt-bindings/dma/cv1800-dma.h b/include/dt-bindings/dma/cv1800-dma.h
>>> new file mode 100644
>>> index 000000000000..3ce9dac25259
>>> --- /dev/null
>>> +++ b/include/dt-bindings/dma/cv1800-dma.h
>>> @@ -0,0 +1,55 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>>> +
>>> +#ifndef __DT_BINDINGS_DMA_CV1800_H__
>>> +#define __DT_BINDINGS_DMA_CV1800_H__
>>> +
>>> +#define DMA_I2S0_RX		0
>>> +#define DMA_I2S0_TX		1
>>> +#define DMA_I2S1_RX		2
>>> +#define DMA_I2S1_TX		3
>>> +#define DMA_I2S2_RX		4
>>> +#define DMA_I2S2_TX		5
>>> +#define DMA_I2S3_RX		6
>>> +#define DMA_I2S3_TX		7
>>> +#define DMA_UART0_RX		8
>>> +#define DMA_UART0_TX		9
>>> +#define DMA_UART1_RX		10
>>> +#define DMA_UART1_TX		11
>>> +#define DMA_UART2_RX		12
>>> +#define DMA_UART2_TX		13
>>> +#define DMA_UART3_RX		14
>>> +#define DMA_UART3_TX		15
>>> +#define DMA_SPI0_RX		16
>>> +#define DMA_SPI0_TX		17
>>> +#define DMA_SPI1_RX		18
>>> +#define DMA_SPI1_TX		19
>>> +#define DMA_SPI2_RX		20
>>> +#define DMA_SPI2_TX		21
>>> +#define DMA_SPI3_RX		22
>>> +#define DMA_SPI3_TX		23
>>> +#define DMA_I2C0_RX		24
>>> +#define DMA_I2C0_TX		25
>>> +#define DMA_I2C1_RX		26
>>> +#define DMA_I2C1_TX		27
>>> +#define DMA_I2C2_RX		28
>>> +#define DMA_I2C2_TX		29
>>> +#define DMA_I2C3_RX		30
>>> +#define DMA_I2C3_TX		31
>>> +#define DMA_I2C4_RX		32
>>> +#define DMA_I2C4_TX		33
>>> +#define DMA_TDM0_RX		34
>>> +#define DMA_TDM0_TX		35
>>> +#define DMA_TDM1_RX		36
>>> +#define DMA_AUDSRC		37
>>> +#define DMA_SPI_NAND		38
>>> +#define DMA_SPI_NOR		39
>>> +#define DMA_UART4_RX		40
>>> +#define DMA_UART4_TX		41
>>> +#define DMA_SPI_NOR1		42
>>> +
>>> +#define DMA_CPU_A53		0
>>> +#define DMA_CPU_C906_0		1
>>> +#define DMA_CPU_C906_1		2
>>> +
>>> +
>>> +#endif // __DT_BINDINGS_DMA_CV1800_H__
>>> --
>>> 2.44.0
>>>
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> linux-riscv@...ts.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>

Powered by blists - more mailing lists