linux-kernel - Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family

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]

Message-ID: <5c028659-db7e-9a98-7ec1-1e2d19d135f9@phrozen.org>
Date:   Tue, 13 Sep 2016 11:40:43 +0200
From:   John Crispin <john@...ozen.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        qsdk-review@....qualcomm.com
Subject: Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family

On 13/09/2016 03:23, Andrew Lunn wrote:
> So lets see if i have this right.
> 
> Port 0 has no internal phy.
> Port 1 has an internal PHY at MDIO address 0.
> Port 2 has an internal PHY at MDIO address 1.
> ...
> Port 5 has an internal PHY ad MDIO address 4.
> Port 6 has no internal PHY.

Hi Andrew

correct. port 0 is the cpu port. I initially thought that port6 can also
be used as te cpu port but there are various places in the datasheet
stating that the cpu port is 0. in some of the reference designs, port6
is wired to a 2nd gmac of the cpu and in those cases port 6 is then
hardwired to port 5 of the switch and called wan. right now the driver
does not support this feature. i have changed the code to always assume
that port is the cpu port and will send a patch later to allow the
port5/6 wan port setup once the series got accepted.

> 
> This is why you have funky port numbers, and phy_to_port.

this is legacy code from the series Matthieu posted. i agree though that
its a bit dirty. Sergey already told me that the devicetree is also bad
because of this as the unit address of the device tree node and reg
property are not aligned.

> 
> I think it would be a lot cleaner to handle this in qca8k_phy_read()
> and qca8k_phy_write(). 

ok, i will simply substract 1 from the phy_addr inside the mdio
callbacks. this would make the code more readable and make the DT
binding compliant with the ePAPR spec.

> 
> Also, the comment it a bit misleading. You are probing the PHY ID, not
> the switch ID. At least for the Marvell switches, different switches
> can have the same embedded PHY. It would be annoying to find there is
> another incompatible switch with the same PHY ID.

there is only an 8bit field inside the MASK_CTRL register (0x000) which
is 0x13. I've sent an email to QCA asking if this a unique identifier.

> Is the embedded PHY compatible with the at803x driver?

I've sent an email to QCA asking about this

	John