linux-kernel - Re: [PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay

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:   Wed, 10 Mar 2021 08:21:14 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Alexandru Ardelean <aardelean@...iqon.com>
Cc:     linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
        greybus-dev@...ts.linaro.org, devel@...verdev.osuosl.org,
        linux-tegra@...r.kernel.org, bcm-kernel-feedback-list@...adcom.com,
        broonie@...nel.org, gregkh@...uxfoundation.org, elder@...nel.org,
        johan@...nel.org, vireshk@...nel.org, rmfrfs@...il.com,
        f.fainelli@...il.com, ldewangan@...dia.com,
        thierry.reding@...il.com, jonathanh@...dia.com, linux@...iqon.com
Subject: Re: [PATCH 01/10] spi: spi-axi-spi-engine: remove usage of
 delay_usecs

On 3/10/21 8:16 AM, Alexandru Ardelean wrote:
> On Mon, 8 Mar 2021 at 18:42, Lars-Peter Clausen <lars@...afoo.de> wrote:
>> On 3/8/21 3:54 PM, Alexandru Ardelean wrote:
>>> The 'delay_usecs' field was handled for backwards compatibility in case
>>> there were some users that still configured SPI delay transfers with
>>> this field.
>>>
>>> They should all be removed by now.
>>>
>>> Signed-off-by: Alexandru Ardelean <aardelean@...iqon.com>
>>> ---
>>>    drivers/spi/spi-axi-spi-engine.c | 12 ++++--------
>>>    1 file changed, 4 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
>>> index af86e6d6e16b..80c3e38f5c1b 100644
>>> --- a/drivers/spi/spi-axi-spi-engine.c
>>> +++ b/drivers/spi/spi-axi-spi-engine.c
>>> @@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
>>>        unsigned int t;
>>>        int delay;
>>>
>>> -     if (xfer->delay_usecs) {
>>> -             delay = xfer->delay_usecs;
>>> -     } else {
>>> -             delay = spi_delay_to_ns(&xfer->delay, xfer);
>>> -             if (delay < 0)
>>> -                     return;
>>> -             delay /= 1000;
>>> -     }
>>> +     delay = spi_delay_to_ns(&xfer->delay, xfer);
>>> +     if (delay < 0)
>>> +             return;
>> Bit of a nit, but this could be `delay <= 0` and then drop the check for
>> `delay == 0` below.
> hmm, that's a bit debatable, because the `delay == 0` check comes
> after `delay /= 1000` ;
> to do what you're suggesting, it would probably need a
> DIV_ROUND_UP(delay, 1000) to make sure that even sub-microsecond
> delays don't become zero;

Ah, true. Lets keep the code as it is.

On the other hand you could argue that we should round up to ensure the 
delay is at least as long as requested. But that is something that 
should be changed independently from this series.