| Message ID | 20190726135954.11078-6-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: arm_scmi: add SCMI v2.0 fastchannels and reset protocol support | expand |
Hi Sudeep, On Fri, 2019-07-26 at 14:59 +0100, Sudeep Holla wrote: > On some ARM based systems, a separate Cortex-M based System Control > Processor(SCP) provides the overall power, clock, reset and system > control. System Control and Management Interface(SCMI) Message Protocol > is defined for the communication between the Application Cores(AP) > and the SCP. > > Adds support for the resets provided using SCMI protocol for performing > reset management of various devices present on the SoC. Various reset > functionalities are achieved by the means of different ARM SCMI device > operations provided by the ARM SCMI framework. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> thank you for the patch. I have a few suggestions below. > --- > MAINTAINERS | 1 + > drivers/reset/Kconfig | 10 +++ > drivers/reset/Makefile | 1 + > drivers/reset/reset-scmi.c | 133 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 145 insertions(+) > create mode 100644 drivers/reset/reset-scmi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 783569e3c4b4..59df8f88b56d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15545,6 +15545,7 @@ F: drivers/clk/clk-sc[mp]i.c > F: drivers/cpufreq/sc[mp]i-cpufreq.c > F: drivers/firmware/arm_scpi.c > F: drivers/firmware/arm_scmi/ > +F: drivers/reset/reset-scmi.c > F: include/linux/sc[mp]i_protocol.h > > SYSTEM RESET/SHUTDOWN DRIVERS > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > index 21efb7d39d62..09dcc3bf3b7a 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -22,6 +22,16 @@ config RESET_A10SR > This option enables support for the external reset functions for > peripheral PHYs on the Altera Arria10 System Resource Chip. > > +config RESET_ARM_SCMI > + tristate "Reset driver controlled via ARM SCMI interface" > + depends on ARM_SCMI_PROTOCOL || COMPILE_TEST Should this have a + default ARM_SCMI_PROTOCOL ? > + help > + This driver provides support for reset signal/domains that are > + controlled by firmware that implements the SCMI interface. > + > + This driver uses SCMI Message Protocol to interact with the > + firmware providing all the reset signals. s/providing/controlling/ ? > + > config RESET_ATH79 > bool "AR71xx Reset Driver" if COMPILE_TEST > default ATH79 > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile > index 61456b8f659c..2f1103d31938 100644 > --- a/drivers/reset/Makefile > +++ b/drivers/reset/Makefile > @@ -4,6 +4,7 @@ obj-y += hisilicon/ > obj-$(CONFIG_ARCH_STI) += sti/ > obj-$(CONFIG_ARCH_TEGRA) += tegra/ > obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o > +obj-$(CONFIG_RESET_ARM_SCMI) += reset-scmi.o s/RESET_ARM_SCMI/RESET_SCMI/ to match the driver/module name? > obj-$(CONFIG_RESET_ATH79) += reset-ath79.o > obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o > obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o > diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c > new file mode 100644 > index 000000000000..9e5d07cac2ed > --- /dev/null > +++ b/drivers/reset/reset-scmi.c > @@ -0,0 +1,133 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ARM System Control and Management Interface (ARM SCMI) reset driver > + * > + * Copyright (C) 2019 ARM Ltd. > + */ > + > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/reset-controller.h> > +#include <linux/scmi_protocol.h> > + > +/** > + * struct scmi_reset_data - reset controller information structure > + * @rcdev: reset controller entity > + * @handle: ARM SCMI handle used for communication with system controller > + * @dev: reset controller device pointer > + */ > +struct scmi_reset_data { > + struct reset_controller_dev rcdev; > + const struct scmi_handle *handle; > + struct device *dev; dev is unused, you could just remove it. > +}; > + > +#define to_scmi_reset_data(p) \ > + container_of((p), struct scmi_reset_data, rcdev) > + > +/** > + * scmi_reset_assert() - assert device reset > + * @rcdev: reset controller entity > + * @id: ID of the reset to be asserted > + * > + * This function implements the reset driver op to assert a device's reset > + * using the ARM SCMI protocol. > + * > + * Return: 0 for successful request, else a corresponding error value > + */ > +static int > +scmi_reset_assert(struct reset_controller_dev *rcdev, unsigned long id) > +{ > + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); > + const struct scmi_handle *handle = data->handle; This could be shortened to to_scmi_handle(rcdev), since none of the other fields in scmi_reset_data are used by the reset_control_ops callbacks. > + int ret; > + > + ret = handle->reset_ops->assert(handle, id); > + > + return ret; No need for ret here, see _deassert() and _reset(). > +} > + > +/** > + * scmi_reset_deassert() - deassert device reset > + * @rcdev: reset controller entity > + * @id: ID of the reset to be deasserted > + * > + * This function implements the reset driver op to deassert a device's reset > + * using the ARM SCMI protocol. > + * > + * Return: 0 for successful request, else a corresponding error value > + */ > +static int > +scmi_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id) > +{ > + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); > + const struct scmi_handle *handle = data->handle; > + > + return handle->reset_ops->deassert(handle, id); > +} > + > +/** > + * scmi_reset_reset() - reset the device > + * @rcdev: reset controller entity > + * @id: ID of the reset signal to be reset(assert + deassert) > + * > + * This function implements the reset driver op to reset a device's reset > + * signal using the ARM SCMI protocol. "to reset a device" or "to trigger a device's reset signal" would be a more accurate description. > + * > + * Return: 0 for successful request, else a corresponding error value > + */ > +static int > +scmi_reset_reset(struct reset_controller_dev *rcdev, unsigned long id) > +{ > + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); > + const struct scmi_handle *handle = data->handle; > + > + return handle->reset_ops->reset(handle, id); > +} > + > +static const struct reset_control_ops scmi_reset_ops = { > + .assert = scmi_reset_assert, > + .deassert = scmi_reset_deassert, > + .reset = scmi_reset_reset, > +}; > + > +static int scmi_reset_probe(struct scmi_device *sdev) > +{ > + struct scmi_reset_data *data; > + struct device *dev = &sdev->dev; > + struct device_node *np = dev->of_node; > + const struct scmi_handle *handle = sdev->handle; > + > + if (!handle || !handle->reset_ops) > + return -ENODEV; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->rcdev.ops = &scmi_reset_ops; > + data->rcdev.owner = THIS_MODULE; > + data->rcdev.of_node = np; This is missing rcdev.nr_resets. When nr_resets is kept at zero, the check in of_reset_simple_xlate will fail for any id. > + data->dev = dev; > + > + return devm_reset_controller_register(dev, &data->rcdev); > +} > + > +static const struct scmi_device_id scmi_id_table[] = { > + { SCMI_PROTOCOL_RESET }, > + { }, > +}; > +MODULE_DEVICE_TABLE(scmi, scmi_id_table); > + > +static struct scmi_driver scmi_reset_driver = { > + .name = "scmi-reset", > + .probe = scmi_reset_probe, > + .id_table = scmi_id_table, > +}; > +module_scmi_driver(scmi_reset_driver); > + > +MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>"); > +MODULE_DESCRIPTION("ARM SCMI clock driver"); s/clock/reset controller/ > +MODULE_LICENSE("GPL v2"); regards Philipp
On Mon, Jul 29, 2019 at 11:59:15AM +0200, Philipp Zabel wrote: > Hi Sudeep, > > On Fri, 2019-07-26 at 14:59 +0100, Sudeep Holla wrote: > > On some ARM based systems, a separate Cortex-M based System Control > > Processor(SCP) provides the overall power, clock, reset and system > > control. System Control and Management Interface(SCMI) Message Protocol > > is defined for the communication between the Application Cores(AP) > > and the SCP. > > > > Adds support for the resets provided using SCMI protocol for performing > > reset management of various devices present on the SoC. Various reset > > functionalities are achieved by the means of different ARM SCMI device > > operations provided by the ARM SCMI framework. > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > thank you for the patch. I have a few suggestions below. > Thanks for reviewing so quickly, all points taken and fixed locally. Will wait for some more time before posting v2. [...] > > +static int > > +scmi_reset_assert(struct reset_controller_dev *rcdev, unsigned long id) > > +{ > > + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); > > + const struct scmi_handle *handle = data->handle; > > This could be shortened to to_scmi_handle(rcdev), since none of the > other fields in scmi_reset_data are used by the reset_control_ops > callbacks. > Makes sense, missed to see that. [...] > > + > > +static int scmi_reset_probe(struct scmi_device *sdev) > > +{ > > + struct scmi_reset_data *data; > > + struct device *dev = &sdev->dev; > > + struct device_node *np = dev->of_node; > > + const struct scmi_handle *handle = sdev->handle; > > + > > + if (!handle || !handle->reset_ops) > > + return -ENODEV; > > + > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + data->rcdev.ops = &scmi_reset_ops; > > + data->rcdev.owner = THIS_MODULE; > > + data->rcdev.of_node = np; > > This is missing rcdev.nr_resets. When nr_resets is kept at zero, the > check in of_reset_simple_xlate will fail for any id. > I clearly missed to do git add for the above :(. I did find these after testing. > > + data->dev = dev; > > + > > + return devm_reset_controller_register(dev, &data->rcdev); > > +} > > + > > +static const struct scmi_device_id scmi_id_table[] = { > > + { SCMI_PROTOCOL_RESET }, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(scmi, scmi_id_table); > > + > > +static struct scmi_driver scmi_reset_driver = { > > + .name = "scmi-reset", > > + .probe = scmi_reset_probe, > > + .id_table = scmi_id_table, > > +}; > > +module_scmi_driver(scmi_reset_driver); > > + > > +MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>"); > > +MODULE_DESCRIPTION("ARM SCMI clock driver"); > > s/clock/reset controller/ > Stupid copy-paste :( Thanks again. -- Regards, Sudeep
diff --git a/MAINTAINERS b/MAINTAINERS index 783569e3c4b4..59df8f88b56d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15545,6 +15545,7 @@ F: drivers/clk/clk-sc[mp]i.c F: drivers/cpufreq/sc[mp]i-cpufreq.c F: drivers/firmware/arm_scpi.c F: drivers/firmware/arm_scmi/ +F: drivers/reset/reset-scmi.c F: include/linux/sc[mp]i_protocol.h SYSTEM RESET/SHUTDOWN DRIVERS diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index 21efb7d39d62..09dcc3bf3b7a 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -22,6 +22,16 @@ config RESET_A10SR This option enables support for the external reset functions for peripheral PHYs on the Altera Arria10 System Resource Chip. +config RESET_ARM_SCMI + tristate "Reset driver controlled via ARM SCMI interface" + depends on ARM_SCMI_PROTOCOL || COMPILE_TEST + help + This driver provides support for reset signal/domains that are + controlled by firmware that implements the SCMI interface. + + This driver uses SCMI Message Protocol to interact with the + firmware providing all the reset signals. + config RESET_ATH79 bool "AR71xx Reset Driver" if COMPILE_TEST default ATH79 diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 61456b8f659c..2f1103d31938 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -4,6 +4,7 @@ obj-y += hisilicon/ obj-$(CONFIG_ARCH_STI) += sti/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o +obj-$(CONFIG_RESET_ARM_SCMI) += reset-scmi.o obj-$(CONFIG_RESET_ATH79) += reset-ath79.o obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c new file mode 100644 index 000000000000..9e5d07cac2ed --- /dev/null +++ b/drivers/reset/reset-scmi.c @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ARM System Control and Management Interface (ARM SCMI) reset driver + * + * Copyright (C) 2019 ARM Ltd. + */ + +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/reset-controller.h> +#include <linux/scmi_protocol.h> + +/** + * struct scmi_reset_data - reset controller information structure + * @rcdev: reset controller entity + * @handle: ARM SCMI handle used for communication with system controller + * @dev: reset controller device pointer + */ +struct scmi_reset_data { + struct reset_controller_dev rcdev; + const struct scmi_handle *handle; + struct device *dev; +}; + +#define to_scmi_reset_data(p) \ + container_of((p), struct scmi_reset_data, rcdev) + +/** + * scmi_reset_assert() - assert device reset + * @rcdev: reset controller entity + * @id: ID of the reset to be asserted + * + * This function implements the reset driver op to assert a device's reset + * using the ARM SCMI protocol. + * + * Return: 0 for successful request, else a corresponding error value + */ +static int +scmi_reset_assert(struct reset_controller_dev *rcdev, unsigned long id) +{ + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); + const struct scmi_handle *handle = data->handle; + int ret; + + ret = handle->reset_ops->assert(handle, id); + + return ret; +} + +/** + * scmi_reset_deassert() - deassert device reset + * @rcdev: reset controller entity + * @id: ID of the reset to be deasserted + * + * This function implements the reset driver op to deassert a device's reset + * using the ARM SCMI protocol. + * + * Return: 0 for successful request, else a corresponding error value + */ +static int +scmi_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id) +{ + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); + const struct scmi_handle *handle = data->handle; + + return handle->reset_ops->deassert(handle, id); +} + +/** + * scmi_reset_reset() - reset the device + * @rcdev: reset controller entity + * @id: ID of the reset signal to be reset(assert + deassert) + * + * This function implements the reset driver op to reset a device's reset + * signal using the ARM SCMI protocol. + * + * Return: 0 for successful request, else a corresponding error value + */ +static int +scmi_reset_reset(struct reset_controller_dev *rcdev, unsigned long id) +{ + struct scmi_reset_data *data = to_scmi_reset_data(rcdev); + const struct scmi_handle *handle = data->handle; + + return handle->reset_ops->reset(handle, id); +} + +static const struct reset_control_ops scmi_reset_ops = { + .assert = scmi_reset_assert, + .deassert = scmi_reset_deassert, + .reset = scmi_reset_reset, +}; + +static int scmi_reset_probe(struct scmi_device *sdev) +{ + struct scmi_reset_data *data; + struct device *dev = &sdev->dev; + struct device_node *np = dev->of_node; + const struct scmi_handle *handle = sdev->handle; + + if (!handle || !handle->reset_ops) + return -ENODEV; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->rcdev.ops = &scmi_reset_ops; + data->rcdev.owner = THIS_MODULE; + data->rcdev.of_node = np; + data->dev = dev; + + return devm_reset_controller_register(dev, &data->rcdev); +} + +static const struct scmi_device_id scmi_id_table[] = { + { SCMI_PROTOCOL_RESET }, + { }, +}; +MODULE_DEVICE_TABLE(scmi, scmi_id_table); + +static struct scmi_driver scmi_reset_driver = { + .name = "scmi-reset", + .probe = scmi_reset_probe, + .id_table = scmi_id_table, +}; +module_scmi_driver(scmi_reset_driver); + +MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>"); +MODULE_DESCRIPTION("ARM SCMI clock driver"); +MODULE_LICENSE("GPL v2");
On some ARM based systems, a separate Cortex-M based System Control Processor(SCP) provides the overall power, clock, reset and system control. System Control and Management Interface(SCMI) Message Protocol is defined for the communication between the Application Cores(AP) and the SCP. Adds support for the resets provided using SCMI protocol for performing reset management of various devices present on the SoC. Various reset functionalities are achieved by the means of different ARM SCMI device operations provided by the ARM SCMI framework. Cc: Philipp Zabel <p.zabel@pengutronix.de> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- MAINTAINERS | 1 + drivers/reset/Kconfig | 10 +++ drivers/reset/Makefile | 1 + drivers/reset/reset-scmi.c | 133 +++++++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+) create mode 100644 drivers/reset/reset-scmi.c