i.MX6 HDMI framer driver

[gonzo]

Summary

This diff was a part of D4168 review. Per Andrew's suggestion it was split in two parts: HDMI and IPU

[andrew]

sys/arm/freescale/imx/imx6_hdmi.c

631–635

Why set up the interrupt if you're not using it?

sys/arm/freescale/imx/imx6_hdmireg.h

68

There is some inconsistent indentation in this file.

[gonzo]

• Indentation cleanup in imx6_hdmireg.h
• Removed unused interrupt stuff

[andrew]

sys/arm/freescale/imx/imx6_hdmi.c

97

Why not uint8_t?

119–120

Why the cast?

158–160

Is the todo to get it working on something other than DVI mode?

371–372

Extra newline

437–439

What are these magic numbers?

651

You're setting this here and on line 676.

654

Why track this if you pass 0 to bus_release_resource?

724

return (hdmi_edid_read(...));

sys/arm/freescale/imx/imx6_hdmireg.h

30

#ifndef should have a space (I have no idea why the style is different between this and #define).

[gonzo]

Address Andrew's comments

[gonzo]

Rest of the comments were addressed in new revision

sys/arm/freescale/imx/imx6_hdmi.c

158–160

Yes, for now it's only DVI mode. Next stage is going to be adding more real HDMI stuff

437–439

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.

[• ian]

sys/arm/freescale/imx/imx6_hdmi.c

101

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.

558

missing empty line

603

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.

724

return (hdmi_edid_read(device_get_softc(dev), ...) :)

sys/arm/freescale/imx/imx6_hdmireg.h

68

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. :)

sys/arm/freescale/imx/imx_i2c.c

321

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.

[mmel]

sys/arm/freescale/imx/imx_i2c.c

321

Right. And registration of ofw_iicbus was added in r292157.

[gonzo]

Address latest round of review comments

[gonzo]

sys/arm/freescale/imx/imx6_hdmi.c

605

iicbus_transfer_excl works only for devices that are immediate children of iicbus, so I replaced it with iic_require_bus et al.

sys/arm/freescale/imx/imx_i2c.c

321

Thanks for committing it upstream, I removed this workaround

[andrew]

sys/arm/freescale/imx/imx6_hdmi.c

158–160

Ok, the comment make it sound like DVI still needs to be done.

619–620

Extra newline.