[U-Boot] [PATCH 04/16] dm: board: Add a uclass for init functions

[Simon Glass]

Hi Igor,

On 20 March 2017 at 01:54, Igor Grinberg <grinberg at 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 <sjg at 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