[U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

[Mike Frysinger]

On Thursday 12 January 2012 10:27:13 Dirk Behme wrote:
> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> +		unsigned int max_hz, unsigned int mode)
> +{
> +	struct imx_spi_dev_t *imx_spi_slave = NULL;

setting to NULL is kind of pointless when you init it immediately below

> +	imx_spi_slave = (struct imx_spi_dev_t *)
> +			calloc(sizeof(struct imx_spi_dev_t), 1);

no need for that cast on the return of calloc

> +	spi_get_cfg(imx_spi_slave);
> +	spi_io_init(imx_spi_slave, 0);

i don't see these funcs defined anywhere in this patch.  since they aren't part 
of the common SPI API, you should namespace them accordingly (like with an SoC 
prefix or something).

> +	spi_reset(&(imx_spi_slave->slave));

the inner params are not needed.  also, programming of hardware does not 
happen in the spi_setup_slave() step.  that's what the spi_claim_bus() is for.

> +	return &(imx_spi_slave->slave);

drop the paren

> +void spi_free_slave(struct spi_slave *slave)
> +{
> +	struct imx_spi_dev_t *imx_spi_slave;
> +
> +	if (slave) {
> +		imx_spi_slave = to_imx_spi_slave(slave);
> +		free(imx_spi_slave);
> +	}
> +}

the NULL check on "slave" is not necessary.  we assume everywhere else that it 
is valid ahead of time.

> +int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
> +	const u8 *dout, u8 *din, unsigned long flags)

static

> +	if (spi_reg->ctrl_reg == 0) {
> +		printf("Error: spi(base=0x%x) has not been initialized yet\n",
> +				dev->base);

not necessary either ... we don't bother supporting broken callers in the bus 
drivers

> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void
> +		*dout, void *din, unsigned long flags)
> ...
> +	if (!slave)
> +		return -1;

not necessary either

> +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> +{
> +	return 1;
> +}

this can't be right ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120114/dd879533/attachment.pgp>