Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 25 Nov 2022 13:04:54 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Jonathan =?iso-8859-1?Q?Neusch=E4fer?= <j.neuschaefer@gmx.net>
Cc:     linux-spi@vger.kernel.org, openbmc@lists.ozlabs.org,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] spi: wpcm-fiu: Add driver for Nuvoton WPCM450
 Flash Interface Unit (FIU)
Message-ID: <Y4C9druicCm0m3wi@sirena.org.uk>
References: <20221124191400.287918-1-j.neuschaefer@gmx.net>
 <20221124191400.287918-3-j.neuschaefer@gmx.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="smw0hoe4UY9/oecT"
Content-Disposition: inline
In-Reply-To: <20221124191400.287918-3-j.neuschaefer@gmx.net>
Precedence: bulk


--smw0hoe4UY9/oecT
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Nov 24, 2022 at 08:13:59PM +0100, Jonathan Neusch=E4fer wrote:

> The Flash Interface Unit (FIU) is the SPI flash controller in the
> Nuvoton WPCM450 BMC SoC. It supports four chip selects, and direct
> (memory-mapped) access to 16 MiB per chip. Larger flash chips can be
> accessed by software-defined SPI transfers.

Those software defined SPI transfers seem to be most of the way to
supporting normal SPI controller operations, they just need wiring up.
That would both support people hooking other SPI chips up to the board
and might help support future flash stuff without needing custom code in
the driver like you've got now.

> +static int wpcm_fiu_do_uma(struct wpcm_fiu_spi *fiu, unsigned int cs,
> +			   bool use_addr, bool write, int data_bytes)
> +{

This appears to only support half duplex access but the driver doesn't
flag itself as SPI_CONTROLLER_HALF_DUPLEX. =20


> +	cts |=3D FIU_UMA_CTS_D_SIZE(data_bytes);

I'm guessing there's a limit on data_bytes that should be enforced.  The
driver should probably also flag a max transfer size, though that might
cause issues if the limit is different between spi-mem and regular
transfers.

> +/*
> + * RDID (Read Identification) needs special handling because Linux expec=
ts to
> + * be able to read 6 ID bytes and FIU can only read up to 4 at once.
> + *
> + * We're lucky in this case, because executing the RDID instruction twic=
e will
> + * result in the same result.
> + *
> + * What we do is as follows (C: write command/opcode byte, D: read data =
byte,
> + * A: write address byte):
> + *
> + *  1. C D D D
> + *  2. C A A A D D D
> + */

If the driver were implementing regular SPI operations and advertising
a maximum transfer length this should just work without having to jump
through hoops.  The core can split transfers up into sections that fit
within the controller limits transparently.

--smw0hoe4UY9/oecT
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmOAvXUACgkQJNaLcl1U
h9DAMAf/X47Ck+FvdSSBNIQ2z8oJdpjFNxzfM7WEi4iqoF1oOcS8IcsxYATK2qDA
1jz12Ea4fmOTjGJdikq/DeNru44Rcfu/zybRf22nlhAAVMOMh6CAFC1vp2sOhqEM
db+4lBppUmKISAdxNZ2J0JzUjjA+rdXIFEe8aljjQPNo6OkAcoKiRI8pJHX8nYiv
Ps6looHKtrsBiVvyUvrBvTT2OUuFQf976dgg8VwMOLc+5wAJPNyBxvpBrp7YRXrp
rj+wTZ9QHu5bHYR09EHkdkz41AKVd8VfvnzdWD596OjnxpCoRXlwOO3x3O2UKd1J
7ziwXD0IG42CiiNgzG2SAq82ltJGyg==
=ZhBB
-----END PGP SIGNATURE-----

--smw0hoe4UY9/oecT--