> -----Original Message----- > From: Stas Sergeev [mailto:stsp@xxxxxxx] <snip> > But have you looked into the patch I pointed previously? > https://lkml.org/lkml/2015/7/20/711 > You code will likely clash with it because my patch extends > of_phy_register_fixed_link(). > I admin I failed to grasp the details of your change - the lack of ample context Lines makes it a bit difficult. I'm sure your change could be merged then the of parsing could be separated from the actual fixed_phy_register() call if anyone cares about that. > > Then I could call only of_* functions but the end result would be the same > > and of_phy_register_fixed_phy() would not justify its existence that much... > You didn't say you wanted to obsolete the of_phy_register_fixed_phy(). > Since it is there (and even changed by me in a way your > patch will likely clash), IMHO it would be better if it is used, > rather than copy/pasted into the driver. Please note I was referring to a fictional new function that would embed the call to fixed_phy_register(). I was not talking about some existing API, just about a new of_call named of_phy_register_fixed_phy() that would in the end be called by of_phy_register_fixed_link() and by some drivers that want to get in the middle of things and get a hold on status. Maybe the fact we're reviewing two patches in one thread is what makes the discussion less than optimal. > > The getter for status you suggest would be fine, but not sure how one > > would retrieve > > it from the mac_node unless we change of_phy_register_fixed_link() to > > return the > > pointer to phy (and all the drivers that use it...). > If you look for instance to mvneta.c, you'll find the following: > --- > err = of_phy_register_fixed_link(dn); > /* In the case of a fixed PHY, the DT node associated > * to the PHY is the Ethernet MAC DT node. > */ > phy_node = of_node_get(dn); > ... > phy = of_phy_find_device(dn); > --- > > So the answer is: just use the same mac_node for both. I understand, I'll use this approach although is suboptimal imho to scan the device tree again to get a phy pointer that you need just to get some of info that was parsed in a call you just made. > >> Also I meant the description should have been in the patch, > >> not in the e-mail. :) You only wrote _what_ the patch does > >> (which is of course obvious from the code itself), but not > >> _why_ and _what was fixed_ (what didn't work). > >> > > If you refer to the first, separation patch, I thought the description was > enough: > > > > of: separate fixed link parsing from registration > > > > Some drivers may need > "may need"? I don't understand. > If it is a fix, then they _do need_, and in this case it should > be specified what was broken and what is fixed. > If it is just a clean-up, then "may need" may suffice, but it > was not mentioned it is a clean-up. So I still don't know what > this patch is all about. > "Some drivers" - which ones? The ones that are limited to > the purely fixed links, and never support AN or MDIO? > Or some other drivers too? > So really, the description sounds very cryptic to me. Mine, when there is a fixed link node, maybe others. When there isn't any fixed link node, the internal PHY config defaults to 1G full duplex AN enabled and adjust link takes care of things. > > > to parse the fixed link values before registering > > the fixed link phy. Separate the parsing from the actual registration > > and provide an export for the added parsing function. > > > > Signed-off-by: Madalin Bucur <madalin.bucur@xxxxxxxxxxxxx> > > > > For this one it was a bit brief, I admit - the longer version would be that > before it > > we were not using from fixed link anything else but the fact the link was > fixed > > (ignored actual speed, duplex values there) > And what didn't work as the result? > > > and this patch tries to fix that. > What started to work after that patch that didn't without it? 10M half duplex for instance I'd close this thread for now and use in my driver of_phy_find_device(mac_node). Thank you, Madalin ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
