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] [day] [month] [year] [list]
Date:   Mon, 30 Jan 2017 10:29:23 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Rafał Miłecki <zajec5@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jon Mason <jon.mason@...adcom.com>,
        Felix Fietkau <nbd@...nwrt.org>,
        Network Development <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 11:02 PM, Rafał Miłecki wrote:
> On 29 January 2017 at 23:36, Florian Fainelli <f.fainelli@...il.com> wrote:
>> Le 01/29/17 à 13:31, Rafał Miłecki a écrit :
>>> On 29 January 2017 at 21:22, Florian Fainelli <f.fainelli@...il.com> wrote:
>>>> On 01/29/2017 12:14 PM, Rafał Miłecki wrote:
>>>>> 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.
>>>>
>>>> Humm, built-in PHYs would typically appear as 0-5 on the MDIO bus,
>>>> whereas external PHYs would have a different (and non conflicting)
>>>> address, also, there are restrictions on the Roboswitch devices as to
>>>> where you could wire these external PHYs (to port 5, or 7, 8).
>>>
>>> From BCM47186B0:
>>> [    0.942419] bgmac_bcma bcma0:2: Found PHY addr: 25
>>>
>>> From BCM47189:
>>> [    1.758079] bgmac_bcma bcma0:5: Found PHY addr: 0
>>
>> Address 0 is usually a broadcast address, although with most Broadcom
>> switches, it just maps to the first port's built-in PHY.
>>
>>>
>>> I got one more AP device but I bricked it by corrupting NVRAM
>>> (bootloader doesn't start due to that).
>>>
>>>
>>> My BCM47186B0 seems to not have any switch. It isn't that clear in
>>> BCM47189 case. You may be right, some sources say BCM47189 has
>>> built-in switch so maybe it's indeed BCM54210E connected to switch's
>>> port. On the other hand I'm not using any switch driver on my BCM47189
>>> AP board, so how does it work? Just an accidentally working setup left
>>> by the bootloader?
>>
>> That's what I suspect is happening yes. Do you have the different FCC
>> IDs of these routers so maybe looking at pictures of the inside would
>> tell us what kind of internal vs. external PHYs are populated?
> 
> I don't. I read all chipset IDs I could see:
> 1) 2 Winbond RAM chips
> 2) MXIC MX25L25635F flash
> 3) BCM47189 SoC
> 4) B50212E PHY
> 5) BCM43217 WiFi
> 
> There clearly isn't external switch (like BCM43125 or so) but we can't
> say (even from the photos) where that B50212E is connected to. It's
> most likely built-in switch's port as you said.

Yes, that's what I suspect, or, there is a second GMAC available to
which we can directly connect this 50212E external PHY. I think Jon has
a 4749 that could be used to confirm/infirm what PHY IDs we read on that
device.
-- 
Florian