| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2ee54bc0-65ee-1aa7-c045-72d92594683e@gmail.com>
Date: Wed, 5 Apr 2023 10:42:30 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Benjamin Bara <bbara93@...il.com>,
Support Opensource <support.opensource@...semi.com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, Lee Jones <lee@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Steve Twiss <stwiss.opensource@...semi.com>
Cc: DLG-Adam.Thomson.Opensource@...renesas.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
Benjamin Bara <benjamin.bara@...data.com>
Subject: Re: [PATCH v2 2/3] regulator: da9063: implement basic XVP setter
On 4/5/23 08:29, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@...data.com>
>
> Allow to en- and disable voltage monitoring from the device tree.
> Consider that the da9063 only monitors UV *and* OV together, so both
> must be en- or disabled.
>
> Signed-off-by: Benjamin Bara <benjamin.bara@...data.com>
> ---
> drivers/regulator/da9063-regulator.c | 100 +++++++++++++++++++++++++----------
> 1 file changed, 72 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
> index 1c720fc595b3..000fa0daef18 100644
> --- a/drivers/regulator/da9063-regulator.c
> +++ b/drivers/regulator/da9063-regulator.c
> @@ -207,6 +207,24 @@ static const unsigned int da9063_bmem_bio_merged_limits[] = {
> 4600000, 4800000, 5000000, 5200000, 5400000, 5600000, 5800000, 6000000
> };
>
> +static int da9063_set_xvp(struct regulator_dev *rdev, int lim_uV, int severity, bool enable)
> +{
> + struct da9063_regulator *regl = rdev_get_drvdata(rdev);
> + struct device *dev = regl->hw->dev;
> +
> + dev_dbg(dev, "%s: lim: %d, sev: %d, en: %d\n", regl->desc.name, lim_uV, severity, enable);
> +
> + /*
> + * only support enable and disable.
> + * the da9063 offers a GPIO (GP_FB2) which is unasserted if an XV happens.
> + * therefore ignore severity here, as there might be handlers in hardware.
> + */
> + if (lim_uV)
> + return -EINVAL;
> +
> + return regmap_field_write(regl->vmon, enable ? 1 : 0);
> +}
> +
> static int da9063_buck_set_mode(struct regulator_dev *rdev, unsigned int mode)
> {
> struct da9063_regulator *regl = rdev_get_drvdata(rdev);
> @@ -545,37 +563,41 @@ static int da9063_buck_get_current_limit(struct regulator_dev *rdev)
> }
>
> static const struct regulator_ops da9063_buck_ops = {
> - .enable = regulator_enable_regmap,
> - .disable = regulator_disable_regmap,
> - .is_enabled = regulator_is_enabled_regmap,
> - .get_voltage_sel = regulator_get_voltage_sel_regmap,
> - .set_voltage_sel = regulator_set_voltage_sel_regmap,
> - .list_voltage = regulator_list_voltage_linear,
> - .set_current_limit = da9063_buck_set_current_limit,
> - .get_current_limit = da9063_buck_get_current_limit,
> - .set_mode = da9063_buck_set_mode,
> - .get_mode = da9063_buck_get_mode,
> - .get_status = da9063_buck_get_status,
> - .set_suspend_voltage = da9063_set_suspend_voltage,
> - .set_suspend_enable = da9063_suspend_enable,
> - .set_suspend_disable = da9063_suspend_disable,
> - .set_suspend_mode = da9063_buck_set_suspend_mode,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .is_enabled = regulator_is_enabled_regmap,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .list_voltage = regulator_list_voltage_linear,
> + .set_current_limit = da9063_buck_set_current_limit,
> + .get_current_limit = da9063_buck_get_current_limit,
> + .set_mode = da9063_buck_set_mode,
> + .get_mode = da9063_buck_get_mode,
> + .get_status = da9063_buck_get_status,
> + .set_suspend_voltage = da9063_set_suspend_voltage,
> + .set_suspend_enable = da9063_suspend_enable,
> + .set_suspend_disable = da9063_suspend_disable,
> + .set_suspend_mode = da9063_buck_set_suspend_mode,
> + .set_over_voltage_protection = da9063_set_xvp,
> + .set_under_voltage_protection = da9063_set_xvp,
> };
>
> static const struct regulator_ops da9063_ldo_ops = {
> - .enable = regulator_enable_regmap,
> - .disable = regulator_disable_regmap,
> - .is_enabled = regulator_is_enabled_regmap,
> - .get_voltage_sel = regulator_get_voltage_sel_regmap,
> - .set_voltage_sel = regulator_set_voltage_sel_regmap,
> - .list_voltage = regulator_list_voltage_linear,
> - .set_mode = da9063_ldo_set_mode,
> - .get_mode = da9063_ldo_get_mode,
> - .get_status = da9063_ldo_get_status,
> - .set_suspend_voltage = da9063_set_suspend_voltage,
> - .set_suspend_enable = da9063_suspend_enable,
> - .set_suspend_disable = da9063_suspend_disable,
> - .set_suspend_mode = da9063_ldo_set_suspend_mode,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .is_enabled = regulator_is_enabled_regmap,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .list_voltage = regulator_list_voltage_linear,
> + .set_mode = da9063_ldo_set_mode,
> + .get_mode = da9063_ldo_get_mode,
> + .get_status = da9063_ldo_get_status,
> + .set_suspend_voltage = da9063_set_suspend_voltage,
> + .set_suspend_enable = da9063_suspend_enable,
> + .set_suspend_disable = da9063_suspend_disable,
> + .set_suspend_mode = da9063_ldo_set_suspend_mode,
> + .set_over_voltage_protection = da9063_set_xvp,
> + .set_under_voltage_protection = da9063_set_xvp,
> };
During my recent visit in the IIO territory I was told to by Jonathan to
drop the 'pretty indenting' of structs like this. I think this shows
well why - when longer members are added, it's hard to see from the diff
what actually changed. So, if you re-spin and unless Mark has another
opinion, maybe drop the tabs - in my eyes this does not do much for the
readability.
Well, IMO this is definitely not something that would require a re-spin
- and it may be others disagree with me on this. So, FWIW:
Reviewed-by: Matti Vaittinen <mazziesaccount@...il.com>
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Powered by blists - more mailing lists