On 21/04/2019 00:29, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Tue, Mar 26, 2019 at 12:31:32PM +0200, Tomi Valkeinen wrote:
>> It is nicer to have enable/disable functions instead of set(bool enable)
>> style function.
>
> When the two functions have nothing in common, yes.
>
>> Split tc_main_link_stream into tc_stream_enable and tc_stream_disable.
>
> Should you keep the tc_main_link_ prefix ? I suppose it is implied in a
> way, as the stream is carried over the main link.
It sounds a bit redundant, only making the functions names longer.
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>> ---
>> drivers/gpu/drm/bridge/tc358767.c | 81 +++++++++++++++++--------------
>> 1 file changed, 45 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index 86b272422281..bfc673bd5986 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -1013,47 +1013,56 @@ static int tc_main_link_setup(struct tc_data *tc)
>> return ret;
>> }
>>
>> -static int tc_main_link_stream(struct tc_data *tc, int state)
>> +static int tc_stream_enable(struct tc_data *tc)
>> {
>> int ret;
>> u32 value;
>>
>> - dev_dbg(tc->dev, "stream: %d\n", state);
>> + dev_dbg(tc->dev, "stream enable\n");
>
> Maybe "enable video stream\n" (and similarly for the tc_stream_disable()
> function) ?
Ok.
>>
>> - if (state) {
>> - ret = tc_set_video_mode(tc, tc->mode);
>> - if (ret)
>> - goto err;
>> + ret = tc_set_video_mode(tc, tc->mode);
>> + if (ret)
>> + goto err;
>
> Let's return ret directly and remove the err label.
Can't remove the err label, because of the tc_write() calls...
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
