[U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.

[Kursad Oney]

Hi Daniel,

On Wed, Aug 14, 2019 at 10:43 AM Daniel Schwierzeck
<daniel.schwierzeck at gmail.com> wrote:
>
>
>
> Am 14.08.19 um 15:29 schrieb Kursad Oney:
> > Hi Daniel,
> >
> > On Wed, Aug 14, 2019 at 9:19 AM Philippe Reynes
> > <philippe.reynes at softathome.com> wrote:
> >>
> >> From: Kursad Oney <kursad.oney at broadcom.com>
> >>
> >> wait_for_bit_le32 and wait_for_bit_le16 use the raw I/O functions
> >> which would default to big-endian on BE systems. Create the generic
> >> equivalents to use the native endianness.
> >>
> >> Signed-off-by: Kursad Oney <kursad.oney at broadcom.com>
> >> Reviewed-by: Philippe Reynes <philippe.reynes at softathome.com>
> >> ---
> >>  include/wait_bit.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> Changelog:
> >> v3:
> >> - avoid double definition when readw_be/readl_be is defined
> >> v2:
> >> - move this patch from third place to second place
> >>
> >> diff --git a/include/wait_bit.h b/include/wait_bit.h
> >> index 82e09da..79da081 100644
> >> --- a/include/wait_bit.h
> >> +++ b/include/wait_bit.h
> >> @@ -72,10 +72,12 @@ static inline int wait_for_bit_##sfx(const void *reg,                       \
> >>
> >>  BUILD_WAIT_FOR_BIT(8, u8, readb)
> >>  BUILD_WAIT_FOR_BIT(le16, u16, readw)
> >> +BUILD_WAIT_FOR_BIT(16, u16, readw)
> >>  #ifdef readw_be
> >>  BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
> >>  #endif
> >>  BUILD_WAIT_FOR_BIT(le32, u32, readl)
> >> +BUILD_WAIT_FOR_BIT(32, u32, readl)
> >>  #ifdef readl_be
> >>  BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
> >>  #endif
> >> --
> >> 2.7.4
> >>
> >
> > Philippe and I have a question about the wait_for_bit macros for MIPS
> > if you don't mind.
> >
> > It seems in the current code, wait_for_bit_le32 seems to default to
> > readl for MIPS and
> > wait_for_bit_le16 to readw. Is this correct? If so, our
> > wait_for_bit_32 macro would default
> > to LE on MIPS and we need to change that. However, readl seems like it
> > would default
> > to host endianness. Can you help us out?
> >
>
> A readl on MIPS is effectively a __raw_readl, it doesn't force LE. But
> there is one exception: if a CPU/board selects CONFIG_SWAP_IO_SPACE,
> readl/writel will swap to the opposite endianess of the CPU. But you
> don't need to worry about that because that's only used on old hardware
> together with PCI/ISA and it's only enabled on MIPS Malta board.
>
> So your change will work on MIPS in almost all cases because a
> wait_for_bit_32 isn't expected to swap. The only problem would be if the
> CPU operates in BE, the peripheral in LE and the bus system doesn't swap
> in hardware. Then the already existing wait_for_bit_le[16|32] wouldn't
> work as expected because LE isn't enforced. This could only be solved
> when wait_for_bit would use the in_[le|be][16|32|64] primitives. But
> this could be addressed when someone wants to support such a system ;)
>
> --
> - Daniel

Great! Thank you for the explanation!
kursad