Re: [PATCH v2] clk: renesas: mstp: Support 8-bit registers for r7s72100

[Kuninori Morimoto]

Hi Chris

> The RZ/A1 is different than the other Renesas SOCs because the MSTP
> registers are 8-bit instead of 32-bit and if you try writing values as
> 32-bit nothing happens...meaning this driver never worked for r7s72100.
> 
> Fixes: b6face404f38 ("ARM: shmobile: r7s72100: add essential clock nodes to 
> dtsi")
> Signed-off-by: Chris Brandt 
> ---

Thanks. I looks nice for me

Acked-by: Kuninori Morimoto 

> v2:
> * move r7s72100 detection inside cpg_mstp_clocks_init()
> * change width_8bit flag from global to inside group struct
> ---
>  drivers/clk/renesas/clk-mstp.c | 27 ++-
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
> index 9375777..b533f99 100644
> --- a/drivers/clk/renesas/clk-mstp.c
> +++ b/drivers/clk/renesas/clk-mstp.c
> @@ -37,12 +37,14 @@
>   * @smstpcr: module stop control register
>   * @mstpsr: module stop status register (optional)
>   * @lock: protects writes to SMSTPCR
> + * @width_8bit: registers are 8-bit, not 32-bit
>   */
>  struct mstp_clock_group {
>   struct clk_onecell_data data;
>   void __iomem *smstpcr;
>   void __iomem *mstpsr;
>   spinlock_t lock;
> + bool width_8bit;
>  };
>  
>  /**
> @@ -59,6 +61,18 @@ struct mstp_clock {
>  
>  #define to_mstp_clock(_hw) container_of(_hw, struct mstp_clock, hw)
>  
> +static inline u32 cpg_mstp_read(struct mstp_clock_group *group,
> + u32 __iomem *reg)
> +{
> + return group->width_8bit ? readb(reg) : clk_readl(reg);
> +}
> +
> +static inline void cpg_mstp_write(struct mstp_clock_group *group, u32 val,
> +   u32 __iomem *reg)
> +{
> + group->width_8bit ? writeb(val, reg) : clk_writel(val, reg);
> +}
> +
>  static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable)
>  {
>   struct mstp_clock *clock = to_mstp_clock(hw);
> @@ -70,12 +84,12 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, 
> bool enable)
>  
>   spin_lock_irqsave(>lock, flags);
>  
> - value = clk_readl(group->smstpcr);
> + value = cpg_mstp_read(group, group->smstpcr);
>   if (enable)
>   value &= ~bitmask;
>   else
>   value |= bitmask;
> - clk_writel(value, group->smstpcr);
> + cpg_mstp_write(group, value, group->smstpcr);
>  
>   spin_unlock_irqrestore(>lock, flags);
>  
> @@ -83,7 +97,7 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool 
> enable)
>   return 0;
>  
>   for (i = 1000; i > 0; --i) {
> - if (!(clk_readl(group->mstpsr) & bitmask))
> + if (!(cpg_mstp_read(group, group->mstpsr) & bitmask))
>   break;
>   cpu_relax();
>   }
> @@ -114,9 +128,9 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
>   u32 value;
>  
>   if (group->mstpsr)
> - value = clk_readl(group->mstpsr);
> + value = cpg_mstp_read(group, group->mstpsr);
>   else
> - value = clk_readl(group->smstpcr);
> + value = cpg_mstp_read(group, group->smstpcr);
>  
>   return !(value & BIT(clock->bit_index));
>  }
> @@ -188,6 +202,9 @@ static void __init cpg_mstp_clocks_init(struct 
> device_node *np)
>   return;
>   }
>  
> + if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks"))
> + group->width_8bit = true;
> +
>   for (i = 0; i < MSTP_MAX_CLOCKS; ++i)
>   clks[i] = ERR_PTR(-ENOENT);
>  
> -- 
> 2.10.1
> 
> 

Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support

[Linus Walleij]

On Fri, Dec 9, 2016 at 12:21 AM, Laurent Pinchart
 wrote:
> On Friday 09 Dec 2016 00:15:51 Niklas Söderlund wrote:

>> This was changed for the whole driver after the original patch was
>> applied (at the time of change the patch was not yet reverted), see [1].
>> I needed to update this when I resurrected the patch, maybe I could have
>> been more clever and reverted the revert patch but this felt cleaner, if
>> it's better to do it the other way around and revert a revert please let
>> me know so I can do so in the future.
>
> No, this is fine. I'm a bit dubious about [1] given that it consumes more CPU
> cycles without any benefit as far as I can see. Maybe I can't see far enough
> though, Linus Walleij could prove me wrong :-)

This was changed mainly for maintainability and code readability.
Lots of subsystems have the option to carry around a data poiner
and/or allocate an extra memory chunk for state containers.

I am relaxed on it, if someone very much want to use container_of()
because of $REASON I am not really bothered.

Yours,
Linus Walleij

Applied "spi: core: Extract of_spi_parse_dt()" to the spi tree

[Mark Brown]

The patch

   spi: core: Extract of_spi_parse_dt()

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c2e51ac3d0542440d5b2b8b52ff2ad00751af4da Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Mon, 12 Sep 2016 22:50:41 +0200
Subject: [PATCH] spi: core: Extract of_spi_parse_dt()

Extract the parsing of SPI slave-specific properties into its own
function, so it can be reused later for SPI slave controllers.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 drivers/spi/spi.c | 60 +--
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 656dd3e3220c..1861255866d7 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1502,37 +1502,18 @@ static int spi_master_initialize_queue(struct 
spi_master *master)
 /*-*/
 
 #if defined(CONFIG_OF)
-static struct spi_device *
-of_register_spi_device(struct spi_master *master, struct device_node *nc)
+static int of_spi_parse_dt(struct spi_master *master, struct spi_device *spi,
+  struct device_node *nc)
 {
-   struct spi_device *spi;
-   int rc;
u32 value;
-
-   /* Alloc an spi_device */
-   spi = spi_alloc_device(master);
-   if (!spi) {
-   dev_err(>dev, "spi_device alloc error for %s\n",
-   nc->full_name);
-   rc = -ENOMEM;
-   goto err_out;
-   }
-
-   /* Select device driver */
-   rc = of_modalias_node(nc, spi->modalias,
-   sizeof(spi->modalias));
-   if (rc < 0) {
-   dev_err(>dev, "cannot find modalias for %s\n",
-   nc->full_name);
-   goto err_out;
-   }
+   int rc;
 
/* Device address */
rc = of_property_read_u32(nc, "reg", );
if (rc) {
dev_err(>dev, "%s has no valid 'reg' property (%d)\n",
nc->full_name, rc);
-   goto err_out;
+   return rc;
}
spi->chip_select = value;
 
@@ -1590,10 +1571,41 @@ of_register_spi_device(struct spi_master *master, 
struct device_node *nc)
if (rc) {
dev_err(>dev, "%s has no valid 'spi-max-frequency' 
property (%d)\n",
nc->full_name, rc);
-   goto err_out;
+   return rc;
}
spi->max_speed_hz = value;
 
+   return 0;
+}
+
+static struct spi_device *
+of_register_spi_device(struct spi_master *master, struct device_node *nc)
+{
+   struct spi_device *spi;
+   int rc;
+
+   /* Alloc an spi_device */
+   spi = spi_alloc_device(master);
+   if (!spi) {
+   dev_err(>dev, "spi_device alloc error for %s\n",
+   nc->full_name);
+   rc = -ENOMEM;
+   goto err_out;
+   }
+
+   /* Select device driver */
+   rc = of_modalias_node(nc, spi->modalias,
+   sizeof(spi->modalias));
+   if (rc < 0) {
+   dev_err(>dev, "cannot find modalias for %s\n",
+   nc->full_name);
+   goto err_out;
+   }
+
+   rc = of_spi_parse_dt(master, spi, nc);
+   if (rc)
+   goto err_out;
+
/* Store a pointer to the node in the device structure */
of_node_get(nc);
spi->dev.of_node = nc;
-- 
2.11.0

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Mark Brown]

On Mon, Sep 12, 2016 at 10:50:42PM +0200, Geert Uytterhoeven wrote:

> TBD:
>   - s/spi_master/spi_controller/ where appropriate,
>   - Provide wrappers (e.g. "#define spi_master spi_controller" until all
> SPI drivers have been converted),
>   - Do we want a separate spi_register_slave() instead?

This basically looks fine to me - there's these TBDs so I might be
missing things and we probably need some GPIO chip select handling but
that's a separate thing.  Sorry it took me so long to review this.


signature.asc
Description: PGP signature

Re: [PATCH/RFC v2 3/7] spi: core: Add support for registering SPI slave controllers

[Mark Brown]

On Sun, Sep 18, 2016 at 11:04:18AM +0200, Geert Uytterhoeven wrote:

> This is caused by moving the setup of master->dev.class.
> To fix this, I can
>   1) Introduce a separate spi_alloc_slave() function, which sets up
>  spi_slave_class instead of spi_master class,

This seems more idiomatic.


signature.asc
Description: PGP signature

[PATCH v2] clk: renesas: mstp: Support 8-bit registers for r7s72100

[Chris Brandt]

The RZ/A1 is different than the other Renesas SOCs because the MSTP
registers are 8-bit instead of 32-bit and if you try writing values as
32-bit nothing happens...meaning this driver never worked for r7s72100.

Fixes: b6face404f38 ("ARM: shmobile: r7s72100: add essential clock nodes to 
dtsi")
Signed-off-by: Chris Brandt 
---
v2:
* move r7s72100 detection inside cpg_mstp_clocks_init()
* change width_8bit flag from global to inside group struct
---
 drivers/clk/renesas/clk-mstp.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index 9375777..b533f99 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -37,12 +37,14 @@
  * @smstpcr: module stop control register
  * @mstpsr: module stop status register (optional)
  * @lock: protects writes to SMSTPCR
+ * @width_8bit: registers are 8-bit, not 32-bit
  */
 struct mstp_clock_group {
struct clk_onecell_data data;
void __iomem *smstpcr;
void __iomem *mstpsr;
spinlock_t lock;
+   bool width_8bit;
 };
 
 /**
@@ -59,6 +61,18 @@ struct mstp_clock {
 
 #define to_mstp_clock(_hw) container_of(_hw, struct mstp_clock, hw)
 
+static inline u32 cpg_mstp_read(struct mstp_clock_group *group,
+   u32 __iomem *reg)
+{
+   return group->width_8bit ? readb(reg) : clk_readl(reg);
+}
+
+static inline void cpg_mstp_write(struct mstp_clock_group *group, u32 val,
+ u32 __iomem *reg)
+{
+   group->width_8bit ? writeb(val, reg) : clk_writel(val, reg);
+}
+
 static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable)
 {
struct mstp_clock *clock = to_mstp_clock(hw);
@@ -70,12 +84,12 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool 
enable)
 
spin_lock_irqsave(>lock, flags);
 
-   value = clk_readl(group->smstpcr);
+   value = cpg_mstp_read(group, group->smstpcr);
if (enable)
value &= ~bitmask;
else
value |= bitmask;
-   clk_writel(value, group->smstpcr);
+   cpg_mstp_write(group, value, group->smstpcr);
 
spin_unlock_irqrestore(>lock, flags);
 
@@ -83,7 +97,7 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool 
enable)
return 0;
 
for (i = 1000; i > 0; --i) {
-   if (!(clk_readl(group->mstpsr) & bitmask))
+   if (!(cpg_mstp_read(group, group->mstpsr) & bitmask))
break;
cpu_relax();
}
@@ -114,9 +128,9 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
u32 value;
 
if (group->mstpsr)
-   value = clk_readl(group->mstpsr);
+   value = cpg_mstp_read(group, group->mstpsr);
else
-   value = clk_readl(group->smstpcr);
+   value = cpg_mstp_read(group, group->smstpcr);
 
return !(value & BIT(clock->bit_index));
 }
@@ -188,6 +202,9 @@ static void __init cpg_mstp_clocks_init(struct device_node 
*np)
return;
}
 
+   if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks"))
+   group->width_8bit = true;
+
for (i = 0; i < MSTP_MAX_CLOCKS; ++i)
clks[i] = ERR_PTR(-ENOENT);
 
-- 
2.10.1

[PATCH v2 0/2] arm64: dts: r8a7796: salvator-x: Enable EthernetAVB

[Geert Uytterhoeven]

Hi Simon, Magnus,

This patch series enables networking on r8a7796/salvator-x.

Network performance is currently limited to 100 Mbps by the ravb driver.
Performance figures with nuttcp are ca. 94 Mbps for both receive and
transmit, at 12% vs. 4% CPU load. For receive, this is comparable to
sh_eth on Koelsch; for transmit, it's ca. 20% faster.

With "ravb: Support 1Gbps on R-Car H3 ES1.1+ and R-Car M3-W" applied,
network throughput is higher, but has a high variance:
  - receive: 245-426 Mbps, at 100% CPU load (ksoftirqd + nuttcp),
  - transmit: 426 Mbps, at 95% CPU load (nuttcp).
When limiting memory to 1 GiB, and thus avoiding swiotlb bounce buffers,
network throughput increases (CPU load is similar though) to 546 resp.
625 Mbps.
The 50% performance penalty of swiotlb is expected to be mitigated when
IOMMU support will become available.

Note that at high receive speeds, the driver sometimes prints:

ravb e680.ethernet eth0: Receive Descriptor Empty

This may even cause nuttcp to fail with:

nuttcp-t: v6.1.2: Error: server not ACKing data

NFS root survives fine, though.
I'm wondering if this is also seen on the R-Car Gen2 boards where
EthernetAVB is available?

Changes compared to v1, as sent by Laurent before:
  - Add pinctrl. Drive strength is not yet included, like on current
r8a7795/salvator-x.

This series is based on renesas-devel-20161212-v4.9. To actually work,
you need to merge my sh-pfc-for-v4.10 branch, which is upstream and will
be in v4.10-rc1. As this is a pure runtime dependency, and does not
imply a regression, this series can be applied now.

Thanks for applying!

Laurent Pinchart (2):
  arm64: dts: renesas: r8a7796: Add EthernetAVB instance
  arm64: dts: r8a7796: salvator-x: Enable EthernetAVB

 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 32 
 arch/arm64/boot/dts/renesas/r8a7796.dtsi   | 43 ++
 2 files changed, 75 insertions(+)

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

[PATCH v2 2/2] arm64: dts: r8a7796: salvator-x: Enable EthernetAVB

[Geert Uytterhoeven]

From: Laurent Pinchart 

Signed-off-by: Laurent Pinchart 
[geert: Add pinctrl]
Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Add pinctrl.
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 32 ++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index 38bde9de3250ecbb..c7f40f8f3169f36b 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -18,6 +18,7 @@
 
aliases {
serial0 = 
+   ethernet0 = 
};
 
chosen {
@@ -107,6 +108,11 @@
pinctrl-0 = <_clk_pins>;
pinctrl-names = "default";
 
+   avb_pins: avb {
+   groups = "avb_mdc";
+   function = "avb";
+   };
+
scif2_pins: scif2 {
groups = "scif2_data_a";
function = "scif2";
@@ -158,6 +164,32 @@
};
 };
 
+ {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+   renesas,no-ether-link;
+   phy-handle = <>;
+   status = "okay";
+
+   phy0: ethernet-phy@0 {
+   rxc-skew-ps = <900>;
+   rxdv-skew-ps = <0>;
+   rxd0-skew-ps = <0>;
+   rxd1-skew-ps = <0>;
+   rxd2-skew-ps = <0>;
+   rxd3-skew-ps = <0>;
+   txc-skew-ps = <900>;
+   txen-skew-ps = <0>;
+   txd0-skew-ps = <0>;
+   txd1-skew-ps = <0>;
+   txd2-skew-ps = <0>;
+   txd3-skew-ps = <0>;
+   reg = <0>;
+   interrupt-parent = <>;
+   interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+   };
+};
+
 _clk {
clock-frequency = <1666>;
 };
-- 
1.9.1

[PATCH v2 1/2] arm64: dts: r8a7796: Add EthernetAVB instance

[Geert Uytterhoeven]

From: Laurent Pinchart 

Signed-off-by: Laurent Pinchart 
Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Rebased.
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 43 
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 7bf0f2f6c2243cfc..d9d8dc53b408c36e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -421,6 +421,49 @@
};
};
 
+   avb: ethernet@e680 {
+   compatible = "renesas,etheravb-r8a7796",
+"renesas,etheravb-rcar-gen3";
+   reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "ch0", "ch1", "ch2", "ch3",
+ "ch4", "ch5", "ch6", "ch7",
+ "ch8", "ch9", "ch10", "ch11",
+ "ch12", "ch13", "ch14", "ch15",
+ "ch16", "ch17", "ch18", "ch19",
+ "ch20", "ch21", "ch22", "ch23",
+ "ch24";
+   clocks = < CPG_MOD 812>;
+   power-domains = < R8A7796_PD_ALWAYS_ON>;
+   phy-mode = "rgmii-id";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
scif2: serial@e6e88000 {
compatible = "renesas,scif-r8a7796",
 "renesas,rcar-gen3-scif", "renesas,scif";
-- 
1.9.1

[PATCH resend] arm64: dts: r8a7796: salvator-x: Update memory node to 4 GiB map

[Geert Uytterhoeven]

From: Takeshi Kihara 

This patch updates memory region:

  - After changes, the new map of the Salvator-X board on R8A7796 SoC
Bank0: 2GiB RAM : 0x4800 -> 0x000bfff
Bank1: 2GiB RAM : 0x0006 -> 0x0067fff

  - Before changes, the old map looked like this:
Bank0: 2GiB RAM : 0x4800 -> 0x000bfff

Signed-off-by: Takeshi Kihara 
[geert: Correct size of old map]
Signed-off-by: Geert Uytterhoeven 
---
Simon, please apply!

U-Boot already adds the second memory region to the "reg" property of
the first memory node in DT, so this patch is actually a no-op.
IMHO it doesn't make much sense to keep on pretending we don't have
enabled memory outside the 32-bit address space.
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index f35e96ca7d6050b7..38bde9de3250ecbb 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -31,6 +31,11 @@
reg = <0x0 0x4800 0x0 0x7800>;
};
 
+   memory@6 {
+   device_type = "memory";
+   reg = <0x6 0x 0x0 0x8000>;
+   };
+
reg_1p8v: regulator0 {
compatible = "regulator-fixed";
regulator-name = "fixed-1.8V";
-- 
1.9.1

RE: [PATCH] clk: renesas: mstp: Support 8-bit registers for r7s72100

[Chris Brandt]

Hi Geert and Morimoto-san,

On December 15, 2016, Geert Uytterhoeven wrote:
> Or you can add an of_device_is_compatible(np, "renesas,r7s72100-mstp-
> clocks") check to cpg_mstp_clocks_init(), keeping the single
> CLK_OF_DECLARE().
> 
> I think that would also performs slightly better, as only nodes compatible
> with "renesas,cpg-mstp-clocks" would be subject to the additional check.
> 

I think now I am leaning more towards that idea:

 * use of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")
 * add reg_width_8bit to group (not global)


The reason is that I HOPE that this one SOC will be the only Renesas device 
that will ever
have 8-bit MSTP registers. All the other MPU devices seem to have 32-bit.

Therefore, adding a simple 1-line of_device_is_compatible for this one case 
seems the best.

Thank you for the suggestions.

Chris

Re: [PATCHv3 2/4] v4l: vsp1: Refactor video pipeline configuration

[Kieran Bingham]

Hi Laurent,

On 14/12/16 16:30, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tuesday 13 Dec 2016 17:59:42 Kieran Bingham wrote:
>> With multiple inputs through the BRU it is feasible for the streams to
>> race each other at stream-on.
> 
> Could you please explain the race condition in the commit message ? The issue 
> is that multiple VIDIOC_STREAMON calls racing each other could have process 
> N-1 skipping over the pipeline setup section and then start the pipeline, if 
> videobuf2 has already enqueued buffers to the driver for process N but not 
> called the .start_streaming() operation yet.
> 
>> In the case of the video pipelines, this
>> can present two serious issues.
>>
>>  1) A null-dereference if the pipe->dl is committed at the same time as
>> the vsp1_video_setup_pipeline() is processing
>>
>>  2) A hardware hang, where a display list is committed without having
>> called vsp1_video_setup_pipeline() first
>>
>> Along side these race conditions, the work done by
>> vsp1_video_setup_pipeline() is undone by the re-initialisation during a
>> suspend resume cycle, and an active pipeline does not attempt to
>> reconfigure the correct routing and init parameters for the entities.
>>
>> To repair all of these issues, we can move the call to a conditional
>> inside vsp1_video_pipeline_run() and ensure that this can only be called
>> on the last stream which calls into vsp1_video_start_streaming()
>>
>> As a convenient side effect of this, by specifying that the
>> configuration has been lost during suspend/resume actions - the
>> vsp1_video_pipeline_run() call can re-initialise pipelines when
>> necessary thus repairing resume actions for active m2m pipelines.
>>
>> Signed-off-by: Kieran Bingham 
>>
>> ---
>> v3:
>>  - Move 'flag reset' to be inside the vsp1_reset_wpf() function call
>>  - Tidy up the wpf->pipe reference for the configured flag
>>
>>  drivers/media/platform/vsp1/vsp1_drv.c   |  4 
>>  drivers/media/platform/vsp1/vsp1_pipe.c  |  1 +
>>  drivers/media/platform/vsp1/vsp1_pipe.h  |  2 ++
>>  drivers/media/platform/vsp1/vsp1_video.c | 20 +---
>>  4 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
>> b/drivers/media/platform/vsp1/vsp1_drv.c index 57c713a4e1df..1dc3726c4e83
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_drv.c
>> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
>> @@ -413,6 +413,7 @@ static int vsp1_create_entities(struct vsp1_device
>> *vsp1)
>>
>>  int vsp1_reset_wpf(struct vsp1_device *vsp1, unsigned int index)
>>  {
>> +struct vsp1_rwpf *wpf = vsp1->wpf[index];
>>  unsigned int timeout;
>>  u32 status;
>>
>> @@ -429,6 +430,9 @@ int vsp1_reset_wpf(struct vsp1_device *vsp1, unsigned
>> int index) usleep_range(1000, 2000);
>>  }
>>
>> +if (wpf->pipe)
>> +wpf->pipe->configured = false;
>> +
>>  if (!timeout) {
>>  dev_err(vsp1->dev, "failed to reset wpf.%u\n", index);
>>  return -ETIMEDOUT;
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
>> b/drivers/media/platform/vsp1/vsp1_pipe.c index 756ca4ea7668..7ddf862ee403
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
>> @@ -208,6 +208,7 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe)
>>
>>  INIT_LIST_HEAD(>entities);
>>  pipe->state = VSP1_PIPELINE_STOPPED;
>> +pipe->configured = false;
>>  }
>>
>>  /* Must be called with the pipe irqlock held. */
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
>> b/drivers/media/platform/vsp1/vsp1_pipe.h index ac4ad261..0743b9fcb655
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
>> @@ -61,6 +61,7 @@ enum vsp1_pipeline_state {
>>   * @pipe: the media pipeline
>>   * @irqlock: protects the pipeline state
>>   * @state: current state
>> + * @configured: determines routing configuration active on cell.
> 
> I'm not sure to understand that. How about "true if the pipeline has been set 
> up" ? Or maybe "true if the pipeline has been set up for video streaming" as 
> it only applies to pipelines handled through the V4L2 API ?


Yes, Reading it now - I have no idea what context I was writing that in.
I hope it was late and I was tired ... otherwise I have no excuse :D



>>   * @wq: wait queue to wait for state change completion
>>   * @frame_end: frame end interrupt handler
>>   * @lock: protects the pipeline use count and stream count
>> @@ -86,6 +87,7 @@ struct vsp1_pipeline {
>>
>>  spinlock_t irqlock;
>>  enum vsp1_pipeline_state state;
>> +bool configured;
>>  wait_queue_head_t wq;
>>
>>  void (*frame_end)(struct vsp1_pipeline *pipe);
>> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
>> b/drivers/media/platform/vsp1/vsp1_video.c index 44b687c0b8df..7ff9f4c19ff0
>> 100644
>> --- 

Re: [PULL REQUEST] renesas/topic/sdhi-emmc-hs for renesas drivers

[Wolfram Sang]

> >   mmc: sh_mobile_sdhi: simplify accessing DT data
> >   mmc: sh_mobile_sdhi: improve prerequisite for hw_reset
> >   mmc: sh_mobile_sdhi: improve prerequisites for tuning
> >   mmc: sh_mobile_sdhi: remove superfluous check in hw_reset
> >   mmc: sh_mobile_sdhi: remove superfluous check in init_tuning
> >   mmc: sh_mobile_sdhi: remove superfluous check in SCC error check
> >   mmc: sh_mobile_sdhi: enable HS200
> 
> Please base this on Ulf's mmc/next, or on v4.10-rc1 once it becomes available.

Okay, thought so. Will wait for the latter.

Thanks,

   Wolfram



signature.asc
Description: PGP signature

Re: [PULL REQUEST] renesas/topic/sdhi-emmc-hs for renesas drivers

[Geert Uytterhoeven]

Hi Wolfram,

On Thu, Dec 15, 2016 at 11:42 AM, Wolfram Sang  wrote:
> here is a topic branch for renesas-drivers adding HS200 support to SDHI.
> It is based on the tag "renesas-drivers-2016-12-06-v4.9-rc8" because the
> feature is built on top of the SDR104 series by Simon which was merged
> upstream just recently in the merge window. Is this okay? Or do you
> prefer another branch based on v4.10-rc1 once it is available?

Please don't base pull requests on old renesas-drivers releases, if possible,
else I'll have to rebase your branch(es) myself.

>   mmc: sh_mobile_sdhi: simplify accessing DT data
>   mmc: sh_mobile_sdhi: improve prerequisite for hw_reset
>   mmc: sh_mobile_sdhi: improve prerequisites for tuning
>   mmc: sh_mobile_sdhi: remove superfluous check in hw_reset
>   mmc: sh_mobile_sdhi: remove superfluous check in init_tuning
>   mmc: sh_mobile_sdhi: remove superfluous check in SCC error check
>   mmc: sh_mobile_sdhi: enable HS200

Please base this on Ulf's mmc/next, or on v4.10-rc1 once it becomes available.

>   arm64: dts: r8a7795: enable HS200 for eMMC
>   arm64: dts: r8a7796: enable HS200 for eMMC

Please base this on an appropriate reasonable stable tree, e.g.
renesas-devel-20161212-v4.9.

Thank you!

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

[PULL REQUEST] renesas/topic/sdhi-emmc-hs for renesas drivers

[Wolfram Sang]

Hi Geert,

here is a topic branch for renesas-drivers adding HS200 support to SDHI.
It is based on the tag "renesas-drivers-2016-12-06-v4.9-rc8" because the
feature is built on top of the SDR104 series by Simon which was merged
upstream just recently in the merge window. Is this okay? Or do you
prefer another branch based on v4.10-rc1 once it is available?

Kind regards,

   Wolfram


The following changes since commit 2122ab4824fda8009d20b754f80eefa66e5b4b18:

  Merge branch 'r8a7795-hdmi-out-dts-v1' of git://github.com/uli/kernel into 
renesas-drivers (2016-12-06 14:18:10 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
renesas/topic/sdhi-emmc-hs

for you to fetch changes up to 9f9bc225f2ada33944f95d928a9a9190b7497e33:

  arm64: dts: r8a7796: enable HS200 for eMMC (2016-12-12 14:56:02 +0100)


Wolfram Sang (9):
  mmc: sh_mobile_sdhi: simplify accessing DT data
  mmc: sh_mobile_sdhi: improve prerequisite for hw_reset
  mmc: sh_mobile_sdhi: improve prerequisites for tuning
  mmc: sh_mobile_sdhi: remove superfluous check in hw_reset
  mmc: sh_mobile_sdhi: remove superfluous check in init_tuning
  mmc: sh_mobile_sdhi: remove superfluous check in SCC error check
  mmc: sh_mobile_sdhi: enable HS200
  arm64: dts: r8a7795: enable HS200 for eMMC
  arm64: dts: r8a7796: enable HS200 for eMMC

 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |  1 +
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |  1 +
 drivers/mmc/host/sh_mobile_sdhi.c  | 67 --
 3 files changed, 27 insertions(+), 42 deletions(-)


signature.asc
Description: PGP signature

Re: [PATCH] ARM: dts: r8a7791: link DU to VSPDs

[Magnus Damm]

Hi Sergei,

On Thu, Dec 15, 2016 at 7:07 AM, Sergei Shtylyov
 wrote:
> Add the "vsps" property to the DU device node in order to link this node to
> the VSPD nodes.
>
> Signed-off-by: Sergei Shtylyov 
>
> ---
> This patch is against the 'renesas-devel-20161212-v4.9' of Simon Horman's
> 'renesas.git' repo.  It's  only meaningful if the DU driver patch I've just
> posted is applied.

Next time, please consider pointing out the exact name of the patch
you are referring to.

Thanks,

/ magnus

Re: [RFC] New Device Tree property - "bonding"

[Geert Uytterhoeven]

On Tue, Dec 6, 2016 at 6:45 PM, Ramesh Shanmugasundaram
 wrote:
>> >> On Monday 05 Dec 2016 09:57:32 Rob Herring wrote:
>> >> > On Mon, Dec 5, 2016 at 8:40 AM, Laurent Pinchart wrote:
>> >> > > On Monday 05 Dec 2016 08:18:34 Rob Herring wrote:
>> >> > >> On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram wrote:
>> >> > >>> Hello DT maintainers,
>> >> > >>>
>> >> > >>> In one of the Renesas SoCs we have a device called DRIF
>> >> > >>> (Digital Radio
>> >> > >>> Interface) controller. A DRIF channel contains 4 external pins
>> >> > >>> - SCK, SYNC, Data pins D0 & D1.
>> >> > >>>
>> >> > >>> Internally a DRIF channel is made up of two SPI slave devices
>> >> > >>> (also called sub-channels here) that share common CLK & SYNC
>> >> > >>> signals but have their own resource set. The DRIF channel can
>> >> > >>> have either one of the sub-channel active at a time or both.
>> >> > >>> When both sub-channels are active, they need to be managed
>> >> > >>> together as one device as they share same CLK & SYNC. We plan
>> >> > >>> to tie these two sub-channels together with a new property called
>> "renesas,bonding".
>> >> > >>
>> >> > >> Is there no need to describe the master device? No GPIOs,
>> >> > >> regulators or other sideband controls needed? If that's never
>> >> > >> needed (which seems doubtful), then I would do something
>> >> > >> different here probably with the master device as a child of one
>> >> > >> DRIF and then phandles to master from the other DRIFs.
>> >> > >> Otherwise, this looks
>> >> fine to me.
>> >> > >
>> >> > > Here's a bit of background.
>> >> > >
>> >> > > The DRIF is an SPI receiver. It has three input pins, a clock
>> >> > > line, a data line and a sync signal. The device is designed to be
>> >> > > connected to a variety of data sources, usually plain SPI (1 data
>> >> > > line), IIS (1 data
>> >> > > line) but also radio tuners that output I/Q data
>> >> > > (http://www.ni.com/tutorial/4805/en/) over two data lines.
>> >> > >
>> >> > > In the case of IQ each data sample is split in two I and Q values
>> >> > > (typically 16 to 20 bits each in this case), and the values are
>> >> > > transmitted serially over one data line each. The synchronization
>> >> > > and clock signals are common to both data lines. The DRIF is
>> >> > > optimized for this use case as the DRIF instances in the SoC
>> >> > > (each of them having independent clocks, interrupts and control
>> >> > > registers) are grouped by two, and the two instances in a group
>> >> > > handle a single data line each but share the same clock and sync
>> input.
>> >> > >
>> >> > > On the software side we need to group the I and Q values, which
>> >> > > are DMA'ed to memory by the two DRIF instances, and make them
>> >> > > available to userspace. The V4L2 API used here in SDR (Software
>> >> > > Defined Radio) mode supports such use cases and exposes a single
>> >> > > device node to userspace that allows control of the two DRIF
>> >> > > instances as a single device. To be able to implement this we
>> >> > > need kernel code to be aware of DRIF groups and, while binding to
>> >> > > the DRIF instances separately, expose only one V4L2 device to
>> userspace for each group.
>> >> > >
>> >> > > There's no master or slave instance from a hardware point of
>> >> > > view, but the two instances are not interchangeable as they carry
>> >> > > separate
>> >> information.
>> >> > > They must thus be identified at the driver level.
>> >> >
>> >> > By master, I meant the external master device that generates the IQ
>> >> > data, not which of the internal DRIF blocks is a master of the other.
>> >> > So back to my question, does the external master device need to be
>> >> > described? I worry the answer now for a simple case is no, but then
>> >> > later people are going to have cases needing to describe more. We
>> >> > need to answer this question first before we can decide what this
>> >> > binding should look like.
>> >>
>> >> Oh yes the external device certainly needs to be described. As it is
>> >> controlled through a separate, general-purpose I2C or SPI controller,
>> >> it should be a child node of that controller. The DRIF handles the
>> >> data interface only, not the control interface of the external device.
>> >
>> > Yes, as Laurent mentioned, the external master will be described
>> separately. The data interface with the master is described through port
>> nodes. E.g.
>> >
>> > port {
>> > drif0_ep: endpoint {
>> >  remote-endpoint = <_ep>;
>> > };
>> > };
>> >
>> > Do we agree on this model please?
>>
>> Well, that's not complete as you should have both DRIF0 and DRIF1 having
>> connections to the tuner. Then you can walk the graph and find everything,
>> and you then don't need the bonding property.
>
> Assuming the third party tuner exposes it's two data lines as two endpoints, 
> it seems possible with 

Re: [PATCH] clk: renesas: mstp: Support 8-bit registers for r7s72100

[Geert Uytterhoeven]

Hi Chris,

On Thu, Dec 15, 2016 at 3:58 AM, Chris Brandt  wrote:
> On Dec 14, 2016, Kuninori Morimoto wrote:
>> I don't think using global variable is a good idea.
>> For example, how about add reg_width_8bit into mstp_clock_group, and
>> cpg_mstp_read/write requests it, or something like that ?
>
> If I make a separate CLK_OF_DECLARE like this:
>
> static void __init cpg_mstp_clocks_init8(struct device_node *np) {
> reg_width_8bit = true;
> cpg_mstp_clocks_init(np);
> }
> CLK_OF_DECLARE(cpg_mstp_clks8, "renesas,r7s72100-mstp-clocks",
>cpg_mstp_clocks_init8);
>
>
> The only way I can pass the 8-bit/32-bit choice into cpg_mstp_clocks_init()
> is using a global variable.
>
>
> Unless, I change the API to cpg_mstp_clocks_init(np, reg_width_8bit)
>
> But, that means adding another function:
>
> CLK_OF_DECLARE(cpg_mstp_clks32, "renesas,cpg-mstp-clocks",
>cpg_mstp_clocks_init32);
>
> CLK_OF_DECLARE(cpg_mstp_clks8, "renesas,r7s72100-mstp-clocks",
>cpg_mstp_clocks_init8);
>
> static void __init cpg_mstp_clocks_init32(struct device_node *np)
> {
> cpg_mstp_clocks_init(np, false);
> }
> static void __init cpg_mstp_clocks_init8(struct device_node *np)
> {
> cpg_mstp_clocks_init(np, true);
> }

Or you can add an of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")
check to cpg_mstp_clocks_init(), keeping the single CLK_OF_DECLARE().

I think that would also performs slightly better, as only nodes compatible
with "renesas,cpg-mstp-clocks" would be subject to the additional check.

> A global variable is much easier, but if no one likes it, I can change to
> mstp_clock_group.reg_width_8bit instead.

Personally, I have no problem with the global variable: either you're running
on RZ/A1, and all MSTP clocks need 8-bit accesses, or you're not.

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