| Message ID | 1405074557-5519-1-git-send-email-b29396@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/11/2014 03:59 PM, Dong Aisheng wrote: > add M_CAN device tree binding documentation > > Cc: Wolfgang Grandegger <wg@grandegger.com> > Cc: Marc Kleine-Budde <mkl@pengutronix.de> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Oliver Hartkopp <socketcan@hartkopp.net> > Cc: Varka Bhadram <varkabhadram@gmail.com> > Signed-off-by: Dong Aisheng <b29396@freescale.com> > --- > .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ > 1 files changed, 65 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt > > diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt > new file mode 100644 > index 0000000..c4cb263 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/can/m_can.txt > @@ -0,0 +1,65 @@ > +Bosch MCAN controller Device Tree Bindings > +------------------------------------------------- > + > +Required properties: > +- compatible : Should be "bosch,m_can" for M_CAN controllers > +- reg : physical base address and size of the M_CAN > + registers map and Message RAM > +- reg-names : Should be "m_can" and "message_ram" > +- interrupts : Should be the interrupt number of M_CAN interrupt > + line 0 and line 1, could be same if sharing > + the same interrupt. > +- interrupt-names : Should contain "int0" and "int1" > +- clocks : Clocks used by controller, should be host clock > + and CAN clock. > +- clock-names : Should contain "hclk" and "cclk" > +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt I think this should be pinctrl-0 > +- pinctrl-names : Names corresponding to the numbered pinctrl states remove 1 tab space before : > +- mram-cfg : Message RAM configuration data. > + Multiple M_CAN instances can share the same Message RAM and each element(e.g > + Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > + so this property is telling driver how the shared or private Message RAM > + are used by this M_CAN controller. > + It may written like: mram-cfg : Message RAM configuration data Multiple M_CAN instances can share the same Message RAM and each element (e.g Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, ... > + The format should be as follows: > + <offset sidf_elems xidf_elems rxf0_elems rxf1_elems rxb_elems > + txe_elems txb_elems> > + The 'offset' is an address offset of the Message RAM where the following > + elements start from. This is usually set to 0x0 if you're using a private > + Message RAM. The remain cells are used to specify how many elements are used > + for each FIFO/Buffer. > + > +M_CAN includes the following elements according to user manual: > +11-bit Filter 0-128 elements / 0-128 words > +29-bit Filter 0-64 elements / 0-128 words > +Rx FIFO 0 0-64 elements / 0-1152 words > +Rx FIFO 1 0-64 elements / 0-1152 words > +Rx Buffers 0-64 elements / 0-1152 words > +Tx Event FIFO 0-32 elements / 0-64 words > +Tx Buffers 0-32 elements / 0-576 words > + > +Please refer to 2.4.1 Message RAM Configuration in Bosch M_CAN user manual > +for details. > + > +Example: > +SoC dtsi: > +m_can1: can@020e8000 { > + compatible = "bosch,m_can"; > + reg = <0x020e8000 0x4000>, <0x02298000 0x4000>; > + reg-names = "m_can", "message_ram"; > + interrupts = <0 114 0x04>, > + <0 114 0x04>; > + interrupt-names = "int0", "int1"; > + clocks = <&clks IMX6SX_CLK_CANFD>, > + <&clks IMX6SX_CLK_CANFD>; > + clock-names = "hclk", "cclk"; > + mram-cfg = <0x0 0 0 32 0 0 0 1>; > + status = "disabled"; > +}; > + > +Board dtsi: > +&m_can1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_m_can1>; > + status = "enabled"; > +};
On 07/11/2014 12:29 PM, Dong Aisheng wrote: > add M_CAN device tree binding documentation > > Cc: Wolfgang Grandegger <wg@grandegger.com> > Cc: Marc Kleine-Budde <mkl@pengutronix.de> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Oliver Hartkopp <socketcan@hartkopp.net> > Cc: Varka Bhadram <varkabhadram@gmail.com> > Signed-off-by: Dong Aisheng <b29396@freescale.com> > --- > .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ > 1 files changed, 65 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt > > diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt > new file mode 100644 > index 0000000..c4cb263 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/can/m_can.txt > @@ -0,0 +1,65 @@ > +Bosch MCAN controller Device Tree Bindings > +------------------------------------------------- > + > +Required properties: > +- compatible : Should be "bosch,m_can" for M_CAN controllers > +- reg : physical base address and size of the M_CAN > + registers map and Message RAM > +- reg-names : Should be "m_can" and "message_ram" > +- interrupts : Should be the interrupt number of M_CAN interrupt > + line 0 and line 1, could be same if sharing > + the same interrupt. > +- interrupt-names : Should contain "int0" and "int1" > +- clocks : Clocks used by controller, should be host clock > + and CAN clock. > +- clock-names : Should contain "hclk" and "cclk" > +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > +- pinctrl-names : Names corresponding to the numbered pinctrl states > +- mram-cfg : Message RAM configuration data. > + Multiple M_CAN instances can share the same Message RAM and each element(e.g > + Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > + so this property is telling driver how the shared or private Message RAM > + are used by this M_CAN controller. Will you have to modify the binding, espeically the mram-cfg, if you add canfd support to the driver? I think mram-cfg should have a vendor prefix, e.g.: bosch,mram-cfg > + > + The format should be as follows: > + <offset sidf_elems xidf_elems rxf0_elems rxf1_elems rxb_elems > + txe_elems txb_elems> > + The 'offset' is an address offset of the Message RAM where the following > + elements start from. This is usually set to 0x0 if you're using a private > + Message RAM. The remain cells are used to specify how many elements are used > + for each FIFO/Buffer. > + > +M_CAN includes the following elements according to user manual: > +11-bit Filter 0-128 elements / 0-128 words > +29-bit Filter 0-64 elements / 0-128 words > +Rx FIFO 0 0-64 elements / 0-1152 words > +Rx FIFO 1 0-64 elements / 0-1152 words > +Rx Buffers 0-64 elements / 0-1152 words > +Tx Event FIFO 0-32 elements / 0-64 words > +Tx Buffers 0-32 elements / 0-576 words > + > +Please refer to 2.4.1 Message RAM Configuration in Bosch M_CAN user manual > +for details. > + > +Example: > +SoC dtsi: > +m_can1: can@020e8000 { > + compatible = "bosch,m_can"; > + reg = <0x020e8000 0x4000>, <0x02298000 0x4000>; > + reg-names = "m_can", "message_ram"; > + interrupts = <0 114 0x04>, > + <0 114 0x04>; > + interrupt-names = "int0", "int1"; > + clocks = <&clks IMX6SX_CLK_CANFD>, > + <&clks IMX6SX_CLK_CANFD>; > + clock-names = "hclk", "cclk"; > + mram-cfg = <0x0 0 0 32 0 0 0 1>; > + status = "disabled"; > +}; > + > +Board dtsi: > +&m_can1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_m_can1>; > + status = "enabled"; > +}; > Marc
On Fri, Jul 11, 2014 at 04:11:03PM +0530, Varka Bhadram wrote: > On 07/11/2014 03:59 PM, Dong Aisheng wrote: > >add M_CAN device tree binding documentation > > > >Cc: Wolfgang Grandegger <wg@grandegger.com> > >Cc: Marc Kleine-Budde <mkl@pengutronix.de> > >Cc: Mark Rutland <mark.rutland@arm.com> > >Cc: Oliver Hartkopp <socketcan@hartkopp.net> > >Cc: Varka Bhadram <varkabhadram@gmail.com> > >Signed-off-by: Dong Aisheng <b29396@freescale.com> > >--- > > .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ > > 1 files changed, 65 insertions(+), 0 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt > > > >diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt > >new file mode 100644 > >index 0000000..c4cb263 > >--- /dev/null > >+++ b/Documentation/devicetree/bindings/net/can/m_can.txt > >@@ -0,0 +1,65 @@ > >+Bosch MCAN controller Device Tree Bindings > >+------------------------------------------------- > >+ > >+Required properties: > >+- compatible : Should be "bosch,m_can" for M_CAN controllers > >+- reg : physical base address and size of the M_CAN > >+ registers map and Message RAM > >+- reg-names : Should be "m_can" and "message_ram" > >+- interrupts : Should be the interrupt number of M_CAN interrupt > >+ line 0 and line 1, could be same if sharing > >+ the same interrupt. > >+- interrupt-names : Should contain "int0" and "int1" > >+- clocks : Clocks used by controller, should be host clock > >+ and CAN clock. > >+- clock-names : Should contain "hclk" and "cclk" > >+- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > > I think this should be pinctrl-0 > First, this part is defined by pinctrl binding doc. Second i think it may be possible someone wants to add other pinctrl states when implement low power state in the future. So i just keep it as pinctrl-<n>. > >+- pinctrl-names : Names corresponding to the numbered pinctrl states > > remove 1 tab space before : > It's a bit strange. Other line like pinctrl-<n> is also two tabs. And the code looks fine and already aligned. - pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt - pinctrl-names : Names corresponding to the numbered pinctrl states Do you mean change line of pinctrl-names from two tabs to one space and a tab before :? > >+- mram-cfg : Message RAM configuration data. > >+ Multiple M_CAN instances can share the same Message RAM and each element(e.g > >+ Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > >+ so this property is telling driver how the shared or private Message RAM > >+ are used by this M_CAN controller. > >+ > > It may written like: > mram-cfg : Message RAM configuration data > Multiple M_CAN instances can share the same Message RAM and each element > (e.g Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > ... > I'm fine with that. The question is it's easy to over 80 columns if writing like that, is it ok? Regards Dong Aisheng > >+ The format should be as follows: > >+ <offset sidf_elems xidf_elems rxf0_elems rxf1_elems rxb_elems > >+ txe_elems txb_elems> > >+ The 'offset' is an address offset of the Message RAM where the following > >+ elements start from. This is usually set to 0x0 if you're using a private > >+ Message RAM. The remain cells are used to specify how many elements are used > >+ for each FIFO/Buffer. > >+ > >+M_CAN includes the following elements according to user manual: > >+11-bit Filter 0-128 elements / 0-128 words > >+29-bit Filter 0-64 elements / 0-128 words > >+Rx FIFO 0 0-64 elements / 0-1152 words > >+Rx FIFO 1 0-64 elements / 0-1152 words > >+Rx Buffers 0-64 elements / 0-1152 words > >+Tx Event FIFO 0-32 elements / 0-64 words > >+Tx Buffers 0-32 elements / 0-576 words > >+ > >+Please refer to 2.4.1 Message RAM Configuration in Bosch M_CAN user manual > >+for details. > >+ > >+Example: > >+SoC dtsi: > >+m_can1: can@020e8000 { > >+ compatible = "bosch,m_can"; > >+ reg = <0x020e8000 0x4000>, <0x02298000 0x4000>; > >+ reg-names = "m_can", "message_ram"; > >+ interrupts = <0 114 0x04>, > >+ <0 114 0x04>; > >+ interrupt-names = "int0", "int1"; > >+ clocks = <&clks IMX6SX_CLK_CANFD>, > >+ <&clks IMX6SX_CLK_CANFD>; > >+ clock-names = "hclk", "cclk"; > >+ mram-cfg = <0x0 0 0 32 0 0 0 1>; > >+ status = "disabled"; > >+}; > >+ > >+Board dtsi: > >+&m_can1 { > >+ pinctrl-names = "default"; > >+ pinctrl-0 = <&pinctrl_m_can1>; > >+ status = "enabled"; > >+}; > > > -- > Regards, > Varka Bhadram. >
On 07/14/2014 08:54 AM, Dong Aisheng wrote: > On Fri, Jul 11, 2014 at 04:11:03PM +0530, Varka Bhadram wrote: >> On 07/11/2014 03:59 PM, Dong Aisheng wrote: >>> add M_CAN device tree binding documentation >>> >>> Cc: Wolfgang Grandegger <wg@grandegger.com> >>> Cc: Marc Kleine-Budde <mkl@pengutronix.de> >>> Cc: Mark Rutland <mark.rutland@arm.com> >>> Cc: Oliver Hartkopp <socketcan@hartkopp.net> >>> Cc: Varka Bhadram <varkabhadram@gmail.com> >>> Signed-off-by: Dong Aisheng <b29396@freescale.com> >>> --- >>> .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ >>> 1 files changed, 65 insertions(+), 0 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt >>> >>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt >>> new file mode 100644 >>> index 0000000..c4cb263 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt >>> @@ -0,0 +1,65 @@ >>> +Bosch MCAN controller Device Tree Bindings >>> +------------------------------------------------- >>> + >>> +Required properties: >>> +- compatible : Should be "bosch,m_can" for M_CAN controllers >>> +- reg : physical base address and size of the M_CAN >>> + registers map and Message RAM >>> +- reg-names : Should be "m_can" and "message_ram" >>> +- interrupts : Should be the interrupt number of M_CAN interrupt >>> + line 0 and line 1, could be same if sharing >>> + the same interrupt. >>> +- interrupt-names : Should contain "int0" and "int1" >>> +- clocks : Clocks used by controller, should be host clock >>> + and CAN clock. >>> +- clock-names : Should contain "hclk" and "cclk" >>> +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt >> I think this should be pinctrl-0 >> > First, this part is defined by pinctrl binding doc. > Second i think it may be possible someone wants to add other pinctrl states > when implement low power state in the future. > So i just keep it as pinctrl-<n>. Normally we will use pinctrl-0 for mentioning the pinctrl bindings. If somebody going to add something to the pinctrl bindings they will add separately with pinctrl-1: bla bla bla... >>> +- pinctrl-names : Names corresponding to the numbered pinctrl states >> remove 1 tab space before : >> > It's a bit strange. > Other line like pinctrl-<n> is also two tabs. > And the code looks fine and already aligned. > - pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > - pinctrl-names : Names corresponding to the numbered pinctrl states > Do you mean change line of pinctrl-names from two tabs to one space and a tab before :? > When i see in the patch the alignment is like this pinctrl-<n> : pinctrl-name : >>> +- mram-cfg : Message RAM configuration data. >>> + Multiple M_CAN instances can share the same Message RAM and each element(e.g >>> + Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, >>> + so this property is telling driver how the shared or private Message RAM >>> + are used by this M_CAN controller. >>> + >> It may written like: >> mram-cfg : Message RAM configuration data >> Multiple M_CAN instances can share the same Message RAM and each element >> (e.g Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, >> ... >> > I'm fine with that. > The question is it's easy to over 80 columns if writing like that, > is it ok? When we follow the above format it would be more readable. > Regards > Dong Aisheng
On Mon, Jul 14, 2014 at 10:07:06AM +0530, Varka Bhadram wrote: > On 07/14/2014 08:54 AM, Dong Aisheng wrote: > >On Fri, Jul 11, 2014 at 04:11:03PM +0530, Varka Bhadram wrote: > >>On 07/11/2014 03:59 PM, Dong Aisheng wrote: > >>>add M_CAN device tree binding documentation > >>> > >>>Cc: Wolfgang Grandegger <wg@grandegger.com> > >>>Cc: Marc Kleine-Budde <mkl@pengutronix.de> > >>>Cc: Mark Rutland <mark.rutland@arm.com> > >>>Cc: Oliver Hartkopp <socketcan@hartkopp.net> > >>>Cc: Varka Bhadram <varkabhadram@gmail.com> > >>>Signed-off-by: Dong Aisheng <b29396@freescale.com> > >>>--- > >>> .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ > >>> 1 files changed, 65 insertions(+), 0 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt > >>> > >>>diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt > >>>new file mode 100644 > >>>index 0000000..c4cb263 > >>>--- /dev/null > >>>+++ b/Documentation/devicetree/bindings/net/can/m_can.txt > >>>@@ -0,0 +1,65 @@ > >>>+Bosch MCAN controller Device Tree Bindings > >>>+------------------------------------------------- > >>>+ > >>>+Required properties: > >>>+- compatible : Should be "bosch,m_can" for M_CAN controllers > >>>+- reg : physical base address and size of the M_CAN > >>>+ registers map and Message RAM > >>>+- reg-names : Should be "m_can" and "message_ram" > >>>+- interrupts : Should be the interrupt number of M_CAN interrupt > >>>+ line 0 and line 1, could be same if sharing > >>>+ the same interrupt. > >>>+- interrupt-names : Should contain "int0" and "int1" > >>>+- clocks : Clocks used by controller, should be host clock > >>>+ and CAN clock. > >>>+- clock-names : Should contain "hclk" and "cclk" > >>>+- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > >>I think this should be pinctrl-0 > >> > >First, this part is defined by pinctrl binding doc. > >Second i think it may be possible someone wants to add other pinctrl states > >when implement low power state in the future. > >So i just keep it as pinctrl-<n>. > > Normally we will use pinctrl-0 for mentioning the pinctrl bindings. > > If somebody going to add something to the pinctrl bindings they will > add separately with pinctrl-1: bla bla bla... > Will it cause misleading that it only supports one state? And if we only define pinctrl-0 here, how do we describe pinctrl-names? It should be "default"? It looks not accurate enough to me. Per my understanding, I think it's better to leave it as standard pinctrl-binding doc states since anyhow people should read pinctrl-binding doc. > >>>+- pinctrl-names : Names corresponding to the numbered pinctrl states > >>remove 1 tab space before : > >> > >It's a bit strange. > >Other line like pinctrl-<n> is also two tabs. > >And the code looks fine and already aligned. > >- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > >- pinctrl-names : Names corresponding to the numbered pinctrl states > >Do you mean change line of pinctrl-names from two tabs to one space and a tab before :? > > > When i see in the patch the alignment is like this > pinctrl-<n> : > pinctrl-name : > Yes,in the original patch it's like: +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt +- pinctrl-names : Names corresponding to the numbered pinctrl states If remove one tab: +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt +- pinctrl-names : Names corresponding to the numbered pinctrl states But in vim reading the code, it's like: - pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt - pinctrl-names : Names corresponding to the numbered pinctrl states I don't know why. What should we do about this case? > >>>+- mram-cfg : Message RAM configuration data. > >>>+ Multiple M_CAN instances can share the same Message RAM and each element(e.g > >>>+ Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > >>>+ so this property is telling driver how the shared or private Message RAM > >>>+ are used by this M_CAN controller. > >>>+ > >>It may written like: > >>mram-cfg : Message RAM configuration data > >> Multiple M_CAN instances can share the same Message RAM and each element > >> (e.g Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > >> ... > >> > >I'm fine with that. > >The question is it's easy to over 80 columns if writing like that, > >is it ok? > > When we follow the above format it would be more readable. > Okay. > >Regards > >Dong Aisheng > > > -- > Regards, > Varka Bhadram. > Regards Dong Aisheng
On 07/14/2014 10:34 AM, Dong Aisheng wrote: > On Mon, Jul 14, 2014 at 10:07:06AM +0530, Varka Bhadram wrote: >> On 07/14/2014 08:54 AM, Dong Aisheng wrote: >>> On Fri, Jul 11, 2014 at 04:11:03PM +0530, Varka Bhadram wrote: >>>> On 07/11/2014 03:59 PM, Dong Aisheng wrote: >>>>> add M_CAN device tree binding documentation >>>>> >>>>> Cc: Wolfgang Grandegger<wg@grandegger.com> >>>>> Cc: Marc Kleine-Budde<mkl@pengutronix.de> >>>>> Cc: Mark Rutland<mark.rutland@arm.com> >>>>> Cc: Oliver Hartkopp<socketcan@hartkopp.net> >>>>> Cc: Varka Bhadram<varkabhadram@gmail.com> >>>>> Signed-off-by: Dong Aisheng<b29396@freescale.com> >>>>> --- >>>>> .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ >>>>> 1 files changed, 65 insertions(+), 0 deletions(-) >>>>> create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt >>>>> new file mode 100644 >>>>> index 0000000..c4cb263 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt >>>>> @@ -0,0 +1,65 @@ >>>>> +Bosch MCAN controller Device Tree Bindings >>>>> +------------------------------------------------- >>>>> + >>>>> +Required properties: >>>>> +- compatible : Should be "bosch,m_can" for M_CAN controllers >>>>> +- reg : physical base address and size of the M_CAN >>>>> + registers map and Message RAM >>>>> +- reg-names : Should be "m_can" and "message_ram" >>>>> +- interrupts : Should be the interrupt number of M_CAN interrupt >>>>> + line 0 and line 1, could be same if sharing >>>>> + the same interrupt. >>>>> +- interrupt-names : Should contain "int0" and "int1" >>>>> +- clocks : Clocks used by controller, should be host clock >>>>> + and CAN clock. >>>>> +- clock-names : Should contain "hclk" and "cclk" >>>>> +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt >>>> I think this should be pinctrl-0 >>>> >>> First, this part is defined by pinctrl binding doc. >>> Second i think it may be possible someone wants to add other pinctrl states >>> when implement low power state in the future. >>> So i just keep it as pinctrl-<n>. >> Normally we will use pinctrl-0 for mentioning the pinctrl bindings. >> >> If somebody going to add something to the pinctrl bindings they will >> add separately with pinctrl-1: bla bla bla... >> > Will it cause misleading that it only supports one state? > And if we only define pinctrl-0 here, how do we describe pinctrl-names? > It should be "default"? It looks not accurate enough to me. > > Per my understanding, I think it's better to leave it as standard > pinctrl-binding doc states since anyhow people should read pinctrl-binding doc. > >>>>> +- pinctrl-names : Names corresponding to the numbered pinctrl states >>>> remove 1 tab space before : >>>> >>> It's a bit strange. >>> Other line like pinctrl-<n> is also two tabs. >>> And the code looks fine and already aligned. >>> - pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt >>> - pinctrl-names : Names corresponding to the numbered pinctrl states >>> Do you mean change line of pinctrl-names from two tabs to one space and a tab before :? >>> >> When i see in the patch the alignment is like this >> pinctrl-<n> : >> pinctrl-name : >> > Yes,in the original patch it's like: > +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > +- pinctrl-names : Names corresponding to the numbered pinctrl states > > If remove one tab: > +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > +- pinctrl-names : Names corresponding to the numbered pinctrl states > > But in vim reading the code, it's like: > - pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > - pinctrl-names : Names corresponding to the numbered pinctrl states > I don't know why. > What should we do about this case? It seems to me that the style followed in bindings/pinctrl/pinctrl-bindings.txt looks good to me. pinctrl-<n>: Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt pinctrl-names: Names corresponding to the numbered pinctrl states >>>>> +- mram-cfg : Message RAM configuration data. >>>>> + Multiple M_CAN instances can share the same Message RAM and each element(e.g >>>>> + Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, >>>>> + so this property is telling driver how the shared or private Message RAM >>>>> + are used by this M_CAN controller. >>>>> + >>>> It may written like: >>>> mram-cfg : Message RAM configuration data >>>> Multiple M_CAN instances can share the same Message RAM and each element >>>> (e.g Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, >>>> ... >>>> >>> I'm fine with that. >>> The question is it's easy to over 80 columns if writing like that, >>> is it ok? >> When we follow the above format it would be more readable. >> > Okay. > >>> Regards >>> Dong Aisheng >> -- >> Regards, >> Varka Bhadram. >> > Regards > Dong Aisheng > >
On Fri, Jul 11, 2014 at 02:54:00PM +0200, Marc Kleine-Budde wrote: > On 07/11/2014 12:29 PM, Dong Aisheng wrote: > > add M_CAN device tree binding documentation > > > > Cc: Wolfgang Grandegger <wg@grandegger.com> > > Cc: Marc Kleine-Budde <mkl@pengutronix.de> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Oliver Hartkopp <socketcan@hartkopp.net> > > Cc: Varka Bhadram <varkabhadram@gmail.com> > > Signed-off-by: Dong Aisheng <b29396@freescale.com> > > --- > > .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ > > 1 files changed, 65 insertions(+), 0 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt > > > > diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt > > new file mode 100644 > > index 0000000..c4cb263 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/can/m_can.txt > > @@ -0,0 +1,65 @@ > > +Bosch MCAN controller Device Tree Bindings > > +------------------------------------------------- > > + > > +Required properties: > > +- compatible : Should be "bosch,m_can" for M_CAN controllers > > +- reg : physical base address and size of the M_CAN > > + registers map and Message RAM > > +- reg-names : Should be "m_can" and "message_ram" > > +- interrupts : Should be the interrupt number of M_CAN interrupt > > + line 0 and line 1, could be same if sharing > > + the same interrupt. > > +- interrupt-names : Should contain "int0" and "int1" > > +- clocks : Clocks used by controller, should be host clock > > + and CAN clock. > > +- clock-names : Should contain "hclk" and "cclk" > > +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt > > +- pinctrl-names : Names corresponding to the numbered pinctrl states > > +- mram-cfg : Message RAM configuration data. > > + Multiple M_CAN instances can share the same Message RAM and each element(e.g > > + Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, > > + so this property is telling driver how the shared or private Message RAM > > + are used by this M_CAN controller. > > Will you have to modify the binding, espeically the mram-cfg, if you add > canfd support to the driver? > No need, CANFD in i.MX6SX is using the same IP of Bosch M_CAN. > I think mram-cfg should have a vendor prefix, e.g.: bosch,mram-cfg > It's a good idea. Will update it. > > + > > + The format should be as follows: > > + <offset sidf_elems xidf_elems rxf0_elems rxf1_elems rxb_elems > > + txe_elems txb_elems> > > + The 'offset' is an address offset of the Message RAM where the following > > + elements start from. This is usually set to 0x0 if you're using a private > > + Message RAM. The remain cells are used to specify how many elements are used > > + for each FIFO/Buffer. > > + > > +M_CAN includes the following elements according to user manual: > > +11-bit Filter 0-128 elements / 0-128 words > > +29-bit Filter 0-64 elements / 0-128 words > > +Rx FIFO 0 0-64 elements / 0-1152 words > > +Rx FIFO 1 0-64 elements / 0-1152 words > > +Rx Buffers 0-64 elements / 0-1152 words > > +Tx Event FIFO 0-32 elements / 0-64 words > > +Tx Buffers 0-32 elements / 0-576 words > > + > > +Please refer to 2.4.1 Message RAM Configuration in Bosch M_CAN user manual > > +for details. > > + > > +Example: > > +SoC dtsi: > > +m_can1: can@020e8000 { > > + compatible = "bosch,m_can"; > > + reg = <0x020e8000 0x4000>, <0x02298000 0x4000>; > > + reg-names = "m_can", "message_ram"; > > + interrupts = <0 114 0x04>, > > + <0 114 0x04>; > > + interrupt-names = "int0", "int1"; > > + clocks = <&clks IMX6SX_CLK_CANFD>, > > + <&clks IMX6SX_CLK_CANFD>; > > + clock-names = "hclk", "cclk"; > > + mram-cfg = <0x0 0 0 32 0 0 0 1>; > > + status = "disabled"; > > +}; > > + > > +Board dtsi: > > +&m_can1 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_m_can1>; > > + status = "enabled"; > > +}; > > > > Marc > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions | Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | > Regards Dong Aisheng
diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt new file mode 100644 index 0000000..c4cb263 --- /dev/null +++ b/Documentation/devicetree/bindings/net/can/m_can.txt @@ -0,0 +1,65 @@ +Bosch MCAN controller Device Tree Bindings +------------------------------------------------- + +Required properties: +- compatible : Should be "bosch,m_can" for M_CAN controllers +- reg : physical base address and size of the M_CAN + registers map and Message RAM +- reg-names : Should be "m_can" and "message_ram" +- interrupts : Should be the interrupt number of M_CAN interrupt + line 0 and line 1, could be same if sharing + the same interrupt. +- interrupt-names : Should contain "int0" and "int1" +- clocks : Clocks used by controller, should be host clock + and CAN clock. +- clock-names : Should contain "hclk" and "cclk" +- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt +- pinctrl-names : Names corresponding to the numbered pinctrl states +- mram-cfg : Message RAM configuration data. + Multiple M_CAN instances can share the same Message RAM and each element(e.g + Rx FIFO or Tx Buffer and etc) number in Message RAM is also configurable, + so this property is telling driver how the shared or private Message RAM + are used by this M_CAN controller. + + The format should be as follows: + <offset sidf_elems xidf_elems rxf0_elems rxf1_elems rxb_elems + txe_elems txb_elems> + The 'offset' is an address offset of the Message RAM where the following + elements start from. This is usually set to 0x0 if you're using a private + Message RAM. The remain cells are used to specify how many elements are used + for each FIFO/Buffer. + +M_CAN includes the following elements according to user manual: +11-bit Filter 0-128 elements / 0-128 words +29-bit Filter 0-64 elements / 0-128 words +Rx FIFO 0 0-64 elements / 0-1152 words +Rx FIFO 1 0-64 elements / 0-1152 words +Rx Buffers 0-64 elements / 0-1152 words +Tx Event FIFO 0-32 elements / 0-64 words +Tx Buffers 0-32 elements / 0-576 words + +Please refer to 2.4.1 Message RAM Configuration in Bosch M_CAN user manual +for details. + +Example: +SoC dtsi: +m_can1: can@020e8000 { + compatible = "bosch,m_can"; + reg = <0x020e8000 0x4000>, <0x02298000 0x4000>; + reg-names = "m_can", "message_ram"; + interrupts = <0 114 0x04>, + <0 114 0x04>; + interrupt-names = "int0", "int1"; + clocks = <&clks IMX6SX_CLK_CANFD>, + <&clks IMX6SX_CLK_CANFD>; + clock-names = "hclk", "cclk"; + mram-cfg = <0x0 0 0 32 0 0 0 1>; + status = "disabled"; +}; + +Board dtsi: +&m_can1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_m_can1>; + status = "enabled"; +};
add M_CAN device tree binding documentation Cc: Wolfgang Grandegger <wg@grandegger.com> Cc: Marc Kleine-Budde <mkl@pengutronix.de> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Oliver Hartkopp <socketcan@hartkopp.net> Cc: Varka Bhadram <varkabhadram@gmail.com> Signed-off-by: Dong Aisheng <b29396@freescale.com> --- .../devicetree/bindings/net/can/m_can.txt | 65 ++++++++++++++++++++ 1 files changed, 65 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/can/m_can.txt