2022-12-19 15:07:01

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH 0/7] Support for Marvell modifications for Cadence XSPI

This patch series is fixing bugs, and adding support for
Marvell changes for Cadence XSPI IP.
It includes:
- Polling mode support
- Changes for modebyte handling
- Busycycles calculations
- Marvell specific IP changes

Witold Sadowski (7):
spi: cadence: Fix busy cycles calculation
spi: cadence: Change dt-bindings documentation for Cadence XSPI
controller
spi: cadence: Add polling mode support
spi: cadence: Change dt-bindings documentation for Cadence XSPI
controller
spi: cadence: Add read access size switch
spi: cadence: Add Marvell IP modification changes
spi: cadence: Force single modebyte

.../devicetree/bindings/spi/cdns,xspi.yaml | 6 +-
drivers/spi/Kconfig | 12 +
drivers/spi/spi-cadence-xspi.c | 360 ++++++++++++++++--
3 files changed, 354 insertions(+), 24 deletions(-)

--
2.17.1


2022-12-19 15:26:22

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH 4/7] spi: cadence: Change dt-bindings documentation for Cadence XSPI controller

Add parameter cdns,read-size.
Parameter is controlling SDMA read size length.

Signed-off-by: Witold Sadowski <[email protected]>
---
Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index f71a9c74e2ca..1274e3bf68e6 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -37,6 +37,10 @@ properties:
interrupts:
maxItems: 1

+ cdns,read-size:
+ items:
+ - description: size of single SDMA read operation
+
required:
- compatible
- reg
@@ -60,6 +64,7 @@ examples:
reg-names = "io", "sdma", "aux";
interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&gic>;
+ cdns,read-size=<0>;

flash@0 {
compatible = "jedec,spi-nor";
--
2.17.1

2022-12-19 15:56:57

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH 7/7] spi: cadence: Force single modebyte

During dummy-cycles xSPI will switch GPIO into Hi-Z mode.
To prevent unforeseen consequences of that behaviour, force send
single modebyte(0x00).
Modebyte will be send only if number of dummy-cycles is not equal
to 0. Code must also reduce dummycycle byte count by one - as one byte
is send as modebyte.

Signed-off-by: Witold Sadowski <[email protected]>
Reviewed-by: Chandrakala Chavva <[email protected]>
---
drivers/spi/spi-cadence-xspi.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index c73faf6b0546..7d467169f62b 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -146,6 +146,9 @@
#define CDNS_XSPI_STIG_DONE_FLAG BIT(0)
#define CDNS_XSPI_TRD_STATUS 0x0104

+#define MODE_NO_OF_BYTES GENMASK(25, 24)
+#define MODEBYTES_COUNT 1
+
/* Helper macros for filling command registers */
#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase) ( \
FIELD_PREP(CDNS_XSPI_CMD_INSTR_TYPE, (data_phase) ? \
@@ -158,9 +161,10 @@
FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR3, ((op)->addr.val >> 24) & 0xFF) | \
FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR4, ((op)->addr.val >> 32) & 0xFF))

-#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op) ( \
+#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op, modebytes) ( \
FIELD_PREP(CDNS_XSPI_CMD_P1_R3_ADDR5, ((op)->addr.val >> 40) & 0xFF) | \
FIELD_PREP(CDNS_XSPI_CMD_P1_R3_CMD, (op)->cmd.opcode) | \
+ FIELD_PREP(MODE_NO_OF_BYTES, modebytes) | \
FIELD_PREP(CDNS_XSPI_CMD_P1_R3_NUM_ADDR_BYTES, (op)->addr.nbytes))

#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_4(op, chipsel) ( \
@@ -174,12 +178,12 @@
#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_2(op) \
FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R2_DCNT_L, (op)->data.nbytes & 0xFFFF)

-#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op) ( \
+#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op, dummybytes) ( \
FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R3_DCNT_H, \
((op)->data.nbytes >> 16) & 0xffff) | \
FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R3_NUM_OF_DUMMY, \
(op)->dummy.buswidth != 0 ? \
- (((op)->dummy.nbytes * 8) / (op)->dummy.buswidth) : \
+ (((dummybytes) * 8) / (op)->dummy.buswidth) : \
0))

#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_4(op, chipsel) ( \
@@ -607,6 +611,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
u32 cmd_regs[6];
u32 cmd_status;
int ret;
+ int dummybytes = op->dummy.nbytes;

ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
if (ret < 0)
@@ -621,7 +626,12 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
memset(cmd_regs, 0, sizeof(cmd_regs));
cmd_regs[1] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase);
cmd_regs[2] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_2(op);
- cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op);
+ if (dummybytes != 0) {
+ cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op, 1);
+ dummybytes--;
+ } else {
+ cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op, 0);
+ }
cmd_regs[4] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_4(op,
cdns_xspi->cur_cs);

@@ -631,7 +641,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
cmd_regs[0] = CDNS_XSPI_STIG_DONE_FLAG;
cmd_regs[1] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_1(op);
cmd_regs[2] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_2(op);
- cmd_regs[3] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op);
+ cmd_regs[3] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op, dummybytes);
cmd_regs[4] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_4(op,
cdns_xspi->cur_cs);

--
2.17.1

2022-12-19 18:57:50

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 7/7] spi: cadence: Force single modebyte

On Mon, Dec 19, 2022 at 06:42:54AM -0800, Witold Sadowski wrote:
> During dummy-cycles xSPI will switch GPIO into Hi-Z mode.
> To prevent unforeseen consequences of that behaviour, force send
> single modebyte(0x00).

Fixes should go at the start of the patch series so they don't depend on
any new features and can be sent as fixes.


Attachments:
(No filename) (341.00 B)
signature.asc (499.00 B)
Download all attachments

2022-12-19 19:10:19

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/7] spi: cadence: Change dt-bindings documentation for Cadence XSPI controller

On Mon, Dec 19, 2022 at 06:42:51AM -0800, Witold Sadowski wrote:

> Add parameter cdns,read-size.
> Parameter is controlling SDMA read size length.

Why is this something we would want to configure statically in DT?


Attachments:
(No filename) (222.00 B)
signature.asc (499.00 B)
Download all attachments

2022-12-19 21:38:42

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 4/7] spi: cadence: Change dt-bindings documentation for Cadence XSPI controller


On Mon, 19 Dec 2022 06:42:51 -0800, Witold Sadowski wrote:
> Add parameter cdns,read-size.
> Parameter is controlling SDMA read size length.
>
> Signed-off-by: Witold Sadowski <[email protected]>
> ---
> Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 5 +++++
> 1 file changed, 5 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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: properties:cdns,read-size: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
'description' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('items' was unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: properties:cdns,read-size: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: properties:cdns,read-size: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2022-12-20 14:23:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] spi: cadence: Change dt-bindings documentation for Cadence XSPI controller

On 19/12/2022 15:42, Witold Sadowski wrote:
> Add parameter cdns,read-size.
> Parameter is controlling SDMA read size length.

Use subject prefixes matching the subsystem (git log --oneline -- ...).

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

>
> Signed-off-by: Witold Sadowski <[email protected]>
> ---
> Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index f71a9c74e2ca..1274e3bf68e6 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -37,6 +37,10 @@ properties:
> interrupts:
> maxItems: 1
>
> + cdns,read-size:
> + items:
> + - description: size of single SDMA read operation

Why is this a property of DT?

> +
> required:
> - compatible
> - reg
> @@ -60,6 +64,7 @@ examples:
> reg-names = "io", "sdma", "aux";
> interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-parent = <&gic>;
> + cdns,read-size=<0>;

That's not DT coding style.

>
> flash@0 {
> compatible = "jedec,spi-nor";

Best regards,
Krzysztof

2022-12-27 12:27:12

by Mark Brown

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/7] Support for Marvell modifications for Cadence XSPI

On Mon, 19 Dec 2022 06:42:47 -0800, Witold Sadowski wrote:
> This patch series is fixing bugs, and adding support for
> Marvell changes for Cadence XSPI IP.
> It includes:
> - Polling mode support
> - Changes for modebyte handling
> - Busycycles calculations
> - Marvell specific IP changes
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/7] spi: cadence: Fix busy cycles calculation
commit: e8bb8f19e73a1e855e54788f8673b9b49e46b5cd

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2024-04-29 15:20:28

by Witold Sadowski

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH 4/7] spi: cadence: Change dt-bindings documentation for Cadence XSPI controller

> ----------------------------------------------------------------------
> On 19/12/2022 15:42, Witold Sadowski wrote:
> > Add parameter cdns,read-size.
> > Parameter is controlling SDMA read size length.
>
> Use subject prefixes matching the subsystem (git log --oneline -- ...).
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> >
> > Signed-off-by: Witold Sadowski <[email protected]>
> > ---
> > Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > index f71a9c74e2ca..1274e3bf68e6 100644
> > --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > @@ -37,6 +37,10 @@ properties:
> > interrupts:
> > maxItems: 1
> >
> > + cdns,read-size:
> > + items:
> > + - description: size of single SDMA read operation
>
> Why is this a property of DT?

Removed, read size check will be based on matching.
The problem is internal bus connection to xSPI IP.

>
> > +
> > required:
> > - compatible
> > - reg
> > @@ -60,6 +64,7 @@ examples:
> > reg-names = "io", "sdma", "aux";
> > interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
> > interrupt-parent = <&gic>;
> > + cdns,read-size=<0>;
>
> That's not DT coding style.
>
> >
> > flash@0 {
> > compatible = "jedec,spi-nor";
>
> Best regards,
> Krzysztof

Regards
Witek