Re: [PATCH] mpc52xx_psc_spi: fix it for CONFIG_PPC_MERGE

[Sylvain Munaut <tnt@246tNt.com>]

David Brownell wrote:
> On Wednesday 16 May 2007, Sylvain Munaut wrote:
>   
>> Well, this comment is not about the patch but about the driver it self,
>> I didn't see it before today.
>>     
>
> It merged earlier in the 2.6.22 cycle.  If you don't have criticisms
> about the patch itself, I'll forward it for merging after I get at
> least an ack from Dragos.
>   
Yes, I saw when looking at the spi-devl archive. Would have been nice if the
author though of cc-ing the ppc-embedded list ;)

The patch looks ok to me (and needed actually since as Domen pointed
out, 52xx
has been replaced by 5200 in the device tree).
And cell-index has been added to know the psc id without dirty tricks.

>>  - 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.
>>     
>
> That PPC_MERGE stuff does look messy.
>   
Yes, trying to support both in a driver is really not pretty.
Once we can finally get rid of it I'll submit a patch to clear that out.

>>  - 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.
>>  - 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
>>     
>
> Best to use <linux/clk.h> for all of those, but it seems powerpc/ppc
> don't support those interfaces yet ... is there maybe a plan for
> resolving that issue?
>   
Mmm, I wasn't aware of that interface, I'll look into that. Thanks.


    Sylvain