On 12/13/18 11:47 AM, Hannes Reinecke wrote:
> Use dyndebug to allow the user to enable debugging during runtime.
This patch also converts ordinary printks() to dev_*() helpers and
removes some redundant debug messages. Please remember to also mention
all this in the patch description.
> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> ---
> drivers/ata/pata_pdc2027x.c | 72 ++++++++++++++++++---------------------------
> 1 file changed, 29 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
> index 1a18e675ba9f..8ba41c14e1ee 100644
> --- a/drivers/ata/pata_pdc2027x.c
> +++ b/drivers/ata/pata_pdc2027x.c
> @@ -36,13 +36,6 @@
>
> #define DRV_NAME "pata_pdc2027x"
> #define DRV_VERSION "1.0"
> -#undef PDC_DEBUG
> -
> -#ifdef PDC_DEBUG
> -#define PDPRINTK(fmt, args...) printk(KERN_ERR "%s: " fmt, __func__, ## args)
> -#else
> -#define PDPRINTK(fmt, args...)
> -#endif
>
> enum {
> PDC_MMIO_BAR = 5,
> @@ -220,11 +213,13 @@ static int pdc2027x_cable_detect(struct ata_port *ap)
> if (cgcr & (1 << 26))
> goto cbl40;
>
> - PDPRINTK("No cable or 80-conductor cable on port %d\n", ap->port_no);
> + ata_port_dbg(ap, "No cable or 80-conductor cable on port %d\n",
> + ap->port_no);
Please preserve __func__ printing in the conversion.
Also please use dev_*() helper instead.
>
> return ATA_CBL_PATA80;
> cbl40:
> - printk(KERN_INFO DRV_NAME ": 40-conductor cable detected on port %d\n", ap->port_no);
> + ata_port_info(ap, "40-conductor cable detected on port %d\n",
> + ap->port_no);
please use dev_*() helper instead.
> return ATA_CBL_PATA40;
> }
>
> @@ -298,17 +293,17 @@ static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev)
> unsigned int pio = adev->pio_mode - XFER_PIO_0;
> u32 ctcr0, ctcr1;
>
> - PDPRINTK("adev->pio_mode[%X]\n", adev->pio_mode);
> + ata_port_dbg(ap, "adev->pio_mode[%X]\n", adev->pio_mode);
Please preserve __func__ printing in the conversion.
Also please use dev_*() helper instead.
>
> /* Sanity check */
> if (pio > 4) {
> - printk(KERN_ERR DRV_NAME ": Unknown pio mode [%d] ignored\n", pio);
> + ata_port_err(ap, "Unknown pio mode [%d] ignored\n", pio);
Please use dev_*() helper instead.
> return;
>
> }
>
> /* Set the PIO timing registers using value table for 133MHz */
> - PDPRINTK("Set pio regs... \n");
> + ata_port_dbg(ap, "Set pio regs... \n");
Please preserve __func__ printing in the conversion.
Also please use dev_*() helper instead.
>
> ctcr0 = ioread32(dev_mmio(ap, adev, PDC_CTCR0));
> ctcr0 &= 0xffff0000;
> @@ -321,9 +316,7 @@ static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev)
> ctcr1 |= (pdc2027x_pio_timing_tbl[pio].value2 << 24);
> iowrite32(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
>
> - PDPRINTK("Set pio regs done\n");
> -
> - PDPRINTK("Set to pio mode[%u] \n", pio);
> + ata_port_dbg(ap, "Set to pio mode[%u] \n", pio);
ditto
> }
>
> /**
> @@ -356,7 +349,7 @@ static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
> iowrite32(ctcr1 & ~(1 << 7), dev_mmio(ap, adev, PDC_CTCR1));
> }
>
> - PDPRINTK("Set udma regs... \n");
> + ata_port_dbg(ap, "Set udma regs... \n");
ditto
>
> ctcr1 = ioread32(dev_mmio(ap, adev, PDC_CTCR1));
> ctcr1 &= 0xff000000;
> @@ -365,16 +358,14 @@ static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
> (pdc2027x_udma_timing_tbl[udma_mode].value2 << 16);
> iowrite32(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
>
> - PDPRINTK("Set udma regs done\n");
> -
> - PDPRINTK("Set to udma mode[%u] \n", udma_mode);
> + ata_port_dbg(ap, "Set to udma mode[%u] \n", udma_mode);
ditto
>
> } else if ((dma_mode >= XFER_MW_DMA_0) &&
> (dma_mode <= XFER_MW_DMA_2)) {
> /* Set the MDMA timing registers with value table for 133MHz */
> unsigned int mdma_mode = dma_mode & 0x07;
>
> - PDPRINTK("Set mdma regs... \n");
> + ata_port_dbg(ap, "Set mdma regs... \n");
ditto
> ctcr0 = ioread32(dev_mmio(ap, adev, PDC_CTCR0));
>
> ctcr0 &= 0x0000ffff;
> @@ -382,11 +373,10 @@ static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
> (pdc2027x_mdma_timing_tbl[mdma_mode].value1 << 24);
>
> iowrite32(ctcr0, dev_mmio(ap, adev, PDC_CTCR0));
> - PDPRINTK("Set mdma regs done\n");
>
> - PDPRINTK("Set to mdma mode[%u] \n", mdma_mode);
> + ata_port_dbg(ap, "Set to mdma mode[%u] \n", mdma_mode);
ditto
> } else {
> - printk(KERN_ERR DRV_NAME ": Unknown dma mode [%u] ignored\n", dma_mode);
> + ata_port_err(ap, "Unknown dma mode [%u] ignored\n", dma_mode);
> }
> }
>
> @@ -420,7 +410,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
> ctcr1 |= (1 << 25);
> iowrite32(ctcr1, dev_mmio(ap, dev, PDC_CTCR1));
>
> - PDPRINTK("Turn on prefetch\n");
> + ata_dev_dbg(dev, "Turn on prefetch\n");
ditto
> } else {
> pdc2027x_set_dmamode(ap, dev);
> }
> @@ -491,8 +481,8 @@ static long pdc_read_counter(struct ata_host *host)
>
> counter = (bccrh << 15) | bccrl;
>
> - PDPRINTK("bccrh [%X] bccrl [%X]\n", bccrh, bccrl);
> - PDPRINTK("bccrhv[%X] bccrlv[%X]\n", bccrhv, bccrlv);
> + dev_dbg(host->dev, "bccrh [%X] bccrl [%X]\n", bccrh, bccrl);
> + dev_dbg(host->dev, "bccrhv[%X] bccrlv[%X]\n", bccrhv, bccrlv);
ditto
> /*
> * The 30-bit decreasing counter are read by 2 pieces.
> @@ -501,7 +491,7 @@ static long pdc_read_counter(struct ata_host *host)
> */
> if (retry && !(bccrh == bccrhv && bccrl >= bccrlv)) {
> retry--;
> - PDPRINTK("rereading counter\n");
> + dev_dbg(host->dev, "rereading counter\n");
ditto
> goto retry;
> }
>
> @@ -526,20 +516,18 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
>
> /* Sanity check */
> if (unlikely(pll_clock_khz < 5000L || pll_clock_khz > 70000L)) {
> - printk(KERN_ERR DRV_NAME ": Invalid PLL input clock %ldkHz, give up!\n", pll_clock_khz);
> + dev_err(host->dev, "Invalid PLL input clock %ldkHz, give up!\n", pll_clock_khz);
> return;
> }
>
> -#ifdef PDC_DEBUG
> - PDPRINTK("pout_required is %ld\n", pout_required);
> + dev_dbg(host->dev, "pout_required is %ld\n", pout_required);
Please preserve __func__ printing in the conversion.
> /* Show the current clock value of PLL control register
> * (maybe already configured by the firmware)
> */
> pll_ctl = ioread16(mmio_base + PDC_PLL_CTL);
>
> - PDPRINTK("pll_ctl[%X]\n", pll_ctl);
> -#endif
> + dev_dbg(host->dev, "pll_ctl[%X]\n", pll_ctl);
ditto
> /*
> * Calculate the ratio of F, R and OD
> @@ -558,7 +546,7 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
> R = 0x00;
> } else {
> /* Invalid ratio */
> - printk(KERN_ERR DRV_NAME ": Invalid ratio %ld, give up!\n", ratio);
> + dev_err(host->dev, "Invalid ratio %ld, give up!\n", ratio);
> return;
> }
>
> @@ -566,15 +554,15 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
>
> if (unlikely(F < 0 || F > 127)) {
> /* Invalid F */
> - printk(KERN_ERR DRV_NAME ": F[%d] invalid!\n", F);
> + dev_err(host->dev, "F[%d] invalid!\n", F);
> return;
> }
>
> - PDPRINTK("F[%d] R[%d] ratio*1000[%ld]\n", F, R, ratio);
> + dev_dbg(host->dev, "F[%d] R[%d] ratio*1000[%ld]\n", F, R, ratio);
ditto
> pll_ctl = (R << 8) | F;
>
> - PDPRINTK("Writing pll_ctl[%X]\n", pll_ctl);
> + dev_dbg(host->dev, "Writing pll_ctl[%X]\n", pll_ctl);
ditto
> iowrite16(pll_ctl, mmio_base + PDC_PLL_CTL);
> ioread16(mmio_base + PDC_PLL_CTL); /* flush */
> @@ -582,15 +570,13 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
> /* Wait the PLL circuit to be stable */
> msleep(30);
>
> -#ifdef PDC_DEBUG
> /*
> * Show the current clock value of PLL control register
> * (maybe configured by the firmware)
> */
> pll_ctl = ioread16(mmio_base + PDC_PLL_CTL);
>
> - PDPRINTK("pll_ctl[%X]\n", pll_ctl);
> -#endif
> + dev_dbg(host->dev, "pll_ctl[%X]\n", pll_ctl);
ditto
> return;
> }
> @@ -611,7 +597,7 @@ static long pdc_detect_pll_input_clock(struct ata_host *host)
>
> /* Start the test mode */
> scr = ioread32(mmio_base + PDC_SYS_CTL);
> - PDPRINTK("scr[%X]\n", scr);
> + dev_dbg(host->dev, "scr[%X]\n", scr);
ditto
> iowrite32(scr | (0x01 << 14), mmio_base + PDC_SYS_CTL);
> ioread32(mmio_base + PDC_SYS_CTL); /* flush */
>
> @@ -628,7 +614,7 @@ static long pdc_detect_pll_input_clock(struct ata_host *host)
>
> /* Stop the test mode */
> scr = ioread32(mmio_base + PDC_SYS_CTL);
> - PDPRINTK("scr[%X]\n", scr);
> + dev_dbg(host->dev, "scr[%X]\n", scr);
ditto
> iowrite32(scr & ~(0x01 << 14), mmio_base + PDC_SYS_CTL);
> ioread32(mmio_base + PDC_SYS_CTL); /* flush */
>
> @@ -638,8 +624,8 @@ static long pdc_detect_pll_input_clock(struct ata_host *host)
> pll_clock = ((start_count - end_count) & 0x3fffffff) / 100 *
> (100000000 / usec_elapsed);
>
> - PDPRINTK("start[%ld] end[%ld] \n", start_count, end_count);
> - PDPRINTK("PLL input clock[%ld]Hz\n", pll_clock);
> + dev_dbg(host->dev, "start[%ld] end[%ld] \n", start_count, end_count);
> + dev_dbg(host->dev, "PLL input clock[%ld]Hz\n", pll_clock);
ditto
> return pll_clock;
> }
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
