| Message ID | 20240710221630.29561-5-james.quinlan@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: brcnstb: Enable STB 7712 SOC | expand |
On 7/11/2024 3:46 AM, Jim Quinlan wrote: > The 7712 SOC has a bridge reset which can be described in the device tree. > If it is present, use it. Otherwise, continue to use the legacy method to > reset the bridge. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index c257434edc08..92816d8d215a 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -265,6 +265,7 @@ struct brcm_pcie { > enum pcie_type type; > struct reset_control *rescal; > struct reset_control *perst_reset; > + struct reset_control *bridge; > int num_memc; > u64 memc_size[PCIE_BRCM_MAX_MEMC]; > u32 hw_rev; > @@ -732,12 +733,19 @@ static void __iomem *brcm7425_pcie_map_bus(struct pci_bus *bus, > > 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 (pcie->bridge) { > + if (val) > + reset_control_assert(pcie->bridge); > + else > + reset_control_deassert(pcie->bridge); > + } else { > + 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)); > + 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)); > + } > } > > static void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32 val) > @@ -1621,6 +1629,10 @@ static int brcm_pcie_probe(struct platform_device *pdev) > if (IS_ERR(pcie->perst_reset)) > return PTR_ERR(pcie->perst_reset); > > + pcie->bridge = devm_reset_control_get_optional_exclusive(&pdev->dev, "bridge"); > + if (IS_ERR(pcie->bridge)) > + return PTR_ERR(pcie->bridge); How about using "dev_err_probe," which utilizes "dev_err" for logging error messages and recording the deferred probe reason? > + > ret = clk_prepare_enable(pcie->clk); > if (ret) { > dev_err(&pdev->dev, "could not enable clock\n");
On 7/11/24 01:16, Jim Quinlan wrote: > The 7712 SOC has a bridge reset which can be described in the device tree. > If it is present, use it. Otherwise, continue to use the legacy method to > reset the bridge. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) Reviewed-by: Stanimir Varbanov <svarbanov@suse.de> ~Stan
On 7/10/24 15:16, Jim Quinlan wrote: > The 7712 SOC has a bridge reset which can be described in the device tree. > If it is present, use it. Otherwise, continue to use the legacy method to > reset the bridge. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
On Sat, Jul 13, 2024 at 3:12 PM Amit Singh Tomar <amitsinght@marvell.com> wrote: > > On 7/11/2024 3:46 AM, Jim Quinlan wrote: > > The 7712 SOC has a bridge reset which can be described in the device tree. > > If it is present, use it. Otherwise, continue to use the legacy method to > > reset the bridge. > > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > > --- > > drivers/pci/controller/pcie-brcmstb.c | 22 +++++++++++++++++----- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > > index c257434edc08..92816d8d215a 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -265,6 +265,7 @@ struct brcm_pcie { > > enum pcie_type type; > > struct reset_control *rescal; > > struct reset_control *perst_reset; > > + struct reset_control *bridge; > > int num_memc; > > u64 memc_size[PCIE_BRCM_MAX_MEMC]; > > u32 hw_rev; > > @@ -732,12 +733,19 @@ static void __iomem *brcm7425_pcie_map_bus(struct pci_bus *bus, > > > > 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 (pcie->bridge) { > > + if (val) > > + reset_control_assert(pcie->bridge); > > + else > > + reset_control_deassert(pcie->bridge); > > + } else { > > + 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)); > > + 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)); > > + } > > } > > > > static void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32 val) > > @@ -1621,6 +1629,10 @@ static int brcm_pcie_probe(struct platform_device *pdev) > > if (IS_ERR(pcie->perst_reset)) > > return PTR_ERR(pcie->perst_reset); > > > > + pcie->bridge = devm_reset_control_get_optional_exclusive(&pdev->dev, "bridge"); > > + if (IS_ERR(pcie->bridge)) > > + return PTR_ERR(pcie->bridge); > How about using "dev_err_probe," which utilizes "dev_err" for logging > error messages and recording the deferred probe reason? Hello Amit, Someone else brought this up and I want to defer moving to dev_err_probe() in a future series. I'd like to get this series out now as Stan is waiting to apply his commits on top of it. Regards, Jim Quinlan Broadcom STB/CM > > + > > ret = clk_prepare_enable(pcie->clk); > > if (ret) { > > dev_err(&pdev->dev, "could not enable clock\n"); >
On 7/17/2024 12:10 AM, Jim Quinlan wrote: > On Sat, Jul 13, 2024 at 3:12 PM Amit Singh Tomar <amitsinght@marvell.com> wrote: >> >> On 7/11/2024 3:46 AM, Jim Quinlan wrote: >>> The 7712 SOC has a bridge reset which can be described in the device tree. >>> If it is present, use it. Otherwise, continue to use the legacy method to >>> reset the bridge. >>> >>> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> >>> --- >>> drivers/pci/controller/pcie-brcmstb.c | 22 +++++++++++++++++----- >>> 1 file changed, 17 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c >>> index c257434edc08..92816d8d215a 100644 >>> --- a/drivers/pci/controller/pcie-brcmstb.c >>> +++ b/drivers/pci/controller/pcie-brcmstb.c >>> @@ -265,6 +265,7 @@ struct brcm_pcie { >>> enum pcie_type type; >>> struct reset_control *rescal; >>> struct reset_control *perst_reset; >>> + struct reset_control *bridge; >>> int num_memc; >>> u64 memc_size[PCIE_BRCM_MAX_MEMC]; >>> u32 hw_rev; >>> @@ -732,12 +733,19 @@ static void __iomem *brcm7425_pcie_map_bus(struct pci_bus *bus, >>> >>> 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 (pcie->bridge) { >>> + if (val) >>> + reset_control_assert(pcie->bridge); >>> + else >>> + reset_control_deassert(pcie->bridge); >>> + } else { >>> + 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)); >>> + 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)); >>> + } >>> } >>> >>> static void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32 val) >>> @@ -1621,6 +1629,10 @@ static int brcm_pcie_probe(struct platform_device *pdev) >>> if (IS_ERR(pcie->perst_reset)) >>> return PTR_ERR(pcie->perst_reset); >>> >>> + pcie->bridge = devm_reset_control_get_optional_exclusive(&pdev->dev, "bridge"); >>> + if (IS_ERR(pcie->bridge)) >>> + return PTR_ERR(pcie->bridge); >> How about using "dev_err_probe," which utilizes "dev_err" for logging >> error messages and recording the deferred probe reason? > > Hello Amit, > > Someone else brought this up and I want to defer moving to > dev_err_probe() in a future series. I'd like to get this series out > now as Stan is waiting to apply his commits on top of it. > Sure, that's fine with me. Thanks, -Amit
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index c257434edc08..92816d8d215a 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -265,6 +265,7 @@ struct brcm_pcie { enum pcie_type type; struct reset_control *rescal; struct reset_control *perst_reset; + struct reset_control *bridge; int num_memc; u64 memc_size[PCIE_BRCM_MAX_MEMC]; u32 hw_rev; @@ -732,12 +733,19 @@ static void __iomem *brcm7425_pcie_map_bus(struct pci_bus *bus, 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 (pcie->bridge) { + if (val) + reset_control_assert(pcie->bridge); + else + reset_control_deassert(pcie->bridge); + } else { + 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)); + 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)); + } } static void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32 val) @@ -1621,6 +1629,10 @@ static int brcm_pcie_probe(struct platform_device *pdev) if (IS_ERR(pcie->perst_reset)) return PTR_ERR(pcie->perst_reset); + pcie->bridge = devm_reset_control_get_optional_exclusive(&pdev->dev, "bridge"); + if (IS_ERR(pcie->bridge)) + return PTR_ERR(pcie->bridge); + ret = clk_prepare_enable(pcie->clk); if (ret) { dev_err(&pdev->dev, "could not enable clock\n");
The 7712 SOC has a bridge reset which can be described in the device tree. If it is present, use it. Otherwise, continue to use the legacy method to reset the bridge. Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> --- drivers/pci/controller/pcie-brcmstb.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)