Skip to contentSkip to conversation

vsp1: fix video output on R8A77970

PATCH In review Sergei Shtylyov 3 participants 8 messages Jan 18, 2018 – Feb 22, 2018 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 vsp1: fix video output on R8A77970

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts...

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")

---
This patch is against the 'media_tree.git' repo's 'fixes' branch.

Changes in version 3:
- reworded the comment in lif_configure();
- reworded the patch description.

Changes in version 2:
- added a comment before the V3M SoC check;
- fixed indetation in that check;
- reformatted the patch description.

 drivers/media/platform/vsp1/vsp1_lif.c  |   15 +++++++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |    5 +++++
 2 files changed, 20 insertions(+)
drivers/media/platform/vsp1/vsp1_lif.c +15 -0
Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
===================================================================
@@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
 			(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
 			(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
 			VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+	/*
+	 * On R-Car V3M the LIF0 buffer attribute register has to be set
+	 * to a non-default value to guarantee proper operation (otherwise
+	 * artifacts may appear on the output). The value required by
+	 * the manual is not explained but is likely a buffer size or
+	 * threshold...
+	 */
+	if ((entity->vsp1->version &
+	     (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
+	    (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
+		vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+			       VI6_LIF_LBA_LBA0 |
+			       (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+	}
 }
 
 static const struct vsp1_entity_operations lif_entity_ops = {
drivers/media/platform/vsp1/vsp1_regs.h +5 -0
Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
===================================================================
@@ -693,6 +693,11 @@
 #define VI6_LIF_CSBTH_LBTH_MASK		(0x7ff << 0)
 #define VI6_LIF_CSBTH_LBTH_SHIFT	0
 
+#define VI6_LIF_LBA			0x3b0c
+#define VI6_LIF_LBA_LBA0		(1 << 31)
+#define VI6_LIF_LBA_LBA1_MASK		(0xfff << 16)
+#define VI6_LIF_LBA_LBA1_SHIFT		16
+
 /* -----------------------------------------------------------------------------
  * Security Control Registers
  */
Reviewed-by on v3 1/1
#1 lore raw

Hi Sergei,

Thank you for the patch.

Quoted 35 lines · On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts...

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and

VSP2-D instances") Signed-off-by: Sergei Shtylyov
<[email protected]>

---
This patch is against the 'media_tree.git' repo's 'fixes' branch.

Changes in version 3:
- reworded the comment in lif_configure();
- reworded the patch description.

Changes in version 2:
- added a comment before the V3M SoC check;
- fixed indetation in that check;
- reformatted the patch description.

 drivers/media/platform/vsp1/vsp1_lif.c  |   15 +++++++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |    5 +++++
 2 files changed, 20 insertions(+)

Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
===================================================================

drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 164

Quoted 10 lines
drivers/media/platform/vsp1/vsp1_lif.c +7 -0
@@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
 			(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
 			(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
 			VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+	/*
+	 * On R-Car V3M the LIF0 buffer attribute register has to be set
+	 * to a non-default value to guarantee proper operation (otherwise
+	 * artifacts may appear on the output). The value required by
+	 * the manual is not explained but is likely a buffer size or
+	 * threshold...

One period is enough :-)

drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 172

Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c +8 -0
+	 */
+	if ((entity->vsp1->version &
+	     (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
+	    (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
+		vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+			       VI6_LIF_LBA_LBA0 |
+			       (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+	}

There's no need for braces or inner parentheses. To make this easier to read I propose defining a new macro VI6_IP_VERSION_MASK that combines both the model and SoC. Otherwise this looks good to me,

I'll post a v4 with that change in reply to this e-mail, please let me know if you're fine with it.

Quoted 19 lines
drivers/media/platform/vsp1/vsp1_lif.c +0 -0
 }
 
 static const struct vsp1_entity_operations lif_entity_ops = {

Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
===================================================================

drivers/media/platform/vsp1/vsp1_regs.h +5 -0
@@ -693,6 +693,11 @@
 #define VI6_LIF_CSBTH_LBTH_MASK		(0x7ff << 0)
 #define VI6_LIF_CSBTH_LBTH_SHIFT	0
 
+#define VI6_LIF_LBA			0x3b0c
+#define VI6_LIF_LBA_LBA0		(1 << 31)
+#define VI6_LIF_LBA_LBA1_MASK		(0xfff << 16)
+#define VI6_LIF_LBA_LBA1_SHIFT		16
+
 /* ------------------------------------------------------------------------
  * Security Control Registers
  */
Signature · 3 lines
Regards,

Laurent Pinchart
#2 lore raw

Hi Sergei,

Quoted 51 lines · On Thursday, 22 February 2018 18:26:20 EET Laurent Pinchart wrote:
Quoted 37 lines · On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts...

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and

VSP2-D instances") Signed-off-by: Sergei Shtylyov
<[email protected]>

---
This patch is against the 'media_tree.git' repo's 'fixes' branch.

Changes in version 3:
- reworded the comment in lif_configure();
- reworded the patch description.

Changes in version 2:
- added a comment before the V3M SoC check;
- fixed indetation in that check;
- reformatted the patch description.

 drivers/media/platform/vsp1/vsp1_lif.c  |   15 +++++++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |    5 +++++
 2 files changed, 20 insertions(+)

Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
===================================================================

drivers/media/platform/vsp1/vsp1_lif.c +0 -0
@@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
 
 			(obth << VI6_LIF_CTRL_OBTH_SHIFT) |

drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 166

Quoted 10 lines
drivers/media/platform/vsp1/vsp1_lif.c +7 -0
 			(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
 			VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
 
+
+	/*
+	 * On R-Car V3M the LIF0 buffer attribute register has to be set
+	 * to a non-default value to guarantee proper operation (otherwise
+	 * artifacts may appear on the output). The value required by
+	 * the manual is not explained but is likely a buffer size or
+	 * threshold...

One period is enough :-)

Quoted 10 lines

drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 172

Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c +8 -0
+	 */
+	if ((entity->vsp1->version &
+	     (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
+	    (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
+		vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+			       VI6_LIF_LBA_LBA0 |
+			       (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+	}

There's no need for braces or inner parentheses.

There's of course a need for the inner parentheses, please ignore this comment.

Quoted 29 lines

To make this easier to read I propose defining a new macro VI6_IP_VERSION_MASK that combines both the model and SoC. Otherwise this looks good to me,

I'll post a v4 with that change in reply to this e-mail, please let me know if you're fine with it.

Quoted 20 lines
drivers/media/platform/vsp1/vsp1_lif.c +0 -0
 }

static const struct vsp1_entity_operations lif_entity_ops = {

Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
===================================================================

drivers/media/platform/vsp1/vsp1_regs.h +5 -0
@@ -693,6 +693,11 @@
 #define VI6_LIF_CSBTH_LBTH_MASK		(0x7ff << 0)
 #define VI6_LIF_CSBTH_LBTH_SHIFT	0
 
+#define VI6_LIF_LBA			0x3b0c
+#define VI6_LIF_LBA_LBA0		(1 << 31)
+#define VI6_LIF_LBA_LBA1_MASK		(0xfff << 16)
+#define VI6_LIF_LBA_LBA1_SHIFT		16
+
 /* ----------------------------------------------------------------------
  * Security Control Registers
  */
Signature · 3 lines
Regards,

Laurent Pinchart
#3 lore raw

PATCH v4 1/1 v4l: vsp1: Fix video output on R8A77970

In reply to Laurent Pinchart

From: Sergei Shtylyov <[email protected]>

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts.

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")
[Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
---
 drivers/media/platform/vsp1/vsp1_lif.c  | 12 ++++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |  6 ++++++
 2 files changed, 18 insertions(+)
drivers/media/platform/vsp1/vsp1_lif.c +12 -0
index e6fa16d7fda8..704920753998 100644
@@ -155,6 +155,18 @@ static void lif_configure(struct vsp1_entity *entity,
 			(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
 			(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
 			VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+	/*
+	 * On R-Car V3M the LIF0 buffer attribute register has to be set to a
+	 * non-default value to guarantee proper operation (otherwise artifacts
+	 * may appear on the output). The value required by the manual is not
+	 * explained but is likely a buffer size or threshold.
+	 */
+	if ((entity->vsp1->version & VI6_IP_VERSION_MASK) ==
+	    (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M))
+		vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+			       VI6_LIF_LBA_LBA0 |
+			       (1536 << VI6_LIF_LBA_LBA1_SHIFT));
 }
 
 static const struct vsp1_entity_operations lif_entity_ops = {
drivers/media/platform/vsp1/vsp1_regs.h +6 -0
index b1912c83a1da..dae0c1901297 100644
@@ -693,6 +693,11 @@
 #define VI6_LIF_CSBTH_LBTH_MASK		(0x7ff << 0)
 #define VI6_LIF_CSBTH_LBTH_SHIFT	0
 
+#define VI6_LIF_LBA			0x3b0c
+#define VI6_LIF_LBA_LBA0		(1 << 31)
+#define VI6_LIF_LBA_LBA1_MASK		(0xfff << 16)
+#define VI6_LIF_LBA_LBA1_SHIFT		16
+
 /* -----------------------------------------------------------------------------
  * Security Control Registers
  */
@@ -705,6 +710,7 @@
  */
 
 #define VI6_IP_VERSION			0x3f00
+#define VI6_IP_VERSION_MASK		(0xffff << 0)
 #define VI6_IP_VERSION_MODEL_MASK	(0xff << 8)
 #define VI6_IP_VERSION_MODEL_VSPS_H2	(0x09 << 8)
 #define VI6_IP_VERSION_MODEL_VSPR_H2	(0x0a << 8)
Signature · 3 lines
Regards,

Laurent Pinchart
#4 lore raw

Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970

Quoted 17 lines · Laurent Pinchart

From: Sergei Shtylyov <[email protected]>

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts.

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")
[Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]

[...]

Quoted 4 lines
drivers/media/platform/vsp1/vsp1_regs.h +0 -0
index b1912c83a1da..dae0c1901297 100644

[...]

drivers/media/platform/vsp1/vsp1_regs.h line 713

Quoted 5 lines
drivers/media/platform/vsp1/vsp1_regs.h +1 -0
@@ -705,6 +710,7 @@
  */
 
 #define VI6_IP_VERSION			0x3f00
+#define VI6_IP_VERSION_MASK		(0xffff << 0)

Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be clearer?

MBR, Sergei

#5 lore raw

Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970

Hi Sergei,

Quoted 34 lines · On Thursday, 22 February 2018 20:34:37 EET Sergei Shtylyov wrote:
Quoted 19 lines · Laurent Pinchart

From: Sergei Shtylyov <[email protected]>

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts.

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and

VSP2-D instances") Signed-off-by: Sergei Shtylyov <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> [Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
Signed-off-by: Laurent Pinchart <[email protected]>

[...]

Quoted 5 lines
(quoted diff) +0 -0

b/drivers/media/platform/vsp1/vsp1_regs.h index
b1912c83a1da..dae0c1901297 100644

drivers/media/platform/vsp1/vsp1_regs.h +0 -0

[...]

Quoted 3 lines
drivers/media/platform/vsp1/vsp1_regs.h +0 -0
@@ -705,6 +710,7 @@
  */
 
Quoted 6 lines

drivers/media/platform/vsp1/vsp1_regs.h line 713

Quoted 3 lines
drivers/media/platform/vsp1/vsp1_regs.h +1 -0
 #define VI6_IP_VERSION			0x3f00
 
+#define VI6_IP_VERSION_MASK		(0xffff << 0)

Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be clearer?

I thought about it and the line length went over 80 characters so I went for an easy solution. I can change it if you want.

Signature · 3 lines
Regards,

Laurent Pinchart
#6 lore raw

Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970

Quoted 35 lines · Laurent Pinchart
Quoted 35 lines
Quoted 19 lines

From: Sergei Shtylyov <[email protected]>

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts.

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and

VSP2-D instances") Signed-off-by: Sergei Shtylyov <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> [Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
Signed-off-by: Laurent Pinchart <[email protected]>

[...]

Quoted 5 lines
(quoted diff) +0 -0

b/drivers/media/platform/vsp1/vsp1_regs.h index
b1912c83a1da..dae0c1901297 100644

drivers/media/platform/vsp1/vsp1_regs.h +0 -0

[...]

Quoted 5 lines
drivers/media/platform/vsp1/vsp1_regs.h +0 -0
@@ -705,6 +710,7 @@
  */
 
 #define VI6_IP_VERSION			0x3f00
 
Quoted 7 lines
Quoted 4 lines

drivers/media/platform/vsp1/vsp1_regs.h line 713

Quoted 1 line
drivers/media/platform/vsp1/vsp1_regs.h +1 -0
+#define VI6_IP_VERSION_MASK		(0xffff << 0)

Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be clearer?

I thought about it and the line length went over 80 characters so I went for an easy solution. I can change it if you want.

OK, let's be leave it as is.

MBR, Sergei

#7 lore raw
In reply to Laurent Pinchart
Quoted 44 lines · Laurent Pinchart

Hi Sergei,

Thank you for the patch.

Quoted 39 lines · On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL, and VSP2-D instances") added support for the VSP2-D found in the R-Car V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish garbage-like line repeated every 8 screen rows. It turns out that R-Car V3M has the LIF0 buffer attribute register that you need to set to a non- default value in order to get rid of the output artifacts...

Based on the original (and large) patch by Daisuke Matsushita <[email protected]>.

Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and

VSP2-D instances") Signed-off-by: Sergei Shtylyov
<[email protected]>

---
This patch is against the 'media_tree.git' repo's 'fixes' branch.

Changes in version 3:
- reworded the comment in lif_configure();
- reworded the patch description.

Changes in version 2:
- added a comment before the V3M SoC check;
- fixed indetation in that check;
- reformatted the patch description.

 drivers/media/platform/vsp1/vsp1_lif.c  |   15 +++++++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |    5 +++++
 2 files changed, 20 insertions(+)

Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
===================================================================

drivers/media/platform/vsp1/vsp1_lif.c +1 -0
@@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
 			(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
 			(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
 			VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
Quoted 8 lines

drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 164

Quoted 6 lines
drivers/media/platform/vsp1/vsp1_lif.c +6 -0
+	/*
+	 * On R-Car V3M the LIF0 buffer attribute register has to be set
+	 * to a non-default value to guarantee proper operation (otherwise
+	 * artifacts may appear on the output). The value required by
+	 * the manual is not explained but is likely a buffer size or
+	 * threshold...

One period is enough :-)

Naw, I just like ellipsis! :-)

Quoted 10 lines

drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 172

Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c +8 -0
+	 */
+	if ((entity->vsp1->version &
+	     (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
+	    (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
+		vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+			       VI6_LIF_LBA_LBA0 |
+			       (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+	}

There's no need for braces

Well, multi-line statement was asking for them...

Quoted 1 line

or inner parentheses.

I thought so too. :-)

Quoted 8 lines

To make this easier to read I
propose defining a new macro VI6_IP_VERSION_MASK that combines both the model and SoC. Otherwise this looks good to me,

I'll post a v4 with that change in reply to this e-mail, please let me know if you're fine with it.

Generally fine, yes...

[...]

MBR, Sergei