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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 26 Jan 2023 18:33:33 +0100
From:   Jonas Gorski <jonas.gorski@...il.com>
To:     Kursad Oney <kursad.oney@...adcom.com>
Cc:     William Zhang <william.zhang@...adcom.com>,
        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

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.

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.


Regards
Jonas