On Mon, Feb 17, 2025 at 05:36:40PM -0800, Kyle Hendry wrote: > This patch adds support for the internal gigabit PHY on the Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > BCM63268 SoC. The PHY has a low power mode that has can be > enabled/disabled through the GPHY control register. The > register is passed in through the device tree, and the > relevant bits are set in the suspend and resume functions. > ... > +int bcm63268_gphy_resume(struct phy_device *phydev) > +{ > + int ret; > + > + ret = bcm63268_gphy_set(phydev, true); > + if (ret) > + return ret; > + > + ret = genphy_resume(phydev); > + if (ret) > + return ret; > + > + return 0; > +} > + > +int bcm63268_gphy_suspend(struct phy_device *phydev) Why these are not static? Where is EXPORT_SYMBOL and kerneldoc? > +{ > + int ret; > + > + ret = genphy_suspend(phydev); > + if (ret) > + return ret; > + > + ret = bcm63268_gphy_set(phydev, false); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int bcm63268_gphy_probe(struct phy_device *phydev) > +{ > + struct device_node *np = dev_of_node(&phydev->mdio.bus->dev); > + struct mdio_device *mdio = &phydev->mdio; > + struct device *dev = &mdio->dev; > + struct bcm_gphy_priv *priv; > + struct regmap *regmap; > + int err; > + > + err = devm_phy_package_join(dev, phydev, 0, 0); > + if (err) > + return err; > + > + priv = devm_kzalloc(dev, sizeof(struct bcm_gphy_priv), GFP_KERNEL); sizeof(*) > + if (!priv) > + return -ENOMEM; > + > + phydev->priv = priv; > + > + regmap = syscon_regmap_lookup_by_phandle(np, "brcm,gphy-ctrl"); No. ABI break without any explanation in commit msg. Best regards, Krzysztof
