* [PATCH net-next] net: phy: bcm87xx: improve feature configuration
@ 2019-04-16 22:06 Heiner Kallweit
2019-04-16 22:25 ` Andrew Lunn
2019-04-18 23:30 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Heiner Kallweit @ 2019-04-16 22:06 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
This driver is the only user of PHY_10GBIT_FEC_FEATURES. So we may be
able to remove this predefined feature constant later. Setting
phydev->advertising to what is supported is done by phy_probe(),
therefore we don't have to do it in the config_init callback.
Resetting phydev->autoneg if autoneg isn't supported is also done by
phy_probe() now, and manually setting the phylib state machine to
NOLINK is neither needed nor advisable.
In addition propagate the return value of bcm87xx_of_reg_init.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/bcm87xx.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
index f0c0eefe2..805fe62e4 100644
--- a/drivers/net/phy/bcm87xx.c
+++ b/drivers/net/phy/bcm87xx.c
@@ -81,22 +81,19 @@ static int bcm87xx_of_reg_init(struct phy_device *phydev)
}
#endif /* CONFIG_OF_MDIO */
-static int bcm87xx_config_init(struct phy_device *phydev)
+static int bcm87xx_get_features(struct phy_device *phydev)
{
linkmode_zero(phydev->supported);
linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseR_FEC_BIT,
phydev->supported);
- linkmode_zero(phydev->advertising);
- linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseR_FEC_BIT,
- phydev->advertising);
- phydev->state = PHY_NOLINK;
- phydev->autoneg = AUTONEG_DISABLE;
-
- bcm87xx_of_reg_init(phydev);
-
return 0;
}
+static int bcm87xx_config_init(struct phy_device *phydev)
+{
+ return bcm87xx_of_reg_init(phydev);
+}
+
static int bcm87xx_config_aneg(struct phy_device *phydev)
{
return -EINVAL;
@@ -194,7 +191,7 @@ static struct phy_driver bcm87xx_driver[] = {
.phy_id = PHY_ID_BCM8706,
.phy_id_mask = 0xffffffff,
.name = "Broadcom BCM8706",
- .features = PHY_10GBIT_FEC_FEATURES,
+ .get_features = bcm87xx_get_features,
.config_init = bcm87xx_config_init,
.config_aneg = bcm87xx_config_aneg,
.read_status = bcm87xx_read_status,
@@ -206,7 +203,7 @@ static struct phy_driver bcm87xx_driver[] = {
.phy_id = PHY_ID_BCM8727,
.phy_id_mask = 0xffffffff,
.name = "Broadcom BCM8727",
- .features = PHY_10GBIT_FEC_FEATURES,
+ .get_features = bcm87xx_get_features,
.config_init = bcm87xx_config_init,
.config_aneg = bcm87xx_config_aneg,
.read_status = bcm87xx_read_status,
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] net: phy: bcm87xx: improve feature configuration
2019-04-16 22:06 [PATCH net-next] net: phy: bcm87xx: improve feature configuration Heiner Kallweit
@ 2019-04-16 22:25 ` Andrew Lunn
2019-04-16 22:34 ` Heiner Kallweit
2019-04-18 23:30 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-04-16 22:25 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
> @@ -194,7 +191,7 @@ static struct phy_driver bcm87xx_driver[] = {
> .phy_id = PHY_ID_BCM8706,
> .phy_id_mask = 0xffffffff,
> .name = "Broadcom BCM8706",
> - .features = PHY_10GBIT_FEC_FEATURES,
> + .get_features = bcm87xx_get_features,
> .config_init = bcm87xx_config_init,
> .config_aneg = bcm87xx_config_aneg,
> .read_status = bcm87xx_read_status,
> @@ -206,7 +203,7 @@ static struct phy_driver bcm87xx_driver[] = {
> .phy_id = PHY_ID_BCM8727,
> .phy_id_mask = 0xffffffff,
> .name = "Broadcom BCM8727",
> - .features = PHY_10GBIT_FEC_FEATURES,
> + .get_features = bcm87xx_get_features,
> .config_init = bcm87xx_config_init,
> .config_aneg = bcm87xx_config_aneg,
> .read_status = bcm87xx_read_status,
Hi Heiner
The product briefs for these two devices say:
MDIO interface compliant to IEEE802.3ae Clause 45 with
extended indirect address register access
It might be possible to just detect most of this?
Florian: Do you have access to any boards using these devices?
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] net: phy: bcm87xx: improve feature configuration
2019-04-16 22:25 ` Andrew Lunn
@ 2019-04-16 22:34 ` Heiner Kallweit
2019-04-16 23:47 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2019-04-16 22:34 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
On 17.04.2019 00:25, Andrew Lunn wrote:
>> @@ -194,7 +191,7 @@ static struct phy_driver bcm87xx_driver[] = {
>> .phy_id = PHY_ID_BCM8706,
>> .phy_id_mask = 0xffffffff,
>> .name = "Broadcom BCM8706",
>> - .features = PHY_10GBIT_FEC_FEATURES,
>> + .get_features = bcm87xx_get_features,
>> .config_init = bcm87xx_config_init,
>> .config_aneg = bcm87xx_config_aneg,
>> .read_status = bcm87xx_read_status,
>> @@ -206,7 +203,7 @@ static struct phy_driver bcm87xx_driver[] = {
>> .phy_id = PHY_ID_BCM8727,
>> .phy_id_mask = 0xffffffff,
>> .name = "Broadcom BCM8727",
>> - .features = PHY_10GBIT_FEC_FEATURES,
>> + .get_features = bcm87xx_get_features,
>> .config_init = bcm87xx_config_init,
>> .config_aneg = bcm87xx_config_aneg,
>> .read_status = bcm87xx_read_status,
>
> Hi Heiner
>
> The product briefs for these two devices say:
>
> MDIO interface compliant to IEEE802.3ae Clause 45 with
> extended indirect address register access
>
> It might be possible to just detect most of this?
>
Unfortunately I have no datasheet. There would be more things to
fix in this old driver. The C45 register accesses should be
switched to proper phy_mmd_read/write calls.
> Florian: Do you have access to any boards using these devices?
>
> Andrew
>
Heiner
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] net: phy: bcm87xx: improve feature configuration
2019-04-16 22:34 ` Heiner Kallweit
@ 2019-04-16 23:47 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2019-04-16 23:47 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn; +Cc: David Miller, netdev@vger.kernel.org
On 16/04/2019 15:34, Heiner Kallweit wrote:
> On 17.04.2019 00:25, Andrew Lunn wrote:
>>> @@ -194,7 +191,7 @@ static struct phy_driver bcm87xx_driver[] = {
>>> .phy_id = PHY_ID_BCM8706,
>>> .phy_id_mask = 0xffffffff,
>>> .name = "Broadcom BCM8706",
>>> - .features = PHY_10GBIT_FEC_FEATURES,
>>> + .get_features = bcm87xx_get_features,
>>> .config_init = bcm87xx_config_init,
>>> .config_aneg = bcm87xx_config_aneg,
>>> .read_status = bcm87xx_read_status,
>>> @@ -206,7 +203,7 @@ static struct phy_driver bcm87xx_driver[] = {
>>> .phy_id = PHY_ID_BCM8727,
>>> .phy_id_mask = 0xffffffff,
>>> .name = "Broadcom BCM8727",
>>> - .features = PHY_10GBIT_FEC_FEATURES,
>>> + .get_features = bcm87xx_get_features,
>>> .config_init = bcm87xx_config_init,
>>> .config_aneg = bcm87xx_config_aneg,
>>> .read_status = bcm87xx_read_status,
>>
>> Hi Heiner
>>
>> The product briefs for these two devices say:
>>
>> MDIO interface compliant to IEEE802.3ae Clause 45 with
>> extended indirect address register access
>>
>> It might be possible to just detect most of this?
>>
> Unfortunately I have no datasheet. There would be more things to
> fix in this old driver. The C45 register accesses should be
> switched to proper phy_mmd_read/write calls.
Checking the datasheet would not be a problem, I can reply later next
week when I am back to work.
>
>> Florian: Do you have access to any boards using these devices?
That I do not unfortunately, those are early 10G PHYs.
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: bcm87xx: improve feature configuration
2019-04-16 22:06 [PATCH net-next] net: phy: bcm87xx: improve feature configuration Heiner Kallweit
2019-04-16 22:25 ` Andrew Lunn
@ 2019-04-18 23:30 ` David Miller
2019-04-25 17:32 ` Heiner Kallweit
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2019-04-18 23:30 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 17 Apr 2019 00:06:28 +0200
> This driver is the only user of PHY_10GBIT_FEC_FEATURES. So we may be
> able to remove this predefined feature constant later. Setting
> phydev->advertising to what is supported is done by phy_probe(),
> therefore we don't have to do it in the config_init callback.
> Resetting phydev->autoneg if autoneg isn't supported is also done by
> phy_probe() now, and manually setting the phylib state machine to
> NOLINK is neither needed nor advisable.
> In addition propagate the return value of bcm87xx_of_reg_init.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
I read the discussion on this patch, there doesn't seem to be outright
NACK but rather some talk about future enhancements and something about
Florian looking at a few datasheets next week.
What's the story?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: bcm87xx: improve feature configuration
2019-04-18 23:30 ` David Miller
@ 2019-04-25 17:32 ` Heiner Kallweit
0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2019-04-25 17:32 UTC (permalink / raw)
To: David Miller; +Cc: andrew, f.fainelli, netdev
On 19.04.2019 01:30, David Miller wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Wed, 17 Apr 2019 00:06:28 +0200
>
>> This driver is the only user of PHY_10GBIT_FEC_FEATURES. So we may be
>> able to remove this predefined feature constant later. Setting
>> phydev->advertising to what is supported is done by phy_probe(),
>> therefore we don't have to do it in the config_init callback.
>> Resetting phydev->autoneg if autoneg isn't supported is also done by
>> phy_probe() now, and manually setting the phylib state machine to
>> NOLINK is neither needed nor advisable.
>> In addition propagate the return value of bcm87xx_of_reg_init.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> I read the discussion on this patch, there doesn't seem to be outright
> NACK but rather some talk about future enhancements and something about
> Florian looking at a few datasheets next week.
>
> What's the story?
>
Apart from this smaller improvement there may be more to improve in this
driver, but it needs the datasheet to find out.
I think it should also be ok to apply the current patch now, and further
improvements later.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-25 17:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-16 22:06 [PATCH net-next] net: phy: bcm87xx: improve feature configuration Heiner Kallweit
2019-04-16 22:25 ` Andrew Lunn
2019-04-16 22:34 ` Heiner Kallweit
2019-04-16 23:47 ` Florian Fainelli
2019-04-18 23:30 ` David Miller
2019-04-25 17:32 ` Heiner Kallweit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox