From 59e15cde9f7bb5b8d824295059d42f41e009a420 Mon Sep 17 00:00:00 2001 From: Dipen Patel Date: Mon, 24 Feb 2020 12:15:53 -0800 Subject: drivers: staging: gte_test: Unwind and other issues There are bugs in test driver as follows: 1) gpio unwind is not done during init 2) No guard against re-registering event, for example, if user issues echo 1 > gpio_en_dis, it will overwrite existing event descriptor Also, it has other issues as below: 1) print debug messages correction in init 2) No print messages when timestamp is not available 3) Kconfig has extra blank lines This CL addresses all above issue. Bug 2865212 Change-Id: I66f4bd377c4d3557b46ab5828dabad4ee8394454 Signed-off-by: Dipen Patel Reviewed-on: https://git-master.nvidia.com/r/c/linux-nvidia/+/2302019 Reviewed-by: Stephen Warren Reviewed-by: Winnie Hsu Reviewed-by: mobile promotions Tested-by: mobile promotions GVS: Gerrit_Virtual_Submit --- drivers/staging/platform/tegra/gte_test/Kconfig | 3 +- .../platform/tegra/gte_test/tegra194_gte_test.c | 76 ++++++++++++++++++---- 2 files changed, 64 insertions(+), 15 deletions(-) (limited to 'drivers') diff --git a/drivers/staging/platform/tegra/gte_test/Kconfig b/drivers/staging/platform/tegra/gte_test/Kconfig index 7b4fbf1d2..5130e1638 100644 --- a/drivers/staging/platform/tegra/gte_test/Kconfig +++ b/drivers/staging/platform/tegra/gte_test/Kconfig @@ -4,5 +4,4 @@ config TEGRA_GTE_TEST help Enable this option for integrated generic timestamping support on NVIDIA Tegra systems-on-chip. The driver supports LIC IRQs and AON - GPIO monitoring for hardware timestamping. - + GPIO monitoring for hardware timestamping. \ No newline at end of file diff --git a/drivers/staging/platform/tegra/gte_test/tegra194_gte_test.c b/drivers/staging/platform/tegra/gte_test/tegra194_gte_test.c index bea0df14c..2311aa143 100644 --- a/drivers/staging/platform/tegra/gte_test/tegra194_gte_test.c +++ b/drivers/staging/platform/tegra/gte_test/tegra194_gte_test.c @@ -83,17 +83,34 @@ static ssize_t store_gpio_en_dis(struct kobject *kobj, } if (val == 1) { + if (gte.data_gpio) { + pr_info("gpio_in is already registered\n"); + ret = -EEXIST; + goto error; + } gte.data_gpio = tegra_gte_register_event(np, gpio_in); if (IS_ERR(gte.data_gpio)) { pr_err("Could not register gpio\n"); - ret = -EINVAL; + ret = PTR_ERR(gte.data_gpio); + gte.data_gpio = NULL; goto error; } } else if (val == 0) { - if (tegra_gte_unregister_event(gte.data_gpio) != 0) { + if (!gte.data_gpio) { + pr_info("gpio_in is not registered\n"); ret = -EINVAL; goto error; } + ret = tegra_gte_unregister_event(gte.data_gpio); + if (ret == -EBUSY) { + /* User should retry */ + pr_err("failed to unregister gpio in\n"); + goto error; + } else { /* For anything else set data to null */ + gte.data_gpio = NULL; + if (ret == 0) + ret = count; + } } else { ret = -EINVAL; } @@ -126,17 +143,34 @@ static ssize_t store_lic_irq_en_dis(struct kobject *kobj, } if (val == 1) { + if (gte.data_lic) { + pr_info("lic_irq is already registered\n"); + ret = -EEXIST; + goto error; + } gte.data_lic = tegra_gte_register_event(np, lic_irq); if (IS_ERR(gte.data_lic)) { pr_err("Could not register lic irq\n"); - ret = -EINVAL; + ret = PTR_ERR(gte.data_lic); + gte.data_lic = NULL; goto error; } } else if (val == 0) { - if (tegra_gte_unregister_event(gte.data_lic) != 0) { + if (!gte.data_lic) { + pr_info("lic_irq is not registered\n"); ret = -EINVAL; goto error; } + ret = tegra_gte_unregister_event(gte.data_lic); + if (ret == -EBUSY) { + /* User should retry */ + pr_err("failed to unregister lic irq\n"); + goto error; + } else { /* For anything else set data to null */ + gte.data_lic = NULL; + if (ret == 0) + ret = count; + } } else { ret = -EINVAL; } @@ -206,8 +240,10 @@ static irqreturn_t tegra_gte_test_gpio_isr(int irq, void *data) struct tegra_gte_ev_detail hts; struct tegra_gte_test *gte = data; - if (tegra_gte_retrieve_event((gte->data_gpio), &hts) != 0) + if (tegra_gte_retrieve_event((gte->data_gpio), &hts) != 0) { + pr_info("No timestamp available\n"); return IRQ_HANDLED; + } pr_info("GPIO HW Timestamp: raw %llu, ns %llu\n", hts.ts_raw, hts.ts_ns); @@ -219,10 +255,13 @@ static int __init tegra_gte_test_init(void) int ret = 0; if (gpio_out == -EINVAL || gpio_in == -EINVAL || lic_irq == EINVAL) { - pr_err("Invalid gpio_out, gpio_in, gpiochip_base and irq\n"); + pr_err("Invalid gpio_out, gpio_in and irq\n"); return -EINVAL; } + gte.data_lic = NULL; + gte.data_gpio = NULL; + ret = gpio_request(gpio_out, "gte_test_gpio_out"); if (ret) { pr_err("failed request gpio out\n"); @@ -232,26 +271,31 @@ static int __init tegra_gte_test_init(void) ret = gpio_direction_output(gpio_out, 0); if (ret) { pr_err("failed to set pin direction\n"); - return -EINVAL; + ret = -EINVAL; + goto free_gpio_out; } ret = gpio_request(gpio_in, "gte_test_gpio_in"); if (ret) { pr_err("failed to request gpio in\n"); - return -EINVAL; + ret = -EINVAL; + goto free_gpio_out; } ret = gpio_direction_input(gpio_in); if (ret) { pr_err("failed to set pin direction\n"); - return -EINVAL; + ret = -EINVAL; + goto free_gpio_in; + } /* IRQ setup */ ret = gpio_to_irq(gpio_in); if (ret < 0) { pr_err("failed to map GPIO to IRQ: %d\n", ret); - return -EINVAL; + ret = -EINVAL; + goto free_gpio_in; } gte.gpio_in_irq = ret; @@ -261,14 +305,15 @@ static int __init tegra_gte_test_init(void) "tegra_gte_test_isr", >e); if (ret) { pr_err("failed to acquire IRQ\n"); - return ret; + ret = -EINVAL; + goto free_gpio_in; } ret = tegra_gte_test_sysfs_create(); if (ret != 0) { pr_err("sysfs creation failed\n"); ret = -EINVAL; - goto error; + goto free_irq; } setup_timer(>e.timer, gpio_timer_cb, 0); @@ -276,8 +321,13 @@ static int __init tegra_gte_test_init(void) return 0; -error: +free_irq: free_irq(gte.gpio_in_irq, >e); +free_gpio_in: + gpio_free(gpio_in); +free_gpio_out: + gpio_free(gpio_out); + return ret; } -- cgit v1.2.2