Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

2007-10-09 Thread Jeff Garzik

Matthew Wilcox wrote:

On Tue, Oct 09, 2007 at 11:35:14AM -0400, Jeff Garzik wrote:
Not true.  The original code used 'int irq', and the printf format was 
correct as a result.


You changed the code to use 'unsigned int irq' as found in struct 
pci_dev, without changing IRQ_FMT as needed.


If you wanted to avoid the regression another way, I suppose patch #1 
should update IRQ_FMT to use "%u", and patch #14 should move the "%u" 
use to its caller.


What do you think the regression is, exactly?

The *only* difference in how vsnprintf handles %d vs %u is the sign bit.


Because when you change the type, you should change the printf format to 
match?


It used to match, before your patchset.  Now it doesn't.

Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

2007-10-09 Thread Matthew Wilcox
On Tue, Oct 09, 2007 at 11:35:14AM -0400, Jeff Garzik wrote:
> Not true.  The original code used 'int irq', and the printf format was 
> correct as a result.
> 
> You changed the code to use 'unsigned int irq' as found in struct 
> pci_dev, without changing IRQ_FMT as needed.
> 
> If you wanted to avoid the regression another way, I suppose patch #1 
> should update IRQ_FMT to use "%u", and patch #14 should move the "%u" 
> use to its caller.

What do you think the regression is, exactly?

The *only* difference in how vsnprintf handles %d vs %u is the sign bit.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

2007-10-09 Thread Jeff Garzik

Matthew Wilcox wrote:

On Fri, Oct 05, 2007 at 04:59:47PM -0400, Jeff Garzik wrote:

NAK

The proper format string is '%u' because irq is unsigned


I'll change it to %u in the next revision.  However, I respectfully
decline your NAK as this was not a regression.


Not true.  The original code used 'int irq', and the printf format was 
correct as a result.


You changed the code to use 'unsigned int irq' as found in struct 
pci_dev, without changing IRQ_FMT as needed.


If you wanted to avoid the regression another way, I suppose patch #1 
should update IRQ_FMT to use "%u", and patch #14 should move the "%u" 
use to its caller.


Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

2007-10-09 Thread Matthew Wilcox
On Fri, Oct 05, 2007 at 04:59:47PM -0400, Jeff Garzik wrote:
> NAK
> 
> The proper format string is '%u' because irq is unsigned

I'll change it to %u in the next revision.  However, I respectfully
decline your NAK as this was not a regression.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

2007-10-05 Thread Jeff Garzik

Matthew Wilcox wrote:

@@ -1196,8 +1193,8 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, 
off_t offset, int len)
copy_info(&info, "Chip " NAME53C "%s, device id 0x%x, "
 "revision id 0x%x\n", np->s.chip_name,
 np->s.device->device, np->s.device->revision);
-   copy_info(&info, "At PCI address %s, IRQ " IRQ_FMT "\n",
-   pci_name(np->s.device), IRQ_PRM(np->s.device->irq));
+   copy_info(&info, "At PCI address %s, IRQ %d\n",
+pci_name(np->s.device), np->s.device->irq);
copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
 (int) (np->minsync_dt ? np->minsync_dt : np->minsync),
 np->maxwide ? "Wide" : "Narrow",
@@ -1283,9 +1280,9 @@ static struct Scsi_Host * __devinit sym_attach(struct 
scsi_host_template *tpnt,
unsigned long flags;
struct sym_fw *fw;
 
-	printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq " IRQ_FMT "\n",

+   printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq %d\n",
unit, dev->chip.name, pdev->revision, pci_name(pdev),
-   IRQ_PRM(pdev->irq));
+   pdev->irq);
 
 	/*

 *  Get the firmware for this chip.


NAK

The proper format string is '%u' because irq is unsigned

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html