On Sun, Aug 30, 2020 at 06:38:49PM +0200, Luca Ceresoli wrote:
> Current code calls gpiod_get_value() without error checking. Should the
> GPIO controller fail, execution would continue without any error message.
>
> Fix by checking for negative error values.
>
> Reported-by: Tom Rix <trix@xxxxxxxxxx>
> Reviewed-by: Tom Rix <trix@xxxxxxxxxx>
> Signed-off-by: Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
>
> ---
>
> Changes in v4:
> - add Reviewed-by Tom Rix
>
> Changes in v3:
> - rebase on previous patches
>
> This patch is new in v2
> ---
> drivers/fpga/xilinx-spi.c | 35 +++++++++++++++++++++++++++--------
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index fba8eb4866a7..52aab5a1f0ba 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -27,11 +27,22 @@ struct xilinx_spi_conf {
> struct gpio_desc *done;
> };
>
> -static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
> +static int get_done_gpio(struct fpga_manager *mgr)
> {
> struct xilinx_spi_conf *conf = mgr->priv;
> + int ret;
> +
> + ret = gpiod_get_value(conf->done);
> +
> + if (ret < 0)
> + dev_err(&mgr->dev, "Error reading DONE (%d)\n", ret);
>
> - if (!gpiod_get_value(conf->done))
> + return ret;
> +}
> +
> +static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
> +{
> + if (!get_done_gpio(mgr))
> return FPGA_MGR_STATE_RESET;
>
> return FPGA_MGR_STATE_UNKNOWN;
> @@ -57,10 +68,21 @@ static int wait_for_init_b(struct fpga_manager *mgr, int value,
>
> if (conf->init_b) {
> while (time_before(jiffies, timeout)) {
> - if (gpiod_get_value(conf->init_b) == value)
> + int ret = gpiod_get_value(conf->init_b);
> +
> + if (ret == value)
> return 0;
> +
> + if (ret < 0) {
> + dev_err(&mgr->dev, "Error reading INIT_B (%d)\n", ret);
> + return ret;
> + }
> +
> usleep_range(100, 400);
> }
> +
> + dev_err(&mgr->dev, "Timeout waiting for INIT_B to %s\n",
> + value ? "assert" : "deassert");
> return -ETIMEDOUT;
> }
>
> @@ -85,7 +107,6 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
>
> err = wait_for_init_b(mgr, 1, 1); /* min is 500 ns */
> if (err) {
> - dev_err(&mgr->dev, "INIT_B pin did not go low\n");
> gpiod_set_value(conf->prog_b, 0);
> return err;
> }
> @@ -93,12 +114,10 @@ static int xilinx_spi_write_init(struct fpga_manager *mgr,
> gpiod_set_value(conf->prog_b, 0);
>
> err = wait_for_init_b(mgr, 0, 0);
> - if (err) {
> - dev_err(&mgr->dev, "INIT_B pin did not go high\n");
> + if (err)
> return err;
> - }
>
> - if (gpiod_get_value(conf->done)) {
> + if (get_done_gpio(mgr)) {
> dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
> return -EIO;
> }
> --
> 2.28.0
>
Applied to for-next,
Thanks
