On Tue, Apr 22, 2025 at 07:50:50PM +0800, Hans Zhang wrote: > > > On 2025/4/22 19:39, Niklas Cassel wrote: > > On Tue, Apr 22, 2025 at 07:28:30PM +0800, Hans Zhang wrote: > > > Link-up detection manually checked PCIE_LINKUP bits across RC/EP modes, > > > leading to code duplication. Centralize the logic using FIELD_GET. This > > > removes redundancy and abstracts hardware-specific bit masking, ensuring > > > consistent link state handling. > > > > > > Signed-off-by: Hans Zhang <18255117159@xxxxxxx> > > > --- > > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 15 +++++---------- > > > 1 file changed, 5 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > > index cdc8afc6cfc1..2b26060af5c2 100644 > > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > > @@ -196,10 +196,7 @@ static int rockchip_pcie_link_up(struct dw_pcie *pci) > > > struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > > > u32 val = rockchip_pcie_get_ltssm(rockchip); > > > - if ((val & PCIE_LINKUP) == PCIE_LINKUP) > > > - return 1; > > > - > > > - return 0; > > > + return FIELD_GET(PCIE_LINKUP_MASK, val) == 3; > > > > While I like the idea of your patch, here you are replacing something that > > is easy to read (PCIE_LINKUP) with a magic value, which IMO is a step in > > the wrong direction. > > > > Hi Niklas, > > Thank you very much for your reply. How about I add another macro > definition? > > #define PCIE_LINKUP 3 Yes, adding another macro for it is what I meant. Kind regards, Niklas
