Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 19 December 2017 at 04:56, Masahiro Yamada
 wrote:
> Hi Ulf,
>
>
> 2017-12-15 18:18 GMT+09:00 Ulf Hansson :
>> On 24 November 2017 at 17:24, Masahiro Yamada
>>  wrote:
>>>
>>> I am working on this IP for Socionext SoCs.
>>>
>>> I was hit by several issues, and noticed various
>>> clean-up candidates.
>>>
>>>  - Fix and clean-up Kconfig
>>>  - Fix various card detection problems
>>>  - Move Renesas private data out of TMIO core
>>>  - Allow to perform platform-specific settings before MMC host starts
>>>  - Fix weird IRQ handling
>>>
>>> I am getting more and more patches for TMIO.
>>> I put all in a single series to clarify the patch order.
>>>
>>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>>
>>>
>>> Masahiro Yamada (22):
>>>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>>>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>>>   mmc: renesas_sdhi: remove eprobe jump label
>>>   mmc: tmio: set tmio_mmc_host to driver data
>>>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>>>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>>>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>>>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>>>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>>> TMIO_MMC_USE_GPIO_CD
>>>   mmc: tmio: support IP-builtin card detection logic
>>>   mmc: renesas_sdhi: remove always false condition
>>>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>>>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>>> renesas_sdhi.h
>>>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>>>   mmc: tmio: change bus_shift to unsigned int
>>>   mmc: tmio: fix never-detected card insertion bug
>>>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>>>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>>> tmio_mmc_host_probe()
>>>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>>>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>>>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>>>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>>
>>>  drivers/mmc/host/Kconfig  |   5 +-
>>>  drivers/mmc/host/Makefile |   8 +-
>>>  drivers/mmc/host/renesas_sdhi.h   |  22 
>>>  drivers/mmc/host/renesas_sdhi_core.c  |  49 -
>>>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>>>  drivers/mmc/host/renesas_sdhi_sys_dmac.c  |  35 +++---
>>>  drivers/mmc/host/tmio_mmc.c   |  23 ++--
>>>  drivers/mmc/host/tmio_mmc.h   |  23 +---
>>>  drivers/mmc/host/tmio_mmc_core.c  | 149 
>>> +-
>>>  9 files changed, 170 insertions(+), 158 deletions(-)
>>>
>>> --
>>> 2.7.4
>>>
>>
>> To get this moving, I have applied patch 1->8 for next, thanks!
>
>
> Could you apply 11->15 as well?
> They were reviewed by Wolfram.

Applied for next!

>
> We can skip 9, 10.

Okay, thought there were a dependency.

Thanks and kind regards
Uffe

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

Hi Ulf,


2017-12-15 18:18 GMT+09:00 Ulf Hansson :
> On 24 November 2017 at 17:24, Masahiro Yamada
>  wrote:
>>
>> I am working on this IP for Socionext SoCs.
>>
>> I was hit by several issues, and noticed various
>> clean-up candidates.
>>
>>  - Fix and clean-up Kconfig
>>  - Fix various card detection problems
>>  - Move Renesas private data out of TMIO core
>>  - Allow to perform platform-specific settings before MMC host starts
>>  - Fix weird IRQ handling
>>
>> I am getting more and more patches for TMIO.
>> I put all in a single series to clarify the patch order.
>>
>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>
>>
>> Masahiro Yamada (22):
>>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>>   mmc: renesas_sdhi: remove eprobe jump label
>>   mmc: tmio: set tmio_mmc_host to driver data
>>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>> TMIO_MMC_USE_GPIO_CD
>>   mmc: tmio: support IP-builtin card detection logic
>>   mmc: renesas_sdhi: remove always false condition
>>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>> renesas_sdhi.h
>>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>>   mmc: tmio: change bus_shift to unsigned int
>>   mmc: tmio: fix never-detected card insertion bug
>>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>> tmio_mmc_host_probe()
>>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>
>>  drivers/mmc/host/Kconfig  |   5 +-
>>  drivers/mmc/host/Makefile |   8 +-
>>  drivers/mmc/host/renesas_sdhi.h   |  22 
>>  drivers/mmc/host/renesas_sdhi_core.c  |  49 -
>>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>>  drivers/mmc/host/renesas_sdhi_sys_dmac.c  |  35 +++---
>>  drivers/mmc/host/tmio_mmc.c   |  23 ++--
>>  drivers/mmc/host/tmio_mmc.h   |  23 +---
>>  drivers/mmc/host/tmio_mmc_core.c  | 149 
>> +-
>>  9 files changed, 170 insertions(+), 158 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> To get this moving, I have applied patch 1->8 for next, thanks!


Could you apply 11->15 as well?
They were reviewed by Wolfram.

We can skip 9, 10.





-- 
Best Regards
Masahiro Yamada

RE: [PATCH 4/4] arm64: dts: r8a7796: Add DEVFREQ support for GSX

[Gaku Inami]

Hi Geert-san, Ulrich-san,

> Do they really all use the same voltage?
>
> And AFAIU, DVFS is shared between CPU and GPU.

We support the opp table of GPU with the same voltage for each in
R-Car H3/M3. As you know, DVFS is shared between CA57 and GPU.
We understand that arbitration control of the voltage from both
cores can't be realized with the current CPUFREQ and DEVFREQ
framework. Therefore, we have implemented as above now.

In our test with GPU team, it is fine except power consumption.

Regards,
Inami

-Original Message-
From: geert.uytterhoe...@gmail.com [mailto:geert.uytterhoe...@gmail.com] On 
Behalf Of Geert Uytterhoeven
Sent: Monday, December 18, 2017 7:27 PM
To: Ulrich Hecht 
Cc: Linux-Renesas ; Laurent Pinchart 
; Magnus Damm ; Gaku 
Inami ; TAKESHI KIHARA 
; Hien Dang 
Subject: Re: [PATCH 4/4] arm64: dts: r8a7796: Add DEVFREQ support for GSX

Hi Uli,

On Fri, Dec 15, 2017 at 4:14 PM, Ulrich Hecht  
wrote:
> From: Gaku Inami 
>
> This patch supports GSX frequency scaling using devfreq.
>
> Includes update of the OPP tables for GSX because the typical voltage 
> of VDD_DVFS (power optimized) was updated to 0.83V by R-Car Gen3 EC 
> Rev0.54E specification.
>
> Signed-off-by: Gaku Inami 
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Hien Dang 
> Signed-off-by: Ulrich Hecht 

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -166,6 +166,27 @@
> };
> };
>
> +   gsx_opp_table: gsx_opp_table {
> +   compatible = "operating-points-v2";
> +
> +   opp@2 {

opp-2 (try "make dtbs W=1")

> +   opp-hz = /bits/ 64 <2>;
> +   opp-microvolt = <83>;
> +   };
> +   opp@3 {
> +   opp-hz = /bits/ 64 <3>;
> +   opp-microvolt = <83>;
> +   };
> +   opp@4 {
> +   opp-hz = /bits/ 64 <4>;
> +   opp-microvolt = <83>;
> +   };
> +   opp@6 {
> +   opp-hz = /bits/ 64 <6>;
> +   opp-microvolt = <83>;

Do they really all use the same voltage?

And AFAIU, DVFS is shared between CPU and GPU.

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 6/6] phy: renesas: rcar-gen3-usb2: add gpio handling

[Rob Herring]

On Sun, Dec 17, 2017 at 11:16 PM, Yoshihiro Shimoda
 wrote:
> Hi Rob,
>
> Thank you for your comments!
>
>> -Original Message-
>> From: Rob Herring, Sent: Sunday, December 17, 2017 3:05 AM
>>
>> On Fri, Dec 15, 2017 at 02:45:26PM +0900, Yoshihiro Shimoda wrote:
>> > Some R-Car SoCs (e.g. R-Car D3) doesn't have dedicated pins of VBUS
>> > and ID. So, they may be connected to gpio pins. To handle the gpio
>> > pins, this patch adds the handling of VBUS and ID pins instead of
>> > dedicated pins.
>> >
>> > Signed-off-by: Yoshihiro Shimoda 
>> > ---
>> >  .../devicetree/bindings/phy/rcar-gen3-phy-usb2.txt |  2 +
>> >  drivers/phy/renesas/phy-rcar-gen3-usb2.c   | 77 
>> > --
>> >  2 files changed, 72 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
>> b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
>> > index 99b651b..999a6ef 100644
>> > --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
>> > +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
>> > @@ -27,6 +27,8 @@ channel as USB OTG:
>> >  - interrupts: interrupt specifier for the PHY.
>> >  - vbus-supply: Phandle to a regulator that provides power to the VBUS. 
>> > This
>> >regulator will be managed during the PHY power on/off sequence.
>> > +- vbus-gpios: use gpio to control vbus instead of dedicated pin.
>>
>> How does this relate to vbus-supply? A gpio-regulator doesn't work here?
>
> Thank you for the pointed out. You're correct. A gpio-regulator can work 
> intead of "vbus-gpios".
> So, I will drop this.
>
>> > +- id-gpios: use gpio to detect id instead of dedicated pin.
>>
>> These aren't part of the phy, but really belong in a connector node.
>
> I'm afraid but I don't understand "a connector node" mean.
> Is it an extcon device for instance?
> If so, I'll try to use extcon-usb-gpio.c instead of own gpio handling on the 
> driver.

No, extcon binding has a lot of Linuxisms in it. I mean something like
hdmi-connector binding. There's been some attempts[1] for USB
connectors, but nothing finalized.

Rob

[1] https://www.spinics.net/lists/linux-usb/msg161104.html

Re: [PATCH/RFC v2 06/15] rcar-csi2: use frame description information when propagating .s_stream()

[Niklas Söderlund]

Hi Sakari,

Thanks for your comments.

On 2017-12-15 15:19:36 +0200, Sakari Ailus wrote:
> Hi Niklas,
> 
> On Thu, Dec 14, 2017 at 08:08:26PM +0100, Niklas Söderlund wrote:
> > Use the frame description from the remote subdevice of the rcar-csi2's
> > sink pad to get the remote pad and stream pad needed to propagate the
> > .s_stream() operation.
> > 
> > The CSI-2 virtual channel which should be acted upon can be determined
> > by looking at which of the rcar-csi2 source pad the .s_stream() was
> > called on. This is because the rcar-csi2 acts as a demultiplexer for the
> > CSI-2 link on the one sink pad and outputs each virtual channel on a
> > distinct and known source pad.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 58 
> > -
> >  1 file changed, 41 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index e0f56cc3d25179a9..6b607b2e31e26063 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -614,20 +614,31 @@ static void rcar_csi2_stop(struct rcar_csi2 *priv)
> > rcar_csi2_reset(priv);
> >  }
> >  
> > -static int rcar_csi2_sd_info(struct rcar_csi2 *priv, struct v4l2_subdev 
> > **sd)
> > +static int rcar_csi2_get_source_info(struct rcar_csi2 *priv,
> > +struct v4l2_subdev **subdev,
> 
> I wonder if using struct media_pad for this would be cleaner.

I can give it a try and see how it works out, thanks for the suggestion.

> 
> > +unsigned int *pad,
> > +struct v4l2_mbus_frame_desc *fd)
> >  {
> > -   struct media_pad *pad;
> > +   struct media_pad *remote_pad;
> >  
> > -   pad = media_entity_remote_pad(>pads[RCAR_CSI2_SINK]);
> > -   if (!pad) {
> > -   dev_err(priv->dev, "Could not find remote pad\n");
> > +   /* Get source subdevice and pad */
> > +   remote_pad = media_entity_remote_pad(>pads[RCAR_CSI2_SINK]);
> > +   if (!remote_pad) {
> > +   dev_err(priv->dev, "Could not find remote source pad\n");
> > return -ENODEV;
> > }
> > +   *subdev = media_entity_to_v4l2_subdev(remote_pad->entity);
> > +   *pad = remote_pad->index;
> >  
> > -   *sd = media_entity_to_v4l2_subdev(pad->entity);
> > -   if (!*sd) {
> > -   dev_err(priv->dev, "Could not find remote subdevice\n");
> > -   return -ENODEV;
> > +   /* Get frame descriptor */
> > +   if (v4l2_subdev_call(*subdev, pad, get_frame_desc, *pad, fd)) {
> > +   dev_err(priv->dev, "Could not read frame desc\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   if (fd->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> > +   dev_err(priv->dev, "Frame desc do not describe CSI-2 link");
> > +   return -EINVAL;
> 
> I think this should also work with drivers that do not support frame
> descriptors.
> 
> Alternatively support could be added for all drivers. In practice this
> would mean having a few bus specific implementations of get_frame_desc op
> that would dig the information from the frame format.
> 
> Perhaps the former option would make sense here, for now.

I will try to give it some thought when I rework this series. At the 
moment I'm not sure what is the best idea :-)

> 
> > }
> >  
> > return 0;
> > @@ -637,9 +648,10 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> > unsigned int pad,
> >   unsigned int stream, int enable)
> >  {
> > struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +   struct v4l2_mbus_frame_desc fd;
> > struct v4l2_subdev *nextsd;
> > -   unsigned int i, count = 0;
> > -   int ret, vc;
> > +   unsigned int i, rpad, count = 0;
> > +   int ret, vc, rstream = -1;
> >  
> > /* Only allow stream control on source pads and valid vc */
> > vc = rcar_csi2_pad_to_vc(pad);
> > @@ -650,11 +662,23 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> > unsigned int pad,
> > if (stream != 0)
> > return -EINVAL;
> >  
> > -   mutex_lock(>lock);
> > -
> > -   ret = rcar_csi2_sd_info(priv, );
> > +   /* Get information about multiplexed link */
> > +   ret = rcar_csi2_get_source_info(priv, , , );
> > if (ret)
> > -   goto out;
> > +   return ret;
> > +
> > +   /* Get stream on multiplexed link */
> > +   for (i = 0; i < fd.num_entries; i++)
> > +   if (fd.entry[i].bus.csi2.channel == vc)
> > +   rstream = fd.entry[i].stream;
> 
> Virtual channel does not equate to the stream. You'll need the data type,
> too.
> 
> You should actually obtain this from the configured routes instead.

You are correct, will fix.

> 
> How does this work btw. if you have several streams on the same virtual
> channel that only have different data types?

I have not been able to test this 

Re: [PATCH/RFC v2 05/15] rcar-csi2: count usage for each source pad

[Niklas Söderlund]

Hi Sakari,

Tack för dina kommentarer.

On 2017-12-15 14:25:27 +0200, Sakari Ailus wrote:
> On Thu, Dec 14, 2017 at 08:08:25PM +0100, Niklas Söderlund wrote:
> > The R-Car CSI-2 hardware can output the same virtual channel
> > simultaneously to more then one R-Car VIN. For this reason we need to
> > move the usage counting from the global device to each source pad.
> > 
> > If a source pads usage count go from 0 to 1 we need to signal that a new
> > stream should start, likewise if it go from 1 to 0 we need to stop a
> > stream. At the same time we only want to start or stop the R-Car
> > CSI-2 device only on the first or last stream.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 38 
> > +++--
> >  1 file changed, 30 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 8ce0bfeef1113f9c..e0f56cc3d25179a9 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -328,6 +328,14 @@ enum rcar_csi2_pads {
> > NR_OF_RCAR_CSI2_PAD,
> >  };
> >  
> > +static int rcar_csi2_pad_to_vc(unsigned int pad)
> > +{
> > +   if (pad < RCAR_CSI2_SOURCE_VC0 || pad > RCAR_CSI2_SOURCE_VC3)
> > +   return -EINVAL;
> > +
> > +   return pad - RCAR_CSI2_SOURCE_VC0;
> > +}
> > +
> >  struct rcar_csi2_info {
> > const struct phypll_hsfreqrange *hsfreqrange;
> > const struct phtw_testdin_data *testdin_data;
> > @@ -350,7 +358,7 @@ struct rcar_csi2 {
> > struct v4l2_mbus_framefmt mf;
> >  
> > struct mutex lock;
> > -   int stream_count;
> > +   int stream_count[4];
> 
> Why 4?

There are 4 source pads connected to up to 8 different remote sink pads.

> 
> >  
> > unsigned short lanes;
> > unsigned char lane_swap[4];
> > @@ -630,7 +638,13 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> > unsigned int pad,
> >  {
> > struct rcar_csi2 *priv = sd_to_csi2(sd);
> > struct v4l2_subdev *nextsd;
> > -   int ret;
> > +   unsigned int i, count = 0;
> > +   int ret, vc;
> > +
> > +   /* Only allow stream control on source pads and valid vc */
> > +   vc = rcar_csi2_pad_to_vc(pad);
> > +   if (vc < 0)
> > +   return vc;
> >  
> > /* Only one stream on each source pad */
> > if (stream != 0)
> > @@ -642,7 +656,10 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> > unsigned int pad,
> > if (ret)
> > goto out;
> >  
> > -   if (enable && priv->stream_count == 0) {
> > +   for (i = 0; i < 4; i++)
> > +   count += priv->stream_count[i];
> > +
> > +   if (enable && count == 0) {
> > pm_runtime_get_sync(priv->dev);
> >  
> > ret = rcar_csi2_start(priv);
> > @@ -650,20 +667,23 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> > unsigned int pad,
> > pm_runtime_put(priv->dev);
> > goto out;
> > }
> > +   } else if (!enable && count == 1) {
> > +   rcar_csi2_stop(priv);
> > +   pm_runtime_put(priv->dev);
> > +   }
> >  
> > +   if (enable && priv->stream_count[vc] == 0) {
> > ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
> > if (ret) {
> > rcar_csi2_stop(priv);
> > pm_runtime_put(priv->dev);
> > goto out;
> > }
> > -   } else if (!enable && priv->stream_count == 1) {
> > -   rcar_csi2_stop(priv);
> > +   } else if (!enable && priv->stream_count[vc] == 1) {
> 
> Do I understand correctly that you can start and streams here one by one,
> independently of each other?

That is still an area we are figuring out. At this time I don't know if 
the hardware is capable of starting and stopping individual streams. We 
are working with our setup to try and get stuff up and running but we 
are having issues at our sensor side. For our experiments I wished to 
prepare to test this as I know I can route a single CSI-2 VC to more 
then one video device consumer and start and stop them independently.

Maybe it will be different once we manage to get more simultaneously VCs 
running.

> 
> Sometimes this might not be the case. I wonder if we should have a way to
> tell that to the caller.

You make a good point. How should this really be handeld? Maybe I'm to 
focused on my test setup which might not work as I think it dose. Would 
you say a better approach would be to drop the stream parameter to pad 
ops s_stream() and just start all streams of routes who are enabled? 

> 
> > ret = v4l2_subdev_call(nextsd, video, s_stream, 0);
> > -   pm_runtime_put(priv->dev);
> > }
> >  
> > -   priv->stream_count += enable ? 1 : -1;
> > +   priv->stream_count[vc] += enable ? 1 : -1;
> >  out:
> > mutex_unlock(>lock);
> >  
> > @@ -919,7 +939,9 @@ static int 

Re: [PATCH/RFC v2 03/15] rcar-vin: use the pad and stream aware s_stream

[Niklas Söderlund]

Hej Sakari,

Tack för dina kommentarer.

On 2017-12-15 14:07:39 +0200, Sakari Ailus wrote:
> Hej,
> 
> On Thu, Dec 14, 2017 at 08:08:23PM +0100, Niklas Söderlund wrote:
> > To work with multiplexed streams the pad and stream aware s_stream
> > operation needs to be used.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-dma.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> > b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index cf30e5fceb1d493a..8435491535060eae 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -1180,7 +1180,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int 
> > on)
> >  
> > if (!on) {
> > media_pipeline_stop(vin->vdev.entity.pads);
> > -   return v4l2_subdev_call(sd, video, s_stream, 0);
> > +   return v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 0);
> 
> Have you thought of adding a wrapper for the s_stream callback?

I toyed with the idea, then I reached the same conclusion you do bellow.

> 
> I think you should either change all s_stream callbacks from video to pad,
> or add a wrapper which then calls the video op instead of the pad op if the
> pad op does not exist. Otherwise we again have two non-interoperable
> classes of drivers for no good reason.

Agreed.

> 
> Thinking about it, I'm not all that certain changing all instances would be
> that much work in the end; it should be done anyway. Devices that have a
> single stream (i.e. everything right now) just don't care about the pad
> number.

I tried to cover this in the cover-letter, I believe the correct 
approach is probably to move s_stream() from video ops to pad ops in the 
long run. I did post a similar patch for this a while ago [1] but I fear 
it's outdated by now. Before I refreshed that particular patch I was 
interested on the feedback on this from this series as I don't want to 
send out a patch touching so many drivers without at least some 
discussion :-)

I would be find with both the helper and a full conversion approach. Or 
to do it in stages, add a helper now and slowly convert all drivers and 
then removing the helper. What would be your preferred way of dealing 
with this?

1.  
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-June/091250.html

> 
> > }
> >  
> > fmt.pad = pad->index;
> > @@ -1239,12 +1239,14 @@ static int rvin_set_stream(struct rvin_dev *vin, 
> > int on)
> > if (media_pipeline_start(vin->vdev.entity.pads, pipe))
> > return -EPIPE;
> >  
> > -   ret = v4l2_subdev_call(sd, video, s_stream, 1);
> > +   ret = v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 1);
> > if (ret == -ENOIOCTLCMD)
> > ret = 0;
> > if (ret)
> > media_pipeline_stop(vin->vdev.entity.pads);
> >  
> > +   vin_dbg(vin, "pad: %u stream: 0 enable: %d\n", pad->index, on);
> > +
> > return ret;
> >  }
> >  
> > -- 
> > 2.15.1
> > 
> 
> -- 
> Sakari Ailus
> sakari.ai...@linux.intel.com

-- 
Regards,
Niklas Söderlund

Re: [PATCH/RFC v2 02/15] rcar-vin: use pad as the starting point for a pipeline

[Niklas Söderlund]

Hej Sakari,

Tack för dina kommentarer.

On 2017-12-15 13:54:02 +0200, Sakari Ailus wrote:
> On Thu, Dec 14, 2017 at 08:08:22PM +0100, Niklas Söderlund wrote:
> > The pipeline will be moved from the entity to the pads; reflect this in
> > the media pipeline function API.
> 
> I'll merge this to "media: entity: Use pad as the starting point for a
> pipeline" if you're fine with that.

I'm fine with that, the issue is that the rcar-vin Gen3 driver is not 
yet upstream :-( If it makes it upstream before the work in your vc 
branch feel free to squash this in. Until then I fear I need to keep 
carry this in this series.

> 
> I haven't compiled everything for some time, and newly added drivers may be
> lacking these changes. I'll re-check that soonish.
> 
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-dma.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> > b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index 03a914361a33125c..cf30e5fceb1d493a 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -1179,7 +1179,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int 
> > on)
> > return -EPIPE;
> >  
> > if (!on) {
> > -   media_pipeline_stop(>vdev.entity);
> > +   media_pipeline_stop(vin->vdev.entity.pads);
> > return v4l2_subdev_call(sd, video, s_stream, 0);
> > }
> >  
> > @@ -1235,15 +1235,15 @@ static int rvin_set_stream(struct rvin_dev *vin, 
> > int on)
> > fmt.format.height != vin->format.height)
> > return -EPIPE;
> >  
> > -   pipe = sd->entity.pipe ? sd->entity.pipe : >vdev.pipe;
> > -   if (media_pipeline_start(>vdev.entity, pipe))
> > +   pipe = sd->entity.pads->pipe ? sd->entity.pads->pipe : >vdev.pipe;
> > +   if (media_pipeline_start(vin->vdev.entity.pads, pipe))
> > return -EPIPE;
> >  
> > ret = v4l2_subdev_call(sd, video, s_stream, 1);
> > if (ret == -ENOIOCTLCMD)
> > ret = 0;
> > if (ret)
> > -   media_pipeline_stop(>vdev.entity);
> > +   media_pipeline_stop(vin->vdev.entity.pads);
> >  
> > return ret;
> >  }
> > -- 
> > 2.15.1
> > 
> 
> -- 
> Sakari Ailus
> sakari.ai...@linux.intel.com

-- 
Regards,
Niklas Söderlund

Re: [PATCH/RFC v2 01/15] v4l2-subdev.h: add pad and stream aware s_stream

[Niklas Söderlund]

Hej Sakari,

Tack för dina kommentarer.

