* [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
@ 2007-02-10 17:46 Richard Knutsson
2007-02-10 18:27 ` James Bottomley
0 siblings, 1 reply; 13+ messages in thread
From: Richard Knutsson @ 2007-02-10 17:46 UTC (permalink / raw)
To: James.Bottomley; +Cc: linux-kernel, linux-scsi, Richard Knutsson
Convert:
FALSE -> false
TRUE -> true
Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---
Compile-tested with "allyes", "allmod" & "allno" on i386
Whitespace cleaning on affected lines
drivers/scsi/aic7xxx_old.c | 242 +++++++++++++++-----------------
drivers/scsi/aic7xxx_old/aic7xxx_proc.c | 2
2 files changed, 119 insertions(+), 125 deletions(-)
diff --git a/drivers/scsi/aic7xxx_old.c b/drivers/scsi/aic7xxx_old.c
index 7d1fec6..92e32ee 100644
--- a/drivers/scsi/aic7xxx_old.c
+++ b/drivers/scsi/aic7xxx_old.c
@@ -256,12 +256,6 @@
#define ALL_LUNS -1
#define MAX_TARGETS 16
#define MAX_LUNS 8
-#ifndef TRUE
-# define TRUE 1
-#endif
-#ifndef FALSE
-# define FALSE 0
-#endif
#if defined(__powerpc__) || defined(__i386__) || defined(__x86_64__)
# define MMAPIO
@@ -1383,7 +1377,7 @@ aic7xxx_setup(char *s)
char *tok, *tok_end, *tok_end2;
char tok_list[] = { '.', ',', '{', '}', '\0' };
int i, instance = -1, device = -1;
- unsigned char done = FALSE;
+ unsigned char done = false;
base = p;
tok = base + n + 1; /* Forward us just past the ':' */
@@ -1411,14 +1405,14 @@ aic7xxx_setup(char *s)
case ',':
case '.':
if (instance == -1)
- done = TRUE;
+ done = true;
else if (device >= 0)
device++;
else if (instance >= 0)
instance++;
if ( (device >= MAX_TARGETS) ||
(instance >= ARRAY_SIZE(aic7xxx_tag_info)) )
- done = TRUE;
+ done = true;
tok++;
if (!done)
{
@@ -1426,10 +1420,10 @@ aic7xxx_setup(char *s)
}
break;
case '\0':
- done = TRUE;
+ done = true;
break;
default:
- done = TRUE;
+ done = true;
tok_end = strchr(tok, '\0');
for(i=0; tok_list[i]; i++)
{
@@ -1437,7 +1431,7 @@ aic7xxx_setup(char *s)
if ( (tok_end2) && (tok_end2 < tok_end) )
{
tok_end = tok_end2;
- done = FALSE;
+ done = false;
}
}
if ( (instance >= 0) && (device >= 0) &&
@@ -1772,7 +1766,7 @@ aic7xxx_loadseq(struct aic7xxx_host *p)
aic_outb(p, 0, SEQADDR0);
aic_outb(p, 0, SEQADDR1);
aic_outb(p, FASTMODE | FAILDIS, SEQCTL);
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
mdelay(1);
pause_sequencer(p);
aic_outb(p, FASTMODE, SEQCTL);
@@ -1821,7 +1815,7 @@ aic7xxx_print_sequencer(struct aic7xxx_host *p, int downloaded)
aic_outb(p, 0, SEQADDR0);
aic_outb(p, 0, SEQADDR1);
aic_outb(p, FASTMODE | FAILDIS, SEQCTL);
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
mdelay(1);
pause_sequencer(p);
aic_outb(p, FASTMODE, SEQCTL);
@@ -1869,7 +1863,7 @@ aic7xxx_find_syncrate(struct aic7xxx_host *p, unsigned int *period,
unsigned int maxsync, unsigned char *options)
{
struct aic7xxx_syncrate *syncrate;
- int done = FALSE;
+ int done = false;
switch(*options)
{
@@ -1925,7 +1919,7 @@ aic7xxx_find_syncrate(struct aic7xxx_host *p, unsigned int *period,
case MSG_EXT_PPR_OPTION_DT_UNITS:
if(!(syncrate->sxfr_ultra2 & AHC_SYNCRATE_CRC))
{
- done = TRUE;
+ done = true;
/*
* oops, we went too low for the CRC/DualEdge signalling, so
* clear the options byte
@@ -1939,7 +1933,7 @@ aic7xxx_find_syncrate(struct aic7xxx_host *p, unsigned int *period,
}
else
{
- done = TRUE;
+ done = true;
if(syncrate == &aic7xxx_syncrates[maxsync])
{
*period = syncrate->period;
@@ -1949,7 +1943,7 @@ aic7xxx_find_syncrate(struct aic7xxx_host *p, unsigned int *period,
default:
if(!(syncrate->sxfr_ultra2 & AHC_SYNCRATE_CRC))
{
- done = TRUE;
+ done = true;
if(syncrate == &aic7xxx_syncrates[maxsync])
{
*period = syncrate->period;
@@ -2721,7 +2715,7 @@ aic7xxx_done(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
if ((scb->flags & SCB_MSGOUT_BITS) != 0)
{
unsigned short mask;
- int message_error = FALSE;
+ int message_error = false;
mask = 0x01 << tindex;
@@ -2733,7 +2727,7 @@ aic7xxx_done(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
((scb->cmd->sense_buffer[12] == 0x43) || /* INVALID_MESSAGE */
(scb->cmd->sense_buffer[12] == 0x49))) /* MESSAGE_ERROR */
{
- message_error = TRUE;
+ message_error = true;
}
if (scb->flags & SCB_MSGOUT_WDTR)
@@ -2831,7 +2825,7 @@ aic7xxx_done(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
if (!(scb->tag_action))
{
aic7xxx_index_busy_target(p, scb->hscb->target_channel_lun,
- /* unbusy */ TRUE);
+ /* unbusy */ true);
if (cmd->device->simple_tags)
{
aic_dev->temp_q_depth = aic_dev->max_q_depth;
@@ -2891,7 +2885,7 @@ aic7xxx_done(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
* Description:
* Calls the aic7xxx_done() for the scsi_cmnd of each scb in the
* aborted list, and adds each scb to the free list. If complete
- * is TRUE, we also process the commands complete list.
+ * is 'true', we also process the commands complete list.
*-F*************************************************************************/
static void
aic7xxx_run_done_queue(struct aic7xxx_host *p, /*complete*/ int complete)
@@ -3034,7 +3028,7 @@ aic7xxx_search_qinfifo(struct aic7xxx_host *p, int target, int channel,
if ( !(scbp->tag_action & TAG_ENB) )
{
aic7xxx_index_busy_target(p, scbp->hscb->target_channel_lun,
- TRUE);
+ true);
}
}
else if (requeue)
@@ -3050,10 +3044,10 @@ aic7xxx_search_qinfifo(struct aic7xxx_host *p, int target, int channel,
*/
scbp->flags = flags | (scbp->flags & SCB_RECOVERY_SCB);
if (aic7xxx_index_busy_target(p, scbp->hscb->target_channel_lun,
- FALSE) == scbp->hscb->tag)
+ false) == scbp->hscb->tag)
{
aic7xxx_index_busy_target(p, scbp->hscb->target_channel_lun,
- TRUE);
+ true);
}
}
found++;
@@ -3098,11 +3092,11 @@ aic7xxx_scb_on_qoutfifo(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
while(p->qoutfifo[(p->qoutfifonext + i) & 0xff ] != SCB_LIST_NULL)
{
if(p->qoutfifo[(p->qoutfifonext + i) & 0xff ] == scb->hscb->tag)
- return TRUE;
+ return true;
else
i++;
}
- return FALSE;
+ return false;
}
@@ -3128,7 +3122,7 @@ aic7xxx_reset_device(struct aic7xxx_host *p, int target, int channel,
struct aic7xxx_scb *scbp, *prev_scbp;
struct scsi_device *sd;
unsigned char active_scb, tcl, scb_tag;
- int i = 0, init_lists = FALSE;
+ int i = 0, init_lists = false;
struct aic_dev_data *aic_dev;
/*
@@ -3182,9 +3176,9 @@ aic7xxx_reset_device(struct aic7xxx_host *p, int target, int channel,
aic_dev->temp_q_depth = aic_dev->max_q_depth;
}
tcl = (sd->id << 4) | (sd->channel << 3) | sd->lun;
- if ( (aic7xxx_index_busy_target(p, tcl, FALSE) == tag) ||
+ if ( (aic7xxx_index_busy_target(p, tcl, false) == tag) ||
(tag == SCB_LIST_NULL) )
- aic7xxx_index_busy_target(p, tcl, /* unbusy */ TRUE);
+ aic7xxx_index_busy_target(p, tcl, /* unbusy */ true);
prev_scbp = NULL;
scbp = aic_dev->delayed_scbs.head;
while (scbp != NULL)
@@ -3208,7 +3202,7 @@ aic7xxx_reset_device(struct aic7xxx_host *p, int target, int channel,
if (aic7xxx_verbose & (VERBOSE_ABORT_PROCESS | VERBOSE_RESET_PROCESS))
printk(INFO_LEAD "Cleaning QINFIFO.\n", p->host_no, channel, target, lun );
aic7xxx_search_qinfifo(p, target, channel, lun, tag,
- SCB_RESET | SCB_QUEUED_FOR_DONE, /* requeue */ FALSE, NULL);
+ SCB_RESET | SCB_QUEUED_FOR_DONE, /* requeue */ false, NULL);
/*
* Search the waiting_scbs queue for matches, this catches any SCB_QUEUED
@@ -3366,7 +3360,7 @@ aic7xxx_reset_device(struct aic7xxx_host *p, int target, int channel,
{
printk(WARN_LEAD "Free list inconsistency!.\n", p->host_no, channel,
target, lun);
- init_lists = TRUE;
+ init_lists = true;
next = SCB_LIST_NULL;
}
else
@@ -3527,7 +3521,7 @@ aic7xxx_reset_channel(struct aic7xxx_host *p, int channel, int initiate_reset)
if (aic7xxx_verbose & VERBOSE_RESET_PROCESS)
printk(INFO_LEAD "Reset channel called, %s initiate reset.\n",
- p->host_no, channel, -1, -1, (initiate_reset==TRUE) ? "will" : "won't" );
+ p->host_no, channel, -1, -1, initiate_reset ? "will" : "won't" );
if (channel == 1)
@@ -3691,7 +3685,7 @@ aic7xxx_run_waiting_queues(struct aic7xxx_host *p)
{
pause_sequencer(p);
aic_outb(p, p->qinfifonext, KERNEL_QINPOS);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
}
if (p->activescbs > p->max_activescbs)
p->max_activescbs = p->activescbs;
@@ -3909,7 +3903,7 @@ aic7xxx_handle_device_reset(struct aic7xxx_host *p, int target, int channel)
if (aic7xxx_verbose & VERBOSE_RESET_PROCESS)
printk(INFO_LEAD "Bus Device Reset delivered.\n", p->host_no, channel,
target, -1);
- aic7xxx_run_done_queue(p, /*complete*/ TRUE);
+ aic7xxx_run_done_queue(p, /*complete*/ true);
}
/*+F*************************************************************************
@@ -3926,7 +3920,7 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
struct aic_dev_data *aic_dev;
unsigned short target_mask;
unsigned char target, lun, tindex;
- unsigned char queue_flag = FALSE;
+ unsigned char queue_flag = false;
char channel;
int result;
@@ -3984,8 +3978,8 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
"LASTPHASE 0x%x, SAVED_TCL 0x%x\n", p->host_no, channel, target,
lun, aic_inb(p, LASTPHASE), aic_inb(p, SAVED_TCL));
- aic7xxx_reset_channel(p, channel, /*initiate reset*/ TRUE);
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_reset_channel(p, channel, /*initiate reset*/ true);
+ aic7xxx_run_done_queue(p, true);
}
break;
@@ -4112,7 +4106,7 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
}
}
}
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_run_done_queue(p, true);
aic7xxx_verbose = old_verbose;
/*
* Wait until after the for loop to set the busy index since
@@ -4337,7 +4331,7 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
break;
case QUEUE_FULL:
- queue_flag = TRUE; /* Mark that this is a QUEUE_FULL and */
+ queue_flag = true; /* Mark that this is a QUEUE_FULL and */
case BUSY: /* drop through to here */
{
struct aic7xxx_scb *next_scbp, *prev_scbp;
@@ -4371,7 +4365,7 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
}
aic7xxx_search_qinfifo(p, target, channel, lun,
SCB_LIST_NULL, SCB_QUEUED_FOR_DONE | SCB_QUEUE_FULL,
- FALSE, NULL);
+ false, NULL);
next_scbp = NULL;
active_hscb = aic_inb(p, SCBPTR);
prev_hscb = next_hscb = scb_index = SCB_LIST_NULL;
@@ -4416,7 +4410,7 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
} /* scb_index >= p->scb_data->numscbs */
}
aic_outb(p, active_hscb, SCBPTR);
- aic7xxx_run_done_queue(p, FALSE);
+ aic7xxx_run_done_queue(p, false);
#ifdef AIC7XXX_VERBOSE_DEBUGGING
if( (aic7xxx_verbose & VERBOSE_MINOR_ERROR) ||
@@ -4903,7 +4897,7 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)
/*
* Clear the sequencer interrupt and unpause the sequencer.
*/
- unpause_sequencer(p, /* unpause always */ TRUE);
+ unpause_sequencer(p, /* unpause always */ true);
}
/*+F*************************************************************************
@@ -4930,7 +4924,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
target = scb->cmd->device->id;
channel = scb->cmd->device->channel;
lun = scb->cmd->device->lun;
- reply = reject = done = FALSE;
+ reply = reject = done = false;
tindex = TARGET_INDEX(scb->cmd);
aic_dev = AIC_DEV(scb->cmd);
target_scsirate = aic_inb(p, TARG_SCSIRATE + tindex);
@@ -4940,13 +4934,13 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
* Parse as much of the message as is available,
* rejecting it if we don't support it. When
* the entire message is available and has been
- * handled, return TRUE indicating that we have
+ * handled, return 'true' indicating that we have
* parsed an entire message.
*/
if (p->msg_buf[0] != MSG_EXTENDED)
{
- reject = TRUE;
+ reject = true;
}
/*
@@ -4992,7 +4986,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
if (p->msg_buf[1] != MSG_EXT_SDTR_LEN)
{
- reject = TRUE;
+ reject = true;
break;
}
@@ -5060,12 +5054,12 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
* don't need a SDTR with this target (for whatever reason),
* so reject this incoming SDTR
*/
- reject = TRUE;
+ reject = true;
break;
}
/* The device is sending this message first and we have to reply */
- reply = TRUE;
+ reply = true;
if (aic7xxx_verbose & VERBOSE_NEGOTIATION2)
{
@@ -5097,7 +5091,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
if ((new_offset == 0) && (new_offset != offset))
{
aic_dev->needsdtr_copy = 0;
- reply = TRUE;
+ reply = true;
}
/*
@@ -5127,7 +5121,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
AHC_TRANS_ACTIVE|AHC_TRANS_CUR, aic_dev);
aic_dev->needsdtr = 0;
}
- done = TRUE;
+ done = true;
break;
}
case MSG_EXT_WDTR:
@@ -5135,7 +5129,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
if (p->msg_buf[1] != MSG_EXT_WDTR_LEN)
{
- reject = TRUE;
+ reject = true;
break;
}
@@ -5153,7 +5147,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
{
default:
{
- reject = TRUE;
+ reject = true;
if ( (aic7xxx_verbose & VERBOSE_NEGOTIATION2) &&
((aic_dev->flags & DEVICE_PRINT_DTR) ||
(aic7xxx_verbose > 0xffff)) )
@@ -5229,12 +5223,12 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
* don't need a WDTR with this target (for whatever reason),
* so reject this incoming WDTR
*/
- reject = TRUE;
+ reject = true;
break;
}
/* The device is sending this message first and we have to reply */
- reply = TRUE;
+ reply = true;
if (aic7xxx_verbose & VERBOSE_NEGOTIATION2)
{
@@ -5295,7 +5289,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
AHC_TRANS_ACTIVE|AHC_TRANS_CUR|AHC_TRANS_QUITE,
aic_dev);
aic_dev->needsdtr = aic_dev->needsdtr_copy;
- done = TRUE;
+ done = true;
break;
}
case MSG_EXT_PPR:
@@ -5303,7 +5297,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
if (p->msg_buf[1] != MSG_EXT_PPR_LEN)
{
- reject = TRUE;
+ reject = true;
break;
}
@@ -5389,12 +5383,12 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
* don't need a PPR with this target (for whatever reason),
* so reject this incoming PPR
*/
- reject = TRUE;
+ reject = true;
break;
}
/* The device is sending this message first and we have to reply */
- reply = TRUE;
+ reply = true;
if (aic7xxx_verbose & VERBOSE_NEGOTIATION2)
{
@@ -5420,7 +5414,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
((aic_dev->flags & DEVICE_PRINT_DTR) ||
(aic7xxx_verbose > 0xffff)) )
{
- reply = TRUE;
+ reply = true;
printk(INFO_LEAD "Requesting %d bit transfers, rejecting.\n",
p->host_no, CTL_OF_SCB(scb), 8 * (0x01 << bus_width));
}
@@ -5494,7 +5488,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
* to async (should never happen with a device that uses PPR
* messages, but have to be complete)
*/
- reply = TRUE;
+ reply = true;
}
if(reply)
@@ -5508,12 +5502,12 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
{
aic_dev->needppr = 0;
}
- done = TRUE;
+ done = true;
break;
}
default:
{
- reject = TRUE;
+ reject = true;
break;
}
} /* end of switch(p->msg_type) */
@@ -5523,7 +5517,7 @@ aic7xxx_parse_msg(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
{
aic_outb(p, MSG_MESSAGE_REJECT, MSG_OUT);
aic_outb(p, aic_inb(p, SCSISIGO) | ATNO, SCSISIGO);
- done = TRUE;
+ done = true;
}
return(done);
}
@@ -5542,7 +5536,7 @@ aic7xxx_handle_reqinit(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
{
unsigned char lastbyte;
unsigned char phasemis;
- int done = FALSE;
+ int done = false;
switch(p->msg_type)
{
@@ -5587,7 +5581,7 @@ aic7xxx_handle_reqinit(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
p->host_no, CTL_OF_SCB(scb));
#endif
}
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
}
else
{
@@ -5636,7 +5630,7 @@ aic7xxx_handle_reqinit(struct aic7xxx_host *p, struct aic7xxx_scb *scb)
aic_outb(p, aic_inb(p, SIMODE1) & ~ENREQINIT, SIMODE1);
aic_outb(p, CLRSCSIINT, CLRINT);
p->flags &= ~AHC_HANDLING_REQINITS;
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
}
break;
}
@@ -5698,8 +5692,8 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
* Go through and abort all commands for the channel, but do not
* reset the channel again.
*/
- aic7xxx_reset_channel(p, channel, /* Initiate Reset */ FALSE);
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_reset_channel(p, channel, /* Initiate Reset */ false);
+ aic7xxx_run_done_queue(p, true);
scb = NULL;
}
else if ( ((status & BUSFREE) != 0) && ((status & SELTO) == 0) )
@@ -5713,7 +5707,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
unsigned char saved_tcl = aic_inb(p, SAVED_TCL);
unsigned char target = (saved_tcl >> 4) & 0x0F;
int channel;
- int printerror = TRUE;
+ int printerror = true;
if ( (p->chip & AHC_CHIPID_MASK) == AHC_AIC7770 )
channel = (aic_inb(p, SBLKCTL) & SELBUSB) >> 3;
@@ -5735,7 +5729,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
CTL_OF_SCB(scb), scb->hscb->tag);
aic7xxx_reset_device(p, target, channel, ALL_LUNS,
(message == MSG_ABORT) ? SCB_LIST_NULL : scb->hscb->tag );
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_run_done_queue(p, true);
scb = NULL;
printerror = 0;
}
@@ -5757,7 +5751,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
*/
printerror = 0;
aic7xxx_reset_device(p, target, channel, ALL_LUNS, scb->hscb->tag);
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_run_done_queue(p, true);
scb = NULL;
}
if (printerror != 0)
@@ -5775,12 +5769,12 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
tag = SCB_LIST_NULL;
}
aic7xxx_reset_device(p, target, channel, ALL_LUNS, tag);
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_run_done_queue(p, true);
}
else
{
aic7xxx_reset_device(p, target, channel, ALL_LUNS, SCB_LIST_NULL);
- aic7xxx_run_done_queue(p, TRUE);
+ aic7xxx_run_done_queue(p, true);
}
printk(INFO_LEAD "Unexpected busfree, LASTPHASE = 0x%x, "
"SEQADDR = 0x%x\n", p->host_no, channel, target, -1, lastphase,
@@ -5794,7 +5788,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
aic_outb(p, CLRBUSFREE, CLRSINT1);
aic_outb(p, CLRSCSIINT, CLRINT);
restart_sequencer(p);
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
}
else if ((status & SELTO) != 0)
{
@@ -5915,7 +5909,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
* are allowed to reselect in.
*/
restart_sequencer(p);
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
}
else if (scb == NULL)
{
@@ -5931,7 +5925,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
*/
aic_outb(p, status, CLRSINT1);
aic_outb(p, CLRSCSIINT, CLRINT);
- unpause_sequencer(p, /* unpause always */ TRUE);
+ unpause_sequencer(p, /* unpause always */ true);
scb = NULL;
}
else if (status & SCSIPERR)
@@ -6061,7 +6055,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
}
aic_outb(p, CLRSCSIPERR, CLRSINT1);
aic_outb(p, CLRSCSIINT, CLRINT);
- unpause_sequencer(p, /* unpause_always */ TRUE);
+ unpause_sequencer(p, /* unpause_always */ true);
}
else if ( (status & REQINIT) &&
(p->flags & AHC_HANDLING_REQINITS) )
@@ -6085,7 +6079,7 @@ aic7xxx_handle_scsiint(struct aic7xxx_host *p, unsigned char intstat)
p->host_no, -1, -1, -1, status);
aic_outb(p, status, CLRSINT1);
aic_outb(p, CLRSCSIINT, CLRINT);
- unpause_sequencer(p, /* unpause always */ TRUE);
+ unpause_sequencer(p, /* unpause always */ true);
scb = NULL;
}
if (scb != NULL)
@@ -6117,14 +6111,14 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
* only partially linked in. Therefore, once we pass the scan phase
* of the bus, we really should disable this function.
*/
- bogus = FALSE;
+ bogus = false;
memset(&scb_status[0], 0, sizeof(scb_status));
pause_sequencer(p);
saved_scbptr = aic_inb(p, SCBPTR);
if (saved_scbptr >= p->scb_data->maxhscbs)
{
printk("Bogus SCBPTR %d\n", saved_scbptr);
- bogus = TRUE;
+ bogus = true;
}
scb_status[saved_scbptr] = SCB_CURRENTLY_ACTIVE;
free_scbh = aic_inb(p, FREE_SCBH);
@@ -6132,7 +6126,7 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
(free_scbh >= p->scb_data->maxhscbs) )
{
printk("Bogus FREE_SCBH %d\n", free_scbh);
- bogus = TRUE;
+ bogus = true;
}
else
{
@@ -6143,7 +6137,7 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
{
printk("HSCB %d on multiple lists, status 0x%02x", temp,
scb_status[temp] | SCB_FREE_LIST);
- bogus = TRUE;
+ bogus = true;
}
scb_status[temp] |= SCB_FREE_LIST;
aic_outb(p, temp, SCBPTR);
@@ -6156,7 +6150,7 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
(dis_scbh >= p->scb_data->maxhscbs) )
{
printk("Bogus DISCONNECTED_SCBH %d\n", dis_scbh);
- bogus = TRUE;
+ bogus = true;
}
else
{
@@ -6167,7 +6161,7 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
{
printk("HSCB %d on multiple lists, status 0x%02x", temp,
scb_status[temp] | SCB_DISCONNECTED_LIST);
- bogus = TRUE;
+ bogus = true;
}
scb_status[temp] |= SCB_DISCONNECTED_LIST;
aic_outb(p, temp, SCBPTR);
@@ -6180,7 +6174,7 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
(wait_scbh >= p->scb_data->maxhscbs) )
{
printk("Bogus WAITING_SCBH %d\n", wait_scbh);
- bogus = TRUE;
+ bogus = true;
}
else
{
@@ -6191,7 +6185,7 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
{
printk("HSCB %d on multiple lists, status 0x%02x", temp,
scb_status[temp] | SCB_WAITING_LIST);
- bogus = TRUE;
+ bogus = true;
}
scb_status[temp] |= SCB_WAITING_LIST;
aic_outb(p, temp, SCBPTR);
@@ -6208,23 +6202,23 @@ aic7xxx_check_scbs(struct aic7xxx_host *p, char *buffer)
(temp >= p->scb_data->maxhscbs)) )
{
printk("HSCB %d bad, SCB_NEXT invalid(%d).\n", i, temp);
- bogus = TRUE;
+ bogus = true;
}
if ( temp == i )
{
printk("HSCB %d bad, SCB_NEXT points to self.\n", i);
- bogus = TRUE;
+ bogus = true;
}
if (scb_status[i] == 0)
lost++;
if (lost > 1)
{
printk("Too many lost scbs.\n");
- bogus=TRUE;
+ bogus=true;
}
}
aic_outb(p, saved_scbptr, SCBPTR);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
if (bogus)
{
printk("Bogus parameters found in card SCB array structures.\n");
@@ -6296,14 +6290,14 @@ aic7xxx_handle_command_completion_intr(struct aic7xxx_host *p)
if ( ((aic_inb(p, LASTPHASE) & PHASE_MASK) != P_BUSFREE) &&
(aic_inb(p, SCB_TAG) == scb->hscb->tag) )
{
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
continue;
}
aic7xxx_reset_device(p, scb->cmd->device->id, scb->cmd->device->channel,
scb->cmd->device->lun, scb->hscb->tag);
scb->flags &= ~(SCB_QUEUED_FOR_DONE | SCB_RESET | SCB_ABORT |
SCB_QUEUED_ABORT);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
}
else if (scb->flags & SCB_ABORT)
{
@@ -6439,7 +6433,7 @@ aic7xxx_isr(void *dev_id)
}
#endif
aic_outb(p, CLRPARERR | CLRBRKADRINT, CLRINT);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
}
if (intstat & SEQINT)
@@ -6531,7 +6525,7 @@ aic7xxx_init_transinfo(struct aic7xxx_host *p, struct aic_dev_data *aic_dev)
MSG_EXT_WDTR_BUS_8_BIT, (AHC_TRANS_ACTIVE |
AHC_TRANS_GOAL |
AHC_TRANS_CUR), aic_dev );
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
}
if ( sdpnt->sdtr && p->user[tindex].offset )
{
@@ -6642,7 +6636,7 @@ aic7xxx_slave_alloc(struct scsi_device *SDptr)
static void
aic7xxx_device_queue_depth(struct aic7xxx_host *p, struct scsi_device *device)
{
- int tag_enabled = FALSE;
+ int tag_enabled = false;
struct aic_dev_data *aic_dev = device->hostdata;
unsigned char tindex;
@@ -6653,7 +6647,7 @@ aic7xxx_device_queue_depth(struct aic7xxx_host *p, struct scsi_device *device)
if (device->tagged_supported)
{
- tag_enabled = TRUE;
+ tag_enabled = true;
if (!(p->discenable & (1 << tindex)))
{
@@ -6661,20 +6655,20 @@ aic7xxx_device_queue_depth(struct aic7xxx_host *p, struct scsi_device *device)
printk(INFO_LEAD "Disconnection disabled, unable to "
"enable tagged queueing.\n",
p->host_no, device->channel, device->id, device->lun);
- tag_enabled = FALSE;
+ tag_enabled = false;
}
else
{
if (p->instance >= ARRAY_SIZE(aic7xxx_tag_info))
{
- static int print_warning = TRUE;
+ static int print_warning = true;
if(print_warning)
{
printk(KERN_INFO "aic7xxx: WARNING, insufficient tag_info instances for"
" installed controllers.\n");
printk(KERN_INFO "aic7xxx: Please update the aic7xxx_tag_info array in"
" the aic7xxx.c source file.\n");
- print_warning = FALSE;
+ print_warning = false;
}
aic_dev->max_q_depth = aic_dev->temp_q_depth =
aic7xxx_default_queue_depth;
@@ -6684,7 +6678,7 @@ aic7xxx_device_queue_depth(struct aic7xxx_host *p, struct scsi_device *device)
if (aic7xxx_tag_info[p->instance].tag_commands[tindex] == 255)
{
- tag_enabled = FALSE;
+ tag_enabled = false;
}
else if (aic7xxx_tag_info[p->instance].tag_commands[tindex] == 0)
{
@@ -6823,13 +6817,13 @@ aic7xxx_probe(int slot, int base, ahc_flag_type *flags)
int bios_disabled;
} AIC7xxx[] = {
{ 4, { 0x04, 0x90, 0x77, 0x70 },
- AHC_AIC7770|AHC_EISA, FALSE }, /* mb 7770 */
+ AHC_AIC7770|AHC_EISA, false }, /* mb 7770 */
{ 4, { 0x04, 0x90, 0x77, 0x71 },
- AHC_AIC7770|AHC_EISA, FALSE }, /* host adapter 274x */
+ AHC_AIC7770|AHC_EISA, false }, /* host adapter 274x */
{ 4, { 0x04, 0x90, 0x77, 0x56 },
- AHC_AIC7770|AHC_VL, FALSE }, /* 284x BIOS enabled */
+ AHC_AIC7770|AHC_VL, false }, /* 284x BIOS enabled */
{ 4, { 0x04, 0x90, 0x77, 0x57 },
- AHC_AIC7770|AHC_VL, TRUE } /* 284x BIOS disabled */
+ AHC_AIC7770|AHC_VL, true } /* 284x BIOS disabled */
};
/*
@@ -8340,7 +8334,7 @@ aic7xxx_register(struct scsi_host_template *template, struct aic7xxx_host *p,
p->host_no, -1, -1 , -1);
aic7xxx_clear_intstat(p);
- unpause_sequencer(p, /* unpause_always */ TRUE);
+ unpause_sequencer(p, /* unpause_always */ true);
return (found);
}
@@ -10109,7 +10103,7 @@ skip_pci_controller:
pause_sequencer(p);
aic7xxx_print_card(p);
aic7xxx_print_scratch_ram(p);
- unpause_sequencer(p, TRUE);
+ unpause_sequencer(p, true);
}
}
current_p = temp_p;
@@ -10445,7 +10439,7 @@ static int __aic7xxx_bus_device_reset(struct scsi_cmnd *cmd)
* attempt a bus reset.
*/
saved_scbptr = aic_inb(p, SCBPTR);
- disconnected = FALSE;
+ disconnected = false;
if (lastphase != P_BUSFREE)
{
@@ -10454,7 +10448,7 @@ static int __aic7xxx_bus_device_reset(struct scsi_cmnd *cmd)
printk(WARN_LEAD "Invalid SCB ID %d is active, "
"SCB flags = 0x%x.\n", p->host_no,
CTL_OF_CMD(cmd), scb->hscb->tag, scb->flags);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
return FAILED;
}
if (scb->hscb->tag == aic_inb(p, SCB_TAG))
@@ -10463,7 +10457,7 @@ static int __aic7xxx_bus_device_reset(struct scsi_cmnd *cmd)
{
printk(WARN_LEAD "Device reset, Message buffer "
"in use\n", p->host_no, CTL_OF_SCB(scb));
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
return FAILED;
}
@@ -10476,7 +10470,7 @@ static int __aic7xxx_bus_device_reset(struct scsi_cmnd *cmd)
/* Send the abort message to the active SCB. */
aic_outb(p, HOST_MSG, MSG_OUT);
aic_outb(p, lastphase | ATNO, SCSISIGO);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
spin_unlock_irq(p->host->host_lock);
ssleep(1);
spin_lock_irq(p->host->host_lock);
@@ -10498,9 +10492,9 @@ static int __aic7xxx_bus_device_reset(struct scsi_cmnd *cmd)
* not need to queue the command again since the card should start it soon
*/
if (aic7xxx_search_qinfifo(p, cmd->device->channel, cmd->device->id, cmd->device->lun, hscb->tag,
- 0, TRUE, NULL) == 0)
+ 0, true, NULL) == 0)
{
- disconnected = TRUE;
+ disconnected = true;
if ((hscb_index = aic7xxx_find_scb(p, scb)) != SCB_LIST_NULL)
{
unsigned char scb_control;
@@ -10535,7 +10529,7 @@ static int __aic7xxx_bus_device_reset(struct scsi_cmnd *cmd)
}
}
aic_outb(p, saved_scbptr, SCBPTR);
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
spin_unlock_irq(p->host->host_lock);
msleep(1000/4);
spin_lock_irq(p->host->host_lock);
@@ -10663,7 +10657,7 @@ static int __aic7xxx_abort(struct scsi_cmnd *cmd)
*/
if ( ((found = aic7xxx_search_qinfifo(p, cmd->device->id, cmd->device->channel,
cmd->device->lun, scb->hscb->tag, SCB_ABORT | SCB_QUEUED_FOR_DONE,
- FALSE, NULL)) != 0) &&
+ false, NULL)) != 0) &&
(aic7xxx_verbose & VERBOSE_ABORT_PROCESS))
{
printk(INFO_LEAD "SCB found in QINFIFO and aborted.\n", p->host_no,
@@ -10744,7 +10738,7 @@ static int __aic7xxx_abort(struct scsi_cmnd *cmd)
*/
printk(INFO_LEAD "message buffer busy, unable to abort.\n",
p->host_no, CTL_OF_SCB(scb));
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
return FAILED;
}
/* Fallthrough to below, set ATNO after we set SCB_CONTROL */
@@ -10781,7 +10775,7 @@ static int __aic7xxx_abort(struct scsi_cmnd *cmd)
else
aic_outb(p, p->qinfifonext, KERNEL_QINPOS);
}
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
spin_unlock_irq(p->host->host_lock);
msleep(1000/4);
spin_lock_irq(p->host->host_lock);
@@ -10800,8 +10794,8 @@ static int __aic7xxx_abort(struct scsi_cmnd *cmd)
success:
if (aic7xxx_verbose & VERBOSE_ABORT_RETURN)
printk(INFO_LEAD "Abort successful.\n", p->host_no, CTL_OF_CMD(cmd));
- aic7xxx_run_done_queue(p, TRUE);
- unpause_sequencer(p, FALSE);
+ aic7xxx_run_done_queue(p, true);
+ unpause_sequencer(p, false);
return SUCCESS;
}
@@ -10874,7 +10868,7 @@ static int aic7xxx_reset(struct scsi_cmnd *cmd)
* We just completed the command when we ran the isr stuff, so we no
* longer have it.
*/
- unpause_sequencer(p, FALSE);
+ unpause_sequencer(p, false);
spin_unlock_irq(p->host->host_lock);
return SUCCESS;
}
@@ -10883,10 +10877,10 @@ static int aic7xxx_reset(struct scsi_cmnd *cmd)
* By this point, we want to already know what we are going to do and
* only have the following code implement our course of action.
*/
- aic7xxx_reset_channel(p, cmd->device->channel, TRUE);
+ aic7xxx_reset_channel(p, cmd->device->channel, true);
if (p->features & AHC_TWIN)
{
- aic7xxx_reset_channel(p, cmd->device->channel ^ 0x01, TRUE);
+ aic7xxx_reset_channel(p, cmd->device->channel ^ 0x01, true);
restart_sequencer(p);
}
aic_outb(p, aic_inb(p, SIMODE1) & ~(ENREQINIT|ENBUSFREE), SIMODE1);
@@ -10895,8 +10889,8 @@ static int aic7xxx_reset(struct scsi_cmnd *cmd)
p->msg_type = MSG_TYPE_NONE;
p->msg_index = 0;
p->msg_len = 0;
- aic7xxx_run_done_queue(p, TRUE);
- unpause_sequencer(p, FALSE);
+ aic7xxx_run_done_queue(p, true);
+ unpause_sequencer(p, false);
spin_unlock_irq(p->host->host_lock);
ssleep(2);
return SUCCESS;
diff --git a/drivers/scsi/aic7xxx_old/aic7xxx_proc.c b/drivers/scsi/aic7xxx_old/aic7xxx_proc.c
index b07e4f0..cb5a119 100644
--- a/drivers/scsi/aic7xxx_old/aic7xxx_proc.c
+++ b/drivers/scsi/aic7xxx_old/aic7xxx_proc.c
@@ -105,7 +105,7 @@ aic7xxx_proc_info ( struct Scsi_Host *HBAptr, char *buffer, char **start, off_t
}
}
- if (inout == TRUE) /* Has data been written to the file? */
+ if (inout) /* Has data been written to the file? */
{
return (aic7xxx_set_info(buffer, length, HBAptr));
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-10 17:46 [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values Richard Knutsson
@ 2007-02-10 18:27 ` James Bottomley
2007-02-10 20:35 ` Richard Knutsson
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: James Bottomley @ 2007-02-10 18:27 UTC (permalink / raw)
To: Richard Knutsson; +Cc: linux-kernel, linux-scsi
On Sat, 2007-02-10 at 18:46 +0100, Richard Knutsson wrote:
> Convert:
> FALSE -> false
> TRUE -> true
Actually, downcasing true and false in this driver is pretty much a
retrograde step. The reason for their being uppercased is that they
represent constants (and uppercase is the traditional defined constant
specifier).
When discussion about TRUE and FALSE came up a long time a go in the
context of the mid layer we agreed to strip the defined constants out of
that code and just go with 1 and 0 inline ... because the code was
pretty much being rewritten. We also decided to encourage but not force
the driver writers simply to use 1 and 0 as well ... a lot of people are
deeply wedded to the TRUE and FALSE defines, it turned out.
James
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-10 18:27 ` James Bottomley
@ 2007-02-10 20:35 ` Richard Knutsson
2007-02-10 20:43 ` Richard Knutsson
2007-02-12 20:27 ` Andrew Morton
2 siblings, 0 replies; 13+ messages in thread
From: Richard Knutsson @ 2007-02-10 20:35 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-kernel, linux-scsi
James Bottomley wrote:
> On Sat, 2007-02-10 at 18:46 +0100, Richard Knutsson wrote:
>
>> Convert:
>> FALSE -> false
>> TRUE -> true
>>
>
> Actually, downcasing true and false in this driver is pretty much a
> retrograde step. The reason for their being uppercased is that they
> represent constants (and uppercase is the traditional defined constant
> specifier).
>
I would argue that 'false' and 'true' are values and not constants, but
further more C99 is defining them in lowercase (stdbool.h).
> When discussion about TRUE and FALSE came up a long time a go in the
> context of the mid layer we agreed to strip the defined constants out of
> that code and just go with 1 and 0 inline ... because the code was
> pretty much being rewritten. We also decided to encourage but not force
> the driver writers simply to use 1 and 0 as well ... a lot of people are
> deeply wedded to the TRUE and FALSE defines, it turned out.
>
As I have expressed before, I don't understand why people seem to
dislike 'false'/'true' but anyway, since you seem to approve booleans,
would it be possible to convert the obvious variables/functions into
boolean-type?
Richard Knutsson
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-10 18:27 ` James Bottomley
2007-02-10 20:35 ` Richard Knutsson
@ 2007-02-10 20:43 ` Richard Knutsson
2007-02-12 20:27 ` Andrew Morton
2 siblings, 0 replies; 13+ messages in thread
From: Richard Knutsson @ 2007-02-10 20:43 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-kernel, linux-scsi
James Bottomley wrote:
> On Sat, 2007-02-10 at 18:46 +0100, Richard Knutsson wrote:
>
>> Convert:
>> FALSE -> false
>> TRUE -> true
>>
>
> Actually, downcasing true and false in this driver is pretty much a
> retrograde step. The reason for their being uppercased is that they
> represent constants (and uppercase is the traditional defined constant
> specifier).
>
> When discussion about TRUE and FALSE came up a long time a go in the
> context of the mid layer we agreed to strip the defined constants out of
> that code and just go with 1 and 0 inline ... because the code was
> pretty much being rewritten. We also decided to encourage but not force
> the driver writers simply to use 1 and 0 as well ... a lot of people are
> deeply wedded to the TRUE and FALSE defines, it turned out.
>
Btw, is this just for aic7xxx_old and not aic7xxx? Is it going to be
replaced? In that case, I will just leave it alone.
Richard Knutsson
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-10 18:27 ` James Bottomley
2007-02-10 20:35 ` Richard Knutsson
2007-02-10 20:43 ` Richard Knutsson
@ 2007-02-12 20:27 ` Andrew Morton
2007-02-16 16:42 ` James Bottomley
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2007-02-12 20:27 UTC (permalink / raw)
To: James Bottomley; +Cc: ricknu-0, linux-kernel, linux-scsi
> On Sat, 10 Feb 2007 12:27:42 -0600 James Bottomley <James.Bottomley@SteelEye.com> wrote:
> When discussion about TRUE and FALSE came up a long time a go in the
> context of the mid layer we agreed to strip the defined constants out of
> that code and just go with 1 and 0 inline ... because the code was
> pretty much being rewritten. We also decided to encourage but not force
> the driver writers simply to use 1 and 0 as well ... a lot of people are
> deeply wedded to the TRUE and FALSE defines, it turned out.
Given that we now have a standard kernel-wide, c99-friendly way of
expressing true and false, I'd suggest that this decision can be revisited.
Because a "true" is significantly more meaningful (and hence readable)
thing than a bare "1".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-12 20:27 ` Andrew Morton
@ 2007-02-16 16:42 ` James Bottomley
2007-02-16 18:04 ` Richard Knutsson
2007-02-16 18:34 ` Andrew Morton
0 siblings, 2 replies; 13+ messages in thread
From: James Bottomley @ 2007-02-16 16:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: ricknu-0, linux-kernel, linux-scsi
On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
> Given that we now have a standard kernel-wide, c99-friendly way of
> expressing true and false, I'd suggest that this decision can be revisited.
>
> Because a "true" is significantly more meaningful (and hence readable)
> thing than a bare "1".
OK, I'm really not happy with doing this for three reasons:
1. It's inviting huge amounts of driver churn changing bitfields to
booleans
2. I do find it to be a readability issue. Like most driver writers,
I'm used to register layouts, and those are simple bitfields, so I don't
tend to think true and false, I think 1 and 0.
3. Having a different, special, type for single bit bitfields (while
still using u<n> for multi bit bitfields) is asking for confusion, and
hence trouble at the driver level.
James
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 16:42 ` James Bottomley
@ 2007-02-16 18:04 ` Richard Knutsson
2007-02-16 18:23 ` James Bottomley
2007-02-16 18:34 ` Andrew Morton
1 sibling, 1 reply; 13+ messages in thread
From: Richard Knutsson @ 2007-02-16 18:04 UTC (permalink / raw)
To: James Bottomley; +Cc: Andrew Morton, linux-kernel, linux-scsi
James Bottomley wrote:
> On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
>
>> Given that we now have a standard kernel-wide, c99-friendly way of
>> expressing true and false, I'd suggest that this decision can be revisited.
>>
>> Because a "true" is significantly more meaningful (and hence readable)
>> thing than a bare "1".
>>
>
> OK, I'm really not happy with doing this for three reasons:
>
> 1. It's inviting huge amounts of driver churn changing bitfields to
> booleans
>
Have been some work done already. Has there been any problems?
> 2. I do find it to be a readability issue. Like most driver writers,
> I'm used to register layouts, and those are simple bitfields, so I don't
> tend to think true and false, I think 1 and 0.
>
It is a fundamental difference between an integer and a boolean. Have
you seen anyone trying to do "bool var = true + true;"? ;)
> 3. Having a different, special, type for single bit bitfields (while
> still using u<n> for multi bit bitfields) is asking for confusion, and
> hence trouble at the driver level.
>
I don't think a boolean should be view as a single bit bitfield. Ex:
u8 a:1;
...
int b = 4 + a;
is obviously not a boolean, while:
u8 a:1;
...
if (a)
is, and a should be "bool a:1;" (imho)
Richard Knutsson
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 18:04 ` Richard Knutsson
@ 2007-02-16 18:23 ` James Bottomley
2007-02-16 19:10 ` Richard Knutsson
0 siblings, 1 reply; 13+ messages in thread
From: James Bottomley @ 2007-02-16 18:23 UTC (permalink / raw)
To: Richard Knutsson; +Cc: Andrew Morton, linux-kernel, linux-scsi
On Fri, 2007-02-16 at 19:04 +0100, Richard Knutsson wrote:
> James Bottomley wrote:
> > On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
> >
> >> Given that we now have a standard kernel-wide, c99-friendly way of
> >> expressing true and false, I'd suggest that this decision can be revisited.
> >>
> >> Because a "true" is significantly more meaningful (and hence readable)
> >> thing than a bare "1".
> >>
> >
> > OK, I'm really not happy with doing this for three reasons:
> >
> > 1. It's inviting huge amounts of driver churn changing bitfields to
> > booleans
> >
> Have been some work done already. Has there been any problems?
There's always an issue when two people work on the same driver ... it
causes patch conflicts, which is why we try to avoid it where we can.
> > 2. I do find it to be a readability issue. Like most driver writers,
> > I'm used to register layouts, and those are simple bitfields, so I don't
> > tend to think true and false, I think 1 and 0.
> >
> It is a fundamental difference between an integer and a boolean. Have
> you seen anyone trying to do "bool var = true + true;"? ;)
I don't quite see how this is relevant to the readability issue?
> > 3. Having a different, special, type for single bit bitfields (while
> > still using u<n> for multi bit bitfields) is asking for confusion, and
> > hence trouble at the driver level.
> >
> I don't think a boolean should be view as a single bit bitfield. Ex:
> u8 a:1;
> ...
> int b = 4 + a;
> is obviously not a boolean, while:
> u8 a:1;
> ...
> if (a)
> is, and a should be "bool a:1;" (imho)
This again, doesn't really address the argument. I'm saying I'd rather
not have confusion over what types to use in the driver. You're saying
that if you only check the value for truth or falsehood it should be a
boolean. That's actually worse than I was anticipating because you're
now saying that single bit bitfields may or may not be booleans
depending on use. This looks like worse potential for confusion than
before.
James
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 16:42 ` James Bottomley
2007-02-16 18:04 ` Richard Knutsson
@ 2007-02-16 18:34 ` Andrew Morton
2007-02-16 18:42 ` James Bottomley
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2007-02-16 18:34 UTC (permalink / raw)
To: James Bottomley; +Cc: ricknu-0, linux-kernel, linux-scsi
On Fri, 16 Feb 2007 10:42:12 -0600 James Bottomley <James.Bottomley@SteelEye.com> wrote:
> On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
> > Given that we now have a standard kernel-wide, c99-friendly way of
> > expressing true and false, I'd suggest that this decision can be revisited.
> >
> > Because a "true" is significantly more meaningful (and hence readable)
> > thing than a bare "1".
>
> OK, I'm really not happy with doing this for three reasons:
>
> 1. It's inviting huge amounts of driver churn changing bitfields to
> booleans
>
> 2. I do find it to be a readability issue. Like most driver writers,
> I'm used to register layouts, and those are simple bitfields, so I don't
> tend to think true and false, I think 1 and 0.
>
> 3. Having a different, special, type for single bit bitfields (while
> still using u<n> for multi bit bitfields) is asking for confusion, and
> hence trouble at the driver level.
>
Confused. The patch changes TRUE to true and FALSE to false. The code
wasn't using bitfields before and isn't using them afterwards. I wouldn't
expect there to be any change in generated code.
All it's doing is replacing the driver's private TRUE/FALSE with the
kernel-wide ones.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 18:34 ` Andrew Morton
@ 2007-02-16 18:42 ` James Bottomley
2007-02-16 18:50 ` Andrew Morton
0 siblings, 1 reply; 13+ messages in thread
From: James Bottomley @ 2007-02-16 18:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: ricknu-0, linux-kernel, linux-scsi
On Fri, 2007-02-16 at 10:34 -0800, Andrew Morton wrote:
> On Fri, 16 Feb 2007 10:42:12 -0600 James Bottomley <James.Bottomley@SteelEye.com> wrote:
>
> > On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
> > > Given that we now have a standard kernel-wide, c99-friendly way of
> > > expressing true and false, I'd suggest that this decision can be revisited.
> > >
> > > Because a "true" is significantly more meaningful (and hence readable)
> > > thing than a bare "1".
> >
> > OK, I'm really not happy with doing this for three reasons:
> >
> > 1. It's inviting huge amounts of driver churn changing bitfields to
> > booleans
> >
> > 2. I do find it to be a readability issue. Like most driver writers,
> > I'm used to register layouts, and those are simple bitfields, so I don't
> > tend to think true and false, I think 1 and 0.
> >
> > 3. Having a different, special, type for single bit bitfields (while
> > still using u<n> for multi bit bitfields) is asking for confusion, and
> > hence trouble at the driver level.
> >
>
> Confused. The patch changes TRUE to true and FALSE to false. The code
> wasn't using bitfields before and isn't using them afterwards. I wouldn't
> expect there to be any change in generated code.
Sorry, I was addressing the general idea of using booleans in drivers.
> All it's doing is replacing the driver's private TRUE/FALSE with the
> kernel-wide ones.
I already addressed that one ... I prefer bare 0 and 1. However, if the
driver writer wants to use TRUE/FALSE, I won't specifically reject it.
I really don't like the lower case true/false.
James
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 18:42 ` James Bottomley
@ 2007-02-16 18:50 ` Andrew Morton
2007-02-16 21:43 ` Doug Ledford
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2007-02-16 18:50 UTC (permalink / raw)
To: James Bottomley; +Cc: ricknu-0, linux-kernel, linux-scsi
On Fri, 16 Feb 2007 12:42:27 -0600 James Bottomley <James.Bottomley@SteelEye.com> wrote:
> On Fri, 2007-02-16 at 10:34 -0800, Andrew Morton wrote:
> > On Fri, 16 Feb 2007 10:42:12 -0600 James Bottomley <James.Bottomley@SteelEye.com> wrote:
> >
> > > On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
> > > > Given that we now have a standard kernel-wide, c99-friendly way of
> > > > expressing true and false, I'd suggest that this decision can be revisited.
> > > >
> > > > Because a "true" is significantly more meaningful (and hence readable)
> > > > thing than a bare "1".
> > >
> > > OK, I'm really not happy with doing this for three reasons:
> > >
> > > 1. It's inviting huge amounts of driver churn changing bitfields to
> > > booleans
> > >
> > > 2. I do find it to be a readability issue. Like most driver writers,
> > > I'm used to register layouts, and those are simple bitfields, so I don't
> > > tend to think true and false, I think 1 and 0.
> > >
> > > 3. Having a different, special, type for single bit bitfields (while
> > > still using u<n> for multi bit bitfields) is asking for confusion, and
> > > hence trouble at the driver level.
> > >
> >
> > Confused. The patch changes TRUE to true and FALSE to false. The code
> > wasn't using bitfields before and isn't using them afterwards. I wouldn't
> > expect there to be any change in generated code.
>
> Sorry, I was addressing the general idea of using booleans in drivers.
>
> > All it's doing is replacing the driver's private TRUE/FALSE with the
> > kernel-wide ones.
>
> I already addressed that one ... I prefer bare 0 and 1. However, if the
> driver writer wants to use TRUE/FALSE, I won't specifically reject it.
> I really don't like the lower case true/false.
>
Me no understand.
If you take the specific example of
void
ahd_set_syncrate(struct ahd_softc *ahd, struct ahd_devinfo *devinfo,
u_int period, u_int offset, u_int ppr_options,
u_int type, int paused)
then if is crufty, inappropriate and wrong that `paused' is a scalar type.
It's just not true or sensible that the code is written so that `paused'
can take a value of seventy eight. It _is_ a boolean. It is a truth
value. Declaring it as such in the source is all goodness. Passing the
value `true' into calls to this function improve readability over passing
"1".
So I don't agree with (or understand) your objections. But I can certainly
understand reluctance to merge a large-but-minor, do-nothing-much patch into
a large and not-very-maintained driver.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 18:23 ` James Bottomley
@ 2007-02-16 19:10 ` Richard Knutsson
0 siblings, 0 replies; 13+ messages in thread
From: Richard Knutsson @ 2007-02-16 19:10 UTC (permalink / raw)
To: James Bottomley; +Cc: Andrew Morton, linux-kernel, linux-scsi
James Bottomley wrote:
> On Fri, 2007-02-16 at 19:04 +0100, Richard Knutsson wrote:
>
>> James Bottomley wrote:
>>
>>> On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
>>>
>>>
>>>> Given that we now have a standard kernel-wide, c99-friendly way of
>>>> expressing true and false, I'd suggest that this decision can be revisited.
>>>>
>>>> Because a "true" is significantly more meaningful (and hence readable)
>>>> thing than a bare "1".
>>>>
>>>>
>>> OK, I'm really not happy with doing this for three reasons:
>>>
>>> 1. It's inviting huge amounts of driver churn changing bitfields to
>>> booleans
>>>
>>>
>> Have been some work done already. Has there been any problems?
>>
>
> There's always an issue when two people work on the same driver ... it
> causes patch conflicts, which is why we try to avoid it where we can.
>
But it is (hopefully) a one-time change.
>>> 2. I do find it to be a readability issue. Like most driver writers,
>>> I'm used to register layouts, and those are simple bitfields, so I don't
>>> tend to think true and false, I think 1 and 0.
>>>
>>>
>> It is a fundamental difference between an integer and a boolean. Have
>> you seen anyone trying to do "bool var = true + true;"? ;)
>>
>
> I don't quite see how this is relevant to the readability issue?
>
Your "simple bitfields" :)
But on the note of readability; there is in no way any constraints to
only use 'false'/'true'. Right now I'm converting some files who already
uses FALSE/TRUE and may change 0/1 for consitency.
But I think it all boils down to what people are used to (I don't think
there are any C++/Java-developers who complains about boolean and
false/true). I am not too concerned about 0/1 being used or false/true,
but the proper use of booleans and values.
>
>>> 3. Having a different, special, type for single bit bitfields (while
>>> still using u<n> for multi bit bitfields) is asking for confusion, and
>>> hence trouble at the driver level.
>>>
>>>
>> I don't think a boolean should be view as a single bit bitfield. Ex:
>> u8 a:1;
>> ...
>> int b = 4 + a;
>> is obviously not a boolean, while:
>> u8 a:1;
>> ...
>> if (a)
>> is, and a should be "bool a:1;" (imho)
>>
>
> This again, doesn't really address the argument. I'm saying I'd rather
> not have confusion over what types to use in the driver. You're saying
> that if you only check the value for truth or falsehood it should be a
> boolean. That's actually worse than I was anticipating because you're
> now saying that single bit bitfields may or may not be booleans
> depending on use. This looks like worse potential for confusion than
> before.
>
Actually I find it to be simpler. Which would you chose?:
u8 a:1;
or
int a:1;
for a value? But if 'a' is a statement, then we just use:
bool a:1 (if the size of the variable is of relevance, otherwise just
"bool a"). (You know what the variables are going to be used for when
declaring them, right??)
Btw, how do you know if "a += 1" is just used to flip 'a's value or if
it is a bug (in the case of boolean)?
Richard "come over to the dark side" Knutsson
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
2007-02-16 18:50 ` Andrew Morton
@ 2007-02-16 21:43 ` Doug Ledford
0 siblings, 0 replies; 13+ messages in thread
From: Doug Ledford @ 2007-02-16 21:43 UTC (permalink / raw)
To: Andrew Morton; +Cc: James Bottomley, ricknu-0, linux-kernel, linux-scsi
[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]
On Fri, 2007-02-16 at 10:50 -0800, Andrew Morton wrote:
> Me no understand.
>
> If you take the specific example of
>
> void
> ahd_set_syncrate(struct ahd_softc *ahd, struct ahd_devinfo *devinfo,
> u_int period, u_int offset, u_int ppr_options,
> u_int type, int paused)
>
> then if is crufty, inappropriate and wrong that `paused' is a scalar type.
Although you picked a code segment out of the modern aic7xxx, there is a
matching similar one in aic7xxx_old. Now, in all fairness, I was at one
point playing with a much more preemptable model for that driver that
allowed nested pauses, at which point the value of pause would have made
sense to be scalar, but that was a *long* time ago.
>
> It's just not true or sensible that the code is written so that `paused'
> can take a value of seventy eight. It _is_ a boolean. It is a truth
> value. Declaring it as such in the source is all goodness. Passing the
> value `true' into calls to this function improve readability over passing
> "1".
Hence the reason for the original upper case TRUE/FALSE. I have to
admit, I don't really like the lower case true/false, it looks like a
variable that can be assigned, thereby changing the implementation of
the function call when in fact each calling location is hard coding a
constant. But, that's just me and my crufty old C that differentiates
between hard coded things and variables via case.
> So I don't agree with (or understand) your objections. But I can certainly
> understand reluctance to merge a large-but-minor, do-nothing-much patch into
> a large and not-very-maintained driver.
Hehehe...and here I was thinking of factoring that thing into files and
actually bringing it into the current century.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: CFBFF194
http://people.redhat.com/dledford
Infiniband specific RPMs available at
http://people.redhat.com/dledford/Infiniband
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2007-02-16 21:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-10 17:46 [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values Richard Knutsson
2007-02-10 18:27 ` James Bottomley
2007-02-10 20:35 ` Richard Knutsson
2007-02-10 20:43 ` Richard Knutsson
2007-02-12 20:27 ` Andrew Morton
2007-02-16 16:42 ` James Bottomley
2007-02-16 18:04 ` Richard Knutsson
2007-02-16 18:23 ` James Bottomley
2007-02-16 19:10 ` Richard Knutsson
2007-02-16 18:34 ` Andrew Morton
2007-02-16 18:42 ` James Bottomley
2007-02-16 18:50 ` Andrew Morton
2007-02-16 21:43 ` Doug Ledford
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.