New style dpalloc/hostalloc routines (diff).

Sorry forgot to attach the patch.

Pantelis Antoniou writes:

+# Support new type of routines, usable from modules
+bool 'Use new type dpalloc routines()' CONFIG_NEW_DPALLOC
+bool 'Use new type hostalloc routines()' CONFIG_NEW_HOSTALLOC
+if [ "$CONFIG_NEW_DPALLOC" = "y" -o "$CONFIG_NEW_HOSTALLOC" = "y" ]; then
+  define_bool CONFIG_CPM_RHEAP y
+fi

I don't want to see config options that select between different
internal implementations of the same thing.  Either your new routines
are better, and we'll use them, or they are worse, and we'll use the
old ones.  Having a config option just leads to tons of ifdefs
throughout the code, which makes it harder to read and understand.
Having two implementations of the same thing is just bloat.

Similarly, I don't like the way all your new routines have a "new_"
prefix on the name.  You should be thinking of replacing the existing
routines rather than providing an alternative implementation with a
different name.  Where you have changed the API, either fix the
drivers or provide a compatibility routine.

The way it looks at the moment, it seems that you don't really have
the conviction that your code is better than what is there already.
Please redo your patch so that it just replaces the old routines.  And
please don't send it as a bkpatch since they are impossible to read, a
plain diff -u is much better.

Paul.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Paul Mackerras wrote:

Pantelis Antoniou writes:

quoted

+# Support new type of routines, usable from modules
+bool 'Use new type dpalloc routines()' CONFIG_NEW_DPALLOC
+bool 'Use new type hostalloc routines()' CONFIG_NEW_HOSTALLOC
+if [ "$CONFIG_NEW_DPALLOC" = "y" -o "$CONFIG_NEW_HOSTALLOC" = "y" ]; then
+  define_bool CONFIG_CPM_RHEAP y
+fi

I don't want to see config options that select between different
internal implementations of the same thing.  Either your new routines
are better, and we'll use them, or they are worse, and we'll use the
old ones.  Having a config option just leads to tons of ifdefs
throughout the code, which makes it harder to read and understand.
Having two implementations of the same thing is just bloat.

Similarly, I don't like the way all your new routines have a "new_"
prefix on the name.  You should be thinking of replacing the existing
routines rather than providing an alternative implementation with a
different name.  Where you have changed the API, either fix the
drivers or provide a compatibility routine.

The way it looks at the moment, it seems that you don't really have
the conviction that your code is better than what is there already.
Please redo your patch so that it just replaces the old routines.  And
please don't send it as a bkpatch since they are impossible to read, a
plain diff -u is much better.

Paul.

Well that's easy to do since that's the way it is in my tree.

The only reason for the new_ prefixes and the config options was to
make it easy for people to test them.

I didn't sent it only as a bkpatch, a following mail had the patch in
diff format.

Anyway I'll repost them as per you suggestions...

Pantelis


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Paul Mackerras wrote:

Pantelis Antoniou writes:

quoted

+# Support new type of routines, usable from modules
+bool 'Use new type dpalloc routines()' CONFIG_NEW_DPALLOC
+bool 'Use new type hostalloc routines()' CONFIG_NEW_HOSTALLOC
+if [ "$CONFIG_NEW_DPALLOC" = "y" -o "$CONFIG_NEW_HOSTALLOC" = "y" ]; then
+  define_bool CONFIG_CPM_RHEAP y
+fi

I don't want to see config options that select between different
internal implementations of the same thing.  Either your new routines
are better, and we'll use them, or they are worse, and we'll use the
old ones.  Having a config option just leads to tons of ifdefs
throughout the code, which makes it harder to read and understand.
Having two implementations of the same thing is just bloat.

Similarly, I don't like the way all your new routines have a "new_"
prefix on the name.  You should be thinking of replacing the existing
routines rather than providing an alternative implementation with a
different name.  Where you have changed the API, either fix the
drivers or provide a compatibility routine.

The way it looks at the moment, it seems that you don't really have
the conviction that your code is better than what is there already.
Please redo your patch so that it just replaces the old routines.  And
please don't send it as a bkpatch since they are impossible to read, a
plain diff -u is much better.

Paul.

OK

Here is the updated patch in diff format as per your suggestions.

Any more suggestions?

Pantelis

On Fri, Dec 20, 2002 at 11:57:24AM +0200, Pantelis Antoniou wrote:

Here is the updated patch in diff format as per your suggestions.

Looks good.  My only real issue is that this is a big change, so I would
rather see either (or both) of the following happen:
Make a patch vs 2.5.  This will require additional 8xx work to get
things going again.
Please hold on this until all of the otherwise stable 8xx work can get
out of the linuxppc_2_4_devel tree and into linuxppc_2_4 / Marcelo.

Also, Dan Malek, any comments on all of this?

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Tom Rini wrote:

Also, Dan Malek, any comments on all of this?

I've had some life distractions for the past couple of weeks.
I'll have some time to look at this over the next few days.

When are we going to stop using 2.4 as our development tree?
Things like this should first be developed and proven in the 2.5
tree then moved into 2.4 after we agree it's necessary.

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Tom Rini wrote:

On Fri, Dec 20, 2002 at 11:57:24AM +0200, Pantelis Antoniou wrote:

quoted

Here is the updated patch in diff format as per your suggestions.

Looks good.  My only real issue is that this is a big change, so I would
rather see either (or both) of the following happen:
Make a patch vs 2.5.  This will require additional 8xx work to get
things going again.
Please hold on this until all of the otherwise stable 8xx work can get
out of the linuxppc_2_4_devel tree and into linuxppc_2_4 / Marcelo.

Also, Dan Malek, any comments on all of this?

Well, I'll try but I haven't messed with 2.5 yet.
Does it even work at all for 8xx?
Has anybody in the list worked with it?

I would very much like the two trees getting merged
finally, as I'm forced to maintaining my own tree,
forked off Marcelo.

Regards, and Merry Christmas to all

Pantelis

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Tue, Dec 24, 2002 at 09:01:27AM +0200, Pantelis Antoniou wrote:

Tom Rini wrote:

quoted

On Fri, Dec 20, 2002 at 11:57:24AM +0200, Pantelis Antoniou wrote:

quoted

Here is the updated patch in diff format as per your suggestions.

Looks good.  My only real issue is that this is a big change, so I would
rather see either (or both) of the following happen:
Make a patch vs 2.5.  This will require additional 8xx work to get
things going again.
Please hold on this until all of the otherwise stable 8xx work can get
out of the linuxppc_2_4_devel tree and into linuxppc_2_4 / Marcelo.

Also, Dan Malek, any comments on all of this?

Well, I'll try but I haven't messed with 2.5 yet.
Does it even work at all for 8xx?

No, it does not.  As I said above, it needs a few fixups here and there.

Has anybody in the list worked with it?

On all of the other types of boards, yes.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Mon, Dec 23, 2002 at 03:07:37PM -0500, Dan Malek wrote:

Tom Rini wrote:

quoted

Also, Dan Malek, any comments on all of this?

I've had some life distractions for the past couple of weeks.
I'll have some time to look at this over the next few days.

So, have you had time to look at all of this yet?

When are we going to stop using 2.4 as our development tree?
Things like this should first be developed and proven in the 2.5
tree then moved into 2.4 after we agree it's necessary.

Mainly because no one with the indepth knowledge of 8xx (*hint*hint* :))
has gone and fixed the very low level issues on 8xx which need to be
fixed, such as some MM thing or another from 2.5.5/2.5.10 timeframe and
the exception handling stuff ne3ds a look at, at a minimum based on what
Paul has done for 'classic' and what Paul and/or David and others have
done for 40x.  At which point the more trivial things like fixing up
drivers can happen. :)

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/