| Message ID | 20231006090014.277448-1-tudor.ambarus@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | soc: samsung: usi: remove superfluous parameter | expand |
On Fri, Oct 6, 2023 at 4:00 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > > exynos_usi_set_sw_conf() has a single caller, exynos_usi_configure, > which called it with exynos_usi_set_sw_conf(usi, usi->mode). Since the > mode is already defined in struct exynos_usi, remove the superfluous > pass of the mode parameter. While in exynos_usi_set_sw_conf(), remove the > reinitialization of usi->mode with its same value. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- This driver was designed with the easiness of its further possible extension in mind. For example, passing the "mode" into exynos_usi_set_sw_conf() allows one to implement some sysfs nodes for switching USI configuration manually, just by passing the user's mode into exynos_usi_set_sw_conf(). Other things which might also appear as over-engineered, make it easy to add USI_V1 implementation, as we discussed that possibility at the time. So yeah, it was done for a reason, and I'd prefer to leave it as is. > drivers/soc/samsung/exynos-usi.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/soc/samsung/exynos-usi.c b/drivers/soc/samsung/exynos-usi.c > index 114352695ac2..269ed930c5a9 100644 > --- a/drivers/soc/samsung/exynos-usi.c > +++ b/drivers/soc/samsung/exynos-usi.c > @@ -95,14 +95,14 @@ MODULE_DEVICE_TABLE(of, exynos_usi_dt_match); > /** > * exynos_usi_set_sw_conf - Set USI block configuration mode > * @usi: USI driver object > - * @mode: Mode index > * > * Select underlying serial protocol (UART/SPI/I2C) in USI IP-core. > * > * Return: 0 on success, or negative error code on failure. > */ > -static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode) > +static int exynos_usi_set_sw_conf(struct exynos_usi *usi) > { > + size_t mode = usi->mode; > unsigned int val; > int ret; > > @@ -115,8 +115,7 @@ static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode) > if (ret) > return ret; > > - usi->mode = mode; > - dev_dbg(usi->dev, "protocol: %s\n", exynos_usi_modes[usi->mode].name); > + dev_dbg(usi->dev, "protocol: %s\n", exynos_usi_modes[mode].name); > > return 0; > } > @@ -164,7 +163,7 @@ static int exynos_usi_configure(struct exynos_usi *usi) > { > int ret; > > - ret = exynos_usi_set_sw_conf(usi, usi->mode); > + ret = exynos_usi_set_sw_conf(usi); > if (ret) > return ret; > > -- > 2.42.0.609.gbb76f46606-goog >
Hi, On 10/7/23 00:08, Sam Protsenko wrote: > On Fri, Oct 6, 2023 at 4:00 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote: >> >> exynos_usi_set_sw_conf() has a single caller, exynos_usi_configure, >> which called it with exynos_usi_set_sw_conf(usi, usi->mode). Since the >> mode is already defined in struct exynos_usi, remove the superfluous >> pass of the mode parameter. While in exynos_usi_set_sw_conf(), remove the >> reinitialization of usi->mode with its same value. >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >> --- > > This driver was designed with the easiness of its further possible > extension in mind. For example, passing the "mode" into > exynos_usi_set_sw_conf() allows one to implement some sysfs nodes for > switching USI configuration manually, just by passing the user's mode > into exynos_usi_set_sw_conf(). Other things which might also appear as > over-engineered, make it easy to add USI_V1 implementation, as we > discussed that possibility at the time. So yeah, it was done for a > reason, and I'd prefer to leave it as is. > Fine by me, thanks for the explanation. Cheers, ta
diff --git a/drivers/soc/samsung/exynos-usi.c b/drivers/soc/samsung/exynos-usi.c index 114352695ac2..269ed930c5a9 100644 --- a/drivers/soc/samsung/exynos-usi.c +++ b/drivers/soc/samsung/exynos-usi.c @@ -95,14 +95,14 @@ MODULE_DEVICE_TABLE(of, exynos_usi_dt_match); /** * exynos_usi_set_sw_conf - Set USI block configuration mode * @usi: USI driver object - * @mode: Mode index * * Select underlying serial protocol (UART/SPI/I2C) in USI IP-core. * * Return: 0 on success, or negative error code on failure. */ -static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode) +static int exynos_usi_set_sw_conf(struct exynos_usi *usi) { + size_t mode = usi->mode; unsigned int val; int ret; @@ -115,8 +115,7 @@ static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode) if (ret) return ret; - usi->mode = mode; - dev_dbg(usi->dev, "protocol: %s\n", exynos_usi_modes[usi->mode].name); + dev_dbg(usi->dev, "protocol: %s\n", exynos_usi_modes[mode].name); return 0; } @@ -164,7 +163,7 @@ static int exynos_usi_configure(struct exynos_usi *usi) { int ret; - ret = exynos_usi_set_sw_conf(usi, usi->mode); + ret = exynos_usi_set_sw_conf(usi); if (ret) return ret;
exynos_usi_set_sw_conf() has a single caller, exynos_usi_configure, which called it with exynos_usi_set_sw_conf(usi, usi->mode). Since the mode is already defined in struct exynos_usi, remove the superfluous pass of the mode parameter. While in exynos_usi_set_sw_conf(), remove the reinitialization of usi->mode with its same value. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- drivers/soc/samsung/exynos-usi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)