Re: [PATCH 19/31] swim: Deduplicate polling loops

[Laurent Vivier <laurent@vivier.eu>]

Le 16/07/2026 à 12:02, Finn Thain a écrit :
> Replace duplicated polling loops with poll_timeout_us(). Change the
> interruptible sleep to uninterruptible because signal delivery shouldn't
> be allowed to shorten delays required by the drive hardware.
> 
> Change the timeout for the !STEP transition to 20 ms in accordance with
> the maximum interval required by the UPD72070 spec. The existing 1 second
> timeout is impractical considering the number of steps in a typical seek.
> 
> Change the return type of swim_readbit() to bool because that way the
> bit names make sense i.e. the reader doesn't have to remember to invert
> the active-low logic used for drive signals.
> 
> Fixes: 8852ecd97488 ("m68k: mac - Add SWIM floppy support")
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> ---
>   drivers/block/swim.c | 42 +++++++++++-------------------------------
>   1 file changed, 11 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/block/swim.c b/drivers/block/swim.c
> index b4bf96a788db..c10ad1266c3b 100644
> --- a/drivers/block/swim.c
> +++ b/drivers/block/swim.c
> @@ -19,6 +19,7 @@
>   #include <linux/major.h>
>   #include <linux/mutex.h>
>   #include <linux/hdreg.h>
> +#include <linux/iopoll.h>
>   #include <linux/kernel.h>
>   #include <linux/delay.h>
>   #include <linux/platform_device.h>
> @@ -296,7 +297,7 @@ static inline void swim_action(struct swim __iomem *base, int action)
>   	local_irq_restore(flags);
>   }
>   
> -static inline int swim_readbit(struct swim __iomem *base, int bit)
> +static inline bool swim_readbit(struct swim __iomem *base, int bit)
>   {
>   	int stat;
>   
> @@ -309,6 +310,12 @@ static inline int swim_readbit(struct swim __iomem *base, int bit)
>   	return (stat & SENSE) == 0;
>   }
>   
> +#define swim_readbit_timeout(base, bit, val, timeout_us) \
> +	poll_timeout_us(, swim_readbit(base, bit) == val, 1000, timeout_us, false)
> +
> +#define swim_readbit_timeout_atomic(base, bit, val, timeout_us) \
> +	poll_timeout_us_atomic(, swim_readbit(base, bit) == val, 1, timeout_us, false)
> +
>   static inline void swim_drive(struct swim __iomem *base,
>   			      enum drive_location location)
>   {
> @@ -331,16 +338,8 @@ static inline void swim_motor(struct swim __iomem *base,
>   			      enum motor_action action)
>   {
>   	if (action == ON) {
> -		int i;
> -
>   		swim_action(base, MOTOR_ON);
> -
> -		for (i = 0; i < 2*HZ; i++) {
> -			if (swim_readbit(base, MOTOR_ON))
> -				break;
> -			set_current_state(TASK_INTERRUPTIBLE);
> -			schedule_timeout(1);
> -		}
> +		swim_readbit_timeout(base, MOTOR_ON, true, 2000 * 1000);
>   	} else if (action == OFF) {
>   		swim_action(base, MOTOR_OFF);
>   		swim_write(base, phase, RELAX);
> @@ -349,16 +348,8 @@ static inline void swim_motor(struct swim __iomem *base,
>   
>   static inline void swim_eject(struct swim __iomem *base)
>   {
> -	int i;
> -
>   	swim_action(base, EJECT);
> -
> -	for (i = 0; i < 2*HZ; i++) {
> -		if (!swim_readbit(base, DISK_IN))
> -			break;
> -		set_current_state(TASK_INTERRUPTIBLE);
> -		schedule_timeout(1);
> -	}
> +	swim_readbit_timeout(base, DISK_IN, false, 2000 * 1000);
>   }
>   
>   static inline void swim_head(struct swim __iomem *base, enum head head)
> @@ -373,19 +364,8 @@ static inline void swim_head(struct swim __iomem *base, enum head head)
>   
>   static inline int swim_step(struct swim __iomem *base)
>   {
> -	int wait;
> -
>   	swim_action(base, STEP);
> -
> -	for (wait = 0; wait < HZ; wait++) {
> -
> -		set_current_state(TASK_INTERRUPTIBLE);
> -		schedule_timeout(1);
> -
> -		if (!swim_readbit(base, STEP))
> -			return 0;
> -	}
> -	return -1;
> +	return swim_readbit_timeout_atomic(base, STEP, false, 20 * 1000);

I don't understand why we need _atomic() here.

Anyway:
Reviewed-by: Laurent Vivier <laurent@vivier.eu>

>   }
>   
>   static inline int swim_track00(struct swim __iomem *base)