[PATCH 3/3] ASoC: pxa: add COMPILE_TEST on SND_MMP_SOC

[Kuninori Morimoto]

From: Kuninori Morimoto 

It doesn't use asm header. We can add COMPILE_TEST

Signed-off-by: Kuninori Morimoto 
---
 sound/soc/pxa/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/pxa/Kconfig b/sound/soc/pxa/Kconfig
index 960744e..6feb34b 100644
--- a/sound/soc/pxa/Kconfig
+++ b/sound/soc/pxa/Kconfig
@@ -9,7 +9,7 @@ config SND_PXA2XX_SOC
 
 config SND_MMP_SOC
bool "Soc Audio for Marvell MMP chips"
-   depends on ARCH_MMP
+   depends on ARCH_MMP || COMPILE_TEST
select MMP_SRAM
select SND_SOC_GENERIC_DMAENGINE_PCM
select SND_ARM
-- 
1.9.1

[PATCH 1/3] ASoC: fsl: mpc5200_dma: remove unused psc_dma

[Kuninori Morimoto]

linux/sound/soc/fsl/mpc5200_dma.c:305:18: warning: unused variable \
psc_dma’ [-Wunused-variable]

Signed-off-by: Kuninori Morimoto 
---
 sound/soc/fsl/mpc5200_dma.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 93885d9..cd8ccaba 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -302,7 +302,6 @@ static int psc_dma_new(struct snd_soc_pcm_runtime *rtd)
struct snd_card *card = rtd->card->snd_card;
struct snd_soc_dai *dai = rtd->cpu_dai;
struct snd_pcm *pcm = rtd->pcm;
-   struct psc_dma *psc_dma = snd_soc_dai_get_drvdata(rtd->cpu_dai);
size_t size = psc_dma_hardware.buffer_bytes_max;
int rc;
 
-- 
1.9.1

Re: [RFC][PATCH 0/x] ASoC: replace platform to component

[Kuninori Morimoto]

Hi Mark, Lars-Peter

> 1st Note is that it is still using rtd->platform_com style.

It is still using this "rtd->xxx" style, but we should expand it.
I'm thinking that we can connect each component to rtd by using list
and we can get it by "driver" (or something).
Then, we can add new snd_soc_lookup_runtime() function for it.
But, what do you think ?

- rtd->platform_com = component;
+ list_add(component->rtd_list, rtd->list_head);

- struct device *dev = rtd->platform_com->dev;
+ struct snd_soc_component *component = snd_soc_lookup_runtime(rtd, 
driver);
+ struct device *dev = component->dev;

struct snd_soc_component *snd_soc_lookup_runtime(rtd, driver)
{
...
list_for_each_entry(component, >list_head, xxx) {
if (driver == component->driver)
return component;
}

return NULL;
}


Best regards
---
Kuninori Morimoto

[PATCH 3/3] ASoC: audio-graph-scu-card: use asoc_simple_card_of_canonicalize_cpu()

[Kuninori Morimoto]

From: Kuninori Morimoto 

snd_soc_find_dai() will check dai_name after of_node matching
if dai_link has it. but, it will never match if name was
created by fmt_single_name(). Thus, we need to remove cpu_dai_name
if cpu was single.

Before, simple-card assumed that CPU was single if Card has single
link. It is no problem in below case

/* Card uses 1 link */
card {
compatible = "audio-graph-card";
...
dais = <_port0>;
};

/* CPU has single endpoints */
cpu {
...
cpu_port0: port@0 {
endpoint { ... };
};
};

But it can't handle correctly below case.
This patch uses new asoc_simple_card_of_canonicalize_cpu() and
confirm it was single or not by counting endpoint.

/* Card uses only 1 link */
card {
compatible = "audio-graph-card";
...
dais = <_port0>;
};

/* CPU has many endpoints */
cpu {
...
ports {
cpu_port0: port@0 {
endpoint { ... };
};
cpu_port1: port@1 {
endpoint { ... };
};
...
};
};

Signed-off-by: Kuninori Morimoto 
---
 sound/soc/generic/audio-graph-scu-card.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/generic/audio-graph-scu-card.c 
