mbox series

[00/16] Add support for Lattice MachXO2 programming via I2C

Message ID 20220825141343.1375690-1-j.zink@pengutronix.de (mailing list archive)
Headers show
Series Add support for Lattice MachXO2 programming via I2C | expand

Message

Johannes Zink Aug. 25, 2022, 2:13 p.m. UTC
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

Comments

Ivan Bornyakov Aug. 25, 2022, 3:25 p.m. UTC | #1
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.
Johannes Zink Aug. 26, 2022, 6:32 a.m. UTC | #2
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
Ivan Bornyakov Aug. 26, 2022, 8:15 a.m. UTC | #3
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    |
>
Sascha Hauer Aug. 26, 2022, 8:25 a.m. UTC | #4
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
Ivan Bornyakov Aug. 26, 2022, 9 a.m. UTC | #5
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 |
Ivan Bornyakov Aug. 26, 2022, 9:19 a.m. UTC | #6
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 |
Xu Yilun Aug. 26, 2022, 3:26 p.m. UTC | #7
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 |
>