netdev - Re: [PATCH v2 2/5] can: rcar_canfd: Add support for RZ/G2L family

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c8ec5fe0c8eb86898416edb7c68dcf0eeeaccf54.camel@pengutronix.de>
Date:   Tue, 20 Jul 2021 12:23:07 +0200
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Rob Herring <robh+dt@...nel.org>,
        Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-can@...r.kernel.org, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...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 2/5] can: rcar_canfd: Add support for RZ/G2L family

On Mon, 2021-07-19 at 15:38 +0100, Lad Prabhakar wrote:
> CANFD block on RZ/G2L SoC is almost identical to one found on
> R-Car Gen3 SoC's. On RZ/G2L SoC interrupt sources for each channel
> are split into different sources and the IP doesn't divide (1/2)
> CANFD clock within the IP.
> 
> This patch adds compatible string for RZ/G2L family and registers
> the irq handlers required for CANFD operation. IRQ numbers are now
> fetched based on names instead of indices. For backward compatibility
> on non RZ/G2L SoC's we fallback reading based on indices.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
> ---
>  drivers/net/can/rcar/rcar_canfd.c | 178 ++++++++++++++++++++++++------
>  1 file changed, 147 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 311e6ca3bdc4..d4affc002fb3 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -37,9 +37,15 @@
[...]
> +	if (gpriv->chip_id == RENESAS_RZG2L) {
> +		gpriv->rstc1 = devm_reset_control_get_exclusive_by_index(&pdev->dev, 0);
> +		if (IS_ERR(gpriv->rstc1)) {
> +			dev_err(&pdev->dev, "failed to get reset index 0\n");

Please consider requesting the reset controls by name instead of by
index. See also my reply to the binding patch.

> +			return PTR_ERR(gpriv->rstc1);
> +		}
> +
> +		err = reset_control_reset(gpriv->rstc1);
> +		if (err)
> +			return err;

I suggest to wait until after all resource requests have succeeded
before triggering the resets, i.e. first get all reset controls and
clocks, etc., and only then trigger resets, enable clocks, and so on.

That way there will be no spurious resets in case of probe deferrals.

regards
Philipp