| 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: <20210716101001.m5sgit3l354mljai@pengutronix.de>
Date: Fri, 16 Jul 2021 12:10:01 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Cc: Geert Uytterhoeven <geert+renesas@...der.be>,
Rob Herring <robh+dt@...nel.org>,
Wolfgang Grandegger <wg@...ndegger.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
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 2/6] can: rcar_canfd: Add support for RZ/G2L family
On 15.07.2021 19:21:19, 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, irq handlers for the same are added.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
Thanks for the patch! Some nitpicks inline, Geert already commented to
use the same IRQ handler for all interrutps.
> ---
> drivers/net/can/rcar/rcar_canfd.c | 275 ++++++++++++++++++++++++++----
> 1 file changed, 244 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 311e6ca3bdc4..5dfbc5fa2d81 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -37,9 +37,13 @@
> #include <linux/bitmap.h>
> #include <linux/bitops.h>
> #include <linux/iopoll.h>
> +#include <linux/reset.h>
>
> #define RCANFD_DRV_NAME "rcar_canfd"
>
> +#define RENESAS_RCAR_GEN3 0
> +#define RENESAS_RZG2L 1
> +
Please make this an enum.
> /* Global register bits */
>
> /* RSCFDnCFDGRMCFG */
> @@ -513,6 +517,9 @@ struct rcar_canfd_global {
> enum rcar_canfd_fcanclk fcan; /* CANFD or Ext clock */
> unsigned long channels_mask; /* Enabled channels mask */
> bool fdmode; /* CAN FD or Classical CAN only mode */
> + struct reset_control *rstc1; /* Pointer to reset source1 */
> + struct reset_control *rstc2; /* Pointer to reset source2 */
> + unsigned int chip_id;
enum here, too
> };
>
> /* CAN FD mode nominal rate constants */
> @@ -1070,6 +1077,56 @@ static void rcar_canfd_tx_done(struct net_device *ndev)
> can_led_event(ndev, CAN_LED_EVENT_TX);
> }
>
[...]
> @@ -1635,8 +1784,11 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> struct rcar_canfd_global *gpriv;
> struct device_node *of_child;
> unsigned long channels_mask = 0;
> - int err, ch_irq, g_irq;
> + int err, ch_irq, g_irq, g_rx_irq;
> bool fdmode = true; /* CAN FD only mode - default */
> + unsigned int chip_id;
> +
> + chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
The cast looks wrong.
>
> if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
> fdmode = false; /* Classical CAN only mode */
> @@ -1649,27 +1801,56 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> if (of_child && of_device_is_available(of_child))
> channels_mask |= BIT(1); /* Channel 1 */
>
> - ch_irq = platform_get_irq(pdev, 0);
> - if (ch_irq < 0) {
> - err = ch_irq;
> - goto fail_dev;
> - }
> + if (chip_id == RENESAS_RCAR_GEN3) {
> + ch_irq = platform_get_irq(pdev, 0);
> + if (ch_irq < 0)
> + return ch_irq;
>
> - g_irq = platform_get_irq(pdev, 1);
> - if (g_irq < 0) {
> - err = g_irq;
> - goto fail_dev;
> + g_irq = platform_get_irq(pdev, 1);
> + if (g_irq < 0)
> + return g_irq;
> + } else {
> + g_irq = platform_get_irq(pdev, 0);
> + if (g_irq < 0)
> + return g_irq;
> +
> + g_rx_irq = platform_get_irq(pdev, 1);
> + if (g_rx_irq < 0)
> + return g_rx_irq;
> }
>
> /* Global controller context */
> gpriv = devm_kzalloc(&pdev->dev, sizeof(*gpriv), GFP_KERNEL);
> - if (!gpriv) {
> - err = -ENOMEM;
> - goto fail_dev;
> - }
> + if (!gpriv)
> + return -ENOMEM;
> +
> gpriv->pdev = pdev;
> gpriv->channels_mask = channels_mask;
> gpriv->fdmode = fdmode;
> + gpriv->chip_id = chip_id;
> +
> + 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");
> + return PTR_ERR(gpriv->rstc1);
> + }
> +
> + err = reset_control_reset(gpriv->rstc1);
> + if (err)
> + return err;
> +
> + gpriv->rstc2 = devm_reset_control_get_exclusive_by_index(&pdev->dev, 1);
> + if (IS_ERR(gpriv->rstc2)) {
> + dev_err(&pdev->dev, "failed to get reset index 1\n");
> + return PTR_ERR(gpriv->rstc2);
> + }
> + err = reset_control_reset(gpriv->rstc2);
> + if (err) {
> + reset_control_assert(gpriv->rstc1);
> + return err;
> + }
> + }
>
> /* Peripheral clock */
> gpriv->clkp = devm_clk_get(&pdev->dev, "fck");
> @@ -1699,7 +1880,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> }
> fcan_freq = clk_get_rate(gpriv->can_clk);
>
> - if (gpriv->fcan == RCANFD_CANFDCLK)
> + if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id == RENESAS_RCAR_GEN3)
> /* CANFD clock is further divided by (1/2) within the IP */
> fcan_freq /= 2;
>
> @@ -1711,21 +1892,43 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> gpriv->base = addr;
>
> /* Request IRQ that's common for both channels */
> - err = devm_request_irq(&pdev->dev, ch_irq,
> - rcar_canfd_channel_interrupt, 0,
> - "canfd.chn", gpriv);
> - if (err) {
> - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> - ch_irq, err);
> - goto fail_dev;
> - }
> - err = devm_request_irq(&pdev->dev, g_irq,
> - rcar_canfd_global_interrupt, 0,
> - "canfd.gbl", gpriv);
> - if (err) {
> - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> - g_irq, err);
> - goto fail_dev;
> + if (gpriv->chip_id == RENESAS_RCAR_GEN3) {
> + err = devm_request_irq(&pdev->dev, ch_irq,
> + rcar_canfd_channel_interrupt, 0,
> + "canfd.chn", gpriv);
> + if (err) {
> + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> + ch_irq, err);
> + goto fail_dev;
> + }
> +
> + err = devm_request_irq(&pdev->dev, g_irq,
> + rcar_canfd_global_interrupt, 0,
> + "canfd.gbl", gpriv);
> + if (err) {
> + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> + g_irq, err);
> + goto fail_dev;
> + }
> + } else {
> + err = devm_request_irq(&pdev->dev, g_rx_irq,
> + rcar_canfd_global_recieve_fifo_interrupt, 0,
> + "canfd.gblrx", gpriv);
> +
> + if (err) {
> + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> + g_rx_irq, err);
> + goto fail_dev;
> + }
> +
> + err = devm_request_irq(&pdev->dev, g_irq,
> + rcar_canfd_global_err_interrupt, 0,
> + "canfd.gblerr", gpriv);
> + if (err) {
> + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> + g_irq, err);
> + goto fail_dev;
> + }
> }
>
> /* Enable peripheral clock for register access */
> @@ -1791,6 +1994,10 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> fail_clk:
> clk_disable_unprepare(gpriv->clkp);
> fail_dev:
> + if (gpriv->chip_id == RENESAS_RZG2L) {
> + reset_control_assert(gpriv->rstc1);
> + reset_control_assert(gpriv->rstc2);
reset_control_assert() can handle NULL pointers
> + }
> return err;
> }
>
> @@ -1810,6 +2017,11 @@ static int rcar_canfd_remove(struct platform_device *pdev)
> /* Enter global sleep mode */
> rcar_canfd_set_bit(gpriv->base, RCANFD_GCTR, RCANFD_GCTR_GSLPR);
> clk_disable_unprepare(gpriv->clkp);
> + if (gpriv->chip_id == RENESAS_RZG2L) {
> + reset_control_assert(gpriv->rstc1);
> + reset_control_assert(gpriv->rstc2);
> + }
same here
> +
> return 0;
> }
>
> @@ -1827,7 +2039,8 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
> rcar_canfd_resume);
>
> static const struct of_device_id rcar_canfd_of_table[] = {
> - { .compatible = "renesas,rcar-gen3-canfd" },
> + { .compatible = "renesas,rcar-gen3-canfd", .data = (void *)RENESAS_RCAR_GEN3 },
> + { .compatible = "renesas,rzg2l-canfd", .data = (void *)RENESAS_RZG2L },
> { }
> };
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists