Date: Mon, 31 Jan 2005 10:25:42 -0800
From: Greg KH <greg@kroah.com>
To: "Mark A. Greer" <mgreer@mvista.com>
Cc: phil@netroedge.com, sensors@stimpy.netroedge.com,
       lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH][I2C] Marvell mv64xxx i2c driver
Message-ID: <20050131182542.GB21438@kroah.com>
References: <41F6F1D5.90601@mvista.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <41F6F1D5.90601@mvista.com>
User-Agent: Mutt/1.5.6i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 846
Lines: 23

On Tue, Jan 25, 2005 at 06:26:45PM -0700, Mark A. Greer wrote:
> +static inline void
> +mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)

This is a much too big of a function to be "inline".  Please change it.
Same for your other inline functions, that's not really needed, right?

> +{
> +	pr_debug("mv64xxx_i2c_fsm: ENTER--state: %d, status: 0x%x\n",
> +		drv_data->state, status);

Please use the dev_* calls instead.  It gives you an accurate
description of the specific device that emits the messages.  Also use it
for all of the printk() calls in the driver too.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/