[PATCH net v2 0/3] gre: fix lwtunnel support

[PATCH net v2 0/3] gre: fix lwtunnel support

[Jiri Benc]

This patchset fixes a few bugs in gre metadata mode implementation, mainly
with ipgre.

As an example, in this setup:

ip a a 192.168.1.1/24 dev eth0
ip l a gre1 type gre external
ip l s gre1 up
ip a a 192.168.99.1/24 dev gre1
ip r a 192.168.99.2/32 encap ip dst 192.168.1.2 ttl 10 dev gre1
ping 192.168.99.2

the traffic does not go through before this patchset and does as expected
with it applied.

v2: Rejecting invalid configuration, added patch 3, dropped patch for
    ETH_P_TEB (will target net-next).

Jiri Benc (3):
  gre: do not assign header_ops in collect metadata mode
  gre: build header correctly for collect metadata tunnels
  gre: allow creation of gretap interfaces in metadata mode

 net/ipv4/ip_gre.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH net v2 1/3] gre: do not assign header_ops in collect metadata mode

[Jiri Benc]

In ipgre mode (i.e. not gretap) with collect metadata flag set, the tunnel
is incorrectly assumed to be mGRE in NBMA mode (see commit 6a5f44d7a048c).
This is not the case, we're controlling the encapsulation addresses by
lwtunnel metadata. And anyway, assigning dev->header_ops in collect metadata
mode does not make sense.

Similarly, when a multicast remote IP address is set together with the
collect metadata flag, the processing described above would happen, too. As
there's not much sense in specifying remote/local IP address for lwtunnels,
reject such configuration.

v2: Reject configuration specifying both remote/local address and collect
    metadata flag.

Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 net/ipv4/ip_gre.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index af5d1f38217f..c035b43b1d4b 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -902,8 +902,9 @@ static int ipgre_tunnel_init(struct net_device *dev)
 			dev->header_ops = &ipgre_header_ops;
 		}
 #endif
-	} else
+	} else if (!tunnel->collect_md) {
 		dev->header_ops = &ipgre_header_ops;
+	}
 
 	return ip_tunnel_init(dev);
 }
@@ -946,6 +947,15 @@ static int ipgre_tunnel_validate(struct nlattr *tb[], struct nlattr *data[])
 	if (flags & (GRE_VERSION|GRE_ROUTING))
 		return -EINVAL;
 
+	if (data[IFLA_GRE_COLLECT_METADATA]) {
+		if (data[IFLA_GRE_REMOTE] &&
+		    nla_get_in_addr(data[IFLA_GRE_REMOTE]))
+			return -EINVAL;
+		if (data[IFLA_GRE_LOCAL] &&
+		    nla_get_in_addr(data[IFLA_GRE_LOCAL]))
+			return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
1.8.3.1

Re: [PATCH net v2 1/3] gre: do not assign header_ops in collect metadata mode

[Sergei Shtylyov]

Hello.

On 4/24/2016 2:00 PM, Jiri Benc wrote:

> In ipgre mode (i.e. not gretap) with collect metadata flag set, the tunnel
> is incorrectly assumed to be mGRE in NBMA mode (see commit 6a5f44d7a048c).

    Didn't checkpatch.pl complain about the commit citing style?

> This is not the case, we're controlling the encapsulation addresses by
> lwtunnel metadata. And anyway, assigning dev->header_ops in collect metadata
> mode does not make sense.
>
> Similarly, when a multicast remote IP address is set together with the
> collect metadata flag, the processing described above would happen, too. As
> there's not much sense in specifying remote/local IP address for lwtunnels,
> reject such configuration.
>
> v2: Reject configuration specifying both remote/local address and collect
>      metadata flag.
>
> Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
[...]

MBR, Sergei

Re: [PATCH net v2 1/3] gre: do not assign header_ops in collect metadata mode

[Jiri Benc]

On Sun, 24 Apr 2016 13:00:20 +0200, Jiri Benc wrote:
> In ipgre mode (i.e. not gretap) with collect metadata flag set, the tunnel
> is incorrectly assumed to be mGRE in NBMA mode (see commit 6a5f44d7a048c).
> This is not the case, we're controlling the encapsulation addresses by
> lwtunnel metadata. And anyway, assigning dev->header_ops in collect metadata
> mode does not make sense.
> 
> Similarly, when a multicast remote IP address is set together with the
> collect metadata flag, the processing described above would happen, too. As
> there's not much sense in specifying remote/local IP address for lwtunnels,
> reject such configuration.
> 
> v2: Reject configuration specifying both remote/local address and collect
>     metadata flag.

Thinking about this more, this *is* uAPI breakage and we can't do that.
It affects also gretap and current users of gretap lwtunnels would be
broken. This is even more likely as it's not possible to create gretap
lwtunnel using iproute2 without specifying a fake remote address.

In theory, we could wrap the current ipgre_tunnel_validate with a new
function that would be set as ipgre_link_ops->validate but that would
add unnecessary complexity and would make ipgre and gretap behavior
different.

I'm sorry, we have to live with what we have. This should have been
taken into account when you implemented lwtunnels for GRE, we can't
change this now, it's too late.

I'll send v3 with this patch being restored to exactly what I had in v1.

 Jiri

[PATCH net v2 2/3] gre: build header correctly for collect metadata tunnels

[Jiri Benc]

In ipgre (i.e. not gretap) + collect metadata mode, the skb was assumed to
contain Ethernet header and was encapsulated as ETH_P_TEB. This is not the
case, the interface is ARPHRD_IPGRE and the protocol to be used for
encapsulation is skb->protocol.

Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
v2: unchanged
---
 net/ipv4/ip_gre.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index c035b43b1d4b..02b12db59437 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -523,7 +523,8 @@ static struct rtable *gre_get_rt(struct sk_buff *skb,
 	return ip_route_output_key(net, fl);
 }
 
-static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev)
+static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev,
+			__be16 proto)
 {
 	struct ip_tunnel_info *tun_info;
 	const struct ip_tunnel_key *key;
@@ -575,7 +576,7 @@ static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	flags = tun_info->key.tun_flags & (TUNNEL_CSUM | TUNNEL_KEY);
-	build_header(skb, tunnel_hlen, flags, htons(ETH_P_TEB),
+	build_header(skb, tunnel_hlen, flags, proto,
 		     tunnel_id_to_key(tun_info->key.tun_id), 0);
 
 	df = key->tun_flags & TUNNEL_DONT_FRAGMENT ?  htons(IP_DF) : 0;
@@ -616,7 +617,7 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
 	const struct iphdr *tnl_params;
 
 	if (tunnel->collect_md) {
-		gre_fb_xmit(skb, dev);
+		gre_fb_xmit(skb, dev, skb->protocol);
 		return NETDEV_TX_OK;
 	}
 
@@ -660,7 +661,7 @@ static netdev_tx_t gre_tap_xmit(struct sk_buff *skb,
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 
 	if (tunnel->collect_md) {
-		gre_fb_xmit(skb, dev);
+		gre_fb_xmit(skb, dev, htons(ETH_P_TEB));
 		return NETDEV_TX_OK;
 	}
 
-- 
1.8.3.1

[PATCH net v2 3/3] gre: allow creation of gretap interfaces in metadata mode

[Jiri Benc]

The IFLA_GRE_REMOTE attribute does not make sense together with collect
metadata and is ignored in such case. However, iproute2 always sets it; it
will be zero if there's no remote address specified on the command line.

Remove the check for non-zero IFLA_GRE_REMOTE when collect medata flag is
set.

Before the patch, this command returns failure, after the patch, it works as
expected:

ip link add gre1 type gretap external

Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
New in v2.
---
 net/ipv4/ip_gre.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 02b12db59437..27716c72b8e2 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -974,7 +974,7 @@ static int ipgre_tap_validate(struct nlattr *tb[], struct nlattr *data[])
 	if (!data)
 		goto out;
 
-	if (data[IFLA_GRE_REMOTE]) {
+	if (data[IFLA_GRE_REMOTE] && !data[IFLA_GRE_COLLECT_METADATA]) {
 		memcpy(&daddr, nla_data(data[IFLA_GRE_REMOTE]), 4);
 		if (!daddr)
 			return -EINVAL;
-- 
1.8.3.1

Re: [PATCH net v2 3/3] gre: allow creation of gretap interfaces in metadata mode

[pravin shelar]

On Sun, Apr 24, 2016 at 4:00 AM, Jiri Benc <jbenc@redhat.com> wrote:
> The IFLA_GRE_REMOTE attribute does not make sense together with collect
> metadata and is ignored in such case. However, iproute2 always sets it; it
> will be zero if there's no remote address specified on the command line.
>
> Remove the check for non-zero IFLA_GRE_REMOTE when collect medata flag is
> set.
>
Rather than cover up in ip_gre kernel module, why not just fix
iproute2 to set the attribute correctly?


> Before the patch, this command returns failure, after the patch, it works as
> expected:
>
> ip link add gre1 type gretap external
>
> Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
> New in v2.
> ---

>  net/ipv4/ip_gre.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Re: [PATCH net v2 3/3] gre: allow creation of gretap interfaces in metadata mode

[Jiri Benc]

On Mon, 25 Apr 2016 11:00:35 -0700, pravin shelar wrote:
> On Sun, Apr 24, 2016 at 4:00 AM, Jiri Benc <jbenc@redhat.com> wrote:
> > The IFLA_GRE_REMOTE attribute does not make sense together with collect
> > metadata and is ignored in such case. However, iproute2 always sets it; it
> > will be zero if there's no remote address specified on the command line.
> >
> > Remove the check for non-zero IFLA_GRE_REMOTE when collect medata flag is
> > set.
> >
> Rather than cover up in ip_gre kernel module, why not just fix
> iproute2 to set the attribute correctly?

Well, who defines "correctly"? :-)

I can do that, it really doesn't matter. One way or the other, the
result will be the same. I'll move this to iproute2 in v3.

 Jiri

[PATCH iproute2 0/2] ip link gre: fix external mode handling

[Jiri Benc]

Fix two bugs with handling of the 'external' keyword for GRE.

Jiri Benc (2):
  ip link gre: create interfaces in external mode correctly
  ip link gre: print only relevant info in external mode

 ip/link_gre.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

-- 
1.8.3.1

[PATCH iproute2 1/2] ip link gre: create interfaces in external mode correctly

[Jiri Benc]

For GRE interfaces in 'external' mode, the kernel ignores all manual
settings like remote IP address or TTL. However, for some of those
attributes, kernel checks their value and does not allow them to be zero
(even though they're ignored later).

Currently, 'ip link' always includes all attributes in the netlink message.
This leads to problem with creating interfaces in 'external' mode. For
example, this command does not work:

ip link add gre1 type gretap external

and needs a bogus remote IP address to be specified, as the kernel enforces
remote IP address to be either not present, or not null.

Ignore the parameters that do not make sense in 'external' mode.
Unfortunately, we cannot error out, as there may be existing deployments
that workarounded the bug by specifying bogus values.

Fixes: 926b39e1feffd ("gre: add support for collect metadata flag")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 ip/link_gre.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index bcf003aaa5d7..36ce1252675b 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -315,24 +315,26 @@ get_failed:
 		return -1;
 	}
 
-	addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
-	addattr32(n, 1024, IFLA_GRE_OKEY, okey);
-	addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
-	addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
-	addattr_l(n, 1024, IFLA_GRE_LOCAL, &saddr, 4);
-	addattr_l(n, 1024, IFLA_GRE_REMOTE, &daddr, 4);
-	addattr_l(n, 1024, IFLA_GRE_PMTUDISC, &pmtudisc, 1);
-	if (link)
-		addattr32(n, 1024, IFLA_GRE_LINK, link);
-	addattr_l(n, 1024, IFLA_GRE_TTL, &ttl, 1);
-	addattr_l(n, 1024, IFLA_GRE_TOS, &tos, 1);
+	if (!metadata) {
+		addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
+		addattr32(n, 1024, IFLA_GRE_OKEY, okey);
+		addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
+		addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
+		addattr_l(n, 1024, IFLA_GRE_LOCAL, &saddr, 4);
+		addattr_l(n, 1024, IFLA_GRE_REMOTE, &daddr, 4);
+		addattr_l(n, 1024, IFLA_GRE_PMTUDISC, &pmtudisc, 1);
+		if (link)
+			addattr32(n, 1024, IFLA_GRE_LINK, link);
+		addattr_l(n, 1024, IFLA_GRE_TTL, &ttl, 1);
+		addattr_l(n, 1024, IFLA_GRE_TOS, &tos, 1);
+	} else {
+		addattr_l(n, 1024, IFLA_GRE_COLLECT_METADATA, NULL, 0);
+	}
 
 	addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
 	addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
 	addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
 	addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
-	if (metadata)
-		addattr_l(n, 1024, IFLA_GRE_COLLECT_METADATA, NULL, 0);
 
 	return 0;
 }
-- 
1.8.3.1

[PATCH iproute2 2/2] ip link gre: print only relevant info in external mode

[Jiri Benc]

Display only attributes that are relevant when a GRE interface is in
'external' mode instead of the default values (which are ignored by the
kernel even if passed back).

Fixes: 926b39e1feffd ("gre: add support for collect metadata flag")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 ip/link_gre.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 36ce1252675b..492c22053b89 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -339,7 +339,7 @@ get_failed:
 	return 0;
 }
 
-static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
 	const char *local = "any";
@@ -347,9 +347,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 
-	if (!tb)
-		return;
-
 	if (tb[IFLA_GRE_REMOTE]) {
 		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_REMOTE]);
 
@@ -421,8 +418,16 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		fputs("icsum ", f);
 	if (oflags & GRE_CSUM)
 		fputs("ocsum ", f);
+}
 
-	if (tb[IFLA_GRE_COLLECT_METADATA])
+static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+{
+	if (!tb)
+		return;
+
+	if (!tb[IFLA_GRE_COLLECT_METADATA])
+		gre_print_direct_opt(f, tb);
+	else
 		fputs("external ", f);
 
 	if (tb[IFLA_GRE_ENCAP_TYPE] &&
-- 
1.8.3.1

Re: [PATCH iproute2 0/2] ip link gre: fix external mode handling

[Stephen Hemminger]

On Wed, 27 Apr 2016 16:11:12 +0200
Jiri Benc <jbenc@redhat.com> wrote:

> Fix two bugs with handling of the 'external' keyword for GRE.
> 
> Jiri Benc (2):
>   ip link gre: create interfaces in external mode correctly
>   ip link gre: print only relevant info in external mode
> 
>  ip/link_gre.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
> 

Applied

[GIT PULL] Renesas ARM Based SoC R-Car SYSC Updates for v4.7

[Simon Horman]

Hi Olof, Hi Kevin, Hi Arnd,

Please consider these Renesas ARM based SoC R-Car SYSC updates for v4.7.


The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca:

  Linux 4.6-rc1 (2016-03-26 16:03:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-rcar-sysc-for-v4.7

for you to fetch changes up to 839a04d847f5871516f500091519c52dc40fe01d:

  soc: renesas: Add r8a7795 SYSC PM Domain Binding Definitions (2016-04-15 11:01:58 +1000)

----------------------------------------------------------------
Renesas ARM Based SoC R-Car SYSC Updates for v4.7

* Add DT bindings for the R-Car System Controller.
  An implementation is intended to follow.

----------------------------------------------------------------
Geert Uytterhoeven (7):
      PM / Domains: Add DT bindings for the R-Car System Controller
      soc: renesas: Add r8a7779 SYSC PM Domain Binding Definitions
      soc: renesas: Add r8a7790 SYSC PM Domain Binding Definitions
      soc: renesas: Add r8a7791 SYSC PM Domain Binding Definitions
      soc: renesas: Add r8a7793 SYSC PM Domain Binding Definitions
      soc: renesas: Add r8a7794 SYSC PM Domain Binding Definitions
      soc: renesas: Add r8a7795 SYSC PM Domain Binding Definitions

 .../bindings/power/renesas,rcar-sysc.txt           | 48 ++++++++++++++++++++++
 include/dt-bindings/power/r8a7779-sysc.h           | 27 ++++++++++++
 include/dt-bindings/power/r8a7790-sysc.h           | 34 +++++++++++++++
 include/dt-bindings/power/r8a7791-sysc.h           | 26 ++++++++++++
 include/dt-bindings/power/r8a7793-sysc.h           | 28 +++++++++++++
 include/dt-bindings/power/r8a7794-sysc.h           | 26 ++++++++++++
 include/dt-bindings/power/r8a7795-sysc.h           | 42 +++++++++++++++++++
 7 files changed, 231 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
 create mode 100644 include/dt-bindings/power/r8a7779-sysc.h
 create mode 100644 include/dt-bindings/power/r8a7790-sysc.h
 create mode 100644 include/dt-bindings/power/r8a7791-sysc.h
 create mode 100644 include/dt-bindings/power/r8a7793-sysc.h
 create mode 100644 include/dt-bindings/power/r8a7794-sysc.h
 create mode 100644 include/dt-bindings/power/r8a7795-sysc.h

[PATCH 1/7] PM / Domains: Add DT bindings for the R-Car System Controller

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

The Renesas R-Car System Controller provides power management for the
CPU cores and various coprocessors, following the generic PM domain
bindings in Documentation/devicetree/bindings/power/power_domain.txt.

This supports R-Car Gen1 (H1), Gen2, and Gen3.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 .../bindings/power/renesas,rcar-sysc.txt           | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt

diff --git a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
new file mode 100644
index 000000000000..b74e4d4785ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
@@ -0,0 +1,48 @@
+DT bindings for the Renesas R-Car System Controller
+
+== System Controller Node ==
+
+The R-Car System Controller provides power management for the CPU cores and
+various coprocessors.
+
+Required properties:
+  - compatible: Must contain exactly one of the following:
+      - "renesas,r8a7779-sysc" (R-Car H1)
+      - "renesas,r8a7790-sysc" (R-Car H2)
+      - "renesas,r8a7791-sysc" (R-Car M2-W)
+      - "renesas,r8a7792-sysc" (R-Car V2H)
+      - "renesas,r8a7793-sysc" (R-Car M2-N)
+      - "renesas,r8a7794-sysc" (R-Car E2)
+      - "renesas,r8a7795-sysc" (R-Car H3)
+  - reg: Address start and address range for the device.
+  - #power-domain-cells: Must be 1.
+
+
+Example:
+
+	sysc: system-controller@e6180000 {
+		compatible = "renesas,r8a7791-sysc";
+		reg = <0 0xe6180000 0 0x0200>;
+		#power-domain-cells = <1>;
+	};
+
+
+== PM Domain Consumers ==
+
+Devices residing in a power area must refer to that power area, as documented
+by the generic PM domain bindings in
+Documentation/devicetree/bindings/power/power_domain.txt.
+
+Required properties:
+  - power-domains: A phandle and symbolic PM domain specifier, as defined in
+		   <dt-bindings/power/r8a77*-sysc.h>.
+
+
+Example:
+
+	L2_CA15: cache-controller@0 {
+		compatible = "cache";
+		power-domains = <&sysc R8A7791_PD_CA15_SCU>;
+		cache-unified;
+		cache-level = <2>;
+	};
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 1/7] PM / Domains: Add DT bindings for the R-Car System Controller

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

The Renesas R-Car System Controller provides power management for the
CPU cores and various coprocessors, following the generic PM domain
bindings in Documentation/devicetree/bindings/power/power_domain.txt.

This supports R-Car Gen1 (H1), Gen2, and Gen3.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 .../bindings/power/renesas,rcar-sysc.txt           | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt

diff --git a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
new file mode 100644
index 000000000000..b74e4d4785ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
@@ -0,0 +1,48 @@
+DT bindings for the Renesas R-Car System Controller
+
+== System Controller Node ==
+
+The R-Car System Controller provides power management for the CPU cores and
+various coprocessors.
+
+Required properties:
+  - compatible: Must contain exactly one of the following:
+      - "renesas,r8a7779-sysc" (R-Car H1)
+      - "renesas,r8a7790-sysc" (R-Car H2)
+      - "renesas,r8a7791-sysc" (R-Car M2-W)
+      - "renesas,r8a7792-sysc" (R-Car V2H)
+      - "renesas,r8a7793-sysc" (R-Car M2-N)
+      - "renesas,r8a7794-sysc" (R-Car E2)
+      - "renesas,r8a7795-sysc" (R-Car H3)
+  - reg: Address start and address range for the device.
+  - #power-domain-cells: Must be 1.
+
+
+Example:
+
+	sysc: system-controller at e6180000 {
+		compatible = "renesas,r8a7791-sysc";
+		reg = <0 0xe6180000 0 0x0200>;
+		#power-domain-cells = <1>;
+	};
+
+
+== PM Domain Consumers ==
+
+Devices residing in a power area must refer to that power area, as documented
+by the generic PM domain bindings in
+Documentation/devicetree/bindings/power/power_domain.txt.
+
+Required properties:
+  - power-domains: A phandle and symbolic PM domain specifier, as defined in
+		   <dt-bindings/power/r8a77*-sysc.h>.
+
+
+Example:
+
+	L2_CA15: cache-controller at 0 {
+		compatible = "cache";
+		power-domains = <&sysc R8A7791_PD_CA15_SCU>;
+		cache-unified;
+		cache-level = <2>;
+	};
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 2/7] soc: renesas: Add r8a7779 SYSC PM Domain Binding Definitions

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7779-sysc.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7779-sysc.h

diff --git a/include/dt-bindings/power/r8a7779-sysc.h b/include/dt-bindings/power/r8a7779-sysc.h
new file mode 100644
index 000000000000..183571da507e
--- /dev/null
+++ b/include/dt-bindings/power/r8a7779-sysc.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7779_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7779_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7779_PD_ARM1			 1
+#define R8A7779_PD_ARM2			 2
+#define R8A7779_PD_ARM3			 3
+#define R8A7779_PD_SGX			20
+#define R8A7779_PD_VDP			21
+#define R8A7779_PD_IMP			24
+
+/* Always-on power area */
+#define R8A7779_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7779_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 2/7] soc: renesas: Add r8a7779 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7779-sysc.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7779-sysc.h

diff --git a/include/dt-bindings/power/r8a7779-sysc.h b/include/dt-bindings/power/r8a7779-sysc.h
new file mode 100644
index 000000000000..183571da507e
--- /dev/null
+++ b/include/dt-bindings/power/r8a7779-sysc.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7779_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7779_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7779_PD_ARM1			 1
+#define R8A7779_PD_ARM2			 2
+#define R8A7779_PD_ARM3			 3
+#define R8A7779_PD_SGX			20
+#define R8A7779_PD_VDP			21
+#define R8A7779_PD_IMP			24
+
+/* Always-on power area */
+#define R8A7779_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7779_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 3/7] soc: renesas: Add r8a7790 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7790-sysc.h | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7790-sysc.h

diff --git a/include/dt-bindings/power/r8a7790-sysc.h b/include/dt-bindings/power/r8a7790-sysc.h
new file mode 100644
index 000000000000..6af4e9929bd0
--- /dev/null
+++ b/include/dt-bindings/power/r8a7790-sysc.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7790_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7790_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7790_PD_CA15_CPU0		 0
+#define R8A7790_PD_CA15_CPU1		 1
+#define R8A7790_PD_CA15_CPU2		 2
+#define R8A7790_PD_CA15_CPU3		 3
+#define R8A7790_PD_CA7_CPU0		 5
+#define R8A7790_PD_CA7_CPU1		 6
+#define R8A7790_PD_CA7_CPU2		 7
+#define R8A7790_PD_CA7_CPU3		 8
+#define R8A7790_PD_CA15_SCU		12
+#define R8A7790_PD_SH_4A		16
+#define R8A7790_PD_RGX			20
+#define R8A7790_PD_CA7_SCU		21
+#define R8A7790_PD_IMP			24
+
+/* Always-on power area */
+#define R8A7790_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7790_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 4/7] soc: renesas: Add r8a7791 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7791-sysc.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7791-sysc.h

diff --git a/include/dt-bindings/power/r8a7791-sysc.h b/include/dt-bindings/power/r8a7791-sysc.h
new file mode 100644
index 000000000000..1403baa0514f
--- /dev/null
+++ b/include/dt-bindings/power/r8a7791-sysc.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7791_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7791_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7791_PD_CA15_CPU0		 0
+#define R8A7791_PD_CA15_CPU1		 1
+#define R8A7791_PD_CA15_SCU		12
+#define R8A7791_PD_SH_4A		16
+#define R8A7791_PD_SGX			20
+
+/* Always-on power area */
+#define R8A7791_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7791_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 4/7] soc: renesas: Add r8a7791 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7791-sysc.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7791-sysc.h

diff --git a/include/dt-bindings/power/r8a7791-sysc.h b/include/dt-bindings/power/r8a7791-sysc.h
new file mode 100644
index 000000000000..1403baa0514f
--- /dev/null
+++ b/include/dt-bindings/power/r8a7791-sysc.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7791_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7791_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7791_PD_CA15_CPU0		 0
+#define R8A7791_PD_CA15_CPU1		 1
+#define R8A7791_PD_CA15_SCU		12
+#define R8A7791_PD_SH_4A		16
+#define R8A7791_PD_SGX			20
+
+/* Always-on power area */
+#define R8A7791_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7791_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 5/7] soc: renesas: Add r8a7793 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

R-Car M2-N is identical to R-Car M2-W w.r.t. power domains, so reuse the
definitions from the latter.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7793-sysc.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7793-sysc.h

diff --git a/include/dt-bindings/power/r8a7793-sysc.h b/include/dt-bindings/power/r8a7793-sysc.h
new file mode 100644
index 000000000000..b5693df3d830
--- /dev/null
+++ b/include/dt-bindings/power/r8a7793-sysc.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7793_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7793_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ *
+ * Note that R-Car M2-N is identical to R-Car M2-W w.r.t. power domains.
+ */
+
+#define R8A7793_PD_CA15_CPU0		 0
+#define R8A7793_PD_CA15_CPU1		 1
+#define R8A7793_PD_CA15_SCU		12
+#define R8A7793_PD_SH_4A		16
+#define R8A7793_PD_SGX			20
+
+/* Always-on power area */
+#define R8A7793_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7793_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 5/7] soc: renesas: Add r8a7793 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

R-Car M2-N is identical to R-Car M2-W w.r.t. power domains, so reuse the
definitions from the latter.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7793-sysc.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7793-sysc.h

diff --git a/include/dt-bindings/power/r8a7793-sysc.h b/include/dt-bindings/power/r8a7793-sysc.h
new file mode 100644
index 000000000000..b5693df3d830
--- /dev/null
+++ b/include/dt-bindings/power/r8a7793-sysc.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7793_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7793_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ *
+ * Note that R-Car M2-N is identical to R-Car M2-W w.r.t. power domains.
+ */
+
+#define R8A7793_PD_CA15_CPU0		 0
+#define R8A7793_PD_CA15_CPU1		 1
+#define R8A7793_PD_CA15_SCU		12
+#define R8A7793_PD_SH_4A		16
+#define R8A7793_PD_SGX			20
+
+/* Always-on power area */
+#define R8A7793_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7793_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 6/7] soc: renesas: Add r8a7794 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7794-sysc.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7794-sysc.h

diff --git a/include/dt-bindings/power/r8a7794-sysc.h b/include/dt-bindings/power/r8a7794-sysc.h
new file mode 100644
index 000000000000..862241c2d27b
--- /dev/null
+++ b/include/dt-bindings/power/r8a7794-sysc.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7794_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7794_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7794_PD_CA7_CPU0		 5
+#define R8A7794_PD_CA7_CPU1		 6
+#define R8A7794_PD_SH_4A		16
+#define R8A7794_PD_SGX			20
+#define R8A7794_PD_CA7_SCU		21
+
+/* Always-on power area */
+#define R8A7794_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7794_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 7/7] soc: renesas: Add r8a7795 SYSC PM Domain Binding Definitions

[Simon Horman]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7795-sysc.h | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7795-sysc.h

diff --git a/include/dt-bindings/power/r8a7795-sysc.h b/include/dt-bindings/power/r8a7795-sysc.h
new file mode 100644
index 000000000000..ee2e26ba605e
--- /dev/null
+++ b/include/dt-bindings/power/r8a7795-sysc.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7795_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7795_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7795_PD_CA57_CPU0		 0
+#define R8A7795_PD_CA57_CPU1		 1
+#define R8A7795_PD_CA57_CPU2		 2
+#define R8A7795_PD_CA57_CPU3		 3
+#define R8A7795_PD_CA53_CPU0		 5
+#define R8A7795_PD_CA53_CPU1		 6
+#define R8A7795_PD_CA53_CPU2		 7
+#define R8A7795_PD_CA53_CPU3		 8
+#define R8A7795_PD_A3VP			 9
+#define R8A7795_PD_CA57_SCU		12
+#define R8A7795_PD_CR7			13
+#define R8A7795_PD_A3VC			14
+#define R8A7795_PD_3DG_A		17
+#define R8A7795_PD_3DG_B		18
+#define R8A7795_PD_3DG_C		19
+#define R8A7795_PD_3DG_D		20
+#define R8A7795_PD_CA53_SCU		21
+#define R8A7795_PD_3DG_E		22
+#define R8A7795_PD_A3IR			24
+#define R8A7795_PD_A2VC0		25
+#define R8A7795_PD_A2VC1		26
+
+/* Always-on power area */
+#define R8A7795_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7795_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

[GIT PULL] Renesas ARM Based SoC R-Car SYSC Updates for v4.7

[Arnd Bergmann]

On Thursday 21 April 2016 13:44:48 Simon Horman wrote:
> Renesas ARM Based SoC R-Car SYSC Updates for v4.7
> 
> * Add DT bindings for the R-Car System Controller.
>   An implementation is intended to follow.
> 

This seems to best fit into next/dt, but we can also stick it into other
branches if it's needed as a dependency.

Pulled now, thanks!

	Arnd

[GIT PULL] Renesas ARM Based SoC R-Car SYSC Updates for v4.7

[Simon Horman]

On Sun, Apr 24, 2016 at 11:40:47PM +0200, Arnd Bergmann wrote:
> On Thursday 21 April 2016 13:44:48 Simon Horman wrote:
> > Renesas ARM Based SoC R-Car SYSC Updates for v4.7
> > 
> > * Add DT bindings for the R-Car System Controller.
> >   An implementation is intended to follow.
> > 
> 
> This seems to best fit into next/dt, but we can also stick it into other
> branches if it's needed as a dependency.

Hi Arnd,

Unfortunately this is more complex than I expected when I prepared this
pull request though the pull request itself is still correct.

There are driver changes that seem to logically fit on top of this pull
request. Though strictly speaking I don't believe there are any hard
run-time dependencies.

Then there are DT changes to enable the new driver that have
hard run-time dependencies on the driver changes themselves.

I have queued things up as described above and you can see them in the
renesas-next-20160422v3-v4.6-rc1 tag of next branch of the renesas tree.

I am planning to send corresponding pull requests in the near future.
I would be very happy to hear any feedback you have before I take that
step.

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

clk_get on a disabled clock node will return EPROBE_DEFER, which can
cause drivers to be deferred forever if such clocks are referenced in
their clocks property.

Update the various disabled external clock nodes to default to a
frequency of 0, but don't disable them to prevent this.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

 arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
 arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 1adf877..da59c28 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -660,6 +660,7 @@
 };
 
 &pcie_bus_clk {
+	clock-frequency = <100000000>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index 9554d13..19b257e 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -413,6 +413,7 @@
 };
 
 &pcie_bus_clk {
+	clock-frequency = <100000000>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 8693888..676df63 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -1104,8 +1104,7 @@
 		pcie_bus_clk: pcie_bus {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
-			clock-frequency = <100000000>;
-			status = "disabled";
+			clock-frequency = <0>;
 		};
 
 		/* External SCIF clock */
@@ -1114,7 +1113,6 @@
 			#clock-cells = <0>;
 			/* This value must be overridden by the board. */
 			clock-frequency = <0>;
-			status = "disabled";
 		};
 
 		/* External USB clock - can be overridden by the board */
@@ -1130,7 +1128,6 @@
 			#clock-cells = <0>;
 			/* This value must be overridden by the board. */
 			clock-frequency = <0>;
-			status = "disabled";
 		};
 
 		/* Special CPG clocks */
-- 
2.8.0.rc3

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

clk_get on a disabled clock node will return EPROBE_DEFER, which can
cause drivers to be deferred forever if such clocks are referenced in
their clocks property.

Update the various disabled external clock nodes to default to a
frequency of 0, but don't disable them to prevent this.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

 arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
 arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 1adf877..da59c28 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -660,6 +660,7 @@
 };
 
 &pcie_bus_clk {
+	clock-frequency = <100000000>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index 9554d13..19b257e 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -413,6 +413,7 @@
 };
 
 &pcie_bus_clk {
+	clock-frequency = <100000000>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 8693888..676df63 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -1104,8 +1104,7 @@
 		pcie_bus_clk: pcie_bus {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
-			clock-frequency = <100000000>;
-			status = "disabled";
+			clock-frequency = <0>;
 		};
 
 		/* External SCIF clock */
@@ -1114,7 +1113,6 @@
 			#clock-cells = <0>;
 			/* This value must be overridden by the board. */
 			clock-frequency = <0>;
-			status = "disabled";
 		};
 
 		/* External USB clock - can be overridden by the board */
@@ -1130,7 +1128,6 @@
 			#clock-cells = <0>;
 			/* This value must be overridden by the board. */
 			clock-frequency = <0>;
-			status = "disabled";
 		};
 
 		/* Special CPG clocks */
-- 
2.8.0.rc3

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk (this time with the new Mike)

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk (this time with the new Mike)

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

CC Mike, Stephen, linux-clk (this time with the new Mike)

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.

Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore disabled DT
clock providers")?

> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>  };
>
>  &pcie_bus_clk {
> +       clock-frequency = <100000000>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>                 pcie_bus_clk: pcie_bus {
>                         compatible = "fixed-clock";
>                         #clock-cells = <0>;
> -                       clock-frequency = <100000000>;
> -                       status = "disabled";
> +                       clock-frequency = <0>;
>                 };
>
>                 /* External SCIF clock */
> @@ -1114,7 +1113,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* External USB clock - can be overridden by the board */
> @@ -1130,7 +1128,6 @@
>                         #clock-cells = <0>;
>                         /* This value must be overridden by the board. */
>                         clock-frequency = <0>;
> -                       status = "disabled";
>                 };
>
>                 /* Special CPG clocks */
> --
> 2.8.0.rc3

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> CC Mike, Stephen, linux-clk (this time with the new Mike)
> 
> On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > 
> > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > can
> > cause drivers to be deferred forever if such clocks are referenced
> > in
> > their clocks property.
> Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> disabled DT
> clock providers")?

Yes it seems so. Reverting that patch means that i can drop this one
and get the expected behaviour again.

Though even so I'm not sure what the convention is for clocks like
these, the r8a7791.dtsi is inconsistent, as some are disabled while
others (e.g. the audio clocks) are 0hz. Would be good to get some input
on that regardless.

> > 
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > 
> > ---
> > 
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >  3 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index 1adf877..da59c28 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -660,6 +660,7 @@
> >  };
> > 
> >  &pcie_bus_clk {
> > +       clock-frequency = <100000000>;
> >         status = "okay";
> >  };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> > b/arch/arm/boot/dts/r8a7791-porter.dts
> > index 9554d13..19b257e 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -413,6 +413,7 @@
> >  };
> > 
> >  &pcie_bus_clk {
> > +       clock-frequency = <100000000>;
> >         status = "okay";
> >  };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> > b/arch/arm/boot/dts/r8a7791.dtsi
> > index 8693888..676df63 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -1104,8 +1104,7 @@
> >                 pcie_bus_clk: pcie_bus {
> >                         compatible = "fixed-clock";
> >                         #clock-cells = <0>;
> > -                       clock-frequency = <100000000>;
> > -                       status = "disabled";
> > +                       clock-frequency = <0>;
> >                 };
> > 
> >                 /* External SCIF clock */
> > @@ -1114,7 +1113,6 @@
> >                         #clock-cells = <0>;
> >                         /* This value must be overridden by the
> > board. */
> >                         clock-frequency = <0>;
> > -                       status = "disabled";
> >                 };
> > 
> >                 /* External USB clock - can be overridden by the
> > board */
> > @@ -1130,7 +1128,6 @@
> >                         #clock-cells = <0>;
> >                         /* This value must be overridden by the
> > board. */
> >                         clock-frequency = <0>;
> > -                       status = "disabled";
> >                 };
> > 
> >                 /* Special CPG clocks */
> > --
> > 2.8.0.rc3
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linu
> x-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

-- 
Sjoerd Simons
Collabora Ltd.

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> CC Mike, Stephen, linux-clk (this time with the new Mike)
> 
> On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > 
> > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > can
> > cause drivers to be deferred forever if such clocks are referenced
> > in
> > their clocks property.
> Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> disabled DT
> clock providers")?

Yes it seems so. Reverting that patch means that i can drop this one
and get the expected behaviour again.

Though even so I'm not sure what the convention is for clocks like
these, the r8a7791.dtsi is inconsistent, as some are disabled while
others (e.g. the audio clocks) are 0hz. Would be good to get some input
on that regardless.

> > 
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent?this.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > 
> > ---
> > 
> > ?arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> > ?arch/arm/boot/dts/r8a7791-porter.dts??| 1 +
> > ?arch/arm/boot/dts/r8a7791.dtsi????????| 5 +----
> > ?3 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index 1adf877..da59c28 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -660,6 +660,7 @@
> > ?};
> > 
> > ?&pcie_bus_clk {
> > +???????clock-frequency = <100000000>;
> > ????????status = "okay";
> > ?};
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> > b/arch/arm/boot/dts/r8a7791-porter.dts
> > index 9554d13..19b257e 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -413,6 +413,7 @@
> > ?};
> > 
> > ?&pcie_bus_clk {
> > +???????clock-frequency = <100000000>;
> > ????????status = "okay";
> > ?};
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> > b/arch/arm/boot/dts/r8a7791.dtsi
> > index 8693888..676df63 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -1104,8 +1104,7 @@
> > ????????????????pcie_bus_clk: pcie_bus {
> > ????????????????????????compatible = "fixed-clock";
> > ????????????????????????#clock-cells = <0>;
> > -???????????????????????clock-frequency = <100000000>;
> > -???????????????????????status = "disabled";
> > +???????????????????????clock-frequency = <0>;
> > ????????????????};
> > 
> > ????????????????/* External SCIF clock */
> > @@ -1114,7 +1113,6 @@
> > ????????????????????????#clock-cells = <0>;
> > ????????????????????????/* This value must be overridden by the
> > board. */
> > ????????????????????????clock-frequency = <0>;
> > -???????????????????????status = "disabled";
> > ????????????????};
> > 
> > ????????????????/* External USB clock - can be overridden by the
> > board */
> > @@ -1130,7 +1128,6 @@
> > ????????????????????????#clock-cells = <0>;
> > ????????????????????????/* This value must be overridden by the
> > board. */
> > ????????????????????????clock-frequency = <0>;
> > -???????????????????????status = "disabled";
> > ????????????????};
> > 
> > ????????????????/* Special CPG clocks */
> > --
> > 2.8.0.rc3
> Gr{oetje,eeting}s,
> 
> ????????????????????????Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linu
> x-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

-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

T24gV2VkLCAyMDE2LTA0LTA2IGF0IDE1OjExICswMjAwLCBHZWVydCBVeXR0ZXJob2V2ZW4gd3Jv
dGU6Cj4gQ0MgTWlrZSwgU3RlcGhlbiwgbGludXgtY2xrICh0aGlzIHRpbWUgd2l0aCB0aGUgbmV3
IE1pa2UpCj4gCj4gT24gV2VkLCBBcHIgNiwgMjAxNiBhdCAyOjUyIFBNLCBTam9lcmQgU2ltb25z
Cj4gPHNqb2VyZC5zaW1vbnNAY29sbGFib3JhLmNvLnVrPiB3cm90ZToKPiA+IAo+ID4gY2xrX2dl
dCBvbiBhIGRpc2FibGVkIGNsb2NrIG5vZGUgd2lsbCByZXR1cm4gRVBST0JFX0RFRkVSLCB3aGlj
aAo+ID4gY2FuCj4gPiBjYXVzZSBkcml2ZXJzIHRvIGJlIGRlZmVycmVkIGZvcmV2ZXIgaWYgc3Vj
aCBjbG9ja3MgYXJlIHJlZmVyZW5jZWQKPiA+IGluCj4gPiB0aGVpciBjbG9ja3MgcHJvcGVydHku
Cj4gSXMgdGhpcyBhIHNpZGUgZWZmZWN0IG9mIGNvbW1pdCAzZTVkZDZmNmU2OTAwNDhkICgiY2xr
OiBJZ25vcmUKPiBkaXNhYmxlZCBEVAo+IGNsb2NrIHByb3ZpZGVycyIpPwoKWWVzIGl0IHNlZW1z
IHNvLiBSZXZlcnRpbmcgdGhhdCBwYXRjaCBtZWFucyB0aGF0IGkgY2FuIGRyb3AgdGhpcyBvbmUK
YW5kIGdldCB0aGUgZXhwZWN0ZWQgYmVoYXZpb3VyIGFnYWluLgoKVGhvdWdoIGV2ZW4gc28gSSdt
IG5vdCBzdXJlIHdoYXQgdGhlIGNvbnZlbnRpb24gaXMgZm9yIGNsb2NrcyBsaWtlCnRoZXNlLCB0
aGUgcjhhNzc5MS5kdHNpIGlzIGluY29uc2lzdGVudCwgYXMgc29tZSBhcmUgZGlzYWJsZWQgd2hp
bGUKb3RoZXJzIChlLmcuIHRoZSBhdWRpbyBjbG9ja3MpIGFyZSAwaHouIFdvdWxkIGJlIGdvb2Qg
dG8gZ2V0IHNvbWUgaW5wdXQKb24gdGhhdCByZWdhcmRsZXNzLgoKPiA+IAo+ID4gVXBkYXRlIHRo
ZSB2YXJpb3VzIGRpc2FibGVkIGV4dGVybmFsIGNsb2NrIG5vZGVzIHRvIGRlZmF1bHQgdG8gYQo+
ID4gZnJlcXVlbmN5IG9mIDAsIGJ1dCBkb24ndCBkaXNhYmxlIHRoZW0gdG8gcHJldmVudMKgdGhp
cy4KPiA+IAo+ID4gU2lnbmVkLW9mZi1ieTogU2pvZXJkIFNpbW9ucyA8c2pvZXJkLnNpbW9uc0Bj
b2xsYWJvcmEuY28udWs+Cj4gPiAKPiA+IC0tLQo+ID4gCj4gPiDCoGFyY2gvYXJtL2Jvb3QvZHRz
L3I4YTc3OTEta29lbHNjaC5kdHMgfCAxICsKPiA+IMKgYXJjaC9hcm0vYm9vdC9kdHMvcjhhNzc5
MS1wb3J0ZXIuZHRzwqDCoHwgMSArCj4gPiDCoGFyY2gvYXJtL2Jvb3QvZHRzL3I4YTc3OTEuZHRz
acKgwqDCoMKgwqDCoMKgwqB8IDUgKy0tLS0KPiA+IMKgMyBmaWxlcyBjaGFuZ2VkLCAzIGluc2Vy
dGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9i
b290L2R0cy9yOGE3NzkxLWtvZWxzY2guZHRzCj4gPiBiL2FyY2gvYXJtL2Jvb3QvZHRzL3I4YTc3
OTEta29lbHNjaC5kdHMKPiA+IGluZGV4IDFhZGY4NzcuLmRhNTljMjggMTAwNjQ0Cj4gPiAtLS0g
YS9hcmNoL2FybS9ib290L2R0cy9yOGE3NzkxLWtvZWxzY2guZHRzCj4gPiArKysgYi9hcmNoL2Fy
bS9ib290L2R0cy9yOGE3NzkxLWtvZWxzY2guZHRzCj4gPiBAQCAtNjYwLDYgKzY2MCw3IEBACj4g
PiDCoH07Cj4gPiAKPiA+IMKgJnBjaWVfYnVzX2NsayB7Cj4gPiArwqDCoMKgwqDCoMKgwqBjbG9j
ay1mcmVxdWVuY3kgPSA8MTAwMDAwMDAwPjsKPiA+IMKgwqDCoMKgwqDCoMKgwqBzdGF0dXMgPSAi
b2theSI7Cj4gPiDCoH07Cj4gPiAKPiA+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9ib290L2R0cy9y
OGE3NzkxLXBvcnRlci5kdHMKPiA+IGIvYXJjaC9hcm0vYm9vdC9kdHMvcjhhNzc5MS1wb3J0ZXIu
ZHRzCj4gPiBpbmRleCA5NTU0ZDEzLi4xOWIyNTdlIDEwMDY0NAo+ID4gLS0tIGEvYXJjaC9hcm0v
Ym9vdC9kdHMvcjhhNzc5MS1wb3J0ZXIuZHRzCj4gPiArKysgYi9hcmNoL2FybS9ib290L2R0cy9y
OGE3NzkxLXBvcnRlci5kdHMKPiA+IEBAIC00MTMsNiArNDEzLDcgQEAKPiA+IMKgfTsKPiA+IAo+
ID4gwqAmcGNpZV9idXNfY2xrIHsKPiA+ICvCoMKgwqDCoMKgwqDCoGNsb2NrLWZyZXF1ZW5jeSA9
IDwxMDAwMDAwMDA+Owo+ID4gwqDCoMKgwqDCoMKgwqDCoHN0YXR1cyA9ICJva2F5IjsKPiA+IMKg
fTsKPiA+IAo+ID4gZGlmZiAtLWdpdCBhL2FyY2gvYXJtL2Jvb3QvZHRzL3I4YTc3OTEuZHRzaQo+
ID4gYi9hcmNoL2FybS9ib290L2R0cy9yOGE3NzkxLmR0c2kKPiA+IGluZGV4IDg2OTM4ODguLjY3
NmRmNjMgMTAwNjQ0Cj4gPiAtLS0gYS9hcmNoL2FybS9ib290L2R0cy9yOGE3NzkxLmR0c2kKPiA+
ICsrKyBiL2FyY2gvYXJtL2Jvb3QvZHRzL3I4YTc3OTEuZHRzaQo+ID4gQEAgLTExMDQsOCArMTEw
NCw3IEBACj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHBjaWVfYnVzX2Nsazog
cGNpZV9idXMgewo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgY29tcGF0aWJsZSA9ICJmaXhlZC1jbG9jayI7Cj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAjY2xvY2stY2VsbHMgPSA8MD47Cj4gPiAtwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGNsb2NrLWZyZXF1ZW5j
eSA9IDwxMDAwMDAwMDA+Owo+ID4gLcKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqBzdGF0dXMgPSAiZGlzYWJsZWQiOwo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBjbG9jay1mcmVxdWVuY3kgPSA8MD47Cj4gPiDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoH07Cj4gPiAKPiA+IMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgLyogRXh0ZXJuYWwgU0NJRiBjbG9jayAqLwo+ID4gQEAgLTExMTQsNyAr
MTExMyw2IEBACj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqAjY2xvY2stY2VsbHMgPSA8MD47Cj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqAvKiBUaGlzIHZhbHVlIG11c3QgYmUgb3ZlcnJpZGRlbiBieSB0
aGUKPiA+IGJvYXJkLiAqLwo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgY2xvY2stZnJlcXVlbmN5ID0gPDA+Owo+ID4gLcKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBzdGF0dXMgPSAiZGlzYWJsZWQiOwo+ID4gwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB9Owo+ID4gCj4gPiDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoC8qIEV4dGVybmFsIFVTQiBjbG9jayAtIGNhbiBiZSBvdmVycmlkZGVu
IGJ5IHRoZQo+ID4gYm9hcmQgKi8KPiA+IEBAIC0xMTMwLDcgKzExMjgsNiBAQAo+ID4gwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgI2Nsb2NrLWNlbGxzID0g
PDA+Owo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
LyogVGhpcyB2YWx1ZSBtdXN0IGJlIG92ZXJyaWRkZW4gYnkgdGhlCj4gPiBib2FyZC4gKi8KPiA+
IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGNsb2NrLWZy
ZXF1ZW5jeSA9IDwwPjsKPiA+IC3CoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgc3RhdHVzID0gImRpc2FibGVkIjsKPiA+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgfTsKPiA+IAo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAvKiBT
cGVjaWFsIENQRyBjbG9ja3MgKi8KPiA+IC0tCj4gPiAyLjguMC5yYzMKPiBHcntvZXRqZSxlZXRp
bmd9cywKPiAKPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqBHZWVydAo+IAo+IC0tCj4gR2VlcnQgVXl0dGVyaG9ldmVuIC0tIFRoZXJlJ3MgbG90cyBvZiBM
aW51eCBiZXlvbmQgaWEzMiAtLSBnZWVydEBsaW51Cj4geC1tNjhrLm9yZwo+IAo+IEluIHBlcnNv
bmFsIGNvbnZlcnNhdGlvbnMgd2l0aCB0ZWNobmljYWwgcGVvcGxlLCBJIGNhbGwgbXlzZWxmIGEK
PiBoYWNrZXIuIEJ1dAo+IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlzdHMgSSBqdXN0IHNh
eSAicHJvZ3JhbW1lciIgb3Igc29tZXRoaW5nCj4gbGlrZSB0aGF0Lgo+IMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAtLSBMaW51
cyBUb3J2YWxkcwoKLS0gClNqb2VyZCBTaW1vbnMKQ29sbGFib3JhIEx0ZC4K

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> CC Mike, Stephen, linux-clk (this time with the new Mike)
> 
> On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > 
> > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > can
> > cause drivers to be deferred forever if such clocks are referenced
> > in
> > their clocks property.
> Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> disabled DT
> clock providers")?

Yes it seems so. Reverting that patch means that i can drop this one
and get the expected behaviour again.

Though even so I'm not sure what the convention is for clocks like
these, the r8a7791.dtsi is inconsistent, as some are disabled while
others (e.g. the audio clocks) are 0hz. Would be good to get some input
on that regardless.

> > 
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > 
> > ---
> > 
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >  arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >  3 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index 1adf877..da59c28 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -660,6 +660,7 @@
> >  };
> > 
> >  &pcie_bus_clk {
> > +       clock-frequency = <100000000>;
> >         status = "okay";
> >  };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> > b/arch/arm/boot/dts/r8a7791-porter.dts
> > index 9554d13..19b257e 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -413,6 +413,7 @@
> >  };
> > 
> >  &pcie_bus_clk {
> > +       clock-frequency = <100000000>;
> >         status = "okay";
> >  };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> > b/arch/arm/boot/dts/r8a7791.dtsi
> > index 8693888..676df63 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -1104,8 +1104,7 @@
> >                 pcie_bus_clk: pcie_bus {
> >                         compatible = "fixed-clock";
> >                         #clock-cells = <0>;
> > -                       clock-frequency = <100000000>;
> > -                       status = "disabled";
> > +                       clock-frequency = <0>;
> >                 };
> > 
> >                 /* External SCIF clock */
> > @@ -1114,7 +1113,6 @@
> >                         #clock-cells = <0>;
> >                         /* This value must be overridden by the
> > board. */
> >                         clock-frequency = <0>;
> > -                       status = "disabled";
> >                 };
> > 
> >                 /* External USB clock - can be overridden by the
> > board */
> > @@ -1130,7 +1128,6 @@
> >                         #clock-cells = <0>;
> >                         /* This value must be overridden by the
> > board. */
> >                         clock-frequency = <0>;
> > -                       status = "disabled";
> >                 };
> > 
> >                 /* Special CPG clocks */
> > --
> > 2.8.0.rc3
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linu
> x-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

-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/06, Sjoerd Simons wrote:
> On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > 
> > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > <sjoerd.simons@collabora.co.uk> wrote:
> > > 
> > > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > > can
> > > cause drivers to be deferred forever if such clocks are referenced
> > > in
> > > their clocks property.
> > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > disabled DT
> > clock providers")?
> 
> Yes it seems so. Reverting that patch means that i can drop this one
> and get the expected behaviour again.

The DT is broken then? Is it possible to mark these status =
"okay" so that things work again?

> 
> Though even so I'm not sure what the convention is for clocks like
> these, the r8a7791.dtsi is inconsistent, as some are disabled while
> others (e.g. the audio clocks) are 0hz. Would be good to get some input
> on that regardless.
> 

What's the question here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/06, Sjoerd Simons wrote:
> On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > 
> > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > <sjoerd.simons@collabora.co.uk> wrote:
> > > 
> > > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > > can
> > > cause drivers to be deferred forever if such clocks are referenced
> > > in
> > > their clocks property.
> > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > disabled DT
> > clock providers")?
> 
> Yes it seems so. Reverting that patch means that i can drop this one
> and get the expected behaviour again.

The DT is broken then? Is it possible to mark these status =
"okay" so that things work again?

> 
> Though even so I'm not sure what the convention is for clocks like
> these, the r8a7791.dtsi is inconsistent, as some are disabled while
> others (e.g. the audio clocks) are 0hz. Would be good to get some input
> on that regardless.
> 

What's the question here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/06, Sjoerd Simons wrote:
> On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > 
> > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > <sjoerd.simons@collabora.co.uk> wrote:
> > > 
> > > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > > can
> > > cause drivers to be deferred forever if such clocks are referenced
> > > in
> > > their clocks property.
> > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > disabled DT
> > clock providers")?
> 
> Yes it seems so. Reverting that patch means that i can drop this one
> and get the expected behaviour again.

The DT is broken then? Is it possible to mark these status =
"okay" so that things work again?

> 
> Though even so I'm not sure what the convention is for clocks like
> these, the r8a7791.dtsi is inconsistent, as some are disabled while
> others (e.g. the audio clocks) are 0hz. Would be good to get some input
> on that regardless.
> 

What's the question here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> On 04/06, Sjoerd Simons wrote:
> > 
> > On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > > 
> > > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > > 
> > > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > > <sjoerd.simons@collabora.co.uk> wrote:
> > > > 
> > > > 
> > > > clk_get on a disabled clock node will return EPROBE_DEFER,
> > > > which
> > > > can
> > > > cause drivers to be deferred forever if such clocks are
> > > > referenced
> > > > in
> > > > their clocks property.
> > > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > > disabled DT
> > > clock providers")?
> > Yes it seems so. Reverting that patch means that i can drop this
> > one
> > and get the expected behaviour again.
> The DT is broken then? Is it possible to mark these status =
> "okay" so that things work again?

> > 
> > 
> > Though even so I'm not sure what the convention is for clocks like
> > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > input
> > on that regardless.
> > 
> What's the question here?

So the question is how to model unconnected external clocks in device-
tree.

The dtsi we're loooking at has (in pseudo dt):

device {
  clock-names = "internal", "external";
  clocks = <&internal, &external>
};

external {
  compatible = "fixed-clock";
  clock-frequency = <12345>;
  status = "disabled";
};

Before 3e5dd6f6e690048d ("clk: Ignore disabled DT clock providers")
this apparently worked. Afterwards drivers getting all the clocks would
fail to probe with -EPROBE_DEFER.

Judging by your comment I assume this way of modelling it is broken
(and the behaviour caused by the patch is correct)? 

And as a follow-up, is modelling unconnected clocks as enabled with a
frequency of 0hz as my proposed patch does seen as the right way of
doing things?


-- 
Sjoerd Simons
Collabora Ltd.

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> On 04/06, Sjoerd Simons wrote:
> > 
> > On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > > 
> > > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > > 
> > > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > > <sjoerd.simons@collabora.co.uk> wrote:
> > > > 
> > > > 
> > > > clk_get on a disabled clock node will return EPROBE_DEFER,
> > > > which
> > > > can
> > > > cause drivers to be deferred forever if such clocks are
> > > > referenced
> > > > in
> > > > their clocks property.
> > > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > > disabled DT
> > > clock providers")?
> > Yes it seems so. Reverting that patch means that i can drop this
> > one
> > and get the expected behaviour again.
> The DT is broken then? Is it possible to mark these status =
> "okay" so that things work again?

> > 
> > 
> > Though even so I'm not sure what the convention is for clocks like
> > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > input
> > on that regardless.
> > 
> What's the question here?

So the question is how to model unconnected external clocks in device-
tree.

The dtsi we're loooking at has (in pseudo dt):

device {
? clock-names = "internal", "external";
? clocks = <&internal, &external>
};

external {
? compatible = "fixed-clock";
? clock-frequency = <12345>;
? status = "disabled";
};

Before?3e5dd6f6e690048d ("clk: Ignore?disabled DT?clock providers")
this apparently worked. Afterwards drivers getting all the clocks would
fail to probe with -EPROBE_DEFER.

Judging by your comment I assume this way of modelling it is broken
(and the behaviour caused by the patch is correct)??

And as a follow-up, is modelling unconnected clocks as enabled with a
frequency of 0hz as my proposed patch does seen as the right way of
doing things?


-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> On 04/06, Sjoerd Simons wrote:
> >=20
> > On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > >=20
> > > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > >=20
> > > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > > <sjoerd.simons@collabora.co.uk> wrote:
> > > >=20
> > > >=20
> > > > clk_get on a disabled clock node will return EPROBE_DEFER,
> > > > which
> > > > can
> > > > cause drivers to be deferred forever if such clocks are
> > > > referenced
> > > > in
> > > > their clocks property.
> > > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > > disabled DT
> > > clock providers")?
> > Yes it seems so. Reverting that patch means that i can drop this
> > one
> > and get the expected behaviour again.
> The DT is broken then? Is it possible to mark these status =3D
> "okay" so that things work again?

> >=20
> >=20
> > Though even so I'm not sure what the convention is for clocks like
> > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > input
> > on that regardless.
> >=20
> What's the question here?

So the question is how to model unconnected external clocks in device-
tree.

The dtsi we're loooking at has (in pseudo dt):

device {
=C2=A0 clock-names =3D "internal", "external";
=C2=A0 clocks =3D <&internal, &external>
};

external {
=C2=A0 compatible =3D "fixed-clock";
=C2=A0 clock-frequency =3D <12345>;
=C2=A0 status =3D "disabled";
};

Before=C2=A03e5dd6f6e690048d ("clk: Ignore=C2=A0disabled DT=C2=A0clock prov=
iders")
this apparently worked. Afterwards drivers getting all the clocks would
fail to probe with -EPROBE_DEFER.

Judging by your comment I assume this way of modelling it is broken
(and the behaviour caused by the patch is correct)?=C2=A0

And as a follow-up, is modelling unconnected clocks as enabled with a
frequency of 0hz as my proposed patch does seen as the right way of
doing things?


--=20
Sjoerd Simons
Collabora Ltd.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> On 04/06, Sjoerd Simons wrote:
> > 
> > On Wed, 2016-04-06 at 15:11 +0200, Geert Uytterhoeven wrote:
> > > 
> > > CC Mike, Stephen, linux-clk (this time with the new Mike)
> > > 
> > > On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> > > <sjoerd.simons@collabora.co.uk> wrote:
> > > > 
> > > > 
> > > > clk_get on a disabled clock node will return EPROBE_DEFER,
> > > > which
> > > > can
> > > > cause drivers to be deferred forever if such clocks are
> > > > referenced
> > > > in
> > > > their clocks property.
> > > Is this a side effect of commit 3e5dd6f6e690048d ("clk: Ignore
> > > disabled DT
> > > clock providers")?
> > Yes it seems so. Reverting that patch means that i can drop this
> > one
> > and get the expected behaviour again.
> The DT is broken then? Is it possible to mark these status =
> "okay" so that things work again?

> > 
> > 
> > Though even so I'm not sure what the convention is for clocks like
> > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > input
> > on that regardless.
> > 
> What's the question here?

So the question is how to model unconnected external clocks in device-
tree.

The dtsi we're loooking at has (in pseudo dt):

device {
  clock-names = "internal", "external";
  clocks = <&internal, &external>
};

external {
  compatible = "fixed-clock";
  clock-frequency = <12345>;
  status = "disabled";
};

Before 3e5dd6f6e690048d ("clk: Ignore disabled DT clock providers")
this apparently worked. Afterwards drivers getting all the clocks would
fail to probe with -EPROBE_DEFER.

Judging by your comment I assume this way of modelling it is broken
(and the behaviour caused by the patch is correct)? 

And as a follow-up, is modelling unconnected clocks as enabled with a
frequency of 0hz as my proposed patch does seen as the right way of
doing things?


-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/08, Sjoerd Simons wrote:
> On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> > On 04/06, Sjoerd Simons wrote:
> > > 
> > > Though even so I'm not sure what the convention is for clocks like
> > > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > > input
> > > on that regardless.
> > > 
> > What's the question here?
> 
> So the question is how to model unconnected external clocks in device-
> tree.
> 
> The dtsi we're loooking at has (in pseudo dt):
> 
> device {
>   clock-names = "internal", "external";
>   clocks = <&internal, &external>
> };
> 
> external {
>   compatible = "fixed-clock";
>   clock-frequency = <12345>;
>   status = "disabled";
> };
> 
> Before 3e5dd6f6e690048d ("clk: Ignore disabled DT clock providers")
> this apparently worked. Afterwards drivers getting all the clocks would
> fail to probe with -EPROBE_DEFER.
> 
> Judging by your comment I assume this way of modelling it is broken
> (and the behaviour caused by the patch is correct)? 
> 
> And as a follow-up, is modelling unconnected clocks as enabled with a
> frequency of 0hz as my proposed patch does seen as the right way of
> doing things?
> 

Right. In the case where the external clk is populated, I imagine
there would be a DT node describing it with the clock-frequency
property if it's a fixed rate clk. If the clk is not populated on
the board, then we would have a "ground" clk node that has a
frequency of 0. This way, if we have some mux clk that is default
connected to the external clk but can switch to some internal clk
it isn't orphaned forever. This is actually a problem for orphan
clk deferral right now.

My head starts to spin when we consider something like expansion
boards that have clk pins on them though. Hopefully for things
like that, we can populate clks with DT overlays and then change
the root hierarchy of the clk tree by swapping out the "ground"
clk for some real clk on the expansion board. The usage of
strings to describe the clk tree is probably going to get us here
though. Fun!

If we can't do this DT design because of backwards compatibility
concerns, then perhaps we need to expand the core to return a
fixed rate of 0 clk whenever clk_get() is called on a provider
that's status = "disabled"? Here's an untested patch to show that
idea.

---8<----
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fb74dc1f7520..19c3777b1cea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3085,6 +3085,17 @@ int of_clk_parent_fill(struct device_node *np, const char **parents,
 }
 EXPORT_SYMBOL_GPL(of_clk_parent_fill);
 
+static void init_disabled_clk_provider(struct device_node *np)
+{
+	struct clk *clk;
+
+	clk = clk_register_fixed_rate(NULL, of_node_full_name(np), NULL, 0, 0);
+	if (IS_ERR(clk))
+		return;
+
+	of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
 struct clock_provider {
 	of_clk_init_cb_t clk_init_cb;
 	struct device_node *np;
@@ -3150,9 +3161,6 @@ void __init of_clk_init(const struct of_device_id *matches)
 	for_each_matching_node_and_match(np, matches, &match) {
 		struct clock_provider *parent;
 
-		if (!of_device_is_available(np))
-			continue;
-
 		parent = kzalloc(sizeof(*parent), GFP_KERNEL);
 		if (!parent) {
 			list_for_each_entry_safe(clk_provider, next,
@@ -3165,7 +3173,18 @@ void __init of_clk_init(const struct of_device_id *matches)
 			return;
 		}
 
-		parent->clk_init_cb = match->data;
+		/*
+		 * Sometimes DT nodes are status = "disabled" but they're used
+		 * by other clk providers. In that case we make the disabled
+		 * provider return fixed rate clks with a frequency of 0 so
+		 * that nothing is orphaned and drivers can still get all
+		 * their clks.
+		 */
+		if (!of_device_is_available(np)) {
+			parent->clk_init_cb = init_disabled_clk_provider;
+		} else {
+			parent->clk_init_cb = match->data;
+		}
 		parent->np = of_node_get(np);
 		list_add_tail(&parent->node, &clk_provider_list);
 	}
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/08, Sjoerd Simons wrote:
> On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> > On 04/06, Sjoerd Simons wrote:
> > > 
> > > Though even so I'm not sure what the convention is for clocks like
> > > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > > input
> > > on that regardless.
> > > 
> > What's the question here?
> 
> So the question is how to model unconnected external clocks in device-
> tree.
> 
> The dtsi we're loooking at has (in pseudo dt):
> 
> device {
> ? clock-names = "internal", "external";
> ? clocks = <&internal, &external>
> };
> 
> external {
> ? compatible = "fixed-clock";
> ? clock-frequency = <12345>;
> ? status = "disabled";
> };
> 
> Before?3e5dd6f6e690048d ("clk: Ignore?disabled DT?clock providers")
> this apparently worked. Afterwards drivers getting all the clocks would
> fail to probe with -EPROBE_DEFER.
> 
> Judging by your comment I assume this way of modelling it is broken
> (and the behaviour caused by the patch is correct)??
> 
> And as a follow-up, is modelling unconnected clocks as enabled with a
> frequency of 0hz as my proposed patch does seen as the right way of
> doing things?
> 

Right. In the case where the external clk is populated, I imagine
there would be a DT node describing it with the clock-frequency
property if it's a fixed rate clk. If the clk is not populated on
the board, then we would have a "ground" clk node that has a
frequency of 0. This way, if we have some mux clk that is default
connected to the external clk but can switch to some internal clk
it isn't orphaned forever. This is actually a problem for orphan
clk deferral right now.

My head starts to spin when we consider something like expansion
boards that have clk pins on them though. Hopefully for things
like that, we can populate clks with DT overlays and then change
the root hierarchy of the clk tree by swapping out the "ground"
clk for some real clk on the expansion board. The usage of
strings to describe the clk tree is probably going to get us here
though. Fun!

If we can't do this DT design because of backwards compatibility
concerns, then perhaps we need to expand the core to return a
fixed rate of 0 clk whenever clk_get() is called on a provider
that's status = "disabled"? Here's an untested patch to show that
idea.

---8<----
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fb74dc1f7520..19c3777b1cea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3085,6 +3085,17 @@ int of_clk_parent_fill(struct device_node *np, const char **parents,
 }
 EXPORT_SYMBOL_GPL(of_clk_parent_fill);
 
+static void init_disabled_clk_provider(struct device_node *np)
+{
+	struct clk *clk;
+
+	clk = clk_register_fixed_rate(NULL, of_node_full_name(np), NULL, 0, 0);
+	if (IS_ERR(clk))
+		return;
+
+	of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
 struct clock_provider {
 	of_clk_init_cb_t clk_init_cb;
 	struct device_node *np;
@@ -3150,9 +3161,6 @@ void __init of_clk_init(const struct of_device_id *matches)
 	for_each_matching_node_and_match(np, matches, &match) {
 		struct clock_provider *parent;
 
-		if (!of_device_is_available(np))
-			continue;
-
 		parent = kzalloc(sizeof(*parent), GFP_KERNEL);
 		if (!parent) {
 			list_for_each_entry_safe(clk_provider, next,
@@ -3165,7 +3173,18 @@ void __init of_clk_init(const struct of_device_id *matches)
 			return;
 		}
 
-		parent->clk_init_cb = match->data;
+		/*
+		 * Sometimes DT nodes are status = "disabled" but they're used
+		 * by other clk providers. In that case we make the disabled
+		 * provider return fixed rate clks with a frequency of 0 so
+		 * that nothing is orphaned and drivers can still get all
+		 * their clks.
+		 */
+		if (!of_device_is_available(np)) {
+			parent->clk_init_cb = init_disabled_clk_provider;
+		} else {
+			parent->clk_init_cb = match->data;
+		}
 		parent->np = of_node_get(np);
 		list_add_tail(&parent->node, &clk_provider_list);
 	}
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/08, Sjoerd Simons wrote:
> On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> > On 04/06, Sjoerd Simons wrote:
> > > 
> > > Though even so I'm not sure what the convention is for clocks like
> > > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > > input
> > > on that regardless.
> > > 
> > What's the question here?
> 
> So the question is how to model unconnected external clocks in device-
> tree.
> 
> The dtsi we're loooking at has (in pseudo dt):
> 
> device {
> � clock-names = "internal", "external";
> � clocks = <&internal, &external>
> };
> 
> external {
> � compatible = "fixed-clock";
> � clock-frequency = <12345>;
> � status = "disabled";
> };
> 
> Before�3e5dd6f6e690048d ("clk: Ignore�disabled DT�clock providers")
> this apparently worked. Afterwards drivers getting all the clocks would
> fail to probe with -EPROBE_DEFER.
> 
> Judging by your comment I assume this way of modelling it is broken
> (and the behaviour caused by the patch is correct)?�
> 
> And as a follow-up, is modelling unconnected clocks as enabled with a
> frequency of 0hz as my proposed patch does seen as the right way of
> doing things?
> 

Right. In the case where the external clk is populated, I imagine
there would be a DT node describing it with the clock-frequency
property if it's a fixed rate clk. If the clk is not populated on
the board, then we would have a "ground" clk node that has a
frequency of 0. This way, if we have some mux clk that is default
connected to the external clk but can switch to some internal clk
it isn't orphaned forever. This is actually a problem for orphan
clk deferral right now.

My head starts to spin when we consider something like expansion
boards that have clk pins on them though. Hopefully for things
like that, we can populate clks with DT overlays and then change
the root hierarchy of the clk tree by swapping out the "ground"
clk for some real clk on the expansion board. The usage of
strings to describe the clk tree is probably going to get us here
though. Fun!

If we can't do this DT design because of backwards compatibility
concerns, then perhaps we need to expand the core to return a
fixed rate of 0 clk whenever clk_get() is called on a provider
that's status = "disabled"? Here's an untested patch to show that
idea.

---8<----
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fb74dc1f7520..19c3777b1cea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3085,6 +3085,17 @@ int of_clk_parent_fill(struct device_node *np, const char **parents,
 }
 EXPORT_SYMBOL_GPL(of_clk_parent_fill);
 
+static void init_disabled_clk_provider(struct device_node *np)
+{
+	struct clk *clk;
+
+	clk = clk_register_fixed_rate(NULL, of_node_full_name(np), NULL, 0, 0);
+	if (IS_ERR(clk))
+		return;
+
+	of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
 struct clock_provider {
 	of_clk_init_cb_t clk_init_cb;
 	struct device_node *np;
@@ -3150,9 +3161,6 @@ void __init of_clk_init(const struct of_device_id *matches)
 	for_each_matching_node_and_match(np, matches, &match) {
 		struct clock_provider *parent;
 
-		if (!of_device_is_available(np))
-			continue;
-
 		parent = kzalloc(sizeof(*parent), GFP_KERNEL);
 		if (!parent) {
 			list_for_each_entry_safe(clk_provider, next,
@@ -3165,7 +3173,18 @@ void __init of_clk_init(const struct of_device_id *matches)
 			return;
 		}
 
-		parent->clk_init_cb = match->data;
+		/*
+		 * Sometimes DT nodes are status = "disabled" but they're used
+		 * by other clk providers. In that case we make the disabled
+		 * provider return fixed rate clks with a frequency of 0 so
+		 * that nothing is orphaned and drivers can still get all
+		 * their clks.
+		 */
+		if (!of_device_is_available(np)) {
+			parent->clk_init_cb = init_disabled_clk_provider;
+		} else {
+			parent->clk_init_cb = match->data;
+		}
 		parent->np = of_node_get(np);
 		list_add_tail(&parent->node, &clk_provider_list);
 	}
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Stephen Boyd]

On 04/08, Sjoerd Simons wrote:
> On Thu, 2016-04-07 at 16:21 -0700, Stephen Boyd wrote:
> > On 04/06, Sjoerd Simons wrote:
> > > 
> > > Though even so I'm not sure what the convention is for clocks like
> > > these, the r8a7791.dtsi is inconsistent, as some are disabled while
> > > others (e.g. the audio clocks) are 0hz. Would be good to get some
> > > input
> > > on that regardless.
> > > 
> > What's the question here?
> 
> So the question is how to model unconnected external clocks in device-
> tree.
> 
> The dtsi we're loooking at has (in pseudo dt):
> 
> device {
>   clock-names = "internal", "external";
>   clocks = <&internal, &external>
> };
> 
> external {
>   compatible = "fixed-clock";
>   clock-frequency = <12345>;
>   status = "disabled";
> };
> 
> Before 3e5dd6f6e690048d ("clk: Ignore disabled DT clock providers")
> this apparently worked. Afterwards drivers getting all the clocks would
> fail to probe with -EPROBE_DEFER.
> 
> Judging by your comment I assume this way of modelling it is broken
> (and the behaviour caused by the patch is correct)? 
> 
> And as a follow-up, is modelling unconnected clocks as enabled with a
> frequency of 0hz as my proposed patch does seen as the right way of
> doing things?
> 

Right. In the case where the external clk is populated, I imagine
there would be a DT node describing it with the clock-frequency
property if it's a fixed rate clk. If the clk is not populated on
the board, then we would have a "ground" clk node that has a
frequency of 0. This way, if we have some mux clk that is default
connected to the external clk but can switch to some internal clk
it isn't orphaned forever. This is actually a problem for orphan
clk deferral right now.

My head starts to spin when we consider something like expansion
boards that have clk pins on them though. Hopefully for things
like that, we can populate clks with DT overlays and then change
the root hierarchy of the clk tree by swapping out the "ground"
clk for some real clk on the expansion board. The usage of
strings to describe the clk tree is probably going to get us here
though. Fun!

If we can't do this DT design because of backwards compatibility
concerns, then perhaps we need to expand the core to return a
fixed rate of 0 clk whenever clk_get() is called on a provider
that's status = "disabled"? Here's an untested patch to show that
idea.

---8<----
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fb74dc1f7520..19c3777b1cea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3085,6 +3085,17 @@ int of_clk_parent_fill(struct device_node *np, const char **parents,
 }
 EXPORT_SYMBOL_GPL(of_clk_parent_fill);
 
+static void init_disabled_clk_provider(struct device_node *np)
+{
+	struct clk *clk;
+
+	clk = clk_register_fixed_rate(NULL, of_node_full_name(np), NULL, 0, 0);
+	if (IS_ERR(clk))
+		return;
+
+	of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
 struct clock_provider {
 	of_clk_init_cb_t clk_init_cb;
 	struct device_node *np;
@@ -3150,9 +3161,6 @@ void __init of_clk_init(const struct of_device_id *matches)
 	for_each_matching_node_and_match(np, matches, &match) {
 		struct clock_provider *parent;
 
-		if (!of_device_is_available(np))
-			continue;
-
 		parent = kzalloc(sizeof(*parent), GFP_KERNEL);
 		if (!parent) {
 			list_for_each_entry_safe(clk_provider, next,
@@ -3165,7 +3173,18 @@ void __init of_clk_init(const struct of_device_id *matches)
 			return;
 		}
 
-		parent->clk_init_cb = match->data;
+		/*
+		 * Sometimes DT nodes are status = "disabled" but they're used
+		 * by other clk providers. In that case we make the disabled
+		 * provider return fixed rate clks with a frequency of 0 so
+		 * that nothing is orphaned and drivers can still get all
+		 * their clks.
+		 */
+		if (!of_device_is_available(np)) {
+			parent->clk_init_cb = init_disabled_clk_provider;
+		} else {
+			parent->clk_init_cb = match->data;
+		}
 		parent->np = of_node_get(np);
 		list_add_tail(&parent->node, &clk_provider_list);
 	}
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sergei Shtylyov]

On 04/06/2016 03:52 PM, Sjoerd Simons wrote:

> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>   arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>   arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>   arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>   3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>   };
>
>   &pcie_bus_clk {
> +	clock-frequency = <100000000>;

    Hmmm, looking at the Koelsch schematics, I don't see this clock. :-/

>   	status = "okay";
>   };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>   };
>
>   &pcie_bus_clk {
> +	clock-frequency = <100000000>;
>   	status = "okay";
>   };
>

    Again, looking at the Porter schematics, I don't see this clock either. :-/

> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>   		pcie_bus_clk: pcie_bus {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
> -			clock-frequency = <100000000>;
> -			status = "disabled";
> +			clock-frequency = <0>;

    If the clock has a good default frequency, I don't think you need to 
remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be overridden).

[...]

MBR, Sergei

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sergei Shtylyov]

On 04/06/2016 03:52 PM, Sjoerd Simons wrote:

> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>   arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>   arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>   arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>   3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>   };
>
>   &pcie_bus_clk {
> +	clock-frequency = <100000000>;

    Hmmm, looking at the Koelsch schematics, I don't see this clock. :-/

>   	status = "okay";
>   };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>   };
>
>   &pcie_bus_clk {
> +	clock-frequency = <100000000>;
>   	status = "okay";
>   };
>

    Again, looking at the Porter schematics, I don't see this clock either. :-/

> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>   		pcie_bus_clk: pcie_bus {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
> -			clock-frequency = <100000000>;
> -			status = "disabled";
> +			clock-frequency = <0>;

    If the clock has a good default frequency, I don't think you need to 
remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be overridden).

[...]

MBR, Sergei

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

Hey Sergei,

Thanks for your review.

On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> 
> > 
> > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > can
> > cause drivers to be deferred forever if such clocks are referenced
> > in
> > their clocks property.
> > 
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > 
> > ---
> > 
> >   arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >   arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >   arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >   3 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index 1adf877..da59c28 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -660,6 +660,7 @@
> >   };
> > 
> >   &pcie_bus_clk {
> > +	clock-frequency = <100000000>;
>     Hmmm, looking at the Koelsch schematics, I don't see this clock.
> :-/

I don't have the schematics so i was simply keeping the current state.
I've added Phil Edworthy to the list as he was the one originally
enable the bus clk for Koelsh according to git. Hopefully he can
clarify :)

> >   	status = "okay";
> >   };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> > b/arch/arm/boot/dts/r8a7791-porter.dts
> > index 9554d13..19b257e 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -413,6 +413,7 @@
> >   };
> > 
> >   &pcie_bus_clk {
> > +	clock-frequency = <100000000>;
> >   	status = "okay";
> >   };
> > 
>     Again, looking at the Porter schematics, I don't see this clock
> either. :-/

You were the one enabling this clock for Porter ;) I don't have PCIE
hardware to test with on my porter board, might be worth if you could
disable this clock and see if PCI-E still fucntions as expected (maybe
in practise it just happens to prefer the internal clock?) ?

> > 
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> > b/arch/arm/boot/dts/r8a7791.dtsi
> > index 8693888..676df63 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -1104,8 +1104,7 @@
> >   		pcie_bus_clk: pcie_bus {
> >   			compatible = "fixed-clock";
> >   			#clock-cells = <0>;
> > -			clock-frequency = <100000000>;
> > -			status = "disabled";
> > +			clock-frequency = <0>;
>     If the clock has a good default frequency, I don't think you need
> to 
> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> overridden).

I did that as it was by default disabled, so if i do enable it but
don't drop the default frequency to 0 board swithout that clock will
suddenly have it added to their dtb.

For the usb external clock I didn't touch it as it was already enabled
by default with a proper frequency, so it wouldn't hit the issue i was
trying to fix here. But i agree, both looking at other Renesas dtsis
and how all other external clocks are done in this dtsi, this node is
odd.


-- 
Sjoerd Simons
Collabora Ltd.

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

Hey Sergei,

Thanks for your review.

On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> 
> > 
> > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > can
> > cause drivers to be deferred forever if such clocks are referenced
> > in
> > their clocks property.
> > 
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > 
> > ---
> > 
> > ? arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> > ? arch/arm/boot/dts/r8a7791-porter.dts??| 1 +
> > ? arch/arm/boot/dts/r8a7791.dtsi????????| 5 +----
> > ? 3 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index 1adf877..da59c28 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -660,6 +660,7 @@
> > ? };
> > 
> > ? &pcie_bus_clk {
> > +	clock-frequency = <100000000>;
> ????Hmmm, looking at the Koelsch schematics, I don't see this clock.
> :-/

I don't have the schematics so i was simply keeping the current state.
I've added Phil Edworthy to the list as he was the one originally
enable the bus clk for Koelsh according to git. Hopefully he can
clarify :)

> > ??	status = "okay";
> > ? };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> > b/arch/arm/boot/dts/r8a7791-porter.dts
> > index 9554d13..19b257e 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -413,6 +413,7 @@
> > ? };
> > 
> > ? &pcie_bus_clk {
> > +	clock-frequency = <100000000>;
> > ??	status = "okay";
> > ? };
> > 
> ????Again, looking at the Porter schematics, I don't see this clock
> either. :-/

You were the one enabling this clock for Porter ;) I don't have PCIE
hardware to test with on my porter board, might be worth if you could
disable this clock and see if PCI-E still fucntions as expected (maybe
in practise it just happens to prefer the internal clock?) ?

> > 
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> > b/arch/arm/boot/dts/r8a7791.dtsi
> > index 8693888..676df63 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -1104,8 +1104,7 @@
> > ??		pcie_bus_clk: pcie_bus {
> > ??			compatible = "fixed-clock";
> > ??			#clock-cells = <0>;
> > -			clock-frequency = <100000000>;
> > -			status = "disabled";
> > +			clock-frequency = <0>;
> ????If the clock has a good default frequency, I don't think you need
> to?
> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> overridden).

I did that as it was by default disabled, so if i do enable it but
don't drop the default frequency to 0 board swithout that clock will
suddenly have it added to their dtb.

For the usb external clock I didn't touch it as it was already enabled
by default with a proper frequency, so it wouldn't hit the issue i was
trying to fix here. But i agree, both looking at other Renesas dtsis
and how all other external clocks are done in this dtsi, this node is
odd.


-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sjoerd Simons]

Hey Sergei,

Thanks for your review.

On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> 
> > 
> > clk_get on a disabled clock node will return EPROBE_DEFER, which
> > can
> > cause drivers to be deferred forever if such clocks are referenced
> > in
> > their clocks property.
> > 
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> > 
> > ---
> > 
> >   arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >   arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >   arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >   3 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index 1adf877..da59c28 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -660,6 +660,7 @@
> >   };
> > 
> >   &pcie_bus_clk {
> > +	clock-frequency = <100000000>;
>     Hmmm, looking at the Koelsch schematics, I don't see this clock.
> :-/

I don't have the schematics so i was simply keeping the current state.
I've added Phil Edworthy to the list as he was the one originally
enable the bus clk for Koelsh according to git. Hopefully he can
clarify :)

> >   	status = "okay";
> >   };
> > 
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> > b/arch/arm/boot/dts/r8a7791-porter.dts
> > index 9554d13..19b257e 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -413,6 +413,7 @@
> >   };
> > 
> >   &pcie_bus_clk {
> > +	clock-frequency = <100000000>;
> >   	status = "okay";
> >   };
> > 
>     Again, looking at the Porter schematics, I don't see this clock
> either. :-/

You were the one enabling this clock for Porter ;) I don't have PCIE
hardware to test with on my porter board, might be worth if you could
disable this clock and see if PCI-E still fucntions as expected (maybe
in practise it just happens to prefer the internal clock?) ?

> > 
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> > b/arch/arm/boot/dts/r8a7791.dtsi
> > index 8693888..676df63 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -1104,8 +1104,7 @@
> >   		pcie_bus_clk: pcie_bus {
> >   			compatible = "fixed-clock";
> >   			#clock-cells = <0>;
> > -			clock-frequency = <100000000>;
> > -			status = "disabled";
> > +			clock-frequency = <0>;
>     If the clock has a good default frequency, I don't think you need
> to 
> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> overridden).

I did that as it was by default disabled, so if i do enable it but
don't drop the default frequency to 0 board swithout that clock will
suddenly have it added to their dtb.

For the usb external clock I didn't touch it as it was already enabled
by default with a proper frequency, so it wouldn't hit the issue i was
trying to fix here. But i agree, both looking at other Renesas dtsis
and how all other external clocks are done in this dtsi, this node is
odd.


-- 
Sjoerd Simons
Collabora Ltd.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sergei Shtylyov]

Hello.

On 04/07/2016 10:00 AM, Sjoerd Simons wrote:

> Hey Sergei,
>
> Thanks for your review.

    You're welcome. :-)

> On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
>> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
>>
>>>
>>> clk_get on a disabled clock node will return EPROBE_DEFER, which
>>> can
>>> cause drivers to be deferred forever if such clocks are referenced
>>> in
>>> their clocks property.
>>>
>>> Update the various disabled external clock nodes to default to a
>>> frequency of 0, but don't disable them to prevent this.
>>>
>>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>>>
>>> ---
>>>
>>>    arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>>>    arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>>>    arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>>>    3 files changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> b/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> index 1adf877..da59c28 100644
>>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> @@ -660,6 +660,7 @@
>>>    };
>>>
>>>    &pcie_bus_clk {
>>> +	clock-frequency = <100000000>;

>>      Hmmm, looking at the Koelsch schematics, I don't see this clock.
>> :-/
>
> I don't have the schematics so i was simply keeping the current state.

    I understand. I was just surprised by what checking the code against the 
schematics revealed.

> I've added Phil Edworthy to the list as he was the one originally

    I should've CC'ed Phil myself...

> enable the bus clk for Koelsh according to git. Hopefully he can
> clarify :)

    Let's hope he'd reply...

>>>    	status = "okay";
>>>    };
>>>
>>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
>>> b/arch/arm/boot/dts/r8a7791-porter.dts
>>> index 9554d13..19b257e 100644
>>> --- a/arch/arm/boot/dts/r8a7791-porter.dts
>>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
>>> @@ -413,6 +413,7 @@
>>>    };
>>>
>>>    &pcie_bus_clk {
>>> +	clock-frequency = <100000000>;
>>>    	status = "okay";
>>>    };
>>>
>>      Again, looking at the Porter schematics, I don't see this clock
>> either. :-/
>
> You were the one enabling this clock for Porter ;) I don't have PCIE

    Yes, of course. I don't remember the gory details already -- I might have 
blindly copied what was in the Koelsch's .dts.

> hardware to test with on my porter board, might be worth if you could
> disable this clock and see if PCI-E still fucntions as expected (maybe
> in practise it just happens to prefer the internal clock?) ?

    I know the PCIe driver does require this clock in order to function -- it 
would be the same eternal -EPROBE_DEFER condition you'd already described;
see drivers/pci/host/pcie-rcar.c yourself...

>>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi
>>> b/arch/arm/boot/dts/r8a7791.dtsi
>>> index 8693888..676df63 100644
>>> --- a/arch/arm/boot/dts/r8a7791.dtsi
>>> +++ b/arch/arm/boot/dts/r8a7791.dtsi
>>> @@ -1104,8 +1104,7 @@
>>>    		pcie_bus_clk: pcie_bus {
>>>    			compatible = "fixed-clock";
>>>    			#clock-cells = <0>;
>>> -			clock-frequency = <100000000>;
>>> -			status = "disabled";
>>> +			clock-frequency = <0>;
>>      If the clock has a good default frequency, I don't think you need
>> to
>> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
>> overridden).
>
> I did that as it was by default disabled, so if i do enable it but
> don't drop the default frequency to 0 board swithout that clock will
> suddenly have it added to their dtb.

    Zero frequency is hardly any better then the default...

> For the usb external clock I didn't touch it as it was already enabled
> by default with a proper frequency, so it wouldn't hit the issue i was
> trying to fix here. But i agree, both looking at other Renesas dtsis
> and how all other external clocks are done in this dtsi, this node is
> odd.

    It's not that odd given that the R-Car gen2 manual specifically says it 
should be 48 MHz.
    Looking into the manual again, the PCIe bus clock must be the onne 
presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be 
100 MHz... So Phil was correct.

MBR, Sergei

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Sergei Shtylyov]

Hello.

On 04/07/2016 10:00 AM, Sjoerd Simons wrote:

> Hey Sergei,
>
> Thanks for your review.

    You're welcome. :-)

> On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
>> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
>>
>>>
>>> clk_get on a disabled clock node will return EPROBE_DEFER, which
>>> can
>>> cause drivers to be deferred forever if such clocks are referenced
>>> in
>>> their clocks property.
>>>
>>> Update the various disabled external clock nodes to default to a
>>> frequency of 0, but don't disable them to prevent this.
>>>
>>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>>>
>>> ---
>>>
>>>    arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>>>    arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>>>    arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>>>    3 files changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> b/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> index 1adf877..da59c28 100644
>>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
>>> @@ -660,6 +660,7 @@
>>>    };
>>>
>>>    &pcie_bus_clk {
>>> +	clock-frequency = <100000000>;

>>      Hmmm, looking at the Koelsch schematics, I don't see this clock.
>> :-/
>
> I don't have the schematics so i was simply keeping the current state.

    I understand. I was just surprised by what checking the code against the 
schematics revealed.

> I've added Phil Edworthy to the list as he was the one originally

    I should've CC'ed Phil myself...

> enable the bus clk for Koelsh according to git. Hopefully he can
> clarify :)

    Let's hope he'd reply...

