On Wed, Dec 16, 2020 at 5:27 AM Adrien Grassein
<adrien.grassein@xxxxxxxxx> wrote:
>
> Fix the ternary condition which is a bad coding style
> in the kernel
>
> I also remove the defering configuration of the nxp,phase-shift.
> The configuration is now done at parsing time. It save some memory
> and it's better for comprehension.
>
> I also use the OTP default configuration when the paramater is wrong
> or not specified.
> I think that it's better to use the default configuration from the chip
> than an arbitrary value.
>
> Signed-off-by: Adrien Grassein <adrien.grassein@xxxxxxxxx>
> ---
> drivers/regulator/pf8x00-regulator.c | 45 +++++++++++++---------------
> 1 file changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/regulator/pf8x00-regulator.c b/drivers/regulator/pf8x00-regulator.c
> index 5ad940b3db0a..b8b3ac393ee8 100644
> --- a/drivers/regulator/pf8x00-regulator.c
> +++ b/drivers/regulator/pf8x00-regulator.c
> @@ -107,7 +107,6 @@ enum pf8x00_ldo_states {
>
> #define PF8X00_SWXILIM_MASK GENMASK(4, 3)
> #define PF8X00_SWXPHASE_MASK GENMASK(2, 0)
> -#define PF8X00_SWXPHASE_DEFAULT 0
Keep this as it is and assign it 1 as the below code assigns 1.
> #define PF8X00_SWXPHASE_SHIFT 7
>
> enum pf8x00_devid {
> @@ -121,7 +120,6 @@ enum pf8x00_devid {
>
> struct pf8x00_regulator {
> struct regulator_desc desc;
> - u8 phase_shift;
> };
>
> struct pf8x00_chip {
> @@ -167,17 +165,13 @@ static const int pf8x00_vsnvs_voltages[] = {
> 0, 1800000, 3000000, 3300000,
> };
>
> -static struct pf8x00_regulator *desc_to_regulator(const struct regulator_desc *desc)
> -{
> - return container_of(desc, struct pf8x00_regulator, desc);
> -}
> -
> static int pf8x00_of_parse_cb(struct device_node *np,
> const struct regulator_desc *desc,
> struct regulator_config *config)
> {
> - struct pf8x00_regulator *data = desc_to_regulator(desc);
> struct pf8x00_chip *chip = config->driver_data;
> + unsigned char id = desc->id - PF8X00_LDO4;
> + unsigned char reg = PF8X00_SW_BASE(id) + SW_CONFIG2;
> int phase;
> int val;
> int ret;
> @@ -185,21 +179,30 @@ static int pf8x00_of_parse_cb(struct device_node *np,
> ret = of_property_read_u32(np, "nxp,phase-shift", &val);
> if (ret) {
> dev_dbg(chip->dev,
> - "unspecified phase-shift for BUCK%d, use 0 degrees\n",
> - desc->id - PF8X00_LDO4);
> - val = PF8X00_SWXPHASE_DEFAULT;
> + "unspecified phase-shift for BUCK%d, using OTP configuration\n",
> + id);
> + goto end;
> }
>
> - phase = val / 45;
> - if ((phase * 45) != val) {
> + if (val < 0 || val > 315 || val % 45 != 0) {
> dev_warn(config->dev,
> - "invalid phase_shift %d for BUCK%d, use 0 degrees\n",
> - (phase * 45), desc->id - PF8X00_LDO4);
> - phase = PF8X00_SWXPHASE_SHIFT;
> + "invalid phase_shift %d for BUCK%d, using OTP configuration\n",
> + val, id);
> + goto end;
> }
>
> - data->phase_shift = (phase >= 1) ? phase - 1 : PF8X00_SWXPHASE_SHIFT;
> + phase = val / 45;
> +
> + if (phase >= 1)
> + phase -= 1;
> + else
> + phase = PF8X00_SWXPHASE_SHIFT;
> +
> + regmap_update_bits(chip->regmap, reg,
> + PF8X00_SWXPHASE_MASK,
> + phase);
Add all these arguments in the same line.
