All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
@ 2018-01-29  1:15 Andre Przywara
  2018-01-29  1:15 ` [U-Boot] [PATCH 1/3] sunxi: gpio: add missing compatible strings Andre Przywara
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Andre Przywara @ 2018-01-29  1:15 UTC (permalink / raw)
  To: u-boot

The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
which matched our own DTs. Now that the Linux kernel got a driver, lets
update our probe code to handle those Linux DTs as well.
The first patch adds the missing compatible strings for the pinctrl drivers,
which is needed for using the sunxi_name_to_gpio() lookup function.
Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
with the new, generic Allwinner pinctrl bindings.
The final patch extends the probe routine in the Ethernet driver to deal
with both the old and the new bindings.
This series allows to copy in the DTs from the latest kernel. Unfortunately
right now updating the DTs for the H5 and A64 breaks the build, as the
resulting binary (which embeds the DT) gets to large and triggers our new
image size check. As the H5 and H3 share most of the DT, we can't just
update the H3 DTs either. Hopefully we find some neat trick to work
around that.

Cheers,
Andre.

Andre Przywara (3):
  sunxi: gpio: add missing compatible strings
  net: sun8i-emac: support new pinctrl DT bindings
  net: sun8i-emac: add support for new EMAC DT binding

 drivers/gpio/sunxi_gpio.c |   4 ++
 drivers/net/sun8i_emac.c  | 107 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 91 insertions(+), 20 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/3] sunxi: gpio: add missing compatible strings
  2018-01-29  1:15 [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Andre Przywara
@ 2018-01-29  1:15 ` Andre Przywara
  2018-01-29  1:15 ` [U-Boot] [PATCH 2/3] net: sun8i-emac: support new pinctrl DT bindings Andre Przywara
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2018-01-29  1:15 UTC (permalink / raw)
  To: u-boot

The sunxi GPIO driver is missing some compatible strings for recent
SoCs. While most of the sunxi GPIO code seems to not rely on this (and
so works anyway), the sunxi_name_to_gpio() function does and fails at
the moment (for instance when resolving the MMC CD pin name).
Add the compatible strings for the A64, H5 and V3s, which were missing
from the list. This now covers all pinctrl nodes in our own DTs.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/gpio/sunxi_gpio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c
index 3cf01b6e36..90fd7c1596 100644
--- a/drivers/gpio/sunxi_gpio.c
+++ b/drivers/gpio/sunxi_gpio.c
@@ -354,12 +354,16 @@ static const struct udevice_id sunxi_gpio_ids[] = {
 	ID("allwinner,sun8i-a83t-pinctrl",	a_all),
 	ID("allwinner,sun8i-h3-pinctrl",	a_all),
 	ID("allwinner,sun8i-r40-pinctrl",	a_all),
+	ID("allwinner,sun8i-v3s-pinctrl",	a_all),
 	ID("allwinner,sun9i-a80-pinctrl",	a_all),
+	ID("allwinner,sun50i-a64-pinctrl",	a_all),
+	ID("allwinner,sun50i-h5-pinctrl",	a_all),
 	ID("allwinner,sun6i-a31-r-pinctrl",	l_2),
 	ID("allwinner,sun8i-a23-r-pinctrl",	l_1),
 	ID("allwinner,sun8i-a83t-r-pinctrl",	l_1),
 	ID("allwinner,sun8i-h3-r-pinctrl",	l_1),
 	ID("allwinner,sun9i-a80-r-pinctrl",	l_3),
+	ID("allwinner,sun50i-a64-r-pinctrl",	l_1),
 	{ }
 };
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/3] net: sun8i-emac: support new pinctrl DT bindings
  2018-01-29  1:15 [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Andre Przywara
  2018-01-29  1:15 ` [U-Boot] [PATCH 1/3] sunxi: gpio: add missing compatible strings Andre Przywara
