Re: [PATCHv2] powerpc: DBox2 Board Support
On Wednesday 26 December 2007, Jochen Friedrich wrote:
> >> + memory {
> >> + device_type = "memory";
> >> + reg = <0 200>;
> >> + };
> >
> > I thought there are both models with 32MB and 16MB available.
> > If that's true, shouldn't this be filled out by the boot loader?
>
> IIRC, the cuImage wrapper overwrites this with the boot loader
> parameters.
Then maybe set it to zero size in the dts file, and add a comment.
> OTOH, the memory is part of the localbus (CS1). Should it be a child
> of localbus in this case?
No, memory nodes are required to be at the root of the device tree
for historic reasons.
> I didn't check FB_OF. On Dbox2, the avia driver creates a
> framebuffer device.
fb_of needs some properties in the device tree set up correctly,
but is very simple otherwise. If it does all you need, it's
probably a good idea to use that and get rid of the avia framebuffer
driver
> There are two mods available using the block layer, although currently i don't
> support any of them:
>
> 1. using the GPIO lines of the modem port, it's possible to connect a SD card.
>It might be possible to use it using the GPIO SPI driver (but it would need
>some glue code to create a platform device).
>
> 2. using the memory expansion port and CS2, there is an IDE interface
> available
>with a CPLD acting as localbus/IDE bridge. There is a device driver for
>ARCH=ppc.
>
> Unfortunately, i don't own any of these modifications.
If neither of these is in the mainline kernel, it doesn't make sense to
have support for the block layer in defconfig, so just try how much
memory you can free up with this.
> >> +static void __init dbox2_setup_arch(void)
> >> +{
> >> + struct device_node *np;
> >> + static u8 __iomem *config;
> >> +
> >> + cpm_reset();
> >> + init_ioports();
> >> +
> >> + /* Enable external IRQs for AVIA chips */
> >> + clrbits32(&mpc8xx_immr->im_siu_conf.sc_siumcr, 0x0c00);
> >
> > This smells like a hack. What are AVIA chips, and shouldn't
> > their driver enable the IRQs?
>
> No. This just configures the Pin as IRQ input. Maybe, the new GPIO
> functions could be used for this, instead. Then it could be done
> in the driver (the driver should't directly mess with SIU internals).
Maybe it should then be done in the boot wrapper. If the device
tree lists this line as an interrupt, I'd say that is what it should
be.
> >> +static struct of_device_id __initdata of_bus_ids[] = {
> >> + { .name = "soc", },
> >> + { .name = "cpm", },
> >> + { .name = "localbus", },
> >> + {},
> >> +};
> >
> > Shouldn't this check for 'compatible' properties instead of 'name'?
>
> I copied this from mpc885ads_setup.c.
Right, I noticed before that we're rather inconsistent with this.
It would really be good to have more common standards on this.
> > I also once did a patch that let you have a default 'of_bus_ids'
> > member in the ppc_md. I never got around to submitting that, but
> > if there is interest, I can dig it out again.
>
> That would be nice :)
The reason I haven't submitted it yet is that I'm not sure whether
there are cases where we actually _need_ to test for 'name' instead
of 'compatible' because of existing device trees in firmware.
I might just do a patch and see if someone complains about the
idea.
Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2] powerpc: DBox2 Board Support
Hi Arnd,
This patch adds device tree source, default config and setup code for
DBox2 devices.
ia > Cool stuff. I used to have one of these boxes myself, maybe I should
get one again when it's hitting mainline.
Is this already a complete port, or do you also need some device
drivers or boot wrapper code to go along with it?
Currently, it needs the u-boot patches from cvs.tuxbox.org and some
external device drivers (however, they can all be loaded as modules).
The port is complete enough to get a console on the serial port and
a rootfs either on flash or on nfs.
+ memory {
+ device_type = "memory";
+ reg = <0 200>;
+ };
I thought there are both models with 32MB and 16MB available.
If that's true, shouldn't this be filled out by the boot loader?
IIRC, the cuImage wrapper overwrites this with the boot loader
parameters.
OTOH, the memory is part of the localbus (CS1). Should it be a child
of localbus in this case?
+#
+# Frame buffer hardware drivers
+#
+# CONFIG_FB_OF is not set
+# CONFIG_FB_VGA16 is not set
+# CONFIG_FB_S1D13XXX is not set
+# CONFIG_FB_IBM_GXT4500 is not set
+# CONFIG_FB_VIRTUAL is not set
+# CONFIG_BACKLIGHT_LCD_SUPPORT is not set
No hardware framebuffer driver? Can't you use
the FB_OF driver by default? I'd guess that a
set-top box without output is rather pointless ;-)
I didn't check FB_OF. On Dbox2, the avia driver creates a
framebuffer device.
+# DOS/FAT/NT Filesystems
+#
+CONFIG_FAT_FS=y
+CONFIG_MSDOS_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_FAT_DEFAULT_CODEPAGE=437
+CONFIG_FAT_DEFAULT_IOCHARSET="iso8859-1"
+# CONFIG_NTFS_FS is not set
What media can you connect that use the FAT file system?
I'd guess that if you can get rid of these, you can also
disable the entire block layer, which should free up
some kernel memory.
There are two mods available using the block layer, although currently i don't
support any of them:
1. using the GPIO lines of the modem port, it's possible to connect a SD card.
It might be possible to use it using the GPIO SPI driver (but it would need
some glue code to create a platform device).
2. using the memory expansion port and CS2, there is an IDE interface available
with a CPLD acting as localbus/IDE bridge. There is a device driver for
ARCH=ppc.
Unfortunately, i don't own any of these modifications.
+
+/* Vendor information in BR Bootloader
+ */
+
+#define DBOX2_VENDOR_OFFSET(0x1ffe0)
+
+enum dbox2_mid {
+ DBOX2_MID_NOKIA = 1,
+ DBOX2_MID_PHILIPS = 2,
+ DBOX2_MID_SAGEM = 3,
+};
+
+enum dbox2_mid dbox2_get_mid(void);
Can you move this functionality from the kernel to the boot wrapper?
It looks like this is something that really belongs into the device
tree.
OK.
+ if (dbox2_manuf_id == DBOX2_MID_NOKIA)
+ np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
PROTECTED]");
+ else
+ np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
PROTECTED]");
+
+ if (np) {
+ of_detach_node(np);
+ of_node_put(np);
+ }
+
+ if (dbox2_manuf_id == DBOX2_MID_PHILIPS)
+ np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
PROTECTED]");
+ else
+ np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
PROTECTED]");
+
+ if (np) {
+ of_detach_node(np);
+ of_node_put(np);
+ }
+}
What is this code for? Why do you want to detach nodes from the device
tree that have been put in there by the boot loader?
There are small differences between the 3 manufacturers. I guess this
should go to the boot wrapper, as well.
This is even more important if the localbus uses #address-cells = <2>, as
the differen manufacturers set up the address mapping for CS3, CS5 and CS7
differently.
+static void __init dbox2_setup_arch(void)
+{
+ struct device_node *np;
+ static u8 __iomem *config;
+
+ cpm_reset();
+ init_ioports();
+
+ /* Enable external IRQs for AVIA chips */
+ clrbits32(&mpc8xx_immr->im_siu_conf.sc_siumcr, 0x0c00);
This smells like a hack. What are AVIA chips, and shouldn't
their driver enable the IRQs?
No. This just configures the Pin as IRQ input. Maybe, the new GPIO
functions could be used for this, instead. Then it could be done
in the driver (the driver should't directly mess with SIU internals).
+static struct of_device_id __initdata of_bus_ids[] = {
+ { .name = "soc", },
+ { .name = "cpm", },
+ { .name = "localbus", },
+ {},
+};
Shouldn't this check for 'compatible' properties instead of 'name'?
I copied this from mpc885ads_setup.c.
+static int __init declare_of_platform_devices(void)
+{
+ /* Publish the QE devices */
+ if (machine_is(dbox2))
+ of_platform_bus_probe(NULL, of_bus_ids, NULL);
+
+ return 0;
+}
+device_initcall(declare_of_platform_devices);
This is a candidate for the new platform_initcall
Re: [PATCHv2] powerpc: DBox2 Board Support
On Sunday 23 December 2007, Jochen Friedrich wrote:
> This patch adds device tree source, default config and setup code for
> DBox2 devices.
Cool stuff. I used to have one of these boxes myself, maybe I should
get one again when it's hitting mainline.
Is this already a complete port, or do you also need some device
drivers or boot wrapper code to go along with it?
> + memory {
> + device_type = "memory";
> + reg = <0 200>;
> + };
I thought there are both models with 32MB and 16MB available.
If that's true, shouldn't this be filled out by the boot loader?
> +#
> +# Frame buffer hardware drivers
> +#
> +# CONFIG_FB_OF is not set
> +# CONFIG_FB_VGA16 is not set
> +# CONFIG_FB_S1D13XXX is not set
> +# CONFIG_FB_IBM_GXT4500 is not set
> +# CONFIG_FB_VIRTUAL is not set
> +# CONFIG_BACKLIGHT_LCD_SUPPORT is not set
No hardware framebuffer driver? Can't you use
the FB_OF driver by default? I'd guess that a
set-top box without output is rather pointless ;-)
> +# DOS/FAT/NT Filesystems
> +#
> +CONFIG_FAT_FS=y
> +CONFIG_MSDOS_FS=y
> +CONFIG_VFAT_FS=y
> +CONFIG_FAT_DEFAULT_CODEPAGE=437
> +CONFIG_FAT_DEFAULT_IOCHARSET="iso8859-1"
> +# CONFIG_NTFS_FS is not set
What media can you connect that use the FAT file system?
I'd guess that if you can get rid of these, you can also
disable the entire block layer, which should free up
some kernel memory.
> @@ -0,0 +1,30 @@
> +/*
> + * A collection of structures, addresses, and values associated with
> + * the DBox2.
> + *
> + * Author: (c) 2007 Jochen Friedrich
> + *
> + * This file is licensed under the
> + * terms of the GNU General Public License version 2. This program is
> licensed
> + * "as is" without any warranty of any kind, whether express or implied.
> + */
> +
> +#ifdef __KERNEL__
> +#ifndef __ASM_DBOX2_H__
> +#define __ASM_DBOX2_H__
You don't need the #ifdef __KERNEL__ any more if you don't have
any user-visible parts in the header. Just leave it out of the
list of exported header files (as you already do).
> +
> +/* Vendor information in BR Bootloader
> + */
> +
> +#define DBOX2_VENDOR_OFFSET (0x1ffe0)
> +
> +enum dbox2_mid {
> + DBOX2_MID_NOKIA = 1,
> + DBOX2_MID_PHILIPS = 2,
> + DBOX2_MID_SAGEM = 3,
> +};
> +
> +enum dbox2_mid dbox2_get_mid(void);
Can you move this functionality from the kernel to the boot wrapper?
It looks like this is something that really belongs into the device
tree.
> +static void __init dbox2_setup_arch(void)
> +{
> + struct device_node *np;
> + static u8 __iomem *config;
> +
> + cpm_reset();
> + init_ioports();
> +
> + /* Enable external IRQs for AVIA chips */
> + clrbits32(&mpc8xx_immr->im_siu_conf.sc_siumcr, 0x0c00);
This smells like a hack. What are AVIA chips, and shouldn't
their driver enable the IRQs?
> + if (dbox2_manuf_id == DBOX2_MID_NOKIA)
> + np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
> PROTECTED]");
> + else
> + np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
> PROTECTED]");
> +
> + if (np) {
> + of_detach_node(np);
> + of_node_put(np);
> + }
> +
> + if (dbox2_manuf_id == DBOX2_MID_PHILIPS)
> + np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
> PROTECTED]");
> + else
> + np = of_find_node_by_path("/[EMAIL PROTECTED]/[EMAIL
> PROTECTED]");
> +
> + if (np) {
> + of_detach_node(np);
> + of_node_put(np);
> + }
> +}
What is this code for? Why do you want to detach nodes from the device
tree that have been put in there by the boot loader?
> +static struct of_device_id __initdata of_bus_ids[] = {
> + { .name = "soc", },
> + { .name = "cpm", },
> + { .name = "localbus", },
> + {},
> +};
Shouldn't this check for 'compatible' properties instead of 'name'?
> +static int __init declare_of_platform_devices(void)
> +{
> + /* Publish the QE devices */
> + if (machine_is(dbox2))
> + of_platform_bus_probe(NULL, of_bus_ids, NULL);
> +
> + return 0;
> +}
> +device_initcall(declare_of_platform_devices);
This is a candidate for the new platform_initcall stuff.
I also once did a patch that let you have a default 'of_bus_ids'
member in the ppc_md. I never got around to submitting that, but
if there is interest, I can dig it out again.
> --- a/include/asm-powerpc/mpc8xx.h
> +++ b/include/asm-powerpc/mpc8xx.h
> @@ -23,6 +23,10 @@
> #include
> #endif
>
> +#if defined(CONFIG_DBOX2)
> +#include
> +#endif
> +
Don't hide #includes or platform specific #defines in #ifdef.
Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2] powerpc: DBox2 Board Support
This patch adds device tree source, default config and setup code for
DBox2 devices.
Signed-off-by: Jochen Friedrich <[EMAIL PROTECTED]>
---
arch/powerpc/boot/dts/dbox2.dts | 263
arch/powerpc/configs/dbox2_defconfig | 1042 ++
arch/powerpc/platforms/8xx/Kconfig |7 +
arch/powerpc/platforms/8xx/Makefile |1 +
arch/powerpc/platforms/8xx/dbox2.h | 30 +
arch/powerpc/platforms/8xx/dbox2_setup.c | 226 +++
include/asm-powerpc/mpc8xx.h |4 +
7 files changed, 1573 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/boot/dts/dbox2.dts
create mode 100644 arch/powerpc/configs/dbox2_defconfig
create mode 100644 arch/powerpc/platforms/8xx/dbox2.h
create mode 100644 arch/powerpc/platforms/8xx/dbox2_setup.c
diff --git a/arch/powerpc/boot/dts/dbox2.dts b/arch/powerpc/boot/dts/dbox2.dts
new file mode 100644
index 000..8d91510
--- /dev/null
+++ b/arch/powerpc/boot/dts/dbox2.dts
@@ -0,0 +1,263 @@
+/*
+ * DBOX2 Device Tree Source
+ *
+ * Copyright 2007 Jochen Friedrich <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+/ {
+ model = "Dbox2";
+ compatible = "betaresearch,dbox2";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ PowerPC,[EMAIL PROTECTED] {
+ device_type = "cpu";
+ reg = <0>;
+ d-cache-line-size = ;
+ i-cache-line-size = ;
+ d-cache-size = ;
+ i-cache-size = ;
+ timebase-frequency = <0>;
+ bus-frequency = <0>;
+ clock-frequency = <0>;
+ interrupts = ; // decrementer interrupt
+ interrupt-parent = <&PIC>;
+ };
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0 200>;
+ };
+
+ [EMAIL PROTECTED] {
+ compatible = "betaresearch,dbox2-localbus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <800 1800>;
+
+ ranges = <0 800 1800>;
+
+ [EMAIL PROTECTED] {
+ compatible = "c-cube,enx";
+ reg = <0 3400 100 20>;
+ interrupts = <2 2>;
+ interrupt-parent = <&PIC>;
+ };
+
+ [EMAIL PROTECTED] {
+ compatible = "c-cube,gtx";
+ reg = <40 3000 0 20>;
+ interrupts = <2 2>;
+ interrupt-parent = <&PIC>;
+ };
+
+ [EMAIL PROTECTED] {
+ compatible = "betaresearch,dbox2-fp";
+ interrupts = <4 2>;
+ interrupt-parent = <&PIC>;
+ gpios = <0 e>;
+ gpio-parent = <&CPM1_PIO>;
+ };
+
+ [EMAIL PROTECTED] {
+ compatible = "betaresearch,dbox2-fe";
+ interrupts = ;
+ interrupt-parent = <&PIC>;
+ };
+
+ [EMAIL PROTECTED] {
+ compatible = "c-cube,avia";
+ reg = <200 200>;
+ interrupts = <8 2>;
+ interrupt-parent = <&PIC>;
+ };
+ [EMAIL PROTECTED] {
+ compatible = "betaresearch,dbox2-cam";
+ reg = <400 2>;
+ interrupts = <6 2>;
+ interrupt-parent = <&PIC>;
+ gpios = <1 1c 1 1d 1 1e 1 1f>;
+ gpio-parent = <&CPM1_PIO>;
+ };
+
+ [EMAIL PROTECTED] {
+ compatible = "betaresearch,dbox2-cam";
+ reg = <404 2>;
+ interrupts = <6 2>;
+ interrupt-parent = <&PIC>;
+ gpios = <1 1c 1 1d 1 1e 1 1f>;
+ gpio-parent = <&CPM1_PIO>;
+ };
+
+ [EMAIL PROTECTED] {
+ // Flash also has info about model needed by setup
+ compatible = "cfi-flash",
+"betaresearch,dbox2-config";
+ reg = <800 80>;
+ bank-width = <4>;
+ device-width = <1>;
+ #address-cells = <1>;
+ #size-cells = <
