[PATCH 4/4] drm: zte: add VGA driver support

[Shawn Guo]

On Mon, Apr 03, 2017 at 11:23:51AM +0200, Daniel Vetter wrote:
> > +static int zx_vga_ddc_register(struct zx_vga *vga)
> > +{
> > +	struct device *dev = vga->dev;
> > +	struct i2c_adapter *adap;
> > +	struct zx_vga_i2c *ddc;
> > +	int ret;
> > +
> > +	ddc = devm_kzalloc(dev, sizeof(*ddc), GFP_KERNEL);
> > +	if (!ddc)
> > +		return -ENOMEM;
> > +
> > +	vga->ddc = ddc;
> > +	mutex_init(&ddc->lock);
> > +
> > +	adap = &ddc->adap;
> > +	adap->owner = THIS_MODULE;
> > +	adap->class = I2C_CLASS_DDC;
> > +	adap->dev.parent = dev;
> > +	adap->algo = &zx_vga_algorithm;
> > +	snprintf(adap->name, sizeof(adap->name), "zx vga i2c");
> > +
> > +	ret = i2c_add_adapter(adap);
> > +	if (ret) {
> > +		DRM_DEV_ERROR(dev, "failed to add I2C adapter: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	i2c_set_adapdata(adap, vga);
> 
> Since this really isn't a full-featured i2c adapter but a specialized
> "give me the edid" thing I think you should look into drm_do_get_edid and
> _not_ expose the i2c thing. That should simplify your code a lot, and
> avoid any kind of synchronization issues between userspace i2c access and
> your driver's access (which you atm don't handle at all).

I'm a bit confused here.  Could you give me some more details on how to
know it's a full-featured I2C bus or specialized "give me the edid" one?
My understanding is that the ZTE hardware for reading HDMI and VGA EDID
are different but pretty similar.  But in HDMI driver review [1], you
gave the opposite comment, asking to change to I2C adapter from
drm_do_get_edid().  I'm wondering how you see that the hardware in HDMI
is a full-featured I2C bus, while the one in VGA is a specialized bus?

Shawn

[1] https://patchwork.kernel.org/patch/9349195/