netdev - Re: [PATCH V2 3/3] net: bgmac: use PHY subsystem for initializing 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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Sun, 29 Jan 2017 21:14:25 +0100
From:   Rafał Miłecki <zajec5@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     Jon Mason <jon.mason@...adcom.com>,
        Felix Fietkau <nbd@...nwrt.org>, netdev@...r.kernel.org,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH V2 3/3] net: bgmac: use PHY subsystem for initializing PHY

On 01/29/2017 04:08 AM, Florian Fainelli wrote:
> On 01/28/2017 01:08 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@...ecki.pl>
>>
>> This adds support for using bgmac with PHYs supported by standalone PHY
>> drivers. Having any PHY initialization in bgmac is hacky and shouldn't
>> be extended but rather removed if anyone has hardware to test it.
>>
>> Signed-off-by: Rafał Miłecki <rafal@...ecki.pl>
>> ---
>>  drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
>> index 9d9984999dce..6ce80cbcb48e 100644
>> --- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
>> +++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
>> @@ -132,6 +132,10 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
>>  	struct bcma_chipinfo *ci = &bgmac->bcma.core->bus->chipinfo;
>>  	u8 i;
>>
>> +	/* For some legacy hardware we do chipset-based PHY initialization here
>> +	 * without even detecting PHY ID. It's hacky and should be cleaned as
>> +	 * soon as someone can test it.
>> +	 */
>>  	if (ci->id == BCMA_CHIP_ID_BCM5356) {
>>  		for (i = 0; i < 5; i++) {
>>  			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x008b);
>> @@ -140,6 +144,7 @@ static void bcma_mdio_phy_init(struct bgmac *bgmac)
>>  			bcma_mdio_phy_write(bgmac, i, 0x12, 0x2aaa);
>>  			bcma_mdio_phy_write(bgmac, i, 0x1f, 0x000b);
>>  		}
>> +		return;
>
> That part is clearly initializing the built-in Ethernet switch's PHYs,
> and so the natural place for that would be to stick these init values
> into the Broadcom PHY driver. When b53-srab/b53_common attaches the
> switch, it will scan all of these port's builtin PHYs and bind to an
> appropriate PHY driver which could have this initialization as part of
> the config_init routine for instance. Right now, we are most likely
> using the Generic PHY.

I don't think this code is for switch's PHYs. I believe this code is for
wireless access points that have no switch and have Ethernet interface connected
directly to some single-port PHY. I saw 2 or 3 devices like this. They often
also use PoE.


> Here are the different PHY IDs you should read from these models if you
> want to make a subsequent patch that moves this initialization down to
> the Broadcom PHY driver:
>
> 5356: 0x03625DA0
> 5357/53572: 0x03625F00
> 4749: could either be 0x600D85F0 or the same as 53010 (0x600D8760),
> unclear where that product came from... Jon, would you know by chance?

This is very valuable info, thank you! I'll definitely work on this. So far I
tried using bgmac.ko + broadcom.ko on AP device with BCM47186B0 but it doesn't
work for some reason, I'll work on this & keep moving PHY code out of bgmac.