@ 2018-01-29  1:15 ` Andre Przywara
  2018-01-29  1:15 ` [U-Boot] [PATCH 3/3] net: sun8i-emac: add support for new EMAC DT binding Andre Przywara
  2018-01-29  8:51 ` [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Maxime Ripard
  3 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2018-01-29  1:15 UTC (permalink / raw)
  To: u-boot

The Linux kernel driver for the Allwinner pin controller gained support
for generic properties, which are now also used in the DTs.
The sun8i-emac Ethernet driver for new Allwinner MACs reads the pins from
the DT, but so far only supported the old binding.
Update the parsing routine to cope with both the old and new bindings,
so that the newer DTs can be used with U-Boot and its Ethernet driver.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/net/sun8i_emac.c | 52 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 3ccc6b0bb6..df2b857310 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -21,6 +21,7 @@
 #include <malloc.h>
 #include <miiphy.h>
 #include <net.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
 #ifdef CONFIG_DM_GPIO
 #include <asm-generic/gpio.h>
 #endif
@@ -465,30 +466,55 @@ static int parse_phy_pins(struct udevice *dev)
 	}
 
 	drive = fdt_getprop_u32_default_node(gd->fdt_blob, offset, 0,
-					     "allwinner,drive", 4);
-	pull = fdt_getprop_u32_default_node(gd->fdt_blob, offset, 0,
-					    "allwinner,pull", 0);
+					     "drive-strength", ~0);
+	if (drive != ~0) {
+		if (drive <= 10)
+			drive = SUN4I_PINCTRL_10_MA;
+		else if (drive <= 20)
+			drive = SUN4I_PINCTRL_20_MA;
+		else if (drive <= 30)
+			drive = SUN4I_PINCTRL_30_MA;
+		else
+			drive = SUN4I_PINCTRL_40_MA;
+	} else {
+		drive = fdt_getprop_u32_default_node(gd->fdt_blob, offset, 0,
+						     "allwinner,drive", 4);
+	}
+
+	if (fdt_get_property(gd->fdt_blob, offset, "bias-pull-up", NULL))
+		pull = SUN4I_PINCTRL_PULL_UP;
+	else if (fdt_get_property(gd->fdt_blob, offset, "bias-disable", NULL))
+		pull = SUN4I_PINCTRL_NO_PULL;
+	else if (fdt_get_property(gd->fdt_blob, offset, "bias-pull-down", NULL))
+		pull = SUN4I_PINCTRL_PULL_DOWN;
+	else
+		pull = fdt_getprop_u32_default_node(gd->fdt_blob, offset, 0,
+						    "allwinner,pull", 0);
 	for (i = 0; ; i++) {
 		int pin;
 
 		pin_name = fdt_stringlist_get(gd->fdt_blob, offset,
 					      "allwinner,pins", i, NULL);
-		if (!pin_name)
-			break;
-		if (pin_name[0] != 'P')
-			continue;
-		pin = (pin_name[1] - 'A') << 5;
-		if (pin >= 26 << 5)
+		if (!pin_name) {
+			pin_name = fdt_stringlist_get(gd->fdt_blob, offset,
+						      "pins", i, NULL);
+			if (!pin_name)
+				break;
+		}
+
+		pin = sunxi_name_to_gpio(pin_name);
+		if (pin < 0)
 			continue;
-		pin += simple_strtol(&pin_name[2], NULL, 10);
 
 		sunxi_gpio_set_cfgpin(pin, SUN8I_GPD8_GMAC);
-		sunxi_gpio_set_drv(pin, drive);
-		sunxi_gpio_set_pull(pin, pull);
+		if (drive != ~0)
+			sunxi_gpio_set_drv(pin, drive);
+		if (pull != ~0)
+			sunxi_gpio_set_pull(pin, pull);
 	}
 
 	if (!i) {
-		printf("WARNING: emac: cannot find allwinner,pins property\n");
+		printf("WARNING: emac: cannot find pins property\n");
 		return -2;
 	}
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 3/3] net: sun8i-emac: add support for new EMAC DT binding
  2018-01-29  1:15 [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Andre Przywara
  2018-01-29  1:15 ` [U-Boot] [PATCH 1/3] sunxi: gpio: add missing compatible strings Andre Przywara
  2018-01-29  1:15 ` [U-Boot] [PATCH 2/3] net: sun8i-emac: support new pinctrl DT bindings Andre Przywara
@ 2018-01-29  1:15 ` Andre Przywara
  2018-01-29  8:51 ` [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Maxime Ripard
  3 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2018-01-29  1:15 UTC (permalink / raw)
  To: u-boot

The Ethernet MAC used in newer Allwinner SoCs (H3, A64, H5) got an
upstream Linux driver in v4.15.
This one uses a slightly different binding from the original one used
by the U-Boot driver.
The differences to the old binding are:
- The "syscon" address is held in a separate node, referenced via a
  phandle in the "syscon" property.
- The reference to the PHY is held in a property called "phy-handle",
  not "phy".
- The PHY register is at offset 0x30 in the syscon device, not at 0.
- The internal PHY is activated when the node, which phy-handle points
  to, is a child node of an "allwinner,sun8i-h3-mdio-internal" node.

Teach the U-Boot driver how to find its resources in a "new-style" DT,
so that we can use a Linux kernel compatible DT for U-Boot as well.
This keeps support for the old binding (for now?), to allow a smooth
transition.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/net/sun8i_emac.c | 55 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 7 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index df2b857310..4ba8959239 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -279,7 +279,7 @@ static int sun8i_emac_set_syscon(struct emac_eth_dev *priv)
 	int ret;
 	u32 reg;
 
-	reg = readl(priv->sysctl_reg);
+	reg = readl(priv->sysctl_reg + 0x30);
 
 	if (priv->variant == H3_EMAC) {
 		ret = sun8i_emac_set_syscon_ephy(priv, &reg);
@@ -310,7 +310,7 @@ static int sun8i_emac_set_syscon(struct emac_eth_dev *priv)
 		return -EINVAL;
 	}
 
-	writel(reg, priv->sysctl_reg);
+	writel(reg, priv->sysctl_reg + 0x30);
 
 	return 0;
 }
@@ -806,17 +806,50 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice *dev)
 #endif
 
 	pdata->iobase = devfdt_get_addr_name(dev, "emac");
+	if (pdata->iobase == FDT_ADDR_T_NONE)
+		pdata->iobase = devfdt_get_addr(dev);
+	if (pdata->iobase == FDT_ADDR_T_NONE) {
+		debug("%s: Cannot find MAC base address\n", __func__);
+		return -EINVAL;
+	}
+
 	priv->sysctl_reg = devfdt_get_addr_name(dev, "syscon");
+	if (priv->sysctl_reg == FDT_ADDR_T_NONE) {
+		const fdt32_t *reg;
+
+		offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "syscon");
+		if (offset < 0) {
+			debug("%s: cannot find syscon node\n", __func__);
+			return -EINVAL;
+		}
+		reg = fdt_getprop(gd->fdt_blob, offset, "reg", NULL);
+		if (!reg) {
+			debug("%s: cannot find reg property in syscon node\n",
+			      __func__);
+			return -EINVAL;
+		}
+		priv->sysctl_reg = fdt_translate_address((void *)gd->fdt_blob,
+							 offset, reg);
+	} else
+		priv->sysctl_reg -= 0x30;
+	if (priv->sysctl_reg == FDT_ADDR_T_NONE) {
+		debug("%s: Cannot find syscon base address\n", __func__);
+		return -EINVAL;
+	}
 
 	pdata->phy_interface = -1;
 	priv->phyaddr = -1;
 	priv->use_internal_phy = false;
 
-	offset = fdtdec_lookup_phandle(gd->fdt_blob, node,
-				       "phy");
-	if (offset > 0)
-		priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg",
-					       -1);
+	offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy");
+	if (offset < 0)
+		offset = fdtdec_lookup_phandle(gd->fdt_blob, node,
+					       "phy-handle");
+	if (offset < 0) {
+		debug("%s: Cannot find PHY address\n", __func__);
+		return -EINVAL;
+	}
+	priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
 
 	phy_mode = fdt_getprop(gd->fdt_blob, node, "phy-mode", NULL);
 
@@ -841,6 +874,14 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice *dev)
 		if (fdt_getprop(gd->fdt_blob, node,
 				"allwinner,use-internal-phy", NULL))
 			priv->use_internal_phy = true;
+		else {
+			int parent = fdt_parent_offset(gd->fdt_blob, offset);
+
+			if (parent >= 0 &&
+			    !fdt_node_check_compatible(gd->fdt_blob, parent,
+					"allwinner,sun8i-h3-mdio-internal"))
+				priv->use_internal_phy = true;
+		}
 	}
 
 	priv->interface = pdata->phy_interface;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29  1:15 [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Andre Przywara
                   ` (2 preceding siblings ...)
  2018-01-29  1:15 ` [U-Boot] [PATCH 3/3] net: sun8i-emac: add support for new EMAC DT binding Andre Przywara
@ 2018-01-29  8:51 ` Maxime Ripard
  2018-01-29  9:44   ` Andre Przywara
  3 siblings, 1 reply; 18+ messages in thread
From: Maxime Ripard @ 2018-01-29  8:51 UTC (permalink / raw)
  To: u-boot

Hi,

On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
> which matched our own DTs. Now that the Linux kernel got a driver, lets
> update our probe code to handle those Linux DTs as well.
> The first patch adds the missing compatible strings for the pinctrl drivers,
> which is needed for using the sunxi_name_to_gpio() lookup function.
> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
> with the new, generic Allwinner pinctrl bindings.
> The final patch extends the probe routine in the Ethernet driver to deal
> with both the old and the new bindings.

Thanks for posting this

> This series allows to copy in the DTs from the latest kernel. Unfortunately
> right now updating the DTs for the H5 and A64 breaks the build, as the
> resulting binary (which embeds the DT) gets to large and triggers our new
> image size check.

Sigh...

> As the H5 and H3 share most of the DT, we can't just update the H3
> DTs either. Hopefully we find some neat trick to work around that.

Is it just because of the DT size, or because there's more code?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180129/1b59b7de/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29  8:51 ` [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Maxime Ripard
@ 2018-01-29  9:44   ` Andre Przywara
  2018-01-29  9:58     ` Maxime Ripard
  0 siblings, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2018-01-29  9:44 UTC (permalink / raw)
  To: u-boot

Hi,

On 29/01/18 08:51, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
>> which matched our own DTs. Now that the Linux kernel got a driver, lets
>> update our probe code to handle those Linux DTs as well.
>> The first patch adds the missing compatible strings for the pinctrl drivers,
>> which is needed for using the sunxi_name_to_gpio() lookup function.
>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
>> with the new, generic Allwinner pinctrl bindings.
>> The final patch extends the probe routine in the Ethernet driver to deal
>> with both the old and the new bindings.
> 
> Thanks for posting this
> 
>> This series allows to copy in the DTs from the latest kernel. Unfortunately
>> right now updating the DTs for the H5 and A64 breaks the build, as the
>> resulting binary (which embeds the DT) gets to large and triggers our new
>> image size check.
> 
> Sigh...
> 
>> As the H5 and H3 share most of the DT, we can't just update the H3
>> DTs either. Hopefully we find some neat trick to work around that.
> 
> Is it just because of the DT size, or because there's more code?

My impression the code itself is always growing a tiny bit over the
weeks, but this time around it's really the DT update.
The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
Similar for the H5: going from 9.5KB to 14.5KB.

Since you did a pretty good job already in identifying the code hogs, I
couldn't find *easy* mitigations over the weekend.
One possible fix is to remove the second .dtb in the Pine64 case, for
which I sent a patch Friday night.
Another thing that stuck out is the sha256 checksum. It's "default y" if
you have FIT. We need FIT for the SPL loader - but we don't do or need
the checksum there.
Some people do FIT loading for the kernel and initrd in U-Boot proper, I
suppose, but I am not sure how many depend on SHA256 checksums in their
images.
Disabling this buys us another 8KB, which would more than offset the DT
grow.

What do you think?

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29  9:44   ` Andre Przywara
@ 2018-01-29  9:58     ` Maxime Ripard
  2018-01-29 10:02       ` [U-Boot] [linux-sunxi] " Chen-Yu Tsai
  2018-01-29 10:38       ` [U-Boot] " Andre Przywara
  0 siblings, 2 replies; 18+ messages in thread
From: Maxime Ripard @ 2018-01-29  9:58 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
> Hi,
> 
> On 29/01/18 08:51, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
> >> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
> >> which matched our own DTs. Now that the Linux kernel got a driver, lets
> >> update our probe code to handle those Linux DTs as well.
> >> The first patch adds the missing compatible strings for the pinctrl drivers,
> >> which is needed for using the sunxi_name_to_gpio() lookup function.
> >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
> >> with the new, generic Allwinner pinctrl bindings.
> >> The final patch extends the probe routine in the Ethernet driver to deal
> >> with both the old and the new bindings.
> > 
> > Thanks for posting this
> > 
> >> This series allows to copy in the DTs from the latest kernel. Unfortunately
> >> right now updating the DTs for the H5 and A64 breaks the build, as the
> >> resulting binary (which embeds the DT) gets to large and triggers our new
> >> image size check.
> > 
> > Sigh...
> > 
> >> As the H5 and H3 share most of the DT, we can't just update the H3
> >> DTs either. Hopefully we find some neat trick to work around that.
> > 
> > Is it just because of the DT size, or because there's more code?
> 
> My impression the code itself is always growing a tiny bit over the
> weeks, but this time around it's really the DT update.
> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
> Similar for the H5: going from 9.5KB to 14.5KB.
> 
> Since you did a pretty good job already in identifying the code hogs, I
> couldn't find *easy* mitigations over the weekend.
> One possible fix is to remove the second .dtb in the Pine64 case, for
> which I sent a patch Friday night.

Since the DT is fed to the C preprocessor, we could also put some
#ifdef 0 around the nodes that are never used by U-Boot (like the
clocks, timer, psci, dma, GIC, RTC, RSB, etc.)

This should give us some room.

> Another thing that stuck out is the sha256 checksum. It's "default y" if
> you have FIT. We need FIT for the SPL loader - but we don't do or need
> the checksum there.
> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
> suppose, but I am not sure how many depend on SHA256 checksums in their
> images.

