Hi Maksim,
On 2024-11-01 19:17, Sebastian Reichel wrote:
Hi,
On Fri, Nov 01, 2024 at 01:34:51PM +0300, bigunclemax@gmail.com wrote:
From: Maksim Kiselev bigunclemax@gmail.com
Add support for VBUS supply regulator.
When our type-c port acts as a host(SRC), this regulator used for control VBUS supply.
Signed-off-by: Maksim Kiselev bigunclemax@gmail.com
drivers/usb/tcpm/fusb302.c | 44 +++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/tcpm/fusb302.c b/drivers/usb/tcpm/fusb302.c index ee283782792..2a258d6429b 100644 --- a/drivers/usb/tcpm/fusb302.c +++ b/drivers/usb/tcpm/fusb302.c @@ -10,6 +10,7 @@ #include <asm/gpio.h> #include <linux/delay.h> #include <linux/err.h> +#include <power/regulator.h> #include <dm/device_compat.h> #include <usb/tcpm.h> #include "fusb302_reg.h" @@ -50,10 +51,13 @@ struct fusb302_chip {
/* port status */ bool vconn_on;
- bool vbus_on; bool vbus_present; enum typec_cc_polarity cc_polarity; enum typec_cc_status cc1; enum typec_cc_status cc2;
- struct udevice *vbus;
};
static int fusb302_i2c_write(struct udevice *dev, u8 address, u8 data) @@ -506,7 +510,28 @@ done:
static int fusb302_set_vbus(struct udevice *dev, bool on, bool charge) {
- return 0;
- struct fusb302_chip *chip = dev_get_priv(dev);
- int ret = 0;
- if (chip->vbus_on == on) {
dev_dbg(dev, "vbus is already %s\n", on ? "On" : "Off");- } else {
if (CONFIG_IS_ENABLED(DM_REGULATOR) && chip->vbus) {should be enough to check for chip->vbus, since the regulator device should be NULL when DM_REGULATOR is disabled. If due to some bug its not NULL regulator_set_enable would fail gracefully with -ENOSYS, so that somebody can debug the problem.
if (on)ret = regulator_set_enable(chip->vbus, true);elseret = regulator_set_enable(chip->vbus, false);regulator_set_enable(chip->vbus, on);
Please use something like following:
ret = regulator_set_enable_if_allowed(chip->vbus, on); if (ret && ret != -ENOSYS)
Should be safe when DM_REGULATOR disabled or when chip->vbus is NULL, also work when regulator is always-on or already enabled.
if (ret < 0) {dev_dbg(dev, "cannot %s vbus regulator, ret=%d\n",on ? "enable" : "disable", ret);dev_err()?
return ret;}}chip->vbus_on = on;
vbus_on could probably be dropped, fixed and gpio regulators is currently already reference counted. Or is the set_vbus ops not called evenly and current state needs to be known?
dev_dbg(dev, "vbus := %s\n", on ? "On" : "Off");- }
- return ret;
I suggest converting this to
int ret;
if (chip->vbus_on == on) { dev_dbg(...); return 0; }
... remaining code without extra indent ...
Otherwise LGTM.
-- Sebastian
}
static int fusb302_pd_tx_flush(struct udevice *dev) @@ -1293,6 +1318,22 @@ static int fusb302_get_connector_node(struct udevice *dev, ofnode *connector_nod return 0; }
+static int fusb302_probe(struct udevice *dev) +{
- struct fusb302_chip *chip = dev_get_priv(dev);
- int ret;
- if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
ret = device_get_supply_regulator(dev, "vbus-supply", &chip->vbus);if (ret && ret != -ENOENT) {
You could probably just drop the DM_REGULATOR wrap and use:
if (ret && ret != -ENOSYS && ret != -ENOENT) {
Regards, Jonas
dev_err(dev, "Failed to get vbus-supply regulator\n");return ret;}- }
- return 0;
+}
static struct dm_tcpm_ops fusb302_ops = { .get_connector_node = fusb302_get_connector_node, .init = fusb302_init, @@ -1320,4 +1361,5 @@ U_BOOT_DRIVER(fusb302) = { .of_match = fusb302_ids, .ops = &fusb302_ops, .priv_auto = sizeof(struct fusb302_chip),
- .probe = fusb302_probe,
};
2.45.2