[PATCH v2 10/12] sun5i: add support for DIP detection to CHIP board

Köry Maincent kory.maincent at bootlin.com
Tue Mar 2 17:14:44 CET 2021


Hello Maxime,

Thanks for your review.

On Tue, 2 Mar 2021 17:05:38 +0100
Maxime Ripard <maxime at cerno.tech> wrote:
 
> The split of that patch and the previous one is weird: you're adding a
> Kconfig symbol doing close to nothing in patch 9, and you add the logic
> behind it here
> 
> It would be more natural to have the Kconfig option and the code here,
> and 

Ok I will merge them into one commit.

> 
> > diff --git a/board/sunxi/chip.c b/board/sunxi/chip.c
> > new file mode 100644
> > index 0000000000..fc3d14e497
> > --- /dev/null
> > +++ b/board/sunxi/chip.c
> > @@ -0,0 +1,104 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2021
> > + * Köry Maincent, Bootlin, <kory.maincent at bootlin.com>
> > + * Based on initial code from Maxime Ripard
> > + */
> > +
> > +#include <common.h>
> > +#include <malloc.h>
> > +#include <dm.h>
> > +#include <w1.h>
> > +#include <w1-eeprom.h>
> > +#include <dm/device-internal.h>
> > +
> > +#include <asm/arch/gpio.h>
> > +
> > +#include <extension_board.h>
> > +
> > +#ifdef CONFIG_CMD_EXTENSION
> > +
> > +#define for_each_1w_device(b, d) \
> > +	for (device_find_first_child(b, d); *d;
> > device_find_next_child(d))  
> 
> The name is inconsistent with the rest of the 1-wire acronyms in the
> file (1w vs w1)

Yes, I didn't notice this.

> 
> Also, reusing macro arguments is fairly dangerous but since it's used
> only once we don't really need that macro?

It was to increase the readability of the loop, but if you judge it not
pertinent I can remove it.



More information about the U-Boot mailing list