linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 [RESEND] 0/2] drivers/qcom: add Command DB support
@ 2018-04-06 15:13 Lina Iyer
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver Lina Iyer
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs Lina Iyer
  0 siblings, 2 replies; 10+ messages in thread
From: Lina Iyer @ 2018-04-06 15:13 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, sboyd, evgreen, dianders,
	Lina Iyer

Resending with minor rephrase of 'reg' property description.

Changes since v6:
	- Fix 'reg' property
	- Add reviewed-by tags

Changes since v5:
        - Remove indirection of command db start address
        - Rebase on top of 4.16

Changes since v4:
        - Address comments from Stephen and Bjorn

These patches add support for reading a shared memory database in the newer
QCOM SoCs called Command DB. With the new architecture on SDM845, shared
resources like clocks, regulators etc., have dynamic properties. These
properties may change based on external components, board configurations or
available feature set. A remote processor detects these parameters and fills up
the database with the resource and available state information. Platform
drivers that need these shared resources will need to query this database to
get the address and properties and vote for the state.

The information in the database is static. The database is read-only memory
location that is available for Linux. A pre-defined string is used as a key into
an entry in the database. Generally, platform drivers query the database only
at init to get the information they need.

[v1]: https://www.spinics.net/lists/linux-arm-msm/msg32462.html
[v2]: https://lkml.org/lkml/2018/2/8/588
[v3]: https://lkml.org/lkml/2018/2/16/842
[v4]: https://patchwork.kernel.org/patch/10242935/
[v5]: https://lkml.org/lkml/2018/3/14/787
[v6]: https://lkml.org/lkml/2018/4/5/451

Lina Iyer (2):
  drivers: qcom: add command DB driver
  dt-bindings: introduce Command DB for QCOM SoCs

 .../bindings/reserved-memory/qcom,cmd-db.txt       |  36 +++
 drivers/of/platform.c                              |   1 +
 drivers/soc/qcom/Kconfig                           |   9 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/cmd-db.c                          | 310 +++++++++++++++++++++
 include/soc/qcom/cmd-db.h                          |  45 +++
 6 files changed, 402 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/qcom,cmd-db.txt
 create mode 100644 drivers/soc/qcom/cmd-db.c
 create mode 100644 include/soc/qcom/cmd-db.h

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver
  2018-04-06 15:13 [PATCH v7 [RESEND] 0/2] drivers/qcom: add Command DB support Lina Iyer
@ 2018-04-06 15:13 ` Lina Iyer
  2018-04-06 23:15   ` Stephen Boyd
  2018-04-06 23:23   ` Stephen Boyd
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs Lina Iyer
  1 sibling, 2 replies; 10+ messages in thread
From: Lina Iyer @ 2018-04-06 15:13 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, sboyd, evgreen, dianders,
	Lina Iyer, Mahesh Sivasubramanian

From: Mahesh Sivasubramanian <msivasub@codeaurora.org>

Command DB is a simple database in the shared memory of QCOM SoCs, that
provides information regarding shared resources. Some shared resources
in the SoC have properties that are probed dynamically at boot by the
remote processor. The information pertaining to the SoC and the platform
are made available in the shared memory. Drivers can query this
information using predefined strings.

Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes in v6:
	- Remove indirection and use the actual command db memory in DT

Changes in v5:
	- Use strncmp
	- Add check in probe to ensure the location at dictionary address
	  is same as the one specified in reserved-memory
	- Whitespace fixes
	- Comments updated
---
 drivers/of/platform.c     |   1 +
 drivers/soc/qcom/Kconfig  |   9 ++
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/cmd-db.c | 310 ++++++++++++++++++++++++++++++++++++++++++++++
 include/soc/qcom/cmd-db.h |  45 +++++++
 5 files changed, 366 insertions(+)
 create mode 100644 drivers/soc/qcom/cmd-db.c
 create mode 100644 include/soc/qcom/cmd-db.h

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index c00d81dfac0b..26fb43847f4b 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -494,6 +494,7 @@ EXPORT_SYMBOL_GPL(of_platform_default_populate);
 #ifndef CONFIG_PPC
 static const struct of_device_id reserved_mem_matches[] = {
 	{ .compatible = "qcom,rmtfs-mem" },
+	{ .compatible = "qcom,cmd-db" },
 	{ .compatible = "ramoops" },
 	{}
 };
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e050eb83341d..b12868a2b92d 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -3,6 +3,15 @@
 #
 menu "Qualcomm SoC drivers"
 
+config QCOM_COMMAND_DB
+	bool "Qualcomm Command DB"
+	depends on (ARCH_QCOM && OF) || COMPILE_TEST
+	help
+	  Command DB queries shared memory by key string for shared system
+	  resources. Platform drivers that require to set state of a shared
+	  resource on a RPM-hardened platform must use this database to get
+	  SoC specific identifier and information for the shared resources.
+
 config QCOM_GLINK_SSR
 	tristate "Qualcomm Glink SSR driver"
 	depends on RPMSG
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index dcebf2814e6d..bbd1230fc441 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GLINK_SSR) +=	glink_ssr.o
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
new file mode 100644
index 000000000000..b5172049f608
--- /dev/null
+++ b/drivers/soc/qcom/cmd-db.c
@@ -0,0 +1,310 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include <soc/qcom/cmd-db.h>
+
+#define NUM_PRIORITY		2
+#define MAX_SLV_ID		8
+#define CMD_DB_MAGIC		0x0C0330DBUL
+#define SLAVE_ID_MASK		0x7
+#define SLAVE_ID_SHIFT		16
+
+#define ENTRY_HEADER(hdr)	((void *)cmd_db_header +	\
+				sizeof(*cmd_db_header) +	\
+				hdr->header_offset)
+
+#define RSC_OFFSET(hdr, ent)	((void *)cmd_db_header +	\
+				sizeof(*cmd_db_header) +	\
+				hdr.data_offset + ent.offset)
+
+/**
+ * struct entry_header: header for each entry in cmddb
+ *
+ * @id: resource's identifier
+ * @priority: unused
+ * @addr: the address of the resource
+ * @len: length of the data
+ * @offset: offset from :@data_offset, start of the data
+ */
+struct entry_header {
+	u64 id;
+	u32 priority[NUM_PRIORITY];
+	u32 addr;
+	u16 len;
+	u16 offset;
+};
+
+/**
+ * struct rsc_hdr: resource header information
+ *
+ * @slv_id: id for the resource
+ * @header_offset: entry's header at offset from the end of the cmd_db_header
+ * @data_offset: entry's data at offset from the end of the cmd_db_header
+ * @cnt: number of entries for HW type
+ * @version: MSB is major, LSB is minor
+ * @reserved: reserved for future use.
+ */
+struct rsc_hdr {
+	u16 slv_id;
+	u16 header_offset;
+	u16 data_offset;
+	u16 cnt;
+	u16 version;
+	u16 reserved[3];
+};
+
+/**
+ * struct cmd_db_header: The DB header information
+ *
+ * @version: The cmd db version
+ * @magic_number: constant expected in the database
+ * @header: array of resources
+ * @checksum: checksum for the header. Unused.
+ * @reserved: reserved memory
+ * @data: driver specific data
+ */
+struct cmd_db_header {
+	u32 version;
+	u32 magic_num;
+	struct rsc_hdr header[MAX_SLV_ID];
+	u32 checksum;
+	u32 reserved;
+	u8 data[];
+};
+
+/**
+ * DOC: Description of the Command DB database.
+ *
+ * At the start of the command DB memory is the cmd_db_header structure.
+ * The cmd_db_header holds the version, checksum, magic key as well as an
+ * array for header for each slave (depicted by the rsc_header). Each h/w
+ * based accelerator is a 'slave' (shared resource) and has slave id indicating
+ * the type of accelerator. The rsc_header is the header for such individual
+ * slaves of a given type. The entries for each of these slaves begin at the
+ * rsc_hdr.header_offset. In addition each slave could have auxiliary data
+ * that may be needed by the driver. The data for the slave starts at the
+ * entry_header.offset to the location pointed to by the rsc_hdr.data_offset.
+ *
+ * Drivers have a stringified key to a slave/resource. They can query the slave
+ * information and get the slave id and the auxiliary data and the length of the
+ * data. Using this information, they can format the request to be sent to the
+ * h/w accelerator and request a resource state.
+ */
+
+static struct cmd_db_header *cmd_db_header;
+
+/**
+ * cmd_db_ready - Indicates if command DB is available
+ *
+ * Return: 0 on success, errno otherwise
+ */
+int cmd_db_ready(void)
+{
+	if (cmd_db_header == NULL)
+		return -EPROBE_DEFER;
+	else if (cmd_db_header->magic_num != CMD_DB_MAGIC)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL(cmd_db_ready);
+
+static u64 cmd_db_get_u64_id(const char *id)
+{
+	u64 rsc_id = 0;
+	u8 *ch = (u8 *)&rsc_id;
+
+	strncpy(ch, id, min(strlen(id), sizeof(rsc_id)));
+
+	return rsc_id;
+}
+
+static int cmd_db_get_header(u64 query, struct entry_header *eh,
+			     struct rsc_hdr *rh)
+{
+	struct rsc_hdr *rsc_hdr;
+	struct entry_header *ent;
+	int ret, i, j;
+
+	ret = cmd_db_ready();
+	if (ret)
+		return ret;
+
+	if (!eh || !rh)
+		return -EINVAL;
+
+	for (i = 0; i < MAX_SLV_ID; i++) {
+		rsc_hdr = &cmd_db_header->header[i];
+		if (!rsc_hdr->slv_id)
+			break;
+
+		ent = ENTRY_HEADER(rsc_hdr);
+		for (j = 0; j < rsc_hdr->cnt; j++, ent++) {
+			if (ent->id == query)
+				break;
+		}
+
+		if (j < rsc_hdr->cnt) {
+			memcpy(eh, ent, sizeof(*ent));
+			memcpy(rh, rsc_hdr, sizeof(*rh));
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int cmd_db_get_header_by_rsc_id(const char *id,
+				       struct entry_header *ent_hdr,
+				       struct rsc_hdr *rsc_hdr)
+{
+	u64 rsc_id = cmd_db_get_u64_id(id);
+
+	return cmd_db_get_header(rsc_id, ent_hdr, rsc_hdr);
+}
+
+/**
+ * cmd_db_read_addr() - Query command db for resource id address.
+ *
+ * @id: resource id to query for address
+ *
+ * Return: resource address on success, 0 on error
+ *
+ * This is used to retrieve resource address based on resource
+ * id.
+ */
+u32 cmd_db_read_addr(const char *id)
+{
+	int ret;
+	struct entry_header ent;
+	struct rsc_hdr rsc_hdr;
+
+	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
+
+	return ret < 0 ? 0 : ent.addr;
+}
+EXPORT_SYMBOL(cmd_db_read_addr);
+
+/**
+ * cmd_db_read_aux_data() - Query command db for aux data.
+ *
+ *  @id: Resource to retrieve AUX Data on.
+ *  @data: Data buffer to copy returned aux data to. Returns size on NULL
+ *  @len: Caller provides size of data buffer passed in.
+ *
+ *  Return: size of data on success, errno otherwise
+ */
+int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
+{
+	int ret;
+	struct entry_header ent;
+	struct rsc_hdr rsc_hdr;
+
+	if (!data)
+		return -EINVAL;
+
+	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
+	if (ret)
+		return ret;
+
+	if (len < ent.len)
+		return -EINVAL;
+
+	len = min_t(u16, ent.len, len);
+	memcpy(data, RSC_OFFSET(rsc_hdr, ent), len);
+
+	return len;
+}
+EXPORT_SYMBOL(cmd_db_read_aux_data);
+
+/**
+ * cmd_db_read_aux_data_len - Get the length of the auxiliary data stored in DB.
+ *
+ * @id: Resource to retrieve AUX Data.
+ *
+ * Return: size on success, 0 on error
+ */
+size_t cmd_db_read_aux_data_len(const char *id)
+{
+	int ret;
+	struct entry_header ent;
+	struct rsc_hdr rsc_hdr;
+
+	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
+
+	return ret < 0 ? 0 : ent.len;
+}
+EXPORT_SYMBOL(cmd_db_read_aux_data_len);
+
+/**
+ * cmd_db_read_slave_id - Get the slave ID for a given resource address
+ *
+ * @id: Resource id to query the DB for version
+ *
+ * Return: cmd_db_hw_type enum on success, CMD_DB_HW_INVALID on error
+ */
+enum cmd_db_hw_type cmd_db_read_slave_id(const char *id)
+{
+	int ret;
+	struct entry_header ent;
+	struct rsc_hdr rsc_hdr;
+
+	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
+
+	return ret < 0 ? CMD_DB_HW_INVALID :
+		       (ent.addr >> SLAVE_ID_SHIFT) & SLAVE_ID_MASK;
+}
+EXPORT_SYMBOL(cmd_db_read_slave_id);
+
+static int cmd_db_dev_probe(struct platform_device *pdev)
+{
+	struct reserved_mem *rmem;
+	int ret = 0;
+
+	rmem = of_reserved_mem_lookup(pdev->dev.of_node);
+	if (!rmem) {
+		dev_err(&pdev->dev, "failed to acquire memory region\n");
+		return -EINVAL;
+	}
+
+	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WB);
+	if (IS_ERR_OR_NULL(cmd_db_header)) {
+		ret = PTR_ERR(cmd_db_header);
+		cmd_db_header = NULL;
+		return ret;
+	}
+
+	if (cmd_db_header->magic_num != CMD_DB_MAGIC) {
+		dev_err(&pdev->dev, "Invalid Command DB Magic\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id cmd_db_match_table[] = {
+	{ .compatible = "qcom,cmd-db" },
+	{ },
+};
+
+static struct platform_driver cmd_db_dev_driver = {
+	.probe  = cmd_db_dev_probe,
+	.driver = {
+		   .name = "cmd-db",
+		   .of_match_table = cmd_db_match_table,
+	},
+};
+
+static int __init cmd_db_device_init(void)
+{
+	return platform_driver_register(&cmd_db_dev_driver);
+}
+arch_initcall(cmd_db_device_init);
diff --git a/include/soc/qcom/cmd-db.h b/include/soc/qcom/cmd-db.h
new file mode 100644
index 000000000000..578180cbc134
--- /dev/null
+++ b/include/soc/qcom/cmd-db.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */
+
+#ifndef __QCOM_COMMAND_DB_H__
+#define __QCOM_COMMAND_DB_H__
+
+
+enum cmd_db_hw_type {
+	CMD_DB_HW_INVALID = 0,
+	CMD_DB_HW_MIN     = 3,
+	CMD_DB_HW_ARC     = CMD_DB_HW_MIN,
+	CMD_DB_HW_VRM     = 4,
+	CMD_DB_HW_BCM     = 5,
+	CMD_DB_HW_MAX     = CMD_DB_HW_BCM,
+	CMD_DB_HW_ALL     = 0xff,
+};
+
+#if IS_ENABLED(CONFIG_QCOM_COMMAND_DB)
+u32 cmd_db_read_addr(const char *resource_id);
+
+int cmd_db_read_aux_data(const char *resource_id, u8 *data, size_t len);
+
+size_t cmd_db_read_aux_data_len(const char *resource_id);
+
+enum cmd_db_hw_type cmd_db_read_slave_id(const char *resource_id);
+
+int cmd_db_ready(void);
+#else
+static inline u32 cmd_db_read_addr(const char *resource_id)
+{ return 0; }
+
+static inline int cmd_db_read_aux_data(const char *resource_id, u8 *data,
+				       size_t len)
+{ return -ENODEV; }
+
+static inline size_t cmd_db_read_aux_data_len(const char *resource_id)
+{ return -ENODEV; }
+
+static inline enum cmd_db_hw_type cmd_db_read_slave_id(const char *resource_id)
+{ return -ENODEV; }
+
+static inline int cmd_db_ready(void)
+{ return -ENODEV; }
+#endif /* CONFIG_QCOM_COMMAND_DB */
+#endif /* __QCOM_COMMAND_DB_H__ */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs
  2018-04-06 15:13 [PATCH v7 [RESEND] 0/2] drivers/qcom: add Command DB support Lina Iyer
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver Lina Iyer
@ 2018-04-06 15:13 ` Lina Iyer
  2018-04-06 23:16   ` Stephen Boyd
  1 sibling, 1 reply; 10+ messages in thread
From: Lina Iyer @ 2018-04-06 15:13 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, sboyd, evgreen, dianders,
	Lina Iyer, devicetree, Mahesh Sivasubramanian

From: Mahesh Sivasubramanian <msivasub@codeaurora.org>

Command DB provides information on shared resources like clocks,
regulators etc., probed at boot by the remote subsytem and made
available in shared memory.

Cc: devicetree@vger.kernel.org
Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes in v7:
	- Fix description of 'reg' property

Changes in v6:
	- Move to bindings/reserved-memory
	- Remove indirection address and use only the actual reserved
	  memory

Changes in v4:
	- Fix unwanted capitalization
	- Add reg property
---
 .../bindings/reserved-memory/qcom,cmd-db.txt       | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/qcom,cmd-db.txt

diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,cmd-db.txt b/Documentation/devicetree/bindings/reserved-memory/qcom,cmd-db.txt
new file mode 100644
index 000000000000..1390cd3a7f0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/qcom,cmd-db.txt
@@ -0,0 +1,36 @@
+Command DB
+---------
+
+Command DB is a database that provides a mapping between resource key and the
+resource address for a system resource managed by a remote processor. The data
+is stored in a shared memory region and is loaded by the remote processor.
+
+Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for
+controlling shared resources. Depending on the board configuration the shared
+resource properties may change. These properties are dynamically probed by the
+remote processor and made available in the shared memory.
+
+The bindings for Command DB is specified in the reserved-memory section in
+devicetree. The devicetree representation of the command DB driver should be:
+
+Properties:
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: Should be "qcom,cmd-db"
+
+- reg:
+	Usage: required
+	Value type: <prop encoded array>
+	Definition: The register address that points to the actual location of
+		    the Command DB in memory.
+
+Example:
+
+	reserved-memory {
+		[...]
+		qcom,cmd-db@85fe0000 {
+			reg = <0x0 0x85fe0000 0x0 0x20000>;
+			compatible = "qcom,cmd-db";
+		};
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver Lina Iyer
@ 2018-04-06 23:15   ` Stephen Boyd
  2018-04-06 23:23   ` Stephen Boyd
  1 sibling, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-04-06 23:15 UTC (permalink / raw)
  To: Lina Iyer, andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, evgreen, dianders,
	Lina Iyer, Mahesh Sivasubramanian

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> 
> Command DB is a simple database in the shared memory of QCOM SoCs, that
> provides information regarding shared resources. Some shared resources
> in the SoC have properties that are probed dynamically at boot by the
> remote processor. The information pertaining to the SoC and the platform
> are made available in the shared memory. Drivers can query this
> information using predefined strings.
> 
> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

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

* Re: [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs Lina Iyer
@ 2018-04-06 23:16   ` Stephen Boyd
  2018-04-09 20:42     ` Evan Green
  2018-04-10 14:10     ` Rob Herring
  0 siblings, 2 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-04-06 23:16 UTC (permalink / raw)
  To: Lina Iyer, andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, evgreen, dianders,
	Lina Iyer, devicetree, Mahesh Sivasubramanian

Quoting Lina Iyer (2018-04-06 08:13:56)
> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> 
> Command DB provides information on shared resources like clocks,
> regulators etc., probed at boot by the remote subsytem and made
> available in shared memory.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

> +       reserved-memory {
> +               [...]
> +               qcom,cmd-db@85fe0000 {

Nitpick: This may want to be called 'memory@85fe0000' because we prefer
generic node names.

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

* Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver
  2018-04-06 15:13 ` [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver Lina Iyer
  2018-04-06 23:15   ` Stephen Boyd
@ 2018-04-06 23:23   ` Stephen Boyd
  2018-04-10 17:46     ` Lina Iyer
  1 sibling, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2018-04-06 23:23 UTC (permalink / raw)
  To: Lina Iyer, andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, evgreen, dianders,
	Lina Iyer, Mahesh Sivasubramanian

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> 
> Command DB is a simple database in the shared memory of QCOM SoCs, that
> provides information regarding shared resources. Some shared resources
> in the SoC have properties that are probed dynamically at boot by the
> remote processor. The information pertaining to the SoC and the platform
> are made available in the shared memory. Drivers can query this
> information using predefined strings.
> 
> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

I have this patch on top to fix the endian stuff. Care to test it out
and see if it still works?

From: Stephen Boyd <swboyd@chromium.org>
Subject: soc: qcom: cmd-db: Make endian-agnostic
    
This driver deals with memory that is stored in little-endian format.
Update the structures with the proper little-endian types and then
do the proper conversions when reading the fields. Note that we compare
the ids with a memcmp() because we already pad out the string 'id' field
to exactly 8 bytes with the strncpy() onto the stack.

Signed-off-by: Stephen Boyd <swboyd@chromium.org>

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index b5172049f608..a56dc9edab82 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -13,18 +13,10 @@
 
 #define NUM_PRIORITY		2
 #define MAX_SLV_ID		8
-#define CMD_DB_MAGIC		0x0C0330DBUL
+static const char CMD_DB_MAGIC[] = { 0xdb, 0x33, 0x03, 0x0c };
 #define SLAVE_ID_MASK		0x7
 #define SLAVE_ID_SHIFT		16
 
-#define ENTRY_HEADER(hdr)	((void *)cmd_db_header +	\
-				sizeof(*cmd_db_header) +	\
-				hdr->header_offset)
-
-#define RSC_OFFSET(hdr, ent)	((void *)cmd_db_header +	\
-				sizeof(*cmd_db_header) +	\
-				hdr.data_offset + ent.offset)
-
 /**
  * struct entry_header: header for each entry in cmddb
  *
@@ -35,11 +27,11 @@
  * @offset: offset from :@data_offset, start of the data
  */
 struct entry_header {
-	u64 id;
-	u32 priority[NUM_PRIORITY];
-	u32 addr;
-	u16 len;
-	u16 offset;
+	u8 id[8];
+	__le32 priority[NUM_PRIORITY];
+	__le32 addr;
+	__le16 len;
+	__le16 offset;
 };
 
 /**
@@ -53,12 +45,12 @@ struct entry_header {
  * @reserved: reserved for future use.
  */
 struct rsc_hdr {
-	u16 slv_id;
-	u16 header_offset;
-	u16 data_offset;
-	u16 cnt;
-	u16 version;
-	u16 reserved[3];
+	__le16 slv_id;
+	__le16 header_offset;
+	__le16 data_offset;
+	__le16 cnt;
+	__le16 version;
+	__le16 reserved[3];
 };
 
 /**
@@ -72,11 +64,11 @@ struct rsc_hdr {
  * @data: driver specific data
  */
 struct cmd_db_header {
-	u32 version;
-	u32 magic_num;
+	__le32 version;
+	__le32 magic_num;
 	struct rsc_hdr header[MAX_SLV_ID];
-	u32 checksum;
-	u32 reserved;
+	__le32 checksum;
+	__le32 reserved;
 	u8 data[];
 };
 
@@ -101,6 +93,22 @@ struct cmd_db_header {
 
 static struct cmd_db_header *cmd_db_header;
 
+static inline void *rsc_to_entry_header(struct rsc_hdr *hdr)
+{
+	u16 offset = le16_to_cpu(hdr->header_offset);
+
+	return cmd_db_header->data + offset;
+}
+
+static inline void *
+rsc_offset(struct rsc_hdr *hdr, struct entry_header *ent)
+{
+	u16 offset = le16_to_cpu(hdr->header_offset);
+	u16 loffset = le16_to_cpu(ent->offset);
+
+	return cmd_db_header->data + offset +  loffset;
+}
+
 /**
  * cmd_db_ready - Indicates if command DB is available
  *
@@ -110,29 +118,20 @@ int cmd_db_ready(void)
 {
 	if (cmd_db_header == NULL)
 		return -EPROBE_DEFER;
-	else if (cmd_db_header->magic_num != CMD_DB_MAGIC)
+	else if (memcmp(&cmd_db_header->magic_num, CMD_DB_MAGIC, sizeof(CMD_DB_MAGIC)))
 		return -EINVAL;
 
 	return 0;
 }
 EXPORT_SYMBOL(cmd_db_ready);
 
-static u64 cmd_db_get_u64_id(const char *id)
-{
-	u64 rsc_id = 0;
-	u8 *ch = (u8 *)&rsc_id;
-
-	strncpy(ch, id, min(strlen(id), sizeof(rsc_id)));
-
-	return rsc_id;
-}
-
-static int cmd_db_get_header(u64 query, struct entry_header *eh,
+static int cmd_db_get_header(const char *id, struct entry_header *eh,
 			     struct rsc_hdr *rh)
 {
 	struct rsc_hdr *rsc_hdr;
 	struct entry_header *ent;
 	int ret, i, j;
+	char query[8];
 
 	ret = cmd_db_ready();
 	if (ret)
@@ -141,18 +140,21 @@ static int cmd_db_get_header(u64 query, struct entry_header *eh,
 	if (!eh || !rh)
 		return -EINVAL;
 
+	/* Pad out query string to same length as in DB */
+	strncpy(query, id, sizeof(query));
+
 	for (i = 0; i < MAX_SLV_ID; i++) {
 		rsc_hdr = &cmd_db_header->header[i];
 		if (!rsc_hdr->slv_id)
 			break;
 
-		ent = ENTRY_HEADER(rsc_hdr);
-		for (j = 0; j < rsc_hdr->cnt; j++, ent++) {
-			if (ent->id == query)
+		ent = rsc_to_entry_header(rsc_hdr);
+		for (j = 0; j < le16_to_cpu(rsc_hdr->cnt); j++, ent++) {
+			if (memcmp(ent->id, query, sizeof(ent->id)))
 				break;
 		}
 
-		if (j < rsc_hdr->cnt) {
+		if (j < le16_to_cpu(rsc_hdr->cnt)) {
 			memcpy(eh, ent, sizeof(*ent));
 			memcpy(rh, rsc_hdr, sizeof(*rh));
 			return 0;
@@ -166,9 +168,7 @@ static int cmd_db_get_header_by_rsc_id(const char *id,
 				       struct entry_header *ent_hdr,
 				       struct rsc_hdr *rsc_hdr)
 {
-	u64 rsc_id = cmd_db_get_u64_id(id);
-
-	return cmd_db_get_header(rsc_id, ent_hdr, rsc_hdr);
+	return cmd_db_get_header(id, ent_hdr, rsc_hdr);
 }
 
 /**
@@ -189,7 +189,7 @@ u32 cmd_db_read_addr(const char *id)
 
 	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
 
-	return ret < 0 ? 0 : ent.addr;
+	return ret < 0 ? 0 : le32_to_cpu(ent.addr);
 }
 EXPORT_SYMBOL(cmd_db_read_addr);
 
@@ -207,6 +207,7 @@ int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
 	int ret;
 	struct entry_header ent;
 	struct rsc_hdr rsc_hdr;
+	u16 ent_len;
 
 	if (!data)
 		return -EINVAL;
@@ -215,11 +216,12 @@ int cmd_db_read_aux_data(const char *id, u8 *data, size_t len)
 	if (ret)
 		return ret;
 
-	if (len < ent.len)
+	ent_len = le16_to_cpu(ent.len);
+	if (len < ent_len)
 		return -EINVAL;
 
-	len = min_t(u16, ent.len, len);
-	memcpy(data, RSC_OFFSET(rsc_hdr, ent), len);
+	len = min_t(u16, ent_len, len);
+	memcpy(data, rsc_offset(&rsc_hdr, &ent), len);
 
 	return len;
 }
@@ -240,7 +242,7 @@ size_t cmd_db_read_aux_data_len(const char *id)
 
 	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
 
-	return ret < 0 ? 0 : ent.len;
+	return ret < 0 ? 0 : le16_to_cpu(ent.len);
 }
 EXPORT_SYMBOL(cmd_db_read_aux_data_len);
 
@@ -256,11 +258,14 @@ enum cmd_db_hw_type cmd_db_read_slave_id(const char *id)
 	int ret;
 	struct entry_header ent;
 	struct rsc_hdr rsc_hdr;
+	u32 addr;
 
 	ret = cmd_db_get_header_by_rsc_id(id, &ent, &rsc_hdr);
+	if (ret < 0)
+		return CMD_DB_HW_INVALID;
 
-	return ret < 0 ? CMD_DB_HW_INVALID :
-		       (ent.addr >> SLAVE_ID_SHIFT) & SLAVE_ID_MASK;
+	addr = le32_to_cpu(ent.addr);
+	return (addr >> SLAVE_ID_SHIFT) & SLAVE_ID_MASK;
 }
 EXPORT_SYMBOL(cmd_db_read_slave_id);
 
@@ -282,7 +287,7 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (cmd_db_header->magic_num != CMD_DB_MAGIC) {
+	if (memcmp(&cmd_db_header->magic_num, CMD_DB_MAGIC, sizeof(CMD_DB_MAGIC))) {
 		dev_err(&pdev->dev, "Invalid Command DB Magic\n");
 		return -EINVAL;
 	}

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

* Re: [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs
  2018-04-06 23:16   ` Stephen Boyd
@ 2018-04-09 20:42     ` Evan Green
  2018-04-10 14:10     ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Evan Green @ 2018-04-09 20:42 UTC (permalink / raw)
  To: swboyd
  Cc: Lina Iyer, Andy Gross, David Brown, linux-arm-msm, linux-soc,
	Rajendra Nayak, Bjorn Andersson, linux-kernel, Doug Anderson,
	devicetree, msivasub

On Fri, Apr 6, 2018 at 4:16 PM Stephen Boyd <swboyd@chromium.org> wrote:

> Quoting Lina Iyer (2018-04-06 08:13:56)
> > From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> >
> > Command DB provides information on shared resources like clocks,
> > regulators etc., probed at boot by the remote subsytem and made
> > available in shared memory.
> >
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> > Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >

> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

> > +       reserved-memory {
> > +               [...]
> > +               qcom,cmd-db@85fe0000 {

> Nitpick: This may want to be called 'memory@85fe0000' because we prefer
> generic node names.

Another nit: the cmd-db region seems to need "no-map" to make the example
actually work.

Reviewed-by: Evan Green <evgreen@chromium.org>

-Evan

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

* Re: [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs
  2018-04-06 23:16   ` Stephen Boyd
  2018-04-09 20:42     ` Evan Green
@ 2018-04-10 14:10     ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2018-04-10 14:10 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Lina Iyer, andy.gross, david.brown, linux-arm-msm, linux-soc,
	rnayak, bjorn.andersson, linux-kernel, evgreen, dianders,
	devicetree, Mahesh Sivasubramanian

On Fri, Apr 06, 2018 at 04:16:42PM -0700, Stephen Boyd wrote:
> Quoting Lina Iyer (2018-04-06 08:13:56)
> > From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> > 
> > Command DB provides information on shared resources like clocks,
> > regulators etc., probed at boot by the remote subsytem and made
> > available in shared memory.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> > Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> 
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> 
> > +       reserved-memory {
> > +               [...]
> > +               qcom,cmd-db@85fe0000 {
> 
> Nitpick: This may want to be called 'memory@85fe0000' because we prefer
> generic node names.

Well, "memory" is for nodes of memory that aren't reserved. So 
"reserved-memory@..." would be better.

Otherwise,

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

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

* Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver
  2018-04-06 23:23   ` Stephen Boyd
@ 2018-04-10 17:46     ` Lina Iyer
  2018-04-12 20:55       ` Stephen Boyd
  0 siblings, 1 reply; 10+ messages in thread
From: Lina Iyer @ 2018-04-10 17:46 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: andy.gross, david.brown, linux-arm-msm, linux-soc, rnayak,
	bjorn.andersson, linux-kernel, evgreen, dianders,
	Mahesh Sivasubramanian

On Fri, Apr 06 2018 at 17:23 -0600, Stephen Boyd wrote:
>Quoting Lina Iyer (2018-04-06 08:13:55)
>> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
>>
>> Command DB is a simple database in the shared memory of QCOM SoCs, that
>> provides information regarding shared resources. Some shared resources
>> in the SoC have properties that are probed dynamically at boot by the
>> remote processor. The information pertaining to the SoC and the platform
>> are made available in the shared memory. Drivers can query this
>> information using predefined strings.
>>
>> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> ---
>
>I have this patch on top to fix the endian stuff. Care to test it out
>and see if it still works?
>
>From: Stephen Boyd <swboyd@chromium.org>
>Subject: soc: qcom: cmd-db: Make endian-agnostic
>
>This driver deals with memory that is stored in little-endian format.
>Update the structures with the proper little-endian types and then
>do the proper conversions when reading the fields. Note that we compare
>the ids with a memcmp() because we already pad out the string 'id' field
>to exactly 8 bytes with the strncpy() onto the stack.
>
>Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>
>diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
>index b5172049f608..a56dc9edab82 100644
>--- a/drivers/soc/qcom/cmd-db.c
>+++ b/drivers/soc/qcom/cmd-db.c
>@@ -13,18 +13,10 @@
>
> #define NUM_PRIORITY		2
> #define MAX_SLV_ID		8
>-#define CMD_DB_MAGIC		0x0C0330DBUL
>+static const char CMD_DB_MAGIC[] = { 0xdb, 0x33, 0x03, 0x0c };
This has to be { 0xdb, 0x30, 0x03, 0x0c }

Otherwise it works.

Thanks,
Lina

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

* Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver
  2018-04-10 17:46     ` Lina Iyer
@ 2018-04-12 20:55       ` Stephen Boyd
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-04-12 20:55 UTC (permalink / raw)
  To: Lina Iyer
  Cc: andy.gross, david.brown, linux-arm-msm, linux-soc, rnayak,
	bjorn.andersson, linux-kernel, evgreen, dianders,
	Mahesh Sivasubramanian

Quoting Lina Iyer (2018-04-10 10:46:29)
> On Fri, Apr 06 2018 at 17:23 -0600, Stephen Boyd wrote:
> >Quoting Lina Iyer (2018-04-06 08:13:55)
> >> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> >>
> >> Command DB is a simple database in the shared memory of QCOM SoCs, that
> >> provides information regarding shared resources. Some shared resources
> >> in the SoC have properties that are probed dynamically at boot by the
> >> remote processor. The information pertaining to the SoC and the platform
> >> are made available in the shared memory. Drivers can query this
> >> information using predefined strings.
> >>
> >> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
> >> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> >> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >> ---
> >
> >I have this patch on top to fix the endian stuff. Care to test it out
> >and see if it still works?
> >
> >From: Stephen Boyd <swboyd@chromium.org>
> >Subject: soc: qcom: cmd-db: Make endian-agnostic
> >
> >This driver deals with memory that is stored in little-endian format.
> >Update the structures with the proper little-endian types and then
> >do the proper conversions when reading the fields. Note that we compare
> >the ids with a memcmp() because we already pad out the string 'id' field
> >to exactly 8 bytes with the strncpy() onto the stack.
> >
> >Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >
> >diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> >index b5172049f608..a56dc9edab82 100644
> >--- a/drivers/soc/qcom/cmd-db.c
> >+++ b/drivers/soc/qcom/cmd-db.c
> >@@ -13,18 +13,10 @@
> >
> > #define NUM_PRIORITY          2
> > #define MAX_SLV_ID            8
> >-#define CMD_DB_MAGIC          0x0C0330DBUL
> >+static const char CMD_DB_MAGIC[] = { 0xdb, 0x33, 0x03, 0x0c };
> This has to be { 0xdb, 0x30, 0x03, 0x0c }
> 
> Otherwise it works.
> 

Perfect! Thanks for catching that. I will resend with the typo fix.

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

end of thread, other threads:[~2018-04-12 20:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-06 15:13 [PATCH v7 [RESEND] 0/2] drivers/qcom: add Command DB support Lina Iyer
2018-04-06 15:13 ` [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver Lina Iyer
2018-04-06 23:15   ` Stephen Boyd
2018-04-06 23:23   ` Stephen Boyd
2018-04-10 17:46     ` Lina Iyer
2018-04-12 20:55       ` Stephen Boyd
2018-04-06 15:13 ` [PATCH v7 [RESEND] 2/2] dt-bindings: introduce Command DB for QCOM SoCs Lina Iyer
2018-04-06 23:16   ` Stephen Boyd
2018-04-09 20:42     ` Evan Green
2018-04-10 14:10     ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).