Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Hello Vitaly,
Vitaly Bordug wrote:
On Tue, 18 Mar 2008 09:04:14 +0100
Heiko Schocher wrote:
[...]
OK. Another thought about this. Shouldnt this table go in the dts?
A device node like
cpm_pin {
pins = port pin flags;
};
would be nice, or?
This has been a disputable question some time ago, and decided (or it looks
like decided) that devtree describes hardware, and not the way it is
configured at the moment. Therefor, best way for pin stuff is considered, as
Scott mentioned, to set up stuff inside the firmware.
OK, I dont want to start a new discusion about that, but I always thought
(maybe learned, not sure anymore ;-) Linux should be as much independent
as possible from the bootloader(=firmware). If the firmware doesnt make
things right Linux maybe hang ... :-(
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Hello Stephen,
Stephen Rothwell wrote:
On Fri, 14 Mar 2008 10:24:30 +0100 Heiko Schocher [EMAIL PROTECTED] wrote:
[...]
+struct cpm_pin {
+int port, pin, flags;
+};
I wish someone would consolidate all these definitions of cpm_pin.
Hmm... do you mean something like, moving this struct
in cpm1.h and ...
+static void __init init_ioports(void)
+{
+int i;
+
+for (i = 0; i ARRAY_SIZE(mgsuvd_pins); i++) {
+struct cpm_pin *pin = mgsuvd_pins[i];
+cpm1_set_pin(pin-port, pin-pin, pin-flags);
+}
And the code that uses them ...
making in arch/powerpc/sysdev/cpm1.c a function
cpm1_setup_pins (struct cpm_pin *pins) ?
+static void __init mgsuvd_setup_arch(void)
+{
+struct device_node *cpu;
+
+cpu = of_find_node_by_type(NULL, cpu);
+if (cpu != 0) {
cpu is a pointer, so cpu != NULL or just cpu
+const unsigned int *fp;
+
+fp = of_get_property(cpu, clock-frequency, NULL);
+if (fp != 0)
Ditto for fp test
Its no longer necessary, thanks to Scott
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Hi Heiko,
On Tue, 18 Mar 2008 08:13:06 +0100 Heiko Schocher [EMAIL PROTECTED] wrote:
Stephen Rothwell wrote:
On Fri, 14 Mar 2008 10:24:30 +0100 Heiko Schocher [EMAIL PROTECTED] wrote:
[...]
+struct cpm_pin {
+ int port, pin, flags;
+};
I wish someone would consolidate all these definitions of cpm_pin.
Hmm... do you mean something like, moving this struct
in cpm1.h and ...
Yes or somewhere else appropriate.
+ for (i = 0; i ARRAY_SIZE(mgsuvd_pins); i++) {
+ struct cpm_pin *pin = mgsuvd_pins[i];
+ cpm1_set_pin(pin-port, pin-pin, pin-flags);
+ }
And the code that uses them ...
making in arch/powerpc/sysdev/cpm1.c a function
cpm1_setup_pins (struct cpm_pin *pins) ?
Yes.
This is not necessary for your patch, but would be a nice cleanup later.
N.B. this struct is alos used by users of cpm2_set_pin().
--
Cheers,
Stephen Rothwell[EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/
pgpTpsZFLoAhE.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Hello Stephen,
Stephen Rothwell wrote:
On Tue, 18 Mar 2008 08:13:06 +0100 Heiko Schocher [EMAIL PROTECTED] wrote:
Stephen Rothwell wrote:
On Fri, 14 Mar 2008 10:24:30 +0100 Heiko Schocher [EMAIL PROTECTED] wrote:
[...]
+struct cpm_pin {
+ int port, pin, flags;
+};
I wish someone would consolidate all these definitions of cpm_pin.
Hmm... do you mean something like, moving this struct
in cpm1.h and ...
Yes or somewhere else appropriate.
+ for (i = 0; i ARRAY_SIZE(mgsuvd_pins); i++) {
+ struct cpm_pin *pin = mgsuvd_pins[i];
+ cpm1_set_pin(pin-port, pin-pin, pin-flags);
+ }
And the code that uses them ...
making in arch/powerpc/sysdev/cpm1.c a function
cpm1_setup_pins (struct cpm_pin *pins) ?
Yes.
This is not necessary for your patch, but would be a nice cleanup later.
N.B. this struct is alos used by users of cpm2_set_pin().
OK. Another thought about this. Shouldnt this table go in the dts?
A device node like
cpm_pin {
pins = port pin flags;
};
would be nice, or?
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
On Tue, Mar 18, 2008 at 09:04:14AM +0100, Heiko Schocher wrote:
OK. Another thought about this. Shouldnt this table go in the dts?
A device node like
cpm_pin {
pins = port pin flags;
};
would be nice, or?
Well, the device tree is a mechanism for communicating from the firmware
to the kernel, and if we could control the firmware better we'd just make
it set the pins properly to begin with. :-)
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
On Tue, 18 Mar 2008 09:04:14 +0100
Heiko Schocher wrote:
Hello Stephen,
Stephen Rothwell wrote:
On Tue, 18 Mar 2008 08:13:06 +0100 Heiko Schocher [EMAIL PROTECTED]
wrote:
Stephen Rothwell wrote:
On Fri, 14 Mar 2008 10:24:30 +0100 Heiko Schocher [EMAIL PROTECTED]
wrote:
[...]
+struct cpm_pin {
+int port, pin, flags;
+};
I wish someone would consolidate all these definitions of cpm_pin.
Hmm... do you mean something like, moving this struct
in cpm1.h and ...
Yes or somewhere else appropriate.
+for (i = 0; i ARRAY_SIZE(mgsuvd_pins); i++) {
+struct cpm_pin *pin = mgsuvd_pins[i];
+cpm1_set_pin(pin-port, pin-pin, pin-flags);
+}
And the code that uses them ...
making in arch/powerpc/sysdev/cpm1.c a function
cpm1_setup_pins (struct cpm_pin *pins) ?
Yes.
This is not necessary for your patch, but would be a nice cleanup
later. N.B. this struct is alos used by users of cpm2_set_pin().
OK. Another thought about this. Shouldnt this table go in the dts?
A device node like
cpm_pin {
pins = port pin flags;
};
would be nice, or?
This has been a disputable question some time ago, and decided (or it looks
like decided) that devtree describes hardware, and not the way it is configured
at the moment. Therefor, best way for pin stuff is considered, as Scott
mentioned, to set up stuff inside the firmware.
-Vitaly
bye,
Heiko
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Dear Scott, in message [EMAIL PROTECTED] you wrote: Well, the device tree is a mechanism for communicating from the firmware to the kernel, and if we could control the firmware better we'd just make it set the pins properly to begin with. :-) Is this just a comment, or do you oppose Heiko's suggestion? Other uses of the device tree seem possible and reasonable, too. For example, we can use the device tree to configure the firmware (U-Boot in this case). Using the device tree to describe the pin configuration of the hardware sounds easier to me than hard-coding it in some source (or header) file - no matter if this is in the kernel and/or in the firmware. What do you think? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED] A quarrel is quickly settled when deserted by one party; there is no battle unless there be two. - Seneca ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Wolfgang Denk wrote: Dear Scott, in message [EMAIL PROTECTED] you wrote: Well, the device tree is a mechanism for communicating from the firmware to the kernel, and if we could control the firmware better we'd just make it set the pins properly to begin with. :-) Is this just a comment, or do you oppose Heiko's suggestion? It was intended more as a gentle nudge to set up the pins in firmware when not constrained by existing firmwares in the field that must be supported. Other uses of the device tree seem possible and reasonable, too. For example, we can use the device tree to configure the firmware (U-Boot in this case). Using the device tree to describe the pin configuration of the hardware sounds easier to me than hard-coding it in some source (or header) file - no matter if this is in the kernel and/or in the firmware. What do you think? I'm fine with putting it in the device tree for firmware's benefit, though I'm not sure I fully agree with the easier bit until we get support for expressions and named constants in dts, so that the flags would be less opaque. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
On Fri, Mar 14, 2008 at 10:24:30AM +0100, Heiko Schocher wrote:
+ setbits16(mpc8xx_immr-im_ioport.iop_pcso, 0x300);
+ cpm1_clk_setup(CPM_CLK_SCC3, CPM_CLK5, CPM_CLK_RX);
+ cpm1_clk_setup(CPM_CLK_SCC3, CPM_CLK6, CPM_CLK_TX);
+ setbits32(mpc8xx_immr-im_cpm.cp_pbpar, 0x300);
+ setbits32(mpc8xx_immr-im_cpm.cp_pbdir, 0x300);
Any particular reason not to use cpm1_set_pin() rather than those setbits?
+static void __init mgsuvd_setup_arch(void)
+{
+ struct device_node *cpu;
+
+ cpu = of_find_node_by_type(NULL, cpu);
+ if (cpu != 0) {
+ const unsigned int *fp;
+
+ fp = of_get_property(cpu, clock-frequency, NULL);
+ if (fp != 0)
+ loops_per_jiffy = *fp / HZ;
+ else
+ loops_per_jiffy = 5000 / HZ;
+ of_node_put(cpu);
+ }
+
This is obsolete and unnecessary.
+ ROOT_DEV = Root_NFS;
Likewise.
+static int __init mgsuvd_probe(void)
+{
+ char *model = of_get_flat_dt_prop(of_get_flat_dt_root(),
+ model, NULL);
+ if (model == NULL)
+ return 0;
+ if (strcmp(model, MGSUVD))
+ return 0;
+
+ return 1;
+}
Check compatible (using of_flat_dt_is_compatible), not model.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.
Hi Heiko,
On Fri, 14 Mar 2008 10:24:30 +0100 Heiko Schocher [EMAIL PROTECTED] wrote:
This add support of the mgsuvd board from keymile to
arch/powerpc. Supported SMC1 (serial console),
SCC3 Ethernet (10Mbps hdx)
Paul would probably like some description of the board here.
+++ b/arch/powerpc/platforms/8xx/mgsuvd.c
+extern void cpm_reset(void);
This declaration is in asm/cpm1.h
+struct cpm_pin {
+ int port, pin, flags;
+};
I wish someone would consolidate all these definitions of cpm_pin.
+static void __init init_ioports(void)
+{
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(mgsuvd_pins); i++) {
+ struct cpm_pin *pin = mgsuvd_pins[i];
+ cpm1_set_pin(pin-port, pin-pin, pin-flags);
+ }
And the code that uses them ...
+static void __init mgsuvd_setup_arch(void)
+{
+ struct device_node *cpu;
+
+ cpu = of_find_node_by_type(NULL, cpu);
+ if (cpu != 0) {
cpu is a pointer, so cpu != NULL or just cpu
+ const unsigned int *fp;
+
+ fp = of_get_property(cpu, clock-frequency, NULL);
+ if (fp != 0)
Ditto for fp test
--
Cheers,
Stephen Rothwell[EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/
pgpIR4sKicdT3.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
