Re: [PATCH] pinctrl: Convert to using %pOF instead of full_name

[Ludovic Desroches]

On Tue, Jul 18, 2017 at 04:43:23PM -0500, Rob Herring wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
> 
> Signed-off-by: Rob Herring 
> Cc: Linus Walleij 
> Cc: Lee Jones 
> Cc: Eric Anholt 
> Cc: Stefan Wahren 
> Cc: Florian Fainelli 
> Cc: Ray Jui 
> Cc: Scott Branden 
> Cc: bcm-kernel-feedback-l...@broadcom.com
> Cc: Ludovic Desroches 
> Cc: Patrice Chotard 
> Cc: Tomasz Figa 
> Cc: Krzysztof Kozlowski 
> Cc: Sylwester Nawrocki 
> Cc: Laurent Pinchart 
> Cc: Geert Uytterhoeven 
> Cc: Barry Song 
> Cc: linux-g...@vger.kernel.org
> Cc: linux-rpi-ker...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: ker...@stlinux.com
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  drivers/pinctrl/bcm/pinctrl-bcm2835.c | 25 +++--
>  drivers/pinctrl/devicetree.c  |  4 ++--
>  drivers/pinctrl/freescale/pinctrl-imx.c   |  8 +++-
>  drivers/pinctrl/pinconf-generic.c |  7 +++
>  drivers/pinctrl/pinctrl-at91-pio4.c   | 11 +--

Acked-by: Ludovic Desroches 

>  drivers/pinctrl/pinctrl-st.c  |  2 +-
>  drivers/pinctrl/pinctrl-tb10x.c   |  4 ++--
>  drivers/pinctrl/samsung/pinctrl-samsung.c |  6 +++---
>  drivers/pinctrl/sh-pfc/pinctrl.c  |  2 +-
>  drivers/pinctrl/sirf/pinctrl-sirf.c   |  6 +++---
>  10 files changed, 34 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
> b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 230883168e99..3e71e5d782ee 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -692,8 +692,7 @@ static int bcm2835_pctl_dt_node_to_map_func(struct 
> bcm2835_pinctrl *pc,
>   struct pinctrl_map *map = *maps;
> 
>   if (fnum >= ARRAY_SIZE(bcm2835_functions)) {
> - dev_err(pc->dev, "%s: invalid brcm,function %d\n",
> - of_node_full_name(np), fnum);
> + dev_err(pc->dev, "%pOF: invalid brcm,function %d\n", np, fnum);
>   return -EINVAL;
>   }
> 
> @@ -713,8 +712,7 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
> bcm2835_pinctrl *pc,
>   unsigned long *configs;
> 
>   if (pull > 2) {
> - dev_err(pc->dev, "%s: invalid brcm,pull %d\n",
> - of_node_full_name(np), pull);
> + dev_err(pc->dev, "%pOF: invalid brcm,pull %d\n", np, pull);
>   return -EINVAL;
>   }
> 
> @@ -745,8 +743,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
> 
>   pins = of_find_property(np, "brcm,pins", NULL);
>   if (!pins) {
> - dev_err(pc->dev, "%s: missing brcm,pins property\n",
> - of_node_full_name(np));
> + dev_err(pc->dev, "%pOF: missing brcm,pins property\n", np);
>   return -EINVAL;
>   }
> 
> @@ -755,8 +752,8 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
> 
>   if (!funcs && !pulls) {
>   dev_err(pc->dev,
> - "%s: neither brcm,function nor brcm,pull specified\n",
> - of_node_full_name(np));
> + "%pOF: neither brcm,function nor brcm,pull specified\n",
> + np);
>   return -EINVAL;
>   }
> 
> @@ -766,15 +763,15 @@ static int bcm2835_pctl_dt_node_to_map(struct 
> pinctrl_dev *pctldev,
> 
>   if (num_funcs > 1 && num_funcs != num_pins) {
>   dev_err(pc->dev,
> - "%s: brcm,function must have 1 or %d entries\n",
> - of_node_full_name(np), num_pins);
> + "%pOF: brcm,function must have 1 or %d entries\n",
> + np, num_pins);
>   return -EINVAL;
>   }
> 
>   if (num_pulls > 1 && num_pulls != num_pins) {
>   dev_err(pc->dev,
> - "%s: brcm,pull must have 1 or %d entries\n",
> - of_node_full_name(np), num_pins);
> + "%pOF: brcm,pull must have 1 or %d entries\n",
> + np, num_pins);
>   return -EINVAL;
>   }
> 
> @@ -793,8 +790,8 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
> *pctldev,
>   if (err)
>   goto out;
>   if (pin >= ARRAY_SIZE(bcm2835_gpio_pins)) {
> - dev_err(pc->dev, "%s: invalid brcm,pins 

Re: [RFC v2 4/6] serial: core: support deferring serdev controller registration

[Rob Herring]

On Mon, Jul 17, 2017 at 10:24 AM, Ulrich Hecht
 wrote:
> serdev controllers may depend on other devices (such as multiplexers)
> and thus require deferred probing support.

I wonder if instead of deferring, we could just delay binding the
devices. There's really no reason to defer the controller. This is a
small addition, but deferring involves a lot of undoing and redoing of
initialization.

Rob

Re: [PATCH 1/5] watchdog: renesas_wdt: avoid (theoretical) type overflow

[Guenter Roeck]

On 07/18/2017 12:21 AM, Wolfram Sang wrote:

-   unsigned long rate;
-   unsigned int clks_per_sec;
+   unsigned long rate, clks_per_sec;


If you make this change, you should also update rwdt_priv.clks_per_sec
(yes I know it's removed in a later patch in this series).


Right. I will change but also wait a bit for more comments.



I for my part don't have any. Feel free to add

Reviewed-by: Guenter Roeck 

to the series when you resend.

Thanks,
Guenter

Re: [RFC v2 0/6] serdev multiplexing support

[Rob Herring]

On Mon, Jul 17, 2017 at 10:24 AM, Ulrich Hecht
 wrote:
> Hi!
>
> This is a new attempt to add multiplexer support to serdev. It is now based
> on the mux subsystem, making it more generic than the previous iteration
> ("[RFC 0/4] serdev GPIO-based multiplexing support").
>
> Thanks to reviewers for their comments. This revision incorporates the
> changes suggested as far as they are still applicable, which mostly applies
> to those concerning the MAX9260 i2c adapter driver.
>
> New patches have been added that fix a small issue in the mux include files
> ("mux: include compiler.h from mux/consumer.h"), and implement deferred
> probing of serdev controllers ("serial: core: support deferring serdev
> controller registration"), hopefully correctly.
>
> This series depends on the "pinctrl: sh-pfc: r8a7792: Add SCIF1 pin groups"
> patch as well as v15 of the mux subsystem series ("[PATCH v15 00/13] mux
> controller abstraction and iio/i2c muxes").

Please also document how muxed devices are represented in DT in
serial/slave-device.txt.

Rob

Re: [RFC v2 2/6] serdev: add method to set parity

[Rob Herring]

On Mon, Jul 17, 2017 at 10:24 AM, Ulrich Hecht
 wrote:
> Adds serdev_device_set_parity() and an implementation for ttyport.
>
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/tty/serdev/core.c   | 12 
>  drivers/tty/serdev/serdev-ttyport.c | 17 +
>  include/linux/serdev.h  |  4 
>  3 files changed, 33 insertions(+)

Reviewed-by: Rob Herring 

Re: [RFC v2 3/6] serdev: add multiplexer support

[Rob Herring]

On Mon, Jul 17, 2017 at 10:24 AM, Ulrich Hecht
 wrote:
> Adds an interface for slave device multiplexing using the mux subsystem.
>
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/tty/serdev/Kconfig  |  3 +++
>  drivers/tty/serdev/Makefile |  1 +
>  drivers/tty/serdev/core.c   | 14 +-
>  drivers/tty/serdev/mux.c| 66 
> +
>  include/linux/serdev.h  | 16 ---
>  5 files changed, 96 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/tty/serdev/mux.c
>
> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
> index cdc6b82..9096b71 100644
> --- a/drivers/tty/serdev/Kconfig
> +++ b/drivers/tty/serdev/Kconfig
> @@ -13,4 +13,7 @@ config SERIAL_DEV_CTRL_TTYPORT
> depends on TTY
> depends on SERIAL_DEV_BUS != m
>
> +config SERIAL_DEV_MUX
> +   bool "Serial device multiplexer support"

I think this breaks for modules.

I'm inclined to just keep mux support in the core.

> +
>  endif
> diff --git a/drivers/tty/serdev/Makefile b/drivers/tty/serdev/Makefile
> index 0cbdb94..d713381 100644
> --- a/drivers/tty/serdev/Makefile
> +++ b/drivers/tty/serdev/Makefile
> @@ -1,5 +1,6 @@
>  serdev-objs := core.o
>
>  obj-$(CONFIG_SERIAL_DEV_BUS) += serdev.o
> +obj-$(CONFIG_SERIAL_DEV_MUX) += mux.o
>
>  obj-$(CONFIG_SERIAL_DEV_CTRL_TTYPORT) += serdev-ttyport.o
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 1fbaa4c..92c5bfa 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -305,7 +305,8 @@ struct serdev_device *serdev_device_alloc(struct 
> serdev_controller *ctrl)
> return NULL;
>
> serdev->ctrl = ctrl;
> -   ctrl->serdev = serdev;
> +   if (!ctrl->serdev)
> +   ctrl->serdev = serdev;
> device_initialize(>dev);
> serdev->dev.parent = >dev;
> serdev->dev.bus = _bus_type;
> @@ -368,6 +369,8 @@ static int of_serdev_register_devices(struct 
> serdev_controller *ctrl)
> struct serdev_device *serdev = NULL;
> int err;
> bool found = false;
> +   int nr = 0;
> +   u32 reg;
>
> for_each_available_child_of_node(ctrl->dev.of_node, node) {
> if (!of_get_property(node, "compatible", NULL))
> @@ -380,6 +383,10 @@ static int of_serdev_register_devices(struct 
> serdev_controller *ctrl)
> continue;
>
> serdev->dev.of_node = node;
> +   serdev->nr = nr++;
> +
> +   if (!of_property_read_u32(node, "reg", ))
> +   serdev->mux_addr = reg;

Perhaps nr should just be assigned reg value.

>
> err = serdev_device_add(serdev);
> if (err) {
> @@ -414,6 +421,11 @@ int serdev_controller_add(struct serdev_controller *ctrl)
> if (ret)
> return ret;
>
> +#ifdef CONFIG_SERIAL_DEV_MUX
> +   if (of_serdev_register_mux(ctrl) == -EPROBE_DEFER)
> +   return -EPROBE_DEFER;
> +#endif
> +
> ret = of_serdev_register_devices(ctrl);
> if (ret)
> goto out_dev_del;
> diff --git a/drivers/tty/serdev/mux.c b/drivers/tty/serdev/mux.c
> new file mode 100644
> index 000..fc9405b
> --- /dev/null
> +++ b/drivers/tty/serdev/mux.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (C) 2017 Ulrich Hecht
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +int serdev_device_mux_select(struct serdev_device *serdev)
> +{
> +   struct mux_control *mux = serdev->ctrl->mux;
> +   int ret;
> +
> +   if (!mux)
> +   return -EINVAL;
> +
> +   ret = mux_control_select(mux, serdev->mux_addr);
> +   serdev->ctrl->mux_do_not_deselect = ret < 0;
> +
> +   serdev->ctrl->serdev = serdev;
> +
> +   return ret;
> +}
> +
> +int serdev_device_mux_deselect(struct serdev_device *serdev)
> +{
> +   struct mux_control *mux = serdev->ctrl->mux;
> +
> +   if (!mux)
> +   return -EINVAL;
> +
> +   if (!serdev->ctrl->mux_do_not_deselect)
> +   return mux_control_deselect(mux);
> +   else
> +   return 0;
> +}
> +
> +int of_serdev_register_mux(struct serdev_controller *ctrl)
> +{
> +   struct mux_control *mux;
> +   struct device *dev = >dev;
> +
> +   mux = devm_mux_control_get(dev, NULL);
> +
> +   if (IS_ERR(mux)) {
> +   if (PTR_ERR(mux) != -EPROBE_DEFER)

[PATCH] clk: Convert to using %pOF instead of full_name

[Rob Herring]

Now that we have a custom printf format specifier, convert users of
full_name to use %pOF instead. This is preparation to remove storing
of the full path string for each node.

Signed-off-by: Rob Herring 
Cc: Michael Turquette 
Cc: Stephen Boyd 
Cc: Maxime Coquelin 
Cc: Alexandre Torgue 
Cc: Russell King 
Cc: Matthias Brugger 
Cc: Geert Uytterhoeven 
Cc: Maxime Ripard 
Cc: Chen-Yu Tsai 
Cc: "Emilio López" 
Cc: Peter De Schrijver 
Cc: Prashant Gaikwad 
Cc: Thierry Reding 
Cc: Jonathan Hunter 
Cc: Tero Kristo 
Cc: linux-...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-media...@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-te...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
---
 drivers/clk/berlin/bg2.c |  3 +--
 drivers/clk/berlin/bg2q.c|  7 +++
 drivers/clk/clk-asm9260.c|  4 ++--
 drivers/clk/clk-conf.c   | 16 
 drivers/clk/clk-moxart.c | 12 ++--
 drivers/clk/clk-qoriq.c  |  7 +++
 drivers/clk/clk-stm32f4.c|  4 ++--
 drivers/clk/clk-xgene.c  | 15 ++-
 drivers/clk/clk.c|  4 ++--
 drivers/clk/clkdev.c |  4 ++--
 drivers/clk/mediatek/clk-cpumux.c|  2 +-
 drivers/clk/mediatek/clk-mtk.c   |  2 +-
 drivers/clk/mediatek/reset.c |  2 +-
 drivers/clk/renesas/clk-mstp.c   |  2 +-
 drivers/clk/renesas/clk-rcar-gen2.c  |  3 +--
 drivers/clk/sunxi-ng/ccu-sun5i.c |  3 +--
 drivers/clk/sunxi-ng/ccu-sun6i-a31.c |  3 +--
 drivers/clk/sunxi-ng/ccu-sun8i-a23.c |  3 +--
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c |  3 +--
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c  |  3 +--
 drivers/clk/sunxi-ng/ccu-sun8i-r.c   |  3 +--
 drivers/clk/sunxi-ng/ccu-sun8i-v3s.c |  3 +--
 drivers/clk/sunxi/clk-sunxi.c| 17 +++--
 drivers/clk/tegra/clk-emc.c  | 12 +---
 drivers/clk/ti/clockdomain.c |  4 ++--
 25 files changed, 61 insertions(+), 80 deletions(-)

diff --git a/drivers/clk/berlin/bg2.c b/drivers/clk/berlin/bg2.c
index 1d99292e2039..e7331ace0337 100644
--- a/drivers/clk/berlin/bg2.c
+++ b/drivers/clk/berlin/bg2.c
@@ -679,8 +679,7 @@ static void __init berlin2_clock_setup(struct device_node 
*np)
if (!IS_ERR(hws[n]))
continue;

-   pr_err("%s: Unable to register leaf clock %d\n",
-  np->full_name, n);
+   pr_err("%pOF: Unable to register leaf clock %d\n", np, n);
goto bg2_fail;
}

diff --git a/drivers/clk/berlin/bg2q.c b/drivers/clk/berlin/bg2q.c
index 3b784b593afd..67c270b143f7 100644
--- a/drivers/clk/berlin/bg2q.c
+++ b/drivers/clk/berlin/bg2q.c
@@ -304,14 +304,14 @@ static void __init berlin2q_clock_setup(struct 
device_node *np)

gbase = of_iomap(parent_np, 0);
if (!gbase) {
-   pr_err("%s: Unable to map global base\n", np->full_name);
+   pr_err("%pOF: Unable to map global base\n", np);
return;
}

/* BG2Q CPU PLL is not part of global registers */
cpupll_base = of_iomap(parent_np, 1);
if (!cpupll_base) {
-   pr_err("%s: Unable to map cpupll base\n", np->full_name);
+   pr_err("%pOF: Unable to map cpupll base\n", np);
iounmap(gbase);
return;
}
@@ -376,8 +376,7 @@ static void __init berlin2q_clock_setup(struct device_node 
*np)
if (!IS_ERR(hws[n]))
continue;

-   pr_err("%s: Unable to register leaf clock %d\n",
-  np->full_name, n);
+   pr_err("%pOF: Unable to register leaf clock %d\n", np, n);
goto bg2q_fail;
}

diff --git a/drivers/clk/clk-asm9260.c b/drivers/clk/clk-asm9260.c
index ea8568536193..bf0582cbbf38 100644
--- a/drivers/clk/clk-asm9260.c
+++ b/drivers/clk/clk-asm9260.c
@@ -338,8 +338,8 @@ static void __init asm9260_acc_init(struct device_node *np)
if (!IS_ERR(hws[n]))
continue;

-   pr_err("%s: Unable to register leaf clock %d\n",
-   np->full_name, n);
+   pr_err("%pOF: Unable to register leaf clock %d\n",
+   np, n);
goto fail;
}

diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 7ec36722f8ab..49819b546134 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -23,8 +23,8 @@ static int __set_clk_parents(struct device_node *node, bool 
clk_supplier)
   

[PATCH] media: Convert to using %pOF instead of full_name

[Rob Herring]

Now that we have a custom printf format specifier, convert users of
full_name to use %pOF instead. This is preparation to remove storing
of the full path string for each node.

Signed-off-by: Rob Herring 
Cc: Kyungmin Park 
Cc: Andrzej Hajda 
Cc: Mauro Carvalho Chehab 
Cc: "Lad, Prabhakar" 
Cc: Songjun Wu 
Cc: Sylwester Nawrocki 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: Javier Martinez Canillas 
Cc: Minghsiu Tsai 
Cc: Houlong Wei 
Cc: Andrew-CT Chen 
Cc: Matthias Brugger 
Cc: Laurent Pinchart 
Cc: "Niklas Söderlund" 
Cc: Guennadi Liakhovetski 
Cc: Hyun Kwon 
Cc: Michal Simek 
Cc: "Sören Brinkmann" 
Cc: linux-me...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-media...@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
 drivers/media/i2c/s5k5baf.c|  7 ++--
 drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
 drivers/media/platform/atmel/atmel-isc.c   |  4 +-
 drivers/media/platform/davinci/vpif_capture.c  | 16 
 drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
 drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
 drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
 drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
 drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
 drivers/media/platform/omap3isp/isp.c  |  8 ++--
 drivers/media/platform/pxa_camera.c|  2 +-
 drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
 drivers/media/platform/soc_camera/soc_camera.c |  6 +--
 drivers/media/platform/xilinx/xilinx-vipp.c| 52 +-
 drivers/media/v4l2-core/v4l2-async.c   |  4 +-
 drivers/media/v4l2-core/v4l2-clk.c |  3 +-
 include/media/v4l2-clk.h   |  4 +-
 19 files changed, 71 insertions(+), 83 deletions(-)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c 
b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index f434fb2ee6fc..cdc4f2392ef9 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1635,8 +1635,7 @@ static int s5c73m3_get_platform_data(struct s5c73m3 
*state)

node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
-   dev_warn(dev, "no endpoint defined for node: %s\n",
-   node->full_name);
+   dev_warn(dev, "no endpoint defined for node: %pOF\n", node);
return 0;
}

diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 962051b9939d..9c22fc963901 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1863,8 +1863,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf 
*state, struct device *dev)

node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
-   dev_err(dev, "no endpoint defined at node %s\n",
-   node->full_name);
+   dev_err(dev, "no endpoint defined at node %pOF\n", node);
return -EINVAL;
}

@@ -1882,8 +1881,8 @@ static int s5k5baf_parse_device_node(struct s5k5baf 
*state, struct device *dev)
case V4L2_MBUS_PARALLEL:
break;
default:
-   dev_err(dev, "unsupported bus in endpoint defined at node %s\n",
-   node->full_name);
+   dev_err(dev, "unsupported bus in endpoint defined at node 
%pOF\n",
+   node);
return -EINVAL;
}

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index 466aba8b0e00..dfcc484cab89 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2490,8 +2490,8 @@ vpfe_get_pdata(struct platform_device *pdev)

rem = of_graph_get_remote_port_parent(endpoint);
if (!rem) {
-   dev_err(>dev, "Remote device at %s not found\n",
-   endpoint->full_name);
+   dev_err(>dev, "Remote device at %pOF not found\n",
+   endpoint);
goto done;
}

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d6534252cdcd..b2c66b239cf2 100644
--- 

[PATCH] pinctrl: Convert to using %pOF instead of full_name

[Rob Herring]

Now that we have a custom printf format specifier, convert users of
full_name to use %pOF instead. This is preparation to remove storing
of the full path string for each node.

Signed-off-by: Rob Herring 
Cc: Linus Walleij 
Cc: Lee Jones 
Cc: Eric Anholt 
Cc: Stefan Wahren 
Cc: Florian Fainelli 
Cc: Ray Jui 
Cc: Scott Branden 
Cc: bcm-kernel-feedback-l...@broadcom.com
Cc: Ludovic Desroches 
Cc: Patrice Chotard 
Cc: Tomasz Figa 
Cc: Krzysztof Kozlowski 
Cc: Sylwester Nawrocki 
Cc: Laurent Pinchart 
Cc: Geert Uytterhoeven 
Cc: Barry Song 
Cc: linux-g...@vger.kernel.org
Cc: linux-rpi-ker...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: ker...@stlinux.com
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 25 +++--
 drivers/pinctrl/devicetree.c  |  4 ++--
 drivers/pinctrl/freescale/pinctrl-imx.c   |  8 +++-
 drivers/pinctrl/pinconf-generic.c |  7 +++
 drivers/pinctrl/pinctrl-at91-pio4.c   | 11 +--
 drivers/pinctrl/pinctrl-st.c  |  2 +-
 drivers/pinctrl/pinctrl-tb10x.c   |  4 ++--
 drivers/pinctrl/samsung/pinctrl-samsung.c |  6 +++---
 drivers/pinctrl/sh-pfc/pinctrl.c  |  2 +-
 drivers/pinctrl/sirf/pinctrl-sirf.c   |  6 +++---
 10 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c 
b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 230883168e99..3e71e5d782ee 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -692,8 +692,7 @@ static int bcm2835_pctl_dt_node_to_map_func(struct 
bcm2835_pinctrl *pc,
struct pinctrl_map *map = *maps;

if (fnum >= ARRAY_SIZE(bcm2835_functions)) {
-   dev_err(pc->dev, "%s: invalid brcm,function %d\n",
-   of_node_full_name(np), fnum);
+   dev_err(pc->dev, "%pOF: invalid brcm,function %d\n", np, fnum);
return -EINVAL;
}

@@ -713,8 +712,7 @@ static int bcm2835_pctl_dt_node_to_map_pull(struct 
bcm2835_pinctrl *pc,
unsigned long *configs;

if (pull > 2) {
-   dev_err(pc->dev, "%s: invalid brcm,pull %d\n",
-   of_node_full_name(np), pull);
+   dev_err(pc->dev, "%pOF: invalid brcm,pull %d\n", np, pull);
return -EINVAL;
}

@@ -745,8 +743,7 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,

pins = of_find_property(np, "brcm,pins", NULL);
if (!pins) {
-   dev_err(pc->dev, "%s: missing brcm,pins property\n",
-   of_node_full_name(np));
+   dev_err(pc->dev, "%pOF: missing brcm,pins property\n", np);
return -EINVAL;
}

@@ -755,8 +752,8 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,

if (!funcs && !pulls) {
dev_err(pc->dev,
-   "%s: neither brcm,function nor brcm,pull specified\n",
-   of_node_full_name(np));
+   "%pOF: neither brcm,function nor brcm,pull specified\n",
+   np);
return -EINVAL;
}

@@ -766,15 +763,15 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,

if (num_funcs > 1 && num_funcs != num_pins) {
dev_err(pc->dev,
-   "%s: brcm,function must have 1 or %d entries\n",
-   of_node_full_name(np), num_pins);
+   "%pOF: brcm,function must have 1 or %d entries\n",
+   np, num_pins);
return -EINVAL;
}

if (num_pulls > 1 && num_pulls != num_pins) {
dev_err(pc->dev,
-   "%s: brcm,pull must have 1 or %d entries\n",
-   of_node_full_name(np), num_pins);
+   "%pOF: brcm,pull must have 1 or %d entries\n",
+   np, num_pins);
return -EINVAL;
}

@@ -793,8 +790,8 @@ static int bcm2835_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
if (err)
goto out;
if (pin >= ARRAY_SIZE(bcm2835_gpio_pins)) {
-   dev_err(pc->dev, "%s: invalid brcm,pins value %d\n",
-   of_node_full_name(np), pin);
+   dev_err(pc->dev, "%pOF: invalid brcm,pins value %d\n",
+   np, pin);
err = -EINVAL;
goto 

[PATCH] soc: Convert to using %pOF instead of full_name

[Rob Herring]

Now that we have a custom printf format specifier, convert users of
full_name to use %pOF instead. This is preparation to remove storing
of the full path string for each node.

Signed-off-by: Rob Herring 
Cc: Scott Wood 
Cc: Qiang Zhao 
Cc: Matthias Brugger 
Cc: Simon Horman 
Cc: Magnus Damm 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: Javier Martinez Canillas 
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-media...@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-samsung-...@vger.kernel.org
---
 drivers/soc/fsl/qbman/bman_ccsr.c| 10 +-
 drivers/soc/fsl/qbman/bman_portal.c  |  8 +++-
 drivers/soc/fsl/qbman/qman_ccsr.c| 12 ++--
 drivers/soc/fsl/qbman/qman_portal.c  | 11 ---
 drivers/soc/fsl/qe/gpio.c|  4 ++--
 drivers/soc/mediatek/mtk-pmic-wrap.c |  4 ++--
 drivers/soc/renesas/rcar-rst.c   |  4 ++--
 drivers/soc/renesas/rcar-sysc.c  |  6 +++---
 drivers/soc/samsung/pm_domains.c |  8 
 9 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c 
b/drivers/soc/fsl/qbman/bman_ccsr.c
index a8e8389a6894..eaa9585c7347 100644
--- a/drivers/soc/fsl/qbman/bman_ccsr.c
+++ b/drivers/soc/fsl/qbman/bman_ccsr.c
@@ -177,8 +177,8 @@ static int fsl_bman_probe(struct platform_device *pdev)

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
-   dev_err(dev, "Can't get %s property 'IORESOURCE_MEM'\n",
-   node->full_name);
+   dev_err(dev, "Can't get %pOF property 'IORESOURCE_MEM'\n",
+   node);
return -ENXIO;
}
bm_ccsr_start = devm_ioremap(dev, res->start, resource_size(res));
@@ -205,14 +205,14 @@ static int fsl_bman_probe(struct platform_device *pdev)

err_irq = platform_get_irq(pdev, 0);
if (err_irq <= 0) {
-   dev_info(dev, "Can't get %s IRQ\n", node->full_name);
+   dev_info(dev, "Can't get %pOF IRQ\n", node);
return -ENODEV;
}
ret = devm_request_irq(dev, err_irq, bman_isr, IRQF_SHARED, "bman-err",
   dev);
if (ret)  {
-   dev_err(dev, "devm_request_irq() failed %d for '%s'\n",
-   ret, node->full_name);
+   dev_err(dev, "devm_request_irq() failed %d for '%pOF'\n",
+   ret, node);
return ret;
}
/* Disable Buffer Pool State Change */
diff --git a/drivers/soc/fsl/qbman/bman_portal.c 
b/drivers/soc/fsl/qbman/bman_portal.c
index 8354d4dabdad..39b39c8f1399 100644
--- a/drivers/soc/fsl/qbman/bman_portal.c
+++ b/drivers/soc/fsl/qbman/bman_portal.c
@@ -103,16 +103,14 @@ static int bman_portal_probe(struct platform_device *pdev)
addr_phys[0] = platform_get_resource(pdev, IORESOURCE_MEM,
 DPAA_PORTAL_CE);
if (!addr_phys[0]) {
-   dev_err(dev, "Can't get %s property 'reg::CE'\n",
-   node->full_name);
+   dev_err(dev, "Can't get %pOF property 'reg::CE'\n", node);
return -ENXIO;
}

addr_phys[1] = platform_get_resource(pdev, IORESOURCE_MEM,
 DPAA_PORTAL_CI);
if (!addr_phys[1]) {
-   dev_err(dev, "Can't get %s property 'reg::CI'\n",
-   node->full_name);
+   dev_err(dev, "Can't get %pOF property 'reg::CI'\n", node);
return -ENXIO;
}

@@ -120,7 +118,7 @@ static int bman_portal_probe(struct platform_device *pdev)

irq = platform_get_irq(pdev, 0);
if (irq <= 0) {
-   dev_err(dev, "Can't get %s IRQ'\n", node->full_name);
+   dev_err(dev, "Can't get %pOF IRQ'\n", node);
return -ENXIO;
}
pcfg->irq = irq;
diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
b/drivers/soc/fsl/qbman/qman_ccsr.c
index 90bc40c48675..835ce947ffca 100644
--- a/drivers/soc/fsl/qbman/qman_ccsr.c
+++ b/drivers/soc/fsl/qbman/qman_ccsr.c
@@ -695,8 +695,8 @@ static int fsl_qman_probe(struct platform_device *pdev)

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
-   dev_err(dev, "Can't get %s property 'IORESOURCE_MEM'\n",
-   node->full_name);
+   dev_err(dev, "Can't get %pOF property 'IORESOURCE_MEM'\n",
+   node);
return -ENXIO;
}
qm_ccsr_start = devm_ioremap(dev, res->start, resource_size(res));
@@ -740,15 +740,15 @@ static int fsl_qman_probe(struct platform_device *pdev)

err_irq = platform_get_irq(pdev, 0);
if (err_irq <= 0) {
-

[PATCH] ARM: Convert to using %pOF instead of full_name

[Rob Herring]

Now that we have a custom printf format specifier, convert users of
full_name to use %pOF instead. This is preparation to remove storing
of the full path string for each node.

Signed-off-by: Rob Herring 
Cc: Russell King 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: Javier Martinez Canillas 
Cc: Shawn Guo 
Cc: Sascha Hauer 
Cc: Fabio Estevam 
Cc: Jason Cooper 
Cc: Andrew Lunn 
Cc: Gregory Clement 
Cc: Sebastian Hesselbarth 
Cc: Tony Lindgren 
Cc: "Benoît Cousson" 
Cc: Paul Walmsley 
Cc: Heiko Stuebner 
Cc: Simon Horman 
Cc: Magnus Damm 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
---
 arch/arm/kernel/cpuidle.c| 4 ++--
 arch/arm/kernel/devtree.c| 6 +++---
 arch/arm/kernel/topology.c   | 4 ++--
 arch/arm/mach-exynos/suspend.c   | 8 
 arch/arm/mach-imx/gpc.c  | 4 ++--
 arch/arm/mach-mvebu/kirkwood.c   | 4 ++--
 arch/arm/mach-omap2/omap-wakeupgen.c | 4 ++--
 arch/arm/mach-omap2/omap_hwmod.c | 4 ++--
 arch/arm/mach-rockchip/platsmp.c | 4 ++--
 arch/arm/mach-shmobile/pm-rmobile.c  | 8 
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index a3308ad1a024..fda5579123a8 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -101,8 +101,8 @@ static int __init arm_cpuidle_read_ops(struct device_node 
*dn, int cpu)

ops = arm_cpuidle_get_ops(enable_method);
if (!ops) {
-   pr_warn("%s: unsupported enable-method property: %s\n",
-   dn->full_name, enable_method);
+   pr_warn("%pOF: unsupported enable-method property: %s\n",
+   dn, enable_method);
return -EOPNOTSUPP;
}

diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index f676febbb270..28174c9a94ac 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -95,7 +95,7 @@ void __init arm_dt_init_cpu_maps(void)
if (of_node_cmp(cpu->type, "cpu"))
continue;

-   pr_debug(" * %s...\n", cpu->full_name);
+   pr_debug(" * %pOF...\n", cpu);
/*
 * A device tree containing CPU nodes with missing "reg"
 * properties is considered invalid to build the
@@ -103,8 +103,8 @@ void __init arm_dt_init_cpu_maps(void)
 */
cell = of_get_property(cpu, "reg", _bytes);
if (!cell || prop_bytes < sizeof(*cell)) {
-   pr_debug(" * %s missing reg property\n",
-cpu->full_name);
+   pr_debug(" * %pOF missing reg property\n",
+cpu);
of_node_put(cpu);
return;
}
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index bf949a763dbe..e596c5b8f931 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -127,8 +127,8 @@ static void __init parse_dt_topology(void)

rate = of_get_property(cn, "clock-frequency", );
if (!rate || len != 4) {
-   pr_err("%s missing clock-frequency property\n",
-   cn->full_name);
+   pr_err("%pOF missing clock-frequency property\n",
+   cn);
continue;
}

diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 748cfb8d5212..3c7b66c22697 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -187,21 +187,21 @@ static int __init exynos_pmu_irq_init(struct device_node 
*node,
struct irq_domain *parent_domain, *domain;

if (!parent) {
-   pr_err("%s: no parent, giving up\n", node->full_name);
+   pr_err("%pOF: no parent, giving up\n", node);
return -ENODEV;
}

parent_domain = irq_find_host(parent);
if (!parent_domain) {
-   pr_err("%s: unable to obtain parent domain\n", node->full_name);
+   pr_err("%pOF: unable to obtain parent domain\n", node);
return -ENXIO;
}

pmu_base_addr = of_iomap(node, 0);

if (!pmu_base_addr) {
-   pr_err("%s: failed to find exynos pmu register\n",
-  node->full_name);
+   

Re: [PATCH v4 3/3] v4l: async: add subnotifier to subdevices

[Niklas Söderlund]

Hi Hans,

On 2017-07-18 17:06:15 +0200, Hans Verkuil wrote:
> On 18/07/17 16:47, Niklas Söderlund wrote:
> >>>  void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> >>>  {
> >>> - struct v4l2_subdev *sd, *tmp;
> >>> + struct v4l2_subdev *sd, *tmp, **subdev;
> >>>   unsigned int notif_n_subdev = notifier->num_subdevs;
> >>>   unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
> >>>   struct device **dev;
> >>> @@ -217,6 +293,12 @@ void v4l2_async_notifier_unregister(struct 
> >>> v4l2_async_notifier *notifier)
> >>>   "Failed to allocate device cache!\n");
> >>>   }
> >>>  
> >>> + subdev = kvmalloc_array(n_subdev, sizeof(*subdev), GFP_KERNEL);
> >>> + if (!dev) {
> >>> + dev_err(notifier->v4l2_dev->dev,
> >>> + "Failed to allocate subdevice cache!\n");
> >>> + }
> >>> +
> >>
> >> How about making a little struct:
> >>
> >>struct whatever {
> >>struct device *dev;
> >>struct v4l2_subdev *sd;
> >>};
> >>
> >> and allocate an array of that. Only need to call kvmalloc_array once.
> > 
> > Neat idea, will do so for next version.
> > 
> >>
> >> Some comments after the dev_err of why you ignore the failed memory 
> >> allocation
> >> and what the consequences of that are would be helpful. It is unexpected 
> >> code,
> >> and that needs documentation.
> > 
> > I agree that it's unexpected and I don't know the reason for it, I was 
> > just mimic the existing behavior. If you are OK with it I be more then 
> > happy to add patch to this series returning -ENOMEM if the allocation 
> > failed as Geert pointed out if this allocation fails I think we are in a 
> > lot of trouble anyhow...
> > 
> > Let me know what you think, but I don't think I can add a comment 
> > explaining why the function don't simply abort on failure since I don't 
> > understand it myself.
> 
> So you don't understand the device_release_driver/device_attach reprobing bit 
> either?
> 
> I did some digging and found this thread:
> 
> http://lkml.iu.edu/hypermail/linux/kernel/1210.2/00713.html
> 
> It explains the reason for this.

Nice, thanks for digging this out.

> 
> I'm pretty sure Greg K-H never saw this code :-)
> 
> Looking in drivers/base/bus.c I see this function: device_reprobe().
> 
> I think we need to use that instead.

I have now looked at device_reprobe() and unfortunately it can't be used 
in v4l2_async_notifier_unregister(). This is because some v4l2 drivers 
call v4l2_async_notifier_unregister() in there remove functions leading 
to call chains similar to this:

SyS_delete_module()
  rcar_vin_driver_exit()
platform_driver_unregister()
  driver_unregister()
bus_remove_driver()
  driver_detach()
device_lock(dev->parent); <- Here the lock is taken
device_release_driver_internal()
  platform_drv_remove()
rcar_vin_remove()
  v4l2_async_notifier_unregister()
device_reprobe()
  device_lock(dev->parent); <- Here we dead lock

So we are stuck with calling device_release_driver() and device_attach() 
directly from v4l2-async.

> 
> Regards,
> 
>   Hans

-- 
Regards,
Niklas Söderlund

[RFC PATCH 0/2] clk: renesas: rcar-gen3-cpg: refactor accessing the SD div table

[Wolfram Sang]

Here is a patch series trying to implement the conclusions about accessing the
SD divider table, coming from this discussion [1]. Patch 1 is a tiny, yet
seperate cleanup. Patch 2 is the bulk of the work doing the checks only when 
needed.
I think the code looks much better now.

I tested this on a Salvator-X (R-Car M3-W) with highspeed and UHS SD cards. I
could access them and checksum files. I also did not notice any performance
regressions. There are also no WARNings in the boot log.

Please let me know if this is what you had in mind and if you can think of
other ways of testing this change.

Thanks,

   Wolfram

[1] https://patchwork.kernel.org/patch/9688509/

Wolfram Sang (2):
  clk: renesas: rcar-gen3-cpg: drop superfluous variable
  clk: renesas: rcar-gen3-cpg.c: refactor checks for accessing the div
table

 drivers/clk/renesas/rcar-gen3-cpg.c | 47 -
 1 file changed, 20 insertions(+), 27 deletions(-)

-- 
2.11.0

[RFC PATCH 1/2] clk: renesas: rcar-gen3-cpg: drop superfluous variable

[Wolfram Sang]

'rate' is not used, so we can use 'parent_rate' directly.

Signed-off-by: Wolfram Sang 
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index 3dee900522b703..71b8a986bd4889 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -135,7 +135,6 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw 
*hw,
unsigned long parent_rate)
 {
struct sd_clock *clock = to_sd_clock(hw);
-   unsigned long rate = parent_rate;
u32 val, sd_fc;
unsigned int i;
 
@@ -149,7 +148,7 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw 
*hw,
if (i >= clock->div_num)
return -EINVAL;
 
-   return DIV_ROUND_CLOSEST(rate, clock->div_table[i].div);
+   return DIV_ROUND_CLOSEST(parent_rate, clock->div_table[i].div);
 }
 
 static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
-- 
2.11.0

[RFC PATCH 2/2] clk: renesas: rcar-gen3-cpg.c: refactor checks for accessing the div table

[Wolfram Sang]

Do the checks for accessing the SD divider table only when the rate gets
updated, namely on init and set_rate. In all other cases, reuse the last
value. This simplifies code, runtime load, and error reporting.

Signed-off-by: Wolfram Sang 
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 46 -
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index 71b8a986bd4889..d4d27cf6110d38 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -60,6 +60,7 @@ struct sd_clock {
unsigned int div_num;
unsigned int div_min;
unsigned int div_max;
+   unsigned int cur_div_idx;
 };
 
 /* SDn divider
@@ -96,21 +97,10 @@ static const struct sd_div_table cpg_sd_div_table[] = {
 static int cpg_sd_clock_enable(struct clk_hw *hw)
 {
struct sd_clock *clock = to_sd_clock(hw);
-   u32 val, sd_fc;
-   unsigned int i;
-
-   val = readl(clock->reg);
-
-   sd_fc = val & CPG_SD_FC_MASK;
-   for (i = 0; i < clock->div_num; i++)
-   if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
-   break;
-
-   if (i >= clock->div_num)
-   return -EINVAL;
+   u32 val = readl(clock->reg);
 
val &= ~(CPG_SD_STP_MASK);
-   val |= clock->div_table[i].val & CPG_SD_STP_MASK;
+   val |= clock->div_table[clock->cur_div_idx].val & CPG_SD_STP_MASK;
 
writel(val, clock->reg);
 
@@ -135,20 +125,9 @@ static unsigned long cpg_sd_clock_recalc_rate(struct 
clk_hw *hw,
unsigned long parent_rate)
 {
struct sd_clock *clock = to_sd_clock(hw);
-   u32 val, sd_fc;
-   unsigned int i;
-
-   val = readl(clock->reg);
-
-   sd_fc = val & CPG_SD_FC_MASK;
-   for (i = 0; i < clock->div_num; i++)
-   if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
-   break;
-
-   if (i >= clock->div_num)
-   return -EINVAL;
 
-   return DIV_ROUND_CLOSEST(parent_rate, clock->div_table[i].div);
+   return DIV_ROUND_CLOSEST(parent_rate,
+clock->div_table[clock->cur_div_idx].div);
 }
 
 static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
@@ -189,6 +168,8 @@ static int cpg_sd_clock_set_rate(struct clk_hw *hw, 
unsigned long rate,
if (i >= clock->div_num)
return -EINVAL;
 
+   clock->cur_div_idx = i;
+
val = readl(clock->reg);
val &= ~(CPG_SD_STP_MASK | CPG_SD_FC_MASK);
val |= clock->div_table[i].val & (CPG_SD_STP_MASK | CPG_SD_FC_MASK);
@@ -214,6 +195,7 @@ static struct clk * __init cpg_sd_clk_register(const struct 
cpg_core_clk *core,
struct sd_clock *clock;
struct clk *clk;
unsigned int i;
+   u32 sd_fc;
 
clock = kzalloc(sizeof(*clock), GFP_KERNEL);
if (!clock)
@@ -230,6 +212,18 @@ static struct clk * __init cpg_sd_clk_register(const 
struct cpg_core_clk *core,
clock->div_table = cpg_sd_div_table;
clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
 
+   sd_fc = readl(clock->reg) & CPG_SD_FC_MASK;
+   for (i = 0; i < clock->div_num; i++)
+   if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
+   break;
+
+   if (WARN_ON(i >= clock->div_num)) {
+   kfree(clock);
+   return ERR_PTR(-EINVAL);
+   }
+
+   clock->cur_div_idx = i;
+
clock->div_max = clock->div_table[0].div;
clock->div_min = clock->div_max;
for (i = 1; i < clock->div_num; i++) {
-- 
2.11.0

Re: [PATCH v4 2/3] v4l: async: do not hold list_lock when reprobing devices

[Niklas Söderlund]

On 2017-07-18 16:50:15 +0200, Hans Verkuil wrote:
> On 18/07/17 16:39, Niklas Söderlund wrote:
> > Hi Hans,
> > 
> > Thanks for your feedback.
> > 
> > On 2017-07-18 16:22:14 +0200, Hans Verkuil wrote:
> >> On 17/07/17 18:59, Niklas Söderlund wrote:
> >>> There is no good reason to hold the list_lock when reprobing the devices
> >>> and it prevents a clean implementation of subdevice notifiers. Move the
> >>> actual release of the devices outside of the loop which requires the
> >>> lock to be held.
> >>>
> >>> Signed-off-by: Niklas Söderlund 
> >>> ---
> >>>  drivers/media/v4l2-core/v4l2-async.c | 29 ++---
> >>>  1 file changed, 10 insertions(+), 19 deletions(-)
> >>>
> >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> >>> b/drivers/media/v4l2-core/v4l2-async.c
> >>> index 0acf288d7227ba97..8fc84f7962386ddd 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-async.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-async.c
> >>> @@ -206,7 +206,7 @@ void v4l2_async_notifier_unregister(struct 
> >>> v4l2_async_notifier *notifier)
> >>>   unsigned int notif_n_subdev = notifier->num_subdevs;
> >>>   unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
> >>>   struct device **dev;
> >>> - int i = 0;
> >>> + int i, count = 0;
> >>>  
> >>>   if (!notifier->v4l2_dev)
> >>>   return;
> >>> @@ -222,37 +222,28 @@ void v4l2_async_notifier_unregister(struct 
> >>> v4l2_async_notifier *notifier)
> >>>   list_del(>list);
> >>>  
> >>>   list_for_each_entry_safe(sd, tmp, >done, async_list) {
> >>> - struct device *d;
> >>> -
> >>> - d = get_device(sd->dev);
> >>> + if (dev)
> >>> + dev[count] = get_device(sd->dev);
> >>> + count++;
> >>>  
> >>>   if (notifier->unbind)
> >>>   notifier->unbind(notifier, sd, sd->asd);
> >>>  
> >>>   v4l2_async_cleanup(sd);
> >>> + }
> >>>  
> >>> - /* If we handled USB devices, we'd have to lock the parent too 
> >>> */
> >>> - device_release_driver(d);
> >>> + mutex_unlock(_lock);
> >>>  
> >>> - /*
> >>> -  * Store device at the device cache, in order to call
> >>> -  * put_device() on the final step
> >>> -  */
> >>> + for (i = 0; i < count; i++) {
> >>> + /* If we handled USB devices, we'd have to lock the parent too 
> >>> */
> >>>   if (dev)
> >>> - dev[i++] = d;
> >>> - else
> >>> - put_device(d);
> >>> + device_release_driver(dev[i]);
> >>
> >> This changes the behavior. If the alloc failed, then at least put_device 
> >> was still called.
> >> Now that no longer happens.
> > 
> > Yes, but also changes the behavior to also only call get_device() if the 
> > allocation was successful. So the behavior is kept the same as far as I 
> > understands it.
> 
> Ah, I missed that. Sorry about that.
> 
> But regardless of that the device_release_driver(d) isn't called anymore.
> It's not clear at all to me whether that is a problem or not.

You are right I missed that, thanks for pointing it out, please see 
bellow.

> 
> > 
> >>
> >> Frankly I don't understand this code, it is in desperate need of some 
> >> comments explaining
> >> this whole reprobing thing.
> > 
> > I agree that the code is in need of comments, but I feel a patch that 
> > separates the v4l2-async work from the re-probing work is a step in the 
> > right direction :-)
> 
> Would it help to simplify this function to:
> 
> dev = kvmalloc_array(n_subdev, sizeof(*dev), GFP_KERNEL);
> if (!dev) {
> dev_err(notifier->v4l2_dev->dev,
> "Failed to allocate device cache!\n");
> 
>   mutex_lock(_lock);
> 
>   list_del(>list);
> 
>   /* this assumes device_release_driver(d) isn't necessary */
>   list_for_each_entry_safe(sd, tmp, >done, async_list) {
>   if (notifier->unbind)
>   notifier->unbind(notifier, sd, sd->asd);
> 
>   v4l2_async_cleanup(sd);
>   }
> 
>   mutex_unlock(_lock);
>   return;
>   }
> 
>   ...and here the code where dev is non-NULL...
> 
> Yes, there is some code duplication, but it is a lot easier to understand.

I be fine with this, or simply aborting with -ENOMEM if the allocation 
fails. If the allocation fails I say we are in a lot of trouble anyhow, 
as Geert pointed out the kernel would already printed a warning and 
invoked the OOM-killer.

If you are OK with it I will rework the next version of this series to 
introduce this behavior. Let me know what you think.

> 
> Regards,
> 
>   Hans
> 
> > 
> >>
> >> I have this strong feeling that this function needs to be reworked.
> > 
> > I also strongly agree with this.
> > 
> >>
> >> Regards,
> >>
> >>Hans
> >>
> >>>   }
> >>>  
> >>> - mutex_unlock(_lock);
> 

Re: [PATCH v4 3/3] v4l: async: add subnotifier to subdevices

[Hans Verkuil]

On 18/07/17 16:47, Niklas Söderlund wrote:
>>>  void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
>>>  {
>>> -   struct v4l2_subdev *sd, *tmp;
>>> +   struct v4l2_subdev *sd, *tmp, **subdev;
>>> unsigned int notif_n_subdev = notifier->num_subdevs;
>>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
>>> struct device **dev;
>>> @@ -217,6 +293,12 @@ void v4l2_async_notifier_unregister(struct 
>>> v4l2_async_notifier *notifier)
>>> "Failed to allocate device cache!\n");
>>> }
>>>  
>>> +   subdev = kvmalloc_array(n_subdev, sizeof(*subdev), GFP_KERNEL);
>>> +   if (!dev) {
>>> +   dev_err(notifier->v4l2_dev->dev,
>>> +   "Failed to allocate subdevice cache!\n");
>>> +   }
>>> +
>>
>> How about making a little struct:
>>
>>  struct whatever {
>>  struct device *dev;
>>  struct v4l2_subdev *sd;
>>  };
>>
>> and allocate an array of that. Only need to call kvmalloc_array once.
> 
> Neat idea, will do so for next version.
> 
>>
>> Some comments after the dev_err of why you ignore the failed memory 
>> allocation
>> and what the consequences of that are would be helpful. It is unexpected 
>> code,
>> and that needs documentation.
> 
> I agree that it's unexpected and I don't know the reason for it, I was 
> just mimic the existing behavior. If you are OK with it I be more then 
> happy to add patch to this series returning -ENOMEM if the allocation 
> failed as Geert pointed out if this allocation fails I think we are in a 
> lot of trouble anyhow...
> 
> Let me know what you think, but I don't think I can add a comment 
> explaining why the function don't simply abort on failure since I don't 
> understand it myself.

So you don't understand the device_release_driver/device_attach reprobing bit 
either?

I did some digging and found this thread:

http://lkml.iu.edu/hypermail/linux/kernel/1210.2/00713.html

It explains the reason for this.

I'm pretty sure Greg K-H never saw this code :-)

Looking in drivers/base/bus.c I see this function: device_reprobe().

I think we need to use that instead.

Regards,

Hans

Re: [PATCH v4 2/3] v4l: async: do not hold list_lock when reprobing devices

[Hans Verkuil]

On 18/07/17 16:39, Niklas Söderlund wrote:
> Hi Hans,
> 
> Thanks for your feedback.
> 
> On 2017-07-18 16:22:14 +0200, Hans Verkuil wrote:
>> On 17/07/17 18:59, Niklas Söderlund wrote:
>>> There is no good reason to hold the list_lock when reprobing the devices
>>> and it prevents a clean implementation of subdevice notifiers. Move the
>>> actual release of the devices outside of the loop which requires the
>>> lock to be held.
>>>
>>> Signed-off-by: Niklas Söderlund 
>>> ---
>>>  drivers/media/v4l2-core/v4l2-async.c | 29 ++---
>>>  1 file changed, 10 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
>>> b/drivers/media/v4l2-core/v4l2-async.c
>>> index 0acf288d7227ba97..8fc84f7962386ddd 100644
>>> --- a/drivers/media/v4l2-core/v4l2-async.c
>>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>>> @@ -206,7 +206,7 @@ void v4l2_async_notifier_unregister(struct 
>>> v4l2_async_notifier *notifier)
>>> unsigned int notif_n_subdev = notifier->num_subdevs;
>>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
>>> struct device **dev;
>>> -   int i = 0;
>>> +   int i, count = 0;
>>>  
>>> if (!notifier->v4l2_dev)
>>> return;
>>> @@ -222,37 +222,28 @@ void v4l2_async_notifier_unregister(struct 
>>> v4l2_async_notifier *notifier)
>>> list_del(>list);
>>>  
>>> list_for_each_entry_safe(sd, tmp, >done, async_list) {
>>> -   struct device *d;
>>> -
>>> -   d = get_device(sd->dev);
>>> +   if (dev)
>>> +   dev[count] = get_device(sd->dev);
>>> +   count++;
>>>  
>>> if (notifier->unbind)
>>> notifier->unbind(notifier, sd, sd->asd);
>>>  
>>> v4l2_async_cleanup(sd);
>>> +   }
>>>  
>>> -   /* If we handled USB devices, we'd have to lock the parent too 
>>> */
>>> -   device_release_driver(d);
>>> +   mutex_unlock(_lock);
>>>  
>>> -   /*
>>> -* Store device at the device cache, in order to call
>>> -* put_device() on the final step
>>> -*/
>>> +   for (i = 0; i < count; i++) {
>>> +   /* If we handled USB devices, we'd have to lock the parent too 
>>> */
>>> if (dev)
>>> -   dev[i++] = d;
>>> -   else
>>> -   put_device(d);
>>> +   device_release_driver(dev[i]);
>>
>> This changes the behavior. If the alloc failed, then at least put_device was 
>> still called.
>> Now that no longer happens.
> 
> Yes, but also changes the behavior to also only call get_device() if the 
> allocation was successful. So the behavior is kept the same as far as I 
> understands it.

Ah, I missed that. Sorry about that.

But regardless of that the device_release_driver(d) isn't called anymore.
It's not clear at all to me whether that is a problem or not.

> 
>>
>> Frankly I don't understand this code, it is in desperate need of some 
>> comments explaining
>> this whole reprobing thing.
> 
> I agree that the code is in need of comments, but I feel a patch that 
> separates the v4l2-async work from the re-probing work is a step in the 
> right direction :-)

Would it help to simplify this function to:

dev = kvmalloc_array(n_subdev, sizeof(*dev), GFP_KERNEL);
if (!dev) {
dev_err(notifier->v4l2_dev->dev,
"Failed to allocate device cache!\n");

mutex_lock(_lock);

list_del(>list);

/* this assumes device_release_driver(d) isn't necessary */
list_for_each_entry_safe(sd, tmp, >done, async_list) {
if (notifier->unbind)
notifier->unbind(notifier, sd, sd->asd);

v4l2_async_cleanup(sd);
}

mutex_unlock(_lock);
return;
}

...and here the code where dev is non-NULL...

Yes, there is some code duplication, but it is a lot easier to understand.

Regards,

Hans

> 
>>
>> I have this strong feeling that this function needs to be reworked.
> 
> I also strongly agree with this.
> 
>>
>> Regards,
>>
>>  Hans
>>
>>> }
>>>  
>>> -   mutex_unlock(_lock);
>>> -
>>> /*
>>>  * Call device_attach() to reprobe devices
>>> -*
>>> -* NOTE: If dev allocation fails, i is 0, and the whole loop won't be
>>> -* executed.
>>>  */
>>> -   while (i--) {
>>> +   for (i = 0; dev && i < count; i++) {
>>> struct device *d = dev[i];
>>>  
>>> if (d && device_attach(d) < 0) {
>>>
>>
> 

Re: [PATCH v4 3/3] v4l: async: add subnotifier to subdevices

[Niklas Söderlund]

Hi Hans,

Thanks for your feedback.

On 2017-07-18 16:22:43 +0200, Hans Verkuil wrote:
> On 17/07/17 18:59, Niklas Söderlund wrote:
> > Add a subdevice specific notifier which can be used by a subdevice
> > driver to compliment the master device notifier to extend the subdevice
> 
> compliment -> complement
> 
> Just one character difference, but a wildly different meaning :-)
> 
> Although it was very polite of the subdevice driver to compliment the
> master driver. Impeccable manners. :-)

It's all about good manners, is it not? But yes the next version of the 
series won't be this polite.

> 
> > discovery.
> > 
> > The master device registers the subdevices closest to itself in its
> > notifier while the subdevice(s) register notifiers for their closest
> > neighboring devices. Subdevice drivers configures a notifier at probe
> 
> configures -> configure
> 
> > time which are registered by the v4l2-async framework once the subdevice
> 
> are -> is
> 
> > itself is register, since it's only at this point the v4l2_dev is
> 
> register -> registered

Thanks.

> 
> > available to the subnotifier.
> > 
> > Using this incremental approach two problems can be solved:
> > 
> > 1. The master device no longer has to care how many devices exist in
> >the pipeline. It only needs to care about its closest subdevice and
> >arbitrary long pipelines can be created without having to adapt the
> >master device for each case.
> > 
> > 2. Subdevices which are represented as a single DT node but register
> >more than one subdevice can use this to improve the pipeline
> >discovery, since the subdevice driver is the only one who knows which
> >of its subdevices is linked with which subdevice of a neighboring DT
> >node.
> > 
> > To allow subdevices to provide its own list of subdevices to the
> 
> its -> their

Will fix.

> 
> > v4l2-async framework v4l2_async_subdev_register_notifier() is added.
> > This new function must be called before the subdevice itself is
> > registered with the v4l2-async framework using
> > v4l2_async_register_subdev().
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  Documentation/media/kapi/v4l2-subdev.rst |  12 +++
> >  drivers/media/v4l2-core/v4l2-async.c | 134 
> > +--
> >  include/media/v4l2-async.h   |  25 ++
> >  include/media/v4l2-subdev.h  |   5 ++
> >  4 files changed, 168 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/media/kapi/v4l2-subdev.rst 
> > b/Documentation/media/kapi/v4l2-subdev.rst
> > index e1f0b726e438f963..5957176965a6a3ef 100644
> > --- a/Documentation/media/kapi/v4l2-subdev.rst
> > +++ b/Documentation/media/kapi/v4l2-subdev.rst
> > @@ -262,6 +262,18 @@ is called. After all subdevices have been located the 
> > .complete() callback is
> >  called. When a subdevice is removed from the system the .unbind() method is
> >  called. All three callbacks are optional.
> >  
> > +Subdevice drivers might in turn register subnotifier objects with an
> > +array of other subdevice descriptors that the subdevice needs for its
> > +own operation. Subnotifiers are an extension of the bridge drivers
> > +notifier to allow for a incremental registering and matching of
> > +subdevices. This is useful when a driver only has information about
> > +which subdevice is closest to itself and would require knowledge from the
> > +driver of that subdevice to know which other subdevice(s) lie beyond.
> > +By registering subnotifiers drivers can incrementally move the subdevice
> > +matching down the chain of drivers. This is performed using the
> > +:c:func:`v4l2_async_subdev_register_notifier` call which must be performed
> > +before registering the subdevice using 
> > :c:func:`v4l2_async_register_subdev`.
> > +
> >  V4L2 sub-device userspace API
> >  -
> >  
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 8fc84f7962386ddd..558fb3ec07e7fba8 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -100,6 +100,61 @@ static struct v4l2_async_subdev 
> > *v4l2_async_belongs(struct v4l2_async_notifier *
> > return NULL;
> >  }
> >  
> > +static int v4l2_async_notifier_complete(struct v4l2_async_notifier 
> > *notifier)
> > +{
> > +   struct v4l2_subdev *sd, *tmp;
> > +
> > +   if (!notifier->num_subdevs)
> > +   return 0;
> > +
> > +   list_for_each_entry_safe(sd, tmp, >done, async_list) {
> > +   v4l2_async_notifier_complete(>subnotifier);
> > +   }
> 
> Curly brackets aren't needed here (didn't checkpatch complain about this?)

It did not, and I was unsure how to handle this since 
list_for_each_entry_safe() is a macro. Will drop them for next version 
provided checkpatch don't complain about it.

> 
> > +
> > +   if (notifier->complete)
> > +   return notifier->complete(notifier);
> > +

Re: [PATCH v4 2/3] v4l: async: do not hold list_lock when reprobing devices

[Niklas Söderlund]

Hi Hans,

Thanks for your feedback.

On 2017-07-18 16:22:14 +0200, Hans Verkuil wrote:
> On 17/07/17 18:59, Niklas Söderlund wrote:
> > There is no good reason to hold the list_lock when reprobing the devices
> > and it prevents a clean implementation of subdevice notifiers. Move the
> > actual release of the devices outside of the loop which requires the
> > lock to be held.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 29 ++---
> >  1 file changed, 10 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 0acf288d7227ba97..8fc84f7962386ddd 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -206,7 +206,7 @@ void v4l2_async_notifier_unregister(struct 
> > v4l2_async_notifier *notifier)
> > unsigned int notif_n_subdev = notifier->num_subdevs;
> > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
> > struct device **dev;
> > -   int i = 0;
> > +   int i, count = 0;
> >  
> > if (!notifier->v4l2_dev)
> > return;
> > @@ -222,37 +222,28 @@ void v4l2_async_notifier_unregister(struct 
> > v4l2_async_notifier *notifier)
> > list_del(>list);
> >  
> > list_for_each_entry_safe(sd, tmp, >done, async_list) {
> > -   struct device *d;
> > -
> > -   d = get_device(sd->dev);
> > +   if (dev)
> > +   dev[count] = get_device(sd->dev);
> > +   count++;
> >  
> > if (notifier->unbind)
> > notifier->unbind(notifier, sd, sd->asd);
> >  
> > v4l2_async_cleanup(sd);
> > +   }
> >  
> > -   /* If we handled USB devices, we'd have to lock the parent too 
> > */
> > -   device_release_driver(d);
> > +   mutex_unlock(_lock);
> >  
> > -   /*
> > -* Store device at the device cache, in order to call
> > -* put_device() on the final step
> > -*/
> > +   for (i = 0; i < count; i++) {
> > +   /* If we handled USB devices, we'd have to lock the parent too 
> > */
> > if (dev)
> > -   dev[i++] = d;
> > -   else
> > -   put_device(d);
> > +   device_release_driver(dev[i]);
> 
> This changes the behavior. If the alloc failed, then at least put_device was 
> still called.
> Now that no longer happens.

Yes, but also changes the behavior to also only call get_device() if the 
allocation was successful. So the behavior is kept the same as far as I 
understands it.

> 
> Frankly I don't understand this code, it is in desperate need of some 
> comments explaining
> this whole reprobing thing.

I agree that the code is in need of comments, but I feel a patch that 
separates the v4l2-async work from the re-probing work is a step in the 
right direction :-)

> 
> I have this strong feeling that this function needs to be reworked.

I also strongly agree with this.

> 
> Regards,
> 
>   Hans
> 
> > }
> >  
> > -   mutex_unlock(_lock);
> > -
> > /*
> >  * Call device_attach() to reprobe devices
> > -*
> > -* NOTE: If dev allocation fails, i is 0, and the whole loop won't be
> > -* executed.
> >  */
> > -   while (i--) {
> > +   for (i = 0; dev && i < count; i++) {
> > struct device *d = dev[i];
> >  
> > if (d && device_attach(d) < 0) {
> > 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v4 2/2] media: entity: Add media_entity_get_fwnode_pad() function

[Kieran Bingham]

Hi Niklas,

Small spelling error discovered in here:

On 15/06/17 10:17, Niklas Söderlund wrote:
> This is a wrapper around the media entity get_fwnode_pad operation.
> 
> Signed-off-by: Niklas Söderlund 
> Acked-by: Sakari Ailus 
> ---
>  drivers/media/media-entity.c | 36 
>  include/media/media-entity.h | 23 +++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index bc44193efa4798b4..82d6755bd5d0d5f0 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -18,6 +18,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -386,6 +387,41 @@ struct media_entity *media_graph_walk_next(struct 
> media_graph *graph)
>  }
>  EXPORT_SYMBOL_GPL(media_graph_walk_next);
>  
> +int media_entity_get_fwnode_pad(struct media_entity *entity,
> + struct fwnode_handle *fwnode,

Could fwnode be 'ep' or such to show that we want the pad of the remote 
endpoint?

fwnode is confusing ...

> + unsigned long direction_flags)
> +{
> + struct fwnode_endpoint endpoint;
> + unsigned int i;
> + int ret;
> +
> + if (!entity->ops || !entity->ops->get_fwnode_pad) {
> + for (i = 0; i < entity->num_pads; i++) {
> + if (entity->pads[i].flags & direction_flags)
> + return i;
> + }
> +
> + return -ENXIO;
> + }
> +
> + ret = fwnode_graph_parse_endpoint(fwnode, );
> + if (ret)
> + return ret;
> +
> + ret = entity->ops->get_fwnode_pad();
> + if (ret < 0)
> + return ret;
> +
> + if (ret >= entity->num_pads)
> + return -ENXIO;
> +
> + if (!(entity->pads[ret].flags & direction_flags))
> + return -ENXIO;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_entity_get_fwnode_pad);
> +
>  /* 
> -
>   * Pipeline management
>   */
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 46eeb036aa330534..754182d296689675 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -821,6 +821,29 @@ struct media_pad *media_entity_remote_pad(struct 
> media_pad *pad);
>  struct media_entity *media_entity_get(struct media_entity *entity);
>  
>  /**
> + * media_entity_get_fwnode_pad - Get pad number from fwnode
> + *
> + * @entity: The entity
> + * @fwnode: Pointer to the fwnode_handle which should be used to find the pad
> + * @direction_flags: Expected direction of the pad, as defined in
> + *:ref:`include/uapi/linux/media.h `
> + *(seek for ``MEDIA_PAD_FL_*``)
> + *
> + * This function can be used to resolve the media pad number from
> + * a fwnode. This is useful for devices which use more complex
> + * mappings of media pads.
> + *
> + * If the entity dose not implement the get_fwnode_pad() operation

s/dose/does/

> + * then this function searches the entity for the first pad that
> + * matches the @direction_flags.
> + *
> + * Return: returns the pad number on success or a negative error code.
> + */
> +int media_entity_get_fwnode_pad(struct media_entity *entity,
> + struct fwnode_handle *fwnode,
> + unsigned long direction_flags);
> +
> +/**
>   * media_graph_walk_init - Allocate resources used by graph walk.
>   *
>   * @graph: Media graph structure that will be used to walk the graph
> 

[PATCH v2 3/3] ARM: dts: r8a7794: Add SMP support

[Geert Uytterhoeven]

From: Sergei Shtylyov 

Add the device tree node for the Advanced Power Management Unit (APMU).
Use the "enable-method" prop to  point out that the APMU should be used
for the SMP support.

Signed-off-by: Sergei Shtylyov 
Signed-off-by: Geert Uytterhoeven 
---
v2:
  - No changes.
---
 arch/arm/boot/dts/r8a7794.dtsi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi
index a4c35d29f77c16dc..8d70e860468c5603 100644
--- a/arch/arm/boot/dts/r8a7794.dtsi
+++ b/arch/arm/boot/dts/r8a7794.dtsi
@@ -37,6 +37,7 @@
cpus {
#address-cells = <1>;
#size-cells = <0>;
+   enable-method = "renesas,apmu";
 
cpu0: cpu@0 {
device_type = "cpu";
@@ -65,6 +66,12 @@
};
};
 
+   apmu@e6151000 {
+   compatible = "renesas,r8a7794-apmu", "renesas,apmu";
+   reg = <0 0xe6151000 0 0x188>;
+   cpus = < >;
+   };
+
gic: interrupt-controller@f1001000 {
compatible = "arm,gic-400";
#interrupt-cells = <3>;
-- 
2.7.4

[PATCH v2 1/3] ARM: Add definition for monitor mode

[Geert Uytterhoeven]

 provides *_MODE definitions for the various processor
modes, but monitor mode was missing.

Add MON_MODE to avoid code using the hardcoded value.

Suggested-by: Marc Zyngier 
Signed-off-by: Geert Uytterhoeven 
---
ARM maintainers: Please provide your ack, as this is a dependency for a
mach-shmobile patch.

v2:
  - New.
---
 arch/arm/include/uapi/asm/ptrace.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/uapi/asm/ptrace.h 
b/arch/arm/include/uapi/asm/ptrace.h
index 5af0ed1b825a2aa9..70ff6bf489f31193 100644
--- a/arch/arm/include/uapi/asm/ptrace.h
+++ b/arch/arm/include/uapi/asm/ptrace.h
@@ -53,6 +53,7 @@
 #endif
 #define FIQ_MODE   0x0011
 #define IRQ_MODE   0x0012
+#define MON_MODE   0x0016
 #define ABT_MODE   0x0017
 #define HYP_MODE   0x001a
 #define UND_MODE   0x001b
-- 
2.7.4

[PATCH v2 2/3] ARM: shmobile: rcar-gen2: Make sure CNTVOFF is initialized on CA7/15

[Geert Uytterhoeven]

On Cortex-A7, the arch timer CNTVOFF register is uninitialized.
Ideally it should be initialized by the boot loader, but it isn't.

For the boot CPU, CNTVOFF is initialized by Linux since commit
9ce3fa6816c2fb59 ("ARM: shmobile: rcar-gen2: Add CA7 arch_timer
initialization for r8a7794").
For secondary CPU cores, no such initialization is done.

Hence when enabling SMP on r8a7794, the kernel log is spammed with:

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

As Marc Zyngier pointed out that Cortex-A15 and Cortex-A7 are similar with
respect to CNTVOFF, we have been very lucky this just worked on R-Car
Gen2 SoCs with Cortex-A15 cores.

To fix this:
  - Move the existing inline asm code to initialize CNTVOFF to an
assembler source file (adding comments and replacing hardcoded
constants by definitions in the process), so it can be reused,
  - Perform the initialization of CNTVOFF on the boot CPU (Cortex-A15 or
Cortex-A7) on all R-Car Gen2 and RZ/G1 parts,
  - Wrap the standard secondary_startup() routine inside a routine which
initializes CNTVOFF.

Based on patches by Hisashi Nakamura in the BSP.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Initialize CNTVOFF on Cortex-A15, too,
  - Use *_MODE definitions instead of hardcoded values,
  - Reduce duplication by calling the asm version from C,
  - Always build headsmp-apmu.o on R-Car Gen2.
---
 arch/arm/mach-shmobile/Makefile  |  1 +
 arch/arm/mach-shmobile/common.h  |  2 ++
 arch/arm/mach-shmobile/headsmp-apmu.S| 37 
 arch/arm/mach-shmobile/platsmp-apmu.c|  2 +-
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 20 ++---
 5 files changed, 43 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm/mach-shmobile/headsmp-apmu.S

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 64611a1b4276517b..32176a00c664ba16 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -22,6 +22,7 @@ cpu-y := platsmp.o headsmp.o
 # Shared SoC family objects
 obj-$(CONFIG_ARCH_RCAR_GEN2)   += setup-rcar-gen2.o platsmp-apmu.o $(cpu-y)
 CFLAGS_setup-rcar-gen2.o   += -march=armv7-a
+obj-$(CONFIG_ARCH_RCAR_GEN2)   += headsmp-apmu.o
 obj-$(CONFIG_ARCH_R8A7790) += regulator-quirk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7791) += regulator-quirk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7793) += regulator-quirk-rcar-gen2.o
diff --git a/arch/arm/mach-shmobile/common.h b/arch/arm/mach-shmobile/common.h
index 1a8f7b3ab449db56..ea6e9e2be3f7f4da 100644
--- a/arch/arm/mach-shmobile/common.h
+++ b/arch/arm/mach-shmobile/common.h
@@ -1,6 +1,7 @@
 #ifndef __ARCH_MACH_COMMON_H
 #define __ARCH_MACH_COMMON_H
 
+extern void shmobile_init_cntvoff(void);
 extern void shmobile_init_delay(void);
 extern void shmobile_boot_vector(void);
 extern unsigned long shmobile_boot_fn;
@@ -11,6 +12,7 @@ extern void shmobile_smp_hook(unsigned int cpu, unsigned long 
fn,
  unsigned long arg);
 extern bool shmobile_smp_cpu_can_disable(unsigned int cpu);
 extern bool shmobile_smp_init_fallback_ops(void);
+extern void shmobile_boot_apmu(void);
 extern void shmobile_boot_scu(void);
 extern void shmobile_smp_scu_prepare_cpus(phys_addr_t scu_base_phys,
  unsigned int max_cpus);
diff --git a/arch/arm/mach-shmobile/headsmp-apmu.S 
b/arch/arm/mach-shmobile/headsmp-apmu.S
new file mode 100644
index ..933d3190ffc02066
--- /dev/null
+++ b/arch/arm/mach-shmobile/headsmp-apmu.S
@@ -0,0 +1,37 @@
+/*
+ * SMP support for APMU based systems with Cortex A7/A15
+ *
+ * Copyright (C) 2014  Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+
+ENTRY(shmobile_init_cntvoff)
+   /*
+* CNTVOFF has to be initialized either from non-secure Hypervisor
+* mode or secure Monitor mode with SCR.NS==1. If TrustZone is enabled
+* then it should be handled by the secure code
+*/
+   cps #MON_MODE
+   mrc p15, 0, r1, c1, c1, 0   /* Get Secure Config */
+   orr r0, r1, #1
+   mcr p15, 0, r0, c1, c1, 0   /* Set Non Secure bit */
+   isb
+   mov r0, #0
+   mcrrp15, 4, r0, r0, c14 /* CNTVOFF = 0 */
+   isb
+   mcr p15, 0, r1, c1, c1, 0   /* Set Secure bit */
+   isb
+   cps #SVC_MODE
+   ret lr
+ENDPROC(shmobile_init_cntvoff)
+
+ENTRY(shmobile_boot_apmu)
+   bl  shmobile_init_cntvoff
+   b   secondary_startup
+ENDPROC(shmobile_boot_apmu)

[PATCH v2 0/3] ARM: renesas: Enable SMP on R-Car E2

[Geert Uytterhoeven]

Hi all,

This patch series enables SMP on R-Car E2 (r8a7794).
The main hurdle here is that R-Car Gen2 boot loaders do not initialize the
arch_timer CNTVOFF register, which thus needs workarounds on Linux.

  - The first patch adds a definition for MON_MODE, as suggested by Marc
Zyngier,
  - The second patch makes sure CNTVOFF is initialized for boot and
secondary Cortex-A15 and Cortex-A7 CPU cores, like is already done
for the boot Cortex-A7 CPU core.  Without this, the ARM arch timer
does not work on secondary CPU cores.
This patch depends on "[PATCH v2] ARM: shmobile: rcar-gen2: Correct
arch timer frequency on RZ/G1E".
  - The third patch adds the required infrastructure (APMU device node
and corresponding enable-method) to DT.
Obviously this must not be applied on a branch that does not contain
the first two patches!

Due to dependencies, I think it is easiest if the ARM maintainers provide
their Acked-by for patch 1, so the whole series can go in through Simon's
Renesas tree.

Changes compared to v1:
  - New patch "[PATCH v2 1/3] ARM: Add definition for monitor mode",
  - Initialize CNTVOFF on Cortex-A15, too,
  - Use *_MODE definitions instead of hardcoded values,
  - Reduce duplication by calling the asm version from C,
  - Always build headsmp-apmu.o on R-Car Gen2.

This has been tested on r8a7794/alt (dual Cortex-A7), and regression-tested
on r8a7790/lager (quad Cortex-A15), and r8a7791/koelsch, r8a7791/porter,
r8a7792/blanche, and r8a7793/gose (dual Cortex-A15).

Thanks!

Geert Uytterhoeven (2):
  ARM: Add definition for monitor mode
  ARM: shmobile: rcar-gen2: Make sure CNTVOFF is initialized on CA7/15

Sergei Shtylyov (1):
  ARM: dts: r8a7794: Add SMP support

 arch/arm/boot/dts/r8a7794.dtsi   |  7 ++
 arch/arm/include/uapi/asm/ptrace.h   |  1 +
 arch/arm/mach-shmobile/Makefile  |  1 +
 arch/arm/mach-shmobile/common.h  |  2 ++
 arch/arm/mach-shmobile/headsmp-apmu.S| 37 
 arch/arm/mach-shmobile/platsmp-apmu.c|  2 +-
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 20 ++---
 7 files changed, 51 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm/mach-shmobile/headsmp-apmu.S

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

Re: [PATCH v4 3/3] v4l: async: add subnotifier to subdevices

[Hans Verkuil]

On 17/07/17 18:59, Niklas Söderlund wrote:
> Add a subdevice specific notifier which can be used by a subdevice
> driver to compliment the master device notifier to extend the subdevice

compliment -> complement

Just one character difference, but a wildly different meaning :-)

Although it was very polite of the subdevice driver to compliment the
master driver. Impeccable manners. :-)

> discovery.
> 
> The master device registers the subdevices closest to itself in its
> notifier while the subdevice(s) register notifiers for their closest
> neighboring devices. Subdevice drivers configures a notifier at probe

configures -> configure

> time which are registered by the v4l2-async framework once the subdevice

are -> is

> itself is register, since it's only at this point the v4l2_dev is

register -> registered

> available to the subnotifier.
> 
> Using this incremental approach two problems can be solved:
> 
> 1. The master device no longer has to care how many devices exist in
>the pipeline. It only needs to care about its closest subdevice and
>arbitrary long pipelines can be created without having to adapt the
>master device for each case.
> 
> 2. Subdevices which are represented as a single DT node but register
>more than one subdevice can use this to improve the pipeline
>discovery, since the subdevice driver is the only one who knows which
>of its subdevices is linked with which subdevice of a neighboring DT
>node.
> 
> To allow subdevices to provide its own list of subdevices to the

its -> their

> v4l2-async framework v4l2_async_subdev_register_notifier() is added.
> This new function must be called before the subdevice itself is
> registered with the v4l2-async framework using
> v4l2_async_register_subdev().
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  Documentation/media/kapi/v4l2-subdev.rst |  12 +++
>  drivers/media/v4l2-core/v4l2-async.c | 134 
> +--
>  include/media/v4l2-async.h   |  25 ++
>  include/media/v4l2-subdev.h  |   5 ++
>  4 files changed, 168 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/media/kapi/v4l2-subdev.rst 
> b/Documentation/media/kapi/v4l2-subdev.rst
> index e1f0b726e438f963..5957176965a6a3ef 100644
> --- a/Documentation/media/kapi/v4l2-subdev.rst
> +++ b/Documentation/media/kapi/v4l2-subdev.rst
> @@ -262,6 +262,18 @@ is called. After all subdevices have been located the 
> .complete() callback is
>  called. When a subdevice is removed from the system the .unbind() method is
>  called. All three callbacks are optional.
>  
> +Subdevice drivers might in turn register subnotifier objects with an
> +array of other subdevice descriptors that the subdevice needs for its
> +own operation. Subnotifiers are an extension of the bridge drivers
> +notifier to allow for a incremental registering and matching of
> +subdevices. This is useful when a driver only has information about
> +which subdevice is closest to itself and would require knowledge from the
> +driver of that subdevice to know which other subdevice(s) lie beyond.
> +By registering subnotifiers drivers can incrementally move the subdevice
> +matching down the chain of drivers. This is performed using the
> +:c:func:`v4l2_async_subdev_register_notifier` call which must be performed
> +before registering the subdevice using :c:func:`v4l2_async_register_subdev`.
> +
>  V4L2 sub-device userspace API
>  -
>  
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 8fc84f7962386ddd..558fb3ec07e7fba8 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -100,6 +100,61 @@ static struct v4l2_async_subdev 
> *v4l2_async_belongs(struct v4l2_async_notifier *
>   return NULL;
>  }
>  
> +static int v4l2_async_notifier_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd, *tmp;
> +
> + if (!notifier->num_subdevs)
> + return 0;
> +
> + list_for_each_entry_safe(sd, tmp, >done, async_list) {
> + v4l2_async_notifier_complete(>subnotifier);
> + }

