diff mbox series

[v6,29/37] regulator: scmi: port driver to the new scmi_voltage_proto_ops interface

Message ID 20210202221555.41167-30-cristian.marussi@arm.com (mailing list archive)
State New, archived
Headers show
Series SCMI vendor protocols and modularization | expand

Commit Message

Cristian Marussi Feb. 2, 2021, 10:15 p.m. UTC 
Port driver to the new SCMI Voltage interface based on protocol handles
and common devm_get_ops().

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v4 --> v5
- using renamed devm_get/put_protocol
---
 drivers/regulator/scmi-regulator.c | 40 ++++++++++++++++--------------
 1 file changed, 22 insertions(+), 18 deletions(-)

Comments

Mark Brown Feb. 3, 2021, 1:24 p.m. UTC | #1 
On Tue, Feb 02, 2021 at 10:15:47PM +0000, Cristian Marussi wrote:
> Port driver to the new SCMI Voltage interface based on protocol handles
> and common devm_get_ops().

I have no cover letter or other context for this, what's the story here?
Cristian Marussi Feb. 3, 2021, 2:39 p.m. UTC | #2 
Hi Mark

On Wed, Feb 03, 2021 at 01:24:32PM +0000, Mark Brown wrote:
> On Tue, Feb 02, 2021 at 10:15:47PM +0000, Cristian Marussi wrote:
> > Port driver to the new SCMI Voltage interface based on protocol handles
> > and common devm_get_ops().
> 
> I have no cover letter or other context for this, what's the story here?

Sorry I've CCed only the relevant patches to the subsystem maintainers.

The whole story is in the series cover-letter here:

https://lore.kernel.org/linux-arm-kernel/20210202221555.41167-1-cristian.marussi@arm.com/

In summary we wanted to ease both protocols modularization in general and1
the implementation of custom vendor custom protocols and drivers (as
allowed by SCMI spec): to do that I reviewed the internals of SCMI protos
handling to centralize resource accounting (so that I can track who is
using which protocol at any time), and I generalized the interface exposed
to the SCMI drivers so that as an example:

handle->voltage_ops->level_set(handle, ...)

becomes:

/*get access to the proto and ops in probe()*/
vops = handle->devm_get_protocol(handle, SCMI_PROTOCOL_VOLTAGE, &ph);

/* use it */
vops->level_set(ph, ...)

This way the interface is unified for all protocols both standard and
custom and if you develop a new custom proto implementation, and driver,
you'll end-up using it in a similar way:

cops = handle->devm_get_protocol(handle, SCMI_PROTOCOL_CUSTOM, &ph);

cops->custom_method(ph, )

without having to add an endless stream of new custom *_ops structs to
the handle struct.

To attain the above, though, there are also a number of internal changes
in the series that are tightly related to the drivers' interface which I
am changing as an example with this patch, so the series contains a bit
of transient code added and removed along the way around the drivers
changes to maintain bisectability throughout all the series.

Apologies to have not included the cover-letter at first to provide
context.

Thanks

Cristian
Mark Brown Feb. 3, 2021, 4:18 p.m. UTC | #3 
On Tue, Feb 02, 2021 at 10:15:47PM +0000, Cristian Marussi wrote:
> Port driver to the new SCMI Voltage interface based on protocol handles
> and common devm_get_ops().

Acked-by: Mark Brown <broonie@kernel.org>
diff mbox series

Patch

diff --git a/drivers/regulator/scmi-regulator.c b/drivers/regulator/scmi-regulator.c
index 0e8b3caa8146..9a4297276098 100644
--- a/drivers/regulator/scmi-regulator.c
+++ b/drivers/regulator/scmi-regulator.c
@@ -33,9 +33,12 @@ 
 #include <linux/slab.h>
 #include <linux/types.h>
 
