| Message ID | 20241101-asahi-spi-v3-0-3b411c5fb8e5@jannau.net |
|---|---|
| Headers | show |
| Series | Apple SPI controller driver | expand |
On Fri, Nov 01, 2024 at 08:26:11PM +0100, Janne Grunau wrote: > Hi all, > > This updated series address the review comments from the original > submission in 2021 [1]. It adds a new SPI controller driver for Apple > SoCs and is based on spi-sifive. It has been tested with the generic > jedec,spi-nor support and with a downstream driver for an Apple specific > HID over SPI transport. > > As usual, I'm splitting off the MAINTAINERS and DT binding changes. > We would rather merge the MAINTAINERS change through the Asahi-SoC > tree to avoid merge conflicts as things trickle upstream, since > we have other submissions touching that section of the file. > > The DT binding change can go via the SPI tree or via ours, but it's > easier if we merge it, as then we can make the DT changes to > instantiate it without worrying about DT validation failures depending > on merge order. > > This is mostly Hector's work with a few minor changes to address review > comments from me. > > [1] https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ > > v2: > - removed '#address-cells' and '#size-cells' from the bindings and added > Rob's Rb: Where? Best regards, Krzysztof
Le 01/11/2024 à 20:26, Janne Grunau via B4 Relay a écrit : > From: Hector Martin <marcan-WKacp4m3WJJeoWH0uzbU5w@public.gmane.org> > > 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 <marcan-WKacp4m3WJJeoWH0uzbU5w@public.gmane.org> > Signed-off-by: Janne Grunau <j@jannau.net> > --- Hi, > 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? > +#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> ... > +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() > + > + spi->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(spi->regs)) > + return PTR_ERR(spi->regs); > + > + spi->clk = devm_clk_get_enabled(&pdev->dev, NULL); > + if (IS_ERR(spi->clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(spi->clk), > + "Unable to find or enable bus clock\n"); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + ret = devm_request_irq(&pdev->dev, irq, apple_spi_irq, 0, > + dev_name(&pdev->dev), spi); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "Unable to bind to interrupt\n"); > + > + ctlr->dev.of_node = pdev->dev.of_node; > + ctlr->bus_num = pdev->id; > + ctlr->num_chipselect = 1; > + ctlr->mode_bits = SPI_CPHA | SPI_CPOL | SPI_LSB_FIRST; > + ctlr->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32); > + ctlr->prepare_message = apple_spi_prepare_message; > + ctlr->set_cs = apple_spi_set_cs; > + ctlr->transfer_one = apple_spi_transfer_one; > + ctlr->auto_runtime_pm = true; > + > + pm_runtime_set_active(&pdev->dev); > + ret = devm_pm_runtime_enable(&pdev->dev); > + if (ret < 0) > + return ret; > + > + apple_spi_init(spi); > + > + ret = devm_spi_register_controller(&pdev->dev, ctlr); > + if (ret < 0) > + return dev_err_probe(&pdev->dev, ret, "devm_spi_register_controller failed\n"); > + > + return 0; > +} ... CJ
On Sat, Nov 02, 2024 at 02:11:51PM +0100, Krzysztof Kozlowski wrote: > On Fri, Nov 01, 2024 at 08:26:11PM +0100, Janne Grunau wrote: > > Hi all, > > > > This updated series address the review comments from the original > > submission in 2021 [1]. It adds a new SPI controller driver for Apple > > SoCs and is based on spi-sifive. It has been tested with the generic > > jedec,spi-nor support and with a downstream driver for an Apple specific > > HID over SPI transport. > > > > As usual, I'm splitting off the MAINTAINERS and DT binding changes. > > We would rather merge the MAINTAINERS change through the Asahi-SoC > > tree to avoid merge conflicts as things trickle upstream, since > > we have other submissions touching that section of the file. > > > > The DT binding change can go via the SPI tree or via ours, but it's > > easier if we merge it, as then we can make the DT changes to > > instantiate it without worrying about DT validation failures depending > > on merge order. > > > > This is mostly Hector's work with a few minor changes to address review > > comments from me. > > > > [1] https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ > > > > v2: > > - removed '#address-cells' and '#size-cells' from the bindings and added > > Rob's Rb: > > Where? Apparently only in my mind. fixed for v4 thanks, Janne
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 : > > From: Hector Martin <marcan-WKacp4m3WJJeoWH0uzbU5w@public.gmane.org> > > > > 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 <marcan-WKacp4m3WJJeoWH0uzbU5w@public.gmane.org> > > Signed-off-by: Janne Grunau <j@jannau.net> > > --- > > Hi, > > > 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 > > +#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> > > ... > > > +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
Hi all, This updated series address the review comments from the original submission in 2021 [1]. It adds a new SPI controller driver for Apple SoCs and is based on spi-sifive. It has been tested with the generic jedec,spi-nor support and with a downstream driver for an Apple specific HID over SPI transport. As usual, I'm splitting off the MAINTAINERS and DT binding changes. We would rather merge the MAINTAINERS change through the Asahi-SoC tree to avoid merge conflicts as things trickle upstream, since we have other submissions touching that section of the file. The DT binding change can go via the SPI tree or via ours, but it's easier if we merge it, as then we can make the DT changes to instantiate it without worrying about DT validation failures depending on merge order. This is mostly Hector's work with a few minor changes to address review comments from me. [1] https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ v2: - removed '#address-cells' and '#size-cells' from the bindings and added Rob's Rb: - fixed (new) checkpatch warnings - added t8112 (M2) SoC - shorted long and complex source code lines - switch to devm_clk_prepare_enable() and devm_pm_runtime_enable() - switch to dev_err_probe() in probe function - removed "pdev->dev.dma_mask = NULL;" - got rid of apple_spi_remove() Signed-off-by: Janne Grunau <j@jannau.net> --- Changes in v3: - fixed bindings_check warning - converted top file comment to C++ style comments - dropped verbose dev_err_probe after failed devm_* function - stopped setting field in zero initialized struct to 0 - added error handling for devm_pm_runtime_enable() - Link to v2: https://lore.kernel.org/r/20241101-asahi-spi-v2-0-763a8a84d834@jannau.net Changes in v2: - removed '#address-cells' and '#size-cells' from the bindings and added Rob's Rb: - fixed (new) checkpatch warnings - added t8112 (M2) SoC - shorted long and complex source code lines - switch to devm_clk_prepare_enable() and devm_pm_runtime_enable() - switch to dev_err_probe() in probe function - removed "pdev->dev.dma_mask = NULL;" - got rid of apple_spi_remove() - Link to v1: https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ --- Hector Martin (3): dt-bindings: spi: apple,spi: Add binding for Apple SPI controllers spi: apple: Add driver for Apple SPI controller MAINTAINERS: Add apple-spi driver & binding files .../devicetree/bindings/spi/apple,spi.yaml | 62 +++ MAINTAINERS | 2 + drivers/spi/Kconfig | 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-apple.c | 531 +++++++++++++++++++++ 5 files changed, 607 insertions(+) --- base-commit: 98f7e32f20d28ec452afb208f9cffc08448a2652 change-id: 20240811-asahi-spi-f8740ba797d7 Best regards,