Re: [git pull] clk: renesas: Updates for v4.17

[Stephen Boyd]

Quoting Geert Uytterhoeven (2018-03-02 02:17:52)
> Hi Mike, Stephen,
> 
> The following changes since commit 7928b2cbe55b2a410a0f5c1f154610059c57b1b2:
> 
>   Linux 4.16-rc1 (2018-02-11 15:04:29 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
> tags/clk-renesas-for-v4.17-tag1
> 
> for you to fetch changes up to 7ce36da900c0a2ff4777d9ba51c4f1cb74205463:
> 
>   clk: renesas: cpg-mssr: Add support for R-Car M3-N (2018-02-26 09:13:29 
> +0100)
> 
> 
> clk: renesas: Updates for v4.17
> 
>   - Update legacy DT Kconfig default,
>   - Add support for CPU (Z/Z2) clocks on R-Car H3 and M3-W,
>   - Add support for the watchdog module clocks on R-Car Gen2 and RZ/G1,
>   - Add support for the new R-Car M3-N and V3H SoCs.
> 
> Thanks for pulling!
> 

Thanks. Pulled into clk-next.

Did you need to use clk_readl() or was it just copy-paste? I hope we can
get rid of that function at some point.

[PATCH] arm64: dts: renesas: v3msk: add EtherAVB pins

[Sergei Shtylyov]

Add the (previously omitted) EtherAVB pin data to the V3M Starter Kit
board's device tree.

Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com>

---
The patch is against the 'renesas-devel-20180314-v4.16-rc5' tag of Simon's
'renesas.git repo. It depends on the R8A77970 PFC driver patch adding EtherAVB
pin groups (posted yesterday) in order to work properly...

 arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts |8 
 1 file changed, 8 insertions(+)

Index: renesas/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
===
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
+++ renesas/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
@@ -32,6 +32,9 @@
 };
 
  {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
renesas,no-ether-link;
phy-handle = <>;
phy-mode = "rgmii-id";
@@ -52,6 +55,11 @@
 };
 
  {
+   avb_pins: avb0 {
+   groups = "avb0_mdio", "avb0_rgmii", "avb0_txcrefclk";
+   function = "avb0";
+   };
+
scif0_pins: scif0 {
groups = "scif0_data";
function = "scif0";

[PATCH v2] arm64: dts: renesas: eagle: add EtherAVB pins

[Sergei Shtylyov]

Add the (previously omitted) EtherAVB pin data to the Eagle board's
device tree.

Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com>

---
The patch is against the 'renesas-devel-20180314-v4.16-rc5' tag of Simon's
'renesas.git repo. It depends on the R8A77970 PFC driver patch adding EtherAVB
pin groups (posted yesterday) in order to work properly...

Changes in version 2:
- updated the pin groups according to the recent PFC driver patch;
- updated due to  the patch reordering.

 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts |8 
 1 file changed, 8 insertions(+)

Index: renesas/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
===
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
+++ renesas/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
@@ -34,6 +34,9 @@
 };
 
  {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
renesas,no-ether-link;
phy-handle = <>;
phy-mode = "rgmii-id";
@@ -71,6 +74,11 @@
 };
 
  {
+   avb_pins: avb0 {
+   groups = "avb0_mdio", "avb0_rgmii", "avb0_txcrefclk";
+   function = "avb0";
+   };
+
i2c0_pins: i2c0 {
groups = "i2c0";
function = "i2c0";

Re: [PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

[Sergei Shtylyov]

On 03/14/2018 09:04 PM, jacopo mondi wrote:

>>> Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel
>>> output decoder.
>>>
>>> Signed-off-by: Jacopo Mondi 
>> [...]
>>> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
>>> b/drivers/gpu/drm/bridge/thc63lvd1024.c
>>> new file mode 100644
>>> index 000..4b059c0
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
>>> @@ -0,0 +1,241 @@
[...]
>>> +static void thc63_enable(struct drm_bridge *bridge)
>>> +{
>>> +   struct thc63_dev *thc63 = to_thc63(bridge);
>>> +   struct regulator *vcc;
>>> +   unsigned int i;
>>> +   int ret;
>>> +
>>> +   for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
>>> +   vcc = thc63->vccs[i];
>>> +   if (vcc) {
>>> +   ret = regulator_enable(vcc);
>>> +   if (ret)
>>
>>You hardly need this variable, could do a call right in this *if*.
>>
>> [...]
>>> +error_vcc_enable:
>>> +   dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
>>> +}
>>> +
>>
>>Why not do this instead of *goto* before?
> 
> Well, goto breaks the loop, if I only print out the error message, the
> enable sequence will go on and enable the other regulators.

> I can print out and break, but I don't see that much benefit

   Sorry, I meant that you should *return* there instead of *goto*.

> One thing I could do instead, is not only print out the error message,
> but disable the already enabled regulators if one fails to start.

   Yeah, probably...

[...]
>>> +static int thc63_gpio_init(struct thc63_dev *thc63)
>>> +{
>>> +   thc63->pwdn = devm_gpiod_get_optional(thc63->dev, "pwdn",
>>> + GPIOD_OUT_LOW);
>>> +   if (IS_ERR(thc63->pwdn)) {
>>> +   dev_err(thc63->dev, "Unable to get GPIO \"pwdn\"\n");
>>
>>"pwdn-gpios" maybe?
>>
>>> +   return PTR_ERR(thc63->pwdn);
>>> +   }
>>> +
>>> +   thc63->oe = devm_gpiod_get_optional(thc63->dev, "oe", GPIOD_OUT_LOW);
>>> +   if (IS_ERR(thc63->oe)) {
>>> +   dev_err(thc63->dev, "Unable to get GPIO \"oe\"\n");
>>
>>"oe-gpios" maybe?
> 
> Are you referring to the error message?

   Yes, seems more clear what to look for this way, IMHO...

[...]

MBR, Sergei

Re: [PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

[jacopo mondi]

Hi Sergei,
   thanks for review

On Wed, Mar 14, 2018 at 08:09:52PM +0300, Sergei Shtylyov wrote:
> On 03/13/2018 05:30 PM, Jacopo Mondi wrote:
>
> > Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel
> > output decoder.
> >
> > Signed-off-by: Jacopo Mondi 
> [...]
> > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
> > b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > new file mode 100644
> > index 000..4b059c0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > @@ -0,0 +1,241 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * THC63LVD1024 LVDS to parallel data DRM bridge driver.
> > + *
> > + * Copyright (C) 2018 Jacopo Mondi 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static const char * const thc63_reg_names[] = {
> > +   "vcc", "lvcc", "pvcc", "cvcc", };
>
>Your bracing style is pretty strange -- neither here nor there. Please 
> place };
> on the next line...

Yeah, I had doubt about this.. The most common style I found around is

static const char * const foo[] = {
"bar",
"baz",
"...",
};

But seems really too many lines for a bunch of 4 character strings...

>
> [...]
> > +static void thc63_enable(struct drm_bridge *bridge)
> > +{
> > +   struct thc63_dev *thc63 = to_thc63(bridge);
> > +   struct regulator *vcc;
> > +   unsigned int i;
> > +   int ret;
> > +
> > +   for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> > +   vcc = thc63->vccs[i];
> > +   if (vcc) {
> > +   ret = regulator_enable(vcc);
> > +   if (ret)
>
>You hardly need this variable, could do a call right in this *if*.
>
> [...]
> > +error_vcc_enable:
> > +   dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
> > +}
> > +
>
>Why not do this instead of *goto* before?

Well, goto breaks the loop, if I only print out the error message, the
enable sequence will go on and enable the other regulators.

I can print out and break, but I don't see that much benefit

One thing I could do instead, is not only print out the error message,
but disable the already enabled regulators if one fails to start.

>
> > +static void thc63_disable(struct drm_bridge *bridge)
> > +{
> > +   struct thc63_dev *thc63 = to_thc63(bridge);
> > +   struct regulator *vcc;
> > +   unsigned int i;
> > +   int ret;
> > +
> > +   for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> > +   vcc = thc63->vccs[i];
> > +   if (vcc) {
> > +   ret = regulator_disable(vcc);
> > +   if (ret)
>
>Again, no need for 'ret' whatsoever...
>
> > +   goto error_vcc_disable;
> > +   }
> > +   }
> > +
> > +   if (thc63->pwdn)
> > +   gpiod_set_value(thc63->pwdn, 1);
> > +
> > +   if (thc63->oe)
> > +   gpiod_set_value(thc63->oe, 0);
> > +
> > +   return;
> > +
> > +error_vcc_disable:
> > +   dev_err(thc63->dev, "Failed to disable regulator %u\n", i);
>
>Again, why not do it instead of *goto*?

ditto

>
> [...]
> > +static int thc63_gpio_init(struct thc63_dev *thc63)
> > +{
> > +   thc63->pwdn = devm_gpiod_get_optional(thc63->dev, "pwdn",
> > + GPIOD_OUT_LOW);
> > +   if (IS_ERR(thc63->pwdn)) {
> > +   dev_err(thc63->dev, "Unable to get GPIO \"pwdn\"\n");
>
>"pwdn-gpios" maybe?
>
> > +   return PTR_ERR(thc63->pwdn);
> > +   }
> > +
> > +   thc63->oe = devm_gpiod_get_optional(thc63->dev, "oe", GPIOD_OUT_LOW);
> > +   if (IS_ERR(thc63->oe)) {
> > +   dev_err(thc63->dev, "Unable to get GPIO \"oe\"\n");
>
>"oe-gpios" maybe?

Are you referring to the error message? I can change this, but again, I
see no standards around.

Thanks
   j

>
> [...]
>
> MBR, Sergei


signature.asc
Description: PGP signature

Re: [PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

[Sergei Shtylyov]

On 03/13/2018 05:30 PM, Jacopo Mondi wrote:

> Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel
> output decoder.
> 
> Signed-off-by: Jacopo Mondi 
[...]
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
> b/drivers/gpu/drm/bridge/thc63lvd1024.c
> new file mode 100644
> index 000..4b059c0
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * THC63LVD1024 LVDS to parallel data DRM bridge driver.
> + *
> + * Copyright (C) 2018 Jacopo Mondi 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +static const char * const thc63_reg_names[] = {
> + "vcc", "lvcc", "pvcc", "cvcc", };

   Your bracing style is pretty strange -- neither here nor there. Please place 
};
on the next line...

[...]
> +static void thc63_enable(struct drm_bridge *bridge)
> +{
> + struct thc63_dev *thc63 = to_thc63(bridge);
> + struct regulator *vcc;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> + vcc = thc63->vccs[i];
> + if (vcc) {
> + ret = regulator_enable(vcc);
> + if (ret)

   You hardly need this variable, could do a call right in this *if*.

[...]
> +error_vcc_enable:
> + dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
> +}
> +

   Why not do this instead of *goto* before?
 
> +static void thc63_disable(struct drm_bridge *bridge)
> +{
> + struct thc63_dev *thc63 = to_thc63(bridge);
> + struct regulator *vcc;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> + vcc = thc63->vccs[i];
> + if (vcc) {
> + ret = regulator_disable(vcc);
> + if (ret)

   Again, no need for 'ret' whatsoever...

> + goto error_vcc_disable;
> + }
> + }
> +
> + if (thc63->pwdn)
> + gpiod_set_value(thc63->pwdn, 1);
> +
> + if (thc63->oe)
> + gpiod_set_value(thc63->oe, 0);
> +
> + return;
> +
> +error_vcc_disable:
> + dev_err(thc63->dev, "Failed to disable regulator %u\n", i);

   Again, why not do it instead of *goto*?

[...]
> +static int thc63_gpio_init(struct thc63_dev *thc63)
> +{
> + thc63->pwdn = devm_gpiod_get_optional(thc63->dev, "pwdn",
> +   GPIOD_OUT_LOW);
> + if (IS_ERR(thc63->pwdn)) {
> + dev_err(thc63->dev, "Unable to get GPIO \"pwdn\"\n");

   "pwdn-gpios" maybe?

> + return PTR_ERR(thc63->pwdn);
> + }
> +
> + thc63->oe = devm_gpiod_get_optional(thc63->dev, "oe", GPIOD_OUT_LOW);
> + if (IS_ERR(thc63->oe)) {
> + dev_err(thc63->dev, "Unable to get GPIO \"oe\"\n");

   "oe-gpios" maybe?

[...]

MBR, Sergei

Re: [PATCH 1/3] rcar-vin: remove duplicated check of state in irq handler

[Niklas Söderlund]

Hi Jacopo,

On 2018-03-14 16:17:33 +0100, Jacopo Mondi wrote:
> Hi Niklas, Kieran,
> 
> On Tue, Mar 13, 2018 at 06:56:54PM +0100, Niklas Söderlund wrote:
> > Hi Kieran,
> >
> > Thanks for your feedback.
> >
> > On 2018-03-13 17:42:25 +0100, Kieran Bingham wrote:
> > > Hi Niklas,
> > >
> > > Thanks for the patch series :) - I've been looking forward to seeing this 
> > > one !
> > >
> > > On 10/03/18 01:09, Niklas Söderlund wrote:
> > > > This is an error from when the driver where converted from soc-camera.
> > > > There is absolutely no gain to check the state variable two times to be
> > > > extra sure if the hardware is stopped.
> > >
> > > I'll not say this isn't a redundant check ... but isn't the check against 
> > > two
> > > different states, and thus the remaining check doesn't actually catch the 
> > > case
> > > now where state == STOPPED ?
> >
> > Thanks for noticing this, you are correct. I think I need to refresh my
> > glasses subscription after missing this :-)
> >
> > >
> > > (Perhaps state != RUNNING would be better ?, but I haven't checked the 
> > > rest of
> > > the code)
> >
> > I will respin this in a v2 and either use state != RUNNING or at least
> > combine the two checks to prevent future embarrassing mistakes like
> > this.
> 
> I am sorry I have missed this comment, but I think your patch has some
> merits. Ofc no need to hold on v2 of this series for this, but still I
> think you can re-propose this later (and I didn't get from
> your commit message you were confusing STOPPED/STOPPING).
> 
> In rvin_stop_streaming(), you enter STOPPING state, disable the
> interface cleaning ME bit in VnMC and single/continuous capture mode
> in VnFC, and then poll on CA bit of VnMS until the VIN peripheral has
> not been stopped, at this  point you set interface state to STOPPED.
> 
> As you loop you can still receive interrupts, as you are releasing the
> spinlock when sleeping before testing the ME bit again, so it's fine
> checking for STOPPING state in irq handler.
> It seems to me though, that once you enter STOPPED state, you are sure the
> peripheral has stopped and you should not receive any more interrupt, spurious
> ones apart or when the peripheral fails to stop at all, but things went
> south already at that point.
> 
> Again no need to have this part of this series, but you may want to
> take into consideration this for the future, as with this change you can
> remove the STOPPED state at all from the driver.

You are correct.

This patch was extracted from another series I plan to post after the 
VIN Gen3 beast is done. The aim of that series is to add SEQ_TB/BT 
support to VIN and to do so another state STARTING is needed to handle 
the first few fields. But to avoid growing that series too large I 
thought I could get away with adding this cleanup to this series which 
cleans up the interrupt handler. So this patch will comeback in some 
form when I post that series :-)

But for now I'm happy to drop it as the performance gain with this 
patch-set applied are so nice when running into buffer starved 
situations.

> 
> Thanks
>j
> 
> >
> > >
> > > --
> > > Kieran
> > >
> > >
> > > >
> > > > Signed-off-by: Niklas Söderlund 
> > > > ---
> > > >  drivers/media/platform/rcar-vin/rcar-dma.c | 6 --
> > > >  1 file changed, 6 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> > > > b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > > index 23fdff7a7370842e..b4be75d5009080f7 100644
> > > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > > @@ -916,12 +916,6 @@ static irqreturn_t rvin_irq(int irq, void *data)
> > > > rvin_ack_interrupt(vin);
> > > > handled = 1;
> > > >
> > > > -   /* Nothing to do if capture status is 'STOPPED' */
> > > > -   if (vin->state == STOPPED) {
> > > > -   vin_dbg(vin, "IRQ while state stopped\n");
> > > > -   goto done;
> > > > -   }
> > > > -
> > > > /* Nothing to do if capture status is 'STOPPING' */
> > > > if (vin->state == STOPPING) {
> > > > vin_dbg(vin, "IRQ while state stopping\n");
> > > >
> >
> > --
> > Regards,
> > Niklas Söderlund



-- 
Regards,
Niklas Söderlund

Re: [PATCH/RFC 1/2] PM / wakeup: Add callback for wake-up change notification

[Sergei Shtylyov]

On 03/14/2018 02:26 PM, Geert Uytterhoeven wrote:

> Add a callback to inform a device that his wake-up setting has been

   s/his/its/.

> changed.  This allows a device to synchronize device configuration with
> an external user action.
> 
> E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power
> switch, the system suspend procedure is:
>   1. Configure PMIC for DDR backup mode, which changes the role of the
>  accessory power switch from a power to a wake-up switch,
>   2. Switch accessory power switch off, to prepare for system suspend,
>   3. Suspend system.
> 
> Hence step 1 cannot be done in the PMIC's suspend callback, but it can
> be done in the new callback, in response to the user writing "enabled"
> to the PMIC's wakeup virtual file in sysfs.
> 
> Signed-off-by: Geert Uytterhoeven 
[...]

MBR, Sergei

Re: [GIT PULL FOR renesas-drivers] vsp1/du/interlaced/v2

[Geert Uytterhoeven]

Hi Kieran,

On Wed, Mar 14, 2018 at 4:46 PM, Kieran Bingham
 wrote:
> Please consider including this release in renesas-drivers.
>
> --
> Regards
>
> Kieran
>
> The following changes since commit 397eb3811ec096d0ceefa1dbea2d0ae68feb0587:
>
>   media: vsp1: Move video configuration to a cached dlb (2018-03-07 21:19:29 
> +)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git 
> tags/vsp1/du/interlaced/v2
>
> for you to fetch changes up to 334f0b14fde0ddcfd02a12c41878a1b792221e4d:
>
>   drm: rcar-du: Support interlaced video output through vsp1 (2018-03-13 
> 18:50:00 +0100)

Thank you, merges cleanly into today's linux-next.

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

[GIT PULL FOR renesas-drivers] vsp1/du/interlaced/v2

[Kieran Bingham]

Hi Geert,

Please consider including this release in renesas-drivers.

--
Regards

Kieran

The following changes since commit 397eb3811ec096d0ceefa1dbea2d0ae68feb0587:

  media: vsp1: Move video configuration to a cached dlb (2018-03-07 21:19:29 
+)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git 
tags/vsp1/du/interlaced/v2

for you to fetch changes up to 334f0b14fde0ddcfd02a12c41878a1b792221e4d:

  drm: rcar-du: Support interlaced video output through vsp1 (2018-03-13 
18:50:00 +0100)


Kieran Bingham (11):
  media: vsp1: drm: Fix minor grammar error
  media: vsp1: Remove packed attributes from aligned structures
  media: vsp1: Rename dl_child to dl_next
  media: vsp1: Remove unused display list structure field
  media: vsp1: Clean up DLM objects on error
  media: vsp1: Provide VSP1 feature helper macro
  media: vsp1: Use header display lists for all WPF outputs linked to the DU
  media: vsp1: Add support for extended display list headers
  media: vsp1: Provide support for extended command pools
  media: vsp1: Support Interlaced display pipelines
  drm: rcar-du: Support interlaced video output through vsp1

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |   1 +
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |   3 +
 drivers/media/platform/vsp1/vsp1.h  |   3 +
 drivers/media/platform/vsp1/vsp1_dl.c   | 406 +---
 drivers/media/platform/vsp1/vsp1_dl.h   |  32 ++-
 drivers/media/platform/vsp1/vsp1_drm.c  |  13 +-
 drivers/media/platform/vsp1/vsp1_drv.c  |  23 +-
 drivers/media/platform/vsp1/vsp1_regs.h |   6 +-
 drivers/media/platform/vsp1/vsp1_rpf.c  |  72 +-
 drivers/media/platform/vsp1/vsp1_rwpf.h |   1 +
 drivers/media/platform/vsp1/vsp1_wpf.c  |   6 +-
 include/media/vsp1.h|   1 +
 12 files changed, 455 insertions(+), 112 deletions(-)

Re: [PATCH 1/3] rcar-vin: remove duplicated check of state in irq handler

[jacopo mondi]

Hi Niklas, Kieran,

On Tue, Mar 13, 2018 at 06:56:54PM +0100, Niklas Söderlund wrote:
> Hi Kieran,
>
> Thanks for your feedback.
>
> On 2018-03-13 17:42:25 +0100, Kieran Bingham wrote:
> > Hi Niklas,
> >
> > Thanks for the patch series :) - I've been looking forward to seeing this 
> > one !
> >
> > On 10/03/18 01:09, Niklas Söderlund wrote:
> > > This is an error from when the driver where converted from soc-camera.
> > > There is absolutely no gain to check the state variable two times to be
> > > extra sure if the hardware is stopped.
> >
> > I'll not say this isn't a redundant check ... but isn't the check against 
> > two
> > different states, and thus the remaining check doesn't actually catch the 
> > case
> > now where state == STOPPED ?
>
> Thanks for noticing this, you are correct. I think I need to refresh my
> glasses subscription after missing this :-)
>
> >
> > (Perhaps state != RUNNING would be better ?, but I haven't checked the rest 
> > of
> > the code)
>
> I will respin this in a v2 and either use state != RUNNING or at least
> combine the two checks to prevent future embarrassing mistakes like
> this.

I am sorry I have missed this comment, but I think your patch has some
merits. Ofc no need to hold on v2 of this series for this, but still I
think you can re-propose this later (and I didn't get from
your commit message you were confusing STOPPED/STOPPING).

In rvin_stop_streaming(), you enter STOPPING state, disable the
interface cleaning ME bit in VnMC and single/continuous capture mode
in VnFC, and then poll on CA bit of VnMS until the VIN peripheral has
not been stopped, at this  point you set interface state to STOPPED.

As you loop you can still receive interrupts, as you are releasing the
spinlock when sleeping before testing the ME bit again, so it's fine
checking for STOPPING state in irq handler.
It seems to me though, that once you enter STOPPED state, you are sure the
peripheral has stopped and you should not receive any more interrupt, spurious
ones apart or when the peripheral fails to stop at all, but things went
south already at that point.

Again no need to have this part of this series, but you may want to
take into consideration this for the future, as with this change you can
remove the STOPPED state at all from the driver.

Thanks
   j

>
> >
> > --
> > Kieran
> >
> >
> > >
> > > Signed-off-by: Niklas Söderlund 
> > > ---
> > >  drivers/media/platform/rcar-vin/rcar-dma.c | 6 --
> > >  1 file changed, 6 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> > > b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > index 23fdff7a7370842e..b4be75d5009080f7 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > @@ -916,12 +916,6 @@ static irqreturn_t rvin_irq(int irq, void *data)
> > >   rvin_ack_interrupt(vin);
> > >   handled = 1;
> > >
> > > - /* Nothing to do if capture status is 'STOPPED' */
> > > - if (vin->state == STOPPED) {
> > > - vin_dbg(vin, "IRQ while state stopped\n");
> > > - goto done;
> > > - }
> > > -
> > >   /* Nothing to do if capture status is 'STOPPING' */
> > >   if (vin->state == STOPPING) {
> > >   vin_dbg(vin, "IRQ while state stopping\n");
> > >
>
> --
> Regards,
> Niklas Söderlund


signature.asc
Description: PGP signature

Re: [PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Sergei Shtylyov]

Hello!

On 03/14/2018 03:09 PM, Geert Uytterhoeven wrote:

> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
> need to be kept powered when backup mode is enabled.  Reflect this in
> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
> 
> The accessory power switch (SW23) is a toggle switch, hense specify
> "rohm,rstbmode-level".
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
> v2:
>   - Add rohm,rstbmode-level.
> ---
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi 
> b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -376,6 +376,8 @@
>   #interrupt-cells = <2>;
>   gpio-controller;
>   #gpio-cells = <2>;
> + rohm,ddr-backup-power = <15>;

  Why not 0xf if those are all bit flags?

[...]

MBR, Sergei

Re: [PATCH] arm64: dts: renesas: v3msk: add SCIF0 pins

[Simon Horman]

On Wed, Mar 14, 2018 at 02:14:27PM +0100, Geert Uytterhoeven wrote:
> On Tue, Mar 13, 2018 at 8:26 PM, Sergei Shtylyov
>  wrote:
> > Add the (previously omitted) SCIF0 pin data to the V3M Starter Kit board's
> > device tree.
> >
> > Signed-off-by: Sergei Shtylyov 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied.

Re: [PATCH] pinctrl: sh-pfc: r8a77970: add EtherAVB pin groups

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Mar 13, 2018 at 8:54 PM, Sergei Shtylyov
 wrote:
> Add the EtherAVB pin groups to the R8A77970 PFC driver.
>
> Signed-off-by: Sergei Shtylyov 

Thanks!

Reviewed-by: Geert Uytterhoeven 
and queued in sh-pfc-for-v4.17.

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 v7] ARM: shmobile: Add watchdog support

[Fabrizio Castro]

Hello Simon,

> Subject: Re: [PATCH v7] ARM: shmobile: Add watchdog support
>
> On Wed, Mar 14, 2018 at 11:13:53AM +, Fabrizio Castro wrote:
> > On R-Car Gen2 and RZ/G1 platforms, we use the SBAR registers to make non
> > boot CPUs run a routine designed to bring up SMP and deal with hot plug.
> > The value contained in the SBAR registers is not initialized by a WDT
> > triggered reset, which means that after a WDT triggered reset we jump
> > to the SMP bring up routine, preventing the system from executing the
> > bootrom code.
> >
> > The purpose of this patch is to jump to the bootrom code in case of a
> > WDT triggered reset, and keep the SMP functionality untouched.
> > In order to tell if the code had been called due to the WDT overflowing
> > we are testing WOVF from register RWTCSRA.
> >
> > The new function shmobile_boot_vector_gen2 isn't replacing
> > shmobile_boot_vector for backward compatibility reasons. The kernel
> > will install the best option (either shmobile_boot_vector or
> > shmobile_boot_vector_gen2) to ICRAM1 after parsing the device tree,
> > according to the amount of memory available.
> >
> > Since shmobile_boot_vector has become bigger, "reg" property of nodes
> > compatible with "renesas,smp-sram" now need to be set to a value
> > greater or equal to "<0 0x60>".
> >
> > Signed-off-by: Fabrizio Castro 
> > Signed-off-by: Ramesh Shanmugasundaram 
> > 
> > Reviewed-by: Geert Uytterhoeven 
> > ---
> > v6->v7:
> > * restored ifdef within arch/arm/mach-shmobile/headsmp.S
>
> Thanks. I believe that this is the same as the hand modified
> version of v5 that I applied yesterday.

yes, I believe v5 modified and applied by you is the same as v7.
Thank you for taking the patch.

Fab



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

Re: [PATCH] arm64: dts: renesas: v3msk: add SCIF0 pins

[Geert Uytterhoeven]

On Tue, Mar 13, 2018 at 8:26 PM, Sergei Shtylyov
 wrote:
> Add the (previously omitted) SCIF0 pin data to the V3M Starter Kit board's
> device tree.
>
> Signed-off-by: Sergei Shtylyov 

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 v7] ARM: shmobile: Add watchdog support

[Simon Horman]

On Wed, Mar 14, 2018 at 11:13:53AM +, Fabrizio Castro wrote:
> On R-Car Gen2 and RZ/G1 platforms, we use the SBAR registers to make non
> boot CPUs run a routine designed to bring up SMP and deal with hot plug.
> The value contained in the SBAR registers is not initialized by a WDT
> triggered reset, which means that after a WDT triggered reset we jump
> to the SMP bring up routine, preventing the system from executing the
> bootrom code.
> 
> The purpose of this patch is to jump to the bootrom code in case of a
> WDT triggered reset, and keep the SMP functionality untouched.
> In order to tell if the code had been called due to the WDT overflowing
> we are testing WOVF from register RWTCSRA.
> 
> The new function shmobile_boot_vector_gen2 isn't replacing
> shmobile_boot_vector for backward compatibility reasons. The kernel
> will install the best option (either shmobile_boot_vector or
> shmobile_boot_vector_gen2) to ICRAM1 after parsing the device tree,
> according to the amount of memory available.
> 
> Since shmobile_boot_vector has become bigger, "reg" property of nodes
> compatible with "renesas,smp-sram" now need to be set to a value
> greater or equal to "<0 0x60>".
> 
> Signed-off-by: Fabrizio Castro 
> Signed-off-by: Ramesh Shanmugasundaram 
> 
> Reviewed-by: Geert Uytterhoeven 
> ---
> v6->v7:
> * restored ifdef within arch/arm/mach-shmobile/headsmp.S

Thanks. I believe that this is the same as the hand modified
version of v5 that I applied yesterday.

[PATCH v2 0/3] arm64: dts: renesas: salvator-x(s)/ulcb: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

Hi Simon, Magnus,

The ROHM BD9571MWV PMIC on the Renesas Salvator-X(S) and ULCB
development boards supports DDR Backup Power, which means that the DDR
power rails can be kept powered while the main SoC is powered down.

This patch series is the DT counterpart of the series "[PATCH v2 0/4]
regulator: bd9571mwv: Add support for DDR backup mode" [1] and
"[PATCH/RFC 0/2] regulator: bd9571mwv: Add support for toggle power
switches" [2].

The first patch adds the missing device node for the BD9571 PMIC on the
ULCB boards, and can be applied immediately.
The last two patches add DDR Backup Mode configuration, and depend on
the DT bindings in the driver patch being accepted.

Changes compared to v2:
  - Add support for ULCB,
  - Add "rohm,rstbmode-level" for Salvator-X(S).

This has been tested on M3ULCB (thanks Jacopo!), and on Salvator-X(S).

For testing, driver and DTS patches are available in the
topic/bd9571-ddr-backup-mode-driver-v2 and
topic/bd9571-ddr-backup-mode-dt-v2 branches of my renesas-drivers git
repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.

Thanks for your comments!

[1] https://lkml.org/lkml/2018/3/14/302
[2] https://lkml.org/lkml/2018/3/14/324

Geert Uytterhoeven (3):
  arm64: dts: renesas: ulcb: Add BD9571 PMIC
  arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup
Power
  arm64: dts: renesas: ulcb: Configure PMIC for DDR Backup Power

 arch/arm64/boot/dts/renesas/salvator-common.dtsi |  2 ++
 arch/arm64/boot/dts/renesas/ulcb.dtsi| 31 
 2 files changed, 33 insertions(+)

-- 
2.7.4

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 v2 3/3] arm64: dts: renesas: ulcb: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

On the R-Car Starter Kit Premier/Pro, all of the DDR0, DDR1, DDR0C, and
DDR1C power rails need to be kept powered when backup mode is enabled.
Reflect this in the "rohm,ddr-backup-power" property for the BD9571MWV
PMIC node.

The accessory power switch (SW8) is a momentary switch, hense specify
"rohm,rstbmode-pulse".

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - New.
---
 arch/arm64/boot/dts/renesas/ulcb.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi 
b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index bb21ae335e8b8489..6caf2e4b776faf40 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -256,6 +256,8 @@
#interrupt-cells = <2>;
gpio-controller;
#gpio-cells = <2>;
+   rohm,ddr-backup-power = <15>;
+   rohm,rstbmode-pulse;
 
regulators {
dvfs: dvfs {
-- 
2.7.4

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
need to be kept powered when backup mode is enabled.  Reflect this in
the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.

The accessory power switch (SW23) is a toggle switch, hense specify
"rohm,rstbmode-level".

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Add rohm,rstbmode-level.
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi 
b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 2a7f36abd2dd85c6..80794c38c2669d75 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -376,6 +376,8 @@
#interrupt-cells = <2>;
gpio-controller;
#gpio-cells = <2>;
+   rohm,ddr-backup-power = <15>;
+   rohm,rstbmode-level;
 
regulators {
dvfs: dvfs {
-- 
2.7.4

[PATCH v2 1/3] arm64: dts: renesas: ulcb: Add BD9571 PMIC

[Geert Uytterhoeven]

Add a device node for the ROHM BD9571MWV PMIC.

This was based on the example in the DT binding documentation, but using
IRQ0 instead of a GPIO interrupt, as that matches the schematics, and
because INTC-EX is a simpler block.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - New.
---
 arch/arm64/boot/dts/renesas/ulcb.dtsi | 29 +
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi 
b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index 6f814845f8b665f3..bb21ae335e8b8489 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -243,6 +243,30 @@
 
 _dvfs {
status = "okay";
+
+   pmic: pmic@30 {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
+   compatible = "rohm,bd9571mwv";
+   reg = <0x30>;
+   interrupt-parent = <_ex>;
+   interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+   interrupt-controller;
+   #interrupt-cells = <2>;
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   regulators {
+   dvfs: dvfs {
+   regulator-name = "dvfs";
+   regulator-min-microvolt = <75>;
+   regulator-max-microvolt = <103>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   };
+   };
 };
 
  {
@@ -276,6 +300,11 @@
function = "i2c2";
};
 
+   irq0_pins: irq0 {
+   groups = "intc_ex_irq0";
+   function = "intc_ex";
+   };
+
scif2_pins: scif2 {
groups = "scif2_data_a";
function = "scif2";
-- 
2.7.4

Re: [PATCH] PCI: fix semicolon.cocci warnings

[Lorenzo Pieralisi]

On Wed, Mar 07, 2018 at 09:34:29AM +0100, Julia Lawall wrote:
> From: Fengguang Wu 
> 
>  Remove unneeded semicolon.
> 
> Generated by: scripts/coccinelle/misc/semicolon.cocci
> 
> Signed-off-by: Fengguang Wu 
> Signed-off-by: Julia Lawall 
> ---
> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
> pci-header-cleanup
> head:   40fb7646c6ec16d2fd3de422be876baca9edf86a
> commit: c44f2aed62c2314f1c0ecda606716090b73cc730 [4/5] PCI: improve
> compile test coverage
> :: branch date: 9 hours ago
> :: commit date: 9 hours ago
> 
> 
>  pcie-rcar.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hi Julia,

FYI, I marked this as superseded in patchwork since Rob already posted
it as part of this series, I will pick it up from there:

https://patchwork.ozlabs.org/project/linux-pci/list/?series=32449

Thanks,
Lorenzo

> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -435,7 +435,7 @@ static void rcar_pcie_force_speedup(stru
>   }
> 
>   msleep(1);
> - };
> + }
> 
>   dev_err(dev, "Speed change timed out\n");
> 

[PATCH/RFC 0/2] regulator: bd9571mwv: Add support for toggle power switches

[Geert Uytterhoeven]

Hi all,

The ROHM BD9571MWV PMIC on the Renesas Salvator-X(S) and ULCB
development boards supports DDR Backup Power, which means that the DDR
power rails can be kept powered while the main SoC is powered down.

This patch series extends the support for DDR backup mode[1] to systems
with toggle instead of momentary power switches.

With a toggle power switch (or level signal), the following steps must
be followed exactly:
   1. Configure PMIC for backup mode, which changes the role of the
  power switch to a wake-up switch, 
   2. Switch accessory power switch off, to prepare for system suspend,
  which is a manual step not controlled by software,
   3. Suspend system,
   4. Switch accessory power switch on, to resume.

Unlike on systems with a momentary toggle switch, an additional step 2
must be performed in between step 1 and step 3.  Hence step 1 can no
longer be handled in the PMIC's suspend callback.

This patch series proposes a new callback for wake-up change
notification, which allows performing step 1 when the user writes
"enabled" to the PMIC's "wakeup" virtual file in sysfs, e.g.

echo enabled > 
/sys/devices/platform/soc/e60b.i2c/i2c-7/7-0030/bd9571mwv-regulator.2.auto/power/wakeup

Conversely, writing "disabled" reverts the role of the accessory switch
to a power switch.
Note that unlike with momentary switches, backup mode is not enabled by
default, as enabling it prevents the board from being powered off using
the power switch, which may confuse the user.

This has been tested on M3ULCB (thanks Jacopo!), and on Salvator-X(S).

For testing, driver and DTS patches are available in the
topic/bd9571-ddr-backup-mode-driver-v2 and
topic/bd9571-ddr-backup-mode-dt-v2 branches of my renesas-drivers git
repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.

Thanks for your comments!

[1] https://lkml.org/lkml/2018/3/14/302

Geert Uytterhoeven (2):
  PM / wakeup: Add callback for wake-up change notification
  regulator: bd9571mwv: Add support for toggle power switches

 drivers/base/power/wakeup.c |  4 
 drivers/regulator/bd9571mwv-regulator.c | 24 
 include/linux/pm.h  |  1 +
 3 files changed, 29 insertions(+)

-- 
2.7.4

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/RFC 1/2] PM / wakeup: Add callback for wake-up change notification

[Geert Uytterhoeven]

Add a callback to inform a device that his wake-up setting has been
changed.  This allows a device to synchronize device configuration with
an external user action.

E.g. on systems using a Rohm BD9571MWV PMIC and a toggle accessory power
switch, the system suspend procedure is:
  1. Configure PMIC for DDR backup mode, which changes the role of the
 accessory power switch from a power to a wake-up switch,
  2. Switch accessory power switch off, to prepare for system suspend,
  3. Suspend system.

Hence step 1 cannot be done in the PMIC's suspend callback, but it can
be done in the new callback, in response to the user writing "enabled"
to the PMIC's wakeup virtual file in sysfs.

Signed-off-by: Geert Uytterhoeven 
---
Is there a better way to handle this?
---
 drivers/base/power/wakeup.c | 4 
 include/linux/pm.h  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index ea01621ed769ab26..40bda13d4dcd36f3 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -256,6 +256,8 @@ static int device_wakeup_attach(struct device *dev, struct 
wakeup_source *ws)
if (dev->power.wakeirq)
device_wakeup_attach_irq(dev, dev->power.wakeirq);
spin_unlock_irq(>power.lock);
+   if (dev->power.wakeup_change_notify)
+   dev->power.wakeup_change_notify(dev, true);
return 0;
 }
 
@@ -373,6 +375,8 @@ static struct wakeup_source *device_wakeup_detach(struct 
device *dev)
 {
struct wakeup_source *ws;
 
+   if (dev->power.wakeup_change_notify)
+   dev->power.wakeup_change_notify(dev, false);
spin_lock_irq(>power.lock);
ws = dev->power.wakeup;
dev->power.wakeup = NULL;
diff --git a/include/linux/pm.h b/include/linux/pm.h
index e723b78d835706f2..3dec274bc6f8 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -599,6 +599,7 @@ struct dev_pm_info {
struct list_headentry;
struct completion   completion;
struct wakeup_source*wakeup;
+   void (*wakeup_change_notify)(struct device *dev, bool enable);
boolwakeup_path:1;
boolsyscore:1;
boolno_pm_callbacks:1;  /* Owned by the PM core 
*/
-- 
2.7.4

[PATCH/RFC 2/2] regulator: bd9571mwv: Add support for toggle power switches

[Geert Uytterhoeven]

Extend the existing support for backup mode to toggle power switches.
With a toggle power switch (or level signal), the following steps must
be followed exactly:
   1. Configure PMIC for backup mode,
   2. Switch accessory power switch off, to prepare for system suspend,
  which is a manual step not controlled by software,
   3. Suspend system.
Hence the PMIC is configured for backup mode when "enabled" is written
to the PMIC's "wakeup" virtual file in sysfs, using the newly introduced
callback for wake-up change notification.
Unlike with momentary switches, backup mode is not enabled by default,
as enabling it prevents the board from being powered off using the power
switch, which may confuse the user.
Conversely, writing "disabled" reverts the role of the accessory switch
to a power switch.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/regulator/bd9571mwv-regulator.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/regulator/bd9571mwv-regulator.c 
b/drivers/regulator/bd9571mwv-regulator.c
index be574eb444ebda97..435c8da576b604cf 100644
--- a/drivers/regulator/bd9571mwv-regulator.c
+++ b/drivers/regulator/bd9571mwv-regulator.c
@@ -171,6 +171,24 @@ static int bd9571mwv_bkup_mode_write(struct bd9571mwv *bd, 
unsigned int mode)
return 0;
 }
 
+static void bd9571mwv_wakeup_change_notify(struct device *dev, bool enable)
+{
+   struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
+
+   unsigned int mode;
+   int ret;
+
+   ret = bd9571mwv_bkup_mode_read(bdreg->bd, );
+   if (ret)
+   return;
+
+   mode &= ~BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK;
+   if (enable)
+   mode |= bdreg->bkup_mode_cnt_keepon;
+
+   bd9571mwv_bkup_mode_write(bdreg->bd, mode);
+}
+
 static int bd9571mwv_suspend(struct device *dev)
 {
struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
@@ -277,6 +295,11 @@ static int bd9571mwv_regulator_probe(struct 
platform_device *pdev)
 * explicit user setup in level mode.
 */
device_set_wakeup_enable(>dev, bdreg->rstbmode_pulse);
+#ifdef CONFIG_PM_SLEEP
+   if (bdreg->rstbmode_level)
+   pdev->dev.power.wakeup_change_notify =
+   bd9571mwv_wakeup_change_notify;
+#endif /* CONFIG_PM_SLEEP */
}
 
return 0;
-- 
2.7.4

[PATCH v7] ARM: shmobile: Add watchdog support

[Fabrizio Castro]

On R-Car Gen2 and RZ/G1 platforms, we use the SBAR registers to make non
boot CPUs run a routine designed to bring up SMP and deal with hot plug.
The value contained in the SBAR registers is not initialized by a WDT
triggered reset, which means that after a WDT triggered reset we jump
to the SMP bring up routine, preventing the system from executing the
bootrom code.

The purpose of this patch is to jump to the bootrom code in case of a
WDT triggered reset, and keep the SMP functionality untouched.
In order to tell if the code had been called due to the WDT overflowing
we are testing WOVF from register RWTCSRA.

The new function shmobile_boot_vector_gen2 isn't replacing
shmobile_boot_vector for backward compatibility reasons. The kernel
will install the best option (either shmobile_boot_vector or
shmobile_boot_vector_gen2) to ICRAM1 after parsing the device tree,
according to the amount of memory available.

Since shmobile_boot_vector has become bigger, "reg" property of nodes
compatible with "renesas,smp-sram" now need to be set to a value
greater or equal to "<0 0x60>".

Signed-off-by: Fabrizio Castro 
Signed-off-by: Ramesh Shanmugasundaram 
Reviewed-by: Geert Uytterhoeven 
---
v6->v7:
* restored ifdef within arch/arm/mach-shmobile/headsmp.S

 arch/arm/mach-shmobile/common.h  |  4 +++
 arch/arm/mach-shmobile/headsmp.S | 55 
 2 files changed, 59 insertions(+)

diff --git a/arch/arm/mach-shmobile/common.h b/arch/arm/mach-shmobile/common.h
index a8fa4f7..43c1ac69 100644
--- a/arch/arm/mach-shmobile/common.h
+++ b/arch/arm/mach-shmobile/common.h
@@ -7,6 +7,10 @@ extern void shmobile_init_delay(void);
 extern void shmobile_boot_vector(void);
 extern unsigned long shmobile_boot_fn;
 extern unsigned long shmobile_boot_size;
+extern void shmobile_boot_vector_gen2(void);
+extern unsigned long shmobile_boot_fn_gen2;
+extern unsigned long shmobile_boot_cpu_gen2;
+extern unsigned long shmobile_boot_size_gen2;
 extern void shmobile_smp_boot(void);
 extern void shmobile_smp_sleep(void);
 extern void shmobile_smp_hook(unsigned int cpu, unsigned long fn,
diff --git a/arch/arm/mach-shmobile/headsmp.S b/arch/arm/mach-shmobile/headsmp.S
index 32e0bf6..cef8e8c 100644
--- a/arch/arm/mach-shmobile/headsmp.S
+++ b/arch/arm/mach-shmobile/headsmp.S
@@ -16,6 +16,11 @@
 #include 
 #include 
 
+#define SCTLR_MMU  0x01
+#define BOOTROM_ADDRESS0xE634
+#define RWTCSRA_ADDRESS0xE6020004
+#define RWTCSRA_WOVF   0x10
+
 /*
  * Reset vector for secondary CPUs.
  * This will be mapped at address 0 by SBAR register.
@@ -37,6 +42,56 @@ shmobile_boot_fn:
 shmobile_boot_size:
.long   . - shmobile_boot_vector
 
+#ifdef CONFIG_ARCH_RCAR_GEN2
+/*
+ * Reset vector for R-Car Gen2 and RZ/G1 secondary CPUs.
+ * This will be mapped at address 0 by SBAR register.
+ */
+ENTRY(shmobile_boot_vector_gen2)
+   mrc p15, 0, r0, c0, c0, 5   @ r0 = MPIDR
+   ldr r1, shmobile_boot_cpu_gen2
+   cmp r0, r1
+   bne shmobile_smp_continue_gen2
+
+   mrc p15, 0, r1, c1, c0, 0   @ r1 = SCTLR
+   and r0, r1, #SCTLR_MMU
+   cmp r0, #SCTLR_MMU
+   beq shmobile_smp_continue_gen2
+
+   ldr r0, rwtcsra
+   mov r1, #0
+   ldrbr1, [r0]
+   and r0, r1, #RWTCSRA_WOVF
+   cmp r0, #RWTCSRA_WOVF
+   bne shmobile_smp_continue_gen2
+
+   ldr r0, bootrom
+   bx  r0
+
+shmobile_smp_continue_gen2:
+   ldr r1, shmobile_boot_fn_gen2
+   bx  r1
+
+ENDPROC(shmobile_boot_vector_gen2)
+
+   .align  4
+rwtcsra:
+   .word   RWTCSRA_ADDRESS
+bootrom:
+   .word   BOOTROM_ADDRESS
+   .globl  shmobile_boot_cpu_gen2
+shmobile_boot_cpu_gen2:
+   .word   0x
+
+   .align  2
+   .globl  shmobile_boot_fn_gen2
+shmobile_boot_fn_gen2:
+   .space  4
+   .globl  shmobile_boot_size_gen2
+shmobile_boot_size_gen2:
+   .long   . - shmobile_boot_vector_gen2
+#endif /* CONFIG_ARCH_RCAR_GEN2 */
+
 /*
  * Per-CPU SMP boot function/argument selection code based on MPIDR
  */
-- 
2.7.4

[PATCH v2 1/4] dt-bindings: mfd: bd9571mwv: Document DDR Backup Mode properties

[Geert Uytterhoeven]

Document the new optional properties related to DDR Backup Mode and
toggle/momentary power switches.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Improve property description,
  - Add properties for power switch type.
---
 Documentation/devicetree/bindings/mfd/bd9571mwv.txt | 21 +
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt 
b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
index 9ab216a851d5619b..8804a214759845c5 100644
--- a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
+++ b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
@@ -25,6 +25,25 @@ Required properties:
Each child node is defined using the standard
binding for regulators.
 
+Optional properties:
+  - rohm,ddr-backup-power : Value to use for DDR-Backup Power (default 0).
+   This is a bitmask that specifies which DDR power
+   rails need to be kept powered when backup mode is
+   entered, for system suspend:
+ - bit 0: DDR0
+ - bit 1: DDR1
+ - bit 2: DDR0C
+ - bit 3: DDR1C
+   These bits match the KEEPON_DDR* bits in the
+   documentation for the "BKUP Mode Cnt" register.
+  - rohm,rstbmode-level: The RSTB signal is configured for level mode, to
+accommodate a toggle power switch (the RSTBMODE pin is
+strapped low).
+  - rohm,rstbmode-pulse: The RSTB signal is configured for pulse mode, to
+accommodate a momentary power switch (the RSTBMODE pin
+is strapped high).
+The two properties above are mutually exclusive.
+
 Example:
 
pmic: pmic@30 {
@@ -36,6 +55,8 @@ Example:
#interrupt-cells = <2>;
gpio-controller;
#gpio-cells = <2>;
+   rohm,ddr-backup-power = <15>;
+   rohm,rstbmode-pulse;
 
regulators {
dvfs: dvfs {
-- 
2.7.4

[PATCH v2 4/4] regulator: bd9571mwv: Add support for DDR backup mode

[Geert Uytterhoeven]

The BD9571MWV PMIC supports DDR backup mode, which keeps one or more DDR
rails powered while the main SoC is powered down.

Which DDR rails are to be kept powered is board-specific, and controlled
using the optional "rohm,ddr-backup-power" DT property.  In the absence
of this property, backup mode is not available.

Backup mode can be enabled or disabled by the user using the standard
"wakeup" virtual file in sysfs, e.g. to enable:

echo enabled > 
/sys/devices/platform/soc/e60b.i2c/i2c-7/7-0030/bd9571mwv-regulator.2.auto/power/wakeup

When the PMIC is configured for backup mode, the role of the accessory
power switch changes from a power switch to a wake-up switch.
Two types of switches (or signals) can be used:
  A. With a momentary power switch (or pulse signal), the PMIC is
 configured for backup mode in the PMIC driver's suspend callback,
 during system suspend.
 Backup mode is enabled by default, as there is no further impact
 during normal system operation.

  B. With a toggle power switch (or level signal), the following steps
 must be followed exactly:
   1. Configure PMIC for backup mode,
   2. Switch accessory power switch off, to prepare for system
  suspend, which is a manual step not controlled by software,
   3. Suspend system.
 This mode is not yet supported by the driver.

As the switch type is board-specific, and cannot be determined
automatically, it is obtained from the presence of one of the
"rohm,rstbmode-*" properties in DT.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Use normal "wakeup" sysfs virtual file instead of adding our own
"backup_mode" file,
  - Differentiate between momentary and toggle power switches:
  - Support for momentary switches is enabled automatically,
  - Toggle switches are not yet supported (but still work when
backup mode is enabled using i2set from userspace).
---
 drivers/regulator/bd9571mwv-regulator.c | 127 +++-
 1 file changed, 126 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/bd9571mwv-regulator.c 
b/drivers/regulator/bd9571mwv-regulator.c
index c67a83d53c4cb76b..be574eb444ebda97 100644
--- a/drivers/regulator/bd9571mwv-regulator.c
+++ b/drivers/regulator/bd9571mwv-regulator.c
@@ -24,6 +24,18 @@
 
 #include 
 
+struct bd9571mwv_reg {
+   struct bd9571mwv *bd;
+
+   /* DDR Backup Power */
+   u8 bkup_mode_cnt_keepon;/* from "rohm,ddr-backup-power" */
+   u8 bkup_mode_cnt_saved;
+
+   /* Power switch type */
+   bool rstbmode_level;
+   bool rstbmode_pulse;
+};
+
 enum bd9571mwv_regulators { VD09, VD18, VD25, VD33, DVFS };
 
 #define BD9571MWV_REG(_name, _of, _id, _ops, _vr, _vm, _nv, _min, _step, 
_lmin)\
@@ -131,14 +143,99 @@ static struct regulator_desc regulators[] = {
  0x80, 60, 1, 0x3c),
 };
 
+#ifdef CONFIG_PM_SLEEP
+static int bd9571mwv_bkup_mode_read(struct bd9571mwv *bd, unsigned int *mode)
+{
+   int ret;
+
+   ret = regmap_read(bd->regmap, BD9571MWV_BKUP_MODE_CNT, mode);
+   if (ret) {
+   dev_err(bd->dev, "failed to read backup mode (%d)\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int bd9571mwv_bkup_mode_write(struct bd9571mwv *bd, unsigned int mode)
+{
+   int ret;
+
+   ret = regmap_write(bd->regmap, BD9571MWV_BKUP_MODE_CNT, mode);
+   if (ret) {
+   dev_err(bd->dev, "failed to configure backup mode 0x%x (%d)\n",
+   mode, ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int bd9571mwv_suspend(struct device *dev)
+{
+   struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
+   unsigned int mode;
+   int ret;
+
+   if (!device_may_wakeup(dev))
+   return 0;
+
+   /* Save DDR Backup Mode */
+   ret = bd9571mwv_bkup_mode_read(bdreg->bd, );
+   if (ret)
+   return ret;
+
+   bdreg->bkup_mode_cnt_saved = mode;
+
+   if (!bdreg->rstbmode_pulse)
+   return 0;
+
+   /* Enable DDR Backup Mode */
+   mode &= ~BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK;
+   mode |= bdreg->bkup_mode_cnt_keepon;
+
+   if (mode != bdreg->bkup_mode_cnt_saved)
+   return bd9571mwv_bkup_mode_write(bdreg->bd, mode);
+
+   return 0;
+}
+
+static int bd9571mwv_resume(struct device *dev)
+{
+   struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
+
+   if (!device_may_wakeup(dev))
+   return 0;
+
+   /* Restore DDR Backup Mode */
+   return bd9571mwv_bkup_mode_write(bdreg->bd, bdreg->bkup_mode_cnt_saved);
+}
+
+static const struct dev_pm_ops bd9571mwv_pm  = {
+   SET_SYSTEM_SLEEP_PM_OPS(bd9571mwv_suspend, bd9571mwv_resume)
+};
+
+#define DEV_PM_OPS _pm
+#else
+#define DEV_PM_OPS NULL
+#endif /* CONFIG_PM_SLEEP */
+
 static int bd9571mwv_regulator_probe(struct platform_device *pdev)
 {
struct bd9571mwv 

[PATCH v2 0/4] regulator: bd9571mwv: Add support for DDR backup mode

[Geert Uytterhoeven]

Hi all,

The ROHM BD9571MWV PMIC on the Renesas Salvator-X(S) and ULCB
development boards supports DDR Backup Power, which means that the DDR
power rails can be kept powered while the main SoC is powered down.

Currently performing a system suspend/resume cycle involves several
manual steps:
  1. Configure the PMIC for Backup Mode, using
 "i2cset -f -y 7 0x30 0x20 0x0f", which changes the role of the
 power switch to a wake-up switch,
  2. Switch off SW23 (the ACC toggle switch) to prepare for suspend
 (only on Salvator-X(S)),
  3. Suspend to RAM, using "echo mem > /sys/power/state",
  4. Switch on SW23 (on Salvator-X(S)) or push momentary switch SW8 (on
 ULCB) to resume.
Note the need for step 2 on systems equipped with a toggle instead of
momentary power switch.

Especially the first and second steps are cumbersome, as they rely on
  1. Intimate knowledge about the PMIC and actual board design,
  2. Direct i2c access,
  3. Having the i2cset utility available,
  4. A manual togle switch operation (on Salvator-X(S)).
In addition, all of this has to be redone after system resume.

This patch series integrates Backup Mode configuration into the PMIC
driver, using the "wakeup" sysfs virtual file.  I.e. it can be enabled
or disabled by writing "enabled" or "disabled" to e.g.


/sys/devices/platform/soc/e60b.i2c/i2c-7/7-0030/bd9571mwv-regulator.2.auto/power/wakeup

Which DDR rails are to be kept powered is board-specific, and controlled
using the optional "rohm,ddr-backup-power" property in the board DTS
file.

As the power switch type is board-specific, and cannot be determined
automatically, it is obtained from the presence of one of the
"rohm,rstbmode-{pulse,level}" properties in DT.

For now only momentary power switches are supported, and wake-up is
enabled by default, as it doesn't have any run-time side-effects.
Support for toggle switches will be added in a follow-up series.

Changes compared to v1:
  - Improve DT property description,
  - Add DT properties for power switch type,
  - Add Acked-by,
  - Use normal "wakeup" sysfs virtual file instead of adding our own
"backup_mode" file,
  - Differentiate between momentary and toggle power switches:
  - Support for momentary switches is enabled automatically,
  - Toggle switches are not yet supported (but still work when
backup mode is enabled using i2set from userspace).
  - Split off DTS part into its own series.

This has been tested on M3ULCB (thanks Jacopo!), and on Salvator-X(S)
(still using i2set from userspace).

For testing, driver and DTS patches are available in the
topic/bd9571-ddr-backup-mode-driver-v2 and
topic/bd9571-ddr-backup-mode-dt-v2 branches of my renesas-drivers git
repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.

Thanks for your comments!

Geert Uytterhoeven (4):
  dt-bindings: mfd: bd9571mwv: Document DDR Backup Mode properties
  mfd: bd9571mwv: Add DDR Backup Power register bit definitions
  mfd: bd9571mwv: Allow DDR Backup Power register access
  regulator: bd9571mwv: Add support for DDR backup mode

 .../devicetree/bindings/mfd/bd9571mwv.txt  |  21 
 drivers/mfd/bd9571mwv.c|   2 +
 drivers/regulator/bd9571mwv-regulator.c| 127 -
 include/linux/mfd/bd9571mwv.h  |   5 +
 4 files changed, 154 insertions(+), 1 deletion(-)

-- 
2.7.4

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 v2 3/4] mfd: bd9571mwv: Allow DDR Backup Power register access

[Geert Uytterhoeven]

Enable read/write access to the BD9571MWV_BKUP_MODE_CNT register, which
is amongst others used to configure DDR Backup Power.

Signed-off-by: Geert Uytterhoeven 
---
Acked-for-MFD-by: Lee Jones 

v2:
  - Expand "a.o.",
  - Add Acked-for-MFD-by (for Lee's own reference).
---
 drivers/mfd/bd9571mwv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
index 64e088dfe7b05b5b..503979c81dae11bb 100644
--- a/drivers/mfd/bd9571mwv.c
+++ b/drivers/mfd/bd9571mwv.c
@@ -29,6 +29,7 @@ static const struct mfd_cell bd9571mwv_cells[] = {
 
 static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+   regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
@@ -44,6 +45,7 @@ static const struct regmap_access_table 
bd9571mwv_readable_table = {
 };
 
 static const struct regmap_range bd9571mwv_writable_yes_ranges[] = {
+   regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
regmap_reg_range(BD9571MWV_AVS_VD09_VID(0), BD9571MWV_AVS_VD09_VID(3)),
regmap_reg_range(BD9571MWV_DVFS_SETVID, BD9571MWV_DVFS_SETVID),
regmap_reg_range(BD9571MWV_GPIO_DIR, BD9571MWV_GPIO_OUT),
-- 
2.7.4

[PATCH v2 2/4] mfd: bd9571mwv: Add DDR Backup Power register bit definitions

[Geert Uytterhoeven]

Add definitions for the KEEPON_* bits in the "BKUP Mode Cnt" register,
which control the DDR rails to be kept powered when backup mode is
enabled.

Signed-off-by: Geert Uytterhoeven 
Acked-by: Lee Jones 
---
v2:
  - Add Acked-by.
---
 include/linux/mfd/bd9571mwv.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mfd/bd9571mwv.h b/include/linux/mfd/bd9571mwv.h
index f0708ba4cbbae2dc..eb05569f752bb089 100644
--- a/include/linux/mfd/bd9571mwv.h
+++ b/include/linux/mfd/bd9571mwv.h
@@ -33,6 +33,11 @@
 #define BD9571MWV_I2C_MD2_E1_BIT_2 0x12
 
 #define BD9571MWV_BKUP_MODE_CNT0x20
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_MASKGENMASK(3, 0)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR0BIT(0)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR1BIT(1)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR0C   BIT(2)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR1C   BIT(3)
 #define BD9571MWV_BKUP_MODE_STATUS 0x21
 #define BD9571MWV_BKUP_RECOVERY_CNT0x22
 #define BD9571MWV_BKUP_CTRL_TIM_CNT0x23
-- 
2.7.4

RE: [PATCH v6 01/26] ARM: shmobile: Add watchdog support

[Fabrizio Castro]

Hi Geert,

> Subject: Re: [PATCH v6 01/26] ARM: shmobile: Add watchdog support
>
> Hi Fabrizio,
>
> On Wed, Feb 28, 2018 at 6:40 PM, Fabrizio Castro
>  wrote:
> > On R-Car Gen2 and RZ/G1 platforms, we use the SBAR registers to make non
> > boot CPUs run a routine designed to bring up SMP and deal with hot plug.
> > The value contained in the SBAR registers is not initialized by a WDT
> > triggered reset, which means that after a WDT triggered reset we jump
> > to the SMP bring up routine, preventing the system from executing the
> > bootrom code.
> >
> > The purpose of this patch is to jump to the bootrom code in case of a
> > WDT triggered reset, and keep the SMP functionality untouched.
> > In order to tell if the code had been called due to the WDT overflowing
> > we are testing WOVF from register RWTCSRA.
> >
> > The new function shmobile_boot_vector_gen2 isn't replacing
> > shmobile_boot_vector for backward compatibility reasons. The kernel
> > will install the best option (either shmobile_boot_vector or
> > shmobile_boot_vector_gen2) to ICRAM1 after parsing the device tree,
> > according to the amount of memory available.
> >
> > Since shmobile_boot_vector has become bigger, "reg" property of nodes
> > compatible with "renesas,smp-sram" now need to be set to a value
> > greater or equal to "<0 0x60>".
> >
> > Signed-off-by: Fabrizio Castro 
> > Signed-off-by: Ramesh Shanmugasundaram 
> > 
> > Reviewed-by: Geert Uytterhoeven 
> > ---
> > v5->v6:
> > * taken ifdefs out as per Geert's suggestion.
>
> My intention was to remove the #ifdefs from the header file only, not
> from arch/arm/mach-shmobile/headsmp.S, as the latter is used for
> other SoC families, too.

Ah ok, I'll send another version to restore the ifdef within headsmp.S then.

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



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

Re: [PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

[jacopo mondi]

Hi Andrzej,
thanks for review

On Wed, Mar 14, 2018 at 09:42:36AM +0100, Andrzej Hajda wrote:
> On 13.03.2018 15:30, Jacopo Mondi wrote:
> > Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel
> > output decoder.
>
> IMO converter suits here better, but it is just suggestion.
>
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  drivers/gpu/drm/bridge/Kconfig|   7 +
> >  drivers/gpu/drm/bridge/Makefile   |   1 +
> >  drivers/gpu/drm/bridge/thc63lvd1024.c | 241 
> > ++
> >  3 files changed, 249 insertions(+)
> >  create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index 3b99d5a..facf6bd 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -92,6 +92,13 @@ config DRM_SII9234
> >   It is an I2C driver, that detects connection of MHL bridge
> >   and starts encapsulation of HDMI signal.
> >
> > +config DRM_THINE_THC63LVD1024
> > +   tristate "Thine THC63LVD1024 LVDS decoder bridge"
> > +   depends on OF
> > +   select DRM_PANEL_BRIDGE
>
> You don't use PANEL_BRIDGE, it should be possible to drop the select.
>

Ack

> > +   ---help---
> > + Thine THC63LVD1024 LVDS decoder bridge driver.
>
> Decoder to what?
> Maybe it would be better to say it is LVDS/parallel or LVDS/RGB
> converter or bridge.

"LVDS to digital CMOS/TTL parallel data converter" as the manual
describes the chip?

>
> > +
> >  config DRM_TOSHIBA_TC358767
> > tristate "Toshiba TC358767 eDP bridge"
> > depends on OF
> > diff --git a/drivers/gpu/drm/bridge/Makefile 
> > b/drivers/gpu/drm/bridge/Makefile
> > index 373eb28..fd90b16 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> >  obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
> >  obj-$(CONFIG_DRM_SII902X) += sii902x.o
> >  obj-$(CONFIG_DRM_SII9234) += sii9234.o
> > +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> >  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> >  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> >  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
> > b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > new file mode 100644
> > index 000..4b059c0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > @@ -0,0 +1,241 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * THC63LVD1024 LVDS to parallel data DRM bridge driver.
> > + *
> > + * Copyright (C) 2018 Jacopo Mondi 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static const char * const thc63_reg_names[] = {
> > +   "vcc", "lvcc", "pvcc", "cvcc", };
> > +
> > +struct thc63_dev {
> > +   struct device *dev;
> > +
> > +   struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)];
> > +
> > +   struct gpio_desc *pwdn;
> > +   struct gpio_desc *oe;
> > +
> > +   struct drm_bridge bridge;
> > +   struct drm_bridge *next;
> > +};
> > +
> > +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > +{
> > +   return container_of(bridge, struct thc63_dev, bridge);
> > +}
> > +
> > +static int thc63_attach(struct drm_bridge *bridge)
> > +{
> > +   struct thc63_dev *thc63 = to_thc63(bridge);
> > +
> > +   return drm_bridge_attach(bridge->encoder, thc63->next, bridge);
> > +}
> > +
> > +static void thc63_enable(struct drm_bridge *bridge)
> > +{
> > +   struct thc63_dev *thc63 = to_thc63(bridge);
> > +   struct regulator *vcc;
> > +   unsigned int i;
> > +   int ret;
> > +
> > +   for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> > +   vcc = thc63->vccs[i];
> > +   if (vcc) {
> > +   ret = regulator_enable(vcc);
> > +   if (ret)
> > +   goto error_vcc_enable;
> > +   }
> > +   }
> > +
> > +   if (thc63->pwdn)
> > +   gpiod_set_value(thc63->pwdn, 0);
> > +
> > +   if (thc63->oe)
> > +   gpiod_set_value(thc63->oe, 1);
> > +
> > +   return;
> > +
> > +error_vcc_enable:
> > +   dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
> > +}
> > +
> > +static void thc63_disable(struct drm_bridge *bridge)
> > +{
> > +   struct thc63_dev *thc63 = to_thc63(bridge);
> > +   struct regulator *vcc;
> > +   unsigned int i;
> > +   int ret;
> > +
> > +   for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> > +   vcc = thc63->vccs[i];
> > +   if (vcc) {
> > +   ret = regulator_disable(vcc);
> > +   if (ret)
> > +   goto error_vcc_disable;
>
> I think in disable path you can report an error and continue - should be
> safer.
>

Ack

> > +   }
> > +   }
> > +
> > +   if (thc63->pwdn)
> > +   gpiod_set_value(thc63->pwdn, 1);
> > +
> > +   

Re: [PATCH 1/3] dt-bindings: iommu: ipmmu-vmsa: Add device tree support for r8a774[35]

[Joerg Roedel]

On Wed, Mar 07, 2018 at 09:09:21AM +0100, Simon Horman wrote:
> [CC Alex Williamson]
> 
> It looks like the last patch to this file was taken by Alex.
> Perhaps he would be willing to take this one too if it it was
> reposted with him CCed.

Alex was taking care of IOMMU patches while I was off at the end of last
year. I will take care of these.


Regards,

Joerg

Re: [PATCH v3 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Andrzej,
sorry for the mess :(

On Wed, Mar 14, 2018 at 09:15:42AM +0100, Andrzej Hajda wrote:
> On 13.03.2018 15:30, Jacopo Mondi wrote:
> > Document Thine THC63LVD1024 LVDS decoder device tree bindings.
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  .../bindings/display/bridge/thine,thc63lvd1024.txt | 63 
> > ++
> >  1 file changed, 63 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt 
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > new file mode 100644
> > index 000..5b5afcd
> > --- /dev/null
> > +++ 
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,63 @@
> > +Thine Electronics THC63LVD1024 LVDS decoder
> > +---
> > +
> > +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS 
> > streams
> > +to parallel data outputs. The chip supports single/dual input/output modes,
> > +handling up to two two input LVDS stream and up to two digital CMOS/TTL 
> > outputs.
> > +
> > +Required properties:
> > +- compatible: Shall be "thine,thc63lvd1024",
> > +
> > +Optional properties:
> > +- vcc-supply: Power supply for TTL output and digital circuitry
> > +- cvcc-supply: Power supply for TTL CLOCKOUT signal
> > +- lvcc-supply: Power supply for LVDS inputs
> > +- pvcc-supply: Power supply for PLL circuitry
>
> I wonder if it wouldn't be better to make them required (at least VCC) -
> it is closer to reality.

In cases like our Eagle board, where VCC is directly connected to the
powering rail and not through a controllable regulator, I feel like
making this mandatory requires more effort (not much, I agree, just a
"fixed-regulator" more) with no additional benefits.

But I understand your point, and I'm open to whatever fits better with
the already existing DRM bridges bindings

>
> > +- pwnd-gpios: Power down GPIO signal. Active low.
>
> As I said before, specs[1] says about "/PDWN" pin. Is it your typo, or
> different docs?

I didn't notice the typo in first review, and I thought you were referring to
the initial '/' which I found weird to be part of the gpio name... Then I now
realized I badly typed in "pwnd" in place of "pwdn", which is not even correct
because it has to be "pdwn"... Sorry about this mess, I will fix in v4

> Moreover there are already bindings for THC63LVDM83D with the same
> dichotomy [2].

Seems like this is 'wrong' as well.. The chip manual says the pin is
named "pdwn" here too..

>
> Out of curiosity I have googled for "pwnd pin" and it looks like some
> vendors use this form.
> For me both forms are quite misleading: power down signal, active low,
> why they couldn't call it just 'enable, active high'.
>

It's not much the actual physical active level that bugs me, but the fact
that the GPIO name defines if it has to be set to "active" or
"inactive" logical state in enable/disable routines that I don't
like..

> [1]: http://www.thine.co.jp/files/topics/179_ext_12_0.pdf
> [2]:
> https://elixir.bootlin.com/linux/v4.16-rc5/source/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
>
> > +- oe-gpios: Output enable GPIO signal. Active high.
> > +
> > +The THC63LVD1024 video port connections are modeled according
> > +to OF graph bindings specified by 
> > Documentation/devicetree/bindings/graph.txt
> > +
> > +Required video port nodes:
> > +- Port@0: First LVDS input port
> > +- Port@2: First digital CMOS/TTL parallel output
> > +
> > +Optional video port nodes:
> > +- Port@1: Second LVDS input port
> > +- Port@3: Second digital CMOS/TTL parallel output
> > +
> > +Example:
> > +
> > +
> > +   thc63lvd1024: lvds-decoder {
> > +   compatible = "thine,thc63lvd1024";
> > +
> > +   vcc-supply = <_lvds_vcc>;
> > +   lvcc-supply = <_lvds_lvcc>;
> > +
> > +   pwdn-gpio = < 15 GPIO_ACTIVE_LOW>;
> And here another variation :), should be pdwn-gpios.

Next time it will be "pndw".. Is there a prize if I do insert all
permutations of the same name in a single bindings document? :)

Will fix this shortly.

Thanks
   j


signature.asc
Description: PGP signature

Re: [PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

[Andrzej Hajda]

On 13.03.2018 15:30, Jacopo Mondi wrote:
> Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel
> output decoder.

IMO converter suits here better, but it is just suggestion.

>
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/gpu/drm/bridge/Kconfig|   7 +
>  drivers/gpu/drm/bridge/Makefile   |   1 +
>  drivers/gpu/drm/bridge/thc63lvd1024.c | 241 
> ++
>  3 files changed, 249 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 3b99d5a..facf6bd 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -92,6 +92,13 @@ config DRM_SII9234
> It is an I2C driver, that detects connection of MHL bridge
> and starts encapsulation of HDMI signal.
>  
> +config DRM_THINE_THC63LVD1024
> + tristate "Thine THC63LVD1024 LVDS decoder bridge"
> + depends on OF
> + select DRM_PANEL_BRIDGE

You don't use PANEL_BRIDGE, it should be possible to drop the select.

> + ---help---
> +   Thine THC63LVD1024 LVDS decoder bridge driver.

Decoder to what?
Maybe it would be better to say it is LVDS/parallel or LVDS/RGB
converter or bridge.

> +
>  config DRM_TOSHIBA_TC358767
>   tristate "Toshiba TC358767 eDP bridge"
>   depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 373eb28..fd90b16 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>  obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
>  obj-$(CONFIG_DRM_SII9234) += sii9234.o
> +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
> b/drivers/gpu/drm/bridge/thc63lvd1024.c
> new file mode 100644
> index 000..4b059c0
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * THC63LVD1024 LVDS to parallel data DRM bridge driver.
> + *
> + * Copyright (C) 2018 Jacopo Mondi 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +static const char * const thc63_reg_names[] = {
> + "vcc", "lvcc", "pvcc", "cvcc", };
> +
> +struct thc63_dev {
> + struct device *dev;
> +
> + struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)];
> +
> + struct gpio_desc *pwdn;
> + struct gpio_desc *oe;
> +
> + struct drm_bridge bridge;
> + struct drm_bridge *next;
> +};
> +
> +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct thc63_dev, bridge);
> +}
> +
> +static int thc63_attach(struct drm_bridge *bridge)
> +{
> + struct thc63_dev *thc63 = to_thc63(bridge);
> +
> + return drm_bridge_attach(bridge->encoder, thc63->next, bridge);
> +}
> +
> +static void thc63_enable(struct drm_bridge *bridge)
> +{
> + struct thc63_dev *thc63 = to_thc63(bridge);
> + struct regulator *vcc;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> + vcc = thc63->vccs[i];
> + if (vcc) {
> + ret = regulator_enable(vcc);
> + if (ret)
> + goto error_vcc_enable;
> + }
> + }
> +
> + if (thc63->pwdn)
> + gpiod_set_value(thc63->pwdn, 0);
> +
> + if (thc63->oe)
> + gpiod_set_value(thc63->oe, 1);
> +
> + return;
> +
> +error_vcc_enable:
> + dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
> +}
> +
> +static void thc63_disable(struct drm_bridge *bridge)
> +{
> + struct thc63_dev *thc63 = to_thc63(bridge);
> + struct regulator *vcc;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
> + vcc = thc63->vccs[i];
> + if (vcc) {
> + ret = regulator_disable(vcc);
> + if (ret)
> + goto error_vcc_disable;

I think in disable path you can report an error and continue - should be
safer.

> + }
> + }
> +
> + if (thc63->pwdn)
> + gpiod_set_value(thc63->pwdn, 1);
> +
> + if (thc63->oe)
> + gpiod_set_value(thc63->oe, 0);

Usually disable uses reverse order. Ie first disable oe, then, pwdn,
then regulators, also in reverse order.
Looks more reasonable.

> +
> + return;
> +
> +error_vcc_disable:
> + dev_err(thc63->dev, "Failed to disable regulator %u\n", i);
> +}
> +
> +struct drm_bridge_funcs thc63_bridge_func = {
> + .attach = 

Re: [PATCH v2 02/11] media: vsp1: Remove packed attributes from aligned structures

[Geert Uytterhoeven]

On Tue, Mar 13, 2018 at 7:05 PM, Kieran Bingham
 wrote:
> The use of the packed attribute can cause a performance penalty for
> all accesses to the struct members, as the compiler will assume that the
> structure has the potential to have an unaligned base.
>
> These structures are all correctly aligned and contain no holes, thus
> the attribute is redundant and negatively impacts performance, so we
> remove the attributes entirely.
>
> Signed-off-by: Kieran Bingham 

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 v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1

[Geert Uytterhoeven]

Hi Simon,

On Tue, Mar 13, 2018 at 9:05 PM, Simon Horman  wrote:
> On Thu, Mar 01, 2018 at 11:20:11AM +0100, Geert Uytterhoeven wrote:
>> On Fri, Feb 23, 2018 at 9:14 AM, Simon Horman  wrote:
>> > On Thu, Feb 22, 2018 at 09:38:39AM +0100, Geert Uytterhoeven wrote:
>> >> On Wed, Feb 21, 2018 at 7:32 PM, Simon Horman  wrote:
>> >> > On Wed, Feb 21, 2018 at 05:30:12PM +0100, Geert Uytterhoeven wrote:
>> >> >> On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman  
>> >> >> wrote:
>> >> >> > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote:
>> >> >> >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
>> >> >> >>  wrote:
>> >> >> >> > this series has been around for some time as RFC, and it has 
>> >> >> >> > collected
>> >> >> >> > useful comments from the community along the way.
>> >> >> >> > The solution proposed by this patch set works for most R-Car Gen2 
>> >> >> >> > and
>> >> >> >> > RZ/G1 devices, but not all of them. We now know that for some 
>> >> >> >> > R-Car
>> >> >> >> > Gen2 early revisions there is no proper software fix. Anyway, no
>> >> >> >> > product has been built around early revisions, but development 
>> >> >> >> > boards
>> >> >> >> > mounting early revisions (basically prototypes) are still out 
>> >> >> >> > there.
>> >> >> >> > As a result, this series isn't enabling the internal watchdog on 
>> >> >> >> > R-Car
>> >> >> >> > Gen2 boards, developers may enable it in board specific device 
>> >> >> >> > trees
>> >> >> >> > if needed.
>> >> >> >> > This series has been tested by me on the iwg20d, iwg22d, Lager, 
>> >> >> >> > Alt,
>> >> >> >> > and Koelsch boards.
>> >> >> >> >
>> >> >> >> > The problem
>> >> >> >> > ===
>> >> >> >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset 
>> >> >> >> > vector
>> >> >> >> > to ICRAM1 and we program the [S]BAR registers so that when we 
>> >> >> >> > turn ON
>> >> >> >> > the non-boot CPUs they are redirected to the reset vector 
>> >> >> >> > installed by
>> >> >> >> > Linux in ICRAM1, and eventually they continue the execution to 
>> >> >> >> > RAM,
>> >> >> >> > where the SMP bring-up code will take care of the rest.
>> >> >> >> > The content of the [S]BAR registers survives a watchdog triggered 
>> >> >> >> > reset,
>> >> >> >> > and as such after the watchdog fires the boot core will try and 
>> >> >> >> > execute
>> >> >> >> > the SMP bring-up code instead of jumping to the bootrom code.
>> >> >> >> >
>> >> >> >> > The fix
>> >> >> >> > ===
>> >> >> >> > The main strategy for the solution is to let the reset vector 
>> >> >> >> > decide
>> >> >> >> > if it needs to jump to shmobile_boot_fn or to the bootrom code.
>> >> >> >> > In a watchdog triggered reset scenario, since the [S]BAR 
>> >> >> >> > registers keep
>> >> >> >> > their values, the boot CPU will jump into the newly designed reset
>> >> >> >> > vector, the assembly routine will eventually test WOVF (a bit in 
>> >> >> >> > register
>> >> >> >> > RWTCSRA that indicates if the watchdog counter has overflown, the 
>> >> >> >> > value
>> >> >> >> > of this bit gets retained in this scenario), and jump to the 
>> >> >> >> > bootrom code
>> >> >> >> > which will in turn load up the bootloader, etc.
>> >> >> >> > When bringing up SMP or using CPU hotplug, the reset vector will 
>> >> >> >> > jump
>> >> >> >> > to shmobile_boot_fn instead.
>> >> >> >> >
>> >> >> >> > Thank you All for your help.
>> >> >> >> >
>> >> >> >> > Best regards,
>> >> >> >> >
>> >> >> >> > Fabrizio Castro (26):
>> >> >> >> >   ARM: shmobile: Add watchdog support
>> >> >> >> >   ARM: dts: r8a7743: Adjust SMP routine size
>> >> >> >> >   ARM: dts: r8a7745: Adjust SMP routine size
>> >> >> >> >   ARM: dts: r8a7790: Adjust SMP routine size
>> >> >> >> >   ARM: dts: r8a7791: Adjust SMP routine size
>> >> >> >> >   ARM: dts: r8a7792: Adjust SMP routine size
>> >> >> >> >   ARM: dts: r8a7793: Adjust SMP routine size
>> >> >> >> >   ARM: dts: r8a7794: Adjust SMP routine size
>> >> >> >> >   soc: renesas: rcar-rst: Enable watchdog as reset trigger for 
>> >> >> >> > Gen2
>> >> >> >> >   ARM: shmobile: rcar-gen2: Add watchdog support
>> >> >> >> >   dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support
>> >> >> >> >   watchdog: renesas_wdt: Add R-Car Gen2 support
>> >> >> >> >   watchdog: renesas_wdt: Add restart handler
>> >> >> >> >   ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN
>> >> >> >> >   clk: renesas: r8a7743: Add rwdt clock
>> >> >> >> >   clk: renesas: r8a7745: Add rwdt clock
>> >> >> >> >   clk: renesas: r8a7790: Add rwdt clock
>> >> >> >> >   clk: renesas: r8a7791/r8a7793: Add rwdt clock
>> >> >> >> >   clk: renesas: r8a7794: Add rwdt clock
>> >> >> >> >   ARM: dts: r8a7743: Add watchdog support to SoC dtsi
>> >> >> >> >   ARM: dts: r8a7745: Add watchdog support to SoC dtsi
>> >> >> >> >   ARM: dts: r8a7790: Add watchdog support to SoC dtsi
>> >> >> 

Re: [PATCH v3 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Andrzej Hajda]

On 13.03.2018 15:30, Jacopo Mondi wrote:
> Document Thine THC63LVD1024 LVDS decoder device tree bindings.
>
> Signed-off-by: Jacopo Mondi 
> ---
>  .../bindings/display/bridge/thine,thc63lvd1024.txt | 63 
> ++
>  1 file changed, 63 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt 
> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> new file mode 100644
> index 000..5b5afcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -0,0 +1,63 @@
> +Thine Electronics THC63LVD1024 LVDS decoder
> +---
> +
> +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS 
> streams
> +to parallel data outputs. The chip supports single/dual input/output modes,
> +handling up to two two input LVDS stream and up to two digital CMOS/TTL 
> outputs.
> +
> +Required properties:
> +- compatible: Shall be "thine,thc63lvd1024",
> +
> +Optional properties:
> +- vcc-supply: Power supply for TTL output and digital circuitry
> +- cvcc-supply: Power supply for TTL CLOCKOUT signal
> +- lvcc-supply: Power supply for LVDS inputs
> +- pvcc-supply: Power supply for PLL circuitry

I wonder if it wouldn't be better to make them required (at least VCC) -
it is closer to reality.

> +- pwnd-gpios: Power down GPIO signal. Active low.

As I said before, specs[1] says about "/PDWN" pin. Is it your typo, or
different docs?
Moreover there are already bindings for THC63LVDM83D with the same
dichotomy [2].

Out of curiosity I have googled for "pwnd pin" and it looks like some
vendors use this form.
For me both forms are quite misleading: power down signal, active low,
why they couldn't call it just 'enable, active high'.

[1]: http://www.thine.co.jp/files/topics/179_ext_12_0.pdf
[2]:
https://elixir.bootlin.com/linux/v4.16-rc5/source/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt

> +- oe-gpios: Output enable GPIO signal. Active high.
> +
> +The THC63LVD1024 video port connections are modeled according
> +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
> +
> +Required video port nodes:
> +- Port@0: First LVDS input port
> +- Port@2: First digital CMOS/TTL parallel output
> +
> +Optional video port nodes:
> +- Port@1: Second LVDS input port
> +- Port@3: Second digital CMOS/TTL parallel output
> +
> +Example:
> +
> +
> + thc63lvd1024: lvds-decoder {
> + compatible = "thine,thc63lvd1024";
> +
> + vcc-supply = <_lvds_vcc>;
> + lvcc-supply = <_lvds_lvcc>;
> +
> + pwdn-gpio = < 15 GPIO_ACTIVE_LOW>;
And here another variation :), should be pdwn-gpios.

> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + lvds_dec_in_0: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + port@2{
> + reg = <2>;
> +
> + lvds_dec_out_2: endpoint {
> + remote-endpoint = <_in>;
> + };
> +
> + };
> +
> + };
> + };