Re: [PATCHv4 2/3] spi: Add spi driver for Socionext Synquacer platform

From: Jassi Brar <jassisinghbrar@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-spi <linux-spi@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>, Trent Piepho <tpiepho@impinj.com>,
	Mark Brown <broonie@kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Masami Hiramatsu <masami.hiramatsu@linaro.org>,
	Jassi Brar <jaswinder.singh@linaro.org>
Subject: Re: [PATCHv4 2/3] spi: Add spi driver for Socionext Synquacer platform
Date: Thu, 1 Mar 2018 00:05:26 +0530	[thread overview]
Message-ID: <CABb+yY2LRqeA-_8g8Q+kwWXAoTYLKXenGLzu=ism_S--9WTJww@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdWmaovpjuyzjYRRLBKk6g0WDeA8YHDCWwr94shpom0YNA@mail.gmail.com>

On Wed, Feb 28, 2018 at 4:47 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Jassi,
>
> On Tue, Feb 27, 2018 at 1:58 PM,  <jassisinghbrar@gmail.com> wrote:
>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> This patch adds support for controller found on synquacer platforms.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> Thanks for your patch!
>
>> --- /dev/null
>> +++ b/drivers/spi/spi-synquacer.c
>
>> +static void read_fifo(struct synquacer_spi *sspi)
>> +{
>> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
>> +       int i;
>
> unsigned int, as len is unsigned.
>
>> +static void write_fifo(struct synquacer_spi *sspi)
>> +{
>> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
>> +       int i;
>
> unsigned int, as len is unsigned.
>
>> +static int synquacer_spi_config(struct spi_master *master,
>> +                               struct spi_device *spi,
>> +                               struct spi_transfer *xfer)
>> +{
>> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
>> +       unsigned int speed, mode, bpw, cs, bus_width;
>> +       unsigned long rate;
>
> unsigned int, as max_speed_hz is u32, else you'll do a 64/32-bit division later.
>
>> +static int synquacer_spi_transfer_one(struct spi_master *master,
>> +                                     struct spi_device *spi,
>> +                                     struct spi_transfer *xfer)
>> +{
>> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
>> +       int ret, words, busy = 0;
>
> unsigned int words, as xfr->len is unsigned.
>
>> +       unsigned long bpw;
>
> unsigned int is plenty (bits_per_word is even u8).
>
Will fix all.

Thank you.