Re: [PATCH 3/3] Add handling CMP output connection

[Clemens Ladisch <clemens@ladisch.de>]

Takashi Sakamoto wrote:
> To handle CMP output connection, this patch adds some macros, codes with
> condition of direction and new functions. Once cmp_connection_init() is
> executed with its direction, CMP input and output connection can be
> handled by the same way.

> +++ b/sound/firewire/cmp.c

> +	if (c->direction == CMP_INPUT)
> +		offset = CSR_REGISTER_BASE + CSR_IPCR(c->pcr_index);
> +	else
> +		offset = CSR_REGISTER_BASE + CSR_OPCR(c->pcr_index);

This code is used twice and could be moved into a helper function.

> +static int get_overhead_id(struct cmp_connection *c)

> +	/*
> +	 * apply "oPCR overhead ID encoding"
> +	 * the encoding table can convert up to 512.
> +	 * here the value over 512 is converted as the same way as 512.
> +	 */

	/*
	 * Apply "oPCR overhead ID encoding":
	 * The encoding table can convert up to 512.
	 * Here any value over 512 is converted in the same way as 512.
	 */

> +	for (id = 1; id < 16; id += 1) {
> +		if (c->resources.bandwidth_overhead < (id << 5))
> +			break;
> +	}
> +	if (id == 16)
> +		id = 0;

	id = DIV_ROUND_UP(c->resources.bandwidth_overhead, 32);
	if (id >= 16)
		id = 0;

> +static __be32 opcr_set_modify(struct cmp_connection *c, __be32 opcr)

> +	/* generate speed and extended speed field value */

This comment is superfluous; it does not tell anything non-obvious about
the code.

> +	/*
> +	 * here zero is applied to payload field.
> +	 * it means the maximum number of quadlets in an isochronous packet is
> +	 * 1024 when spd is less than three, 1024 * 2 * xspd + 1 when spd is
> +	 * equal to three. An arbitrary value can be set here but 0 is enough
> +	 * for our purpose.
> +	 */
> +	opcr |= cpu_to_be32(0 << OPCR_PAYLOAD_SHIFT);

In an oPCR, the payload field is changed only by the device.


Regards,
Clemens