lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 27 May 2024 05:37:47 +0000
From: Sai Krishna Gajula <saikrishnag@...vell.com>
To: Ramón Nordin Rodriguez
	<ramon.nordin.rodriguez@...roamp.se>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "linux@...linux.org.uk"
	<linux@...linux.org.uk>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org"
	<kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "parthiban.veerasooran@...rochip.com"
	<parthiban.veerasooran@...rochip.com>
Subject: RE: [EXTERNAL] [PATCH 1/1] net: phy: microchip_t1s: enable lan865x
 revb1


> -----Original Message-----
> From: Ramón Nordin Rodriguez <ramon.nordin.rodriguez@...roamp.se>
> Sent: Friday, May 24, 2024 7:37 PM
> To: andrew@...n.ch; hkallweit1@...il.com; linux@...linux.org.uk;
> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; netdev@...r.kernel.org; linux-kernel@...r.kernel.org
> Cc: parthiban.veerasooran@...rochip.com; Ramón Nordin Rodriguez
> <ramon.nordin.rodriguez@...roamp.se>
> Subject: [EXTERNAL] [PATCH 1/1] net: phy: microchip_t1s: enable lan865x
> revb1
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> The microchip_t1s module is extended with support for lan865x rev.b1, prior
> to this commit support for rev.b0 already exists.
> 
> Some of the operations performed in the hw probe and init of the phy require
> access to registers only accessible via the macphy spi interface (lan865x is an
> integrated phy), meaning the init call has to interop with layers above, namely
> via read/write_c45.
> 
> Signed-off-by: Ramón Nordin Rodriguez
> <ramon.nordin.rodriguez@...roamp.se>

Fixes tag is required if the patch is for NET tree. Also, it seems like build for 32 bit seems to be failing. Please check.

> ---
>  drivers/net/phy/microchip_t1s.c | 189 ++++++++++++++++++++++++++++-
> ---
>  1 file changed, 166 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/phy/microchip_t1s.c
> b/drivers/net/phy/microchip_t1s.c index 534ca7d1b061..3c5153aa5c7a
> 100644
> --- a/drivers/net/phy/microchip_t1s.c
> +++ b/drivers/net/phy/microchip_t1s.c
> @@ -5,6 +5,7 @@
>   * Support: Microchip Phys:
>   *  lan8670/1/2 Rev.B1
>   *  lan8650/1 Rev.B0 Internal PHYs
> + *  lan8650/1 Rev.B1 Internal PHYs
>   */
> 
>  #include <linux/kernel.h>
> @@ -12,7 +13,7 @@
>  #include <linux/phy.h>
> 
>  #define PHY_ID_LAN867X_REVB1 0x0007C162 -#define
> PHY_ID_LAN865X_REVB0 0x0007C1B3
> +#define PHY_ID_LAN865X 0x0007C1B3
> 
>  #define LAN867X_REG_STS2 0x0019
> 
> @@ -22,6 +23,7 @@
>  #define LAN865X_REG_CFGPARAM_DATA 0x00D9  #define
> LAN865X_REG_CFGPARAM_CTRL 0x00DA  #define LAN865X_REG_STS2
> 0x0019
> +#define LAN865X_REG_DEVID 0x94
> 
>  #define LAN865X_CFGPARAM_READ_ENABLE BIT(1)
> 
> @@ -84,6 +86,65 @@ static const u16 lan865x_revb0_fixup_cfg_regs[5] = {
>  	0x0084, 0x008A, 0x00AD, 0x00AE, 0x00AF  };
> 
> +enum lan865x_revision {
> +	REV_B0 = 0x1,
> +	REV_B1 = 0x2,
> +};
> +
> +struct lan865x_revb1_fixup_op {
> +	u16 mms;
> +	u16 addr;
> +	u16 value;
> +};
> +
> +static const struct lan865x_revb1_fixup_op lan865x_revb1_fixup_cfg[20]
> += {
> +	{.mms = 4, .addr = 0x00d0, .value = 0x3f31},
> +	{.mms = 4, .addr = 0x00e0, .value = 0xc000},
> +	{.mms = 4, .addr = 0x0084, .value = 0x0000},
> +	{.mms = 4, .addr = 0x008a, .value = 0x0000},
> +	{.mms = 4, .addr = 0x00e9, .value = 0x9e50},
> +	{.mms = 4, .addr = 0x00f5, .value = 0x1cf8},
> +	{.mms = 4, .addr = 0x00f4, .value = 0xc020},
> +	{.mms = 4, .addr = 0x00f8, .value = 0x9b00},
> +	{.mms = 4, .addr = 0x00f9, .value = 0x4e53},
> +	{.mms = 4, .addr = 0x0081, .value = 0x0080},
> +	{.mms = 4, .addr = 0x0091, .value = 0x9660},
> +	{.mms = 1, .addr = 0x0077, .value = 0x0028},
> +	{.mms = 4, .addr = 0x0043, .value = 0x00ff},
> +	{.mms = 4, .addr = 0x0044, .value = 0xffff},
> +	{.mms = 4, .addr = 0x0045, .value = 0x0000},
> +	{.mms = 4, .addr = 0x0053, .value = 0x00ff},
> +	{.mms = 4, .addr = 0x0054, .value = 0xffff},
> +	{.mms = 4, .addr = 0x0055, .value = 0x0000},
> +	{.mms = 4, .addr = 0x0040, .value = 0x0002},
> +	{.mms = 4, .addr = 0x0050, .value = 0x0002} };
> +
> +/* this func is copy pasted from Parthibans ongoing work with oa_tc6
> + * see
> +https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_ne
> +tdev_20240418125648.372526-2D7-2DParthiban.Veerasooran-
> 40microchip.com_
> +&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=c3MsgrR-U-
> HFhmFd6R4MWRZG-8QeikJn5P
> +kjqMTpBSg&m=XjkUpKCoT2lM06i5fF3KdqFf-notCV0B-
> LT0mcLVMbPDQ29Vs7R3Ek2zHA7
> +0BlwP&s=rNsk2zP8m89VjgtwQo3jL-STsPKU5S7Ig8TGr0IsZTY&e=
> + */
> +static int lan865x_phy_read_mmd(struct phy_device *phydev, int devnum,
> +				u16 regnum)
> +{
> +	struct mii_bus *bus = phydev->mdio.bus;
> +	int addr = phydev->mdio.addr;
> +
> +	return bus->read_c45(bus, addr, devnum, regnum); }
> +
> +/* this func is copy pasted from Parthibans ongoing work with oa_tc6
> + * see
> +https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_ne
> +tdev_20240418125648.372526-2D7-2DParthiban.Veerasooran-
> 40microchip.com_
> +&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=c3MsgrR-U-
> HFhmFd6R4MWRZG-8QeikJn5P
> +kjqMTpBSg&m=XjkUpKCoT2lM06i5fF3KdqFf-notCV0B-
> LT0mcLVMbPDQ29Vs7R3Ek2zHA7
> +0BlwP&s=rNsk2zP8m89VjgtwQo3jL-STsPKU5S7Ig8TGr0IsZTY&e=
> + */
> +static int lan865x_phy_write_mmd(struct phy_device *phydev, int devnum,
> +				 u16 regnum, u16 val)
> +{
> +	struct mii_bus *bus = phydev->mdio.bus;
> +	int addr = phydev->mdio.addr;
> +
> +	return bus->write_c45(bus, addr, devnum, regnum, val); }
> +
>  /* Pulled from AN1760 describing 'indirect read'
>   *
>   * write_register(0x4, 0x00D8, addr)
> @@ -112,7 +173,7 @@ static int lan865x_revb0_indirect_read(struct
> phy_device *phydev, u16 addr)
>  /* This is pulled straight from AN1760 from 'calculation of offset 1' &
>   * 'calculation of offset 2'
>   */
> -static int lan865x_generate_cfg_offsets(struct phy_device *phydev, s8
> offsets[2])
> +static int lan865x_revb0_generate_cfg_offsets(struct phy_device
> +*phydev, s8 offsets[2])
>  {
>  	const u16 fixup_regs[2] = {0x0004, 0x0008};
>  	int ret;
> @@ -130,7 +191,41 @@ static int lan865x_generate_cfg_offsets(struct
> phy_device *phydev, s8 offsets[2]
>  	return 0;
>  }
> 
> -static int lan865x_read_cfg_params(struct phy_device *phydev, u16
> cfg_params[])
> +static int lan865x_revb1_gen_cfgparams(struct phy_device *phydev, u16
> +params[2]) {
> +	const u16 fixup_regs[2] = {0x0004, 0x0008};
> +	int ret;
> +
> +	// this loop attempts to de-weirdify the method described in AN1760
> +	for (int i = 0; i < ARRAY_SIZE(fixup_regs); i++) {
> +		// only the lower 8 bits of the results here are used
> +		ret = lan865x_revb0_indirect_read(phydev, fixup_regs[i]);
> +		if (ret < 0)
> +			return ret;
> +		// The AN states that the readback value is in the range [-5,
> 15]
> +		// and that it uses 5 bits, bit wonky of a description, it really
> only
> +		// makes sense if the read is sign extended.
> +		// Since the regs are part of the reserved range there is no way
> +		// of telling how this really works though.
> +		if (ret & BIT(4))
> +			// The AN specifies 'ret - 0x20' which is again pretty
> weird
> +			// the addition here gives the same result on a s8
> (with overflow)
> +			// the assignment here are downcasting to s8 for sign
> extension
> +			params[i] = (s8)(ret + 0xE0);
> +		else
> +			params[i] = (s8)ret;
> +	}
> +
> +	// And now for the really weird part, there's no explanation to what
> any of this
> +	// means, just that we need these results written to magic regs.
> +	// Since these ops shift a lot, the previous sign extension is probably
> meaningless.
> +	params[0] = (((params[0] + 9) & 0x3F) << 10) | ((params[0] + 14) &
> 0x3f) << 4 | 0x3;
> +	params[1] = ((params[1] + 40) & 0x3F) << 10;
> +
> +	return 0;
> +}
> +
> +static int lan865x_revb0_read_cfg_params(struct phy_device *phydev, u16
> +cfg_params[])
>  {
>  	int ret;
> 
> @@ -145,7 +240,7 @@ static int lan865x_read_cfg_params(struct
> phy_device *phydev, u16 cfg_params[])
>  	return 0;
>  }
> 
> -static int lan865x_write_cfg_params(struct phy_device *phydev, u16
> cfg_params[])
> +static int lan865x_revb0_write_cfg_params(struct phy_device *phydev,
> +u16 cfg_params[])
>  {
>  	int ret;
> 
> @@ -160,18 +255,29 @@ static int lan865x_write_cfg_params(struct
> phy_device *phydev, u16 cfg_params[])
>  	return 0;
>  }
> 
> -static int lan865x_setup_cfgparam(struct phy_device *phydev)
> +static int lan865x_revb0_setup_cfgparam(struct phy_device *phydev)
>  {
>  	u16 cfg_params[ARRAY_SIZE(lan865x_revb0_fixup_cfg_regs)];
>  	u16 cfg_results[5];
>  	s8 offsets[2];
>  	int ret;
> 
> -	ret = lan865x_generate_cfg_offsets(phydev, offsets);
> +	/* Reference to AN1760
> +	 * https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__ww1.microchip.com_downloads_aemDocuments_documents_AIS_Prod
> uctDocuments_SupportingCollateral_AN-2DLAN8650-2D1-2DConfiguration-
> 2D60001760.pdf&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=c3MsgrR-U-
> HFhmFd6R4MWRZG-8QeikJn5PkjqMTpBSg&m=XjkUpKCoT2lM06i5fF3KdqFf-
> notCV0B-
> LT0mcLVMbPDQ29Vs7R3Ek2zHA70BlwP&s=9lbQcePQzdtdyNZYMB_GAniwOj
> qhUp3KA0VBMaGmi6M&e=
> +	 */
> +	for (int i = 0; i < ARRAY_SIZE(lan865x_revb0_fixup_registers); i++) {
> +		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
> +				    lan865x_revb0_fixup_registers[i],
> +				    lan865x_revb0_fixup_values[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = lan865x_revb0_generate_cfg_offsets(phydev, offsets);
>  	if (ret)
>  		return ret;
> 
> -	ret = lan865x_read_cfg_params(phydev, cfg_params);
> +	ret = lan865x_revb0_read_cfg_params(phydev, cfg_params);
>  	if (ret)
>  		return ret;
> 
> @@ -190,27 +296,64 @@ static int lan865x_setup_cfgparam(struct
> phy_device *phydev)
>  			  FIELD_PREP(GENMASK(15, 8), 17 + offsets[0]) |
>  			  (22 + offsets[0]);
> 
> -	return lan865x_write_cfg_params(phydev, cfg_results);
> +	return lan865x_revb0_write_cfg_params(phydev, cfg_results);
>  }
> 
> -static int lan865x_revb0_config_init(struct phy_device *phydev)
> +static int lan865x_revb1_setup_cfgparam(struct phy_device *phydev)
>  {
> +	const struct lan865x_revb1_fixup_op *op;
> +	u16 generated_params[2];
> +	u16 fixup_val;
>  	int ret;
> 
> -	/* Reference to AN1760
> -	 * https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__ww1.microchip.com_downloads_aemDocuments_documents_AIS_Prod
> uctDocuments_SupportingCollateral_AN-2DLAN8650-2D1-2DConfiguration-
> 2D60001760.pdf&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=c3MsgrR-U-
> HFhmFd6R4MWRZG-8QeikJn5PkjqMTpBSg&m=XjkUpKCoT2lM06i5fF3KdqFf-
> notCV0B-
> LT0mcLVMbPDQ29Vs7R3Ek2zHA70BlwP&s=9lbQcePQzdtdyNZYMB_GAniwOj
> qhUp3KA0VBMaGmi6M&e=
> -	 */
> -	for (int i = 0; i < ARRAY_SIZE(lan865x_revb0_fixup_registers); i++) {
> -		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
> -				    lan865x_revb0_fixup_registers[i],
> -				    lan865x_revb0_fixup_values[i]);
> +	ret = lan865x_revb1_gen_cfgparams(phydev, generated_params);
> +	if (ret)
> +		return ret;
> +
> +	for (int i = 0; i < ARRAY_SIZE(lan865x_revb1_fixup_cfg); i++) {
> +		op = &lan865x_revb1_fixup_cfg[i];
> +		fixup_val = op->value;
> +		if (i == 2)
> +			fixup_val = generated_params[0];
> +		else if (i == 3)
> +			fixup_val = generated_params[1];
> +		/* All of the regs locatd in mms 4 could use phy_write_mmd
> as these are
> +		 * accessible via MDIO_MMD_VEND2, but mms 1 not 'indirect
> accessible'.
> +		 * Less conditionals are favored here by doing the set in just
> one way.
> +		 */
> +		ret = lan865x_phy_write_mmd(phydev, op->mms | BIT(31),
> op->addr,
> +fixup_val);
>  		if (ret)
>  			return ret;
>  	}
> -	/* Function to calculate and write the configuration parameters in the
> -	 * 0x0084, 0x008A, 0x00AD, 0x00AE and 0x00AF registers (from
> AN1760)
> +	return 0;
> +}
> +
> +static int lan865x_config_init(struct phy_device *phydev) {
> +	u32 rev_mask = GENMASK(1, 0);
> +	enum lan865x_revision rev;
> +	u32 mms10 = 10;
> +	u32 devid;
> +
> +	/* The standard reg OA_PHYID (mms 0, addr 0x1) does not contain
> any
> +	 * information that distinguishes hardware revisions.
> +	 * HW rev can be read from the vendor specific DEVID reg mms 10,
> addr
> +0x94
>  	 */
> -	return lan865x_setup_cfgparam(phydev);
> +	devid = lan865x_phy_read_mmd(phydev, mms10 | BIT(31),
> +LAN865X_REG_DEVID);
> +
> +	if (devid < 0)
> +		return devid;
> +
> +	rev = devid & rev_mask;
> +	switch (rev) {
> +	case REV_B0:
> +		return lan865x_revb0_setup_cfgparam(phydev);
> +	case REV_B1:
> +		return lan865x_revb1_setup_cfgparam(phydev);
> +	}
> +
> +	dev_err(&phydev->mdio.dev, "unsupported chip rev: 0x%x", rev);
> +	return -ENODEV;
>  }
> 
>  static int lan867x_revb1_config_init(struct phy_device *phydev) @@ -280,10
> +423,10 @@ static struct phy_driver microchip_t1s_driver[] = {
>  		.get_plca_status    = genphy_c45_plca_get_status,
>  	},
>  	{
> -		PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB0),
> -		.name               = "LAN865X Rev.B0 Internal Phy",
> +		PHY_ID_MATCH_EXACT(PHY_ID_LAN865X),
> +		.name               = "LAN865X Rev.B0/1 Internal Phy",
>  		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
> -		.config_init        = lan865x_revb0_config_init,
> +		.config_init        = lan865x_config_init,
>  		.read_status        = lan86xx_read_status,
>  		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
>  		.set_plca_cfg	    = genphy_c45_plca_set_cfg,
> @@ -295,7 +438,7 @@ module_phy_driver(microchip_t1s_driver);
> 
>  static struct mdio_device_id __maybe_unused tbl[] = {
>  	{ PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1) },
> -	{ PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB0) },
> +	{ PHY_ID_MATCH_EXACT(PHY_ID_LAN865X) },
>  	{ }
>  };
> 
> --
> 2.43.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