>>>    	status = "okay";
>>>    };
>>>
>>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
>>> b/arch/arm/boot/dts/r8a7791-porter.dts
>>> index 9554d13..19b257e 100644
>>> --- a/arch/arm/boot/dts/r8a7791-porter.dts
>>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
>>> @@ -413,6 +413,7 @@
>>>    };
>>>
>>>    &pcie_bus_clk {
>>> +	clock-frequency = <100000000>;
>>>    	status = "okay";
>>>    };
>>>
>>      Again, looking at the Porter schematics, I don't see this clock
>> either. :-/
>
> You were the one enabling this clock for Porter ;) I don't have PCIE

    Yes, of course. I don't remember the gory details already -- I might have 
blindly copied what was in the Koelsch's .dts.

> hardware to test with on my porter board, might be worth if you could
> disable this clock and see if PCI-E still fucntions as expected (maybe
> in practise it just happens to prefer the internal clock?) ?

    I know the PCIe driver does require this clock in order to function -- it 
would be the same eternal -EPROBE_DEFER condition you'd already described;
see drivers/pci/host/pcie-rcar.c yourself...

>>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi
>>> b/arch/arm/boot/dts/r8a7791.dtsi
>>> index 8693888..676df63 100644
>>> --- a/arch/arm/boot/dts/r8a7791.dtsi
>>> +++ b/arch/arm/boot/dts/r8a7791.dtsi
>>> @@ -1104,8 +1104,7 @@
>>>    		pcie_bus_clk: pcie_bus {
>>>    			compatible = "fixed-clock";
>>>    			#clock-cells = <0>;
>>> -			clock-frequency = <100000000>;
>>> -			status = "disabled";
>>> +			clock-frequency = <0>;
>>      If the clock has a good default frequency, I don't think you need
>> to
>> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
>> overridden).
>
> I did that as it was by default disabled, so if i do enable it but
> don't drop the default frequency to 0 board swithout that clock will
> suddenly have it added to their dtb.

    Zero frequency is hardly any better then the default...

> For the usb external clock I didn't touch it as it was already enabled
> by default with a proper frequency, so it wouldn't hit the issue i was
> trying to fix here. But i agree, both looking at other Renesas dtsis
> and how all other external clocks are done in this dtsi, this node is
> odd.

    It's not that odd given that the R-Car gen2 manual specifically says it 
should be 48 MHz.
    Looking into the manual again, the PCIe bus clock must be the onne 
presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be 
100 MHz... So Phil was correct.

MBR, Sergei

RE: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Phil Edworthy]

Hi,

On 07 April 2016 20:14, Sergei Shtylyov wrote:
> On 04/07/2016 10:00 AM, Sjoerd Simons wrote:
> 
> > Hey Sergei,
> >
> > Thanks for your review.
> 
>     You're welcome. :-)
> 
> > On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> >> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> >>
> >>>
> >>> clk_get on a disabled clock node will return EPROBE_DEFER, which
> >>> can
> >>> cause drivers to be deferred forever if such clocks are referenced
> >>> in
> >>> their clocks property.
> >>>
> >>> Update the various disabled external clock nodes to default to a
> >>> frequency of 0, but don't disable them to prevent this.
> >>>
> >>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> >>>
> >>> ---
> >>>
> >>>    arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >>>    arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >>>    arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >>>    3 files changed, 3 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> index 1adf877..da59c28 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> @@ -660,6 +660,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> 
> >>      Hmmm, looking at the Koelsch schematics, I don't see this clock.
> >> :-/
> >
> > I don't have the schematics so i was simply keeping the current state.
> 
>     I understand. I was just surprised by what checking the code against the
> schematics revealed.
> 
> > I've added Phil Edworthy to the list as he was the one originally
> 
>     I should've CC'ed Phil myself...
>
> > enable the bus clk for Koelsh according to git. Hopefully he can
> > clarify :)
> 
>     Let's hope he'd reply...
> 
> >>>    	status = "okay";
> >>>    };
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> index 9554d13..19b257e 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> @@ -413,6 +413,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> >>>    	status = "okay";
> >>>    };
> >>>
> >>      Again, looking at the Porter schematics, I don't see this clock
> >> either. :-/
> >
> > You were the one enabling this clock for Porter ;) I don't have PCIE
> 
>     Yes, of course. I don't remember the gory details already -- I might have
> blindly copied what was in the Koelsch's .dts.
> 
> > hardware to test with on my porter board, might be worth if you could
> > disable this clock and see if PCI-E still fucntions as expected (maybe
> > in practise it just happens to prefer the internal clock?) ?
> 
>     I know the PCIe driver does require this clock in order to function -- it
> would be the same eternal -EPROBE_DEFER condition you'd already described;
> see drivers/pci/host/pcie-rcar.c yourself...
> 
> >>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> >>> b/arch/arm/boot/dts/r8a7791.dtsi
> >>> index 8693888..676df63 100644
> >>> --- a/arch/arm/boot/dts/r8a7791.dtsi
> >>> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> >>> @@ -1104,8 +1104,7 @@
> >>>    		pcie_bus_clk: pcie_bus {
> >>>    			compatible = "fixed-clock";
> >>>    			#clock-cells = <0>;
> >>> -			clock-frequency = <100000000>;
> >>> -			status = "disabled";
> >>> +			clock-frequency = <0>;
> >>      If the clock has a good default frequency, I don't think you need
> >> to
> >> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> >> overridden).
> >
> > I did that as it was by default disabled, so if i do enable it but
> > don't drop the default frequency to 0 board swithout that clock will
> > suddenly have it added to their dtb.
> 
>     Zero frequency is hardly any better then the default...
> 
> > For the usb external clock I didn't touch it as it was already enabled
> > by default with a proper frequency, so it wouldn't hit the issue i was
> > trying to fix here. But i agree, both looking at other Renesas dtsis
> > and how all other external clocks are done in this dtsi, this node is
> > odd.
> 
>     It's not that odd given that the R-Car gen2 manual specifically says it
> should be 48 MHz.
>     Looking into the manual again, the PCIe bus clock must be the onne
> presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be
> 100 MHz... So Phil was correct.

PCIe always requires a 100MHz reference clock. On Koelsch, this clock is
generated by U6 and always on. Some boards may need additional hardware
to be setup in order to output the clock, hence the DT node for the clock.

I hope that clarifies the situation!
Phil

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Phil Edworthy]

Hi,

On 07 April 2016 20:14, Sergei Shtylyov wrote:
> On 04/07/2016 10:00 AM, Sjoerd Simons wrote:
> 
> > Hey Sergei,
> >
> > Thanks for your review.
> 
>     You're welcome. :-)
> 
> > On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> >> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> >>
> >>>
> >>> clk_get on a disabled clock node will return EPROBE_DEFER, which
> >>> can
> >>> cause drivers to be deferred forever if such clocks are referenced
> >>> in
> >>> their clocks property.
> >>>
> >>> Update the various disabled external clock nodes to default to a
> >>> frequency of 0, but don't disable them to prevent this.
> >>>
> >>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> >>>
> >>> ---
> >>>
> >>>    arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >>>    arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >>>    arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >>>    3 files changed, 3 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> index 1adf877..da59c28 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> @@ -660,6 +660,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> 
> >>      Hmmm, looking at the Koelsch schematics, I don't see this clock.
> >> :-/
> >
> > I don't have the schematics so i was simply keeping the current state.
> 
>     I understand. I was just surprised by what checking the code against the
> schematics revealed.
> 
> > I've added Phil Edworthy to the list as he was the one originally
> 
>     I should've CC'ed Phil myself...
>
> > enable the bus clk for Koelsh according to git. Hopefully he can
> > clarify :)
> 
>     Let's hope he'd reply...
> 
> >>>    	status = "okay";
> >>>    };
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> index 9554d13..19b257e 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> @@ -413,6 +413,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> >>>    	status = "okay";
> >>>    };
> >>>
> >>      Again, looking at the Porter schematics, I don't see this clock
> >> either. :-/
> >
> > You were the one enabling this clock for Porter ;) I don't have PCIE
> 
>     Yes, of course. I don't remember the gory details already -- I might have
> blindly copied what was in the Koelsch's .dts.
> 
> > hardware to test with on my porter board, might be worth if you could
> > disable this clock and see if PCI-E still fucntions as expected (maybe
> > in practise it just happens to prefer the internal clock?) ?
> 
>     I know the PCIe driver does require this clock in order to function -- it
> would be the same eternal -EPROBE_DEFER condition you'd already described;
> see drivers/pci/host/pcie-rcar.c yourself...
> 
> >>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> >>> b/arch/arm/boot/dts/r8a7791.dtsi
> >>> index 8693888..676df63 100644
> >>> --- a/arch/arm/boot/dts/r8a7791.dtsi
> >>> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> >>> @@ -1104,8 +1104,7 @@
> >>>    		pcie_bus_clk: pcie_bus {
> >>>    			compatible = "fixed-clock";
> >>>    			#clock-cells = <0>;
> >>> -			clock-frequency = <100000000>;
> >>> -			status = "disabled";
> >>> +			clock-frequency = <0>;
> >>      If the clock has a good default frequency, I don't think you need
> >> to
> >> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> >> overridden).
> >
> > I did that as it was by default disabled, so if i do enable it but
> > don't drop the default frequency to 0 board swithout that clock will
> > suddenly have it added to their dtb.
> 
>     Zero frequency is hardly any better then the default...
> 
> > For the usb external clock I didn't touch it as it was already enabled
> > by default with a proper frequency, so it wouldn't hit the issue i was
> > trying to fix here. But i agree, both looking at other Renesas dtsis
> > and how all other external clocks are done in this dtsi, this node is
> > odd.
> 
>     It's not that odd given that the R-Car gen2 manual specifically says it
> should be 48 MHz.
>     Looking into the manual again, the PCIe bus clock must be the onne
> presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be
> 100 MHz... So Phil was correct.

PCIe always requires a 100MHz reference clock. On Koelsch, this clock is
generated by U6 and always on. Some boards may need additional hardware
to be setup in order to output the clock, hence the DT node for the clock.

I hope that clarifies the situation!
Phil

RE: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Phil Edworthy]

Hi,

On 07 April 2016 20:14, Sergei Shtylyov wrote:
> On 04/07/2016 10:00 AM, Sjoerd Simons wrote:
> 
> > Hey Sergei,
> >
> > Thanks for your review.
> 
>     You're welcome. :-)
> 
> > On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> >> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> >>
> >>>
> >>> clk_get on a disabled clock node will return EPROBE_DEFER, which
> >>> can
> >>> cause drivers to be deferred forever if such clocks are referenced
> >>> in
> >>> their clocks property.
> >>>
> >>> Update the various disabled external clock nodes to default to a
> >>> frequency of 0, but don't disable them to prevent this.
> >>>
> >>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> >>>
> >>> ---
> >>>
> >>>    arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >>>    arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >>>    arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >>>    3 files changed, 3 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> index 1adf877..da59c28 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> @@ -660,6 +660,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> 
> >>      Hmmm, looking at the Koelsch schematics, I don't see this clock.
> >> :-/
> >
> > I don't have the schematics so i was simply keeping the current state.
> 
>     I understand. I was just surprised by what checking the code against the
> schematics revealed.
> 
> > I've added Phil Edworthy to the list as he was the one originally
> 
>     I should've CC'ed Phil myself...
>
> > enable the bus clk for Koelsh according to git. Hopefully he can
> > clarify :)
> 
>     Let's hope he'd reply...
> 
> >>>    	status = "okay";
> >>>    };
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> index 9554d13..19b257e 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> @@ -413,6 +413,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> >>>    	status = "okay";
> >>>    };
> >>>
> >>      Again, looking at the Porter schematics, I don't see this clock
> >> either. :-/
> >
> > You were the one enabling this clock for Porter ;) I don't have PCIE
> 
>     Yes, of course. I don't remember the gory details already -- I might have
> blindly copied what was in the Koelsch's .dts.
> 
> > hardware to test with on my porter board, might be worth if you could
> > disable this clock and see if PCI-E still fucntions as expected (maybe
> > in practise it just happens to prefer the internal clock?) ?
> 
>     I know the PCIe driver does require this clock in order to function -- it
> would be the same eternal -EPROBE_DEFER condition you'd already described;
> see drivers/pci/host/pcie-rcar.c yourself...
> 
> >>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> >>> b/arch/arm/boot/dts/r8a7791.dtsi
> >>> index 8693888..676df63 100644
> >>> --- a/arch/arm/boot/dts/r8a7791.dtsi
> >>> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> >>> @@ -1104,8 +1104,7 @@
> >>>    		pcie_bus_clk: pcie_bus {
> >>>    			compatible = "fixed-clock";
> >>>    			#clock-cells = <0>;
> >>> -			clock-frequency = <100000000>;
> >>> -			status = "disabled";
> >>> +			clock-frequency = <0>;
> >>      If the clock has a good default frequency, I don't think you need
> >> to
> >> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> >> overridden).
> >
> > I did that as it was by default disabled, so if i do enable it but
> > don't drop the default frequency to 0 board swithout that clock will
> > suddenly have it added to their dtb.
> 
>     Zero frequency is hardly any better then the default...
> 
> > For the usb external clock I didn't touch it as it was already enabled
> > by default with a proper frequency, so it wouldn't hit the issue i was
> > trying to fix here. But i agree, both looking at other Renesas dtsis
> > and how all other external clocks are done in this dtsi, this node is
> > odd.
> 
>     It's not that odd given that the R-Car gen2 manual specifically says it
> should be 48 MHz.
>     Looking into the manual again, the PCIe bus clock must be the onne
> presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be
> 100 MHz... So Phil was correct.

PCIe always requires a 100MHz reference clock. On Koelsch, this clock is
generated by U6 and always on. Some boards may need additional hardware
to be setup in order to output the clock, hence the DT node for the clock.

I hope that clarifies the situation!
Phil

RE: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Phil Edworthy]

Hi,

On 07 April 2016 20:14, Sergei Shtylyov wrote:
> On 04/07/2016 10:00 AM, Sjoerd Simons wrote:
> 
> > Hey Sergei,
> >
> > Thanks for your review.
> 
>     You're welcome. :-)
> 
> > On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote:
> >> On 04/06/2016 03:52 PM, Sjoerd Simons wrote:
> >>
> >>>
> >>> clk_get on a disabled clock node will return EPROBE_DEFER, which
> >>> can
> >>> cause drivers to be deferred forever if such clocks are referenced
> >>> in
> >>> their clocks property.
> >>>
> >>> Update the various disabled external clock nodes to default to a
> >>> frequency of 0, but don't disable them to prevent this.
> >>>
> >>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> >>>
> >>> ---
> >>>
> >>>    arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> >>>    arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
> >>>    arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
> >>>    3 files changed, 3 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> index 1adf877..da59c28 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> >>> @@ -660,6 +660,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> 
> >>      Hmmm, looking at the Koelsch schematics, I don't see this clock.
> >> :-/
> >
> > I don't have the schematics so i was simply keeping the current state.
> 
>     I understand. I was just surprised by what checking the code against the
> schematics revealed.
> 
> > I've added Phil Edworthy to the list as he was the one originally
> 
>     I should've CC'ed Phil myself...
>
> > enable the bus clk for Koelsh according to git. Hopefully he can
> > clarify :)
> 
>     Let's hope he'd reply...
> 
> >>>    	status = "okay";
> >>>    };
> >>>
> >>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> index 9554d13..19b257e 100644
> >>> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> >>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> >>> @@ -413,6 +413,7 @@
> >>>    };
> >>>
> >>>    &pcie_bus_clk {
> >>> +	clock-frequency = <100000000>;
> >>>    	status = "okay";
> >>>    };
> >>>
> >>      Again, looking at the Porter schematics, I don't see this clock
> >> either. :-/
> >
> > You were the one enabling this clock for Porter ;) I don't have PCIE
> 
>     Yes, of course. I don't remember the gory details already -- I might have
> blindly copied what was in the Koelsch's .dts.
> 
> > hardware to test with on my porter board, might be worth if you could
> > disable this clock and see if PCI-E still fucntions as expected (maybe
> > in practise it just happens to prefer the internal clock?) ?
> 
>     I know the PCIe driver does require this clock in order to function -- it
> would be the same eternal -EPROBE_DEFER condition you'd already described;
> see drivers/pci/host/pcie-rcar.c yourself...
> 
> >>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi
> >>> b/arch/arm/boot/dts/r8a7791.dtsi
> >>> index 8693888..676df63 100644
> >>> --- a/arch/arm/boot/dts/r8a7791.dtsi
> >>> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> >>> @@ -1104,8 +1104,7 @@
> >>>    		pcie_bus_clk: pcie_bus {
> >>>    			compatible = "fixed-clock";
> >>>    			#clock-cells = <0>;
> >>> -			clock-frequency = <100000000>;
> >>> -			status = "disabled";
> >>> +			clock-frequency = <0>;
> >>      If the clock has a good default frequency, I don't think you need
> >> to
> >> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be
> >> overridden).
> >
> > I did that as it was by default disabled, so if i do enable it but
> > don't drop the default frequency to 0 board swithout that clock will
> > suddenly have it added to their dtb.
> 
>     Zero frequency is hardly any better then the default...
> 
> > For the usb external clock I didn't touch it as it was already enabled
> > by default with a proper frequency, so it wouldn't hit the issue i was
> > trying to fix here. But i agree, both looking at other Renesas dtsis
> > and how all other external clocks are done in this dtsi, this node is
> > odd.
> 
>     It's not that odd given that the R-Car gen2 manual specifically says it
> should be 48 MHz.
>     Looking into the manual again, the PCIe bus clock must be the onne
> presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be
> 100 MHz... So Phil was correct.

PCIe always requires a 100MHz reference clock. On Koelsch, this clock is
generated by U6 and always on. Some boards may need additional hardware
to be setup in order to output the clock, hence the DT node for the clock.

I hope that clarifies the situation!
Phil

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Geert Uytterhoeven]

On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Simon Horman]

On Tue, Apr 19, 2016 at 09:18:55AM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > clk_get on a disabled clock node will return EPROBE_DEFER, which can
> > cause drivers to be deferred forever if such clocks are referenced in
> > their clocks property.
> >
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> >
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I will add your tag.

[PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Simon Horman]

On Tue, Apr 19, 2016 at 09:18:55AM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > clk_get on a disabled clock node will return EPROBE_DEFER, which can
> > cause drivers to be deferred forever if such clocks are referenced in
> > their clocks property.
> >
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> >
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I will add your tag.

Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks

[Simon Horman]

On Tue, Apr 19, 2016 at 09:18:55AM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 6, 2016 at 2:52 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > clk_get on a disabled clock node will return EPROBE_DEFER, which can
> > cause drivers to be deferred forever if such clocks are referenced in
> > their clocks property.
> >
> > Update the various disabled external clock nodes to default to a
> > frequency of 0, but don't disable them to prevent this.
> >
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I will add your tag.

[PATCH net-next v3 0/4] vxlan: implement Generic Protocol Extension (GPE)

[Jiri Benc]

v3: just rebased on top of the current net-next, no changes

This patchset implements VXLAN-GPE. It follows the same model as the tun/tap
driver: depending on the chosen mode, the vxlan interface is created either
as ARPHRD_ETHER (non-GPE) or ARPHRD_NONE (GPE).

Note that the internal fdb control plane cannot be used together with
VXLAN-GPE and attempt to configure it will be rejected by the driver. In
fact, COLLECT_METADATA is required to be set for now. This can be relaxed in
the future by adding support for static PtP configuration; it will be
backward compatible and won't affect existing users.

The previous version of the patchset supported two GPE modes, L2 and L3. The
L2 mode (now called "ether mode" in the code) was removed from this version.
It can be easily added later if there's demand. The L3 mode is now called
"raw mode" and supports also encapsulated Ethernet headers (via ETH_P_TEB).

The only limitation of not having "ether mode" for GPE is for ip route based
encapsulation: with such setup, only IP packets can be encapsulated. Meaning
no Ethernet encapsulation. It seems there's not much use for this, though.
If it turns out to be useful, we'll add it.

Jiri Benc (4):
  vxlan: move Ethernet initialization to a separate function
  vxlan: move fdb code to common location in vxlan_xmit
  ip_tunnel: implement __iptunnel_pull_header
  vxlan: implement GPE

 drivers/net/vxlan.c          | 210 ++++++++++++++++++++++++++++++++++++-------
 include/net/ip_tunnels.h     |  11 ++-
 include/net/vxlan.h          |  68 ++++++++++++++
 include/uapi/linux/if_link.h |   1 +
 net/ipv4/ip_tunnel_core.c    |   8 +-
 5 files changed, 258 insertions(+), 40 deletions(-)

-- 
1.8.3.1

[PATCH net-next v3 1/4] vxlan: move Ethernet initialization to a separate function

[Jiri Benc]

This will allow to initialize vxlan in ARPHRD_NONE mode based on the passed
rtnl attributes.

v2: renamed "l2mode" to "ether".

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 drivers/net/vxlan.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1c0fa364323e..6bd5b874ead7 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2404,7 +2404,7 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
 	return 0;
 }
 
