Hello Maxime, Thanks for your review.
On Tue, 2 Mar 2021 17:05:38 +0100 Maxime Ripard <[email protected]> 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, <[email protected]> > > + * 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.
