Re: [PATCH v1 3/7] spi: qspi-tegra: Add support for Tegra210 QSPI controller

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

On Wed, Dec 02, 2020 at 11:17:18AM -0800, Sowjanya Komatineni wrote:
> On 12/2/20 9:27 AM, Mark Brown wrote:
> > On Tue, Dec 01, 2020 at 01:12:44PM -0800, Sowjanya Komatineni wrote:
[...]
> > > +static int tegra_qspi_setup(struct spi_device *spi)
> > > +{
> > > +	if (cdata && cdata->tx_clk_tap_delay)
> > > +		tx_tap = cdata->tx_clk_tap_delay;
> > > +	if (cdata && cdata->rx_clk_tap_delay)
> > > +		rx_tap = cdata->rx_clk_tap_delay;
> > > +	tqspi->def_command2_reg = QSPI_TX_TAP_DELAY(tx_tap) |
> > > +				  QSPI_RX_TAP_DELAY(rx_tap);
> > > +	tegra_qspi_writel(tqspi, tqspi->def_command2_reg, QSPI_COMMAND2);
> > The setup for one device shouldn't be able to affect the operation of
> > another, already running, device so either these need to be configured
> > as part of the controller probe or these configurations need to be
> > deferred until we're actually doing a transfer.
> We will only have 1 device on QSPI as we only support single chip select.

Even so we could make the driver operate as if there were multiple
devices. This has the advantage of setting a better example for someone
who might be reading this code as reference, and it might come in handy
if for whatever reason we ever end up with a QSPI controller that does
support multiple chip selects.

If that's overly complicated, maybe a compromise would be to document
very explicitly that this only works because Tegra QSPI supports a
single chip select?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]