* [PATCH V2 2/2] net: phy: micrel: Update KSZ87xx PHY name
2019-10-10 19:46 [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs Marek Vasut
@ 2019-10-10 19:46 ` Marek Vasut
2019-10-11 5:57 ` [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs Simon Horman
2019-10-12 20:58 ` Heiner Kallweit
2 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2019-10-10 19:46 UTC (permalink / raw)
To: netdev
Cc: Marek Vasut, Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Heiner Kallweit, Tristram Ha, Woojung Huh
The KSZ8795 PHY ID is in fact used by KSZ8794/KSZ8795/KSZ8765 switches.
Update the PHY ID and name to reflect that, as this family of switches
is commonly refered to as KSZ87xx
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
---
V2: Rebase on top of 1/2, no functional change
---
drivers/net/phy/micrel.c | 4 ++--
include/linux/micrel_phy.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 028a4a177790..2c5b0f81d4f3 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -387,7 +387,7 @@ static int ksz8795_match_phy_device(struct phy_device *phydev)
{
int ret;
- if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8795)
+ if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ87XX)
return 0;
ret = phy_read(phydev, MII_BMSR);
@@ -1174,7 +1174,7 @@ static struct phy_driver ksphy_driver[] = {
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
- .name = "Micrel KSZ8795",
+ .name = "Micrel KSZ87XX Switch",
/* PHY_BASIC_FEATURES */
.config_init = kszphy_config_init,
.config_aneg = ksz8873mll_config_aneg,
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index ad24554f11f9..75f880c25bb8 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -31,7 +31,7 @@
#define PHY_ID_KSZ886X 0x00221430
#define PHY_ID_KSZ8863 0x00221435
-#define PHY_ID_KSZ8795 0x00221550
+#define PHY_ID_KSZ87XX 0x00221550
#define PHY_ID_KSZ9477 0x00221631
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs
2019-10-10 19:46 [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs Marek Vasut
2019-10-10 19:46 ` [PATCH V2 2/2] net: phy: micrel: Update KSZ87xx PHY name Marek Vasut
@ 2019-10-11 5:57 ` Simon Horman
2019-10-11 6:52 ` Marek Vasut
2019-10-12 20:58 ` Heiner Kallweit
2 siblings, 1 reply; 8+ messages in thread
From: Simon Horman @ 2019-10-11 5:57 UTC (permalink / raw)
To: Marek Vasut
Cc: netdev, Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Heiner Kallweit, Tristram Ha, Woojung Huh
On Thu, Oct 10, 2019 at 09:46:21PM +0200, Marek Vasut wrote:
> The KSZ8051 PHY and the KSZ8794/KSZ8795/KSZ8765 switch share exactly the
> same PHY ID. Since KSZ8051 is higher in the ksphy_driver[] list of PHYs
> in the micrel PHY driver, it is used even with the KSZ87xx switch. This
> is wrong, since the KSZ8051 configures registers of the PHY which are
> not present on the simplified KSZ87xx switch PHYs and misconfigures
> other registers of the KSZ87xx switch PHYs.
>
> Fortunatelly, it is possible to tell apart the KSZ8051 PHY from the
> KSZ87xx switch by checking the Basic Status register Bit 0, which is
> read-only and indicates presence of the Extended Capability Registers.
> The KSZ8051 PHY has those registers while the KSZ87xx switch does not.
>
> This patch implements simple check for the presence of this bit for
> both the KSZ8051 PHY and KSZ87xx switch, to let both use the correct
> PHY driver instance.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: George McCollister <george.mccollister@gmail.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> Cc: Woojung Huh <woojung.huh@microchip.com>
> ---
> NOTE: It was also suggested to populate phydev->dev_flags to discern
> the PHY from the switch, this does not work for setups where
> the switch is used as a PHY without a DSA driver. Checking the
> BMSR Bit 0 for Extended Capability Register works for both DSA
> and non-DSA usecase.
> V2: Move phy_id check into ksz8051_match_phy_device() and
> ksz8795_match_phy_device() and drop phy_id{,_mask} from the
> ksphy_driver[] list to avoid matching on other PHY IDs.
> ---
> drivers/net/phy/micrel.c | 40 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 2fea5541c35a..028a4a177790 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -341,6 +341,25 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
> return genphy_config_aneg(phydev);
> }
>
> +static int ksz8051_match_phy_device(struct phy_device *phydev)
> +{
> + int ret;
> +
> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8051)
> + return 0;
> +
> + ret = phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + return ret;
> +
> + /* KSZ8051 PHY and KSZ8794/KSZ8795/KSZ8765 switch share the same
> + * exact PHY ID. However, they can be told apart by the extended
> + * capability registers presence. The KSZ8051 PHY has them while
> + * the switch does not.
> + */
> + return ret & BMSR_ERCAP;
> +}
> +
> static int ksz8081_config_init(struct phy_device *phydev)
> {
> /* KSZPHY_OMSO_FACTORY_TEST is set at de-assertion of the reset line
> @@ -364,6 +383,21 @@ static int ksz8061_config_init(struct phy_device *phydev)
> return kszphy_config_init(phydev);
> }
>
> +static int ksz8795_match_phy_device(struct phy_device *phydev)
> +{
> + int ret;
> +
> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8795)
> + return 0;
> +
> + ret = phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + return ret;
> +
> + /* See comment in ksz8051_match_phy_device() for details. */
> + return !(ret & BMSR_ERCAP);
> +}
> +
Hi Marek,
given the similarity between ksz8051_match_phy_device() and
ksz8795_match_phy_device() I wonder if a common helper is appropriate.
> static int ksz9021_load_values_from_of(struct phy_device *phydev,
> const struct device_node *of_node,
> u16 reg,
> @@ -1017,8 +1051,6 @@ static struct phy_driver ksphy_driver[] = {
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> - .phy_id = PHY_ID_KSZ8051,
> - .phy_id_mask = MICREL_PHY_ID_MASK,
> .name = "Micrel KSZ8051",
> /* PHY_BASIC_FEATURES */
> .driver_data = &ksz8051_type,
> @@ -1029,6 +1061,7 @@ static struct phy_driver ksphy_driver[] = {
> .get_sset_count = kszphy_get_sset_count,
> .get_strings = kszphy_get_strings,
> .get_stats = kszphy_get_stats,
> + .match_phy_device = ksz8051_match_phy_device,
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> @@ -1141,13 +1174,12 @@ static struct phy_driver ksphy_driver[] = {
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> - .phy_id = PHY_ID_KSZ8795,
> - .phy_id_mask = MICREL_PHY_ID_MASK,
> .name = "Micrel KSZ8795",
> /* PHY_BASIC_FEATURES */
> .config_init = kszphy_config_init,
> .config_aneg = ksz8873mll_config_aneg,
> .read_status = ksz8873mll_read_status,
> + .match_phy_device = ksz8795_match_phy_device,
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs
2019-10-11 5:57 ` [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs Simon Horman
@ 2019-10-11 6:52 ` Marek Vasut
2019-10-11 9:47 ` Simon Horman
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2019-10-11 6:52 UTC (permalink / raw)
To: Simon Horman
Cc: netdev, Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Heiner Kallweit, Tristram Ha, Woojung Huh
On 10/11/19 7:57 AM, Simon Horman wrote:
[...]
>> +static int ksz8795_match_phy_device(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8795)
>> + return 0;
>> +
>> + ret = phy_read(phydev, MII_BMSR);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* See comment in ksz8051_match_phy_device() for details. */
>> + return !(ret & BMSR_ERCAP);
>> +}
>> +
>
> Hi Marek,
>
> given the similarity between ksz8051_match_phy_device() and
> ksz8795_match_phy_device() I wonder if a common helper is appropriate.
Then one (or both) of them look like this:
static int ksz8795_match_phy_device(struct phy_device *phydev)
{
int ret;
/* See comment in ksz8051_match_phy_device() for details. */
ret = ksz8051_match_phy_device(phydev);
if (ret < 0)
return ret;
return !ret;
}
It's not that much better.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs
2019-10-11 6:52 ` Marek Vasut
@ 2019-10-11 9:47 ` Simon Horman
0 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2019-10-11 9:47 UTC (permalink / raw)
To: Marek Vasut
Cc: netdev, Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Heiner Kallweit, Tristram Ha, Woojung Huh
On Fri, Oct 11, 2019 at 08:52:13AM +0200, Marek Vasut wrote:
> On 10/11/19 7:57 AM, Simon Horman wrote:
> [...]
> >> +static int ksz8795_match_phy_device(struct phy_device *phydev)
> >> +{
> >> + int ret;
> >> +
> >> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8795)
> >> + return 0;
> >> +
> >> + ret = phy_read(phydev, MII_BMSR);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + /* See comment in ksz8051_match_phy_device() for details. */
> >> + return !(ret & BMSR_ERCAP);
> >> +}
> >> +
> >
> > Hi Marek,
> >
> > given the similarity between ksz8051_match_phy_device() and
> > ksz8795_match_phy_device() I wonder if a common helper is appropriate.
>
> Then one (or both) of them look like this:
>
> static int ksz8795_match_phy_device(struct phy_device *phydev)
> {
> int ret;
>
> /* See comment in ksz8051_match_phy_device() for details. */
> ret = ksz8051_match_phy_device(phydev);
> if (ret < 0)
> return ret;
>
> return !ret;
> }
>
> It's not that much better.
Hi Marek,
I think I slightly prefer this but I do see your point
and I have no objections to leaving the patch as-is.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs
2019-10-10 19:46 [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs Marek Vasut
2019-10-10 19:46 ` [PATCH V2 2/2] net: phy: micrel: Update KSZ87xx PHY name Marek Vasut
2019-10-11 5:57 ` [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs Simon Horman
@ 2019-10-12 20:58 ` Heiner Kallweit
2019-10-12 21:22 ` Heiner Kallweit
2 siblings, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2019-10-12 20:58 UTC (permalink / raw)
To: Marek Vasut, netdev
Cc: Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Tristram Ha, Woojung Huh
On 10.10.2019 21:46, Marek Vasut wrote:
> The KSZ8051 PHY and the KSZ8794/KSZ8795/KSZ8765 switch share exactly the
> same PHY ID. Since KSZ8051 is higher in the ksphy_driver[] list of PHYs
> in the micrel PHY driver, it is used even with the KSZ87xx switch. This
> is wrong, since the KSZ8051 configures registers of the PHY which are
> not present on the simplified KSZ87xx switch PHYs and misconfigures
> other registers of the KSZ87xx switch PHYs.
>
> Fortunatelly, it is possible to tell apart the KSZ8051 PHY from the
> KSZ87xx switch by checking the Basic Status register Bit 0, which is
> read-only and indicates presence of the Extended Capability Registers.
> The KSZ8051 PHY has those registers while the KSZ87xx switch does not.
>
> This patch implements simple check for the presence of this bit for
> both the KSZ8051 PHY and KSZ87xx switch, to let both use the correct
> PHY driver instance.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: George McCollister <george.mccollister@gmail.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> Cc: Woojung Huh <woojung.huh@microchip.com>
> ---
> NOTE: It was also suggested to populate phydev->dev_flags to discern
> the PHY from the switch, this does not work for setups where
> the switch is used as a PHY without a DSA driver. Checking the
> BMSR Bit 0 for Extended Capability Register works for both DSA
> and non-DSA usecase.
> V2: Move phy_id check into ksz8051_match_phy_device() and
> ksz8795_match_phy_device() and drop phy_id{,_mask} from the
> ksphy_driver[] list to avoid matching on other PHY IDs.
> ---
> drivers/net/phy/micrel.c | 40 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 2fea5541c35a..028a4a177790 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -341,6 +341,25 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
> return genphy_config_aneg(phydev);
> }
>
> +static int ksz8051_match_phy_device(struct phy_device *phydev)
> +{
> + int ret;
> +
> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8051)
> + return 0;
> +
> + ret = phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + return ret;
> +
> + /* KSZ8051 PHY and KSZ8794/KSZ8795/KSZ8765 switch share the same
> + * exact PHY ID. However, they can be told apart by the extended
> + * capability registers presence. The KSZ8051 PHY has them while
> + * the switch does not.
> + */
> + return ret & BMSR_ERCAP;
> +}
> +
> static int ksz8081_config_init(struct phy_device *phydev)
> {
> /* KSZPHY_OMSO_FACTORY_TEST is set at de-assertion of the reset line
> @@ -364,6 +383,21 @@ static int ksz8061_config_init(struct phy_device *phydev)
> return kszphy_config_init(phydev);
> }
>
> +static int ksz8795_match_phy_device(struct phy_device *phydev)
> +{
> + int ret;
> +
> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8795)
> + return 0;
> +
> + ret = phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + return ret;
> +
> + /* See comment in ksz8051_match_phy_device() for details. */
> + return !(ret & BMSR_ERCAP);
> +}
> +
> static int ksz9021_load_values_from_of(struct phy_device *phydev,
> const struct device_node *of_node,
> u16 reg,
> @@ -1017,8 +1051,6 @@ static struct phy_driver ksphy_driver[] = {
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> - .phy_id = PHY_ID_KSZ8051,
> - .phy_id_mask = MICREL_PHY_ID_MASK,
> .name = "Micrel KSZ8051",
> /* PHY_BASIC_FEATURES */
> .driver_data = &ksz8051_type,
> @@ -1029,6 +1061,7 @@ static struct phy_driver ksphy_driver[] = {
> .get_sset_count = kszphy_get_sset_count,
> .get_strings = kszphy_get_strings,
> .get_stats = kszphy_get_stats,
> + .match_phy_device = ksz8051_match_phy_device,
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> @@ -1141,13 +1174,12 @@ static struct phy_driver ksphy_driver[] = {
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
> - .phy_id = PHY_ID_KSZ8795,
> - .phy_id_mask = MICREL_PHY_ID_MASK,
> .name = "Micrel KSZ8795",
> /* PHY_BASIC_FEATURES */
> .config_init = kszphy_config_init,
> .config_aneg = ksz8873mll_config_aneg,
> .read_status = ksz8873mll_read_status,
> + .match_phy_device = ksz8795_match_phy_device,
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> }, {
>
Patch needs to be annotated as "net-next".
See https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
Apart from that:
Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs
2019-10-12 20:58 ` Heiner Kallweit
@ 2019-10-12 21:22 ` Heiner Kallweit
2019-10-13 10:53 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2019-10-12 21:22 UTC (permalink / raw)
To: Marek Vasut, netdev
Cc: Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Tristram Ha, Woojung Huh
On 12.10.2019 22:58, Heiner Kallweit wrote:
> On 10.10.2019 21:46, Marek Vasut wrote:
>> The KSZ8051 PHY and the KSZ8794/KSZ8795/KSZ8765 switch share exactly the
>> same PHY ID. Since KSZ8051 is higher in the ksphy_driver[] list of PHYs
>> in the micrel PHY driver, it is used even with the KSZ87xx switch. This
>> is wrong, since the KSZ8051 configures registers of the PHY which are
>> not present on the simplified KSZ87xx switch PHYs and misconfigures
>> other registers of the KSZ87xx switch PHYs.
>>
>> Fortunatelly, it is possible to tell apart the KSZ8051 PHY from the
>> KSZ87xx switch by checking the Basic Status register Bit 0, which is
>> read-only and indicates presence of the Extended Capability Registers.
>> The KSZ8051 PHY has those registers while the KSZ87xx switch does not.
>>
>> This patch implements simple check for the presence of this bit for
>> both the KSZ8051 PHY and KSZ87xx switch, to let both use the correct
>> PHY driver instance.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: George McCollister <george.mccollister@gmail.com>
>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
>> Cc: Woojung Huh <woojung.huh@microchip.com>
>> ---
>> NOTE: It was also suggested to populate phydev->dev_flags to discern
>> the PHY from the switch, this does not work for setups where
>> the switch is used as a PHY without a DSA driver. Checking the
>> BMSR Bit 0 for Extended Capability Register works for both DSA
>> and non-DSA usecase.
>> V2: Move phy_id check into ksz8051_match_phy_device() and
>> ksz8795_match_phy_device() and drop phy_id{,_mask} from the
>> ksphy_driver[] list to avoid matching on other PHY IDs.
>> ---
>> drivers/net/phy/micrel.c | 40 ++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 2fea5541c35a..028a4a177790 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -341,6 +341,25 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
>> return genphy_config_aneg(phydev);
>> }
>>
>> +static int ksz8051_match_phy_device(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8051)
>> + return 0;
>> +
>> + ret = phy_read(phydev, MII_BMSR);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* KSZ8051 PHY and KSZ8794/KSZ8795/KSZ8765 switch share the same
>> + * exact PHY ID. However, they can be told apart by the extended
>> + * capability registers presence. The KSZ8051 PHY has them while
>> + * the switch does not.
>> + */
>> + return ret & BMSR_ERCAP;
>> +}
>> +
>> static int ksz8081_config_init(struct phy_device *phydev)
>> {
>> /* KSZPHY_OMSO_FACTORY_TEST is set at de-assertion of the reset line
>> @@ -364,6 +383,21 @@ static int ksz8061_config_init(struct phy_device *phydev)
>> return kszphy_config_init(phydev);
>> }
>>
>> +static int ksz8795_match_phy_device(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + if ((phydev->phy_id & MICREL_PHY_ID_MASK) != PHY_ID_KSZ8795)
>> + return 0;
>> +
>> + ret = phy_read(phydev, MII_BMSR);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* See comment in ksz8051_match_phy_device() for details. */
>> + return !(ret & BMSR_ERCAP);
>> +}
>> +
>> static int ksz9021_load_values_from_of(struct phy_device *phydev,
>> const struct device_node *of_node,
>> u16 reg,
>> @@ -1017,8 +1051,6 @@ static struct phy_driver ksphy_driver[] = {
>> .suspend = genphy_suspend,
>> .resume = genphy_resume,
>> }, {
>> - .phy_id = PHY_ID_KSZ8051,
>> - .phy_id_mask = MICREL_PHY_ID_MASK,
>> .name = "Micrel KSZ8051",
>> /* PHY_BASIC_FEATURES */
>> .driver_data = &ksz8051_type,
>> @@ -1029,6 +1061,7 @@ static struct phy_driver ksphy_driver[] = {
>> .get_sset_count = kszphy_get_sset_count,
>> .get_strings = kszphy_get_strings,
>> .get_stats = kszphy_get_stats,
>> + .match_phy_device = ksz8051_match_phy_device,
>> .suspend = genphy_suspend,
>> .resume = genphy_resume,
>> }, {
>> @@ -1141,13 +1174,12 @@ static struct phy_driver ksphy_driver[] = {
>> .suspend = genphy_suspend,
>> .resume = genphy_resume,
>> }, {
>> - .phy_id = PHY_ID_KSZ8795,
>> - .phy_id_mask = MICREL_PHY_ID_MASK,
>> .name = "Micrel KSZ8795",
>> /* PHY_BASIC_FEATURES */
>> .config_init = kszphy_config_init,
>> .config_aneg = ksz8873mll_config_aneg,
>> .read_status = ksz8873mll_read_status,
>> + .match_phy_device = ksz8795_match_phy_device,
>> .suspend = genphy_suspend,
>> .resume = genphy_resume,
>> }, {
>>
>
> Patch needs to be annotated as "net-next".
> See https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
>
Except you consider this a fix, then it would require a Fixes tag and
should be annotated "net". The question is:
Do KSZ87xx switches misbehave currently?
> Apart from that:
> Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] net: phy: micrel: Discern KSZ8051 and KSZ8795 PHYs
2019-10-12 21:22 ` Heiner Kallweit
@ 2019-10-13 10:53 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2019-10-13 10:53 UTC (permalink / raw)
To: Heiner Kallweit, netdev
Cc: Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Tristram Ha, Woojung Huh
On 10/12/19 11:22 PM, Heiner Kallweit wrote:
[...]
>>> @@ -1141,13 +1174,12 @@ static struct phy_driver ksphy_driver[] = {
>>> .suspend = genphy_suspend,
>>> .resume = genphy_resume,
>>> }, {
>>> - .phy_id = PHY_ID_KSZ8795,
>>> - .phy_id_mask = MICREL_PHY_ID_MASK,
>>> .name = "Micrel KSZ8795",
>>> /* PHY_BASIC_FEATURES */
>>> .config_init = kszphy_config_init,
>>> .config_aneg = ksz8873mll_config_aneg,
>>> .read_status = ksz8873mll_read_status,
>>> + .match_phy_device = ksz8795_match_phy_device,
>>> .suspend = genphy_suspend,
>>> .resume = genphy_resume,
>>> }, {
>>>
>>
>> Patch needs to be annotated as "net-next".
>> See https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
>>
> Except you consider this a fix, then it would require a Fixes tag and
> should be annotated "net". The question is:
> Do KSZ87xx switches misbehave currently?
Well yes they do, otherwise I won't be sending this fix.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread