mbox series

[RFC,0/3] acpi: Add acpi mdio support code

Message ID cover.1541660504.git.dongsheng.wang@hxt-semitech.com (mailing list archive)
Headers show
Series acpi: Add acpi mdio support code | expand

Message

Wang, Dongsheng Nov. 8, 2018, 7:21 a.m. UTC 
Originally I just push "phy-handle" support for ACPI on the QCOM QDF2400
platform. After some discussion and following Andrew's advice, I send
out with a generic version of ACPI.

Current there is no clear documentation about MDIO/PHY for ACPI, so when
I reading some documents about ACPI [1], I think we just need to reuse the
DT binding in the ACPI.[2]. However, this series of patches are not
fully compatible with all contents specified in DT binding.

The most important thing about this iseries is link the phy device and
fwnode of acpi. Besides, we need to carry out bus scan at the mdio
register. Therefore, I am not compatible with more DT binding properties
in this series of patches. More support will be in the follow-up patches
support, or some people do the support.

Example:
Based on ACPI doc:
    Documentation/acpi/dsd/data-node-references.txt
    Documentation/acpi/dsd/graph.txt
With _DSD device properties we can finally do this:
    Device (MDIO) {
        Name (_DSD, Package () {
            ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () { Package () { "ethernet-phy@0", PHY0 }, }
        })
        Name (PHY0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () { Package () { "reg", 0x0 }, }
        })
    }

    Device (MACO) {
        Name (_DSD, Package () {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () { Package () { "phy-handle", \_SB.MDIO, "ethernet-phy@0" }, }
        })
    }

Tested: QDF2400 (ACPI), buildin/insmod/rmmod

[1]:
Documentation/acpi/dsd/data-node-references.txt
Documentation/acpi/dsd/graph.txt

[2]:
Documentation/devicetree/bindings/phy/phy-bindings.txt
https://lore.kernel.org/patchwork/patch/597296/

Wang Dongsheng (3):
  acpi: Add acpi mdio support code
  net: qcom/emac: split phy_config to mdio bus create and get phy device
  net: qcom/emac: add phy-handle support for ACPI

 drivers/acpi/Kconfig                          |   6 +
 drivers/acpi/Makefile                         |   1 +
 drivers/acpi/acpi_mdio.c                      | 167 ++++++++++++++++++
 drivers/net/ethernet/qualcomm/emac/emac-mac.c |  19 +-
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 142 ++++++++++-----
 drivers/net/phy/mdio_bus.c                    |   3 +
 include/linux/acpi_mdio.h                     |  82 +++++++++
 7 files changed, 369 insertions(+), 51 deletions(-)
 create mode 100644 drivers/acpi/acpi_mdio.c
 create mode 100644 include/linux/acpi_mdio.h

Comments

Andrew Lunn Nov. 8, 2018, 11:23 p.m. UTC | #1 
On Thu, Nov 08, 2018 at 03:21:29PM +0800, Wang Dongsheng wrote:
> Originally I just push "phy-handle" support for ACPI on the QCOM QDF2400
> platform. After some discussion and following Andrew's advice, I send
> out with a generic version of ACPI.
> 
> Current there is no clear documentation about MDIO/PHY for ACPI, so when
> I reading some documents about ACPI [1], I think we just need to reuse the
> DT binding in the ACPI.[2]. However, this series of patches are not
> fully compatible with all contents specified in DT binding.
> 
> The most important thing about this iseries is link the phy device and
> fwnode of acpi. Besides, we need to carry out bus scan at the mdio
> register. Therefore, I am not compatible with more DT binding properties
> in this series of patches. More support will be in the follow-up patches
> support, or some people do the support.
> 
> Example:
> Based on ACPI doc:
>     Documentation/acpi/dsd/data-node-references.txt
>     Documentation/acpi/dsd/graph.txt
> With _DSD device properties we can finally do this:
>     Device (MDIO) {
>         Name (_DSD, Package () {
>             ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>             Package () { Package () { "ethernet-phy@0", PHY0 }, }
>         })
>         Name (PHY0, Package() {
>             ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>             Package () { Package () { "reg", 0x0 }, }
>         })

I don't know much about ACPI. I do know DT. MDIO busses can have
multiple PHYs on them. Is the following valid to list two PHYs?

     Device (MDIO) {
         Name (_DSD, Package () {
             ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
             Package () { Package () { "ethernet-phy@0", PHY0 }, }
         })
         Name (PHY0, Package() {
             ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
             Package () { Package () { "reg", 0x0 }, }
         })
         Name (_DSD, Package () {
             ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
             Package () { Package () { "ethernet-phy@10", PHY1 }, }
         })
         Name (PHY1, Package() {
             ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
             Package () { Package () { "reg", 0x10 }, }
         })
     }


An MDIO bus can also have more than PHYs on them. There can be
Ethernet switches. Broadcom also have some with generic PHY devices on
them, and other odd things. That means whatever is on an MDIO bus is a
device in the Linux device model. How does that work? Do we need some
form Device (PHY) {}?

     Device (MDIO) {
         Device (PHY) {
             Name (_DSD, Package () {
                 ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
                 Package () { Package () { "ethernet-phy@0", PHY0 }, }
             })
             Name (PHY0, Package() {
                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                 Package () { Package () { "reg", 0x0 }, }
             })
         }
         Device (PHY) {
             Name (_DSD, Package () {
                 ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
                 Package () { Package () { "ethernet-phy@10", PHY1 }, }
             })
             Name (PHY1, Package() {
                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                 Package () { Package () { "reg", 0x10 }, }
             })
         Device (SWITCH) {
             Name (_DSD, Package () {
                 ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
                 Package () { Package () { "switch@11", SWITCH0 }, }
             })
             Name (SWITCH0, Package() {
                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                 Package () { Package () { "reg", 0x11 }, }
             })
         }
      }

I'm just trying to ensure whatever is defined is flexible enough that
we really can later support everything which DT does. We have PHYs on
MDIO busses, inside switches, which are on MDIO busses, which are
inside Ethernet interfaces, etc.

An MDIO bus is very similar to an i2c bus. How is that described in
ACPI? Anything we can learn from that?

      Thanks
         Andrew
Timur Tabi Nov. 9, 2018, 12:37 a.m. UTC | #2 
On 11/8/18 5:23 PM, Andrew Lunn wrote:
> I don't know much about ACPI. I do know DT. MDIO busses can have
> multiple PHYs on them. Is the following valid to list two PHYs?
> 
>       Device (MDIO) {
>           Name (_DSD, Package () {
>               ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>               Package () { Package () { "ethernet-phy@0", PHY0 }, }
>           })
>           Name (PHY0, Package() {
>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>               Package () { Package () { "reg", 0x0 }, }
>           })
>           Name (_DSD, Package () {
>               ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>               Package () { Package () { "ethernet-phy@10", PHY1 }, }
>           })
>           Name (PHY1, Package() {
>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>               Package () { Package () { "reg", 0x10 }, }
>           })
>       }

You can't have the same DSD twice.  It would need to look like this:

          Name (PHY1, Package() {
              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
              Package () { Package () { "reg", 0, 0x10 }, }
          })
Wang, Dongsheng Nov. 10, 2018, 9:10 a.m. UTC | #3 
Hi Andrew,

On 2018/11/9 7:23, Andrew Lunn wrote:
> On Thu, Nov 08, 2018 at 03:21:29PM +0800, Wang Dongsheng wrote:
>> Originally I just push "phy-handle" support for ACPI on the QCOM QDF2400
>> platform. After some discussion and following Andrew's advice, I send
>> out with a generic version of ACPI.
>>
>> Current there is no clear documentation about MDIO/PHY for ACPI, so when
>> I reading some documents about ACPI [1], I think we just need to reuse the
>> DT binding in the ACPI.[2]. However, this series of patches are not
>> fully compatible with all contents specified in DT binding.
>>
>> The most important thing about this iseries is link the phy device and
>> fwnode of acpi. Besides, we need to carry out bus scan at the mdio
>> register. Therefore, I am not compatible with more DT binding properties
>> in this series of patches. More support will be in the follow-up patches
>> support, or some people do the support.
>>
>> Example:
>> Based on ACPI doc:
>>     Documentation/acpi/dsd/data-node-references.txt
>>     Documentation/acpi/dsd/graph.txt
>> With _DSD device properties we can finally do this:
>>     Device (MDIO) {
>>         Name (_DSD, Package () {
>>             ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>>             Package () { Package () { "ethernet-phy@0", PHY0 }, }
>>         })
>>         Name (PHY0, Package() {
>>             ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>             Package () { Package () { "reg", 0x0 }, }
>>         })
> I don't know much about ACPI. I do know DT. MDIO busses can have
> multiple PHYs on them. Is the following valid to list two PHYs?
>
>      Device (MDIO) {
>          Name (_DSD, Package () {
>              ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>              Package () { Package () { "ethernet-phy@0", PHY0 }, }
>          })
>          Name (PHY0, Package() {
>              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>              Package () { Package () { "reg", 0x0 }, }
>          })
>          Name (_DSD, Package () {
>              ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>              Package () { Package () { "ethernet-phy@10", PHY1 }, }
>          })
>          Name (PHY1, Package() {
>              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>              Package () { Package () { "reg", 0x10 }, }
>          })
>      }

Multiple PHYs example:


    Device (MDIO)
    {
        Name (_DSD, Package () {
        ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () {

                Package () { "ethernet-phy@0", PHY0 },

                Package () { "ethernet-phy@1", PHY1 },
                ...

                ...

            }
        })
       
        Name (PHY0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x0 },
            }
        })


        Name (PHY1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x1 },
            }
        })
   }

    Device (MAC0)

    {

        // _DSD: Device-Specific Data
        Name (_DSD, Package (0x02) {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
Properties for _DSD */,
            Package () {
                Package () { "phy-handle", Package () { \_SB.MDIO,
"ethernet-phy@0" } },
                ...
                ...
            }
        })

    }


    Device (MAC1)

    {

        // _DSD: Device-Specific Data
        Name (_DSD, Package (0x02) {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
Properties for _DSD */,
            Package () {
                Package () { "phy-handle", Package () { \_SB.MDIO,
"ethernet-phy@1" } },
                ...
                ...
            }
        })

    }

