Commit 3b6bddda authored by Thomas Bogendoerfer's avatar Thomas Bogendoerfer Committed by Alexandre Belloni
Browse files

rtc: ds1685: use threaded interrupt



Handling of extended interrupts (kickstart, wake-up, ram-clear) was
moved off to a work queue, but the interrupts aren't acknowledged
in the interrupt handler. This leads to a deadlock, if driver
is used with interrupts. To fix this we use a threaded interrupt, get rid
of the work queue and do locking with just the rtc mutex lock.

Fixes: aaaf5fbf ("rtc: add driver for DS1685 family of real time clocks")
Signed-off-by: default avatarThomas Bogendoerfer <tbogendoerfer@suse.de>
Signed-off-by: default avatarAlexandre Belloni <alexandre.belloni@bootlin.com>
parent e330c3d5
Loading
Loading
Loading
Loading
+104 −116
Original line number Diff line number Diff line
@@ -510,10 +510,6 @@ static int
ds1685_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
{
	struct ds1685_priv *rtc = dev_get_drvdata(dev);
	unsigned long flags = 0;

	/* Enable/disable the Alarm IRQ-Enable flag. */
	spin_lock_irqsave(&rtc->lock, flags);

	/* Flip the requisite interrupt-enable bit. */
	if (enabled)
@@ -525,7 +521,6 @@ ds1685_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)

	/* Read Control C to clear all the flag bits. */
	rtc->read(rtc, RTC_CTRL_C);
	spin_unlock_irqrestore(&rtc->lock, flags);

	return 0;
}
@@ -533,98 +528,18 @@ ds1685_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)


/* ----------------------------------------------------------------------- */
/* IRQ handler & workqueue. */

/**
 * ds1685_rtc_irq_handler - IRQ handler.
 * @irq: IRQ number.
 * @dev_id: platform device pointer.
 */
static irqreturn_t
ds1685_rtc_irq_handler(int irq, void *dev_id)
{
	struct platform_device *pdev = dev_id;
	struct ds1685_priv *rtc = platform_get_drvdata(pdev);
	u8 ctrlb, ctrlc;
	unsigned long events = 0;
	u8 num_irqs = 0;

	/* Abort early if the device isn't ready yet (i.e., DEBUG_SHIRQ). */
	if (unlikely(!rtc))
		return IRQ_HANDLED;

	/* Ctrlb holds the interrupt-enable bits and ctrlc the flag bits. */
	spin_lock(&rtc->lock);
	ctrlb = rtc->read(rtc, RTC_CTRL_B);
	ctrlc = rtc->read(rtc, RTC_CTRL_C);

	/* Is the IRQF bit set? */
	if (likely(ctrlc & RTC_CTRL_C_IRQF)) {
		/*
		 * We need to determine if it was one of the standard
		 * events: PF, AF, or UF.  If so, we handle them and
		 * update the RTC core.
		 */
		if (likely(ctrlc & RTC_CTRL_B_PAU_MASK)) {
			events = RTC_IRQF;

			/* Check for a periodic interrupt. */
			if ((ctrlb & RTC_CTRL_B_PIE) &&
			    (ctrlc & RTC_CTRL_C_PF)) {
				events |= RTC_PF;
				num_irqs++;
			}

			/* Check for an alarm interrupt. */
			if ((ctrlb & RTC_CTRL_B_AIE) &&
			    (ctrlc & RTC_CTRL_C_AF)) {
				events |= RTC_AF;
				num_irqs++;
			}

			/* Check for an update interrupt. */
			if ((ctrlb & RTC_CTRL_B_UIE) &&
			    (ctrlc & RTC_CTRL_C_UF)) {
				events |= RTC_UF;
				num_irqs++;
			}

			rtc_update_irq(rtc->dev, num_irqs, events);
		} else {
			/*
			 * One of the "extended" interrupts was received that
			 * is not recognized by the RTC core.  These need to
			 * be handled in task context as they can call other
			 * functions and the time spent in irq context needs
			 * to be minimized.  Schedule them into a workqueue
			 * and inform the RTC core that the IRQs were handled.
			 */
			spin_unlock(&rtc->lock);
			schedule_work(&rtc->work);
			rtc_update_irq(rtc->dev, 0, 0);
			return IRQ_HANDLED;
		}
	}
	spin_unlock(&rtc->lock);

	return events ? IRQ_HANDLED : IRQ_NONE;
}
/* IRQ handler */

