RE: [PATCH v3 net-next] r8169: add support for rtl8168h(revid 0x2a) + rtl8211fs fiber application

[Hau <hau@realtek.com>]

> > +#define MDIO_READ 2
> > +#define MDIO_WRITE 1
> > +/* MDIO bus init function */
> > +static int rtl_mdio_bitbang_init(struct rtl8169_private *tp) {
> > +	struct bb_info *bitbang;
> > +	struct device *d = tp_to_dev(tp);
> > +	struct mii_bus *new_bus;
> > +
> > +	/* create bit control struct for PHY */
> > +	bitbang = devm_kzalloc(d, sizeof(struct bb_info), GFP_KERNEL);
> > +	if (!bitbang)
> > +		return -ENOMEM;
> > +
> > +	/* bitbang init */
> > +	bitbang->tp = tp;
> > +	bitbang->ctrl.ops = &bb_ops;
> > +	bitbang->ctrl.op_c22_read = MDIO_READ;
> > +	bitbang->ctrl.op_c22_write = MDIO_WRITE;
> > +
> > +	/* MII controller setting */
> > +	new_bus = devm_mdiobus_alloc(d);
> > +	if (!new_bus)
> > +		return -ENOMEM;
> > +
> > +	new_bus->read = mdiobb_read;
> > +	new_bus->write = mdiobb_write;
> > +	new_bus->priv = &bitbang->ctrl;
> > +
> 
> This looks like an open-coded version of alloc_mdio_bitbang().
> 
Yes, it is part of alloc_mdio_bitbang().

> > +	tp->mii_bus = new_bus;
> > +
> > +	return 0;
> > +}
> > +
> > +static void rtl_sfp_bitbang_init(struct rtl8169_private *tp,
> > +				  struct rtl_sfp_if_mask *sfp_mask) {
> > +	struct mii_bus *bus = tp->mii_bus;
> > +	struct bb_info *bitbang = container_of(bus->priv, struct bb_info,
> > +ctrl);
> > +
> > +	r8168_mac_ocp_modify(tp, PINPU, sfp_mask->pin_mask, 0);
> > +	r8168_mac_ocp_modify(tp, PINOE, 0, sfp_mask->pin_mask);
> > +	bitbang->pinoe_value = r8168_mac_ocp_read(tp, PINOE);
> > +	bitbang->pin_i_sel_1_value = r8168_mac_ocp_read(tp,
> PIN_I_SEL_1);
> > +	bitbang->pin_i_sel_2_value = r8168_mac_ocp_read(tp,
> PIN_I_SEL_2);
> > +	memcpy(&bitbang->sfp_mask, sfp_mask, sizeof(struct
> > +rtl_sfp_if_mask)); }
> > +
> > +static void rtl_sfp_mdio_write(struct rtl8169_private *tp,
> > +				  u8 reg,
> > +				  u16 val)
> > +{
> > +	struct mii_bus *bus = tp->mii_bus;
> > +	struct bb_info *bitbang;
> > +
> > +	if (!bus)
> > +		return;
> > +
> > +	bitbang = container_of(bus->priv, struct bb_info, ctrl);
> > +	bus->write(bus, bitbang->sfp_mask.phy_addr, reg, val); }
> > +
> > +static u16 rtl_sfp_mdio_read(struct rtl8169_private *tp,
> > +				  u8 reg)
> > +{
> > +	struct mii_bus *bus = tp->mii_bus;
> > +	struct bb_info *bitbang;
> > +
> > +	if (!bus)
> > +		return ~0;
> > +
> > +	bitbang = container_of(bus->priv, struct bb_info, ctrl);
> > +
> > +	return bus->read(bus, bitbang->sfp_mask.phy_addr, reg); }
> > +
> > +static void rtl_sfp_mdio_modify(struct rtl8169_private *tp, u32 reg, u16
> mask,
> > +				 u16 set)
> > +{
> > +	u16 data = rtl_sfp_mdio_read(tp, reg);
> > +
> > +	rtl_sfp_mdio_write(tp, reg, (data & ~mask) | set); }
> > +
> > +#define RTL8211FS_PHY_ID_1 0x001c
> > +#define RTL8211FS_PHY_ID_2 0xc916
> > +
> 
> There shouldn't be a dependency on a specific PHY type. It may not reflect
> your use cases, but it should be perfectly possible to combine this MAC with
> other PHY types, also from other vendors, supporting fiber.
> 
This is a special use case. rtl8168h is not designed as MAC to connect to other PHY.

Thanks,
Hau
 ------Please consider the environment before printing this e-mail.