2023-01-26 17:35:36

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH v3] memory: renesas-rpc-if: Fix PHYCNT.STRTIM setting

According to the datasheets, the Strobe Timing Adjustment bit (STRTIM)
setting is different on R-Car SoCs, i.e.

R-Car M3 ES1.* : STRTIM[2:0] is set to 0x6
other R-Car Gen3: STRTIM[2:0] is set to 0x7
other R-Car Gen4: STRTIM[3:0] is set to 0xf

To fix this issue, a DT match data was added to specify the setting
for special use cases.

Signed-off-by: Cong Dang <[email protected]>
Signed-off-by: Hai Pham <[email protected]>
[wsa: rebased, restructured, added Gen4 support]
Signed-off-by: Wolfram Sang <[email protected]>
---

Changes since v2:
* dropped support for H3 ES1 (we don't support it upstream anymore)
* M3-W ES1 now handled via 'compatible' (Thanks, Geert!)
* -> dropped soc_device_match()
* removed old rpcif_type entry in struct rpcif (Thanks, Prabhakar)

drivers/memory/renesas-rpc-if.c | 45 +++++++++++++++++++++++----------
include/memory/renesas-rpc-if.h | 7 ++++-
2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index c36b407851ff..1dc684a93ff4 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -7,6 +7,7 @@
* Copyright (C) 2019-2020 Cogent Embedded, Inc.
*/

+#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -163,6 +164,25 @@ static const struct regmap_access_table rpcif_volatile_table = {
.n_yes_ranges = ARRAY_SIZE(rpcif_volatile_ranges),
};

+static const struct rpcif_info rpcif_info_r8a7796 = {
+ .type = RPCIF_RCAR_GEN3,
+ .strtim = 6,
+};
+
+static const struct rpcif_info rpcif_info_gen3 = {
+ .type = RPCIF_RCAR_GEN3,
+ .strtim = 7,
+};
+
+static const struct rpcif_info rpcif_info_rz_g2l = {
+ .type = RPCIF_RZ_G2L,
+ .strtim = 7,
+};
+
+static const struct rpcif_info rpcif_info_gen4 = {
+ .type = RPCIF_RCAR_GEN4,
+ .strtim = 15,
+};

/*
* Custom accessor functions to ensure SM[RW]DR[01] are always accessed with
@@ -276,9 +296,9 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
rpc->dirmap = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(rpc->dirmap))
return PTR_ERR(rpc->dirmap);
- rpc->size = resource_size(res);

- rpc->type = (uintptr_t)of_device_get_match_data(dev);
+ rpc->size = resource_size(res);
+ rpc->info = of_device_get_match_data(dev);
rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);

return PTR_ERR_OR_ZERO(rpc->rstc);
@@ -305,7 +325,7 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)

pm_runtime_get_sync(rpc->dev);

- if (rpc->type == RPCIF_RZ_G2L) {
+ if (rpc->info->type == RPCIF_RZ_G2L) {
int ret;

ret = reset_control_reset(rpc->rstc);
@@ -321,12 +341,10 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
/* DMA Transfer is not supported */
regmap_update_bits(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_HS, 0);

- if (rpc->type == RPCIF_RCAR_GEN3)
- regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
- RPCIF_PHYCNT_STRTIM(7), RPCIF_PHYCNT_STRTIM(7));
- else if (rpc->type == RPCIF_RCAR_GEN4)
- regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
- RPCIF_PHYCNT_STRTIM(15), RPCIF_PHYCNT_STRTIM(15));
+ regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
+ /* create mask with all affected bits set */
+ RPCIF_PHYCNT_STRTIM(BIT(fls(rpc->info->strtim)) - 1),
+ RPCIF_PHYCNT_STRTIM(rpc->info->strtim));

regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET1, RPCIF_PHYOFFSET1_DDRTMG(3),
RPCIF_PHYOFFSET1_DDRTMG(3));
@@ -337,7 +355,7 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
regmap_update_bits(rpc->regmap, RPCIF_PHYINT,
RPCIF_PHYINT_WPVAL, 0);

- if (rpc->type == RPCIF_RZ_G2L)
+ if (rpc->info->type == RPCIF_RZ_G2L)
regmap_update_bits(rpc->regmap, RPCIF_CMNCR,
RPCIF_CMNCR_MOIIO(3) | RPCIF_CMNCR_IOFV(3) |
RPCIF_CMNCR_BSZ(3),
@@ -720,9 +738,10 @@ static int rpcif_remove(struct platform_device *pdev)
}

static const struct of_device_id rpcif_of_match[] = {
- { .compatible = "renesas,rcar-gen3-rpc-if", .data = (void *)RPCIF_RCAR_GEN3 },
- { .compatible = "renesas,rcar-gen4-rpc-if", .data = (void *)RPCIF_RCAR_GEN4 },
- { .compatible = "renesas,rzg2l-rpc-if", .data = (void *)RPCIF_RZ_G2L },
+ { .compatible = "renesas,r8a7796-rpc-if", .data = &rpcif_info_r8a7796 },
+ { .compatible = "renesas,rcar-gen3-rpc-if", .data = &rpcif_info_gen3 },
+ { .compatible = "renesas,rcar-gen4-rpc-if", .data = &rpcif_info_gen4 },
+ { .compatible = "renesas,rzg2l-rpc-if", .data = &rpcif_info_rz_g2l },
{},
};
MODULE_DEVICE_TABLE(of, rpcif_of_match);
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index 862eff613dc7..405011008a5a 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -63,6 +63,11 @@ enum rpcif_type {
RPCIF_RZ_G2L,
};

+struct rpcif_info {
+ enum rpcif_type type;
+ u8 strtim;
+};
+
struct rpcif {
struct device *dev;
void __iomem *base;
@@ -70,7 +75,7 @@ struct rpcif {
struct regmap *regmap;
struct reset_control *rstc;
size_t size;
- enum rpcif_type type;
+ const struct rpcif_info *info;
enum rpcif_data_dir dir;
u8 bus_size;
u8 xfer_size;
--
2.30.2



2023-01-27 08:17:44

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v3] memory: renesas-rpc-if: Fix PHYCNT.STRTIM setting

Hi Wolfram,

On Thu, Jan 26, 2023 at 6:41 PM Wolfram Sang
<[email protected]> wrote:
> According to the datasheets, the Strobe Timing Adjustment bit (STRTIM)
> setting is different on R-Car SoCs, i.e.
>
> R-Car M3 ES1.* : STRTIM[2:0] is set to 0x6
> other R-Car Gen3: STRTIM[2:0] is set to 0x7
> other R-Car Gen4: STRTIM[3:0] is set to 0xf
>
> To fix this issue, a DT match data was added to specify the setting
> for special use cases.
>
> Signed-off-by: Cong Dang <[email protected]>
> Signed-off-by: Hai Pham <[email protected]>
> [wsa: rebased, restructured, added Gen4 support]
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Changes since v2:
> * dropped support for H3 ES1 (we don't support it upstream anymore)
> * M3-W ES1 now handled via 'compatible' (Thanks, Geert!)
> * -> dropped soc_device_match()
> * removed old rpcif_type entry in struct rpcif (Thanks, Prabhakar)

Thanks for the update!

> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c

> @@ -276,9 +296,9 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
> rpc->dirmap = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(rpc->dirmap))
> return PTR_ERR(rpc->dirmap);
> - rpc->size = resource_size(res);
>
> - rpc->type = (uintptr_t)of_device_get_match_data(dev);
> + rpc->size = resource_size(res);

Nit: No need to move the initialization of rpc->size.

> + rpc->info = of_device_get_match_data(dev);
> rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>
> return PTR_ERR_OR_ZERO(rpc->rstc);

> @@ -321,12 +341,10 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> /* DMA Transfer is not supported */
> regmap_update_bits(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_HS, 0);
>
> - if (rpc->type == RPCIF_RCAR_GEN3)
> - regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> - RPCIF_PHYCNT_STRTIM(7), RPCIF_PHYCNT_STRTIM(7));
> - else if (rpc->type == RPCIF_RCAR_GEN4)
> - regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> - RPCIF_PHYCNT_STRTIM(15), RPCIF_PHYCNT_STRTIM(15));
> + regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> + /* create mask with all affected bits set */
> + RPCIF_PHYCNT_STRTIM(BIT(fls(rpc->info->strtim)) - 1),

