From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Joe Perches <joe@perches.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>,
Borislav Petkov <petkovbb@gmail.com>, Tejun Heo <tj@kernel.org>,
linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] drivers/ide/ide-core: Unsplit constant strings for pr_<level> and dev_<level>
Date: Sun, 24 May 2009 16:12:11 +0400 [thread overview]
Message-ID: <4A19399B.8060207@ru.mvista.com> (raw)
In-Reply-To: <57f9dd5c33bcdd863793a3ee95ab050e8456bab4.1243062772.git.joe@perches.com>
Hello.
Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
>
[...]
> diff --git a/drivers/ide/ide-acpi.c b/drivers/ide/ide-acpi.c
> index f323a60..560b9c4 100644
> --- a/drivers/ide/ide-acpi.c
> +++ b/drivers/ide/ide-acpi.c
> @@ -435,8 +435,8 @@ void ide_acpi_get_timing(ide_hwif_t *hwif)
> if (!out_obj->buffer.length || !out_obj->buffer.pointer ||
> out_obj->buffer.length != sizeof(struct GTM_buffer)) {
> kfree(output.pointer);
> - pr_err("%s: unexpected _GTM length (0x%x)[should be 0x%zx] or "
> - "addr (0x%p)\n",
> + pr_err(
> + "%s: unexpected _GTM length (0x%x)[should be 0x%zx] or addr (0x%p)\n",
>
Oh no... do you really think somebody will search for this message
using strings like "should be"?
> __func__, out_obj->buffer.length,
> sizeof(struct GTM_buffer), out_obj->buffer.pointer);
> return;
> diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
> index 46cf46e..1e9e2d6 100644
> --- a/drivers/ide/ide-atapi.c
> +++ b/drivers/ide/ide-atapi.c
> @@ -60,11 +60,13 @@ int ide_check_atapi_device(ide_drive_t *drive, const char *s)
> dev_err(&drive->gendev, "%s: the removable flag is not set\n",
> s);
> else if (drive->media == ide_floppy && drq_type == 3)
> - dev_err(&drive->gendev, "%s: sorry, DRQ type (0x%02x) not "
> - "supported\n", s, drq_type);
> + dev_err(&drive->gendev,
> + "%s: sorry, DRQ type (0x%02x) not supported\n",
>
Same question. I think the user will search for "sorry, DRQ type".
> + s, drq_type);
> else if (packet_size != 0)
> - dev_err(&drive->gendev, "%s: packet size (0x%02x) is not 12 "
> - "bytes\n", s, packet_size);
> + dev_err(&drive->gendev,
> + "%s: packet size (0x%02x) is not 12 bytes\n",
>
When the message is broken by the format specifier, turning it into
one liner can hardly help seraching...
> @@ -439,8 +440,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
> if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
> pc->flags &= ~PC_FLAG_DMA_IN_PROGRESS;
> dev_err(&drive->gendev,
> - "The device wants to issue more interrupts "
> - "in DMA mode\n");
> + "The device wants to issue more interrupts in DMA mode\n");
>
Oh noes, the indentation...
> @@ -509,14 +509,12 @@ static u8 ide_wait_ireason(ide_drive_t *drive, u8 ireason)
> while (retries-- && ((ireason & ATAPI_COD) == 0 ||
> (ireason & ATAPI_IO))) {
> dev_err(&drive->gendev,
> - "(IO,CoD != (0,1) while issuing "
> - "a packet command, retrying\n");
> + "(IO,CoD != (0,1) while issuing a packet command, retrying\n");
>
Sigh...
> udelay(100);
> ireason = ide_read_ireason(drive);
> if (retries == 0) {
> dev_err(&drive->gendev,
> - "(IO,CoD != (0,1) while issuing "
> - "a packet command, ignoring\n");
> + "(IO,CoD != (0,1) while issuing a packet command, ignoring\n");
>
Sigh...
> @@ -547,8 +545,7 @@ static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
>
> if (ide_wait_stat(&startstop, drive, ATA_DRQ, ATA_BUSY, WAIT_READY)) {
> dev_err(&drive->gendev,
> - "Strange, packet command initiated yet "
> - "DRQ isn't asserted\n");
> + "Strange, packet command initiated yet DRQ isn't asserted\n");
>
Sigh...
> @@ -590,8 +587,7 @@ static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
>
> if ((ireason & ATAPI_COD) == 0 || (ireason & ATAPI_IO)) {
> dev_err(&drive->gendev,
> - "(IO,CoD) != (0,1) while issuing "
> - "a packet command\n");
> + "(IO,CoD) != (0,1) while issuing a packet command\n");
>
Sigh...
> diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
> index 51c7cb4..07565c9 100644
> --- a/drivers/ide/ide-io.c
> +++ b/drivers/ide/ide-io.c
> @@ -743,8 +743,9 @@ static void unexpected_intr(int irq, ide_hwif_t *hwif)
>
> if (time_after(jiffies, last_msgtime + HZ)) {
> last_msgtime = jiffies;
> - dev_err(&hwif->gendev, "unexpected interrupt, "
> - "status=0x%02x, count=%ld\n", stat, count);
> + dev_err(&hwif->gendev,
> + "unexpected interrupt, status=0x%02x, count=%ld\n",
>
Hardly won anything...
> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
> index 6dbb96e..c5ab2ec 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -259,8 +259,8 @@ no_80w:
> if (drive->dev_flags & IDE_DFLAG_UDMA33_WARNED)
> return 0;
>
> - dev_warn(&drive->gendev, "%s side 80-wire cable detection failed, "
> - "limiting max speed to UDMA33\n",
> + dev_warn(&drive->gendev,
> + "%s side 80-wire cable detection failed, limiting max speed to UDMA33\n",
>
Will you really use the full message to serch here?
> diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
> index c4b44e2..a4120e2 100644
> --- a/drivers/ide/ide-probe.c
> +++ b/drivers/ide/ide-probe.c
> @@ -271,8 +271,9 @@ int ide_dev_read_id(ide_drive_t *drive, u8 cmd, u16 *id)
> s = tp_ops->read_status(hwif);
> if ((a ^ s) & ~ATA_IDX)
> /* ancient Seagate drives, broken interfaces */
> - dev_info(&drive->gendev, "probing with STATUS(0x%02x) "
> - "instead of ALTSTATUS(0x%02x)\n", s, a);
> + dev_info(&drive->gendev,
> + "probing with STATUS(0x%02x) instead of ALTSTATUS(0x%02x)\n",
>
Same comment about the line broken by the format specifiers. This
wins absolutely nothing.
> @@ -415,8 +416,8 @@ static int do_probe (ide_drive_t *drive, u8 cmd)
>
> if (rc == 1 && cmd == ATA_CMD_ID_ATAPI) {
> dev_err(&drive->gendev,
> - "no response (status = 0x%02x), "
> - "resetting drive\n", stat);
> + "no response (status = 0x%02x), resetting drive\n",
>
And again...
> diff --git a/drivers/ide/ide-proc.c b/drivers/ide/ide-proc.c
> index 23b9d6a..cd580b1 100644
> --- a/drivers/ide/ide-proc.c
> +++ b/drivers/ide/ide-proc.c
> @@ -464,8 +464,7 @@ static int ide_replace_subdriver(ide_drive_t *drive, const char *driver)
> strlcpy(drive->driver_req, driver, sizeof(drive->driver_req));
> err = device_attach(dev);
> if (err < 0)
> - pr_warning("IDE: %s: device_attach error: %d\n",
> - __func__, err);
> + pr_warning("IDE: %s: device_attach error: %d\n", __func__, err);
>
I'm nt sure why this isn't a part of printk(KERN_*, ...); to
pr_*(...) conversion patch. You're not unsplitting the constant here.
> diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> index 8d741d1..687b3be 100644
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -579,8 +579,9 @@ int ide_taskfile_ioctl(ide_drive_t *drive, unsigned long arg)
> case TASKFILE_MULTI_OUT:
> if (!drive->mult_count) {
> /* (hs): give up if multcount is not set */
> - dev_err(&drive->gendev, "%s Multimode Write " \
> - "multcount is not set\n", __func__);
> + dev_err(&drive->gendev,
> + "%s Multimode Write multcount is not set\n",
>
Sigh...
> @@ -599,8 +600,8 @@ int ide_taskfile_ioctl(ide_drive_t *drive, unsigned long arg)
> if (!drive->mult_count) {
> /* (hs): give up if multcount is not set */
> dev_err(&drive->gendev,
> - "%s Multimode Read failure "
> - "multcount is not set\n", __func__);
> + "%s Multimode Read failure multcount is not set\n",
> + __func__);
>
Sigh...
> diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> index f139cf3..3aca89a 100644
> --- a/drivers/ide/setup-pci.c
> +++ b/drivers/ide/setup-pci.c
> @@ -39,17 +39,18 @@ static int ide_setup_pci_baseregs(struct pci_dev *dev, const char *name)
> if (pci_read_config_byte(dev, PCI_CLASS_PROG, &progif) ||
> (progif & 5) != 5) {
> if ((progif & 0xa) != 0xa) {
> - pr_info("%s %s: device not capable of full "
> - "native PCI mode\n", name, pci_name(dev));
> + pr_info(
> + "%s %s: device not capable of full native PCI mode\n",
>
Sigh...
> (void) pci_write_config_byte(dev, PCI_CLASS_PROG, progif|5);
> if (pci_read_config_byte(dev, PCI_CLASS_PROG, &progif) ||
> (progif & 5) != 5) {
> - pr_err("%s %s: rewrite of PROGIF failed, "
> - "wanted 0x%04x, got 0x%04x\n",
> + pr_err(
> + "%s %s: rewrite of PROGIF failed, wanted 0x%04x, got 0x%04x\n",
>
The same comment about the string broken by yhe format specifiers.
This wins nothing, well, almost...
> @@ -322,8 +323,8 @@ static int ide_hw_configure(struct pci_dev *dev, const struct ide_port_info *d,
> if ((d->host_flags & IDE_HFLAG_ISA_PORTS) == 0) {
> if (ide_pci_check_iomem(dev, d, 2 * port) ||
> ide_pci_check_iomem(dev, d, 2 * port + 1)) {
> - pr_err("%s %s: I/O baseregs (BIOS) are "
> - "reported as MEM for port %d!\n",
> + pr_err(
> + "%s %s: I/O baseregs (BIOS) are reported as MEM for port %d!\n",
>
Sigh...
> @@ -519,8 +520,9 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
>
> if (ide_pci_is_in_compatibility_mode(dev)) {
> if (noisy)
> - pr_info("%s %s: not 100%% native mode: will "
> - "probe irqs later\n", d->name, pci_name(dev));
> + pr_info(
> + "%s %s: not 100%% native mode: will probe irqs later\n",
>
Sigh...
MBR, Sergei
next prev parent reply other threads:[~2009-05-24 12:12 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-23 7:41 [PATCH 0/3] drivers/ide/ide-core: Use dev_<level> and pr_<level> Joe Perches
2009-05-23 7:41 ` [PATCH 1/3] drivers/ide/ide-core: Convert printk(KERN_<level> to dev_<level> Joe Perches
2009-06-02 13:57 ` Bartlomiej Zolnierkiewicz
2009-05-23 7:41 ` [PATCH 2/3] drivers/ide/ide-core: Convert printk(KERN_<level> to pr_<level> Joe Perches
2009-06-02 14:02 ` Bartlomiej Zolnierkiewicz
2009-05-23 7:41 ` [PATCH 3/3] drivers/ide/ide-core: Unsplit constant strings for pr_<level> and dev_<level> Joe Perches
2009-05-24 12:12 ` Sergei Shtylyov [this message]
2009-05-24 16:46 ` Joe Perches
2009-05-24 18:00 ` Sergei Shtylyov
2009-06-02 14:19 ` Bartlomiej Zolnierkiewicz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A19399B.8060207@ru.mvista.com \
--to=sshtylyov@ru.mvista.com \
--cc=bzolnier@gmail.com \
--cc=joe@perches.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=petkovbb@gmail.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.