* [PATCH 0/7] parport: Use generic kernel logging styles [not found] <69fb1d36-b6cf-7c46-96d1-9403de6ab47a@infradead.org> @ 2020-02-28 8:32 ` Joe Perches 2020-02-28 8:32 ` [PATCH 1/7] parport: Convert printk(KERN_<LEVEL> to pr_<level>( Joe Perches ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Joe Perches @ 2020-02-28 8:32 UTC (permalink / raw) To: Randy Dunlap, linux-parisc Cc: Sudip Mukherjee, James E.J. Bottomley, Helge Deller, linux-kernel Well, if the parport logging is getting some generic fixing, here's some more generic logging fixing... Joe Perches (7): parport: Convert printk(KERN_<LEVEL> to pr_<level>( parport: Use more comon logging styles parport: daisy: Convert DPRINTK to pr_debug parport_amiga: Convert DPRINTK to pr_debug parport_mfc3: Convert DPRINTK to pr_debug parport_pc: Convert DPRINTK to pr_debug parport: Standardize use of printmode drivers/parport/daisy.c | 29 +--- drivers/parport/ieee1284.c | 4 +- drivers/parport/ieee1284_ops.c | 3 +- drivers/parport/parport_amiga.c | 22 +-- drivers/parport/parport_atari.c | 2 +- drivers/parport/parport_cs.c | 6 +- drivers/parport/parport_gsc.c | 25 +-- drivers/parport/parport_gsc.h | 21 ++- drivers/parport/parport_ip32.c | 117 +++++++------- drivers/parport/parport_mfc3.c | 21 +-- drivers/parport/parport_pc.c | 263 +++++++++++++------------------ drivers/parport/parport_sunbpp.c | 2 +- drivers/parport/probe.c | 34 ++-- drivers/parport/procfs.c | 6 +- drivers/parport/share.c | 37 +++-- 15 files changed, 261 insertions(+), 331 deletions(-) -- 2.24.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/7] parport: Convert printk(KERN_<LEVEL> to pr_<level>( 2020-02-28 8:32 ` [PATCH 0/7] parport: Use generic kernel logging styles Joe Perches @ 2020-02-28 8:32 ` Joe Perches 2020-02-28 8:32 ` [PATCH 2/7] parport: Use more comon logging styles Joe Perches ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2020-02-28 8:32 UTC (permalink / raw) To: Randy Dunlap, Sudip Mukherjee, James E.J. Bottomley, Helge Deller Cc: linux-kernel, linux-parisc Use the more common kernel style. Miscellanea: o Coalesce formats o Realign arguments Signed-off-by: Joe Perches <joe@perches.com> --- drivers/parport/daisy.c | 6 +- drivers/parport/ieee1284.c | 4 +- drivers/parport/ieee1284_ops.c | 3 +- drivers/parport/parport_amiga.c | 2 +- drivers/parport/parport_atari.c | 2 +- drivers/parport/parport_cs.c | 6 +- drivers/parport/parport_gsc.c | 7 +- drivers/parport/parport_ip32.c | 25 ++--- drivers/parport/parport_mfc3.c | 2 +- drivers/parport/parport_pc.c | 166 +++++++++++++------------------ drivers/parport/parport_sunbpp.c | 2 +- drivers/parport/probe.c | 7 +- drivers/parport/share.c | 24 ++--- 13 files changed, 110 insertions(+), 146 deletions(-) diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c index 3b00e2c..f87cc80 100644 --- a/drivers/parport/daisy.c +++ b/drivers/parport/daisy.c @@ -145,8 +145,7 @@ int parport_daisy_init(struct parport *port) ((num_ports = num_mux_ports(port)) == 2 || num_ports == 4)) { /* Leave original as port zero. */ port->muxport = 0; - printk(KERN_INFO - "%s: 1st (default) port of %d-way multiplexor\n", + pr_info("%s: 1st (default) port of %d-way multiplexor\n", port->name, num_ports); for (i = 1; i < num_ports; i++) { /* Clone the port. */ @@ -159,8 +158,7 @@ int parport_daisy_init(struct parport *port) continue; } - printk(KERN_INFO - "%s: %d%s port of %d-way multiplexor on %s\n", + pr_info("%s: %d%s port of %d-way multiplexor on %s\n", extra->name, i + 1, th[i + 1], num_ports, port->name); diff --git a/drivers/parport/ieee1284.c b/drivers/parport/ieee1284.c index ba562bc..f28d6a3 100644 --- a/drivers/parport/ieee1284.c +++ b/drivers/parport/ieee1284.c @@ -319,7 +319,7 @@ int parport_negotiate (struct parport *port, int mode) #ifndef CONFIG_PARPORT_1284 if (mode == IEEE1284_MODE_COMPAT) return 0; - printk (KERN_ERR "parport: IEEE1284 not supported in this kernel\n"); + pr_err("parport: IEEE1284 not supported in this kernel\n"); return -1; #else int m = mode & ~IEEE1284_ADDR; @@ -674,7 +674,7 @@ ssize_t parport_write (struct parport *port, const void *buffer, size_t len) ssize_t parport_read (struct parport *port, void *buffer, size_t len) { #ifndef CONFIG_PARPORT_1284 - printk (KERN_ERR "parport: IEEE1284 not supported in this kernel\n"); + pr_err("parport: IEEE1284 not supported in this kernel\n"); return -ENODEV; #else int mode = port->physport->ieee1284.mode; diff --git a/drivers/parport/ieee1284_ops.c b/drivers/parport/ieee1284_ops.c index b1c9f51..2c11bd 100644 --- a/drivers/parport/ieee1284_ops.c +++ b/drivers/parport/ieee1284_ops.c @@ -579,8 +579,7 @@ size_t parport_ieee1284_ecp_read_data (struct parport *port, pr_debug("ECP read timed out at 45\n"); if (command) - printk (KERN_WARNING - "%s: command ignored (%02x)\n", + pr_warn("%s: command ignored (%02x)\n", port->name, byte); break; diff --git a/drivers/parport/parport_amiga.c b/drivers/parport/parport_amiga.c index 3301861f..8c7a598a 100644 --- a/drivers/parport/parport_amiga.c +++ b/drivers/parport/parport_amiga.c @@ -212,7 +212,7 @@ static int __init amiga_parallel_probe(struct platform_device *pdev) if (err) goto out_irq; - printk(KERN_INFO "%s: Amiga built-in port using irq\n", p->name); + pr_info("%s: Amiga built-in port using irq\n", p->name); /* XXX: set operating mode */ parport_announce_port(p); diff --git a/drivers/parport/parport_atari.c b/drivers/parport/parport_atari.c index f8dd368b..2ff0fe0 100644 --- a/drivers/parport/parport_atari.c +++ b/drivers/parport/parport_atari.c @@ -200,7 +200,7 @@ static int __init parport_atari_init(void) } this_port = p; - printk(KERN_INFO "%s: Atari built-in port using irq\n", p->name); + pr_info("%s: Atari built-in port using irq\n", p->name); parport_announce_port (p); return 0; diff --git a/drivers/parport/parport_cs.c b/drivers/parport/parport_cs.c index e77044c2..8e7e3ac 100644 --- a/drivers/parport/parport_cs.c +++ b/drivers/parport/parport_cs.c @@ -142,10 +142,8 @@ static int parport_config(struct pcmcia_device *link) link->irq, PARPORT_DMA_NONE, &link->dev, IRQF_SHARED); if (p == NULL) { - printk(KERN_NOTICE "parport_cs: parport_pc_probe_port() at " - "0x%3x, irq %u failed\n", - (unsigned int) link->resource[0]->start, - link->irq); + pr_notice("parport_cs: parport_pc_probe_port() at 0x%3x, irq %u failed\n", + (unsigned int)link->resource[0]->start, link->irq); goto failed; } diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c index 922535a..81082d 100644 --- a/drivers/parport/parport_gsc.c +++ b/drivers/parport/parport_gsc.c @@ -282,7 +282,7 @@ struct parport *parport_gsc_probe_port(unsigned long base, p->size = (p->modes & PARPORT_MODE_EPP)?8:3; p->private_data = priv; - printk(KERN_INFO "%s: PC-style at 0x%lx", p->name, p->base); + pr_info("%s: PC-style at 0x%lx", p->name, p->base); p->irq = irq; if (p->irq == PARPORT_IRQ_AUTO) { p->irq = PARPORT_IRQ_NONE; @@ -315,8 +315,7 @@ struct parport *parport_gsc_probe_port(unsigned long base, if (p->irq != PARPORT_IRQ_NONE) { if (request_irq (p->irq, parport_irq_handler, 0, p->name, p)) { - printk (KERN_WARNING "%s: irq %d in use, " - "resorting to polled operation\n", + pr_warn("%s: irq %d in use, resorting to polled operation\n", p->name, p->irq); p->irq = PARPORT_IRQ_NONE; p->dma = PARPORT_DMA_NONE; @@ -347,7 +346,7 @@ static int __init parport_init_chip(struct parisc_device *dev) unsigned long port; if (!dev->irq) { - printk(KERN_WARNING "IRQ not found for parallel device at 0x%llx\n", + pr_warn("IRQ not found for parallel device at 0x%llx\n", (unsigned long long)dev->hpa.start); return -ENODEV; } diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c index ab215b..4f76e4 100644 --- a/drivers/parport/parport_ip32.c +++ b/drivers/parport/parport_ip32.c @@ -1337,9 +1337,8 @@ static unsigned int parport_ip32_fwp_wait_interrupt(struct parport *p) ecr = parport_ip32_read_econtrol(p); if ((ecr & ECR_F_EMPTY) && !(ecr & ECR_SERVINTR) && !lost_interrupt) { - printk(KERN_WARNING PPIP32 - "%s: lost interrupt in %s\n", - p->name, __func__); + pr_warn(PPIP32 "%s: lost interrupt in %s\n", + p->name, __func__); lost_interrupt = 1; } } @@ -1643,8 +1642,8 @@ static size_t parport_ip32_compat_write_data(struct parport *p, DSR_nBUSY | DSR_nFAULT)) { /* Avoid to flood the logs */ if (ready_before) - printk(KERN_INFO PPIP32 "%s: not ready in %s\n", - p->name, __func__); + pr_info(PPIP32 "%s: not ready in %s\n", + p->name, __func__); ready_before = 0; goto stop; } @@ -1724,8 +1723,8 @@ static size_t parport_ip32_ecp_write_data(struct parport *p, DSR_nBUSY | DSR_nFAULT)) { /* Avoid to flood the logs */ if (ready_before) - printk(KERN_INFO PPIP32 "%s: not ready in %s\n", - p->name, __func__); + pr_info(PPIP32 "%s: not ready in %s\n", + p->name, __func__); ready_before = 0; goto stop; } @@ -2064,8 +2063,7 @@ static __init struct parport *parport_ip32_probe_port(void) p->modes |= PARPORT_MODE_TRISTATE; if (!parport_ip32_fifo_supported(p)) { - printk(KERN_WARNING PPIP32 - "%s: error: FIFO disabled\n", p->name); + pr_warn(PPIP32 "%s: error: FIFO disabled\n", p->name); /* Disable hardware modes depending on a working FIFO. */ features &= ~PARPORT_IP32_ENABLE_SPP; features &= ~PARPORT_IP32_ENABLE_ECP; @@ -2077,8 +2075,7 @@ static __init struct parport *parport_ip32_probe_port(void) if (features & PARPORT_IP32_ENABLE_IRQ) { int irq = MACEISA_PARALLEL_IRQ; if (request_irq(irq, parport_ip32_interrupt, 0, p->name, p)) { - printk(KERN_WARNING PPIP32 - "%s: error: IRQ disabled\n", p->name); + pr_warn(PPIP32 "%s: error: IRQ disabled\n", p->name); /* DMA cannot work without interrupts. */ features &= ~PARPORT_IP32_ENABLE_DMA; } else { @@ -2091,8 +2088,7 @@ static __init struct parport *parport_ip32_probe_port(void) /* Allocate DMA resources */ if (features & PARPORT_IP32_ENABLE_DMA) { if (parport_ip32_dma_register()) - printk(KERN_WARNING PPIP32 - "%s: error: DMA disabled\n", p->name); + pr_warn(PPIP32 "%s: error: DMA disabled\n", p->name); else { pr_probe(p, "DMA support enabled\n"); p->dma = 0; /* arbitrary value != PARPORT_DMA_NONE */ @@ -2134,8 +2130,7 @@ static __init struct parport *parport_ip32_probe_port(void) parport_ip32_dump_state(p, "end init", 0); /* Print out what we found */ - printk(KERN_INFO "%s: SGI IP32 at 0x%lx (0x%lx)", - p->name, p->base, p->base_hi); + pr_info("%s: SGI IP32 at 0x%lx (0x%lx)", p->name, p->base, p->base_hi); if (p->irq != PARPORT_IRQ_NONE) printk(", irq %d", p->irq); printk(" ["); diff --git a/drivers/parport/parport_mfc3.c b/drivers/parport/parport_mfc3.c index 9f87faf..3190ef0 100644 --- a/drivers/parport/parport_mfc3.c +++ b/drivers/parport/parport_mfc3.c @@ -325,7 +325,7 @@ static int __init parport_mfc3_init(void) p->dev = &z->dev; this_port[pias++] = p; - printk(KERN_INFO "%s: Multiface III port using irq\n", p->name); + pr_info("%s: Multiface III port using irq\n", p->name); /* XXX: set operating mode */ p->private_data = (void *)piabase; diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c index 1f17a3..2872c67 100644 --- a/drivers/parport/parport_pc.c +++ b/drivers/parport/parport_pc.c @@ -982,28 +982,24 @@ static void show_parconfig_smsc37c669(int io, int key) outb(0xaa, io); if (verbose_probing) { - printk(KERN_INFO - "SMSC 37c669 LPT Config: cr_1=0x%02x, 4=0x%02x, " - "A=0x%2x, 23=0x%02x, 26=0x%02x, 27=0x%02x\n", + pr_info("SMSC 37c669 LPT Config: cr_1=0x%02x, 4=0x%02x, A=0x%2x, 23=0x%02x, 26=0x%02x, 27=0x%02x\n", cr1, cr4, cra, cr23, cr26, cr27); /* The documentation calls DMA and IRQ-Lines by letters, so the board maker can/will wire them appropriately/randomly... G=reserved H=IDE-irq, */ - printk(KERN_INFO - "SMSC LPT Config: io=0x%04x, irq=%c, dma=%c, fifo threshold=%d\n", - cr23 * 4, - (cr27 & 0x0f) ? 'A' - 1 + (cr27 & 0x0f) : '-', - (cr26 & 0x0f) ? 'A' - 1 + (cr26 & 0x0f) : '-', - cra & 0x0f); - printk(KERN_INFO "SMSC LPT Config: enabled=%s power=%s\n", - (cr23 * 4 >= 0x100) ? "yes" : "no", - (cr1 & 4) ? "yes" : "no"); - printk(KERN_INFO - "SMSC LPT Config: Port mode=%s, EPP version =%s\n", - (cr1 & 0x08) ? "Standard mode only (SPP)" - : modes[cr4 & 0x03], - (cr4 & 0x40) ? "1.7" : "1.9"); + pr_info("SMSC LPT Config: io=0x%04x, irq=%c, dma=%c, fifo threshold=%d\n", + cr23 * 4, + (cr27 & 0x0f) ? 'A' - 1 + (cr27 & 0x0f) : '-', + (cr26 & 0x0f) ? 'A' - 1 + (cr26 & 0x0f) : '-', + cra & 0x0f); + pr_info("SMSC LPT Config: enabled=%s power=%s\n", + (cr23 * 4 >= 0x100) ? "yes" : "no", + (cr1 & 4) ? "yes" : "no"); + pr_info("SMSC LPT Config: Port mode=%s, EPP version =%s\n", + (cr1 & 0x08) ? "Standard mode only (SPP)" + : modes[cr4 & 0x03], + (cr4 & 0x40) ? "1.7" : "1.9"); } /* Heuristics ! BIOS setup for this mainboard device limits @@ -1013,7 +1009,7 @@ static void show_parconfig_smsc37c669(int io, int key) if (cr23 * 4 >= 0x100) { /* if active */ s = find_free_superio(); if (s == NULL) - printk(KERN_INFO "Super-IO: too many chips!\n"); + pr_info("Super-IO: too many chips!\n"); else { int d; switch (cr23 * 4) { @@ -1078,26 +1074,24 @@ static void show_parconfig_winbond(int io, int key) outb(0xaa, io); if (verbose_probing) { - printk(KERN_INFO - "Winbond LPT Config: cr_30=%02x 60,61=%02x%02x 70=%02x 74=%02x, f0=%02x\n", - cr30, cr60, cr61, cr70, cr74, crf0); - printk(KERN_INFO "Winbond LPT Config: active=%s, io=0x%02x%02x irq=%d, ", - (cr30 & 0x01) ? "yes" : "no", cr60, cr61, cr70 & 0x0f); + pr_info("Winbond LPT Config: cr_30=%02x 60,61=%02x%02x 70=%02x 74=%02x, f0=%02x\n", + cr30, cr60, cr61, cr70, cr74, crf0); + pr_info("Winbond LPT Config: active=%s, io=0x%02x%02x irq=%d, ", + (cr30 & 0x01) ? "yes" : "no", cr60, cr61, cr70 & 0x0f); if ((cr74 & 0x07) > 3) pr_cont("dma=none\n"); else pr_cont("dma=%d\n", cr74 & 0x07); - printk(KERN_INFO - "Winbond LPT Config: irqtype=%s, ECP fifo threshold=%d\n", - irqtypes[crf0>>7], (crf0>>3)&0x0f); - printk(KERN_INFO "Winbond LPT Config: Port mode=%s\n", - modes[crf0 & 0x07]); + pr_info("Winbond LPT Config: irqtype=%s, ECP fifo threshold=%d\n", + irqtypes[crf0 >> 7], (crf0 >> 3) & 0x0f); + pr_info("Winbond LPT Config: Port mode=%s\n", + modes[crf0 & 0x07]); } if (cr30 & 0x01) { /* the settings can be interrogated later ... */ s = find_free_superio(); if (s == NULL) - printk(KERN_INFO "Super-IO: too many chips!\n"); + pr_info("Super-IO: too many chips!\n"); else { s->io = (cr60 << 8) | cr61; s->irq = cr70 & 0x0f; @@ -1151,9 +1145,8 @@ static void decode_winbond(int efer, int key, int devid, int devrev, int oldid) progif = 0; if (verbose_probing) - printk(KERN_INFO "Winbond chip at EFER=0x%x key=0x%02x " - "devid=%02x devrev=%02x oldid=%02x type=%s\n", - efer, key, devid, devrev, oldid, type); + pr_info("Winbond chip at EFER=0x%x key=0x%02x devid=%02x devrev=%02x oldid=%02x type=%s\n", + efer, key, devid, devrev, oldid, type); if (progif == 2) show_parconfig_winbond(efer, key); @@ -1184,9 +1177,8 @@ static void decode_smsc(int efer, int key, int devid, int devrev) type = "37c666GT"; if (verbose_probing) - printk(KERN_INFO "SMSC chip at EFER=0x%x " - "key=0x%02x devid=%02x devrev=%02x type=%s\n", - efer, key, devid, devrev, type); + pr_info("SMSC chip at EFER=0x%x key=0x%02x devid=%02x devrev=%02x type=%s\n", + efer, key, devid, devrev, type); if (func) func(efer, key); @@ -1358,7 +1350,7 @@ static void detect_and_report_it87(void) dev |= inb(0x2f); if (dev == 0x8712 || dev == 0x8705 || dev == 0x8715 || dev == 0x8716 || dev == 0x8718 || dev == 0x8726) { - printk(KERN_INFO "IT%04X SuperIO detected.\n", dev); + pr_info("IT%04X SuperIO detected\n", dev); outb(0x07, 0x2E); /* Parallel Port */ outb(0x03, 0x2F); outb(0xF0, 0x2E); /* BOOT 0x80 off */ @@ -1445,8 +1437,8 @@ static int parport_SPP_supported(struct parport *pb) if (user_specified) /* That didn't work, but the user thinks there's a * port here. */ - printk(KERN_INFO "parport 0x%lx (WARNING): CTR: " - "wrote 0x%02x, read 0x%02x\n", pb->base, w, r); + pr_info("parport 0x%lx (WARNING): CTR: wrote 0x%02x, read 0x%02x\n", + pb->base, w, r); /* Try the data register. The data lines aren't tri-stated at * this stage, so we expect back what we wrote. */ @@ -1464,10 +1456,9 @@ static int parport_SPP_supported(struct parport *pb) if (user_specified) { /* Didn't work, but the user is convinced this is the * place. */ - printk(KERN_INFO "parport 0x%lx (WARNING): DATA: " - "wrote 0x%02x, read 0x%02x\n", pb->base, w, r); - printk(KERN_INFO "parport 0x%lx: You gave this address, " - "but there is probably no parallel port there!\n", + pr_info("parport 0x%lx (WARNING): DATA: wrote 0x%02x, read 0x%02x\n", + pb->base, w, r); + pr_info("parport 0x%lx: You gave this address, but there is probably no parallel port there!\n", pb->base); } @@ -1642,7 +1633,7 @@ static int parport_ECP_supported(struct parport *pb) if (i <= priv->fifo_depth) { if (verbose_probing) - printk(KERN_INFO "0x%lx: readIntrThreshold is %d\n", + pr_info("0x%lx: readIntrThreshold is %d\n", pb->base, i); } else /* Number of bytes we can read if we get an interrupt. */ @@ -1657,17 +1648,14 @@ static int parport_ECP_supported(struct parport *pb) switch (pword) { case 0: pword = 2; - printk(KERN_WARNING "0x%lx: Unsupported pword size!\n", - pb->base); + pr_warn("0x%lx: Unsupported pword size!\n", pb->base); break; case 2: pword = 4; - printk(KERN_WARNING "0x%lx: Unsupported pword size!\n", - pb->base); + pr_warn("0x%lx: Unsupported pword size!\n", pb->base); break; default: - printk(KERN_WARNING "0x%lx: Unknown implementation ID\n", - pb->base); + pr_warn("0x%lx: Unknown implementation ID\n", pb->base); /* Fall through - Assume 1 */ case 1: pword = 1; @@ -2107,9 +2095,9 @@ struct parport *parport_pc_probe_port(unsigned long int base, p->size = (p->modes & PARPORT_MODE_EPP) ? 8 : 3; - printk(KERN_INFO "%s: PC-style at 0x%lx", p->name, p->base); + pr_info("%s: PC-style at 0x%lx", p->name, p->base); if (p->base_hi && priv->ecr) - printk(KERN_CONT " (0x%lx)", p->base_hi); + pr_cont(" (0x%lx)", p->base_hi); if (p->irq == PARPORT_IRQ_AUTO) { p->irq = PARPORT_IRQ_NONE; parport_irq_probe(p); @@ -2120,7 +2108,7 @@ struct parport *parport_pc_probe_port(unsigned long int base, p->irq = PARPORT_IRQ_NONE; } if (p->irq != PARPORT_IRQ_NONE) { - printk(KERN_CONT ", irq %d", p->irq); + pr_cont(", irq %d", p->irq); priv->ctr_writable |= 0x10; if (p->dma == PARPORT_DMA_AUTO) { @@ -2144,21 +2132,21 @@ struct parport *parport_pc_probe_port(unsigned long int base, /* p->ops->ecp_read_data = parport_pc_ecp_read_block_pio; */ #endif /* IEEE 1284 support */ if (p->dma != PARPORT_DMA_NONE) { - printk(KERN_CONT ", dma %d", p->dma); + pr_cont(", dma %d", p->dma); p->modes |= PARPORT_MODE_DMA; } else - printk(KERN_CONT ", using FIFO"); + pr_cont(", using FIFO"); } else /* We can't use the DMA channel after all. */ p->dma = PARPORT_DMA_NONE; #endif /* Allowed to use FIFO/DMA */ - printk(KERN_CONT " ["); + pr_cont(" ["); #define printmode(x) \ {\ if (p->modes & PARPORT_MODE_##x) {\ - printk(KERN_CONT "%s%s", f ? "," : "", #x);\ + pr_cont("%s%s", f ? "," : "", #x); \ f++;\ } \ } @@ -2174,11 +2162,11 @@ struct parport *parport_pc_probe_port(unsigned long int base, } #undef printmode #ifndef CONFIG_PARPORT_1284 - printk(KERN_CONT "(,...)"); + pr_cont("(,...)"); #endif /* CONFIG_PARPORT_1284 */ - printk(KERN_CONT "]\n"); + pr_cont("]\n"); if (probedirq != PARPORT_IRQ_NONE) - printk(KERN_INFO "%s: irq %d detected\n", p->name, probedirq); + pr_info("%s: irq %d detected\n", p->name, probedirq); /* If No ECP release the ports grabbed above. */ if (ECR_res && (p->modes & PARPORT_MODE_ECP) == 0) { @@ -2193,8 +2181,7 @@ struct parport *parport_pc_probe_port(unsigned long int base, if (p->irq != PARPORT_IRQ_NONE) { if (request_irq(p->irq, parport_irq_handler, irqflags, p->name, p)) { - printk(KERN_WARNING "%s: irq %d in use, " - "resorting to polled operation\n", + pr_warn("%s: irq %d in use, resorting to polled operation\n", p->name, p->irq); p->irq = PARPORT_IRQ_NONE; p->dma = PARPORT_DMA_NONE; @@ -2204,8 +2191,7 @@ struct parport *parport_pc_probe_port(unsigned long int base, #ifdef HAS_DMA if (p->dma != PARPORT_DMA_NONE) { if (request_dma(p->dma, p->name)) { - printk(KERN_WARNING "%s: dma %d in use, " - "resorting to PIO operation\n", + pr_warn("%s: dma %d in use, resorting to PIO operation\n", p->name, p->dma); p->dma = PARPORT_DMA_NONE; } else { @@ -2215,9 +2201,7 @@ struct parport *parport_pc_probe_port(unsigned long int base, &priv->dma_handle, GFP_KERNEL); if (!priv->dma_buf) { - printk(KERN_WARNING "%s: " - "cannot get buffer for DMA, " - "resorting to PIO operation\n", + pr_warn("%s: cannot get buffer for DMA, resorting to PIO operation\n", p->name); free_dma(p->dma); p->dma = PARPORT_DMA_NONE; @@ -2330,7 +2314,7 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma, } } if (i >= 5) { - printk(KERN_INFO "parport_pc: cannot find ITE8872 INTA\n"); + pr_info("parport_pc: cannot find ITE8872 INTA\n"); return 0; } @@ -2339,29 +2323,28 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma, switch (type) { case 0x2: - printk(KERN_INFO "parport_pc: ITE8871 found (1P)\n"); + pr_info("parport_pc: ITE8871 found (1P)\n"); ite8872set = 0x64200000; break; case 0xa: - printk(KERN_INFO "parport_pc: ITE8875 found (1P)\n"); + pr_info("parport_pc: ITE8875 found (1P)\n"); ite8872set = 0x64200000; break; case 0xe: - printk(KERN_INFO "parport_pc: ITE8872 found (2S1P)\n"); + pr_info("parport_pc: ITE8872 found (2S1P)\n"); ite8872set = 0x64e00000; break; case 0x6: - printk(KERN_INFO "parport_pc: ITE8873 found (1S)\n"); + pr_info("parport_pc: ITE8873 found (1S)\n"); release_region(inta_addr[i], 32); return 0; case 0x8: - printk(KERN_INFO "parport_pc: ITE8874 found (2S)\n"); + pr_info("parport_pc: ITE8874 found (2S)\n"); release_region(inta_addr[i], 32); return 0; default: - printk(KERN_INFO "parport_pc: unknown ITE887x\n"); - printk(KERN_INFO "parport_pc: please mail 'lspci -nvv' " - "output to Rich.Liu@ite.com.tw\n"); + pr_info("parport_pc: unknown ITE887x\n"); + pr_info("parport_pc: please mail 'lspci -nvv' output to Rich.Liu@ite.com.tw\n"); release_region(inta_addr[i], 32); return 0; } @@ -2396,9 +2379,8 @@ static int sio_ite_8872_probe(struct pci_dev *pdev, int autoirq, int autodma, release_region(inta_addr[i], 32); if (parport_pc_probe_port(ite8872_lpt, ite8872_lpthi, irq, PARPORT_DMA_NONE, &pdev->dev, 0)) { - printk(KERN_INFO - "parport_pc: ITE 8872 parallel port: io=0x%X", - ite8872_lpt); + pr_info("parport_pc: ITE 8872 parallel port: io=0x%X", + ite8872_lpt); if (irq != PARPORT_IRQ_NONE) pr_cont(", irq=%d", irq); pr_cont("\n"); @@ -2525,7 +2507,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, pci_write_config_byte(pdev, via->via_pci_superio_config_reg, tmp); if (siofunc == VIA_FUNCTION_PARPORT_DISABLE) { - printk(KERN_INFO "parport_pc: VIA parallel port disabled in BIOS\n"); + pr_info("parport_pc: VIA parallel port disabled in BIOS\n"); return 0; } @@ -2558,9 +2540,8 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, case 0x278: port2 = 0x678; break; default: - printk(KERN_INFO - "parport_pc: Weird VIA parport base 0x%X, ignoring\n", - port1); + pr_info("parport_pc: Weird VIA parport base 0x%X, ignoring\n", + port1); return 0; } @@ -2579,8 +2560,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, /* finally, do the probe with values obtained */ if (parport_pc_probe_port(port1, port2, irq, dma, &pdev->dev, 0)) { - printk(KERN_INFO - "parport_pc: VIA parallel port: io=0x%X", port1); + pr_info("parport_pc: VIA parallel port: io=0x%X", port1); if (irq != PARPORT_IRQ_NONE) pr_cont(", irq=%d", irq); if (dma != PARPORT_DMA_NONE) @@ -2589,7 +2569,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, return 1; } - printk(KERN_WARNING "parport_pc: Strange, can't probe VIA parallel port: io=0x%X, irq=%d, dma=%d\n", + pr_warn("parport_pc: Strange, can't probe VIA parallel port: io=0x%X, irq=%d, dma=%d\n", port1, irq, dma); return 0; } @@ -3111,7 +3091,7 @@ static int __init parport_parse_param(const char *s, int *val, if (ep != s) *val = r; else { - printk(KERN_ERR "parport: bad specifier `%s'\n", s); + pr_err("parport: bad specifier `%s'\n", s); return -1; } } @@ -3201,10 +3181,7 @@ static int __init parse_parport_params(void) irqval[0] = val; break; default: - printk(KERN_WARNING - "parport_pc: irq specified " - "without base address. Use 'io=' " - "to specify one\n"); + pr_warn("parport_pc: irq specified without base address. Use 'io=' to specify one\n"); } if (dma[0] && !parport_parse_dma(dma[0], &val)) @@ -3214,10 +3191,7 @@ static int __init parse_parport_params(void) dmaval[0] = val; break; default: - printk(KERN_WARNING - "parport_pc: dma specified " - "without base address. Use 'io=' " - "to specify one\n"); + pr_warn("parport_pc: dma specified without base address. Use 'io=' to specify one\n"); } } return 0; @@ -3256,12 +3230,12 @@ static int __init parport_setup(char *str) val = simple_strtoul(str, &endptr, 0); if (endptr == str) { - printk(KERN_WARNING "parport=%s not understood\n", str); + pr_warn("parport=%s not understood\n", str); return 1; } if (parport_setup_ptr == PARPORT_PC_MAX_PORTS) { - printk(KERN_ERR "parport=%s ignored, too many ports\n", str); + pr_err("parport=%s ignored, too many ports\n", str); return 1; } diff --git a/drivers/parport/parport_sunbpp.c b/drivers/parport/parport_sunbpp.c index d5a669..e840c1b 100644 --- a/drivers/parport/parport_sunbpp.c +++ b/drivers/parport/parport_sunbpp.c @@ -314,7 +314,7 @@ static int bpp_probe(struct platform_device *op) value_tcr &= ~P_TCR_DIR; sbus_writeb(value_tcr, ®s->p_tcr); - printk(KERN_INFO "%s: sunbpp at 0x%lx\n", p->name, p->base); + pr_info("%s: sunbpp at 0x%lx\n", p->name, p->base); dev_set_drvdata(&op->dev, p); diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c index e5e6a46..10ed0d1 100644 --- a/drivers/parport/probe.c +++ b/drivers/parport/probe.c @@ -38,7 +38,7 @@ static void pretty_print(struct parport *port, int device) { struct parport_device_info *info = &port->probe_info[device + 1]; - printk(KERN_INFO "%s", port->name); + pr_info("%s", port->name); if (device >= 0) printk (" (addr %d)", device); @@ -58,7 +58,7 @@ static void parse_data(struct parport *port, int device, char *str) struct parport_device_info *info = &port->probe_info[device + 1]; if (!txt) { - printk(KERN_WARNING "%s probe: memory squeeze\n", port->name); + pr_warn("%s probe: memory squeeze\n", port->name); return; } strcpy(txt, str); @@ -98,7 +98,8 @@ static void parse_data(struct parport *port, int device, char *str) goto rock_on; } } - printk(KERN_WARNING "%s probe: warning, class '%s' not understood.\n", port->name, sep); + pr_warn("%s probe: warning, class '%s' not understood\n", + port->name, sep); info->class = PARPORT_CLASS_OTHER; } else if (!strcmp(p, "CMD") || !strcmp(p, "COMMAND SET")) { diff --git a/drivers/parport/share.c b/drivers/parport/share.c index d6920eb..b0f451 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -554,8 +554,8 @@ void parport_announce_port(struct parport *port) #endif if (!port->dev) - printk(KERN_WARNING "%s: fix this legacy no-device port driver!\n", - port->name); + pr_warn("%s: fix this legacy no-device port driver!\n", + port->name); parport_proc_register(port); mutex_lock(®istration_lock); @@ -727,7 +727,8 @@ parport_register_device(struct parport *port, const char *name, if (flags & PARPORT_DEV_LURK) { if (!pf || !kf) { - printk(KERN_INFO "%s: refused to register lurking device (%s) without callbacks\n", port->name, name); + pr_info("%s: refused to register lurking device (%s) without callbacks\n", + port->name, name); return NULL; } } @@ -996,7 +997,7 @@ void parport_unregister_device(struct pardevice *dev) #ifdef PARPORT_PARANOID if (!dev) { - printk(KERN_ERR "parport_unregister_device: passed NULL\n"); + pr_err("%s: passed NULL\n", __func__); return; } #endif @@ -1137,8 +1138,7 @@ int parport_claim(struct pardevice *dev) unsigned long flags; if (port->cad == dev) { - printk(KERN_INFO "%s: %s already owner\n", - dev->port->name,dev->name); + pr_info("%s: %s already owner\n", dev->port->name, dev->name); return 0; } @@ -1158,9 +1158,8 @@ int parport_claim(struct pardevice *dev) * I think we'll actually deadlock rather than * get here, but just in case.. */ - printk(KERN_WARNING - "%s: %s released port when preempted!\n", - port->name, oldcad->name); + pr_warn("%s: %s released port when preempted!\n", + port->name, oldcad->name); if (port->cad) goto blocked; } @@ -1320,8 +1319,8 @@ void parport_release(struct pardevice *dev) write_lock_irqsave(&port->cad_lock, flags); if (port->cad != dev) { write_unlock_irqrestore(&port->cad_lock, flags); - printk(KERN_WARNING "%s: %s tried to release parport when not owner\n", - port->name, dev->name); + pr_warn("%s: %s tried to release parport when not owner\n", + port->name, dev->name); return; } @@ -1361,7 +1360,8 @@ void parport_release(struct pardevice *dev) if (dev->port->cad) /* racy but no matter */ return; } else { - printk(KERN_ERR "%s: don't know how to wake %s\n", port->name, pd->name); + pr_err("%s: don't know how to wake %s\n", + port->name, pd->name); } } -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/7] parport: Use more comon logging styles 2020-02-28 8:32 ` [PATCH 0/7] parport: Use generic kernel logging styles Joe Perches 2020-02-28 8:32 ` [PATCH 1/7] parport: Convert printk(KERN_<LEVEL> to pr_<level>( Joe Perches @ 2020-02-28 8:32 ` Joe Perches 2020-03-02 19:29 ` Helge Deller 2020-02-28 8:32 ` [PATCH 7/7] parport: Standardize use of printmode Joe Perches 2020-02-29 16:40 ` [PATCH 0/7] parport: Use generic kernel logging styles Randy Dunlap 3 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2020-02-28 8:32 UTC (permalink / raw) To: Randy Dunlap, James E.J. Bottomley, Helge Deller, Sudip Mukherjee Cc: linux-parisc, linux-kernel Some of the logging can be poorly formatted because of unexpected line breaks given printks without KERN_CONT that should be pr_cont. Miscellanea: o Remove unnecessary spaces between function name and open parenthesis o Convert bare printks to pr_<level> where appropriate o Convert embedded function names to use %s, __func__ o Coalesce formats o Realign arguments o Use do {} while (0) in a macro and not a bare if Signed-off-by: Joe Perches <joe@perches.com> --- drivers/parport/parport_gsc.c | 10 ++-- drivers/parport/parport_gsc.h | 21 ++++---- drivers/parport/parport_ip32.c | 92 ++++++++++++++++++---------------- drivers/parport/parport_pc.c | 62 +++++++++++------------ drivers/parport/probe.c | 27 ++++------ drivers/parport/share.c | 13 +++-- 6 files changed, 108 insertions(+), 117 deletions(-) diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c index 81082d..cb6a08 100644 --- a/drivers/parport/parport_gsc.c +++ b/drivers/parport/parport_gsc.c @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base, priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL); if (!priv) { - printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base); + printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base); return NULL; } ops = kmemdup(&parport_gsc_ops, sizeof(struct parport_operations), GFP_KERNEL); if (!ops) { - printk (KERN_DEBUG "parport (0x%lx): no memory for ops!\n", - base); + printk(KERN_DEBUG "parport (0x%lx): no memory for ops!\n", + base); kfree (priv); return NULL; } @@ -359,11 +359,11 @@ static int __init parport_init_chip(struct parisc_device *dev) if (boot_cpu_data.cpu_type > pcxt && !pdc_add_valid(port+4)) { /* Initialize bidirectional-mode (0x10) & data-tranfer-mode #1 (0x20) */ - printk("%s: initialize bidirectional-mode.\n", __func__); + pr_info("%s: initialize bidirectional-mode\n", __func__); parport_writeb ( (0x10 + 0x20), port + 4); } else { - printk("%s: enhanced parport-modes not supported.\n", __func__); + pr_info("%s: enhanced parport-modes not supported\n", __func__); } p = parport_gsc_probe_port(port, 0, dev->irq, diff --git a/drivers/parport/parport_gsc.h b/drivers/parport/parport_gsc.h index 4c4d3c..9301217 100644 --- a/drivers/parport/parport_gsc.h +++ b/drivers/parport/parport_gsc.h @@ -71,7 +71,7 @@ struct parport_gsc_private { static inline void parport_gsc_write_data(struct parport *p, unsigned char d) { #ifdef DEBUG_PARPORT - printk (KERN_DEBUG "parport_gsc_write_data(%p,0x%02x)\n", p, d); + printk(KERN_DEBUG "%s(%p,0x%02x)\n", __func__, p, d); #endif parport_writeb(d, DATA(p)); } @@ -80,8 +80,7 @@ static inline unsigned char parport_gsc_read_data(struct parport *p) { unsigned char val = parport_readb (DATA (p)); #ifdef DEBUG_PARPORT - printk (KERN_DEBUG "parport_gsc_read_data(%p) = 0x%02x\n", - p, val); + printk(KERN_DEBUG "%s(%p) = 0x%02x\n", __func__, p, val); #endif return val; } @@ -95,9 +94,9 @@ static inline unsigned char __parport_gsc_frob_control(struct parport *p, struct parport_gsc_private *priv = p->physport->private_data; unsigned char ctr = priv->ctr; #ifdef DEBUG_PARPORT - printk (KERN_DEBUG - "__parport_gsc_frob_control(%02x,%02x): %02x -> %02x\n", - mask, val, ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable); + printk(KERN_DEBUG "%s(%02x,%02x): %02x -> %02x\n", + __func__, mask, val, + ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable); #endif ctr = (ctr & ~mask) ^ val; ctr &= priv->ctr_writable; /* only write writable bits. */ @@ -126,8 +125,8 @@ static inline void parport_gsc_write_control(struct parport *p, /* Take this out when drivers have adapted to newer interface. */ if (d & 0x20) { - printk (KERN_DEBUG "%s (%s): use data_reverse for this!\n", - p->name, p->cad->name); + printk(KERN_DEBUG "%s (%s): use data_reverse for this!\n", + p->name, p->cad->name); parport_gsc_data_reverse (p); } @@ -155,9 +154,9 @@ static inline unsigned char parport_gsc_frob_control(struct parport *p, /* Take this out when drivers have adapted to newer interface. */ if (mask & 0x20) { - printk (KERN_DEBUG "%s (%s): use data_%s for this!\n", - p->name, p->cad->name, - (val & 0x20) ? "reverse" : "forward"); + printk(KERN_DEBUG "%s (%s): use data_%s for this!\n", + p->name, p->cad->name, + (val & 0x20) ? "reverse" : "forward"); if (val & 0x20) parport_gsc_data_reverse (p); else diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c index 4f76e4..48b084 100644 --- a/drivers/parport/parport_ip32.c +++ b/drivers/parport/parport_ip32.c @@ -328,19 +328,19 @@ static void parport_ip32_dump_state(struct parport *p, char *str, "TST", "CFG"}; unsigned int ecr = readb(priv->regs.ecr); printk(KERN_DEBUG PPIP32 " ecr=0x%02x", ecr); - printk(" %s", - ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]); + pr_cont(" %s", + ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]); if (ecr & ECR_nERRINTR) - printk(",nErrIntrEn"); + pr_cont(",nErrIntrEn"); if (ecr & ECR_DMAEN) - printk(",dmaEn"); + pr_cont(",dmaEn"); if (ecr & ECR_SERVINTR) - printk(",serviceIntr"); + pr_cont(",serviceIntr"); if (ecr & ECR_F_FULL) - printk(",f_full"); + pr_cont(",f_full"); if (ecr & ECR_F_EMPTY) - printk(",f_empty"); - printk("\n"); + pr_cont(",f_empty"); + pr_cont("\n"); } if (show_ecp_config) { unsigned int oecr, cnfgA, cnfgB; @@ -352,52 +352,53 @@ static void parport_ip32_dump_state(struct parport *p, char *str, writeb(ECR_MODE_PS2, priv->regs.ecr); writeb(oecr, priv->regs.ecr); printk(KERN_DEBUG PPIP32 " cnfgA=0x%02x", cnfgA); - printk(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses"); + pr_cont(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses"); switch (cnfgA & CNFGA_ID_MASK) { case CNFGA_ID_8: - printk(",8 bits"); + pr_cont(",8 bits"); break; case CNFGA_ID_16: - printk(",16 bits"); + pr_cont(",16 bits"); break; case CNFGA_ID_32: - printk(",32 bits"); + pr_cont(",32 bits"); break; default: - printk(",unknown ID"); + pr_cont(",unknown ID"); break; } if (!(cnfgA & CNFGA_nBYTEINTRANS)) - printk(",ByteInTrans"); + pr_cont(",ByteInTrans"); if ((cnfgA & CNFGA_ID_MASK) != CNFGA_ID_8) - printk(",%d byte%s left", cnfgA & CNFGA_PWORDLEFT, - ((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : ""); - printk("\n"); + pr_cont(",%d byte%s left", + cnfgA & CNFGA_PWORDLEFT, + ((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : ""); + pr_cont("\n"); printk(KERN_DEBUG PPIP32 " cnfgB=0x%02x", cnfgB); - printk(" irq=%u,dma=%u", - (cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT, - (cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT); - printk(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL)); + pr_cont(" irq=%u,dma=%u", + (cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT, + (cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT); + pr_cont(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL)); if (cnfgB & CNFGB_COMPRESS) - printk(",compress"); - printk("\n"); + pr_cont(",compress"); + pr_cont("\n"); } for (i = 0; i < 2; i++) { unsigned int dcr = i ? priv->dcr_cache : readb(priv->regs.dcr); printk(KERN_DEBUG PPIP32 " dcr(%s)=0x%02x", i ? "soft" : "hard", dcr); - printk(" %s", (dcr & DCR_DIR) ? "rev" : "fwd"); + pr_cont(" %s", (dcr & DCR_DIR) ? "rev" : "fwd"); if (dcr & DCR_IRQ) - printk(",ackIntEn"); + pr_cont(",ackIntEn"); if (!(dcr & DCR_SELECT)) - printk(",nSelectIn"); + pr_cont(",nSelectIn"); if (dcr & DCR_nINIT) - printk(",nInit"); + pr_cont(",nInit"); if (!(dcr & DCR_AUTOFD)) - printk(",nAutoFD"); + pr_cont(",nAutoFD"); if (!(dcr & DCR_STROBE)) - printk(",nStrobe"); - printk("\n"); + pr_cont(",nStrobe"); + pr_cont("\n"); } #define sep (f++ ? ',' : ' ') { @@ -405,20 +406,20 @@ static void parport_ip32_dump_state(struct parport *p, char *str, unsigned int dsr = readb(priv->regs.dsr); printk(KERN_DEBUG PPIP32 " dsr=0x%02x", dsr); if (!(dsr & DSR_nBUSY)) - printk("%cBusy", sep); + pr_cont("%cBusy", sep); if (dsr & DSR_nACK) - printk("%cnAck", sep); + pr_cont("%cnAck", sep); if (dsr & DSR_PERROR) - printk("%cPError", sep); + pr_cont("%cPError", sep); if (dsr & DSR_SELECT) - printk("%cSelect", sep); + pr_cont("%cSelect", sep); if (dsr & DSR_nFAULT) - printk("%cnFault", sep); + pr_cont("%cnFault", sep); if (!(dsr & DSR_nPRINT)) - printk("%c(Print)", sep); + pr_cont("%c(Print)", sep); if (dsr & DSR_TIMEOUT) - printk("%cTimeout", sep); - printk("\n"); + pr_cont("%cTimeout", sep); + pr_cont("\n"); } #undef sep } @@ -1703,7 +1704,7 @@ static size_t parport_ip32_ecp_write_data(struct parport *p, /* Event 49: PError goes high. */ if (parport_wait_peripheral(p, DSR_PERROR, DSR_PERROR)) { - printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s", + printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s\n", p->name, __func__); physport->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN; return 0; @@ -2132,10 +2133,13 @@ static __init struct parport *parport_ip32_probe_port(void) /* Print out what we found */ pr_info("%s: SGI IP32 at 0x%lx (0x%lx)", p->name, p->base, p->base_hi); if (p->irq != PARPORT_IRQ_NONE) - printk(", irq %d", p->irq); - printk(" ["); -#define printmode(x) if (p->modes & PARPORT_MODE_##x) \ - printk("%s%s", f++ ? "," : "", #x) + pr_cont(", irq %d", p->irq); + pr_cont(" ["); +#define printmode(x) \ +do { \ + if (p->modes & PARPORT_MODE_##x) \ + pr_cont("%s%s", f++ ? "," : "", #x); \ +} while (0) { unsigned int f = 0; printmode(PCSPP); @@ -2146,7 +2150,7 @@ static __init struct parport *parport_ip32_probe_port(void) printmode(DMA); } #undef printmode - printk("]\n"); + pr_cont("]\n"); parport_announce_port(p); return p; diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c index 2872c67..ab3b04f 100644 --- a/drivers/parport/parport_pc.c +++ b/drivers/parport/parport_pc.c @@ -298,8 +298,8 @@ static size_t parport_pc_epp_read_data(struct parport *port, void *buf, status = inb(STATUS(port)); if (status & 0x01) { /* EPP timeout should never occur... */ - printk(KERN_DEBUG -"%s: EPP timeout occurred while talking to w91284pic (should not have done)\n", port->name); + printk(KERN_DEBUG "%s: EPP timeout occurred while talking to w91284pic (should not have done)\n", + port->name); clear_epp_timeout(port); } } @@ -727,7 +727,7 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port, r = change_mode(port, ECR_PPF); /* Parallel port FIFO */ if (r) printk(KERN_DEBUG "%s: Warning change_mode ECR_PPF failed\n", - port->name); + port->name); port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA; @@ -770,9 +770,8 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port, PARPORT_STATUS_BUSY, PARPORT_STATUS_BUSY); if (r) - printk(KERN_DEBUG - "%s: BUSY timeout (%d) in compat_write_block_pio\n", - port->name, r); + printk(KERN_DEBUG "%s: BUSY timeout (%d) in compat_write_block_pio\n", + port->name, r); port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE; @@ -810,8 +809,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, PARPORT_STATUS_PAPEROUT, PARPORT_STATUS_PAPEROUT); if (r) { - printk(KERN_DEBUG "%s: PError timeout (%d) " - "in ecp_write_block_pio\n", port->name, r); + printk(KERN_DEBUG "%s: PError timeout (%d) in ecp_write_block_pio\n", + port->name, r); } } @@ -824,7 +823,7 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, r = change_mode(port, ECR_ECP); /* ECP FIFO */ if (r) printk(KERN_DEBUG "%s: Warning change_mode ECR_ECP failed\n", - port->name); + port->name); port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA; /* Write the data to the FIFO. */ @@ -867,8 +866,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, parport_frob_control(port, PARPORT_CONTROL_INIT, 0); r = parport_wait_peripheral(port, PARPORT_STATUS_PAPEROUT, 0); if (r) - printk(KERN_DEBUG "%s: PE,1 timeout (%d) " - "in ecp_write_block_pio\n", port->name, r); + printk(KERN_DEBUG "%s: PE,1 timeout (%d) in ecp_write_block_pio\n", + port->name, r); parport_frob_control(port, PARPORT_CONTROL_INIT, @@ -877,17 +876,16 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, PARPORT_STATUS_PAPEROUT, PARPORT_STATUS_PAPEROUT); if (r) - printk(KERN_DEBUG "%s: PE,2 timeout (%d) " - "in ecp_write_block_pio\n", port->name, r); + printk(KERN_DEBUG "%s: PE,2 timeout (%d) in ecp_write_block_pio\n", + port->name, r); } r = parport_wait_peripheral(port, PARPORT_STATUS_BUSY, PARPORT_STATUS_BUSY); if (r) - printk(KERN_DEBUG - "%s: BUSY timeout (%d) in ecp_write_block_pio\n", - port->name, r); + printk(KERN_DEBUG "%s: BUSY timeout (%d) in ecp_write_block_pio\n", + port->name, r); port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE; @@ -1611,7 +1609,7 @@ static int parport_ECP_supported(struct parport *pb) if (i <= priv->fifo_depth) { if (verbose_probing) printk(KERN_DEBUG "0x%lx: writeIntrThreshold is %d\n", - pb->base, i); + pb->base, i); } else /* Number of bytes we know we can write if we get an interrupt. */ @@ -1664,14 +1662,14 @@ static int parport_ECP_supported(struct parport *pb) if (verbose_probing) { printk(KERN_DEBUG "0x%lx: PWord is %d bits\n", - pb->base, 8 * pword); + pb->base, 8 * pword); - printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n", pb->base, - config & 0x80 ? "Level" : "Pulses"); + printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n", + pb->base, config & 0x80 ? "Level" : "Pulses"); configb = inb(CONFIGB(pb)); printk(KERN_DEBUG "0x%lx: ECP port cfgA=0x%02x cfgB=0x%02x\n", - pb->base, config, configb); + pb->base, config, configb); printk(KERN_DEBUG "0x%lx: ECP settings irq=", pb->base); if ((configb >> 3) & 0x07) pr_cont("%d", intrline[(configb >> 3) & 0x07]); @@ -2453,8 +2451,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, have_epp = 1; break; default: - printk(KERN_DEBUG - "parport_pc: probing current configuration\n"); + printk(KERN_DEBUG "parport_pc: probing current configuration\n"); siofunc = VIA_FUNCTION_PROBE; break; } @@ -2490,12 +2487,11 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, port1 = inb(VIA_CONFIG_DATA) << 2; printk(KERN_DEBUG "parport_pc: Current parallel port base: 0x%X\n", - port1); + port1); if (port1 == 0x3BC && have_epp) { outb(via->viacfg_parport_base, VIA_CONFIG_INDEX); outb((0x378 >> 2), VIA_CONFIG_DATA); - printk(KERN_DEBUG - "parport_pc: Parallel port base changed to 0x378\n"); + printk(KERN_DEBUG "parport_pc: Parallel port base changed to 0x378\n"); port1 = 0x378; } @@ -2834,14 +2830,12 @@ static int parport_pc_pci_probe(struct pci_dev *dev, /* TODO: test if sharing interrupts works */ irq = dev->irq; if (irq == IRQ_NONE) { - printk(KERN_DEBUG - "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n", - id->vendor, id->device, io_lo, io_hi); + printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n", + id->vendor, id->device, io_lo, io_hi); irq = PARPORT_IRQ_NONE; } else { - printk(KERN_DEBUG - "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n", - id->vendor, id->device, io_lo, io_hi, irq); + printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n", + id->vendor, id->device, io_lo, io_hi, irq); } data->ports[count] = parport_pc_probe_port(io_lo, io_hi, irq, @@ -3113,8 +3107,8 @@ static int __init parport_parse_dma(const char *dmastr, int *val) #ifdef CONFIG_PCI static int __init parport_init_mode_setup(char *str) { - printk(KERN_DEBUG - "parport_pc.c: Specified parameter parport_init_mode=%s\n", str); + printk(KERN_DEBUG "parport_pc.c: Specified parameter parport_init_mode=%s\n", + str); if (!strcmp(str, "spp")) parport_init_mode = 1; diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c index 10ed0d1..7e6d71 100644 --- a/drivers/parport/probe.c +++ b/drivers/parport/probe.c @@ -41,13 +41,13 @@ static void pretty_print(struct parport *port, int device) pr_info("%s", port->name); if (device >= 0) - printk (" (addr %d)", device); + pr_cont(" (addr %d)", device); - printk (": %s", classes[info->class].descr); + pr_cont(": %s", classes[info->class].descr); if (info->class) - printk(", %s %s", info->mfr, info->model); + pr_cont(", %s %s", info->mfr, info->model); - printk("\n"); + pr_cont("\n"); } static void parse_data(struct parport *port, int device, char *str) @@ -178,9 +178,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, * just return constant nibble forever. This catches * also those cases. */ if (idlens[0] == 0 || idlens[0] > 0xFFF) { - printk (KERN_DEBUG "%s: reported broken Device ID" - " length of %#zX bytes\n", - port->name, idlens[0]); + printk(KERN_DEBUG "%s: reported broken Device ID length of %#zX bytes\n", + port->name, idlens[0]); return -EIO; } numidlens = 2; @@ -202,10 +201,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, if (port->physport->ieee1284.phase != IEEE1284_PH_HBUSY_DAVAIL) { if (belen != len) { - printk (KERN_DEBUG "%s: Device ID was %zd bytes" - " while device told it would be %d" - " bytes\n", - port->name, len, belen); + printk(KERN_DEBUG "%s: Device ID was %zd bytes while device told it would be %d bytes\n", + port->name, len, belen); } goto done; } @@ -215,11 +212,9 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, * the first 256 bytes or so that we must have read so * far. */ if (buffer[len-1] == ';') { - printk (KERN_DEBUG "%s: Device ID reading stopped" - " before device told data not available. " - "Current idlen %u of %u, len bytes %02X %02X\n", - port->name, current_idlen, numidlens, - length[0], length[1]); + printk(KERN_DEBUG "%s: Device ID reading stopped before device told data not available. Current idlen %u of %u, len bytes %02X %02X\n", + port->name, current_idlen, numidlens, + length[0], length[1]); goto done; } } diff --git a/drivers/parport/share.c b/drivers/parport/share.c index b0f451..3169fee 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -720,8 +720,7 @@ parport_register_device(struct parport *port, const char *name, if (port->physport->flags & PARPORT_FLAG_EXCL) { /* An exclusive device is registered. */ - printk(KERN_DEBUG "%s: no more devices allowed\n", - port->name); + printk(KERN_DEBUG "%s: no more devices allowed\n", port->name); return NULL; } @@ -789,9 +788,8 @@ parport_register_device(struct parport *port, const char *name, if (flags & PARPORT_DEV_EXCL) { if (port->physport->devices) { spin_unlock(&port->physport->pardevice_lock); - printk(KERN_DEBUG - "%s: cannot grant exclusive access for device %s\n", - port->name, name); + printk(KERN_DEBUG "%s: cannot grant exclusive access for device %s\n", + port->name, name); goto out_free_all; } port->flags |= PARPORT_FLAG_EXCL; @@ -1259,7 +1257,8 @@ int parport_claim_or_block(struct pardevice *dev) r = parport_claim(dev); if (r == -EAGAIN) { #ifdef PARPORT_DEBUG_SHARING - printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n", dev->name); + printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n", + dev->name); #endif /* * FIXME!!! Use the proper locking for dev->waiting, @@ -1292,7 +1291,7 @@ int parport_claim_or_block(struct pardevice *dev) if (dev->port->physport->cad != dev) printk(KERN_DEBUG "%s: exiting parport_claim_or_block but %s owns port!\n", dev->name, dev->port->physport->cad ? - dev->port->physport->cad->name:"nobody"); + dev->port->physport->cad->name : "nobody"); #endif } dev->waiting = 0; -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/7] parport: Use more comon logging styles 2020-02-28 8:32 ` [PATCH 2/7] parport: Use more comon logging styles Joe Perches @ 2020-03-02 19:29 ` Helge Deller 2020-03-02 19:39 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Helge Deller @ 2020-03-02 19:29 UTC (permalink / raw) To: Joe Perches, Randy Dunlap, James E.J. Bottomley, Sudip Mukherjee Cc: linux-parisc, linux-kernel On 28.02.20 09:32, Joe Perches wrote: > Some of the logging can be poorly formatted because of unexpected > line breaks given printks without KERN_CONT that should be pr_cont. > > Miscellanea: > > o Remove unnecessary spaces between function name and open parenthesis > o Convert bare printks to pr_<level> where appropriate > o Convert embedded function names to use %s, __func__ > o Coalesce formats > o Realign arguments > o Use do {} while (0) in a macro and not a bare if > > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/parport/parport_gsc.c | 10 ++-- > drivers/parport/parport_gsc.h | 21 ++++---- > drivers/parport/parport_ip32.c | 92 ++++++++++++++++++---------------- > drivers/parport/parport_pc.c | 62 +++++++++++------------ > drivers/parport/probe.c | 27 ++++------ > drivers/parport/share.c | 13 +++-- > 6 files changed, 108 insertions(+), 117 deletions(-) > > diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c > index 81082d..cb6a08 100644 > --- a/drivers/parport/parport_gsc.c > +++ b/drivers/parport/parport_gsc.c > @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base, > > priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL); > if (!priv) { > - printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base); > + printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base); pr_warn() instead? > return NULL; > } > ops = kmemdup(&parport_gsc_ops, sizeof(struct parport_operations), > GFP_KERNEL); > if (!ops) { > - printk (KERN_DEBUG "parport (0x%lx): no memory for ops!\n", > - base); > + printk(KERN_DEBUG "parport (0x%lx): no memory for ops!\n", > + base); pr_warn() instead too ? > kfree (priv); > return NULL; > } > @@ -359,11 +359,11 @@ static int __init parport_init_chip(struct parisc_device *dev) > if (boot_cpu_data.cpu_type > pcxt && !pdc_add_valid(port+4)) { > > /* Initialize bidirectional-mode (0x10) & data-tranfer-mode #1 (0x20) */ > - printk("%s: initialize bidirectional-mode.\n", __func__); > + pr_info("%s: initialize bidirectional-mode\n", __func__); > parport_writeb ( (0x10 + 0x20), port + 4); > > } else { > - printk("%s: enhanced parport-modes not supported.\n", __func__); > + pr_info("%s: enhanced parport-modes not supported\n", __func__); > } > > p = parport_gsc_probe_port(port, 0, dev->irq, > diff --git a/drivers/parport/parport_gsc.h b/drivers/parport/parport_gsc.h > index 4c4d3c..9301217 100644 > --- a/drivers/parport/parport_gsc.h > +++ b/drivers/parport/parport_gsc.h > @@ -71,7 +71,7 @@ struct parport_gsc_private { > static inline void parport_gsc_write_data(struct parport *p, unsigned char d) > { > #ifdef DEBUG_PARPORT > - printk (KERN_DEBUG "parport_gsc_write_data(%p,0x%02x)\n", p, d); > + printk(KERN_DEBUG "%s(%p,0x%02x)\n", __func__, p, d); pr_debug() ? > #endif > parport_writeb(d, DATA(p)); > } > @@ -80,8 +80,7 @@ static inline unsigned char parport_gsc_read_data(struct parport *p) > { > unsigned char val = parport_readb (DATA (p)); > #ifdef DEBUG_PARPORT > - printk (KERN_DEBUG "parport_gsc_read_data(%p) = 0x%02x\n", > - p, val); > + printk(KERN_DEBUG "%s(%p) = 0x%02x\n", __func__, p, val); pr_debug() ? > #endif > return val; > } > @@ -95,9 +94,9 @@ static inline unsigned char __parport_gsc_frob_control(struct parport *p, > struct parport_gsc_private *priv = p->physport->private_data; > unsigned char ctr = priv->ctr; > #ifdef DEBUG_PARPORT > - printk (KERN_DEBUG > - "__parport_gsc_frob_control(%02x,%02x): %02x -> %02x\n", > - mask, val, ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable); > + printk(KERN_DEBUG "%s(%02x,%02x): %02x -> %02x\n", pr_debug() ? > + __func__, mask, val, > + ctr, ((ctr & ~mask) ^ val) & priv->ctr_writable); > #endif > ctr = (ctr & ~mask) ^ val; > ctr &= priv->ctr_writable; /* only write writable bits. */ > @@ -126,8 +125,8 @@ static inline void parport_gsc_write_control(struct parport *p, > > /* Take this out when drivers have adapted to newer interface. */ > if (d & 0x20) { > - printk (KERN_DEBUG "%s (%s): use data_reverse for this!\n", > - p->name, p->cad->name); > + printk(KERN_DEBUG "%s (%s): use data_reverse for this!\n", > + p->name, p->cad->name); here too? > parport_gsc_data_reverse (p); > } > > @@ -155,9 +154,9 @@ static inline unsigned char parport_gsc_frob_control(struct parport *p, > > /* Take this out when drivers have adapted to newer interface. */ > if (mask & 0x20) { > - printk (KERN_DEBUG "%s (%s): use data_%s for this!\n", > - p->name, p->cad->name, > - (val & 0x20) ? "reverse" : "forward"); > + printk(KERN_DEBUG "%s (%s): use data_%s for this!\n", and here? > + p->name, p->cad->name, > + (val & 0x20) ? "reverse" : "forward"); > if (val & 0x20) > parport_gsc_data_reverse (p); > else > diff --git a/drivers/parport/parport_ip32.c b/drivers/parport/parport_ip32.c > index 4f76e4..48b084 100644 > --- a/drivers/parport/parport_ip32.c > +++ b/drivers/parport/parport_ip32.c > @@ -328,19 +328,19 @@ static void parport_ip32_dump_state(struct parport *p, char *str, > "TST", "CFG"}; > unsigned int ecr = readb(priv->regs.ecr); > printk(KERN_DEBUG PPIP32 " ecr=0x%02x", ecr); > - printk(" %s", > - ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]); > + pr_cont(" %s", > + ecr_modes[(ecr & ECR_MODE_MASK) >> ECR_MODE_SHIFT]); > if (ecr & ECR_nERRINTR) > - printk(",nErrIntrEn"); > + pr_cont(",nErrIntrEn"); > if (ecr & ECR_DMAEN) > - printk(",dmaEn"); > + pr_cont(",dmaEn"); > if (ecr & ECR_SERVINTR) > - printk(",serviceIntr"); > + pr_cont(",serviceIntr"); > if (ecr & ECR_F_FULL) > - printk(",f_full"); > + pr_cont(",f_full"); > if (ecr & ECR_F_EMPTY) > - printk(",f_empty"); > - printk("\n"); > + pr_cont(",f_empty"); > + pr_cont("\n"); > } > if (show_ecp_config) { > unsigned int oecr, cnfgA, cnfgB; > @@ -352,52 +352,53 @@ static void parport_ip32_dump_state(struct parport *p, char *str, > writeb(ECR_MODE_PS2, priv->regs.ecr); > writeb(oecr, priv->regs.ecr); > printk(KERN_DEBUG PPIP32 " cnfgA=0x%02x", cnfgA); > - printk(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses"); > + pr_cont(" ISA-%s", (cnfgA & CNFGA_IRQ) ? "Level" : "Pulses"); > switch (cnfgA & CNFGA_ID_MASK) { > case CNFGA_ID_8: > - printk(",8 bits"); > + pr_cont(",8 bits"); > break; > case CNFGA_ID_16: > - printk(",16 bits"); > + pr_cont(",16 bits"); > break; > case CNFGA_ID_32: > - printk(",32 bits"); > + pr_cont(",32 bits"); > break; > default: > - printk(",unknown ID"); > + pr_cont(",unknown ID"); > break; > } > if (!(cnfgA & CNFGA_nBYTEINTRANS)) > - printk(",ByteInTrans"); > + pr_cont(",ByteInTrans"); > if ((cnfgA & CNFGA_ID_MASK) != CNFGA_ID_8) > - printk(",%d byte%s left", cnfgA & CNFGA_PWORDLEFT, > - ((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : ""); > - printk("\n"); > + pr_cont(",%d byte%s left", > + cnfgA & CNFGA_PWORDLEFT, > + ((cnfgA & CNFGA_PWORDLEFT) > 1) ? "s" : ""); > + pr_cont("\n"); > printk(KERN_DEBUG PPIP32 " cnfgB=0x%02x", cnfgB); > - printk(" irq=%u,dma=%u", > - (cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT, > - (cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT); > - printk(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL)); > + pr_cont(" irq=%u,dma=%u", > + (cnfgB & CNFGB_IRQ_MASK) >> CNFGB_IRQ_SHIFT, > + (cnfgB & CNFGB_DMA_MASK) >> CNFGB_DMA_SHIFT); > + pr_cont(",intrValue=%d", !!(cnfgB & CNFGB_INTRVAL)); > if (cnfgB & CNFGB_COMPRESS) > - printk(",compress"); > - printk("\n"); > + pr_cont(",compress"); > + pr_cont("\n"); > } > for (i = 0; i < 2; i++) { > unsigned int dcr = i ? priv->dcr_cache : readb(priv->regs.dcr); > printk(KERN_DEBUG PPIP32 " dcr(%s)=0x%02x", > i ? "soft" : "hard", dcr); > - printk(" %s", (dcr & DCR_DIR) ? "rev" : "fwd"); > + pr_cont(" %s", (dcr & DCR_DIR) ? "rev" : "fwd"); > if (dcr & DCR_IRQ) > - printk(",ackIntEn"); > + pr_cont(",ackIntEn"); > if (!(dcr & DCR_SELECT)) > - printk(",nSelectIn"); > + pr_cont(",nSelectIn"); > if (dcr & DCR_nINIT) > - printk(",nInit"); > + pr_cont(",nInit"); > if (!(dcr & DCR_AUTOFD)) > - printk(",nAutoFD"); > + pr_cont(",nAutoFD"); > if (!(dcr & DCR_STROBE)) > - printk(",nStrobe"); > - printk("\n"); > + pr_cont(",nStrobe"); > + pr_cont("\n"); > } > #define sep (f++ ? ',' : ' ') > { > @@ -405,20 +406,20 @@ static void parport_ip32_dump_state(struct parport *p, char *str, > unsigned int dsr = readb(priv->regs.dsr); > printk(KERN_DEBUG PPIP32 " dsr=0x%02x", dsr); > if (!(dsr & DSR_nBUSY)) > - printk("%cBusy", sep); > + pr_cont("%cBusy", sep); > if (dsr & DSR_nACK) > - printk("%cnAck", sep); > + pr_cont("%cnAck", sep); > if (dsr & DSR_PERROR) > - printk("%cPError", sep); > + pr_cont("%cPError", sep); > if (dsr & DSR_SELECT) > - printk("%cSelect", sep); > + pr_cont("%cSelect", sep); > if (dsr & DSR_nFAULT) > - printk("%cnFault", sep); > + pr_cont("%cnFault", sep); > if (!(dsr & DSR_nPRINT)) > - printk("%c(Print)", sep); > + pr_cont("%c(Print)", sep); > if (dsr & DSR_TIMEOUT) > - printk("%cTimeout", sep); > - printk("\n"); > + pr_cont("%cTimeout", sep); > + pr_cont("\n"); > } > #undef sep > } > @@ -1703,7 +1704,7 @@ static size_t parport_ip32_ecp_write_data(struct parport *p, > > /* Event 49: PError goes high. */ > if (parport_wait_peripheral(p, DSR_PERROR, DSR_PERROR)) { > - printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s", > + printk(KERN_DEBUG PPIP32 "%s: PError timeout in %s\n", and here? > p->name, __func__); > physport->ieee1284.phase = IEEE1284_PH_ECP_DIR_UNKNOWN; > return 0; > @@ -2132,10 +2133,13 @@ static __init struct parport *parport_ip32_probe_port(void) > /* Print out what we found */ > pr_info("%s: SGI IP32 at 0x%lx (0x%lx)", p->name, p->base, p->base_hi); > if (p->irq != PARPORT_IRQ_NONE) > - printk(", irq %d", p->irq); > - printk(" ["); > -#define printmode(x) if (p->modes & PARPORT_MODE_##x) \ > - printk("%s%s", f++ ? "," : "", #x) > + pr_cont(", irq %d", p->irq); > + pr_cont(" ["); > +#define printmode(x) \ > +do { \ > + if (p->modes & PARPORT_MODE_##x) \ > + pr_cont("%s%s", f++ ? "," : "", #x); \ > +} while (0) > { > unsigned int f = 0; > printmode(PCSPP); > @@ -2146,7 +2150,7 @@ static __init struct parport *parport_ip32_probe_port(void) > printmode(DMA); > } > #undef printmode > - printk("]\n"); > + pr_cont("]\n"); > > parport_announce_port(p); > return p; > diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c > index 2872c67..ab3b04f 100644 > --- a/drivers/parport/parport_pc.c > +++ b/drivers/parport/parport_pc.c > @@ -298,8 +298,8 @@ static size_t parport_pc_epp_read_data(struct parport *port, void *buf, > status = inb(STATUS(port)); > if (status & 0x01) { > /* EPP timeout should never occur... */ > - printk(KERN_DEBUG > -"%s: EPP timeout occurred while talking to w91284pic (should not have done)\n", port->name); > + printk(KERN_DEBUG "%s: EPP timeout occurred while talking to w91284pic (should not have done)\n", here. > + port->name); > clear_epp_timeout(port); > } > } > @@ -727,7 +727,7 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port, > r = change_mode(port, ECR_PPF); /* Parallel port FIFO */ > if (r) > printk(KERN_DEBUG "%s: Warning change_mode ECR_PPF failed\n", and here? There are more such below... Helge > - port->name); > + port->name); > > port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA; > > @@ -770,9 +770,8 @@ static size_t parport_pc_compat_write_block_pio(struct parport *port, > PARPORT_STATUS_BUSY, > PARPORT_STATUS_BUSY); > if (r) > - printk(KERN_DEBUG > - "%s: BUSY timeout (%d) in compat_write_block_pio\n", > - port->name, r); > + printk(KERN_DEBUG "%s: BUSY timeout (%d) in compat_write_block_pio\n", > + port->name, r); > > port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE; > > @@ -810,8 +809,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, > PARPORT_STATUS_PAPEROUT, > PARPORT_STATUS_PAPEROUT); > if (r) { > - printk(KERN_DEBUG "%s: PError timeout (%d) " > - "in ecp_write_block_pio\n", port->name, r); > + printk(KERN_DEBUG "%s: PError timeout (%d) in ecp_write_block_pio\n", > + port->name, r); > } > } > > @@ -824,7 +823,7 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, > r = change_mode(port, ECR_ECP); /* ECP FIFO */ > if (r) > printk(KERN_DEBUG "%s: Warning change_mode ECR_ECP failed\n", > - port->name); > + port->name); > port->physport->ieee1284.phase = IEEE1284_PH_FWD_DATA; > > /* Write the data to the FIFO. */ > @@ -867,8 +866,8 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, > parport_frob_control(port, PARPORT_CONTROL_INIT, 0); > r = parport_wait_peripheral(port, PARPORT_STATUS_PAPEROUT, 0); > if (r) > - printk(KERN_DEBUG "%s: PE,1 timeout (%d) " > - "in ecp_write_block_pio\n", port->name, r); > + printk(KERN_DEBUG "%s: PE,1 timeout (%d) in ecp_write_block_pio\n", > + port->name, r); > > parport_frob_control(port, > PARPORT_CONTROL_INIT, > @@ -877,17 +876,16 @@ static size_t parport_pc_ecp_write_block_pio(struct parport *port, > PARPORT_STATUS_PAPEROUT, > PARPORT_STATUS_PAPEROUT); > if (r) > - printk(KERN_DEBUG "%s: PE,2 timeout (%d) " > - "in ecp_write_block_pio\n", port->name, r); > + printk(KERN_DEBUG "%s: PE,2 timeout (%d) in ecp_write_block_pio\n", > + port->name, r); > } > > r = parport_wait_peripheral(port, > PARPORT_STATUS_BUSY, > PARPORT_STATUS_BUSY); > if (r) > - printk(KERN_DEBUG > - "%s: BUSY timeout (%d) in ecp_write_block_pio\n", > - port->name, r); > + printk(KERN_DEBUG "%s: BUSY timeout (%d) in ecp_write_block_pio\n", > + port->name, r); > > port->physport->ieee1284.phase = IEEE1284_PH_FWD_IDLE; > > @@ -1611,7 +1609,7 @@ static int parport_ECP_supported(struct parport *pb) > if (i <= priv->fifo_depth) { > if (verbose_probing) > printk(KERN_DEBUG "0x%lx: writeIntrThreshold is %d\n", > - pb->base, i); > + pb->base, i); > } else > /* Number of bytes we know we can write if we get an > interrupt. */ > @@ -1664,14 +1662,14 @@ static int parport_ECP_supported(struct parport *pb) > > if (verbose_probing) { > printk(KERN_DEBUG "0x%lx: PWord is %d bits\n", > - pb->base, 8 * pword); > + pb->base, 8 * pword); > > - printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n", pb->base, > - config & 0x80 ? "Level" : "Pulses"); > + printk(KERN_DEBUG "0x%lx: Interrupts are ISA-%s\n", > + pb->base, config & 0x80 ? "Level" : "Pulses"); > > configb = inb(CONFIGB(pb)); > printk(KERN_DEBUG "0x%lx: ECP port cfgA=0x%02x cfgB=0x%02x\n", > - pb->base, config, configb); > + pb->base, config, configb); > printk(KERN_DEBUG "0x%lx: ECP settings irq=", pb->base); > if ((configb >> 3) & 0x07) > pr_cont("%d", intrline[(configb >> 3) & 0x07]); > @@ -2453,8 +2451,7 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, > have_epp = 1; > break; > default: > - printk(KERN_DEBUG > - "parport_pc: probing current configuration\n"); > + printk(KERN_DEBUG "parport_pc: probing current configuration\n"); > siofunc = VIA_FUNCTION_PROBE; > break; > } > @@ -2490,12 +2487,11 @@ static int sio_via_probe(struct pci_dev *pdev, int autoirq, int autodma, > port1 = inb(VIA_CONFIG_DATA) << 2; > > printk(KERN_DEBUG "parport_pc: Current parallel port base: 0x%X\n", > - port1); > + port1); > if (port1 == 0x3BC && have_epp) { > outb(via->viacfg_parport_base, VIA_CONFIG_INDEX); > outb((0x378 >> 2), VIA_CONFIG_DATA); > - printk(KERN_DEBUG > - "parport_pc: Parallel port base changed to 0x378\n"); > + printk(KERN_DEBUG "parport_pc: Parallel port base changed to 0x378\n"); > port1 = 0x378; > } > > @@ -2834,14 +2830,12 @@ static int parport_pc_pci_probe(struct pci_dev *dev, > /* TODO: test if sharing interrupts works */ > irq = dev->irq; > if (irq == IRQ_NONE) { > - printk(KERN_DEBUG > - "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n", > - id->vendor, id->device, io_lo, io_hi); > + printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx)\n", > + id->vendor, id->device, io_lo, io_hi); > irq = PARPORT_IRQ_NONE; > } else { > - printk(KERN_DEBUG > - "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n", > - id->vendor, id->device, io_lo, io_hi, irq); > + printk(KERN_DEBUG "PCI parallel port detected: %04x:%04x, I/O at %#lx(%#lx), IRQ %d\n", > + id->vendor, id->device, io_lo, io_hi, irq); > } > data->ports[count] = > parport_pc_probe_port(io_lo, io_hi, irq, > @@ -3113,8 +3107,8 @@ static int __init parport_parse_dma(const char *dmastr, int *val) > #ifdef CONFIG_PCI > static int __init parport_init_mode_setup(char *str) > { > - printk(KERN_DEBUG > - "parport_pc.c: Specified parameter parport_init_mode=%s\n", str); > + printk(KERN_DEBUG "parport_pc.c: Specified parameter parport_init_mode=%s\n", > + str); > > if (!strcmp(str, "spp")) > parport_init_mode = 1; > diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c > index 10ed0d1..7e6d71 100644 > --- a/drivers/parport/probe.c > +++ b/drivers/parport/probe.c > @@ -41,13 +41,13 @@ static void pretty_print(struct parport *port, int device) > pr_info("%s", port->name); > > if (device >= 0) > - printk (" (addr %d)", device); > + pr_cont(" (addr %d)", device); > > - printk (": %s", classes[info->class].descr); > + pr_cont(": %s", classes[info->class].descr); > if (info->class) > - printk(", %s %s", info->mfr, info->model); > + pr_cont(", %s %s", info->mfr, info->model); > > - printk("\n"); > + pr_cont("\n"); > } > > static void parse_data(struct parport *port, int device, char *str) > @@ -178,9 +178,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, > * just return constant nibble forever. This catches > * also those cases. */ > if (idlens[0] == 0 || idlens[0] > 0xFFF) { > - printk (KERN_DEBUG "%s: reported broken Device ID" > - " length of %#zX bytes\n", > - port->name, idlens[0]); > + printk(KERN_DEBUG "%s: reported broken Device ID length of %#zX bytes\n", > + port->name, idlens[0]); > return -EIO; > } > numidlens = 2; > @@ -202,10 +201,8 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, > > if (port->physport->ieee1284.phase != IEEE1284_PH_HBUSY_DAVAIL) { > if (belen != len) { > - printk (KERN_DEBUG "%s: Device ID was %zd bytes" > - " while device told it would be %d" > - " bytes\n", > - port->name, len, belen); > + printk(KERN_DEBUG "%s: Device ID was %zd bytes while device told it would be %d bytes\n", > + port->name, len, belen); > } > goto done; > } > @@ -215,11 +212,9 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, > * the first 256 bytes or so that we must have read so > * far. */ > if (buffer[len-1] == ';') { > - printk (KERN_DEBUG "%s: Device ID reading stopped" > - " before device told data not available. " > - "Current idlen %u of %u, len bytes %02X %02X\n", > - port->name, current_idlen, numidlens, > - length[0], length[1]); > + printk(KERN_DEBUG "%s: Device ID reading stopped before device told data not available. Current idlen %u of %u, len bytes %02X %02X\n", > + port->name, current_idlen, numidlens, > + length[0], length[1]); > goto done; > } > } > diff --git a/drivers/parport/share.c b/drivers/parport/share.c > index b0f451..3169fee 100644 > --- a/drivers/parport/share.c > +++ b/drivers/parport/share.c > @@ -720,8 +720,7 @@ parport_register_device(struct parport *port, const char *name, > > if (port->physport->flags & PARPORT_FLAG_EXCL) { > /* An exclusive device is registered. */ > - printk(KERN_DEBUG "%s: no more devices allowed\n", > - port->name); > + printk(KERN_DEBUG "%s: no more devices allowed\n", port->name); > return NULL; > } > > @@ -789,9 +788,8 @@ parport_register_device(struct parport *port, const char *name, > if (flags & PARPORT_DEV_EXCL) { > if (port->physport->devices) { > spin_unlock(&port->physport->pardevice_lock); > - printk(KERN_DEBUG > - "%s: cannot grant exclusive access for device %s\n", > - port->name, name); > + printk(KERN_DEBUG "%s: cannot grant exclusive access for device %s\n", > + port->name, name); > goto out_free_all; > } > port->flags |= PARPORT_FLAG_EXCL; > @@ -1259,7 +1257,8 @@ int parport_claim_or_block(struct pardevice *dev) > r = parport_claim(dev); > if (r == -EAGAIN) { > #ifdef PARPORT_DEBUG_SHARING > - printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n", dev->name); > + printk(KERN_DEBUG "%s: parport_claim() returned -EAGAIN\n", > + dev->name); > #endif > /* > * FIXME!!! Use the proper locking for dev->waiting, > @@ -1292,7 +1291,7 @@ int parport_claim_or_block(struct pardevice *dev) > if (dev->port->physport->cad != dev) > printk(KERN_DEBUG "%s: exiting parport_claim_or_block but %s owns port!\n", > dev->name, dev->port->physport->cad ? > - dev->port->physport->cad->name:"nobody"); > + dev->port->physport->cad->name : "nobody"); > #endif > } > dev->waiting = 0; > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/7] parport: Use more comon logging styles 2020-03-02 19:29 ` Helge Deller @ 2020-03-02 19:39 ` Joe Perches 2020-03-02 20:35 ` Helge Deller 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2020-03-02 19:39 UTC (permalink / raw) To: Helge Deller, Randy Dunlap, James E.J. Bottomley, Sudip Mukherjee Cc: linux-parisc, linux-kernel On Mon, 2020-03-02 at 20:29 +0100, Helge Deller wrote: > On 28.02.20 09:32, Joe Perches wrote: > > Some of the logging can be poorly formatted because of unexpected > > line breaks given printks without KERN_CONT that should be pr_cont. [] > > diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c [] > > @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base, > > > > priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL); > > if (!priv) { > > - printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base); > > + printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base); > > pr_warn() instead? For all of your remarks, the intent here is to keep the same output. Logging level changes and printk(KERN_DEBUG -> pr_debug( conversions cause the dmesg output to be different. printk(KERN_DEBUG...) is always emitted when the console level allows but pr_debug(...) is not normally compiled in at all. So it's possible for all printk(KERN_DEBUG to be pr_debug but that causes no logging at all to be emitted when DEBUG is not defined or CONFIG_DYNAMIC_DEBUG not enabled. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/7] parport: Use more comon logging styles 2020-03-02 19:39 ` Joe Perches @ 2020-03-02 20:35 ` Helge Deller 0 siblings, 0 replies; 10+ messages in thread From: Helge Deller @ 2020-03-02 20:35 UTC (permalink / raw) To: Joe Perches, Randy Dunlap, James E.J. Bottomley, Sudip Mukherjee Cc: linux-parisc, linux-kernel On 02.03.20 20:39, Joe Perches wrote: > On Mon, 2020-03-02 at 20:29 +0100, Helge Deller wrote: >> On 28.02.20 09:32, Joe Perches wrote: >>> Some of the logging can be poorly formatted because of unexpected >>> line breaks given printks without KERN_CONT that should be pr_cont. > [] >>> diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c > [] >>> @@ -238,14 +238,14 @@ struct parport *parport_gsc_probe_port(unsigned long base, >>> >>> priv = kzalloc (sizeof (struct parport_gsc_private), GFP_KERNEL); >>> if (!priv) { >>> - printk (KERN_DEBUG "parport (0x%lx): no memory!\n", base); >>> + printk(KERN_DEBUG "parport (0x%lx): no memory!\n", base); >> >> pr_warn() instead? > > For all of your remarks, the intent here is to keep the > same output. Ok. > Logging level changes and printk(KERN_DEBUG -> pr_debug( > conversions cause the dmesg output to be different. > > printk(KERN_DEBUG...) > > is always emitted when the console level allows but > > pr_debug(...) > > is not normally compiled in at all. > > So it's possible for all printk(KERN_DEBUG to be pr_debug > but that causes no logging at all to be emitted when > DEBUG is not defined or CONFIG_DYNAMIC_DEBUG not enabled. Ok, then keep it as you had. Thanks! Helge ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 7/7] parport: Standardize use of printmode 2020-02-28 8:32 ` [PATCH 0/7] parport: Use generic kernel logging styles Joe Perches 2020-02-28 8:32 ` [PATCH 1/7] parport: Convert printk(KERN_<LEVEL> to pr_<level>( Joe Perches 2020-02-28 8:32 ` [PATCH 2/7] parport: Use more comon logging styles Joe Perches @ 2020-02-28 8:32 ` Joe Perches 2020-02-29 16:40 ` [PATCH 0/7] parport: Use generic kernel logging styles Randy Dunlap 3 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2020-02-28 8:32 UTC (permalink / raw) To: Randy Dunlap, James E.J. Bottomley, Helge Deller, Sudip Mukherjee Cc: linux-parisc, linux-kernel Standardize the define and the uses of printmode. Miscellanea: o Add missing statement termination ; where necessary Signed-off-by: Joe Perches <joe@perches.com> --- drivers/parport/parport_gsc.c | 8 ++++++-- drivers/parport/parport_pc.c | 14 ++++++-------- drivers/parport/procfs.c | 6 +++++- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/drivers/parport/parport_gsc.c b/drivers/parport/parport_gsc.c index cb6a08..9228e8 100644 --- a/drivers/parport/parport_gsc.c +++ b/drivers/parport/parport_gsc.c @@ -299,12 +299,16 @@ struct parport *parport_gsc_probe_port(unsigned long base, p->dma = PARPORT_DMA_NONE; pr_cont(" ["); -#define printmode(x) {if(p->modes&PARPORT_MODE_##x){pr_cont("%s%s",f?",":"",#x);f++;}} +#define printmode(x) \ +do { \ + if (p->modes & PARPORT_MODE_##x) \ + pr_cont("%s%s", f++ ? "," : "", #x); \ +} while (0) { int f = 0; printmode(PCSPP); printmode(TRISTATE); - printmode(COMPAT) + printmode(COMPAT); printmode(EPP); // printmode(ECP); // printmode(DMA); diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c index aae03b..77e37e3 100644 --- a/drivers/parport/parport_pc.c +++ b/drivers/parport/parport_pc.c @@ -2134,19 +2134,17 @@ struct parport *parport_pc_probe_port(unsigned long int base, pr_cont(" ["); -#define printmode(x) \ - {\ - if (p->modes & PARPORT_MODE_##x) {\ - pr_cont("%s%s", f ? "," : "", #x); \ - f++;\ - } \ - } +#define printmode(x) \ +do { \ + if (p->modes & PARPORT_MODE_##x) \ + pr_cont("%s%s", f++ ? "," : "", #x); \ +} while (0) { int f = 0; printmode(PCSPP); printmode(TRISTATE); - printmode(COMPAT) + printmode(COMPAT); printmode(EPP); printmode(ECP); printmode(DMA); diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index 488040..e957be 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -213,7 +213,11 @@ static int do_hardware_modes(struct ctl_table *table, int write, return -EACCES; { -#define printmode(x) {if(port->modes&PARPORT_MODE_##x){len+=sprintf(buffer+len,"%s%s",f?",":"",#x);f++;}} +#define printmode(x) \ +do { \ + if (port->modes & PARPORT_MODE_##x) \ + len += sprintf(buffer + len, "%s%s", f++ ? "," : "", #x); \ +} while (0) int f = 0; printmode(PCSPP); printmode(TRISTATE); -- 2.24.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/7] parport: Use generic kernel logging styles 2020-02-28 8:32 ` [PATCH 0/7] parport: Use generic kernel logging styles Joe Perches ` (2 preceding siblings ...) 2020-02-28 8:32 ` [PATCH 7/7] parport: Standardize use of printmode Joe Perches @ 2020-02-29 16:40 ` Randy Dunlap 2020-02-29 19:33 ` Joe Perches 3 siblings, 1 reply; 10+ messages in thread From: Randy Dunlap @ 2020-02-29 16:40 UTC (permalink / raw) To: Joe Perches, linux-parisc Cc: Sudip Mukherjee, James E.J. Bottomley, Helge Deller, linux-kernel On 2/28/20 12:32 AM, Joe Perches wrote: > Well, if the parport logging is getting some generic fixing, > here's some more generic logging fixing... > > Joe Perches (7): > parport: Convert printk(KERN_<LEVEL> to pr_<level>( > parport: Use more comon logging styles > parport: daisy: Convert DPRINTK to pr_debug > parport_amiga: Convert DPRINTK to pr_debug > parport_mfc3: Convert DPRINTK to pr_debug > parport_pc: Convert DPRINTK to pr_debug > parport: Standardize use of printmode Reviewed-by: Randy Dunlap <rdunlap@infradead.org> Thanks, Joe. > drivers/parport/daisy.c | 29 +--- > drivers/parport/ieee1284.c | 4 +- > drivers/parport/ieee1284_ops.c | 3 +- > drivers/parport/parport_amiga.c | 22 +-- > drivers/parport/parport_atari.c | 2 +- > drivers/parport/parport_cs.c | 6 +- > drivers/parport/parport_gsc.c | 25 +-- > drivers/parport/parport_gsc.h | 21 ++- > drivers/parport/parport_ip32.c | 117 +++++++------- > drivers/parport/parport_mfc3.c | 21 +-- > drivers/parport/parport_pc.c | 263 +++++++++++++------------------ > drivers/parport/parport_sunbpp.c | 2 +- > drivers/parport/probe.c | 34 ++-- > drivers/parport/procfs.c | 6 +- > drivers/parport/share.c | 37 +++-- > 15 files changed, 261 insertions(+), 331 deletions(-) > -- ~Randy ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/7] parport: Use generic kernel logging styles 2020-02-29 16:40 ` [PATCH 0/7] parport: Use generic kernel logging styles Randy Dunlap @ 2020-02-29 19:33 ` Joe Perches 2020-03-06 13:49 ` Sudip Mukherjee 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2020-02-29 19:33 UTC (permalink / raw) To: Randy Dunlap, linux-parisc Cc: Sudip Mukherjee, James E.J. Bottomley, Helge Deller, linux-kernel On Sat, 2020-02-29 at 08:40 -0800, Randy Dunlap wrote: > On 2/28/20 12:32 AM, Joe Perches wrote: > > Well, if the parport logging is getting some generic fixing, > > here's some more generic logging fixing... > > > > Joe Perches (7): > > parport: Convert printk(KERN_<LEVEL> to pr_<level>( > > parport: Use more comon logging styles > > parport: daisy: Convert DPRINTK to pr_debug > > parport_amiga: Convert DPRINTK to pr_debug > > parport_mfc3: Convert DPRINTK to pr_debug > > parport_pc: Convert DPRINTK to pr_debug > > parport: Standardize use of printmode > > Reviewed-by: Randy Dunlap <rdunlap@infradead.org> btw: Sudip After this recent daisy change: --------------------------------------------------------------------- commit 60f8a59ddcdc7fb7c17180ba10d9c49bc91156c7 Author: Sudip Mukherjee <sudipm.mukherjee@gmail.com> Date: Wed Oct 16 15:45:40 2019 +0100 parport: daisy: use new parport device model Modify parport daisy driver to use the new parallel port device model. Last attempt was '1aec4211204d ("parport: daisy: use new parport device model")' which failed as daisy was also trying to load the low level driver and that resulted in a deadlock. --------------------------------------------------------------------- parport_register_device is no longer in use and parport_register_dev_model is used instead. Documentation/driver-api/parport-lowlevel.rst should be updated and the various comments can be updated or deleted. $ git grep -w parport_register_device Documentation/driver-api/parport-lowlevel.rst: parport_register_device Documentation/driver-api/parport-lowlevel.rst: dev[count++] = parport_register_device (...); Documentation/driver-api/parport-lowlevel.rst:parport_unregister_driver, parport_register_device, parport_enumerate Documentation/driver-api/parport-lowlevel.rst:parport_register_device - register to use a port Documentation/driver-api/parport-lowlevel.rst: struct pardevice *parport_register_device(struct parport *port, Documentation/driver-api/parport-lowlevel.rst: private->dev = parport_register_device (port, "toaster", preempt, Documentation/driver-api/parport-lowlevel.rst:This function is the opposite of parport_register_device. After using Documentation/driver-api/parport-lowlevel.rst:This is like parport_register_device but takes a device number instead Documentation/driver-api/parport-lowlevel.rst:See parport_register_device. If no device is associated with devnum, Documentation/driver-api/parport-lowlevel.rst:parport_register_device drivers/parport/daisy.c: * This function is similar to parport_register_device(), except drivers/parport/daisy.c: * parport_register_device(). The return value is the same as drivers/parport/daisy.c: * for parport_register_device(). drivers/parport/daisy.c: * parport_register_device(). drivers/parport/share.c: * parport_register_device() on that port will do this for you. drivers/parport/share.c: * parport_register_device - register a device on a parallel port drivers/parport/share.c:parport_register_device(struct parport *port, const char *name, drivers/parport/share.c:EXPORT_SYMBOL(parport_register_device); drivers/parport/share.c: * This undoes the effect of parport_register_device(). include/linux/parport.h:/* parport_register_device declares that a device is connected to a include/linux/parport.h:struct pardevice *parport_register_device(struct parport *port, scripts/kernel-doc: # - parport_register_device (function pointer parameters) Maybe something like this is necessary: (it will need to be edited, I just deleted the references) --- Documentation/driver-api/parport-lowlevel.rst | 72 ++-------- drivers/auxdisplay/panel.c | 2 +- drivers/parport/daisy.c | 10 +- drivers/parport/share.c | 197 +------------------------- include/linux/parport.h | 12 -- scripts/kernel-doc | 1 - 6 files changed, 17 insertions(+), 277 deletions(-) diff --git a/Documentation/driver-api/parport-lowlevel.rst b/Documentation/driver-api/parport-lowlevel.rst index 0633d70..42b1c87 100644 --- a/Documentation/driver-api/parport-lowlevel.rst +++ b/Documentation/driver-api/parport-lowlevel.rst @@ -10,7 +10,7 @@ Global functions:: parport_register_driver parport_unregister_driver parport_enumerate - parport_register_device + parport_register_dev_model parport_unregister_device parport_claim parport_claim_or_block @@ -205,7 +205,7 @@ EXAMPLE { ... private = kmalloc (...); - dev[count++] = parport_register_device (...); + dev[count++] = parport_register_dev_model (...); ... } @@ -235,7 +235,7 @@ EXAMPLE SEE ALSO ^^^^^^^^ -parport_unregister_driver, parport_register_device, parport_enumerate +parport_unregister_driver, parport_enumerate @@ -354,7 +354,7 @@ parport_register_driver, parport_unregister_driver -parport_register_device - register to use a port +parport_register_dev_model - register to use a port ------------------------------------------------ SYNOPSIS @@ -368,60 +368,21 @@ SYNOPSIS typedef void (*wakeup_func) (void *handle); typedef int (*irq_func) (int irq, void *handle, struct pt_regs *); - struct pardevice *parport_register_device(struct parport *port, - const char *name, - preempt_func preempt, - wakeup_func wakeup, - irq_func irq, - int flags, - void *handle); + struct pardevice *parport_register_dev_model(struct parport *port, + const char *name, + const struct pardev_cb *par_dev_cb, + int cnt); DESCRIPTION ^^^^^^^^^^^ Use this function to register your device driver on a parallel port -(``port``). Once you have done that, you will be able to use -parport_claim and parport_release in order to use the port. +(``port``). The (``name``) argument is the name of the device that appears in /proc filesystem. The string must be valid for the whole lifetime of the device (until parport_unregister_device is called). -This function will register three callbacks into your driver: -``preempt``, ``wakeup`` and ``irq``. Each of these may be NULL in order to -indicate that you do not want a callback. - -When the ``preempt`` function is called, it is because another driver -wishes to use the parallel port. The ``preempt`` function should return -non-zero if the parallel port cannot be released yet -- if zero is -returned, the port is lost to another driver and the port must be -re-claimed before use. - -The ``wakeup`` function is called once another driver has released the -port and no other driver has yet claimed it. You can claim the -parallel port from within the ``wakeup`` function (in which case the -claim is guaranteed to succeed), or choose not to if you don't need it -now. - -If an interrupt occurs on the parallel port your driver has claimed, -the ``irq`` function will be called. (Write something about shared -interrupts here.) - -The ``handle`` is a pointer to driver-specific data, and is passed to -the callback functions. - -``flags`` may be a bitwise combination of the following flags: - - ===================== ================================================= - Flag Meaning - ===================== ================================================= - PARPORT_DEV_EXCL The device cannot share the parallel port at all. - Use this only when absolutely necessary. - ===================== ================================================= - -The typedefs are not actually defined -- they are only shown in order -to make the function prototype more readable. - The visible parts of the returned ``struct pardevice`` are:: struct pardevice { @@ -468,9 +429,7 @@ EXAMPLE static int toaster_detect (struct toaster *private, struct parport *port) { - private->dev = parport_register_device (port, "toaster", preempt, - wakeup, NULL, 0, - private); + private->dev = parport_register_dev_model(port, "toaster", ...); if (!private->dev) /* Couldn't register with parport. */ return -EIO; @@ -511,7 +470,7 @@ SYNPOPSIS DESCRIPTION ^^^^^^^^^^^ -This function is the opposite of parport_register_device. After using +This function is the opposite of parport_register_dev_model. After using parport_unregister_device, ``dev`` is no longer a valid device handle. You should not unregister a device that is currently claimed, although @@ -879,20 +838,15 @@ SYNOPSIS DESCRIPTION ^^^^^^^^^^^ -This is like parport_register_device but takes a device number instead +This is like parport_register_dev_model but takes a device number instead of a pointer to a struct parport. RETURN VALUE ^^^^^^^^^^^^ -See parport_register_device. If no device is associated with devnum, +See parport_register_dev_model. If no device is associated with devnum, NULL is returned. -SEE ALSO -^^^^^^^^ - -parport_register_device - parport_close - unregister device for particular device number diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c index 85965953..cf4b12 100644 --- a/drivers/auxdisplay/panel.c +++ b/drivers/auxdisplay/panel.c @@ -1589,7 +1589,7 @@ static void panel_attach(struct parport *port) pprt = parport_register_dev_model(port, "panel", &panel_cb, 0); if (!pprt) { - pr_err("%s: port->number=%d parport=%d, parport_register_device() failed\n", + pr_err("%s: port->number=%d parport=%d, parport_register_dev_model() failed\n", __func__, port->number, parport); return; } diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c index 6d78ec..7dc641 100644 --- a/drivers/parport/daisy.c +++ b/drivers/parport/daisy.c @@ -228,14 +228,6 @@ void parport_daisy_fini(struct parport *port) * parport_open - find a device by canonical device number * @devnum: canonical device number * @name: name to associate with the device - * - * This function is similar to parport_register_device(), except - * that it locates a device by its number rather than by the port - * it is attached to. - * - * All parameters except for @devnum are the same as for - * parport_register_device(). The return value is the same as - * for parport_register_device(). **/ struct pardevice *parport_open(int devnum, const char *name) @@ -289,7 +281,7 @@ struct pardevice *parport_open(int devnum, const char *name) * @dev: device to close * * This is to parport_open() as parport_unregister_device() is to - * parport_register_device(). + * parport_register_dev_model(). **/ void parport_close(struct pardevice *dev) diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 3169fee..484723 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -262,8 +262,7 @@ static int port_detect(struct device *dev, void *dev_drv) * The driver's attach() function may block. The port that * attach() is given will be valid for the duration of the * callback, but if the driver wants to take a copy of the - * pointer it must call parport_get_port() to do so. Calling - * parport_register_device() on that port will do this for you. + * pointer it must call parport_get_port() to do so. * * The driver's detach() function may block. The port that * detach() is given will be valid for the duration of the @@ -641,198 +640,6 @@ void parport_remove_port(struct parport *port) } EXPORT_SYMBOL(parport_remove_port); -/** - * parport_register_device - register a device on a parallel port - * @port: port to which the device is attached - * @name: a name to refer to the device - * @pf: preemption callback - * @kf: kick callback (wake-up) - * @irq_func: interrupt handler - * @flags: registration flags - * @handle: data for callback functions - * - * This function, called by parallel port device drivers, - * declares that a device is connected to a port, and tells the - * system all it needs to know. - * - * The @name is allocated by the caller and must not be - * deallocated until the caller calls @parport_unregister_device - * for that device. - * - * The preemption callback function, @pf, is called when this - * device driver has claimed access to the port but another - * device driver wants to use it. It is given @handle as its - * parameter, and should return zero if it is willing for the - * system to release the port to another driver on its behalf. - * If it wants to keep control of the port it should return - * non-zero, and no action will be taken. It is good manners for - * the driver to try to release the port at the earliest - * opportunity after its preemption callback rejects a preemption - * attempt. Note that if a preemption callback is happy for - * preemption to go ahead, there is no need to release the port; - * it is done automatically. This function may not block, as it - * may be called from interrupt context. If the device driver - * does not support preemption, @pf can be %NULL. - * - * The wake-up ("kick") callback function, @kf, is called when - * the port is available to be claimed for exclusive access; that - * is, parport_claim() is guaranteed to succeed when called from - * inside the wake-up callback function. If the driver wants to - * claim the port it should do so; otherwise, it need not take - * any action. This function may not block, as it may be called - * from interrupt context. If the device driver does not want to - * be explicitly invited to claim the port in this way, @kf can - * be %NULL. - * - * The interrupt handler, @irq_func, is called when an interrupt - * arrives from the parallel port. Note that if a device driver - * wants to use interrupts it should use parport_enable_irq(), - * and can also check the irq member of the parport structure - * representing the port. - * - * The parallel port (lowlevel) driver is the one that has called - * request_irq() and whose interrupt handler is called first. - * This handler does whatever needs to be done to the hardware to - * acknowledge the interrupt (for PC-style ports there is nothing - * special to be done). It then tells the IEEE 1284 code about - * the interrupt, which may involve reacting to an IEEE 1284 - * event depending on the current IEEE 1284 phase. After this, - * it calls @irq_func. Needless to say, @irq_func will be called - * from interrupt context, and may not block. - * - * The %PARPORT_DEV_EXCL flag is for preventing port sharing, and - * so should only be used when sharing the port with other device - * drivers is impossible and would lead to incorrect behaviour. - * Use it sparingly! Normally, @flags will be zero. - * - * This function returns a pointer to a structure that represents - * the device on the port, or %NULL if there is not enough memory - * to allocate space for that structure. - **/ - -struct pardevice * -parport_register_device(struct parport *port, const char *name, - int (*pf)(void *), void (*kf)(void *), - void (*irq_func)(void *), - int flags, void *handle) -{ - struct pardevice *tmp; - - if (port->physport->flags & PARPORT_FLAG_EXCL) { - /* An exclusive device is registered. */ - printk(KERN_DEBUG "%s: no more devices allowed\n", port->name); - return NULL; - } - - if (flags & PARPORT_DEV_LURK) { - if (!pf || !kf) { - pr_info("%s: refused to register lurking device (%s) without callbacks\n", - port->name, name); - return NULL; - } - } - - if (flags & PARPORT_DEV_EXCL) { - if (port->physport->devices) { - /* - * If a device is already registered and this new - * device wants exclusive access, then no need to - * continue as we can not grant exclusive access to - * this device. - */ - pr_err("%s: cannot grant exclusive access for device %s\n", - port->name, name); - return NULL; - } - } - - /* - * We up our own module reference count, and that of the port - * on which a device is to be registered, to ensure that - * neither of us gets unloaded while we sleep in (e.g.) - * kmalloc. - */ - if (!try_module_get(port->ops->owner)) - return NULL; - - parport_get_port(port); - - tmp = kmalloc(sizeof(struct pardevice), GFP_KERNEL); - if (!tmp) - goto out; - - tmp->state = kmalloc(sizeof(struct parport_state), GFP_KERNEL); - if (!tmp->state) - goto out_free_pardevice; - - tmp->name = name; - tmp->port = port; - tmp->daisy = -1; - tmp->preempt = pf; - tmp->wakeup = kf; - tmp->private = handle; - tmp->flags = flags; - tmp->irq_func = irq_func; - tmp->waiting = 0; - tmp->timeout = 5 * HZ; - tmp->devmodel = false; - - /* Chain this onto the list */ - tmp->prev = NULL; - /* - * This function must not run from an irq handler so we don' t need - * to clear irq on the local CPU. -arca - */ - spin_lock(&port->physport->pardevice_lock); - - if (flags & PARPORT_DEV_EXCL) { - if (port->physport->devices) { - spin_unlock(&port->physport->pardevice_lock); - printk(KERN_DEBUG "%s: cannot grant exclusive access for device %s\n", - port->name, name); - goto out_free_all; - } - port->flags |= PARPORT_FLAG_EXCL; - } - - tmp->next = port->physport->devices; - wmb(); /* - * Make sure that tmp->next is written before it's - * added to the list; see comments marked 'no locking - * required' - */ - if (port->physport->devices) - port->physport->devices->prev = tmp; - port->physport->devices = tmp; - spin_unlock(&port->physport->pardevice_lock); - - init_waitqueue_head(&tmp->wait_q); - tmp->timeslice = parport_default_timeslice; - tmp->waitnext = tmp->waitprev = NULL; - - /* - * This has to be run as last thing since init_state may need other - * pardevice fields. -arca - */ - port->ops->init_state(tmp, tmp->state); - if (!test_and_set_bit(PARPORT_DEVPROC_REGISTERED, &port->devflags)) { - port->proc_device = tmp; - parport_device_proc_register(tmp); - } - return tmp; - - out_free_all: - kfree(tmp->state); - out_free_pardevice: - kfree(tmp); - out: - parport_put_port(port); - module_put(port->ops->owner); - - return NULL; -} -EXPORT_SYMBOL(parport_register_device); - static void free_pardevice(struct device *dev) { struct pardevice *par_dev = to_pardevice(dev); @@ -986,7 +793,7 @@ EXPORT_SYMBOL(parport_register_dev_model); * parport_unregister_device - deregister a device on a parallel port * @dev: pointer to structure representing device * - * This undoes the effect of parport_register_device(). + * This undoes the effect of parport_register_dev_model(). **/ void parport_unregister_device(struct pardevice *dev) diff --git a/include/linux/parport.h b/include/linux/parport.h index 13932ce..b7a5d06 100644 --- a/include/linux/parport.h +++ b/include/linux/parport.h @@ -325,18 +325,6 @@ struct pardev_cb { unsigned int flags; }; -/* parport_register_device declares that a device is connected to a - port, and tells the kernel all it needs to know. - - pf is the preemption function (may be NULL for no callback) - - kf is the wake-up function (may be NULL for no callback) - - irq_func is the interrupt handler (may be NULL for no interrupts) - - handle is a user pointer that gets handed to callback functions. */ -struct pardevice *parport_register_device(struct parport *port, - const char *name, - int (*pf)(void *), void (*kf)(void *), - void (*irq_func)(void *), - int flags, void *handle); - struct pardevice * parport_register_dev_model(struct parport *port, const char *name, const struct pardev_cb *par_dev_cb, int cnt); diff --git a/scripts/kernel-doc b/scripts/kernel-doc index f2d73f..acd0e3 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -1607,7 +1607,6 @@ sub dump_function($$) { # # If you mess with these regexps, it's a good idea to check that # the following functions' documentation still comes out right: - # - parport_register_device (function pointer parameters) # - atomic_set (macro) # - pci_match_device, __copy_to_user (long return type) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/7] parport: Use generic kernel logging styles 2020-02-29 19:33 ` Joe Perches @ 2020-03-06 13:49 ` Sudip Mukherjee 0 siblings, 0 replies; 10+ messages in thread From: Sudip Mukherjee @ 2020-03-06 13:49 UTC (permalink / raw) To: Joe Perches Cc: Randy Dunlap, linux-parisc, James E.J. Bottomley, Helge Deller, linux-kernel On Sat, Feb 29, 2020 at 7:35 PM Joe Perches <joe@perches.com> wrote: > > On Sat, 2020-02-29 at 08:40 -0800, Randy Dunlap wrote: > > On 2/28/20 12:32 AM, Joe Perches wrote: > > > Well, if the parport logging is getting some generic fixing, > > > here's some more generic logging fixing... > > > > > > Joe Perches (7): > > > parport: Convert printk(KERN_<LEVEL> to pr_<level>( > > > parport: Use more comon logging styles > > > parport: daisy: Convert DPRINTK to pr_debug > > > parport_amiga: Convert DPRINTK to pr_debug > > > parport_mfc3: Convert DPRINTK to pr_debug > > > parport_pc: Convert DPRINTK to pr_debug > > > parport: Standardize use of printmode > > > > Reviewed-by: Randy Dunlap <rdunlap@infradead.org> > > btw: Sudip > > After this recent daisy change: > --------------------------------------------------------------------- > > commit 60f8a59ddcdc7fb7c17180ba10d9c49bc91156c7 > Author: Sudip Mukherjee <sudipm.mukherjee@gmail.com> > Date: Wed Oct 16 15:45:40 2019 +0100 > > parport: daisy: use new parport device model > > Modify parport daisy driver to use the new parallel port device model. > > Last attempt was '1aec4211204d ("parport: daisy: use new parport device > model")' which failed as daisy was also trying to load the low level > driver and that resulted in a deadlock. > > --------------------------------------------------------------------- > > parport_register_device is no longer in use and > parport_register_dev_model is used instead. Yes, I will do the cleanup, if you have not done that already. ;-) This last change was done after a failed attempt where the previous change had to be reverted by Linus for a regression. So, just thought it to be safe to wait for a cycle for any regression report before I remove the last bits. -- Regards Sudip ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-03-06 13:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <69fb1d36-b6cf-7c46-96d1-9403de6ab47a@infradead.org> 2020-02-28 8:32 ` [PATCH 0/7] parport: Use generic kernel logging styles Joe Perches 2020-02-28 8:32 ` [PATCH 1/7] parport: Convert printk(KERN_<LEVEL> to pr_<level>( Joe Perches 2020-02-28 8:32 ` [PATCH 2/7] parport: Use more comon logging styles Joe Perches 2020-03-02 19:29 ` Helge Deller 2020-03-02 19:39 ` Joe Perches 2020-03-02 20:35 ` Helge Deller 2020-02-28 8:32 ` [PATCH 7/7] parport: Standardize use of printmode Joe Perches 2020-02-29 16:40 ` [PATCH 0/7] parport: Use generic kernel logging styles Randy Dunlap 2020-02-29 19:33 ` Joe Perches 2020-03-06 13:49 ` Sudip Mukherjee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).