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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241105075012.GC923511@robin.jannau.net>
Date: Tue, 5 Nov 2024 08:50:12 +0100
From: Janne Grunau <j@...nau.net>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Janne Grunau via B4 Relay <devnull+j.jannau.net@...nel.org>,
	asahi@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-spi@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, Hector Martin <marcan@...can.st>,
	Sven Peter <sven@...npeter.dev>,
	Alyssa Rosenzweig <alyssa@...enzweig.io>,
	Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>
Subject: 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 :
> > From: Hector Martin <marcan-WKacp4m3WJJeoWH0uzbU5w@...lic.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@...lic.gmane.org>
> > Signed-off-by: Janne Grunau <j@...nau.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