Hi Arnd!
On Mon, 2023-07-10 at 12:47 +0200, Arnd Bergmann wrote:
> It looks like only the "noioport" variant got some of the
> extra macro definitions, but the version for PCI still needs the
> same six macros, plus the ones of inb/outb etc, something like
> this:
>
> diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
> index 24c560c065ec7..2135e32145c54 100644
> --- a/arch/sh/include/asm/io.h
> +++ b/arch/sh/include/asm/io.h
> @@ -241,6 +241,26 @@ __BUILD_IOPORT_STRING(q, u64)
>
> #endif
>
> +#define inb(addr) inb(addr)
> +#define inw(addr) inw(addr)
> +#define inl(addr) inl(addr)
> +#define outb(x, addr) outb((x), (addr))
> +#define outw(x, addr) outw((x), (addr))
> +#define outl(x, addr) outl((x), (addr))
> +
> +#define inb_p(addr) inb(addr)
> +#define inw_p(addr) inw(addr)
> +#define inl_p(addr) inl(addr)
> +#define outb_p(x, addr) outb((x), (addr))
> +#define outw_p(x, addr) outw((x), (addr))
> +#define outl_p(x, addr) outl((x), (addr))
> +
> +#define insb insb
> +#define insw insw
> +#define insl insl
> +#define outsb outsb
> +#define outsw outsw
> +#define outsl outsl
>
> #define IO_SPACE_LIMIT 0xffffffff
>
> diff --git a/arch/sh/include/asm/io_noioport.h b/arch/sh/include/asm/io_noioport.h
> index 5ba4116b4265c..12dad91f41c1e 100644
> --- a/arch/sh/include/asm/io_noioport.h
> +++ b/arch/sh/include/asm/io_noioport.h
> @@ -46,20 +46,6 @@ static inline void ioport_unmap(void __iomem *addr)
> BUG();
> }
>
> -#define inb_p(addr) inb(addr)
> -#define inw_p(addr) inw(addr)
> -#define inl_p(addr) inl(addr)
> -#define outb_p(x, addr) outb((x), (addr))
> -#define outw_p(x, addr) outw((x), (addr))
> -#define outl_p(x, addr) outl((x), (addr))
> -
> -#define insb insb
> -#define insw insw
> -#define insl insl
> -#define outsb outsb
> -#define outsw outsw
> -#define outsl outsl
> -
> static inline void insb(unsigned long port, void *dst, unsigned long count)
> {
> BUG();
I am fine with this fix.
Acked-by: John Paul Adrian Glaubitz <glaubitz@xxxxxxxxxxxxxxxxxxx>
> I think ideally all the I/O port stuff in arch/sh/ could just be
> removed after the conversion to asm-generic/io.h, but the
> microdev_ioport_map() function oddity gets in the way of that,
> unless someone wants to clean up that platform. As far as I
> can tell, the ethernet, display, USB and PCI devices on it already
> broke at some point (afbb9d8d5266b, 46bc85872040a), so it might
> be easier to remove it entirely.
I don't have this particular hardware, so I cannot comment on this.
> > I'm not happy though that this patch is in linux-next without being Acked by me
> > or being reviewed by anyone. We should always make sure first that the code
> > actually builds and has been tested on real hardware.
>
> I think that if the series has been posted eight times, you had
> your chance to do a review, especially since I pointed out that
> merging this one would have avoid the unxlate_dev_mem_ptr() bug
> as well.
I have only been the maintainer of arch/sh for a few weeks, so it's natural
that I am not doing a perfect job and might miss something. Also, I am not
getting paid for this work, I am doing this in my free time.
> Having the series go into linux-next sounds appropriate like this,
> the entire purpose of that is to find such bugs and Andrew can jus
> fold the fixup into the broken patch.
>
> Let me know if you prefer the simple version with the extra
> #defines or if we should just use the generic inb/outb implementation
> immediately and drop microdev in a separate patch.
Please go ahead with the simple version.
Thanks,
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
