netdev - Re: [PATCH net-next] net: ks8851: Connect and start/stop the internal PHY

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 for Android: free password hash cracker in your pocket

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

Message-ID: <d20d2e2e-9745-4f75-3179-f68e1efcf29b@gmail.com>
Date:   Mon, 11 Jan 2021 15:43:16 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Marek Vasut <marex@...x.de>, netdev@...r.kernel.org
Cc:     Andrew Lunn <andrew@...n.ch>, Lukas Wunner <lukas@...ner.de>
Subject: Re: [PATCH net-next] net: ks8851: Connect and start/stop the internal
 PHY

On 11.01.2021 15:10, Marek Vasut wrote:
> On 1/11/21 2:50 PM, Heiner Kallweit wrote:
>> On 11.01.2021 14:38, Marek Vasut wrote:
>>> On 1/11/21 2:26 PM, Heiner Kallweit wrote:
>>> [...]
>>>
>>>> LGTM. When having a brief look at the driver I stumbled across two things:
>>>>
>>>> 1. Do MAC/PHY support any pause mode? Then a call to
>>>>      phy_support_(a)sym_pause() would be missing.
>>>
>>> https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8851-16MLL-Single-Port-Ethernet-MAC-Controller-with-8-Bit-or-16-Bit-Non-PCI-Interface-DS00002357B.pdf
>>> page 64
>>>
>>> https://www.mouser.com/datasheet/2/268/ksz8851-16mll_ds-776208.pdf
>>> page 65
>>>
>>> The later is more complete.
>>>
>>> Apparently it does support pause.
> 
> Based on the datasheet, does it support sym or asym pause ?
> 

According to the description of flow control on p.23 it can support asym pause.
However on the MAC side flow control doesn't seem to be always active, it's
controlled by these two bits:

p.49, TXCR, bit 3
p.50, RXCR1, bit 10

Default seems to be that flow control is disabled.

>>>> 2. Don't have the datasheet, but IRQ_LCI seems to be the link change
>>>>      interrupt. So far it's ignored by the driver. You could configure
>>>>      it and use phy_mac_interrupt() to operate the internal PHY in
>>>>      interrupt mode.
>>>
>>> That's only for link state change, shouldn't the PHY interrupt trigger on other things as well ?
>>
>> No, it's sufficient if the interrupt can signal link state change.
>> In r8169 I have exactly that case.
> 
> I'll do that in a subsequent patch, once I verify it works as it should.