Hi Maxime,

On Wed, 5 Jun 2019 at 11:51, Maxime Ripard [off-list ref] wrote:

On Tue, Jun 04, 2019 at 06:29:50PM +0200, Clément Péron wrote:

quoted

We are using RXINT bits definition when looking at RXSTA register.

These bits are equal but it's not really proper.

Introduce the RXSTA bits and use them to have coherency.

Signed-off-by: Clément Péron <redacted>
---
 drivers/media/rc/sunxi-cir.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 0504ebfc831f..572bd2257d35 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c

@@ -48,11 +48,11 @@

 /* Rx Interrupt Enable */
 #define SUNXI_IR_RXINT_REG    0x2C
-/* Rx FIFO Overflow */
+/* Rx FIFO Overflow Interrupt Enable */
 #define REG_RXINT_ROI_EN             BIT(0)
-/* Rx Packet End */
+/* Rx Packet End Interrupt Enable */
 #define REG_RXINT_RPEI_EN            BIT(1)
-/* Rx FIFO Data Available */
+/* Rx FIFO Data Available Interrupt Enable */
 #define REG_RXINT_RAI_EN             BIT(4)

 /* Rx FIFO available byte level */

@@ -60,6 +60,12 @@

 /* Rx Interrupt Status */
 #define SUNXI_IR_RXSTA_REG    0x30
+/* Rx FIFO Overflow */
+#define REG_RXSTA_ROI                        BIT(0)
+/* Rx Packet End */
+#define REG_RXSTA_RPE                        BIT(1)
+/* Rx FIFO Data Available */
+#define REG_RXSTA_RA                 BIT(4)

I'm fine with it on principle, but if the consistency needs to be
maintained then we could just reuse the above defines

There is no comment why we can reuse them, they can also be some wrong
case for example the RXINT_DRQ_EN bit is not present in RXSTA and same
for STAT bit present in RXSTA and not present in RXINT.

I have discover and read this code a month ago and this logic is
really not obvious nor explain.

Maybe this hack could be done when we will introduce a quirks, but for
the moment I really think that it's more proper and readable to
introduce them properly.

Regards,
Clément

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel