| Message ID | 20250211194918.2517593-3-james.a.macinnes@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add PMI8998 VBUS Regulator Support | expand |
On Tue, Feb 11, 2025 at 11:49:15AM -0800, James A. MacInnes wrote: > - .curr_table = curr_table, > - .n_current_limits = ARRAY_SIZE(curr_table), > + .curr_table = NULL, > + .n_current_limits = 0, Things that are not initialised in static variables are implicitly 0. > + // Determine PMIC type > + pmic_type = of_device_get_match_data(dev); > + if (pmic_type && strcmp(pmic_type, "pmi8998") == 0) { > + qcom_usb_vbus_rdesc.curr_table = curr_table_pmi8998; > + qcom_usb_vbus_rdesc.n_current_limits = > + ARRAY_SIZE(curr_table_pmi8998); > + } else if (pmic_type && strcmp(pmic_type, "pm8150b") == 0) { > + qcom_usb_vbus_rdesc.curr_table = curr_table_pm8150b; > + qcom_usb_vbus_rdesc.n_current_limits = > + ARRAY_SIZE(curr_table_pm8150b); > + } else { > + return -ENODEV; > + } Instead of modifying the static variable (which you had to remove const from...) the driver should take a copy of it and modify that, that way nothing goes wrong if we get two of the devices or anything. > static const struct of_device_id qcom_usb_vbus_regulator_match[] = { > - { .compatible = "qcom,pm8150b-vbus-reg" }, > + { .compatible = "qcom,pm8150b-vbus-reg", .data = "pm8150b" }, > + { .compatible = "qcom,pmi8998-vbus-reg", .data = "pmi8998" }, The driver data should be a pointer to some quirk data for the device, if you want a string to identify the device that should be a member of the quirk data struct. This will make the use of the data safer and more idiomatic.
On Tue, Feb 11, 2025 at 11:49:15AM -0800, 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@gmail.com> > --- > drivers/regulator/qcom_usb_vbus-regulator.c | 33 +++++++++++++++++---- > 1 file changed, 28 insertions(+), 5 deletions(-) > > diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c > index cd94ed67621f..bfcb77698ba2 100644 > --- a/drivers/regulator/qcom_usb_vbus-regulator.c > +++ b/drivers/regulator/qcom_usb_vbus-regulator.c > @@ -20,10 +20,15 @@ > #define OTG_CFG 0x53 > #define OTG_EN_SRC_CFG BIT(1) > > -static const unsigned int curr_table[] = { > +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, > +}; > + > static const struct regulator_ops qcom_usb_vbus_reg_ops = { > .enable = regulator_enable_regmap, > .disable = regulator_disable_regmap, > @@ -37,8 +42,8 @@ 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), > + .curr_table = NULL, > + .n_current_limits = 0, > }; > > static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) > @@ -50,6 +55,7 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) > struct regulator_init_data *init_data; > int ret; > u32 base; > + const char *pmic_type; > > ret = of_property_read_u32(dev->of_node, "reg", &base); > if (ret < 0) { > @@ -68,6 +74,19 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) > if (!init_data) > return -ENOMEM; > > + // Determine PMIC type > + pmic_type = of_device_get_match_data(dev); > + if (pmic_type && strcmp(pmic_type, "pmi8998") == 0) { I think a traditional way is to define an enum and then use that enum values as match data. Or you can just add a struct with curr_table and get that as a match data. > + qcom_usb_vbus_rdesc.curr_table = curr_table_pmi8998; > + qcom_usb_vbus_rdesc.n_current_limits = > + ARRAY_SIZE(curr_table_pmi8998); > + } else if (pmic_type && strcmp(pmic_type, "pm8150b") == 0) { > + qcom_usb_vbus_rdesc.curr_table = curr_table_pm8150b; > + qcom_usb_vbus_rdesc.n_current_limits = > + ARRAY_SIZE(curr_table_pm8150b); > + } else { > + return -ENODEV; > + } > 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 +99,22 @@ 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_info(dev, "Registered QCOM %s VBUS regulator\n", > + pmic_type); dev_dbg, the driver should be silent by default. > + > return 0; > } > > static const struct of_device_id qcom_usb_vbus_regulator_match[] = { > - { .compatible = "qcom,pm8150b-vbus-reg" }, > + { .compatible = "qcom,pm8150b-vbus-reg", .data = "pm8150b" }, > + { .compatible = "qcom,pmi8998-vbus-reg", .data = "pmi8998" }, > { } > }; > MODULE_DEVICE_TABLE(of, qcom_usb_vbus_regulator_match); > -- > 2.43.0 >
On 11/02/2025 19:49, James A. MacInnes wrote: > + // Determine PMIC type > + pmic_type = of_device_get_match_data(dev); > + if (pmic_type && strcmp(pmic_type, "pmi8998") == 0) { > + qcom_usb_vbus_rdesc.curr_table = curr_table_pmi8998; > + qcom_usb_vbus_rdesc.n_current_limits = > + ARRAY_SIZE(curr_table_pmi8998); > + } else if (pmic_type && strcmp(pmic_type, "pm8150b") == 0) { > + qcom_usb_vbus_rdesc.curr_table = curr_table_pm8150b; > + qcom_usb_vbus_rdesc.n_current_limits = > + ARRAY_SIZE(curr_table_pm8150b); > + } else { > + return -ENODEV; > + } > 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 +99,22 @@ 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_info(dev, "Registered QCOM %s VBUS regulator\n", > + pmic_type); > + > return 0; > } > > static const struct of_device_id qcom_usb_vbus_regulator_match[] = { > - { .compatible = "qcom,pm8150b-vbus-reg" }, > + { .compatible = "qcom,pm8150b-vbus-reg", .data = "pm8150b" }, > + { .compatible = "qcom,pmi8998-vbus-reg", .data = "pmi8998" }, I think the other two said much the same thing but .data should point to the differentiator instead of being a string which you disjoin on and then hook your differentiated data. i.e. .data = &my_driver_specific_static_data_here. --- bod
On Wed, Feb 12, 2025 at 01:16:37AM +0200, Dmitry Baryshkov wrote: > On Tue, Feb 11, 2025 at 11:49:15AM -0800, James A. MacInnes wrote: > > + pmic_type = of_device_get_match_data(dev); > > + if (pmic_type && strcmp(pmic_type, "pmi8998") == 0) { > I think a traditional way is to define an enum and then use that enum > values as match data. Or you can just add a struct with curr_table and > get that as a match data. Either approach works, yeah.
diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c index cd94ed67621f..bfcb77698ba2 100644 --- a/drivers/regulator/qcom_usb_vbus-regulator.c +++ b/drivers/regulator/qcom_usb_vbus-regulator.c @@ -20,10 +20,15 @@ #define OTG_CFG 0x53 #define OTG_EN_SRC_CFG BIT(1) -static const unsigned int curr_table[] = { +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, +}; + static const struct regulator_ops qcom_usb_vbus_reg_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, @@ -37,8 +42,8 @@ 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), + .curr_table = NULL, + .n_current_limits = 0, }; static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) @@ -50,6 +55,7 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) struct regulator_init_data *init_data; int ret; u32 base; + const char *pmic_type; ret = of_property_read_u32(dev->of_node, "reg", &base); if (ret < 0) { @@ -68,6 +74,19 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) if (!init_data) return -ENOMEM; + // Determine PMIC type + pmic_type = of_device_get_match_data(dev); + if (pmic_type && strcmp(pmic_type, "pmi8998") == 0) { + qcom_usb_vbus_rdesc.curr_table = curr_table_pmi8998; + qcom_usb_vbus_rdesc.n_current_limits = + ARRAY_SIZE(curr_table_pmi8998); + } else if (pmic_type && strcmp(pmic_type, "pm8150b") == 0) { + qcom_usb_vbus_rdesc.curr_table = curr_table_pm8150b; + qcom_usb_vbus_rdesc.n_current_limits = + ARRAY_SIZE(curr_table_pm8150b); + } else { + return -ENODEV; + } 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 +99,22 @@ 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_info(dev, "Registered QCOM %s VBUS regulator\n", + pmic_type); + return 0; } static const struct of_device_id qcom_usb_vbus_regulator_match[] = { - { .compatible = "qcom,pm8150b-vbus-reg" }, + { .compatible = "qcom,pm8150b-vbus-reg", .data = "pm8150b" }, + { .compatible = "qcom,pmi8998-vbus-reg", .data = "pmi8998" }, { } }; MODULE_DEVICE_TABLE(of, qcom_usb_vbus_regulator_match);
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@gmail.com> --- drivers/regulator/qcom_usb_vbus-regulator.c | 33 +++++++++++++++++---- 1 file changed, 28 insertions(+), 5 deletions(-)