On Fri, Jul 11, 2014 at 04:11:03PM +0530, Varka Bhadram wrote:

On 07/11/2014 03:59 PM, Dong Aisheng wrote:

quoted

add M_CAN device tree binding documentation

Cc: Wolfgang Grandegger <redacted>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Varka Bhadram <redacted>
Signed-off-by: Dong Aisheng <redacted>
---
 .../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>.

quoted

+- 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 :?

quoted

+- 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

quoted

+  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 at 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.