diff mbox series

[03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init

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

Commit Message

Johannes Zink Aug. 25, 2022, 2:13 p.m. UTC
This commit adds a pin which initiates the FPGA programming sequence
once pulsed low.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 .../devicetree/bindings/fpga/lattice,machxo2-slave.yaml    | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Rob Herring Aug. 25, 2022, 6:51 p.m. UTC | #1
On Thu, 25 Aug 2022 16:13:30 +0200, Johannes Zink wrote:
> This commit adds a pin which initiates the FPGA programming sequence
> once pulsed low.
> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
>  .../devicetree/bindings/fpga/lattice,machxo2-slave.yaml    | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.example.dts:28.51-52 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1420: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Johannes Zink Aug. 26, 2022, 7:56 a.m. UTC | #2
On Thu, 2022-08-25 at 13:51 -0500, Rob Herring wrote:
> On Thu, 25 Aug 2022 16:13:30 +0200, Johannes Zink wrote:
> > This commit adds a pin which initiates the FPGA programming
> > sequence
> > once pulsed low.
> > 
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
> >  .../devicetree/bindings/fpga/lattice,machxo2-slave.yaml    | 7
> > +++++++
> >  1 file changed, 7 insertions(+)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/fpga/lattice,machxo2-
> slave.example.dts:28.51-52 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:384:
> Documentation/devicetree/bindings/fpga/lattice,machxo2-
> slave.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1420: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/
> 
> This check can fail if there are any dependencies. The base for a
> patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up
> to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 
> 
Hi, 

I was able to reproduce the error and will fix it in v2. 

Thanks and best regards
Johannes
Xu Yilun Aug. 29, 2022, 7:45 a.m. UTC | #3
On 2022-08-25 at 16:13:30 +0200, Johannes Zink wrote:
> This commit adds a pin which initiates the FPGA programming sequence
> once pulsed low.

Why we don't have to use this pin before?

Thanks,
Yilun

> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
>  .../devicetree/bindings/fpga/lattice,machxo2-slave.yaml    | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> index 78f0da8f772f..03dc134ec7b8 100644
> --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> @@ -26,6 +26,12 @@ properties:
>      enum:
>        - lattice,machxo2-slave-spi
>  
> +  program-gpios:
> +    maxItems: 1
> +    description: |
> +      GPIO Output tied to the FPGA's n_program pin to initiate a
> +      programming sequence. This pin is active low.
> +
>    lattice,erase-sram:
>      type: boolean
>      description: SRAM is to be erased during flash erase operation
> @@ -57,5 +63,6 @@ examples:
>              reg = <0>;
>              lattice,erase-sram;
>              lattice,erase-feature-row;
> +            lattice,program-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>
>          };
>      };
> -- 
> 2.30.2
>
Johannes Zink Aug. 31, 2022, 7:51 a.m. UTC | #4
Hi Yilun, 
On Mon, 2022-08-29 at 23:21 +0800, Xu Yilun wrote:
> On 2022-08-29 at 11:01:16 +0200, Johannes Zink wrote:
> > On Mon, 2022-08-29 at 15:45 +0800, Xu Yilun wrote:
> > > On 2022-08-25 at 16:13:30 +0200, Johannes Zink wrote:
> > > > This commit adds a pin which initiates the FPGA programming
> > > > sequence
> > > > once pulsed low.
> > > 
> > > Why we don't have to use this pin before?
> > > 
> > > Thanks,
> > > Yilun
> > > 
> > 
> > According to the MachXO2 Programming and Configuration User Guide
> > (FPGA-TN-02155-4.4) one of the 3 following methods can be used to
> > enter
> > the programming mode: 
> > 
> > - asserting a low pulse on the program gpio
> > - cycling power to the MachXO2
> > - sending the refresh command using a configuration port
> > 
> > In most cases, the refresh command being sent on initialization of
> > the
> > driver (as in the orignal driver) will do the job, but since you
> > can
> > deactivate the configuration ports, asserting the program gpio is
> > the
> > safe way to enter programming mode. Since the original driver did
> > not
> > support setting it, I added it as optional to the binding in order
> > not
> > to introduce any breakage. 
> 
> So do we need to skip the 3rd method if we already have the 2nd?
> 
> Thanks,
> Yilun 

the datasheet suggests that the methods are not mutually exclusive, so
doing both will not hurt. Since I want to keep backwards compatibility
with the existing driver (which only supports the 3rd option: sending
the refresh command), i'd rather not drop it in order not to introduce
any breakage. 

However, in my application I cannot power-cycle the FPGA (method 2) and
the configuration port is disabled after in-factory initial programming
via a security bit in the Feature Row Flash Area, which only leaves me
with Method 1 (asserting a low pulse on the program_n pin of the FPGA)
to enter programming mode for flashing another bitstream to the FPGA. 

This patch adds support for exactly this pin, while for other system
setups both of the other methods will work just as they did before.

Best regards
Johannes

