Re: [PATCH v4] dt-bindings: pwm: Add R-Car M3-W device tree bindings

[Thierry Reding]

On Fri, May 12, 2017 at 10:09:42AM +0200, Simon Horman wrote:
> From: Ulrich Hecht 
> 
> Add device tree bindings for the PWM controller found on R-Car M3-W SoCs.
> 
> Signed-off-by: Ulrich Hecht 
> ---
> v4
> * Broke out of lager patchset on which it appears to have no dependencies
> * Added Reviewed by tags 
> ---
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

[PATCH v2] ASoC: hdmi-codec: ELD control corresponds to the PCM stream

[Kuninori Morimoto]

From: Kuninori Morimoto 

Current hdmi-codec driver is using hdmi_controls for "ELD" control.
But, hdmi-codec driver might be used from many HDMIs. Thus, we need
to correspond device number, otherwise we will receive below error.

xxx: control x:x:x:ELD:x is already present

This patch registers ELD control in .pcm_new by using
.device = rtd->pcm->device to corresponding to PCM stream.

Signed-off-by: Kuninori Morimoto 
[Takashi: use snd_ctl_new1()/snd_ctl_add()]
Signed-off-by: Takashi Iwai 
---
v1 -> v2

 - change title
- ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new
+ ASoC: hdmi-codec: ELD control corresponds to the PCM stream
 - hdmi_eld_ctl is now local variable
 - Author should be Takashi ?

 sound/soc/codecs/hdmi-codec.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc..7686a80 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -399,18 +399,6 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
*kcontrol,
return 0;
 }
 
-
-static const struct snd_kcontrol_new hdmi_controls[] = {
-   {
-   .access = SNDRV_CTL_ELEM_ACCESS_READ |
- SNDRV_CTL_ELEM_ACCESS_VOLATILE,
-   .iface = SNDRV_CTL_ELEM_IFACE_PCM,
-   .name = "ELD",
-   .info = hdmi_eld_ctl_info,
-   .get = hdmi_eld_ctl_get,
-   },
-};
-
 static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
 struct snd_soc_dai *dai)
 {
@@ -668,6 +656,16 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
 {
struct snd_soc_dai_driver *drv = dai->driver;
struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+   struct snd_kcontrol *kctl;
+   struct snd_kcontrol_new hdmi_eld_ctl = {
+   .access = SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+   .iface  = SNDRV_CTL_ELEM_IFACE_PCM,
+   .name   = "ELD",
+   .info   = hdmi_eld_ctl_info,
+   .get= hdmi_eld_ctl_get,
+   .device = rtd->pcm->device,
+   };
int ret;
 
dev_dbg(dai->dev, "%s()\n", __func__);
@@ -686,7 +684,12 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
hcp->chmap_info->chmap = hdmi_codec_stereo_chmaps;
hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
 
-   return 0;
+   /* add ELD ctl with the device number corresponding to the PCM stream */
+   kctl = snd_ctl_new1(_eld_ctl, dai->component);
+   if (!kctl)
+   return -ENOMEM;
+
+   return snd_ctl_add(rtd->card->snd_card, kctl);
 }
 
 static struct snd_soc_dai_driver hdmi_i2s_dai = {
@@ -732,8 +735,6 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
*component,
 
 static struct snd_soc_codec_driver hdmi_codec = {
.component_driver = {
-   .controls   = hdmi_controls,
-   .num_controls   = ARRAY_SIZE(hdmi_controls),
.dapm_widgets   = hdmi_widgets,
.num_dapm_widgets   = ARRAY_SIZE(hdmi_widgets),
.dapm_routes= hdmi_routes,
-- 
1.9.1

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Stephen Boyd]

On 06/28, Yoshihiro Shimoda wrote:
> +
> +
> + platform_set_drvdata(pdev, priv);
> + dev_set_drvdata(dev, priv);
> +
> + init.name = "rcar_usb2_clock_sel";
> + init.ops = _clock_sel_clock_ops;
> + init.flags = CLK_IS_BASIC;

Please drop CLK_IS_BASIC unless you need it.

> + init.parent_names = NULL;
> + init.num_parents = 0;
> + priv->hw.init = 
> +
> + clk = clk_register(NULL, >hw);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + error = of_clk_add_provider(np, of_clk_src_simple_get, clk);

Can you use clk_hw_register() and of_clk_add_hw_provider()
please?

> + if (error)
> + return error;
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops rcar_usb2_clock_sel_pm_ops = {
> + .suspend= rcar_usb2_clock_sel_suspend,
> + .resume = rcar_usb2_clock_sel_resume,
> +};
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6] media: platform: Renesas IMR driver

[Sergei Shtylyov]

Hello!

On 07/03/2017 03:25 PM, Hans Verkuil wrote:


From: Konstantin Kozhevnikov 

The image renderer, or the distortion correction engine, is a drawing
processor with a simple instruction system capable of referencing video
capture data or data in an external memory as the 2D texture data and
performing texture mapping and drawing with respect to any shape that is
split into triangular objects.

This V4L2 memory-to-memory device driver only supports image renderer light
extended 4 (IMR-LX4) found in the R-Car gen3 SoCs; the R-Car gen2 support
can be added later...

[Sergei: merged 2 original patches, added  the patch description, removed
unrelated parts,  added the binding document and the UAPI documentation,
ported the driver to the modern kernel, renamed the UAPI header file and
the guard macros to match the driver name, extended the copyrights, fixed
up Kconfig prompt/depends/help, made use of the BIT/GENMASK() macros,
sorted  #include's, replaced 'imr_ctx::crop' array with the 'imr_ctx::rect'
structure, replaced imr_{g|s}_crop() with imr_{g|s}_selection(), completely
rewrote imr_queue_setup(), removed 'imr_format_info::name', moved the
applicable code from imr_buf_queue() to imr_buf_prepare() and moved the
rest of imr_buf_queue() after imr_buf_finish(), assigned 'src_vq->dev' and
'dst_vq->dev' in imr_queue_init(), removed imr_start_streaming(), assigned
'src_vq->dev' and 'dst_vq->dev' in imr_queue_init(), clarified the math in
imt_tri_type_{a|b|c}_length(), clarified the pointer math and avoided casts
to 'void *' in imr_tri_set_type_{a|b|c}(), replaced imr_{reqbufs|querybuf|
dqbuf|expbuf|streamon|streamoff}() with the generic helpers, implemented
vidioc_{create_bufs|prepare_buf}() methods, used ALIGN() macro and merged
the matrix size checks and replaced kmalloc()/copy_from_user() calls with
memdup_user() call in imr_ioctl_map(), moved setting device capabilities
from imr_querycap() to imr_probe(), set the valid default queue format in
imr_probe(), removed leading dots and fixed grammar in the comments, fixed
up  the indentation  to use  tabs where possible, renamed DLSR, CMRCR.
DY1{0|2}, and ICR bits to match the manual, changed the prefixes of the
CMRCR[2]/TRI{M|C}R bits/fields to match the manual, removed non-existent
TRIMR.D{Y|U}D{X|V}M bits, added/used the IMR/{UV|CP}DPOR/SUSR bits/fields/
shifts, separated the register offset/bit #define's, sorted instruction
macros by opcode, removed unsupported LINE instruction, masked the register
address in WTL[2]/WTS instruction macros, moved the display list #define's
after the register #define's, removing the redundant comment, avoided
setting reserved bits when writing CMRCCR[2]/TRIMCR, used the SR bits
instead of a bare number, removed *inline* from .c file, fixed lines over
80 columns, removed useless spaces, comments, parens, operators, casts,
braces, variables, #include's, statements, and even 1 function, added
useful local variable, uppercased and spelled out the abbreviations,
made comment wording more consistent/correct, fixed the comment typos,
reformatted some multiline comments, inserted empty line after declaration,
removed extra empty lines,  reordered some local variable desclarations,
removed calls to 4l2_err() on kmalloc() failure, replaced '*' with 'x'
in some format strings for v4l2_dbg(), fixed the error returned by
imr_default(), avoided code duplication in the IRQ handler, used '__packed'
for the UAPI structures, declared 'imr_map_desc::data' as '__u64' instead
of 'void *', switched to '__u{16|32}' in the UAPI header, enclosed the
macro parameters in parens, exchanged the values of IMR_MAP_AUTO{S|D}G
macros.]


As Geert suggested, just replace this with a 'Based-on' line.


   OK, the list grew too long indeed. :-)





Index: media_tree/drivers/media/platform/rcar_imr.c
===
--- /dev/null
+++ media_tree/drivers/media/platform/rcar_imr.c
@@ -0,0 +1,1877 @@





+/* add reference to the current configuration */
+static struct imr_cfg *imr_cfg_ref(struct imr_ctx *ctx)


imr_cfg_ref -> imr_cfg_ref_get


   OK, but imr_cfg_get() seems a better name.


+{
+struct imr_cfg *cfg = ctx->cfg;
+
+BUG_ON(!cfg);


Perhaps this can be replaced by:

if (WARN_ON(!cfg))
return NULL;


   I'm afraid imr_device_run() will cause oops in this case...


+cfg->refcount++;
+return cfg;
+}
+
+/* mesh configuration destructor */
+static void imr_cfg_unref(struct imr_ctx *ctx, struct imr_cfg *cfg)


imr_cfg_unref -> imr_cfg_ref_put


  OK, but I'll call it imr_cfg_put().


That follows the standard naming conventions for refcounting.


+{
+struct imr_device *imr = ctx->imr;
+
+/* no atomicity is required as operation is locked with device mutex */
+if (!cfg || --cfg->refcount)
+return;
+
+/* release memory allocated for a display list */
+if (cfg->dl_vaddr)
+dma_free_writecombine(imr->dev, 

Re: [PATCHv6] arm64: dts: renesas: salvator-x: Add ADV7482 support

[Niklas Söderlund]

On 2017-07-05 11:22:21 +0100, Kieran Bingham wrote:
> On 05/07/17 10:43, Geert Uytterhoeven wrote:
> > On Tue, Jun 27, 2017 at 5:15 PM, Kieran Bingham  wrote:
> >> From: Kieran Bingham 
> >>
> >> The Salvator boards use an ADV7482 receiver for HDMI and CVBS inputs.
> >>
> >> Provide ADV7482 node on the i2c4 bus, along with connectors for the
> >> hdmi and cvbs inputs, and link to the csi20 and csi40 nodes as outputs.
> >>
> >> Signed-off-by: Kieran Bingham 
> >> Reviewed-by: Laurent Pinchart 
> >>
> >> ---
> >>
> >> Submitting this directly to Niklas, as these changes depend on his VIN
> >> DT patches, and has agreed to 'take' this patch on to that series.
> > 
> > Yeah, haven't seen any patches yet adding "csi20" and "csi40" nodes ;-)
> 
> I defer this comment to Niklas :-)

Yes :-)

My plan is to include this in my vin-dt branch which I also would like 
to include in my submission to renesas-drivers. But given the fact that 
I up until now have not included any DT changes for VIN in my 
renesas-drivers pull requests and that the DT file structure is recently 
changed I have yet to do so.

My plan was to wait for -rc1 and rebase the DT patches on top of that 
and then ask Geert and/or Simon to have a quick look at it so that I 
don't create problems for the next renesas-drivers release. But if you 
wish I can expedite this and rebase it on-top of the latest 
renesas-drivers release and pester someone to look at it so that it's 
ready for the next renesas-drivers release :-)

> 
> --
> Kieran
> 
> 
> > 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
> > 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 2/2] ARM: shmobile: defconfig: Enable Ethernet AVB

[Sergei Shtylyov]

Hello!

On 07/05/2017 06:56 PM, Biju Das wrote:


The iWave RZ/G1M Q7 SOM supports Gigabit Ethernet Phy (Micrel KSZ9031MNX).
Gigabit Ethernet support is available in Renesas AVB driver.
To increase hardware support enable the driver in the shmobile_defconfig
multiplatform configuration.

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
Acked-by: Geert Uytterhoeven 
---
 arch/arm/configs/shmobile_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/shmobile_defconfig 
