Re: [PATCH v2] fpga: altera-cvp: Truncated bitstream error support

From: Xu Yilun <yilun.xu@intel.com>
To: tien.sung.ang@intel.com
Cc: christophe.jaillet@wanadoo.fr, hao.wu@intel.com,
	linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
	mdf@kernel.org, trix@redhat.com
Subject: Re: [PATCH v2] fpga: altera-cvp: Truncated bitstream error support
Date: Mon, 27 Jun 2022 16:52:55 +0800	[thread overview]
Message-ID: <20220627085255.GA2264502@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <20220627074553.3136597-1-tien.sung.ang@intel.com>

On Mon, Jun 27, 2022 at 03:45:53PM +0800, tien.sung.ang@intel.com wrote:
> > > Though without doubt, your solution is doable, I have a 
> > > discussion with the Intel architect and they insisted that
> > > the device driver must not make such a decision. The decision to 
> > > drop or accept a transfer is up to the firmware. The firmware
> 
> > Why dropping or accepting is forbidden, but padding is allowed? If the
> > decision is no operation to the data, then padding should also be
> > forbidden.
> 
> > > insisted that the buffer be padded with whatever value. They
> 
> > If I understand right, the 2 decisions are conflicted. On one hand,
> > the driver should not care about the data. On another hand, the
> > driver should still care about the data for some rule.
> 
> > So please firstly reach your agreement before making patches.
> 
> Had some delays in replying,from the perspective of the driver, 
> Intel architects want it to be just purely on the transfer of 
> data without inspecting the data. 
> If the size of a transfer is 4096bytes, we can padd it. 
> I can do that which I think it is not an issue.
> Let me create a reworked patch to pad the payload with Zeros.

No, it's not about padding with Zeros or other values. If we should
stick on no image touching, then please fix the firmware. If we need
a workaround in driver, then making a decent fix. Blindly copy the image
is just a bad idea, and it could actually be avoided.

Thanks,
Yilun

> Would this be OK for patch v3?
> 
> > > Yes, it could look confusing to other programmers. And, yes, the 
> > > padding doesn't matter. Let me relook into this. As the driver is 
> > > already re-tested by the silicon validatioin. 
> > > I want to avoid making any change as it
> > > would meant another couple of weeks of re-testing. 
> > > Can this be accepted as it is?
> 
> > Sorry, I can't. Some clarification is needed.
> 
> Reply as above, let us implement the padding in the
> next patch v3. Appreciate your inputs. Thanks