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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
