Message ID | 1617197790-30627-3-git-send-email-yangyicong@hisilicon.com |
---|---|
State | New |
Headers | show |
Series | Add support for HiSilicon I2C controller | expand |
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h > index 10bd0b0..7268180 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -47,6 +47,26 @@ typedef int (*i2c_slave_cb_t)(struct i2c_client *client, > #define I2C_MAX_HIGH_SPEED_MODE_FREQ 3400000 > #define I2C_MAX_ULTRA_FAST_MODE_FREQ 5000000 > > +static inline const char *i2c_freq_mode_string(u32 bus_freq_hz) > +{ > + switch (bus_freq_hz) { > + case I2C_MAX_STANDARD_MODE_FREQ: > + return "Standard Mode (100 kHz)"; > + case I2C_MAX_FAST_MODE_FREQ: > + return "Fast Mode (400 kHz)"; > + case I2C_MAX_FAST_MODE_PLUS_FREQ: > + return "Fast Mode Plus (1.0 MHz)"; > + case I2C_MAX_TURBO_MODE_FREQ: > + return "Turbo Mode (1.4 MHz)"; > + case I2C_MAX_HIGH_SPEED_MODE_FREQ: > + return "High Speed Mode (3.4 MHz)"; > + case I2C_MAX_ULTRA_FAST_MODE_FREQ: > + return "Ultra Fast Mode (5.0 MHz)"; > + default: > + return "Unknown Mode"; > + } > +} Any reason ehy this is an inline function? My gut feeling says it would be better added to the core?
On 2021/4/7 3:54, Wolfram Sang wrote: > >> diff --git a/include/linux/i2c.h b/include/linux/i2c.h >> index 10bd0b0..7268180 100644 >> --- a/include/linux/i2c.h >> +++ b/include/linux/i2c.h >> @@ -47,6 +47,26 @@ typedef int (*i2c_slave_cb_t)(struct i2c_client *client, >> #define I2C_MAX_HIGH_SPEED_MODE_FREQ 3400000 >> #define I2C_MAX_ULTRA_FAST_MODE_FREQ 5000000 >> >> +static inline const char *i2c_freq_mode_string(u32 bus_freq_hz) >> +{ >> + switch (bus_freq_hz) { >> + case I2C_MAX_STANDARD_MODE_FREQ: >> + return "Standard Mode (100 kHz)"; >> + case I2C_MAX_FAST_MODE_FREQ: >> + return "Fast Mode (400 kHz)"; >> + case I2C_MAX_FAST_MODE_PLUS_FREQ: >> + return "Fast Mode Plus (1.0 MHz)"; >> + case I2C_MAX_TURBO_MODE_FREQ: >> + return "Turbo Mode (1.4 MHz)"; >> + case I2C_MAX_HIGH_SPEED_MODE_FREQ: >> + return "High Speed Mode (3.4 MHz)"; >> + case I2C_MAX_ULTRA_FAST_MODE_FREQ: >> + return "Ultra Fast Mode (5.0 MHz)"; >> + default: >> + return "Unknown Mode"; >> + } >> +} > > Any reason ehy this is an inline function? My gut feeling says it would > be better added to the core? > it's not a complicated function so i didn't think it'll make much difference, so i just put it in the header along with the coresponding macro definitions. do you want me to move it to the core? Thanks
On Wed, Apr 07, 2021 at 04:29:29PM +0800, Yicong Yang wrote: > On 2021/4/7 3:54, Wolfram Sang wrote: > > > >> diff --git a/include/linux/i2c.h b/include/linux/i2c.h > >> index 10bd0b0..7268180 100644 > >> --- a/include/linux/i2c.h > >> +++ b/include/linux/i2c.h > >> @@ -47,6 +47,26 @@ typedef int (*i2c_slave_cb_t)(struct i2c_client *client, > >> #define I2C_MAX_HIGH_SPEED_MODE_FREQ 3400000 > >> #define I2C_MAX_ULTRA_FAST_MODE_FREQ 5000000 > >> > >> +static inline const char *i2c_freq_mode_string(u32 bus_freq_hz) > >> +{ > >> + switch (bus_freq_hz) { > >> + case I2C_MAX_STANDARD_MODE_FREQ: > >> + return "Standard Mode (100 kHz)"; > >> + case I2C_MAX_FAST_MODE_FREQ: > >> + return "Fast Mode (400 kHz)"; > >> + case I2C_MAX_FAST_MODE_PLUS_FREQ: > >> + return "Fast Mode Plus (1.0 MHz)"; > >> + case I2C_MAX_TURBO_MODE_FREQ: > >> + return "Turbo Mode (1.4 MHz)"; > >> + case I2C_MAX_HIGH_SPEED_MODE_FREQ: > >> + return "High Speed Mode (3.4 MHz)"; > >> + case I2C_MAX_ULTRA_FAST_MODE_FREQ: > >> + return "Ultra Fast Mode (5.0 MHz)"; > >> + default: > >> + return "Unknown Mode"; > >> + } > >> +} > > > > Any reason ehy this is an inline function? My gut feeling says it would > > be better added to the core? > > > > it's not a complicated function so i didn't think it'll make much difference, > so i just put it in the header along with the coresponding macro definitions. > do you want me to move it to the core? I guess exporting will save few dozens of bytes if the function is used more than once. (All strings will be duplicated or multiplied in that case) -- With Best Regards, Andy Shevchenko
On 2021/4/7 18:08, Andy Shevchenko wrote: > On Wed, Apr 07, 2021 at 04:29:29PM +0800, Yicong Yang wrote: >> On 2021/4/7 3:54, Wolfram Sang wrote: >>> >>>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h >>>> index 10bd0b0..7268180 100644 >>>> --- a/include/linux/i2c.h >>>> +++ b/include/linux/i2c.h >>>> @@ -47,6 +47,26 @@ typedef int (*i2c_slave_cb_t)(struct i2c_client *client, >>>> #define I2C_MAX_HIGH_SPEED_MODE_FREQ 3400000 >>>> #define I2C_MAX_ULTRA_FAST_MODE_FREQ 5000000 >>>> >>>> +static inline const char *i2c_freq_mode_string(u32 bus_freq_hz) >>>> +{ >>>> + switch (bus_freq_hz) { >>>> + case I2C_MAX_STANDARD_MODE_FREQ: >>>> + return "Standard Mode (100 kHz)"; >>>> + case I2C_MAX_FAST_MODE_FREQ: >>>> + return "Fast Mode (400 kHz)"; >>>> + case I2C_MAX_FAST_MODE_PLUS_FREQ: >>>> + return "Fast Mode Plus (1.0 MHz)"; >>>> + case I2C_MAX_TURBO_MODE_FREQ: >>>> + return "Turbo Mode (1.4 MHz)"; >>>> + case I2C_MAX_HIGH_SPEED_MODE_FREQ: >>>> + return "High Speed Mode (3.4 MHz)"; >>>> + case I2C_MAX_ULTRA_FAST_MODE_FREQ: >>>> + return "Ultra Fast Mode (5.0 MHz)"; >>>> + default: >>>> + return "Unknown Mode"; >>>> + } >>>> +} >>> >>> Any reason ehy this is an inline function? My gut feeling says it would >>> be better added to the core? >>> >> >> it's not a complicated function so i didn't think it'll make much difference, >> so i just put it in the header along with the coresponding macro definitions. >> do you want me to move it to the core? > > I guess exporting will save few dozens of bytes if the function is used more > than once. (All strings will be duplicated or multiplied in that case) > yes, that's one concern. since we don't need this to perform fast, an inline one maybe unnecessary.
> > I guess exporting will save few dozens of bytes if the function is used more > > than once. (All strings will be duplicated or multiplied in that case) > > > > yes, that's one concern. since we don't need this to perform fast, an inline > one maybe unnecessary. Exactly. I also don't see an advantage of the function being inline. But potential disadvantage, even if just small memory overhead. So, I'd still rather see it as a core function.
diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 10bd0b0..7268180 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -47,6 +47,26 @@ typedef int (*i2c_slave_cb_t)(struct i2c_client *client, #define I2C_MAX_HIGH_SPEED_MODE_FREQ 3400000 #define I2C_MAX_ULTRA_FAST_MODE_FREQ 5000000 +static inline const char *i2c_freq_mode_string(u32 bus_freq_hz) +{ + switch (bus_freq_hz) { + case I2C_MAX_STANDARD_MODE_FREQ: + return "Standard Mode (100 kHz)"; + case I2C_MAX_FAST_MODE_FREQ: + return "Fast Mode (400 kHz)"; + case I2C_MAX_FAST_MODE_PLUS_FREQ: + return "Fast Mode Plus (1.0 MHz)"; + case I2C_MAX_TURBO_MODE_FREQ: + return "Turbo Mode (1.4 MHz)"; + case I2C_MAX_HIGH_SPEED_MODE_FREQ: + return "High Speed Mode (3.4 MHz)"; + case I2C_MAX_ULTRA_FAST_MODE_FREQ: + return "Ultra Fast Mode (5.0 MHz)"; + default: + return "Unknown Mode"; + } +} + struct module; struct property_entry;