netdev - RE: [PATCH net-next 3/4] net: phy: realteck: add dt property to disable ALDPS mode

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:   Wed, 2 Jun 2021 02:22:38 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Russell King <linux@...linux.org.uk>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next 3/4] net: phy: realteck: add dt property to
 disable ALDPS mode


Hi Russell,

> -----Original Message-----
> From: Russell King <linux@...linux.org.uk>
> Sent: 2021年6月1日 19:52
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: davem@...emloft.net; kuba@...nel.org; robh+dt@...nel.org;
> andrew@...n.ch; hkallweit1@...il.com; f.fainelli@...il.com; dl-linux-imx
> <linux-imx@....com>; netdev@...r.kernel.org; devicetree@...r.kernel.org;
> linux-kernel@...r.kernel.org
> Subject: Re: [PATCH net-next 3/4] net: phy: realteck: add dt property to disable
> ALDPS mode
> 
> On Tue, Jun 01, 2021 at 05:04:07PM +0800, Joakim Zhang wrote:
> > @@ -325,8 +329,10 @@ static int rtl8211f_config_init(struct phy_device
> *phydev)
> >  	u16 val;
> >  	int ret;
> >
> > -	val = RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_PLL_OFF |
> RTL8211F_ALDPS_XTAL_OFF;
> > -	phy_modify_paged_changed(phydev, 0xa43, RTL8211F_PHYCR1, val, val);
> > +	if (!(priv->quirks & RTL821X_ALDPS_DISABLE_FEATURE)) {
> > +		val = RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_PLL_OFF |
> RTL8211F_ALDPS_XTAL_OFF;
> > +		phy_modify_paged_changed(phydev, 0xa43, RTL8211F_PHYCR1, val,
> val);
> > +	}
> 
> Similar questions as with the previous patch, but also... this doesn't actually
> disable the feature if it was previously turned on. E.g. a
> kexec() from a current kernel that has set these features into a subsequent
> kernel that the DT requests the feature to be disabled. Or a boot loader that
> has enabled this feature.
Sorry, I don't know what your meanings. As I explained before, boot loader also can configure PHY registers,
but kernel should not depend on what boot loader did.

If you use kexec() to boot kernel with DT request to disable this clock, PHY probe also can parse this property to disable it.
I know little about kexec(), could you please explain more if I misunderstand?

> If DT specifies that this feature is disabled, shouldn't this code be disabling it
> explicitly?
Yes, exactly should.

Best Regards,
Joakim Zhang
> --
> RMK's Patch system:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=04%7C01%7Cqiangqin
> g.zhang%40nxp.com%7Ceacabd25941448301acb08d924f3b6de%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C637581451436345731%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3jyYeGVBXXb5jCDtyTrt3DI9MwVE
> OT5Et9tpCZG26gY%3D&amp;reserved=0
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!