| Message ID | 1312945564-6626-6-git-send-email-holt@sgi.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Robin, On Tue, Aug 09, 2011 at 10:06:04PM -0500, Robin Holt wrote: > In working with the socketcan developers, we have come to the conclusion > the Documentation...fsl-flexcan.txt device tree documentation needs to > be cleaned up. The driver does not depend upon any properties other > than the required properties so we are removing the file. Additionally, > the p1010*dts* files are not following the standard for node naming in > that they have a trailing -v1.0. > > Signed-off-by: Robin Holt <holt@sgi.com> > To: Marc Kleine-Budde <mkl@pengutronix.de>, > To: Wolfgang Grandegger <wg@grandegger.com>, > To: U Bhaskar-B22300 <B22300@freescale.com> > To: Scott Wood <scottwood@freescale.com> > Cc: socketcan-core@lists.berlios.de, > Cc: netdev@vger.kernel.org, > Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > Cc: Kumar Gala <galak@kernel.crashing.org> > --- > .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- > arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > 3 files changed, 4 insertions(+), 73 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt I suggest that you set devicetree-discuss@lists.ozlabs.org and Grant Likely on Cc: for this patch. rsc
Grant, Do you need this patch resent with you on the Cc: list or can you pick up the discussion from here? I am just trying to minimize noise on the mailing lists if it is not needed. The essence of the discussion to this point is: 1) Freescale built a board support package for a new processor and eval board (p1010) based upon some older boards. 2) As part of that effort, they made changes as they saw fit to get it working without submitting those changes to the community. 3) In the case of the flexcan device, that included defining a device tree node for the integrated Controller Area Network device and set .compatible="fsl,flexcan-v1.0". Apparently this was copied from another device, possibly mscan. 4) In an effort to get my p1010rdb eval board working with a community kernel, I started submitting patches for flexcan which have evolved to the point of nearly being accepted with the exception of we need to settle on the node's .compatible= strings and whether it should be documented. 5) To this point, I believe the concensus has settled on "fsl,p1010-flexcan" and "fsl,flexcan" as the compatible strings. This leaves the most flexibility for segragating future specific revisions of the flexcan interface if needed. 6) I failed to Cc: you or the device tree list when I submitted version 10 of the patches. I have added both to the patch header so they will be included in the next pass. Thanks, Robin Holt On Tue, Aug 09, 2011 at 10:06:04PM -0500, Robin Holt wrote: > In working with the socketcan developers, we have come to the conclusion > the Documentation...fsl-flexcan.txt device tree documentation needs to > be cleaned up. The driver does not depend upon any properties other > than the required properties so we are removing the file. Additionally, > the p1010*dts* files are not following the standard for node naming in > that they have a trailing -v1.0. > > Signed-off-by: Robin Holt <holt@sgi.com> > To: Marc Kleine-Budde <mkl@pengutronix.de>, > To: Wolfgang Grandegger <wg@grandegger.com>, > To: U Bhaskar-B22300 <B22300@freescale.com> > To: Scott Wood <scottwood@freescale.com> > Cc: socketcan-core@lists.berlios.de, > Cc: netdev@vger.kernel.org, > Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > Cc: Kumar Gala <galak@kernel.crashing.org> > --- > .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- > arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > 3 files changed, 4 insertions(+), 73 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > deleted file mode 100644 > index 1a729f0..0000000 > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > +++ /dev/null > @@ -1,61 +0,0 @@ > -CAN Device Tree Bindings > ------------------------- > -2011 Freescale Semiconductor, Inc. > - > -fsl,flexcan-v1.0 nodes > ------------------------ > -In addition to the required compatible-, reg- and interrupt-properties, you can > -also specify which clock source shall be used for the controller. > - > -CPI Clock- Can Protocol Interface Clock > - This CLK_SRC bit of CTRL(control register) selects the clock source to > - the CAN Protocol Interface(CPI) to be either the peripheral clock > - (driven by the PLL) or the crystal oscillator clock. The selected clock > - is the one fed to the prescaler to generate the Serial Clock (Sclock). > - The PRESDIV field of CTRL(control register) controls a prescaler that > - generates the Serial Clock (Sclock), whose period defines the > - time quantum used to compose the CAN waveform. > - > -Can Engine Clock Source > - There are two sources for CAN clock > - - Platform Clock It represents the bus clock > - - Oscillator Clock > - > - Peripheral Clock (PLL) > - -------------- > - | > - --------- ------------- > - | |CPI Clock | Prescaler | Sclock > - | |---------------->| (1.. 256) |------------> > - --------- ------------- > - | | > - -------------- ---------------------CLK_SRC > - Oscillator Clock > - > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects > - the peripheral clock. PLL clock is fed to the > - prescaler to generate the Serial Clock (Sclock). > - Valid values are "oscillator" and "platform" > - "oscillator": CAN engine clock source is oscillator clock. > - "platform" The CAN engine clock source is the bus clock > - (platform clock). > - > -- fsl,flexcan-clock-divider : for the reference and system clock, an additional > - clock divider can be specified. > -- clock-frequency: frequency required to calculate the bitrate for FlexCAN. > - > -Note: > - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC. > - - P1010 does not have oscillator as the Clock Source.So the default > - Clock Source is platform clock. > -Examples: > - > - can0@1c000 { > - compatible = "fsl,flexcan-v1.0"; > - reg = <0x1c000 0x1000>; > - interrupts = <48 0x2>; > - interrupt-parent = <&mpic>; > - fsl,flexcan-clock-source = "platform"; > - fsl,flexcan-clock-divider = <2>; > - clock-frequency = <fixed by u-boot>; > - }; > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts > index 6b33b73..d6a0bb2 100644 > --- a/arch/powerpc/boot/dts/p1010rdb.dts > +++ b/arch/powerpc/boot/dts/p1010rdb.dts > @@ -169,14 +169,6 @@ > }; > }; > > - can0@1c000 { > - fsl,flexcan-clock-source = "platform"; > - }; > - > - can1@1d000 { > - fsl,flexcan-clock-source = "platform"; > - }; > - > usb@22000 { > phy_type = "utmi"; > }; > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi > index 7f51104..20c396d 100644 > --- a/arch/powerpc/boot/dts/p1010si.dtsi > +++ b/arch/powerpc/boot/dts/p1010si.dtsi > @@ -141,19 +141,19 @@ > }; > > can0@1c000 { > - compatible = "fsl,flexcan-v1.0"; > + compatible = "fsl,p1010-flexcan", > + "fsl,flexcan"; > reg = <0x1c000 0x1000>; > interrupts = <48 0x2>; > interrupt-parent = <&mpic>; > - fsl,flexcan-clock-divider = <2>; > }; > > can1@1d000 { > - compatible = "fsl,flexcan-v1.0"; > + compatible = "fsl,p1010-flexcan", > + "fsl,flexcan"; > reg = <0x1d000 0x1000>; > interrupts = <61 0x2>; > interrupt-parent = <&mpic>; > - fsl,flexcan-clock-divider = <2>; > }; > > L2: l2-cache-controller@20000 { > -- > 1.7.2.1
Hi Robin, On 08/10/2011 05:06 AM, Robin Holt wrote: > In working with the socketcan developers, we have come to the conclusion > the Documentation...fsl-flexcan.txt device tree documentation needs to > be cleaned up. The driver does not depend upon any properties other Your first sentence could be misleading. Please just describe what the patch does and why, something like: "This patch cleans up the documentation of the device-tree binding for the Flexcan devices on Freescale's PowerPC and ARM cores. Extra properties are not needed as the frequency of the source clock is fixed..." and so on. > than the required properties so we are removing the file. Additionally, > the p1010*dts* files are not following the standard for node naming in > that they have a trailing -v1.0. > Signed-off-by: Robin Holt <holt@sgi.com> > To: Marc Kleine-Budde <mkl@pengutronix.de>, > To: Wolfgang Grandegger <wg@grandegger.com>, > To: U Bhaskar-B22300 <B22300@freescale.com> > To: Scott Wood <scottwood@freescale.com> > Cc: socketcan-core@lists.berlios.de, > Cc: netdev@vger.kernel.org, > Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > Cc: Kumar Gala <galak@kernel.crashing.org> > --- > .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- > arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > 3 files changed, 4 insertions(+), 73 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > deleted file mode 100644 > index 1a729f0..0000000 > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > +++ /dev/null > @@ -1,61 +0,0 @@ > -CAN Device Tree Bindings > ------------------------- > -2011 Freescale Semiconductor, Inc. > - > -fsl,flexcan-v1.0 nodes > ------------------------ > -In addition to the required compatible-, reg- and interrupt-properties, you can > -also specify which clock source shall be used for the controller. > - > -CPI Clock- Can Protocol Interface Clock > - This CLK_SRC bit of CTRL(control register) selects the clock source to > - the CAN Protocol Interface(CPI) to be either the peripheral clock > - (driven by the PLL) or the crystal oscillator clock. The selected clock > - is the one fed to the prescaler to generate the Serial Clock (Sclock). > - The PRESDIV field of CTRL(control register) controls a prescaler that > - generates the Serial Clock (Sclock), whose period defines the > - time quantum used to compose the CAN waveform. > - > -Can Engine Clock Source > - There are two sources for CAN clock > - - Platform Clock It represents the bus clock > - - Oscillator Clock > - > - Peripheral Clock (PLL) > - -------------- > - | > - --------- ------------- > - | |CPI Clock | Prescaler | Sclock > - | |---------------->| (1.. 256) |------------> > - --------- ------------- > - | | > - -------------- ---------------------CLK_SRC > - Oscillator Clock > - > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects > - the peripheral clock. PLL clock is fed to the > - prescaler to generate the Serial Clock (Sclock). > - Valid values are "oscillator" and "platform" > - "oscillator": CAN engine clock source is oscillator clock. > - "platform" The CAN engine clock source is the bus clock > - (platform clock). > - > -- fsl,flexcan-clock-divider : for the reference and system clock, an additional > - clock divider can be specified. > -- clock-frequency: frequency required to calculate the bitrate for FlexCAN. > - > -Note: > - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC. > - - P1010 does not have oscillator as the Clock Source.So the default > - Clock Source is platform clock. > -Examples: > - > - can0@1c000 { > - compatible = "fsl,flexcan-v1.0"; > - reg = <0x1c000 0x1000>; > - interrupts = <48 0x2>; > - interrupt-parent = <&mpic>; > - fsl,flexcan-clock-source = "platform"; > - fsl,flexcan-clock-divqider = <2>; > - clock-frequency = <fixed by u-boot>; > - }; Do we really want to drop the documentation for that binding. I think something like the following text would be still useful: ------------------------ Flexcan CAN contoller on Freescale's ARM and PowerPC processors Required properties: - compatible : Should be "fsl,flexcan" and optionally "fsl,flexcan-<processor>" - reg : Offset and length of the register set for this device - interrupts : Interrupt tuple for this device Example: can@1c000 { compatible = "fsl,p1010-flexcan", "fsl,flexcan"; reg = <0x1c000 0x1000>; interrupts = <48 0x2>; interrupt-parent = <&mpic>; }; ------------------------- What do you think? > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts > index 6b33b73..d6a0bb2 100644 > --- a/arch/powerpc/boot/dts/p1010rdb.dts > +++ b/arch/powerpc/boot/dts/p1010rdb.dts > @@ -169,14 +169,6 @@ > }; > }; > > - can0@1c000 { > - fsl,flexcan-clock-source = "platform"; > - }; > - > - can1@1d000 { > - fsl,flexcan-clock-source = "platform"; > - }; > - > usb@22000 { > phy_type = "utmi"; > }; > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi > index 7f51104..20c396d 100644 > --- a/arch/powerpc/boot/dts/p1010si.dtsi > +++ b/arch/powerpc/boot/dts/p1010si.dtsi > @@ -141,19 +141,19 @@ > }; > > can0@1c000 { > - compatible = "fsl,flexcan-v1.0"; > + compatible = "fsl,p1010-flexcan", > + "fsl,flexcan"; Does fit on one line. > reg = <0x1c000 0x1000>; > interrupts = <48 0x2>; > interrupt-parent = <&mpic>; > - fsl,flexcan-clock-divider = <2>; > }; > > can1@1d000 { > - compatible = "fsl,flexcan-v1.0"; > + compatible = "fsl,p1010-flexcan", > + "fsl,flexcan"; Ditto > reg = <0x1d000 0x1000>; > interrupts = <61 0x2>; > interrupt-parent = <&mpic>; > - fsl,flexcan-clock-divider = <2>; > }; > > L2: l2-cache-controller@20000 { Please also correct the node names (not using the number suffix). Wolfgang.
On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > Hi Robin, > > On 08/10/2011 05:06 AM, Robin Holt wrote: > > In working with the socketcan developers, we have come to the conclusion > > the Documentation...fsl-flexcan.txt device tree documentation needs to > > be cleaned up. The driver does not depend upon any properties other > > Your first sentence could be misleading. Please just describe what the > patch does and why, something like: > > "This patch cleans up the documentation of the device-tree binding for > the Flexcan devices on Freescale's PowerPC and ARM cores. Extra > properties are not needed as the frequency of the source clock is > fixed..." and so on. I borrowed heavily from your message. ;) > > than the required properties so we are removing the file. Additionally, > > the p1010*dts* files are not following the standard for node naming in > > that they have a trailing -v1.0. > > > Signed-off-by: Robin Holt <holt@sgi.com> > > To: Marc Kleine-Budde <mkl@pengutronix.de>, > > To: Wolfgang Grandegger <wg@grandegger.com>, > > To: U Bhaskar-B22300 <B22300@freescale.com> > > To: Scott Wood <scottwood@freescale.com> > > Cc: socketcan-core@lists.berlios.de, > > Cc: netdev@vger.kernel.org, > > Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > > Cc: Kumar Gala <galak@kernel.crashing.org> > > --- > > .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- > > arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > > arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > > 3 files changed, 4 insertions(+), 73 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > deleted file mode 100644 > > index 1a729f0..0000000 > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > +++ /dev/null > > @@ -1,61 +0,0 @@ > > -CAN Device Tree Bindings > > ------------------------- > > -2011 Freescale Semiconductor, Inc. > > - > > -fsl,flexcan-v1.0 nodes > > ------------------------ > > -In addition to the required compatible-, reg- and interrupt-properties, you can > > -also specify which clock source shall be used for the controller. > > - > > -CPI Clock- Can Protocol Interface Clock > > - This CLK_SRC bit of CTRL(control register) selects the clock source to > > - the CAN Protocol Interface(CPI) to be either the peripheral clock > > - (driven by the PLL) or the crystal oscillator clock. The selected clock > > - is the one fed to the prescaler to generate the Serial Clock (Sclock). > > - The PRESDIV field of CTRL(control register) controls a prescaler that > > - generates the Serial Clock (Sclock), whose period defines the > > - time quantum used to compose the CAN waveform. > > - > > -Can Engine Clock Source > > - There are two sources for CAN clock > > - - Platform Clock It represents the bus clock > > - - Oscillator Clock > > - > > - Peripheral Clock (PLL) > > - -------------- > > - | > > - --------- ------------- > > - | |CPI Clock | Prescaler | Sclock > > - | |---------------->| (1.. 256) |------------> > > - --------- ------------- > > - | | > > - -------------- ---------------------CLK_SRC > > - Oscillator Clock > > - > > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects > > - the peripheral clock. PLL clock is fed to the > > - prescaler to generate the Serial Clock (Sclock). > > - Valid values are "oscillator" and "platform" > > - "oscillator": CAN engine clock source is oscillator clock. > > - "platform" The CAN engine clock source is the bus clock > > - (platform clock). > > - > > -- fsl,flexcan-clock-divider : for the reference and system clock, an additional > > - clock divider can be specified. > > -- clock-frequency: frequency required to calculate the bitrate for FlexCAN. > > - > > -Note: > > - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC. > > - - P1010 does not have oscillator as the Clock Source.So the default > > - Clock Source is platform clock. > > -Examples: > > - > > - can0@1c000 { > > - compatible = "fsl,flexcan-v1.0"; > > - reg = <0x1c000 0x1000>; > > - interrupts = <48 0x2>; > > - interrupt-parent = <&mpic>; > > - fsl,flexcan-clock-source = "platform"; > > - fsl,flexcan-clock-divqider = <2>; > > - clock-frequency = <fixed by u-boot>; > > - }; > > Do we really want to drop the documentation for that binding. I think > something like the following text would be still useful: > > ------------------------ > Flexcan CAN contoller on Freescale's ARM and PowerPC processors > > Required properties: > > - compatible : Should be "fsl,flexcan" and optionally > "fsl,flexcan-<processor>" > - reg : Offset and length of the register set for this device > - interrupts : Interrupt tuple for this device > > Example: > > can@1c000 { > compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > reg = <0x1c000 0x1000>; > interrupts = <48 0x2>; > interrupt-parent = <&mpic>; > }; > ------------------------- Done, except the > compatible = "fsl,p1010-flexcan", "fsl,flexcan"; line is compatible = "fsl,flexcan", "fsl,flexcan-p1010"; > > What do you think? > > > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts > > index 6b33b73..d6a0bb2 100644 > > --- a/arch/powerpc/boot/dts/p1010rdb.dts > > +++ b/arch/powerpc/boot/dts/p1010rdb.dts > > @@ -169,14 +169,6 @@ > > }; > > }; > > > > - can0@1c000 { > > - fsl,flexcan-clock-source = "platform"; > > - }; > > - > > - can1@1d000 { > > - fsl,flexcan-clock-source = "platform"; > > - }; > > - > > usb@22000 { > > phy_type = "utmi"; > > }; > > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi > > index 7f51104..20c396d 100644 > > --- a/arch/powerpc/boot/dts/p1010si.dtsi > > +++ b/arch/powerpc/boot/dts/p1010si.dtsi > > @@ -141,19 +141,19 @@ > > }; > > > > can0@1c000 { > > - compatible = "fsl,flexcan-v1.0"; > > + compatible = "fsl,p1010-flexcan", > > + "fsl,flexcan"; > > Does fit on one line. > > > reg = <0x1c000 0x1000>; > > interrupts = <48 0x2>; > > interrupt-parent = <&mpic>; > > - fsl,flexcan-clock-divider = <2>; > > }; > > > > can1@1d000 { > > - compatible = "fsl,flexcan-v1.0"; > > + compatible = "fsl,p1010-flexcan", > > + "fsl,flexcan"; > > Ditto > > > reg = <0x1d000 0x1000>; > > interrupts = <61 0x2>; > > interrupt-parent = <&mpic>; > > - fsl,flexcan-clock-divider = <2>; > > }; > > > > L2: l2-cache-controller@20000 { > > Please also correct the node names (not using the number suffix). So the node names should be can@1c000 { can@1d000 { correct? Robin
> -----Original Message----- > From: Robin Holt [mailto:holt@sgi.com] > Sent: Wednesday, August 10, 2011 7:46 PM > To: Wolfgang Grandegger > Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421; > netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC > list > Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree > binding. > > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > > Hi Robin, > > > > On 08/10/2011 05:06 AM, Robin Holt wrote: > > > In working with the socketcan developers, we have come to the > > > conclusion the Documentation...fsl-flexcan.txt device tree > > > documentation needs to be cleaned up. The driver does not depend > > > upon any properties other > > > > Your first sentence could be misleading. Please just describe what the > > patch does and why, something like: > > > > "This patch cleans up the documentation of the device-tree binding for > > the Flexcan devices on Freescale's PowerPC and ARM cores. Extra > > properties are not needed as the frequency of the source clock is > > fixed..." and so on. > > I borrowed heavily from your message. ;) > > > > than the required properties so we are removing the file. > > > Additionally, the p1010*dts* files are not following the standard > > > for node naming in that they have a trailing -v1.0. > > > > > Signed-off-by: Robin Holt <holt@sgi.com> > > > To: Marc Kleine-Budde <mkl@pengutronix.de>, > > > To: Wolfgang Grandegger <wg@grandegger.com>, > > > To: U Bhaskar-B22300 <B22300@freescale.com> > > > To: Scott Wood <scottwood@freescale.com> > > > Cc: socketcan-core@lists.berlios.de, > > > Cc: netdev@vger.kernel.org, > > > Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > > > Cc: Kumar Gala <galak@kernel.crashing.org> > > > --- > > > .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 ---------- > ---------- > > > arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > > > arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > > > 3 files changed, 4 insertions(+), 73 deletions(-) delete mode > > > 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > > > diff --git > > > a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > deleted file mode 100644 > > > index 1a729f0..0000000 > > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > +++ /dev/null > > > @@ -1,61 +0,0 @@ > > > -CAN Device Tree Bindings > > > ------------------------- > > > -2011 Freescale Semiconductor, Inc. > > > - > > > -fsl,flexcan-v1.0 nodes > > > ------------------------ > > > -In addition to the required compatible-, reg- and > > > interrupt-properties, you can -also specify which clock source shall > be used for the controller. > > > - > > > -CPI Clock- Can Protocol Interface Clock > > > - This CLK_SRC bit of CTRL(control register) selects the clock source > to > > > - the CAN Protocol Interface(CPI) to be either the peripheral clock > > > - (driven by the PLL) or the crystal oscillator clock. The selected > clock > > > - is the one fed to the prescaler to generate the Serial Clock > (Sclock). > > > - The PRESDIV field of CTRL(control register) controls a prescaler > that > > > - generates the Serial Clock (Sclock), whose period defines the > > > - time quantum used to compose the CAN waveform. > > > - > > > -Can Engine Clock Source > > > - There are two sources for CAN clock > > > - - Platform Clock It represents the bus clock > > > - - Oscillator Clock > > > - > > > - Peripheral Clock (PLL) > > > - -------------- > > > - | > > > - --------- ------------- > > > - | |CPI Clock | Prescaler | Sclock > > > - | |---------------->| (1.. 256) |------------> > > > - --------- ------------- > > > - | | > > > - -------------- ---------------------CLK_SRC > > > - Oscillator Clock > > > - > > > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property > selects > > > - the peripheral clock. PLL clock is fed to the > > > - prescaler to generate the Serial Clock (Sclock). > > > - Valid values are "oscillator" and "platform" > > > - "oscillator": CAN engine clock source is > oscillator clock. > > > - "platform" The CAN engine clock source is the bus > clock > > > - (platform clock). > > > - > > > -- fsl,flexcan-clock-divider : for the reference and system clock, an > additional > > > - clock divider can be specified. > > > -- clock-frequency: frequency required to calculate the bitrate for > FlexCAN. > > > - > > > -Note: > > > - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 > SOC. > > > - - P1010 does not have oscillator as the Clock Source.So the default > > > - Clock Source is platform clock. > > > -Examples: > > > - > > > - can0@1c000 { > > > - compatible = "fsl,flexcan-v1.0"; > > > - reg = <0x1c000 0x1000>; > > > - interrupts = <48 0x2>; > > > - interrupt-parent = <&mpic>; > > > - fsl,flexcan-clock-source = "platform"; > > > - fsl,flexcan-clock-divqider = <2>; > > > - clock-frequency = <fixed by u-boot>; > > > - }; > > > > Do we really want to drop the documentation for that binding. I think > > something like the following text would be still useful: > > > > ------------------------ > > Flexcan CAN contoller on Freescale's ARM and PowerPC processors > > > > Required properties: > > > > - compatible : Should be "fsl,flexcan" and optionally > > "fsl,flexcan-<processor>" > > - reg : Offset and length of the register set for this device > > - interrupts : Interrupt tuple for this device > > > > Example: > > > > can@1c000 { > > compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > > reg = <0x1c000 0x1000>; > > interrupts = <48 0x2>; > > interrupt-parent = <&mpic>; > > }; > > ------------------------- > > Done, except the > > compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > > line is > compatible = "fsl,flexcan", "fsl,flexcan-p1010"; > > > > > What do you think? > > > > > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts > > > b/arch/powerpc/boot/dts/p1010rdb.dts > > > index 6b33b73..d6a0bb2 100644 > > > --- a/arch/powerpc/boot/dts/p1010rdb.dts > > > +++ b/arch/powerpc/boot/dts/p1010rdb.dts > > > @@ -169,14 +169,6 @@ > > > }; > > > }; > > > > > > - can0@1c000 { > > > - fsl,flexcan-clock-source = "platform"; > > > - }; > > > - > > > - can1@1d000 { > > > - fsl,flexcan-clock-source = "platform"; > > > - }; > > > - > > > usb@22000 { > > > phy_type = "utmi"; > > > }; > > > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi > > > b/arch/powerpc/boot/dts/p1010si.dtsi > > > index 7f51104..20c396d 100644 > > > --- a/arch/powerpc/boot/dts/p1010si.dtsi > > > +++ b/arch/powerpc/boot/dts/p1010si.dtsi > > > @@ -141,19 +141,19 @@ > > > }; > > > > > > can0@1c000 { > > > - compatible = "fsl,flexcan-v1.0"; > > > + compatible = "fsl,p1010-flexcan", > > > + "fsl,flexcan"; > > > > Does fit on one line. > > > > > reg = <0x1c000 0x1000>; > > > interrupts = <48 0x2>; > > > interrupt-parent = <&mpic>; > > > - fsl,flexcan-clock-divider = <2>; > > > }; > > > > > > can1@1d000 { > > > - compatible = "fsl,flexcan-v1.0"; > > > + compatible = "fsl,p1010-flexcan", > > > + "fsl,flexcan"; > > > > Ditto > > > > > reg = <0x1d000 0x1000>; > > > interrupts = <61 0x2>; > > > interrupt-parent = <&mpic>; > > > - fsl,flexcan-clock-divider = <2>; > > > }; > > > > > > L2: l2-cache-controller@20000 { > > > > Please also correct the node names (not using the number suffix). > > So the node names should be > can@1c000 { > can@1d000 { > correct? > [Bhaskar] As there are two CAN controllers on P1010,So won't it be better to distinguish it by can0 and can1 instead by simple "can" ? > Robin
On 08/10/2011 04:15 PM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: ... > Done, except the >> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > > line is > compatible = "fsl,flexcan", "fsl,flexcan-p1010"; IIRC, there order is more to less specific, e.g. for I2C: compatible = "fsl,mpc5200-i2c", "fsl-i2c" ... >> Please also correct the node names (not using the number suffix). > > So the node names should be > can@1c000 { > can@1d000 { > correct? Yes, just have a look how other node names are constructed, e.g. for sata, serial, rtc, etc.: $ grep serial@ *.dts ... pdm360ng.dts: serial@11000 { pdm360ng.dts: serial@11100 { pdm360ng.dts: serial@11200 { pdm360ng.dts: serial@11300 { pdm360ng.dts: serial@11400 { pdm360ng.dts: serial@11600 { pdm360ng.dts: serial@11800 { pdm360ng.dts: serial@11B00 { ... Wolfgang.
On Aug 10, 2011, at 9:15 AM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: >> Hi Robin, >> >> On 08/10/2011 05:06 AM, Robin Holt wrote: >>> In working with the socketcan developers, we have come to the conclusion >>> the Documentation...fsl-flexcan.txt device tree documentation needs to >>> be cleaned up. The driver does not depend upon any properties other >> >> Your first sentence could be misleading. Please just describe what the >> patch does and why, something like: >> >> "This patch cleans up the documentation of the device-tree binding for >> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra >> properties are not needed as the frequency of the source clock is >> fixed..." and so on. > > I borrowed heavily from your message. ;) > >>> than the required properties so we are removing the file. Additionally, >>> the p1010*dts* files are not following the standard for node naming in >>> that they have a trailing -v1.0. >> >>> Signed-off-by: Robin Holt <holt@sgi.com> >>> To: Marc Kleine-Budde <mkl@pengutronix.de>, >>> To: Wolfgang Grandegger <wg@grandegger.com>, >>> To: U Bhaskar-B22300 <B22300@freescale.com> >>> To: Scott Wood <scottwood@freescale.com> >>> Cc: socketcan-core@lists.berlios.de, >>> Cc: netdev@vger.kernel.org, >>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org> >>> Cc: Kumar Gala <galak@kernel.crashing.org> >>> --- >>> .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- >>> arch/powerpc/boot/dts/p1010rdb.dts | 8 --- >>> arch/powerpc/boot/dts/p1010si.dtsi | 8 +- >>> 3 files changed, 4 insertions(+), 73 deletions(-) >>> delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt I don't understand how we can do this? What binding spec covers the P1010 CAN support if you remove this? - k
On Aug 10, 2011, at 9:41 AM, Wolfgang Grandegger wrote: > On 08/10/2011 04:15 PM, Robin Holt wrote: >> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > ... >> Done, except the >>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; >> >> line is >> compatible = "fsl,flexcan", "fsl,flexcan-p1010"; > > IIRC, there order is more to less specific, e.g. for I2C: > > compatible = "fsl,mpc5200-i2c", "fsl-i2c" > > ... > >>> Please also correct the node names (not using the number suffix). >> >> So the node names should be >> can@1c000 { >> can@1d000 { >> correct? > > Yes, just have a look how other node names are constructed, e.g. for > sata, serial, rtc, etc.: > > $ grep serial@ *.dts > ... > pdm360ng.dts: serial@11000 { > pdm360ng.dts: serial@11100 { > pdm360ng.dts: serial@11200 { > pdm360ng.dts: serial@11300 { > pdm360ng.dts: serial@11400 { > pdm360ng.dts: serial@11600 { > pdm360ng.dts: serial@11800 { > pdm360ng.dts: serial@11B00 { > ... > > Wolfgang. Agree w/Wolfgang here, can@1c000, can@1d000 is what we should use. - k
On Wed, Aug 10, 2011 at 09:45:17AM -0500, Kumar Gala wrote: > > On Aug 10, 2011, at 9:15 AM, Robin Holt wrote: > > > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > >> Hi Robin, > >> > >> On 08/10/2011 05:06 AM, Robin Holt wrote: > >>> In working with the socketcan developers, we have come to the conclusion > >>> the Documentation...fsl-flexcan.txt device tree documentation needs to > >>> be cleaned up. The driver does not depend upon any properties other > >> > >> Your first sentence could be misleading. Please just describe what the > >> patch does and why, something like: > >> > >> "This patch cleans up the documentation of the device-tree binding for > >> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra > >> properties are not needed as the frequency of the source clock is > >> fixed..." and so on. > > > > I borrowed heavily from your message. ;) > > > >>> than the required properties so we are removing the file. Additionally, > >>> the p1010*dts* files are not following the standard for node naming in > >>> that they have a trailing -v1.0. > >> > >>> Signed-off-by: Robin Holt <holt@sgi.com> > >>> To: Marc Kleine-Budde <mkl@pengutronix.de>, > >>> To: Wolfgang Grandegger <wg@grandegger.com>, > >>> To: U Bhaskar-B22300 <B22300@freescale.com> > >>> To: Scott Wood <scottwood@freescale.com> > >>> Cc: socketcan-core@lists.berlios.de, > >>> Cc: netdev@vger.kernel.org, > >>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > >>> Cc: Kumar Gala <galak@kernel.crashing.org> > >>> --- > >>> .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- > >>> arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > >>> arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > >>> 3 files changed, 4 insertions(+), 73 deletions(-) > >>> delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > I don't understand how we can do this? What binding spec covers the P1010 CAN support if you remove this? We have added it back in based upon an earlier comment from Wolfgang. I will post a new version shortly. Robin
On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: > > > > -----Original Message----- > > From: Robin Holt [mailto:holt@sgi.com] > > Sent: Wednesday, August 10, 2011 7:46 PM > > To: Wolfgang Grandegger > > Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421; > > netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC > > list > > Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree > > binding. > > > > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > > > Hi Robin, > > > > > > On 08/10/2011 05:06 AM, Robin Holt wrote: > > > > In working with the socketcan developers, we have come to the > > > > conclusion the Documentation...fsl-flexcan.txt device tree > > > > documentation needs to be cleaned up. The driver does not depend > > > > upon any properties other > > > > > > Your first sentence could be misleading. Please just describe what the > > > patch does and why, something like: > > > > > > "This patch cleans up the documentation of the device-tree binding for > > > the Flexcan devices on Freescale's PowerPC and ARM cores. Extra > > > properties are not needed as the frequency of the source clock is > > > fixed..." and so on. > > > > I borrowed heavily from your message. ;) > > > > > > than the required properties so we are removing the file. > > > > Additionally, the p1010*dts* files are not following the standard > > > > for node naming in that they have a trailing -v1.0. > > > > > > > Signed-off-by: Robin Holt <holt@sgi.com> > > > > To: Marc Kleine-Budde <mkl@pengutronix.de>, > > > > To: Wolfgang Grandegger <wg@grandegger.com>, > > > > To: U Bhaskar-B22300 <B22300@freescale.com> > > > > To: Scott Wood <scottwood@freescale.com> > > > > Cc: socketcan-core@lists.berlios.de, > > > > Cc: netdev@vger.kernel.org, > > > > Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > > > > Cc: Kumar Gala <galak@kernel.crashing.org> > > > > --- > > > > .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 ---------- > > ---------- > > > > arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > > > > arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > > > > 3 files changed, 4 insertions(+), 73 deletions(-) delete mode > > > > 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > deleted file mode 100644 > > > > index 1a729f0..0000000 > > > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > +++ /dev/null > > > > @@ -1,61 +0,0 @@ > > > > -CAN Device Tree Bindings > > > > ------------------------- > > > > -2011 Freescale Semiconductor, Inc. > > > > - > > > > -fsl,flexcan-v1.0 nodes > > > > ------------------------ > > > > -In addition to the required compatible-, reg- and > > > > interrupt-properties, you can -also specify which clock source shall > > be used for the controller. > > > > - > > > > -CPI Clock- Can Protocol Interface Clock > > > > - This CLK_SRC bit of CTRL(control register) selects the clock source > > to > > > > - the CAN Protocol Interface(CPI) to be either the peripheral clock > > > > - (driven by the PLL) or the crystal oscillator clock. The selected > > clock > > > > - is the one fed to the prescaler to generate the Serial Clock > > (Sclock). > > > > - The PRESDIV field of CTRL(control register) controls a prescaler > > that > > > > - generates the Serial Clock (Sclock), whose period defines the > > > > - time quantum used to compose the CAN waveform. > > > > - > > > > -Can Engine Clock Source > > > > - There are two sources for CAN clock > > > > - - Platform Clock It represents the bus clock > > > > - - Oscillator Clock > > > > - > > > > - Peripheral Clock (PLL) > > > > - -------------- > > > > - | > > > > - --------- ------------- > > > > - | |CPI Clock | Prescaler | Sclock > > > > - | |---------------->| (1.. 256) |------------> > > > > - --------- ------------- > > > > - | | > > > > - -------------- ---------------------CLK_SRC > > > > - Oscillator Clock > > > > - > > > > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property > > selects > > > > - the peripheral clock. PLL clock is fed to the > > > > - prescaler to generate the Serial Clock (Sclock). > > > > - Valid values are "oscillator" and "platform" > > > > - "oscillator": CAN engine clock source is > > oscillator clock. > > > > - "platform" The CAN engine clock source is the bus > > clock > > > > - (platform clock). > > > > - > > > > -- fsl,flexcan-clock-divider : for the reference and system clock, an > > additional > > > > - clock divider can be specified. > > > > -- clock-frequency: frequency required to calculate the bitrate for > > FlexCAN. > > > > - > > > > -Note: > > > > - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 > > SOC. > > > > - - P1010 does not have oscillator as the Clock Source.So the default > > > > - Clock Source is platform clock. > > > > -Examples: > > > > - > > > > - can0@1c000 { > > > > - compatible = "fsl,flexcan-v1.0"; > > > > - reg = <0x1c000 0x1000>; > > > > - interrupts = <48 0x2>; > > > > - interrupt-parent = <&mpic>; > > > > - fsl,flexcan-clock-source = "platform"; > > > > - fsl,flexcan-clock-divqider = <2>; > > > > - clock-frequency = <fixed by u-boot>; > > > > - }; > > > > > > Do we really want to drop the documentation for that binding. I think > > > something like the following text would be still useful: > > > > > > ------------------------ > > > Flexcan CAN contoller on Freescale's ARM and PowerPC processors > > > > > > Required properties: > > > > > > - compatible : Should be "fsl,flexcan" and optionally > > > "fsl,flexcan-<processor>" > > > - reg : Offset and length of the register set for this device > > > - interrupts : Interrupt tuple for this device > > > > > > Example: > > > > > > can@1c000 { > > > compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > > > reg = <0x1c000 0x1000>; > > > interrupts = <48 0x2>; > > > interrupt-parent = <&mpic>; > > > }; > > > ------------------------- > > > > Done, except the > > > compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > > > > line is > > compatible = "fsl,flexcan", "fsl,flexcan-p1010"; > > > > > > > > What do you think? > > > > > > > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts > > > > b/arch/powerpc/boot/dts/p1010rdb.dts > > > > index 6b33b73..d6a0bb2 100644 > > > > --- a/arch/powerpc/boot/dts/p1010rdb.dts > > > > +++ b/arch/powerpc/boot/dts/p1010rdb.dts > > > > @@ -169,14 +169,6 @@ > > > > }; > > > > }; > > > > > > > > - can0@1c000 { > > > > - fsl,flexcan-clock-source = "platform"; > > > > - }; > > > > - > > > > - can1@1d000 { > > > > - fsl,flexcan-clock-source = "platform"; > > > > - }; > > > > - > > > > usb@22000 { > > > > phy_type = "utmi"; > > > > }; > > > > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi > > > > b/arch/powerpc/boot/dts/p1010si.dtsi > > > > index 7f51104..20c396d 100644 > > > > --- a/arch/powerpc/boot/dts/p1010si.dtsi > > > > +++ b/arch/powerpc/boot/dts/p1010si.dtsi > > > > @@ -141,19 +141,19 @@ > > > > }; > > > > > > > > can0@1c000 { > > > > - compatible = "fsl,flexcan-v1.0"; > > > > + compatible = "fsl,p1010-flexcan", > > > > + "fsl,flexcan"; > > > > > > Does fit on one line. > > > > > > > reg = <0x1c000 0x1000>; > > > > interrupts = <48 0x2>; > > > > interrupt-parent = <&mpic>; > > > > - fsl,flexcan-clock-divider = <2>; > > > > }; > > > > > > > > can1@1d000 { > > > > - compatible = "fsl,flexcan-v1.0"; > > > > + compatible = "fsl,p1010-flexcan", > > > > + "fsl,flexcan"; > > > > > > Ditto > > > > > > > reg = <0x1d000 0x1000>; > > > > interrupts = <61 0x2>; > > > > interrupt-parent = <&mpic>; > > > > - fsl,flexcan-clock-divider = <2>; > > > > }; > > > > > > > > L2: l2-cache-controller@20000 { > > > > > > Please also correct the node names (not using the number suffix). > > > > So the node names should be > > can@1c000 { > > can@1d000 { > > correct? > > > [Bhaskar] As there are two CAN controllers on P1010,So won't it be better > to distinguish it by can0 and can1 instead by simple "can" ? It looks like the way to do that is to assign a label to those devices and then associate the label with an alias. I have no idea how that works under the hood, but it is the way other files are set up. Take a look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial interfaces. Grant or Wolfgang, is that the right way to handle the concern about names or does it have no practical effect with the Linux kernel? Thanks, Robin
On Aug 10, 2011, at 11:00 AM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: >> >> >>> -----Original Message----- >>> From: Robin Holt [mailto:holt@sgi.com] >>> Sent: Wednesday, August 10, 2011 7:46 PM >>> To: Wolfgang Grandegger >>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421; >>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC >>> list >>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree >>> binding. >>> >>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: >>>> Hi Robin, >>>> >>>> On 08/10/2011 05:06 AM, Robin Holt wrote: >>>>> In working with the socketcan developers, we have come to the >>>>> conclusion the Documentation...fsl-flexcan.txt device tree >>>>> documentation needs to be cleaned up. The driver does not depend >>>>> upon any properties other >>>> >>>> Your first sentence could be misleading. Please just describe what the >>>> patch does and why, something like: >>>> >>>> "This patch cleans up the documentation of the device-tree binding for >>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra >>>> properties are not needed as the frequency of the source clock is >>>> fixed..." and so on. >>> >>> I borrowed heavily from your message. ;) >>> >>>>> than the required properties so we are removing the file. >>>>> Additionally, the p1010*dts* files are not following the standard >>>>> for node naming in that they have a trailing -v1.0. >>>> >>>>> Signed-off-by: Robin Holt <holt@sgi.com> >>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>, >>>>> To: Wolfgang Grandegger <wg@grandegger.com>, >>>>> To: U Bhaskar-B22300 <B22300@freescale.com> >>>>> To: Scott Wood <scottwood@freescale.com> >>>>> Cc: socketcan-core@lists.berlios.de, >>>>> Cc: netdev@vger.kernel.org, >>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org> >>>>> Cc: Kumar Gala <galak@kernel.crashing.org> >>>>> --- >>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 ---------- >>> ---------- >>>>> arch/powerpc/boot/dts/p1010rdb.dts | 8 --- >>>>> arch/powerpc/boot/dts/p1010si.dtsi | 8 +- >>>>> 3 files changed, 4 insertions(+), 73 deletions(-) delete mode >>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>> deleted file mode 100644 >>>>> index 1a729f0..0000000 >>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>> +++ /dev/null >>>>> @@ -1,61 +0,0 @@ >>>>> -CAN Device Tree Bindings >>>>> ------------------------- >>>>> -2011 Freescale Semiconductor, Inc. >>>>> - >>>>> -fsl,flexcan-v1.0 nodes >>>>> ------------------------ >>>>> -In addition to the required compatible-, reg- and >>>>> interrupt-properties, you can -also specify which clock source shall >>> be used for the controller. >>>>> - >>>>> -CPI Clock- Can Protocol Interface Clock >>>>> - This CLK_SRC bit of CTRL(control register) selects the clock source >>> to >>>>> - the CAN Protocol Interface(CPI) to be either the peripheral clock >>>>> - (driven by the PLL) or the crystal oscillator clock. The selected >>> clock >>>>> - is the one fed to the prescaler to generate the Serial Clock >>> (Sclock). >>>>> - The PRESDIV field of CTRL(control register) controls a prescaler >>> that >>>>> - generates the Serial Clock (Sclock), whose period defines the >>>>> - time quantum used to compose the CAN waveform. >>>>> - >>>>> -Can Engine Clock Source >>>>> - There are two sources for CAN clock >>>>> - - Platform Clock It represents the bus clock >>>>> - - Oscillator Clock >>>>> - >>>>> - Peripheral Clock (PLL) >>>>> - -------------- >>>>> - | >>>>> - --------- ------------- >>>>> - | |CPI Clock | Prescaler | Sclock >>>>> - | |---------------->| (1.. 256) |------------> >>>>> - --------- ------------- >>>>> - | | >>>>> - -------------- ---------------------CLK_SRC >>>>> - Oscillator Clock >>>>> - >>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property >>> selects >>>>> - the peripheral clock. PLL clock is fed to the >>>>> - prescaler to generate the Serial Clock (Sclock). >>>>> - Valid values are "oscillator" and "platform" >>>>> - "oscillator": CAN engine clock source is >>> oscillator clock. >>>>> - "platform" The CAN engine clock source is the bus >>> clock >>>>> - (platform clock). >>>>> - >>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, an >>> additional >>>>> - clock divider can be specified. >>>>> -- clock-frequency: frequency required to calculate the bitrate for >>> FlexCAN. >>>>> - >>>>> -Note: >>>>> - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 >>> SOC. >>>>> - - P1010 does not have oscillator as the Clock Source.So the default >>>>> - Clock Source is platform clock. >>>>> -Examples: >>>>> - >>>>> - can0@1c000 { >>>>> - compatible = "fsl,flexcan-v1.0"; >>>>> - reg = <0x1c000 0x1000>; >>>>> - interrupts = <48 0x2>; >>>>> - interrupt-parent = <&mpic>; >>>>> - fsl,flexcan-clock-source = "platform"; >>>>> - fsl,flexcan-clock-divqider = <2>; >>>>> - clock-frequency = <fixed by u-boot>; >>>>> - }; >>>> >>>> Do we really want to drop the documentation for that binding. I think >>>> something like the following text would be still useful: >>>> >>>> ------------------------ >>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors >>>> >>>> Required properties: >>>> >>>> - compatible : Should be "fsl,flexcan" and optionally >>>> "fsl,flexcan-<processor>" >>>> - reg : Offset and length of the register set for this device >>>> - interrupts : Interrupt tuple for this device >>>> >>>> Example: >>>> >>>> can@1c000 { >>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; >>>> reg = <0x1c000 0x1000>; >>>> interrupts = <48 0x2>; >>>> interrupt-parent = <&mpic>; >>>> }; >>>> ------------------------- >>> >>> Done, except the >>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; >>> >>> line is >>> compatible = "fsl,flexcan", "fsl,flexcan-p1010"; >>> >>>> >>>> What do you think? >>>> >>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts >>>>> b/arch/powerpc/boot/dts/p1010rdb.dts >>>>> index 6b33b73..d6a0bb2 100644 >>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts >>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts >>>>> @@ -169,14 +169,6 @@ >>>>> }; >>>>> }; >>>>> >>>>> - can0@1c000 { >>>>> - fsl,flexcan-clock-source = "platform"; >>>>> - }; >>>>> - >>>>> - can1@1d000 { >>>>> - fsl,flexcan-clock-source = "platform"; >>>>> - }; >>>>> - >>>>> usb@22000 { >>>>> phy_type = "utmi"; >>>>> }; >>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi >>>>> b/arch/powerpc/boot/dts/p1010si.dtsi >>>>> index 7f51104..20c396d 100644 >>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi >>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi >>>>> @@ -141,19 +141,19 @@ >>>>> }; >>>>> >>>>> can0@1c000 { >>>>> - compatible = "fsl,flexcan-v1.0"; >>>>> + compatible = "fsl,p1010-flexcan", >>>>> + "fsl,flexcan"; >>>> >>>> Does fit on one line. >>>> >>>>> reg = <0x1c000 0x1000>; >>>>> interrupts = <48 0x2>; >>>>> interrupt-parent = <&mpic>; >>>>> - fsl,flexcan-clock-divider = <2>; >>>>> }; >>>>> >>>>> can1@1d000 { >>>>> - compatible = "fsl,flexcan-v1.0"; >>>>> + compatible = "fsl,p1010-flexcan", >>>>> + "fsl,flexcan"; >>>> >>>> Ditto >>>> >>>>> reg = <0x1d000 0x1000>; >>>>> interrupts = <61 0x2>; >>>>> interrupt-parent = <&mpic>; >>>>> - fsl,flexcan-clock-divider = <2>; >>>>> }; >>>>> >>>>> L2: l2-cache-controller@20000 { >>>> >>>> Please also correct the node names (not using the number suffix). >>> >>> So the node names should be >>> can@1c000 { >>> can@1d000 { >>> correct? >>> >> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better >> to distinguish it by can0 and can1 instead by simple "can" ? > > It looks like the way to do that is to assign a label to those devices > and then associate the label with an alias. I have no idea how that > works under the hood, but it is the way other files are set up. Take a > look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial > interfaces. > > Grant or Wolfgang, is that the right way to handle the concern about > names or does it have no practical effect with the Linux kernel? It has not effect. The label is just if you need to reference it via some other means. - k
On Wed, Aug 10, 2011 at 11:53:15AM -0500, Kumar Gala wrote: > > On Aug 10, 2011, at 11:00 AM, Robin Holt wrote: > > > On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: > >> > >> > >>> -----Original Message----- > >>> From: Robin Holt [mailto:holt@sgi.com] > >>> Sent: Wednesday, August 10, 2011 7:46 PM > >>> To: Wolfgang Grandegger > >>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421; > >>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC > >>> list > >>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree > >>> binding. > >>> > >>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > >>>> Hi Robin, > >>>> > >>>> On 08/10/2011 05:06 AM, Robin Holt wrote: > >>>>> In working with the socketcan developers, we have come to the > >>>>> conclusion the Documentation...fsl-flexcan.txt device tree > >>>>> documentation needs to be cleaned up. The driver does not depend > >>>>> upon any properties other > >>>> > >>>> Your first sentence could be misleading. Please just describe what the > >>>> patch does and why, something like: > >>>> > >>>> "This patch cleans up the documentation of the device-tree binding for > >>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra > >>>> properties are not needed as the frequency of the source clock is > >>>> fixed..." and so on. > >>> > >>> I borrowed heavily from your message. ;) > >>> > >>>>> than the required properties so we are removing the file. > >>>>> Additionally, the p1010*dts* files are not following the standard > >>>>> for node naming in that they have a trailing -v1.0. > >>>> > >>>>> Signed-off-by: Robin Holt <holt@sgi.com> > >>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>, > >>>>> To: Wolfgang Grandegger <wg@grandegger.com>, > >>>>> To: U Bhaskar-B22300 <B22300@freescale.com> > >>>>> To: Scott Wood <scottwood@freescale.com> > >>>>> Cc: socketcan-core@lists.berlios.de, > >>>>> Cc: netdev@vger.kernel.org, > >>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > >>>>> Cc: Kumar Gala <galak@kernel.crashing.org> > >>>>> --- > >>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 ---------- > >>> ---------- > >>>>> arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > >>>>> arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > >>>>> 3 files changed, 4 insertions(+), 73 deletions(-) delete mode > >>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>> > >>>>> diff --git > >>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>> deleted file mode 100644 > >>>>> index 1a729f0..0000000 > >>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>> +++ /dev/null > >>>>> @@ -1,61 +0,0 @@ > >>>>> -CAN Device Tree Bindings > >>>>> ------------------------- > >>>>> -2011 Freescale Semiconductor, Inc. > >>>>> - > >>>>> -fsl,flexcan-v1.0 nodes > >>>>> ------------------------ > >>>>> -In addition to the required compatible-, reg- and > >>>>> interrupt-properties, you can -also specify which clock source shall > >>> be used for the controller. > >>>>> - > >>>>> -CPI Clock- Can Protocol Interface Clock > >>>>> - This CLK_SRC bit of CTRL(control register) selects the clock source > >>> to > >>>>> - the CAN Protocol Interface(CPI) to be either the peripheral clock > >>>>> - (driven by the PLL) or the crystal oscillator clock. The selected > >>> clock > >>>>> - is the one fed to the prescaler to generate the Serial Clock > >>> (Sclock). > >>>>> - The PRESDIV field of CTRL(control register) controls a prescaler > >>> that > >>>>> - generates the Serial Clock (Sclock), whose period defines the > >>>>> - time quantum used to compose the CAN waveform. > >>>>> - > >>>>> -Can Engine Clock Source > >>>>> - There are two sources for CAN clock > >>>>> - - Platform Clock It represents the bus clock > >>>>> - - Oscillator Clock > >>>>> - > >>>>> - Peripheral Clock (PLL) > >>>>> - -------------- > >>>>> - | > >>>>> - --------- ------------- > >>>>> - | |CPI Clock | Prescaler | Sclock > >>>>> - | |---------------->| (1.. 256) |------------> > >>>>> - --------- ------------- > >>>>> - | | > >>>>> - -------------- ---------------------CLK_SRC > >>>>> - Oscillator Clock > >>>>> - > >>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property > >>> selects > >>>>> - the peripheral clock. PLL clock is fed to the > >>>>> - prescaler to generate the Serial Clock (Sclock). > >>>>> - Valid values are "oscillator" and "platform" > >>>>> - "oscillator": CAN engine clock source is > >>> oscillator clock. > >>>>> - "platform" The CAN engine clock source is the bus > >>> clock > >>>>> - (platform clock). > >>>>> - > >>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, an > >>> additional > >>>>> - clock divider can be specified. > >>>>> -- clock-frequency: frequency required to calculate the bitrate for > >>> FlexCAN. > >>>>> - > >>>>> -Note: > >>>>> - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 > >>> SOC. > >>>>> - - P1010 does not have oscillator as the Clock Source.So the default > >>>>> - Clock Source is platform clock. > >>>>> -Examples: > >>>>> - > >>>>> - can0@1c000 { > >>>>> - compatible = "fsl,flexcan-v1.0"; > >>>>> - reg = <0x1c000 0x1000>; > >>>>> - interrupts = <48 0x2>; > >>>>> - interrupt-parent = <&mpic>; > >>>>> - fsl,flexcan-clock-source = "platform"; > >>>>> - fsl,flexcan-clock-divqider = <2>; > >>>>> - clock-frequency = <fixed by u-boot>; > >>>>> - }; > >>>> > >>>> Do we really want to drop the documentation for that binding. I think > >>>> something like the following text would be still useful: > >>>> > >>>> ------------------------ > >>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors > >>>> > >>>> Required properties: > >>>> > >>>> - compatible : Should be "fsl,flexcan" and optionally > >>>> "fsl,flexcan-<processor>" > >>>> - reg : Offset and length of the register set for this device > >>>> - interrupts : Interrupt tuple for this device > >>>> > >>>> Example: > >>>> > >>>> can@1c000 { > >>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > >>>> reg = <0x1c000 0x1000>; > >>>> interrupts = <48 0x2>; > >>>> interrupt-parent = <&mpic>; > >>>> }; > >>>> ------------------------- > >>> > >>> Done, except the > >>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > >>> > >>> line is > >>> compatible = "fsl,flexcan", "fsl,flexcan-p1010"; > >>> > >>>> > >>>> What do you think? > >>>> > >>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts > >>>>> b/arch/powerpc/boot/dts/p1010rdb.dts > >>>>> index 6b33b73..d6a0bb2 100644 > >>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts > >>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts > >>>>> @@ -169,14 +169,6 @@ > >>>>> }; > >>>>> }; > >>>>> > >>>>> - can0@1c000 { > >>>>> - fsl,flexcan-clock-source = "platform"; > >>>>> - }; > >>>>> - > >>>>> - can1@1d000 { > >>>>> - fsl,flexcan-clock-source = "platform"; > >>>>> - }; > >>>>> - > >>>>> usb@22000 { > >>>>> phy_type = "utmi"; > >>>>> }; > >>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi > >>>>> b/arch/powerpc/boot/dts/p1010si.dtsi > >>>>> index 7f51104..20c396d 100644 > >>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi > >>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi > >>>>> @@ -141,19 +141,19 @@ > >>>>> }; > >>>>> > >>>>> can0@1c000 { > >>>>> - compatible = "fsl,flexcan-v1.0"; > >>>>> + compatible = "fsl,p1010-flexcan", > >>>>> + "fsl,flexcan"; > >>>> > >>>> Does fit on one line. > >>>> > >>>>> reg = <0x1c000 0x1000>; > >>>>> interrupts = <48 0x2>; > >>>>> interrupt-parent = <&mpic>; > >>>>> - fsl,flexcan-clock-divider = <2>; > >>>>> }; > >>>>> > >>>>> can1@1d000 { > >>>>> - compatible = "fsl,flexcan-v1.0"; > >>>>> + compatible = "fsl,p1010-flexcan", > >>>>> + "fsl,flexcan"; > >>>> > >>>> Ditto > >>>> > >>>>> reg = <0x1d000 0x1000>; > >>>>> interrupts = <61 0x2>; > >>>>> interrupt-parent = <&mpic>; > >>>>> - fsl,flexcan-clock-divider = <2>; > >>>>> }; > >>>>> > >>>>> L2: l2-cache-controller@20000 { > >>>> > >>>> Please also correct the node names (not using the number suffix). > >>> > >>> So the node names should be > >>> can@1c000 { > >>> can@1d000 { > >>> correct? > >>> > >> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better > >> to distinguish it by can0 and can1 instead by simple "can" ? > > > > It looks like the way to do that is to assign a label to those devices > > and then associate the label with an alias. I have no idea how that > > works under the hood, but it is the way other files are set up. Take a > > look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial > > interfaces. > > > > Grant or Wolfgang, is that the right way to handle the concern about > > names or does it have no practical effect with the Linux kernel? > > It has not effect. The label is just if you need to reference it via some other means. Does the alias have an effect? Robin
On Aug 10, 2011, at 12:16 PM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 11:53:15AM -0500, Kumar Gala wrote: >> >> On Aug 10, 2011, at 11:00 AM, Robin Holt wrote: >> >>> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: Robin Holt [mailto:holt@sgi.com] >>>>> Sent: Wednesday, August 10, 2011 7:46 PM >>>>> To: Wolfgang Grandegger >>>>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421; >>>>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC >>>>> list >>>>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree >>>>> binding. >>>>> >>>>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: >>>>>> Hi Robin, >>>>>> >>>>>> On 08/10/2011 05:06 AM, Robin Holt wrote: >>>>>>> In working with the socketcan developers, we have come to the >>>>>>> conclusion the Documentation...fsl-flexcan.txt device tree >>>>>>> documentation needs to be cleaned up. The driver does not depend >>>>>>> upon any properties other >>>>>> >>>>>> Your first sentence could be misleading. Please just describe what the >>>>>> patch does and why, something like: >>>>>> >>>>>> "This patch cleans up the documentation of the device-tree binding for >>>>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra >>>>>> properties are not needed as the frequency of the source clock is >>>>>> fixed..." and so on. >>>>> >>>>> I borrowed heavily from your message. ;) >>>>> >>>>>>> than the required properties so we are removing the file. >>>>>>> Additionally, the p1010*dts* files are not following the standard >>>>>>> for node naming in that they have a trailing -v1.0. >>>>>> >>>>>>> Signed-off-by: Robin Holt <holt@sgi.com> >>>>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>, >>>>>>> To: Wolfgang Grandegger <wg@grandegger.com>, >>>>>>> To: U Bhaskar-B22300 <B22300@freescale.com> >>>>>>> To: Scott Wood <scottwood@freescale.com> >>>>>>> Cc: socketcan-core@lists.berlios.de, >>>>>>> Cc: netdev@vger.kernel.org, >>>>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org> >>>>>>> Cc: Kumar Gala <galak@kernel.crashing.org> >>>>>>> --- >>>>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 ---------- >>>>> ---------- >>>>>>> arch/powerpc/boot/dts/p1010rdb.dts | 8 --- >>>>>>> arch/powerpc/boot/dts/p1010si.dtsi | 8 +- >>>>>>> 3 files changed, 4 insertions(+), 73 deletions(-) delete mode >>>>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>>>> deleted file mode 100644 >>>>>>> index 1a729f0..0000000 >>>>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt >>>>>>> +++ /dev/null >>>>>>> @@ -1,61 +0,0 @@ >>>>>>> -CAN Device Tree Bindings >>>>>>> ------------------------- >>>>>>> -2011 Freescale Semiconductor, Inc. >>>>>>> - >>>>>>> -fsl,flexcan-v1.0 nodes >>>>>>> ------------------------ >>>>>>> -In addition to the required compatible-, reg- and >>>>>>> interrupt-properties, you can -also specify which clock source shall >>>>> be used for the controller. >>>>>>> - >>>>>>> -CPI Clock- Can Protocol Interface Clock >>>>>>> - This CLK_SRC bit of CTRL(control register) selects the clock source >>>>> to >>>>>>> - the CAN Protocol Interface(CPI) to be either the peripheral clock >>>>>>> - (driven by the PLL) or the crystal oscillator clock. The selected >>>>> clock >>>>>>> - is the one fed to the prescaler to generate the Serial Clock >>>>> (Sclock). >>>>>>> - The PRESDIV field of CTRL(control register) controls a prescaler >>>>> that >>>>>>> - generates the Serial Clock (Sclock), whose period defines the >>>>>>> - time quantum used to compose the CAN waveform. >>>>>>> - >>>>>>> -Can Engine Clock Source >>>>>>> - There are two sources for CAN clock >>>>>>> - - Platform Clock It represents the bus clock >>>>>>> - - Oscillator Clock >>>>>>> - >>>>>>> - Peripheral Clock (PLL) >>>>>>> - -------------- >>>>>>> - | >>>>>>> - --------- ------------- >>>>>>> - | |CPI Clock | Prescaler | Sclock >>>>>>> - | |---------------->| (1.. 256) |------------> >>>>>>> - --------- ------------- >>>>>>> - | | >>>>>>> - -------------- ---------------------CLK_SRC >>>>>>> - Oscillator Clock >>>>>>> - >>>>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property >>>>> selects >>>>>>> - the peripheral clock. PLL clock is fed to the >>>>>>> - prescaler to generate the Serial Clock (Sclock). >>>>>>> - Valid values are "oscillator" and "platform" >>>>>>> - "oscillator": CAN engine clock source is >>>>> oscillator clock. >>>>>>> - "platform" The CAN engine clock source is the bus >>>>> clock >>>>>>> - (platform clock). >>>>>>> - >>>>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, an >>>>> additional >>>>>>> - clock divider can be specified. >>>>>>> -- clock-frequency: frequency required to calculate the bitrate for >>>>> FlexCAN. >>>>>>> - >>>>>>> -Note: >>>>>>> - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 >>>>> SOC. >>>>>>> - - P1010 does not have oscillator as the Clock Source.So the default >>>>>>> - Clock Source is platform clock. >>>>>>> -Examples: >>>>>>> - >>>>>>> - can0@1c000 { >>>>>>> - compatible = "fsl,flexcan-v1.0"; >>>>>>> - reg = <0x1c000 0x1000>; >>>>>>> - interrupts = <48 0x2>; >>>>>>> - interrupt-parent = <&mpic>; >>>>>>> - fsl,flexcan-clock-source = "platform"; >>>>>>> - fsl,flexcan-clock-divqider = <2>; >>>>>>> - clock-frequency = <fixed by u-boot>; >>>>>>> - }; >>>>>> >>>>>> Do we really want to drop the documentation for that binding. I think >>>>>> something like the following text would be still useful: >>>>>> >>>>>> ------------------------ >>>>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors >>>>>> >>>>>> Required properties: >>>>>> >>>>>> - compatible : Should be "fsl,flexcan" and optionally >>>>>> "fsl,flexcan-<processor>" >>>>>> - reg : Offset and length of the register set for this device >>>>>> - interrupts : Interrupt tuple for this device >>>>>> >>>>>> Example: >>>>>> >>>>>> can@1c000 { >>>>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; >>>>>> reg = <0x1c000 0x1000>; >>>>>> interrupts = <48 0x2>; >>>>>> interrupt-parent = <&mpic>; >>>>>> }; >>>>>> ------------------------- >>>>> >>>>> Done, except the >>>>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; >>>>> >>>>> line is >>>>> compatible = "fsl,flexcan", "fsl,flexcan-p1010"; >>>>> >>>>>> >>>>>> What do you think? >>>>>> >>>>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts >>>>>>> b/arch/powerpc/boot/dts/p1010rdb.dts >>>>>>> index 6b33b73..d6a0bb2 100644 >>>>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts >>>>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts >>>>>>> @@ -169,14 +169,6 @@ >>>>>>> }; >>>>>>> }; >>>>>>> >>>>>>> - can0@1c000 { >>>>>>> - fsl,flexcan-clock-source = "platform"; >>>>>>> - }; >>>>>>> - >>>>>>> - can1@1d000 { >>>>>>> - fsl,flexcan-clock-source = "platform"; >>>>>>> - }; >>>>>>> - >>>>>>> usb@22000 { >>>>>>> phy_type = "utmi"; >>>>>>> }; >>>>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi >>>>>>> b/arch/powerpc/boot/dts/p1010si.dtsi >>>>>>> index 7f51104..20c396d 100644 >>>>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi >>>>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi >>>>>>> @@ -141,19 +141,19 @@ >>>>>>> }; >>>>>>> >>>>>>> can0@1c000 { >>>>>>> - compatible = "fsl,flexcan-v1.0"; >>>>>>> + compatible = "fsl,p1010-flexcan", >>>>>>> + "fsl,flexcan"; >>>>>> >>>>>> Does fit on one line. >>>>>> >>>>>>> reg = <0x1c000 0x1000>; >>>>>>> interrupts = <48 0x2>; >>>>>>> interrupt-parent = <&mpic>; >>>>>>> - fsl,flexcan-clock-divider = <2>; >>>>>>> }; >>>>>>> >>>>>>> can1@1d000 { >>>>>>> - compatible = "fsl,flexcan-v1.0"; >>>>>>> + compatible = "fsl,p1010-flexcan", >>>>>>> + "fsl,flexcan"; >>>>>> >>>>>> Ditto >>>>>> >>>>>>> reg = <0x1d000 0x1000>; >>>>>>> interrupts = <61 0x2>; >>>>>>> interrupt-parent = <&mpic>; >>>>>>> - fsl,flexcan-clock-divider = <2>; >>>>>>> }; >>>>>>> >>>>>>> L2: l2-cache-controller@20000 { >>>>>> >>>>>> Please also correct the node names (not using the number suffix). >>>>> >>>>> So the node names should be >>>>> can@1c000 { >>>>> can@1d000 { >>>>> correct? >>>>> >>>> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better >>>> to distinguish it by can0 and can1 instead by simple "can" ? >>> >>> It looks like the way to do that is to assign a label to those devices >>> and then associate the label with an alias. I have no idea how that >>> works under the hood, but it is the way other files are set up. Take a >>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial >>> interfaces. >>> >>> Grant or Wolfgang, is that the right way to handle the concern about >>> names or does it have no practical effect with the Linux kernel? >> >> It has not effect. The label is just if you need to reference it via some other means. > > Does the alias have an effect? nope - k
On Wed, Aug 10, 2011 at 12:17:45PM -0500, Kumar Gala wrote: > > On Aug 10, 2011, at 12:16 PM, Robin Holt wrote: > > > On Wed, Aug 10, 2011 at 11:53:15AM -0500, Kumar Gala wrote: > >> > >> On Aug 10, 2011, at 11:00 AM, Robin Holt wrote: > >> > >>> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: > >>>> > >>>> > >>>>> -----Original Message----- > >>>>> From: Robin Holt [mailto:holt@sgi.com] > >>>>> Sent: Wednesday, August 10, 2011 7:46 PM > >>>>> To: Wolfgang Grandegger > >>>>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421; > >>>>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC > >>>>> list > >>>>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree > >>>>> binding. > >>>>> > >>>>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote: > >>>>>> Hi Robin, > >>>>>> > >>>>>> On 08/10/2011 05:06 AM, Robin Holt wrote: > >>>>>>> In working with the socketcan developers, we have come to the > >>>>>>> conclusion the Documentation...fsl-flexcan.txt device tree > >>>>>>> documentation needs to be cleaned up. The driver does not depend > >>>>>>> upon any properties other > >>>>>> > >>>>>> Your first sentence could be misleading. Please just describe what the > >>>>>> patch does and why, something like: > >>>>>> > >>>>>> "This patch cleans up the documentation of the device-tree binding for > >>>>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra > >>>>>> properties are not needed as the frequency of the source clock is > >>>>>> fixed..." and so on. > >>>>> > >>>>> I borrowed heavily from your message. ;) > >>>>> > >>>>>>> than the required properties so we are removing the file. > >>>>>>> Additionally, the p1010*dts* files are not following the standard > >>>>>>> for node naming in that they have a trailing -v1.0. > >>>>>> > >>>>>>> Signed-off-by: Robin Holt <holt@sgi.com> > >>>>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>, > >>>>>>> To: Wolfgang Grandegger <wg@grandegger.com>, > >>>>>>> To: U Bhaskar-B22300 <B22300@freescale.com> > >>>>>>> To: Scott Wood <scottwood@freescale.com> > >>>>>>> Cc: socketcan-core@lists.berlios.de, > >>>>>>> Cc: netdev@vger.kernel.org, > >>>>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org> > >>>>>>> Cc: Kumar Gala <galak@kernel.crashing.org> > >>>>>>> --- > >>>>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 ---------- > >>>>> ---------- > >>>>>>> arch/powerpc/boot/dts/p1010rdb.dts | 8 --- > >>>>>>> arch/powerpc/boot/dts/p1010si.dtsi | 8 +- > >>>>>>> 3 files changed, 4 insertions(+), 73 deletions(-) delete mode > >>>>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>>>> > >>>>>>> diff --git > >>>>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>>>> deleted file mode 100644 > >>>>>>> index 1a729f0..0000000 > >>>>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > >>>>>>> +++ /dev/null > >>>>>>> @@ -1,61 +0,0 @@ > >>>>>>> -CAN Device Tree Bindings > >>>>>>> ------------------------- > >>>>>>> -2011 Freescale Semiconductor, Inc. > >>>>>>> - > >>>>>>> -fsl,flexcan-v1.0 nodes > >>>>>>> ------------------------ > >>>>>>> -In addition to the required compatible-, reg- and > >>>>>>> interrupt-properties, you can -also specify which clock source shall > >>>>> be used for the controller. > >>>>>>> - > >>>>>>> -CPI Clock- Can Protocol Interface Clock > >>>>>>> - This CLK_SRC bit of CTRL(control register) selects the clock source > >>>>> to > >>>>>>> - the CAN Protocol Interface(CPI) to be either the peripheral clock > >>>>>>> - (driven by the PLL) or the crystal oscillator clock. The selected > >>>>> clock > >>>>>>> - is the one fed to the prescaler to generate the Serial Clock > >>>>> (Sclock). > >>>>>>> - The PRESDIV field of CTRL(control register) controls a prescaler > >>>>> that > >>>>>>> - generates the Serial Clock (Sclock), whose period defines the > >>>>>>> - time quantum used to compose the CAN waveform. > >>>>>>> - > >>>>>>> -Can Engine Clock Source > >>>>>>> - There are two sources for CAN clock > >>>>>>> - - Platform Clock It represents the bus clock > >>>>>>> - - Oscillator Clock > >>>>>>> - > >>>>>>> - Peripheral Clock (PLL) > >>>>>>> - -------------- > >>>>>>> - | > >>>>>>> - --------- ------------- > >>>>>>> - | |CPI Clock | Prescaler | Sclock > >>>>>>> - | |---------------->| (1.. 256) |------------> > >>>>>>> - --------- ------------- > >>>>>>> - | | > >>>>>>> - -------------- ---------------------CLK_SRC > >>>>>>> - Oscillator Clock > >>>>>>> - > >>>>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property > >>>>> selects > >>>>>>> - the peripheral clock. PLL clock is fed to the > >>>>>>> - prescaler to generate the Serial Clock (Sclock). > >>>>>>> - Valid values are "oscillator" and "platform" > >>>>>>> - "oscillator": CAN engine clock source is > >>>>> oscillator clock. > >>>>>>> - "platform" The CAN engine clock source is the bus > >>>>> clock > >>>>>>> - (platform clock). > >>>>>>> - > >>>>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, an > >>>>> additional > >>>>>>> - clock divider can be specified. > >>>>>>> -- clock-frequency: frequency required to calculate the bitrate for > >>>>> FlexCAN. > >>>>>>> - > >>>>>>> -Note: > >>>>>>> - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 > >>>>> SOC. > >>>>>>> - - P1010 does not have oscillator as the Clock Source.So the default > >>>>>>> - Clock Source is platform clock. > >>>>>>> -Examples: > >>>>>>> - > >>>>>>> - can0@1c000 { > >>>>>>> - compatible = "fsl,flexcan-v1.0"; > >>>>>>> - reg = <0x1c000 0x1000>; > >>>>>>> - interrupts = <48 0x2>; > >>>>>>> - interrupt-parent = <&mpic>; > >>>>>>> - fsl,flexcan-clock-source = "platform"; > >>>>>>> - fsl,flexcan-clock-divqider = <2>; > >>>>>>> - clock-frequency = <fixed by u-boot>; > >>>>>>> - }; > >>>>>> > >>>>>> Do we really want to drop the documentation for that binding. I think > >>>>>> something like the following text would be still useful: > >>>>>> > >>>>>> ------------------------ > >>>>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors > >>>>>> > >>>>>> Required properties: > >>>>>> > >>>>>> - compatible : Should be "fsl,flexcan" and optionally > >>>>>> "fsl,flexcan-<processor>" > >>>>>> - reg : Offset and length of the register set for this device > >>>>>> - interrupts : Interrupt tuple for this device > >>>>>> > >>>>>> Example: > >>>>>> > >>>>>> can@1c000 { > >>>>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > >>>>>> reg = <0x1c000 0x1000>; > >>>>>> interrupts = <48 0x2>; > >>>>>> interrupt-parent = <&mpic>; > >>>>>> }; > >>>>>> ------------------------- > >>>>> > >>>>> Done, except the > >>>>>> compatible = "fsl,p1010-flexcan", "fsl,flexcan"; > >>>>> > >>>>> line is > >>>>> compatible = "fsl,flexcan", "fsl,flexcan-p1010"; > >>>>> > >>>>>> > >>>>>> What do you think? > >>>>>> > >>>>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts > >>>>>>> b/arch/powerpc/boot/dts/p1010rdb.dts > >>>>>>> index 6b33b73..d6a0bb2 100644 > >>>>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts > >>>>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts > >>>>>>> @@ -169,14 +169,6 @@ > >>>>>>> }; > >>>>>>> }; > >>>>>>> > >>>>>>> - can0@1c000 { > >>>>>>> - fsl,flexcan-clock-source = "platform"; > >>>>>>> - }; > >>>>>>> - > >>>>>>> - can1@1d000 { > >>>>>>> - fsl,flexcan-clock-source = "platform"; > >>>>>>> - }; > >>>>>>> - > >>>>>>> usb@22000 { > >>>>>>> phy_type = "utmi"; > >>>>>>> }; > >>>>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi > >>>>>>> b/arch/powerpc/boot/dts/p1010si.dtsi > >>>>>>> index 7f51104..20c396d 100644 > >>>>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi > >>>>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi > >>>>>>> @@ -141,19 +141,19 @@ > >>>>>>> }; > >>>>>>> > >>>>>>> can0@1c000 { > >>>>>>> - compatible = "fsl,flexcan-v1.0"; > >>>>>>> + compatible = "fsl,p1010-flexcan", > >>>>>>> + "fsl,flexcan"; > >>>>>> > >>>>>> Does fit on one line. > >>>>>> > >>>>>>> reg = <0x1c000 0x1000>; > >>>>>>> interrupts = <48 0x2>; > >>>>>>> interrupt-parent = <&mpic>; > >>>>>>> - fsl,flexcan-clock-divider = <2>; > >>>>>>> }; > >>>>>>> > >>>>>>> can1@1d000 { > >>>>>>> - compatible = "fsl,flexcan-v1.0"; > >>>>>>> + compatible = "fsl,p1010-flexcan", > >>>>>>> + "fsl,flexcan"; > >>>>>> > >>>>>> Ditto > >>>>>> > >>>>>>> reg = <0x1d000 0x1000>; > >>>>>>> interrupts = <61 0x2>; > >>>>>>> interrupt-parent = <&mpic>; > >>>>>>> - fsl,flexcan-clock-divider = <2>; > >>>>>>> }; > >>>>>>> > >>>>>>> L2: l2-cache-controller@20000 { > >>>>>> > >>>>>> Please also correct the node names (not using the number suffix). > >>>>> > >>>>> So the node names should be > >>>>> can@1c000 { > >>>>> can@1d000 { > >>>>> correct? > >>>>> > >>>> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better > >>>> to distinguish it by can0 and can1 instead by simple "can" ? > >>> > >>> It looks like the way to do that is to assign a label to those devices > >>> and then associate the label with an alias. I have no idea how that > >>> works under the hood, but it is the way other files are set up. Take a > >>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial > >>> interfaces. > >>> > >>> Grant or Wolfgang, is that the right way to handle the concern about > >>> names or does it have no practical effect with the Linux kernel? > >> > >> It has not effect. The label is just if you need to reference it via some other means. > > > > Does the alias have an effect? > > nope Then how does the device number get associated with a particular device and how is user-space ensured a consistent namespace? Robin
>>>>>>> So the node names should be >>>>>>> can@1c000 { >>>>>>> can@1d000 { >>>>>>> correct? >>>>>>> >>>>>> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better >>>>>> to distinguish it by can0 and can1 instead by simple "can" ? >>>>> >>>>> It looks like the way to do that is to assign a label to those devices >>>>> and then associate the label with an alias. I have no idea how that >>>>> works under the hood, but it is the way other files are set up. Take a >>>>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial >>>>> interfaces. >>>>> >>>>> Grant or Wolfgang, is that the right way to handle the concern about >>>>> names or does it have no practical effect with the Linux kernel? >>>> >>>> It has not effect. The label is just if you need to reference it via some other means. >>> >>> Does the alias have an effect? >> >> nope > > Then how does the device number get associated with a particular device What do you mean by device number? > and how is user-space ensured a consistent namespace? that is left to udev rules. - k
On 08/10/2011 06:00 PM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: ... > It looks like the way to do that is to assign a label to those devices > and then associate the label with an alias. I have no idea how that > works under the hood, but it is the way other files are set up. Take a > look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial > interfaces. With a label you mean "label:" at the beginning of a node. Such labels are translated by the device tree compiler in node handles, which can be referenced within nodes by using <&label>, e.g.: UIC0: interrupt-controller0 { ... }; UIC1: interrupt-controller1 { ... interrupt-parent = <&UIC0>; ... }; It has nothing to do with the name of the node. Wolfgang.
On 08/10/2011 01:23 PM, Wolfgang Grandegger wrote: > On 08/10/2011 06:00 PM, Robin Holt wrote: >> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: > ... >> It looks like the way to do that is to assign a label to those devices >> and then associate the label with an alias. I have no idea how that >> works under the hood, but it is the way other files are set up. Take a >> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial >> interfaces. > > With a label you mean "label:" at the beginning of a node. Such labels > are translated by the device tree compiler in node handles, which can be > referenced within nodes by using <&label>, e.g.: > > UIC0: interrupt-controller0 { > ... > }; > UIC1: interrupt-controller1 { > ... > interrupt-parent = <&UIC0>; > ... > }; > > It has nothing to do with the name of the node. "...and then associate the label with an alias." The alias can then be used if you want "can0" versus "can1". Appending numbers to the node name is typically only done when there's no unit address, and a need to disambiguate. -Scott
On Wed, Aug 10, 2011 at 01:27:52PM -0500, Scott Wood wrote: > On 08/10/2011 01:23 PM, Wolfgang Grandegger wrote: > > On 08/10/2011 06:00 PM, Robin Holt wrote: > >> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote: > > ... > >> It looks like the way to do that is to assign a label to those devices > >> and then associate the label with an alias. I have no idea how that > >> works under the hood, but it is the way other files are set up. Take a > >> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial > >> interfaces. > > > > With a label you mean "label:" at the beginning of a node. Such labels > > are translated by the device tree compiler in node handles, which can be > > referenced within nodes by using <&label>, e.g.: > > > > UIC0: interrupt-controller0 { > > ... > > }; > > UIC1: interrupt-controller1 { > > ... > > interrupt-parent = <&UIC0>; > > ... > > }; > > > > It has nothing to do with the name of the node. > > "...and then associate the label with an alias." > > The alias can then be used if you want "can0" versus "can1". Does the alias get used by either the kernel or something else or is it just extra detail with no purpose? Robin
On 08/10/2011 01:35 PM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 01:27:52PM -0500, Scott Wood wrote: >> "...and then associate the label with an alias." >> >> The alias can then be used if you want "can0" versus "can1". > > Does the alias get used by either the kernel or something else or is it > just extra detail with no purpose? It could be used by udev to produce a symlink. Currently, the major non-human consumer of the aliases is U-Boot. -Scott
diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt deleted file mode 100644 index 1a729f0..0000000 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ /dev/null @@ -1,61 +0,0 @@ -CAN Device Tree Bindings ------------------------- -2011 Freescale Semiconductor, Inc. - -fsl,flexcan-v1.0 nodes ------------------------ -In addition to the required compatible-, reg- and interrupt-properties, you can -also specify which clock source shall be used for the controller. - -CPI Clock- Can Protocol Interface Clock - This CLK_SRC bit of CTRL(control register) selects the clock source to - the CAN Protocol Interface(CPI) to be either the peripheral clock - (driven by the PLL) or the crystal oscillator clock. The selected clock - is the one fed to the prescaler to generate the Serial Clock (Sclock). - The PRESDIV field of CTRL(control register) controls a prescaler that - generates the Serial Clock (Sclock), whose period defines the - time quantum used to compose the CAN waveform. - -Can Engine Clock Source - There are two sources for CAN clock - - Platform Clock It represents the bus clock - - Oscillator Clock - - Peripheral Clock (PLL) - -------------- - | - --------- ------------- - | |CPI Clock | Prescaler | Sclock - | |---------------->| (1.. 256) |------------> - --------- ------------- - | | - -------------- ---------------------CLK_SRC - Oscillator Clock - -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects - the peripheral clock. PLL clock is fed to the - prescaler to generate the Serial Clock (Sclock). - Valid values are "oscillator" and "platform" - "oscillator": CAN engine clock source is oscillator clock. - "platform" The CAN engine clock source is the bus clock - (platform clock). - -- fsl,flexcan-clock-divider : for the reference and system clock, an additional - clock divider can be specified. -- clock-frequency: frequency required to calculate the bitrate for FlexCAN. - -Note: - - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC. - - P1010 does not have oscillator as the Clock Source.So the default - Clock Source is platform clock. -Examples: - - can0@1c000 { - compatible = "fsl,flexcan-v1.0"; - reg = <0x1c000 0x1000>; - interrupts = <48 0x2>; - interrupt-parent = <&mpic>; - fsl,flexcan-clock-source = "platform"; - fsl,flexcan-clock-divider = <2>; - clock-frequency = <fixed by u-boot>; - }; diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts index 6b33b73..d6a0bb2 100644 --- a/arch/powerpc/boot/dts/p1010rdb.dts +++ b/arch/powerpc/boot/dts/p1010rdb.dts @@ -169,14 +169,6 @@ }; }; - can0@1c000 { - fsl,flexcan-clock-source = "platform"; - }; - - can1@1d000 { - fsl,flexcan-clock-source = "platform"; - }; - usb@22000 { phy_type = "utmi"; }; diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi index 7f51104..20c396d 100644 --- a/arch/powerpc/boot/dts/p1010si.dtsi +++ b/arch/powerpc/boot/dts/p1010si.dtsi @@ -141,19 +141,19 @@ }; can0@1c000 { - compatible = "fsl,flexcan-v1.0"; + compatible = "fsl,p1010-flexcan", + "fsl,flexcan"; reg = <0x1c000 0x1000>; interrupts = <48 0x2>; interrupt-parent = <&mpic>; - fsl,flexcan-clock-divider = <2>; }; can1@1d000 { - compatible = "fsl,flexcan-v1.0"; + compatible = "fsl,p1010-flexcan", + "fsl,flexcan"; reg = <0x1d000 0x1000>; interrupts = <61 0x2>; interrupt-parent = <&mpic>; - fsl,flexcan-clock-divider = <2>; }; L2: l2-cache-controller@20000 {
In working with the socketcan developers, we have come to the conclusion the Documentation...fsl-flexcan.txt device tree documentation needs to be cleaned up. The driver does not depend upon any properties other than the required properties so we are removing the file. Additionally, the p1010*dts* files are not following the standard for node naming in that they have a trailing -v1.0. Signed-off-by: Robin Holt <holt@sgi.com> To: Marc Kleine-Budde <mkl@pengutronix.de>, To: Wolfgang Grandegger <wg@grandegger.com>, To: U Bhaskar-B22300 <B22300@freescale.com> To: Scott Wood <scottwood@freescale.com> Cc: socketcan-core@lists.berlios.de, Cc: netdev@vger.kernel.org, Cc: PPC list <linuxppc-dev@lists.ozlabs.org> Cc: Kumar Gala <galak@kernel.crashing.org> --- .../devicetree/bindings/net/can/fsl-flexcan.txt | 61 -------------------- arch/powerpc/boot/dts/p1010rdb.dts | 8 --- arch/powerpc/boot/dts/p1010si.dtsi | 8 +- 3 files changed, 4 insertions(+), 73 deletions(-) delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt