Grant Likely wrote:
> Both this patch and the CM5200 support patch (#6 in your series) are
> pretty much clones of lite5200.c. I don't think this is the right
> approach. Don't duplicate code in this way. Determine the common
> bits and put them in a common place to be usable by any 5200 board
> port.
>
> It might even be better just to add a platform that matches on
> compatible='mpc5200-generic' which is usable for mpc5200 boards that
> don't need any custom setup by the kernel at platform setup time.
> (which will probably be most 5200 boards).
Agree, will try more generic approach.
>> +static void __init
>> +tqm5200_setup_cpu(void)
>> +{
>> + struct mpc52xx_gpio __iomem *gpio;
>> + u32 port_config;
>> +
>> + /* Map zones */
>> + gpio = mpc52xx_find_and_map("mpc5200-gpio");
>> + if (!gpio) {
>> + printk(KERN_ERR __FILE__ ": "
>> + "Error while mapping GPIO register for port config. "
>> + "Expect some abnormal behavior\n");
>> + goto error;
>> + }
>> +
>> + /* Set port config */
>> + port_config = in_be32(&gpio->port_config);
>> +
>> + port_config &= ~0x00800000; /* 48Mhz internal, pin is GPIO */
>> +
>> + port_config &= ~0x00007000; /* USB port : Differential mode */
>> + port_config |= 0x00001000; /* USB 1 only */
>> +
>> + port_config &= ~0x03000000; /* ATA CS is on csb_4/5 */
>> + port_config |= 0x01000000;
>
> Are you *sure* you want this? You should only be touching port_config
> if firmware fails to set it up correctly. Don't blindly copy what was
> done for the lite5200.
>
> Lite5200 touches it because firmware does *not* do the right thing at
> the moment.
Yes, that's needed, but will be moved to U-boot.
>> +void tqm5200_show_cpuinfo(struct seq_file *m)
>> +{
>> + struct device_node* np = of_find_all_nodes(NULL);
>> + const char *model = NULL;
>> +
>> + if (np)
>> + model = of_get_property(np, "model", NULL);
>> +
>> + seq_printf(m, "vendor\t\t: Freescale Semiconductor\n");
>
> Freescale? Really?
Well, not really...
Something like
seq_printf(m, "Vendor\t\t: TQ Components\n");
seq_printf(m, "Machine\t\t: %s\n", model);
and model set to 'tqc,tqm5200' would be more accurate but going for
compatible='mpc5200-generic' platform we may need to drop Vendor line
anyway.
m.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