I think there was someone (Tom?) that said that it was useful in some
circumstances?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180129/9c54a709/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29  9:58     ` Maxime Ripard
@ 2018-01-29 10:02       ` Chen-Yu Tsai
  2018-01-29 10:24         ` Maxime Ripard
  2018-01-29 10:38       ` [U-Boot] " Andre Przywara
  1 sibling, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2018-01-29 10:02 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 29, 2018 at 5:58 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
>> Hi,
>>
>> On 29/01/18 08:51, Maxime Ripard wrote:
>> > Hi,
>> >
>> > On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
>> >> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
>> >> which matched our own DTs. Now that the Linux kernel got a driver, lets
>> >> update our probe code to handle those Linux DTs as well.
>> >> The first patch adds the missing compatible strings for the pinctrl drivers,
>> >> which is needed for using the sunxi_name_to_gpio() lookup function.
>> >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
>> >> with the new, generic Allwinner pinctrl bindings.
>> >> The final patch extends the probe routine in the Ethernet driver to deal
>> >> with both the old and the new bindings.
>> >
>> > Thanks for posting this
>> >
>> >> This series allows to copy in the DTs from the latest kernel. Unfortunately
>> >> right now updating the DTs for the H5 and A64 breaks the build, as the
>> >> resulting binary (which embeds the DT) gets to large and triggers our new
>> >> image size check.
>> >
>> > Sigh...
>> >
>> >> As the H5 and H3 share most of the DT, we can't just update the H3
>> >> DTs either. Hopefully we find some neat trick to work around that.
>> >
>> > Is it just because of the DT size, or because there's more code?
>>
>> My impression the code itself is always growing a tiny bit over the
>> weeks, but this time around it's really the DT update.
>> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
>> Similar for the H5: going from 9.5KB to 14.5KB.
>>
>> Since you did a pretty good job already in identifying the code hogs, I
>> couldn't find *easy* mitigations over the weekend.
>> One possible fix is to remove the second .dtb in the Pine64 case, for
>> which I sent a patch Friday night.
>
> Since the DT is fed to the C preprocessor, we could also put some
> #ifdef 0 around the nodes that are never used by U-Boot (like the
> clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
>
> This should give us some room.

Another possibility is trimming the device tree of unneeded blocks,
possibly automatically by a tool. I think this was raised before?

ChenYu

>> Another thing that stuck out is the sha256 checksum. It's "default y" if
>> you have FIT. We need FIT for the SPL loader - but we don't do or need
>> the checksum there.
>> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
>> suppose, but I am not sure how many depend on SHA256 checksums in their
>> images.
>
> I think there was someone (Tom?) that said that it was useful in some
> circumstances?
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29 10:02       ` [U-Boot] [linux-sunxi] " Chen-Yu Tsai
@ 2018-01-29 10:24         ` Maxime Ripard
  0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2018-01-29 10:24 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 29, 2018 at 06:02:45PM +0800, Chen-Yu Tsai wrote:
> On Mon, Jan 29, 2018 at 5:58 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
> >> Hi,
> >>
> >> On 29/01/18 08:51, Maxime Ripard wrote:
> >> > Hi,
> >> >
> >> > On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
> >> >> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
> >> >> which matched our own DTs. Now that the Linux kernel got a driver, lets
> >> >> update our probe code to handle those Linux DTs as well.
> >> >> The first patch adds the missing compatible strings for the pinctrl drivers,
> >> >> which is needed for using the sunxi_name_to_gpio() lookup function.
> >> >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
> >> >> with the new, generic Allwinner pinctrl bindings.
> >> >> The final patch extends the probe routine in the Ethernet driver to deal
> >> >> with both the old and the new bindings.
> >> >
> >> > Thanks for posting this
> >> >
> >> >> This series allows to copy in the DTs from the latest kernel. Unfortunately
> >> >> right now updating the DTs for the H5 and A64 breaks the build, as the
> >> >> resulting binary (which embeds the DT) gets to large and triggers our new
> >> >> image size check.
> >> >
> >> > Sigh...
> >> >
> >> >> As the H5 and H3 share most of the DT, we can't just update the H3
> >> >> DTs either. Hopefully we find some neat trick to work around that.
> >> >
> >> > Is it just because of the DT size, or because there's more code?
> >>
> >> My impression the code itself is always growing a tiny bit over the
> >> weeks, but this time around it's really the DT update.
> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
> >> Similar for the H5: going from 9.5KB to 14.5KB.
> >>
> >> Since you did a pretty good job already in identifying the code hogs, I
> >> couldn't find *easy* mitigations over the weekend.
> >> One possible fix is to remove the second .dtb in the Pine64 case, for
> >> which I sent a patch Friday night.
> >
> > Since the DT is fed to the C preprocessor, we could also put some
> > #ifdef 0 around the nodes that are never used by U-Boot (like the
> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
> >
> > This should give us some room.
> 
> Another possibility is trimming the device tree of unneeded blocks,
> possibly automatically by a tool. I think this was raised before?

Yes, but it turns out that it's actually hard, since you have no idea
with the overlays if the block is still going to be unneeded at
runtime.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180129/67774b2c/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29  9:58     ` Maxime Ripard
  2018-01-29 10:02       ` [U-Boot] [linux-sunxi] " Chen-Yu Tsai
@ 2018-01-29 10:38       ` Andre Przywara
  2018-01-31  8:21         ` Maxime Ripard
  1 sibling, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2018-01-29 10:38 UTC (permalink / raw)
  To: u-boot

Hi,

On 29/01/18 09:58, Maxime Ripard wrote:
> On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
>> Hi,
>>
>> On 29/01/18 08:51, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
>>>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
>>>> which matched our own DTs. Now that the Linux kernel got a driver, lets
>>>> update our probe code to handle those Linux DTs as well.
>>>> The first patch adds the missing compatible strings for the pinctrl drivers,
>>>> which is needed for using the sunxi_name_to_gpio() lookup function.
>>>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
>>>> with the new, generic Allwinner pinctrl bindings.
>>>> The final patch extends the probe routine in the Ethernet driver to deal
>>>> with both the old and the new bindings.
>>>
>>> Thanks for posting this
>>>
>>>> This series allows to copy in the DTs from the latest kernel. Unfortunately
>>>> right now updating the DTs for the H5 and A64 breaks the build, as the
>>>> resulting binary (which embeds the DT) gets to large and triggers our new
>>>> image size check.
>>>
>>> Sigh...
>>>
>>>> As the H5 and H3 share most of the DT, we can't just update the H3
>>>> DTs either. Hopefully we find some neat trick to work around that.
>>>
>>> Is it just because of the DT size, or because there's more code?
>>
>> My impression the code itself is always growing a tiny bit over the
>> weeks, but this time around it's really the DT update.
>> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
>> Similar for the H5: going from 9.5KB to 14.5KB.
>>
>> Since you did a pretty good job already in identifying the code hogs, I
>> couldn't find *easy* mitigations over the weekend.
>> One possible fix is to remove the second .dtb in the Pine64 case, for
>> which I sent a patch Friday night.
> 
> Since the DT is fed to the C preprocessor, we could also put some
> #ifdef 0 around the nodes that are never used by U-Boot (like the
> clocks, timer, psci, dma, GIC, RTC, RSB, etc.)

Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think
/aliases, /chosen, the reg of USB and Ethernet). But to be honest I
don't want to go there. First it would be a constant churn to keep this
up-to-date, but more importantly for proper UEFI boot we just reuse
U-Boot's .dtb to pass it on to the kernel. That is actually the purpose
of this whole exercise. That already works today (at least for A64), but
would benefit from some updates.

So I would refrain from tinkering with U-Boot's .dtbs.

> This should give us some room.
> 
>> Another thing that stuck out is the sha256 checksum. It's "default y" if
>> you have FIT. We need FIT for the SPL loader - but we don't do or need
>> the checksum there.
>> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
>> suppose, but I am not sure how many depend on SHA256 checksums in their
>> images.
> 
> I think there was someone (Tom?) that said that it was useful in some
> circumstances?

Yes, I clearly see that it is *useful*, but I wonder how many people
would actually miss it today? We would bring it back once we dumped
ENV_IS_IN_MMC, so it's only temporarily.
I think we can just disable it in some defconfigs, to avoid collateral
damage to other boards.
If people have a special need, they can always disable the MMC env and
enable stuff at their likings, it's just the standard "make
.._defconfig; make" process that needs to be fixed with some band-aids
for now.

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-29 10:38       ` [U-Boot] " Andre Przywara
@ 2018-01-31  8:21         ` Maxime Ripard
  2018-01-31  8:29           ` [U-Boot] [linux-sunxi] " Julian Calaby
  2018-01-31 13:40           ` [U-Boot] " Andre Przywara
  0 siblings, 2 replies; 18+ messages in thread
From: Maxime Ripard @ 2018-01-31  8:21 UTC (permalink / raw)
  To: u-boot

Hi,

