2023-01-18 12:34:32

by Wolfram Sang

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

From: Cong Dang <[email protected]>

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

R-Car H3 ES1.* : STRTIM[2:0] is set to 0x0
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 a little, added Gen4 support]
Signed-off-by: Wolfram Sang <[email protected]>
---

Change since V1:

* use proper mask when updating STRTIM bits (thanks, Geert!)

look for 'RPCIF_PHYCNT_STRTIM', there is the change. Rest is the same.

drivers/memory/renesas-rpc-if.c | 63 ++++++++++++++++++++++++++-------
include/memory/renesas-rpc-if.h | 6 ++++
2 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index c36b407851ff..845b535a5350 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>
@@ -15,6 +16,7 @@
#include <linux/of_device.h>
#include <linux/regmap.h>
#include <linux/reset.h>
+#include <linux/sys_soc.h>

#include <memory/renesas-rpc-if.h>

@@ -163,6 +165,36 @@ static const struct regmap_access_table rpcif_volatile_table = {
.n_yes_ranges = ARRAY_SIZE(rpcif_volatile_ranges),
};

+static const struct rpcif_info rpcif_info_r8a7795_es1 = {
+ .type = RPCIF_RCAR_GEN3,
+ .strtim = 0,
+};
+
+static const struct rpcif_info rpcif_info_r8a7796_es1 = {
+ .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,
+};
+
+static const struct soc_device_attribute rpcif_info_match[] = {
+ { .soc_id = "r8a7795", .revision = "ES1.*", .data = &rpcif_info_r8a7795_es1 },
+ { .soc_id = "r8a7796", .revision = "ES1.*", .data = &rpcif_info_r8a7796_es1 },
+ { /* Sentinel. */ }
+};

/*
* Custom accessor functions to ensure SM[RW]DR[01] are always accessed with
@@ -256,6 +288,8 @@ static const struct regmap_config rpcif_regmap_config = {
int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
+ const struct soc_device_attribute *attr;
+ const struct rpcif_info *info;
struct resource *res;

rpc->dev = dev;
@@ -276,9 +310,14 @@ 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);
+ info = of_device_get_match_data(dev);
+ attr = soc_device_match(rpcif_info_match);
+ if (attr)
+ info = attr->data;
+
+ rpc->info = info;
+ rpc->size = resource_size(res);
rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);

return PTR_ERR_OR_ZERO(rpc->rstc);
@@ -305,7 +344,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 +360,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 +374,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 +757,9 @@ 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,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..75da785a18ff 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;
@@ -71,6 +76,7 @@ struct rpcif {
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-22 12:19:13

by Krzysztof Kozlowski

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

On 18/01/2023 11:46, Wolfram Sang wrote:
> From: Cong Dang <[email protected]>
>
> According to the datasheets, the Strobe Timing Adjustment bit (STRTIM)
> setting is different on R-Car SoCs, i.e.
>
> R-Car H3 ES1.* : STRTIM[2:0] is set to 0x0
> 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 a little, added Gen4 support]
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Change since V1:
>
> * use proper mask when updating STRTIM bits (thanks, Geert!)
>
> look for 'RPCIF_PHYCNT_STRTIM', there is the change. Rest is the same.
>
> drivers/memory/renesas-rpc-if.c | 63 ++++++++++++++++++++++++++-------
> include/memory/renesas-rpc-if.h | 6 ++++
> 2 files changed, 56 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index c36b407851ff..845b535a5350 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>
> @@ -15,6 +16,7 @@
> #include <linux/of_device.h>
> #include <linux/regmap.h>
> #include <linux/reset.h>
> +#include <linux/sys_soc.h>
>
> #include <memory/renesas-rpc-if.h>
>
> @@ -163,6 +165,36 @@ static const struct regmap_access_table rpcif_volatile_table = {
> .n_yes_ranges = ARRAY_SIZE(rpcif_volatile_ranges),
> };
>
> +static const struct rpcif_info rpcif_info_r8a7795_es1 = {
> + .type = RPCIF_RCAR_GEN3,
> + .strtim = 0,
> +};
> +
> +static const struct rpcif_info rpcif_info_r8a7796_es1 = {
> + .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,
> +};
> +
> +static const struct soc_device_attribute rpcif_info_match[] = {
> + { .soc_id = "r8a7795", .revision = "ES1.*", .data = &rpcif_info_r8a7795_es1 },
> + { .soc_id = "r8a7796", .revision = "ES1.*", .data = &rpcif_info_r8a7796_es1 },

Why do you need soc match? Can't this be inferred from device
compatible? Maybe the device compatible is not specific enough? Devices
should not be interested in which SoC they are running - it does not
matter for them, because the device difference is in the device itself,
not in the SoC (different SoCs come with different devices).

Best regards,
Krzysztof