Re: [PATCH] spi: sh-msiof: Fix the limit of data length when calculating the length of words

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Hoan,

On Wed, Apr 3, 2019 at 11:17 AM Nguyen An Hoan <na-hoan@xxxxxxxxxxx> wrote:
> From: Hoan Nguyen An <na-hoan@xxxxxxxxxxx>
>
> We can use each word (data length) of 32bits (4 bytes),
> so that if the length is greater than 3bytes, we can
> align it with 4bytes of words.
>
> Signed-off-by: Hoan Nguyen An <na-hoan@xxxxxxxxxxx>

Thanks for your patch!

> ---
>  drivers/spi/spi-sh-msiof.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index e2eb466..1552c14 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -930,7 +930,7 @@ static int sh_msiof_transfer_one(struct spi_controller *ctlr,
>         if (!spi_controller_is_slave(p->ctlr))
>                 sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
>
> -       while (ctlr->dma_tx && len > 15) {
> +       while (ctlr->dma_tx && len > 3) {

This check is here to avoid using DMA (which incurs DMA setup overhead)
for short transfers.
Have you measured the performance impact of your change?

Perhaps the code should be changed to:

    #define DMA_MIN_LEN     16

    while (ctlr->dma_tx && len >= DMA_MIN_LEN) {

>                 /*
>                  *  DMA supports 32-bit words only, hence pack 8-bit and 16-bit
>                  *  words, with byte resp. word swapping.
> @@ -974,7 +974,7 @@ static int sh_msiof_transfer_one(struct spi_controller *ctlr,
>                         return 0;
>         }
>
> -       if (bits <= 8 && len > 15) {
> +       if (bits <= 8 && len > 3) {

Likewise.

>                 bits = 32;
>                 swab = true;
>         } else {

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds