Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230113165724.27199-1-Sergey.Semin@baikalelectronics.ru>
In-Reply-To: <20230113165724.27199-1-Sergey.Semin@baikalelectronics.ru>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Fri, 13 Jan 2023 19:33:16 +0200
Message-ID: <CAHp75Vc3+_SiVFzmJPM2dtPqUGErGY6Qc6_deS1yaDkjtT35rQ@mail.gmail.com>
Subject: Re: [PATCH] spi: dw: Fix wrong FIFO level setting for long xfers
To:     Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc:     Serge Semin <fancer.lancer@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
        Sergey Nazarov <Sergey.Nazarov@baikalelectronics.ru>,
        linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 13, 2023 at 6:57 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> Due to using the u16 type in the min_t() macros the SPI transfer length
> will be cast to word before participating in the conditional statement
> implied by the macro. Thus if the transfer length is greater than 64KB the
> Tx/Rx FIFO threshold level value will be determined by the leftover of the
> truncated after the type-case length. In the worst case it will cause
> having the "Tx FIFO Empty" or "Rx FIFO Full" interrupts triggered on each
> word sent/received to/from the bus. In its turn it will cause the
> dramatical performance drop.
>
> The problem can be easily fixed by using the min() macros instead of
> min_t() which doesn't imply any type casting thus preventing the possible
> data loss.

But this would be problematic if the types of the parameters are different.
Currently they are u32 vs. unsigned int. I would rather assume that
FIFO length is always less than or equal to 64K and just change the
type in min_t to follow what dws->tx_len is.

-- 
With Best Regards,
Andy Shevchenko