On 05/20/16 16:21, Tomi Valkeinen wrote:
>> > +
>> > +static int dispc_errata_i734_wa_init(void)
>> > +{
>> > +	size_t buff_size = i734.ovli.width * i734.ovli.height *
> "buf_size" would match the other names =).
> 
> Usually I try to do only simple initializations when declaring the
> locals. This is a bit on the complex side, and also there's a check
> below that can make all this init calculation not needed.
> 

Compiler optimizations should delay the initialization to happen only if
it is needed.

I usually try to keep the functions as compact as possible so that they
fit on the screen at once. But I think it is better to move the size
also inside the struct i734_buf so it we only need to calculate the size
once and functions become even smaller :).

...
>> > +
>> > +static void dispc_errata_i734_wa(void)
>> > +{
>> > +	u32 vsync_irq = dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_LCD);
>> > +	u32 framedone_irq = dispc_mgr_get_framedone_irq(OMAP_DSS_CHANNEL_LCD);
>> > +	u32 gatestate = REG_GET(DISPC_CONTROL, 8, 4);
> Here too, too complex initializations. Especially doing register
> reads/writes should be something done in the "real" code. The bits you
> read might not even be valid on some platforms which don't have the i734.
> 

You are right about the register read. Compiler should not optimize the
register access. But - but with your permission - I'll keep the IRQ mask
initializations as they are quite constant in nature anyway.

BR,
Jyri