Re: [PATCH v2 1/8] dt-bindings: dma: marvell,mmp-dma: Add SpacemiT K1 PDMA support

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Guodong Xu <guodong@riscstar.com>
Cc: "Vinod Koul" <vkoul@kernel.org>, "Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Yixun Lan" <dlan@gentoo.org>,
	"Duje Mihanović" <duje.mihanovic@skole.hr>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Alex Elder" <elder@riscstar.com>,
	"Vivian Wang" <wangruikang@iscas.ac.cn>,
	dmaengine@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	spacemit@lists.linux.dev
Subject: Re: [PATCH v2 1/8] dt-bindings: dma: marvell,mmp-dma: Add SpacemiT K1 PDMA support
Date: Wed, 2 Jul 2025 22:38:00 +0200	[thread overview]
Message-ID: <43f19fbc-838b-431e-9d02-ecacf5f43731@kernel.org> (raw)
In-Reply-To: <CAH1PCMbKGeXL9Te6M28ZdX8VBvaToKcK7f+JmN6JsCfttG_Mtg@mail.gmail.com>

On 01/07/2025 11:52, Guodong Xu wrote:
> Hi, Krzysztof
> 
> On Tue, Jul 1, 2025 at 3:35 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 01/07/2025 07:36, Guodong Xu wrote:
>>> Add "spacemit,k1-pdma" compatible string to support SpacemiT K1 PDMA
>>> controller. This variant requires:
>>
>> Why is this marvell? This should be explained here, it's really unexpected.
>>
> 
> SpacemiT K1 SoC uses the same DMA controller as Marvell MMP. They share most
> of the registers (and address offsets) and only enhanced in addressing space
> capability (from 32bit to 64bit).

I hope you got the last comment...

> 
> Also, spacemit,k1-pdma and marvell,pdma-1.0 use the same driver (mmp_pdma.c),
> that's the reason why I chose keeping them in the same binding file.

That's moderate reason. I explained here further - don't grow old
bindings with completely new devices, because you keep growing old,
poorer patterns. You have a new device, you can make it right.

...

>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: spacemit,k1-pdma
>>> +    then:
>>> +      required:
>>> +        - clocks
>>> +        - resets
>>> +    else:
>>> +      properties:
>>> +        clocks: false
>>> +        resets: false
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - marvell,pdma-1.0
>>> +              - spacemit,k1-pdma
>>> +    then:
>>> +      properties:
>>> +        '#dma-cells':
>>> +          const: 2
>>> +          description:
>>> +            The first cell contains the DMA request number for the peripheral
>>> +            device. The second cell is currently unused but must be present for
>>> +            backward compatibility.
>>> +    else:
>>> +      properties:
>>> +        '#dma-cells':
>>> +          const: 1
>>> +          description:
>>> +            The cell contains the DMA request number for the peripheral device.
>>
>>
>> It's getting complicated. I suggest to make your own schema. Then you
>> would also switch to preferred 'sram' property instead of that legacy
>> 'asram'.
>>
>> Really, ancient schemas should not be grown for new, completely
> 
> The reason that they share the same device driver may not be strong enough
> compared to what you said here.

If DMA maintainer(s) reject complexity in driver, then it is fine. But
till that happens, you should rather come with a clean new binding and
don't grow legacy.

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv