Hi Jitendra,
On Wednesday 19 Oct 2016 18:37:38 Sharma, Jitendra wrote:
> On 10/19/2016 5:21 PM, Laurent Pinchart wrote:
> > 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@xxxxxxxxxxxxxx>
> >> ---
> >> 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 ?
> >
> >> 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
For the code block in the probe function I understand, but for the
initializers here I don't see the point.
And one might argue that using id->driver_data unconditionally would be better
as it would save a call to of_device_get_match_data()...
> >> { }
> >>
> >> };
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