/**
 * ds1685_rtc_work_queue - work queue handler.
 * @work: work_struct containing data to work on in task context.
 * ds1685_rtc_extended_irq - take care of extended interrupts
 * @rtc: pointer to the ds1685 rtc structure.
 * @pdev: platform device pointer.
 */
static void
ds1685_rtc_work_queue(struct work_struct *work)
ds1685_rtc_extended_irq(struct ds1685_priv *rtc, struct platform_device *pdev)
{
	struct ds1685_priv *rtc = container_of(work,
					       struct ds1685_priv, work);
	struct platform_device *pdev = to_platform_device(&rtc->dev->dev);
	struct mutex *rtc_mutex = &rtc->dev->ops_lock;
	u8 ctrl4a, ctrl4b;

	mutex_lock(rtc_mutex);

	ds1685_rtc_switch_to_bank1(rtc);
	ctrl4a = rtc->read(rtc, RTC_EXT_CTRL_4A);
	ctrl4b = rtc->read(rtc, RTC_EXT_CTRL_4B);
@@ -703,8 +618,76 @@ ds1685_rtc_work_queue(struct work_struct *work)
				 "RAM-Clear IRQ just occurred!\n");
	}
	ds1685_rtc_switch_to_bank0(rtc);
}

/**
 * ds1685_rtc_irq_handler - IRQ handler.
 * @irq: IRQ number.
 * @dev_id: platform device pointer.
 */
static irqreturn_t
ds1685_rtc_irq_handler(int irq, void *dev_id)
{
	struct platform_device *pdev = dev_id;
	struct ds1685_priv *rtc = platform_get_drvdata(pdev);
	struct mutex *rtc_mutex;
	u8 ctrlb, ctrlc;
	unsigned long events = 0;
	u8 num_irqs = 0;

	/* Abort early if the device isn't ready yet (i.e., DEBUG_SHIRQ). */
	if (unlikely(!rtc))
		return IRQ_HANDLED;

	rtc_mutex = &rtc->dev->ops_lock;
	mutex_lock(rtc_mutex);

	/* Ctrlb holds the interrupt-enable bits and ctrlc the flag bits. */
	ctrlb = rtc->read(rtc, RTC_CTRL_B);
	ctrlc = rtc->read(rtc, RTC_CTRL_C);

	/* Is the IRQF bit set? */
	if (likely(ctrlc & RTC_CTRL_C_IRQF)) {
		/*
		 * We need to determine if it was one of the standard
		 * events: PF, AF, or UF.  If so, we handle them and
		 * update the RTC core.
		 */
		if (likely(ctrlc & RTC_CTRL_B_PAU_MASK)) {
			events = RTC_IRQF;

			/* Check for a periodic interrupt. */
			if ((ctrlb & RTC_CTRL_B_PIE) &&
			    (ctrlc & RTC_CTRL_C_PF)) {
				events |= RTC_PF;
				num_irqs++;
			}

			/* Check for an alarm interrupt. */
			if ((ctrlb & RTC_CTRL_B_AIE) &&
			    (ctrlc & RTC_CTRL_C_AF)) {
				events |= RTC_AF;
				num_irqs++;
			}

			/* Check for an update interrupt. */
			if ((ctrlb & RTC_CTRL_B_UIE) &&
			    (ctrlc & RTC_CTRL_C_UF)) {
				events |= RTC_UF;
				num_irqs++;
			}
		} else {
			/*
			 * One of the "extended" interrupts was received that
			 * is not recognized by the RTC core.
			 */
			ds1685_rtc_extended_irq(rtc, pdev);
		}
	}
	rtc_update_irq(rtc->dev, num_irqs, events);
	mutex_unlock(rtc_mutex);

	return events ? IRQ_HANDLED : IRQ_NONE;
}
/* ----------------------------------------------------------------------- */

@@ -833,11 +816,15 @@ static int ds1685_nvram_read(void *priv, unsigned int pos, void *val,
			     size_t size)
{
	struct ds1685_priv *rtc = priv;
	struct mutex *rtc_mutex = &rtc->dev->ops_lock;
	ssize_t count;
	unsigned long flags = 0;
	u8 *buf = val;
	int err;

	err = mutex_lock_interruptible(rtc_mutex);
	if (err)
		return err;

	spin_lock_irqsave(&rtc->lock, flags);
	ds1685_rtc_switch_to_bank0(rtc);

	/* Read NVRAM in time and bank0 registers. */
@@ -887,7 +874,7 @@ static int ds1685_nvram_read(void *priv, unsigned int pos, void *val,
		ds1685_rtc_switch_to_bank0(rtc);
	}
#endif /* !CONFIG_RTC_DRV_DS1689 */
	spin_unlock_irqrestore(&rtc->lock, flags);
	mutex_unlock(rtc_mutex);

	return 0;
}
@@ -896,11 +883,15 @@ static int ds1685_nvram_write(void *priv, unsigned int pos, void *val,
			      size_t size)
{
	struct ds1685_priv *rtc = priv;
	struct mutex *rtc_mutex = &rtc->dev->ops_lock;
	ssize_t count;
	unsigned long flags = 0;
	u8 *buf = val;
	int err;

	err = mutex_lock_interruptible(rtc_mutex);
	if (err)
		return err;

	spin_lock_irqsave(&rtc->lock, flags);
	ds1685_rtc_switch_to_bank0(rtc);

	/* Write NVRAM in time and bank0 registers. */
@@ -950,7 +941,7 @@ static int ds1685_nvram_write(void *priv, unsigned int pos, void *val,
		ds1685_rtc_switch_to_bank0(rtc);
	}
#endif /* !CONFIG_RTC_DRV_DS1689 */
	spin_unlock_irqrestore(&rtc->lock, flags);
	mutex_unlock(rtc_mutex);

	return 0;
}
@@ -1141,9 +1132,7 @@ ds1685_rtc_probe(struct platform_device *pdev)
	if (pdata->plat_post_ram_clear)
		rtc->post_ram_clear = pdata->plat_post_ram_clear;

	/* Init the spinlock, workqueue, & set the driver data. */
	spin_lock_init(&rtc->lock);
	INIT_WORK(&rtc->work, ds1685_rtc_work_queue);
	/* set the driver data. */
	platform_set_drvdata(pdev, rtc);

	/* Turn the oscillator on if is not already on (DV1 = 1). */
@@ -1299,13 +1288,16 @@ ds1685_rtc_probe(struct platform_device *pdev)
	 */
	if (!pdata->no_irq) {
		ret = platform_get_irq(pdev, 0);
		if (ret > 0) {
		if (ret <= 0)
			return ret;

		rtc->irq_num = ret;

		/* Request an IRQ. */
			ret = devm_request_irq(&pdev->dev, rtc->irq_num,
					       ds1685_rtc_irq_handler,
					       IRQF_SHARED, pdev->name, pdev);
		ret = devm_request_threaded_irq(&pdev->dev, rtc->irq_num,
				       NULL, ds1685_rtc_irq_handler,
				       IRQF_SHARED | IRQF_ONESHOT,
				       pdev->name, pdev);

		/* Check to see if something came back. */
		if (unlikely(ret)) {
@@ -1313,8 +1305,6 @@ ds1685_rtc_probe(struct platform_device *pdev)
				 "RTC interrupt not available\n");
			rtc->irq_num = 0;
		}
		} else
			return ret;
	}
	rtc->no_irq = pdata->no_irq;

@@ -1361,8 +1351,6 @@ ds1685_rtc_remove(struct platform_device *pdev)
		   (rtc->read(rtc, RTC_EXT_CTRL_4A) &
		    ~(RTC_CTRL_4A_RWK_MASK)));

	cancel_work_sync(&rtc->work);

	return 0;
}

+0 −2
Original line number Diff line number Diff line
@@ -48,8 +48,6 @@ struct ds1685_priv {
	u32 regstep;
	resource_size_t baseaddr;
	size_t size;
	spinlock_t lock;
	struct work_struct work;
	int irq_num;
	bool bcd_mode;
	bool no_irq;