All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Rowand <frowand.list@gmail.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Ohad Ben-Cohen <ohad@wizery.com>,
	linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-remoteproc@vger.kernel.org,
	Nicolas Dechesne <nicolas.dechesne@linaro.org>
Subject: Re: [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework
Date: Thu, 30 Aug 2018 20:57:57 -0700	[thread overview]
Message-ID: <c6e2e87c-e2fd-c0ea-dbb9-5246b8694272@gmail.com> (raw)
In-Reply-To: <20180420011757.22389-1-bjorn.andersson@linaro.org>

Hi Bjorn,


On 04/19/18 18:17, Bjorn Andersson wrote:
> Attempt to acquire the APCS IPC through the mailbox framework and fall
> back to the old syscon based approach, to allow us to move away from
> using the syscon.
> 
> Reviewed-by: Arun Kumar Neelakantam <aneela@codeaurora.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - Added comment about mbox_send_message() return value.
> 
>  .../devicetree/bindings/soc/qcom/qcom,smd.txt      |  8 ++-
>  drivers/rpmsg/Kconfig                              |  1 +
>  drivers/rpmsg/qcom_smd.c                           | 67 ++++++++++++++++------
>  3 files changed, 56 insertions(+), 20 deletions(-)

This patch in the mainline Linux kernel as commit ab460a2e72dabecfdabd45eb7e3ee2d73fc876d4
causes a problem with the APQ8074 Dragonboard.  The mmc device is not set up
with the patch applied, thus I do not have the block device my root file system
is located on.

Testing on v4.18, if I revert this commit the mmc device is available.

I'll reply to this email with the console messages for 4.18 and for 4.18 with
this commit reverted.

-Frank


> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> index ea1dc75ec9ea..234ae2256501 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smd.txt
> @@ -22,9 +22,15 @@ The edge is described by the following properties:
>  	Definition: should specify the IRQ used by the remote processor to
>  		    signal this processor about communication related updates
>  
> -- qcom,ipc:
> +- mboxes:
>  	Usage: required
>  	Value type: <prop-encoded-array>
> +	Definition: reference to the associated doorbell in APCS, as described
> +		    in mailbox/mailbox.txt
> +
> +- qcom,ipc:
> +	Usage: required, unless mboxes is specified
> +	Value type: <prop-encoded-array>
>  	Definition: three entries specifying the outgoing ipc bit used for
>  		    signaling the remote processor:
>  		    - phandle to a syscon node representing the apcs registers
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> index 0fe6eac46512..2e4fb4ffd562 100644
> --- a/drivers/rpmsg/Kconfig
> +++ b/drivers/rpmsg/Kconfig
> @@ -39,6 +39,7 @@ config RPMSG_QCOM_GLINK_SMEM
>  
>  config RPMSG_QCOM_SMD
>  	tristate "Qualcomm Shared Memory Driver (SMD)"
> +	depends on MAILBOX
>  	depends on QCOM_SMEM
>  	select RPMSG
>  	help
> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> index bc0b30657230..3ff271a44bef 100644
> --- a/drivers/rpmsg/qcom_smd.c
> +++ b/drivers/rpmsg/qcom_smd.c
> @@ -14,6 +14,7 @@
>  
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/mailbox_client.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of_irq.h>
> @@ -107,6 +108,8 @@ static const struct {
>   * @ipc_regmap:		regmap handle holding the outgoing ipc register
>   * @ipc_offset:		offset within @ipc_regmap of the register for ipc
>   * @ipc_bit:		bit in the register at @ipc_offset of @ipc_regmap
> + * @mbox_client:	mailbox client handle
> + * @mbox_chan:		apcs ipc mailbox channel handle
>   * @channels:		list of all channels detected on this edge
>   * @channels_lock:	guard for modifications of @channels
>   * @allocated:		array of bitmaps representing already allocated channels
> @@ -129,6 +132,9 @@ struct qcom_smd_edge {
>  	int ipc_offset;
>  	int ipc_bit;
>  
> +	struct mbox_client mbox_client;
> +	struct mbox_chan *mbox_chan;
> +
>  	struct list_head channels;
>  	spinlock_t channels_lock;
>  
> @@ -366,7 +372,17 @@ static void qcom_smd_signal_channel(struct qcom_smd_channel *channel)
>  {
>  	struct qcom_smd_edge *edge = channel->edge;
>  
> -	regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
> +	if (edge->mbox_chan) {
> +		/*
> +		 * We can ignore a failing mbox_send_message() as the only
> +		 * possible cause is that the FIFO in the framework is full of
> +		 * other writes to the same bit.
> +		 */
> +		mbox_send_message(edge->mbox_chan, NULL);
> +		mbox_client_txdone(edge->mbox_chan, 0);
> +	} else {
> +		regmap_write(edge->ipc_regmap, edge->ipc_offset, BIT(edge->ipc_bit));
> +	}
>  }
>  
>  /*
> @@ -1326,27 +1342,37 @@ static int qcom_smd_parse_edge(struct device *dev,
>  	key = "qcom,remote-pid";
>  	of_property_read_u32(node, key, &edge->remote_pid);
>  
> -	syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
> -	if (!syscon_np) {
> -		dev_err(dev, "no qcom,ipc node\n");
> -		return -ENODEV;
> -	}
> +	edge->mbox_client.dev = dev;
> +	edge->mbox_client.knows_txdone = true;
> +	edge->mbox_chan = mbox_request_channel(&edge->mbox_client, 0);
> +	if (IS_ERR(edge->mbox_chan)) {
> +		if (PTR_ERR(edge->mbox_chan) != -ENODEV)
> +			return PTR_ERR(edge->mbox_chan);
>  
> -	edge->ipc_regmap = syscon_node_to_regmap(syscon_np);
> -	if (IS_ERR(edge->ipc_regmap))
> -		return PTR_ERR(edge->ipc_regmap);
> +		edge->mbox_chan = NULL;
>  
> -	key = "qcom,ipc";
> -	ret = of_property_read_u32_index(node, key, 1, &edge->ipc_offset);
> -	if (ret < 0) {
> -		dev_err(dev, "no offset in %s\n", key);
> -		return -EINVAL;
> -	}
> +		syscon_np = of_parse_phandle(node, "qcom,ipc", 0);
> +		if (!syscon_np) {
> +			dev_err(dev, "no qcom,ipc node\n");
> +			return -ENODEV;
> +		}
>  
> -	ret = of_property_read_u32_index(node, key, 2, &edge->ipc_bit);
> -	if (ret < 0) {
> -		dev_err(dev, "no bit in %s\n", key);
> -		return -EINVAL;
> +		edge->ipc_regmap = syscon_node_to_regmap(syscon_np);
> +		if (IS_ERR(edge->ipc_regmap))
> +			return PTR_ERR(edge->ipc_regmap);
> +
> +		key = "qcom,ipc";
> +		ret = of_property_read_u32_index(node, key, 1, &edge->ipc_offset);
> +		if (ret < 0) {
> +			dev_err(dev, "no offset in %s\n", key);
> +			return -EINVAL;
> +		}
> +
> +		ret = of_property_read_u32_index(node, key, 2, &edge->ipc_bit);
> +		if (ret < 0) {
> +			dev_err(dev, "no bit in %s\n", key);
> +			return -EINVAL;
> +		}
>  	}
>  
>  	ret = of_property_read_string(node, "label", &edge->name);
> @@ -1452,6 +1478,8 @@ struct qcom_smd_edge *qcom_smd_register_edge(struct device *parent,
>  	return edge;
>  
>  unregister_dev:
> +	if (!IS_ERR_OR_NULL(edge->mbox_chan))
> +		mbox_free_channel(edge->mbox_chan);
>  	put_device(&edge->dev);
>  	return ERR_PTR(ret);
>  }
> @@ -1480,6 +1508,7 @@ int qcom_smd_unregister_edge(struct qcom_smd_edge *edge)
>  	if (ret)
>  		dev_warn(&edge->dev, "can't remove smd device: %d\n", ret);
>  
> +	mbox_free_channel(edge->mbox_chan);
>  	device_unregister(&edge->dev);
>  
>  	return 0;
> 

  parent reply	other threads:[~2018-08-31  3:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20  1:17 [PATCH v3] rpmsg: qcom_smd: Access APCS through mailbox framework Bjorn Andersson
2018-04-27 13:37 ` Rob Herring
2018-08-31  3:57 ` Frank Rowand [this message]
2018-08-31  4:04   ` Frank Rowand
2018-08-31  4:05   ` Frank Rowand
2018-08-31  4:07   ` Bjorn Andersson
2018-08-31 20:41     ` Frank Rowand
2018-08-31 20:55       ` Bjorn Andersson
2018-08-31 21:34         ` Frank Rowand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c6e2e87c-e2fd-c0ea-dbb9-5246b8694272@gmail.com \
    --to=frowand.list@gmail.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=linux-soc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nicolas.dechesne@linaro.org \
    --cc=ohad@wizery.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.