Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

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 
> >>
> >> 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 
> >> Signed-off-by: Lina Iyer 
> >> Reviewed-by: Bjorn Andersson 
> >> ---
> >
> >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 
> >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 
> >
> >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_ID8
> >-#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.

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

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 
> >>
> >> 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 
> >> Signed-off-by: Lina Iyer 
> >> Reviewed-by: Bjorn Andersson 
> >> ---
> >
> >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 
> >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 
> >
> >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_ID8
> >-#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.

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Lina Iyer]

On Fri, Apr 06 2018 at 17:23 -0600, Stephen Boyd wrote:

Quoting Lina Iyer (2018-04-06 08:13:55)

From: Mahesh Sivasubramanian 

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 
Signed-off-by: Lina Iyer 
Reviewed-by: Bjorn Andersson 
---


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 
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 

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_PRIORITY2
#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

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Lina Iyer]

On Fri, Apr 06 2018 at 17:23 -0600, Stephen Boyd wrote:

Quoting Lina Iyer (2018-04-06 08:13:55)

From: Mahesh Sivasubramanian 

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 
Signed-off-by: Lina Iyer 
Reviewed-by: Bjorn Andersson 
---


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 
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 

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_PRIORITY2
#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

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> 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 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

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 
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 

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(_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 *)_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 = 

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> 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 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

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 
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 

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(_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 *)_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 = _db_header->header[i];
if (!rsc_hdr->slv_id)

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> 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 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

Reviewed-by: Stephen Boyd 

Re: [PATCH v7 [RESEND] 1/2] drivers: qcom: add command DB driver

[Stephen Boyd]

Quoting Lina Iyer (2018-04-06 08:13:55)
> From: Mahesh Sivasubramanian 
> 
> 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 
> Signed-off-by: Lina Iyer 
> Reviewed-by: Bjorn Andersson 
> ---

Reviewed-by: Stephen Boyd