linux-kernel - Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.

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]

Date:	Mon, 11 Nov 2013 15:39:57 +0100
From:	"Dr. H. Nikolaus Schaller" <hns@...delico.com>
To:	Tomi Valkeinen <tomi.valkeinen@...com>
Cc:	Belisko Marek <marek.belisko@...il.com>,
	Tony Lindgren <tony@...mide.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	<linux-fbdev@...r.kernel.org>, NeilBrown <neilb@...e.de>,
	Marek Belisko <marek@...delico.com>
Subject: Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.


Am 11.11.2013 um 15:13 schrieb Tomi Valkeinen:

> On 2013-11-11 15:57, Dr. H. Nikolaus Schaller wrote:
>> Hi Tomi,
>> 
>> Am 11.11.2013 um 14:29 schrieb Tomi Valkeinen:
>> 
>>> Hi,
>>> 
>>> On 2013-11-05 09:24, Belisko Marek wrote:
>>>> Hi,
>>>> 
>>>> ping.
>>>> 
>>>> On Mon, Oct 14, 2013 at 11:02 PM, Marek Belisko <marek@...delico.com> wrote:
>>>>> This patches is adding bypass and acbias functionality to omapdss venc driver.
>>>>> In first patch we export updatin bypass and acbias in devconf1 register. Next patch
>>>>> add handling for updating in venc driver and last patch add driver for opa362 which
>>>>> is used on gta04 board and set bypass + acbias.
>>>> Is there a chance to get this series to 3.13? Thanks.
>>> 
>>> Sorry, I haven't had time to do much reviewing.
>>> 
>>> The code in omap3-tvout.c should be included in the display.c file,
>>> which already contains some things like muxing.
>> 
>> Ok, that might be better - but we were not sure where to place code to
>> modify the DEVCONF1 registers. It is not directly DSS related but a special
>> control of the OMAP3 SoC. Therefore we think it is better located in omap3-tvout.c
> 
> The display.c file is not strictly DSS stuff, but DSS related "glue" for
> the SoC. For example, the muxing of the DSI pads is also done on the
> CONTROL module, and it's also in display.c.
> 
> The file is getting a bit large, so I'm not against splitting it. But I
> don't think there's point to add omap3-tvout.c file, which most likely
> will ever contain only that one function.
> 
>>> Also, func(bool, bool)
>>> style functions are rather confusing to read. Maybe an enum would be
>>> better, so you'd instead have something like:
>>> 
>>> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN)
>> 
>> We have no special preference on that.
> 
> It's just about readability. Which one tells you better what the code does:
> 
> func(true, true);
> 
> or
> 
> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN);
> 
>>> But the main issue is: while this series probably works well, I really
>>> don't like it that the OPA driver needs to pass bypass and acbias. It
>>> shouldn't know anything about such things. I'm just not certain how to
>>> implement that with the current omapdss driver.
>> 
>> Well, it must know bypass and acbias because it requires the OMAP
>> CPU to be configured accordingly. I.e. it is the one who pushes the
>> button. Or we get a problem that people might misconfigure it.
> 
> While the omapdss display drivers are currently OMAP specific, I want to
> (or at least try to) keep them platform independent. Afaik, acbias and
> bypass are OMAP VENC specific things, not something that every SoC with
> an analog TV-out have. Thus, the OPA driver should not know about them,
> and it should be something that only the DSS glue code in mach-omap2/
> and the venc driver know about.

What about calling it prepare_venc_for_external_amplifier (or similar). Then,
venc.c can hide that really has to be done and call whatever SoC API is
needed to program DEFCONF1?

> 
>> I would see it similar as a driver must be able to control power. Here
>> it must be able to control bypass and acbias in a special way that it works.
> 
> The difference is that on all possible SoCs where you could add OPA
> chip, you'll need to manage the power for OPA, and it's done in a common
> way. Whereas the bypass and acbias are OMAP specific things.
> 
>>> I'll try to find time to think about this more, but I don't think I can
>>> merge this for 3.13.
>> 
>> Please take your time.
>> 
>> And please also consider the approach to merge it into 3.13 as it is
>> and we can then fix it in the weeks while release candidates are pushed.
> 
> If it was purely omapdss code, I could possibly take it. But it contains
> arch/arm code also, and I don't want to cause needless changes on code
> that I do not maintain.
> 
> Tomi
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/