-static const struct net_device_ops vxlan_netdev_ops = {
+static const struct net_device_ops vxlan_netdev_ether_ops = {
 	.ndo_init		= vxlan_init,
 	.ndo_uninit		= vxlan_uninit,
 	.ndo_open		= vxlan_open,
@@ -2458,10 +2458,6 @@ static void vxlan_setup(struct net_device *dev)
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	unsigned int h;
 
-	eth_hw_addr_random(dev);
-	ether_setup(dev);
-
-	dev->netdev_ops = &vxlan_netdev_ops;
 	dev->destructor = free_netdev;
 	SET_NETDEV_DEVTYPE(dev, &vxlan_type);
 
@@ -2476,8 +2472,7 @@ static void vxlan_setup(struct net_device *dev)
 	dev->hw_features |= NETIF_F_GSO_SOFTWARE;
 	dev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
 	netif_keep_dst(dev);
-	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
-	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE;
+	dev->priv_flags |= IFF_NO_QUEUE;
 
 	INIT_LIST_HEAD(&vxlan->next);
 	spin_lock_init(&vxlan->hash_lock);
@@ -2496,6 +2491,15 @@ static void vxlan_setup(struct net_device *dev)
 		INIT_HLIST_HEAD(&vxlan->fdb_head[h]);
 }
 
+static void vxlan_ether_setup(struct net_device *dev)
+{
+	eth_hw_addr_random(dev);
+	ether_setup(dev);
+	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
+	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	dev->netdev_ops = &vxlan_netdev_ether_ops;
+}
+
 static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
 	[IFLA_VXLAN_ID]		= { .type = NLA_U32 },
 	[IFLA_VXLAN_GROUP]	= { .len = FIELD_SIZEOF(struct iphdr, daddr) },
@@ -2722,6 +2726,8 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 	__be16 default_port = vxlan->cfg.dst_port;
 	struct net_device *lowerdev = NULL;
 
+	vxlan_ether_setup(dev);
+
 	vxlan->net = src_net;
 
 	dst->remote_vni = conf->vni;
-- 
1.8.3.1

[PATCH net-next v3 2/4] vxlan: move fdb code to common location in vxlan_xmit

[Jiri Benc]

Handle VXLAN_F_COLLECT_METADATA before VXLAN_F_PROXY. The latter does not
make sense with the former, as it needs populated fdb which does not happen
in metadata mode.

After this cleanup, the fdb code in vxlan_xmit is moved to a common location
and can be later skipped for VXLAN-GPE which does not necessarily carry
inner Ethernet header.

v2: changed commit description to not reference L3 mode

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 drivers/net/vxlan.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 6bd5b874ead7..d62eebaa9720 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2106,9 +2106,17 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 	info = skb_tunnel_info(skb);
 
 	skb_reset_mac_header(skb);
-	eth = eth_hdr(skb);
 
-	if ((vxlan->flags & VXLAN_F_PROXY)) {
+	if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
+		if (info && info->mode & IP_TUNNEL_INFO_TX)
+			vxlan_xmit_one(skb, dev, NULL, false);
+		else
+			kfree_skb(skb);
+		return NETDEV_TX_OK;
+	}
+
+	if (vxlan->flags & VXLAN_F_PROXY) {
+		eth = eth_hdr(skb);
 		if (ntohs(eth->h_proto) == ETH_P_ARP)
 			return arp_reduce(dev, skb);
 #if IS_ENABLED(CONFIG_IPV6)
@@ -2123,18 +2131,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 				    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
 					return neigh_reduce(dev, skb);
 		}
-		eth = eth_hdr(skb);
 #endif
 	}
 
-	if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
-		if (info && info->mode & IP_TUNNEL_INFO_TX)
-			vxlan_xmit_one(skb, dev, NULL, false);
-		else
-			kfree_skb(skb);
-		return NETDEV_TX_OK;
-	}
-
+	eth = eth_hdr(skb);
 	f = vxlan_find_mac(vxlan, eth->h_dest);
 	did_rsc = false;
 
-- 
1.8.3.1

[PATCH net-next v3 3/4] ip_tunnel: implement __iptunnel_pull_header

[Jiri Benc]

Allow calling of iptunnel_pull_header without special casing ETH_P_TEB inner
protocol.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
New in v2.
---
 include/net/ip_tunnels.h  | 11 +++++++++--
 net/ipv4/ip_tunnel_core.c |  8 ++++----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 56050f913339..16435d8b1f93 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -295,8 +295,15 @@ static inline u8 ip_tunnel_ecn_encap(u8 tos, const struct iphdr *iph,
 	return INET_ECN_encapsulate(tos, inner);
 }
 
-int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto,
-			 bool xnet);
+int __iptunnel_pull_header(struct sk_buff *skb, int hdr_len,
+			   __be16 inner_proto, bool raw_proto, bool xnet);
+
+static inline int iptunnel_pull_header(struct sk_buff *skb, int hdr_len,
+				       __be16 inner_proto, bool xnet)
+{
+	return __iptunnel_pull_header(skb, hdr_len, inner_proto, false, xnet);
+}
+
 void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
 		   __be32 src, __be32 dst, u8 proto,
 		   u8 tos, u8 ttl, __be16 df, bool xnet);
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index b3ab1205dfdf..43445df61efd 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -86,15 +86,15 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
 }
 EXPORT_SYMBOL_GPL(iptunnel_xmit);
 
-int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto,
-			 bool xnet)
+int __iptunnel_pull_header(struct sk_buff *skb, int hdr_len,
+			   __be16 inner_proto, bool raw_proto, bool xnet)
 {
 	if (unlikely(!pskb_may_pull(skb, hdr_len)))
 		return -ENOMEM;
 
 	skb_pull_rcsum(skb, hdr_len);
 
-	if (inner_proto == htons(ETH_P_TEB)) {
+	if (!raw_proto && inner_proto == htons(ETH_P_TEB)) {
 		struct ethhdr *eh;
 
 		if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
@@ -117,7 +117,7 @@ int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto,
 
 	return iptunnel_pull_offloads(skb);
 }
-EXPORT_SYMBOL_GPL(iptunnel_pull_header);
+EXPORT_SYMBOL_GPL(__iptunnel_pull_header);
 
 struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
 					     gfp_t flags)
-- 
1.8.3.1

[PATCH net-next v3 4/4] vxlan: implement GPE

[Jiri Benc]

Implement VXLAN-GPE. Only COLLECT_METADATA is supported for now (it is
possible to support static configuration, too, if there is demand for it).

The GPE header parsing has to be moved before iptunnel_pull_header, as we
need to know the protocol.

v2: Removed what was called "L2 mode" in v1 of the patchset. Only "L3 mode"
    (now called "raw mode") is added by this patch. This mode does not allow
    Ethernet header to be encapsulated in VXLAN-GPE when using ip route to
    specify the encapsulation, IP header is encapsulated instead. The patch
    does support Ethernet to be encapsulated, though, using ETH_P_TEB in
    skb->protocol. This will be utilized by other COLLECT_METADATA users
    (openvswitch in particular).

    If there is ever demand for Ethernet encapsulation with VXLAN-GPE using
    ip route, it's easy to add a new flag switching the interface to
    "Ethernet mode" (called "L2 mode" in v1 of this patchset). For now,
    leave this out, it seems we don't need it.

    Disallowed more flag combinations, especially RCO with GPE.
    Added comment explaining that GBP and GPE cannot be set together.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 drivers/net/vxlan.c          | 170 ++++++++++++++++++++++++++++++++++++++-----
 include/net/vxlan.h          |  68 +++++++++++++++++
 include/uapi/linux/if_link.h |   1 +
 3 files changed, 222 insertions(+), 17 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d62eebaa9720..51cccddfe403 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1192,6 +1192,45 @@ out:
 	unparsed->vx_flags &= ~VXLAN_GBP_USED_BITS;
 }
 
+static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed,
+				__be32 *protocol,
+				struct sk_buff *skb, u32 vxflags)
+{
+	struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)unparsed;
+
+	/* Need to have Next Protocol set for interfaces in GPE mode. */
+	if (!gpe->np_applied)
+		return false;
+	/* "The initial version is 0. If a receiver does not support the
+	 * version indicated it MUST drop the packet.
+	 */
+	if (gpe->version != 0)
+		return false;
+	/* "When the O bit is set to 1, the packet is an OAM packet and OAM
+	 * processing MUST occur." However, we don't implement OAM
+	 * processing, thus drop the packet.
+	 */
+	if (gpe->oam_flag)
+		return false;
+
+	switch (gpe->next_protocol) {
+	case VXLAN_GPE_NP_IPV4:
+		*protocol = htons(ETH_P_IP);
+		break;
+	case VXLAN_GPE_NP_IPV6:
+		*protocol = htons(ETH_P_IPV6);
+		break;
+	case VXLAN_GPE_NP_ETHERNET:
+		*protocol = htons(ETH_P_TEB);
+		break;
+	default:
+		return false;
+	}
+
+	unparsed->vx_flags &= ~VXLAN_GPE_USED_BITS;
+	return true;
+}
+
 static bool vxlan_set_mac(struct vxlan_dev *vxlan,
 			  struct vxlan_sock *vs,
 			  struct sk_buff *skb)
@@ -1257,9 +1296,11 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	struct vxlanhdr unparsed;
 	struct vxlan_metadata _md;
 	struct vxlan_metadata *md = &_md;
+	__be32 protocol = htons(ETH_P_TEB);
+	bool raw_proto = false;
 	void *oiph;
 
-	/* Need Vxlan and inner Ethernet header to be present */
+	/* Need UDP and VXLAN header to be present */
 	if (!pskb_may_pull(skb, VXLAN_HLEN))
 		return 1;
 
@@ -1283,9 +1324,18 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 	if (!vxlan)
 		goto drop;
 
-	if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB),
-				 !net_eq(vxlan->net, dev_net(vxlan->dev))))
-		goto drop;
+	/* For backwards compatibility, only allow reserved fields to be
+	 * used by VXLAN extensions if explicitly requested.
+	 */
+	if (vs->flags & VXLAN_F_GPE) {
+		if (!vxlan_parse_gpe_hdr(&unparsed, &protocol, skb, vs->flags))
+			goto drop;
+		raw_proto = true;
+	}
+
+	if (__iptunnel_pull_header(skb, VXLAN_HLEN, protocol, raw_proto,
+				   !net_eq(vxlan->net, dev_net(vxlan->dev))))
+			goto drop;
 
 	if (vxlan_collect_metadata(vs)) {
 		__be32 vni = vxlan_vni(vxlan_hdr(skb)->vx_vni);
@@ -1304,14 +1354,14 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		memset(md, 0, sizeof(*md));
 	}
 
-	/* For backwards compatibility, only allow reserved fields to be
-	 * used by VXLAN extensions if explicitly requested.
-	 */
 	if (vs->flags & VXLAN_F_REMCSUM_RX)
 		if (!vxlan_remcsum(&unparsed, skb, vs->flags))
 			goto drop;
 	if (vs->flags & VXLAN_F_GBP)
 		vxlan_parse_gbp_hdr(&unparsed, skb, vs->flags, md);
+	/* Note that GBP and GPE can never be active together. This is
+	 * ensured in vxlan_dev_configure.
+	 */
 
 	if (unparsed.vx_flags || unparsed.vx_vni) {
 		/* If there are any unprocessed flags remaining treat
@@ -1325,8 +1375,13 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 		goto drop;
 	}
 
-	if (!vxlan_set_mac(vxlan, vs, skb))
-		goto drop;
+	if (!raw_proto) {
+		if (!vxlan_set_mac(vxlan, vs, skb))
+			goto drop;
+	} else {
+		skb->dev = vxlan->dev;
+		skb->pkt_type = PACKET_HOST;
+	}
 
 	oiph = skb_network_header(skb);
 	skb_reset_network_header(skb);
@@ -1685,6 +1740,27 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
 	gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
 }
 
+static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
+			       __be16 protocol)
+{
+	struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh;
+
+	gpe->np_applied = 1;
+
+	switch (protocol) {
+	case htons(ETH_P_IP):
+		gpe->next_protocol = VXLAN_GPE_NP_IPV4;
+		return 0;
+	case htons(ETH_P_IPV6):
+		gpe->next_protocol = VXLAN_GPE_NP_IPV6;
+		return 0;
+	case htons(ETH_P_TEB):
+		gpe->next_protocol = VXLAN_GPE_NP_ETHERNET;
+		return 0;
+	}
+	return -EPFNOSUPPORT;
+}
+
 static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
 			   int iphdr_len, __be32 vni,
 			   struct vxlan_metadata *md, u32 vxflags,
@@ -1694,6 +1770,7 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
 	int min_headroom;
 	int err;
 	int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+	__be16 inner_protocol = htons(ETH_P_TEB);
 
 	if ((vxflags & VXLAN_F_REMCSUM_TX) &&
 	    skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -1712,10 +1789,8 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
 
 	/* Need space for new headers (invalidates iph ptr) */
 	err = skb_cow_head(skb, min_headroom);
-	if (unlikely(err)) {
-		kfree_skb(skb);
-		return err;
-	}
+	if (unlikely(err))
+		goto out_free;
 
 	skb = vlan_hwaccel_push_inside(skb);
 	if (WARN_ON(!skb))
@@ -1744,9 +1819,19 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
 
 	if (vxflags & VXLAN_F_GBP)
 		vxlan_build_gbp_hdr(vxh, vxflags, md);
+	if (vxflags & VXLAN_F_GPE) {
+		err = vxlan_build_gpe_hdr(vxh, vxflags, skb->protocol);
+		if (err < 0)
+			goto out_free;
+		inner_protocol = skb->protocol;
+	}
 
-	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
+	skb_set_inner_protocol(skb, inner_protocol);
 	return 0;
+
+out_free:
+	kfree_skb(skb);
+	return err;
 }
 
 static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
@@ -2421,6 +2506,17 @@ static const struct net_device_ops vxlan_netdev_ether_ops = {
 	.ndo_fill_metadata_dst	= vxlan_fill_metadata_dst,
 };
 
+static const struct net_device_ops vxlan_netdev_raw_ops = {
+	.ndo_init		= vxlan_init,
+	.ndo_uninit		= vxlan_uninit,
+	.ndo_open		= vxlan_open,
+	.ndo_stop		= vxlan_stop,
+	.ndo_start_xmit		= vxlan_xmit,
+	.ndo_get_stats64	= ip_tunnel_get_stats64,
+	.ndo_change_mtu		= vxlan_change_mtu,
+	.ndo_fill_metadata_dst	= vxlan_fill_metadata_dst,
+};
+
 /* Info for udev, that this is a virtual tunnel endpoint */
 static struct device_type vxlan_type = {
 	.name = "vxlan",
@@ -2500,6 +2596,17 @@ static void vxlan_ether_setup(struct net_device *dev)
 	dev->netdev_ops = &vxlan_netdev_ether_ops;
 }
 
+static void vxlan_raw_setup(struct net_device *dev)
+{
+	dev->type = ARPHRD_NONE;
+	dev->hard_header_len = 0;
+	dev->addr_len = 0;
+	dev->mtu = ETH_DATA_LEN;
+	dev->tx_queue_len = 1000;
+	dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+	dev->netdev_ops = &vxlan_netdev_raw_ops;
+}
+
 static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
 	[IFLA_VXLAN_ID]		= { .type = NLA_U32 },
 	[IFLA_VXLAN_GROUP]	= { .len = FIELD_SIZEOF(struct iphdr, daddr) },
@@ -2526,6 +2633,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
 	[IFLA_VXLAN_REMCSUM_TX]	= { .type = NLA_U8 },
 	[IFLA_VXLAN_REMCSUM_RX]	= { .type = NLA_U8 },
 	[IFLA_VXLAN_GBP]	= { .type = NLA_FLAG, },
+	[IFLA_VXLAN_GPE]	= { .type = NLA_FLAG, },
 	[IFLA_VXLAN_REMCSUM_NOPARTIAL]	= { .type = NLA_FLAG },
 };
 
@@ -2726,7 +2834,20 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 	__be16 default_port = vxlan->cfg.dst_port;
 	struct net_device *lowerdev = NULL;
 
-	vxlan_ether_setup(dev);
+	if (conf->flags & VXLAN_F_GPE) {
+		if (conf->flags & ~VXLAN_F_ALLOWED_GPE)
+			return -EINVAL;
+		/* For now, allow GPE only together with COLLECT_METADATA.
+		 * This can be relaxed later; in such case, the other side
+		 * of the PtP link will have to be provided.
+		 */
+		if (!(conf->flags & VXLAN_F_COLLECT_METADATA))
+			return -EINVAL;
+
+		vxlan_raw_setup(dev);
+	} else {
+		vxlan_ether_setup(dev);
+	}
 
 	vxlan->net = src_net;
 
@@ -2789,8 +2910,12 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 	dev->needed_headroom = needed_headroom;
 
 	memcpy(&vxlan->cfg, conf, sizeof(*conf));
-	if (!vxlan->cfg.dst_port)
-		vxlan->cfg.dst_port = default_port;
+	if (!vxlan->cfg.dst_port) {
+		if (conf->flags & VXLAN_F_GPE)
+			vxlan->cfg.dst_port = 4790; /* IANA assigned VXLAN-GPE port */
+		else
+			vxlan->cfg.dst_port = default_port;
+	}
 	vxlan->flags |= conf->flags;
 
 	if (!vxlan->cfg.age_interval)
@@ -2961,6 +3086,9 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
 	if (data[IFLA_VXLAN_GBP])
 		conf.flags |= VXLAN_F_GBP;
 
+	if (data[IFLA_VXLAN_GPE])
+		conf.flags |= VXLAN_F_GPE;
+
 	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
 		conf.flags |= VXLAN_F_REMCSUM_NOPARTIAL;
 
@@ -2977,6 +3105,10 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
 	case -EEXIST:
 		pr_info("duplicate VNI %u\n", be32_to_cpu(conf.vni));
 		break;
+
+	case -EINVAL:
+		pr_info("unsupported combination of extensions\n");
+		break;
 	}
 
 	return err;
@@ -3104,6 +3236,10 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
 	    nla_put_flag(skb, IFLA_VXLAN_GBP))
 		goto nla_put_failure;
 
+	if (vxlan->flags & VXLAN_F_GPE &&
+	    nla_put_flag(skb, IFLA_VXLAN_GPE))
+		goto nla_put_failure;
+
 	if (vxlan->flags & VXLAN_F_REMCSUM_NOPARTIAL &&
 	    nla_put_flag(skb, IFLA_VXLAN_REMCSUM_NOPARTIAL))
 		goto nla_put_failure;
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 73ed2e951c02..dcc6f4057115 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -119,6 +119,64 @@ struct vxlanhdr_gbp {
 #define VXLAN_GBP_POLICY_APPLIED	(BIT(3) << 16)
 #define VXLAN_GBP_ID_MASK		(0xFFFF)
 
+/*
+ * VXLAN Generic Protocol Extension (VXLAN_F_GPE):
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |R|R|Ver|I|P|R|O|       Reserved                |Next Protocol  |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |                VXLAN Network Identifier (VNI) |   Reserved    |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * Ver = Version. Indicates VXLAN GPE protocol version.
+ *
+ * P = Next Protocol Bit. The P bit is set to indicate that the
+ *     Next Protocol field is present.
+ *
+ * O = OAM Flag Bit. The O bit is set to indicate that the packet
+ *     is an OAM packet.
+ *
+ * Next Protocol = This 8 bit field indicates the protocol header
+ * immediately following the VXLAN GPE header.
+ *
+ * https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01
+ */
+
+struct vxlanhdr_gpe {
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+	u8	oam_flag:1,
+		reserved_flags1:1,
+		np_applied:1,
+		instance_applied:1,
+		version:2,
+reserved_flags2:2;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+	u8	reserved_flags2:2,
+		version:2,
+		instance_applied:1,
+		np_applied:1,
+		reserved_flags1:1,
+		oam_flag:1;
+#endif
+	u8	reserved_flags3;
+	u8	reserved_flags4;
+	u8	next_protocol;
+	__be32	vx_vni;
+};
+
+/* VXLAN-GPE header flags. */
+#define VXLAN_HF_VER	cpu_to_be32(BIT(29) | BIT(28))
+#define VXLAN_HF_NP	cpu_to_be32(BIT(26))
+#define VXLAN_HF_OAM	cpu_to_be32(BIT(24))
+
+#define VXLAN_GPE_USED_BITS (VXLAN_HF_VER | VXLAN_HF_NP | VXLAN_HF_OAM | \
+			     cpu_to_be32(0xff))
+
+/* VXLAN-GPE header Next Protocol. */
+#define VXLAN_GPE_NP_IPV4      0x01
+#define VXLAN_GPE_NP_IPV6      0x02
+#define VXLAN_GPE_NP_ETHERNET  0x03
+#define VXLAN_GPE_NP_NSH       0x04
+
 struct vxlan_metadata {
 	u32		gbp;
 };
@@ -206,16 +264,26 @@ struct vxlan_dev {
 #define VXLAN_F_GBP			0x800
 #define VXLAN_F_REMCSUM_NOPARTIAL	0x1000
 #define VXLAN_F_COLLECT_METADATA	0x2000
+#define VXLAN_F_GPE			0x4000
 
 /* Flags that are used in the receive path. These flags must match in
  * order for a socket to be shareable
  */
 #define VXLAN_F_RCV_FLAGS		(VXLAN_F_GBP |			\
+					 VXLAN_F_GPE |			\
 					 VXLAN_F_UDP_ZERO_CSUM6_RX |	\
 					 VXLAN_F_REMCSUM_RX |		\
 					 VXLAN_F_REMCSUM_NOPARTIAL |	\
 					 VXLAN_F_COLLECT_METADATA)
 
+/* Flags that can be set together with VXLAN_F_GPE. */
+#define VXLAN_F_ALLOWED_GPE		(VXLAN_F_GPE |			\
+					 VXLAN_F_IPV6 |			\
+					 VXLAN_F_UDP_ZERO_CSUM_TX |	\
+					 VXLAN_F_UDP_ZERO_CSUM6_TX |	\
+					 VXLAN_F_UDP_ZERO_CSUM6_RX |	\
+					 VXLAN_F_COLLECT_METADATA)
+
 struct net_device *vxlan_dev_create(struct net *net, const char *name,
 				    u8 name_assign_type, struct vxlan_config *conf);
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c488066fb53a..9427f17d06d6 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -488,6 +488,7 @@ enum {
 	IFLA_VXLAN_REMCSUM_NOPARTIAL,
 	IFLA_VXLAN_COLLECT_METADATA,
 	IFLA_VXLAN_LABEL,
+	IFLA_VXLAN_GPE,
 	__IFLA_VXLAN_MAX
 };
 #define IFLA_VXLAN_MAX	(__IFLA_VXLAN_MAX - 1)
-- 
1.8.3.1

Re: [PATCH net-next v3 4/4] vxlan: implement GPE

[Tom Herbert]

On Tue, Apr 5, 2016 at 9:47 AM, Jiri Benc <jbenc@redhat.com> wrote:
> Implement VXLAN-GPE. Only COLLECT_METADATA is supported for now (it is
> possible to support static configuration, too, if there is demand for it).
>
> The GPE header parsing has to be moved before iptunnel_pull_header, as we
> need to know the protocol.
>
> v2: Removed what was called "L2 mode" in v1 of the patchset. Only "L3 mode"
>     (now called "raw mode") is added by this patch. This mode does not allow
>     Ethernet header to be encapsulated in VXLAN-GPE when using ip route to
>     specify the encapsulation, IP header is encapsulated instead. The patch
>     does support Ethernet to be encapsulated, though, using ETH_P_TEB in
>     skb->protocol. This will be utilized by other COLLECT_METADATA users
>     (openvswitch in particular).
>
>     If there is ever demand for Ethernet encapsulation with VXLAN-GPE using
>     ip route, it's easy to add a new flag switching the interface to
>     "Ethernet mode" (called "L2 mode" in v1 of this patchset). For now,
>     leave this out, it seems we don't need it.
>
>     Disallowed more flag combinations, especially RCO with GPE.
>     Added comment explaining that GBP and GPE cannot be set together.
>
I requested input from VXLAN protocol experts on whether RCO is
architecturally correct in VXLAN and whether we are using the right
fields both on the mailing list and in WG meeting yesterday @IETF.
Have not gotten any response, so I am going to assume all this is
reasonable. I will add explicit support to VXLAN-RCO draft for
VXLAN-GPE. The configuration option for RX RCO can be removed and RCO
can be supported for VXLAN/VXLAN-GPE in same way. Presumably, GBP
might make same assumptions but GBP format as defined for VXLAN isn't
compatible with VXLAN-GPE.

Tom

> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
>  drivers/net/vxlan.c          | 170 ++++++++++++++++++++++++++++++++++++++-----
>  include/net/vxlan.h          |  68 +++++++++++++++++
>  include/uapi/linux/if_link.h |   1 +
>  3 files changed, 222 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index d62eebaa9720..51cccddfe403 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1192,6 +1192,45 @@ out:
>         unparsed->vx_flags &= ~VXLAN_GBP_USED_BITS;
>  }
>
> +static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed,
> +                               __be32 *protocol,
> +                               struct sk_buff *skb, u32 vxflags)
> +{
> +       struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)unparsed;
> +
> +       /* Need to have Next Protocol set for interfaces in GPE mode. */
> +       if (!gpe->np_applied)
> +               return false;
> +       /* "The initial version is 0. If a receiver does not support the
> +        * version indicated it MUST drop the packet.
> +        */
> +       if (gpe->version != 0)
> +               return false;
> +       /* "When the O bit is set to 1, the packet is an OAM packet and OAM
> +        * processing MUST occur." However, we don't implement OAM
> +        * processing, thus drop the packet.
> +        */
> +       if (gpe->oam_flag)
> +               return false;
> +
> +       switch (gpe->next_protocol) {
> +       case VXLAN_GPE_NP_IPV4:
> +               *protocol = htons(ETH_P_IP);
> +               break;
> +       case VXLAN_GPE_NP_IPV6:
> +               *protocol = htons(ETH_P_IPV6);
> +               break;
> +       case VXLAN_GPE_NP_ETHERNET:
> +               *protocol = htons(ETH_P_TEB);
> +               break;
> +       default:
> +               return false;
> +       }
> +
> +       unparsed->vx_flags &= ~VXLAN_GPE_USED_BITS;
> +       return true;
> +}
> +
>  static bool vxlan_set_mac(struct vxlan_dev *vxlan,
>                           struct vxlan_sock *vs,
>                           struct sk_buff *skb)
> @@ -1257,9 +1296,11 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>         struct vxlanhdr unparsed;
>         struct vxlan_metadata _md;
>         struct vxlan_metadata *md = &_md;
> +       __be32 protocol = htons(ETH_P_TEB);
> +       bool raw_proto = false;
>         void *oiph;
>
> -       /* Need Vxlan and inner Ethernet header to be present */
> +       /* Need UDP and VXLAN header to be present */
>         if (!pskb_may_pull(skb, VXLAN_HLEN))
>                 return 1;
>
> @@ -1283,9 +1324,18 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>         if (!vxlan)
>                 goto drop;
>
> -       if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB),
> -                                !net_eq(vxlan->net, dev_net(vxlan->dev))))
> -               goto drop;
> +       /* For backwards compatibility, only allow reserved fields to be
> +        * used by VXLAN extensions if explicitly requested.
> +        */
> +       if (vs->flags & VXLAN_F_GPE) {
> +               if (!vxlan_parse_gpe_hdr(&unparsed, &protocol, skb, vs->flags))
> +                       goto drop;
> +               raw_proto = true;
> +       }
> +
> +       if (__iptunnel_pull_header(skb, VXLAN_HLEN, protocol, raw_proto,
> +                                  !net_eq(vxlan->net, dev_net(vxlan->dev))))
> +                       goto drop;
>
>         if (vxlan_collect_metadata(vs)) {
>                 __be32 vni = vxlan_vni(vxlan_hdr(skb)->vx_vni);
> @@ -1304,14 +1354,14 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>                 memset(md, 0, sizeof(*md));
>         }
>
> -       /* For backwards compatibility, only allow reserved fields to be
> -        * used by VXLAN extensions if explicitly requested.
> -        */
>         if (vs->flags & VXLAN_F_REMCSUM_RX)
>                 if (!vxlan_remcsum(&unparsed, skb, vs->flags))
>                         goto drop;
>         if (vs->flags & VXLAN_F_GBP)
>                 vxlan_parse_gbp_hdr(&unparsed, skb, vs->flags, md);
> +       /* Note that GBP and GPE can never be active together. This is
> +        * ensured in vxlan_dev_configure.
> +        */
>
>         if (unparsed.vx_flags || unparsed.vx_vni) {
>                 /* If there are any unprocessed flags remaining treat
> @@ -1325,8 +1375,13 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>                 goto drop;
>         }
>
> -       if (!vxlan_set_mac(vxlan, vs, skb))
> -               goto drop;
> +       if (!raw_proto) {
> +               if (!vxlan_set_mac(vxlan, vs, skb))
> +                       goto drop;
> +       } else {
> +               skb->dev = vxlan->dev;
> +               skb->pkt_type = PACKET_HOST;
> +       }
>
>         oiph = skb_network_header(skb);
>         skb_reset_network_header(skb);
> @@ -1685,6 +1740,27 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
>         gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
>  }
>
> +static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
> +                              __be16 protocol)
> +{
> +       struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh;
> +
> +       gpe->np_applied = 1;
> +
> +       switch (protocol) {
> +       case htons(ETH_P_IP):
> +               gpe->next_protocol = VXLAN_GPE_NP_IPV4;
> +               return 0;
> +       case htons(ETH_P_IPV6):
> +               gpe->next_protocol = VXLAN_GPE_NP_IPV6;
> +               return 0;
> +       case htons(ETH_P_TEB):
> +               gpe->next_protocol = VXLAN_GPE_NP_ETHERNET;
> +               return 0;
> +       }
> +       return -EPFNOSUPPORT;
> +}
> +
>  static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>                            int iphdr_len, __be32 vni,
>                            struct vxlan_metadata *md, u32 vxflags,
> @@ -1694,6 +1770,7 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>         int min_headroom;
>         int err;
>         int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
> +       __be16 inner_protocol = htons(ETH_P_TEB);
>
>         if ((vxflags & VXLAN_F_REMCSUM_TX) &&
>             skb->ip_summed == CHECKSUM_PARTIAL) {
> @@ -1712,10 +1789,8 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>
>         /* Need space for new headers (invalidates iph ptr) */
>         err = skb_cow_head(skb, min_headroom);
> -       if (unlikely(err)) {
> -               kfree_skb(skb);
> -               return err;
> -       }
> +       if (unlikely(err))
> +               goto out_free;
>
>         skb = vlan_hwaccel_push_inside(skb);
>         if (WARN_ON(!skb))
> @@ -1744,9 +1819,19 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>
>         if (vxflags & VXLAN_F_GBP)
>                 vxlan_build_gbp_hdr(vxh, vxflags, md);
> +       if (vxflags & VXLAN_F_GPE) {
> +               err = vxlan_build_gpe_hdr(vxh, vxflags, skb->protocol);
> +               if (err < 0)
> +                       goto out_free;
> +               inner_protocol = skb->protocol;
> +       }
>
> -       skb_set_inner_protocol(skb, htons(ETH_P_TEB));
> +       skb_set_inner_protocol(skb, inner_protocol);
>         return 0;
> +
> +out_free:
> +       kfree_skb(skb);
> +       return err;
>  }
>
>  static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
> @@ -2421,6 +2506,17 @@ static const struct net_device_ops vxlan_netdev_ether_ops = {
>         .ndo_fill_metadata_dst  = vxlan_fill_metadata_dst,
>  };
>
> +static const struct net_device_ops vxlan_netdev_raw_ops = {
> +       .ndo_init               = vxlan_init,
> +       .ndo_uninit             = vxlan_uninit,
> +       .ndo_open               = vxlan_open,
> +       .ndo_stop               = vxlan_stop,
> +       .ndo_start_xmit         = vxlan_xmit,
> +       .ndo_get_stats64        = ip_tunnel_get_stats64,
> +       .ndo_change_mtu         = vxlan_change_mtu,
> +       .ndo_fill_metadata_dst  = vxlan_fill_metadata_dst,
> +};
> +
>  /* Info for udev, that this is a virtual tunnel endpoint */
>  static struct device_type vxlan_type = {
>         .name = "vxlan",
> @@ -2500,6 +2596,17 @@ static void vxlan_ether_setup(struct net_device *dev)
>         dev->netdev_ops = &vxlan_netdev_ether_ops;
>  }
>
> +static void vxlan_raw_setup(struct net_device *dev)
> +{
> +       dev->type = ARPHRD_NONE;
> +       dev->hard_header_len = 0;
> +       dev->addr_len = 0;
> +       dev->mtu = ETH_DATA_LEN;
> +       dev->tx_queue_len = 1000;
> +       dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
> +       dev->netdev_ops = &vxlan_netdev_raw_ops;
> +}
> +
>  static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
>         [IFLA_VXLAN_ID]         = { .type = NLA_U32 },
>         [IFLA_VXLAN_GROUP]      = { .len = FIELD_SIZEOF(struct iphdr, daddr) },
> @@ -2526,6 +2633,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
>         [IFLA_VXLAN_REMCSUM_TX] = { .type = NLA_U8 },
>         [IFLA_VXLAN_REMCSUM_RX] = { .type = NLA_U8 },
>         [IFLA_VXLAN_GBP]        = { .type = NLA_FLAG, },
> +       [IFLA_VXLAN_GPE]        = { .type = NLA_FLAG, },
>         [IFLA_VXLAN_REMCSUM_NOPARTIAL]  = { .type = NLA_FLAG },
>  };
>
> @@ -2726,7 +2834,20 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
>         __be16 default_port = vxlan->cfg.dst_port;
>         struct net_device *lowerdev = NULL;
>
> -       vxlan_ether_setup(dev);
> +       if (conf->flags & VXLAN_F_GPE) {
> +               if (conf->flags & ~VXLAN_F_ALLOWED_GPE)
> +                       return -EINVAL;
> +               /* For now, allow GPE only together with COLLECT_METADATA.
> +                * This can be relaxed later; in such case, the other side
> +                * of the PtP link will have to be provided.
> +                */
> +               if (!(conf->flags & VXLAN_F_COLLECT_METADATA))
> +                       return -EINVAL;
> +
> +               vxlan_raw_setup(dev);
> +       } else {
> +               vxlan_ether_setup(dev);
> +       }
>
>         vxlan->net = src_net;
>
> @@ -2789,8 +2910,12 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
>         dev->needed_headroom = needed_headroom;
>
>         memcpy(&vxlan->cfg, conf, sizeof(*conf));
> -       if (!vxlan->cfg.dst_port)
> -               vxlan->cfg.dst_port = default_port;
> +       if (!vxlan->cfg.dst_port) {
> +               if (conf->flags & VXLAN_F_GPE)
> +                       vxlan->cfg.dst_port = 4790; /* IANA assigned VXLAN-GPE port */
> +               else
> +                       vxlan->cfg.dst_port = default_port;
> +       }
>         vxlan->flags |= conf->flags;
>
>         if (!vxlan->cfg.age_interval)
> @@ -2961,6 +3086,9 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
>         if (data[IFLA_VXLAN_GBP])
>                 conf.flags |= VXLAN_F_GBP;
>
> +       if (data[IFLA_VXLAN_GPE])
> +               conf.flags |= VXLAN_F_GPE;
> +
>         if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
>                 conf.flags |= VXLAN_F_REMCSUM_NOPARTIAL;
>
> @@ -2977,6 +3105,10 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
>         case -EEXIST:
>                 pr_info("duplicate VNI %u\n", be32_to_cpu(conf.vni));
>                 break;
> +
> +       case -EINVAL:
> +               pr_info("unsupported combination of extensions\n");
> +               break;
>         }
>
>         return err;
> @@ -3104,6 +3236,10 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
>             nla_put_flag(skb, IFLA_VXLAN_GBP))
>                 goto nla_put_failure;
>
> +       if (vxlan->flags & VXLAN_F_GPE &&
> +           nla_put_flag(skb, IFLA_VXLAN_GPE))
> +               goto nla_put_failure;
> +
>         if (vxlan->flags & VXLAN_F_REMCSUM_NOPARTIAL &&
>             nla_put_flag(skb, IFLA_VXLAN_REMCSUM_NOPARTIAL))
>                 goto nla_put_failure;
> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index 73ed2e951c02..dcc6f4057115 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -119,6 +119,64 @@ struct vxlanhdr_gbp {
>  #define VXLAN_GBP_POLICY_APPLIED       (BIT(3) << 16)
>  #define VXLAN_GBP_ID_MASK              (0xFFFF)
>
> +/*
> + * VXLAN Generic Protocol Extension (VXLAN_F_GPE):
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |R|R|Ver|I|P|R|O|       Reserved                |Next Protocol  |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |                VXLAN Network Identifier (VNI) |   Reserved    |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + *
> + * Ver = Version. Indicates VXLAN GPE protocol version.
> + *
> + * P = Next Protocol Bit. The P bit is set to indicate that the
> + *     Next Protocol field is present.
> + *
> + * O = OAM Flag Bit. The O bit is set to indicate that the packet
> + *     is an OAM packet.
> + *
> + * Next Protocol = This 8 bit field indicates the protocol header
> + * immediately following the VXLAN GPE header.
> + *
> + * https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01
> + */
> +
> +struct vxlanhdr_gpe {
> +#if defined(__LITTLE_ENDIAN_BITFIELD)
> +       u8      oam_flag:1,
> +               reserved_flags1:1,
> +               np_applied:1,
> +               instance_applied:1,
> +               version:2,
> +reserved_flags2:2;
> +#elif defined(__BIG_ENDIAN_BITFIELD)
> +       u8      reserved_flags2:2,
> +               version:2,
> +               instance_applied:1,
> +               np_applied:1,
> +               reserved_flags1:1,
> +               oam_flag:1;
> +#endif
> +       u8      reserved_flags3;
> +       u8      reserved_flags4;
> +       u8      next_protocol;
> +       __be32  vx_vni;
> +};
> +
> +/* VXLAN-GPE header flags. */
> +#define VXLAN_HF_VER   cpu_to_be32(BIT(29) | BIT(28))
> +#define VXLAN_HF_NP    cpu_to_be32(BIT(26))
> +#define VXLAN_HF_OAM   cpu_to_be32(BIT(24))
> +
> +#define VXLAN_GPE_USED_BITS (VXLAN_HF_VER | VXLAN_HF_NP | VXLAN_HF_OAM | \
> +                            cpu_to_be32(0xff))
> +
> +/* VXLAN-GPE header Next Protocol. */
> +#define VXLAN_GPE_NP_IPV4      0x01
> +#define VXLAN_GPE_NP_IPV6      0x02
> +#define VXLAN_GPE_NP_ETHERNET  0x03
> +#define VXLAN_GPE_NP_NSH       0x04
> +
>  struct vxlan_metadata {
>         u32             gbp;
>  };
> @@ -206,16 +264,26 @@ struct vxlan_dev {
>  #define VXLAN_F_GBP                    0x800
>  #define VXLAN_F_REMCSUM_NOPARTIAL      0x1000
>  #define VXLAN_F_COLLECT_METADATA       0x2000
> +#define VXLAN_F_GPE                    0x4000
>
>  /* Flags that are used in the receive path. These flags must match in
>   * order for a socket to be shareable
>   */
>  #define VXLAN_F_RCV_FLAGS              (VXLAN_F_GBP |                  \
> +                                        VXLAN_F_GPE |                  \
>                                          VXLAN_F_UDP_ZERO_CSUM6_RX |    \
>                                          VXLAN_F_REMCSUM_RX |           \
>                                          VXLAN_F_REMCSUM_NOPARTIAL |    \
>                                          VXLAN_F_COLLECT_METADATA)
>
> +/* Flags that can be set together with VXLAN_F_GPE. */
> +#define VXLAN_F_ALLOWED_GPE            (VXLAN_F_GPE |                  \
> +                                        VXLAN_F_IPV6 |                 \
> +                                        VXLAN_F_UDP_ZERO_CSUM_TX |     \
> +                                        VXLAN_F_UDP_ZERO_CSUM6_TX |    \
> +                                        VXLAN_F_UDP_ZERO_CSUM6_RX |    \
> +                                        VXLAN_F_COLLECT_METADATA)
> +
>  struct net_device *vxlan_dev_create(struct net *net, const char *name,
>                                     u8 name_assign_type, struct vxlan_config *conf);
>
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index c488066fb53a..9427f17d06d6 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -488,6 +488,7 @@ enum {
>         IFLA_VXLAN_REMCSUM_NOPARTIAL,
>         IFLA_VXLAN_COLLECT_METADATA,
>         IFLA_VXLAN_LABEL,
> +       IFLA_VXLAN_GPE,
>         __IFLA_VXLAN_MAX
>  };
>  #define IFLA_VXLAN_MAX (__IFLA_VXLAN_MAX - 1)
> --
> 1.8.3.1
>

Re: [PATCH net-next v3 4/4] vxlan: implement GPE, Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks, No serial since ARM: dts: r8a7791: Add BRG support for (H)SCIF, [PATCH net v2 1/3] gre: do not assign header_ops in collect metadata mode, [PATCH iproute2 0/2] ip link gre: fix external mode handling, [PATCH 3/7] soc: renesas: Add r8a7790 SYSC PM Domain Binding Definitions

[Tom Herbert, Sergei Shtylyov, Sjoerd Simons, Jiri Benc, Jiri Benc, Simon Horman]

On Tue, Apr 5, 2016 at 9:47 AM, Jiri Benc <jbenc@redhat.com> wrote:
> Implement VXLAN-GPE. Only COLLECT_METADATA is supported for now (it is
> possible to support static configuration, too, if there is demand for it).
>
> The GPE header parsing has to be moved before iptunnel_pull_header, as we
> need to know the protocol.
>
> v2: Removed what was called "L2 mode" in v1 of the patchset. Only "L3 mode"
>     (now called "raw mode") is added by this patch. This mode does not allow
>     Ethernet header to be encapsulated in VXLAN-GPE when using ip route to
>     specify the encapsulation, IP header is encapsulated instead. The patch
>     does support Ethernet to be encapsulated, though, using ETH_P_TEB in
>     skb->protocol. This will be utilized by other COLLECT_METADATA users
>     (openvswitch in particular).
>
>     If there is ever demand for Ethernet encapsulation with VXLAN-GPE using
>     ip route, it's easy to add a new flag switching the interface to
>     "Ethernet mode" (called "L2 mode" in v1 of this patchset). For now,
>     leave this out, it seems we don't need it.
>
>     Disallowed more flag combinations, especially RCO with GPE.
>     Added comment explaining that GBP and GPE cannot be set together.
>
I requested input from VXLAN protocol experts on whether RCO is
architecturally correct in VXLAN and whether we are using the right
fields both on the mailing list and in WG meeting yesterday @IETF.
Have not gotten any response, so I am going to assume all this is
reasonable. I will add explicit support to VXLAN-RCO draft for
VXLAN-GPE. The configuration option for RX RCO can be removed and RCO
can be supported for VXLAN/VXLAN-GPE in same way. Presumably, GBP
might make same assumptions but GBP format as defined for VXLAN isn't
compatible with VXLAN-GPE.

Tom

> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
>  drivers/net/vxlan.c          | 170 ++++++++++++++++++++++++++++++++++++++-----
>  include/net/vxlan.h          |  68 +++++++++++++++++
>  include/uapi/linux/if_link.h |   1 +
>  3 files changed, 222 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index d62eebaa9720..51cccddfe403 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1192,6 +1192,45 @@ out:
>         unparsed->vx_flags &= ~VXLAN_GBP_USED_BITS;
>  }
>
> +static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed,
> +                               __be32 *protocol,
> +                               struct sk_buff *skb, u32 vxflags)
> +{
> +       struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)unparsed;
> +
> +       /* Need to have Next Protocol set for interfaces in GPE mode. */
> +       if (!gpe->np_applied)
> +               return false;
> +       /* "The initial version is 0. If a receiver does not support the
> +        * version indicated it MUST drop the packet.
> +        */
> +       if (gpe->version != 0)
> +               return false;
> +       /* "When the O bit is set to 1, the packet is an OAM packet and OAM
> +        * processing MUST occur." However, we don't implement OAM
> +        * processing, thus drop the packet.
> +        */
> +       if (gpe->oam_flag)
> +               return false;
> +
> +       switch (gpe->next_protocol) {
> +       case VXLAN_GPE_NP_IPV4:
> +               *protocol = htons(ETH_P_IP);
> +               break;
> +       case VXLAN_GPE_NP_IPV6:
> +               *protocol = htons(ETH_P_IPV6);
> +               break;
> +       case VXLAN_GPE_NP_ETHERNET:
> +               *protocol = htons(ETH_P_TEB);
> +               break;
> +       default:
> +               return false;
> +       }
> +
> +       unparsed->vx_flags &= ~VXLAN_GPE_USED_BITS;
> +       return true;
> +}
> +
>  static bool vxlan_set_mac(struct vxlan_dev *vxlan,
>                           struct vxlan_sock *vs,
>                           struct sk_buff *skb)
> @@ -1257,9 +1296,11 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>         struct vxlanhdr unparsed;
>         struct vxlan_metadata _md;
>         struct vxlan_metadata *md = &_md;
> +       __be32 protocol = htons(ETH_P_TEB);
> +       bool raw_proto = false;
>         void *oiph;
>
> -       /* Need Vxlan and inner Ethernet header to be present */
> +       /* Need UDP and VXLAN header to be present */
>         if (!pskb_may_pull(skb, VXLAN_HLEN))
>                 return 1;
>
> @@ -1283,9 +1324,18 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>         if (!vxlan)
>                 goto drop;
>
> -       if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB),
> -                                !net_eq(vxlan->net, dev_net(vxlan->dev))))
> -               goto drop;
> +       /* For backwards compatibility, only allow reserved fields to be
> +        * used by VXLAN extensions if explicitly requested.
> +        */
> +       if (vs->flags & VXLAN_F_GPE) {
> +               if (!vxlan_parse_gpe_hdr(&unparsed, &protocol, skb, vs->flags))
> +                       goto drop;
> +               raw_proto = true;
> +       }
> +
> +       if (__iptunnel_pull_header(skb, VXLAN_HLEN, protocol, raw_proto,
> +                                  !net_eq(vxlan->net, dev_net(vxlan->dev))))
> +                       goto drop;
>
>         if (vxlan_collect_metadata(vs)) {
>                 __be32 vni = vxlan_vni(vxlan_hdr(skb)->vx_vni);
> @@ -1304,14 +1354,14 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>                 memset(md, 0, sizeof(*md));
>         }
>
> -       /* For backwards compatibility, only allow reserved fields to be
> -        * used by VXLAN extensions if explicitly requested.
> -        */
>         if (vs->flags & VXLAN_F_REMCSUM_RX)
>                 if (!vxlan_remcsum(&unparsed, skb, vs->flags))
>                         goto drop;
>         if (vs->flags & VXLAN_F_GBP)
>                 vxlan_parse_gbp_hdr(&unparsed, skb, vs->flags, md);
> +       /* Note that GBP and GPE can never be active together. This is
> +        * ensured in vxlan_dev_configure.
> +        */
>
>         if (unparsed.vx_flags || unparsed.vx_vni) {
>                 /* If there are any unprocessed flags remaining treat
> @@ -1325,8 +1375,13 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>                 goto drop;
>         }
>
> -       if (!vxlan_set_mac(vxlan, vs, skb))
> -               goto drop;
> +       if (!raw_proto) {
> +               if (!vxlan_set_mac(vxlan, vs, skb))
> +                       goto drop;
> +       } else {
> +               skb->dev = vxlan->dev;
> +               skb->pkt_type = PACKET_HOST;
> +       }
>
>         oiph = skb_network_header(skb);
>         skb_reset_network_header(skb);
> @@ -1685,6 +1740,27 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
>         gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
>  }
>
> +static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
> +                              __be16 protocol)
> +{
> +       struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh;
> +
> +       gpe->np_applied = 1;
> +
> +       switch (protocol) {
> +       case htons(ETH_P_IP):
> +               gpe->next_protocol = VXLAN_GPE_NP_IPV4;
> +               return 0;
> +       case htons(ETH_P_IPV6):
> +               gpe->next_protocol = VXLAN_GPE_NP_IPV6;
> +               return 0;
> +       case htons(ETH_P_TEB):
> +               gpe->next_protocol = VXLAN_GPE_NP_ETHERNET;
> +               return 0;
> +       }
> +       return -EPFNOSUPPORT;
> +}
> +
>  static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>                            int iphdr_len, __be32 vni,
>                            struct vxlan_metadata *md, u32 vxflags,
> @@ -1694,6 +1770,7 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>         int min_headroom;
>         int err;
>         int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
> +       __be16 inner_protocol = htons(ETH_P_TEB);
>
>         if ((vxflags & VXLAN_F_REMCSUM_TX) &&
>             skb->ip_summed == CHECKSUM_PARTIAL) {
> @@ -1712,10 +1789,8 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>
>         /* Need space for new headers (invalidates iph ptr) */
>         err = skb_cow_head(skb, min_headroom);
> -       if (unlikely(err)) {
> -               kfree_skb(skb);
> -               return err;
> -       }
> +       if (unlikely(err))
> +               goto out_free;
>
>         skb = vlan_hwaccel_push_inside(skb);
>         if (WARN_ON(!skb))
> @@ -1744,9 +1819,19 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
>
>         if (vxflags & VXLAN_F_GBP)
>                 vxlan_build_gbp_hdr(vxh, vxflags, md);
> +       if (vxflags & VXLAN_F_GPE) {
> +               err = vxlan_build_gpe_hdr(vxh, vxflags, skb->protocol);
> +               if (err < 0)
> +                       goto out_free;
> +               inner_protocol = skb->protocol;
> +       }
>
> -       skb_set_inner_protocol(skb, htons(ETH_P_TEB));
> +       skb_set_inner_protocol(skb, inner_protocol);
>         return 0;
> +
> +out_free:
> +       kfree_skb(skb);
> +       return err;
>  }
>
>  static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
> @@ -2421,6 +2506,17 @@ static const struct net_device_ops vxlan_netdev_ether_ops = {
>         .ndo_fill_metadata_dst  = vxlan_fill_metadata_dst,
>  };
>
> +static const struct net_device_ops vxlan_netdev_raw_ops = {
> +       .ndo_init               = vxlan_init,
> +       .ndo_uninit             = vxlan_uninit,
> +       .ndo_open               = vxlan_open,
> +       .ndo_stop               = vxlan_stop,
> +       .ndo_start_xmit         = vxlan_xmit,
> +       .ndo_get_stats64        = ip_tunnel_get_stats64,
> +       .ndo_change_mtu         = vxlan_change_mtu,
> +       .ndo_fill_metadata_dst  = vxlan_fill_metadata_dst,
> +};
> +
>  /* Info for udev, that this is a virtual tunnel endpoint */
>  static struct device_type vxlan_type = {
>         .name = "vxlan",
> @@ -2500,6 +2596,17 @@ static void vxlan_ether_setup(struct net_device *dev)
>         dev->netdev_ops = &vxlan_netdev_ether_ops;
>  }
>
> +static void vxlan_raw_setup(struct net_device *dev)
> +{
> +       dev->type = ARPHRD_NONE;
> +       dev->hard_header_len = 0;
> +       dev->addr_len = 0;
> +       dev->mtu = ETH_DATA_LEN;
> +       dev->tx_queue_len = 1000;
> +       dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
> +       dev->netdev_ops = &vxlan_netdev_raw_ops;
> +}
> +
>  static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
>         [IFLA_VXLAN_ID]         = { .type = NLA_U32 },
>         [IFLA_VXLAN_GROUP]      = { .len = FIELD_SIZEOF(struct iphdr, daddr) },
> @@ -2526,6 +2633,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
>         [IFLA_VXLAN_REMCSUM_TX] = { .type = NLA_U8 },
>         [IFLA_VXLAN_REMCSUM_RX] = { .type = NLA_U8 },
>         [IFLA_VXLAN_GBP]        = { .type = NLA_FLAG, },
> +       [IFLA_VXLAN_GPE]        = { .type = NLA_FLAG, },
>         [IFLA_VXLAN_REMCSUM_NOPARTIAL]  = { .type = NLA_FLAG },
>  };
>
> @@ -2726,7 +2834,20 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
>         __be16 default_port = vxlan->cfg.dst_port;
>         struct net_device *lowerdev = NULL;
>
> -       vxlan_ether_setup(dev);
> +       if (conf->flags & VXLAN_F_GPE) {
> +               if (conf->flags & ~VXLAN_F_ALLOWED_GPE)
> +                       return -EINVAL;
> +               /* For now, allow GPE only together with COLLECT_METADATA.
> +                * This can be relaxed later; in such case, the other side
> +                * of the PtP link will have to be provided.
> +                */
> +               if (!(conf->flags & VXLAN_F_COLLECT_METADATA))
> +                       return -EINVAL;
> +
> +               vxlan_raw_setup(dev);
> +       } else {
> +               vxlan_ether_setup(dev);
> +       }
>
>         vxlan->net = src_net;
>
> @@ -2789,8 +2910,12 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
>         dev->needed_headroom = needed_headroom;
>
>         memcpy(&vxlan->cfg, conf, sizeof(*conf));
> -       if (!vxlan->cfg.dst_port)
> -               vxlan->cfg.dst_port = default_port;
> +       if (!vxlan->cfg.dst_port) {
> +               if (conf->flags & VXLAN_F_GPE)
> +                       vxlan->cfg.dst_port = 4790; /* IANA assigned VXLAN-GPE port */
> +               else
> +                       vxlan->cfg.dst_port = default_port;
> +       }
>         vxlan->flags |= conf->flags;
>
>         if (!vxlan->cfg.age_interval)
> @@ -2961,6 +3086,9 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
>         if (data[IFLA_VXLAN_GBP])
>                 conf.flags |= VXLAN_F_GBP;
>
> +       if (data[IFLA_VXLAN_GPE])
> +               conf.flags |= VXLAN_F_GPE;
> +
>         if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
>                 conf.flags |= VXLAN_F_REMCSUM_NOPARTIAL;
>
> @@ -2977,6 +3105,10 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
>         case -EEXIST:
>                 pr_info("duplicate VNI %u\n", be32_to_cpu(conf.vni));
>                 break;
> +
> +       case -EINVAL:
> +               pr_info("unsupported combination of extensions\n");
> +               break;
>         }
>
>         return err;
> @@ -3104,6 +3236,10 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
>             nla_put_flag(skb, IFLA_VXLAN_GBP))
>                 goto nla_put_failure;
>
> +       if (vxlan->flags & VXLAN_F_GPE &&
> +           nla_put_flag(skb, IFLA_VXLAN_GPE))
> +               goto nla_put_failure;
> +
>         if (vxlan->flags & VXLAN_F_REMCSUM_NOPARTIAL &&
>             nla_put_flag(skb, IFLA_VXLAN_REMCSUM_NOPARTIAL))
>                 goto nla_put_failure;
> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index 73ed2e951c02..dcc6f4057115 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -119,6 +119,64 @@ struct vxlanhdr_gbp {
>  #define VXLAN_GBP_POLICY_APPLIED       (BIT(3) << 16)
>  #define VXLAN_GBP_ID_MASK              (0xFFFF)
>
> +/*
> + * VXLAN Generic Protocol Extension (VXLAN_F_GPE):
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |R|R|Ver|I|P|R|O|       Reserved                |Next Protocol  |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |                VXLAN Network Identifier (VNI) |   Reserved    |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + *
> + * Ver = Version. Indicates VXLAN GPE protocol version.
> + *
> + * P = Next Protocol Bit. The P bit is set to indicate that the
> + *     Next Protocol field is present.
> + *
> + * O = OAM Flag Bit. The O bit is set to indicate that the packet
> + *     is an OAM packet.
> + *
> + * Next Protocol = This 8 bit field indicates the protocol header
> + * immediately following the VXLAN GPE header.
> + *
> + * https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01
> + */
> +
> +struct vxlanhdr_gpe {
> +#if defined(__LITTLE_ENDIAN_BITFIELD)
> +       u8      oam_flag:1,
> +               reserved_flags1:1,
> +               np_applied:1,
> +               instance_applied:1,
> +               version:2,
> +reserved_flags2:2;
> +#elif defined(__BIG_ENDIAN_BITFIELD)
> +       u8      reserved_flags2:2,
> +               version:2,
> +               instance_applied:1,
> +               np_applied:1,
> +               reserved_flags1:1,
> +               oam_flag:1;
> +#endif
> +       u8      reserved_flags3;
> +       u8      reserved_flags4;
> +       u8      next_protocol;
> +       __be32  vx_vni;
> +};
> +
> +/* VXLAN-GPE header flags. */
> +#define VXLAN_HF_VER   cpu_to_be32(BIT(29) | BIT(28))
> +#define VXLAN_HF_NP    cpu_to_be32(BIT(26))
> +#define VXLAN_HF_OAM   cpu_to_be32(BIT(24))
> +
> +#define VXLAN_GPE_USED_BITS (VXLAN_HF_VER | VXLAN_HF_NP | VXLAN_HF_OAM | \
> +                            cpu_to_be32(0xff))
> +
> +/* VXLAN-GPE header Next Protocol. */
> +#define VXLAN_GPE_NP_IPV4      0x01
> +#define VXLAN_GPE_NP_IPV6      0x02
> +#define VXLAN_GPE_NP_ETHERNET  0x03
> +#define VXLAN_GPE_NP_NSH       0x04
> +
>  struct vxlan_metadata {
>         u32             gbp;
>  };
> @@ -206,16 +264,26 @@ struct vxlan_dev {
>  #define VXLAN_F_GBP                    0x800
>  #define VXLAN_F_REMCSUM_NOPARTIAL      0x1000
>  #define VXLAN_F_COLLECT_METADATA       0x2000
> +#define VXLAN_F_GPE                    0x4000
>
>  /* Flags that are used in the receive path. These flags must match in
>   * order for a socket to be shareable
>   */
>  #define VXLAN_F_RCV_FLAGS              (VXLAN_F_GBP |                  \
> +                                        VXLAN_F_GPE |                  \
>                                          VXLAN_F_UDP_ZERO_CSUM6_RX |    \
>                                          VXLAN_F_REMCSUM_RX |           \
>                                          VXLAN_F_REMCSUM_NOPARTIAL |    \
>                                          VXLAN_F_COLLECT_METADATA)
>
> +/* Flags that can be set together with VXLAN_F_GPE. */
> +#define VXLAN_F_ALLOWED_GPE            (VXLAN_F_GPE |                  \
> +                                        VXLAN_F_IPV6 |                 \
> +                                        VXLAN_F_UDP_ZERO_CSUM_TX |     \
> +                                        VXLAN_F_UDP_ZERO_CSUM6_TX |    \
> +                                        VXLAN_F_UDP_ZERO_CSUM6_RX |    \
> +                                        VXLAN_F_COLLECT_METADATA)
> +
>  struct net_device *vxlan_dev_create(struct net *net, const char *name,
>                                     u8 name_assign_type, struct vxlan_config *conf);
>
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index c488066fb53a..9427f17d06d6 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -488,6 +488,7 @@ enum {
>         IFLA_VXLAN_REMCSUM_NOPARTIAL,
>         IFLA_VXLAN_COLLECT_METADATA,
>         IFLA_VXLAN_LABEL,
> +       IFLA_VXLAN_GPE,
>         __IFLA_VXLAN_MAX
>  };
>  #define IFLA_VXLAN_MAX (__IFLA_VXLAN_MAX - 1)
> --
> 1.8.3.1
>

On 04/06/2016 03:52 PM, Sjoerd Simons wrote:

> clk_get on a disabled clock node will return EPROBE_DEFER, which can
> cause drivers to be deferred forever if such clocks are referenced in
> their clocks property.
>
> Update the various disabled external clock nodes to default to a
> frequency of 0, but don't disable them to prevent this.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>   arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>   arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>   arch/arm/boot/dts/r8a7791.dtsi        | 5 +----
>   3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 1adf877..da59c28 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -660,6 +660,7 @@
>   };
>
>   &pcie_bus_clk {
> +	clock-frequency = <100000000>;

    Hmmm, looking at the Koelsch schematics, I don't see this clock. :-/

>   	status = "okay";
>   };
>
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 9554d13..19b257e 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -413,6 +413,7 @@
>   };
>
>   &pcie_bus_clk {
> +	clock-frequency = <100000000>;
>   	status = "okay";
>   };
>

    Again, looking at the Porter schematics, I don't see this clock either. :-/

> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 8693888..676df63 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -1104,8 +1104,7 @@
>   		pcie_bus_clk: pcie_bus {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
> -			clock-frequency = <100000000>;
> -			status = "disabled";
> +			clock-frequency = <0>;

    If the clock has a good default frequency, I don't think you need to 
remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be overridden).

[...]

MBR, Sergei

Hey,

I've got a Porter board (Revision B) which stopped showing serial
output since the patch mentioned in the subject. The terrible clearly
wrong hack below gets serial back (reverting to just the  fck clock for
scif0) on both linux-next and linux master.

Unfortunately I don't have enough documentation on the board/hardware
to diagnose this further, but hopefully someone else does ;)

-- 
Sjoerd Simons
Collabora Ltd.

---
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 8693888..b2c1f1c 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -731,9 +731,8 @@
 			     "renesas,scif";
 		reg = <0 0xe6e60000 0 64>;
 		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&mstp7_clks R8A7791_CLK_SCIF0>, <&zs_clk>,
-			 <&scif_clk>;
-		clock-names = "fck", "brg_int", "scif_clk";
+		clocks = <&mstp7_clks R8A7791_CLK_SCIF0>;
+		clock-names = "fck";
 		dmas = <&dmac0 0x29>, <&dmac0 0x2a>,
 		       <&dmac1 0x29>, <&dmac1 0x2a>;
 		dma-names = "tx", "rx", "tx", "rx";

In ipgre mode (i.e. not gretap) with collect metadata flag set, the tunnel
is incorrectly assumed to be mGRE in NBMA mode (see commit 6a5f44d7a048c).
This is not the case, we're controlling the encapsulation addresses by
lwtunnel metadata. And anyway, assigning dev->header_ops in collect metadata
mode does not make sense.

Similarly, when a multicast remote IP address is set together with the
collect metadata flag, the processing described above would happen, too. As
there's not much sense in specifying remote/local IP address for lwtunnels,
reject such configuration.

v2: Reject configuration specifying both remote/local address and collect
    metadata flag.

Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 net/ipv4/ip_gre.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index af5d1f38217f..c035b43b1d4b 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -902,8 +902,9 @@ static int ipgre_tunnel_init(struct net_device *dev)
 			dev->header_ops = &ipgre_header_ops;
 		}
 #endif
-	} else
+	} else if (!tunnel->collect_md) {
 		dev->header_ops = &ipgre_header_ops;
+	}
 
 	return ip_tunnel_init(dev);
 }
@@ -946,6 +947,15 @@ static int ipgre_tunnel_validate(struct nlattr *tb[], struct nlattr *data[])
 	if (flags & (GRE_VERSION|GRE_ROUTING))
 		return -EINVAL;
 
+	if (data[IFLA_GRE_COLLECT_METADATA]) {
+		if (data[IFLA_GRE_REMOTE] &&
+		    nla_get_in_addr(data[IFLA_GRE_REMOTE]))
+			return -EINVAL;
+		if (data[IFLA_GRE_LOCAL] &&
+		    nla_get_in_addr(data[IFLA_GRE_LOCAL]))
+			return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
1.8.3.1

Fix two bugs with handling of the 'external' keyword for GRE.

Jiri Benc (2):
  ip link gre: create interfaces in external mode correctly
  ip link gre: print only relevant info in external mode

 ip/link_gre.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

-- 
1.8.3.1

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/dt-bindings/power/r8a7790-sysc.h | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 include/dt-bindings/power/r8a7790-sysc.h

diff --git a/include/dt-bindings/power/r8a7790-sysc.h b/include/dt-bindings/power/r8a7790-sysc.h
new file mode 100644
index 000000000000..6af4e9929bd0
--- /dev/null
+++ b/include/dt-bindings/power/r8a7790-sysc.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+#ifndef __DT_BINDINGS_POWER_R8A7790_SYSC_H__
+#define __DT_BINDINGS_POWER_R8A7790_SYSC_H__
+
+/*
+ * These power domain indices match the numbers of the interrupt bits
+ * representing the power areas in the various Interrupt Registers
+ * (e.g. SYSCISR, Interrupt Status Register)
+ */
+
+#define R8A7790_PD_CA15_CPU0		 0
+#define R8A7790_PD_CA15_CPU1		 1
+#define R8A7790_PD_CA15_CPU2		 2
+#define R8A7790_PD_CA15_CPU3		 3
+#define R8A7790_PD_CA7_CPU0		 5
+#define R8A7790_PD_CA7_CPU1		 6
+#define R8A7790_PD_CA7_CPU2		 7
+#define R8A7790_PD_CA7_CPU3		 8
+#define R8A7790_PD_CA15_SCU		12
+#define R8A7790_PD_SH_4A		16
+#define R8A7790_PD_RGX			20
+#define R8A7790_PD_CA7_SCU		21
+#define R8A7790_PD_IMP			24
+
+/* Always-on power area */
+#define R8A7790_PD_ALWAYS_ON		32
+
+#endif /* __DT_BINDINGS_POWER_R8A7790_SYSC_H__ */
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH net-next v3 4/4] vxlan: implement GPE

[Jiri Benc]

On Tue, 5 Apr 2016 10:50:57 -0300, Tom Herbert wrote:
> I requested input from VXLAN protocol experts on whether RCO is
> architecturally correct in VXLAN and whether we are using the right
> fields both on the mailing list and in WG meeting yesterday @IETF.
> Have not gotten any response, so I am going to assume all this is
> reasonable. I will add explicit support to VXLAN-RCO draft for
> VXLAN-GPE. The configuration option for RX RCO can be removed and RCO
> can be supported for VXLAN/VXLAN-GPE in same way. Presumably, GBP
> might make same assumptions but GBP format as defined for VXLAN isn't
> compatible with VXLAN-GPE.

Cool, thanks! We'll loosen the restriction after it's added to the RFC,
it should be a simple one line patch.

 Jiri

Re: [PATCH net-next v3 0/4] vxlan: implement Generic Protocol Extension (GPE)

[David Miller]

From: Jiri Benc <jbenc@redhat.com>
Date: Tue,  5 Apr 2016 14:47:09 +0200

> v3: just rebased on top of the current net-next, no changes
> 
> This patchset implements VXLAN-GPE. It follows the same model as the tun/tap
> driver: depending on the chosen mode, the vxlan interface is created either
> as ARPHRD_ETHER (non-GPE) or ARPHRD_NONE (GPE).
> 
> Note that the internal fdb control plane cannot be used together with
> VXLAN-GPE and attempt to configure it will be rejected by the driver. In
> fact, COLLECT_METADATA is required to be set for now. This can be relaxed in
> the future by adding support for static PtP configuration; it will be
> backward compatible and won't affect existing users.
> 
> The previous version of the patchset supported two GPE modes, L2 and L3. The
> L2 mode (now called "ether mode" in the code) was removed from this version.
> It can be easily added later if there's demand. The L3 mode is now called
> "raw mode" and supports also encapsulated Ethernet headers (via ETH_P_TEB).
> 
> The only limitation of not having "ether mode" for GPE is for ip route based
> encapsulation: with such setup, only IP packets can be encapsulated. Meaning
> no Ethernet encapsulation. It seems there's not much use for this, though.
> If it turns out to be useful, we'll add it.

Series applied, thanks Jiri.