+static const struct scmi_voltage_proto_ops *voltage_ops;
+
 struct scmi_regulator {
 	u32 id;
 	struct scmi_device *sdev;
+	struct scmi_protocol_handle *ph;
 	struct regulator_dev *rdev;
 	struct device_node *of_node;
 	struct regulator_desc desc;
@@ -50,19 +53,17 @@  struct scmi_regulator_info {
 static int scmi_reg_enable(struct regulator_dev *rdev)
 {
 	struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
-	const struct scmi_handle *handle = sreg->sdev->handle;
 
-	return handle->voltage_ops->config_set(handle, sreg->id,
-					       SCMI_VOLTAGE_ARCH_STATE_ON);
+	return voltage_ops->config_set(sreg->ph, sreg->id,
+				       SCMI_VOLTAGE_ARCH_STATE_ON);
 }
 
 static int scmi_reg_disable(struct regulator_dev *rdev)
 {
 	struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
-	const struct scmi_handle *handle = sreg->sdev->handle;
 
-	return handle->voltage_ops->config_set(handle, sreg->id,
-					       SCMI_VOLTAGE_ARCH_STATE_OFF);
+	return voltage_ops->config_set(sreg->ph, sreg->id,
+				       SCMI_VOLTAGE_ARCH_STATE_OFF);
 }
 
 static int scmi_reg_is_enabled(struct regulator_dev *rdev)
@@ -70,10 +71,8 @@  static int scmi_reg_is_enabled(struct regulator_dev *rdev)
 	int ret;
 	u32 config;
 	struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
-	const struct scmi_handle *handle = sreg->sdev->handle;
 
-	ret = handle->voltage_ops->config_get(handle, sreg->id,
-					      &config);
+	ret = voltage_ops->config_get(sreg->ph, sreg->id, &config);
 	if (ret) {
 		dev_err(&sreg->sdev->dev,
 			"Error %d reading regulator %s status.\n",
@@ -89,9 +88,8 @@  static int scmi_reg_get_voltage_sel(struct regulator_dev *rdev)
 	int ret;
 	s32 volt_uV;
 	struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
-	const struct scmi_handle *handle = sreg->sdev->handle;
 
-	ret = handle->voltage_ops->level_get(handle, sreg->id, &volt_uV);
+	ret = voltage_ops->level_get(sreg->ph, sreg->id, &volt_uV);
 	if (ret)
 		return ret;
 
@@ -103,13 +101,12 @@  static int scmi_reg_set_voltage_sel(struct regulator_dev *rdev,
 {
 	s32 volt_uV;
 	struct scmi_regulator *sreg = rdev_get_drvdata(rdev);
-	const struct scmi_handle *handle = sreg->sdev->handle;
 
 	volt_uV = sreg->desc.ops->list_voltage(rdev, selector);
 	if (volt_uV <= 0)
 		return -EINVAL;
 
-	return handle->voltage_ops->level_set(handle, sreg->id, 0x0, volt_uV);
+	return voltage_ops->level_set(sreg->ph, sreg->id, 0x0, volt_uV);
 }
 
 static const struct regulator_ops scmi_reg_fixed_ops = {
@@ -204,11 +201,10 @@  scmi_config_discrete_regulator_mappings(struct scmi_regulator *sreg,
 static int scmi_regulator_common_init(struct scmi_regulator *sreg)
 {
 	int ret;
-	const struct scmi_handle *handle = sreg->sdev->handle;
 	struct device *dev = &sreg->sdev->dev;
 	const struct scmi_voltage_info *vinfo;
 
-	vinfo = handle->voltage_ops->info_get(handle, sreg->id);
+	vinfo = voltage_ops->info_get(sreg->ph, sreg->id);
 	if (!vinfo) {
 		dev_warn(dev, "Failure to get voltage domain %d\n",
 			 sreg->id);
@@ -257,6 +253,7 @@  static int scmi_regulator_common_init(struct scmi_regulator *sreg)
 }
 
 static int process_scmi_regulator_of_node(struct scmi_device *sdev,
+					  struct scmi_protocol_handle *ph,
 					  struct device_node *np,
 					  struct scmi_regulator_info *rinfo)
 {
@@ -284,6 +281,7 @@  static int process_scmi_regulator_of_node(struct scmi_device *sdev,
 
 	rinfo->sregv[dom]->id = dom;
 	rinfo->sregv[dom]->sdev = sdev;
+	rinfo->sregv[dom]->ph = ph;
 
 	/* get hold of good nodes */
 	of_node_get(np);
@@ -302,11 +300,17 @@  static int scmi_regulator_probe(struct scmi_device *sdev)
 	struct device_node *np, *child;
 	const struct scmi_handle *handle = sdev->handle;
 	struct scmi_regulator_info *rinfo;
+	struct scmi_protocol_handle *ph;
 
-	if (!handle || !handle->voltage_ops)
+	if (!handle)
 		return -ENODEV;
 
-	num_doms = handle->voltage_ops->num_domains_get(handle);
+	voltage_ops = handle->devm_get_protocol(sdev,
+						SCMI_PROTOCOL_VOLTAGE, &ph);
+	if (IS_ERR(voltage_ops))
+		return PTR_ERR(voltage_ops);
+
+	num_doms = voltage_ops->num_domains_get(ph);
 	if (num_doms <= 0) {
 		if (!num_doms) {
 			dev_err(&sdev->dev,
@@ -341,7 +345,7 @@  static int scmi_regulator_probe(struct scmi_device *sdev)
 	 */
 	np = of_find_node_by_name(handle->dev->of_node, "regulators");
 	for_each_child_of_node(np, child) {
-		ret = process_scmi_regulator_of_node(sdev, child, rinfo);
+		ret = process_scmi_regulator_of_node(sdev, ph, child, rinfo);
 		/* abort on any mem issue */
 		if (ret == -ENOMEM)
 			return ret;