Re: [PATCH 15/28] media: ti-vpe: cal: remove wait when stopping camerarx

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

On 18/04/2021 15:46, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Apr 12, 2021 at 02:34:44PM +0300, Tomi Valkeinen wrote:
>> Asserting ComplexIO reset seems to affect the HW (ie. asserting reset
>> will break an active capture), but the RESET_DONE bit never changes to
>> "reset is ongoing" state. Thus we always get a timeout.
>>
>> Drop the wait, as it seems to achieve nothing.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   drivers/media/platform/ti-vpe/cal-camerarx.c | 15 ++-------------
>>   1 file changed, 2 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
>> index 0354f311c5d2..245c601b992c 100644
>> --- a/drivers/media/platform/ti-vpe/cal-camerarx.c
>> +++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
>> @@ -405,7 +405,6 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
>>   
>>   static void cal_camerarx_stop(struct cal_camerarx *phy)
>>   {
>> -	unsigned int i;
>>   	int ret;
>>   
>>   	cal_camerarx_ppi_disable(phy);
>> @@ -419,19 +418,9 @@ static void cal_camerarx_stop(struct cal_camerarx *phy)
>>   			CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL,
>>   			CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL_MASK);
>>   
>> -	/* Wait for power down completion */
>> -	for (i = 0; i < 10; i++) {
>> -		if (cal_read_field(phy->cal,
>> -				   CAL_CSI2_COMPLEXIO_CFG(phy->instance),
>> -				   CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_MASK) ==
>> -		    CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETONGOING)
> 
> Isn't this the wrong condition ? I would have expected
> CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETCOMPLETED, not
> CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETONGOING. That could explain why
> you always get a timeout.

No, I don't think so. The complexio reset is set active just before the 
wait. So the reset status should show reset ongoing, until at some point 
we release the reset (we do that when starting the PHY again).

The TRM doesn't talk about this, though. So, I guess the status might go 
to RESETONGOING for a very short time and back to RESETCOMPLETED before 
the code can see the RESETONGOING. But I suspect the status just stays 
at RESETCOMPLETED.

  Tomi