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

[Andrew Lunn]

> > +#define RT_SFP_ST (1)
> > +#define RT_SFP_OP_W (1)
> > +#define RT_SFP_OP_R (2)
> > +#define RT_SFP_TA_W (2)
> > +#define RT_SFP_TA_R (0)
> > +
> > +static void rtl_sfp_if_write(struct rtl8169_private *tp,
> > +				  struct rtl_sfp_if_mask *sfp_if_mask, u8 reg, u16 val)
> > +{
> > +	struct rtl_sfp_if_info sfp_if_info = {0};
> > +	const u16 mdc_reg = PIN_I_SEL_1;
> > +	const u16 mdio_reg = PIN_I_SEL_2;
> > +
> > +	rtl_select_sfp_if(tp, sfp_if_mask, &sfp_if_info);
> > +
> > +	/* change to output mode */
> > +	r8168_mac_ocp_write(tp, PINOE, sfp_if_info.mdio_oe_o);
> > +
> > +	/* init sfp interface */
> > +	r8168_mac_ocp_write(tp, mdc_reg, sfp_if_info.mdc_pd);
> > +	r8168_mac_ocp_write(tp, mdio_reg, sfp_if_info.mdio_pu);
> > +
> > +	/* preamble 32bit of 1 */
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, 0xffffffff, 32);
> > +
> > +	/* opcode write */
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, RT_SFP_ST, 2);
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, RT_SFP_OP_W, 2);
> > +
> > +	/* phy address */
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, sfp_if_mask->phy_addr, 5);
> > +
> > +	/* phy reg */
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, reg, 5);
> > +
> > +	/* turn-around(TA) */
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, RT_SFP_TA_W, 2);
> > +
> > +	/* write phy data */
> > +	rtl_sfp_shift_bit_in(tp, &sfp_if_info, val, 16);

This looks like a bit-banging MDIO bus? If so, please use the kernel
code, drivers/net/mdio/mdio-bitbang.c. You just need to provide it
with functions to write and read a bit, and it will do the rest,
including C45 which you don't seem to support here.

> > +static enum rtl_sfp_if_type rtl8168h_check_sfp(struct rtl8169_private *tp)
> > +{
> > +	int i;
> > +	int const checkcnt = 4;
> > +
> > +	rtl_sfp_eeprom_write(tp, 0x1f, 0x0000);
> > +	for (i = 0; i < checkcnt; i++) {
> > +		if (rtl_sfp_eeprom_read(tp, 0x02) != RTL8211FS_PHY_ID_1 ||
> > +			rtl_sfp_eeprom_read(tp, 0x03) != RTL8211FS_PHY_ID_2)
> > +			break;
> > +	}

Reading registers 2 and 3 for a PhY idea? Who not just use phylib, and
a PHY driver?

  Andrew