On Wed, Dec 28, 2016 at 05:46:27PM +0100, Thomas Petazzoni wrote:
> +#define MVPP22_SMI_MISC_CFG_REG 0x2a204
> +#define MVPP22_SMI_POLLING_EN BIT(10)
> +
...
> + if (priv->hw_version == MVPP21) {
> + val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
> + val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
> + writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
> + } else {
> + val = readl(priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
> + val &= ~MVPP22_SMI_POLLING_EN;
> + writel(val, priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
> + }
The MVPP22_SMI_MISC_CFG_REG register is within the MDIO driver's
register set, although the mvmdio driver does not access this register.
Shouldn't this be taken care of by the mvmdio driver?
Also, a point that I've noticed while reviewing this is the mvmdio
driver also accesses these registers:
#define MVMDIO_ERR_INT_CAUSE 0x007C
#define MVMDIO_ERR_INT_MASK 0x0080
in addition to the un-named register at offset 0. The driver writes
to these registers unconditionally when unbinding:
writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
However, the various bindings for the driver have:
arch/arm/boot/dts/armada-370-xp.dtsi: compatible = "marvell,orion-mdio";
arch/arm/boot/dts/armada-370-xp.dtsi- reg = <0x72004 0x4>;
arch/arm/boot/dts/armada-375.dtsi: compatible = "marvell,orion-mdio";
arch/arm/boot/dts/armada-375.dtsi- reg = <0xc0054 0x4>;
arch/arm/boot/dts/dove.dtsi: compatible = "marvell,orion-mdio";
arch/arm/boot/dts/dove.dtsi- #address-cells = <1>;
arch/arm/boot/dts/dove.dtsi- #size-cells = <0>;
arch/arm/boot/dts/dove.dtsi- reg = <0x72004 0x84>;
arch/arm/boot/dts/orion5x.dtsi: compatible = "marvell,orion-mdio";
arch/arm/boot/dts/orion5x.dtsi- #address-cells = <1>;
arch/arm/boot/dts/orion5x.dtsi- #size-cells = <0>;
arch/arm/boot/dts/orion5x.dtsi- reg = <0x72004 0x84>;
arch/arm/boot/dts/kirkwood.dtsi: compatible = "marvell,orion-mdio";
arch/arm/boot/dts/kirkwood.dtsi- #address-cells = <1>;
arch/arm/boot/dts/kirkwood.dtsi- #size-cells = <0>;
arch/arm/boot/dts/kirkwood.dtsi- reg = <0x72004 0x84>;
arch/arm/boot/dts/armada-38x.dtsi: compatible = "marvell,orion-mdio";
arch/arm/boot/dts/armada-38x.dtsi- reg = <0x72004 0x4>;
So, for many of these, we're accessing registers outside of the given
binding, which sounds incorrect. I guess that write should be
conditional upon an interrupt being present.
The binding document says:
- reg: address and length of the SMI register
which is clearly wrong for those cases where the interrupt is used.
I also notice that the binding for CP110 uses a register size of 0x10
(even in your tree) - but I guess this should be 4.
I'm starting to wonder whether the orion-mdio driver really is a
separate chunk of hardware that warrants a separate description in
DT from the ethernet controller - it appears in all cases to be
embedded with an ethernet controller, sharing its register space
and at least some of the ethernet controllers clocks. That says
to me that it isn't an independent functional unit of hardware.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
