| Message ID | 47ccbcbd23e44159bbb11274b540d7c2bb66be7c.1600073975.git.mchehab+huawei@kernel.org |
|---|---|
| State | Accepted |
| Commit | 044e27ae78869c87c53805069480c5c9b2beb15e |
| Headers | show |
| Series | [RFC,01/11] media: vidtv: add modaliases for the bridge driver | expand |
Hi Mauro, > Genmask is always highest order to low order. It doesn't make > any sense to make it depends on endiannes. > I added these #ifdefs due to this: https://lwn.net/Articles/741762/ i.e. Fields to access are specified as GENMASK() values - an N-bit field starting at bit #M is encoded as GENMASK(M + N - 1, N). Note that bit numbers refer to endianness of the object we are working with - e.g. GENMASK(11, 0) in __be16 refers to the second byte and the lower 4 bits of the first byte. In __le16 it would refer to the first byte and the lower 4 bits of the second byte, etc. I am not 100% sure, but maybe we actually need them? - Daniel
Hi Daniel, Em Mon, 14 Sep 2020 12:14:38 -0300 "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu: > Hi Mauro, > > > Genmask is always highest order to low order. It doesn't make > > any sense to make it depends on endiannes. > > > > I added these #ifdefs due to this: > > https://lwn.net/Articles/741762/ > > i.e. > > Fields to access are specified as GENMASK() values - an N-bit field > starting at bit #M is encoded as GENMASK(M + N - 1, N). Note that > bit numbers refer to endianness of the object we are working with - > e.g. GENMASK(11, 0) in __be16 refers to the second byte and the lower > 4 bits of the first byte. In __le16 it would refer to the first byte > and the lower 4 bits of the second byte, etc. > > I am not 100% sure, but maybe we actually need them? By looking at the changes you did with regards to bitfields, it sounds that you didn't quite get how BE/LE works. Basically, if the CPU needs to store a value (like 0x8001) on some place, it will store two values: 0x80 and 0x01. Depending on the endiannes, either 0x80 or 0x01 will be stored first. See: https://en.wikipedia.org/wiki/Endianness In any case, when you do something like: mask = GENMASK(11, 0); ret = be16_to_cpu(s->bitfield) & mask; The be16_to_cpu() will ensure that the bits will be at the position expected by the CPU endiannes. So, no need to check for __BIG_ENDIAN or __LITTLE_ENDIAN when be*_to_cpu() macros are used. Please also notice that, when there's just one byte to be stored (e. g. 8 bits), the endiannes won't matter, as the bits will still be stored at the same way. that's why there's no be8_to_cpu() or cpu_to_be8() macros. Thanks, Mauro
diff --git a/drivers/media/test-drivers/vidtv/vidtv_psi.c b/drivers/media/test-drivers/vidtv/vidtv_psi.c index 761034d10d9d..b8b638244b1d 100644 --- a/drivers/media/test-drivers/vidtv/vidtv_psi.c +++ b/drivers/media/test-drivers/vidtv/vidtv_psi.c @@ -99,11 +99,7 @@ static inline u16 vidtv_psi_sdt_serv_get_desc_loop_len(struct vidtv_psi_table_sd u16 mask; u16 ret; - #if defined(__BIG_ENDIAN) - mask = GENMASK(0, 11); - #else mask = GENMASK(11, 0); - #endif ret = be16_to_cpu(s->bitfield) & mask; return ret; @@ -114,11 +110,7 @@ static inline u16 vidtv_psi_pmt_stream_get_desc_loop_len(struct vidtv_psi_table_ u16 mask; u16 ret; - #if defined(__BIG_ENDIAN) - mask = GENMASK(0, 9); - #else mask = GENMASK(9, 0); - #endif ret = be16_to_cpu(s->bitfield2) & mask; return ret; @@ -129,11 +121,7 @@ static inline u16 vidtv_psi_pmt_get_desc_loop_len(struct vidtv_psi_table_pmt *p) u16 mask; u16 ret; - #if defined(__BIG_ENDIAN) - mask = GENMASK(0, 9); - #else mask = GENMASK(9, 0); - #endif ret = be16_to_cpu(p->bitfield2) & mask; return ret; @@ -144,11 +132,7 @@ static inline u16 vidtv_psi_get_sec_len(struct vidtv_psi_table_header *h) u16 mask; u16 ret; - #if defined(__BIG_ENDIAN) - mask = GENMASK(0, 11); - #else mask = GENMASK(11, 0); - #endif ret = be16_to_cpu(h->bitfield) & mask; return ret; @@ -159,11 +143,7 @@ inline u16 vidtv_psi_get_pat_program_pid(struct vidtv_psi_table_pat_program *p) u16 mask; u16 ret; - #if defined(__BIG_ENDIAN) - mask = GENMASK(0, 12); - #else mask = GENMASK(12, 0); - #endif ret = be16_to_cpu(p->bitfield) & mask; return ret; @@ -174,11 +154,7 @@ inline u16 vidtv_psi_pmt_stream_get_elem_pid(struct vidtv_psi_table_pmt_stream * u16 mask; u16 ret; - #if defined(__BIG_ENDIAN) - mask = GENMASK(0, 12); - #else mask = GENMASK(12, 0); - #endif ret = be16_to_cpu(s->bitfield) & mask; return ret; @@ -189,11 +165,7 @@ static inline void vidtv_psi_set_desc_loop_len(__be16 *bitfield, u16 new_len, u8 u16 mask; __be16 new; - #if defined(__BIG_ENDIAN) - mask = GENMASK(desc_len_nbits, 15); - #else mask = GENMASK(15, desc_len_nbits); - #endif new = cpu_to_be16((be16_to_cpu(*bitfield) & mask) | new_len); *bitfield = new; @@ -205,11 +177,7 @@ static void vidtv_psi_set_sec_len(struct vidtv_psi_table_header *h, u16 new_len) __be16 new; u16 mask; - #if defined(__BIG_ENDIAN) - mask = GENMASK(13, 15); - #else mask = GENMASK(15, 13); - #endif new = cpu_to_be16((be16_to_cpu(h->bitfield) & mask) | new_len);
Genmask is always highest order to low order. It doesn't make any sense to make it depends on endiannes. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- drivers/media/test-drivers/vidtv/vidtv_psi.c | 32 -------------------- 1 file changed, 32 deletions(-)