| Message ID | 20220825141343.1375690-1-j.zink@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for Lattice MachXO2 programming via I2C | expand |
Hi, Johannes! I just came across your patches. Surprisingly, our work interferes. I recently posted patch-series for configuring ECP5 and was asked to make generalized sysCONFIG driver with support for both ECP5 and MachXO2, which I did. Sadly I don't have hardware with MachXO2, but you clearly do :) Please, take a look at https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ and please help test MachXO2 variant. When we pull this off, you may add I2C interface on top.
On Thu, 2022-08-25 at 18:25 +0300, Ivan Bornyakov wrote: > Hi, Johannes! Hi Ivan, > > I just came across your patches. Surprisingly, our work interferes. > > I recently posted patch-series for configuring ECP5 and was asked to > make > generalized sysCONFIG driver with support for both ECP5 and MachXO2, > which > I did. That looks very interesting indeed. > Sadly I don't have hardware with MachXO2, but you clearly do :) > > Please, take a look at > > > https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ > > and please help test MachXO2 variant. When we pull this off, you may > add I2C > interface on top. > > > my hardware has only I2C connected to the MachXO2 (hence the patch series...), so I cannot test your patches directly. Since adding I2C requires some quirks with respect to the programming commands (some are differ to the SPI ones, ...) it will take me some time to add my patches on top of yours in order to test, but after having had a short glance at your patch series, I think it should be feasible. Though, I think you should allow the program-gpios, init-gpios and done-gpios for machxo2 and have them as optional, at least for machxo2. Best regards Johannes
On Fri, Aug 26, 2022 at 08:32:49AM +0200, Johannes Zink wrote: > On Thu, 2022-08-25 at 18:25 +0300, Ivan Bornyakov wrote: > > Hi, Johannes! > > Hi Ivan, > > > > I just came across your patches. Surprisingly, our work interferes. > > > > I recently posted patch-series for configuring ECP5 and was asked to > > make > > generalized sysCONFIG driver with support for both ECP5 and MachXO2, > > which > > I did. > > That looks very interesting indeed. > > > Sadly I don't have hardware with MachXO2, but you clearly do :) > > > > Please, take a look at > > > > > > https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ > > > > and please help test MachXO2 variant. When we pull this off, you may > > add I2C > > interface on top. > > > > > > > > my hardware has only I2C connected to the MachXO2 (hence the patch > series...), so I cannot test your patches directly. That's unfortunate, anyway please join the review so your changes would be easier to apply on top. > > Since adding I2C requires some quirks with respect to the programming > commands (some are differ to the SPI ones, ...) it will take me some > time to add my patches on top of yours in order to test, but after > having had a short glance at your patch series, I think it should be > feasible. > > Though, I think you should allow the program-gpios, init-gpios and > done-gpios for machxo2 and have them as optional, at least for machxo2. > > Best regards > Johannes > > -- > Pengutronix e.K. | Johannes Zink | > Steuerwalder Str. 21 | https://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 | >
Hi Ivan, On Thu, Aug 25, 2022 at 06:25:14PM +0300, Ivan Bornyakov wrote: > Hi, Johannes! > > I just came across your patches. Surprisingly, our work interferes. > > I recently posted patch-series for configuring ECP5 and was asked to make > generalized sysCONFIG driver with support for both ECP5 and MachXO2, which > I did. Sadly I don't have hardware with MachXO2, but you clearly do :) > > Please, take a look at > > https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ > > and please help test MachXO2 variant. When we pull this off, you may add I2C > interface on top. You are adding a new driver for something we already have a driver for in the tree. The final goal should be that we only have a single driver for sysconfig based FPGAs in the tree. Johannes' series is a step in that direction: He cleans up the existing driver and starts abstracting out common sysconfig functions so that they can be used by both the I2C and SPI interface. He just told me that the abstraction is likely not enough to integrate ECP5 support right away, one reason being that the machxo2 has a flash whereas the ECP5 does not. Unless you can explain why the existing driver is broken beyond repair I think we should rather incrementally improve the existing driver instead of adding a new one with a conflicting compatible. So despite you were in the room earlier I think you should rather base your work on Johannes' series. Just my 2 cents, maybe one of the maintainers has a few words on it. Sascha
On Fri, Aug 26, 2022 at 10:25:39AM +0200, Sascha Hauer wrote: > Hi Ivan, > > On Thu, Aug 25, 2022 at 06:25:14PM +0300, Ivan Bornyakov wrote: > > Hi, Johannes! > > > > I just came across your patches. Surprisingly, our work interferes. > > > > I recently posted patch-series for configuring ECP5 and was asked to make > > generalized sysCONFIG driver with support for both ECP5 and MachXO2, which > > I did. Sadly I don't have hardware with MachXO2, but you clearly do :) > > > > Please, take a look at > > > > https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ > > > > and please help test MachXO2 variant. When we pull this off, you may add I2C > > interface on top. > > You are adding a new driver for something we already have a driver for > in the tree. My intent was to add new driver and drop old one once the new driver is proven to be working. > The final goal should be that we only have a single driver > for sysconfig based FPGAs in the tree. It is. > Johannes' series is a step in > that direction: He cleans up the existing driver and starts abstracting > out common sysconfig functions so that they can be used by both the I2C > and SPI interface. He just told me that the abstraction is likely not > enough to integrate ECP5 support right away, one reason being that the > machxo2 has a flash whereas the ECP5 does not. > > Unless you can explain why the existing driver is broken beyond repair > I think we should rather incrementally improve the existing driver > instead of adding a new one with a conflicting compatible. Yeah, conflicting compatible was my oversight. > So despite you were in the room earlier I think you should rather base > your work on Johannes' series. We on par here. You guys didn't join ongoing work, I didn't rework existing driver. > > Just my 2 cents, maybe one of the maintainers has a few words on it. > > Sascha > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Aug 26, 2022 at 12:00:44PM +0300, Ivan Bornyakov wrote: > On Fri, Aug 26, 2022 at 10:25:39AM +0200, Sascha Hauer wrote: > > Hi Ivan, > > > > On Thu, Aug 25, 2022 at 06:25:14PM +0300, Ivan Bornyakov wrote: > > > Hi, Johannes! > > > > > > I just came across your patches. Surprisingly, our work interferes. > > > > > > I recently posted patch-series for configuring ECP5 and was asked to make > > > generalized sysCONFIG driver with support for both ECP5 and MachXO2, which > > > I did. Sadly I don't have hardware with MachXO2, but you clearly do :) > > > > > > Please, take a look at > > > > > > https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ > > > > > > and please help test MachXO2 variant. When we pull this off, you may add I2C > > > interface on top. > > > > You are adding a new driver for something we already have a driver for > > in the tree. > > My intent was to add new driver and drop old one once the new driver is > proven to be working. > > > The final goal should be that we only have a single driver > > for sysconfig based FPGAs in the tree. > > It is. > > > Johannes' series is a step in > > that direction: He cleans up the existing driver and starts abstracting > > out common sysconfig functions so that they can be used by both the I2C > > and SPI interface. He just told me that the abstraction is likely not > > enough to integrate ECP5 support right away, one reason being that the > > machxo2 has a flash whereas the ECP5 does not. > > > > Unless you can explain why the existing driver is broken beyond repair > > I think we should rather incrementally improve the existing driver > > instead of adding a new one with a conflicting compatible. > > Yeah, conflicting compatible was my oversight. > Wait! They are not conflicting :) The new one is "lattice,machxo2-fpga-mgr", the old one is "lattice,machxo2-slave-spi" > > So despite you were in the room earlier I think you should rather base > > your work on Johannes' series. > > We on par here. You guys didn't join ongoing work, I didn't rework > existing driver. > > > > > Just my 2 cents, maybe one of the maintainers has a few words on it. > > > > Sascha > > > > > > -- > > Pengutronix e.K. | | > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 2022-08-26 at 12:19:12 +0300, Ivan Bornyakov wrote: > On Fri, Aug 26, 2022 at 12:00:44PM +0300, Ivan Bornyakov wrote: > > On Fri, Aug 26, 2022 at 10:25:39AM +0200, Sascha Hauer wrote: > > > Hi Ivan, > > > > > > On Thu, Aug 25, 2022 at 06:25:14PM +0300, Ivan Bornyakov wrote: > > > > Hi, Johannes! > > > > > > > > I just came across your patches. Surprisingly, our work interferes. > > > > > > > > I recently posted patch-series for configuring ECP5 and was asked to make > > > > generalized sysCONFIG driver with support for both ECP5 and MachXO2, which > > > > I did. Sadly I don't have hardware with MachXO2, but you clearly do :) > > > > > > > > Please, take a look at > > > > > > > > https://lore.kernel.org/linux-fpga/20220825112433.14583-1-i.bornyakov@metrotek.ru/ > > > > > > > > and please help test MachXO2 variant. When we pull this off, you may add I2C > > > > interface on top. > > > > > > You are adding a new driver for something we already have a driver for > > > in the tree. > > > > My intent was to add new driver and drop old one once the new driver is > > proven to be working. > > > > > The final goal should be that we only have a single driver > > > for sysconfig based FPGAs in the tree. > > > > It is. > > > > > Johannes' series is a step in > > > that direction: He cleans up the existing driver and starts abstracting > > > out common sysconfig functions so that they can be used by both the I2C > > > and SPI interface. He just told me that the abstraction is likely not > > > enough to integrate ECP5 support right away, one reason being that the > > > machxo2 has a flash whereas the ECP5 does not. > > > > > > Unless you can explain why the existing driver is broken beyond repair > > > I think we should rather incrementally improve the existing driver > > > instead of adding a new one with a conflicting compatible. > > > > Yeah, conflicting compatible was my oversight. > > > > Wait! They are not conflicting :) The new one is "lattice,machxo2-fpga-mgr", > the old one is "lattice,machxo2-slave-spi" > > > > So despite you were in the room earlier I think you should rather base > > > your work on Johannes' series. > > > > We on par here. You guys didn't join ongoing work, I didn't rework > > existing driver. I didn't look into the code yet, so maybe some misunderstanding. I'd rather we pay more attention on the code design for the FULL feature of this sysCONFIG interface, rather than worrying about whose patches should go first. So just review patches for each other and collabrate for a well design, then your features can be accepted faster. Thanks, Yilun > > > > > > > > Just my 2 cents, maybe one of the maintainers has a few words on it. > > > > > > Sascha > > > > > > > > > -- > > > Pengutronix e.K. | | > > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
Lattice MachXO2 FPGAs have internal configuration flash which can be reprogrammed over different interfaces. The former driver implementation supported programming via SPI, this patch series adds programming via I2C. The first 4 patches convert the MachXO2 Slave binding from textual format to YAML and add additional features like different flash areas to be erased upon a programming cycle, a GPIO which can be used to explicitely initialize the programming sequence, and finally I2C as additional programming interface. The following 10 patches clean up and refactor the previous machxo2-spi driver code, extract functionalities common to both spi and i2c programming interfaces as a preparation, add additional flash areas to be erased and signalling for start of the programming sequence via gpio. Since the original driver did not yield enough time to erase machxo2 variants with large flash memory, a variation of erase timeout handling is added with another patch, introducing a more datasheet conformant way of dealing with large flash sizes due to larger LUT counts. The final patch adds the I2C bus as an additional interface for programming. Johannes Zink (15): dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML dt-bindings: fpga: machxo2-slave: add erasure properties dt-bindings: fpga: machxo2-slave: add pin for program sequence init dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible fpga: machxo2-spi: remove #ifdef DEBUG fpga: machxo2-spi: factor out status check for readability fpga: machxo2-spi: fix big-endianness incompatibility fpga: machxo2-spi: simplify with spi_sync_transfer() fpga: machxo2-spi: simplify spi write commands fpga: machxo2-spi: prepare extraction of common code fpga: machxo2: move non-spi-related functionality to common code fpga: machxo2: improve status register dump fpga: machxo2: add optional additional flash areas to be erased fpga: machxo2: add program initialization signalling via gpio fpga: machxo2: extend erase timeout for machxo2 FPGA Peter Jensen (1): fpga: machxo2: add configuration over i2c .../bindings/fpga/lattice,machxo2-slave.yaml | 80 ++++ .../bindings/fpga/lattice-machxo2-spi.txt | 29 -- drivers/fpga/Kconfig | 14 + drivers/fpga/Makefile | 2 + drivers/fpga/machxo2-common.c | 392 ++++++++++++++++++ drivers/fpga/machxo2-common.h | 43 ++ drivers/fpga/machxo2-i2c.c | 137 ++++++ drivers/fpga/machxo2-spi.c | 366 ++-------------- 8 files changed, 712 insertions(+), 351 deletions(-) create mode 100644 Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml delete mode 100644 Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt create mode 100644 drivers/fpga/machxo2-common.c create mode 100644 drivers/fpga/machxo2-common.h create mode 100644 drivers/fpga/machxo2-i2c.c