Received-SPF: pass (google.com: domain of linux-kernel+bounces-104624-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Fri, 15 Mar 2024 18:23:09 +0200
From: Justin Swartz <justin.swartz@risingedge.co.za>
To: Mark Brown <broonie@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno
 <angelogioacchino.delregno@collabora.com>, linux-spi@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] spi: mt7621: allow GPIO chip select lines
In-Reply-To: <d562be73-ad76-4450-8bff-38ed5d144714@sirena.org.uk>
References: <20240315015708.13948-1-justin.swartz@risingedge.co.za>
 <d562be73-ad76-4450-8bff-38ed5d144714@sirena.org.uk>
Message-ID: <2dbc59c9133542f6f8bc465113d9630b@risingedge.co.za>
User-Agent: Roundcube Webmail/1.3.17

On 2024-03-15 16:45, Mark Brown wrote:
> On Fri, Mar 15, 2024 at 03:57:07AM +0200, Justin Swartz wrote:
> 
>> Add the mt7621_spi_set_cs_gpio() function to control the
>> logical state of a GPIO chip select line, agnostic of the
>> electrical line state and activation polarity.
> 
> The core should handle GPIO chip selects for you?

As far as I can tell, it doesn't - at least as far the state
of spi-mt7621.c is concerned prior to the patch, plus kernel
configuration choices, device tree definition, and other
factors I might not be taking into account.

But maybe I'm doing something wrong, or perhaps have a
misconfiguration somewhere. So, if you're able to point out
something I've done incorrectly, it would be appreciated.

To attempt to confirm if the core will handle my desired
GPIO chip select lines without explicit state toggling,
I tried to set the value of use_gpio_descriptors to true,
without any other modifications to spi-mt7621.c as of
commit 90d35da658da8cff0d4ecbb5113f5fac9d00eb72:

[... Sorry if my tabs decide to be spaces instead ...]

---%---
--- a/drivers/spi/spi-mt7621.c
+++ b/drivers/spi/spi-mt7621.c
@@ -357,6 +357,7 @@ static int mt7621_spi_probe(struct platform_device 
*pdev)
         host->bits_per_word_mask = SPI_BPW_MASK(8);
         host->dev.of_node = pdev->dev.of_node;
         host->num_chipselect = 2;
+       host->use_gpio_descriptors = true;

         dev_set_drvdata(&pdev->dev, host);
---%---

I use a smallish program to write(2) a few bytes from
stdin to an spidev node.

---%---
#include <linux/spi/spidev.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>

static int      Device      = -1;
static uint32_t Mode        = 0;
static uint32_t BitsPerWord = 8;
static uint32_t MaxSpeed    = 100000;

#define BUFFER_SIZE 4096
static uint8_t  Buffer[BUFFER_SIZE];

static int  openDevice(char *);
static void closeDevice(void);
static int  transmit(void);

int main(int argc, char *argv[])
{
         if (argc != 2) {
                 puts("usage: spiw SPI-DEVICE");
                 return EXIT_FAILURE;
         }

         atexit(closeDevice);

         if (openDevice(argv[1]) == -1)
                 return EXIT_FAILURE;

         while (!feof(stdin))
                 if (transmit() == -1)
                         return EXIT_FAILURE;

         return EXIT_SUCCESS;
}

static int openDevice(char *filename)
{
         Device = open(filename, O_RDWR);

         if (Device == -1)
                 return -1;

         if (ioctl(Device, SPI_IOC_WR_MODE32, &Mode) == -1)
                 return -1;

         if (ioctl(Device, SPI_IOC_WR_BITS_PER_WORD, &BitsPerWord) == -1)
                 return -1;

         if (ioctl(Device, SPI_IOC_WR_MAX_SPEED_HZ, &MaxSpeed) == -1)
                 return -1;

         return 0;
}

static void closeDevice(void)
{
         if (Device != -1)
                 close(Device);
}

static int transmit(void)
{
         size_t length = fread(Buffer, 1, sizeof(Buffer), stdin);

         if (ferror(stdin))
                 return -1;

         return write(Device, Buffer, length);
}
---%---

If I send write some data to a device associated with one
of the GPIO chip selects while watching the signals on the
SPI bus, I can see the expected transitions on SCLK and MOSI
but there isn't any change on the expected CS line, nor any
others:

~ # printf "\x41" | /tmp/spiw /dev/spidev0.2


A rough diagram, to show that although 'A' was sent, the
chip select wasn't activated:

       ______________________________________________________
CS2

                __    __    __    __    __    __    __    __
SCLK  ________|  |__|  |__|  |__|  |__|  |__|  |__|  |__|  |

       ________    _____                               ______
MOSI          |__|     |_____________________________|
                      :                                   :
                :     :     :     :     :     :     :     :
                0     1     0     0     0     0     0     1