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@xxxxxxxxxxxxxx> > >>>Cc: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> > >>>Cc: Mark Rutland <mark.rutland@xxxxxxx> > >>>Cc: Oliver Hartkopp <socketcan@xxxxxxxxxxxx> > >>>Cc: Varka Bhadram <varkabhadram@xxxxxxxxx> > >>>Signed-off-by: Dong Aisheng <b29396@xxxxxxxxxxxxx> > >>>--- > >>> .../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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
