Re: [PATCH 2/2] spi: Add Baikal-T1 System Boot SPI Controller driver

[Chris Packham]

On 10/05/20 12:20 pm, Serge Semin wrote:
> On Fri, May 08, 2020 at 12:37:51PM +0100, Mark Brown wrote:
<snip>
>>> +	writel(BIT(req->cs), bs->regs + BC_SPI_SER);
>>> +	if (req->cs_gpiod) {
>>> +		gpiod_set_value_cansleep(req->cs_gpiod,
>>> +					 !!(bs->cfg.mode & SPI_CS_HIGH));
>> If you have a GPIO chip select you should just let the core manage it
>> through cs_gpiod rather than open coding.
> Of course I know this, and normally I would have omitted the GPIO manual
> assertion (hopefully soon my hands get to merging the AX99100 driver I've
> developed some time ago). The thing is that this Baikal-T1 System SSI device
> driver has been initially written before commit 05766050d5bd ("spi: spi-mem:
> fallback to using transfers when CS gpios are used"). So asserting GPIO CS had
> been required to initiate the SPI memory communications seeing the generic
> spi_mem_exec_op() doesn't do this. Manual GPIO manipulation is indeed redundant
> for the current SPI-mem op execution procedure.
>
> Secondly the message of that commit states "Devices with chip selects driven
> via GPIO are not compatible with the spi-mem operations." I find this statement
> questionable, because for instance this device supports memory operations with
> GPIO-driven CS. Though in current implementation the driver fallback to using normal
> push-pull IO mode if GPIO CS is utilized as safer one. But even in this case
> it's better than splitting the memory operations up into the transfers, which is
> developed in the spi_mem_exec_op() method.
On this specific bit. My use-case for 05766050d5bd was a SPI controller 
that supported direct mem accesses but a hardware design that required a 
GPIO CS. So yes I probably should have qualified it as _some_ devices.
> So in this matter my question is: how to modify the SPI-mem interface so the
> SPI-memory operations would also work with GPIO driven CS? Some additional flag
> might work...