* [PATCH net-next v2 RESEND 0/3] net: ethernet: mediatek: check the hw lro capability by the chip id instead of the dtsi
From: Nelson Chang @ 2016-10-05 12:32 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
The series modify to check if hw lro is supported by the chip id.
changes since v2:
- Refine mtk_get_chip_id() function
changes since v1:
- Because hw lro started to be supported from MT7623, the proper way to check if the feature is capable is to judge by the chip id instead of by the dtsi.
Nelson Chang (3):
net: ethernet: mediatek: get the chip id by ETHDMASYS registers
net: ethernet: mediatek: get hw lro capability by the chip id instead
of by the dtsi
net: ethernet: mediatek: remove hwlro property in the device tree
.../devicetree/bindings/net/mediatek-net.txt | 2 --
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 41 ++++++++++++++++++++--
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 6 ++++
3 files changed, 45 insertions(+), 4 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-10-05 12:31 UTC (permalink / raw)
To: Jassi Brar
Cc: CK Hu, Daniel Kurtz, Monica Wang, Jiaguang Zhang, Nicolas Boichat,
Jassi Brar, cawa cheng, Bibby Hsieh, YT Shen, Damon Chu,
Devicetree List, Sascha Hauer, Daoyuan Huang, Sascha Hauer,
Glory Hung, Rob Herring, linux-mediatek, Matthias Brugger,
"linux-arm-kernel@lists.infradead.org" <linux-arm->
In-Reply-To: <CAJe_ZhcXw1-MOEiiix5VTtKEVr9Z23s_OyikboDYCd0RQwS5pg@mail.gmail.com>
On Wed, 2016-10-05 at 09:07 +0530, Jassi Brar wrote:
> On 5 October 2016 at 08:24, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > On Fri, 2016-09-30 at 17:47 +0800, Horng-Shyang Liao wrote:
> >> On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
>
> >
> > After I trace mailbox driver, I realize that CMDQ driver cannot use
> > tx_done.
> >
> > CMDQ clients will flush many tasks into CMDQ driver, and then CMDQ
> > driver will apply these tasks into GCE HW "immediately". These tasks,
> > which are queued in GCE HW, may not execute immediately since they
> > may need to wait event(s), e.g. vsync.
> >
> > However, in mailbox driver, mailbox uses a software buffer to queue
> > sent messages. It only sends next message until previous message is
> > done. This cannot fulfill CMDQ's requirement.
> >
> I understand
> a) GCE HW can internally queue many tasks in some 'FIFO'
> b) Execution of some task may have to wait until some external event
> occurs (like vsync)
> c) GCE does not generate irq/flag for each task executed (?)
>
> If so, may be your tx_done should return 'true' so long as the GCE HW
> can accept tasks in its 'FIFO'. For mailbox api, any task that is
> queued on GCE, is assumed to be transmitted.
>
> > Quote some code from mailbox driver. Please notice "active_req" part.
> >
> > static void msg_submit(struct mbox_chan *chan)
> > {
> > ...
> > if (!chan->msg_count || chan->active_req)
> > goto exit;
> > ...
> > err = chan->mbox->ops->send_data(chan, data);
> > if (!err) {
> > chan->active_req = data;
> > chan->msg_count--;
> > }
> > ...
> > }
> >
> > static void tx_tick(struct mbox_chan *chan, int r)
> > {
> > ...
> > spin_lock_irqsave(&chan->lock, flags);
> > mssg = chan->active_req;
> > chan->active_req = NULL;
> > spin_unlock_irqrestore(&chan->lock, flags);
> > ...
> > }
> >
> > Current workable CMDQ driver uses mbox_client_txdone() to prevent
> > this issue, and then uses self callback functions to handle done tasks.
> >
> > int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task
> > *task, cmdq_async_flush_cb cb, void *data)
> > {
> > ...
> > mbox_send_message(client->chan, task);
> > /* We can send next task immediately, so just call txdone. */
> > mbox_client_txdone(client->chan, 0);
> > ...
> > }
> >
> > Another solution is to use rx_callback; i.e. CMDQ mailbox controller
> > call mbox_chan_received_data() when CMDQ task is done. But, this may
> > violate the design of mailbox. What do you think?
> >
> If my point (c) above does not hold, maybe look at implementing
> tx_done() callback and submit next task from the callback of last
> done.
Hi Jassi,
For point (c), GCE irq means 1~n tasks done or
0~n tasks done + 1 task error.
In irq, we can know which tasks are done by register and GCE pc.
As I mentioned before, we cannot submit next task after previous task
call tx_done. We need to submit multiple tasks to GCE HW immediately
and queue them in GCE HW. Let me explain this requirement by mouse
cursor example. User may move mouse quickly between two vsync, so DRM
may update display registers frequently. For CMDQ, that means many tasks
are flushed into CMDQ driver, and CMDQ driver needs to process all of
them in next vblank. Therefore, we cannot block any CMDQ task in SW
buffer.
CMDQ needs to call callback function to notice clients which tasks are
done. In my previous e-mail, I mentioned that rx_callback may be an
alternative solution. However, it seems to violate the design of
mailbox. Therefore, I think mailbox may not have a good solution for
CMDQ callback currently. IMHO, the better way is to use CMDQ self
callback for now.
Thanks,
HS
^ permalink raw reply
* Re: [PATCH net-next v2 2/3] net: ethernet: mediatek: get hw lro capability by the chip id instead of by the dtsi
From: Sergei Shtylyov @ 2016-10-05 12:18 UTC (permalink / raw)
To: Nelson Chang, john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw
In-Reply-To: <1475669532-23894-3-git-send-email-nelson.chang@mediatek.com>
Hello.
On 10/05/2016 03:12 PM, Nelson Chang wrote:
> Because hw lro started to be supported from MT7623, the proper way to check if
> the feature is capable is to judge by the chip id instead of by the dtsi.
>
> Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 ++++++++++--
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 0c67ab1..07f3ffa 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2348,6 +2348,14 @@ static int mtk_get_chip_id(struct mtk_eth *eth, u32 *chip_id)
> return 0;
> }
>
> +static bool mtk_is_hwlro_supported(struct mtk_eth *eth)
> +{
> + if (eth->chip_id == MT7623_ETH)
> + return true;
> + else
> + return false;
return eth->chip_id == MT7623_ETH;
[...]
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index a5b422b..58738fd 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -345,6 +345,7 @@
> /* ethernet subsystem chip id register */
> #define ETHSYS_CHIPID0_3 0x0
> #define ETHSYS_CHIPID4_7 0x4
> +#define MT7623_ETH (7623)
() not needed at all.
MBR, Sergei
^ permalink raw reply
* [PATCH net-next v2 3/3] net: ethernet: mediatek: remove hwlro property in the device tree
From: Nelson Chang @ 2016-10-05 12:12 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
In-Reply-To: <1475669532-23894-1-git-send-email-nelson.chang@mediatek.com>
Since the proper way to check the hw lro capability is by the chip id,
hwlro property in the device tree should be removed.
Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
---
Documentation/devicetree/bindings/net/mediatek-net.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
index f095257..c010faf 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -24,7 +24,6 @@ Required properties:
Optional properties:
- interrupt-parent: Should be the phandle for the interrupt controller
that services interrupts for this device
-- mediatek,hwlro: the capability if the hardware supports LRO functions
* Ethernet MAC node
@@ -54,7 +53,6 @@ eth: ethernet@1b100000 {
reset-names = "eth";
mediatek,ethsys = <ðsys>;
mediatek,pctl = <&syscfg_pctl_a>;
- mediatek,hwlro;
#address-cells = <1>;
#size-cells = <0>;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 2/3] net: ethernet: mediatek: get hw lro capability by the chip id instead of by the dtsi
From: Nelson Chang @ 2016-10-05 12:12 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
In-Reply-To: <1475669532-23894-1-git-send-email-nelson.chang@mediatek.com>
Because hw lro started to be supported from MT7623, the proper way to check if
the feature is capable is to judge by the chip id instead of by the dtsi.
Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 ++++++++++--
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 0c67ab1..07f3ffa 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2348,6 +2348,14 @@ static int mtk_get_chip_id(struct mtk_eth *eth, u32 *chip_id)
return 0;
}
+static bool mtk_is_hwlro_supported(struct mtk_eth *eth)
+{
+ if (eth->chip_id == MT7623_ETH)
+ return true;
+ else
+ return false;
+}
+
static int mtk_probe(struct platform_device *pdev)
{
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2387,8 +2395,6 @@ static int mtk_probe(struct platform_device *pdev)
return PTR_ERR(eth->pctl);
}
- eth->hwlro = of_property_read_bool(pdev->dev.of_node, "mediatek,hwlro");
-
for (i = 0; i < 3; i++) {
eth->irq[i] = platform_get_irq(pdev, i);
if (eth->irq[i] < 0) {
@@ -2417,6 +2423,8 @@ static int mtk_probe(struct platform_device *pdev)
if (err)
return err;
+ eth->hwlro = mtk_is_hwlro_supported(eth);
+
for_each_child_of_node(pdev->dev.of_node, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index a5b422b..58738fd 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -345,6 +345,7 @@
/* ethernet subsystem chip id register */
#define ETHSYS_CHIPID0_3 0x0
#define ETHSYS_CHIPID4_7 0x4
+#define MT7623_ETH (7623)
/* ethernet subsystem config register */
#define ETHSYS_SYSCFG0 0x14
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 1/3] net: ethernet: mediatek: get the chip id by ETHDMASYS registers
From: Nelson Chang @ 2016-10-05 12:12 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
In-Reply-To: <1475669532-23894-1-git-send-email-nelson.chang@mediatek.com>
The driver gets the chip id by ETHSYS_CHIPID0_3/ETHSYS_CHIPID4_7 registers
in mtk_probe().
Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 29 +++++++++++++++++++++++++++++
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 5 +++++
2 files changed, 34 insertions(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ad4ab97..0c67ab1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2323,6 +2323,31 @@ free_netdev:
return err;
}
+static int mtk_get_chip_id(struct mtk_eth *eth, u32 *chip_id)
+{
+ u32 val[2], id[4];
+
+ regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
+ regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
+
+ id[3] = ((val[0] >> 16) & 0xff) - '0';
+ id[2] = ((val[0] >> 24) & 0xff) - '0';
+ id[1] = (val[1] & 0xff) - '0';
+ id[0] = ((val[1] >> 8) & 0xff) - '0';
+
+ *chip_id = (id[3] * 1000) + (id[2] * 100) +
+ (id[1] * 10) + id[0];
+
+ if (!(*chip_id)) {
+ dev_err(eth->dev, "failed to get chip id\n");
+ return -ENODEV;
+ }
+
+ dev_info(eth->dev, "chip id = %d\n", *chip_id);
+
+ return 0;
+}
+
static int mtk_probe(struct platform_device *pdev)
{
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2388,6 +2413,10 @@ static int mtk_probe(struct platform_device *pdev)
if (err)
return err;
+ err = mtk_get_chip_id(eth, ð->chip_id);
+ if (err)
+ return err;
+
for_each_child_of_node(pdev->dev.of_node, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 3003195..a5b422b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -342,6 +342,10 @@
#define GPIO_BIAS_CTRL 0xed0
#define GPIO_DRV_SEL10 0xf00
+/* ethernet subsystem chip id register */
+#define ETHSYS_CHIPID0_3 0x0
+#define ETHSYS_CHIPID4_7 0x4
+
/* ethernet subsystem config register */
#define ETHSYS_SYSCFG0 0x14
#define SYSCFG0_GE_MASK 0x3
@@ -534,6 +538,7 @@ struct mtk_eth {
unsigned long sysclk;
struct regmap *ethsys;
struct regmap *pctl;
+ u32 chip_id;
bool hwlro;
atomic_t dma_refcnt;
struct mtk_tx_ring tx_ring;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: ethernet: mediatek: check the hw lro capability by the chip id instead of the dtsi
From: Nelson Chang @ 2016-10-05 12:12 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
The series modify to check if hw lro is supported by the chip id.
changes since v2:
- Refine mtk_get_chip_id() function
changes since v1:
- Because hw lro started to be supported from MT7623, the proper way to check if the feature is capable is to judge by the chip id instead of by the dtsi.
Nelson Chang (3):
net: ethernet: mediatek: get the chip id by ETHDMASYS registers
net: ethernet: mediatek: get hw lro capability by the chip id instead
of by the dtsi
net: ethernet: mediatek: remove hwlro property in the device tree
.../devicetree/bindings/net/mediatek-net.txt | 2 --
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 ++++++++++++++++++++--
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 6 ++++
3 files changed, 43 insertions(+), 4 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Jassi Brar @ 2016-10-05 3:37 UTC (permalink / raw)
To: Horng-Shyang Liao
Cc: CK Hu, Daniel Kurtz, Monica Wang, Jiaguang Zhang, Nicolas Boichat,
Jassi Brar, cawa cheng, Bibby Hsieh, YT Shen, Damon Chu,
Devicetree List, Sascha Hauer, Daoyuan Huang, Sascha Hauer,
Glory Hung, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" <linux-arm-kernel>
In-Reply-To: <1475636064.21937.25.camel@mtksdaap41>
On 5 October 2016 at 08:24, Horng-Shyang Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> On Fri, 2016-09-30 at 17:47 +0800, Horng-Shyang Liao wrote:
>> On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
>
> After I trace mailbox driver, I realize that CMDQ driver cannot use
> tx_done.
>
> CMDQ clients will flush many tasks into CMDQ driver, and then CMDQ
> driver will apply these tasks into GCE HW "immediately". These tasks,
> which are queued in GCE HW, may not execute immediately since they
> may need to wait event(s), e.g. vsync.
>
> However, in mailbox driver, mailbox uses a software buffer to queue
> sent messages. It only sends next message until previous message is
> done. This cannot fulfill CMDQ's requirement.
>
I understand
a) GCE HW can internally queue many tasks in some 'FIFO'
b) Execution of some task may have to wait until some external event
occurs (like vsync)
c) GCE does not generate irq/flag for each task executed (?)
If so, may be your tx_done should return 'true' so long as the GCE HW
can accept tasks in its 'FIFO'. For mailbox api, any task that is
queued on GCE, is assumed to be transmitted.
> Quote some code from mailbox driver. Please notice "active_req" part.
>
> static void msg_submit(struct mbox_chan *chan)
> {
> ...
> if (!chan->msg_count || chan->active_req)
> goto exit;
> ...
> err = chan->mbox->ops->send_data(chan, data);
> if (!err) {
> chan->active_req = data;
> chan->msg_count--;
> }
> ...
> }
>
> static void tx_tick(struct mbox_chan *chan, int r)
> {
> ...
> spin_lock_irqsave(&chan->lock, flags);
> mssg = chan->active_req;
> chan->active_req = NULL;
> spin_unlock_irqrestore(&chan->lock, flags);
> ...
> }
>
> Current workable CMDQ driver uses mbox_client_txdone() to prevent
> this issue, and then uses self callback functions to handle done tasks.
>
> int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task
> *task, cmdq_async_flush_cb cb, void *data)
> {
> ...
> mbox_send_message(client->chan, task);
> /* We can send next task immediately, so just call txdone. */
> mbox_client_txdone(client->chan, 0);
> ...
> }
>
> Another solution is to use rx_callback; i.e. CMDQ mailbox controller
> call mbox_chan_received_data() when CMDQ task is done. But, this may
> violate the design of mailbox. What do you think?
>
If my point (c) above does not hold, maybe look at implementing
tx_done() callback and submit next task from the callback of last
done.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-10-05 2:54 UTC (permalink / raw)
To: CK Hu
Cc: Rob Herring, Matthias Brugger, Jassi Brar, Daniel Kurtz,
Sascha Hauer, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, srv_heupstream, Sascha Hauer, Philipp Zabel,
Nicolas Boichat, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang,
Damon Chu, Josh-YC Liu, Glory Hung
In-Reply-To: <1475228829.3658.1.camel@mtksdaap41>
On Fri, 2016-09-30 at 17:47 +0800, Horng-Shyang Liao wrote:
> On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
> > Hi, HS:
> >
> > One comment inline
> >
> > On Fri, 2016-09-30 at 16:56 +0800, Horng-Shyang Liao wrote:
> > > Hi CK,
> > >
> > > Please see my inline reply.
> > >
> > > On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> > > > Hi, HS:
> > > >
> > > > On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > > > > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > > > > CMDQ is used to help write registers with critical time limitation,
> > > > > such as updating display configuration during the vblank. It controls
> > > > > Global Command Engine (GCE) hardware to achieve this requirement.
> > > > > Currently, CMDQ only supports display related hardwares, but we expect
> > > > > it can be extended to other hardwares for future requirements.
> > > > >
> > > > > Signed-off-by: HS Liao <hs.liao@mediatek.com>
> > > > > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > > > > ---
> > > >
> > > > [snip...]
> > > >
> > > > > +
> > > > > +struct cmdq_task {
> > > > > + struct cmdq *cmdq;
> > > > > + struct list_head list_entry;
> > > > > + void *va_base;
> > > > > + dma_addr_t pa_base;
> > > > > + size_t cmd_buf_size; /* command occupied size */
> > > > > + size_t buf_size; /* real buffer size */
> > > > > + bool finalized;
> > > > > + struct cmdq_thread *thread;
> > > >
> > > > I think thread info could be removed from cmdq_task. Only
> > > > cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> > > > task->thread and caller of both function has the thread info. So you
> > > > could just pass thread info into these two function and remove thread
> > > > info in cmdq_task.
> > >
> > > This modification will remove 1 pointer but add 2 pointers. Moreover,
> > > more pointers will need to be delivered between functions for future
> > > extension. IMHO, it would be better to keep thread pointer inside
> > > cmdq_task.
> > >
> > > > > + struct cmdq_task_cb cb;
> > > >
> > > > I think this callback function is equal to mailbox client tx_done
> > > > callback. It's better to use already-defined interface rather than
> > > > creating your own.
> > >
> > > This is because CMDQ driver allows different callback functions for
> > > different tasks, but mailbox only allows one callback function per
> > > channel. But, I think I can add a wrapper for tx_done to call CMDQ
> > > callback functions. So, I will use tx_done in CMDQ v15.
> >
> > Up to now, one callback function for one channel is enough for DRM. So
> > 'different callback function for different sent-message' looks like an
> > advanced function. Maybe you should not include it in first patch.
> >
> > Regards,
> > CK
>
> Hi CK,
>
> OK. I will do it.
>
> Thanks,
> HS
>
> [snip...]
Hi CK,
After I trace mailbox driver, I realize that CMDQ driver cannot use
tx_done.
CMDQ clients will flush many tasks into CMDQ driver, and then CMDQ
driver will apply these tasks into GCE HW "immediately". These tasks,
which are queued in GCE HW, may not execute immediately since they
may need to wait event(s), e.g. vsync.
However, in mailbox driver, mailbox uses a software buffer to queue
sent messages. It only sends next message until previous message is
done. This cannot fulfill CMDQ's requirement.
Quote some code from mailbox driver. Please notice "active_req" part.
static void msg_submit(struct mbox_chan *chan)
{
...
if (!chan->msg_count || chan->active_req)
goto exit;
...
err = chan->mbox->ops->send_data(chan, data);
if (!err) {
chan->active_req = data;
chan->msg_count--;
}
...
}
static void tx_tick(struct mbox_chan *chan, int r)
{
...
spin_lock_irqsave(&chan->lock, flags);
mssg = chan->active_req;
chan->active_req = NULL;
spin_unlock_irqrestore(&chan->lock, flags);
...
}
Current workable CMDQ driver uses mbox_client_txdone() to prevent
this issue, and then uses self callback functions to handle done tasks.
int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task
*task, cmdq_async_flush_cb cb, void *data)
{
...
mbox_send_message(client->chan, task);
/* We can send next task immediately, so just call txdone. */
mbox_client_txdone(client->chan, 0);
...
}
Another solution is to use rx_callback; i.e. CMDQ mailbox controller
call mbox_chan_received_data() when CMDQ task is done. But, this may
violate the design of mailbox. What do you think?
Thanks,
HS
Hi Jassi,
Do you have any suggestion about previous situation?
Thanks,
HS
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: ethernet: mediatek: get hw lro capability by the chip id instead of by the dtsi
From: Nelson Chang @ 2016-10-04 12:22 UTC (permalink / raw)
To: john, davem; +Cc: netdev, nbd, linux-mediatek, nelsonch.tw
Hi John,
do you plan to add more chips to the mtk_is_hwlro_supporte() function ?
=> yes, there will be more chips with hw lro in the future, so i think
using mtk_is_hwlro_supporte() function can have the scalability.
Thanks.
Nelson
-----Original Message-----
From: John Crispin [mailto:john@phrozen.org]
Sent: Tuesday, October 04, 2016 3:19 AM
To: Nelson Chang (張家祥); davem@davemloft.net
Cc: netdev@vger.kernel.org; nbd@openwrt.org;
linux-mediatek@lists.infradead.org; nelsonch.tw@gmail.com
Subject: Re: [PATCH net-next 2/3] net: ethernet: mediatek: get hw lro
capability by the chip id instead of by the dtsi
Hi Nelson,
comment inline
On 03/10/2016 09:18, Nelson Chang wrote:
> Because hw lro started to be supported from MT7623, the proper way to
> check if the feature is capable is to judge by the chip id instead of
by the dtsi.
>
> Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 ++++++++++--
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index a3e4ae6..3d16a0c 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2344,6 +2344,14 @@ static u32 mtk_get_chip_id(struct mtk_eth *eth)
> return chip_id;
> }
>
> +static bool mtk_is_hwlro_supported(struct mtk_eth *eth) {
> + if (eth->chip_id == MT7623_ETH)
> + return true;
> + else
> + return false;
> +}
> +
> static int mtk_probe(struct platform_device *pdev) {
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM,
> 0); @@ -2383,8 +2391,6 @@ static int mtk_probe(struct platform_device
*pdev)
> return PTR_ERR(eth->pctl);
> }
>
> - eth->hwlro = of_property_read_bool(pdev->dev.of_node,
"mediatek,hwlro");
> -
> for (i = 0; i < 3; i++) {
> eth->irq[i] = platform_get_irq(pdev, i);
> if (eth->irq[i] < 0) {
> @@ -2415,6 +2421,8 @@ static int mtk_probe(struct platform_device
*pdev)
> return -ENODEV;
> }
>
> + eth->hwlro = mtk_is_hwlro_supported(eth);
> +
do you plan to add more chips to the mtk_is_hwlro_supporte() function ?
if not a simple
eth->hwlro = (eth->chip_id == MT7623_ETH);
might be enough
John
> for_each_child_of_node(pdev->dev.of_node, mac_np) {
> if (!of_device_is_compatible(mac_np,
> "mediatek,eth-mac"))
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index a5b422b..58738fd 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -345,6 +345,7 @@
> /* ethernet subsystem chip id register */
> #define ETHSYS_CHIPID0_3 0x0
> #define ETHSYS_CHIPID4_7 0x4
> +#define MT7623_ETH (7623)
>
> /* ethernet subsystem config register */
> #define ETHSYS_SYSCFG0 0x14
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: ethernet: mediatek: get the chip id by ETHDMASYS registers
From: John Crispin @ 2016-10-04 12:14 UTC (permalink / raw)
To: Nelson Chang, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw
In-Reply-To: <1475583160.7983.4.camel@mtksdaap41>
On 04/10/2016 14:12, Nelson Chang wrote:
> Hi John,
>
> Thanks for your review!
> I will modify that as below. Would you think it is okay?
>
> static int mtk_get_chip_id(struct mtk_eth *eth, u32 *chip_id)
> {
> u32 val[2], id[4];
>
> regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
> regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
>
> id[3] = ((val[0] >> 16) & 0xff) - '0';
> id[2] = ((val[0] >> 24) & 0xff) - '0';
> id[1] = (val[1] & 0xff) - '0';
> id[0] = ((val[1] >> 8) & 0xff) - '0';
>
> *chip_id = (id[3] * 1000) + (id[2] * 100) +
> (id[1] * 10) + id[0];
>
> if (!(*chip_id)) {
> dev_err(eth->dev, "failed to get chip id\n");
> return -ENODEV;
> }
>
> dev_info(eth->dev, "chip id = %d\n", *chip_id);
>
> return 0;
> }
> ...
> static int mtk_probe(struct platform_device *pdev)
> {
> ...
> err = mtk_get_chip_id(eth, ð->chip_id);
> if (err)
> return err;
> ...
> }
>
>
> Nelson
Hi Nelson,
I think that looks nicer, thanks !
John
>
> -----Original Message-----
> From: John Crispin [mailto:john@phrozen.org]
> Sent: Tuesday, October 04, 2016 3:17 AM
> To: Nelson Chang (張家祥); davem@davemloft.net
> Cc: nbd@openwrt.org; netdev@vger.kernel.org;
> linux-mediatek@lists.infradead.org; nelsonch.tw@gmail.com
> Subject: Re: [PATCH net-next 1/3] net: ethernet: mediatek: get the chip
> id by ETHDMASYS registers
>
> Hi Nelson,
>
> comments inline
>
> On 03/10/2016 09:18, Nelson Chang wrote:
>> The driver gets the chip id by ETHSYS_CHIPID0_3/ETHSYS_CHIPID4_7
>> registers in mtk_probe().
>>
>> Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
>> ---
>> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 27
>> +++++++++++++++++++++++++++
>> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 5 +++++
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> index ad4ab97..a3e4ae6 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -2323,6 +2323,27 @@ free_netdev:
>> return err;
>> }
>>
>> +static u32 mtk_get_chip_id(struct mtk_eth *eth) {
>> + u32 val[2], id[4];
>> + u32 chip_id;
>> +
>> + regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
>> + regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
>> +
>> + id[3] = ((val[0] >> 16) & 0xff) - '0';
>> + id[2] = ((val[0] >> 24) & 0xff) - '0';
>> + id[1] = (val[1] & 0xff) - '0';
>> + id[0] = ((val[1] >> 8) & 0xff) - '0';
>> +
>> + chip_id = (id[3] * 1000) + (id[2] * 100) +
>> + (id[1] * 10) + id[0];
>> +
>> + dev_info(eth->dev, "chip id = %d\n", chip_id);
>
> the chip id is printed here
>
>> + return chip_id;
>> +}
>> +
>> static int mtk_probe(struct platform_device *pdev) {
>> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM,
>> 0); @@ -2388,6 +2409,12 @@ static int mtk_probe(struct platform_device
> *pdev)
>> if (err)
>> return err;
>>
>> + eth->chip_id = mtk_get_chip_id(eth);
>> + if (!eth->chip_id) {
>> + dev_err(&pdev->dev, "failed to get chip id\n");
>> + return -ENODEV;
>> + }
>> +
>
> and the error check happens here. maybe you could move the dev_err to
> the above function.
>
> John
>
>> for_each_child_of_node(pdev->dev.of_node, mac_np) {
>> if (!of_device_is_compatible(mac_np,
>> "mediatek,eth-mac"))
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> index 3003195..a5b422b 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> @@ -342,6 +342,10 @@
>> #define GPIO_BIAS_CTRL 0xed0
>> #define GPIO_DRV_SEL10 0xf00
>>
>> +/* ethernet subsystem chip id register */
>> +#define ETHSYS_CHIPID0_3 0x0
>> +#define ETHSYS_CHIPID4_7 0x4
>> +
>> /* ethernet subsystem config register */
>> #define ETHSYS_SYSCFG0 0x14
>> #define SYSCFG0_GE_MASK 0x3
>> @@ -534,6 +538,7 @@ struct mtk_eth {
>> unsigned long sysclk;
>> struct regmap *ethsys;
>> struct regmap *pctl;
>> + u32 chip_id;
>> bool hwlro;
>> atomic_t dma_refcnt;
>> struct mtk_tx_ring tx_ring;
>>
>
>
^ permalink raw reply
* RE: [PATCH net-next 1/3] net: ethernet: mediatek: get the chip id by ETHDMASYS registers
From: Nelson Chang @ 2016-10-04 12:12 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw
Hi John,
Thanks for your review!
I will modify that as below. Would you think it is okay?
static int mtk_get_chip_id(struct mtk_eth *eth, u32 *chip_id)
{
u32 val[2], id[4];
regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
id[3] = ((val[0] >> 16) & 0xff) - '0';
id[2] = ((val[0] >> 24) & 0xff) - '0';
id[1] = (val[1] & 0xff) - '0';
id[0] = ((val[1] >> 8) & 0xff) - '0';
*chip_id = (id[3] * 1000) + (id[2] * 100) +
(id[1] * 10) + id[0];
if (!(*chip_id)) {
dev_err(eth->dev, "failed to get chip id\n");
return -ENODEV;
}
dev_info(eth->dev, "chip id = %d\n", *chip_id);
return 0;
}
...
static int mtk_probe(struct platform_device *pdev)
{
...
err = mtk_get_chip_id(eth, ð->chip_id);
if (err)
return err;
...
}
Nelson
-----Original Message-----
From: John Crispin [mailto:john@phrozen.org]
Sent: Tuesday, October 04, 2016 3:17 AM
To: Nelson Chang (張家祥); davem@davemloft.net
Cc: nbd@openwrt.org; netdev@vger.kernel.org;
linux-mediatek@lists.infradead.org; nelsonch.tw@gmail.com
Subject: Re: [PATCH net-next 1/3] net: ethernet: mediatek: get the chip
id by ETHDMASYS registers
Hi Nelson,
comments inline
On 03/10/2016 09:18, Nelson Chang wrote:
> The driver gets the chip id by ETHSYS_CHIPID0_3/ETHSYS_CHIPID4_7
> registers in mtk_probe().
>
> Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 27
> +++++++++++++++++++++++++++
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 5 +++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index ad4ab97..a3e4ae6 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2323,6 +2323,27 @@ free_netdev:
> return err;
> }
>
> +static u32 mtk_get_chip_id(struct mtk_eth *eth) {
> + u32 val[2], id[4];
> + u32 chip_id;
> +
> + regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
> + regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
> +
> + id[3] = ((val[0] >> 16) & 0xff) - '0';
> + id[2] = ((val[0] >> 24) & 0xff) - '0';
> + id[1] = (val[1] & 0xff) - '0';
> + id[0] = ((val[1] >> 8) & 0xff) - '0';
> +
> + chip_id = (id[3] * 1000) + (id[2] * 100) +
> + (id[1] * 10) + id[0];
> +
> + dev_info(eth->dev, "chip id = %d\n", chip_id);
the chip id is printed here
> + return chip_id;
> +}
> +
> static int mtk_probe(struct platform_device *pdev) {
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM,
> 0); @@ -2388,6 +2409,12 @@ static int mtk_probe(struct platform_device
*pdev)
> if (err)
> return err;
>
> + eth->chip_id = mtk_get_chip_id(eth);
> + if (!eth->chip_id) {
> + dev_err(&pdev->dev, "failed to get chip id\n");
> + return -ENODEV;
> + }
> +
and the error check happens here. maybe you could move the dev_err to
the above function.
John
> for_each_child_of_node(pdev->dev.of_node, mac_np) {
> if (!of_device_is_compatible(mac_np,
> "mediatek,eth-mac"))
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 3003195..a5b422b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -342,6 +342,10 @@
> #define GPIO_BIAS_CTRL 0xed0
> #define GPIO_DRV_SEL10 0xf00
>
> +/* ethernet subsystem chip id register */
> +#define ETHSYS_CHIPID0_3 0x0
> +#define ETHSYS_CHIPID4_7 0x4
> +
> /* ethernet subsystem config register */
> #define ETHSYS_SYSCFG0 0x14
> #define SYSCFG0_GE_MASK 0x3
> @@ -534,6 +538,7 @@ struct mtk_eth {
> unsigned long sysclk;
> struct regmap *ethsys;
> struct regmap *pctl;
> + u32 chip_id;
> bool hwlro;
> atomic_t dma_refcnt;
> struct mtk_tx_ring tx_ring;
>
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: ethernet: mediatek: get hw lro capability by the chip id instead of by the dtsi
From: John Crispin @ 2016-10-03 19:18 UTC (permalink / raw)
To: Nelson Chang, davem; +Cc: netdev, nbd, linux-mediatek, nelsonch.tw
In-Reply-To: <1475479131-19822-3-git-send-email-nelson.chang@mediatek.com>
Hi Nelson,
comment inline
On 03/10/2016 09:18, Nelson Chang wrote:
> Because hw lro started to be supported from MT7623, the proper way to check if
> the feature is capable is to judge by the chip id instead of by the dtsi.
>
> Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 ++++++++++--
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index a3e4ae6..3d16a0c 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2344,6 +2344,14 @@ static u32 mtk_get_chip_id(struct mtk_eth *eth)
> return chip_id;
> }
>
> +static bool mtk_is_hwlro_supported(struct mtk_eth *eth)
> +{
> + if (eth->chip_id == MT7623_ETH)
> + return true;
> + else
> + return false;
> +}
> +
> static int mtk_probe(struct platform_device *pdev)
> {
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -2383,8 +2391,6 @@ static int mtk_probe(struct platform_device *pdev)
> return PTR_ERR(eth->pctl);
> }
>
> - eth->hwlro = of_property_read_bool(pdev->dev.of_node, "mediatek,hwlro");
> -
> for (i = 0; i < 3; i++) {
> eth->irq[i] = platform_get_irq(pdev, i);
> if (eth->irq[i] < 0) {
> @@ -2415,6 +2421,8 @@ static int mtk_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + eth->hwlro = mtk_is_hwlro_supported(eth);
> +
do you plan to add more chips to the mtk_is_hwlro_supporte() function ?
if not a simple
eth->hwlro = (eth->chip_id == MT7623_ETH);
might be enough
John
> for_each_child_of_node(pdev->dev.of_node, mac_np) {
> if (!of_device_is_compatible(mac_np,
> "mediatek,eth-mac"))
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index a5b422b..58738fd 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -345,6 +345,7 @@
> /* ethernet subsystem chip id register */
> #define ETHSYS_CHIPID0_3 0x0
> #define ETHSYS_CHIPID4_7 0x4
> +#define MT7623_ETH (7623)
>
> /* ethernet subsystem config register */
> #define ETHSYS_SYSCFG0 0x14
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: ethernet: mediatek: get the chip id by ETHDMASYS registers
From: John Crispin @ 2016-10-03 19:17 UTC (permalink / raw)
To: Nelson Chang, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw
In-Reply-To: <1475479131-19822-2-git-send-email-nelson.chang@mediatek.com>
Hi Nelson,
comments inline
On 03/10/2016 09:18, Nelson Chang wrote:
> The driver gets the chip id by ETHSYS_CHIPID0_3/ETHSYS_CHIPID4_7 registers
> in mtk_probe().
>
> Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 27 +++++++++++++++++++++++++++
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 5 +++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index ad4ab97..a3e4ae6 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2323,6 +2323,27 @@ free_netdev:
> return err;
> }
>
> +static u32 mtk_get_chip_id(struct mtk_eth *eth)
> +{
> + u32 val[2], id[4];
> + u32 chip_id;
> +
> + regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
> + regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
> +
> + id[3] = ((val[0] >> 16) & 0xff) - '0';
> + id[2] = ((val[0] >> 24) & 0xff) - '0';
> + id[1] = (val[1] & 0xff) - '0';
> + id[0] = ((val[1] >> 8) & 0xff) - '0';
> +
> + chip_id = (id[3] * 1000) + (id[2] * 100) +
> + (id[1] * 10) + id[0];
> +
> + dev_info(eth->dev, "chip id = %d\n", chip_id);
the chip id is printed here
> + return chip_id;
> +}
> +
> static int mtk_probe(struct platform_device *pdev)
> {
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -2388,6 +2409,12 @@ static int mtk_probe(struct platform_device *pdev)
> if (err)
> return err;
>
> + eth->chip_id = mtk_get_chip_id(eth);
> + if (!eth->chip_id) {
> + dev_err(&pdev->dev, "failed to get chip id\n");
> + return -ENODEV;
> + }
> +
and the error check happens here. maybe you could move the dev_err to
the above function.
John
> for_each_child_of_node(pdev->dev.of_node, mac_np) {
> if (!of_device_is_compatible(mac_np,
> "mediatek,eth-mac"))
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 3003195..a5b422b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -342,6 +342,10 @@
> #define GPIO_BIAS_CTRL 0xed0
> #define GPIO_DRV_SEL10 0xf00
>
> +/* ethernet subsystem chip id register */
> +#define ETHSYS_CHIPID0_3 0x0
> +#define ETHSYS_CHIPID4_7 0x4
> +
> /* ethernet subsystem config register */
> #define ETHSYS_SYSCFG0 0x14
> #define SYSCFG0_GE_MASK 0x3
> @@ -534,6 +538,7 @@ struct mtk_eth {
> unsigned long sysclk;
> struct regmap *ethsys;
> struct regmap *pctl;
> + u32 chip_id;
> bool hwlro;
> atomic_t dma_refcnt;
> struct mtk_tx_ring tx_ring;
>
^ permalink raw reply
* Re: [PATCH] drm/mediatek: fix a typo
From: Matthias Brugger @ 2016-10-03 8:46 UTC (permalink / raw)
To: Bibby Hsieh
Cc: CK Hu, David Airlie, Daniel Vetter, dri-devel, linux-mediatek,
Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Philipp Zabel, YT Shen,
Thierry Reding, Mao Huang, linux-arm-kernel, linux-kernel,
Sascha Hauer
In-Reply-To: <1475205119.13345.6.camel@mtksdaap41>
On 30/09/16 05:11, Bibby Hsieh wrote:
> On Thu, 2016-09-29 at 10:46 +0200, Matthias Brugger wrote:
>>
>> On 29/09/16 06:01, CK Hu wrote:
>>> Acked-by: CK Hu <ck.hu@mediatek.com>
>>>
>>> On Thu, 2016-09-29 at 11:22 +0800, Bibby Hsieh wrote:
>>>> Fix the typo: OD_RELAYMODE->OD_CFG
>>>>
>>
>
> Hi, Matthias
> Thanks for your reply.
>
>> Although it is quite clear what the patch does, could you write one
>> sentence to explain what it does. Maybe explain even which effect it
>> has, which error get fixed etc.
>
> Ok, I will do that.
>
>> As we are getting public available boards now, we should take more care
>> about fixes. If you have a fix for a commit introduced in an earlier
>> version of linux and it should be fixed for this version as well (e.g.
>> v4.6 does have the feature but it does not work correctly) then please
>> add these two lines before your Signed-off-by:
>> Fixes: <commit-hash> ("<commit subject line>")
>> Cc: stable@vger.kernel.org # v4.6+
>>
>> Where v4.6+ stands for the oldest version where this should get fixed.
>>
>
> Ok, but the patch hasn't been merged into v4.8 (just in drm-next [1] and
> linux-next [2]), how can I mark that?
>
If the commit get's into v4.9 then make sure that this patch get's
merged into the fixes for v4.9 by the corresponding maintainer.
Regards,
Matthias
> [1]
> https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=7216436420414144646f5d8343d061355fd23483
> [2]
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=7216436420414144646f5d8343d061355fd23483
>
>
>> Thanks a lot,
>> Matthias
>>
>>>> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
>>>> ---
>>>> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
>>>> index df33b3c..aa5f20f 100644
>>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
>>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
>>>> @@ -123,7 +123,7 @@ static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
>>>> unsigned int bpc)
>>>> {
>>>> writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
>>>> - writel(OD_RELAYMODE, comp->regs + OD_RELAYMODE);
>>>> + writel(OD_RELAYMODE, comp->regs + OD_CFG);
>>>> mtk_dither_set(comp, bpc, DISP_OD_CFG);
>>>> }
>>>>
>>>
>>>
>
^ permalink raw reply
* [PATCH net-next 3/3] net: ethernet: mediatek: remove hwlro property in the device tree
From: Nelson Chang @ 2016-10-03 7:18 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
In-Reply-To: <1475479131-19822-1-git-send-email-nelson.chang@mediatek.com>
Since the proper way to check the hw lro capability is by the chip id,
hwlro property in the device tree should be removed.
Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
---
Documentation/devicetree/bindings/net/mediatek-net.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
index f095257..c010faf 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -24,7 +24,6 @@ Required properties:
Optional properties:
- interrupt-parent: Should be the phandle for the interrupt controller
that services interrupts for this device
-- mediatek,hwlro: the capability if the hardware supports LRO functions
* Ethernet MAC node
@@ -54,7 +53,6 @@ eth: ethernet@1b100000 {
reset-names = "eth";
mediatek,ethsys = <ðsys>;
mediatek,pctl = <&syscfg_pctl_a>;
- mediatek,hwlro;
#address-cells = <1>;
#size-cells = <0>;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 2/3] net: ethernet: mediatek: get hw lro capability by the chip id instead of by the dtsi
From: Nelson Chang @ 2016-10-03 7:18 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
In-Reply-To: <1475479131-19822-1-git-send-email-nelson.chang@mediatek.com>
Because hw lro started to be supported from MT7623, the proper way to check if
the feature is capable is to judge by the chip id instead of by the dtsi.
Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 ++++++++++--
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index a3e4ae6..3d16a0c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2344,6 +2344,14 @@ static u32 mtk_get_chip_id(struct mtk_eth *eth)
return chip_id;
}
+static bool mtk_is_hwlro_supported(struct mtk_eth *eth)
+{
+ if (eth->chip_id == MT7623_ETH)
+ return true;
+ else
+ return false;
+}
+
static int mtk_probe(struct platform_device *pdev)
{
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2383,8 +2391,6 @@ static int mtk_probe(struct platform_device *pdev)
return PTR_ERR(eth->pctl);
}
- eth->hwlro = of_property_read_bool(pdev->dev.of_node, "mediatek,hwlro");
-
for (i = 0; i < 3; i++) {
eth->irq[i] = platform_get_irq(pdev, i);
if (eth->irq[i] < 0) {
@@ -2415,6 +2421,8 @@ static int mtk_probe(struct platform_device *pdev)
return -ENODEV;
}
+ eth->hwlro = mtk_is_hwlro_supported(eth);
+
for_each_child_of_node(pdev->dev.of_node, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index a5b422b..58738fd 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -345,6 +345,7 @@
/* ethernet subsystem chip id register */
#define ETHSYS_CHIPID0_3 0x0
#define ETHSYS_CHIPID4_7 0x4
+#define MT7623_ETH (7623)
/* ethernet subsystem config register */
#define ETHSYS_SYSCFG0 0x14
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 1/3] net: ethernet: mediatek: get the chip id by ETHDMASYS registers
From: Nelson Chang @ 2016-10-03 7:18 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
In-Reply-To: <1475479131-19822-1-git-send-email-nelson.chang@mediatek.com>
The driver gets the chip id by ETHSYS_CHIPID0_3/ETHSYS_CHIPID4_7 registers
in mtk_probe().
Signed-off-by: Nelson Chang <nelson.chang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 27 +++++++++++++++++++++++++++
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 5 +++++
2 files changed, 32 insertions(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ad4ab97..a3e4ae6 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2323,6 +2323,27 @@ free_netdev:
return err;
}
+static u32 mtk_get_chip_id(struct mtk_eth *eth)
+{
+ u32 val[2], id[4];
+ u32 chip_id;
+
+ regmap_read(eth->ethsys, ETHSYS_CHIPID0_3, &val[0]);
+ regmap_read(eth->ethsys, ETHSYS_CHIPID4_7, &val[1]);
+
+ id[3] = ((val[0] >> 16) & 0xff) - '0';
+ id[2] = ((val[0] >> 24) & 0xff) - '0';
+ id[1] = (val[1] & 0xff) - '0';
+ id[0] = ((val[1] >> 8) & 0xff) - '0';
+
+ chip_id = (id[3] * 1000) + (id[2] * 100) +
+ (id[1] * 10) + id[0];
+
+ dev_info(eth->dev, "chip id = %d\n", chip_id);
+
+ return chip_id;
+}
+
static int mtk_probe(struct platform_device *pdev)
{
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2388,6 +2409,12 @@ static int mtk_probe(struct platform_device *pdev)
if (err)
return err;
+ eth->chip_id = mtk_get_chip_id(eth);
+ if (!eth->chip_id) {
+ dev_err(&pdev->dev, "failed to get chip id\n");
+ return -ENODEV;
+ }
+
for_each_child_of_node(pdev->dev.of_node, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 3003195..a5b422b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -342,6 +342,10 @@
#define GPIO_BIAS_CTRL 0xed0
#define GPIO_DRV_SEL10 0xf00
+/* ethernet subsystem chip id register */
+#define ETHSYS_CHIPID0_3 0x0
+#define ETHSYS_CHIPID4_7 0x4
+
/* ethernet subsystem config register */
#define ETHSYS_SYSCFG0 0x14
#define SYSCFG0_GE_MASK 0x3
@@ -534,6 +538,7 @@ struct mtk_eth {
unsigned long sysclk;
struct regmap *ethsys;
struct regmap *pctl;
+ u32 chip_id;
bool hwlro;
atomic_t dma_refcnt;
struct mtk_tx_ring tx_ring;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 0/3] net: ethernet: mediatek: check the hw lro capability by the chip id instead of the dtsi
From: Nelson Chang @ 2016-10-03 7:18 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, nelsonch.tw, Nelson Chang
The series modify to check if hw lro is supported by the chip id.
Because hw lro started to be supported from MT7623, the proper way to check if
the feature is capable is to judge by the chip id instead of by the dtsi.
Nelson Chang (3):
net: ethernet: mediatek: get the chip id by ETHDMASYS registers
net: ethernet: mediatek: get hw lro capability by the chip id instead
of by the dtsi
net: ethernet: mediatek: remove hwlro property in the device tree
.../devicetree/bindings/net/mediatek-net.txt | 2 --
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 ++++++++++++++++++++--
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 6 ++++
3 files changed, 43 insertions(+), 4 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH] net: ethernet: mediatek: mark symbols static where possible
From: David Miller @ 2016-10-03 5:24 UTC (permalink / raw)
To: baoyou.xie
Cc: nbd, blogic, matthias.bgg, netdev, linux-arm-kernel,
linux-mediatek, linux-kernel, arnd, xie.baoyou, han.fei,
tang.qiang007
In-Reply-To: <1475221730-18773-1-git-send-email-baoyou.xie@linaro.org>
From: Baoyou Xie <baoyou.xie@linaro.org>
Date: Fri, 30 Sep 2016 15:48:50 +0800
> We get 2 warnings when building kernel with W=1:
> drivers/net/ethernet/mediatek/mtk_eth_soc.c:2041:5: warning: no previous prototype for 'mtk_get_link_ksettings' [-Wmissing-prototypes]
> drivers/net/ethernet/mediatek/mtk_eth_soc.c:2052:5: warning: no previous prototype for 'mtk_set_link_ksettings' [-Wmissing-prototypes]
>
> In fact, these functions are only used in the file in which they are
> declared and don't need a declaration, but can be made static.
> So this patch marks these functions with 'static'.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] mtd: mtk: avoid warning in mtk_ecc_encode
From: Boris Brezillon @ 2016-10-01 9:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David Woodhouse, Brian Norris, Richard Weinberger,
Matthias Brugger, linux-mtd, linux-arm-kernel, linux-mediatek,
linux-kernel
In-Reply-To: <201609301925.17577.arnd@arndb.de>
On Fri, 30 Sep 2016 19:25:17 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 30 September 2016, Boris Brezillon wrote:
> > > + /* copy into possibly unaligned OOB region with actual length */
> > > + memcpy(data + bytes, eccdata, len);
> >
> > Is it better than
> >
> > for (i = 0; i < len; i += 4) {
> > u32 val = __raw_readl(ecc->regs + ECC_ENCPAR(i / 4));
> >
> > memcpy(data + bytes + i, &val, min(len, 4));
> > }
> >
> > I'm probably missing something, but what's the point of creating a
> > temporary buffer of 112 bytes on the stack since you'll have to copy
> > this data to the oob buffer at some point?
>
>
> I tried something like that first, but wasn't too happy with it for
> a number of small reasons:
>
> - __raw_readl in a driver is not usually the right API, __memcpy32_from_io
> uses it internally, but it's better for a driver not to rely on that,
> in case we need some barriers (which we may in factt need for other drivers).
I agree, even though calling something prefixed with __ (in this case,
__ioread32_copy()) sounds like a bad thing too :).
>
> - the min(len,4) expression is incorrect, fixing that makes it more complicated
> again
Sorry, it's min(len - i, 4), which is not that complicated :P.
>
> - I didn't like to call memcpy() multiple times, as that might get turned
> into an external function call (the compiler is free to optimize small
> memcpy calls or not).
Okay.
>
> I agree that he 112 byte buffer isn't ideal either, it just seemed to
> be the lesser annoyance.
How about we keep your approach, but put the buffer in the mtk_ecc
struct?
^ permalink raw reply
* Re: [PATCH] mtd: mtk: avoid warning in mtk_ecc_encode
From: Arnd Bergmann @ 2016-09-30 17:25 UTC (permalink / raw)
To: Boris Brezillon
Cc: Richard Weinberger, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
David Woodhouse,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20160930185139.15c8be66@bbrezillon>
On Friday 30 September 2016, Boris Brezillon wrote:
> > + /* copy into possibly unaligned OOB region with actual length */
> > + memcpy(data + bytes, eccdata, len);
>
> Is it better than
>
> for (i = 0; i < len; i += 4) {
> u32 val = __raw_readl(ecc->regs + ECC_ENCPAR(i / 4));
>
> memcpy(data + bytes + i, &val, min(len, 4));
> }
>
> I'm probably missing something, but what's the point of creating a
> temporary buffer of 112 bytes on the stack since you'll have to copy
> this data to the oob buffer at some point?
I tried something like that first, but wasn't too happy with it for
a number of small reasons:
- __raw_readl in a driver is not usually the right API, __memcpy32_from_io
uses it internally, but it's better for a driver not to rely on that,
in case we need some barriers (which we may in factt need for other drivers).
- the min(len,4) expression is incorrect, fixing that makes it more complicated
again
- I didn't like to call memcpy() multiple times, as that might get turned
into an external function call (the compiler is free to optimize small
memcpy calls or not).
I agree that he 112 byte buffer isn't ideal either, it just seemed to
be the lesser annoyance.
Arnd
^ permalink raw reply
* Re: [PATCH] mtd: mtk: avoid warning in mtk_ecc_encode
From: Boris Brezillon @ 2016-09-30 16:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David Woodhouse, Brian Norris, Richard Weinberger,
Matthias Brugger, linux-mtd, linux-arm-kernel, linux-mediatek,
linux-kernel
In-Reply-To: <20160930163429.380785-1-arnd@arndb.de>
Hi Arnd,
On Fri, 30 Sep 2016 18:33:02 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> warning for the mtk_ecc_encode function:
>
> drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The function for some reason contains a double byte swap on big-endian
> builds to get the OOB data into the correct order again, and is written
> in a slightly confusing way.
>
> Using a simple memcpy32_fromio() to read the data simplifies it a lot
> so it becomes more readable and produces no warning. However, the
> output might not have 32-bit alignment, so we have to use another
> memcpy to avoid taking alignment faults or writing beyond the end
> of the array.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/mtd/nand/mtk_ecc.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index d54f666417e1..237c83124a7d 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -366,9 +366,9 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> u8 *data, u32 bytes)
> {
> dma_addr_t addr;
> - u8 *p;
> - u32 len, i, val;
> - int ret = 0;
> + u32 len;
> + u8 eccdata[112];
> + int ret;
>
> addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> ret = dma_mapping_error(ecc->dev, addr);
> @@ -393,14 +393,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>
> /* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> - p = data + bytes;
>
> - /* write the parity bytes generated by the ECC back to the OOB region */
> - for (i = 0; i < len; i++) {
> - if ((i % 4) == 0)
> - val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> - p[i] = (val >> ((i % 4) * 8)) & 0xff;
> - }
> + /* write the parity bytes generated by the ECC back to temp buffer */
> + __ioread32_copy(eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> +
> + /* copy into possibly unaligned OOB region with actual length */
> + memcpy(data + bytes, eccdata, len);
Is it better than
for (i = 0; i < len; i += 4) {
u32 val = __raw_readl(ecc->regs + ECC_ENCPAR(i / 4));
memcpy(data + bytes + i, &val, min(len, 4));
}
I'm probably missing something, but what's the point of creating a
temporary buffer of 112 bytes on the stack since you'll have to copy
this data to the oob buffer at some point?
> timeout:
>
> dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
^ permalink raw reply
* [PATCH] mtd: mtk: avoid warning in mtk_ecc_encode
From: Arnd Bergmann @ 2016-09-30 16:33 UTC (permalink / raw)
To: Boris Brezillon, David Woodhouse, Brian Norris
Cc: Arnd Bergmann, Richard Weinberger, linux-kernel, linux-mediatek,
Matthias Brugger, linux-mtd, linux-arm-kernel
When building with -Wmaybe-uninitialized, gcc produces a silly false positive
warning for the mtk_ecc_encode function:
drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
The function for some reason contains a double byte swap on big-endian
builds to get the OOB data into the correct order again, and is written
in a slightly confusing way.
Using a simple memcpy32_fromio() to read the data simplifies it a lot
so it becomes more readable and produces no warning. However, the
output might not have 32-bit alignment, so we have to use another
memcpy to avoid taking alignment faults or writing beyond the end
of the array.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/mtd/nand/mtk_ecc.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index d54f666417e1..237c83124a7d 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -366,9 +366,9 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
u8 *data, u32 bytes)
{
dma_addr_t addr;
- u8 *p;
- u32 len, i, val;
- int ret = 0;
+ u32 len;
+ u8 eccdata[112];
+ int ret;
addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
ret = dma_mapping_error(ecc->dev, addr);
@@ -393,14 +393,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
- p = data + bytes;
- /* write the parity bytes generated by the ECC back to the OOB region */
- for (i = 0; i < len; i++) {
- if ((i % 4) == 0)
- val = readl(ecc->regs + ECC_ENCPAR(i / 4));
- p[i] = (val >> ((i % 4) * 8)) & 0xff;
- }
+ /* write the parity bytes generated by the ECC back to temp buffer */
+ __ioread32_copy(eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
+
+ /* copy into possibly unaligned OOB region with actual length */
+ memcpy(data + bytes, eccdata, len);
timeout:
dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
--
2.9.0
^ permalink raw reply related
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-30 9:47 UTC (permalink / raw)
To: CK Hu
Cc: Rob Herring, Matthias Brugger, Jassi Brar, Daniel Kurtz,
Sascha Hauer, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, cawa cheng, Bibby Hsieh, YT Shen,
Daoyuan Huang, Damon Chu, Josh-YC Liu, Glory Hung
In-Reply-To: <1475226691.13398.35.camel@mtksdaap41>
On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
> Hi, HS:
>
> One comment inline
>
> On Fri, 2016-09-30 at 16:56 +0800, Horng-Shyang Liao wrote:
> > Hi CK,
> >
> > Please see my inline reply.
> >
> > On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> > > Hi, HS:
> > >
> > > On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > > > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > > > CMDQ is used to help write registers with critical time limitation,
> > > > such as updating display configuration during the vblank. It controls
> > > > Global Command Engine (GCE) hardware to achieve this requirement.
> > > > Currently, CMDQ only supports display related hardwares, but we expect
> > > > it can be extended to other hardwares for future requirements.
> > > >
> > > > Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > Signed-off-by: CK Hu <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > ---
> > >
> > > [snip...]
> > >
> > > > +
> > > > +struct cmdq_task {
> > > > + struct cmdq *cmdq;
> > > > + struct list_head list_entry;
> > > > + void *va_base;
> > > > + dma_addr_t pa_base;
> > > > + size_t cmd_buf_size; /* command occupied size */
> > > > + size_t buf_size; /* real buffer size */
> > > > + bool finalized;
> > > > + struct cmdq_thread *thread;
> > >
> > > I think thread info could be removed from cmdq_task. Only
> > > cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> > > task->thread and caller of both function has the thread info. So you
> > > could just pass thread info into these two function and remove thread
> > > info in cmdq_task.
> >
> > This modification will remove 1 pointer but add 2 pointers. Moreover,
> > more pointers will need to be delivered between functions for future
> > extension. IMHO, it would be better to keep thread pointer inside
> > cmdq_task.
> >
> > > > + struct cmdq_task_cb cb;
> > >
> > > I think this callback function is equal to mailbox client tx_done
> > > callback. It's better to use already-defined interface rather than
> > > creating your own.
> >
> > This is because CMDQ driver allows different callback functions for
> > different tasks, but mailbox only allows one callback function per
> > channel. But, I think I can add a wrapper for tx_done to call CMDQ
> > callback functions. So, I will use tx_done in CMDQ v15.
>
> Up to now, one callback function for one channel is enough for DRM. So
> 'different callback function for different sent-message' looks like an
> advanced function. Maybe you should not include it in first patch.
>
> Regards,
> CK
Hi CK,
OK. I will do it.
Thanks,
HS
[snip...]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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