<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 22, 2017 at 6:06 AM, Simon Glass <span dir="ltr"><<a href="mailto:sjg@chromium.org" target="_blank">sjg@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Maxim,<br>
<span class=""><br>
On 21 March 2017 at 17:44, Maxim Sloyko <<a href="mailto:maxims@google.com">maxims@google.com</a>> wrote:<br>
> Hi Joe,<br>
><br>
> Please see responses inline, simply ACK'ed comments will be addressed<br>
> in the next version.<br>
><br>
> On Tue, Mar 21, 2017 at 12:32 PM, Joe Hershberger<br>
> <<a href="mailto:joe.hershberger@gmail.com">joe.hershberger@gmail.com</a>> wrote:<br>
>> On Thu, Mar 16, 2017 at 4:36 PM, Maxim Sloyko <<a href="mailto:maxims@google.com">maxims@google.com</a>> wrote:<br>
>>> The device that Aspeed uses is basically Faraday FTGMAC100, but with<br>
>>> some differences here and there. Since I don't have access to a properly<br>
>>> implemented FTGMAC100 though, I can't really test it and so I don't<br>
>>> feel comfortable claiming compatibility, even though I reused a lot of<br>
>>> FTGMAC100 driver code.<br>
>><br>
>> I think it would be better to attempt to integrate this driver with<br>
>> the FTGMAC driver and ask others on the list who have that HW to test<br>
>> your changes to ensure no regressions. I prefer we have fewer drivers<br>
>> to maintain.<br>
><br>
> One concern: this driver also performs its clock configuration, which<br>
> I believe is very specific to the SoC, so to have that compatibility<br>
> clock configuration needs to be externalized somehow. I don't know<br>
> what is the best way to do it.<br>
<br>
</span>Generally the clock is defined by a DT property in the node, so this<br>
should work out OK.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Regards,<br>
Simon<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div><b>M</b>axim <b>S</b>loyko</div></div>
</div></div>