Re: [PATCH phy v2 2/6] phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation
From: Vinod Koul <vkoul@kernel.org>
Date: 2021-12-23 12:41:01
On 08-12-21, 03:40, Marek Behún wrote:
From: Pali Rohár <pali@kernel.org>
+/* COMPHY registers */ +#define COMPHY_POWER_PLL_CTRL 0x01 +#define PU_IVREF_BIT BIT(15) +#define PU_PLL_BIT BIT(14) +#define PU_RX_BIT BIT(13) +#define PU_TX_BIT BIT(12) +#define PU_TX_INTP_BIT BIT(11) +#define PU_DFE_BIT BIT(10) +#define RESET_DTL_RX_BIT BIT(9) +#define PLL_LOCK_BIT BIT(8) +#define REF_FREF_SEL_MASK GENMASK(4, 0) +#define REF_FREF_SEL_SERDES_25MHZ (0x1 << 0) +#define REF_FREF_SEL_SERDES_40MHZ (0x3 << 0) +#define REF_FREF_SEL_SERDES_50MHZ (0x4 << 0) +#define REF_FREF_SEL_PCIE_USB3_25MHZ (0x2 << 0) +#define REF_FREF_SEL_PCIE_USB3_40MHZ (0x3 << 0) +#define COMPHY_MODE_MASK GENMASK(7, 5) +#define COMPHY_MODE_SATA (0x0 << 5) +#define COMPHY_MODE_PCIE (0x3 << 5) +#define COMPHY_MODE_SERDES (0x4 << 5) +#define COMPHY_MODE_USB3 (0x5 << 5)
Any reason why these are not using GENMASK. I guess documentation would define these as BIT x-y right?
+/* Changes to 40M1G25 mode data required for running 40M3G125 init mode */
+static struct gbe_phy_init_data_fix gbe_phy_init_fix[] = {
+ {0x005, 0x07CC}, {0x015, 0x0000}, {0x01B, 0x0000}, {0x01D, 0x0000},
+ {0x01E, 0x0000}, {0x01F, 0x0000}, {0x020, 0x0000}, {0x021, 0x0030},
+ {0x026, 0x0888}, {0x04D, 0x0152}, {0x04F, 0xA020}, {0x050, 0x07CC},
+ {0x053, 0xE9CA}, {0x055, 0xBD97}, {0x071, 0x3015}, {0x076, 0x03AA},
+ {0x07C, 0x0FDF}, {0x0C2, 0x3030}, {0x0C3, 0x8000}, {0x0E2, 0x5550},
+ {0x0E3, 0x12A4}, {0x0E4, 0x7D00}, {0x0E6, 0x0C83}, {0x101, 0xFCC0},
+ {0x104, 0x0C10}
empty space after { and before } makes it look neater and it typically
the convention followed at many places
+/* PHY selector configures with corresponding modes */
+static int
+mvebu_a3700_comphy_set_phy_selector(struct mvebu_a3700_comphy_lane *lane)
+{
+ u32 old, new, clr = 0, set = 0;
+ unsigned long flags;
+
+ switch (lane->mode) {
+ case PHY_MODE_SATA:
+ /* SATA must be in Lane2 */
+ if (lane->id == 2)
+ clr = COMPHY_SELECTOR_USB3_PHY_SEL_BIT;
+ else
+ goto error;
+ break;
+
+ case PHY_MODE_ETHERNET:
+ if (lane->id == 0)
+ clr = COMPHY_SELECTOR_USB3_GBE1_SEL_BIT;
+ else if (lane->id == 1)
+ clr = COMPHY_SELECTOR_PCIE_GBE0_SEL_BIT;
+ else
+ goto error;
+ break;
+
+ case PHY_MODE_USB_HOST_SS:
+ if (lane->id == 2)
+ set = COMPHY_SELECTOR_USB3_PHY_SEL_BIT;
+ else if (lane->id == 0)
+ set = COMPHY_SELECTOR_USB3_GBE1_SEL_BIT;
+ else
+ goto error;
+ break;
+
+ case PHY_MODE_PCIE:
+ /* PCIE must be in Lane1 */
+ if (lane->id == 1)
+ set = COMPHY_SELECTOR_PCIE_GBE0_SEL_BIT;
+ else
+ goto error;
+ break;
+
+ default:
+ goto error;
+ }
+
+ raw_spin_lock_irqsave(&lane->priv->lock, flags);
+
+ old = readl(lane->priv->comphy_regs + COMPHY_SELECTOR_PHY_REG);
+ new = (old & ~clr) | set;
+ writel(new, lane->priv->comphy_regs + COMPHY_SELECTOR_PHY_REG);
+
+ raw_spin_unlock_irqrestore(&lane->priv->lock, flags);any reason to use raw_ variant? why not spin_lock_irqsave() -- ~Vinod -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy