On 01/18, Lina Iyer wrote:
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index b81374bb6713..f21c5d53e721 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -97,4 +97,11 @@ config QCOM_WCNSS_CTRL
> Client driver for the WCNSS_CTRL SMD channel, used to download nv
> firmware to a newly booted WCNSS chip.
>
> +config QCOM_COMMAND_DB
> + bool "Qualcomm Command DB"
> + depends on ARCH_QCOM
Add || COMPILE_TEST?
> + help
> + Command DB queries shared memory by key string for shared system
> + resources
> +
> endmenu
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 40c56f67e94a..7b64135b22eb 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
> obj-$(CONFIG_QCOM_SMP2P) += smp2p.o
> obj-$(CONFIG_QCOM_SMSM) += smsm.o
> obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
> +obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
> diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> new file mode 100644
> index 000000000000..eb10ea8cf963
> --- /dev/null
> +++ b/drivers/soc/qcom/cmd-db.c
> @@ -0,0 +1,306 @@
> +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
It's not a module.
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#include <soc/qcom/cmd-db.h>
> +
> +#define RESOURCE_ID_LEN 8
> +#define NUM_PRIORITY 2
> +#define MAX_SLV_ID 8
> +#define CMD_DB_MAGIC 0x0C0330DBUL
> +#define SLAVE_ID_MASK 0x7
> +#define SLAVE_ID_SHIFT 16
> +
> +/**
> + * entry_header: header for each entry in cmddb
> + *
> + * @res_id: resource's identifier
> + * @priority: unused
> + * @addr: the address of the resource
> + * @len: length of the data
> + * @offset: offset at which dats starts
> + */
> +struct entry_header {
> + uint64_t res_id;
> + u32 priority[NUM_PRIORITY];
> + u32 addr;
> + u16 len;
> + u16 offset;
Are these little endian? Needs to be __le16 and __le32 then.
> +};
> +
> +/**
> + * rsc_hdr: resource header information
> + *
> + * @slv_id: id for the resource
> + * @header_offset: Entry header offset from data
> + * @data_offset: Entry offset for datda location
> + * @cnt: number of enteries for HW type
> + * @version: MSB is major, LSB is minor
> + */
> +struct rsc_hdr {
> + u16 slv_id;
> + u16 header_offset;
> + u16 data_offset;
> + u16 cnt;
> + u16 version;
> + u16 reserved[3];
Same comment.
> +};
> +
> +/**
> + * cmd_db_header: The DB header information
> + *
> + * @version: The cmd db version
> + * @magic_number: constant expected in the database
> + * @header: array of resources
> + */
> +struct cmd_db_header {
> + u32 version;
> + u32 magic_num;
> + struct rsc_hdr header[MAX_SLV_ID];
> + u32 check_sum;
> + u32 reserved;
> + u8 data[];
I hope struct randomization doesn't strike back. Same comment
about endianness.
> +};
> +
> +/**
> + * cmd_db_entry: Inforamtion for each line in the cmd db
> + *
> + * @resource_id: unique identifier for each entry
> + * @addr: slave id + offset address
> + * @priority: Bitmask for DRV ids
> + * @len: aux data len
> + * @data: data assocaited with the resource
> + */
> +struct cmd_db_entry {
> + const char resource_id[RESOURCE_ID_LEN + 1];
> + const u32 addr;
> + const u32 priority[NUM_PRIORITY];
> + u32 len;
But this isn't const?
> + u16 version;
Endian.
> + u8 data[];
> +};
> +
> +static void __iomem *start_addr;
> +static struct cmd_db_header *cmd_db_header;
> +static int cmd_db_status = -EPROBE_DEFER;
Hopefully we never have more than one commmand db? Do consumers
"just know" to use this code? I haven't looked at the DT binding,
but perhaps consumers need to point to command db via DT phandles
so we can identify the consumers. That may make probe defer
easier too.
> +
> +static u64 cmd_db_get_u64_id(const char *id)
> +{
> + uint64_t rsc_id = 0;
> + uint8_t *ch = (uint8_t *)&rsc_id;
Use u* instead of uint*_t please.
> + int i;
> +
> + for (i = 0; ((i < sizeof(rsc_id)) && id[i]); i++)
Drop useless parenthesis.
> + ch[i] = id[i];
Is this a strcpy?
> +
> + return rsc_id;
> +}
> +
> +static int cmd_db_get_header(u64 query, struct entry_header *eh,
> + struct rsc_hdr *rh, bool use_addr)
> +{
> + struct rsc_hdr *rsc_hdr;
> + int i, j;
> +
> + if (!cmd_db_header)
> + return -EPROBE_DEFER;
> +
> + if (!eh || !rh)
> + return -EINVAL;
> +
> + rsc_hdr = &cmd_db_header->header[0];
> +
> + for (i = 0; i < MAX_SLV_ID ; i++, rsc_hdr++) {
> + struct entry_header *ent;
> +
> + if (!rsc_hdr->slv_id)
> + break;
> +
> + ent = (struct entry_header *)(start_addr
Useless cast?
> + + sizeof(*cmd_db_header)
> + + rsc_hdr->header_offset);
> +
> + for (j = 0; j < rsc_hdr->cnt; j++, ent++) {
> + if (use_addr) {
> + if (ent->addr == (u32)(query))
> + break;
> + } else if (ent->res_id == query)
> + break;
> + }
> +
> + if (j < rsc_hdr->cnt) {
> + memcpy(eh, ent, sizeof(*ent));
> + memcpy(rh, &cmd_db_header->header[i], sizeof(*rh));
> + return 0;
> + }
> + }
> + return -ENODEV;
> +}
> +
> +static int cmd_db_get_header_by_rsc_id(const char *resource_id,
> + struct entry_header *ent_hdr,
> + struct rsc_hdr *rsc_hdr)
> +{
> + u64 rsc_id = cmd_db_get_u64_id(resource_id);
> +
> + return cmd_db_get_header(rsc_id, ent_hdr, rsc_hdr, false);
> +}
> +
> +u32 cmd_db_get_addr(const char *resource_id)
> +{
> + int ret;
> + struct entry_header ent;
> + struct rsc_hdr rsc_hdr;
> +
> + ret = cmd_db_get_header_by_rsc_id(resource_id, &ent, &rsc_hdr);
> +
> + return ret < 0 ? 0 : ent.addr;
> +}
> +EXPORT_SYMBOL(cmd_db_get_addr);
> +
> +int cmd_db_get_aux_data(const char *resource_id, u8 *data, int len)
> +{
> + int ret;
> + struct entry_header ent;
> + struct rsc_hdr rsc_hdr;
> +
> + if (!data)
> + return -EINVAL;
> +
> + ret = cmd_db_get_header_by_rsc_id(resource_id, &ent, &rsc_hdr);
> +
> + if (ret)
> + return ret;
> +
> + if (ent.len < len)
> + return -EINVAL;
> +
> + len = (ent.len < len) ? ent.len : len;
> +
> + memcpy_fromio(data,
> + start_addr + sizeof(*cmd_db_header)
> + + rsc_hdr.data_offset + ent.offset,
Maybe make a macro for rsc_offset() or ent_offset() or something
like that?
> + len);
> + return len;
> +}
> +EXPORT_SYMBOL(cmd_db_get_aux_data);
> +
> +int cmd_db_get_aux_data_len(const char *resource_id)
> +{
> + int ret;
> + struct entry_header ent;
> + struct rsc_hdr rsc_hdr;
> +
> + ret = cmd_db_get_header_by_rsc_id(resource_id, &ent, &rsc_hdr);
> +
> + return ret < 0 ? 0 : ent.len;
> +}
> +EXPORT_SYMBOL(cmd_db_get_aux_data_len);
> +
> +u16 cmd_db_get_version(const char *resource_id)
> +{
> + int ret;
> + struct entry_header ent;
> + struct rsc_hdr rsc_hdr;
> +
> + ret = cmd_db_get_header_by_rsc_id(resource_id, &ent, &rsc_hdr);
> + return ret < 0 ? 0 : rsc_hdr.version;
> +}
> +EXPORT_SYMBOL(cmd_db_get_version);
> +
> +int cmd_db_ready(void)
> +{
> + return cmd_db_status;
> +}
> +EXPORT_SYMBOL(cmd_db_ready);
> +
> +int cmd_db_get_slave_id(const char *resource_id)
> +{
> + int ret;
> + struct entry_header ent;
> + struct rsc_hdr rsc_hdr;
> +
> + ret = cmd_db_get_header_by_rsc_id(resource_id, &ent, &rsc_hdr);
> + return ret < 0 ? 0 : (ent.addr >> SLAVE_ID_SHIFT) & SLAVE_ID_MASK;
> +}
> +EXPORT_SYMBOL(cmd_db_get_slave_id);
> +
> +static int cmd_db_dev_probe(struct platform_device *pdev)
> +{
> + struct resource res;
> + void __iomem *dict;
> +
> + dict = of_iomap(pdev->dev.of_node, 0);
> + if (!dict) {
> + cmd_db_status = -ENOMEM;
> + goto failed;
> + }
> +
> + /*
> + * Read start address and size of the command DB address from
> + * shared dictionary location
> + */
> + res.start = readl_relaxed(dict);
> + res.end = res.start + readl_relaxed(dict + 0x4);
> + res.flags = IORESOURCE_MEM;
> + iounmap(dict);
> +
> + start_addr = devm_ioremap_resource(&pdev->dev, &res);
And if this mapping fails? Is it in DDR? We would need to not use
ioremap then.
> +
> + cmd_db_header = devm_kzalloc(&pdev->dev,
> + sizeof(*cmd_db_header), GFP_KERNEL);
> +
> + if (!cmd_db_header) {
> + cmd_db_status = -ENOMEM;
> + goto failed;
> + }
> +
> + memcpy(cmd_db_header, start_addr, sizeof(*cmd_db_header));
Why are we copying this? We can't just read from the remmapped
ram directly?
> +
> + if (cmd_db_header->magic_num != CMD_DB_MAGIC) {
memcmp?
> + pr_err("%s(): Invalid Magic\n", __func__);
> + cmd_db_status = -EINVAL;
> + goto failed;
> + }
> + cmd_db_status = 0;
> + of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
What are we populating?
> +
> +failed:
> + return cmd_db_status;
> +}
> +
> +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",
> + .owner = THIS_MODULE,
Drop this.
> + .of_match_table = cmd_db_match_table,
> + },
> +};
> +
> +int __init cmd_db_device_init(void)
static? Please run sparse.
> +{
> + 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..1e42f7509cf9
> --- /dev/null
> +++ b/include/soc/qcom/cmd-db.h
> @@ -0,0 +1,119 @@
> +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __QCOM_COMMAND_DB_H__
> +#define __QCOM_COMMAND_DB_H__
> +
> +
> +enum cmd_db_hw_type {
> + 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,
> +};
> +
> +#ifdef CONFIG_QCOM_COMMAND_DB
> +/**
> + * cmd_db_get_addr() - Query command db for resource id address.
> + *
> + * This is used to retrieve resource address based on resource
> + * id.
> + *
> + * @resource_id : resource id to query for address
> + *
> + * returns resource address on success or 0 on error otherwise
> + */
> +u32 cmd_db_get_addr(const char *resource_id);
> +
> +/**
> + * cmd_db_get_aux_data() - Query command db for aux data. This is used to
> + * retrieve a command DB entry based resource address.
> + *
> + * @resource_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.
> + *
> + * returns size of data on success, errno on error
> + */
> +int cmd_db_get_aux_data(const char *resource_id, u8 *data, int len);
> +
> +/**
> + * cmd_db_get_aux_data_len - Get the length of the auxllary data stored in DB.
> + *
> + * @resource_id: Resource to retrieve AUX Data.
> + *
> + * returns size on success, errno on error
> + */
> +int cmd_db_get_aux_data_len(const char *resource_id);
> +
> +/**
> + * cmd_db_get_version - Get the version of the command DB data
> + *
> + * @resource_id: Resource id to query the DB for version
> + *
> + * returns version on success, 0 on error.
> + * Major number in MSB, minor number in LSB
> + */
> +u16 cmd_db_get_version(const char *resource_id);
> +
> +/**
> + * cmd_db_ready - Indicates if command DB is probed
> + *
> + * returns 0 on success , errno otherwise
> + */
> +int cmd_db_ready(void);
> +
> +/**
> + * cmd_db_get_slave_id - Get the slave ID for a given resource address
> + *
> + * @resource_id: Resource id to query the DB for version
> + *
> + * return cmd_db_hw_type enum on success, errno on error
> + */
> +int cmd_db_get_slave_id(const char *resource_id);
> +
> +#else
> +
> +static inline u32 cmd_db_get_addr(const char *resource_id)
> +{
> + return 0;
> +}
> +
> +int cmd_db_get_aux_data(const char *resource_id, u8 *data, int len)
All these below are missing static inline?
> +{
> + return -ENODEV;
> +}
> +
> +int cmd_db_get_aux_data_len(const char *resource_id)
> +{
> + return -ENODEV;
> +}
> +
> +u16 cmd_db_get_version(const char *resource_id)
> +{
> + return 0;
> +}
> +
> +int cmd_db_ready(void)
> +{
> + return -ENODEV;
> +}
> +
> +int cmd_db_get_slave_id(const char *resource_id)
> +{
> + return -ENODEV;
> +}
> +
> +#endif /* CONFIG_QCOM_COMMAND_DB */
> +#endif /* __QCOM_COMMAND_DB_H__ */
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
