[v1,04/40] i2c: altera: Use generic definitions for bus frequencies
diff mbox series

Message ID 20200224151530.31713-4-andriy.shevchenko@linux.intel.com
State Superseded
Headers show
Series
  • [v1,01/40] i2c: qup: Move bus frequency definitions to i2c.h
Related show

Commit Message

Andy Shevchenko Feb. 24, 2020, 3:14 p.m. UTC
Since we have generic definitions for bus frequencies, let's use them.

Cc: Thor Thayer <thor.thayer@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-altera.c | 8 ++++----
 include/linux/i2c.h             | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

Comments

Jarkko Nikula Feb. 25, 2020, 7:30 a.m. UTC | #1
Hi

On 2/24/20 5:14 PM, Andy Shevchenko wrote:
> Since we have generic definitions for bus frequencies, let's use them.
> 
> Cc: Thor Thayer <thor.thayer@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-altera.c | 8 ++++----
>   include/linux/i2c.h             | 1 +
>   2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-altera.c b/drivers/i2c/busses/i2c-altera.c
> index 5255d3755411..1f70bbd6e084 100644
> --- a/drivers/i2c/busses/i2c-altera.c
> +++ b/drivers/i2c/busses/i2c-altera.c
> @@ -142,12 +142,12 @@ static inline void altr_i2c_stop(struct altr_i2c_dev *idev)
>   static void altr_i2c_init(struct altr_i2c_dev *idev)
>   {
>   	u32 divisor = clk_get_rate(idev->i2c_clk) / idev->bus_clk_rate;
> -	u32 clk_mhz = clk_get_rate(idev->i2c_clk) / 1000000;
> +	u32 clk_mhz = clk_get_rate(idev->i2c_clk) / HZ_PER_MHZ;

IMHO these kind of defines don't improve readability in formulas. What 
is HZ_PER_MHZ? You have to go and grep it first in order to be sure. Of 
course it protects against typos like "100000" or "10000000" that are 
difficult to spot at quick look.

Better would be just "1MHz" or even better calc_something_MHz().

Patch
diff mbox series

diff --git a/drivers/i2c/busses/i2c-altera.c b/drivers/i2c/busses/i2c-altera.c
index 5255d3755411..1f70bbd6e084 100644
--- a/drivers/i2c/busses/i2c-altera.c
+++ b/drivers/i2c/busses/i2c-altera.c
@@ -142,12 +142,12 @@  static inline void altr_i2c_stop(struct altr_i2c_dev *idev)
 static void altr_i2c_init(struct altr_i2c_dev *idev)
 {
 	u32 divisor = clk_get_rate(idev->i2c_clk) / idev->bus_clk_rate;
-	u32 clk_mhz = clk_get_rate(idev->i2c_clk) / 1000000;
+	u32 clk_mhz = clk_get_rate(idev->i2c_clk) / HZ_PER_MHZ;
 	u32 tmp = (ALTR_I2C_THRESHOLD << ALTR_I2C_CTRL_RXT_SHFT) |
 		  (ALTR_I2C_THRESHOLD << ALTR_I2C_CTRL_TCT_SHFT);
 	u32 t_high, t_low;
 
-	if (idev->bus_clk_rate <= 100000) {
+	if (idev->bus_clk_rate <= I2C_STANDARD_MODE_FREQ) {
 		tmp &= ~ALTR_I2C_CTRL_BSPEED;
 		/* Standard mode SCL 50/50 */
 		t_high = divisor * 1 / 2;
@@ -423,10 +423,10 @@  static int altr_i2c_probe(struct platform_device *pdev)
 				       &idev->bus_clk_rate);
 	if (val) {
 		dev_err(&pdev->dev, "Default to 100kHz\n");
-		idev->bus_clk_rate = 100000;	/* default clock rate */
+		idev->bus_clk_rate = I2C_STANDARD_MODE_FREQ;	/* default clock rate */
 	}
 
-	if (idev->bus_clk_rate > 400000) {
+	if (idev->bus_clk_rate > I2C_FAST_MODE_FREQ) {
 		dev_err(&pdev->dev, "invalid clock-frequency %d\n",
 			idev->bus_clk_rate);
 		return -EINVAL;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 708ac1262a0c..1b9c483bd9f5 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -40,6 +40,7 @@  typedef int (*i2c_slave_cb_t)(struct i2c_client *client,
 			      enum i2c_slave_event event, u8 *val);
 
 #define HZ_PER_KHZ			1000
+#define HZ_PER_MHZ			1000000
 
 /* I2C Frequency Modes */
 #define I2C_STANDARD_MODE_FREQ		(100 * HZ_PER_KHZ)