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