On 2017-12-15 13:51:46 +0200, Sakari Ailus wrote:
> Hejssan Niklas,
> 
> Tack för uppdaterade lappor!
> 
> On Thu, Dec 14, 2017 at 08:08:21PM +0100, Niklas Söderlund wrote:
> > To be able to start and stop individual streams of a multiplexed pad the
> > s_stream operation needs to be both pad and stream aware. Add a new
> > operation to pad ops to facilitate this.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  include/media/v4l2-subdev.h | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index a30a94fad8dbacde..7288209338a48fda 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -669,6 +669,9 @@ struct v4l2_subdev_pad_config {
> >   *
> >   * @set_frame_desc: set the low level media bus frame parameters, @fd array
> >   *  may be adjusted by the subdev driver to device 
> > capabilities.
> > + *
> > + * @s_stream: used to notify the driver that a stream will start or has
> > + * stopped.
> 
> This is actually the callback which is used to control the stream state.
> The above suggests that it's a notification of something that has happened
> (or about to happen). How about:
> 
> Enable or disable streaming on a sub-device pad.

Better, will use it in next version.

> 
> >   */
> >  struct v4l2_subdev_pad_ops {
> > int (*init_cfg)(struct v4l2_subdev *sd,
> > @@ -713,6 +716,8 @@ struct v4l2_subdev_pad_ops {
> >struct v4l2_subdev_routing *route);
> > int (*set_routing)(struct v4l2_subdev *sd,
> >struct v4l2_subdev_routing *route);
> > +   int (*s_stream)(struct v4l2_subdev *sd, unsigned int pad,
> > +   unsigned int stream, int enable);
> 
> How about bool for enable?

I tried to use what the current s_stream uses, but I be happy to make 
enable a bool in the next version.

> 
> >  };
> >  
> >  /**
> 
> -- 
> Sakari Ailus
> sakari.ai...@linux.intel.com

-- 
Regards,
Niklas Söderlund

Re: [PATCH/RFC v2 14/15] adv748x: csi2: add get_routing support

[Niklas Söderlund]

Hi Kieran,

Thanks for your comments,

On 2017-12-14 23:27:57 +, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 14/12/17 19:08, Niklas Söderlund wrote:
> > To support multiplexed streams the internal routing between the
> > adv748x sink pad and its source pad needs to be described.
> 
> The adv748x has quite a few sink and source pads... I presume here you mean 
> the
> adv748x csi2 sink and source pad :D

Yes :-) Will fix for next version, thanks.

> 
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/i2c/adv748x/adv748x-csi2.c | 22 ++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c 
> > b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > index 291b35bef49d41fb..dbefb53f5b8c414d 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > @@ -262,10 +262,32 @@ static int adv748x_csi2_get_frame_desc(struct 
> > v4l2_subdev *sd, unsigned int pad,
> > return 0;
> >  }
> >  
> > +static int adv748x_csi2_get_routing(struct v4l2_subdev *subdev,
> > +   struct v4l2_subdev_routing *routing)
> > +{
> > +   struct v4l2_subdev_route *r = routing->routes;
> > +
> > +   if (routing->num_routes < 1) {
> > +   routing->num_routes = 1;
> > +   return -ENOSPC;
> > +   }
> > +
> > +   routing->num_routes = 1;
> > +
> > +   r->sink_pad = ADV748X_CSI2_SINK;
> > +   r->sink_stream = 0;
> > +   r->source_pad = ADV748X_CSI2_SOURCE;
> > +   r->source_stream = 0;
> > +   r->flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE | V4L2_SUBDEV_ROUTE_FL_IMMUTABLE;
> > +
> > +   return 0;
> > +}
> > +
> 
> So - I think this is fine - but it seems a lot of code to define a static
> default route which describes a single link between it's sink pad - and its
> source pad ...
> 
> I suspect this should/could be wrapped by some helpers in core for cases like
> this, as it's the simple case - but as we don't currently have that I guess we
> have to put this in here for now ?

Yes for now we need to fill in the information here.

> 
> Maybe we should have a helper to make this

I'm sure there could be v4l2 helpers for such a case. I don't even think 
you wound need to prime it with any information. If there is only one 
source and one sink I'm sure a helper function can figure it out :-)

> 
> return v4l2_subdev_single_route(subdev, routing,
> ADV748X_CS2_SINK, 0,
> ADV748X_CSI2_SOURCE, 0,
>   V4L2_SUBDEV_ROUTE_FL_ACTIVE | 
> V4L2_SUBDEV_ROUTE_FL_IMMUTABLE);
> 
> Or maybe even define these static routes in a struct somehow?

For more complex setups a struct could be used together with a helper 
function to decode it. But then again maybe it's easier to just define a 
const v4l2_subdev_route array and 'routing->routes = my_const_routes' ?

> 
> >  static const struct v4l2_subdev_pad_ops adv748x_csi2_pad_ops = {
> > .get_fmt = adv748x_csi2_get_format,
> > .set_fmt = adv748x_csi2_set_format,
> > .get_frame_desc = adv748x_csi2_get_frame_desc,
> > +   .get_routing = adv748x_csi2_get_routing,
> > .s_stream = adv748x_csi2_s_stream,
> >  };
> >  
> > 

-- 
Regards,
Niklas Söderlund

Re: [PATCH/RFC v2 09/15] adv748x: csi2: add module param for virtual channel

[Niklas Söderlund]

Hi Kieran,

Thanks for your comments.

On 2017-12-14 22:19:59 +, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 14/12/17 19:08, Niklas Söderlund wrote:
> > The hardware can output on any of the 4 (0-3) Virtual Channels of the
> > CSI-2 bus. Add a module parameter each for TXA and TXB to allow the user
> > to specify which channel should be used.
> 
> This patch only configures the channel at initialisation time, (which is a 
> valid
> thing to do here at the moment I think) - but will we expect to provide
> functionality to change the virtual channel later ?

I had no plan to add such functionality. But I would be open to 
suggesters on where to add such a control. I thought about this when 
working with this patch-set and I can think of three other places to 
control this.

1. A debugfs file
2. A configfs file
3. Define 4 streams in the frame descriptor for the source pad, one for 
   each CSI-2 VC. Then use the new G/S_ROUTE ioctls to control which 
   stream of the source the sink is connected to.

I thought option 1 and 2 was not the correct place for such a control.  
And option 3 would make the control of the VC output depending on this 
patch-set and all its dependencies. And since my use-case for this was 
patch is to test this patch-set it seemed silly at the time :-) But the 
more I think of this might be the best way forward for this type of 
things, what do you think?

> 
> Do we need to communicate the virtual channel in use across the media pad 
> links
> somehow? (or does that already happen?)

Yes, this is part of this patch-set and its dependencies. The frame 
descriptor contains information about stream to CSI-2 virtual channel 
(and data type) mapping. While the G/S_ROUTE operations contains the 
controls and information on which stream of a multiplexed pad is routed 
to which 'normal' pad.

> 
> Perhaps the commit message could be clear on the fact that this only sets the
> channels initialisation value - and that modifying the module parameter after
> module load will have no effect?

Is this not true for all module parameters, they only have an effect at 
module load time? Can you even modify them after module load?

> 
> Regards
> 
> Kieran
> 
> 
> > Signed-off-by: Niklas Söderlund 
> >
> > ---
> >  drivers/media/i2c/adv748x/adv748x-core.c | 10 ++
> >  drivers/media/i2c/adv748x/adv748x-csi2.c |  2 +-
> >  drivers/media/i2c/adv748x/adv748x.h  |  1 +
> >  3 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/adv748x/adv748x-core.c 
> > b/drivers/media/i2c/adv748x/adv748x-core.c
> > index fd92c9e4b519d2c5..3cad52532ead2e34 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-core.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> > @@ -31,6 +31,9 @@
> >  
> >  #include "adv748x.h"
> >  
> > +static unsigned int txavc;
> > +static unsigned int txbvc;
> > +
> >  /* 
> > -
> >   * Register manipulation
> >   */
> > @@ -747,6 +750,7 @@ static int adv748x_probe(struct i2c_client *client,
> > }
> >  
> > /* Initialise TXA */
> > +   state->txa.vc = txavc;
> > ret = adv748x_csi2_init(state, >txa);
> > if (ret) {
> > adv_err(state, "Failed to probe TXA");
> > @@ -754,6 +758,7 @@ static int adv748x_probe(struct i2c_client *client,
> > }
> >  
> > /* Initialise TXB */
> > +   state->txb.vc = txbvc;
> > ret = adv748x_csi2_init(state, >txb);
> > if (ret) {
> > adv_err(state, "Failed to probe TXB");
> > @@ -824,6 +829,11 @@ static struct i2c_driver adv748x_driver = {
> >  
> >  module_i2c_driver(adv748x_driver);
> >  
> > +module_param(txavc, uint, 0644);
> > +MODULE_PARM_DESC(txavc, "Virtual Channel for TXA");
> > +module_param(txbvc, uint, 0644);
> > +MODULE_PARM_DESC(txbvc, "Virtual Channel for TXB");
> > +
> >  MODULE_AUTHOR("Kieran Bingham ");
> >  MODULE_DESCRIPTION("ADV748X video decoder");
> >  MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c 
> > b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > index 820b44ed56a8679f..2a5dff8c571013bf 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > @@ -281,7 +281,7 @@ int adv748x_csi2_init(struct adv748x_state *state, 
> > struct adv748x_csi2 *tx)
> > }
> >  
> > /* Initialise the virtual channel */
> > -   adv748x_csi2_set_virtual_channel(tx, 0);
> > +   adv748x_csi2_set_virtual_channel(tx, tx->vc);
> >  
> > adv748x_subdev_init(>sd, state, _csi2_ops,
> > MEDIA_ENT_F_UNKNOWN,
> > diff --git a/drivers/media/i2c/adv748x/adv748x.h 
> > b/drivers/media/i2c/adv748x/adv748x.h
> > index 6789e2f3bc8c2b49..f6e40ee3924e8f12 100644
> > --- a/drivers/media/i2c/adv748x/adv748x.h
> > +++ b/drivers/media/i2c/adv748x/adv748x.h
> > @@ -92,6 +92,7 @@ enum adv748x_csi2_pads {
> >  
> >  struct 

Re: [PATCH 3/8] media: v4l2-async: simplify v4l2_async_subdev structure

[Alexandre Belloni]

On 18/12/2017 at 17:53:57 -0200, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.
> 
> At drivers, this makes even clearer about the match criteria.
> 

For atmel:
Acked-by: Alexandre Belloni 

> Acked-by: Sylwester Nawrocki 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c|  6 +++---
>  drivers/media/platform/atmel/atmel-isc.c   |  2 +-
>  drivers/media/platform/atmel/atmel-isi.c   |  2 +-
>  drivers/media/platform/davinci/vpif_capture.c  |  4 ++--
>  drivers/media/platform/exynos4-is/media-dev.c  |  4 ++--
>  drivers/media/platform/pxa_camera.c|  2 +-
>  drivers/media/platform/qcom/camss-8x16/camss.c |  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c|  2 +-
>  drivers/media/platform/rcar_drif.c |  4 ++--
>  drivers/media/platform/soc_camera/soc_camera.c |  2 +-
>  drivers/media/platform/stm32/stm32-dcmi.c  |  2 +-
>  drivers/media/platform/ti-vpe/cal.c|  2 +-
>  drivers/media/platform/xilinx/xilinx-vipp.c|  2 +-
>  drivers/media/v4l2-core/v4l2-async.c   | 16 
>  drivers/media/v4l2-core/v4l2-fwnode.c  | 10 +-
>  drivers/staging/media/imx/imx-media-dev.c  |  4 ++--
>  include/media/v4l2-async.h |  8 ++--
>  17 files changed, 35 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index 0997c640191d..601ae6487617 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
>   vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
>  
>   for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
> - if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
> - asd[i].match.fwnode.fwnode) {
> + if (vpfe->cfg->asd[i]->match.fwnode ==
> + asd[i].match.fwnode) {
>   sdinfo = >cfg->sub_devs[i];
>   vpfe->sd[i] = subdev;
>   vpfe->sd[i]->grp_id = sdinfo->grp_id;
> @@ -2510,7 +2510,7 @@ vpfe_get_pdata(struct platform_device *pdev)
>   }
>  
>   pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
> + pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
>   of_node_put(rem);
>   }
>  
> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
> b/drivers/media/platform/atmel/atmel-isc.c
> index 0c2635647f69..34676409ca08 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -2088,7 +2088,7 @@ static int isc_parse_dt(struct device *dev, struct 
> isc_device *isc)
>   subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
>  
>   subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - subdev_entity->asd->match.fwnode.fwnode =
> + subdev_entity->asd->match.fwnode =
>   of_fwnode_handle(rem);
>   list_add_tail(_entity->list, >subdev_entities);
>   }
> diff --git a/drivers/media/platform/atmel/atmel-isi.c 
> b/drivers/media/platform/atmel/atmel-isi.c
> index e900995143a3..9958918e2449 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1128,7 +1128,7 @@ static int isi_graph_parse(struct atmel_isi *isi, 
> struct device_node *node)
>   /* Remote node to connect */
>   isi->entity.node = remote;
>   isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
>   return 0;
>   }
>  }
> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
> b/drivers/media/platform/davinci/vpif_capture.c
> index e45916f69def..e1c273c8b9a6 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier 
> *notifier,
>  
>   for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
>   struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> - const struct fwnode_handle *fwnode = _asd->match.fwnode.fwnode;
> + const 

Re: [PATCH/RFC v2 13/15] adv748x: csi2: only allow formats on sink pads

[Niklas Söderlund]

HI Kieran,

Thanks for your comments.

On 2017-12-14 23:16:08 +, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 14/12/17 19:08, Niklas Söderlund wrote:
> > The driver is now pad and stream aware, only allow to get/set format on
> > sink pads.
> 
> Ok - I can see the patch is doing this ...
> 
> > Also record a different format for each sink pad since it's
> > no longer true that they are all the same
> 
> But I can't see how the patch is doing this ^ ?
> 
> What have I missed?

I have missed moving this commit message to another patch when moving 
code around :-) Thanks for noticing. Will fix this for next version.

> 
> --
> Kieran
> 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/i2c/adv748x/adv748x-csi2.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c 
> > b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > index 39f993282dd3bb5c..291b35bef49d41fb 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > @@ -176,6 +176,9 @@ static int adv748x_csi2_get_format(struct v4l2_subdev 
> > *sd,
> > struct adv748x_state *state = tx->state;
> > struct v4l2_mbus_framefmt *mbusformat;
> >  
> > +   if (sdformat->pad != ADV748X_CSI2_SINK)
> > +   return -EINVAL;
> > +
> > mbusformat = adv748x_csi2_get_pad_format(sd, cfg, sdformat->pad,
> >  sdformat->which);
> > if (!mbusformat)
> > @@ -199,6 +202,9 @@ static int adv748x_csi2_set_format(struct v4l2_subdev 
> > *sd,
> > struct v4l2_mbus_framefmt *mbusformat;
> > int ret = 0;
> >  
> > +   if (sdformat->pad != ADV748X_CSI2_SINK)
> > +   return -EINVAL;
> > +
> > mbusformat = adv748x_csi2_get_pad_format(sd, cfg, sdformat->pad,
> >  sdformat->which);
> > if (!mbusformat)
> > 

-- 
Regards,
Niklas Söderlund

[RFC PATCH 6/7] i2c: sh_mobile: add new frequency calculation for later SoC

[Wolfram Sang]

The formula to generate the desired bus speeds has changed a little over
time. Implement the new formula and allow drivers to opt-in by changing
to this new config set. Ensure in probe that we don't divide by zero.
The returned values on a R-Car H2 (r8a7790/Lager board) match the
suggested values in the datasheet.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index b91a449a69..a0fddfe3c4497d 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -299,6 +299,18 @@ static int sh_mobile_i2c_init(struct sh_mobile_i2c_data 
*pd)
return sh_mobile_i2c_check_timing(pd);
 }
 
+static int sh_mobile_i2c_v2_init(struct sh_mobile_i2c_data *pd)
+{
+   unsigned long clks_per_cycle;
+
+   /* L = 5, H = 4, L + H = 9 */
+   clks_per_cycle = clk_get_rate(pd->clk) / pd->bus_speed;
+   pd->iccl = DIV_ROUND_UP(clks_per_cycle * 5 / 9 - 1, pd->clks_per_count);
+   pd->icch = DIV_ROUND_UP(clks_per_cycle * 4 / 9 - 5, pd->clks_per_count);
+
+   return sh_mobile_i2c_check_timing(pd);
+}
+
 static unsigned char i2c_op(struct sh_mobile_i2c_data *pd,
enum sh_mobile_i2c_op op, unsigned char data)
 {
@@ -785,6 +797,11 @@ static const struct sh_mobile_dt_config 
fast_clock_dt_config = {
.setup = sh_mobile_i2c_init,
 };
 
+static const struct sh_mobile_dt_config v2_freq_calc_dt_config = {
+   .clks_per_count = 2,
+   .setup = sh_mobile_i2c_v2_init,
+};
+
 static const struct sh_mobile_dt_config r8a7740_dt_config = {
.clks_per_count = 1,
.setup = sh_mobile_i2c_r8a7740_workaround,
@@ -875,7 +892,7 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
return PTR_ERR(pd->reg);
 
ret = of_property_read_u32(dev->dev.of_node, "clock-frequency", 
_speed);
-   pd->bus_speed = ret ? STANDARD_MODE : bus_speed;
+   pd->bus_speed = (ret || !bus_speed) ? STANDARD_MODE : bus_speed;
pd->clks_per_count = 1;
 
/* Newer variants come with two new bits in ICIC */
-- 
2.11.0

[RFC PATCH 7/7] i2c: sh_mobile: let r8a7790 (R-Car H2) use the new formula

[Wolfram Sang]

Make use of the new formula for more precise bus frequencies.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index a0fddfe3c4497d..d856bc211715e5 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -810,7 +810,7 @@ static const struct sh_mobile_dt_config r8a7740_dt_config = 
{
 static const struct of_device_id sh_mobile_i2c_dt_ids[] = {
{ .compatible = "renesas,iic-r8a73a4", .data = _clock_dt_config },
{ .compatible = "renesas,iic-r8a7740", .data = _dt_config },
-   { .compatible = "renesas,iic-r8a7790", .data = _clock_dt_config },
+   { .compatible = "renesas,iic-r8a7790", .data = _freq_calc_dt_config 
},
{ .compatible = "renesas,iic-r8a7791", .data = _clock_dt_config },
{ .compatible = "renesas,iic-r8a7792", .data = _clock_dt_config },
{ .compatible = "renesas,iic-r8a7793", .data = _clock_dt_config },
-- 
2.11.0

[RFC PATCH 0/7] i2c: sh_mobile: support per-SoC frequency calculation

[Wolfram Sang]

The current calculation for I2C bus speeds does not match the datasheet and is,
in deed, a little too fast for 100kHz settings (~107kHz have been measured).

This series implements a second calculation according to the datasheet. In
order to not cause regressions on the various old SoCs, using this new formula
is opt-in per SoC. Once the new formula has been verified to work, we can
switch over. By default, the old behaviour is retained.

So, the first patches are refactoring the driver to support per-SoC frequency
calculations. This is done by making the already existing per-SoC
setup()-callback mandatory and let the frequency calculation be done in there.

This series has been tested on a R-Car H2 Lager board (r8a7790). The computed
values match the suggested values in the datasheet. Also, measurements showed
that the bus frequency is now 100kHz. For 400 kHz, the overall bus speed did
not change much, but the low-to-high relationship is now matching the
datasheet, too.

This series is RFC because I think we need to discuss when to enable a SoC to
the new formula. For Lager, I did some basic checking but did not run a full
V4L testsuite or other complex configurations making use of I2C clients. Gen3,
in theory, should be easier to test because only the PMIC and an EEPROM are
connected to IIC_DVFS. However, there are no testpoints available for measuring
on Salvator-X(S). And the suggested values in the datasheet do not match the
module clock we have. Do ULCB boards maybe have testpoints for IIC_DVFS? For
Lager, it could be argued that the resulting speed is slower than before which
should not introduce regressions. But I am open to discussion here.

A branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/iic-refactor

Thanks, and looking forward to comments

   Wolfram


Wolfram Sang (7):
  i2c: sh_mobile: move type detection upwards
  i2c: sh_mobile: allow setup callback to return errno
  i2c: sh_mobile: require setup callback
  i2c: sh_mobile: let RuntimePM do the clock handling
  i2c: sh_mobile: add helper to check frequency calculations
  i2c: sh_mobile: add new frequency calculation for later SoC
  i2c: sh_mobile: let r8a7790 (R-Car H2) use the new formula

 drivers/i2c/busses/i2c-sh_mobile.c | 117 ++---
 1 file changed, 69 insertions(+), 48 deletions(-)

-- 
2.11.0

[RFC PATCH 5/7] i2c: sh_mobile: add helper to check frequency calculations

[Wolfram Sang]

Because we will add a second formula soon, put the sanity checks for the
computed results into a seperate function.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 49 +-
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index f1a9b971e2c14a..b91a449a69 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -246,11 +246,36 @@ static u32 sh_mobile_i2c_icch(unsigned long count_khz, 
u32 tHIGH, u32 tf)
return (((count_khz * (tHIGH + tf)) + 5000) / 1);
 }
 
+static int sh_mobile_i2c_check_timing(struct sh_mobile_i2c_data *pd)
+{
+   u16 max_val = pd->flags & IIC_FLAG_HAS_ICIC67 ? 0x1ff : 0xff;
+
+   if (pd->iccl > max_val || pd->icch > max_val) {
+   dev_err(pd->dev, "timing values out of range: L/H=0x%x/0x%x\n",
+   pd->iccl, pd->icch);
+   return -EINVAL;
+   }
+
+   /* one more bit of ICCL in ICIC */
+   if (pd->iccl & 0x100)
+   pd->icic |= ICIC_ICCLB8;
+   else
+   pd->icic &= ~ICIC_ICCLB8;
+
+   /* one more bit of ICCH in ICIC */
+   if (pd->icch & 0x100)
+   pd->icic |= ICIC_ICCHB8;
+   else
+   pd->icic &= ~ICIC_ICCHB8;
+
+   dev_dbg(pd->dev, "timing values: L/H=0x%x/0x%x\n", pd->iccl, pd->icch);
+   return 0;
+}
+
 static int sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
 {
unsigned long i2c_clk_khz;
u32 tHIGH, tLOW, tf;
-   uint16_t max_val;
 
i2c_clk_khz = clk_get_rate(pd->clk) / 1000 / pd->clks_per_count;
 
@@ -271,27 +296,7 @@ static int sh_mobile_i2c_init(struct sh_mobile_i2c_data 
*pd)
pd->iccl = sh_mobile_i2c_iccl(i2c_clk_khz, tLOW, tf);
pd->icch = sh_mobile_i2c_icch(i2c_clk_khz, tHIGH, tf);
 
-   max_val = pd->flags & IIC_FLAG_HAS_ICIC67 ? 0x1ff : 0xff;
-   if (pd->iccl > max_val || pd->icch > max_val) {
-   dev_err(pd->dev, "timing values out of range: L/H=0x%x/0x%x\n",
-   pd->iccl, pd->icch);
-   return -EINVAL;
-   }
-
-   /* one more bit of ICCL in ICIC */
-   if (pd->iccl & 0x100)
-   pd->icic |= ICIC_ICCLB8;
-   else
-   pd->icic &= ~ICIC_ICCLB8;
-
-   /* one more bit of ICCH in ICIC */
-   if (pd->icch & 0x100)
-   pd->icic |= ICIC_ICCHB8;
-   else
-   pd->icic &= ~ICIC_ICCHB8;
-
-   dev_dbg(pd->dev, "timing values: L/H=0x%x/0x%x\n", pd->iccl, pd->icch);
-   return 0;
+   return sh_mobile_i2c_check_timing(pd);
 }
 
 static unsigned char i2c_op(struct sh_mobile_i2c_data *pd,
-- 
2.11.0

[RFC PATCH 4/7] i2c: sh_mobile: let RuntimePM do the clock handling

[Wolfram Sang]

Start RuntimePM a bit earlier, so we can use it to enable the clock
during probe for frequency calculations. Make sure it is enabled before
calling setup().

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index 88af45225003a5..f1a9b971e2c14a 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -252,11 +252,7 @@ static int sh_mobile_i2c_init(struct sh_mobile_i2c_data 
*pd)
u32 tHIGH, tLOW, tf;
uint16_t max_val;
 
-   /* Get clock rate after clock is enabled */
-   clk_prepare_enable(pd->clk);
-   i2c_clk_khz = clk_get_rate(pd->clk) / 1000;
-   clk_disable_unprepare(pd->clk);
-   i2c_clk_khz /= pd->clks_per_count;
+   i2c_clk_khz = clk_get_rate(pd->clk) / 1000 / pd->clks_per_count;
 
if (pd->bus_speed == STANDARD_MODE) {
tLOW= 47;   /* tLOW = 4.7 us */
@@ -881,6 +877,20 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
if (resource_size(res) > 0x17)
pd->flags |= IIC_FLAG_HAS_ICIC67;
 
+   /* Enable Runtime PM for this device.
+*
+* Also tell the Runtime PM core to ignore children
+* for this device since it is valid for us to suspend
+* this I2C master driver even though the slave devices
+* on the I2C bus may not be suspended.
+*
+* The state of the I2C hardware bus is unaffected by
+* the Runtime PM state.
+*/
+   pm_suspend_ignore_children(>dev, true);
+   pm_runtime_enable(>dev);
+   pm_runtime_get_sync(>dev);
+
config = of_device_get_match_data(>dev);
if (config) {
pd->clks_per_count = config->clks_per_count;
@@ -888,6 +898,8 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
} else {
ret = sh_mobile_i2c_init(pd);
}
+
+   pm_runtime_put_sync(>dev);
if (ret)
return ret;
 
@@ -896,19 +908,6 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
pd->dma_direction = DMA_NONE;
pd->dma_rx = pd->dma_tx = ERR_PTR(-EPROBE_DEFER);
 
-   /* Enable Runtime PM for this device.
-*
-* Also tell the Runtime PM core to ignore children
-* for this device since it is valid for us to suspend
-* this I2C master driver even though the slave devices
-* on the I2C bus may not be suspended.
-*
-* The state of the I2C hardware bus is unaffected by
-* the Runtime PM state.
-*/
-   pm_suspend_ignore_children(>dev, true);
-   pm_runtime_enable(>dev);
-
/* setup the private data */
adap = >adap;
i2c_set_adapdata(adap, pd);
-- 
2.11.0

[RFC PATCH 1/7] i2c: sh_mobile: move type detection upwards

[Wolfram Sang]

For refactoring reasons, we will need this information before the setup
callback. Also, simplify the comment to a oneliner.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index b01607b4fce2b8..1ac896e46b390c 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -873,6 +873,10 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
pd->bus_speed = ret ? STANDARD_MODE : bus_speed;
pd->clks_per_count = 1;
 
+   /* Newer variants come with two new bits in ICIC */
+   if (resource_size(res) > 0x17)
+   pd->flags |= IIC_FLAG_HAS_ICIC67;
+
config = of_device_get_match_data(>dev);
if (config) {
pd->clks_per_count = config->clks_per_count;
@@ -881,12 +885,6 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
config->setup(pd);
}
 
-   /* The IIC blocks on SH-Mobile ARM processors
-* come with two new bits in ICIC.
-*/
-   if (resource_size(res) > 0x17)
-   pd->flags |= IIC_FLAG_HAS_ICIC67;
-
ret = sh_mobile_i2c_init(pd);
if (ret)
return ret;
-- 
2.11.0

[RFC PATCH 2/7] i2c: sh_mobile: allow setup callback to return errno

[Wolfram Sang]

The setup callback will be more generic and, thus, need to be able to
return error codes. Change the return type to 'int' for that.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index 1ac896e46b390c..a3a377a10fa789 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -149,7 +149,7 @@ struct sh_mobile_i2c_data {
 
 struct sh_mobile_dt_config {
int clks_per_count;
-   void (*setup)(struct sh_mobile_i2c_data *pd);
+   int (*setup)(struct sh_mobile_i2c_data *pd);
 };
 
 #define IIC_FLAG_HAS_ICIC67(1 << 0)
@@ -749,7 +749,7 @@ static const struct i2c_algorithm sh_mobile_i2c_algorithm = 
{
  * r8a7740 chip has lasting errata on I2C I/O pad reset.
  * this is work-around for it.
  */
-static void sh_mobile_i2c_r8a7740_workaround(struct sh_mobile_i2c_data *pd)
+static int sh_mobile_i2c_r8a7740_workaround(struct sh_mobile_i2c_data *pd)
 {
iic_set_clr(pd, ICCR, ICCR_ICE, 0);
iic_rd(pd, ICCR); /* dummy read */
@@ -770,6 +770,8 @@ static void sh_mobile_i2c_r8a7740_workaround(struct 
sh_mobile_i2c_data *pd)
udelay(10);
iic_wr(pd, ICCR, ICCR_TRS);
udelay(10);
+
+   return 0;
 }
 
 static const struct sh_mobile_dt_config default_dt_config = {
@@ -881,8 +883,11 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
if (config) {
pd->clks_per_count = config->clks_per_count;
 
-   if (config->setup)
-   config->setup(pd);
+   if (config->setup) {
+   ret = config->setup(pd);
+   if (ret)
+   return ret;
+   }
}
 
ret = sh_mobile_i2c_init(pd);
-- 
2.11.0

[RFC PATCH 3/7] i2c: sh_mobile: require setup callback

[Wolfram Sang]

Require the setup callback and move the frequency calculation into it.
This is in preparation for supporting multiple formulas.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index a3a377a10fa789..88af45225003a5 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -771,15 +771,17 @@ static int sh_mobile_i2c_r8a7740_workaround(struct 
sh_mobile_i2c_data *pd)
iic_wr(pd, ICCR, ICCR_TRS);
udelay(10);
 
-   return 0;
+   return sh_mobile_i2c_init(pd);
 }
 
 static const struct sh_mobile_dt_config default_dt_config = {
.clks_per_count = 1,
+   .setup = sh_mobile_i2c_init,
 };
 
 static const struct sh_mobile_dt_config fast_clock_dt_config = {
.clks_per_count = 2,
+   .setup = sh_mobile_i2c_init,
 };
 
 static const struct sh_mobile_dt_config r8a7740_dt_config = {
@@ -882,15 +884,10 @@ static int sh_mobile_i2c_probe(struct platform_device 
*dev)
config = of_device_get_match_data(>dev);
if (config) {
pd->clks_per_count = config->clks_per_count;
-
-   if (config->setup) {
-   ret = config->setup(pd);
-   if (ret)
-   return ret;
-   }
+   ret = config->setup(pd);
+   } else {
+   ret = sh_mobile_i2c_init(pd);
}
-
-   ret = sh_mobile_i2c_init(pd);
if (ret)
return ret;
 
-- 
2.11.0

[PATCH 0/2] ARM: dts: r8a7745: move timer node nodes out of bus

[Simon Horman]

Move the timer node, which have no reg property, out of the root node.
The nodes that have been moved do not have any register properties and thus
shouldn't be placed on the bus.

In preparation for the above sort nodes in the root node alphabetically.

Based on renesas-devel-20171218-v4.15-rc4

Simon Horman (2):
  ARM: dts: r8a7745: sort root sub-nodes alphabetically
  ARM: dts: r8a7745: move timer node out of bus

 arch/arm/boot/dts/r8a7745.dtsi | 64 --
 1 file changed, 30 insertions(+), 34 deletions(-)

-- 
2.11.0

[PATCH 2/2] ARM: dts: r8a7745: move timer node out of bus

[Simon Horman]

The timer node does not have any register properties and thus shouldn't be
placed on the bus.

This problem is flagged by the compiler as follows:
$ make dtbs W=1
...
arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dtb: Warning (simple_bus_reg): Node 
/soc/timer missing or empty reg/ranges property
arch/arm/boot/dts/r8a7745-iwg22d-sodimm-dbhd-ca.dtb: Warning (simple_bus_reg): 
Node /soc/timer missing or empty reg/ranges property
  DTC arch/arm/boot/dts/r8a7745-sk-rzg1e.dtb
arch/arm/boot/dts/r8a7745-sk-rzg1e.dtb: Warning (simple_bus_reg): Node 
/soc/timer missing or empty reg/ranges property

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7745.dtsi | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 65d92076bdde..612de9eeb7e2 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -247,18 +247,6 @@
resets = < 407>;
};
 
-   timer {
-   compatible = "arm,armv7-timer";
-   interrupts = ,
-,
-,
-;
-   };
-
cpg: clock-controller@e615 {
compatible = "renesas,r8a7745-cpg-mssr";
reg = <0 0xe615 0 0x1000>;
@@ -1143,6 +1131,14 @@
};
};
 
+   timer {
+   compatible = "arm,armv7-timer";
+   interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>;
+   };
+
/* External USB clock - can be overridden by the board */
usb_extal_clk: usb_extal {
compatible = "fixed-clock";
-- 
2.11.0

[PATCH 1/2] ARM: dts: r8a7745: sort root sub-nodes alphabetically

[Simon Horman]

Sort root sub-nodes alphabetically for allow for easier maintenance
of this file.

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7745.dtsi | 48 +-
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 0fa78612746f..65d92076bdde 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -35,6 +35,14 @@
vin1 = 
};
 
+   /* External CAN clock */
+   can_clk: can {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -67,6 +75,22 @@
};
};
 
+   /* External root clock */
+   extal_clk: extal {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
+   /* External SCIF clock */
+   scif_clk: scif {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
soc {
compatible = "simple-bus";
interrupt-parent = <>;
@@ -1119,34 +1143,10 @@
};
};
 
-   /* External root clock */
-   extal_clk: extal {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
/* External USB clock - can be overridden by the board */
usb_extal_clk: usb_extal {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <4800>;
};
-
-   /* External CAN clock */
-   can_clk: can {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
-   /* External SCIF clock */
-   scif_clk: scif {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
 };
-- 
2.11.0

[PATCH v2 1/2] ARM: dts: r8a7743: sort root sub-nodes alphabetically

[Simon Horman]

Sort root sub-nodes alphabetically for allow for easier maintenance
of this file.

Signed-off-by: Simon Horman 
---
v2
* Rebase
---
 arch/arm/boot/dts/r8a7743.dtsi | 100 ++---
 1 file changed, 54 insertions(+), 46 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index 59860c8ef362..d333a545956d 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -37,6 +37,29 @@
vin2 = 
};
 
+   /*
+* The external audio clocks are configured as 0 Hz fixed frequency
+* clocks by default.
+* Boards that provide audio clocks should override them.
+*/
+   audio_clk_a: audio_clk_a {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+
+   audio_clk_b: audio_clk_b {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+
+   audio_clk_c: audio_clk_c {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -79,6 +102,37 @@
};
};
 
+   /* External CAN clock */
+   can_clk: can {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
+   /* External root clock */
+   extal_clk: extal {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
+   /* External PCIe clock - can be overridden by the board */
+   pcie_bus_clk: pcie_bus {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+
+   /* External SCIF clock */
+   scif_clk: scif {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
soc {
compatible = "simple-bus";
interrupt-parent = <>;
@@ -1485,56 +1539,10 @@
clock-frequency = <0>;
};
 
-   /*
-* The external audio clocks are configured as 0 Hz fixed frequency
-* clocks by default.
-* Boards that provide audio clocks should override them.
-*/
-   audio_clk_a: audio_clk_a {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-
-   audio_clk_b: audio_clk_b {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-
-   audio_clk_c: audio_clk_c {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-
/* External USB clock - can be overridden by the board */
usb_extal_clk: usb_extal {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <4800>;
};
-
-   /* External CAN clock */
-   can_clk: can {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
-   /* External PCIe clock - can be overridden by the board */
-   pcie_bus_clk: pcie_bus {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-
-   /* External SCIF clock */
-   scif_clk: scif {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
 };
-- 
2.11.0

[PATCH v2 0/2] ARM: dts: r8a7743: move timer nodes nodes out of bus

[Simon Horman]

Move the timer node, which have no reg property, out of the root node.
The nodes that have been moved do not have any register properties and thus
shouldn't be placed on the bus.

In preparation for the above sort nodes in the root node alphabetically.

Based renesas-devel-20171218-v4.15-rc4

Changes since v1:
* Rebase
* Avoid line-wrapping nodes

Simon Horman (2):
  ARM: dts: r8a7743: sort root sub-nodes alphabetically
  ARM: dts: r8a7743: move timer node out of bus

 arch/arm/boot/dts/r8a7743.dtsi | 116 +
 1 file changed, 60 insertions(+), 56 deletions(-)

-- 
2.11.0

[PATCH v2 2/2] ARM: dts: r8a7743: move timer node out of bus

[Simon Horman]

The timer node does not have any register properties and thus shouldn't be
placed on the bus.

This problem is flagged by the compiler as follows:
$ make
  DTC arch/arm/boot/dts/r8a7743-iwg20d-q7-dbcm-ca.dtb
arch/arm/boot/dts/r8a7743-iwg20d-q7.dtb: Warning (simple_bus_reg): Node 
/soc/timer missing or empty reg/ranges property
arch/arm/boot/dts/r8a7743-iwg20d-q7-dbcm-ca.dtb: Warning (simple_bus_reg): Node 
/soc/timer missing or empty reg/ranges property
  DTC arch/arm/boot/dts/r8a7743-sk-rzg1m.dtb
arch/arm/boot/dts/r8a7743-sk-rzg1m.dtb: Warning (simple_bus_reg): Node 
/soc/timer missing or empty reg/ranges property

Signed-off-by: Simon Horman 
---
v2
* Avoid line-wrapping properties
---
 arch/arm/boot/dts/r8a7743.dtsi | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index d333a545956d..5ba0af76b29c 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -304,18 +304,6 @@
resets = < 407>;
};
 
-   timer {
-   compatible = "arm,armv7-timer";
-   interrupts = ,
-,
-,
-;
-   };
-
cpg: clock-controller@e615 {
compatible = "renesas,r8a7743-cpg-mssr";
reg = <0 0xe615 0 0x1000>;
@@ -1539,6 +1527,14 @@
clock-frequency = <0>;
};
 
+   timer {
+   compatible = "arm,armv7-timer";
+   interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>;
+   };
+
/* External USB clock - can be overridden by the board */
usb_extal_clk: usb_extal {
compatible = "fixed-clock";
-- 
2.11.0

[PATCH 2/2] ARM: dts: r8a7792: move timer node out of bus

[Simon Horman]

The timer node does not have any register properties and thus shouldn't be
placed on the bus.

This problem is flagged by the compiler as follows:
$ make dtbs W=1
...
  DTC arch/arm/boot/dts/r8a7792-wheat.dtb
arch/arm/boot/dts/r8a7792-blanche.dtb: Warning (simple_bus_reg): Node 
/soc/timer missing or empty reg/ranges property
arch/arm/boot/dts/r8a7792-wheat.dtb: Warning (simple_bus_reg): Node /soc/timer 
missing or empty reg/ranges property

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7792.dtsi | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7792.dtsi b/arch/arm/boot/dts/r8a7792.dtsi
index d31258958c36..5317e353569f 100644
--- a/arch/arm/boot/dts/r8a7792.dtsi
+++ b/arch/arm/boot/dts/r8a7792.dtsi
@@ -137,18 +137,6 @@
resets = < 407>;
};
 
-   timer {
-   compatible = "arm,armv7-timer";
-   interrupts = ,
-,
-,
-;
-   };
-
rst: reset-controller@e616 {
compatible = "renesas,r8a7792-rst";
reg = <0 0xe616 0 0x0100>;
@@ -857,4 +845,12 @@
#reset-cells = <1>;
};
};
+
+   timer {
+   compatible = "arm,armv7-timer";
+   interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>,
+ < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) 
| IRQ_TYPE_LEVEL_LOW)>;
+   };
 };
-- 
2.11.0

[PATCH 1/2] ARM: dts: r8a7792: sort root sub-nodes alphabetically

[Simon Horman]

Sort root sub-nodes alphabetically for allow for easier maintenance
of this file.

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7792.dtsi | 48 +-
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7792.dtsi b/arch/arm/boot/dts/r8a7792.dtsi
index ac05fdb91798..d31258958c36 100644
--- a/arch/arm/boot/dts/r8a7792.dtsi
+++ b/arch/arm/boot/dts/r8a7792.dtsi
@@ -36,6 +36,22 @@
vin5 = 
};
 
+   /* External root clock */
+   extal_clk: extal {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
+   /* External CAN clock */
+   can_clk: can {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -69,6 +85,14 @@
};
};
 
+   /* External SCIF clock */
+   scif_clk: scif {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
soc {
compatible = "simple-bus";
interrupt-parent = <>;
@@ -833,28 +857,4 @@
#reset-cells = <1>;
};
};
-
-   /* External root clock */
-   extal_clk: extal {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
-   /* External SCIF clock */
-   scif_clk: scif {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
-   /* External CAN clock */
-   can_clk: can {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
 };
-- 
2.11.0

[PATCH 0/2] ARM: dts: r8a7792: Timer node nodes out of bus

[Simon Horman]

Move the timer node, which have no reg property, out of the root node.
The nodes that have been moved do not have any register properties and thus
shouldn't be placed on the bus.

In preparation for the above sort nodes in the root node alphabetically.

Based on renesas-devel-20171218-v4.15-rc4

Simon Horman (2):
  ARM: dts: r8a7792: sort root sub-nodes alphabetically
  ARM: dts: r8a7792: move timer node out of bus

 arch/arm/boot/dts/r8a7792.dtsi | 64 --
 1 file changed, 30 insertions(+), 34 deletions(-)

-- 
2.11.0

Re: [PATCH 3/8] media: v4l2-async: simplify v4l2_async_subdev structure

[Benoit Parrot]

For am437x/am437x-vpfe.c & ti-vpe/cal.c

Acked-by: Benoit Parrot 

Mauro Carvalho Chehab  wrote on Mon [2017-Dec-18 
17:53:57 -0200]:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.
> 
> At drivers, this makes even clearer about the match criteria.
> 
> Acked-by: Sylwester Nawrocki 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c|  6 +++---
>  drivers/media/platform/atmel/atmel-isc.c   |  2 +-
>  drivers/media/platform/atmel/atmel-isi.c   |  2 +-
>  drivers/media/platform/davinci/vpif_capture.c  |  4 ++--
>  drivers/media/platform/exynos4-is/media-dev.c  |  4 ++--
>  drivers/media/platform/pxa_camera.c|  2 +-
>  drivers/media/platform/qcom/camss-8x16/camss.c |  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c|  2 +-
>  drivers/media/platform/rcar_drif.c |  4 ++--
>  drivers/media/platform/soc_camera/soc_camera.c |  2 +-
>  drivers/media/platform/stm32/stm32-dcmi.c  |  2 +-
>  drivers/media/platform/ti-vpe/cal.c|  2 +-
>  drivers/media/platform/xilinx/xilinx-vipp.c|  2 +-
>  drivers/media/v4l2-core/v4l2-async.c   | 16 
>  drivers/media/v4l2-core/v4l2-fwnode.c  | 10 +-
>  drivers/staging/media/imx/imx-media-dev.c  |  4 ++--
>  include/media/v4l2-async.h |  8 ++--
>  17 files changed, 35 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index 0997c640191d..601ae6487617 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
>   vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
>  
>   for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
> - if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
> - asd[i].match.fwnode.fwnode) {
> + if (vpfe->cfg->asd[i]->match.fwnode ==
> + asd[i].match.fwnode) {
>   sdinfo = >cfg->sub_devs[i];
>   vpfe->sd[i] = subdev;
>   vpfe->sd[i]->grp_id = sdinfo->grp_id;
> @@ -2510,7 +2510,7 @@ vpfe_get_pdata(struct platform_device *pdev)
>   }
>  
>   pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
> + pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
>   of_node_put(rem);
>   }
>  
> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
> b/drivers/media/platform/atmel/atmel-isc.c
> index 0c2635647f69..34676409ca08 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -2088,7 +2088,7 @@ static int isc_parse_dt(struct device *dev, struct 
> isc_device *isc)
>   subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
>  
>   subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> - subdev_entity->asd->match.fwnode.fwnode =
> + subdev_entity->asd->match.fwnode =
>   of_fwnode_handle(rem);
>   list_add_tail(_entity->list, >subdev_entities);
>   }
> diff --git a/drivers/media/platform/atmel/atmel-isi.c 
> b/drivers/media/platform/atmel/atmel-isi.c
> index e900995143a3..9958918e2449 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1128,7 +1128,7 @@ static int isi_graph_parse(struct atmel_isi *isi, 
> struct device_node *node)
>   /* Remote node to connect */
>   isi->entity.node = remote;
>   isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
> + isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
>   return 0;
>   }
>  }
> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
> b/drivers/media/platform/davinci/vpif_capture.c
> index e45916f69def..e1c273c8b9a6 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier 
> *notifier,
>  
>   for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
>   struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> - const struct fwnode_handle *fwnode = 

[PATCH 3/8] media: v4l2-async: simplify v4l2_async_subdev structure

[Mauro Carvalho Chehab]

The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
match criteria requires just a device name.

So, it doesn't make sense to enclose those into structs,
as the criteria can go directly into the union.

That makes easier to document it, as we don't need to document
weird senseless structs.

At drivers, this makes even clearer about the match criteria.

Acked-by: Sylwester Nawrocki 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/am437x/am437x-vpfe.c|  6 +++---
 drivers/media/platform/atmel/atmel-isc.c   |  2 +-
 drivers/media/platform/atmel/atmel-isi.c   |  2 +-
 drivers/media/platform/davinci/vpif_capture.c  |  4 ++--
 drivers/media/platform/exynos4-is/media-dev.c  |  4 ++--
 drivers/media/platform/pxa_camera.c|  2 +-
 drivers/media/platform/qcom/camss-8x16/camss.c |  2 +-
 drivers/media/platform/rcar-vin/rcar-core.c|  2 +-
 drivers/media/platform/rcar_drif.c |  4 ++--
 drivers/media/platform/soc_camera/soc_camera.c |  2 +-
 drivers/media/platform/stm32/stm32-dcmi.c  |  2 +-
 drivers/media/platform/ti-vpe/cal.c|  2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c|  2 +-
 drivers/media/v4l2-core/v4l2-async.c   | 16 
 drivers/media/v4l2-core/v4l2-fwnode.c  | 10 +-
 drivers/staging/media/imx/imx-media-dev.c  |  4 ++--
 include/media/v4l2-async.h |  8 ++--
 17 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index 0997c640191d..601ae6487617 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
 
for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
-   if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
-   asd[i].match.fwnode.fwnode) {
+   if (vpfe->cfg->asd[i]->match.fwnode ==
+   asd[i].match.fwnode) {
sdinfo = >cfg->sub_devs[i];
vpfe->sd[i] = subdev;
vpfe->sd[i]->grp_id = sdinfo->grp_id;
@@ -2510,7 +2510,7 @@ vpfe_get_pdata(struct platform_device *pdev)
}
 
pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
-   pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
+   pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
of_node_put(rem);
}
 
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 0c2635647f69..34676409ca08 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2088,7 +2088,7 @@ static int isc_parse_dt(struct device *dev, struct 
isc_device *isc)
subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
 
subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-   subdev_entity->asd->match.fwnode.fwnode =
+   subdev_entity->asd->match.fwnode =
of_fwnode_handle(rem);
list_add_tail(_entity->list, >subdev_entities);
}
diff --git a/drivers/media/platform/atmel/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
index e900995143a3..9958918e2449 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1128,7 +1128,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct 
device_node *node)
/* Remote node to connect */
isi->entity.node = remote;
isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
-   isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
+   isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
return 0;
}
 }
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index e45916f69def..e1c273c8b9a6 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
 
for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
-   const struct fwnode_handle *fwnode = _asd->match.fwnode.fwnode;
+   const struct fwnode_handle *fwnode = _asd->match.fwnode;
 
if (fwnode == subdev->fwnode) {
vpif_obj.sd[i] = subdev;
@@ -1595,7 +1595,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
}
 

[PATCH 5/8] media: v4l2-mediabus: convert flags to enums and document them

[Mauro Carvalho Chehab]

There is a mess with media bus flags: there are two sets of
flags, one used by parallel and ITU-R BT.656 outputs,
and another one for CSI2.

Depending on the type, the same bit has different meanings.

That's very confusing, and counter-intuitive. So, split them
into two sets of flags, inside an enum.

This way, it becomes clearer that there are two separate sets
of flags. It also makes easier if CSI1, CSP, CSI3, etc. would
need a different set of flags.

As a side effect, enums can be documented via kernel-docs,
so there will be an improvement at flags documentation.

Unfortunately, soc_camera and pxa_camera do a mess with
the flags, using either one set of flags without proper
checks about the type. That could be fixed, but, as both drivers
are obsolete and in the process of cleanings, I opted to just
keep the behavior, using an unsigned int inside those two
drivers.

Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/adv7180.c|  10 +-
 drivers/media/i2c/ml86v7667.c  |   5 +-
 drivers/media/i2c/mt9m111.c|   8 +-
 drivers/media/i2c/ov6650.c |  19 +--
 drivers/media/i2c/soc_camera/imx074.c  |   6 +-
 drivers/media/i2c/soc_camera/mt9m001.c |  10 +-
 drivers/media/i2c/soc_camera/mt9t031.c |  11 +-
 drivers/media/i2c/soc_camera/mt9t112.c |  11 +-
 drivers/media/i2c/soc_camera/mt9v022.c |  16 ++-
 drivers/media/i2c/soc_camera/ov5642.c  |   5 +-
 drivers/media/i2c/soc_camera/ov772x.c  |  10 +-
 drivers/media/i2c/soc_camera/ov9640.c  |  10 +-
 drivers/media/i2c/soc_camera/ov9740.c  |  10 +-
 drivers/media/i2c/soc_camera/rj54n1cb0c.c  |  12 +-
 drivers/media/i2c/soc_camera/tw9910.c  |  13 +-
 drivers/media/i2c/tc358743.c   |  10 +-
 drivers/media/i2c/tvp5150.c|   6 +-
 drivers/media/platform/pxa_camera.c|   8 +-
 drivers/media/platform/rcar-vin/rcar-core.c|   4 +-
 drivers/media/platform/rcar-vin/rcar-dma.c |   4 +-
 .../platform/soc_camera/sh_mobile_ceu_camera.c |   2 +-
 drivers/media/platform/soc_camera/soc_camera.c |   3 +-
 .../platform/soc_camera/soc_camera_platform.c  |   2 +-
 drivers/media/platform/soc_camera/soc_mediabus.c   |   2 +-
 drivers/media/v4l2-core/v4l2-fwnode.c  |   5 +-
 include/media/v4l2-fwnode.h|   4 +-
 include/media/v4l2-mediabus.h  | 145 ++---
 27 files changed, 218 insertions(+), 133 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..4bf25a72ef4f 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -743,16 +743,16 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
 
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
cfg->type = V4L2_MBUS_CSI2;
-   cfg->flags = V4L2_MBUS_CSI2_1_LANE |
-   V4L2_MBUS_CSI2_CHANNEL_0 |
-   V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   cfg->csi2_flags = V4L2_MBUS_CSI2_1_LANE
+ | V4L2_MBUS_CSI2_CHANNEL_0
+ | V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
} else {
/*
 * The ADV7180 sensor supports BT.601/656 output modes.
 * The BT.656 is default and not yet configurable by s/w.
 */
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-V4L2_MBUS_DATA_ACTIVE_HIGH;
+   cfg->pb_flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
+   | V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_BT656;
}
 
diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
index 57ef901edb06..a25114d0c31f 100644
--- a/drivers/media/i2c/ml86v7667.c
+++ b/drivers/media/i2c/ml86v7667.c
@@ -226,8 +226,9 @@ static int ml86v7667_fill_fmt(struct v4l2_subdev *sd,
 static int ml86v7667_g_mbus_config(struct v4l2_subdev *sd,
   struct v4l2_mbus_config *cfg)
 {
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-V4L2_MBUS_DATA_ACTIVE_HIGH;
+   cfg->pb_flags = V4L2_MBUS_MASTER
+   | V4L2_MBUS_PCLK_SAMPLE_RISING
+   | V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_BT656;
 
return 0;
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index b1665d97e0fd..d9698b535080 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -857,9 +857,11 @@ static int mt9m111_enum_mbus_code(struct v4l2_subdev *sd,
 static int 

Re: [PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

[Mauro Carvalho Chehab]

Em Sat, 30 Sep 2017 01:05:24 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> (Removing the non-list recipients.)
> 
> On Fri, Sep 29, 2017 at 06:27:13AM -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 28 Sep 2017 15:09:21 +0300
> > Sakari Ailus  escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:  
> > > > The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> > > > struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> > > > match criteria requires just a device name.
> > > > 
> > > > So, it doesn't make sense to enclose those into structs,
> > > > as the criteria can go directly into the union.
> > > > 
> > > > That makes easier to document it, as we don't need to document
> > > > weird senseless structs.
> > > 
> > > The idea is that in the union, there's a struct which is specific to the
> > > match_type field. I wouldn't call it senseless.  
> > 
> > Why a struct for each specific match_type is **needed**? It it is not
> > needed, then it is senseless per definition :-) 
> > 
> > In the specific case of fwnode, there's already a named struct
> > for fwnode_handle. The only thing is that it is declared outside
> > enum v4l2_async_match_type. So, I don't see any reason to do things
> > like:
> > 
> > struct {
> > struct fwnode_handle *fwnode;
> > } fwnode;
> > 
> > If you're in doubt about that, think on how would you document
> > both fwnode structs. Both fwnode structs specify the match
> > criteria if %V4L2_ASYNC_MATCH_FWNODE.
> > 
> > The same applies to this:
> > 
> > struct {
> > const char *name;
> > } device_name;
> > 
> > Both device_name and name specifies the match criteria if
> > %V4L2_ASYNC_MATCH_DEVNAME.
> >   
> > > 
> > > In the two cases there's just a single field in the containing struct. You
> > > could remove the struct in that case as you do in this patch, and just use
> > > the field. But I think the result is less clean and so I wouldn't make 
> > > this
> > > change.  
> > 
> > It is actually cleaner without the stucts.
> > 
> > Without the useless struct, if one wants to match a firmware node, it
> > should be doing:
> > 
> > pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
> > pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);  
> 
> This code should be and will be moved out of drivers. See:
> 
> 
> 
> So there are going to be quite a bit fewer instances of it, and none should
> remain in drivers. I frankly don't have a strong opinion on this; there are
> arguments for and against. I just don't see a reason to change it.

There are still a few occurrences on drivers. Just rebased it.
I'll post it in a few, inside a new patch series.

Simplifying the name of the match rules makes easier to understand
what's going on.


Thanks,
Mauro

Re: [PATCH 04/24] media: v4l2-mediabus: convert flags to enums and document them

[Mauro Carvalho Chehab]

Em Wed, 11 Oct 2017 23:26:44 +0200
Pavel Machek  escreveu:

> On Mon 2017-10-09 07:19:10, Mauro Carvalho Chehab wrote:
> > There is a mess with media bus flags: there are two sets of
> > flags, one used by parallel and ITU-R BT.656 outputs,
> > and another one for CSI2.
> > 
> > Depending on the type, the same bit has different meanings.
> >   
> 
> > @@ -86,11 +125,22 @@ enum v4l2_mbus_type {
> >  /**
> >   * struct v4l2_mbus_config - media bus configuration
> >   * @type:  in: interface type
> > - * @flags: in / out: configuration flags, depending on @type
> > + * @pb_flags:  in / out: configuration flags, if @type is
> > + * %V4L2_MBUS_PARALLEL or %V4L2_MBUS_BT656.
> > + * @csi2_flags:in / out: configuration flags, if @type is
> > + * %V4L2_MBUS_CSI2.
> > + * @flag:  access flags, no matter the @type.
> > + * Used just to avoid needing to rewrite the logic inside
> > + * soc_camera and pxa_camera drivers. Don't use on newer
> > + * drivers!
> >   */
> >  struct v4l2_mbus_config {
> > enum v4l2_mbus_type type;
> > -   unsigned int flags;
> > +   union {
> > +   enum v4l2_mbus_parallel_and_bt656_flags pb_flags;
> > +   enum v4l2_mbus_csi2_flags   csi2_flags;
> > +   unsigned intflag;
> > +   };
> >  };
> >  
> >  static inline void v4l2_fill_pix_format(struct v4l2_pix_format
> >  *pix_fmt,  
> 
> The flags->flag conversion is quite subtle, and "flag" is confusing
> because there is more than one inside. What about something like
> __legacy_flags?

Good idea.

> 
>   Pavel

Thanks,
Mauro

[PATCH] media: v4l2-mediabus: convert flags to enums and document them

There is a mess with media bus flags: there are two sets of
flags, one used by parallel and ITU-R BT.656 outputs,
and another one for CSI2.

Depending on the type, the same bit has different meanings.

That's very confusing, and counter-intuitive. So, split them
into two sets of flags, inside an enum.

This way, it becomes clearer that there are two separate sets
of flags. It also makes easier if CSI1, CSP, CSI3, etc. would
need a different set of flags.

As a side effect, enums can be documented via kernel-docs,
so there will be an improvement at flags documentation.

Unfortunately, soc_camera and pxa_camera do a mess with
the flags, using either one set of flags without proper
checks about the type. That could be fixed, but, as both drivers
are obsolete and in the process of cleanings, I opted to just
keep the behavior, using an unsigned int inside those two
drivers.

Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..4bf25a72ef4f 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -743,16 +743,16 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
 
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
cfg->type = V4L2_MBUS_CSI2;
-   cfg->flags = V4L2_MBUS_CSI2_1_LANE |
-   V4L2_MBUS_CSI2_CHANNEL_0 |
-   V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   cfg->csi2_flags = V4L2_MBUS_CSI2_1_LANE
+ | V4L2_MBUS_CSI2_CHANNEL_0
+ | V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
} else {
/*
 * The ADV7180 sensor supports BT.601/656 output modes.
 * The BT.656 is default and not yet configurable by s/w.
 */
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-V4L2_MBUS_DATA_ACTIVE_HIGH;
+   cfg->pb_flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
+   | V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_BT656;
}
 
diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
index 57ef901edb06..a25114d0c31f 100644
--- a/drivers/media/i2c/ml86v7667.c
+++ b/drivers/media/i2c/ml86v7667.c
@@ -226,8 +226,9 @@ static int ml86v7667_fill_fmt(struct v4l2_subdev *sd,
 static int ml86v7667_g_mbus_config(struct v4l2_subdev *sd,
   struct v4l2_mbus_config *cfg)
 {
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-V4L2_MBUS_DATA_ACTIVE_HIGH;
+   cfg->pb_flags = V4L2_MBUS_MASTER
+   | V4L2_MBUS_PCLK_SAMPLE_RISING
+   | V4L2_MBUS_DATA_ACTIVE_HIGH;
cfg->type = V4L2_MBUS_BT656;
 
return 0;
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index b1665d97e0fd..d9698b535080 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -857,9 +857,11 @@ static int 

[PATCH v2 5/5] ARM: dts: iwg20d-q7-common: Sound DMA support via DVC on DTS

[Biju Das]

DMA transfer uses DVC

 DMA   DMApp
[MEM] -> [SRC] -> [DVC] -> [SSIU] -> [SSI]

 DMA   DMApp
[MEM] <- [DVC] <- [SRC] <- [SSIU] <- [SSI]

Signed-off-by: Biju Das 
Reviewed-by: Fabrizio Castro 
Acked-by: Kuninori Morimoto 
---
v1->v2
 * No change

 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi 
b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index 7b283e0..5c604c74 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -8,6 +8,29 @@
  * kind, whether express or implied.
  */
 
+/*
+ * SSI-SGTL5000
+ *
+ * This command is required when Playback/Capture
+ *
+ *  amixer set "DVC Out" 100%
+ *  amixer set "DVC In" 100%
+ *
+ * You can use Mute
+ *
+ *  amixer set "DVC Out Mute" on
+ *  amixer set "DVC In Mute" on
+ *
+ * You can use Volume Ramp
+ *
+ *  amixer set "DVC Out Ramp Up Rate"   "0.125 dB/64 steps"
+ *  amixer set "DVC Out Ramp Down Rate" "0.125 dB/512 steps"
+ *  amixer set "DVC Out Ramp" on
+ *  aplay xxx.wav &
+ *  amixer set "DVC Out"  80%  // Volume Down
+ *  amixer set "DVC Out" 100%  // Volume Up
+ */
+
 / {
aliases {
serial0 = 
@@ -208,8 +231,8 @@
 
rcar_sound,dai {
dai0 {
-   playback = < >;
-   capture = < >;
+   playback = <  >;
+   capture = <  >;
};
};
 };
-- 
1.9.1

[PATCH v2 4/5] ARM: dts: iwg20d-q7-common: Sound DMA support via SRC on DTS

[Biju Das]

DMA transfer to/from SRC

 DMA  DMApp
[MEM] -> [SRC] -> [SSIU] -> [SSI]

 DMA  DMApp
[MEM] <- [SRC] <- [SSIU] <- [SSI]

Current sound driver is supporting SSI/SRC random connection.
So, this patch is trying
SSI1 -> SRC3
SSI0 <- SRC2

Signed-off-by: Biju Das 
Reviewed-by: Fabrizio Castro 
Acked-by: Kuninori Morimoto 
---
v1->v2
 * No change

 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi 
b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index 19467fc..7b283e0 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -208,8 +208,8 @@
 
rcar_sound,dai {
dai0 {
-   playback = <>;
-   capture = <>;
+   playback = < >;
+   capture = < >;
};
};
 };
-- 
1.9.1

[PATCH v2 3/5] ARM: dts: iwg20d-q7-common: Sound DMA support via BUSIF on DTS

[Biju Das]

DMA transfer to/from SSIU

 DMA
[MEM] -> [SSIU] -> [SSI]

 DMA
[MEM] <- [SSIU] <- [SSI]

Signed-off-by: Biju Das 
Reviewed-by: Fabrizio Castro 
Acked-by: Kuninori Morimoto 
---
v1->v2
 * No change

 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi 
b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index bc8d253..19467fc 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -242,12 +242,7 @@
status = "okay";
 };
 
- {
-   no-busif;
-};
-
  {
-   no-busif;
shared-pin;
 };
 
-- 
1.9.1

[PATCH v2 2/5] ARM: dts: iwg20d-q7-common: Sound DMA support on DTS

[Biju Das]

DMA transfer to/from SSI

 DMA
[MEM] -> [SSI]

 DMA
[MEM] <- [SSI]

Signed-off-by: Biju Das 
Reviewed-by: Fabrizio Castro 
Acked-by: Kuninori Morimoto 
---
v1->v2
  * No change

 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi 
b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index ed67201..bc8d253 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -243,11 +243,11 @@
 };
 
  {
-   pio-transfer;
+   no-busif;
 };
 
  {
-   pio-transfer;
+   no-busif;
shared-pin;
 };
 
-- 
1.9.1

[PATCH v2 0/5] Add sound support

[Biju Das]

This series aims to add sound support for iWave RZ/G1M board.

This patch series has below dependency
1)https://patchwork.kernel.org/patch/10108041/

Biju Das (5):
  ARM: dts: iwg20d-q7-common: Sound PIO support
  ARM: dts: iwg20d-q7-common: Sound DMA support on DTS
  ARM: dts: iwg20d-q7-common: Sound DMA support via BUSIF on DTS
  ARM: dts: iwg20d-q7-common: Sound DMA support via SRC on DTS
  ARM: dts: iwg20d-q7-common: Sound DMA support via DVC on DTS

 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 64 +
 1 file changed, 64 insertions(+)

-- 
1.9.1

[PATCH v2 1/5] ARM: dts: iwg20d-q7-common: Sound PIO support

[Biju Das]

Enable sound PIO support on carrier board.

Signed-off-by: Biju Das 
Reviewed-by: Fabrizio Castro 
Acked-by: Kuninori Morimoto 
---
v1-->v2
 * Reworked sorting

 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 46 +
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi 
b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index 2070b14..ed67201 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -34,6 +34,22 @@
regulator-always-on;
};
 
+   rsnd_sgtl5000: sound {
+   compatible = "simple-audio-card";
+
+   simple-audio-card,format = "i2s";
+   simple-audio-card,bitclock-master = <>;
+   simple-audio-card,frame-master = <>;
+
+   sndcpu: simple-audio-card,cpu {
+   sound-dai = <_sound>;
+   };
+
+   sndcodec: simple-audio-card,codec {
+   sound-dai = <>;
+   };
+   };
+
vcc_sdhi1: regulator-vcc-sdhi1 {
compatible = "regulator-fixed";
 
@@ -166,6 +182,11 @@
power-source = <1800>;
};
 
+   sound_pins: sound {
+   groups = "ssi0129_ctrl", "ssi0_data", "ssi1_data";
+   function = "ssi";
+   };
+
usb0_pins: usb0 {
groups = "usb0";
function = "usb0";
@@ -177,6 +198,22 @@
};
 };
 
+_sound {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+   status = "okay";
+
+   /* Single DAI */
+   #sound-dai-cells = <0>;
+
+   rcar_sound,dai {
+   dai0 {
+   playback = <>;
+   capture = <>;
+   };
+   };
+};
+
  {
pinctrl-0 = <_pins>;
pinctrl-names = "default";
@@ -205,6 +242,15 @@
status = "okay";
 };
 
+ {
+   pio-transfer;
+};
+
+ {
+   pio-transfer;
+   shared-pin;
+};
+
  {
status = "okay";
 };
-- 
1.9.1

[PATCH v2 0/2] R-Car D3 (r8a77995) CAN support

[Ulrich Hecht]

Hi!

These two patches are the leftovers of the "CAN for D3" series; as far
as I can tell, everything else has been picked up.

This revision fixes a copy/paste error and adds Acks and Reviewed-Bys.

CU
Uli


Ulrich Hecht (2):
  can: rcar_can: document r8a77995 (R-Car D3) compatibility strings
  can: rcar_canfd: document r8a77995 (R-Car D3) compatibility strings

 Documentation/devicetree/bindings/net/can/rcar_can.txt   | 13 +++--
 Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 13 +++--
 2 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH v2 1/2] can: rcar_can: document r8a77995 (R-Car D3) compatibility strings

[Ulrich Hecht]

Signed-off-by: Ulrich Hecht 
Acked-by: Rob Herring 
Reviewed-by: Geert Uytterhoeven 
---
 Documentation/devicetree/bindings/net/can/rcar_can.txt | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt 
b/Documentation/devicetree/bindings/net/can/rcar_can.txt
index 06bb7cc..f7d0358 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
@@ -11,6 +11,7 @@ Required properties:
  "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
  "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
  "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
+ "renesas,can-r8a77995" if CAN controller is a part of R8A77995 
SoC.
  "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible 
device.
  "renesas,rcar-gen2-can" for a generic R-Car Gen2 compatible 
device.
  "renesas,rcar-gen3-can" for a generic R-Car Gen3 compatible 
device.
@@ -25,12 +26,12 @@ Required properties:
 - pinctrl-0: pin control group to be used for this controller.
 - pinctrl-names: must be "default".
 
-Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
-compatible:
-In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
-and can be used by both CAN and CAN FD controller at the same time. It needs to
-be scaled to maximum frequency if any of these controllers use it. This is done
-using the below properties:
+Required properties for "renesas,can-r8a7795", "renesas,can-r8a7796" and
+"renesas,can-r8a77995" compatible:
+In R8A7795, R8A7796 and R8A77995 SoCs, "clkp2" can be CANFD clock. This is a
+div6 clock and can be used by both CAN and CAN FD controller at the same
+time. It needs to be scaled to maximum frequency if any of these
+controllers use it. This is done using the below properties:
 
 - assigned-clocks: phandle of clkp2(CANFD) clock.
 - assigned-clock-rates: maximum frequency of this clock.
-- 
2.7.4

RE: [PATCH] pinctrl: sh-pfc: r8a7794: Add i2c5 pin groups and function

[Fabrizio Castro]

Dear All,

this patch depends on the following patches:

pinctrl: sh-pfc: r8a7794: Add PWM[0123456] support
pinctrl: sh-pfc: r8a7794: Add tpu groups and function

Thanks,
Fab

> Subject: [PATCH] pinctrl: sh-pfc: r8a7794: Add i2c5 pin groups and function
>
> Add i2c5 pin groups and function to r8a7745 PFC driver.
>
> Signed-off-by: Biju Das <biju@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com>
> ---
> This patch is based on renesas devel tag renesas-devel-20171218-v4.15-rc4
>
>  drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 41 
> 
>  1 file changed, 41 insertions(+)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> index f133b4f..1640024 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> @@ -2228,6 +2228,35 @@ enum {
>  static const unsigned int i2c4_e_mux[] = {
>  I2C4_SCL_E_MARK, I2C4_SDA_E_MARK,
>  };
> +/* - I2C5 
> --- */
> +static const unsigned int i2c5_pins[] = {
> +/* SCL, SDA */
> +RCAR_GP_PIN(4, 14), RCAR_GP_PIN(4, 15),
> +};
> +static const unsigned int i2c5_mux[] = {
> +I2C5_SCL_MARK, I2C5_SDA_MARK,
> +};
> +static const unsigned int i2c5_b_pins[] = {
> +/* SCL, SDA */
> +RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 15),
> +};
> +static const unsigned int i2c5_b_mux[] = {
> +I2C5_SCL_B_MARK, I2C5_SDA_B_MARK,
> +};
> +static const unsigned int i2c5_c_pins[] = {
> +/* SCL, SDA */
> +RCAR_GP_PIN(4, 31), RCAR_GP_PIN(4, 30),
> +};
> +static const unsigned int i2c5_c_mux[] = {
> +I2C5_SCL_C_MARK, I2C5_SDA_C_MARK,
> +};
> +static const unsigned int i2c5_d_pins[] = {
> +/* SCL, SDA */
> +RCAR_GP_PIN(3, 13), RCAR_GP_PIN(3, 14),
> +};
> +static const unsigned int i2c5_d_mux[] = {
> +I2C5_SCL_D_MARK, I2C5_SDA_D_MARK,
> +};
>  /* - INTC 
> --- */
>  static const unsigned int intc_irq0_pins[] = {
>  /* IRQ0 */
> @@ -3821,6 +3850,10 @@ enum {
>  SH_PFC_PIN_GROUP(i2c4_c),
>  SH_PFC_PIN_GROUP(i2c4_d),
>  SH_PFC_PIN_GROUP(i2c4_e),
> +SH_PFC_PIN_GROUP(i2c5),
> +SH_PFC_PIN_GROUP(i2c5_b),
> +SH_PFC_PIN_GROUP(i2c5_c),
> +SH_PFC_PIN_GROUP(i2c5_d),
>  SH_PFC_PIN_GROUP(intc_irq0),
>  SH_PFC_PIN_GROUP(intc_irq1),
>  SH_PFC_PIN_GROUP(intc_irq2),
> @@ -4196,6 +4229,13 @@ enum {
>  "i2c4_e",
>  };
>
> +static const char * const i2c5_groups[] = {
> +"i2c5",
> +"i2c5_b",
> +"i2c5_c",
> +"i2c5_d",
> +};
> +
>  static const char * const intc_groups[] = {
>  "intc_irq0",
>  "intc_irq1",
> @@ -4537,6 +4577,7 @@ enum {
>  SH_PFC_FUNCTION(i2c2),
>  SH_PFC_FUNCTION(i2c3),
>  SH_PFC_FUNCTION(i2c4),
> +SH_PFC_FUNCTION(i2c5),
>  SH_PFC_FUNCTION(intc),
>  SH_PFC_FUNCTION(mmc),
>  SH_PFC_FUNCTION(msiof0),
> --
> 1.9.1



[https://www2.renesas.eu/media/email/unicef_2017.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here<https://www.unicef.org/> to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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

[PATCH v2 4/4] ARM: dts: r8a7745: Add TPU support

[Fabrizio Castro]

Add TPU support to SoC DT.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2:
* No change

 arch/arm/boot/dts/r8a7745.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index e68e20f..be316ca 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -1045,6 +1045,16 @@
status = "disabled";
};
 
+   tpu: pwm@e60f {
+   compatible = "renesas,tpu-r8a7745", "renesas,tpu";
+   reg = <0 0xe60f 0 0x148>;
+   clocks = < CPG_MOD 304>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 304>;
+   #pwm-cells = <3>;
+   status = "disabled";
+   };
+
sdhi0: sd@ee10 {
compatible = "renesas,sdhi-r8a7745",
 "renesas,rcar-gen2-sdhi";
-- 
2.7.4

[PATCH v2 0/4] Add PWM and TPU support to r8a7745

[Fabrizio Castro]

Dear All,

this series includes all that is required to add PWM and TPU SoC specific
support for the r8a7745.

Thanks,

Fabrizio Castro (4):
  pinctrl: sh-pfc: r8a7794: Add PWM[0123456] support
  ARM: dts: r8a7745: Add PWM SoC support
  pinctrl: sh-pfc: r8a7794: Add tpu groups and function
  ARM: dts: r8a7745: Add TPU support

 arch/arm/boot/dts/r8a7745.dtsi   |  87 
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 266 +++
 2 files changed, 353 insertions(+)

-- 
2.7.4

[PATCH v2 2/4] ARM: dts: r8a7745: Add PWM SoC support

[Fabrizio Castro]

Add the definitions for pwm[0123456] to the SoC .dtsi.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2:
* No change

 arch/arm/boot/dts/r8a7745.dtsi | 77 ++
 1 file changed, 77 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 0b67ee5..e68e20f 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -968,6 +968,83 @@
status = "disabled";
};
 
+   pwm0: pwm@e6e3 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e3 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm1: pwm@e6e31000 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e31000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm2: pwm@e6e32000 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e32000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm3: pwm@e6e33000 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e33000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm4: pwm@e6e34000 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e34000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm5: pwm@e6e35000 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e35000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm6: pwm@e6e36000 {
+   compatible = "renesas,pwm-r8a7745",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e36000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
sdhi0: sd@ee10 {
compatible = "renesas,sdhi-r8a7745",
 "renesas,rcar-gen2-sdhi";
-- 
2.7.4

[PATCH v2 3/4] pinctrl: sh-pfc: r8a7794: Add tpu groups and function

[Fabrizio Castro]

This patch adds tpu groups and function to r8a7745/r8a7794.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
---
v1->v2:
* Reworked sorting

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

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
index dccc20a..f133b4f 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
@@ -3493,6 +3493,79 @@ static const unsigned int ssi9_ctrl_b_pins[] = {
 static const unsigned int ssi9_ctrl_b_mux[] = {
SSI_SCK9_B_MARK, SSI_WS9_B_MARK,
 };
+/* - TPU  
*/
+static const unsigned int tpu_to0_pins[] = {
+   RCAR_GP_PIN(3, 31),
+};
+static const unsigned int tpu_to0_mux[] = {
+   TPUTO0_MARK,
+};
+static const unsigned int tpu_to0_b_pins[] = {
+   RCAR_GP_PIN(3, 30),
+};
+static const unsigned int tpu_to0_b_mux[] = {
+   TPUTO0_B_MARK,
+};
+static const unsigned int tpu_to0_c_pins[] = {
+   RCAR_GP_PIN(1, 18),
+};
+static const unsigned int tpu_to0_c_mux[] = {
+   TPUTO0_C_MARK,
+};
+static const unsigned int tpu_to1_pins[] = {
+   RCAR_GP_PIN(4, 9),
+};
+static const unsigned int tpu_to1_mux[] = {
+   TPUTO1_MARK,
+};
+static const unsigned int tpu_to1_b_pins[] = {
+   RCAR_GP_PIN(4, 0),
+};
+static const unsigned int tpu_to1_b_mux[] = {
+   TPUTO1_B_MARK,
+};
+static const unsigned int tpu_to1_c_pins[] = {
+   RCAR_GP_PIN(4, 4),
+};
+static const unsigned int tpu_to1_c_mux[] = {
+   TPUTO1_C_MARK,
+};
+static const unsigned int tpu_to2_pins[] = {
+   RCAR_GP_PIN(1, 3),
+};
+static const unsigned int tpu_to2_mux[] = {
+   TPUTO2_MARK,
+};
+static const unsigned int tpu_to2_b_pins[] = {
+   RCAR_GP_PIN(1, 0),
+};
+static const unsigned int tpu_to2_b_mux[] = {
+   TPUTO2_B_MARK,
+};
+static const unsigned int tpu_to2_c_pins[] = {
+   RCAR_GP_PIN(0, 22),
+};
+static const unsigned int tpu_to2_c_mux[] = {
+   TPUTO2_C_MARK,
+};
+static const unsigned int tpu_to3_pins[] = {
+   RCAR_GP_PIN(1, 14),
+};
+static const unsigned int tpu_to3_mux[] = {
+   TPUTO3_MARK,
+};
+static const unsigned int tpu_to3_b_pins[] = {
+   RCAR_GP_PIN(1, 13),
+};
+static const unsigned int tpu_to3_b_mux[] = {
+   TPUTO3_B_MARK,
+};
+static const unsigned int tpu_to3_c_pins[] = {
+   RCAR_GP_PIN(0, 21),
+};
+static const unsigned int tpu_to3_c_mux[] = {
+   TPUTO3_C_MARK,
+};
 /* - USB0 --- 
*/
 static const unsigned int usb0_pins[] = {
RCAR_GP_PIN(5, 24), /* PWEN */
@@ -3926,6 +3999,18 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
SH_PFC_PIN_GROUP(ssi9_ctrl),
SH_PFC_PIN_GROUP(ssi9_data_b),
SH_PFC_PIN_GROUP(ssi9_ctrl_b),
+   SH_PFC_PIN_GROUP(tpu_to0),
+   SH_PFC_PIN_GROUP(tpu_to0_b),
+   SH_PFC_PIN_GROUP(tpu_to0_c),
+   SH_PFC_PIN_GROUP(tpu_to1),
+   SH_PFC_PIN_GROUP(tpu_to1_b),
+   SH_PFC_PIN_GROUP(tpu_to1_c),
+   SH_PFC_PIN_GROUP(tpu_to2),
+   SH_PFC_PIN_GROUP(tpu_to2_b),
+   SH_PFC_PIN_GROUP(tpu_to2_c),
+   SH_PFC_PIN_GROUP(tpu_to3),
+   SH_PFC_PIN_GROUP(tpu_to3_b),
+   SH_PFC_PIN_GROUP(tpu_to3_c),
SH_PFC_PIN_GROUP(usb0),
SH_PFC_PIN_GROUP(usb1),
VIN_DATA_PIN_GROUP(vin0_data, 24),
@@ -4388,6 +4473,21 @@ static const char * const ssi_groups[] = {
"ssi9_ctrl_b",
 };
 
+static const char * const tpu_groups[] = {
+   "tpu_to0",
+   "tpu_to0_b",
+   "tpu_to0_c",
+   "tpu_to1",
+   "tpu_to1_b",
+   "tpu_to1_c",
+   "tpu_to2",
+   "tpu_to2_b",
+   "tpu_to2_c",
+   "tpu_to3",
+   "tpu_to3_b",
+   "tpu_to3_c",
+};
+
 static const char * const usb0_groups[] = {
"usb0",
 };
@@ -4470,6 +4570,7 @@ static const struct sh_pfc_function pinmux_functions[] = {
SH_PFC_FUNCTION(sdhi1),
SH_PFC_FUNCTION(sdhi2),
SH_PFC_FUNCTION(ssi),
+   SH_PFC_FUNCTION(tpu),
SH_PFC_FUNCTION(usb0),
SH_PFC_FUNCTION(usb1),
SH_PFC_FUNCTION(vin0),
-- 
2.7.4

[PATCH v2 1/4] pinctrl: sh-pfc: r8a7794: Add PWM[0123456] support

[Fabrizio Castro]

This patch adds PFC PWM[0123456] pin groups and functions, enabling
PWM on the r8a7794 and r8a7745.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
---
v1->v2:
* No change

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

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
index c557bc8..dccc20a 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
@@ -2546,6 +2546,109 @@ static const unsigned int msiof2_tx_b_pins[] = {
 static const unsigned int msiof2_tx_b_mux[] = {
MSIOF2_TXD_B_MARK,
 };
+/* - PWM  
*/
+static const unsigned int pwm0_pins[] = {
+   RCAR_GP_PIN(1, 14),
+};
+static const unsigned int pwm0_mux[] = {
+   PWM0_MARK,
+};
+static const unsigned int pwm0_b_pins[] = {
+   RCAR_GP_PIN(5, 3),
+};
+static const unsigned int pwm0_b_mux[] = {
+   PWM0_B_MARK,
+};
+static const unsigned int pwm1_pins[] = {
+   RCAR_GP_PIN(4, 5),
+};
+static const unsigned int pwm1_mux[] = {
+   PWM1_MARK,
+};
+static const unsigned int pwm1_b_pins[] = {
+   RCAR_GP_PIN(5, 10),
+};
+static const unsigned int pwm1_b_mux[] = {
+   PWM1_B_MARK,
+};
+static const unsigned int pwm1_c_pins[] = {
+   RCAR_GP_PIN(1, 18),
+};
+static const unsigned int pwm1_c_mux[] = {
+   PWM1_C_MARK,
+};
+static const unsigned int pwm2_pins[] = {
+   RCAR_GP_PIN(4, 10),
+};
+static const unsigned int pwm2_mux[] = {
+   PWM2_MARK,
+};
+static const unsigned int pwm2_b_pins[] = {
+   RCAR_GP_PIN(5, 17),
+};
+static const unsigned int pwm2_b_mux[] = {
+   PWM2_B_MARK,
+};
+static const unsigned int pwm2_c_pins[] = {
+   RCAR_GP_PIN(0, 13),
+};
+static const unsigned int pwm2_c_mux[] = {
+   PWM2_C_MARK,
+};
+static const unsigned int pwm3_pins[] = {
+   RCAR_GP_PIN(4, 13),
+};
+static const unsigned int pwm3_mux[] = {
+   PWM3_MARK,
+};
+static const unsigned int pwm3_b_pins[] = {
+   RCAR_GP_PIN(0, 16),
+};
+static const unsigned int pwm3_b_mux[] = {
+   PWM3_B_MARK,
+};
+static const unsigned int pwm4_pins[] = {
+   RCAR_GP_PIN(1, 3),
+};
+static const unsigned int pwm4_mux[] = {
+   PWM4_MARK,
+};
+static const unsigned int pwm4_b_pins[] = {
+   RCAR_GP_PIN(0, 21),
+};
+static const unsigned int pwm4_b_mux[] = {
+   PWM4_B_MARK,
+};
+static const unsigned int pwm5_pins[] = {
+   RCAR_GP_PIN(3, 30),
+};
+static const unsigned int pwm5_mux[] = {
+   PWM5_MARK,
+};
+static const unsigned int pwm5_b_pins[] = {
+   RCAR_GP_PIN(4, 0),
+};
+static const unsigned int pwm5_b_mux[] = {
+   PWM5_B_MARK,
+};
+static const unsigned int pwm5_c_pins[] = {
+   RCAR_GP_PIN(0, 10),
+};
+static const unsigned int pwm5_c_mux[] = {
+   PWM5_C_MARK,
+};
+static const unsigned int pwm6_pins[] = {
+   RCAR_GP_PIN(4, 8),
+};
+static const unsigned int pwm6_mux[] = {
+   PWM6_MARK,
+};
+static const unsigned int pwm6_b_pins[] = {
+   RCAR_GP_PIN(0, 7),
+};
+static const unsigned int pwm6_b_mux[] = {
+   PWM6_B_MARK,
+};
 /* - QSPI --- 
*/
 static const unsigned int qspi_ctrl_pins[] = {
/* SPCLK, SSL */
@@ -3689,6 +3792,23 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
SH_PFC_PIN_GROUP(msiof2_ss2_b),
SH_PFC_PIN_GROUP(msiof2_rx_b),
SH_PFC_PIN_GROUP(msiof2_tx_b),
+   SH_PFC_PIN_GROUP(pwm0),
+   SH_PFC_PIN_GROUP(pwm0_b),
+   SH_PFC_PIN_GROUP(pwm1),
+   SH_PFC_PIN_GROUP(pwm1_b),
+   SH_PFC_PIN_GROUP(pwm1_c),
+   SH_PFC_PIN_GROUP(pwm2),
+   SH_PFC_PIN_GROUP(pwm2_b),
+   SH_PFC_PIN_GROUP(pwm2_c),
+   SH_PFC_PIN_GROUP(pwm3),
+   SH_PFC_PIN_GROUP(pwm3_b),
+   SH_PFC_PIN_GROUP(pwm4),
+   SH_PFC_PIN_GROUP(pwm4_b),
+   SH_PFC_PIN_GROUP(pwm5),
+   SH_PFC_PIN_GROUP(pwm5_b),
+   SH_PFC_PIN_GROUP(pwm5_c),
+   SH_PFC_PIN_GROUP(pwm6),
+   SH_PFC_PIN_GROUP(pwm6_b),
SH_PFC_PIN_GROUP(qspi_ctrl),
SH_PFC_PIN_GROUP(qspi_data2),
SH_PFC_PIN_GROUP(qspi_data4),
@@ -4050,6 +4170,44 @@ static const char * const msiof2_groups[] = {
"msiof2_tx_b",
 };
 
+static const char * const pwm0_groups[] = {
+   "pwm0",
+   "pwm0_b",
+};
+
+static const char * const pwm1_groups[] = {
+   "pwm1",
+   "pwm1_b",
+   "pwm1_c",
+};
+
+static const char * const pwm2_groups[] = {
+   "pwm2",
+   "pwm2_b",
+   "pwm2_c",
+};
+
+static const char * const pwm3_groups[] = {
+   "pwm3",
+   "pwm3_b",
+};
+
+static const char * const pwm4_groups[] = {
+   "pwm4",
+   "pwm4_b",
+};
+
+static const char * const pwm5_groups[] = {
+   "pwm5",
+   "pwm5_b",
+   "pwm5_c",
+};
+
+static const 

[PATCH] pinctrl: sh-pfc: r8a7794: Add i2c5 pin groups and function

[Biju Das]

Add i2c5 pin groups and function to r8a7745 PFC driver.

Signed-off-by: Biju Das <biju@bp.renesas.com>
Reviewed-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com>
---
This patch is based on renesas devel tag renesas-devel-20171218-v4.15-rc4 

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

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
index f133b4f..1640024 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
@@ -2228,6 +2228,35 @@ enum {
 static const unsigned int i2c4_e_mux[] = {
I2C4_SCL_E_MARK, I2C4_SDA_E_MARK,
 };
+/* - I2C5 --- 
*/
+static const unsigned int i2c5_pins[] = {
+   /* SCL, SDA */
+   RCAR_GP_PIN(4, 14), RCAR_GP_PIN(4, 15),
+};
+static const unsigned int i2c5_mux[] = {
+   I2C5_SCL_MARK, I2C5_SDA_MARK,
+};
+static const unsigned int i2c5_b_pins[] = {
+   /* SCL, SDA */
+   RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 15),
+};
+static const unsigned int i2c5_b_mux[] = {
+   I2C5_SCL_B_MARK, I2C5_SDA_B_MARK,
+};
+static const unsigned int i2c5_c_pins[] = {
+   /* SCL, SDA */
+   RCAR_GP_PIN(4, 31), RCAR_GP_PIN(4, 30),
+};
+static const unsigned int i2c5_c_mux[] = {
+   I2C5_SCL_C_MARK, I2C5_SDA_C_MARK,
+};
+static const unsigned int i2c5_d_pins[] = {
+   /* SCL, SDA */
+   RCAR_GP_PIN(3, 13), RCAR_GP_PIN(3, 14),
+};
+static const unsigned int i2c5_d_mux[] = {
+   I2C5_SCL_D_MARK, I2C5_SDA_D_MARK,
+};
 /* - INTC --- 
*/
 static const unsigned int intc_irq0_pins[] = {
/* IRQ0 */
@@ -3821,6 +3850,10 @@ enum {
SH_PFC_PIN_GROUP(i2c4_c),
SH_PFC_PIN_GROUP(i2c4_d),
SH_PFC_PIN_GROUP(i2c4_e),
+   SH_PFC_PIN_GROUP(i2c5),
+   SH_PFC_PIN_GROUP(i2c5_b),
+   SH_PFC_PIN_GROUP(i2c5_c),
+   SH_PFC_PIN_GROUP(i2c5_d),
SH_PFC_PIN_GROUP(intc_irq0),
SH_PFC_PIN_GROUP(intc_irq1),
SH_PFC_PIN_GROUP(intc_irq2),
@@ -4196,6 +4229,13 @@ enum {
"i2c4_e",
 };
 
+static const char * const i2c5_groups[] = {
+   "i2c5",
+   "i2c5_b",
+   "i2c5_c",
+   "i2c5_d",
+};
+
 static const char * const intc_groups[] = {
"intc_irq0",
"intc_irq1",
@@ -4537,6 +4577,7 @@ enum {
SH_PFC_FUNCTION(i2c2),
SH_PFC_FUNCTION(i2c3),
SH_PFC_FUNCTION(i2c4),
+   SH_PFC_FUNCTION(i2c5),
SH_PFC_FUNCTION(intc),
SH_PFC_FUNCTION(mmc),
SH_PFC_FUNCTION(msiof0),
-- 
1.9.1

[PATCH v2 4/6] dt-bindings: pwm: renesas-tpu: Document r8a774[35] support

[Fabrizio Castro]

Document r8a774[35] specific compatible strings. No driver change is
needed as the fallback compatible string "renesas,tpu" activates the
right code in the driver.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
Reviewed-by: Rob Herring 
Reviewed-by: Geert Uytterhoeven 
---
v1->v2
* No change

 Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt 
b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
index 1aadc80..16e5748 100644
--- a/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt
@@ -5,8 +5,10 @@ Required Properties:
   - compatible: should be one of the following.
 - "renesas,tpu-r8a73a4": for R8A77A4 (R-Mobile APE6) compatible PWM 
controller.
 - "renesas,tpu-r8a7740": for R8A7740 (R-Mobile A1) compatible PWM 
controller.
+- "renesas,tpu-r8a7743": for R8A7743 (RZ/G1M) compatible PWM controller.
+- "renesas,tpu-r8a7745": for R8A7745 (RZ/G1E) compatible PWM controller.
 - "renesas,tpu-r8a7790": for R8A7790 (R-Car H2) compatible PWM controller.
-- "renesas,tpu": for generic R-Car TPU PWM controller.
+- "renesas,tpu": for generic R-Car and RZ/G1 TPU PWM controller.
 
   - reg: Base address and length of each memory resource used by the PWM
 controller hardware module.
-- 
2.7.4

[PATCH v2 5/6] pinctrl: sh-pfc: r8a7791: Add tpu groups and function

[Fabrizio Castro]

This patch adds tpu groups and function to r8a7743/r8a7791/r8a7793.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
---
v1->v2
* Reworked sorting

 drivers/pinctrl/sh-pfc/pfc-r8a7791.c | 42 ++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
index c01ef02d..5811784 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
@@ -4145,6 +4145,32 @@ static const unsigned int ssi9_ctrl_b_mux[] = {
SSI_SCK9_B_MARK, SSI_WS9_B_MARK,
 };
 
+/* - TPU  
*/
+static const unsigned int tpu_to0_pins[] = {
+   RCAR_GP_PIN(6, 14),
+};
+static const unsigned int tpu_to0_mux[] = {
+   TPU_TO0_MARK,
+};
+static const unsigned int tpu_to1_pins[] = {
+   RCAR_GP_PIN(1, 17),
+};
+static const unsigned int tpu_to1_mux[] = {
+   TPU_TO1_MARK,
+};
+static const unsigned int tpu_to2_pins[] = {
+   RCAR_GP_PIN(1, 18),
+};
+static const unsigned int tpu_to2_mux[] = {
+   TPU_TO2_MARK,
+};
+static const unsigned int tpu_to3_pins[] = {
+   RCAR_GP_PIN(1, 24),
+};
+static const unsigned int tpu_to3_mux[] = {
+   TPU_TO3_MARK,
+};
+
 /* - USB0 --- 
*/
 static const unsigned int usb0_pins[] = {
RCAR_GP_PIN(7, 23), /* PWEN */
@@ -4431,7 +4457,7 @@ static const unsigned int vin2_clk_mux[] = {
 };
 
 static const struct {
-   struct sh_pfc_pin_group common[342];
+   struct sh_pfc_pin_group common[346];
struct sh_pfc_pin_group r8a779x[9];
 } pinmux_groups = {
.common = {
@@ -4743,6 +4769,10 @@ static const struct {
SH_PFC_PIN_GROUP(ssi9_data_b),
SH_PFC_PIN_GROUP(ssi9_ctrl),
SH_PFC_PIN_GROUP(ssi9_ctrl_b),
+   SH_PFC_PIN_GROUP(tpu_to0),
+   SH_PFC_PIN_GROUP(tpu_to1),
+   SH_PFC_PIN_GROUP(tpu_to2),
+   SH_PFC_PIN_GROUP(tpu_to3),
SH_PFC_PIN_GROUP(usb0),
SH_PFC_PIN_GROUP(usb1),
VIN_DATA_PIN_GROUP(vin0_data, 24),
@@ -5278,6 +5308,13 @@ static const char * const ssi_groups[] = {
"ssi9_ctrl_b",
 };
 
+static const char * const tpu_groups[] = {
+   "tpu_to0",
+   "tpu_to1",
+   "tpu_to2",
+   "tpu_to3",
+};
+
 static const char * const usb0_groups[] = {
"usb0",
 };
@@ -5327,7 +5364,7 @@ static const char * const vin2_groups[] = {
 };
 
 static const struct {
-   struct sh_pfc_function common[57];
+   struct sh_pfc_function common[58];
struct sh_pfc_function r8a779x[2];
 } pinmux_functions = {
.common = {
@@ -5383,6 +5420,7 @@ static const struct {
SH_PFC_FUNCTION(sdhi1),
SH_PFC_FUNCTION(sdhi2),
SH_PFC_FUNCTION(ssi),
+   SH_PFC_FUNCTION(tpu),
SH_PFC_FUNCTION(usb0),
SH_PFC_FUNCTION(usb1),
SH_PFC_FUNCTION(vin0),
-- 
2.7.4

[PATCH v2 6/6] ARM: dts: r8a7743: Add TPU support

[Fabrizio Castro]

Add TPU support to SoC DT.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2
* No change

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

diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index d73617a..11defe7c 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -1159,6 +1159,16 @@
status = "disabled";
};
 
+   tpu: pwm@e60f {
+   compatible = "renesas,tpu-r8a7743", "renesas,tpu";
+   reg = <0 0xe60f 0 0x148>;
+   clocks = < CPG_MOD 304>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 304>;
+   #pwm-cells = <3>;
+   status = "disabled";
+   };
+
sdhi0: sd@ee10 {
compatible = "renesas,sdhi-r8a7743",
 "renesas,rcar-gen2-sdhi";
-- 
2.7.4

[PATCH v2 2/6] dt-bindings: pwm: rcar: Document r8a774[35] PWM bindings

[Fabrizio Castro]

This patch adds compatible strings specific to r8a774[35], no driver
change is needed as the fallback compatible string will activate the
right code. Also, this patch replaces the example with a DT snippet used
for adding PWM0 support to an r8a7743 based platform.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
Reviewed-by: Rob Herring 
Reviewed-by: Geert Uytterhoeven 
---
v1->v2
* No change

 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt 
b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
index 74c1180..f3bba59 100644
--- a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
+++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
@@ -2,6 +2,8 @@
 
 Required Properties:
 - compatible: should be "renesas,pwm-rcar" and one of the following.
+ - "renesas,pwm-r8a7743": for RZ/G1M
+ - "renesas,pwm-r8a7745": for RZ/G1E
  - "renesas,pwm-r8a7778": for R-Car M1A
  - "renesas,pwm-r8a7779": for R-Car H1
  - "renesas,pwm-r8a7790": for R-Car H2
@@ -17,13 +19,15 @@ Required Properties:
 - pinctrl-0: phandle, referring to a default pin configuration node.
 - pinctrl-names: Set to "default".
 
-Example: R8A7790 (R-Car H2) PWM Timer node
+Example: R8A7743 (RZ/G1M) PWM Timer node
 
pwm0: pwm@e6e3 {
-   compatible = "renesas,pwm-r8a7790", "renesas,pwm-rcar";
+   compatible = "renesas,pwm-r8a7743", "renesas,pwm-rcar";
reg = <0 0xe6e3 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
#pwm-cells = <2>;
-   clocks = <_clks R8A7790_CLK_PWM>;
pinctrl-0 = <_pins>;
pinctrl-names = "default";
};
-- 
2.7.4

[PATCH v2 3/6] ARM: dts: r8a7743: Add PWM SoC support

[Fabrizio Castro]

Add the definitions for pwm[0123456] to the SoC .dtsi.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2
* No change

 arch/arm/boot/dts/r8a7743.dtsi | 77 ++
 1 file changed, 77 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index c65d8f4..d73617a 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -1082,6 +1082,83 @@
status = "disabled";
};
 
+   pwm0: pwm@e6e3 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e3 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm1: pwm@e6e31000 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e31000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm2: pwm@e6e32000 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e32000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm3: pwm@e6e33000 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e33000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm4: pwm@e6e34000 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e34000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm5: pwm@e6e35000 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e35000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
+   pwm6: pwm@e6e36000 {
+   compatible = "renesas,pwm-r8a7743",
+"renesas,pwm-rcar";
+   reg = <0 0xe6e36000 0 0x8>;
+   clocks = < CPG_MOD 523>;
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 523>;
+   #pwm-cells = <2>;
+   status = "disabled";
+   };
+
sdhi0: sd@ee10 {
compatible = "renesas,sdhi-r8a7743",
 "renesas,rcar-gen2-sdhi";
-- 
2.7.4

[PATCH v2 0/6] Add PWM and TPU support to r8a7743

[Fabrizio Castro]

Dear All,

this series includes all that is required to add PWM and TPU SoC specific
support for the r8a7743.

Thanks,

Fabrizio Castro (6):
  ARM: shmobile: defconfig: Enable PWM
  dt-bindings: pwm: rcar: Document r8a774[35] PWM bindings
  ARM: dts: r8a7743: Add PWM SoC support
  dt-bindings: pwm: renesas-tpu: Document r8a774[35] support
  pinctrl: sh-pfc: r8a7791: Add tpu groups and function
  ARM: dts: r8a7743: Add TPU support

 .../devicetree/bindings/pwm/renesas,pwm-rcar.txt   | 10 ++-
 .../devicetree/bindings/pwm/renesas,tpu-pwm.txt|  4 +-
 arch/arm/boot/dts/r8a7743.dtsi | 87 ++
 arch/arm/configs/shmobile_defconfig|  1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c   | 42 ++-
 5 files changed, 138 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH v2 1/6] ARM: shmobile: defconfig: Enable PWM

[Fabrizio Castro]

RZ/G1 and R-Car platforms have PWM timers. This patch enables PWM support
by default.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2
* No change

 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 d60dbe1..578434c 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -208,6 +208,7 @@ CONFIG_STAGING_BOARD=y
 CONFIG_IIO=y
 CONFIG_AK8975=y
 CONFIG_PWM=y
+CONFIG_PWM_RCAR=y
 CONFIG_PWM_RENESAS_TPU=y
 CONFIG_GENERIC_PHY=y
 CONFIG_PHY_RCAR_GEN2=y
-- 
2.7.4

[PATCH v2 3/3] ARM: dts: r8a7745: Add CMT SoC specific support

[Fabrizio Castro]

Add CMT[01] support to SoC DT.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2:
* status "okay"->"disabled"

 arch/arm/boot/dts/r8a7745.dtsi | 32 
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 0fa7861..0b67ee5 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -235,6 +235,38 @@
  IRQ_TYPE_LEVEL_LOW)>;
};
 
+   cmt0: timer@ffca {
+   compatible = "renesas,r8a7745-cmt0",
+"renesas,rcar-gen2-cmt0";
+   reg = <0 0xffca 0 0x1004>;
+   interrupts = ,
+;
+   clocks = < CPG_MOD 124>;
+   clock-names = "fck";
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 124>;
+   status = "disabled";
+   };
+
+   cmt1: timer@e613 {
+   compatible = "renesas,r8a7745-cmt1",
+"renesas,rcar-gen2-cmt1";
+   reg = <0 0xe613 0 0x1004>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+;
+   clocks = < CPG_MOD 329>;
+   clock-names = "fck";
+   power-domains = < R8A7745_PD_ALWAYS_ON>;
+   resets = < 329>;
+   status = "disabled";
+   };
+
cpg: clock-controller@e615 {
compatible = "renesas,r8a7745-cpg-mssr";
reg = <0 0xe615 0 0x1000>;
-- 
2.7.4

[PATCH v2 0/3] Add CMT support to r8a774[35]

[Fabrizio Castro]

Hello,

this series adds CMT support for r8a7743 and r8a7745.

Thanks,

Fabrizio Castro (3):
  dt-bindings: timer: renesas, cmt: Document r8a774[35] CMT support
  ARM: dts: r8a7743: Add CMT SoC specific support
  ARM: dts: r8a7745: Add CMT SoC specific support

 .../devicetree/bindings/timer/renesas,cmt.txt  | 14 +++---
 arch/arm/boot/dts/r8a7743.dtsi | 32 ++
 arch/arm/boot/dts/r8a7745.dtsi | 32 ++
 3 files changed, 74 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] dt-bindings: timer: renesas, cmt: Document r8a774[35] CMT support

[Fabrizio Castro]

Document SoC specific compatible strings for r8a7743 and r8a7745.
No driver change is needed as the fallback strings will activate
the right code.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
Reviewed-by: Rob Herring 
Reviewed-by: Simon Horman 
---
v1->v2:
* "R-Car Gen2 or RZ/G1." -> "R-Car Gen2 and RZ/G1."
* "all the R-Car" -> "R-Car"

 Documentation/devicetree/bindings/timer/renesas,cmt.txt | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/renesas,cmt.txt 
b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
index d740989..b40add2 100644
--- a/Documentation/devicetree/bindings/timer/renesas,cmt.txt
+++ b/Documentation/devicetree/bindings/timer/renesas,cmt.txt
@@ -22,6 +22,10 @@ Required Properties:
 
 - "renesas,r8a73a4-cmt0" for the 32-bit CMT0 device included in r8a73a4.
 - "renesas,r8a73a4-cmt1" for the 48-bit CMT1 device included in r8a73a4.
+- "renesas,r8a7743-cmt0" for the 32-bit CMT0 device included in r8a7743.
+- "renesas,r8a7743-cmt1" for the 48-bit CMT1 device included in r8a7743.
+- "renesas,r8a7745-cmt0" for the 32-bit CMT0 device included in r8a7745.
+- "renesas,r8a7745-cmt1" for the 48-bit CMT1 device included in r8a7745.
 - "renesas,r8a7790-cmt0" for the 32-bit CMT0 device included in r8a7790.
 - "renesas,r8a7790-cmt1" for the 48-bit CMT1 device included in r8a7790.
 - "renesas,r8a7791-cmt0" for the 32-bit CMT0 device included in r8a7791.
@@ -31,10 +35,12 @@ Required Properties:
 - "renesas,r8a7794-cmt0" for the 32-bit CMT0 device included in r8a7794.
 - "renesas,r8a7794-cmt1" for the 48-bit CMT1 device included in r8a7794.
 
-- "renesas,rcar-gen2-cmt0" for 32-bit CMT0 devices included in R-Car Gen2.
-- "renesas,rcar-gen2-cmt1" for 48-bit CMT1 devices included in R-Car Gen2.
-   These are fallbacks for r8a73a4 and all the R-Car Gen2
-   entries listed above.
+- "renesas,rcar-gen2-cmt0" for 32-bit CMT0 devices included in R-Car Gen2
+   and RZ/G1.
+- "renesas,rcar-gen2-cmt1" for 48-bit CMT1 devices included in R-Car Gen2
+   and RZ/G1.
+   These are fallbacks for r8a73a4, R-Car Gen2 and RZ/G1 entries
+   listed above.
 
   - reg: base address and length of the registers block for the timer module.
   - interrupts: interrupt-specifier for the timer, one per channel.
-- 
2.7.4

[PATCH v2 2/3] ARM: dts: r8a7743: Add CMT SoC specific support

[Fabrizio Castro]

Add CMT[01] support to SoC DT.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Biju Das 
---
v1->v2:
* status "okay"->"disabled"

 arch/arm/boot/dts/r8a7743.dtsi | 32 
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index 59860c8..f250147 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -262,6 +262,38 @@
  IRQ_TYPE_LEVEL_LOW)>;
};
 
+   cmt0: timer@ffca {
+   compatible = "renesas,r8a7743-cmt0",
+"renesas,rcar-gen2-cmt0";
+   reg = <0 0xffca 0 0x1004>;
+   interrupts = ,
+;
+   clocks = < CPG_MOD 124>;
+   clock-names = "fck";
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 124>;
+   status = "disabled";
+   };
+
+   cmt1: timer@e613 {
+   compatible = "renesas,r8a7743-cmt1",
+"renesas,rcar-gen2-cmt1";
+   reg = <0 0xe613 0 0x1004>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+;
+   clocks = < CPG_MOD 329>;
+   clock-names = "fck";
+   power-domains = < R8A7743_PD_ALWAYS_ON>;
+   resets = < 329>;
+   status = "disabled";
+   };
+
cpg: clock-controller@e615 {
compatible = "renesas,r8a7743-cpg-mssr";
reg = <0 0xe615 0 0x1000>;
-- 
2.7.4

Re: [PATCH 00/24] V4L2 kAPI cleanups and documentation improvements part 2

[Mauro Carvalho Chehab]

Em Mon,  9 Oct 2017 07:19:06 -0300
Mauro Carvalho Chehab  escreveu:

> That's the second part of my V4L2 kAPI documentation improvements.
> It is meant to reduce the gap between the kAPI media headers
> and documentation, at least with regards to kernel-doc markups.
> 
> We should likely write more things at the ReST files under Documentation/
> to better describe some of those APIs (VB2 being likely the first candidate),
> but at least let's be sure that all V4L2 bits have kernel-doc markups.

I'm also applying the patches from this series that nobody commented,
or whose comments were fully addressed.

Thanks,
Mauro

RE: [PATCH 3/4] pinctrl: sh-pfc: r8a7794: Add tpu groups and function

[Fabrizio Castro]

Hello Geert,

thank you for your feedback.

> Subject: Re: [PATCH 3/4] pinctrl: sh-pfc: r8a7794: Add tpu groups and function
>
> On Mon, Dec 18, 2017 at 12:09 PM, Geert Uytterhoeven
>  wrote:
> > On Thu, Dec 14, 2017 at 11:57 AM, Fabrizio Castro
> >  wrote:
> >> This patch adds tpu groups and function to r8a7745/r8a7794.
> >>
> >> Signed-off-by: Fabrizio Castro 
> >> Reviewed-by: Biju Das 
> >
> > Reviewed-by: Geert Uytterhoeven 
> > i.e. will queue in sh-pfc-for-v4.16.
>
> Oops, missed one thing: please use alphabetical sort order when adding
> new entries.

I'll send a v2 to fix the order.

Thanks,
Fab

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


[https://www2.renesas.eu/media/email/unicef_2017.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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

RE: [PATCH 5/6] pinctrl: sh-pfc: r8a7791: Add tpu groups and function

[Fabrizio Castro]

Hello Geert,

Thank you for your feedback.

> Subject: Re: [PATCH 5/6] pinctrl: sh-pfc: r8a7791: Add tpu groups and function
>
> Hi Fabrizio,
>
> On Thu, Dec 14, 2017 at 11:56 AM, Fabrizio Castro
>  wrote:
> > This patch adds tpu groups and function to r8a7743/r8a7791/r8a7793.
> >
> > Signed-off-by: Fabrizio Castro 
> > Reviewed-by: Biju Das 
>
> With the sort order fixed:
> Reviewed-by: Geert Uytterhoeven 

I'll send a v2 to fix the order.

Thanks,
Fab

>
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> > @@ -3892,6 +3892,32 @@ static const unsigned int sdhi2_wp_mux[] = {
> > SD2_WP_MARK,
> >  };
> >
> > +/* - TPU 
> >  */
> > +static const unsigned int tpu_to0_pins[] = {
> > +   RCAR_GP_PIN(6, 14),
> > +};
> > +static const unsigned int tpu_to0_mux[] = {
> > +   TPU_TO0_MARK,
> > +};
> > +static const unsigned int tpu_to1_pins[] = {
> > +   RCAR_GP_PIN(1, 17),
> > +};
> > +static const unsigned int tpu_to1_mux[] = {
> > +   TPU_TO1_MARK,
> > +};
> > +static const unsigned int tpu_to2_pins[] = {
> > +   RCAR_GP_PIN(1, 18),
> > +};
> > +static const unsigned int tpu_to2_mux[] = {
> > +   TPU_TO2_MARK,
> > +};
> > +static const unsigned int tpu_to3_pins[] = {
> > +   RCAR_GP_PIN(1, 24),
> > +};
> > +static const unsigned int tpu_to3_mux[] = {
> > +   TPU_TO3_MARK,
> > +};
> > +
> >  /* - SSI 
> >  */
> >  static const unsigned int ssi0_data_pins[] = {
> > /* SDATA */
>
> Please use alphabetical sort order (also below).
>
> 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


[https://www2.renesas.eu/media/email/unicef_2017.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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

Re: [RFC PATCH] mmc: tmio: use ioread* for repeated access to a register

[Masahiro Yamada]

2017-12-18 22:01 GMT+09:00 Ulf Hansson :
> On 18 December 2017 at 01:00, Wolfram Sang
>  wrote:
>> Not all archs define reads* and writes*. Switch to ioread*_rep and
>> friends which is defined everywhere, so we can enable COMPILE_TEST after
>> that.
>>
>> Signed-off-by: Wolfram Sang 
>
> Thanks, applied for next!
>
> I put the patch in front of Yamada-san's patch that made it possible
> to use COMPILE_TEST. Thus avoiding breaking bisecting.
>
>> ---
>>
>> So, I pushed this to buildbot on top of Yamada-san's patch series and there
>> were no complaints, even with COMPILE_TEST enabled. I also did some tests on
>> HW and checksuming huge files on SD cards still works.
>>
>> However, I am not sure about this mixture of read* and ioread* functions. 
>> Shall
>> we convert maybe all of those?
>
> Yes, seems like a good idea.
>
> [...]
>

BTW, is ioread* preferred to read*?

We need to eliminate the root cause.
Other drivers using reads* cannot enable COMPILE_TEST for the same reason.

Wolfram,
Can you send a patch for sparc64?




-- 
Best Regards
Masahiro Yamada

[PATCH v2 2/2] can: rcar_canfd: document r8a77995 (R-Car D3) compatibility strings

[Ulrich Hecht]

Signed-off-by: Ulrich Hecht 
Acked-by: Rob Herring 
Reviewed-by: Geert Uytterhoeven 
---
 Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt 
b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 93c3a6a..eeb9fec 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -6,6 +6,7 @@ Required properties:
   - "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
   - "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible controller.
   - "renesas,r8a7796-canfd" for R8A7796 (R-Car M3) compatible controller.
+  - "renesas,r8a77995-canfd" for R8A77995 (R-Car D3) compatible controller.
 
   When compatible with the generic version, nodes must list the
   SoC-specific version corresponding to the platform first, followed by the
@@ -24,12 +25,12 @@ The name of the child nodes are "channel0" and "channel1" 
respectively. Each
 child node supports the "status" property only, which is used to
 enable/disable the respective channel.
 
-Required properties for "renesas,r8a7795-canfd" and "renesas,r8a7796-canfd"
-compatible:
-In R8A7795 and R8A7796 SoCs, canfd clock is a div6 clock and can be used by 
both
-CAN and CAN FD controller at the same time. It needs to be scaled to maximum
-frequency if any of these controllers use it. This is done using the below
-properties:
+Required properties for "renesas,r8a7795-canfd", "renesas,r8a7796-canfd" and
+"renesas,r8a77995-canfd" compatible:
+In R8A7795, R8A7796 and R8A77995 SoCs, canfd clock is a div6 clock and can
+be used by both CAN and CAN FD controller at the same time. It needs to be
+scaled to maximum frequency if any of these controllers use it. This is
+done using the below properties:
 
 - assigned-clocks: phandle of canfd clock.
 - assigned-clock-rates: maximum frequency of this clock.
-- 
2.7.4

Re: [PATCH 3/4] kms++util: Add verification module

[Tomi Valkeinen]

On 18/12/17 17:41, Laurent Pinchart wrote:

Hi Tomi,

On Monday, 18 December 2017 14:04:45 EET Tomi Valkeinen wrote:

On 18/12/17 13:50, Laurent Pinchart wrote:

That's an option too. I had a look at the code once to find out how
ImageMagick was performing scaling and gave up with a headache soon
afterwards. We need more formats than what ImageMagick currently supports
(it's mostly focused on image file formats instead of raw image formats),
with all kind of RGB, YUV (and ideally Bayer) formats, and I don't think
extending ImageMagick would be the way to go.


Yep, I can image that ImageMagick can't handle it all. I don't really
have much experience with it. I did find that it has enough tunables to
manage rgbx with different byte orders and the "extra" alpha channel:

convert -depth 8 -size $1x$2 -color-matrix "0 0 1 0 1 0 1 0 0" -alpha
deactivate rgba:$3 png:-

But things like argb1555 or yuv422 might prove to be too difficult for
ImageMagick.

Where's the source for v4l2convert? What does it do?


Sorry, it's v4lconvert, not v4l2convert. The library is part of v4l-utils
(available at git://linuxtv.org/pinchartl/v4l-utils.git) and is found in the
lib/libv4lconvert directory.

The purpose of the library is to convert from lots of weird formats supported
by V4L2 to RGB or YUV. It was initially written to handle vendor-specific
compression formats used by webcams, has been extended to support Bayer
formats and different kind of RGB and YUV formats. The code is simple, I think
it could be a good base.


I wonder how difficult it would be to create a brute-force-no-optimizations
style of a function to which you give the exact bit definition of the
buffer's pixel format, and which gives you RGB888 or YUV444, depending on
the input format (I presume most image encoders would accept RGB888 &
YUV444).


Can you come up with a bit definition format that could describe Bayer or
tiled formats without requiring the definition to be written in an interpreted
language ? :-)


I have only worked with RGB and YUV, so that should be enough for 
everybody. ;)


But more seriously, I think kms++ is mainly for display. Do we have any 
display devices that support Bayer? And tiled formats sound a bit 
special platform specific cases.


That said, if we have a library that just does it all, that's obviously 
the best option.


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

RE: [PATCH 2/3] ARM: dts: r8a7743: Add CMT SoC specific support

[Fabrizio Castro]

Hello Simon,

> > Hello Geert,
> >
> > thank you for your feedback.
> >
> > > Hi Fabrizio,
> > >
> > > On Wed, Dec 13, 2017 at 10:42 AM, Fabrizio Castro
> > >  wrote:
> > > >> On Tue, Dec 12, 2017 at 06:49:38PM +, Fabrizio Castro wrote:
> > > >> > Add CMT[01] support to SoC DT.
> > > >> > Signed-off-by: Fabrizio Castro 
> > > >> > Reviewed-by: Biju Das 
> > > >> > ---
> > > >> >  arch/arm/boot/dts/r8a7743.dtsi | 30 ++
> > > >> >  1 file changed, 30 insertions(+)
> > > >>
> > > >> I was expecting the cmt nodes to be "disabled" in the SoC file
> > > >> and then enabled selectively in board files. Am I missing something?
> > > >
> > > > Since this component is just a compare and match timer, I  thought 
> > > > there was no harm in enabling it by default in the SoC specific
> DT.
> > > > The system will park it and leave its clock disabled until actually 
> > > > needed for something.
> > > > The user can still disable it in the board specific DT if he/she 
> > > > doesn't mean to even have the option to use it. Do you prefer I left it
> > > disabled by default?
> > >
> > > It's debatable (thus up to Simon the maintainer ;-).
> > > For I/O devices, we disable them in the SoC .dtsi file.
> > > For core infrastructure like interrupt, DMA, and GPIO controllers, we keep
> > > them enabled.
> > >
> > > Timers are core functionality, but who's actually using these timers?
> >
> > I don't have a use case in mind unfortunately, but it's still core
> > functionality and pretty harmless as far as I can tell. Let's see what
> > Simon thinks about this.
>
> On Renesas SoCs we have a bit more flexibility with timers than might
> be the case on other SoCs. Thus I'd like to keep with the scheme of
> disabling them in .dtsi and enabling them when they are needed.
>
> Please update the patches.

Ok, I'll send a v2.

Thanks,
Fab



[https://www2.renesas.eu/media/email/unicef_2017.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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

Re: [PATCH 3/4] kms++util: Add verification module

[Laurent Pinchart]

Hi Tomi,

On Monday, 18 December 2017 14:04:45 EET Tomi Valkeinen wrote:
> On 18/12/17 13:50, Laurent Pinchart wrote:
> > That's an option too. I had a look at the code once to find out how
> > ImageMagick was performing scaling and gave up with a headache soon
> > afterwards. We need more formats than what ImageMagick currently supports
> > (it's mostly focused on image file formats instead of raw image formats),
> > with all kind of RGB, YUV (and ideally Bayer) formats, and I don't think
> > extending ImageMagick would be the way to go.
> 
> Yep, I can image that ImageMagick can't handle it all. I don't really
> have much experience with it. I did find that it has enough tunables to
> manage rgbx with different byte orders and the "extra" alpha channel:
> 
> convert -depth 8 -size $1x$2 -color-matrix "0 0 1 0 1 0 1 0 0" -alpha
> deactivate rgba:$3 png:-
> 
> But things like argb1555 or yuv422 might prove to be too difficult for
> ImageMagick.
> 
> Where's the source for v4l2convert? What does it do?

Sorry, it's v4lconvert, not v4l2convert. The library is part of v4l-utils 
(available at git://linuxtv.org/pinchartl/v4l-utils.git) and is found in the 
lib/libv4lconvert directory.

The purpose of the library is to convert from lots of weird formats supported 
by V4L2 to RGB or YUV. It was initially written to handle vendor-specific 
compression formats used by webcams, has been extended to support Bayer 
formats and different kind of RGB and YUV formats. The code is simple, I think 
it could be a good base.

> I wonder how difficult it would be to create a brute-force-no-optimizations
> style of a function to which you give the exact bit definition of the
> buffer's pixel format, and which gives you RGB888 or YUV444, depending on
> the input format (I presume most image encoders would accept RGB888 &
> YUV444).

Can you come up with a bit definition format that could describe Bayer or 
tiled formats without requiring the definition to be written in an interpreted 
language ? :-)

> I think the function wouldn't even need to care whether the data is RGB
> or YUV, it would just unpack it according to the format definition.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

[Laurent Pinchart]

Hi Jacopo,

On Monday, 18 December 2017 14:25:12 EET jacopo mondi wrote:
> On Mon, Dec 11, 2017 at 06:15:23PM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> > 
> > Thank you for the patch.
> 
> [snip]
> 
> >> +
> >> +/**
> >> + * ceu_buffer - Link vb2 buffer to the list of available buffers
> > 
> > If you use kerneldoc comments please make them compile. You need to
> > document the structure fields and function arguments.
> 
> Ok, no kernel doc for internal structures then and no kernel doc for
> ugly comments you pointed out below

You can use kerneldoc if you want to, but if you do please make sure it 
compiles :-)

> [snip]
> 
> >> +/**
> >> + * ceu_soft_reset() - Software reset the CEU interface
> >> + */
> >> +static int ceu_soft_reset(struct ceu_device *ceudev)
> >> +{
> >> +  unsigned int reset_done;
> >> +  unsigned int i;
> >> +
> >> +  ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CPKIL);
> >> +
> >> +  reset_done = 0;
> >> +  for (i = 0; i < 1000 && !reset_done; i++) {
> >> +  udelay(1);
> >> +  if (!(ceu_read(ceudev, CEU_CSTSR) & CEU_CSTRST_CPTON))
> >> +  reset_done++;
> >> +  }
> > 
> > How many iterations does this typically require ? Wouldn't a sleep be
> > better than a delay ? As far as I can tell the ceu_soft_reset() function
> > is only called with interrupts disabled (in interrupt context) from
> > ceu_capture() in an error path, and that code should be reworked to make
> > it possible to sleep if a reset takes too long.
> 
> The HW manual does not provide any indication about absolute timings.
> I can empirically try and see, but that would just be a hint.

That's why I asked how many iterations it typically takes :-) A hint is enough 
to start with, preferably on both SH and ARM SoCs.

> Also, the reset function is called in many places (runtime_pm
> suspend/resume) s_stream(0) and in error path of ceu_capture().
> 
> In ceu_capture() we reset the interface if the previous frame was bad,
> and we do that before re-enabling the capture interrupt (so interrupts
> are not -disabled-, just the one we care about is not enabled yet..)

The ceu_capture() function is called from the driver's interrupt handler, so 
interrupts are disabled in that code path.

> But that's not big deal, as if we fail there, we are about to abort
> capture anyhow and so if we miss some spurious capture interrupt it's
> ok...
> 
> >> +  if (!reset_done) {
> >> +  v4l2_err(>v4l2_dev, "soft reset time out\n");
> > 
> > How about dev_err() instead ?
> 
> Is dev_err/dev_dbg preferred over v4l2_err/v4l2_dbg? Is this because
> of dynamic debug?

Yes, and the fact that the V4L2 macros don't provide us anymore with much 
compared to the dev_* macros.

> >> +
> >> +/**
> >> + * ceu_capture() - Trigger start of a capture sequence
> >> + *
> >> + * Return value doesn't reflect the success/failure to queue the new
> >> buffer,
> >> + * but rather the status of the previous capture.
> >> + */
> >> +static int ceu_capture(struct ceu_device *ceudev)
> >> +{
> >> +  struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> >> +  dma_addr_t phys_addr_top;
> >> +  u32 status;
> >> +
> >> +  /* Clean interrupt status and re-enable interrupts */
> >> +  status = ceu_read(ceudev, CEU_CETCR);
> >> +  ceu_write(ceudev, CEU_CEIER,
> >> +ceu_read(ceudev, CEU_CEIER) & ~CEU_CEIER_MASK);
> >> +  ceu_write(ceudev, CEU_CETCR, ~status & CEU_CETCR_MAGIC);
> >> +  ceu_write(ceudev, CEU_CEIER, CEU_CEIER_MASK);
> > 
> > I wonder why there's a need to disable and reenable interrupts here.
> 
> The original driver clearly said "The hardware is -very- picky about
> this sequence" and I got scared and nerver touched this.

How about experimenting to see how the hardware reacts ?

> Also, I very much dislike the CEU_CETRC_MAGIC mask, but again the old driver
> said "Acknoledge magical interrupt sources" and I was afraid to change it
> (I can rename it though, to something lioke CEU_CETCR_ALL_INT because that's
> what it is, a mask with all available interrupt source enabled).

I think renaming it is a good idea. Additionally, regardless of whether there 
is any hidden interrupt source, the datasheet mentions for all reserved bits 
that "The write  value  should always  be 0". They should read as 0, but 
masking them would be an additional safeguard.

Also not that on the RZ/A1 platform bit 22 is documented as reserved, so you 
might want to compute the mask based on the CEU model.

If you have time you could add a debug print when an undocumented interrupt is 
flagged and see if that happens for real.

> >> +
> >> +static irqreturn_t ceu_irq(int irq, void *data)
> >> +{
> >> +  struct ceu_device *ceudev = data;
> >> +  struct vb2_v4l2_buffer *vbuf;
> >> +  struct ceu_buffer *buf;
> >> +  int ret;
> >> +
> >> +  spin_lock(>lock);
> >> +  vbuf = ceudev->active;
> >> +  if (!vbuf)
> >> +  /* Stale interrupt from a released buffer */
> >> +  goto out;
> > 
> > Shouldn't you at least clear the 

Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

[jacopo mondi]

Hi Hans,
   thanks for review comments

On Wed, Dec 13, 2017 at 01:03:03PM +0100, Hans Verkuil wrote:
> On 15/11/17 11:55, Jacopo Mondi wrote:
> > Add driver for Renesas Capture Engine Unit (CEU).
> > +
> > +   /* Register the video device */
> > +   strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME));
> > +   vdev->v4l2_dev  = v4l2_dev;
> > +   vdev->lock  = >mlock;
> > +   vdev->queue = >vb2_vq;
> > +   vdev->ctrl_handler  = v4l2_sd->ctrl_handler;
> > +   vdev->fops  = _fops;
> > +   vdev->ioctl_ops = _ioctl_ops;
> > +   vdev->release   = ceu_vdev_release;
> > +   vdev->device_caps   = V4L2_CAP_VIDEO_CAPTURE_MPLANE |
>
> Why MPLANE? It doesn't appear to be needed since there are no multiplane
> (really: multibuffer) pixelformats defined.

The driver support NV12/21 and NV16/61 as output pixel formats (along
with single plane YUYV).

NV* formats are semi-planar, as luma is stored in one buffer, while
chrominances are stored together in a different one. Am I wrong?

> >
>
> Regards,

Thanks
   j

>
>   Hans

Re: [RFC PATCH] mmc: tmio: use ioread* for repeated access to a register

[Ulf Hansson]

On 18 December 2017 at 01:00, Wolfram Sang
 wrote:
> Not all archs define reads* and writes*. Switch to ioread*_rep and
> friends which is defined everywhere, so we can enable COMPILE_TEST after
> that.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

I put the patch in front of Yamada-san's patch that made it possible
to use COMPILE_TEST. Thus avoiding breaking bisecting.

> ---
>
> So, I pushed this to buildbot on top of Yamada-san's patch series and there
> were no complaints, even with COMPILE_TEST enabled. I also did some tests on
> HW and checksuming huge files on SD cards still works.
>
> However, I am not sure about this mixture of read* and ioread* functions. 
> Shall
> we convert maybe all of those?

Yes, seems like a good idea.

[...]

Kind regards
Uffe

Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver

[jacopo mondi]

Hi Laurent,
thanks for review comments...

On Mon, Dec 11, 2017 at 06:15:23PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
[snip]

> > +
> > +/**
> > + * ceu_buffer - Link vb2 buffer to the list of available buffers
>
> If you use kerneldoc comments please make them compile. You need to document
> the structure fields and function arguments.
>

Ok, no kernel doc for internal structures then and no kernel doc for
ugly comments you pointed out below

[snip]

> > +/**
> > + * ceu_soft_reset() - Software reset the CEU interface
> > + */
> > +static int ceu_soft_reset(struct ceu_device *ceudev)
> > +{
> > +   unsigned int reset_done;
> > +   unsigned int i;
> > +
> > +   ceu_write(ceudev, CEU_CAPSR, CEU_CAPSR_CPKIL);
> > +
> > +   reset_done = 0;
> > +   for (i = 0; i < 1000 && !reset_done; i++) {
> > +   udelay(1);
> > +   if (!(ceu_read(ceudev, CEU_CSTSR) & CEU_CSTRST_CPTON))
> > +   reset_done++;
> > +   }
>
> How many iterations does this typically require ? Wouldn't a sleep be better
> than a delay ? As far as I can tell the ceu_soft_reset() function is only
> called with interrupts disabled (in interrupt context) from ceu_capture() in
> an error path, and that code should be reworked to make it possible to sleep
> if a reset takes too long.
>

The HW manual does not provide any indication about absolute timings.
I can empirically try and see, but that would just be a hint.

Also, the reset function is called in many places (runtime_pm
suspend/resume) s_stream(0) and in error path of ceu_capture().

In ceu_capture() we reset the interface if the previous frame was bad,
and we do that before re-enabling the capture interrupt (so interrupts
are not -disabled-, just the one we care about is not enabled yet..)

But that's not big deal, as if we fail there, we are about to abort
capture anyhow and so if we miss some spurious capture interrupt it's
ok...


> > +   if (!reset_done) {
> > +   v4l2_err(>v4l2_dev, "soft reset time out\n");
>
> How about dev_err() instead ?

Is dev_err/dev_dbg preferred over v4l2_err/v4l2_dbg? Is this because
of dynamic debug?

> > +
> > +/**
> > + * ceu_capture() - Trigger start of a capture sequence
> > + *
> > + * Return value doesn't reflect the success/failure to queue the new
> > buffer,
> > + * but rather the status of the previous capture.
> > + */
> > +static int ceu_capture(struct ceu_device *ceudev)
> > +{
> > +   struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> > +   dma_addr_t phys_addr_top;
> > +   u32 status;
> > +
> > +   /* Clean interrupt status and re-enable interrupts */
> > +   status = ceu_read(ceudev, CEU_CETCR);
> > +   ceu_write(ceudev, CEU_CEIER,
> > + ceu_read(ceudev, CEU_CEIER) & ~CEU_CEIER_MASK);
> > +   ceu_write(ceudev, CEU_CETCR, ~status & CEU_CETCR_MAGIC);
> > +   ceu_write(ceudev, CEU_CEIER, CEU_CEIER_MASK);
>
> I wonder why there's a need to disable and reenable interrupts here.

The original driver clearly said "The hardware is -very- picky about
this sequence" and I got scared and nerver touched this. Also, I very
much dislike the CEU_CETRC_MAGIC mask, but again the old driver said
"Acknoledge magical interrupt sources" and I was afraid to change it
(I can rename it though, to something lioke CEU_CETCR_ALL_INT because
that's what it is, a mask with all available interrupt source
enabled).

> > +
> > +static irqreturn_t ceu_irq(int irq, void *data)
> > +{
> > +   struct ceu_device *ceudev = data;
> > +   struct vb2_v4l2_buffer *vbuf;
> > +   struct ceu_buffer *buf;
> > +   int ret;
> > +
> > +   spin_lock(>lock);
> > +   vbuf = ceudev->active;
> > +   if (!vbuf)
> > +   /* Stale interrupt from a released buffer */
> > +   goto out;
>
> Shouldn't you at least clear the interrupt source (done at the beginning of
> the ceu_capture() function) in this case ? I'd move the handling of the
> interrupt status from ceu_capture() to here and pass the status to the capture
> function. Or even handle the status here completely, as status handling isn't
> needed when ceu_capture() is called from ceu_start_streaming().

I'll try to move interrupt management here, and use flags to tell to
ceu_capture() what happened

>
> > +   /* Prepare a new 'active' buffer and trigger a new capture */
> > +   buf = vb2_to_ceu(vbuf);
> > +   vbuf->vb2_buf.timestamp = ktime_get_ns();
> > +
> > +   if (!list_empty(>capture)) {
> > +   buf = list_first_entry(>capture, struct ceu_buffer,
> > +  queue);
> > +   list_del(>queue);
> > +   ceudev->active = >vb;
> > +   } else {
> > +   ceudev->active = NULL;
> > +   }
> > +
> > +   /*
> > +* If the new capture started successfully, mark the previous buffer
> > +* as "DONE".
> > +*/
> > +   ret = ceu_capture(ceudev);
> > +   if (!ret) {
> > +   vbuf->field = ceudev->field;
> > +   vbuf->sequence = ceudev->sequence++;
>
> Shouldn't you set the 

Re: [PATCH 3/4] kms++util: Add verification module

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Dec 18, 2017 at 12:50 PM, Laurent Pinchart
 wrote:
> On Monday, 18 December 2017 13:46:22 EET Tomi Valkeinen wrote:
>> On 18/12/17 13:36, Laurent Pinchart wrote:
>> > The problem with PNG (or any other format really) is that you not only
>> > need to encode the image into the target format (PNG or JPG would require
>> > external libraries, simpler formats such as BMP or PNM could be handled
>> > internally), but you also need to convert the image to a particular RGB
>> > or YUV format depending on what the output format requires.
>>
>> Yes, that's the "hassle" part I was referring to =).
>>
>> Maybe we need to invent a new file format that can store all kinds of
>> formats? (https://xkcd.com/927/)
>>
>> > If you want to do so, I would like to reuse code from the v4l2convert
>> > library. The code should be moved to a library that doesn't depend on
>> > V4L2, as the current API encapsulate conversion in other operations.
>> > Other tools such as raw2rgbpnm could then be ported to use that library;
>>
>> Yep, so I agree that doing color format conversions shouldn't really be
>> kms++'s job, but at the same time I'd like to be able to export
>> framebuffers.
>>
>> And I'm fine with adding dependencies to kms++, as long as all those are
>> optional.
>>
>> Aren't there "big" image conversion libraries that would do the job?
>> Imagemagick? Or something? I have used "convert" command to do some
>> conversions, that comes from ImageMagick.
>
> That's an option too. I had a look at the code once to find out how
> ImageMagick was performing scaling and gave up with a headache soon
> afterwards. We need more formats than what ImageMagick currently supports
> (it's mostly focused on image file formats instead of raw image formats), with
> all kind of RGB, YUV (and ideally Bayer) formats, and I don't think extending
> ImageMagick would be the way to go.

netpbm is your friend.

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 3/4] kms++util: Add verification module

[Tomi Valkeinen]

On 18/12/17 13:50, Laurent Pinchart wrote:


That's an option too. I had a look at the code once to find out how
ImageMagick was performing scaling and gave up with a headache soon
afterwards. We need more formats than what ImageMagick currently supports
(it's mostly focused on image file formats instead of raw image formats), with
all kind of RGB, YUV (and ideally Bayer) formats, and I don't think extending
ImageMagick would be the way to go.


Yep, I can image that ImageMagick can't handle it all. I don't really 
have much experience with it. I did find that it has enough tunables to 
manage rgbx with different byte orders and the "extra" alpha channel:


convert -depth 8 -size $1x$2 -color-matrix "0 0 1 0 1 0 1 0 0" -alpha 
deactivate rgba:$3 png:-


But things like argb1555 or yuv422 might prove to be too difficult for 
ImageMagick.


Where's the source for v4l2convert? What does it do?

I wonder how difficult it would be to create a 
brute-force-no-optimizations style of a function to which you give the 
exact bit definition of the buffer's pixel format, and which gives you 
RGB888 or YUV444, depending on the input format (I presume most image 
encoders would accept RGB888 & YUV444).


I think the function wouldn't even need to care whether the data is RGB 
or YUV, it would just unpack it according to the format definition.


 Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 3/4] kms++util: Add verification module

[Laurent Pinchart]

Hi Tomi,

On Monday, 18 December 2017 13:46:22 EET Tomi Valkeinen wrote:
> On 18/12/17 13:36, Laurent Pinchart wrote:
> > The problem with PNG (or any other format really) is that you not only
> > need to encode the image into the target format (PNG or JPG would require
> > external libraries, simpler formats such as BMP or PNM could be handled
> > internally), but you also need to convert the image to a particular RGB
> > or YUV format depending on what the output format requires.
> 
> Yes, that's the "hassle" part I was referring to =).
> 
> Maybe we need to invent a new file format that can store all kinds of
> formats? (https://xkcd.com/927/)
> 
> > If you want to do so, I would like to reuse code from the v4l2convert
> > library. The code should be moved to a library that doesn't depend on
> > V4L2, as the current API encapsulate conversion in other operations.
> > Other tools such as raw2rgbpnm could then be ported to use that library;
> 
> Yep, so I agree that doing color format conversions shouldn't really be
> kms++'s job, but at the same time I'd like to be able to export
> framebuffers.
> 
> And I'm fine with adding dependencies to kms++, as long as all those are
> optional.
> 
> Aren't there "big" image conversion libraries that would do the job?
> Imagemagick? Or something? I have used "convert" command to do some
> conversions, that comes from ImageMagick.

That's an option too. I had a look at the code once to find out how 
ImageMagick was performing scaling and gave up with a headache soon 
afterwards. We need more formats than what ImageMagick currently supports 
(it's mostly focused on image file formats instead of raw image formats), with 
all kind of RGB, YUV (and ideally Bayer) formats, and I don't think extending 
ImageMagick would be the way to go.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] kms++util: Add verification module

[Tomi Valkeinen]

On 16/12/17 18:13, Kieran Bingham wrote:


+void save_raw_frame(IFramebuffer& fb, const char *filename)
+{
+    unique_ptr os;
+    os = unique_ptr(new ofstream(filename, ofstream::binary));
+
+    for (unsigned i = 0; i < fb.num_planes(); ++i)
+    os->write((char*)fb.map(i), fb.size(i));
+}


You don't need any of that unique_ptr stuff here. I needed it as the code needed
to handle the case where we don't save, i.e. os = null.


Ah OK - I thought it was providing the hook up to automatically close the stream
at the end of the function.

I guess an explicit close would be just as clean :)


You don't need an explicit close. The ofstream instance will be 
destructed automatically, and thus the stream will be closed.


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 3/4] kms++util: Add verification module

[Tomi Valkeinen]

On 18/12/17 13:36, Laurent Pinchart wrote:


The problem with PNG (or any other format really) is that you not only need to
encode the image into the target format (PNG or JPG would require external
libraries, simpler formats such as BMP or PNM could be handled internally),
but you also need to convert the image to a particular RGB or YUV format
depending on what the output format requires.


Yes, that's the "hassle" part I was referring to =).

Maybe we need to invent a new file format that can store all kinds of 
formats? (https://xkcd.com/927/)



If you want to do so, I would like to reuse code from the v4l2convert library.
The code should be moved to a library that doesn't depend on V4L2, as the
current API encapsulate conversion in other operations. Other tools such as
raw2rgbpnm could then be ported to use that library;


Yep, so I agree that doing color format conversions shouldn't really be 
kms++'s job, but at the same time I'd like to be able to export 
framebuffers.


And I'm fine with adding dependencies to kms++, as long as all those are 
optional.


Aren't there "big" image conversion libraries that would do the job? 
Imagemagick? Or something? I have used "convert" command to do some 
conversions, that comes from ImageMagick.


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 4/4] kms++util: Add frame compare functionality

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 14 December 2017 01:10:12 EET Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Provide a means to compare two identically sized framebuffers.
> 
> This basic implementation expects the two buffers to have the same
> formats and sizes, and will return zero for identical frames, or a
> positive float representing and average difference otherwise.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  kms++util/inc/kms++util/kms++util.h |  1 +-
>  kms++util/src/verification.cpp  | 31 ++-
>  py/pykms/pykmsutil.cpp  |  2 ++-
>  3 files changed, 34 insertions(+)
> 
> diff --git a/kms++util/inc/kms++util/kms++util.h
> b/kms++util/inc/kms++util/kms++util.h index 431de0bb159a..753cee189451
> 100644
> --- a/kms++util/inc/kms++util/kms++util.h
> +++ b/kms++util/inc/kms++util/kms++util.h
> @@ -29,6 +29,7 @@ void draw_color_bar(IFramebuffer& buf, int old_xpos, int
> xpos, int width); void draw_test_pattern(IFramebuffer , YUVType yuvt =
> YUVType::BT601_Lim);
> 
>  void save_raw_frame(IFramebuffer& fb, const char *filename);
> +float compare_framebuffers(IFramebuffer& a, IFramebuffer& b);
>  }
> 
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> diff --git a/kms++util/src/verification.cpp b/kms++util/src/verification.cpp
> index 3210bb144d2b..a46d6f924095 100644
> --- a/kms++util/src/verification.cpp
> +++ b/kms++util/src/verification.cpp
> @@ -18,4 +18,35 @@ void save_raw_frame(IFramebuffer& fb, const char
> *filename) os->write((char*)fb.map(i), fb.size(i));
>  }
> 
> +float compare_framebuffers(IFramebuffer& a, IFramebuffer& b)
> +{
> + unsigned int i;
> + unsigned int pixels = a.width() * a.height();
> + uint8_t *pa = a.map(0);
> + uint8_t *pb = b.map(0);
> +
> + float diff = 0;
> +
> + if (a.format() != b.format())
> + throw std::invalid_argument("Pixel formats differ...");
> +
> + if ((a.width() != b.width() ||
> + (a.height() != b.height(
> + throw std::invalid_argument("Frame sizes differ...");
> +
> + // Formats are identical, so num_planes are already identical
> + for (i = 0; i < a.num_planes(); i++) {
> + if ((a.offset(i) != b.offset(i)) ||
> + (a.stride(i) != b.stride(i)))
> + throw std::invalid_argument("Planes differ...");
> + }

Is this required ? Why can't we compare two image of identical size stored in 
frame buffers with different strides ?

> + // Simple byte comparisons to start.
> + // This expects a frame to be identical, including non-visible data.

Do we need to compare the non-visible data ?

> + for (i = 0; i < a.size(0) && i < b.size(0); i++)
> + diff += abs(pa[i] - pb[i]);
> +
> + return diff / pixels;
> +}
> +
>  }
> diff --git a/py/pykms/pykmsutil.cpp b/py/pykms/pykmsutil.cpp
> index 2d741751ba75..b86690a3d306 100644
> --- a/py/pykms/pykmsutil.cpp
> +++ b/py/pykms/pykmsutil.cpp
> @@ -64,4 +64,6 @@ void init_pykmstest(py::module )
>   m.def("save_raw_frame", [](Framebuffer& fb, const char * filename) {
>   save_raw_frame(fb, filename);
>   });
> + m.def("compare_framebuffers", [](Framebuffer& a, Framebuffer& b) {
> + return compare_framebuffers(a, b); } );
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] kms++util: Add verification module

[Laurent Pinchart]

Hi Kieran,

On Saturday, 16 December 2017 18:13:51 EET Kieran Bingham wrote:
> On 15/12/17 13:43, Tomi Valkeinen wrote:
> > On 14/12/17 01:10, Kieran Bingham wrote:
> >> From: Kieran Bingham 
> >> 
> >> Provide a util module to provide helpers involved in validation and
> >> verification of data frames.
> >> 
> >> The first addition is a raw frame binary output with bindings to python
> >> modelled on Tomi's implementation in wbcap.
> > 
> > I don't think verification.cpp is a good name for this. A function to save
> > the fb sounds like a generic fb utility.
> 
> Yes, I was originally going to put it in one of the existing framebuffer
> classes - but then realised they were abstracted classes, and it had to use
> the base type :)
> 
> I was looking at verifying frames, so this was the first name that came to
> my head :D
> 
> > In fact, I'd like to have a helper utility function to save as png, as
> > converting a raw image to png is a bit of a hassle. Then again, we'd need
> > a png library for that, and possibly bigger pieces of code to handle all
> > the different pixel formats...
> 
> So would I ! I'm glad you bring the topic up :)
> 
> Are you happy to bring in external libraries to support such functionality?
> 
> I guess we can make it so the configure scripts select the feature if
> libraries are available or such.
> 
> Being able to save directly to easily viewable file formats would certainly
> ease things, (while still being able to save raw files for some manual
> verifications)

The problem with PNG (or any other format really) is that you not only need to 
encode the image into the target format (PNG or JPG would require external 
libraries, simpler formats such as BMP or PNM could be handled internally), 
but you also need to convert the image to a particular RGB or YUV format 
depending on what the output format requires.

If you want to do so, I would like to reuse code from the v4l2convert library. 
The code should be moved to a library that doesn't depend on V4L2, as the 
current API encapsulate conversion in other operations. Other tools such as 
raw2rgbpnm could then be ported to use that library;

> > So, a function to save the raw bits is fine, but how about "fbutils.cpp"
> > or such?
> 
> Yes, that sounds reasonable.
> 
> >> Signed-off-by: Kieran Bingham 
> >> ---
> >>   kms++util/inc/kms++util/kms++util.h |  2 ++
> >>   kms++util/src/verification.cpp  | 21 +
> >>   py/pykms/pykmsutil.cpp  |  5 +
> >>   3 files changed, 28 insertions(+)
> >>   create mode 100644 kms++util/src/verification.cpp
> >> 
> >> diff --git a/kms++util/inc/kms++util/kms++util.h
> >> b/kms++util/inc/kms++util/kms++util.h
> >> index 8e45b0df3cde..431de0bb159a 100644
> >> --- a/kms++util/inc/kms++util/kms++util.h
> >> +++ b/kms++util/inc/kms++util/kms++util.h
> >> @@ -27,6 +27,8 @@ void draw_text(IFramebuffer& buf, uint32_t x, uint32_t
> >> y,
> >> const std::string& str
> >>   void draw_color_bar(IFramebuffer& buf, int old_xpos, int xpos, int
> >> width); void draw_test_pattern(IFramebuffer , YUVType yuvt =
> >> YUVType::BT601_Lim); +
> >> +void save_raw_frame(IFramebuffer& fb, const char *filename);
> >>   }
> >> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> >> diff --git a/kms++util/src/verification.cpp
> >> b/kms++util/src/verification.cpp new file mode 100644
> >> index ..3210bb144d2b
> >> --- /dev/null
> >> +++ b/kms++util/src/verification.cpp
> >> @@ -0,0 +1,21 @@
> >> +
> >> +#include 
> >> +#include 
> >> +
> >> +#include 
> >> +
> >> +using namespace std;
> >> +
> >> +namespace kms
> >> +{
> >> +
> >> +void save_raw_frame(IFramebuffer& fb, const char *filename)
> >> +{
> >> +unique_ptr os;
> >> +os = unique_ptr(new ofstream(filename, ofstream::binary));
> >> +
> >> +for (unsigned i = 0; i < fb.num_planes(); ++i)
> >> +os->write((char*)fb.map(i), fb.size(i));
> >> +}
> > 
> > You don't need any of that unique_ptr stuff here. I needed it as the code
> > needed to handle the case where we don't save, i.e. os = null.
> 
> Ah OK - I thought it was providing the hook up to automatically close the
> stream at the end of the function.
> 
> I guess an explicit close would be just as clean :)
> 
> As the commit message suggests, this was a direct copy paste from your
> implementation after I saw it as a better implementation than my previous
> 'C' version. (I don't spend a lot of time in C++ land)
> 
> > And I'm not fond of the function name, "frame" doesn't sound good. Maybe
> > rather save_raw_fb(). Or save_fb_raw(), so in the future we might have
> > also save_fb_png().
> 
> Agreed, sounds fine. I'm definitely looking forward to a save_fb_png().
> 
> >> +
> >> +}
> >> diff --git a/py/pykms/pykmsutil.cpp b/py/pykms/pykmsutil.cpp
> >> index 518d5eaa88f0..2d741751ba75 100644
> >> --- a/py/pykms/pykmsutil.cpp
> >> +++ 

Re: [PATCH 2/4] py: pyvid: Provide stream_off binding

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 14 December 2017 01:10:10 EET Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> The videodevice module defines a stream_off function call, but this is
> not available in the python bindings interface.
> 
> Provide the binding of VideoStreamer::stream_off.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  py/pykms/pyvid.cpp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/py/pykms/pyvid.cpp b/py/pykms/pyvid.cpp
> index 6a6080e4b2b7..92006c404038 100644
> --- a/py/pykms/pyvid.cpp
> +++ b/py/pykms/pyvid.cpp
> @@ -34,5 +34,6 @@ void init_pyvid(py::module )
>   .def("queue", ::queue)
>   .def("dequeue", ::dequeue)
>   .def("stream_on", ::stream_on)
> + .def("stream_off", ::stream_off)

I've always wondered why V4L2 has two ioctls for stream on and stream off, 
when a single one with an on/off parameter would be perfectly fine. I'm thus 
tempted to implement stream control through a single function, but that's a 
matter of taste. Feel free to give it a go if you want, otherwise

Acked-by: Laurent Pinchart 

>   ;
>  }

-- 
Regards,

Laurent Pinchart

RE: [PATCH 1/5] ARM: dts: iwg20d-q7-common: Sound PIO support

[Biju Das]

Hi Simon,

Thanks. I will send v2.

Regards,
Biju

> -Original Message-
> From: Simon Horman [mailto:ho...@verge.net.au]
> Sent: 18 December 2017 11:22
> To: Biju Das 
> Cc: Rob Herring ; Mark Rutland
> ; Kuninori Morimoto
> ; Magnus Damm
> ; Chris Paterson ;
> Fabrizio Castro ;
> devicet...@vger.kernel.org; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 1/5] ARM: dts: iwg20d-q7-common: Sound PIO support
>
> On Fri, Dec 15, 2017 at 02:50:49PM +, Biju Das wrote:
> > Enable sound PIO support on carrier board.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Fabrizio Castro 
> > ---
> >  arch/arm/boot/dts/iwg20d-q7-common.dtsi | 46
> > +
>
> It appears that the nodes in this file are in (or close to in) alphabetical 
> order.
> Please add new nodes in a manner that preserves that. If necessary please
> provide a clean-up patch to shift nodes into alphabetical order.


[https://www2.renesas.eu/media/email/unicef_2017.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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

Re: [PATCH 1/4] videodevice: Fix minor spacing

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 14 December 2017 01:10:09 EET Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Provide a space between the return type and the function definition
> 
> Signed-off-by: Kieran Bingham 

Acked-by: Laurent Pinchart 

> ---
>  kms++util/src/videodevice.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kms++util/src/videodevice.cpp b/kms++util/src/videodevice.cpp
> index e21916bcd06b..efe1678d031e 100644
> --- a/kms++util/src/videodevice.cpp
> +++ b/kms++util/src/videodevice.cpp
> @@ -436,7 +436,7 @@ void VideoStreamer::queue(DumbFramebuffer* fb)
>   v4l2_queue_dmabuf(m_fd, idx, fb, get_buf_type(m_type));
>  }
> 
> -DumbFramebuffer*VideoStreamer::dequeue()
> +DumbFramebuffer* VideoStreamer::dequeue()
>  {
>   uint32_t idx = v4l2_dequeue(m_fd, get_buf_type(m_type));

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/5] ARM: dts: iwg20d-q7-common: Sound PIO support

[Simon Horman]

On Fri, Dec 15, 2017 at 02:50:49PM +, Biju Das wrote:
> Enable sound PIO support on carrier board.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> ---
>  arch/arm/boot/dts/iwg20d-q7-common.dtsi | 46 
> +

It appears that the nodes in this file are in (or close to in) alphabetical
order. Please add new nodes in a manner that preserves that. If necessary
please provide a clean-up patch to shift nodes into alphabetical order.

Re: [PATCH v2 3/3] soc: renesas: rcar-sysc: Keep wakeup sources active during system suspend

[Geert Uytterhoeven]

On Thu, Dec 14, 2017 at 3:11 PM, Ulf Hansson  wrote:
> On 9 November 2017 at 14:27, Geert Uytterhoeven  
> wrote:
>> If an R-Car SYSC slave device is part of the CPG/MSTP or CPG/MSSR Clock
>> Domain and to be used as a wakeup source, it must be kept active during
>> system suspend.
>>
>> Currently this is handled in device-specific drivers by explicitly
>> increasing the use count of the module clock when the device is
>> configured as a wakeup source.  However, the proper way to prevent the
>> device from being stopped is to inform this requirement to the genpd
>> core, by setting the GENPD_FLAG_ACTIVE_WAKEUP flag.
>>
>> Note that this will only affect devices configured as wakeup sources.
>>
>> Signed-off-by: Geert Uytterhoeven 
>
> Reviewed-by: Ulf Hansson 

Thank you!

Simon: can you please queue this up for v4.16?
I have queued the other 2 clock patches in clk-renesas-for-v4.16, and sent
a PR for it.
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 2/3] ARM: dts: r8a7743: Add CMT SoC specific support

[Simon Horman]

On Wed, Dec 13, 2017 at 10:15:39AM +, Fabrizio Castro wrote:
> Hello Geert,
> 
> thank you for your feedback.
> 
> > Hi Fabrizio,
> >
> > On Wed, Dec 13, 2017 at 10:42 AM, Fabrizio Castro
> >  wrote:
> > >> On Tue, Dec 12, 2017 at 06:49:38PM +, Fabrizio Castro wrote:
> > >> > Add CMT[01] support to SoC DT.
> > >> > Signed-off-by: Fabrizio Castro 
> > >> > Reviewed-by: Biju Das 
> > >> > ---
> > >> >  arch/arm/boot/dts/r8a7743.dtsi | 30 ++
> > >> >  1 file changed, 30 insertions(+)
> > >>
> > >> I was expecting the cmt nodes to be "disabled" in the SoC file
> > >> and then enabled selectively in board files. Am I missing something?
> > >
> > > Since this component is just a compare and match timer, I  thought there 
> > > was no harm in enabling it by default in the SoC specific DT.
> > > The system will park it and leave its clock disabled until actually 
> > > needed for something.
> > > The user can still disable it in the board specific DT if he/she doesn't 
> > > mean to even have the option to use it. Do you prefer I left it
> > disabled by default?
> >
> > It's debatable (thus up to Simon the maintainer ;-).
> > For I/O devices, we disable them in the SoC .dtsi file.
> > For core infrastructure like interrupt, DMA, and GPIO controllers, we keep
> > them enabled.
> >
> > Timers are core functionality, but who's actually using these timers?
> 
> I don't have a use case in mind unfortunately, but it's still core
> functionality and pretty harmless as far as I can tell. Let's see what
> Simon thinks about this.

On Renesas SoCs we have a bit more flexibility with timers than might
be the case on other SoCs. Thus I'd like to keep with the scheme of
disabling them in .dtsi and enabling them when they are needed.

Please update the patches.

Re: [PATCH 5/6] pinctrl: sh-pfc: r8a7791: Add tpu groups and function

[Geert Uytterhoeven]

Hi Fabrizio,

On Thu, Dec 14, 2017 at 11:56 AM, Fabrizio Castro
 wrote:
> This patch adds tpu groups and function to r8a7743/r8a7791/r8a7793.
>
> Signed-off-by: Fabrizio Castro 
> Reviewed-by: Biju Das 

With the sort order fixed:
Reviewed-by: Geert Uytterhoeven 

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> @@ -3892,6 +3892,32 @@ static const unsigned int sdhi2_wp_mux[] = {
> SD2_WP_MARK,
>  };
>
> +/* - TPU 
>  */
> +static const unsigned int tpu_to0_pins[] = {
> +   RCAR_GP_PIN(6, 14),
> +};
> +static const unsigned int tpu_to0_mux[] = {
> +   TPU_TO0_MARK,
> +};
> +static const unsigned int tpu_to1_pins[] = {
> +   RCAR_GP_PIN(1, 17),
> +};
> +static const unsigned int tpu_to1_mux[] = {
> +   TPU_TO1_MARK,
> +};
> +static const unsigned int tpu_to2_pins[] = {
> +   RCAR_GP_PIN(1, 18),
> +};
> +static const unsigned int tpu_to2_mux[] = {
> +   TPU_TO2_MARK,
> +};
> +static const unsigned int tpu_to3_pins[] = {
> +   RCAR_GP_PIN(1, 24),
> +};
> +static const unsigned int tpu_to3_mux[] = {
> +   TPU_TO3_MARK,
> +};
> +
>  /* - SSI 
>  */
>  static const unsigned int ssi0_data_pins[] = {
> /* SDATA */

Please use alphabetical sort order (also below).

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 3/4] pinctrl: sh-pfc: r8a7794: Add tpu groups and function

[Geert Uytterhoeven]

On Mon, Dec 18, 2017 at 12:09 PM, Geert Uytterhoeven
 wrote:
> On Thu, Dec 14, 2017 at 11:57 AM, Fabrizio Castro
>  wrote:
>> This patch adds tpu groups and function to r8a7745/r8a7794.
>>
>> Signed-off-by: Fabrizio Castro 
>> Reviewed-by: Biju Das 
>
> Reviewed-by: Geert Uytterhoeven 
> i.e. will queue in sh-pfc-for-v4.16.

Oops, missed one thing: please use alphabetical sort order when adding
new entries.

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 0/5] Add Sound support for iWave RZ/G1M board

[Simon Horman]

On Wed, Dec 13, 2017 at 09:27:30AM +, Biju Das wrote:
> Hi Simon,
> 
> Thanks.
> 
> There is a typo in the documentation dependency link.
> The correct one is https://patchwork.kernel.org/patch/10108015/

Thanks, series applied.

Re: [PATCH 3/4] pinctrl: sh-pfc: r8a7794: Add tpu groups and function

[Geert Uytterhoeven]

On Thu, Dec 14, 2017 at 11:57 AM, Fabrizio Castro
 wrote:
> This patch adds tpu groups and function to r8a7745/r8a7794.
>
> Signed-off-by: Fabrizio Castro 
> Reviewed-by: Biju Das 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.16.

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/4] pinctrl: sh-pfc: r8a7794: Add PWM[0123456] support

[Geert Uytterhoeven]

On Thu, Dec 14, 2017 at 11:57 AM, Fabrizio Castro
 wrote:
> This patch adds PFC PWM[0123456] pin groups and functions, enabling
> PWM on the r8a7794 and r8a7745.
>
> Signed-off-by: Fabrizio Castro 
> Reviewed-by: Biju Das 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.16.

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 2/3] arm64: dts: renesas: r8a7796: move nodes which have no reg property out of bus

[Simon Horman]

On Wed, Dec 13, 2017 at 09:34:15AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Wed, Dec 13, 2017 at 9:25 AM, Simon Horman  wrote:
> > For now I've applied the following:
> >
> > From: Simon Horman 
> > Date: Tue, 12 Dec 2017 09:24:35 +0100
> > Subject: [PATCH] arm64: dts: renesas: r8a7796: move nodes which have no reg
> >  property out of bus
> >
> > Move pmu_a5[73], timer and thermal-zones nodes from soc node to root node.
> > The nodes that have been moved do not have any register properties and thus
> > shouldn't be placed on the bus.
> 
> > index c1b0d0344329..e82b4db1ad1a 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > @@ -154,6 +154,23 @@
> > clock-frequency = <0>;
> > };
> >
> > +   pmu_a57 {
> > +   compatible = "arm,cortex-a57-pmu";
> > +   interrupts-extended = < GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
> > + < GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
> > +   interrupt-affinity = <_0>,
> > +<_1>;
> 
> Merge?

Sure, sorry for missing that.

> 
> > +   };
> > +
> > +   pmu_a53 {
> > +   compatible = "arm,cortex-a53-pmu";
> > +   interrupts-extended = < GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>,
> > + < GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>,
> > + < GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
> > + < GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> > +   interrupt-affinity = <_0>, <_1>, <_2>, <_3>;
> 
> Thanks!
> 
> > +   };
> 
> > @@ -2027,4 +1968,56 @@
> > resets = < 822>;
> > };
> > };
> > +
> > +   timer {
> > +   compatible = "arm,armv8-timer";
> > +   interrupts-extended = < GIC_PPI 13 
> > (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> > + < GIC_PPI 14 
> > (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> > + < GIC_PPI 11 
> > (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> > + < GIC_PPI 10 
> > (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
> 
> Much better!
> 
> Or even
> 
> > +   interrupts-extended =
> > +   < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | 
> > IRQ_TYPE_LEVEL_LOW)>,
> > +   < GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | 
> > IRQ_TYPE_LEVEL_LOW)>,
> > +   < GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | 
> > IRQ_TYPE_LEVEL_LOW)>,
> > +   < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | 
> > IRQ_TYPE_LEVEL_LOW)>;
> 
> I don't care that much, it extends beyond 80 characters anyway.

As its over 80 characters anyway lets go with my version.

Re: [PATCH 4/6] dt-bindings: pwm: renesas-tpu: Document r8a774[35] support

[Geert Uytterhoeven]

On Thu, Dec 14, 2017 at 11:56 AM, Fabrizio Castro
 wrote:
> Document r8a774[35] specific compatible strings. No driver change is
> needed as the fallback compatible string "renesas,tpu" activates the
> right code in the driver.
>
> Signed-off-by: Fabrizio Castro 
> Reviewed-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 2/6] dt-bindings: pwm: rcar: Document r8a774[35] PWM bindings

[Geert Uytterhoeven]

On Thu, Dec 14, 2017 at 11:56 AM, Fabrizio Castro
 wrote:
> This patch adds compatible strings specific to r8a774[35], no driver
> change is needed as the fallback compatible string will activate the
> right code. Also, this patch replaces the example with a DT snippet used
> for adding PWM0 support to an r8a7743 based platform.
>
> Signed-off-by: Fabrizio Castro 
> Reviewed-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 4/4] arm64: dts: r8a7796: Add DEVFREQ support for GSX

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 15, 2017 at 4:14 PM, Ulrich Hecht
 wrote:
> From: Gaku Inami 
>
> This patch supports GSX frequency scaling using devfreq.
>
> Includes update of the OPP tables for GSX because the typical voltage of
> VDD_DVFS (power optimized) was updated to 0.83V by R-Car Gen3 EC Rev0.54E
> specification.
>
> Signed-off-by: Gaku Inami 
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Hien Dang 
> Signed-off-by: Ulrich Hecht 

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -166,6 +166,27 @@
> };
> };
>
> +   gsx_opp_table: gsx_opp_table {
> +   compatible = "operating-points-v2";
> +
> +   opp@2 {

opp-2 (try "make dtbs W=1")

> +   opp-hz = /bits/ 64 <2>;
> +   opp-microvolt = <83>;
> +   };
> +   opp@3 {
> +   opp-hz = /bits/ 64 <3>;
> +   opp-microvolt = <83>;
> +   };
> +   opp@4 {
> +   opp-hz = /bits/ 64 <4>;
> +   opp-microvolt = <83>;
> +   };
> +   opp@6 {
> +   opp-hz = /bits/ 64 <6>;
> +   opp-microvolt = <83>;

Do they really all use the same voltage?

And AFAIU, DVFS is shared between CPU and GPU.

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 3/4] arm64: dts: r8a7796: Add GSX device node

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 15, 2017 at 4:14 PM, Ulrich Hecht
 wrote:
> From: Takeshi Kihara 
>
> This patch adds GSX device node for R8A7796 SoC.
>
> Signed-off-by: Takeshi Kihara 

SoB?

> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -2241,6 +2241,14 @@
> };
> };
>
> +   gsx: gsx@fd00 {

gpu@fd00?

> +   compatible = "renesas,gsx";

That compatible value sounds a bit simplistic to me.
In addition, it's not yet documented.

> +   reg = <0 0xfd00 0 0x3>;

reg = <0 0xfd00 0 0x4>;

> +   interrupts = ;
> +   clocks = < CPG_MOD 112>;

According to Figure 23.2, there's an incoming reset signal, but it's not
listed in Table 8A.28.

> +   power-domains = < R8A7796_PD_3DG_B>;

iommus?

> +   };
> +
> pciec0: pcie@fe00 {
> /* placeholder */
> };

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 2/4] clk: renesas: rcar-gen3: Add ZG clock divider support

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 15, 2017 at 4:14 PM, Ulrich Hecht
 wrote:
> From: Takeshi Kihara 
>
> This patch adds ZG clock divider support for R-Car Gen3 SoC.
>
> Signed-off-by: Takeshi Kihara 

SoB?

> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -73,6 +73,8 @@ static void cpg_simple_notifier_register(struct 
> raw_notifier_head *notifiers,
>   */
>  #define CPG_FRQCRB 0x0004
>  #define CPG_FRQCRB_KICKBIT(31)
> +#define CPG_FRQCRB_ZGFC_MASK   (0x1f << 24)
> +#define CPG_FRQCRB_ZGFC_SHIFT  24
>  #define CPG_FRQCRC 0x00e0
>  #define CPG_FRQCRC_ZFC_MASKGENMASK(12, 8)
>  #define CPG_FRQCRC_Z2FC_MASK   GENMASK(4, 0)
> @@ -197,6 +199,120 @@ static struct clk * __init cpg_z_clk_register(const 
> char *name,
> return clk;
>  }
>
> +static unsigned long cpg_zg_clk_recalc_rate(struct clk_hw *hw,
> +   unsigned long parent_rate)
> +{

I think this can be consolidated with the Z/Z2 clock support, like Simon
did for the Z2 clock:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git/commit/?h=topic/rcar-gen3-z-clk-v3=fda8dbe29dd1f04b61c443147a106962cf5d3f14

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/4] clk: renesas: r8a7796: Add ZG clock

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 15, 2017 at 4:14 PM, Ulrich Hecht
 wrote:
> From: Takeshi Kihara 
>
> This patch adds ZG clock for R8A7796 SoC.
>
> Signed-off-by: Takeshi Kihara 

Forgot to add you're own SoB?

> --- a/drivers/clk/renesas/r8a7796-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
> @@ -76,6 +76,7 @@ static const struct cpg_core_clk r8a7796_core_clks[] 
> __initconst = {
> /* Core Clock Outputs */
> DEF_BASE("z",   R8A7796_CLK_Z, CLK_TYPE_GEN3_Z, CLK_PLL0),
> DEF_BASE("z2",  R8A7796_CLK_Z2,CLK_TYPE_GEN3_Z2, 
> CLK_PLL2),
> +   DEF_FIXED("zg", R8A7796_CLK_ZG,CLK_PLL4,   4, 1),

Shouldn't this be a variable clock of type CLK_TYPE_GEN3_ZG?
What else is the purpose of "[PATCH 2/4] clk: renesas: rcar-gen3: Add ZG
clock divider support"?

> @@ -117,6 +118,7 @@ static const struct cpg_core_clk r8a7796_core_clks[] 
> __initconst = {
>  };
>
>  static const struct mssr_mod_clk r8a7796_mod_clks[] __initconst = {
> +   DEF_MOD("3dge",  112,   R8A7796_CLK_ZG),
> DEF_MOD("scif5", 202,   R8A7796_CLK_S3D4),
> DEF_MOD("scif4", 203,   R8A7796_CLK_S3D4),
> DEF_MOD("scif3", 204,   R8A7796_CLK_S3D4),
> --

This part is fine.

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] tty: serial: sh-sci: Add default for number of ports for compile-testing

[Geert Uytterhoeven]

When compile-testing an allmodconfig kernel for a platform without
sh-sci serial ports, the SERIAL_SH_SCI_NR_UARTS symbol of type "int"
doesn't get assigned a numerical default value, but an empty string,
leading to a build failure:

   .config:3814:warning: symbol value '' invalid for SERIAL_SH_SCI_NR_UARTS
   ...
   make[3]: *** [silentoldconfig] Error 1

Fix this by explicitly providing a default value of 2, like before.

Reported-by: kbuild test robot 
Fixes: f6731485a51978ca ("tty: serial: sh-sci: Hide number of ports config 
question")
Signed-off-by: Geert Uytterhoeven 
---
 drivers/tty/serial/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4e6dfb0a762b5807..3682fd3e960cbd64 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -766,6 +766,7 @@ config SERIAL_SH_SCI_NR_UARTS
default "3" if H8300
default "10" if SUPERH
default "18" if ARCH_RENESAS
+   default "2"
 
 config SERIAL_SH_SCI_CONSOLE
bool "Support for console on SuperH SCI(F)" if EXPERT
-- 
2.7.4

Re: [PATCH v4 2/2] drm: rcar-du: calculate DPLLCR to be more small jitter

[Laurent Pinchart]

Hi Morimoto-san,

On Monday, 18 December 2017 10:38:19 EET Kuninori Morimoto wrote:
> Hi Laurent
> 
> Thank you for your feedback
> 
> >> +   * To be small jitter,
> > 
> > Nitpicking, I would write this "to minimize the jitter".
> 
> (snip)
> 
> >> +   * This code is assuming "used" from 64bit CPU only,
> >> +   * not from 32bit CPU. But both can compile correctly
> > 
> > Nitpicking again, I would write this "This code only runs on 64-bit
> > architectures, the unsigned long type can thus be used for 64-bit
> > computation. It will still compile without any warning on 32-bit
> > architectures."
> 
> I will follow your English ;)
> 
> >> +  /*
> >> +   *  fvco= fin * P *  N / M
> >> +   *  fclkout = fin  * N / M / FDPLL
> >> +   *
> >> +   * To avoid duplicate calculation, let's use below
> >> +   *
> >> +   *  finnm   = fin * N / M
> > 
> > This is called fout in your diagram above, I would use the same name here.
> 
> Oops indeed. I didn't notice
> 
> >> +  unsigned long finnm = input * (n + 1) / (m + 1);
> >> +  unsigned long fvco  = finnm * 2;
> >> +
> >> +  if (fvco < 2000 || fvco > 4096 * 1000 * 1000U)
> >> +  continue;
> > 
> > How about
> > 
> > if (fvco < 1000 || fvco > 2048 * 1000 * 1000)
> > 
> > to avoid computing the intermediate fvco variable ?
> 
> I think you want to say
> 
>   - if (fvco < 1000 || fvco > 2048 * 1000 * 1000)
>   + if (fout < 1000 || fout > 2048 * 1000 * 1000)

Yes, sorry, that's what I meant.

> Actually I notcied about this, but I thought it makes
> user confuse. Thus, I kept original number.
> 
> I'm happy if compiler can adjust it automatically,
> if not, I have no objection to modify it but we want to have such comment ?
> Because above comment/explain mentions about "fvco", not "fout".

Sure, I'll add a comment, it's a good point.

> > If you agree with these small changes there's no need to resubmit the
> > patch, I'll modify it when applying, and
> > 
> > Reviewed-by: Laurent Pinchart 
> 
> Thank you for your help

Thank you for the code :-)

-- 
Regards,

Laurent Pinchart