HVSI serial console review (2.6 patch)

[Greg KH]

On Mon, Apr 12, 2004 at 05:24:02PM -0500, Hollis Blanchard wrote:
> +static void dump_packet(struct hvsi_header *pkt)
> +{
> +	int i;
> +	char *data = payload(pkt);
> +
> +	printk("type 0x%x, len %i, seqno %i:", pkt->type, pkt->len, pkt->seqno);

Yeah, I see you are relying on the printk() that happens right before
this to give you the proper KERN_ level, but then you loose that level
with:

> +
> +	if (len_payload(pkt))
> +		printk("\n     ");

That one.  Why put spaces after a \n if you aren't going to set the
level?

> +static struct hvsi_header *search_for_packet(struct vtty_struct *vtty, int type)
> +{
> +	/* bring in queued packets */
> +	hvsi_load_buffers(vtty);
> +
> +	/* look for the version query response packet */
> +	for (; read != write; read = next_desc(read)) {
> +		struct hvsi_header *pkt = &read->data.hdr;
> +
> +		if (pkt->type == type) {
> +			desc_clear(read);
> +			read = next_desc(read);
> +			return pkt;
> +		}
> +		printk("%s: ignoring packet while waiting for type 0x%x:\n",
> +			__FUNCTION__, type);

Again with the KERN_ level missing.

Actually, why not use the dev_() macros, aren't these devices on that
wierd bus type?

Other than those minor nits, looks fine to me, thanks for posting this.

greg k-h

** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/