diff mbox series

[v1,5/6] regulator: mtk-dvfsrc-regulator: Refactor and add MT8192/MT8195 vregs

Message ID 20240416153805.431118-6-angelogioacchino.delregno@collabora.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series MediaTek DVFSRC Bus Bandwidth and Regulator knobs | expand

Commit Message

AngeloGioacchino Del Regno April 16, 2024, 3:38 p.m. UTC
This driver never worked, and never got even compiled, because it was
missing the DVFSRC driver entirely, including headers it relies on!

Perform a full (or nearly full) refactoring of the MediaTek DVFSRC
controlled Regulators driver: this retains support for the MT6873,
MT8183 and MT8192 SoCs, and adds MT8195.

As part of the refactoring, this driver is now probed using its own
devicetree compatible, as this is a child of the main DVFSRC driver
and gets probed as a subnode of that.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/regulator/mtk-dvfsrc-regulator.c | 242 +++++++++++------------
 1 file changed, 112 insertions(+), 130 deletions(-)

Comments

Mark Brown April 17, 2024, 12:52 a.m. UTC | #1
On Tue, Apr 16, 2024 at 05:38:04PM +0200, AngeloGioacchino Del Regno wrote:
> This driver never worked, and never got even compiled, because it was
> missing the DVFSRC driver entirely, including headers it relies on!
> 
> Perform a full (or nearly full) refactoring of the MediaTek DVFSRC
> controlled Regulators driver: this retains support for the MT6873,
> MT8183 and MT8192 SoCs, and adds MT8195.
> 
> As part of the refactoring, this driver is now probed using its own
> devicetree compatible, as this is a child of the main DVFSRC driver
> and gets probed as a subnode of that.

Given what you're doing this would probably be easier to review as
two commits, one removing the existing driver and another adding the new
code.
AngeloGioacchino Del Regno April 17, 2024, 8:30 a.m. UTC | #2
Il 17/04/24 02:52, Mark Brown ha scritto:
> On Tue, Apr 16, 2024 at 05:38:04PM +0200, AngeloGioacchino Del Regno wrote:
>> This driver never worked, and never got even compiled, because it was
>> missing the DVFSRC driver entirely, including headers it relies on!
>>
>> Perform a full (or nearly full) refactoring of the MediaTek DVFSRC
>> controlled Regulators driver: this retains support for the MT6873,
>> MT8183 and MT8192 SoCs, and adds MT8195.
>>
>> As part of the refactoring, this driver is now probed using its own
>> devicetree compatible, as this is a child of the main DVFSRC driver
>> and gets probed as a subnode of that.
> 
> Given what you're doing this would probably be easier to review as
> two commits, one removing the existing driver and another adding the new
> code.

Yeah, definitely.... I'll do that for v2.

Cheers,
Angelo
diff mbox series

Patch

diff --git a/drivers/regulator/mtk-dvfsrc-regulator.c b/drivers/regulator/mtk-dvfsrc-regulator.c
index f1280d45265d..9d398d72d2fb 100644
--- a/drivers/regulator/mtk-dvfsrc-regulator.c
+++ b/drivers/regulator/mtk-dvfsrc-regulator.c
@@ -1,99 +1,94 @@ 
 // SPDX-License-Identifier: GPL-2.0
-//
-// Copyright (c) 2020 MediaTek Inc.
+/*
+ * Copyright (C) 2020 MediaTek Inc.
+ * Copyright (c) 2024 Collabora Ltd.
+ *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
 
-#include <linux/err.h>
-#include <linux/init.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/of_.h>
+#include <linux/of.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
-#include <linux/soc/mediatek/mtk_dvfsrc.h>
-
-#define DVFSRC_ID_VCORE		0
-#define DVFSRC_ID_VSCP		1
-
-#define MT_DVFSRC_REGULAR(match, _name,	_volt_table)	\
-[DVFSRC_ID_##_name] = {					\
-	.desc = {					\
-		.name = match,				\
-		.of_match = of_match_ptr(match),	\
-		.ops = &dvfsrc_vcore_ops,		\
-		.type = REGULATOR_VOLTAGE,		\
-		.id = DVFSRC_ID_##_name,		\
-		.owner = THIS_MODULE,			\
-		.n_voltages = ARRAY_SIZE(_volt_table),	\
-		.volt_table = _volt_table,		\
-	},	\
-}
-
-/*
- * DVFSRC regulators' information
- *
- * @desc: standard fields of regulator description.
- * @voltage_selector:  Selector used for get_voltage_sel() and
- *			   set_voltage_sel() callbacks
- */
+#include <linux/soc/mediatek/dvfsrc.h>
 
