[PATCH v1] pinctrl: intel: Use NSEC_PER_USEC for debounce calculus

[PATCH v1] pinctrl: intel: Use NSEC_PER_USEC for debounce calculus

[Andy Shevchenko]

Replace hard coded constants with self-explanatory names, i.e.
use NSEC_PER_USEC for debounce calculus.

While here, add a unit suffix to debounce period constant.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 3a7876efd4a6..99c5bca789eb 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -8,12 +8,13 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/gpio/driver.h>
 #include <linux/log2.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/time.h>
 
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
@@ -71,7 +72,7 @@
 #define PADCFG2_DEBOUNCE_SHIFT		1
 #define PADCFG2_DEBOUNCE_MASK		GENMASK(4, 1)
 
-#define DEBOUNCE_PERIOD			31250 /* ns */
+#define DEBOUNCE_PERIOD_NS		31250
 
 struct intel_pad_context {
 	u32 padcfg0;
@@ -566,7 +567,7 @@ static int intel_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
 			return -EINVAL;
 
 		v = (v & PADCFG2_DEBOUNCE_MASK) >> PADCFG2_DEBOUNCE_SHIFT;
-		arg = BIT(v) * DEBOUNCE_PERIOD / 1000;
+		arg = BIT(v) * DEBOUNCE_PERIOD_NS / NSEC_PER_USEC;
 
 		break;
 	}
@@ -683,7 +684,7 @@ static int intel_config_set_debounce(struct intel_pinctrl *pctrl,
 	if (debounce) {
 		unsigned long v;
 
-		v = order_base_2(debounce * 1000 / DEBOUNCE_PERIOD);
+		v = order_base_2(debounce * NSEC_PER_USEC / DEBOUNCE_PERIOD_NS);
 		if (v < 3 || v > 15) {
 			ret = -EINVAL;
 			goto exit_unlock;
-- 
2.20.1

Re: [PATCH v1] pinctrl: intel: Use NSEC_PER_USEC for debounce calculus

[Mika Westerberg]

On Tue, Jul 23, 2019 at 06:54:31PM +0300, Andy Shevchenko wrote:
> Replace hard coded constants with self-explanatory names, i.e.
> use NSEC_PER_USEC for debounce calculus.
> 
> While here, add a unit suffix to debounce period constant.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 3a7876efd4a6..99c5bca789eb 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -8,12 +8,13 @@
>   */
>  
>  #include <linux/acpi.h>
> -#include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/log2.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
> +#include <linux/time.h>
>  
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/pinctrl/pinmux.h>
> @@ -71,7 +72,7 @@
>  #define PADCFG2_DEBOUNCE_SHIFT		1
>  #define PADCFG2_DEBOUNCE_MASK		GENMASK(4, 1)
>  
> -#define DEBOUNCE_PERIOD			31250 /* ns */
> +#define DEBOUNCE_PERIOD_NS		31250

Should this be called DEBOUNCE_PERIOD_NSEC to be consistent with
NSEC_PER_USEC?

Otherwise looks good.

>  
>  struct intel_pad_context {
>  	u32 padcfg0;
> @@ -566,7 +567,7 @@ static int intel_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
>  			return -EINVAL;
>  
>  		v = (v & PADCFG2_DEBOUNCE_MASK) >> PADCFG2_DEBOUNCE_SHIFT;
> -		arg = BIT(v) * DEBOUNCE_PERIOD / 1000;
> +		arg = BIT(v) * DEBOUNCE_PERIOD_NS / NSEC_PER_USEC;
>  
>  		break;
>  	}
> @@ -683,7 +684,7 @@ static int intel_config_set_debounce(struct intel_pinctrl *pctrl,
>  	if (debounce) {
>  		unsigned long v;
>  
> -		v = order_base_2(debounce * 1000 / DEBOUNCE_PERIOD);
> +		v = order_base_2(debounce * NSEC_PER_USEC / DEBOUNCE_PERIOD_NS);
>  		if (v < 3 || v > 15) {
>  			ret = -EINVAL;
>  			goto exit_unlock;
> -- 
> 2.20.1

Re: [PATCH v1] pinctrl: intel: Use NSEC_PER_USEC for debounce calculus

[Andy Shevchenko]

On Tue, Aug 06, 2019 at 01:14:12PM +0300, Mika Westerberg wrote:
> On Tue, Jul 23, 2019 at 06:54:31PM +0300, Andy Shevchenko wrote:
> > Replace hard coded constants with self-explanatory names, i.e.
> > use NSEC_PER_USEC for debounce calculus.
> > 
> > While here, add a unit suffix to debounce period constant.

> > -#define DEBOUNCE_PERIOD			31250 /* ns */
> > +#define DEBOUNCE_PERIOD_NS		31250
> 
> Should this be called DEBOUNCE_PERIOD_NSEC to be consistent with
> NSEC_PER_USEC?

Either will good look to me.

> Otherwise looks good.

Will fix for v2.

Thanks!

-- 
With Best Regards,
Andy Shevchenko