This diff was a part of D4168 review. Per Andrew's suggestion it was split in two parts: HDMI and IPU
Why not uint8_t?
Why the cast?
Is the todo to get it working on something other than DVI mode?
What are these magic numbers?
You're setting this here and on line 676.
Why track this if you pass 0 to bus_release_resource?
#ifndef should have a space (I have no idea why the style is different between this and #define).
Rest of the comments were addressed in new revision
Yes, for now it's only DVI mode. Next stage is going to be adding more real HDMI stuff
Some magic numbers from HDMI spec. They're not combination of flags - they're bits to fill data lines that are not used during certain kind of transfers. I don't knwo why these particular numbers are used.
I can't really tell what the runtime context is here, but if sleeping is allowed then pause() would be better than delay -- an imx6 can get a lot of other work done in a millisecond.
missing empty line
Is this a dedicated bus so that you can be sure that 2 different threads will never try to access the bus at the same time? if not, use iicbus_transfer_excl() to take ownership of the bus during the transaction.
return (hdmi_edid_read(device_get_softc(dev), ...) :)
When doing this style where the bit definitions follow the register name/addr I like to indent the bit names with a tab plus 2 spaces instead of 2 tabs, it keeps it from going beyond col 80 so fast. (and style(9) requires a tab but doesn't say you *can't* add a couple spaces after it. :)
This doesn't seem right. The iic device exists just to provide /dev/iicN for userland access.
I think sc->iicbus should be registered as the provider for the xref, because the usage is iicbus_transfer() which wants the device_t of the bus and calls IICBUS_TRANSFER(device_get_parent(bus)) which gets control into this driver.
If it's working with the iic device, it seems like it must be by accident.