Date:   Tue, 6 Nov 2018 16:51:07 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        andrew@...n.ch
Subject: Re: [PATCH RFC net-next 0/3] net: phy: sfp: Warn when using generic
 PHY driver

On 11/6/18 4:34 PM, Russell King - ARM Linux wrote:
> On Tue, Nov 06, 2018 at 04:09:35PM -0800, Florian Fainelli wrote:
>> On 11/6/18 4:03 PM, Russell King - ARM Linux wrote:
>>> On Tue, Nov 06, 2018 at 03:38:44PM -0800, David Miller wrote:
>>>> From: Florian Fainelli <f.fainelli@...il.com>
>>>> Date: Tue,  6 Nov 2018 15:29:10 -0800
>>>>
>>>>> This patch series allows warning an user that the generic PHY driver(s)
>>>>> are used when a SFP incorporates a PHY (e.g: 1000BaseT SFP) which is
>>>>> likely not going to work at all.
>>>>>
>>>>> Let me know if you would want to do that differently.
>>>>
>>>> Is there ever a possibility that the generic PHY driver could work
>>>> in an SFP situation?
>>>
>>> I don't yet see the reason for Florian's patch series - all the Marvell
>>> 88e1111 based modules I have, or have come across in information from
>>> manufacturers self-configure themselves and don't really need the
>>> Marvell 1G PHY driver.
>>>
>>> For example, the Source Photonics were offering a range of 1GbaseT
>>> modules with the 88e1111 programmed in different modes, but published
>>> instructions for the register accesses to configure them differently
>>> (eg, SGMII vs 1000base-X interface facing the MAC).  Depending on
>>> the module part number determines which mode the PHY has been
>>> programmed to come up in.
>>>
>>> So in theory, you don't need any PHY driver for these modules - but
>>> it's useful to have a functional PHY driver to be able to read out
>>> the negotiated flow control results.
>>>
>>> I'd like more information from Florian about the reasoning behind
>>> this patch series before it's merged.
>>>
>>
>> The module that I am using [1] would not work, as in , no link up being
>> reported without turning on the Marvell PHY driver:
>>
>> https://www.amazon.com/dp/B01LW2P72V/ref=twister_B07F3WQJQX?_encoding=UTF8&psc=1
>>
>> this module uses a 88E1111 PHY as well (OUI: 0x01410cc2).
> 
> From the above URL:
> 
>      * This is 1000M SFP-T Transceiver, not 10/100/1000M Multi-Rate SFP-T. If
>        you want to buy 10/100/1000M Multi-Rate SFP-T, pls contact us.10Gtek
>        offer more compatible options, if your brands not listed above, pls
>        contact us.
> 
> I wonder if this is like the Source Photonics situation, where the
> 1000base-T only variant of their module uses 1000base-X on the MAC
> side, whereas their 10/100/1000base-T variant uses SGMII.  The only
> difference between these are the part numbers and the programming
> of the 88E1111 to tell it which mode to default to for the host
> side.  (There's no true way to know from the EEPROM whether a module
> wants SGMII or 1000base-X.)
> 
> What I also gather is that this is a 10Gtek-manufactured version of
> the Ubiquiti UF-RJ45-1G - the original Ubiquiti version supports
> 10/100/1G speeds which would require the 88e1111 to configure for
> a SGMII host interface by default.
> 
> Now, the reason that modules with an 88E1111 configured to default to
> 1000base-X will work when the marvell PHY driver is present, but not
> with the generic driver is that the marvell PHY driver will see that
> SFP/phylink is wanting to use SGMII mode, and the Marvell PHY driver
> reprograms the PHY to use SGMII.  This is only a problem for these
> modules.
> 
> So, in so far as your patch 3 goes to give a hint that the Marvell
> driver should be selected, that's correct.
> 
> However, where the 88e1111 is configured for SGMII by default, the
> Marvell driver shouldn't be required, and I wonder whether we ought
> to be issuing a warning in that case.  The problem, however, is there
> is no way to know for certain.
> 
> We could have modules that do not use the Marvell PHY, and if we don't
> have a PHY driver for their particular PHY, do we want a warning to be
> issued?

Another approach could be to maintain a list of modules that do not work
with the generic PHY driver and therefore require a specialized driver,
in that case we could even go as far as not letting sfp_sm_probe_phy()
return success. Not sure how well things would scale, probably not too
bad given there are only a handful of users of the SFP framework thus far...

> 
> The whole 1000base-X vs SGMII with SFP modules is all very icky. :(
> 
-- 
Florian

Powered by blists - more mailing lists