Re: [PATCH V2 1/5] firmware: tegra: add helper to check for supported MRQs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 22.10.2018 12.12, Thierry Reding wrote:
On Thu, Oct 18, 2018 at 08:55:40PM +0300, Timo Alho wrote:
Add a helper function to check that firmware is supporting a given MRQ
command.

Signed-off-by: Timo Alho <talho@xxxxxxxxxx>
---
  drivers/firmware/tegra/bpmp.c | 28 ++++++++++++++++++++++++++++
  include/soc/tegra/bpmp.h      |  7 +++++++
  2 files changed, 35 insertions(+)

diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index 41448ba..79859ab 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -470,6 +470,34 @@ void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data)
  }
  EXPORT_SYMBOL_GPL(tegra_bpmp_free_mrq);
+bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, unsigned int mrq)
+{
+	struct mrq_query_abi_request req = { .mrq = cpu_to_le32(mrq) };
+	struct mrq_query_abi_response resp;
+	struct tegra_bpmp_message msg = {
+		.mrq = MRQ_QUERY_ABI,
+		.tx = {
+			.data = &req,
+			.size = sizeof(req),
+		},
+		.rx = {
+			.data = &resp,
+			.size = sizeof(resp),
+		},
+	};
+	int ret;
+
+	ret = tegra_bpmp_transfer(bpmp, &msg);
+	if (ret != 0 || msg.rx.ret != 0) {

Can somebody remind me of the difference between these two? It looks
like we don't have a description of msg.rx.ret anywhere. It comes from
tegra_bpmp_channel.ib->code, which in turn is set directly by BPMP as > part of a transfer.

This is correct. 'ret' is nonzero if the "wire-level" communication between CPU and BPMP fails. In turn, 'msg.rx.ret' is the first word of the response payload from bpmp. A nonzero value indicates that the request made to BPMP was somehow ill-formatted. Possible reasons could be e.g.:
 * Use of non-existing MRQ code
 * Use of non-existing resource identifier (clk, reset, power-domain, ...)

What are the values that it can take?

Error codes are listed in very end of bpmp-abi.h


I think we could use some kerneldoc for struct tegra_bpmp_mb_data in
include/soc/tegra/bpmp.h.

+		/* something went wrong; assume not supported */
+		dev_warn(bpmp->dev, "tegra_bpmp_transfer failed\n");

This should probably contain an error code of some sort to help narrow
down why it fails. Also, please use a more human readable error message
for consistency within the driver. Something along these lines perhaps?

	dev_warn(bpmp->dev, "transfer failed: %d\n", ret);

Or perhaps more specifically:

	dev_warn(bpmp->dev, "ABI query MRQ failed: %d\n", ret);

Sure, I'll make the change.



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux