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

[Joe Hershberger]

Hi Bin,

On Tue, Aug 25, 2015 at 9:36 PM, 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().

I agree we shouldn't use them if it makes sense not to.

> 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?

I think we can evaluate that for each case. We don't necessarily want
the uclass to be a union of all crazy board features.

>> 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 agree.

>> I guess it can just be put back when such a board is converted.
>>
>
> Regards,
> Bin