* Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031
2018-05-14 8:22 [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031 Marco Felsch
@ 2018-05-14 9:05 ` Uwe Kleine-König
2018-05-14 9:41 ` Sergei Shtylyov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2018-05-14 9:05 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, mark.rutland, andrew, f.fainelli, netdev, Markus Niebel,
kernel, devicetree
Hello,
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index f41b224a9cdb..3e4243e4f7a7 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -528,6 +528,8 @@ static int ksz9031_enable_edpd(struct phy_device *phydev)
>
> static int ksz9031_config_init(struct phy_device *phydev)
> {
> + int rc;
> + u16 val;
These could be declared in more local scope.
> const struct device *dev = &phydev->mdio.dev;
> const struct device_node *of_node = dev->of_node;
> static const char *clk_skews[2] = {"rxc-skew-ps", "txc-skew-ps"};
> @@ -573,6 +575,22 @@ static int ksz9031_config_init(struct phy_device *phydev)
> ksz9031_of_load_skew_values(phydev, of_node,
> MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
> tx_data_skews, 4);
> +
> + /* force master mode -> errata #2
> + * attention: Master <-> Master will not work
> + */
> + if (of_property_read_bool(of_node, "force-master")) {
> + rc = phy_read(phydev, MII_CTRL1000);
> + if (rc >= 0) {
> + val = (u16)rc;
> + /* enable master mode, config &
> + * prefer master
> + */
> + val |= (CTL1000_ENABLE_MASTER |
> + CTL1000_AS_MASTER);
> + phy_write(phydev, MII_CTRL1000, val);
There is no need for two variables, just use a single int variable that
on all Linux archs can hold the whole domain of u16 should be fine.
> + }
> + }
Maybe issue a warning if reading MII_CTRL1000 failed?
> }
>
> return ksz9031_center_flp_timing(phydev);
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031
2018-05-14 8:22 [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031 Marco Felsch
2018-05-14 9:05 ` Uwe Kleine-König
@ 2018-05-14 9:41 ` Sergei Shtylyov
2018-05-14 9:42 ` Sergei Shtylyov
2018-05-14 12:11 ` Andrew Lunn
3 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2018-05-14 9:41 UTC (permalink / raw)
To: Marco Felsch, robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, Markus Niebel
Hello!
On 5/14/2018 11:22 AM, Marco Felsch wrote:
> From: Markus Niebel <Markus.Niebel@tqs.de>
>
> handle errata #2 for KSZ9031: force 1000Base-T master
>
> Attention: enabling the workaround will cause no link to
> other GIGE master.
>
> Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
> [m.felsch@pengutronix.de: move dt binding to the KSZ9031 entry]
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
[...]
> @@ -573,6 +575,22 @@ static int ksz9031_config_init(struct phy_device *phydev)
> ksz9031_of_load_skew_values(phydev, of_node,
> MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
> tx_data_skews, 4);
> +
> + /* force master mode -> errata #2
> + * attention: Master <-> Master will not work
> + */
> + if (of_property_read_bool(of_node, "force-master")) {
> + rc = phy_read(phydev, MII_CTRL1000);
> + if (rc >= 0) {
> + val = (u16)rc;
> + /* enable master mode, config &
> + * prefer master
> + */
> + val |= (CTL1000_ENABLE_MASTER |
> + CTL1000_AS_MASTER);
Parens not needed.
> + phy_write(phydev, MII_CTRL1000, val);
> + }
> + }
> }
>
> return ksz9031_center_flp_timing(phydev);
>
MBR, Sergei
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031
2018-05-14 8:22 [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031 Marco Felsch
2018-05-14 9:05 ` Uwe Kleine-König
2018-05-14 9:41 ` Sergei Shtylyov
@ 2018-05-14 9:42 ` Sergei Shtylyov
2018-05-14 12:11 ` Andrew Lunn
3 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2018-05-14 9:42 UTC (permalink / raw)
To: Marco Felsch, robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, Markus Niebel
On 5/14/2018 11:22 AM, Marco Felsch wrote:
> From: Markus Niebel <Markus.Niebel@tqs.de>
>
> handle errata #2 for KSZ9031: force 1000Base-T master
>
> Attention: enabling the workaround will cause no link to
> other GIGE master.
>
> Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
> [m.felsch@pengutronix.de: move dt binding to the KSZ9031 entry]
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> .../devicetree/bindings/net/micrel-ksz90x1.txt | 3 +++
> drivers/net/phy/micrel.c | 18 ++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> index 42a248301615..e2465fbbbcef 100644
> --- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> +++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> @@ -57,6 +57,9 @@ KSZ9031:
> - txd2-skew-ps : Skew control of TX data 2 pad
> - txd3-skew-ps : Skew control of TX data 3 pad
>
> + - force-master: Boolean, force phy to master mode. This is a
Not "micrel,force-master"?
> + workaround at least for KSZ9031 errata #2.
> +
> Examples:
>
> mdio {
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031
2018-05-14 8:22 [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031 Marco Felsch
` (2 preceding siblings ...)
2018-05-14 9:42 ` Sergei Shtylyov
@ 2018-05-14 12:11 ` Andrew Lunn
3 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2018-05-14 12:11 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, mark.rutland, f.fainelli, netdev, devicetree, kernel,
Markus Niebel
On Mon, May 14, 2018 at 10:22:18AM +0200, Marco Felsch wrote:
> From: Markus Niebel <Markus.Niebel@tqs.de>
>
> handle errata #2 for KSZ9031: force 1000Base-T master
>
> Attention: enabling the workaround will cause no link to
> other GIGE master.
Hi Marco
It would be good to document why this is needed, when to use it, what
problems it fixes, etc. At the moment, there is not enough information
to decide if you should turn it on.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread