linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: hsyemail2@gmail.com
Cc: Johan Hovold <johan@kernel.org>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	Sheng-Yuan Huang <syhuang3@nuvoton.com>
Subject: Re: [PATCH v1 1/1] USB: serial: nct_usb_serial: add support for Nuvoton USB adapter
Date: Tue, 3 Jun 2025 06:19:44 +0200	[thread overview]
Message-ID: <2025060325-everyone-padlock-931a@gregkh> (raw)
In-Reply-To: <20250603032057.5174-2-syhuang3@nuvoton.com>

At quick glance, this looks good, but one thing overall stood out:

On Tue, Jun 03, 2025 at 11:20:57AM +0800, hsyemail2@gmail.com wrote:
> +	if (size > NCT_MAX_VENDOR_READ_SIZE)
> +		dev_err(dev, NCT_DRVNAME ": %s - failed to read [%04x]: over size %d\n",
> +			__func__, cmd.p.cmd, size);

When using dev_*() functions, you do not need a "driver prefix" here
like NCT_DRVNAME as it already is in the string that is sent to the
kernel log.  So that all can be removed from all of these.

Also:

> +	dev_dbg(&intf->dev, NCT_DRVNAME ": %s tty baud: 0x%X\n", __func__,
> +		(cflag & CBAUD));

The __func__ is already in the dev_dbg() output, so no need to repeat it
again please.

This should be done for all of the calls like this in the driver.

> +	dev_info(&intf->dev, NCT_DRVNAME ": %s Enabled devices mask:%X\n",
> +		 __func__, buf[0]);

When drivers are working properly, they are quiet, so no need for this
line at all.

thanks,

greg k-h

  reply	other threads:[~2025-06-03  4:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03  3:20 [PATCH v1 0/1] USB: serial: add support for Nuvoton USB adapter hsyemail2
2025-06-03  3:20 ` [PATCH v1 1/1] USB: serial: nct_usb_serial: " hsyemail2
2025-06-03  4:19   ` Greg Kroah-Hartman [this message]
2025-06-04  2:51     ` [PATCH v2 0/1] " hsyemail2
2025-06-04  2:51       ` [PATCH v2 1/1] " hsyemail2
2025-06-19  9:40         ` Greg KH
2025-06-20  1:16           ` Sheng-Yuan Huang
2025-06-03 11:57   ` [PATCH v1 " Oliver Neukum
2025-06-05  1:15     ` 逸曉塵
2025-06-23  7:17     ` [PATCH v3 0/1] " hsyemail2
2025-06-23  7:17       ` [PATCH v3 1/1] " hsyemail2
2025-06-23 10:32         ` Oliver Neukum
2025-06-27  0:59           ` Sheng-Yuan Huang
2025-08-20  8:07             ` [PATCH v4 0/1] " hsyemail2
2025-08-20  8:07               ` [PATCH v4 1/1] " hsyemail2
2025-06-27  7:13         ` [PATCH v3 " kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2025060325-everyone-padlock-931a@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=hsyemail2@gmail.com \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=syhuang3@nuvoton.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).