Hello, [...] > > > static void brcm_pcie_bridge_sw_init_set_generic(struct brcm_pcie *pcie, u32 val) > > > { > > > - u32 tmp, mask = RGR1_SW_INIT_1_INIT_GENERIC_MASK; > > > - u32 shift = RGR1_SW_INIT_1_INIT_GENERIC_SHIFT; > > > + if (val) > > > + reset_control_assert(pcie->bridge_reset); > > > + else > > > + reset_control_deassert(pcie->bridge_reset); > > > > > > - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > > - tmp = (tmp & ~mask) | ((val << shift) & mask); > > > - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > > + if (!pcie->bridge_reset) { > > > + u32 tmp, mask = RGR1_SW_INIT_1_INIT_GENERIC_MASK; > > > + u32 shift = RGR1_SW_INIT_1_INIT_GENERIC_SHIFT; > > > + > > > + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > > + tmp = (tmp & ~mask) | ((val << shift) & mask); > > > + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > > + } > > > > This pattern looks goofy: > > > > reset_control_assert(pcie->bridge_reset); > > if (!pcie->bridge_reset) { > > ... > > > > If we're going to test pcie->bridge_reset at all, it should be first > > so it's obvious what's going on and the reader doesn't have to go > > verify that reset_control_assert() ignores and returns success for a > > NULL pointer: > > > > if (pcie->bridge_reset) { > > if (val) > > reset_control_assert(pcie->bridge_reset); > > else > > reset_control_deassert(pcie->bridge_reset); > > > > return; > > } > > > > u32 tmp, mask = RGR1_SW_INIT_1_INIT_GENERIC_MASK; > > ... > > > Will do. [...] You will do what? If you don't mind me asking. Krzysztof