RE: [EXTERNAL] [PATCH 1/1] net: phy: microchip_t1s: enable lan865x revb1

From: Sai Krishna Gajula
Date: Mon May 27 2024 - 01:38:35 EST

Next message: Sumit Garg: "[PATCH RESEND2 v5 0/3] arm64: dts: qcom: apq8016: Add Schneider HMIBSC board DTS"
Previous message: Tariq Toukan: "Re: [PATCH 3/4] net: ethernet: mlx4: remove unused struct 'mlx4_port_config'"
In reply to: Russell King (Oracle): "Re: [PATCH 1/1] net: phy: microchip_t1s: enable lan865x revb1"
Next in thread: Andrew Lunn: "Re: [PATCH net 0/1] phy: microchip_t1s: lan865x rev.b1 support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> -----Original Message-----
> From: Ramón Nordin Rodriguez <ramon.nordin.rodriguez@xxxxxxxxxxx>
> Sent: Friday, May 24, 2024 7:37 PM
> To: andrew@xxxxxxx; hkallweit1@xxxxxxxxx; linux@xxxxxxxxxxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: parthiban.veerasooran@xxxxxxxxxxxxx; Ramón Nordin Rodriguez
> <ramon.nordin.rodriguez@xxxxxxxxxxx>
> 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@xxxxxxxxxxx>

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
>

Next message: Sumit Garg: "[PATCH RESEND2 v5 0/3] arm64: dts: qcom: apq8016: Add Schneider HMIBSC board DTS"
Previous message: Tariq Toukan: "Re: [PATCH 3/4] net: ethernet: mlx4: remove unused struct 'mlx4_port_config'"
In reply to: Russell King (Oracle): "Re: [PATCH 1/1] net: phy: microchip_t1s: enable lan865x revb1"
Next in thread: Andrew Lunn: "Re: [PATCH net 0/1] phy: microchip_t1s: lan865x rev.b1 support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]