On Wed, 12 Feb 2025 13:55:59 +0100
Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx> wrote:
> On 12.02.2025 2:07 AM, James A. MacInnes wrote:
> > This patch extends the Qualcomm USB VBUS regulator driver to support
> > PMI8998 PMIC alongside the existing support for PM8150B.
> >
> > Key changes:
> > - Added current limit tables specific to PMI8998.
> > - Dynamically configure the VBUS regulator based on the PMIC type.
> > - Updated debug messages to reflect successful initialization for
> > supported PMICs.
> > - Changed registration log message
> >
> > These changes ensure proper VBUS current limit configuration and
> > compatibility across multiple Qualcomm PMICs.
> >
> > Signed-off-by: James A. MacInnes <james.a.macinnes@xxxxxxxxx>
> > ---
> > drivers/regulator/qcom_usb_vbus-regulator.c | 38
> > ++++++++++++++++++--- 1 file changed, 33 insertions(+), 5
> > deletions(-)
> >
> > diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c
> > b/drivers/regulator/qcom_usb_vbus-regulator.c index
> > cd94ed67621f..804dd1a9e057 100644 ---
> > a/drivers/regulator/qcom_usb_vbus-regulator.c +++
> > b/drivers/regulator/qcom_usb_vbus-regulator.c @@ -20,10 +20,30 @@
> > #define OTG_CFG 0x53
> > #define OTG_EN_SRC_CFG BIT(1)
> >
> > -static const unsigned int curr_table[] = {
> > +struct msm_vbus_desc {
> > + const unsigned int *curr_table;
> > + unsigned int n_current_limits;
> > +};
> > +
> > +static const unsigned int curr_table_pm8150b[] = {
> > 500000, 1000000, 1500000, 2000000, 2500000, 3000000,
> > };
> >
> > +static const unsigned int curr_table_pmi8998[] = {
> > + 250000, 500000, 750000, 1000000,
> > + 1250000, 1500000, 1750000, 2000000,
> > +};
>
> To the best of my understanding these numbers are correct
>
Hopefully it is all correct. I pulled the numbers from the datasheet,
but they are known to lie.
> > +
> > +static const struct msm_vbus_desc msm_vbus_desc_pm8150b = {
> > + .curr_table = curr_table_pm8150b,
> > + .n_current_limits = ARRAY_SIZE(curr_table_pm8150b),
> > +};
> > +
> > +static const struct msm_vbus_desc msm_vbus_desc_pmi8998 = {
> > + .curr_table = curr_table_pmi8998,
> > + .n_current_limits = ARRAY_SIZE(curr_table_pmi8998),
> > +};
> > +
> > static const struct regulator_ops qcom_usb_vbus_reg_ops = {
> > .enable = regulator_enable_regmap,
> > .disable = regulator_disable_regmap,
> > @@ -37,8 +57,6 @@ static struct regulator_desc qcom_usb_vbus_rdesc
> > = { .ops = &qcom_usb_vbus_reg_ops,
> > .owner = THIS_MODULE,
> > .type = REGULATOR_VOLTAGE,
> > - .curr_table = curr_table,
> > - .n_current_limits = ARRAY_SIZE(curr_table),
> > };
> >
> > static int qcom_usb_vbus_regulator_probe(struct platform_device
> > *pdev) @@ -48,6 +66,7 @@ static int
> > qcom_usb_vbus_regulator_probe(struct platform_device *pdev) struct
> > regmap *regmap; struct regulator_config config = { };
> > struct regulator_init_data *init_data;
> > + const struct msm_vbus_desc *quirks;
>
> 'quirks' is one way to put it ;) I'd call it 'desc' or 'data' but it's
> totally a potayto/potahto discussion
>
Is there a reasonable name for that? I suspect that later chips may add
more to the structure. I am happy to change it as there is at least one
more revision for this series.
> > int ret;
> > u32 base;
> >
> > @@ -68,6 +87,12 @@ static int qcom_usb_vbus_regulator_probe(struct
> > platform_device *pdev) if (!init_data)
> > return -ENOMEM;
> >
> > + quirks = of_device_get_match_data(dev);
> > + if (!quirks)
> > + return -ENODEV;
> > +
> > + qcom_usb_vbus_rdesc.curr_table = quirks->curr_table;
> > + qcom_usb_vbus_rdesc.n_current_limits =
> > quirks->n_current_limits; qcom_usb_vbus_rdesc.enable_reg = base +
> > CMD_OTG; qcom_usb_vbus_rdesc.enable_mask = OTG_EN;
> > qcom_usb_vbus_rdesc.csel_reg = base +
> > OTG_CURRENT_LIMIT_CFG; @@ -80,18 +105,21 @@ static int
> > qcom_usb_vbus_regulator_probe(struct platform_device *pdev) rdev =
> > devm_regulator_register(dev, &qcom_usb_vbus_rdesc, &config); if
> > (IS_ERR(rdev)) { ret = PTR_ERR(rdev);
> > - dev_err(dev, "not able to register vbus reg %d\n",
> > ret);
> > + dev_err(dev, "Failed to register vbus reg %d\n",
> > ret); return ret;
> > }
> >
> > /* Disable HW logic for VBUS enable */
> > regmap_update_bits(regmap, base + OTG_CFG, OTG_EN_SRC_CFG,
> > 0);
> > + dev_dbg(dev, "Registered QCOM VBUS regulator\n");
>
> Not sure how useful this is given the previous call creates a sysfs
> entry on success, but sure
>
> Konrad
I like having a "I'm here" message so I can see when something is
loaded and to know where in the boot log it happened. Happy to remove it
if that is the standard.
James