b/sound/soc/generic/audio-graph-scu-card.c
index 05934b2..bd924b1 100644
--- a/sound/soc/generic/audio-graph-scu-card.c
+++ b/sound/soc/generic/audio-graph-scu-card.c
@@ -123,9 +123,7 @@ static int asoc_graph_card_dai_link_of(struct device_node 
*ep,
if (ret < 0)
return ret;
 
-   /* card->num_links includes Codec */
-   asoc_simple_card_canonicalize_cpu(dai_link,
-   (card->num_links - 1) == 1);
+   asoc_simple_card_of_canonicalize_cpu(dai_link);
} else {
/* FE is dummy */
dai_link->cpu_of_node   = NULL;
-- 
1.9.1

[PATCH 2/3] ASoC: audio-graph-card: use asoc_simple_card_of_canonicalize_cpu()

[Kuninori Morimoto]

From: Kuninori Morimoto 

snd_soc_find_dai() will check dai_name after of_node matching
if dai_link has it. but, it will never match if name was
created by fmt_single_name(). Thus, we need to remove cpu_dai_name
if cpu was single.

Before, simple-card assumed that CPU was single if Card has single
link. It is no problem in below case

/* Card uses 1 link */
card {
compatible = "audio-graph-card";
...
dais = <_port0>;
};

/* CPU has single endpoints */
cpu {
...
cpu_port0: port@0 {
endpoint { ... };
};
};

But it can't handle correctly below case.
This patch uses new asoc_simple_card_of_canonicalize_cpu() and
confirm it was single or not by counting endpoint.

/* Card uses only 1 link */
card {
compatible = "audio-graph-card";
...
dais = <_port0>;
};

/* CPU has many endpoints */
cpu {
...
ports {
cpu_port0: port@0 {
endpoint { ... };
};
cpu_port1: port@1 {
endpoint { ... };
};
...
};
};

Signed-off-by: Kuninori Morimoto 
---
 sound/soc/generic/audio-graph-card.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c 
b/sound/soc/generic/audio-graph-card.c
index 885b405..5e5b8bb 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -100,7 +100,6 @@ static int asoc_graph_card_dai_link_of(struct device_node 
*cpu_port,
struct graph_dai_props *dai_props = graph_priv_to_props(priv, idx);
struct asoc_simple_dai *cpu_dai = _props->cpu_dai;
struct asoc_simple_dai *codec_dai = _props->codec_dai;
-   struct snd_soc_card *card = graph_priv_to_card(priv);
struct device_node *cpu_ep= of_get_next_child(cpu_port, NULL);
struct device_node *codec_ep = of_graph_get_remote_endpoint(cpu_ep);
struct device_node *rcpu_ep = of_graph_get_remote_endpoint(codec_ep);
@@ -161,8 +160,7 @@ static int asoc_graph_card_dai_link_of(struct device_node 
*cpu_port,
dai_link->ops = _graph_card_ops;
dai_link->init = asoc_graph_card_dai_init;
 
-   asoc_simple_card_canonicalize_cpu(dai_link,
- card->num_links == 1);
+   asoc_simple_card_of_canonicalize_cpu(dai_link);
 
 dai_link_of_err:
of_node_put(cpu_ep);
-- 
1.9.1

[PATCH 1/3] ASoC: simple-card-utils: add asoc_simple_card_of_canonicalize_cpu()

[Kuninori Morimoto]

From: Kuninori Morimoto 

snd_soc_find_dai() will check dai_name after of_node matching
if dai_link has it. but, it will never match if name was
created by fmt_single_name(). Thus, we need to remove cpu_dai_name
if cpu was single.

Before, simple-card assumed that CPU was single if Card has single
link. It is no problem in below case

/* Card uses 1 link */
card {
compatible = "audio-graph-card";
...
dais = <_port0>;
};

/* CPU has single endpoints */
cpu {
...
cpu_port0: port@0 {
endpoint { ... };
};
};

But it can't handle correctly below case.
This patch adds new asoc_simple_card_of_canonicalize_cpu() and
confirm it was single or not by counting endpoint.

/* Card uses only 1 link */
card {
compatible = "audio-graph-card";
...
dais = <_port0>;
};

/* CPU has many endpoints */
cpu {
...
ports {
cpu_port0: port@0 {
endpoint { ... };
};
cpu_port1: port@1 {
endpoint { ... };
};
...
};
};

Signed-off-by: Kuninori Morimoto 
---
 include/sound/simple_card_utils.h |  1 +
 sound/soc/generic/simple-card-utils.c | 20 
 2 files changed, 21 insertions(+)

diff --git a/include/sound/simple_card_utils.h 
b/include/sound/simple_card_utils.h
index 42c6a6a..bfb3dca 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -90,6 +90,7 @@ int asoc_simple_card_init_dai(struct snd_soc_dai *dai,
  struct asoc_simple_dai *simple_dai);
 
 int asoc_simple_card_canonicalize_dailink(struct snd_soc_dai_link *dai_link);
+void asoc_simple_card_of_canonicalize_cpu(struct snd_soc_dai_link *dai_link);
 void asoc_simple_card_canonicalize_cpu(struct snd_soc_dai_link *dai_link,
  int is_single_links);
 
diff --git a/sound/soc/generic/simple-card-utils.c 
b/sound/soc/generic/simple-card-utils.c
index 26d64fa..bc2e9a2 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -343,6 +343,26 @@ int asoc_simple_card_canonicalize_dailink(struct 
snd_soc_dai_link *dai_link)
 }
 EXPORT_SYMBOL_GPL(asoc_simple_card_canonicalize_dailink);
 
+void asoc_simple_card_of_canonicalize_cpu(struct snd_soc_dai_link *dai_link)
+{
+   /*
+* soc_bind_dai_link() will check cpu name after
+* of_node matching if dai_link has cpu_dai_name.
+* but, it will never match if name was created by
+* fmt_single_name(). remove cpu_dai_name if cpu_args
+* was 0. See:
+*  fmt_single_name()
+*  fmt_multiple_name()
+*
+* simple card utils assumes if driver has many endpoint,
+* it is using fmt_multiple_name()
+*/
+
+   if (of_graph_get_endpoint_count(dai_link->cpu_of_node) == 1)
+   dai_link->cpu_dai_name = NULL;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_card_of_canonicalize_cpu);
+
 void asoc_simple_card_canonicalize_cpu(struct snd_soc_dai_link *dai_link,
   int is_single_links)
 {
-- 
1.9.1

[PATCH 0/3] ASoC: add/use asoc_simple_card_of_canonicalize_cpu()

[Kuninori Morimoto]

Hi Mark

These fixes audio graph cards DAI counting bug.
Simple Card side doesn't have this issue, but Audio Graph Card
side has it.

Kuninori Morimoto (3):
  ASoC: simple-card-utils: add asoc_simple_card_of_canonicalize_cpu()
  ASoC: audio-graph-card: use asoc_simple_card_of_canonicalize_cpu()
  ASoC: audio-graph-scu-card: use asoc_simple_card_of_canonicalize_cpu()

 include/sound/simple_card_utils.h|  1 +
 sound/soc/generic/audio-graph-card.c |  4 +---
 sound/soc/generic/audio-graph-scu-card.c |  4 +---
 sound/soc/generic/simple-card-utils.c| 20 
 4 files changed, 23 insertions(+), 6 deletions(-)

-- 
1.9.1

Re: [PATCH 0/2] arm64: defconfig: ak4613/renesas sound will be kernle module

[Kuninori Morimoto]

Hi Simon

> > These exchange Renesas sound related driver as module.
> > But, it doesn't exchange simple-sound-card which is
> > used from other platform.
> > 
> > Kuninori Morimoto (2):
> >   arm64: defconfig: compile ak4613 as kernel module
> >   arm64: defconfig: compile renesas sound as kernel module
> 
> Thanks Morimoto-san,
> 
> do you mind if I squash these into one patch?

Thanks.
I have no objection about it

Best regards
---
Kuninori Morimoto

Re: [PATCH] i2c: algo-bit: add support for I2C_M_STOP

[Wolfram Sang]

Hi Jean,

> > +   bool did_stop = false;
> 
> I'm pretty certain you want to declare and initialize this variable
> outside the loop.

Why? Well, it doesn't matter anymore but what is wrong with limiting the
variable like this?

> > +   if (pmsg->flags & I2C_M_STOP && i != num - 1) {
> 
> I recommend adding parentheses around the bit matching when combined
> with &&. I know it is not strictly needed and the compiler doesn't
> care, but I find it easier to read, and there seems to be a consensus
> (90 %) on that in the kernel tree.

Either both in parens, or none ;) But doesn't matter as well anymore.

> 1* Repeated start happens between messages of a same transaction, and
> you handle that case above. However in the case of 10-bit address
> clients, there is also a repeated start happening during the address
> phase of the transaction, if the first message is a read. Did you check
> what the SCCB protocol expects in that case?

SCCB defines addresses to be 7 bit.

> 2* I'm not sure why you add the enforced stop at the end of one
> iteration and the start at the beginning of the next iteration. It
> would be more simple and efficient to do both at the beginning of the
> next iteration. The only case where it would make a difference is if
> both I2C_M_NOSTART and I2C_M_STOP are specified. In this case you
> currently emit a stop condition but no start, which I don't think can
> work at all.

Ehrm, probably I was just too tied to the ordering of "first start -
then data - then stop - then next loop" :)

> What about something like that instead? Or I am missing something?

No, I did miss it.

> --- linux-4.11.orig/drivers/i2c/algos/i2c-algo-bit.c  2017-06-19 
> 09:57:17.949074198 +0200
> +++ linux-4.11/drivers/i2c/algos/i2c-algo-bit.c   2017-06-19 
> 10:23:26.711088536 +0200
> @@ -553,8 +553,17 @@ static int bit_xfer(struct i2c_adapter *
>   nak_ok = pmsg->flags & I2C_M_IGNORE_NAK;
>   if (!(pmsg->flags & I2C_M_NOSTART)) {
>   if (i) {
> - bit_dbg(3, _adap->dev, "emitting "
> - "repeated start condition\n");
> + if (msgs[i - 1].flags & I2C_M_STOP) {
> + bit_dbg(3, _adap->dev,
> + "emitting enforced stop 
> condition\n");
> + i2c_stop(adap);
> + bit_dbg(3, _adap->dev,
> + "emitting start condition\n");
> + i2c_start(adap);
> + }

else

> +
> + bit_dbg(3, _adap->dev,
> + "emitting repeated start condition\n");
>   i2c_repstart(adap);
>   }
>   ret = bit_doAddress(i2c_adap, pmsg);

A lot better! I like it very much. And also

Tested-by: Wolfram Sang 

Do you want to cook up a patch or shall I? I'd just need a SoB then.

Thanks for the improvement!

   Wolfram



signature.asc
Description: PGP signature

[PATCH] pinctrl: sh-pfc: r8a7792: Add SCIF1 and SCIF2 pin groups

[Ulrich Hecht]

Add SCIF1 and SCIF2 pin groups to the R8A7792 PFC driver.

Signed-off-by: Ulrich Hecht 
---

Superseding "[PATCH] pinctrl: sh-pfc: r8a7792: Add SCIF1 pin groups", this
adds the missing SCIF1 control pins as well as SCIF2, as suggested by Geert.

CU
Uli


 drivers/pinctrl/sh-pfc/pfc-r8a7792.c | 55 
 1 file changed, 55 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7792.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7792.c
index 21badb6..cc3597f 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7792.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7792.c
@@ -1137,6 +1137,43 @@ static const unsigned int scif0_ctrl_pins[] = {
 static const unsigned int scif0_ctrl_mux[] = {
RTS0_N_MARK, CTS0_N_MARK,
 };
+/* - SCIF1 -- 
*/
+static const unsigned int scif1_data_pins[] = {
+   /* RX, TX */
+   RCAR_GP_PIN(10, 19), RCAR_GP_PIN(10, 18),
+};
+static const unsigned int scif1_data_mux[] = {
+   RX1_MARK, TX1_MARK,
+};
+static const unsigned int scif1_clk_pins[] = {
+   /* SCK */
+   RCAR_GP_PIN(10, 15),
+};
+static const unsigned int scif1_clk_mux[] = {
+   SCK1_MARK,
+};
+static const unsigned int scif1_ctrl_pins[] = {
+   /* RTS, CTS */
+   RCAR_GP_PIN(10, 17), RCAR_GP_PIN(10, 16),
+};
+static const unsigned int scif1_ctrl_mux[] = {
+   RTS1_N_MARK, CTS1_N_MARK,
+};
+/* - SCIF2 -- 
*/
+static const unsigned int scif2_data_pins[] = {
+   /* RX, TX */
+   RCAR_GP_PIN(10, 22), RCAR_GP_PIN(10, 21),
+};
+static const unsigned int scif2_data_mux[] = {
+   RX2_MARK, TX2_MARK,
+};
+static const unsigned int scif2_clk_pins[] = {
+   /* SCK */
+   RCAR_GP_PIN(10, 20),
+};
+static const unsigned int scif2_clk_mux[] = {
+   SCK2_MARK,
+};
 /* - SCIF3 -- 
*/
 static const unsigned int scif3_data_pins[] = {
/* RX, TX */
@@ -1680,6 +1717,11 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
SH_PFC_PIN_GROUP(scif0_data),
SH_PFC_PIN_GROUP(scif0_clk),
SH_PFC_PIN_GROUP(scif0_ctrl),
+   SH_PFC_PIN_GROUP(scif1_data),
+   SH_PFC_PIN_GROUP(scif1_clk),
+   SH_PFC_PIN_GROUP(scif1_ctrl),
+   SH_PFC_PIN_GROUP(scif2_data),
+   SH_PFC_PIN_GROUP(scif2_clk),
SH_PFC_PIN_GROUP(scif3_data),
SH_PFC_PIN_GROUP(scif3_clk),
SH_PFC_PIN_GROUP(sdhi0_data1),
@@ -1826,6 +1868,17 @@ static const char * const scif0_groups[] = {
"scif0_ctrl",
 };
 
+static const char * const scif1_groups[] = {
+   "scif1_data",
+   "scif1_clk",
+   "scif1_ctrl",
+};
+
+static const char * const scif2_groups[] = {
+   "scif2_data",
+   "scif2_clk",
+};
+
 static const char * const scif3_groups[] = {
"scif3_data",
"scif3_clk",
@@ -1924,6 +1977,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
SH_PFC_FUNCTION(msiof1),
SH_PFC_FUNCTION(qspi),
SH_PFC_FUNCTION(scif0),
+   SH_PFC_FUNCTION(scif1),
+   SH_PFC_FUNCTION(scif2),
SH_PFC_FUNCTION(scif3),
SH_PFC_FUNCTION(sdhi0),
SH_PFC_FUNCTION(vin0),
-- 
2.7.4

Re: [PATCH 0/2] arm64: defconfig: ak4613/renesas sound will be kernle module

[Simon Horman]

On Mon, Jun 19, 2017 at 12:23:22AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> These exchange Renesas sound related driver as module.
> But, it doesn't exchange simple-sound-card which is
> used from other platform.
> 
> Kuninori Morimoto (2):
>   arm64: defconfig: compile ak4613 as kernel module
>   arm64: defconfig: compile renesas sound as kernel module

Thanks Morimoto-san,

do you mind if I squash these into one patch?

Re: Rebasing mmc/next

[Geert Uytterhoeven]

Hi Ulf,

On Tue, Jun 20, 2017 at 10:07 AM, Ulf Hansson  wrote:
> On 20 June 2017 at 09:17, Geert Uytterhoeven  wrote:
>> It looks like you rebase mmc/next almost daily. Is there any specific reason
>> for that?
>
> I don't do it daily, but often, yes. :-)

A few years ago, I got bashed by Linus for rebasing the m68k "for-linus"
branch on every -rc.

>> I'm asking because I create a "renesas-drivers" tree on a regular basis
>> (cfr. e.g. https://www.spinics.net/lists/linux-renesas-soc/msg15111.html).
>> This tree is meant to ease development of platform support and drivers
>> for Renesas ARM SoCs. It is created by merging (a) the for-next branches
>> of various subsystem trees and (b) branches with driver code submitted
>> or planned for submission to maintainers into the development branch of
>> Simon Horman's renesas.git tree.
>
> If you are asking me to keep my next branch immutable, then please no,
> I don't like to do that. Reason explained below.

100% immutable is not needed.

> I don't have a problem to share specific renesas mmc branches with
> you, if that helps?

Hmm, that would be more work on your side. Plus communication overhead.

>> If for (b), people submit driver code based on mmc/next, it may start
>> to conflict
>> with subsequent mmc/next releases soon, requiring the submitter or me to
>> rebase the code before including it in renesas-drivers.
>>
>> Most subsystem maintainers don't rebase their for-next branch, unless there's
>> a very good reason for it (e.g. a serious breakage hindering bisection).
>
> I do it for a couple of reasons.
>
> First, sometimes I apply changes before people have provided enough
> tested by tags, to instead allow the changes to be tested in
> linux-next. This may lead to some situations when I need to re-base my
> next branch.
> *) Something breaks, then I need to drop the changes. Revert doesn't
> play well here, especially if it's a series of changes.
> **) Avoid breaking bisect.

OK.

> ***) I want to give people cred, adding peoples tested-by, reviewed-by
> tags, after the changes have been queued on my next branch.

I think you're about the only one who does that ;-)

> Second, even if the changes queued on next has been thoroughly tested,
> sometimes error reports still show up. I most cases I prefer to avoid
> breaking bisect, which then leaves me in no other option, but
> re-basing my branch (to either drop changes or amend them).
>
> I guess what I can do, is to host a pre-next branch, which serves as
> my pre-integration branch, before I moves things to next. However,
> this does put some more administrative work on me, so I would like to
> avoid that.

OK.

> I am trying to understand the purpose of your renesas integration
> tree, and why it's a problem for you to pick up my re-based branch?
> Could you perhaps elaborate on this?

The purpose is to make it easier for the upstream Renesas kernel team and
associated testers to consume our work-in-progress.

Hence I merge the for-next branches of selected subsystems, followed by
topic branches with work-in-progress driver code.

E.g. Simon had asked me to include
git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git
topic/sdhi-gen3-dma-2017
That branch was based on your mmc/next.

But by the time I created the renesas-drivers release, mmc/next had been
rebased.  Hence after merging the new mmc/next, I had to rebase his work:
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/sdhi-gen3-dma-2017-rebased1

I hope this explains our problem.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Rebasing mmc/next

[Ulf Hansson]

On 20 June 2017 at 09:17, Geert Uytterhoeven  wrote:
> Hi Ulf,
>
> It looks like you rebase mmc/next almost daily. Is there any specific reason
> for that?

I don't do it daily, but often, yes. :-)

>
> I'm asking because I create a "renesas-drivers" tree on a regular basis
> (cfr. e.g. https://www.spinics.net/lists/linux-renesas-soc/msg15111.html).
> This tree is meant to ease development of platform support and drivers
> for Renesas ARM SoCs. It is created by merging (a) the for-next branches
> of various subsystem trees and (b) branches with driver code submitted
> or planned for submission to maintainers into the development branch of
> Simon Horman's renesas.git tree.

If you are asking me to keep my next branch immutable, then please no,
I don't like to do that. Reason explained below.

I don't have a problem to share specific renesas mmc branches with
you, if that helps?

>
> If for (b), people submit driver code based on mmc/next, it may start
> to conflict
> with subsequent mmc/next releases soon, requiring the submitter or me to
> rebase the code before including it in renesas-drivers.
>
> Most subsystem maintainers don't rebase their for-next branch, unless there's
> a very good reason for it (e.g. a serious breakage hindering bisection).

I do it for a couple of reasons.

First, sometimes I apply changes before people have provided enough
tested by tags, to instead allow the changes to be tested in
linux-next. This may lead to some situations when I need to re-base my
next branch.
*) Something breaks, then I need to drop the changes. Revert doesn't
play well here, especially if it's a series of changes.
**) Avoid breaking bisect.
***) I want to give people cred, adding peoples tested-by, reviewed-by
tags, after the changes have been queued on my next branch.

Second, even if the changes queued on next has been thoroughly tested,
sometimes error reports still show up. I most cases I prefer to avoid
breaking bisect, which then leaves me in no other option, but
re-basing my branch (to either drop changes or amend them).

I guess what I can do, is to host a pre-next branch, which serves as
my pre-integration branch, before I moves things to next. However,
this does put some more administrative work on me, so I would like to
avoid that.

I am trying to understand the purpose of your renesas integration
tree, and why it's a problem for you to pick up my re-based branch?
Could you perhaps elaborate on this?

Kind regards
Uffe

Re: [PATCH v1 04/12] media: i2c: mt9m111: Add source pad

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Monday 19 Jun 2017 19:04:41 Jacopo Mondi wrote:
> Add a single source pad to mt9m111 media entity.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/mt9m111.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index 8e86d51..7e70969 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -141,6 +141,8 @@
>  #define MT9M111_MAX_HEIGHT   1024
>  #define MT9M111_MAX_WIDTH1280
> 
> +#define MT9M111_MEDIA_PAD1
> +
>  struct mt9m111_context {
>   u16 read_mode;
>   u16 blanking_h;
> @@ -215,6 +217,8 @@ struct mt9m111 {
>   int power_count;
>   const struct mt9m111_datafmt *fmt;
>   int lastpage;   /* PageMap cache value */
> +
> + struct media_pad pad;
>  };
> 
>  /* Find a data format by a pixel code */
> @@ -963,13 +967,23 @@ static int mt9m111_probe(struct i2c_client *client,
>   if (ret < 0)
>   goto out_hdlfree;
> 
> + mt9m111->pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(>subdev.entity, 
MT9M111_MEDIA_PAD,

The second argument is the number of pads, not a pad number. The macro name is 
quite confusing. Given that you use it once only, you can drop it and hardcode 
the value.

> +  >pad);
> + if (ret)
> + goto out_hdlfree;
> +
>   mt9m111->subdev.dev = >dev;
> + mt9m111->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> + mt9m111->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>   ret = v4l2_async_register_subdev(>subdev);
>   if (ret < 0)
> - goto out_hdlfree;
> + goto out_mediafree;
> 
>   return 0;
> 
> +out_mediafree:

Maybe out_media_cleanup ?

> + media_entity_cleanup(>subdev.entity);
>  out_hdlfree:
>   v4l2_ctrl_handler_free(>hdl);
>  out_clkput:
> @@ -982,6 +996,7 @@ static int mt9m111_remove(struct i2c_client *client)
>  {
>   struct mt9m111 *mt9m111 = to_mt9m111(client);
> 
> + media_entity_cleanup(>subdev.entity);
>   v4l2_async_unregister_subdev(>subdev);
>   v4l2_clk_put(mt9m111->clk);
>   v4l2_ctrl_handler_free(>hdl);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 03/12] media: i2c: mt9m111: Skip chid identification

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Monday 19 Jun 2017 19:04:40 Jacopo Mondi wrote:
> Reads of chip identification code (both on registers 0x00 and 0xff)
> always return 0x00.

This shouldn't be the case. It might mean that your I2C master controller 
doesn't handle reads correctly. I don't think this patch is correct.

> Skip chip identification to have the device complete probing.
>
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/mt9m111.c | 19 ---
>  1 file changed, 19 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index 72e71b7..8e86d51 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -890,31 +890,12 @@ static struct v4l2_subdev_ops mt9m111_subdev_ops = {
>  static int mt9m111_video_probe(struct i2c_client *client)
>  {
>   struct mt9m111 *mt9m111 = to_mt9m111(client);
> - s32 data;
>   int ret;
> 
>   ret = mt9m111_s_power(>subdev, 1);
>   if (ret < 0)
>   return ret;
> 
> - data = reg_read(CHIP_VERSION);
> -
> - switch (data) {
> - case 0x143a: /* MT9M111 or MT9M131 */
> - dev_info(>dev,
> - "Detected a MT9M111/MT9M131 chip ID %x\n", data);
> - break;
> - case 0x148c: /* MT9M112 */
> - dev_info(>dev, "Detected a MT9M112 chip ID %x\n", 
data);
> - break;
> - default:
> - dev_err(>dev,
> - "No MT9M111/MT9M112/MT9M131 chip detected register 
read %x\n",
> - data);
> - ret = -ENODEV;
> - goto done;
> - }
> -
>   ret = mt9m111_init(mt9m111);
>   if (ret)
>   goto done;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 01/12] arm64: boot: dts: salvator-x: Add camera module

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Monday 19 Jun 2017 19:04:38 Jacopo Mondi wrote:
> Add camera module to Salvator-X M3W device tree.
> The camera module sits on a i2c-gpio interface and it connected to VIN
> channel #4 parallel video input port.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 35 +++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts index ca4eb98..41c94c3
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> @@ -103,6 +103,41 @@
>   states = <330 1
> 180 0>;
>   };
> +
> + camera_module {

This node describes an I2C master controller, it should be called accordingly.

> + compatible = "i2c-gpio";
> + gpios = < 9 GPIO_ACTIVE_HIGH
> +   11 GPIO_ACTIVE_HIGH
> + >;
> +
> + i2c-gpio,delay-us = <4>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mclk: xclk {
> + compatible = "fixed-clock";
> + #clock-cells = <1>;
> + clock-frequency  = <2700>;
> + clock-output-names = "mclk";
> + };

This node describes a fixed oscillator not connected to the I2C bus, so it 
should be moved to the top-level of the device tree instead of being a child 
of the I2C controller.

> + camera: mt9m111@48 {

No need for a label, and you should call the node according to its function, 
not the device model. Maybe something like camera-sensor@48 ? Or just 
camera@48 ?

> + compatible = "micron,mt9m111";
> + reg = <0x48>;
> +
> + clocks = < 0>;
> + clock-names = "mclk";
> +
> + port {
> + mt9m111_out: endpoint {
> + bus_width = <8>;
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> + };
> +
>  };
> 
>   {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 09/12] media: rcar: vin: Install notifier for digital input

[jmondi]

Hi Niklas,

On Mon, Jun 19, 2017 at 09:51:15PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> On 2017-06-19 19:04:46 +0200, Jacopo Mondi wrote:
> > Install async notifier for digital input on Gen3 when no other CSI-2
> > input has been found connected.
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 23 +--
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 78ca232..6e5d84a 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -866,17 +866,28 @@ static int rvin_group_graph_init(struct rvin_dev *vin)
> > }
> >
> > i = 0;
> > -   for_each_set_bit(bit, , RVIN_CSI_MAX) {
> > -   subdevs[i++] = >group->csi[bit].asd;
> > +   for_each_set_bit(bit, , RVIN_INPUT_MAX) {
> > +   if (bit < RVIN_CSI_MAX)
> > +   subdevs[i++] = >group->csi[bit].asd;
> > +   else if (bit == RVIN_PARALLEL_IN) {
> > +   subdevs[0] = >digital.asd;
> > +   vin->notifier.num_subdevs = 1;
> > +   }
> > }
> >
> > vin_dbg(vin, "Claimed %d subdevices for group\n", count);
> >
> > -   vin->notifier.num_subdevs = count;
> > vin->notifier.subdevs = subdevs;
> > -   vin->notifier.bound = rvin_group_notify_bound;
> > -   vin->notifier.unbind = rvin_group_notify_unbind;
> > -   vin->notifier.complete = rvin_group_notify_complete;
> > +   if (!vin->notifier.num_subdevs) {
> > +   vin->notifier.num_subdevs = count;
> > +   vin->notifier.bound = rvin_group_notify_bound;
> > +   vin->notifier.unbind = rvin_group_notify_unbind;
> > +   vin->notifier.complete = rvin_group_notify_complete;
> > +   } else {
> > +   vin->notifier.bound = rvin_digital_notify_bound;
> > +   vin->notifier.unbind = rvin_digital_notify_unbind;
> > +   vin->notifier.complete = rvin_digital_notify_complete;
> > +   }
>
> Hum, if there is a digital subdevice you ignore to look for CSI-2
> devices? This feels wrong, it probably works (for now) since all CSI-2
> subdevs are found by VIN0 since it's probed first and the digital input
> is only valid for VIN4 and VIN5 right? I hope to remedy this before the
> VIN Gen3 code is ready for upstream and the goal is that each VIN should
> look for the subdevices that effects it self. With that change this will
> break :-(

This is something I probably did not get: I thought parallel input and
CSI-2 input where mutually exclusive. In particular, looking at chsel
tables (from Table 26.14 on), where routing between CSI inputs and VIN
channel is described, I did not get how parallel input fit in that
picture. My best assumption now is that when a channel is NC it means
it can be used for digital input?

>
> >
> > mutex_unlock(>group->lock);
> >
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

Re: [PATCH v1 05/12] media: rcar: vin: Prepare to parse Gen3 digital input

[jmondi]

Hi Niklas,
thanks for review

On Mon, Jun 19, 2017 at 09:39:46PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2017-06-19 19:04:42 +0200, Jacopo Mondi wrote:
> > Support parsing digital input on configurable port id and reg
> > properties. Also make the function return -ENOENT when no subdevice is
> > found.
> > This change is needed to support parsing digital input on Gen3.
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 22 --
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 175f138..ef61bcc 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -511,7 +511,9 @@ static int rvin_digital_notify_bound(struct 
> > v4l2_async_notifier *notifier,
> > return 0;
> >  }
> >
> > -static int rvin_digital_graph_parse(struct rvin_dev *vin)
> > +static int rvin_digital_graph_parse(struct rvin_dev *vin,
> > +   unsigned int port,
> > +   unsigned int reg)
> >  {
> > struct device_node *ep, *np;
> > int ret;
> > @@ -519,13 +521,9 @@ static int rvin_digital_graph_parse(struct rvin_dev 
> > *vin)
> > vin->digital.asd.match.fwnode.fwnode = NULL;
> > vin->digital.subdev = NULL;
> >
> > -   /*
> > -* Port 0 id 0 is local digital input, try to get it.
> > -* Not all instances can or will have this, that is OK
> > -*/
> > -   ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, 0, 0);
> > +   ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, port, reg);
>
> I don't think it's necessary to supply the port and reg from the caller.
> If the DT proposed in rcar_vin.txt is used.
>
> > if (!ep)
> > -   return 0;
> > +   return -ENOENT;
>
> I'm not saying this is wrong, but why do you change this? I can't see a
> clear advantage in doing so. And if do it I think it should be done in a
> separate patch explaining why.

For both questions, as I parse entries in port@2 and
rvin_digital_notify_bound was intended to use port@0 by default I made
port and reg parameters. I use -ENOENT to distinguish the case where
no endpoint has been found

>
> >
> > np = of_graph_get_remote_port_parent(ep);
> > if (!np) {
> > @@ -555,11 +553,15 @@ static int rvin_digital_graph_init(struct rvin_dev 
> > *vin)
> > if (ret)
> > return ret;
> >
> > -   ret = rvin_digital_graph_parse(vin);
> > -   if (ret)
> > +   /*
> > +* Port 0 id 0 is local digital input, try to get it.
> > +* Not all instances can or will have this, that is OK
> > +*/
> > +   ret = rvin_digital_graph_parse(vin, 0, 0);
> > +   if (ret && ret != -ENOENT)
> > return ret;
> >
> > -   if (!vin->digital.asd.match.fwnode.fwnode) {
> > +   if (ret == -ENOENT) {
> > vin_dbg(vin, "No digital subdevice found\n");
> > return -ENODEV;
> > }
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

Re: [PATCH v1 01/12] arm64: boot: dts: salvator-x: Add camera module

[Geert Uytterhoeven]

Hi Jacopo,

On Mon, Jun 19, 2017 at 7:04 PM, Jacopo Mondi  wrote:
> Add camera module to Salvator-X M3W device tree.
> The camera module sits on a i2c-gpio interface and it connected to VIN

is connected

> channel #4 parallel video input port.
>
> Signed-off-by: Jacopo Mondi 
> ---
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 35 
> ++
>  1 file changed, 35 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts 
> b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> index ca4eb98..41c94c3 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> @@ -103,6 +103,41 @@
> states = <330 1
>   180 0>;
> };
> +
> +   camera_module {
> +   compatible = "i2c-gpio";
> +   gpios = < 9 GPIO_ACTIVE_HIGH
> + 11 GPIO_ACTIVE_HIGH
> +   >;
> +
> +   i2c-gpio,delay-us = <4>;
> +
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   mclk: xclk {
> +   compatible = "fixed-clock";
> +   #clock-cells = <1>;
> +   clock-frequency  = <2700>;
> +   clock-output-names = "mclk";

Please the drop clock-output-names for clock with a single output, as it is
deprecated.  The clock name will then be taken from the node name, so you
may want to adjust that.

> +   };

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Rebasing mmc/next

[Geert Uytterhoeven]

Hi Ulf,

It looks like you rebase mmc/next almost daily. Is there any specific reason
for that?

I'm asking because I create a "renesas-drivers" tree on a regular basis
(cfr. e.g. https://www.spinics.net/lists/linux-renesas-soc/msg15111.html).
This tree is meant to ease development of platform support and drivers
for Renesas ARM SoCs. It is created by merging (a) the for-next branches
of various subsystem trees and (b) branches with driver code submitted
or planned for submission to maintainers into the development branch of
Simon Horman's renesas.git tree.

If for (b), people submit driver code based on mmc/next, it may start
to conflict
with subsequent mmc/next releases soon, requiring the submitter or me to
rebase the code before including it in renesas-drivers.

Most subsystem maintainers don't rebase their for-next branch, unless there's
a very good reason for it (e.g. a serious breakage hindering bisection).

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds