linux-kernel - Re: [PATCH v4 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support

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:
 <DS7PR19MB888335F938BA5A8F19FAC8F49D6BA@DS7PR19MB8883.namprd19.prod.outlook.com>
Date: Mon, 9 Jun 2025 16:53:12 +0400
From: George Moussalem <george.moussalem@...look.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Florian Fainelli <f.fainelli@...il.com>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, netdev@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 linux-clk@...r.kernel.org
Subject: Re: [PATCH v4 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018
 Internal PHY support

On 6/9/25 16:24, Andrew Lunn wrote:
>> -#include <linux/phy.h>
>> -#include <linux/module.h>
>> -#include <linux/string.h>
>> -#include <linux/netdevice.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>>   #include <linux/etherdevice.h>
>>   #include <linux/ethtool_netlink.h>
>> -#include <linux/bitfield.h>
>> -#include <linux/regulator/of_regulator.h>
>> -#include <linux/regulator/driver.h>
>> -#include <linux/regulator/consumer.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/netdevice.h>
>>   #include <linux/of.h>
>> +#include <linux/phy.h>
>>   #include <linux/phylink.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/of_regulator.h>
>> +#include <linux/reset.h>
>>   #include <linux/sfp.h>
>> +#include <linux/string.h>
>>   #include <dt-bindings/net/qca-ar803x.h>
> 
> Please sort the headers in a separate patch. Cleanup and new features
> are logically different things, so should not be mixed.

Will do in v5, thanks.

> 
>> +static void ipq5018_link_change_notify(struct phy_device *phydev)
>> +{
>> +	mdiobus_modify_changed(phydev->mdio.bus, phydev->mdio.addr,
>> +			       IPQ5018_PHY_FIFO_CONTROL, IPQ5018_PHY_FIFO_RESET,
>> +			       phydev->link ? IPQ5018_PHY_FIFO_RESET : 0);
>> +}
> 
> 
> link_change_notify is pretty much only used when the PHY is broken. So
> it would be good to add a comment what is happening here. Why does the
> FIFO need to be reset?

in the downstream QCA-SSDK, the phy is reset when the link state changes 
to 'up'. And as part of that sequence, the FIFO buffer is cleared. I 
don't have access to the documentation, unfortunately.

Link: 
https://git.codelinaro.org/clo/qsdk/oss/lklm/qca-ssdk/-/blob/NHSS.QSDK.12.5/src/adpt/mp/adpt_mp_portctrl.c#L1150

> 
> 	Andrew

George