Re: [PATCH V13 7/9] regulator: Add a regulator driver for the PM8008 PMIC

From: Satya Priya Kakitapalli (Temp)
Date: Wed Jun 01 2022 - 07:19:08 EST



On 6/1/2022 7:36 AM, Stephen Boyd wrote:
Quoting Satya Priya Kakitapalli (Temp) (2022-05-30 03:33:47)
On 5/28/2022 2:32 AM, Stephen Boyd wrote:
Quoting Satya Priya Kakitapalli (Temp) (2022-05-27 01:24:19)
On 5/21/2022 8:26 AM, Stephen Boyd wrote:
+ }
+
+ pm8008_reg->dev = dev;
+
+ rc = of_property_read_string(dev->of_node, "regulator-name", &name);
+ if (rc)
+ return rc;
+
+ /* get the required regulator data */
+ for (i = 0; i < ARRAY_SIZE(reg_data); i++)
+ if (strstr(name, reg_data[i].name))

Why not find this via reg/address instead? It would save storing the
regulator name in the reg_data table.


You mean match this using base address? then we should add base address in the
reg_data table. We will need the name to be stored in reg_data table anyway for
the pm8008_reg->rdesc.of_match
Why? Now that this driver binds to each node individually the usage of
of_match doesn't make any sense to me. Can you set 'struct
regulator_config::dev' instead and not set of_match?

Currently we are setting regulator_config::dev as dev->parent i.e.,
pm8008@8, because the parent supplies are present under pm8008@8, to get
the regulators mapped correctly to the parent supplies we are using
dev->parent.

If we do not set of_match in regulator descriptor,
regulator_of_get_init_node() would return NULL, causing init_data to be
NULL during regulator_register and regulators are not getting probed.
This can be resolved, if we get the init_data during pm8008_probe
itself. I'll do that in the next version.

Ok then it seems ok to leave it as is. I suspect getting init data
during probe is more code vs. having the node name and the address in
the table.


Okay. If we are leaving it as is then, there is no need to add address in the table.