Hi Igor, On 20 March 2017 at 01:54, Igor Grinberg <grinb...@compulab.co.il> wrote: > > On 03/19/17 20:59, Simon Glass wrote: >> Add a uclass to handle board init. This allows drivers to be provided to >> perform the various phases of init. Functions are provided to call all >> devices that can handle a particular phase. >> >> Signed-off-by: Simon Glass <s...@chromium.org> >> --- >> >> common/Kconfig | 31 +++++++ >> common/init/Makefile | 1 + >> common/init/board-uclass.c | 108 ++++++++++++++++++++++++ >> include/asm-generic/global_data.h | 5 ++ >> include/board.h | 170 >> ++++++++++++++++++++++++++++++++++++++ >> include/dm/uclass-id.h | 1 + >> 6 files changed, 316 insertions(+) >> create mode 100644 common/init/board-uclass.c >> create mode 100644 include/board.h > > [...] > >> diff --git a/include/board.h b/include/board.h >> new file mode 100644 >> index 0000000000..0975f5ac12 >> --- /dev/null >> +++ b/include/board.h > > [...] > >> +/* Operations for the board driver */ >> +struct board_ops { >> + /** >> + * phase() - Execute a phase of board init >> + * >> + * @dev: Device to use >> + * @phase: Phase to execute >> + * @return 0 if done, -ENOSYS if not supported (which is often fine), >> + BOARD_PHASE_CLAIMED if this was handled and that processing >> + of this phase should stop (i.e. do not send it to other >> + devices), other error if something went wrong >> + */ >> + int (*phase)(struct udevice *dev, enum board_phase_t phase); > > That looks a bit tiny interface. > This will force all the boards to define switch to figure out what the > current phase is... This might cause a problem (probably #ifdefs) in the board > code as some code will be available in SPL and some not. > > I would prefer a wider interface instead of a single entry point to any > kind of flexibility to the board driver.
Yes that certainly seems nicer. But it means we might have ~20 or so methods in the interface. Do you think most of them would be used? Also I am thinking a linker list idea might help with code size, and would need some sort of 'phase' parameter I think. > >> + >> + /** >> + * get_desc() - Get a description string for a board >> + * >> + * @dev: Device to check (UCLASS_BOARD) >> + * @buf: Buffer to place string >> + * @size: Size of string space >> + * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error >> + */ >> + int (*get_desc)(struct udevice *dev, char *buf, int size); >> +}; >> + >> +#define board_get_ops(dev) ((struct board_ops *)(dev)->driver->ops) >> + > > [...] > > > -- > Regards, > Igor. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot