[PATCH 0/6] hpet: misc fix

[PATCH 0/6] hpet: misc fix

[Corentin Labbe]

The original intent was to remove two build warnings, but finaly took
the opportunity to fix some style issues.
compile-tested for both x86/ia64.

Corentin Labbe (6):
  hpet: remove unused variable hpet in hpet_ioctl_common
  hpet: remove unused writeq/readq function definitons
  hpet: fix checkpatch complains about spaces
  hpet: replace printk by their pr_xxx counterparts
  hpet: removing unused variable m in hpet_interrupt
  hpet: fix style issue about braces and alignment

 drivers/char/hpet.c | 56 +++++++++++++++++------------------------------------
 1 file changed, 18 insertions(+), 38 deletions(-)

-- 
2.10.2

[PATCH 1/6] hpet: remove unused variable hpet in hpet_ioctl_common

[Corentin Labbe]

This patch fix the following warning:
drivers/char/hpet.c:582:23: attention : variable ‘hpet’ set but not used [-Wunused-but-set-variable]
by removing the unused variable hpet in hpet_ioctl_common

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/char/hpet.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index b941e6d..f0e6427 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -579,7 +579,6 @@ hpet_ioctl_common(struct hpet_dev *devp, unsigned int cmd, unsigned long arg,
 		  struct hpet_info *info)
 {
 	struct hpet_timer __iomem *timer;
-	struct hpet __iomem *hpet;
 	struct hpets *hpetp;
 	int err;
 	unsigned long v;
@@ -591,7 +590,6 @@ hpet_ioctl_common(struct hpet_dev *devp, unsigned int cmd, unsigned long arg,
 	case HPET_DPI:
 	case HPET_IRQFREQ:
 		timer = devp->hd_timer;
-		hpet = devp->hd_hpet;
 		hpetp = devp->hd_hpets;
 		break;
 	case HPET_IE_ON:
-- 
2.10.2

[PATCH 2/6] hpet: remove unused writeq/readq function definitions

[Corentin Labbe]

hpet is availlable only on x86/ia64 and thoses arch both provides
readq/writeq functions.
So this patch remove unused writeq/readq function definitions in hpet.c

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/char/hpet.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index f0e6427..a22543d 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -123,22 +123,6 @@ static struct hpets *hpets;
 #define	HPET_PERIODIC		0x0004
 #define	HPET_SHARED_IRQ		0x0008
 
-
-#ifndef readq
-static inline unsigned long long readq(void __iomem *addr)
-{
-	return readl(addr) | (((unsigned long long)readl(addr + 4)) << 32LL);
-}
-#endif
-
-#ifndef writeq
-static inline void writeq(unsigned long long v, void __iomem *addr)
-{
-	writel(v & 0xffffffff, addr);
-	writel(v >> 32, addr + 4);
-}
-#endif
-
 static irqreturn_t hpet_interrupt(int irq, void *data)
 {
 	struct hpet_dev *devp;
-- 
2.10.2

Re: [PATCH 2/6] hpet: remove unused writeq/readq function definitions

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3052 bytes --]

Hi Corentin,

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.11-rc4 next-20170324]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Corentin-Labbe/hpet-misc-fix/20170327-070101
config: i386-randconfig-x017-201713 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers//char/hpet.c: In function 'hpet_timer_set_irq':
>> drivers//char/hpet.c:207:7: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]
     v = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK) >>
          ^~~~~
   drivers//char/hpet.c: In function 'hpet_release':
>> drivers//char/hpet.c:413:2: error: implicit declaration of function 'writeq' [-Werror=implicit-function-declaration]
     writeq((readq(&timer->hpet_config) & ~Tn_INT_ENB_CNF_MASK),
     ^~~~~~
   cc1: some warnings being treated as errors

vim +/readq +207 drivers//char/hpet.c

70ef6d59 Kevin Hao 2008-05-29  191  	spin_lock_irq(&hpet_lock);
70ef6d59 Kevin Hao 2008-05-29  192  	if (devp->hd_hdwirq) {
70ef6d59 Kevin Hao 2008-05-29  193  		spin_unlock_irq(&hpet_lock);
70ef6d59 Kevin Hao 2008-05-29  194  		return;
70ef6d59 Kevin Hao 2008-05-29  195  	}
70ef6d59 Kevin Hao 2008-05-29  196  
70ef6d59 Kevin Hao 2008-05-29  197  	timer = devp->hd_timer;
70ef6d59 Kevin Hao 2008-05-29  198  
70ef6d59 Kevin Hao 2008-05-29  199  	/* we prefer level triggered mode */
70ef6d59 Kevin Hao 2008-05-29  200  	v = readl(&timer->hpet_config);
70ef6d59 Kevin Hao 2008-05-29  201  	if (!(v & Tn_INT_TYPE_CNF_MASK)) {
70ef6d59 Kevin Hao 2008-05-29  202  		v |= Tn_INT_TYPE_CNF_MASK;
70ef6d59 Kevin Hao 2008-05-29  203  		writel(v, &timer->hpet_config);
70ef6d59 Kevin Hao 2008-05-29  204  	}
70ef6d59 Kevin Hao 2008-05-29  205  	spin_unlock_irq(&hpet_lock);
70ef6d59 Kevin Hao 2008-05-29  206  
70ef6d59 Kevin Hao 2008-05-29 @207  	v = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK) >>
70ef6d59 Kevin Hao 2008-05-29  208  				 Tn_INT_ROUTE_CAP_SHIFT;
70ef6d59 Kevin Hao 2008-05-29  209  
70ef6d59 Kevin Hao 2008-05-29  210  	/*
70ef6d59 Kevin Hao 2008-05-29  211  	 * In PIC mode, skip IRQ0-4, IRQ6-9, IRQ12-15 which is always used by
70ef6d59 Kevin Hao 2008-05-29  212  	 * legacy device. In IO APIC mode, we skip all the legacy IRQS.
70ef6d59 Kevin Hao 2008-05-29  213  	 */
70ef6d59 Kevin Hao 2008-05-29  214  	if (acpi_irq_model == ACPI_IRQ_MODEL_PIC)
70ef6d59 Kevin Hao 2008-05-29  215  		v &= ~0xf3df;

:::::: The code at line 207 was first introduced by commit
:::::: 70ef6d595b6e51618a0cbe44b848d8c9db11a010 x86: get irq for hpet timer

:::::: TO: Kevin Hao <kexin.hao@windriver.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23239 bytes --]

Re: [PATCH 2/6] hpet: remove unused writeq/readq function definitions

[Corentin Labbe]

