2008-02-09 00:04:17

by Adrian Bunk

[permalink] [raw]
Subject: scsi/arm/fas216.c compile error

Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following
compile error:

<-- snip -->

...
CC drivers/scsi/arm/fas216.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done':
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen'
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg'
make[4]: *** [drivers/scsi/arm/fas216.o] Error 1

<-- snip -->

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed


2008-02-10 13:08:29

by Boaz Harrosh

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sat, Feb 09 2008 at 2:04 +0200, Adrian Bunk <[email protected]> wrote:
> Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following
> compile error:
>
> <-- snip -->
>
> ...
> CC drivers/scsi/arm/fas216.o
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done':
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen'
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg'
> make[4]: *** [drivers/scsi/arm/fas216.o] Error 1
>
> <-- snip -->
>
> cu
> Adrian
>
It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
[SCSI] arm: convert to accessors and !use_sg cleanup

Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
we finally managed a Tested-by:.

I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
schedule.

Boaz

2008-02-10 13:53:44

by Russell King

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
> On Sat, Feb 09 2008 at 2:04 +0200, Adrian Bunk <[email protected]> wrote:
> > Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following
> > compile error:
> >
> > <-- snip -->
> >
> > ...
> > CC drivers/scsi/arm/fas216.o
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done':
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen'
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg'
> > make[4]: *** [drivers/scsi/arm/fas216.o] Error 1
> >
> > <-- snip -->
> >
> > cu
> > Adrian
> >
> It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
> [SCSI] arm: convert to accessors and !use_sg cleanup
>
> Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
> to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
> we finally managed a Tested-by:.
>
> I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
> schedule.

Oh, just 20 minutes in your opinion? Reality works at a different tick
rate to your timing then.

However, if you're seeing a bulid error, it means that the *WRONG* patch
went in. No idea why I even bothered to test it if that's what happens.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2008-02-10 13:59:22

by Adrian Bunk

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
> On Sat, Feb 09 2008 at 2:04 +0200, Adrian Bunk <[email protected]> wrote:
> > Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following
> > compile error:
> >
> > <-- snip -->
> >
> > ...
> > CC drivers/scsi/arm/fas216.o
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done':
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen'
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg'
> > make[4]: *** [drivers/scsi/arm/fas216.o] Error 1
> >
> > <-- snip -->
> >
> > cu
> > Adrian
> >
> It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
> [SCSI] arm: convert to accessors and !use_sg cleanup
>...

fas216.c != acornscsi.c

> Boaz

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-02-10 13:59:41

by Russell King

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
> It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
> [SCSI] arm: convert to accessors and !use_sg cleanup
>
> Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
> to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
> we finally managed a Tested-by:.
>
> I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
> schedule.

Oh, I was ill for most of December, particularly at the time that you
sent the patch, and by the time I recovered, it was buried in my mailbox.

Suggest you have some consideration for others who might not be able to
do your beg and call at the immediate moment that you want it, and
consider that their email management skills may not be as l33t as yours.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2008-02-10 14:16:28

by Boaz Harrosh

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, Feb 10 2008 at 15:58 +0200, Russell King <[email protected]> wrote:
> On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
>> It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
>> [SCSI] arm: convert to accessors and !use_sg cleanup
>>
>> Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
>> to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
>> we finally managed a Tested-by:.
>>
>> I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
>> schedule.
>
> Oh, I was ill for most of December, particularly at the time that you
> sent the patch, and by the time I recovered, it was buried in my mailbox.
>
> Suggest you have some consideration for others who might not be able to
> do your beg and call at the immediate moment that you want it, and
> consider that their email management skills may not be as l33t as yours.
>
Dear Russell.
You are right. I apologize. I was too trigger happy.
I should have resend the request. The patches were in scsi-pending and
in -mm. And I assumed it was all good. But it was not accepted into
scsi-misc and was somewhat forgotten.

I assure you, my email management skills are just as laking as yours.
Just that my responsibility's are few.

Boaz

2008-02-10 14:20:37

by James Bottomley

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error


On Sun, 2008-02-10 at 13:58 +0000, Russell King wrote:
> On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
> > It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
> > [SCSI] arm: convert to accessors and !use_sg cleanup
> >
> > Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
> > to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
> > we finally managed a Tested-by:.
> >
> > I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
> > schedule.
>
> Oh, I was ill for most of December, particularly at the time that you
> sent the patch, and by the time I recovered, it was buried in my mailbox.
>
> Suggest you have some consideration for others who might not be able to
> do your beg and call at the immediate moment that you want it, and
> consider that their email management skills may not be as l33t as yours.

OK, sorry about this, it's a bit of a cockup all around. The patch that
fixes this problem is still in SCSI pending largely because it's patch
description:

[SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation

Doesn't lead one to think it might be build critical, so I concentrated
on getting the other arm patch out.

Russell, could you give it a quick test, and I'll put it in with a
tested-by tag?

Thanks,

James

---

From: Boaz Harrosh <[email protected]>
Date: Mon, 10 Sep 2007 22:39:11 +0300
Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation

- Use new scsi_eh_prep/restor_cmnd() for synchronous
REQUEST_SENSE invocation.

Signed-off-by: Boaz Harrosh <[email protected]>
Cc: Russell King <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
---
drivers/scsi/arm/fas216.c | 16 +++-------------
drivers/scsi/arm/fas216.h | 3 +++
2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index fb5f202..a715632 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
* the upper layers to process. This would have been set
* correctly by fas216_std_done.
*/
+ scsi_eh_restore_cmnd(SCpnt, &info->ses);
SCpnt->scsi_done(SCpnt);
}

@@ -2103,23 +2104,12 @@ request_sense:
if (SCpnt->cmnd[0] == REQUEST_SENSE)
goto done;

+ scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
"requesting sense");
- memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
- SCpnt->cmnd[0] = REQUEST_SENSE;
- SCpnt->cmnd[1] = SCpnt->device->lun << 5;
- SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
- SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
- SCpnt->SCp.buffer = NULL;
- SCpnt->SCp.buffers_residual = 0;
- SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;
- SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
- SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
+ init_SCp(SCpnt);
SCpnt->SCp.Message = 0;
SCpnt->SCp.Status = 0;
- SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
- SCpnt->sc_data_direction = DMA_FROM_DEVICE;
- SCpnt->use_sg = 0;
SCpnt->tag = 0;
SCpnt->host_scribble = (void *)fas216_rq_sns_done;

diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
index 00e5f05..3e73e26 100644
--- a/drivers/scsi/arm/fas216.h
+++ b/drivers/scsi/arm/fas216.h
@@ -16,6 +16,8 @@
#define NO_IRQ 255
#endif

+#include <scsi/scsi_eh.h>
+
#include "queue.h"
#include "msgqueue.h"

@@ -311,6 +313,7 @@ typedef struct {

/* miscellaneous */
int internal_done; /* flag to indicate request done */
+ struct scsi_eh_save *ses; /* holds request sense restore info */
unsigned long magic_end;
} FAS216_Info;

--
1.5.3.8


2008-02-10 14:38:19

by Boaz Harrosh

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, Feb 10 2008 at 16:20 +0200, James Bottomley <[email protected]> wrote:
> On Sun, 2008-02-10 at 13:58 +0000, Russell King wrote:
>> On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
>>> It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
>>> [SCSI] arm: convert to accessors and !use_sg cleanup
>>>
>>> Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
>>> to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
>>> we finally managed a Tested-by:.
>>>
>>> I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
>>> schedule.
>> Oh, I was ill for most of December, particularly at the time that you
>> sent the patch, and by the time I recovered, it was buried in my mailbox.
>>
>> Suggest you have some consideration for others who might not be able to
>> do your beg and call at the immediate moment that you want it, and
>> consider that their email management skills may not be as l33t as yours.
>
> OK, sorry about this, it's a bit of a cockup all around. The patch that
> fixes this problem is still in SCSI pending largely because it's patch
> description:
>
> [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
>
> Doesn't lead one to think it might be build critical, so I concentrated
> on getting the other arm patch out.
>
All the patches I pushed are build critical. The complete
"Use scsi_eh API for REQUEST_SENSE" and the error refactoring patches
were in support for the scsi_data_buffer effort. Well that was the last
one so all is well I guess.

(With out these patches, code is still pushing none-use_sg requests,
apart from the members rename of scsi_cmnd)

> Russell, could you give it a quick test, and I'll put it in with a
> tested-by tag?
>
> Thanks,
>
> James
>

Thanks to all
Boaz

2008-02-10 22:02:46

by Russell King

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, Feb 10, 2008 at 08:20:24AM -0600, James Bottomley wrote:
>
> On Sun, 2008-02-10 at 13:58 +0000, Russell King wrote:
> > On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
> > > It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
> > > [SCSI] arm: convert to accessors and !use_sg cleanup
> > >
> > > Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
> > > to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
> > > we finally managed a Tested-by:.
> > >
> > > I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
> > > schedule.
> >
> > Oh, I was ill for most of December, particularly at the time that you
> > sent the patch, and by the time I recovered, it was buried in my mailbox.
> >
> > Suggest you have some consideration for others who might not be able to
> > do your beg and call at the immediate moment that you want it, and
> > consider that their email management skills may not be as l33t as yours.
>
> OK, sorry about this, it's a bit of a cockup all around. The patch that
> fixes this problem is still in SCSI pending largely because it's patch
> description:
>
> [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
>
> Doesn't lead one to think it might be build critical, so I concentrated
> on getting the other arm patch out.
>
> Russell, could you give it a quick test, and I'll put it in with a
> tested-by tag?

It's not looking good:

CC drivers/scsi/arm/fas216.o
drivers/scsi/arm/fas216.c: In function `fas216_rq_sns_done':
drivers/scsi/arm/fas216.c:2021: warning: passing arg 2 of `scsi_eh_restore_cmnd' from incompatible pointer type
drivers/scsi/arm/fas216.c: In function `fas216_std_done':
drivers/scsi/arm/fas216.c:2107: warning: passing arg 2 of `scsi_eh_prep_cmnd' from incompatible pointer type

Since the second argument of scsi_eh_prep_cmnd is 'struct scsi_eh_save *ses'
this patch is most definitely bad. Not even booted it.

>
> Thanks,
>
> James
>
> ---
>
> From: Boaz Harrosh <[email protected]>
> Date: Mon, 10 Sep 2007 22:39:11 +0300
> Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
>
> - Use new scsi_eh_prep/restor_cmnd() for synchronous
> REQUEST_SENSE invocation.
>
> Signed-off-by: Boaz Harrosh <[email protected]>
> Cc: Russell King <[email protected]>
> Signed-off-by: James Bottomley <[email protected]>
> ---
> drivers/scsi/arm/fas216.c | 16 +++-------------
> drivers/scsi/arm/fas216.h | 3 +++
> 2 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
> index fb5f202..a715632 100644
> --- a/drivers/scsi/arm/fas216.c
> +++ b/drivers/scsi/arm/fas216.c
> @@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
> * the upper layers to process. This would have been set
> * correctly by fas216_std_done.
> */
> + scsi_eh_restore_cmnd(SCpnt, &info->ses);
> SCpnt->scsi_done(SCpnt);
> }
>
> @@ -2103,23 +2104,12 @@ request_sense:
> if (SCpnt->cmnd[0] == REQUEST_SENSE)
> goto done;
>
> + scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
> fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
> "requesting sense");
> - memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
> - SCpnt->cmnd[0] = REQUEST_SENSE;
> - SCpnt->cmnd[1] = SCpnt->device->lun << 5;
> - SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
> - SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
> - SCpnt->SCp.buffer = NULL;
> - SCpnt->SCp.buffers_residual = 0;
> - SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;
> - SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
> - SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
> + init_SCp(SCpnt);
> SCpnt->SCp.Message = 0;
> SCpnt->SCp.Status = 0;
> - SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
> - SCpnt->sc_data_direction = DMA_FROM_DEVICE;
> - SCpnt->use_sg = 0;
> SCpnt->tag = 0;
> SCpnt->host_scribble = (void *)fas216_rq_sns_done;
>
> diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
> index 00e5f05..3e73e26 100644
> --- a/drivers/scsi/arm/fas216.h
> +++ b/drivers/scsi/arm/fas216.h
> @@ -16,6 +16,8 @@
> #define NO_IRQ 255
> #endif
>
> +#include <scsi/scsi_eh.h>
> +
> #include "queue.h"
> #include "msgqueue.h"
>
> @@ -311,6 +313,7 @@ typedef struct {
>
> /* miscellaneous */
> int internal_done; /* flag to indicate request done */
> + struct scsi_eh_save *ses; /* holds request sense restore info */

Looks to me like this line has a stray '*' on?

> unsigned long magic_end;
> } FAS216_Info;

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2008-02-10 22:44:34

by James Bottomley

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Sun, 2008-02-10 at 22:02 +0000, Russell King wrote:
> On Sun, Feb 10, 2008 at 08:20:24AM -0600, James Bottomley wrote:
> >
> > On Sun, 2008-02-10 at 13:58 +0000, Russell King wrote:
> > > On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
> > > > It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
> > > > [SCSI] arm: convert to accessors and !use_sg cleanup
> > > >
> > > > Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
> > > > to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
> > > > we finally managed a Tested-by:.
> > > >
> > > > I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
> > > > schedule.
> > >
> > > Oh, I was ill for most of December, particularly at the time that you
> > > sent the patch, and by the time I recovered, it was buried in my mailbox.
> > >
> > > Suggest you have some consideration for others who might not be able to
> > > do your beg and call at the immediate moment that you want it, and
> > > consider that their email management skills may not be as l33t as yours.
> >
> > OK, sorry about this, it's a bit of a cockup all around. The patch that
> > fixes this problem is still in SCSI pending largely because it's patch
> > description:
> >
> > [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
> >
> > Doesn't lead one to think it might be build critical, so I concentrated
> > on getting the other arm patch out.
> >
> > Russell, could you give it a quick test, and I'll put it in with a
> > tested-by tag?
>
> It's not looking good:
>
> CC drivers/scsi/arm/fas216.o
> drivers/scsi/arm/fas216.c: In function `fas216_rq_sns_done':
> drivers/scsi/arm/fas216.c:2021: warning: passing arg 2 of `scsi_eh_restore_cmnd' from incompatible pointer type
> drivers/scsi/arm/fas216.c: In function `fas216_std_done':
> drivers/scsi/arm/fas216.c:2107: warning: passing arg 2 of `scsi_eh_prep_cmnd' from incompatible pointer type
>
> Since the second argument of scsi_eh_prep_cmnd is 'struct scsi_eh_save *ses'
> this patch is most definitely bad. Not even booted it.

Yes, there looks to be a fatal screw up in the definition in
FAS216_Info. Could you try this ... I think I've corrected it.

James

---

>From 35be7297dc581b42c05ea7751ee595b0ce78f669 Mon Sep 17 00:00:00 2001
From: Boaz Harrosh <[email protected]>
Date: Mon, 10 Sep 2007 22:39:11 +0300
Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation

- Use new scsi_eh_prep/restor_cmnd() for synchronous
REQUEST_SENSE invocation.

Signed-off-by: Boaz Harrosh <[email protected]>
Cc: Russell King <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
---
drivers/scsi/arm/fas216.c | 16 +++-------------
drivers/scsi/arm/fas216.h | 3 +++
2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index fb5f202..a715632 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
* the upper layers to process. This would have been set
* correctly by fas216_std_done.
*/
+ scsi_eh_restore_cmnd(SCpnt, &info->ses);
SCpnt->scsi_done(SCpnt);
}

@@ -2103,23 +2104,12 @@ request_sense:
if (SCpnt->cmnd[0] == REQUEST_SENSE)
goto done;

+ scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
"requesting sense");
- memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
- SCpnt->cmnd[0] = REQUEST_SENSE;
- SCpnt->cmnd[1] = SCpnt->device->lun << 5;
- SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
- SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
- SCpnt->SCp.buffer = NULL;
- SCpnt->SCp.buffers_residual = 0;
- SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;
- SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
- SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
+ init_SCp(SCpnt);
SCpnt->SCp.Message = 0;
SCpnt->SCp.Status = 0;
- SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
- SCpnt->sc_data_direction = DMA_FROM_DEVICE;
- SCpnt->use_sg = 0;
SCpnt->tag = 0;
SCpnt->host_scribble = (void *)fas216_rq_sns_done;

diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
index 00e5f05..b65f4cf 100644
--- a/drivers/scsi/arm/fas216.h
+++ b/drivers/scsi/arm/fas216.h
@@ -16,6 +16,8 @@
#define NO_IRQ 255
#endif

+#include <scsi/scsi_eh.h>
+
#include "queue.h"
#include "msgqueue.h"

