On Thu, 8 May 2008, Haavard Skinnemoen wrote:
I thought, like (pseudocode)
static struct spi_host busses[SPI_BUSSES];
struct spi_slave *spi_init() { list_for_each_entry(slave, &busses[bus].slaves, list) { if (slave->device == device) return (struct spi_slave *)-EBUSY; } slave = malloc(); slave->bus = bus; slave->device = device; ret = busses[bus].init(slave); if (ret) { free(slave); return (struct spi_slave *)ret; } return slave; }
int spi_xfer() { list_for_each_entry(ix, &busses[bus].slaves, list) { if (ix == slave) break; } if (ix != slave) return -EINVAL;
if (slave->bus->busy) return -EBUSY;
return slave->bus->xfer(); }
I was thinking about something much simpler:
struct spi_slave *spi_init_slave(bus, cs, max_hz, mode) { slave = malloc(); slave->regs = get_spi_controller_base_address(bus); slave->mode_reg = get_suitable_settings_for(cs, max_hz, mode); return slave; }
int spi_xfer(slave, ...) { __raw_writel(slave->mode_reg, slave->regs + SPI_MR); if (flags & SPI_XFER_BEGIN) assert_chip_select();
do_the_actual_spi_transfer();
if (flags & SPI_XFER_END) deassert_chip_select();
return whatever; }
Of course, your solution will work with multiple, different SPI controllers while mine won't, but is that really necessary?
Your solution comes with more error checking as well, which might be a good thing, but since it comes with a cost of additional memory and flash footprint, I think it should be optional. Maybe we could provide some library functions to simplify the drivers that want this?
I see. Well, I don't have a strong preference. So, either we need more votes, or the one who implements it decides:-)
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de