On Mon, Jan 29, 2018 at 10:38:25AM +0000, Andre Przywara wrote:
> On 29/01/18 09:58, Maxime Ripard wrote:
> > On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
> >> On 29/01/18 08:51, Maxime Ripard wrote:
> >>> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
> >>>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
> >>>> which matched our own DTs. Now that the Linux kernel got a driver, lets
> >>>> update our probe code to handle those Linux DTs as well.
> >>>> The first patch adds the missing compatible strings for the pinctrl drivers,
> >>>> which is needed for using the sunxi_name_to_gpio() lookup function.
> >>>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
> >>>> with the new, generic Allwinner pinctrl bindings.
> >>>> The final patch extends the probe routine in the Ethernet driver to deal
> >>>> with both the old and the new bindings.
> >>>
> >>> Thanks for posting this
> >>>
> >>>> This series allows to copy in the DTs from the latest kernel. Unfortunately
> >>>> right now updating the DTs for the H5 and A64 breaks the build, as the
> >>>> resulting binary (which embeds the DT) gets to large and triggers our new
> >>>> image size check.
> >>>
> >>> Sigh...
> >>>
> >>>> As the H5 and H3 share most of the DT, we can't just update the H3
> >>>> DTs either. Hopefully we find some neat trick to work around that.
> >>>
> >>> Is it just because of the DT size, or because there's more code?
> >>
> >> My impression the code itself is always growing a tiny bit over the
> >> weeks, but this time around it's really the DT update.
> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
> >> Similar for the H5: going from 9.5KB to 14.5KB.
> >>
> >> Since you did a pretty good job already in identifying the code hogs, I
> >> couldn't find *easy* mitigations over the weekend.
> >> One possible fix is to remove the second .dtb in the Pine64 case, for
> >> which I sent a patch Friday night.
> > 
> > Since the DT is fed to the C preprocessor, we could also put some
> > #ifdef 0 around the nodes that are never used by U-Boot (like the
> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
> 
> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think
> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I
> don't want to go there. First it would be a constant churn to keep this
> up-to-date,

I'm not too worried about the churn, it would be there only for the
time until we fully migrate to the FAT environment, so one-two release
now. And we're not syncing the DT very often these days (now that we
have support for the EMAC and USB that is all U-Boot cares about).

> but more importantly for proper UEFI boot we just reuse U-Boot's
> .dtb to pass it on to the kernel. That is actually the purpose of
> this whole exercise. That already works today (at least for A64),
> but would benefit from some updates.
> 
> So I would refrain from tinkering with U-Boot's .dtbs.

That sucks :/

> > This should give us some room.
> > 
> >> Another thing that stuck out is the sha256 checksum. It's "default y" if
> >> you have FIT. We need FIT for the SPL loader - but we don't do or need
> >> the checksum there.
> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
> >> suppose, but I am not sure how many depend on SHA256 checksums in their
> >> images.
> > 
> > I think there was someone (Tom?) that said that it was useful in some
> > circumstances?
> 
> Yes, I clearly see that it is *useful*, but I wonder how many people
> would actually miss it today? We would bring it back once we dumped
> ENV_IS_IN_MMC, so it's only temporarily.

His words were stronger actually, and he said that we want to keep it.

> I think we can just disable it in some defconfigs, to avoid collateral
> damage to other boards.
> If people have a special need, they can always disable the MMC env and
> enable stuff at their likings, it's just the standard "make
> .._defconfig; make" process that needs to be fixed with some band-aids
> for now.

I really don't want to go down the "let's fix each defconfig when we
find out it broke", this is very likely to be broken with no-one
noticing.

Is this issue happening when you sync the whole DT, and would it break
if you just convert the EMAC binding?

Otherwise, we might try to revive the DTC garbage collection of unused
nodes patches. This would prevent us from using the overlays on such a
DT, but that doesn't like like an unfair tradeoff.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180131/75a38ca3/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-31  8:21         ` Maxime Ripard
@ 2018-01-31  8:29           ` Julian Calaby
  2018-01-31  8:36             ` Maxime Ripard
  2018-01-31 13:40           ` [U-Boot] " Andre Przywara
  1 sibling, 1 reply; 18+ messages in thread
From: Julian Calaby @ 2018-01-31  8:29 UTC (permalink / raw)
  To: u-boot

Hi Maxime,

On Wed, Jan 31, 2018 at 7:21 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Mon, Jan 29, 2018 at 10:38:25AM +0000, Andre Przywara wrote:
>> On 29/01/18 09:58, Maxime Ripard wrote:
>> > On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
>> >> On 29/01/18 08:51, Maxime Ripard wrote:
>> >>> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
>> >>>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
>> >>>> which matched our own DTs. Now that the Linux kernel got a driver, lets
>> >>>> update our probe code to handle those Linux DTs as well.
>> >>>> The first patch adds the missing compatible strings for the pinctrl drivers,
>> >>>> which is needed for using the sunxi_name_to_gpio() lookup function.
>> >>>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
>> >>>> with the new, generic Allwinner pinctrl bindings.
>> >>>> The final patch extends the probe routine in the Ethernet driver to deal
>> >>>> with both the old and the new bindings.
>> >>>
>> >>> Thanks for posting this
>> >>>
>> >>>> This series allows to copy in the DTs from the latest kernel. Unfortunately
>> >>>> right now updating the DTs for the H5 and A64 breaks the build, as the
>> >>>> resulting binary (which embeds the DT) gets to large and triggers our new
>> >>>> image size check.
>> >>>
>> >>> Sigh...
>> >>>
>> >>>> As the H5 and H3 share most of the DT, we can't just update the H3
>> >>>> DTs either. Hopefully we find some neat trick to work around that.
>> >>>
>> >>> Is it just because of the DT size, or because there's more code?
>> >>
>> >> My impression the code itself is always growing a tiny bit over the
>> >> weeks, but this time around it's really the DT update.
>> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
>> >> Similar for the H5: going from 9.5KB to 14.5KB.
>> >>
>> >> Since you did a pretty good job already in identifying the code hogs, I
>> >> couldn't find *easy* mitigations over the weekend.
>> >> One possible fix is to remove the second .dtb in the Pine64 case, for
>> >> which I sent a patch Friday night.
>> >
>> > Since the DT is fed to the C preprocessor, we could also put some
>> > #ifdef 0 around the nodes that are never used by U-Boot (like the
>> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
>>
>> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think
>> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I
>> don't want to go there. First it would be a constant churn to keep this
>> up-to-date,
>
> I'm not too worried about the churn, it would be there only for the
> time until we fully migrate to the FAT environment, so one-two release
> now. And we're not syncing the DT very often these days (now that we
> have support for the EMAC and USB that is all U-Boot cares about).
>
>> but more importantly for proper UEFI boot we just reuse U-Boot's
>> .dtb to pass it on to the kernel. That is actually the purpose of
>> this whole exercise. That already works today (at least for A64),
>> but would benefit from some updates.
>>
>> So I would refrain from tinkering with U-Boot's .dtbs.
>
> That sucks :/
>
>> > This should give us some room.
>> >
>> >> Another thing that stuck out is the sha256 checksum. It's "default y" if
>> >> you have FIT. We need FIT for the SPL loader - but we don't do or need
>> >> the checksum there.
>> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
>> >> suppose, but I am not sure how many depend on SHA256 checksums in their
>> >> images.
>> >
>> > I think there was someone (Tom?) that said that it was useful in some
>> > circumstances?
>>
>> Yes, I clearly see that it is *useful*, but I wonder how many people
>> would actually miss it today? We would bring it back once we dumped
>> ENV_IS_IN_MMC, so it's only temporarily.
>
> His words were stronger actually, and he said that we want to keep it.
>
>> I think we can just disable it in some defconfigs, to avoid collateral
>> damage to other boards.
>> If people have a special need, they can always disable the MMC env and
>> enable stuff at their likings, it's just the standard "make
>> .._defconfig; make" process that needs to be fixed with some band-aids
>> for now.
>
> I really don't want to go down the "let's fix each defconfig when we
> find out it broke", this is very likely to be broken with no-one
> noticing.
>
> Is this issue happening when you sync the whole DT, and would it break
> if you just convert the EMAC binding?
>
> Otherwise, we might try to revive the DTC garbage collection of unused
> nodes patches. This would prevent us from using the overlays on such a
> DT, but that doesn't like like an unfair tradeoff.

Stupid question:

As I understand it, the boot process is SPL => Full U-Boot => Linux.

Would it therefore be possible to use a cut-down DT for the SPL (just
the bits it cares about) then use a full one afterwards?

I'm guessing that the SPL wants to patch the DT we pass to Linux,
would we be able to handle that using overlays?

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-31  8:29           ` [U-Boot] [linux-sunxi] " Julian Calaby
@ 2018-01-31  8:36             ` Maxime Ripard
  2018-01-31  9:07               ` Julian Calaby
  0 siblings, 1 reply; 18+ messages in thread
From: Maxime Ripard @ 2018-01-31  8:36 UTC (permalink / raw)
  To: u-boot

Hi Julian,

On Wed, Jan 31, 2018 at 07:29:13PM +1100, Julian Calaby wrote:
> Hi Maxime,
> 
> On Wed, Jan 31, 2018 at 7:21 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Mon, Jan 29, 2018 at 10:38:25AM +0000, Andre Przywara wrote:
> >> On 29/01/18 09:58, Maxime Ripard wrote:
> >> > On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
> >> >> On 29/01/18 08:51, Maxime Ripard wrote:
> >> >>> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
> >> >>>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
> >> >>>> which matched our own DTs. Now that the Linux kernel got a driver, lets
> >> >>>> update our probe code to handle those Linux DTs as well.
> >> >>>> The first patch adds the missing compatible strings for the pinctrl drivers,
> >> >>>> which is needed for using the sunxi_name_to_gpio() lookup function.
> >> >>>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
> >> >>>> with the new, generic Allwinner pinctrl bindings.
> >> >>>> The final patch extends the probe routine in the Ethernet driver to deal
> >> >>>> with both the old and the new bindings.
> >> >>>
> >> >>> Thanks for posting this
> >> >>>
> >> >>>> This series allows to copy in the DTs from the latest kernel. Unfortunately
> >> >>>> right now updating the DTs for the H5 and A64 breaks the build, as the
> >> >>>> resulting binary (which embeds the DT) gets to large and triggers our new
> >> >>>> image size check.
> >> >>>
> >> >>> Sigh...
> >> >>>
> >> >>>> As the H5 and H3 share most of the DT, we can't just update the H3
> >> >>>> DTs either. Hopefully we find some neat trick to work around that.
> >> >>>
> >> >>> Is it just because of the DT size, or because there's more code?
> >> >>
> >> >> My impression the code itself is always growing a tiny bit over the
> >> >> weeks, but this time around it's really the DT update.
> >> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
> >> >> Similar for the H5: going from 9.5KB to 14.5KB.
> >> >>
> >> >> Since you did a pretty good job already in identifying the code hogs, I
> >> >> couldn't find *easy* mitigations over the weekend.
> >> >> One possible fix is to remove the second .dtb in the Pine64 case, for
> >> >> which I sent a patch Friday night.
> >> >
> >> > Since the DT is fed to the C preprocessor, we could also put some
> >> > #ifdef 0 around the nodes that are never used by U-Boot (like the
> >> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
> >>
> >> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think
> >> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I
> >> don't want to go there. First it would be a constant churn to keep this
> >> up-to-date,
> >
> > I'm not too worried about the churn, it would be there only for the
> > time until we fully migrate to the FAT environment, so one-two release
> > now. And we're not syncing the DT very often these days (now that we
> > have support for the EMAC and USB that is all U-Boot cares about).
> >
> >> but more importantly for proper UEFI boot we just reuse U-Boot's
> >> .dtb to pass it on to the kernel. That is actually the purpose of
> >> this whole exercise. That already works today (at least for A64),
> >> but would benefit from some updates.
> >>
> >> So I would refrain from tinkering with U-Boot's .dtbs.
> >
> > That sucks :/
> >
> >> > This should give us some room.
> >> >
> >> >> Another thing that stuck out is the sha256 checksum. It's "default y" if
> >> >> you have FIT. We need FIT for the SPL loader - but we don't do or need
> >> >> the checksum there.
> >> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
> >> >> suppose, but I am not sure how many depend on SHA256 checksums in their
> >> >> images.
> >> >
> >> > I think there was someone (Tom?) that said that it was useful in some
> >> > circumstances?
> >>
> >> Yes, I clearly see that it is *useful*, but I wonder how many people
> >> would actually miss it today? We would bring it back once we dumped
> >> ENV_IS_IN_MMC, so it's only temporarily.
> >
> > His words were stronger actually, and he said that we want to keep it.
> >
> >> I think we can just disable it in some defconfigs, to avoid collateral
> >> damage to other boards.
> >> If people have a special need, they can always disable the MMC env and
> >> enable stuff at their likings, it's just the standard "make
> >> .._defconfig; make" process that needs to be fixed with some band-aids
> >> for now.
> >
> > I really don't want to go down the "let's fix each defconfig when we
> > find out it broke", this is very likely to be broken with no-one
> > noticing.
> >
> > Is this issue happening when you sync the whole DT, and would it break
> > if you just convert the EMAC binding?
> >
> > Otherwise, we might try to revive the DTC garbage collection of unused
> > nodes patches. This would prevent us from using the overlays on such a
> > DT, but that doesn't like like an unfair tradeoff.
> 
> Stupid question:

It's not really stupid :)

> As I understand it, the boot process is SPL => Full U-Boot => Linux.
> 
> Would it therefore be possible to use a cut-down DT for the SPL (just
> the bits it cares about) then use a full one afterwards?

The thing is, we're not using the DT for the SPL, and the DT size
we're discussing about is the one in the main U-Boot binary.

> I'm guessing that the SPL wants to patch the DT we pass to Linux,
> would we be able to handle that using overlays?

In a "standard" setup (or at least the one you described), U-Boot will
patch, or apply the overlay to, the DT provided by Linux, not its own,
so even if we prevent the overlay usage on U-Boot's own, the only
downside would be that the UEFI case Andre was describing would not
work with overlays anymore.

