| Message ID | 20230126173513.36659-1-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v3] memory: renesas-rpc-if: Fix PHYCNT.STRTIM setting | expand |
Hi Wolfram, On Thu, Jan 26, 2023 at 6:41 PM Wolfram Sang <wsa+renesas@sang-engineering.com> 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 <cong.dang.xn@renesas.com> > Signed-off-by: Hai Pham <hai.pham.ud@renesas.com> > [wsa: rebased, restructured, added Gen4 support] > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > 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 <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
> > - 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 <geert+renesas@glider.be> Thanks!
On Thu, Jan 26, 2023 at 5:35 PM Wolfram Sang <wsa+renesas@sang-engineering.com> 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 <cong.dang.xn@renesas.com> > Signed-off-by: Hai Pham <hai.pham.ud@renesas.com> > [wsa: rebased, restructured, added Gen4 support] > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > 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 <prabhakar.mahadev-lad.rj@bp.renesas.com> 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 >
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;