enjoy-digital / litepcie

Small footprint and configurable PCIe core
Other
449 stars 110 forks source link

Expanding spiflash functionality in c++ driver with I2C bitbang and SPIMaster functions #129

Closed Johnsel closed 4 months ago

Johnsel commented 5 months ago

HI @enjoy-digital. As you know we are writing code making use of the I2C and SPIMaster interface over PCIe.

I am wondering, would it make sense to expand liblitepcie and include this functionality there? We already have spiflash functionality. It would be basically the LiteX bios code adjusted to write to PCIe CSRs.

enjoy-digital commented 4 months ago

Hi @Johnsel,

I have a preference to keep only MMAP/DMA features in the LitePCIe utilities to keep them simple and related to the functionality of the core and have more project specific features integrated separately. If the current driver is not flexible enough to allow this, we can expand it in this direction. At least that's the current approach I have, but maybe I'm wrong. If you want to ease reuse, we can also add link to specific projects with these features in the wiki or README to give them visibility.

Johnsel commented 4 months ago

Hi @enjoy-digital,

To give you some context for the discussion, we're currently writing a scopehal driver. Adding especially the bitbang code to the scopehal driver is undesirable, so our discussion was where to put it instead? It would be easy to add it to liblitepcie, and have it be generated on request/bus availability, similar to how the LiteX bios has I2C support implemented. If you prefer to keep it out of liblitepcie then that is fine too, we will implement some layer of abstraction then. I just wanted to discuss it with you seeing as I think the code is more belonging to the LiteX project than our application, and there is already litepcie_flash.c.

For helping others we can definitely link to it, sure. That was not the reason why I proposed it though :)

enjoy-digital commented 4 months ago

In fact, I'm also not sure litepcie_flash.c should be there. The best for this would be to have some common code in the repository also defining the gateware (so LiteX for the SPIFlash, SPIMaster and I2C), code that could be used from a firmware or from user-space. But the code also depends of the way it is instanciated and configured, so that's several parameters to take into account.

Johnsel commented 4 months ago

One disadvantage of removing that is that with litepcie_flash.c we can implement firmware update over PCIe. Right, now we have litex/soc/software and litepcie/software. Abstracting the shared code away cleanly might be a possibility. In the mean time we will write an abstraction and perhaps we can include it somewhere as a demo for others how to replicate the result.

enjoy-digital commented 4 months ago

@Johnsel: I don't really want to remove litepcie_flash.c, but as for the other peripherals, having the main functionnality in LiteX and just reusing it in liblitepcie would probably be a better approach.

Johnsel commented 4 months ago

Sounds good. I am not sure if I can easily include it from LiteX with the current implementation, but I may open an issue or PR there then to discuss further.

Op za 10 feb 2024 om 08:46 schreef enjoy-digital @.***>

@Johnsel https://github.com/Johnsel: I don't really want to remove litepcie_flash.c, but as for the other peripherals, having the main functionnality in LiteX and just reusing it in liblitepcie would probably be a better approach.

— Reply to this email directly, view it on GitHub https://github.com/enjoy-digital/litepcie/issues/129#issuecomment-1936919154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDD4ZJCF2QDDYY2W5IGVDYS4QWNAVCNFSM6AAAAABDBMAYQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZWHEYTSMJVGQ . You are receiving this because you were mentioned.Message ID: @.***>