Re: [U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.
On Wed, Aug 14, 2019 at 03:18:33PM +0200, Philippe Reynes wrote: > From: Kursad Oney > > 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 > Reviewed-by: Philippe Reynes > Reviewed-by: Daniel Schwierzeck Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.
Hi Daniel, On Wed, Aug 14, 2019 at 10:43 AM Daniel Schwierzeck wrote: > > > > Am 14.08.19 um 15:29 schrieb Kursad Oney: > > Hi Daniel, > > > > On Wed, Aug 14, 2019 at 9:19 AM Philippe Reynes > > wrote: > >> > >> From: Kursad Oney > >> > >> 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 > >> Reviewed-by: Philippe Reynes > >> --- > >> 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.
Am 14.08.19 um 15:18 schrieb Philippe Reynes: > From: Kursad Oney > > 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 > Reviewed-by: Philippe Reynes > --- > 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 > Reviewed-by: Daniel Schwierzeck -- - Daniel ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.
Am 14.08.19 um 15:29 schrieb Kursad Oney: > Hi Daniel, > > On Wed, Aug 14, 2019 at 9:19 AM Philippe Reynes > wrote: >> >> From: Kursad Oney >> >> 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 >> Reviewed-by: Philippe Reynes >> --- >> 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.
Hi Daniel, On Wed, Aug 14, 2019 at 9:19 AM Philippe Reynes wrote: > > From: Kursad Oney > > 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 > Reviewed-by: Philippe Reynes > --- > 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? Thanks! kursad ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH V3 02/10] waitbit: Add the generic wait_for_bit macros for 16 and 32 bits.
From: Kursad Oney 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 Reviewed-by: Philippe Reynes --- 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