Note that this relies on the strtim value being 4 or higher.
As this value is not user-configurable, but fixed in the driver,
it's probably OK.

> + RPCIF_PHYCNT_STRTIM(rpc->info->strtim));
>
> regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET1, RPCIF_PHYOFFSET1_DDRTMG(3),
> RPCIF_PHYOFFSET1_DDRTMG(3));

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2023-01-27 08:31:08

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v3] memory: renesas-rpc-if: Fix PHYCNT.STRTIM setting


> > - rpc->type = (uintptr_t)of_device_get_match_data(dev);
> > + rpc->size = resource_size(res);
>
> Nit: No need to move the initialization of rpc->size.

True. It is more cosmetic to have all rpc->* inits in one block. But I
think it is worth it.

> > + regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> > + /* create mask with all affected bits set */
> > + RPCIF_PHYCNT_STRTIM(BIT(fls(rpc->info->strtim)) - 1),
>
> Note that this relies on the strtim value being 4 or higher.
> As this value is not user-configurable, but fixed in the driver,
> it's probably OK.

If there was a chance that newer SoCs use such a value, I'd go for the
seperate mask. But I am confident this time has passed and the next
value is likely >15.


> Reviewed-by: Geert Uytterhoeven <[email protected]>

Thanks!


Attachments:
(No filename) (870.00 B)
signature.asc (833.00 B)
Download all attachments

2023-01-27 10:45:51

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3] memory: renesas-rpc-if: Fix PHYCNT.STRTIM setting

