> -----Original Message----- > From: Joakim Zhang > Sent: Friday, 17 May, 2019 06:52 AM > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>; Wolfgang Grandegger > <wg@xxxxxxxxxxxxxx>; Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> > Cc: linux-can@xxxxxxxxxxxxxxx > Subject: RE: [PATCH v2 2/2] can: flexcan: make PE clock info conditional > > > > -----Original Message----- > > From: linux-can-owner@xxxxxxxxxxxxxxx > > <linux-can-owner@xxxxxxxxxxxxxxx> On Behalf Of Pankaj Bansal > > Sent: 2019年5月16日 16:12 > > To: Wolfgang Grandegger <wg@xxxxxxxxxxxxxx>; Marc Kleine-Budde > > <mkl@xxxxxxxxxxxxxx> > > Cc: linux-can@xxxxxxxxxxxxxxx > > Subject: [PATCH v2 2/2] can: flexcan: make PE clock info conditional > > > > PE clock info can be skipped if the synchronous clocking is to be used > > in flexcan. > > > > Refer Documentation/devicetree/bindings/net/can/fsl-flexcan.txt for > > more info. > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx> > > --- > > > > Notes: > > Dependencies: > > [1] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .ker > nel.org%2Fpatchwork%2Fcover%2F1024536%2F&data=02%7C01%7Cqia > > ngqing.zhang%40nxp.com%7C819ade0cfed94b81874308d6d9d63fab%7C686 > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636935911549889558&am > > p;sdata=HDUiHNV8GBS%2B2X9wE0IWB6ihJk87voAqsC8xkP75Dh4%3D& > > reserved=0 > > [2] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .ker > nel.org%2Fpatchwork%2Fpatch%2F1001918%2F&data=02%7C01%7Cqia > > ngqing.zhang%40nxp.com%7C819ade0cfed94b81874308d6d9d63fab%7C686 > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636935911549889558&am > > p;sdata=%2Bq70ASaROgZHnNo8SEgGS0dCHrSuzY7GyaB%2FjxuW41Y%3D&am > > p;reserved=0 > > > > V2: > > - modified the clk_src == 0 to !clk_src > > - modified changes in flexcan_clks_enable > > - Added Dependencies in Notes > > > > drivers/net/can/flexcan.c | 39 +++++++++++++++++++++++++------------ > > 1 file changed, 27 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > index 5f4075d598d0..249fc0a6f85d 100644 > > --- a/drivers/net/can/flexcan.c > > +++ b/drivers/net/can/flexcan.c > > @@ -515,21 +515,24 @@ static int flexcan_clks_enable(const struct > > flexcan_priv *priv) { > > int err; > > > > - err = clk_prepare_enable(priv->clk_ipg); > > + err = clk_prepare_enable(priv->clk_per); > > if (err) > > return err; > > > > - err = clk_prepare_enable(priv->clk_per); > > - if (err) > > - clk_disable_unprepare(priv->clk_ipg); > > + if (priv->clk_ipg) { > > + err = clk_prepare_enable(priv->clk_ipg); > > + if (err) > > + clk_disable_unprepare(priv->clk_per); > > + } > > > > return err; > > } > > > > static void flexcan_clks_disable(const struct flexcan_priv *priv) { > > + if (priv->clk_ipg) > > + clk_disable_unprepare(priv->clk_ipg); > > clk_disable_unprepare(priv->clk_per); > > - clk_disable_unprepare(priv->clk_ipg); > > } > > > > static inline int flexcan_transceiver_enable(const struct > > flexcan_priv *priv) @@ -1688,18 +1691,30 @@ static int > > flexcan_probe(struct platform_device > > *pdev) > > } > > > > if (!clock_freq) { > > - clk_ipg = devm_clk_get(&pdev->dev, "ipg"); > > - if (IS_ERR(clk_ipg)) { > > - dev_err(&pdev->dev, "no ipg clock defined\n"); > > - return PTR_ERR(clk_ipg); > > - } > > - > > clk_per = devm_clk_get(&pdev->dev, "per"); > > if (IS_ERR(clk_per)) { > > dev_err(&pdev->dev, "no per clock defined\n"); > > return PTR_ERR(clk_per); > > } > > - clock_freq = clk_get_rate(clk_per); > > + > > + if (!clk_src) { > > + // only get oscillator clock if asynchronous clocking > > + // is to be used. > > + clk_ipg = devm_clk_get(&pdev->dev, "ipg"); > > + if (IS_ERR(clk_ipg)) { > > + dev_err(&pdev->dev, "no ipg clock defined\n"); > > + return PTR_ERR(clk_ipg); > > + } > > + > > + clock_freq = clk_get_rate(clk_ipg); > > + if (clock_freq >= clk_get_rate(clk_per)) { > > + dev_err(&pdev->dev, > > + "PE clock should be less than CHI\n"); > > + return -EINVAL; > > + } > > + } else { > > + clock_freq = clk_get_rate(clk_per); > > + } > > } > > [Joakim Zhang] When SOC implements the synchronous clock(clk_src = 1), it just > means that CHI and PE share the same clock source. > Also can select a different clock frequency for PE clock, right? If we are going to use synchronous clock, then no need to enable PE clock. It can remain in disabled state However, if the SOC doesn't have CTRL[CLK_SRC] bit implemented, then SOC's flexcan node *must* contain fsl,clk-source = <0> > > clocks = <&sysclk>, <&clockgen 4 7>; > clock-names = "ipg", "per"; > *assigned-clocks = <&sysclk>;* > *assigned-clcoks-rates = <40000000>;* > > We also need to get PE clock info with this case. > > Best Regards, > Joakim Zhang > > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > -- > > 2.17.1
