linux-kernel - Re: [PATCH v2 08/14] spi: bcm63xx-hsspi: Handle cs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f208f86e-06dd-5d46-071d-501968c773bc@broadcom.com>
Date:   Thu, 26 Jan 2023 19:10:22 -0800
From:   William Zhang <william.zhang@...adcom.com>
To:     Jonas Gorski <jonas.gorski@...il.com>,
        Kursad Oney <kursad.oney@...adcom.com>
Cc:     Linux SPI List <linux-spi@...r.kernel.org>,
        Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>,
        tomer.yacoby@...adcom.com, dregan@...l.com, f.fainelli@...il.com,
        anand.gore@...adcom.com, dan.beygelman@...adcom.com,
        joel.peshkin@...adcom.com, Mark Brown <broonie@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 08/14] spi: bcm63xx-hsspi: Handle cs_change correctly



On 01/26/2023 09:33 AM, Jonas Gorski wrote:
> Hi Kursad,
> 
> On Thu, 26 Jan 2023 at 17:22, Kursad Oney <kursad.oney@...adcom.com> 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
> 

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4212 bytes)