On 05/10/2016 02:08 AM, Laurent Pinchart wrote: > Hi Archit, > > On Tuesday 03 May 2016 12:27:38 Archit Taneja wrote: >> On 04/22/2016 10:40 AM, Archit Taneja wrote: >>> On 04/22/2016 03:59 AM, Laurent Pinchart wrote: >>>> On Wednesday 09 Mar 2016 16:27:15 Archit Taneja wrote: >>>>> In order to pass DSI specific parameters to the DSI host, we need the >>>>> driver to create a mipi_dsi_device DSI device that attaches to the >>>>> host. >>>>> >>>>> Use of_graph helpers to get the DSI host DT node. Create a MIPI DSI >>>>> device using this host. Finally, attach this device to the DSI host. >>>>> >>>>> Populate DT parameters (number of data lanes for now) that are required >>>>> for DSI RX to work correctly. Hardcode few other parameters (rgb, >>>>> embedded_sync) for now. >>>>> >>>>> Signed-off-by: Archit Taneja <architt at codeaurora.org> >>>> >>>> This adds a hard dependency on CONFIG_DRM_MIPI_DSI, otherwise the >>>> kernel won't link. As the ADV7511 doesn't require DSI, could you make it >>>> optional with conditional compilation to avoid pulling in dead code ? >>> >>> You're right. The driver's Kconfig should at least select DRM_MIPI_DSI >>> in the current state to make sure we don't break build. >>> >>> Do you suggest we create another config option for ADV7533, which >>> selects DRM_MIPI_DSI and builds the ADV7533 parts? Or did you mean >>> something else? >> >> Do you have any suggestions for this point? For the next revision, I've >> just selected DRM_MIPI_DSI in the Kconfig to ensure build doesn't break. >> >> For this driver, to conditionally compile DRM_MIPI_DSI, it essentially >> means we allow conditional support for ADV7533. I would imagine us >> #ifdef-ing out the ADV7533 compatible string in the of_match_table. Is >> this something we want to do? > > If it's not much trouble I think that would be useful to avoid bloating the > kernel with unused features, yes. You might want to add stub functions to > include/drm/drm_mipi_dsi.h when CONFIG_DRM_MIPI_DSI is not defined to avoid > sprinkling the driver with lots of #ifdefs.
Yeah, I can do this. Now that there isn't a chance for it to get in the 4.7 merge window, there's plenty time. Archit > >>>>> --- >>>>> >>>>> drivers/gpu/drm/i2c/adv7511.c | 130 +++++++++++++++++++++++++++++++++--- >>>>> 1 file changed, 123 insertions(+), 7 deletions(-) > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
