Re: [PATCH v4 2/3] spi: Add Amlogic SPISG driver

[Xianwei Zhao]

Hi Mark,

On 2025/7/9 15:02, Xianwei Zhao wrote:
> Hi Mark,
>     Thanks for your advice.
>
> On 2025/7/8 21:50, Mark Brown wrote:
> > Subject:
> > Re: [PATCH v4 2/3] spi: Add Amlogic SPISG driver
> > From:
> > Mark Brown <broonie@xxxxxxxxxx>
> > Date:
> > 2025/7/8 21:50
> >
> > To:
> > Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
> > CC:
> > Sunny Luo <sunny.luo@xxxxxxxxxxx>, Rob Herring <robh@xxxxxxxxxx>, 
> > Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>, Conor Dooley 
> > <conor+dt@xxxxxxxxxx>, linux-amlogic@xxxxxxxxxxxxxxxxxxx, 
> > linux-spi@xxxxxxxxxxxxxxx, devicetree@xxxxxxxxxxxxxxx, 
> > linux-kernel@xxxxxxxxxxxxxxx
> >
> >
> >
> > On Tue, Jul 08, 2025 at 06:34:02PM +0800, Xianwei Zhao wrote:
> > > On 2025/7/7 21:05, Mark Brown wrote:
> > > > Is it worth having a copybreak such that smaller transfers are done
> > > > using PIO?  With a lot of controllers that increases performance due to
> > > > the extra overhead of setting up DMA, talking to the DMA and interrupt
> > > > controllers can be as expensive as directly accessing the FIFOs.
> > > If the data volume of a single transfer (xfer) is small, PIO mode 
> > > does offer
> > > some advantages. However, since PIO requires the CPU to wait in a 
> > > busy loop
> > > for the transfer to complete, it continuously occupies CPU resources. 
> > > As a
> > > result, its advantages are not particularly significant.
> > The CPU overhead tends to be higher (you can avoid some of it with a
> > dead reckoning sleep), but the latency vastly improved which for many
> > applications is a worthwhile advantage.  It tends to be things like
> > accesses to one or two registers on a device with registers where this
> > wins, 16 bytes or lower would be a common number off the top of my head.
> >
> > > If PIO is to be implemented, it can only handle one transfer at a 
> > > time (via
> > > transfer_one), and not entire messages (which consist of multiple
> > > transfers). In contrast, when processing messages, the SPI controller 
> > > can
> > > handle the entire sequence in one go, which also provides certain 
> > > benefits.
> > It's probably worth adding something to the framework to be able to take
> > a decision at the message level, for writes this tends to all fall out
> > naturally since the write will tend to be a single transfer anyway.
>
> I will try to add new API message_can_dma for framework, and implement 
> PIO for message.

I tried to implement PIO mode in the driver, but it turned out to be too 
slow. Due to the lack of an internal FIFO, data could only be 
transmitted one word at a time, and each transmission required 
reconfiguring the corresponding registers. As a result, the efficiency 
was quite low.

The only simplifications provided by PIO mode were in two areas:

1. The allocation and release of the transfer descriptor
2. The DMA mapping and unmapping process

Therefore, I suggest not implementing PIO mode in this driver. I will 
document clearly in the code that PIO mode is not supported and explain 
the reasons behind this decision.