>
> An MDIO bus can also have more than PHYs on them. There can be
> Ethernet switches. Broadcom also have some with generic PHY devices on
> them, and other odd things. That means whatever is on an MDIO bus is a
> device in the Linux device model. How does that work? Do we need some
> form Device (PHY) {}?

ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b") describes a ACPI data node.

The data node can contain property or pointer.


Let's look at the table I'm using:

Device (MAC1)
{
    // _DSD: Device-Specific Data
    Name (_DSD, Package (0x02) {
        ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
Properties for _DSD */,
        Package (0x07) {
            Package () { "phy-handle", Package () { \_SB.MAC1.MDIO,
"ethernet-phy@1" } },
            Package () { "dev-refs", \_SB.MAC0 },
            Package () { "refs0-dev", Package () { \_SB.MAC1.MDIO,
"refs@0" } },
            Package () { "refs1-dev", Package () { \_SB.MAC1.MDIO,
"refs@1" } },
            ...
            ...
        }
    })

    Device (MDIO)
    {
        Name (_DSD, Package () {
        ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () {
                Package () { "ethernet-phy@1", PHY1 },
                Package () { "refs@0",  REF0},
                Package () { "refs@1",  REF1},
            }
        })
        
        //Contain a property
        Name (PHY1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x1 },
            }
        })
        
        //Point to a device
        Name (REF0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "child-refs-dev", \_SB.MAC0 },
            }
        })
        
        //Contain a property and a pointer that point to a device
        Name (REF1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "property", 0x5 },
                Package () { "child-refs-dev", \_SB.MAC0 },
            }
        })
    }
}


>      Device (MDIO) {
>          Device (PHY) {
>              Name (_DSD, Package () {
>                  ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                  Package () { Package () { "ethernet-phy@0", PHY0 }, }
>              })
>              Name (PHY0, Package() {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () { Package () { "reg", 0x0 }, }
>              })
>          }
>          Device (PHY) {
>              Name (_DSD, Package () {
>                  ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                  Package () { Package () { "ethernet-phy@10", PHY1 }, }
>              })
>              Name (PHY1, Package() {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () { Package () { "reg", 0x10 }, }
>              })
>          Device (SWITCH) {
>              Name (_DSD, Package () {
>                  ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                  Package () { Package () { "switch@11", SWITCH0 }, }
>              })
>              Name (SWITCH0, Package() {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () { Package () { "reg", 0x11 }, }
>              })
>          }

    Device (MDIO)
    {
        Name (_DSD, Package () {
        ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () {
                Package () { "ethernet-phy@1", PHY1 },
                Package () { "switch@0",  SW00 },

                Package () { "switch@1",  SW01 },
            }
        })
        
        //Contain a property
        Name (PHY1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x1 },
            }
        })
 
        Name (SW00, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x0 },
            }
        })

        Name (SW01, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "property", 0x5 },
            }
        })
    }


>       }
>
> I'm just trying to ensure whatever is defined is flexible enough that
> we really can later support everything which DT does. We have PHYs on
> MDIO busses, inside switches, which are on MDIO busses, which are
> inside Ethernet interfaces, etc.

I think it can be satisfied. See the table I'm using above.


> An MDIO bus is very similar to an i2c bus. How is that described in
> ACPI? Anything we can learn from that?

About the data node, I just read the Kernel Documentation/acpi/dsd/

And others learn from ACPI spec.


Cheers,

Dongsheng
Wang, Dongsheng Nov. 12, 2018, 2:52 a.m. UTC | #4 
On 2018/11/9 7:23, Andrew Lunn wrote:
> I'm just trying to ensure whatever is defined is flexible enough that
> we really can later support everything which DT does. We have PHYs on
> MDIO busses, inside switches, which are on MDIO busses, which are
> inside Ethernet interfaces, etc.
>
> An MDIO bus is very similar to an i2c bus. How is that described in
> ACPI? Anything we can learn from that?

All the documents I've seen are from UEFII/ACPICA and I have not seen any
independent description of MDIO in the ACPI documents. Maybe I missed a
document, too.

I think not all devices need to be described in ACPI. The ACPI defines what
should be included in a device, as well as some special definitions. Our
MDIO
doesn't need a special definition, because it's enough to use a generic ACPI
description. I think that's why MDIO doesn't have a separate description.

Cheers,
Dongsheng
Andrew Lunn Nov. 12, 2018, 5:20 p.m. UTC | #5 
> > I'm just trying to ensure whatever is defined is flexible enough that
> > we really can later support everything which DT does. We have PHYs on
> > MDIO busses, inside switches, which are on MDIO busses, which are
> > inside Ethernet interfaces, etc.
> 
> I think it can be satisfied. See the table I'm using above.

Hi Dongsheng


Since i don't know anything better, i think i have to trust you have
this correct.

It would be good to document this, so that the next person who needs
to add ACPI support for a PHY has some documentation to look at.
Could you add something to Documentation/acpi/dsd?

      Andrew
Wang, Dongsheng Nov. 13, 2018, 7:36 a.m. UTC | #6 
Hello Rafael,

On 2018/11/13 1:19, Andrew Lunn wrote:
>>> I'm just trying to ensure whatever is defined is flexible enough that
>>> we really can later support everything which DT does. We have PHYs on
>>> MDIO busses, inside switches, which are on MDIO busses, which are
>>> inside Ethernet interfaces, etc.
>> I think it can be satisfied. See the table I'm using above.
> Hi Dongsheng
>
>
> Since i don't know anything better, i think i have to trust you have
> this correct.
>
> It would be good to document this, so that the next person who needs
> to add ACPI support for a PHY has some documentation to look at.
> Could you add something to Documentation/acpi/dsd?

How about Andrew suggestion? I agree with Andrew.

Or We need to add the documentation to Documentation/acpi?
I see that both GPIO and I2C are down here.


Cheers,
Dongsheng