On Thu, Jan 26, 2023 at 5:35 PM Wolfram Sang
<[email protected]> wrote:
>
> According to the datasheets, the Strobe Timing Adjustment bit (STRTIM)
> setting is different on R-Car SoCs, i.e.
>
> R-Car M3 ES1.* : STRTIM[2:0] is set to 0x6
> other R-Car Gen3: STRTIM[2:0] is set to 0x7
> other R-Car Gen4: STRTIM[3:0] is set to 0xf
>
> To fix this issue, a DT match data was added to specify the setting
> for special use cases.
>
> Signed-off-by: Cong Dang <[email protected]>
> Signed-off-by: Hai Pham <[email protected]>
> [wsa: rebased, restructured, added Gen4 support]
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Changes since v2:
> * dropped support for H3 ES1 (we don't support it upstream anymore)
> * M3-W ES1 now handled via 'compatible' (Thanks, Geert!)
> * -> dropped soc_device_match()
> * removed old rpcif_type entry in struct rpcif (Thanks, Prabhakar)
>
> drivers/memory/renesas-rpc-if.c | 45 +++++++++++++++++++++++----------
> include/memory/renesas-rpc-if.h | 7 ++++-
> 2 files changed, 38 insertions(+), 14 deletions(-)
>
Tested on RZ/G2L SMARC EVK.

Tested-by: Lad Prabhakar <[email protected]>

Cheers,
Prabhakar

> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index c36b407851ff..1dc684a93ff4 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
> @@ -7,6 +7,7 @@
> * Copyright (C) 2019-2020 Cogent Embedded, Inc.
> */
>
> +#include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/io.h>
> #include <linux/module.h>
> @@ -163,6 +164,25 @@ static const struct regmap_access_table rpcif_volatile_table = {
> .n_yes_ranges = ARRAY_SIZE(rpcif_volatile_ranges),
> };
>
> +static const struct rpcif_info rpcif_info_r8a7796 = {
> + .type = RPCIF_RCAR_GEN3,
> + .strtim = 6,
> +};
> +
> +static const struct rpcif_info rpcif_info_gen3 = {
> + .type = RPCIF_RCAR_GEN3,
> + .strtim = 7,
> +};
> +
> +static const struct rpcif_info rpcif_info_rz_g2l = {
> + .type = RPCIF_RZ_G2L,
> + .strtim = 7,
> +};
> +
> +static const struct rpcif_info rpcif_info_gen4 = {
> + .type = RPCIF_RCAR_GEN4,
> + .strtim = 15,
> +};
>
> /*
> * Custom accessor functions to ensure SM[RW]DR[01] are always accessed with
> @@ -276,9 +296,9 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
> rpc->dirmap = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(rpc->dirmap))
> return PTR_ERR(rpc->dirmap);
> - rpc->size = resource_size(res);
>
> - rpc->type = (uintptr_t)of_device_get_match_data(dev);
> + rpc->size = resource_size(res);
> + rpc->info = of_device_get_match_data(dev);
> rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>
> return PTR_ERR_OR_ZERO(rpc->rstc);
> @@ -305,7 +325,7 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
>
> pm_runtime_get_sync(rpc->dev);
>
> - if (rpc->type == RPCIF_RZ_G2L) {
> + if (rpc->info->type == RPCIF_RZ_G2L) {
> int ret;
>
> ret = reset_control_reset(rpc->rstc);
> @@ -321,12 +341,10 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> /* DMA Transfer is not supported */
> regmap_update_bits(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_HS, 0);
>
> - if (rpc->type == RPCIF_RCAR_GEN3)
> - regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> - RPCIF_PHYCNT_STRTIM(7), RPCIF_PHYCNT_STRTIM(7));
> - else if (rpc->type == RPCIF_RCAR_GEN4)
> - regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> - RPCIF_PHYCNT_STRTIM(15), RPCIF_PHYCNT_STRTIM(15));
> + regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> + /* create mask with all affected bits set */
> + RPCIF_PHYCNT_STRTIM(BIT(fls(rpc->info->strtim)) - 1),
> + RPCIF_PHYCNT_STRTIM(rpc->info->strtim));
>
> regmap_update_bits(rpc->regmap, RPCIF_PHYOFFSET1, RPCIF_PHYOFFSET1_DDRTMG(3),
> RPCIF_PHYOFFSET1_DDRTMG(3));
> @@ -337,7 +355,7 @@ int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> regmap_update_bits(rpc->regmap, RPCIF_PHYINT,
> RPCIF_PHYINT_WPVAL, 0);
>
> - if (rpc->type == RPCIF_RZ_G2L)
> + if (rpc->info->type == RPCIF_RZ_G2L)
> regmap_update_bits(rpc->regmap, RPCIF_CMNCR,
> RPCIF_CMNCR_MOIIO(3) | RPCIF_CMNCR_IOFV(3) |
> RPCIF_CMNCR_BSZ(3),
> @@ -720,9 +738,10 @@ static int rpcif_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id rpcif_of_match[] = {
> - { .compatible = "renesas,rcar-gen3-rpc-if", .data = (void *)RPCIF_RCAR_GEN3 },
> - { .compatible = "renesas,rcar-gen4-rpc-if", .data = (void *)RPCIF_RCAR_GEN4 },
> - { .compatible = "renesas,rzg2l-rpc-if", .data = (void *)RPCIF_RZ_G2L },
> + { .compatible = "renesas,r8a7796-rpc-if", .data = &rpcif_info_r8a7796 },
> + { .compatible = "renesas,rcar-gen3-rpc-if", .data = &rpcif_info_gen3 },
> + { .compatible = "renesas,rcar-gen4-rpc-if", .data = &rpcif_info_gen4 },
> + { .compatible = "renesas,rzg2l-rpc-if", .data = &rpcif_info_rz_g2l },
> {},
> };
> MODULE_DEVICE_TABLE(of, rpcif_of_match);
> diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
> index 862eff613dc7..405011008a5a 100644
> --- a/include/memory/renesas-rpc-if.h
> +++ b/include/memory/renesas-rpc-if.h
> @@ -63,6 +63,11 @@ enum rpcif_type {
> RPCIF_RZ_G2L,
> };
>
> +struct rpcif_info {
> + enum rpcif_type type;
> + u8 strtim;
> +};
> +
> struct rpcif {
> struct device *dev;
> void __iomem *base;
> @@ -70,7 +75,7 @@ struct rpcif {
> struct regmap *regmap;
> struct reset_control *rstc;
> size_t size;
> - enum rpcif_type type;
> + const struct rpcif_info *info;
> enum rpcif_data_dir dir;
> u8 bus_size;
> u8 xfer_size;
> --
> 2.30.2
>