[alsa-devel] [PATCH v5 01/14] ASoC: hdac_hdmi: Add hotplug notification and read eld

[Mark Brown]

On Wed, Feb 10, 2016 at 10:03:58AM +0530, Vinod Koul wrote:
> On Tue, Feb 09, 2016 at 07:42:06PM +0000, Mark Brown wrote:
> > On Sat, Jan 30, 2016 at 07:13:35PM +0530, Subhransu S. Prusty wrote:

> > > +	if (!pin->eld.monitor_present || !pin->eld.eld_valid) {
> > > +
> > > +		dev_info(&edev->hdac.dev, "%s: disconnect for pin %d\n",
> > > +						__func__, pin->nid);
> > > +		goto put_hdac_device;

> > Will this get noisy?

> Not really, we print when we get the hotplug notification. Usually that wont
> happen too often unless someone is moneky testing :)

That sounds like it might get old very quickly, we presumably already
have the graphics stack doing the same thing.

> > > +
> > > +			print_hex_dump_bytes("Eld: ", DUMP_PREFIX_OFFSET,
> > > +					pin->eld.eld_buffer, pin->eld.eld_size);

> > ELD is an acronym, please write it properly (there's some other examples
> > in the patch).

> ELD is quite often used acronym in HDMI world, It is already there in
> drivers (did a quick grep :)

I know it's widely used, I'm saying you should write it properly.

> > > +		} else {
> > > +			dev_err(&edev->hdac.dev, "ELD invalid\n");
> > > +			pin->eld.monitor_present = false;
> > > +			pin->eld.eld_valid = false;
> > > +		}

> > Is it really invalid or did we fail to read it?  There's a difference as
> > far as debugging is concerned (one means there's an I/O problem, the
> > other means that the display is reporting nonsense).  Perhaps this
> > should be silent and let the read function be more specific?

> There is no way to know if this was IO or garbage from display. So we just
> throw up error and retry few times.

It seems implausible that there is no way of identifying I/O errors, and
even if that is the case the point about the message not being terribly
helpful still stands - moving the reporting to the point where we decide
there is an error would be a lot more helpful.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160210/a0fa15fe/attachment.sig>