Subject: Re: [PATCH] mpc52xx_psc_spi: fix it for CONFIG_PPC_MERGE

From: Sylvain Munaut <[email-masked]>
Date: Wed, 16 May 2007 10:19:35 +0200

Well, this comment is not about the patch but about the driver it self,
I didn't see it before today.
So here's a few things I see from a quick glance at the code :

 - Chaning port_config in the driver is just wrong ... you should _not_
do that. That should have been setup by the bootloader or at worse in
the platform setup code.
 - You do read/write/modify operation on CDM shared register
(clk_enables) from a driver, you should have added something in common
52xx code to do theses with proper locking.
 - MPC52xx_PA(MPC52xx_PSCx_OFFSET(...)) ??? You should get that from the
resource of the platform_device. This macro is just there for early
console stuff.
 - You can get f_system from the device tree instead of just assuming
it's 512 MHz. It probably need to be done the same way it's done to find
ipb_freq.
 - Would have been nice to be able to somehow configure MCLK rather than
#define it
 - I hope to remove all arch/ppc stuff by 2.6.23 if I can make the
cuimage stuff work in arch/powerpc so just including the platform code
stuff for 1 kernel version ...

    Sylvain