@@ -1,9 +1,5 @@## Makefile for the PowerPC 83xx linux kernel.#-obj-y:=misc.o+obj-y:=setup.omisc.oobj-$(CONFIG_PCI)+=pci.o-obj-$(CONFIG_MPC834x_SYS)+=mpc834x_sys.o-obj-$(CONFIG_MPC834x_ITX)+=mpc834x_itx.o-obj-$(CONFIG_MPC8360E_PB)+=mpc8360e_pb.o-obj-$(CONFIG_MPC832x_MDS)+=mpc832x_mds.o
@@ -1,19 +0,0 @@-/*- * Copyright (C) Freescale Semicondutor, Inc. 2006. All rights reserved.- *- * Description:- * MPC832x MDS board specific header.- *- * 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.- *- */--#ifndef __MACH_MPC832x_MDS_H__-#define __MACH_MPC832x_MDS_H__--extern u8 *get_bcsr(void);--#endif /* __MACH_MPC832x_MDS_H__ */
@@ -1,121 +0,0 @@-/*- * arch/powerpc/platforms/83xx/mpc834x_itx.c- *- * MPC834x ITX board specific routines- *- * Maintainer: Kumar Gala <galak@kernel.crashing.org>- *- * 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.- */--#include <linux/stddef.h>-#include <linux/kernel.h>-#include <linux/init.h>-#include <linux/errno.h>-#include <linux/reboot.h>-#include <linux/pci.h>-#include <linux/kdev_t.h>-#include <linux/major.h>-#include <linux/console.h>-#include <linux/delay.h>-#include <linux/seq_file.h>-#include <linux/root_dev.h>--#include <asm/system.h>-#include <asm/atomic.h>-#include <asm/time.h>-#include <asm/io.h>-#include <asm/machdep.h>-#include <asm/ipic.h>-#include <asm/bootinfo.h>-#include <asm/irq.h>-#include <asm/prom.h>-#include <asm/udbg.h>-#include <sysdev/fsl_soc.h>--#include "mpc83xx.h"--#include <platforms/83xx/mpc834x_sys.h>--#ifndef CONFIG_PCI-unsigned long isa_io_base = 0;-unsigned long isa_mem_base = 0;-#endif--/* ************************************************************************- *- * Setup the architecture- *- */-static void __init mpc834x_itx_setup_arch(void)-{- struct device_node *np;-- if (ppc_md.progress)- ppc_md.progress("mpc834x_itx_setup_arch()", 0);-- np = of_find_node_by_type(NULL, "cpu");- if (np != 0) {- const unsigned int *fp =- get_property(np, "clock-frequency", NULL);- if (fp != 0)- loops_per_jiffy = *fp / HZ;- else- loops_per_jiffy = 50000000 / HZ;- of_node_put(np);- }-#ifdef CONFIG_PCI- for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)- add_bridge(np);-- ppc_md.pci_exclude_device = mpc83xx_exclude_device;-#endif--#ifdef CONFIG_ROOT_NFS- ROOT_DEV = Root_NFS;-#else- ROOT_DEV = Root_HDA1;-#endif-}--void __init mpc834x_itx_init_IRQ(void)-{- struct device_node *np;-- np = of_find_node_by_type(NULL, "ipic");- if (!np)- return;-- ipic_init(np, 0);-- /* Initialize the default interrupt mapping priorities,- * in case the boot rom changed something on us.- */- ipic_set_default_priority();-}--/*- * Called very early, MMU is off, device-tree isn't unflattened- */-static int __init mpc834x_itx_probe(void)-{- /* We always match for now, eventually we should look at the flat- dev tree to ensure this is the board we are suppose to run on- */- return 1;-}--define_machine(mpc834x_itx) {- .name = "MPC834x ITX",- .probe = mpc834x_itx_probe,- .setup_arch = mpc834x_itx_setup_arch,- .init_IRQ = mpc834x_itx_init_IRQ,- .get_irq = ipic_get_irq,- .restart = mpc83xx_restart,- .time_init = mpc83xx_time_init,- .calibrate_decr = generic_calibrate_decr,- .progress = udbg_progress,-};
@@ -1,23 +0,0 @@-/*- * arch/powerpc/platforms/83xx/mpc834x_itx.h- *- * MPC834X ITX common board definitions- *- * Maintainer: Kumar Gala <galak@kernel.crashing.org>- *- * 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.- *- */--#ifndef __MACH_MPC83XX_ITX_H__-#define __MACH_MPC83XX_ITX_H__--#define PIRQA MPC83xx_IRQ_EXT4-#define PIRQB MPC83xx_IRQ_EXT5-#define PIRQC MPC83xx_IRQ_EXT6-#define PIRQD MPC83xx_IRQ_EXT7--#endif /* __MACH_MPC83XX_ITX_H__ */
@@ -1,140 +0,0 @@-/*- * arch/powerpc/platforms/83xx/mpc834x_sys.c- *- * MPC834x SYS board specific routines- *- * Maintainer: Kumar Gala <galak@kernel.crashing.org>- *- * 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.- */--#include <linux/stddef.h>-#include <linux/kernel.h>-#include <linux/init.h>-#include <linux/errno.h>-#include <linux/reboot.h>-#include <linux/pci.h>-#include <linux/kdev_t.h>-#include <linux/major.h>-#include <linux/console.h>-#include <linux/delay.h>-#include <linux/seq_file.h>-#include <linux/root_dev.h>--#include <asm/system.h>-#include <asm/atomic.h>-#include <asm/time.h>-#include <asm/io.h>-#include <asm/machdep.h>-#include <asm/ipic.h>-#include <asm/bootinfo.h>-#include <asm/irq.h>-#include <asm/prom.h>-#include <asm/udbg.h>-#include <sysdev/fsl_soc.h>--#include "mpc83xx.h"--#ifndef CONFIG_PCI-unsigned long isa_io_base = 0;-unsigned long isa_mem_base = 0;-#endif--/* ************************************************************************- *- * Setup the architecture- *- */-static void __init mpc834x_sys_setup_arch(void)-{- struct device_node *np;-- if (ppc_md.progress)- ppc_md.progress("mpc834x_sys_setup_arch()", 0);-- np = of_find_node_by_type(NULL, "cpu");- if (np != 0) {- const unsigned int *fp =- get_property(np, "clock-frequency", NULL);- if (fp != 0)- loops_per_jiffy = *fp / HZ;- else- loops_per_jiffy = 50000000 / HZ;- of_node_put(np);- }-#ifdef CONFIG_PCI- for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)- add_bridge(np);-- ppc_md.pci_exclude_device = mpc83xx_exclude_device;-#endif--#ifdef CONFIG_ROOT_NFS- ROOT_DEV = Root_NFS;-#else- ROOT_DEV = Root_HDA1;-#endif-}--void __init mpc834x_sys_init_IRQ(void)-{- struct device_node *np;-- np = of_find_node_by_type(NULL, "ipic");- if (!np)- return;-- ipic_init(np, 0);-- /* Initialize the default interrupt mapping priorities,- * in case the boot rom changed something on us.- */- ipic_set_default_priority();-}--#if defined(CONFIG_I2C_MPC) && defined(CONFIG_SENSORS_DS1374)-extern ulong ds1374_get_rtc_time(void);-extern int ds1374_set_rtc_time(ulong);--static int __init mpc834x_rtc_hookup(void)-{- struct timespec tv;-- ppc_md.get_rtc_time = ds1374_get_rtc_time;- ppc_md.set_rtc_time = ds1374_set_rtc_time;-- tv.tv_nsec = 0;- tv.tv_sec = (ppc_md.get_rtc_time) ();- do_settimeofday(&tv);-- return 0;-}--late_initcall(mpc834x_rtc_hookup);-#endif--/*- * Called very early, MMU is off, device-tree isn't unflattened- */-static int __init mpc834x_sys_probe(void)-{- /* We always match for now, eventually we should look at the flat- dev tree to ensure this is the board we are suppose to run on- */- return 1;-}--define_machine(mpc834x_sys) {- .name = "MPC834x SYS",- .probe = mpc834x_sys_probe,- .setup_arch = mpc834x_sys_setup_arch,- .init_IRQ = mpc834x_sys_init_IRQ,- .get_irq = ipic_get_irq,- .restart = mpc83xx_restart,- .time_init = mpc83xx_time_init,- .calibrate_decr = generic_calibrate_decr,- .progress = udbg_progress,-};
@@ -1,23 +0,0 @@-/*- * arch/powerpc/platforms/83xx/mpc834x_sys.h- *- * MPC834X SYS common board definitions- *- * Maintainer: Kumar Gala <galak@kernel.crashing.org>- *- * 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.- *- */--#ifndef __MACH_MPC83XX_SYS_H__-#define __MACH_MPC83XX_SYS_H__--#define PIRQA MPC83xx_IRQ_EXT4-#define PIRQB MPC83xx_IRQ_EXT5-#define PIRQC MPC83xx_IRQ_EXT6-#define PIRQD MPC83xx_IRQ_EXT7--#endif /* __MACH_MPC83XX_SYS_H__ */
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-12-09 07:15:01
On Fri, 2006-12-08 at 19:07 -0600, Kim Phillips wrote:
Eliminate code redundancy. mpc83[246]x_{mds,itx,sys,pb} files merged
into a single setup.c. machine_probe, instead of using the model property,
checks the compatible property for "MPC83xx" (dts files updated appropriately).
This patch also utilizes of_platform_bus_probe() in lieu of manually
calling of_platform_device_create for each ucc_geth device.
Signed-off-by: Kim Phillips <redacted>
I am not completely certain this is the right approach.
While factoring code is good, I think that every single board should
have it's own ppc_md, though you can definitely provide "common"
functions for mpc83xx that can optinally be used by those different
boards.
Maybe put all the freescale ones in one file if you want...
The rationale here is that while your approach is fine for your eval
boards, I don't think it's good for embedded customers. They may want
more complex platforms, with their own directory even if they have a lot
of custom stuff on the board while still possibly picking some of your
"common" code (and their board shouldn't match your overly generic
probe() implementation).
Cheers,
Ben.
From: Kumar Gala <hidden> Date: 2006-12-11 03:41:25
On Dec 9, 2006, at 1:14 AM, Benjamin Herrenschmidt wrote:
On Fri, 2006-12-08 at 19:07 -0600, Kim Phillips wrote:
quoted
Eliminate code redundancy. mpc83[246]x_{mds,itx,sys,pb} files merged
into a single setup.c. machine_probe, instead of using the model
property,
checks the compatible property for "MPC83xx" (dts files updated
appropriately).
This patch also utilizes of_platform_bus_probe() in lieu of manually
calling of_platform_device_create for each ucc_geth device.
Signed-off-by: Kim Phillips <redacted>
Nack on the patch.
I am not completely certain this is the right approach.
While factoring code is good, I think that every single board should
have it's own ppc_md, though you can definitely provide "common"
functions for mpc83xx that can optinally be used by those different
boards.
Maybe put all the freescale ones in one file if you want...
The rationale here is that while your approach is fine for your eval
boards, I don't think it's good for embedded customers. They may want
more complex platforms, with their own directory even if they have
a lot
of custom stuff on the board while still possibly picking some of your
"common" code (and their board shouldn't match your overly generic
probe() implementation).
Cheers,
Ben.
I'm with Ben on this. I think consolidating the code that is common
is fine, but we should have a define_machine() per board. You can
put them all in one mpc83xx/fsl.c
- k
From: Kim Phillips <hidden> Date: 2006-12-11 21:52:04
On Sun, 10 Dec 2006 21:41:24 -0600
Kumar Gala [off-list ref] wrote:
On Dec 9, 2006, at 1:14 AM, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2006-12-08 at 19:07 -0600, Kim Phillips wrote:
quoted
Eliminate code redundancy. mpc83[246]x_{mds,itx,sys,pb} files merged
into a single setup.c. machine_probe, instead of using the model
property,
checks the compatible property for "MPC83xx" (dts files updated
appropriately).
This patch also utilizes of_platform_bus_probe() in lieu of manually
calling of_platform_device_create for each ucc_geth device.
Signed-off-by: Kim Phillips <redacted>
Nack on the patch.
quoted
I am not completely certain this is the right approach.
While factoring code is good, I think that every single board should
have it's own ppc_md, though you can definitely provide "common"
functions for mpc83xx that can optinally be used by those different
boards.
Maybe put all the freescale ones in one file if you want...
The rationale here is that while your approach is fine for your eval
boards, I don't think it's good for embedded customers. They may want
more complex platforms, with their own directory even if they have
a lot
of custom stuff on the board while still possibly picking some of your
"common" code (and their board shouldn't match your overly generic
probe() implementation).
Cheers,
Ben.
I'm with Ben on this. I think consolidating the code that is common
is fine, but we should have a define_machine() per board. You can
put them all in one mpc83xx/fsl.c
so the contents of 83xx/fsl.c would look like:
#ifdef CONFIG_MPC834x_SYS
define_machine(mpc834x_sys) {
.name = "MPC834x SYS",
.probe = mpc83xx_probe,
.setup_arch = mpc83xx_setup_arch,
.init_IRQ = mpc83xx_init_IRQ,
.get_irq = ipic_get_irq,
.restart = mpc83xx_restart,
.time_init = mpc83xx_time_init,
.calibrate_decr = generic_calibrate_decr,
.progress = udbg_progress,
};
#else
#ifdef CONFIG_MPC834x_ITX
define_machine(mpc83xx) {
.name = "MPC834x ITX",
<rest is the same>
#else
..and so on and so forth for CONFIG_MPC8360E_PB, CONFIG_MPC832x_MDS, and now CONFIG_MPC831x_RDB. So the only thing that changes is the name? And perhaps the _probe function? That's still pretty redundant, esp. considering the source of the name to match in the dt is easily modifiable. And it's not as if other platforms (52xx, 86xx, pasemi etc.) don't use of_flat_dt_is_compatible.
All 83xx platform code, define_machine included, is exactly the same on all boards, modulo the QE stuff which is ifdef protected and easily configured. All board differences can be specified in the device tree and/or handled with proper kernel configuration. If an embedded customer wants, they can configure their kernel based on the various 83xx defconfigs, customize their _probe() to match their device tree, all at will, just as before. I'm concerned because 83xx has the capability of being a single kernel image basically for free (we just need to clean some things up in the ucc_geth driver first).
That said, I'm not an embedded customer, and the motivation for this patch was to prep 83xx platform filespace so that we don't have to:
sed s/834/831/ < mpc834x_itx.c > mpc831x_rdb.c
sed s/834/831/ < mpc834x_itx.h > mpc831x_rdb.h
and copy and paste the Makefile and Kconfig lines, which just feels like a terrible thing to do (and keep on doing).
Kim
From: Kumar Gala <hidden> Date: 2006-12-11 22:08:23
On Dec 11, 2006, at 3:51 PM, Kim Phillips wrote:
On Sun, 10 Dec 2006 21:41:24 -0600
Kumar Gala [off-list ref] wrote:
quoted
On Dec 9, 2006, at 1:14 AM, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2006-12-08 at 19:07 -0600, Kim Phillips wrote:
quoted
Eliminate code redundancy. mpc83[246]x_{mds,itx,sys,pb} files
merged
into a single setup.c. machine_probe, instead of using the model
property,
checks the compatible property for "MPC83xx" (dts files updated
appropriately).
This patch also utilizes of_platform_bus_probe() in lieu of
manually
calling of_platform_device_create for each ucc_geth device.
Signed-off-by: Kim Phillips <redacted>
Nack on the patch.
quoted
I am not completely certain this is the right approach.
While factoring code is good, I think that every single board should
have it's own ppc_md, though you can definitely provide "common"
functions for mpc83xx that can optinally be used by those different
boards.
Maybe put all the freescale ones in one file if you want...
The rationale here is that while your approach is fine for your eval
boards, I don't think it's good for embedded customers. They may
want
more complex platforms, with their own directory even if they have
a lot
of custom stuff on the board while still possibly picking some of
your
"common" code (and their board shouldn't match your overly generic
probe() implementation).
Cheers,
Ben.
I'm with Ben on this. I think consolidating the code that is common
is fine, but we should have a define_machine() per board. You can
put them all in one mpc83xx/fsl.c
so the contents of 83xx/fsl.c would look like:
#ifdef CONFIG_MPC834x_SYS
define_machine(mpc834x_sys) {
.name = "MPC834x SYS",
.probe = mpc83xx_probe,
.setup_arch = mpc83xx_setup_arch,
.init_IRQ = mpc83xx_init_IRQ,
.get_irq = ipic_get_irq,
.restart = mpc83xx_restart,
.time_init = mpc83xx_time_init,
.calibrate_decr = generic_calibrate_decr,
.progress = udbg_progress,
};
#else
#ifdef CONFIG_MPC834x_ITX
define_machine(mpc83xx) {
.name = "MPC834x ITX",
<rest is the same>
#else
..and so on and so forth for CONFIG_MPC8360E_PB,
CONFIG_MPC832x_MDS, and now CONFIG_MPC831x_RDB. So the only thing
that changes is the name? And perhaps the _probe function? That's
still pretty redundant, esp. considering the source of the name to
match in the dt is easily modifiable. And it's not as if other
platforms (52xx, 86xx, pasemi etc.) don't use
of_flat_dt_is_compatible.
Why would you need the ifdef's around the define_machine()?
All 83xx platform code, define_machine included, is exactly the
same on all boards, modulo the QE stuff which is ifdef protected
and easily configured. All board differences can be specified in
the device tree and/or handled with proper kernel configuration.
If an embedded customer wants, they can configure their kernel
based on the various 83xx defconfigs, customize their _probe() to
match their device tree, all at will, just as before. I'm
concerned because 83xx has the capability of being a single kernel
image basically for free (we just need to clean some things up in
the ucc_geth driver first).
You say this, but miss the point that its only true for ALL FREESCALE
boards, which are the only boards currently in the tree. We aren't
precluding the ability to have a single kernel image for 83xx.
That said, I'm not an embedded customer, and the motivation for
this patch was to prep 83xx platform filespace so that we don't
have to:
sed s/834/831/ < mpc834x_itx.c > mpc831x_rdb.c
sed s/834/831/ < mpc834x_itx.h > mpc831x_rdb.h
and copy and paste the Makefile and Kconfig lines, which just feels
like a terrible thing to do (and keep on doing).
And I'm suggesting that for 831x_rdb, All you'd add is a new
define_machine() struct.
- kumar
From: Kim Phillips <hidden> Date: 2006-12-12 02:11:00
On Mon, 11 Dec 2006 16:08:20 -0600
Kumar Gala [off-list ref] wrote:
On Dec 11, 2006, at 3:51 PM, Kim Phillips wrote:
quoted
On Sun, 10 Dec 2006 21:41:24 -0600
Kumar Gala [off-list ref] wrote:
quoted
On Dec 9, 2006, at 1:14 AM, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2006-12-08 at 19:07 -0600, Kim Phillips wrote:
quoted
Eliminate code redundancy. mpc83[246]x_{mds,itx,sys,pb} files
merged
into a single setup.c. machine_probe, instead of using the model
property,
checks the compatible property for "MPC83xx" (dts files updated
appropriately).
This patch also utilizes of_platform_bus_probe() in lieu of
manually
calling of_platform_device_create for each ucc_geth device.
Signed-off-by: Kim Phillips <redacted>
Nack on the patch.
quoted
I am not completely certain this is the right approach.
While factoring code is good, I think that every single board should
have it's own ppc_md, though you can definitely provide "common"
functions for mpc83xx that can optinally be used by those different
boards.
Maybe put all the freescale ones in one file if you want...
The rationale here is that while your approach is fine for your eval
boards, I don't think it's good for embedded customers. They may
want
more complex platforms, with their own directory even if they have
a lot
of custom stuff on the board while still possibly picking some of
your
"common" code (and their board shouldn't match your overly generic
probe() implementation).
Cheers,
Ben.
I'm with Ben on this. I think consolidating the code that is common
is fine, but we should have a define_machine() per board. You can
put them all in one mpc83xx/fsl.c
so the contents of 83xx/fsl.c would look like:
#ifdef CONFIG_MPC834x_SYS
define_machine(mpc834x_sys) {
.name = "MPC834x SYS",
.probe = mpc83xx_probe,
.setup_arch = mpc83xx_setup_arch,
.init_IRQ = mpc83xx_init_IRQ,
.get_irq = ipic_get_irq,
.restart = mpc83xx_restart,
.time_init = mpc83xx_time_init,
.calibrate_decr = generic_calibrate_decr,
.progress = udbg_progress,
};
#else
#ifdef CONFIG_MPC834x_ITX
define_machine(mpc83xx) {
.name = "MPC834x ITX",
<rest is the same>
#else
..and so on and so forth for CONFIG_MPC8360E_PB,
CONFIG_MPC832x_MDS, and now CONFIG_MPC831x_RDB. So the only thing
that changes is the name? And perhaps the _probe function? That's
still pretty redundant, esp. considering the source of the name to
match in the dt is easily modifiable. And it's not as if other
platforms (52xx, 86xx, pasemi etc.) don't use
of_flat_dt_is_compatible.
Why would you need the ifdef's around the define_machine()?
how else is platform_probe going to find the right match? it's either that, or adding _probe()s for each platform.
quoted
All 83xx platform code, define_machine included, is exactly the
same on all boards, modulo the QE stuff which is ifdef protected
and easily configured. All board differences can be specified in
the device tree and/or handled with proper kernel configuration.
If an embedded customer wants, they can configure their kernel
based on the various 83xx defconfigs, customize their _probe() to
match their device tree, all at will, just as before. I'm
concerned because 83xx has the capability of being a single kernel
image basically for free (we just need to clean some things up in
the ucc_geth driver first).
You say this, but miss the point that its only true for ALL FREESCALE
boards, which are the only boards currently in the tree. We aren't
it's not my fault that only fsl boards are the only ones in the tree. And why is it so impossible that other boards are the same way?
precluding the ability to have a single kernel image for 83xx.
good to know.
And I'm suggesting that for 831x_rdb, All you'd add is a new
define_machine() struct.
how about making a single _probe() try and match the ppc_md.name with the model in the device tree?
Kim
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-12-12 02:29:38
how else is platform_probe going to find the right match?
it's either that, or adding _probe()s for each platform.
Well, either you want all freescale boards have one platform.
in which case you write one ppc_md() structure, call it
mpc83xx_fslboards or something like that, and have a probe routine that
test for all matches, or create as many ppc_md structures as you have
boards each with it's own probe().
The point here is that other developpers making their own mpc83xx based
boards will not want to use your ppc_md. They will have differences and
those differences will be handled in their board support code via
different hooks in ppc_md.
It's goot if you provide them "generic" things they can use in mpc83xx.c
for areas where they don't differ from the freescale reference
implementations, but don't make the fsl board support code try to match
on everything that has a mpc83xx in it !
Thus, make sure that your DTs for the fsl boards have clear
identification (via the model or compatible property at the root of the
tree is the usual way of doing so) and have a ppc_md or several, as you
like, match specifically those boards. Also, make that ppc_md live in a
different file that doesn't have to be compiled along with the generic
mpc83xx code.
That way, embedded developpers have the choice of either building a
multiplatform kernel that supports their board -and- fsl ones, or a
tailored kernel for their board that doesn't include the fsl specific
stuff, all depending on config options.
Ben.
From: Kumar Gala <hidden> Date: 2006-12-12 02:31:55
On Dec 11, 2006, at 8:29 PM, Benjamin Herrenschmidt wrote:
quoted
how else is platform_probe going to find the right match?
it's either that, or adding _probe()s for each platform.
Well, either you want all freescale boards have one platform.
in which case you write one ppc_md() structure, call it
mpc83xx_fslboards or something like that, and have a probe routine
that
test for all matches, or create as many ppc_md structures as you have
boards each with it's own probe().
The point here is that other developpers making their own mpc83xx
based
boards will not want to use your ppc_md. They will have differences
and
those differences will be handled in their board support code via
different hooks in ppc_md.
It's goot if you provide them "generic" things they can use in
mpc83xx.c
for areas where they don't differ from the freescale reference
implementations, but don't make the fsl board support code try to
match
on everything that has a mpc83xx in it !
Thus, make sure that your DTs for the fsl boards have clear
identification (via the model or compatible property at the root of
the
tree is the usual way of doing so) and have a ppc_md or several, as
you
like, match specifically those boards. Also, make that ppc_md live
in a
different file that doesn't have to be compiled along with the generic
mpc83xx code.
That way, embedded developpers have the choice of either building a
multiplatform kernel that supports their board -and- fsl ones, or a
tailored kernel for their board that doesn't include the fsl specific
stuff, all depending on config options.
Thank you Ben, you described this far better than I was.
- k
From: Scott Wood <hidden> Date: 2006-12-12 21:30:28
Benjamin Herrenschmidt wrote:
Well, either you want all freescale boards have one platform.
in which case you write one ppc_md() structure, call it
mpc83xx_fslboards or something like that, and have a probe routine that
test for all matches, or create as many ppc_md structures as you have
boards each with it's own probe().
The point here is that other developpers making their own mpc83xx based
boards will not want to use your ppc_md.
They *may* not want to (and they certainly shouldn't be forced to), but
some may not want to define a new ppc_md (or modify a probe function)
for every new board if all of the differences are encapsulated in the
device tree. I thought one of the main goals of having a device tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
If a board has truly board-specific logic that needs custom code in the
kernel itself (rather than the bootloader), then it can go in as a
driver with a device tree node (this should be done with the BCSR stuff
where needed).
What about something like the original patch, but with "mpc83xx-generic"
(or similar) as the compatible match? This would address the "matches
everything with mpc83xx in it" concern, without requiring kernel changes
when a new device tree is all that's really needed, and without
requiring non-freescale boards to have something like "fslboards" in the
compatible property just in order to use generic platform
initialization code *if they want to*. Once the BCSR and RTC stuff is
(re)moved, there's really not much of anything fslboard-specific in there.
More generally (and longer-term), what about a completely generic
platform init file that implements the "booting-without-of.txt"
platform? That is, a string that can be placed in the compatible
property, regardless of board or CPU, in order to assert that nothing
board-specific has to be done other than as specified by the device
tree. The model property could still hold the actual board ID if needed
to present to the user, or for matching a more specialized machine
description if problems arise and the device tree cannot be easily
changed (the generic probe could be arranged to run last).
Alternately, just allow the kernel to boot without finding a matching
probe, if generic code is able to extract enough information from the
device tree for generic versions of any non-optional ppc_md functions to
work. If a probe does match, then it can fill in any ppc_md fields it
wants to override (and/or do special initialization, etc). ppc_md
fields can also be filled in by CPU-specific code, or by drivers the
device tree instantiates.
-Scott
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-12-12 21:47:46
quoted
The point here is that other developpers making their own mpc83xx based
boards will not want to use your ppc_md.
They *may* not want to (and they certainly shouldn't be forced to), but
some may not want to define a new ppc_md (or modify a probe function)
for every new board if all of the differences are encapsulated in the
device tree. I thought one of the main goals of having a device tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
That's the ideal situation yes. However, from a more realistic point of
view, I do expect embedded vendors to have their own ppc_md (though it
may cover multiple boards from that vendor). For things like board
specific initialisations, magic GPIOs, reset lines, etc...
The problem with Kim initial patch is that it matches on anything that
says "mpc83xx", thus you completely lose the ability to match somethign
else unless you remove that property, which I find a bit gross.
I do prefer the middle ground approach he (and you) proposed to have an
"mpc83xx_generic" in the compatible property and match on that, but I'm
not 100% certain we are really there yet and I would have been a bit
more comfortable limiting that to known fsl boards. But you are the guys
to maintain those things, so do as you like there.
More generally (and longer-term), what about a completely generic
platform init file that implements the "booting-without-of.txt"
platform? That is, a string that can be placed in the compatible
property, regardless of board or CPU, in order to assert that nothing
board-specific has to be done other than as specified by the device
tree. The model property could still hold the actual board ID if needed
to present to the user, or for matching a more specialized machine
description if problems arise and the device tree cannot be easily
changed (the generic probe could be arranged to run last).
Alternately, just allow the kernel to boot without finding a matching
probe, if generic code is able to extract enough information from the
device tree for generic versions of any non-optional ppc_md functions to
work. If a probe does match, then it can fill in any ppc_md fields it
wants to override (and/or do special initialization, etc). ppc_md
fields can also be filled in by CPU-specific code, or by drivers the
device tree instantiates.
That's sort of a very long term ideal, yes, but again, we aren't quite
there and I'd rather not try to go too fast in that direction. We still
have plenty of stuff to port over from arch/ppc, cleanups to do (like
merging the PCI code, the kernel init code) etc... before we should
spend too much time on that I think.
Ben.
From: Kumar Gala <hidden> Date: 2006-12-12 22:03:13
On Dec 12, 2006, at 3:30 PM, Scott Wood wrote:
Benjamin Herrenschmidt wrote:
quoted
Well, either you want all freescale boards have one platform.
in which case you write one ppc_md() structure, call it
mpc83xx_fslboards or something like that, and have a probe routine
that
test for all matches, or create as many ppc_md structures as you have
boards each with it's own probe().
The point here is that other developpers making their own mpc83xx
based
boards will not want to use your ppc_md.
They *may* not want to (and they certainly shouldn't be forced to),
but
some may not want to define a new ppc_md (or modify a probe function)
for every new board if all of the differences are encapsulated in the
device tree. I thought one of the main goals of having a device
tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
That's true, and if that's the case you'd just set your "model" to
match an existing supported ppc_md.
If a board has truly board-specific logic that needs custom code in
the
kernel itself (rather than the bootloader), then it can go in as a
driver with a device tree node (this should be done with the BCSR
stuff
where needed).
This is not always the case, there are times when you have board
specific modifications you make in the early kernel code. There are
a number of different reasons you would want to do this. The BCSR
stuff you reference is a Freescale board specific feature.
What about something like the original patch, but with "mpc83xx-
generic"
(or similar) as the compatible match? This would address the "matches
everything with mpc83xx in it" concern, without requiring kernel
changes
when a new device tree is all that's really needed, and without
requiring non-freescale boards to have something like "fslboards"
in the
compatible property just in order to use generic platform
initialization code *if they want to*. Once the BCSR and RTC stuff is
(re)moved, there's really not much of anything fslboard-specific in
there.
True, but I dont see what the desire is to create a 'generic' 83xx
support. Who gets to define what is considered 'generic'? What
issue are you guys trying to solve?
Once upon a time I thought the concept of a generic board and such
was a good thing. After many a discussion with DanM, I've been
convinced there isn't that much utility to it in the embedded space :).
If there is some real issue you guys are thinking about, lets talk
about it. But the concept of 'generic' 83xx board support is as
useless waste of time. I'm all for refactoring code so my board code
is simpler, but at the end of the day I know there are people that
are going to need board specific code for their environments.
- kumar
More generally (and longer-term), what about a completely generic
platform init file that implements the "booting-without-of.txt"
platform? That is, a string that can be placed in the compatible
property, regardless of board or CPU, in order to assert that nothing
board-specific has to be done other than as specified by the device
tree. The model property could still hold the actual board ID if
needed
to present to the user, or for matching a more specialized machine
description if problems arise and the device tree cannot be easily
changed (the generic probe could be arranged to run last).
Alternately, just allow the kernel to boot without finding a matching
probe, if generic code is able to extract enough information from the
device tree for generic versions of any non-optional ppc_md
functions to
work. If a probe does match, then it can fill in any ppc_md fields it
wants to override (and/or do special initialization, etc). ppc_md
fields can also be filled in by CPU-specific code, or by drivers the
device tree instantiates.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
From: Kumar Gala <hidden> Date: 2006-12-12 22:06:49
On Dec 12, 2006, at 3:47 PM, Benjamin Herrenschmidt wrote:
quoted
quoted
The point here is that other developpers making their own mpc83xx
based
boards will not want to use your ppc_md.
They *may* not want to (and they certainly shouldn't be forced
to), but
some may not want to define a new ppc_md (or modify a probe function)
for every new board if all of the differences are encapsulated in the
device tree. I thought one of the main goals of having a device
tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
That's the ideal situation yes. However, from a more realistic
point of
view, I do expect embedded vendors to have their own ppc_md (though it
may cover multiple boards from that vendor). For things like board
specific initialisations, magic GPIOs, reset lines, etc...
The problem with Kim initial patch is that it matches on anything that
says "mpc83xx", thus you completely lose the ability to match
somethign
else unless you remove that property, which I find a bit gross.
I do prefer the middle ground approach he (and you) proposed to
have an
"mpc83xx_generic" in the compatible property and match on that, but
I'm
not 100% certain we are really there yet and I would have been a bit
more comfortable limiting that to known fsl boards. But you are the
guys
to maintain those things, so do as you like there.
I'm against the idea of "mpc83xx_generic" if they want to introduce a
"mpc83xx_freescale" or "mpc83xx_fsl_generic" I'm fine with that, but
there is not such thing as a "mpc83xx_generic".
- kumar
From: Kim Phillips <hidden> Date: 2006-12-12 22:24:19
On Tue, 12 Dec 2006 16:06:41 -0600
Kumar Gala [off-list ref] wrote:
quoted
I do prefer the middle ground approach he (and you) proposed to
have an
"mpc83xx_generic" in the compatible property and match on that, but
I'm
not 100% certain we are really there yet and I would have been a bit
more comfortable limiting that to known fsl boards. But you are the
guys
to maintain those things, so do as you like there.
I'm against the idea of "mpc83xx_generic" if they want to introduce a
"mpc83xx_freescale" or "mpc83xx_fsl_generic" I'm fine with that, but
there is not such thing as a "mpc83xx_generic".
I took a look at the TQM8349 code, and it looks like it will be identical in the platform code space. That would subtract the 'fsl' part from the equation. How about 'mpc83xx_eval'? btw, this would be taking us back to the original patch, which I like since I personally don't want to see one file per eval board (I could ifdef protect platforms in machdefs.c if that works for you).
Kim
From: Kumar Gala <hidden> Date: 2006-12-12 22:28:46
On Dec 12, 2006, at 4:24 PM, Kim Phillips wrote:
On Tue, 12 Dec 2006 16:06:41 -0600
Kumar Gala [off-list ref] wrote:
quoted
quoted
I do prefer the middle ground approach he (and you) proposed to
have an
"mpc83xx_generic" in the compatible property and match on that, but
I'm
not 100% certain we are really there yet and I would have been a bit
more comfortable limiting that to known fsl boards. But you are the
guys
to maintain those things, so do as you like there.
I'm against the idea of "mpc83xx_generic" if they want to introduce a
"mpc83xx_freescale" or "mpc83xx_fsl_generic" I'm fine with that, but
there is not such thing as a "mpc83xx_generic".
I took a look at the TQM8349 code, and it looks like it will be
identical in the platform code space. That would subtract the
'fsl' part from the equation. How about 'mpc83xx_eval'? btw, this
would be taking us back to the original patch, which I like since I
personally don't want to see one file per eval board (I could ifdef
protect platforms in machdefs.c if that works for you).
What's the issue with a file per board if all it has is the ppc_md/
define_machine() in it. Someone explain to me why this is a bad thing?
- k
From: Kim Phillips <hidden> Date: 2006-12-12 22:38:08
On Tue, 12 Dec 2006 16:28:38 -0600
Kumar Gala [off-list ref] wrote:
On Dec 12, 2006, at 4:24 PM, Kim Phillips wrote:
quoted
On Tue, 12 Dec 2006 16:06:41 -0600
Kumar Gala [off-list ref] wrote:
quoted
quoted
I do prefer the middle ground approach he (and you) proposed to
have an
"mpc83xx_generic" in the compatible property and match on that, but
I'm
not 100% certain we are really there yet and I would have been a bit
more comfortable limiting that to known fsl boards. But you are the
guys
to maintain those things, so do as you like there.
I'm against the idea of "mpc83xx_generic" if they want to introduce a
"mpc83xx_freescale" or "mpc83xx_fsl_generic" I'm fine with that, but
there is not such thing as a "mpc83xx_generic".
I took a look at the TQM8349 code, and it looks like it will be
identical in the platform code space. That would subtract the
'fsl' part from the equation. How about 'mpc83xx_eval'? btw, this
would be taking us back to the original patch, which I like since I
personally don't want to see one file per eval board (I could ifdef
protect platforms in machdefs.c if that works for you).
What's the issue with a file per board if all it has is the ppc_md/
define_machine() in it. Someone explain to me why this is a bad thing?
well it depends on what you do with the _probe()s. If you have multiple define_machine definitions built in, the generic probe will always succeed to match on the first machine probe_machine() tests, which may or may not be the machine it's currently running on.
Kim
From: Scott Wood <hidden> Date: 2006-12-12 22:40:37
Kumar Gala wrote:
What's the issue with a file per board if all it has is the ppc_md/
define_machine() in it. Someone explain to me why this is a bad thing?
It requires that the kernel be changed even when a new board doesn't
require that the kernel actually *do* anything differently. It's
gratuitous duplication, even if the bit being duplicated is small.
Why is a single ppc_md, which no board is forced to use, but can be used
by any board without special requirements, a bad thing?
-Scott
From: Scott Wood <hidden> Date: 2006-12-12 22:41:22
Kumar Gala wrote:
On Dec 12, 2006, at 3:30 PM, Scott Wood wrote:
quoted
They *may* not want to (and they certainly shouldn't be forced to), but
some may not want to define a new ppc_md (or modify a probe function)
for every new board if all of the differences are encapsulated in the
device tree. I thought one of the main goals of having a device tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
That's true, and if that's the case you'd just set your "model" to
match an existing supported ppc_md.
Having an 831x explicitly claim to be an 834x is just a tad icky...
quoted
If a board has truly board-specific logic that needs custom code in the
kernel itself (rather than the bootloader), then it can go in as a
driver with a device tree node (this should be done with the BCSR stuff
where needed).
This is not always the case, there are times when you have board
specific modifications you make in the early kernel code.
Sure -- I'm not proposing doing away with board-specific machine
descriptions entirely, just reducing the circumstances where they're
required.
True, but I dont see what the desire is to create a 'generic' 83xx
support. Who gets to define what is considered 'generic'?
"Generic" is any board that has needs that aren't expressed in the
device tree.
I'm all for refactoring code so my board code
is simpler, but at the end of the day I know there are people that are
going to need board specific code for their environments.
And an 83xx-generic machine description does not stop them from doing
so. "Generic" does not mean "universal". It means "there's nothing
special about this board". If you need board-specific code in the
kernel, then don't label it generic.
-Scott
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-12-12 22:44:45
well it depends on what you do with the _probe()s. If you have multiple define_machine definitions built in, the generic probe will always succeed to match on the first machine probe_machine() tests, which may or may not be the machine it's currently running on.
Which is an argument for _NOT_ doing a generic probe !
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2006-12-12 22:46:41
On Tue, 2006-12-12 at 16:41 -0600, Scott Wood wrote:
Kumar Gala wrote:
quoted
On Dec 12, 2006, at 3:30 PM, Scott Wood wrote:
quoted
They *may* not want to (and they certainly shouldn't be forced to), but
some may not want to define a new ppc_md (or modify a probe function)
for every new board if all of the differences are encapsulated in the
device tree. I thought one of the main goals of having a device tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
That's true, and if that's the case you'd just set your "model" to
match an existing supported ppc_md.
Having an 831x explicitly claim to be an 834x is just a tad icky...
Indeed, but "compatible" doesn't mean exact match and can be a list. In
fact, it should probably be the other way around, that is 834x claim to
be compatible with 831x... something like:
"834x\0831x\083xx" (with appropriate board name appended of course).
And an 83xx-generic machine description does not stop them from doing
so. "Generic" does not mean "universal". It means "there's nothing
special about this board". If you need board-specific code in the
kernel, then don't label it generic.
As long as you are careful enough so that your "generic" ppc_md. doesn't
try to match everything with a 83xx in it :-0
Ben.
From: Kim Phillips <hidden> Date: 2006-12-12 22:51:42
On Wed, 13 Dec 2006 09:44:38 +1100
Benjamin Herrenschmidt [off-list ref] wrote:
quoted
well it depends on what you do with the _probe()s. If you have multiple define_machine definitions built in, the generic probe will always succeed to match on the first machine probe_machine() tests, which may or may not be the machine it's currently running on.
Which is an argument for _NOT_ doing a generic probe !
Ben.
correct, not doing a generic probe with a file per board (if all it has is the ppc_md/ define_machine() in it), is bad. It needs to be accompanied with its _probe(). Isn't that what the question was?
Kim
From: Kumar Gala <hidden> Date: 2006-12-13 00:20:49
On Dec 12, 2006, at 4:41 PM, Scott Wood wrote:
Kumar Gala wrote:
quoted
On Dec 12, 2006, at 3:30 PM, Scott Wood wrote:
quoted
They *may* not want to (and they certainly shouldn't be forced
to), but
some may not want to define a new ppc_md (or modify a probe
function)
for every new board if all of the differences are encapsulated in
the
device tree. I thought one of the main goals of having a device
tree is
that if it's done right, the kernel need not know about every single
model of board, just the different components that a device tree can
specify.
That's true, and if that's the case you'd just set your "model"
to match an existing supported ppc_md.
Having an 831x explicitly claim to be an 834x is just a tad icky...
quoted
quoted
If a board has truly board-specific logic that needs custom code
in the
kernel itself (rather than the bootloader), then it can go in as a
driver with a device tree node (this should be done with the
BCSR stuff
where needed).
This is not always the case, there are times when you have board
specific modifications you make in the early kernel code.
Sure -- I'm not proposing doing away with board-specific machine
descriptions entirely, just reducing the circumstances where
they're required.
And I'm asking why we are arguing over 10-15 lines of code per board.
quoted
True, but I dont see what the desire is to create a 'generic'
83xx support. Who gets to define what is considered 'generic'?
"Generic" is any board that has needs that aren't expressed in the
device tree.
Which device tree specification? The one today, the one six months
ago, the one six months from now? The concept is ever changing and
doesn't provide much value.
quoted
I'm all for refactoring code so my board code is simpler, but at
the end of the day I know there are people that are going to need
board specific code for their environments.
And an 83xx-generic machine description does not stop them from
doing so. "Generic" does not mean "universal". It means "there's
nothing special about this board". If you need board-specific code
in the kernel, then don't label it generic.
But what value does this have? 83xx, and the majority of freescale's
devices are not put into something as standard as a desktop computer.
- k
From: Kumar Gala <hidden> Date: 2006-12-13 00:23:28
On Dec 12, 2006, at 4:40 PM, Scott Wood wrote:
Kumar Gala wrote:
quoted
What's the issue with a file per board if all it has is the
ppc_md/ define_machine() in it. Someone explain to me why this is
a bad thing?
It requires that the kernel be changed even when a new board
doesn't require that the kernel actually *do* anything
differently. It's gratuitous duplication, even if the bit being
duplicated is small.
But at least someone had to think about the duplication and it
provides remove for customization if its needed in the future.
Why is a single ppc_md, which no board is forced to use, but can be
used by any board without special requirements, a bad thing?
It adds code to all those people that don't need it just so we don't
duplicate a few lines of source code.
- k
It adds code to all those people that don't need it just so we don't
duplicate a few lines of source code.
Sounds like you're describing the raison d'être for device-trees though?
After all, if you want to build a kernel that supports these minor h/w
variations depending on the device-tree it's booted with, then the "few
lines of duplicated source code" you're talking about would also "add
code to all those people that don't need it".
Kumar Gala also wrote:
On Dec 12, 2006, at 4:41 PM, Scott Wood wrote:
quoted
And an 83xx-generic machine description does not stop them from
doing so. "Generic" does not mean "universal". It means "there's
nothing special about this board". If you need board-specific code
in the kernel, then don't label it generic.
But what value does this have? 83xx, and the majority of freescale's
devices are not put into something as standard as a desktop computer.
Then what value do device-trees have at all? Why require new code for
new h/w if it's technically unnecessary? If I've understood correctly (I
confess to not having followed all of the discussion nor the finer
technical points), this would require new code to find its way
"upstream" (to whoever/wherever/whatever that means) from freescale and
then downstream to it's user before the h/w is supported, when this
situation is precisely what device-trees apparently ought to resolve.
Maybe I'm missing something (quite possible). Ben's objection seemed to
be one of naming, but yours seems to be that new h/w should require new
code because it's not wintel fodder for desktop grannies? So why bother
separating h/w description from the compiled kernel in the first place?
Cheers,
Geoff
From: Kumar Gala <hidden> Date: 2006-12-13 06:07:48
On Dec 12, 2006, at 11:25 PM, Geoff Thorpe wrote:
Kumar Gala wrote:
quoted
It adds code to all those people that don't need it just so we =20
don't duplicate a few lines of source code.
Sounds like you're describing the raison d'=EAtre for device-trees =20
though? After all, if you want to build a kernel that supports =20
these minor h/w variations depending on the device-tree it's booted =20=
with, then the "few lines of duplicated source code" you're talking =20=
about would also "add code to all those people that don't need it".
No, because those people (myself included) wouldn't build in support =20
for the freescale referend boards into the kernel I'm building for my =20=
custom board.
My question has been what's the value in trying to save a few lines =20
of code for the reference boards. The idea of a generic board =20
doesn't make sense in the embedded space. Just because the reference =20=
boards for mpc83xx look similar doesn't mean anything else using it =20
will. I know both of the boards I've worked would and still do =20
require custom code.
The reason for the custom code is the device tree doesn't describe =20
all variants of all hardware. Its just not spec'd that far. How do =20
you describe the FPGA and local bus interface to it on my board? How =20
do you describe the compact flash drive on localbus? How do you =20
describe the microcontroller connect over SPI? You dont because =20
there isn't any spec. The majority of developers dont have the time =20
to spend trying to come up with one to solve their specific problem =20
so they hard code some solution that works for them.
Over time will we improve the spec, it will cover more cases and =20
that's great, but trying to come up with some generic board port =20
right now is a waste of time. There are a ton of better things to be =20=
spending your guys time on.
I've yet to see anything that describes any real value to a customer.
Kumar Gala also wrote:
quoted
On Dec 12, 2006, at 4:41 PM, Scott Wood wrote:
quoted
And an 83xx-generic machine description does not stop them from =20
doing so. "Generic" does not mean "universal". It means =20
"there's nothing special about this board". If you need board-=20
specific code in the kernel, then don't label it generic.
But what value does this have? 83xx, and the majority of =20
freescale's devices are not put into something as standard as a =20
desktop computer.
Then what value do device-trees have at all? Why require new code =20
for new h/w if it's technically unnecessary? If I've understood =20
correctly (I confess to not having followed all of the discussion =20
nor the finer technical points), this would require new code to =20
find its way "upstream" (to whoever/wherever/whatever that means) =20
from freescale and then downstream to it's user before the h/w is =20
supported, when this situation is precisely what device-trees =20
apparently ought to resolve.
This is partial true, but if freescale puts out a new processor/board =20=
there is some expectation that its going to require some new code. =20
If nothing else it's going to require a device-tree be provided. If =20
the concern is about how long it takes to support new HW for existing =20=
functionality I think that's BS.
Maybe I'm missing something (quite possible). Ben's objection =20
seemed to be one of naming, but yours seems to be that new h/w =20
should require new code because it's not wintel fodder for desktop =20
grannies? So why bother separating h/w description from the =20
compiled kernel in the first place?
My argument is that trying to describe all HW variants for embedded =20
systems in the device tree is never going to happen. Describing the =20
generality of devices on SoC is useful because everyone has to deal =20
with that. Once you start going past that you get into trouble =20
because of all the various ways people hook things up to busses.
There are a number of subtle reasons I think a generic port is =20
pointless and the only arguments I've heard are some concern about =20
duplication of code and the ability to boot a kernel w/o modification =20=
on new HW.
The duplication code I believe is a style issue and we can reduce the =20=
duplication to a minimum. The ability to boot a kernel w/o =20
modification on new HW is a nice to have, but I dont see this as =20
providing any "real value".
I'd rather see people spending time on problems which need solutions =20
and this isn't one of them.
- k=
'day,
I'm inclined to defer to the maintainer's prerogative of judging the
acceptability of code. :-) Thanks for responding.
However I have a couple of little responses regarding other matters;
Kumar Gala wrote:
There are a number of subtle reasons I think a generic port is
pointless and the only arguments I've heard are some concern about
duplication of code and the ability to boot a kernel w/o modification
on new HW.
To each their own "concern" I guess. "Duplication of code and the
ability to boot a kernel w/o modification on new HW" both seem like
pretty legitimate concerns to me. That this may or may not be acceptable
on a technical level should be the focus, but it seems irrational to
presume to stipulate what interests/concerns other hackers should and
should not have? This is linux, not windows.
The duplication code I believe is a style issue and we can reduce the
duplication to a minimum. The ability to boot a kernel w/o
modification on new HW is a nice to have, but I dont see this as
providing any "real value".
Same comment.
I'd rather see people spending time on problems which need solutions
and this isn't one of them.
Um, with all due respect. You are invoking what could only be described
as "employer's privilege". I doubt the concern here came purely from
someone having nothing better to do and being totally lost for ideas. No
doubt they'll correct me if I'm wrong :-)
I have no comment on your technical objections, only the(se)
non-technical one(s).
(Seasonal) cheers,
Geoff
From: Kim Phillips <hidden> Date: 2006-12-13 18:21:50
On Wed, 13 Dec 2006 00:07:43 -0600
Kumar Gala [off-list ref] wrote:
=20
On Dec 12, 2006, at 11:25 PM, Geoff Thorpe wrote:
=20
quoted
Kumar Gala wrote:
quoted
It adds code to all those people that don't need it just so we =20
don't duplicate a few lines of source code.
Sounds like you're describing the raison d'=EAtre for device-trees =20
though? After all, if you want to build a kernel that supports =20
these minor h/w variations depending on the device-tree it's booted =20
with, then the "few lines of duplicated source code" you're talking =20
about would also "add code to all those people that don't need it".
=20
No, because those people (myself included) wouldn't build in support =20
for the freescale referend boards into the kernel I'm building for my =20
custom board.
my understanding is that embedded developers would *add* code to the generi=
c platform code base that this exercise proves exists.
=20
My question has been what's the value in trying to save a few lines =20
of code for the reference boards. The idea of a generic board =20
doesn't make sense in the embedded space. Just because the reference =20
boards for mpc83xx look similar doesn't mean anything else using it =20
will. I know both of the boards I've worked would and still do =20
require custom code.
the boards aren't that similar, it's just that all board specific code has =
actually migrated to the right place for it; it now goes where it belongs, =
e.g. RTCs go to the RTC subsystem, PHYs go the PHY layer, etc.
=20
The reason for the custom code is the device tree doesn't describe =20
all variants of all hardware. Its just not spec'd that far. How do =20
you describe the FPGA and local bus interface to it on my board? How =20
do you describe the compact flash drive on localbus? How do you =20
describe the microcontroller connect over SPI? You dont because =20
there isn't any spec. The majority of developers dont have the time =20
to spend trying to come up with one to solve their specific problem =20
so they hard code some solution that works for them.
this has always been the case, and we're not changing that. You can still a=
dd code to support unspec'ed functionality. We're just trying to refactor t=
he code for its obvious benefits.
=20
Over time will we improve the spec, it will cover more cases and =20
that's great, but trying to come up with some generic board port =20
right now is a waste of time. There are a ton of better things to be =20
spending your guys time on.
=20
I've yet to see anything that describes any real value to a customer.
The ability to have a single kernel run on multiple boards is of value, esp=
. if the only thing preventing that from happening is a strcmp between a ke=
rnel string and a string in the dt (both software sources).
=20
quoted
Kumar Gala also wrote:
quoted
On Dec 12, 2006, at 4:41 PM, Scott Wood wrote:
quoted
And an 83xx-generic machine description does not stop them from =20
doing so. "Generic" does not mean "universal". It means =20
"there's nothing special about this board". If you need board-=20
specific code in the kernel, then don't label it generic.
But what value does this have? 83xx, and the majority of =20
freescale's devices are not put into something as standard as a =20
desktop computer.
Then what value do device-trees have at all? Why require new code =20
for new h/w if it's technically unnecessary? If I've understood =20
correctly (I confess to not having followed all of the discussion =20
nor the finer technical points), this would require new code to =20
find its way "upstream" (to whoever/wherever/whatever that means) =20
from freescale and then downstream to it's user before the h/w is =20
supported, when this situation is precisely what device-trees =20
apparently ought to resolve.
=20
This is partial true, but if freescale puts out a new processor/board =20
there is some expectation that its going to require some new code. =20
If nothing else it's going to require a device-tree be provided. If =20
the concern is about how long it takes to support new HW for existing =20
functionality I think that's BS.
=20
I don't think it's that, my problem is the redundancy in the code. I wanted=
it gone before we added more :)
quoted
Maybe I'm missing something (quite possible). Ben's objection =20
seemed to be one of naming, but yours seems to be that new h/w =20
should require new code because it's not wintel fodder for desktop =20
grannies? So why bother separating h/w description from the =20
compiled kernel in the first place?
=20
My argument is that trying to describe all HW variants for embedded =20
systems in the device tree is never going to happen. Describing the =20
generality of devices on SoC is useful because everyone has to deal =20
with that. Once you start going past that you get into trouble =20
because of all the various ways people hook things up to busses.
but this patch isn't claiming to do that. It doesn't stop providing a base=
to start from.
There are a number of subtle reasons I think a generic port is =20
pointless and the only arguments I've heard are some concern about =20
duplication of code and the ability to boot a kernel w/o modification =20
on new HW.
yes, and those are indeed valid points. I for one, have access to multiple=
83xx based boards and would love to not have to build a kernel that has on=
e text in a string preventing it from booting on another.
The duplication code I believe is a style issue and we can reduce the =20
duplication to a minimum. The ability to boot a kernel w/o =20
modification on new HW is a nice to have, but I dont see this as =20
providing any "real value".
=20
I'd rather see people spending time on problems which need solutions =20
and this isn't one of them.
=20
sorry, this redundancy has been going on for too long. I brought this issu=
e up a long time ago on this list asking for your opinion and got no respon=
se. Now a new board is here. So how long would you have us submit code ge=
nerated by sed?
Having said all that, I acknowledge your comments. For now, I'm willing to=
settle for at least one line of less redundant code than what's in the tre=
e today. How can we make that happen?
Kim
From: Dan Malek <hidden> Date: 2006-12-13 21:13:40
On Dec 13, 2006, at 1:07 AM, Kumar Gala wrote:
My question has been what's the value in trying to save a few lines
of code for the reference boards. The idea of a generic board
doesn't make sense in the embedded space.
This grand unification of embedded board software just
isn't useful. It's a waste of time for the reference board
developers that should be writing useful device drivers
for the SOC peripherals, and also for those of us trying
to develop real products. I spend way too much time
trying to find configuration options or other indirect
function hooks so the special functions I need to be
performed are done properly.
The current 85xx is a pretty good implementation.
Things that are truly generic are provided as functions
that the individual board port initialization files/functions
can use as needed.
.... The majority of developers dont have the time
to spend trying to come up with one to solve their specific problem
so they hard code some solution that works for them.
Don't make it sound like some "hard code" hack :-)
We just have to write software to support all of these
unique product designs. Many of these boards have
custom external hardware than needs to be controlled
with some special configurations or algorithm. There
are just times I have to toggle a set of GPIOs with a
particular algorithm, for example. This was my comment
about "generic" board ports and device tree a while
ago. The device tree is just an information data base,
it doesn't help me when I really have to write some code
that is unique to a board.
If you want to make similar Freescale boards utilize
common code, that's great. Don't assume any product
will look like one of these boards, no matter how much
you want people to do that. In almost all cases, they
aren't even close due to other product design requirements.
All of these efforts to "help" us develop products by
providing generic board ports with the goal of a single
kernel on multiple boards actually costs us development
time due to the lack of flexibility we need. We just need
a library of generic functions, and then 50 to 100 lines of
"board support" initialization that will initialize custom
hardware, data structures, variables, and then call the
more generic functions. There may be only very minor
differences between the board files, but this simplicity
is much nicer than complex indirect paths to enable
features with cute #defines or other complicated
data structures.
Thanks.
-- Dan
Why do you have this #else here? Doesn't it just restrict you for no
gain?
#ifdef CONFIG_MPC834x_ITX
define_machine(mpc83xx) {
.name = "MPC834x ITX",
<rest is the same>
Your probe function can set ppc_md.name to whatever it wants. So you
could have a define_machine(fsl_83xx_eval_board) whose probe function
would pick up the actual board name from the device tree (e.g. the
root node's model property), put that in ppc_md and return 1 (assuming
of course that the device tree looks like one of your boards).
Paul.
Why do you have this #else here? Doesn't it just restrict you for no
gain?
I think you're looking at an old version of the patch.
quoted
#ifdef CONFIG_MPC834x_ITX
define_machine(mpc83xx) {
.name = "MPC834x ITX",
<rest is the same>
Your probe function can set ppc_md.name to whatever it wants. So you
could have a define_machine(fsl_83xx_eval_board) whose probe function
would pick up the actual board name from the device tree (e.g. the
root node's model property), put that in ppc_md and return 1 (assuming
of course that the device tree looks like one of your boards).
Paul.