[PATCH v2 1/3] units: Add SI metric prefix definitions

[PATCH v2 1/3] units: Add SI metric prefix definitions

[Andy Shevchenko]

Sometimes it's useful to have well-defined SI metric prefix to be used
to self-describe the formulas or equations.

List most popular ones in the units.h.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no changes
 include/linux/units.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/units.h b/include/linux/units.h
index dcc30a53fa93..7366fcd45ec2 100644
--- a/include/linux/units.h
+++ b/include/linux/units.h
@@ -4,6 +4,22 @@
 
 #include <linux/math.h>
 
+/* Metric prefixes in accordance with Système international (d'unités) */
+#define PETA	1000000000000000LL
+#define TERA	1000000000000LL
+#define GIGA	1000000000L
+#define MEGA	1000000L
+#define KILO	1000L
+#define HECTO	100L
+#define DECA	10L
+#define DECI	10L
+#define CENTI	100L
+#define MILLI	1000L
+#define MICRO	1000000L
+#define NANO	1000000000L
+#define PICO	1000000000000LL
+#define FEMTO	1000000000000000LL
+
 #define MILLIWATT_PER_WATT	1000L
 #define MICROWATT_PER_MILLIWATT	1000L
 #define MICROWATT_PER_WATT	1000000L
-- 
2.30.2

[PATCH v2 2/3] i2c: designware: Use DIV_ROUND_CLOSEST() macro

[Andy Shevchenko]

Instead of open-coding DIV_ROUND_CLOSEST() and similar use the macros directly.
While at it, replace numbers with predefined SI metric prefixes.

No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: to get 10^0 based numbers divide by 10^-6 which is MICRO (Jarkko)
 drivers/i2c/busses/i2c-designware-common.c  | 8 ++++----
 drivers/i2c/busses/i2c-designware-platdrv.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index fdc34d9e3702..bf2a4920638a 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -24,6 +24,7 @@
 #include <linux/regmap.h>
 #include <linux/swab.h>
 #include <linux/types.h>
+#include <linux/units.h>
 
 #include "i2c-designware-core.h"
 
@@ -350,7 +351,7 @@ u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
 		 *
 		 * If your hardware is free from tHD;STA issue, try this one.
 		 */
-		return (ic_clk * tSYMBOL + 500000) / 1000000 - 8 + offset;
+		return DIV_ROUND_CLOSEST(ic_clk * tSYMBOL, MICRO) - 8 + offset;
 	else
 		/*
 		 * Conditional expression:
@@ -366,8 +367,7 @@ u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
 		 * The reason why we need to take into account "tf" here,
 		 * is the same as described in i2c_dw_scl_lcnt().
 		 */
-		return (ic_clk * (tSYMBOL + tf) + 500000) / 1000000
-			- 3 + offset;
+		return DIV_ROUND_CLOSEST(ic_clk * (tSYMBOL + tf), MICRO) - 3 + offset;
 }
 
 u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
@@ -383,7 +383,7 @@ u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
 	 * account the fall time of SCL signal (tf).  Default tf value
 	 * should be 0.3 us, for safety.
 	 */
-	return ((ic_clk * (tLOW + tf) + 500000) / 1000000) - 1 + offset;
+	return DIV_ROUND_CLOSEST(ic_clk * (tLOW + tf), MICRO) - 1 + offset;
 }
 
 int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 4b37f28ec0c6..21113665ddea 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -31,12 +31,13 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <linux/units.h>
 
 #include "i2c-designware-core.h"
 
 static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 {
-	return clk_get_rate(dev->clk)/1000;
+	return clk_get_rate(dev->clk) / KILO;
 }
 
 #ifdef CONFIG_ACPI
@@ -270,7 +271,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 
 		if (!dev->sda_hold_time && t->sda_hold_ns)
 			dev->sda_hold_time =
-				div_u64(clk_khz * t->sda_hold_ns + 500000, 1000000);
+				DIV_S64_ROUND_CLOSEST(clk_khz * t->sda_hold_ns, MICRO);
 	}
 
 	adap = &dev->adapter;
