Re: [PATCH v2] ARM: imx: allow to disable board specific PHY fixups

[Oleksij Rempel]

On Tue, Mar 31, 2020 at 09:19:18AM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Mar 31, 2020 at 10:00:12AM +0200, Marc Kleine-Budde wrote:
> > On 3/31/20 9:54 AM, Russell King - ARM Linux admin wrote:
> > > On Tue, Mar 31, 2020 at 09:47:19AM +0200, Marc Kleine-Budde wrote:
> > >> On 3/30/20 7:41 PM, Russell King - ARM Linux admin wrote:
> > >>>>> arch/arm/mach-imx/mach-imx6q.c:167:		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
> > >>>>> arch/arm/mach-imx/mach-imx6q.c:169:		phy_register_fixup_for_uid(PHY_ID_KSZ9031, MICREL_PHY_ID_MASK,
> > >>>>> arch/arm/mach-imx/mach-imx6q.c:171:		phy_register_fixup_for_uid(PHY_ID_AR8031, 0xffffffef,
> > >>>>> arch/arm/mach-imx/mach-imx6q.c:173:		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
> > >>>
> > >>> As far as I'm concerned, the AR8035 fixup is there with good reason.
> > >>> It's not just "random" but is required to make the AR8035 usable with
> > >>> the iMX6 SoCs.  Not because of a board level thing, but because it's
> > >>> required for the AR8035 to be usable with an iMX6 SoC.
> > >>
> > >> Is this still ture, if the AR8035 is attached to a switch behind an iMX6?
> > > 
> > > Do you know of such a setup, or are you talking about theoretical
> > > situations?
> > 
> > Granted, not for the AR8035, but for one of the KSZ Phys. This is why
> > Oleksij started looking into this issue in the first place.
> 
> Maybe there's an easy solution to this - check whether the PHY being
> fixed up is connected to the iMX6 SoC:
> 
> static bool phy_connected_to(struct phy_device *phydev,
> 			     const struct of_device_id *matches)
> {
> 	struct device_node *np, *phy_np;
> 
> 	for_each_matching_node(np, matches) {
> 		phy_np = of_parse_phandle(np, "phy-handle", 0);
> 		if (!phy_np)
> 			phy_np = of_parse_phandle(np, "phy", 0);
> 		if (!phy_np)
> 			phy_np = of_parse_phandle(np, "phy-device", 0);
> 		if (phy_np && phydev->mdio.dev.of_node == phy_np) {
> 			of_node_put(phy_np);
> 			of_node_put(np);
> 			return true;
> 		}
> 		of_node_put(phy_np);
> 	}
> 	return false;
> }
> 
> static struct of_device_id imx_fec_ids[] = {
> 	{ .compatible = "fsl,imx6q-fec", },
> 	...
> 	{ },
> };
> 
> static bool phy_connected_to_fec(struct phy_device *phydev)
> {
> 	return phy_connected_to(phydev, imx_fec_ids);
> }
> 
> and then in the fixups:
> 
> 	if (!phy_connected_to_fec(phydev))
> 		return 0;
> 

Ok, i see. We will limit fixup impact to some specific devicetree nodes.
And if we wont to disable fixup completely, some special devicetree binding will
be needed. Correct? Is this acceptable mainline way?
For the usb ethernet fixups we will need some thing similar.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |