2022-02-27 21:58:08

by Cristian Marussi

[permalink] [raw]
Subject: [RFC PATCH 14/16] firmware: arm_scmi: Add testing Voltage protocol support

firmware: arm_scmi: Add testing Voltage protocol support

Signed-off-by: Cristian Marussi <[email protected]>
---
.../arm_scmi/scmi_test_driver/Makefile | 2 +-
.../arm_scmi/scmi_test_driver/scmi_test.c | 2 +
.../arm_scmi/scmi_test_driver/test_common.h | 1 +
.../arm_scmi/scmi_test_driver/test_voltages.c | 51 +++++++++++++++++++
4 files changed, 55 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c

diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/Makefile b/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
index 68a3d94a6a88..3b7df18de250 100644
--- a/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
+++ b/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
scmi_test_driver-objs := scmi_test.o test_common.o test_clocks.o test_sensors.o \
- test_powers.o
+ test_powers.o test_voltages.o
obj-$(CONFIG_ARM_SCMI_TEST_DRIVER) += scmi_test_driver.o

diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c b/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
index df0d3e572010..2ca9f82c5bf3 100644
--- a/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
+++ b/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
@@ -26,6 +26,7 @@ int (*scmi_test_init[SCMI_MAX_PROTOCOLS])(struct scmi_test_setup *) = {
[SCMI_PROTOCOL_POWER] = scmi_test_power_init,
[SCMI_PROTOCOL_CLOCK] = scmi_test_clock_init,
[SCMI_PROTOCOL_SENSOR] = scmi_test_sensor_init,
+ [SCMI_PROTOCOL_VOLTAGE] = scmi_test_voltage_init,
};

static void
@@ -125,6 +126,7 @@ static int scmi_testing_probe(struct scmi_device *sdev)
}

static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_VOLTAGE, "__scmi_test-voltage" },
{ SCMI_PROTOCOL_CLOCK, "__scmi_test-clock" },
{ SCMI_PROTOCOL_SENSOR, "__scmi_test-sensor" },
{ SCMI_PROTOCOL_POWER, "__scmi_test-power" },
diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h b/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
index 9f3d35ba4477..338b65da593f 100644
--- a/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
+++ b/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
@@ -102,6 +102,7 @@ int scmi_test_release(struct inode *ino, struct file *filp);

int scmi_test_clock_init(struct scmi_test_setup *tsp);
int scmi_test_sensor_init(struct scmi_test_setup *tsp);
+int scmi_test_voltage_init(struct scmi_test_setup *tsp);
int scmi_test_power_init(struct scmi_test_setup *tsp);

#endif /* __SCMI_TEST_COMMON_H */
diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c b/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
new file mode 100644
index 000000000000..ab91080e3a0f
--- /dev/null
+++ b/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SCMI Testing Driver - Voltage Protocol
+ *
+ * Copyright (C) 2022 ARM Ltd.
+ */
+
+#include <linux/dcache.h>
+#include <linux/debugfs.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/kstrtox.h>
+#include <linux/module.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+
+#include "test_common.h"
+
+struct scmi_voltage_data {
+ unsigned int version;
+ int count;
+};
+
+int scmi_test_voltage_init(struct scmi_test_setup *tsp)
+{
+ struct scmi_voltage_data *vdata;
+ struct device *dev = &tsp->sdev->dev;
+ const struct scmi_voltage_proto_ops *voltage_ops;
+
+ vdata = devm_kzalloc(dev, sizeof(*vdata), GFP_KERNEL);
+ if (!vdata)
+ return -ENOMEM;
+
+ voltage_ops = tsp->ops;
+ vdata->version = voltage_ops->version_get(tsp->ph);
+ vdata->count = voltage_ops->num_domains_get(tsp->ph);
+
+ if (vdata->count <= 0) {
+ dev_err(dev, "number of voltage doms invalid: %d\n",
+ vdata->count);
+ return vdata->count ?: -EINVAL;
+ }
+
+ dev_info(dev, "Found %d voltage resources.\n", vdata->count);
+
+ tsp->priv = vdata;
+ debugfs_create_x32("version", 0400, tsp->parent, &vdata->version);
+
+ return 0;
+}
--
2.17.1


2022-08-01 04:51:37

by Arun KS

[permalink] [raw]
Subject: Re: [RFC PATCH 14/16] firmware: arm_scmi: Add testing Voltage protocol support

Hi Cristian,

