Re: [PATCH 19/31] swim: Deduplicate polling loops
Laurent Vivier <[email protected]> Sun, 26 Jul 2026 16:43:38 +0200
| Newsgroups | org.kernel.vger.linux-m68k,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Le 16/07/2026 =C3=A0 12:02, Finn Thain a =C3=A9crit=C2=A0:
> 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.
>=20
> Change the timeout for the !STEP transition to 20 ms in accordance with
> the maximum interval required by the UPD72070 spec. The existing 1 secon=
d
> timeout is impractical considering the number of steps in a typical seek=
.
>=20
> 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.
>=20
> Fixes: 8852ecd97488 ("m68k: mac - Add SWIM floppy support")
> Signed-off-by: Finn Thain <[email protected]>
> ---
> drivers/block/swim.c | 42 +++++++++++-------------------------------
> 1 file changed, 11 insertions(+), 31 deletions(-)
>=20
> 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);
> }
> =20
> -static inline int swim_readbit(struct swim __iomem *base, int bit)
> +static inline bool swim_readbit(struct swim __iomem *base, int bit)
> {
> int stat;
> =20
> @@ -309,6 +310,12 @@ static inline int swim_readbit(struct swim __iomem =
*base, int bit)
> return (stat & SENSE) =3D=3D 0;
> }
> =20
> +#define swim_readbit_timeout(base, bit, val, timeout_us) \
> + poll_timeout_us(, swim_readbit(base, bit) =3D=3D val, 1000, timeout_us=
, false)
> +
> +#define swim_readbit_timeout_atomic(base, bit, val, timeout_us) \
> + poll_timeout_us_atomic(, swim_readbit(base, bit) =3D=3D val, 1, timeou=
t_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 =3D=3D ON) {
> - int i;
> -
> swim_action(base, MOTOR_ON);
> -
> - for (i =3D 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 =3D=3D OFF) {
> swim_action(base, MOTOR_OFF);
> swim_write(base, phase, RELAX);
> @@ -349,16 +348,8 @@ static inline void swim_motor(struct swim __iomem *=
base,
> =20
> static inline void swim_eject(struct swim __iomem *base)
> {
> - int i;
> -
> swim_action(base, EJECT);
> -
> - for (i =3D 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);
> }
> =20
> static inline void swim_head(struct swim __iomem *base, enum head head=
)
> @@ -373,19 +364,8 @@ static inline void swim_head(struct swim __iomem *b=
ase, enum head head)
> =20
> static inline int swim_step(struct swim __iomem *base)
> {
> - int wait;
> -
> swim_action(base, STEP);
> -
> - for (wait =3D 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 <[email protected]>
> }
> =20
> static inline int swim_track00(struct swim __iomem *base)