| Message ID | 20220227205608.30812-15-cristian.marussi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce SCMI test driver and related Kselftest | expand |
Hi Cristian, On Mon, Feb 28, 2022 at 4:23 AM Cristian Marussi <cristian.marussi@arm.com> wrote: > > firmware: arm_scmi: Add testing Voltage protocol support > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > --- > .../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 >
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 > <cristian.marussi@arm.com> wrote: > > > > firmware: arm_scmi: Add testing Voltage protocol support > > > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > > --- > > .../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
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; +}
firmware: arm_scmi: Add testing Voltage protocol support Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> --- .../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