On Mon, Feb 28, 2022 at 4:23 AM Cristian Marussi
<[email protected]> wrote:
>
> firmware: arm_scmi: Add testing Voltage protocol support
>
> Signed-off-by: Cristian Marussi <[email protected]>
> ---
> .../arm_scmi/scmi_test_driver/Makefile | 2 +-
> .../arm_scmi/scmi_test_driver/scmi_test.c | 2 +
> .../arm_scmi/scmi_test_driver/test_common.h | 1 +
> .../arm_scmi/scmi_test_driver/test_voltages.c | 51 +++++++++++++++++++
> 4 files changed, 55 insertions(+), 1 deletion(-)
> create mode 100644 drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
>
> diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/Makefile b/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
> index 68a3d94a6a88..3b7df18de250 100644
> --- a/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
> +++ b/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> scmi_test_driver-objs := scmi_test.o test_common.o test_clocks.o test_sensors.o \
> - test_powers.o
> + test_powers.o test_voltages.o
> obj-$(CONFIG_ARM_SCMI_TEST_DRIVER) += scmi_test_driver.o
>
> diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c b/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
> index df0d3e572010..2ca9f82c5bf3 100644
> --- a/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
> +++ b/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
> @@ -26,6 +26,7 @@ int (*scmi_test_init[SCMI_MAX_PROTOCOLS])(struct scmi_test_setup *) = {
> [SCMI_PROTOCOL_POWER] = scmi_test_power_init,
> [SCMI_PROTOCOL_CLOCK] = scmi_test_clock_init,
> [SCMI_PROTOCOL_SENSOR] = scmi_test_sensor_init,
> + [SCMI_PROTOCOL_VOLTAGE] = scmi_test_voltage_init,
> };
>
> static void
> @@ -125,6 +126,7 @@ static int scmi_testing_probe(struct scmi_device *sdev)
> }
>
> static const struct scmi_device_id scmi_id_table[] = {
> + { SCMI_PROTOCOL_VOLTAGE, "__scmi_test-voltage" },
> { SCMI_PROTOCOL_CLOCK, "__scmi_test-clock" },
> { SCMI_PROTOCOL_SENSOR, "__scmi_test-sensor" },
> { SCMI_PROTOCOL_POWER, "__scmi_test-power" },
> diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h b/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
> index 9f3d35ba4477..338b65da593f 100644
> --- a/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
> +++ b/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
> @@ -102,6 +102,7 @@ int scmi_test_release(struct inode *ino, struct file *filp);
>
> int scmi_test_clock_init(struct scmi_test_setup *tsp);
> int scmi_test_sensor_init(struct scmi_test_setup *tsp);
> +int scmi_test_voltage_init(struct scmi_test_setup *tsp);
> int scmi_test_power_init(struct scmi_test_setup *tsp);
>
> #endif /* __SCMI_TEST_COMMON_H */
> diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c b/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
> new file mode 100644
> index 000000000000..ab91080e3a0f
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SCMI Testing Driver - Voltage Protocol
> + *
> + * Copyright (C) 2022 ARM Ltd.
> + */
> +
> +#include <linux/dcache.h>
> +#include <linux/debugfs.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/kstrtox.h>
> +#include <linux/module.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
> +
> +#include "test_common.h"
> +
> +struct scmi_voltage_data {
> + unsigned int version;
> + int count;
> +};
> +
> +int scmi_test_voltage_init(struct scmi_test_setup *tsp)
> +{
> + struct scmi_voltage_data *vdata;
> + struct device *dev = &tsp->sdev->dev;
> + const struct scmi_voltage_proto_ops *voltage_ops;
> +
> + vdata = devm_kzalloc(dev, sizeof(*vdata), GFP_KERNEL);
> + if (!vdata)
> + return -ENOMEM;
> +
> + voltage_ops = tsp->ops;
> + vdata->version = voltage_ops->version_get(tsp->ph);
> + vdata->count = voltage_ops->num_domains_get(tsp->ph);
> +
> + if (vdata->count <= 0) {
> + dev_err(dev, "number of voltage doms invalid: %d\n",
> + vdata->count);
> + return vdata->count ?: -EINVAL;
> + }
> +
> + dev_info(dev, "Found %d voltage resources.\n", vdata->count);
> +
> + tsp->priv = vdata;
> + debugfs_create_x32("version", 0400, tsp->parent, &vdata->version);
Any particular reason, why we are not creating debugfs entires for
regulator level_get/level_set like it was done for
clocks(rate_get_set)?
> +
> + return 0;
> +}
> --
> 2.17.1
>

2022-08-01 07:04:54

by Cristian Marussi

[permalink] [raw]
Subject: Re: [RFC PATCH 14/16] firmware: arm_scmi: Add testing Voltage protocol support

On Mon, Aug 01, 2022 at 10:17:00AM +0530, Arun KS wrote:
> Hi Cristian,
>

Hi Arun,

first of all thanks for the feedback !

> On Mon, Feb 28, 2022 at 4:23 AM Cristian Marussi
> <[email protected]> wrote:
> >
> > firmware: arm_scmi: Add testing Voltage protocol support
> >
> > Signed-off-by: Cristian Marussi <[email protected]>
> > ---
> > .../arm_scmi/scmi_test_driver/Makefile | 2 +-
> > .../arm_scmi/scmi_test_driver/scmi_test.c | 2 +
> > .../arm_scmi/scmi_test_driver/test_common.h | 1 +
> > .../arm_scmi/scmi_test_driver/test_voltages.c | 51 +++++++++++++++++++
> > 4 files changed, 55 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
> >
> > diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/Makefile b/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
> > index 68a3d94a6a88..3b7df18de250 100644
> > --- a/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
> > +++ b/drivers/firmware/arm_scmi/scmi_test_driver/Makefile
> > @@ -1,5 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > scmi_test_driver-objs := scmi_test.o test_common.o test_clocks.o test_sensors.o \
> > - test_powers.o
> > + test_powers.o test_voltages.o
> > obj-$(CONFIG_ARM_SCMI_TEST_DRIVER) += scmi_test_driver.o
> >
> > diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c b/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
> > index df0d3e572010..2ca9f82c5bf3 100644
> > --- a/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
> > +++ b/drivers/firmware/arm_scmi/scmi_test_driver/scmi_test.c
> > @@ -26,6 +26,7 @@ int (*scmi_test_init[SCMI_MAX_PROTOCOLS])(struct scmi_test_setup *) = {
> > [SCMI_PROTOCOL_POWER] = scmi_test_power_init,
> > [SCMI_PROTOCOL_CLOCK] = scmi_test_clock_init,
> > [SCMI_PROTOCOL_SENSOR] = scmi_test_sensor_init,
> > + [SCMI_PROTOCOL_VOLTAGE] = scmi_test_voltage_init,
> > };
> >
> > static void
> > @@ -125,6 +126,7 @@ static int scmi_testing_probe(struct scmi_device *sdev)
> > }
> >
> > static const struct scmi_device_id scmi_id_table[] = {
> > + { SCMI_PROTOCOL_VOLTAGE, "__scmi_test-voltage" },
> > { SCMI_PROTOCOL_CLOCK, "__scmi_test-clock" },
> > { SCMI_PROTOCOL_SENSOR, "__scmi_test-sensor" },
> > { SCMI_PROTOCOL_POWER, "__scmi_test-power" },
> > diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h b/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
> > index 9f3d35ba4477..338b65da593f 100644
> > --- a/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
> > +++ b/drivers/firmware/arm_scmi/scmi_test_driver/test_common.h
> > @@ -102,6 +102,7 @@ int scmi_test_release(struct inode *ino, struct file *filp);
> >
> > int scmi_test_clock_init(struct scmi_test_setup *tsp);
> > int scmi_test_sensor_init(struct scmi_test_setup *tsp);
> > +int scmi_test_voltage_init(struct scmi_test_setup *tsp);
> > int scmi_test_power_init(struct scmi_test_setup *tsp);
> >
> > #endif /* __SCMI_TEST_COMMON_H */
> > diff --git a/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c b/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
> > new file mode 100644
> > index 000000000000..ab91080e3a0f
> > --- /dev/null
> > +++ b/drivers/firmware/arm_scmi/scmi_test_driver/test_voltages.c
> > @@ -0,0 +1,51 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SCMI Testing Driver - Voltage Protocol
> > + *
> > + * Copyright (C) 2022 ARM Ltd.
> > + */
> > +
> > +#include <linux/dcache.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/err.h>
> > +#include <linux/fs.h>
> > +#include <linux/io.h>
> > +#include <linux/kstrtox.h>
> > +#include <linux/module.h>
> > +#include <linux/scmi_protocol.h>
> > +#include <linux/slab.h>
> > +
> > +#include "test_common.h"
> > +
> > +struct scmi_voltage_data {
> > + unsigned int version;
> > + int count;
> > +};
> > +
> > +int scmi_test_voltage_init(struct scmi_test_setup *tsp)
> > +{
> > + struct scmi_voltage_data *vdata;
> > + struct device *dev = &tsp->sdev->dev;
> > + const struct scmi_voltage_proto_ops *voltage_ops;
> > +
> > + vdata = devm_kzalloc(dev, sizeof(*vdata), GFP_KERNEL);
> > + if (!vdata)
> > + return -ENOMEM;
> > +
> > + voltage_ops = tsp->ops;
> > + vdata->version = voltage_ops->version_get(tsp->ph);
> > + vdata->count = voltage_ops->num_domains_get(tsp->ph);
> > +
> > + if (vdata->count <= 0) {
> > + dev_err(dev, "number of voltage doms invalid: %d\n",
> > + vdata->count);
> > + return vdata->count ?: -EINVAL;
> > + }
> > +
> > + dev_info(dev, "Found %d voltage resources.\n", vdata->count);
> > +
> > + tsp->priv = vdata;
> > + debugfs_create_x32("version", 0400, tsp->parent, &vdata->version);
> Any particular reason, why we are not creating debugfs entires for
> regulator level_get/level_set like it was done for
> clocks(rate_get_set)?

No, it is just that this RFC initial series was meant to gather feedback
on this approach at testing and to experiment with this solution itself
a bit, before committing more work to more extensive cover all SCMI protocols
and ops... so the series is 'incomplete' by design :P

... having said that, despite the series had not received so much
feedback at the end, I have worked in the background on extending its
SCMI coverage, so that now I can support all SCMI protocols (exposing all
ops on debugfs) ... I'll plan to post a new more extensive series in the
near(-ish) future once I'll have the time to clean it up and add more
example KSFT testcases (and fix the dummy ones ...)

Thanks,
Cristian