On Thursday 07 June 2007, Andrei Konovalov wrote: > >> +/* Simple macros to get the code more readable */ > >> +#define xspi_in16(addr) in_be16((u16 __iomem *)(addr)) > >> +#define xspi_in32(addr) in_be32((u32 __iomem *)(addr)) > >> +#define xspi_out16(addr, value) out_be16((u16 __iomem *)(addr), (value)) > >> +#define xspi_out32(addr, value) out_be32((u32 __iomem *)(addr), (value)) > > > > I'm rather used to seeing I/O addressses passed around as "void __iomem *" > > so those sorts of cast are not needed... :) > > I've decided to make the base_address (u8 __iomem *) to make it easier to > add the register offsets to the base. Like that:
Adding register offsets works with "void __iomem *" too ... > > You should not need an abort primitive. ... > > I am paranoid enough not to rely 100% on the spi_device's and the spi_bitbang > doing all that :) > Agreed, xspi_abort_transfer is bad name here. But I still would like to > leave these two writes: > > + /* Deselect the slave on the SPI bus */ > + xspi_out32(regs_base + XSPI_SSR_OFFSET, 0xffff); > + /* Disable the transmitter */ > + xspi_out16(regs_base + XSPI_CR_OFFSET, > + XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSELECT); > > in xilinx_spi_remove(). Just in case :) What happens when you add a BUG_ON to catch the device still being active? Really, you need to rely on the rest of the system working correctly. > > I take it you can't support SPI_CS_HIGH?? > > There is no clear indication in the SPI controller docs that SPI_CS_HIGH > would work. I suspect it would as we don't use the ability of the controller > to generate the chip selects automatically (the auto mode doesn't support > SPI_CS_HIGH). > But it is more safe to advertise SPI_CS_HIGH is not supported. If you don't explicitly support it, it's unlikely to work. :) - Dave _______________________________________________ Linuxppc-embedded mailing list [email protected] https://ozlabs.org/mailman/listinfo/linuxppc-embedded