Actually, we're not building U-boot's DT with overlay support anyway
at the moment, so it's not a regression.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180131/8e395383/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-31  8:36             ` Maxime Ripard
@ 2018-01-31  9:07               ` Julian Calaby
  0 siblings, 0 replies; 18+ messages in thread
From: Julian Calaby @ 2018-01-31  9:07 UTC (permalink / raw)
  To: u-boot

Hi Maxime,

On Wed, Jan 31, 2018 at 7:36 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Julian,
>
> On Wed, Jan 31, 2018 at 07:29:13PM +1100, Julian Calaby wrote:
>> Hi Maxime,
>>
>> On Wed, Jan 31, 2018 at 7:21 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Hi,
>> >
>> > On Mon, Jan 29, 2018 at 10:38:25AM +0000, Andre Przywara wrote:
>> >> On 29/01/18 09:58, Maxime Ripard wrote:
>> >> > On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
>> >> >> On 29/01/18 08:51, Maxime Ripard wrote:
>> >> >>> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
>> >> >>>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
>> >> >>>> which matched our own DTs. Now that the Linux kernel got a driver, lets
>> >> >>>> update our probe code to handle those Linux DTs as well.
>> >> >>>> The first patch adds the missing compatible strings for the pinctrl drivers,
>> >> >>>> which is needed for using the sunxi_name_to_gpio() lookup function.
>> >> >>>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
>> >> >>>> with the new, generic Allwinner pinctrl bindings.
>> >> >>>> The final patch extends the probe routine in the Ethernet driver to deal
>> >> >>>> with both the old and the new bindings.
>> >> >>>
>> >> >>> Thanks for posting this
>> >> >>>
>> >> >>>> This series allows to copy in the DTs from the latest kernel. Unfortunately
>> >> >>>> right now updating the DTs for the H5 and A64 breaks the build, as the
>> >> >>>> resulting binary (which embeds the DT) gets to large and triggers our new
>> >> >>>> image size check.
>> >> >>>
>> >> >>> Sigh...
>> >> >>>
>> >> >>>> As the H5 and H3 share most of the DT, we can't just update the H3
>> >> >>>> DTs either. Hopefully we find some neat trick to work around that.
>> >> >>>
>> >> >>> Is it just because of the DT size, or because there's more code?
>> >> >>
>> >> >> My impression the code itself is always growing a tiny bit over the
>> >> >> weeks, but this time around it's really the DT update.
>> >> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
>> >> >> Similar for the H5: going from 9.5KB to 14.5KB.
>> >> >>
>> >> >> Since you did a pretty good job already in identifying the code hogs, I
>> >> >> couldn't find *easy* mitigations over the weekend.
>> >> >> One possible fix is to remove the second .dtb in the Pine64 case, for
>> >> >> which I sent a patch Friday night.
>> >> >
>> >> > Since the DT is fed to the C preprocessor, we could also put some
>> >> > #ifdef 0 around the nodes that are never used by U-Boot (like the
>> >> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
>> >>
>> >> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think
>> >> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I
>> >> don't want to go there. First it would be a constant churn to keep this
>> >> up-to-date,
>> >
>> > I'm not too worried about the churn, it would be there only for the
>> > time until we fully migrate to the FAT environment, so one-two release
>> > now. And we're not syncing the DT very often these days (now that we
>> > have support for the EMAC and USB that is all U-Boot cares about).
>> >
>> >> but more importantly for proper UEFI boot we just reuse U-Boot's
>> >> .dtb to pass it on to the kernel. That is actually the purpose of
>> >> this whole exercise. That already works today (at least for A64),
>> >> but would benefit from some updates.
>> >>
>> >> So I would refrain from tinkering with U-Boot's .dtbs.
>> >
>> > That sucks :/
>> >
>> >> > This should give us some room.
>> >> >
>> >> >> Another thing that stuck out is the sha256 checksum. It's "default y" if
>> >> >> you have FIT. We need FIT for the SPL loader - but we don't do or need
>> >> >> the checksum there.
>> >> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
>> >> >> suppose, but I am not sure how many depend on SHA256 checksums in their
>> >> >> images.
>> >> >
>> >> > I think there was someone (Tom?) that said that it was useful in some
>> >> > circumstances?
>> >>
>> >> Yes, I clearly see that it is *useful*, but I wonder how many people
>> >> would actually miss it today? We would bring it back once we dumped
>> >> ENV_IS_IN_MMC, so it's only temporarily.
>> >
>> > His words were stronger actually, and he said that we want to keep it.
>> >
>> >> I think we can just disable it in some defconfigs, to avoid collateral
>> >> damage to other boards.
>> >> If people have a special need, they can always disable the MMC env and
>> >> enable stuff at their likings, it's just the standard "make
>> >> .._defconfig; make" process that needs to be fixed with some band-aids
>> >> for now.
>> >
>> > I really don't want to go down the "let's fix each defconfig when we
>> > find out it broke", this is very likely to be broken with no-one
>> > noticing.
>> >
>> > Is this issue happening when you sync the whole DT, and would it break
>> > if you just convert the EMAC binding?
>> >
>> > Otherwise, we might try to revive the DTC garbage collection of unused
>> > nodes patches. This would prevent us from using the overlays on such a
>> > DT, but that doesn't like like an unfair tradeoff.
>>
>> Stupid question:
>
> It's not really stupid :)

Well it is, because I completely missed the UEFI case =)

>> As I understand it, the boot process is SPL => Full U-Boot => Linux.
>>
>> Would it therefore be possible to use a cut-down DT for the SPL (just
>> the bits it cares about) then use a full one afterwards?
>
> The thing is, we're not using the DT for the SPL, and the DT size
> we're discussing about is the one in the main U-Boot binary.
>
>> I'm guessing that the SPL wants to patch the DT we pass to Linux,
>> would we be able to handle that using overlays?
>
> In a "standard" setup (or at least the one you described), U-Boot will
> patch, or apply the overlay to, the DT provided by Linux, not its own,
> so even if we prevent the overlay usage on U-Boot's own, the only
> downside would be that the UEFI case Andre was describing would not
> work with overlays anymore.

So essentially I'm proposing a fix for a problem that doesn't exist.

I'll leave it at this.

Thanks for replying,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-31  8:21         ` Maxime Ripard
  2018-01-31  8:29           ` [U-Boot] [linux-sunxi] " Julian Calaby
@ 2018-01-31 13:40           ` Andre Przywara
  2018-02-01  8:26             ` Maxime Ripard
  1 sibling, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2018-01-31 13:40 UTC (permalink / raw)
  To: u-boot

Hi,

On 31/01/18 08:21, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jan 29, 2018 at 10:38:25AM +0000, Andre Przywara wrote:
>> On 29/01/18 09:58, Maxime Ripard wrote:
>>> On Mon, Jan 29, 2018 at 09:44:44AM +0000, Andre Przywara wrote:
>>>> On 29/01/18 08:51, Maxime Ripard wrote:
>>>>> On Mon, Jan 29, 2018 at 01:15:19AM +0000, Andre Przywara wrote:
>>>>>> The existing sun8i-emac driver in U-Boot uses some preliminary bindings,
>>>>>> which matched our own DTs. Now that the Linux kernel got a driver, lets
>>>>>> update our probe code to handle those Linux DTs as well.
>>>>>> The first patch adds the missing compatible strings for the pinctrl drivers,
>>>>>> which is needed for using the sunxi_name_to_gpio() lookup function.
>>>>>> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to cope
>>>>>> with the new, generic Allwinner pinctrl bindings.
>>>>>> The final patch extends the probe routine in the Ethernet driver to deal
>>>>>> with both the old and the new bindings.
>>>>>
>>>>> Thanks for posting this
>>>>>
>>>>>> This series allows to copy in the DTs from the latest kernel. Unfortunately
>>>>>> right now updating the DTs for the H5 and A64 breaks the build, as the
>>>>>> resulting binary (which embeds the DT) gets to large and triggers our new
>>>>>> image size check.
>>>>>
>>>>> Sigh...
>>>>>
>>>>>> As the H5 and H3 share most of the DT, we can't just update the H3
>>>>>> DTs either. Hopefully we find some neat trick to work around that.
>>>>>
>>>>> Is it just because of the DT size, or because there's more code?
>>>>
>>>> My impression the code itself is always growing a tiny bit over the
>>>> weeks, but this time around it's really the DT update.
>>>> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB.
>>>> Similar for the H5: going from 9.5KB to 14.5KB.
>>>>
>>>> Since you did a pretty good job already in identifying the code hogs, I
>>>> couldn't find *easy* mitigations over the weekend.
>>>> One possible fix is to remove the second .dtb in the Pine64 case, for
>>>> which I sent a patch Friday night.
>>>
>>> Since the DT is fed to the C preprocessor, we could also put some
>>> #ifdef 0 around the nodes that are never used by U-Boot (like the
>>> clocks, timer, psci, dma, GIC, RTC, RSB, etc.)
>>
>> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think
>> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I
>> don't want to go there. First it would be a constant churn to keep this
>> up-to-date,
> 
> I'm not too worried about the churn, it would be there only for the
> time until we fully migrate to the FAT environment, so one-two release
> now. And we're not syncing the DT very often these days (now that we
> have support for the EMAC and USB that is all U-Boot cares about).
> 
>> but more importantly for proper UEFI boot we just reuse U-Boot's
>> .dtb to pass it on to the kernel. That is actually the purpose of
>> this whole exercise. That already works today (at least for A64),
>> but would benefit from some updates.
>>
>> So I would refrain from tinkering with U-Boot's .dtbs.
> 
> That sucks :/
> 
>>> This should give us some room.
>>>
>>>> Another thing that stuck out is the sha256 checksum. It's "default y" if
>>>> you have FIT. We need FIT for the SPL loader - but we don't do or need
>>>> the checksum there.
>>>> Some people do FIT loading for the kernel and initrd in U-Boot proper, I
>>>> suppose, but I am not sure how many depend on SHA256 checksums in their
>>>> images.
>>>
>>> I think there was someone (Tom?) that said that it was useful in some
>>> circumstances?
>>
>> Yes, I clearly see that it is *useful*, but I wonder how many people
>> would actually miss it today? We would bring it back once we dumped
>> ENV_IS_IN_MMC, so it's only temporarily.
> 
> His words were stronger actually, and he said that we want to keep it.
> 
>> I think we can just disable it in some defconfigs, to avoid collateral
>> damage to other boards.
>> If people have a special need, they can always disable the MMC env and
>> enable stuff at their likings, it's just the standard "make
>> .._defconfig; make" process that needs to be fixed with some band-aids
>> for now.
> 
> I really don't want to go down the "let's fix each defconfig when we
> find out it broke", this is very likely to be broken with no-one
> noticing.

Yeah, that's probably true. Looks we are really running out of silver
bullets :-(

> Is this issue happening when you sync the whole DT, and would it break
> if you just convert the EMAC binding?
> 
> Otherwise, we might try to revive the DTC garbage collection of unused
> nodes patches. This would prevent us from using the overlays on such a
> DT, but that doesn't like like an unfair tradeoff.

Well, what's the point in updating the DT if we then deviate from it
again? I would rather suggest to postpone the DT update, until we ditch
the MMC environment. The whole reason I wanted the update is to pass it
on to Linux, so garbage collection will kill that purpose.

So what about this plan:
- We need to do something *now* about the breakage of the 64-bit sunxi
*_defconfigs, which do not build with origin/master (for me). I tried
GCC 7.1.0, 7.3.0 and 6.4.0, also the actual merge point of the sunxi
branch in addition to origin/master.
The fix I sent (remove the Pine64 "non-plus" .dtb from the FIT image)
was supposed do the trick, but in contrast to my test on Friday doesn't
work (anymore?). bananapi_m64_defconfig for instance fails also (with a
single .dtb).
Can someone confirm this? Maybe it's worth to see what made U-Boot
proper grow in the last few days/weeks?
- Regardless of this we take the patches from this series, so U-Boot
*can* deal with both DTs.
- We wait with any DT updates until we remove the MMC environment
support, so that we have enough space to give U-Boot the proper .dts.

What is the current plan for the MMC environment? Remove it (or not
enable it by default) in 2018.05?

Having the support for both DT bindings in the EMAC driver *now* would
allow people to build their own firmware, *configuring* out the MMC
environment already and not suffer from any limits anymore. It would
just be the _defconfig build which keeps compatibility, for users who care.

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-01-31 13:40           ` [U-Boot] " Andre Przywara
@ 2018-02-01  8:26             ` Maxime Ripard
  2018-02-01  9:53               ` Andre Przywara
  0 siblings, 1 reply; 18+ messages in thread
From: Maxime Ripard @ 2018-02-01  8:26 UTC (permalink / raw)
  To: u-boot

Hi Andre,