@@ -311,6 +313,7 @@ typedef struct {

/* miscellaneous */
int internal_done; /* flag to indicate request done */
+ struct scsi_eh_save ses; /* holds request sense restore info */
unsigned long magic_end;
} FAS216_Info;

--
1.5.3.8


2008-02-11 09:49:18

by Boaz Harrosh

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Mon, Feb 11 2008 at 0:44 +0200, James Bottomley <[email protected]> wrote:
> On Sun, 2008-02-10 at 22:02 +0000, Russell King wrote:
>> On Sun, Feb 10, 2008 at 08:20:24AM -0600, James Bottomley wrote:
>>> On Sun, 2008-02-10 at 13:58 +0000, Russell King wrote:
>>>> On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
>>>>> It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
>>>>> [SCSI] arm: convert to accessors and !use_sg cleanup
>>>>>
>>>>> Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable
>>>>> to get a responsive maintainer to ACK on them. until the breakage cause went into mainline
>>>>> we finally managed a Tested-by:.
>>>>>
>>>>> I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're
>>>>> schedule.
>>>> Oh, I was ill for most of December, particularly at the time that you
>>>> sent the patch, and by the time I recovered, it was buried in my mailbox.
>>>>
>>>> Suggest you have some consideration for others who might not be able to
>>>> do your beg and call at the immediate moment that you want it, and
>>>> consider that their email management skills may not be as l33t as yours.
>>> OK, sorry about this, it's a bit of a cockup all around. The patch that
>>> fixes this problem is still in SCSI pending largely because it's patch
>>> description:
>>>
>>> [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
>>>
>>> Doesn't lead one to think it might be build critical, so I concentrated
>>> on getting the other arm patch out.
>>>
>>> Russell, could you give it a quick test, and I'll put it in with a
>>> tested-by tag?
>> It's not looking good:
>>
>> CC drivers/scsi/arm/fas216.o
>> drivers/scsi/arm/fas216.c: In function `fas216_rq_sns_done':
>> drivers/scsi/arm/fas216.c:2021: warning: passing arg 2 of `scsi_eh_restore_cmnd' from incompatible pointer type
>> drivers/scsi/arm/fas216.c: In function `fas216_std_done':
>> drivers/scsi/arm/fas216.c:2107: warning: passing arg 2 of `scsi_eh_prep_cmnd' from incompatible pointer type
>>
>> Since the second argument of scsi_eh_prep_cmnd is 'struct scsi_eh_save *ses'
>> this patch is most definitely bad. Not even booted it.
>
> Yes, there looks to be a fatal screw up in the definition in
> FAS216_Info. Could you try this ... I think I've corrected it.
>
> James
>
> ---
>
>>From 35be7297dc581b42c05ea7751ee595b0ce78f669 Mon Sep 17 00:00:00 2001
> From: Boaz Harrosh <[email protected]>
> Date: Mon, 10 Sep 2007 22:39:11 +0300
> Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
>
> - Use new scsi_eh_prep/restor_cmnd() for synchronous
> REQUEST_SENSE invocation.
>
> Signed-off-by: Boaz Harrosh <[email protected]>
> Cc: Russell King <[email protected]>
> Signed-off-by: James Bottomley <[email protected]>
> ---
> drivers/scsi/arm/fas216.c | 16 +++-------------
> drivers/scsi/arm/fas216.h | 3 +++
> 2 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
> index fb5f202..a715632 100644
> --- a/drivers/scsi/arm/fas216.c
> +++ b/drivers/scsi/arm/fas216.c
> @@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
> * the upper layers to process. This would have been set
> * correctly by fas216_std_done.
> */
> + scsi_eh_restore_cmnd(SCpnt, &info->ses);
> SCpnt->scsi_done(SCpnt);
> }
>
> @@ -2103,23 +2104,12 @@ request_sense:
> if (SCpnt->cmnd[0] == REQUEST_SENSE)
> goto done;
>
> + scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
> fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
> "requesting sense");
> - memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
> - SCpnt->cmnd[0] = REQUEST_SENSE;
> - SCpnt->cmnd[1] = SCpnt->device->lun << 5;
> - SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
> - SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
> - SCpnt->SCp.buffer = NULL;
> - SCpnt->SCp.buffers_residual = 0;
> - SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;
> - SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
> - SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
> + init_SCp(SCpnt);
> SCpnt->SCp.Message = 0;
> SCpnt->SCp.Status = 0;
> - SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
> - SCpnt->sc_data_direction = DMA_FROM_DEVICE;
> - SCpnt->use_sg = 0;
> SCpnt->tag = 0;
> SCpnt->host_scribble = (void *)fas216_rq_sns_done;
>
> diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
> index 00e5f05..b65f4cf 100644
> --- a/drivers/scsi/arm/fas216.h
> +++ b/drivers/scsi/arm/fas216.h
> @@ -16,6 +16,8 @@
> #define NO_IRQ 255
> #endif
>
> +#include <scsi/scsi_eh.h>
> +
> #include "queue.h"
> #include "msgqueue.h"
>
> @@ -311,6 +313,7 @@ typedef struct {
>
> /* miscellaneous */
> int internal_done; /* flag to indicate request done */
> + struct scsi_eh_save ses; /* holds request sense restore info */
> unsigned long magic_end;
> } FAS216_Info;
>

