[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20250623-byeword-update-v2-20-cf1fc08a2e1f@collabora.com>
Date: Mon, 23 Jun 2025 18:05:48 +0200
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Yury Norov <yury.norov@...il.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Jaehoon Chung <jh80.chung@...sung.com>,
Ulf Hansson <ulf.hansson@...aro.org>, Heiko Stuebner <heiko@...ech.de>,
Shreeya Patel <shreeya.patel@...labora.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Sandy Huang <hjc@...k-chips.com>, Andy Yan <andy.yan@...k-chips.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
Nicolas Frattaroli <frattaroli.nicolas@...il.com>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Shawn Lin <shawn.lin@...k-chips.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof WilczyĆski <kwilczynski@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>, Rob Herring <robh@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Chanwoo Choi <cw00.choi@...sung.com>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>, Qin Jian <qinjian@...lus1.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>
Cc: kernel@...labora.com, linux-kernel@...r.kernel.org,
linux-mmc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-media@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linux-phy@...ts.infradead.org,
linux-sound@...r.kernel.org, netdev@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com, linux-pci@...r.kernel.org,
linux-pm@...r.kernel.org, linux-clk@...r.kernel.org, llvm@...ts.linux.dev,
Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Subject: [PATCH v2 20/20] 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@...labora.com>
---
drivers/phy/rockchip/phy-rockchip-pcie.c | 72 ++++++++++----------------------
1 file changed, 21 insertions(+), 51 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
index bd44af36c67a5a504801275c1b0384d373fe7ec7..d7f994c3bcdf93faffd5c04ed4d0f9d29eaf43cc 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -8,6 +8,7 @@
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/hw_bitfield.h>
#include <linux/io.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
@@ -18,23 +19,14 @@
#include <linux/regmap.h>
#include <linux/reset.h>
-/*
- * The higher 16-bit of this register is used for write protection
- * only if BIT(x + 16) set to 1 the BIT(x) can be written.
- */
-#define HIWORD_UPDATE(val, mask, shift) \
- ((val) << (shift) | (mask) << ((shift) + 16))
#define PHY_MAX_LANE_NUM 4
-#define PHY_CFG_DATA_SHIFT 7
-#define PHY_CFG_ADDR_SHIFT 1
-#define PHY_CFG_DATA_MASK 0xf
-#define PHY_CFG_ADDR_MASK 0x3f
-#define PHY_CFG_RD_MASK 0x3ff
+#define PHY_CFG_DATA_MASK GENMASK(10, 7)
+#define PHY_CFG_ADDR_MASK GENMASK(6, 1)
+#define PHY_CFG_RD_MASK GENMASK(9, 0)
#define PHY_CFG_WR_ENABLE 1
#define PHY_CFG_WR_DISABLE 1
-#define PHY_CFG_WR_SHIFT 0
-#define PHY_CFG_WR_MASK 1
+#define PHY_CFG_WR_MASK BIT(0)
#define PHY_CFG_PLL_LOCK 0x10
#define PHY_CFG_CLK_TEST 0x10
#define PHY_CFG_CLK_SCC 0x12
@@ -49,11 +41,7 @@
#define PHY_LANE_RX_DET_SHIFT 11
#define PHY_LANE_RX_DET_TH 0x1
#define PHY_LANE_IDLE_OFF 0x1
-#define PHY_LANE_IDLE_MASK 0x1
-#define PHY_LANE_IDLE_A_SHIFT 3
-#define PHY_LANE_IDLE_B_SHIFT 4
-#define PHY_LANE_IDLE_C_SHIFT 5
-#define PHY_LANE_IDLE_D_SHIFT 6
+#define PHY_LANE_IDLE_MASK BIT(3)
struct rockchip_pcie_data {
unsigned int pcie_conf;
@@ -100,22 +88,14 @@ static inline void phy_wr_cfg(struct rockchip_pcie_phy *rk_phy,
u32 addr, u32 data)
{
regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
- HIWORD_UPDATE(data,
- PHY_CFG_DATA_MASK,
- PHY_CFG_DATA_SHIFT) |
- HIWORD_UPDATE(addr,
- PHY_CFG_ADDR_MASK,
- PHY_CFG_ADDR_SHIFT));
+ FIELD_PREP_WM16(PHY_CFG_DATA_MASK, data) |
+ FIELD_PREP_WM16(PHY_CFG_ADDR_MASK, addr));
udelay(1);
regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
- HIWORD_UPDATE(PHY_CFG_WR_ENABLE,
- PHY_CFG_WR_MASK,
- PHY_CFG_WR_SHIFT));
+ FIELD_PREP_WM16(PHY_CFG_WR_MASK, PHY_CFG_WR_ENABLE));
udelay(1);
regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
- HIWORD_UPDATE(PHY_CFG_WR_DISABLE,
- PHY_CFG_WR_MASK,
- PHY_CFG_WR_SHIFT));
+ FIELD_PREP_WM16(PHY_CFG_WR_MASK, PHY_CFG_WR_DISABLE));
}
static int rockchip_pcie_phy_power_off(struct phy *phy)
@@ -126,11 +106,9 @@ static int rockchip_pcie_phy_power_off(struct phy *phy)
guard(mutex)(&rk_phy->pcie_mutex);
- regmap_write(rk_phy->reg_base,
- rk_phy->phy_data->pcie_laneoff,
- HIWORD_UPDATE(PHY_LANE_IDLE_OFF,
- PHY_LANE_IDLE_MASK,
- PHY_LANE_IDLE_A_SHIFT + inst->index));
+ regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_laneoff,
+ FIELD_PREP_WM16(PHY_LANE_IDLE_MASK,
+ PHY_LANE_IDLE_OFF) << inst->index);
if (--rk_phy->pwr_cnt) {
return 0;
@@ -140,11 +118,9 @@ static int rockchip_pcie_phy_power_off(struct phy *phy)
if (err) {
dev_err(&phy->dev, "assert phy_rst err %d\n", err);
rk_phy->pwr_cnt++;
- regmap_write(rk_phy->reg_base,
- rk_phy->phy_data->pcie_laneoff,
- HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
- PHY_LANE_IDLE_MASK,
- PHY_LANE_IDLE_A_SHIFT + inst->index));
+ regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_laneoff,
+ FIELD_PREP_WM16(PHY_LANE_IDLE_MASK,
+ !PHY_LANE_IDLE_OFF) << inst->index);
return err;
}
@@ -172,15 +148,11 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
}
regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
- HIWORD_UPDATE(PHY_CFG_PLL_LOCK,
- PHY_CFG_ADDR_MASK,
- PHY_CFG_ADDR_SHIFT));
+ FIELD_PREP_WM16(PHY_CFG_ADDR_MASK, PHY_CFG_PLL_LOCK));
- regmap_write(rk_phy->reg_base,
- rk_phy->phy_data->pcie_laneoff,
- HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
- PHY_LANE_IDLE_MASK,
- PHY_LANE_IDLE_A_SHIFT + inst->index));
+ regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_laneoff,
+ FIELD_PREP_WM16(PHY_LANE_IDLE_MASK,
+ !PHY_LANE_IDLE_OFF) << inst->index);
/*
* No documented timeout value for phy operation below,
@@ -211,9 +183,7 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
}
regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
- HIWORD_UPDATE(PHY_CFG_PLL_LOCK,
- PHY_CFG_ADDR_MASK,
- PHY_CFG_ADDR_SHIFT));
+ FIELD_PREP_WM16(PHY_CFG_ADDR_MASK, PHY_CFG_PLL_LOCK));
err = regmap_read_poll_timeout(rk_phy->reg_base,
rk_phy->phy_data->pcie_status,
--
2.50.0
Powered by blists - more mailing lists