1. Patchew
  2. linux
  3. [PATCH v3 00/11] Add PCIe support for bcm2712
  4. View patch

[PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

Stanimir Varbanov posted 11 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset
Posted by Stanimir Varbanov 1 month, 1 week ago 
On PCIe turn off avoid shutdown of bridge reset,
by introducing a quirk flag.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v2 -> v3:
 - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.

 drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index b76c16287f37..757a1646d53c 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -234,10 +234,20 @@ struct inbound_win {
 	u64 cpu_addr;
 };
 
+/*
+ * The RESCAL block is tied to PCIe controller #1, regardless of the number of
+ * controllers, and turning off PCIe controller #1 prevents access to the RESCAL
+ * register blocks, therefore not other controller can access this register
+ * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
+ * or a hang (AXI).
+ */
+#define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
+
 struct pcie_cfg_data {
 	const int *offsets;
 	const enum pcie_soc_base soc_base;
 	const bool has_phy;
+	const u32 quirks;
 	u8 num_inbound_wins;
 	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
 	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
@@ -290,6 +300,7 @@ struct brcm_pcie {
 	struct subdev_regulators *sr;
 	bool			ep_wakeup_capable;
 	bool			has_phy;
+	u32			quirks;
 	u8			num_inbound_wins;
 };
 
@@ -1539,8 +1550,9 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
 	writel(tmp, base + HARD_DEBUG(pcie));
 
-	/* Shutdown PCIe bridge */
-	ret = pcie->bridge_sw_init_set(pcie, 1);
+	if (!(pcie->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
+		/* Shutdown PCIe bridge */
+		ret = pcie->bridge_sw_init_set(pcie, 1);
 
 	return ret;
 }
@@ -1854,6 +1866,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 	pcie->perst_set = data->perst_set;
 	pcie->bridge_sw_init_set = data->bridge_sw_init_set;
 	pcie->has_phy = data->has_phy;
+	pcie->quirks = data->quirks;
 	pcie->num_inbound_wins = data->num_inbound_wins;
 
 	pcie->base = devm_platform_ioremap_resource(pdev, 0);
-- 
2.43.0
Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset
Posted by Jim Quinlan 1 month, 1 week ago 
On Mon, Oct 14, 2024 at 9:07 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> On PCIe turn off avoid shutdown of bridge reset,
> by introducing a quirk flag.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>  - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.
>
>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index b76c16287f37..757a1646d53c 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -234,10 +234,20 @@ struct inbound_win {
>         u64 cpu_addr;
>  };
>
> +/*
> + * The RESCAL block is tied to PCIe controller #1, regardless of the number of
> + * controllers, and turning off PCIe controller #1 prevents access to the RESCAL
> + * register blocks, therefore not other controller can access this register

s/no/not/

I assume that the quirks is specific to 2712 as the 7712 does not need
this since it only has PCIe1
(I'll probably seethis as I read more of your commits).

-- Jim

> + * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
> + * or a hang (AXI).
> + */
> +#define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN                BIT(0)
> +
>  struct pcie_cfg_data {
>         const int *offsets;
>         const enum pcie_soc_base soc_base;
>         const bool has_phy;
> +       const u32 quirks;
>         u8 num_inbound_wins;
>         int (*perst_set)(struct brcm_pcie *pcie, u32 val);
>         int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> @@ -290,6 +300,7 @@ struct brcm_pcie {
>         struct subdev_regulators *sr;
>         bool                    ep_wakeup_capable;
>         bool                    has_phy;
> +       u32                     quirks;
>         u8                      num_inbound_wins;
>  };
>
> @@ -1539,8 +1550,9 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
>         u32p_replace_bits(&tmp, 1, PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
>         writel(tmp, base + HARD_DEBUG(pcie));
>
> -       /* Shutdown PCIe bridge */
> -       ret = pcie->bridge_sw_init_set(pcie, 1);
> +       if (!(pcie->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
> +               /* Shutdown PCIe bridge */
> +               ret = pcie->bridge_sw_init_set(pcie, 1);
>
>         return ret;
>  }
> @@ -1854,6 +1866,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>         pcie->perst_set = data->perst_set;
>         pcie->bridge_sw_init_set = data->bridge_sw_init_set;
>         pcie->has_phy = data->has_phy;
> +       pcie->quirks = data->quirks;
>         pcie->num_inbound_wins = data->num_inbound_wins;
>
>         pcie->base = devm_platform_ioremap_resource(pdev, 0);
> --
> 2.43.0
>
Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset
Posted by Stanimir Varbanov 1 month, 1 week ago 
Hi Jim,

On 10/16/24 20:17, Jim Quinlan wrote:
> On Mon, Oct 14, 2024 at 9:07 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>>
>> On PCIe turn off avoid shutdown of bridge reset,
>> by introducing a quirk flag.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> ---
>> v2 -> v3:
>>  - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.
>>
>>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
>> index b76c16287f37..757a1646d53c 100644
>> --- a/drivers/pci/controller/pcie-brcmstb.c
>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>> @@ -234,10 +234,20 @@ struct inbound_win {
>>         u64 cpu_addr;
>>  };
>>
>> +/*
>> + * The RESCAL block is tied to PCIe controller #1, regardless of the number of
>> + * controllers, and turning off PCIe controller #1 prevents access to the RESCAL
>> + * register blocks, therefore not other controller can access this register
> 
> s/no/not/
> 
> I assume that the quirks is specific to 2712 as the 7712 does not need
> this since it only has PCIe1
> (I'll probably seethis as I read more of your commits).

Yes, the .post_setup op is implemented for 2712 only. Look into next
patch in the series.

~Stan
Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset
Posted by Bjorn Helgaas 1 month, 1 week ago 
On Mon, Oct 14, 2024 at 04:07:05PM +0300, Stanimir Varbanov wrote:
> On PCIe turn off avoid shutdown of bridge reset,
> by introducing a quirk flag.

Can you include something here about *why* we need this change?  I
think the RESCAL comment below would be a good start.

I think this should be squashed with the next commit that adds the use
of CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN.  Otherwise this commit doesn't
have an obvious reason.

> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>  - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.
> 
>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index b76c16287f37..757a1646d53c 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -234,10 +234,20 @@ struct inbound_win {
>  	u64 cpu_addr;
>  };
>  
> +/*
> + * The RESCAL block is tied to PCIe controller #1, regardless of the number of
> + * controllers, and turning off PCIe controller #1 prevents access to the RESCAL
> + * register blocks, therefore not other controller can access this register
> + * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
> + * or a hang (AXI).

s/not other/no other/

> + */
> +#define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
> +
>  struct pcie_cfg_data {
>  	const int *offsets;
>  	const enum pcie_soc_base soc_base;
>  	const bool has_phy;
> +	const u32 quirks;
>  	u8 num_inbound_wins;
>  	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
>  	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> @@ -290,6 +300,7 @@ struct brcm_pcie {
>  	struct subdev_regulators *sr;
>  	bool			ep_wakeup_capable;
>  	bool			has_phy;
> +	u32			quirks;
>  	u8			num_inbound_wins;
>  };
>  
> @@ -1539,8 +1550,9 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
>  	u32p_replace_bits(&tmp, 1, PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
>  	writel(tmp, base + HARD_DEBUG(pcie));
>  
> -	/* Shutdown PCIe bridge */
> -	ret = pcie->bridge_sw_init_set(pcie, 1);
> +	if (!(pcie->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
> +		/* Shutdown PCIe bridge */
> +		ret = pcie->bridge_sw_init_set(pcie, 1);
>  
>  	return ret;
>  }
> @@ -1854,6 +1866,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  	pcie->perst_set = data->perst_set;
>  	pcie->bridge_sw_init_set = data->bridge_sw_init_set;
>  	pcie->has_phy = data->has_phy;
> +	pcie->quirks = data->quirks;
>  	pcie->num_inbound_wins = data->num_inbound_wins;
>  
>  	pcie->base = devm_platform_ioremap_resource(pdev, 0);
> -- 
> 2.43.0
>
Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset
Posted by Florian Fainelli 1 month, 1 week ago 
On 10/14/24 10:01, Bjorn Helgaas wrote:
> On Mon, Oct 14, 2024 at 04:07:05PM +0300, Stanimir Varbanov wrote:
>> On PCIe turn off avoid shutdown of bridge reset,
>> by introducing a quirk flag.
> 
> Can you include something here about *why* we need this change?  I
> think the RESCAL comment below would be a good start.
> 
> I think this should be squashed with the next commit that adds the use
> of CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN.  Otherwise this commit doesn't
> have an obvious reason.

Agreed.
-- 
Florian
Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset
Posted by Stanimir Varbanov 1 month, 1 week ago 
Hi,

On 10/14/24 20:02, Florian Fainelli wrote:
> On 10/14/24 10:01, Bjorn Helgaas wrote:
>> On Mon, Oct 14, 2024 at 04:07:05PM +0300, Stanimir Varbanov wrote:
>>> On PCIe turn off avoid shutdown of bridge reset,
>>> by introducing a quirk flag.
>>
>> Can you include something here about *why* we need this change?  I
>> think the RESCAL comment below would be a good start.
>>
>> I think this should be squashed with the next commit that adds the use
>> of CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN.  Otherwise this commit doesn't
>> have an obvious reason.
> 
> Agreed.

OK, will do. Thank you for the review!

~Stan

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.