Re: [PATCH v3] m68k/amiga - Amiga Zorro NCR53C9x boards: new zorro_esp.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Am 14.03.2018 um 21:30 schrieb Geert Uytterhoeven:
Hi Michael,

On Wed, Mar 14, 2018 at 9:23 AM, Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
thanks for the review - largely uncontroversial except for the volatile...

The presence of volatile in drivers is always considered controversial ;-)

Yes indeed. I just hadn't looked at raw_io.h in ages (too many unhappy
memories), and didn't remember the __force volatile trick there.

Meaning writeb(val, reg) instead of reg = val?

#define out_8(addr,b) (void)((*(__force volatile u8 *) (addr)) = (b))

nicely hides the 'volatile' but suggests I also need to pass it a
pointer, so

writeb((addr >> 24) & 0xff, &dregs->dma_addr)

Yes, you have to pass it an (__iomem) pointer.

Thanks.

I'll have to compare the assembly generated by the two versions before I
dare test that, but I'll give that a try. Liberal use of wmb() did fix
the miscompile but that just looked too ugly.

Using the macros should have the same effect due to the embedded volatile.

I tend to agree, but crashing elgar or trashing the SCSI filesystem
there isn't something I would like to risk (Adrian has been very
accommodating allowing me to test the driver there, but a reset tends to
be taking a few hours thanks to the time zone difference). So I'd rather
make sure nothing can go wrong.

Cheers,

	Michael



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

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux