At Tue, 4 Nov 2008 17:38:39 +0800, Wu Fengguang wrote:
Add support for Intel G45 integrated HDMI audio codecs.
This initial release supports:
- 2 channel stereo sound output
- report monitor's ELD information
Signed-off-by: Wu Fengguang wfg@linux.intel.com
Thanks. The patch looks almost fine, but a few things to be fixed.
+/*
- ELD: EDID Like Data
- */
+struct sink_eld {
- int eld_size;
- int baseline_len;
- int eld_ver; /* (eld_ver == 0) indicates invalid ELD */
- int cea_edid_ver;
- char monitor_name[ELD_MAX_MNL + 1];
- int manufacture_id;
- int product_id;
- long port_id;
If you need a 64bit value, use u64 explicitly.
+struct hdmi_audio_infoframe {
- u8 type; /* 0x84 */
- u8 ver; /* 0x01 */
- u8 len; /* 0x0a */
- u8 checksum; /* PB0 */
- u8 CC02_CT47; /* CC in bits 0:2, CT in 4:7 */
- u8 SS01_SF24;
- u8 CXT04;
- u8 CA;
- u8 LFEPBL01_LSV36_DM_INH7;
- u8 reserved[5]; /* PB6 - PB10 */
+};
+#define GRAB_BITS(buf, byte, lowbit, bits, val) \
- (val) = ((buf)[(byte)] >> (lowbit)) & ((1 << (bits)) - 1)
This should be rather a form
#define GRAB_BITS(buf, byte, lowbit, bits) \ ((buf)[(byte)] >> (lowbit)) & ((1 << (bits)) - 1) and
foo = GRAB_BITS(buf, byte, low, bits);
+static void hdmi_update_short_audio_desc(struct cea_sad *a,
const unsigned char *buf)+{
- int i;
- int val;
- GRAB_BITS(buf, 1, 0, 7, val);
The numbers is GRAB_BITS() could be better defined.
+static int hdmi_update_sink_eld(struct hda_codec *codec,
const unsigned char *buf, int size)
(snip)
- e->port_id = le64_to_cpu(*(u64 *)(buf + 8));
Use get_unaligned_*() for this, just to be more portable. (And include <asm/unaligned.h> beforehand.)
- /* the spec's tendency is little endian */
- e->manufacture_id = le16_to_cpu(*(u16 *)(buf + 16));
- e->product_id = le16_to_cpu(*(u16 *)(buf + 18));
Ditto.
- if (mnl > ELD_MAX_MNL) {
snd_printd(KERN_INFO "MNL is reserved value %d\n", mnl);goto out_fail;- } else if (ELD_FIXED_BYTES + mnl > size) {
snd_printd(KERN_INFO "out of range MNL %d\n", mnl);goto out_fail;- } else {
strncpy(e->monitor_name, buf + ELD_FIXED_BYTES, mnl);e->monitor_name[mnl] = '\0';
Use strlcpy().
+static void hdmi_show_eld(struct hda_codec *codec) +{
- int i;
- int j;
- struct intel_hdmi_spec *spec = codec->spec;
- struct sink_eld *e = &spec->sink;
- char buf[80];
(snip)
- for (i = 0; i < ARRAY_SIZE(cea_speaker_allocation_names); i++) {
if (e->spk_alloc & (1 << i))j += sprintf(buf + j, " %s",cea_speaker_allocation_names[i]);
Use snprintf() instead to avoid overflow.
- }
- buf[j] = '\0';
- printk(KERN_INFO "speaker allocations: (0x%x)%s\n", e->spk_alloc, buf);
- for (i = 0; i < e->sad_count; i++) {
hdmi_show_short_audio_desc(e->sad + i);- }
checkpatch.pl warns about these braces. In general, it's worth to run once before submission.
Could you fix these and repost?
thanks,
Takashi