[U-Boot] [PATCH 3/9] dm: eth: Do not call board_eth_init() or cpu_eth_init()

[Simon Glass]

Hi,

On 25 August 2015 at 19:36, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> On Wed, Aug 26, 2015 at 10:24 AM, Joe Hershberger
> <joe.hershberger at gmail.com> wrote:
>> Hi Bin,
>>
>> On Tue, Aug 25, 2015 at 8:29 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Joe,
>>>
>>> On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger
>>> <joe.hershberger at gmail.com> wrote:
>>>> Hi Bin,
>>>>
>>>> On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>> With driver model, board_eth_init() or cpu_eth_init() is not needed.
>>>>> Remove the call to these in eth_common_init().
>>>>
>>>> I'm pretty sure Simon needed this when he ported some allwinner board
>>>> originally.
>>>>
>>>> 3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing
>>>> Ethernet init for driver model)
>>>>
>>>
>>> I think my patch does not break Simon's. My patch only comments out
>>> the call to board_eth_init() or cpu_eth_init() which is not needed for
>>> driver model. Other stuff in eth_common_init() is still there. In
>>> fact, my patch series also needs phy_init() call (required by pch_gbe
>>> driver).
>>
>> Even if it doesn't break Simon's board, why remove the ability for a
>> board to add eth_init code. You're trying to say that there is no case
>> where a DM board would need to init anything related to eth. That
>> seems unlikely.
>>
>
> I believe with driver model, we should avoid these calls. All the
> drive initialization needs to be done in the bind or probe phase, and
> called by driver model automatically. Like pci_eth_init() which just
> calls non-dm ethernet drivers, and pci_eth_init() can be called from
> board_eth_init() or cpu_eth_init(). But I think you are right, there
> might be some board-specific thing to be put there, even right now it
> does not break any board. But that's maybe we don't have proper driver
> model uclass to handle these misc things?

Normally the board init should happen in board_init(). We don't
currently have a good way to handle 'associated init' for drivers.

For example, if an Ethernet port needs some pinmux settings, or some
clock settings, with driver model we currently have to init this in
board_init() so it is not 'lazy init', it always happens at start of
day.

I don't actually think this is a problem in practice, but it is not in
the spirit of driver model. Drivers should 'pull in' the init that
they need and we should make that work automatically.

But it is very important the we do not put this init into the drivers
themselves. The drivers need to be generic. E.g. an Ethernet driver
should be able to operate on any board that has that Ethernet
controller. The setup to make the controller available on pins, enable
it clock it correctly should not be in driver code.

Soon we will have pinctrl which will deal with the pinmux problem. We
already have a clock framework would could handle clocking. So
progress is being made.

I wonder whether we will end up wanting a way to request that the
resources for a driver be activated, in a general sense. But until we
have a use case I'm not keen to spend much time thinking about it.

>
>> Also, why is it hurting your board to have an optional call to such a
>> function. Presumably you just don't define those functions and you're
>> fine, right?
>>
>
> It does not hurt but I think at least we should comment out the
> following printf for DM.
>
>     } else {
>         printf("Net Initialization Skipped\n");
>     }
>
> This is misleading message.
>
>> I guess it can just be put back when such a board is converted.
>>
>
> Regards,
> Bin

Regards,
Simon