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

William Zhang <william.zhang@xxxxxxxxxxxx> · Thu, 26 Jan 2023 19:10:22 -0800

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