[U-Boot] [PATCH 16/17] aspeed: Add AST2500/AST2400 compatible NIC Driver

[Maxim Sloyko]

On Wed, Mar 22, 2017 at 6:06 AM, Simon Glass <sjg at chromium.org> wrote:

> Hi Maxim,
>
> On 21 March 2017 at 17:44, Maxim Sloyko <maxims at google.com> wrote:
> > Hi Joe,
> >
> > Please see responses inline, simply ACK'ed comments will be addressed
> > in the next version.
> >
> > On Tue, Mar 21, 2017 at 12:32 PM, Joe Hershberger
> > <joe.hershberger at gmail.com> wrote:
> >> On Thu, Mar 16, 2017 at 4:36 PM, Maxim Sloyko <maxims at google.com>
> wrote:
> >>> The device that Aspeed uses is basically Faraday FTGMAC100, but with
> >>> some differences here and there. Since I don't have access to a
> properly
> >>> implemented FTGMAC100 though, I can't really test it and so I don't
> >>> feel comfortable claiming compatibility, even though I reused a lot of
> >>> FTGMAC100 driver code.
> >>
> >> I think it would be better to attempt to integrate this driver with
> >> the FTGMAC driver and ask others on the list who have that HW to test
> >> your changes to ensure no regressions. I prefer we have fewer drivers
> >> to maintain.
> >
> > One concern: this driver also performs its clock configuration, which
> > I believe is very specific to the SoC, so to have that compatibility
> > clock configuration needs to be externalized somehow. I don't know
> > what is the best way to do it.
>
> Generally the clock is defined by a DT property in the node, so this
> should work out OK.
>

Well, this device on this SoC needs two different clocks configured, one
for all devices and one device specific. The device speed is also hardware
strapped, so it reads the unrelated register to figure out which rate to
enable. Not to mention, it's still unclear how it's going to be done in
Linux, so somewhere else in this review Tom actually suggested to go non-DT
way with this.

Anyway, I'm going to drop this driver from this series and work this out
separately, just to keep things moving, because it looks like it raises the
largest number of concerns.


>
> Regards,
> Simon
>



-- 
*M*axim *S*loyko