1 Nov 2009 02:03
Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices()
Grant Likely <grant.likely <at> secretlab.ca>
2009-11-01 01:03:55 GMT
2009-11-01 01:03:55 GMT
Hi Wolfram, Thanks for this work. Comments below. On Fri, Oct 30, 2009 at 1:44 PM, Wolfram Sang <w.sang <at> pengutronix.de> wrote: > This driver has a generic part for the probe/remove routines which probably > used to be called from a platform driver and an of_platform driver. Meanwhile, > the driver is of_platform only, so there is no need for the generic part > anymore. Unifying probe/remove finally enables us to call > of_register_spi_devices(), so spi-device nodes from the device tree will be > parsed. This was also mentioned by: > http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073273.html I haven't really been happy with any of the patches that added the of_register_spi_devices() call, but I never got around to dealing with it properly, or even replying. I found Jon's patch too invasive, and the patch referenced above is a little ugly. Adding the call should be really simple. I've drafted a patch to do only that step and attached it to this mail. If this one works for you, then I'll merge it immediately into -next. > On the way, some patterns have been changed to current best practices (like > using '!ptr' and 'struct resource'). Also, an older, yet uncommented, patch > from me has been incorporated to improve the checks if the selected PSC is > valid: > > http://patchwork.ozlabs.org/patch/36780/ There are at least 3 discrete changes here. I prefer for each to be provided as a separate patch so I can cherry pick the ones that are(Continue reading)
>
>> The other concern is that I don't like that the patch is only applicable
>> to gen_bd tasks.
>
> IMHO the big difference between gen_bd on one hand and ATA and FEC on the
> other is that (afaik) we have *no* other choice than using the implemented
> tasks for the latter two, obviously using the Freescale's example microcode.
> Gen_bd can apparently be used for a plenty of purposes, including LPB (as
> shown in your driver), maybe PCI (there are examples in Freescale's code),
> PSC, etc. Thus, the FEC and ATA drivers should not have an option as to
> change the microcode, whereas it is just convenient for gen_bd.
agreed.
>> I've been thinking about this for a while, and I'm not exactly thrilled
>> with the bestcomm layout which keeps the bestcomm firmware separate from the
>> only driver that actually uses it (ie FEC firmware & support code is
>> separate from the FEC driver. Same for ATA). I don't like the fact that
>> code which is only ever used by the ATA driver is maintained completely
RSS Feed