Changes since v10:
- remove subsys-cell from gce device node
- use of_parse_phandle_with_fixed_args instead of
of_parse_phandle_with_args
Changes since v8 and v9:
- change the error return code in cmdq_dev_get_client_reg()
Changes since v7:
- remove the memory allocation out of cmdq_dev_get_client_reg()
- rebase onto 5.2-rc1
Changes since v6:
- remove cmdq_dev_get_event function and gce event property
- separate some changes to indepentent patch
- change the binding document related to gce-client-reg property
Changes since v5:
- fix typo
- remove gce-event-name form the dt-binding
- add reasons in commit message
Changes since v4:
- refine the architecture of the packet encoder function
- refine the gce enevt property
- change the patch's title
Changes since v3:
- fix a typo in dt-binding and dtsi
- cast the return value to right format
Changes since v2:
- according to CK's review comment, change the property name and
refine the parameter
- change the patch's title
- remove unused property from dt-binding and dts
Changes since v1:
- add prefix "cmdq" in the commit subject
- add dt-binding document for get event and subsys function
- add fix up tag in fixup patch
- fix up some coding style (alignment)
MTK will support gce function on mt8183 platform.
dt-binding: gce: add gce header file for mt8183
mailbox: mediatek: cmdq: support mt8183 gce function
arm64: dts: add gce node for mt8183
Besides above patches, we refine gce driver on those patches.
soc: mediatek: cmdq: reorder the parameter
soc: mediatek: cmdq: change the type of input parameter
mailbox: mediatek: cmdq: move the CMDQ_IRQ_MASK into cmdq driver data
soc: mediatek: cmdq: clear the event in cmdq initial flow
In order to enhance the convenience of gce usage, we add new
helper functions and refine the method of instruction combining.
dt-binding: gce: remove thread-num property
dt-binding: gce: add binding for gce client reg property
soc: mediatek: cmdq: define the instruction struct
soc: mediatek: cmdq: add polling function
soc: mediatek: cmdq: add cmdq_dev_get_client_reg function
Bibby Hsieh (12):
dt-binding: gce: remove thread-num property
dt-binding: gce: add gce header file for mt8183
dt-binding: gce: add binding for gce client reg property
mailbox: mediatek: cmdq: move the CMDQ_IRQ_MASK into cmdq driver data
mailbox: mediatek: cmdq: support mt8183 gce function
soc: mediatek: cmdq: clear the event in cmdq initial flow
soc: mediatek: cmdq: reorder the parameter
soc: mediatek: cmdq: change the type of input parameter
soc: mediatek: cmdq: define the instruction struct
soc: mediatek: cmdq: add polling function
soc: mediatek: cmdq: add cmdq_dev_get_client_reg function
arm64: dts: add gce node for mt8183
.../devicetree/bindings/mailbox/mtk-gce.txt | 23 ++-
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 10 +
drivers/mailbox/mtk-cmdq-mailbox.c | 18 +-
drivers/soc/mediatek/mtk-cmdq-helper.c | 170 +++++++++++++----
include/dt-bindings/gce/mt8183-gce.h | 177 ++++++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 5 +
include/linux/soc/mediatek/mtk-cmdq.h | 53 +++++-
7 files changed, 395 insertions(+), 61 deletions(-)
create mode 100644 include/dt-bindings/gce/mt8183-gce.h
--
2.18.0
The interrupt mask and thread number has positive correlation,
so we move the CMDQ_IRQ_MASK into cmdq driver data and calculate
it by thread number.
Signed-off-by: Bibby Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 00d5219094e5..8fddd26288e8 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -18,7 +18,6 @@
#include <linux/of_device.h>
#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT)
-#define CMDQ_IRQ_MASK 0xffff
#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / CMDQ_INST_SIZE)
#define CMDQ_CURR_IRQ_STATUS 0x10
@@ -72,6 +71,7 @@ struct cmdq {
void __iomem *base;
u32 irq;
u32 thread_nr;
+ u32 irq_mask;
struct cmdq_thread *thread;
struct clk *clock;
bool suspended;
@@ -285,11 +285,11 @@ static irqreturn_t cmdq_irq_handler(int irq, void *dev)
unsigned long irq_status, flags = 0L;
int bit;
- irq_status = readl(cmdq->base + CMDQ_CURR_IRQ_STATUS) & CMDQ_IRQ_MASK;
- if (!(irq_status ^ CMDQ_IRQ_MASK))
+ irq_status = readl(cmdq->base + CMDQ_CURR_IRQ_STATUS) & cmdq->irq_mask;
+ if (!(irq_status ^ cmdq->irq_mask))
return IRQ_NONE;
- for_each_clear_bit(bit, &irq_status, fls(CMDQ_IRQ_MASK)) {
+ for_each_clear_bit(bit, &irq_status, cmdq->thread_nr) {
struct cmdq_thread *thread = &cmdq->thread[bit];
spin_lock_irqsave(&thread->chan->lock, flags);
@@ -473,6 +473,9 @@ static int cmdq_probe(struct platform_device *pdev)
dev_err(dev, "failed to get irq\n");
return -EINVAL;
}
+
+ cmdq->thread_nr = (u32)(unsigned long)of_device_get_match_data(dev);
+ cmdq->irq_mask = GENMASK(cmdq->thread_nr - 1, 0);
err = devm_request_irq(dev, cmdq->irq, cmdq_irq_handler, IRQF_SHARED,
"mtk_cmdq", cmdq);
if (err < 0) {
@@ -489,7 +492,6 @@ static int cmdq_probe(struct platform_device *pdev)
return PTR_ERR(cmdq->clock);
}
- cmdq->thread_nr = (u32)(unsigned long)of_device_get_match_data(dev);
cmdq->mbox.dev = dev;
cmdq->mbox.chans = devm_kcalloc(dev, cmdq->thread_nr,
sizeof(*cmdq->mbox.chans), GFP_KERNEL);
--
2.18.0
The order of gce instructions is [subsys offset value]
so reorder the parameter of cmdq_pkt_write_mask
and cmdq_pkt_write function.
Signed-off-by: Bibby Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 6 +++---
include/linux/soc/mediatek/mtk-cmdq.h | 10 +++++-----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ff9fef5a032b..082b8978651e 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -136,7 +136,7 @@ static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
return 0;
}
-int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 value, u32 subsys, u32 offset)
+int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 subsys, u32 offset, u32 value)
{
u32 arg_a = (offset & CMDQ_ARG_A_WRITE_MASK) |
(subsys << CMDQ_SUBSYS_SHIFT);
@@ -145,8 +145,8 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 value, u32 subsys, u32 offset)
}
EXPORT_SYMBOL(cmdq_pkt_write);
-int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 value,
- u32 subsys, u32 offset, u32 mask)
+int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 subsys,
+ u32 offset, u32 value, u32 mask)
{
u32 offset_mask = offset;
int err = 0;
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index 4e8899972db4..39d813dde4b4 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -60,26 +60,26 @@ void cmdq_pkt_destroy(struct cmdq_pkt *pkt);
/**
* cmdq_pkt_write() - append write command to the CMDQ packet
* @pkt: the CMDQ packet
- * @value: the specified target register value
* @subsys: the CMDQ sub system code
* @offset: register offset from CMDQ sub system
+ * @value: the specified target register value
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 value, u32 subsys, u32 offset);
+int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 subsys, u32 offset, u32 value);
/**
* cmdq_pkt_write_mask() - append write command with mask to the CMDQ packet
* @pkt: the CMDQ packet
- * @value: the specified target register value
* @subsys: the CMDQ sub system code
* @offset: register offset from CMDQ sub system
+ * @value: the specified target register value
* @mask: the specified target register mask
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 value,
- u32 subsys, u32 offset, u32 mask);
+int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 subsys,
+ u32 offset, u32 value, u32 mask);
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
--
2.18.0
Define an instruction structure for gce driver to append command.
This structure can make the client's code more readability.
Signed-off-by: Bibby Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 103 +++++++++++++++--------
include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
2 files changed, 72 insertions(+), 33 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 7aa0517ff2f3..0886c4967ca4 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -9,12 +9,24 @@
#include <linux/mailbox_controller.h>
#include <linux/soc/mediatek/mtk-cmdq.h>
-#define CMDQ_ARG_A_WRITE_MASK 0xffff
#define CMDQ_WRITE_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_EN BIT(0)
#define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
<< 32 | CMDQ_EOC_IRQ_EN)
+struct cmdq_instruction {
+ union {
+ u32 value;
+ u32 mask;
+ };
+ union {
+ u16 offset;
+ u16 event;
+ };
+ u8 subsys;
+ u8 op;
+};
+
static void cmdq_client_timeout(struct timer_list *t)
{
struct cmdq_client *client = from_timer(client, t, timer);
@@ -110,10 +122,8 @@ void cmdq_pkt_destroy(struct cmdq_pkt *pkt)
}
EXPORT_SYMBOL(cmdq_pkt_destroy);
-static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
- u32 arg_a, u32 arg_b)
+static struct cmdq_instruction *cmdq_pkt_append_command(struct cmdq_pkt *pkt)
{
- u64 *cmd_ptr;
if (unlikely(pkt->cmd_buf_size + CMDQ_INST_SIZE > pkt->buf_size)) {
/*
@@ -127,81 +137,108 @@ static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
pkt->cmd_buf_size += CMDQ_INST_SIZE;
WARN_ONCE(1, "%s: buffer size %u is too small !\n",
__func__, (u32)pkt->buf_size);
- return -ENOMEM;
+ return NULL;
}
- cmd_ptr = pkt->va_base + pkt->cmd_buf_size;
- (*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b;
+
pkt->cmd_buf_size += CMDQ_INST_SIZE;
- return 0;
+ return pkt->va_base + pkt->cmd_buf_size - CMDQ_INST_SIZE;
}
int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value)
{
- u32 arg_a = (offset & CMDQ_ARG_A_WRITE_MASK) |
- (subsys << CMDQ_SUBSYS_SHIFT);
+ struct cmdq_instruction *inst;
+
+ inst = cmdq_pkt_append_command(pkt);
+ if (!inst)
+ return -ENOMEM;
+
+ inst->op = CMDQ_CODE_WRITE;
+ inst->value = value;
+ inst->offset = offset;
+ inst->subsys = subsys;
- return cmdq_pkt_append_command(pkt, CMDQ_CODE_WRITE, arg_a, value);
+ return 0;
}
EXPORT_SYMBOL(cmdq_pkt_write);
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask)
{
+ struct cmdq_instruction *inst;
u32 offset_mask = offset;
- int err = 0;
if (mask != 0xffffffff) {
- err = cmdq_pkt_append_command(pkt, CMDQ_CODE_MASK, 0, ~mask);
+ inst = cmdq_pkt_append_command(pkt);
+ if (!inst)
+ return -ENOMEM;
+
+ inst->op = CMDQ_CODE_MASK;
+ inst->mask = ~mask;
offset_mask |= CMDQ_WRITE_ENABLE_MASK;
}
- err |= cmdq_pkt_write(pkt, value, subsys, offset_mask);
- return err;
+ return cmdq_pkt_write(pkt, subsys, offset_mask, value);
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
- u32 arg_b;
+ struct cmdq_instruction *inst;
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
- /*
- * WFE arg_b
- * bit 0-11: wait value
- * bit 15: 1 - wait, 0 - no wait
- * bit 16-27: update value
- * bit 31: 1 - update, 0 - no update
- */
- arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
+ inst = cmdq_pkt_append_command(pkt);
+ if (!inst)
+ return -ENOMEM;
+
+ inst->op = CMDQ_CODE_WFE;
+ inst->value = CMDQ_WFE_OPTION;
+ inst->event = event;
- return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE, event, arg_b);
+ return 0;
}
EXPORT_SYMBOL(cmdq_pkt_wfe);
int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
{
+ struct cmdq_instruction *inst;
+
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
- return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE, event,
- CMDQ_WFE_UPDATE);
+ inst = cmdq_pkt_append_command(pkt);
+ if (!inst)
+ return -ENOMEM;
+
+ inst->op = CMDQ_CODE_WFE;
+ inst->value = CMDQ_WFE_UPDATE;
+ inst->event = event;
+
+ return 0;
}
EXPORT_SYMBOL(cmdq_pkt_clear_event);
static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
- int err;
+ struct cmdq_instruction *inst;
+
+ inst = cmdq_pkt_append_command(pkt);
+ if (!inst)
+ return -ENOMEM;
- /* insert EOC and generate IRQ for each command iteration */
- err = cmdq_pkt_append_command(pkt, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN);
+ inst->op = CMDQ_CODE_EOC;
+ inst->value = CMDQ_EOC_IRQ_EN;
- /* JUMP to end */
- err |= cmdq_pkt_append_command(pkt, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS);
+ inst = cmdq_pkt_append_command(pkt);
+ if (!inst)
+ return -ENOMEM;
+
+ inst->op = CMDQ_CODE_JUMP;
+ inst->value = CMDQ_JUMP_PASS;
- return err;
+ return 0;
}
static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 911475da7a53..c8adedefaf42 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -19,6 +19,8 @@
#define CMDQ_WFE_UPDATE BIT(31)
#define CMDQ_WFE_WAIT BIT(15)
#define CMDQ_WFE_WAIT_VALUE 0x1
+#define CMDQ_WFE_OPTION (CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | \
+ CMDQ_WFE_WAIT_VALUE)
/** cmdq event maximum */
#define CMDQ_MAX_EVENT 0x3ff
--
2.18.0
add gce device node for mt8183
Signed-off-by: Bibby Hsieh <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 08274bfcebd8..98d17d0bdebf 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -8,6 +8,7 @@
#include <dt-bindings/clock/mt8183-clk.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/gce/mt8183-gce.h>
/ {
compatible = "mediatek,mt8183";
@@ -212,6 +213,15 @@
clock-names = "spi", "wrap";
};
+ gce: gce@10238000 {
+ compatible = "mediatek,mt8183-gce";
+ reg = <0 0x10238000 0 0x4000>;
+ interrupts = <GIC_SPI 162 IRQ_TYPE_LEVEL_LOW>;
+ #mbox-cells = <3>;
+ clocks = <&infracfg CLK_INFRA_GCE>;
+ clock-names = "gce";
+ };
+
uart0: serial@11002000 {
compatible = "mediatek,mt8183-uart",
"mediatek,mt6577-uart";
--
2.18.0
add mt8183 compatible name for supporting gce function
Signed-off-by: Bibby Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 8fddd26288e8..69daaadc3a5f 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -539,6 +539,7 @@ static const struct dev_pm_ops cmdq_pm_ops = {
static const struct of_device_id cmdq_of_ids[] = {
{.compatible = "mediatek,mt8173-gce", .data = (void *)16},
+ {.compatible = "mediatek,mt8183-gce", .data = (void *)24},
{}
};
--
2.18.0
According to the cmdq hardware design, the subsys is u8,
the offset is u16 and the event id is u16.
This patch changes the type of subsys, offset and event id
to the correct type.
Signed-off-by: Bibby Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 10 +++++-----
include/linux/soc/mediatek/mtk-cmdq.h | 10 +++++-----
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 082b8978651e..7aa0517ff2f3 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -136,7 +136,7 @@ static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
return 0;
}
-int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 subsys, u32 offset, u32 value)
+int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value)
{
u32 arg_a = (offset & CMDQ_ARG_A_WRITE_MASK) |
(subsys << CMDQ_SUBSYS_SHIFT);
@@ -145,8 +145,8 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 subsys, u32 offset, u32 value)
}
EXPORT_SYMBOL(cmdq_pkt_write);
-int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 subsys,
- u32 offset, u32 value, u32 mask)
+int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
+ u16 offset, u32 value, u32 mask)
{
u32 offset_mask = offset;
int err = 0;
@@ -161,7 +161,7 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u32 event)
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
u32 arg_b;
@@ -181,7 +181,7 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u32 event)
}
EXPORT_SYMBOL(cmdq_pkt_wfe);
-int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u32 event)
+int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
{
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index 39d813dde4b4..9618debb9ceb 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -66,7 +66,7 @@ void cmdq_pkt_destroy(struct cmdq_pkt *pkt);
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 subsys, u32 offset, u32 value);
+int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
/**
* cmdq_pkt_write_mask() - append write command with mask to the CMDQ packet
@@ -78,8 +78,8 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 subsys, u32 offset, u32 value);
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 subsys,
- u32 offset, u32 value, u32 mask);
+int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
+ u16 offset, u32 value, u32 mask);
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
@@ -88,7 +88,7 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 subsys,
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u32 event);
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
/**
* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
@@ -97,7 +97,7 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u32 event);
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u32 event);
+int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
/**
* cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
--
2.18.0
"thread-num" is an unused property so we remove it from example.
Signed-off-by: Bibby Hsieh <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/mailbox/mtk-gce.txt | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
index 7d72b21c9e94..cfe40b01d164 100644
--- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
+++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt
@@ -39,7 +39,6 @@ Example:
interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_LOW>;
clocks = <&infracfg CLK_INFRA_GCE>;
clock-names = "gce";
- thread-num = CMDQ_THR_MAX_COUNT;
#mbox-cells = <3>;
};
--
2.18.0
Hi Bibby, I have inline comment in function cmdq_pkt_write_mask().
On Mon, 2019-07-29 at 15:01 +0800, Bibby Hsieh wrote:
> Define an instruction structure for gce driver to append command.
> This structure can make the client's code more readability.
>
> Signed-off-by: Bibby Hsieh <[email protected]>
> Reviewed-by: CK Hu <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 103 +++++++++++++++--------
> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
> 2 files changed, 72 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 7aa0517ff2f3..0886c4967ca4 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -9,12 +9,24 @@
> #include <linux/mailbox_controller.h>
> #include <linux/soc/mediatek/mtk-cmdq.h>
>
> -#define CMDQ_ARG_A_WRITE_MASK 0xffff
> #define CMDQ_WRITE_ENABLE_MASK BIT(0)
> #define CMDQ_EOC_IRQ_EN BIT(0)
> #define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
> << 32 | CMDQ_EOC_IRQ_EN)
>
> +struct cmdq_instruction {
> + union {
> + u32 value;
> + u32 mask;
> + };
> + union {
> + u16 offset;
> + u16 event;
> + };
> + u8 subsys;
> + u8 op;
> +};
> +
> static void cmdq_client_timeout(struct timer_list *t)
> {
> struct cmdq_client *client = from_timer(client, t, timer);
> @@ -110,10 +122,8 @@ void cmdq_pkt_destroy(struct cmdq_pkt *pkt)
> }
> EXPORT_SYMBOL(cmdq_pkt_destroy);
>
> -static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
> - u32 arg_a, u32 arg_b)
> +static struct cmdq_instruction *cmdq_pkt_append_command(struct cmdq_pkt *pkt)
> {
> - u64 *cmd_ptr;
>
> if (unlikely(pkt->cmd_buf_size + CMDQ_INST_SIZE > pkt->buf_size)) {
> /*
> @@ -127,81 +137,108 @@ static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
> pkt->cmd_buf_size += CMDQ_INST_SIZE;
> WARN_ONCE(1, "%s: buffer size %u is too small !\n",
> __func__, (u32)pkt->buf_size);
> - return -ENOMEM;
> + return NULL;
> }
> - cmd_ptr = pkt->va_base + pkt->cmd_buf_size;
> - (*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b;
> +
> pkt->cmd_buf_size += CMDQ_INST_SIZE;
>
> - return 0;
> + return pkt->va_base + pkt->cmd_buf_size - CMDQ_INST_SIZE;
> }
>
> int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value)
> {
> - u32 arg_a = (offset & CMDQ_ARG_A_WRITE_MASK) |
> - (subsys << CMDQ_SUBSYS_SHIFT);
> + struct cmdq_instruction *inst;
> +
> + inst = cmdq_pkt_append_command(pkt);
> + if (!inst)
> + return -ENOMEM;
> +
> + inst->op = CMDQ_CODE_WRITE;
> + inst->value = value;
> + inst->offset = offset;
> + inst->subsys = subsys;
>
> - return cmdq_pkt_append_command(pkt, CMDQ_CODE_WRITE, arg_a, value);
> + return 0;
> }
> EXPORT_SYMBOL(cmdq_pkt_write);
>
> int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value, u32 mask)
> {
> + struct cmdq_instruction *inst;
> u32 offset_mask = offset;
> - int err = 0;
>
> if (mask != 0xffffffff) {
> - err = cmdq_pkt_append_command(pkt, CMDQ_CODE_MASK, 0, ~mask);
> + inst = cmdq_pkt_append_command(pkt);
> + if (!inst)
> + return -ENOMEM;
> +
> + inst->op = CMDQ_CODE_MASK;
> + inst->mask = ~mask;
There were some discussion about how to reduce cmdq buffer allocation
times reuse a cmdq packet.Please refer to the below links.
https://patchwork.kernel.org/patch/10193349/
https://patchwork.kernel.org/patch/10491161/
If we reuse a cmdq packet, we get the cmdq instruction buffer which may
contains previous data. To generate correct instructions, we may need
consider how to clear the previous data.
1.Set all members of a cmdq_instruction instance.
e.g. Add two lines of code below for this case.
inst->offset = 0;
inst->subsys = 0;
2. Before reuse a packet, do memset() for the command buffer.
How do you think about it?
> offset_mask |= CMDQ_WRITE_ENABLE_MASK;
> }
> - err |= cmdq_pkt_write(pkt, value, subsys, offset_mask);
>
> - return err;
> + return cmdq_pkt_write(pkt, subsys, offset_mask, value);
> }
> EXPORT_SYMBOL(cmdq_pkt_write_mask);
>
> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> {
> - u32 arg_b;
> + struct cmdq_instruction *inst;
>
> if (event >= CMDQ_MAX_EVENT)
> return -EINVAL;
>
> - /*
> - * WFE arg_b
> - * bit 0-11: wait value
> - * bit 15: 1 - wait, 0 - no wait
> - * bit 16-27: update value
> - * bit 31: 1 - update, 0 - no update
> - */
> - arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
> + inst = cmdq_pkt_append_command(pkt);
> + if (!inst)
> + return -ENOMEM;
> +
> + inst->op = CMDQ_CODE_WFE;
> + inst->value = CMDQ_WFE_OPTION;
> + inst->event = event;
>
> - return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE, event, arg_b);
> + return 0;
> }
> EXPORT_SYMBOL(cmdq_pkt_wfe);
>
> int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
> {
> + struct cmdq_instruction *inst;
> +
> if (event >= CMDQ_MAX_EVENT)
> return -EINVAL;
>
> - return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE, event,
> - CMDQ_WFE_UPDATE);
> + inst = cmdq_pkt_append_command(pkt);
> + if (!inst)
> + return -ENOMEM;
> +
> + inst->op = CMDQ_CODE_WFE;
> + inst->value = CMDQ_WFE_UPDATE;
> + inst->event = event;
> +
> + return 0;
> }
> EXPORT_SYMBOL(cmdq_pkt_clear_event);
>
> static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> {
> - int err;
> + struct cmdq_instruction *inst;
> +
> + inst = cmdq_pkt_append_command(pkt);
> + if (!inst)
> + return -ENOMEM;
>
> - /* insert EOC and generate IRQ for each command iteration */
> - err = cmdq_pkt_append_command(pkt, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN);
> + inst->op = CMDQ_CODE_EOC;
> + inst->value = CMDQ_EOC_IRQ_EN;
>
> - /* JUMP to end */
> - err |= cmdq_pkt_append_command(pkt, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS);
> + inst = cmdq_pkt_append_command(pkt);
> + if (!inst)
> + return -ENOMEM;
> +
> + inst->op = CMDQ_CODE_JUMP;
> + inst->value = CMDQ_JUMP_PASS;
>
> - return err;
> + return 0;
> }
>
> static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 911475da7a53..c8adedefaf42 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -19,6 +19,8 @@
> #define CMDQ_WFE_UPDATE BIT(31)
> #define CMDQ_WFE_WAIT BIT(15)
> #define CMDQ_WFE_WAIT_VALUE 0x1
> +#define CMDQ_WFE_OPTION (CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | \
> + CMDQ_WFE_WAIT_VALUE)
> /** cmdq event maximum */
> #define CMDQ_MAX_EVENT 0x3ff
>
On Mon, 2019-07-29 at 15:01 +0800, Bibby Hsieh wrote:
> add gce device node for mt8183
>
> Signed-off-by: Bibby Hsieh <[email protected]>
> ---
> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 08274bfcebd8..98d17d0bdebf 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -8,6 +8,7 @@
> #include <dt-bindings/clock/mt8183-clk.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/gce/mt8183-gce.h>
>
> / {
> compatible = "mediatek,mt8183";
> @@ -212,6 +213,15 @@
> clock-names = "spi", "wrap";
> };
>
> + gce: gce@10238000 {
Use mailbox@ instead of gce@, Rob suggested in
https://patchwork.kernel.org/patch/10491213/
> + compatible = "mediatek,mt8183-gce";
> + reg = <0 0x10238000 0 0x4000>;
> + interrupts = <GIC_SPI 162 IRQ_TYPE_LEVEL_LOW>;
> + #mbox-cells = <3>;
> + clocks = <&infracfg CLK_INFRA_GCE>;
> + clock-names = "gce";
> + };
> +
> uart0: serial@11002000 {
> compatible = "mediatek,mt8183-uart",
> "mediatek,mt6577-uart";
On Sun, 2019-08-11 at 00:12 +0800, houlong wei wrote:
> Hi Bibby, I have inline comment in function cmdq_pkt_write_mask().
>
> On Mon, 2019-07-29 at 15:01 +0800, Bibby Hsieh wrote:
> > Define an instruction structure for gce driver to append command.
> > This structure can make the client's code more readability.
> >
> > Signed-off-by: Bibby Hsieh <[email protected]>
> > Reviewed-by: CK Hu <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 103 +++++++++++++++--------
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 +
> > 2 files changed, 72 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 7aa0517ff2f3..0886c4967ca4 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -9,12 +9,24 @@
> > #include <linux/mailbox_controller.h>
> > #include <linux/soc/mediatek/mtk-cmdq.h>
[...]
> >
> > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask)
> > {
> > + struct cmdq_instruction *inst;
> > u32 offset_mask = offset;
> > - int err = 0;
> >
> > if (mask != 0xffffffff) {
> > - err = cmdq_pkt_append_command(pkt, CMDQ_CODE_MASK, 0, ~mask);
> > + inst = cmdq_pkt_append_command(pkt);
> > + if (!inst)
> > + return -ENOMEM;
> > +
> > + inst->op = CMDQ_CODE_MASK;
> > + inst->mask = ~mask;
> > offset_mask |= CMDQ_WRITE_ENABLE_MASK;
> > }
> > - err |= cmdq_pkt_write(pkt, value, subsys, offset_mask);
> >
> > - return err;
> > + return cmdq_pkt_write(pkt, subsys, offset_mask, value);
We need add a type conversion here, (u8)offset_mask, for your new
function type. Er... it's better to remove local variable 'offset_mask'
and replace it with 'offset'.
> > }
[...]