diff mbox series

spi: mt7621: allow GPIO chip select lines

Message ID 20240315015708.13948-1-justin.swartz@risingedge.co.za
State New
Headers show
Series spi: mt7621: allow GPIO chip select lines | expand

Commit Message

Justin Swartz March 15, 2024, 1:57 a.m. UTC
Extract a magic number, from mt7621_spi_probe(), used to
declare the number of chip select lines (which co-incides
with the native chip select count of 2) to a macro.

Use the newly defined MT7621_NATIVE_CS_COUNT macro to
instead populate both the spi_controller's max_native_cs
and num_chipselect members.

Declare that the spi_controller should use_gpio_descriptors
if present in the device properties (such as those declared
in the cs-gpio property of a "ralink,mt7621-spi" compatible
device-tree node) so that the SPI core will recalulcate
num_chipselect to account for the GPIO descriptors that
it should have populated in the cs_gpiod array member.

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.

Add the mt7621_spi_cleanup() function to ensure that every
GPIO chip select will be deactivated when no longer in use.

Extend mt7621_spi_setup() so that if an SPI device is
associated with a GPIO chip select, its chip select line
will be deactivated before use.

Rename mt7621_spi_set_cs() to mt7621_spi_set_native_cs(),
and redefine mt7621_spi_set_cs() to determine whether:

  to call mt7621_spi_set_cs_gpio(), in the case of the
  passed SPI device being associated with a GPIO chip
  select line,

  or to call mt7621_spi_set_set_native_cs() instead.

Modify mt7621_transfer_one_message() to take into account
that mt7621_spi_set_cs() now returns an int and should use
the returned value for spi_message status indication if a
failure related to GPIO access has occured.

Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
---

See Documentation/devicetree/bindings/spi/spi-controller.yaml
for information about cs-gpios semantics.

Example:

&spi0 {
	cs-gpios = <0>, <0>,
	           <&gpio 18 GPIO_ACTIVE_LOW>,   /* WDT_RST_N */
	           <&gpio 19 GPIO_ACTIVE_HIGH>;  /* PERST_N   */
	status = "ok";

	...

	spidev@2 {
		compatible = "defective,by-design";
		reg = <2>;
		spi-max-frequency = <16000000>;
	};

	spidev@3 {
		compatible = "defective,by-design";
		reg = <3>;
		spi-cs-high;
		spi-max-frequency = <16000000>;
	};
}; 	 

 drivers/spi/spi-mt7621.c | 46 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

--

Comments

Mark Brown March 15, 2024, 2:45 p.m. UTC | #1
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?
Mark Brown March 15, 2024, 8:41 p.m. UTC | #2
On Fri, Mar 15, 2024 at 10:21:53PM +0200, Justin Swartz wrote:
> On 2024-03-15 19:47, Mark Brown wrote:

> > Look at other drivers that support GPIO chip selects?

> Of the 43 drivers (of drivers/spi/*.c) that setup the
> spi_controller's use_gpio_descriptors as true:

>   39 drivers use the transfer_one hook, and
>    4 drivers use the transfer_one_message hook.

> Drivers that use the transfer_one hook benefit from the core
> taking care of chip selection on their behalf.

> Drivers that use the transfer_one_message hook handle chip
> selection on their own, within the function they've pointed
> the hook at.

Oh, this is an old school driver.  Glancing at the code I can't see any
particular reason why it's not using transfer_one(), you should just
convert the driver to that which will reduce the open coding and just
generally improve functionality.  You could add a callback to flush the
write FIFO or add that into the write function, I'm not sure if there's
a meaningful performance benefit there.

> Considering spi-mt7621.c was implemented using the
> transfer_one_message() hook, I'd assumed that it made more

I think it's just old and based on having gone through staging likely
based on even older BSP code.
diff mbox series

Patch

diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c
index 4e9053d03..87e164c86 100644
--- a/drivers/spi/spi-mt7621.c
+++ b/drivers/spi/spi-mt7621.c
@@ -52,6 +52,8 @@ 
 #define MT7621_CPOL		BIT(4)
 #define MT7621_LSB_FIRST	BIT(3)
 
+#define MT7621_NATIVE_CS_COUNT	2
+
 struct mt7621_spi {
 	struct spi_controller	*host;
 	void __iomem		*base;
@@ -75,7 +77,19 @@  static inline void mt7621_spi_write(struct mt7621_spi *rs, u32 reg, u32 val)
 	iowrite32(val, rs->base + reg);
 }
 
-static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
+static int mt7621_spi_set_cs_gpio(struct spi_device *spi, int enable)
+{
+	struct gpio_desc *gpiod = spi_get_csgpiod(spi, 0);
+	int cs = spi_get_chipselect(spi, 0);
+	int status = gpiod_direction_output(gpiod, enable);
+
+	if (status)
+		dev_err(&spi->dev, "set_gpio: failed to set CS%d", cs);
+
+	return status;
+}
+
+static void mt7621_spi_set_native_cs(struct spi_device *spi, int enable)
 {
 	struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
 	int cs = spi_get_chipselect(spi, 0);
@@ -99,6 +113,15 @@  static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
 	mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
 }
 
+static int mt7621_spi_set_cs(struct spi_device *spi, int enable)
+{
+	if (spi_is_csgpiod(spi))
+		return mt7621_spi_set_cs_gpio(spi, enable);
+
+	mt7621_spi_set_native_cs(spi,enable);
+	return 0;
+}
+
 static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)
 {
 	struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
@@ -266,7 +289,9 @@  static int mt7621_spi_transfer_one_message(struct spi_controller *host,
 	}
 
 	/* Assert CS */
-	mt7621_spi_set_cs(spi, 1);
+	status = mt7621_spi_set_cs(spi, 1);
+	if (status)
+		goto msg_done;
 
 	m->actual_length = 0;
 	list_for_each_entry(t, &m->transfers, transfer_list) {
@@ -290,7 +315,7 @@  static int mt7621_spi_transfer_one_message(struct spi_controller *host,
 
 	/* Flush data and deassert CS */
 	mt7621_spi_flush(rs);
-	mt7621_spi_set_cs(spi, 0);
+	status = mt7621_spi_set_cs(spi, 0);
 
 msg_done:
 	m->status = status;
@@ -313,9 +338,18 @@  static int mt7621_spi_setup(struct spi_device *spi)
 		return -EINVAL;
 	}
 
+	if (spi_is_csgpiod(spi))
+		return mt7621_spi_set_cs_gpio(spi, 0);
+
 	return 0;
 }
 
+static void mt7621_spi_cleanup(struct spi_device *spi)
+{
+	if (spi_is_csgpiod(spi))
+		mt7621_spi_set_cs_gpio(spi, 0);
+}
+
 static const struct of_device_id mt7621_spi_match[] = {
 	{ .compatible = "ralink,mt7621-spi" },
 	{},
@@ -353,10 +387,14 @@  static int mt7621_spi_probe(struct platform_device *pdev)
 	host->mode_bits = SPI_LSB_FIRST;
 	host->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	host->setup = mt7621_spi_setup;
+	host->cleanup = mt7621_spi_cleanup;
 	host->transfer_one_message = mt7621_spi_transfer_one_message;
 	host->bits_per_word_mask = SPI_BPW_MASK(8);
 	host->dev.of_node = pdev->dev.of_node;
-	host->num_chipselect = 2;
+
+	host->max_native_cs = MT7621_NATIVE_CS_COUNT;
+	host->num_chipselect = host->max_native_cs;
+	host->use_gpio_descriptors = true;
 
 	dev_set_drvdata(&pdev->dev, host);