Re: [PATCH 5/5] can: flexcan: add mcf5441x support

Angelo Dureghello
2021-06-09T08:42:57Z

Hi Geert,

On 09/06/21 10:12 AM, Geert Uytterhoeven wrote:
> Hi Joakim, Angelo,
>
> On Wed, Jun 9, 2021 at 4:05 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>>> From: Angelo Dureghello <angelo@kernel-space.org>
>>> 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 <qiangqing.zhang@nxp.com>;
>>> Angelo Dureghello <angelo@kernel-space.org>
>>> 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 <angelo@kernel-space.org>
>
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>
>>> @@ -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().
>
>>> @@ -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?
>
Thanks, fixed for v2.

> Gr{oetje,eeting}s,
>
> Geert
>

Regards,
--
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org