From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Frysinger Date: Sat, 14 Jan 2012 20:04:14 -0500 Subject: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver In-Reply-To: <1326382034-31058-1-git-send-email-dirk.behme@de.bosch.com> References: <1326382034-31058-1-git-send-email-dirk.behme@de.bosch.com> Message-ID: <201201142004.15662.vapier@gentoo.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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: