[PATCH v2] fpga: bridge: incorrect set to clear freeze_illegal_request register

[PATCH v2] fpga: bridge: incorrect set to clear freeze_illegal_request register

[tze.yee.ng]

From: Tanmay Kathpalia <tanmay.kathpalia@altera.com>

A Partial Region Controller can be connected to one or more
Freeze Bridge. Each Freeze Bridge has an illegal_request
bit represented in the freeze_illegal_request register.
Thus, instead of just set to clear the illegal_request bit
for first Freeze Bridge, we need to ensure the set to clear
action is applied to which ever Freeze Bridge that has
occurrence of illegal request.

Fixes: ca24a648f535 ("fpga: add altera freeze bridge support")
Cc: stable@vger.kernel.org # v4.10+

Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Taking over this patch from Tanmay.

Changes in v2:
- Add "Cc: stable@vger.kernel.org # v4.10+" into the signoff area so the
  stable-kernel rule is satisfied (reported by kernel test robot).
- Remove Chiau Ee's signoff as he is no longer working on this patch.

Link v1: 
https://lore.kernel.org/all/20250311151601.12264-1-tanmay.kathpalia@altera.com/
---
 drivers/fpga/altera-freeze-bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index c24c976117c8..2591dcc53d83 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -51,7 +51,7 @@ static int altera_freeze_br_req_ack(struct altera_freeze_br_data *priv,
 		if (illegal) {
 			dev_err(dev, "illegal request detected 0x%x", illegal);
 
-			writel(1, csr_illegal_req_addr);
+			writel(illegal, csr_illegal_req_addr);
 
 			illegal = readl(csr_illegal_req_addr);
 			if (illegal)
-- 
2.43.7

Re: [PATCH v2] fpga: bridge: incorrect set to clear freeze_illegal_request register

[Xu Yilun]

On Sun, Sep 20, 2026 at 07:23:51PM -0700, tze.yee.ng@altera.com wrote:
> From: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
> 
> A Partial Region Controller can be connected to one or more
> Freeze Bridge. Each Freeze Bridge has an illegal_request
> bit represented in the freeze_illegal_request register.

Why only this register reveals details about multiple bridges, while
other registers always operate on all of them? And how can we understand
which bridges are actually in use.

> Thus, instead of just set to clear the illegal_request bit
> for first Freeze Bridge, we need to ensure the set to clear

Imperative mood.

> action is applied to which ever Freeze Bridge that has
> occurrence of illegal request.
> 
> Fixes: ca24a648f535 ("fpga: add altera freeze bridge support")
> Cc: stable@vger.kernel.org # v4.10+
> 
> Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
> Taking over this patch from Tanmay.
> 
> Changes in v2:
> - Add "Cc: stable@vger.kernel.org # v4.10+" into the signoff area so the
>   stable-kernel rule is satisfied (reported by kernel test robot).
> - Remove Chiau Ee's signoff as he is no longer working on this patch.
> 
> Link v1: 
> https://lore.kernel.org/all/20250311151601.12264-1-tanmay.kathpalia@altera.com/
> ---
>  drivers/fpga/altera-freeze-bridge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
> index c24c976117c8..2591dcc53d83 100644
> --- a/drivers/fpga/altera-freeze-bridge.c
> +++ b/drivers/fpga/altera-freeze-bridge.c
> @@ -51,7 +51,7 @@ static int altera_freeze_br_req_ack(struct altera_freeze_br_data *priv,
>  		if (illegal) {
>  			dev_err(dev, "illegal request detected 0x%x", illegal);
>  
> -			writel(1, csr_illegal_req_addr);
> +			writel(illegal, csr_illegal_req_addr);
>  
>  			illegal = readl(csr_illegal_req_addr);
>  			if (illegal)
> -- 
> 2.43.7
> 
> 

Re: [PATCH v2] fpga: bridge: incorrect set to clear freeze_illegal_request register

[NG, TZE YEE]

On 28/9/2026 6:25 pm, Xu Yilun wrote:
> [You don't often get email from yilun.xu@linux.intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Sun, Sep 20, 2026 at 07:23:51PM -0700, tze.yee.ng@altera.com wrote:
>> From: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
>>
>> A Partial Region Controller can be connected to one or more
>> Freeze Bridge. Each Freeze Bridge has an illegal_request
>> bit represented in the freeze_illegal_request register.
> 
> Why only this register reveals details about multiple bridges, while
> other registers always operate on all of them? And how can we understand
> which bridges are actually in use.
> 

Hi Yilun,

The Partial Region Controller fans its single freeze signal out to all 
bridges via a Conduit Splitter, so the status (0x00) and ctrl (0x04) 
registers act on them collectively. But each bridge drives its own 
single-bit illegal_request, and a Conduit Merger concatenates these into 
the n-bit, write-1-to-clear freeze_illegal_request register (0x08), bit 
i is the bridge on freeze interface i, n = number of freeze interfaces. 
(See the Quartus Prime Pro PR User Guide, Conduit Splitter/Merger.)

The driver need not track n: the read value already identifies the 
bridges flagging an illegal request, and writing it back clears exactly 
those bits. Writing a literal 1 clears only bit 0, leaving every other 
bridge latched, so this patch fixes the bug. I'll add this to the commit 
message in v3.

>> Thus, instead of just set to clear the illegal_request bit
>> for first Freeze Bridge, we need to ensure the set to clear
> 
> Imperative mood.
> 

Will fix in v3, e.g. "Clear every illegal_request bit that is set, 
instead of clearing only the bit for the first Freeze Bridge."

Thanks,
Tze Yee

>> action is applied to which ever Freeze Bridge that has
>> occurrence of illegal request.
>>
>> Fixes: ca24a648f535 ("fpga: add altera freeze bridge support")
>> Cc: stable@vger.kernel.org # v4.10+
>>
>> Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
>> Taking over this patch from Tanmay.
>>
>> Changes in v2:
>> - Add "Cc: stable@vger.kernel.org # v4.10+" into the signoff area so the
>>    stable-kernel rule is satisfied (reported by kernel test robot).
>> - Remove Chiau Ee's signoff as he is no longer working on this patch.
>>
>> Link v1:
>> https://lore.kernel.org/all/20250311151601.12264-1-tanmay.kathpalia@altera.com/
>> ---
>>   drivers/fpga/altera-freeze-bridge.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
>> index c24c976117c8..2591dcc53d83 100644
>> --- a/drivers/fpga/altera-freeze-bridge.c
>> +++ b/drivers/fpga/altera-freeze-bridge.c
>> @@ -51,7 +51,7 @@ static int altera_freeze_br_req_ack(struct altera_freeze_br_data *priv,
>>                if (illegal) {
>>                        dev_err(dev, "illegal request detected 0x%x", illegal);
>>
>> -                     writel(1, csr_illegal_req_addr);
>> +                     writel(illegal, csr_illegal_req_addr);
>>
>>                        illegal = readl(csr_illegal_req_addr);
>>                        if (illegal)
>> --
>> 2.43.7
>>
>>