-- 
2.30.2

[PATCH v2 3/3] i2c: designware: Fix indentation in the header

[Andy Shevchenko]

In couple of places the indentation makes harder to read the code.
Fix it to be sane.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no changes
 drivers/i2c/busses/i2c-designware-core.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 6a53f75abf7c..60a2e750cee9 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -117,7 +117,7 @@
 
 #define DW_IC_ERR_TX_ABRT	0x1
 
-#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
+#define DW_IC_TAR_10BITADDR_MASTER	BIT(12)
 
 #define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH	(BIT(2) | BIT(3))
 #define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK	GENMASK(3, 2)
@@ -245,7 +245,7 @@ struct dw_i2c_dev {
 	struct clk		*clk;
 	struct clk		*pclk;
 	struct reset_control	*rst;
-	struct i2c_client		*slave;
+	struct i2c_client	*slave;
 	u32			(*get_clk_rate_khz) (struct dw_i2c_dev *dev);
 	int			cmd_err;
 	struct i2c_msg		*msgs;
-- 
2.30.2

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Joe Perches]

On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
> Sometimes it's useful to have well-defined SI metric prefix to be used
> to self-describe the formulas or equations.
> 
> List most popular ones in the units.h.
[]
> diff --git a/include/linux/units.h b/include/linux/units.h
[]
> @@ -4,6 +4,22 @@
>  
>  #include <linux/math.h>
>  
> +/* Metric prefixes in accordance with Système international (d'unités) */
> +#define PETA	1000000000000000LL
> +#define TERA	1000000000000LL
> +#define GIGA	1000000000L
> +#define MEGA	1000000L
> +#define KILO	1000L
> +#define HECTO	100L
> +#define DECA	10L
> +#define DECI	10L
> +#define CENTI	100L
> +#define MILLI	1000L
> +#define MICRO	1000000L
> +#define NANO	1000000000L
> +#define PICO	1000000000000LL
> +#define FEMTO	1000000000000000LL
> +
>  #define MILLIWATT_PER_WATT	1000L
>  #define MICROWATT_PER_MILLIWATT	1000L
>  #define MICROWATT_PER_WATT	1000000L

Somewhat surprisingly to me, this seems safe.

(though I suggest using UL and ULL rather than L and LL)

The only use of any of these seems to be:

sound/pcmcia/vx/vxp_ops.c:      [VX_MICRO]      = 0x0c,         // MICRO
sound/pcmcia/vx/vxp_ops.c:              vx_outb(chip, MICRO, level);
sound/pcmcia/vx/vxp_ops.c:                      vx_outb(chip, MICRO, vx_compute_mic_level(chip->mic_level));

and these vx_outb uses are themselves macros that prepend VX_ to the 2nd arg.

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Andy Shevchenko]

On Mon, Jun 07, 2021 at 08:43:02AM -0700, Joe Perches wrote:
> On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
> > Sometimes it's useful to have well-defined SI metric prefix to be used
> > to self-describe the formulas or equations.

...

> > +/* Metric prefixes in accordance with Système international (d'unités) */
> > +#define PETA	1000000000000000LL
> > +#define TERA	1000000000000LL
> > +#define GIGA	1000000000L
> > +#define MEGA	1000000L
> > +#define KILO	1000L
> > +#define HECTO	100L
> > +#define DECA	10L
> > +#define DECI	10L
> > +#define CENTI	100L
> > +#define MILLI	1000L
> > +#define MICRO	1000000L
> > +#define NANO	1000000000L
> > +#define PICO	1000000000000LL
> > +#define FEMTO	1000000000000000LL

> Somewhat surprisingly to me, this seems safe.
> 
> (though I suggest using UL and ULL rather than L and LL)

Okay.

> The only use of any of these seems to be:
> 
> sound/pcmcia/vx/vxp_ops.c:      [VX_MICRO]      = 0x0c,         // MICRO
> sound/pcmcia/vx/vxp_ops.c:              vx_outb(chip, MICRO, level);
> sound/pcmcia/vx/vxp_ops.c:                      vx_outb(chip, MICRO, vx_compute_mic_level(chip->mic_level));
> 
> and these vx_outb uses are themselves macros that prepend VX_ to the 2nd arg.

Is it a real issue there?

(Yes, I saw it, but I didn't check compilation in the assumption that units.h
 is not anyhow included in that file).

Okay, I have compiled it, no problems registered.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Joe Perches]

On Mon, 2021-06-07 at 18:57 +0300, Andy Shevchenko wrote:
> On Mon, Jun 07, 2021 at 08:43:02AM -0700, Joe Perches wrote:
> > On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
> > > Sometimes it's useful to have well-defined SI metric prefix to be used
> > > to self-describe the formulas or equations.
-=
> > The only use of any of these seems to be:
> > 
> > sound/pcmcia/vx/vxp_ops.c:      [VX_MICRO]      = 0x0c,         // MICRO
> > sound/pcmcia/vx/vxp_ops.c:              vx_outb(chip, MICRO, level);
> > sound/pcmcia/vx/vxp_ops.c:                      vx_outb(chip, MICRO, vx_compute_mic_level(chip->mic_level));
> > 
> > and these vx_outb uses are themselves macros that prepend VX_ to the 2nd arg.
> 
> Is it a real issue there?

No, it's all good.

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Andy Shevchenko]

On Mon, Jun 07, 2021 at 09:00:41AM -0700, Joe Perches wrote:
> On Mon, 2021-06-07 at 18:57 +0300, Andy Shevchenko wrote:
> > On Mon, Jun 07, 2021 at 08:43:02AM -0700, Joe Perches wrote:
> > > On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
> > > > Sometimes it's useful to have well-defined SI metric prefix to be used
> > > > to self-describe the formulas or equations.
> -=
> > > The only use of any of these seems to be:
> > > 
> > > sound/pcmcia/vx/vxp_ops.c:      [VX_MICRO]      = 0x0c,         // MICRO
> > > sound/pcmcia/vx/vxp_ops.c:              vx_outb(chip, MICRO, level);
> > > sound/pcmcia/vx/vxp_ops.c:                      vx_outb(chip, MICRO, vx_compute_mic_level(chip->mic_level));
> > > 
> > > and these vx_outb uses are themselves macros that prepend VX_ to the 2nd arg.
> > 
> > Is it a real issue there?
> 
> No, it's all good.

Thanks for confirming!

So, I will send v3 (I guess tomorrow) with L->UL and LL accordingly.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Joe Perches]

On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
> Sometimes it's useful to have well-defined SI metric prefix to be used
> to self-describe the formulas or equations.
> 
> List most popular ones in the units.h.

trivia:

> diff --git a/include/linux/units.h b/include/linux/units.h
[]
> @@ -4,6 +4,22 @@
>  
> 
>  #include <linux/math.h>
>  
> +/* Metric prefixes in accordance with Système international (d'unités) */
> +#define PETA	1000000000000000LL
> +#define TERA	1000000000000LL
> +#define GIGA	1000000000L
> +#define MEGA	1000000L
> +#define KILO	1000L

It's perhaps more legible to avoid the long sequences of zeros by
using preprocessor mechanisms.

#define KILO (1000UL)
#define MEGA (KILO * 1000)
#define GIGA (MEGA * 1000)
#define TERA (GIGA * 1000ULL)
#define PETA (TERA * 1000)

or maybe:

#define KILO (1000UL)
#define MEGA (1000UL * 1000UL)
#define GIGA (1000UL * 1000UL * 1000UL)
#define TERA (1000ULL * 1000ULL * 1000ULL * 1000ULL)
#define PETA (1000ULL * 1000ULL * 1000ULL * 1000ULL * 1000ULL)

etc...

> +#define HECTO 100L
> +#define DECA 10L

