Re: [PATCH 2/3] regulator: qcom_usb_vbus: Add support for PMI8998 VBUS

Mark Brown
2025-02-11T20:00:52Z

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]