Re: [RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sakari Ailus 
Hi Mauro,

On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.

The idea is that in the union, there's a struct which is specific to the
match_type field. I wouldn't call it senseless.

In the two cases there's just a single field in the containing struct. You
could remove the struct in that case as you do in this patch, and just use
the field. But I think the result is less clean and so I wouldn't make this
change.

The confusion comes possibly from the fact that the struct is named the
same as the field in the struct. These used to be called of and node, but
with the fwnode property framework the references to the fwnode are, well,
typically similarly called "fwnode". There's no underlying firmware
interface with that name, fwnode property API is just an API.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sylwester Nawrocki 
On 09/28/2017 02:53 PM, Mauro Carvalho Chehab wrote:
> (Resending for Mauro, while dropping the non-list recipients. The original
> likely had too many recipients.)
> 
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.
> 
> At drivers, this makes even clearer about the match criteria.
> 
> Signed-off-by: Mauro Carvalho Chehab 


Acked-by: Sylwester Nawrocki 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sakari Ailus 
Hi Mauro,

On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.

The idea is that in the union, there's a struct which is specific to the
match_type field. I wouldn't call it senseless.

In the two cases there's just a single field in the containing struct. You
could remove the struct in that case as you do in this patch, and just use
the field. But I think the result is less clean and so I wouldn't make this
change.

The confusion comes possibly from the fact that the struct is named the
same as the field in the struct. These used to be called of and node, but
with the fwnode property framework the references to the fwnode are, well,
typically similarly called "fwnode". There's no underlying firmware
interface with that name, fwnode property API is just an API.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Mauro Carvalho Chehab 
(Resending for Mauro, while dropping the non-list recipients. The original
likely had too many recipients.)

The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
match criteria requires just a device name.

So, it doesn't make sense to enclose those into structs,
as the criteria can go directly into the union.

That makes easier to document it, as we don't need to document
weird senseless structs.

At drivers, this makes even clearer about the match criteria.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/am437x/am437x-vpfe.c| 6 +++---
 drivers/media/platform/atmel/atmel-isc.c   | 2 +-
 drivers/media/platform/atmel/atmel-isi.c   | 2 +-
 drivers/media/platform/davinci/vpif_capture.c  | 4 ++--
 drivers/media/platform/exynos4-is/media-dev.c  | 4 ++--
 drivers/media/platform/omap3isp/isp.c  | 4 ++--
 drivers/media/platform/pxa_camera.c| 2 +-
 drivers/media/platform/qcom/camss-8x16/camss.c | 2 +-
 drivers/media/platform/rcar-vin/rcar-core.c| 8 
 drivers/media/platform/rcar_drif.c | 4 ++--
 drivers/media/platform/soc_camera/soc_camera.c | 2 +-
 drivers/media/platform/stm32/stm32-dcmi.c  | 2 +-
 drivers/media/platform/ti-vpe/cal.c| 2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c| 2 +-
 drivers/media/v4l2-core/v4l2-async.c   | 4 ++--
 drivers/staging/media/imx/imx-media-dev.c  | 8 
 include/media/v4l2-async.h | 8 ++--
 17 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index dfcc484cab89..f87e8f9467e9 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
 
for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
-   if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
-   asd[i].match.fwnode.fwnode) {
+   if (vpfe->cfg->asd[i]->match.fwnode ==
+   asd[i].match.fwnode) {
sdinfo = &vpfe->cfg->sub_devs[i];
vpfe->sd[i] = subdev;
vpfe->sd[i]->grp_id = sdinfo->grp_id;
@@ -2505,7 +2505,7 @@ vpfe_get_pdata(struct platform_device *pdev)
}
 
pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
-   pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
+   pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
of_node_put(rem);
}
 
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d7103c5f92c3..c04d9a4dbfac 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1742,7 +1742,7 @@ static int isc_parse_dt(struct device *dev, struct 
isc_device *isc)
subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
 
subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-   subdev_entity->asd->match.fwnode.fwnode =
+   subdev_entity->asd->match.fwnode =
of_fwnode_handle(rem);
list_add_tail(&subdev_entity->list, &isc->subdev_entities);
}
diff --git a/drivers/media/platform/atmel/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
index 891fa2505efa..8c52f9f5f2db 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1124,7 +1124,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct 
device_node *node)
/* Remote node to connect */
isi->entity.node = remote;
isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
-   isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
+   isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
return 0;
}
 }
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 0ef36cec21d1..0cf141635cbc 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
 
for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
-   const struct fwnode_handle *fwnode = _asd->match.fwnode.fwnode;
+   const struct fwnode_handle *fwnode = _asd->match.fwnode;
 
if (fwnode == subdev->fwnode) {
vpif_obj.sd[i] = subdev;
@@ -1588,7 +1588,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
}