On Wed, Feb 10, 2016 at 07:23:28AM +0000, Mark Brown wrote:
> 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.
Last I did not see that, but yes lets make it less noisy, will move this to
debug
>
> > > > +
> > > > + 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.
Okay, I think your concern is on writing, sorry I though about the term,
will fix it now :)
> > > > + } 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.
Hrmmm, looking back and checking the get_eld does print more verbose
information on the failure, so this one is really not required, we will
remove this
Thanks
--
~Vinod
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
