Re: [PATCH] parport: mfc3: Convert to a modern zorro driver

[Geert Uytterhoeven]

Hi Daniel,

Thanks for your patch!

On Thu, 26 Feb 2026 at 16:07, Daniel Palmer <daniel@xxxxxxxxx> wrote:
> Currently this driver causes my Amiga 4000 to crash if it
> is not a module because it tries to iterate over the zorro
> devices before that has been setup.

Interesting, that is not supposed to happen.

> Convert the driver over to the modern zorro driver API so
> it probes like the other zorro drivers and doesn't crash
> my machine.

Thanks, doing the conversion is definitely worthwhile!

> Claude was able to do the manual work of moving the original
> code but I had to fix up some mistakes, clean up the
> allocations to use devm versions and so on.
>
> Only build, boot, and "checked the driver is registered correctly"
> tested as I don't have the card and it seems to be one of the
> few things winuae doesn't emulate. Using claude to fix it up
> seemed better than just deleting it considering all of the effort
> Joerg put in to documenting the hardware etc.
>
> Assisted-by: Claude:claude-4.6-sonnet
> Signed-off-by: Daniel Palmer <daniel@xxxxxxxxx>

> --- a/drivers/parport/parport_mfc3.c
> +++ b/drivers/parport/parport_mfc3.c
> @@ -66,17 +66,19 @@
>  #include <asm/irq.h>
>  #include <asm/amigaints.h>
>
> -/* Maximum Number of Cards supported */
> -#define MAX_MFC 5
> -
>  #undef DEBUG
>
> -static struct parport *this_port[MAX_MFC] = {NULL, };
>  static volatile int dummy; /* for trigger readds */
>
>  #define pia(dev) ((struct pia *)(dev->base))
>  static struct parport_operations pp_mfc3_ops;
>
> +struct mfc3_card {
> +       struct parport *port;
> +       unsigned long piabase;
> +       bool has_irq;
> +};
> +
>  static void mfc3_write_data(struct parport *p, unsigned char data)
>  {
>         pr_debug("write_data %c\n", data);
> @@ -173,14 +175,18 @@ static int use_cnt;
>
>  static irqreturn_t mfc3_interrupt(int irq, void *dev_id)
>  {
> -       int i;
> +       struct zorro_dev *z = NULL;
>
> -       for( i = 0; i < MAX_MFC; i++)
> -               if (this_port[i] != NULL)
> -                       if (pia(this_port[i])->crb & 128) { /* Board caused interrupt */
> -                               dummy = pia(this_port[i])->pprb; /* clear irq bit */
> -                               parport_generic_irq(this_port[i]);
> -                       }
> +       while ((z = zorro_find_device(ZORRO_PROD_BSC_MULTIFACE_III, z))) {

Please no such while loops in an interrupt handler.
I guess you can just register multiple interrupt handlers (IRQF_SHARED,
fwiw), one per instance, and pass the mfc3_card structure as dev_id
instead? That would let you get rid of has_irq and use_cnt, too.

> +               struct mfc3_card *card = zorro_get_drvdata(z);
> +
> +               if (!card || !card->port)
> +                       continue;
> +               if (pia(card->port)->crb & 128) { /* Board caused interrupt */
> +                       dummy = pia(card->port)->pprb; /* clear irq bit */
> +                       parport_generic_irq(card->port);
> +               }
> +       }
>         return IRQ_HANDLED;

Pre-existing: the handler should return IRQ_NONE if
parport_generic_irq() was not called.

>  }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds