linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Ungerer <gerg@snapgear.com>
To: Matthew Wilcox <willy@debian.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH]: 2.5.38uc1 (MMU-less support)
Date: Thu, 26 Sep 2002 01:33:29 +1000	[thread overview]
Message-ID: <3D91D749.40106@snapgear.com> (raw)
In-Reply-To: <20020925151943.B25721@parcelfarce.linux.theplanet.co.uk>

Hi Matthew,

Matthew Wilcox wrote:
> Thanks for splitting the patch up, makes it easier to see what's going on.
> Let's have another go at making this better...

:-)


> Motorola 5272 ethernet driver:
> * In Config.in, let's conditionalise it on CONFIG_PPC or something

The FEC ethernet core has been used by Motorola in both PowerPC parts
(the MPC860) and the ColdFire (5272). But it could be conditional
on either of these.


> * Can you use module_init() so it doesn't need an entry in Space.c?

Easy done :-)


> * You're defining CONFIG_* variables in the .c file.  I don't know whether
>   this is something we're still trying to avoid doing ... Greg, you seem
>   to be CodingStyle enforcer, what's the word?

To be honest I don't know why all of these are not always included.
The extra code size is not that significant (tehse rae just PHY 
definitions).

Some history:
I didn't write this driver. It is Dan Malek's driver for the ethernet
core in the PowerPC 860. I took that code and generalizd it for use
on the ColdFire 5272 which also uses the same ethernet core design.

By and large I have tried to leave the original drivers functionalty
"as it was", conditionalizing the 5272 support as required (there are
a few key differences to be dealt with).

I approached Dan in the past about merging the code, and he was not
interrested.


> * Why do you need to EXPORT_SYMBOL fec_register_ph and fec_unregister_ph?

I can't see any use for this...


> * There's an awful lot of stuff conditionalised on CONFIG_M5272.  In general,
>   having #ifdefs within functions is frowned upon.

Yeah, there is a bit of that. All due to trying to handle the
silicon implementation differences on the 860 and 5272.
I'll see if I can clean it up some.


> Motorola 68328 and ColdFire serial drivers:
> * Move to drivers/serial
> * Lose this change from the Makefile:
> -			selection.o sonypi.o sysrq.o tty_io.o tty_ioctl.o
> +			selection.o sonypi.o sysrq.o tty_io.o tty_ioctl.o \
> * Drop the custom MIN() definition.
> * Port to new serial driver framework.

This is on my todo list :-)


> MTD driver patches for uClinux supported platforms:
> I don't see any problems.  Submit to Linus via Dave Woodhouse, I guess.

Already done. They have just not hit the mailline tree yet.
But David has it in his CVS now.


> Motorola 68328 framebuffer:
> Don't see any problems here either.
> 
> uClinux FLAT file format exe loader:
> * Drop the MAX() macro.
> * +#include "../lib/inflate2.c".  Er.  You seem to have missed inflate2.c
>   from your patch, and this really isn't the right way to do it anyway.
>   Can't you share inflate.c these days?
> * I'm also a little unsure about your per-arch #defines.  Could you put
>   comments by each saying why they're necessary?

I'll look at cleaning this up too. This code has been hacked
on by so many people, I am still cleaning the chewy bits of it :-)


> I haven't reveiwed the other two patches.

mmnommu is probably the most contenious, and effects the most
files. I am still looking at merging the separated mmnommu
directory at the top level back into the mm directory. Just
thinking about the cleanest way to do that.

The m68knommu is purely architecture support, doesn't mess
with anything else in the kernel.

Thanks for the feedback, much appreciated!

Regards
Greg



------------------------------------------------------------------------
Greg Ungerer  --  Chief Software Wizard        EMAIL:  gerg@snapgear.com
Snapgear Pty Ltd                               PHONE:    +61 7 3279 1822
825 Stanley St,                                  FAX:    +61 7 3279 1820
Woolloongabba, QLD, 4102, Australia              WEB:   www.SnapGear.com


  reply	other threads:[~2002-09-25 15:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-09-25 14:19 Matthew Wilcox
2002-09-25 15:33 ` Greg Ungerer [this message]
2002-09-25 15:43 ` Greg KH
2002-09-26  2:35 ` Greg Ungerer
2002-09-26  3:19   ` Randy Dunlap
2002-09-26  4:29     ` Greg Ungerer
2002-09-26 14:56   ` Matthew Wilcox
2002-09-27  4:09     ` Greg Ungerer
2002-09-26  6:07 ` Greg Ungerer
  -- strict thread matches above, loose matches on Subject: below --
2002-09-25  3:48 Greg Ungerer
2002-09-25 15:45 ` Greg KH
2002-09-25 23:57   ` Greg Ungerer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3D91D749.40106@snapgear.com \
    --to=gerg@snapgear.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=willy@debian.org \
    --subject='Re: [PATCH]: 2.5.38uc1 (MMU-less support)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).