All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/4] add Tegra210 BPMP driver
@ 2019-01-21 12:28 Timo Alho
  2019-01-21 12:28 ` [PATCH V2 1/4] firmware: tegra: reword messaging terminology Timo Alho
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Timo Alho @ 2019-01-21 12:28 UTC (permalink / raw)
  To: treding, sivaramn, robh; +Cc: linux-tegra, devicetree, Timo Alho

Hi,

This is a second version of "add Tegra210 BPMP driver" series. It has
only some minor updates to original series based on feedback received
from Sivaram Nair and Rob Herring. The original cover letter of patch
series one is below.

This series adds a driver for Tegra210 BPMP firmware. The BPMP is a
specific processor in Tegra210 chip, which runs firmware for assisting
in entering deep low power states (suspend to ram), and offloading
DRAM memory clock scaling on some platforms.

The series consist of first refactoring the existing BPMP driver by
first renaming the terminology used for communication, then splitting
out the Tegra186 chip specific parts, and finally adding the needed
functionality for Tegra210.

Timo Alho (4):
  firmware: tegra: reword messaging terminology
  firmware: tegra: refactor bpmp driver
  firmware: tegra: add bpmp driver for Tegra210
  dt-bindings: firmware: Add bindings for Tegra210 BPMP

 .../bindings/firmware/nvidia,tegra210-bpmp.txt     |  36 +++
 drivers/firmware/tegra/Makefile                    |   2 +
 drivers/firmware/tegra/bpmp-private.h              |  29 ++
 drivers/firmware/tegra/bpmp-t186.c                 | 303 +++++++++++++++++
 drivers/firmware/tegra/bpmp-t210.c                 | 241 ++++++++++++++
 drivers/firmware/tegra/bpmp.c                      | 359 ++++++++-------------
 include/soc/tegra/bpmp.h                           |  13 +-
 7 files changed, 751 insertions(+), 232 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
 create mode 100644 drivers/firmware/tegra/bpmp-private.h
 create mode 100644 drivers/firmware/tegra/bpmp-t186.c
 create mode 100644 drivers/firmware/tegra/bpmp-t210.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH V2 1/4] firmware: tegra: reword messaging terminology
  2019-01-21 12:28 [PATCH V2 0/4] add Tegra210 BPMP driver Timo Alho
@ 2019-01-21 12:28 ` Timo Alho
  2019-01-24 11:33   ` Jon Hunter
  2019-01-21 12:28 ` [PATCH V2 2/4] firmware: tegra: refactor bpmp driver Timo Alho
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Timo Alho @ 2019-01-21 12:28 UTC (permalink / raw)
  To: treding, sivaramn, robh; +Cc: linux-tegra, devicetree, Timo Alho

As a preparatory change to refactor bpmp driver to support other than
t186/t194 chip generations, reword and slightly refactor some of the
functions to better match with what is actually happening in the
wire-level protocol.

The communication with bpmp is essentially a Remote Procedure Call
consisting of "request" and "response". Either side (BPMP or CPU) can
initiate the communication. The state machine for communication
consists of following steps (from Linux point of view):

Linux initiating the call:
 1) check that channel is free to transmit a request (is_req_channel_free)
 2) copy request message payload to shared location
 3) post the request in channel (post_req)
 4) notify BPMP that channel state has been updated (ring_doorbell)
 5) wait for response (is_resp_ready)
 6) copy response message payload from shared location
 7) acknowledge the response in channel (ack_resp)

BPMP initiating the call:
 1) wait for request (is_req_ready)
 2) copy request message payload from shared location
 3) acknowledge the request in channel (ack_req)
 4) check that channel is free to transmit response (is_resp_channel_free)
 5) copy response message payload to shared location
 6) post the response message to channel (post_resp)
 7) notify BPMP that channel state has been updated (ring_doorbell)

Signed-off-by: Timo Alho <talho@nvidia.com>
---
 drivers/firmware/tegra/bpmp.c | 106 +++++++++++++++++++++++++++---------------
 1 file changed, 68 insertions(+), 38 deletions(-)

diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index 689478b..af123de 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -96,7 +96,7 @@ static bool tegra_bpmp_message_valid(const struct tegra_bpmp_message *msg)
 	       (msg->rx.size == 0 || msg->rx.data);
 }
 
-static bool tegra_bpmp_master_acked(struct tegra_bpmp_channel *channel)
+static bool tegra_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
 {
 	void *frame;
 
@@ -111,7 +111,12 @@ static bool tegra_bpmp_master_acked(struct tegra_bpmp_channel *channel)
 	return true;
 }
 
-static int tegra_bpmp_wait_ack(struct tegra_bpmp_channel *channel)
+static bool tegra_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
+{
+	return tegra_bpmp_is_resp_ready(channel);
+}
+
+static int tegra_bpmp_wait_resp(struct tegra_bpmp_channel *channel)
 {
 	unsigned long timeout = channel->bpmp->soc->channels.cpu_tx.timeout;
 	ktime_t end;
@@ -119,14 +124,24 @@ static int tegra_bpmp_wait_ack(struct tegra_bpmp_channel *channel)
 	end = ktime_add_us(ktime_get(), timeout);
 
 	do {
-		if (tegra_bpmp_master_acked(channel))
+		if (tegra_bpmp_is_resp_ready(channel))
 			return 0;
 	} while (ktime_before(ktime_get(), end));
 
 	return -ETIMEDOUT;
 }
 
-static bool tegra_bpmp_master_free(struct tegra_bpmp_channel *channel)
+static int tegra_bpmp_ack_resp(struct tegra_bpmp_channel *channel)
+{
+	return tegra_ivc_read_advance(channel->ivc);
+}
+
+static int tegra_bpmp_ack_req(struct tegra_bpmp_channel *channel)
+{
+	return tegra_ivc_read_advance(channel->ivc);
+}
+
+static bool tegra_bpmp_is_req_channel_free(struct tegra_bpmp_channel *channel)
 {
 	void *frame;
 
@@ -141,7 +156,12 @@ static bool tegra_bpmp_master_free(struct tegra_bpmp_channel *channel)
 	return true;
 }
 
-static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
+static bool tegra_bpmp_is_resp_channel_free(struct tegra_bpmp_channel *channel)
+{
+	return tegra_bpmp_is_req_channel_free(channel);
+}
+
+static int tegra_bpmp_wait_req_channel_free(struct tegra_bpmp_channel *channel)
 {
 	unsigned long timeout = channel->bpmp->soc->channels.cpu_tx.timeout;
 	ktime_t start, now;
@@ -149,7 +169,7 @@ static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
 	start = ns_to_ktime(local_clock());
 
 	do {
-		if (tegra_bpmp_master_free(channel))
+		if (tegra_bpmp_is_req_channel_free(channel))
 			return 0;
 
 		now = ns_to_ktime(local_clock());
@@ -158,6 +178,32 @@ static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
 	return -ETIMEDOUT;
 }
 
+static int tegra_bpmp_post_req(struct tegra_bpmp_channel *channel)
+{
+	return tegra_ivc_write_advance(channel->ivc);
+}
+
+static int tegra_bpmp_post_resp(struct tegra_bpmp_channel *channel)
+{
+	return tegra_ivc_write_advance(channel->ivc);
+}
+
+static int tegra_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
+{
+	int err;
+
+	if (WARN_ON(bpmp->mbox.channel == NULL))
+		return -EINVAL;
+
+	err = mbox_send_message(bpmp->mbox.channel, NULL);
+	if (err < 0)
+		return err;
+
+	mbox_client_txdone(bpmp->mbox.channel, 0);
+
+	return 0;
+}
+
 static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
 					 void *data, size_t size, int *ret)
 {
@@ -166,7 +212,7 @@ static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
 	if (data && size > 0)
 		memcpy(data, channel->ib->data, size);
 
-	err = tegra_ivc_read_advance(channel->ivc);
+	err = tegra_bpmp_ack_resp(channel);
 	if (err < 0)
 		return err;
 
@@ -210,7 +256,7 @@ static ssize_t __tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,
 	if (data && size > 0)
 		memcpy(channel->ob->data, data, size);
 
-	return tegra_ivc_write_advance(channel->ivc);
+	return tegra_bpmp_post_req(channel);
 }
 
 static struct tegra_bpmp_channel *
@@ -238,7 +284,7 @@ tegra_bpmp_write_threaded(struct tegra_bpmp *bpmp, unsigned int mrq,
 
 	channel = &bpmp->threaded_channels[index];
 
-	if (!tegra_bpmp_master_free(channel)) {
+	if (!tegra_bpmp_is_req_channel_free(channel)) {
 		err = -EBUSY;
 		goto unlock;
 	}
@@ -270,7 +316,7 @@ static ssize_t tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,
 {
 	int err;
 
-	err = tegra_bpmp_wait_master_free(channel);
+	err = tegra_bpmp_wait_req_channel_free(channel);
 	if (err < 0)
 		return err;
 
@@ -302,13 +348,11 @@ int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp,
 
 	spin_unlock(&bpmp->atomic_tx_lock);
 
-	err = mbox_send_message(bpmp->mbox.channel, NULL);
+	err = tegra_bpmp_ring_doorbell(bpmp);
 	if (err < 0)
 		return err;
 
-	mbox_client_txdone(bpmp->mbox.channel, 0);
-
-	err = tegra_bpmp_wait_ack(channel);
+	err = tegra_bpmp_wait_resp(channel);
 	if (err < 0)
 		return err;
 
@@ -335,12 +379,10 @@ int tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
 	if (IS_ERR(channel))
 		return PTR_ERR(channel);
 
-	err = mbox_send_message(bpmp->mbox.channel, NULL);
+	err = tegra_bpmp_ring_doorbell(bpmp);
 	if (err < 0)
 		return err;
 
-	mbox_client_txdone(bpmp->mbox.channel, 0);
-
 	timeout = usecs_to_jiffies(bpmp->soc->channels.thread.timeout);
 
 	err = wait_for_completion_timeout(&channel->completion, timeout);
@@ -369,38 +411,34 @@ void tegra_bpmp_mrq_return(struct tegra_bpmp_channel *channel, int code,
 {
 	unsigned long flags = channel->ib->flags;
 	struct tegra_bpmp *bpmp = channel->bpmp;
-	struct tegra_bpmp_mb_data *frame;
 	int err;
 
 	if (WARN_ON(size > MSG_DATA_MIN_SZ))
 		return;
 
-	err = tegra_ivc_read_advance(channel->ivc);
+	err = tegra_bpmp_ack_req(channel);
 	if (WARN_ON(err < 0))
 		return;
 
 	if ((flags & MSG_ACK) == 0)
 		return;
 
-	frame = tegra_ivc_write_get_next_frame(channel->ivc);
-	if (WARN_ON(IS_ERR(frame)))
+	if (WARN_ON(!tegra_bpmp_is_resp_channel_free(channel)))
 		return;
 
-	frame->code = code;
+	channel->ob->code = code;
 
 	if (data && size > 0)
-		memcpy(frame->data, data, size);
+		memcpy(channel->ob->data, data, size);
 
-	err = tegra_ivc_write_advance(channel->ivc);
+	err = tegra_bpmp_post_resp(channel);
 	if (WARN_ON(err < 0))
 		return;
 
 	if (flags & MSG_RING) {
-		err = mbox_send_message(bpmp->mbox.channel, NULL);
+		err = tegra_bpmp_ring_doorbell(bpmp);
 		if (WARN_ON(err < 0))
 			return;
-
-		mbox_client_txdone(bpmp->mbox.channel, 0);
 	}
 }
 EXPORT_SYMBOL_GPL(tegra_bpmp_mrq_return);
@@ -638,7 +676,7 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
 	count = bpmp->soc->channels.thread.count;
 	busy = bpmp->threaded.busy;
 
-	if (tegra_bpmp_master_acked(channel))
+	if (tegra_bpmp_is_req_ready(channel))
 		tegra_bpmp_handle_mrq(bpmp, channel->ib->code, channel);
 
 	spin_lock(&bpmp->lock);
@@ -648,7 +686,7 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
 
 		channel = &bpmp->threaded_channels[i];
 
-		if (tegra_bpmp_master_acked(channel)) {
+		if (tegra_bpmp_is_resp_ready(channel)) {
 			tegra_bpmp_channel_signal(channel);
 			clear_bit(i, busy);
 		}
@@ -660,16 +698,8 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
 static void tegra_bpmp_ivc_notify(struct tegra_ivc *ivc, void *data)
 {
 	struct tegra_bpmp *bpmp = data;
-	int err;
 
-	if (WARN_ON(bpmp->mbox.channel == NULL))
-		return;
-
-	err = mbox_send_message(bpmp->mbox.channel, NULL);
-	if (err < 0)
-		return;
-
-	mbox_client_txdone(bpmp->mbox.channel, 0);
+	WARN_ON(tegra_bpmp_ring_doorbell(bpmp));
 }
 
 static int tegra_bpmp_channel_init(struct tegra_bpmp_channel *channel,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 2/4] firmware: tegra: refactor bpmp driver
  2019-01-21 12:28 [PATCH V2 0/4] add Tegra210 BPMP driver Timo Alho
  2019-01-21 12:28 ` [PATCH V2 1/4] firmware: tegra: reword messaging terminology Timo Alho
@ 2019-01-21 12:28 ` Timo Alho
  2019-01-24 11:45   ` Jon Hunter
  2019-01-21 12:28 ` [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210 Timo Alho
  2019-01-21 12:28 ` [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP Timo Alho
  3 siblings, 1 reply; 16+ messages in thread
From: Timo Alho @ 2019-01-21 12:28 UTC (permalink / raw)
  To: treding, sivaramn, robh; +Cc: linux-tegra, devicetree, Timo Alho

Split bpmp driver into common and chip specific parts to facilitae
adding support for previous and futurue Tegra chips that are using
bpmp as co-processor.

Signed-off-by: Timo Alho <talho@nvidia.com>
---
 drivers/firmware/tegra/Makefile       |   1 +
 drivers/firmware/tegra/bpmp-private.h |  28 ++++
 drivers/firmware/tegra/bpmp-t186.c    | 303 ++++++++++++++++++++++++++++++++++
 drivers/firmware/tegra/bpmp.c         | 253 ++++++----------------------
 include/soc/tegra/bpmp.h              |  12 +-
 5 files changed, 389 insertions(+), 208 deletions(-)
 create mode 100644 drivers/firmware/tegra/bpmp-private.h
 create mode 100644 drivers/firmware/tegra/bpmp-t186.c

diff --git a/drivers/firmware/tegra/Makefile b/drivers/firmware/tegra/Makefile
index 1b826dc..367d482 100644
--- a/drivers/firmware/tegra/Makefile
+++ b/drivers/firmware/tegra/Makefile
@@ -1,4 +1,5 @@
 tegra-bpmp-y			= bpmp.o
+tegra-bpmp-$(CONFIG_ARCH_TEGRA_186_SOC)	+= bpmp-t186.o
 tegra-bpmp-$(CONFIG_DEBUG_FS)	+= bpmp-debugfs.o
 obj-$(CONFIG_TEGRA_BPMP)	+= tegra-bpmp.o
 obj-$(CONFIG_TEGRA_IVC)		+= ivc.o
diff --git a/drivers/firmware/tegra/bpmp-private.h b/drivers/firmware/tegra/bpmp-private.h
new file mode 100644
index 0000000..2354337
--- /dev/null
+++ b/drivers/firmware/tegra/bpmp-private.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, NVIDIA CORPORATION.
+ */
+
+#ifndef __FIRMWARE_TEGRA_BPMP_PRIVATE_H
+#define __FIRMWARE_TEGRA_BPMP_PRIVATE_H
+
+#include <soc/tegra/bpmp.h>
+
+struct tegra_bpmp_ops {
+	int (*init)(struct tegra_bpmp *bpmp);
+	void (*deinit)(struct tegra_bpmp *bpmp);
+	bool (*is_resp_ready)(struct tegra_bpmp_channel *channel);
+	bool (*is_req_ready)(struct tegra_bpmp_channel *channel);
+	int (*ack_resp)(struct tegra_bpmp_channel *channel);
+	int (*ack_req)(struct tegra_bpmp_channel *channel);
+	bool (*is_resp_channel_free)(struct tegra_bpmp_channel *channel);
+	bool (*is_req_channel_free)(struct tegra_bpmp_channel *channel);
+	int (*post_resp)(struct tegra_bpmp_channel *channel);
+	int (*post_req)(struct tegra_bpmp_channel *channel);
+	int (*ring_doorbell)(struct tegra_bpmp *bpmp);
+	int (*resume)(struct tegra_bpmp *bpmp);
+};
+
+extern struct tegra_bpmp_ops tegra186_bpmp_ops;
+
+#endif
diff --git a/drivers/firmware/tegra/bpmp-t186.c b/drivers/firmware/tegra/bpmp-t186.c
new file mode 100644
index 0000000..e425c6f
--- /dev/null
+++ b/drivers/firmware/tegra/bpmp-t186.c
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, NVIDIA CORPORATION.
+ */
+
+#include <linux/clk/tegra.h>
+#include <linux/genalloc.h>
+#include <linux/mailbox_client.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/semaphore.h>
+
+#include <soc/tegra/bpmp.h>
+#include <soc/tegra/bpmp-abi.h>
+#include <soc/tegra/ivc.h>
+
+#include "bpmp-private.h"
+
+struct tegra186_bpmp {
+	struct tegra_bpmp *parent;
+
+	struct {
+		struct gen_pool *pool;
+		dma_addr_t phys;
+		void *virt;
+	} tx, rx;
+
+	struct {
+		struct mbox_client client;
+		struct mbox_chan *channel;
+	} mbox;
+};
+
+static inline struct tegra_bpmp *
+mbox_client_to_bpmp(struct mbox_client *client)
+{
+	struct tegra186_bpmp *priv;
+
+	priv = container_of(client, struct tegra186_bpmp, mbox.client);
+
+	return priv->parent;
+}
+
+static bool tegra186_bpmp_is_message_ready(struct tegra_bpmp_channel *channel)
+{
+	void *frame;
+
+	frame = tegra_ivc_read_get_next_frame(channel->ivc);
+	if (IS_ERR(frame)) {
+		channel->ib = NULL;
+		return false;
+	}
+
+	channel->ib = frame;
+
+	return true;
+}
+
+static bool tegra186_bpmp_is_channel_free(struct tegra_bpmp_channel *channel)
+{
+	void *frame;
+
+	frame = tegra_ivc_write_get_next_frame(channel->ivc);
+	if (IS_ERR(frame)) {
+		channel->ob = NULL;
+		return false;
+	}
+
+	channel->ob = frame;
+
+	return true;
+}
+
+static int tegra186_bpmp_ack_message(struct tegra_bpmp_channel *channel)
+{
+	return tegra_ivc_read_advance(channel->ivc);
+}
+
+static int tegra186_bpmp_post_message(struct tegra_bpmp_channel *channel)
+{
+	return tegra_ivc_write_advance(channel->ivc);
+}
+
+static int tegra186_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
+{
+	struct tegra186_bpmp *priv = bpmp->priv;
+	int err;
+
+	err = mbox_send_message(priv->mbox.channel, NULL);
+	if (err < 0)
+		return err;
+
+	mbox_client_txdone(priv->mbox.channel, 0);
+
+	return 0;
+}
+
+static void tegra186_bpmp_ivc_notify(struct tegra_ivc *ivc, void *data)
+{
+	struct tegra_bpmp *bpmp = data;
+	struct tegra186_bpmp *priv = bpmp->priv;
+
+	if (WARN_ON(priv->mbox.channel == NULL))
+		return;
+
+	tegra186_bpmp_ring_doorbell(bpmp);
+}
+
+static int tegra186_bpmp_channel_init(struct tegra_bpmp_channel *channel,
+				   struct tegra_bpmp *bpmp,
+				   unsigned int index)
+{
+	struct tegra186_bpmp *priv = bpmp->priv;
+	size_t message_size, queue_size;
+	unsigned int offset;
+	int err;
+
+	channel->ivc = devm_kzalloc(bpmp->dev, sizeof(*channel->ivc),
+				    GFP_KERNEL);
+	if (!channel->ivc)
+		return -ENOMEM;
+
+	message_size = tegra_ivc_align(MSG_MIN_SZ);
+	queue_size = tegra_ivc_total_queue_size(message_size);
+	offset = queue_size * index;
+
+	err = tegra_ivc_init(channel->ivc, NULL,
+			     priv->rx.virt + offset, priv->rx.phys + offset,
+			     priv->tx.virt + offset, priv->tx.phys + offset,
+			     1, message_size, tegra186_bpmp_ivc_notify,
+			     bpmp);
+	if (err < 0) {
+		dev_err(bpmp->dev, "failed to setup IVC for channel %u: %d\n",
+			index, err);
+		return err;
+	}
+
+	init_completion(&channel->completion);
+	channel->bpmp = bpmp;
+
+	return 0;
+}
+
+static void tegra186_bpmp_channel_reset(struct tegra_bpmp_channel *channel)
+{
+	/* reset the channel state */
+	tegra_ivc_reset(channel->ivc);
+
+	/* sync the channel state with BPMP */
+	while (tegra_ivc_notified(channel->ivc))
+		;
+}
+
+static void tegra186_bpmp_channel_cleanup(struct tegra_bpmp_channel *channel)
+{
+	tegra_ivc_cleanup(channel->ivc);
+}
+
+static void mbox_handle_rx(struct mbox_client *client, void *data)
+{
+	struct tegra_bpmp *bpmp = mbox_client_to_bpmp(client);
+
+	tegra_bpmp_handle_rx(bpmp);
+}
+
+static int tegra186_bpmp_init(struct tegra_bpmp *bpmp)
+{
+	struct tegra186_bpmp *priv;
+	unsigned int i;
+	int err;
+
+	priv = devm_kzalloc(bpmp->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	bpmp->priv = priv;
+	priv->parent = bpmp;
+
+	priv->tx.pool = of_gen_pool_get(bpmp->dev->of_node, "shmem", 0);
+	if (!priv->tx.pool) {
+		dev_err(bpmp->dev, "TX shmem pool not found\n");
+		return -ENOMEM;
+	}
+
+	priv->tx.virt = gen_pool_dma_alloc(priv->tx.pool, 4096, &priv->tx.phys);
+	if (!priv->tx.virt) {
+		dev_err(bpmp->dev, "failed to allocate from TX pool\n");
+		return -ENOMEM;
+	}
+
+	priv->rx.pool = of_gen_pool_get(bpmp->dev->of_node, "shmem", 1);
+	if (!priv->rx.pool) {
+		dev_err(bpmp->dev, "RX shmem pool not found\n");
+		err = -ENOMEM;
+		goto free_tx;
+	}
+
+	priv->rx.virt = gen_pool_dma_alloc(priv->rx.pool, 4096, &priv->rx.phys);
+	if (!priv->rx.virt) {
+		dev_err(bpmp->dev, "failed to allocate from RX pool\n");
+		err = -ENOMEM;
+		goto free_tx;
+	}
+
+	err = tegra186_bpmp_channel_init(bpmp->tx_channel, bpmp,
+					 bpmp->soc->channels.cpu_tx.offset);
+	if (err < 0)
+		goto free_rx;
+
+	err = tegra186_bpmp_channel_init(bpmp->rx_channel, bpmp,
+					 bpmp->soc->channels.cpu_rx.offset);
+	if (err < 0)
+		goto cleanup_tx_channel;
+
+	for (i = 0; i < bpmp->threaded.count; i++) {
+		err = tegra186_bpmp_channel_init(
+			&bpmp->threaded_channels[i], bpmp,
+			bpmp->soc->channels.thread.offset + i);
+		if (err < 0)
+			goto cleanup_channels;
+	}
+
+	/* mbox registration */
+	priv->mbox.client.dev = bpmp->dev;
+	priv->mbox.client.rx_callback = mbox_handle_rx;
+	priv->mbox.client.tx_block = false;
+	priv->mbox.client.knows_txdone = false;
+
+	priv->mbox.channel = mbox_request_channel(&priv->mbox.client, 0);
+	if (IS_ERR(priv->mbox.channel)) {
+		err = PTR_ERR(priv->mbox.channel);
+		dev_err(bpmp->dev, "failed to get HSP mailbox: %d\n", err);
+		goto cleanup_channels;
+	}
+
+	tegra186_bpmp_channel_reset(bpmp->tx_channel);
+	tegra186_bpmp_channel_reset(bpmp->rx_channel);
+	for (i = 0; i < bpmp->threaded.count; i++)
+		tegra186_bpmp_channel_reset(&bpmp->threaded_channels[i]);
+
+	return 0;
+
+cleanup_channels:
+	for (i = 0; i < bpmp->threaded.count; i++) {
+		if (bpmp->threaded_channels[i].bpmp)
+			tegra186_bpmp_channel_cleanup(
+				&bpmp->threaded_channels[i]);
+	}
+
+	tegra186_bpmp_channel_cleanup(bpmp->rx_channel);
+cleanup_tx_channel:
+	tegra186_bpmp_channel_cleanup(bpmp->tx_channel);
+free_rx:
+	gen_pool_free(priv->rx.pool, (unsigned long)priv->rx.virt, 4096);
+free_tx:
+	gen_pool_free(priv->tx.pool, (unsigned long)priv->tx.virt, 4096);
+
+	return err;
+}
+
+static void tegra186_bpmp_deinit(struct tegra_bpmp *bpmp)
+{
+	struct tegra186_bpmp *priv = bpmp->priv;
+	unsigned int i;
+
+	mbox_free_channel(priv->mbox.channel);
+	for (i = 0; i < bpmp->threaded.count; i++)
+		tegra186_bpmp_channel_cleanup(&bpmp->threaded_channels[i]);
+	tegra186_bpmp_channel_cleanup(bpmp->rx_channel);
+	tegra186_bpmp_channel_cleanup(bpmp->tx_channel);
+	gen_pool_free(priv->rx.pool, (unsigned long)priv->rx.virt, 4096);
+	gen_pool_free(priv->tx.pool, (unsigned long)priv->tx.virt, 4096);
+}
+
+static int tegra186_bpmp_resume(struct tegra_bpmp *bpmp)
+{
+	int i;
+
+	/* reset message channels */
+	tegra186_bpmp_channel_reset(bpmp->tx_channel);
+	tegra186_bpmp_channel_reset(bpmp->rx_channel);
+
+	for (i = 0; i < bpmp->threaded.count; i++)
+		tegra186_bpmp_channel_reset(&bpmp->threaded_channels[i]);
+
+	return 0;
+}
+
+struct tegra_bpmp_ops tegra186_bpmp_ops = {
+	.init = tegra186_bpmp_init,
+	.deinit = tegra186_bpmp_deinit,
+	.is_resp_ready = tegra186_bpmp_is_message_ready,
+	.is_req_ready = tegra186_bpmp_is_message_ready,
+	.ack_resp = tegra186_bpmp_ack_message,
+	.ack_req = tegra186_bpmp_ack_message,
+	.is_resp_channel_free = tegra186_bpmp_is_channel_free,
+	.is_req_channel_free = tegra186_bpmp_is_channel_free,
+	.post_resp = tegra186_bpmp_post_message,
+	.post_req = tegra186_bpmp_post_message,
+	.ring_doorbell = tegra186_bpmp_ring_doorbell,
+	.resume = tegra186_bpmp_resume,
+};
diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index af123de..c6716ec 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -26,6 +26,8 @@
 #include <soc/tegra/bpmp-abi.h>
 #include <soc/tegra/ivc.h>
 
+#include "bpmp-private.h"
+
 #define MSG_ACK		BIT(0)
 #define MSG_RING	BIT(1)
 #define TAG_SZ		32
@@ -36,6 +38,14 @@ mbox_client_to_bpmp(struct mbox_client *client)
 	return container_of(client, struct tegra_bpmp, mbox.client);
 }
 
+static inline struct tegra_bpmp_ops *
+channel_to_ops(struct tegra_bpmp_channel *channel)
+{
+	struct tegra_bpmp *bpmp = channel->bpmp;
+
+	return bpmp->soc->ops;
+}
+
 struct tegra_bpmp *tegra_bpmp_get(struct device *dev)
 {
 	struct platform_device *pdev;
@@ -98,22 +108,16 @@ static bool tegra_bpmp_message_valid(const struct tegra_bpmp_message *msg)
 
 static bool tegra_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
 {
-	void *frame;
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
 
-	frame = tegra_ivc_read_get_next_frame(channel->ivc);
-	if (IS_ERR(frame)) {
-		channel->ib = NULL;
-		return false;
-	}
-
-	channel->ib = frame;
-
-	return true;
+	return ops->is_resp_ready(channel);
 }
 
 static bool tegra_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
 {
-	return tegra_bpmp_is_resp_ready(channel);
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
+
+	return ops->is_req_ready(channel);
 }
 
 static int tegra_bpmp_wait_resp(struct tegra_bpmp_channel *channel)
@@ -133,32 +137,30 @@ static int tegra_bpmp_wait_resp(struct tegra_bpmp_channel *channel)
 
 static int tegra_bpmp_ack_resp(struct tegra_bpmp_channel *channel)
 {
-	return tegra_ivc_read_advance(channel->ivc);
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
+
+	return ops->ack_resp(channel);
 }
 
 static int tegra_bpmp_ack_req(struct tegra_bpmp_channel *channel)
 {
-	return tegra_ivc_read_advance(channel->ivc);
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
+
+	return ops->ack_req(channel);
 }
 
 static bool tegra_bpmp_is_req_channel_free(struct tegra_bpmp_channel *channel)
 {
-	void *frame;
-
-	frame = tegra_ivc_write_get_next_frame(channel->ivc);
-	if (IS_ERR(frame)) {
-		channel->ob = NULL;
-		return false;
-	}
-
-	channel->ob = frame;
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
 
-	return true;
+	return ops->is_req_channel_free(channel);
 }
 
 static bool tegra_bpmp_is_resp_channel_free(struct tegra_bpmp_channel *channel)
 {
-	return tegra_bpmp_is_req_channel_free(channel);
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
+
+	return ops->is_resp_channel_free(channel);
 }
 
 static int tegra_bpmp_wait_req_channel_free(struct tegra_bpmp_channel *channel)
@@ -180,28 +182,21 @@ static int tegra_bpmp_wait_req_channel_free(struct tegra_bpmp_channel *channel)
 
 static int tegra_bpmp_post_req(struct tegra_bpmp_channel *channel)
 {
-	return tegra_ivc_write_advance(channel->ivc);
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
+
+	return ops->post_req(channel);
 }
 
 static int tegra_bpmp_post_resp(struct tegra_bpmp_channel *channel)
 {
-	return tegra_ivc_write_advance(channel->ivc);
+	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
+
+	return ops->post_resp(channel);
 }
 
 static int tegra_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
 {
-	int err;
-
-	if (WARN_ON(bpmp->mbox.channel == NULL))
-		return -EINVAL;
-
-	err = mbox_send_message(bpmp->mbox.channel, NULL);
-	if (err < 0)
-		return err;
-
-	mbox_client_txdone(bpmp->mbox.channel, 0);
-
-	return 0;
+	return bpmp->soc->ops->ring_doorbell(bpmp);
 }
 
 static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
@@ -665,9 +660,8 @@ static void tegra_bpmp_channel_signal(struct tegra_bpmp_channel *channel)
 	complete(&channel->completion);
 }
 
-static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
+void tegra_bpmp_handle_rx(struct tegra_bpmp *bpmp)
 {
-	struct tegra_bpmp *bpmp = mbox_client_to_bpmp(client);
 	struct tegra_bpmp_channel *channel;
 	unsigned int i, count;
 	unsigned long *busy;
@@ -695,66 +689,9 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
 	spin_unlock(&bpmp->lock);
 }
 
-static void tegra_bpmp_ivc_notify(struct tegra_ivc *ivc, void *data)
-{
-	struct tegra_bpmp *bpmp = data;
-
-	WARN_ON(tegra_bpmp_ring_doorbell(bpmp));
-}
-
-static int tegra_bpmp_channel_init(struct tegra_bpmp_channel *channel,
-				   struct tegra_bpmp *bpmp,
-				   unsigned int index)
-{
-	size_t message_size, queue_size;
-	unsigned int offset;
-	int err;
-
-	channel->ivc = devm_kzalloc(bpmp->dev, sizeof(*channel->ivc),
-				    GFP_KERNEL);
-	if (!channel->ivc)
-		return -ENOMEM;
-
-	message_size = tegra_ivc_align(MSG_MIN_SZ);
-	queue_size = tegra_ivc_total_queue_size(message_size);
-	offset = queue_size * index;
-
-	err = tegra_ivc_init(channel->ivc, NULL,
-			     bpmp->rx.virt + offset, bpmp->rx.phys + offset,
-			     bpmp->tx.virt + offset, bpmp->tx.phys + offset,
-			     1, message_size, tegra_bpmp_ivc_notify,
-			     bpmp);
-	if (err < 0) {
-		dev_err(bpmp->dev, "failed to setup IVC for channel %u: %d\n",
-			index, err);
-		return err;
-	}
-
-	init_completion(&channel->completion);
-	channel->bpmp = bpmp;
-
-	return 0;
-}
-
-static void tegra_bpmp_channel_reset(struct tegra_bpmp_channel *channel)
-{
-	/* reset the channel state */
-	tegra_ivc_reset(channel->ivc);
-
-	/* sync the channel state with BPMP */
-	while (tegra_ivc_notified(channel->ivc))
-		;
-}
-
-static void tegra_bpmp_channel_cleanup(struct tegra_bpmp_channel *channel)
-{
-	tegra_ivc_cleanup(channel->ivc);
-}
-
 static int tegra_bpmp_probe(struct platform_device *pdev)
 {
 	struct tegra_bpmp *bpmp;
-	unsigned int i;
 	char tag[TAG_SZ];
 	size_t size;
 	int err;
@@ -766,32 +703,6 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
 	bpmp->soc = of_device_get_match_data(&pdev->dev);
 	bpmp->dev = &pdev->dev;
 
-	bpmp->tx.pool = of_gen_pool_get(pdev->dev.of_node, "shmem", 0);
-	if (!bpmp->tx.pool) {
-		dev_err(&pdev->dev, "TX shmem pool not found\n");
-		return -ENOMEM;
-	}
-
-	bpmp->tx.virt = gen_pool_dma_alloc(bpmp->tx.pool, 4096, &bpmp->tx.phys);
-	if (!bpmp->tx.virt) {
-		dev_err(&pdev->dev, "failed to allocate from TX pool\n");
-		return -ENOMEM;
-	}
-
-	bpmp->rx.pool = of_gen_pool_get(pdev->dev.of_node, "shmem", 1);
-	if (!bpmp->rx.pool) {
-		dev_err(&pdev->dev, "RX shmem pool not found\n");
-		err = -ENOMEM;
-		goto free_tx;
-	}
-
-	bpmp->rx.virt = gen_pool_dma_alloc(bpmp->rx.pool, 4096, &bpmp->rx.phys);
-	if (!bpmp->rx.virt) {
-		dev_err(&pdev->dev, "failed to allocate from RX pool\n");
-		err = -ENOMEM;
-		goto free_tx;
-	}
-
 	INIT_LIST_HEAD(&bpmp->mrqs);
 	spin_lock_init(&bpmp->lock);
 
@@ -801,81 +712,38 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
 	size = BITS_TO_LONGS(bpmp->threaded.count) * sizeof(long);
 
 	bpmp->threaded.allocated = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
-	if (!bpmp->threaded.allocated) {
-		err = -ENOMEM;
-		goto free_rx;
-	}
+	if (!bpmp->threaded.allocated)
+		return -ENOMEM;
 
 	bpmp->threaded.busy = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
-	if (!bpmp->threaded.busy) {
-		err = -ENOMEM;
-		goto free_rx;
-	}
+	if (!bpmp->threaded.busy)
+		return -ENOMEM;
 
 	spin_lock_init(&bpmp->atomic_tx_lock);
 	bpmp->tx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->tx_channel),
 					GFP_KERNEL);
-	if (!bpmp->tx_channel) {
-		err = -ENOMEM;
-		goto free_rx;
-	}
+	if (!bpmp->tx_channel)
+		return -ENOMEM;
 
 	bpmp->rx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->rx_channel),
 	                                GFP_KERNEL);
-	if (!bpmp->rx_channel) {
-		err = -ENOMEM;
-		goto free_rx;
-	}
+	if (!bpmp->rx_channel)
+		return -ENOMEM;
 
 	bpmp->threaded_channels = devm_kcalloc(&pdev->dev, bpmp->threaded.count,
 					       sizeof(*bpmp->threaded_channels),
 					       GFP_KERNEL);
-	if (!bpmp->threaded_channels) {
-		err = -ENOMEM;
-		goto free_rx;
-	}
-
-	err = tegra_bpmp_channel_init(bpmp->tx_channel, bpmp,
-				      bpmp->soc->channels.cpu_tx.offset);
-	if (err < 0)
-		goto free_rx;
+	if (!bpmp->threaded_channels)
+		return -ENOMEM;
 
-	err = tegra_bpmp_channel_init(bpmp->rx_channel, bpmp,
-				      bpmp->soc->channels.cpu_rx.offset);
+	err = bpmp->soc->ops->init(bpmp);
 	if (err < 0)
-		goto cleanup_tx_channel;
-
-	for (i = 0; i < bpmp->threaded.count; i++) {
-		err = tegra_bpmp_channel_init(
-			&bpmp->threaded_channels[i], bpmp,
-			bpmp->soc->channels.thread.offset + i);
-		if (err < 0)
-			goto cleanup_threaded_channels;
-	}
-
-	/* mbox registration */
-	bpmp->mbox.client.dev = &pdev->dev;
-	bpmp->mbox.client.rx_callback = tegra_bpmp_handle_rx;
-	bpmp->mbox.client.tx_block = false;
-	bpmp->mbox.client.knows_txdone = false;
-
-	bpmp->mbox.channel = mbox_request_channel(&bpmp->mbox.client, 0);
-	if (IS_ERR(bpmp->mbox.channel)) {
-		err = PTR_ERR(bpmp->mbox.channel);
-		dev_err(&pdev->dev, "failed to get HSP mailbox: %d\n", err);
-		goto cleanup_threaded_channels;
-	}
-
-	/* reset message channels */
-	tegra_bpmp_channel_reset(bpmp->tx_channel);
-	tegra_bpmp_channel_reset(bpmp->rx_channel);
-	for (i = 0; i < bpmp->threaded.count; i++)
-		tegra_bpmp_channel_reset(&bpmp->threaded_channels[i]);
+		return err;
 
 	err = tegra_bpmp_request_mrq(bpmp, MRQ_PING,
 				     tegra_bpmp_mrq_handle_ping, bpmp);
 	if (err < 0)
-		goto free_mbox;
+		goto deinit;
 
 	err = tegra_bpmp_ping(bpmp);
 	if (err < 0) {
@@ -917,37 +785,17 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
 
 free_mrq:
 	tegra_bpmp_free_mrq(bpmp, MRQ_PING, bpmp);
-free_mbox:
-	mbox_free_channel(bpmp->mbox.channel);
-cleanup_threaded_channels:
-	for (i = 0; i < bpmp->threaded.count; i++) {
-		if (bpmp->threaded_channels[i].bpmp)
-			tegra_bpmp_channel_cleanup(&bpmp->threaded_channels[i]);
-	}
+deinit:
+	bpmp->soc->ops->deinit(bpmp);
 
-	tegra_bpmp_channel_cleanup(bpmp->rx_channel);
-cleanup_tx_channel:
-	tegra_bpmp_channel_cleanup(bpmp->tx_channel);
-free_rx:
-	gen_pool_free(bpmp->rx.pool, (unsigned long)bpmp->rx.virt, 4096);
-free_tx:
-	gen_pool_free(bpmp->tx.pool, (unsigned long)bpmp->tx.virt, 4096);
 	return err;
 }
 
 static int __maybe_unused tegra_bpmp_resume(struct device *dev)
 {
 	struct tegra_bpmp *bpmp = dev_get_drvdata(dev);
-	unsigned int i;
-
-	/* reset message channels */
-	tegra_bpmp_channel_reset(bpmp->tx_channel);
-	tegra_bpmp_channel_reset(bpmp->rx_channel);
 
-	for (i = 0; i < bpmp->threaded.count; i++)
-		tegra_bpmp_channel_reset(&bpmp->threaded_channels[i]);
-
-	return 0;
+	return bpmp->soc->ops->resume(bpmp);
 }
 
 static SIMPLE_DEV_PM_OPS(tegra_bpmp_pm_ops, NULL, tegra_bpmp_resume);
@@ -968,6 +816,7 @@ static const struct tegra_bpmp_soc tegra186_soc = {
 			.timeout = 0,
 		},
 	},
+	.ops = &tegra186_bpmp_ops,
 	.num_resets = 193,
 };
 
diff --git a/include/soc/tegra/bpmp.h b/include/soc/tegra/bpmp.h
index b02f926..a4818c0 100644
--- a/include/soc/tegra/bpmp.h
+++ b/include/soc/tegra/bpmp.h
@@ -23,6 +23,7 @@
 #include <soc/tegra/bpmp-abi.h>
 
 struct tegra_bpmp_clk;
+struct tegra_bpmp_ops;
 
 struct tegra_bpmp_soc {
 	struct {
@@ -32,6 +33,8 @@ struct tegra_bpmp_soc {
 			unsigned int timeout;
 		} cpu_tx, thread, cpu_rx;
 	} channels;
+
+	struct tegra_bpmp_ops *ops;
 	unsigned int num_resets;
 };
 
@@ -63,12 +66,7 @@ struct tegra_bpmp_mrq {
 struct tegra_bpmp {
 	const struct tegra_bpmp_soc *soc;
 	struct device *dev;
-
-	struct {
-		struct gen_pool *pool;
-		dma_addr_t phys;
-		void *virt;
-	} tx, rx;
+	void *priv;
 
 	struct {
 		struct mbox_client client;
@@ -173,6 +171,8 @@ static inline bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp,
 }
 #endif
 
+void tegra_bpmp_handle_rx(struct tegra_bpmp *bpmp);
+
 #if IS_ENABLED(CONFIG_CLK_TEGRA_BPMP)
 int tegra_bpmp_init_clocks(struct tegra_bpmp *bpmp);
 #else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210
  2019-01-21 12:28 [PATCH V2 0/4] add Tegra210 BPMP driver Timo Alho
  2019-01-21 12:28 ` [PATCH V2 1/4] firmware: tegra: reword messaging terminology Timo Alho
  2019-01-21 12:28 ` [PATCH V2 2/4] firmware: tegra: refactor bpmp driver Timo Alho
@ 2019-01-21 12:28 ` Timo Alho
  2019-01-24 12:16   ` Jon Hunter
  2019-01-21 12:28 ` [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP Timo Alho
  3 siblings, 1 reply; 16+ messages in thread
From: Timo Alho @ 2019-01-21 12:28 UTC (permalink / raw)
  To: treding, sivaramn, robh; +Cc: linux-tegra, devicetree, Timo Alho

This patch adds driver for Tegra210 BPMP firmware.

The BPMP is a specific processor in Tegra210 chip, which runs firmware
for assisting in entering deep low power states (suspend to ram), and
offloading DRAM memory clock scaling on some platforms.

Based on work by Sivaram Nair <sivaramn@nvidia.com>

Signed-off-by: Timo Alho <talho@nvidia.com>
---
 drivers/firmware/tegra/Makefile       |   1 +
 drivers/firmware/tegra/bpmp-private.h |   1 +
 drivers/firmware/tegra/bpmp-t210.c    | 241 ++++++++++++++++++++++++++++++++++
 drivers/firmware/tegra/bpmp.c         |  46 +++++--
 include/soc/tegra/bpmp.h              |   1 +
 5 files changed, 281 insertions(+), 9 deletions(-)
 create mode 100644 drivers/firmware/tegra/bpmp-t210.c

diff --git a/drivers/firmware/tegra/Makefile b/drivers/firmware/tegra/Makefile
index 367d482..dc41d6f 100644
--- a/drivers/firmware/tegra/Makefile
+++ b/drivers/firmware/tegra/Makefile
@@ -1,5 +1,6 @@
 tegra-bpmp-y			= bpmp.o
 tegra-bpmp-$(CONFIG_ARCH_TEGRA_186_SOC)	+= bpmp-t186.o
+tegra-bpmp-$(CONFIG_ARCH_TEGRA_210_SOC)	+= bpmp-t210.o
 tegra-bpmp-$(CONFIG_DEBUG_FS)	+= bpmp-debugfs.o
 obj-$(CONFIG_TEGRA_BPMP)	+= tegra-bpmp.o
 obj-$(CONFIG_TEGRA_IVC)		+= ivc.o
diff --git a/drivers/firmware/tegra/bpmp-private.h b/drivers/firmware/tegra/bpmp-private.h
index 2354337..5132234 100644
--- a/drivers/firmware/tegra/bpmp-private.h
+++ b/drivers/firmware/tegra/bpmp-private.h
@@ -24,5 +24,6 @@ struct tegra_bpmp_ops {
 };
 
 extern struct tegra_bpmp_ops tegra186_bpmp_ops;
+extern struct tegra_bpmp_ops tegra210_bpmp_ops;
 
 #endif
diff --git a/drivers/firmware/tegra/bpmp-t210.c b/drivers/firmware/tegra/bpmp-t210.c
new file mode 100644
index 0000000..cb16c94
--- /dev/null
+++ b/drivers/firmware/tegra/bpmp-t210.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, NVIDIA CORPORATION.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <soc/tegra/bpmp.h>
+
+#include "bpmp-private.h"
+
+#define TRIGGER_OFFSET		0x000
+#define RESULT_OFFSET(id)	(0xc00 + id * 4)
+#define TRIGGER_ID_SHIFT	16
+#define TRIGGER_CMD_GET		4
+
+#define STA_OFFSET		0
+#define SET_OFFSET		4
+#define CLR_OFFSET		8
+
+#define CH_MASK(ch)	(0x3 << ((ch) * 2))
+#define SL_SIGL(ch)	(0x0 << ((ch) * 2))
+#define SL_QUED(ch)	(0x1 << ((ch) * 2))
+#define MA_FREE(ch)	(0x2 << ((ch) * 2))
+#define MA_ACKD(ch)	(0x3 << ((ch) * 2))
+
+struct tegra210_bpmp {
+	void __iomem *atomics;
+	void __iomem *arb_sema;
+	unsigned int txirq;
+};
+
+static uint32_t bpmp_ch_sta(struct tegra_bpmp *bpmp, int ch)
+{
+	struct tegra210_bpmp *priv = bpmp->priv;
+
+	return __raw_readl(priv->arb_sema + STA_OFFSET) & CH_MASK(ch);
+}
+
+static bool tegra210_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
+{
+	int nr = channel->nr;
+
+	return bpmp_ch_sta(channel->bpmp, nr) == MA_ACKD(nr);
+}
+
+static bool tegra210_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
+{
+	int nr = channel->nr;
+
+	return bpmp_ch_sta(channel->bpmp, nr) == SL_SIGL(nr);
+}
+
+static bool tegra210_bpmp_is_req_channel_free(
+	struct tegra_bpmp_channel *channel)
+{
+	int nr = channel->nr;
+
+	return bpmp_ch_sta(channel->bpmp, nr) == MA_FREE(nr);
+}
+
+static bool tegra210_bpmp_is_resp_channel_free(
+	struct tegra_bpmp_channel *channel)
+{
+	int nr = channel->nr;
+
+	return bpmp_ch_sta(channel->bpmp, nr) == SL_QUED(nr);
+}
+
+static int tegra210_bpmp_post_req(struct tegra_bpmp_channel *channel)
+{
+	struct tegra210_bpmp *priv = channel->bpmp->priv;
+	int nr = channel->nr;
+
+	__raw_writel(CH_MASK(nr), priv->arb_sema + CLR_OFFSET);
+
+	return 0;
+}
+
+static int tegra210_bpmp_post_resp(struct tegra_bpmp_channel *channel)
+{
+	struct tegra210_bpmp *priv = channel->bpmp->priv;
+	int nr = channel->nr;
+
+	__raw_writel(MA_ACKD(nr), priv->arb_sema + SET_OFFSET);
+
+	return 0;
+}
+
+static int tegra210_bpmp_ack_resp(struct tegra_bpmp_channel *channel)
+{
+	struct tegra210_bpmp *priv = channel->bpmp->priv;
+	int nr = channel->nr;
+
+	__raw_writel(MA_ACKD(nr) ^ MA_FREE(nr),
+		     priv->arb_sema + CLR_OFFSET);
+
+	return 0;
+}
+
+static int tegra210_bpmp_ack_req(struct tegra_bpmp_channel *channel)
+{
+	struct tegra210_bpmp *priv = channel->bpmp->priv;
+	int nr = channel->nr;
+
+	__raw_writel(SL_QUED(nr), priv->arb_sema + SET_OFFSET);
+
+	return 0;
+}
+
+static int tegra210_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
+{
+	struct tegra210_bpmp *priv = bpmp->priv;
+	struct irq_data *irq_data;
+
+	/* Tegra Legacy Interrupt Controller (LIC) is used to notify
+	 * BPMP of available messages
+	 */
+	irq_data = irq_get_irq_data(priv->txirq);
+	if (!irq_data)
+		return -EINVAL;
+
+	return irq_data->chip->irq_retrigger(irq_data);
+}
+
+static irqreturn_t rx_irq(int irq, void *data)
+{
+	struct tegra_bpmp *bpmp = data;
+
+	tegra_bpmp_handle_rx(bpmp);
+
+	return IRQ_HANDLED;
+}
+
+static int tegra210_bpmp_channel_init(struct tegra_bpmp_channel *channel,
+				      struct tegra_bpmp *bpmp,
+				      unsigned int index)
+{
+	struct tegra210_bpmp *priv = bpmp->priv;
+	uint32_t a;
+	void *p;
+
+	/* Retrieve channel base address from bpmp */
+	writel(index << TRIGGER_ID_SHIFT | TRIGGER_CMD_GET,
+	       priv->atomics + TRIGGER_OFFSET);
+	a = readl(priv->atomics + RESULT_OFFSET(index));
+
+	p = ioremap(a, 0x80);
+	if (!p)
+		return -ENOMEM;
+
+	channel->ib = p;
+	channel->ob = p;
+	channel->nr = index;
+	init_completion(&channel->completion);
+	channel->bpmp = bpmp;
+
+	return 0;
+}
+
+static int tegra210_bpmp_init(struct tegra_bpmp *bpmp)
+{
+	struct platform_device *pdev = to_platform_device(bpmp->dev);
+	struct tegra210_bpmp *priv;
+	struct resource *res;
+	unsigned int i;
+	int err;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	bpmp->priv = priv;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->atomics = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->atomics))
+		return PTR_ERR(priv->atomics);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	priv->arb_sema = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->arb_sema))
+		return PTR_ERR(priv->arb_sema);
+
+	err = tegra210_bpmp_channel_init(bpmp->tx_channel, bpmp,
+					 bpmp->soc->channels.cpu_tx.offset);
+	if (err < 0)
+		return err;
+
+	err = tegra210_bpmp_channel_init(bpmp->rx_channel, bpmp,
+					 bpmp->soc->channels.cpu_rx.offset);
+	if (err < 0)
+		return err;
+
+	for (i = 0; i < bpmp->threaded.count; i++) {
+		err = tegra210_bpmp_channel_init(
+			&bpmp->threaded_channels[i], bpmp,
+			bpmp->soc->channels.thread.offset + i);
+		if (err < 0)
+			return err;
+	}
+
+	err = platform_get_irq_byname(pdev, "tx");
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to get tx IRQ: %d\n", err);
+		return err;
+	}
+	priv->txirq = err;
+
+	err = platform_get_irq_byname(pdev, "rx");
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to get rx IRQ: %d\n", err);
+		return err;
+	}
+	err = devm_request_irq(&pdev->dev, err, rx_irq,
+			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), bpmp);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to request IRQ: %d\n",
+			err);
+		return err;
+	}
+
+	return 0;
+}
+
+struct tegra_bpmp_ops tegra210_bpmp_ops = {
+	.init = tegra210_bpmp_init,
+	.is_resp_ready = tegra210_bpmp_is_resp_ready,
+	.is_req_ready = tegra210_bpmp_is_req_ready,
+	.ack_resp = tegra210_bpmp_ack_resp,
+	.ack_req = tegra210_bpmp_ack_req,
+	.is_resp_channel_free = tegra210_bpmp_is_resp_channel_free,
+	.is_req_channel_free = tegra210_bpmp_is_req_channel_free,
+	.post_resp = tegra210_bpmp_post_resp,
+	.post_req = tegra210_bpmp_post_req,
+	.ring_doorbell = tegra210_bpmp_ring_doorbell,
+};
diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index c6716ec..22c91ab 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -765,17 +765,23 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto free_mrq;
 
-	err = tegra_bpmp_init_clocks(bpmp);
-	if (err < 0)
-		goto free_mrq;
+	if (of_find_property(pdev->dev.of_node, "#clock-cells", NULL)) {
+		err = tegra_bpmp_init_clocks(bpmp);
+		if (err < 0)
+			goto free_mrq;
+	}
 
-	err = tegra_bpmp_init_resets(bpmp);
-	if (err < 0)
-		goto free_mrq;
+	if (of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
+		err = tegra_bpmp_init_resets(bpmp);
+		if (err < 0)
+			goto free_mrq;
+	}
 
-	err = tegra_bpmp_init_powergates(bpmp);
-	if (err < 0)
-		goto free_mrq;
+	if (of_find_property(pdev->dev.of_node, "#power-domain-cells", NULL)) {
+		err = tegra_bpmp_init_powergates(bpmp);
+		if (err < 0)
+			goto free_mrq;
+	}
 
 	err = tegra_bpmp_init_debugfs(bpmp);
 	if (err < 0)
@@ -820,8 +826,30 @@ static const struct tegra_bpmp_soc tegra186_soc = {
 	.num_resets = 193,
 };
 
+static const struct tegra_bpmp_soc tegra210_soc = {
+	.channels = {
+		.cpu_tx = {
+			.offset = 0,
+			.count = 1,
+			.timeout = 60 * USEC_PER_SEC,
+		},
+		.thread = {
+			.offset = 4,
+			.count = 1,
+			.timeout = 600 * USEC_PER_SEC,
+		},
+		.cpu_rx = {
+			.offset = 8,
+			.count = 1,
+			.timeout = 0,
+		},
+	},
+	.ops = &tegra210_bpmp_ops,
+};
+
 static const struct of_device_id tegra_bpmp_match[] = {
 	{ .compatible = "nvidia,tegra186-bpmp", .data = &tegra186_soc },
+	{ .compatible = "nvidia,tegra210-bpmp", .data = &tegra210_soc },
 	{ }
 };
 
diff --git a/include/soc/tegra/bpmp.h b/include/soc/tegra/bpmp.h
index a4818c0..efcbeed 100644
--- a/include/soc/tegra/bpmp.h
+++ b/include/soc/tegra/bpmp.h
@@ -50,6 +50,7 @@ struct tegra_bpmp_channel {
 	struct tegra_bpmp_mb_data *ob;
 	struct completion completion;
 	struct tegra_ivc *ivc;
+	int nr;
 };
 
 typedef void (*tegra_bpmp_mrq_handler_t)(unsigned int mrq,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP
  2019-01-21 12:28 [PATCH V2 0/4] add Tegra210 BPMP driver Timo Alho
                   ` (2 preceding siblings ...)
  2019-01-21 12:28 ` [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210 Timo Alho
@ 2019-01-21 12:28 ` Timo Alho
  2019-01-21 17:38   ` Rob Herring
  2019-01-24 12:19   ` Jon Hunter
  3 siblings, 2 replies; 16+ messages in thread
From: Timo Alho @ 2019-01-21 12:28 UTC (permalink / raw)
  To: treding, sivaramn, robh; +Cc: linux-tegra, devicetree, Timo Alho

The BPMP is a specific processor in Tegra210 chip, which is designed
for boot process handling, assisting in entering deep low power states
(suspend to ram), and offloading DRAM memory clock scaling on some
platforms.

Signed-off-by: Timo Alho <talho@nvidia.com>
---
 .../bindings/firmware/nvidia,tegra210-bpmp.txt     | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt

diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
new file mode 100644
index 0000000..63f439a
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
@@ -0,0 +1,36 @@
+NVIDIA Tegra210 Boot and Power Management Processor (BPMP)
+
+The Boot and Power Management Processor (BPMP) is a co-processor found
+in Tegra210 SoC. It is designed to handle the early stages of the boot
+process as well as to assisting in entering deep low power state
+(suspend to ram), and also offloading DRAM memory clock scaling on
+some platforms. The binding document defines the resources that would
+be used by the BPMP T210 firmware driver, which can create the
+interprocessor communication (IPC) between the CPU and BPMP.
+
+Required properties:
+- name : Should be bpmp
+- compatible
+    Array of strings
+    One of:
+    - "nvidia,tegra210-bpmp"
+- reg: physical base address and length for HW synchornization primitives
+       1) base address and length to Tegra 'atomics' hardware
+       2) base address and length to Tegra 'semaphore' hardware
+- interrupts: specifies the interrupt number for receiving messages ("rx")
+              and for triggering messages ("tx")
+
+Optional properties:
+- #clock-cells : Should be 1 for platforms where DRAM clock control is
+                 offloaded to bpmp.
+
+Example:
+
+bpmp@70016000 {
+	compatible = "nvidia,tegra210-bpmp";
+	reg = <0x0 0x70016000 0x0 0x2000
+	       0x0 0x60001000 0x0 0x1000>;
+	interrupts = <0 6 IRQ_TYPE_NONE>,
+		     <0 4 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "tx", "rx";
+};
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP
  2019-01-21 12:28 ` [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP Timo Alho
@ 2019-01-21 17:38   ` Rob Herring
  2019-01-24 12:19   ` Jon Hunter
  1 sibling, 0 replies; 16+ messages in thread
From: Rob Herring @ 2019-01-21 17:38 UTC (permalink / raw)
  To: Timo Alho; +Cc: treding, sivaramn, robh, linux-tegra, devicetree

On Mon, 21 Jan 2019 14:28:51 +0200, Timo Alho wrote:
> The BPMP is a specific processor in Tegra210 chip, which is designed
> for boot process handling, assisting in entering deep low power states
> (suspend to ram), and offloading DRAM memory clock scaling on some
> platforms.
> 
> Signed-off-by: Timo Alho <talho@nvidia.com>
> ---
>  .../bindings/firmware/nvidia,tegra210-bpmp.txt     | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/4] firmware: tegra: reword messaging terminology
  2019-01-21 12:28 ` [PATCH V2 1/4] firmware: tegra: reword messaging terminology Timo Alho
@ 2019-01-24 11:33   ` Jon Hunter
  2019-01-24 12:25     ` Timo Alho
  0 siblings, 1 reply; 16+ messages in thread
From: Jon Hunter @ 2019-01-24 11:33 UTC (permalink / raw)
  To: Timo Alho, treding, sivaramn, robh; +Cc: linux-tegra, devicetree


On 21/01/2019 12:28, Timo Alho wrote:
> As a preparatory change to refactor bpmp driver to support other than
> t186/t194 chip generations, reword and slightly refactor some of the
> functions to better match with what is actually happening in the
> wire-level protocol.
> 
> The communication with bpmp is essentially a Remote Procedure Call
> consisting of "request" and "response". Either side (BPMP or CPU) can
> initiate the communication. The state machine for communication
> consists of following steps (from Linux point of view):
> 
> Linux initiating the call:
>  1) check that channel is free to transmit a request (is_req_channel_free)
>  2) copy request message payload to shared location
>  3) post the request in channel (post_req)
>  4) notify BPMP that channel state has been updated (ring_doorbell)
>  5) wait for response (is_resp_ready)
>  6) copy response message payload from shared location
>  7) acknowledge the response in channel (ack_resp)
> 
> BPMP initiating the call:
>  1) wait for request (is_req_ready)
>  2) copy request message payload from shared location
>  3) acknowledge the request in channel (ack_req)
>  4) check that channel is free to transmit response (is_resp_channel_free)
>  5) copy response message payload to shared location
>  6) post the response message to channel (post_resp)
>  7) notify BPMP that channel state has been updated (ring_doorbell)
> 
> Signed-off-by: Timo Alho <talho@nvidia.com>
> ---
>  drivers/firmware/tegra/bpmp.c | 106 +++++++++++++++++++++++++++---------------
>  1 file changed, 68 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
> index 689478b..af123de 100644
> --- a/drivers/firmware/tegra/bpmp.c
> +++ b/drivers/firmware/tegra/bpmp.c
> @@ -96,7 +96,7 @@ static bool tegra_bpmp_message_valid(const struct tegra_bpmp_message *msg)
>  	       (msg->rx.size == 0 || msg->rx.data);
>  }
>  
> -static bool tegra_bpmp_master_acked(struct tegra_bpmp_channel *channel)
> +static bool tegra_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
>  {
>  	void *frame;
>  
> @@ -111,7 +111,12 @@ static bool tegra_bpmp_master_acked(struct tegra_bpmp_channel *channel)
>  	return true;
>  }
>  
> -static int tegra_bpmp_wait_ack(struct tegra_bpmp_channel *channel)
> +static bool tegra_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_bpmp_is_resp_ready(channel);
> +}
> +

Any reason not to call this something more generic like
tegra_bpmp_is_message_ready()? I am just wondering if you need to have
this additional function and if it can be avoid. However, not a big
deal, so completely your call.

> +static int tegra_bpmp_wait_resp(struct tegra_bpmp_channel *channel)
>  {
>  	unsigned long timeout = channel->bpmp->soc->channels.cpu_tx.timeout;
>  	ktime_t end;
> @@ -119,14 +124,24 @@ static int tegra_bpmp_wait_ack(struct tegra_bpmp_channel *channel)
>  	end = ktime_add_us(ktime_get(), timeout);
>  
>  	do {
> -		if (tegra_bpmp_master_acked(channel))
> +		if (tegra_bpmp_is_resp_ready(channel))
>  			return 0;
>  	} while (ktime_before(ktime_get(), end));
>  
>  	return -ETIMEDOUT;
>  }
>  
> -static bool tegra_bpmp_master_free(struct tegra_bpmp_channel *channel)
> +static int tegra_bpmp_ack_resp(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_ivc_read_advance(channel->ivc);
> +}
> +
> +static int tegra_bpmp_ack_req(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_ivc_read_advance(channel->ivc);
> +}
> +
> +static bool tegra_bpmp_is_req_channel_free(struct tegra_bpmp_channel *channel)
>  {
>  	void *frame;
>  
> @@ -141,7 +156,12 @@ static bool tegra_bpmp_master_free(struct tegra_bpmp_channel *channel)
>  	return true;
>  }
>  
> -static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
> +static bool tegra_bpmp_is_resp_channel_free(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_bpmp_is_req_channel_free(channel);
> +}
> +

Same here, any reason why not just tegra_bpmp_is_channel_free()?

> +static int tegra_bpmp_wait_req_channel_free(struct tegra_bpmp_channel *channel)
>  {
>  	unsigned long timeout = channel->bpmp->soc->channels.cpu_tx.timeout;
>  	ktime_t start, now;
> @@ -149,7 +169,7 @@ static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
>  	start = ns_to_ktime(local_clock());
>  
>  	do {
> -		if (tegra_bpmp_master_free(channel))
> +		if (tegra_bpmp_is_req_channel_free(channel))
>  			return 0;
>  
>  		now = ns_to_ktime(local_clock());
> @@ -158,6 +178,32 @@ static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
>  	return -ETIMEDOUT;
>  }
>  
> +static int tegra_bpmp_post_req(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_ivc_write_advance(channel->ivc);
> +}
> +
> +static int tegra_bpmp_post_resp(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_ivc_write_advance(channel->ivc);
> +}
> +
> +static int tegra_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
> +{
> +	int err;
> +
> +	if (WARN_ON(bpmp->mbox.channel == NULL))
> +		return -EINVAL;
> +
> +	err = mbox_send_message(bpmp->mbox.channel, NULL);
> +	if (err < 0)
> +		return err;
> +
> +	mbox_client_txdone(bpmp->mbox.channel, 0);
> +
> +	return 0;
> +}
> +
>  static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
>  					 void *data, size_t size, int *ret)
>  {
> @@ -166,7 +212,7 @@ static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
>  	if (data && size > 0)
>  		memcpy(data, channel->ib->data, size);
>  
> -	err = tegra_ivc_read_advance(channel->ivc);
> +	err = tegra_bpmp_ack_resp(channel);
>  	if (err < 0)
>  		return err;
>  
> @@ -210,7 +256,7 @@ static ssize_t __tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,
>  	if (data && size > 0)
>  		memcpy(channel->ob->data, data, size);
>  
> -	return tegra_ivc_write_advance(channel->ivc);
> +	return tegra_bpmp_post_req(channel);
>  }
>  
>  static struct tegra_bpmp_channel *
> @@ -238,7 +284,7 @@ tegra_bpmp_write_threaded(struct tegra_bpmp *bpmp, unsigned int mrq,
>  
>  	channel = &bpmp->threaded_channels[index];
>  
> -	if (!tegra_bpmp_master_free(channel)) {
> +	if (!tegra_bpmp_is_req_channel_free(channel)) {
>  		err = -EBUSY;
>  		goto unlock;
>  	}
> @@ -270,7 +316,7 @@ static ssize_t tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,
>  {
>  	int err;
>  
> -	err = tegra_bpmp_wait_master_free(channel);
> +	err = tegra_bpmp_wait_req_channel_free(channel);
>  	if (err < 0)
>  		return err;
>  
> @@ -302,13 +348,11 @@ int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp,
>  
>  	spin_unlock(&bpmp->atomic_tx_lock);
>  
> -	err = mbox_send_message(bpmp->mbox.channel, NULL);
> +	err = tegra_bpmp_ring_doorbell(bpmp);
>  	if (err < 0)
>  		return err;
>  
> -	mbox_client_txdone(bpmp->mbox.channel, 0);
> -
> -	err = tegra_bpmp_wait_ack(channel);
> +	err = tegra_bpmp_wait_resp(channel);
>  	if (err < 0)
>  		return err;
>  
> @@ -335,12 +379,10 @@ int tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
>  	if (IS_ERR(channel))
>  		return PTR_ERR(channel);
>  
> -	err = mbox_send_message(bpmp->mbox.channel, NULL);
> +	err = tegra_bpmp_ring_doorbell(bpmp);
>  	if (err < 0)
>  		return err;
>  
> -	mbox_client_txdone(bpmp->mbox.channel, 0);
> -
>  	timeout = usecs_to_jiffies(bpmp->soc->channels.thread.timeout);
>  
>  	err = wait_for_completion_timeout(&channel->completion, timeout);
> @@ -369,38 +411,34 @@ void tegra_bpmp_mrq_return(struct tegra_bpmp_channel *channel, int code,
>  {
>  	unsigned long flags = channel->ib->flags;
>  	struct tegra_bpmp *bpmp = channel->bpmp;
> -	struct tegra_bpmp_mb_data *frame;
>  	int err;
>  
>  	if (WARN_ON(size > MSG_DATA_MIN_SZ))
>  		return;
>  
> -	err = tegra_ivc_read_advance(channel->ivc);
> +	err = tegra_bpmp_ack_req(channel);
>  	if (WARN_ON(err < 0))
>  		return;
>  
>  	if ((flags & MSG_ACK) == 0)
>  		return;
>  
> -	frame = tegra_ivc_write_get_next_frame(channel->ivc);
> -	if (WARN_ON(IS_ERR(frame)))
> +	if (WARN_ON(!tegra_bpmp_is_resp_channel_free(channel)))
>  		return;
>  
> -	frame->code = code;
> +	channel->ob->code = code;
>  
>  	if (data && size > 0)
> -		memcpy(frame->data, data, size);
> +		memcpy(channel->ob->data, data, size);
>  
> -	err = tegra_ivc_write_advance(channel->ivc);
> +	err = tegra_bpmp_post_resp(channel);
>  	if (WARN_ON(err < 0))
>  		return;
>  
>  	if (flags & MSG_RING) {
> -		err = mbox_send_message(bpmp->mbox.channel, NULL);
> +		err = tegra_bpmp_ring_doorbell(bpmp);
>  		if (WARN_ON(err < 0))
>  			return;
> -
> -		mbox_client_txdone(bpmp->mbox.channel, 0);
>  	}
>  }
>  EXPORT_SYMBOL_GPL(tegra_bpmp_mrq_return);
> @@ -638,7 +676,7 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
>  	count = bpmp->soc->channels.thread.count;
>  	busy = bpmp->threaded.busy;
>  
> -	if (tegra_bpmp_master_acked(channel))
> +	if (tegra_bpmp_is_req_ready(channel))
>  		tegra_bpmp_handle_mrq(bpmp, channel->ib->code, channel);
>  
>  	spin_lock(&bpmp->lock);
> @@ -648,7 +686,7 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
>  
>  		channel = &bpmp->threaded_channels[i];
>  
> -		if (tegra_bpmp_master_acked(channel)) {
> +		if (tegra_bpmp_is_resp_ready(channel)) {
>  			tegra_bpmp_channel_signal(channel);
>  			clear_bit(i, busy);
>  		}
> @@ -660,16 +698,8 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
>  static void tegra_bpmp_ivc_notify(struct tegra_ivc *ivc, void *data)
>  {
>  	struct tegra_bpmp *bpmp = data;
> -	int err;
>  
> -	if (WARN_ON(bpmp->mbox.channel == NULL))
> -		return;
> -
> -	err = mbox_send_message(bpmp->mbox.channel, NULL);
> -	if (err < 0)
> -		return;
> -
> -	mbox_client_txdone(bpmp->mbox.channel, 0);
> +	WARN_ON(tegra_bpmp_ring_doorbell(bpmp));
>  }
>  
>  static int tegra_bpmp_channel_init(struct tegra_bpmp_channel *channel,
> 

Otherwise ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 2/4] firmware: tegra: refactor bpmp driver
  2019-01-21 12:28 ` [PATCH V2 2/4] firmware: tegra: refactor bpmp driver Timo Alho
@ 2019-01-24 11:45   ` Jon Hunter
  2019-01-24 14:26     ` Timo Alho
  0 siblings, 1 reply; 16+ messages in thread
From: Jon Hunter @ 2019-01-24 11:45 UTC (permalink / raw)
  To: Timo Alho, treding, sivaramn, robh; +Cc: linux-tegra, devicetree


On 21/01/2019 12:28, Timo Alho wrote:
> Split bpmp driver into common and chip specific parts to facilitae

s/facilitae/facilitate

> adding support for previous and futurue Tegra chips that are using

s/futurue/future

> bpmp as co-processor.
> 
> Signed-off-by: Timo Alho <talho@nvidia.com>
> ---
>  drivers/firmware/tegra/Makefile       |   1 +
>  drivers/firmware/tegra/bpmp-private.h |  28 ++++
>  drivers/firmware/tegra/bpmp-t186.c    | 303 ++++++++++++++++++++++++++++++++++
>  drivers/firmware/tegra/bpmp.c         | 253 ++++++----------------------
>  include/soc/tegra/bpmp.h              |  12 +-
>  5 files changed, 389 insertions(+), 208 deletions(-)
>  create mode 100644 drivers/firmware/tegra/bpmp-private.h
>  create mode 100644 drivers/firmware/tegra/bpmp-t186.c
> 
> diff --git a/drivers/firmware/tegra/Makefile b/drivers/firmware/tegra/Makefile
> index 1b826dc..367d482 100644
> --- a/drivers/firmware/tegra/Makefile
> +++ b/drivers/firmware/tegra/Makefile
> @@ -1,4 +1,5 @@
>  tegra-bpmp-y			= bpmp.o
> +tegra-bpmp-$(CONFIG_ARCH_TEGRA_186_SOC)	+= bpmp-t186.o
>  tegra-bpmp-$(CONFIG_DEBUG_FS)	+= bpmp-debugfs.o
>  obj-$(CONFIG_TEGRA_BPMP)	+= tegra-bpmp.o
>  obj-$(CONFIG_TEGRA_IVC)		+= ivc.o
> diff --git a/drivers/firmware/tegra/bpmp-private.h b/drivers/firmware/tegra/bpmp-private.h
> new file mode 100644
> index 0000000..2354337
> --- /dev/null
> +++ b/drivers/firmware/tegra/bpmp-private.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, NVIDIA CORPORATION.
> + */
> +
> +#ifndef __FIRMWARE_TEGRA_BPMP_PRIVATE_H
> +#define __FIRMWARE_TEGRA_BPMP_PRIVATE_H
> +
> +#include <soc/tegra/bpmp.h>
> +
> +struct tegra_bpmp_ops {
> +	int (*init)(struct tegra_bpmp *bpmp);
> +	void (*deinit)(struct tegra_bpmp *bpmp);
> +	bool (*is_resp_ready)(struct tegra_bpmp_channel *channel);
> +	bool (*is_req_ready)(struct tegra_bpmp_channel *channel);
> +	int (*ack_resp)(struct tegra_bpmp_channel *channel);
> +	int (*ack_req)(struct tegra_bpmp_channel *channel);
> +	bool (*is_resp_channel_free)(struct tegra_bpmp_channel *channel);
> +	bool (*is_req_channel_free)(struct tegra_bpmp_channel *channel);
> +	int (*post_resp)(struct tegra_bpmp_channel *channel);
> +	int (*post_req)(struct tegra_bpmp_channel *channel);
> +	int (*ring_doorbell)(struct tegra_bpmp *bpmp);
> +	int (*resume)(struct tegra_bpmp *bpmp);
> +};
> +
> +extern struct tegra_bpmp_ops tegra186_bpmp_ops;
> +
> +#endif
> diff --git a/drivers/firmware/tegra/bpmp-t186.c b/drivers/firmware/tegra/bpmp-t186.c
> new file mode 100644
> index 0000000..e425c6f
> --- /dev/null
> +++ b/drivers/firmware/tegra/bpmp-t186.c
> @@ -0,0 +1,303 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, NVIDIA CORPORATION.
> + */
> +
> +#include <linux/clk/tegra.h>
> +#include <linux/genalloc.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/semaphore.h>
> +
> +#include <soc/tegra/bpmp.h>
> +#include <soc/tegra/bpmp-abi.h>
> +#include <soc/tegra/ivc.h>
> +
> +#include "bpmp-private.h"
> +
> +struct tegra186_bpmp {
> +	struct tegra_bpmp *parent;
> +
> +	struct {
> +		struct gen_pool *pool;
> +		dma_addr_t phys;
> +		void *virt;
> +	} tx, rx;
> +
> +	struct {
> +		struct mbox_client client;
> +		struct mbox_chan *channel;
> +	} mbox;
> +};
> +
> +static inline struct tegra_bpmp *
> +mbox_client_to_bpmp(struct mbox_client *client)
> +{
> +	struct tegra186_bpmp *priv;
> +
> +	priv = container_of(client, struct tegra186_bpmp, mbox.client);
> +
> +	return priv->parent;
> +}
> +
> +static bool tegra186_bpmp_is_message_ready(struct tegra_bpmp_channel *channel)
> +{
> +	void *frame;
> +
> +	frame = tegra_ivc_read_get_next_frame(channel->ivc);
> +	if (IS_ERR(frame)) {
> +		channel->ib = NULL;
> +		return false;
> +	}
> +
> +	channel->ib = frame;
> +
> +	return true;
> +}
> +
> +static bool tegra186_bpmp_is_channel_free(struct tegra_bpmp_channel *channel)
> +{
> +	void *frame;
> +
> +	frame = tegra_ivc_write_get_next_frame(channel->ivc);
> +	if (IS_ERR(frame)) {
> +		channel->ob = NULL;
> +		return false;
> +	}
> +
> +	channel->ob = frame;
> +
> +	return true;
> +}
> +
> +static int tegra186_bpmp_ack_message(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_ivc_read_advance(channel->ivc);
> +}
> +
> +static int tegra186_bpmp_post_message(struct tegra_bpmp_channel *channel)
> +{
> +	return tegra_ivc_write_advance(channel->ivc);
> +}
> +
> +static int tegra186_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
> +{
> +	struct tegra186_bpmp *priv = bpmp->priv;
> +	int err;
> +
> +	err = mbox_send_message(priv->mbox.channel, NULL);
> +	if (err < 0)
> +		return err;
> +
> +	mbox_client_txdone(priv->mbox.channel, 0);
> +
> +	return 0;
> +}
> +
> +static void tegra186_bpmp_ivc_notify(struct tegra_ivc *ivc, void *data)
> +{
> +	struct tegra_bpmp *bpmp = data;
> +	struct tegra186_bpmp *priv = bpmp->priv;
> +
> +	if (WARN_ON(priv->mbox.channel == NULL))
> +		return;
> +
> +	tegra186_bpmp_ring_doorbell(bpmp);
> +}
> +
> +static int tegra186_bpmp_channel_init(struct tegra_bpmp_channel *channel,
> +				   struct tegra_bpmp *bpmp,
> +				   unsigned int index)
> +{
> +	struct tegra186_bpmp *priv = bpmp->priv;
> +	size_t message_size, queue_size;
> +	unsigned int offset;
> +	int err;
> +
> +	channel->ivc = devm_kzalloc(bpmp->dev, sizeof(*channel->ivc),
> +				    GFP_KERNEL);
> +	if (!channel->ivc)
> +		return -ENOMEM;
> +
> +	message_size = tegra_ivc_align(MSG_MIN_SZ);
> +	queue_size = tegra_ivc_total_queue_size(message_size);
> +	offset = queue_size * index;
> +
> +	err = tegra_ivc_init(channel->ivc, NULL,
> +			     priv->rx.virt + offset, priv->rx.phys + offset,
> +			     priv->tx.virt + offset, priv->tx.phys + offset,
> +			     1, message_size, tegra186_bpmp_ivc_notify,
> +			     bpmp);
> +	if (err < 0) {
> +		dev_err(bpmp->dev, "failed to setup IVC for channel %u: %d\n",
> +			index, err);
> +		return err;
> +	}
> +
> +	init_completion(&channel->completion);
> +	channel->bpmp = bpmp;
> +
> +	return 0;
> +}
> +
> +static void tegra186_bpmp_channel_reset(struct tegra_bpmp_channel *channel)
> +{
> +	/* reset the channel state */
> +	tegra_ivc_reset(channel->ivc);
> +
> +	/* sync the channel state with BPMP */
> +	while (tegra_ivc_notified(channel->ivc))
> +		;
> +}
> +
> +static void tegra186_bpmp_channel_cleanup(struct tegra_bpmp_channel *channel)
> +{
> +	tegra_ivc_cleanup(channel->ivc);
> +}
> +
> +static void mbox_handle_rx(struct mbox_client *client, void *data)
> +{
> +	struct tegra_bpmp *bpmp = mbox_client_to_bpmp(client);
> +
> +	tegra_bpmp_handle_rx(bpmp);
> +}
> +
> +static int tegra186_bpmp_init(struct tegra_bpmp *bpmp)
> +{
> +	struct tegra186_bpmp *priv;
> +	unsigned int i;
> +	int err;
> +
> +	priv = devm_kzalloc(bpmp->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	bpmp->priv = priv;
> +	priv->parent = bpmp;
> +
> +	priv->tx.pool = of_gen_pool_get(bpmp->dev->of_node, "shmem", 0);
> +	if (!priv->tx.pool) {
> +		dev_err(bpmp->dev, "TX shmem pool not found\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv->tx.virt = gen_pool_dma_alloc(priv->tx.pool, 4096, &priv->tx.phys);
> +	if (!priv->tx.virt) {
> +		dev_err(bpmp->dev, "failed to allocate from TX pool\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv->rx.pool = of_gen_pool_get(bpmp->dev->of_node, "shmem", 1);
> +	if (!priv->rx.pool) {
> +		dev_err(bpmp->dev, "RX shmem pool not found\n");
> +		err = -ENOMEM;
> +		goto free_tx;
> +	}
> +
> +	priv->rx.virt = gen_pool_dma_alloc(priv->rx.pool, 4096, &priv->rx.phys);
> +	if (!priv->rx.virt) {
> +		dev_err(bpmp->dev, "failed to allocate from RX pool\n");
> +		err = -ENOMEM;
> +		goto free_tx;
> +	}
> +
> +	err = tegra186_bpmp_channel_init(bpmp->tx_channel, bpmp,
> +					 bpmp->soc->channels.cpu_tx.offset);
> +	if (err < 0)
> +		goto free_rx;
> +
> +	err = tegra186_bpmp_channel_init(bpmp->rx_channel, bpmp,
> +					 bpmp->soc->channels.cpu_rx.offset);
> +	if (err < 0)
> +		goto cleanup_tx_channel;
> +
> +	for (i = 0; i < bpmp->threaded.count; i++) {
> +		err = tegra186_bpmp_channel_init(
> +			&bpmp->threaded_channels[i], bpmp,
> +			bpmp->soc->channels.thread.offset + i);
> +		if (err < 0)
> +			goto cleanup_channels;
> +	}
> +
> +	/* mbox registration */
> +	priv->mbox.client.dev = bpmp->dev;
> +	priv->mbox.client.rx_callback = mbox_handle_rx;
> +	priv->mbox.client.tx_block = false;
> +	priv->mbox.client.knows_txdone = false;
> +
> +	priv->mbox.channel = mbox_request_channel(&priv->mbox.client, 0);
> +	if (IS_ERR(priv->mbox.channel)) {
> +		err = PTR_ERR(priv->mbox.channel);
> +		dev_err(bpmp->dev, "failed to get HSP mailbox: %d\n", err);
> +		goto cleanup_channels;
> +	}
> +
> +	tegra186_bpmp_channel_reset(bpmp->tx_channel);
> +	tegra186_bpmp_channel_reset(bpmp->rx_channel);
> +	for (i = 0; i < bpmp->threaded.count; i++)
> +		tegra186_bpmp_channel_reset(&bpmp->threaded_channels[i]);
> +
> +	return 0;
> +
> +cleanup_channels:
> +	for (i = 0; i < bpmp->threaded.count; i++) {
> +		if (bpmp->threaded_channels[i].bpmp)
> +			tegra186_bpmp_channel_cleanup(
> +				&bpmp->threaded_channels[i]);
> +	}
> +
> +	tegra186_bpmp_channel_cleanup(bpmp->rx_channel);
> +cleanup_tx_channel:
> +	tegra186_bpmp_channel_cleanup(bpmp->tx_channel);
> +free_rx:
> +	gen_pool_free(priv->rx.pool, (unsigned long)priv->rx.virt, 4096);
> +free_tx:
> +	gen_pool_free(priv->tx.pool, (unsigned long)priv->tx.virt, 4096);
> +
> +	return err;
> +}
> +
> +static void tegra186_bpmp_deinit(struct tegra_bpmp *bpmp)
> +{
> +	struct tegra186_bpmp *priv = bpmp->priv;
> +	unsigned int i;
> +
> +	mbox_free_channel(priv->mbox.channel);
> +	for (i = 0; i < bpmp->threaded.count; i++)
> +		tegra186_bpmp_channel_cleanup(&bpmp->threaded_channels[i]);
> +	tegra186_bpmp_channel_cleanup(bpmp->rx_channel);
> +	tegra186_bpmp_channel_cleanup(bpmp->tx_channel);
> +	gen_pool_free(priv->rx.pool, (unsigned long)priv->rx.virt, 4096);
> +	gen_pool_free(priv->tx.pool, (unsigned long)priv->tx.virt, 4096);
> +}
> +
> +static int tegra186_bpmp_resume(struct tegra_bpmp *bpmp)
> +{
> +	int i;
> +
> +	/* reset message channels */
> +	tegra186_bpmp_channel_reset(bpmp->tx_channel);
> +	tegra186_bpmp_channel_reset(bpmp->rx_channel);
> +
> +	for (i = 0; i < bpmp->threaded.count; i++)
> +		tegra186_bpmp_channel_reset(&bpmp->threaded_channels[i]);
> +
> +	return 0;
> +}
> +
> +struct tegra_bpmp_ops tegra186_bpmp_ops = {
> +	.init = tegra186_bpmp_init,
> +	.deinit = tegra186_bpmp_deinit,
> +	.is_resp_ready = tegra186_bpmp_is_message_ready,
> +	.is_req_ready = tegra186_bpmp_is_message_ready,
> +	.ack_resp = tegra186_bpmp_ack_message,
> +	.ack_req = tegra186_bpmp_ack_message,
> +	.is_resp_channel_free = tegra186_bpmp_is_channel_free,
> +	.is_req_channel_free = tegra186_bpmp_is_channel_free,
> +	.post_resp = tegra186_bpmp_post_message,
> +	.post_req = tegra186_bpmp_post_message,
> +	.ring_doorbell = tegra186_bpmp_ring_doorbell,
> +	.resume = tegra186_bpmp_resume,
> +};
> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
> index af123de..c6716ec 100644
> --- a/drivers/firmware/tegra/bpmp.c
> +++ b/drivers/firmware/tegra/bpmp.c
> @@ -26,6 +26,8 @@
>  #include <soc/tegra/bpmp-abi.h>
>  #include <soc/tegra/ivc.h>
>  
> +#include "bpmp-private.h"
> +
>  #define MSG_ACK		BIT(0)
>  #define MSG_RING	BIT(1)
>  #define TAG_SZ		32
> @@ -36,6 +38,14 @@ mbox_client_to_bpmp(struct mbox_client *client)
>  	return container_of(client, struct tegra_bpmp, mbox.client);
>  }
>  
> +static inline struct tegra_bpmp_ops *
> +channel_to_ops(struct tegra_bpmp_channel *channel)
> +{
> +	struct tegra_bpmp *bpmp = channel->bpmp;
> +
> +	return bpmp->soc->ops;
> +}
> +
>  struct tegra_bpmp *tegra_bpmp_get(struct device *dev)
>  {
>  	struct platform_device *pdev;
> @@ -98,22 +108,16 @@ static bool tegra_bpmp_message_valid(const struct tegra_bpmp_message *msg)
>  
>  static bool tegra_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
>  {
> -	void *frame;
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
>  
> -	frame = tegra_ivc_read_get_next_frame(channel->ivc);
> -	if (IS_ERR(frame)) {
> -		channel->ib = NULL;
> -		return false;
> -	}
> -
> -	channel->ib = frame;
> -
> -	return true;
> +	return ops->is_resp_ready(channel);
>  }
>  
>  static bool tegra_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
>  {
> -	return tegra_bpmp_is_resp_ready(channel);
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
> +
> +	return ops->is_req_ready(channel);
>  }
>  
>  static int tegra_bpmp_wait_resp(struct tegra_bpmp_channel *channel)
> @@ -133,32 +137,30 @@ static int tegra_bpmp_wait_resp(struct tegra_bpmp_channel *channel)
>  
>  static int tegra_bpmp_ack_resp(struct tegra_bpmp_channel *channel)
>  {
> -	return tegra_ivc_read_advance(channel->ivc);
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
> +
> +	return ops->ack_resp(channel);
>  }
>  
>  static int tegra_bpmp_ack_req(struct tegra_bpmp_channel *channel)
>  {
> -	return tegra_ivc_read_advance(channel->ivc);
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
> +
> +	return ops->ack_req(channel);
>  }
>  
>  static bool tegra_bpmp_is_req_channel_free(struct tegra_bpmp_channel *channel)
>  {
> -	void *frame;
> -
> -	frame = tegra_ivc_write_get_next_frame(channel->ivc);
> -	if (IS_ERR(frame)) {
> -		channel->ob = NULL;
> -		return false;
> -	}
> -
> -	channel->ob = frame;
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
>  
> -	return true;
> +	return ops->is_req_channel_free(channel);
>  }
>  
>  static bool tegra_bpmp_is_resp_channel_free(struct tegra_bpmp_channel *channel)
>  {
> -	return tegra_bpmp_is_req_channel_free(channel);
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
> +
> +	return ops->is_resp_channel_free(channel);
>  }
>  
>  static int tegra_bpmp_wait_req_channel_free(struct tegra_bpmp_channel *channel)
> @@ -180,28 +182,21 @@ static int tegra_bpmp_wait_req_channel_free(struct tegra_bpmp_channel *channel)
>  
>  static int tegra_bpmp_post_req(struct tegra_bpmp_channel *channel)
>  {
> -	return tegra_ivc_write_advance(channel->ivc);
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
> +
> +	return ops->post_req(channel);
>  }
>  
>  static int tegra_bpmp_post_resp(struct tegra_bpmp_channel *channel)
>  {
> -	return tegra_ivc_write_advance(channel->ivc);
> +	struct tegra_bpmp_ops *ops = channel_to_ops(channel);
> +
> +	return ops->post_resp(channel);
>  }
>  
>  static int tegra_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
>  {
> -	int err;
> -
> -	if (WARN_ON(bpmp->mbox.channel == NULL))
> -		return -EINVAL;
> -
> -	err = mbox_send_message(bpmp->mbox.channel, NULL);
> -	if (err < 0)
> -		return err;
> -
> -	mbox_client_txdone(bpmp->mbox.channel, 0);
> -
> -	return 0;
> +	return bpmp->soc->ops->ring_doorbell(bpmp);
>  }
>  
>  static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
> @@ -665,9 +660,8 @@ static void tegra_bpmp_channel_signal(struct tegra_bpmp_channel *channel)
>  	complete(&channel->completion);
>  }
>  
> -static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
> +void tegra_bpmp_handle_rx(struct tegra_bpmp *bpmp)
>  {
> -	struct tegra_bpmp *bpmp = mbox_client_to_bpmp(client);
>  	struct tegra_bpmp_channel *channel;
>  	unsigned int i, count;
>  	unsigned long *busy;
> @@ -695,66 +689,9 @@ static void tegra_bpmp_handle_rx(struct mbox_client *client, void *data)
>  	spin_unlock(&bpmp->lock);
>  }
>  
> -static void tegra_bpmp_ivc_notify(struct tegra_ivc *ivc, void *data)
> -{
> -	struct tegra_bpmp *bpmp = data;
> -
> -	WARN_ON(tegra_bpmp_ring_doorbell(bpmp));
> -}
> -
> -static int tegra_bpmp_channel_init(struct tegra_bpmp_channel *channel,
> -				   struct tegra_bpmp *bpmp,
> -				   unsigned int index)
> -{
> -	size_t message_size, queue_size;
> -	unsigned int offset;
> -	int err;
> -
> -	channel->ivc = devm_kzalloc(bpmp->dev, sizeof(*channel->ivc),
> -				    GFP_KERNEL);
> -	if (!channel->ivc)
> -		return -ENOMEM;
> -
> -	message_size = tegra_ivc_align(MSG_MIN_SZ);
> -	queue_size = tegra_ivc_total_queue_size(message_size);
> -	offset = queue_size * index;
> -
> -	err = tegra_ivc_init(channel->ivc, NULL,
> -			     bpmp->rx.virt + offset, bpmp->rx.phys + offset,
> -			     bpmp->tx.virt + offset, bpmp->tx.phys + offset,
> -			     1, message_size, tegra_bpmp_ivc_notify,
> -			     bpmp);
> -	if (err < 0) {
> -		dev_err(bpmp->dev, "failed to setup IVC for channel %u: %d\n",
> -			index, err);
> -		return err;
> -	}
> -
> -	init_completion(&channel->completion);
> -	channel->bpmp = bpmp;
> -
> -	return 0;
> -}
> -
> -static void tegra_bpmp_channel_reset(struct tegra_bpmp_channel *channel)
> -{
> -	/* reset the channel state */
> -	tegra_ivc_reset(channel->ivc);
> -
> -	/* sync the channel state with BPMP */
> -	while (tegra_ivc_notified(channel->ivc))
> -		;
> -}
> -
> -static void tegra_bpmp_channel_cleanup(struct tegra_bpmp_channel *channel)
> -{
> -	tegra_ivc_cleanup(channel->ivc);
> -}
> -
>  static int tegra_bpmp_probe(struct platform_device *pdev)
>  {
>  	struct tegra_bpmp *bpmp;
> -	unsigned int i;
>  	char tag[TAG_SZ];
>  	size_t size;
>  	int err;
> @@ -766,32 +703,6 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  	bpmp->soc = of_device_get_match_data(&pdev->dev);
>  	bpmp->dev = &pdev->dev;
>  
> -	bpmp->tx.pool = of_gen_pool_get(pdev->dev.of_node, "shmem", 0);
> -	if (!bpmp->tx.pool) {
> -		dev_err(&pdev->dev, "TX shmem pool not found\n");
> -		return -ENOMEM;
> -	}
> -
> -	bpmp->tx.virt = gen_pool_dma_alloc(bpmp->tx.pool, 4096, &bpmp->tx.phys);
> -	if (!bpmp->tx.virt) {
> -		dev_err(&pdev->dev, "failed to allocate from TX pool\n");
> -		return -ENOMEM;
> -	}
> -
> -	bpmp->rx.pool = of_gen_pool_get(pdev->dev.of_node, "shmem", 1);
> -	if (!bpmp->rx.pool) {
> -		dev_err(&pdev->dev, "RX shmem pool not found\n");
> -		err = -ENOMEM;
> -		goto free_tx;
> -	}
> -
> -	bpmp->rx.virt = gen_pool_dma_alloc(bpmp->rx.pool, 4096, &bpmp->rx.phys);
> -	if (!bpmp->rx.virt) {
> -		dev_err(&pdev->dev, "failed to allocate from RX pool\n");
> -		err = -ENOMEM;
> -		goto free_tx;
> -	}
> -
>  	INIT_LIST_HEAD(&bpmp->mrqs);
>  	spin_lock_init(&bpmp->lock);
>  
> @@ -801,81 +712,38 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  	size = BITS_TO_LONGS(bpmp->threaded.count) * sizeof(long);
>  
>  	bpmp->threaded.allocated = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> -	if (!bpmp->threaded.allocated) {
> -		err = -ENOMEM;
> -		goto free_rx;
> -	}
> +	if (!bpmp->threaded.allocated)
> +		return -ENOMEM;
>  
>  	bpmp->threaded.busy = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> -	if (!bpmp->threaded.busy) {
> -		err = -ENOMEM;
> -		goto free_rx;
> -	}
> +	if (!bpmp->threaded.busy)
> +		return -ENOMEM;
>  
>  	spin_lock_init(&bpmp->atomic_tx_lock);
>  	bpmp->tx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->tx_channel),
>  					GFP_KERNEL);
> -	if (!bpmp->tx_channel) {
> -		err = -ENOMEM;
> -		goto free_rx;
> -	}
> +	if (!bpmp->tx_channel)
> +		return -ENOMEM;
>  
>  	bpmp->rx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->rx_channel),
>  	                                GFP_KERNEL);
> -	if (!bpmp->rx_channel) {
> -		err = -ENOMEM;
> -		goto free_rx;
> -	}
> +	if (!bpmp->rx_channel)
> +		return -ENOMEM;
>  
>  	bpmp->threaded_channels = devm_kcalloc(&pdev->dev, bpmp->threaded.count,
>  					       sizeof(*bpmp->threaded_channels),
>  					       GFP_KERNEL);
> -	if (!bpmp->threaded_channels) {
> -		err = -ENOMEM;
> -		goto free_rx;
> -	}
> -
> -	err = tegra_bpmp_channel_init(bpmp->tx_channel, bpmp,
> -				      bpmp->soc->channels.cpu_tx.offset);
> -	if (err < 0)
> -		goto free_rx;
> +	if (!bpmp->threaded_channels)
> +		return -ENOMEM;
>  
> -	err = tegra_bpmp_channel_init(bpmp->rx_channel, bpmp,
> -				      bpmp->soc->channels.cpu_rx.offset);
> +	err = bpmp->soc->ops->init(bpmp);
>  	if (err < 0)
> -		goto cleanup_tx_channel;
> -
> -	for (i = 0; i < bpmp->threaded.count; i++) {
> -		err = tegra_bpmp_channel_init(
> -			&bpmp->threaded_channels[i], bpmp,
> -			bpmp->soc->channels.thread.offset + i);
> -		if (err < 0)
> -			goto cleanup_threaded_channels;
> -	}
> -
> -	/* mbox registration */
> -	bpmp->mbox.client.dev = &pdev->dev;
> -	bpmp->mbox.client.rx_callback = tegra_bpmp_handle_rx;
> -	bpmp->mbox.client.tx_block = false;
> -	bpmp->mbox.client.knows_txdone = false;
> -
> -	bpmp->mbox.channel = mbox_request_channel(&bpmp->mbox.client, 0);
> -	if (IS_ERR(bpmp->mbox.channel)) {
> -		err = PTR_ERR(bpmp->mbox.channel);
> -		dev_err(&pdev->dev, "failed to get HSP mailbox: %d\n", err);
> -		goto cleanup_threaded_channels;
> -	}
> -
> -	/* reset message channels */
> -	tegra_bpmp_channel_reset(bpmp->tx_channel);
> -	tegra_bpmp_channel_reset(bpmp->rx_channel);
> -	for (i = 0; i < bpmp->threaded.count; i++)
> -		tegra_bpmp_channel_reset(&bpmp->threaded_channels[i]);
> +		return err;
>  
>  	err = tegra_bpmp_request_mrq(bpmp, MRQ_PING,
>  				     tegra_bpmp_mrq_handle_ping, bpmp);
>  	if (err < 0)
> -		goto free_mbox;
> +		goto deinit;
>  
>  	err = tegra_bpmp_ping(bpmp);
>  	if (err < 0) {
> @@ -917,37 +785,17 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  
>  free_mrq:
>  	tegra_bpmp_free_mrq(bpmp, MRQ_PING, bpmp);
> -free_mbox:
> -	mbox_free_channel(bpmp->mbox.channel);
> -cleanup_threaded_channels:
> -	for (i = 0; i < bpmp->threaded.count; i++) {
> -		if (bpmp->threaded_channels[i].bpmp)
> -			tegra_bpmp_channel_cleanup(&bpmp->threaded_channels[i]);
> -	}
> +deinit:
> +	bpmp->soc->ops->deinit(bpmp);
>  
> -	tegra_bpmp_channel_cleanup(bpmp->rx_channel);
> -cleanup_tx_channel:
> -	tegra_bpmp_channel_cleanup(bpmp->tx_channel);
> -free_rx:
> -	gen_pool_free(bpmp->rx.pool, (unsigned long)bpmp->rx.virt, 4096);
> -free_tx:
> -	gen_pool_free(bpmp->tx.pool, (unsigned long)bpmp->tx.virt, 4096);
>  	return err;
>  }
>  
>  static int __maybe_unused tegra_bpmp_resume(struct device *dev)
>  {
>  	struct tegra_bpmp *bpmp = dev_get_drvdata(dev);
> -	unsigned int i;
> -
> -	/* reset message channels */
> -	tegra_bpmp_channel_reset(bpmp->tx_channel);
> -	tegra_bpmp_channel_reset(bpmp->rx_channel);
>  
> -	for (i = 0; i < bpmp->threaded.count; i++)
> -		tegra_bpmp_channel_reset(&bpmp->threaded_channels[i]);
> -
> -	return 0;
> +	return bpmp->soc->ops->resume(bpmp);
>  }
>  
>  static SIMPLE_DEV_PM_OPS(tegra_bpmp_pm_ops, NULL, tegra_bpmp_resume);

Looks fine to me, but I do wonder if you need any checks to see if the
ops handler is populated? For example, looking at the Tegra210 BPMP
driver it does not populate resume, yet resume I believe would be called?

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210
  2019-01-21 12:28 ` [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210 Timo Alho
@ 2019-01-24 12:16   ` Jon Hunter
  2019-01-24 13:41     ` Timo Alho
  0 siblings, 1 reply; 16+ messages in thread
From: Jon Hunter @ 2019-01-24 12:16 UTC (permalink / raw)
  To: Timo Alho, treding, sivaramn, robh; +Cc: linux-tegra, devicetree



On 21/01/2019 12:28, Timo Alho wrote:
> This patch adds driver for Tegra210 BPMP firmware.
> 
> The BPMP is a specific processor in Tegra210 chip, which runs firmware
> for assisting in entering deep low power states (suspend to ram), and
> offloading DRAM memory clock scaling on some platforms.
> 
> Based on work by Sivaram Nair <sivaramn@nvidia.com>
> 
> Signed-off-by: Timo Alho <talho@nvidia.com>
> ---
>  drivers/firmware/tegra/Makefile       |   1 +
>  drivers/firmware/tegra/bpmp-private.h |   1 +
>  drivers/firmware/tegra/bpmp-t210.c    | 241 ++++++++++++++++++++++++++++++++++
>  drivers/firmware/tegra/bpmp.c         |  46 +++++--
>  include/soc/tegra/bpmp.h              |   1 +
>  5 files changed, 281 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/firmware/tegra/bpmp-t210.c
> 
> diff --git a/drivers/firmware/tegra/Makefile b/drivers/firmware/tegra/Makefile
> index 367d482..dc41d6f 100644
> --- a/drivers/firmware/tegra/Makefile
> +++ b/drivers/firmware/tegra/Makefile
> @@ -1,5 +1,6 @@
>  tegra-bpmp-y			= bpmp.o
>  tegra-bpmp-$(CONFIG_ARCH_TEGRA_186_SOC)	+= bpmp-t186.o
> +tegra-bpmp-$(CONFIG_ARCH_TEGRA_210_SOC)	+= bpmp-t210.o
>  tegra-bpmp-$(CONFIG_DEBUG_FS)	+= bpmp-debugfs.o
>  obj-$(CONFIG_TEGRA_BPMP)	+= tegra-bpmp.o
>  obj-$(CONFIG_TEGRA_IVC)		+= ivc.o
> diff --git a/drivers/firmware/tegra/bpmp-private.h b/drivers/firmware/tegra/bpmp-private.h
> index 2354337..5132234 100644
> --- a/drivers/firmware/tegra/bpmp-private.h
> +++ b/drivers/firmware/tegra/bpmp-private.h
> @@ -24,5 +24,6 @@ struct tegra_bpmp_ops {
>  };
>  
>  extern struct tegra_bpmp_ops tegra186_bpmp_ops;
> +extern struct tegra_bpmp_ops tegra210_bpmp_ops;
>  
>  #endif
> diff --git a/drivers/firmware/tegra/bpmp-t210.c b/drivers/firmware/tegra/bpmp-t210.c
> new file mode 100644
> index 0000000..cb16c94
> --- /dev/null
> +++ b/drivers/firmware/tegra/bpmp-t210.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, NVIDIA CORPORATION.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <soc/tegra/bpmp.h>
> +
> +#include "bpmp-private.h"
> +
> +#define TRIGGER_OFFSET		0x000
> +#define RESULT_OFFSET(id)	(0xc00 + id * 4)
> +#define TRIGGER_ID_SHIFT	16
> +#define TRIGGER_CMD_GET		4
> +
> +#define STA_OFFSET		0
> +#define SET_OFFSET		4
> +#define CLR_OFFSET		8
> +
> +#define CH_MASK(ch)	(0x3 << ((ch) * 2))
> +#define SL_SIGL(ch)	(0x0 << ((ch) * 2))
> +#define SL_QUED(ch)	(0x1 << ((ch) * 2))
> +#define MA_FREE(ch)	(0x2 << ((ch) * 2))
> +#define MA_ACKD(ch)	(0x3 << ((ch) * 2))
> +
> +struct tegra210_bpmp {
> +	void __iomem *atomics;
> +	void __iomem *arb_sema;
> +	unsigned int txirq;
> +};
> +
> +static uint32_t bpmp_ch_sta(struct tegra_bpmp *bpmp, int ch)
> +{
> +	struct tegra210_bpmp *priv = bpmp->priv;
> +
> +	return __raw_readl(priv->arb_sema + STA_OFFSET) & CH_MASK(ch);
> +}
> +
> +static bool tegra210_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
> +{
> +	int nr = channel->nr;
> +
> +	return bpmp_ch_sta(channel->bpmp, nr) == MA_ACKD(nr);
> +}
> +
> +static bool tegra210_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
> +{
> +	int nr = channel->nr;
> +
> +	return bpmp_ch_sta(channel->bpmp, nr) == SL_SIGL(nr);
> +}
> +
> +static bool tegra210_bpmp_is_req_channel_free(
> +	struct tegra_bpmp_channel *channel)
> +{
> +	int nr = channel->nr;
> +
> +	return bpmp_ch_sta(channel->bpmp, nr) == MA_FREE(nr);
> +}
> +
> +static bool tegra210_bpmp_is_resp_channel_free(
> +	struct tegra_bpmp_channel *channel)
> +{
> +	int nr = channel->nr;
> +
> +	return bpmp_ch_sta(channel->bpmp, nr) == SL_QUED(nr);
> +}
> +
> +static int tegra210_bpmp_post_req(struct tegra_bpmp_channel *channel)
> +{
> +	struct tegra210_bpmp *priv = channel->bpmp->priv;
> +	int nr = channel->nr;
> +
> +	__raw_writel(CH_MASK(nr), priv->arb_sema + CLR_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int tegra210_bpmp_post_resp(struct tegra_bpmp_channel *channel)
> +{
> +	struct tegra210_bpmp *priv = channel->bpmp->priv;
> +	int nr = channel->nr;
> +
> +	__raw_writel(MA_ACKD(nr), priv->arb_sema + SET_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int tegra210_bpmp_ack_resp(struct tegra_bpmp_channel *channel)
> +{
> +	struct tegra210_bpmp *priv = channel->bpmp->priv;
> +	int nr = channel->nr;
> +
> +	__raw_writel(MA_ACKD(nr) ^ MA_FREE(nr),
> +		     priv->arb_sema + CLR_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int tegra210_bpmp_ack_req(struct tegra_bpmp_channel *channel)
> +{
> +	struct tegra210_bpmp *priv = channel->bpmp->priv;
> +	int nr = channel->nr;
> +
> +	__raw_writel(SL_QUED(nr), priv->arb_sema + SET_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int tegra210_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
> +{
> +	struct tegra210_bpmp *priv = bpmp->priv;
> +	struct irq_data *irq_data;
> +
> +	/* Tegra Legacy Interrupt Controller (LIC) is used to notify
> +	 * BPMP of available messages
> +	 */
> +	irq_data = irq_get_irq_data(priv->txirq);
> +	if (!irq_data)
> +		return -EINVAL;

Why not check this in probe?

> +
> +	return irq_data->chip->irq_retrigger(irq_data);

We should check that the irq_retrigger is populated as well.

In general, I am not sure if there is a better way to do this, but I
don't see an alternative.

> +}
> +
> +static irqreturn_t rx_irq(int irq, void *data)
> +{
> +	struct tegra_bpmp *bpmp = data;
> +
> +	tegra_bpmp_handle_rx(bpmp);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int tegra210_bpmp_channel_init(struct tegra_bpmp_channel *channel,
> +				      struct tegra_bpmp *bpmp,
> +				      unsigned int index)
> +{
> +	struct tegra210_bpmp *priv = bpmp->priv;
> +	uint32_t a;
> +	void *p;
> +
> +	/* Retrieve channel base address from bpmp */
> +	writel(index << TRIGGER_ID_SHIFT | TRIGGER_CMD_GET,
> +	       priv->atomics + TRIGGER_OFFSET);
> +	a = readl(priv->atomics + RESULT_OFFSET(index));
> +
> +	p = ioremap(a, 0x80);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	channel->ib = p;
> +	channel->ob = p;
> +	channel->nr = index;
> +	init_completion(&channel->completion);
> +	channel->bpmp = bpmp;
> +
> +	return 0;
> +}
> +
> +static int tegra210_bpmp_init(struct tegra_bpmp *bpmp)
> +{
> +	struct platform_device *pdev = to_platform_device(bpmp->dev);
> +	struct tegra210_bpmp *priv;
> +	struct resource *res;
> +	unsigned int i;
> +	int err;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	bpmp->priv = priv;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->atomics = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->atomics))
> +		return PTR_ERR(priv->atomics);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	priv->arb_sema = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->arb_sema))
> +		return PTR_ERR(priv->arb_sema);
> +
> +	err = tegra210_bpmp_channel_init(bpmp->tx_channel, bpmp,
> +					 bpmp->soc->channels.cpu_tx.offset);
> +	if (err < 0)
> +		return err;
> +
> +	err = tegra210_bpmp_channel_init(bpmp->rx_channel, bpmp,
> +					 bpmp->soc->channels.cpu_rx.offset);
> +	if (err < 0)
> +		return err;

Don't we need to unmap any iomem on error that we mapped in
tegra210_bpmp_channel_init()?

> +
> +	for (i = 0; i < bpmp->threaded.count; i++) {
> +		err = tegra210_bpmp_channel_init(
> +			&bpmp->threaded_channels[i], bpmp,
> +			bpmp->soc->channels.thread.offset + i);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	err = platform_get_irq_byname(pdev, "tx");
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "failed to get tx IRQ: %d\n", err);
> +		return err;
> +	}
> +	priv->txirq = err;
> +
> +	err = platform_get_irq_byname(pdev, "rx");
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "failed to get rx IRQ: %d\n", err);
> +		return err;
> +	}
> +	err = devm_request_irq(&pdev->dev, err, rx_irq,
> +			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), bpmp);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "failed to request IRQ: %d\n",
> +			err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +struct tegra_bpmp_ops tegra210_bpmp_ops = {
> +	.init = tegra210_bpmp_init,
> +	.is_resp_ready = tegra210_bpmp_is_resp_ready,
> +	.is_req_ready = tegra210_bpmp_is_req_ready,
> +	.ack_resp = tegra210_bpmp_ack_resp,
> +	.ack_req = tegra210_bpmp_ack_req,
> +	.is_resp_channel_free = tegra210_bpmp_is_resp_channel_free,
> +	.is_req_channel_free = tegra210_bpmp_is_req_channel_free,
> +	.post_resp = tegra210_bpmp_post_resp,
> +	.post_req = tegra210_bpmp_post_req,
> +	.ring_doorbell = tegra210_bpmp_ring_doorbell,
> +};
> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
> index c6716ec..22c91ab 100644
> --- a/drivers/firmware/tegra/bpmp.c
> +++ b/drivers/firmware/tegra/bpmp.c
> @@ -765,17 +765,23 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto free_mrq;
>  
> -	err = tegra_bpmp_init_clocks(bpmp);
> -	if (err < 0)
> -		goto free_mrq;
> +	if (of_find_property(pdev->dev.of_node, "#clock-cells", NULL)) {
> +		err = tegra_bpmp_init_clocks(bpmp);
> +		if (err < 0)
> +			goto free_mrq;
> +	}
>  
> -	err = tegra_bpmp_init_resets(bpmp);
> -	if (err < 0)
> -		goto free_mrq;
> +	if (of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
> +		err = tegra_bpmp_init_resets(bpmp);
> +		if (err < 0)
> +			goto free_mrq;
> +	}
>  
> -	err = tegra_bpmp_init_powergates(bpmp);
> -	if (err < 0)
> -		goto free_mrq;
> +	if (of_find_property(pdev->dev.of_node, "#power-domain-cells", NULL)) {
> +		err = tegra_bpmp_init_powergates(bpmp);
> +		if (err < 0)
> +			goto free_mrq;
> +	}

Should we use soc_data for these rather than relying on the nodes to be
populated correctly in DT?

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP
  2019-01-21 12:28 ` [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP Timo Alho
  2019-01-21 17:38   ` Rob Herring
@ 2019-01-24 12:19   ` Jon Hunter
  2019-01-24 13:02     ` Jon Hunter
  1 sibling, 1 reply; 16+ messages in thread
From: Jon Hunter @ 2019-01-24 12:19 UTC (permalink / raw)
  To: Timo Alho, treding, sivaramn, robh; +Cc: linux-tegra, devicetree


On 21/01/2019 12:28, Timo Alho wrote:
> The BPMP is a specific processor in Tegra210 chip, which is designed
> for boot process handling, assisting in entering deep low power states
> (suspend to ram), and offloading DRAM memory clock scaling on some
> platforms.
> 
> Signed-off-by: Timo Alho <talho@nvidia.com>
> ---
>  .../bindings/firmware/nvidia,tegra210-bpmp.txt     | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
> new file mode 100644
> index 0000000..63f439a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
> @@ -0,0 +1,36 @@
> +NVIDIA Tegra210 Boot and Power Management Processor (BPMP)
> +
> +The Boot and Power Management Processor (BPMP) is a co-processor found
> +in Tegra210 SoC. It is designed to handle the early stages of the boot
> +process as well as to assisting in entering deep low power state
> +(suspend to ram), and also offloading DRAM memory clock scaling on
> +some platforms. The binding document defines the resources that would
> +be used by the BPMP T210 firmware driver, which can create the
> +interprocessor communication (IPC) between the CPU and BPMP.
> +
> +Required properties:
> +- name : Should be bpmp
> +- compatible
> +    Array of strings
> +    One of:
> +    - "nvidia,tegra210-bpmp"
> +- reg: physical base address and length for HW synchornization primitives
> +       1) base address and length to Tegra 'atomics' hardware
> +       2) base address and length to Tegra 'semaphore' hardware
> +- interrupts: specifies the interrupt number for receiving messages ("rx")
> +              and for triggering messages ("tx")
> +
> +Optional properties:
> +- #clock-cells : Should be 1 for platforms where DRAM clock control is
> +                 offloaded to bpmp.
> +
> +Example:
> +
> +bpmp@70016000 {
> +	compatible = "nvidia,tegra210-bpmp";
> +	reg = <0x0 0x70016000 0x0 0x2000
> +	       0x0 0x60001000 0x0 0x1000>;
> +	interrupts = <0 6 IRQ_TYPE_NONE>,
> +		     <0 4 IRQ_TYPE_EDGE_RISING>;
> +	interrupt-names = "tx", "rx";
> +};

Under required properties you have 'name' but it does not feature in the
above example. In fact, I see the same for the Tegra186 BPMP binding.

Do we actually use/need this name property?

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/4] firmware: tegra: reword messaging terminology
  2019-01-24 11:33   ` Jon Hunter
@ 2019-01-24 12:25     ` Timo Alho
  0 siblings, 0 replies; 16+ messages in thread
From: Timo Alho @ 2019-01-24 12:25 UTC (permalink / raw)
  To: Jon Hunter, treding, sivaramn, robh; +Cc: linux-tegra, devicetree


Hi Jon,

On 24.1.2019 13.33, Jon Hunter wrote:
> 
> On 21/01/2019 12:28, Timo Alho wrote:
>> As a preparatory change to refactor bpmp driver to support other than
>> t186/t194 chip generations, reword and slightly refactor some of the
>> functions to better match with what is actually happening in the
>> wire-level protocol.
>>
>> The communication with bpmp is essentially a Remote Procedure Call
>> consisting of "request" and "response". Either side (BPMP or CPU) can
>> initiate the communication. The state machine for communication
>> consists of following steps (from Linux point of view):
>>
>> Linux initiating the call:
>>   1) check that channel is free to transmit a request (is_req_channel_free)
>>   2) copy request message payload to shared location
>>   3) post the request in channel (post_req)
>>   4) notify BPMP that channel state has been updated (ring_doorbell)
>>   5) wait for response (is_resp_ready)
>>   6) copy response message payload from shared location
>>   7) acknowledge the response in channel (ack_resp)
>>
>> BPMP initiating the call:
>>   1) wait for request (is_req_ready)
>>   2) copy request message payload from shared location
>>   3) acknowledge the request in channel (ack_req)
>>   4) check that channel is free to transmit response (is_resp_channel_free)
>>   5) copy response message payload to shared location
>>   6) post the response message to channel (post_resp)
>>   7) notify BPMP that channel state has been updated (ring_doorbell)
>>
>> Signed-off-by: Timo Alho <talho@nvidia.com>
>> ---
>>   drivers/firmware/tegra/bpmp.c | 106 +++++++++++++++++++++++++++---------------
>>   1 file changed, 68 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
>> index 689478b..af123de 100644
>> --- a/drivers/firmware/tegra/bpmp.c
>> +++ b/drivers/firmware/tegra/bpmp.c
>> @@ -96,7 +96,7 @@ static bool tegra_bpmp_message_valid(const struct tegra_bpmp_message *msg)
>>   	       (msg->rx.size == 0 || msg->rx.data);
>>   }
>>   
>> -static bool tegra_bpmp_master_acked(struct tegra_bpmp_channel *channel)
>> +static bool tegra_bpmp_is_resp_ready(struct tegra_bpmp_channel *channel)
>>   {
>>   	void *frame;
>>   
>> @@ -111,7 +111,12 @@ static bool tegra_bpmp_master_acked(struct tegra_bpmp_channel *channel)
>>   	return true;
>>   }
>>   
>> -static int tegra_bpmp_wait_ack(struct tegra_bpmp_channel *channel)
>> +static bool tegra_bpmp_is_req_ready(struct tegra_bpmp_channel *channel)
>> +{
>> +	return tegra_bpmp_is_resp_ready(channel);
>> +}
>> +
> 
> Any reason not to call this something more generic like
> tegra_bpmp_is_message_ready()? I am just wondering if you need to have
> this additional function and if it can be avoid. However, not a big
> deal, so completely your call.

It is true that it looks bit nonsensical in this patch. However, I made 
it like this in the anticipation of the follow up patches -- the use of 
separate req_ready() and resp_ready() becomes obvious once t210 bpmp 
support is added (where req and resp have different implementation).

So, I'd just leave it as is and I see also Acked this patch already. Thanks.

BR,
Timo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP
  2019-01-24 12:19   ` Jon Hunter
@ 2019-01-24 13:02     ` Jon Hunter
  2019-01-24 14:49       ` Timo Alho
  0 siblings, 1 reply; 16+ messages in thread
From: Jon Hunter @ 2019-01-24 13:02 UTC (permalink / raw)
  To: Timo Alho, treding, sivaramn, robh; +Cc: linux-tegra, devicetree


On 24/01/2019 12:19, Jon Hunter wrote:
> 
> On 21/01/2019 12:28, Timo Alho wrote:
>> The BPMP is a specific processor in Tegra210 chip, which is designed
>> for boot process handling, assisting in entering deep low power states
>> (suspend to ram), and offloading DRAM memory clock scaling on some
>> platforms.
>>
>> Signed-off-by: Timo Alho <talho@nvidia.com>
>> ---
>>  .../bindings/firmware/nvidia,tegra210-bpmp.txt     | 36 ++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
>> new file mode 100644
>> index 0000000..63f439a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
>> @@ -0,0 +1,36 @@
>> +NVIDIA Tegra210 Boot and Power Management Processor (BPMP)
>> +
>> +The Boot and Power Management Processor (BPMP) is a co-processor found
>> +in Tegra210 SoC. It is designed to handle the early stages of the boot
>> +process as well as to assisting in entering deep low power state
>> +(suspend to ram), and also offloading DRAM memory clock scaling on
>> +some platforms. The binding document defines the resources that would
>> +be used by the BPMP T210 firmware driver, which can create the
>> +interprocessor communication (IPC) between the CPU and BPMP.
>> +
>> +Required properties:
>> +- name : Should be bpmp
>> +- compatible
>> +    Array of strings
>> +    One of:
>> +    - "nvidia,tegra210-bpmp"
>> +- reg: physical base address and length for HW synchornization primitives
>> +       1) base address and length to Tegra 'atomics' hardware
>> +       2) base address and length to Tegra 'semaphore' hardware
>> +- interrupts: specifies the interrupt number for receiving messages ("rx")
>> +              and for triggering messages ("tx")
>> +
>> +Optional properties:
>> +- #clock-cells : Should be 1 for platforms where DRAM clock control is
>> +                 offloaded to bpmp.
>> +
>> +Example:
>> +
>> +bpmp@70016000 {
>> +	compatible = "nvidia,tegra210-bpmp";
>> +	reg = <0x0 0x70016000 0x0 0x2000
>> +	       0x0 0x60001000 0x0 0x1000>;
>> +	interrupts = <0 6 IRQ_TYPE_NONE>,
>> +		     <0 4 IRQ_TYPE_EDGE_RISING>;
>> +	interrupt-names = "tx", "rx";
>> +};
> 
> Under required properties you have 'name' but it does not feature in the
> above example. In fact, I see the same for the Tegra186 BPMP binding.
> 
> Do we actually use/need this name property?

I am wondering if 'TYPE_NONE' for TX is correct here. I understand that
Linux does not request this interrupt, but still shouldn't DT reflect it
actual type?

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210
  2019-01-24 12:16   ` Jon Hunter
@ 2019-01-24 13:41     ` Timo Alho
  2019-01-24 13:57       ` Jon Hunter
  0 siblings, 1 reply; 16+ messages in thread
From: Timo Alho @ 2019-01-24 13:41 UTC (permalink / raw)
  To: Jon Hunter, treding, sivaramn, robh; +Cc: linux-tegra, devicetree

Hi Jon,

Thanks for reviewing :)

On 24.1.2019 14.16, Jon Hunter wrote:

...

>> +static int tegra210_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
>> +{
>> +	struct tegra210_bpmp *priv = bpmp->priv;
>> +	struct irq_data *irq_data;
>> +
>> +	/* Tegra Legacy Interrupt Controller (LIC) is used to notify
>> +	 * BPMP of available messages
>> +	 */
>> +	irq_data = irq_get_irq_data(priv->txirq);
>> +	if (!irq_data)
>> +		return -EINVAL;
> 
> Why not check this in probe?
> 

Indeed I can move irq_get_irq_data() to probe and store the return value 
directly to priv->txirq instead. I'll just do that then.

>> +
>> +	return irq_data->chip->irq_retrigger(irq_data);
> 
> We should check that the irq_retrigger is populated as well.

I'll add a check.

> In general, I am not sure if there is a better way to do this, but I
> don't see an alternative.
> 

I'd be also happy to hear if someone has good alternative.

...

>> +static int tegra210_bpmp_init(struct tegra_bpmp *bpmp)
>> +{
>> +	struct platform_device *pdev = to_platform_device(bpmp->dev);
>> +	struct tegra210_bpmp *priv;
>> +	struct resource *res;
>> +	unsigned int i;
>> +	int err;
>> +
>> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	bpmp->priv = priv;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	priv->atomics = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(priv->atomics))
>> +		return PTR_ERR(priv->atomics);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	priv->arb_sema = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(priv->arb_sema))
>> +		return PTR_ERR(priv->arb_sema);
>> +
>> +	err = tegra210_bpmp_channel_init(bpmp->tx_channel, bpmp,
>> +					 bpmp->soc->channels.cpu_tx.offset);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	err = tegra210_bpmp_channel_init(bpmp->rx_channel, bpmp,
>> +					 bpmp->soc->channels.cpu_rx.offset);
>> +	if (err < 0)
>> +		return err;
> 
> Don't we need to unmap any iomem on error that we mapped in
> tegra210_bpmp_channel_init()?

Good point. I'll just replace ioremap() with devm_ioremap(). I think
that should do it.

...

>> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
>> index c6716ec..22c91ab 100644
>> --- a/drivers/firmware/tegra/bpmp.c
>> +++ b/drivers/firmware/tegra/bpmp.c
>> @@ -765,17 +765,23 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>>   	if (err < 0)
>>   		goto free_mrq;
>>   
>> -	err = tegra_bpmp_init_clocks(bpmp);
>> -	if (err < 0)
>> -		goto free_mrq;
>> +	if (of_find_property(pdev->dev.of_node, "#clock-cells", NULL)) {
>> +		err = tegra_bpmp_init_clocks(bpmp);
>> +		if (err < 0)
>> +			goto free_mrq;
>> +	}
>>   
>> -	err = tegra_bpmp_init_resets(bpmp);
>> -	if (err < 0)
>> -		goto free_mrq;
>> +	if (of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
>> +		err = tegra_bpmp_init_resets(bpmp);
>> +		if (err < 0)
>> +			goto free_mrq;
>> +	}
>>   
>> -	err = tegra_bpmp_init_powergates(bpmp);
>> -	if (err < 0)
>> -		goto free_mrq;
>> +	if (of_find_property(pdev->dev.of_node, "#power-domain-cells", NULL)) {
>> +		err = tegra_bpmp_init_powergates(bpmp);
>> +		if (err < 0)
>> +			goto free_mrq;
>> +	}
> 
> Should we use soc_data for these rather than relying on the nodes to be
> populated correctly in DT?

There are some t210 systems where BPMP functions as clocks provider (for 
EMC), and some where it does not. So controlling this via device tree 
can be handier.

> Cheers
> Jon
> 

BR,
Timo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210
  2019-01-24 13:41     ` Timo Alho
@ 2019-01-24 13:57       ` Jon Hunter
  0 siblings, 0 replies; 16+ messages in thread
From: Jon Hunter @ 2019-01-24 13:57 UTC (permalink / raw)
  To: Timo Alho, treding, sivaramn, robh; +Cc: linux-tegra, devicetree


On 24/01/2019 13:41, Timo Alho wrote:
> Hi Jon,
> 
> Thanks for reviewing :)
> 
> On 24.1.2019 14.16, Jon Hunter wrote:
> 
> ...
> 
>>> +static int tegra210_bpmp_ring_doorbell(struct tegra_bpmp *bpmp)
>>> +{
>>> +    struct tegra210_bpmp *priv = bpmp->priv;
>>> +    struct irq_data *irq_data;
>>> +
>>> +    /* Tegra Legacy Interrupt Controller (LIC) is used to notify
>>> +     * BPMP of available messages
>>> +     */
>>> +    irq_data = irq_get_irq_data(priv->txirq);
>>> +    if (!irq_data)
>>> +        return -EINVAL;
>>
>> Why not check this in probe?
>>
> 
> Indeed I can move irq_get_irq_data() to probe and store the return value
> directly to priv->txirq instead. I'll just do that then.
> 
>>> +
>>> +    return irq_data->chip->irq_retrigger(irq_data);
>>
>> We should check that the irq_retrigger is populated as well.
> 
> I'll add a check.
> 
>> In general, I am not sure if there is a better way to do this, but I
>> don't see an alternative.
>>
> 
> I'd be also happy to hear if someone has good alternative.
> 
> ...
> 
>>> +static int tegra210_bpmp_init(struct tegra_bpmp *bpmp)
>>> +{
>>> +    struct platform_device *pdev = to_platform_device(bpmp->dev);
>>> +    struct tegra210_bpmp *priv;
>>> +    struct resource *res;
>>> +    unsigned int i;
>>> +    int err;
>>> +
>>> +    priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>> +    if (!priv)
>>> +        return -ENOMEM;
>>> +
>>> +    bpmp->priv = priv;
>>> +
>>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +    priv->atomics = devm_ioremap_resource(&pdev->dev, res);
>>> +    if (IS_ERR(priv->atomics))
>>> +        return PTR_ERR(priv->atomics);
>>> +
>>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> +    priv->arb_sema = devm_ioremap_resource(&pdev->dev, res);
>>> +    if (IS_ERR(priv->arb_sema))
>>> +        return PTR_ERR(priv->arb_sema);
>>> +
>>> +    err = tegra210_bpmp_channel_init(bpmp->tx_channel, bpmp,
>>> +                     bpmp->soc->channels.cpu_tx.offset);
>>> +    if (err < 0)
>>> +        return err;
>>> +
>>> +    err = tegra210_bpmp_channel_init(bpmp->rx_channel, bpmp,
>>> +                     bpmp->soc->channels.cpu_rx.offset);
>>> +    if (err < 0)
>>> +        return err;
>>
>> Don't we need to unmap any iomem on error that we mapped in
>> tegra210_bpmp_channel_init()?
> 
> Good point. I'll just replace ioremap() with devm_ioremap(). I think
> that should do it.
> 
> ...
> 
>>> diff --git a/drivers/firmware/tegra/bpmp.c
>>> b/drivers/firmware/tegra/bpmp.c
>>> index c6716ec..22c91ab 100644
>>> --- a/drivers/firmware/tegra/bpmp.c
>>> +++ b/drivers/firmware/tegra/bpmp.c
>>> @@ -765,17 +765,23 @@ static int tegra_bpmp_probe(struct
>>> platform_device *pdev)
>>>       if (err < 0)
>>>           goto free_mrq;
>>>   -    err = tegra_bpmp_init_clocks(bpmp);
>>> -    if (err < 0)
>>> -        goto free_mrq;
>>> +    if (of_find_property(pdev->dev.of_node, "#clock-cells", NULL)) {
>>> +        err = tegra_bpmp_init_clocks(bpmp);
>>> +        if (err < 0)
>>> +            goto free_mrq;
>>> +    }
>>>   -    err = tegra_bpmp_init_resets(bpmp);
>>> -    if (err < 0)
>>> -        goto free_mrq;
>>> +    if (of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
>>> +        err = tegra_bpmp_init_resets(bpmp);
>>> +        if (err < 0)
>>> +            goto free_mrq;
>>> +    }
>>>   -    err = tegra_bpmp_init_powergates(bpmp);
>>> -    if (err < 0)
>>> -        goto free_mrq;
>>> +    if (of_find_property(pdev->dev.of_node, "#power-domain-cells",
>>> NULL)) {
>>> +        err = tegra_bpmp_init_powergates(bpmp);
>>> +        if (err < 0)
>>> +            goto free_mrq;
>>> +    }
>>
>> Should we use soc_data for these rather than relying on the nodes to be
>> populated correctly in DT?
> 
> There are some t210 systems where BPMP functions as clocks provider (for
> EMC), and some where it does not. So controlling this via device tree
> can be handier.

Is it the same T210 device or could the compatibility string be used
here for this?

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 2/4] firmware: tegra: refactor bpmp driver
  2019-01-24 11:45   ` Jon Hunter
@ 2019-01-24 14:26     ` Timo Alho
  0 siblings, 0 replies; 16+ messages in thread
From: Timo Alho @ 2019-01-24 14:26 UTC (permalink / raw)
  To: Jon Hunter, treding, sivaramn, robh; +Cc: linux-tegra, devicetree

Hi Jon,


On 24.1.2019 13.45, Jon Hunter wrote:
> 
> On 21/01/2019 12:28, Timo Alho wrote:
>> Split bpmp driver into common and chip specific parts to facilitae
> 
> s/facilitae/facilitate
> 
>> adding support for previous and futurue Tegra chips that are using
> 
> s/futurue/future >

Thanks for pointing out. I think I need to setup up some spell checker...

...

>>   static int tegra_bpmp_probe(struct platform_device *pdev)
>>   {
>>   	struct tegra_bpmp *bpmp;
>> -	unsigned int i;
>>   	char tag[TAG_SZ];
>>   	size_t size;
>>   	int err;
>> @@ -766,32 +703,6 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>>   	bpmp->soc = of_device_get_match_data(&pdev->dev);
>>   	bpmp->dev = &pdev->dev;
>>   
>> -	bpmp->tx.pool = of_gen_pool_get(pdev->dev.of_node, "shmem", 0);
>> -	if (!bpmp->tx.pool) {
>> -		dev_err(&pdev->dev, "TX shmem pool not found\n");
>> -		return -ENOMEM;
>> -	}
>> -
>> -	bpmp->tx.virt = gen_pool_dma_alloc(bpmp->tx.pool, 4096, &bpmp->tx.phys);
>> -	if (!bpmp->tx.virt) {
>> -		dev_err(&pdev->dev, "failed to allocate from TX pool\n");
>> -		return -ENOMEM;
>> -	}
>> -
>> -	bpmp->rx.pool = of_gen_pool_get(pdev->dev.of_node, "shmem", 1);
>> -	if (!bpmp->rx.pool) {
>> -		dev_err(&pdev->dev, "RX shmem pool not found\n");
>> -		err = -ENOMEM;
>> -		goto free_tx;
>> -	}
>> -
>> -	bpmp->rx.virt = gen_pool_dma_alloc(bpmp->rx.pool, 4096, &bpmp->rx.phys);
>> -	if (!bpmp->rx.virt) {
>> -		dev_err(&pdev->dev, "failed to allocate from RX pool\n");
>> -		err = -ENOMEM;
>> -		goto free_tx;
>> -	}
>> -
>>   	INIT_LIST_HEAD(&bpmp->mrqs);
>>   	spin_lock_init(&bpmp->lock);
>>   
>> @@ -801,81 +712,38 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>>   	size = BITS_TO_LONGS(bpmp->threaded.count) * sizeof(long);
>>   
>>   	bpmp->threaded.allocated = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
>> -	if (!bpmp->threaded.allocated) {
>> -		err = -ENOMEM;
>> -		goto free_rx;
>> -	}
>> +	if (!bpmp->threaded.allocated)
>> +		return -ENOMEM;
>>   
>>   	bpmp->threaded.busy = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
>> -	if (!bpmp->threaded.busy) {
>> -		err = -ENOMEM;
>> -		goto free_rx;
>> -	}
>> +	if (!bpmp->threaded.busy)
>> +		return -ENOMEM;
>>   
>>   	spin_lock_init(&bpmp->atomic_tx_lock);
>>   	bpmp->tx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->tx_channel),
>>   					GFP_KERNEL);
>> -	if (!bpmp->tx_channel) {
>> -		err = -ENOMEM;
>> -		goto free_rx;
>> -	}
>> +	if (!bpmp->tx_channel)
>> +		return -ENOMEM;
>>   
>>   	bpmp->rx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->rx_channel),
>>   	                                GFP_KERNEL);
>> -	if (!bpmp->rx_channel) {
>> -		err = -ENOMEM;
>> -		goto free_rx;
>> -	}
>> +	if (!bpmp->rx_channel)
>> +		return -ENOMEM;
>>   
>>   	bpmp->threaded_channels = devm_kcalloc(&pdev->dev, bpmp->threaded.count,
>>   					       sizeof(*bpmp->threaded_channels),
>>   					       GFP_KERNEL);
>> -	if (!bpmp->threaded_channels) {
>> -		err = -ENOMEM;
>> -		goto free_rx;
>> -	}
>> -
>> -	err = tegra_bpmp_channel_init(bpmp->tx_channel, bpmp,
>> -				      bpmp->soc->channels.cpu_tx.offset);
>> -	if (err < 0)
>> -		goto free_rx;
>> +	if (!bpmp->threaded_channels)
>> +		return -ENOMEM;
>>   
>> -	err = tegra_bpmp_channel_init(bpmp->rx_channel, bpmp,
>> -				      bpmp->soc->channels.cpu_rx.offset);
>> +	err = bpmp->soc->ops->init(bpmp);
>>   	if (err < 0)
>> -		goto cleanup_tx_channel;
>> -
>> -	for (i = 0; i < bpmp->threaded.count; i++) {
>> -		err = tegra_bpmp_channel_init(
>> -			&bpmp->threaded_channels[i], bpmp,
>> -			bpmp->soc->channels.thread.offset + i);
>> -		if (err < 0)
>> -			goto cleanup_threaded_channels;
>> -	}
>> -
>> -	/* mbox registration */
>> -	bpmp->mbox.client.dev = &pdev->dev;
>> -	bpmp->mbox.client.rx_callback = tegra_bpmp_handle_rx;
>> -	bpmp->mbox.client.tx_block = false;
>> -	bpmp->mbox.client.knows_txdone = false;
>> -
>> -	bpmp->mbox.channel = mbox_request_channel(&bpmp->mbox.client, 0);
>> -	if (IS_ERR(bpmp->mbox.channel)) {
>> -		err = PTR_ERR(bpmp->mbox.channel);
>> -		dev_err(&pdev->dev, "failed to get HSP mailbox: %d\n", err);
>> -		goto cleanup_threaded_channels;
>> -	}
>> -
>> -	/* reset message channels */
>> -	tegra_bpmp_channel_reset(bpmp->tx_channel);
>> -	tegra_bpmp_channel_reset(bpmp->rx_channel);
>> -	for (i = 0; i < bpmp->threaded.count; i++)
>> -		tegra_bpmp_channel_reset(&bpmp->threaded_channels[i]);
>> +		return err;
>>   
>>   	err = tegra_bpmp_request_mrq(bpmp, MRQ_PING,
>>   				     tegra_bpmp_mrq_handle_ping, bpmp);
>>   	if (err < 0)
>> -		goto free_mbox;
>> +		goto deinit;
>>   
>>   	err = tegra_bpmp_ping(bpmp);
>>   	if (err < 0) {
>> @@ -917,37 +785,17 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>>   
>>   free_mrq:
>>   	tegra_bpmp_free_mrq(bpmp, MRQ_PING, bpmp);
>> -free_mbox:
>> -	mbox_free_channel(bpmp->mbox.channel);
>> -cleanup_threaded_channels:
>> -	for (i = 0; i < bpmp->threaded.count; i++) {
>> -		if (bpmp->threaded_channels[i].bpmp)
>> -			tegra_bpmp_channel_cleanup(&bpmp->threaded_channels[i]);
>> -	}
>> +deinit:
>> +	bpmp->soc->ops->deinit(bpmp);
>>   
>> -	tegra_bpmp_channel_cleanup(bpmp->rx_channel);
>> -cleanup_tx_channel:
>> -	tegra_bpmp_channel_cleanup(bpmp->tx_channel);
>> -free_rx:
>> -	gen_pool_free(bpmp->rx.pool, (unsigned long)bpmp->rx.virt, 4096);
>> -free_tx:
>> -	gen_pool_free(bpmp->tx.pool, (unsigned long)bpmp->tx.virt, 4096);
>>   	return err;
>>   }
>>   
>>   static int __maybe_unused tegra_bpmp_resume(struct device *dev)
>>   {
>>   	struct tegra_bpmp *bpmp = dev_get_drvdata(dev);
>> -	unsigned int i;
>> -
>> -	/* reset message channels */
>> -	tegra_bpmp_channel_reset(bpmp->tx_channel);
>> -	tegra_bpmp_channel_reset(bpmp->rx_channel);
>>   
>> -	for (i = 0; i < bpmp->threaded.count; i++)
>> -		tegra_bpmp_channel_reset(&bpmp->threaded_channels[i]);
>> -
>> -	return 0;
>> +	return bpmp->soc->ops->resume(bpmp);
>>   }
>>   
>>   static SIMPLE_DEV_PM_OPS(tegra_bpmp_pm_ops, NULL, tegra_bpmp_resume);
> 
> Looks fine to me, but I do wonder if you need any checks to see if the
> ops handler is populated? For example, looking at the Tegra210 BPMP
> driver it does not populate resume, yet resume I believe would be called?
> 

Good point, I'll add checks for ops that clearly can be optional. In 
this case it would be deinit() and resume() ops.


> Cheers
> Jon
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP
  2019-01-24 13:02     ` Jon Hunter
@ 2019-01-24 14:49       ` Timo Alho
  0 siblings, 0 replies; 16+ messages in thread
From: Timo Alho @ 2019-01-24 14:49 UTC (permalink / raw)
  To: Jon Hunter, treding, sivaramn, robh; +Cc: linux-tegra, devicetree

Hi Jon,


On 24.1.2019 15.02, Jon Hunter wrote:
> 
> On 24/01/2019 12:19, Jon Hunter wrote:
>>
>> On 21/01/2019 12:28, Timo Alho wrote:
>>> The BPMP is a specific processor in Tegra210 chip, which is designed
>>> for boot process handling, assisting in entering deep low power states
>>> (suspend to ram), and offloading DRAM memory clock scaling on some
>>> platforms.
>>>
>>> Signed-off-by: Timo Alho <talho@nvidia.com>
>>> ---
>>>   .../bindings/firmware/nvidia,tegra210-bpmp.txt     | 36 ++++++++++++++++++++++
>>>   1 file changed, 36 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
>>> new file mode 100644
>>> index 0000000..63f439a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra210-bpmp.txt
>>> @@ -0,0 +1,36 @@
>>> +NVIDIA Tegra210 Boot and Power Management Processor (BPMP)
>>> +
>>> +The Boot and Power Management Processor (BPMP) is a co-processor found
>>> +in Tegra210 SoC. It is designed to handle the early stages of the boot
>>> +process as well as to assisting in entering deep low power state
>>> +(suspend to ram), and also offloading DRAM memory clock scaling on
>>> +some platforms. The binding document defines the resources that would
>>> +be used by the BPMP T210 firmware driver, which can create the
>>> +interprocessor communication (IPC) between the CPU and BPMP.
>>> +
>>> +Required properties:
>>> +- name : Should be bpmp
>>> +- compatible
>>> +    Array of strings
>>> +    One of:
>>> +    - "nvidia,tegra210-bpmp"
>>> +- reg: physical base address and length for HW synchornization primitives
>>> +       1) base address and length to Tegra 'atomics' hardware
>>> +       2) base address and length to Tegra 'semaphore' hardware
>>> +- interrupts: specifies the interrupt number for receiving messages ("rx")
>>> +              and for triggering messages ("tx")
>>> +
>>> +Optional properties:
>>> +- #clock-cells : Should be 1 for platforms where DRAM clock control is
>>> +                 offloaded to bpmp.
>>> +
>>> +Example:
>>> +
>>> +bpmp@70016000 {
>>> +	compatible = "nvidia,tegra210-bpmp";
>>> +	reg = <0x0 0x70016000 0x0 0x2000
>>> +	       0x0 0x60001000 0x0 0x1000>;
>>> +	interrupts = <0 6 IRQ_TYPE_NONE>,
>>> +		     <0 4 IRQ_TYPE_EDGE_RISING>;
>>> +	interrupt-names = "tx", "rx";
>>> +};
>>
>> Under required properties you have 'name' but it does not feature in the
>> above example. In fact, I see the same for the Tegra186 BPMP binding.
>>
>> Do we actually use/need this name property?
> 
> I am wondering if 'TYPE_NONE' for TX is correct here. I understand that
> Linux does not request this interrupt, but still shouldn't DT reflect it
> actual type?

Makes sense, I'll update. And while at it, I'll just change '0' to 
'GIC_SPI' to have it corrected as well.

> Cheers
> Jon
> 

BR,
Timo

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-01-24 14:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-21 12:28 [PATCH V2 0/4] add Tegra210 BPMP driver Timo Alho
2019-01-21 12:28 ` [PATCH V2 1/4] firmware: tegra: reword messaging terminology Timo Alho
2019-01-24 11:33   ` Jon Hunter
2019-01-24 12:25     ` Timo Alho
2019-01-21 12:28 ` [PATCH V2 2/4] firmware: tegra: refactor bpmp driver Timo Alho
2019-01-24 11:45   ` Jon Hunter
2019-01-24 14:26     ` Timo Alho
2019-01-21 12:28 ` [PATCH V2 3/4] firmware: tegra: add bpmp driver for Tegra210 Timo Alho
2019-01-24 12:16   ` Jon Hunter
2019-01-24 13:41     ` Timo Alho
2019-01-24 13:57       ` Jon Hunter
2019-01-21 12:28 ` [PATCH V2 4/4] dt-bindings: firmware: Add bindings for Tegra210 BPMP Timo Alho
2019-01-21 17:38   ` Rob Herring
2019-01-24 12:19   ` Jon Hunter
2019-01-24 13:02     ` Jon Hunter
2019-01-24 14:49       ` Timo Alho

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.