On Wed, Jan 31, 2018 at 01:40:23PM +0000, Andre Przywara wrote:
> > Is this issue happening when you sync the whole DT, and would it break
> > if you just convert the EMAC binding?
> > 
> > Otherwise, we might try to revive the DTC garbage collection of unused
> > nodes patches. This would prevent us from using the overlays on such a
> > DT, but that doesn't like like an unfair tradeoff.
> 
> Well, what's the point in updating the DT if we then deviate from it
> again? I would rather suggest to postpone the DT update, until we ditch
> the MMC environment. The whole reason I wanted the update is to pass it
> on to Linux, so garbage collection will kill that purpose.

Not really, the garbage collection I had in mind is:
https://patchwork.kernel.org/patch/3840371/

ie, removing only nodes marked as such that are not referenced
anywhere in your tree (typically pinctrl nodes). It should totally be
usable from Linux, since the usable part of the DT will remain
untouched.

As I was saying, the only noticeable "glitch" would be that you don't
have nodes in the base DT anymore that used to be there, and overlays
won't be able to apply properly if they depended on those nodes.

However, that's not really bad, because:
  - U-Boot doesn't build its DT with the overlay support anyway
  - Linux in itself is pretty dumb when it comes to applying overlays
    at the moment, so there's not a lot of things you can do. The only
    user in tree (as far as I'm aware) is the FPGA manager, but it
    doesn't really apply to our SoCs.

> So what about this plan:
> - We need to do something *now* about the breakage of the 64-bit sunxi
> *_defconfigs, which do not build with origin/master (for me). I tried
> GCC 7.1.0, 7.3.0 and 6.4.0, also the actual merge point of the sunxi
> branch in addition to origin/master.
> The fix I sent (remove the Pine64 "non-plus" .dtb from the FIT image)
> was supposed do the trick, but in contrast to my test on Friday doesn't
> work (anymore?). bananapi_m64_defconfig for instance fails also (with a
> single .dtb).

Yes, we should definitely merge that patch.

> Can someone confirm this? Maybe it's worth to see what made U-Boot
> proper grow in the last few days/weeks?

I guess I'm (at least partially) to blame with the introduction of the
FAT environment...

How much size do we need?

I've looked at the gains we could have with the dtc patch mentionned
above if we flag all the PIO nodes, and its roughly around 500-800
bytes for an A64 DTS. Would it be enough?

> - Regardless of this we take the patches from this series, so U-Boot
> *can* deal with both DTs.

We should merge those patches, but I don't get why we should maintain
the compatibility with the old bindings. Yes, I know what you are
going to say, but the deal that was made about those bindings when
they were introduced was that we would simply update those bindings
when Linux would have decided.

And not maintain any kind of compatibility. Apparently, that deal has
changed without consulting or notifying anyone. Great.

And since most of that discussion has been around using the U-Boot DT
from Linux, I'm not even sure what the pro would be to keep the old
binding around.

> - We wait with any DT updates until we remove the MMC environment
> support, so that we have enough space to give U-Boot the proper .dts.
> 
> What is the current plan for the MMC environment? Remove it (or not
> enable it by default) in 2018.05?

Not enabling it by default in 2018.05, and removing the migration code
later on seems like a good idea yes.

> Having the support for both DT bindings in the EMAC driver *now* would
> allow people to build their own firmware, *configuring* out the MMC
> environment already and not suffer from any limits anymore.

I understand your frustration, and honestly, it frustrates me even
more to see that the hunt of config option and the discussion we had a
month ago are already rendered useless, and we need to do it again.

> It would just be the _defconfig build which keeps compatibility, for
> users who care.

We do agree on that :)
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180201/56053e42/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-02-01  8:26             ` Maxime Ripard
@ 2018-02-01  9:53               ` Andre Przywara
  2018-02-02 21:42                 ` Maxime Ripard
  0 siblings, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2018-02-01  9:53 UTC (permalink / raw)
  To: u-boot

Hi,

On 01/02/18 08:26, Maxime Ripard wrote:
> Hi Andre,
> 
> On Wed, Jan 31, 2018 at 01:40:23PM +0000, Andre Przywara wrote:
>>> Is this issue happening when you sync the whole DT, and would it break
>>> if you just convert the EMAC binding?
>>>
>>> Otherwise, we might try to revive the DTC garbage collection of unused
>>> nodes patches. This would prevent us from using the overlays on such a
>>> DT, but that doesn't like like an unfair tradeoff.
>>
>> Well, what's the point in updating the DT if we then deviate from it
>> again? I would rather suggest to postpone the DT update, until we ditch
>> the MMC environment. The whole reason I wanted the update is to pass it
>> on to Linux, so garbage collection will kill that purpose.
> 
> Not really, the garbage collection I had in mind is:
> https://patchwork.kernel.org/patch/3840371/

Ah, OK, sorry. Did miss that.

> ie, removing only nodes marked as such that are not referenced
> anywhere in your tree (typically pinctrl nodes). It should totally be
> usable from Linux, since the usable part of the DT will remain
> untouched.
> 
> As I was saying, the only noticeable "glitch" would be that you don't
> have nodes in the base DT anymore that used to be there, and overlays
> won't be able to apply properly if they depended on those nodes.
> 
> However, that's not really bad, because:
>   - U-Boot doesn't build its DT with the overlay support anyway
>   - Linux in itself is pretty dumb when it comes to applying overlays
>     at the moment, so there's not a lot of things you can do. The only
>     user in tree (as far as I'm aware) is the FPGA manager, but it
>     doesn't really apply to our SoCs.

Yeah, that makes sense and would be fine for me. However I am not sure
it will gain us enough to be significant.
My impression is that the DT growth is mostly due to the AXP803 nodes. A
lot of them are unreferenced as well, would that be covered by that
method also?

>> So what about this plan:
>> - We need to do something *now* about the breakage of the 64-bit sunxi
>> *_defconfigs, which do not build with origin/master (for me). I tried
>> GCC 7.1.0, 7.3.0 and 6.4.0, also the actual merge point of the sunxi
>> branch in addition to origin/master.
>> The fix I sent (remove the Pine64 "non-plus" .dtb from the FIT image)
>> was supposed do the trick, but in contrast to my test on Friday doesn't
>> work (anymore?). bananapi_m64_defconfig for instance fails also (with a
>> single .dtb).
> 
> Yes, we should definitely merge that patch.
> 
>> Can someone confirm this? Maybe it's worth to see what made U-Boot
>> proper grow in the last few days/weeks?
> 
> I guess I'm (at least partially) to blame with the introduction of the
> FAT environment...
> 
> How much size do we need?

I saw it being off by roughly 3KB, unfortunately. But that is my
particular setup here, I think that differs with compiler versions and
what not.
Plus I just realised that ATF is of course also part of the .itb, and
more or less by accident I had a slightly bigger version pointed to by
my bl31.bin link (being 5KB bigger than usual).
So sorry for the noise, with the standard 32K ATF image sunxi64 boards
with one .dtb build fine. Well, 936 bytes below the limit ;-).
It's just the Pine64 double .dtb that needs fixing immediately.

> I've looked at the gains we could have with the dtc patch mentionned
> above if we flag all the PIO nodes, and its roughly around 500-800
> bytes for an A64 DTS. Would it be enough?

So not for the issue I reported yesterday, but that seems to be moot now.
That's why I was originally suggesting SHA256, which gives us 8KB, so
some room for the inevitable growth here and there, even after the merge
window. We might keep that in mind for emergency cases ;-)

>> - Regardless of this we take the patches from this series, so U-Boot
>> *can* deal with both DTs.
> 
> We should merge those patches, but I don't get why we should maintain
> the compatibility with the old bindings. Yes, I know what you are
> going to say, but the deal that was made about those bindings when
> they were introduced was that we would simply update those bindings
> when Linux would have decided.
> 
> And not maintain any kind of compatibility. Apparently, that deal has
> changed without consulting or notifying anyone. Great.
> 
> And since most of that discussion has been around using the U-Boot DT
> from Linux, I'm not even sure what the pro would be to keep the old
> binding around.

You could have saved your breath ;-)
I am totally fine with removing support for the old binding. And (in
contrast to Linux!) I would like to consider U-Boot the source of the
.dtb, so we can do changes.
And indeed the old bindings were never agreed upon.

The reason I kept them in is simply to not break bisectability. The
moment you remove the support, you break the board's boot. This is
easily fixed with a follow up patch to update the DT, but unless you
want to squash code and DT changes into one patch, we should keep them,
at least for some commits.
This is especially true if we don't know yet when the DT will be updated.

Solutions I see:
1) We keep support for the old bindings in. Once we merged the DT update
(which could be months away, possibly, due to the size issue), we add a
patch to remove support for the old bindings.
2) We queue those patches, plus update just the nodes affected (EMAC +
respective pinctrl nodes) to the Linux ones *now*. This should not
increase the size of the DT. Then we can immediately add a patch to
remove support for the old binding. So we have this double support in
for just two commits or so.

I guess there is not much to say against 2), I am happy to send patches
for that.

>> - We wait with any DT updates until we remove the MMC environment
>> support, so that we have enough space to give U-Boot the proper .dts.
>>
>> What is the current plan for the MMC environment? Remove it (or not
>> enable it by default) in 2018.05?
> 
> Not enabling it by default in 2018.05, and removing the migration code
> later on seems like a good idea yes.
> 
>> Having the support for both DT bindings in the EMAC driver *now* would
>> allow people to build their own firmware, *configuring* out the MMC
>> environment already and not suffer from any limits anymore.
> 
> I understand your frustration, and honestly, it frustrates me even
> more to see that the hunt of config option and the discussion we had a
> month ago are already rendered useless, and we need to do it again.

I know what you mean!

So quick summary:
- I update this series to add one patch to update just the EMAC DT node,
plus one patch to remove support for the old binding.
- DT sync with Linux is postponed for now until we can afford the size.
- We queue the Pine64 non-plus .dtb patch, to fix the build for
pine64_plus_defconfig.
- We keep an eye on the build size for the next weeks, to spot
regressions, especially before the 2018.03 release.
- We start chilling the champagne for the day MMC env support gets disabled.

Thanks a lot for wading through this mess ;-)

Cheers,
Andre.

>> It would just be the _defconfig build which keeps compatibility, for
>> users who care.
> 
> We do agree on that :)
> Maxime
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
  2018-02-01  9:53               ` Andre Przywara
