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