* [PATCH v3 01/10] media: i2c: imx290: set the format before VIDIOC_SUBDEV_G_FMT is called
From: Andrey Konovalov @ 2020-05-24 19:24 UTC (permalink / raw)
To: mchehab, sakari.ailus, manivannan.sadhasivam
Cc: devicetree, c.barrett, linux-kernel, a.brela, peter.griffin,
Andrey Konovalov, linux-arm-kernel, linux-media
In-Reply-To: <20200524192505.20682-1-andrey.konovalov@linaro.org>
With the current driver 'media-ctl -p' issued right after the imx290 driver
is loaded prints:
pad0: Source
[fmt:unknown/0x0]
The format value of zero is due to the current_format field of the imx290
struct not being initialized yet.
As imx290_entity_init_cfg() calls imx290_set_fmt(), the current_mode field
is also initialized, so the line which set current_mode to a default value
in driver's probe() function is no longer needed.
Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/media/i2c/imx290.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index f7678e5a5d87..2d8c38ffe2f0 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -722,9 +722,6 @@ static int imx290_probe(struct i2c_client *client)
goto free_err;
}
- /* Set default mode to max resolution */
- imx290->current_mode = &imx290_modes[0];
-
/* get system clock (xclk) */
imx290->xclk = devm_clk_get(dev, "xclk");
if (IS_ERR(imx290->xclk)) {
@@ -809,6 +806,9 @@ static int imx290_probe(struct i2c_client *client)
goto free_ctrl;
}
+ /* Initialize the frame format (this also sets imx290->current_mode) */
+ imx290_entity_init_cfg(&imx290->sd, NULL);
+
ret = v4l2_async_register_subdev(&imx290->sd);
if (ret < 0) {
dev_err(dev, "Could not register v4l2 device\n");
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5 13/13] soc: mediatek: cmdq: add set event function
From: Matthias Brugger @ 2020-05-24 18:15 UTC (permalink / raw)
To: Dennis-YC Hsieh
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <1590341965.31286.27.camel@mtkswgap22>
On 24/05/2020 19:39, Dennis-YC Hsieh wrote:
> Hi Matthias,
>
> Thanks for your comment.
>
>
> On Sat, 2020-05-16 at 20:32 +0200, Matthias Brugger wrote:
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> Add set event function in cmdq helper functions to set specific event.
>>>
>>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
>>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
>>> include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
>>> 3 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index ec5637d43254..3294c9285994 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_clear_event);
>>>
>>> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> +
>>> + if (event >= CMDQ_MAX_EVENT)
>>> + return -EINVAL;
>>> +
>>> + inst.op = CMDQ_CODE_WFE;
>>> + inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
>>> + inst.event = event;
>>> +
>>> + return cmdq_pkt_append_command(pkt, inst);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_set_event);
>>> +
>>> int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>>> u16 offset, u32 value)
>>> {
>>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> index 42d2a30e6a70..ba2d811183a9 100644
>>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> @@ -17,6 +17,7 @@
>>> #define CMDQ_JUMP_PASS CMDQ_INST_SIZE
>>>
>>> #define CMDQ_WFE_UPDATE BIT(31)
>>> +#define CMDQ_WFE_UPDATE_VALUE BIT(16)
>>> #define CMDQ_WFE_WAIT BIT(15)
>>> #define CMDQ_WFE_WAIT_VALUE 0x1
>>>
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index d63749440697..ca70296ae120 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
>>> */
>>> int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
>>>
>>> +/**
>>> + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
>>> + * @pkt: the CMDQ packet
>>> + * @event: the desired event to be set
>>
>> Can we add the events and their code, so that later on, when a consumer calls
>> cmdq_pkt_set_event() we don't have any magic values that are hard to understand?
>
> Please see patch 02/13:
> http://lists.infradead.org/pipermail/linux-mediatek/2020-March/027801.html
>
> Definitions begin with CMDQ_EVENT_ is the event id to this function.
> Since the event id is different between platform, client must parse it
> from device tree. So no magic values require when call this function.
>
>
Got it, thanks for clarification.
Matthias
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 09/13] soc: mediatek: cmdq: add write_s value function
From: Matthias Brugger @ 2020-05-24 18:13 UTC (permalink / raw)
To: Dennis-YC Hsieh
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <1590341462.31286.19.camel@mtkswgap22>
On 24/05/2020 19:31, Dennis-YC Hsieh wrote:
> Hi Matthias,
>
> Thanks for your comment.
>
> On Sat, 2020-05-16 at 20:20 +0200, Matthias Brugger wrote:
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> add write_s function in cmdq helper functions which
>>> writes a constant value to address with large dma
>>> access support.
>>>
>>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
>>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 26 ++++++++++++++++++++++++++
>>> include/linux/soc/mediatek/mtk-cmdq.h | 14 ++++++++++++++
>>> 2 files changed, 40 insertions(+)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index 03c129230cd7..a9ebbabb7439 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -269,6 +269,32 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_write_s);
>>>
>>> +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>>> + u16 addr_low, u32 value, u32 mask)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> + int err;
>>> +
>>> + if (mask != U32_MAX) {
>>> + inst.op = CMDQ_CODE_MASK;
>>> + inst.mask = ~mask;
>>> + err = cmdq_pkt_append_command(pkt, inst);
>>> + if (err < 0)
>>> + return err;
>>> +
>>> + inst.op = CMDQ_CODE_WRITE_S_MASK;
>>> + } else {
>>> + inst.op = CMDQ_CODE_WRITE_S;
>>> + }
>>> +
>>> + inst.sop = high_addr_reg_idx;
>>
>> Writing u16 value in a 5 bit wide variable?
>
> We need only 5 bits in this case. I'll change high_addr_reg_idx
> parameter to u8.
>
Ok, please make sure to mask the value, so that it's explicit in the code that
we only use the lowest 5 bits of high_addr_reg_idx.
Regards,
Matthias
>>
>>> + inst.offset = addr_low;
>>> + inst.value = value;
>>> +
>>> + return cmdq_pkt_append_command(pkt, inst);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_write_s_value);
>>> +
>>> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
>>> {
>>> struct cmdq_instruction inst = { {0} };
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index 01b4184af310..fec292aac83c 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -135,6 +135,20 @@ int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
>>> int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>>> u16 addr_low, u16 src_reg_idx, u32 mask);
>>>
>>> +/**
>>> + * cmdq_pkt_write_s_value() - append write_s command with mask to the CMDQ
>>> + * packet which write value to a physical address
>>> + * @pkt: the CMDQ packet
>>> + * @high_addr_reg_idx: internal regisger ID which contains high address of pa
>>
>> register
>
> will fix
>
>
> Regards,
> Dennis
>
>>
>>> + * @addr_low: low address of pa
>>> + * @value: the specified target value
>>> + * @mask: the specified target mask
>>> + *
>>> + * Return: 0 for success; else the error code is returned
>>> + */
>>> +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>>> + u16 addr_low, u32 value, u32 mask);
>>> +
>>> /**
>>> * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
>>> * @pkt: the CMDQ packet
>>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 07/13] soc: mediatek: cmdq: add write_s function
From: Matthias Brugger @ 2020-05-24 18:12 UTC (permalink / raw)
To: Dennis-YC Hsieh
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <1590341186.31286.16.camel@mtkswgap22>
On 24/05/2020 19:26, Dennis-YC Hsieh wrote:
> Hi Mattias,
>
> Thanks for your comment.
>
> On Sat, 2020-05-16 at 20:14 +0200, Matthias Brugger wrote:
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> add write_s function in cmdq helper functions which
>>> writes value contains in internal register to address
>>> with large dma access support.
>>>
>>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
>>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 34 +++++++++++++++++++++++-
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
>>> include/linux/soc/mediatek/mtk-cmdq.h | 20 ++++++++++++++
>>> 3 files changed, 55 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index 33153d17c9d9..90f1ff2b4b00 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -18,6 +18,10 @@ struct cmdq_instruction {
>>> union {
>>> u32 value;
>>> u32 mask;
>>> + struct {
>>> + u16 arg_c;
>>> + u16 src_reg;
>>> + };
>>> };
>>> union {
>>> u16 offset;
>>> @@ -29,7 +33,7 @@ struct cmdq_instruction {
>>> struct {
>>> u8 sop:5;
>>> u8 arg_c_t:1;
>>> - u8 arg_b_t:1;
>>> + u8 src_t:1;
>>
>> fixing patch 6/13 please. seems the struct should be added in this patch.
>
> ok, will move to this patch.
>
>>
>>> u8 dst_t:1;
>>> };
>>> };
>>> @@ -222,6 +226,34 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_write_mask);
>>>
>>> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>>> + u16 addr_low, u16 src_reg_idx, u32 mask)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> + int err;
>>> +
>>> + if (mask != U32_MAX) {
>>> + inst.op = CMDQ_CODE_MASK;
>>> + inst.mask = ~mask;
>>> + err = cmdq_pkt_append_command(pkt, inst);
>>> + if (err < 0)
>>> + return err;
>>> +
>>> + inst.mask = 0;
>>> + inst.op = CMDQ_CODE_WRITE_S_MASK;
>>> + } else {
>>> + inst.op = CMDQ_CODE_WRITE_S;
>>> + }
>>> +
>>> + inst.src_t = CMDQ_REG_TYPE;
>>
>> Not defined.
>> Please make sure that every patch compiles on it's own and does not add a
>> regression. This is very helpful if we have to bisect the kernel in the future.
>
> May I know which part not defined? The src_t defined on top of this
> patch and CMDQ_REG_TYPE defined in last patc (see 06/13).
correct, sorry for the noise.
>
>>
>>> + inst.sop = high_addr_reg_idx;
>>> + inst.offset = addr_low;
>>> + inst.src_reg = src_reg_idx;
>>> +
>>> + return cmdq_pkt_append_command(pkt, inst);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_write_s);
>>> +
>>> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
>>> {
>>> struct cmdq_instruction inst = { {0} };
>>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> index 121c3bb6d3de..8ef87e1bd03b 100644
>>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> @@ -59,6 +59,8 @@ enum cmdq_code {
>>> CMDQ_CODE_JUMP = 0x10,
>>> CMDQ_CODE_WFE = 0x20,
>>> CMDQ_CODE_EOC = 0x40,
>>> + CMDQ_CODE_WRITE_S = 0x90,
>>> + CMDQ_CODE_WRITE_S_MASK = 0x91,
>>> CMDQ_CODE_LOGIC = 0xa0,
>>> };
>>>
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index 83340211e1d3..c72d826d8934 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -12,6 +12,8 @@
>>> #include <linux/timer.h>
>>>
>>> #define CMDQ_NO_TIMEOUT 0xffffffffu
>>> +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
>>> +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | BIT(1))
>>>
>>> struct cmdq_pkt;
>>>
>>> @@ -102,6 +104,24 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
>>> int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
>>> u16 offset, u32 value, u32 mask);
>>>
>>> +/**
>>> + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
>>> + * @pkt: the CMDQ packet
>>> + * @high_addr_reg_idx: internal regisger ID which contains high address of pa
>>
>> s/regisger/register
>
> will fix
>
>>
>>> + * @addr_low: low address of pa
>>> + * @src_reg_idx: the CMDQ internal register ID which cache source value
>>> + * @mask: the specified target address mask, use U32_MAX if no need
>>> + *
>>> + * Return: 0 for success; else the error code is returned
>>> + *
>>> + * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
>>> + * to get high addrees and call cmdq_pkt_assign() to assign value into internal
>>
>> s/addrees/address
>
> will fix
>
>>
>>> + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameterwhen
>>
>> s/parameterwhen/parameter when
>
> will fix
>
>>
>>> + * call to this function.
>>> + */
>>> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>>> + u16 addr_low, u16 src_reg_idx, u32 mask);
>>> +
>>
>> In general I wonder if we shouldn't provide two functions, one that writes a
>> mask and on for the else case.
>
> ok, I'll separate this function to cmdq_pkt_write_s and
> cmdq_pkt_write_s_mask. Let the client choose which case is more
> suitable.
Sound good, thanks.
>
>
>>
>> Regards,
>> Matthias
>>
>>> /**
>>> * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
>>> * @pkt: the CMDQ packet
>>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 06/13] soc: mediatek: cmdq: add assign function
From: Matthias Brugger @ 2020-05-24 18:09 UTC (permalink / raw)
To: Dennis-YC Hsieh
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <1590339695.31286.4.camel@mtkswgap22>
On 24/05/2020 19:01, Dennis-YC Hsieh wrote:
> Hi Matthias,
>
> Thanks for your comment.
>
> On Sat, 2020-05-16 at 19:59 +0200, Matthias Brugger wrote:
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> Add assign function in cmdq helper which assign constant value into
>>> internal register by index.
>>>
>>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
>>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 24 +++++++++++++++++++++++-
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
>>> include/linux/soc/mediatek/mtk-cmdq.h | 14 ++++++++++++++
>>> 3 files changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index 98f23ba3ba47..33153d17c9d9 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -12,6 +12,7 @@
>>> #define CMDQ_WRITE_ENABLE_MASK BIT(0)
>>> #define CMDQ_POLL_ENABLE_MASK BIT(0)
>>> #define CMDQ_EOC_IRQ_EN BIT(0)
>>> +#define CMDQ_REG_TYPE 1
>>>
>>> struct cmdq_instruction {
>>> union {
>>> @@ -21,8 +22,17 @@ struct cmdq_instruction {
>>> union {
>>> u16 offset;
>>> u16 event;
>>> + u16 reg_dst;
>>> + };
>>> + union {
>>> + u8 subsys;
>>> + struct {
>>> + u8 sop:5;
>>> + u8 arg_c_t:1;
>>> + u8 arg_b_t:1;
>>> + u8 dst_t:1;
>>> + };
>>
>> This union seems without context in this patch. Please drop.
>>
>
> The dst_t use in cmdq_pkt_assign function so how about merge other
Ah didn't realize this. Then I think it's OK like it is.
Regards,
Matthias
> variables to reserved and leave dst_t ?
>
> struct {
> u8 reserved_t:7;
> u8 dst_t:1;
> };
>
>
> Regards,
> Dennis
>
>
>> Regards,
>> Matthias
>>
>>> };
>>> - u8 subsys;
>>> u8 op;
>>> };
>>>
>>> @@ -277,6 +287,18 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_poll_mask);
>>>
>>> +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> +
>>> + inst.op = CMDQ_CODE_LOGIC;
>>> + inst.dst_t = CMDQ_REG_TYPE;
>>> + inst.reg_dst = reg_idx;
>>> + inst.value = value;
>>> + return cmdq_pkt_append_command(pkt, inst);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_assign);
>>> +
>>> static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>> {
>>> struct cmdq_instruction inst = { {0} };
>>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> index dfe5b2eb85cc..121c3bb6d3de 100644
>>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> @@ -59,6 +59,7 @@ enum cmdq_code {
>>> CMDQ_CODE_JUMP = 0x10,
>>> CMDQ_CODE_WFE = 0x20,
>>> CMDQ_CODE_EOC = 0x40,
>>> + CMDQ_CODE_LOGIC = 0xa0,
>>> };
>>>
>>> enum cmdq_cb_status {
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index a74c1d5acdf3..83340211e1d3 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -152,6 +152,20 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>>> */
>>> int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>>> u16 offset, u32 value, u32 mask);
>>> +
>>> +/**
>>> + * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE
>>> + * to execute an instruction that set a constant value into
>>> + * internal register and use as value, mask or address in
>>> + * read/write instruction.
>>> + * @pkt: the CMDQ packet
>>> + * @reg_idx: the CMDQ internal register ID
>>> + * @value: the specified value
>>> + *
>>> + * Return: 0 for success; else the error code is returned
>>> + */
>>> +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
>>> +
>>> /**
>>> * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
>>> * packet and call back at the end of done packet
>>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 0/4] Introduce the Counter character device interface
From: William Breathitt Gray @ 2020-05-24 17:54 UTC (permalink / raw)
To: Jonathan Cameron
Cc: kamel.bouhara, gwendal, david, linux-iio, patrick.havelange,
alexandre.belloni, linux-kernel, mcoquelin.stm32, fabrice.gasnier,
syednwaris, linux-stm32, linux-arm-kernel, alexandre.torgue
In-Reply-To: <20200524172542.31ff6ac7@archlinux>
[-- Attachment #1.1: Type: text/plain, Size: 9099 bytes --]
On Sun, May 24, 2020 at 05:25:42PM +0100, Jonathan Cameron wrote:
>
> ...
>
> > The following are some questions I have about this patchset:
> >
> > 1. Should the data format of the character device be configured via a
> > sysfs attribute?
> >
> > In this patchset, the first 196095 bytes of the character device are
> > dedicated as a selection area to choose which Counter components or
> > extensions should be exposed; the subsequent bytes are the actual
> > data for the Counter components and extensions that were selected.
>
> That sounds like the worst of all possible worlds. Reality is you need
> to do some magic library so at that point you might as well have ioctl
> options to configure it. I wonder if you can keep the data flow
> to be a simple 'read' from the chardev but move the control away from
> that. Either control via some chrdevs but keep them to the 'set / get'
> if this element is going to turn up in the read or not. You rapidly
> run into problems though, such as now to see how large a given element
> is going to be etc. Plus ioctls are rather messier to extend than
> simply adding a new sysfs file. Various subsystems do complex
> 'descriptor' type approaches to get around this, or you could do
> self describing records rather than raw data - like an input
> ev_dev event.
Yes I agree, I don't think combining nondata with data is good design --
it's better if users are able to simply perform read/write on the
character device without having to keep track of valid offsets and
controls.
After giving this some more thought, I believe human-readable sysfs
attributes are the way to go to support configuration of the character
device. I am thinking of a system like this:
* Users configure the counter character device via a sysfs attribute
such as /sys/bus/counter/devices/counterX/chrdev_format or similar.
* Users may write to this sysfs attribute to select the components they
want to interface -- the layout can be determined as well from the
order. For example:
# echo "C0 C3 C2" > /sys/bus/counter/devices/counter0/chrdev_format
This would select Counts 0, 3, and 2 (in that order) to be available
in the /dev/counter0 node as a contiguous memory region.
You can select extensions in a similar fashion:
# echo "C4E2 S1E0" > /sys/bus/counter/devices/counter0/chrdev_format
This would select extension 2 from Count 4, and extension 0 from
Signal 1.
* Users may read from this chrdev_format sysfs attribute in order to see
the currently configured format of the character device.
* Users may perform read/write operations on the /dev/counterX node
directly; the layout of the data is what they user has configured via
the chrdev_format sysfs attribute. For example:
# echo "C0 C1 S0 S1" > /sys/bus/counter/devices/counter0/chrdev_format
Yields the following /dev/counter0 memory layout:
+-----------------+------------------+----------+----------+
| Byte 0 - Byte 7 | Byte 8 - Byte 15 | Byte 16 | Byte 17 |
+-----------------+------------------+----------+----------+
| Count 0 | Count 1 | Signal 0 | Signal 2 |
+-----------------+------------------+----------+----------+
* Users may perform select/poll operations on the /dev/counterX node.
Users can be notified if data is available or events have occurred.
The benefit of this design is that the format is robust so users can
choose the components they want to interface and in the layout they
want. For example, if I am writing a userspace application to control a
dual-axis positioning table, I can select the two counts I care about
for the position axes. This allows me to read just those two values
directly from /dev/counterX with a simple read() call, without having to
fumble around seeking to an offset and parsing the layout.
Similarly, support for future extensions is simple to implement. When
timestamp support is implemented, users can just select the desired
timestamp extension and read it directly from the /dev/counterX node;
they should also be able to perform a select()/poll() call to be
notified on new timestamps.
So what do you think of this sort of design? I think there is a useful
robustness to the simplicity of performing a single read/write call on
/dev/counterX.
> >
> > Moving this selection to a sysfs attribute and dedicating the
> > character device to just data transfer might be a better design. If
> > such a design is chosen, should the selection attribute be
> > human-readable or binary?
>
> Sysfs basically requires things are more or less human readable.
> So if you go that way I think it needs to be.
>
> >
> > 2. How much space should allotted for strings?
> >
> > Each Counter component and extension has a respective size allotted
> > for its data (u8 data is allotted 1 byte, u64 data is allotted 8
> > bytes, etc.); I have arbitrarily chosen to allot 64 bytes for
> > strings. Is this an apt size, or should string data be allotted more
> > or less space?
>
> I'd go with that being big enough, but try to keep the expose interface
> such that the size can change it it needs to the in the future.
Following along with the separation of control vs data as discussed
above, we could support a more variable size by exposing it through a
sysfs attribute (maybe a chrdev_string_size attribute or similar).
>
> >
> > 3. Should the owning component of an extension be handled by the device
> > driver or Counter subsystem?
> >
> > The Counter subsystem figures out the owner (enum counter_owner_type)
> > for each component/extension in the counter-sysfs and counter-chrdev
> > code. When a callback must be executed, there are various switch
> > statements throughout the code to check whether the respective
> > Device, Signal, or Count version of the callback should be executed;
> > similarly, the appropriate owner type must match for the struct
> > counter_data macros such as COUNTER_DATA_DEVICE_U64,
> > COUNTER_DATA_SIGNAL_U64, COUNTER_DATA_COUNT_U64, etc.
> >
> > All this complexity in the Counter subsystem code can be eliminated
> > if a single callback type with a `void *owner` parameter is defined
> > for use with all three owner types (Device, Signal, and Count). The
> > device driver would then be responsible for casting the callback
> > argument to the appropriate owner type; but in theory, this should
> > not be much of a problem since the device driver is responsible for
> > assigning the callbacks to the owning component anyway.
>
> Whilst its more complex for subsytem I think it's better to keep everything
> typed if we possibly can. Always a trade off though, so use your discretion.
>
> Jonathan
I'm going to keep it all typed for now since I don't want to make too
many changes at once. Since this is somewhat unrelated to the purpose of
introducing Counter character devices, I'll postpone the discussion to a
later date after the Counter character device interface is merged.
William Breathitt Gray
>
>
> >
> > William Breathitt Gray (4):
> > counter: Internalize sysfs interface code
> > docs: counter: Update to reflect sysfs internalization
> > counter: Add character device interface
> > docs: counter: Document character device interface
> >
> > Documentation/driver-api/generic-counter.rst | 275 +++-
> > MAINTAINERS | 3 +-
> > drivers/counter/104-quad-8.c | 547 +++----
> > drivers/counter/Makefile | 1 +
> > drivers/counter/counter-chrdev.c | 656 ++++++++
> > drivers/counter/counter-chrdev.h | 16 +
> > drivers/counter/counter-core.c | 187 +++
> > drivers/counter/counter-sysfs.c | 881 +++++++++++
> > drivers/counter/counter-sysfs.h | 14 +
> > drivers/counter/counter.c | 1496 ------------------
> > drivers/counter/ftm-quaddec.c | 89 +-
> > drivers/counter/stm32-lptimer-cnt.c | 161 +-
> > drivers/counter/stm32-timer-cnt.c | 139 +-
> > drivers/counter/ti-eqep.c | 211 +--
> > include/linux/counter.h | 626 ++++----
> > include/linux/counter_enum.h | 45 -
> > include/uapi/linux/counter-types.h | 45 +
> > 17 files changed, 2826 insertions(+), 2566 deletions(-)
> > create mode 100644 drivers/counter/counter-chrdev.c
> > create mode 100644 drivers/counter/counter-chrdev.h
> > create mode 100644 drivers/counter/counter-core.c
> > create mode 100644 drivers/counter/counter-sysfs.c
> > create mode 100644 drivers/counter/counter-sysfs.h
> > delete mode 100644 drivers/counter/counter.c
> > delete mode 100644 include/linux/counter_enum.h
> > create mode 100644 include/uapi/linux/counter-types.h
> >
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/1] pwm: mtk_disp: implement .apply()
From: Uwe Kleine-König @ 2020-05-24 17:44 UTC (permalink / raw)
To: Jitao Shi
Cc: linux-pwm, srv_heupstream, huijuan.xie, stonea168, linux-kernel,
cawa.cheng, ck.hu, Thierry Reding, linux-mediatek, bibby.hsieh,
Matthias Brugger, yingjoe.chen, eddie.huang, linux-arm-kernel
In-Reply-To: <20200410031955.111392-1-jitao.shi@mediatek.com>
Hello,
On Fri, Apr 10, 2020 at 11:19:55AM +0800, Jitao Shi wrote:
> implement the apply() for pwm.
>
> Fix the clock clk_prepare_enable and clk_disable_unprepare mismatch,
> switch the driver to support the ->apply() method.
Adding support for get_state is a separate change.
Other than that this patch looks more complicated that I would expect,
I think you can make this easier to understand/review by moving the
mismatch to a separate patch, too.
Can you please respin with these hints in mind, then I'm willing to
invest some time for a deeper review. In general it's very welcome to
convert the mtk-disp driver to the atomic API. Thanks for addressing
that.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 12/13] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
From: Dennis-YC Hsieh @ 2020-05-24 17:32 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <6029e0eb-70fa-825f-7fd4-f5127384bd8d@gmail.com>
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:30 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Add clear parameter to let client decide if
> > event should be clear to 0 after GCE receive it.
> >
> > Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +-
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 5 +++--
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 3 +--
> > include/linux/soc/mediatek/mtk-cmdq.h | 5 +++--
> > 4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 7daaabc26eb1..a065b3a412cf 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -488,7 +488,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > if (mtk_crtc->cmdq_client) {
> > cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
>
> This should be an independent patch
ok, I'll separate this part.
Regards,
Dennis
>
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index bb5be20fc70a..ec5637d43254 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -296,15 +296,16 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_s_value);
> >
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
> > {
> > struct cmdq_instruction inst = { {0} };
> > + u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
> >
> > if (event >= CMDQ_MAX_EVENT)
> > return -EINVAL;
> >
> > inst.op = CMDQ_CODE_WFE;
> > - inst.value = CMDQ_WFE_OPTION;
> > + inst.value = CMDQ_WFE_OPTION | clear_option;
> > inst.event = event;
> >
> > return cmdq_pkt_append_command(pkt, inst);
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 3f6bc0dfd5da..42d2a30e6a70 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -27,8 +27,7 @@
> > * bit 16-27: update value
> > * bit 31: 1 - update, 0 - no update
> > */
> > -#define CMDQ_WFE_OPTION (CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | \
> > - CMDQ_WFE_WAIT_VALUE)
> > +#define CMDQ_WFE_OPTION (CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE)
> >
> > /** cmdq event maximum */
> > #define CMDQ_MAX_EVENT 0x3ff
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 1a6c56f3bec1..d63749440697 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -152,11 +152,12 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> > - * @event: the desired event type to "wait and CLEAR"
> > + * @event: the desired event type to wait
> > + * @clear: clear event or not after event arrive
> > *
> > * Return: 0 for success; else the error code is returned
> > */
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
> >
> > /**
> > * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 13/13] soc: mediatek: cmdq: add set event function
From: Dennis-YC Hsieh @ 2020-05-24 17:39 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <d00dd874-edda-701a-2acc-540ed19d1300@gmail.com>
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:32 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Add set event function in cmdq helper functions to set specific event.
> >
> > Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> > include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
> > 3 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index ec5637d43254..3294c9285994 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
> > }
> > EXPORT_SYMBOL(cmdq_pkt_clear_event);
> >
> > +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > +
> > + if (event >= CMDQ_MAX_EVENT)
> > + return -EINVAL;
> > +
> > + inst.op = CMDQ_CODE_WFE;
> > + inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
> > + inst.event = event;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_set_event);
> > +
> > int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value)
> > {
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 42d2a30e6a70..ba2d811183a9 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -17,6 +17,7 @@
> > #define CMDQ_JUMP_PASS CMDQ_INST_SIZE
> >
> > #define CMDQ_WFE_UPDATE BIT(31)
> > +#define CMDQ_WFE_UPDATE_VALUE BIT(16)
> > #define CMDQ_WFE_WAIT BIT(15)
> > #define CMDQ_WFE_WAIT_VALUE 0x1
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index d63749440697..ca70296ae120 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
> > */
> > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
> >
> > +/**
> > + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @event: the desired event to be set
>
> Can we add the events and their code, so that later on, when a consumer calls
> cmdq_pkt_set_event() we don't have any magic values that are hard to understand?
Please see patch 02/13:
http://lists.infradead.org/pipermail/linux-mediatek/2020-March/027801.html
Definitions begin with CMDQ_EVENT_ is the event id to this function.
Since the event id is different between platform, client must parse it
from device tree. So no magic values require when call this function.
Regard,
Dennis
>
> Regards,
> Matthias
>
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
> > +
> > /**
> > * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
> > * execute an instruction that wait for a specified
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 10/13] soc: mediatek: cmdq: export finalize function
From: Dennis-YC Hsieh @ 2020-05-24 17:32 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <5d6b61b2-23c9-647f-fa22-73e779010bd8@gmail.com>
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:22 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Export finalize function to client which helps append eoc and jump
> > command to pkt. Let client decide call finalize or not.
> >
> > Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
> > include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
> > 3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 0dfcd1787e65..7daaabc26eb1 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > + cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> > }
> > #endif
>
> This should be a independent patch.
> Other then that patch looks good.
ok, I'll separate this part.
Regards,
Dennis
>
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index a9ebbabb7439..59bc1164b411 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> > }
> > EXPORT_SYMBOL(cmdq_pkt_assign);
> >
> > -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > {
> > struct cmdq_instruction inst = { {0} };
> > int err;
> > @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >
> > return err;
> > }
> > +EXPORT_SYMBOL(cmdq_pkt_finalize);
> >
> > static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
> > {
> > @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
> > unsigned long flags = 0;
> > struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
> >
> > - err = cmdq_pkt_finalize(pkt);
> > - if (err < 0)
> > - return err;
> > -
> > pkt->cb.cb = cb;
> > pkt->cb.data = data;
> > pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index fec292aac83c..99e77155f967 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> > */
> > int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
> >
> > +/**
> > + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
> > + * @pkt: the CMDQ packet
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
> > +
> > /**
> > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
> > * packet and call back at the end of done packet
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 09/13] soc: mediatek: cmdq: add write_s value function
From: Dennis-YC Hsieh @ 2020-05-24 17:31 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <f9fd9ea8-f706-ed4a-4c83-c53ad092035c@gmail.com>
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:20 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > add write_s function in cmdq helper functions which
> > writes a constant value to address with large dma
> > access support.
> >
> > Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 26 ++++++++++++++++++++++++++
> > include/linux/soc/mediatek/mtk-cmdq.h | 14 ++++++++++++++
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 03c129230cd7..a9ebbabb7439 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -269,6 +269,32 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_s);
> >
> > +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > + u16 addr_low, u32 value, u32 mask)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > + int err;
> > +
> > + if (mask != U32_MAX) {
> > + inst.op = CMDQ_CODE_MASK;
> > + inst.mask = ~mask;
> > + err = cmdq_pkt_append_command(pkt, inst);
> > + if (err < 0)
> > + return err;
> > +
> > + inst.op = CMDQ_CODE_WRITE_S_MASK;
> > + } else {
> > + inst.op = CMDQ_CODE_WRITE_S;
> > + }
> > +
> > + inst.sop = high_addr_reg_idx;
>
> Writing u16 value in a 5 bit wide variable?
We need only 5 bits in this case. I'll change high_addr_reg_idx
parameter to u8.
>
> > + inst.offset = addr_low;
> > + inst.value = value;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_write_s_value);
> > +
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 01b4184af310..fec292aac83c 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -135,6 +135,20 @@ int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
> > int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > u16 addr_low, u16 src_reg_idx, u32 mask);
> >
> > +/**
> > + * cmdq_pkt_write_s_value() - append write_s command with mask to the CMDQ
> > + * packet which write value to a physical address
> > + * @pkt: the CMDQ packet
> > + * @high_addr_reg_idx: internal regisger ID which contains high address of pa
>
> register
will fix
Regards,
Dennis
>
> > + * @addr_low: low address of pa
> > + * @value: the specified target value
> > + * @mask: the specified target mask
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > + u16 addr_low, u32 value, u32 mask);
> > +
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 07/13] soc: mediatek: cmdq: add write_s function
From: Dennis-YC Hsieh @ 2020-05-24 17:26 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <58575c2c-0b5a-55c0-f3b0-082bfd4b6144@gmail.com>
Hi Mattias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:14 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > add write_s function in cmdq helper functions which
> > writes value contains in internal register to address
> > with large dma access support.
> >
> > Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 34 +++++++++++++++++++++++-
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
> > include/linux/soc/mediatek/mtk-cmdq.h | 20 ++++++++++++++
> > 3 files changed, 55 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 33153d17c9d9..90f1ff2b4b00 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -18,6 +18,10 @@ struct cmdq_instruction {
> > union {
> > u32 value;
> > u32 mask;
> > + struct {
> > + u16 arg_c;
> > + u16 src_reg;
> > + };
> > };
> > union {
> > u16 offset;
> > @@ -29,7 +33,7 @@ struct cmdq_instruction {
> > struct {
> > u8 sop:5;
> > u8 arg_c_t:1;
> > - u8 arg_b_t:1;
> > + u8 src_t:1;
>
> fixing patch 6/13 please. seems the struct should be added in this patch.
ok, will move to this patch.
>
> > u8 dst_t:1;
> > };
> > };
> > @@ -222,6 +226,34 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > + u16 addr_low, u16 src_reg_idx, u32 mask)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > + int err;
> > +
> > + if (mask != U32_MAX) {
> > + inst.op = CMDQ_CODE_MASK;
> > + inst.mask = ~mask;
> > + err = cmdq_pkt_append_command(pkt, inst);
> > + if (err < 0)
> > + return err;
> > +
> > + inst.mask = 0;
> > + inst.op = CMDQ_CODE_WRITE_S_MASK;
> > + } else {
> > + inst.op = CMDQ_CODE_WRITE_S;
> > + }
> > +
> > + inst.src_t = CMDQ_REG_TYPE;
>
> Not defined.
> Please make sure that every patch compiles on it's own and does not add a
> regression. This is very helpful if we have to bisect the kernel in the future.
May I know which part not defined? The src_t defined on top of this
patch and CMDQ_REG_TYPE defined in last patc (see 06/13).
>
> > + inst.sop = high_addr_reg_idx;
> > + inst.offset = addr_low;
> > + inst.src_reg = src_reg_idx;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_write_s);
> > +
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 121c3bb6d3de..8ef87e1bd03b 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,8 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_WRITE_S = 0x90,
> > + CMDQ_CODE_WRITE_S_MASK = 0x91,
> > CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 83340211e1d3..c72d826d8934 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -12,6 +12,8 @@
> > #include <linux/timer.h>
> >
> > #define CMDQ_NO_TIMEOUT 0xffffffffu
> > +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> > +#define CMDQ_ADDR_LOW(addr) ((u16)(addr) | BIT(1))
> >
> > struct cmdq_pkt;
> >
> > @@ -102,6 +104,24 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
> > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask);
> >
> > +/**
> > + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @high_addr_reg_idx: internal regisger ID which contains high address of pa
>
> s/regisger/register
will fix
>
> > + * @addr_low: low address of pa
> > + * @src_reg_idx: the CMDQ internal register ID which cache source value
> > + * @mask: the specified target address mask, use U32_MAX if no need
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + *
> > + * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
> > + * to get high addrees and call cmdq_pkt_assign() to assign value into internal
>
> s/addrees/address
will fix
>
> > + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameterwhen
>
> s/parameterwhen/parameter when
will fix
>
> > + * call to this function.
> > + */
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > + u16 addr_low, u16 src_reg_idx, u32 mask);
> > +
>
> In general I wonder if we shouldn't provide two functions, one that writes a
> mask and on for the else case.
ok, I'll separate this function to cmdq_pkt_write_s and
cmdq_pkt_write_s_mask. Let the client choose which case is more
suitable.
>
> Regards,
> Matthias
>
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 06/13] soc: mediatek: cmdq: add assign function
From: Dennis-YC Hsieh @ 2020-05-24 17:01 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Philipp Zabel, wsd_upstream,
David Airlie, Jassi Brar, linux-kernel, dri-devel, HS Liao,
Rob Herring, linux-mediatek, Houlong Wei, Daniel Vetter, CK Hu,
Bibby Hsieh, linux-arm-kernel
In-Reply-To: <c671ea8a-07fa-a050-4679-c7aa15d19a9d@gmail.com>
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 19:59 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Add assign function in cmdq helper which assign constant value into
> > internal register by index.
> >
> > Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 24 +++++++++++++++++++++++-
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> > include/linux/soc/mediatek/mtk-cmdq.h | 14 ++++++++++++++
> > 3 files changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 98f23ba3ba47..33153d17c9d9 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -12,6 +12,7 @@
> > #define CMDQ_WRITE_ENABLE_MASK BIT(0)
> > #define CMDQ_POLL_ENABLE_MASK BIT(0)
> > #define CMDQ_EOC_IRQ_EN BIT(0)
> > +#define CMDQ_REG_TYPE 1
> >
> > struct cmdq_instruction {
> > union {
> > @@ -21,8 +22,17 @@ struct cmdq_instruction {
> > union {
> > u16 offset;
> > u16 event;
> > + u16 reg_dst;
> > + };
> > + union {
> > + u8 subsys;
> > + struct {
> > + u8 sop:5;
> > + u8 arg_c_t:1;
> > + u8 arg_b_t:1;
> > + u8 dst_t:1;
> > + };
>
> This union seems without context in this patch. Please drop.
>
The dst_t use in cmdq_pkt_assign function so how about merge other
variables to reserved and leave dst_t ?
struct {
u8 reserved_t:7;
u8 dst_t:1;
};
Regards,
Dennis
> Regards,
> Matthias
>
> > };
> > - u8 subsys;
> > u8 op;
> > };
> >
> > @@ -277,6 +287,18 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_poll_mask);
> >
> > +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > +
> > + inst.op = CMDQ_CODE_LOGIC;
> > + inst.dst_t = CMDQ_REG_TYPE;
> > + inst.reg_dst = reg_idx;
> > + inst.value = value;
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_assign);
> > +
> > static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index dfe5b2eb85cc..121c3bb6d3de 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,7 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > enum cmdq_cb_status {
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index a74c1d5acdf3..83340211e1d3 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -152,6 +152,20 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
> > */
> > int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value, u32 mask);
> > +
> > +/**
> > + * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE
> > + * to execute an instruction that set a constant value into
> > + * internal register and use as value, mask or address in
> > + * read/write instruction.
> > + * @pkt: the CMDQ packet
> > + * @reg_idx: the CMDQ internal register ID
> > + * @value: the specified value
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
> > +
> > /**
> > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
> > * packet and call back at the end of done packet
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 0/4] Introduce the Counter character device interface
From: Jonathan Cameron @ 2020-05-24 16:25 UTC (permalink / raw)
To: William Breathitt Gray
Cc: kamel.bouhara, gwendal, david, linux-iio, patrick.havelange,
alexandre.belloni, linux-kernel, mcoquelin.stm32, fabrice.gasnier,
syednwaris, linux-stm32, linux-arm-kernel, alexandre.torgue
In-Reply-To: <cover.1589654470.git.vilhelm.gray@gmail.com>
...
> The following are some questions I have about this patchset:
>
> 1. Should the data format of the character device be configured via a
> sysfs attribute?
>
> In this patchset, the first 196095 bytes of the character device are
> dedicated as a selection area to choose which Counter components or
> extensions should be exposed; the subsequent bytes are the actual
> data for the Counter components and extensions that were selected.
That sounds like the worst of all possible worlds. Reality is you need
to do some magic library so at that point you might as well have ioctl
options to configure it. I wonder if you can keep the data flow
to be a simple 'read' from the chardev but move the control away from
that. Either control via some chrdevs but keep them to the 'set / get'
if this element is going to turn up in the read or not. You rapidly
run into problems though, such as now to see how large a given element
is going to be etc. Plus ioctls are rather messier to extend than
simply adding a new sysfs file. Various subsystems do complex
'descriptor' type approaches to get around this, or you could do
self describing records rather than raw data - like an input
ev_dev event.
>
> Moving this selection to a sysfs attribute and dedicating the
> character device to just data transfer might be a better design. If
> such a design is chosen, should the selection attribute be
> human-readable or binary?
Sysfs basically requires things are more or less human readable.
So if you go that way I think it needs to be.
>
> 2. How much space should allotted for strings?
>
> Each Counter component and extension has a respective size allotted
> for its data (u8 data is allotted 1 byte, u64 data is allotted 8
> bytes, etc.); I have arbitrarily chosen to allot 64 bytes for
> strings. Is this an apt size, or should string data be allotted more
> or less space?
I'd go with that being big enough, but try to keep the expose interface
such that the size can change it it needs to the in the future.
>
> 3. Should the owning component of an extension be handled by the device
> driver or Counter subsystem?
>
> The Counter subsystem figures out the owner (enum counter_owner_type)
> for each component/extension in the counter-sysfs and counter-chrdev
> code. When a callback must be executed, there are various switch
> statements throughout the code to check whether the respective
> Device, Signal, or Count version of the callback should be executed;
> similarly, the appropriate owner type must match for the struct
> counter_data macros such as COUNTER_DATA_DEVICE_U64,
> COUNTER_DATA_SIGNAL_U64, COUNTER_DATA_COUNT_U64, etc.
>
> All this complexity in the Counter subsystem code can be eliminated
> if a single callback type with a `void *owner` parameter is defined
> for use with all three owner types (Device, Signal, and Count). The
> device driver would then be responsible for casting the callback
> argument to the appropriate owner type; but in theory, this should
> not be much of a problem since the device driver is responsible for
> assigning the callbacks to the owning component anyway.
Whilst its more complex for subsytem I think it's better to keep everything
typed if we possibly can. Always a trade off though, so use your discretion.
Jonathan
>
> William Breathitt Gray (4):
> counter: Internalize sysfs interface code
> docs: counter: Update to reflect sysfs internalization
> counter: Add character device interface
> docs: counter: Document character device interface
>
> Documentation/driver-api/generic-counter.rst | 275 +++-
> MAINTAINERS | 3 +-
> drivers/counter/104-quad-8.c | 547 +++----
> drivers/counter/Makefile | 1 +
> drivers/counter/counter-chrdev.c | 656 ++++++++
> drivers/counter/counter-chrdev.h | 16 +
> drivers/counter/counter-core.c | 187 +++
> drivers/counter/counter-sysfs.c | 881 +++++++++++
> drivers/counter/counter-sysfs.h | 14 +
> drivers/counter/counter.c | 1496 ------------------
> drivers/counter/ftm-quaddec.c | 89 +-
> drivers/counter/stm32-lptimer-cnt.c | 161 +-
> drivers/counter/stm32-timer-cnt.c | 139 +-
> drivers/counter/ti-eqep.c | 211 +--
> include/linux/counter.h | 626 ++++----
> include/linux/counter_enum.h | 45 -
> include/uapi/linux/counter-types.h | 45 +
> 17 files changed, 2826 insertions(+), 2566 deletions(-)
> create mode 100644 drivers/counter/counter-chrdev.c
> create mode 100644 drivers/counter/counter-chrdev.h
> create mode 100644 drivers/counter/counter-core.c
> create mode 100644 drivers/counter/counter-sysfs.c
> create mode 100644 drivers/counter/counter-sysfs.h
> delete mode 100644 drivers/counter/counter.c
> delete mode 100644 include/linux/counter_enum.h
> create mode 100644 include/uapi/linux/counter-types.h
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 4/4] docs: counter: Document character device interface
From: Jonathan Cameron @ 2020-05-24 16:19 UTC (permalink / raw)
To: William Breathitt Gray
Cc: kamel.bouhara, gwendal, david, linux-iio, patrick.havelange,
alexandre.belloni, linux-kernel, mcoquelin.stm32, fabrice.gasnier,
syednwaris, linux-stm32, linux-arm-kernel, alexandre.torgue
In-Reply-To: <db0a9206d31c82f8381316ef5ff9872bfb53665b.1589654470.git.vilhelm.gray@gmail.com>
On Sat, 16 May 2020 15:20:02 -0400
William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
> This patch adds high-level documentation about the Counter subsystem
> character device interface.
>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
> Documentation/driver-api/generic-counter.rst | 112 +++++++++++++------
> 1 file changed, 76 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
> index 8f85c30dea0b..58045b33b576 100644
> --- a/Documentation/driver-api/generic-counter.rst
> +++ b/Documentation/driver-api/generic-counter.rst
> @@ -223,19 +223,6 @@ whether an input line is differential or single-ended) and instead focus
> on the core idea of what the data and process represent (e.g. position
> as interpreted from quadrature encoding data).
>
> -Userspace Interface
> -===================
> -
> -Several sysfs attributes are generated by the Generic Counter interface,
> -and reside under the /sys/bus/counter/devices/counterX directory, where
> -counterX refers to the respective counter device. Please see
> -Documentation/ABI/testing/sysfs-bus-counter for detailed
> -information on each Generic Counter interface sysfs attribute.
> -
> -Through these sysfs attributes, programs and scripts may interact with
> -the Generic Counter paradigm Counts, Signals, and Synapses of respective
> -counter devices.
> -
> Driver API
> ==========
>
> @@ -377,13 +364,13 @@ driver can be exemplified by the following::
> +----------------------------+ |
> | Processes data from device | -------------------
> |----------------------------| / driver callbacks /
> - | Type: unsigned long | -------------------
> + | Type: u64 | -------------------
> | Value: 42 | |
> +----------------------------+ |
> | |
> - ---------------- |
> - / unsigned long / |
> - ---------------- |
> + ---------- |
> + / u64 / |
> + ---------- |
> | |
> | V
> | +----------------------+
> @@ -398,25 +385,32 @@ driver can be exemplified by the following::
> | / driver callbacks /
> | -------------------
> | |
> - +-------+ |
> + +-------+---------------+ |
> + | | |
> + | +-------|-------+
> + | | |
> + V | V
> + +--------------------+ | +---------------------+
> + | Counter sysfs |<-+->| Counter chrdev |
> + +--------------------+ +---------------------+
> + | Translates to the | | Translates to the |
> + | standard Counter | | standard Counter |
> + | sysfs output | | character device |
> + |--------------------| |---------------------+
> + | Type: const char * | | Type: u64 |
> + | Value: "42" | | Value: 42 |
> + +--------------------+ +---------------------+
> | |
> - | +---------------+
> - | |
> - V |
> - +--------------------+ |
> - | Counter sysfs |<-+
> - +--------------------+
> - | Translates to the |
> - | standard Counter |
> - | sysfs output |
> - |--------------------|
> - | Type: const char * |
> - | Value: "42" |
> - +--------------------+
> - |
> - ---------------
> - / const char * /
> - ---------------
> + --------------- ----------
> + / const char * / / u64 /
> + --------------- ----------
> + | |
> + | V
> + | +-----------+
> + | | read |
> + | +-----------+
> + | \ Count: 42 /
> + | -----------
> |
> V
> +--------------------------------------------------+
> @@ -425,7 +419,7 @@ driver can be exemplified by the following::
> \ Count: "42" /
> --------------------------------------------------
>
> -There are three primary components involved:
> +There are four primary components involved:
>
> Counter device driver
> ---------------------
> @@ -445,3 +439,49 @@ and vice versa.
> Please refer to the `Documentation/ABI/testing/sysfs-bus-counter` file
> for a detailed breakdown of the available Generic Counter interface
> sysfs attributes.
> +
> +Counter chrdev
> +--------------
> +Translates counter data to the standard Counter character device; data
> +is transferred via standard character device read/write calls.
> +
> +Sysfs Interface
> +===============
> +
> +Several sysfs attributes are generated by the Generic Counter interface,
> +and reside under the `/sys/bus/counter/devices/counterX` directory,
> +where `X` is to the respective counter device id. Please see
> +Documentation/ABI/testing/sysfs-bus-counter for detailed information on
> +each Generic Counter interface sysfs attribute.
> +
> +Through these sysfs attributes, programs and scripts may interact with
> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> +counter devices.
> +
> +Counter Character Device
> +========================
> +
> +Counter character device nodes are created under the `/dev` directory as
> +`counterX`, where `X` is the respective counter device id. Defines for
> +the standard Counter data types are exposed via the userspace
> +`include/uapi/linux/counter-types.h` file.
> +
> +The first 196095 bytes of the character device serve as a control
> +selection area where control exposure of desired Counter components and
> +extensions may be selected. Each byte serves as a boolean selection
> +indicator for a respective Counter component or extension. The format of
> +this area is as follows:
> +
> +* For each device extension, a byte is required.
> +* For each Signal, a byte is reserved for the Signal component, and a
> + byte is reserved for each Signal extension.
> +* For each Count, a byte is reserved for the Count component, a byte is
> + reserved for the count function, a byte is reserved for each Synapse
> + action, and byte is reserved for each Count extension.
> +
> +The selected Counter components and extensions may then be interfaced
> +after the first 196095 bytes via standard character device read/write
> +operations. The number of bytes available for each component or
> +extension is dependent on their respective data type: u8 will have 1
> +byte available, u64 will have 8 bytes available, strings will have 64
> +bytes available, etc.
From what I recall of the earlier conversation, I'm not sure this is what
was being suggested. I 'think' what people were after was a simple
read interface for just the counters (+ timestamps). This would also
include the option to use select / poll on the counter.
Simply moving over to a read / write really doesn't help for ease of use.
I'm not sure what the right control approach is, or perhaps if we even
need one (could just output all counts provided by hardware once the
chrdev is opened).
Jonathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/4] docs: counter: Update to reflect sysfs internalization
From: Jonathan Cameron @ 2020-05-24 16:12 UTC (permalink / raw)
To: William Breathitt Gray
Cc: kamel.bouhara, gwendal, david, linux-iio, patrick.havelange,
alexandre.belloni, linux-kernel, mcoquelin.stm32, fabrice.gasnier,
syednwaris, linux-stm32, linux-arm-kernel, alexandre.torgue
In-Reply-To: <2a8d7fe2e0ea435e19c89ba2aff15218c7b6eb91.1589654470.git.vilhelm.gray@gmail.com>
On Sat, 16 May 2020 15:20:00 -0400
William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
> The Counter subsystem architecture and driver implementations have
> changed in order to handle Counter sysfs interactions in a more
> consistent way. This patch updates the Generic Counter interface
> documentation to reflect the changes.
>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Looks good to me.
Jonathan
> ---
> Documentation/driver-api/generic-counter.rst | 215 +++++++++++++------
> 1 file changed, 149 insertions(+), 66 deletions(-)
>
> diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
> index e622f8f6e56a..8f85c30dea0b 100644
> --- a/Documentation/driver-api/generic-counter.rst
> +++ b/Documentation/driver-api/generic-counter.rst
> @@ -250,8 +250,8 @@ for defining a counter device.
> .. kernel-doc:: drivers/counter/counter.c
> :export:
>
> -Implementation
> -==============
> +Driver Implementation
> +=====================
>
> To support a counter device, a driver must first allocate the available
> Counter Signals via counter_signal structures. These Signals should
> @@ -267,25 +267,58 @@ respective counter_count structure. These counter_count structures are
> set to the counts array member of an allocated counter_device structure
> before the Counter is registered to the system.
>
> -Driver callbacks should be provided to the counter_device structure via
> -a constant counter_ops structure in order to communicate with the
> -device: to read and write various Signals and Counts, and to set and get
> -the "action mode" and "function mode" for various Synapses and Counts
> -respectively.
> +Driver callbacks must be provided to the counter_device structure in
> +order to communicate with the device: to read and write various Signals
> +and Counts, and to set and get the "action mode" and "function mode" for
> +various Synapses and Counts respectively.
>
> A defined counter_device structure may be registered to the system by
> passing it to the counter_register function, and unregistered by passing
> it to the counter_unregister function. Similarly, the
> -devm_counter_register and devm_counter_unregister functions may be used
> -if device memory-managed registration is desired.
> -
> -Extension sysfs attributes can be created for auxiliary functionality
> -and data by passing in defined counter_device_ext, counter_count_ext,
> -and counter_signal_ext structures. In these cases, the
> -counter_device_ext structure is used for global/miscellaneous exposure
> -and configuration of the respective Counter device, while the
> -counter_count_ext and counter_signal_ext structures allow for auxiliary
> -exposure and configuration of a specific Count or Signal respectively.
> +devm_counter_register function may be used if device memory-managed
> +registration is desired.
> +
> +The struct counter_data structure is used to define counter extensions
> +for Signals, Synapses, and Counts.
> +
> +The "type" member specifies the type of data (e.g. unsigned long,
> +boolean, etc.) handled by this extension. The "read" and "write" members
> +can then be set by the counter device driver with callbacks to handle
> +that data.
> +
> +Convenience macros such as `COUNTER_DATA_COUNT_U64` are provided for use
> +by driver authors. In particular, driver authors are expected to use
> +the provided macros for standard Counter subsystem attributes in order
> +to maintain a consistent interface for userspace. For example, a counter
> +device driver may define several standard attributes like so::
> +
> + struct counter_data count_ext[] = {
> + COUNTER_DATA_DIRECTION(count_direction_read),
> + COUNTER_DATA_ENABLE(count_enable_read, count_enable_write),
> + COUNTER_DATA_CEILING(count_ceiling_read, count_ceiling_write),
> + };
> +
> +This makes it intuitive to see, add, and modify the attributes that are
> +supported by this driver ("direction", "enable", and "ceiling") and to
> +maintain this code without getting lost in a web of struct braces.
> +
> +Callbacks must match the function type expected for the respective
> +component or extension. These function types are defined in the struct
> +counter_data structure as the "`*_read`" and "`*_write`" union members.
> +
> +The corresponding callback prototypes for the extensions mentioned in
> +the previous example above would be::
> +
> + int count_direction_read(struct counter_device *counter,
> + struct counter_count *count, u8 *direction);
> + int count_enable_read(struct counter_device *counter,
> + struct counter_count *count, u8 *enable);
> + int count_enable_write(struct counter_device *counter,
> + struct counter_count *count, u8 enable);
> + int count_ceiling_read(struct counter_device *counter,
> + struct counter_count *count, u64 *ceiling);
> + int count_ceiling_write(struct counter_device *counter,
> + struct counter_count *count, u64 ceiling);
>
> Determining the type of extension to create is a matter of scope.
>
> @@ -313,52 +346,102 @@ Determining the type of extension to create is a matter of scope.
> chip overheated via a device extension called "error_overtemp":
> /sys/bus/counter/devices/counterX/error_overtemp
>
> -Architecture
> -============
> -
> -When the Generic Counter interface counter module is loaded, the
> -counter_init function is called which registers a bus_type named
> -"counter" to the system. Subsequently, when the module is unloaded, the
> -counter_exit function is called which unregisters the bus_type named
> -"counter" from the system.
> -
> -Counter devices are registered to the system via the counter_register
> -function, and later removed via the counter_unregister function. The
> -counter_register function establishes a unique ID for the Counter
> -device and creates a respective sysfs directory, where X is the
> -mentioned unique ID:
> -
> - /sys/bus/counter/devices/counterX
> -
> -Sysfs attributes are created within the counterX directory to expose
> -functionality, configurations, and data relating to the Counts, Signals,
> -and Synapses of the Counter device, as well as options and information
> -for the Counter device itself.
> -
> -Each Signal has a directory created to house its relevant sysfs
> -attributes, where Y is the unique ID of the respective Signal:
> -
> - /sys/bus/counter/devices/counterX/signalY
> -
> -Similarly, each Count has a directory created to house its relevant
> -sysfs attributes, where Y is the unique ID of the respective Count:
> -
> - /sys/bus/counter/devices/counterX/countY
> -
> -For a more detailed breakdown of the available Generic Counter interface
> -sysfs attributes, please refer to the
> -Documentation/ABI/testing/sysfs-bus-counter file.
> -
> -The Signals and Counts associated with the Counter device are registered
> -to the system as well by the counter_register function. The
> -signal_read/signal_write driver callbacks are associated with their
> -respective Signal attributes, while the count_read/count_write and
> -function_get/function_set driver callbacks are associated with their
> -respective Count attributes; similarly, the same is true for the
> -action_get/action_set driver callbacks and their respective Synapse
> -attributes. If a driver callback is left undefined, then the respective
> -read/write permission is left disabled for the relevant attributes.
> -
> -Similarly, extension sysfs attributes are created for the defined
> -counter_device_ext, counter_count_ext, and counter_signal_ext
> -structures that are passed in.
> +Subsystem Architecture
> +======================
> +
> +Counter drivers pass and take data natively (i.e. `u8`, `u64`, `char *`,
> +etc.) and the shared counter module handles the translation between the
> +sysfs interface. This gurantees a standard userspace interface for all
> +counter drivers, and helps generalize the Generic Counter driver ABI in
> +order to support the Generic Counter chrdev interface without
> +significant changes to the existing counter drivers.
> +
> +A high-level view of how a count value is passed down from a counter
> +driver can be exemplified by the following::
> +
> + Count data request:
> + ~~~~~~~~~~~~~~~~~~~
> + ----------------------
> + / Counter device \
> + +----------------------+
> + | Count register: 0x28 |
> + +----------------------+
> + |
> + -----------------
> + / raw count data /
> + -----------------
> + |
> + V
> + +----------------------------+
> + | Counter device driver |----------+
> + +----------------------------+ |
> + | Processes data from device | -------------------
> + |----------------------------| / driver callbacks /
> + | Type: unsigned long | -------------------
> + | Value: 42 | |
> + +----------------------------+ |
> + | |
> + ---------------- |
> + / unsigned long / |
> + ---------------- |
> + | |
> + | V
> + | +----------------------+
> + | | Counter core |
> + | +----------------------+
> + | | Routes device driver |
> + | | callbacks to the |
> + | | userspace interfaces |
> + | +----------------------+
> + | |
> + | -------------------
> + | / driver callbacks /
> + | -------------------
> + | |
> + +-------+ |
> + | |
> + | +---------------+
> + | |
> + V |
> + +--------------------+ |
> + | Counter sysfs |<-+
> + +--------------------+
> + | Translates to the |
> + | standard Counter |
> + | sysfs output |
> + |--------------------|
> + | Type: const char * |
> + | Value: "42" |
> + +--------------------+
> + |
> + ---------------
> + / const char * /
> + ---------------
> + |
> + V
> + +--------------------------------------------------+
> + | `/sys/bus/counter/devices/counterX/countY/count` |
> + +--------------------------------------------------+
> + \ Count: "42" /
> + --------------------------------------------------
> +
> +There are three primary components involved:
> +
> +Counter device driver
> +---------------------
> +Communicates with the hardware device to read/write data; e.g. counter
> +drivers for quadrature encoders, timers, etc.
> +
> +Counter core
> +------------
> +Registers the counter device driver to the system so that the respective
> +callbacks are called during userspace interaction.
> +
> +Counter sysfs
> +-------------
> +Translates counter data to the standard Counter sysfs interface format
> +and vice versa.
> +
> +Please refer to the `Documentation/ABI/testing/sysfs-bus-counter` file
> +for a detailed breakdown of the available Generic Counter interface
> +sysfs attributes.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/2] arm64: dts: rockchip: rename label and nodename pinctrl subnodes that end with gpio
From: Johan Jonker @ 2020-05-24 16:06 UTC (permalink / raw)
To: heiko; +Cc: devicetree, robh+dt, linux-kernel, linux-arm-kernel,
linux-rockchip
In-Reply-To: <20200524160636.16547-1-jbx6244@gmail.com>
A test with the command below gives for example this error:
arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dt.yaml:
tsadc: tsadc-otp-gpio:
{'phandle': [[90]], 'rockchip,pins': [[0, 6, 0, 123]]}
is not of type 'array'
'gpio' is a sort of reserved nodename and should not be used
for pinctrl in combination with 'rockchip,pins', so change
nodes that end with 'gpio' to end with 'pin' or 'pins'.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/
dtschema/schemas/gpio/gpio.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
arch/arm64/boot/dts/rockchip/px30.dtsi | 6 +-
arch/arm64/boot/dts/rockchip/rk3308.dtsi | 6 +-
arch/arm64/boot/dts/rockchip/rk3328-evb.dts | 2 +-
arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts | 2 +-
arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 2 +-
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 24 +++----
.../arm64/boot/dts/rockchip/rk3368-lion-haikou.dts | 2 +-
arch/arm64/boot/dts/rockchip/rk3368.dtsi | 6 +-
arch/arm64/boot/dts/rockchip/rk3399-firefly.dts | 4 +-
.../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 2 +-
arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 4 +-
arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts | 8 +--
arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts | 8 +--
.../boot/dts/rockchip/rk3399-pinebook-pro.dts | 74 +++++++++++-----------
arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 8 +--
arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts | 8 +--
arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi | 4 +-
arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 8 +--
arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi | 4 +-
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 +-
20 files changed, 94 insertions(+), 94 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index f8ad67194..2d4728cf8 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -733,9 +733,9 @@
rockchip,grf = <&grf>;
rockchip,hw-tshut-temp = <120000>;
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&tsadc_otp_gpio>;
+ pinctrl-0 = <&tsadc_otp_pin>;
pinctrl-1 = <&tsadc_otp_out>;
- pinctrl-2 = <&tsadc_otp_gpio>;
+ pinctrl-2 = <&tsadc_otp_pin>;
#thermal-sensor-cells = <1>;
status = "disabled";
};
@@ -1371,7 +1371,7 @@
};
tsadc {
- tsadc_otp_gpio: tsadc-otp-gpio {
+ tsadc_otp_pin: tsadc-otp-pin {
rockchip,pins =
<0 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3308.dtsi b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
index 79c1dd1fe..ae40bfa16 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
@@ -1629,7 +1629,7 @@
};
tsadc {
- tsadc_otp_gpio: tsadc-otp-gpio {
+ tsadc_otp_pin: tsadc-otp-pin {
rockchip,pins =
<0 RK_PB2 0 &pcfg_pull_none>;
};
@@ -1657,7 +1657,7 @@
<2 RK_PA3 1 &pcfg_pull_none>;
};
- uart0_rts_gpio: uart0-rts-gpio {
+ uart0_rts_pin: uart0-rts-pin {
rockchip,pins =
<2 RK_PA3 0 &pcfg_pull_none>;
};
@@ -1730,7 +1730,7 @@
<4 RK_PA7 1 &pcfg_pull_none>;
};
- uart4_rts_gpio: uart4-rts-gpio {
+ uart4_rts_pin: uart4-rts-pin {
rockchip,pins =
<4 RK_PA7 0 &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
index ac29c2744..1969dab84 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
@@ -41,7 +41,7 @@
compatible = "regulator-fixed";
gpio = <&gpio0 30 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc0m1_gpio>;
+ pinctrl-0 = <&sdmmc0m1_pin>;
regulator-name = "vcc_sd";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
index 34db48c27..b70ffb1c6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
@@ -34,7 +34,7 @@
compatible = "regulator-fixed";
gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc0m1_gpio>;
+ pinctrl-0 = <&sdmmc0m1_pin>;
regulator-boot-on;
regulator-name = "vcc_sd";
regulator-min-microvolt = <3300000>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 6e09c223e..86cfb5c50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -25,7 +25,7 @@
compatible = "regulator-fixed";
gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc0m1_gpio>;
+ pinctrl-0 = <&sdmmc0m1_pin>;
regulator-name = "vcc_sd";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index fdf0e1b26..2b7e9d713 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -552,9 +552,9 @@
clocks = <&cru SCLK_TSADC>, <&cru PCLK_TSADC>;
clock-names = "tsadc", "apb_pclk";
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&otp_gpio>;
+ pinctrl-0 = <&otp_pin>;
pinctrl-1 = <&otp_out>;
- pinctrl-2 = <&otp_gpio>;
+ pinctrl-2 = <&otp_pin>;
resets = <&cru SRST_TSADC>;
reset-names = "tsadc-apb";
rockchip,grf = <&grf>;
@@ -1155,7 +1155,7 @@
rockchip,pins = <0 RK_PA5 2 &pcfg_pull_none>,
<0 RK_PA6 2 &pcfg_pull_none>;
};
- i2c3_gpio: i2c3-gpio {
+ i2c3_pins: i2c3-pins {
rockchip,pins =
<0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>,
<0 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
@@ -1226,7 +1226,7 @@
};
tsadc {
- otp_gpio: otp-gpio {
+ otp_pin: otp-pin {
rockchip,pins = <2 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>;
};
@@ -1249,7 +1249,7 @@
rockchip,pins = <1 RK_PB2 1 &pcfg_pull_none>;
};
- uart0_rts_gpio: uart0-rts-gpio {
+ uart0_rts_pin: uart0-rts-pin {
rockchip,pins = <1 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
@@ -1268,7 +1268,7 @@
rockchip,pins = <3 RK_PA5 4 &pcfg_pull_none>;
};
- uart1_rts_gpio: uart1-rts-gpio {
+ uart1_rts_pin: uart1-rts-pin {
rockchip,pins = <3 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
@@ -1494,7 +1494,7 @@
rockchip,pins = <2 RK_PA7 1 &pcfg_pull_up_4ma>;
};
- sdmmc0m0_gpio: sdmmc0m0-gpio {
+ sdmmc0m0_pin: sdmmc0m0-pin {
rockchip,pins = <2 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up_4ma>;
};
};
@@ -1504,7 +1504,7 @@
rockchip,pins = <0 RK_PD6 3 &pcfg_pull_up_4ma>;
};
- sdmmc0m1_gpio: sdmmc0m1-gpio {
+ sdmmc0m1_pin: sdmmc0m1-pin {
rockchip,pins = <0 RK_PD6 RK_FUNC_GPIO &pcfg_pull_up_4ma>;
};
};
@@ -1537,7 +1537,7 @@
<1 RK_PA3 1 &pcfg_pull_up_8ma>;
};
- sdmmc0_gpio: sdmmc0-gpio {
+ sdmmc0_pins: sdmmc0-pins {
rockchip,pins =
<1 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up_4ma>,
<1 RK_PA4 RK_FUNC_GPIO &pcfg_pull_up_4ma>,
@@ -1579,7 +1579,7 @@
<3 RK_PA7 3 &pcfg_pull_up_4ma>;
};
- sdmmc0ext_gpio: sdmmc0ext-gpio {
+ sdmmc0ext_pins: sdmmc0ext-pins {
rockchip,pins =
<3 RK_PA0 RK_FUNC_GPIO &pcfg_pull_up_4ma>,
<3 RK_PA1 RK_FUNC_GPIO &pcfg_pull_up_4ma>,
@@ -1624,7 +1624,7 @@
<1 RK_PC1 1 &pcfg_pull_up_8ma>;
};
- sdmmc1_gpio: sdmmc1-gpio {
+ sdmmc1_pins: sdmmc1-pins {
rockchip,pins =
<1 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up_4ma>,
<1 RK_PB5 RK_FUNC_GPIO &pcfg_pull_up_4ma>,
@@ -1818,7 +1818,7 @@
tsadc_int: tsadc-int {
rockchip,pins = <2 RK_PB5 2 &pcfg_pull_none>;
};
- tsadc_gpio: tsadc-gpio {
+ tsadc_pin: tsadc-pin {
rockchip,pins = <2 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts
index e8774347b..7fcb1eace 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3368-lion-haikou.dts
@@ -125,7 +125,7 @@
};
sdmmc {
- sdmmc_cd_gpio: sdmmc-cd-gpio {
+ sdmmc_cd_pin: sdmmc-cd-pin {
rockchip,pins =
<2 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 1ebb0eef4..5d25a9d04 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -483,9 +483,9 @@
resets = <&cru SRST_TSADC>;
reset-names = "tsadc-apb";
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&otp_gpio>;
+ pinctrl-0 = <&otp_pin>;
pinctrl-1 = <&otp_out>;
- pinctrl-2 = <&otp_gpio>;
+ pinctrl-2 = <&otp_pin>;
#thermal-sensor-cells = <1>;
rockchip,hw-tshut-temp = <95000>;
status = "disabled";
@@ -1145,7 +1145,7 @@
};
tsadc {
- otp_gpio: otp-gpio {
+ otp_pin: otp-pin {
rockchip,pins = <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
index 20b5599f5..6db18808b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
@@ -589,11 +589,11 @@
};
pmic {
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index 4373ed732..60cd1c18c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -499,7 +499,7 @@ camera: &i2c7 {
};
/* there is no external pull up, so need to set this pin pull up */
-&sdmmc_cd_gpio {
+&sdmmc_cd_pin {
rockchip,pins = <1 RK_PB3 RK_FUNC_GPIO &pcfg_pull_up>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 2f3997740..32dcaf210 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -516,7 +516,7 @@ ap_i2c_audio: &i2c8 {
* configured as SDMMC and not JTAG.
*/
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd &sdmmc_cd_gpio
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd &sdmmc_cd_pin
&sdmmc_bus4>;
bus-width = <4>;
@@ -767,7 +767,7 @@ ap_i2c_audio: &i2c8 {
};
/* This is where we actually hook up CD; has external pull */
- sdmmc_cd_gpio: sdmmc-cd-gpio {
+ sdmmc_cd_pin: sdmmc-cd-pin {
rockchip,pins = <4 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts b/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts
index bf87fa32d..341d074ed 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-hugsun-x99.dts
@@ -205,7 +205,7 @@
compatible = "silergy,syr827";
reg = <0x40>;
regulator-compatible = "fan53555-reg";
- pinctrl-0 = <&vsel1_gpio>;
+ pinctrl-0 = <&vsel1_pin>;
regulator-name = "vdd_cpu_b";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -223,7 +223,7 @@
compatible = "silergy,syr828";
reg = <0x41>;
regulator-compatible = "fan53555-reg";
- pinctrl-0 = <&vsel2_gpio>;
+ pinctrl-0 = <&vsel2_pin>;
regulator-name = "vdd_gpu";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -521,12 +521,12 @@
<1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins =
<1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins =
<1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
index 73be38a53..1fa80ac15 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
@@ -341,7 +341,7 @@
reg = <0x40>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel1_gpio>;
+ pinctrl-0 = <&vsel1_pin>;
regulator-name = "vdd_cpu_b";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -360,7 +360,7 @@
reg = <0x41>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel2_gpio>;
+ pinctrl-0 = <&vsel2_pin>;
regulator-name = "vdd_gpu";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -447,11 +447,11 @@
rockchip,pins = <1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts
index cb0245d22..32d2af547 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts
@@ -33,7 +33,7 @@
backlight = <&backlight>;
enable-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&panel_en_gpio>;
+ pinctrl-0 = <&panel_en_pin>;
power-supply = <&vcc3v3_panel>;
ports {
@@ -60,7 +60,7 @@
gpio-key-lid {
compatible = "gpio-keys";
pinctrl-names = "default";
- pinctrl-0 = <&lidbtn_gpio>;
+ pinctrl-0 = <&lidbtn_pin>;
lid {
debounce-interval = <20>;
@@ -76,7 +76,7 @@
gpio-key-power {
compatible = "gpio-keys";
pinctrl-names = "default";
- pinctrl-0 = <&pwrbtn_gpio>;
+ pinctrl-0 = <&pwrbtn_pin>;
power {
debounce-interval = <20>;
@@ -117,7 +117,7 @@
clocks = <&rk808 1>;
clock-names = "ext_clock";
pinctrl-names = "default";
- pinctrl-0 = <&wifi_enable_h_gpio>;
+ pinctrl-0 = <&wifi_enable_h_pin>;
post-power-on-delay-ms = <100>;
power-off-delay-us = <500000>;
@@ -129,7 +129,7 @@
es8316-sound {
compatible = "simple-audio-card";
pinctrl-names = "default";
- pinctrl-0 = <&hp_det_gpio>;
+ pinctrl-0 = <&hp_det_pin>;
simple-audio-card,name = "rockchip,es8316-codec";
simple-audio-card,format = "i2s";
simple-audio-card,mclk-fs = <256>;
@@ -213,7 +213,7 @@
enable-active-high;
gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&pwr_5v_gpio>;
+ pinctrl-0 = <&pwr_5v_pin>;
regulator-name = "vcc5v0_usb";
regulator-always-on;
regulator-min-microvolt = <5000000>;
@@ -270,7 +270,7 @@
enable-active-high;
gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc0_pwr_h_gpio>;
+ pinctrl-0 = <&sdmmc0_pwr_h_pin>;
regulator-name = "vcc3v0_sd";
regulator-always-on;
regulator-min-microvolt = <3000000>;
@@ -288,7 +288,7 @@
enable-active-high;
gpio = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&lcdvcc_en_gpio>;
+ pinctrl-0 = <&lcdvcc_en_pin>;
regulator-name = "vcc3v3_panel";
regulator-always-on;
regulator-min-microvolt = <3300000>;
@@ -317,7 +317,7 @@
enable-active-high;
gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&vcc5v0_host_en_gpio>;
+ pinctrl-0 = <&vcc5v0_host_en_pin>;
regulator-name = "vcc5v0_otg";
regulator-always-on;
regulator-min-microvolt = <5000000>;
@@ -336,7 +336,7 @@
enable-active-high;
gpio = <&gpio1 RK_PA3 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&vcc5v0_typec0_en_gpio>;
+ pinctrl-0 = <&vcc5v0_typec0_en_pin>;
regulator-name = "vbus_5vout";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
@@ -368,7 +368,7 @@
/* Also triggered by USB charger */
pinctrl-names = "default";
- pinctrl-0 = <&dc_det_gpio>;
+ pinctrl-0 = <&dc_det_pin>;
};
};
@@ -447,7 +447,7 @@
interrupt-parent = <&gpio3>;
interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
- pinctrl-0 = <&pmic_int_l_gpio>;
+ pinctrl-0 = <&pmic_int_l_pin>;
rockchip,system-power-controller;
wakeup-source;
@@ -627,7 +627,7 @@
reg = <0x40>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel1_gpio>;
+ pinctrl-0 = <&vsel1_pin>;
regulator-name = "vdd_cpu_b";
regulator-always-on;
regulator-boot-on;
@@ -646,7 +646,7 @@
reg = <0x41>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel2_gpio>;
+ pinctrl-0 = <&vsel2_pin>;
regulator-name = "vdd_gpu";
regulator-always-on;
regulator-boot-on;
@@ -693,7 +693,7 @@
interrupt-parent = <&gpio1>;
interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
- pinctrl-0 = <&fusb0_int_gpio>;
+ pinctrl-0 = <&fusb0_int_pin>;
vbus-supply = <&vbus_typec>;
connector {
@@ -745,7 +745,7 @@
&i2s1 {
pinctrl-names = "default";
- pinctrl-0 = <&i2s_8ch_mclk_gpio>, <&i2s1_2ch_bus>;
+ pinctrl-0 = <&i2s_8ch_mclk_pin>, <&i2s1_2ch_bus>;
rockchip,capture-channels = <8>;
rockchip,playback-channels = <8>;
status = "okay";
@@ -777,49 +777,49 @@
&pinctrl {
buttons {
- pwrbtn_gpio: pwrbtn-gpio {
+ pwrbtn_pin: pwrbtn-pin {
rockchip,pins = <0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>;
};
- lidbtn_gpio: lidbtn-gpio {
+ lidbtn_pin: lidbtn-pin {
rockchip,pins = <1 RK_PA1 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
dc-charger {
- dc_det_gpio: dc-det-gpio {
+ dc_det_pin: dc-det-pin {
rockchip,pins = <4 RK_PD0 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
es8316 {
- hp_det_gpio: hp-det-gpio {
+ hp_det_pin: hp-det-pin {
rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
fusb302x {
- fusb0_int_gpio: fusb0-int-gpio {
+ fusb0_int_pin: fusb0-int-pin {
rockchip,pins = <1 RK_PA2 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
i2s1 {
- i2s_8ch_mclk_gpio: i2s-8ch-mclk-gpio {
+ i2s_8ch_mclk_pin: i2s-8ch-mclk-pin {
rockchip,pins = <4 RK_PA0 1 &pcfg_pull_none>;
};
};
lcd-panel {
- lcdvcc_en_gpio: lcdvcc-en-gpio {
+ lcdvcc_en_pin: lcdvcc-en-pin {
rockchip,pins = <1 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
};
- panel_en_gpio: panel-en-gpio {
+ panel_en_pin: panel-en-pin {
rockchip,pins = <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
};
- lcd_panel_reset_gpio: lcd-panel-reset-gpio {
+ lcd_panel_reset_pin: lcd-panel-reset-pin {
rockchip,pins = <4 RK_PD6 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
@@ -835,58 +835,58 @@
};
pmic {
- pmic_int_l_gpio: pmic-int-l-gpio {
+ pmic_int_l_pin: pmic-int-l-pin {
rockchip,pins = <3 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
sdcard {
- sdmmc0_pwr_h_gpio: sdmmc0-pwr-h-gpio {
+ sdmmc0_pwr_h_pin: sdmmc0-pwr-h-pin {
rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
sdio-pwrseq {
- wifi_enable_h_gpio: wifi-enable-h-gpio {
+ wifi_enable_h_pin: wifi-enable-h-pin {
rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
usb-typec {
- vcc5v0_typec0_en_gpio: vcc5v0-typec0-en-gpio {
+ vcc5v0_typec0_en_pin: vcc5v0-typec0-en-pin {
rockchip,pins = <1 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
usb2 {
- pwr_5v_gpio: pwr-5v-gpio {
+ pwr_5v_pin: pwr-5v-pin {
rockchip,pins = <1 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>;
};
- vcc5v0_host_en_gpio: vcc5v0-host-en-gpio {
+ vcc5v0_host_en_pin: vcc5v0-host-en-pin {
rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
wireless-bluetooth {
- bt_wake_gpio: bt-wake-gpio {
+ bt_wake_pin: bt-wake-pin {
rockchip,pins = <2 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>;
};
- bt_host_wake_gpio: bt-host-wake-gpio {
+ bt_host_wake_pin: bt-host-wake-pin {
rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
};
- bt_reset_gpio: bt-reset-gpio {
+ bt_reset_pin: bt-reset-pin {
rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
@@ -1034,7 +1034,7 @@
host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
max-speed = <1500000>;
pinctrl-names = "default";
- pinctrl-0 = <&bt_host_wake_gpio &bt_wake_gpio &bt_reset_gpio>;
+ pinctrl-0 = <&bt_host_wake_pin &bt_wake_pin &bt_reset_pin>;
shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
vbat-supply = <&wifi_bat>;
vddio-supply = <&vcc_wl>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
index cec70f2bf..b85ec31cd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
@@ -456,7 +456,7 @@
reg = <0x40>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel1_gpio>;
+ pinctrl-0 = <&vsel1_pin>;
regulator-name = "vdd_cpu_b";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -475,7 +475,7 @@
reg = <0x41>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel2_gpio>;
+ pinctrl-0 = <&vsel2_pin>;
regulator-name = "vdd_gpu";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -609,11 +609,11 @@
};
pmic {
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts
index 3923ec01e..60f98a3e1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dts
@@ -390,7 +390,7 @@
reg = <0x40>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel1_gpio>;
+ pinctrl-0 = <&vsel1_pin>;
regulator-name = "vdd_cpu_b";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -409,7 +409,7 @@
reg = <0x41>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel2_gpio>;
+ pinctrl-0 = <&vsel2_pin>;
regulator-name = "vdd_gpu";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -532,11 +532,11 @@
rockchip,pins = <1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
index ba7c75c9f..5e3ac589b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi
@@ -470,12 +470,12 @@
<1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins =
<1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins =
<1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index c39d0f411..96c787aa2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -445,7 +445,7 @@
reg = <0x40>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel1_gpio>;
+ pinctrl-0 = <&vsel1_pin>;
regulator-name = "vdd_cpu_b";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -464,7 +464,7 @@
reg = <0x41>;
fcs,suspend-voltage-selector = <1>;
pinctrl-names = "default";
- pinctrl-0 = <&vsel2_gpio>;
+ pinctrl-0 = <&vsel2_pin>;
regulator-name = "vdd_gpu";
regulator-min-microvolt = <712500>;
regulator-max-microvolt = <1500000>;
@@ -612,11 +612,11 @@
rockchip,pins = <3 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi
index 1bc157967..701a567d7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi
@@ -481,11 +481,11 @@
<1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
};
- vsel1_gpio: vsel1-gpio {
+ vsel1_pin: vsel1-pin {
rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
};
- vsel2_gpio: vsel2-gpio {
+ vsel2_pin: vsel2-pin {
rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
};
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index a4dc1bf2e..4db355f9d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -845,9 +845,9 @@
rockchip,grf = <&grf>;
rockchip,hw-tshut-temp = <95000>;
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&otp_gpio>;
+ pinctrl-0 = <&otp_pin>;
pinctrl-1 = <&otp_out>;
- pinctrl-2 = <&otp_gpio>;
+ pinctrl-2 = <&otp_pin>;
#thermal-sensor-cells = <1>;
status = "disabled";
};
@@ -2475,7 +2475,7 @@
};
tsadc {
- otp_gpio: otp-gpio {
+ otp_pin: otp-pin {
rockchip,pins = <1 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
};
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/2] ARM: dts: rockchip: rename label and nodename pinctrl subnodes that end with gpio
From: Johan Jonker @ 2020-05-24 16:06 UTC (permalink / raw)
To: heiko; +Cc: devicetree, robh+dt, linux-kernel, linux-arm-kernel,
linux-rockchip
A test with the command below gives for example this error:
arch/arm/boot/dts/rk3288-tinker.dt.yaml: tsadc: otp-gpio:
{'phandle': [[54]], 'rockchip,pins': [[0, 10, 0, 118]]}
is not of type 'array'
'gpio' is a sort of reserved nodename and should not be used
for pinctrl in combination with 'rockchip,pins', so change
nodes that end with 'gpio' to end with 'pin' or 'pins'.
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/
dtschema/schemas/gpio/gpio.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
arch/arm/boot/dts/rk322x.dtsi | 6 +++---
arch/arm/boot/dts/rk3288-veyron-jaq.dts | 2 +-
arch/arm/boot/dts/rk3288-veyron-jerry.dts | 2 +-
arch/arm/boot/dts/rk3288-veyron-mighty.dts | 6 +++---
arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 +-
arch/arm/boot/dts/rk3288-veyron-pinky.dts | 6 +++---
arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi | 2 +-
arch/arm/boot/dts/rk3288-veyron-speedy.dts | 2 +-
arch/arm/boot/dts/rk3288.dtsi | 6 +++---
arch/arm/boot/dts/rv1108.dtsi | 12 ++++++------
10 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 7a1a69183..3590ce15a 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -522,9 +522,9 @@
resets = <&cru SRST_TSADC>;
reset-names = "tsadc-apb";
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&otp_gpio>;
+ pinctrl-0 = <&otp_pin>;
pinctrl-1 = <&otp_out>;
- pinctrl-2 = <&otp_gpio>;
+ pinctrl-2 = <&otp_pin>;
#thermal-sensor-cells = <0>;
rockchip,hw-tshut-temp = <95000>;
status = "disabled";
@@ -1103,7 +1103,7 @@
};
tsadc {
- otp_gpio: otp-gpio {
+ otp_pin: otp-pin {
rockchip,pins = <0 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
index 171ba6185..8efba9dea 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
@@ -47,7 +47,7 @@
&sdmmc {
disable-wp;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_pin
&sdmmc_bus4>;
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
index 66f00d288..2c916c50d 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
@@ -192,7 +192,7 @@
&sdmmc {
disable-wp;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_pin
&sdmmc_bus4>;
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-mighty.dts b/arch/arm/boot/dts/rk3288-veyron-mighty.dts
index 27fbc0747..fa695a88f 100644
--- a/arch/arm/boot/dts/rk3288-veyron-mighty.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-mighty.dts
@@ -18,8 +18,8 @@
};
&sdmmc {
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
- &sdmmc_wp_gpio &sdmmc_bus4>;
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_pin
+ &sdmmc_wp_pin &sdmmc_bus4>;
wp-gpios = <&gpio7 10 GPIO_ACTIVE_HIGH>;
/delete-property/ disable-wp;
@@ -27,7 +27,7 @@
&pinctrl {
sdmmc {
- sdmmc_wp_gpio: sdmmc-wp-gpio {
+ sdmmc_wp_pin: sdmmc-wp-pin {
rockchip,pins = <7 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index 383fad1a8..f8b69e0a1 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -114,7 +114,7 @@
&sdmmc {
disable-wp;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_pin
&sdmmc_bus4>;
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
index 71e6629cc..4e9fdb0f7 100644
--- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
@@ -105,7 +105,7 @@
};
sdmmc {
- sdmmc_wp_gpio: sdmmc-wp-gpio {
+ sdmmc_wp_pin: sdmmc-wp-pin {
rockchip,pins = <7 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>;
};
};
@@ -126,8 +126,8 @@
&sdmmc {
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
- &sdmmc_wp_gpio &sdmmc_bus4>;
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_pin
+ &sdmmc_wp_pin &sdmmc_bus4>;
wp-gpios = <&gpio7 RK_PB2 GPIO_ACTIVE_HIGH>;
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi b/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
index fe950f986..27fb06ce9 100644
--- a/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-sdmmc.dtsi
@@ -41,7 +41,7 @@
};
/* This is where we actually hook up CD */
- sdmmc_cd_gpio: sdmmc-cd-gpio {
+ sdmmc_cd_pin: sdmmc-cd-pin {
rockchip,pins = <7 RK_PA5 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
index e354c61a4..4a3ea934d 100644
--- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
@@ -54,7 +54,7 @@
&sdmmc {
disable-wp;
pinctrl-names = "default";
- pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_pin
&sdmmc_bus4>;
};
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 0cd88774d..d052a4758 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -575,9 +575,9 @@
resets = <&cru SRST_TSADC>;
reset-names = "tsadc-apb";
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&otp_gpio>;
+ pinctrl-0 = <&otp_pin>;
pinctrl-1 = <&otp_out>;
- pinctrl-2 = <&otp_gpio>;
+ pinctrl-2 = <&otp_pin>;
#thermal-sensor-cells = <1>;
rockchip,grf = <&grf>;
rockchip,hw-tshut-temp = <95000>;
@@ -1930,7 +1930,7 @@
};
tsadc {
- otp_gpio: otp-gpio {
+ otp_pin: otp-pin {
rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
};
diff --git a/arch/arm/boot/dts/rv1108.dtsi b/arch/arm/boot/dts/rv1108.dtsi
index b453f8d0f..f3ff54753 100644
--- a/arch/arm/boot/dts/rv1108.dtsi
+++ b/arch/arm/boot/dts/rv1108.dtsi
@@ -352,9 +352,9 @@
clocks = <&cru SCLK_TSADC>, <&cru PCLK_TSADC>;
clock-names = "tsadc", "apb_pclk";
pinctrl-names = "init", "default", "sleep";
- pinctrl-0 = <&otp_gpio>;
+ pinctrl-0 = <&otp_pin>;
pinctrl-1 = <&otp_out>;
- pinctrl-2 = <&otp_gpio>;
+ pinctrl-2 = <&otp_pin>;
resets = <&cru SRST_TSADC>;
reset-names = "tsadc-apb";
rockchip,hw-tshut-temp = <120000>;
@@ -729,7 +729,7 @@
<0 RK_PC6 3 &pcfg_pull_none>;
};
- i2c2m1_gpio: i2c2m1-gpio {
+ i2c2m1_pins: i2c2m1-pins {
rockchip,pins = <0 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>,
<0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
};
@@ -741,7 +741,7 @@
<1 RK_PD4 2 &pcfg_pull_none>;
};
- i2c2m05v_gpio: i2c2m05v-gpio {
+ i2c2m05v_pins: i2c2m05v-pins {
rockchip,pins = <1 RK_PD5 RK_FUNC_GPIO &pcfg_pull_none>,
<1 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
};
@@ -868,7 +868,7 @@
rockchip,pins = <0 RK_PB7 1 &pcfg_pull_none>;
};
- otp_gpio: otp-gpio {
+ otp_pin: otp-pin {
rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
@@ -887,7 +887,7 @@
rockchip,pins = <3 RK_PA3 1 &pcfg_pull_none>;
};
- uart0_rts_gpio: uart0-rts-gpio {
+ uart0_rts_pin: uart0-rts-pin {
rockchip,pins = <3 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>;
};
};
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5 5/5] counter: Add microchip TCB capture counter
From: William Breathitt Gray @ 2020-05-24 15:35 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Mark Rutland, Kamel Bouhara, Alexandre Belloni, devicetree,
linux-iio, Ludovic Desroches, Rob Herring, Thomas Petazzoni,
linux-input, linux-arm-kernel
In-Reply-To: <20200524161624.2c932a30@archlinux>
[-- Attachment #1.1: Type: text/plain, Size: 1396 bytes --]
On Sun, May 24, 2020 at 04:16:24PM +0100, Jonathan Cameron wrote:
> On Tue, 19 May 2020 10:37:16 +0200
> Kamel Bouhara <kamel.bouhara@bootlin.com> wrote:
>
> > This drivers allows to use the capture mode of the Timer Counter Block
> > hardware block available in Microchip SoCs through the counter subsystem.
> >
> > Two functions of the counter are supported for the moment: period
> > capture and quadrature decoder. The latter is only supported by the
> > SAMA5 series of SoCs.
> >
> > For the period capture mode a basic setup has been chosen that will
> > reset the counter each time the period is actually reached. Of course
> > the device offers much more possibilities.
> >
> > For quadrature mode, both channel 0 and 1 must be configured even if we
> > only capture the position (no revolution/rotation).
> >
> > Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
>
> Looks good to me. After reviews from William, DT and maybe
> something from SoC side if intent is I'm taking this.
>
> Jonathan
I'll defer to the DT maintainers for the dt-binding changes. Regarding
this counter driver, I believe there have been no changes since I
signed-off for it in the v3 patchset thread. If that's true, I haven't
changed my mind -- I'm still statisfied with this particular version and
have no objections if it's merged. ;-)
William Breathitt Gray
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [arm-platforms:pci/xilinx 3/3] checkpatch: ERROR: space required before the open parenthesis '('
From: kbuild test robot @ 2020-05-24 15:17 UTC (permalink / raw)
To: Marc Zyngier; +Cc: kbuild-all, linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git pci/xilinx
head: 8b5d158374e59edf26e61c512eeb00ca7c9d891d
commit: 8b5d158374e59edf26e61c512eeb00ca7c9d891d [3/3] PCI: xilinx-cpm: Revamp irq handling
reproduce:
scripts/checkpatch.pl 0001-PCI-xilinx-cpm-Revamp-irq-handling.patch
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
# many are suggestions rather than must-fix
ERROR: space required before the open parenthesis '('
#456: drivers/pci/controller/pcie-xilinx-cpm.c:342:
+ switch(d->hwirq) {
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 5/5] counter: Add microchip TCB capture counter
From: Jonathan Cameron @ 2020-05-24 15:16 UTC (permalink / raw)
To: Kamel Bouhara
Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-iio,
William Breathitt Gray, Ludovic Desroches, Rob Herring,
Thomas Petazzoni, linux-input, linux-arm-kernel
In-Reply-To: <20200519083716.938384-6-kamel.bouhara@bootlin.com>
On Tue, 19 May 2020 10:37:16 +0200
Kamel Bouhara <kamel.bouhara@bootlin.com> wrote:
> This drivers allows to use the capture mode of the Timer Counter Block
> hardware block available in Microchip SoCs through the counter subsystem.
>
> Two functions of the counter are supported for the moment: period
> capture and quadrature decoder. The latter is only supported by the
> SAMA5 series of SoCs.
>
> For the period capture mode a basic setup has been chosen that will
> reset the counter each time the period is actually reached. Of course
> the device offers much more possibilities.
>
> For quadrature mode, both channel 0 and 1 must be configured even if we
> only capture the position (no revolution/rotation).
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Looks good to me. After reviews from William, DT and maybe
something from SoC side if intent is I'm taking this.
Jonathan
> ---
> drivers/counter/Kconfig | 11 +
> drivers/counter/Makefile | 1 +
> drivers/counter/microchip-tcb-capture.c | 397 ++++++++++++++++++++++++
> 3 files changed, 409 insertions(+)
> create mode 100644 drivers/counter/microchip-tcb-capture.c
>
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index c80fa76bb531..2de53ab0dd25 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -70,4 +70,15 @@ config FTM_QUADDEC
> To compile this driver as a module, choose M here: the
> module will be called ftm-quaddec.
>
> +config MICROCHIP_TCB_CAPTURE
> + tristate "Microchip Timer Counter Capture driver"
> + depends on HAS_IOMEM && OF
> + select REGMAP_MMIO
> + help
> + Select this option to enable the Microchip Timer Counter Block
> + capture driver.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called microchip-tcb-capture.
> +
> endif # COUNTER
> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> index 55142d1f4c43..0a393f71e481 100644
> --- a/drivers/counter/Makefile
> +++ b/drivers/counter/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o
> obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
> obj-$(CONFIG_TI_EQEP) += ti-eqep.o
> obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o
> +obj-$(CONFIG_MICROCHIP_TCB_CAPTURE) += microchip-tcb-capture.o
> diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c
> new file mode 100644
> index 000000000000..f7b7743ddb94
> --- /dev/null
> +++ b/drivers/counter/microchip-tcb-capture.c
> @@ -0,0 +1,397 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/**
> + * Copyright (C) 2020 Microchip
> + *
> + * Author: Kamel Bouhara <kamel.bouhara@bootlin.com>
> + */
> +#include <linux/clk.h>
> +#include <linux/counter.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <soc/at91/atmel_tcb.h>
> +
> +#define ATMEL_TC_CMR_MASK (ATMEL_TC_LDRA_RISING | ATMEL_TC_LDRB_FALLING | \
> + ATMEL_TC_ETRGEDG_RISING | ATMEL_TC_LDBDIS | \
> + ATMEL_TC_LDBSTOP)
> +
> +#define ATMEL_TC_QDEN BIT(8)
> +#define ATMEL_TC_POSEN BIT(9)
> +
> +struct mchp_tc_data {
> + const struct atmel_tcb_config *tc_cfg;
> + struct counter_device counter;
> + struct regmap *regmap;
> + int qdec_mode;
> + int num_channels;
> + int channel[2];
> + bool trig_inverted;
> +};
> +
> +enum mchp_tc_count_function {
> + MCHP_TC_FUNCTION_INCREASE,
> + MCHP_TC_FUNCTION_QUADRATURE,
> +};
> +
> +static enum counter_count_function mchp_tc_count_functions[] = {
> + [MCHP_TC_FUNCTION_INCREASE] = COUNTER_COUNT_FUNCTION_INCREASE,
> + [MCHP_TC_FUNCTION_QUADRATURE] = COUNTER_COUNT_FUNCTION_QUADRATURE_X4,
> +};
> +
> +enum mchp_tc_synapse_action {
> + MCHP_TC_SYNAPSE_ACTION_NONE = 0,
> + MCHP_TC_SYNAPSE_ACTION_RISING_EDGE,
> + MCHP_TC_SYNAPSE_ACTION_FALLING_EDGE,
> + MCHP_TC_SYNAPSE_ACTION_BOTH_EDGE
> +};
> +
> +static enum counter_synapse_action mchp_tc_synapse_actions[] = {
> + [MCHP_TC_SYNAPSE_ACTION_NONE] = COUNTER_SYNAPSE_ACTION_NONE,
> + [MCHP_TC_SYNAPSE_ACTION_RISING_EDGE] = COUNTER_SYNAPSE_ACTION_RISING_EDGE,
> + [MCHP_TC_SYNAPSE_ACTION_FALLING_EDGE] = COUNTER_SYNAPSE_ACTION_FALLING_EDGE,
> + [MCHP_TC_SYNAPSE_ACTION_BOTH_EDGE] = COUNTER_SYNAPSE_ACTION_BOTH_EDGES,
> +};
> +
> +static struct counter_signal mchp_tc_count_signals[] = {
> + {
> + .id = 0,
> + .name = "Channel A",
> + },
> + {
> + .id = 1,
> + .name = "Channel B",
> + }
> +};
> +
> +static struct counter_synapse mchp_tc_count_synapses[] = {
> + {
> + .actions_list = mchp_tc_synapse_actions,
> + .num_actions = ARRAY_SIZE(mchp_tc_synapse_actions),
> + .signal = &mchp_tc_count_signals[0]
> + },
> + {
> + .actions_list = mchp_tc_synapse_actions,
> + .num_actions = ARRAY_SIZE(mchp_tc_synapse_actions),
> + .signal = &mchp_tc_count_signals[1]
> + }
> +};
> +
> +static int mchp_tc_count_function_get(struct counter_device *counter,
> + struct counter_count *count,
> + size_t *function)
> +{
> + struct mchp_tc_data *const priv = counter->priv;
> +
> + if (priv->qdec_mode)
> + *function = MCHP_TC_FUNCTION_QUADRATURE;
> + else
> + *function = MCHP_TC_FUNCTION_INCREASE;
> +
> + return 0;
> +}
> +
> +static int mchp_tc_count_function_set(struct counter_device *counter,
> + struct counter_count *count,
> + size_t function)
> +{
> + struct mchp_tc_data *const priv = counter->priv;
> + u32 bmr, cmr;
> +
> + regmap_read(priv->regmap, ATMEL_TC_BMR, &bmr);
> + regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], CMR), &cmr);
> +
> + /* Set capture mode */
> + cmr &= ~ATMEL_TC_WAVE;
> +
> + switch (function) {
> + case MCHP_TC_FUNCTION_INCREASE:
> + priv->qdec_mode = 0;
> + /* Set highest rate based on whether soc has gclk or not */
> + bmr &= ~(ATMEL_TC_QDEN | ATMEL_TC_POSEN);
> + if (priv->tc_cfg->has_gclk)
> + cmr |= ATMEL_TC_TIMER_CLOCK2;
> + else
> + cmr |= ATMEL_TC_TIMER_CLOCK1;
> + /* Setup the period capture mode */
> + cmr |= ATMEL_TC_CMR_MASK;
> + cmr &= ~(ATMEL_TC_ABETRG | ATMEL_TC_XC0);
> + break;
> + case MCHP_TC_FUNCTION_QUADRATURE:
> + if (!priv->tc_cfg->has_qdec)
> + return -EINVAL;
> + /* In QDEC mode settings both channels 0 and 1 are required */
> + if (priv->num_channels < 2 || priv->channel[0] != 0 ||
> + priv->channel[1] != 1) {
> + pr_err("Invalid channels number or id for quadrature mode\n");
> + return -EINVAL;
> + }
> + priv->qdec_mode = 1;
> + bmr |= ATMEL_TC_QDEN | ATMEL_TC_POSEN;
> + cmr |= ATMEL_TC_ETRGEDG_RISING | ATMEL_TC_ABETRG | ATMEL_TC_XC0;
> + break;
> + }
> +
> + regmap_write(priv->regmap, ATMEL_TC_BMR, bmr);
> + regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], CMR), cmr);
> +
> + /* Enable clock and trigger counter */
> + regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], CCR),
> + ATMEL_TC_CLKEN | ATMEL_TC_SWTRG);
> +
> + if (priv->qdec_mode) {
> + regmap_write(priv->regmap,
> + ATMEL_TC_REG(priv->channel[1], CMR), cmr);
> + regmap_write(priv->regmap,
> + ATMEL_TC_REG(priv->channel[1], CCR),
> + ATMEL_TC_CLKEN | ATMEL_TC_SWTRG);
> + }
> +
> + return 0;
> +}
> +
> +static int mchp_tc_count_signal_read(struct counter_device *counter,
> + struct counter_signal *signal,
> + enum counter_signal_value *val)
> +{
> + struct mchp_tc_data *const priv = counter->priv;
> + bool sigstatus;
> + u32 sr;
> +
> + regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], SR), &sr);
> +
> + if (priv->trig_inverted)
> + sigstatus = (sr & ATMEL_TC_MTIOB);
> + else
> + sigstatus = (sr & ATMEL_TC_MTIOA);
> +
> + *val = sigstatus ? COUNTER_SIGNAL_HIGH : COUNTER_SIGNAL_LOW;
> +
> + return 0;
> +}
> +
> +static int mchp_tc_count_action_get(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_synapse *synapse,
> + size_t *action)
> +{
> + struct mchp_tc_data *const priv = counter->priv;
> + u32 cmr;
> +
> + regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], CMR), &cmr);
> +
> + *action = MCHP_TC_SYNAPSE_ACTION_NONE;
> +
> + if (cmr & ATMEL_TC_ETRGEDG_NONE)
> + *action = MCHP_TC_SYNAPSE_ACTION_NONE;
> + else if (cmr & ATMEL_TC_ETRGEDG_RISING)
> + *action = MCHP_TC_SYNAPSE_ACTION_RISING_EDGE;
> + else if (cmr & ATMEL_TC_ETRGEDG_FALLING)
> + *action = MCHP_TC_SYNAPSE_ACTION_FALLING_EDGE;
> + else if (cmr & ATMEL_TC_ETRGEDG_BOTH)
> + *action = MCHP_TC_SYNAPSE_ACTION_BOTH_EDGE;
> +
> + return 0;
> +}
> +
> +static int mchp_tc_count_action_set(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_synapse *synapse,
> + size_t action)
> +{
> + struct mchp_tc_data *const priv = counter->priv;
> + u32 edge = ATMEL_TC_ETRGEDG_NONE;
> +
> + /* QDEC mode is rising edge only */
> + if (priv->qdec_mode)
> + return -EINVAL;
> +
> + switch (action) {
> + case MCHP_TC_SYNAPSE_ACTION_NONE:
> + edge = ATMEL_TC_ETRGEDG_NONE;
> + break;
> + case MCHP_TC_SYNAPSE_ACTION_RISING_EDGE:
> + edge = ATMEL_TC_ETRGEDG_RISING;
> + break;
> + case MCHP_TC_SYNAPSE_ACTION_FALLING_EDGE:
> + edge = ATMEL_TC_ETRGEDG_FALLING;
> + break;
> + case MCHP_TC_SYNAPSE_ACTION_BOTH_EDGE:
> + edge = ATMEL_TC_ETRGEDG_BOTH;
> + break;
> + }
> +
> + return regmap_write_bits(priv->regmap,
> + ATMEL_TC_REG(priv->channel[0], CMR),
> + ATMEL_TC_ETRGEDG, edge);
> +}
> +
> +static int mchp_tc_count_read(struct counter_device *counter,
> + struct counter_count *count,
> + unsigned long *val)
> +{
> + struct mchp_tc_data *const priv = counter->priv;
> + u32 cnt;
> +
> + regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], CV), &cnt);
> + *val = cnt;
> +
> + return 0;
> +}
> +
> +static struct counter_count mchp_tc_counts[] = {
> + {
> + .id = 0,
> + .name = "Timer Counter",
> + .functions_list = mchp_tc_count_functions,
> + .num_functions = ARRAY_SIZE(mchp_tc_count_functions),
> + .synapses = mchp_tc_count_synapses,
> + .num_synapses = ARRAY_SIZE(mchp_tc_count_synapses),
> + },
> +};
> +
> +static struct counter_ops mchp_tc_ops = {
> + .signal_read = mchp_tc_count_signal_read,
> + .count_read = mchp_tc_count_read,
> + .function_get = mchp_tc_count_function_get,
> + .function_set = mchp_tc_count_function_set,
> + .action_get = mchp_tc_count_action_get,
> + .action_set = mchp_tc_count_action_set
> +};
> +
> +static const struct atmel_tcb_config tcb_rm9200_config = {
> + .counter_width = 16,
> +};
> +
> +static const struct atmel_tcb_config tcb_sam9x5_config = {
> + .counter_width = 32,
> +};
> +
> +static const struct atmel_tcb_config tcb_sama5d2_config = {
> + .counter_width = 32,
> + .has_gclk = true,
> + .has_qdec = true,
> +};
> +
> +static const struct atmel_tcb_config tcb_sama5d3_config = {
> + .counter_width = 32,
> + .has_qdec = true,
> +};
> +
> +static const struct of_device_id atmel_tc_of_match[] = {
> + { .compatible = "atmel,at91rm9200-tcb", .data = &tcb_rm9200_config, },
> + { .compatible = "atmel,at91sam9x5-tcb", .data = &tcb_sam9x5_config, },
> + { .compatible = "atmel,sama5d2-tcb", .data = &tcb_sama5d2_config, },
> + { .compatible = "atmel,sama5d3-tcb", .data = &tcb_sama5d3_config, },
> + { /* sentinel */ }
> +};
> +
> +static void mchp_tc_clk_remove(void *ptr)
> +{
> + clk_disable_unprepare((struct clk *)ptr);
> +}
> +
> +static int mchp_tc_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const struct atmel_tcb_config *tcb_config;
> + const struct of_device_id *match;
> + struct mchp_tc_data *priv;
> + char clk_name[7];
> + struct regmap *regmap;
> + struct clk *clk[3];
> + int channel;
> + int ret, i;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + match = of_match_node(atmel_tc_of_match, np->parent);
> + tcb_config = match->data;
> + if (!tcb_config) {
> + dev_err(&pdev->dev, "No matching parent node found\n");
> + return -ENODEV;
> + }
> +
> + regmap = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(priv->regmap))
> + return PTR_ERR(priv->regmap);
> +
> + /* max. channels number is 2 when in QDEC mode */
> + priv->num_channels = of_property_count_u32_elems(np, "reg");
> + if (priv->num_channels < 0) {
> + dev_err(&pdev->dev, "Invalid or missing channel\n");
> + return -EINVAL;
> + }
> +
> + /* Register channels and initialize clocks */
> + for (i = 0; i < priv->num_channels; i++) {
> + ret = of_property_read_u32_index(np, "reg", i, &channel);
> + if (ret < 0 || channel > 2)
> + return -ENODEV;
> +
> + priv->channel[i] = channel;
> +
> + snprintf(clk_name, sizeof(clk_name), "t%d_clk", channel);
> +
> + clk[i] = of_clk_get_by_name(np->parent, clk_name);
> + if (IS_ERR(clk[i])) {
> + /* Fallback to t0_clk */
> + clk[i] = of_clk_get_by_name(np->parent, "t0_clk");
> + if (IS_ERR(clk[i]))
> + return PTR_ERR(clk[i]);
> + }
> +
> + ret = clk_prepare_enable(clk[i]);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&pdev->dev,
> + mchp_tc_clk_remove,
> + clk[i]);
> + if (ret)
> + return ret;
> +
> + dev_dbg(&pdev->dev,
> + "Initialized capture mode on channel %d\n",
> + channel);
> + }
> +
> + priv->tc_cfg = tcb_config;
> + priv->regmap = regmap;
> + priv->counter.name = dev_name(&pdev->dev);
> + priv->counter.parent = &pdev->dev;
> + priv->counter.ops = &mchp_tc_ops;
> + priv->counter.num_counts = ARRAY_SIZE(mchp_tc_counts);
> + priv->counter.counts = mchp_tc_counts;
> + priv->counter.num_signals = ARRAY_SIZE(mchp_tc_count_signals);
> + priv->counter.signals = mchp_tc_count_signals;
> + priv->counter.priv = priv;
> +
> + return devm_counter_register(&pdev->dev, &priv->counter);
> +}
> +
> +static const struct of_device_id mchp_tc_dt_ids[] = {
> + { .compatible = "microchip,tcb-capture", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mchp_tc_dt_ids);
> +
> +static struct platform_driver mchp_tc_driver = {
> + .probe = mchp_tc_probe,
> + .driver = {
> + .name = "microchip-tcb-capture",
> + .of_match_table = mchp_tc_dt_ids,
> + },
> +};
> +module_platform_driver(mchp_tc_driver);
> +
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("Microchip TCB Capture driver");
> +MODULE_LICENSE("GPL v2");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] mt76: mt7615: add support for MT7611N
From: Alexander Schlichte @ 2020-05-24 14:58 UTC (permalink / raw)
To: DENG Qingfang, linux-wireless
Cc: ryder.lee, matthias.bgg, linux-mediatek, kvalo, royluo,
lorenzo.bianconi83, linux-arm-kernel, nbd
In-Reply-To: <20200524034110.22576-1-dqfext@gmail.com>
Just some random drive-by nitpicks, so take it with a grain of salt.
On 24.05.20 05:41, DENG Qingfang wrote:
> MT7611N is basically the same as MT7615N, except it only supports 5GHz
> It is used by some TP-Link and Mercury wireless routers
>
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> v1 -> v2:
> make is_mt7615(&dev->mt76) return true for mt7611
>
> drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c | 7 +++++++
> drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h | 7 ++++++-
> drivers/net/wireless/mediatek/mt76/mt7615/pci.c | 1 +
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c b/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c
> index 6a5ae047c63b..edac37e7847b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c
> @@ -111,6 +111,12 @@ mt7615_eeprom_parse_hw_band_cap(struct mt7615_dev *dev)
> return;
> }
>
> + if (is_mt7611(&dev->mt76)) {
> + /* 5GHz only */
> + dev->mt76.cap.has_5ghz = true;
> + return;
> + }
> +
> val = FIELD_GET(MT_EE_NIC_WIFI_CONF_BAND_SEL,
> eeprom[MT_EE_WIFI_CONF]);
> switch (val) {
> @@ -310,6 +316,7 @@ static void mt7615_cal_free_data(struct mt7615_dev *dev)
> mt7622_apply_cal_free_data(dev);
> break;
> case 0x7615:
> + case 0x7611:
> mt7615_apply_cal_free_data(dev);
> break;
> }
Maybe sort this alphabetically?
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h b/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
> index ebdfca64b079..4ce8b379a147 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
> @@ -411,7 +411,7 @@ static inline bool is_mt7622(struct mt76_dev *dev)
>
> static inline bool is_mt7615(struct mt76_dev *dev)
> {
> - return mt76_chip(dev) == 0x7615;
> + return mt76_chip(dev) == 0x7615 || mt76_chip(dev) == 0x7611;
> }
Now the function name is a bit confusing because you can't use it to
differentiate between 7611 and 7615. Would it be an option to give it a more
fitting name? Maybe `is_mt761x` or `is_mt7611_or_7615`?
Apart from that you might want to call `is_mt7611` here. That would avoid
duplicating the check and order these is_* functions alphabetically.
>
> static inline bool is_mt7663(struct mt76_dev *dev)
> @@ -419,6 +419,11 @@ static inline bool is_mt7663(struct mt76_dev *dev)
> return mt76_chip(dev) == 0x7663;
> }
>
> +static inline bool is_mt7611(struct mt76_dev *dev)
> +{
> + return mt76_chip(dev) == 0x7611;
> +}
> +
> static inline void mt7615_irq_enable(struct mt7615_dev *dev, u32 mask)
> {
> mt76_set_irq_mask(&dev->mt76, 0, 0, mask);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/pci.c b/drivers/net/wireless/mediatek/mt76/mt7615/pci.c
> index 88ff14564521..b09d08d0dac9 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/pci.c
> @@ -14,6 +14,7 @@
> static const struct pci_device_id mt7615_pci_device_table[] = {
> { PCI_DEVICE(0x14c3, 0x7615) },
> { PCI_DEVICE(0x14c3, 0x7663) },
> + { PCI_DEVICE(0x14c3, 0x7611) },
> { },
> };
Maybe keep this sorted alphabetically?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks
From: Jonathan Cameron @ 2020-05-24 13:54 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linus.walleij, alexandre.torgue, linux-iio, s.hauer, linux-kernel,
songqiang1304521, mcoquelin.stm32, lorenzo.bianconi83, shawnguo,
linux-stm32, linux-arm-kernel
In-Reply-To: <20200522104632.517470-2-alexandru.ardelean@analog.com>
On Fri, 22 May 2020 13:46:31 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This should be squashed into the first patch, but it's the more peculiar of
> the changes.
> I am not sure whether this is correct. The touchscreen channels shouldn't
> be enabled by the IIO framework. So, we may need a different way to handle
> those if needed.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Hmm. Unfortunately I can't remember exactly what is going on here.
From a quick look my suspicion is we can handle this using the same
'is it a triggered buffer' test as you now have in the core code.
The touchscreen path operates as a non triggered buffer (I think...)
I'm definitely looking for an ack and preferably a tested-by for this
one. You are right - it's non obvious!
Jonathan
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 9abbbdcc7420..f71071096392 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -937,14 +937,6 @@ static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
> return 0;
> }
>
> -static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> -{
> - if (at91_adc_current_chan_is_touch(indio_dev))
> - return 0;
> -
> - return iio_triggered_buffer_postenable(indio_dev);
> -}
> -
> static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
> {
> struct at91_adc_state *st = iio_priv(indio_dev);
> @@ -995,19 +987,9 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
> return 0;
> }
>
> -static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> -{
> - if (at91_adc_current_chan_is_touch(indio_dev))
> - return 0;
> -
> - return iio_triggered_buffer_predisable(indio_dev);
> -}
> -
> static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
> .preenable = &at91_adc_buffer_preenable,
> .postdisable = &at91_adc_buffer_postdisable,
> - .postenable = &at91_adc_buffer_postenable,
> - .predisable = &at91_adc_buffer_predisable,
> };
>
> static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/3] iio: Move attach/detach of the poll func to the core
From: Jonathan Cameron @ 2020-05-24 13:41 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linus.walleij, Lars-Peter Clausen, alexandre.torgue, linux-iio,
s.hauer, linux-kernel, songqiang1304521, mcoquelin.stm32,
lorenzo.bianconi83, shawnguo, linux-stm32, linux-arm-kernel
In-Reply-To: <20200522104632.517470-1-alexandru.ardelean@analog.com>
On Fri, 22 May 2020 13:46:30 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
>
> All devices using a triggered buffer need to attach and detach the trigger
> to the device in order to properly work. Instead of doing this in each and
> every driver by hand move this into the core.
>
> At this point in time, all drivers should have been resolved to
> attach/detach the poll-function in the same order.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Looks good to me.
Jonathan
> ---
> .../buffer/industrialio-triggered-buffer.c | 10 +--------
> drivers/iio/iio_core_trigger.h | 17 ++++++++++++++
> drivers/iio/industrialio-buffer.c | 13 +++++++++++
> drivers/iio/industrialio-trigger.c | 22 ++++---------------
> include/linux/iio/trigger_consumer.h | 7 ------
> 5 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
> index e8046c1ecd6b..6c20a83f887e 100644
> --- a/drivers/iio/buffer/industrialio-triggered-buffer.c
> +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
> @@ -13,11 +13,6 @@
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/trigger_consumer.h>
>
> -static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
> - .postenable = &iio_triggered_buffer_postenable,
> - .predisable = &iio_triggered_buffer_predisable,
> -};
> -
> /**
> * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
> * @indio_dev: IIO device structure
> @@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
> }
>
> /* Ring buffer functions - here trigger setup related */
> - if (setup_ops)
> - indio_dev->setup_ops = setup_ops;
> - else
> - indio_dev->setup_ops = &iio_triggered_buffer_setup_ops;
> + indio_dev->setup_ops = setup_ops;
>
> /* Flag that polled ring buffering is possible */
> indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
> diff --git a/drivers/iio/iio_core_trigger.h b/drivers/iio/iio_core_trigger.h
> index e59fe2f36bbb..9d1a92cc6480 100644
> --- a/drivers/iio/iio_core_trigger.h
> +++ b/drivers/iio/iio_core_trigger.h
> @@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct iio_dev *indio_dev);
> **/
> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
>
> +
> +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf);
> +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf);
> +
> #else
>
> /**
> @@ -37,4 +43,15 @@ static void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> {
> }
>
> +static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> +{
> + return 0;
> +}
> +static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_TRIGGER_CONSUMER */
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index ec4f531994fa..88d756107fb2 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -20,6 +20,7 @@
>
> #include <linux/iio/iio.h>
> #include "iio_core.h"
> +#include "iio_core_trigger.h"
> #include <linux/iio/sysfs.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/buffer_impl.h>
> @@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
> }
> }
>
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> + ret = iio_trigger_attach_poll_func(indio_dev->trig,
> + indio_dev->pollfunc);
> + if (ret)
> + goto err_disable_buffers;
> + }
> +
> return 0;
>
> err_disable_buffers:
> @@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
> if (list_empty(&indio_dev->buffer_list))
> return 0;
>
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> + iio_trigger_detach_poll_func(indio_dev->trig,
> + indio_dev->pollfunc);
> + }
> +
> /*
> * If things go wrong at some step in disable we still need to continue
> * to perform the other steps, otherwise we leave the device in a
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 53d1931f6be8..6f16357fd732 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger *trig, int irq)
> * the relevant function is in there may be the best option.
> */
> /* Worth protecting against double additions? */
> -static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> - struct iio_poll_func *pf)
> +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> {
> int ret = 0;
> bool notinuse
> @@ -290,8 +290,8 @@ static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> return ret;
> }
>
> -static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> - struct iio_poll_func *pf)
> +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> {
> int ret = 0;
> bool no_other_users
> @@ -705,17 +705,3 @@ void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> if (indio_dev->trig)
> iio_trigger_put(indio_dev->trig);
> }
> -
> -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev)
> -{
> - return iio_trigger_attach_poll_func(indio_dev->trig,
> - indio_dev->pollfunc);
> -}
> -EXPORT_SYMBOL(iio_triggered_buffer_postenable);
> -
> -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
> -{
> - return iio_trigger_detach_poll_func(indio_dev->trig,
> - indio_dev->pollfunc);
> -}
> -EXPORT_SYMBOL(iio_triggered_buffer_predisable);
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index c3c6ba5ec423..3aa2f132dd67 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -50,11 +50,4 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
>
> void iio_trigger_notify_done(struct iio_trigger *trig);
>
> -/*
> - * Two functions for common case where all that happens is a pollfunc
> - * is attached and detached from a trigger
> - */
> -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
> -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
> -
> #endif
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
From: Jonathan Cameron @ 2020-05-24 13:38 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linus.walleij, Lars-Peter Clausen, alexandre.torgue, linux-iio,
s.hauer, linux-kernel, songqiang1304521, mcoquelin.stm32,
lorenzo.bianconi83, shawnguo, linux-stm32, linux-arm-kernel
In-Reply-To: <20200522104632.517470-3-alexandru.ardelean@analog.com>
On Fri, 22 May 2020 13:46:32 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
>
> This patch should be squashed into the first one, as the first one is
> breaking the build (intentionally) to make the IIO core files easier to
> review.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Yeah! Didn't realise you'd finally gotten to the end of your mammoth rework
leading to this.
A few really minor things inline to tidy up.
Thanks,
Jonathan
> diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
> index b5c814ef1637..c87f9a7d2453 100644
> --- a/drivers/iio/accel/st_accel_buffer.c
> +++ b/drivers/iio/accel/st_accel_buffer.c
> @@ -33,13 +33,9 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
> {
> int err;
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err < 0)
> - return err;
> -
> err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
> if (err < 0)
> - goto st_accel_buffer_predisable;
> + return err;
>
> err = st_sensors_set_enable(indio_dev, true);
> if (err < 0)
> @@ -49,8 +45,6 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
>
> st_accel_buffer_enable_all_axis:
> st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> -st_accel_buffer_predisable:
> - iio_triggered_buffer_predisable(indio_dev);
> return err;
> }
>
> @@ -60,12 +54,10 @@ static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
>
> err = st_sensors_set_enable(indio_dev, false);
> if (err < 0)
> - goto st_accel_buffer_predisable;
> -
> - err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> + return err;
>
> -st_accel_buffer_predisable:
> - err2 = iio_triggered_buffer_predisable(indio_dev);
> + err2 = st_sensors_set_axis_enable(indio_dev,
> + ST_SENSORS_ENABLE_ALL_AXIS);
> if (!err)
I don't think you can get here with err set.
> err = err2;
>
...
> diff --git a/drivers/iio/gyro/st_gyro_buffer.c b/drivers/iio/gyro/st_gyro_buffer.c
> index 9c92ff7a82be..7b86502d5da3 100644
> --- a/drivers/iio/gyro/st_gyro_buffer.c
> +++ b/drivers/iio/gyro/st_gyro_buffer.c
> @@ -33,13 +33,9 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
> {
> int err;
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err < 0)
> - return err;
> -
> err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
> if (err < 0)
> - goto st_gyro_buffer_predisable;
> + return err;
>
> err = st_sensors_set_enable(indio_dev, true);
> if (err < 0)
> @@ -49,8 +45,6 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
>
> st_gyro_buffer_enable_all_axis:
> st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> -st_gyro_buffer_predisable:
> - iio_triggered_buffer_predisable(indio_dev);
> return err;
> }
>
> @@ -59,13 +53,8 @@ static int st_gyro_buffer_predisable(struct iio_dev *indio_dev)
> int err, err2;
>
> err = st_sensors_set_enable(indio_dev, false);
> - if (err < 0)
> - goto st_gyro_buffer_predisable;
Previously we didn't bother trying to carry on if this failed. I don't think we
should start doing so now.
> -
> - err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
>
> -st_gyro_buffer_predisable:
> - err2 = iio_triggered_buffer_predisable(indio_dev);
> + err2 = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> if (!err)
> err = err2;
>
...
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index 070d4cd0cf54..29d7af33efa1 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -1390,12 +1390,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
>
> mutex_lock(&data->lock);
I guess it doesn't matter, but no idea why this was ever under the local lock!
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err < 0) {
> - mutex_unlock(&data->lock);
> - return err;
> - }
> -
> /*
> * Enable triggers according to the scan_mask. Enabling either
> * LIGHT_CLEAR or LIGHT_IR scan mode results in enabling ALS
> @@ -1430,8 +1424,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
> err = -ENOMEM;
>
> error_unlock:
> - if (err < 0)
> - iio_triggered_buffer_predisable(indio_dev);
> mutex_unlock(&data->lock);
>
> return err;
> @@ -1465,8 +1457,6 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
> if (err == 0)
> kfree(data->buffer);
>
> - iio_triggered_buffer_predisable(indio_dev);
> -
> mutex_unlock(&data->lock);
>
> return err;
...
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 2a4b3d331055..0fee767af026 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -957,29 +957,20 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
> int ret;
> int cmd;
>
> - ret = iio_triggered_buffer_postenable(indio_dev);
> - if (ret)
> - return ret;
> -
> /* Do not enable the buffer if we are already capturing events. */
> - if (vcnl4010_is_in_periodic_mode(data)) {
> - ret = -EBUSY;
> - goto end;
> - }
> + if (vcnl4010_is_in_periodic_mode(data))
> + return -EBUSY;
>
> ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
> VCNL4010_INT_PROX_EN);
> if (ret < 0)
> - goto end;
> + return ret;
>
> cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
> +
> ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
> if (ret < 0)
> - goto end;
> -
> - return 0;
> -end:
> - iio_triggered_buffer_predisable(indio_dev);
> + i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
>
> return ret;
> }
> @@ -987,18 +978,14 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
> static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct vcnl4000_data *data = iio_priv(indio_dev);
> - int ret, ret_disable;
> + int ret, ret2;
>
> ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
> - if (ret < 0)
> - goto end;
>
> - ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
> + ret2 = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
hmm. This does change the flow a tiny bit. I wonder if we really
care about carrying on if we get an error on the first write?
We are device not responding territory at that point. Maybe just return
immediately and avoid the dance with the two ret variables?
>
> -end:
> - ret_disable = iio_triggered_buffer_predisable(indio_dev);
> if (ret == 0)
> - ret = ret_disable;
> + ret = ret2;
>
> return ret;
> }
...
> static const struct iio_buffer_setup_ops st_press_buffer_setup_ops = {
> diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
> index 37fe851f89af..e082ad007b22 100644
> --- a/drivers/iio/pressure/zpa2326.c
> +++ b/drivers/iio/pressure/zpa2326.c
> @@ -1240,12 +1240,7 @@ static int zpa2326_preenable_buffer(struct iio_dev *indio_dev)
> static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> {
> const struct zpa2326_private *priv = iio_priv(indio_dev);
> - int err;
> -
> - /* Plug our own trigger event handler. */
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err)
> - goto err;
> + int err = 0;
>
> if (!priv->waken) {
> /*
> @@ -1254,7 +1249,7 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> */
> err = zpa2326_clear_fifo(indio_dev, 0);
> if (err)
> - goto err_buffer_predisable;
> + goto out;
> }
>
> if (!iio_trigger_using_own(indio_dev) && priv->waken) {
> @@ -1264,14 +1259,10 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> */
> err = zpa2326_config_oneshot(indio_dev, priv->irq);
> if (err)
> - goto err_buffer_predisable;
> + goto out;
> }
>
> - return 0;
> -
> -err_buffer_predisable:
> - iio_triggered_buffer_predisable(indio_dev);
> -err:
> +out:
> zpa2326_err(indio_dev, "failed to enable buffering (%d)", err);
Doesn't this now print the error in the good path?
Probably still want the return 0. It's a bit messier but I'd
just move the prints into the error paths and return directly from
each. Will be cleaner code that this.
>
> return err;
> @@ -1287,7 +1278,6 @@ static int zpa2326_postdisable_buffer(struct iio_dev *indio_dev)
> static const struct iio_buffer_setup_ops zpa2326_buffer_setup_ops = {
> .preenable = zpa2326_preenable_buffer,
> .postenable = zpa2326_postenable_buffer,
> - .predisable = iio_triggered_buffer_predisable,
> .postdisable = zpa2326_postdisable_buffer
> };
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox