Re: [PATCH v2 1/2] SPI: Add SPI driver for Sunplus SP7021

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]

On Thu, Nov 11, 2021 at 08:32:39AM +0000, Lh Kuo 郭力豪 wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> #define SPI_CS_HIGH   0x04 /* chipselect active high? */
> Is it mean?
> CASE1 : standby, CS high => start transfer CS become low => transfer end CS become high and standby
> CASE2 : standby, CS low => start transfer CS become high => transfer end CS become low and standby

> I think SPI_CS_HIGH means CASE2, But it is strange that more chipset work in CASE1 but drivers set SPI_CS_HIGH as defined.

SPI_CS_HIGH is case 2.

> 2. And in the CASE1 I should set 
> cs_gpios = 	<gpio 3 2 GPIO_ACTIVE_LOW>,
> or
> cs_gpios = 	<gpio 3 2 GPIO_ACTIVE_HIGH>,

_ACTIVE_HIGH if _CS_HIGH is specified, though the binding will try to
sort things out either way.

> 3. If I did not set the max_transfer_size of spi_control
> And use transfer_one set max_transfer_size and use_gpio_descriptors
> Can it transmit data that exceeds FIFO max bytes (even exceed HW's one-time transfer) in one transmission activity?

Yes, if you don't set a maximum transfer size the driver might get any
transfer size.  If you set a maximum transfer size then the driver will
not see any transfers that exceed the maximum transfer size.

> This is my concern, so I use Transfer_One_message

I can't understand how that would follow on.  If there's a limit on the
size of an individual transfer then tell the framework about that limit,
that's all that needs doing.  Why would it be preferable to not tell the
core about the limit and instead open code things?

*Please* think about the lengthy explanation I provided in my last
message about putting things that are not device specific in the
framework not the driver.

> Ex : Need to transmit 4000 bytes. 
>   Then I set Ctlr->transfer_one and use_gpio_descriptors
>     ctlr->max_transfer_size = 255;
>     The CS of device is low active

>    When the transmission starts, I can see the signal gpio-CS changes from high to low
> Ctlr->transfer_one will be triggered to execute 16 times, and transfer end gpio-CS changes from low to high.

This is exactly what will happen if you do as has been repeatedly
suggested.  Set a maximum *transfer* (not message) size, let the core
handle the chip select GPIO and implement transfer_one().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]