* [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-30 20:30 ` Rob Herring
2022-08-25 14:13 ` [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties Johannes Zink
` (15 subsequent siblings)
16 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
This commit prepares adding additional properties to the machxo2-slave
device. No functional changes.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
.../bindings/fpga/lattice,machxo2-slave.yaml | 46 +++++++++++++++++++
.../bindings/fpga/lattice-machxo2-spi.txt | 29 ------------
2 files changed, 46 insertions(+), 29 deletions(-)
create mode 100644 Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
delete mode 100644 Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt
diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
new file mode 100644
index 000000000000..d05acd6b0fc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fpga/lattice,machxo2-slave.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lattice MachXO2 Slave FPGA Manager Device Tree Bindings
+
+maintainers:
+ - Johannes Zink <j.zink@pengutronix.de>
+
+description: |
+ Device used for loading the bitstream of Lattice MachXO2 FPGAs. The
+ programming sequence is described in FPGA-TN-02155 on www.latticesemi.com
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: lattice,machxo2-slave-spi
+ then:
+ $ref: /schemas/spi/spi-peripheral-props.yaml#
+properties:
+ compatible:
+ enum:
+ - lattice,machxo2-slave-spi
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fpga@0 {
+ compatible = "lattice,machxo2-slave-spi";
+ spi-max-frequency = <8000000>;
+ reg = <0>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt b/Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt
deleted file mode 100644
index a8c362eb160c..000000000000
--- a/Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-Lattice MachXO2 Slave SPI FPGA Manager
-
-Lattice MachXO2 FPGAs support a method of loading the bitstream over
-'slave SPI' interface.
-
-See 'MachXO2ProgrammingandConfigurationUsageGuide.pdf' on www.latticesemi.com
-
-Required properties:
-- compatible: should contain "lattice,machxo2-slave-spi"
-- reg: spi chip select of the FPGA
-
-Example for full FPGA configuration:
-
- fpga-region0 {
- compatible = "fpga-region";
- fpga-mgr = <&fpga_mgr_spi>;
- #address-cells = <0x1>;
- #size-cells = <0x1>;
- };
-
- spi1: spi@2000 {
- ...
-
- fpga_mgr_spi: fpga-mgr@0 {
- compatible = "lattice,machxo2-slave-spi";
- spi-max-frequency = <8000000>;
- reg = <0>;
- };
- };
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML
2022-08-25 14:13 ` [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML Johannes Zink
@ 2022-08-30 20:30 ` Rob Herring
2022-08-31 7:12 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Rob Herring @ 2022-08-30 20:30 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Moritz Fischer, Wu Hao, Xu Yilun, kernel
On Thu, Aug 25, 2022 at 04:13:28PM +0200, Johannes Zink wrote:
> This commit prepares adding additional properties to the machxo2-slave
> device. No functional changes.
Please use get_maintainers.pl so *all* the maintainers/lists get Cc'ed.
>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
> .../bindings/fpga/lattice,machxo2-slave.yaml | 46 +++++++++++++++++++
> .../bindings/fpga/lattice-machxo2-spi.txt | 29 ------------
> 2 files changed, 46 insertions(+), 29 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> delete mode 100644 Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt
>
> diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> new file mode 100644
> index 000000000000..d05acd6b0fc6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/fpga/lattice,machxo2-slave.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Lattice MachXO2 Slave FPGA Manager Device Tree Bindings
> +
> +maintainers:
> + - Johannes Zink <j.zink@pengutronix.de>
> +
> +description: |
> + Device used for loading the bitstream of Lattice MachXO2 FPGAs. The
> + programming sequence is described in FPGA-TN-02155 on www.latticesemi.com
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: lattice,machxo2-slave-spi
> + then:
> + $ref: /schemas/spi/spi-peripheral-props.yaml#
You don't need the 'if' because the schema is only applied if the
compatible matches.
Blank line needed here.
> +properties:
> + compatible:
> + enum:
> + - lattice,machxo2-slave-spi
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fpga@0 {
> + compatible = "lattice,machxo2-slave-spi";
> + spi-max-frequency = <8000000>;
> + reg = <0>;
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt b/Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt
> deleted file mode 100644
> index a8c362eb160c..000000000000
> --- a/Documentation/devicetree/bindings/fpga/lattice-machxo2-spi.txt
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -Lattice MachXO2 Slave SPI FPGA Manager
> -
> -Lattice MachXO2 FPGAs support a method of loading the bitstream over
> -'slave SPI' interface.
> -
> -See 'MachXO2ProgrammingandConfigurationUsageGuide.pdf' on www.latticesemi.com
> -
> -Required properties:
> -- compatible: should contain "lattice,machxo2-slave-spi"
> -- reg: spi chip select of the FPGA
> -
> -Example for full FPGA configuration:
> -
> - fpga-region0 {
> - compatible = "fpga-region";
> - fpga-mgr = <&fpga_mgr_spi>;
> - #address-cells = <0x1>;
> - #size-cells = <0x1>;
> - };
> -
> - spi1: spi@2000 {
> - ...
> -
> - fpga_mgr_spi: fpga-mgr@0 {
> - compatible = "lattice,machxo2-slave-spi";
> - spi-max-frequency = <8000000>;
> - reg = <0>;
> - };
> - };
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML
2022-08-30 20:30 ` Rob Herring
@ 2022-08-31 7:12 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-31 7:12 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-fpga, Moritz Fischer, kernel, Xu Yilun, Wu Hao
Hi Rob,
On Tue, 2022-08-30 at 15:30 -0500, Rob Herring wrote:
> On Thu, Aug 25, 2022 at 04:13:28PM +0200, Johannes Zink wrote:
> > This commit prepares adding additional properties to the machxo2-
> > slave
> > device. No functional changes.
>
> Please use get_maintainers.pl so *all* the maintainers/lists get
> Cc'ed.
>
ok, will do that in the future.
> >
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
> > .../bindings/fpga/lattice,machxo2-slave.yaml | 46
> > +++++++++++++++++++
> > .../bindings/fpga/lattice-machxo2-spi.txt | 29 ------------
> > 2 files changed, 46 insertions(+), 29 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> > delete mode 100644 Documentation/devicetree/bindings/fpga/lattice-
> > machxo2-spi.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> > b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> > new file mode 100644
> > index 000000000000..d05acd6b0fc6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > slave.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/fpga/lattice,machxo2-slave.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Lattice MachXO2 Slave FPGA Manager Device Tree Bindings
> > +
> > +maintainers:
> > + - Johannes Zink <j.zink@pengutronix.de>
> > +
> > +description: |
> > + Device used for loading the bitstream of Lattice MachXO2 FPGAs.
> > The
> > + programming sequence is described in FPGA-TN-02155 on
> > www.latticesemi.com
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: lattice,machxo2-slave-spi
> > + then:
> > + $ref: /schemas/spi/spi-peripheral-props.yaml#
>
> You don't need the 'if' because the schema is only applied if the
> compatible matches.
>
> Blank line needed here.
ack, will fix it in v2
Best regards
Johannes
>
> > +properties:
> > + compatible:
> > + enum:
> > + - lattice,machxo2-slave-spi
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + spi0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + fpga@0 {
> > + compatible = "lattice,machxo2-slave-spi";
> > + spi-max-frequency = <8000000>;
> > + reg = <0>;
> > + };
> > + };
> > diff --git a/Documentation/devicetree/bindings/fpga/lattice-
> > machxo2-spi.txt b/Documentation/devicetree/bindings/fpga/lattice-
> > machxo2-spi.txt
> > deleted file mode 100644
> > index a8c362eb160c..000000000000
> > --- a/Documentation/devicetree/bindings/fpga/lattice-machxo2-
> > spi.txt
> > +++ /dev/null
> > @@ -1,29 +0,0 @@
> > -Lattice MachXO2 Slave SPI FPGA Manager
> > -
> > -Lattice MachXO2 FPGAs support a method of loading the bitstream
> > over
> > -'slave SPI' interface.
> > -
> > -See 'MachXO2ProgrammingandConfigurationUsageGuide.pdf' on
> > www.latticesemi.com
> > -
> > -Required properties:
> > -- compatible: should contain "lattice,machxo2-slave-spi"
> > -- reg: spi chip select of the FPGA
> > -
> > -Example for full FPGA configuration:
> > -
> > - fpga-region0 {
> > - compatible = "fpga-region";
> > - fpga-mgr = <&fpga_mgr_spi>;
> > - #address-cells = <0x1>;
> > - #size-cells = <0x1>;
> > - };
> > -
> > - spi1: spi@2000 {
> > - ...
> > -
> > - fpga_mgr_spi: fpga-mgr@0 {
> > - compatible = "lattice,machxo2-slave-spi";
> > - spi-max-frequency = <8000000>;
> > - reg = <0>;
> > - };
> > - };
> > --
> > 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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
2022-08-25 14:13 ` [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-29 7:39 ` Xu Yilun
2022-08-30 20:36 ` Rob Herring
2022-08-25 14:13 ` [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init Johannes Zink
` (14 subsequent siblings)
16 siblings, 2 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
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
+
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
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties
2022-08-25 14:13 ` [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties Johannes Zink
@ 2022-08-29 7:39 ` Xu Yilun
[not found] ` <9d5512768acb4d57f339942007402a9ed9483e84.camel@pengutronix.de>
2022-08-30 20:36 ` Rob Herring
1 sibling, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 7:39 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
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
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties
2022-08-25 14:13 ` [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties Johannes Zink
2022-08-29 7:39 ` Xu Yilun
@ 2022-08-30 20:36 ` Rob Herring
2022-08-31 7:07 ` Johannes Zink
1 sibling, 1 reply; 48+ messages in thread
From: Rob Herring @ 2022-08-30 20:36 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Moritz Fischer, Wu Hao, Xu Yilun, kernel
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
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties
2022-08-30 20:36 ` Rob Herring
@ 2022-08-31 7:07 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-31 7:07 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-fpga, Moritz Fischer, kernel, Xu Yilun, Wu Hao
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
>
>
--
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
2022-08-25 14:13 ` [PATCH 01/16] dt-bindings: fpga: convert Lattice MachXO2 Slave binding to YAML Johannes Zink
2022-08-25 14:13 ` [PATCH 02/16] dt-bindings: fpga: machxo2-slave: add erasure properties Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 18:51 ` Rob Herring
2022-08-29 7:45 ` Xu Yilun
2022-08-25 14:13 ` [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible Johannes Zink
` (13 subsequent siblings)
16 siblings, 2 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
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(+)
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
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init
2022-08-25 14:13 ` [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init Johannes Zink
@ 2022-08-25 18:51 ` Rob Herring
2022-08-26 7:56 ` Johannes Zink
2022-08-29 7:45 ` Xu Yilun
1 sibling, 1 reply; 48+ messages in thread
From: Rob Herring @ 2022-08-25 18:51 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Moritz Fischer, kernel, Wu Hao,
Rob Herring, Xu Yilun
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.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init
2022-08-25 18:51 ` Rob Herring
@ 2022-08-26 7:56 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-26 7:56 UTC (permalink / raw)
To: Rob Herring
Cc: linux-fpga, devicetree, Moritz Fischer, kernel, Wu Hao,
Rob Herring, Xu Yilun
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
--
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init
2022-08-25 14:13 ` [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init Johannes Zink
2022-08-25 18:51 ` Rob Herring
@ 2022-08-29 7:45 ` Xu Yilun
[not found] ` <a42d72cd71c96ca675f5bb0cf59128c7f1cb04bb.camel@pengutronix.de>
1 sibling, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 7:45 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
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
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (2 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 03/16] dt-bindings: fpga: machxo2-slave: add pin for program sequence init Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-30 20:40 ` Rob Herring
2022-08-25 14:13 ` [PATCH 05/16] fpga: machxo2-spi: remove #ifdef DEBUG Johannes Zink
` (12 subsequent siblings)
16 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
Lattice MachXO2 FPGAs allow reconfiguration over I2C as well as over
SPI. Add the I2C option to the binding as well.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
.../bindings/fpga/lattice,machxo2-slave.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
index 03dc134ec7b8..d48d92f27c92 100644
--- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
+++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
@@ -21,10 +21,22 @@ allOf:
const: lattice,machxo2-slave-spi
then:
$ref: /schemas/spi/spi-peripheral-props.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: lattice,machxo2-slave-i2c
+ then:
+ properties:
+ reg:
+ description: I2C address
+ maxItems: 1
+
properties:
compatible:
enum:
- lattice,machxo2-slave-spi
+ - lattice,machxo2-slave-i2c
program-gpios:
maxItems: 1
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible
2022-08-25 14:13 ` [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible Johannes Zink
@ 2022-08-30 20:40 ` Rob Herring
2022-08-31 7:10 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Rob Herring @ 2022-08-30 20:40 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Moritz Fischer, Wu Hao, Xu Yilun, kernel
On Thu, Aug 25, 2022 at 04:13:31PM +0200, Johannes Zink wrote:
> Lattice MachXO2 FPGAs allow reconfiguration over I2C as well as over
> SPI. Add the I2C option to the binding as well.
>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
> .../bindings/fpga/lattice,machxo2-slave.yaml | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> index 03dc134ec7b8..d48d92f27c92 100644
> --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> @@ -21,10 +21,22 @@ allOf:
> const: lattice,machxo2-slave-spi
> then:
> $ref: /schemas/spi/spi-peripheral-props.yaml#
Okay, I guess you do need the if/then. Technically, you don't need it
til this patch, but that's fine.
The allOf/if part should go after 'required' section though.
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: lattice,machxo2-slave-i2c
> + then:
> + properties:
> + reg:
> + description: I2C address
> + maxItems: 1
'reg' is needed in either case. The only conditional part is the
description which you don't need. So this can be dropped.
> +
> properties:
> compatible:
> enum:
> - lattice,machxo2-slave-spi
> + - lattice,machxo2-slave-i2c
>
> program-gpios:
> maxItems: 1
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible
2022-08-30 20:40 ` Rob Herring
@ 2022-08-31 7:10 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-31 7:10 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-fpga, Moritz Fischer, kernel, Xu Yilun, Wu Hao
Hi Rob,
On Tue, 2022-08-30 at 15:40 -0500, Rob Herring wrote:
> On Thu, Aug 25, 2022 at 04:13:31PM +0200, Johannes Zink wrote:
> > Lattice MachXO2 FPGAs allow reconfiguration over I2C as well as
> > over
> > SPI. Add the I2C option to the binding as well.
> >
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
> > .../bindings/fpga/lattice,machxo2-slave.yaml | 12
> > ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> > b/Documentation/devicetree/bindings/fpga/lattice,machxo2-slave.yaml
> > index 03dc134ec7b8..d48d92f27c92 100644
> > --- a/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > slave.yaml
> > +++ b/Documentation/devicetree/bindings/fpga/lattice,machxo2-
> > slave.yaml
> > @@ -21,10 +21,22 @@ allOf:
> > const: lattice,machxo2-slave-spi
> > then:
> > $ref: /schemas/spi/spi-peripheral-props.yaml#
>
> Okay, I guess you do need the if/then. Technically, you don't need it
> til this patch, but that's fine.
>
> The allOf/if part should go after 'required' section though.
ack.
>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: lattice,machxo2-slave-i2c
> > + then:
> > + properties:
> > + reg:
> > + description: I2C address
> > + maxItems: 1
>
> 'reg' is needed in either case. The only conditional part is the
> description which you don't need. So this can be dropped.
ack. I will revisit both for v2.
Best regards
Johannes
>
> > +
> > properties:
> > compatible:
> > enum:
> > - lattice,machxo2-slave-spi
> > + - lattice,machxo2-slave-i2c
> >
> > program-gpios:
> > maxItems: 1
> > --
> > 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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 05/16] fpga: machxo2-spi: remove #ifdef DEBUG
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (3 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 04/16] dt-bindings: fpga: machxo2-slave: add lattice,machxo2-slave-i2c compatible Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 06/16] fpga: machxo2-spi: factor out status check for readability Johannes Zink
` (11 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
This provides dynamic debug support, pr_debug checks anyway if DEBUG is
defined statically or is activated dynamically.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-spi.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 905607992a12..39dd62359821 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -88,7 +88,6 @@ static int get_status(struct spi_device *spi, unsigned long *status)
return 0;
}
-#ifdef DEBUG
static const char *get_err_string(u8 err)
{
switch (err) {
@@ -104,16 +103,13 @@ static const char *get_err_string(u8 err)
return "Default switch case";
}
-#endif
static void dump_status_reg(unsigned long *status)
{
-#ifdef DEBUG
pr_debug("machxo2 status: 0x%08lX - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
*status, test_bit(DONE, status), test_bit(ENAB, status),
test_bit(BUSY, status), test_bit(FAIL, status),
test_bit(DVER, status), get_err_string(get_err(status)));
-#endif
}
static int wait_until_not_busy(struct spi_device *spi)
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 06/16] fpga: machxo2-spi: factor out status check for readability
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (4 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 05/16] fpga: machxo2-spi: remove #ifdef DEBUG Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility Johannes Zink
` (10 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
We have the same sequence at two different places, so factor it out
into a helper to improve readability.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-spi.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 39dd62359821..5e12612c7289 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -167,14 +167,19 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
return ret;
}
+static bool machxo2_status_done(unsigned long status)
+{
+ return !test_bit(BUSY, &status) && test_bit(DONE, &status) &&
+ get_err(&status) == ENOERR;
+}
+
static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager *mgr)
{
struct spi_device *spi = mgr->priv;
unsigned long status;
get_status(spi, &status);
- if (!test_bit(BUSY, &status) && test_bit(DONE, &status) &&
- get_err(&status) == ENOERR)
+ if (machxo2_status_done(status))
return FPGA_MGR_STATE_OPERATING;
return FPGA_MGR_STATE_UNKNOWN;
@@ -329,8 +334,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
/* check refresh status */
get_status(spi, &status);
dump_status_reg(&status);
- if (!test_bit(BUSY, &status) && test_bit(DONE, &status) &&
- get_err(&status) == ENOERR)
+ if (machxo2_status_done(status))
break;
if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
machxo2_cleanup(mgr);
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (5 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 06/16] fpga: machxo2-spi: factor out status check for readability Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-29 8:19 ` Xu Yilun
2022-08-25 14:13 ` [PATCH 08/16] fpga: machxo2-spi: simplify with spi_sync_transfer() Johannes Zink
` (9 subsequent siblings)
16 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
The SPI message is written into the lowest-addressed bits of an
unsigned long variable, but be32_to_cpu is called on the least
significant bits of the variable. On big-endian 64-bit systems,
this would give a wrong result. Fix this by using the fixed-size
u32 instead of unsigned long. This clashes with the use of
test_bit, which is unnecessary for a single u32 variable, so
we adjust all usage sites appropriately and prefix the macros
with MACHXO2_ while at it.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-spi.c | 110 ++++++++++++++++++-------------------
1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 5e12612c7289..d1a8f28e04e7 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -9,6 +9,7 @@
*/
#include <linux/delay.h>
+#include <linux/bitfield.h>
#include <linux/fpga/fpga-mgr.h>
#include <linux/gpio/consumer.h>
#include <linux/module.h>
@@ -41,41 +42,40 @@
#define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
/* Status register bits, errors and error mask */
-#define BUSY 12
-#define DONE 8
-#define DVER 27
-#define ENAB 9
-#define ERRBITS 23
-#define ERRMASK 7
-#define FAIL 13
-
-#define ENOERR 0 /* no error */
-#define EID 1
-#define ECMD 2
-#define ECRC 3
-#define EPREAM 4 /* preamble error */
-#define EABRT 5 /* abort error */
-#define EOVERFL 6 /* overflow error */
-#define ESDMEOF 7 /* SDM EOF */
-
-static inline u8 get_err(unsigned long *status)
+#define MACHXO2_BUSY BIT(12)
+#define MACHXO2_DONE BIT(8)
+#define MACHXO2_DVER BIT(27)
+#define MACHXO2_ENAB BIT(9)
+#define MACHXO2_ERR GENMASK(25, 23)
+#define MACHXO2_ERR_ENOERR 0 /* no error */
+#define MACHXO2_ERR_EID 1
+#define MACHXO2_ERR_ECMD 2
+#define MACHXO2_ERR_ECRC 3
+#define MACHXO2_ERR_EPREAM 4 /* preamble error */
+#define MACHXO2_ERR_EABRT 5 /* abort error */
+#define MACHXO2_ERR_EOVERFL 6 /* overflow error */
+#define MACHXO2_ERR_ESDMEOF 7 /* SDM EOF */
+#define MACHXO2_FAIL BIT(13)
+
+static inline u8 get_err(u32 status)
{
- return (*status >> ERRBITS) & ERRMASK;
+ return FIELD_GET(MACHXO2_ERR, status);
}
-static int get_status(struct spi_device *spi, unsigned long *status)
+static int get_status(struct spi_device *spi, u32 *status)
{
struct spi_message msg;
struct spi_transfer rx, tx;
static const u8 cmd[] = LSC_READ_STATUS;
+ __be32 tmp;
int ret;
memset(&rx, 0, sizeof(rx));
memset(&tx, 0, sizeof(tx));
tx.tx_buf = cmd;
tx.len = sizeof(cmd);
- rx.rx_buf = status;
- rx.len = 4;
+ rx.rx_buf = &tmp;
+ rx.len = sizeof(tmp);
spi_message_init(&msg);
spi_message_add_tail(&tx, &msg);
spi_message_add_tail(&rx, &msg);
@@ -83,7 +83,7 @@ static int get_status(struct spi_device *spi, unsigned long *status)
if (ret)
return ret;
- *status = be32_to_cpu(*status);
+ *status = be32_to_cpu(tmp);
return 0;
}
@@ -91,30 +91,30 @@ static int get_status(struct spi_device *spi, unsigned long *status)
static const char *get_err_string(u8 err)
{
switch (err) {
- case ENOERR: return "No Error";
- case EID: return "ID ERR";
- case ECMD: return "CMD ERR";
- case ECRC: return "CRC ERR";
- case EPREAM: return "Preamble ERR";
- case EABRT: return "Abort ERR";
- case EOVERFL: return "Overflow ERR";
- case ESDMEOF: return "SDM EOF";
+ case MACHXO2_ERR_ENOERR: return "No Error";
+ case MACHXO2_ERR_EID: return "ID ERR";
+ case MACHXO2_ERR_ECMD: return "CMD ERR";
+ case MACHXO2_ERR_ECRC: return "CRC ERR";
+ case MACHXO2_ERR_EPREAM: return "Preamble ERR";
+ case MACHXO2_ERR_EABRT: return "Abort ERR";
+ case MACHXO2_ERR_EOVERFL: return "Overflow ERR";
+ case MACHXO2_ERR_ESDMEOF: return "SDM EOF";
}
- return "Default switch case";
+ return "Unknown";
}
-static void dump_status_reg(unsigned long *status)
+static void dump_status_reg(u32 status)
{
- pr_debug("machxo2 status: 0x%08lX - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
- *status, test_bit(DONE, status), test_bit(ENAB, status),
- test_bit(BUSY, status), test_bit(FAIL, status),
- test_bit(DVER, status), get_err_string(get_err(status)));
+ pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
+ status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
+ !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
+ !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
}
static int wait_until_not_busy(struct spi_device *spi)
{
- unsigned long status;
+ u32 status;
int ret, loop = 0;
do {
@@ -123,7 +123,7 @@ static int wait_until_not_busy(struct spi_device *spi)
return ret;
if (++loop >= MACHXO2_MAX_BUSY_LOOP)
return -EBUSY;
- } while (test_bit(BUSY, &status));
+ } while (status & MACHXO2_BUSY);
return 0;
}
@@ -169,14 +169,14 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
static bool machxo2_status_done(unsigned long status)
{
- return !test_bit(BUSY, &status) && test_bit(DONE, &status) &&
- get_err(&status) == ENOERR;
+ return (((status & (MACHXO2_BUSY | MACHXO2_DONE)) == MACHXO2_DONE) &&
+ get_err(status) == MACHXO2_ERR_ENOERR);
}
static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager *mgr)
{
struct spi_device *spi = mgr->priv;
- unsigned long status;
+ u32 status;
get_status(spi, &status);
if (machxo2_status_done(status))
@@ -195,7 +195,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
static const u8 enable[] = ISC_ENABLE;
static const u8 erase[] = ISC_ERASE;
static const u8 initaddr[] = LSC_INITADDRESS;
- unsigned long status;
+ u32 status;
int ret;
if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
@@ -205,7 +205,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
}
get_status(spi, &status);
- dump_status_reg(&status);
+ dump_status_reg(status);
memset(tx, 0, sizeof(tx));
spi_message_init(&msg);
tx[0].tx_buf = &enable;
@@ -226,11 +226,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
goto fail;
get_status(spi, &status);
- if (test_bit(FAIL, &status)) {
+ if (status & MACHXO2_FAIL) {
ret = -EINVAL;
goto fail;
}
- dump_status_reg(&status);
+ dump_status_reg(status);
spi_message_init(&msg);
tx[2].tx_buf = &initaddr;
@@ -241,7 +241,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
goto fail;
get_status(spi, &status);
- dump_status_reg(&status);
+ dump_status_reg(status);
return 0;
fail:
@@ -258,7 +258,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
struct spi_transfer tx;
static const u8 progincr[] = LSC_PROGINCRNV;
u8 payload[MACHXO2_BUF_SIZE];
- unsigned long status;
+ u32 status;
int i, ret;
if (count % MACHXO2_PAGE_SIZE != 0) {
@@ -266,7 +266,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
return -EINVAL;
}
get_status(spi, &status);
- dump_status_reg(&status);
+ dump_status_reg(status);
memcpy(payload, &progincr, sizeof(progincr));
for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
@@ -284,7 +284,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
}
}
get_status(spi, &status);
- dump_status_reg(&status);
+ dump_status_reg(status);
return 0;
}
@@ -297,7 +297,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
struct spi_transfer tx[2];
static const u8 progdone[] = ISC_PROGRAMDONE;
static const u8 refresh[] = LSC_REFRESH;
- unsigned long status;
+ u32 status;
int ret, refreshloop = 0;
memset(tx, 0, sizeof(tx));
@@ -313,8 +313,8 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
goto fail;
get_status(spi, &status);
- dump_status_reg(&status);
- if (!test_bit(DONE, &status)) {
+ dump_status_reg(status);
+ if (!(status & MACHXO2_DONE)) {
machxo2_cleanup(mgr);
ret = -EINVAL;
goto fail;
@@ -333,7 +333,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
/* check refresh status */
get_status(spi, &status);
- dump_status_reg(&status);
+ dump_status_reg(status);
if (machxo2_status_done(status))
break;
if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
@@ -344,7 +344,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
} while (1);
get_status(spi, &status);
- dump_status_reg(&status);
+ dump_status_reg(status);
return 0;
fail:
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility
2022-08-25 14:13 ` [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility Johannes Zink
@ 2022-08-29 8:19 ` Xu Yilun
2022-08-29 10:41 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 8:19 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
On 2022-08-25 at 16:13:34 +0200, Johannes Zink wrote:
> The SPI message is written into the lowest-addressed bits of an
> unsigned long variable, but be32_to_cpu is called on the least
> significant bits of the variable. On big-endian 64-bit systems,
> this would give a wrong result. Fix this by using the fixed-size
> u32 instead of unsigned long. This clashes with the use of
> test_bit, which is unnecessary for a single u32 variable, so
> we adjust all usage sites appropriately and prefix the macros
> with MACHXO2_ while at it.
>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
> drivers/fpga/machxo2-spi.c | 110 ++++++++++++++++++-------------------
> 1 file changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
> index 5e12612c7289..d1a8f28e04e7 100644
> --- a/drivers/fpga/machxo2-spi.c
> +++ b/drivers/fpga/machxo2-spi.c
> @@ -9,6 +9,7 @@
> */
>
> #include <linux/delay.h>
> +#include <linux/bitfield.h>
> #include <linux/fpga/fpga-mgr.h>
> #include <linux/gpio/consumer.h>
> #include <linux/module.h>
> @@ -41,41 +42,40 @@
> #define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
>
> /* Status register bits, errors and error mask */
> -#define BUSY 12
> -#define DONE 8
> -#define DVER 27
> -#define ENAB 9
> -#define ERRBITS 23
> -#define ERRMASK 7
> -#define FAIL 13
> -
> -#define ENOERR 0 /* no error */
> -#define EID 1
> -#define ECMD 2
> -#define ECRC 3
> -#define EPREAM 4 /* preamble error */
> -#define EABRT 5 /* abort error */
> -#define EOVERFL 6 /* overflow error */
> -#define ESDMEOF 7 /* SDM EOF */
> -
> -static inline u8 get_err(unsigned long *status)
> +#define MACHXO2_BUSY BIT(12)
> +#define MACHXO2_DONE BIT(8)
> +#define MACHXO2_DVER BIT(27)
> +#define MACHXO2_ENAB BIT(9)
> +#define MACHXO2_ERR GENMASK(25, 23)
> +#define MACHXO2_ERR_ENOERR 0 /* no error */
> +#define MACHXO2_ERR_EID 1
> +#define MACHXO2_ERR_ECMD 2
> +#define MACHXO2_ERR_ECRC 3
> +#define MACHXO2_ERR_EPREAM 4 /* preamble error */
> +#define MACHXO2_ERR_EABRT 5 /* abort error */
> +#define MACHXO2_ERR_EOVERFL 6 /* overflow error */
> +#define MACHXO2_ERR_ESDMEOF 7 /* SDM EOF */
> +#define MACHXO2_FAIL BIT(13)
> +
> +static inline u8 get_err(u32 status)
> {
> - return (*status >> ERRBITS) & ERRMASK;
> + return FIELD_GET(MACHXO2_ERR, status);
> }
So far the changes have nothing to do with the endian issue, is it? So
please put it into another patch.
>
> -static int get_status(struct spi_device *spi, unsigned long *status)
> +static int get_status(struct spi_device *spi, u32 *status)
> {
> struct spi_message msg;
> struct spi_transfer rx, tx;
> static const u8 cmd[] = LSC_READ_STATUS;
> + __be32 tmp;
> int ret;
>
> memset(&rx, 0, sizeof(rx));
> memset(&tx, 0, sizeof(tx));
> tx.tx_buf = cmd;
> tx.len = sizeof(cmd);
> - rx.rx_buf = status;
> - rx.len = 4;
> + rx.rx_buf = &tmp;
> + rx.len = sizeof(tmp);
> spi_message_init(&msg);
> spi_message_add_tail(&tx, &msg);
> spi_message_add_tail(&rx, &msg);
> @@ -83,7 +83,7 @@ static int get_status(struct spi_device *spi, unsigned long *status)
> if (ret)
> return ret;
>
> - *status = be32_to_cpu(*status);
> + *status = be32_to_cpu(tmp);
Why not directly operate on the status, I don't see the difference.
>
> return 0;
> }
> @@ -91,30 +91,30 @@ static int get_status(struct spi_device *spi, unsigned long *status)
> static const char *get_err_string(u8 err)
> {
> switch (err) {
> - case ENOERR: return "No Error";
> - case EID: return "ID ERR";
> - case ECMD: return "CMD ERR";
> - case ECRC: return "CRC ERR";
> - case EPREAM: return "Preamble ERR";
> - case EABRT: return "Abort ERR";
> - case EOVERFL: return "Overflow ERR";
> - case ESDMEOF: return "SDM EOF";
> + case MACHXO2_ERR_ENOERR: return "No Error";
> + case MACHXO2_ERR_EID: return "ID ERR";
> + case MACHXO2_ERR_ECMD: return "CMD ERR";
> + case MACHXO2_ERR_ECRC: return "CRC ERR";
> + case MACHXO2_ERR_EPREAM: return "Preamble ERR";
> + case MACHXO2_ERR_EABRT: return "Abort ERR";
> + case MACHXO2_ERR_EOVERFL: return "Overflow ERR";
> + case MACHXO2_ERR_ESDMEOF: return "SDM EOF";
> }
Same concern, no relation to the commit message
>
> - return "Default switch case";
> + return "Unknown";
> }
>
> -static void dump_status_reg(unsigned long *status)
> +static void dump_status_reg(u32 status)
> {
> - pr_debug("machxo2 status: 0x%08lX - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
> - *status, test_bit(DONE, status), test_bit(ENAB, status),
> - test_bit(BUSY, status), test_bit(FAIL, status),
> - test_bit(DVER, status), get_err_string(get_err(status)));
> + pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
> + status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
> + !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
> + !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
Same concern. I'll not point out one by one below.
Thanks,
Yilun
> }
>
> static int wait_until_not_busy(struct spi_device *spi)
> {
> - unsigned long status;
> + u32 status;
> int ret, loop = 0;
>
> do {
> @@ -123,7 +123,7 @@ static int wait_until_not_busy(struct spi_device *spi)
> return ret;
> if (++loop >= MACHXO2_MAX_BUSY_LOOP)
> return -EBUSY;
> - } while (test_bit(BUSY, &status));
> + } while (status & MACHXO2_BUSY);
>
> return 0;
> }
> @@ -169,14 +169,14 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
>
> static bool machxo2_status_done(unsigned long status)
> {
> - return !test_bit(BUSY, &status) && test_bit(DONE, &status) &&
> - get_err(&status) == ENOERR;
> + return (((status & (MACHXO2_BUSY | MACHXO2_DONE)) == MACHXO2_DONE) &&
> + get_err(status) == MACHXO2_ERR_ENOERR);
> }
>
> static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager *mgr)
> {
> struct spi_device *spi = mgr->priv;
> - unsigned long status;
> + u32 status;
>
> get_status(spi, &status);
> if (machxo2_status_done(status))
> @@ -195,7 +195,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
> static const u8 enable[] = ISC_ENABLE;
> static const u8 erase[] = ISC_ERASE;
> static const u8 initaddr[] = LSC_INITADDRESS;
> - unsigned long status;
> + u32 status;
> int ret;
>
> if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
> @@ -205,7 +205,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
> }
>
> get_status(spi, &status);
> - dump_status_reg(&status);
> + dump_status_reg(status);
> memset(tx, 0, sizeof(tx));
> spi_message_init(&msg);
> tx[0].tx_buf = &enable;
> @@ -226,11 +226,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
> goto fail;
>
> get_status(spi, &status);
> - if (test_bit(FAIL, &status)) {
> + if (status & MACHXO2_FAIL) {
> ret = -EINVAL;
> goto fail;
> }
> - dump_status_reg(&status);
> + dump_status_reg(status);
>
> spi_message_init(&msg);
> tx[2].tx_buf = &initaddr;
> @@ -241,7 +241,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
> goto fail;
>
> get_status(spi, &status);
> - dump_status_reg(&status);
> + dump_status_reg(status);
>
> return 0;
> fail:
> @@ -258,7 +258,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
> struct spi_transfer tx;
> static const u8 progincr[] = LSC_PROGINCRNV;
> u8 payload[MACHXO2_BUF_SIZE];
> - unsigned long status;
> + u32 status;
> int i, ret;
>
> if (count % MACHXO2_PAGE_SIZE != 0) {
> @@ -266,7 +266,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
> return -EINVAL;
> }
> get_status(spi, &status);
> - dump_status_reg(&status);
> + dump_status_reg(status);
> memcpy(payload, &progincr, sizeof(progincr));
> for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
> @@ -284,7 +284,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
> }
> }
> get_status(spi, &status);
> - dump_status_reg(&status);
> + dump_status_reg(status);
>
> return 0;
> }
> @@ -297,7 +297,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
> struct spi_transfer tx[2];
> static const u8 progdone[] = ISC_PROGRAMDONE;
> static const u8 refresh[] = LSC_REFRESH;
> - unsigned long status;
> + u32 status;
> int ret, refreshloop = 0;
>
> memset(tx, 0, sizeof(tx));
> @@ -313,8 +313,8 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
> goto fail;
>
> get_status(spi, &status);
> - dump_status_reg(&status);
> - if (!test_bit(DONE, &status)) {
> + dump_status_reg(status);
> + if (!(status & MACHXO2_DONE)) {
> machxo2_cleanup(mgr);
> ret = -EINVAL;
> goto fail;
> @@ -333,7 +333,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
>
> /* check refresh status */
> get_status(spi, &status);
> - dump_status_reg(&status);
> + dump_status_reg(status);
> if (machxo2_status_done(status))
> break;
> if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
> @@ -344,7 +344,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
> } while (1);
>
> get_status(spi, &status);
> - dump_status_reg(&status);
> + dump_status_reg(status);
>
> return 0;
> fail:
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility
2022-08-29 8:19 ` Xu Yilun
@ 2022-08-29 10:41 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-29 10:41 UTC (permalink / raw)
To: Xu Yilun
Cc: devicetree, linux-fpga, Rob Herring, Moritz Fischer, kernel, Wu Hao
Hi Yilun,
On Mon, 2022-08-29 at 16:19 +0800, Xu Yilun wrote:
> On 2022-08-25 at 16:13:34 +0200, Johannes Zink wrote:
> > The SPI message is written into the lowest-addressed bits of an
> > unsigned long variable, but be32_to_cpu is called on the least
> > significant bits of the variable. On big-endian 64-bit systems,
> > this would give a wrong result. Fix this by using the fixed-size
> > u32 instead of unsigned long. This clashes with the use of
> > test_bit, which is unnecessary for a single u32 variable, so
> > we adjust all usage sites appropriately and prefix the macros
> > with MACHXO2_ while at it.
> >
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
> > drivers/fpga/machxo2-spi.c | 110 ++++++++++++++++++---------------
> > ----
> > 1 file changed, 55 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-
> > spi.c
> > index 5e12612c7289..d1a8f28e04e7 100644
> > --- a/drivers/fpga/machxo2-spi.c
> > +++ b/drivers/fpga/machxo2-spi.c
> > @@ -9,6 +9,7 @@
> > */
> >
> > #include <linux/delay.h>
> > +#include <linux/bitfield.h>
> > #include <linux/fpga/fpga-mgr.h>
> > #include <linux/gpio/consumer.h>
> > #include <linux/module.h>
> > @@ -41,41 +42,40 @@
> > #define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
> >
> > /* Status register bits, errors and error mask */
> > -#define BUSY 12
> > -#define DONE 8
> > -#define DVER 27
> > -#define ENAB 9
> > -#define ERRBITS 23
> > -#define ERRMASK 7
> > -#define FAIL 13
> > -
> > -#define ENOERR 0 /* no error */
> > -#define EID 1
> > -#define ECMD 2
> > -#define ECRC 3
> > -#define EPREAM 4 /* preamble error */
> > -#define EABRT 5 /* abort error */
> > -#define EOVERFL 6 /* overflow error */
> > -#define ESDMEOF 7 /* SDM EOF */
> > -
> > -static inline u8 get_err(unsigned long *status)
> > +#define MACHXO2_BUSY BIT(12)
> > +#define MACHXO2_DONE BIT(8)
> > +#define MACHXO2_DVER BIT(27)
> > +#define MACHXO2_ENAB BIT(9)
> > +#define MACHXO2_ERR GENMASK(25, 23)
> > +#define MACHXO2_ERR_ENOERR 0 /* no error */
> > +#define MACHXO2_ERR_EID 1
> > +#define MACHXO2_ERR_ECMD 2
> > +#define MACHXO2_ERR_ECRC 3
> > +#define MACHXO2_ERR_EPREAM 4 /* preamble error */
> > +#define MACHXO2_ERR_EABRT 5 /* abort error */
> > +#define MACHXO2_ERR_EOVERFL 6 /* overflow error */
> > +#define MACHXO2_ERR_ESDMEOF 7 /* SDM EOF */
> > +#define MACHXO2_FAIL BIT(13)
> > +
> > +static inline u8 get_err(u32 status)
> > {
> > - return (*status >> ERRBITS) & ERRMASK;
> > + return FIELD_GET(MACHXO2_ERR, status);
> > }
>
> So far the changes have nothing to do with the endian issue, is it?
> So
> please put it into another patch.
This is explained in the commit message. We need to get rid of unsigned
long, but test_bit requires using that type. Using the BIT and GENMASK
macros instead allows us to work on a u32 directly.
Admittedly, prefixing the macros with MACHXO2_ can be separated out,
but as I am already touching them to make them into masks, which is
required when not using test_bit, I chose to squash this here.
This is also mentioned in the commit message. If you prefer, I do
the rename in a separate patch. I can do that for v2, but I don't
see how I can cleanly separate s/unsigned long/u32/ and test_bit
removal.
>
> >
> > -static int get_status(struct spi_device *spi, unsigned long
> > *status)
> > +static int get_status(struct spi_device *spi, u32 *status)
> > {
> > struct spi_message msg;
> > struct spi_transfer rx, tx;
> > static const u8 cmd[] = LSC_READ_STATUS;
> > + __be32 tmp;
> > int ret;
> >
> > memset(&rx, 0, sizeof(rx));
> > memset(&tx, 0, sizeof(tx));
> > tx.tx_buf = cmd;
> > tx.len = sizeof(cmd);
> > - rx.rx_buf = status;
> > - rx.len = 4;
> > + rx.rx_buf = &tmp;
> > + rx.len = sizeof(tmp);
> > spi_message_init(&msg);
> > spi_message_add_tail(&tx, &msg);
> > spi_message_add_tail(&rx, &msg);
> > @@ -83,7 +83,7 @@ static int get_status(struct spi_device *spi,
> > unsigned long *status)
> > if (ret)
> > return ret;
> >
> > - *status = be32_to_cpu(*status);
> > + *status = be32_to_cpu(tmp);
>
> Why not directly operate on the status, I don't see the difference.
I wanted to state more clearly that the value read in the rx message is
big endian and needs conversion.
Besides, since be32_to_cpu takes a __be32 as an argument, passing
status, which is an u32, should give a static analyzer warning.
>
> >
> > return 0;
> > }
> > @@ -91,30 +91,30 @@ static int get_status(struct spi_device *spi,
> > unsigned long *status)
> > static const char *get_err_string(u8 err)
> > {
> > switch (err) {
> > - case ENOERR: return "No Error";
> > - case EID: return "ID ERR";
> > - case ECMD: return "CMD ERR";
> > - case ECRC: return "CRC ERR";
> > - case EPREAM: return "Preamble ERR";
> > - case EABRT: return "Abort ERR";
> > - case EOVERFL: return "Overflow ERR";
> > - case ESDMEOF: return "SDM EOF";
> > + case MACHXO2_ERR_ENOERR: return "No Error";
> > + case MACHXO2_ERR_EID: return "ID ERR";
> > + case MACHXO2_ERR_ECMD: return "CMD ERR";
> > + case MACHXO2_ERR_ECRC: return "CRC ERR";
> > + case MACHXO2_ERR_EPREAM: return "Preamble ERR";
> > + case MACHXO2_ERR_EABRT: return "Abort ERR";
> > + case MACHXO2_ERR_EOVERFL: return "Overflow ERR";
> > + case MACHXO2_ERR_ESDMEOF: return "SDM EOF";
> > }
>
> Same concern, no relation to the commit message
please see my comment above. If you prefer, I split it out in a
separate patch.
>
> >
> > - return "Default switch case";
> > + return "Unknown";
> > }
> >
> > -static void dump_status_reg(unsigned long *status)
> > +static void dump_status_reg(u32 status)
> > {
> > - pr_debug("machxo2 status: 0x%08lX - done=%d, cfgena=%d,
> > busy=%d, fail=%d, devver=%d, err=%s\n",
> > - *status, test_bit(DONE, status), test_bit(ENAB,
> > status),
> > - test_bit(BUSY, status), test_bit(FAIL, status),
> > - test_bit(DVER, status),
> > get_err_string(get_err(status)));
> > + pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d,
> > busy=%d, fail=%d, devver=%d, err=%s\n",
> > + status, !!FIELD_GET(MACHXO2_DONE, status),
> > !!FIELD_GET(MACHXO2_ENAB, status),
> > + !!FIELD_GET(MACHXO2_BUSY, status),
> > !!FIELD_GET(MACHXO2_FAIL, status),
> > + !!FIELD_GET(MACHXO2_DVER, status),
> > get_err_string(get_err(status)));
>
> Same concern. I'll not point out one by one below.
>
> Thanks,
> Yilun
>
as I explained in the commit message, I am removing test_bit. As far as
the macro renaming is concerned, please see my comments above.
Best regards
Johannes
> > }
>
>
> >
> > static int wait_until_not_busy(struct spi_device *spi)
> > {
> > - unsigned long status;
> > + u32 status;
> > int ret, loop = 0;
> >
> > do {
> > @@ -123,7 +123,7 @@ static int wait_until_not_busy(struct
> > spi_device *spi)
> > return ret;
> > if (++loop >= MACHXO2_MAX_BUSY_LOOP)
> > return -EBUSY;
> > - } while (test_bit(BUSY, &status));
> > + } while (status & MACHXO2_BUSY);
> >
> > return 0;
> > }
> > @@ -169,14 +169,14 @@ static int machxo2_cleanup(struct
> > fpga_manager *mgr)
> >
> > static bool machxo2_status_done(unsigned long status)
> > {
> > - return !test_bit(BUSY, &status) && test_bit(DONE, &status)
> > &&
> > - get_err(&status) == ENOERR;
> > + return (((status & (MACHXO2_BUSY | MACHXO2_DONE)) ==
> > MACHXO2_DONE) &&
> > + get_err(status) == MACHXO2_ERR_ENOERR);
> > }
> >
> > static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager
> > *mgr)
> > {
> > struct spi_device *spi = mgr->priv;
> > - unsigned long status;
> > + u32 status;
> >
> > get_status(spi, &status);
> > if (machxo2_status_done(status))
> > @@ -195,7 +195,7 @@ static int machxo2_write_init(struct
> > fpga_manager *mgr,
> > static const u8 enable[] = ISC_ENABLE;
> > static const u8 erase[] = ISC_ERASE;
> > static const u8 initaddr[] = LSC_INITADDRESS;
> > - unsigned long status;
> > + u32 status;
> > int ret;
> >
> > if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
> > @@ -205,7 +205,7 @@ static int machxo2_write_init(struct
> > fpga_manager *mgr,
> > }
> >
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> > memset(tx, 0, sizeof(tx));
> > spi_message_init(&msg);
> > tx[0].tx_buf = &enable;
> > @@ -226,11 +226,11 @@ static int machxo2_write_init(struct
> > fpga_manager *mgr,
> > goto fail;
> >
> > get_status(spi, &status);
> > - if (test_bit(FAIL, &status)) {
> > + if (status & MACHXO2_FAIL) {
> > ret = -EINVAL;
> > goto fail;
> > }
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> >
> > spi_message_init(&msg);
> > tx[2].tx_buf = &initaddr;
> > @@ -241,7 +241,7 @@ static int machxo2_write_init(struct
> > fpga_manager *mgr,
> > goto fail;
> >
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> >
> > return 0;
> > fail:
> > @@ -258,7 +258,7 @@ static int machxo2_write(struct fpga_manager
> > *mgr, const char *buf,
> > struct spi_transfer tx;
> > static const u8 progincr[] = LSC_PROGINCRNV;
> > u8 payload[MACHXO2_BUF_SIZE];
> > - unsigned long status;
> > + u32 status;
> > int i, ret;
> >
> > if (count % MACHXO2_PAGE_SIZE != 0) {
> > @@ -266,7 +266,7 @@ static int machxo2_write(struct fpga_manager
> > *mgr, const char *buf,
> > return -EINVAL;
> > }
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> > memcpy(payload, &progincr, sizeof(progincr));
> > for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> > memcpy(&payload[sizeof(progincr)], &buf[i],
> > MACHXO2_PAGE_SIZE);
> > @@ -284,7 +284,7 @@ static int machxo2_write(struct fpga_manager
> > *mgr, const char *buf,
> > }
> > }
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> >
> > return 0;
> > }
> > @@ -297,7 +297,7 @@ static int machxo2_write_complete(struct
> > fpga_manager *mgr,
> > struct spi_transfer tx[2];
> > static const u8 progdone[] = ISC_PROGRAMDONE;
> > static const u8 refresh[] = LSC_REFRESH;
> > - unsigned long status;
> > + u32 status;
> > int ret, refreshloop = 0;
> >
> > memset(tx, 0, sizeof(tx));
> > @@ -313,8 +313,8 @@ static int machxo2_write_complete(struct
> > fpga_manager *mgr,
> > goto fail;
> >
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > - if (!test_bit(DONE, &status)) {
> > + dump_status_reg(status);
> > + if (!(status & MACHXO2_DONE)) {
> > machxo2_cleanup(mgr);
> > ret = -EINVAL;
> > goto fail;
> > @@ -333,7 +333,7 @@ static int machxo2_write_complete(struct
> > fpga_manager *mgr,
> >
> > /* check refresh status */
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> > if (machxo2_status_done(status))
> > break;
> > if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
> > @@ -344,7 +344,7 @@ static int machxo2_write_complete(struct
> > fpga_manager *mgr,
> > } while (1);
> >
> > get_status(spi, &status);
> > - dump_status_reg(&status);
> > + dump_status_reg(status);
> >
> > return 0;
> > fail:
> > --
> > 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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 08/16] fpga: machxo2-spi: simplify with spi_sync_transfer()
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (6 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 07/16] fpga: machxo2-spi: fix big-endianness incompatibility Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 09/16] fpga: machxo2-spi: simplify spi write commands Johannes Zink
` (8 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
Use helper functions in order to improve readability.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-spi.c | 96 ++++++++++++++------------------------
1 file changed, 36 insertions(+), 60 deletions(-)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index d1a8f28e04e7..7f7d1066ddee 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -64,22 +64,17 @@ static inline u8 get_err(u32 status)
static int get_status(struct spi_device *spi, u32 *status)
{
- struct spi_message msg;
- struct spi_transfer rx, tx;
+ struct spi_transfer transfers[2] = {};
static const u8 cmd[] = LSC_READ_STATUS;
__be32 tmp;
int ret;
- memset(&rx, 0, sizeof(rx));
- memset(&tx, 0, sizeof(tx));
- tx.tx_buf = cmd;
- tx.len = sizeof(cmd);
- rx.rx_buf = &tmp;
- rx.len = sizeof(tmp);
- spi_message_init(&msg);
- spi_message_add_tail(&tx, &msg);
- spi_message_add_tail(&rx, &msg);
- ret = spi_sync(spi, &msg);
+ transfers[0].tx_buf = cmd;
+ transfers[0].len = sizeof(cmd);
+ transfers[1].rx_buf = &tmp;
+ transfers[1].len = sizeof(tmp);
+
+ ret = spi_sync_transfer(spi, transfers, ARRAY_SIZE(transfers));
if (ret)
return ret;
@@ -131,18 +126,14 @@ static int wait_until_not_busy(struct spi_device *spi)
static int machxo2_cleanup(struct fpga_manager *mgr)
{
struct spi_device *spi = mgr->priv;
- struct spi_message msg;
- struct spi_transfer tx[2];
static const u8 erase[] = ISC_ERASE;
static const u8 refresh[] = LSC_REFRESH;
+ struct spi_transfer tx = {};
int ret;
- memset(tx, 0, sizeof(tx));
- spi_message_init(&msg);
- tx[0].tx_buf = &erase;
- tx[0].len = sizeof(erase);
- spi_message_add_tail(&tx[0], &msg);
- ret = spi_sync(spi, &msg);
+ tx.tx_buf = &erase;
+ tx.len = sizeof(erase);
+ ret = spi_sync_transfer(spi, &tx, 1);
if (ret)
goto fail;
@@ -150,13 +141,11 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
if (ret)
goto fail;
- spi_message_init(&msg);
- tx[1].tx_buf = &refresh;
- tx[1].len = sizeof(refresh);
- tx[1].delay.value = MACHXO2_REFRESH_USEC;
- tx[1].delay.unit = SPI_DELAY_UNIT_USECS;
- spi_message_add_tail(&tx[1], &msg);
- ret = spi_sync(spi, &msg);
+ tx.tx_buf = &refresh;
+ tx.len = sizeof(refresh);
+ tx.delay.value = MACHXO2_REFRESH_USEC;
+ tx.delay.unit = SPI_DELAY_UNIT_USECS;
+ ret = spi_sync_transfer(spi, &tx, 1);
if (ret)
goto fail;
@@ -190,8 +179,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
const char *buf, size_t count)
{
struct spi_device *spi = mgr->priv;
- struct spi_message msg;
- struct spi_transfer tx[3];
+ struct spi_transfer tx[2] = {};
static const u8 enable[] = ISC_ENABLE;
static const u8 erase[] = ISC_ERASE;
static const u8 initaddr[] = LSC_INITADDRESS;
@@ -206,18 +194,15 @@ static int machxo2_write_init(struct fpga_manager *mgr,
get_status(spi, &status);
dump_status_reg(status);
- memset(tx, 0, sizeof(tx));
- spi_message_init(&msg);
+
tx[0].tx_buf = &enable;
tx[0].len = sizeof(enable);
tx[0].delay.value = MACHXO2_LOW_DELAY_USEC;
tx[0].delay.unit = SPI_DELAY_UNIT_USECS;
- spi_message_add_tail(&tx[0], &msg);
tx[1].tx_buf = &erase;
tx[1].len = sizeof(erase);
- spi_message_add_tail(&tx[1], &msg);
- ret = spi_sync(spi, &msg);
+ ret = spi_sync_transfer(spi, tx, ARRAY_SIZE(tx));
if (ret)
goto fail;
@@ -232,11 +217,9 @@ static int machxo2_write_init(struct fpga_manager *mgr,
}
dump_status_reg(status);
- spi_message_init(&msg);
- tx[2].tx_buf = &initaddr;
- tx[2].len = sizeof(initaddr);
- spi_message_add_tail(&tx[2], &msg);
- ret = spi_sync(spi, &msg);
+ tx[0].tx_buf = &initaddr;
+ tx[0].len = sizeof(initaddr);
+ ret = spi_sync_transfer(spi, &tx[0], 1);
if (ret)
goto fail;
@@ -254,8 +237,6 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
size_t count)
{
struct spi_device *spi = mgr->priv;
- struct spi_message msg;
- struct spi_transfer tx;
static const u8 progincr[] = LSC_PROGINCRNV;
u8 payload[MACHXO2_BUF_SIZE];
u32 status;
@@ -269,15 +250,15 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
dump_status_reg(status);
memcpy(payload, &progincr, sizeof(progincr));
for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
+ struct spi_transfer tx = {};
+
memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
- memset(&tx, 0, sizeof(tx));
- spi_message_init(&msg);
+
tx.tx_buf = payload;
tx.len = MACHXO2_BUF_SIZE;
tx.delay.value = MACHXO2_HIGH_DELAY_USEC;
tx.delay.unit = SPI_DELAY_UNIT_USECS;
- spi_message_add_tail(&tx, &msg);
- ret = spi_sync(spi, &msg);
+ ret = spi_sync_transfer(spi, &tx, 1);
if (ret) {
dev_err(&mgr->dev, "Error loading the bitstream.\n");
return ret;
@@ -293,19 +274,15 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
struct fpga_image_info *info)
{
struct spi_device *spi = mgr->priv;
- struct spi_message msg;
- struct spi_transfer tx[2];
+ struct spi_transfer tx = {};
static const u8 progdone[] = ISC_PROGRAMDONE;
static const u8 refresh[] = LSC_REFRESH;
u32 status;
int ret, refreshloop = 0;
- memset(tx, 0, sizeof(tx));
- spi_message_init(&msg);
- tx[0].tx_buf = &progdone;
- tx[0].len = sizeof(progdone);
- spi_message_add_tail(&tx[0], &msg);
- ret = spi_sync(spi, &msg);
+ tx.tx_buf = &progdone;
+ tx.len = sizeof(progdone);
+ ret = spi_sync_transfer(spi, &tx, 1);
if (ret)
goto fail;
ret = wait_until_not_busy(spi);
@@ -320,14 +297,13 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
goto fail;
}
+ tx.tx_buf = &refresh;
+ tx.len = sizeof(refresh);
+ tx.delay.value = MACHXO2_REFRESH_USEC;
+ tx.delay.unit = SPI_DELAY_UNIT_USECS;
+
do {
- spi_message_init(&msg);
- tx[1].tx_buf = &refresh;
- tx[1].len = sizeof(refresh);
- tx[1].delay.value = MACHXO2_REFRESH_USEC;
- tx[1].delay.unit = SPI_DELAY_UNIT_USECS;
- spi_message_add_tail(&tx[1], &msg);
- ret = spi_sync(spi, &msg);
+ ret = spi_sync_transfer(spi, &tx, 1);
if (ret)
goto fail;
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 09/16] fpga: machxo2-spi: simplify spi write commands
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (7 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 08/16] fpga: machxo2-spi: simplify with spi_sync_transfer() Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 10/16] fpga: machxo2-spi: prepare extraction of common code Johannes Zink
` (7 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
Refactor the spi transfer preparation into a separate function. This
commit prepares moving the non-spi-specific part of the programming
sequence into a separate file.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-spi.c | 128 +++++++++++++++++++++++--------------
1 file changed, 79 insertions(+), 49 deletions(-)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 7f7d1066ddee..d696b1cfb18a 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -65,7 +65,7 @@ static inline u8 get_err(u32 status)
static int get_status(struct spi_device *spi, u32 *status)
{
struct spi_transfer transfers[2] = {};
- static const u8 cmd[] = LSC_READ_STATUS;
+ u8 cmd[] = LSC_READ_STATUS;
__be32 tmp;
int ret;
@@ -107,7 +107,7 @@ static void dump_status_reg(u32 status)
!!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
}
-static int wait_until_not_busy(struct spi_device *spi)
+static int machxo2_wait_until_not_busy(struct spi_device *spi)
{
u32 status;
int ret, loop = 0;
@@ -123,29 +123,57 @@ static int wait_until_not_busy(struct spi_device *spi)
return 0;
}
+struct machxo2_cmd {
+ u8 *cmd;
+ size_t cmd_len;
+ u16 delay_us;
+};
+
+static int machxo2_write_spi(struct spi_device *spi, struct machxo2_cmd *cmds, size_t cmd_count)
+{
+ struct spi_transfer *transfers;
+ int i, ret;
+
+ transfers = kcalloc(cmd_count, sizeof(*transfers), GFP_KERNEL);
+ for (i = 0; i < cmd_count; i++) {
+ transfers[i].tx_buf = cmds[i].cmd;
+ transfers[i].len = cmds[i].cmd_len;
+
+ if (cmds[i].delay_us) {
+ transfers[i].delay.value = cmds[i].delay_us;
+ transfers[i].delay.unit = SPI_DELAY_UNIT_USECS;
+ }
+ }
+
+ ret = spi_sync_transfer(spi, transfers, cmd_count);
+
+ kfree(transfers);
+
+ return ret;
+}
+
static int machxo2_cleanup(struct fpga_manager *mgr)
{
struct spi_device *spi = mgr->priv;
- static const u8 erase[] = ISC_ERASE;
- static const u8 refresh[] = LSC_REFRESH;
- struct spi_transfer tx = {};
+ u8 erase[] = ISC_ERASE;
+ u8 refresh[] = LSC_REFRESH;
+ struct machxo2_cmd cmd = {};
int ret;
- tx.tx_buf = &erase;
- tx.len = sizeof(erase);
- ret = spi_sync_transfer(spi, &tx, 1);
+ cmd.cmd = erase;
+ cmd.cmd_len = sizeof(erase);
+ ret = machxo2_write_spi(spi, &cmd, 1);
if (ret)
goto fail;
- ret = wait_until_not_busy(spi);
+ ret = machxo2_wait_until_not_busy(spi);
if (ret)
goto fail;
- tx.tx_buf = &refresh;
- tx.len = sizeof(refresh);
- tx.delay.value = MACHXO2_REFRESH_USEC;
- tx.delay.unit = SPI_DELAY_UNIT_USECS;
- ret = spi_sync_transfer(spi, &tx, 1);
+ cmd.cmd = refresh;
+ cmd.cmd_len = sizeof(refresh);
+ cmd.delay_us = MACHXO2_REFRESH_USEC;
+ ret = machxo2_write_spi(spi, &cmd, 1);
if (ret)
goto fail;
@@ -179,10 +207,10 @@ static int machxo2_write_init(struct fpga_manager *mgr,
const char *buf, size_t count)
{
struct spi_device *spi = mgr->priv;
- struct spi_transfer tx[2] = {};
- static const u8 enable[] = ISC_ENABLE;
- static const u8 erase[] = ISC_ERASE;
- static const u8 initaddr[] = LSC_INITADDRESS;
+ u8 enable[] = ISC_ENABLE;
+ u8 erase[] = ISC_ERASE;
+ u8 initaddr[] = LSC_INITADDRESS;
+ struct machxo2_cmd cmd[2] = {};
u32 status;
int ret;
@@ -195,18 +223,18 @@ static int machxo2_write_init(struct fpga_manager *mgr,
get_status(spi, &status);
dump_status_reg(status);
- tx[0].tx_buf = &enable;
- tx[0].len = sizeof(enable);
- tx[0].delay.value = MACHXO2_LOW_DELAY_USEC;
- tx[0].delay.unit = SPI_DELAY_UNIT_USECS;
+ cmd[0].cmd = enable;
+ cmd[0].cmd_len = sizeof(enable);
+ cmd[0].delay_us = MACHXO2_LOW_DELAY_USEC;
+
+ cmd[1].cmd = erase;
+ cmd[1].cmd_len = sizeof(erase);
+ ret = machxo2_write_spi(spi, cmd, ARRAY_SIZE(cmd));
- tx[1].tx_buf = &erase;
- tx[1].len = sizeof(erase);
- ret = spi_sync_transfer(spi, tx, ARRAY_SIZE(tx));
if (ret)
goto fail;
- ret = wait_until_not_busy(spi);
+ ret = machxo2_wait_until_not_busy(spi);
if (ret)
goto fail;
@@ -217,9 +245,9 @@ static int machxo2_write_init(struct fpga_manager *mgr,
}
dump_status_reg(status);
- tx[0].tx_buf = &initaddr;
- tx[0].len = sizeof(initaddr);
- ret = spi_sync_transfer(spi, &tx[0], 1);
+ cmd[0].cmd = initaddr;
+ cmd[0].cmd_len = sizeof(initaddr);
+ ret = machxo2_write_spi(spi, &cmd[0], 1);
if (ret)
goto fail;
@@ -237,8 +265,9 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
size_t count)
{
struct spi_device *spi = mgr->priv;
- static const u8 progincr[] = LSC_PROGINCRNV;
+ u8 progincr[] = LSC_PROGINCRNV;
u8 payload[MACHXO2_BUF_SIZE];
+ struct machxo2_cmd cmd = {};
u32 status;
int i, ret;
@@ -248,17 +277,19 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
}
get_status(spi, &status);
dump_status_reg(status);
+
+ cmd.cmd = payload;
+ cmd.cmd_len = MACHXO2_BUF_SIZE;
+ cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
+
memcpy(payload, &progincr, sizeof(progincr));
for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
- struct spi_transfer tx = {};
-
memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
- tx.tx_buf = payload;
- tx.len = MACHXO2_BUF_SIZE;
- tx.delay.value = MACHXO2_HIGH_DELAY_USEC;
- tx.delay.unit = SPI_DELAY_UNIT_USECS;
- ret = spi_sync_transfer(spi, &tx, 1);
+ cmd.cmd = payload;
+ cmd.cmd_len = MACHXO2_BUF_SIZE;
+ cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
+ ret = machxo2_write_spi(spi, &cmd, 1);
if (ret) {
dev_err(&mgr->dev, "Error loading the bitstream.\n");
return ret;
@@ -274,18 +305,18 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
struct fpga_image_info *info)
{
struct spi_device *spi = mgr->priv;
- struct spi_transfer tx = {};
- static const u8 progdone[] = ISC_PROGRAMDONE;
- static const u8 refresh[] = LSC_REFRESH;
+ struct machxo2_cmd cmd = {};
+ u8 progdone[] = ISC_PROGRAMDONE;
+ u8 refresh[] = LSC_REFRESH;
u32 status;
int ret, refreshloop = 0;
- tx.tx_buf = &progdone;
- tx.len = sizeof(progdone);
- ret = spi_sync_transfer(spi, &tx, 1);
+ cmd.cmd = progdone;
+ cmd.cmd_len = sizeof(progdone);
+ ret = machxo2_write_spi(spi, &cmd, 1);
if (ret)
goto fail;
- ret = wait_until_not_busy(spi);
+ ret = machxo2_wait_until_not_busy(spi);
if (ret)
goto fail;
@@ -297,13 +328,12 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
goto fail;
}
- tx.tx_buf = &refresh;
- tx.len = sizeof(refresh);
- tx.delay.value = MACHXO2_REFRESH_USEC;
- tx.delay.unit = SPI_DELAY_UNIT_USECS;
+ cmd.cmd = refresh;
+ cmd.cmd_len = sizeof(refresh);
+ cmd.delay_us = MACHXO2_REFRESH_USEC;
do {
- ret = spi_sync_transfer(spi, &tx, 1);
+ ret = machxo2_write_spi(spi, &cmd, 1);
if (ret)
goto fail;
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 10/16] fpga: machxo2-spi: prepare extraction of common code
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (8 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 09/16] fpga: machxo2-spi: simplify spi write commands Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 11/16] fpga: machxo2: move non-spi-related functionality to " Johannes Zink
` (6 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
This commit introduces a machxo2_common_priv structure which is used
instead of holding the driver's spi_device directly as priv member of the
fpga_mgr structure.
Additionally it serves as a container for a machxo2_spi_priv
struct, this prepares the addition of i2c as another bus in a later commit.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-spi.c | 80 +++++++++++++++++++++++++-------------
1 file changed, 52 insertions(+), 28 deletions(-)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index d696b1cfb18a..5f1d6505f828 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/spi/spi.h>
+#include <linux/container_of.h>
/* MachXO2 Programming Guide - sysCONFIG Programming Commands */
#define IDCODE_PUB {0xe0, 0x00, 0x00, 0x00}
@@ -57,13 +58,27 @@
#define MACHXO2_ERR_ESDMEOF 7 /* SDM EOF */
#define MACHXO2_FAIL BIT(13)
+struct machxo2_common_priv {
+};
+
+struct machxo2_spi_priv {
+ struct machxo2_common_priv common;
+ struct spi_device *spi;
+};
+
+static inline struct machxo2_spi_priv *to_machxo2_spi_priv(struct machxo2_common_priv *priv)
+{
+ return container_of(priv, struct machxo2_spi_priv, common);
+}
+
static inline u8 get_err(u32 status)
{
return FIELD_GET(MACHXO2_ERR, status);
}
-static int get_status(struct spi_device *spi, u32 *status)
+static int get_status(struct machxo2_common_priv *priv, u32 *status)
{
+ struct spi_device *spi = to_machxo2_spi_priv(priv)->spi;
struct spi_transfer transfers[2] = {};
u8 cmd[] = LSC_READ_STATUS;
__be32 tmp;
@@ -107,13 +122,13 @@ static void dump_status_reg(u32 status)
!!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
}
-static int machxo2_wait_until_not_busy(struct spi_device *spi)
+static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
{
u32 status;
int ret, loop = 0;
do {
- ret = get_status(spi, &status);
+ ret = get_status(priv, &status);
if (ret)
return ret;
if (++loop >= MACHXO2_MAX_BUSY_LOOP)
@@ -129,8 +144,10 @@ struct machxo2_cmd {
u16 delay_us;
};
-static int machxo2_write_spi(struct spi_device *spi, struct machxo2_cmd *cmds, size_t cmd_count)
+static int machxo2_write_spi(struct machxo2_common_priv *priv,
+ struct machxo2_cmd *cmds, size_t cmd_count)
{
+ struct spi_device *spi = to_machxo2_spi_priv(priv)->spi;
struct spi_transfer *transfers;
int i, ret;
@@ -154,7 +171,7 @@ static int machxo2_write_spi(struct spi_device *spi, struct machxo2_cmd *cmds, s
static int machxo2_cleanup(struct fpga_manager *mgr)
{
- struct spi_device *spi = mgr->priv;
+ struct machxo2_common_priv *priv = mgr->priv;
u8 erase[] = ISC_ERASE;
u8 refresh[] = LSC_REFRESH;
struct machxo2_cmd cmd = {};
@@ -162,18 +179,18 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
cmd.cmd = erase;
cmd.cmd_len = sizeof(erase);
- ret = machxo2_write_spi(spi, &cmd, 1);
+ ret = machxo2_write_spi(priv, &cmd, 1);
if (ret)
goto fail;
- ret = machxo2_wait_until_not_busy(spi);
+ ret = machxo2_wait_until_not_busy(priv);
if (ret)
goto fail;
cmd.cmd = refresh;
cmd.cmd_len = sizeof(refresh);
cmd.delay_us = MACHXO2_REFRESH_USEC;
- ret = machxo2_write_spi(spi, &cmd, 1);
+ ret = machxo2_write_spi(priv, &cmd, 1);
if (ret)
goto fail;
@@ -192,10 +209,10 @@ static bool machxo2_status_done(unsigned long status)
static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager *mgr)
{
- struct spi_device *spi = mgr->priv;
+ struct machxo2_common_priv *priv = mgr->priv;
u32 status;
- get_status(spi, &status);
+ get_status(priv, &status);
if (machxo2_status_done(status))
return FPGA_MGR_STATE_OPERATING;
@@ -206,7 +223,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
struct fpga_image_info *info,
const char *buf, size_t count)
{
- struct spi_device *spi = mgr->priv;
+ struct machxo2_common_priv *priv = mgr->priv;
u8 enable[] = ISC_ENABLE;
u8 erase[] = ISC_ERASE;
u8 initaddr[] = LSC_INITADDRESS;
@@ -220,7 +237,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
return -ENOTSUPP;
}
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
cmd[0].cmd = enable;
@@ -234,11 +251,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
if (ret)
goto fail;
- ret = machxo2_wait_until_not_busy(spi);
+ ret = machxo2_wait_until_not_busy(priv);
if (ret)
goto fail;
- get_status(spi, &status);
+ get_status(priv, &status);
if (status & MACHXO2_FAIL) {
ret = -EINVAL;
goto fail;
@@ -247,11 +264,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
cmd[0].cmd = initaddr;
cmd[0].cmd_len = sizeof(initaddr);
- ret = machxo2_write_spi(spi, &cmd[0], 1);
+ ret = machxo2_write_spi(priv, &cmd[0], 1);
if (ret)
goto fail;
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
return 0;
@@ -264,7 +281,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
static int machxo2_write(struct fpga_manager *mgr, const char *buf,
size_t count)
{
- struct spi_device *spi = mgr->priv;
+ struct machxo2_common_priv *priv = mgr->priv;
u8 progincr[] = LSC_PROGINCRNV;
u8 payload[MACHXO2_BUF_SIZE];
struct machxo2_cmd cmd = {};
@@ -275,7 +292,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
dev_err(&mgr->dev, "Malformed payload.\n");
return -EINVAL;
}
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
cmd.cmd = payload;
@@ -289,13 +306,13 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
cmd.cmd = payload;
cmd.cmd_len = MACHXO2_BUF_SIZE;
cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
- ret = machxo2_write_spi(spi, &cmd, 1);
+ ret = machxo2_write_spi(priv, &cmd, 1);
if (ret) {
dev_err(&mgr->dev, "Error loading the bitstream.\n");
return ret;
}
}
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
return 0;
@@ -304,7 +321,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
static int machxo2_write_complete(struct fpga_manager *mgr,
struct fpga_image_info *info)
{
- struct spi_device *spi = mgr->priv;
+ struct machxo2_common_priv *priv = mgr->priv;
struct machxo2_cmd cmd = {};
u8 progdone[] = ISC_PROGRAMDONE;
u8 refresh[] = LSC_REFRESH;
@@ -313,14 +330,14 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
cmd.cmd = progdone;
cmd.cmd_len = sizeof(progdone);
- ret = machxo2_write_spi(spi, &cmd, 1);
+ ret = machxo2_write_spi(priv, &cmd, 1);
if (ret)
goto fail;
- ret = machxo2_wait_until_not_busy(spi);
+ ret = machxo2_wait_until_not_busy(priv);
if (ret)
goto fail;
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
if (!(status & MACHXO2_DONE)) {
machxo2_cleanup(mgr);
@@ -333,12 +350,12 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
cmd.delay_us = MACHXO2_REFRESH_USEC;
do {
- ret = machxo2_write_spi(spi, &cmd, 1);
+ ret = machxo2_write_spi(priv, &cmd, 1);
if (ret)
goto fail;
/* check refresh status */
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
if (machxo2_status_done(status))
break;
@@ -349,7 +366,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
}
} while (1);
- get_status(spi, &status);
+ get_status(priv, &status);
dump_status_reg(status);
return 0;
@@ -368,6 +385,7 @@ static const struct fpga_manager_ops machxo2_ops = {
static int machxo2_spi_probe(struct spi_device *spi)
{
+ struct machxo2_spi_priv *priv;
struct device *dev = &spi->dev;
struct fpga_manager *mgr;
@@ -376,8 +394,14 @@ static int machxo2_spi_probe(struct spi_device *spi)
return -EINVAL;
}
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->spi = spi;
+
mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
- &machxo2_ops, spi);
+ &machxo2_ops, &priv->common);
return PTR_ERR_OR_ZERO(mgr);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 11/16] fpga: machxo2: move non-spi-related functionality to common code
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (9 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 10/16] fpga: machxo2-spi: prepare extraction of common code Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 12/16] fpga: machxo2: improve status register dump Johannes Zink
` (5 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
This commit seperates the general programming algorithm from
bus-specific access functions. This is a preparation for adding i2c as
another bus.
While at it: rename some functions to allow easier separation between
spi-specific functions and common code.
While at it: clean up includes
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/Kconfig | 6 +
drivers/fpga/Makefile | 1 +
drivers/fpga/machxo2-common.c | 310 +++++++++++++++++++++++++++++++++
drivers/fpga/machxo2-common.h | 37 ++++
drivers/fpga/machxo2-spi.c | 318 +---------------------------------
5 files changed, 359 insertions(+), 313 deletions(-)
create mode 100644 drivers/fpga/machxo2-common.c
create mode 100644 drivers/fpga/machxo2-common.h
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 6c416955da53..e5869a732246 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -77,9 +77,15 @@ config FPGA_MGR_ICE40_SPI
help
FPGA manager driver support for Lattice iCE40 FPGAs over SPI.
+config FPGA_MGR_MACHXO2_COMMON
+ tristate
+ help
+ FPGA manager driver common code for Lattice MachXO2 configuration
+
config FPGA_MGR_MACHXO2_SPI
tristate "Lattice MachXO2 SPI"
depends on SPI
+ select FPGA_MGR_MACHXO2_COMMON
help
FPGA manager driver support for Lattice MachXO2 configuration
over slave SPI interface.
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 42ae8b58abce..f247a8de83ad 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_FPGA) += fpga-mgr.o
obj-$(CONFIG_FPGA_MGR_ALTERA_CVP) += altera-cvp.o
obj-$(CONFIG_FPGA_MGR_ALTERA_PS_SPI) += altera-ps-spi.o
obj-$(CONFIG_FPGA_MGR_ICE40_SPI) += ice40-spi.o
+obj-$(CONFIG_FPGA_MGR_MACHXO2_COMMON) += machxo2-common.o
obj-$(CONFIG_FPGA_MGR_MACHXO2_SPI) += machxo2-spi.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o
diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
new file mode 100644
index 000000000000..33127ee67d19
--- /dev/null
+++ b/drivers/fpga/machxo2-common.c
@@ -0,0 +1,310 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Lattice MachXO2 Common Driver
+ *
+ * Manage Lattice FPGA firmware
+ *
+ * Copyright (C) 2018 Paolo Pisati <p.pisati@gmail.com>
+ * Copyright (C) 2022 Pengutronix, Johannes Zink <kernel@pengutronix.de>
+ */
+
+#include <linux/delay.h>
+#include <linux/bitfield.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include "machxo2-common.h"
+
+#define MACHXO2_LOW_DELAY_USEC 5
+#define MACHXO2_HIGH_DELAY_USEC 200
+#define MACHXO2_REFRESH_USEC 4800
+#define MACHXO2_MAX_BUSY_LOOP 128
+#define MACHXO2_MAX_REFRESH_LOOP 16
+
+#define MACHXO2_PAGE_SIZE 16
+#define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
+
+/* Status register bits, errors and error mask */
+#define MACHXO2_BUSY BIT(12)
+#define MACHXO2_DONE BIT(8)
+#define MACHXO2_DVER BIT(27)
+#define MACHXO2_ENAB BIT(9)
+#define MACHXO2_ERR GENMASK(25, 23)
+#define MACHXO2_ERR_ENOERR 0 /* no error */
+#define MACHXO2_ERR_EEID 1
+#define MACHXO2_ERR_EECMD 2
+#define MACHXO2_ERR_EECRC 3
+#define MACHXO2_ERR_EEPREAM 4 /* preamble error */
+#define MACHXO2_ERR_EEABRT 5 /* abort error */
+#define MACHXO2_ERR_EEOVERFL 6 /* overflow error */
+#define MACHXO2_ERR_EESDMEOF 7 /* SDM EOF */
+#define MACHXO2_FAIL BIT(13)
+
+
+static inline u8 get_err(u32 status)
+{
+ return FIELD_GET(MACHXO2_ERR, status);
+}
+
+static const char *get_err_string(u8 err)
+{
+ switch (err) {
+ case MACHXO2_ERR_ENOERR: return "No Error";
+ case MACHXO2_ERR_EEID: return "ID ERR";
+ case MACHXO2_ERR_EECMD: return "CMD ERR";
+ case MACHXO2_ERR_EECRC: return "CRC ERR";
+ case MACHXO2_ERR_EEPREAM: return "Preamble ERR";
+ case MACHXO2_ERR_EEABRT: return "Abort ERR";
+ case MACHXO2_ERR_EEOVERFL: return "Overflow ERR";
+ case MACHXO2_ERR_EESDMEOF: return "SDM EOF";
+ }
+
+ return "Unknown";
+}
+
+static void dump_status_reg(u32 status)
+{
+ pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
+ status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
+ !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
+ !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
+}
+
+static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
+{
+ u32 status;
+ int ret, loop = 0;
+
+ do {
+ ret = priv->get_status(priv, &status);
+ if (ret)
+ return ret;
+ if (++loop >= MACHXO2_MAX_BUSY_LOOP)
+ return -EBUSY;
+ } while (status & MACHXO2_BUSY);
+
+ return 0;
+}
+
+static int machxo2_cleanup(struct fpga_manager *mgr)
+{
+ struct machxo2_common_priv *priv = mgr->priv;
+ u8 erase[] = ISC_ERASE;
+ u8 refresh[] = LSC_REFRESH;
+ struct machxo2_cmd cmd = {};
+ int ret;
+
+ cmd.cmd = erase;
+ cmd.cmd_len = sizeof(erase);
+ ret = priv->write_commands(priv, &cmd, 1);
+ if (ret)
+ goto fail;
+
+ ret = machxo2_wait_until_not_busy(priv);
+ if (ret)
+ goto fail;
+
+ cmd.cmd = refresh;
+ cmd.cmd_len = sizeof(refresh);
+ cmd.delay_us = MACHXO2_REFRESH_USEC;
+ ret = priv->write_commands(priv, &cmd, 1);
+ if (ret)
+ goto fail;
+
+ return 0;
+fail:
+ dev_err(&mgr->dev, "Cleanup failed\n");
+
+ return ret;
+}
+
+static bool machxo2_status_done(unsigned long status)
+{
+ return (((status & (MACHXO2_BUSY | MACHXO2_DONE)) == MACHXO2_DONE) &&
+ get_err(status) == MACHXO2_ERR_ENOERR);
+}
+
+static enum fpga_mgr_states machxo2_state(struct fpga_manager *mgr)
+{
+ struct machxo2_common_priv *priv = mgr->priv;
+ u32 status;
+
+ priv->get_status(priv, &status);
+ if (machxo2_status_done(status))
+ return FPGA_MGR_STATE_OPERATING;
+
+ return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int machxo2_write_init(struct fpga_manager *mgr,
+ struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct machxo2_common_priv *priv = mgr->priv;
+ u8 enable[] = ISC_ENABLE;
+ u8 erase[] = ISC_ERASE;
+ u8 initaddr[] = LSC_INITADDRESS;
+ struct machxo2_cmd cmd[2] = {};
+ u32 status;
+ int ret;
+
+ if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
+ dev_err(&mgr->dev,
+ "Partial reconfiguration is not supported\n");
+ return -ENOTSUPP;
+ }
+
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+
+ cmd[0].cmd = enable;
+ cmd[0].cmd_len = sizeof(enable);
+ cmd[0].delay_us = MACHXO2_LOW_DELAY_USEC;
+
+ cmd[1].cmd = erase;
+ cmd[1].cmd_len = sizeof(erase);
+ ret = priv->write_commands(priv, cmd, 2);
+ if (ret)
+ goto fail;
+
+ ret = machxo2_wait_until_not_busy(priv);
+ if (ret)
+ goto fail;
+
+ priv->get_status(priv, &status);
+ if (status & MACHXO2_FAIL) {
+ ret = -EINVAL;
+ goto fail;
+ }
+ dump_status_reg(status);
+
+ cmd[0].cmd = initaddr;
+ cmd[0].cmd_len = sizeof(initaddr);
+ ret = priv->write_commands(priv, cmd, 1);
+ if (ret)
+ goto fail;
+
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+
+ return 0;
+fail:
+ dev_err(&mgr->dev, "Error during FPGA init.\n");
+
+ return ret;
+}
+
+static int machxo2_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ struct machxo2_common_priv *priv = mgr->priv;
+ u8 progincr[] = LSC_PROGINCRNV;
+ u8 payload[MACHXO2_BUF_SIZE];
+ struct machxo2_cmd cmd = {};
+ u32 status;
+ int i, ret;
+
+ if (count % MACHXO2_PAGE_SIZE != 0) {
+ dev_err(&mgr->dev, "Malformed payload.\n");
+ return -EINVAL;
+ }
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+ cmd.cmd = payload;
+ cmd.cmd_len = MACHXO2_BUF_SIZE;
+ cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
+
+ memcpy(payload, &progincr, sizeof(progincr));
+ for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
+ memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
+
+ cmd.cmd = payload;
+ cmd.cmd_len = MACHXO2_BUF_SIZE;
+ cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
+ ret = priv->write_commands(priv, &cmd, 1);
+ if (ret) {
+ dev_err(&mgr->dev, "Error loading the bitstream.\n");
+ return ret;
+ }
+ }
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+
+ return 0;
+}
+
+static int machxo2_write_complete(struct fpga_manager *mgr,
+ struct fpga_image_info *info)
+{
+ struct machxo2_common_priv *priv = mgr->priv;
+ struct machxo2_cmd cmd = {};
+ u8 progdone[] = ISC_PROGRAMDONE;
+ u8 refresh[] = LSC_REFRESH;
+ u32 status;
+ int ret, refreshloop = 0;
+
+ cmd.cmd = progdone;
+ cmd.cmd_len = sizeof(progdone);
+ ret = priv->write_commands(priv, &cmd, 1);
+ if (ret)
+ goto fail;
+ ret = machxo2_wait_until_not_busy(priv);
+ if (ret)
+ goto fail;
+
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+ if (!(status & MACHXO2_DONE)) {
+ machxo2_cleanup(mgr);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ cmd.cmd = refresh;
+ cmd.cmd_len = sizeof(refresh);
+ cmd.delay_us = MACHXO2_REFRESH_USEC;
+
+ do {
+ ret = priv->write_commands(priv, &cmd, 1);
+ if (ret)
+ goto fail;
+
+ /* check refresh status */
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+ if (machxo2_status_done(status))
+ break;
+ if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
+ machxo2_cleanup(mgr);
+ ret = -EINVAL;
+ goto fail;
+ }
+ } while (1);
+
+ priv->get_status(priv, &status);
+ dump_status_reg(status);
+
+ return 0;
+fail:
+ dev_err(&mgr->dev, "Refresh failed.\n");
+
+ return ret;
+}
+
+static const struct fpga_manager_ops machxo2_ops = {
+ .state = machxo2_state,
+ .write_init = machxo2_write_init,
+ .write = machxo2_write,
+ .write_complete = machxo2_write_complete,
+};
+
+int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev)
+{
+ struct fpga_manager *mgr;
+
+ mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
+ &machxo2_ops, priv);
+
+ return PTR_ERR_OR_ZERO(mgr);
+}
diff --git a/drivers/fpga/machxo2-common.h b/drivers/fpga/machxo2-common.h
new file mode 100644
index 000000000000..908203644209
--- /dev/null
+++ b/drivers/fpga/machxo2-common.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Paolo Pisati <p.pisati@gmail.com>
+ * Copyright (C) 2021 Peter Jensen <pdj@bang-olufsen.dk>
+ * Copyright (C) 2022 Pengutronix, Johannes Zink <kernel@pengutronix.de>
+ */
+
+#ifndef __LINUX_FPGA_MGR_MACHXO2_COMMON_H
+#define __LINUX_FPGA_MGR_MACHXO2_COMMON_H
+
+#include <linux/types.h>
+
+/* MachXO2 Programming Guide - sysCONFIG Programming Commands */
+#define IDCODE_PUB {0xe0, 0x00, 0x00, 0x00}
+#define ISC_ENABLE {0xc6, 0x08, 0x00, 0x00}
+#define ISC_ERASE {0x0e, 0x04, 0x00, 0x00}
+#define ISC_PROGRAMDONE {0x5e, 0x00, 0x00, 0x00}
+#define LSC_INITADDRESS {0x46, 0x00, 0x00, 0x00}
+#define LSC_PROGINCRNV {0x70, 0x00, 0x00, 0x01}
+#define LSC_READ_STATUS {0x3c, 0x00, 0x00, 0x00}
+#define LSC_REFRESH {0x79, 0x00, 0x00, 0x00}
+
+struct machxo2_cmd {
+ u8 *cmd;
+ size_t cmd_len;
+ u16 delay_us;
+};
+
+struct machxo2_common_priv {
+ int (*write_commands)(struct machxo2_common_priv *priv,
+ struct machxo2_cmd *cmds, size_t cmd_count);
+ int (*get_status)(struct machxo2_common_priv *priv, u32 *status);
+};
+
+int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev);
+
+#endif
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 5f1d6505f828..30965a3c293e 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -8,24 +8,10 @@
* Copyright (C) 2018 Paolo Pisati <p.pisati@gmail.com>
*/
-#include <linux/delay.h>
-#include <linux/bitfield.h>
-#include <linux/fpga/fpga-mgr.h>
-#include <linux/gpio/consumer.h>
#include <linux/module.h>
-#include <linux/of.h>
#include <linux/spi/spi.h>
#include <linux/container_of.h>
-
-/* MachXO2 Programming Guide - sysCONFIG Programming Commands */
-#define IDCODE_PUB {0xe0, 0x00, 0x00, 0x00}
-#define ISC_ENABLE {0xc6, 0x08, 0x00, 0x00}
-#define ISC_ERASE {0x0e, 0x04, 0x00, 0x00}
-#define ISC_PROGRAMDONE {0x5e, 0x00, 0x00, 0x00}
-#define LSC_INITADDRESS {0x46, 0x00, 0x00, 0x00}
-#define LSC_PROGINCRNV {0x70, 0x00, 0x00, 0x01}
-#define LSC_READ_STATUS {0x3c, 0x00, 0x00, 0x00}
-#define LSC_REFRESH {0x79, 0x00, 0x00, 0x00}
+#include "machxo2-common.h"
/*
* Max CCLK in Slave SPI mode according to 'MachXO2 Family Data
@@ -33,34 +19,6 @@
*/
#define MACHXO2_MAX_SPEED 66000000
-#define MACHXO2_LOW_DELAY_USEC 5
-#define MACHXO2_HIGH_DELAY_USEC 200
-#define MACHXO2_REFRESH_USEC 4800
-#define MACHXO2_MAX_BUSY_LOOP 128
-#define MACHXO2_MAX_REFRESH_LOOP 16
-
-#define MACHXO2_PAGE_SIZE 16
-#define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
-
-/* Status register bits, errors and error mask */
-#define MACHXO2_BUSY BIT(12)
-#define MACHXO2_DONE BIT(8)
-#define MACHXO2_DVER BIT(27)
-#define MACHXO2_ENAB BIT(9)
-#define MACHXO2_ERR GENMASK(25, 23)
-#define MACHXO2_ERR_ENOERR 0 /* no error */
-#define MACHXO2_ERR_EID 1
-#define MACHXO2_ERR_ECMD 2
-#define MACHXO2_ERR_ECRC 3
-#define MACHXO2_ERR_EPREAM 4 /* preamble error */
-#define MACHXO2_ERR_EABRT 5 /* abort error */
-#define MACHXO2_ERR_EOVERFL 6 /* overflow error */
-#define MACHXO2_ERR_ESDMEOF 7 /* SDM EOF */
-#define MACHXO2_FAIL BIT(13)
-
-struct machxo2_common_priv {
-};
-
struct machxo2_spi_priv {
struct machxo2_common_priv common;
struct spi_device *spi;
@@ -71,12 +29,7 @@ static inline struct machxo2_spi_priv *to_machxo2_spi_priv(struct machxo2_common
return container_of(priv, struct machxo2_spi_priv, common);
}
-static inline u8 get_err(u32 status)
-{
- return FIELD_GET(MACHXO2_ERR, status);
-}
-
-static int get_status(struct machxo2_common_priv *priv, u32 *status)
+static int machxo2_get_status_spi(struct machxo2_common_priv *priv, u32 *status)
{
struct spi_device *spi = to_machxo2_spi_priv(priv)->spi;
struct spi_transfer transfers[2] = {};
@@ -98,52 +51,6 @@ static int get_status(struct machxo2_common_priv *priv, u32 *status)
return 0;
}
-static const char *get_err_string(u8 err)
-{
- switch (err) {
- case MACHXO2_ERR_ENOERR: return "No Error";
- case MACHXO2_ERR_EID: return "ID ERR";
- case MACHXO2_ERR_ECMD: return "CMD ERR";
- case MACHXO2_ERR_ECRC: return "CRC ERR";
- case MACHXO2_ERR_EPREAM: return "Preamble ERR";
- case MACHXO2_ERR_EABRT: return "Abort ERR";
- case MACHXO2_ERR_EOVERFL: return "Overflow ERR";
- case MACHXO2_ERR_ESDMEOF: return "SDM EOF";
- }
-
- return "Unknown";
-}
-
-static void dump_status_reg(u32 status)
-{
- pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
- status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
- !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
- !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
-}
-
-static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
-{
- u32 status;
- int ret, loop = 0;
-
- do {
- ret = get_status(priv, &status);
- if (ret)
- return ret;
- if (++loop >= MACHXO2_MAX_BUSY_LOOP)
- return -EBUSY;
- } while (status & MACHXO2_BUSY);
-
- return 0;
-}
-
-struct machxo2_cmd {
- u8 *cmd;
- size_t cmd_len;
- u16 delay_us;
-};
-
static int machxo2_write_spi(struct machxo2_common_priv *priv,
struct machxo2_cmd *cmds, size_t cmd_count)
{
@@ -169,225 +76,10 @@ static int machxo2_write_spi(struct machxo2_common_priv *priv,
return ret;
}
-static int machxo2_cleanup(struct fpga_manager *mgr)
-{
- struct machxo2_common_priv *priv = mgr->priv;
- u8 erase[] = ISC_ERASE;
- u8 refresh[] = LSC_REFRESH;
- struct machxo2_cmd cmd = {};
- int ret;
-
- cmd.cmd = erase;
- cmd.cmd_len = sizeof(erase);
- ret = machxo2_write_spi(priv, &cmd, 1);
- if (ret)
- goto fail;
-
- ret = machxo2_wait_until_not_busy(priv);
- if (ret)
- goto fail;
-
- cmd.cmd = refresh;
- cmd.cmd_len = sizeof(refresh);
- cmd.delay_us = MACHXO2_REFRESH_USEC;
- ret = machxo2_write_spi(priv, &cmd, 1);
- if (ret)
- goto fail;
-
- return 0;
-fail:
- dev_err(&mgr->dev, "Cleanup failed\n");
-
- return ret;
-}
-
-static bool machxo2_status_done(unsigned long status)
-{
- return (((status & (MACHXO2_BUSY | MACHXO2_DONE)) == MACHXO2_DONE) &&
- get_err(status) == MACHXO2_ERR_ENOERR);
-}
-
-static enum fpga_mgr_states machxo2_spi_state(struct fpga_manager *mgr)
-{
- struct machxo2_common_priv *priv = mgr->priv;
- u32 status;
-
- get_status(priv, &status);
- if (machxo2_status_done(status))
- return FPGA_MGR_STATE_OPERATING;
-
- return FPGA_MGR_STATE_UNKNOWN;
-}
-
-static int machxo2_write_init(struct fpga_manager *mgr,
- struct fpga_image_info *info,
- const char *buf, size_t count)
-{
- struct machxo2_common_priv *priv = mgr->priv;
- u8 enable[] = ISC_ENABLE;
- u8 erase[] = ISC_ERASE;
- u8 initaddr[] = LSC_INITADDRESS;
- struct machxo2_cmd cmd[2] = {};
- u32 status;
- int ret;
-
- if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
- dev_err(&mgr->dev,
- "Partial reconfiguration is not supported\n");
- return -ENOTSUPP;
- }
-
- get_status(priv, &status);
- dump_status_reg(status);
-
- cmd[0].cmd = enable;
- cmd[0].cmd_len = sizeof(enable);
- cmd[0].delay_us = MACHXO2_LOW_DELAY_USEC;
-
- cmd[1].cmd = erase;
- cmd[1].cmd_len = sizeof(erase);
- ret = machxo2_write_spi(spi, cmd, ARRAY_SIZE(cmd));
-
- if (ret)
- goto fail;
-
- ret = machxo2_wait_until_not_busy(priv);
- if (ret)
- goto fail;
-
- get_status(priv, &status);
- if (status & MACHXO2_FAIL) {
- ret = -EINVAL;
- goto fail;
- }
- dump_status_reg(status);
-
- cmd[0].cmd = initaddr;
- cmd[0].cmd_len = sizeof(initaddr);
- ret = machxo2_write_spi(priv, &cmd[0], 1);
- if (ret)
- goto fail;
-
- get_status(priv, &status);
- dump_status_reg(status);
-
- return 0;
-fail:
- dev_err(&mgr->dev, "Error during FPGA init.\n");
-
- return ret;
-}
-
-static int machxo2_write(struct fpga_manager *mgr, const char *buf,
- size_t count)
-{
- struct machxo2_common_priv *priv = mgr->priv;
- u8 progincr[] = LSC_PROGINCRNV;
- u8 payload[MACHXO2_BUF_SIZE];
- struct machxo2_cmd cmd = {};
- u32 status;
- int i, ret;
-
- if (count % MACHXO2_PAGE_SIZE != 0) {
- dev_err(&mgr->dev, "Malformed payload.\n");
- return -EINVAL;
- }
- get_status(priv, &status);
- dump_status_reg(status);
-
- cmd.cmd = payload;
- cmd.cmd_len = MACHXO2_BUF_SIZE;
- cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
-
- memcpy(payload, &progincr, sizeof(progincr));
- for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
- memcpy(&payload[sizeof(progincr)], &buf[i], MACHXO2_PAGE_SIZE);
-
- cmd.cmd = payload;
- cmd.cmd_len = MACHXO2_BUF_SIZE;
- cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
- ret = machxo2_write_spi(priv, &cmd, 1);
- if (ret) {
- dev_err(&mgr->dev, "Error loading the bitstream.\n");
- return ret;
- }
- }
- get_status(priv, &status);
- dump_status_reg(status);
-
- return 0;
-}
-
-static int machxo2_write_complete(struct fpga_manager *mgr,
- struct fpga_image_info *info)
-{
- struct machxo2_common_priv *priv = mgr->priv;
- struct machxo2_cmd cmd = {};
- u8 progdone[] = ISC_PROGRAMDONE;
- u8 refresh[] = LSC_REFRESH;
- u32 status;
- int ret, refreshloop = 0;
-
- cmd.cmd = progdone;
- cmd.cmd_len = sizeof(progdone);
- ret = machxo2_write_spi(priv, &cmd, 1);
- if (ret)
- goto fail;
- ret = machxo2_wait_until_not_busy(priv);
- if (ret)
- goto fail;
-
- get_status(priv, &status);
- dump_status_reg(status);
- if (!(status & MACHXO2_DONE)) {
- machxo2_cleanup(mgr);
- ret = -EINVAL;
- goto fail;
- }
-
- cmd.cmd = refresh;
- cmd.cmd_len = sizeof(refresh);
- cmd.delay_us = MACHXO2_REFRESH_USEC;
-
- do {
- ret = machxo2_write_spi(priv, &cmd, 1);
- if (ret)
- goto fail;
-
- /* check refresh status */
- get_status(priv, &status);
- dump_status_reg(status);
- if (machxo2_status_done(status))
- break;
- if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
- machxo2_cleanup(mgr);
- ret = -EINVAL;
- goto fail;
- }
- } while (1);
-
- get_status(priv, &status);
- dump_status_reg(status);
-
- return 0;
-fail:
- dev_err(&mgr->dev, "Refresh failed.\n");
-
- return ret;
-}
-
-static const struct fpga_manager_ops machxo2_ops = {
- .state = machxo2_spi_state,
- .write_init = machxo2_write_init,
- .write = machxo2_write,
- .write_complete = machxo2_write_complete,
-};
-
static int machxo2_spi_probe(struct spi_device *spi)
{
struct machxo2_spi_priv *priv;
struct device *dev = &spi->dev;
- struct fpga_manager *mgr;
if (spi->max_speed_hz > MACHXO2_MAX_SPEED) {
dev_err(dev, "Speed is too high\n");
@@ -399,10 +91,10 @@ static int machxo2_spi_probe(struct spi_device *spi)
return -ENOMEM;
priv->spi = spi;
+ priv->common.write_commands = machxo2_write_spi;
+ priv->common.get_status = machxo2_get_status_spi;
- mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
- &machxo2_ops, &priv->common);
- return PTR_ERR_OR_ZERO(mgr);
+ return machxo2_common_init(&(priv->common), dev);
}
#ifdef CONFIG_OF
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 12/16] fpga: machxo2: improve status register dump
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (10 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 11/16] fpga: machxo2: move non-spi-related functionality to " Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 13/16] fpga: machxo2: add optional additional flash areas to be erased Johannes Zink
` (4 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
Since the previous patches introduce a priv datastructure, the dev
pointer can now be stored upon init. This allows the dump_status_reg
function to use dev_dbg instead of a raw pr_debug.
No functional changes.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-common.c | 28 +++++++++++++++-------------
drivers/fpga/machxo2-common.h | 1 +
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
index 33127ee67d19..71f886a60cba 100644
--- a/drivers/fpga/machxo2-common.c
+++ b/drivers/fpga/machxo2-common.c
@@ -63,12 +63,12 @@ static const char *get_err_string(u8 err)
return "Unknown";
}
-static void dump_status_reg(u32 status)
+static void dump_status_reg(struct device *dev, u32 status)
{
- pr_debug("machxo2 status: 0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
- status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
- !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
- !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
+ dev_dbg(dev, "0x%08X - done=%d, cfgena=%d, busy=%d, fail=%d, devver=%d, err=%s\n",
+ status, !!FIELD_GET(MACHXO2_DONE, status), !!FIELD_GET(MACHXO2_ENAB, status),
+ !!FIELD_GET(MACHXO2_BUSY, status), !!FIELD_GET(MACHXO2_FAIL, status),
+ !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
}
static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
@@ -156,7 +156,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
}
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
cmd[0].cmd = enable;
cmd[0].cmd_len = sizeof(enable);
@@ -177,7 +177,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
ret = -EINVAL;
goto fail;
}
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
cmd[0].cmd = initaddr;
cmd[0].cmd_len = sizeof(initaddr);
@@ -186,7 +186,7 @@ static int machxo2_write_init(struct fpga_manager *mgr,
goto fail;
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
return 0;
fail:
@@ -210,7 +210,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
return -EINVAL;
}
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
cmd.cmd = payload;
cmd.cmd_len = MACHXO2_BUF_SIZE;
cmd.delay_us = MACHXO2_HIGH_DELAY_USEC;
@@ -229,7 +229,7 @@ static int machxo2_write(struct fpga_manager *mgr, const char *buf,
}
}
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
return 0;
}
@@ -254,7 +254,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
goto fail;
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
if (!(status & MACHXO2_DONE)) {
machxo2_cleanup(mgr);
ret = -EINVAL;
@@ -272,7 +272,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
/* check refresh status */
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
if (machxo2_status_done(status))
break;
if (++refreshloop == MACHXO2_MAX_REFRESH_LOOP) {
@@ -283,7 +283,7 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
} while (1);
priv->get_status(priv, &status);
- dump_status_reg(status);
+ dump_status_reg(priv->dev, status);
return 0;
fail:
@@ -303,6 +303,8 @@ int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev)
{
struct fpga_manager *mgr;
+ priv->dev = dev;
+
mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
&machxo2_ops, priv);
diff --git a/drivers/fpga/machxo2-common.h b/drivers/fpga/machxo2-common.h
index 908203644209..a16060602a3f 100644
--- a/drivers/fpga/machxo2-common.h
+++ b/drivers/fpga/machxo2-common.h
@@ -30,6 +30,7 @@ struct machxo2_common_priv {
int (*write_commands)(struct machxo2_common_priv *priv,
struct machxo2_cmd *cmds, size_t cmd_count);
int (*get_status)(struct machxo2_common_priv *priv, u32 *status);
+ struct device *dev;
};
int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 13/16] fpga: machxo2: add optional additional flash areas to be erased
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (11 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 12/16] fpga: machxo2: improve status register dump Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 14/16] fpga: machxo2: add program initialization signalling via gpio Johannes Zink
` (3 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
This patch allows additional flash areas to be erased, i.e. not only the
configuration flash, but also sram, feature row and UFM (user flash
memory) can be erased.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-common.c | 40 +++++++++++++++++++++++++++++------
drivers/fpga/machxo2-common.h | 1 +
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
index 71f886a60cba..d93c304cceb9 100644
--- a/drivers/fpga/machxo2-common.c
+++ b/drivers/fpga/machxo2-common.c
@@ -10,10 +10,13 @@
#include <linux/delay.h>
#include <linux/bitfield.h>
+#include <linux/byteorder/generic.h>
+#include <asm-generic/unaligned.h>
#include <linux/fpga/fpga-mgr.h>
#include <linux/gpio/consumer.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/property.h>
#include "machxo2-common.h"
#define MACHXO2_LOW_DELAY_USEC 5
@@ -41,6 +44,16 @@
#define MACHXO2_ERR_EESDMEOF 7 /* SDM EOF */
#define MACHXO2_FAIL BIT(13)
+/*
+ * second byte ('operand') of ISC_ERASE can be ORed with the
+ * following bitmasks to not only erase configuration flash,
+ * but also SRAM, Feature Row or UFM, respectively. See MachXO2
+ * Programming and Configuration Usage Guide
+ */
+#define ISC_ERASE_SRAM BIT(16)
+#define ISC_ERASE_FEATURE_ROW BIT(17)
+#define ISC_ERASE_UFM BIT(19)
+
static inline u8 get_err(u32 status)
{
@@ -90,13 +103,13 @@ static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
static int machxo2_cleanup(struct fpga_manager *mgr)
{
struct machxo2_common_priv *priv = mgr->priv;
- u8 erase[] = ISC_ERASE;
u8 refresh[] = LSC_REFRESH;
struct machxo2_cmd cmd = {};
int ret;
- cmd.cmd = erase;
- cmd.cmd_len = sizeof(erase);
+ cmd.cmd = (u8 *)&priv->erase_cmd;
+ cmd.cmd_len = sizeof(priv->erase_cmd);
+
ret = priv->write_commands(priv, &cmd, 1);
if (ret)
goto fail;
@@ -143,7 +156,6 @@ static int machxo2_write_init(struct fpga_manager *mgr,
{
struct machxo2_common_priv *priv = mgr->priv;
u8 enable[] = ISC_ENABLE;
- u8 erase[] = ISC_ERASE;
u8 initaddr[] = LSC_INITADDRESS;
struct machxo2_cmd cmd[2] = {};
u32 status;
@@ -162,8 +174,9 @@ static int machxo2_write_init(struct fpga_manager *mgr,
cmd[0].cmd_len = sizeof(enable);
cmd[0].delay_us = MACHXO2_LOW_DELAY_USEC;
- cmd[1].cmd = erase;
- cmd[1].cmd_len = sizeof(erase);
+ cmd[1].cmd = (u8 *)&priv->erase_cmd;
+ cmd[1].cmd_len = sizeof(priv->erase_cmd);
+
ret = priv->write_commands(priv, cmd, 2);
if (ret)
goto fail;
@@ -302,6 +315,21 @@ static const struct fpga_manager_ops machxo2_ops = {
int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev)
{
struct fpga_manager *mgr;
+ u8 erase[] = ISC_ERASE;
+ u32 erase_cmd;
+
+ erase_cmd = get_unaligned_be32(erase);
+
+ if (device_property_read_bool(dev, "lattice,erase-sram"))
+ erase_cmd |= ISC_ERASE_SRAM;
+
+ if (device_property_read_bool(dev, "lattice,erase-feature-row"))
+ erase_cmd |= ISC_ERASE_FEATURE_ROW;
+
+ if (device_property_read_bool(dev, "lattice,erase-userflash"))
+ erase_cmd |= ISC_ERASE_UFM;
+
+ priv->erase_cmd = cpu_to_be32(erase_cmd);
priv->dev = dev;
diff --git a/drivers/fpga/machxo2-common.h b/drivers/fpga/machxo2-common.h
index a16060602a3f..1b54154f91b1 100644
--- a/drivers/fpga/machxo2-common.h
+++ b/drivers/fpga/machxo2-common.h
@@ -31,6 +31,7 @@ struct machxo2_common_priv {
struct machxo2_cmd *cmds, size_t cmd_count);
int (*get_status)(struct machxo2_common_priv *priv, u32 *status);
struct device *dev;
+ __be32 erase_cmd;
};
int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 14/16] fpga: machxo2: add program initialization signalling via gpio
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (12 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 13/16] fpga: machxo2: add optional additional flash areas to be erased Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-25 14:13 ` [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA Johannes Zink
` (2 subsequent siblings)
16 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
Before initiating the programming sequence, this patch generates a low
pulse on FPGA's program_n pin which enables the FPGA's in circuit
programming interface.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-common.c | 15 +++++++++++++++
drivers/fpga/machxo2-common.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
index d93c304cceb9..ccf9a50fc590 100644
--- a/drivers/fpga/machxo2-common.c
+++ b/drivers/fpga/machxo2-common.c
@@ -167,6 +167,17 @@ static int machxo2_write_init(struct fpga_manager *mgr,
return -ENOTSUPP;
}
+ if (priv->fpga_program_n) {
+ /* set fpga in program mode. The timing is chosen very
+ * conservatively, since the MachXO2 Family Datasheet
+ * indicates a low pulse longer than 55ns will be accepted
+ */
+ gpiod_set_value_cansleep(priv->fpga_program_n, 1);
+ usleep_range(1000, 1500);
+ gpiod_set_value_cansleep(priv->fpga_program_n, 0);
+ usleep_range(1000, 1500);
+ }
+
priv->get_status(priv, &status);
dump_status_reg(priv->dev, status);
@@ -331,6 +342,10 @@ int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev)
priv->erase_cmd = cpu_to_be32(erase_cmd);
+ priv->fpga_program_n = devm_gpiod_get_optional(dev,
+ "lattice,program",
+ GPIOD_OUT_LOW);
+
priv->dev = dev;
mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
diff --git a/drivers/fpga/machxo2-common.h b/drivers/fpga/machxo2-common.h
index 1b54154f91b1..0f9f53b48152 100644
--- a/drivers/fpga/machxo2-common.h
+++ b/drivers/fpga/machxo2-common.h
@@ -32,6 +32,7 @@ struct machxo2_common_priv {
int (*get_status)(struct machxo2_common_priv *priv, u32 *status);
struct device *dev;
__be32 erase_cmd;
+ struct gpio_desc *fpga_program_n;
};
int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (13 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 14/16] fpga: machxo2: add program initialization signalling via gpio Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-29 9:26 ` Xu Yilun
2022-08-25 14:13 ` [PATCH 16/16] fpga: machxo2: add configuration over i2c Johannes Zink
2022-08-25 15:25 ` [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Ivan Bornyakov
16 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
Measurements showed that some FPGAs take significantly longer than the
default wait function supplied. The datasheet inidicates up to 30
seconds erase times for some MachXO2 FPGAs, depending on the number of
LUTs (and the corresponding configuration flash size).
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/machxo2-common.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
index ccf9a50fc590..e8967cdee2c6 100644
--- a/drivers/fpga/machxo2-common.c
+++ b/drivers/fpga/machxo2-common.c
@@ -17,6 +17,8 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/property.h>
+#include <linux/iopoll.h>
+#include <linux/time.h>
#include "machxo2-common.h"
#define MACHXO2_LOW_DELAY_USEC 5
@@ -24,6 +26,8 @@
#define MACHXO2_REFRESH_USEC 4800
#define MACHXO2_MAX_BUSY_LOOP 128
#define MACHXO2_MAX_REFRESH_LOOP 16
+#define MACHXO2_MAX_ERASE_USEC (30 * USEC_PER_SEC)
+#define MACHXO2_ERASE_USEC_SLEEP (20 * USEC_PER_MSEC)
#define MACHXO2_PAGE_SIZE 16
#define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
@@ -54,6 +58,18 @@
#define ISC_ERASE_FEATURE_ROW BIT(17)
#define ISC_ERASE_UFM BIT(19)
+static inline int machxo2_wait_until_not_busy_timeout(struct machxo2_common_priv *priv)
+{
+ int ret, pollret;
+ u32 status = MACHXO2_BUSY;
+
+ pollret = read_poll_timeout(priv->get_status, ret,
+ (ret && ret != -EAGAIN) || !(status & MACHXO2_BUSY),
+ MACHXO2_ERASE_USEC_SLEEP, MACHXO2_MAX_ERASE_USEC,
+ true, priv, &status);
+
+ return ret ?: pollret;
+}
static inline u8 get_err(u32 status)
{
@@ -114,6 +130,12 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
if (ret)
goto fail;
+ ret = machxo2_wait_until_not_busy_timeout(priv);
+ if (ret) {
+ dev_err(&mgr->dev, "Erase operation failed (%d)", ret);
+ goto fail;
+ }
+
ret = machxo2_wait_until_not_busy(priv);
if (ret)
goto fail;
@@ -192,9 +214,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
if (ret)
goto fail;
- ret = machxo2_wait_until_not_busy(priv);
- if (ret)
+ ret = machxo2_wait_until_not_busy_timeout(priv);
+ if (ret) {
+ dev_err(&mgr->dev, "Erase operation failed (%d)", ret);
goto fail;
+ }
priv->get_status(priv, &status);
if (status & MACHXO2_FAIL) {
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA
2022-08-25 14:13 ` [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA Johannes Zink
@ 2022-08-29 9:26 ` Xu Yilun
2022-08-29 10:51 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 9:26 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
On 2022-08-25 at 16:13:42 +0200, Johannes Zink wrote:
> Measurements showed that some FPGAs take significantly longer than the
> default wait function supplied. The datasheet inidicates up to 30
> seconds erase times for some MachXO2 FPGAs, depending on the number of
> LUTs (and the corresponding configuration flash size).
>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
> drivers/fpga/machxo2-common.c | 28 ++++++++++++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
> index ccf9a50fc590..e8967cdee2c6 100644
> --- a/drivers/fpga/machxo2-common.c
> +++ b/drivers/fpga/machxo2-common.c
> @@ -17,6 +17,8 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/property.h>
> +#include <linux/iopoll.h>
> +#include <linux/time.h>
> #include "machxo2-common.h"
>
> #define MACHXO2_LOW_DELAY_USEC 5
> @@ -24,6 +26,8 @@
> #define MACHXO2_REFRESH_USEC 4800
> #define MACHXO2_MAX_BUSY_LOOP 128
> #define MACHXO2_MAX_REFRESH_LOOP 16
> +#define MACHXO2_MAX_ERASE_USEC (30 * USEC_PER_SEC)
> +#define MACHXO2_ERASE_USEC_SLEEP (20 * USEC_PER_MSEC)
>
> #define MACHXO2_PAGE_SIZE 16
> #define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
> @@ -54,6 +58,18 @@
> #define ISC_ERASE_FEATURE_ROW BIT(17)
> #define ISC_ERASE_UFM BIT(19)
>
> +static inline int machxo2_wait_until_not_busy_timeout(struct machxo2_common_priv *priv)
> +{
> + int ret, pollret;
> + u32 status = MACHXO2_BUSY;
> +
> + pollret = read_poll_timeout(priv->get_status, ret,
> + (ret && ret != -EAGAIN) || !(status & MACHXO2_BUSY),
> + MACHXO2_ERASE_USEC_SLEEP, MACHXO2_MAX_ERASE_USEC,
> + true, priv, &status);
Why just taking care of erase timeout? I see the busy wait in many
places.
> +
> + return ret ?: pollret;
> +}
>
> static inline u8 get_err(u32 status)
> {
> @@ -114,6 +130,12 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
> if (ret)
> goto fail;
>
> + ret = machxo2_wait_until_not_busy_timeout(priv);
> + if (ret) {
> + dev_err(&mgr->dev, "Erase operation failed (%d)", ret);
> + goto fail;
> + }
> +
> ret = machxo2_wait_until_not_busy(priv);
Is this line still needed?
> if (ret)
> goto fail;
> @@ -192,9 +214,11 @@ static int machxo2_write_init(struct fpga_manager *mgr,
> if (ret)
> goto fail;
>
> - ret = machxo2_wait_until_not_busy(priv);
> - if (ret)
> + ret = machxo2_wait_until_not_busy_timeout(priv);
> + if (ret) {
> + dev_err(&mgr->dev, "Erase operation failed (%d)", ret);
> goto fail;
> + }
>
> priv->get_status(priv, &status);
> if (status & MACHXO2_FAIL) {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA
2022-08-29 9:26 ` Xu Yilun
@ 2022-08-29 10:51 ` Johannes Zink
2022-08-29 14:57 ` Xu Yilun
0 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-29 10:51 UTC (permalink / raw)
To: Xu Yilun
Cc: devicetree, linux-fpga, Rob Herring, Moritz Fischer, kernel, Wu Hao
Hi Yilun,
On Mon, 2022-08-29 at 17:26 +0800, Xu Yilun wrote:
> On 2022-08-25 at 16:13:42 +0200, Johannes Zink wrote:
> > Measurements showed that some FPGAs take significantly longer than
> > the
> > default wait function supplied. The datasheet inidicates up to 30
> > seconds erase times for some MachXO2 FPGAs, depending on the number
> > of
> > LUTs (and the corresponding configuration flash size).
> >
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
> > drivers/fpga/machxo2-common.c | 28 ++++++++++++++++++++++++++--
> > 1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-
> > common.c
> > index ccf9a50fc590..e8967cdee2c6 100644
> > --- a/drivers/fpga/machxo2-common.c
> > +++ b/drivers/fpga/machxo2-common.c
> > @@ -17,6 +17,8 @@
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/property.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/time.h>
> > #include "machxo2-common.h"
> >
> > #define MACHXO2_LOW_DELAY_USEC 5
> > @@ -24,6 +26,8 @@
> > #define MACHXO2_REFRESH_USEC 4800
> > #define MACHXO2_MAX_BUSY_LOOP 128
> > #define MACHXO2_MAX_REFRESH_LOOP 16
> > +#define MACHXO2_MAX_ERASE_USEC (30 * USEC_PER_SEC)
> > +#define MACHXO2_ERASE_USEC_SLEEP (20 * USEC_PER_MSEC)
> >
> > #define MACHXO2_PAGE_SIZE 16
> > #define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
> > @@ -54,6 +58,18 @@
> > #define ISC_ERASE_FEATURE_ROW BIT(17)
> > #define ISC_ERASE_UFM BIT(19)
> >
> > +static inline int machxo2_wait_until_not_busy_timeout(struct
> > machxo2_common_priv *priv)
> > +{
> > + int ret, pollret;
> > + u32 status = MACHXO2_BUSY;
> > +
> > + pollret = read_poll_timeout(priv->get_status, ret,
> > + (ret && ret != -EAGAIN) ||
> > !(status & MACHXO2_BUSY),
> > + MACHXO2_ERASE_USEC_SLEEP,
> > MACHXO2_MAX_ERASE_USEC,
> > + true, priv, &status);
>
> Why just taking care of erase timeout? I see the busy wait in many
> places.
>
Erasing the flash memory takes significantly longer than the other
operations (up to 30s), which is why I decided to use this separate
implementation. For other commands the fpga indicates no-more-busy much
faster than for the erase_flash command.
> > +
> > + return ret ?: pollret;
> > +}
> >
> > static inline u8 get_err(u32 status)
> > {
> > @@ -114,6 +130,12 @@ static int machxo2_cleanup(struct fpga_manager
> > *mgr)
> > if (ret)
> > goto fail;
> >
> > + ret = machxo2_wait_until_not_busy_timeout(priv);
> > + if (ret) {
> > + dev_err(&mgr->dev, "Erase operation failed (%d)",
> > ret);
> > + goto fail;
> > + }
> > +
> > ret = machxo2_wait_until_not_busy(priv);
>
> Is this line still needed?
agreed, this line should become obsolete, since if we reach this point
the fpga is not indicating busy any longer or the wait has been aborted
due to an error. I will remove it in v2.
>
> > if (ret)
> > goto fail;
> > @@ -192,9 +214,11 @@ static int machxo2_write_init(struct
> > fpga_manager *mgr,
> > if (ret)
> > goto fail;
> >
> > - ret = machxo2_wait_until_not_busy(priv);
> > - if (ret)
> > + ret = machxo2_wait_until_not_busy_timeout(priv);
> > + if (ret) {
> > + dev_err(&mgr->dev, "Erase operation failed (%d)",
> > ret);
> > goto fail;
> > + }
> >
> > priv->get_status(priv, &status);
> > if (status & MACHXO2_FAIL) {
> > --
> > 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 |
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA
2022-08-29 10:51 ` Johannes Zink
@ 2022-08-29 14:57 ` Xu Yilun
2022-08-31 7:56 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 14:57 UTC (permalink / raw)
To: Johannes Zink
Cc: devicetree, linux-fpga, Rob Herring, Moritz Fischer, kernel, Wu Hao
On 2022-08-29 at 12:51:19 +0200, Johannes Zink wrote:
> Hi Yilun,
>
> On Mon, 2022-08-29 at 17:26 +0800, Xu Yilun wrote:
> > On 2022-08-25 at 16:13:42 +0200, Johannes Zink wrote:
> > > Measurements showed that some FPGAs take significantly longer than
> > > the
> > > default wait function supplied. The datasheet inidicates up to 30
> > > seconds erase times for some MachXO2 FPGAs, depending on the number
> > > of
> > > LUTs (and the corresponding configuration flash size).
> > >
> > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > ---
> > > drivers/fpga/machxo2-common.c | 28 ++++++++++++++++++++++++++--
> > > 1 file changed, 26 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-
> > > common.c
> > > index ccf9a50fc590..e8967cdee2c6 100644
> > > --- a/drivers/fpga/machxo2-common.c
> > > +++ b/drivers/fpga/machxo2-common.c
> > > @@ -17,6 +17,8 @@
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > #include <linux/property.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/time.h>
> > > #include "machxo2-common.h"
> > >
> > > #define MACHXO2_LOW_DELAY_USEC 5
> > > @@ -24,6 +26,8 @@
> > > #define MACHXO2_REFRESH_USEC 4800
> > > #define MACHXO2_MAX_BUSY_LOOP 128
> > > #define MACHXO2_MAX_REFRESH_LOOP 16
> > > +#define MACHXO2_MAX_ERASE_USEC (30 * USEC_PER_SEC)
> > > +#define MACHXO2_ERASE_USEC_SLEEP (20 * USEC_PER_MSEC)
> > >
> > > #define MACHXO2_PAGE_SIZE 16
> > > #define MACHXO2_BUF_SIZE (MACHXO2_PAGE_SIZE + 4)
> > > @@ -54,6 +58,18 @@
> > > #define ISC_ERASE_FEATURE_ROW BIT(17)
> > > #define ISC_ERASE_UFM BIT(19)
> > >
> > > +static inline int machxo2_wait_until_not_busy_timeout(struct
> > > machxo2_common_priv *priv)
> > > +{
> > > + int ret, pollret;
> > > + u32 status = MACHXO2_BUSY;
> > > +
> > > + pollret = read_poll_timeout(priv->get_status, ret,
> > > + (ret && ret != -EAGAIN) ||
> > > !(status & MACHXO2_BUSY),
> > > + MACHXO2_ERASE_USEC_SLEEP,
> > > MACHXO2_MAX_ERASE_USEC,
> > > + true, priv, &status);
> >
> > Why just taking care of erase timeout? I see the busy wait in many
> > places.
> >
>
> Erasing the flash memory takes significantly longer than the other
> operations (up to 30s), which is why I decided to use this separate
> implementation. For other commands the fpga indicates no-more-busy much
> faster than for the erase_flash command.
It is almost always better to have a relatively measureable timeout,
unless it is really time critical. Apparently spi/i2c transfer is not
time critical itself. So since you have implemented a better function,
why not use it?
Thanks,
Yilun
>
> > > +
> > > + return ret ?: pollret;
> > > +}
> > >
> > > static inline u8 get_err(u32 status)
> > > {
> > > @@ -114,6 +130,12 @@ static int machxo2_cleanup(struct fpga_manager
> > > *mgr)
> > > if (ret)
> > > goto fail;
> > >
> > > + ret = machxo2_wait_until_not_busy_timeout(priv);
> > > + if (ret) {
> > > + dev_err(&mgr->dev, "Erase operation failed (%d)",
> > > ret);
> > > + goto fail;
> > > + }
> > > +
> > > ret = machxo2_wait_until_not_busy(priv);
> >
> > Is this line still needed?
>
> agreed, this line should become obsolete, since if we reach this point
> the fpga is not indicating busy any longer or the wait has been aborted
> due to an error. I will remove it in v2.
>
> >
> > > if (ret)
> > > goto fail;
> > > @@ -192,9 +214,11 @@ static int machxo2_write_init(struct
> > > fpga_manager *mgr,
> > > if (ret)
> > > goto fail;
> > >
> > > - ret = machxo2_wait_until_not_busy(priv);
> > > - if (ret)
> > > + ret = machxo2_wait_until_not_busy_timeout(priv);
> > > + if (ret) {
> > > + dev_err(&mgr->dev, "Erase operation failed (%d)",
> > > ret);
> > > goto fail;
> > > + }
> > >
> > > priv->get_status(priv, &status);
> > > if (status & MACHXO2_FAIL) {
> > > --
> > > 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 |
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA
2022-08-29 14:57 ` Xu Yilun
@ 2022-08-31 7:56 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-31 7:56 UTC (permalink / raw)
To: Xu Yilun
Cc: devicetree, linux-fpga, Rob Herring, Moritz Fischer, kernel, Wu Hao
Hi Yilun,
On Mon, 2022-08-29 at 22:57 +0800, Xu Yilun wrote:
> On 2022-08-29 at 12:51:19 +0200, Johannes Zink wrote:
> > Hi Yilun,
> >
> > On Mon, 2022-08-29 at 17:26 +0800, Xu Yilun wrote:
> > > On 2022-08-25 at 16:13:42 +0200, Johannes Zink wrote:
> > > > Measurements showed that some FPGAs take significantly longer
> > > > than
> > > > the
> > > > default wait function supplied. The datasheet inidicates up to
> > > > 30
> > > > seconds erase times for some MachXO2 FPGAs, depending on the
> > > > number
> > > > of
> > > > LUTs (and the corresponding configuration flash size).
> > > >
> > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > > ---
[snip]
> > > > +static inline int machxo2_wait_until_not_busy_timeout(struct
> > > > machxo2_common_priv *priv)
> > > > +{
> > > > + int ret, pollret;
> > > > + u32 status = MACHXO2_BUSY;
> > > > +
> > > > + pollret = read_poll_timeout(priv->get_status, ret,
> > > > + (ret && ret != -EAGAIN) ||
> > > > !(status & MACHXO2_BUSY),
> > > > + MACHXO2_ERASE_USEC_SLEEP,
> > > > MACHXO2_MAX_ERASE_USEC,
> > > > + true, priv, &status);
> > >
> > > Why just taking care of erase timeout? I see the busy wait in
> > > many
> > > places.
> > >
> >
> > Erasing the flash memory takes significantly longer than the other
> > operations (up to 30s), which is why I decided to use this separate
> > implementation. For other commands the fpga indicates no-more-busy
> > much
> > faster than for the erase_flash command.
>
> It is almost always better to have a relatively measureable timeout,
> unless it is really time critical. Apparently spi/i2c transfer is not
> time critical itself. So since you have implemented a better
> function,
> why not use it?
>
> Thanks,
> Yilun
That's a fair point. I will look in the datasheet how long the timeouts
on the other operations should be set and will replace the former busy
wait implementation in v2. I would not expect any breakage from that.
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 16/16] fpga: machxo2: add configuration over i2c
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (14 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA Johannes Zink
@ 2022-08-25 14:13 ` Johannes Zink
2022-08-29 9:47 ` Xu Yilun
2022-08-25 15:25 ` [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Ivan Bornyakov
16 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-25 14:13 UTC (permalink / raw)
To: linux-fpga
Cc: devicetree, Rob Herring, Moritz Fischer, Wu Hao, Xu Yilun,
kernel, Johannes Zink
From: Peter Jensen <pdj@bang-olufsen.dk>
The configuration flash of the machxo2 fpga can also be erased and
written over i2c instead of spi. Add this functionality to the
refactored common driver. Since some commands are shorter over I2C than
they are over SPI some quirks are added to the common driver in order to
account for that.
Signed-off-by: Peter Jensen <pdj@bang-olufsen.dk>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/fpga/Kconfig | 8 ++
drivers/fpga/Makefile | 1 +
drivers/fpga/machxo2-common.c | 15 +++-
drivers/fpga/machxo2-common.h | 3 +
drivers/fpga/machxo2-i2c.c | 137 ++++++++++++++++++++++++++++++++++
5 files changed, 163 insertions(+), 1 deletion(-)
create mode 100644 drivers/fpga/machxo2-i2c.c
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index e5869a732246..97081bbd7c19 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -90,6 +90,14 @@ config FPGA_MGR_MACHXO2_SPI
FPGA manager driver support for Lattice MachXO2 configuration
over slave SPI interface.
+config FPGA_MGR_MACHXO2_I2C
+ tristate "Lattice MachXO2 I2C"
+ depends on I2C
+ select FPGA_MGR_MACHXO2_COMMON
+ help
+ FPGA manager driver support for Lattice MachXO2 configuration
+ over slave I2C interface.
+
config FPGA_MGR_TS73XX
tristate "Technologic Systems TS-73xx SBC FPGA Manager"
depends on ARCH_EP93XX && MACH_TS72XX
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index f247a8de83ad..fcdf79f4d424 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_FPGA_MGR_ALTERA_PS_SPI) += altera-ps-spi.o
obj-$(CONFIG_FPGA_MGR_ICE40_SPI) += ice40-spi.o
obj-$(CONFIG_FPGA_MGR_MACHXO2_COMMON) += machxo2-common.o
obj-$(CONFIG_FPGA_MGR_MACHXO2_SPI) += machxo2-spi.o
+obj-$(CONFIG_FPGA_MGR_MACHXO2_I2C) += machxo2-i2c.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o
obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC) += stratix10-soc.o
diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
index e8967cdee2c6..0a3c126675da 100644
--- a/drivers/fpga/machxo2-common.c
+++ b/drivers/fpga/machxo2-common.c
@@ -100,7 +100,7 @@ static void dump_status_reg(struct device *dev, u32 status)
!!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
}
-static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
+int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
{
u32 status;
int ret, loop = 0;
@@ -143,6 +143,11 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
cmd.cmd = refresh;
cmd.cmd_len = sizeof(refresh);
cmd.delay_us = MACHXO2_REFRESH_USEC;
+
+ /* quirk: refresh command over i2c is 1 byte shorter */
+ if (priv->refresh_3b)
+ cmd.cmd_len--;
+
ret = priv->write_commands(priv, &cmd, 1);
if (ret)
goto fail;
@@ -207,6 +212,10 @@ static int machxo2_write_init(struct fpga_manager *mgr,
cmd[0].cmd_len = sizeof(enable);
cmd[0].delay_us = MACHXO2_LOW_DELAY_USEC;
+ /* quirk: enable command over i2c is 1 byte shorter */
+ if (priv->enable_3b)
+ cmd[0].cmd_len--;
+
cmd[1].cmd = (u8 *)&priv->erase_cmd;
cmd[1].cmd_len = sizeof(priv->erase_cmd);
@@ -313,6 +322,10 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
cmd.cmd_len = sizeof(refresh);
cmd.delay_us = MACHXO2_REFRESH_USEC;
+ /* quirk: refresh command over i2c is 1 byte shorter */
+ if (priv->refresh_3b)
+ cmd.cmd_len--;
+
do {
ret = priv->write_commands(priv, &cmd, 1);
if (ret)
diff --git a/drivers/fpga/machxo2-common.h b/drivers/fpga/machxo2-common.h
index 0f9f53b48152..8c09345adee5 100644
--- a/drivers/fpga/machxo2-common.h
+++ b/drivers/fpga/machxo2-common.h
@@ -32,9 +32,12 @@ struct machxo2_common_priv {
int (*get_status)(struct machxo2_common_priv *priv, u32 *status);
struct device *dev;
__be32 erase_cmd;
+ u8 enable_3b:1;
+ u8 refresh_3b:1;
struct gpio_desc *fpga_program_n;
};
int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev);
+int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv);
#endif
diff --git a/drivers/fpga/machxo2-i2c.c b/drivers/fpga/machxo2-i2c.c
new file mode 100644
index 000000000000..a309016def1c
--- /dev/null
+++ b/drivers/fpga/machxo2-i2c.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Lattice MachXO2 Slave I2C Driver
+ *
+ * Manage Lattice FPGA firmware that is loaded over I2C using
+ * the slave serial configuration interface.
+ *
+ * Copyright (C) 2018 Paolo Pisati <p.pisati@gmail.com>
+ * Copyright (C) 2021 Peter Jensen <pdj@bang-olufsen.dk>
+ */
+
+#include <linux/i2c.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include "machxo2-common.h"
+
+
+struct machxo2_i2c_priv {
+ struct machxo2_common_priv common;
+ struct i2c_client *client;
+};
+
+static inline struct machxo2_i2c_priv *to_machxo2_i2c_priv(struct machxo2_common_priv *common)
+{
+ return container_of(common, struct machxo2_i2c_priv, common);
+}
+
+static int machxo2_i2c_get_status(struct machxo2_common_priv *bus, u32 *status)
+{
+ struct machxo2_i2c_priv *i2cPriv = to_machxo2_i2c_priv(bus);
+ struct i2c_client *client = i2cPriv->client;
+ u8 read_status[] = LSC_READ_STATUS;
+ __be32 tmp;
+ int ret;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .buf = read_status,
+ .len = ARRAY_SIZE(read_status),
+ }, {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .buf = (u8 *) &tmp,
+ .len = sizeof(tmp)
+ }
+ };
+
+ ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ if (ret < 0)
+ return ret;
+ if (ret != ARRAY_SIZE(msg))
+ return -EIO;
+ *status = be32_to_cpu(tmp);
+
+ return 0;
+}
+
+static int machxo2_i2c_write(struct machxo2_common_priv *common,
+ struct machxo2_cmd *cmds, size_t cmd_count)
+{
+ struct machxo2_i2c_priv *i2c_priv = to_machxo2_i2c_priv(common);
+ struct i2c_client *client = i2c_priv->client;
+ size_t i;
+ int ret;
+
+ for (i = 0; i < cmd_count; i++) {
+ struct i2c_msg msg[] = {
+ {
+ .addr = client->addr,
+ .buf = cmds[i].cmd,
+ .len = cmds[i].cmd_len,
+ },
+ };
+
+ ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ if (ret < 0)
+ return ret;
+ if (ret != ARRAY_SIZE(msg))
+ return -EIO;
+ if (cmds[i].delay_us)
+ usleep_range(cmds[i].delay_us, cmds[i].delay_us +
+ cmds[i].delay_us / 4);
+ if (i < cmd_count - 1) /* on any iteration except for the last one... */
+ ret = machxo2_wait_until_not_busy(common);
+ }
+
+ return 0;
+}
+
+static int machxo2_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct machxo2_i2c_priv *priv;
+
+ priv = devm_kzalloc(dev, sizeof(struct machxo2_i2c_priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->client = client;
+ priv->common.get_status = machxo2_i2c_get_status;
+ priv->common.write_commands = machxo2_i2c_write;
+
+ /* Commands are usually 4b, but these aren't for i2c */
+ priv->common.enable_3b = true;
+ priv->common.refresh_3b = true;
+
+ return machxo2_common_init(&priv->common, dev);
+}
+
+static const struct of_device_id of_match[] = {
+ { .compatible = "lattice,machxo2-slave-i2c", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, of_match);
+
+static const struct i2c_device_id lattice_ids[] = {
+ { "machxo2-slave-i2c", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, lattice_ids);
+
+static struct i2c_driver machxo2_i2c_driver = {
+ .driver = {
+ .name = "machxo2-slave-i2c",
+ .of_match_table = of_match_ptr(of_match),
+ },
+ .probe = machxo2_i2c_probe,
+ .id_table = lattice_ids,
+};
+
+module_i2c_driver(machxo2_i2c_driver);
+
+MODULE_AUTHOR("Peter Jensen <pdj@bang-olufsen.dk>");
+MODULE_DESCRIPTION("Load Lattice FPGA firmware over I2C");
+MODULE_LICENSE("GPL");
--
2.30.2
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH 16/16] fpga: machxo2: add configuration over i2c
2022-08-25 14:13 ` [PATCH 16/16] fpga: machxo2: add configuration over i2c Johannes Zink
@ 2022-08-29 9:47 ` Xu Yilun
2022-08-29 13:21 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 9:47 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
On 2022-08-25 at 16:13:43 +0200, Johannes Zink wrote:
> From: Peter Jensen <pdj@bang-olufsen.dk>
>
> The configuration flash of the machxo2 fpga can also be erased and
> written over i2c instead of spi. Add this functionality to the
> refactored common driver. Since some commands are shorter over I2C than
> they are over SPI some quirks are added to the common driver in order to
> account for that.
>
> Signed-off-by: Peter Jensen <pdj@bang-olufsen.dk>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
> drivers/fpga/Kconfig | 8 ++
> drivers/fpga/Makefile | 1 +
> drivers/fpga/machxo2-common.c | 15 +++-
> drivers/fpga/machxo2-common.h | 3 +
> drivers/fpga/machxo2-i2c.c | 137 ++++++++++++++++++++++++++++++++++
> 5 files changed, 163 insertions(+), 1 deletion(-)
> create mode 100644 drivers/fpga/machxo2-i2c.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index e5869a732246..97081bbd7c19 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -90,6 +90,14 @@ config FPGA_MGR_MACHXO2_SPI
> FPGA manager driver support for Lattice MachXO2 configuration
> over slave SPI interface.
>
> +config FPGA_MGR_MACHXO2_I2C
> + tristate "Lattice MachXO2 I2C"
> + depends on I2C
> + select FPGA_MGR_MACHXO2_COMMON
> + help
> + FPGA manager driver support for Lattice MachXO2 configuration
> + over slave I2C interface.
> +
> config FPGA_MGR_TS73XX
> tristate "Technologic Systems TS-73xx SBC FPGA Manager"
> depends on ARCH_EP93XX && MACH_TS72XX
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index f247a8de83ad..fcdf79f4d424 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_FPGA_MGR_ALTERA_PS_SPI) += altera-ps-spi.o
> obj-$(CONFIG_FPGA_MGR_ICE40_SPI) += ice40-spi.o
> obj-$(CONFIG_FPGA_MGR_MACHXO2_COMMON) += machxo2-common.o
> obj-$(CONFIG_FPGA_MGR_MACHXO2_SPI) += machxo2-spi.o
> +obj-$(CONFIG_FPGA_MGR_MACHXO2_I2C) += machxo2-i2c.o
> obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
> obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o
> obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC) += stratix10-soc.o
> diff --git a/drivers/fpga/machxo2-common.c b/drivers/fpga/machxo2-common.c
> index e8967cdee2c6..0a3c126675da 100644
> --- a/drivers/fpga/machxo2-common.c
> +++ b/drivers/fpga/machxo2-common.c
> @@ -100,7 +100,7 @@ static void dump_status_reg(struct device *dev, u32 status)
> !!FIELD_GET(MACHXO2_DVER, status), get_err_string(get_err(status)));
> }
>
> -static int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
> +int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv)
> {
> u32 status;
> int ret, loop = 0;
> @@ -143,6 +143,11 @@ static int machxo2_cleanup(struct fpga_manager *mgr)
> cmd.cmd = refresh;
> cmd.cmd_len = sizeof(refresh);
> cmd.delay_us = MACHXO2_REFRESH_USEC;
> +
> + /* quirk: refresh command over i2c is 1 byte shorter */
> + if (priv->refresh_3b)
> + cmd.cmd_len--;
> +
> ret = priv->write_commands(priv, &cmd, 1);
> if (ret)
> goto fail;
> @@ -207,6 +212,10 @@ static int machxo2_write_init(struct fpga_manager *mgr,
> cmd[0].cmd_len = sizeof(enable);
> cmd[0].delay_us = MACHXO2_LOW_DELAY_USEC;
>
> + /* quirk: enable command over i2c is 1 byte shorter */
> + if (priv->enable_3b)
> + cmd[0].cmd_len--;
> +
> cmd[1].cmd = (u8 *)&priv->erase_cmd;
> cmd[1].cmd_len = sizeof(priv->erase_cmd);
>
> @@ -313,6 +322,10 @@ static int machxo2_write_complete(struct fpga_manager *mgr,
> cmd.cmd_len = sizeof(refresh);
> cmd.delay_us = MACHXO2_REFRESH_USEC;
>
> + /* quirk: refresh command over i2c is 1 byte shorter */
> + if (priv->refresh_3b)
> + cmd.cmd_len--;
> +
> do {
> ret = priv->write_commands(priv, &cmd, 1);
> if (ret)
> diff --git a/drivers/fpga/machxo2-common.h b/drivers/fpga/machxo2-common.h
> index 0f9f53b48152..8c09345adee5 100644
> --- a/drivers/fpga/machxo2-common.h
> +++ b/drivers/fpga/machxo2-common.h
> @@ -32,9 +32,12 @@ struct machxo2_common_priv {
> int (*get_status)(struct machxo2_common_priv *priv, u32 *status);
> struct device *dev;
> __be32 erase_cmd;
> + u8 enable_3b:1;
> + u8 refresh_3b:1;
> struct gpio_desc *fpga_program_n;
> };
>
> int machxo2_common_init(struct machxo2_common_priv *priv, struct device *dev);
> +int machxo2_wait_until_not_busy(struct machxo2_common_priv *priv);
>
> #endif
> diff --git a/drivers/fpga/machxo2-i2c.c b/drivers/fpga/machxo2-i2c.c
> new file mode 100644
> index 000000000000..a309016def1c
> --- /dev/null
> +++ b/drivers/fpga/machxo2-i2c.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lattice MachXO2 Slave I2C Driver
> + *
> + * Manage Lattice FPGA firmware that is loaded over I2C using
> + * the slave serial configuration interface.
> + *
> + * Copyright (C) 2018 Paolo Pisati <p.pisati@gmail.com>
> + * Copyright (C) 2021 Peter Jensen <pdj@bang-olufsen.dk>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include "machxo2-common.h"
> +
> +
> +struct machxo2_i2c_priv {
> + struct machxo2_common_priv common;
> + struct i2c_client *client;
> +};
> +
> +static inline struct machxo2_i2c_priv *to_machxo2_i2c_priv(struct machxo2_common_priv *common)
> +{
> + return container_of(common, struct machxo2_i2c_priv, common);
> +}
> +
> +static int machxo2_i2c_get_status(struct machxo2_common_priv *bus, u32 *status)
> +{
> + struct machxo2_i2c_priv *i2cPriv = to_machxo2_i2c_priv(bus);
> + struct i2c_client *client = i2cPriv->client;
> + u8 read_status[] = LSC_READ_STATUS;
The command word could also be bus agnostic. I think a callback like
write_then_read(bus, txbuf, n_tx, rxbuf, n_rx) could be a better
abstraction.
> + __be32 tmp;
> + int ret;
> + struct i2c_msg msg[] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> + .buf = read_status,
> + .len = ARRAY_SIZE(read_status),
> + }, {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .buf = (u8 *) &tmp,
> + .len = sizeof(tmp)
> + }
> + };
> +
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (ret < 0)
> + return ret;
> + if (ret != ARRAY_SIZE(msg))
> + return -EIO;
> + *status = be32_to_cpu(tmp);
> +
> + return 0;
> +}
> +
> +static int machxo2_i2c_write(struct machxo2_common_priv *common,
> + struct machxo2_cmd *cmds, size_t cmd_count)
> +{
> + struct machxo2_i2c_priv *i2c_priv = to_machxo2_i2c_priv(common);
> + struct i2c_client *client = i2c_priv->client;
> + size_t i;
> + int ret;
> +
> + for (i = 0; i < cmd_count; i++) {
> + struct i2c_msg msg[] = {
> + {
> + .addr = client->addr,
> + .buf = cmds[i].cmd,
> + .len = cmds[i].cmd_len,
> + },
> + };
> +
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + if (ret < 0)
> + return ret;
> + if (ret != ARRAY_SIZE(msg))
> + return -EIO;
> + if (cmds[i].delay_us)
> + usleep_range(cmds[i].delay_us, cmds[i].delay_us +
> + cmds[i].delay_us / 4);
> + if (i < cmd_count - 1) /* on any iteration except for the last one... */
> + ret = machxo2_wait_until_not_busy(common);
Seems no need to implement the loop and wait in transportation layer,
they are common. A callback like write(bus, txbuf, n_tx) is better?
Thanks,
Yilun
> + }
> +
> + return 0;
> +}
> +
> +static int machxo2_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct machxo2_i2c_priv *priv;
> +
> + priv = devm_kzalloc(dev, sizeof(struct machxo2_i2c_priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->client = client;
> + priv->common.get_status = machxo2_i2c_get_status;
> + priv->common.write_commands = machxo2_i2c_write;
> +
> + /* Commands are usually 4b, but these aren't for i2c */
> + priv->common.enable_3b = true;
> + priv->common.refresh_3b = true;
> +
> + return machxo2_common_init(&priv->common, dev);
> +}
> +
> +static const struct of_device_id of_match[] = {
> + { .compatible = "lattice,machxo2-slave-i2c", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, of_match);
> +
> +static const struct i2c_device_id lattice_ids[] = {
> + { "machxo2-slave-i2c", 0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, lattice_ids);
> +
> +static struct i2c_driver machxo2_i2c_driver = {
> + .driver = {
> + .name = "machxo2-slave-i2c",
> + .of_match_table = of_match_ptr(of_match),
> + },
> + .probe = machxo2_i2c_probe,
> + .id_table = lattice_ids,
> +};
> +
> +module_i2c_driver(machxo2_i2c_driver);
> +
> +MODULE_AUTHOR("Peter Jensen <pdj@bang-olufsen.dk>");
> +MODULE_DESCRIPTION("Load Lattice FPGA firmware over I2C");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 16/16] fpga: machxo2: add configuration over i2c
2022-08-29 9:47 ` Xu Yilun
@ 2022-08-29 13:21 ` Johannes Zink
2022-08-29 14:45 ` Xu Yilun
0 siblings, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-29 13:21 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
Hi Yilun,
On Mon, 2022-08-29 at 17:47 +0800, Xu Yilun wrote:
> On 2022-08-25 at 16:13:43 +0200, Johannes Zink wrote:
> > From: Peter Jensen <pdj@bang-olufsen.dk>
> >
> > The configuration flash of the machxo2 fpga can also be erased and
> > written over i2c instead of spi. Add this functionality to the
> > refactored common driver. Since some commands are shorter over I2C
> > than
> > they are over SPI some quirks are added to the common driver in
> > order to
> > account for that.
> >
> > Signed-off-by: Peter Jensen <pdj@bang-olufsen.dk>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
[snip]
>
> > +static int machxo2_i2c_get_status(struct machxo2_common_priv *bus,
> > u32 *status)
> > +{
> > + struct machxo2_i2c_priv *i2cPriv =
> > to_machxo2_i2c_priv(bus);
> > + struct i2c_client *client = i2cPriv->client;
> > + u8 read_status[] = LSC_READ_STATUS;
>
> The command word could also be bus agnostic. I think a callback like
> write_then_read(bus, txbuf, n_tx, rxbuf, n_rx) could be a better
> abstraction.
I agree. The only command reading from the fpga is the get_status
functionality but your proposal provides a cleaner implementation.
I will add it in v2.
>
> > + __be32 tmp;
> > + int ret;
> > + struct i2c_msg msg[] = {
> > + {
> > + .addr = client->addr,
> > + .flags = 0,
> > + .buf = read_status,
> > + .len = ARRAY_SIZE(read_status),
> > + }, {
> > + .addr = client->addr,
> > + .flags = I2C_M_RD,
> > + .buf = (u8 *) &tmp,
> > + .len = sizeof(tmp)
> > + }
> > + };
> > +
> > + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> > + if (ret < 0)
> > + return ret;
> > + if (ret != ARRAY_SIZE(msg))
> > + return -EIO;
> > + *status = be32_to_cpu(tmp);
> > +
> > + return 0;
> > +}
> > +
> > +static int machxo2_i2c_write(struct machxo2_common_priv *common,
> > + struct machxo2_cmd *cmds, size_t
> > cmd_count)
> > +{
> > + struct machxo2_i2c_priv *i2c_priv =
> > to_machxo2_i2c_priv(common);
> > + struct i2c_client *client = i2c_priv->client;
> > + size_t i;
> > + int ret;
> > +
> > + for (i = 0; i < cmd_count; i++) {
> > + struct i2c_msg msg[] = {
> > + {
> > + .addr = client->addr,
> > + .buf = cmds[i].cmd,
> > + .len = cmds[i].cmd_len,
> > + },
> > + };
> > +
> > + ret = i2c_transfer(client->adapter, msg,
> > ARRAY_SIZE(msg));
> > + if (ret < 0)
> > + return ret;
> > + if (ret != ARRAY_SIZE(msg))
> > + return -EIO;
> > + if (cmds[i].delay_us)
> > + usleep_range(cmds[i].delay_us,
> > cmds[i].delay_us +
> > + cmds[i].delay_us / 4);
> > + if (i < cmd_count - 1) /* on any iteration except
> > for the last one... */
> > + ret = machxo2_wait_until_not_busy(common);
>
> Seems no need to implement the loop and wait in transportation layer,
> they are common. A callback like write(bus, txbuf, n_tx) is better?
>
> Thanks,
> Yilun
I have chosen this implementation mostly due to the fact that I don't
have a SPI machxo2 device to test against, so I am intentionally
keeping changes to a minimum.
Moving the wait between single commands into the transport layer is not
functionally equivalent, e.g. the ISC_ENABLE - ISC_ERASE command
sequence in the machxo2_write_init function would require two separate
messages with a wait time between them, which would deassert the CS
line between sending the messages via SPI if not sent as a sequence of
SPI transfers. For some of the commands, the fpga requires a delay
between the different commands, which was implemented by setting the
delay property of the spi transfer objects in the original driver.
This implementation tries to mimic the timing behaviour of the SPI
transfer delay property for the I2C implementation.
Best regards
Johannes
>
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int machxo2_i2c_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct device *dev = &client->dev;
> > + struct machxo2_i2c_priv *priv;
> > +
> > + priv = devm_kzalloc(dev, sizeof(struct machxo2_i2c_priv),
> > GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->client = client;
> > + priv->common.get_status = machxo2_i2c_get_status;
> > + priv->common.write_commands = machxo2_i2c_write;
> > +
> > + /* Commands are usually 4b, but these aren't for i2c */
> > + priv->common.enable_3b = true;
> > + priv->common.refresh_3b = true;
> > +
> > + return machxo2_common_init(&priv->common, dev);
> > +}
> > +
> > +static const struct of_device_id of_match[] = {
> > + { .compatible = "lattice,machxo2-slave-i2c", },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, of_match);
> > +
> > +static const struct i2c_device_id lattice_ids[] = {
> > + { "machxo2-slave-i2c", 0 },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(i2c, lattice_ids);
> > +
> > +static struct i2c_driver machxo2_i2c_driver = {
> > + .driver = {
> > + .name = "machxo2-slave-i2c",
> > + .of_match_table = of_match_ptr(of_match),
> > + },
> > + .probe = machxo2_i2c_probe,
> > + .id_table = lattice_ids,
> > +};
> > +
> > +module_i2c_driver(machxo2_i2c_driver);
> > +
> > +MODULE_AUTHOR("Peter Jensen <pdj@bang-olufsen.dk>");
> > +MODULE_DESCRIPTION("Load Lattice FPGA firmware over I2C");
> > +MODULE_LICENSE("GPL");
> > --
> > 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 |
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 16/16] fpga: machxo2: add configuration over i2c
2022-08-29 13:21 ` Johannes Zink
@ 2022-08-29 14:45 ` Xu Yilun
2022-08-31 16:07 ` Johannes Zink
0 siblings, 1 reply; 48+ messages in thread
From: Xu Yilun @ 2022-08-29 14:45 UTC (permalink / raw)
To: Johannes Zink
Cc: linux-fpga, devicetree, Rob Herring, Moritz Fischer, Wu Hao, kernel
On 2022-08-29 at 15:21:19 +0200, Johannes Zink wrote:
> Hi Yilun,
>
> On Mon, 2022-08-29 at 17:47 +0800, Xu Yilun wrote:
> > On 2022-08-25 at 16:13:43 +0200, Johannes Zink wrote:
> > > From: Peter Jensen <pdj@bang-olufsen.dk>
> > >
> > > The configuration flash of the machxo2 fpga can also be erased and
> > > written over i2c instead of spi. Add this functionality to the
> > > refactored common driver. Since some commands are shorter over I2C
> > > than
> > > they are over SPI some quirks are added to the common driver in
> > > order to
> > > account for that.
> > >
> > > Signed-off-by: Peter Jensen <pdj@bang-olufsen.dk>
> > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > ---
>
> [snip]
> >
>
> > > +static int machxo2_i2c_get_status(struct machxo2_common_priv *bus,
> > > u32 *status)
> > > +{
> > > + struct machxo2_i2c_priv *i2cPriv =
> > > to_machxo2_i2c_priv(bus);
> > > + struct i2c_client *client = i2cPriv->client;
> > > + u8 read_status[] = LSC_READ_STATUS;
> >
> > The command word could also be bus agnostic. I think a callback like
> > write_then_read(bus, txbuf, n_tx, rxbuf, n_rx) could be a better
> > abstraction.
>
> I agree. The only command reading from the fpga is the get_status
> functionality but your proposal provides a cleaner implementation.
> I will add it in v2.
> >
> > > + __be32 tmp;
> > > + int ret;
> > > + struct i2c_msg msg[] = {
> > > + {
> > > + .addr = client->addr,
> > > + .flags = 0,
> > > + .buf = read_status,
> > > + .len = ARRAY_SIZE(read_status),
> > > + }, {
> > > + .addr = client->addr,
> > > + .flags = I2C_M_RD,
> > > + .buf = (u8 *) &tmp,
> > > + .len = sizeof(tmp)
> > > + }
> > > + };
> > > +
> > > + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> > > + if (ret < 0)
> > > + return ret;
> > > + if (ret != ARRAY_SIZE(msg))
> > > + return -EIO;
> > > + *status = be32_to_cpu(tmp);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int machxo2_i2c_write(struct machxo2_common_priv *common,
> > > + struct machxo2_cmd *cmds, size_t
> > > cmd_count)
> > > +{
> > > + struct machxo2_i2c_priv *i2c_priv =
> > > to_machxo2_i2c_priv(common);
> > > + struct i2c_client *client = i2c_priv->client;
> > > + size_t i;
> > > + int ret;
> > > +
> > > + for (i = 0; i < cmd_count; i++) {
> > > + struct i2c_msg msg[] = {
> > > + {
> > > + .addr = client->addr,
> > > + .buf = cmds[i].cmd,
> > > + .len = cmds[i].cmd_len,
> > > + },
> > > + };
> > > +
> > > + ret = i2c_transfer(client->adapter, msg,
> > > ARRAY_SIZE(msg));
> > > + if (ret < 0)
> > > + return ret;
> > > + if (ret != ARRAY_SIZE(msg))
> > > + return -EIO;
> > > + if (cmds[i].delay_us)
> > > + usleep_range(cmds[i].delay_us,
> > > cmds[i].delay_us +
> > > + cmds[i].delay_us / 4);
> > > + if (i < cmd_count - 1) /* on any iteration except
> > > for the last one... */
> > > + ret = machxo2_wait_until_not_busy(common);
> >
> > Seems no need to implement the loop and wait in transportation layer,
> > they are common. A callback like write(bus, txbuf, n_tx) is better?
> >
> > Thanks,
> > Yilun
>
> I have chosen this implementation mostly due to the fact that I don't
> have a SPI machxo2 device to test against, so I am intentionally
> keeping changes to a minimum.
>
> Moving the wait between single commands into the transport layer is not
> functionally equivalent, e.g. the ISC_ENABLE - ISC_ERASE command
> sequence in the machxo2_write_init function would require two separate
> messages with a wait time between them, which would deassert the CS
> line between sending the messages via SPI if not sent as a sequence of
> SPI transfers. For some of the commands, the fpga requires a delay
> between the different commands, which was implemented by setting the
> delay property of the spi transfer objects in the original driver.
Not sure if it is really a problem, but I remember SPI has various APIs
to deal with different requirements.
>
> This implementation tries to mimic the timing behaviour of the SPI
> transfer delay property for the I2C implementation.
Could you firstly try on that until we have real problem? Ideally this
is a cleaner implementation, is it?
Thanks,
Yilun
>
> Best regards
> Johannes
>
> >
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int machxo2_i2c_probe(struct i2c_client *client,
> > > + const struct i2c_device_id *id)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct machxo2_i2c_priv *priv;
> > > +
> > > + priv = devm_kzalloc(dev, sizeof(struct machxo2_i2c_priv),
> > > GFP_KERNEL);
> > > + if (!priv)
> > > + return -ENOMEM;
> > > +
> > > + priv->client = client;
> > > + priv->common.get_status = machxo2_i2c_get_status;
> > > + priv->common.write_commands = machxo2_i2c_write;
> > > +
> > > + /* Commands are usually 4b, but these aren't for i2c */
> > > + priv->common.enable_3b = true;
> > > + priv->common.refresh_3b = true;
> > > +
> > > + return machxo2_common_init(&priv->common, dev);
> > > +}
> > > +
> > > +static const struct of_device_id of_match[] = {
> > > + { .compatible = "lattice,machxo2-slave-i2c", },
> > > + { },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, of_match);
> > > +
> > > +static const struct i2c_device_id lattice_ids[] = {
> > > + { "machxo2-slave-i2c", 0 },
> > > + { },
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, lattice_ids);
> > > +
> > > +static struct i2c_driver machxo2_i2c_driver = {
> > > + .driver = {
> > > + .name = "machxo2-slave-i2c",
> > > + .of_match_table = of_match_ptr(of_match),
> > > + },
> > > + .probe = machxo2_i2c_probe,
> > > + .id_table = lattice_ids,
> > > +};
> > > +
> > > +module_i2c_driver(machxo2_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Peter Jensen <pdj@bang-olufsen.dk>");
> > > +MODULE_DESCRIPTION("Load Lattice FPGA firmware over I2C");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 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 |
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 16/16] fpga: machxo2: add configuration over i2c
2022-08-29 14:45 ` Xu Yilun
@ 2022-08-31 16:07 ` Johannes Zink
0 siblings, 0 replies; 48+ messages in thread
From: Johannes Zink @ 2022-08-31 16:07 UTC (permalink / raw)
To: Xu Yilun
Cc: devicetree, linux-fpga, Rob Herring, Moritz Fischer, kernel, Wu Hao
Hi Yilun,
On Mon, 2022-08-29 at 22:45 +0800, Xu Yilun wrote:
> On 2022-08-29 at 15:21:19 +0200, Johannes Zink wrote:
> > Hi Yilun,
> >
> > On Mon, 2022-08-29 at 17:47 +0800, Xu Yilun wrote:
> > > On 2022-08-25 at 16:13:43 +0200, Johannes Zink wrote:
> > > > From: Peter Jensen <pdj@bang-olufsen.dk>
> > > >
> > > > The configuration flash of the machxo2 fpga can also be erased
> > > > and
> > > > written over i2c instead of spi. Add this functionality to the
> > > > refactored common driver. Since some commands are shorter over
> > > > I2C
> > > > than
> > > > they are over SPI some quirks are added to the common driver in
> > > > order to
> > > > account for that.
> > > >
> > > > Signed-off-by: Peter Jensen <pdj@bang-olufsen.dk>
> > > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > > ---
[snip]
> > > >
> >
> > >
> > > > +
> > > > +static int machxo2_i2c_write(struct machxo2_common_priv
> > > > *common,
> > > > + struct machxo2_cmd *cmds, size_t
> > > > cmd_count)
> > > > +{
> > > > + struct machxo2_i2c_priv *i2c_priv =
> > > > to_machxo2_i2c_priv(common);
> > > > + struct i2c_client *client = i2c_priv->client;
> > > > + size_t i;
> > > > + int ret;
> > > > +
> > > > + for (i = 0; i < cmd_count; i++) {
> > > > + struct i2c_msg msg[] = {
> > > > + {
> > > > + .addr = client->addr,
> > > > + .buf = cmds[i].cmd,
> > > > + .len = cmds[i].cmd_len,
> > > > + },
> > > > + };
> > > > +
> > > > + ret = i2c_transfer(client->adapter, msg,
> > > > ARRAY_SIZE(msg));
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + if (ret != ARRAY_SIZE(msg))
> > > > + return -EIO;
> > > > + if (cmds[i].delay_us)
> > > > + usleep_range(cmds[i].delay_us,
> > > > cmds[i].delay_us +
> > > > + cmds[i].delay_us / 4);
> > > > + if (i < cmd_count - 1) /* on any iteration
> > > > except
> > > > for the last one... */
> > > > + ret =
> > > > machxo2_wait_until_not_busy(common);
> > >
> > > Seems no need to implement the loop and wait in transportation
> > > layer,
> > > they are common. A callback like write(bus, txbuf, n_tx) is
> > > better?
> > >
> > > Thanks,
> > > Yilun
> >
> > I have chosen this implementation mostly due to the fact that I
> > don't
> > have a SPI machxo2 device to test against, so I am intentionally
> > keeping changes to a minimum.
> >
> > Moving the wait between single commands into the transport layer is
> > not
> > functionally equivalent, e.g. the ISC_ENABLE - ISC_ERASE command
> > sequence in the machxo2_write_init function would require two
> > separate
> > messages with a wait time between them, which would deassert the CS
> > line between sending the messages via SPI if not sent as a sequence
> > of
> > SPI transfers. For some of the commands, the fpga requires a delay
> > between the different commands, which was implemented by setting
> > the
> > delay property of the spi transfer objects in the original driver.
>
> Not sure if it is really a problem, but I remember SPI has various
> APIs
> to deal with different requirements.
I assume this could probably be implemented by clearing the cs_change
bit in the SPI transfer, though just sending multiple transfers in
sequence with the appropriate timing appears a bit more elegant to me,
since it doesn't reimplement the behaviour for spi, it simply extends
the i2c part for what is not supported natively in the i2c api. Either
way, some sort of waiting has to be implemented (please see my comment
below).
Also, please bear in mind that I do not have SPI connected on my board,
which is why I opted to stay as close as possible to the original
implementation and only refactor the spi transfers with functionally
equivalent code in order to keep the risk of breaking things as low as
possible.
>
> >
> > This implementation tries to mimic the timing behaviour of the SPI
> > transfer delay property for the I2C implementation.
>
> Could you firstly try on that until we have real problem? Ideally
> this
> is a cleaner implementation, is it?
The delays themselves are required by the device family datasheet and
by the sysConfig protocol definition, as part of the command sequence
timing.
Since I have no SPI connected on my hardware, I am only able to test
the I2C implementation, which works well with the timings taken from
the original spi driver (except for the erase timeout, which needs to
be extended as seen in Patch 15 of this series). Extending the delay
times such that usleep_range can be used has proven to be acceptable,
at least for the i2c implementation.
Best regards
Johannes
>
> Thanks,
> Yilun
>
> >
> > Best regards
> > Johannes
> >
> > >
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int machxo2_i2c_probe(struct i2c_client *client,
> > > > + const struct i2c_device_id *id)
> > > > +{
> > > > + struct device *dev = &client->dev;
> > > > + struct machxo2_i2c_priv *priv;
> > > > +
> > > > + priv = devm_kzalloc(dev, sizeof(struct
> > > > machxo2_i2c_priv),
> > > > GFP_KERNEL);
> > > > + if (!priv)
> > > > + return -ENOMEM;
> > > > +
> > > > + priv->client = client;
> > > > + priv->common.get_status = machxo2_i2c_get_status;
> > > > + priv->common.write_commands = machxo2_i2c_write;
> > > > +
> > > > + /* Commands are usually 4b, but these aren't for i2c */
> > > > + priv->common.enable_3b = true;
> > > > + priv->common.refresh_3b = true;
> > > > +
> > > > + return machxo2_common_init(&priv->common, dev);
> > > > +}
> > > > +
> > > > +static const struct of_device_id of_match[] = {
> > > > + { .compatible = "lattice,machxo2-slave-i2c", },
> > > > + { },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, of_match);
> > > > +
> > > > +static const struct i2c_device_id lattice_ids[] = {
> > > > + { "machxo2-slave-i2c", 0 },
> > > > + { },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(i2c, lattice_ids);
> > > > +
> > > > +static struct i2c_driver machxo2_i2c_driver = {
> > > > + .driver = {
> > > > + .name = "machxo2-slave-i2c",
> > > > + .of_match_table = of_match_ptr(of_match),
> > > > + },
> > > > + .probe = machxo2_i2c_probe,
> > > > + .id_table = lattice_ids,
> > > > +};
> > > > +
> > > > +module_i2c_driver(machxo2_i2c_driver);
> > > > +
> > > > +MODULE_AUTHOR("Peter Jensen <pdj@bang-olufsen.dk>");
> > > > +MODULE_DESCRIPTION("Load Lattice FPGA firmware over I2C");
> > > > +MODULE_LICENSE("GPL");
> > > > --
> > > > 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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-25 14:13 [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Johannes Zink
` (15 preceding siblings ...)
2022-08-25 14:13 ` [PATCH 16/16] fpga: machxo2: add configuration over i2c Johannes Zink
@ 2022-08-25 15:25 ` Ivan Bornyakov
2022-08-26 6:32 ` Johannes Zink
2022-08-26 8:25 ` Sascha Hauer
16 siblings, 2 replies; 48+ messages in thread
From: Ivan Bornyakov @ 2022-08-25 15:25 UTC (permalink / raw)
To: j.zink
Cc: Ivan Bornyakov, devicetree, hao.wu, kernel, linux-fpga, mdf,
robh+dt, yilun.xu
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.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-25 15:25 ` [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Ivan Bornyakov
@ 2022-08-26 6:32 ` Johannes Zink
2022-08-26 8:15 ` Ivan Bornyakov
2022-08-26 8:25 ` Sascha Hauer
1 sibling, 1 reply; 48+ messages in thread
From: Johannes Zink @ 2022-08-26 6:32 UTC (permalink / raw)
To: Ivan Bornyakov
Cc: devicetree, linux-fpga, robh+dt, mdf, kernel, yilun.xu, hao.wu
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
--
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-26 6:32 ` Johannes Zink
@ 2022-08-26 8:15 ` Ivan Bornyakov
0 siblings, 0 replies; 48+ messages in thread
From: Ivan Bornyakov @ 2022-08-26 8:15 UTC (permalink / raw)
To: Johannes Zink
Cc: devicetree, linux-fpga, robh+dt, mdf, kernel, yilun.xu, hao.wu
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 |
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-25 15:25 ` [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C Ivan Bornyakov
2022-08-26 6:32 ` Johannes Zink
@ 2022-08-26 8:25 ` Sascha Hauer
2022-08-26 9:00 ` Ivan Bornyakov
1 sibling, 1 reply; 48+ messages in thread
From: Sascha Hauer @ 2022-08-26 8:25 UTC (permalink / raw)
To: Ivan Bornyakov
Cc: j.zink, devicetree, linux-fpga, robh+dt, mdf, kernel, yilun.xu, hao.wu
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
--
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-26 8:25 ` Sascha Hauer
@ 2022-08-26 9:00 ` Ivan Bornyakov
2022-08-26 9:19 ` Ivan Bornyakov
0 siblings, 1 reply; 48+ messages in thread
From: Ivan Bornyakov @ 2022-08-26 9:00 UTC (permalink / raw)
To: Sascha Hauer
Cc: j.zink, devicetree, linux-fpga, robh+dt, mdf, kernel, yilun.xu, hao.wu
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-26 9:00 ` Ivan Bornyakov
@ 2022-08-26 9:19 ` Ivan Bornyakov
2022-08-26 15:26 ` Xu Yilun
0 siblings, 1 reply; 48+ messages in thread
From: Ivan Bornyakov @ 2022-08-26 9:19 UTC (permalink / raw)
To: Sascha Hauer
Cc: j.zink, devicetree, linux-fpga, robh+dt, mdf, kernel, yilun.xu, hao.wu
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 |
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] Add support for Lattice MachXO2 programming via I2C
2022-08-26 9:19 ` Ivan Bornyakov
@ 2022-08-26 15:26 ` Xu Yilun
0 siblings, 0 replies; 48+ messages in thread
From: Xu Yilun @ 2022-08-26 15:26 UTC (permalink / raw)
To: Ivan Bornyakov
Cc: Sascha Hauer, j.zink, devicetree, linux-fpga, robh+dt, mdf,
kernel, hao.wu
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 |
>
^ permalink raw reply [flat|nested] 48+ messages in thread