On Wed, Jun 24, 2015 at 10:54 AM, Maninder Singh maninder1.s@samsung.com wrote:

Hi,

...

...

•   if (!sender || !data_out || !len_out) {
  

•           DRM_ERROR("Invalid parameters\n");
  

•           return -EINVAL;
  

•   }
  

I would prefer to have these kind of checks where it actually matters (ie. in __read_panel_data()). The saner thing would be to move the dereference until after the check and remove the duplicated check from mdfld_dsi_read_mcs(). That would prevent any further need for adding additional checks whenever calling __read_panel_data().

Ok agree, But i am thinking whether this initilaization has to be there? struct drm_device *dev = sender->dev;

Because in function __read_panel_data I saw no usage of this dev struct, So along with check from mdfld_dsi_read_mcs, can we remove this dev from __read_panel_data also ? Or i missed something in code?

REG_READ and REG_WRITE macros need the drm device (see psb_drv.h) so it must stay.

Thanks, Maninder

---