> -----Original Message-----
> From: Mark Brown <broonie@xxxxxxxxxx>
> Sent: 04 February 2022 19:39
> To: Krishna Yarlagadda <kyarlagadda@xxxxxxxxxx>
> Cc: thierry.reding@xxxxxxxxx; Jonathan Hunter <jonathanh@xxxxxxxxxx>;
> linux-spi@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; Sowjanya Komatineni
> <skomatineni@xxxxxxxxxx>; Laxman Dewangan <ldewangan@xxxxxxxxxx>;
> robh+dt@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> Subject: Re: [PATCH 6/6] spi: tegra210-quad: combined sequence mode
>
> On Fri, Feb 04, 2022 at 03:59:36PM +0530, Krishna Yarlagadda wrote:
>
> > + /* Process individual transfer list */
> > + list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > + if (transfer_phase == CMD_TRANSFER) {
>
> > + } else if (transfer_phase == ADDR_TRANSFER) {
>
> > + } else {
>
> Looks like you're writing a switch statement here...
Yes. This can be switch statement.
>
> > + /* X1 SDR mode */
> > + cmd_config = tegra_qspi_cmd_config(false, 0,
> > + xfer->len);
> > + cmd_value = *((const u8 *)(xfer->tx_buf));
> > +
> > + len = xfer->len;
>
> > + /* X1 SDR mode */
> > + addr_config = tegra_qspi_addr_config(false, 0,
> > + xfer->len);
> > + address_value = *((const u32 *)(xfer->tx_buf));
>
> > + /* Program Command, Address value in register */
> > + tegra_qspi_writel(tqspi, cmd_value,
> QSPI_CMB_SEQ_CMD);
> > + tegra_qspi_writel(tqspi, address_value,
> > + QSPI_CMB_SEQ_ADDR);
> > + /* Program Command and Address config in register
> */
> > + tegra_qspi_writel(tqspi, cmd_config,
> > + QSPI_CMB_SEQ_CMD_CFG);
> > + tegra_qspi_writel(tqspi, addr_config,
> > + QSPI_CMB_SEQ_ADDR_CFG);
>
> It looks like the command and address have to be specific lengths? If that's the
> case then
Cmd and address are configurable to a limit. Will add min and max check.
>
> > + if (cdata->is_cmb_xfer && transfer_count == 3)
> > + ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
> > + else
> > + ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);
>
> This check needs to be more specific. But like I said in reply to the binding
> patch I don't see why we can't just pattern match on the data without requiring
> a property here, we'd need to check that the message is suitable no matter
> what.
There is no real-world use case we encountered so far preventing us stick to pattern.
But this was to avoid any corner case where there could be 3 different transfers sent in single msg.
