| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3739744b-0a10-6d6b-8d1c-9c82b6afe0b3@canonical.com>
Date: Tue, 26 Oct 2021 16:46:58 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Rob Herring <robh+dt@...nel.org>,
Vignesh Raghavendra <vigneshr@...com>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
Mark Brown <broonie@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Sergei Shtylyov <sergei.shtylyov@...il.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mtd@...ts.infradead.org, linux-spi@...r.kernel.org,
linux-renesas-soc@...r.kernel.org,
Prabhakar <prabhakar.csengg@...il.com>,
Biju Das <biju.das.jz@...renesas.com>
Subject: Re: [PATCH v2 7/7] memory: renesas-rpc-if: Add support for RZ/G2L
On 25/10/2021 22:56, Lad Prabhakar wrote:
> SPI Multi I/O Bus Controller on RZ/G2L SoC is almost identical to
> the RPC-IF interface found on R-Car Gen3 SoC's.
>
> This patch adds a new compatible string for the RZ/G2L family so
> that the timing values on RZ/G2L can be adjusted.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
> ---
> v1->v2:
> * Updated macros as suggested by Wolfram
> ---
> drivers/memory/renesas-rpc-if.c | 72 ++++++++++++++++++++++++++++-----
> drivers/mtd/hyperbus/rpc-if.c | 4 +-
> drivers/spi/spi-rpc-if.c | 4 +-
> include/memory/renesas-rpc-if.h | 8 +++-
> 4 files changed, 75 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index 0c56decc91f2..8c51145c0f5c 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
> @@ -12,6 +12,7 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/regmap.h>
> #include <linux/reset.h>
>
> @@ -27,8 +28,8 @@
> #define RPCIF_CMNCR_MOIIO_HIZ (RPCIF_CMNCR_MOIIO0(3) | \
> RPCIF_CMNCR_MOIIO1(3) | \
> RPCIF_CMNCR_MOIIO2(3) | RPCIF_CMNCR_MOIIO3(3))
> -#define RPCIF_CMNCR_IO3FV(val) (((val) & 0x3) << 14) /* undocumented */
> -#define RPCIF_CMNCR_IO2FV(val) (((val) & 0x3) << 12) /* undocumented */
> +#define RPCIF_CMNCR_IO3FV(val) (((val) & 0x3) << 14) /* documented for RZ/G2L */
> +#define RPCIF_CMNCR_IO2FV(val) (((val) & 0x3) << 12) /* documented for RZ/G2L */
> #define RPCIF_CMNCR_IO0FV(val) (((val) & 0x3) << 8)
> #define RPCIF_CMNCR_IOFV_HIZ (RPCIF_CMNCR_IO0FV(3) | RPCIF_CMNCR_IO2FV(3) | \
> RPCIF_CMNCR_IO3FV(3))
> @@ -126,6 +127,9 @@
> #define RPCIF_SMDRENR_OPDRE BIT(4)
> #define RPCIF_SMDRENR_SPIDRE BIT(0)
>
> +#define RPCIF_PHYADD 0x0070 /* R/W available on R-Car E3/D3/V3M and RZ/G2{E,L} */
> +#define RPCIF_PHYWR 0x0074 /* R/W available on R-Car E3/D3/V3M and RZ/G2{E,L} */
> +
> #define RPCIF_PHYCNT 0x007C /* R/W */
> #define RPCIF_PHYCNT_CAL BIT(31)
> #define RPCIF_PHYCNT_OCTA(v) (((v) & 0x3) << 22)
> @@ -133,10 +137,12 @@
> #define RPCIF_PHYCNT_OCT BIT(20)
> #define RPCIF_PHYCNT_DDRCAL BIT(19)
> #define RPCIF_PHYCNT_HS BIT(18)
> -#define RPCIF_PHYCNT_STRTIM(v) (((v) & 0x7) << 15)
> +#define RPCIF_PHYCNT_CKSEL(v) (((v) & 0x3) << 16) /* valid only for RZ/G2L */
> +#define RPCIF_PHYCNT_STRTIM(v) (((v) & 0x7) << 15) /* valid for R-Car and RZ/G2{E,H,M,N} */
> #define RPCIF_PHYCNT_WBUF2 BIT(4)
> #define RPCIF_PHYCNT_WBUF BIT(2)
> #define RPCIF_PHYCNT_PHYMEM(v) (((v) & 0x3) << 0)
> +#define RPCIF_PHYCNT_PHYMEM_MASK GENMASK(1, 0)
>
> #define RPCIF_PHYOFFSET1 0x0080 /* R/W */
> #define RPCIF_PHYOFFSET1_DDRTMG(v) (((v) & 0x3) << 28)
> @@ -244,18 +250,46 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
> return PTR_ERR(rpc->dirmap);
> rpc->size = resource_size(res);
>
> + rpc->type = (enum rpcif_type)of_device_get_match_data(dev);
> rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>
> return PTR_ERR_OR_ZERO(rpc->rstc);
> }
> EXPORT_SYMBOL(rpcif_sw_init);
>
> -void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> +static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif *rpc)
> +{
> + u32 data;
> +
> + regmap_write(rpc->regmap, RPCIF_PHYWR, 0xa5390000);
> + regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000000);
> + regmap_write(rpc->regmap, RPCIF_PHYWR, 0x00008080);
> + regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000022);
> + regmap_write(rpc->regmap, RPCIF_PHYWR, 0x00008080);
> + regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000024);
> +
> + regmap_read(rpc->regmap, RPCIF_PHYCNT, &data);
> + regmap_write(rpc->regmap, RPCIF_PHYCNT, data | RPCIF_PHYCNT_CKSEL(3));
> + regmap_write(rpc->regmap, RPCIF_PHYWR, 0x00000030);
> + regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000032);
> +}
> +
> +int rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> {
> u32 dummy;
>
> pm_runtime_get_sync(rpc->dev);
>
> + if (rpc->type == RPCIF_RZ_G2L) {
> + int ret;
> +
> + ret = reset_control_reset(rpc->rstc);
> + if (ret)
> + return ret;
> + usleep_range(200, 300);
> + 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,
> @@ -264,8 +298,15 @@ void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> * On H3 ES1.x, the value should be 0, while on others,
> * the value should be 7.
> */
> - regmap_write(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_STRTIM(7) |
> - RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260);
> + 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);
> + }
>
> /*
> * NOTE: The 0x1511144 are undocumented bits, but they must be set
> @@ -282,9 +323,17 @@ void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> regmap_update_bits(rpc->regmap, RPCIF_PHYINT,
> RPCIF_PHYINT_WPVAL, 0);
>
> - regmap_write(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_SFDE |
> - RPCIF_CMNCR_MOIIO_HIZ | RPCIF_CMNCR_IOFV_HIZ |
> - RPCIF_CMNCR_BSZ(hyperflash ? 1 : 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));
> + 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));
> +
> /* Set RCF after BSZ update */
> regmap_write(rpc->regmap, RPCIF_DRCR, RPCIF_DRCR_RCF);
> /* Dummy read according to spec */
> @@ -295,6 +344,8 @@ void rpcif_hw_init(struct rpcif *rpc, bool hyperflash)
> pm_runtime_put(rpc->dev);
>
> rpc->bus_size = hyperflash ? 2 : 1;
> +
> + return 0;
> }
> EXPORT_SYMBOL(rpcif_hw_init);
>
> @@ -657,7 +708,8 @@ static int rpcif_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id rpcif_of_match[] = {
> - { .compatible = "renesas,rcar-gen3-rpc-if", },
> + { .compatible = "renesas,rcar-gen3-rpc-if", .data = (void *)RPCIF_RCAR_GEN3 },
> + { .compatible = "renesas,rzg2l-rpc-if", .data = (void *)RPCIF_RZ_G2L },
> {},
> };
> MODULE_DEVICE_TABLE(of, rpcif_of_match);
> diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
> index 367b0d72bf62..40bca89268c3 100644
> --- a/drivers/mtd/hyperbus/rpc-if.c
> +++ b/drivers/mtd/hyperbus/rpc-if.c
> @@ -132,7 +132,9 @@ static int rpcif_hb_probe(struct platform_device *pdev)
>
> rpcif_enable_rpm(&hyperbus->rpc);
>
> - rpcif_hw_init(&hyperbus->rpc, true);
> + error = rpcif_hw_init(&hyperbus->rpc, true);
> + if (error)
> + return error;
>
Not related to this patch, but the concept used here looks fragile. The
child driver calls also rpcif_sw_init() and ignores the error code. What
happens in case of rpcif_sw_init() failure or child probe deferral?
Since the SW and HW init is called in context of child device, the
parent won't do anything. Then, second bind of child device (manual or
because of deferral) will fail on devm_reset_control_get_exclusive()
with -EBUSY.
Initializing parent's resources should be rather done from parent's
context (so renesas-rpc-if.c) to handle properly deferred probe and
other failures. Doing it from a child, breaks encapsulation and
separation of devices.
Is there any reason why memory/renesas-rpc-if.c cannot do SW and HW init?
Best regards,
Krzysztof
Powered by blists - more mailing lists