On 2020-02-27 13:21, Andy Shevchenko wrote:
> There are few maximum bus frequencies being used in the I²C core code.
> Provide generic definitions for bus frequencies and use them in the core.
>
> The drivers may use predefined constants where it is appropriate.
> Some of them are already using these under slightly different names.
> We will convert them later to use newly introduced defines.
>
> These definitions will also help to avoid typos in the numbers that
> may lead to subtle errors.
>
> Acked-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> v2:
> started with a core patch
> used MAX in the definition names
> dropped HZ_PER_*
> added tag
>
> drivers/i2c/i2c-core-acpi.c | 2 +-
> drivers/i2c/i2c-core-base.c | 8 ++++----
> include/linux/i2c.h | 8 ++++++++
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index 8f3dbc97a057..7665685e3ca8 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -318,7 +318,7 @@ static acpi_status i2c_acpi_lookup_speed(acpi_handle handle, u32 level,
> lookup->min_speed = lookup->speed;
>
> if (acpi_match_device_ids(adev, i2c_acpi_force_400khz_device_ids) == 0)
> - lookup->force_speed = 400000;
> + lookup->force_speed = I2C_MAX_FAST_MODE_FREQ;
>
> return AE_OK;
> }
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index cefad0881942..9b2972c7faa2 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1612,13 +1612,13 @@ void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_de
>
> ret = device_property_read_u32(dev, "clock-frequency", &t->bus_freq_hz);
> if (ret && use_defaults)
> - t->bus_freq_hz = 100000;
> + t->bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
>
> ret = device_property_read_u32(dev, "i2c-scl-rising-time-ns", &t->scl_rise_ns);
> if (ret && use_defaults) {
> - if (t->bus_freq_hz <= 100000)
> + if (t->bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ)
> t->scl_rise_ns = 1000;
> - else if (t->bus_freq_hz <= 400000)
> + else if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ)
> t->scl_rise_ns = 300;
> else
> t->scl_rise_ns = 120;
> @@ -1626,7 +1626,7 @@ void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool use_de
>
> ret = device_property_read_u32(dev, "i2c-scl-falling-time-ns", &t->scl_fall_ns);
> if (ret && use_defaults) {
> - if (t->bus_freq_hz <= 400000)
> + if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ)
> t->scl_fall_ns = 300;
> else
> t->scl_fall_ns = 120;
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index f834687989f7..df70c493aed5 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -39,6 +39,14 @@ enum i2c_slave_event;
> typedef int (*i2c_slave_cb_t)(struct i2c_client *client,
> enum i2c_slave_event event, u8 *val);
>
> +/* I2C Frequency Modes */
> +#define I2C_MAX_STANDARD_MODE_FREQ 100000
> +#define I2C_MAX_FAST_MODE_FREQ 400000
> +#define I2C_MAX_FAST_PLUS_MODE_FREQ 1000000
> +#define I2C_MAX_TURBO_MODE_FREQ 1400000
> +#define I2C_MAX_HIGH_SPEED_MODE_FREQ 3400000
> +#define I2C_MAX_ULTRA_SPEED_MODE_FREQ 5000000
Am I the only one who do /not/ find these names readable? I can't seem to
remember what frequency TURBO, HIGH, etc are and their ordering is
difficult. It's all sounds like marketing buzz to me and my brain shuts
off instantly.
This feels a lot like moving away from octal permissions for files, which
is frowned upon...
Can we include some kind of indication of the actual frequency in the
names as well please?
Perhaps something like I2C_MAX_STANDARD_MODE_100KHZ?
Cheers,
Peter
> +
> struct module;
> struct property_entry;
>
>
