Message ID | 1425378228-6095-1-git-send-email-hofrat@osadl.org |
---|---|
State | Superseded |
Delegated to: | David Miller |
Headers | show |
On Tue, 2015-03-03 at 05:23 -0500, Nicholas Mc Guire wrote: > Use jiffies_to_msecs for converting jiffies as it handles all of the corner > cases reliably and also helps readability. The printk format is fixed up > as jiffies_to_msecs returns unsigned int not unsigned long. Then ideally you'd use %u not %d. > Note that this patch will give a checkpatch warning due to "quoted string > split across lines" but thats not related to this change - not sure if this > should be fixed - looks intentional to me. It's not. It'd be better coalesced. > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c [] > @@ -1793,11 +1793,11 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor) > tape->best_dsc_rw_freq = clamp_t(unsigned long, t, IDETAPE_DSC_RW_MIN, > IDETAPE_DSC_RW_MAX); > printk(KERN_INFO "ide-tape: %s <-> %s: %dKBps, %d*%dkB buffer, " > - "%lums tDSC%s\n", > + "%dms tDSC%s\n", > drive->name, tape->name, *(u16 *)&tape->caps[14], > (*(u16 *)&tape->caps[16] * 512) / tape->buffer_size, > tape->buffer_size / 1024, > - tape->best_dsc_rw_freq * 1000 / HZ, > + jiffies_to_msecs(tape->best_dsc_rw_freq), > (drive->dev_flags & IDE_DFLAG_USING_DMA) ? ", DMA" : ""); -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 03 Mar 2015, Joe Perches wrote: > On Tue, 2015-03-03 at 05:23 -0500, Nicholas Mc Guire wrote: > > Use jiffies_to_msecs for converting jiffies as it handles all of the corner > > cases reliably and also helps readability. The printk format is fixed up > > as jiffies_to_msecs returns unsigned int not unsigned long. > > Then ideally you'd use %u not %d. > ok - was not clear about that as there are very many cases wher u32 is printed with %d and compilers seem to be happy with this - gcc (Debian 4.7.2-5) 4.7.2 in my case. will fix that up and repost. > > Note that this patch will give a checkpatch warning due to "quoted string > > split across lines" but thats not related to this change - not sure if this > > should be fixed - looks intentional to me. > > It's not. It'd be better coalesced. well it did seem quite systematic in this file, aside from it also needing to move over the pr_* so thats going to need a few seperate cleanups. > > > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c > [] > > @@ -1793,11 +1793,11 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor) > > tape->best_dsc_rw_freq = clamp_t(unsigned long, t, IDETAPE_DSC_RW_MIN, > > IDETAPE_DSC_RW_MAX); > > printk(KERN_INFO "ide-tape: %s <-> %s: %dKBps, %d*%dkB buffer, " > > - "%lums tDSC%s\n", > > + "%dms tDSC%s\n", > > drive->name, tape->name, *(u16 *)&tape->caps[14], > > (*(u16 *)&tape->caps[16] * 512) / tape->buffer_size, > > tape->buffer_size / 1024, > > - tape->best_dsc_rw_freq * 1000 / HZ, > > + jiffies_to_msecs(tape->best_dsc_rw_freq), > > (drive->dev_flags & IDE_DFLAG_USING_DMA) ? ", DMA" : ""); > > thanks for the review notes! thx! hofrat -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-03-03 at 11:57 +0100, Nicholas Mc Guire wrote: > On Tue, 03 Mar 2015, Joe Perches wrote: > > On Tue, 2015-03-03 at 05:23 -0500, Nicholas Mc Guire wrote: > > > Note that this patch will give a checkpatch warning due to "quoted string > > > split across lines" but thats not related to this change - not sure if this > > > should be fixed - looks intentional to me. > > > > It's not. It'd be better coalesced. > > well it did seem quite systematic in this file, aside from it also needing > to move over the pr_* so thats going to need a few seperate cleanups. Sure, no worries, whenever/ifever. I'm pretty sure there aren't many active users of ide-tape devices. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 03 Mar 2015, Joe Perches wrote: > On Tue, 2015-03-03 at 11:57 +0100, Nicholas Mc Guire wrote: > > On Tue, 03 Mar 2015, Joe Perches wrote: > > > On Tue, 2015-03-03 at 05:23 -0500, Nicholas Mc Guire wrote: > > > > Note that this patch will give a checkpatch warning due to "quoted string > > > > split across lines" but thats not related to this change - not sure if this > > > > should be fixed - looks intentional to me. > > > > > > It's not. It'd be better coalesced. > > > > well it did seem quite systematic in this file, aside from it also needing > > to move over the pr_* so thats going to need a few seperate cleanups. > > Sure, no worries, whenever/ifever. > > I'm pretty sure there aren't many active users > of ide-tape devices. > Im sure thats true - the motivation for cleanups though is that what ever stays in old drivers - even if hardly in use- gets copied into new drivers at some point... thx! hofrat -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 1793aea..d9ce376 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -1793,11 +1793,11 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor) tape->best_dsc_rw_freq = clamp_t(unsigned long, t, IDETAPE_DSC_RW_MIN, IDETAPE_DSC_RW_MAX); printk(KERN_INFO "ide-tape: %s <-> %s: %dKBps, %d*%dkB buffer, " - "%lums tDSC%s\n", + "%dms tDSC%s\n", drive->name, tape->name, *(u16 *)&tape->caps[14], (*(u16 *)&tape->caps[16] * 512) / tape->buffer_size, tape->buffer_size / 1024, - tape->best_dsc_rw_freq * 1000 / HZ, + jiffies_to_msecs(tape->best_dsc_rw_freq), (drive->dev_flags & IDE_DFLAG_USING_DMA) ? ", DMA" : ""); ide_proc_register_driver(drive, tape->driver);
Use jiffies_to_msecs for converting jiffies as it handles all of the corner cases reliably and also helps readability. The printk format is fixed up as jiffies_to_msecs returns unsigned int not unsigned long. Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> --- Note that this patch will give a checkpatch warning due to "quoted string split across lines" but thats not related to this change - not sure if this should be fixed - looks intentional to me. This was only compile tested with x86_64_defconfig + CONFIG_IDE=m, CONFIG_BLK_DEV_IDETAPE=m Patch is against 4.0-rc1 (localversion-next is -next-20150303) drivers/ide/ide-tape.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)