* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Kumar Gala @ 2011-08-10 14:52 UTC (permalink / raw)
To: Robin Holt
Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300,
socketcan-core, Scott Wood, PPC list
In-Reply-To: <20110809205900.GC4926@sgi.com>
On Aug 9, 2011, at 3:59 PM, Robin Holt wrote:
> I guess my poor wording may have gotten me in trouble. I am getting
> ready to repost this patch, but I want to ensure I am getting it as
> right as possible.
>=20
> I think I should reword the commit message to indicate we are removing
> the Documentation/.../fsl-flexcan.txt file which has essentially =
become
> empty and change the p1010si.dtsi file's can nodes to =
"fsl,p1010-flexcan",
> "fsl,flexcan". Is that correct?
>=20
> Thanks,
> Robin
This is wrong. Again, what binding covers "fsl,flexcan" if you remove =
fsl-flexcan.txt:
[galak@right powerpc]$ git grep flexcan =
Documentation/devicetree/bindings
=
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:fsl,flexcan-v1.0=
nodes
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:- =
fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:- =
fsl,flexcan-clock-divider : for the reference and system clock, an =
additional
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: - v1.0 =
of flexcan-v1.0 represent the IP block version for P1010 SOC.
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: =
compatible =3D "fsl,flexcan-v1.0";
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: =
fsl,flexcan-clock-source =3D "platform";
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: =
fsl,flexcan-clock-divider =3D <2>;
Not seeing anything that covers it.
I think the issue should be resolved by patching fsl-flexcan.txt to =
remove wording or update it.
- k=
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Kumar Gala @ 2011-08-10 14:46 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, Scott Wood,
PPC list
In-Reply-To: <4E42987F.9040304@grandegger.com>
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
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Kumar Gala @ 2011-08-10 14:45 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, Scott Wood, PPC list
In-Reply-To: <20110810141550.GQ4926@sgi.com>
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,
>>=20
>> 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
>>=20
>> Your first sentence could be misleading. Please just describe what =
the
>> patch does and why, something like:
>>=20
>> "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.
>=20
> I borrowed heavily from your message. ;)
>=20
>>> 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.
>>=20
>>> 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
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Wolfgang Grandegger @ 2011-08-10 14:41 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, Scott Wood, PPC list
In-Reply-To: <20110810141550.GQ4926@sgi.com>
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.
^ permalink raw reply
* RE: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: U Bhaskar-B22300 @ 2011-08-10 14:36 UTC (permalink / raw)
To: Robin Holt, Wolfgang Grandegger
Cc: Wood Scott-B07421, netdev@vger.kernel.org,
socketcan-core@lists.berlios.de, PPC list
In-Reply-To: <20110810141550.GQ4926@sgi.com>
> -----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.
>=20
> 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.
>=20
> I borrowed heavily from your message. ;)
>=20
> > > 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 =3D "fsl,flexcan-v1.0";
> > > - reg =3D <0x1c000 0x1000>;
> > > - interrupts =3D <48 0x2>;
> > > - interrupt-parent =3D <&mpic>;
> > > - fsl,flexcan-clock-source =3D "platform";
> > > - fsl,flexcan-clock-divqider =3D <2>;
> > > - clock-frequency =3D <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 =3D "fsl,p1010-flexcan", "fsl,flexcan";
> > reg =3D <0x1c000 0x1000>;
> > interrupts =3D <48 0x2>;
> > interrupt-parent =3D <&mpic>;
> > };
> > -------------------------
>=20
> Done, except the
> > compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
>=20
> line is
> compatible =3D "fsl,flexcan", "fsl,flexcan-p1010";
>=20
> >
> > 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 =3D "platform";
> > > - };
> > > -
> > > - can1@1d000 {
> > > - fsl,flexcan-clock-source =3D "platform";
> > > - };
> > > -
> > > usb@22000 {
> > > phy_type =3D "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 =3D "fsl,flexcan-v1.0";
> > > + compatible =3D "fsl,p1010-flexcan",
> > > + "fsl,flexcan";
> >
> > Does fit on one line.
> >
> > > reg =3D <0x1c000 0x1000>;
> > > interrupts =3D <48 0x2>;
> > > interrupt-parent =3D <&mpic>;
> > > - fsl,flexcan-clock-divider =3D <2>;
> > > };
> > >
> > > can1@1d000 {
> > > - compatible =3D "fsl,flexcan-v1.0";
> > > + compatible =3D "fsl,p1010-flexcan",
> > > + "fsl,flexcan";
> >
> > Ditto
> >
> > > reg =3D <0x1d000 0x1000>;
> > > interrupts =3D <61 0x2>;
> > > interrupt-parent =3D <&mpic>;
> > > - fsl,flexcan-clock-divider =3D <2>;
> > > };
> > >
> > > L2: l2-cache-controller@20000 {
> >
> > Please also correct the node names (not using the number suffix).
>=20
> So the node names should be
> can@1c000 {
> can@1d000 {
> correct?
>=20
[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
^ permalink raw reply
* Re: [PATCH v10 3/5] [flexcan] Add of_match to platform_device definition.
From: Robin Holt @ 2011-08-10 14:33 UTC (permalink / raw)
To: Robin Holt, Wolfgang Grandegger
Cc: socketcan-core, netdev, PPC list, U Bhaskar-B22300
In-Reply-To: <1312945564-6626-4-git-send-email-holt@sgi.com>
On Tue, Aug 09, 2011 at 10:06:02PM -0500, Robin Holt wrote:
> On powerpc, the OpenFirmware devices are not matched without specifying
> an of_match array. Introduce that array as that is used for matching
> on the Freescale P1010 processor.
>
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
> To: U Bhaskar-B22300 <B22300@freescale.com>
> Cc: socketcan-core@lists.berlios.de
> Cc: netdev@vger.kernel.org
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> ---
> drivers/net/can/flexcan.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 68cbe52..662f832 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1027,8 +1027,19 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static struct of_device_id flexcan_of_match[] = {
> + {
> + .compatible = "fsl,flexcan",
Let me make sure I have this correct. At this point, we would want it
to be fsl,flexcan here. If, at some point, we find the i.MX-wonderful
has diverged from the -p1010, we would, at that point in the code, use
of_device_is_compatible to differentiate the two, correct? That would
mean we should make no change to this patch for the fsl,flexcan-p1010,
right?
Robin
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-10 14:15 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, Scott Wood,
PPC list
In-Reply-To: <4E428BFF.7060402@grandegger.com>
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
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Wolfgang Grandegger @ 2011-08-10 13:47 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, Scott Wood, PPC list
In-Reply-To: <1312945564-6626-6-git-send-email-holt@sgi.com>
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.
^ permalink raw reply
* [PATCH 11/10] KVM: PPC: Add sanity checking to vcpu_run
From: Alexander Graf @ 2011-08-10 12:31 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, paulus, kvm
In-Reply-To: <1312907508-14599-1-git-send-email-agraf@suse.de>
There are multiple features in PowerPC KVM that can now be enabled
depending on the user's wishes. Some of the combinations don't make
sense or don't work though.
So this patch adds a way to check if the executing environment would
actually be able to run the guest properly. It also adds sanity
checks if PVR is set (should always be true given the current code
flow), if PAPR is only used with book3s_64 where it works and that
HV KVM is only used in PAPR mode.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
arch/powerpc/include/asm/kvm.h | 5 +++++
arch/powerpc/include/asm/kvm_host.h | 2 ++
arch/powerpc/include/asm/kvm_ppc.h | 1 +
arch/powerpc/kvm/44x.c | 2 ++
arch/powerpc/kvm/book3s_hv.c | 8 ++++++++
arch/powerpc/kvm/book3s_pr.c | 10 ++++++++++
arch/powerpc/kvm/booke.c | 10 +++++++++-
arch/powerpc/kvm/e500.c | 2 ++
arch/powerpc/kvm/powerpc.c | 28 ++++++++++++++++++++++++++++
9 files changed, 67 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index a6a253e..08fe69e 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -284,6 +284,11 @@ struct kvm_guest_debug_arch {
#define KVM_INTERRUPT_UNSET -2U
#define KVM_INTERRUPT_SET_LEVEL -3U
+#define KVM_CPU_440 1
+#define KVM_CPU_E500V2 2
+#define KVM_CPU_3S_32 3
+#define KVM_CPU_3S_64 4
+
/* for KVM_CAP_SPAPR_TCE */
struct kvm_create_spapr_tce {
__u64 liobn;
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index e681302..2b8284f 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -390,6 +390,8 @@ struct kvm_vcpu_arch {
u8 osi_needed;
u8 osi_enabled;
u8 papr_enabled;
+ u8 sane;
+ u8 cpu_type;
u8 hcall_needed;
u32 cpr0_cfgaddr; /* holds the last set cpr0_cfgaddr */
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index d121f49..46efd1a 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -66,6 +66,7 @@ extern int kvmppc_emulate_instruction(struct kvm_run *run,
extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu);
extern void kvmppc_emulate_dec(struct kvm_vcpu *vcpu);
extern u32 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb);
+extern int kvmppc_sanity_check(struct kvm_vcpu *vcpu);
/* Core-specific hooks */
diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c
index da3a122..ca1f88b 100644
--- a/arch/powerpc/kvm/44x.c
+++ b/arch/powerpc/kvm/44x.c
@@ -78,6 +78,8 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
for (i = 0; i < ARRAY_SIZE(vcpu_44x->shadow_refs); i++)
vcpu_44x->shadow_refs[i].gtlb_index = -1;
+ vcpu->arch.cpu_type = KVM_CPU_440;
+
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index cc0d7f1..830e07a 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -510,6 +510,9 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
spin_unlock(&vcore->lock);
vcpu->arch.vcore = vcore;
+ vcpu->arch.cpu_type = KVM_CPU_3S_64;
+ kvmppc_sanity_check(vcpu);
+
return vcpu;
free_vcpu:
@@ -800,6 +803,11 @@ int kvmppc_vcpu_run(struct kvm_run *run, struct kvm_vcpu *vcpu)
{
int r;
+ if (!vcpu->arch.sane) {
+ kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+ return -EINVAL;
+ }
+
do {
r = kvmppc_run_vcpu(run, vcpu);
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 48558f6..6e3488b 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -153,6 +153,7 @@ void kvmppc_set_pvr(struct kvm_vcpu *vcpu, u32 pvr)
if (!to_book3s(vcpu)->hior_sregs)
to_book3s(vcpu)->hior = 0xfff00000;
to_book3s(vcpu)->msr_mask = 0xffffffffffffffffULL;
+ vcpu->arch.cpu_type = KVM_CPU_3S_64;
} else
#endif
{
@@ -160,8 +161,11 @@ void kvmppc_set_pvr(struct kvm_vcpu *vcpu, u32 pvr)
if (!to_book3s(vcpu)->hior_sregs)
to_book3s(vcpu)->hior = 0;
to_book3s(vcpu)->msr_mask = 0xffffffffULL;
+ vcpu->arch.cpu_type = KVM_CPU_3S_32;
}
+ kvmppc_sanity_check(vcpu);
+
/* If we are in hypervisor level on 970, we can tell the CPU to
* treat DCBZ as 32 bytes store */
vcpu->arch.hflags &= ~BOOK3S_HFLAG_DCBZ32;
@@ -938,6 +942,12 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
#endif
ulong ext_msr;
+ /* Check if we can run the vcpu at all */
+ if (!vcpu->arch.sane) {
+ kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+ return -EINVAL;
+ }
+
/* No need to go into the guest when all we do is going out */
if (signal_pending(current)) {
kvm_run->exit_reason = KVM_EXIT_INTR;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index ee45fa0..bb6c988 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -316,6 +316,11 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
{
int ret;
+ if (!vcpu->arch.sane) {
+ kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+ return -EINVAL;
+ }
+
local_irq_disable();
kvm_guest_enter();
ret = __kvmppc_vcpu_run(kvm_run, vcpu);
@@ -618,6 +623,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
{
int i;
+ int r;
vcpu->arch.pc = 0;
vcpu->arch.shared->msr = 0;
@@ -634,7 +640,9 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
kvmppc_init_timing_stats(vcpu);
- return kvmppc_core_vcpu_setup(vcpu);
+ r = kvmppc_core_vcpu_setup(vcpu);
+ kvmppc_sanity_check(vcpu);
+ return r;
}
int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
index 797a744..26d2090 100644
--- a/arch/powerpc/kvm/e500.c
+++ b/arch/powerpc/kvm/e500.c
@@ -73,6 +73,8 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
/* Since booke kvm only support one core, update all vcpus' PIR to 0 */
vcpu->vcpu_id = 0;
+ vcpu->arch.cpu_type = KVM_CPU_E500V2;
+
return 0;
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 13bc798..a8000ce 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -95,6 +95,31 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
return r;
}
+int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
+{
+ int r = false;
+
+ /* We have to know what CPU to virtualize */
+ if (!vcpu->arch.pvr)
+ goto out;
+
+ /* PAPR only works with book3s_64 */
+ if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled)
+ goto out;
+
+#ifdef CONFIG_KVM_BOOK3S_64_HV
+ /* HV KVM can only do PAPR mode for now */
+ if (!vcpu->arch.papr_enabled)
+ goto out;
+#endif
+
+ r = true;
+
+out:
+ vcpu->arch.sane = r;
+ return r ? 0 : -EINVAL;
+}
+
int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
{
enum emulation_result er;
@@ -582,6 +607,9 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
break;
}
+ if (!r)
+ r = kvmppc_sanity_check(vcpu);
+
return r;
}
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH 10/10] KVM: PPC: Enable the PAPR CAP for Book3S
From: Alexander Graf @ 2011-08-10 12:29 UTC (permalink / raw)
To: Paul Mackerras
Cc: linuxppc-dev@ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <20110810122632.GB5536@bloggs.ozlabs.ibm.com>
On 08/10/2011 02:26 PM, Paul Mackerras wrote:
> On Wed, Aug 10, 2011 at 09:59:41AM +0200, Alexander Graf wrote:
>> Am 10.08.2011 um 06:42 schrieb Paul Mackerras<paulus@samba.org>:
>>
>>> On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:
>>>
>>>> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
>>>> enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
>>>> mode.
>>> Don't we want to enable it only for 64-bit hosts? Trying to run a
>>> PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
>>> missing something...
>> I agree that it doesn't make sense, but if anything we should
>> restrict it to 64-bit _guests_. you can also run 32-bit guests on
>> 64-bit hosts.
> I had a look in PAPR and I didn't find anything that says the
> processor has to be 64-bit, so I guess a 32-bit PAPR guest is possible
> in theory. However, I don't think there are currently any 32-bit PAPR
> operating systems that would use hcalls.
That's what I figured :). The code flow we're affecting here is pretty
much generic.
>> And so far, we don't have a single interface setting PVR and PAPR
>> mode at the same time, so you could still enable PAPR with a 64-bit
>> guest CPU and then switch to a 32-bit CPU.
>>
>> It'd be a nightmare to check all configurations on every setter function.
>>
>> Unless...
>>
>> We could introduce a sanity check function that gets executed every
>> time we change PVR or enable PAPR. That could set a variable in the
>> vcpu struct to indicate that the config is ok. We could then check
>> that on vcpu_run.
> It's probably not worth worrying about it.
Too late, already implemented it :). It really does make sense to have
some sort of checking here - even if it only means that our hypercall
implementation can't handle it yet or that we didn't test it. And we can
put the "HV KVM can only run PAPR" check in there as well.
Alex
^ permalink raw reply
* Re: [PATCH 10/10] KVM: PPC: Enable the PAPR CAP for Book3S
From: Paul Mackerras @ 2011-08-10 12:26 UTC (permalink / raw)
To: Alexander Graf
Cc: linuxppc-dev@ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <0B73DEC7-C430-43D7-8095-D3D0BEDA27AA@suse.de>
On Wed, Aug 10, 2011 at 09:59:41AM +0200, Alexander Graf wrote:
>
> Am 10.08.2011 um 06:42 schrieb Paul Mackerras <paulus@samba.org>:
>
> > On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:
> >
> >> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap and
> >> enable it for all Book3S targets. Enabling that CAP switches KVM into PAPR
> >> mode.
> >
> > Don't we want to enable it only for 64-bit hosts? Trying to run a
> > PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
> > missing something...
>
> I agree that it doesn't make sense, but if anything we should
> restrict it to 64-bit _guests_. you can also run 32-bit guests on
> 64-bit hosts.
I had a look in PAPR and I didn't find anything that says the
processor has to be 64-bit, so I guess a 32-bit PAPR guest is possible
in theory. However, I don't think there are currently any 32-bit PAPR
operating systems that would use hcalls.
> And so far, we don't have a single interface setting PVR and PAPR
> mode at the same time, so you could still enable PAPR with a 64-bit
> guest CPU and then switch to a 32-bit CPU.
>
> It'd be a nightmare to check all configurations on every setter function.
>
> Unless...
>
> We could introduce a sanity check function that gets executed every
> time we change PVR or enable PAPR. That could set a variable in the
> vcpu struct to indicate that the config is ok. We could then check
> that on vcpu_run.
It's probably not worth worrying about it.
The rest of the series looks very nice.
Regards,
Paul.
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
From: Anton Blanchard @ 2011-08-10 11:28 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: paulus, linuxppc-dev
In-Reply-To: <20110810183328.cfdfeec82b0d9eb0721687a8@canb.auug.org.au>
Hi Stephen,
> > Index: linux-powerpc/arch/powerpc/mm/numa.c
> > ===================================================================
> > --- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-06-06
> > 08:07:35.148708089 +1000 +++
> > linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10
> > 11:31:59.723379868 +1000 @@ -1214,7 +1214,6 @@ int
> > hot_add_node_scn_to_nid(unsigned lon break; }
> >
> > - of_node_put(memory);
> > if (nid >= 0)
> > break;
> > }
>
> Won't that need an of_node_put(memory) after the loop if (nid >=
> 0) ? In fact you should be able to just move the of_node_put(memory)
> to after the loop since of_node_put(NULL) is fine.
Nice catch! Will respin the patch.
Anton
^ permalink raw reply
* Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)
From: Marc Kleine-Budde @ 2011-08-10 11:09 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, PPC list
In-Reply-To: <1312945564-6626-1-git-send-email-holt@sgi.com>
[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]
On 08/10/2011 05:05 AM, Robin Holt wrote:
> With all the patches applied, my p1010rdb works for communicating between
> its two can ports and also can communicate with an external PSOC. I have
> done no testing beyond compile testing on an arm system as I have no
> access to an arm based system.
>
> For the first three patches in the series, I believe they are all ready
> for forwarding to David S. Miller for the netdev tree. I think patch
> 4 is ready for submission to the PPC85xx maintainer. Patch 5 changed
> from the previous post by adding a second compatible string for the
> fsl,p1010_flexcan.
This patch series is working on phytec's pcm043 (mx35) based on current
net-next. Feel free to add my Acked-by to all patches.
good work,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)
From: Marc Kleine-Budde @ 2011-08-10 10:24 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, PPC list
In-Reply-To: <20110810101113.GN4926@sgi.com>
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
On 08/10/2011 12:11 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 12:01:58PM +0200, Marc Kleine-Budde wrote:
>> On 08/10/2011 05:05 AM, Robin Holt wrote:
>>> With all the patches applied, my p1010rdb works for communicating between
>>> its two can ports and also can communicate with an external PSOC. I have
>>> done no testing beyond compile testing on an arm system as I have no
>>> access to an arm based system.
>>>
>>> For the first three patches in the series, I believe they are all ready
>>> for forwarding to David S. Miller for the netdev tree. I think patch
>>> 4 is ready for submission to the PPC85xx maintainer. Patch 5 changed
>>> from the previous post by adding a second compatible string for the
>>> fsl,p1010_flexcan.
>>
>> One remark for the subjects. It seems no one is using "[$subsystem]"
>> anymore, but rather "$subsystem:".
>
> I will, one day, get these patches subject lines correct ;)
>
> Did you happen to get a chance to test the patches on arm?
It's still compiling...
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH v10 4/5] [powerpc] Add flexcan device support for p1010rdb.
From: Robin Holt @ 2011-08-10 10:15 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list
In-Reply-To: <4E4255B1.6050001@pengutronix.de>
On Wed, Aug 10, 2011 at 11:56:01AM +0200, Marc Kleine-Budde wrote:
> On 08/10/2011 05:06 AM, Robin Holt wrote:
> > diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> > new file mode 100644
> > index 0000000..16fae04
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/85xx/clock.c
> > @@ -0,0 +1,53 @@
...
> > +void __init mpc85xx_clk_init(void)
> > +{
> > + clk_functions = mpc85xx_clk_functions;
> > +}
> > +
>
> git is even picker then me: "new blank line at EOF."
> please fix
That reminded me that I have slacked on using checkpatch.pl in a few
resends and it really does not like this patch at all so I will rework
the patch with its concerns. I am going to hold off on resending until
there are more substantial changes.
Thanks,
Robin
^ permalink raw reply
* Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)
From: Robin Holt @ 2011-08-10 10:11 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list
In-Reply-To: <4E425716.4010104@pengutronix.de>
On Wed, Aug 10, 2011 at 12:01:58PM +0200, Marc Kleine-Budde wrote:
> On 08/10/2011 05:05 AM, Robin Holt wrote:
> > With all the patches applied, my p1010rdb works for communicating between
> > its two can ports and also can communicate with an external PSOC. I have
> > done no testing beyond compile testing on an arm system as I have no
> > access to an arm based system.
> >
> > For the first three patches in the series, I believe they are all ready
> > for forwarding to David S. Miller for the netdev tree. I think patch
> > 4 is ready for submission to the PPC85xx maintainer. Patch 5 changed
> > from the previous post by adding a second compatible string for the
> > fsl,p1010_flexcan.
>
> One remark for the subjects. It seems no one is using "[$subsystem]"
> anymore, but rather "$subsystem:".
I will, one day, get these patches subject lines correct ;)
Did you happen to get a chance to test the patches on arm?
Thanks,
Robin
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-10 10:09 UTC (permalink / raw)
To: Grant Likely, devicetree-discuss
Cc: netdev, U Bhaskar-B22300, socketcan-core, Scott Wood, PPC list
In-Reply-To: <1312945564-6626-6-git-send-email-holt@sgi.com>
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
^ permalink raw reply
* Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)
From: Marc Kleine-Budde @ 2011-08-10 10:01 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, PPC list
In-Reply-To: <1312945564-6626-1-git-send-email-holt@sgi.com>
[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]
On 08/10/2011 05:05 AM, Robin Holt wrote:
> With all the patches applied, my p1010rdb works for communicating between
> its two can ports and also can communicate with an external PSOC. I have
> done no testing beyond compile testing on an arm system as I have no
> access to an arm based system.
>
> For the first three patches in the series, I believe they are all ready
> for forwarding to David S. Miller for the netdev tree. I think patch
> 4 is ready for submission to the PPC85xx maintainer. Patch 5 changed
> from the previous post by adding a second compatible string for the
> fsl,p1010_flexcan.
One remark for the subjects. It seems no one is using "[$subsystem]"
anymore, but rather "$subsystem:".
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH v10 4/5] [powerpc] Add flexcan device support for p1010rdb.
From: Marc Kleine-Budde @ 2011-08-10 9:56 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, PPC list
In-Reply-To: <1312945564-6626-5-git-send-email-holt@sgi.com>
[-- Attachment #1: Type: text/plain, Size: 4724 bytes --]
On 08/10/2011 05:06 AM, Robin Holt wrote:
> I added a simple clock source for the p1010rdb so the flexcan driver
> could determine a clock frequency. The p1010 can device only has an
> oscillator of system bus frequency divided by 2.
>
> Signed-off-by: Robin Holt <holt@sgi.com>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@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>
> ---
> arch/powerpc/platforms/85xx/Kconfig | 2 +
> arch/powerpc/platforms/85xx/Makefile | 2 +
> arch/powerpc/platforms/85xx/clock.c | 53 ++++++++++++++++++++++++++++++++
> arch/powerpc/platforms/85xx/p1010rdb.c | 8 +++++
> 4 files changed, 65 insertions(+), 0 deletions(-)
> create mode 100644 arch/powerpc/platforms/85xx/clock.c
>
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index 498534c..c4304ae 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -70,6 +70,8 @@ config MPC85xx_RDB
> config P1010_RDB
> bool "Freescale P1010RDB"
> select DEFAULT_UIMAGE
> + select HAVE_CAN_FLEXCAN if NET && CAN
> + select PPC_CLOCK if CAN_FLEXCAN
> help
> This option enables support for the MPC85xx RDB (P1010 RDB) board
>
> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> index a971b32..cc7f381 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -3,6 +3,8 @@
> #
> obj-$(CONFIG_SMP) += smp.o
>
> +obj-$(CONFIG_PPC_CLOCK) += clock.o
> +
> obj-$(CONFIG_MPC8540_ADS) += mpc85xx_ads.o
> obj-$(CONFIG_MPC8560_ADS) += mpc85xx_ads.o
> obj-$(CONFIG_MPC85xx_CDS) += mpc85xx_cds.o
> diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> new file mode 100644
> index 0000000..16fae04
> --- /dev/null
> +++ b/arch/powerpc/platforms/85xx/clock.c
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright 2011 SGI, inc.
> + *
> + * This code is licensed for use under the GPL V2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +
> +#include <asm/clk_interface.h>
> +
> +#include <sysdev/fsl_soc.h>
> +
> +/*
> + * p1010 needs to provide a clock source for the flexcan driver. The
> + * oscillator for the p1010 processor is only ever the system clock / 2.
> + */
> +
> +static struct clk *mpc85xx_clk_get(struct device *dev, const char *id)
> +{
> + if (!dev)
> + return ERR_PTR(-ENOENT);
> +
> + if (!dev->of_node ||
> + !of_device_is_compatible(dev->of_node, "fsl,flexcan"))
> + return ERR_PTR(-ENOENT);
> +
> + return NULL;
> +}
> +
> +static void mpc85xx_clk_put(struct clk *clk)
> +{
> + return;
> +}
> +
> +static unsigned long mpc85xx_clk_get_rate(struct clk *clk)
> +{
> + return fsl_get_sys_freq() / 2;
> +}
> +
> +static struct clk_interface mpc85xx_clk_functions = {
> + .clk_get = mpc85xx_clk_get,
> + .clk_get_rate = mpc85xx_clk_get_rate,
> + .clk_put = mpc85xx_clk_put,
> +};
> +
> +void __init mpc85xx_clk_init(void)
> +{
> + clk_functions = mpc85xx_clk_functions;
> +}
> +
git is even picker then me: "new blank line at EOF."
please fix
> diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
> index d7387fa..5e52122 100644
> --- a/arch/powerpc/platforms/85xx/p1010rdb.c
> +++ b/arch/powerpc/platforms/85xx/p1010rdb.c
> @@ -81,6 +81,13 @@ static void __init p1010_rdb_setup_arch(void)
> printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
> }
>
> +extern void mpc85xx_clk_init(void);
> +
> +static void __init p1010_rdb_init(void)
> +{
> + mpc85xx_clk_init();
> +}
> +
> static struct of_device_id __initdata p1010rdb_ids[] = {
> { .type = "soc", },
> { .compatible = "soc", },
> @@ -111,6 +118,7 @@ define_machine(p1010_rdb) {
> .name = "P1010 RDB",
> .probe = p1010_rdb_probe,
> .setup_arch = p1010_rdb_setup_arch,
> + .init = p1010_rdb_init,
> .init_IRQ = p1010_rdb_pic_init,
> #ifdef CONFIG_PCI
> .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
From: Stephen Rothwell @ 2011-08-10 8:33 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev
In-Reply-To: <20110810064937.424887717@samba.org>
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
Hi Anton,
On Wed, 10 Aug 2011 16:49:34 +1000 Anton Blanchard <anton@samba.org> wrote:
>
> During memory hotplug testing, I got the following warning:
>
>
> ERROR: Bad of_node_put() on /memory@0
>
> of_find_node_by_type() loop does of_node_put for us so remove the
> duplicate one inside the loop.
But does an of_node_get() on its return value ..
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Cc: stable@kernel.org
> ---
>
> Index: linux-powerpc/arch/powerpc/mm/numa.c
> ===================================================================
> --- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-06-06 08:07:35.148708089 +1000
> +++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10 11:31:59.723379868 +1000
> @@ -1214,7 +1214,6 @@ int hot_add_node_scn_to_nid(unsigned lon
> break;
> }
>
> - of_node_put(memory);
> if (nid >= 0)
> break;
> }
Won't that need an of_node_put(memory) after the loop if (nid >= 0) ? In
fact you should be able to just move the of_node_put(memory) to after the
loop since of_node_put(NULL) is fine.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Robert Schwebel @ 2011-08-10 8:15 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, Scott Wood, PPC list
In-Reply-To: <1312945564-6626-6-git-send-email-holt@sgi.com>
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
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 10/10] KVM: PPC: Enable the PAPR CAP for Book3S
From: Alexander Graf @ 2011-08-10 7:59 UTC (permalink / raw)
To: Paul Mackerras
Cc: linuxppc-dev@ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <20110810044215.GA5404@drongo>
Am 10.08.2011 um 06:42 schrieb Paul Mackerras <paulus@samba.org>:
> On Tue, Aug 09, 2011 at 06:31:48PM +0200, Alexander Graf wrote:
>=20
>> Now that Book3S PV mode can also run PAPR guests, we can add a PAPR cap a=
nd
>> enable it for all Book3S targets. Enabling that CAP switches KVM into PAP=
R
>> mode.
>=20
> Don't we want to enable it only for 64-bit hosts? Trying to run a
> PAPR guest on a 32-bit Book 3S host won't work very well, unless I am
> missing something...
I agree that it doesn't make sense, but if anything we should restrict it to=
64-bit _guests_. you can also run 32-bit guests on 64-bit hosts.
And so far, we don't have a single interface setting PVR and PAPR mode at th=
e same time, so you could still enable PAPR with a 64-bit guest CPU and then=
switch to a 32-bit CPU.
It'd be a nightmare to check all configurations on every setter function.
Unless...
We could introduce a sanity check function that gets executed every time we c=
hange PVR or enable PAPR. That could set a variable in the vcpu struct to in=
dicate that the config is ok. We could then check that on vcpu_run.
Alex
>=20
^ permalink raw reply
* [PATCH 3/3] powerpc: Coding style cleanups
From: Anton Blanchard @ 2011-08-10 6:49 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20110810064937.424887717@samba.org>
While converting code to use for_each_node_by_type I noticed a
number of coding style issues.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-powerpc/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/setup_64.c 2011-08-10 16:19:00.629356097 +1000
+++ linux-powerpc/arch/powerpc/kernel/setup_64.c 2011-08-10 16:21:31.642031299 +1000
@@ -281,11 +281,11 @@ static void __init initialize_cache_info
for_each_node_by_type(np, "cpu") {
num_cpus += 1;
- /* We're assuming *all* of the CPUs have the same
+ /*
+ * We're assuming *all* of the CPUs have the same
* d-cache and i-cache sizes... -Peter
*/
-
- if ( num_cpus == 1 ) {
+ if (num_cpus == 1) {
const u32 *sizep, *lsizep;
u32 size, lsize;
@@ -294,10 +294,13 @@ static void __init initialize_cache_info
sizep = of_get_property(np, "d-cache-size", NULL);
if (sizep != NULL)
size = *sizep;
- lsizep = of_get_property(np, "d-cache-block-size", NULL);
+ lsizep = of_get_property(np, "d-cache-block-size",
+ NULL);
/* fallback if block size missing */
if (lsizep == NULL)
- lsizep = of_get_property(np, "d-cache-line-size", NULL);
+ lsizep = of_get_property(np,
+ "d-cache-line-size",
+ NULL);
if (lsizep != NULL)
lsize = *lsizep;
if (sizep == 0 || lsizep == 0)
@@ -314,9 +317,12 @@ static void __init initialize_cache_info
sizep = of_get_property(np, "i-cache-size", NULL);
if (sizep != NULL)
size = *sizep;
- lsizep = of_get_property(np, "i-cache-block-size", NULL);
+ lsizep = of_get_property(np, "i-cache-block-size",
+ NULL);
if (lsizep == NULL)
- lsizep = of_get_property(np, "i-cache-line-size", NULL);
+ lsizep = of_get_property(np,
+ "i-cache-line-size",
+ NULL);
if (lsizep != NULL)
lsize = *lsizep;
if (sizep == 0 || lsizep == 0)
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-08-10 16:19:57.080356036 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10 16:20:46.981240046 +1000
@@ -709,7 +709,6 @@ static void __init parse_drconf_memory(s
static int __init parse_numa_properties(void)
{
- struct device_node *cpu = NULL;
struct device_node *memory;
int default_nid = 0;
unsigned long i;
@@ -732,6 +731,7 @@ static int __init parse_numa_properties(
* each node to be onlined must have NODE_DATA etc backing it.
*/
for_each_present_cpu(i) {
+ struct device_node *cpu;
int nid;
cpu = of_get_cpu_node(i, NULL);
@@ -800,8 +800,9 @@ new_range:
}
/*
- * Now do the same thing for each MEMBLOCK listed in the ibm,dynamic-memory
- * property in the ibm,dynamic-reconfiguration-memory node.
+ * Now do the same thing for each MEMBLOCK listed in the
+ * ibm,dynamic-memory property in the
+ * ibm,dynamic-reconfiguration-memory node.
*/
memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (memory)
^ permalink raw reply
* [PATCH 2/3] powerpc: Use for_each_node_by_type instead of open coding it
From: Anton Blanchard @ 2011-08-10 6:49 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20110810064937.424887717@samba.org>
Use for_each_node_by_type instead of open coding it.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-powerpc/arch/powerpc/kernel/machine_kexec_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/machine_kexec_64.c 2011-08-10 16:17:52.508167607 +1000
+++ linux-powerpc/arch/powerpc/kernel/machine_kexec_64.c 2011-08-10 16:18:03.000000000 +1000
@@ -74,8 +74,7 @@ int default_machine_kexec_prepare(struct
}
/* We also should not overwrite the tce tables */
- for (node = of_find_node_by_type(NULL, "pci"); node != NULL;
- node = of_find_node_by_type(node, "pci")) {
+ for_each_node_by_type(node, "pci") {
basep = of_get_property(node, "linux,tce-base", NULL);
sizep = of_get_property(node, "linux,tce-size", NULL);
if (basep == NULL || sizep == NULL)
Index: linux-powerpc/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/setup_64.c 2011-08-10 16:17:52.518167781 +1000
+++ linux-powerpc/arch/powerpc/kernel/setup_64.c 2011-08-10 16:19:00.629356097 +1000
@@ -278,7 +278,7 @@ static void __init initialize_cache_info
DBG(" -> initialize_cache_info()\n");
- for (np = NULL; (np = of_find_node_by_type(np, "cpu"));) {
+ for_each_node_by_type(np, "cpu") {
num_cpus += 1;
/* We're assuming *all* of the CPUs have the same
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-08-10 16:17:52.508167607 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10 16:19:57.080356036 +1000
@@ -710,7 +710,7 @@ static void __init parse_drconf_memory(s
static int __init parse_numa_properties(void)
{
struct device_node *cpu = NULL;
- struct device_node *memory = NULL;
+ struct device_node *memory;
int default_nid = 0;
unsigned long i;
@@ -750,8 +750,8 @@ static int __init parse_numa_properties(
}
get_n_mem_cells(&n_mem_addr_cells, &n_mem_size_cells);
- memory = NULL;
- while ((memory = of_find_node_by_type(memory, "memory")) != NULL) {
+
+ for_each_node_by_type(memory, "memory") {
unsigned long start;
unsigned long size;
int nid;
@@ -1187,10 +1187,10 @@ static int hot_add_drconf_scn_to_nid(str
*/
int hot_add_node_scn_to_nid(unsigned long scn_addr)
{
- struct device_node *memory = NULL;
+ struct device_node *memory;
int nid = -1;
- while ((memory = of_find_node_by_type(memory, "memory")) != NULL) {
+ for_each_node_by_type(memory, "memory") {
unsigned long start, size;
int ranges;
const unsigned int *memcell_buf;
^ permalink raw reply
* [PATCH 1/3] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
From: Anton Blanchard @ 2011-08-10 6:49 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
During memory hotplug testing, I got the following warning:
ERROR: Bad of_node_put() on /memory@0
of_node_release
kref_put
of_node_put
of_find_node_by_type
hot_add_node_scn_to_nid
hot_add_scn_to_nid
memory_add_physaddr_to_nid
...
of_find_node_by_type() loop does of_node_put for us so remove the
duplicate one inside the loop.
Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@kernel.org
---
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-06-06 08:07:35.148708089 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10 11:31:59.723379868 +1000
@@ -1214,7 +1214,6 @@ int hot_add_node_scn_to_nid(unsigned lon
break;
}
- of_node_put(memory);
if (nid >= 0)
break;
}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox