Re: [PATCH] SPI: Fix distinct pointer types warning for ARCH=MIPS

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Nathan,

On Thu, Jan 7, 2021 at 6:31 PM Nathan Chancellor
<natechancellor@xxxxxxxxx> wrote:
> On Thu, Jan 07, 2021 at 07:57:04PM +0800, Yanteng Si wrote:
> > Fix a new warning report by build for make ARCH=MIPS allmodconfig:
> >
> > drivers/spi/spi-cadence-quadspi.c: In function 'cqspi_direct_read_execute':
> >  ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast
> >     18 |  (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> >        |                            ^~
> >  ./include/linux/minmax.h:32:4: note: in expansion of macro '__typecheck'
> >     32 |   (__typecheck(x, y) && __no_side_effects(x, y))
> >        |    ^~~~~~~~~~~
> >  ./include/linux/minmax.h:42:24: note: in expansion of macro '__safe_cmp'
> >     42 |  __builtin_choose_expr(__safe_cmp(x, y), \
> >        |                        ^~~~~~~~~~
> >  ./include/linux/minmax.h:58:19: note: in expansion of macro '__careful_cmp'
> >     58 | #define max(x, y) __careful_cmp(x, y, >)
> >        |                   ^~~~~~~~~~~~~
> >  drivers/spi/spi-cadence-quadspi.c:1153:24: note: in expansion of macro 'max'
> >   1153 |       msecs_to_jiffies(max(len, 500UL)))) {
> >        |                        ^~~
> >
> > "len" is unsigned,however,"500" is unsigned long.

len is size_t, which us either unsigned int or unsigned long.

> > Signed-off-by: Yanteng Si <siyanteng@xxxxxxxxxxx>
> > ---
> >  drivers/spi/spi-cadence-quadspi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> > index 06a65e9a8a60..576610ba1118 100644
> > --- a/drivers/spi/spi-cadence-quadspi.c
> > +++ b/drivers/spi/spi-cadence-quadspi.c
> > @@ -1150,7 +1150,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
> >
> >       dma_async_issue_pending(cqspi->rx_chan);
> >       if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
> > -                                      msecs_to_jiffies(max(len, 500UL)))) {
> > +                                      msecs_to_jiffies(max(len, 500U)))) {
> >               dmaengine_terminate_sync(cqspi->rx_chan);
> >               dev_err(dev, "DMA wait_for_completion_timeout\n");
> >               ret = -ETIMEDOUT;
> > --
> > 2.27.0
> >
>
> Isn't this just going to cause warnings on 64-bit platforms now because
> size_t is defined as unsigned long and it will now be compared against
> unsigned int? This fix should work for everyone, not sure how pretty it

Yes it is!

> is though.

> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1150,7 +1150,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>
>         dma_async_issue_pending(cqspi->rx_chan);
>         if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
> -                                        msecs_to_jiffies(max(len, 500UL)))) {
> +                                        msecs_to_jiffies(max_t(size_t, len, 500)))) {
>                 dmaengine_terminate_sync(cqspi->rx_chan);
>                 dev_err(dev, "DMA wait_for_completion_timeout\n");
>                 ret = -ETIMEDOUT;

Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

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