linux-kernel - Re: [PATCH 2/4] remoteproc: Supply controller driver for ST's Remote Processors

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <55DDF252.2010901@mentor.com>
Date:	Wed, 26 Aug 2015 12:07:30 -0500
From:	Nathan Lynch <Nathan_Lynch@...tor.com>
To:	Lee Jones <lee.jones@...aro.org>
CC:	<linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>, <ohad@...ery.com>,
	<devicetree@...r.kernel.org>, <kernel@...inux.com>
Subject: Re: [PATCH 2/4] remoteproc: Supply controller driver for ST's Remote
 Processors

On 08/26/2015 08:08 AM, Lee Jones wrote:
> --- /dev/null
> +++ b/drivers/remoteproc/st_remoteproc.c
> @@ -0,0 +1,300 @@
> +/*
> + * ST's Remote Processor Control Driver
> + *
> + * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
> + *
> + * Author: Ludovic Barre <ludovic.barre@...com>

When submitting code you didn't write, I'd say it's better practice to
clearly indicate its provenance in the commit message.  E.g. something
like "Driver based on code authored by Ludovic Barre for ST".  And
obtain signoffs etc if possible.


> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.

Please review the wording here.  It's unclear whether this is intended
to be v2-only or v2 or later.


> +static int st_rproc_stop(struct rproc *rproc)
> +{
> +	struct st_rproc *st_rproc = rproc->priv;
> +	int err = 0;
> +
> +	if (st_rproc->config->sw_reset) {
> +		err = reset_control_assert(st_rproc->sw_reset);
> +		if (err)
> +			dev_warn(&rproc->dev, "Failed to assert S/W Reset\n");
> +	}
> +
> +	if (st_rproc->config->pwr_reset) {
> +		err = reset_control_assert(st_rproc->pwr_reset);
> +		if (err)
> +			dev_warn(&rproc->dev, "Failed to assert Power Reset\n");
> +	}
> +
> +	clk_disable(st_rproc->clk);
> +
> +	return 0;
> +}

Seems like st_rproc_stop should propagate errors back to its caller
instead of always returning 0.

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