[PATCH V6 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Marek Vasut]

On 6/8/21 11:42 AM, Robert Foss wrote:
> On Tue, 8 Jun 2021 at 11:34, Marek Vasut <marex at denx.de> wrote:
>>
>> On 6/8/21 10:51 AM, Robert Foss wrote:
>>> Hey Neil,
>>>
>>> On Tue, 8 Jun 2021 at 10:10, Neil Armstrong <narmstrong at baylibre.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 07/06/2021 19:42, Marek Vasut wrote:
>>>>> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
>>>>> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
>>>>> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
>>>>> but easy to add.
>>>>>
>>>>> The driver operates the chip via I2C bus. Currently the LVDS clock are
>>>>> always derived from DSI clock lane, which is the usual mode of operation.
>>>>> Support for clock from external oscillator is not implemented, but it is
>>>>> easy to add if ever needed. Only RGB888 pixel format is implemented, the
>>>>> LVDS666 is not supported, but could be added if needed.
>>>>>
>>>>> Reviewed-by: Linus Walleij <linus.walleij at linaro.org>
>>>>> Reviewed-by: Frieder Schrempf <frieder.schrempf at kontron.de>
>>>>> Tested-by: Frieder Schrempf <frieder.schrempf at kontron.de>
>>>>> Tested-by: Adam Ford <aford173 at gmail.com>
>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>> Cc: Douglas Anderson <dianders at chromium.org>
>>>>> Cc: Jagan Teki <jagan at amarulasolutions.com>
>>>>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>>> Cc: Linus Walleij <linus.walleij at linaro.org>
>>>>> Cc: Loic Poulain <loic.poulain at linaro.org>
>>>>> Cc: Philippe Schenker <philippe.schenker at toradex.com>
>>>>> Cc: Sam Ravnborg <sam at ravnborg.org>
>>>>> Cc: Stephen Boyd <swboyd at chromium.org>
>>>>> Cc: Valentin Raevsky <valentin at compulab.co.il>
>>>>> To: dri-devel at lists.freedesktop.org
>>>>> ---
>>>>> V2: - Use dev_err_probe()
>>>>>       - Set REG_RC_RESET as volatile
>>>>>       - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
>>>>>       - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
>>>>>       - Add tested DSI84 support in dual-link mode
>>>>>       - Correctly set VCOM
>>>>>       - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
>>>>>         datasheets, with that all the reserved bits make far more sense
>>>>>         as the DSI83 and DSI84 seems to be reduced version of DSI85
>>>>> V3: - Handle the dual-link LVDS with two port panel or bridge
>>>>> V4: - Add RB from Linus Walleij
>>>>>       - Rename REG_DSI_LANE_LVDS_LINK_CFG_DUAL to
>>>>>         REG_DSI_LANE_DSI_CHANNEL_MODE_SINGLE and fill in the remaining
>>>>>         DSI link options from DSI85 datasheet. DSI85 can do dual and 2x
>>>>>         single DSI mode, but DSI85 is currently unsupported by the
>>>>>         driver. Add a comment about DSI85, so that all the places which
>>>>>         need to be adjusted for DSI85 are marked accordingly.
>>>>>       - Add REG_DSI_LANE_LEFT_RIGHT_PIXELS bit for DSI
>>>>>       - Add handling for JEIDA18/JEIDA24/SPWG24 LVDS formats. Use SPWG24
>>>>>         as fallback on output bridges until they are all fixed.
>>>>>       - Patch DSI bus format to fixed RGB888_1X24 instead of passing
>>>>>         through the LVDS bus format.
>>>>> V5: - Move bus format patching to mode_fixup
>>>>>       - Use cpu_to_le16() to guarantee endianness in regmap_bulk_write()
>>>>> V6: - Cast of_device_get_match_data() output to uintptr_t first
>>>>> ---
>>>>>    drivers/gpu/drm/bridge/Kconfig        |  10 +
>>>>>    drivers/gpu/drm/bridge/Makefile       |   1 +
>>>>>    drivers/gpu/drm/bridge/ti-sn65dsi83.c | 709 ++++++++++++++++++++++++++
>>>>>    3 files changed, 720 insertions(+)
>>>>>    create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c
>>>>>
>>>>
>>>> Looks complete & well reviewed, LGTM
>>>>
>>>> Rob, Laurent ? is it ok for you ?
>>>
>>> This does look like it is ready. I was just about to merge it, but
>>> found some checkpatch --strict formatting warnings. Do you mind fixing
>>> those in a real quick v7?
>>
>> We already discussed this thing with open parenthesis alignment before
>> in one of the previous versions of this series, and the readability of
>> the driver was worse with that change in place.
>>
>> Is this change now a blocker ?
> 
> Ah, sorry I missed that part of the conversation. In that case no.
> Let's go ahead.

Nevermind, I sent V7. Apparently line over 80 is no longer an issue and 
line over 100 is, so the result doesn't look so bad. Pick the V7 please.