Hi

sorry for the late reply.

On Tue, Oct 06, 2020 at 11:56:37AM +0100, Mark Brown wrote:

On Mon, Oct 05, 2020 at 11:26:22PM +0100, Cristian Marussi wrote:

quoted

- .get_voltage / .set_voltage: routed via SCMI Voltage Domain Protocol
- .get_voltage_sel/.set_voltage_sel: using regulator framework helpers

You should not be implementing both of these interfaces, pick one.  It
looks like the direct voltage operations are the redundant ones here,
while the protocol uses actual voltages to communicate with the firmware
which makes the direct voltage operations a better fit it seems like the
expectation is that only a limited set of voltages is supported (as is
normal for the underlying physical regulators) so you want selectors.

I'm dropping non _sel methods in V2.

quoted

+	sreg->name = devm_kasprintf(dev, GFP_KERNEL, "%s", vinfo->name);
+	sreg->desc.name = devm_kasprintf(dev, GFP_KERNEL,
+					 "Vscmi.%s", sreg->name);
+	if (!sreg->name || !sreg->desc.name)
+		return -ENOMEM;

Why are we using different names here?

Not really a good reason...dropping internal name and "Vscmi" prefix
in V2.

quoted

+	num_doms = handle->voltage_ops->num_domains_get(handle);
+	if (num_doms <= 0) {
+		dev_err(&sdev->dev, "number of voltage domains invalid\n");
+		return num_doms ?: -EINVAL;

Please write normal conditional statements to improve legibility.

Ok.

Thanks

Cristian



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel