Subject: Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver

From: Wolfgang Grandegger <[email-masked]>
Date: Sun, 24 Jan 2010 15:40:31 +0100

Arnd Bergmann wrote:
> On Thursday 21 January 2010, Wolfgang Grandegger wrote:
>> The major problem that Anatolij tries to solve are the different
>> register layouts of the supported SOCs, MPC52xx and MPC8xx. They use the
>> same registers but at different offsets. Therefore we cannot handle
>> this with a single "fec_t" struct to allow building a single kernel
>> image. Instead it's handled by filling a table with register addresses:
>>
>>         if (of_device_is_compatible(ofdev->node, "fsl,mpc5121-fec")) {
>>                 fep->fec.fec_id = FS_ENET_MPC5121_FEC;
>>                 fec_reg_mpc5121(ievent);
>>                 fec_reg_mpc5121(imask);
>>                 ...
>>         } else {
>>                 fec_reg_mpc8xx(ievent);
>>                 fec_reg_mpc8xx(imask);
>>                 ...
>>         }
>>
>> Do you see a more clever solution to this problem? Nevertheless, the
>> code could be improved by using "offsetof", I think.
> 
> Is there any chance of building a kernel that runs on both mpc8xx and
> mpc5121? AFAIK, the 5121 is built on a 6xx core which is fundamentally
> incompatible with 8xx due to different memory management etc.
> 
> Since this makes it all a compile-time decision, it should be solvable
> with a very small number of carefully placed #ifdef in the header files
> an no runtime detection at all.
> 
> Obviously this approach would not work for drivers that want to be portable
> across different register layouts on otherwise compatible platforms.

You are probably right and your proposal would likely result in more
transparent (less ugly) code. There has been some discussion about
unifying FEC drivers when the patches (with the same subject) have been
submitted for the first time in May last year, but it was not about 512x
and 8xx, IIRC.

Wolfgang.