[PATCH 1/4] media: staging: rkisp1: remove two unused fields in uapi struct

Dafna Hirschfeld dafna.hirschfeld at collabora.com
Mon Jul 6 10:16:50 EDT 2020


Hi,

On 26.06.20 14:47, Ezequiel Garcia wrote:
> Hi Dafna,
> 
> Thanks for all the rkisp1 patches, I hope we can get
> the driver out of staging soon.

me too.

> 
> On Thu, 2020-06-25 at 20:50 +0200, Dafna Hirschfeld wrote:
>> The fields 'config_width', 'config_height' in struct
>> 'rkisp1_cif_isp_lsc_config' are not used by the driver and
>> therefore are not needed. This patch removes them.
>> In later patch, documentation of the fields in struct
>> 'rkisp1_cif_isp_lsc_config' will be added.
>>
> 
> If I understand correctly, you are affecting the uAPI here.
> 
> The fact that the driver doesn't use it now, doesn't mean
> it won't need it at some later point.
> 
> I'm not saying the change is wrong, but that the "not currently
> in use" reason might be insufficient for a uAPI. If you
> want to remove this field, I suggest you make sure
> the field is inappropriate for this interface.
> 
I looked at the documentation and didn't find
any width/height values related to the LSC interface.
So I it seems to be ok to remove those fields.

> Also, it would be better if you could add a cover letter
> on all the series, there are a bunch of rkisp1 patches now
> and having a cover letter helps by adding some context.
> 

I did add a cover-letter: https://patchwork.kernel.org/cover/11625921/

Thanks,
Dafna

> Thanks,
> Ezequiel
> 
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
>> ---
>>   drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>> index ca0d031b14ac..7331bacf7dfd 100644
>> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>> @@ -285,8 +285,6 @@ struct rkisp1_cif_isp_lsc_config {
>>   
>>   	__u32 x_size_tbl[RKISP1_CIF_ISP_LSC_SIZE_TBL_SIZE];
>>   	__u32 y_size_tbl[RKISP1_CIF_ISP_LSC_SIZE_TBL_SIZE];
>> -	__u16 config_width;
>> -	__u16 config_height;
>>   } __packed;
>>   
>>   /**
> 
> 



More information about the Linux-rockchip mailing list