Re: [PATCH 3/3] m68knommu: Add ethernet driver for MCF547x/MCF548x

Hi Greg,

On Thu, Oct 04, 2012 at 11:33:32PM +1000, Greg Ungerer wrote:
> Hi Philippe,
>
> On 10/04/2012 05:34 PM, Philippe De Muyter wrote:
>> On Thu, Oct 04, 2012 at 04:56:35PM +1000, Greg Ungerer wrote:
>>> Hi Philippe,
>>>
>>> I think this needs to be done as a platform driver. It is really the
>>> standard way to deal with platform specifics cleanly. I know this
>>> hardware really only exists on one device, but that is no reason
>>> not to do it. Follow the example of the other Freescale FEC driver.
>>> It won't really change the code too much, it just makes the
>>> platform hardware specifics more cleanly separated from the driver
>>> proper.
>>>
>>> A few inline comments below too.
>>>
>>
>> Thanks for your comments.  I had already started the conversion
>> to the dma_map_single api, but I am worried that those functions
>> are not inlined altough IIRC they often resolve to nothing (for the
>> unmap case) or to a single asm("nop")  (for the map case).
>>
>> I'll look at your other comments, and especially the notion of
>> platform driver, that I do not really know.
>
> My biggest concern is the amount of MCD/DMA support code. And it is
> all done quite differently to everything else in the kernel. We may
> get a bit of push back from kernel folk who look after DMA.

Actually, there is already a similar code in arch/powerpc/sysdev/bestcomm
(also from freescale, maybe an identical part, but I did not find any
usable doc), but the powerpc folks kept that hidden in the arch/powerpc
tree, instead of installing it in drivers/dma.

I wanted rather to make the MCD DMA code visible, so that if freescale
reuses it for a powerpc (mpc) or arm (iMX) part it does not end duplicated in
the kernel tree.  But if that's a too hard way, let's keep it hidden
in arch/m68k.

Best regards

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
_______________________________________________
uClinux-dev mailing list
[email protected]
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by [email protected]
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev