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

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

On Tue, Jan 09, 2018 at 06:40:48PM +0530, jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:

This is basically fine, a couple of style things plus the question I had
on the bindings:

> +++ b/drivers/spi/spi-synquacer.c
> @@ -0,0 +1,664 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Synquacer HSSPI controller driver

Mixing C and C++ style comments on adjacent lines looks messy, just make
the whole thing C++.

> +       speed = xfer->speed_hz ? : spi->max_speed_hz;
> +       bpw = xfer->bits_per_word ? : spi->bits_per_word;

I'm not a big fan of all the ternery operator use in the driver, it's
not super helpful for legibility.

> +	if (sspi->bpw == 8)
> +		words = xfer->len / 1;
> +	else if (sspi->bpw == 16)
> +		words = xfer->len / 2;
> +	else
> +		words = xfer->len / 4;

This should be a switch statement; a safety check for invalid values
wouldn't go amiss either.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]