Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
From: Jassi Brar @ 2017-01-26  4:38 UTC (permalink / raw)
  To: HS Liao
  Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
	Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, linux-mediatek,
	srv_heupstream, Sascha Hauer, Philipp Zabel, Nicolas Boichat,
	CK HU, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang, Damon Chu
In-Reply-To: <1483499169-16329-3-git-send-email-hs.liao@mediatek.com>

On Wed, Jan 4, 2017 at 8:36 AM, HS Liao <hs.liao@mediatek.com> wrote:

> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> new file mode 100644
> index 0000000..747bcd3
> --- /dev/null
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c

...

> +static void cmdq_task_exec(struct cmdq_pkt *pkt, struct cmdq_thread *thread)
> +{
> +       struct cmdq *cmdq;
> +       struct cmdq_task *task;
> +       unsigned long curr_pa, end_pa;
> +
> +       cmdq = dev_get_drvdata(thread->chan->mbox->dev);
> +
> +       /* Client should not flush new tasks if suspended. */
> +       WARN_ON(cmdq->suspended);
> +
> +       task = kzalloc(sizeof(*task), GFP_ATOMIC);
> +       task->cmdq = cmdq;
> +       INIT_LIST_HEAD(&task->list_entry);
> +       task->pa_base = dma_map_single(cmdq->mbox.dev, pkt->va_base,
> +                                      pkt->cmd_buf_size, DMA_TO_DEVICE);
>
You seem to parse the requests and responses, that should ideally be
done in client driver.
Also, we are here in atomic context, can you move it in client driver
(before the spin_lock)?
Maybe by adding a new 'pa_base' member as well in 'cmdq_pkt'.

....
> +
> +       cmdq->mbox.num_chans = CMDQ_THR_MAX_COUNT;
> +       cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> +       cmdq->mbox.of_xlate = cmdq_xlate;
> +
> +       /* make use of TXDONE_BY_ACK */
> +       cmdq->mbox.txdone_irq = false;
> +       cmdq->mbox.txdone_poll = false;
> +
> +       for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
>
You mean  i < CMDQ_THR_MAX_COUNT

> +               cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> +                               CMDQ_THR_SIZE * i;
> +               INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
>
You seem the queue mailbox requests in this controller driver? why not
use the mailbox api for that?

> +               init_timer(&cmdq->thread[i].timeout);
> +               cmdq->thread[i].timeout.function = cmdq_thread_handle_timeout;
> +               cmdq->thread[i].timeout.data = (unsigned long)&cmdq->thread[i];
>
Here again... you seem to ignore the polling mechanism provided by the
mailbox api, and implement your own.


> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> new file mode 100644
> index 0000000..3433c64
> --- /dev/null
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
....
> +
> +struct cmdq_pkt {
> +       void                    *va_base;
>
maybe add 'pa_base' here so you don't have to do dma_map_single() in send_data

> +       size_t                  cmd_buf_size; /* command occupied size */
> +       size_t                  buf_size; /* real buffer size */
> +       struct cmdq_task_cb     cb;
> +};
> +
> +#endif /* __MTK_CMDQ_MAILBOX_H__ */
> --
> 1.9.1
>

^ permalink raw reply

* Re: [PATCH v20 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2017-01-26  8:37 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
	Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, linux-mediatek,
	srv_heupstream, Sascha Hauer, Philipp Zabel, Nicolas Boichat,
	CK HU, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang, Damon Chu,
	Jo
In-Reply-To: <CABb+yY23TUB8NpmpiO=cCmVFAVJMqKPfGgX14j+_Yj7tJvMOuQ@mail.gmail.com>

Hi Jassi,

On Thu, 2017-01-26 at 10:08 +0530, Jassi Brar wrote:
> On Wed, Jan 4, 2017 at 8:36 AM, HS Liao <hs.liao@mediatek.com> wrote:
> 
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > new file mode 100644
> > index 0000000..747bcd3
> > --- /dev/null
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> 
> ...
> 
> > +static void cmdq_task_exec(struct cmdq_pkt *pkt, struct cmdq_thread *thread)
> > +{
> > +       struct cmdq *cmdq;
> > +       struct cmdq_task *task;
> > +       unsigned long curr_pa, end_pa;
> > +
> > +       cmdq = dev_get_drvdata(thread->chan->mbox->dev);
> > +
> > +       /* Client should not flush new tasks if suspended. */
> > +       WARN_ON(cmdq->suspended);
> > +
> > +       task = kzalloc(sizeof(*task), GFP_ATOMIC);
> > +       task->cmdq = cmdq;
> > +       INIT_LIST_HEAD(&task->list_entry);
> > +       task->pa_base = dma_map_single(cmdq->mbox.dev, pkt->va_base,
> > +                                      pkt->cmd_buf_size, DMA_TO_DEVICE);
> >
> You seem to parse the requests and responses, that should ideally be
> done in client driver.
> Also, we are here in atomic context, can you move it in client driver
> (before the spin_lock)?
> Maybe by adding a new 'pa_base' member as well in 'cmdq_pkt'.

will do

> ....
> > +
> > +       cmdq->mbox.num_chans = CMDQ_THR_MAX_COUNT;
> > +       cmdq->mbox.ops = &cmdq_mbox_chan_ops;
> > +       cmdq->mbox.of_xlate = cmdq_xlate;
> > +
> > +       /* make use of TXDONE_BY_ACK */
> > +       cmdq->mbox.txdone_irq = false;
> > +       cmdq->mbox.txdone_poll = false;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
> >
> You mean  i < CMDQ_THR_MAX_COUNT

will do

> > +               cmdq->thread[i].base = cmdq->base + CMDQ_THR_BASE +
> > +                               CMDQ_THR_SIZE * i;
> > +               INIT_LIST_HEAD(&cmdq->thread[i].task_busy_list);
> >
> You seem the queue mailbox requests in this controller driver? why not
> use the mailbox api for that?
> 
> > +               init_timer(&cmdq->thread[i].timeout);
> > +               cmdq->thread[i].timeout.function = cmdq_thread_handle_timeout;
> > +               cmdq->thread[i].timeout.data = (unsigned long)&cmdq->thread[i];
> >
> Here again... you seem to ignore the polling mechanism provided by the
> mailbox api, and implement your own.

The queue is used to record the tasks which are flushed into CMDQ
hardware (GCE). We are handling time critical tasks, so we have to
queue them in GCE rather than a software queue (e.g. mailbox buffer).
Let me use display as an example. Many display tasks are flushed into
CMDQ to wait next vsync event. When vsync event is triggered by display
hardware, GCE needs to process all flushed tasks "within vblank" to
prevent garbage on screen. This is all done by GCE (without CPU)
to fulfill time critical requirement. After GCE finish its work,
it will generate interrupts, and then CMDQ driver will let clients know
which tasks are done.

We have discussed the similar thing before.
Please take a look at the following link,
especially from 2016/10/5 to.2016/10/11 about tx_done.
https://patchwork.kernel.org/patch/9312953/

> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > new file mode 100644
> > index 0000000..3433c64
> > --- /dev/null
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> ....
> > +
> > +struct cmdq_pkt {
> > +       void                    *va_base;
> >
> maybe add 'pa_base' here so you don't have to do dma_map_single() in send_data

will do

> > +       size_t                  cmd_buf_size; /* command occupied size */
> > +       size_t                  buf_size; /* real buffer size */
> > +       struct cmdq_task_cb     cb;
> > +};
> > +
> > +#endif /* __MTK_CMDQ_MAILBOX_H__ */
> > --
> > 1.9.1
> >

Thanks,
HS

^ permalink raw reply

* [PATCH] mmc: host: constify mmc_host_ops structures
From: Bhumika Goyal @ 2017-01-26 14:50 UTC (permalink / raw)
  To: julia.lawall, ulf.hansson, matthias.bgg, saschasommer,
	maxime.ripard, wens, tony.olech, linux, linux-mmc, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-usb
  Cc: Bhumika Goyal

Declare mmc_host_ops structures as const as they are only stored in the
ops field of a mmc_host structure. This field is of type const, so
mmc_host_ops structures having this property can be made const too.
Done using Coccinelle:

@r disable optional_qualifier@
identifier x;
position p;
@@
static struct mmc_host_ops x@p={...};

@ok@
struct mmc_host mmc;
identifier r.x;
position p;
@@
mmc.ops=&x@p;

@bad@
position p != {r.p,ok.p};
identifier r.x;
@@
x@p

@depends on !bad disable optional_qualifier@
identifier r.x;
@@
+const
struct mmc_host_ops x;

File size details before and after patching.
First line of every .o file shows the file size before patching
and second line shows the size after patching.

   text	   data	    bss	    dec	    hex	filename

   4710	    344	      0	   5054	   13be	drivers/mmc/host/moxart-mmc.o
   4854	    192	      0	   5046	   13b6	drivers/mmc/host/moxart-mmc.o

  10743	    344	      0	  11087	   2b4f	drivers/mmc/host/mtk-sd.o
  10887	    192	      0	  11079	   2b47	drivers/mmc/host/mtk-sd.o

   2760	    376	      4	   3140	    c44	drivers/mmc/host/sdricoh_cs.o
   2920	    240	      4	   3164	    c5c	drivers/mmc/host/sdricoh_cs.o

   9485	    344	      0	   9829	   2665	drivers/mmc/host/sh_mmcif.o
   9645	    192	      0	   9837	   266d	drivers/mmc/host/sh_mmcif.o

   7281	    344	      0	   7625	   1dc9	drivers/mmc/host/sunxi-mmc.o
   7441	    192	      0	   7633	   1dd1	drivers/mmc/host/sunxi-mmc.o

   4982	    408	      0	   5390	   150e	drivers/mmc/host/toshsd.o
   5142	    264	      0	   5406	   151e	drivers/mmc/host/toshsd.o

  11239	    344	      0	  11583	   2d3f	drivers/mmc/host/usdhi6rol0.o
  11391	    192	      0	  11583	   2d3f	drivers/mmc/host/usdhi6rol0.o

  19444	    541	     29	  20014	   4e2e	drivers/mmc/host/vub300.o
  19604	    381	     29	  20014	   4e2e	drivers/mmc/host/vub300.o

   5487	    376	      0	   5863	   16e7	drivers/mmc/host/wmt-sdmmc.o
   5639	    220	      0	   5859	   16e3	drivers/mmc/host/wmt-sdmmc.o

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/mmc/host/moxart-mmc.c | 2 +-
 drivers/mmc/host/mtk-sd.c     | 2 +-
 drivers/mmc/host/sdricoh_cs.c | 2 +-
 drivers/mmc/host/sh_mmcif.c   | 2 +-
 drivers/mmc/host/sunxi-mmc.c  | 2 +-
 drivers/mmc/host/toshsd.c     | 2 +-
 drivers/mmc/host/usdhi6rol0.c | 2 +-
 drivers/mmc/host/vub300.c     | 2 +-
 drivers/mmc/host/wmt-sdmmc.c  | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index bbad309..ac07dab 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -548,7 +548,7 @@ static int moxart_get_ro(struct mmc_host *mmc)
 	return !!(readl(host->base + REG_STATUS) & WRITE_PROT);
 }
 
-static struct mmc_host_ops moxart_ops = {
+static const struct mmc_host_ops moxart_ops = {
 	.request = moxart_request,
 	.set_ios = moxart_set_ios,
 	.get_ro = moxart_get_ro,
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 10ef2ae..fa0c331 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -1467,7 +1467,7 @@ static void msdc_hw_reset(struct mmc_host *mmc)
 	sdr_clr_bits(host->base + EMMC_IOCON, 1);
 }
 
-static struct mmc_host_ops mt_msdc_ops = {
+static const struct mmc_host_ops mt_msdc_ops = {
 	.post_req = msdc_post_req,
 	.pre_req = msdc_pre_req,
 	.request = msdc_ops_request,
diff --git a/drivers/mmc/host/sdricoh_cs.c b/drivers/mmc/host/sdricoh_cs.c
index 5ff26ab..6732753 100644
--- a/drivers/mmc/host/sdricoh_cs.c
+++ b/drivers/mmc/host/sdricoh_cs.c
@@ -388,7 +388,7 @@ static int sdricoh_get_ro(struct mmc_host *mmc)
 	return (status & STATUS_CARD_LOCKED);
 }
 
-static struct mmc_host_ops sdricoh_ops = {
+static const struct mmc_host_ops sdricoh_ops = {
 	.request = sdricoh_request,
 	.set_ios = sdricoh_set_ios,
 	.get_ro = sdricoh_get_ro,
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 9007784..93cff28 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1095,7 +1095,7 @@ static int sh_mmcif_get_cd(struct mmc_host *mmc)
 		return p->get_cd(host->pd);
 }
 
-static struct mmc_host_ops sh_mmcif_ops = {
+static const struct mmc_host_ops sh_mmcif_ops = {
 	.request	= sh_mmcif_request,
 	.set_ios	= sh_mmcif_set_ios,
 	.get_cd		= sh_mmcif_get_cd,
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index b1d1303..e75e11a 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1033,7 +1033,7 @@ static int sunxi_mmc_card_busy(struct mmc_host *mmc)
 	return !!(mmc_readl(host, REG_STAS) & SDXC_CARD_DATA_BUSY);
 }
 
-static struct mmc_host_ops sunxi_mmc_ops = {
+static const struct mmc_host_ops sunxi_mmc_ops = {
 	.request	 = sunxi_mmc_request,
 	.set_ios	 = sunxi_mmc_set_ios,
 	.get_ro		 = mmc_gpio_get_ro,
diff --git a/drivers/mmc/host/toshsd.c b/drivers/mmc/host/toshsd.c
index 553ef41..dd961c5 100644
--- a/drivers/mmc/host/toshsd.c
+++ b/drivers/mmc/host/toshsd.c
@@ -550,7 +550,7 @@ static int toshsd_get_cd(struct mmc_host *mmc)
 	return !!(ioread16(host->ioaddr + SD_CARDSTATUS) & SD_CARD_PRESENT_0);
 }
 
-static struct mmc_host_ops toshsd_ops = {
+static const struct mmc_host_ops toshsd_ops = {
 	.request = toshsd_request,
 	.set_ios = toshsd_set_ios,
 	.get_ro = toshsd_get_ro,
diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
index 1bd5f1a..64da6a8 100644
--- a/drivers/mmc/host/usdhi6rol0.c
+++ b/drivers/mmc/host/usdhi6rol0.c
@@ -1185,7 +1185,7 @@ static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
 	return ret;
 }
 
-static struct mmc_host_ops usdhi6_ops = {
+static const struct mmc_host_ops usdhi6_ops = {
 	.request	= usdhi6_request,
 	.set_ios	= usdhi6_set_ios,
 	.get_cd		= usdhi6_get_cd,
diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index bb3e0d1..ec23121 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2083,7 +2083,7 @@ static void vub300_init_card(struct mmc_host *mmc, struct mmc_card *card)
 	dev_info(&vub300->udev->dev, "NO host QUIRKS for this card\n");
 }
 
-static struct mmc_host_ops vub300_mmc_ops = {
+static const struct mmc_host_ops vub300_mmc_ops = {
 	.request = vub300_mmc_request,
 	.set_ios = vub300_mmc_set_ios,
 	.get_ro = vub300_mmc_get_ro,
diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
index 5af0055..1bf725a 100644
--- a/drivers/mmc/host/wmt-sdmmc.c
+++ b/drivers/mmc/host/wmt-sdmmc.c
@@ -725,7 +725,7 @@ static int wmt_mci_get_cd(struct mmc_host *mmc)
 	return !(cd ^ priv->cd_inverted);
 }
 
-static struct mmc_host_ops wmt_mci_ops = {
+static const struct mmc_host_ops wmt_mci_ops = {
 	.request = wmt_mci_request,
 	.set_ios = wmt_mci_set_ios,
 	.get_ro = wmt_mci_get_ro,
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] pinctrl: mediatek: Use real dependencies
From: Linus Walleij @ 2017-01-26 15:26 UTC (permalink / raw)
  To: Jean Delvare
  Cc: linux-gpio@vger.kernel.org,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Biao Huang, James Liao, Andreas Färber
In-Reply-To: <20170125103209.1d97b0bd@endymion>

On Wed, Jan 25, 2017 at 10:32 AM, Jean Delvare <jdelvare@suse.de> wrote:

> Do not hide pinctrl drivers for Mediatek platforms using
> conditionals. Doing so actually leaves the symbols present (but
> always disabled) on all other platforms, which is confusing and
> inefficient. Better use real dependencies so that the symbols do not
> exist at all on platforms where they are not relevant.
>
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Reported-by: Andreas Färber <afaerber@suse.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>

Patch applied with Matthias' review tag.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 1/2] spi: When no dma_chan map buffers with spi_master's parent
From: Daniel Kurtz @ 2017-01-26 16:21 UTC (permalink / raw)
  Cc: dtor, groeck, drinkcat, Robin Murphy, Daniel Kurtz, Leilk Liu,
	Mark Brown, Matthias Brugger, open list:SPI SUBSYSTEM, open list,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support

Back before commit 1dccb598df54 ("arm64: simplify dma_get_ops"), for
arm64, devices for which dma_ops were not explicitly set were automatically
configured to use swiotlb_dma_ops, since this was hard-coded as the
global "dma_ops" in arm64_dma_init().

Now that global "dma_ops" has been removed, all devices much have their
dma_ops explicitly set by a call to arch_setup_dma_ops(), otherwise the
device is assigned dummy_dma_ops, and thus calls to map_sg for such a
device will fail (return 0).

Mediatek SPI uses DMA but does not use a dma channel.  Support for this
was added by commit c37f45b5f1cd ("spi: support spi without dma channel
to use can_dma()"), which uses the master_spi dev to DMA map buffers.

The master_spi device is not a platform device, rather it is created
in spi_alloc_device(), and therefore its dma_ops are never set.

Therefore, when the mediatek SPI driver when it does DMA (for large SPI
transactions > 32 bytes), SPI will use spi_map_buf()->dma_map_sg() to
map the buffer for use in DMA.  But dma_map_sg()->dma_map_sg_attrs() returns
0, because ops->map_sg is dummy_dma_ops->__dummy_map_sg, and hence
spi_map_buf() returns -ENOMEM (-12).

Fix this by using the real spi_master's parent device which should be a
real physical device with DMA properties.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Fixes: c37f45b5f1cd ("spi: support spi without dma channel to use can_dma()")
Cc: Leilk Liu <leilk.liu@mediatek.com>
---
 drivers/spi/spi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 656dd3e3220c..f4d412e48e1c 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -805,12 +805,12 @@ static int __spi_map_msg(struct spi_master *master, struct spi_message *msg)
 	if (master->dma_tx)
 		tx_dev = master->dma_tx->device->dev;
 	else
-		tx_dev = &master->dev;
+		tx_dev = master->dev.parent;
 
 	if (master->dma_rx)
 		rx_dev = master->dma_rx->device->dev;
 	else
-		rx_dev = &master->dev;
+		rx_dev = master->dev.parent;
 
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		if (!master->can_dma(master, msg->spi, xfer))
@@ -852,12 +852,12 @@ static int __spi_unmap_msg(struct spi_master *master, struct spi_message *msg)
 	if (master->dma_tx)
 		tx_dev = master->dma_tx->device->dev;
 	else
-		tx_dev = &master->dev;
+		tx_dev = master->dev.parent;
 
 	if (master->dma_rx)
 		rx_dev = master->dma_rx->device->dev;
 	else
-		rx_dev = &master->dev;
+		rx_dev = master->dev.parent;
 
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		if (!master->can_dma(master, msg->spi, xfer))
-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply related

* [PATCH 2/2] spi: mediatek: Only do dma for 4-byte aligned buffers
From: Daniel Kurtz @ 2017-01-26 16:21 UTC (permalink / raw)
  Cc: dtor-F7+t8E8rja9g9hUCZPvPmw, groeck-F7+t8E8rja9g9hUCZPvPmw,
	drinkcat-F7+t8E8rja9g9hUCZPvPmw, Robin Murphy, Daniel Kurtz,
	Leilk Liu, Mark Brown, Matthias Brugger, open list:SPI SUBSYSTEM,
	open list, moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support
In-Reply-To: <20170126162154.124287-1-djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Mediatek SPI DMA only works when tx and rx buffer addresses are 4-byte
aligned.

Unaligned DMA transactions appeared to work previously, since we the
spi core was incorrectly using the spi_master device for dma, which
had a 0 dma_mask, and therefore the swiotlb dma map operations were
falling back to using bounce buffers.  Since each DMA transaction would
use its own buffer, the mapped starting address of each transaction was
always aligned.  When doing real DMA, the mapped address will share the
alignment of the raw tx/rx buffer provided by the SPI user, which may or
may not be aligned.

If a buffer is not aligned, we cannot use DMA, and must use FIFO based
transaction instead.

So, this patch implements a scheme that allows using the FIFO for
arbitrary length transactions (larger than the 32-byte FIFO size) by
reloading the FIFO in the interrupt handler.

Signed-off-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Leilk Liu <leilk.liu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/spi/spi-mt65xx.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 899d7a8f0889..4b592fc25194 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -73,7 +73,7 @@
 #define MTK_SPI_IDLE 0
 #define MTK_SPI_PAUSED 1
 
-#define MTK_SPI_MAX_FIFO_SIZE 32
+#define MTK_SPI_MAX_FIFO_SIZE 32U
 #define MTK_SPI_PACKET_SIZE 1024
 
 struct mtk_spi_compatible {
@@ -333,7 +333,7 @@ static int mtk_spi_fifo_transfer(struct spi_master *master,
 	struct mtk_spi *mdata = spi_master_get_devdata(master);
 
 	mdata->cur_transfer = xfer;
-	mdata->xfer_len = xfer->len;
+	mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, xfer->len);
 	mtk_spi_prepare_transfer(master, xfer);
 	mtk_spi_setup_packet(master);
 
@@ -410,7 +410,10 @@ static bool mtk_spi_can_dma(struct spi_master *master,
 			    struct spi_device *spi,
 			    struct spi_transfer *xfer)
 {
-	return xfer->len > MTK_SPI_MAX_FIFO_SIZE;
+	/* Buffers for DMA transactions must be 4-byte aligned */
+	return (xfer->len > MTK_SPI_MAX_FIFO_SIZE &&
+		(unsigned long)xfer->tx_buf % 4 == 0 &&
+		(unsigned long)xfer->rx_buf % 4 == 0);
 }
 
 static int mtk_spi_setup(struct spi_device *spi)
@@ -451,7 +454,33 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
 					&reg_val, remainder);
 			}
 		}
-		spi_finalize_current_transfer(master);
+
+		trans->len -= mdata->xfer_len;
+		if (!trans->len) {
+			spi_finalize_current_transfer(master);
+			return IRQ_HANDLED;
+		}
+
+		if (trans->tx_buf)
+			trans->tx_buf += mdata->xfer_len;
+		if (trans->rx_buf)
+			trans->rx_buf += mdata->xfer_len;
+
+		mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, trans->len);
+		mtk_spi_setup_packet(master);
+
+		cnt = trans->len / 4;
+		iowrite32_rep(mdata->base + SPI_TX_DATA_REG, trans->tx_buf, cnt);
+
+		remainder = trans->len % 4;
+		if (remainder > 0) {
+			reg_val = 0;
+			memcpy(&reg_val, trans->tx_buf + (cnt * 4), remainder);
+			writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+		}
+
+		mtk_spi_enable_transfer(master);
+
 		return IRQ_HANDLED;
 	}
 
-- 
2.11.0.483.g087da7b7c-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 related

* Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device
From: Daniel Kurtz @ 2017-01-26 16:29 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Leilk Liu, open list, open list:SPI SUBSYSTEM, Matthias Brugger,
	Mark Brown, moderated list:ARM/Mediatek SoC support, groeck,
	Dmitry Torokhov, moderated list:ARM/Mediatek SoC support
In-Reply-To: <0548db75-042b-4bef-02db-767ea9ccd7e4@arm.com>

Hi Robin,

On Thu, Jan 26, 2017 at 1:59 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 25/01/17 10:24, Daniel Kurtz wrote:
> > Hi Robin,
> >
> > On Tue, Jan 24, 2017 at 11:14 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> >> Hi Dan,
> >
> > [snip...]
> >
> >>> And I really don't know why we needed to set the coherent_dma_mask to 0 to
> >>> avoid SPI transaction errors.
> >>
> >> Following what I mentioned above, "git grep dma_alloc drivers/spi" makes
> >> it seem like coherent DMA isn't used anywhere relevant, which is rather
> >> puzzling. Unless of course somewhere along the line somebody's done the
> >> dev->dma_mask = &dev->dma_coherent_mask hack, with the result that
> >> writing one mask hits both, making *all* DMA fail and big transfers fall
> >> back to PIO.
> >
> > You mean this last line?
> >
> >>> @@ -575,6 +576,10 @@ static int mtk_spi_probe(struct platform_device *pdev)
> >>>               goto err_put_master;
> >>>       }
> >>>
> >>> +     /* Call of_dma_configure to set up spi_master's dma_ops */
> >>> +     of_dma_configure(&master->dev, master->dev.of_node);
> >>> +     /* But explicitly set the coherent_dma_mask to 0 */
> >>> +     master->dev.coherent_dma_mask = 0;
> >>>       if (!pdev->dev.dma_mask)
> >>>               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> >                                   ^^^^^
>
> Ha, I totally failed to spot that!
>
> > As predicted, setting the dma_mask = 0 causes "all DMA to fail".  In
> > particular, dma_capable() always returns false, so
> > swiotlb_map_sg_attrs() takes the map_single() path, instead of just
> > assigning:
> >
> >    sg->dma_address = dev_addr;
> >
> > swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems,
> >                      enum dma_data_direction dir, unsigned long attrs)
> > {
> >         struct scatterlist *sg;
> >         int i;
> >
> >         BUG_ON(dir == DMA_NONE);
> >
> >         for_each_sg(sgl, sg, nelems, i) {
> >                 phys_addr_t paddr = sg_phys(sg);
> >                 dma_addr_t dev_addr = phys_to_dma(hwdev, paddr);
> >
> >                 if (swiotlb_force == SWIOTLB_FORCE ||
> >                     !dma_capable(hwdev, dev_addr, sg->length)) {
> >                         phys_addr_t map = map_single(hwdev, sg_phys(sg),
> >                                                      sg->length, dir, attrs);
> >                         ...
> >                         }
> >                         sg->dma_address = phys_to_dma(hwdev, map);
> >                 } else
> >                         sg->dma_address = dev_addr;
> >                 sg_dma_len(sg) = sg->length;
> >         }
> >         return nelems;
> > }
> >
> > So, I think this means that the only reason the MTK SPI driver ever
> > worked before was that it was tested on an older kernel, so the
> > spi_master was defaulting to swiotlb_dma_ops with a 0 dma_mask, and
> > therefore it was using SWIOTLB bounce buffers (via 'map_single'), and
> > not actually ever doing real DMA.
>
> Well, it's still "real DMA" if the device is able to slurp data out of
> the bounce buffer. That suggests there might be some mismatch between
> the default DMA mask it's getting given and the actual hardware
> capability (i.e. the bounce buffer happens to fall somewhere accessible,
> but other addresses may not be) - is crazy 33-bit mode involved here?

AFAICT, the Mediatek SPI does not have a 33-bit mode.  The Mediatek
SPI DMA registers are only 32-bits wide, so the default 0xffffffff
dma_mask is correct.  For larger addresses, swiotlb properly falls
back to using a bounce buffer.

However, I did discover what the real problem was.  The Mediatek SPI
DMA does not like un-aligned addresses.
Teaching the driver to use the FIFO even for large buffers, and
forcing un-aligned buffers to use the FIFO makes the SPI very
reliable:

https://patchwork.kernel.org/patch/9539735/

Thanks for your help!

-Dan

^ permalink raw reply

* Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device
From: Dmitry Torokhov @ 2017-01-26 16:35 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: Leilk Liu, Dmitry Torokhov, open list, open list:SPI SUBSYSTEM,
	Guenter Roeck, Mark Brown,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Robin Murphy, moderated list:ARM/Mediatek SoC support
In-Reply-To: <CAGS+omDexcrnWL6v4bAQ3K2QSHKcwCs-WRCC+BUbGbXgVYbK9w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Jan 26, 2017 at 8:29 AM, Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Hi Robin,
>
> On Thu, Jan 26, 2017 at 1:59 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>>
>> On 25/01/17 10:24, Daniel Kurtz wrote:
>> > Hi Robin,
>> >
>> > On Tue, Jan 24, 2017 at 11:14 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> >> Hi Dan,
>> >
>> > [snip...]
>> >
>> >>> And I really don't know why we needed to set the coherent_dma_mask to 0 to
>> >>> avoid SPI transaction errors.
>> >>
>> >> Following what I mentioned above, "git grep dma_alloc drivers/spi" makes
>> >> it seem like coherent DMA isn't used anywhere relevant, which is rather
>> >> puzzling. Unless of course somewhere along the line somebody's done the
>> >> dev->dma_mask = &dev->dma_coherent_mask hack, with the result that
>> >> writing one mask hits both, making *all* DMA fail and big transfers fall
>> >> back to PIO.
>> >
>> > You mean this last line?
>> >
>> >>> @@ -575,6 +576,10 @@ static int mtk_spi_probe(struct platform_device *pdev)
>> >>>               goto err_put_master;
>> >>>       }
>> >>>
>> >>> +     /* Call of_dma_configure to set up spi_master's dma_ops */
>> >>> +     of_dma_configure(&master->dev, master->dev.of_node);
>> >>> +     /* But explicitly set the coherent_dma_mask to 0 */
>> >>> +     master->dev.coherent_dma_mask = 0;
>> >>>       if (!pdev->dev.dma_mask)
>> >>>               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>> >                                   ^^^^^
>>
>> Ha, I totally failed to spot that!
>>
>> > As predicted, setting the dma_mask = 0 causes "all DMA to fail".  In
>> > particular, dma_capable() always returns false, so
>> > swiotlb_map_sg_attrs() takes the map_single() path, instead of just
>> > assigning:
>> >
>> >    sg->dma_address = dev_addr;
>> >
>> > swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems,
>> >                      enum dma_data_direction dir, unsigned long attrs)
>> > {
>> >         struct scatterlist *sg;
>> >         int i;
>> >
>> >         BUG_ON(dir == DMA_NONE);
>> >
>> >         for_each_sg(sgl, sg, nelems, i) {
>> >                 phys_addr_t paddr = sg_phys(sg);
>> >                 dma_addr_t dev_addr = phys_to_dma(hwdev, paddr);
>> >
>> >                 if (swiotlb_force == SWIOTLB_FORCE ||
>> >                     !dma_capable(hwdev, dev_addr, sg->length)) {
>> >                         phys_addr_t map = map_single(hwdev, sg_phys(sg),
>> >                                                      sg->length, dir, attrs);
>> >                         ...
>> >                         }
>> >                         sg->dma_address = phys_to_dma(hwdev, map);
>> >                 } else
>> >                         sg->dma_address = dev_addr;
>> >                 sg_dma_len(sg) = sg->length;
>> >         }
>> >         return nelems;
>> > }
>> >
>> > So, I think this means that the only reason the MTK SPI driver ever
>> > worked before was that it was tested on an older kernel, so the
>> > spi_master was defaulting to swiotlb_dma_ops with a 0 dma_mask, and
>> > therefore it was using SWIOTLB bounce buffers (via 'map_single'), and
>> > not actually ever doing real DMA.
>>
>> Well, it's still "real DMA" if the device is able to slurp data out of
>> the bounce buffer. That suggests there might be some mismatch between
>> the default DMA mask it's getting given and the actual hardware
>> capability (i.e. the bounce buffer happens to fall somewhere accessible,
>> but other addresses may not be) - is crazy 33-bit mode involved here?
>
> AFAICT, the Mediatek SPI does not have a 33-bit mode.  The Mediatek
> SPI DMA registers are only 32-bits wide, so the default 0xffffffff
> dma_mask is correct.  For larger addresses, swiotlb properly falls
> back to using a bounce buffer.
>
> However, I did discover what the real problem was.  The Mediatek SPI
> DMA does not like un-aligned addresses.

Nobody doing DMA likes unaligned buffers. In fact, memory used for DMA
must be cacheline aligned, or you going to have trouble. What
component supplies unaligned buffers to SPI core for transfers?

Thanks,
Dmitry

^ permalink raw reply

* Re: [PATCH v2 1/2] Documentation: mtk-quadspi: update DT bindings
From: Cyrille Pitchen @ 2017-01-26 16:38 UTC (permalink / raw)
  To: Guochun Mao, Brian Norris, John Crispin
  Cc: David Woodhouse, Boris Brezillon, Marek Vasut, Richard Weinberger,
	Rob Herring, Mark Rutland, Matthias Brugger, Russell King,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1485315515-29942-2-git-send-email-guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Le 25/01/2017 à 04:38, Guochun Mao a écrit :
> Add "mediatek,mt2701-nor" for nor flash node's compatible.
> 
> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  .../devicetree/bindings/mtd/mtk-quadspi.txt        |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> index fb314f0..5ded66a 100644
> --- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -1,7 +1,13 @@
>  * Serial NOR flash controller for MTK MT81xx (and similar)
>  
>  Required properties:
> -- compatible: 	  should be "mediatek,mt8173-nor";
> +- compatible: 	  The possible values are:
> +		  "mediatek,mt2701-nor"
> +		  "mediatek,mt7623-nor"
> +		  "mediatek,mt8173-nor"
> +		  For mt8173, compatible should be "mediatek,mt8173-nor".
> +		  For every other SoC, should contain both the SoC-specific compatible string
> +		  and "mediatek,mt8173-nor".
>  - reg: 		  physical base address and length of the controller's register
>  - clocks: 	  the phandle of the clocks needed by the nor controller
>  - clock-names: 	  the names of the clocks
> 

Rob, is it ok for you if I take this patch in the spi-nor tree?

Best regards,

Cyrille
--
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 2/2] spi: mediatek: Only do dma for 4-byte aligned buffers
From: Dmitry Torokhov @ 2017-01-26 16:42 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: drinkcat-F7+t8E8rja9g9hUCZPvPmw, Leilk Liu, Dmitry Torokhov,
	open list, open list:SPI SUBSYSTEM, Matthias Brugger, Mark Brown,
	moderated list:ARM/Mediatek SoC support, Guenter Roeck,
	Robin Murphy, moderated list:ARM/Mediatek SoC support
In-Reply-To: <20170126162154.124287-2-djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Hi Daniel,

On Thu, Jan 26, 2017 at 8:21 AM, Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Mediatek SPI DMA only works when tx and rx buffer addresses are 4-byte
> aligned.
>
> Unaligned DMA transactions appeared to work previously, since we the
> spi core was incorrectly using the spi_master device for dma, which
> had a 0 dma_mask, and therefore the swiotlb dma map operations were
> falling back to using bounce buffers.  Since each DMA transaction would
> use its own buffer, the mapped starting address of each transaction was
> always aligned.  When doing real DMA, the mapped address will share the
> alignment of the raw tx/rx buffer provided by the SPI user, which may or
> may not be aligned.
>
> If a buffer is not aligned, we cannot use DMA, and must use FIFO based
> transaction instead.

>From spi.h:

/**
 * struct spi_transfer - a read/write buffer pair
 * @tx_buf: data to be written (dma-safe memory), or NULL
 * @rx_buf: data to be read (dma-safe memory), or NULL

DMA-safe memory is ___cacheline_aligned, so it appears to be user (of
the SPI) error providing "bad" memory for transfers.

Thanks,
Dmitry

^ permalink raw reply

* Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device
From: Daniel Kurtz @ 2017-01-26 16:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Leilk Liu, open list, open list:SPI SUBSYSTEM, Guenter Roeck,
	Mark Brown, moderated list:ARM/Mediatek SoC support,
	Matthias Brugger, Robin Murphy,
	moderated list:ARM/Mediatek SoC support
In-Reply-To: <CAE_wzQ8ChZtLait3WWin1nyRY5CQCzs8VHTeAcP8y2DqSGmb7Q@mail.gmail.com>

On Fri, Jan 27, 2017 at 12:35 AM, Dmitry Torokhov <dtor@chromium.org> wrote:
> On Thu, Jan 26, 2017 at 8:29 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>> Hi Robin,
>>
>> On Thu, Jan 26, 2017 at 1:59 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>>>
>>> On 25/01/17 10:24, Daniel Kurtz wrote:
>>> > Hi Robin,
>>> >
>>> > On Tue, Jan 24, 2017 at 11:14 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>>> >> Hi Dan,
>>> >
>>> > [snip...]
>>> >
>>> >>> And I really don't know why we needed to set the coherent_dma_mask to 0 to
>>> >>> avoid SPI transaction errors.
>>> >>
>>> >> Following what I mentioned above, "git grep dma_alloc drivers/spi" makes
>>> >> it seem like coherent DMA isn't used anywhere relevant, which is rather
>>> >> puzzling. Unless of course somewhere along the line somebody's done the
>>> >> dev->dma_mask = &dev->dma_coherent_mask hack, with the result that
>>> >> writing one mask hits both, making *all* DMA fail and big transfers fall
>>> >> back to PIO.
>>> >
>>> > You mean this last line?
>>> >
>>> >>> @@ -575,6 +576,10 @@ static int mtk_spi_probe(struct platform_device *pdev)
>>> >>>               goto err_put_master;
>>> >>>       }
>>> >>>
>>> >>> +     /* Call of_dma_configure to set up spi_master's dma_ops */
>>> >>> +     of_dma_configure(&master->dev, master->dev.of_node);
>>> >>> +     /* But explicitly set the coherent_dma_mask to 0 */
>>> >>> +     master->dev.coherent_dma_mask = 0;
>>> >>>       if (!pdev->dev.dma_mask)
>>> >>>               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>>> >                                   ^^^^^
>>>
>>> Ha, I totally failed to spot that!
>>>
>>> > As predicted, setting the dma_mask = 0 causes "all DMA to fail".  In
>>> > particular, dma_capable() always returns false, so
>>> > swiotlb_map_sg_attrs() takes the map_single() path, instead of just
>>> > assigning:
>>> >
>>> >    sg->dma_address = dev_addr;
>>> >
>>> > swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems,
>>> >                      enum dma_data_direction dir, unsigned long attrs)
>>> > {
>>> >         struct scatterlist *sg;
>>> >         int i;
>>> >
>>> >         BUG_ON(dir == DMA_NONE);
>>> >
>>> >         for_each_sg(sgl, sg, nelems, i) {
>>> >                 phys_addr_t paddr = sg_phys(sg);
>>> >                 dma_addr_t dev_addr = phys_to_dma(hwdev, paddr);
>>> >
>>> >                 if (swiotlb_force == SWIOTLB_FORCE ||
>>> >                     !dma_capable(hwdev, dev_addr, sg->length)) {
>>> >                         phys_addr_t map = map_single(hwdev, sg_phys(sg),
>>> >                                                      sg->length, dir, attrs);
>>> >                         ...
>>> >                         }
>>> >                         sg->dma_address = phys_to_dma(hwdev, map);
>>> >                 } else
>>> >                         sg->dma_address = dev_addr;
>>> >                 sg_dma_len(sg) = sg->length;
>>> >         }
>>> >         return nelems;
>>> > }
>>> >
>>> > So, I think this means that the only reason the MTK SPI driver ever
>>> > worked before was that it was tested on an older kernel, so the
>>> > spi_master was defaulting to swiotlb_dma_ops with a 0 dma_mask, and
>>> > therefore it was using SWIOTLB bounce buffers (via 'map_single'), and
>>> > not actually ever doing real DMA.
>>>
>>> Well, it's still "real DMA" if the device is able to slurp data out of
>>> the bounce buffer. That suggests there might be some mismatch between
>>> the default DMA mask it's getting given and the actual hardware
>>> capability (i.e. the bounce buffer happens to fall somewhere accessible,
>>> but other addresses may not be) - is crazy 33-bit mode involved here?
>>
>> AFAICT, the Mediatek SPI does not have a 33-bit mode.  The Mediatek
>> SPI DMA registers are only 32-bits wide, so the default 0xffffffff
>> dma_mask is correct.  For larger addresses, swiotlb properly falls
>> back to using a bounce buffer.
>>
>> However, I did discover what the real problem was.  The Mediatek SPI
>> DMA does not like un-aligned addresses.
>
> Nobody doing DMA likes unaligned buffers. In fact, memory used for DMA
> must be cacheline aligned, or you going to have trouble. What
> component supplies unaligned buffers to SPI core for transfers?

