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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20240817.045010.1342766151118215404.fujita.tomonori@gmail.com>
Date: Sat, 17 Aug 2024 04:50:10 +0000 (UTC)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: andrew@...n.ch
Cc: fujita.tomonori@...il.com, netdev@...r.kernel.org,
 rust-for-linux@...r.kernel.org, tmgross@...ch.edu,
 miguel.ojeda.sandonis@...il.com, benno.lossin@...ton.me,
 aliceryhl@...gle.com
Subject: Re: [PATCH net-next v3 6/6] net: phy: add Applied Micro QT2025 PHY
 driver

On Fri, 16 Aug 2024 22:47:04 +0200
Andrew Lunn <andrew@...n.ch> wrote:

>> >> +    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?
>> 
>> I've just added debug code and found that the upper bytes of the
>> register is 0xb3 even after loading the firmware.
>> 
>> I checked the original code again and found that if the bytes isn't
>> 0xb3, the driver initialization fails. I guess that the probe should
>> return an error here (ENODEV?). 
> 
> O.K. Maybe add a comment that the vendor driver does this, but we have
> no idea why.

Yes, added the comment.

>> >> +        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 ????
>> 
>> Yeah, strange. But I can't find any hints on them in the datasheet or
>> the original code.
> 
> Seems unlikely it is seeding anything. So i guess they have reused the
> registers for something else. This is actually a bit odd, because all
> the other registers are in the ranges reserved for vendor usage.
> 
> If these were legitimate register usage, i would of suggested adding
> #define to include/uapi/linux/mdio.h for them.

I see.