* Re: [BUG]2.6.25-rc6:Unable to handle kernel paging request
From: Paul Mackerras @ 2008-03-21 9:07 UTC (permalink / raw)
To: skumar; +Cc: Poornima Nayak, linuxppc-dev, akpm, linux-kernel
In-Reply-To: <20080320184216.GB32259@in.ibm.com>
Sudhir Kumar writes:
> > I suggest you turn off CONFIG_SENSORS_F71805F.
> It was a new feature so I turned it on while compiling.
> I have tried by turning off CONFIG_SENSORS_F71805F but the bug
> is still present.
Do you mean that you still saw a crash in f71805f_find? If so, then
you have some problem with the way you rebuilt the kernel, or you
didn't boot the kernel you thought you did, or possibly that
CONFIG_SENSORS_F71805F is getting turned back on by a select statement
somewhere in some Kconfig file.
If you really turned CONFIG_SENSORS_F71805F off then there would be no
f71805f_find function in the kernel.
Paul.
^ permalink raw reply
* Re: crash in init_ipic_sysfs on efika
From: Paul Mackerras @ 2008-03-21 9:03 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, Olaf Hering
In-Reply-To: <fa686aa40803202214k7d8596a5y922a34f33d33dca2@mail.gmail.com>
Grant Likely writes:
> Confirmed, this patch fixes the problem. Paulus or Kumar, can you
> please pick it up for .25?
Sure, will do. I thought about putting it in the last batch but I
wanted an ack from you.
Anyone else got any last-minute things for 2.6.25?
Paul.
^ permalink raw reply
* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Benjamin Herrenschmidt @ 2008-03-21 9:00 UTC (permalink / raw)
To: Stefan Roese; +Cc: linuxppc-dev
In-Reply-To: <1205943310-26323-1-git-send-email-sr@denx.de>
On Wed, 2008-03-19 at 17:15 +0100, Stefan Roese wrote:
> + cpu@0 {
> + device_type = "cpu";
> + model = "PowerPC,460GT";
> + reg = <0>;
>
I wonder if we should do something here to differenciate the SoC chip
from the core. After all, all those 4xx mostly have the same core (there
are 2 or 3 revisions of the core maybe ...) but they tend to have all
different PVR which is a pain and won't scale...
Maybe AMCC could do something in HW (splitting the PVR from whatever
indicates what "chip" it is, and keeping the PVR purely for the core
rev) but I'm wondering if we should also do something in the DTS..
Stefan can you talk to your AMCC contacts about this ?
As for the DTS, maybe a "compatible" property in the CPU might make some
sense with a content along the lines of "ppc440x6" or whatever rev of
the 440 core it is.
What do you think ?
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH] Add Fixed PHY support for ucc_geth
From: Li Yang @ 2008-03-21 8:51 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Netdev, Joakim Tjernlund, Linuxppc-Embedded@Ozlabs.Org
In-Reply-To: <1205833615-15833-1-git-send-email-Joakim.Tjernlund@transmode.se>
> -----Original Message-----
> From: Joakim Tjernlund [mailto:Joakim.Tjernlund@transmode.se]=20
> Sent: Tuesday, March 18, 2008 5:47 PM
> To: Netdev; Li Yang; Linuxppc-Embedded@Ozlabs.Org
> Cc: Joakim Tjernlund
> Subject: [PATCH] Add Fixed PHY support for ucc_geth
>=20
> The new Fixed PHY method, fixed-link property, isn't
> impl. for ucc_geth which makes fixed PHYs non functional.
> Add support for the new method to restore the Fixed PHY
> functionality.
>=20
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Signed-off-by: Li Yang <leoli@freescale.com>
---
Ps: This patch depends on the patch "ucc_geth: use correct thread number
for 10/100Mbps link" to apply, which hasn't made to Linus' tree for now
but has already been in Jeff and David's trees.
> ---
>=20
> This is a regression as fixed PHYs works in 2.6.23 and I am
> using it.
>=20
> drivers/net/ucc_geth.c | 53=20
> +++++++++++++++++++++++++++--------------------
> 1 files changed, 30 insertions(+), 23 deletions(-)
>=20
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index ecc5712..18c8b39 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -3836,6 +3836,7 @@ static int ucc_geth_probe(struct=20
> of_device* ofdev, const struct of_device_id *ma
> struct device_node *phy;
> int err, ucc_num, max_speed =3D 0;
> const phandle *ph;
> + const u32 *fixed_link;
> const unsigned int *prop;
> const char *sprop;
> const void *mac_addr;
> @@ -3926,18 +3927,38 @@ static int ucc_geth_probe(struct=20
> of_device* ofdev, const struct of_device_id *ma
> =20
> ug_info->uf_info.regs =3D res.start;
> ug_info->uf_info.irq =3D irq_of_parse_and_map(np, 0);
> + fixed_link =3D of_get_property(np, "fixed-link", NULL);
> + if (fixed_link) {
> + ug_info->mdio_bus =3D 0;
> + ug_info->phy_address =3D fixed_link[0];
> + phy =3D NULL;
> + } else {
> + ph =3D of_get_property(np, "phy-handle", NULL);
> + phy =3D of_find_node_by_phandle(*ph);
> =20
> - ph =3D of_get_property(np, "phy-handle", NULL);
> - phy =3D of_find_node_by_phandle(*ph);
> + if (phy =3D=3D NULL)
> + return -ENODEV;
> =20
> - if (phy =3D=3D NULL)
> - return -ENODEV;
> + /* set the PHY address */
> + prop =3D of_get_property(phy, "reg", NULL);
> + if (prop =3D=3D NULL)
> + return -1;
> + ug_info->phy_address =3D *prop;
> +
> + /* Set the bus id */
> + mdio =3D of_get_parent(phy);
> +
> + if (mdio =3D=3D NULL)
> + return -1;
> =20
> - /* set the PHY address */
> - prop =3D of_get_property(phy, "reg", NULL);
> - if (prop =3D=3D NULL)
> - return -1;
> - ug_info->phy_address =3D *prop;
> + err =3D of_address_to_resource(mdio, 0, &res);
> + of_node_put(mdio);
> +
> + if (err)
> + return -1;
> +
> + ug_info->mdio_bus =3D res.start;
> + }
> =20
> /* get the phy interface type, or default to MII */
> prop =3D of_get_property(np, "phy-connection-type", NULL);
> @@ -3982,20 +4003,6 @@ static int ucc_geth_probe(struct=20
> of_device* ofdev, const struct of_device_id *ma
> ug_info->numThreadsRx =3D UCC_GETH_NUM_OF_THREADS_4;
> }
> =20
> - /* Set the bus id */
> - mdio =3D of_get_parent(phy);
> -
> - if (mdio =3D=3D NULL)
> - return -1;
> -
> - err =3D of_address_to_resource(mdio, 0, &res);
> - of_node_put(mdio);
> -
> - if (err)
> - return -1;
> -
> - ug_info->mdio_bus =3D res.start;
> -
> if (netif_msg_probe(&debug))
> printk(KERN_INFO "ucc_geth: UCC%1d at 0x%8x=20
> (irq =3D %d) \n",
> ug_info->uf_info.ucc_num + 1,=20
> ug_info->uf_info.regs,
> --=20
> 1.5.4.3
>=20
>=20
^ permalink raw reply
* Re: DTS question
From: David Gibson @ 2008-03-21 7:05 UTC (permalink / raw)
To: Sean MacLennan; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20080321010941.5e4bf9d4@lappy.seanm.ca>
On Fri, Mar 21, 2008 at 01:09:41AM -0400, Sean MacLennan wrote:
> On Thu, 20 Mar 2008 22:34:21 -0600
> "Grant Likely" <grant.likely@secretlab.ca> wrote:
>
> > Convention is to use the stock ticker symbol. If the company is
> > private and has no stock ticker symbol, then the company name should
> > be used.
>
> I didn't know that. ADI it is then.
Well.. stock ticker is the new convention. IEEE1275 used IEEE
assigned OUI strings (Organization Unique Identifiers). Often those
are the same as the stock ticker, but not always.
Stock ticker is a good choice for new things, but for anything from a
vendor which has existing 1275 bindings for its products, I think we
should keep the original assigned OUI, even if it differs from the
stock ticker.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: Oops with TQM5200 on TQM5200
From: Wolfgang Grandegger @ 2008-03-21 6:30 UTC (permalink / raw)
To: Bartlomiej Sieka; +Cc: linuxppc-dev, Anatolij Gustschin, Paul Mackerras
In-Reply-To: <47E2FC3E.1020705@semihalf.com>
Bartlomiej Sieka wrote:
> Grant Likely wrote:
>> Paul, my git server is down at the moment. Can you please pick this
>> one up for .25? It is needed to boot the tqm5200 board.
>
> Could we also have similar fixes for cm5200.dts and motionpro.dts picked
> up? I could provide the patches within a day -- will this be OK?
At that occasion, could you please also add two CAN nodes to tqm5200.dts
as shown:
s$ diff -u tqm5200.dts tqm5200.dts.CAN
--- tqm5200.dts 2008-03-20 18:44:40.000000000 +0100
+++ tqm5200.dts.CAN 2008-03-21 07:27:49.000000000 +0100
@@ -68,6 +68,22 @@
fsl,has-wdt;
};
+ can@900 {
+ compatible = "fsl,mpc5200-mscan";
+ cell-index = <0>;
+ interrupts = <2 11 0>;
+ interrupt-parent = <&mpc5200_pic>;
+ reg = <900 80>;
+ };
+
+ can@980 {
+ compatible = "fsl,mpc5200-mscan";
+ cell-index = <1>;
+ interrupts = <2 12 0>;
+ interrupt-parent = <&mpc5200_pic>;
+ reg = <980 80>;
+ };
+
gpio@b00 {
compatible = "fsl,mpc5200-gpio";
reg = <b00 40>;
Thanks a lot.
Wolfgang.
^ permalink raw reply
* Re: crash in init_ipic_sysfs on efika
From: Grant Likely @ 2008-03-21 5:14 UTC (permalink / raw)
To: Olaf Hering, Paul Mackerras, Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <20080317195305.GA13298@aepfle.de>
On Mon, Mar 17, 2008 at 1:53 PM, Olaf Hering <olaf@aepfle.de> wrote:
>
> I cant reproduce this bug on my board, but:
>
> The global primary_ipic in arch/powerpc/sysdev/ipic.c can remain NULL if
> ipic_init() fails. init_ipic_sysfs() will crash in that case.
>
> Something like this may fix it:
>
> Index: linux-2.6.25-rc6/arch/powerpc/sysdev/ipic.c
> ===================================================================
> --- linux-2.6.25-rc6.orig/arch/powerpc/sysdev/ipic.c
> +++ linux-2.6.25-rc6/arch/powerpc/sysdev/ipic.c
> @@ -906,7 +906,7 @@ static int __init init_ipic_sysfs(void)
> {
> int rc;
>
> - if (!primary_ipic->regs)
> + if (!primary_ipic || !primary_ipic->regs)
> return -ENODEV;
> printk(KERN_DEBUG "Registering ipic with sysfs...\n");
Confirmed, this patch fixes the problem. Paulus or Kumar, can you
please pick it up for .25?
Thanks,
g.
>
>
>
> ok boot eth:192.168.2.52,inst32 inst32 console=ttyPSC0,115200
>
> SuSE Linux zImage starting: loaded at 00040000-00d24cbc (0/0/01003ed8;
> sp: 017ffe80)
> uncompressing ELF header done. (00000100 bytes)
> Allocated 008584d4 bytes for kernel @ 02000000
> Allocated 00a963a2 bytes for initrd @ 02859000
> uncompressing kernel done. (004faeb8 bytes)
> entering kernel at 02010000(2859000/a963a2/01003ed8)
> OF stdout device is: /failsafe
> command line: console=ttyPSC0,115200
> memory layout at init:
> alloc_bottom : 032f0000
> alloc_top : 30000000
> alloc_top_hi : f000c000
> rmo_top : 30000000
> ram_top : f000c000
> Looking for displays
> instantiating rtas at 0x07ffb000 ... done
> Applying EFIKA device tree fixups
> Fixing bestcomm interrupts property
> Adding Ethernet MDIO node
> Adding Ethernet PHY node
> copying OF device tree ...
> Building dt strings...
> Building dt structure...
> Device tree strings 0x032f1000 -> 0x032f17e0
> Device tree struct 0x032f2000 -> 0x03315000
> Calling quiesce ...
> returning from prom_init
> Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
> Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
> Memory: 113264k/131072k available (4808k kernel code, 17648k reserved,
> 164k data, 461k bss, 228k init)
> Security Framework initialized
> AppArmor: AppArmor initialized
> AppArmor: Unable to log event (1505) to audit subsys
> AppArmor: Registered secondary security module name="capability"
> AppArmor: Unable to log event (1505) to audit subsys
> Capability LSM initialized as secondary
> Failure registering Root Plug module with the kernel
> AppArmor: Unable to register %s as a secondary security module
> name="root_plug"
> AppArmor: Unable to log event (1505) to audit subsys
> Failure registering Root Plug module with primary security module.
> Mount-cache hash table entries: 512
> net_namespace: 544 bytes
> NET: Registered protocol family 16
> PCI: Probing PCI hardware
> DMA: MPC52xx BestComm driver
> DMA: MPC52xx BestComm engine @f0001200 ok !
> Unable to handle kernel paging request for data at address 0x00000000
> Faulting instruction address: 0xc0486aac
> Oops: Kernel access of bad area, sig: 11 [#1]
> Efika
> Modules linked in:
> NIP: c0486aac LR: c0479200 CTR: c0486a90
> REGS: c782bea0 TRAP: 0300 Not tainted (2.6.25-rc5-git2-5-default)
> MSR: 00009032 <EE,ME,IR,DR> CR: 44002082 XER: 20000000
> DAR: 00000000, DSISR: 20000000
> TASK = c78237c0[1] 'swapper' THREAD: c782a000
> GPR00: c0479200 c782bf50 c78237c0 ffffffed 00000003 00000000 00000000
> 00000000
> GPR08: fffffffc 00000000 c7800780 000f06a0 22002022 dffffff7 c0405540
> c0405558
> GPR16: c0405568 c040558c c0405594 c04055a4 c04055d0 c0400000 024184a4
> ffffffff
> GPR24: 00000000 c04a6468 c0405550 c782a000 c04e0000 00000000 00000000
> c04af198
> NIP [c0486aac] init_ipic_sysfs+0x1c/0x90
> LR [c0479200] kernel_init+0xf8/0x2a0
> Call Trace:
> [c782bf50] [c04869fc] mpc52xx_bcom_init+0x24/0x34 (unreliable)
> [c782bf60] [c0479200] kernel_init+0xf8/0x2a0
> [c782bff0] [c001330c] kernel_thread+0x44/0x60
> Instruction dump:
> 80010024 bb61000c 38210020 7c0803a6 4e800020 9421fff0 7c0802a6 3d20c04f
> 3860ffed 90010014 93e1000c 81299730 <80090000> 2f800000 41be0058 3c60c041
> ---[ end trace 8640abe69a316dee ]---
> Kernel panic - not syncing: Attempted to kill init!
> Rebooting in 180 seconds..
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: DTS question
From: Sean MacLennan @ 2008-03-21 5:09 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <fa686aa40803202134p7db6a106n8487aa0040b5990c@mail.gmail.com>
On Thu, 20 Mar 2008 22:34:21 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:
> Convention is to use the stock ticker symbol. If the company is
> private and has no stock ticker symbol, then the company name should
> be used.
I didn't know that. ADI it is then.
IIC0: i2c@ef600700 {
compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
reg = <ef600700 14>;
interrupt-parent = <&UIC0>;
interrupts = <2 4>;
index = <0>;
#address-cells = <1>;
#size-cells = <0>;
ad7414@4a {
compatible = "adi,ad7414";
reg = <4a>;
interrupts = <19 8>;
interrupt-parent = <&UIC0>;
};
};
Cheers,
Sean
^ permalink raw reply
* Re: DTS question
From: Grant Likely @ 2008-03-21 4:34 UTC (permalink / raw)
To: Sean MacLennan; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20080321001236.4e37bba4@lappy.seanm.ca>
On Thu, Mar 20, 2008 at 10:12 PM, Sean MacLennan
<smaclennan@pikatech.com> wrote:
> On Thu, 20 Mar 2008 17:26:42 -0500
> "Scott Wood" <scottwood@freescale.com> wrote:
>
> > > ad7414 {
> > > compatible = "ad7414";
> >
> > This should proabably have a manufacturer prefix.
>
> I wondered about this. Since the AD from Analog Devices is built into
> the part number, I didn't know if it was needed. And analog-devices is
> pretty long ;)
>
> But I am willing to put it in if it is necessary.
Convention is to use the stock ticker symbol. If the company is
private and has no stock ticker symbol, then the company name should
be used.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: DTS question
From: Sean MacLennan @ 2008-03-21 4:12 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <47E2E4A2.9000801@freescale.com>
On Thu, 20 Mar 2008 17:26:42 -0500
"Scott Wood" <scottwood@freescale.com> wrote:
> > ad7414 {
> > compatible = "ad7414";
>
> This should proabably have a manufacturer prefix.
I wondered about this. Since the AD from Analog Devices is built into
the part number, I didn't know if it was needed. And analog-devices is
pretty long ;)
But I am willing to put it in if it is necessary.
Cheers,
Sean
^ permalink raw reply
* Re: [PATCH 1/3] [POWERPC] of_serial: Fix possible null dereference.
From: Josh Boyer @ 2008-03-21 1:41 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, John Linn, Arnd Bergmann
In-Reply-To: <fa686aa40803201658g6fbdcab0he06dd1e3c83ca900@mail.gmail.com>
On Thu, 20 Mar 2008 17:58:07 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:
> On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com> wrote:
> > From: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> >
> > The of_serial driver queries the current-speed property and attempts
> > to use it to register the custom_divisor property of the uart_port.
> > However, if current-speed is not set, then this code will dereference
> > a bad pointer. The fix is to only set custom_divisor when a
> > current-speed property appears in the device tree.
> >
> > Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> > Signed-off-by: John Linn <john.linn@xilinx.com>
>
> This looks correct and appropriate to me.
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
Agreed.
Acked-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> Josh/Arnd, could either of you pick this up?
Arnd, can you send this on to Andrew? Or if you are busy I can.
josh
> > ---
> > drivers/serial/of_serial.c | 4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
> > index a64d858..2efb892 100644
> > --- a/drivers/serial/of_serial.c
> > +++ b/drivers/serial/of_serial.c
> > @@ -56,7 +56,9 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev,
> > port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
> > | UPF_FIXED_PORT;
> > port->dev = &ofdev->dev;
> > - port->custom_divisor = *clk / (16 * (*spd));
> > + /* If current-speed was set, then try not to change it. */
> > + if (spd)
> > + port->custom_divisor = *clk / (16 * (*spd));
> >
> > return 0;
> > }
> > --
> > 1.5.2.1
> >
> >
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> >
>
>
>
^ permalink raw reply
* Re: crash in init_ipic_sysfs on efika
From: Paul Mackerras @ 2008-03-21 1:38 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Olaf Hering
In-Reply-To: <63858171-157B-4726-94F2-E64B85A0D8D5@kernel.crashing.org>
Kumar Gala writes:
> We should look and see if we can change init_ipic_sysfs() from a
> subsys_initcall() to just getting called at the end of ipic_init().
If init_ipic is called from init_IRQ then I would expect that's way
too early to be calling sysfs stuff.
Paul.
^ permalink raw reply
* Re: Oops with TQM5200 on TQM5200
From: Grant Likely @ 2008-03-21 0:20 UTC (permalink / raw)
To: Bartlomiej Sieka; +Cc: linuxppc-dev, Anatolij Gustschin, Paul Mackerras
In-Reply-To: <47E2FC3E.1020705@semihalf.com>
On Thu, Mar 20, 2008 at 6:07 PM, Bartlomiej Sieka <tur@semihalf.com> wrote:
> Grant Likely wrote:
> > Paul, my git server is down at the moment. Can you please pick this
> > one up for .25? It is needed to boot the tqm5200 board.
>
> Could we also have similar fixes for cm5200.dts and motionpro.dts picked
> up? I could provide the patches within a day -- will this be OK?
Yes, please do; heck put them all into one patch if you like; and I'll ack them.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Grant Likely @ 2008-03-21 0:19 UTC (permalink / raw)
To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <20080320144402.3063517C005D@mail148-sin.bigfish.com>
On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com> wrote:
> The Xilinx 16550 uart core is not a standard 16550, because it uses
> word-based addressing rather than byte-based addressing. As a result,
> it is not compatible with the open firmware 'ns16550' compatible
> binding. This code introduces new bindings, which pass the correct
> register base and regshift properties to the uart driver to enable
> this core to be used. Doing this cleanly required some refactoring of
> the existing code.
Personally, I'm not fond of this approach. There is already some
traction to using the reg-shift property to specify spacing, and I
think it would be appropriate to also define a reg-offset property to
handle the +3 offset and then let the xilinx 16550 nodes use those.
More comments below.
>
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
> drivers/serial/of_serial.c | 47 ++++++++++++++++++++++++++++++-------------
> 1 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
> index 2efb892..910c94f 100644
> --- a/drivers/serial/of_serial.c
> +++ b/drivers/serial/of_serial.c
> @@ -20,13 +20,15 @@
> struct of_serial_info {
> int type;
> int line;
> + int regshift;
> + int regoffset;
> };
>
> /*
> * Fill a struct uart_port for a given device node
> */
> static int __devinit of_platform_serial_setup(struct of_device *ofdev,
> - int type, struct uart_port *port)
> + struct of_serial_info *info, struct uart_port *port)
> {
> struct resource resource;
> struct device_node *np = ofdev->node;
> @@ -48,17 +50,16 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev,
> }
>
> spin_lock_init(&port->lock);
> - port->mapbase = resource.start;
> + port->mapbase = resource.start + info->regoffset;
> port->irq = irq_of_parse_and_map(np, 0);
> port->iotype = UPIO_MEM;
> - port->type = type;
> + port->type = info->type;
> + port->regshift = info->regshift;
> port->uartclk = *clk;
> port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
> | UPF_FIXED_PORT;
> port->dev = &ofdev->dev;
> - /* If current-speed was set, then try not to change it. */
> - if (spd)
> - port->custom_divisor = *clk / (16 * (*spd));
> + port->custom_divisor = *clk / (16 * (*spd));
Oops, looks like you're undoing your previous patch here.
>
> return 0;
> }
> @@ -81,8 +82,9 @@ static int __devinit of_platform_serial_probe(struct of_device *ofdev,
> if (info == NULL)
> return -ENOMEM;
>
> - port_type = (unsigned long)id->data;
> - ret = of_platform_serial_setup(ofdev, port_type, &port);
> + memcpy(info, id->data, sizeof(struct of_serial_info));
> + port_type = info->type;
> + ret = of_platform_serial_setup(ofdev, info, &port);
> if (ret)
> goto out;
>
> @@ -100,7 +102,6 @@ static int __devinit of_platform_serial_probe(struct of_device *ofdev,
> if (ret < 0)
> goto out;
>
> - info->type = port_type;
> info->line = ret;
> ofdev->dev.driver_data = info;
> return 0;
> @@ -128,15 +129,33 @@ static int of_platform_serial_remove(struct of_device *ofdev)
> return 0;
> }
>
> +static struct of_serial_info __devinitdata ns8250_info = { .type = PORT_8250 };
> +static struct of_serial_info __devinitdata ns16450_info = { .type = PORT_16450 };
> +static struct of_serial_info __devinitdata ns16550_info = { .type = PORT_16550 };
> +static struct of_serial_info __devinitdata ns16750_info = { .type = PORT_16750 };
> +static struct of_serial_info __devinitdata xilinx_16550_info = {
> + .type = PORT_16550,
> + .regshift = 2,
> + .regoffset = 3,
> +};
> +static struct of_serial_info __devinitdata unknown_info = { .type = PORT_UNKNOWN };
In support of my argument; the fact that you need a table of data says
to me that this data should really be encoded in the device tree. :-)
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Oops with TQM5200 on TQM5200
From: Bartlomiej Sieka @ 2008-03-21 0:07 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Anatolij Gustschin
In-Reply-To: <fa686aa40803201650t289cd800gfda389b192c1834c@mail.gmail.com>
Grant Likely wrote:
> Paul, my git server is down at the moment. Can you please pick this
> one up for .25? It is needed to boot the tqm5200 board.
Could we also have similar fixes for cm5200.dts and motionpro.dts picked
up? I could provide the patches within a day -- will this be OK?
Regards,
Bartlomiej
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> Cheers,
> g.
>
> On Thu, Mar 20, 2008 at 9:24 AM, Anatolij Gustschin <agust@denx.de> wrote:
>> Hello Wolfgang,
>>
>>
>> Wolfgang Grandegger wrote:
>>
>> > I just tried Linux 2.6.25-rc6 on my TQM5200 module and got the attached
>> > oops. Are there any known patches fixing the problems?
>>
>> try the patch below for tqm5200.dts, rebuild dtb and boot
>> again. Not sure if it works for Linux 2.6.25-rc6, but for
>> 2.6.25-rc3 it does.
>>
>> Anatolij
>> --
>> diff --git a/arch/powerpc/boot/dts/tqm5200.dts b/arch/powerpc/boot/dts/tqm5200.dts
>> index c86464f..7c23bb3 100644
>> --- a/arch/powerpc/boot/dts/tqm5200.dts
>> +++ b/arch/powerpc/boot/dts/tqm5200.dts
>> @@ -83,6 +83,7 @@
>> };
>>
>> dma-controller@1200 {
>> + device_type = "dma-controller";
>> compatible = "fsl,mpc5200-bestcomm";
>> reg = <1200 80>;
>> interrupts = <3 0 0 3 1 0 3 2 0 3 3 0
>> @@ -127,10 +128,25 @@
>> ethernet@3000 {
>> device_type = "network";
>> compatible = "fsl,mpc5200-fec";
>> - reg = <3000 800>;
>> + reg = <3000 400>;
>> local-mac-address = [ 00 00 00 00 00 00 ];
>> interrupts = <2 5 0>;
>> interrupt-parent = <&mpc5200_pic>;
>> + phy-handle = <&phy0>;
>> + };
>> +
>> + mdio@3000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "fsl,mpc5200b-mdio";
>> + reg = <3000 400>; // fec range, since we need to setup fec interrupts
>> + interrupts = <2 5 0>; // these are for "mii command finished", not link changes & co.
>> + interrupt-parent = <&mpc5200_pic>;
>> +
>> + phy0:ethernet-phy@0 {
>> + device_type = "ethernet-phy";
>> + reg = <0>;
>> + };
>> };
>>
>> ata@3a00 {
^ permalink raw reply
* Re: [PATCH v2] Make 83xx perfmon support selectable
From: Jerry Van Baren @ 2008-03-20 23:59 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev list, Phillips Kim, Paul Mackerras
In-Reply-To: <E3C4695C-A2A9-4657-B771-97DFDD3F6ACE@kernel.crashing.org>
Kumar Gala wrote:
>
> On Mar 18, 2008, at 12:05 PM, Scott Wood wrote:
>
>> On Fri, Mar 07, 2008 at 05:59:03PM -0600, Andy Fleming wrote:
>>> Not all e300 cores support the performance monitors, and the ones
>>> that don't will be confused by the mf/mtpmr instructions. This
>>> allows the support to be optional, so the 8349 can turn it off
>>> while the 8379 can turn it on. Sadly, those aren't config options,
>>> so it will be left to the defconfigs and the users to make that
>>> determination.
>>
>> So does this mean we can't do multiplatform of something with perfmon and
>> something without perfmon? Seems like this should come from the device
>> tree, or PVR, or some other runtime check.
>
> It possible if your binutils supports generating the instructions. I
> believe Kim was going to look at doing a patch to use a #define
> MFPMR(x)/#define MTPMR() so we don't have to worry about toolchain
> versions.
>
> - k
Hi Kumar, Scott,
Per Andy (and my limited reading of the UMs), some 83xx have the PMR
registers and some don't. The compiler either supports the PMR register
or it doesn't. If you make it runtime configurable, people running CPUs
that don't support the specific PMR will have to change compiler
configurations in order to compile with the PMR register in there (could
have unintended consequences).
Also, if you look at the code (arch/powerpc/kernel/pmc.c), there are
several different types of PMR registers, based on the core and flavor
of the core. Finding or making a compiler setup that supports all of
the possible PMR registers could be a problem.
You would still have to make the PMR read/write runtime selectable
because the CPUs that don't support that register will raise an
exception IIRC (an Really Bad Thing[tm]).
It crossed my mind /very briefly/ to use the FDT to select this but was
not willing to put that much effort into it.
* Toolchain issues (first & second objection, above) - non-trivial
* Would have to read the enable/disable into a static (global?) variable
to minimize the time impact for the runtime decision - ugh
* Need to add yet another definition to the booting-without-of.txt
(bwoof) and appropriate .dts files - OK, I'm lazy
Best regards,
gvb
^ permalink raw reply
* Re: [PATCH 1/3] [POWERPC] of_serial: Fix possible null dereference.
From: Grant Likely @ 2008-03-20 23:58 UTC (permalink / raw)
To: John Linn, Josh Boyer, Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <20080320144402.6A0E3518070@mail63-sin.bigfish.com>
On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com> wrote:
> From: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
>
> The of_serial driver queries the current-speed property and attempts
> to use it to register the custom_divisor property of the uart_port.
> However, if current-speed is not set, then this code will dereference
> a bad pointer. The fix is to only set custom_divisor when a
> current-speed property appears in the device tree.
>
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> Signed-off-by: John Linn <john.linn@xilinx.com>
This looks correct and appropriate to me.
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Josh/Arnd, could either of you pick this up?
Cheers,
g.
> ---
> drivers/serial/of_serial.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
> index a64d858..2efb892 100644
> --- a/drivers/serial/of_serial.c
> +++ b/drivers/serial/of_serial.c
> @@ -56,7 +56,9 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev,
> port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
> | UPF_FIXED_PORT;
> port->dev = &ofdev->dev;
> - port->custom_divisor = *clk / (16 * (*spd));
> + /* If current-speed was set, then try not to change it. */
> + if (spd)
> + port->custom_divisor = *clk / (16 * (*spd));
>
> return 0;
> }
> --
> 1.5.2.1
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Oops with TQM5200 on TQM5200
From: Grant Likely @ 2008-03-20 23:50 UTC (permalink / raw)
To: Anatolij Gustschin, Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <47E281B6.20702@denx.de>
Paul, my git server is down at the moment. Can you please pick this
one up for .25? It is needed to boot the tqm5200 board.
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Cheers,
g.
On Thu, Mar 20, 2008 at 9:24 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Hello Wolfgang,
>
>
> Wolfgang Grandegger wrote:
>
> > I just tried Linux 2.6.25-rc6 on my TQM5200 module and got the attached
> > oops. Are there any known patches fixing the problems?
>
> try the patch below for tqm5200.dts, rebuild dtb and boot
> again. Not sure if it works for Linux 2.6.25-rc6, but for
> 2.6.25-rc3 it does.
>
> Anatolij
> --
> diff --git a/arch/powerpc/boot/dts/tqm5200.dts b/arch/powerpc/boot/dts/tqm5200.dts
> index c86464f..7c23bb3 100644
> --- a/arch/powerpc/boot/dts/tqm5200.dts
> +++ b/arch/powerpc/boot/dts/tqm5200.dts
> @@ -83,6 +83,7 @@
> };
>
> dma-controller@1200 {
> + device_type = "dma-controller";
> compatible = "fsl,mpc5200-bestcomm";
> reg = <1200 80>;
> interrupts = <3 0 0 3 1 0 3 2 0 3 3 0
> @@ -127,10 +128,25 @@
> ethernet@3000 {
> device_type = "network";
> compatible = "fsl,mpc5200-fec";
> - reg = <3000 800>;
> + reg = <3000 400>;
> local-mac-address = [ 00 00 00 00 00 00 ];
> interrupts = <2 5 0>;
> interrupt-parent = <&mpc5200_pic>;
> + phy-handle = <&phy0>;
> + };
> +
> + mdio@3000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,mpc5200b-mdio";
> + reg = <3000 400>; // fec range, since we need to setup fec interrupts
> + interrupts = <2 5 0>; // these are for "mii command finished", not link changes & co.
> + interrupt-parent = <&mpc5200_pic>;
> +
> + phy0:ethernet-phy@0 {
> + device_type = "ethernet-phy";
> + reg = <0>;
> + };
> };
>
> ata@3a00 {
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/2 v2] Driver for Freescale 8610 and 5121 DIU
From: Peter Zijlstra @ 2008-03-20 23:02 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-fbdev-devel, linux-kernel, York Sun, linuxppc-dev,
Ingo Molnar, timur
In-Reply-To: <20080320152708.23c6c734.akpm@linux-foundation.org>
On Thu, 2008-03-20 at 15:27 -0700, Andrew Morton wrote:
> >
> > +static struct diu_hw dr = {
> > + .mode = MFB_MODE1,
> > + .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> > +};
>
> I'm not clear on what's supposed to happen with __SPIN_LOCK_UNLOCKED(). I
> do know that its documentation is crap.
>
> I guess you should have used SPIN_LOCK_UNLOCKED there rather than
> open-coding its equivalent. And SPIN_LOCK_UNLOCKED _is_ documented. It
> says "don't use this".
>
> Now, s/documentation/guesswork-and-grep/ says that you're supposed to pick
> a tree-wide-unique string here as your lockdep key.
>
> So I did this:
>
> --- a/drivers/video/fsl-diu-fb.c~fbdev-driver-for-freescale-8610-and-5121-diu-fix
> +++ a/drivers/video/fsl-diu-fb.c
> @@ -274,7 +274,7 @@ static struct mfb_info mfb_template[] =
>
> static struct diu_hw dr = {
> .mode = MFB_MODE1,
> - .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> + .reg_lock = __SPIN_LOCK_UNLOCKED(diu_hw.reg_lock),
> };
#define __SPIN_LOCK_UNLOCKED(lockname)
seems pretty suggestive to me, also DEFINE_SPINLOCK(x) shows the proper
usage; the right thing would have been:
static struct diu_hw dr = {
.mode = MFB_MODE1,
- .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
+ .reg_lock = __SPIN_LOCK_UNLOCKED(dr.reg_lock),
};
Where 'dr.reg_lock' is the expression to locate the actual object.
Does something like this make sense to you:
---
Subject: lockdep: document __SPIN_LOCK_UNLOCKED() usage
Small comment clarifying the intended usage of __SPIN_LOCK_UNLOCKED()
[ for the observant readers; yes this prescribes a usage that is more
than strictly needed, it does however enable interesting future uses ]
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
diff --git a/include/linux/spinlock_types.h b/include/linux/spinlock_types.h
index 68d88f7..4278558 100644
--- a/include/linux/spinlock_types.h
+++ b/include/linux/spinlock_types.h
@@ -63,6 +63,10 @@ typedef struct {
# define RW_DEP_MAP_INIT(lockname)
#endif
+/*
+ * __*_LOCK_UNLOCKED(lockname), where 'lockname' is a valid C expression
+ * that evaluates to the actual object being initialized.
+ */
#ifdef CONFIG_DEBUG_SPINLOCK
# define __SPIN_LOCK_UNLOCKED(lockname) \
(spinlock_t) { .raw_lock = __RAW_SPIN_LOCK_UNLOCKED, \
---
Perhaps I should make the macros do something like:
typecheck(spinlock_t, lockname)
And sweep the tree to make it compile again.
> static struct diu_pool pool;
>
> > +static struct diu_pool pool;
> > +
> > +/* To allocate memory for framebuffer. First try __get_free_pages(). If it
> > + * fails, try rh_alloc. The reason is __get_free_pages() cannot allocate
> > + * very large memory (more than 4MB). We don't want to allocate all memory
> > + * in rheap since small memory allocation/deallocation will fragment the
> > + * rheap and make the furture large allocation fail.
> > + */
> > +
> > +void *fsl_diu_alloc(unsigned long size, phys_addr_t *phys)
> > +{
> > + void *virt;
> > +
> > + pr_debug("size=%lu\n", size);
> > +
> > + virt = (void *)__get_free_pages(GFP_DMA | GFP_ATOMIC, get_order(size));
>
> GFP_DMA implies GFP_ATOMIC, but it's appropriate for documentation purposes.
FWIW, I prefer the form: GFP_type | __GFP_modifiers
For instance: 'GFP_KERNEL | __GFP_DMA | __GFP_ZERO'
^ permalink raw reply related
* Re: muram in device tree for mpc8250 in arch/powerpc
From: James Black @ 2008-03-20 23:02 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-embedded
In-Reply-To: <47E2AADD.6090101@freescale.com>
On Thu, Mar 20, 2008 at 12:20 PM, Scott Wood <scottwood@freescale.com> wrote:
> James Black wrote:
> > Zone PFN ranges:
> > DMA 0 -> 16384
> > Normal 16384 -> 16384
> > Movable zone start PFN for each node
> > early_node_map[1] active PFN ranges
> > 0: 0 -> 16384
> > Unable to handle kernel paging request for data at address 0xe001a000
> > Faulting instruction address: 0xc00e1a6c
>
> What function is 0xc00e1a6c in?
Drilled down into the source and find that mm/bootmem.c:line 232
: find_next_zero_bit()
may be the culprit. When I put printks in, the line moves around a bit.
JB
> Is it possible that you have an SMC device initialized by your firmware
> that is corrupting parameter RAM?
No other devices have been configured except FCC1, SCC1, SCC3. The
unused gpio pins have been set as outputs.
> > muram@0 {
> > #address-cells = <1>;
> > #size-cells = <1>;
> > ranges = <0 0 10000>;
> >
> > data@0 {
> > compatible = "fsl,cpm-muram-data";
> > reg = <0 4000 8000 1000 B000 1000>;
>
> You can't use all of 0x8000-0x8fff; there is device parameter RAM in
> there. If you can figure out the portions that aren't in use, you can
> use those, but I wouldn't bother unless you really need the extra muram.
So, are you are saying that the muram node is giving the ram to the
kernel and is no longer available for devices to use as buffer
descriptors? I imagined the opposite. I thought I was telling the
kernel where the parameter ram and buffer descriptors live. I am going
to turn on the MCC2 device eventually and will need all of the dpram
for buffer descriptors.
The memory map in my mind right now is
0x0000 - 0x3FFF --> buffer descriptors for SCCx and MCC2.
0x8000 - 0x8FFF --> paramter ram for SCCx and MCC2, et al. (reserved)
0xB000 - 0xB8FF --> FCCx specific (reserved)
In my case, I shouldn't define any of the dual port ram for the kernel?
Jim Black
>
> -Scott
>
^ permalink raw reply
* Re: [PATCH 2/2 v2] Add DIU platform code for MPC8610HPCD
From: Andrew Morton @ 2008-03-20 22:33 UTC (permalink / raw)
To: York Sun; +Cc: linux-fbdev-devel, linux-kernel, linuxppc-dev, yorksun
In-Reply-To: <12059526271964-git-send-email-yorksun@freescale.com>
On Wed, 19 Mar 2008 13:50:27 -0500
York Sun <yorksun@freescale.com> wrote:
> Add platform code to support Freescale DIU. The platform code includes
> framebuffer memory allocation, pixel format, monitor port, etc.
>
> ...
>
> +unsigned int mpc8610hpcd_get_pixel_format
> + (unsigned int bits_per_pixel, int monitor_port)
Again, please do
unsigned int mpc8610hpcd_get_pixel_format(unsigned int bits_per_pixel,
int monitor_port)
(and anywhere else where this was done)
> +int __init preallocate_diu_videomemory(void);
Nope, please don't put extern declarations in .c files. Find a suitable
header for it - one which is included by the defining file and by all users
of the symbol.
Andy, checkpatch missed this.
> }
> #endif
> +
^ permalink raw reply
* Re: [PATCH 1/2 v2] Driver for Freescale 8610 and 5121 DIU
From: Andrew Morton @ 2008-03-20 22:27 UTC (permalink / raw)
To: York Sun
Cc: linux-fbdev-devel, Peter Zijlstra, linux-kernel, yorksun,
linuxppc-dev, timur
In-Reply-To: <12059526274026-git-send-email-yorksun@freescale.com>
On Wed, 19 Mar 2008 13:50:26 -0500
York Sun <yorksun@freescale.com> wrote:
> The following features are supported:
> plane 0 works as a regular frame buffer, can be accessed by /dev/fb0
> plane 1 has two AOIs (area of interest), can be accessed by /dev/fb1 and /dev/fb2
> plane 2 has two AOIs, can be accessed by /dev/fb3 and /dev/fb4
> Special ioctls support AOIs
>
> All /dev/fb* can be used as regular frame buffer devices, except hardware change can
> only be made through /dev/fb0. Changing pixel clock has no effect on other fbs.
>
> Limitation of usage of AOIs:
> AOIs on the same plane can not be horizonally overlapped
> AOIs have horizonal order, i.e. AOI0 should be always on top of AOI1
> AOIs can not beyond phisical display area. Application should check AOI geometry
> before changing physical resolution on /dev/fb0
>
> required command line parameters to preallocate memory for frame buffer
> diufb=15M
>
> optional command line parameters to set modes and monitor
> video=fslfb:[resolution][,bpp][,monitor]
> Syntax:
>
> Resolution
> xres x yres-bpp@refresh_rate, the -bpp and @refresh_rate are optional
> eg, 1024x768, 1280x1024, 1280x1024-32, 1280x1024@60, 1280x1024-32@60, 1280x480-32@60
>
> Bpp
> bpp=32, bpp=24, or bpp=16
>
> Monitor
> monitor=0, monitor=1, monitor=2
> 0 is DVI
> 1 is Single link LVDS
> 2 is Double link LVDS
>
> Note: switching monitor is a board feather, not DIU feather. MPC8610HPCD has three
> monitor ports to swtich to. MPC5121ADS doesn't have additional monitor port. So switching
> monirot port for MPC5121ADS has no effect.
>
> If compiled as a module, it takes pamameters mode, bpp, monitor with the same syntax above.
>
> ...
>
> +static struct diu_hw dr = {
> + .mode = MFB_MODE1,
> + .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> +};
I'm not clear on what's supposed to happen with __SPIN_LOCK_UNLOCKED(). I
do know that its documentation is crap.
I guess you should have used SPIN_LOCK_UNLOCKED there rather than
open-coding its equivalent. And SPIN_LOCK_UNLOCKED _is_ documented. It
says "don't use this".
Now, s/documentation/guesswork-and-grep/ says that you're supposed to pick
a tree-wide-unique string here as your lockdep key.
So I did this:
--- a/drivers/video/fsl-diu-fb.c~fbdev-driver-for-freescale-8610-and-5121-diu-fix
+++ a/drivers/video/fsl-diu-fb.c
@@ -274,7 +274,7 @@ static struct mfb_info mfb_template[] =
static struct diu_hw dr = {
.mode = MFB_MODE1,
- .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
+ .reg_lock = __SPIN_LOCK_UNLOCKED(diu_hw.reg_lock),
};
static struct diu_pool pool;
> +static struct diu_pool pool;
> +
> +/* To allocate memory for framebuffer. First try __get_free_pages(). If it
> + * fails, try rh_alloc. The reason is __get_free_pages() cannot allocate
> + * very large memory (more than 4MB). We don't want to allocate all memory
> + * in rheap since small memory allocation/deallocation will fragment the
> + * rheap and make the furture large allocation fail.
> + */
> +
> +void *fsl_diu_alloc(unsigned long size, phys_addr_t *phys)
> +{
> + void *virt;
> +
> + pr_debug("size=%lu\n", size);
> +
> + virt = (void *)__get_free_pages(GFP_DMA | GFP_ATOMIC, get_order(size));
GFP_DMA implies GFP_ATOMIC, but it's appropriate for documentation purposes.
> + if (virt) {
> + *phys = virt_to_phys(virt);
> + pr_debug("virt %p, phys=%llx\n", virt, (uint64_t) *phys);
> + memset(virt, 0, size);
Could have used __GFP_ZERO, I guess.
> + return virt;
> + }
> + if (!diu_ops.diu_mem) {
> + printk(KERN_INFO "%s: no diu_mem."
> + " To reserve more memory, put 'diufb=15M' "
> + "in the command line\n", __func__);
> + return NULL;
> + }
> +
> + virt = (void *) rh_alloc(&diu_ops.diu_rh_info, size, "DIU");
hm, I'd have expected checkpatch to whine about the space after the cast
there. Whatever.
checkpatch does turn up some significant problems:
WARNING: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
#205: FILE: drivers/video/fsl-diu-fb.c:32:
+#include <asm/uaccess.h>
WARNING: consider using strict_strtoul in preference to simple_strtoul
#1599: FILE: drivers/video/fsl-diu-fb.c:1426:
+ val = simple_strtoul(buf, last, 0);
WARNING: consider using strict_strtoul in preference to simple_strtoul
#1821: FILE: drivers/video/fsl-diu-fb.c:1648:
+ val = simple_strtoul(opt + 8, NULL, 0);
WARNING: consider using strict_strtoul in preference to simple_strtoul
#1825: FILE: drivers/video/fsl-diu-fb.c:1652:
+ default_bpp = simple_strtoul(opt + 4, NULL, 0);
please take a look, and please use checkpatch on all future patches.
> + if (virt) {
> + *phys = virt_to_bus(virt);
> + memset(virt, 0, size);
> + }
> +
> + pr_debug("rh virt=%p phys=%lx\n", virt, *phys);
> +
> + return virt;
> +}
> +
> +void fsl_diu_free(void *p, unsigned long size)
> +{
> + pr_debug("p=%p size=%lu\n", p, size);
> +
> + if (!p)
> + return;
> +
> + if ((p >= diu_ops.diu_mem) &&
> + (p < (diu_ops.diu_mem + diu_ops.diu_size))) {
> + pr_debug("rh\n");
> + rh_free(&diu_ops.diu_rh_info, (unsigned long) p);
> + } else {
> + pr_debug("dma\n");
> + free_pages((unsigned long)p, get_order(size));
> + }
> +}
> +
> +
> +/*
> + * Checks to see if the hardware supports the state requested by var passed
> + * in. This function does not alter the hardware state! If the var passed in
> + * is slightly off by what the hardware can support then we alter the var
> + * PASSED in to what we can do. If the hardware doesn't support mode change
> + * a -EINVAL will be returned by the upper layers.
> + */
> +static int fsl_diu_check_var
> + (struct fb_var_screeninfo *var, struct fb_info *info)
Like this:
static int fsl_diu_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
please.
> +{
> + unsigned long htotal, vtotal;
> + struct mfb_info *pmfbi, *cmfbi, *mfbi = info->par;
> + struct fsl_diu_data *machine_data = mfbi->parent;
> + int index = mfbi->index;
> +
>
> ..
>
> +static void update_lcdc(struct fb_info *info)
> +{
> + struct fb_var_screeninfo *var = &info->var;
> + struct mfb_info *mfbi = info->par;
> + struct fsl_diu_data *machine_data = mfbi->parent;
> + struct diu *hw;
> + int i, j;
> + char __iomem *cursor_base, *gamma_table_base;
> +
> + u32 temp;
> +
> + spin_lock_init(&dr.reg_lock);
we already did that at compile-time?
> + hw = dr.diu_reg;
> +
> + if (mfbi->type == MFB_TYPE_OFF) {
> + fsl_diu_disable_panel(info);
> + return;
> + }
>
> ...
>
> +static void request_irq_local(int irq)
> +{
> + unsigned long status, ints;
> + struct diu *hw;
> +
> + hw = dr.diu_reg;
> +
> + /* Read to clear the status */
> + status = in_be32(&(hw->int_status));
> +
> + if (request_irq(irq, fsl_diu_isr, 0, "diu", 0))
> + pr_info("Request diu IRQ failed.\n");
> + else {
> + ints = INT_PARERR | INT_LS_BF_VS;
> +#if !defined(CONFIG_NOT_COHERENT_CACHE)
> + ints |= INT_VSYNC;
> +#endif
> + if (dr.mode == MFB_MODE2 || dr.mode == MFB_MODE3)
> + ints |= INT_VSYNC_WB;
> +
> + /* Read to clear the status */
> + status = in_be32(&(hw->int_status));
> + out_be32(&(hw->int_mask), ints);
> + }
> +}
The request_irq() return value gets dropped on the floor.
> +static void free_irq_local(int irq)
> +{
> + struct diu *hw = dr.diu_reg;
> +
> + /* Disable all LCDC interrupt */
> + out_be32(&(hw->int_mask), 0x1f);
> +
> + free_irq(irq, 0);
> +}
and the free_irq() will go splat?
> +#ifdef CONFIG_PM
> +/*
> + * Power management hooks. Note that we won't be called from IRQ context,
> + * unlike the blank functions above, so we may sleep.
> + */
> +static int fsl_diu_suspend(struct of_device *dev, pm_message_t state)
> +{
> + struct fsl_diu_data *machine_data;
> +
> + machine_data = dev_get_drvdata(&ofdev->dev);
> + disable_lcdc(machine_data->fsl_diu_info[0]);
> +
> + return 0;
> +}
> +
> +static int fsl_diu_resume(struct of_device *dev)
> +{
> + struct fsl_diu_data *machine_data;
> +
> + machine_data = dev_get_drvdata(&ofdev->dev);
> + enable_lcdc(machine_data->fsl_diu_info[0]);
> +
> + return 0;
> +}
Conventionally we'll do
#else
#define fsl_diu_suspend NULL
#define fsl_diu_resume NULL
here, then remove the ifdefs from fsl_diu_driver.
> +#endif /* CONFIG_PM */
> +
> +/* Align to 64-bit(8-byte), 32-byte, etc. */
> +static int allocate_buf(struct diu_addr *buf, u32 size, u32 bytes_align)
> +{
> + u32 offset, ssize;
> + u32 mask;
> + dma_addr_t paddr = 0;
> +
> + ssize = size + bytes_align;
> + buf->vaddr = dma_alloc_coherent(0, ssize, &paddr, GFP_DMA | GFP_KERNEL);
> + if (!buf->vaddr)
> + return -ENOMEM;
> +
> + buf->paddr = (__u32) paddr;
> + memset(buf->vaddr, 0, ssize);
__GFP_ZERO?
> + mask = bytes_align - 1;
> + offset = (u32)buf->paddr & mask;
> + if (offset) {
> + buf->offset = bytes_align - offset;
> + buf->paddr = (u32)buf->paddr + offset;
> + } else
> + buf->offset = 0;
> + return 0;
> +}
> +
> +static int fsl_diu_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct device_node *np = ofdev->node;
> + struct mfb_info *mfbi;
> + phys_addr_t dummy_ad_addr;
> + int ret, i, error = 0;
> + struct resource res;
> + struct fsl_diu_data *machine_data;
> +
> + machine_data = kzalloc(sizeof(struct fsl_diu_data), GFP_KERNEL);
> + if (!machine_data)
> + return -ENOMEM;
> +
> + for (i = 0; i < sizeof(machine_data->fsl_diu_info) /
> + sizeof(struct fb_info *); i++) {
Please use ARRAY_SIZE() here, and in all the other places which open-code
it.
> + machine_data->fsl_diu_info[i] =
> + framebuffer_alloc(sizeof(struct mfb_info), &ofdev->dev);
> + if (!machine_data->fsl_diu_info[i]) {
> + dev_err(&ofdev->dev, "cannot allocate memory\n");
> + ret = -ENOMEM;
> + goto error2;
> + }
^ permalink raw reply
* Re: DTS question
From: Scott Wood @ 2008-03-20 22:26 UTC (permalink / raw)
To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20080320181926.2ff7e297@lappy.seanm.ca>
Sean MacLennan wrote:
> Ok, here is what I did....
>
> IIC0: i2c@ef600700 {
> compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
> reg = <ef600700 14>;
> interrupt-parent = <&UIC0>;
> interrupts = <2 4>;
> index = <0>;
>
> ad7414 {
> compatible = "ad7414";
This should proabably have a manufacturer prefix.
> interrupts = <19 8>;
> interrupt-parent = <&UIC0>;
> };
Should have a reg property indicating the i2c address, and
#address-cells = <1>/#size-cells = <0> in the i2c controller node.
-Scott
^ permalink raw reply
* Re: DTS question
From: Sean MacLennan @ 2008-03-20 22:19 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20080320173302.7075a1d9@lappy.seanm.ca>
Ok, here is what I did....
IIC0: i2c@ef600700 {
compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
reg = <ef600700 14>;
interrupt-parent = <&UIC0>;
interrupts = <2 4>;
index = <0>;
ad7414 {
compatible = "ad7414";
interrupts = <19 8>;
interrupt-parent = <&UIC0>;
};
};
Comments?
Cheers,
Sean
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] Xilinx: boot support for Xilinx uart 16550.
From: Grant Likely @ 2008-03-20 22:04 UTC (permalink / raw)
To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <20080320144402.695C1518064@mail63-sin.bigfish.com>
On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com> wrote:
> The Xilinx 16550 uart core is not a standard 16550, because it uses
> word-based addressing rather than byte-based addressing. As a result,
> it is not compatible with the open firmware 'ns16550' compatible
> binding.
>
> This code adds the Xilinx uart 16550 to the serial console
> of the boot. A new initialization function for the Xilinx 16550 uart
> is added to the ns16550 driver. It sets up the correct register base
> and regshift properties specific to the Xilinx 16550.
>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
> arch/powerpc/boot/ns16550.c | 24 ++++++++++++++++++++++++
> arch/powerpc/boot/ops.h | 1 +
> arch/powerpc/boot/serial.c | 8 ++++++++
> 3 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/boot/ns16550.c b/arch/powerpc/boot/ns16550.c
> index f8f1b2f..5385255 100644
> --- a/arch/powerpc/boot/ns16550.c
> +++ b/arch/powerpc/boot/ns16550.c
> @@ -77,3 +77,27 @@ int ns16550_console_init(void *devp, struct serial_console_data *scdp)
>
> return 0;
> }
> +
> +int xilinx16550_console_init(void *devp, struct serial_console_data *scdp)
> +{
> + int n;
> + unsigned long reg_phys;
> +
> + n = getprop(devp, "virtual-reg", ®_base, sizeof(reg_base));
> + if (n != sizeof(reg_base)) {
> + if (!dt_xlate_reg(devp, 0, ®_phys, NULL))
> + return -1;
> +
> + reg_base = (void *)reg_phys + 3;
> + }
> +
> + reg_shift = 2;
> +
> + scdp->open = ns16550_open;
> + scdp->putc = ns16550_putc;
> + scdp->getc = ns16550_getc;
> + scdp->tstc = ns16550_tstc;
> + scdp->close = NULL;
> +
> + return 0;
This is mostly duplicated code. xilinx16550_console_init should
simply call ns16550_console_init and then modify reg_base/reg_shift
after it returns.
However, if something like sparse16550 is used, then the need for this
function disappears. In fact, ns16550_console_init already supports
the reg-shift property. It just doesn't know about the 3 byte offset.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ 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