linux-kernel - Re: [patch v23 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 29 May 2018 15:11:09 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Oleksandr Shamray <oleksandrs@...lanox.com>
Cc:     arnd@...db.de, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        openbmc@...ts.ozlabs.org, joel@....id.au, jiri@...nulli.us,
        tklauser@...tanz.ch, linux-serial@...r.kernel.org,
        vadimp@...lanox.com, system-sw-low-level@...lanox.com,
        robh+dt@...nel.org, openocd-devel-owner@...ts.sourceforge.net,
        linux-api@...r.kernel.org, davem@...emloft.net, mchehab@...nel.org
Subject: Re: [patch v23 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx
 families JTAG master driver

On Tue, May 29, 2018 at 02:49:03PM +0300, Oleksandr Shamray wrote:
> +static int aspeed_jtag_idle(struct jtag *jtag,
> +			    struct jtag_run_test_idle *runtest)
> +{
> +	struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> +
> +	dev_dbg(aspeed_jtag->dev, "runtest, status:%d, mode:%s, state:%s, reset:%d, tck:%d\n",
> +		aspeed_jtag->status,
> +		aspeed_jtag->mode & JTAG_XFER_HW_MODE ? "HW" : "SW",
> +		end_status_str[runtest->endstate], runtest->reset,
> +		runtest->tck);

You have dev_dbg() lines all over the place.  They can all be removed
now, right?  ftrace works well, and if you really want to do this
properly, use tracepoints in the jtag core code so that you can get the
information for all devices as needed.

But don't put them all over the driver, now that it's all working,
there's no need, right?

> +static void aspeed_jtag_xfer_push_data_last(struct aspeed_jtag *aspeed_jtag,
> +					    enum jtag_xfer_type type,
> +					    u32 shift_bits,
> +					    enum jtag_endstate endstate)
> +{
> +	if (endstate == JTAG_STATE_IDLE) {
> +		if (type == JTAG_SIR_XFER) {
> +			dev_dbg(aspeed_jtag->dev, "IR Keep Pause\n");

Like this, doesn't provide much info, does it?

thanks,

greg k-h