On Tue, May 20, 2014 at 03:56:15PM +0200, Benjamin Gaignard wrote:
> Add driver to support analog TV ouput.
Unfortunate that this has the same abbreviation as High-Definition
Audio...
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
[...]
> +/* Video DACs control */
> +#define VIDEO_DACS_CONTROL_MASK 0x0FFF
> +#define VIDEO_DACS_CONTROL_SYSCFG2535 0x085C /* for stih416 */
> +#define DAC_CFG_HD_OFF_SHIFT 5
> +#define DAC_CFG_HD_OFF_MASK (0x7 << DAC_CFG_HD_OFF_SHIFT)
> +#define VIDEO_DACS_CONTROL_SYSCFG5072 0x0120 /* for stih407 */
syscfg is starting to look more and more like it could be a syscon
driver or some other specialized, platform-specific driver.
> +/* Index (in supported modes table) of the preferred video mode */
> +#define HDA_PREF_MODE_IDX 0
I don't think this symbolic name is particularly useful.
> +/* Reference to the hda device */
> +struct device *hda_dev;
This shouldn't be needed.
> +static void sti_hda_configure_awg(struct sti_hda *hda, u32 *awg_instr, int nb)
> +{
> + int i;
unsigned
> +/*
> + * Get modes
> + *
> + * @drm_connector: pointer on the drm connector
> + *
> + * Return number of modes
> + */
> +static int sti_hda_get_modes(struct drm_connector *drm_connector)
> +{
> + struct drm_device *dev = drm_connector->dev;
> + struct drm_display_mode *mode;
> + int i, count;
unsigned for i. count should probably stay signed since DRM uses that
throughout.
> +
> + DRM_DEBUG_DRIVER("\n");
> +
> + for (i = 0, count = 0; i < ARRAY_SIZE(hda_supported_modes); i++) {
You can initialize count when it's declared to make the loop more
idiomatic.
> + mode = drm_mode_duplicate(dev, &hda_supported_modes[i].mode);
> + if (!mode)
> + continue;
> + mode->vrefresh = drm_mode_vrefresh(mode);
> +
> + /* Set the preferred mode */
> + if (i == HDA_PREF_MODE_IDX)
> + mode->type |= DRM_MODE_TYPE_PREFERRED;
> +
> + drm_mode_probed_add(drm_connector, mode);
> + count++;
> + }
> +
> + drm_mode_sort(&drm_connector->modes);
> +
> + return count;
> +}
> +
> +
Gratuituous newline.
> +static int sti_hda_probe(struct platform_device *pdev)
> +{
[...]
> +}
> +
> +static int sti_hda_remove(struct platform_device *pdev)
> +{
[...]
> +}
> +
> +static struct of_device_id hda_match_types[] = {
> + {
> + .compatible = "st,stih416-hda",
> + },
> + {
> + .compatible = "st,stih407-hda",
> + },
> + { /* end node */ }
> +};
> +MODULE_DEVICE_TABLE(of, hda_match_types);
> +
> +struct platform_driver sti_hda_driver = {
> + .driver = {
> + .name = "sti-hda",
> + .owner = THIS_MODULE,
> + .of_match_table = hda_match_types,
> + },
> + .probe = sti_hda_probe,
> + .remove = sti_hda_remove,
> +};
> +
> +module_platform_driver(sti_hda_driver);
> +
> +MODULE_LICENSE("GPL");
Same comments here as for all previous patches.
Thierry
Attachment:
pgpdWTqK3YJ4z.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