@ 2018-02-02 21:42                 ` Maxime Ripard
  0 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2018-02-02 21:42 UTC (permalink / raw)
  To: u-boot

Hi,

On Thu, Feb 01, 2018 at 09:53:22AM +0000, Andre Przywara wrote:
> > ie, removing only nodes marked as such that are not referenced
> > anywhere in your tree (typically pinctrl nodes). It should totally be
> > usable from Linux, since the usable part of the DT will remain
> > untouched.
> > 
> > As I was saying, the only noticeable "glitch" would be that you don't
> > have nodes in the base DT anymore that used to be there, and overlays
> > won't be able to apply properly if they depended on those nodes.
> > 
> > However, that's not really bad, because:
> >   - U-Boot doesn't build its DT with the overlay support anyway
> >   - Linux in itself is pretty dumb when it comes to applying overlays
> >     at the moment, so there's not a lot of things you can do. The only
> >     user in tree (as far as I'm aware) is the FPGA manager, but it
> >     doesn't really apply to our SoCs.
> 
> Yeah, that makes sense and would be fine for me. However I am not sure
> it will gain us enough to be significant.
> My impression is that the DT growth is mostly due to the AXP803 nodes. A
> lot of them are unreferenced as well, would that be covered by that
> method also?

Only the nodes marked as such can be removed if they are unreferenced,
and unfortunately, while the pinctrl nodes are harmless to remove, the
AXP nodes have a side effect. Indeed, if the regulator node isn't in
the DT anymore, the kernel will not disable it if it's unused. So I'm
not sure it would be a good idea :/

> >> So what about this plan:
> >> - We need to do something *now* about the breakage of the 64-bit sunxi
> >> *_defconfigs, which do not build with origin/master (for me). I tried
> >> GCC 7.1.0, 7.3.0 and 6.4.0, also the actual merge point of the sunxi
> >> branch in addition to origin/master.
> >> The fix I sent (remove the Pine64 "non-plus" .dtb from the FIT image)
> >> was supposed do the trick, but in contrast to my test on Friday doesn't
> >> work (anymore?). bananapi_m64_defconfig for instance fails also (with a
> >> single .dtb).
> > 
> > Yes, we should definitely merge that patch.
> > 
> >> Can someone confirm this? Maybe it's worth to see what made U-Boot
> >> proper grow in the last few days/weeks?
> > 
> > I guess I'm (at least partially) to blame with the introduction of the
> > FAT environment...
> > 
> > How much size do we need?
> 
> I saw it being off by roughly 3KB, unfortunately. But that is my
> particular setup here, I think that differs with compiler versions and
> what not.
> Plus I just realised that ATF is of course also part of the .itb, and
> more or less by accident I had a slightly bigger version pointed to by
> my bl31.bin link (being 5KB bigger than usual).
> So sorry for the noise, with the standard 32K ATF image sunxi64 boards
> with one .dtb build fine. Well, 936 bytes below the limit ;-).

Ok, good. I guess one of the related discussion would be to add the
bl31 binary to our travis-ci build, so that we actually catch those
kind of issues.

> It's just the Pine64 double .dtb that needs fixing immediately.

Ack. Consider it merged.

> > I've looked at the gains we could have with the dtc patch mentionned
> > above if we flag all the PIO nodes, and its roughly around 500-800
> > bytes for an A64 DTS. Would it be enough?
> 
> So not for the issue I reported yesterday, but that seems to be moot now.
> That's why I was originally suggesting SHA256, which gives us 8KB, so
> some room for the inevitable growth here and there, even after the merge
> window. We might keep that in mind for emergency cases ;-)

Yay, two bullets left :)

> >> - Regardless of this we take the patches from this series, so U-Boot
> >> *can* deal with both DTs.
> > 
> > We should merge those patches, but I don't get why we should maintain
> > the compatibility with the old bindings. Yes, I know what you are
> > going to say, but the deal that was made about those bindings when
> > they were introduced was that we would simply update those bindings
> > when Linux would have decided.
> > 
> > And not maintain any kind of compatibility. Apparently, that deal has
> > changed without consulting or notifying anyone. Great.
> > 
> > And since most of that discussion has been around using the U-Boot DT
> > from Linux, I'm not even sure what the pro would be to keep the old
> > binding around.
> 
> You could have saved your breath ;-)
> I am totally fine with removing support for the old binding. And (in
> contrast to Linux!) I would like to consider U-Boot the source of the
> .dtb, so we can do changes.
> And indeed the old bindings were never agreed upon.
> 
> The reason I kept them in is simply to not break bisectability. The
> moment you remove the support, you break the board's boot. This is
> easily fixed with a follow up patch to update the DT, but unless you
> want to squash code and DT changes into one patch, we should keep them,
> at least for some commits.
> This is especially true if we don't know yet when the DT will be updated.
> 
> Solutions I see:
> 1) We keep support for the old bindings in. Once we merged the DT update
> (which could be months away, possibly, due to the size issue), we add a
> patch to remove support for the old bindings.
> 2) We queue those patches, plus update just the nodes affected (EMAC +
> respective pinctrl nodes) to the Linux ones *now*. This should not
> increase the size of the DT. Then we can immediately add a patch to
> remove support for the old binding. So we have this double support in
> for just two commits or so.
> 
> I guess there is not much to say against 2), I am happy to send patches
> for that.

2 definitely works for me

> >> - We wait with any DT updates until we remove the MMC environment
> >> support, so that we have enough space to give U-Boot the proper .dts.
> >>
> >> What is the current plan for the MMC environment? Remove it (or not
> >> enable it by default) in 2018.05?
> > 
> > Not enabling it by default in 2018.05, and removing the migration code
> > later on seems like a good idea yes.
> > 
> >> Having the support for both DT bindings in the EMAC driver *now* would
> >> allow people to build their own firmware, *configuring* out the MMC
> >> environment already and not suffer from any limits anymore.
> > 
> > I understand your frustration, and honestly, it frustrates me even
> > more to see that the hunt of config option and the discussion we had a
> > month ago are already rendered useless, and we need to do it again.
> 
> I know what you mean!
> 
> So quick summary:
> - I update this series to add one patch to update just the EMAC DT node,
> plus one patch to remove support for the old binding.
> - DT sync with Linux is postponed for now until we can afford the size.
> - We queue the Pine64 non-plus .dtb patch, to fix the build for
> pine64_plus_defconfig.
> - We keep an eye on the build size for the next weeks, to spot
> regressions, especially before the 2018.03 release.

Yep

> - We start chilling the champagne for the day MMC env support gets
>   disabled.

It's already in a cool place.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180202/a5a7e5d9/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-02-02 21:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-29  1:15 [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Andre Przywara
2018-01-29  1:15 ` [U-Boot] [PATCH 1/3] sunxi: gpio: add missing compatible strings Andre Przywara
2018-01-29  1:15 ` [U-Boot] [PATCH 2/3] net: sun8i-emac: support new pinctrl DT bindings Andre Przywara
2018-01-29  1:15 ` [U-Boot] [PATCH 3/3] net: sun8i-emac: add support for new EMAC DT binding Andre Przywara
2018-01-29  8:51 ` [U-Boot] [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings Maxime Ripard
2018-01-29  9:44   ` Andre Przywara
2018-01-29  9:58     ` Maxime Ripard
2018-01-29 10:02       ` [U-Boot] [linux-sunxi] " Chen-Yu Tsai
2018-01-29 10:24         ` Maxime Ripard
2018-01-29 10:38       ` [U-Boot] " Andre Przywara
2018-01-31  8:21         ` Maxime Ripard
2018-01-31  8:29           ` [U-Boot] [linux-sunxi] " Julian Calaby
2018-01-31  8:36             ` Maxime Ripard
2018-01-31  9:07               ` Julian Calaby
2018-01-31 13:40           ` [U-Boot] " Andre Przywara
2018-02-01  8:26             ` Maxime Ripard
2018-02-01  9:53               ` Andre Przywara
2018-02-02 21:42                 ` Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.