cros-ec-spi

>
> Thanks,
> Dmitry

^ permalink raw reply

* Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device
From: Mark Brown @ 2017-01-26 17:07 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: Dmitry Torokhov, Robin Murphy, Leilk Liu, open list,
	open list:SPI SUBSYSTEM, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Guenter Roeck,
	moderated list:ARM/Mediatek SoC support
In-Reply-To: <CAGS+omC-LnTx9Qt4Cv+7u-Oh=4syZy_mA-FqF8bUzN0DQmgAig@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 500 bytes --]

On Fri, Jan 27, 2017 at 12:52:15AM +0800, Daniel Kurtz wrote:
> On Fri, Jan 27, 2017 at 12:35 AM, Dmitry Torokhov <dtor@chromium.org> wrote:

> > Nobody doing DMA likes unaligned buffers. In fact, memory used for DMA
> > must be cacheline aligned, or you going to have trouble. What
> > component supplies unaligned buffers to SPI core for transfers?

> cros-ec-spi

That needs fixing then - I keep thinking we should add a warning to the
core for this, especially now we do DMA mapping in the core.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device
From: Dmitry Torokhov @ 2017-01-26 17:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: Daniel Kurtz, Dmitry Torokhov, Robin Murphy, Leilk Liu, open list,
	open list:SPI SUBSYSTEM, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Guenter Roeck,
	moderated list:ARM/Mediatek SoC support
In-Reply-To: <20170126170747.r6gegu3buqeedp54-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On Thu, Jan 26, 2017 at 9:07 AM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Jan 27, 2017 at 12:52:15AM +0800, Daniel Kurtz wrote:
>> On Fri, Jan 27, 2017 at 12:35 AM, Dmitry Torokhov <dtor-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
>> > Nobody doing DMA likes unaligned buffers. In fact, memory used for DMA
>> > must be cacheline aligned, or you going to have trouble. What
>> > component supplies unaligned buffers to SPI core for transfers?
>
>> cros-ec-spi
>
> That needs fixing then - I keep thinking we should add a warning to the
> core for this, especially now we do DMA mapping in the core.

OTOH the full buffer used by cros ec *is* DMA-safe (allocated with
kmalloc), it is that cors ec uses it at offset while transferring data
piece by piece. Do we want to support this?
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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] clk: mediatek: Fix MT8135 dependencies
From: Stephen Boyd @ 2017-01-27  0:04 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Matthias Brugger, James Liao,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Andreas Färber
In-Reply-To: <20170124130912.14375a32@endymion>

On 01/24, Jean Delvare wrote:
> The MT8135 is a 32-bit SoC, so only propose it on ARM architecture,
> not ARM64.
> 
> Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Fixes: 234d511d8c15 ("clk: mediatek: Add hardware dependency")
> Cc: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
> Cc: James Liao <jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v2] clk: mediatek: Fix MT2701 dependencies
From: Stephen Boyd @ 2017-01-27  0:04 UTC (permalink / raw)
  To: Jean Delvare
  Cc: James Liao, Erin Lo, Andreas Färber, Michael Turquette,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shunli Wang,
	Matthias Brugger, linux-clk-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170124130704.14015da3@endymion>

On 01/24, Jean Delvare wrote:
> If I say "no" to "Clock driver for Mediatek MT2701", I don't want to
> be asked individually about each sub-driver. No means no.
> 
> Additionally, this driver shouldn't be proposed at all on non-mediatek
> builds, unless build-testing.
> 
> Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Fixes: e9862118272a ("clk: mediatek: Add MT2701 clock support")
> Reviewed-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
> Reviewed-by: James Liao <jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Cc: Shunli Wang <shunli.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Cc: Erin Lo <erin.lo-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Cc: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH 0/2] clk: mediatek: add missing ethernet reset definition
From: Stephen Boyd @ 2017-01-27  0:14 UTC (permalink / raw)
  To: John Crispin
  Cc: James Liao, Michael Turquette, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
	Erin Lo, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1485175707-58528-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>

On 01/23, John Crispin wrote:
> When the clk code for mt2701 was merged, we forgot to add the reset
> controller of the ethernet subsystem. This series adds the missing code
> and header files required to reference the bits from within a devicetree
> file.
> 
> John Crispin (2):
>   clk: mediatek: add mt2701 ethernet reset
>   reset: mediatek: Add MT2701 ethsys reset controller include file
> 
>  drivers/clk/mediatek/clk-mt2701-eth.c     |    2 ++
>  include/dt-bindings/reset/mt2701-resets.h |    7 +++++++

Any planned DT users in the next merge window? Should I put this
into a topic branch? Do you want a clk maintainer ack? At the
least we need someone like James to ack this first.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH 0/2] clk: mediatek: add missing ethernet reset definition
From: John Crispin @ 2017-01-27  5:00 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: James Liao, Michael Turquette, linux-mediatek, Matthias Brugger,
	Erin Lo, linux-clk, linux-arm-kernel
In-Reply-To: <20170127001441.GS8801@codeaurora.org>



On 27/01/2017 01:14, Stephen Boyd wrote:
> On 01/23, John Crispin wrote:
>> When the clk code for mt2701 was merged, we forgot to add the reset
>> controller of the ethernet subsystem. This series adds the missing code
>> and header files required to reference the bits from within a devicetree
>> file.
>>
>> John Crispin (2):
>>   clk: mediatek: add mt2701 ethernet reset
>>   reset: mediatek: Add MT2701 ethsys reset controller include file
>>
>>  drivers/clk/mediatek/clk-mt2701-eth.c     |    2 ++
>>  include/dt-bindings/reset/mt2701-resets.h |    7 +++++++
> 
> Any planned DT users in the next merge window? Should I put this
> into a topic branch? Do you want a clk maintainer ack? At the
> least we need someone like James to ack this first.
> 

Hi Stephen,

the addition of the dtsi snippets for the ethernet core is pending due
to this patch. getting it into v4.11 would be geat so that i can sent
dtsi changes during the next merge window.

	John

^ permalink raw reply

* [PATCH] Soc: mediatek - Fix possible NULL derefrence.
From: Shailendra Verma @ 2017-01-27 11:17 UTC (permalink / raw)
  To: Matthias Brugger, John Crispin, Henry Chen, Daniel Kurtz,
	Arnd Bergmann, linux-arm-kernel, linux-mediatek, linux-kernel,
	p.shailesh, ashish.kalra, Shailendra Verma, Shailendra Verma
In-Reply-To: <CGME20170127111755epcas4p4f4bf0401e49ddd4563b0365b838d3dde@epcas4p4.samsung.com>

of_match_device could return NULL, and so can cause a NULL
pointer dereference later.

Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index a5f1093..c48db98 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1117,6 +1117,11 @@ static int pwrap_probe(struct platform_device *pdev)
 	const struct of_device_id *of_slave_id = NULL;
 	struct resource *res;
 
+	if (!of_id) {
+		dev_err(&pdev->dev, "Error: No device match found\n");
+		return -ENODEV;
+	}
+
 	if (pdev->dev.of_node->child)
 		of_slave_id = of_match_node(of_slave_match_tbl,
 					    pdev->dev.of_node->child);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] Net: ethernet: mediatek - Fix possible NULL derefrence.
From: Shailendra Verma @ 2017-01-27 11:19 UTC (permalink / raw)
  To: Felix Fietkau, John Crispin, Matthias Brugger, netdev,
	linux-arm-kernel, linux-mediatek, linux-kernel, p.shailesh,
	ashish.kalra, Shailendra Verma, Shailendra Verma
In-Reply-To: <CGME20170127111947epcas2p14f5ea86f0069142be02339754fda4a55@epcas2p1.samsung.com>

of_match_device could return NULL, and so can cause a NULL
pointer dereference later.

Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 4a62ffd..4495b7b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2369,6 +2369,10 @@ static int mtk_probe(struct platform_device *pdev)
 	int i;
 
 	match = of_match_device(of_mtk_match, &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "Error: No device match found\n");
+		return -ENODEV;
+	}
 	soc = (struct mtk_soc_data *)match->data;
 
 	eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] Net: ethernet: mediatek - Fix possible NULL derefrence.
From: Corentin Labbe @ 2017-01-27 12:44 UTC (permalink / raw)
  To: Shailendra Verma
  Cc: Felix Fietkau, ashish.kalra-Sze3O3UU22JBDgjK7y7TUQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	p.shailesh-Sze3O3UU22JBDgjK7y7TUQ,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
	Shailendra Verma,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, John Crispin
