[2/7] memory: emif: simplify locking with guard()

Commit Message

Krzysztof Kozlowski Aug. 23, 2024, 10:15 a.m. UTC

Simplify error handling (less gotos) over locks with guard().

The driver used file-scope variable 'irq_state' for storing IRQ state
with spin_lock_irqsave, so move it into respective local scopes.  This
should be equivalent, but more readable (less global variables).

Signed-off-by: Krzysztof Kozlowski <[e-mail]>

---

Not tested on hardware, although I don't think that moving 'irq_state'
scope would affect anything.
---
 drivers/memory/emif.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Jonathan Cameron Aug. 23, 2024, 11:40 a.m. UTC | #1

On Fri, 23 Aug 2024 12:15:57 +0200
Krzysztof Kozlowski <[e-mail]> wrote:

> Simplify error handling (less gotos) over locks with guard().
> 
> The driver used file-scope variable 'irq_state' for storing IRQ state
> with spin_lock_irqsave, so move it into respective local scopes.  This
> should be equivalent, but more readable (less global variables).
> 
> Signed-off-by: Krzysztof Kozlowski <[e-mail]>
> 
LGTM. File scoped irq_state is certainly unusual...

Reviewed-by: Jonathan Cameron <[e-mail]>

Patch

diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index b32f3a8f9d71..99eb7d1baa5f 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -7,6 +7,7 @@ 
  * Aneesh V <[e-mail]>
  * Santosh Shilimkar <[e-mail]>
  */
+#include <linux/cleanup.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/reboot.h>
@@ -68,7 +69,6 @@  struct emif_data {
 
 static struct emif_data *emif1;
 static DEFINE_SPINLOCK(emif_lock);
-static unsigned long	irq_state;
 static LIST_HEAD(device_list);
 
 static void do_emif_regdump_show(struct seq_file *s, struct emif_data *emif,
@@ -522,18 +522,18 @@  static void setup_temperature_sensitive_regs(struct emif_data *emif,
 static irqreturn_t handle_temp_alert(void __iomem *base, struct emif_data *emif)
 {
 	u32		old_temp_level;
-	irqreturn_t	ret = IRQ_HANDLED;
+	irqreturn_t	ret;
 	struct emif_custom_configs *custom_configs;
 
-	spin_lock_irqsave(&emif_lock, irq_state);
+	guard(spinlock_irqsave)(&emif_lock);
 	old_temp_level = emif->temperature_level;
 	get_temperature_level(emif);
 
 	if (unlikely(emif->temperature_level == old_temp_level)) {
-		goto out;
+		return IRQ_HANDLED;
 	} else if (!emif->curr_regs) {
 		dev_err(emif->dev, "temperature alert before registers are calculated, not de-rating timings\n");
-		goto out;
+		return IRQ_HANDLED;
 	}
 
 	custom_configs = emif->plat_data->custom_configs;
@@ -553,8 +553,7 @@  static irqreturn_t handle_temp_alert(void __iomem *base, struct emif_data *emif)
 			 * from thread context
 			 */
 			emif->temperature_level = SDRAM_TEMP_VERY_HIGH_SHUTDOWN;
-			ret = IRQ_WAKE_THREAD;
-			goto out;
+			return IRQ_WAKE_THREAD;
 		}
 	}
 
@@ -570,10 +569,9 @@  static irqreturn_t handle_temp_alert(void __iomem *base, struct emif_data *emif)
 		/* Temperature is going up - handle immediately */
 		setup_temperature_sensitive_regs(emif, emif->curr_regs);
 		do_freq_update();
+		ret = IRQ_HANDLED;
 	}
 
-out:
-	spin_unlock_irqrestore(&emif_lock, irq_state);
 	return ret;
 }
 
@@ -616,6 +614,7 @@  static irqreturn_t emif_interrupt_handler(int irq, void *dev_id)
 static irqreturn_t emif_threaded_isr(int irq, void *dev_id)
 {
 	struct emif_data	*emif = dev_id;
+	unsigned long		irq_state;
 
 	if (emif->temperature_level == SDRAM_TEMP_VERY_HIGH_SHUTDOWN) {
 		dev_emerg(emif->dev, "SDRAM temperature exceeds operating limit.. Needs shut down!!!\n");