Re: [PATCH 6/5] spi: spi-geni-qcom: Simplify setup_fifo_xfer()

[Stephen Boyd]

Quoting Doug Anderson (2020-06-18 17:40:59)
> On Thu, Jun 18, 2020 at 4:40 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> > @@ -373,12 +366,14 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
> >         len &= TRANS_LEN_MSK;
> >
> >         mas->cur_xfer = xfer;
> > -       if (m_cmd & SPI_TX_ONLY) {
> > +       if (xfer->tx_buf) {
> > +               m_cmd |= SPI_TX_ONLY;
> >                 mas->tx_rem_bytes = xfer->len;
> >                 writel(len, se->base + SE_SPI_TX_TRANS_LEN);
> >         }
> >
> > -       if (m_cmd & SPI_RX_ONLY) {
> > +       if (xfer->rx_buf) {
> > +               m_cmd |= SPI_RX_ONLY;
> 
> If you're going to touch this, could you change "SPI_TX_ONLY" to
> "SPI_TX_ENABLED" and "SPI_RX_ONLY" to 'SPI_RX_ENABLED".  It felt
> really weird to me that if you had full duplex you were setting
> "RX_ONLY" and "TX_ONLY".

I agree, except in the register documentation it is called "TX only",
"RX only" and "Full-Duplex". So I'd rather leave it alone.

> 
> Other than that, your change is nice and cleans things up a bit, so
> even if you don't do the extra cleanup:
> 
> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> 
> 

Thanks.