Re: [PATCH] ARM: rdu2: program e1000 iNVM

[Andrey Smirnov <andrew.smirnov@gmail.com>]

On Wed, Jun 20, 2018 at 2:51 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Check if the iNVM is already correctly written. If it isn't write
> the correct value in, so the device uses the correct PCI ID for the
> Linux driver.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  arch/arm/boards/zii-imx6q-rdu2/board.c | 29 ++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm/boards/zii-imx6q-rdu2/board.c b/arch/arm/boards/zii-imx6q-rdu2/board.c
> index 265f97e2f464..984ca42a3772 100644
> --- a/arch/arm/boards/zii-imx6q-rdu2/board.c
> +++ b/arch/arm/boards/zii-imx6q-rdu2/board.c
> @@ -15,6 +15,7 @@
>
>  #include <common.h>
>  #include <envfs.h>
> +#include <fs.h>
>  #include <gpio.h>
>  #include <init.h>
>  #include <mach/bbu.h>
> @@ -151,3 +152,31 @@ static int rdu2_devices_init(void)
>         return 0;
>  }
>  device_initcall(rdu2_devices_init);
> +
> +static int rdu2_i210_invm(void)
> +{
> +       int fd;
> +       u32 val;
> +
> +       if (!of_machine_is_compatible("zii,imx6q-zii-rdu2") &&
> +           !of_machine_is_compatible("zii,imx6qp-zii-rdu2"))
> +               return 0;
> +
> +       fd = open("/dev/e1000-invm0", O_RDWR);
> +       if (fd < 0) {
> +               pr_err("could not open e1000 iNVM device!\n");
> +               return fd;
> +       }
> +
> +       pread(fd, &val, sizeof(val), 0);

I know that pread() will succeed 99.9% of the time, but wrapping it in
WARN_ON (or any other form of error indication) might be helpful in
cases where it doesn't.

> +       if (val == 0x157b1a11) {
> +               pr_debug("i210 already programmed correctly\n");
> +               return 0;
> +       }
> +
> +       val = 0x157b1a11;

Speaking of magic, maybe this would be better as a named constant?

> +       pwrite(fd, &val, sizeof(val), 0);
> +

Maybe some error indication here as well?

> +       return 0;

I don't see the file being close here or in the early return above, is
this intentional?

Thanks,
Andrey Smirnov

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