On 2/8/22 7:11 AM, Mauro Carvalho Chehab wrote:
> Em Wed, 2 Feb 2022 17:36:09 +0200
> Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu:
>
>> As part of removing mbus config flags, remove VC flag use in the
>> microchip-csi2dc driver. The support can be reintroduced later on as part
>> of the streams patches.
>>
>> Cc: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
>
> Hmm... that sounds a regression to me. What effects this will cause at
> the driver?
>
> Eugen, any comments?
Hi ,
I am not happy with this change. It looks like I wasn't even CC-ed on
the original patch e-mail.
The effect on the driver will be that everything will be treated as
virtual channel=0 .
I do not yet understand why we are about to remove
V4L2_MBUS_CSI2_CHANNEL_* as I remember this was just introduced.
Is there any alternative in place ?
My opinion is that if we want to replace something existing with a new
API or something else, we should first add the new support, block any
new adopters for the old API such that everyone uses the new API, and
only after that convert the old API clients to the new API.
So 'can be reintroduced later on' is not okay. We can't remove things in
the hope that it would be reintroduced later. Just my personal take on
this, feel free to have a different opinion.
In the end you guys are the maintainers for the subsystem and can have
this change if you like, I am more unhappy about the fact that changes
happen suddenly and without notice.
Eugen
>
>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>> ---
>> .../media/platform/atmel/microchip-csi2dc.c | 18 ++----------------
>> 1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/media/platform/atmel/microchip-csi2dc.c b/drivers/media/platform/atmel/microchip-csi2dc.c
>> index 6bc549c28e05..6a7f5b4b0e3b 100644
>> --- a/drivers/media/platform/atmel/microchip-csi2dc.c
>> +++ b/drivers/media/platform/atmel/microchip-csi2dc.c
>> @@ -348,24 +348,15 @@ static int csi2dc_get_mbus_config(struct csi2dc_device *csi2dc)
>> if (ret == -ENOIOCTLCMD) {
>> dev_dbg(csi2dc->dev,
>> "no remote mbus configuration available\n");
>> - goto csi2dc_get_mbus_config_defaults;
>> + return 0;
>> }
>>
>> if (ret) {
>> dev_err(csi2dc->dev,
>> "failed to get remote mbus configuration\n");
>> - goto csi2dc_get_mbus_config_defaults;
>> + return 0;
>> }
>>
>> - if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_0)
>> - csi2dc->vc = 0;
>> - else if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_1)
>> - csi2dc->vc = 1;
>> - else if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_2)
>> - csi2dc->vc = 2;
>> - else if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_3)
>> - csi2dc->vc = 3;
>> -
>> dev_dbg(csi2dc->dev, "subdev sending on channel %d\n", csi2dc->vc);
>>
>> csi2dc->clk_gated = mbus_config.flags &
>> @@ -375,11 +366,6 @@ static int csi2dc_get_mbus_config(struct csi2dc_device *csi2dc)
>> csi2dc->clk_gated ? "gated" : "free running");
>>
>> return 0;
>> -
>> -csi2dc_get_mbus_config_defaults:
>> - csi2dc->vc = 0; /* Virtual ID 0 by default */
>> -
>> - return 0;
>> }
>>
>> static void csi2dc_vp_update(struct csi2dc_device *csi2dc)