Curly brackets aren't needed here (didn't checkpatch complain about this?)

> +
> + if (notifier->complete)
> + return notifier->complete(notifier);
> +
> + return 0;
> +}
> +
> +static bool
> +v4l2_async_is_notifier_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd, *tmp;
> +
> + if (!list_empty(>waiting))
> + return false;
> +
> + list_for_each_entry_safe(sd, tmp, >done, async_list) {
> + /* Don't consider empty subnotifiers */
> + if (!sd->subnotifier.num_subdevs)
> + continue;
> +
> + if (!v4l2_async_is_notifier_complete(>subnotifier))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static int

Re: [PATCH v4 2/3] v4l: async: do not hold list_lock when reprobing devices

[Hans Verkuil]

On 17/07/17 18:59, Niklas Söderlund wrote:
> There is no good reason to hold the list_lock when reprobing the devices
> and it prevents a clean implementation of subdevice notifiers. Move the
> actual release of the devices outside of the loop which requires the
> lock to be held.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 29 ++---
>  1 file changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 0acf288d7227ba97..8fc84f7962386ddd 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -206,7 +206,7 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>   unsigned int notif_n_subdev = notifier->num_subdevs;
>   unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
>   struct device **dev;
> - int i = 0;
> + int i, count = 0;
>  
>   if (!notifier->v4l2_dev)
>   return;
> @@ -222,37 +222,28 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>   list_del(>list);
>  
>   list_for_each_entry_safe(sd, tmp, >done, async_list) {
> - struct device *d;
> -
> - d = get_device(sd->dev);
> + if (dev)
> + dev[count] = get_device(sd->dev);
> + count++;
>  
>   if (notifier->unbind)
>   notifier->unbind(notifier, sd, sd->asd);
>  
>   v4l2_async_cleanup(sd);
> + }
>  
> - /* If we handled USB devices, we'd have to lock the parent too 
> */
> - device_release_driver(d);
> + mutex_unlock(_lock);
>  
> - /*
> -  * Store device at the device cache, in order to call
> -  * put_device() on the final step
> -  */
> + for (i = 0; i < count; i++) {
> + /* If we handled USB devices, we'd have to lock the parent too 
> */
>   if (dev)
> - dev[i++] = d;
> - else
> - put_device(d);
> + device_release_driver(dev[i]);

This changes the behavior. If the alloc failed, then at least put_device was 
still called.
Now that no longer happens.

Frankly I don't understand this code, it is in desperate need of some comments 
explaining
this whole reprobing thing.

I have this strong feeling that this function needs to be reworked.

Regards,

Hans

>   }
>  
> - mutex_unlock(_lock);
> -
>   /*
>* Call device_attach() to reprobe devices
> -  *
> -  * NOTE: If dev allocation fails, i is 0, and the whole loop won't be
> -  * executed.
>*/
> - while (i--) {
> + for (i = 0; dev && i < count; i++) {
>   struct device *d = dev[i];
>  
>   if (d && device_attach(d) < 0) {
> 

RE: [PATCH] ARM: shmobile: rcar-gen2: Correct arch timer frequency on RZ/G1E

[Biju Das]

It looks good to me.

Regards,
Biju

> -Original Message-
> From: Geert Uytterhoeven [mailto:geert+rene...@glider.be]
> Sent: 18 July 2017 14:29
> To: Simon Horman ; Magnus Damm
> 
> Cc: Sergei Shtylyov ; Biju Das
> ; linux-renesas-soc@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Geert Uytterhoeven 
> Subject: [PATCH] ARM: shmobile: rcar-gen2: Correct arch timer frequency on
> RZ/G1E
>
> According to the datasheet, the frequency of the ARM architecture timer on
> RZ/G1E depends on the frequency of the ZS clock, just like on R-Car
> E2 and V2H.
>
> Signed-off-by: Geert Uytterhoeven 
> ---
> Untested due to lack of hardware.
>
> v2:
>   - Rebased to avoid dependencies.
> ---
>  arch/arm/mach-shmobile/setup-rcar-gen2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-
> shmobile/setup-rcar-gen2.c
> index 3bd505da31726869..7ab1690fab8299eb 100644
> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> @@ -70,7 +70,8 @@ void __init rcar_gen2_timer_init(void)
>  void __iomem *base;
>  u32 freq;
>
> -if (of_machine_is_compatible("renesas,r8a7792") ||
> +if (of_machine_is_compatible("renesas,r8a7745") ||
> +of_machine_is_compatible("renesas,r8a7792") ||
>  of_machine_is_compatible("renesas,r8a7794")) {
>  freq = 26000 / 8;/* ZS / 8 */
>  /* CNTVOFF has to be initialized either from non-secure
> --
> 2.7.4




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

[PATCH v2] ARM: shmobile: rcar-gen2: Correct arch timer frequency on RZ/G1E

[Geert Uytterhoeven]

According to the datasheet, the frequency of the ARM architecture timer
on RZ/G1E depends on the frequency of the ZS clock, just like on R-Car
E2 and V2H.

Signed-off-by: Geert Uytterhoeven 
---
Untested due to lack of hardware.

v2:
  - Rebased to avoid dependencies.
---
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c 
b/arch/arm/mach-shmobile/setup-rcar-gen2.c
index 3bd505da31726869..7ab1690fab8299eb 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -70,7 +70,8 @@ void __init rcar_gen2_timer_init(void)
void __iomem *base;
u32 freq;
 
-   if (of_machine_is_compatible("renesas,r8a7792") ||
+   if (of_machine_is_compatible("renesas,r8a7745") ||
+   of_machine_is_compatible("renesas,r8a7792") ||
of_machine_is_compatible("renesas,r8a7794")) {
freq = 26000 / 8;   /* ZS / 8 */
/* CNTVOFF has to be initialized either from non-secure
-- 
2.7.4

[PATCH] ARM: shmobile: rcar-gen2: Correct arch timer frequency on RZ/G1E

[Geert Uytterhoeven]

According to the datasheet, the frequency of the ARM architecture timer
on RZ/G1E depends on the frequency of the ZS clock, just like on R-Car
E2 and V2H.

Signed-off-by: Geert Uytterhoeven 
---
Untested due to lack of hardware.

v2:
  - Rebased to avoid dependencies.
---
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c 
b/arch/arm/mach-shmobile/setup-rcar-gen2.c
index 3bd505da31726869..7ab1690fab8299eb 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -70,7 +70,8 @@ void __init rcar_gen2_timer_init(void)
void __iomem *base;
u32 freq;
 
-   if (of_machine_is_compatible("renesas,r8a7792") ||
+   if (of_machine_is_compatible("renesas,r8a7745") ||
+   of_machine_is_compatible("renesas,r8a7792") ||
of_machine_is_compatible("renesas,r8a7794")) {
freq = 26000 / 8;   /* ZS / 8 */
/* CNTVOFF has to be initialized either from non-secure
-- 
2.7.4

Re: [PATCH 3/3] usb: gadget: udc: renesas_usb3: protect usb3_ep->started in usb3_start_pipen()

[Geert Uytterhoeven]

On Tue, Jul 18, 2017 at 2:26 PM, Yoshihiro Shimoda
 wrote:
> This patch fixes an issue that unexpected behavior happens when
> both the interrupt handler and renesas_usb3_ep_enable() are called.
> In this case, since usb3_start_pipen() checked the usb3_ep->started,
> but the flags was not protected. So, this patch protects the flag
> by usb3->lock. Since renesas_usb3_ep_enable() for EP0 will be not called,
> this patch doesn't take care of usb3_start_pipe0().
>
> Signed-off-by: Yoshihiro Shimoda 

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 1/3] usb: gadget: udc: renesas_usb3: fix free size in renesas_usb3_dma_free_prd()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Jul 18, 2017 at 2:26 PM, Yoshihiro Shimoda
 wrote:
> The commit 2d4aa21a73ba ("usb: gadget: udc: renesas_usb3: add support
> for dedicated DMAC") has a bug in the renesas_usb3_dma_free_prd().
> The size of dma_free_coherent() should be the same with dma_alloc_coherent()
> Otherwise, this code causes a WARNING by mm/page_alloc.c when
> renesas_usb3_dma_free_prd() is called. So, this patch fixes it.
>
> Fixes: 2d4aa21a73ba ("usb: gadget: udc: renesas_usb3: add support for 
> dedicated DMAC")
> Signed-off-by: Yoshihiro Shimoda 

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 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Daniel Vetter]

On Tue, Jul 18, 2017 at 2:47 PM, Laurent Pinchart
 wrote:
> On Tuesday 18 Jul 2017 14:08:39 Daniel Vetter wrote:
>> On Tue, Jul 18, 2017 at 01:14:12PM +0300, Laurent Pinchart wrote:
>> > On Tuesday 18 Jul 2017 09:05:22 Maxime Ripard wrote:
>> >> On Fri, Jul 14, 2017 at 02:43:12AM +0300, Laurent Pinchart wrote:
>> >>> On Thursday 13 Jul 2017 16:41:13 Maxime Ripard wrote:
>>  The current drm_atomic_helper_commit_tail helper works only if the
>>  CRTC is accessible, and documents an alternative implementation that
>>  is supposed to be used if that happens.
>> 
>>  That implementation is then duplicated by some drivers. Instead of
>>  documenting it, let's implement an helper that all the relevant users
>>  can use directly.
>> 
>>  Signed-off-by: Maxime Ripard 
>>  ---
>> 
>>   drivers/gpu/drm/drm_atomic_helper.c| 47 +
>>   drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
>>   drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
>> >>>
>> >>> I've submitted "[PATCH] drm: rcar-du: Setup planes before enabling
>> >>> CRTC to avoid flicker" that changes the rcar-du implementation to the
>> >>> standard disable/update planes/enable order, so I'd appreciate if you
>> >>> could drop the rcar-du part of this patch to avoid conflicts.
>> >>
>> >> I will.
>> >>
>> >>> This being said, the reason why I switched back from the "runtime PM"
>> >>> to the "standard" order is probably of interest to you. Quoting the
>> >>> commit message,
>> >>>
>>  Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
>>  start to CRTC resume") changed the order of the plane commit and CRTC
>>  enable operations to accommodate the runtime PM requirements.
>>  However, this introduced corruption in the first displayed frame, as
>>  the CRTC is now enabled without any plane configured. On Gen2
>>  hardware the first frame will be black and likely unnoticed, but on
>>  Gen3 hardware we end up starting the display before the VSP
>>  compositor, which is more noticeable.
>> 
>>  To fix this, revert the order of the commit operations back, and
>>  handle runtime PM requirements in the CRTC .atomic_begin() and
>>  .atomic_enable() helper operation handlers.
>> >>>
>> >>> I believe that the "runtime PM" order is problematic in most drivers.
>> >>> The problem usually goes unnoticed as most monitors will not even
>> >>> display the first frame, and I assume many devices will just output it
>> >>> black, but it's an issue nonetheless.
>> >>>
>> >>> Note that my driver hasn't lost the "runtime PM" requirements, so I
>> >>> had to support them with the "standard" order. The best way I've found
>> >>> was to runtime resume in the one of .atomic_begin() and .enable() that
>> >>> is run first. Not very neat, as similar code would be needed in most
>> >>> drivers. I wonder whether it wouldn't be useful to add resume/suspend
>> >>> helper callbacks for the CRTC.
>> >>
>> >> I'm not sure it would apply. Our driver doesn't use runtime_pm at all,
>> >> but in order for the commits to happen, we need to have the CRTC
>> >> active, but it will remain powered up the whole time. I'm not sure if
>> >> we'll ever see such a frame.
>> >>
>> >> But since this seems to be a pretty generic, maybe we should address
>> >> it in the helper itself?
>> >
>> > I think that would make sense.
>> >
>> > There are a few options that result in too many combinations for separate
>> > commit tail helpers to be provided in my opinion:
>> >
>> > - disable/enable/planes vs. disable/planes/enable
>> > - DRM_PLANE_COMMIT_ACTIVE_ONLY vs. all CRTCs
>> > - drm_atomic_helper_wait_for_vblanks vs
>> > drm_atomic_helper_wait_for_flip_done
>> >
>> > Maybe we could add a few CRTC commit helper flags along the line of
>> > DRM_PLANE_COMMIT_ACTIVE_ONLY, add a field to the drm_crtc structure to
>> > store them, and have drm_atomic_helper_commit_tail() use those flags to
>> > control the sequence of operations.
>>
>> Why not write your own? Yes it's a bit of copypaste, but imo that's really
>> not horrible.
>
> I don't find it horrible either, it's not too much code. The question was more
> about which version(s) to consider standard enough to provide a core helper
> for.
>
> What bothers me a bit more with the copy implementations isn't so much
> the commit tail handling itself, but the consequences it has on the rest of
> the driver. Drivers pick the order they want based on their requirements, and
> that order then leads to different race conditions between the drivers. We
> don't document the potential issues there, so new drivers (and for that
> matter, possibly existing ones) will likely have bugs if the author is not
> aware of the subtle issues related to the particular operation order they
> happen to use.

Imo the answer to that is implementing CRC support 

Re: [PATCH v2 00/14] Renesas R-Car VSP: Add H3 ES2.0 support

[Hans Verkuil]

On 26/06/17 20:12, Laurent Pinchart wrote:
> Hello,
> 
> This patch series implements support for the R-Car H3 ES2.0 SoC in the VSP
> and DU drivers.
>  
> Compared to the H3 ES1.1, the H3 ES2.0 has a new VSP2-DL instance that
> includes two blending units, a BRU and a BRS. The BRS is similar to the BRU
> but has two inputs only, and is used to service a second DU channel from the
> same VSP through a second LIF instances connected to WPF.1.
> 
> The patch series starts with a small fixes and cleanups in patches 01/14 to
> 05/14. Patch 06/14 prepares the VSP driver for multiple DU channels support by
> extending the DU-VSP API with an additional argument. Patches 07/14 to 10/14
> gradually build H3 ES2.0 support on top of that by implementing all needed
> features in the VSP driver.
> 
> So far the VSP driver always used headerless display lists when operating in
> connection with the DU. This mode of operation is only available on WPF.0, so
> support for regular display lists with headers when operating with the DU is
> added in patch 11/14.
> 
> The remaining patches finally implement H3 ES2.0 support in the DU driver,
> with support for VSP sharing implemented in patch 12/14, for H3 ES2.0 PLL in
> patch 13/14 (by restricting the ES1.x workaround to ES1.x SoCs) and for RGB
> output routing in patch 14/14.
> 
> Compared to v1, the series has gone under considerable changes. Testing
> locally on H3 ES2.0 uncovered multiple issues in the previous partially tested
> version, which have been fixed in additional patches. The following changes
> can be noted in particular.
> 
> - New small cleanups in patches 02/14 to 05/14
> - Pass the pipe index to vsp1_du_atomic_update() explicitly
> - Rebase on top of the VSP-DU flicker fixes, resulting in a major rework of
>   "v4l: vsp1: Add support for header display lists in continuous mode"
> - New patches 09/14, 10/14 and 12/14 to support the previously untested VGA
>   output
> 
> The series is based on top of Dave's latest drm-next branch as it depends on
> patches merged by Dave for v4.13. It depends, for testing, on
> 
> - the sh-pfc-for-v4.13 branch from Geert's renesas-drivers tree
> - the "[PATCH v2 0/2] R-Car H3 ES2.0 Salvator-X: Enable DU support in DT"
>   patch series
> 
> For convenience, a branch merging this series with all dependencies is
> available from
> 
>   git://linuxtv.org/pinchartl/media.git drm/next/h3-es2/merged
> 
> with the DT and driver series split in two branches respectively tagged
> drm-h3-es2-dt-20170626 and drm-h3-es2-vsp-du-20170626.
> 
> The patches have been tested on the Lager, Salvator-X H3 ES1.x, Salvator-X
> M3-W and Salvator-XS boards. All outputs have been tested using modetest
> without any noticeable regression.
> 
> Laurent Pinchart (14):
>   v4l: vsp1: Fill display list headers without holding dlm spinlock
>   v4l: vsp1: Don't recycle active list at display start
>   v4l: vsp1: Don't set WPF sink pointer
>   v4l: vsp1: Store source and sink pointers as vsp1_entity
>   v4l: vsp1: Don't create links for DRM pipeline
>   v4l: vsp1: Add pipe index argument to the VSP-DU API
>   v4l: vsp1: Add support for the BRS entity
>   v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances
>   v4l: vsp1: Add support for multiple LIF instances
>   v4l: vsp1: Add support for multiple DRM pipelines
>   v4l: vsp1: Add support for header display lists in continuous mode
>   drm: rcar-du: Support multiple sources from the same VSP
>   drm: rcar-du: Restrict DPLL duty cycle workaround to H3 ES1.x
>   drm: rcar-du: Configure DPAD0 routing through last group on Gen3

For this patch series:

Acked-by: Hans Verkuil 

Regards,

Hans

> 
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c|  39 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h|   3 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c   |  21 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  91 --
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c |  37 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h |  10 +-
>  drivers/media/platform/vsp1/vsp1.h|   7 +-
>  drivers/media/platform/vsp1/vsp1_bru.c|  45 +++--
>  drivers/media/platform/vsp1/vsp1_bru.h|   4 +-
>  drivers/media/platform/vsp1/vsp1_dl.c | 205 +-
>  drivers/media/platform/vsp1/vsp1_dl.h |   1 -
>  drivers/media/platform/vsp1/vsp1_drm.c| 283 
> +++---
>  drivers/media/platform/vsp1/vsp1_drm.h|  38 ++--
>  drivers/media/platform/vsp1/vsp1_drv.c| 115 
>  drivers/media/platform/vsp1/vsp1_entity.c |  40 +++--
>  drivers/media/platform/vsp1/vsp1_entity.h |   5 +-
>  drivers/media/platform/vsp1/vsp1_lif.c|   5 +-
>  drivers/media/platform/vsp1/vsp1_lif.h|   2 +-
>  drivers/media/platform/vsp1/vsp1_pipe.c   |   7 +-
>  drivers/media/platform/vsp1/vsp1_regs.h   |  46 +++--
>  drivers/media/platform/vsp1/vsp1_video.c  |  63 ---
>  drivers/media/platform/vsp1/vsp1_wpf.c|   4 +-
>  

Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Laurent Pinchart]

Hi Daniel,

On Tuesday 18 Jul 2017 14:08:39 Daniel Vetter wrote:
> On Tue, Jul 18, 2017 at 01:14:12PM +0300, Laurent Pinchart wrote:
> > On Tuesday 18 Jul 2017 09:05:22 Maxime Ripard wrote:
> >> On Fri, Jul 14, 2017 at 02:43:12AM +0300, Laurent Pinchart wrote:
> >>> On Thursday 13 Jul 2017 16:41:13 Maxime Ripard wrote:
>  The current drm_atomic_helper_commit_tail helper works only if the
>  CRTC is accessible, and documents an alternative implementation that
>  is supposed to be used if that happens.
>  
>  That implementation is then duplicated by some drivers. Instead of
>  documenting it, let's implement an helper that all the relevant users
>  can use directly.
>  
>  Signed-off-by: Maxime Ripard 
>  ---
>  
>   drivers/gpu/drm/drm_atomic_helper.c| 47 +
>   drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
>   drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
> >>> 
> >>> I've submitted "[PATCH] drm: rcar-du: Setup planes before enabling
> >>> CRTC to avoid flicker" that changes the rcar-du implementation to the
> >>> standard disable/update planes/enable order, so I'd appreciate if you
> >>> could drop the rcar-du part of this patch to avoid conflicts.
> >> 
> >> I will.
> >> 
> >>> This being said, the reason why I switched back from the "runtime PM"
> >>> to the "standard" order is probably of interest to you. Quoting the
> >>> commit message,
> >>> 
>  Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
>  start to CRTC resume") changed the order of the plane commit and CRTC
>  enable operations to accommodate the runtime PM requirements.
>  However, this introduced corruption in the first displayed frame, as
>  the CRTC is now enabled without any plane configured. On Gen2
>  hardware the first frame will be black and likely unnoticed, but on
>  Gen3 hardware we end up starting the display before the VSP
>  compositor, which is more noticeable.
>  
>  To fix this, revert the order of the commit operations back, and
>  handle runtime PM requirements in the CRTC .atomic_begin() and
>  .atomic_enable() helper operation handlers.
> >>> 
> >>> I believe that the "runtime PM" order is problematic in most drivers.
> >>> The problem usually goes unnoticed as most monitors will not even
> >>> display the first frame, and I assume many devices will just output it
> >>> black, but it's an issue nonetheless.
> >>> 
> >>> Note that my driver hasn't lost the "runtime PM" requirements, so I
> >>> had to support them with the "standard" order. The best way I've found
> >>> was to runtime resume in the one of .atomic_begin() and .enable() that
> >>> is run first. Not very neat, as similar code would be needed in most
> >>> drivers. I wonder whether it wouldn't be useful to add resume/suspend
> >>> helper callbacks for the CRTC.
> >> 
> >> I'm not sure it would apply. Our driver doesn't use runtime_pm at all,
> >> but in order for the commits to happen, we need to have the CRTC
> >> active, but it will remain powered up the whole time. I'm not sure if
> >> we'll ever see such a frame.
> >> 
> >> But since this seems to be a pretty generic, maybe we should address
> >> it in the helper itself?
> > 
> > I think that would make sense.
> > 
> > There are a few options that result in too many combinations for separate
> > commit tail helpers to be provided in my opinion:
> > 
> > - disable/enable/planes vs. disable/planes/enable
> > - DRM_PLANE_COMMIT_ACTIVE_ONLY vs. all CRTCs
> > - drm_atomic_helper_wait_for_vblanks vs
> > drm_atomic_helper_wait_for_flip_done
> > 
> > Maybe we could add a few CRTC commit helper flags along the line of
> > DRM_PLANE_COMMIT_ACTIVE_ONLY, add a field to the drm_crtc structure to
> > store them, and have drm_atomic_helper_commit_tail() use those flags to
> > control the sequence of operations.
> 
> Why not write your own? Yes it's a bit of copypaste, but imo that's really
> not horrible.

I don't find it horrible either, it's not too much code. The question was more 
about which version(s) to consider standard enough to provide a core helper 
for.

What bothers me a bit more with the copy implementations isn't so much 
the commit tail handling itself, but the consequences it has on the rest of 
the driver. Drivers pick the order they want based on their requirements, and 
that order then leads to different race conditions between the drivers. We 
don't document the potential issues there, so new drivers (and for that 
matter, possibly existing ones) will likely have bugs if the author is not 
aware of the subtle issues related to the particular operation order they 
happen to use.

> I'm already not happy with the flags for commit_planes because the docs for
> them are not great and it's hard to know when to use them and when not to.
> 
> ->commit_tail was specifically done to allow 

[PATCH 1/3] usb: gadget: udc: renesas_usb3: fix free size in renesas_usb3_dma_free_prd()

[Yoshihiro Shimoda]

The commit 2d4aa21a73ba ("usb: gadget: udc: renesas_usb3: add support
for dedicated DMAC") has a bug in the renesas_usb3_dma_free_prd().
The size of dma_free_coherent() should be the same with dma_alloc_coherent()
Otherwise, this code causes a WARNING by mm/page_alloc.c when
renesas_usb3_dma_free_prd() is called. So, this patch fixes it.

Fixes: 2d4aa21a73ba ("usb: gadget: udc: renesas_usb3: add support for dedicated 
DMAC")
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/renesas_usb3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index d827832..923ad5a 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -1369,7 +1369,7 @@ static int renesas_usb3_dma_free_prd(struct renesas_usb3 
*usb3,
 
usb3_for_each_dma(usb3, dma, i) {
if (dma->prd) {
-   dma_free_coherent(dev, USB3_DMA_MAX_XFER_SIZE,
+   dma_free_coherent(dev, USB3_DMA_PRD_SIZE,
  dma->prd, dma->prd_dma);
dma->prd = NULL;
}
-- 
1.9.1

[PATCH 3/3] usb: gadget: udc: renesas_usb3: protect usb3_ep->started in usb3_start_pipen()

[Yoshihiro Shimoda]

This patch fixes an issue that unexpected behavior happens when
both the interrupt handler and renesas_usb3_ep_enable() are called.
In this case, since usb3_start_pipen() checked the usb3_ep->started,
but the flags was not protected. So, this patch protects the flag
by usb3->lock. Since renesas_usb3_ep_enable() for EP0 will be not called,
this patch doesn't take care of usb3_start_pipe0().

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/renesas_usb3.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 1cc5f0d..62dc9c7 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -1415,12 +1415,12 @@ static void usb3_start_pipen(struct renesas_usb3_ep 
*usb3_ep,
int ret = -EAGAIN;
u32 enable_bits = 0;
 
+   spin_lock_irqsave(>lock, flags);
if (usb3_ep->halt || usb3_ep->started)
-   return;
+   goto out;
if (usb3_req != usb3_req_first)
-   return;
+   goto out;
 
-   spin_lock_irqsave(>lock, flags);
if (usb3_pn_change(usb3, usb3_ep->num) < 0)
goto out;
 
-- 
1.9.1

[PATCH 2/3] usb: gadget: udc: renesas_usb3: fix zlp transfer by the dmac

[Yoshihiro Shimoda]

The dedicated dmac can transfer a zero-length-packet (zlp) if some bits
of the USB_COM_CON register. However, the commit 2d4aa21a73ba ("usb:
gadget: udc: renesas_usb3: add support for dedicated DMAC") didn't set
the bits to 1. So, this patch fixes it.

Fixes: 2d4aa21a73b ("usb: gadget: udc: renesas_usb3: add support for dedicated 
DMAC)
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/renesas_usb3.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 923ad5a..1cc5f0d 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -89,6 +89,9 @@
 
 /* USB_COM_CON */
 #define USB_COM_CON_CONF   BIT(24)
+#define USB_COM_CON_PN_WDATAIF_NL  BIT(23)
+#define USB_COM_CON_PN_RDATAIF_NL  BIT(22)
+#define USB_COM_CON_PN_LSTTR_PPBIT(21)
 #define USB_COM_CON_SPD_MODE   BIT(17)
 #define USB_COM_CON_EP0_EN BIT(16)
 #define USB_COM_CON_DEV_ADDR_SHIFT 8
@@ -686,6 +689,9 @@ static void renesas_usb3_init_controller(struct 
renesas_usb3 *usb3)
 {
usb3_init_axi_bridge(usb3);
usb3_init_epc_registers(usb3);
+   usb3_set_bit(usb3, USB_COM_CON_PN_WDATAIF_NL |
+USB_COM_CON_PN_RDATAIF_NL | USB_COM_CON_PN_LSTTR_PP,
+USB3_USB_COM_CON);
usb3_write(usb3, USB_OTG_IDMON, USB3_USB_OTG_INT_STA);
usb3_write(usb3, USB_OTG_IDMON, USB3_USB_OTG_INT_ENA);
 
-- 
1.9.1

[PATCH 0/3] usb: gadget: udc: renesas_usb3: fixes for v4.13-rc1

[Yoshihiro Shimoda]

This patch is based on the latest Felipe's usb.git / testing/fixes branch
(The commit id = 4a71fcb8ac5f94c07bf47a43b13258a52e4fe3ad).

Yoshihiro Shimoda (3):
  usb: gadget: udc: renesas_usb3: fix free size in
renesas_usb3_dma_free_prd()
  usb: gadget: udc: renesas_usb3: fix zlp transfer by the dmac
  usb: gadget: udc: renesas_usb3: protect usb3_ep->started in
usb3_start_pipen()

 drivers/usb/gadget/udc/renesas_usb3.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
1.9.1

Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Daniel Vetter]

On Tue, Jul 18, 2017 at 01:14:12PM +0300, Laurent Pinchart wrote:
> Hi Maxime,
> 
> On Tuesday 18 Jul 2017 09:05:22 Maxime Ripard wrote:
> > On Fri, Jul 14, 2017 at 02:43:12AM +0300, Laurent Pinchart wrote:
> > > On Thursday 13 Jul 2017 16:41:13 Maxime Ripard wrote:
> > >> The current drm_atomic_helper_commit_tail helper works only if the CRTC
> > >> is accessible, and documents an alternative implementation that is
> > >> supposed to be used if that happens.
> > >> 
> > >> That implementation is then duplicated by some drivers. Instead of
> > >> documenting it, let's implement an helper that all the relevant users
> > >> can use directly.
> > >> 
> > >> Signed-off-by: Maxime Ripard 
> > >> ---
> > >> 
> > >>  drivers/gpu/drm/drm_atomic_helper.c| 47 +++
> > >>  drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
> > >>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
> > > 
> > > I've submitted "[PATCH] drm: rcar-du: Setup planes before enabling CRTC to
> > > avoid flicker" that changes the rcar-du implementation to the standard
> > > disable/update planes/enable order, so I'd appreciate if you could drop
> > > the rcar-du part of this patch to avoid conflicts.
> > 
> > I will.
> > 
> > > This being said, the reason why I switched back from the "runtime PM" to
> > > the "standard" order is probably of interest to you. Quoting the commit
> > > message,
> > >
> > >> Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
> > >> start to CRTC resume") changed the order of the plane commit and CRTC
> > >> enable operations to accommodate the runtime PM requirements. However,
> > >> this introduced corruption in the first displayed frame, as the CRTC is
> > >> now enabled without any plane configured. On Gen2 hardware the first
> > >> frame will be black and likely unnoticed, but on Gen3 hardware we end up
> > >> starting the display before the VSP compositor, which is more
> > >> noticeable.
> > >> 
> > >> To fix this, revert the order of the commit operations back, and handle
> > >> runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
> > >> helper operation handlers.
> > > 
> > > I believe that the "runtime PM" order is problematic in most drivers. The
> > > problem usually goes unnoticed as most monitors will not even display the
> > > first frame, and I assume many devices will just output it black, but it's
> > > an issue nonetheless.
> > > 
> > > Note that my driver hasn't lost the "runtime PM" requirements, so I had to
> > > support them with the "standard" order. The best way I've found was to
> > > runtime resume in the one of .atomic_begin() and .enable() that is run
> > > first. Not very neat, as similar code would be needed in most drivers. I
> > > wonder whether it wouldn't be useful to add resume/suspend helper
> > > callbacks for the CRTC.
> > 
> > I'm not sure it would apply. Our driver doesn't use runtime_pm at all,
> > but in order for the commits to happen, we need to have the CRTC
> > active, but it will remain powered up the whole time. I'm not sure if
> > we'll ever see such a frame.
> > 
> > But since this seems to be a pretty generic, maybe we should address
> > it in the helper itself?
> 
> I think that would make sense.
> 
> There are a few options that result in too many combinations for separate 
> commit tail helpers to be provided in my opinion:
> 
> - disable/enable/planes vs. disable/planes/enable
> - DRM_PLANE_COMMIT_ACTIVE_ONLY vs. all CRTCs
> - drm_atomic_helper_wait_for_vblanks vs drm_atomic_helper_wait_for_flip_done
> 
> Maybe we could add a few CRTC commit helper flags along the line of 
> DRM_PLANE_COMMIT_ACTIVE_ONLY, add a field to the drm_crtc structure to store 
> them, and have drm_atomic_helper_commit_tail() use those flags to control the 
> sequence of operations.

Why not write your own? Yes it's a bit of copypaste, but imo that's really
not horrible. I'm already not happy with the flags for commit_planes
because the docs for them are not great and it's hard to know when to use
them and when not to.

->commit_tail was specifically done to allow drivers to overwrite the hw
commit stage without having to reinvent all the other commit tracking. I
expect most non-simple drivers to have their own commit_tail function.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Hans Verkuil]

On 08/07/17 15:31, Konstantin Kozhevnikov wrote:
> Hello all,
> 
> the sample is made publicly available, and can be taken from 
> https://github.com/CogentEmbedded/imr-sv-utest/blob/master/utest/utest-imr.c.
> 
> It doesn't show how luminance/chrominance correction actually works, however. 
> That feature has been tested once a while ago, and probably we are going to 
> release that soon.
> 
> Regarding usage of "chromacity" word in the comments, I actually meant 
> "chrominance" or "chroma". The difference for non-native speaker is probably 
> a bit vague, just like one between "luminance" and
> "luminosity". In the R-Car manual it is referred to as "hue", but what is 
> meant is the "luma" and "chroma". These short forms seem a bit weird to me, 
> hence I was using the words "luminance" and
> "chromacity". If that's confusing, I don't mind them be replaced with just 
> "luma"/"chroma".

luma/chroma works well for me. 'chromacity' was confusing for me because it is 
close to
'chromaticity' which is a valid word but it has a different meaning.

> 
> For documentation part, I am not 100% sure Renesas is okay with disclosing 
> each and every detail, it might be the case we should obtain a permit from 
> their legals. Still, I believe the person who is
> about to use the driver is having an access to at least Technical Reference 
> Manual of R-Car Gen2/3, so adding a reference to a chapter in TRM will most 
> likely be sufficient.

A reference to the TRM is fine. I assume people who write code for this will 
have the TRM
available.

Regarding the code: having that example code is certainly useful, but what I am 
really looking
for is a code snippet that allocates and fills in the data to pass on with the 
ioctl.

Especially the handling of the 'type' field remains very fuzzy to me. 
Especially the fact
that it is a bitmask is strange. Usually a type is an enum that defines how to 
interpret
the struct (e.g. similar to type in struct v4l2_format). I get the feeling your 
mixing
type with flags. Perhaps if you split it up into a type and flags field things 
will make
more sense.

> 
> The question about usage of "user-pointer" buffers (V4L2_MEMORY_USERPTR) is a 
> bit confusing. Indeed, right now we are using that scheme, though I wouldn't 
> say we are absolutely required to do that.
> Specifically, we are allocating the contiguous buffers using Renesas' 
> proprietary "mmngr" driver (it's not a rocket science thing, but it's made 
> proprietary for some reason). Then we are using the
> buffers for various persons, both in EGL and in IMR. I guess we are okay with 
> moving completely to DMA-fd (given the fact we have an accompanying driver 
> "mmngrbuf" which serves for translation of
> memory pointers to DMA-fds for further cross-process sharing and stuff). I 
> mean, if USERPTR is tagged as an obsolete / deprecated function, we are fine 
> with dropping it. However, there is one thing
> I'd like to understand from V4L2 experts. I do see sometimes (during 
> application closing or shortly after it) the bunches of warnings from kernel 
> regarding "corrupted" MMU state (don't recall exactly,
> but it sounds like page which is supposed to be free gets somehow corrupted). 
> Is that something that is related to (mis)use of USERPTR? I was trying to 
> find out if there is any memory corruption
> caused by application logic, came to conclusion it's not. To me it looks like 
> a race condition between unmapping of VMAs and V4L2 buffers deallocation 
> which yields sometimes unpredictable results. Can
> you please provide some details about possible issues with usage of USERPTR 
> with DMA-contiguous buffer driver, it would be good to find a match.

I am not aware of any warnings like you described.

Originally USERPTR was designed to be used with scatter-gather DMA engines. It 
requires
a really good DMA engine that is able to handle partial pages and address 
alignments
of 8 bytes or less. That way it allowed userspace to use malloc to allocate the 
buffers.

Later it was abused for embedded systems that used contiguous DMA but had 
special code
that carved out memory. Userspace would pass a pointer to the driver using the 
USERPTR
API and the driver would verify that it indeed pointed to physically contiguous 
memory.

When dmabuf came along (together with the CMA subsystem) the need for abusing 
USERPTR
in that way disappeared and our recommendation for new drivers is to always 
support
DMABUF and only use USERPTR if the hardware can *really* handle malloc()ed 
buffers.

I.e. without special alignment requirements such as the buffer must start at a 
page
boundary. Most hardware fails that test.

In practice we discourage the use of USERPTR for embedded systems.

Apologies for the delay in replying, I was on vacation last week.

Regards,

Hans

renesas-drivers-2017-07-18-v4.13-rc1

[Geert Uytterhoeven]

I have pushed renesas-drivers-2017-07-18-v4.13-rc1 to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

This tree is meant to ease development of platform support and drivers
for Renesas ARM SoCs. It is created by merging (a) the for-next branches
of various subsystem trees and (b) branches with driver code submitted
or planned for submission to maintainers into the development branch of
Simon Horman's renesas.git tree.

Today's version is based on renesas-devel-20170714-v4.12.

Note that due to holidays, there will be no release on August the first.
Next release will probably be renesas-drivers-2017-08-16-v4.13-rc5.

Included branches with driver code:
  - clk-renesas-for-v4.14
  - sh-pfc-for-v4.14
  - topic/rza1-pfc-gpio-v6
  - topic/suspend-to-idle-v1
  - topic/sh-sh7722-sh7757-sh7264-sh7269-fix-pinctrl-v2
  - topic/rcar2-cpg-mssr-dt-v2
  - git://git.ragnatech.se/linux#for-renesas-drivers
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#renesas/topic/i2c-core-dma
  - git://linuxtv.org/pinchartl/media.git#tags/drm-h3-es2-vsp-du-20170626
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#adv748x/driver
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#vsp1/pa-improvements/v2
  - topic/bd9571-mfd-v4

Included fixes:
  - ARM: shmobile: rcar-gen2: Fix deadlock in regulator quirk
  - Revert "ACPI / boot: Correct address space of __acpi_map_table()"
  - ACPI / boot: Correct address space of __acpi_map_table()
  - drm/vblank: fix for "switch compat_drm_wait_vblank() to drm_ioctl_kernel()"
  - of_mdio: Fix broken PHY IRQ in case of probe deferral

Included subsystem trees:
  - git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git#linux-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git#clk-next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git#for-next
  - git://git.infradead.org/users/dedekind/l2-mtd-2.6.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git#tty-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#i2c/for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git#usb-next
  - git://people.freedesktop.org/~airlied/linux#drm-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git#next
  - git://linuxtv.org/mchehab/media-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git#for-next
  - git://git.linaro.org/people/daniel.lezcano/linux.git#clockevents/next
  - git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git#testing/next
  - git://git.infradead.org/users/vkoul/slave-dma.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git#staging-next
  - git://git.armlinux.org.uk/~rmk/linux-arm.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git#for-next
  - git://git.infradead.org/users/jcooper/linux.git#irqchip/for-next
  - git://github.com/bzolnier/linux.git#fbdev-for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git#for-next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git#for-next
  - git://www.linux-watchdog.org/linux-watchdog-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git#for-next/core
  - git://anongit.freedesktop.org/drm/drm-misc#for-linux-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git#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

[PATCH v3 3/4] i2c: sh_mobile: use helper to decide if DMA is useful

[Wolfram Sang]

This ensures that we fall back to PIO if the buffer is too small for DMA
being useful. Otherwise, we use DMA. A bounce buffer might be applied if
the original message buffer is not DMA safe

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

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index 2e097d97d258bc..19f45bcd9b35ca 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -145,6 +145,7 @@ struct sh_mobile_i2c_data {
struct dma_chan *dma_rx;
struct scatterlist sg;
enum dma_data_direction dma_direction;
+   u8 *bounce_buf;
 };
 
 struct sh_mobile_dt_config {
@@ -548,6 +549,8 @@ static void sh_mobile_i2c_dma_callback(void *data)
pd->pos = pd->msg->len;
pd->stop_after_dma = true;
 
+   i2c_release_dma_bounce_buf(pd->msg, pd->bounce_buf);
+
iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
 }
 
@@ -595,6 +598,7 @@ static void sh_mobile_i2c_xfer_dma(struct 
sh_mobile_i2c_data *pd)
struct dma_async_tx_descriptor *txdesc;
dma_addr_t dma_addr;
dma_cookie_t cookie;
+   u8 *dma_buf = pd->bounce_buf ?: pd->msg->buf;
 
if (PTR_ERR(chan) == -EPROBE_DEFER) {
if (read)
@@ -608,7 +612,7 @@ static void sh_mobile_i2c_xfer_dma(struct 
sh_mobile_i2c_data *pd)
if (IS_ERR(chan))
return;
 
-   dma_addr = dma_map_single(chan->device->dev, pd->msg->buf, 
pd->msg->len, dir);
+   dma_addr = dma_map_single(chan->device->dev, dma_buf, pd->msg->len, 
dir);
if (dma_mapping_error(chan->device->dev, dma_addr)) {
dev_dbg(pd->dev, "dma map failed, using PIO\n");
return;
@@ -665,7 +669,7 @@ static int start_ch(struct sh_mobile_i2c_data *pd, struct 
i2c_msg *usr_msg,
pd->pos = -1;
pd->sr = 0;
 
-   if (pd->msg->len > 8)
+   if (i2c_check_msg_for_dma(pd->msg, 8, >bounce_buf) == 0)
sh_mobile_i2c_xfer_dma(pd);
 
/* Enable all interrupts to begin with */
-- 
2.11.0

[PATCH v3 2/4] i2c: add docs to clarify DMA handling

[Wolfram Sang]

Signed-off-by: Wolfram Sang 
---
Changes since v2:

* documentation updates. Hopefully better wording now

 Documentation/i2c/DMA-considerations | 38 
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/i2c/DMA-considerations

diff --git a/Documentation/i2c/DMA-considerations 
b/Documentation/i2c/DMA-considerations
new file mode 100644
index 00..e46c24d65c8556
--- /dev/null
+++ b/Documentation/i2c/DMA-considerations
@@ -0,0 +1,38 @@
+Linux I2C and DMA
+-
+
+Given that I2C is a low-speed bus where largely small messages are transferred,
+it is not considered a prime user of DMA access. At this time of writing, only
+10% of I2C bus master drivers have DMA support implemented. And the vast
+majority of transactions are so small that setting up DMA for it will likely
+add more overhead than a plain PIO transfer.
+
+Therefore, it is *not* mandatory that the buffer of an I2C message is DMA safe.
+It does not seem reasonable to apply additional burdens when the feature is so
+rarely used. However, it is recommended to use a DMA-safe buffer if your
+message size is likely applicable for DMA. Most drivers have this threshold
+around 8 bytes. As of today, this is mostly an educated guess, however.
+
+To support this scenario, drivers wishing to implement DMA can use helper
+functions from the I2C core. One checks if a message is DMA capable in terms of
+size and memory type. It can optionally also create a bounce buffer:
+
+   i2c_check_msg_for_dma(msg, threshold, _buf);
+
+The bounce buffer handling from the core is generic and simple. It will always
+allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
+reusing pre-allocated buffers), you can leave the pointer to the bounce buffer
+empty and implement your own handling based on the return value of the above
+function.
+
+The other helper function releases the bounce buffer. It ensures data is copied
+back to the message:
+
+   i2c_release_dma_bounce_buf(msg, bounce_buf);
+
+Please check the in-kernel documentation for details. The i2c-sh_mobile driver
+can be used as a reference example.
+
+If you plan to use DMA with I2C (or with any other bus, actually) make sure you
+have CONFIG_DMA_API_DEBUG enabled during development. It can help you find
+various issues which can be complex to debug otherwise.
-- 
2.11.0

[PATCH v3 1/4] i2c: add helpers to ease DMA handling

[Wolfram Sang]

One helper checks if DMA is suitable and optionally creates a bounce
buffer, if not. The other function returns the bounce buffer and makes
sure the data is properly copied back to the message.

Signed-off-by: Wolfram Sang 
---
Changes since v2:

* rebased to v4.13-rc1
* helper functions are not inlined anymore but moved to i2c core
* __must_check has been added to the buffer check helper
* the release function has been renamed to contain 'dma' as well

 drivers/i2c/i2c-core-base.c | 68 +
 include/linux/i2c.h |  5 
 2 files changed, 73 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index c89dac7fd2e7b7..7326a9d2e4eb69 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -44,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "i2c-core.h"
@@ -2240,6 +2242,72 @@ void i2c_put_adapter(struct i2c_adapter *adap)
 }
 EXPORT_SYMBOL(i2c_put_adapter);
 
+/**
+ * i2c_check_msg_for_dma() - check if a message is suitable for DMA
+ * @msg: the message to be checked
+ * @threshold: the amount of byte from which using DMA makes sense
+ * @ptr_for_bounce_buf: if not NULL, a bounce buffer will be attached to this
+ * ptr, if needed. The bounce buffer must be freed by the
+ * caller using i2c_release_dma_bounce_buf().
+ *
+ * Return: -ERANGE if message is smaller than threshold
+ *-EFAULT if message buffer is not DMA capable and no bounce buffer
+ *was requested
+ *-ENOMEM if a bounce buffer could not be created
+ *0 if message is suitable for DMA
+ *
+ * The return value must be checked.
+ *
+ * Note: This function should only be called from process context! It uses
+ * helper functions which work on the 'current' task.
+ */
+int i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
+   u8 **ptr_for_bounce_buf)
+{
+   if (ptr_for_bounce_buf)
+   *ptr_for_bounce_buf = NULL;
+
+   if (msg->len < threshold)
+   return -ERANGE;
+
+   if (!virt_addr_valid(msg->buf) || object_is_on_stack(msg->buf)) {
+   pr_debug("msg buffer to 0x%04x is not DMA safe%s\n", msg->addr,
+ptr_for_bounce_buf ? ", trying bounce buffer" : "");
+   if (ptr_for_bounce_buf) {
+   if (msg->flags & I2C_M_RD)
+   *ptr_for_bounce_buf = kzalloc(msg->len, 
GFP_KERNEL);
+   else
+   *ptr_for_bounce_buf = kmemdup(msg->buf, 
msg->len,
+ GFP_KERNEL);
+   if (!*ptr_for_bounce_buf)
+   return -ENOMEM;
+   } else {
+   return -EFAULT;
+   }
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_check_msg_for_dma);
+
+/**
+ * i2c_release_bounce_buf - copy data back from bounce buffer and release it
+ * @msg: the message to be copied back to
+ * @bounce_buf: the bounce buffer obtained from i2c_check_msg_for_dma().
+ * May be NULL.
+ */
+void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf)
+{
+   if (!bounce_buf)
+   return;
+
+   if (msg->flags & I2C_M_RD)
+   memcpy(msg->buf, bounce_buf, msg->len);
+
+   kfree(bounce_buf);
+}
+EXPORT_SYMBOL_GPL(i2c_release_bounce_buf);
+
 MODULE_AUTHOR("Simon G. Vogl ");
 MODULE_DESCRIPTION("I2C-Bus main module");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 00ca5b86a753f8..ac02287b6c0d8f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -766,6 +766,11 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
i2c_msg *msg)
return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
 }
 
+int __must_check i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int 
threshold,
+   u8 **ptr_for_bounce_buf);
+
+void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf);
+
 int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
