| Message ID | 20220825141343.1375690-3-j.zink@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for Lattice MachXO2 programming via I2C | expand |
On 2022-08-25 at 16:13:29 +0200, Johannes Zink wrote: > This patch introduces additional memory areas of the machxo2-slave fpga > to be erased. > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de> > --- > .../bindings/fpga/lattice,machxo2-slave.yaml | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > index d05acd6b0fc6..78f0da8f772f 100644 > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > @@ -26,6 +26,19 @@ properties: > enum: > - lattice,machxo2-slave-spi > > + lattice,erase-sram: > + type: boolean > + description: SRAM is to be erased during flash erase operation > + > + lattice,erase-feature-row: > + type: boolean > + description: Feature row is to be erased during flash erase operation > + > + lattice,erase-userflash: > + type: boolean > + description: | > + UFM (user flash memory) is to be erased during flash erase operation In which conditions should we decide to erase each area? Thanks, Yilun > + > required: > - compatible > - reg > @@ -42,5 +55,7 @@ examples: > compatible = "lattice,machxo2-slave-spi"; > spi-max-frequency = <8000000>; > reg = <0>; > + lattice,erase-sram; > + lattice,erase-feature-row; > }; > }; > -- > 2.30.2 >
On Thu, Aug 25, 2022 at 04:13:29PM +0200, Johannes Zink wrote: > This patch introduces additional memory areas of the machxo2-slave fpga > to be erased. Why? > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de> > --- > .../bindings/fpga/lattice,machxo2-slave.yaml | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > index d05acd6b0fc6..78f0da8f772f 100644 > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > @@ -26,6 +26,19 @@ properties: > enum: > - lattice,machxo2-slave-spi > > + lattice,erase-sram: > + type: boolean > + description: SRAM is to be erased during flash erase operation > + > + lattice,erase-feature-row: > + type: boolean > + description: Feature row is to be erased during flash erase operation > + > + lattice,erase-userflash: > + type: boolean > + description: | > + UFM (user flash memory) is to be erased during flash erase operation These seem like policy. It this something that's really static to a particular board rather than something the user would configure each time. Rob
Hi Rob, On Tue, 2022-08-30 at 15:36 -0500, Rob Herring wrote: > On Thu, Aug 25, 2022 at 04:13:29PM +0200, Johannes Zink wrote: > > This patch introduces additional memory areas of the machxo2-slave > > fpga > > to be erased. > > Why? > Depending on the bitstream loaded to the FPGA, parts of the Flash Memory or SRAM can hold configuration data which is non-volatile over erase cycles. With this property, the board integrator, who knows about the fpga design, can decide whether these areas shall be erased on update or not. As an example, think of MAC addresses for a softcore network interface stored in UFM (user flash memory), the board integrator might want to decide to protect this memory area over reflashing the fpga. > > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de> > > --- > > .../bindings/fpga/lattice,machxo2-slave.yaml | 15 > > +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > > b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml > > index d05acd6b0fc6..78f0da8f772f 100644 > > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > slave.yaml > > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > slave.yaml > > @@ -26,6 +26,19 @@ properties: > > enum: > > - lattice,machxo2-slave-spi > > > > + lattice,erase-sram: > > + type: boolean > > + description: SRAM is to be erased during flash erase operation > > + > > + lattice,erase-feature-row: > > + type: boolean > > + description: Feature row is to be erased during flash erase > > operation > > + > > + lattice,erase-userflash: > > + type: boolean > > + description: | > > + UFM (user flash memory) is to be erased during flash erase > > operation > > These seem like policy. It this something that's really static to a > particular board rather than something the user would configure each > time. From the usecases I can think of, for a given board with a given FPGA design this is static. Best regards Johannes > > Rob > >
Hi Yilun, On Mon, 2022-08-29 at 23:09 +0800, Xu Yilun wrote: > On 2022-08-29 at 10:41:42 +0200, Johannes Zink wrote: > > On Mon, 2022-08-29 at 15:39 +0800, Xu Yilun wrote: > > > On 2022-08-25 at 16:13:29 +0200, Johannes Zink wrote: > > > > This patch introduces additional memory areas of the machxo2- > > > > slave > > > > fpga > > > > to be erased. > > > > > > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de> > > > > --- > > > > .../bindings/fpga/lattice,machxo2-slave.yaml | 15 > > > > +++++++++++++++ > > > > 1 file changed, 15 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > slave.yaml > > > > b/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > slave.yaml > > > > index d05acd6b0fc6..78f0da8f772f 100644 > > > > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > slave.yaml > > > > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > slave.yaml > > > > @@ -26,6 +26,19 @@ properties: > > > > enum: > > > > - lattice,machxo2-slave-spi > > > > > > > > + lattice,erase-sram: > > > > + type: boolean > > > > + description: SRAM is to be erased during flash erase > > > > operation > > > > + > > > > + lattice,erase-feature-row: > > > > + type: boolean > > > > + description: Feature row is to be erased during flash > > > > erase > > > > operation > > > > + > > > > + lattice,erase-userflash: > > > > + type: boolean > > > > + description: | > > > > + UFM (user flash memory) is to be erased during flash > > > > erase > > > > operation > > > > > > In which conditions should we decide to erase each area? > > > > > > Thanks, > > > Yilun > > > > Hi Yilun, > > > > the flash regions to be erased depend on the system design or > > usecase. > > For example, if non-volatile configuration is stored in the user > > flash > > memory, you might want to keep it from being erased in an in-field > > upgrade, but you might want to clear it at factory bringup. > > So these are all about user requirement, not the hardware > capabilities. > I think you should not put them here. If the user wants a different > erase option supported by HW, why the driver prevents it? > > Thanks, > Yilun I think it is rather a decision made by board-integrator, who will also write the DT, than a decision made by the user at runtime, because the integrator may decide to use the UFM (user flash memory) as a non- volatile storage, e.g. for mac addresses to a softcore ethernet mac implementation, or may decide to keep the security and readout- protection flags in the user row from being erased. On the other hand I do not have a very strong preference to have these properties set via the device tree (I also guess that you may have different usecases in mind), it simply appeared to me to fit quite well when I thought about how this property is used. If you prefer this property to be set by another interface, please suggest how to hand these information into the driver, since I am not too familiar with the fpga-mgr framework and its interfaces. I can then migrate to it for v2. Best regards Johannes > > > > > Best regards > > Johannes > > > > > > > + > > > > required: > > > > - compatible > > > > - reg > > > > @@ -42,5 +55,7 @@ examples: > > > > compatible = "lattice,machxo2-slave-spi"; > > > > spi-max-frequency = <8000000>; > > > > reg = <0>; > > > > + lattice,erase-sram; > > > > + lattice,erase-feature-row; > > > > }; > > > > }; > > > > -- > > > > 2.30.2 > > > > > > > > > > > -- > > 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 | > > >
On 2022-08-31 at 09:38:31 +0200, Johannes Zink wrote: > Hi Yilun, > > On Mon, 2022-08-29 at 23:09 +0800, Xu Yilun wrote: > > On 2022-08-29 at 10:41:42 +0200, Johannes Zink wrote: > > > On Mon, 2022-08-29 at 15:39 +0800, Xu Yilun wrote: > > > > On 2022-08-25 at 16:13:29 +0200, Johannes Zink wrote: > > > > > This patch introduces additional memory areas of the machxo2- > > > > > slave > > > > > fpga > > > > > to be erased. > > > > > > > > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de> > > > > > --- > > > > > .../bindings/fpga/lattice,machxo2-slave.yaml | 15 > > > > > +++++++++++++++ > > > > > 1 file changed, 15 insertions(+) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > > slave.yaml > > > > > b/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > > slave.yaml > > > > > index d05acd6b0fc6..78f0da8f772f 100644 > > > > > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > > slave.yaml > > > > > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2- > > > > > slave.yaml > > > > > @@ -26,6 +26,19 @@ properties: > > > > > enum: > > > > > - lattice,machxo2-slave-spi > > > > > > > > > > + lattice,erase-sram: > > > > > + type: boolean > > > > > + description: SRAM is to be erased during flash erase > > > > > operation > > > > > + > > > > > + lattice,erase-feature-row: > > > > > + type: boolean > > > > > + description: Feature row is to be erased during flash > > > > > erase > > > > > operation > > > > > + > > > > > + lattice,erase-userflash: > > > > > + type: boolean > > > > > + description: | > > > > > + UFM (user flash memory) is to be erased during flash > > > > > erase > > > > > operation > > > > > > > > In which conditions should we decide to erase each area? > > > > > > > > Thanks, > > > > Yilun > > > > > > Hi Yilun, > > > > > > the flash regions to be erased depend on the system design or > > > usecase. > > > For example, if non-volatile configuration is stored in the user > > > flash > > > memory, you might want to keep it from being erased in an in-field > > > upgrade, but you might want to clear it at factory bringup. > > > > So these are all about user requirement, not the hardware > > capabilities. > > I think you should not put them here. If the user wants a different > > erase option supported by HW, why the driver prevents it? > > > > Thanks, > > Yilun > > I think it is rather a decision made by board-integrator, who will also > write the DT, than a decision made by the user at runtime, because the > integrator may decide to use the UFM (user flash memory) as a non- > volatile storage, e.g. for mac addresses to a softcore ethernet mac > implementation, or may decide to keep the security and readout- > protection flags in the user row from being erased. > > On the other hand I do not have a very strong preference to have these > properties set via the device tree (I also guess that you may have > different usecases in mind), it simply appeared to me to fit quite well > when I thought about how this property is used. > > If you prefer this property to be set by another interface, please > suggest how to hand these information into the driver, since I am not > too familiar with the fpga-mgr framework and its interfaces. I can then > migrate to it for v2. I looked into the MACHOX2 driver & SPEC again, and found the major work is to program the flash. I think this is actually not within the scope of FPGA manager framework. The FPGA manager focus on the reprogramming (or so called configuration in MACHOX2 spec) of the FPGA region (FPGA active logic), and the removal & enumeration of the devices within the region, before and after reprogramming. The existing MACHOX2 driver actually combines the reprograming of the flash & the FPGA region, it hides many details about flash. Which is barely OK at that time. But when you are trying to extend more functionalities about flash, it is not proper here. So my idea is to separate the 2 steps: 1. managing the flash, this should be implemented in other subsystem, maybe MTD? 2. the configuration of the FPGA region retrieved from flash, this is within the scope of FPGA manager. We don't have code now, maybe it's the time to work on it. Thanks, Yilun > > Best regards > Johannes > > > > > > > > > Best regards > > > Johannes > > > > > > > > > + > > > > > required: > > > > > - compatible > > > > > - reg > > > > > @@ -42,5 +55,7 @@ examples: > > > > > compatible = "lattice,machxo2-slave-spi"; > > > > > spi-max-frequency = <8000000>; > > > > > reg = <0>; > > > > > + lattice,erase-sram; > > > > > + lattice,erase-feature-row; > > > > > }; > > > > > }; > > > > > -- > > > > > 2.30.2 > > > > > > > > > > > > > > > -- > > > 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 | > > > > > > > -- > 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 | >
diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml index d05acd6b0fc6..78f0da8f772f 100644 --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml @@ -26,6 +26,19 @@ properties: enum: - lattice,machxo2-slave-spi + lattice,erase-sram: + type: boolean + description: SRAM is to be erased during flash erase operation + + lattice,erase-feature-row: + type: boolean + description: Feature row is to be erased during flash erase operation + + lattice,erase-userflash: + type: boolean + description: | + UFM (user flash memory) is to be erased during flash erase operation + required: - compatible - reg @@ -42,5 +55,7 @@ examples: compatible = "lattice,machxo2-slave-spi"; spi-max-frequency = <8000000>; reg = <0>; + lattice,erase-sram; + lattice,erase-feature-row; }; };
This patch introduces additional memory areas of the machxo2-slave fpga to be erased. Signed-off-by: Johannes Zink <j.zink@pengutronix.de> --- .../bindings/fpga/lattice,machxo2-slave.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+)