| Message ID | 20220317223051.1227110-4-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: (adt7475) pin configuration | expand |
On Fri, Mar 18, 2022 at 11:30:50AM +1300, Chris Packham wrote: > Simplify load_attenuators() by making use of enum chips instead of int. > That isn't the only thing the patch is doing. > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > > Notes: > Changes in v2: > - New > > drivers/hwmon/adt7475.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c > index 6de501de41b2..ebe4a85eb62e 100644 > --- a/drivers/hwmon/adt7475.c > +++ b/drivers/hwmon/adt7475.c > @@ -1569,7 +1569,7 @@ static int set_property_bit(const struct i2c_client *client, char *property, > return ret; > } > > -static int load_attenuators(const struct i2c_client *client, int chip, > +static int load_attenuators(const struct i2c_client *client, enum chips chip, > struct adt7475_data *data) > { > int ret; > @@ -1588,7 +1588,7 @@ static int load_attenuators(const struct i2c_client *client, int chip, > data->config4); > if (ret < 0) > return ret; > - } else if (chip == adt7473 || chip == adt7475) { > + } else { This is the real change. Well, in theory. It doesn't really make a difference, it is just (currently) unnecessary but clarifies that the following code only applies to the two chips. It may be better to replace the if/else with a switch statement to clarify this. Dropping the conditional would not require to change the parameter type. That only really adds value if you also use a switch statement (without dummy default). Thanks, Guenter > set_property_bit(client, "adi,bypass-attenuator-in1", > &data->config2, 5); >
On 19/03/22 03:32, Guenter Roeck wrote: > On Fri, Mar 18, 2022 at 11:30:50AM +1300, Chris Packham wrote: >> Simplify load_attenuators() by making use of enum chips instead of int. >> > That isn't the only thing the patch is doing. > >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> >> Notes: >> Changes in v2: >> - New >> >> drivers/hwmon/adt7475.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c >> index 6de501de41b2..ebe4a85eb62e 100644 >> --- a/drivers/hwmon/adt7475.c >> +++ b/drivers/hwmon/adt7475.c >> @@ -1569,7 +1569,7 @@ static int set_property_bit(const struct i2c_client *client, char *property, >> return ret; >> } >> >> -static int load_attenuators(const struct i2c_client *client, int chip, >> +static int load_attenuators(const struct i2c_client *client, enum chips chip, >> struct adt7475_data *data) >> { >> int ret; >> @@ -1588,7 +1588,7 @@ static int load_attenuators(const struct i2c_client *client, int chip, >> data->config4); >> if (ret < 0) >> return ret; >> - } else if (chip == adt7473 || chip == adt7475) { >> + } else { > This is the real change. Well, in theory. It doesn't really make a difference, > it is just (currently) unnecessary but clarifies that the following code only > applies to the two chips. It may be better to replace the if/else with a switch > statement to clarify this. Dropping the conditional would not require to change > the parameter type. That only really adds value if you also use a switch > statement (without dummy default). I've written a v3 that updates this to use a switch statement but I'll wait to send it in case there is any feedback on the first 2 patches. > Thanks, > Guenter > >> set_property_bit(client, "adi,bypass-attenuator-in1", >> &data->config2, 5); >>
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c index 6de501de41b2..ebe4a85eb62e 100644 --- a/drivers/hwmon/adt7475.c +++ b/drivers/hwmon/adt7475.c @@ -1569,7 +1569,7 @@ static int set_property_bit(const struct i2c_client *client, char *property, return ret; } -static int load_attenuators(const struct i2c_client *client, int chip, +static int load_attenuators(const struct i2c_client *client, enum chips chip, struct adt7475_data *data) { int ret; @@ -1588,7 +1588,7 @@ static int load_attenuators(const struct i2c_client *client, int chip, data->config4); if (ret < 0) return ret; - } else if (chip == adt7473 || chip == adt7475) { + } else { set_property_bit(client, "adi,bypass-attenuator-in1", &data->config2, 5);
Simplify load_attenuators() by making use of enum chips instead of int. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v2: - New drivers/hwmon/adt7475.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)