C45 support and mdiobus_scan

C45 support and mdiobus_scan

[Jose Abreu]

Hi All,

I'm preparing to add support for 10G in stmmac and I noticed that
Generic 10G PHY needs C45 support. Digging through the
registration callbacks for phy that are used in stmmac I reached
to mdiobus_scan() and the following call:

phydev = get_phy_device(bus, addr, false);

The last parameter is "is_c45", and is always being set to false ...

Does this mean that I can't use the Generic 10G PHY in stmmac? I
don't mind link being fixed for 10G for now.

(Notice I'm using a PCI based setup so no DT bindings can help me
for this).

Thanks and Best Regards,
Jose Miguel Abreu

Re: C45 support and mdiobus_scan

[Andrew Lunn]

On Thu, Aug 09, 2018 at 02:54:11PM +0100, Jose Abreu wrote:
> Hi All,
> 
> I'm preparing to add support for 10G in stmmac and I noticed that
> Generic 10G PHY needs C45 support. Digging through the
> registration callbacks for phy that are used in stmmac I reached
> to mdiobus_scan() and the following call:
> 
> phydev = get_phy_device(bus, addr, false);
> 
> The last parameter is "is_c45", and is always being set to false ...
> 
> Does this mean that I can't use the Generic 10G PHY in stmmac? I
> don't mind link being fixed for 10G for now.

Hi Jose

So far, all MACs which support 10G have used phy-handle to point to a
PHY on am MDIO bus, and that PHY uses .compatible =
"ethernet-phy-ieee802.3-c45". of_mdiobus_register() will then find the
PHY and register it. You really should try to follow this, if you can.

> (Notice I'm using a PCI based setup so no DT bindings can help me
> for this).

That is not necessarily true. Take a look at:

arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi

&pcie {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_pcie>;
        reset-gpio = <&gpio7 12 GPIO_ACTIVE_LOW>;
        status = "okay";

        host@0 {
                reg = <0 0 0 0 0>;

                #address-cells = <3>;
                #size-cells = <2>;

                i210: i210@0 {
                        reg = <0 0 0 0 0>;
                };
        };
};

The PCIe core will look in the device tree and when it creates the
platform device for the i210 on the pcie bus, it points
pdev->dev.of_node at this node. So long as you are using a platform
with DT, you can do this. I hope you are not using x86..

     Andrew

Re: C45 support and mdiobus_scan

[Jose Abreu]

Hi Andrew,

Thanks for your answer :)

On 09-08-2018 16:03, Andrew Lunn wrote:
> On Thu, Aug 09, 2018 at 02:54:11PM +0100, Jose Abreu wrote:
>> Hi All,
>>
>> I'm preparing to add support for 10G in stmmac and I noticed that
>> Generic 10G PHY needs C45 support. Digging through the
>> registration callbacks for phy that are used in stmmac I reached
>> to mdiobus_scan() and the following call:
>>
>> phydev = get_phy_device(bus, addr, false);
>>
>> The last parameter is "is_c45", and is always being set to false ...
>>
>> Does this mean that I can't use the Generic 10G PHY in stmmac? I
>> don't mind link being fixed for 10G for now.
> Hi Jose
>
> So far, all MACs which support 10G have used phy-handle to point to a
> PHY on am MDIO bus, and that PHY uses .compatible =
> "ethernet-phy-ieee802.3-c45". of_mdiobus_register() will then find the
> PHY and register it. You really should try to follow this, if you can.
>
>> (Notice I'm using a PCI based setup so no DT bindings can help me
>> for this).
> That is not necessarily true. Take a look at:
>
> arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
>
> &pcie {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_pcie>;
>         reset-gpio = <&gpio7 12 GPIO_ACTIVE_LOW>;
>         status = "okay";
>
>         host@0 {
>                 reg = <0 0 0 0 0>;
>
>                 #address-cells = <3>;
>                 #size-cells = <2>;
>
>                 i210: i210@0 {
>                         reg = <0 0 0 0 0>;
>                 };
>         };
> };
>
> The PCIe core will look in the device tree and when it creates the
> platform device for the i210 on the pcie bus, it points
> pdev->dev.of_node at this node. So long as you are using a platform
> with DT, you can do this. I hope you are not using x86..

Yes I am :( Any possible solution for this?

I guess in ultimate case I will have to switch to ARM based setup.

Thanks and Best Regards,
Jose Miguel Abreu

>
>      Andrew

Re: C45 support and mdiobus_scan

[Andrew Lunn]

> > The PCIe core will look in the device tree and when it creates the
> > platform device for the i210 on the pcie bus, it points
> > pdev->dev.of_node at this node. So long as you are using a platform
> > with DT, you can do this. I hope you are not using x86..
> 
> Yes I am :( Any possible solution for this?

Well, DT can be used with x86. I think Edison did that. But i assume
your PCIe host is in ACPI, not DT. So getting this linking working
will not be easy.

There has been some work to add an ACPI binding for PHYs. I don't know
if it actually got far enough that you can hack your DSDT to add a
PHY. But i'm sure it did not get far enough that you can describe an
MDIO bus in DSDT, so it probably is not going to help you.

> I guess in ultimate case I will have to switch to ARM based setup.

Yes, or MIPS.

     Andrew

Re: C45 support and mdiobus_scan

[Tom Lendacky]

On 8/9/2018 10:25 AM, Andrew Lunn wrote:
>>> The PCIe core will look in the device tree and when it creates the
>>> platform device for the i210 on the pcie bus, it points
>>> pdev->dev.of_node at this node. So long as you are using a platform
>>> with DT, you can do this. I hope you are not using x86..
>>
>> Yes I am :( Any possible solution for this?

I haven't looked too closely, but maybe you can add a new mdiobus_scan
function for 10G that attempts get_phy_device() with is_c45 set to true
and if nothing is found falls back to get_phy_device() with is_c45 set to
false.  I don't know what would happen if you have a non-c45 phy attached,
but it's worth a shot to try it and see for each situation.

Thanks,
Tom

> 
> Well, DT can be used with x86. I think Edison did that. But i assume
> your PCIe host is in ACPI, not DT. So getting this linking working
> will not be easy.
> 
> There has been some work to add an ACPI binding for PHYs. I don't know
> if it actually got far enough that you can hack your DSDT to add a
> PHY. But i'm sure it did not get far enough that you can describe an
> MDIO bus in DSDT, so it probably is not going to help you.
> 
>> I guess in ultimate case I will have to switch to ARM based setup.
> 
> Yes, or MIPS.
> 
>      Andrew
> 

Re: C45 support and mdiobus_scan

[Andrew Lunn]

On Fri, Aug 10, 2018 at 10:20:56AM -0500, Tom Lendacky wrote:
> On 8/9/2018 10:25 AM, Andrew Lunn wrote:
> >>> The PCIe core will look in the device tree and when it creates the
> >>> platform device for the i210 on the pcie bus, it points
> >>> pdev->dev.of_node at this node. So long as you are using a platform
> >>> with DT, you can do this. I hope you are not using x86..
> >>
> >> Yes I am :( Any possible solution for this?
> 
> I haven't looked too closely, but maybe you can add a new mdiobus_scan
> function for 10G that attempts get_phy_device() with is_c45 set to true
> and if nothing is found falls back to get_phy_device() with is_c45 set to
> false.

Hi Tom

I did consider at one point adding extra flags to the struct mii_bus
to indicate if the bus master supports C22 and or C45, and then scan
the bus as appropriate. We cannot unconditionally do a C45 scan on all
busses, because most bus drivers don't look for MII_ADDR_C45, and so
are wrongly going to do a C22 transaction. There is also one bus
driver i know of which can only do C45. But it at least returns
EOPNOTSUPP if you ask it to do a C22.

I think this needs addressing at some point. We are seeing more 2.5G,
5G and 10G MAC/PHY combinations, and they often need C45. So maybe
adding a flag saying C45 is supported, and then scanning is a good way
forward. Adding a flag saying C22 is supported might be too much work,
without enough return.

> I don't know what would happen if you have a non-c45 phy attached,
> but it's worth a shot to try it and see for each situation.

That should be fine. A C22 PHY should ignore a C45 transaction. At
least that was the design idea when C45 was introduced. But i would
not be too surprised if we find the odd C22 phy get confused, and we
need to add some sort of quirks.

     Andrew