Message ID | 20221124191400.287918-1-j.neuschaefer@gmx.net |
---|---|
Headers | show |
Series | Nuvoton WPCM450 FIU SPI flash controller | expand |
On Thu, Nov 24, 2022 at 08:13:59PM +0100, Jonathan Neuschäfer wrote: > The Flash Interface Unit (FIU) is the SPI flash controller in the > Nuvoton WPCM450 BMC SoC. It supports four chip selects, and direct > (memory-mapped) access to 16 MiB per chip. Larger flash chips can be > accessed by software-defined SPI transfers. Those software defined SPI transfers seem to be most of the way to supporting normal SPI controller operations, they just need wiring up. That would both support people hooking other SPI chips up to the board and might help support future flash stuff without needing custom code in the driver like you've got now. > +static int wpcm_fiu_do_uma(struct wpcm_fiu_spi *fiu, unsigned int cs, > + bool use_addr, bool write, int data_bytes) > +{ This appears to only support half duplex access but the driver doesn't flag itself as SPI_CONTROLLER_HALF_DUPLEX. > + cts |= FIU_UMA_CTS_D_SIZE(data_bytes); I'm guessing there's a limit on data_bytes that should be enforced. The driver should probably also flag a max transfer size, though that might cause issues if the limit is different between spi-mem and regular transfers. > +/* > + * RDID (Read Identification) needs special handling because Linux expects to > + * be able to read 6 ID bytes and FIU can only read up to 4 at once. > + * > + * We're lucky in this case, because executing the RDID instruction twice will > + * result in the same result. > + * > + * What we do is as follows (C: write command/opcode byte, D: read data byte, > + * A: write address byte): > + * > + * 1. C D D D > + * 2. C A A A D D D > + */ If the driver were implementing regular SPI operations and advertising a maximum transfer length this should just work without having to jump through hoops. The core can split transfers up into sections that fit within the controller limits transparently.
Hello, On Fri, Nov 25, 2022 at 01:04:54PM +0000, Mark Brown wrote: > On Thu, Nov 24, 2022 at 08:13:59PM +0100, Jonathan Neuschäfer wrote: > > > The Flash Interface Unit (FIU) is the SPI flash controller in the > > Nuvoton WPCM450 BMC SoC. It supports four chip selects, and direct > > (memory-mapped) access to 16 MiB per chip. Larger flash chips can be > > accessed by software-defined SPI transfers. > > Those software defined SPI transfers seem to be most of the way to > supporting normal SPI controller operations, they just need wiring up. > That would both support people hooking other SPI chips up to the board > and might help support future flash stuff without needing custom code in > the driver like you've got now. I'm not so sure. The hardware mechanism allowing "software defined" SPI transfers is strongly oriented towards SPI flash, and it already felt like a stretch to implement all the features that Linux expects of a SPI MEM controller. As to connecting non-memory chips: There is also a second, completely different SPI controller in this SoC, which is used on some boards (in factory configuration) to drive a little status LCD. I think it would be easiest to use that one for custom hardware extensions. > > > +static int wpcm_fiu_do_uma(struct wpcm_fiu_spi *fiu, unsigned int cs, > > + bool use_addr, bool write, int data_bytes) > > +{ > > This appears to only support half duplex access but the driver doesn't > flag itself as SPI_CONTROLLER_HALF_DUPLEX. Ok, I'll add it. > > > + cts |= FIU_UMA_CTS_D_SIZE(data_bytes); > > I'm guessing there's a limit on data_bytes that should be enforced. The > driver should probably also flag a max transfer size, though that might > cause issues if the limit is different between spi-mem and regular > transfers. For the existing spi-mem case, the transfer size is limited through wpcm_fiu_adjust_op_size. I *think* this is enough, but please correct me if I'm wrong. > > +/* > > + * RDID (Read Identification) needs special handling because Linux expects to > > + * be able to read 6 ID bytes and FIU can only read up to 4 at once. > > + * > > + * We're lucky in this case, because executing the RDID instruction twice will > > + * result in the same result. > > + * > > + * What we do is as follows (C: write command/opcode byte, D: read data byte, > > + * A: write address byte): > > + * > > + * 1. C D D D > > + * 2. C A A A D D D > > + */ > > If the driver were implementing regular SPI operations and advertising > a maximum transfer length this should just work without having to jump > through hoops. The core can split transfers up into sections that fit > within the controller limits transparently. As far as I'm aware, the controller is not capable of performing a pure read transfer, because the command byte (a byte that is written, in half-duplex) is always included at the start. I think this limitation would break your idea. IOW, the hoops aren't nice, but I think they're necessary. Thanks for your review, Jonathan
On Fri, Nov 25, 2022 at 05:33:31PM +0100, Jonathan Neuschäfer wrote: > As to connecting non-memory chips: There is also a second, completely > different SPI controller in this SoC, which is used on some boards (in > factory configuration) to drive a little status LCD. I think it would be > easiest to use that one for custom hardware extensions. That's never stopped hardware engineers. Perhaps it's simpler for pinmuxing, layout or other reasons in a given design. > > If the driver were implementing regular SPI operations and advertising > > a maximum transfer length this should just work without having to jump > > through hoops. The core can split transfers up into sections that fit > > within the controller limits transparently. > As far as I'm aware, the controller is not capable of performing a pure > read transfer, because the command byte (a byte that is written, in > half-duplex) is always included at the start. I think this limitation > would break your idea. > IOW, the hoops aren't nice, but I think they're necessary. Ah, I see. That is very limiting. I'm very surprised that the 6 byte thing works at all TBH.