mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 15/16] i.MX: Default CONFI_USB_IMX_PHY to 'y' on Vybrid
From: Andrey Smirnov @ 2016-12-05 14:54 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov
In-Reply-To: <1480949684-18520-1-git-send-email-andrew.smirnov@gmail.com>

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/usb/imx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/imx/Kconfig b/drivers/usb/imx/Kconfig
index b0c6a41..8e6f315 100644
--- a/drivers/usb/imx/Kconfig
+++ b/drivers/usb/imx/Kconfig
@@ -16,4 +16,4 @@ config USB_IMX_CHIPIDEA
 
 config USB_IMX_PHY
 	bool
-	default y if ARCH_IMX6 && GENERIC_PHY
+	default y if (ARCH_IMX6 || ARCH_VF610) && GENERIC_PHY
-- 
2.5.5


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply related

* [PATCH 14/16] i.MX: imx-usb-phy: Add VF610 OF compatiblity string
From: Andrey Smirnov @ 2016-12-05 14:54 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov
In-Reply-To: <1480949684-18520-1-git-send-email-andrew.smirnov@gmail.com>

From looking at analogous Linux driver code it seems that all of the
differences between code "imx23-usbphy" and "vf610-usbphy" pertain to
suspend/resume functionality, which shouldn't affetct Barebox. As a
result this commit just adds a compatiblity string and no other code.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/usb/imx/imx-usb-phy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/imx/imx-usb-phy.c b/drivers/usb/imx/imx-usb-phy.c
index 9f46f8d..274153b 100644
--- a/drivers/usb/imx/imx-usb-phy.c
+++ b/drivers/usb/imx/imx-usb-phy.c
@@ -168,6 +168,8 @@ static __maybe_unused struct of_device_id imx_usbphy_dt_ids[] = {
 	{
 		.compatible = "fsl,imx23-usbphy",
 	}, {
+		.compatible = "fsl,vf610-usbphy",
+	}, {
 		/* sentinel */
 	},
 };
-- 
2.5.5


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply related

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Stefan Lengfeld @ 2016-12-05 15:14 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox
In-Reply-To: <1480949684-18520-3-git-send-email-andrew.smirnov@gmail.com>

Hi Andrey,

a similiar patch was posted some days ago to support two MAC addresses for the
i.MX6 UltraLite. See

    http://lists.infradead.org/pipermail/barebox/2016-November/028628.html

Your approach uses an extra device attribute  'mac_idx' to select the meaning
of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1. 

I find it less error prone and confusing to use two seperate device attributes
'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
value of 'mac_idx before reading or writing the MAC addresses.

Both approaches are backwards compatible since the name of 'mac_addr' for the
first MAC address is not changed.

Mit freundlichen Grüßen / Kind regards,
	Stefan Lengfeld

On Mon, Dec 05, 2016 at 06:54:30AM -0800, Andrey Smirnov wrote:
> i.MX SoC variants like Vybrid have more than one built-in Ethernet
> interface and as a consequence support storing more than one MAC address
> in OCOTP module. Add 'mac_idx' variable to allow to select which mac
> address is being referred to by 'mac_addr' variable and the code to
> handle it appropriately.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/mach-imx/ocotp.c | 50 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/ocotp.c b/arch/arm/mach-imx/ocotp.c
> index 68ff0ce..9a07922 100644
> --- a/arch/arm/mach-imx/ocotp.c
> +++ b/arch/arm/mach-imx/ocotp.c
> @@ -69,12 +69,16 @@
>  /* Other definitions */
>  #define IMX6_OTP_DATA_ERROR_VAL		0xBADABADA
>  #define DEF_RELAX			20
> -#define MAC_OFFSET			(0x22 * 4)
> +#define MAC_OFFSET_0			(0x22 * 4)
> +#define MAC_OFFSET_1			(0x24 * 4)
> +#define MAX_MAC_OFFSETS			2
>  #define MAC_BYTES			8
>  
>  struct imx_ocotp_data {
>  	int num_regs;
>  	u32 (*addr_to_offset)(u32 addr);
> +	u8  mac_offsets[MAX_MAC_OFFSETS];
> +	u8  mac_offsets_num;
>  };
>  
>  struct ocotp_priv {
> @@ -87,6 +91,7 @@ struct ocotp_priv {
>  	char ethaddr[6];
>  	struct regmap_config map_config;
>  	const struct imx_ocotp_data *data;
> +	int  mac_offset_idx;
>  };
>  
>  static struct ocotp_priv *imx_ocotp;
> @@ -402,8 +407,10 @@ static int imx_ocotp_get_mac(struct param_d *param, void *priv)
>  	struct ocotp_priv *ocotp_priv = priv;
>  	char buf[8];
>  	int i, ret;
> +	u8  mac_offset;
>  
> -	ret = regmap_bulk_read(ocotp_priv->map, MAC_OFFSET, buf, MAC_BYTES);
> +	mac_offset = ocotp_priv->data->mac_offsets[ocotp_priv->mac_offset_idx];
> +	ret = regmap_bulk_read(ocotp_priv->map, mac_offset, buf, MAC_BYTES);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -418,18 +425,43 @@ static int imx_ocotp_set_mac(struct param_d *param, void *priv)
>  	struct ocotp_priv *ocotp_priv = priv;
>  	char buf[8];
>  	int i, ret;
> +	u8 mac_offset;
> +
> +	mac_offset = ocotp_priv->data->mac_offsets[ocotp_priv->mac_offset_idx];
>  
>  	for (i = 0; i < 6; i++)
>  		buf[5 - i] = ocotp_priv->ethaddr[i];
>  	buf[6] = 0; buf[7] = 0;
>  
> -	ret = regmap_bulk_write(ocotp_priv->map, MAC_OFFSET, buf, MAC_BYTES);
> +	ret = regmap_bulk_write(ocotp_priv->map, mac_offset, buf, MAC_BYTES);
>  	if (ret < 0)
>  		return ret;
>  
>  	return 0;
>  }
>  
> +static int imx_ocotp_set_mac_idx(struct param_d *param, void *priv)
> +{
> +	struct ocotp_priv *ocotp_priv = priv;
> +	const int min = 0;
> +	const int max = ocotp_priv->data->mac_offsets_num - 1;
> +	int old, new, ret = 0;
> +
> +	old = ocotp_priv->mac_offset_idx;
> +	new = clamp(old, min, max);
> +
> +	if (old != new) {
> +		dev_err(&ocotp_priv->dev,
> +			"%d is out of bounds for '%s', clamping to %d\n",
> +			old, param->name, new);
> +		ret = -EINVAL;
> +	}
> +
> +	ocotp_priv->mac_offset_idx = new;
> +
> +	return ret;
> +}
> +
>  static struct regmap_bus imx_ocotp_regmap_bus = {
>  	.reg_write = imx_ocotp_reg_write,
>  	.reg_read = imx_ocotp_reg_read,
> @@ -486,9 +518,13 @@ static int imx_ocotp_probe(struct device_d *dev)
>  				NULL, NULL, &priv->permanent_write_enable, NULL);
>  	}
>  
> -	if (IS_ENABLED(CONFIG_NET))
> +	if (IS_ENABLED(CONFIG_NET)) {
> +		dev_add_param_int(&priv->dev, "mac_idx",
> +				  imx_ocotp_set_mac_idx, NULL,
> +				  &priv->mac_offset_idx, "%d", priv);
>  		dev_add_param_mac(&(priv->dev), "mac_addr", imx_ocotp_set_mac,
>  				imx_ocotp_get_mac, priv->ethaddr, priv);
> +	}
>  
>  	dev_add_param_bool(&(priv->dev), "sense_enable", NULL, NULL, &priv->sense_enable, priv);
>  
> @@ -527,16 +563,22 @@ static u32 vf610_addr_to_offset(u32 addr)
>  static struct imx_ocotp_data imx6q_ocotp_data = {
>  	.num_regs = 512,
>  	.addr_to_offset = imx6q_addr_to_offset,
> +	.mac_offsets_num = 1,
> +	.mac_offsets = { MAC_OFFSET_0 },
>  };
>  
>  static struct imx_ocotp_data imx6sl_ocotp_data = {
>  	.num_regs = 256,
>  	.addr_to_offset = imx6sl_addr_to_offset,
> +	.mac_offsets_num = 1,
> +	.mac_offsets = { MAC_OFFSET_0 },
>  };
>  
>  static struct imx_ocotp_data vf610_ocotp_data = {
>  	.num_regs = 512,
>  	.addr_to_offset = vf610_addr_to_offset,
> +	.mac_offsets_num = 2,
> +	.mac_offsets = { MAC_OFFSET_0, MAC_OFFSET_1 },
>  };
>  
>  static __maybe_unused struct of_device_id imx_ocotp_dt_ids[] = {
> -- 
> 2.5.5
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Andrey Smirnov @ 2016-12-06 14:48 UTC (permalink / raw)
  To: Stefan Lengfeld; +Cc: barebox@lists.infradead.org
In-Reply-To: <20161205151431.GC14158@lws-christ>

Hi Stefan,

On Mon, Dec 5, 2016 at 7:14 AM, Stefan Lengfeld <s.lengfeld@phytec.de> wrote:
> Hi Andrey,
>
> a similiar patch was posted some days ago to support two MAC addresses for the
> i.MX6 UltraLite. See
>
>     http://lists.infradead.org/pipermail/barebox/2016-November/028628.html
>

I did see that patch go through, however since the title of it is
"arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
it any further.


> Your approach uses an extra device attribute  'mac_idx' to select the meaning
> of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
>
> I find it less error prone and confusing to use two seperate device attributes
> 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
> value of 'mac_idx before reading or writing the MAC addresses.

Can't say that I necessarily agree with you assessment. There's no
need to check 'mac_idx' if it is written every time as a part of MAC
address assignment operation. The reason I chose the approach that I
did was mainly because it allowed to both have backwards compatibility
with the old naming scheme and avoid having variable naming
inconsistency (as you see in your example where one variable has a
numerical suffix and the other doesn't) which I was concerned would be
make scripting it more clunky than necessary.

Anyway, I don't think I am impartial enough to make a good judge of
merits of both approaches, so I'll let Sascha decide which way he
wants to go.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Stefan Lengfeld @ 2016-12-07  8:51 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox@lists.infradead.org
In-Reply-To: <CAHQ1cqErON12d4KTdYg614U5i9j8X=_-kD8+g9JtfFgak9MXBA@mail.gmail.com>

Hi Andrey,

> I did see that patch go through, however since the title of it is
> "arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
> it any further.

In the light of the Vybrid also supports two MAC addresses your patch title

    i.MX: ocotp: Add provisions for storing multiple MAC addresses

is better, since it is more generic.

> > Your approach uses an extra device attribute  'mac_idx' to select the meaning
> > of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
> >
> > I find it less error prone and confusing to use two seperate device attributes
> > 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
> > value of 'mac_idx before reading or writing the MAC addresses.
> 
> Can't say that I necessarily agree with you assessment. There's no
> need to check 'mac_idx' if it is written every time as a part of MAC
> address assignment operation. The reason I chose the approach that I
> did was mainly because it allowed to both have backwards compatibility
> with the old naming scheme and avoid having variable naming
> inconsistency (as you see in your example where one variable has a
> numerical suffix and the other doesn't) which I was concerned would be
> make scripting it more clunky than necessary.

hmm, I find it more convenient use

    bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
    bootloader$ ocotp0.mac_addr1=22:33:44:AA:AA:AA

than 

    bootloader$ ocotp0.mac_idx=0
    bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
    bootloader$ ocotp0.mac_idx=1
    bootloader$ ocotp0.mac_addr=22:33:44:AA:AA:AA

The same goes for reading the MAC addresses. The first one is more obvious,
because the meaning of the variable "mac_addr" does not depend on another
variable "mac_idx".

Maybe we can settle on a different approach to avoid the inconsistent variable
names "mac_addr" and "mac_addr1":

	if (IS_ENABLED(CONFIG_NET)) { 
		if (!data->scnd_mac_addr) {
			/* one MAC */
		        dev_add_param_mac(&(priv->dev), "mac_addr", imx_ocotp_set_mac0,
			       imx_ocotp_get_mac0, priv->ethaddr[0], priv);

		} else {
			/* two MACs: Vybrid and UltraLite */
		        dev_add_param_mac(&(priv->dev), "mac_addr0", imx_ocotp_set_mac0,
			       imx_ocotp_get_mac0, priv->ethaddr[0], priv);
                        dev_add_param_mac(&(priv->dev), "mac_addr1", imx_ocotp_set_mac1,
				imx_ocotp_get_mac1, priv->ethaddr[1], priv);
		}
	}

All existing boards still uses "mac_addr" for there single MAC address and new
boards Vybrid and UltraLite use "mac_addr0" and "mac_addr1". In the wild there
will be two different sets of - for example - factory MAC burning scripts
anyway.  One to handle a single MAC address and another to handle two MAC
addresses.

What do you think?

Mit freundlichen Grüßen / Kind regards,
	Stefan Lengfeld

On Tue, Dec 06, 2016 at 06:48:16AM -0800, Andrey Smirnov wrote:
> Hi Stefan,
> 
> On Mon, Dec 5, 2016 at 7:14 AM, Stefan Lengfeld <s.lengfeld@phytec.de> wrote:
> > Hi Andrey,
> >
> > a similiar patch was posted some days ago to support two MAC addresses for the
> > i.MX6 UltraLite. See
> >
> >     http://lists.infradead.org/pipermail/barebox/2016-November/028628.html
> >
> 
> I did see that patch go through, however since the title of it is
> "arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
> it any further.
> 
> 
> > Your approach uses an extra device attribute  'mac_idx' to select the meaning
> > of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
> >
> > I find it less error prone and confusing to use two seperate device attributes
> > 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
> > value of 'mac_idx before reading or writing the MAC addresses.
> 
> Can't say that I necessarily agree with you assessment. There's no
> need to check 'mac_idx' if it is written every time as a part of MAC
> address assignment operation. The reason I chose the approach that I
> did was mainly because it allowed to both have backwards compatibility
> with the old naming scheme and avoid having variable naming
> inconsistency (as you see in your example where one variable has a
> numerical suffix and the other doesn't) which I was concerned would be
> make scripting it more clunky than necessary.
> 
> Anyway, I don't think I am impartial enough to make a good judge of
> merits of both approaches, so I'll let Sascha decide which way he
> wants to go.
> 
> Thanks,
> Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* [PATCH] spi: mvebu: fix baudrate selection for Armada 370/XP
From: Uwe Kleine-König @ 2016-12-07 15:59 UTC (permalink / raw)
  To: barebox

There are two problems that made the driver choose the wrong baudrate
calculation algorithm:

a) The compatible used on 370/XP isn't marvell,armada-370-xp-spi but
   marvell,armada-370-spi or marvell,armada-xp-spi respectively.
b) The probe function uses
   	match = of_match_node(mvebu_spi_dt_ids, dev->device_node);
   to determine the right algorithm. As the devices are also compatible
   to marvell,orion-spi and this comes first in mvebu_spi_dt_ids[]
   it's always the older Orion algorithm that is used.

This patch fixes both problems.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/mvebu_spi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/mvebu_spi.c b/drivers/spi/mvebu_spi.c
index ee7b212962f7..e340686bd676 100644
--- a/drivers/spi/mvebu_spi.c
+++ b/drivers/spi/mvebu_spi.c
@@ -317,16 +317,18 @@ err_transfer:
 }
 
 static struct of_device_id mvebu_spi_dt_ids[] = {
-	{ .compatible = "marvell,orion-spi",
-	  .data = &mvebu_spi_set_baudrate },
 #if defined(CONFIG_ARCH_ARMADA_370) || defined(CONFIG_ARCH_ARMADA_XP)
-	{ .compatible = "marvell,armada-370-xp-spi",
+	{ .compatible = "marvell,armada-370-spi",
+	  .data = &armada_370_xp_spi_set_baudrate },
+	{ .compatible = "marvell,armada-xp-spi",
 	  .data = &armada_370_xp_spi_set_baudrate },
 #endif
 #if defined(CONFIG_ARCH_DOVE)
 	{ .compatible = "marvell,dove-spi",
 	  .data = &dove_spi_set_baudrate },
 #endif
+	{ .compatible = "marvell,orion-spi",
+	  .data = &mvebu_spi_set_baudrate },
 	{ }
 };
 
-- 
2.10.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply related

* Re: [PATCH] spi: mvebu: fix baudrate selection for Armada 370/XP
From: Sascha Hauer @ 2016-12-07 17:21 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox
In-Reply-To: <20161207155900.29320-1-u.kleine-koenig@pengutronix.de>

On Wed, Dec 07, 2016 at 04:59:00PM +0100, Uwe Kleine-König wrote:
> There are two problems that made the driver choose the wrong baudrate
> calculation algorithm:
> 
> a) The compatible used on 370/XP isn't marvell,armada-370-xp-spi but
>    marvell,armada-370-spi or marvell,armada-xp-spi respectively.
> b) The probe function uses
>    	match = of_match_node(mvebu_spi_dt_ids, dev->device_node);
>    to determine the right algorithm. As the devices are also compatible
>    to marvell,orion-spi and this comes first in mvebu_spi_dt_ids[]
>    it's always the older Orion algorithm that is used.
> 
> This patch fixes both problems.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Sascha Hauer @ 2016-12-07 19:13 UTC (permalink / raw)
  To: Stefan Lengfeld; +Cc: Andrey Smirnov, barebox@lists.infradead.org
In-Reply-To: <20161207085105.GC2371@lws-christ>

On Wed, Dec 07, 2016 at 09:51:05AM +0100, Stefan Lengfeld wrote:
> Hi Andrey,
> 
> > I did see that patch go through, however since the title of it is
> > "arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
> > it any further.
> 
> In the light of the Vybrid also supports two MAC addresses your patch title
> 
>     i.MX: ocotp: Add provisions for storing multiple MAC addresses
> 
> is better, since it is more generic.
> 
> > > Your approach uses an extra device attribute  'mac_idx' to select the meaning
> > > of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
> > >
> > > I find it less error prone and confusing to use two seperate device attributes
> > > 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
> > > value of 'mac_idx before reading or writing the MAC addresses.
> > 
> > Can't say that I necessarily agree with you assessment. There's no
> > need to check 'mac_idx' if it is written every time as a part of MAC
> > address assignment operation. The reason I chose the approach that I
> > did was mainly because it allowed to both have backwards compatibility
> > with the old naming scheme and avoid having variable naming
> > inconsistency (as you see in your example where one variable has a
> > numerical suffix and the other doesn't) which I was concerned would be
> > make scripting it more clunky than necessary.
> 
> hmm, I find it more convenient use
> 
>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
>     bootloader$ ocotp0.mac_addr1=22:33:44:AA:AA:AA
> 
> than 
> 
>     bootloader$ ocotp0.mac_idx=0
>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
>     bootloader$ ocotp0.mac_idx=1
>     bootloader$ ocotp0.mac_addr=22:33:44:AA:AA:AA
> 
> The same goes for reading the MAC addresses. The first one is more obvious,
> because the meaning of the variable "mac_addr" does not depend on another
> variable "mac_idx".

I agree here.

> 
> Maybe we can settle on a different approach to avoid the inconsistent variable
> names "mac_addr" and "mac_addr1":

How about always registering mac_addr0 with mac_addr as alias? Then we
have consistent variable naming and still the backward compatible
standard mac address.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 06/16] i.MX: vf610: Ramp CPU clock to maximum frequency
From: Sascha Hauer @ 2016-12-07 19:27 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox
In-Reply-To: <1480949684-18520-7-git-send-email-andrew.smirnov@gmail.com>

On Mon, Dec 05, 2016 at 06:54:34AM -0800, Andrey Smirnov wrote:
> Mask ROM leaves the CPU running at 264Mhz, so configure the clock tree
> to such that CPU runs at maximum supported frequency, based on speed
> grading burned into OCOTP fusebox.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/mach-imx/Kconfig   |  13 +++
>  drivers/clk/imx/clk-vf610.c | 187 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 198 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index af533ea..1752335 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -750,6 +750,19 @@ config HABV3_IMG_CRT_DER
>  
>  endif
>  
> +config ADJUST_CPU_CLOCK
> +       bool "Adjust CPU clock based on its speed grading"
> +       select IMX_OCOTP
> +       depends on ARCH_VF610
> +       default y
> +       help
> +	  Some i.MX SoCs (e. g. Vybrid) are manufactured to have
> +	  several variants of the same chip different only in maxumum

s/maxumum/maximum/

> +	  CPU frequency supported. MaskROM on such chips plays it safe
> +	  and uses the lowest possible frequency. This option
> +	  configures Barebox to read chip's speed grade information
> +	  and increase CPU clock to it's highest possible value.

Why do we need this configurable?

> +
>  endmenu
>  
>  endif
> diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
> index 04cb02f..1cf2b65 100644
> --- a/drivers/clk/imx/clk-vf610.c
> +++ b/drivers/clk/imx/clk-vf610.c
> @@ -16,7 +16,9 @@
>  #include <of_address.h>
>  #include <linux/clkdev.h>
>  #include <linux/clk.h>
> +#include <notifier.h>
>  #include <dt-bindings/clock/vf610-clock.h>
> +#include <mach/vf610-regs.h>
>  
>  #include "clk.h"
>  
> @@ -76,6 +78,7 @@
>  #define PLL6_CTRL		(anatop_base + 0xa0)
>  #define PLL7_CTRL		(anatop_base + 0x20)
>  #define ANA_MISC1		(anatop_base + 0x160)
> +#define PLL_LOCK		(anatop_base + 0x2c0)
>  
>  static void __iomem *anatop_base;
>  static void __iomem *ccm_base;
> @@ -188,8 +191,9 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
>  	clk[VF610_CLK_PLL6_BYPASS_SRC] = imx_clk_mux("pll6_bypass_src", PLL6_CTRL, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
>  	clk[VF610_CLK_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", PLL7_CTRL, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
>  
> -	clk[VF610_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll1", "pll1_bypass_src", PLL1_CTRL, 0x1);
> -	clk[VF610_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", PLL2_CTRL, 0x1);
> +	clk[VF610_CLK_PLL1] = imx_clk_pllv3_locked(IMX_PLLV3_SYS_VF610, "pll1", "pll1_bypass_src", PLL1_CTRL, 0x1, PLL_LOCK, BIT(6));
> +	clk[VF610_CLK_PLL2] = imx_clk_pllv3_locked(IMX_PLLV3_SYS_VF610, "pll2", "pll2_bypass_src", PLL2_CTRL, 0x1, PLL_LOCK, BIT(5));
> +
>  	clk[VF610_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB_VF610,     "pll3", "pll3_bypass_src", PLL3_CTRL, 0x2);
>  	clk[VF610_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV,      "pll4", "pll4_bypass_src", PLL4_CTRL, 0x7f);
>  	clk[VF610_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_ENET,    "pll5", "pll5_bypass_src", PLL5_CTRL, 0x3);
> @@ -441,3 +445,182 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
>  	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>  }
>  CLK_OF_DECLARE(vf610, "fsl,vf610-ccm", vf610_clocks_init);
> +
> +#ifdef CONFIG_ADJUST_CPU_CLOCK
> +
> +enum {
> +	OCOTP_SPEED_GRADING_OFFSET = (0x4 * sizeof(uint32_t)),
> +	OCOTP_SPEED_GRADING_SHIFT  = 18,
> +	OCOTP_SPEED_GRADING_MASK   = 0b1111,
> +
> +	VF610_SPEED_500 = 0b1110,
> +	VF610_SPEED_400 = 0b1001,
> +	VF610_SPEED_266 = 0b0001,
> +
> +	DDRMC_CR117 = 0x01d4,
> +	DDRMC_CR117_AXI0_FITYPEREG_SYNC = 0b01 << 16,
> +};
> +
> +static int vf610_switch_cpu_clock_to_500mhz(void)
> +{
> +	int ret;
> +
> +	/*
> +	 * When switching A5 CPU to 500Mhz we expect DDRC to be
> +	 * clocked by PLL2_PFD2 and the system to be configured in
> +	 * asynchronous mode.
> +	 *
> +	 * We also can't just use default PFD1 output of PLL1 due to
> +	 * Errata e6235, so we have to re-clock the PLL itself and use
> +	 * its output to clock the CPU directly.
> +	 */
> +
> +	if (clk_get_parent(clk[VF610_CLK_DDR_SEL]) != clk[VF610_CLK_PLL2_PFD2]) {
> +		pr_warn("DDRC is clocked by PLL1, can't switch CPU clock");
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_parent(clk[VF610_CLK_SYS_SEL], clk[VF610_CLK_PLL2_BUS]);
> +	if (ret < 0) {
> +		pr_crit("Unable to re-parent '%s'\n",
> +			clk[VF610_CLK_SYS_SEL]->name);
> +		return ret;
> +	}
> +
> +	ret = clk_set_rate(clk[VF610_CLK_PLL1], 500000000);
> +	if (ret < 0) {
> +		pr_crit("Unable to set %s to 500Mhz %d\n",
> +			clk[VF610_CLK_PLL1]->name, ret);
> +		return ret;
> +	}
> +
> +	ret = clk_set_parent(clk[VF610_CLK_PLL1_PFD_SEL], clk[VF610_CLK_PLL1_SYS]);
> +	if (ret < 0) {
> +		pr_crit("Unable to re-parent '%s'\n",
> +			clk[VF610_CLK_PLL1_PFD_SEL]->name);
> +		return ret;
> +	}
> +
> +	ret = clk_set_parent(clk[VF610_CLK_SYS_SEL], clk[VF610_CLK_PLL1_PFD_SEL]);
> +	if (ret < 0) {
> +		pr_crit("Unable to re-parent '%s'\n",
> +			clk[VF610_CLK_SYS_SEL]->name);
> +		return ret;
> +	}
> +
> +	/*
> +	 * imx_clk_divider has no error path in its set_rate hook
> +	 */
> +	clk_set_rate(clk[VF610_CLK_SYS_BUS], clk_get_rate(clk[VF610_CLK_SYS_SEL]));
> +	clk_set_rate(clk[VF610_CLK_PLATFORM_BUS], clk_get_rate(clk[VF610_CLK_SYS_BUS]) / 3);
> +
> +	return ret;
> +}
> +
> +static int vf610_switch_cpu_clock_to_400mhz(void)
> +{
> +	int ret;
> +	uint32_t cr117;
> +	void * __iomem ddrmc = IOMEM(VF610_DDR_BASE_ADDR);
> +
> +	if (clk_get_parent(clk[VF610_CLK_DDR_SEL]) != clk[VF610_CLK_PLL2_PFD2]) {
> +		pr_warn("DDRC is clocked by PLL1, can't switch CPU clock");
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_parent(clk[VF610_CLK_PLL2_PFD_SEL], clk[VF610_CLK_PLL2_PFD2]);
> +	if (ret < 0) {
> +		pr_crit("Unable to re-parent '%s'\n",
> +			clk[VF610_CLK_PLL2_PFD_SEL]->name);
> +		return ret;
> +	}
> +
> +	ret = clk_set_parent(clk[VF610_CLK_SYS_SEL], clk[VF610_CLK_PLL2_PFD_SEL]);
> +	if (ret < 0) {
> +		pr_crit("Unable to re-parent '%s'\n",
> +			clk[VF610_CLK_SYS_SEL]->name);
> +		return ret;
> +	}
> +
> +	/*
> +	 * imx_clk_divider has no error path in its set_rate hook
> +	 */
> +	clk_set_rate(clk[VF610_CLK_SYS_BUS], clk_get_rate(clk[VF610_CLK_SYS_SEL]));
> +	clk_set_rate(clk[VF610_CLK_PLATFORM_BUS], clk_get_rate(clk[VF610_CLK_SYS_BUS]) / 3);
> +
> +	/*
> +	 * Now that we are running off of the same clock as DDRMC we
> +	 * shouldn't need to use clock domain corssing FIFO and

s/corssing/crossing/?

> +	 * asynchronous mode and instead can swithch to sychronous
> +	 * mode for AXI0 accesses
> +	 */
> +	cr117 =  readl(ddrmc + DDRMC_CR117);
> +	cr117 |= DDRMC_CR117_AXI0_FITYPEREG_SYNC;
> +	writel(cr117, ddrmc + DDRMC_CR117);
> +
> +	return 0;
> +}
> +
> +static int vf610_switch_cpu_clock(void)
> +{
> +	int ret;
> +	struct device_node *ocotp_node;
> +	struct cdev *ocotp;
> +	uint32_t speed_grading;
> +
> +	if (!of_machine_is_compatible("fsl,vf610"))
> +		return 0;
> +
> +	ocotp_node = of_find_compatible_node(NULL, NULL, "fsl,vf610-ocotp");
> +	if (!ocotp_node) {
> +		pr_err("Unable to find OCOTP DT node\n");
> +		return -ENODEV;
> +	}
> +
> +	ocotp = cdev_by_device_node(ocotp_node);
> +	if (!ocotp) {
> +		pr_err("No OCOTP character device\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = cdev_read(ocotp, &speed_grading, sizeof(speed_grading),
> +		       OCOTP_SPEED_GRADING_OFFSET, 0);
> +	if (ret != sizeof(speed_grading)) {
> +		pr_err("Failed to read speed grading data\n");
> +		return ret < 0 ? ret : -EIO;
> +	}

There's a better way to access ocotp registers. see
imx_ocotp_read_field().

> +
> +	speed_grading >>= OCOTP_SPEED_GRADING_SHIFT;
> +	speed_grading &=  OCOTP_SPEED_GRADING_MASK;
> +
> +	switch (speed_grading) {
> +	default:
> +		pr_err("Unknown CPU speed grading %x\n", speed_grading);
> +		return -EINVAL;
> +
> +	case VF610_SPEED_266:
> +		return 0;
> +
> +	case VF610_SPEED_500:
> +		ret = vf610_switch_cpu_clock_to_500mhz();
> +		break;
> +
> +	case VF610_SPEED_400:
> +		ret = vf610_switch_cpu_clock_to_400mhz();
> +		break;
> +	}
> +
> +	clock_notifier_call_chain();
> +	return ret;
> +}
> +/*
> + * We can probably gain a bit of a boot speed if we switch CPU clock
> + * earlier, but if we do this we'd need to figure out a way how to
> + * re-adjust the baud rate settings of the UART for DEBUG_LL
> + * functionality, or, accept the fact that it will be unavailbe after

s/unavailbe/unavailable/

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH] spi: mvebu: fix error handling for transfer problems
From: Sascha Hauer @ 2016-12-07 19:31 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox
In-Reply-To: <20161205102743.3896-1-u.kleine-koenig@pengutronix.de>

On Mon, Dec 05, 2016 at 11:27:43AM +0100, Uwe Kleine-König wrote:
> When a message transfer fails no further messages are transferred, but
> the error value was not propagated to the caller.
> 
> Fixes: 5db1a578d6ed ("spi: add Marvell MVEBU SoC SPI driver")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/spi/mvebu_spi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/spi/mvebu_spi.c b/drivers/spi/mvebu_spi.c
> index 335774d4c609..ee7b212962f7 100644
> --- a/drivers/spi/mvebu_spi.c
> +++ b/drivers/spi/mvebu_spi.c
> @@ -305,14 +305,14 @@ static int mvebu_spi_transfer(struct spi_device *spi, struct spi_message *msg)
>  	list_for_each_entry(t, &msg->transfers, transfer_list) {
>  		ret = mvebu_spi_do_transfer(spi, t);
>  		if (ret)
> -			break;
> +			goto err_transfer;
>  		msg->actual_length += t->len;
>  	}
>  
> -	ret = mvebu_spi_set_cs(priv, spi->chip_select, spi->mode, false);
> -	if (ret)
> -		return ret;
> +	return mvebu_spi_set_cs(priv, spi->chip_select, spi->mode, false);
>  
> +err_transfer:
> +	mvebu_spi_set_cs(priv, spi->chip_select, spi->mode, false);
>  	return ret;
>  }
>  
> -- 
> 2.10.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Andrey Smirnov @ 2016-12-07 19:32 UTC (permalink / raw)
  To: Stefan Lengfeld; +Cc: barebox@lists.infradead.org
In-Reply-To: <20161207085105.GC2371@lws-christ>

On Wed, Dec 7, 2016 at 12:51 AM, Stefan Lengfeld <s.lengfeld@phytec.de> wrote:
> Hi Andrey,
>
>> I did see that patch go through, however since the title of it is
>> "arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
>> it any further.
>
> In the light of the Vybrid also supports two MAC addresses your patch title
>
>     i.MX: ocotp: Add provisions for storing multiple MAC addresses
>
> is better, since it is more generic.
>
>> > Your approach uses an extra device attribute  'mac_idx' to select the meaning
>> > of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
>> >
>> > I find it less error prone and confusing to use two seperate device attributes
>> > 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
>> > value of 'mac_idx before reading or writing the MAC addresses.
>>
>> Can't say that I necessarily agree with you assessment. There's no
>> need to check 'mac_idx' if it is written every time as a part of MAC
>> address assignment operation. The reason I chose the approach that I
>> did was mainly because it allowed to both have backwards compatibility
>> with the old naming scheme and avoid having variable naming
>> inconsistency (as you see in your example where one variable has a
>> numerical suffix and the other doesn't) which I was concerned would be
>> make scripting it more clunky than necessary.
>
> hmm, I find it more convenient use
>
>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
>     bootloader$ ocotp0.mac_addr1=22:33:44:AA:AA:AA
>
> than
>
>     bootloader$ ocotp0.mac_idx=0
>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
>     bootloader$ ocotp0.mac_idx=1
>     bootloader$ ocotp0.mac_addr=22:33:44:AA:AA:AA
>
> The same goes for reading the MAC addresses. The first one is more obvious,
> because the meaning of the variable "mac_addr" does not depend on another
> variable "mac_idx".

Well you original point was about being error prone and confusing, not
convenience of use. ;-) My "scheme' involves way more typing, so it is
most certainly would be less convenient.

>
> Maybe we can settle on a different approach to avoid the inconsistent variable
> names "mac_addr" and "mac_addr1":
>
>         if (IS_ENABLED(CONFIG_NET)) {
>                 if (!data->scnd_mac_addr) {
>                         /* one MAC */
>                         dev_add_param_mac(&(priv->dev), "mac_addr", imx_ocotp_set_mac0,
>                                imx_ocotp_get_mac0, priv->ethaddr[0], priv);
>
>                 } else {
>                         /* two MACs: Vybrid and UltraLite */
>                         dev_add_param_mac(&(priv->dev), "mac_addr0", imx_ocotp_set_mac0,
>                                imx_ocotp_get_mac0, priv->ethaddr[0], priv);
>                         dev_add_param_mac(&(priv->dev), "mac_addr1", imx_ocotp_set_mac1,
>                                 imx_ocotp_get_mac1, priv->ethaddr[1], priv);
>                 }
>         }
>
> All existing boards still uses "mac_addr" for there single MAC address and new
> boards Vybrid and UltraLite use "mac_addr0" and "mac_addr1". In the wild there
> will be two different sets of - for example - factory MAC burning scripts
> anyway.  One to handle a single MAC address and another to handle two MAC
> addresses.
>
> What do you think?

Sounds reasonable. I didn't go this route originally because I wanted
to be backwards compatible, but thinking more on it I don't know if it
(backwards compatibility) matters too much.

Thanks,
Andrey

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Andrey Smirnov @ 2016-12-07 19:36 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox@lists.infradead.org, Stefan Lengfeld
In-Reply-To: <20161207191343.v2y3dmzo4m67ws3c@pengutronix.de>

On Wed, Dec 7, 2016 at 11:13 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Dec 07, 2016 at 09:51:05AM +0100, Stefan Lengfeld wrote:
>> Hi Andrey,
>>
>> > I did see that patch go through, however since the title of it is
>> > "arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
>> > it any further.
>>
>> In the light of the Vybrid also supports two MAC addresses your patch title
>>
>>     i.MX: ocotp: Add provisions for storing multiple MAC addresses
>>
>> is better, since it is more generic.
>>
>> > > Your approach uses an extra device attribute  'mac_idx' to select the meaning
>> > > of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
>> > >
>> > > I find it less error prone and confusing to use two seperate device attributes
>> > > 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
>> > > value of 'mac_idx before reading or writing the MAC addresses.
>> >
>> > Can't say that I necessarily agree with you assessment. There's no
>> > need to check 'mac_idx' if it is written every time as a part of MAC
>> > address assignment operation. The reason I chose the approach that I
>> > did was mainly because it allowed to both have backwards compatibility
>> > with the old naming scheme and avoid having variable naming
>> > inconsistency (as you see in your example where one variable has a
>> > numerical suffix and the other doesn't) which I was concerned would be
>> > make scripting it more clunky than necessary.
>>
>> hmm, I find it more convenient use
>>
>>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
>>     bootloader$ ocotp0.mac_addr1=22:33:44:AA:AA:AA
>>
>> than
>>
>>     bootloader$ ocotp0.mac_idx=0
>>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
>>     bootloader$ ocotp0.mac_idx=1
>>     bootloader$ ocotp0.mac_addr=22:33:44:AA:AA:AA
>>
>> The same goes for reading the MAC addresses. The first one is more obvious,
>> because the meaning of the variable "mac_addr" does not depend on another
>> variable "mac_idx".
>
> I agree here.
>
>>
>> Maybe we can settle on a different approach to avoid the inconsistent variable
>> names "mac_addr" and "mac_addr1":
>
> How about always registering mac_addr0 with mac_addr as alias? Then we
> have consistent variable naming and still the backward compatible
> standard mac address.

That'd definetly would be even better way to go if we want to preserve
backwards compatibility. It might be a bit confusing UI wise to have
"ocotp0.mac_addr0", "ocotp0.mac_addr1" and "ocotp0.mac_addr" at the
same time, though.

I don't really have a any preference for either way.

Thanks,
Andrey

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [RFC v2 4/8] import initial kexec stuff
From: Sascha Hauer @ 2016-12-07 19:47 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox, Peter Mamonov
In-Reply-To: <20161205094033.31569-5-antonynpavlov@gmail.com>

On Mon, Dec 05, 2016 at 12:40:29PM +0300, Antony Pavlov wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> Signed-off-by: Peter Mamonov <pmamonov@gmail.com>

The commit message could be a bit more verbose and at least mention
where this code is derived from and from which version.

> +
> +static int build_mem_ehdr(const char *buf, off_t len, struct mem_ehdr *ehdr)
> +{
> +	unsigned char e_ident[EI_NIDENT];
> +	int result;
> +
> +	memset(ehdr, 0, sizeof(*ehdr));
> +
> +	if ((size_t)len < sizeof(e_ident)) {
> +		printf("Buffer is too small to hold ELF e_ident\n");
> +
> +		return -1;
> +	}
> +
> +	memcpy(e_ident, buf, sizeof(e_ident));
> +
> +	ehdr->ei_class   = e_ident[EI_CLASS];
> +	ehdr->ei_data    = e_ident[EI_DATA];
> +	if (	(ehdr->ei_class != ELFCLASS32) &&
> +		(ehdr->ei_class != ELFCLASS64))
> +	{
> +		printf("Not a supported ELF class\n");
> +		return -1;
> +	}
> +
> +	if (	(ehdr->ei_data != ELFDATA2LSB) &&
> +		(ehdr->ei_data != ELFDATA2MSB))
> +	{
> +		printf("Not a supported ELF data format\n");
> +		return -1;
> +	}
> +
> +	result = -1;
> +	if (ehdr->ei_class == ELFCLASS32) {
> +		result = build_mem_elf32_ehdr(buf, len, ehdr);
> +	}

The opening braces are sometimes on the same line and sometimees on a
new line. Is this from the original code?

> +
> +	/* Now walk and normalize the notes */
> +	ehdr->e_note = xmalloc(sizeof(*ehdr->e_note) * ehdr->e_notenum);
> +	for (i = 0, note = note_start; note < note_end;
> +			note += note_size, i++) {
> +		const unsigned char *name, *desc;
> +		ElfNN_Nhdr hdr;
> +		read_nhdr(ehdr, &hdr, note);
> +		note_size  = sizeof(hdr);
> +		name       = note + note_size;
> +		note_size += (hdr.n_namesz + 3) & ~3;
> +		desc       = note + note_size;
> +		note_size += (hdr.n_descsz + 3) & ~3;
> +
> +		if ((hdr.n_namesz != 0) && (name[hdr.n_namesz -1] != '\0')) {
> +			/* If note name string is not null terminated, just
> +			 * warn user about it and continue processing. This
> +			 * allows us to parse /proc/kcore on older kernels
> +			 * where /proc/kcore elf notes were not null
> +			 * terminated. It has been fixed in 2.6.19.
> +			 */
> +			printf("Warning: Elf Note name is not null "
> +					"terminated\n");
> +		}

Is this relevant for barebox? We do not parse /proc/kcore.

> +		ehdr->e_note[i].n_type = hdr.n_type;
> +		ehdr->e_note[i].n_name = (char *)name;
> +		ehdr->e_note[i].n_desc = desc;
> +		ehdr->e_note[i].n_descsz = hdr.n_descsz;
> +
> +	}
> +
> +	return 0;
> +}
> +

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [RFC v2 6/8] bootm: add kexec ELF support
From: Sascha Hauer @ 2016-12-07 19:59 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox
In-Reply-To: <20161205094033.31569-7-antonynpavlov@gmail.com>

On Mon, Dec 05, 2016 at 12:40:31PM +0300, Antony Pavlov wrote:
> Also introduce reboot() for starting already loaded
> via kexec ELF segments.
> 
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  include/linux/reboot.h      | 14 ++++++++++++++
>  lib/kexec/Makefile          |  1 +
>  lib/kexec/kexec-bootm-elf.c | 37 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
> 
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> new file mode 100644
> index 0000000..454ed33
> --- /dev/null
> +++ b/include/linux/reboot.h
> @@ -0,0 +1,14 @@
> +#ifndef _LINUX_REBOOT_H
> +#define _LINUX_REBOOT_H
> +
> +/*
> + * Commands accepted by the _reboot() system call.
> + *
> + * KEXEC       Restart system using a previously loaded Linux kernel
> + */
> +
> +#define	LINUX_REBOOT_CMD_KEXEC		0x45584543
> +
> +extern int reboot(int cmd);
> +
> +#endif /* _LINUX_REBOOT_H */
> diff --git a/lib/kexec/Makefile b/lib/kexec/Makefile
> index 8febef1..2f3dc1d 100644
> --- a/lib/kexec/Makefile
> +++ b/lib/kexec/Makefile
> @@ -1,3 +1,4 @@
>  obj-y	+= kexec.o
>  obj-y	+= kexec-elf.o
>  obj-y	+= kexec-elf-exec.o
> +obj-y	+= kexec-bootm-elf.o
> diff --git a/lib/kexec/kexec-bootm-elf.c b/lib/kexec/kexec-bootm-elf.c
> new file mode 100644
> index 0000000..ceef6c7
> --- /dev/null
> +++ b/lib/kexec/kexec-bootm-elf.c
> @@ -0,0 +1,37 @@
> +#include <bootm.h>
> +#include <init.h>
> +#include <binfmt.h>
> +#include <errno.h>
> +#include <linux/reboot.h>
> +#include <environment.h>
> +
> +#include "kexec.h"
> +
> +static int do_bootm_elf(struct image_data *data)
> +{
> +	kexec_load_file(data->os_file, 0);
> +	setenv("global.bootm.image", data->os_file);
> +	reboot(LINUX_REBOOT_CMD_KEXEC);
> +
> +	return -ERESTARTSYS;
> +}
> +
> +static struct image_handler elf_handler = {
> +	.name = "ELF",
> +	.bootm = do_bootm_elf,
> +	.filetype = filetype_elf,
> +};
> +
> +static struct binfmt_hook binfmt_elf_hook = {
> +	.type = filetype_elf,
> +	.exec = "bootm",
> +};
> +
> +static int elf_register_image_handler(void)
> +{
> +	register_image_handler(&elf_handler);
> +	binfmt_register(&binfmt_elf_hook);
> +
> +	return 0;
> +}
> +late_initcall(elf_register_image_handler);

The code needed to actually start a kexec kernel is architecture
specific, so the registration of the ELF handler should be done by this
architecture specific code. Then you also don't need this awkward reboot()
stuff. I'm thinking of a

int bootm_register_kexec_handler(int (*do_bootm)(struct image_data *data));

to be called by MIPS or malta specific code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH] MIPS: qemu-malta_defconfig: set MAX_IMAGE_SIZE = 4 MiB
From: Sascha Hauer @ 2016-12-07 20:00 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox
In-Reply-To: <20161205082829.22468-1-antonynpavlov@gmail.com>

On Mon, Dec 05, 2016 at 11:28:29AM +0300, Antony Pavlov wrote:
> If barebox binary image size exceeds the 4 MiB then
> qemu exits with the 'Could not load MIPS bios' message.
> 
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  arch/mips/configs/qemu-malta_defconfig | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks

Sascha

> 
> diff --git a/arch/mips/configs/qemu-malta_defconfig b/arch/mips/configs/qemu-malta_defconfig
> index 004adac..9671e93 100644
> --- a/arch/mips/configs/qemu-malta_defconfig
> +++ b/arch/mips/configs/qemu-malta_defconfig
> @@ -1,6 +1,7 @@
>  CONFIG_BUILTIN_DTB=y
>  CONFIG_BUILTIN_DTB_NAME="qemu-malta"
>  CONFIG_PBL_IMAGE=y
> +CONFIG_BAREBOX_MAX_IMAGE_SIZE=0x400000
>  CONFIG_STACK_SIZE=0x7000
>  CONFIG_EXPERIMENTAL=y
>  CONFIG_BAUDRATE=38400
> -- 
> 2.10.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH] x86: update boot message "UBOOT2" to "BAREBOX"
From: Sascha Hauer @ 2016-12-07 20:02 UTC (permalink / raw)
  To: Ulrich Ölmann; +Cc: Barebox List
In-Reply-To: <1480583994-27062-1-git-send-email-u.oelmann@pengutronix.de>

On Thu, Dec 01, 2016 at 10:19:54AM +0100, Ulrich Ölmann wrote:
> Signed-off-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
> ---
>  arch/x86/boot/boot_hdisk.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/x86/boot/boot_hdisk.S b/arch/x86/boot/boot_hdisk.S
> index 143336d3b43e..6f98197512a0 100644
> --- a/arch/x86/boot/boot_hdisk.S
> +++ b/arch/x86/boot/boot_hdisk.S
> @@ -164,7 +164,7 @@ output_message:
>  
>  	.section .boot_data
>  
> -notification_string:	.asciz "UBOOT2 "
> +notification_string:	.asciz "BAREBOX "
>  chs_string:	.asciz "CHS "
>  jmp_string:	.asciz "JMP "
>  
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 1/2] imx6qdl-phytec-phycore-som.dtsi: rename eeprom
From: Sascha Hauer @ 2016-12-07 20:06 UTC (permalink / raw)
  To: Stefan Lengfeld; +Cc: barebox
In-Reply-To: <1480501660-4540-1-git-send-email-s.lengfeld@phytec.de>

On Wed, Nov 30, 2016 at 11:27:39AM +0100, Stefan Lengfeld wrote:
> From: Jan Remmet <j.remmet@phytec.de>
> 
> Chose name according to kernel. This is needed for bootstate.
> 
> Signed-off-by: Jan Remmet <j.remmet@phytec.de>
> Signed-off-by: Christian Hemp <c.hemp@phytec.de>

Applied, thanks

Sascha

> ---
>  arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi b/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi
> index d446a5e..37ad878 100644
> --- a/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi
> +++ b/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi
> @@ -119,9 +119,9 @@
>  	clock-frequency = <400000>;
>  	status = "okay";
>  
> -	eeprom: 24c32@50 {
> +	eeprom: eeprom@50 {
>  		status = "disabled";
> -		compatible = "st,24c32";
> +		compatible = "24c32";
>  		reg = <0x50>;
>  	};
>  };
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH] mtd: m25p80: add support for Everspin MR25H40
From: Sascha Hauer @ 2016-12-07 20:07 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox
In-Reply-To: <20161124205918.20093-1-u.kleine-koenig@pengutronix.de>

On Thu, Nov 24, 2016 at 09:59:18PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/mtd/devices/m25p80.c  | 2 +-
>  drivers/mtd/spi-nor/spi-nor.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 656a7f5ef045..8a67792125e3 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -179,7 +179,7 @@ static const struct platform_device_id m25p_ids[] = {
>  	{"en25f32"},    {"en25p32"},    {"en25q32b"},   {"en25p64"},
>  	{"en25q64"},    {"en25qh128"},  {"en25qh256"},
>  	{"f25l32pa"},
> -	{"mr25h256"},   {"mr25h10"},
> +	{"mr25h256"},   {"mr25h10"},    {"mr25h40"},
>  	{"gd25q32"},    {"gd25q64"},
>  	{"160s33b"},    {"320s33b"},    {"640s33b"},
>  	{"mx25l2005a"}, {"mx25l4005a"}, {"mx25l8005"},  {"mx25l1606e"},
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 45be58623839..faeafe1bfb6a 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -504,6 +504,7 @@ static const struct spi_device_id spi_nor_ids[] = {
>  	/* Everspin */
>  	{ "mr25h256", CAT25_INFO( 32 * 1024, 1, 256, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>  	{ "mr25h10",  CAT25_INFO(128 * 1024, 1, 256, 3, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
> +	{ "mr25h40",  CAT25_INFO(512 * 1024, 1, 256, 3, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>  
>  	/* Fujitsu */
>  	{ "mb85rs1mt", INFO(0x047f27, 0, 128 * 1024, 1, SPI_NOR_NO_ERASE) },
> -- 
> 2.10.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 1/2] fixup! gpio: Add GPIO driver for Vybrid
From: Sascha Hauer @ 2016-12-07 20:09 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox
In-Reply-To: <1480108031-17889-1-git-send-email-andrew.smirnov@gmail.com>

On Fri, Nov 25, 2016 at 01:07:10PM -0800, Andrey Smirnov wrote:
> ---
> 
> Sascha,
> 
> It looks like the two affected patches haven't hit 'master' yet, so I
> am hoping these two fixups could be incorporated into 'next'.

Yes, just did that.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 0/6] ubi: remove character device flag from static volumes
From: Sascha Hauer @ 2016-12-07 20:13 UTC (permalink / raw)
  To: Teresa Remmet; +Cc: barebox
In-Reply-To: <1480061167-21590-1-git-send-email-t.remmet@phytec.de>

On Fri, Nov 25, 2016 at 09:06:01AM +0100, Teresa Remmet wrote:
> Instead of marking static volume device files as character devices 
> introduced a truncate callback for device files.
> This makes it possible to dump static ubi volumes from flash.
> 
> Teresa Remmet (6):
>   libfile: copy_file: Only open regular files with O_TRUNC
>   devfs: Add optional truncate callback for device files
>   ubi: Add truncate callback
>   fs: Remove O_TRUNC check for devices when open files
>   commands: ubi: ubiupdatevol: Open device with O_TRUNC
>   ubi: barebox: Remove character device flag from static volumes

Applied, thanks

Sascha

> 
>  commands/ubi.c            |  2 +-
>  drivers/mtd/ubi/barebox.c | 19 ++++++++++++++++---
>  fs/devfs.c                |  5 +++++
>  fs/fs.c                   |  2 +-
>  include/driver.h          |  1 +
>  lib/libfile.c             | 22 ++++++++++++++++------
>  6 files changed, 40 insertions(+), 11 deletions(-)
> 
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 1/3] ARM: dts: phycore-imx6: refactor fec nodes
From: Sascha Hauer @ 2016-12-07 20:14 UTC (permalink / raw)
  To: Stefan Lengfeld; +Cc: barebox
In-Reply-To: <1480322700-3529-1-git-send-email-s.lengfeld@phytec.de>

On Mon, Nov 28, 2016 at 09:44:58AM +0100, Stefan Lengfeld wrote:
> Refactor the common settings for device tree node 'fec' into the generic
> phycore i.MX6 device tree file. This avoid redundant settings and makes
> common fixes easier. Our kernel device tree files have the same layout.
> 
> Signed-off-by: Stefan Lengfeld <s.lengfeld@phytec.de>
> ---
>  arch/arm/dts/imx6dl-phytec-phycore-som-emmc.dts | 17 ++++-------------
>  arch/arm/dts/imx6dl-phytec-phycore-som-nand.dts | 17 ++++-------------
>  arch/arm/dts/imx6q-phytec-phycore-som-emmc.dts  | 17 ++++-------------
>  arch/arm/dts/imx6q-phytec-phycore-som-nand.dts  | 17 ++++-------------
>  arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi    | 13 +++++++++++++
>  5 files changed, 29 insertions(+), 52 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/dts/imx6dl-phytec-phycore-som-emmc.dts b/arch/arm/dts/imx6dl-phytec-phycore-som-emmc.dts
> index fc153a6..bffee5f 100644
> --- a/arch/arm/dts/imx6dl-phytec-phycore-som-emmc.dts
> +++ b/arch/arm/dts/imx6dl-phytec-phycore-som-emmc.dts
> @@ -29,21 +29,12 @@
>  	status = "okay";
>  };
>  
> +&ethphy {
> +	max-speed = <100>;
> +};
> +
>  &fec {
>  	status = "okay";
> -	phy-handle = <&ethphy>;
> -	phy-mode = "rgmii";
> -	phy-reset-gpios = <&gpio1 14 1>;
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		ethphy: ethernet-phy@3 {
> -			reg = <3>;
> -			max-speed = <100>;
> -		};
> -	};
>  };
>  
>  &flash {
> diff --git a/arch/arm/dts/imx6dl-phytec-phycore-som-nand.dts b/arch/arm/dts/imx6dl-phytec-phycore-som-nand.dts
> index 3f2f1c7..1b66fda 100644
> --- a/arch/arm/dts/imx6dl-phytec-phycore-som-nand.dts
> +++ b/arch/arm/dts/imx6dl-phytec-phycore-som-nand.dts
> @@ -24,21 +24,12 @@
>  	status = "okay";
>  };
>  
> +&ethphy {
> +	max-speed = <100>;
> +};
> +
>  &fec {
>  	status = "okay";
> -	phy-handle = <&ethphy>;
> -	phy-mode = "rgmii";
> -	phy-reset-gpios = <&gpio1 14 1>;
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		ethphy: ethernet-phy@3 {
> -			reg = <3>;
> -			max-speed = <100>;
> -		};
> -	};
>  };
>  
>  &gpmi {
> diff --git a/arch/arm/dts/imx6q-phytec-phycore-som-emmc.dts b/arch/arm/dts/imx6q-phytec-phycore-som-emmc.dts
> index 74bc09b..ecc5aa3 100644
> --- a/arch/arm/dts/imx6q-phytec-phycore-som-emmc.dts
> +++ b/arch/arm/dts/imx6q-phytec-phycore-som-emmc.dts
> @@ -28,21 +28,12 @@
>  	status = "okay";
>  };
>  
> +&ethphy {
> +	max-speed = <1000>;
> +};
> +
>  &fec {
>  	status = "okay";
> -	phy-handle = <&ethphy>;
> -	phy-mode = "rgmii";
> -	phy-reset-gpios = <&gpio1 14 1>;
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		ethphy: ethernet-phy@3 {
> -			reg = <3>;
> -			max-speed = <1000>;
> -		};
> -	};
>  };
>  
>  &flash {
> diff --git a/arch/arm/dts/imx6q-phytec-phycore-som-nand.dts b/arch/arm/dts/imx6q-phytec-phycore-som-nand.dts
> index aa2c94a..9ad7eda 100644
> --- a/arch/arm/dts/imx6q-phytec-phycore-som-nand.dts
> +++ b/arch/arm/dts/imx6q-phytec-phycore-som-nand.dts
> @@ -29,21 +29,12 @@
>  	status = "okay";
>  };
>  
> +&ethphy {
> +	max-speed = <1000>;
> +};
> +
>  &fec {
>  	status = "okay";
> -	phy-handle = <&ethphy>;
> -	phy-mode = "rgmii";
> -	phy-reset-gpios = <&gpio1 14 1>;
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		ethphy: ethernet-phy@3 {
> -			reg = <3>;
> -			max-speed = <1000>;
> -		};
> -	};
>  };
>  
>  &flash {
> diff --git a/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi b/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi
> index d446a5e..9649817 100644
> --- a/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi
> +++ b/arch/arm/dts/imx6qdl-phytec-phycore-som.dtsi
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "imx6qdl.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
>  
>  / {
>  	chosen {
> @@ -76,7 +77,19 @@
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> +	phy-handle = <&ethphy>;
> +	phy-mode = "rgmii";
> +	phy-reset-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
>  	status = "disabled";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy: ethernet-phy@3 {
> +			reg = <3>;
> +		};
> +	};
>  };
>  
>  &gpmi {
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: UBIFS trouble
From: Sascha Hauer @ 2016-12-07 20:50 UTC (permalink / raw)
  To: Juergen Borleis; +Cc: barebox
In-Reply-To: <201611291504.41899.jbe@pengutronix.de>

On Tue, Nov 29, 2016 at 03:04:41PM +0100, Juergen Borleis wrote:
> FYI: soft links are broken since 2016.05.0.

Most likely this goes down to:

a63059d ubifs: update implementation from u-boot v2016.03

U-Boot resolves links in each filesystem implementation whereas barebox
handles links natively. Have a look at a63059d, identify the link
resolving code and revert this to the previously working barebox code.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 2/4] ARM i.MX: add SoC type detection for i.MX6SL
From: Sascha Hauer @ 2016-12-07 20:53 UTC (permalink / raw)
  To: Alexander Kurz; +Cc: barebox
In-Reply-To: <1480325255-17750-2-git-send-email-akurz@blala.de>

Hi Alexander,

On Mon, Nov 28, 2016 at 10:27:33AM +0100, Alexander Kurz wrote:
> The i.MX6 series SoC type is determined by barebox by examining the
> USB_ANALOG_DIGPROG aka IMX6_ANATOP_SI_REV register. This register is located
> at a common offset for all mx6 SoC - except for i.MX6SL where a different
> offset is used. This creates a dilemma while distinguishing the mx6sl from
> non-mx6sl SOC since the SoC type identification register location is type
> specific itself.
> 
> Access to undocumented and probably invalid or unpredictable registers should
> be avoided as possible. For the mx6sl detection an access to the general
> USB_ANALOG_DIGPROG @0x260 cannot be avoided when running on mx6sl. This
> register contained the value 0x00014009 for different mx6sl Rev. 1.2 based
> e-book readers using MCIMX6L7DVN10AB and MCIMX6L8DVN10AB SoC. This
> implementation assumes the value of MAJOR_UPPER (here 0x01) to be smaller
> than the smallest non-6sl MAJOR_UPPER (0x61 for mx6s).

The i.MX6SL code would be quite straight forward and easy to merge
without this handicap. I'll probably need a few more days to swallow
this pill. I assume you already searched for better ways to detect a
i.MX6SL, right?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 02/16] i.MX: ocotp: Add provisions for storing multiple MAC addresses
From: Sascha Hauer @ 2016-12-07 20:57 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox@lists.infradead.org, Stefan Lengfeld
In-Reply-To: <CAHQ1cqEMgyfvqrujYDQZp_6TzLGxpQ937RjOQZrkas+8BmutHg@mail.gmail.com>

On Wed, Dec 07, 2016 at 11:36:21AM -0800, Andrey Smirnov wrote:
> On Wed, Dec 7, 2016 at 11:13 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, Dec 07, 2016 at 09:51:05AM +0100, Stefan Lengfeld wrote:
> >> Hi Andrey,
> >>
> >> > I did see that patch go through, however since the title of it is
> >> > "arm: imx6: ocotp: Added support for the i.MX6UL" I didn't look into
> >> > it any further.
> >>
> >> In the light of the Vybrid also supports two MAC addresses your patch title
> >>
> >>     i.MX: ocotp: Add provisions for storing multiple MAC addresses
> >>
> >> is better, since it is more generic.
> >>
> >> > > Your approach uses an extra device attribute  'mac_idx' to select the meaning
> >> > > of the device attribute 'mac_addr': Whether it points to the MAC0 or MAC1.
> >> > >
> >> > > I find it less error prone and confusing to use two seperate device attributes
> >> > > 'mac_addr' and 'mac_addr1' for MAC0 and MAC1. So you don't have to check the
> >> > > value of 'mac_idx before reading or writing the MAC addresses.
> >> >
> >> > Can't say that I necessarily agree with you assessment. There's no
> >> > need to check 'mac_idx' if it is written every time as a part of MAC
> >> > address assignment operation. The reason I chose the approach that I
> >> > did was mainly because it allowed to both have backwards compatibility
> >> > with the old naming scheme and avoid having variable naming
> >> > inconsistency (as you see in your example where one variable has a
> >> > numerical suffix and the other doesn't) which I was concerned would be
> >> > make scripting it more clunky than necessary.
> >>
> >> hmm, I find it more convenient use
> >>
> >>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
> >>     bootloader$ ocotp0.mac_addr1=22:33:44:AA:AA:AA
> >>
> >> than
> >>
> >>     bootloader$ ocotp0.mac_idx=0
> >>     bootloader$ ocotp0.mac_addr=22:33:44:55:66:77
> >>     bootloader$ ocotp0.mac_idx=1
> >>     bootloader$ ocotp0.mac_addr=22:33:44:AA:AA:AA
> >>
> >> The same goes for reading the MAC addresses. The first one is more obvious,
> >> because the meaning of the variable "mac_addr" does not depend on another
> >> variable "mac_idx".
> >
> > I agree here.
> >
> >>
> >> Maybe we can settle on a different approach to avoid the inconsistent variable
> >> names "mac_addr" and "mac_addr1":
> >
> > How about always registering mac_addr0 with mac_addr as alias? Then we
> > have consistent variable naming and still the backward compatible
> > standard mac address.
> 
> That'd definetly would be even better way to go if we want to preserve
> backwards compatibility. It might be a bit confusing UI wise to have
> "ocotp0.mac_addr0", "ocotp0.mac_addr1" and "ocotp0.mac_addr" at the
> same time, though.

Yes, indeed, this might be confusing, but the fact that mac_addr and
mac_addr0 always have the same value should make that clear.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH 2/4] ARM i.MX: add SoC type detection for i.MX6SL
From: Alexander Kurz @ 2016-12-07 21:40 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox
In-Reply-To: <20161207205323.ymjfjjmsevdjuawg@pengutronix.de>



On Wed, 7 Dec 2016, Sascha Hauer wrote:

> Hi Alexander,
> 
> On Mon, Nov 28, 2016 at 10:27:33AM +0100, Alexander Kurz wrote:
> > The i.MX6 series SoC type is determined by barebox by examining the
> > USB_ANALOG_DIGPROG aka IMX6_ANATOP_SI_REV register. This register is located
> > at a common offset for all mx6 SoC - except for i.MX6SL where a different
> > offset is used. This creates a dilemma while distinguishing the mx6sl from
> > non-mx6sl SOC since the SoC type identification register location is type
> > specific itself.
> > 
> > Access to undocumented and probably invalid or unpredictable registers should
> > be avoided as possible. For the mx6sl detection an access to the general
> > USB_ANALOG_DIGPROG @0x260 cannot be avoided when running on mx6sl. This
> > register contained the value 0x00014009 for different mx6sl Rev. 1.2 based
> > e-book readers using MCIMX6L7DVN10AB and MCIMX6L8DVN10AB SoC. This
> > implementation assumes the value of MAJOR_UPPER (here 0x01) to be smaller
> > than the smallest non-6sl MAJOR_UPPER (0x61 for mx6s).
> 
> The i.MX6SL code would be quite straight forward and easy to merge
> without this handicap. I'll probably need a few more days to swallow
> this pill. I assume you already searched for better ways to detect a
> i.MX6SL, right?
>
The uboot-implementation (get_cpu_rev() in arch/arm/cpu/armv7/mx6/soc.c)
first checks the 6SL-digprog register and then proceeds with the non-6SL 
detection. This results in access to non-documented registers for all 
non-6SL SoC. My implementation limits potential trouble to 6SL and any
SoC not detected yet.

I did not find any good hint in the reference manuals, but anyway, if
it would exist, I would assume it to be used in the freescale u-boot 
branch (which is similar to mainline u-boot).

Regards, Alexander
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox