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@xxxxxxxxxxxxx> 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-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
