[3/3] net: phy: dp83826: add support for voltage tuning of logical levels
Commit Message
DP83826 offers the possibility to tune the voltage of logical levels
of the MLT-3 encoded data. This is especially interesting when the
data path is lossy and we want to increase the voltage levels to
compensate the loss.
Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
drivers/net/phy/dp83822.c | 127 ++++++++++++++++++++++++++++++++++++--
1 file changed, 123 insertions(+), 4 deletions(-)
Comments
> +u8 dp83826_cfg_dac_minus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x38, 0x37, 0x36, 0x35, 0x34, 0x33,
> + 0x32, 0x31, 0x30, 0x2f, 0x2e, 0x2d,
> + 0x2c, 0x2b, 0x2a, 0x29, 0x28};
> +u8 dp83826_cfg_dac_plus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
> + 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
> + 0x14, 0x15, 0x16, 0x17, 0x18};
Both of these should be static const.
However, they appear pointless. Plus is just a shift. minus is some
simple arithmetic and a shift.
Andrew
On 11.01.24 17:45, Andrew Lunn wrote:
> [You don't often get email from andrew@lunn.ch. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
>> +u8 dp83826_cfg_dac_minus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x38, 0x37, 0x36, 0x35, 0x34, 0x33,
>> + 0x32, 0x31, 0x30, 0x2f, 0x2e, 0x2d,
>> + 0x2c, 0x2b, 0x2a, 0x29, 0x28};
>> +u8 dp83826_cfg_dac_plus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
>> + 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
>> + 0x14, 0x15, 0x16, 0x17, 0x18};
> Both of these should be static const.
Indeed.
>
> However, they appear pointless. Plus is just a shift. minus is some
> simple arithmetic and a shift.
Well, I found it more clear to use a bit of memory than adding
instructions to compute some raw values from other raw values used as
reference.
>
> Andrew
Hi Catalin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on net-next/main net/main linus/master v6.7 next-20240112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Catalin-Popescu/dt-bindings-net-dp83826-add-ti-cfg-dac-plus-binding/20240112-002701
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20240111161927.3689084-3-catalin.popescu%40leica-geosystems.com
patch subject: [PATCH 3/3] net: phy: dp83826: add support for voltage tuning of logical levels
config: x86_64-randconfig-121-20240112 (https://download.01.org/0day-ci/archive/20240113/202401131120.vAXFTd3t-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240113/202401131120.vAXFTd3t-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401131120.vAXFTd3t-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/dp83822.c:570:4: sparse: sparse: symbol 'dp83826_cfg_dac_minus_raw' was not declared. Should it be static?
>> drivers/net/phy/dp83822.c:573:4: sparse: sparse: symbol 'dp83826_cfg_dac_plus_raw' was not declared. Should it be static?
vim +/dp83826_cfg_dac_minus_raw +570 drivers/net/phy/dp83822.c
569
> 570 u8 dp83826_cfg_dac_minus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x38, 0x37, 0x36, 0x35, 0x34, 0x33,
571 0x32, 0x31, 0x30, 0x2f, 0x2e, 0x2d,
572 0x2c, 0x2b, 0x2a, 0x29, 0x28};
> 573 u8 dp83826_cfg_dac_plus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
574 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
575 0x14, 0x15, 0x16, 0x17, 0x18};
576
@@ -12,6 +12,7 @@
#include <linux/of.h>
#include <linux/phy.h>
#include <linux/netdevice.h>
+#include <linux/bitfield.h>
#define DP83822_PHY_ID 0x2000a240
#define DP83825S_PHY_ID 0x2000a140
@@ -34,6 +35,10 @@
#define MII_DP83822_GENCFG 0x465
#define MII_DP83822_SOR1 0x467
+/* DP83826 specific registers */
+#define MII_DP83826_VOD_CFG1 0x30b
+#define MII_DP83826_VOD_CFG2 0x30c
+
/* GENCFG */
#define DP83822_SIG_DET_LOW BIT(0)
@@ -110,6 +115,16 @@
#define DP83822_RX_ER_STR_MASK GENMASK(9, 8)
#define DP83822_RX_ER_SHIFT 8
+/* DP83826: VOD_CFG1 & VOD_CFG2 */
+#define DP83826_VOD_CFG1_MINUS_MDIX_MASK GENMASK(13, 12)
+#define DP83826_VOD_CFG1_MINUS_MDI_MASK GENMASK(11, 6)
+#define DP83826_VOD_CFG2_MINUS_MDIX_MASK GENMASK(15, 12)
+#define DP83826_VOD_CFG2_PLUS_MDIX_MASK GENMASK(11, 6)
+#define DP83826_VOD_CFG2_PLUS_MDI_MASK GENMASK(5, 0)
+#define DP83826_CFG_DAC_MINUS_MDIX_5_TO_4 GENMASK(5, 4)
+#define DP83826_CFG_DAC_MINUS_MDIX_3_TO_0 GENMASK(3, 0)
+#define DP83826_CFG_DAC_RAW_VALUES_MAX 17
+
#define MII_DP83822_FIBER_ADVERTISE (ADVERTISED_TP | ADVERTISED_MII | \
ADVERTISED_FIBRE | \
ADVERTISED_Pause | ADVERTISED_Asym_Pause)
@@ -118,6 +133,8 @@ struct dp83822_private {
bool fx_signal_det_low;
int fx_enabled;
u16 fx_sd_enable;
+ u8 cfg_dac_minus;
+ u8 cfg_dac_plus;
};
static int dp83822_set_wol(struct phy_device *phydev,
@@ -233,7 +250,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
DP83822_ENERGY_DET_INT_EN |
DP83822_LINK_QUAL_INT_EN);
- /* Private data pointer is NULL on DP83825/26 */
+ /* Private data pointer is NULL on DP83825 */
if (!dp83822 || !dp83822->fx_enabled)
misr_status |= DP83822_ANEG_COMPLETE_INT_EN |
DP83822_DUP_MODE_CHANGE_INT_EN |
@@ -254,7 +271,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
DP83822_PAGE_RX_INT_EN |
DP83822_EEE_ERROR_CHANGE_INT_EN);
- /* Private data pointer is NULL on DP83825/26 */
+ /* Private data pointer is NULL on DP83825 */
if (!dp83822 || !dp83822->fx_enabled)
misr_status |= DP83822_ANEG_ERR_INT_EN |
DP83822_WOL_PKT_INT_EN;
@@ -474,6 +491,46 @@ static int dp83822_config_init(struct phy_device *phydev)
return dp8382x_disable_wol(phydev);
}
+static int dp83826_config_init(struct phy_device *phydev)
+{
+ struct dp83822_private *dp83822 = phydev->priv;
+ u16 vod_cfg1_val = 0, vod_cfg1_msk = 0, vod_cfg2_val = 0, vod_cfg2_msk = 0;
+ int ret;
+
+ if (dp83822->cfg_dac_plus) {
+ vod_cfg2_val = FIELD_PREP(DP83826_VOD_CFG2_PLUS_MDIX_MASK, dp83822->cfg_dac_plus) |
+ FIELD_PREP(DP83826_VOD_CFG2_PLUS_MDI_MASK, dp83822->cfg_dac_plus);
+ vod_cfg2_msk = DP83826_VOD_CFG2_PLUS_MDIX_MASK | DP83826_VOD_CFG2_PLUS_MDI_MASK;
+ }
+
+ if (dp83822->cfg_dac_minus) {
+ vod_cfg2_val |= FIELD_PREP(DP83826_VOD_CFG2_MINUS_MDIX_MASK,
+ FIELD_GET(DP83826_CFG_DAC_MINUS_MDIX_3_TO_0,
+ dp83822->cfg_dac_minus));
+ vod_cfg2_msk |= DP83826_VOD_CFG2_MINUS_MDIX_MASK;
+ vod_cfg1_val = FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDI_MASK, dp83822->cfg_dac_minus) |
+ FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDIX_MASK,
+ FIELD_GET(DP83826_CFG_DAC_MINUS_MDIX_5_TO_4,
+ dp83822->cfg_dac_minus));
+ vod_cfg1_msk = DP83826_VOD_CFG1_MINUS_MDIX_MASK | DP83826_VOD_CFG1_MINUS_MDI_MASK;
+ }
+
+ if (vod_cfg1_msk) {
+ ret = phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83826_VOD_CFG1,
+ vod_cfg1_msk, vod_cfg1_val);
+ if (ret)
+ return ret;
+ }
+ if (vod_cfg2_msk) {
+ ret = phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83826_VOD_CFG2,
+ vod_cfg2_msk, vod_cfg2_val);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int dp8382x_config_init(struct phy_device *phydev)
{
return dp8382x_disable_wol(phydev);
@@ -509,11 +566,41 @@ static int dp83822_of_init(struct phy_device *phydev)
return 0;
}
+
+u8 dp83826_cfg_dac_minus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x38, 0x37, 0x36, 0x35, 0x34, 0x33,
+ 0x32, 0x31, 0x30, 0x2f, 0x2e, 0x2d,
+ 0x2c, 0x2b, 0x2a, 0x29, 0x28};
+u8 dp83826_cfg_dac_plus_raw[DP83826_CFG_DAC_RAW_VALUES_MAX] = {0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
+ 0x14, 0x15, 0x16, 0x17, 0x18};
+
+static int dp83826_of_init(struct phy_device *phydev)
+{
+ struct dp83822_private *dp83822 = phydev->priv;
+ struct device *dev = &phydev->mdio.dev;
+ u32 val;
+ int ret;
+
+ ret = device_property_read_u32(dev, "ti,cfg-dac-minus", &val);
+ if (!ret && val < DP83826_CFG_DAC_RAW_VALUES_MAX)
+ dp83822->cfg_dac_minus = dp83826_cfg_dac_minus_raw[val];
+
+ ret = device_property_read_u32(dev, "ti,cfg-dac-plus", &val);
+ if (!ret && val < DP83826_CFG_DAC_RAW_VALUES_MAX)
+ dp83822->cfg_dac_plus = dp83826_cfg_dac_plus_raw[val];
+
+ return 0;
+}
#else
static int dp83822_of_init(struct phy_device *phydev)
{
return 0;
}
+
+static int dp83826_of_init(struct phy_device *phydev)
+{
+ return 0;
+}
#endif /* CONFIG_OF_MDIO */
static int dp83822_read_straps(struct phy_device *phydev)
@@ -567,6 +654,22 @@ static int dp83822_probe(struct phy_device *phydev)
return 0;
}
+static int dp83826_probe(struct phy_device *phydev)
+{
+ struct dp83822_private *dp83822;
+
+ dp83822 = devm_kzalloc(&phydev->mdio.dev, sizeof(*dp83822),
+ GFP_KERNEL);
+ if (!dp83822)
+ return -ENOMEM;
+
+ phydev->priv = dp83822;
+
+ dp83826_of_init(phydev);
+
+ return 0;
+}
+
static int dp83822_suspend(struct phy_device *phydev)
{
int value;
@@ -610,6 +713,22 @@ static int dp83822_resume(struct phy_device *phydev)
.resume = dp83822_resume, \
}
+#define DP83826_PHY_DRIVER(_id, _name) \
+ { \
+ PHY_ID_MATCH_MODEL(_id), \
+ .name = (_name), \
+ /* PHY_BASIC_FEATURES */ \
+ .probe = dp83826_probe, \
+ .soft_reset = dp83822_phy_reset, \
+ .config_init = dp83826_config_init, \
+ .get_wol = dp83822_get_wol, \
+ .set_wol = dp83822_set_wol, \
+ .config_intr = dp83822_config_intr, \
+ .handle_interrupt = dp83822_handle_interrupt, \
+ .suspend = dp83822_suspend, \
+ .resume = dp83822_resume, \
+ }
+
#define DP8382X_PHY_DRIVER(_id, _name) \
{ \
PHY_ID_MATCH_MODEL(_id), \
@@ -628,8 +747,8 @@ static int dp83822_resume(struct phy_device *phydev)
static struct phy_driver dp83822_driver[] = {
DP83822_PHY_DRIVER(DP83822_PHY_ID, "TI DP83822"),
DP8382X_PHY_DRIVER(DP83825I_PHY_ID, "TI DP83825I"),
- DP8382X_PHY_DRIVER(DP83826C_PHY_ID, "TI DP83826C"),
- DP8382X_PHY_DRIVER(DP83826NC_PHY_ID, "TI DP83826NC"),
+ DP83826_PHY_DRIVER(DP83826C_PHY_ID, "TI DP83826C"),
+ DP83826_PHY_DRIVER(DP83826NC_PHY_ID, "TI DP83826NC"),
DP8382X_PHY_DRIVER(DP83825S_PHY_ID, "TI DP83825S"),
DP8382X_PHY_DRIVER(DP83825CM_PHY_ID, "TI DP83825M"),
DP8382X_PHY_DRIVER(DP83825CS_PHY_ID, "TI DP83825CS"),