For no apparent reason (as there's just one RPM per SoC), all vregs
currently store a copy of a pointer to smd_rpm. Introduce a single,
global one to save up on space in each definition.
bloat-o-meter reports:
Total: Before=43944, After=43924, chg -0.05%
plus sizeof(ptr) on every dynamically allocated regulator :)
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/regulator/qcom_smd-regulator.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/regulator/qcom_smd-regulator.c b/drivers/regulator/qcom_smd-regulator.c
index d1be9568025e..905c15df8c85 100644
--- a/drivers/regulator/qcom_smd-regulator.c
+++ b/drivers/regulator/qcom_smd-regulator.c
@@ -11,11 +11,10 @@
#include <linux/regulator/of_regulator.h>
#include <linux/soc/qcom/smd-rpm.h>
+struct qcom_smd_rpm *smd_vreg_rpm;
+
struct qcom_rpm_reg {
struct device *dev;
-
- struct qcom_smd_rpm *rpm;
-
u32 type;
u32 id;
@@ -70,7 +69,7 @@ static int rpm_reg_write_active(struct qcom_rpm_reg *vreg)
if (!reqlen)
return 0;
- ret = qcom_rpm_smd_write(vreg->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
+ ret = qcom_rpm_smd_write(smd_vreg_rpm, QCOM_SMD_RPM_ACTIVE_STATE,
vreg->type, vreg->id,
req, sizeof(req[0]) * reqlen);
if (!ret) {
@@ -1391,7 +1390,7 @@ MODULE_DEVICE_TABLE(of, rpm_of_match);
* Return: 0 on success, errno on failure
*/
static int rpm_regulator_init_vreg(struct qcom_rpm_reg *vreg, struct device *dev,
- struct device_node *node, struct qcom_smd_rpm *rpm,
+ struct device_node *node,
const struct rpm_regulator_data *pmic_rpm_data)
{
struct regulator_config config = {};
@@ -1409,7 +1408,6 @@ static int rpm_regulator_init_vreg(struct qcom_rpm_reg *vreg, struct device *dev
}
vreg->dev = dev;
- vreg->rpm = rpm;
vreg->type = rpm_data->type;
vreg->id = rpm_data->id;
@@ -1440,11 +1438,10 @@ static int rpm_reg_probe(struct platform_device *pdev)
const struct rpm_regulator_data *vreg_data;
struct device_node *node;
struct qcom_rpm_reg *vreg;
- struct qcom_smd_rpm *rpm;
int ret;
- rpm = dev_get_drvdata(pdev->dev.parent);
- if (!rpm) {
+ smd_vreg_rpm = dev_get_drvdata(pdev->dev.parent);
+ if (!smd_vreg_rpm) {
dev_err(&pdev->dev, "Unable to retrieve handle to rpm\n");
return -ENODEV;
}
@@ -1460,8 +1457,7 @@ static int rpm_reg_probe(struct platform_device *pdev)
return -ENOMEM;
}
- ret = rpm_regulator_init_vreg(vreg, dev, node, rpm, vreg_data);
-
+ ret = rpm_regulator_init_vreg(vreg, dev, node, vreg_data);
if (ret < 0) {
of_node_put(node);
return ret;
---
base-commit: 39676dfe52331dba909c617f213fdb21015c8d10
change-id: 20231227-topic-rpm_vreg_cleanup-fa095cd528ec
Best regards,
--
Konrad Dybcio <[email protected]>
Hi Konrad,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 39676dfe52331dba909c617f213fdb21015c8d10]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/regulator-qcom_smd-Keep-one-rpm-handle-for-all-vregs/20231227-093153
base: 39676dfe52331dba909c617f213fdb21015c8d10
patch link: https://lore.kernel.org/r/20231227-topic-rpm_vreg_cleanup-v1-1-949da0864ac5%40linaro.org
patch subject: [PATCH] regulator: qcom_smd: Keep one rpm handle for all vregs
config: arc-randconfig-001-20231227 (https://download.01.org/0day-ci/archive/20231227/[email protected]/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231227/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/regulator/qcom_smd-regulator.c:1395: warning: Excess function parameter 'rpm' description in 'rpm_regulator_init_vreg'
vim +1395 drivers/regulator/qcom_smd-regulator.c
da65e367b67e15 Bjorn Andersson 2015-08-17 1379
14e2976fbabdac Konrad Dybcio 2021-12-30 1380 /**
14e2976fbabdac Konrad Dybcio 2021-12-30 1381 * rpm_regulator_init_vreg() - initialize all attributes of a qcom_smd-regulator
14e2976fbabdac Konrad Dybcio 2021-12-30 1382 * @vreg: Pointer to the individual qcom_smd-regulator resource
14e2976fbabdac Konrad Dybcio 2021-12-30 1383 * @dev: Pointer to the top level qcom_smd-regulator PMIC device
14e2976fbabdac Konrad Dybcio 2021-12-30 1384 * @node: Pointer to the individual qcom_smd-regulator resource
14e2976fbabdac Konrad Dybcio 2021-12-30 1385 * device node
14e2976fbabdac Konrad Dybcio 2021-12-30 1386 * @rpm: Pointer to the rpm bus node
14e2976fbabdac Konrad Dybcio 2021-12-30 1387 * @pmic_rpm_data: Pointer to a null-terminated array of qcom_smd-regulator
14e2976fbabdac Konrad Dybcio 2021-12-30 1388 * resources defined for the top level PMIC device
14e2976fbabdac Konrad Dybcio 2021-12-30 1389 *
14e2976fbabdac Konrad Dybcio 2021-12-30 1390 * Return: 0 on success, errno on failure
14e2976fbabdac Konrad Dybcio 2021-12-30 1391 */
14e2976fbabdac Konrad Dybcio 2021-12-30 1392 static int rpm_regulator_init_vreg(struct qcom_rpm_reg *vreg, struct device *dev,
d7cdd450e31088 Konrad Dybcio 2023-12-27 1393 struct device_node *node,
14e2976fbabdac Konrad Dybcio 2021-12-30 1394 const struct rpm_regulator_data *pmic_rpm_data)
da65e367b67e15 Bjorn Andersson 2015-08-17 @1395 {
da65e367b67e15 Bjorn Andersson 2015-08-17 1396 struct regulator_config config = {};
14e2976fbabdac Konrad Dybcio 2021-12-30 1397 const struct rpm_regulator_data *rpm_data;
da65e367b67e15 Bjorn Andersson 2015-08-17 1398 struct regulator_dev *rdev;
14e2976fbabdac Konrad Dybcio 2021-12-30 1399 int ret;
14e2976fbabdac Konrad Dybcio 2021-12-30 1400
14e2976fbabdac Konrad Dybcio 2021-12-30 1401 for (rpm_data = pmic_rpm_data; rpm_data->name; rpm_data++)
14e2976fbabdac Konrad Dybcio 2021-12-30 1402 if (of_node_name_eq(node, rpm_data->name))
14e2976fbabdac Konrad Dybcio 2021-12-30 1403 break;
14e2976fbabdac Konrad Dybcio 2021-12-30 1404
14e2976fbabdac Konrad Dybcio 2021-12-30 1405 if (!rpm_data->name) {
14e2976fbabdac Konrad Dybcio 2021-12-30 1406 dev_err(dev, "Unknown regulator %pOFn\n", node);
14e2976fbabdac Konrad Dybcio 2021-12-30 1407 return -EINVAL;
14e2976fbabdac Konrad Dybcio 2021-12-30 1408 }
14e2976fbabdac Konrad Dybcio 2021-12-30 1409
14e2976fbabdac Konrad Dybcio 2021-12-30 1410 vreg->dev = dev;
14e2976fbabdac Konrad Dybcio 2021-12-30 1411 vreg->type = rpm_data->type;
14e2976fbabdac Konrad Dybcio 2021-12-30 1412 vreg->id = rpm_data->id;
14e2976fbabdac Konrad Dybcio 2021-12-30 1413
14e2976fbabdac Konrad Dybcio 2021-12-30 1414 memcpy(&vreg->desc, rpm_data->desc, sizeof(vreg->desc));
14e2976fbabdac Konrad Dybcio 2021-12-30 1415 vreg->desc.name = rpm_data->name;
14e2976fbabdac Konrad Dybcio 2021-12-30 1416 vreg->desc.supply_name = rpm_data->supply;
14e2976fbabdac Konrad Dybcio 2021-12-30 1417 vreg->desc.owner = THIS_MODULE;
14e2976fbabdac Konrad Dybcio 2021-12-30 1418 vreg->desc.type = REGULATOR_VOLTAGE;
14e2976fbabdac Konrad Dybcio 2021-12-30 1419 vreg->desc.of_match = rpm_data->name;
14e2976fbabdac Konrad Dybcio 2021-12-30 1420
14e2976fbabdac Konrad Dybcio 2021-12-30 1421 config.dev = dev;
14e2976fbabdac Konrad Dybcio 2021-12-30 1422 config.of_node = node;
14e2976fbabdac Konrad Dybcio 2021-12-30 1423 config.driver_data = vreg;
14e2976fbabdac Konrad Dybcio 2021-12-30 1424
14e2976fbabdac Konrad Dybcio 2021-12-30 1425 rdev = devm_regulator_register(dev, &vreg->desc, &config);
14e2976fbabdac Konrad Dybcio 2021-12-30 1426 if (IS_ERR(rdev)) {
14e2976fbabdac Konrad Dybcio 2021-12-30 1427 ret = PTR_ERR(rdev);
14e2976fbabdac Konrad Dybcio 2021-12-30 1428 dev_err(dev, "%pOFn: devm_regulator_register() failed, ret=%d\n", node, ret);
14e2976fbabdac Konrad Dybcio 2021-12-30 1429 return ret;
14e2976fbabdac Konrad Dybcio 2021-12-30 1430 }
14e2976fbabdac Konrad Dybcio 2021-12-30 1431
14e2976fbabdac Konrad Dybcio 2021-12-30 1432 return 0;
14e2976fbabdac Konrad Dybcio 2021-12-30 1433 }
14e2976fbabdac Konrad Dybcio 2021-12-30 1434
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 27/12/2023 03:29, Konrad Dybcio wrote:
> For no apparent reason (as there's just one RPM per SoC), all vregs
> currently store a copy of a pointer to smd_rpm. Introduce a single,
> global one to save up on space in each definition.
>
> bloat-o-meter reports:
>
> Total: Before=43944, After=43924, chg -0.05%
>
> plus sizeof(ptr) on every dynamically allocated regulator :)
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/regulator/qcom_smd-regulator.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/regulator/qcom_smd-regulator.c b/drivers/regulator/qcom_smd-regulator.c
> index d1be9568025e..905c15df8c85 100644
> --- a/drivers/regulator/qcom_smd-regulator.c
> +++ b/drivers/regulator/qcom_smd-regulator.c
> @@ -11,11 +11,10 @@
> #include <linux/regulator/of_regulator.h>
> #include <linux/soc/qcom/smd-rpm.h>
>
> +struct qcom_smd_rpm *smd_vreg_rpm;
> +
> struct qcom_rpm_reg {
> struct device *dev;
> -
> - struct qcom_smd_rpm *rpm;
> -
> u32 type;
> u32 id;
>
> @@ -70,7 +69,7 @@ static int rpm_reg_write_active(struct qcom_rpm_reg *vreg)
> if (!reqlen)
> return 0;
>
> - ret = qcom_rpm_smd_write(vreg->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
> + ret = qcom_rpm_smd_write(smd_vreg_rpm, QCOM_SMD_RPM_ACTIVE_STATE,
> vreg->type, vreg->id,
> req, sizeof(req[0]) * reqlen);
> if (!ret) {
> @@ -1391,7 +1390,7 @@ MODULE_DEVICE_TABLE(of, rpm_of_match);
> * Return: 0 on success, errno on failure
> */
> static int rpm_regulator_init_vreg(struct qcom_rpm_reg *vreg, struct device *dev,
> - struct device_node *node, struct qcom_smd_rpm *rpm,
> + struct device_node *node,
> const struct rpm_regulator_data *pmic_rpm_data)
> {
> struct regulator_config config = {};
> @@ -1409,7 +1408,6 @@ static int rpm_regulator_init_vreg(struct qcom_rpm_reg *vreg, struct device *dev
> }
>
> vreg->dev = dev;
> - vreg->rpm = rpm;
> vreg->type = rpm_data->type;
> vreg->id = rpm_data->id;
>
> @@ -1440,11 +1438,10 @@ static int rpm_reg_probe(struct platform_device *pdev)
> const struct rpm_regulator_data *vreg_data;
> struct device_node *node;
> struct qcom_rpm_reg *vreg;
> - struct qcom_smd_rpm *rpm;
> int ret;
>
> - rpm = dev_get_drvdata(pdev->dev.parent);
> - if (!rpm) {
> + smd_vreg_rpm = dev_get_drvdata(pdev->dev.parent);
> + if (!smd_vreg_rpm) {
I thought about having a mutex around (I don't remember if secondary
PMICs and/or chargers can be routed through RPM or not).
Then I went on checking other RPM and SMD-RPM drivers.
clk-rpm: global variable, field
clk-smd-rpm: struct field
regulator_qcom-smd-rpm: struct field
Probably it's worth using the same approach in all four drivers?
> dev_err(&pdev->dev, "Unable to retrieve handle to rpm\n");
> return -ENODEV;
> }
> @@ -1460,8 +1457,7 @@ static int rpm_reg_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - ret = rpm_regulator_init_vreg(vreg, dev, node, rpm, vreg_data);
> -
> + ret = rpm_regulator_init_vreg(vreg, dev, node, vreg_data);
> if (ret < 0) {
> of_node_put(node);
> return ret;
>
> ---
> base-commit: 39676dfe52331dba909c617f213fdb21015c8d10
> change-id: 20231227-topic-rpm_vreg_cleanup-fa095cd528ec
>
> Best regards,
--
With best wishes
Dmitry
On 27.12.2023 12:48, Dmitry Baryshkov wrote:
> On 27/12/2023 03:29, Konrad Dybcio wrote:
>> For no apparent reason (as there's just one RPM per SoC), all vregs
>> currently store a copy of a pointer to smd_rpm. Introduce a single,
>> global one to save up on space in each definition.
>>
>> bloat-o-meter reports:
>>
>> Total: Before=43944, After=43924, chg -0.05%
>>
>> plus sizeof(ptr) on every dynamically allocated regulator :)
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
[...]
>> @@ -1440,11 +1438,10 @@ static int rpm_reg_probe(struct platform_device *pdev)
>> const struct rpm_regulator_data *vreg_data;
>> struct device_node *node;
>> struct qcom_rpm_reg *vreg;
>> - struct qcom_smd_rpm *rpm;
>> int ret;
>> - rpm = dev_get_drvdata(pdev->dev.parent);
>> - if (!rpm) {
>> + smd_vreg_rpm = dev_get_drvdata(pdev->dev.parent);
>> + if (!smd_vreg_rpm) {
>
> I thought about having a mutex around (I don't remember if secondary PMICs and/or chargers can be routed through RPM or not).
A mutex for assigning this?
Konrad
>
> Then I went on checking other RPM and SMD-RPM drivers.
>
> clk-rpm: global variable, field
> clk-smd-rpm: struct field
> regulator_qcom-smd-rpm: struct field
>
> Probably it's worth using the same approach in all four drivers?
On Wed, 3 Jan 2024 at 12:03, Konrad Dybcio <[email protected]> wrote:
>
> On 27.12.2023 12:48, Dmitry Baryshkov wrote:
> > On 27/12/2023 03:29, Konrad Dybcio wrote:
> >> For no apparent reason (as there's just one RPM per SoC), all vregs
> >> currently store a copy of a pointer to smd_rpm. Introduce a single,
> >> global one to save up on space in each definition.
> >>
> >> bloat-o-meter reports:
> >>
> >> Total: Before=43944, After=43924, chg -0.05%
> >>
> >> plus sizeof(ptr) on every dynamically allocated regulator :)
> >>
> >> Signed-off-by: Konrad Dybcio <[email protected]>
> >> ---
>
> [...]
>
> >> @@ -1440,11 +1438,10 @@ static int rpm_reg_probe(struct platform_device *pdev)
> >> const struct rpm_regulator_data *vreg_data;
> >> struct device_node *node;
> >> struct qcom_rpm_reg *vreg;
> >> - struct qcom_smd_rpm *rpm;
> >> int ret;
> >> - rpm = dev_get_drvdata(pdev->dev.parent);
> >> - if (!rpm) {
> >> + smd_vreg_rpm = dev_get_drvdata(pdev->dev.parent);
> >> + if (!smd_vreg_rpm) {
> >
> > I thought about having a mutex around (I don't remember if secondary PMICs and/or chargers can be routed through RPM or not).
>
> A mutex for assigning this?
Yep.
>
> Konrad
> >
> > Then I went on checking other RPM and SMD-RPM drivers.
> >
> > clk-rpm: global variable, field
> > clk-smd-rpm: struct field
> > regulator_qcom-smd-rpm: struct field
> >
> > Probably it's worth using the same approach in all four drivers?
>
>
--
With best wishes
Dmitry