Received-SPF: pass (google.com: domain of linux-kernel+bounces-59595-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 9 Feb 2024 16:21:16 +0000
From: Conor Dooley <conor@kernel.org>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>, broonie@kernel.org,
	robh@kernel.org, andi.shyti@kernel.org, semen.protsenko@linaro.org,
	krzysztof.kozlowski@linaro.org, alim.akhtar@samsung.com,
	linux-spi@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
	andre.draszik@linaro.org, peter.griffin@linaro.org,
	kernel-team@android.com, willmcvicker@google.com,
	conor+dt@kernel.org, devicetree@vger.kernel.org, arnd@arndb.de
Subject: Re: [PATCH 01/12] spi: dt-bindings: introduce the ``fifo-depth``
 property
Message-ID: <20240209-chest-sleet-a119fc3d4243@spud>
References: <20240208135045.3728927-1-tudor.ambarus@linaro.org>
 <20240208135045.3728927-2-tudor.ambarus@linaro.org>
 <20240208-grating-legwarmer-0a04cfb04d61@spud>
 <c2b08463-cb13-4e9b-8797-8ebcf1047f66@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="AYm2FyfJMLjy9NNH"
Content-Disposition: inline
In-Reply-To: <c2b08463-cb13-4e9b-8797-8ebcf1047f66@linaro.org>


--AYm2FyfJMLjy9NNH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 09, 2024 at 01:56:56PM +0000, Tudor Ambarus wrote:
>=20
> + Geert
>=20
> On 2/8/24 18:24, Conor Dooley wrote:
> > On Thu, Feb 08, 2024 at 01:50:34PM +0000, Tudor Ambarus wrote:
> >> There are instances of the same IP that are configured by the integrat=
or
> >> with different FIFO depths. Introduce the fifo-depth property to allow
> >> such nodes to specify their FIFO depth.
> >>
> >> We haven't seen SPI IPs with different FIFO depths for RX and TX, thus
> >> introduce a single property.
> >=20
> > Some citation attached to this would be nice. "We haven't seen" offers
> > no detail as to what IPs that allow this sort of configuration of FIFO
> > size that you have actually checked.
> >=20
> > I went and checked our IP that we use in FPGA fabric, which has a
> > configurable fifo depth. It only has a single knob for both RX and TX
> > FIFOs. The Xilinx xps spi core also has configurable FIFOs, but again RX
> > and TX sizes are tied there. At least that's a sample size of three.
> >=20
> > One of our guys is working on support for the IP I just mentioned and
> > would be defining a vendor property for this, so
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >=20
>=20
> Thanks, Conor. I had in mind that SPI has a shift register and it's
> improbable to have different FIFO depths for RX and TX.

IDK, but I've learned to expect the unexpectable, especially when it
comes to the IPs intended for use in FPGAs.

> At least I don't
> see how it would work, I guess it will use the minimum depth between the
> two?

I'm not really sure how it would work other than that in the general
case, but some use case specific configuration could work, but I do
agree that it is

> I grepped by "fifo" in the spi bindings and I now see that renesas is
> using dedicated properties for RX and TX, but I think that there too the
> FIFOs have the same depths. Looking into drivers/spi/spi-sh-msiof.c I
> see that the of_device_id.data contains 64 bytes FIFOs for RX and TX,
> regardless of the compatible.
>=20
> Geert, any idea if the FIFO depths can differ for RX and TX in
> spi-sh-msiof.c?
>=20
> Anyway, even if there are such imbalanced architectures, I guess we can
> consider them when/if they appear? (add rx/tx-fifo-depth dt properties)

I think so.

> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> Override the default TX fifo size.  Unit is words.  Ignored if 0.
> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> renesas,rx-fifo-size:
> Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml:
> Override the default RX fifo size.  Unit is words.  Ignored if 0.

These renesas ones seemed interesting at first glance due to these
comments, but what's missed by grep the is "deprecated" marking on
these. They seem to have been replaced by soc-specific compatibles.

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

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

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZcZQ+wAKCRB4tDGHoIJi
0tCHAQC+UahOjFrFh4KmlxoZGj3mhl0GhMgYnpK4Y008NNbSKwD7BWwhfjMZ1Zk1
UZE3Fa1Tubov+aQxuH2m9T+rmex3fQg=
=PHwh
-----END PGP SIGNATURE-----

--AYm2FyfJMLjy9NNH--