Re: [PATCH v3 2/3] spi: apple: Add driver for Apple SPI controller

On Mon, Nov 04, 2024 at 08:16:25PM +0100, Christophe JAILLET wrote:

Le 01/11/2024 à 20:26, Janne Grunau via B4 Relay a écrit :

quoted

From: Hector Martin <redacted>

This SPI controller is present in Apple SoCs such as the M1 (t8103) and
M1 Pro/Max (t600x). It is a relatively straightforward design with two
16-entry FIFOs, arbitrary transfer sizes (up to 2**32 - 1) and fully
configurable word size up to 32 bits. It supports one hardware CS line
which can also be driven via the pinctrl/GPIO driver instead, if
desired. TX and RX can be independently enabled.

There are a surprising number of knobs for tweaking details of the
transfer, most of which we do not use right now. Hardware CS control
is available, but we haven't found a way to make it stay low across
multiple logical transfers, so we just use software CS control for now.

There is also a shared DMA offload coprocessor that can be used to handle
larger transfers without requiring an IRQ every 8-16 words, but that
feature depends on a bunch of scaffolding that isn't ready to be
upstreamed yet, so leave it for later.

The hardware shares some register bit definitions with spi-s3c24xx which
suggests it has a shared legacy with Samsung SoCs, but it is too
different to warrant sharing a driver.

Signed-off-by: Hector Martin <redacted>
Signed-off-by: Janne Grunau <j@jannau.net>
---

Hi,

quoted

diff --git a/drivers/spi/spi-apple.c b/drivers/spi/spi-apple.c
new file mode 100644
index 0000000000000000000000000000000000000000..1a3f61501db56d0d7689cc3d6f987bf636130cdb
--- /dev/null
+++ b/drivers/spi/spi-apple.c

@@ -0,0 +1,531 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Apple SoC SPI device driver
+//
+// Copyright The Asahi Linux Contributors
+//
+// Based on spi-sifive.c, Copyright 2018 SiFive, Inc.
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/module.h>

Move a few lines below to keep alphabetical order?

done

quoted

+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>

...

quoted

+static int apple_spi_probe(struct platform_device *pdev)
+{
+	struct apple_spi *spi;
+	int ret, irq;
+	struct spi_controller *ctlr;
+
+	ctlr = devm_spi_alloc_master(&pdev->dev, sizeof(struct apple_spi));
+	if (!ctlr)
+		return -ENOMEM;
+
+	spi = spi_controller_get_devdata(ctlr);
+	init_completion(&spi->done);
+	platform_set_drvdata(pdev, ctlr);

Is it needed?
There is no platform_get_drvdata()

no, it is not used anymore. It's a leftover from v1 which used
platform_get_drvdata() in apple_spi_remove() which is gone now.

thanks, I'll send a v4 shortly
Janne