Hi.,
On 10/19/2016 6:37 PM, Sharma, Jitendra wrote:
> Hi Laurent,
>
>
> On 10/19/2016 5:21 PM, Laurent Pinchart wrote:
>> Hi Jitendra,
>>
>> Thank you for the patch.
>>
>> On Wednesday 19 Oct 2016 17:12:48 Jitendra Sharma wrote:
>>> Remove redundant condition check
>>> Remove not necessary if-else block for checking DT entry because else
>>> part will never be picked as in absence of device node, probe will
>>> fail in initial stage only.
>>>
>>> Remove unused id->driver_data entries
>>> As id->driver_data is not used in driver source. So no need in
>>> Keeping these entries in id_table
>>>
>>> Signed-off-by: Jitendra Sharma <shajit at codeaurora.org>
>>> ---
>>> Probe was not happening in Patch v1 due to removal of .id_table.As the
>>> intention of this patch is not to change any functionality, also
>>> changes looks simple enough.So, didn't verified Patch v1 over hardware
>> You should *ALWAYS* verify patches before sending them out.
> Will keep in mind
>>
>> I assume you've now properly tested this one ?
Tested v2 on Jitendra's behalf.
Thanks,
Archit
>>
>>> Hence fixing the issues in Patch v1 and posting patch v2
>>>
>>> Changes for v2:
>>> - Keep the id_table entries
>>> - Keep the id->driver_data to 0
>>> ---
>>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 +++++--------
>>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8ed3906..3279059
>>> 100644
>>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> @@ -942,10 +942,7 @@ static int adv7511_probe(struct i2c_client *i2c,
>>> const
>>> struct i2c_device_id *id) adv7511->powered = false;
>>> adv7511->status = connector_status_disconnected;
>>>
>>> - if (dev->of_node)
>>> - adv7511->type = (enum
>> adv7511_type)of_device_get_match_data(dev);
>>> - else
>>> - adv7511->type = id->driver_data;
>>> + adv7511->type = (enum adv7511_type)of_device_get_match_data(dev);
>>>
>>> memset(&link_config, 0, sizeof(link_config));
>>>
>>> @@ -1066,11 +1063,11 @@ static int adv7511_remove(struct i2c_client
>>> *i2c)
>>> }
>>>
>>> static const struct i2c_device_id adv7511_i2c_ids[] = {
>>> - { "adv7511", ADV7511 },
>>> - { "adv7511w", ADV7511 },
>>> - { "adv7513", ADV7511 },
>>> + { "adv7511", 0 },
>>> + { "adv7511w", 0 },
>>> + { "adv7513", 0 },
>>> #ifdef CONFIG_DRM_I2C_ADV7533
>>> - { "adv7533", ADV7533 },
>>> + { "adv7533", 0 },
>>> #endif
>> What's the purpose of this ? It doesn't save any memory or CPU cycle.
> Idea is to remove unnecessary code, variables and if possible to reduce
> lines of code for example here by eliminating obvious branching.
> Regarding memory or cpu cyles, no difference could be because of
> compiler optimization
>>
>>> { }
>>> };
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
