| Message ID | 20240628151346.1152838-9-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: (amc6821) Various improvements | expand |
Hi Guenter, On 6/28/24 5:13 PM, Guenter Roeck wrote: > The driver only supports a single chip, so an enum > to determine the chip type is unnecessary. Drop it. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/hwmon/amc6821.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c > index 042e2044de7b..ebffc9393c3d 100644 > --- a/drivers/hwmon/amc6821.c > +++ b/drivers/hwmon/amc6821.c > @@ -36,8 +36,6 @@ module_param(pwminv, int, 0444); > static int init = 1; /*Power-on initialization.*/ > module_param(init, int, 0444); > > -enum chips { amc6821 }; > - > #define AMC6821_REG_DEV_ID 0x3D > #define AMC6821_REG_COMP_ID 0x3E > #define AMC6821_REG_CONF1 0x00 > @@ -943,7 +941,7 @@ static int amc6821_probe(struct i2c_client *client) > } > > static const struct i2c_device_id amc6821_id[] = { > - { "amc6821", amc6821 }, > + { "amc6821", 0 }, amc6821_id being a global variable, its content should be initialized by the compiler if omitted, and the default value of kernel_ulong_t (aka unsigned long) is 0. So I think we could just remove the second parameter there. Doesn't hurt to keep it though and it seems there's a mix of implicit and explicit initialization among the i2c kernel drivers, so with that said: Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> Thanks! Quentin
On 7/1/24 04:36, Quentin Schulz wrote: > Hi Guenter, > > On 6/28/24 5:13 PM, Guenter Roeck wrote: >> The driver only supports a single chip, so an enum >> to determine the chip type is unnecessary. Drop it. >> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> drivers/hwmon/amc6821.c | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c >> index 042e2044de7b..ebffc9393c3d 100644 >> --- a/drivers/hwmon/amc6821.c >> +++ b/drivers/hwmon/amc6821.c >> @@ -36,8 +36,6 @@ module_param(pwminv, int, 0444); >> static int init = 1; /*Power-on initialization.*/ >> module_param(init, int, 0444); >> -enum chips { amc6821 }; >> - >> #define AMC6821_REG_DEV_ID 0x3D >> #define AMC6821_REG_COMP_ID 0x3E >> #define AMC6821_REG_CONF1 0x00 >> @@ -943,7 +941,7 @@ static int amc6821_probe(struct i2c_client *client) >> } >> static const struct i2c_device_id amc6821_id[] = { >> - { "amc6821", amc6821 }, >> + { "amc6821", 0 }, > > amc6821_id being a global variable, its content should be initialized by the compiler if omitted, and the default value of kernel_ulong_t (aka unsigned long) is 0. So I think we could just remove the second parameter there. > > Doesn't hurt to keep it though and it seems there's a mix of implicit and explicit initialization among the i2c kernel drivers, so with that said: > I may use that inconsistently myself. I am never sure if I want to say "keep default" or "explicitly state that the value is 0". I'll keep it as-is. > Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> > Thanks, Guenter
diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c index 042e2044de7b..ebffc9393c3d 100644 --- a/drivers/hwmon/amc6821.c +++ b/drivers/hwmon/amc6821.c @@ -36,8 +36,6 @@ module_param(pwminv, int, 0444); static int init = 1; /*Power-on initialization.*/ module_param(init, int, 0444); -enum chips { amc6821 }; - #define AMC6821_REG_DEV_ID 0x3D #define AMC6821_REG_COMP_ID 0x3E #define AMC6821_REG_CONF1 0x00 @@ -943,7 +941,7 @@ static int amc6821_probe(struct i2c_client *client) } static const struct i2c_device_id amc6821_id[] = { - { "amc6821", amc6821 }, + { "amc6821", 0 }, { } }; @@ -952,7 +950,6 @@ MODULE_DEVICE_TABLE(i2c, amc6821_id); static const struct of_device_id __maybe_unused amc6821_of_match[] = { { .compatible = "ti,amc6821", - .data = (void *)amc6821, }, { } };
The driver only supports a single chip, so an enum to determine the chip type is unnecessary. Drop it. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/hwmon/amc6821.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)