Re: [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> > > > > ---
> > > > >  .../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    |
> 



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux