On Fri, May 13, 2016 at 11:46:33AM -0700, Doug Anderson wrote: > On Thu, May 12, 2016 at 6:02 PM, Shawn Lin <shawn.lin at rock-chips.com> wrote: > > On 2016/5/13 6:43, Brian Norris wrote: > >> @@ -154,6 +167,20 @@ static int rockchip_emmc_phy_power_on(struct phy > >> *phy) > >> struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy); > >> int ret = 0; > >> > >> + /* DLL operation: 170 to 200 MHz */ > > > > > > What is 170 here? Should we expose them to dt instead of hardcoding > > them? > > This was probably my fault. I did some searching and found > <https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf>. > It appears to be docs for a similar (but not identical) PHY. We were > looking at it to try to get more clarity on some bits that were hard > to understand in the docs we had. > > In that doc there appear to be 3 bits for selecting the DLL operation > and they have ranges defined. In Rockchip's PHY there are only 2 > bits. Thus things don't map totally properly. > > Anyway, comment should probably be removed. [...] > So overall: > > * Should re-spin and remove the comment about 170 MHz. > > * I think this could land as-is other than the comment. Right, will fix the first bullet point. Brian