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

[Hau]

> 
> > > +#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.

I will try to use these functions.

> > > +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

Reading register 2 and 3, it is for checking hardware use which pin(eeprom or gpo) to connect to RTL8211FS.

> ------Please consider the environment before printing this e-mail.