On 26/06/2024 06:47, Dmitry Baryshkov wrote:
> On Tue, Jun 25, 2024 at 06:38:13PM GMT, Marc Gonzalez wrote:
>
>> The TI TDP158 is an AC-Coupled HDMI signal to TMDS Redriver supporting
>> DVI 1.0 and HDMI 1.4b and 2.0b output signals.
>>
>> The default settings work fine for our use-case.
>>
>> Signed-off-by: Marc Gonzalez <mgonzalez@xxxxxxxxxx>
>> ---
>> drivers/gpu/drm/bridge/Kconfig | 6 +++
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/ti-tdp158.c | 103 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 110 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index c621be1a99a89..0859f85cb4b69 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -368,6 +368,12 @@ config DRM_TI_DLPC3433
>> It supports up to 720p resolution with 60 and 120 Hz refresh
>> rates.
>>
>> +config DRM_TI_TDP158
>> + tristate "TI TDP158 HDMI/TMDS bridge"
>> + depends on OF
>> + help
>> + Texas Instruments TDP158 HDMI/TMDS Bridge driver
>
> Please run ./scripts/checkpatch.pl on your patch.
Oops, sorry, will do.
For the record, I did run:
$ make -j16 dt_binding_check DT_SCHEMA_FILES=ti,tdp158.yaml
>> + if ((err = regulator_enable(tdp158->vcc)))
>> + pr_err("%s: vcc: %d", __func__, err);
>
> - dev_err
> - please expand error messages
> - ERROR: do not use assignment in if condition
simple-bridge.c uses DRM_ERROR, but it says:
"NOTE: this is deprecated in favor of pr_err()"
Hence, I used pr_err.
Are you saying I need to record the dev,
just to be able to call dev_err?
> empty line
Will do.
>> + return drm_bridge_attach(bridge->encoder, tdp158->next, bridge, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>
> No. Pass flags directly.
Oh, just pass the flags argument here? OK.
>> + tdp158->next = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
>
> Missing `select DRM_PANEL_BRIDGE`
OK.
>> + if (IS_ERR(tdp158->next))
>> + return dev_err_probe(dev, PTR_ERR(tdp158->next), "next");
>
> This results in a cryptic message 'foo: ESOMETHING: next'. Please
> expand.
OK.
Thanks for the in-depth review & suggestions.
Will respin with fixes.
Regards
