[U-Boot] [PATCH v3 7/7] mx53: Add Board support for GE PPD

Martyn Welch martyn.welch at collabora.co.uk
Wed Nov 1 17:56:00 UTC 2017


On Wed, 2017-11-01 at 16:53 +0100, Stefano Babic wrote:
> Hi Martyn, Peter,
> 
> On 01/11/2017 15:23, Martyn Welch wrote:
> > From: Peter Senna Tschudin <peter.senna at collabora.com>
> > 
> > Create board support for GE PPD, based on mx53loco.
> > 
> > Use mx53ppd_defconfig make target to configure for this board.
> > 
> > Signed-off-by: Peter Senna Tschudin <peter.senna at collabora.com>
> > Signed-off-by: Ian Ray <ian.ray at ge.com>
> > Signed-off-by: Nandor Han <nandor.han at ge.com>
> > Signed-off-by: Martyn Welch <martyn.welch at collabora.co.uk>
> > Cc: Stefano Babic <sbabic at denx.de>
> > ---
> > Changes for v2:
> >    - Replacing boot count related defines with default config.
> > 
> > Changes for v3:
> >    - Update config due to BOOTCOUNT Kconfig addition.
> > 
> >  arch/arm/include/asm/mach-types.h       |   1 +
> >  arch/arm/mach-imx/mx5/Kconfig           |   5 +
> >  board/freescale/mx53ppd/Kconfig         |  17 ++
> >  board/freescale/mx53ppd/MAINTAINERS     |   7 +
> >  board/freescale/mx53ppd/Makefile        |  12 +
> >  board/freescale/mx53ppd/imximage.cfg    |  87 ++++++
> >  board/freescale/mx53ppd/mx53ppd.c       | 487 ++++++++++++++++++++++++++++++++
> >  board/freescale/mx53ppd/mx53ppd_video.c | 123 ++++++++
> >  board/freescale/mx53ppd/ppd_gpio.h      |  96 +++++++
> >  board/freescale/mx53ppd/vpd_reader.c    | 230 +++++++++++++++
> >  board/freescale/mx53ppd/vpd_reader.h    |  25 ++
> >  configs/mx53ppd_defconfig               |  39 +++
> >  include/configs/mx53ppd.h               | 246 ++++++++++++++++
> >  13 files changed, 1375 insertions(+)
> >  create mode 100644 board/freescale/mx53ppd/Kconfig
> >  create mode 100644 board/freescale/mx53ppd/MAINTAINERS
> >  create mode 100644 board/freescale/mx53ppd/Makefile
> >  create mode 100644 board/freescale/mx53ppd/imximage.cfg
> >  create mode 100644 board/freescale/mx53ppd/mx53ppd.c
> >  create mode 100644 board/freescale/mx53ppd/mx53ppd_video.c
> >  create mode 100644 board/freescale/mx53ppd/ppd_gpio.h
> >  create mode 100644 board/freescale/mx53ppd/vpd_reader.c
> >  create mode 100644 board/freescale/mx53ppd/vpd_reader.h
> >  create mode 100644 configs/mx53ppd_defconfig
> >  create mode 100644 include/configs/mx53ppd.h
> > 
> > diff --git a/arch/arm/include/asm/mach-types.h b/arch/arm/include/asm/mach-types.h
> > index 9f82efe..2a257cc 100644
> > --- a/arch/arm/include/asm/mach-types.h
> > +++ b/arch/arm/include/asm/mach-types.h
> > @@ -5057,4 +5057,5 @@
> >  #define MACH_TYPE_NASM25               5112
> >  #define MACH_TYPE_TOMATO               5113
> >  #define MACH_TYPE_OMAP3_MRC3D          5114
> > +#define MACH_TYPE_MX53_PPD             5134
> 
> Ouch..still an ancient Kernel ? 2.6.35, maybe ?
> 

No, currently 4.11.

> Even in that case, we agreed to not touch anymore mach-types.h. The
> value should go into the board configuration file,
> 
> 	 #define CONFIG_MACH_TYPE   5134
> 

I hadn't realised I could drop it. Now dropped entirely

<snip>

> > +void check_time(void)
> > +{
> > +	int ret, i;
> > +	struct rtc_time tm;
> > +	u8 retry = 3;
> > +
> > +	unsigned int current_i2c_bus = i2c_get_bus_num();
> > +
> > +	ret = i2c_set_bus_num(CONFIG_SYS_RTC_BUS_NUM);
> > +	if (ret < 0)
> > +		return;
> > +
> > +	rtc_init();
> > +
> > +	for (i = 0; i < retry; i++) {
> > +		ret = rtc_get(&tm);
> > +		if(!ret || ret == -EINVAL) {
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (ret < 0) {
> > +		env_set("rtc_status", "RTC_ERROR");
> > +	}
> > +
> > +	if (tm.tm_year > 2037) {
> > +		tm.tm_sec  = 0;
> > +		tm.tm_min  = 0;
> > +		tm.tm_hour = 0;
> > +		tm.tm_mday = 1;
> > +		tm.tm_wday = 2;
> > +		tm.tm_mon  = 1;
> > +		tm.tm_year = 2036;
> 
> Agree this will be a problem in 2037, but I guess setting the time back
> is not the right solution. I am missing the reason, apart to convince
> kernel that time does not go by.
> 

The device has mitigation in user-space to warn the user should it be
running when the time approaches this date (either because it's actually
2037 or if the device has been accidentally configured to think it is
2037). We believe there exists a risk that the device will fail to boot
correctly if we try booting after the epoch has overflowed.

To us it is preferable that a device boots with the wrong date, but in
an otherwise usable state, rather than doesn't boot. This is a medical
device, so it could actually be a matter of life and death...

<snip>

> > +
> > +static int do_lcd_enable(cmd_tbl_t *cmdtp, int flag, int argc,
> > +			char * const argv[])
> > +{
> > +	lcd_enable();
> > +	return 0;
> > +}
> > +
> 
> If this is related to LCD, why the function is not in mx53ppd_video.c ?
> 

I'll move it.

> > +U_BOOT_CMD(
> > +	ppd_lcd_enable,	1,	1,	do_lcd_enable,
> > +	"enable PPD LCD",
> > +	"no parameters"
> > +);

<snip>

> > diff --git a/board/freescale/mx53ppd/vpd_reader.c b/board/freescale/mx53ppd/vpd_reader.c
> > new file mode 100644
> > index 0000000..ea6daf7
> > --- /dev/null
> > +++ b/board/freescale/mx53ppd/vpd_reader.c
> > @@ -0,0 +1,230 @@
> > +/*
> > + * Copyright 2016 General Electric Company
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +#include "vpd_reader.h"
> > +
> > +#include <linux/bch.h>
> > +#include <stdlib.h>
> > +#include <errno.h>
> > +
> > +
> > +/* BCH configuration */
> > +
> > +const struct {
> > +	int header_ecc_capability_bits;
> > +	int data_ecc_capability_bits;
> > +	unsigned int prim_poly;
> > +	struct {
> > +		int min;
> > +		int max;
> > +	} galois_field_order;
> > +} bch_configuration = {
> > +	.header_ecc_capability_bits = 4,
> > +	.data_ecc_capability_bits = 16,
> > +	.prim_poly = 0,
> > +	.galois_field_order = {
> > +		.min = 5,
> > +		.max = 15,
> > +	},
> > +};
> > +
> > +static int calculate_galois_field_order(size_t source_length)
> > +{
> > +	int gfo = bch_configuration.galois_field_order.min;
> > +
> > +	for (; gfo < bch_configuration.galois_field_order.max &&
> > +	     ((((1 << gfo) - 1) - ((int)source_length * 8)) < 0);
> > +	     gfo++) {
> > +	}
> > +
> > +	if (gfo == bch_configuration.galois_field_order.max) {
> > +		return -1;
> > +	}
> > +
> > +	return gfo + 1;
> > +}
> > +
> 
> mmmhhh....I do not like this. You have already a handling for VPD header
> (your custom header) for the bx50v3 board. You duplicate here a lot of
> code, and I am missing what is different and what is identical.
> 
> I think you should move code in a common directory (as freescale boards
> do), like in ge/common, and the same code is reused by all your board
> instead of duplicating it each time.
> 

Yup, sorry, I'd not thought about it also being in the bx50v3 support.
I'll make this common.

Martyn



More information about the U-Boot mailing list