Re: [PATCH v4 06/11] media: vsp1: Provide VSP1 feature helper macro

2018-05-24 Thread Laurent Pinchart 
Hi Kieran,

Thank you for the patch.

On Thursday, 3 May 2018 16:36:17 EEST Kieran Bingham wrote:
> The VSP1 devices define their specific capabilities through features
> marked in their device info structure. Various parts of the code read
> this info structure to infer if the features are available.
> 
> Wrap this into a more readable vsp1_feature(vsp1, f) macro to ensure
> that usage is consistent throughout the driver.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/platform/vsp1/vsp1.h |  2 ++
>  drivers/media/platform/vsp1/vsp1_drv.c | 16 
>  drivers/media/platform/vsp1/vsp1_wpf.c |  6 +++---
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1.h
> b/drivers/media/platform/vsp1/vsp1.h index 33f632331474..f0d21cc8e9ab
> 100644
> --- a/drivers/media/platform/vsp1/vsp1.h
> +++ b/drivers/media/platform/vsp1/vsp1.h
> @@ -68,6 +68,8 @@ struct vsp1_device_info {
>   bool uapi;
>  };
> 
> +#define vsp1_feature(vsp1, f) ((vsp1)->info->features & (f))
> +
>  struct vsp1_device {
>   struct device *dev;
>   const struct vsp1_device_info *info;
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index d29f9c4baebe..0fc388bf5a33
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -265,7 +265,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) }
> 
>   /* Instantiate all the entities. */
> - if (vsp1->info->features & VSP1_HAS_BRS) {
> + if (vsp1_feature(vsp1, VSP1_HAS_BRS)) {
>   vsp1->brs = vsp1_brx_create(vsp1, VSP1_ENTITY_BRS);
>   if (IS_ERR(vsp1->brs)) {
>   ret = PTR_ERR(vsp1->brs);
> @@ -275,7 +275,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) list_add_tail(>brs->entity.list_dev, >entities);
>   }
> 
> - if (vsp1->info->features & VSP1_HAS_BRU) {
> + if (vsp1_feature(vsp1, VSP1_HAS_BRU)) {
>   vsp1->bru = vsp1_brx_create(vsp1, VSP1_ENTITY_BRU);
>   if (IS_ERR(vsp1->bru)) {
>   ret = PTR_ERR(vsp1->bru);
> @@ -285,7 +285,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) list_add_tail(>bru->entity.list_dev, >entities);
>   }
> 
> - if (vsp1->info->features & VSP1_HAS_CLU) {
> + if (vsp1_feature(vsp1, VSP1_HAS_CLU)) {
>   vsp1->clu = vsp1_clu_create(vsp1);
>   if (IS_ERR(vsp1->clu)) {
>   ret = PTR_ERR(vsp1->clu);
> @@ -311,7 +311,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1)
> 
>   list_add_tail(>hst->entity.list_dev, >entities);
> 
> - if (vsp1->info->features & VSP1_HAS_HGO && vsp1->info->uapi) {
> + if (vsp1_feature(vsp1, VSP1_HAS_HGO) && vsp1->info->uapi) {
>   vsp1->hgo = vsp1_hgo_create(vsp1);
>   if (IS_ERR(vsp1->hgo)) {
>   ret = PTR_ERR(vsp1->hgo);
> @@ -322,7 +322,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) >entities);
>   }
> 
> - if (vsp1->info->features & VSP1_HAS_HGT && vsp1->info->uapi) {
> + if (vsp1_feature(vsp1, VSP1_HAS_HGT) && vsp1->info->uapi) {
>   vsp1->hgt = vsp1_hgt_create(vsp1);
>   if (IS_ERR(vsp1->hgt)) {
>   ret = PTR_ERR(vsp1->hgt);
> @@ -353,7 +353,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) }
>   }
> 
> - if (vsp1->info->features & VSP1_HAS_LUT) {
> + if (vsp1_feature(vsp1, VSP1_HAS_LUT)) {
>   vsp1->lut = vsp1_lut_create(vsp1);
>   if (IS_ERR(vsp1->lut)) {
>   ret = PTR_ERR(vsp1->lut);
> @@ -387,7 +387,7 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) }
>   }
> 
> - if (vsp1->info->features & VSP1_HAS_SRU) {
> + if (vsp1_feature(vsp1, VSP1_HAS_SRU)) {
>   vsp1->sru = vsp1_sru_create(vsp1);
>   if (IS_ERR(vsp1->sru)) {
>   ret = PTR_ERR(vsp1->sru);
> @@ -537,7 +537,7 @@ static int vsp1_device_init(struct vsp1_device *vsp1)
>   vsp1_write(vsp1, VI6_DPR_HSI_ROUTE, VI6_DPR_NODE_UNUSED);
>   vsp1_write(vsp1, VI6_DPR_BRU_ROUTE, VI6_DPR_NODE_UNUSED);
> 
> - if (vsp1->info->features & VSP1_HAS_BRS)
> + if (vsp1_feature(vsp1, VSP1_HAS_BRS))
>   vsp1_write(vsp1, VI6_DPR_ILV_BRS_ROUTE, VI6_DPR_NODE_UNUSED);
> 
>   vsp1_write(vsp1, VI6_DPR_HGO_SMPPT, (7 << VI6_DPR_SMPPT_TGW_SHIFT) |
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c
> b/drivers/media/platform/vsp1/vsp1_wpf.c index 2edea361eee4..ea1d226371b2
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -141,13 +141,13 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
>   if (wpf->entity.index != 0) {
>   /* Only WPF0

[PATCH v4 06/11] media: vsp1: Provide VSP1 feature helper macro

2018-05-03 Thread Kieran Bingham 
The VSP1 devices define their specific capabilities through features
marked in their device info structure. Various parts of the code read
this info structure to infer if the features are available.

Wrap this into a more readable vsp1_feature(vsp1, f) macro to ensure
that usage is consistent throughout the driver.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1.h |  2 ++
 drivers/media/platform/vsp1/vsp1_drv.c | 16 
 drivers/media/platform/vsp1/vsp1_wpf.c |  6 +++---
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1.h 
b/drivers/media/platform/vsp1/vsp1.h
index 33f632331474..f0d21cc8e9ab 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -68,6 +68,8 @@ struct vsp1_device_info {
bool uapi;
 };
 
+#define vsp1_feature(vsp1, f) ((vsp1)->info->features & (f))
+
 struct vsp1_device {
struct device *dev;
const struct vsp1_device_info *info;
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index d29f9c4baebe..0fc388bf5a33 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -265,7 +265,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
}
 
/* Instantiate all the entities. */
-   if (vsp1->info->features & VSP1_HAS_BRS) {
+   if (vsp1_feature(vsp1, VSP1_HAS_BRS)) {
vsp1->brs = vsp1_brx_create(vsp1, VSP1_ENTITY_BRS);
if (IS_ERR(vsp1->brs)) {
ret = PTR_ERR(vsp1->brs);
@@ -275,7 +275,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
list_add_tail(>brs->entity.list_dev, >entities);
}
 
-   if (vsp1->info->features & VSP1_HAS_BRU) {
+   if (vsp1_feature(vsp1, VSP1_HAS_BRU)) {
vsp1->bru = vsp1_brx_create(vsp1, VSP1_ENTITY_BRU);
if (IS_ERR(vsp1->bru)) {
ret = PTR_ERR(vsp1->bru);
@@ -285,7 +285,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
list_add_tail(>bru->entity.list_dev, >entities);
}
 
-   if (vsp1->info->features & VSP1_HAS_CLU) {
+   if (vsp1_feature(vsp1, VSP1_HAS_CLU)) {
vsp1->clu = vsp1_clu_create(vsp1);
if (IS_ERR(vsp1->clu)) {
ret = PTR_ERR(vsp1->clu);
@@ -311,7 +311,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 
list_add_tail(>hst->entity.list_dev, >entities);
 
-   if (vsp1->info->features & VSP1_HAS_HGO && vsp1->info->uapi) {
+   if (vsp1_feature(vsp1, VSP1_HAS_HGO) && vsp1->info->uapi) {
vsp1->hgo = vsp1_hgo_create(vsp1);
if (IS_ERR(vsp1->hgo)) {
ret = PTR_ERR(vsp1->hgo);
@@ -322,7 +322,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
  >entities);
}
 
-   if (vsp1->info->features & VSP1_HAS_HGT && vsp1->info->uapi) {
+   if (vsp1_feature(vsp1, VSP1_HAS_HGT) && vsp1->info->uapi) {
vsp1->hgt = vsp1_hgt_create(vsp1);
if (IS_ERR(vsp1->hgt)) {
ret = PTR_ERR(vsp1->hgt);
@@ -353,7 +353,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
}
}
 
-   if (vsp1->info->features & VSP1_HAS_LUT) {
+   if (vsp1_feature(vsp1, VSP1_HAS_LUT)) {
vsp1->lut = vsp1_lut_create(vsp1);
if (IS_ERR(vsp1->lut)) {
ret = PTR_ERR(vsp1->lut);
@@ -387,7 +387,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
}
}
 
-   if (vsp1->info->features & VSP1_HAS_SRU) {
+   if (vsp1_feature(vsp1, VSP1_HAS_SRU)) {
vsp1->sru = vsp1_sru_create(vsp1);
if (IS_ERR(vsp1->sru)) {
ret = PTR_ERR(vsp1->sru);
@@ -537,7 +537,7 @@ static int vsp1_device_init(struct vsp1_device *vsp1)
vsp1_write(vsp1, VI6_DPR_HSI_ROUTE, VI6_DPR_NODE_UNUSED);
vsp1_write(vsp1, VI6_DPR_BRU_ROUTE, VI6_DPR_NODE_UNUSED);
 
-   if (vsp1->info->features & VSP1_HAS_BRS)
+   if (vsp1_feature(vsp1, VSP1_HAS_BRS))
vsp1_write(vsp1, VI6_DPR_ILV_BRS_ROUTE, VI6_DPR_NODE_UNUSED);
 
vsp1_write(vsp1, VI6_DPR_HGO_SMPPT, (7 << VI6_DPR_SMPPT_TGW_SHIFT) |
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c 
b/drivers/media/platform/vsp1/vsp1_wpf.c
index 2edea361eee4..ea1d226371b2 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -141,13 +141,13 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
if (wpf->entity.index != 0) {
/* Only WPF0 supports flipping. */
num_flip_ctrls = 0;
-   } else if (vsp1->info->features & VSP1_HAS_WPF_HFLIP) {
+   } else if (vsp1_feature(vsp1,