> +#define DRV_VERSION 1.19
> +static int mlxbf_gige_probe(struct platform_device *pdev)
> +{
> + unsigned int phy_int_gpio;
> + struct phy_device *phydev;
> + struct net_device *netdev;
> + struct resource *mac_res;
> + struct resource *llu_res;
> + struct resource *plu_res;
> + struct mlxbf_gige *priv;
> + void __iomem *llu_base;
> + void __iomem *plu_base;
> + void __iomem *base;
> + int addr, version;
> + u64 control;
> + int err = 0;
> +
> + if (device_property_read_u32(&pdev->dev, "version", &version)) {
> + dev_err(&pdev->dev, "Version Info not found\n");
> + return -EINVAL;
> + }
Is this a device tree property? ACPI? If it is device tree property
you need to document the binding, Documentation/devicetree/bindinds/...
> +
> + if (version != (int)DRV_VERSION) {
> + dev_err(&pdev->dev, "Version Mismatch. Expected %d Returned
> %d\n",
> + (int)DRV_VERSION, version);
> + return -EINVAL;
> + }
That is odd. Doubt odd. First of, why (int)1.19? Why not just set
DRV_VERSION to 1? This is the only place you use this, so the .19
seems pointless. Secondly, what does this version in DT/ACPI actually
represent? The hardware version? Then you should be using a compatible
string? Or read a hardware register which tells you have hardware
version.
> +
> + err = device_property_read_u32(&pdev->dev, "phy-int-gpio",
> &phy_int_gpio);
> + if (err < 0)
> + phy_int_gpio = MLXBF_GIGE_DEFAULT_PHY_INT_GPIO;
Again, this probably needs documenting. This is not how you do
interrupts with DT. I also don't think it is correct for ACPI, but i
don't know ACPI.
> + phydev = phy_find_first(priv->mdiobus);
> + if (!phydev) {
> + mlxbf_gige_mdio_remove(priv);
> + return -ENODEV;
> + }
If you are using DT, please use a phandle to the device on the MDIO
bus.
> + /* Sets netdev->phydev to phydev; which will eventually
> + * be used in ioctl calls.
> + * Cannot pass NULL handler.
> + */
> + err = phy_connect_direct(netdev, phydev,
> + mlxbf_gige_adjust_link,
> + PHY_INTERFACE_MODE_GMII);
It does a lot more than just set netdev->phydev. I'm not sure this
comment has any real value.
Andrew
