Re: [PATCH net-next 2/2] net: phy: sfp: enable i2c-bus detection on ACPI based systems

[Ruslan Babayev <ruslan@xxxxxxxxxxx>]

Andrew Lunn writes:

> On Sun, May 05, 2019 at 03:05:23PM -0700, Ruslan Babayev wrote:
>> Lookup I2C adapter using the "i2c-bus" device property on ACPI based
>> systems similar to how it's done with DT.
>>
>> An example DSD describing an SFP on an ACPI based system:
>>
>> Device (SFP0)
>> {
>>     Name (_HID, "PRP0001")
>>     Name (_DSD, Package ()
>>     {
>>         ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>         Package () {
>>             Package () { "compatible", "sff,sfp" },
>>             Package () { "i2c-bus", \_SB.PCI0.RP01.I2C.MUX.CH0 },
>>         },
>>     })
>> }
>
> Hi Ruslan
>
> So this gives you the I2C bus. But what about the 6 GPIOs? And the
> maximum power property? You are defining the ACPI interface which from
> now on everybody has to follow. So it would be good to make it
> complete. ACPI also seems to be poorly documented. There does not
> appear to be anything like Documentation/devicetree. So having one
> patch, with a good commit message, which implements everything makes
> it easier for those that follow.
>
Hi Andrew,

I had the GPIOs and the "maximum-power" property in my ACPI snippet initially,
but then decided to take it out thinking it was not relevant for the
current patch. I can add the missing pieces back in V2.
This is what it would like:

Device (SFP0)
{
    Name (_HID, "PRP0001")
    Name (_CRS, ResourceTemplate()
    {
        GpioIo(Exclusive, PullDefault, 0, 0, IoRestrictionNone,
               "\\_SB.PCI0.RP01.GPIO", 0, ResourceConsumer)
            { 0, 1, 2, 3, 4 }
    })
    Name (_DSD, Package ()
    {
        ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package () {
            Package () { "compatible", "sff,sfp" },
            Package () { "i2c-bus", \_SB.PCI0.RP01.I2C.MUX.CH0 },
            Package () { "maximum-power-milliwatt", 1000 },
            Package () { "tx-disable-gpios", Package () { ^SFP0, 0, 0, 1} },
            Package () { "reset-gpio",       Package () { ^SFP0, 0, 1, 1} },
            Package () { "mod-def0-gpios",   Package () { ^SFP0, 0, 2, 1} },
            Package () { "tx-fault-gpios",   Package () { ^SFP0, 0, 3, 0} },
            Package () { "los-gpios",        Package () { ^SFP0, 0, 4, 1} },
        },
    })
}


> This appears to be enough to get a very minimal SFP instantiated. But
> then what?  How are you using it? How do you instantiate a Phylink
> instance for the MAC? How do you link the SFP to the Phylink?
>
> Before accepting this patch, i would like to know more about the
> complete solution.
>
> Thanks
> 	Andrew

I haven't gotten that far yet, but for the Phylink I was thinking something along the
lines of:

Device (PHY0)
{
    Name (_HID, "PRP0001")
    Name (_DSD, Package ()
    {
        ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package () {
            Package () { "compatible", "ethernet-phy-ieee802.3-c45" },
            Package () { "sfp", \_SB.PCI0.RP01.SFP0 },
        },
    })
}

Phylink is already using the fwnode_property_get_reference_args(fwnode,
"sfp", ...), so it should work with ACPI.

I don't have a complete solution working yet. With these patches
I was hoping to get some early feedback.

Thanks,
Ruslan