mbox series

[0/3] Qualcomm AOSS QMP side channel binding and driver

Message ID 20181112080557.22698-1-bjorn.andersson@linaro.org
Headers show
Series Qualcomm AOSS QMP side channel binding and driver | expand

Message

Bjorn Andersson Nov. 12, 2018, 8:05 a.m. UTC 
Add binding and driver for the QMP based side-channel communication mechanism
to the AOSS, which is used to control resources not exposed through the RPMh
interface.

Currently implemented is a genpd provider, but pending some improvements in the
thermal framework a cooling device will be added at a later point.

Bjorn Andersson (3):
  dt-bindings: soc: qcom: Add AOSS QMP binding
  soc: qcom: Add AOSS QMP communication driver
  soc: qcom: Add AOSS QMP genpd provider

 .../bindings/soc/qcom/qcom,aoss-qmp.txt       |  63 ++++
 drivers/soc/qcom/Kconfig                      |  15 +
 drivers/soc/qcom/Makefile                     |   2 +
 drivers/soc/qcom/aoss-qmp-pd.c                | 135 ++++++++
 drivers/soc/qcom/aoss-qmp.c                   | 313 ++++++++++++++++++
 include/dt-bindings/power/qcom-aoss-qmp.h     |  15 +
 include/linux/soc/qcom/aoss-qmp.h             |  12 +
 7 files changed, 555 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
 create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c
 create mode 100644 drivers/soc/qcom/aoss-qmp.c
 create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
 create mode 100644 include/linux/soc/qcom/aoss-qmp.h

-- 
2.18.0

Comments

Arun Kumar Neelakantam Jan. 3, 2019, 5:48 p.m. UTC | #1 
On 12/27/2018 1:58 AM, Bjorn Andersson wrote:
> On Tue 20 Nov 04:22 PST 2018, Arun Kumar Neelakantam wrote:

>

> Thanks for the review Arun.

>

>> On 11/12/2018 1:35 PM, Bjorn Andersson wrote:

> [..]

>>> +int qmp_send(struct qmp *qmp, const void *data, size_t len)

>>> +{

>>> +	int ret;

>>> +

>>> +	if (WARN_ON(len + sizeof(u32) > qmp->size)) {

>>> +		dev_err(qmp->dev, "message too long\n");

>>> +		return -EINVAL;

>>> +	}

>>> +

>>> +	if (WARN_ON(len % sizeof(u32))) {

>>> +		dev_err(qmp->dev, "message not 32-bit aligned\n");

>>> +		return -EINVAL;

>>> +	}

>>> +

>>> +	mutex_lock(&qmp->tx_lock);

>>> +

>>> +	if (!qmp_message_empty(qmp)) {

>>> +		dev_err(qmp->dev, "mailbox left busy\n");

>>> +		ret = -EINVAL;

>> should it be -EBUSY ?

> That makes more sense.

>

>> And qmp_messge_empty will be done either by remote if it process the data

>> else by this driver in TIMEOUT case, so does we need this check for every TX

>> ? I think we can just reset to Zero once in open time.

> Didn't think about that, should we really make the QMP link ready again

> when we get a timeout? Can we expect that the firmware of the remote

> side is ready to serve future messages?

>

>

> Should we keep this check and remove the writel() below?

I prefer we can just remove this check and keep writel() below same as 
down stream.
>

>>> +		goto out_unlock;

>>> +	}

>>> +

>>> +	/* 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;

>>> +

>>> +		writel(0, qmp->msgram + qmp->offset);

>>> +	} else {

>>> +		ret = 0;

>>> +	}

>>> +

>>> +out_unlock:

>>> +	mutex_unlock(&qmp->tx_lock);

>>> +

>>> +	return ret;

>>> +}

> Regards,

> Bjorn