-struct dvfsrc_regulator {
-	struct regulator_desc	desc;
+enum dvfsrc_regulator_id {
+	DVFSRC_ID_VCORE,
+	DVFSRC_ID_VSCP,
+	DVFSRC_ID_MAX
 };
 
-/*
- * MTK DVFSRC regulators' init data
- *
- * @size: num of regulators
- * @regulator_info: regulator info.
- */
-struct dvfsrc_regulator_init_data {
+struct dvfsrc_regulator_pdata {
+	struct regulator_desc *descs;
 	u32 size;
-	struct dvfsrc_regulator *regulator_info;
 };
 
-static inline struct device *to_dvfsrc_dev(struct regulator_dev *rdev)
+#define MTK_DVFSRC_VREG(match, _name, _volt_table)	\
+{							\
+	.name = match,					\
+	.of_match = match,				\
+	.ops = &dvfsrc_vcore_ops,			\
+	.type = REGULATOR_VOLTAGE,			\
+	.id = DVFSRC_ID_##_name,			\
+	.owner = THIS_MODULE,				\
+	.n_voltages = ARRAY_SIZE(_volt_table),		\
+	.volt_table = _volt_table,			\
+}
+
+static inline struct device *to_dvfs_regulator_dev(struct regulator_dev *rdev)
 {
 	return rdev_get_dev(rdev)->parent;
 }
 
+static inline struct device *to_dvfsrc_dev(struct regulator_dev *rdev)
+{
+	return to_dvfs_regulator_dev(rdev)->parent;
+}
+
+static int dvfsrc_get_cmd(int rdev_id, enum mtk_dvfsrc_cmd *cmd)
+{
+	switch (rdev_id) {
+	case DVFSRC_ID_VCORE:
+		*cmd = MTK_DVFSRC_CMD_VCORE_LEVEL;
+		break;
+	case DVFSRC_ID_VSCP:
+		*cmd = MTK_DVFSRC_CMD_VSCP_LEVEL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int dvfsrc_set_voltage_sel(struct regulator_dev *rdev,
 				  unsigned int selector)
 {
 	struct device *dvfsrc_dev = to_dvfsrc_dev(rdev);
+	enum mtk_dvfsrc_cmd req_cmd;
 	int id = rdev_get_id(rdev);
+	int ret;
 
-	if (id == DVFSRC_ID_VCORE)
-		mtk_dvfsrc_send_request(dvfsrc_dev,
-					MTK_DVFSRC_CMD_VCORE_REQUEST,
-					selector);
-	else if (id == DVFSRC_ID_VSCP)
-		mtk_dvfsrc_send_request(dvfsrc_dev,
-					MTK_DVFSRC_CMD_VSCP_REQUEST,
-					selector);
-	else
-		return -EINVAL;
+	ret = dvfsrc_get_cmd(id, &req_cmd);
+	if (ret)
+		return ret;
 
-	return 0;
+	return mtk_dvfsrc_send_request(dvfsrc_dev, req_cmd, selector);
 }
 
 static int dvfsrc_get_voltage_sel(struct regulator_dev *rdev)
 {
 	struct device *dvfsrc_dev = to_dvfsrc_dev(rdev);
+	enum mtk_dvfsrc_cmd query_cmd;
 	int id = rdev_get_id(rdev);
 	int val, ret;
 
-	if (id == DVFSRC_ID_VCORE)
-		ret = mtk_dvfsrc_query_info(dvfsrc_dev,
-					    MTK_DVFSRC_CMD_VCORE_LEVEL_QUERY,
-					    &val);
-	else if (id == DVFSRC_ID_VSCP)
-		ret = mtk_dvfsrc_query_info(dvfsrc_dev,
-					    MTK_DVFSRC_CMD_VSCP_LEVEL_QUERY,
-					    &val);
-	else
-		return -EINVAL;
+	ret = dvfsrc_get_cmd(id, &query_cmd);
+	if (ret)
+		return ret;
 
-	if (ret != 0)
+	ret = mtk_dvfsrc_query_info(dvfsrc_dev, query_cmd, &val);
+	if (ret)
 		return ret;
 
 	return val;
@@ -105,84 +100,71 @@  static const struct regulator_ops dvfsrc_vcore_ops = {
 	.set_voltage_sel = dvfsrc_set_voltage_sel,
 };
 
+static const unsigned int mt6873_voltages[] = {
+	575000,
+	600000,
+	650000,
+	725000,
+};
+
+static struct regulator_desc mt6873_regulators[] = {
+	MTK_DVFSRC_VREG("dvfsrc-vcore", VCORE, mt6873_voltages),
+	MTK_DVFSRC_VREG("dvfsrc-vscp", VSCP, mt6873_voltages),
+};
+
+static const struct dvfsrc_regulator_pdata mt6873_data = {
+	.descs = mt6873_regulators,
+	.size = ARRAY_SIZE(mt6873_regulators),
+};
+
 static const unsigned int mt8183_voltages[] = {
 	725000,
 	800000,
 };
 
-static struct dvfsrc_regulator mt8183_regulators[] = {
-	MT_DVFSRC_REGULAR("dvfsrc-vcore", VCORE,
-			  mt8183_voltages),
+static struct regulator_desc mt8183_regulators[] = {
+	MTK_DVFSRC_VREG("dvfsrc-vcore", VCORE, mt8183_voltages),
 };
 
-static const struct dvfsrc_regulator_init_data regulator_mt8183_data = {
+static const struct dvfsrc_regulator_pdata mt8183_data = {
+	.descs = mt8183_regulators,
 	.size = ARRAY_SIZE(mt8183_regulators),
-	.regulator_info = &mt8183_regulators[0],
 };
 
-static const unsigned int mt6873_voltages[] = {
-	575000,
+static const unsigned int mt8195_voltages[] = {
+	550000,
 	600000,
 	650000,
-	725000,
+	750000,
 };
 
-static struct dvfsrc_regulator mt6873_regulators[] = {
-	MT_DVFSRC_REGULAR("dvfsrc-vcore", VCORE,
-			  mt6873_voltages),
-	MT_DVFSRC_REGULAR("dvfsrc-vscp", VSCP,
-			  mt6873_voltages),
+static struct regulator_desc mt8195_regulators[] = {
+	MTK_DVFSRC_VREG("dvfsrc-vcore", VCORE, mt8195_voltages),
+	MTK_DVFSRC_VREG("dvfsrc-vscp", VSCP, mt8195_voltages),
 };
 
-static const struct dvfsrc_regulator_init_data regulator_mt6873_data = {
-	.size = ARRAY_SIZE(mt6873_regulators),
-	.regulator_info = &mt6873_regulators[0],
-};
-
-static const struct of_device_id mtk_dvfsrc_regulator_match[] = {
-	{
-		.compatible = "mediatek,mt8183-dvfsrc",
-		.data = &regulator_mt8183_data,
-	}, {
-		.compatible = "mediatek,mt8192-dvfsrc",
-		.data = &regulator_mt6873_data,
-	}, {
-		.compatible = "mediatek,mt6873-dvfsrc",
-		.data = &regulator_mt6873_data,
-	}, {
-		/* sentinel */
-	},
+static const struct dvfsrc_regulator_pdata mt8195_data = {
+	.descs = mt8195_regulators,
+	.size = ARRAY_SIZE(mt8195_regulators),
 };
-MODULE_DEVICE_TABLE(of, mtk_dvfsrc_regulator_match);
 
 static int dvfsrc_vcore_regulator_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match;
-	struct device *dev = &pdev->dev;
-	struct regulator_config config = { };
-	struct regulator_dev *rdev;
-	const struct dvfsrc_regulator_init_data *regulator_init_data;
-	struct dvfsrc_regulator *mt_regulators;
+	struct regulator_config config = { .dev = &pdev->dev };
+	const struct dvfsrc_regulator_pdata *pdata;
 	int i;
 
-	match = of_match_node(mtk_dvfsrc_regulator_match, dev->parent->of_node);
-
-	if (!match) {
-		dev_err(dev, "invalid compatible string\n");
-		return -ENODEV;
-	}
+	pdata = device_get_match_data(&pdev->dev);
+	if (!pdata)
+		return -EINVAL;
 
-	regulator_init_data = match->data;
+	for (i = 0; i < pdata->size; i++) {
+		struct regulator_desc *vrdesc = &pdata->descs[i];
+		struct regulator_dev *rdev;
 
-	mt_regulators = regulator_init_data->regulator_info;
-	for (i = 0; i < regulator_init_data->size; i++) {
-		config.dev = dev->parent;
-		config.driver_data = (mt_regulators + i);
-		rdev = devm_regulator_register(dev, &(mt_regulators + i)->desc,
-					       &config);
+		rdev = devm_regulator_register(&pdev->dev, vrdesc, &config);
 		if (IS_ERR(rdev)) {
-			dev_err(dev, "failed to register %s\n",
-				(mt_regulators + i)->desc.name);
+			dev_err(&pdev->dev, "failed to register %s\n", vrdesc->name);
 			return PTR_ERR(rdev);
 		}
 	}
@@ -190,25 +172,25 @@  static int dvfsrc_vcore_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id mtk_dvfsrc_regulator_match[] = {
+	{ .compatible = "mediatek,mt6873-dvfsrc-regulator", .data = &mt6873_data },
+	{ .compatible = "mediatek,mt8183-dvfsrc-regulator", .data = &mt8183_data },
+	{ .compatible = "mediatek,mt8192-dvfsrc-regulator", .data = &mt6873_data },
+	{ .compatible = "mediatek,mt8195-dvfsrc-regulator", .data = &mt8195_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_dvfsrc_regulator_match);
+
 static struct platform_driver mtk_dvfsrc_regulator_driver = {
 	.driver = {
 		.name  = "mtk-dvfsrc-regulator",
+		.of_match_table = mtk_dvfsrc_regulator_match,
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 	.probe = dvfsrc_vcore_regulator_probe,
 };
+module_platform_driver(mtk_dvfsrc_regulator_driver);
 
-static int __init mtk_dvfsrc_regulator_init(void)
-{
-	return platform_driver_register(&mtk_dvfsrc_regulator_driver);
-}
-subsys_initcall(mtk_dvfsrc_regulator_init);
-
-static void __exit mtk_dvfsrc_regulator_exit(void)
-{
-	platform_driver_unregister(&mtk_dvfsrc_regulator_driver);
-}
-module_exit(mtk_dvfsrc_regulator_exit);
-
+MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>");
 MODULE_AUTHOR("Arvin wang <arvin.wang@mediatek.com>");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");