On Mon, Mar 27, 2017 at 07:49:34AM +0800, kbuild test robot wrote:
> Hi Corentin,
> 
> [auto build test ERROR on char-misc/char-misc-testing]
> [also build test ERROR on v4.11-rc4 next-20170324]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Corentin-Labbe/hpet-misc-fix/20170327-070101
> config: i386-randconfig-x017-201713 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers//char/hpet.c: In function 'hpet_timer_set_irq':
> >> drivers//char/hpet.c:207:7: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]
>      v = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK) >>
>           ^~~~~
>    drivers//char/hpet.c: In function 'hpet_release':
> >> drivers//char/hpet.c:413:2: error: implicit declaration of function 'writeq' [-Werror=implicit-function-declaration]
>      writeq((readq(&timer->hpet_config) & ~Tn_INT_ENB_CNF_MASK),
>      ^~~~~~
>    cc1: some warnings being treated as errors
> 
> vim +/readq +207 drivers//char/hpet.c
> 
> 70ef6d59 Kevin Hao 2008-05-29  191  	spin_lock_irq(&hpet_lock);
> 70ef6d59 Kevin Hao 2008-05-29  192  	if (devp->hd_hdwirq) {
> 70ef6d59 Kevin Hao 2008-05-29  193  		spin_unlock_irq(&hpet_lock);
> 70ef6d59 Kevin Hao 2008-05-29  194  		return;
> 70ef6d59 Kevin Hao 2008-05-29  195  	}
> 70ef6d59 Kevin Hao 2008-05-29  196  
> 70ef6d59 Kevin Hao 2008-05-29  197  	timer = devp->hd_timer;
> 70ef6d59 Kevin Hao 2008-05-29  198  
> 70ef6d59 Kevin Hao 2008-05-29  199  	/* we prefer level triggered mode */
> 70ef6d59 Kevin Hao 2008-05-29  200  	v = readl(&timer->hpet_config);
> 70ef6d59 Kevin Hao 2008-05-29  201  	if (!(v & Tn_INT_TYPE_CNF_MASK)) {
> 70ef6d59 Kevin Hao 2008-05-29  202  		v |= Tn_INT_TYPE_CNF_MASK;
> 70ef6d59 Kevin Hao 2008-05-29  203  		writel(v, &timer->hpet_config);
> 70ef6d59 Kevin Hao 2008-05-29  204  	}
> 70ef6d59 Kevin Hao 2008-05-29  205  	spin_unlock_irq(&hpet_lock);
> 70ef6d59 Kevin Hao 2008-05-29  206  
> 70ef6d59 Kevin Hao 2008-05-29 @207  	v = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK) >>
> 70ef6d59 Kevin Hao 2008-05-29  208  				 Tn_INT_ROUTE_CAP_SHIFT;
> 70ef6d59 Kevin Hao 2008-05-29  209  
> 70ef6d59 Kevin Hao 2008-05-29  210  	/*
> 70ef6d59 Kevin Hao 2008-05-29  211  	 * In PIC mode, skip IRQ0-4, IRQ6-9, IRQ12-15 which is always used by
> 70ef6d59 Kevin Hao 2008-05-29  212  	 * legacy device. In IO APIC mode, we skip all the legacy IRQS.
> 70ef6d59 Kevin Hao 2008-05-29  213  	 */
> 70ef6d59 Kevin Hao 2008-05-29  214  	if (acpi_irq_model == ACPI_IRQ_MODEL_PIC)
> 70ef6d59 Kevin Hao 2008-05-29  215  		v &= ~0xf3df;
> 
> :::::: The code at line 207 was first introduced by commit
> :::::: 70ef6d595b6e51618a0cbe44b848d8c9db11a010 x86: get irq for hpet timer
> 
> :::::: TO: Kevin Hao <kexin.hao@windriver.com>
> :::::: CC: Ingo Molnar <mingo@elte.hu>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Wrongly believed that x86 and x86_64 shared writeq/readq.
Sorry, I will drop this patch

Since the writeq/readq redefined is present in lots of other file, perhaps adding it to i386 could be done.

Regards

Re: [PATCH 2/6] hpet: remove unused writeq/readq function definitions

[Clemens Ladisch]

Corentin Labbe wrote:
> On Mon, Mar 27, 2017 at 07:49:34AM +0800, kbuild test robot wrote:
>>    drivers//char/hpet.c: In function 'hpet_timer_set_irq':
>>>> drivers//char/hpet.c:207:7: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]
>
> Wrongly believed that x86 and x86_64 shared writeq/readq.
> Sorry, I will drop this patch
>
> Since the writeq/readq redefined is present in lots of other file, perhaps adding it to i386 could be done.

Just use <linux/io-64-nonatomic-lo-hi.h> instead.


Regards,
Clemens

Re: [PATCH 2/6] hpet: remove unused writeq/readq function definitions

[Corentin Labbe]

On Mon, Mar 27, 2017 at 09:51:23AM +0200, Clemens Ladisch wrote:
> Corentin Labbe wrote:
> > On Mon, Mar 27, 2017 at 07:49:34AM +0800, kbuild test robot wrote:
> >>    drivers//char/hpet.c: In function 'hpet_timer_set_irq':
> >>>> drivers//char/hpet.c:207:7: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]
> >
> > Wrongly believed that x86 and x86_64 shared writeq/readq.
> > Sorry, I will drop this patch
> >
> > Since the writeq/readq redefined is present in lots of other file, perhaps adding it to i386 could be done.
> 
> Just use <linux/io-64-nonatomic-lo-hi.h> instead.
> 

