[U-Boot] [PATCH v2] Add support for Microchip LAN78xx

[Marek Vasut]

On 04/07/2017 05:43 PM, Yuiko.Oshino at microchip.com wrote:
> From: Yuiko Oshino <yuiko.oshino at microchip.com>
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Monday, March 27, 2017 9:30 AM
>> To: Yuiko Oshino - C18177; u-boot at lists.denx.de
>> Subject: Re: [PATCH v2] Add support for Microchip LAN78xx
>>
>> On 03/27/2017 01:59 PM, Yuiko.Oshino at microchip.com wrote:
>>> From: Yuiko Oshino <yuiko.oshino at microchip.com>
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Friday, March 24, 2017 2:29 PM
>>>> To: Yuiko Oshino - C18177; u-boot at lists.denx.de
>>>> Subject: Re: [PATCH v2] Add support for Microchip LAN78xx
>>>>
>>>> On 03/24/2017 07:25 PM, Yuiko.Oshino at microchip.com wrote:
>>>>> From: Yuiko Oshino <yuiko.oshino at microchip.com>
>>>>>> -----Original Message-----
>>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>>> Sent: Friday, March 10, 2017 2:19 PM
>>>>>> To: Yuiko Oshino - C18177; u-boot at lists.denx.de
>>>>>> Subject: Re: [PATCH v2] Add support for Microchip LAN78xx
>>>>>>
>>>>>> On 03/10/2017 07:38 PM, Yuiko.Oshino at microchip.com wrote:
>>>>>>> From: Yuiko Oshino <yuiko.oshino at microchip.com>
>>>>>>>
>>>>>>> Add support for Microchip LAN7800 and 7850, USB to 10/100/1000
>>>>>>> Ethernet Controllers
>>>>>>>
>>>>>>> Signed-off-by: Yuiko Oshino <yuiko.oshino at microchip.com>
>>>>>>> Cc: Marek Vasut <marex at denx.de>
>>>>>>> ---
>>>>>>> Changes for v2:
>>>>>>>    - lan78xx.c header comment cleanup
>>>>>>>
>>>>>
>>>>> [...]
>>>>>
>>>>>>
>>>>>> Why does a lot of this stuff look like the SMSC95xx driver ?
>>>>>> Do we even need a separate driver or can the SMSC95xx driver be
>>>>>> updated to support the LAN78xx ?
>>>>>>
>>>>>
>>>>> Marek,
>>>>> Thank you so much for your review.
>>>>> The LAN78xx's register addresses and bit definitions are not
>>>>> compatible to
>>>> our old part LAN95xx (==SMSC95xx).
>>>>> The new devices LAN78xx and LAN75xx device have similarities. I plan
>>>>> to
>>>> support these two devices in this driver.
>>>>> I will take care of all your comments and will submit a new PATCH
>>>>> with a file
>>>> name change to lan7x.c.
>>>>>
>>>>
>>>> Is the rename really mandatory ?
>>>>
>>> Yes, if we are going to make one file that supports both LAN78xx and
>> LAN75xx, it's very important for us to make the driver file name to support
>> both devices.
>>
>> The filename has nothing to do with which chips are supported though ?
>>
>>>> Then again, I looked into Linux and that one has two separate drivers .
>>>> So I wonder if it's really a good idea. Maybe we can share some
>>>> common code and have the rest separate ?
>>>
>>> Marek,
>>> 1. we can submit LAN78xx and LA75xx drivers separately like in Linux. We do
>> prefer this option.
>>> 2. we can submit one LAN7x.c that supports both LAN78xx and LAN75xx in
>> one file.
>>> Could you pick one?
>>
>> Can we have option 3 -- pull common code into some file and make both
>> drivers use the common code ?
> 
> Hi Marek,

Hi Yuiko,

> Sure, we will do the option 3 with lan75xx.c, lan78xx.c, lan7xc, and lan7x.h as you like.

Well don't you think this makes sense from the code reuse perspective ?

> Should I keep this thread and make [PATCH v3] Add support for Microchip LAN78xx to submit?
> Even though the new patch will also include LAN75xx as well?

That's fine

> Or new thread and include the LAN75xx in the title?

New thread please. I think it'll be more patches and there'll be some
refactoring first anyway.

> Please let me know.
> 
> Thank you again!

Thanks for keeping up the good work !

-- 
Best regards,
Marek Vasut