b/arch/arm/configs/shmobile_defconfig
index 3c66a42..d992911 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -83,6 +83,7 @@ CONFIG_NETDEVICES=y
 # CONFIG_NET_VENDOR_MICREL is not set
 # CONFIG_NET_VENDOR_NATSEMI is not set
 CONFIG_SH_ETH=y
+CONFIG_RAVB=y


   Hm, I would enable it only as a module, given it's not easy to make use of 
AVB on the 32-bit boards.


[...]

MBR Sergei

[PATCH v2 0/2] Add RZ/G1M (R8A7743) Ethernet AVB support

[Biju Das]

Hi All,

This series aims to upstream the Ethernet AVB driver support for RZ/G1M 
(r8a7743) SoC . 

The iWave RZ/G1M Qseven SOM supports Gigabit Ethernet Phy from 
Micrel(KSZ9031MNX).
Gigabit Ethernet support is available in Renesas AVB driver.

This series has been tested against linux-next tag next-20170704.

Modifications:
v1->v2
-Dropped the driver update,as is it not needed due to the presence of 
family-specific compatible values.
-Added Reviewed-by: Geert Uytterhoeven  for the DT 
binding patch
-Added Acked-by: Geert Uytterhoeven  for the 
defconfig: Enable Ethernet AVB patch

History:
---
[v1]: https://www.spinics.net/lists/arm-kernel/msg592366.html

Biju Das (2):
  ravb: Document binding for r8a7743 SoC
  ARM: shmobile: defconfig: Enable Ethernet AVB

 Documentation/devicetree/bindings/net/renesas,ravb.txt | 3 ++-
 arch/arm/configs/shmobile_defconfig| 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
1.9.1

[PATCH v2 2/2] ARM: shmobile: defconfig: Enable Ethernet AVB

[Biju Das]

The iWave RZ/G1M Q7 SOM supports Gigabit Ethernet Phy (Micrel KSZ9031MNX).
Gigabit Ethernet support is available in Renesas AVB driver.
To increase hardware support enable the driver in the shmobile_defconfig
multiplatform configuration.

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
Acked-by: Geert Uytterhoeven 
---
 arch/arm/configs/shmobile_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/shmobile_defconfig 
b/arch/arm/configs/shmobile_defconfig
index 3c66a42..d992911 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -83,6 +83,7 @@ CONFIG_NETDEVICES=y
 # CONFIG_NET_VENDOR_MICREL is not set
 # CONFIG_NET_VENDOR_NATSEMI is not set
 CONFIG_SH_ETH=y
+CONFIG_RAVB=y
 # CONFIG_NET_VENDOR_SEEQ is not set
 CONFIG_SMSC911X=y
 # CONFIG_NET_VENDOR_STMICRO is not set
-- 
1.9.1

[PATCH v2 1/2] ravb: Document binding for r8a7743 SoC

[Biju Das]

Add a new compatible string for the RZ/G1M (R8A7743) SoC.

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
Reviewed-by: Geert Uytterhoeven 
---
 Documentation/devicetree/bindings/net/renesas,ravb.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt 
b/Documentation/devicetree/bindings/net/renesas,ravb.txt
index b519503..bc692ab 100644
--- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
+++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
@@ -4,7 +4,8 @@ This file provides information on what the device node for the 
Ethernet AVB
 interface contains.
 
 Required properties:
-- compatible: "renesas,etheravb-r8a7790" if the device is a part of R8A7790 
SoC.
+- compatible: "renesas,etheravb-r8a7743" if the device is a part of R8A7743 
SoC.
+ "renesas,etheravb-r8a7790" if the device is a part of R8A7790 SoC.
  "renesas,etheravb-r8a7791" if the device is a part of R8A7791 SoC.
  "renesas,etheravb-r8a7792" if the device is a part of R8A7792 SoC.
  "renesas,etheravb-r8a7793" if the device is a part of R8A7793 SoC.
-- 
1.9.1

RE: [PATCH v 1/2] ravb: Add support for r8a7743 SoC

[Biju Das]

> -Original Message-
> From: devicetree-ow...@vger.kernel.org [mailto:devicetree-
> ow...@vger.kernel.org] On Behalf Of Sergei Shtylyov
> Sent: 05 July 2017 14:21
> To: Biju Das ; Rob Herring ;
> Mark Rutland ; Russell King
> 
> Cc: Simon Horman ; Magnus Damm
> ; Chris Paterson
> ; devicet...@vger.kernel.org; linux-renesas-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> net...@vger.kernel.org
> Subject: Re: [PATCH v 1/2] ravb: Add support for r8a7743 SoC
>
> Hello!
>
> On 07/05/2017 04:01 PM, Biju Das wrote:
>
> > Add support for Gigabit Ethernet E-MAC on r8a7743 (RZ/G1M) SoC.
> > Renesas RZ/G1M (R8A7743) SoC Ethernet AVB IP is identical to the R-Car
> > Gen2 family.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Chris Paterson 
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 5931e85..e35b30f 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1869,6 +1869,7 @@ static int ravb_mdio_release(struct ravb_private
> > *priv)  }
> >
> >  static const struct of_device_id ravb_match_table[] = {
> > +{ .compatible = "renesas,etheravb-r8a7743", .data = (void
> > +*)RCAR_GEN2 },
>
> No, this shouldn't be needed if you specify "renesas,etheravb-rcar-gen2".
>

Thanks for the review. I will remove this change.

> >  { .compatible = "renesas,etheravb-r8a7790", .data = (void
> *)RCAR_GEN2 },
> >  { .compatible = "renesas,etheravb-r8a7794", .data = (void
> *)RCAR_GEN2 },
> >  { .compatible = "renesas,etheravb-rcar-gen2", .data = (void
> > *)RCAR_GEN2 },
>
> MBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in the 
> body
> of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

RE: [PATCH v 1/2] ravb: Add support for r8a7743 SoC

[Chris Paterson]

Hello Geert,

> From: geert.uytterhoe...@gmail.com
> [mailto:geert.uytterhoe...@gmail.com] On Behalf Of Geert Uytterhoeven
> Sent: 05 July 2017 15:56
> 
> Hi Chris,
> 
> On Wed, Jul 5, 2017 at 3:51 PM, Chris Paterson
>  wrote:
> >> From: geert.uytterhoe...@gmail.com
> >> [mailto:geert.uytterhoe...@gmail.com] On Behalf Of Geert
> Uytterhoeven
> >> Sent: 05 July 2017 14:47
> >> On Wed, Jul 5, 2017 at 3:01 PM, Biju Das 
> wrote:
> >> > Add support for Gigabit Ethernet E-MAC on r8a7743 (RZ/G1M) SoC.
> >> > Renesas RZ/G1M (R8A7743) SoC Ethernet AVB IP is identical to the
> >> > R-Car
> >> > Gen2 family.
> >> >
> >> > Signed-off-by: Biju Das 
> >> > Reviewed-by: Chris Paterson 
> >>
> >> Thanks for your patch!
> >>
> >> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> >> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >> > @@ -1869,6 +1869,7 @@ static int ravb_mdio_release(struct
> >> > ravb_private
> >> > *priv)  }
> >> >
> >> >  static const struct of_device_id ravb_match_table[] = {
> >> > +   { .compatible = "renesas,etheravb-r8a7743", .data = (void
> >> > + *)RCAR_GEN2 },
> >> > { .compatible = "renesas,etheravb-r8a7790", .data = (void
> >> *)RCAR_GEN2 },
> >> > { .compatible = "renesas,etheravb-r8a7794", .data = (void
> >> *)RCAR_GEN2 },
> >> > { .compatible = "renesas,etheravb-rcar-gen2", .data = (void
> >> > *)RCAR_GEN2 },
> >>
> >> As Sergei already mentioned, a driver update is not needed due to the
> >> presence of family-specific compatible values.
> >> Please drop that part, and you can add my:
> >> Reviewed-by: Geert Uytterhoeven 
> >
> > Should the other compatible values (r8a7790, 94 etc) be removed then? Or
> are they needed for backwards compatibility?
> 
> The other compatible values in DT bindings and DTS files should be kept, to
> allow handling of SoC-specific quirks if/when they are ever detected.
> 
> The other compatible values in the driver should be kept for backwards-
> compatibility with old DTB files that lack the (newer) family-specific
> compatible values.

Thank you for the comprehensive answer.

Kind regards, Chris


> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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: [PATCH v 1/2] ravb: Add support for r8a7743 SoC

[Geert Uytterhoeven]

Hi Chris,

On Wed, Jul 5, 2017 at 3:51 PM, Chris Paterson
 wrote:
>> From: geert.uytterhoe...@gmail.com
>> [mailto:geert.uytterhoe...@gmail.com] On Behalf Of Geert Uytterhoeven
>> Sent: 05 July 2017 14:47
>> On Wed, Jul 5, 2017 at 3:01 PM, Biju Das  wrote:
>> > Add support for Gigabit Ethernet E-MAC on r8a7743 (RZ/G1M) SoC.
>> > Renesas RZ/G1M (R8A7743) SoC Ethernet AVB IP is identical to the R-Car
>> > Gen2 family.
>> >
>> > Signed-off-by: Biju Das 
>> > Reviewed-by: Chris Paterson 
>>
>> Thanks for your patch!
>>
>> > --- a/drivers/net/ethernet/renesas/ravb_main.c
>> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> > @@ -1869,6 +1869,7 @@ static int ravb_mdio_release(struct ravb_private
>> > *priv)  }
>> >
>> >  static const struct of_device_id ravb_match_table[] = {
>> > +   { .compatible = "renesas,etheravb-r8a7743", .data = (void
>> > + *)RCAR_GEN2 },
>> > { .compatible = "renesas,etheravb-r8a7790", .data = (void
>> *)RCAR_GEN2 },
>> > { .compatible = "renesas,etheravb-r8a7794", .data = (void
>> *)RCAR_GEN2 },
>> > { .compatible = "renesas,etheravb-rcar-gen2", .data = (void
>> > *)RCAR_GEN2 },
>>
>> As Sergei already mentioned, a driver update is not needed due to the
>> presence of family-specific compatible values.
>> Please drop that part, and you can add my:
>> Reviewed-by: Geert Uytterhoeven 
>
> Should the other compatible values (r8a7790, 94 etc) be removed then? Or are 
> they needed for backwards compatibility?

The other compatible values in DT bindings and DTS files should be kept,
to allow handling of SoC-specific quirks if/when they are ever detected.

The other compatible values in the driver should be kept for
backwards-compatibility with old DTB files that lack the (newer)
family-specific compatible values.

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: [PATCH v2] media: platform: rcar_imr: add IMR-LX3 support

[Rob Herring]

On Wed, Jun 28, 2017 at 10:58:44PM +0300, Sergei Shtylyov wrote:
> Add support for the image renderer light extended 3 (IMR-LX3) found only in
> the R-Car V2H (R8A7792) SoC. It's mostly the same as IMR-LSX3 but doesn't
> support video capture data as a source of 2D textures.
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch  is against the 'media_tree.git' repo's 'master' branch plus the
> latest version of  the Renesas IMR driver and the patch adding IMR-LSX3 
> support.
> 
> Changes in version 2:
> - refreshed the patch atop of the IMR driver patch (version 6) and IMR-LSX3
>   patch (version 3).
> 
>  Documentation/devicetree/bindings/media/rcar_imr.txt |4 ++

Acked-by: Rob Herring 

>  drivers/media/platform/rcar_imr.c|   27 
> ---
>  2 files changed, 22 insertions(+), 9 deletions(-)

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Rob Herring]

On Wed, Jun 28, 2017 at 03:28:35PM +0900, Yoshihiro Shimoda wrote:
> R-Car USB 2.0 controller can change the clock source from an oscillator
> to an external clock via a register. So, this patch adds support
> the clock source selector as a clock driver.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  This patch is based on the renesas-drivers.git /
> renesas-drivers-2017-06-27-v4.12-rc7 tag.
> 
>  Changes from v1:
>   - Change this driver as a clock driver from a generic phy driver.
> https://patchwork.kernel.org/patch/9788697/
>   - Remove "RFC" tag.
> 
>  .../bindings/clock/renesas,rcar-usb2-clock-sel.txt |  54 ++
>  drivers/clk/renesas/Kconfig|   5 +
>  drivers/clk/renesas/Makefile   |   1 +
>  drivers/clk/renesas/rcar-usb2-clock-sel.c  | 205 
> +
>  4 files changed, 265 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
>  create mode 100644 drivers/clk/renesas/rcar-usb2-clock-sel.c
> 
> diff --git 
> a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt 
> b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> new file mode 100644
> index 000..75ccafc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> @@ -0,0 +1,54 @@
> +* Renesas R-Car USB 2.0 clock selector
> +
> +This file provides information on what the device node for the R-Car USB 2.0
> +clock selector.
> +
> +If you connect an external clock to the USB_EXTAL pin only, you should set
> +the clock rate to "usb_extal" node only.
> +If you connect an oscillator to both the USB_XTAL and USB_EXTAL, this module
> +is not needed because this is default setting. (Of course, you can set the
> +clock rates to both "usb_extal" and "usb_xtal" nodes.
> +
> +Case 1: An external clock connects to R-Car SoC
> ++--+   +--- R-Car -+
> +|External  |---|USB_EXTAL ---> all usb channels|
> +|clock |   |USB_XTAL   |
> ++--+   +---+
> +In this case, we need this driver with "usb_extal" clock.
> +
> +Case 2: An oscillator connects to R-Car SoC
> ++--+   +--- R-Car -+
> +|Oscillator|---|USB_EXTAL -+-> all usb channels|
> +|  |---|USB_XTAL --+   |
> ++--+   +---+
> +In this case, we don't need this selector.
> +
> +Required properties:
> +- compatible: "renesas,r8a7795-rcar-usb2-clock-sel" if the device is a part 
> of
> +   an R8A7795 SoC.
> +  "renesas,r8a7796-rcar-usb2-clock-sel" if the device if a part 
> of
> +   an R8A7796 SoC.
> +  "renesas,rcar-gen3-usb2-clock-sel" for a generic R-Car Gen3
> +  compatible device.
> +
> +  When compatible with the generic version, nodes must list the
> +  SoC-specific version corresponding to the platform first
> +  followed by the generic version.
> +
> +- reg: offset and length of the USB 2.0 clock selector register block.
> +- clocks: A list of phandles and specifier pairs.
> +- clock-names: Name of the clocks.
> + - The functional clock must be "ehci_ohci"
> + - The USB_EXTAL clock pin must be "usb_extal"
> + - The USB_XTAL clock pin must be "usb_xtal"
> +- #clock-cells: Must be 0
> +
> +Exxample (R-Car H3):
> +
> +usb2_clksel: clock-controller@e6590630 {
> +compatible = "renesas,r8a77950-rcar-usb2-clock-sel",
> + "renesas,rcar-gen3-usb2-clock-sel";
> +reg = <0 0xe6590630 0 0x02>;
> +clocks = < CPG_MOD 703>, <_extal>, <_xtal>;
> +clock-names = "ehci_ohci", "usb_extal", "usb_xtal";

Missing #clock-cells

With that, for the binding:

Acked-by: Rob Herring 

Rob

RE: [PATCH v 1/2] ravb: Add support for r8a7743 SoC

[Chris Paterson]

Hello Geert, Sergei,

> From: geert.uytterhoe...@gmail.com
> [mailto:geert.uytterhoe...@gmail.com] On Behalf Of Geert Uytterhoeven
> Sent: 05 July 2017 14:47
> 
> Hi Biju,
> 
> On Wed, Jul 5, 2017 at 3:01 PM, Biju Das  wrote:
> > Add support for Gigabit Ethernet E-MAC on r8a7743 (RZ/G1M) SoC.
> > Renesas RZ/G1M (R8A7743) SoC Ethernet AVB IP is identical to the R-Car
> > Gen2 family.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Chris Paterson 
> 
> Thanks for your patch!
> 
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1869,6 +1869,7 @@ static int ravb_mdio_release(struct ravb_private
> > *priv)  }
> >
> >  static const struct of_device_id ravb_match_table[] = {
> > +   { .compatible = "renesas,etheravb-r8a7743", .data = (void
> > + *)RCAR_GEN2 },
> > { .compatible = "renesas,etheravb-r8a7790", .data = (void
> *)RCAR_GEN2 },
> > { .compatible = "renesas,etheravb-r8a7794", .data = (void
> *)RCAR_GEN2 },
> > { .compatible = "renesas,etheravb-rcar-gen2", .data = (void
> > *)RCAR_GEN2 },
> 
> As Sergei already mentioned, a driver update is not needed due to the
> presence of family-specific compatible values.
> Please drop that part, and you can add my:
> Reviewed-by: Geert Uytterhoeven 

Should the other compatible values (r8a7790, 94 etc) be removed then? Or are 
they needed for backwards compatibility?

Kind regards, Chris

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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: [PATCH v 1/2] ravb: Add support for r8a7743 SoC

[Sergei Shtylyov]

Hello!

On 07/05/2017 04:01 PM, Biju Das wrote:


Add support for Gigabit Ethernet E-MAC on r8a7743 (RZ/G1M) SoC.
Renesas RZ/G1M (R8A7743) SoC Ethernet AVB IP is identical to the R-Car Gen2
family.

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 

[...]

diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
b/drivers/net/ethernet/renesas/ravb_main.c
index 5931e85..e35b30f 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1869,6 +1869,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 }

 static const struct of_device_id ravb_match_table[] = {
+   { .compatible = "renesas,etheravb-r8a7743", .data = (void *)RCAR_GEN2 },


   No, this shouldn't be needed if you specify "renesas,etheravb-rcar-gen2".


{ .compatible = "renesas,etheravb-r8a7790", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,etheravb-r8a7794", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,etheravb-rcar-gen2", .data = (void *)RCAR_GEN2 
},


MBR, Sergei

[PATCH v 2/2] ARM: shmobile: defconfig: Enable Ethernet AVB

[Biju Das]

The iWave RZ/G1M Q7 SOM supports Gigabit Ethernet Phy (Micrel KSZ9031MNX).
Gigabit Ethernet support is available in Renesas AVB driver.
To increase hardware support enable the driver in the shmobile_defconfig
multiplatform configuration.

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
---
 arch/arm/configs/shmobile_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/shmobile_defconfig 
b/arch/arm/configs/shmobile_defconfig
index 3c66a42..d992911 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -83,6 +83,7 @@ CONFIG_NETDEVICES=y
 # CONFIG_NET_VENDOR_MICREL is not set
 # CONFIG_NET_VENDOR_NATSEMI is not set
 CONFIG_SH_ETH=y
+CONFIG_RAVB=y
 # CONFIG_NET_VENDOR_SEEQ is not set
 CONFIG_SMSC911X=y
 # CONFIG_NET_VENDOR_STMICRO is not set
-- 
1.9.1

[PATCH v 1/2] ravb: Add support for r8a7743 SoC

[Biju Das]

Add support for Gigabit Ethernet E-MAC on r8a7743 (RZ/G1M) SoC.
Renesas RZ/G1M (R8A7743) SoC Ethernet AVB IP is identical to the R-Car Gen2
family.

Signed-off-by: Biju Das 
Reviewed-by: Chris Paterson 
---
 Documentation/devicetree/bindings/net/renesas,ravb.txt | 3 ++-
 drivers/net/ethernet/renesas/ravb_main.c   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt 
b/Documentation/devicetree/bindings/net/renesas,ravb.txt
index b519503..bc692ab 100644
--- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
+++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
@@ -4,7 +4,8 @@ This file provides information on what the device node for the 
Ethernet AVB
 interface contains.
 
 Required properties:
-- compatible: "renesas,etheravb-r8a7790" if the device is a part of R8A7790 
SoC.
+- compatible: "renesas,etheravb-r8a7743" if the device is a part of R8A7743 
SoC.
+ "renesas,etheravb-r8a7790" if the device is a part of R8A7790 SoC.
  "renesas,etheravb-r8a7791" if the device is a part of R8A7791 SoC.
  "renesas,etheravb-r8a7792" if the device is a part of R8A7792 SoC.
  "renesas,etheravb-r8a7793" if the device is a part of R8A7793 SoC.
diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
b/drivers/net/ethernet/renesas/ravb_main.c
index 5931e85..e35b30f 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1869,6 +1869,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 }
 
 static const struct of_device_id ravb_match_table[] = {
+   { .compatible = "renesas,etheravb-r8a7743", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,etheravb-r8a7790", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,etheravb-r8a7794", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,etheravb-rcar-gen2", .data = (void *)RCAR_GEN2 
},
-- 
1.9.1

[PATCH v 0/2] Add RZ/G1M (R8A7743) Ethernet AVB support

[Biju Das]

This series aims to add Gigabit Ethernet E-MAC support on r8a7743 (RZ/G1M) SoC.
The iWave RZ/G1M Qseven SOM supports Gigabit Ethernet Phy from 
Micrel(KSZ9031MNX).
Gigabit Ethernet support is available in Renesas AVB driver.

This series has been tested against linux-next tag next-20170704.

Biju Das (2):
  ravb: Add support for r8a7743 SoC
  ARM: shmobile: defconfig: Enable Ethernet AVB

 Documentation/devicetree/bindings/net/renesas,ravb.txt | 3 ++-
 arch/arm/configs/shmobile_defconfig| 1 +
 drivers/net/ethernet/renesas/ravb_main.c   | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

-- 
1.9.1

Re: [PATCH v2 0/3] Add R8A7743/SK-RZG1M PFC support

[Simon Horman]

On Wed, Jul 05, 2017 at 03:06:59PM +0300, Sergei Shtylyov wrote:
> On 07/05/2017 01:33 PM, Simon Horman wrote:
> 
> >>>   Here's the set of 3 patches against Simon Horman's 'renesas.git' repo,
> >>>'renesas-devel-20170420-v4.11-rc7' tag.  We're adding the R8A7743 PFC node
> >>>and then describing the pins for SCIF0 and Ether devices declared earlier.
> >>>These patches depend on the R8A7743 PFC suport in order to work properly.
> >>
> >>Thanks. I have marked these as deferred pending acceptance of the PFC
> >>driver changes. Please repost or otherwise let me know when dependency
> >>is satisfied.
> >
> >It appears that the driver changes have been acceptedi[1] so
> >I have queued these up for v4.14.
> >
> >[1] 
> >https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg15744.html
> 
>This points to Biju Das' patch acceptance, not mine. :-)

Sorry, I was a bit confused there.

This is a link to the patch acceptance in question.
https://www.spinics.net/lists/linux-renesas-soc/msg14287.html

Re: [PATCH v2] arm64: dts: r8a7796: add IMR-LX4 support

[Simon Horman]

On Wed, Jul 05, 2017 at 03:03:37PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 07/05/2017 12:54 PM, Geert Uytterhoeven wrote:
> 
> >>Describe the IMR-LX4 devices in the R8A7796 device tree.
> >>
> >>Signed-off-by: Sergei Shtylyov 
> >
> >>--- renesas.orig/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >>+++ renesas/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >>@@ -1454,5 +1454,23 @@
> >>};
> >>};
> >>};
> >>+
> >>+   imr-lx4@fe86 {
> >>+   compatible = "renesas,r8a7796-imr-lx4",
> >>+"renesas,imr-lx4";
> >>+   reg = <0 0xfe86 0 0x2000>;
> >>+   interrupts = ;
> >>+   clocks = < CPG_MOD 823>;
> >>+   power-domains = < R8A7796_PD_A3VC>;
> >
> >resets = < 823>;
> >
> >>+   };
> >>+
> >>+   imr-lx4@fe87 {
> >>+   compatible = "renesas,r8a7796-imr-lx4",
> >>+"renesas,imr-lx4";
> >>+   reg = <0 0xfe87 0 0x2000>;
> >>+   interrupts = ;
> >>+   clocks = < CPG_MOD 822>;
> >>+   power-domains = < R8A7796_PD_A3VC>;
> >
> >resets = < 822>;
> >
> >With the issues above fixed:
> >
> >Reviewed-by: Geert Uytterhoeven 
> 
>I'm afraid it's already merged by Simon...

I've gone through and added Geert's tags to patches that I merged recently.
I'm planning to push the updated patches later today.

Re: [PATCH][RESEND] ARM: dts: iwg20d-q7: Add pinctl support for scif0

[Simon Horman]

On Wed, Jul 05, 2017 at 01:18:30PM +0200, Geert Uytterhoeven wrote:
> On Wed, Jul 5, 2017 at 12:57 PM, Biju Das  wrote:
> > Adding pinctrl support for scif0 interface.
> >
> > Signed-off-by: Biju Das 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.14.

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Jul 5, 2017 at 1:57 PM, Yoshihiro Shimoda
 wrote:
>> From: Geert Uytterhoeven
>> Sent: Wednesday, July 5, 2017 7:09 PM
>> On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
>>  wrote:
>> > R-Car USB 2.0 controller can change the clock source from an oscillator
>> > to an external clock via a register. So, this patch adds support
>> > the clock source selector as a clock driver.

>> > --- /dev/null
>> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c

>> > +/* Since this driver needs other ccf drivers, this uses 
>> > subsys_initcall_sync */
>> > +subsys_initcall_sync(rcar_usb2_clock_sel_init);
>>
>> I suppose this is a workaround for the lack of probe deferral support in the
>> USB subsystem?
>
> This is my fault. I added "power-domains" property into this device's node.
> After I remove the power-domains like the cpg node, this driver can use 
> subsys_initcall()
> instead of subsys_initcall_sync().

Does the clock sel module requires the functional clock "ehci_ohci" to be
running before you can access its registers?
If yes, I think there should be a "power-domains" property.

Then, you can simplify the code by calling

pm_runtime_enable(dev);
pm_runtime_get_sync(dev);

and remove the explicit handling of the functional clock.

That does not solve probe deferral handling in the USB subsystem, though.

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: [PATCH v2 0/3] Add R8A7743/SK-RZG1M PFC support

[Sergei Shtylyov]

On 07/05/2017 01:33 PM, Simon Horman wrote:


   Here's the set of 3 patches against Simon Horman's 'renesas.git' repo,
'renesas-devel-20170420-v4.11-rc7' tag.  We're adding the R8A7743 PFC node
and then describing the pins for SCIF0 and Ether devices declared earlier.
These patches depend on the R8A7743 PFC suport in order to work properly.


Thanks. I have marked these as deferred pending acceptance of the PFC
driver changes. Please repost or otherwise let me know when dependency
is satisfied.


It appears that the driver changes have been acceptedi[1] so
I have queued these up for v4.14.

[1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg15744.html


   This points to Biju Das' patch acceptance, not mine. :-)

MBR, Sergei

Re: [PATCH v2] arm64: dts: r8a7796: add IMR-LX4 support

[Sergei Shtylyov]

Hello.

On 07/05/2017 12:54 PM, Geert Uytterhoeven wrote:


Describe the IMR-LX4 devices in the R8A7796 device tree.

Signed-off-by: Sergei Shtylyov 



--- renesas.orig/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -1454,5 +1454,23 @@
};
};
};
+
+   imr-lx4@fe86 {
+   compatible = "renesas,r8a7796-imr-lx4",
+"renesas,imr-lx4";
+   reg = <0 0xfe86 0 0x2000>;
+   interrupts = ;
+   clocks = < CPG_MOD 823>;
+   power-domains = < R8A7796_PD_A3VC>;


resets = < 823>;


+   };
+
+   imr-lx4@fe87 {
+   compatible = "renesas,r8a7796-imr-lx4",
+"renesas,imr-lx4";
+   reg = <0 0xfe87 0 0x2000>;
+   interrupts = ;
+   clocks = < CPG_MOD 822>;
+   power-domains = < R8A7796_PD_A3VC>;


resets = < 822>;

With the issues above fixed:

Reviewed-by: Geert Uytterhoeven 


   I'm afraid it's already merged by Simon...


Gr{oetje,eeting}s,

Geert


MBR, Sergei

RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven
> Sent: Wednesday, July 5, 2017 7:09 PM
> 
> Hi Simoda-san,
> 
> On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
>  wrote:
> > R-Car USB 2.0 controller can change the clock source from an oscillator
> > to an external clock via a register. So, this patch adds support
> > the clock source selector as a clock driver.
> >
> > Signed-off-by: Yoshihiro Shimoda 
> 
> Thanks for your patch!

Thank you very much for the comments!

> > --- /dev/null
> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
> 
> > +static void usb2_clock_sel_enable_extal_only(struct usb2_clock_sel_priv 
> > *priv)
> > +{
> > +   u16 val = readw(priv->base + USB20_CLKSET0);
> > +
> > +   pr_debug("%s: enter %d %d %x\n", __func__,
> > +priv->extal, priv->xtal, val);
> > +
> > +   if (priv->extal && !priv->xtal && val != CLKSET0_EXTAL_ONLY)
> > +   writew(CLKSET0_EXTAL_ONLY, priv->base + USB20_CLKSET0);
> > +}
> > +
> > +static void usb2_clock_sel_disable_extal_only(struct usb2_clock_sel_priv 
> > *priv)
> > +{
> > +   if (priv->extal && !priv->xtal)
> > +   writew(CLKSET0_PRIVATE, priv->base + USB20_CLKSET0);
> > +}
> > +
> > +static int usb2_clock_sel_enable(struct clk_hw *hw)
> > +{
> > +   usb2_clock_sel_enable_extal_only(to_priv(hw));
> > +
> > +   return 0;
> > +}
> > +
> > +static void usb2_clock_sel_disable(struct clk_hw *hw)
> > +{
> > +   usb2_clock_sel_disable_extal_only(to_priv(hw));
> > +}
> > +
> > +/*
> > + * This module seems a mux, but this driver assumes a gate because
> > + * ehci/ohci platform drivers don't support clk_set_parent() for now.
> > + * If this driver acts as a gate, ehci/ohci-platform drivers don't need
> > + * any modification.
> > + */
> > +static const struct clk_ops usb2_clock_sel_clock_ops = {
> > +   .enable = usb2_clock_sel_enable,
> > +   .disable = usb2_clock_sel_disable,
> > +};
> 
> So you do "smart muxing" in the enable routine above?

Yes.

> > +static int __init rcar_usb2_clock_sel_probe(struct platform_device *pdev)
> > +{
> > +   struct device *dev = >dev;
> > +   struct device_node *np = dev->of_node;
> > +   struct usb2_clock_sel_priv *priv;
> > +   struct resource *res;
> > +   struct clk *clk;
> > +   struct clk_init_data init;
> > +   int error;
> > +
> > +   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +   if (!priv)
> > +   return -ENOMEM;
> > +
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   priv->base = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(priv->base))
> > +   return PTR_ERR(priv->base);
> > +
> > +   priv->ehci_clk = devm_clk_get(dev, "ehci_ohci");
> > +   if (!IS_ERR(priv->ehci_clk)) {
> > +   error = clk_prepare_enable(priv->ehci_clk);
> > +   if (error)
> > +   return error;
> > +   }
> > +
> > +   clk = devm_clk_get(dev, "usb_extal");
> > +   if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
> > +   priv->extal = !!clk_get_rate(clk);
> > +   clk_disable_unprepare(clk);
> > +   }
> > +   clk = devm_clk_get(dev, "usb_xtal");
> > +   if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
> > +   priv->xtal = !!clk_get_rate(clk);
> > +   clk_disable_unprepare(clk);
> > +   }
> > +
> > +   if (!priv->extal && !priv->extal) {
> > +   dev_err(dev, "This driver needs usb_extal or usb_xtal\n");
> > +   return -ENOENT;
> 
> Perhaps enabled clocks should be disabled on error?

Oops! I should call clk_disable_unprepare(priv->ehci_clk) here.
I will fix it.

> > +   }
> > +
> > +   platform_set_drvdata(pdev, priv);
> > +   dev_set_drvdata(dev, priv);
> > +
> > +   init.name = "rcar_usb2_clock_sel";
> > +   init.ops = _clock_sel_clock_ops;
> > +   init.flags = CLK_IS_BASIC;
> > +   init.parent_names = NULL;
> > +   init.num_parents = 0;
> > +   priv->hw.init = 
> > +
> > +   clk = clk_register(NULL, >hw);
> > +   if (IS_ERR(clk))
> > +   return PTR_ERR(clk);
> 
> Likewise.

