[alsa-devel] [PATCH v3 2/6] ASoC: hdac-hdmi: Add hdmi driver

[Mark Brown]

On Mon, Nov 02, 2015 at 03:36:42PM +0530, Vinod Koul wrote:
> On Sun, Nov 01, 2015 at 12:02:31PM +0900, Mark Brown wrote:
> > On Tue, Oct 27, 2015 at 04:42:13PM +0900, Vinod Koul wrote:

> > > +	if (err < 0)
> > > +		dev_err(&hdac->dev, "Failed to query pcm params for nid: %d\n", cvt->nid);

> > That looks like the NID is being printed as an error code.

> No this prints the NID for which query fails, but agree we should print the
> error code too, that might be very helpful :)

No, it's also the format for the message - normally we do "message:
code".

> > > +	/*
> > > +	 * Currently on board only 1 pin and 1 converter enabled for
> > > +	 * simplification, more will be added eventually
> > > +	 * So using fixed map for dai_id:pin:cvt
> > > +	 */
> > > +	return hdac_hdmi_init_dai_map(edev, &hdmi->dai_map[0], hdmi->pin_nid[0],
> > > +			hdmi->cvt_nid[0], 0);

> > I'm not entirely sure I understand what this is all doing.  It looks
> > like it's trying to translate the HDA widget map into a DAPM map which
> > seems sensible but it appears it's making some simplifying assumptions
> > about the device it's dealing with?

> The device is actually quite simple and yes we simplified even further by
> ignoring multiple pins for now. We will keep adding more features and adding
> stuff to map as we go along..

That's not giving me a clear picture of what the code is doing or how
we're working with the device - we've got some code parsing the HDA
graph, some code hard coding things and I don't really know why or how
anything fits together so I'm a bit nonplussed about what I'm reviewing
here.
-------------- 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/20151104/11e55be8/attachment-0001.sig>