> 
> > 
> > Best regards
> > Johannes
> > 
> > > > 
> > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > > ---
> > > >  .../devicetree/bindings/fpga/lattice,machxo2-slave.yaml    | 7
> > > > +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > slave.yaml
> > > > b/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > slave.yaml
> > > > index 78f0da8f772f..03dc134ec7b8 100644
> > > > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > slave.yaml
> > > > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > slave.yaml
> > > > @@ -26,6 +26,12 @@ properties:
> > > >      enum:
> > > >        - lattice,machxo2-slave-spi
> > > >  
> > > > +  program-gpios:
> > > > +    maxItems: 1
> > > > +    description: |
> > > > +      GPIO Output tied to the FPGA's n_program pin to initiate
> > > > a
> > > > +      programming sequence. This pin is active low.
> > > > +
> > > >    lattice,erase-sram:
> > > >      type: boolean
> > > >      description: SRAM is to be erased during flash erase
> > > > operation
> > > > @@ -57,5 +63,6 @@ examples:
> > > >              reg = <0>;
> > > >              lattice,erase-sram;
> > > >              lattice,erase-feature-row;
> > > > +            lattice,program-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>
> > > >          };
> > > >      };
> > > > -- 
> > > > 2.30.2
> > > > 
> > > 
> >
Johannes Zink Aug. 31, 2022, 8:08 a.m. UTC | #5
On Wed, 2022-08-31 at 09:51 +0200, Johannes Zink wrote:
> Hi Yilun, 
> On Mon, 2022-08-29 at 23:21 +0800, Xu Yilun wrote:
> > On 2022-08-29 at 11:01:16 +0200, Johannes Zink wrote:
> > > On Mon, 2022-08-29 at 15:45 +0800, Xu Yilun wrote:
> > > > On 2022-08-25 at 16:13:30 +0200, Johannes Zink wrote:
> > > > > This commit adds a pin which initiates the FPGA programming
> > > > > sequence
> > > > > once pulsed low.
> > > > 
> > > > Why we don't have to use this pin before?
> > > > 
> > > > Thanks,
> > > > Yilun
> > > > 
> > > 
> > > According to the MachXO2 Programming and Configuration User Guide
> > > (FPGA-TN-02155-4.4) one of the 3 following methods can be used to
> > > enter
> > > the programming mode: 
> > > 
> > > - asserting a low pulse on the program gpio
> > > - cycling power to the MachXO2
> > > - sending the refresh command using a configuration port
> > > 
> > > In most cases, the refresh command being sent on initialization
> > > of
> > > the
> > > driver (as in the orignal driver) will do the job, but since you
> > > can
> > > deactivate the configuration ports, asserting the program gpio is
> > > the
> > > safe way to enter programming mode. Since the original driver did
> > > not
> > > support setting it, I added it as optional to the binding in
> > > order
> > > not
> > > to introduce any breakage. 
> > 
> > So do we need to skip the 3rd method if we already have the 2nd?
> > 
> > Thanks,
> > Yilun 
> 
> the datasheet suggests that the methods are not mutually exclusive,
> so
> doing both will not hurt. Since I want to keep backwards
> compatibility
> with the existing driver (which only supports the 3rd option: sending
> the refresh command), i'd rather not drop it in order not to
> introduce
> any breakage. 
> 

On a quick side note: Sending the refresh command is required in any
case, if using the "Slave SPI" mode (i.e. flashing the bitstream via
SPI) or the I2C mode. So according to the MachXO2 Programming and
Configuration Usage Guide, this is strictly non-optional.

Best regards
Johannes

> However, in my application I cannot power-cycle the FPGA (method 2)
> and
> the configuration port is disabled after in-factory initial
> programming
> via a security bit in the Feature Row Flash Area, which only leaves
> me
> with Method 1 (asserting a low pulse on the program_n pin of the
> FPGA)
> to enter programming mode for flashing another bitstream to the
> FPGA. 
> 
> This patch adds support for exactly this pin, while for other system
> setups both of the other methods will work just as they did before.
> 
> Best regards
> Johannes
> 
> > 
> > > 
> > > Best regards
> > > Johannes
> > > 
> > > > > 
> > > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > > > ---
> > > > >  .../devicetree/bindings/fpga/lattice,machxo2-slave.yaml    |
> > > > > 7
> > > > > +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > > slave.yaml
> > > > > b/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > > slave.yaml
> > > > > index 78f0da8f772f..03dc134ec7b8 100644
> > > > > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > > slave.yaml
> > > > > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > > > > slave.yaml
> > > > > @@ -26,6 +26,12 @@ properties:
> > > > >      enum:
> > > > >        - lattice,machxo2-slave-spi
> > > > >  
> > > > > +  program-gpios:
> > > > > +    maxItems: 1
> > > > > +    description: |
> > > > > +      GPIO Output tied to the FPGA's n_program pin to
> > > > > initiate
> > > > > a
> > > > > +      programming sequence. This pin is active low.
> > > > > +
> > > > >    lattice,erase-sram:
> > > > >      type: boolean
> > > > >      description: SRAM is to be erased during flash erase
> > > > > operation
> > > > > @@ -57,5 +63,6 @@ examples:
> > > > >              reg = <0>;
> > > > >              lattice,erase-sram;
> > > > >              lattice,erase-feature-row;
> > > > > +            lattice,program-gpios = <&gpio1 2
> > > > > GPIO_ACTIVE_LOW>
> > > > >          };
> > > > >      };
> > > > > -- 
> > > > > 2.30.2
> > > > > 
> > > > 
> > > 
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
index 78f0da8f772f..03dc134ec7b8 100644
--- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
+++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
@@ -26,6 +26,12 @@  properties:
     enum:
       - lattice,machxo2-slave-spi
 
+  program-gpios:
+    maxItems: 1
+    description: |
+      GPIO Output tied to the FPGA's n_program pin to initiate a
+      programming sequence. This pin is active low.
+
   lattice,erase-sram:
     type: boolean
     description: SRAM is to be erased during flash erase operation
@@ -57,5 +63,6 @@  examples:
             reg = <0>;
             lattice,erase-sram;
             lattice,erase-feature-row;
+            lattice,program-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>
         };
     };