linux-kernel - Re: [linux-sunxi] Re: [PATCH v3 04/14] drm/sun4i: tcon: Add support for demuxing TCON output on A31

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGb2v65Kas9hB4bnE7z=RC7wQ7_DvMgfm8V0aoifNsnqNfbcoA@mail.gmail.com>
Date:   Mon, 2 Oct 2017 15:11:37 +0800
From:   Chen-Yu Tsai <wens@...e.org>
To:     Julian Calaby <julian.calaby@...il.com>
Cc:     Chen-Yu Tsai <wens@...e.org>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>,
        Mark Brown <broonie@...nel.org>,
        David Airlie <airlied@...ux.ie>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-clk <linux-clk@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-sunxi <linux-sunxi@...glegroups.com>
Subject: Re: [linux-sunxi] Re: [PATCH v3 04/14] drm/sun4i: tcon: Add support
 for demuxing TCON output on A31

On Sat, Sep 30, 2017 at 2:26 PM, Julian Calaby <julian.calaby@...il.com> wrote:
> Hi Chen-Yu,
>
> On Sat, Sep 30, 2017 at 3:58 PM, Chen-Yu Tsai <wens@...e.org> wrote:
>> On Sat, Sep 30, 2017 at 1:35 PM, Julian Calaby <julian.calaby@...il.com> wrote:
>>> Hi Chen-Yu,
>>>
>>> On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <wens@...e.org> wrote:
>>>> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard
>>>> <maxime.ripard@...e-electrons.com> wrote:
>>>>> On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote:
>>>>>> On systems with 2 TCONs such as the A31, it is possible to demux the
>>>>>> output of the TCONs to one encoder.
>>>>>>
>>>>>> Add support for this for the A31.
>>>>>>
>>>>>> Signed-off-by: Chen-Yu Tsai <wens@...e.org>
>>>>>> ---
>>>>>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 38 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>>> index 7bf51abaee97..c949309d4285 100644
>>>>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable)
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>>>>>>
>>>>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
>>>>>
>>>>> Would that make sense to make it a bit more generic, and pass the id
>>>>> to look for as an argument?
>>>>
>>>> The reason to look for TCON0 explicitly is to access the muxing registers, which
>>>> are only available in TCON0. Other than that, there's nothing else
>>>> shared between
>>>> the two TCONs. So there's no particular reason to look for TCON1 explicitly.
>>>
>>> In that case: in the bizarre case where we're trying to use this mux
>>> type and there is no TCON0, shouldn't we fail?
>>
>> It gives out a big warning, indicating something is wrong. If TCON0 is not found
>> it is most likely your device tree is broken. There's nothing more the
>> driver can do.
>> Are you suggesting to return NULL in this case, and also do error
>> handling in the
>> callers?
>
> You're already returning -EINVAL for other failure cases, so a lack of
> TCON0 might as well do the same.
>
>>> (Also, the code doesn't make sense if we have some TCON1 and TCON2 in
>>> that order as it'll return TCON2)
>>
>> I'm guessing you want it to return NULL.
>
> I'm just pointing out the mismatch between getting the "first" TCON
> and the actual behaviour.

Makes sense. I've renamed it to "_get_tcon0" and added more comments
on it's behavior. Also made it return NULL when tcon0 is not found.

ChenYu