| Message ID | 20190106080915.4493-1-bjorn.andersson@linaro.org |
|---|---|
| Headers | show |
| Series | Qualcomm AOSS QMP side channel binding and driver | expand |
Hi Bjorn, With the changes suggested below: Reviewed-by: Sibi Sankar <sibis@codeaurora.org> Tested-by: Sibi Sankar <sibis@codeaurora.org> On 2019-01-06 13:39, Bjorn Andersson wrote: > From: Rajendra Nayak <rnayak@codeaurora.org> > > With rpmh ARC resources being modelled as power domains with > performance > state, we need to proxy vote on these for SDM845. > Add support to vote on multiple of them, now that genpd supports > associating mutliple power domains to a device. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > [bjorn: Drop device link, improve error handling, name things "proxy"] > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > This is v3 of this patch, but updated to cover "loadstate". v2 can be > found here: > https://lore.kernel.org/lkml/20180904071046.8152-1-rnayak@codeaurora.org/ > > Changes since v2: > - Drop device links, as we can do active and proxy votes using device > links > - Improved error handling, by unrolling some votes on failure > - Rename things proxy, to follow naming of "proxy" and "active" > > drivers/remoteproc/qcom_q6v5_mss.c | 115 ++++++++++++++++++++++++++++- > 1 file changed, 111 insertions(+), 4 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c > b/drivers/remoteproc/qcom_q6v5_mss.c > index 01be7314e176..62cf16ddb7af 100644 > --- a/drivers/remoteproc/qcom_q6v5_mss.c > +++ b/drivers/remoteproc/qcom_q6v5_mss.c > @@ -25,6 +25,8 @@ > #include <linux/of_address.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > #include <linux/remoteproc.h> > @@ -131,6 +133,7 @@ struct rproc_hexagon_res { > char **proxy_clk_names; > char **reset_clk_names; > char **active_clk_names; > + char **proxy_pd_names; > int version; > bool need_mem_protection; > bool has_alt_reset; > @@ -156,9 +159,11 @@ struct q6v5 { > struct clk *active_clks[8]; > struct clk *reset_clks[4]; > struct clk *proxy_clks[4]; > + struct device *proxy_pds[3]; > int active_clk_count; > int reset_clk_count; > int proxy_clk_count; > + int proxy_pd_count; > > struct reg_info active_regs[1]; > struct reg_info proxy_regs[3]; > @@ -321,6 +326,41 @@ static void q6v5_clk_disable(struct device *dev, > clk_disable_unprepare(clks[i]); > } > > +static int q6v5_pds_enable(struct q6v5 *qproc, struct device **pds, > + size_t pd_count) > +{ > + int ret; > + int i; > + > + for (i = 0; i < pd_count; i++) { > + dev_pm_genpd_set_performance_state(pds[i], INT_MAX); > + ret = pm_runtime_get_sync(pds[i]); > + if (ret < 0) > + goto unroll_pd_votes; > + } > + > + return 0; > + > +unroll_pd_votes: > + for (i--; i >= 0; i--) { > + dev_pm_genpd_set_performance_state(pds[i], 0); > + pm_runtime_put(pds[i]); > + } > + > + return ret; > +}; > + > +static void q6v5_pds_disable(struct q6v5 *qproc, struct device **pds, > + size_t pd_count) > +{ > + int i; > + > + for (i = 0; i < pd_count; i++) { > + dev_pm_genpd_set_performance_state(pds[i], 0); > + pm_runtime_put(pds[i]); > + } > +} > + > static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int > *current_perm, > bool remote_owner, phys_addr_t addr, > size_t size) > @@ -690,11 +730,17 @@ static int q6v5_mba_load(struct q6v5 *qproc) > > qcom_q6v5_prepare(&qproc->q6v5); > > + ret = q6v5_pds_enable(qproc, qproc->proxy_pds, > qproc->proxy_pd_count); > + if (ret < 0) { > + dev_err(qproc->dev, "failed to enable proxy power domains\n"); > + goto disable_irqs; > + } > + > ret = q6v5_regulator_enable(qproc, qproc->proxy_regs, > qproc->proxy_reg_count); > if (ret) { > dev_err(qproc->dev, "failed to enable proxy supplies\n"); > - goto disable_irqs; > + goto disable_proxy_pds; > } > > ret = q6v5_clk_enable(qproc->dev, qproc->proxy_clks, > @@ -791,6 +837,8 @@ static int q6v5_mba_load(struct q6v5 *qproc) > disable_proxy_reg: > q6v5_regulator_disable(qproc, qproc->proxy_regs, > qproc->proxy_reg_count); > +disable_proxy_pds: > + q6v5_pds_disable(qproc, qproc->proxy_pds, qproc->proxy_pd_count); > disable_irqs: > qcom_q6v5_unprepare(&qproc->q6v5); > > @@ -1121,6 +1169,7 @@ static void qcom_msa_handover(struct qcom_q6v5 > *q6v5) > qproc->proxy_clk_count); > q6v5_regulator_disable(qproc, qproc->proxy_regs, > qproc->proxy_reg_count); > + q6v5_pds_disable(qproc, qproc->proxy_pds, qproc->proxy_pd_count); we should call proxy pds_disable on mba_reclaim as well: ret = qcom_q6v5_unprepare(&qproc->q6v5); if (ret) { ... q6v5_pds_disable(...); } > } > > static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device > *pdev) > @@ -1181,6 +1230,45 @@ static int q6v5_init_clocks(struct device *dev, > struct clk **clks, > return i; > } > > +static int q6v5_pds_attach(struct device *dev, struct device **devs, > + char **pd_names) > +{ > + size_t num_pds = 0; > + int ret; > + int i; > + > + if (!pd_names) > + return 0; > + > + while (pd_names[num_pds]) > + num_pds++; > + > + for (i = 0; i < num_pds; i++) { > + devs[i] = dev_pm_domain_attach_by_name(dev, pd_names[i]); > + if (IS_ERR(devs[i])) { > + ret = PTR_ERR(devs[i]); > + goto unroll_attach; > + } > + } > + > + return num_pds; > + > +unroll_attach: > + for (i--; i >= 0; i--) > + dev_pm_domain_detach(devs[i], false); > + > + return ret; > +}; > + > +static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds, > + size_t pd_count) > +{ > + int i; > + > + for (i = 0; i < pd_count; i++) > + dev_pm_domain_detach(pds[i], false); > +} > + > static int q6v5_init_reset(struct q6v5 *qproc) > { > qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev, > @@ -1322,10 +1410,18 @@ static int q6v5_probe(struct platform_device > *pdev) > } > qproc->active_reg_count = ret; > > + ret = q6v5_pds_attach(&pdev->dev, qproc->proxy_pds, > + desc->proxy_pd_names); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to init power domains\n"); Should be "Failed to attach proxy power domains" > + goto free_rproc; > + } > + qproc->proxy_pd_count = ret; > + > qproc->has_alt_reset = desc->has_alt_reset; > ret = q6v5_init_reset(qproc); > if (ret) > - goto free_rproc; > + goto detach_proxy_pds; > > qproc->version = desc->version; > qproc->need_mem_protection = desc->need_mem_protection; > @@ -1333,7 +1429,7 @@ static int q6v5_probe(struct platform_device > *pdev) > ret = qcom_q6v5_init(&qproc->q6v5, pdev, rproc, > MPSS_CRASH_REASON_SMEM, > qcom_msa_handover); > if (ret) > - goto free_rproc; > + goto detach_proxy_pds; > > qproc->mpss_perm = BIT(QCOM_SCM_VMID_HLOS); > qproc->mba_perm = BIT(QCOM_SCM_VMID_HLOS); > @@ -1344,10 +1440,12 @@ static int q6v5_probe(struct platform_device > *pdev) > > ret = rproc_add(rproc); > if (ret) > - goto free_rproc; > + goto detach_proxy_pds; > > return 0; > > +detach_proxy_pds: > + q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count); > free_rproc: > rproc_free(rproc); > > @@ -1364,6 +1462,9 @@ static int q6v5_remove(struct platform_device > *pdev) > qcom_remove_glink_subdev(qproc->rproc, &qproc->glink_subdev); > qcom_remove_smd_subdev(qproc->rproc, &qproc->smd_subdev); > qcom_remove_ssr_subdev(qproc->rproc, &qproc->ssr_subdev); > + > + q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count); > + > rproc_free(qproc->rproc); > > return 0; > @@ -1388,6 +1489,12 @@ static const struct rproc_hexagon_res sdm845_mss > = { > "mnoc_axi", > NULL > }, > + .proxy_pd_names = (char*[]){ > + "cx", > + "mx", > + "mss", > + NULL > + }, > .need_mem_protection = true, > .has_alt_reset = true, > .version = MSS_SDM845,
On 2019-01-06 13:39, Bjorn Andersson wrote: > The SDM845 MSS needs the load_state powerdomain voted for during the > duration of the MSS being powered on, to let the AOSS know that it may > not perform certain power save measures. So vote for this. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v1: > - New patch > Reviewed-by: Sibi Sankar <sibis@codeaurora.org> Tested-by: Sibi Sankar <sibis@codeaurora.org> > drivers/remoteproc/qcom_q6v5_mss.c | 31 ++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c > b/drivers/remoteproc/qcom_q6v5_mss.c > index 62cf16ddb7af..7f86d9c551c4 100644 > --- a/drivers/remoteproc/qcom_q6v5_mss.c > +++ b/drivers/remoteproc/qcom_q6v5_mss.c > @@ -133,6 +133,7 @@ struct rproc_hexagon_res { > char **proxy_clk_names; > char **reset_clk_names; > char **active_clk_names; > + char **active_pd_names; > char **proxy_pd_names; > int version; > bool need_mem_protection; > @@ -159,10 +160,12 @@ struct q6v5 { > struct clk *active_clks[8]; > struct clk *reset_clks[4]; > struct clk *proxy_clks[4]; > + struct device *active_pds[1]; > struct device *proxy_pds[3]; > int active_clk_count; > int reset_clk_count; > int proxy_clk_count; > + int active_pd_count; > int proxy_pd_count; > > struct reg_info active_regs[1]; > @@ -730,10 +733,16 @@ static int q6v5_mba_load(struct q6v5 *qproc) > > qcom_q6v5_prepare(&qproc->q6v5); > > + ret = q6v5_pds_enable(qproc, qproc->active_pds, > qproc->active_pd_count); > + if (ret < 0) { > + dev_err(qproc->dev, "failed to enable active power domains\n"); > + goto disable_irqs; > + } > + > ret = q6v5_pds_enable(qproc, qproc->proxy_pds, > qproc->proxy_pd_count); > if (ret < 0) { > dev_err(qproc->dev, "failed to enable proxy power domains\n"); > - goto disable_irqs; > + goto disable_active_pds; > } > > ret = q6v5_regulator_enable(qproc, qproc->proxy_regs, > @@ -839,6 +848,8 @@ static int q6v5_mba_load(struct q6v5 *qproc) > qproc->proxy_reg_count); > disable_proxy_pds: > q6v5_pds_disable(qproc, qproc->proxy_pds, qproc->proxy_pd_count); > +disable_active_pds: > + q6v5_pds_disable(qproc, qproc->active_pds, qproc->active_pd_count); > disable_irqs: > qcom_q6v5_unprepare(&qproc->q6v5); > > @@ -878,6 +889,7 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc) > qproc->active_clk_count); > q6v5_regulator_disable(qproc, qproc->active_regs, > qproc->active_reg_count); > + q6v5_pds_disable(qproc, qproc->active_pds, qproc->active_pd_count); > > /* In case of failure or coredump scenario where reclaiming MBA > memory > * could not happen reclaim it here. > @@ -1410,11 +1422,19 @@ static int q6v5_probe(struct platform_device > *pdev) > } > qproc->active_reg_count = ret; > > + ret = q6v5_pds_attach(&pdev->dev, qproc->active_pds, > + desc->active_pd_names); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to attach active power domains\n"); > + goto free_rproc; > + } > + qproc->active_pd_count = ret; > + > ret = q6v5_pds_attach(&pdev->dev, qproc->proxy_pds, > desc->proxy_pd_names); > if (ret < 0) { > dev_err(&pdev->dev, "Failed to init power domains\n"); > - goto free_rproc; > + goto detach_active_pds; > } > qproc->proxy_pd_count = ret; > > @@ -1446,6 +1466,8 @@ static int q6v5_probe(struct platform_device > *pdev) > > detach_proxy_pds: > q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count); > +detach_active_pds: > + q6v5_pds_detach(qproc, qproc->active_pds, qproc->active_pd_count); > free_rproc: > rproc_free(rproc); > > @@ -1463,6 +1485,7 @@ static int q6v5_remove(struct platform_device > *pdev) > qcom_remove_smd_subdev(qproc->rproc, &qproc->smd_subdev); > qcom_remove_ssr_subdev(qproc->rproc, &qproc->ssr_subdev); > > + q6v5_pds_detach(qproc, qproc->active_pds, qproc->active_pd_count); > q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count); > > rproc_free(qproc->rproc); > @@ -1489,6 +1512,10 @@ static const struct rproc_hexagon_res sdm845_mss > = { > "mnoc_axi", > NULL > }, > + .active_pd_names = (char*[]){ > + "load_state", > + NULL > + }, > .proxy_pd_names = (char*[]){ > "cx", > "mx",
Hi Bjorn, On 1/6/2019 1:39 PM, Bjorn Andersson wrote: > The AOSS QMP genpd provider implements control over power-related > resources related to low-power state associated with the remoteprocs in > the system as well as control over a set of clocks related to debug > hardware in the SoC. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v1: > - Drop compatible, as the QMP DT node is flattened > - Correct size of send > Thanks for the patch. I tested QDSS functionality with this on SDM845 and it works fine. Also posted the patch for the same here: - https://lore.kernel.org/patchwork/patch/1030510/ Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
On 1/6/2019 1:39 PM, Bjorn Andersson wrote: > The AOSS QMP driver is used to communicate with the AOSS for certain > side-channel requests, that are not enabled through the RPMh interface. > > The communication is a very simple synchronous mechanism of messages > being written in message RAM and a doorbell in the AOSS is rung. As the > AOSS has processed the message length is cleared and an interrupt is > fired by the AOSS as acknowledgment. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- Reviewed-by: Arun Kumar Neelakantam <aneela@codeaurora.org> > Changes since v1: > - Skip check in send for empty TX buffer > - Don't follow WARN_ON() with dev_err() > - Register platform_device rather than populate based on of-children > > drivers/soc/qcom/Kconfig | 9 + > drivers/soc/qcom/Makefile | 1 + > drivers/soc/qcom/aoss-qmp.c | 313 ++++++++++++++++++++++++++++++ > include/linux/soc/qcom/aoss-qmp.h | 12 ++ > 4 files changed, 335 insertions(+) > create mode 100644 drivers/soc/qcom/aoss-qmp.c > create mode 100644 include/linux/soc/qcom/aoss-qmp.h > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index a51458022d21..dda19471057f 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -3,6 +3,15 @@ > # > menu "Qualcomm SoC drivers" > > +config QCOM_AOSS_QMP > + tristate "Qualcomm AOSS Messaging Driver" > + depends on ARCH_QCOM || COMPILE_TEST > + depends on MAILBOX > + help > + This driver provides the means for communicating with the > + micro-controller in the AOSS, using QMP, to control certain resource > + that are not exposed through RPMh. > + > config QCOM_COMMAND_DB > bool "Qualcomm Command DB" > depends on ARCH_QCOM || COMPILE_TEST > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile > index 67cb85d0373c..d0d7fdc94d9a 100644 > --- a/drivers/soc/qcom/Makefile > +++ b/drivers/soc/qcom/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > CFLAGS_rpmh-rsc.o := -I$(src) > +obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o > obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o > obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o > obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o > diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c > new file mode 100644 > index 000000000000..de52703b96b6 > --- /dev/null > +++ b/drivers/soc/qcom/aoss-qmp.c > @@ -0,0 +1,313 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, Linaro Ltd > + */ > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/mailbox_client.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/soc/qcom/aoss-qmp.h> > + > +#define QMP_DESC_MAGIC 0x0 > +#define QMP_DESC_VERSION 0x4 > +#define QMP_DESC_FEATURES 0x8 > + > +#define QMP_DESC_UCORE_LINK_STATE 0xc > +#define QMP_DESC_UCORE_LINK_STATE_ACK 0x10 > +#define QMP_DESC_UCORE_CH_STATE 0x14 > +#define QMP_DESC_UCORE_CH_STATE_ACK 0x18 > +#define QMP_DESC_UCORE_MBOX_SIZE 0x1c > +#define QMP_DESC_UCORE_MBOX_OFFSET 0x20 > + > +#define QMP_DESC_MCORE_LINK_STATE 0x24 > +#define QMP_DESC_MCORE_LINK_STATE_ACK 0x28 > +#define QMP_DESC_MCORE_CH_STATE 0x2c > +#define QMP_DESC_MCORE_CH_STATE_ACK 0x30 > +#define QMP_DESC_MCORE_MBOX_SIZE 0x34 > +#define QMP_DESC_MCORE_MBOX_OFFSET 0x38 > + > +#define QMP_STATE_UP 0x0000ffff > +#define QMP_STATE_DOWN 0xffff0000 > + > +#define QMP_MAGIC 0x4d41494c > +#define QMP_VERSION 1 > + > +/** > + * struct qmp - driver state for QMP implementation > + * @msgram: iomem referencing the message RAM used for communication > + * @dev: reference to QMP device > + * @mbox_client: mailbox client used to ring the doorbell on transmit > + * @mbox_chan: mailbox channel used to ring the doorbell on transmit > + * @offset: offset within @msgram where messages should be written > + * @size: maximum size of the messages to be transmitted > + * @event: wait_queue for synchronization with the IRQ > + * @tx_lock: provides syncrhonization between multiple callers of qmp_send() > + * @pd_pdev: platform device for the power-domain child device > + */ > +struct qmp { > + void __iomem *msgram; > + struct device *dev; > + > + struct mbox_client mbox_client; > + struct mbox_chan *mbox_chan; > + > + size_t offset; > + size_t size; > + > + wait_queue_head_t event; > + > + struct mutex tx_lock; > + > + struct platform_device *pd_pdev; > +}; > + > +static void qmp_kick(struct qmp *qmp) > +{ > + mbox_send_message(qmp->mbox_chan, NULL); > + mbox_client_txdone(qmp->mbox_chan, 0); > +} > + > +static bool qmp_magic_valid(struct qmp *qmp) > +{ > + return readl(qmp->msgram + QMP_DESC_MAGIC) == QMP_MAGIC; > +} > + > +static bool qmp_link_acked(struct qmp *qmp) > +{ > + return readl(qmp->msgram + QMP_DESC_MCORE_LINK_STATE_ACK) == QMP_STATE_UP; > +} > + > +static bool qmp_mcore_channel_acked(struct qmp *qmp) > +{ > + return readl(qmp->msgram + QMP_DESC_MCORE_CH_STATE_ACK) == QMP_STATE_UP; > +} > + > +static bool qmp_ucore_channel_up(struct qmp *qmp) > +{ > + return readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE) == QMP_STATE_UP; > +} > + > +static int qmp_open(struct qmp *qmp) > +{ > + int ret; > + u32 val; > + > + ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ); > + if (!ret) { > + dev_err(qmp->dev, "QMP magic doesn't match\n"); > + return -ETIMEDOUT; > + } > + > + val = readl(qmp->msgram + QMP_DESC_VERSION); > + if (val != QMP_VERSION) { > + dev_err(qmp->dev, "unsupported QMP version %d\n", val); > + return -EINVAL; > + } > + > + qmp->offset = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_OFFSET); > + qmp->size = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_SIZE); > + if (!qmp->size) { > + dev_err(qmp->dev, "invalid mailbox size 0x%zx\n", qmp->size); > + return -EINVAL; > + } > + > + /* Ack remote core's link state */ > + val = readl(qmp->msgram + QMP_DESC_UCORE_LINK_STATE); > + writel(val, qmp->msgram + QMP_DESC_UCORE_LINK_STATE_ACK); > + > + /* Set local core's link state to up */ > + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); > + > + qmp_kick(qmp); > + > + ret = wait_event_timeout(qmp->event, qmp_link_acked(qmp), HZ); > + if (!ret) { > + dev_err(qmp->dev, "ucore didn't ack link\n"); > + goto timeout_close_link; > + } > + > + writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_CH_STATE); > + > + ret = wait_event_timeout(qmp->event, qmp_ucore_channel_up(qmp), HZ); > + if (!ret) { > + dev_err(qmp->dev, "ucore didn't open channel\n"); > + goto timeout_close_channel; > + } > + > + /* Ack remote core's channel state */ > + val = readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE); > + writel(val, qmp->msgram + QMP_DESC_UCORE_CH_STATE_ACK); > + > + qmp_kick(qmp); > + > + ret = wait_event_timeout(qmp->event, qmp_mcore_channel_acked(qmp), HZ); > + if (!ret) { > + dev_err(qmp->dev, "ucore didn't ack channel\n"); > + goto timeout_close_channel; > + } > + > + return 0; > + > +timeout_close_channel: > + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE); > + > +timeout_close_link: > + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); > + qmp_kick(qmp); > + > + return -ETIMEDOUT; > +} > + > +static void qmp_close(struct qmp *qmp) > +{ > + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE); > + writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE); > + qmp_kick(qmp); > +} > + > +static irqreturn_t qmp_intr(int irq, void *data) > +{ > + struct qmp *qmp = data; > + > + wake_up_interruptible_all(&qmp->event); > + > + return IRQ_HANDLED; > +} > + > +static bool qmp_message_empty(struct qmp *qmp) > +{ > + return readl(qmp->msgram + qmp->offset) == 0; > +} > + > +/** > + * qmp_send() - send a message to the AOSS > + * @qmp: qmp context > + * @data: message to be sent > + * @len: length of the message > + * > + * Transmit @data to AOSS and wait for the AOSS to acknowledge the message. > + * @len must be a multiple of 4 and not longer than the mailbox size. Access is > + * synchronized by this implementation. > + * > + * Return: 0 on success, negative errno on failure > + */ > +int qmp_send(struct qmp *qmp, const void *data, size_t len) > +{ > + int ret; > + > + if (WARN_ON(len + sizeof(u32) > qmp->size)) > + return -EINVAL; > + > + if (WARN_ON(len % sizeof(u32))) > + return -EINVAL; > + > + mutex_lock(&qmp->tx_lock); > + > + /* The message RAM only implements 32-bit accesses */ > + __iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32), > + data, len / sizeof(u32)); > + writel(len, qmp->msgram + qmp->offset); > + qmp_kick(qmp); > + > + ret = wait_event_interruptible_timeout(qmp->event, > + qmp_message_empty(qmp), HZ); > + if (!ret) { > + dev_err(qmp->dev, "ucore did not ack channel\n"); > + ret = -ETIMEDOUT; > + > + /* Clear message from buffer */ > + writel(0, qmp->msgram + qmp->offset); > + } else { > + ret = 0; > + } > + > + mutex_unlock(&qmp->tx_lock); > + > + return ret; > +} > +EXPORT_SYMBOL(qmp_send); > + > +static int qmp_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + struct qmp *qmp; > + int irq; > + int ret; > + > + qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL); > + if (!qmp) > + return -ENOMEM; > + > + qmp->dev = &pdev->dev; > + init_waitqueue_head(&qmp->event); > + mutex_init(&qmp->tx_lock); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + qmp->msgram = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(qmp->msgram)) > + return PTR_ERR(qmp->msgram); > + > + qmp->mbox_client.dev = &pdev->dev; > + qmp->mbox_client.knows_txdone = true; > + qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0); > + if (IS_ERR(qmp->mbox_chan)) { > + dev_err(&pdev->dev, "failed to acquire ipc mailbox\n"); > + return PTR_ERR(qmp->mbox_chan); > + } > + > + irq = platform_get_irq(pdev, 0); > + ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT, > + "aoss-qmp", qmp); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request interrupt\n"); > + return ret; > + } > + > + ret = qmp_open(qmp); > + if (ret < 0) > + return ret; > + > + platform_set_drvdata(pdev, qmp); > + > + if (of_property_read_bool(pdev->dev.of_node, "#power-domain-cells")) { > + qmp->pd_pdev = platform_device_register_data(&pdev->dev, > + "aoss_qmp_pd", > + PLATFORM_DEVID_NONE, > + NULL, 0); > + if (IS_ERR(qmp->pd_pdev)) > + dev_err(&pdev->dev, "failed to register AOSS PD\n"); > + } > + > + return 0; > +} > + > +static int qmp_remove(struct platform_device *pdev) > +{ > + struct qmp *qmp = platform_get_drvdata(pdev); > + > + platform_device_unregister(qmp->pd_pdev); > + > + qmp_close(qmp); > + > + return 0; > +} > + > +static const struct of_device_id qmp_dt_match[] = { > + { .compatible = "qcom,sdm845-aoss-qmp", }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, qmp_dt_match); > + > +static struct platform_driver qmp_driver = { > + .driver = { > + .name = "aoss_qmp", > + .of_match_table = qmp_dt_match, > + }, > + .probe = qmp_probe, > + .remove = qmp_remove, > +}; > +module_platform_driver(qmp_driver); > + > +MODULE_DESCRIPTION("Qualcomm AOSS QMP driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/soc/qcom/aoss-qmp.h b/include/linux/soc/qcom/aoss-qmp.h > new file mode 100644 > index 000000000000..32ccaa091a9f > --- /dev/null > +++ b/include/linux/soc/qcom/aoss-qmp.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2018, Linaro Ltd > + */ > +#ifndef __AOP_QMP_H__ > +#define __AOP_QMP_H__ > + > +struct qmp; > + > +int qmp_send(struct qmp *qmp, const void *data, size_t len); > + > +#endif
Quoting Bjorn Andersson (2019-01-06 00:09:10) > diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c > new file mode 100644 > index 000000000000..de52703b96b6 > --- /dev/null > +++ b/drivers/soc/qcom/aoss-qmp.c > @@ -0,0 +1,313 @@ [...] > + > +static int qmp_probe(struct platform_device *pdev) > +{ > + struct resource *res; > + struct qmp *qmp; > + int irq; > + int ret; > + > + qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL); > + if (!qmp) > + return -ENOMEM; > + > + qmp->dev = &pdev->dev; > + init_waitqueue_head(&qmp->event); > + mutex_init(&qmp->tx_lock); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + qmp->msgram = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(qmp->msgram)) > + return PTR_ERR(qmp->msgram); > + > + qmp->mbox_client.dev = &pdev->dev; > + qmp->mbox_client.knows_txdone = true; > + qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0); > + if (IS_ERR(qmp->mbox_chan)) { > + dev_err(&pdev->dev, "failed to acquire ipc mailbox\n"); > + return PTR_ERR(qmp->mbox_chan); > + } > + > + irq = platform_get_irq(pdev, 0); > + ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT, > + "aoss-qmp", qmp); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request interrupt\n"); Don't we need to free_mbox_channel() all over the place here? > + return ret; > + } > + > + ret = qmp_open(qmp); > + if (ret < 0) > + return ret; > + > + platform_set_drvdata(pdev, qmp); > + > + if (of_property_read_bool(pdev->dev.of_node, "#power-domain-cells")) { > + qmp->pd_pdev = platform_device_register_data(&pdev->dev, > + "aoss_qmp_pd", > + PLATFORM_DEVID_NONE, > + NULL, 0); > + if (IS_ERR(qmp->pd_pdev)) > + dev_err(&pdev->dev, "failed to register AOSS PD\n"); > + } > + > + return 0; > +} [...] > diff --git a/include/linux/soc/qcom/aoss-qmp.h b/include/linux/soc/qcom/aoss-qmp.h > new file mode 100644 > index 000000000000..32ccaa091a9f > --- /dev/null > +++ b/include/linux/soc/qcom/aoss-qmp.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2018, Linaro Ltd > + */ > +#ifndef __AOP_QMP_H__ > +#define __AOP_QMP_H__ include <linux/types.h> for size_t usage? > + > +struct qmp; > + > +int qmp_send(struct qmp *qmp, const void *data, size_t len); > +
Quoting Bjorn Andersson (2019-01-06 00:09:11) > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index dda19471057f..d81256ef5055 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -12,6 +12,15 @@ config QCOM_AOSS_QMP > micro-controller in the AOSS, using QMP, to control certain resource > that are not exposed through RPMh. > > +config QCOM_AOSS_QMP_PD > + tristate "Qualcomm AOSS Messaging Power Domain driver" > + depends on QCOM_AOSS_QMP > + select PM_GENERIC_DOMAINS > + help > + This driver provides the means of controlling the AOSSs handling of Is that possesive? AOSS's? > + low-power state for resources related to the remoteproc subsystems as > + well as controlling the debug clocks. > + > config QCOM_COMMAND_DB > bool "Qualcomm Command DB" > depends on ARCH_QCOM || COMPILE_TEST > diff --git a/drivers/soc/qcom/aoss-qmp-pd.c b/drivers/soc/qcom/aoss-qmp-pd.c > new file mode 100644 > index 000000000000..62b8fcb9d09e > --- /dev/null > +++ b/drivers/soc/qcom/aoss-qmp-pd.c > @@ -0,0 +1,135 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, Linaro Ltd > + */ > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pm_domain.h> > +#include <linux/soc/qcom/aoss-qmp.h> > +#include <dt-bindings/power/qcom-aoss-qmp.h> > + > +struct qmp_pd { > + struct qmp *qmp; > + > + struct generic_pm_domain pd; > + > + const char *name; > +}; > + > +#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd) > + > +struct qmp_pd_resource { > + const char *name; > + int (*on)(struct generic_pm_domain *domain); > + int (*off)(struct generic_pm_domain *domain); > +}; > + > +static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable) > +{ > + char buf[96]; Is 96 calculated from somewhere? Can it be a define because it's quite magical.
On Mon 14 Jan 14:36 PST 2019, Stephen Boyd wrote: > Quoting Bjorn Andersson (2019-01-06 00:09:10) > > diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c > > new file mode 100644 > > index 000000000000..de52703b96b6 > > --- /dev/null > > +++ b/drivers/soc/qcom/aoss-qmp.c > > @@ -0,0 +1,313 @@ > [...] > > + > > +static int qmp_probe(struct platform_device *pdev) > > +{ > > + struct resource *res; > > + struct qmp *qmp; > > + int irq; > > + int ret; > > + > > + qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL); > > + if (!qmp) > > + return -ENOMEM; > > + > > + qmp->dev = &pdev->dev; > > + init_waitqueue_head(&qmp->event); > > + mutex_init(&qmp->tx_lock); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + qmp->msgram = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(qmp->msgram)) > > + return PTR_ERR(qmp->msgram); > > + > > + qmp->mbox_client.dev = &pdev->dev; > > + qmp->mbox_client.knows_txdone = true; > > + qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0); > > + if (IS_ERR(qmp->mbox_chan)) { > > + dev_err(&pdev->dev, "failed to acquire ipc mailbox\n"); > > + return PTR_ERR(qmp->mbox_chan); > > + } > > + > > + irq = platform_get_irq(pdev, 0); > > + ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT, > > + "aoss-qmp", qmp); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "failed to request interrupt\n"); > > Don't we need to free_mbox_channel() all over the place here? > We do, thanks for spotting this. [..] > > diff --git a/include/linux/soc/qcom/aoss-qmp.h b/include/linux/soc/qcom/aoss-qmp.h > > new file mode 100644 > > index 000000000000..32ccaa091a9f > > --- /dev/null > > +++ b/include/linux/soc/qcom/aoss-qmp.h > > @@ -0,0 +1,12 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (c) 2018, Linaro Ltd > > + */ > > +#ifndef __AOP_QMP_H__ > > +#define __AOP_QMP_H__ > > include <linux/types.h> for size_t usage? > That wouldn't hurt. > > + > > +struct qmp; > > + > > +int qmp_send(struct qmp *qmp, const void *data, size_t len); > > + Thanks, Bjorn
On Mon 14 Jan 14:40 PST 2019, Stephen Boyd wrote: > Quoting Bjorn Andersson (2019-01-06 00:09:11) > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > > index dda19471057f..d81256ef5055 100644 > > --- a/drivers/soc/qcom/Kconfig > > +++ b/drivers/soc/qcom/Kconfig > > @@ -12,6 +12,15 @@ config QCOM_AOSS_QMP > > micro-controller in the AOSS, using QMP, to control certain resource > > that are not exposed through RPMh. > > > > +config QCOM_AOSS_QMP_PD > > + tristate "Qualcomm AOSS Messaging Power Domain driver" > > + depends on QCOM_AOSS_QMP > > + select PM_GENERIC_DOMAINS > > + help > > + This driver provides the means of controlling the AOSSs handling of > > Is that possesive? AOSS's? > That's correct, will fix. > > + low-power state for resources related to the remoteproc subsystems as > > + well as controlling the debug clocks. > > + > > config QCOM_COMMAND_DB > > bool "Qualcomm Command DB" > > depends on ARCH_QCOM || COMPILE_TEST > > diff --git a/drivers/soc/qcom/aoss-qmp-pd.c b/drivers/soc/qcom/aoss-qmp-pd.c > > new file mode 100644 > > index 000000000000..62b8fcb9d09e > > --- /dev/null > > +++ b/drivers/soc/qcom/aoss-qmp-pd.c > > @@ -0,0 +1,135 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018, Linaro Ltd > > + */ > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_domain.h> > > +#include <linux/soc/qcom/aoss-qmp.h> > > +#include <dt-bindings/power/qcom-aoss-qmp.h> > > + > > +struct qmp_pd { > > + struct qmp *qmp; > > + > > + struct generic_pm_domain pd; > > + > > + const char *name; > > +}; > > + > > +#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd) > > + > > +struct qmp_pd_resource { > > + const char *name; > > + int (*on)(struct generic_pm_domain *domain); > > + int (*off)(struct generic_pm_domain *domain); > > +}; > > + > > +static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable) > > +{ > > + char buf[96]; > > Is 96 calculated from somewhere? Can it be a define because it's quite > magical. > This is inherited by downstream, I know it has to be % 4, I'll check and update accordingly. Thanks, Bjorn