Re: [PATCH net-next v3 6/6] net: phy: add Applied Micro QT2025 PHY driver

[Andrew Lunn <andrew@lunn.ch>]

> +#[vtable]
> +impl Driver for PhyQT2025 {
> +    const NAME: &'static CStr = c_str!("QT2025 10Gpbs SFP+");
> +    const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x0043A400);
> +
> +    fn probe(dev: &mut phy::Device) -> Result<()> {
> +        // The hardware is configurable?
> +        let hw_id = dev.read(C45::new(Mmd::PMAPMD, 0xd001))?;
> +        if (hw_id >> 8) & 0xff != 0xb3 {
> +            return Ok(());
> +        }

I don't understand this bit of code. At a guess, if the upper bytes of
that register is not 0xb3, the firmware has already been loaded into
the device?

> +
> +        // The 8051 will remain in the reset state.
> +        dev.write(C45::new(Mmd::PMAPMD, 0xC300), 0x0000)?;
> +        // Configure the 8051 clock frequency.
> +        dev.write(C45::new(Mmd::PMAPMD, 0xC302), 0x0004)?;
> +        // Non loopback mode.
> +        dev.write(C45::new(Mmd::PMAPMD, 0xC319), 0x0038)?;
> +        // Global control bit to select between LAN and WAN (WIS) mode.
> +        dev.write(C45::new(Mmd::PMAPMD, 0xC31A), 0x0098)?;
> +        dev.write(C45::new(Mmd::PCS, 0x0026), 0x0E00)?;
> +        dev.write(C45::new(Mmd::PCS, 0x0027), 0x0893)?;
> +        dev.write(C45::new(Mmd::PCS, 0x0028), 0xA528)?;
> +        dev.write(C45::new(Mmd::PCS, 0x0029), 0x0003)?;

802.3 says:

3.38 through 3.4110/25GBASE-R PCS test pattern seed B ????

> +        // Configure transmit and recovered clock.
> +        dev.write(C45::new(Mmd::PMAPMD, 0xC30A), 0x06E1)?;
> +        // The 8051 will finish the reset state.
> +        dev.write(C45::new(Mmd::PMAPMD, 0xC300), 0x0002)?;
> +        // The 8051 will start running from the boot ROM.
> +        dev.write(C45::new(Mmd::PCS, 0xE854), 0x00C0)?;
> +
> +        let fw = Firmware::request(c_str!("qt2025-2.0.3.3.fw"), dev.as_ref())?;
> +        if fw.data().len() > SZ_16K + SZ_8K {
> +            return Err(code::EFBIG);
> +        }
> +
> +        // The 24kB of program memory space is accessible by MDIO.
> +        // The first 16kB of memory is located in the address range 3.8000h - 3.BFFFh.
> +        // The next 8kB of memory is located at 4.8000h - 4.9FFFh.
> +        let mut j = SZ_32K;
> +        for (i, val) in fw.data().iter().enumerate() {
> +            if i == SZ_16K {
> +                j = SZ_32K;
> +            }
> +
> +            let mmd = if i < SZ_16K { Mmd::PCS } else { Mmd::PHYXS };
> +            dev.write(
> +                C45::new(mmd, j as u16),
> +                <u8 as Into<u16>>::into(*val).to_le(),

This is well past my level of Rust. I assume fw.data is a collection
of bytes, and you enumerate it as bytes. A byte has no endiannes, so
why do you need to convert it to little endian?

	Andrew