On Tue, Apr 22, 2025 at 07:28:29PM +0800, Hans Zhang wrote: > Register definitions were scattered with ambiguous names (e.g., > PCIE_RDLH_LINK_UP_CHGED in PCIE_CLIENT_INTR_STATUS_MISC) and lacked > hierarchical grouping. Magic values for bit operations reduced code > clarity. > > Group registers and their associated bitfields logically. This improves > maintainability and aligns the code with hardware documentation. > > Signed-off-by: Hans Zhang <18255117159@xxxxxxx> > --- > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 42 +++++++++++-------- > 1 file changed, 24 insertions(+), 18 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > index fd5827bbfae3..cdc8afc6cfc1 100644 > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > @@ -8,6 +8,7 @@ > * Author: Simon Xue <xxm@xxxxxxxxxxxxxx> > */ > > +#include <linux/bitfield.h> > #include <linux/clk.h> > #include <linux/gpio/consumer.h> > #include <linux/irqchip/chained_irq.h> > @@ -34,30 +35,35 @@ > > #define to_rockchip_pcie(x) dev_get_drvdata((x)->dev) > > -#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40) > -#define PCIE_CLIENT_EP_MODE HIWORD_UPDATE(0xf0, 0x0) > -#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc) > -#define PCIE_CLIENT_DISABLE_LTSSM HIWORD_UPDATE(0x0c, 0x8) > -#define PCIE_CLIENT_INTR_STATUS_MSG_RX 0x04 > +#define PCIE_CLIENT_GENERAL_CONTROL 0x0 > +#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40) > +#define PCIE_CLIENT_EP_MODE HIWORD_UPDATE(0xf0, 0x0) > +#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc) > +#define PCIE_CLIENT_DISABLE_LTSSM HIWORD_UPDATE(0x0c, 0x8) > + > +#define PCIE_CLIENT_INTR_STATUS_MSG_RX 0x4 > +#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8 > + > #define PCIE_CLIENT_INTR_STATUS_MISC 0x10 > +#define PCIE_RDLH_LINK_UP_CHGED BIT(1) > +#define PCIE_LINK_REQ_RST_NOT_INT BIT(2) > + > +#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c > #define PCIE_CLIENT_INTR_MASK_MISC 0x24 > + > #define PCIE_CLIENT_POWER 0x2c > +#define PME_READY_ENTER_L23 BIT(3) > + > #define PCIE_CLIENT_MSG_GEN 0x34 > -#define PME_READY_ENTER_L23 BIT(3) > -#define PME_TURN_OFF (BIT(4) | BIT(20)) > -#define PME_TO_ACK (BIT(9) | BIT(25)) > -#define PCIE_SMLH_LINKUP BIT(16) > -#define PCIE_RDLH_LINKUP BIT(17) > -#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP) > -#define PCIE_RDLH_LINK_UP_CHGED BIT(1) > -#define PCIE_LINK_REQ_RST_NOT_INT BIT(2) > -#define PCIE_CLIENT_GENERAL_CONTROL 0x0 > -#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8 > -#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c > +#define PME_TURN_OFF HIWORD_UPDATE_BIT(BIT(4)) > +#define PME_TO_ACK HIWORD_UPDATE_BIT(BIT(9)) > + > #define PCIE_CLIENT_HOT_RESET_CTRL 0x180 > +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) > + > #define PCIE_CLIENT_LTSSM_STATUS 0x300 > -#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) > -#define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0) > +#define PCIE_LINKUP_MASK GENMASK(17, 16) Here you are adding a macro (PCIE_LINKUP_MASK) that is not used. I suggest that you move the addition to the patch where it is actually used. Kind regards, Niklas
