On 5/28/2022 2:32 AM, Stephen Boyd wrote:
HTML mail? Please send plaintext next time. Quoting Satya Priya Kakitapalli (Temp) (2022-05-27 01:24:19)On 5/21/2022 8:26 AM, Stephen Boyd wrote: Quoting Satya Priya (2022-05-20 03:49:35) diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c new file mode 100644 index 0000000..6e815c6 --- /dev/null +++ b/drivers/regulator/qcom-pm8008-regulator.c @@ -0,0 +1,225 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2022, The Linux Foundation. All rights reserved. */ + +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/mfd/qcom_pm8008.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> What in of_device.h is used? struct of_device_idThat struct is defined in mod_devicetable.h, not of_device.h+#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/regulator/driver.h> +[...]+}; + +static int pm8008_regulator_get_voltage(struct regulator_dev *rdev) +{ + struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev); + + return pm8008_reg->voltage_selector; Can this read the hardware instead of caching the value from pm8008_regulator_set_voltage()? I can use the regmap_bulk_read like below (which was present in the earlier versions)Please do+ } + + 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_matchWhy? 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.