Same here. So, I will use "goto" for it.

> > +   error = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> > +   if (error)
> > +   return error;
> > +
> > +   return 0;
> 
> return of_clk_add_provider(np, of_clk_src_simple_get, clk);

Thanks. I will fix it.

> > +/* Since this driver needs other ccf drivers, this uses 
> > subsys_initcall_sync */
> > +subsys_initcall_sync(rcar_usb2_clock_sel_init);
> 
> I suppose this is a workaround for the lack of probe deferral support in the
> USB subsystem?

This is my fault. I added "power-domains" property into this device's node.
After I remove the power-domains like the cpg node, this driver can use 
subsys_initcall()
instead of 

[PATCH] clk: renesas: r8a7792: Add IMR-LX3/LSX3 clocks

[Geert Uytterhoeven]

Add the module clocks for the Image Renderer Light (SRAM) Extended 3
(IMR-LX3/LSX3) Distortion Correction Engines on R-Car V2H.

Signed-off-by: Geert Uytterhoeven 
---
To be queued in clk-renesas-for-v4.14.

 drivers/clk/renesas/r8a7792-cpg-mssr.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/renesas/r8a7792-cpg-mssr.c 
b/drivers/clk/renesas/r8a7792-cpg-mssr.c
index a832b9b6f7b0dde8..7f85bbf20bf782b7 100644
--- a/drivers/clk/renesas/r8a7792-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7792-cpg-mssr.c
@@ -118,6 +118,13 @@ static const struct mssr_mod_clk r8a7792_mod_clks[] 
__initconst = {
DEF_MOD("vin1",  810,   R8A7792_CLK_ZG),
DEF_MOD("vin0",  811,   R8A7792_CLK_ZG),
DEF_MOD("etheravb",  812,   R8A7792_CLK_HP),
+   DEF_MOD("imr-lx3",   821,   R8A7792_CLK_ZG),
+   DEF_MOD("imr-lsx3-1",822,   R8A7792_CLK_ZG),
+   DEF_MOD("imr-lsx3-0",823,   R8A7792_CLK_ZG),
+   DEF_MOD("imr-lsx3-5",825,   R8A7792_CLK_ZG),
+   DEF_MOD("imr-lsx3-4",826,   R8A7792_CLK_ZG),
+   DEF_MOD("imr-lsx3-3",827,   R8A7792_CLK_ZG),
+   DEF_MOD("imr-lsx3-2",828,   R8A7792_CLK_ZG),
DEF_MOD("gyro-adc",  901,   R8A7792_CLK_P),
DEF_MOD("gpio7", 904,   R8A7792_CLK_CP),
DEF_MOD("gpio6", 905,   R8A7792_CLK_CP),
-- 
2.7.4

Re: Architecture Timer on R-Car Gen2

[Mark Rutland]

On Tue, Jul 04, 2017 at 08:29:38PM +0200, Geert Uytterhoeven wrote:
> Hi Magnus, Mar[ck], Simon,

Hi,

> We're (finally) getting close to the point where Renesas R-Car Gen2
> platform code no longer relies on hardcoded addresses.
> 
> The remaining parts are related to the ARM Architecture Timer / Generic
> Timer / Generic Counter.  More specifically the code in
> rcar_gen2_timer_init() in arch/arm/mach-shmobile/setup-rcar-gen2.c,
> quoted below.  According to historical information, the issues being
> solved here are (1) non-running timers, and (2) wrong frequencies,
> leading to (m)sleep() not sleeping for the expected period.
> But it's far from clear to me what's really going on...
> 
> >   void __iomem *base;
> >   u32 freq;
> >
> >   /*
> >* Determine frequency.  Depending on SoC-type, it's either
> >*   - Crystal / 2 => (usually) 10 MHz (calculated from extal in DT)
> >*   - ZS clock / 8 => (always) 32.5 MHz (hardcoded)
> >*/
> >   freq = ...
> >
> >   /* Remap "armgcnt address map" space */
> >   base = ioremap(0xe608, PAGE_SIZE);
> 
> I believe this region corresponds to the Generic Counter, which is not
> the same, but related to the Architecture timer?
> It seems no DT bindings exist for the Generic Counter?
> 
> (for a moment I thought this might correspond to "arm,armv7-timer-mem", but
>  I no longer believe that's true)

The short of it is that this is *not* described by
"arm,armv7-timer-mem".

Given that this has the CNTCR and CNTFID0 registers, this is the
"counter module". This is not described by the "arm,armv7-timer-mem"
binding.

This is only expected to be of use to boot FW, and the counter module is
liable to be secure only, so this isn't something we want in that
binding, and I am not keen on adding a new binding for it.

For more details, take a look at ARM DDI 0406C.c, D5.2 "Memory-mapped
counter module" on page D5-2399.

> >   /*
> >* Update the timer if it is either not running, or is not at the
> >* right frequency. The timer is only configurable in secure mode
> >* so this avoids an abort if the loader started the timer and
> >* entered the kernel in non-secure mode.
> >*/
> >
> >   if ((ioread32(base + CNTCR) & 1) == 0 ||
> >   ioread32(base + CNTFID0) != freq) {
> 
> On all SoC/board combos I checked (r8a7790/lager, r8a7791/koelsch,
> r8a7791/porter, r8a7792/blanche, r8a7793/gose, r8a7794/alt), both
> conditions are true, i.e.
>   - The timer (counter?) is not running,
>   - The configured frequency is 13 MHz, which is wrong.
> 
> >   /* Update registers with correct frequency */
> >   iowrite32(freq, base + CNTFID0);

CNTFID<0..n> are ID registers, each describing a frequency that the
counter supports. These may be RO or RW. If the latter, some early
bringup software is expected to fill them in so that subsequent users
see sane values.

These are *not* control registers, and writing to them should have no
affect other than changing the value read back from them.

Note that CNTCR.FCREQ is expected to be programmed with the index of the
desired frequency (resetting to 0).

> This doesn't seem to be needed?
> With or without, both msleep() (kernel) and sleep() (userspace) sleep
> for the expected amount of time.
> 
> >   asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
> 
> This is an open-coded implementation of arch_timer_set_cntfrq(), which
> is not provided by arch/arm/include/asm/arch_timer.h (should it?).

We expect FW to configure CNTFRQ along with the rest of the boot-time
generic timer initialisation which can only be done from the secure
side.

I'm hesitant to expose a helper that makes it look like this is a
sensible thing to do in the kernel, so I'm not keen on a geneic
arch_timer_set_cntfrq() helper.

> Without this, the arch_timer driver complains:
> 
> arch_timer: frequency not available
> 
> leading to failures (e.g. division by zero) later.
> 
> However, adding the proper "clock-frequency" property to the
> "arm,armv7-timer" device node in DT fixes that.
> Probably adding this property is a better way to work around broken(?)
> firmware not setting CNTFRQ, than checking the SoC type and using
> hardcoded values in platform code?

The clock-frequency property is also a bodge, and is problemaic for
virtualisation, or in the presence of userspace timer access.

If you can't fix the FW, it would be much nicer to have a
platform-specific bootloader shim set this all up prior to entering the
kernel.

Thanks,
Mark.

RE: [RFT] mmc: tmio: fix CMD12 (STOP) handling

[Yoshihiro Shimoda]

Hi Wolfram-san,

> -Original Message-
> From: Wolfram Sang
> Sent: Tuesday, July 4, 2017 4:28 AM
> 
> I always anticipated this code to be not correct, but now I had a test
> case to prove it. According to all documentation I have, setting the
> TMIO_STOP_STP bit ever only worked during block transfers. This bit is
> like manually enforcing an autocmd12 during a so far seamless transfer.
> It does NOT work when the block transfer had errors. It also does NOT
> work with any other cmd except block commands. For all those, CMD12 has
> to be treated like any other command. So, basically, we could use this
> bit only for mrq->data->stop cmds. But for these, we happily use the
> autocmd12 feature using the TMIO_STOP_SEC bit. As a result, the above
> bit is not useful for us and we need to treat CMD12 as a regular cmd
> always. Just remove the special handling code. Note that the BSP
> recognized this issue as well yet had a more cautious solution to the
> problem [1]. Which is understandable but makes CMD12 handling even more
> complicated.
> 
> Checked with a Renesas Salvator-X/M3-W which needed to send CMD12 when
> retuning one of my SD cards.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=2838a2ff8ca776f6d18b7fbbe75f3df8dd6
> 4183a
> 
> Signed-off-by: Wolfram Sang 
> ---
> 
> So, this is a friendly call for further testing to make sure no regressions
> happen. I also put the authors of the BSP patch to CC to check if this patch
> also matches their use case. I hope this is fine for these people, please
> accept my apologies if not. I just really like to have your opinion on this
> patch.
> 
> Geert, Simon: any chance you can run this patch on your board farms?
> 
> In any case: as far as my understanding goes, if I am wrong with my
> assumptions, the worst case is that for older SoCs CMD12 handling might become
> a tad more inefficient because we now do in software what was expected to be
> done in hardware. However, I am quite sure that the HW feature was always
> over-estimated and CMD12 support is simply broken. For my test case and the 
> BSP
> patch above, it definately was.
> 
> Thanks for any assistance,
> 
>Wolfram
> 
> 
>  drivers/mmc/host/tmio_mmc_core.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 1851c883bfc82a..fbcd56c58bce24 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -355,12 +355,6 @@ static int tmio_mmc_start_command(struct tmio_mmc_host 
> *host,
>   int c = cmd->opcode;
>   u32 irq_mask = TMIO_MASK_CMD;
> 
> - /* CMD12 is handled by hardware */
> - if (cmd->opcode == MMC_STOP_TRANSMISSION && !cmd->arg) {
> - sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, TMIO_STOP_STP);
> - return 0;
> - }
> -

I and Dung-san are testing this patch.
However, this original code doesn't run by our test cases.
Do you know how this original code runs?
Anyway, we will try to make a test case somehow now.

Best regards,
Yoshihiro Shimoda

>   switch (mmc_resp_type(cmd)) {
>   case MMC_RSP_NONE: c |= RESP_NONE; break;
>   case MMC_RSP_R1:
> --
> 2.11.0

Re: [PATCH][RESEND] ARM: dts: iwg20d-q7: Add pinctl support for scif0

[Geert Uytterhoeven]

On Wed, Jul 5, 2017 at 12:57 PM, Biju Das  wrote:
> Adding pinctrl support for scif0 interface.
>
> Signed-off-by: Biju Das 

Reviewed-by: Geert Uytterhoeven 

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: [PATCH] ARM: dts: r8a7743: Add GPIO support

[Geert Uytterhoeven]

On Tue, Jul 4, 2017 at 6:18 PM, Biju Das  wrote:
> Describe GPIO blocks in the R8A7743 device tree.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Chris Paterson 

Reviewed-by: Geert Uytterhoeven 

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

[PATCH][RESEND] ARM: dts: iwg20d-q7: Add pinctl support for scif0

[Biju Das]

Adding pinctrl support for scif0 interface.

Signed-off-by: Biju Das 
---
v1-->v1-resend
This patch is been tested against linux-next tag next-20170704.
It is reposting,since the PFC depedencies now has been accepted.
https://www.spinics.net/lists/arm-kernel/msg592332.html

 arch/arm/boot/dts/r8a7743-iwg20d-q7.dts | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7743-iwg20d-q7.dts 
b/arch/arm/boot/dts/r8a7743-iwg20d-q7.dts
index 9b54783..497aec0 100644
--- a/arch/arm/boot/dts/r8a7743-iwg20d-q7.dts
+++ b/arch/arm/boot/dts/r8a7743-iwg20d-q7.dts
@@ -20,6 +20,16 @@
};
 };
 
+ {
+   scif0_pins: scif0 {
+   groups = "scif0_data_d";
+   function = "scif0";
+   };
+};
+
  {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
status = "okay";
 };
-- 
1.9.1

Re: [PATCH v2] arm64: dts: r8a7795: add IMR-LX4 support

[Simon Horman]

On Wed, Jul 05, 2017 at 12:18:01PM +0200, Geert Uytterhoeven wrote:
> Hi Sergei,
> 
> On Tue, Jun 27, 2017 at 7:30 PM, Sergei Shtylyov
>  wrote:
> > Describe the IMR-LX4 devices in the R8A7795 device tree.
> >
> > Based on the original (and large) patch by Konstantin Kozhevnikov
> > .
> >
> > Signed-off-by: Konstantin Kozhevnikov 
> > 
> > Signed-off-by: Sergei Shtylyov 
> > Reviewed-by: Geert Uytterhoeven 
> >
> > ---
> > This patch is against the 'renesas-devel-20170626-v4.12-rc7' tag of Simon
> > Horman's 'renesas.git' repo.
> >
> > The IMR-LX4 bindings were documented in the IMR driver patch and ACK'ed by 
> > Rob
> > Herring, so I don't expect them to change...
> >
> > Changes in version 2:
> > - added SoC specific "compatible" prop values;
> > - refreshed the patch;
> > - added Geert's tag.
> 
> In the mean time, r8a7795.dtsi gained reset controller support, so
> you should add "resets" properties were appropriate.

Thanks, I have squashed-in the following.

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index f76cb360e110..31d1ba586ec2 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1542,6 +1542,7 @@
interrupts = ;
clocks = < CPG_MOD 823>;
power-domains = < R8A7795_PD_A3VC>;
+   resets = < 823>;
};
 
imr-lx4@fe87 {
@@ -1551,6 +1552,7 @@
interrupts = ;
clocks = < CPG_MOD 822>;
power-domains = < R8A7795_PD_A3VC>;
+   resets = < 822>;
};
 
imr-lx4@fe88 {
@@ -1560,6 +1562,7 @@
interrupts = ;
clocks = < CPG_MOD 821>;
power-domains = < R8A7795_PD_A3VC>;
+   resets = < 821>;
};
 
imr-lx4@fe89 {
@@ -1569,6 +1572,7 @@
interrupts = ;
clocks = < CPG_MOD 820>;
power-domains = < R8A7795_PD_A3VC>;
+   resets = < 820>;
};
 
vspbc: vsp@fe92 {

Re: [PATCH v2] arm64: dts: r8a7796: add IMR-LX4 support

[Simon Horman]

On Wed, Jul 05, 2017 at 11:54:20AM +0200, Geert Uytterhoeven wrote:
> Hi Sergei,
> 
> On Tue, Jun 27, 2017 at 7:33 PM, Sergei Shtylyov
>  wrote:
> > Describe the IMR-LX4 devices in the R8A7796 device tree.
> >
> > Signed-off-by: Sergei Shtylyov 
> 
> > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > +++ renesas/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > @@ -1454,5 +1454,23 @@
> > };
> > };
> > };
> > +
> > +   imr-lx4@fe86 {
> > +   compatible = "renesas,r8a7796-imr-lx4",
> > +"renesas,imr-lx4";
> > +   reg = <0 0xfe86 0 0x2000>;
> > +   interrupts = ;
> > +   clocks = < CPG_MOD 823>;
> > +   power-domains = < R8A7796_PD_A3VC>;
> 
> resets = < 823>;
> 
> > +   };
> > +
> > +   imr-lx4@fe87 {
> > +   compatible = "renesas,r8a7796-imr-lx4",
> > +"renesas,imr-lx4";
> > +   reg = <0 0xfe87 0 0x2000>;
> > +   interrupts = ;
> > +   clocks = < CPG_MOD 822>;
> > +   power-domains = < R8A7796_PD_A3VC>;
> 
> resets = < 822>;
> 
> With the issues above fixed:
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, fixed.

Re: [PATCH] ARM: dts: r8a7743: Add GPIO support

[Simon Horman]

On Tue, Jul 04, 2017 at 05:18:15PM +0100, Biju Das wrote:
> Describe GPIO blocks in the R8A7743 device tree.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Chris Paterson 

Thanks, I have applied this for v4.14.

Re: [PATCH v2 0/3] Add R8A7743/SK-RZG1M PFC support

[Simon Horman]

On Mon, Apr 24, 2017 at 08:12:18AM +0200, Simon Horman wrote:
> On Thu, Apr 20, 2017 at 09:51:32PM +0300, Sergei Shtylyov wrote:
> > Hello.
> > 
> >Here's the set of 3 patches against Simon Horman's 'renesas.git' repo,
> > 'renesas-devel-20170420-v4.11-rc7' tag.  We're adding the R8A7743 PFC node
> > and then describing the pins for SCIF0 and Ether devices declared earlier.
> > These patches depend on the R8A7743 PFC suport in order to work properly.
> 
> Thanks. I have marked these as deferred pending acceptance of the PFC
> driver changes. Please repost or otherwise let me know when dependency
> is satisfied.

It appears that the driver changes have been acceptedi[1] so
I have queued these up for v4.14.

[1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg15744.html

Re: [PATCH v2] ARM: debug-ll: Add support for r8a7743

[Simon Horman]

On Thu, Jun 08, 2017 at 11:08:14AM +0100, Chris Paterson wrote:
> Enable low-level debugging support for RZ/G1M (r8a7743). RZ/G1M uses
> SCIF0 for the debug console, like most of the R-Car Gen2 SoCs.
> 
> Signed-off-by: Chris Paterson 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.14.

Re: [PATCHv6] arm64: dts: renesas: salvator-x: Add ADV7482 support

[Kieran Bingham]

On 05/07/17 10:43, Geert Uytterhoeven wrote:
> On Tue, Jun 27, 2017 at 5:15 PM, Kieran Bingham  wrote:
>> From: Kieran Bingham 
>>
>> The Salvator boards use an ADV7482 receiver for HDMI and CVBS inputs.
>>
>> Provide ADV7482 node on the i2c4 bus, along with connectors for the
>> hdmi and cvbs inputs, and link to the csi20 and csi40 nodes as outputs.
>>
>> Signed-off-by: Kieran Bingham 
>> Reviewed-by: Laurent Pinchart 
>>
>> ---
>>
>> Submitting this directly to Niklas, as these changes depend on his VIN
>> DT patches, and has agreed to 'take' this patch on to that series.
> 
> Yeah, haven't seen any patches yet adding "csi20" and "csi40" nodes ;-)

I defer this comment to Niklas :-)

--
Kieran


> 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: [PATCH v2] arm64: dts: r8a7795: add IMR-LX4 support

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Jun 27, 2017 at 7:30 PM, Sergei Shtylyov
 wrote:
> Describe the IMR-LX4 devices in the R8A7795 device tree.
>
> Based on the original (and large) patch by Konstantin Kozhevnikov
> .
>
> Signed-off-by: Konstantin Kozhevnikov 
> 
> Signed-off-by: Sergei Shtylyov 
> Reviewed-by: Geert Uytterhoeven 
>
> ---
> This patch is against the 'renesas-devel-20170626-v4.12-rc7' tag of Simon
> Horman's 'renesas.git' repo.
>
> The IMR-LX4 bindings were documented in the IMR driver patch and ACK'ed by Rob
> Herring, so I don't expect them to change...
>
> Changes in version 2:
> - added SoC specific "compatible" prop values;
> - refreshed the patch;
> - added Geert's tag.

In the mean time, r8a7795.dtsi gained reset controller support, so
you should add "resets" properties were appropriate.

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: [PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

[Takashi Iwai]

On Wed, 05 Jul 2017 11:29:43 +0200,
Arnaud Pouliquen wrote:
> 
> 
> 
> On 07/05/2017 10:40 AM, Takashi Iwai wrote:
> > On Wed, 05 Jul 2017 10:15:35 +0200,
> > Kuninori Morimoto wrote:
> >>
> >>
> >> From: Kuninori Morimoto 
> >>
> >> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
> >> But, hdmi-codec driver might be used from many HDMIs. In such case,
> >> they will use same "ELD" name and kernel will indicate below error.
> >>
> >>xxx: control x:x:x:ELD:x is already present
> >>
> >> hdmi_controls will be registered in soc_probe_component(), and we can
> >> replace it by component driver probe function.
> >>
> >> This patch registers current hdmi_controls in new hdmi_probe().
> >> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
> >> (We can keep compatibility).
> >> If hdmi-codec is used from many devices, it will use "ELD.x" as control 
> >> name.
> > 
> > The de facto standard way as HD-audio does is to pass the device
> > number of each ELD control as same as the corresponding PCM stream
> > number.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> In theory, hdmi_codec_pcm_new already does the job.
> snd_pcm_add_chmap_ctls instantiates the controls using PCM device ID. So
> you should observe (with amixer) 2 controls with same name but not same
> device ID.

Indeed, we can add each ELD control there like a (totally untested)
patch below.


thanks,

Takashi

---
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc88f0a..13d9cf5b1831 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -399,16 +399,13 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
*kcontrol,
return 0;
 }
 
-
-static const struct snd_kcontrol_new hdmi_controls[] = {
-   {
-   .access = SNDRV_CTL_ELEM_ACCESS_READ |
- SNDRV_CTL_ELEM_ACCESS_VOLATILE,
-   .iface = SNDRV_CTL_ELEM_IFACE_PCM,
-   .name = "ELD",
-   .info = hdmi_eld_ctl_info,
-   .get = hdmi_eld_ctl_get,
-   },
+static const struct snd_kcontrol_new hdmi_eld_ctl = {
+   .access = SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+   .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+   .name = "ELD",
+   .info = hdmi_eld_ctl_info,
+   .get = hdmi_eld_ctl_get,
 };
 
 static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
@@ -668,6 +665,7 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
 {
struct snd_soc_dai_driver *drv = dai->driver;
struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+   struct snd_kcontrol *kctl;
int ret;
 
dev_dbg(dai->dev, "%s()\n", __func__);
@@ -686,7 +684,12 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
hcp->chmap_info->chmap = hdmi_codec_stereo_chmaps;
hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
 
-   return 0;
+   /* add ELD ctl with the device number corresponding to the PCM stream */
+   kctl = snd_ctl_new1(_eld_ctl, dai->component);
+   if (!kctl)
+   return -ENOMEM;
+   kctl->id.device = rtd->pcm->device;
+   return snd_ctl_add(rtd->card->snd_card, kctl);
 }
 
 static struct snd_soc_dai_driver hdmi_i2s_dai = {
@@ -732,8 +735,6 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
*component,
 
 static struct snd_soc_codec_driver hdmi_codec = {
.component_driver = {
-   .controls   = hdmi_controls,
-   .num_controls   = ARRAY_SIZE(hdmi_controls),
.dapm_widgets   = hdmi_widgets,
.num_dapm_widgets   = ARRAY_SIZE(hdmi_widgets),
.dapm_routes= hdmi_routes,

Re: [PATCH 1/2] ARM: shmobile: Add CA7 arch_timer initialization for secondary CPUs

[Geert Uytterhoeven]

Hi Marc,

On Wed, Jul 5, 2017 at 12:07 PM, Marc Zyngier  wrote:
> On 04/07/17 19:20, Geert Uytterhoeven wrote:
>> On Tue, Jul 4, 2017 at 7:32 PM, Marc Zyngier  wrote:
>>> On 04/07/17 18:02, Geert Uytterhoeven wrote:
 On Cortex-A7, the arch timer CNTVOFF register is uninitialized.
 Hence when enabling SMP on r8a7794, the kernel log is spammed with:

 WARNING: Underflow in clocksource 'arch_sys_counter' observed, time 
 update ignored.
Please report this, consider using a different clocksource, if 
 possible.
Your kernel is probably still fine.

 To fix this, wrap the standard secondary_startup routine inside a
 routine which initializes CNTVOFF when running on a Cortex-A7.
 As the only possibilities are Cortex-A7 or Cortex-A15, checking the low
 nibble of the Primary Part Number is sufficient.

 The initialization is identical to what is already done for the boot CPU
 since commit 9ce3fa6816c2fb59 ("ARM: shmobile: rcar-gen2: Add CA7
 arch_timer initialization for r8a7794").
>>>
>>> Humfff... Pretty horrible. Comments below.
>>
>> I know.  I suppose this should be done by the firmware/boot loader?
>
> That's what is normally expected.
>
>> But we have to live with firmware/boot loaders in the wild...
>
> Yeah, I can tell. It is a shame that firmware people didn't realize that
> just because the old firmware seems to work on a new revision of the
> architecture, it doesn't mean it does everything right for that
> particular revision... Oh well.
>
 --- /dev/null
 +++ b/arch/arm/mach-shmobile/headsmp-apmu.S
 @@ -0,0 +1,38 @@
 +/*
 + * SMP support for APMU based systems
 + *
 + * Copyright (C) 2014  Renesas Electronics Corporation
 + *
 + * 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.
 + */
 +
 +#include 
 +
 +ENTRY(shmobile_boot_apmu)
 + mrc p15, 0, r0, c0, c0, 0   /* Get Main ID */
 + ubfxr1, r0, #4, #4  /* r1=Lo 4bit of Primary 
 Part */
 + cmp r1, #0x7/* 0x7 = CA7, 0xf = CA15 */
 + bne 1f
>>>
>>> Why don't you deal with the A15 parts as well? And TBH, you'd be better
>>> off checking ID_PFR1 for both Generic Timers and Virtualization Extensions.
>>
>> Do we have to deal with the A15 parts here?
>> We're not having issues with the A15 parts, so that's why I left it out.
>
> Well, A15 has the exact same features as A7 when it comes to the timer,
> and CNTVOFF definitely resets as UNKNOWN.

IC.

>> I'm trying to understand what's different between A15 and A7, and why A7
>> needs this code, while A15 apparently doesn't.
>> I'm not seeing any obvious differences in the U-Boot sources handling
>> e.g. r8a7791/koelsch (dual A15) vs. r8a7794/alt (dual A7).
>
> The UNKNOWN above might well be 0 on A15, but I wouldn't bet on it. If
> nothing sets CNTVOFF on these systems, consider yourself lucky!

OK.  That means I can just drop the Primary Part check, as this code path
is exercised only on systems with Cortex-A15 and/or A7 CPU cores.

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: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Geert Uytterhoeven]

Hi Simoda-san,

On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
 wrote:
> R-Car USB 2.0 controller can change the clock source from an oscillator
> to an external clock via a register. So, this patch adds support
> the clock source selector as a clock driver.
>
> Signed-off-by: Yoshihiro Shimoda 

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c

> +static void usb2_clock_sel_enable_extal_only(struct usb2_clock_sel_priv 
> *priv)
> +{
> +   u16 val = readw(priv->base + USB20_CLKSET0);
> +
> +   pr_debug("%s: enter %d %d %x\n", __func__,
> +priv->extal, priv->xtal, val);
> +
> +   if (priv->extal && !priv->xtal && val != CLKSET0_EXTAL_ONLY)
> +   writew(CLKSET0_EXTAL_ONLY, priv->base + USB20_CLKSET0);
> +}
> +
> +static void usb2_clock_sel_disable_extal_only(struct usb2_clock_sel_priv 
> *priv)
> +{
> +   if (priv->extal && !priv->xtal)
> +   writew(CLKSET0_PRIVATE, priv->base + USB20_CLKSET0);
> +}
> +
> +static int usb2_clock_sel_enable(struct clk_hw *hw)
> +{
> +   usb2_clock_sel_enable_extal_only(to_priv(hw));
> +
> +   return 0;
> +}
> +
> +static void usb2_clock_sel_disable(struct clk_hw *hw)
> +{
> +   usb2_clock_sel_disable_extal_only(to_priv(hw));
> +}
> +
> +/*
> + * This module seems a mux, but this driver assumes a gate because
> + * ehci/ohci platform drivers don't support clk_set_parent() for now.
> + * If this driver acts as a gate, ehci/ohci-platform drivers don't need
> + * any modification.
> + */
> +static const struct clk_ops usb2_clock_sel_clock_ops = {
> +   .enable = usb2_clock_sel_enable,
> +   .disable = usb2_clock_sel_disable,
> +};

So you do "smart muxing" in the enable routine above?

> +static int __init rcar_usb2_clock_sel_probe(struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +   struct device_node *np = dev->of_node;
> +   struct usb2_clock_sel_priv *priv;
> +   struct resource *res;
> +   struct clk *clk;
> +   struct clk_init_data init;
> +   int error;
> +
> +   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +   if (!priv)
> +   return -ENOMEM;
> +
> +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +   priv->base = devm_ioremap_resource(dev, res);
> +   if (IS_ERR(priv->base))
> +   return PTR_ERR(priv->base);
> +
> +   priv->ehci_clk = devm_clk_get(dev, "ehci_ohci");
> +   if (!IS_ERR(priv->ehci_clk)) {
> +   error = clk_prepare_enable(priv->ehci_clk);
> +   if (error)
> +   return error;
> +   }
> +
> +   clk = devm_clk_get(dev, "usb_extal");
> +   if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
> +   priv->extal = !!clk_get_rate(clk);
> +   clk_disable_unprepare(clk);
> +   }
> +   clk = devm_clk_get(dev, "usb_xtal");
> +   if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
> +   priv->xtal = !!clk_get_rate(clk);
> +   clk_disable_unprepare(clk);
> +   }
> +
> +   if (!priv->extal && !priv->extal) {
> +   dev_err(dev, "This driver needs usb_extal or usb_xtal\n");
> +   return -ENOENT;

Perhaps enabled clocks should be disabled on error?

> +   }
> +
> +   platform_set_drvdata(pdev, priv);
> +   dev_set_drvdata(dev, priv);
> +
> +   init.name = "rcar_usb2_clock_sel";
> +   init.ops = _clock_sel_clock_ops;
> +   init.flags = CLK_IS_BASIC;
> +   init.parent_names = NULL;
> +   init.num_parents = 0;
> +   priv->hw.init = 
> +
> +   clk = clk_register(NULL, >hw);
> +   if (IS_ERR(clk))
> +   return PTR_ERR(clk);

Likewise.

> +   error = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +   if (error)
> +   return error;
> +
> +   return 0;

return of_clk_add_provider(np, of_clk_src_simple_get, clk);

> +/* Since this driver needs other ccf drivers, this uses subsys_initcall_sync 
> */
> +subsys_initcall_sync(rcar_usb2_clock_sel_init);

I suppose this is a workaround for the lack of probe deferral support in the
USB subsystem?

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: [PATCH 1/2] ARM: shmobile: Add CA7 arch_timer initialization for secondary CPUs

[Marc Zyngier]

On 04/07/17 19:20, Geert Uytterhoeven wrote:
> Hi Marc,
> 
> On Tue, Jul 4, 2017 at 7:32 PM, Marc Zyngier  wrote:
>> On 04/07/17 18:02, Geert Uytterhoeven wrote:
>>> On Cortex-A7, the arch timer CNTVOFF register is uninitialized.
>>> Hence when enabling SMP on r8a7794, the kernel log is spammed with:
>>>
>>> WARNING: Underflow in clocksource 'arch_sys_counter' observed, time 
>>> update ignored.
>>>Please report this, consider using a different clocksource, if 
>>> possible.
>>>Your kernel is probably still fine.
>>>
>>> To fix this, wrap the standard secondary_startup routine inside a
>>> routine which initializes CNTVOFF when running on a Cortex-A7.
>>> As the only possibilities are Cortex-A7 or Cortex-A15, checking the low
>>> nibble of the Primary Part Number is sufficient.
>>>
>>> The initialization is identical to what is already done for the boot CPU
>>> since commit 9ce3fa6816c2fb59 ("ARM: shmobile: rcar-gen2: Add CA7
>>> arch_timer initialization for r8a7794").
>>
>> Humfff... Pretty horrible. Comments below.
> 
> I know.  I suppose this should be done by the firmware/boot loader?

That's what is normally expected.

> But we have to live with firmware/boot loaders in the wild...

Yeah, I can tell. It is a shame that firmware people didn't realize that
just because the old firmware seems to work on a new revision of the
architecture, it doesn't mean it does everything right for that
particular revision... Oh well.

>>> --- /dev/null
>>> +++ b/arch/arm/mach-shmobile/headsmp-apmu.S
>>> @@ -0,0 +1,38 @@
>>> +/*
>>> + * SMP support for APMU based systems
>>> + *
>>> + * Copyright (C) 2014  Renesas Electronics Corporation
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include 
>>> +
>>> +ENTRY(shmobile_boot_apmu)
>>> + mrc p15, 0, r0, c0, c0, 0   /* Get Main ID */
>>> + ubfxr1, r0, #4, #4  /* r1=Lo 4bit of Primary Part 
>>> */
>>> + cmp r1, #0x7/* 0x7 = CA7, 0xf = CA15 */
>>> + bne 1f
>>
>> Why don't you deal with the A15 parts as well? And TBH, you'd be better
>> off checking ID_PFR1 for both Generic Timers and Virtualization Extensions.
> 
> Do we have to deal with the A15 parts here?
> We're not having issues with the A15 parts, so that's why I left it out.

Well, A15 has the exact same features as A7 when it comes to the timer,
and CNTVOFF definitely resets as UNKNOWN.

> 
> I'm trying to understand what's different between A15 and A7, and why A7
> needs this code, while A15 apparently doesn't.
> I'm not seeing any obvious differences in the U-Boot sources handling
> e.g. r8a7791/koelsch (dual A15) vs. r8a7794/alt (dual A7).

The UNKNOWN above might well be 0 on A15, but I wouldn't bet on it. If
nothing sets CNTVOFF on these systems, consider yourself lucky!

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2.1 1/2] dt-bindings: display: rcar-du: Add a VSP channel index to the vsps DT property

[Geert Uytterhoeven]

On Sun, Jul 2, 2017 at 3:40 PM, Laurent Pinchart
 wrote:
> On some R-Car SoCs a single VSP can serve multiple DU channels through
> multiple LIF instances in the VSP. The current DT bindings don't support
> specifying that kind of SoC integration scheme. Extend them with a VSP
> channel index.
>
> Backward compatibility can be ensured in drivers by checking the length
> of the vsps property and setting the channel to 0 when the property
> doesn't contain channel indices.
>
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Geert Uytterhoeven 

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: [PATCH v2] arm64: dts: r8a7796: add IMR-LX4 support

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Jun 27, 2017 at 7:33 PM, Sergei Shtylyov
 wrote:
> Describe the IMR-LX4 devices in the R8A7796 device tree.
>
> Signed-off-by: Sergei Shtylyov 

> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -1454,5 +1454,23 @@
> };
> };
> };
> +
> +   imr-lx4@fe86 {
> +   compatible = "renesas,r8a7796-imr-lx4",
> +"renesas,imr-lx4";
> +   reg = <0 0xfe86 0 0x2000>;
> +   interrupts = ;
> +   clocks = < CPG_MOD 823>;
> +   power-domains = < R8A7796_PD_A3VC>;

resets = < 823>;

> +   };
> +
> +   imr-lx4@fe87 {
> +   compatible = "renesas,r8a7796-imr-lx4",
> +"renesas,imr-lx4";
> +   reg = <0 0xfe87 0 0x2000>;
> +   interrupts = ;
> +   clocks = < CPG_MOD 822>;
> +   power-domains = < R8A7796_PD_A3VC>;

resets = < 822>;

With the issues above fixed:

Reviewed-by: Geert Uytterhoeven 

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: [PATCHv6] arm64: dts: renesas: salvator-x: Add ADV7482 support

[Geert Uytterhoeven]

On Tue, Jun 27, 2017 at 5:15 PM, Kieran Bingham  wrote:
> From: Kieran Bingham 
>
> The Salvator boards use an ADV7482 receiver for HDMI and CVBS inputs.
>
> Provide ADV7482 node on the i2c4 bus, along with connectors for the
> hdmi and cvbs inputs, and link to the csi20 and csi40 nodes as outputs.
>
> Signed-off-by: Kieran Bingham 
> Reviewed-by: Laurent Pinchart 
>
> ---
>
> Submitting this directly to Niklas, as these changes depend on his VIN
> DT patches, and has agreed to 'take' this patch on to that series.

Yeah, haven't seen any patches yet adding "csi20" and "csi40" nodes ;-)

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: [PATCH] arm64: dts: r8a7795: Add DRIF support

[Geert Uytterhoeven]

On Fri, Jun 23, 2017 at 11:13 AM, Ramesh Shanmugasundaram
 wrote:
> Adds the DRIF controller nodes for r8a7795.
>
> Signed-off-by: Ramesh Shanmugasundaram 
> 

Reviewed-by: Geert Uytterhoeven 

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: [PATCH] arm64: dts: r8a7796: Add DRIF support

[Geert Uytterhoeven]

On Tue, Jun 27, 2017 at 2:54 PM, Ramesh Shanmugasundaram
 wrote:
> Adds the DRIF controller nodes for r8a7796.
>
> Signed-off-by: Ramesh Shanmugasundaram 
> 

Reviewed-by: Geert Uytterhoeven 

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: [PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

[Arnaud Pouliquen]

On 07/05/2017 10:40 AM, Takashi Iwai wrote:
> On Wed, 05 Jul 2017 10:15:35 +0200,
> Kuninori Morimoto wrote:
>>
>>
>> From: Kuninori Morimoto 
>>
>> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
>> But, hdmi-codec driver might be used from many HDMIs. In such case,
>> they will use same "ELD" name and kernel will indicate below error.
>>
>>  xxx: control x:x:x:ELD:x is already present
>>
>> hdmi_controls will be registered in soc_probe_component(), and we can
>> replace it by component driver probe function.
>>
>> This patch registers current hdmi_controls in new hdmi_probe().
>> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
>> (We can keep compatibility).
>> If hdmi-codec is used from many devices, it will use "ELD.x" as control name.
> 
> The de facto standard way as HD-audio does is to pass the device
> number of each ELD control as same as the corresponding PCM stream
> number.
> 
> 
> thanks,
> 
> Takashi
> 
In theory, hdmi_codec_pcm_new already does the job.
snd_pcm_add_chmap_ctls instantiates the controls using PCM device ID. So
you should observe (with amixer) 2 controls with same name but not same
device ID.

Regards
Arnaud

>> Signed-off-by: Kuninori Morimoto 
>> ---
>>  sound/soc/codecs/hdmi-codec.c | 60 
>> +--
>>  1 file changed, 46 insertions(+), 14 deletions(-)
>>
>> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
>> index 22ed0dc..5835a90 100644
>> --- a/sound/soc/codecs/hdmi-codec.c
>> +++ b/sound/soc/codecs/hdmi-codec.c
>> @@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc {
>>.mask = FL | FR | LFE | FC | RL | RR | FLC | FRC },
>>  };
>>  
>> +#define ELD_NAME_SIZE   8
>>  struct hdmi_codec_priv {
>>  struct hdmi_codec_pdata hcd;
>>  struct snd_soc_dai_driver *daidrv;
>> @@ -284,7 +285,10 @@ struct hdmi_codec_priv {
>>  struct snd_pcm_substream *current_stream;
>>  uint8_t eld[MAX_ELD_BYTES];
>>  struct snd_pcm_chmap *chmap_info;
>> +struct snd_kcontrol_new control;
>>  unsigned int chmap_idx;
>> +int id;
>> +char eld_name[ELD_NAME_SIZE];
>>  };
>>  
>>  static const struct snd_soc_dapm_widget hdmi_widgets[] = {
>> @@ -399,18 +403,6 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
>> *kcontrol,
>>  return 0;
>>  }
>>  
>> -
>> -static const struct snd_kcontrol_new hdmi_controls[] = {
>> -{
>> -.access = SNDRV_CTL_ELEM_ACCESS_READ |
>> -  SNDRV_CTL_ELEM_ACCESS_VOLATILE,
>> -.iface = SNDRV_CTL_ELEM_IFACE_PCM,
>> -.name = "ELD",
>> -.info = hdmi_eld_ctl_info,
>> -.get = hdmi_eld_ctl_get,
>> -},
>> -};
>> -
>>  static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
>>   struct snd_soc_dai *dai)
>>  {
>> @@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct 
>> snd_soc_pcm_runtime *rtd,
>>  .pcm_new = hdmi_codec_pcm_new,
>>  };
>>  
>> +static int hdmi_last_id = 0;
>> +static int hdmi_probe(struct snd_soc_component *component)
>> +{
>> +struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
>> +static struct snd_soc_component *component_1st = NULL;
>> +char *name = "ELD";
>> +
>> +/*
>> + * It will use "ELD"   if hdmi_codec was used from only 1 device.
>> + * It will use "ELD.x" if hdmi_codec was used from many devices.
>> + * Then, first "ELD" will be replaced to "ELD.0"
>> + */
>> +snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id);
>> +
>> +if (hcp->id == 0)
>> +component_1st = component;
>> +
>> +if (hdmi_last_id > 1) {
>> +name = hcp->eld_name;
>> +
>> +if (component_1st) {
>> +struct hdmi_codec_priv *hcp_1st;
>> +
>> +/* replaced 1st "ELD" to "ELD.0" */
>> +hcp_1st = snd_soc_component_get_drvdata(component_1st);
>> +hcp_1st->control.name = hcp_1st->eld_name;
>> +}
>> +}
>> +
>> +hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ |
>> +  SNDRV_CTL_ELEM_ACCESS_VOLATILE;
>> +hcp->control.iface  = SNDRV_CTL_ELEM_IFACE_PCM;
>> +hcp->control.name   = name;
>> +hcp->control.info   = hdmi_eld_ctl_info;
>> +hcp->control.get= hdmi_eld_ctl_get;
>> +
>> +return snd_soc_add_component_controls(component, >control, 1);
>> +}
>> +
>>  static int hdmi_of_xlate_dai_id(struct snd_soc_component *component,
>>   struct device_node *endpoint)
>>  {
>> @@ -732,8 +763,7 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
>> *component,
>>  
>>  static struct snd_soc_codec_driver hdmi_codec = {
>>  .component_driver = {
>> -.controls   = hdmi_controls,
>> -.num_controls  

Re: [PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

[Takashi Iwai]

On Wed, 05 Jul 2017 10:15:35 +0200,
Kuninori Morimoto wrote:
> 
> 
> From: Kuninori Morimoto 
> 
> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
> But, hdmi-codec driver might be used from many HDMIs. In such case,
> they will use same "ELD" name and kernel will indicate below error.
> 
>   xxx: control x:x:x:ELD:x is already present
> 
> hdmi_controls will be registered in soc_probe_component(), and we can
> replace it by component driver probe function.
> 
> This patch registers current hdmi_controls in new hdmi_probe().
> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
> (We can keep compatibility).
> If hdmi-codec is used from many devices, it will use "ELD.x" as control name.

The de facto standard way as HD-audio does is to pass the device
number of each ELD control as same as the corresponding PCM stream
number.


thanks,

Takashi

> Signed-off-by: Kuninori Morimoto 
> ---
>  sound/soc/codecs/hdmi-codec.c | 60 
> +--
>  1 file changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
> index 22ed0dc..5835a90 100644
> --- a/sound/soc/codecs/hdmi-codec.c
> +++ b/sound/soc/codecs/hdmi-codec.c
> @@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc {
> .mask = FL | FR | LFE | FC | RL | RR | FLC | FRC },
>  };
>  
> +#define ELD_NAME_SIZE8
>  struct hdmi_codec_priv {
>   struct hdmi_codec_pdata hcd;
>   struct snd_soc_dai_driver *daidrv;
> @@ -284,7 +285,10 @@ struct hdmi_codec_priv {
>   struct snd_pcm_substream *current_stream;
>   uint8_t eld[MAX_ELD_BYTES];
>   struct snd_pcm_chmap *chmap_info;
> + struct snd_kcontrol_new control;
>   unsigned int chmap_idx;
> + int id;
> + char eld_name[ELD_NAME_SIZE];
>  };
>  
>  static const struct snd_soc_dapm_widget hdmi_widgets[] = {
> @@ -399,18 +403,6 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
> *kcontrol,
>   return 0;
>  }
>  
> -
> -static const struct snd_kcontrol_new hdmi_controls[] = {
> - {
> - .access = SNDRV_CTL_ELEM_ACCESS_READ |
> -   SNDRV_CTL_ELEM_ACCESS_VOLATILE,
> - .iface = SNDRV_CTL_ELEM_IFACE_PCM,
> - .name = "ELD",
> - .info = hdmi_eld_ctl_info,
> - .get = hdmi_eld_ctl_get,
> - },
> -};
> -
>  static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
>struct snd_soc_dai *dai)
>  {
> @@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
> *rtd,
>   .pcm_new = hdmi_codec_pcm_new,
>  };
>  
> +static int hdmi_last_id = 0;
> +static int hdmi_probe(struct snd_soc_component *component)
> +{
> + struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
> + static struct snd_soc_component *component_1st = NULL;
> + char *name = "ELD";
> +
> + /*
> +  * It will use "ELD"   if hdmi_codec was used from only 1 device.
> +  * It will use "ELD.x" if hdmi_codec was used from many devices.
> +  * Then, first "ELD" will be replaced to "ELD.0"
> +  */
> + snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id);
> +
> + if (hcp->id == 0)
> + component_1st = component;
> +
> + if (hdmi_last_id > 1) {
> + name = hcp->eld_name;
> +
> + if (component_1st) {
> + struct hdmi_codec_priv *hcp_1st;
> +
> + /* replaced 1st "ELD" to "ELD.0" */
> + hcp_1st = snd_soc_component_get_drvdata(component_1st);
> + hcp_1st->control.name = hcp_1st->eld_name;
> + }
> + }
> +
> + hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ |
> +   SNDRV_CTL_ELEM_ACCESS_VOLATILE;
> + hcp->control.iface  = SNDRV_CTL_ELEM_IFACE_PCM;
> + hcp->control.name   = name;
> + hcp->control.info   = hdmi_eld_ctl_info;
> + hcp->control.get= hdmi_eld_ctl_get;
> +
> + return snd_soc_add_component_controls(component, >control, 1);
> +}
> +
>  static int hdmi_of_xlate_dai_id(struct snd_soc_component *component,
>struct device_node *endpoint)
>  {
> @@ -732,8 +763,7 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
> *component,
>  
>  static struct snd_soc_codec_driver hdmi_codec = {
>   .component_driver = {
> - .controls   = hdmi_controls,
> - .num_controls   = ARRAY_SIZE(hdmi_controls),
> + .probe  = hdmi_probe,
>   .dapm_widgets   = hdmi_widgets,
>   .num_dapm_widgets   = ARRAY_SIZE(hdmi_widgets),
>   .dapm_routes= hdmi_routes,
> @@ -768,6 +798,7 @@ static int hdmi_codec_probe(struct platform_device *pdev)
>   if 

[PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

[Kuninori Morimoto]

From: Kuninori Morimoto 

Current hdmi-codec driver is using hdmi_controls for "ELD" control.
But, hdmi-codec driver might be used from many HDMIs. In such case,
they will use same "ELD" name and kernel will indicate below error.

xxx: control x:x:x:ELD:x is already present

hdmi_controls will be registered in soc_probe_component(), and we can
replace it by component driver probe function.

This patch registers current hdmi_controls in new hdmi_probe().
If hdmi-codec is used from only 1 device, it will use "ELD" as control name
(We can keep compatibility).
If hdmi-codec is used from many devices, it will use "ELD.x" as control name.

Signed-off-by: Kuninori Morimoto 
---
 sound/soc/codecs/hdmi-codec.c | 60 +--
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc..5835a90 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc {
  .mask = FL | FR | LFE | FC | RL | RR | FLC | FRC },
 };
 
+#define ELD_NAME_SIZE  8
 struct hdmi_codec_priv {
struct hdmi_codec_pdata hcd;
struct snd_soc_dai_driver *daidrv;
@@ -284,7 +285,10 @@ struct hdmi_codec_priv {
struct snd_pcm_substream *current_stream;
uint8_t eld[MAX_ELD_BYTES];
struct snd_pcm_chmap *chmap_info;
+   struct snd_kcontrol_new control;
unsigned int chmap_idx;
+   int id;
+   char eld_name[ELD_NAME_SIZE];
 };
 
 static const struct snd_soc_dapm_widget hdmi_widgets[] = {
@@ -399,18 +403,6 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol 
*kcontrol,
return 0;
 }
 
-
-static const struct snd_kcontrol_new hdmi_controls[] = {
-   {
-   .access = SNDRV_CTL_ELEM_ACCESS_READ |
- SNDRV_CTL_ELEM_ACCESS_VOLATILE,
-   .iface = SNDRV_CTL_ELEM_IFACE_PCM,
-   .name = "ELD",
-   .info = hdmi_eld_ctl_info,
-   .get = hdmi_eld_ctl_get,
-   },
-};
-
 static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
 struct snd_soc_dai *dai)
 {
@@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime 
*rtd,
.pcm_new = hdmi_codec_pcm_new,
 };
 
+static int hdmi_last_id = 0;
+static int hdmi_probe(struct snd_soc_component *component)
+{
+   struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
+   static struct snd_soc_component *component_1st = NULL;
+   char *name = "ELD";
+
+   /*
+* It will use "ELD"   if hdmi_codec was used from only 1 device.
+* It will use "ELD.x" if hdmi_codec was used from many devices.
+* Then, first "ELD" will be replaced to "ELD.0"
+*/
+   snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id);
+
+   if (hcp->id == 0)
+   component_1st = component;
+
+   if (hdmi_last_id > 1) {
+   name = hcp->eld_name;
+
+   if (component_1st) {
+   struct hdmi_codec_priv *hcp_1st;
+
+   /* replaced 1st "ELD" to "ELD.0" */
+   hcp_1st = snd_soc_component_get_drvdata(component_1st);
+   hcp_1st->control.name = hcp_1st->eld_name;
+   }
+   }
+
+   hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE;
+   hcp->control.iface  = SNDRV_CTL_ELEM_IFACE_PCM;
+   hcp->control.name   = name;
+   hcp->control.info   = hdmi_eld_ctl_info;
+   hcp->control.get= hdmi_eld_ctl_get;
+
+   return snd_soc_add_component_controls(component, >control, 1);
+}
+
 static int hdmi_of_xlate_dai_id(struct snd_soc_component *component,
 struct device_node *endpoint)
 {
@@ -732,8 +763,7 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component 
*component,
 
 static struct snd_soc_codec_driver hdmi_codec = {
.component_driver = {
-   .controls   = hdmi_controls,
-   .num_controls   = ARRAY_SIZE(hdmi_controls),
+   .probe  = hdmi_probe,
.dapm_widgets   = hdmi_widgets,
.num_dapm_widgets   = ARRAY_SIZE(hdmi_widgets),
.dapm_routes= hdmi_routes,
@@ -768,6 +798,7 @@ static int hdmi_codec_probe(struct platform_device *pdev)
if (!hcp)
return -ENOMEM;
 
+   hcp->id = hdmi_last_id;
hcp->hcd = *hcd;
mutex_init(>current_stream_lock);
 
@@ -795,6 +826,7 @@ static int hdmi_codec_probe(struct platform_device *pdev)
}
 
dev_set_drvdata(dev, hcp);
+   hdmi_last_id++;
return 0;
 }
 
-- 
1.9.1