| Message ID | 20190611164157.24656-1-georgi.djakov@linaro.org |
|---|---|
| Headers | show |
| Series | Add QCS404 interconnect provider driver | expand |
On Tue 11 Jun 09:41 PDT 2019, Georgi Djakov wrote: > Register a platform device to handle the communication of bus bandwidth > requests with the remote processor. The interconnect proxy device is part > of this remote processor (RPM) hardware. Let's create a icc-smd-rpm proxy > child device to represent the bus throughput functionality that is provided > by the RPM. > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > > v3: > - New patch. > > drivers/soc/qcom/smd-rpm.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/smd-rpm.c b/drivers/soc/qcom/smd-rpm.c > index 9956bb2c63f2..a028250737ec 100644 > --- a/drivers/soc/qcom/smd-rpm.c > +++ b/drivers/soc/qcom/smd-rpm.c > @@ -27,12 +27,14 @@ > /** > * struct qcom_smd_rpm - state of the rpm device driver > * @rpm_channel: reference to the smd channel > + * @icc: interconnect proxy device > * @ack: completion for acks > * @lock: mutual exclusion around the send/complete pair > * @ack_status: result of the rpm request > */ > struct qcom_smd_rpm { > struct rpmsg_endpoint *rpm_channel; > + struct platform_device *icc; > struct device *dev; > > struct completion ack; > @@ -201,6 +203,7 @@ static int qcom_smd_rpm_callback(struct rpmsg_device *rpdev, > static int qcom_smd_rpm_probe(struct rpmsg_device *rpdev) > { > struct qcom_smd_rpm *rpm; > + int ret; > > rpm = devm_kzalloc(&rpdev->dev, sizeof(*rpm), GFP_KERNEL); > if (!rpm) > @@ -213,11 +216,23 @@ static int qcom_smd_rpm_probe(struct rpmsg_device *rpdev) > rpm->rpm_channel = rpdev->ept; > dev_set_drvdata(&rpdev->dev, rpm); > > - return of_platform_populate(rpdev->dev.of_node, NULL, NULL, &rpdev->dev); > + rpm->icc = platform_device_register_data(&rpdev->dev, "icc_smd_rpm", -1, > + NULL, 0); > + if (!IS_ERR(rpm->icc)) This will be IS_ERR() only if the struct platform_device couldn't be allocated or registered, it does not relate to that driver's probe. As such it makes sense to fail the probe if this failed. So flip this around and return PTR_ERR() here. > + platform_set_drvdata(rpm->icc, rpm); It's possible that the device_register above finds the driver and calls it (pending initcall ordering etc), in which case the child's drvdata wouldn't yet be set. In the other drivers where we do this we have the child to request the drvdata of its parent, so I think you should do the same here. Apart from this, this patch looks good! Regards, Bjorn > + > + ret = of_platform_populate(rpdev->dev.of_node, NULL, NULL, &rpdev->dev); > + if (ret) > + platform_device_unregister(rpm->icc); > + > + return ret; > } > > static void qcom_smd_rpm_remove(struct rpmsg_device *rpdev) > { > + struct qcom_smd_rpm *rpm = dev_get_drvdata(&rpdev->dev); > + > + platform_device_unregister(rpm->icc); > of_platform_depopulate(&rpdev->dev); > } >
On Tue 11 Jun 09:41 PDT 2019, Georgi Djakov wrote: > On some Qualcomm SoCs, there is a remote processor, which controls some of > the Network-On-Chip interconnect resources. Other CPUs express their needs > by communicating with this processor. Add a driver to handle communication > with this remote processor. > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > > v3: > - New patch. > > drivers/interconnect/qcom/Kconfig | 9 ++++ > drivers/interconnect/qcom/Makefile | 2 + > drivers/interconnect/qcom/smd-rpm.c | 72 +++++++++++++++++++++++++++++ > drivers/interconnect/qcom/smd-rpm.h | 15 ++++++ > 4 files changed, 98 insertions(+) > create mode 100644 drivers/interconnect/qcom/smd-rpm.c > create mode 100644 drivers/interconnect/qcom/smd-rpm.h > > diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig > index e76e3e248c41..b0eade1da5d5 100644 > --- a/drivers/interconnect/qcom/Kconfig > +++ b/drivers/interconnect/qcom/Kconfig > @@ -9,6 +9,7 @@ config INTERCONNECT_QCOM_QCS404 > tristate "Qualcomm QCS404 interconnect driver" > depends on INTERCONNECT_QCOM > depends on QCOM_SMD_RPM || COMPILE_TEST > + select INTERCONNECT_QCOM_SMD_RPM > help > This is a driver for the Qualcomm Network-on-Chip on qcs404-based > platforms. > @@ -20,3 +21,11 @@ config INTERCONNECT_QCOM_SDM845 > help > This is a driver for the Qualcomm Network-on-Chip on sdm845-based > platforms. > + > +config INTERCONNECT_QCOM_SMD_RPM > + tristate "Qualcomm SMD RPM interconnect driver" I think it's correct to have INTERCONNECT_QCOM_QCS404 select INTERCONNECT_QCOM_SMD_RPM and then but INTERCONNECT_QCOM_SMD_RPM should not be user selectable. So leave the tristate but drop the description and the help text. > + depends on INTERCONNECT_QCOM > + depends on QCOM_SMD_RPM || COMPILE_TEST > + help > + This is a driver for communicating interconnect related configuration > + details with a remote processor (RPM) on Qualcomm platforms. > diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile > index 059ff325ee6c..67dafb783dec 100644 > --- a/drivers/interconnect/qcom/Makefile > +++ b/drivers/interconnect/qcom/Makefile > @@ -2,6 +2,8 @@ > > qnoc-qcs404-objs := qcs404.o > qnoc-sdm845-objs := sdm845.o > +icc-smd-rpm-objs := smd-rpm.o > > obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o > obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o > +obj-$(CONFIG_INTERCONNECT_QCOM_SMD_RPM) += icc-smd-rpm.o > diff --git a/drivers/interconnect/qcom/smd-rpm.c b/drivers/interconnect/qcom/smd-rpm.c > new file mode 100644 > index 000000000000..af22c0a293e6 > --- /dev/null > +++ b/drivers/interconnect/qcom/smd-rpm.c > @@ -0,0 +1,72 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * RPM over SMD communication wrapper for interconnects > + * > + * Copyright (C) 2019 Linaro Ltd > + * Author: Georgi Djakov <georgi.djakov@linaro.org> > + */ > + > +#include <linux/interconnect-provider.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/soc/qcom/smd-rpm.h> > + > +#include "smd-rpm.h" > + > +#define RPM_KEY_BW 0x00007762 > + > +static struct qcom_smd_rpm *icc_smd_rpm; > + > +struct icc_rpm_smd_req { > + __le32 key; > + __le32 nbytes; > + __le32 value; > +}; > + > +bool qcom_icc_rpm_smd_available(void) > +{ > + if (!icc_smd_rpm) > + return false; > + > + return true; > +} > +EXPORT_SYMBOL_GPL(qcom_icc_rpm_smd_available); > + > +int qcom_icc_rpm_smd_send(int ctx, int rsc_type, int id, u32 val) > +{ > + struct icc_rpm_smd_req req = { > + .key = cpu_to_le32(RPM_KEY_BW), > + .nbytes = cpu_to_le32(sizeof(u32)), > + .value = cpu_to_le32(val), > + }; > + > + return qcom_rpm_smd_write(icc_smd_rpm, ctx, rsc_type, id, &req, > + sizeof(req)); > +} > +EXPORT_SYMBOL_GPL(qcom_icc_rpm_smd_send); > + > +static int qcom_icc_rpm_smd_probe(struct platform_device *pdev) > +{ > + icc_smd_rpm = dev_get_drvdata(pdev->dev.parent); > + > + if (!icc_smd_rpm) { > + dev_err(&pdev->dev, "unable to retrieve handle to RPM\n"); > + return -ENODEV; > + } > + > + return 0; > +} It would probably be nice to have a remove function that zeros out icc_smd_rpm as well. Regards, Bjorn > + > +static struct platform_driver qcom_interconnect_rpm_smd_driver = { > + .driver = { > + .name = "icc_smd_rpm", > + }, > + .probe = qcom_icc_rpm_smd_probe, > +}; > +module_platform_driver(qcom_interconnect_rpm_smd_driver); > +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org>"); > +MODULE_DESCRIPTION("Qualcomm SMD RPM interconnect proxy driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:icc_smd_rpm"); > diff --git a/drivers/interconnect/qcom/smd-rpm.h b/drivers/interconnect/qcom/smd-rpm.h > new file mode 100644 > index 000000000000..ca9d0327b8ac > --- /dev/null > +++ b/drivers/interconnect/qcom/smd-rpm.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2019, Linaro Ltd. > + * Author: Georgi Djakov <georgi.djakov@linaro.org> > + */ > + > +#ifndef __DRIVERS_INTERCONNECT_QCOM_SMD_RPM_H > +#define __DRIVERS_INTERCONNECT_QCOM_SMD_RPM_H > + > +#include <linux/soc/qcom/smd-rpm.h> > + > +bool qcom_icc_rpm_smd_available(void); > +int qcom_icc_rpm_smd_send(int ctx, int rsc_type, int id, u32 val); > + > +#endif
On Tue 11 Jun 09:41 PDT 2019, Georgi Djakov wrote: > Add the DT nodes for the network-on-chip interconnect buses found > on qcs404-based platforms. > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > > v3: > - Updated according to the new binding: added reg property and moved under the > "soc" node. > > arch/arm64/boot/dts/qcom/qcs404.dtsi | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi > index ffedf9640af7..07ff592233b6 100644 > --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi > +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > // Copyright (c) 2018, Linaro Limited > > +#include <dt-bindings/interconnect/qcom,qcs404.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/clock/qcom,gcc-qcs404.h> > #include <dt-bindings/clock/qcom,rpmcc.h> > @@ -411,6 +412,33 @@ > #interrupt-cells = <4>; > }; > > + bimc: interconnect@400000 { Please maintain sort order of address, node name, label name. So this should go between rng@e3000 and remoteproc@b00000. Other than that: Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > + reg = <0x00400000 0x80000>; > + compatible = "qcom,qcs404-bimc"; > + #interconnect-cells = <1>; > + clock-names = "bus_clk", "bus_a_clk"; > + clocks = <&rpmcc RPM_SMD_BIMC_CLK>, > + <&rpmcc RPM_SMD_BIMC_A_CLK>; > + }; > + > + pcnoc: interconnect@500000 { > + reg = <0x00500000 0x15080>; > + compatible = "qcom,qcs404-pcnoc"; > + #interconnect-cells = <1>; > + clock-names = "bus_clk", "bus_a_clk"; > + clocks = <&rpmcc RPM_SMD_PNOC_CLK>, > + <&rpmcc RPM_SMD_PNOC_A_CLK>; > + }; > + > + snoc: interconnect@580000 { > + reg = <0x00580000 0x23080>; > + compatible = "qcom,qcs404-snoc"; > + #interconnect-cells = <1>; > + clock-names = "bus_clk", "bus_a_clk"; > + clocks = <&rpmcc RPM_SMD_SNOC_CLK>, > + <&rpmcc RPM_SMD_SNOC_A_CLK>; > + }; > + > sdcc1: sdcc@7804000 { > compatible = "qcom,sdhci-msm-v5"; > reg = <0x07804000 0x1000>, <0x7805000 0x1000>;