Message ID | 20210731173842.19643-1-digetx@gmail.com |
---|---|
Headers | show |
Series | Add OTG mode support to Tegra USB PHY, SMB347 and Nexus 7 | expand |
Hi, On Sat, Jul 31, 2021 at 08:38:35PM +0300, Dmitry Osipenko wrote: > SMB347 can supply power to USB VBUS, which is required by OTG-cable > devices that want to switch USB port into the host mode. Add USB VBUS > regulator properties. > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > .../power/supply/summit,smb347-charger.yaml | 30 +++++++++++++++++++ > .../dt-bindings/power/summit,smb347-charger.h | 4 +++ > 2 files changed, 34 insertions(+) > > diff --git a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml > index 983fc215c1e5..20862cdfc116 100644 > --- a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml > +++ b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml > @@ -73,6 +73,26 @@ properties: > - 1 # SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT Current compensation > - 2 # SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE Voltage compensation > > + summit,inok-polarity: > + description: | > + Polarity of INOK signal indicating presence of external power supply. > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: > + - 0 # SMB3XX_SYSOK_INOK_ACTIVE_LOW > + - 1 # SMB3XX_SYSOK_INOK_ACTIVE_HIGH > + > + usb-vbus: > + $ref: "../../regulator/regulator.yaml#" > + type: object > + > + properties: > + summit,needs-inok-toggle: > + type: boolean > + description: INOK signal is fixed and polarity needs to be toggled > + in order to enable/disable output mode. > + > + unevaluatedProperties: false > + > allOf: > - if: > properties: > @@ -134,6 +154,7 @@ examples: > reg = <0x7f>; > > summit,enable-charge-control = <SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH>; > + summit,inok-polarity = <SMB3XX_SYSOK_INOK_ACTIVE_LOW>; > summit,chip-temperature-threshold-celsius = <110>; > summit,mains-current-limit-microamp = <2000000>; > summit,usb-current-limit-microamp = <500000>; > @@ -141,6 +162,15 @@ examples: > summit,enable-mains-charging; > > monitored-battery = <&battery>; > + > + usb-vbus { > + regulator-name = "usb_vbus"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + regulator-min-microamp = <750000>; > + regulator-max-microamp = <750000>; > + summit,needs-inok-toggle; > + }; > }; > }; > > diff --git a/include/dt-bindings/power/summit,smb347-charger.h b/include/dt-bindings/power/summit,smb347-charger.h > index d918bf321a71..3205699b5e41 100644 > --- a/include/dt-bindings/power/summit,smb347-charger.h > +++ b/include/dt-bindings/power/summit,smb347-charger.h > @@ -16,4 +16,8 @@ > #define SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW 1 > #define SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH 2 > > +/* Polarity of INOK signal */ > +#define SMB3XX_SYSOK_INOK_ACTIVE_LOW 0 > +#define SMB3XX_SYSOK_INOK_ACTIVE_HIGH 1 > + > #endif > -- > 2.32.0 >
Hi, On Sat, Jul 31, 2021 at 08:38:36PM +0300, Dmitry Osipenko wrote: > The smb347_set_writable() is used by interrupt handler and outside of it. > The interrupt should be disabled when the function is used outside of > interrupt handler in order to prevent racing with the interrupt context. > Add new parameter to smb347_set_writable() that allows to disable IRQ. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/power/supply/smb347-charger.c | 30 +++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c > index df240420f2de..db1378b41f80 100644 > --- a/drivers/power/supply/smb347-charger.c > +++ b/drivers/power/supply/smb347-charger.c > @@ -671,10 +671,22 @@ static int smb347_set_temp_limits(struct smb347_charger *smb) > * > * Returns %0 on success and negative errno in case of failure. > */ > -static int smb347_set_writable(struct smb347_charger *smb, bool writable) > +static int smb347_set_writable(struct smb347_charger *smb, bool writable, > + bool irq_toggle) > { > - return regmap_update_bits(smb->regmap, CMD_A, CMD_A_ALLOW_WRITE, > - writable ? CMD_A_ALLOW_WRITE : 0); > + struct i2c_client *client = to_i2c_client(smb->dev); > + int ret; > + > + if (writable && irq_toggle && !smb->irq_unsupported) > + disable_irq(client->irq); > + > + ret = regmap_update_bits(smb->regmap, CMD_A, CMD_A_ALLOW_WRITE, > + writable ? CMD_A_ALLOW_WRITE : 0); > + > + if ((!writable || ret) && irq_toggle && !smb->irq_unsupported) > + enable_irq(client->irq); > + > + return ret; > } > > static int smb347_hw_init(struct smb347_charger *smb) > @@ -682,7 +694,7 @@ static int smb347_hw_init(struct smb347_charger *smb) > unsigned int val; > int ret; > > - ret = smb347_set_writable(smb, true); > + ret = smb347_set_writable(smb, true, false); > if (ret < 0) > return ret; > > @@ -758,7 +770,7 @@ static int smb347_hw_init(struct smb347_charger *smb) > ret = smb347_start_stop_charging(smb); > > fail: > - smb347_set_writable(smb, false); > + smb347_set_writable(smb, false, false); > return ret; > } > > @@ -866,7 +878,7 @@ static int smb347_irq_set(struct smb347_charger *smb, bool enable) > if (smb->irq_unsupported) > return 0; > > - ret = smb347_set_writable(smb, true); > + ret = smb347_set_writable(smb, true, true); > if (ret < 0) > return ret; > > @@ -891,7 +903,7 @@ static int smb347_irq_set(struct smb347_charger *smb, bool enable) > ret = regmap_update_bits(smb->regmap, CFG_PIN, CFG_PIN_EN_CHARGER_ERROR, > enable ? CFG_PIN_EN_CHARGER_ERROR : 0); > fail: > - smb347_set_writable(smb, false); > + smb347_set_writable(smb, false, true); > return ret; > } > > @@ -919,7 +931,7 @@ static int smb347_irq_init(struct smb347_charger *smb, > if (!client->irq) > return 0; > > - ret = smb347_set_writable(smb, true); > + ret = smb347_set_writable(smb, true, false); > if (ret < 0) > return ret; > > @@ -931,7 +943,7 @@ static int smb347_irq_init(struct smb347_charger *smb, > CFG_STAT_ACTIVE_HIGH | CFG_STAT_DISABLED, > CFG_STAT_DISABLED); > > - smb347_set_writable(smb, false); > + smb347_set_writable(smb, false, false); > > if (ret < 0) { > dev_warn(smb->dev, "failed to initialize IRQ: %d\n", ret); > -- > 2.32.0 >
Hi, On Sat, Jul 31, 2021 at 08:38:39PM +0300, Dmitry Osipenko wrote: > SMB347 can supply power to USB VBUS, implement the USB VBUS regulator. > USB VBUS needs to be powered for switching OTG-cable USB port into host > mode. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/power/supply/Kconfig | 1 + > drivers/power/supply/smb347-charger.c | 219 ++++++++++++++++++++++++++ > 2 files changed, 220 insertions(+) > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 8543c89a0c1a..e83e9e4d512c 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -694,6 +694,7 @@ config CHARGER_BQ256XX > config CHARGER_SMB347 > tristate "Summit Microelectronics SMB3XX Battery Charger" > depends on I2C > + depends on REGULATOR > select REGMAP_I2C > help > Say Y to include support for Summit Microelectronics SMB345, > diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c > index 1c9205ca0993..753944e774c4 100644 > --- a/drivers/power/supply/smb347-charger.c > +++ b/drivers/power/supply/smb347-charger.c > @@ -18,6 +18,7 @@ > #include <linux/power_supply.h> > #include <linux/property.h> > #include <linux/regmap.h> > +#include <linux/regulator/driver.h> > > #include <dt-bindings/power/summit,smb347-charger.h> > > @@ -63,12 +64,15 @@ > #define CFG_THERM_SOFT_COLD_COMPENSATION_SHIFT 2 > #define CFG_THERM_MONITOR_DISABLED BIT(4) > #define CFG_SYSOK 0x08 > +#define CFG_SYSOK_INOK_ACTIVE_HIGH BIT(0) > #define CFG_SYSOK_SUSPEND_HARD_LIMIT_DISABLED BIT(2) > #define CFG_OTHER 0x09 > #define CFG_OTHER_RID_MASK 0xc0 > #define CFG_OTHER_RID_ENABLED_AUTO_OTG 0xc0 > #define CFG_OTG 0x0a > #define CFG_OTG_TEMP_THRESHOLD_MASK 0x30 > +#define CFG_OTG_CURRENT_LIMIT_250mA BIT(2) > +#define CFG_OTG_CURRENT_LIMIT_750mA BIT(3) > #define CFG_OTG_TEMP_THRESHOLD_SHIFT 4 > #define CFG_OTG_CC_COMPENSATION_MASK 0xc0 > #define CFG_OTG_CC_COMPENSATION_SHIFT 6 > @@ -92,6 +96,7 @@ > #define CMD_A 0x30 > #define CMD_A_CHG_ENABLED BIT(1) > #define CMD_A_SUSPEND_ENABLED BIT(2) > +#define CMD_A_OTG_ENABLED BIT(4) > #define CMD_A_ALLOW_WRITE BIT(7) > #define CMD_B 0x31 > #define CMD_C 0x33 > @@ -133,10 +138,12 @@ > * @regmap: pointer to driver regmap > * @mains: power_supply instance for AC/DC power > * @usb: power_supply instance for USB power > + * @usb_rdev: USB VBUS regulator device > * @id: SMB charger ID > * @mains_online: is AC/DC input connected > * @usb_online: is USB input connected > * @irq_unsupported: is interrupt unsupported by SMB hardware > + * @usb_vbus_enabled: is USB VBUS powered by SMB charger > * @max_charge_current: maximum current (in uA) the battery can be charged > * @max_charge_voltage: maximum voltage (in uV) the battery can be charged > * @pre_charge_current: current (in uA) to use in pre-charging phase > @@ -167,6 +174,8 @@ > * @use_usb_otg: USB OTG output can be used (not implemented yet) > * @enable_control: how charging enable/disable is controlled > * (driver/pin controls) > + * @inok_polarity: polarity of INOK signal which denotes presence of external > + * power supply > * > * @use_main, @use_usb, and @use_usb_otg are means to enable/disable > * hardware support for these. This is useful when we want to have for > @@ -189,10 +198,12 @@ struct smb347_charger { > struct regmap *regmap; > struct power_supply *mains; > struct power_supply *usb; > + struct regulator_dev *usb_rdev; > unsigned int id; > bool mains_online; > bool usb_online; > bool irq_unsupported; > + bool usb_vbus_enabled; > > unsigned int max_charge_current; > unsigned int max_charge_voltage; > @@ -213,6 +224,7 @@ struct smb347_charger { > bool use_usb; > bool use_usb_otg; > unsigned int enable_control; > + unsigned int inok_polarity; > }; > > enum smb_charger_chipid { > @@ -362,6 +374,11 @@ static int smb347_charging_set(struct smb347_charger *smb, bool enable) > return 0; > } > > + if (enable && smb->usb_vbus_enabled) { > + dev_dbg(smb->dev, "charging not enabled because USB is in host mode\n"); > + return 0; > + } > + > return regmap_update_bits(smb->regmap, CMD_A, CMD_A_CHG_ENABLED, > enable ? CMD_A_CHG_ENABLED : 0); > } > @@ -1253,6 +1270,13 @@ static void smb347_dt_parse_dev_info(struct smb347_charger *smb) > /* Select charging control */ > device_property_read_u32(dev, "summit,enable-charge-control", > &smb->enable_control); > + > + /* > + * Polarity of INOK signal indicating presence of external power > + * supply connected to the charger. > + */ > + device_property_read_u32(dev, "summit,inok-polarity", > + &smb->inok_polarity); > } > > static int smb347_get_battery_info(struct smb347_charger *smb) > @@ -1304,6 +1328,160 @@ static int smb347_get_battery_info(struct smb347_charger *smb) > return 0; > } > > +static int smb347_usb_vbus_get_current_limit(struct regulator_dev *rdev) > +{ > + struct smb347_charger *smb = rdev_get_drvdata(rdev); > + unsigned int val; > + int ret; > + > + ret = regmap_read(smb->regmap, CFG_OTG, &val); > + if (ret < 0) > + return ret; > + > + /* > + * It's unknown what happens if this bit is unset due to lack of > + * access to the datasheet, assume it's limit-enable. > + */ > + if (!(val & CFG_OTG_CURRENT_LIMIT_250mA)) > + return 0; > + > + return val & CFG_OTG_CURRENT_LIMIT_750mA ? 750000 : 250000; > +} > + > +static int smb347_usb_vbus_set_new_current_limit(struct smb347_charger *smb, > + int max_uA) > +{ > + const unsigned int mask = CFG_OTG_CURRENT_LIMIT_750mA | > + CFG_OTG_CURRENT_LIMIT_250mA; > + unsigned int val = CFG_OTG_CURRENT_LIMIT_250mA; > + int ret; > + > + if (max_uA >= 750000) > + val |= CFG_OTG_CURRENT_LIMIT_750mA; > + > + ret = regmap_update_bits(smb->regmap, CFG_OTG, mask, val); > + if (ret < 0) > + dev_err(smb->dev, "failed to change USB current limit\n"); > + > + return ret; > +} > + > +static int smb347_usb_vbus_set_current_limit(struct regulator_dev *rdev, > + int min_uA, int max_uA) > +{ > + struct smb347_charger *smb = rdev_get_drvdata(rdev); > + int ret; > + > + ret = smb347_set_writable(smb, true, true); > + if (ret < 0) > + return ret; > + > + ret = smb347_usb_vbus_set_new_current_limit(smb, max_uA); > + smb347_set_writable(smb, false, true); > + > + return ret; > +} > + > +static int smb347_usb_vbus_regulator_enable(struct regulator_dev *rdev) > +{ > + struct smb347_charger *smb = rdev_get_drvdata(rdev); > + int ret, max_uA; > + > + ret = smb347_set_writable(smb, true, true); > + if (ret < 0) > + return ret; > + > + smb347_charging_disable(smb); > + > + if (device_property_read_bool(&rdev->dev, "summit,needs-inok-toggle")) { > + unsigned int sysok = 0; > + > + if (smb->inok_polarity == SMB3XX_SYSOK_INOK_ACTIVE_LOW) > + sysok = CFG_SYSOK_INOK_ACTIVE_HIGH; > + > + /* > + * VBUS won't be powered if INOK is active, so we need to > + * manually disable INOK on some platforms. > + */ > + ret = regmap_update_bits(smb->regmap, CFG_SYSOK, > + CFG_SYSOK_INOK_ACTIVE_HIGH, sysok); > + if (ret < 0) { > + dev_err(smb->dev, "failed to disable INOK\n"); > + goto done; > + } > + } > + > + ret = smb347_usb_vbus_get_current_limit(rdev); > + if (ret < 0) { > + dev_err(smb->dev, "failed to get USB VBUS current limit\n"); > + goto done; > + } > + > + max_uA = ret; > + > + ret = smb347_usb_vbus_set_new_current_limit(smb, 250000); > + if (ret < 0) { > + dev_err(smb->dev, "failed to preset USB VBUS current limit\n"); > + goto done; > + } > + > + ret = regmap_set_bits(smb->regmap, CMD_A, CMD_A_OTG_ENABLED); > + if (ret < 0) { > + dev_err(smb->dev, "failed to enable USB VBUS\n"); > + goto done; > + } > + > + smb->usb_vbus_enabled = true; > + > + ret = smb347_usb_vbus_set_new_current_limit(smb, max_uA); > + if (ret < 0) { > + dev_err(smb->dev, "failed to restore USB VBUS current limit\n"); > + goto done; > + } > +done: > + smb347_set_writable(smb, false, true); > + > + return ret; > +} > + > +static int smb347_usb_vbus_regulator_disable(struct regulator_dev *rdev) > +{ > + struct smb347_charger *smb = rdev_get_drvdata(rdev); > + int ret; > + > + ret = smb347_set_writable(smb, true, true); > + if (ret < 0) > + return ret; > + > + ret = regmap_clear_bits(smb->regmap, CMD_A, CMD_A_OTG_ENABLED); > + if (ret < 0) { > + dev_err(smb->dev, "failed to disable USB VBUS\n"); > + goto done; > + } > + > + smb->usb_vbus_enabled = false; > + > + if (device_property_read_bool(&rdev->dev, "summit,needs-inok-toggle")) { > + unsigned int sysok = 0; > + > + if (smb->inok_polarity == SMB3XX_SYSOK_INOK_ACTIVE_HIGH) > + sysok = CFG_SYSOK_INOK_ACTIVE_HIGH; > + > + ret = regmap_update_bits(smb->regmap, CFG_SYSOK, > + CFG_SYSOK_INOK_ACTIVE_HIGH, sysok); > + if (ret < 0) { > + dev_err(smb->dev, "failed to enable INOK\n"); > + goto done; > + } > + } > + > + smb347_start_stop_charging(smb); > +done: > + smb347_set_writable(smb, false, true); > + > + return ret; > +} > + > static const struct regmap_config smb347_regmap = { > .reg_bits = 8, > .val_bits = 8, > @@ -1314,6 +1492,14 @@ static const struct regmap_config smb347_regmap = { > .num_reg_defaults_raw = SMB347_MAX_REGISTER, > }; > > +static const struct regulator_ops smb347_usb_vbus_regulator_ops = { > + .is_enabled = regulator_is_enabled_regmap, > + .enable = smb347_usb_vbus_regulator_enable, > + .disable = smb347_usb_vbus_regulator_disable, > + .get_current_limit = smb347_usb_vbus_get_current_limit, > + .set_current_limit = smb347_usb_vbus_set_current_limit, > +}; > + > static const struct power_supply_desc smb347_mains_desc = { > .name = "smb347-mains", > .type = POWER_SUPPLY_TYPE_MAINS, > @@ -1330,10 +1516,24 @@ static const struct power_supply_desc smb347_usb_desc = { > .num_properties = ARRAY_SIZE(smb347_properties), > }; > > +static const struct regulator_desc smb347_usb_vbus_regulator_desc = { > + .name = "smb347-usb-vbus", > + .of_match = of_match_ptr("usb-vbus"), > + .ops = &smb347_usb_vbus_regulator_ops, > + .type = REGULATOR_VOLTAGE, > + .owner = THIS_MODULE, > + .enable_reg = CMD_A, > + .enable_mask = CMD_A_OTG_ENABLED, > + .enable_val = CMD_A_OTG_ENABLED, > + .fixed_uV = 5000000, > + .n_voltages = 1, > +}; > + > static int smb347_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct power_supply_config mains_usb_cfg = {}; > + struct regulator_config usb_rdev_cfg = {}; > struct device *dev = &client->dev; > struct smb347_charger *smb; > int ret; > @@ -1381,6 +1581,18 @@ static int smb347_probe(struct i2c_client *client, > if (ret) > return ret; > > + usb_rdev_cfg.dev = dev; > + usb_rdev_cfg.driver_data = smb; > + usb_rdev_cfg.regmap = smb->regmap; > + > + smb->usb_rdev = devm_regulator_register(dev, > + &smb347_usb_vbus_regulator_desc, > + &usb_rdev_cfg); > + if (IS_ERR(smb->usb_rdev)) { > + smb347_irq_disable(smb); > + return PTR_ERR(smb->usb_rdev); > + } > + > return 0; > } > > @@ -1388,11 +1600,17 @@ static int smb347_remove(struct i2c_client *client) > { > struct smb347_charger *smb = i2c_get_clientdata(client); > > + smb347_usb_vbus_regulator_disable(smb->usb_rdev); > smb347_irq_disable(smb); > > return 0; > } > > +static void smb347_shutdown(struct i2c_client *client) > +{ > + smb347_remove(client); > +} > + > static const struct i2c_device_id smb347_id[] = { > { "smb345", SMB345 }, > { "smb347", SMB347 }, > @@ -1416,6 +1634,7 @@ static struct i2c_driver smb347_driver = { > }, > .probe = smb347_probe, > .remove = smb347_remove, > + .shutdown = smb347_shutdown, > .id_table = smb347_id, > }; > module_i2c_driver(smb347_driver); > -- > 2.32.0 >
07.08.2021 00:13, Sebastian Reichel пишет: > Hi, > > On Sat, Jul 31, 2021 at 08:38:35PM +0300, Dmitry Osipenko wrote: >> SMB347 can supply power to USB VBUS, which is required by OTG-cable >> devices that want to switch USB port into the host mode. Add USB VBUS >> regulator properties. >> >> Reviewed-by: Rob Herring <robh@kernel.org> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> Sebastian, you can pick up these patches into the power tree: dt-bindings: power: supply: smb347-charger: Document USB VBUS regulator power: supply: smb347-charger: Make smb347_set_writable() IRQ-safe power: supply: smb347-charger: Utilize generic regmap caching power: supply: smb347-charger: Add missing pin control activation power: supply: smb347-charger: Implement USB VBUS regulator The reset of the patches could go via the Tegra tree. It's probably a bit too late for the Tegra patches since Thierry already made 5.15 PR, but should be fine for the power. Thanks in advance!
Hi, On Mon, Aug 16, 2021 at 06:39:09PM +0300, Dmitry Osipenko wrote: > 07.08.2021 00:13, Sebastian Reichel пишет: > > Hi, > > > > On Sat, Jul 31, 2021 at 08:38:35PM +0300, Dmitry Osipenko wrote: > >> SMB347 can supply power to USB VBUS, which is required by OTG-cable > >> devices that want to switch USB port into the host mode. Add USB VBUS > >> regulator properties. > >> > >> Reviewed-by: Rob Herring <robh@kernel.org> > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > > > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > Sebastian, you can pick up these patches into the power tree: > > dt-bindings: power: supply: smb347-charger: Document USB VBUS > regulator > power: supply: smb347-charger: Make smb347_set_writable() IRQ-safe > power: supply: smb347-charger: Utilize generic regmap caching > power: supply: smb347-charger: Add missing pin control activation > power: supply: smb347-charger: Implement USB VBUS regulator > > The reset of the patches could go via the Tegra tree. It's probably a > bit too late for the Tegra patches since Thierry already made 5.15 PR, > but should be fine for the power. Thanks in advance! Queued now. -- Sebastian