addr);
 /**
  * module_i2c_driver() - Helper macro for registering a modular I2C driver
-- 
2.11.0

[PATCH v3 0/4] i2c: document DMA handling and add helpers for it

[Wolfram Sang]

So, after revisiting old mail threads and taking part in a similar discussion
on the USB list, here is what I cooked up to document and ease DMA handling for
I2C within Linux. Please have a look at the documentation introduced in patch 2
for further details.

All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W) and
a Renesas Lager board (r8a7790/H2). A more detailed test description can be
found here: http://elinux.org/Tests:I2C-core-DMA

The branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
renesas/topic/i2c-core-dma-v3

And big kudos to Renesas Electronics for funding this work, thank you very much!

Regards,

   Wolfram


Changes since v2:

* rebased to v4.13-rc1
* helper functions are not inlined anymore but moved to i2c core
* __must_check has been added to the buffer check helper
* the release function has been renamed to contain 'dma' as well
* documentation updates. Hopefully better wording now
* removed the doubled Signed-offs
* adding more potentially interested parties to CC


Wolfram Sang (4):
  i2c: add helpers to ease DMA handling
  i2c: add docs to clarify DMA handling
  i2c: sh_mobile: use helper to decide if DMA is useful
  i2c: rcar: check for DMA-capable buffers

 Documentation/i2c/DMA-considerations | 38 
 drivers/i2c/busses/i2c-rcar.c| 18 +++---
 drivers/i2c/busses/i2c-sh_mobile.c   |  8 +++--
 drivers/i2c/i2c-core-base.c  | 68 
 include/linux/i2c.h  |  5 +++
 5 files changed, 130 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/i2c/DMA-considerations

-- 
2.11.0

Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Laurent Pinchart]

Hi Maxime,

On Tuesday 18 Jul 2017 09:05:22 Maxime Ripard wrote:
> On Fri, Jul 14, 2017 at 02:43:12AM +0300, Laurent Pinchart wrote:
> > On Thursday 13 Jul 2017 16:41:13 Maxime Ripard wrote:
> >> The current drm_atomic_helper_commit_tail helper works only if the CRTC
> >> is accessible, and documents an alternative implementation that is
> >> supposed to be used if that happens.
> >> 
> >> That implementation is then duplicated by some drivers. Instead of
> >> documenting it, let's implement an helper that all the relevant users
> >> can use directly.
> >> 
> >> Signed-off-by: Maxime Ripard 
> >> ---
> >> 
> >>  drivers/gpu/drm/drm_atomic_helper.c| 47 +++
> >>  drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
> >>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
> > 
> > I've submitted "[PATCH] drm: rcar-du: Setup planes before enabling CRTC to
> > avoid flicker" that changes the rcar-du implementation to the standard
> > disable/update planes/enable order, so I'd appreciate if you could drop
> > the rcar-du part of this patch to avoid conflicts.
> 
> I will.
> 
> > This being said, the reason why I switched back from the "runtime PM" to
> > the "standard" order is probably of interest to you. Quoting the commit
> > message,
> >
> >> Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
> >> start to CRTC resume") changed the order of the plane commit and CRTC
> >> enable operations to accommodate the runtime PM requirements. However,
> >> this introduced corruption in the first displayed frame, as the CRTC is
> >> now enabled without any plane configured. On Gen2 hardware the first
> >> frame will be black and likely unnoticed, but on Gen3 hardware we end up
> >> starting the display before the VSP compositor, which is more
> >> noticeable.
> >> 
> >> To fix this, revert the order of the commit operations back, and handle
> >> runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
> >> helper operation handlers.
> > 
> > I believe that the "runtime PM" order is problematic in most drivers. The
> > problem usually goes unnoticed as most monitors will not even display the
> > first frame, and I assume many devices will just output it black, but it's
> > an issue nonetheless.
> > 
> > Note that my driver hasn't lost the "runtime PM" requirements, so I had to
> > support them with the "standard" order. The best way I've found was to
> > runtime resume in the one of .atomic_begin() and .enable() that is run
> > first. Not very neat, as similar code would be needed in most drivers. I
> > wonder whether it wouldn't be useful to add resume/suspend helper
> > callbacks for the CRTC.
> 
> I'm not sure it would apply. Our driver doesn't use runtime_pm at all,
> but in order for the commits to happen, we need to have the CRTC
> active, but it will remain powered up the whole time. I'm not sure if
> we'll ever see such a frame.
> 
> But since this seems to be a pretty generic, maybe we should address
> it in the helper itself?

I think that would make sense.

There are a few options that result in too many combinations for separate 
commit tail helpers to be provided in my opinion:

- disable/enable/planes vs. disable/planes/enable
- DRM_PLANE_COMMIT_ACTIVE_ONLY vs. all CRTCs
- drm_atomic_helper_wait_for_vblanks vs drm_atomic_helper_wait_for_flip_done

Maybe we could add a few CRTC commit helper flags along the line of 
DRM_PLANE_COMMIT_ACTIVE_ONLY, add a field to the drm_crtc structure to store 
them, and have drm_atomic_helper_commit_tail() use those flags to control the 
sequence of operations.

-- 
Regards,

Laurent Pinchart

Re: [PATCH V4 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver

[Lee Jones]

On Mon, 17 Jul 2017, Marek Vasut wrote:

> Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS
> entry. The MFD part only specifies the regmap bits for the PMIC and
> binds the subdevs together.
> 
> Signed-off-by: Marek Vasut 
> Cc: linux-ker...@vger.kernel.org
> Cc: Geert Uytterhoeven 
> Cc: Lee Jones 
> ---
> V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n)
> - Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n)
> - Make the AVS_VD09 range RW, so it can be used by the regulator
>   driver for the VD09 regulator
> - Report the regmap read return values when attempting to read ID
>   registers fails
> V3: No change
> V4: select REGMAP_IRQ
> ---
>  MAINTAINERS   |  11 ++
>  drivers/mfd/Kconfig   |  14 +++
>  drivers/mfd/Makefile  |   1 +
>  drivers/mfd/bd9571mwv.c   | 230 
> ++
>  include/linux/mfd/bd9571mwv.h | 115 +
>  5 files changed, 371 insertions(+)
>  create mode 100644 drivers/mfd/bd9571mwv.c
>  create mode 100644 include/linux/mfd/bd9571mwv.h

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v7 2/3] media: i2c: adv748x: add adv748x driver

[Sakari Ailus]

Hi Kieran,

A few more minor matters that you might want to address on top of Hans's
pull request.

On Thu, Jul 06, 2017 at 12:01:16PM +0100, Kieran Bingham wrote:
...
> +static int adv748x_afe_g_input_status(struct v4l2_subdev *sd, u32 *status)
> +{
> + struct adv748x_afe *afe = adv748x_sd_to_afe(sd);
> + struct adv748x_state *state = adv748x_afe_to_state(afe);
> + int ret;
> +
> + mutex_lock(>mutex);
> +
> + ret = adv748x_afe_status(afe, status, NULL);
> +
> + mutex_unlock(>mutex);

A newline here would be nice.

> + return ret;
> +}

...

> +int adv748x_csi2_set_pixelrate(struct v4l2_subdev *sd, s64 rate)
> +{
> + struct v4l2_ctrl *ctrl;
> +
> + ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);

It'd be much nicer to store the control pointer to your device's own struct
and use it. No need to look it up or check whether it was found.

> + if (!ctrl)
> + return -EINVAL;
> +
> + return v4l2_ctrl_s_ctrl_int64(ctrl, rate);
> +}

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

[R-CAR H3] Can't Modify U-Boot Environment Variables on Updated Firmware

[Eli Sherman]

Hello,
I understand that this is the mailing list for support for the R-Car line of 
boards. If this information is incorrect, please direct me, if possible, to the 
correct resource.
I have posted my issue to various rcar-related repos on github as well as the 
e-linux talk page. Issue is as follows:

I am trying to get up and running with the R-Car H3 Starter Kit Premier Board 
(h3ulcb). I first followed the instructions at 
http://elinux.org/R-Car/Boards/Yocto-Gen3 to build Yocto 2.19 for the board. 
When I attempted to run the built Yocto image on the board (both via tftp and 
via microSD), I received a kernel panic message. The limited resources on the 
internet (i.e., the other posts on this wiki) indicated this could likely be 
resolved by updating the firmware. I then followed the instructions at 
http://elinux.org/R-Car/Boards/H3SK#Flashing_firmware​ to update the firmware 
on the board. Now when I turn the board on, I cannot get to the stage where the 
option of cancelling autoboot (as shown in examples in the links given). I am 
therefore not able to change U-Boot's environment variables to specify the 
Image file to boot. It seems that U-Boot is perhaps not loading at all. Any 
help would be greatly appreciated!
Output of boot sequence is below:



user@user-linux:~$ sudo picocom -b 115200 --send-cmd "ascii-xfr -vvs" 
/dev/ttyUSB0
[sudo] password for user:
picocom v1.7
port is : /dev/ttyUSB0
flowcontrol : none
baudrate is : 115200
parity is : none
databits are : 8
escape is : C-a
local echo is : no
noinit is : no
noreset is : no
nolock is : no
send_cmd is : ascii-xfr -vvs
receive_cmd is : rz -vv
imap is :
omap is :
emap is : crcrlf,delbs,
Terminal ready
[ 0.000194] NOTICE: BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.14
[ 0.005761] NOTICE: BL2: PRR is R-Car H3 Ver2.0
[ 0.010335] NOTICE: BL2: Board is unknown Rev0.0
[ 0.015006] NOTICE: BL2: Boot device is HyperFlash(80MHz)
[ 0.020445] NOTICE: BL2: LCM state is CM
[ 0.024423] NOTICE: BL2: AVS setting succeeded. DVFS_SetVID=0x52
[ 0.030611] NOTICE: BL2: DDR3200(rev.0.22)NOTICE: [COLD_BOOT]NOTICE: ..0
[ 0.091270] NOTICE: BL2: DRAM Split is 4ch
[ 0.095155] NOTICE: BL2: QoS is default setting(rev.0.14)
[ 0.100655] NOTICE: BL2: Lossy Decomp areas
[ 0.104832] NOTICE: Entry 0: DCMPAREACRAx:0x8540 DCMPAREACRBx:0x570
[ 0.111917] NOTICE: Entry 1: DCMPAREACRAx:0x4000 DCMPAREACRBx:0x0
[ 0.118829] NOTICE: Entry 2: DCMPAREACRAx:0x2000 DCMPAREACRBx:0x0
[ 0.125743] NOTICE: BL2: v1.3(release):4eef9a2
[ 0.130233] NOTICE: BL2: Built : 17:35:56, Jul 13 2017
[ 0.135420] NOTICE: BL2: Normal boot
[ 0.139062] NOTICE: BL2: dst=0xe6320190 src=0x818 len=512(0x200)
[ 0.145608] NOTICE: BL2: dst=0x43f0 src=0x8180400 len=6144(0x1800)
[ 0.152070] NOTICE: BL2: dst=0x4400 src=0x81c len=65536(0x1)
[ 0.159295] NOTICE: BL2: dst=0x4410 src=0x820 len=524288(0x8)
[ 0.169775] NOTICE: BL2: dst=0x5000 src=0x864 len=1048576(0x10)

Thank you in advance for your help.
​

Re: [PATCH v4 3/3] v4l: async: add subnotifier to subdevices

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2017-07-18 09:11:19 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Mon, Jul 17, 2017 at 6:59 PM, Niklas Söderlund
>  wrote:
> > Add a subdevice specific notifier which can be used by a subdevice
> > driver to compliment the master device notifier to extend the subdevice
> > discovery.
> >
> > The master device registers the subdevices closest to itself in its
> > notifier while the subdevice(s) register notifiers for their closest
> > neighboring devices. Subdevice drivers configures a notifier at probe
> > time which are registered by the v4l2-async framework once the subdevice
> > itself is register, since it's only at this point the v4l2_dev is
> > available to the subnotifier.
> >
> > Using this incremental approach two problems can be solved:
> >
> > 1. The master device no longer has to care how many devices exist in
> >the pipeline. It only needs to care about its closest subdevice and
> >arbitrary long pipelines can be created without having to adapt the
> >master device for each case.
> >
> > 2. Subdevices which are represented as a single DT node but register
> >more than one subdevice can use this to improve the pipeline
> >discovery, since the subdevice driver is the only one who knows which
> >of its subdevices is linked with which subdevice of a neighboring DT
> >node.
> >
> > To allow subdevices to provide its own list of subdevices to the
> > v4l2-async framework v4l2_async_subdev_register_notifier() is added.
> > This new function must be called before the subdevice itself is
> > registered with the v4l2-async framework using
> > v4l2_async_register_subdev().
> >
> > Signed-off-by: Niklas Söderlund 
> 
> Thanks for your patch!
> 
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> 
> > @@ -217,6 +293,12 @@ void v4l2_async_notifier_unregister(struct 
> > v4l2_async_notifier *notifier)
> > "Failed to allocate device cache!\n");
> > }
> >
> > +   subdev = kvmalloc_array(n_subdev, sizeof(*subdev), GFP_KERNEL);
> > +   if (!dev) {
> 
> if (!subdev) {

Wops, copy-past coding strikes again! Will fix.

> 
> (noticed accidentally[*] :-)
> 
> > +   dev_err(notifier->v4l2_dev->dev,
> > +   "Failed to allocate subdevice cache!\n");
> > +   }
> > +
> > mutex_lock(_lock);
> >
> > list_del(>list);
> > @@ -224,6 +306,8 @@ void v4l2_async_notifier_unregister(struct 
> > v4l2_async_notifier *notifier)
> > list_for_each_entry_safe(sd, tmp, >done, async_list) {
> > if (dev)
> > dev[count] = get_device(sd->dev);
> > +   if (subdev)
> > +   subdev[count] = sd;
> 
> I don't like these "memory allocation fails, let's continue but check the
> pointer first"-games.
> Why not abort when the dev/subdev arrays cannot be allocated? It's not
> like the system is in a good state anyway.
> kmalloc() may have printed a big fat warning and invoked the OOM-killer
> already.

I agree, and I also don't like these "games". In my first attempt to 
work with this function I did remove the memory game, but then it hit me 
that if I did I would alter behavior of the function and I did not dare 
to do so.

Hopefully this can be addressed in the future if someone ever gets 
around to reworking the whole mess of re-probing devices which IMOH 
looks a but like a hack :-) I also ofc be happy to just remove the 
memory game from this function and as you suggest simply abort if the 
allocation fails, but I feel I need the blessing from the v4l2 community 
before doing so. Sometimes v4l2 do funny stuff for legacy reasons beyond 
my understanding.

> 
> [*] while checking if you perhaps removed the "dev" games in a later patch.
>  No, you added another one :-(
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

Re: [PATCH 4/4] drm/sun4i: make sure we don't have a commit pending

[Daniel Vetter]

On Tue, Jul 18, 2017 at 9:07 AM, Maxime Ripard
 wrote:
> On Mon, Jul 17, 2017 at 02:57:19PM +0800, Chen-Yu Tsai wrote:
>> On Mon, Jul 17, 2017 at 2:55 PM, Maxime Ripard
>>  wrote:
>> > On Fri, Jul 14, 2017 at 04:56:01PM +0800, Chen-Yu Tsai wrote:
>> >> Hi,
>> >>
>> >> On Thu, Jul 13, 2017 at 10:41 PM, Maxime Ripard
>> >>  wrote:
>> >> > In the earlier display engine designs, any register access while a 
>> >> > commit
>> >> > is pending is forbidden.
>> >> >
>> >> > One of the symptoms is that reading a register will return another, 
>> >> > random,
>> >> > register value which can lead to register corruptions if we ever do a
>> >> > read/modify/write cycle.
>> >>
>> >> Alternatively, if changes to the backend (layers) are guaranteed to happen
>> >> while the CRTC is disabled (which seems to be the case after looking at
>> >> drm_atomic_helper_commit_planes and drm_atomic_helper_commit_tail), we
>> >> could just turn on register auto-commit all the time and not deal with
>> >> this.
>> >
>> > As far as I understand, it will only be the case if we need a new
>> > modeset or we changed the active CRTC or connectors. But if you change
>> > only the format, buffers or properties it won't be the case, and we'll
>> > need to commit.
>>
>> So in other words, if someone were to use it for actual compositing and
>> moved the upper composited layer around, we would need commit support to be
>> safe.
>>
>> Sounds more or less like something a video player would do.
>
> Not only that. A change of buffer will happen every frame or so, and
> we can change the format whenever we want too (even if it's usually
> going to be in sync with a new buffer). Changing a property can happen
> any time too (like zpos for example).

You can upgrade any property change to an atomic modeset by e.g.
setting connector->mode_changed (and then making sure to call
check_modeset() helper again perhaps). This is for cases where your hw
can't handle a property change within 1 vblank. The default is just
the solution for most common hw.

The other way round works too, you can clear these flags in your
atomic_check callbacks. But that requires a bit more care (to make
sure you never clear it when there's something else also changing that
still needs a full modeset sequence to commit to hw).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [renesas-drivers:topic/renesas-overlays 1/86] drivers//of/configfs.c:48:2: error: implicit declaration of function 'of_fdt_unflatten_tree'

[Geert Uytterhoeven]

On Tue, Jul 18, 2017 at 8:44 AM, Geert Uytterhoeven
 wrote:
> On Mon, Jul 17, 2017 at 7:58 PM, kbuild test robot
>  wrote:
>> tree:   
>> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
>> topic/renesas-overlays
>> head:   8361d7e7432d1937f94e1863211fb3f852e37f36
>> commit: 1ba23467c1d052c0b5c35436c034f0fb7103eeaf [1/86] OF: DT-Overlay 
>> configfs interface (v7)
>> config: sparc64-allmodconfig (attached as .config)
>> compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
>> reproduce:
>> wget 
>> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
>> ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> git checkout 1ba23467c1d052c0b5c35436c034f0fb7103eeaf
>> # save the attached .config to linux build tree
>> make.cross ARCH=sparc64
>>
>> All errors (new ones prefixed by >>):
>>
>>drivers//of/configfs.c: In function 'create_overlay':
 drivers//of/configfs.c:48:2: error: implicit declaration of function 
 'of_fdt_unflatten_tree' [-Werror=implicit-function-declaration]
>>  of_fdt_unflatten_tree(blob, NULL, >overlay);
>>  ^
>>cc1: some warnings being treated as errors
>
> Apparently OF_OVERLAY should depend on OF_FLATTREE.

Oops, I meant CONFIG_OF_CONFIGFS.
But that causes warnings due to recursive dependencies:

drivers/of/Kconfig:59:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/of/Kconfig:59: symbol OF_EARLY_FLATTREE is selected by OF_UNITTEST
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/of/Kconfig:14: symbol OF_UNITTEST depends on OF_IRQ
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/of/Kconfig:83: symbol OF_IRQ depends on IRQ_DOMAIN
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
kernel/irq/Kconfig:63: symbol IRQ_DOMAIN is selected by GENERIC_IRQ_CHIP
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
kernel/irq/Kconfig:58: symbol GENERIC_IRQ_CHIP is selected by GPIO_DWAPB
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpio/Kconfig:160: symbol GPIO_DWAPB depends on GPIOLIB
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpio/Kconfig:13: symbol GPIOLIB is selected by PINCTRL_STM32
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/pinctrl/stm32/Kconfig:3: symbol PINCTRL_STM32 is selected by
PINCTRL_STM32F429
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/pinctrl/stm32/Kconfig:11: symbol PINCTRL_STM32F429 depends on
IRQ_DOMAIN_HIERARCHY
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
kernel/irq/Kconfig:67: symbol IRQ_DOMAIN_HIERARCHY is selected by
GENERIC_MSI_IRQ_DOMAIN
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
kernel/irq/Kconfig:80: symbol GENERIC_MSI_IRQ_DOMAIN is selected by MV_XOR_V2
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/dma/Kconfig:358: symbol MV_XOR_V2 depends on DMADEVICES
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/dma/Kconfig:5: symbol DMADEVICES is selected by CRYPTO_DEV_CCP_DD
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/crypto/ccp/Kconfig:1: symbol CRYPTO_DEV_CCP_DD depends on CRYPTO
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
crypto/Kconfig:15: symbol CRYPTO is selected by IP_SCTP
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
net/sctp/Kconfig:5: symbol IP_SCTP is selected by DLM
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
fs/dlm/Kconfig:1: symbol DLM depends on SYSFS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
fs/sysfs/Kconfig:1: symbol SYSFS is selected by CONFIGFS_FS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection 

Re: [PATCH 1/5] watchdog: renesas_wdt: avoid (theoretical) type overflow

[Wolfram Sang]

> > -   unsigned long rate;
> > -   unsigned int clks_per_sec;
> > +   unsigned long rate, clks_per_sec;
> 
> If you make this change, you should also update rwdt_priv.clks_per_sec
> (yes I know it's removed in a later patch in this series).

Right. I will change but also wait a bit for more comments.

Thanks!



signature.asc
Description: PGP signature

Re: [PATCH v4 3/3] v4l: async: add subnotifier to subdevices

[Geert Uytterhoeven]

Hi Niklas,

On Mon, Jul 17, 2017 at 6:59 PM, Niklas Söderlund
 wrote:
> Add a subdevice specific notifier which can be used by a subdevice
> driver to compliment the master device notifier to extend the subdevice
> discovery.
>
> The master device registers the subdevices closest to itself in its
> notifier while the subdevice(s) register notifiers for their closest
> neighboring devices. Subdevice drivers configures a notifier at probe
> time which are registered by the v4l2-async framework once the subdevice
> itself is register, since it's only at this point the v4l2_dev is
> available to the subnotifier.
>
> Using this incremental approach two problems can be solved:
>
> 1. The master device no longer has to care how many devices exist in
>the pipeline. It only needs to care about its closest subdevice and
>arbitrary long pipelines can be created without having to adapt the
>master device for each case.
>
> 2. Subdevices which are represented as a single DT node but register
>more than one subdevice can use this to improve the pipeline
>discovery, since the subdevice driver is the only one who knows which
>of its subdevices is linked with which subdevice of a neighboring DT
>node.
>
> To allow subdevices to provide its own list of subdevices to the
> v4l2-async framework v4l2_async_subdev_register_notifier() is added.
> This new function must be called before the subdevice itself is
> registered with the v4l2-async framework using
> v4l2_async_register_subdev().
>
> Signed-off-by: Niklas Söderlund 

Thanks for your patch!

> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c

> @@ -217,6 +293,12 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
> "Failed to allocate device cache!\n");
> }
>
> +   subdev = kvmalloc_array(n_subdev, sizeof(*subdev), GFP_KERNEL);
> +   if (!dev) {

if (!subdev) {

(noticed accidentally[*] :-)

> +   dev_err(notifier->v4l2_dev->dev,
> +   "Failed to allocate subdevice cache!\n");
> +   }
> +
> mutex_lock(_lock);
>
> list_del(>list);
> @@ -224,6 +306,8 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
> list_for_each_entry_safe(sd, tmp, >done, async_list) {
> if (dev)
> dev[count] = get_device(sd->dev);
> +   if (subdev)
> +   subdev[count] = sd;

I don't like these "memory allocation fails, let's continue but check the
pointer first"-games.
Why not abort when the dev/subdev arrays cannot be allocated? It's not
like the system is in a good state anyway.
kmalloc() may have printed a big fat warning and invoked the OOM-killer
already.

[*] while checking if you perhaps removed the "dev" games in a later patch.
 No, you added another one :-(

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] drm/sun4i: make sure we don't have a commit pending

[Maxime Ripard]

On Mon, Jul 17, 2017 at 02:57:19PM +0800, Chen-Yu Tsai wrote:
> On Mon, Jul 17, 2017 at 2:55 PM, Maxime Ripard
>  wrote:
> > On Fri, Jul 14, 2017 at 04:56:01PM +0800, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Thu, Jul 13, 2017 at 10:41 PM, Maxime Ripard
> >>  wrote:
> >> > In the earlier display engine designs, any register access while a commit
> >> > is pending is forbidden.
> >> >
> >> > One of the symptoms is that reading a register will return another, 
> >> > random,
> >> > register value which can lead to register corruptions if we ever do a
> >> > read/modify/write cycle.
> >>
> >> Alternatively, if changes to the backend (layers) are guaranteed to happen
> >> while the CRTC is disabled (which seems to be the case after looking at
> >> drm_atomic_helper_commit_planes and drm_atomic_helper_commit_tail), we
> >> could just turn on register auto-commit all the time and not deal with
> >> this.
> >
> > As far as I understand, it will only be the case if we need a new
> > modeset or we changed the active CRTC or connectors. But if you change
> > only the format, buffers or properties it won't be the case, and we'll
> > need to commit.
> 
> So in other words, if someone were to use it for actual compositing and
> moved the upper composited layer around, we would need commit support to be
> safe.
>
> Sounds more or less like something a video player would do.

Not only that. A change of buffer will happen every frame or so, and
we can change the format whenever we want too (even if it's usually
going to be in sync with a new buffer). Changing a property can happen
any time too (like zpos for example).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Maxime Ripard]

Hi Laurent,

On Fri, Jul 14, 2017 at 02:43:12AM +0300, Laurent Pinchart wrote:
> Hi Maxime,
> 
> Thank you for the patch.
> 
> On Thursday 13 Jul 2017 16:41:13 Maxime Ripard wrote:
> > The current drm_atomic_helper_commit_tail helper works only if the CRTC is
> > accessible, and documents an alternative implementation that is supposed to
> > be used if that happens.
> > 
> > That implementation is then duplicated by some drivers. Instead of
> > documenting it, let's implement an helper that all the relevant users can
> > use directly.
> > 
> > Signed-off-by: Maxime Ripard 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c| 47 +++
> >  drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
> 
> I've submitted "[PATCH] drm: rcar-du: Setup planes before enabling CRTC to 
> avoid flicker" that changes the rcar-du implementation to the standard 
> disable/update planes/enable order, so I'd appreciate if you could drop the 
> rcar-du part of this patch to avoid conflicts.

I will.

> This being said, the reason why I switched back from the "runtime PM" to the 
> "standard" order is probably of interest to you. Quoting the commit message,
> 
> > Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
> > start to CRTC resume") changed the order of the plane commit and CRTC
> > enable operations to accommodate the runtime PM requirements. However,
> > this introduced corruption in the first displayed frame, as the CRTC is
> > now enabled without any plane configured. On Gen2 hardware the first
> > frame will be black and likely unnoticed, but on Gen3 hardware we end up
> > starting the display before the VSP compositor, which is more
> > noticeable.
> > 
> > To fix this, revert the order of the commit operations back, and handle
> > runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
> > helper operation handlers.
> 
> I believe that the "runtime PM" order is problematic in most drivers. The 
> problem usually goes unnoticed as most monitors will not even display the 
> first frame, and I assume many devices will just output it black, but it's an 
> issue nonetheless.
> 
> Note that my driver hasn't lost the "runtime PM" requirements, so I had to 
> support them with the "standard" order. The best way I've found was to 
> runtime 
> resume in the one of .atomic_begin() and .enable() that is run first. Not 
> very 
> neat, as similar code would be needed in most drivers. I wonder whether it 
> wouldn't be useful to add resume/suspend helper callbacks for the CRTC.

I'm not sure it would apply. Our driver doesn't use runtime_pm at all,
but in order for the commits to happen, we need to have the CRTC
active, but it will remain powered up the whole time. I'm not sure if
we'll ever see such a frame.

But since this seems to be a pretty generic, maybe we should address
it in the helper itself?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH 1/5] watchdog: renesas_wdt: avoid (theoretical) type overflow

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, Jul 17, 2017 at 5:12 PM, Wolfram Sang
 wrote:
> Because the smallest clock divider we can select is 1, 'clks_per_sec'
> must be the same type as 'rate'.
>
> Signed-off-by: Wolfram Sang 

Thanks for your patch!

> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -112,8 +112,7 @@ static int rwdt_probe(struct platform_device *pdev)
>  {
> struct rwdt_priv *priv;
> struct resource *res;
> -   unsigned long rate;
> -   unsigned int clks_per_sec;
> +   unsigned long rate, clks_per_sec;

If you make this change, you should also update rwdt_priv.clks_per_sec
(yes I know it's removed in a later patch in this series).

> int ret, i;
>
> priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);

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: [RFC v2 6/6] ARM: dts: blanche: add SCIF1 and MAX9260 deserializer

[Geert Uytterhoeven]

Hi Uli,

On Mon, Jul 17, 2017 at 5:24 PM, Ulrich Hecht
 wrote:
> Adds serial port SCIF1 and the MAX9260 deserializers connected to it.
>
> Signed-off-by: Ulrich Hecht 

Thanks for your patch!

>  arch/arm/boot/dts/r8a7792-blanche.dts | 52 
> +++
>  1 file changed, 52 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r8a7792-blanche.dts 
> b/arch/arm/boot/dts/r8a7792-blanche.dts
> index 9b67dca..2ae9a87 100644
> --- a/arch/arm/boot/dts/r8a7792-blanche.dts
> +++ b/arch/arm/boot/dts/r8a7792-blanche.dts

> status = "okay";
>  };
>
> + {
> +   pinctrl-0 = <_pins>;
> +   pinctrl-names = "default";
> +
> +   status = "okay";
> +
> +   mux-controls = <>;
> +
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   gmsl-deserializer@0 {
> +   compatible = "maxim,max9260";
> +   reg = <0x8>;

unit address and reg property don't match.

(try "make dtbs W=1")

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: [RFC v2 5/6] max9260: add driver for i2c over GMSL passthrough

[Geert Uytterhoeven]

Hi Ulrich,

On Mon, Jul 17, 2017 at 5:24 PM, Ulrich Hecht
 wrote:
> This driver implements tunnelling of i2c requests over GMSL via a
> MAX9260 deserializer. It provides an i2c adapter that can be used
> to reach devices on the far side of the link.
>
> Signed-off-by: Ulrich Hecht 

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/media/i2c/max9260.c
> @@ -0,0 +1,288 @@

> +static void max9260_transact(struct max9260_device *dev,
> +int expect,
> +u8 *request, int len)

const u8 *request (or const void *?)

> +static int max9260_read_reg(struct max9260_device *dev, int reg)
> +{
> +   u8 request[] = { 0x79, 0x91, reg, 1 };

static const

I don't know if this buffer is ever copied. If not, perhaps it should not be
on the stack anyway because some serial drivers may not support DMA
from stack buffers?
This applies to all buffers 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: [renesas-drivers:topic/renesas-overlays 1/86] drivers//of/configfs.c:48:2: error: implicit declaration of function 'of_fdt_unflatten_tree'

[Geert Uytterhoeven]

On Mon, Jul 17, 2017 at 7:58 PM, kbuild test robot
 wrote:
> tree:   
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
> topic/renesas-overlays
> head:   8361d7e7432d1937f94e1863211fb3f852e37f36
> commit: 1ba23467c1d052c0b5c35436c034f0fb7103eeaf [1/86] OF: DT-Overlay 
> configfs interface (v7)
> config: sparc64-allmodconfig (attached as .config)
> compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget 
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout 1ba23467c1d052c0b5c35436c034f0fb7103eeaf
> # save the attached .config to linux build tree
> make.cross ARCH=sparc64
>
> All errors (new ones prefixed by >>):
>
>drivers//of/configfs.c: In function 'create_overlay':
>>> drivers//of/configfs.c:48:2: error: implicit declaration of function 
>>> 'of_fdt_unflatten_tree' [-Werror=implicit-function-declaration]
>  of_fdt_unflatten_tree(blob, NULL, >overlay);
>  ^
>cc1: some warnings being treated as errors

Apparently OF_OVERLAY should depend on OF_FLATTREE.

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