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
_______________________________________________
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded
