Re: [PATCH v2 08/14] spi: bcm63xx-hsspi: Handle cs_change correctly

[William Zhang]

On 01/26/2023 09:33 AM, Jonas Gorski wrote:
> Hi Kursad,
>
> On Thu, 26 Jan 2023 at 17:22, Kursad Oney <kursad.oney@xxxxxxxxxxxx> wrote:
> > > While there, you might also want to check the cs_off value(s) as well.
> >
> > Can you explain this please?
>
> I'm talking about the transfer property cs_off:
>
> " @cs_off: performs the transfer with chipselect off."
>
> See how it is handled in the generic spi_transfer_one_message():
>
>          spi_set_cs(msg->spi, !xfer->cs_off, false);
>          ...
>          list_for_each_entry(xfer, &msg->transfers, transfer_list) {
>                  ...
>                  if (xfer->cs_change) {
>                          if (list_is_last(&xfer->transfer_list,
>                                           &msg->transfers)) {
>                                  keep_cs = true;
>                          } else {
>                                  if (!xfer->cs_off)
>                                          spi_set_cs(msg->spi, false, false);
>                                  _spi_transfer_cs_change_delay(msg, xfer);
>                                  if (!list_next_entry(xfer,
> transfer_list)->cs_off)
>                                          spi_set_cs(msg->spi, true, false);
>                          }
>                  } else if (!list_is_last(&xfer->transfer_list,
> &msg->transfers) &&
>                             xfer->cs_off != list_next_entry(xfer,
> transfer_list)->cs_off) {
>                          spi_set_cs(msg->spi, xfer->cs_off, false);
>                  }
>                  ...
>        }
>
> if we fix the cs_change handling, we might as well bring it up to state.
We can blindly port this logic over but this cs_off stuff (from the 
spi.h comment @cs_off: performs the transfer with chipselect off) sounds 
weird.  What kind of device do transfer when cs is off? I don't have any 
device like this to test.

> In theory I would suggest to switch to implementing the set_cs() /
> transfer_one() so you could let the core take care of all of that, but
> that wouldn't work with dynamically switching to prepend mode. Might
> be something for v1.1 though.
That is good idea and I can certainly try that.

> Regards
> Jonas

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature