[PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

another compile fix, pulled straight from m68k CVS

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---

 drivers/macintosh/adb.c |    8 +++++---
 include/linux/adb.h     |    1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

d39fe026c456804272741a7c62ae380257e9a879
diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
index d2ead17..6b7591e 100644
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -476,13 +476,15 @@ adb_request(struct adb_request *req, voi
 		use_sreq = 1;
 	} else
 		use_sreq = 0;
-	req->nbytes = nbytes+1;
+	i = (flags & ADBREQ_RAW) ? 0 : 1;
+	req->nbytes = nbytes+i;
 	req->done = done;
 	req->reply_expected = flags & ADBREQ_REPLY;
 	req->data[0] = ADB_PACKET;
 	va_start(list, nbytes);
-	for (i = 0; i < nbytes; ++i)
-		req->data[i+1] = va_arg(list, int);
+	while (i < req->nbytes) {
+		req->data[i++] = va_arg(list, int);
+	}
 	va_end(list);
 
 	if (flags & ADBREQ_NOSEND)
diff --git a/include/linux/adb.h b/include/linux/adb.h
index e9fdc63..aad7b1c 100644
--- a/include/linux/adb.h
+++ b/include/linux/adb.h
@@ -76,6 +76,7 @@ struct adb_driver {
 #define ADBREQ_REPLY	1	/* expect reply */
 #define ADBREQ_SYNC	2	/* poll until done */
 #define ADBREQ_NOSEND	4	/* build the request, but don't send it */
+#define ADBREQ_RAW	8	/* send raw packet (don't prepend ADB_PACKET) */
 
 /* Messages sent thru the client_list notifier. You should NOT stop
    the operation, at least not with this version */
-- 
0.99.9.GIT

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Roman Zippel]

Hi,

On Thu, 15 Dec 2005, Al Viro wrote:

> another compile fix, pulled straight from m68k CVS

Thanks, but if you pull changes out of CVS could you please keep the 
author intact? CVS may be bad, but it's not that bad.

bye, Roman

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

On Thu, Dec 15, 2005 at 01:00:05PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Thu, 15 Dec 2005, Al Viro wrote:
> 
> > another compile fix, pulled straight from m68k CVS
> 
> Thanks, but if you pull changes out of CVS could you please keep the 
> author intact? CVS may be bad, but it's not that bad.

CVS users, OTOH...
   Mon Oct 22 09:34:34 2001 UTC (4 years, 1 month ago) by zippel
   Branches: MAIN
   CVS tags: m68k-2_5_9, m68k-2_5_8, m68k-2_5_7, m68k-2_5_65,
[snip]
   m68k-2_4_13, m68k-2_4_12
   Branch point for: m68k-2_4
   Diff to previous 1.1: preferred, colored
   Changes since revision 1.1: +1 -0 lines
import Geert's 2.4.12 m68k patch

and the same for drivers/macintosh part.

So who should I put as the author?  You or Geert (or whatever attributions
might have been in said big patch)?  Incidentally,  ADBREQ_RAW had leaked
into mainline (sans definition) in 2.3.45-pre2, which was Feb 13 2000, i.e.
more than 1.5 year before your commit, so there's quite a chunk of history
missing...

I'm serious, BTW - I certainly would have no problem preserving attribution,
but it simply hadn't been there.  CVS logs are only as good as the data
being put there by committers...

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

On Thu, Dec 15, 2005 at 05:16:45PM +0000, Al Viro wrote:
> So who should I put as the author?  You or Geert (or whatever attributions
> might have been in said big patch)?  Incidentally,  ADBREQ_RAW had leaked
> into mainline (sans definition) in 2.3.45-pre2, which was Feb 13 2000, i.e.
> more than 1.5 year before your commit, so there's quite a chunk of history
> missing...
> 
> I'm serious, BTW - I certainly would have no problem preserving attribution,
> but it simply hadn't been there.  CVS logs are only as good as the data
> being put there by committers...

With some archaeology...  It looks like drivers/macintosh part is from
Geert (with chunks from benh? not sure) circa Dec 2000; adb.h is a missing
piece of earlier patch (one that had leaked in Feb 2000, $DEITY knowns how
much older it is)...

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Roman Zippel]

Hi,

On Thu, 15 Dec 2005, Al Viro wrote:

> So who should I put as the author?  You or Geert (or whatever attributions
> might have been in said big patch)?  Incidentally,  ADBREQ_RAW had leaked
> into mainline (sans definition) in 2.3.45-pre2, which was Feb 13 2000, i.e.
> more than 1.5 year before your commit, so there's quite a chunk of history
> missing...

I'd say Geert, but it probably comes from the Mac tree. Anyway, it 
wouldn't be such a bad idea to ask him first why it's in his postponed 
queue:

http://linux-m68k-cvs.ubb.ca/~geert/linux-m68k-2.6.x-merging/POSTPONED/130-adbraw.diff

My guess it needs some ack from the ppc people.

bye, Roman

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Geert Uytterhoeven]

On Thu, 15 Dec 2005, Al Viro wrote:
> On Thu, Dec 15, 2005 at 05:16:45PM +0000, Al Viro wrote:
> > So who should I put as the author?  You or Geert (or whatever attributions
> > might have been in said big patch)?  Incidentally,  ADBREQ_RAW had leaked
> > into mainline (sans definition) in 2.3.45-pre2, which was Feb 13 2000, i.e.
> > more than 1.5 year before your commit, so there's quite a chunk of history
> > missing...
> > 
> > I'm serious, BTW - I certainly would have no problem preserving attribution,
> > but it simply hadn't been there.  CVS logs are only as good as the data
> > being put there by committers...
> 
> With some archaeology...  It looks like drivers/macintosh part is from
> Geert (with chunks from benh? not sure) circa Dec 2000; adb.h is a missing
> piece of earlier patch (one that had leaked in Feb 2000, $DEITY knowns how
> much older it is)...

Nah, I never did Mac development ;-)

IIRC, this change originally came from maclinux CVS (which predates Linux/m68k
CVS by several years) via email. If you wait a few hours, the original author
probably will start yelling ;-)

Alternatively, I can dig up my email archives from before 2000 to identify
him....

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

On Thu, Dec 15, 2005 at 06:51:40PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Thu, 15 Dec 2005, Al Viro wrote:
> 
> > So who should I put as the author?  You or Geert (or whatever attributions
> > might have been in said big patch)?  Incidentally,  ADBREQ_RAW had leaked
> > into mainline (sans definition) in 2.3.45-pre2, which was Feb 13 2000, i.e.
> > more than 1.5 year before your commit, so there's quite a chunk of history
> > missing...
> 
> I'd say Geert, but it probably comes from the Mac tree. Anyway, it 
> wouldn't be such a bad idea to ask him first why it's in his postponed 
> queue:
> 
> http://linux-m68k-cvs.ubb.ca/~geert/linux-m68k-2.6.x-merging/POSTPONED/130-adbraw.diff
> 
> My guess it needs some ack from the ppc people.

It doesn't - behaviour in case when ADBREQ_RAW is not passed in flags had
been obviously unchanged.  And only m68k passes ADBREQ_RAW in there.
So no, it doesn't affect ppc at all.

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Geert Uytterhoeven]

On Thu, 15 Dec 2005, Al Viro wrote:
> On Thu, Dec 15, 2005 at 06:51:40PM +0100, Roman Zippel wrote:
> > On Thu, 15 Dec 2005, Al Viro wrote:
> > > So who should I put as the author?  You or Geert (or whatever attributions
> > > might have been in said big patch)?  Incidentally,  ADBREQ_RAW had leaked
> > > into mainline (sans definition) in 2.3.45-pre2, which was Feb 13 2000, i.e.
> > > more than 1.5 year before your commit, so there's quite a chunk of history
> > > missing...
> > 
> > I'd say Geert, but it probably comes from the Mac tree. Anyway, it 
> > wouldn't be such a bad idea to ask him first why it's in his postponed 
> > queue:

Indeed, usually there's a good reason for being in that state instead of not
being merged ;-)

> > http://linux-m68k-cvs.ubb.ca/~geert/linux-m68k-2.6.x-merging/POSTPONED/130-adbraw.diff
> > 
> > My guess it needs some ack from the ppc people.
> 
> It doesn't - behaviour in case when ADBREQ_RAW is not passed in flags had
> been obviously unchanged.  And only m68k passes ADBREQ_RAW in there.
> So no, it doesn't affect ppc at all.

Even if behavior is unchanged, this doesn't mean that people like their code
being modified behind their back...

Anyway, last time I tried to bring this up with the union of Mac and PowerMac
guys, no one seemed to remember why ADBREQ_RAW was really needed...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

On Thu, Dec 15, 2005 at 07:00:54PM +0100, Geert Uytterhoeven wrote:
> Even if behavior is unchanged, this doesn't mean that people like their code
> being modified behind their back...
> 
> Anyway, last time I tried to bring this up with the union of Mac and PowerMac
> guys, no one seemed to remember why ADBREQ_RAW was really needed...

>From my reading of the code it's a way for mac/misc.c to send a packet that
starts with CUDA_PACKET or PMU_PACKET instead of ADB_PACKET, but otherwise
is the same as normal adb_request() ones...

Used for access to timer, nvram, etc. - looks like that puppy used to
use the same protocol for more than just ADB and the first byte of packet
really selects the destination...

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

On Thu, Dec 15, 2005 at 06:14:05PM +0000, Al Viro wrote:
> >From my reading of the code it's a way for mac/misc.c to send a packet that
> starts with CUDA_PACKET or PMU_PACKET instead of ADB_PACKET, but otherwise
> is the same as normal adb_request() ones...
> 
> Used for access to timer, nvram, etc. - looks like that puppy used to
> use the same protocol for more than just ADB and the first byte of packet
> really selects the destination...

After reading some more...  Is there any reason why mac/misc.c can't
simply use cuda_request() and pmu_request() instead?  At least for
read/write for time and nvram we end up with identical sequence of
operations anyway - if you expand the calls in
        adb_request((struct adb_request *) &req, NULL,
                        ADBREQ_RAW|ADBREQ_SYNC,
                        2, CUDA_PACKET, CUDA_GET_TIME);
[m68k]
and
        if (cuda_request(&req, NULL, 2, CUDA_PACKET, CUDA_GET_TIME) < 0)
		/* bail out */
        while (!req.complete)
                cuda_poll();
[ppc]
until you get to call of cuda_write(), you'll see the same code.  Come
to think of that...  Shouldn't the ifdefs for CONFIG_ADB_PMU in there be for
CONFIG_ADB_PMU68?  The former depends on PMAC_PPC, so it's not particulary
useful thing to check on m68k...

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Brad Boyer]

On Thu, Dec 15, 2005 at 06:58:29PM +0000, Al Viro wrote:
> On Thu, Dec 15, 2005 at 06:14:05PM +0000, Al Viro wrote:
> > >From my reading of the code it's a way for mac/misc.c to send a packet that
> > starts with CUDA_PACKET or PMU_PACKET instead of ADB_PACKET, but otherwise
> > is the same as normal adb_request() ones...
> > 
> > Used for access to timer, nvram, etc. - looks like that puppy used to
> > use the same protocol for more than just ADB and the first byte of packet
> > really selects the destination...
> 
> After reading some more...  Is there any reason why mac/misc.c can't
> simply use cuda_request() and pmu_request() instead?  At least for
> read/write for time and nvram we end up with identical sequence of
> operations anyway - if you expand the calls in
>         adb_request((struct adb_request *) &req, NULL,
>                         ADBREQ_RAW|ADBREQ_SYNC,
>                         2, CUDA_PACKET, CUDA_GET_TIME);
> [m68k]
> and
>         if (cuda_request(&req, NULL, 2, CUDA_PACKET, CUDA_GET_TIME) < 0)
> 		/* bail out */
>         while (!req.complete)
>                 cuda_poll();
> [ppc]
> until you get to call of cuda_write(), you'll see the same code.  Come
> to think of that...  Shouldn't the ifdefs for CONFIG_ADB_PMU in there be for
> CONFIG_ADB_PMU68?  The former depends on PMAC_PPC, so it's not particulary
> useful thing to check on m68k...

Honestly, the RTC/PRAM code is completely broken in 2.6 for mac68k anyway.
We definitely shouldn't be using CONFIG_ADB_PMU in the m68k code. I suspect
that wasn't found due to the fact that the pmu68k driver has never been
reliable enough to use, so everyone blames that for stuff breaking. Changing
that use of CONFIG_ADB_PMU to CONFIG_ADB_PMU68K seems like the correct
thing to do in this case.

I would like to stop using adb_request in mac/misc.c as well, but it's not
as simple as just changing it to use cuda_request and pmu_request. That
should do it for the cuda and pmu based models, but the egret (Mac IIsi
and friends) based models get left out by that fix. If noone else looks
at it before me, I'll check this out after I fix some other stuff related
to m68k mac support.

	Brad Boyer
	flar@allandria.com

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Finn Thain]

On Thu, 15 Dec 2005, Al Viro wrote:

> With some archaeology...  It looks like drivers/macintosh part is from
> Geert (with chunks from benh? not sure) circa Dec 2000; adb.h is a missing
> piece of earlier patch (one that had leaked in Feb 2000, $DEITY knowns how
> much older it is)...


> --- a/drivers/macintosh/adb.c
> +++ b/drivers/macintosh/adb.c
> @@ -476,13 +476,15 @@ adb_request(struct adb_request *req, voi
>                 use_sreq = 1;
>         } else
>                 use_sreq = 0;
> -       req->nbytes = nbytes+1;
> +       i = (flags & ADBREQ_RAW) ? 0 : 1;
> +       req->nbytes = nbytes+i;
>         req->done = done;
>         req->reply_expected = flags & ADBREQ_REPLY;
>         req->data[0] = ADB_PACKET;
>         va_start(list, nbytes);
> -       for (i = 0; i < nbytes; ++i)
> -               req->data[i+1] = va_arg(list, int);
> +       while (i < req->nbytes) {
> +               req->data[i++] = va_arg(list, int);
> +       }

According to the the Linx-mac68k repo, one line of that change belongs to 
Joshua M. Thompson, while the rest of that code was committed by Ray 
Knight, (the changelog says it was a merge from 2.4.0).

http://cvs.sourceforge.net/viewcvs.py/linux-mac68k/linux-mac68k/drivers/macintosh/adb.c?annotate=1.25


>         va_end(list);
>  
>         if (flags & ADBREQ_NOSEND)
> diff --git a/include/linux/adb.h b/include/linux/adb.h
> index e9fdc63..aad7b1c 100644
> --- a/include/linux/adb.h
> +++ b/include/linux/adb.h
> @@ -76,6 +76,7 @@ struct adb_driver {
>  #define ADBREQ_REPLY   1       /* expect reply */
>  #define ADBREQ_SYNC    2       /* poll until done */
>  #define ADBREQ_NOSEND  4       /* build the request, but don't send it */
> +#define ADBREQ_RAW     8       /* send raw packet (don't prepend 
> ADB_PACKET) */
> 

Credit for that change is due to Joshua M. Thompson.

http://cvs.sourceforge.net/viewcvs.py/linux-mac68k/linux-mac68k/include/linux/adb.h?annotate=1.7

-f

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Al Viro]

On Thu, Dec 15, 2005 at 12:05:21PM -0800, Brad Boyer wrote:
> I would like to stop using adb_request in mac/misc.c as well, but it's not
> as simple as just changing it to use cuda_request and pmu_request. That
> should do it for the cuda and pmu based models, but the egret (Mac IIsi
> and friends) based models get left out by that fix. If noone else looks
> at it before me, I'll check this out after I fix some other stuff related
> to m68k mac support.

OK...  AFAICS, patch I've just posted to l-k and linux-m68k (subject:
[PATCH 04/36] m68k: switch mac/misc.c to direct use of appropriate
cuda/pmu/maciisi requests) should so it.  Do you have any problems with
that variant?

Re: [PATCH 2/3] m68k: compile fix - ADBREQ_RAW missing declaration

[Brad Boyer]

On Thu, Dec 22, 2005 at 05:06:55AM +0000, Al Viro wrote:
> On Thu, Dec 15, 2005 at 12:05:21PM -0800, Brad Boyer wrote:
> > I would like to stop using adb_request in mac/misc.c as well, but it's not
> > as simple as just changing it to use cuda_request and pmu_request. That
> > should do it for the cuda and pmu based models, but the egret (Mac IIsi
> > and friends) based models get left out by that fix. If noone else looks
> > at it before me, I'll check this out after I fix some other stuff related
> > to m68k mac support.
> 
> OK...  AFAICS, patch I've just posted to l-k and linux-m68k (subject:
> [PATCH 04/36] m68k: switch mac/misc.c to direct use of appropriate
> cuda/pmu/maciisi requests) should so it.  Do you have any problems with
> that variant?

I don't have any big issues with it. However, I am not at home right now
and can't actually try it out on real hardware. It looks like it should
work as well as the version it is replacing. I'm pretty sure the IIsi
case was broken before. I think we should go ahead and commit this to
get rid of the issues it fixes, and then come back to it later and take
another pass at it to fix the fact that it has never worked in 2.6 on
some models. It's better than nothing.

	Brad Boyer
	flar@allandria.com