netdev - Re: [PATCH net-next v4 2/2] net:dsa:microchip: add property to select

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: <20231024142426.GE3803936@pengutronix.de>
Date: Tue, 24 Oct 2023 16:24:26 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Ante Knezic <ante.knezic@...mholz.de>
Cc: UNGLinuxDriver@...rochip.com, andrew@...n.ch, conor+dt@...nel.org,
	davem@...emloft.net, devicetree@...r.kernel.org,
	edumazet@...gle.com, f.fainelli@...il.com,
	krzysztof.kozlowski+dt@...aro.org, kuba@...nel.org,
	linux-kernel@...r.kernel.org, marex@...x.de, netdev@...r.kernel.org,
	olteanv@...il.com, pabeni@...hat.com, robh+dt@...nel.org,
	woojung.huh@...rochip.com
Subject: Re: [PATCH net-next v4 2/2] net:dsa:microchip: add property to select

On Tue, Oct 24, 2023 at 03:08:32PM +0200, Ante Knezic wrote:
> On Tue, 24 Oct 2023 12:09:15 +0200, Oleksij Rampel wrote:
> 
> > > As you suggested, it looks like KSZ9897 clocking mode depends on RMII interface
> > > mode (with strapping pins), but I don't see this for KSZ8863. The PHY/MAC mode
> > > is selected with Register 0x35 bit 7 and the clocking mode is selected via
> > > strapping pins EN_REFCLKO and SMTXD32 (and additional register 0xC6 bit 3).
> > > I guess its possible for the KSZ8863 to be the clock provider/consumer
> > > regardless of PHY/MAC mode?
> >
> >Register 0x35 bit 7 is for MII mode
> >Register 0xC6 bit 3 is for RMII mode
> >
> >MII != RMII
> 
> Yes, right you are. Looks like I got lost in the datasheets...
> 
> > > Table 3-5: RMII CLOCK SETTING of KSZ8863 datasheet describes the available
> > > clocking modes. If we try to create a relation between KSZ9897 and KSZ8863:
> > >
> > > KSZ9897 "Normal Mode" is equivalent to KSZ8863 mode described in first column
> > > of table 3-5:
> > > - EN_REFCLKO = 0, 0xC6(3) = 0 -> external 50Mhz OSC input to REFCLKI and X1
> > >   pin directly
> > >
> > > KSZ9897 "Clock Mode" is equivalent to KSZ8863 mode described in fourth/fifth
> > > column (difference is only clock frequency) of table 3-5:
> > > - EN_REFCLKO = 1, 0xC6(3) = 1 -> 50/25Mhz on X1 pin, 50/25Mhz RMII clock goes
> > >   to REFCLKI internally. REFCLKI can be pulled down by resistor.
> > >
> > > That leaves us with additional columns 2 and 3 of table 3-5 for KSZ8863, that
> > > are similar to KSZ9897 Clock mode, but REFCLKI needs to be fed externally from
> > > REFCLKO.
> > 
> > All of 5 variants described in "Table 3-5: RMII CLOCK SETTING of KSZ8863"
> > can be boiled down to two main configurations:
> > 
> > REFCLKI is used as clock source for internal MAC == Normal Mode or
> > RevRMII mode.
> > REFCLKI is not used as clock source for internal MAC == Clock Mode or
> > RMII mode.
> > 
> > Variants 1, 2, 3 describe only how can we feed REFCLKI from outside of
> > the chip. Even variant 2 and 3 make the switch to be an actually
> > physical clock provider, we still need to use REFCLKI and wire it
> > outside of the chip which make it practically a Normal Mode or RevRMII mode.
> 
> That is correct, I guess its a matter of nomenclature, but how do you 
> "tell" the switch whether it has REFCLKI routed externally or not if not by 
> setting the 0xC6 bit 3? Is there another way to achieve this?

I do not see any other way to "tell" it. The only thing to change in you
patches is a different way to tell it to the kernel.
Instead of introducing a new devicetree property, you need to reuse
phy-mode property.

> > > > I already did some work to configure CPU interface, where which can be at least
> > > > partially reused for your work:
> > > > https://lore.kernel.org/all/20230517121034.3801640-2-o.rempel@pengutronix.de/
> > > > (Looks I forgot to complete mainlining for this patch)
> > > >
> > > > If implanted as described, no new devicetree properties will be needed.
> > >
> > > I don't quite get how the proposed patch might effect this topic?
> > 
> > You will need to add ksz8_phylink_mac_link_up() as this patch already
> > dose.
> > 
> > > By setting PHY/MAC mode? As noted, I dont see the same relation between clock and
> > > MII mode for KSZ8863 as for KSZ9897?
> > 
> > I hope current mail will clear it.
> 
> I tried your patch but it does not do it for me. As stated, my hw platform does
> not have REFCLKI routed externally so a state at column 4/5 is expected.

My patches do not address your problem. It is just example which already creates
ksz8_phylink_mac_link_up() and do some CPU port configuration, which you
will need do too, but for other registers. Or just for get it to avoid
confusion.

What you need is to set 0xC6 bit 3 for PHY_INTERFACE_MODE_REVRMII and 
clear it for PHY_INTERFACE_MODE_RMII.

Since phy-mode for RMII was never set correctly, it will most probably
break every single devicetree using KSZ switches. It is the price of fixing
things :/

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |