linux-kernel - Re: [PATCH] spi: dt-bindings: samsung: make dma properties not required

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cbbeec8c-45c7-0f62-8947-90511fdc1f25@samsung.com>
Date: Tue, 5 Mar 2024 14:08:19 +0900
From: Jaewon Kim <jaewon02.kim@...sung.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>, Rob Herring <robh@...nel.org>,
	Sam Protsenko <semen.protsenko@...aro.org>
Cc: Mark Brown <broonie@...nel.org>, Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>, andi.shyti@...nel.org,
	conor+dt@...nel.org, linux-spi@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, andre.draszik@...aro.org,
	peter.griffin@...aro.org, willmcvicker@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH] spi: dt-bindings: samsung: make dma properties not
 required

Hello all,


On 24. 3. 5. 03:15, Tudor Ambarus wrote:
> Hi, Rob,
>
> On 3/4/24 16:56, Rob Herring wrote:
>> On Sat, Mar 02, 2024 at 10:23:16AM -0600, Sam Protsenko wrote:
>>> On Sat, Mar 2, 2024 at 3:36 AM Tudor Ambarus <tudor.ambarus@...aro.org> wrote:
>>>>
>>>>
>>>> On 01.03.2024 22:42, Mark Brown wrote:
>>>>> On Fri, Mar 01, 2024 at 01:28:35PM -0600, Sam Protsenko wrote:
>>>>>> On Fri, Mar 1, 2024 at 5:55 AM Tudor Ambarus <tudor.ambarus@...aro.org> wrote:
>>>>>>> Since the addition of the driver in 2009, the driver selects between DMA
>>>>>>> and polling mode depending on the transfer length - DMA mode for
>>>>>>> transfers bigger than the FIFO depth, polling mode otherwise. All
>>>>>>> versions of the IP support polling mode, make the dma properties not
>>>>>>> required.
>>>>>> AFAIU, the device tree has nothing to do with drivers, it's about
>>>>>> hardware description. Does making DMA properties not required here
>>>> correct
>>>>
>>>>>> mean that there are some HW out there which doesn't integrate DMA in
>>>> no, to me it means that the IP can work without DMA, only in PIO mode,
>>>> regardless if DMA is integrated or not. Not required means that the
>>>> property is not mandatory, which is what I'm trying to achieve here.
>>>>
>>>>>> SPI blocks? Even if this change is ok (I'm not sure), the
>>>>>> argumentation doesn't look sound to me.
>>>> switching to PIO mode in the driver for sizes smaller than FIFO depths
>>>> in the driver guarantees that all existing compatibles support PIO mode.
>>>>
>>>> Are you saying that if there is a physical line between an IP and DMA
>>>> controller, then the DMA properties must always be specified in dt? I
>>>> thought they can be marked as optional in this case, and that's what I
>>>> did with this patch.
>>>>
>>> No, I would wait for maintainers to clarify on that bit. Change itself
>>> can be ok. But the commit message shouldn't mention the driver,
>>> because the driver uses (depends on) device tree, not vice versa. The
>>> device tree can be used in other projects as well (like U-Boot and
>>> OP-TEE), so it should be designed to be universal and not depend on
>>> kernel drivers. The commit message should be based on particular HW
>>> layout features and how the patch makes the bindings describe that HW
>>> better. It shouldn't rely on driver implementations.
>> If the controller is DMA capable then it should have dma properties. The
> should have as in required/mandatory?
>
>> compatible should be enough to tell if it is a case of 'can only work
> yes, I agree
>
>> with DMA'. Otherwise, it is going to be up to a specific user. Even
>> within Linux, you may have a serial port that doesn't use DMA for the
>> console, but uses it for the tty or serdev.
>>
>> Of course, if a new device is added without DMA properties and they
>> are added later on, then they are going to be optional even though the
>> DMA support is always there. I can't fully understand everyone's h/w.
>>
> The SPI controller that I'm working with has a dedicated channel to the
> DMA controller. It can work without DMA too, just by polling registers
> or by interrupts.
>
> I can't get the DMA controller to work correctly yet, and since the SPI
> controller can work without DMA, I thought that I can mark the DMA
> properties as optional, add the SPI node in dt without DMA, and add the
> DMA properties later on, after I have the DMA controller working
> correctly. Is this approach wrong?
>
> Thanks,
> ta
>
>

I agree with this patch.

I don`t think DMA property needs to be "required" because it can operate 
well without DMA property.


Last year, I put a patch that makes dma property optional.

  - d1a7718ee8db (spi: s3c64xx: change polling mode to optional)

- https://lore.kernel.org/r/20230502062813.112434-2-jaewon02.kim@samsung.com


In the past, there was SoC without DMA, so it was a quirk that used 
polling mode.

Now, I want to change this to be optional to support cases where DMA is 
not available according to OS environment(Virtual Machine).


Thanks

Jaewon Kim