Yes the pointer thing, Thanks James.

Andrew this patch was in -mm for two month or so. I was under the impression
that you have an arm cross compiler that tries to build every -mm kernel.
Is it possible that for some reason this portion did not get compiled?
Is there a place that one can inspect the output of -mm compilations, Specially
for cross compiled ARCHs?

Boaz

2008-02-11 10:03:47

by Russell King

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Mon, Feb 11, 2008 at 11:47:57AM +0200, Boaz Harrosh wrote:
> On Mon, Feb 11 2008 at 0:44 +0200, James Bottomley <[email protected]> wrote:
> Andrew this patch was in -mm for two month or so. I was under the impression
> that you have an arm cross compiler that tries to build every -mm kernel.
> Is it possible that for some reason this portion did not get compiled?
> Is there a place that one can inspect the output of -mm compilations, Specially
> for cross compiled ARCHs?

Having a patch sit in -mm for ARM doesn't mean a lot since there's no
guarantee that it'll get built, and that is because the ARM architecture
is very diverse; it's not possible to build a single kernel to support
everything.

So, when akpm builds a kernel for ARM, it's normally centered around one
particular ARM defconfig (maybe with allyconfig or allmodconfig afterwards)
but even that won't build all ARM specific code.

This is why we now have kautobuild - http://armlinux.simtec.co.uk/kautobuild/
That's the only way we can get decent compilation coverage.

That system isn't publically accessible (it's not even accessible to me)
and it only builds the mainline kernels. Adding -mm to it might be
possible, but as I understand the situation, even though it uses things
like ccache, it can take about 10 or so hours to complete a set of builds.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2008-02-11 10:18:43

by Boaz Harrosh

[permalink] [raw]
Subject: Re: scsi/arm/fas216.c compile error

On Mon, Feb 11 2008 at 12:02 +0200, Russell King <[email protected]> wrote:
> On Mon, Feb 11, 2008 at 11:47:57AM +0200, Boaz Harrosh wrote:
>> On Mon, Feb 11 2008 at 0:44 +0200, James Bottomley <[email protected]> wrote:
>> Andrew this patch was in -mm for two month or so. I was under the impression
>> that you have an arm cross compiler that tries to build every -mm kernel.
>> Is it possible that for some reason this portion did not get compiled?
>> Is there a place that one can inspect the output of -mm compilations, Specially
>> for cross compiled ARCHs?
>
> Having a patch sit in -mm for ARM doesn't mean a lot since there's no
> guarantee that it'll get built, and that is because the ARM architecture
> is very diverse; it's not possible to build a single kernel to support
> everything.
>
> So, when akpm builds a kernel for ARM, it's normally centered around one
> particular ARM defconfig (maybe with allyconfig or allmodconfig afterwards)
> but even that won't build all ARM specific code.
>
> This is why we now have kautobuild - http://armlinux.simtec.co.uk/kautobuild/
> That's the only way we can get decent compilation coverage.
>
> That system isn't publically accessible (it's not even accessible to me)
> and it only builds the mainline kernels. Adding -mm to it might be
> possible, but as I understand the situation, even though it uses things
> like ccache, it can take about 10 or so hours to complete a set of builds.
>
Thanks Russell. That explains it.

I wish they would include an -mm kernel once in a while. like 2-3 every kernel
cycle. It is much nicer to find the problems before they are already in mainline.
I would certainly sleep better. You have my vote.

Boaz