Incorrect register mask definition

[dlushni-pw]

https://github.com/Xilinx/embeddedsw/blame/3728f546f178a1bcd91cf6efc9f8921447846cec/XilinxProcessorIPLib/drivers/uartps/src/xuartps_hw.h#L119

According to mode register definition in:
https://docs.xilinx.com/r/tRsGljEoh_lnUPXyOVIunw/VVLEO6lNe6~jpmp4StFQ9Q

#define XUARTPS_MR_STOPMODE_MASK 0x000000A0U /**< Stop bits mask */
mask value should be 0xC0 not 0xA0

[anirudha1977]

Thanks.
We will check and fix it as needed.

[ManikantaGuntupalli]

Bit 7 - STTBRK (Start transmitter break), should be 1
Bit 6 - RSTTO (Restart receiver timeout counter), shouldn't be updated, so it should be 0
Bit 5 - TXDIS, should be 1 to disable TX.
Bit 4 - TXEN, should be 0 to disable TX.

So, macro should be 0xA0.

[dlushni-pw]

UART Mode Register XUARTPS_MR_OFFSET at offset 0x4 has a 2-bit field to set the desired stop mode (check XUARTPS_MR_STOPMODE_2_BIT 0x80, XUARTPS_MR_STOPMODE_1_5_BIT 0x40).
This 2-bit bitfield includes consecutive bits 7, and 6 (as evident by above and XUARTPS_MR_STOPMODE_SHIFT defined as 6)
also (0x80 | 0x40) == 0xC0,
Following the logic of the other similarly named constants (e.g. XUARTPS_MR_CHMODE_MASK, XUARTPS_MR_PARITY_MASK, XUARTPS_MR_CHARLEN_MASK, etc.) which have consecutive bits in the respective bitfields set to 1 in the defined mask value I would expect the constant in question XUARTPS_MR_STOPMODE_MASK on line 119 to be defined in a similar way to assist in masking the intended bits in read/set/clear operations on the desired bitfield. If used as such, I would expect the value of XUARTPS_MR_STOPMODE_MASK to be 0xC0 to achieve the desired effect.

I'm perplexed by your comment that seemingly refers to a completely separate register, apparently the UART Control Register XUARTPS_CR_OFFSET at offset 0x0, and the bits defined in that register. I also don't necessarily disagree that a macro that sets bits 7 and 5 in that other register could be useful for something and could be defined as a separate constant in xuartps_hw.h

However as of now I'm convinced that the XUARTPS_MR_STOPMODE_MASK constant was defined for a different purpose and is currently defined incorrectly.

[ManikantaGuntupalli]

XUARTPS_MR_STOPMODE_2_BIT 0x80 and XUARTPS_MR_STOPMODE_1_5_BIT 0x40 are for configuring the number of stop bits (2 stop bits or 1.5 stop bits), the uartps controller issues that many stop bits during stop condition.
Based on use case either XUARTPS_MR_STOPMODE_2_BIT or XUARTPS_MR_STOPMODE_1_5_BIT need to be used.

[dlushni-pw]

That is correct, and the XUARTPS_MR_STOPMODE_MASK is the bitmask that was, apparently, intended to simplify access to this 2-bit bitfield. The bits 6,7 can't be considered independently, values XUARTPS_MR_STOPMODE_2_BIT are XUARTPS_MR_STOPMODE_1_5_BIT are mutually exclusive and the XUARTPS_MR_STOPMODE_MASK mask is there to help with e.g. read-modify-write operations on this bitfield. Just like the other similar masks that I've mentioned in my previous post. For that the value of XUARTPS_MR_STOPMODE_MASK needs to be 0xC0.