| Message ID | 20181213070537.25095-2-qiangqing.zhang@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | can: flexcan: add PE clock source select support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote: > From: Dong Aisheng <aisheng.dong@nxp.com> > > The FlexCAN controller can parse clock source property from DTS file to > select PE clock source. > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > --- > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > index bc77477c6878..a04168605998 100644 > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > @@ -32,6 +32,13 @@ Optional properties: > ack_gpr is the gpr register offset of CAN stop acknowledge. > ack_bit is the bit offset of CAN stop acknowledge. > > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE). > + It's SoC Implementation dependent. Refer to RM for detailed If SoC dependent, then it should be implied by the SoC specific compatible. Also, seems like you should add clock binding support here if you need more clock control. > + definition. If this property is not set in device tree node > + then driver selects clock source 1 by default. > + 0: clock source 0 (oscillator clock) > + 1: clock source 1 (peripheral clock) > + > Example: > > can@1c000 { > @@ -40,4 +47,5 @@ Example: > interrupts = <48 0x2>; > interrupt-parent = <&mpic>; > clock-frequency = <200000000>; // filled in by bootloader > + fsl,clk-source = <0>; // select clock source 0 for PE > }; > -- > 2.17.1 >
> -----Original Message----- > From: Rob Herring [mailto:robh@kernel.org] > Sent: Friday, December 21, 2018 4:23 AM > > On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote: > > From: Dong Aisheng <aisheng.dong@nxp.com> > > > > The FlexCAN controller can parse clock source property from DTS file > > to select PE clock source. > > > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > > --- > > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 > > ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > index bc77477c6878..a04168605998 100644 > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > @@ -32,6 +32,13 @@ Optional properties: > > ack_gpr is the gpr register offset of CAN stop acknowledge. > > ack_bit is the bit offset of CAN stop acknowledge. > > > > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE). > > + It's SoC Implementation dependent. Refer to RM for detailed > > If SoC dependent, then it should be implied by the SoC specific compatible. > Also, seems like you should add clock binding support here if you need more > clock control. The clock source selection is done by a register bit inside the IP block: BIT13 CLKSRC CAN Engine Clock Source 0b - The CAN engine clock source is the oscillator clock. 1b - The CAN engine clock source is the peripheral clock. Currently it's written 1 by default during driver initialization. drivers/net/can/flexcan.c /* select "bus clock", chip must be disabled */ reg = priv->read(®s->ctrl); reg |= FLEXCAN_CTRL_CLK_SRC; priv->write(reg, ®s->ctrl); I'm not sure if it's a typical case to abstract CLKSRC bit into a common clock mux. (Is there any similar case in kernel?) But I think we can also use SoC specific compatible to write 0 for those special Ones (currently only imx8qxp). Then this patch may not be needed. Marc, Please let us know if you have a different idea. Regards Dong Aisheng > > > + definition. If this property is not set in device tree node > > + then driver selects clock source 1 by default. > > + 0: clock source 0 (oscillator clock) > > + 1: clock source 1 (peripheral clock) > > + > > Example: > > > > can@1c000 { > > @@ -40,4 +47,5 @@ Example: > > interrupts = <48 0x2>; > > interrupt-parent = <&mpic>; > > clock-frequency = <200000000>; // filled in by bootloader > > + fsl,clk-source = <0>; // select clock source 0 for PE > > }; > > -- > > 2.17.1 > >
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: 2018年12月21日 4:23 > To: Joakim Zhang <qiangqing.zhang@nxp.com> > Cc: mkl@pengutronix.de; linux-can@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com> > Subject: Re: [PATCH 1/2] Documentation: can: flexcan: add PE clock source > property to device tree > > On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote: > > From: Dong Aisheng <aisheng.dong@nxp.com> > > > > The FlexCAN controller can parse clock source property from DTS file > > to select PE clock source. > > > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > > --- > > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 > > ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > index bc77477c6878..a04168605998 100644 > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > @@ -32,6 +32,13 @@ Optional properties: > > ack_gpr is the gpr register offset of CAN stop acknowledge. > > ack_bit is the bit offset of CAN stop acknowledge. > > > > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE). > > + It's SoC Implementation dependent. Refer to RM for detailed > > If SoC dependent, then it should be implied by the SoC specific compatible. > Also, seems like you should add clock binding support here if you need more > clock control. Hi, My expression here might not be very accurate, this property just help to select the clock source for Protocol Engine (PE). FlexCAN ip support two PE clock sources, so we have to add this property to let driver know that SoC has selected which one. Best Regards, Joakim Zhang > > + definition. If this property is not set in device tree node > > + then driver selects clock source 1 by default. > > + 0: clock source 0 (oscillator clock) > > + 1: clock source 1 (peripheral clock) > > + > > Example: > > > > can@1c000 { > > @@ -40,4 +47,5 @@ Example: > > interrupts = <48 0x2>; > > interrupt-parent = <&mpic>; > > clock-frequency = <200000000>; // filled in by bootloader > > + fsl,clk-source = <0>; // select clock source 0 for PE > > }; > > -- > > 2.17.1 > >
diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt index bc77477c6878..a04168605998 100644 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt @@ -32,6 +32,13 @@ Optional properties: ack_gpr is the gpr register offset of CAN stop acknowledge. ack_bit is the bit offset of CAN stop acknowledge. +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE). + It's SoC Implementation dependent. Refer to RM for detailed + definition. If this property is not set in device tree node + then driver selects clock source 1 by default. + 0: clock source 0 (oscillator clock) + 1: clock source 1 (peripheral clock) + Example: can@1c000 { @@ -40,4 +47,5 @@ Example: interrupts = <48 0x2>; interrupt-parent = <&mpic>; clock-frequency = <200000000>; // filled in by bootloader + fsl,clk-source = <0>; // select clock source 0 for PE };