* [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits
@ 2021-11-17 9:37 Wolfram Sang
2021-11-18 2:54 ` Prabhakar Mahadev Lad
2021-11-22 9:54 ` Krzysztof Kozlowski
0 siblings, 2 replies; 4+ messages in thread
From: Wolfram Sang @ 2021-11-17 9:37 UTC (permalink / raw)
To: linux-renesas-soc
Cc: Lad Prabhakar, Wolfram Sang, Krzysztof Kozlowski, linux-kernel
Instead of writing fixed values with undocumented bits which happen to
be set on some SoCs, better switch to read-modify-write operations
changing only bits which are documented. This is way more future-proof
as we don't know yet how these bits may be on upcoming SoCs.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
This patch is based on the Krzysztof's for-next branch as it depends on
Prabhakar's G2L additions.
Tested on a V3U with some debug code verifying that the same values are
written before and after the patch.
@Prabhakar: would you be so kind and test this patch on G2L?
drivers/memory/renesas-rpc-if.c | 58 +++++++++++++--------------------
1 file changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 8c51145c0f5c..a9f3e7bdddf8 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -20,7 +20,6 @@
#define RPCIF_CMNCR 0x0000 /* R/W */
#define RPCIF_CMNCR_MD BIT(31)
-#define RPCIF_CMNCR_SFDE BIT(24) /* undocumented but must be set */
#define RPCIF_CMNCR_MOIIO3(val) (((val) & 0x3) << 22)
#define RPCIF_CMNCR_MOIIO2(val) (((val) & 0x3) << 20)
#define RPCIF_CMNCR_MOIIO1(val) (((val) & 0x3) << 18)
@@ -290,49 +289,36 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
rpcif_rzg2l_timing_adjust_sdr(rpc);
}
- /*
- * NOTE: The 0x260 are undocumented bits, but they must be set.
- * RPCIF_PHYCNT_STRTIM is strobe timing adjustment bits,
- * 0x0 : the delay is biggest,
- * 0x1 : the delay is 2nd biggest,
- * On H3 ES1.x, the value should be 0, while on others,
- * the value should be 7.
- */
- if (rpc->type == RPCIF_RCAR_GEN3) {
- regmap_write(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_STRTIM(7) |
- RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260);
- } else {
- regmap_read(rpc->regmap, RPCIF_PHYCNT, &dummy);
- dummy &= ~RPCIF_PHYCNT_PHYMEM_MASK;
- dummy |= RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260;
- regmap_write(rpc->regmap, RPCIF_PHYCNT, dummy);
- }
+ regmap_update_bits(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_PHYMEM_MASK,
+ RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0));
+
+ if (rpc->type == RPCIF_RCAR_GEN3)
+ regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
+ RPCIF_PHYCNT_STRTIM(7), RPCIF_PHYCNT_STRTIM(7));
- /*
- * NOTE: The 0x1511144 are undocumented bits, but they must be set
- * for RPCIF_PHYOFFSET1.
- * The 0x31 are undocumented bits, but they must be set
- * for RPCIF_PHYOFFSET2.
- */
- regmap_write(rpc->regmap, RPCIF_PHYOFFSET1, 0x1511144 |
- RPCIF_PHYOFFSET1_DDRTMG(3));
- regmap_write(rpc->regmap, RPCIF_PHYOFFSET2, 0x31 |
- RPCIF_PHYOFFSET2_OCTTMG(4));
+ regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET1, RPCIF_PHYOFFSET1_DDRTMG(3),
+ RPCIF_PHYOFFSET1_DDRTMG(3));
+ regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET2, RPCIF_PHYOFFSET2_OCTTMG(7),
+ RPCIF_PHYOFFSET2_OCTTMG(4));
if (hyperflash)
regmap_update_bits(rpc->regmap, RPCIF_PHYINT,
RPCIF_PHYINT_WPVAL, 0);
if (rpc->type == RPCIF_RCAR_GEN3)
- regmap_write(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_SFDE |
- RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_IOFV_HIZ |
- RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
+ regmap_update_bits(rpc->regmap, RPCIF_CMNCR,
+ RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_BSZ(3),
+ RPCIF_CMNCR_MOIIO_HIZ |
+ RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
else
- regmap_write(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_SFDE |
- RPCIF_CMNCR_MOIIO3(1) | RPCIF_CMNCR_MOIIO2(1) |
- RPCIF_CMNCR_MOIIO1(1) | RPCIF_CMNCR_MOIIO0(1) |
- RPCIF_CMNCR_IO3FV(2) | RPCIF_CMNCR_IO2FV(2) |
- RPCIF_CMNCR_IO0FV(2) | RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
+ regmap_update_bits(rpc->regmap, RPCIF_CMNCR,
+ RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_IOFV_HIZ |
+ RPCIF_CMNCR_BSZ(3),
+ RPCIF_CMNCR_MOIIO3(1) | RPCIF_CMNCR_MOIIO2(1) |
+ RPCIF_CMNCR_MOIIO1(1) | RPCIF_CMNCR_MOIIO0(1) |
+ RPCIF_CMNCR_IO3FV(2) | RPCIF_CMNCR_IO2FV(2) |
+ RPCIF_CMNCR_IO0FV(2) |
+ RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
/* Set RCF after BSZ update */
regmap_write(rpc->regmap, RPCIF_DRCR, RPCIF_DRCR_RCF);
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits
2021-11-17 9:37 [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits Wolfram Sang
@ 2021-11-18 2:54 ` Prabhakar Mahadev Lad
2021-11-18 13:06 ` Wolfram Sang
2021-11-22 9:54 ` Krzysztof Kozlowski
1 sibling, 1 reply; 4+ messages in thread
From: Prabhakar Mahadev Lad @ 2021-11-18 2:54 UTC (permalink / raw)
To: Wolfram Sang, linux-renesas-soc; +Cc: Krzysztof Kozlowski, linux-kernel
Hi Wolfram,
Thank you for the patch.
> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: 17 November 2021 09:37
> To: linux-renesas-soc@vger.kernel.org
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Wolfram Sang <wsa+renesas@sang-
> engineering.com>; Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits
>
> Instead of writing fixed values with undocumented bits which happen to be set on some SoCs, better
> switch to read-modify-write operations changing only bits which are documented. This is way more
> future-proof as we don't know yet how these bits may be on upcoming SoCs.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> This patch is based on the Krzysztof's for-next branch as it depends on Prabhakar's G2L additions.
>
> Tested on a V3U with some debug code verifying that the same values are written before and after the
> patch.
>
> @Prabhakar: would you be so kind and test this patch on G2L?
>
Dumped the registers before and after patch, register values do match. Also tested read/write operation on RZ/G2L.
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cheers,
Prabhakar
> drivers/memory/renesas-rpc-if.c | 58 +++++++++++++--------------------
> 1 file changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c index
> 8c51145c0f5c..a9f3e7bdddf8 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
> @@ -20,7 +20,6 @@
>
> #define RPCIF_CMNCR 0x0000 /* R/W */
> #define RPCIF_CMNCR_MD BIT(31)
> -#define RPCIF_CMNCR_SFDE BIT(24) /* undocumented but must be set */
> #define RPCIF_CMNCR_MOIIO3(val) (((val) & 0x3) << 22)
> #define RPCIF_CMNCR_MOIIO2(val) (((val) & 0x3) << 20)
> #define RPCIF_CMNCR_MOIIO1(val) (((val) & 0x3) << 18)
> @@ -290,49 +289,36 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> rpcif_rzg2l_timing_adjust_sdr(rpc);
> }
>
> - /*
> - * NOTE: The 0x260 are undocumented bits, but they must be set.
> - * RPCIF_PHYCNT_STRTIM is strobe timing adjustment bits,
> - * 0x0 : the delay is biggest,
> - * 0x1 : the delay is 2nd biggest,
> - * On H3 ES1.x, the value should be 0, while on others,
> - * the value should be 7.
> - */
> - if (rpc->type == RPCIF_RCAR_GEN3) {
> - regmap_write(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_STRTIM(7) |
> - RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260);
> - } else {
> - regmap_read(rpc->regmap, RPCIF_PHYCNT, &dummy);
> - dummy &= ~RPCIF_PHYCNT_PHYMEM_MASK;
> - dummy |= RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260;
> - regmap_write(rpc->regmap, RPCIF_PHYCNT, dummy);
> - }
> + regmap_update_bits(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_PHYMEM_MASK,
> + RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0));
> +
> + if (rpc->type == RPCIF_RCAR_GEN3)
> + regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> + RPCIF_PHYCNT_STRTIM(7), RPCIF_PHYCNT_STRTIM(7));
>
> - /*
> - * NOTE: The 0x1511144 are undocumented bits, but they must be set
> - * for RPCIF_PHYOFFSET1.
> - * The 0x31 are undocumented bits, but they must be set
> - * for RPCIF_PHYOFFSET2.
> - */
> - regmap_write(rpc->regmap, RPCIF_PHYOFFSET1, 0x1511144 |
> - RPCIF_PHYOFFSET1_DDRTMG(3));
> - regmap_write(rpc->regmap, RPCIF_PHYOFFSET2, 0x31 |
> - RPCIF_PHYOFFSET2_OCTTMG(4));
> + regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET1, RPCIF_PHYOFFSET1_DDRTMG(3),
> + RPCIF_PHYOFFSET1_DDRTMG(3));
> + regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET2, RPCIF_PHYOFFSET2_OCTTMG(7),
> + RPCIF_PHYOFFSET2_OCTTMG(4));
>
> if (hyperflash)
> regmap_update_bits(rpc->regmap, RPCIF_PHYINT,
> RPCIF_PHYINT_WPVAL, 0);
>
> if (rpc->type == RPCIF_RCAR_GEN3)
> - regmap_write(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_SFDE |
> - RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_IOFV_HIZ |
> - RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
> + regmap_update_bits(rpc->regmap, RPCIF_CMNCR,
> + RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_BSZ(3),
> + RPCIF_CMNCR_MOIIO_HIZ |
> + RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
> else
> - regmap_write(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_SFDE |
> - RPCIF_CMNCR_MOIIO3(1) | RPCIF_CMNCR_MOIIO2(1) |
> - RPCIF_CMNCR_MOIIO1(1) | RPCIF_CMNCR_MOIIO0(1) |
> - RPCIF_CMNCR_IO3FV(2) | RPCIF_CMNCR_IO2FV(2) |
> - RPCIF_CMNCR_IO0FV(2) | RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
> + regmap_update_bits(rpc->regmap, RPCIF_CMNCR,
> + RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_IOFV_HIZ |
> + RPCIF_CMNCR_BSZ(3),
> + RPCIF_CMNCR_MOIIO3(1) | RPCIF_CMNCR_MOIIO2(1) |
> + RPCIF_CMNCR_MOIIO1(1) | RPCIF_CMNCR_MOIIO0(1) |
> + RPCIF_CMNCR_IO3FV(2) | RPCIF_CMNCR_IO2FV(2) |
> + RPCIF_CMNCR_IO0FV(2) |
> + RPCIF_CMNCR_BSZ(hyperflash ? 1 : 0));
>
> /* Set RCF after BSZ update */
> regmap_write(rpc->regmap, RPCIF_DRCR, RPCIF_DRCR_RCF);
> --
> 2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits
2021-11-18 2:54 ` Prabhakar Mahadev Lad
@ 2021-11-18 13:06 ` Wolfram Sang
0 siblings, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2021-11-18 13:06 UTC (permalink / raw)
To: Prabhakar Mahadev Lad
Cc: linux-renesas-soc, Krzysztof Kozlowski, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 330 bytes --]
> Dumped the registers before and after patch, register values do match. Also tested read/write operation on RZ/G2L.
>
> Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Awesome, thanks for the lightning fast testing and review!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits
2021-11-17 9:37 [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits Wolfram Sang
2021-11-18 2:54 ` Prabhakar Mahadev Lad
@ 2021-11-22 9:54 ` Krzysztof Kozlowski
1 sibling, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2021-11-22 9:54 UTC (permalink / raw)
To: linux-renesas-soc, Wolfram Sang
Cc: Krzysztof Kozlowski, Lad Prabhakar, linux-kernel
On Wed, 17 Nov 2021 10:37:10 +0100, Wolfram Sang wrote:
> Instead of writing fixed values with undocumented bits which happen to
> be set on some SoCs, better switch to read-modify-write operations
> changing only bits which are documented. This is way more future-proof
> as we don't know yet how these bits may be on upcoming SoCs.
>
>
Applied, thanks!
[1/1] memory: renesas-rpc-if: avoid use of undocumented bits
commit: 57ea9daad51f7707f61a602a743decf10cf9fea9
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-22 9:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 9:37 [PATCH] memory: renesas-rpc-if: avoid use of undocumented bits Wolfram Sang
2021-11-18 2:54 ` Prabhakar Mahadev Lad
2021-11-18 13:06 ` Wolfram Sang
2021-11-22 9:54 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).