Thanks, much easier than add writeq/readq to whole x86

Regards

[PATCH 3/6] hpet: fix checkpatch complains about spaces

[Corentin Labbe]

This patch make checkpatch happy for spaces.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/char/hpet.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index a22543d..602b810 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -50,7 +50,6 @@
 
 #define HPET_RANGE_SIZE		1024	/* from HPET spec */
 
-
 /* WARNING -- don't get confused.  These macros are never used
  * to write the (single) counter, and rarely to read it.
  * They're badly named; to fix, someday.
@@ -82,6 +81,7 @@ static struct clocksource clocksource_hpet = {
 	.mask		= CLOCKSOURCE_MASK(64),
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
+
 static struct clocksource *hpet_clocksource;
 #endif
 
@@ -280,7 +280,7 @@ static int hpet_open(struct inode *inode, struct file *file)
 }
 
 static ssize_t
-hpet_read(struct file *file, char __user *buf, size_t count, loff_t * ppos)
+hpet_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	DECLARE_WAITQUEUE(wait, current);
 	unsigned long data;
@@ -326,7 +326,7 @@ hpet_read(struct file *file, char __user *buf, size_t count, loff_t * ppos)
 	return retval;
 }
 
-static unsigned int hpet_poll(struct file *file, poll_table * wait)
+static unsigned int hpet_poll(struct file *file, poll_table *wait)
 {
 	unsigned long v;
 	struct hpet_dev *devp;
@@ -686,6 +686,7 @@ hpet_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	if ((cmd == HPET_INFO) && !err) {
 		struct compat_hpet_info __user *u = compat_ptr(arg);
+
 		if (put_user(info.hi_ireqfreq, &u->hi_ireqfreq) ||
 		    put_user(info.hi_flags, &u->hi_flags) ||
 		    put_user(info.hi_hpet, &u->hi_hpet) ||
@@ -901,7 +902,7 @@ int hpet_alloc(struct hpet_data *hdp)
 		"hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
 		hpetp->hp_which, hpetp->hp_ntimer,
 		cap & HPET_COUNTER_SIZE_MASK ? 64 : 32,
-		(unsigned) temp, remainder);
+		(unsigned)temp, remainder);
 
 	mcfg = readq(&hpet->hpet_config);
 	if ((mcfg & HPET_ENABLE_CNF_MASK) == 0) {
-- 
2.10.2

[PATCH 4/6] hpet: replace printk by their pr_xxx counterparts

[Corentin Labbe]

This patch replace all printk by their pr_xxx counterparts.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/char/hpet.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 602b810..e11d6b5 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -490,7 +490,7 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
 		irq_flags = devp->hd_flags & HPET_SHARED_IRQ ? IRQF_SHARED : 0;
 		if (request_irq(irq, hpet_interrupt, irq_flags,
 				devp->hd_name, (void *)devp)) {
-			printk(KERN_ERR "hpet: IRQ %d is not free\n", irq);
+			pr_err("hpet: IRQ %d is not free\n", irq);
 			irq = 0;
 		}
 	}
@@ -840,8 +840,7 @@ int hpet_alloc(struct hpet_data *hdp)
 	 * ACPI has also reported, then we catch it here.
 	 */
 	if (hpet_is_known(hdp)) {
-		printk(KERN_DEBUG "%s: duplicate HPET ignored\n",
-			__func__);
+		pr_debug("%s: duplicate HPET ignored\n", __func__);
 		return 0;
 	}
 
@@ -869,8 +868,7 @@ int hpet_alloc(struct hpet_data *hdp)
 	ntimer = ((cap & HPET_NUM_TIM_CAP_MASK) >> HPET_NUM_TIM_CAP_SHIFT) + 1;
 
 	if (hpetp->hp_ntimer != ntimer) {
-		printk(KERN_WARNING "hpet: number irqs doesn't agree"
-		       " with number of timers\n");
+		pr_warn("hpet: number irqs doesn't agree with number of timers\n");
 		kfree(hpetp);
 		return -ENODEV;
 	}
@@ -889,7 +887,7 @@ int hpet_alloc(struct hpet_data *hdp)
 	do_div(temp, period);
 	hpetp->hp_tick_freq = temp; /* ticks per second */
 
-	printk(KERN_INFO "hpet%d: at MMIO 0x%lx, IRQ%s",
+	pr_info("hpet%d: at MMIO 0x%lx, IRQ%s",
 		hpetp->hp_which, hdp->hd_phys_address,
 		hpetp->hp_ntimer > 1 ? "s" : "");
 	for (i = 0; i < hpetp->hp_ntimer; i++)
@@ -898,8 +896,7 @@ int hpet_alloc(struct hpet_data *hdp)
 
 	temp = hpetp->hp_tick_freq;
 	remainder = do_div(temp, 1000000);
-	printk(KERN_INFO
-		"hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
+	pr_info("hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
 		hpetp->hp_which, hpetp->hp_ntimer,
 		cap & HPET_COUNTER_SIZE_MASK ? 64 : 32,
 		(unsigned)temp, remainder);
@@ -1019,7 +1016,7 @@ static int hpet_acpi_add(struct acpi_device *device)
 	if (!data.hd_address || !data.hd_nirqs) {
 		if (data.hd_address)
 			iounmap(data.hd_address);
-		printk("%s: no address or irqs in _CRS\n", __func__);
+		pr_err("%s: no address or irqs in _CRS\n", __func__);
 		return -ENODEV;
 	}
 
-- 
2.10.2

[PATCH 5/6] hpet: removing unused variable m in hpet_interrupt

[Corentin Labbe]

This patch fix the following warning:
drivers/char/hpet.c:146:17: attention : variable ‘m’ set but not used [-Wunused-but-set-variable]
by removing the unused variable m in hpet_interrupt

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/char/hpet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e11d6b5..7493e4d 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -143,12 +143,11 @@ static irqreturn_t hpet_interrupt(int irq, void *data)
 	 * This has the effect of treating non-periodic like periodic.
 	 */
 	if ((devp->hd_flags & (HPET_IE | HPET_PERIODIC)) == HPET_IE) {
-		unsigned long m, t, mc, base, k;
+		unsigned long t, mc, base, k;
 		struct hpet __iomem *hpet = devp->hd_hpet;
 		struct hpets *hpetp = devp->hd_hpets;
 
 		t = devp->hd_ireqfreq;
-		m = read_counter(&devp->hd_timer->hpet_compare);
 		mc = read_counter(&hpet->hpet_mc);
 		/* The time for the next interrupt would logically be t + m,
 		 * however, if we are very unlucky and the interrupt is delayed
-- 
2.10.2

[PATCH 6/6] hpet: fix style issue about braces and alignment

[Corentin Labbe]

This patch fix all style issue for braces and alignment

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/char/hpet.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 7493e4d..6100e68 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -254,9 +254,9 @@ static int hpet_open(struct inode *inode, struct file *file)
 
 	for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
 		for (i = 0; i < hpetp->hp_ntimer; i++)
-			if (hpetp->hp_dev[i].hd_flags & HPET_OPEN)
+			if (hpetp->hp_dev[i].hd_flags & HPET_OPEN) {
 				continue;
-			else {
+			} else {
 				devp = &hpetp->hp_dev[i];
 				break;
 			}
@@ -303,9 +303,9 @@ hpet_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		devp->hd_irqdata = 0;
 		spin_unlock_irq(&hpet_lock);
 
-		if (data)
+		if (data) {
 			break;
-		else if (file->f_flags & O_NONBLOCK) {
+		} else if (file->f_flags & O_NONBLOCK) {
 			retval = -EAGAIN;
 			goto out;
 		} else if (signal_pending(current)) {
@@ -986,7 +986,8 @@ static acpi_status hpet_resources(struct acpi_resource *res, void *data)
 				break;
 
 			irq = acpi_register_gsi(NULL, irqp->interrupts[i],
-				      irqp->triggering, irqp->polarity);
+						irqp->triggering,
+						irqp->polarity);
 			if (irq < 0)
 				return AE_ERROR;
 
-- 
2.10.2