Hi Geert,

On 9/6/21 6:12 pm, Geert Uytterhoeven wrote:

Hi Joakim, Angelo,

On Wed, Jun 9, 2021 at 4:05 AM Joakim Zhang [off-list ref] wrote:

quoted

quoted

From: Angelo Dureghello <redacted>
Sent: 2021年6月9日 4:46
To: gerg@linux-m68k.org; wg@grandegger.com; mkl@pengutronix.de
Cc: geert@linux-m68k.org; linux-m68k@vger.kernel.org;
linux-can@vger.kernel.org; Joakim Zhang [off-list ref];
Angelo Dureghello [off-list ref]
Subject: [PATCH 5/5] can: flexcan: add mcf5441x support

Add flexcan support for NXP ColdFire mcf5441x family.

This flexcan module is quite similar to imx6 flexcan module, but with some
exceptions:

- 3 separate interrupt sources, MB, BOFF and ERR,
- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a
   platform data check/case has been added,
- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan
   module.

Signed-off-by: Angelo Dureghello <redacted>

quoted

quoted

--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c

quoted

quoted

@@ -637,13 +650,17 @@ static int flexcan_clks_enable(const struct

flexcan_priv *priv)  {
       int err;

-     err = clk_prepare_enable(priv->clk_ipg);
-     if (err)
-             return err;
+     if (priv->clk_ipg) {
+             err = clk_prepare_enable(priv->clk_ipg);
+             if (err)
+                     return err;
+     }

-     err = clk_prepare_enable(priv->clk_per);
-     if (err)
-             clk_disable_unprepare(priv->clk_ipg);
+     if (priv->clk_per) {
+             err = clk_prepare_enable(priv->clk_per);
+             if (err)
+                     clk_disable_unprepare(priv->clk_ipg);
+     }

No need do this check, it will be handled in clk_prepare_enable() / clk_disable_unprepare(). So this change is unnecessary.

Except that the non-CCF implementation of clk_enable() in
arch/m68k/coldfire/clk.c still returns -EINVAL instead of NULL.
Any plans to move to CCF? Or at least fix legacy clk_enable().

That was a recent change, see commit c1fb1bf64bb6 ("m68k: let clk_enable() return
immediately if clk is NULL").

It could just as easily just return on that NULL check.

Regards
Greg

quoted

quoted

@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device

*pdev)

       devtype_data = of_device_get_match_data(&pdev->dev);

+     if (pdata && !devtype_data) {
+             devtype_data =
+                     (struct flexcan_devtype_data *)&fsl_mcf_devtype_data;

Cast not needed?

Gr{oetje,eeting}s,

                         Geert