| Message ID | 1644331940-18986-4-git-send-email-quic_c_skakit@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Qualcomm Technologies, Inc. PM8008 regulator driver | expand |
On 2/10/2022 7:02 AM, Stephen Boyd wrote: > Quoting Satya Priya (2022-02-08 06:52:17) >> diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c >> index c472d7f..e8569cc 100644 >> --- a/drivers/mfd/qcom-pm8008.c >> +++ b/drivers/mfd/qcom-pm8008.c >> @@ -8,6 +8,7 @@ >> #include <linux/interrupt.h> >> #include <linux/irq.h> >> #include <linux/irqdomain.h> >> +#include <linux/mfd/core.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> #include <linux/of_platform.h> >> @@ -27,6 +28,37 @@ >> #define INT_EN_CLR_OFFSET 0x16 >> #define INT_LATCHED_STS_OFFSET 0x18 >> >> +static const struct mfd_cell pm8008_regulator_devs[] = { > Is there some way to not allocate this structure statically forever? I think No. I found that some of the drivers are just using one cell with .name to match with regulator driver and then probing regulators using a loop. I'll do that too. static const struct mfd_cell pm8008_regulator_devs[] = { { .name = "qcom,pm8008-regulators", }, }; >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 0, >> + }, >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 1, >> + }, >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 2, >> + }, >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 3, >> + }, >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 4, >> + }, >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 5, >> + }, >> + { >> + .name = "qcom,pm8008-regulators", >> + .id = 6, >> + }, >> +}; >> + >> enum { >> PM8008_MISC, >> pm8008_temp_alarm, >> @@ -35,6 +67,17 @@ enum { >> PM8008_NUM_PERIPHS, >> }; >> >> +enum { >> + PM8008_INFRA, >> + PM8008_REGULATORS, >> +}; >> + >> +static const struct of_device_id pm8008_match[] = { >> + { .compatible = "qcom,pm8008", .data = (void *)PM8008_INFRA}, >> + { .compatible = "qcom,pm8008-regulators", .data = (void *)PM8008_REGULATORS}, >> + { }, > Nitpick: Drop , on {} so nothing can come after without causing compile > error. Okay. >> +}; >> + >> #define PM8008_PERIPH_0_BASE 0x900 >> #define PM8008_PERIPH_1_BASE 0x2400 >> #define PM8008_PERIPH_2_BASE 0xC000 >> @@ -221,6 +264,7 @@ static int pm8008_probe(struct i2c_client *client) >> { >> int rc; >> struct pm8008_data *chip; >> + const struct of_device_id *id; >> >> chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); >> if (!chip) >> @@ -239,14 +283,19 @@ static int pm8008_probe(struct i2c_client *client) >> dev_err(chip->dev, "Failed to probe irq periphs: %d\n", rc); >> } >> >> + id = of_match_node(pm8008_match, chip->dev->of_node); > Use device_get_match_data()? And then use (uintptr_t) casts to check for > the enum? Using device_get_match_data() allows us to avoid moving the > pm8008_match table. Okay. >> + if (id->data == (void *)PM8008_REGULATORS) { > enum <your_name_here> dev_type = device_get_match_data(dev); > > if (dev_type == PM8008_REGULATORS) > >> + rc = mfd_add_devices(chip->dev, 0, pm8008_regulator_devs, > Why not devm_mfd_add_devices()? Okay. >> + ARRAY_SIZE(pm8008_regulator_devs), NULL, 0, NULL); >> + if (rc) { >> + dev_err(chip->dev, "Failed to add children: %d\n", rc); >> + return rc; >> + } >> + } >> + >> return devm_of_platform_populate(chip->dev); >> } >> >> -static const struct of_device_id pm8008_match[] = { >> - { .compatible = "qcom,pm8008", }, >> - { }, >> -}; > This should have a MODULE_DEVICE_TABLE(of, pm8008_match) here. Okay. >> - >> static struct i2c_driver pm8008_mfd_driver = { >> .driver = { >> .name = "pm8008",
On Fri, 11 Feb 2022, Satya Priya Kakitapalli (Temp) wrote: > > On 2/10/2022 7:02 AM, Stephen Boyd wrote: > > Quoting Satya Priya (2022-02-08 06:52:17) > > > diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c > > > index c472d7f..e8569cc 100644 > > > --- a/drivers/mfd/qcom-pm8008.c > > > +++ b/drivers/mfd/qcom-pm8008.c > > > @@ -8,6 +8,7 @@ > > > #include <linux/interrupt.h> > > > #include <linux/irq.h> > > > #include <linux/irqdomain.h> > > > +#include <linux/mfd/core.h> > > > #include <linux/module.h> > > > #include <linux/of_device.h> > > > #include <linux/of_platform.h> > > > @@ -27,6 +28,37 @@ > > > #define INT_EN_CLR_OFFSET 0x16 > > > #define INT_LATCHED_STS_OFFSET 0x18 > > > > > > +static const struct mfd_cell pm8008_regulator_devs[] = { > > Is there some way to not allocate this structure statically forever? > > > I think No. > > I found that some of the drivers are just using one cell with .name to match > with regulator driver and then probing regulators using a loop. I'll do that > too. > > static const struct mfd_cell pm8008_regulator_devs[] = { > { > .name = "qcom,pm8008-regulators", > }, > }; Please use MFD_CELL_NAME() for these.
Hi, On 2/11/2022 7:04 PM, Lee Jones wrote: > On Fri, 11 Feb 2022, Satya Priya Kakitapalli (Temp) wrote: > >> On 2/10/2022 7:02 AM, Stephen Boyd wrote: >>> Quoting Satya Priya (2022-02-08 06:52:17) >>>> diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c >>>> index c472d7f..e8569cc 100644 >>>> --- a/drivers/mfd/qcom-pm8008.c >>>> +++ b/drivers/mfd/qcom-pm8008.c >>>> @@ -8,6 +8,7 @@ >>>> #include <linux/interrupt.h> >>>> #include <linux/irq.h> >>>> #include <linux/irqdomain.h> >>>> +#include <linux/mfd/core.h> >>>> #include <linux/module.h> >>>> #include <linux/of_device.h> >>>> #include <linux/of_platform.h> >>>> @@ -27,6 +28,37 @@ >>>> #define INT_EN_CLR_OFFSET 0x16 >>>> #define INT_LATCHED_STS_OFFSET 0x18 >>>> >>>> +static const struct mfd_cell pm8008_regulator_devs[] = { >>> Is there some way to not allocate this structure statically forever? >> >> I think No. >> >> I found that some of the drivers are just using one cell with .name to match >> with regulator driver and then probing regulators using a loop. I'll do that >> too. >> >> static const struct mfd_cell pm8008_regulator_devs[] = { >> { >> .name = "qcom,pm8008-regulators", >> }, >> }; > Please use MFD_CELL_NAME() for these. Okay.
diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c index c472d7f..e8569cc 100644 --- a/drivers/mfd/qcom-pm8008.c +++ b/drivers/mfd/qcom-pm8008.c @@ -8,6 +8,7 @@ #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/mfd/core.h> #include <linux/module.h> #include <linux/of_device.h> #include <linux/of_platform.h> @@ -27,6 +28,37 @@ #define INT_EN_CLR_OFFSET 0x16 #define INT_LATCHED_STS_OFFSET 0x18 +static const struct mfd_cell pm8008_regulator_devs[] = { + { + .name = "qcom,pm8008-regulators", + .id = 0, + }, + { + .name = "qcom,pm8008-regulators", + .id = 1, + }, + { + .name = "qcom,pm8008-regulators", + .id = 2, + }, + { + .name = "qcom,pm8008-regulators", + .id = 3, + }, + { + .name = "qcom,pm8008-regulators", + .id = 4, + }, + { + .name = "qcom,pm8008-regulators", + .id = 5, + }, + { + .name = "qcom,pm8008-regulators", + .id = 6, + }, +}; + enum { PM8008_MISC, PM8008_TEMP_ALARM, @@ -35,6 +67,17 @@ enum { PM8008_NUM_PERIPHS, }; +enum { + PM8008_INFRA, + PM8008_REGULATORS, +}; + +static const struct of_device_id pm8008_match[] = { + { .compatible = "qcom,pm8008", .data = (void *)PM8008_INFRA}, + { .compatible = "qcom,pm8008-regulators", .data = (void *)PM8008_REGULATORS}, + { }, +}; + #define PM8008_PERIPH_0_BASE 0x900 #define PM8008_PERIPH_1_BASE 0x2400 #define PM8008_PERIPH_2_BASE 0xC000 @@ -221,6 +264,7 @@ static int pm8008_probe(struct i2c_client *client) { int rc; struct pm8008_data *chip; + const struct of_device_id *id; chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); if (!chip) @@ -239,14 +283,19 @@ static int pm8008_probe(struct i2c_client *client) dev_err(chip->dev, "Failed to probe irq periphs: %d\n", rc); } + id = of_match_node(pm8008_match, chip->dev->of_node); + if (id->data == (void *)PM8008_REGULATORS) { + rc = mfd_add_devices(chip->dev, 0, pm8008_regulator_devs, + ARRAY_SIZE(pm8008_regulator_devs), NULL, 0, NULL); + if (rc) { + dev_err(chip->dev, "Failed to add children: %d\n", rc); + return rc; + } + } + return devm_of_platform_populate(chip->dev); } -static const struct of_device_id pm8008_match[] = { - { .compatible = "qcom,pm8008", }, - { }, -}; - static struct i2c_driver pm8008_mfd_driver = { .driver = { .name = "pm8008",
Register mfd cell ID and name for each of the 7 pm8008 LDOs to probe them through mfd driver without needing a separate compatible for regulator driver. Also, add a different compatible for the mfd node that contains regulators to make sure that the LDOs are registered with the correct mfd device. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- Changes in V5: - Changes newly added from V5. drivers/mfd/qcom-pm8008.c | 59 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-)