phy: rockchip-pcie: switch to FIELD_PREP_WM16 macro

The era of hand-rolled HIWORD_UPDATE macros is over, at least for those
drivers that use constant masks.

The Rockchip PCIe PHY driver, used on the RK3399, has its own definition
of HIWORD_UPDATE.

Remove it, and replace instances of it with hw_bitfield.h's
FIELD_PREP_WM16. To achieve this, some mask defines are reshuffled, as
FIELD_PREP_WM16 uses the mask as both the mask of bits to write and to
derive the shift amount from in order to shift the value.

In order to ensure that the mask is always a constant, the inst->index
shift is performed after the FIELD_PREP_WM16, as this is a runtime
value.

>From this, we gain compile-time error checking, and in my humble opinion
nicer code, as well as a single definition of this macro across the
entire codebase to aid in code comprehension.

Tested on a RK3399 ROCKPro64, where PCIe still works as expected when
accessing an NVMe drive.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>

authored by

Nicolas Frattaroli and committed by

Yury Norov (NVIDIA) 7 months ago 4688bb13 b8b56771

+20 -50

1 changed file

expand all

drivers

phy

rockchip

phy-rockchip-pcie.c

+20 -50

drivers/phy/rockchip/phy-rockchip-pcie.c

··· 8 8 9 9 #include <linux/clk.h> 10 10 #include <linux/delay.h> 11 + #include <linux/hw_bitfield.h> 11 12 #include <linux/io.h> 12 13 #include <linux/mfd/syscon.h> 13 14 #include <linux/module.h> ··· 19 18 #include <linux/regmap.h> 20 19 #include <linux/reset.h> 21 20 22 - /* 23 - * The higher 16-bit of this register is used for write protection 24 - * only if BIT(x + 16) set to 1 the BIT(x) can be written. 25 - */ 26 - #define HIWORD_UPDATE(val, mask, shift) \ 27 - ((val) << (shift) | (mask) << ((shift) + 16)) 28 21 29 22 #define PHY_MAX_LANE_NUM 4 30 - #define PHY_CFG_DATA_SHIFT 7 31 - #define PHY_CFG_ADDR_SHIFT 1 32 - #define PHY_CFG_DATA_MASK 0xf 33 - #define PHY_CFG_ADDR_MASK 0x3f 23 + #define PHY_CFG_DATA_MASK GENMASK(10, 7) 24 + #define PHY_CFG_ADDR_MASK GENMASK(6, 1) 34 25 #define PHY_CFG_WR_ENABLE 1 35 26 #define PHY_CFG_WR_DISABLE 0 36 - #define PHY_CFG_WR_SHIFT 0 37 - #define PHY_CFG_WR_MASK 1 27 + #define PHY_CFG_WR_MASK BIT(0) 38 28 #define PHY_CFG_PLL_LOCK 0x10 39 29 #define PHY_CFG_CLK_TEST 0x10 40 30 #define PHY_CFG_CLK_SCC 0x12 ··· 40 48 #define PHY_LANE_RX_DET_SHIFT 11 41 49 #define PHY_LANE_RX_DET_TH 0x1 42 50 #define PHY_LANE_IDLE_OFF 0x1 43 - #define PHY_LANE_IDLE_MASK 0x1 44 - #define PHY_LANE_IDLE_A_SHIFT 3 45 - #define PHY_LANE_IDLE_B_SHIFT 4 46 - #define PHY_LANE_IDLE_C_SHIFT 5 47 - #define PHY_LANE_IDLE_D_SHIFT 6 51 + #define PHY_LANE_IDLE_MASK BIT(3) 48 52 49 53 struct rockchip_pcie_data { 50 54 unsigned int pcie_conf; ··· 87 99 u32 addr, u32 data) 88 100 { 89 101 regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf, 90 - HIWORD_UPDATE(data, 91 - PHY_CFG_DATA_MASK, 92 - PHY_CFG_DATA_SHIFT) | 93 - HIWORD_UPDATE(addr, 94 - PHY_CFG_ADDR_MASK, 95 - PHY_CFG_ADDR_SHIFT)); 102 + FIELD_PREP_WM16(PHY_CFG_DATA_MASK, data) | 103 + FIELD_PREP_WM16(PHY_CFG_ADDR_MASK, addr)); 96 104 udelay(1); 97 105 regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf, 98 - HIWORD_UPDATE(PHY_CFG_WR_ENABLE, 99 - PHY_CFG_WR_MASK, 100 - PHY_CFG_WR_SHIFT)); 106 + FIELD_PREP_WM16(PHY_CFG_WR_MASK, PHY_CFG_WR_ENABLE)); 101 107 udelay(1); 102 108 regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf, 103 - HIWORD_UPDATE(PHY_CFG_WR_DISABLE, 104 - PHY_CFG_WR_MASK, 105 - PHY_CFG_WR_SHIFT)); 109 + FIELD_PREP_WM16(PHY_CFG_WR_MASK, PHY_CFG_WR_DISABLE)); 106 110 } 107 111 108 112 static int rockchip_pcie_phy_power_off(struct phy *phy) ··· 105 125 106 126 guard(mutex)(&rk_phy->pcie_mutex); 107 127 108 - regmap_write(rk_phy->reg_base, 109 - rk_phy->phy_data->pcie_laneoff, 110 - HIWORD_UPDATE(PHY_LANE_IDLE_OFF, 111 - PHY_LANE_IDLE_MASK, 112 - PHY_LANE_IDLE_A_SHIFT + inst->index)); 128 + regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_laneoff, 129 + FIELD_PREP_WM16(PHY_LANE_IDLE_MASK, 130 + PHY_LANE_IDLE_OFF) << inst->index); 113 131 114 132 if (--rk_phy->pwr_cnt) { 115 133 return 0; ··· 117 139 if (err) { 118 140 dev_err(&phy->dev, "assert phy_rst err %d\n", err); 119 141 rk_phy->pwr_cnt++; 120 - regmap_write(rk_phy->reg_base, 121 - rk_phy->phy_data->pcie_laneoff, 122 - HIWORD_UPDATE(!PHY_LANE_IDLE_OFF, 123 - PHY_LANE_IDLE_MASK, 124 - PHY_LANE_IDLE_A_SHIFT + inst->index)); 142 + regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_laneoff, 143 + FIELD_PREP_WM16(PHY_LANE_IDLE_MASK, 144 + !PHY_LANE_IDLE_OFF) << inst->index); 125 145 return err; 126 146 } 127 147 ··· 135 159 136 160 guard(mutex)(&rk_phy->pcie_mutex); 137 161 138 - regmap_write(rk_phy->reg_base, 139 - rk_phy->phy_data->pcie_laneoff, 140 - HIWORD_UPDATE(!PHY_LANE_IDLE_OFF, 141 - PHY_LANE_IDLE_MASK, 142 - PHY_LANE_IDLE_A_SHIFT + inst->index)); 162 + regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_laneoff, 163 + FIELD_PREP_WM16(PHY_LANE_IDLE_MASK, 164 + !PHY_LANE_IDLE_OFF) << inst->index); 143 165 144 166 if (rk_phy->pwr_cnt++) { 145 167 return 0; ··· 151 177 } 152 178 153 179 regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf, 154 - HIWORD_UPDATE(PHY_CFG_PLL_LOCK, 155 - PHY_CFG_ADDR_MASK, 156 - PHY_CFG_ADDR_SHIFT)); 180 + FIELD_PREP_WM16(PHY_CFG_ADDR_MASK, PHY_CFG_PLL_LOCK)); 157 181 158 182 /* 159 183 * No documented timeout value for phy operation below, ··· 182 210 } 183 211 184 212 regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf, 185 - HIWORD_UPDATE(PHY_CFG_PLL_LOCK, 186 - PHY_CFG_ADDR_MASK, 187 - PHY_CFG_ADDR_SHIFT)); 213 + FIELD_PREP_WM16(PHY_CFG_ADDR_MASK, PHY_CFG_PLL_LOCK)); 188 214 189 215 err = regmap_read_poll_timeout(rk_phy->reg_base, 190 216 rk_phy->phy_data->pcie_status,

Configure Feed

Configure Feed