Re: [Qemu-ppc] [PATCH 16/25] PPC: e500: dt: create serial nodes dynamically

[Alexander Graf]

On 01.06.2012, at 00:02, Scott Wood wrote:

> On 05/30/2012 06:00 AM, Alexander Graf wrote:
>> @@ -196,6 +199,38 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>>     /* XXX should contain a reasonable value */
>>     qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
>> 
>> +    /*
>> +     * We have to generate ser1 first, because Linux takes the first
>> +     * device it finds in the dt as serial output device. And we generate
>> +     * devices in reverse order to the dt.
>> +     */
>> +    sprintf(ser1, "%s/address@hidden", soc,
>> +            MPC8544_SERIAL1_REGS_BASE - MPC8544_CCSRBAR_BASE);
>> +    qemu_devtree_add_subnode(fdt, ser1);
>> +    qemu_devtree_setprop_string(fdt, ser1, "device_type", "serial");
>> +    qemu_devtree_setprop_string(fdt, ser1, "compatible", "ns16550");
>> +    qemu_devtree_setprop_cell2(fdt, ser1, "reg", MPC8544_SERIAL1_REGS_BASE -
>> +                               MPC8544_CCSRBAR_BASE, 0x100);
>> +    qemu_devtree_setprop_cell(fdt, ser1, "cell-index", 1);
>> +    qemu_devtree_setprop_cell(fdt, ser1, "clock-frequency", 0);
>> +    qemu_devtree_setprop_cell2(fdt, ser1, "interrupts", 42, 2);
>> +    qemu_devtree_setprop_phandle(fdt, ser1, "interrupt-parent", mpic);
>> +    qemu_devtree_setprop_string(fdt, "/aliases", "serial1", ser1);
> 
> Please put this somewhere it won't have to be duplicated for every board
> -- preferably in the serial device code itself.

Yeah, I talked to Anthony about that and he didn't like the idea of devices 
knowing about their device tree specifics. Serial is a good candidate to 
understand why. A serial device shouldn't know if it's device 0 or device 1. 
It's just a plain device. The semantics on the port go with the board.

That said, we should probably refactor this as a generic helper, so every board 
that implements a serial port can just call a common helper function. I'd do 
that with whatever board gets converted next - probably bamboo :).


Alex