In-Reply-To: <1485515980-3814-1-git-send-email-shailendra.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Fri, Jan 27, 2017 at 04:49:40PM +0530, Shailendra Verma wrote:
> of_match_device could return NULL, and so can cause a NULL
> pointer dereference later.
> 
> Signed-off-by: Shailendra Verma <shailendra.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 4a62ffd..4495b7b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2369,6 +2369,10 @@ static int mtk_probe(struct platform_device *pdev)
>  	int i;
>  
>  	match = of_match_device(of_mtk_match, &pdev->dev);
> +	if (!match) {
> +		dev_err(&pdev->dev, "Error: No device match found\n");
> +		return -ENODEV;
> +	}
>  	soc = (struct mtk_soc_data *)match->data;
>  
>  	eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
> -- 

Hello

You could use of_device_get_match_data() and simplifiy code

Regards
Corentin Labbe

^ permalink raw reply

* Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device
From: Mark Brown @ 2017-01-27 13:36 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Daniel Kurtz, Robin Murphy, Leilk Liu, open list,
	open list:SPI SUBSYSTEM, Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Guenter Roeck,
	moderated list:ARM/Mediatek SoC support
In-Reply-To: <CAE_wzQ-an3v_32E+JFzTQMKXf=J6BVGf56QZKL+YCV=mZk6-LA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Thu, Jan 26, 2017 at 09:26:13AM -0800, Dmitry Torokhov wrote:

> OTOH the full buffer used by cros ec *is* DMA-safe (allocated with
> kmalloc), it is that cors ec uses it at offset while transferring data
> piece by piece. Do we want to support this?

I'm not sure the complexity is worth it as a general feature, I worry
about the cost of worrying about cutting the transaction up to include a
PIO portion.  That said that's just a gut feeling, if there were a
sufficiently simple/nice implementation it'd definitely avoid bugs.

To me part of DMA safety is the alignment restrictions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH] ASoC: mt8173-max98090: remove the call to snd_soc_jack_add_pins.
From: Enric Balletbo i Serra @ 2017-01-27 15:20 UTC (permalink / raw)
  To: Mark Brown, Jaroslav Kysela, Matthias Brugger
  Cc: alsa-devel, Garlic Tseng, linux-mediatek, linux-kernel,
	linux-arm-kernel

The snd_soc_card_jack_new function can call snd_soc_jack_add_pins for
you, so pass directly the pins struct when you create the new jack.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 sound/soc/mediatek/mt8173/mt8173-max98090.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/soc/mediatek/mt8173/mt8173-max98090.c b/sound/soc/mediatek/mt8173/mt8173-max98090.c
index 5524a2c..46c8e6a 100644
--- a/sound/soc/mediatek/mt8173/mt8173-max98090.c
+++ b/sound/soc/mediatek/mt8173/mt8173-max98090.c
@@ -79,17 +79,11 @@ static int mt8173_max98090_init(struct snd_soc_pcm_runtime *runtime)
 
 	/* enable jack detection */
 	ret = snd_soc_card_jack_new(card, "Headphone", SND_JACK_HEADPHONE,
-				    &mt8173_max98090_jack, NULL, 0);
+				    &mt8173_max98090_jack,
+				    mt8173_max98090_jack_pins,
+				    ARRAY_SIZE(mt8173_max98090_jack_pins));
 	if (ret) {
-		dev_err(card->dev, "Can't snd_soc_jack_new %d\n", ret);
-		return ret;
-	}
-
-	ret = snd_soc_jack_add_pins(&mt8173_max98090_jack,
-				    ARRAY_SIZE(mt8173_max98090_jack_pins),
-				    mt8173_max98090_jack_pins);
-	if (ret) {
-		dev_err(card->dev, "Can't snd_soc_jack_add_pins %d\n", ret);
+		dev_err(card->dev, "Can't create a new Jack %d\n", ret);
 		return ret;
 	}
 
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH] Net: ethernet: mediatek - Fix possible NULL derefrence.
From: David Miller @ 2017-01-27 16:26 UTC (permalink / raw)
  To: clabbe.montjoie
  Cc: nbd, ashish.kalra, netdev, linux-kernel, p.shailesh,
	linux-mediatek, shailendra.v, matthias.bgg, shailendra.capricorn,
	linux-arm-kernel, blogic
In-Reply-To: <20170127124449.GB13611@Red>

From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Fri, 27 Jan 2017 13:44:49 +0100

> On Fri, Jan 27, 2017 at 04:49:40PM +0530, Shailendra Verma wrote:
>> of_match_device could return NULL, and so can cause a NULL
>> pointer dereference later.
>> 
>> Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
>> ---
>>  drivers/net/ethernet/mediatek/mtk_eth_soc.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> index 4a62ffd..4495b7b 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -2369,6 +2369,10 @@ static int mtk_probe(struct platform_device *pdev)
>>  	int i;
>>  
>>  	match = of_match_device(of_mtk_match, &pdev->dev);
>> +	if (!match) {
>> +		dev_err(&pdev->dev, "Error: No device match found\n");
>> +		return -ENODEV;
>> +	}
>>  	soc = (struct mtk_soc_data *)match->data;
>>  
>>  	eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
>> -- 
> 
> Hello
> 
> You could use of_device_get_match_data() and simplifiy code

Agreed.

^ permalink raw reply

* Re: [PATCH v2, 6/6] dt-bindings: phy-mt65xx-usb: add support for new version phy
From: Rob Herring @ 2017-01-27 20:07 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Kishon Vijay Abraham I, Matthias Brugger, Felipe Balbi,
	Mark Rutland, Ian Campbell, linux-kernel, linux-arm-kernel,
	linux-usb, linux-mediatek, devicetree
In-Reply-To: <1484900321-26933-6-git-send-email-chunfeng.yun@mediatek.com>

On Fri, Jan 20, 2017 at 04:18:41PM +0800, Chunfeng Yun wrote:
> add a new compatible string for "mt2712", and move reference clock
> into each port node;
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  .../devicetree/bindings/phy/phy-mt65xx-usb.txt     |   91 +++++++++++++++++---
>  1 file changed, 77 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> index 33a2b1e..1d06604 100644
> --- a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> @@ -6,21 +6,27 @@ This binding describes a usb3.0 phy for mt65xx platforms of Medaitek SoC.
>  Required properties (controller (parent) node):
>   - compatible	: should be one of
>  		  "mediatek,mt2701-u3phy"
> +		  "mediatek,mt2712-u3phy"
>  		  "mediatek,mt8173-u3phy"
> - - reg		: offset and length of register for phy, exclude port's
> -		  register.
> - - clocks	: a list of phandle + clock-specifier pairs, one for each
> -		  entry in clock-names
> - - clock-names	: must contain
> -		  "u3phya_ref": for reference clock of usb3.0 analog phy.
>  
>  Required nodes	: a sub-node is required for each port the controller
>  		  provides. Address range information including the usual
>  		  'reg' property is used inside these nodes to describe
>  		  the controller's topology.
>  
> +Optional properties (controller (parent) node):
> + - reg		: offset and length of register shared by multiple ports,

How is this optional?

> +		  exclude port's private register. It is needed on mt2701
> +		  and mt8173, but not on mt2712.
> +
>  Required properties (port (child) node):
>  - reg		: address and length of the register set for the port.
> +- clocks	: a list of phandle + clock-specifier pairs, one for each
> +		  entry in clock-names
> +- clock-names	: must contain
> +		  "ref_clk": 48M reference clock for HighSpeed analog phy; and

_clk is redundant. Just "ref"

> +			26M reference clock for SuperSpeed analog phy, sometimes is
> +			24M, 25M or 27M, depended on platform.
>  - #phy-cells	: should be 1 (See second example)
>  		  cell after port phandle is phy type from:
>  			- PHY_TYPE_USB2
> @@ -31,21 +37,31 @@ Example:
>  u3phy: usb-phy@11290000 {
>  	compatible = "mediatek,mt8173-u3phy";
>  	reg = <0 0x11290000 0 0x800>;
> -	clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
> -	clock-names = "u3phya_ref";
>  	#address-cells = <2>;
>  	#size-cells = <2>;
>  	ranges;
>  	status = "okay";
>  
> -	phy_port0: port@11290800 {
> -		reg = <0 0x11290800 0 0x800>;
> +	u2port0: port@11290800 {
> +		reg = <0 0x11290800 0 0x100>;
> +		clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
> +		clock-names = "ref_clk";
>  		#phy-cells = <1>;
>  		status = "okay";
>  	};
>  
> -	phy_port1: port@11291000 {
> -		reg = <0 0x11291000 0 0x800>;
> +	u3port0: port@11290900 {
> +		reg = <0 0x11290800 0 0x700>;
> +		clocks = <&clk26m>;
> +		clock-names = "ref_clk";
> +		#phy-cells = <1>;
> +		status = "okay";
> +	};
> +
> +	u2port1: port@11291000 {
> +		reg = <0 0x11291000 0 0x100>;
> +		clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
> +		clock-names = "ref_clk";
>  		#phy-cells = <1>;
>  		status = "okay";
>  	};
> @@ -64,7 +80,54 @@ Example:
>  
>  usb30: usb@11270000 {
>  	...
> -	phys = <&phy_port0 PHY_TYPE_USB3>;
> -	phy-names = "usb3-0";
> +	phys = <&u2port0 PHY_TYPE_USB2>, <&u3port0 PHY_TYPE_USB3>;
> +	phy-names = "usb2-0", "usb3-0";
>  	...
>  };
> +
> +
> +Layout differences of banks between mt8173/mt2701 and mt2712
> +-------------------------------------------------------------
> +mt8173 and mt2701:
> +port        offset    bank
> +shared      0x0000    SPLLC
> +            0x0100    FMREG
> +u2 port0    0x0800    U2PHY_COM
> +u3 port0    0x0900    U3PHYD
> +            0x0a00    U3PHYD_BANK2
> +            0x0b00    U3PHYA
> +            0x0c00    U3PHYA_DA
> +u2 port1    0x1000    U2PHY_COM
> +u3 port1    0x1100    U3PHYD
> +            0x1200    U3PHYD_BANK2
> +            0x1300    U3PHYA
> +            0x1400    U3PHYA_DA
> +u2 port2    0x1800    U2PHY_COM
> +            ...
> +
> +mt2712:
> +port        offset    bank
> +u2 port0    0x0000    MISC
> +            0x0100    FMREG
> +            0x0300    U2PHY_COM
> +u3 port0    0x0700    SPLLC
> +            0x0800    CHIP
> +            0x0900    U3PHYD
> +            0x0a00    U3PHYD_BANK2
> +            0x0b00    U3PHYA
> +            0x0c00    U3PHYA_DA
> +u2 port1    0x1000    MISC
> +            0x1100    FMREG
> +            0x1300    U2PHY_COM
> +u3 port1    0x1700    SPLLC
> +            0x1800    CHIP
> +            0x1900    U3PHYD
> +            0x1a00    U3PHYD_BANK2
> +            0x1b00    U3PHYA
> +            0x1c00    U3PHYA_DA
> +u2 port2    0x2000    MISC
> +            ...
> +
> +    SPLLC shared by u3 ports and FMREG shared by u2 ports on
> +mt8173/mt2701 are put back into each port; a new bank MISC for
> +u2 ports and CHIP for u3 ports are added on mt2712.
> -- 
> 1.7.9.5
> 

^ permalink raw reply

* Re: [PATCH 13/16] arm: dts: rename mt7623-evb.dts to arch/arm/boot/dts/mt7623n-rfb.dtsi
From: Rob Herring @ 2017-01-27 20:13 UTC (permalink / raw)
  To: John Crispin
  Cc: Matthias Brugger, devicetree, linux-mediatek, Andreas Färber,
	linux-arm-kernel
In-Reply-To: <1485170975-51813-14-git-send-email-john@phrozen.org>

On Mon, Jan 23, 2017 at 12:29:32PM +0100, John Crispin wrote:
> There are 2 versions of the SoC. MT7623N is almost identical to MT7623A
> but has some additional multimedia features. The reference boards are
> available as NAND or MMC and might have a different ethernet setup. In
> order to reduce the duplication of devicetree code we add an intermediate
> dtsi file for these reference boards. Additionally MTK/WCN pointed out,
> that the EVB is yet another board and the board in question is infact the
> RFB. Take this into account while renaming the files.
> 
>  Signed-off-by: John Crispin <john@phrozen.org>
> ---
>  Documentation/devicetree/bindings/arm/mediatek.txt |    4 +--
>  arch/arm/boot/dts/Makefile                         |    2 +-
>  arch/arm/boot/dts/mt7623-evb.dts                   |   33 --------------------
>  arch/arm/boot/dts/mt7623n-rfb-nand.dts             |   21 +++++++++++++
>  arch/arm/boot/dts/mt7623n-rfb.dtsi                 |   29 +++++++++++++++++
>  5 files changed, 53 insertions(+), 36 deletions(-)
>  delete mode 100644 arch/arm/boot/dts/mt7623-evb.dts
>  create mode 100644 arch/arm/boot/dts/mt7623n-rfb-nand.dts
>  create mode 100644 arch/arm/boot/dts/mt7623n-rfb.dtsi
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
> index c860b24..71149cb 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek.txt
> @@ -38,9 +38,9 @@ Supported boards:
>  - Evaluation board for MT6795(Helio X10):
>      Required root node properties:
>        - compatible = "mediatek,mt6795-evb", "mediatek,mt6795";
> -- Evaluation board for MT7623:
> +- Reference  board for MT7623N with NAND:
>      Required root node properties:
> -      - compatible = "mediatek,mt7623-evb", "mediatek,mt7623";
> +      - compatible = "mediatek,mt7623n-rfb-nand", "mediatek,mt7623";

Seems like we should have mt7623n as a compatible.

>  - MTK mt8127 tablet moose EVB:
>      Required root node properties:
>        - compatible = "mediatek,mt8127-moose", "mediatek,mt8127";
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index cccdbcb..9735c2c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -976,7 +976,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += \
>  	mt6580-evbp1.dtb \
>  	mt6589-aquaris5.dtb \
>  	mt6592-evb.dtb \
> -	mt7623-evb.dtb \
> +	mt7623n-rfb-nand.dtb \
>  	mt8127-moose.dtb \
>  	mt8135-evbp1.dtb
>  dtb-$(CONFIG_ARCH_ZX) += zx296702-ad1.dtb
> diff --git a/arch/arm/boot/dts/mt7623-evb.dts b/arch/arm/boot/dts/mt7623-evb.dts
> deleted file mode 100644
> index 58ed038..0000000
> --- a/arch/arm/boot/dts/mt7623-evb.dts
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/*
> - * Copyright (c) 2016 MediaTek Inc.
> - * Author: John Crispin <blogic@openwrt.org>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -/dts-v1/;
> -#include "mt7623-mt6323.dtsi"
> -
> -/ {
> -	model = "MediaTek MT7623 evaluation board";
> -	compatible = "mediatek,mt7623-evb", "mediatek,mt7623";
> -
> -	chosen {
> -		stdout-path = &uart2;
> -	};
> -
> -	memory {
> -		reg = <0 0x80000000 0 0x40000000>;
> -	};
> -};
> -
> -&uart2 {
> -	status = "okay";
> -};
> diff --git a/arch/arm/boot/dts/mt7623n-rfb-nand.dts b/arch/arm/boot/dts/mt7623n-rfb-nand.dts
> new file mode 100644
> index 0000000..436d51c
> --- /dev/null
> +++ b/arch/arm/boot/dts/mt7623n-rfb-nand.dts
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2016 MediaTek Inc.
> + * Author: John Crispin <blogic@openwrt.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/dts-v1/;
> +#include "mt7623n-rfb.dtsi"
> +
> +/ {
> +	model = "MediaTek MT7623N NAND reference board";
> +	compatible = "mediatek,mt7623n-rfb-nand", "mediatek,mt7623";
> +};
> diff --git a/arch/arm/boot/dts/mt7623n-rfb.dtsi b/arch/arm/boot/dts/mt7623n-rfb.dtsi
> new file mode 100644
> index 0000000..d46390e
> --- /dev/null
> +++ b/arch/arm/boot/dts/mt7623n-rfb.dtsi
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright (c) 2016 MediaTek Inc.
> + * Author: John Crispin <blogic@openwrt.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "mt7623-mt6323.dtsi"
> +
> +/ {
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	memory {

Unless it can change:

memory@80000000

> +		reg = <0 0x80000000 0 0x40000000>;
> +	};
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};
> -- 
> 1.7.10.4
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox