| 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: <CAGb2v67iPsUAG=Sgk8z-G3KjVb4WoMUutUPKdQ9s=OOCk_jgqg@mail.gmail.com>
Date: Tue, 17 May 2016 01:06:32 +0800
From: Chen-Yu Tsai <wens@...e.org>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Chen-Yu Tsai <wens@...e.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
devicetree <devicetree@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-sunxi <linux-sunxi@...glegroups.com>
Subject: Re: [PATCH] regulator: axp20x: Add support for the (external)
drivebus regulator
Hi,
On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede@...hat.com> wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
>
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
>
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.
The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
typo Allwinner has in their code.
>
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
>
> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
> ---
> Documentation/devicetree/bindings/mfd/axp20x.txt | 5 ++++
> drivers/regulator/axp20x-regulator.c | 30 ++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
> AXP152/20X: range: 750-1875, Default: 1.5 MHz
> AXP22X/80X: range: 1800-4050, Default: 3 MHz
>
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
> + pin is used as an output pin to control an external regulator
> + to drive the OTG VBus, rather then as an input pin which
> + signals whether the board is driving OTG VBus or not.
> +
You should also add it to the regulators table further down in the binding.
And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
for a boolean property.
> - <input>-supply: a phandle to the regulator supply node. May be omitted if
> inputs are unregulated, such as using the IPSOUT output
> from the PMIC.
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 514a5e8..2422ddd 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -36,6 +36,8 @@
>
> #define AXP20X_FREQ_DCDC_MASK 0x0f
>
> +#define AXP22X_MISC_N_VBUSEN_FUNC BIT(4)
> +
> #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg, \
> _vmask, _ereg, _emask, _enable_val, _disable_val) \
> [_family##_##_id] = { \
> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
> AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
> };
>
> +static const struct regulator_desc axp22x_drivebus_regulator = {
> + .name = "drivebus",
> + .supply_name = "ips",
Given that you are approximating a gpio (in this case, the
N_VBUSEN/DRIVEVBUS pin)
controlled fixed voltage regulator, it would be better if you could:
a) have some way to specify the output voltage, so it looks like a fixed voltage
regulator that can be turned on/off.
And
b) have a separate supply name for this so it properly propagates enable/disable
calls. It would also pass the upstream voltage, thus behaving like a switch
you intended with the axp20x_ops_sw below.
Regards
ChenYu
> + .of_match = of_match_ptr("drivebus"),
> + .regulators_node = of_match_ptr("regulators"),
> + .type = REGULATOR_VOLTAGE,
> + .owner = THIS_MODULE,
> + .enable_reg = AXP20X_VBUS_IPSOUT_MGMT,
> + .enable_mask = BIT(2),
> + .ops = &axp20x_ops_sw,
> +};
> +
> static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
> {
> struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> u32 workmode;
> const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
> const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
> + bool drivebus = false;
>
> switch (axp20x->variant) {
> case AXP202_ID:
> @@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> case AXP223_ID:
> regulators = axp22x_regulators;
> nregulators = AXP22X_REG_ID_MAX;
> + drivebus = of_property_read_bool(pdev->dev.parent->of_node,
> + "x-powers,drivebus");
> break;
> default:
> dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
> @@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> }
> }
>
> + if (drivebus) {
> + /* Change N_VBUSEN sense pin to DRIVEBUS output pin */
> + regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
> + AXP22X_MISC_N_VBUSEN_FUNC, 0);
> + rdev = devm_regulator_register(&pdev->dev,
> + &axp22x_drivebus_regulator,
> + &config);
> + if (IS_ERR(rdev)) {
> + dev_err(&pdev->dev, "Failed to register drivebus\n");
> + return PTR_ERR(rdev);
> + }
> + }
> +
> return 0;
> }
>
> --
> 2.7.4
>
Powered by blists - more mailing lists