| Message ID | 20210712072413.11490-1-ms@dev.tdt.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v5] net: phy: intel-xway: Add RGMII internal delay configuration | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 108 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 7/12/21 9:24 AM, Martin Schiller wrote: > This adds the possibility to configure the RGMII RX/TX clock skew via > devicetree. > > Simply set phy mode to "rgmii-id", "rgmii-rxid" or "rgmii-txid" and add > the "rx-internal-delay-ps" or "tx-internal-delay-ps" property to the > devicetree. > > Furthermore, a warning is now issued if the phy mode is configured to > "rgmii" and an internal delay is set in the phy (e.g. by pin-strapping), > as in the dp83867 driver. > > Signed-off-by: Martin Schiller <ms@dev.tdt.de> Acked-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > > Changes to v4: > o Fix Alignment to match open parenthesis > > Changes to v3: > o Fix typo in commit message > o use FIELD_PREP() and FIELD_GET() macros > o further code cleanups > o always mask rxskew AND txskew value in the register value > > Changes to v2: > o Fix missing whitespace in warning. > > Changes to v1: > o code cleanup and use phy_modify(). > o use default of 2.0ns if delay property is absent instead of returning > an error. > > --- > drivers/net/phy/intel-xway.c | 85 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 85 insertions(+) > > diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c > index d453ec016168..bc7e2fdb8ea7 100644 > --- a/drivers/net/phy/intel-xway.c > +++ b/drivers/net/phy/intel-xway.c > @@ -8,11 +8,16 @@ > #include <linux/module.h> > #include <linux/phy.h> > #include <linux/of.h> > +#include <linux/bitfield.h> > > +#define XWAY_MDIO_MIICTRL 0x17 /* mii control */ > #define XWAY_MDIO_IMASK 0x19 /* interrupt mask */ > #define XWAY_MDIO_ISTAT 0x1A /* interrupt status */ > #define XWAY_MDIO_LED 0x1B /* led control */ > > +#define XWAY_MDIO_MIICTRL_RXSKEW_MASK GENMASK(14, 12) > +#define XWAY_MDIO_MIICTRL_TXSKEW_MASK GENMASK(10, 8) > + > /* bit 15:12 are reserved */ > #define XWAY_MDIO_LED_LED3_EN BIT(11) /* Enable the integrated function of LED3 */ > #define XWAY_MDIO_LED_LED2_EN BIT(10) /* Enable the integrated function of LED2 */ > @@ -157,6 +162,82 @@ > #define PHY_ID_PHY11G_VR9_1_2 0xD565A409 > #define PHY_ID_PHY22F_VR9_1_2 0xD565A419 > > +#if IS_ENABLED(CONFIG_OF_MDIO) > +static const int xway_internal_delay[] = {0, 500, 1000, 1500, 2000, 2500, > + 3000, 3500}; > + > +static int xway_gphy_of_reg_init(struct phy_device *phydev) > +{ > + struct device *dev = &phydev->mdio.dev; > + unsigned int delay_size = ARRAY_SIZE(xway_internal_delay); > + s32 int_delay; > + int val = 0; > + > + if (!phy_interface_is_rgmii(phydev)) > + return 0; > + > + /* Existing behavior was to use default pin strapping delay in rgmii > + * mode, but rgmii should have meant no delay. Warn existing users, > + * but do not change anything at the moment. > + */ > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII) { > + u16 txskew, rxskew; > + > + val = phy_read(phydev, XWAY_MDIO_MIICTRL); > + if (val < 0) > + return val; > + > + txskew = FIELD_GET(XWAY_MDIO_MIICTRL_TXSKEW_MASK, val); > + rxskew = FIELD_GET(XWAY_MDIO_MIICTRL_RXSKEW_MASK, val); > + > + if (txskew > 0 || rxskew > 0) > + phydev_warn(phydev, > + "PHY has delays (e.g. via pin strapping), but phy-mode = 'rgmii'\n" > + "Should be 'rgmii-id' to use internal delays txskew:%d ps rxskew:%d ps\n", > + xway_internal_delay[txskew], > + xway_internal_delay[rxskew]); > + return 0; > + } > + > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) { > + int_delay = phy_get_internal_delay(phydev, dev, > + xway_internal_delay, > + delay_size, true); > + > + if (int_delay < 0) { > + phydev_warn(phydev, "rx-internal-delay-ps is missing, use default of 2.0 ns\n"); > + int_delay = 4; /* 2000 ps */ > + } > + > + val |= FIELD_PREP(XWAY_MDIO_MIICTRL_RXSKEW_MASK, int_delay); > + } > + > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) { > + int_delay = phy_get_internal_delay(phydev, dev, > + xway_internal_delay, > + delay_size, false); > + > + if (int_delay < 0) { > + phydev_warn(phydev, "tx-internal-delay-ps is missing, use default of 2.0 ns\n"); > + int_delay = 4; /* 2000 ps */ > + } > + > + val |= FIELD_PREP(XWAY_MDIO_MIICTRL_TXSKEW_MASK, int_delay); > + } > + > + return phy_modify(phydev, XWAY_MDIO_MIICTRL, > + XWAY_MDIO_MIICTRL_RXSKEW_MASK | > + XWAY_MDIO_MIICTRL_TXSKEW_MASK, val); > +} > +#else > +static int xway_gphy_of_reg_init(struct phy_device *phydev) > +{ > + return 0; > +} > +#endif /* CONFIG_OF_MDIO */ > + > static int xway_gphy_config_init(struct phy_device *phydev) > { > int err; > @@ -204,6 +285,10 @@ static int xway_gphy_config_init(struct phy_device *phydev) > phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2H, ledxh); > phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2L, ledxl); > > + err = xway_gphy_of_reg_init(phydev); > + if (err) > + return err; > + > return 0; > } > >
On Mon, Jul 12, 2021 at 9:24 AM Martin Schiller <ms@dev.tdt.de> wrote: > > This adds the possibility to configure the RGMII RX/TX clock skew via > devicetree. > > Simply set phy mode to "rgmii-id", "rgmii-rxid" or "rgmii-txid" and add > the "rx-internal-delay-ps" or "tx-internal-delay-ps" property to the > devicetree. > > Furthermore, a warning is now issued if the phy mode is configured to > "rgmii" and an internal delay is set in the phy (e.g. by pin-strapping), > as in the dp83867 driver. > > Signed-off-by: Martin Schiller <ms@dev.tdt.de> Thanks for this updated version! Everything's looking good so: Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
On Mon, Jul 12, 2021 at 09:24:13AM +0200, Martin Schiller wrote: > This adds the possibility to configure the RGMII RX/TX clock skew via > devicetree. > > Simply set phy mode to "rgmii-id", "rgmii-rxid" or "rgmii-txid" and add > the "rx-internal-delay-ps" or "tx-internal-delay-ps" property to the > devicetree. > > Furthermore, a warning is now issued if the phy mode is configured to > "rgmii" and an internal delay is set in the phy (e.g. by pin-strapping), > as in the dp83867 driver. > > Signed-off-by: Martin Schiller <ms@dev.tdt.de> Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Thanks.
diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c index d453ec016168..bc7e2fdb8ea7 100644 --- a/drivers/net/phy/intel-xway.c +++ b/drivers/net/phy/intel-xway.c @@ -8,11 +8,16 @@ #include <linux/module.h> #include <linux/phy.h> #include <linux/of.h> +#include <linux/bitfield.h> +#define XWAY_MDIO_MIICTRL 0x17 /* mii control */ #define XWAY_MDIO_IMASK 0x19 /* interrupt mask */ #define XWAY_MDIO_ISTAT 0x1A /* interrupt status */ #define XWAY_MDIO_LED 0x1B /* led control */ +#define XWAY_MDIO_MIICTRL_RXSKEW_MASK GENMASK(14, 12) +#define XWAY_MDIO_MIICTRL_TXSKEW_MASK GENMASK(10, 8) + /* bit 15:12 are reserved */ #define XWAY_MDIO_LED_LED3_EN BIT(11) /* Enable the integrated function of LED3 */ #define XWAY_MDIO_LED_LED2_EN BIT(10) /* Enable the integrated function of LED2 */ @@ -157,6 +162,82 @@ #define PHY_ID_PHY11G_VR9_1_2 0xD565A409 #define PHY_ID_PHY22F_VR9_1_2 0xD565A419 +#if IS_ENABLED(CONFIG_OF_MDIO) +static const int xway_internal_delay[] = {0, 500, 1000, 1500, 2000, 2500, + 3000, 3500}; + +static int xway_gphy_of_reg_init(struct phy_device *phydev) +{ + struct device *dev = &phydev->mdio.dev; + unsigned int delay_size = ARRAY_SIZE(xway_internal_delay); + s32 int_delay; + int val = 0; + + if (!phy_interface_is_rgmii(phydev)) + return 0; + + /* Existing behavior was to use default pin strapping delay in rgmii + * mode, but rgmii should have meant no delay. Warn existing users, + * but do not change anything at the moment. + */ + if (phydev->interface == PHY_INTERFACE_MODE_RGMII) { + u16 txskew, rxskew; + + val = phy_read(phydev, XWAY_MDIO_MIICTRL); + if (val < 0) + return val; + + txskew = FIELD_GET(XWAY_MDIO_MIICTRL_TXSKEW_MASK, val); + rxskew = FIELD_GET(XWAY_MDIO_MIICTRL_RXSKEW_MASK, val); + + if (txskew > 0 || rxskew > 0) + phydev_warn(phydev, + "PHY has delays (e.g. via pin strapping), but phy-mode = 'rgmii'\n" + "Should be 'rgmii-id' to use internal delays txskew:%d ps rxskew:%d ps\n", + xway_internal_delay[txskew], + xway_internal_delay[rxskew]); + return 0; + } + + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) { + int_delay = phy_get_internal_delay(phydev, dev, + xway_internal_delay, + delay_size, true); + + if (int_delay < 0) { + phydev_warn(phydev, "rx-internal-delay-ps is missing, use default of 2.0 ns\n"); + int_delay = 4; /* 2000 ps */ + } + + val |= FIELD_PREP(XWAY_MDIO_MIICTRL_RXSKEW_MASK, int_delay); + } + + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) { + int_delay = phy_get_internal_delay(phydev, dev, + xway_internal_delay, + delay_size, false); + + if (int_delay < 0) { + phydev_warn(phydev, "tx-internal-delay-ps is missing, use default of 2.0 ns\n"); + int_delay = 4; /* 2000 ps */ + } + + val |= FIELD_PREP(XWAY_MDIO_MIICTRL_TXSKEW_MASK, int_delay); + } + + return phy_modify(phydev, XWAY_MDIO_MIICTRL, + XWAY_MDIO_MIICTRL_RXSKEW_MASK | + XWAY_MDIO_MIICTRL_TXSKEW_MASK, val); +} +#else +static int xway_gphy_of_reg_init(struct phy_device *phydev) +{ + return 0; +} +#endif /* CONFIG_OF_MDIO */ + static int xway_gphy_config_init(struct phy_device *phydev) { int err; @@ -204,6 +285,10 @@ static int xway_gphy_config_init(struct phy_device *phydev) phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2H, ledxh); phy_write_mmd(phydev, MDIO_MMD_VEND2, XWAY_MMD_LED2L, ledxl); + err = xway_gphy_of_reg_init(phydev); + if (err) + return err; + return 0; }
This adds the possibility to configure the RGMII RX/TX clock skew via devicetree. Simply set phy mode to "rgmii-id", "rgmii-rxid" or "rgmii-txid" and add the "rx-internal-delay-ps" or "tx-internal-delay-ps" property to the devicetree. Furthermore, a warning is now issued if the phy mode is configured to "rgmii" and an internal delay is set in the phy (e.g. by pin-strapping), as in the dp83867 driver. Signed-off-by: Martin Schiller <ms@dev.tdt.de> --- Changes to v4: o Fix Alignment to match open parenthesis Changes to v3: o Fix typo in commit message o use FIELD_PREP() and FIELD_GET() macros o further code cleanups o always mask rxskew AND txskew value in the register value Changes to v2: o Fix missing whitespace in warning. Changes to v1: o code cleanup and use phy_modify(). o use default of 2.0ns if delay property is absent instead of returning an error. --- drivers/net/phy/intel-xway.c | 85 ++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)