Re: [PATCH v3] fpga: altera-freeze-bridge: clear all illegal request bits

["NG, TZE YEE" <tze.yee.ng@altera.com>]

Hi Yilun,

On 7/11/2026 6:19 pm, Xu Yilun wrote:
> On Tue, Oct 06, 2026 at 10:34:35AM +0800, tze.yee.ng@altera.com wrote:
>> From: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
>>
>> The freeze_illegal_request register is write-1-to-clear with one bit per
>> Freeze Bridge connected to the Partial Reconfiguration Region Controller;
> 
> I still prefer the opening sentences in v2. Don't write like dense
> Hardware spec.
> 

Sure, I'll restore the v2 opening in v4:

   A Partial Reconfiguration 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.

>> bit i is the bridge on freeze interface i. Unlike the status and control
>> registers, which act on all bridges collectively, this register reports
>> each bridge individually.
> 
> This clearly tells the hardware behavior, good.
> 

Thanks, I'll keep that sentence.

> But my real question is, why would the Partial Reconfiguration Region
> Controller connect to multiple bridges? To control multiple PR regions,
> or because a single PR region requires multiple bridges so we need to
> gate them all on its PR?
> 

The latter. One region controller drives a single PR region, but that
region usually crosses the static boundary through more than one bus
interface (e.g. an Avalon-MM slave to the region plus a master to a
transceiver reconfig interface), and each interface needs its own Freeze
Bridge. The controller gates them all together for that region's PR. Per
the Quartus Pro PR User Guide, the Freeze Bridge is instantiated "one
instance for each interface in each PR region". Separate PR regions 
would each have their own region controller and handshake.

If you have no further questions, I'll send out v4 later with the 
restored opening sentences.

Thanks,
Tze Yee

>>
>> Writing a literal 1 clears only bit 0 and leaves the illegal request
>> latched for every other bridge. Write back the value just read instead, so
>> the set-to-clear applies to whichever Freeze Bridge reported the 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>
>> ---
>> Changes in v3:
>> - Reword the commit message in imperative mood (Xu Yilun).
>> - Describe the freeze_illegal_request register layout: one write-1-to-clear
>>    bit per Freeze Bridge via the Conduit Merger, versus the collective
>>    status/ctrl registers (Xu Yilun).
>>
>> 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 v2:
>> https://lore.kernel.org/all/2fe34c2dec61150a39d21e067521b0acc031247c.1789956938.git.tze.yee.ng@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
>>
>>