Should it be DECA or DEKA or both?

https://www.nist.gov/pml/weights-and-measures/metric-si-prefixes
deka Example: dekameter da 	10**1 	Ten

> +#define DECI	10L
> +#define CENTI	100L
> +#define MILLI	1000L
> +#define MICRO	1000000L
> +#define NANO	1000000000L
> +#define PICO	1000000000000LL
> +#define FEMTO	1000000000000000LL

IMO: Might as well include all the prefixes up to 10**24

EXA ZETTA YOTTA, ATTA ZEPTO YOCTO

And how do people avoid using MILLI for KILO, MEGA for MICRO, etc...
The compiler won't care but usage could look odd.

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Jarkko Nikula]

On 6/7/21 10:09 PM, Joe Perches wrote:
> On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
>> +#define HECTO 100L
>> +#define DECA 10L
> 
> Should it be DECA or DEKA or both?
> 
> https://www.nist.gov/pml/weights-and-measures/metric-si-prefixes
> deka Example: dekameter da 	10**1 	Ten
> 
>> +#define DECI	10L
>> +#define CENTI	100L
>> +#define MILLI	1000L
>> +#define MICRO	1000000L
>> +#define NANO	1000000000L
>> +#define PICO	1000000000000LL
>> +#define FEMTO	1000000000000000LL
> 
> IMO: Might as well include all the prefixes up to 10**24
> 
> EXA ZETTA YOTTA, ATTA ZEPTO YOCTO
> 
> And how do people avoid using MILLI for KILO, MEGA for MICRO, etc...
> The compiler won't care but usage could look odd.
> I find this patch creates more confusion than solves a real problem 
(typo where one zero is missing or one extra) and adds potentially 
needless churn. For instance I don't assume much use for the HECTO now 
or in the future.

How about if absolutely necessary just adding only KILO, MEGA and GIGA 
and questionable defines only after when the real needs arises and can 
see does "1/1000L" etc define cause any troubles to real calculations?

Jarkko

Re: [PATCH v2 1/3] units: Add SI metric prefix definitions

[Andy Shevchenko]

On Tue, Jun 08, 2021 at 11:42:46AM +0300, Jarkko Nikula wrote:
> On 6/7/21 10:09 PM, Joe Perches wrote:
> > On Mon, 2021-06-07 at 18:23 +0300, Andy Shevchenko wrote:
> > > +#define HECTO 100L
> > > +#define DECA 10L
> > 
> > Should it be DECA or DEKA or both?
> > 
> > https://www.nist.gov/pml/weights-and-measures/metric-si-prefixes
> > deka Example: dekameter da 	10**1 	Ten
> > 
> > > +#define DECI	10L
> > > +#define CENTI	100L
> > > +#define MILLI	1000L
> > > +#define MICRO	1000000L
> > > +#define NANO	1000000000L
> > > +#define PICO	1000000000000LL
> > > +#define FEMTO	1000000000000000LL
> > 
> > IMO: Might as well include all the prefixes up to 10**24
> > 
> > EXA ZETTA YOTTA, ATTA ZEPTO YOCTO
> > 
> > And how do people avoid using MILLI for KILO, MEGA for MICRO, etc...
> > The compiler won't care but usage could look odd.
> > I find this patch creates more confusion than solves a real problem
> (typo where one zero is missing or one extra) and adds potentially needless
> churn. For instance I don't assume much use for the HECTO now or in the
> future.
> 
> How about if absolutely necessary just adding only KILO, MEGA and GIGA and
> questionable defines only after when the real needs arises and can see does
> "1/1000L" etc define cause any troubles to real calculations?

You are probably looking to the I2C codebase, while I based my choice on other
sightings. I agree about the usage count for DECI/CENTI et al., but having them
defined targets at least the following:

 1) occupies namespace so if we need them in the future there will be no
    ambiguity;
 2) provides a name for multipliers for the code to read and understand the
    semantics better.

-- 
With Best Regards,
Andy Shevchenko