RE: [PATCH] can: flexcan: change .tseg1_min value to 2 in bittiming const
> From: Joakim Zhang
> Sent: Monday, April 29, 2019 3:48 PM
>
> Time Segment1(tseg1) is composed of Propagate Segment(prop_seg) and
> Phase Segmeng1(phase_seg1). The range of Time Segment1(plus 2) is 2 up to
> 16 according to latest reference manual. That means the minimum value of
> PROPSEG and PSEG1 bit field is 0. So change .tseg1 min value to 2.
>
I saw latest MX6Q RM still indicates it's 4-16.
Can you help double check with IC guys whether all RM of related chips
need update?
Regards
Dong Aisheng
> Signed-off-by: Joakim Zhang
> ---
> drivers/net/can/flexcan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index
> e35083ff31ee..2ea35ef4aa27 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -327,7 +327,7 @@ static const struct flexcan_devtype_data
> fsl_ls1021a_r2_devtype_data = {
>
> static const struct can_bittiming_const flexcan_bittiming_const = {
> .name = DRV_NAME,
> - .tseg1_min = 4,
> + .tseg1_min = 2,
> .tseg1_max = 16,
> .tseg2_min = 2,
> .tseg2_max = 8,
> --
> 2.17.1
RE: [PATCH] can: flexcan: fix timeout when set small bitrate
> -Original Message- > From: Joakim Zhang > Sent: Thursday, January 31, 2019 5:37 PM > To: m...@pengutronix.de; linux-...@vger.kernel.org > Cc: w...@grandegger.com; netdev@vger.kernel.org; > linux-ker...@vger.kernel.org; dl-linux-imx ; Joakim > Zhang > Subject: [PATCH] can: flexcan: fix timeout when set small bitrate > > Current we can meet timeout issue when setting a small bitrate like 1 as > follows on i.MX6UL EVK board (ipg clock = 66MHZ, per clock = 30MHZ): > root@imx6ul7d:~# ip link set can0 up type can bitrate 1 A link change > request failed with some changes committed already. > Interface can0 may have been left with an inconsistent configuration, please > check. > RTNETLINK answers: Connection timed out > > It is caused by calling of flexcan_chip_unfreeze() timeout. > > Originally the code is using usleep_range(10, 20) for unfreeze operation, but > the patch (8badd65 can: flexcan: avoid calling usleep_range from interrupt > context) changed it into udelay(10) which is only a half delay of before, > there're also some other delay changes. > > After double to FLEXCAN_TIMEOUT_US to 100 can fix the issue. > > Signed-off-by: Joakim Zhang Reviewed-by: Dong Aisheng Regards Dong Aisheng
RE: [PATCH] can: flexcan: fix timeout when set small bitrate
> From: Joakim Zhang > Sent: Thursday, January 31, 2019 4:49 PM [...] > > > Current we can meet timeout issue when setting a small bitrate like > > > 1 as follows: > > > root@imx6qdlsolo:~# ip link set can0 up type can bitrate 1 A > > > link change request failed with some changes committed already. > > > Interface can0 may have been left with an inconsistent > > > configuration, please check. > > > RTNETLINK answers: Connection timed out > > > > Which SoC are you using? Which clock rate has the flexcan IP core? > > We tested on i.MX6 series boards and all met this issue. And ipg clock rate > is > 66MHZ, per clock rate is 30MHZ. > > > > It is caused by calling of flexcan_chip_unfreeze() timeout. > > > > > > Originally the code is using usleep_range(10, 20) for unfreeze > > > operation, but the patch (8badd65 can: flexcan: avoid calling > > > usleep_range from interrupt context) changed it into udelay(10) > > > which is only a half delay of before, there're also some other delay > changes. > > > > > > After only changed unfreeze delay back to udelay(20), the issue is gone. > > > So other timeout values are kept the same as 8badd65 changed. > > > > Can you change FLEXCAN_TIMEOUT_US instead? > > Of course, we can change FLEXCAN_TIMEOUT_US to 100, but this will > extend the time of enable/disable/softreset. > Which method do you think is better? > That seems not a big deal for an error case. So change FLEXCAN_TIMEOUT_US seems like a good suggestion to me. You can cook a patch with commit message updated. No need keep my author name as it's a different solution. Regards Dong Aisheng > Best Regards, > Joakim Zhang > > > > Signed-off-by: Dong Aisheng > > > Signed-off-by: Joakim Zhang > > > --- > > > drivers/net/can/flexcan.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > > index 2bca867bcfaa..1d3a9053bbeb 100644 > > > --- a/drivers/net/can/flexcan.c > > > +++ b/drivers/net/can/flexcan.c > > > @@ -530,7 +530,7 @@ static int flexcan_chip_unfreeze(struct > > > flexcan_priv > > *priv) > > > priv->write(reg, ®s->mcr); > > > > > > while (timeout-- && (priv->read(®s->mcr) & > FLEXCAN_MCR_FRZ_ACK)) > > > - udelay(10); > > > + udelay(20); > > > > > > if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > > > return -ETIMEDOUT; > > > > > > > Marc > > > > -- > > Pengutronix e.K. | Marc Kleine-Budde | > > Industrial Linux Solutions| Phone: +49-231-2826-924 | > > Vertretung West/Dortmund | Fax: +49-5121-206917- | > > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
RE: [PATCH net-next] can: flexcan: flexcan_chip_start(): fix the error return code in flexcan_setup_stop_mode()
Copy Joakim.
> > From: Wei Yongjun [mailto:weiyongj...@huawei.com]
[...]
> > Subject: [PATCH net-next] can: flexcan: flexcan_chip_start(): fix the
> > error return code in flexcan_setup_stop_mode()
> >
> > The error return code PTR_ERR(gpr_np) is always 0 since gpr_np is
> > equal to NULL in this error handling case. Fix it by return -ENOENT.
> >
> > Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> > Signed-off-by: Wei Yongjun
> > ---
> > drivers/net/can/flexcan.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index
> > 0f36eaf..f412d84 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -1432,7 +1432,7 @@ static int flexcan_setup_stop_mode(struct
> > platform_device *pdev)
> > gpr_np = of_find_node_by_phandle(phandle);
> > if (!gpr_np) {
> > dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
> > - return PTR_ERR(gpr_np);
> > + return -ENOENT;
>
> Good catch.
>
> Reviewed-by: Dong Aisheng
>
> Regards
> Dong Aisheng
>
> > }
> >
> > priv = netdev_priv(dev);
> >
> >
RE: [PATCH net-next] can: flexcan: flexcan_chip_start(): fix the error return code in flexcan_setup_stop_mode()
> -Original Message-
> From: Wei Yongjun [mailto:weiyongj...@huawei.com]
> Sent: Tuesday, December 4, 2018 2:26 PM
> To: Wolfgang Grandegger ; Marc Kleine-Budde
> ; Aisheng DONG
> Cc: Wei Yongjun ; linux-...@vger.kernel.org;
> netdev@vger.kernel.org; kernel-janit...@vger.kernel.org
> Subject: [PATCH net-next] can: flexcan: flexcan_chip_start(): fix the error
> return
> code in flexcan_setup_stop_mode()
>
> The error return code PTR_ERR(gpr_np) is always 0 since gpr_np is equal to
> NULL in this error handling case. Fix it by return -ENOENT.
>
> Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> Signed-off-by: Wei Yongjun
> ---
> drivers/net/can/flexcan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index
> 0f36eaf..f412d84 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1432,7 +1432,7 @@ static int flexcan_setup_stop_mode(struct
> platform_device *pdev)
> gpr_np = of_find_node_by_phandle(phandle);
> if (!gpr_np) {
> dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
> - return PTR_ERR(gpr_np);
> + return -ENOENT;
Good catch.
Reviewed-by: Dong Aisheng
Regards
Dong Aisheng
> }
>
> priv = netdev_priv(dev);
>
>
RE: [PATCH] can: flexcan: Implement CAN Runtime PM
> -Original Message- > From: Marc Kleine-Budde [mailto:m...@pengutronix.de] > Sent: Tuesday, November 27, 2018 3:52 PM > To: Joakim Zhang ; w...@grandegger.com; > da...@davemloft.net > Cc: linux-...@vger.kernel.org; netdev@vger.kernel.org; > linux-ker...@vger.kernel.org; dl-linux-imx ; Aisheng > DONG > Subject: Re: [PATCH] can: flexcan: Implement CAN Runtime PM > > On 11/27/18 8:44 AM, Joakim Zhang wrote: > >>> Flexcan will be disabled during suspend if no wakeup function > >>> required and enabled after resume accordingly. During this period, > >>> we could explicitly disable clocks. > >>> > >>> Implement Runtime PM which will: > >>> 1) Keep device in suspend state (clocks disabled) if it's not > >>> openned > >>> 2) Make Power Domain framework be able to shutdown the corresponding > >>> power domain of this device. > >> > >> Without CONFIG_PM the device fails to probe: > >> > >>> [ 214.420606] flexcan 209.flexcan: 209.flexcan supply > >>> xceiver not found, using dummy regulator [ 214.445952] flexcan > >>> 209.flexcan: Linked as a consumer to regulator.0 [ 214.453448] > >>> flexcan 209.flexcan: registering netdev failed [ 214.459666] > >>> flexcan 209.flexcan: Dropping the link to regulator.0 [ > >>> 214.472066] flexcan: probe of 209.flexcan failed with error -110 > > > I would sent V2 rebased on patch "can: flexcan: add self wakeup > > support", and runtime pm works normally on MX6SX-SDB and MX7D-SDB. > > So, could you tell me which board you tested on that causes the device > > probe failed? > > I tested on an imx6dl riotboard > (https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir. > bootlin.com%2Flinux%2Flatest%2Fsource%2Farch%2Farm%2Fboot%2Fdts%2Fi > mx6dl-riotboard.dts&data=02%7C01%7Caisheng.dong%40nxp.com%7C8 > 4cfd3f3a08b4c7953fd08d6543d42c1%7C686ea1d3bc2b4c6fa92cd99c5c3016 > 35%7C0%7C0%7C636789019420569534&sdata=rugKpmVaNn0ajTdctyi% > 2F%2Bnwr%2FqqIBcikAP7eXLp6fcA%3D&reserved=0 > + patches to enable CAN1) with an external PHY. > > > And that CONFIG_PM has set in imx_v6_v7_defconfig which config file I > used. > > I disabled CONFIG_PM on purpose. Joakim, You need run the same test as Marc. I guess It might because without CONFIG_PM, Runtime PM API Simply return positive but the clock may still not enabled, thus cause a CAN enable timeout. You can check it. Regards Dong Aisheng > > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions| Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917- | > Amtsgericht Hildesheim, HRA 2686 | > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. > pengutronix.de&data=02%7C01%7Caisheng.dong%40nxp.com%7C84cfd3 > f3a08b4c7953fd08d6543d42c1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7 > C0%7C0%7C636789019420569534&sdata=Us6cO9QfYkw%2B6Nhj68zLr > wrNxUCRjcC0WDdmr9MOzHM%3D&reserved=0 |
