| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 28 Feb 2014 22:09:55 +0100
From: Sylwester Nawrocki <snjw23@...il.com>
To: Philipp Zabel <p.zabel@...gutronix.de>
CC: Grant Likely <grant.likely@...aro.org>,
Mauro Carvalho Chehab <m.chehab@...sung.com>,
Russell King - ARM Linux <linux@....linux.org.uk>,
Rob Herring <robh+dt@...nel.org>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Guennadi Liakhovetski <g.liakhovetski@....de>,
Tomi Valkeinen <tomi.valkeinen@...com>,
Kyungmin Park <kyungmin.park@...sung.com>,
linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v5 5/7] [media] of: move common endpoint parsing to drivers/of
On 02/27/2014 06:35 PM, Philipp Zabel wrote:
> This patch adds a new struct of_endpoint which is then embedded in struct
> v4l2_of_endpoint and contains the endpoint properties that are not V4L2
> (or even media) specific: the port number, endpoint id, local device tree
> node and remote endpoint phandle. of_graph_parse_endpoint parses those
> properties and is used by v4l2_of_parse_endpoint, which just adds the
> V4L2 MBUS information to the containing v4l2_of_endpoint structure.
>
> Signed-off-by: Philipp Zabel<p.zabel@...gutronix.de>
> ---
> Changes since v4:
> - Fixed users of struct v4l2_of_endpoint
> - Removed left-over #include<media/of_graph.h> from v4l2-of.h
> ---
> drivers/media/platform/exynos4-is/media-dev.c | 10 ++++-----
> drivers/media/platform/exynos4-is/mipi-csis.c | 2 +-
> drivers/media/v4l2-core/v4l2-of.c | 16 +++-----------
> drivers/of/base.c | 31 +++++++++++++++++++++++++++
> include/linux/of_graph.h | 20 +++++++++++++++++
> include/media/v4l2-of.h | 8 ++-----
> 6 files changed, 62 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index d0f82da..04d6ecd 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -469,10 +469,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> return 0;
>
> v4l2_of_parse_endpoint(ep,&endpoint);
> - if (WARN_ON(endpoint.port == 0) || index>= FIMC_MAX_SENSORS)
> + if (WARN_ON(endpoint.base.port == 0) || index>= FIMC_MAX_SENSORS)
> return -EINVAL;
>
> - pd->mux_id = (endpoint.port - 1)& 0x1;
> + pd->mux_id = (endpoint.base.port - 1)& 0x1;
>
> rem = of_graph_get_remote_port_parent(ep);
> of_node_put(ep);
> @@ -494,13 +494,13 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> return -EINVAL;
> }
>
> - if (fimc_input_is_parallel(endpoint.port)) {
> + if (fimc_input_is_parallel(endpoint.base.port)) {
> if (endpoint.bus_type == V4L2_MBUS_PARALLEL)
> pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601;
> else
> pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_656;
> pd->flags = endpoint.bus.parallel.flags;
> - } else if (fimc_input_is_mipi_csi(endpoint.port)) {
> + } else if (fimc_input_is_mipi_csi(endpoint.base.port)) {
> /*
> * MIPI CSI-2: only input mux selection and
> * the sensor's clock frequency is needed.
> @@ -508,7 +508,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> pd->sensor_bus_type = FIMC_BUS_TYPE_MIPI_CSI2;
> } else {
> v4l2_err(&fmd->v4l2_dev, "Wrong port id (%u) at node %s\n",
> - endpoint.port, rem->full_name);
> + endpoint.base.port, rem->full_name);
> }
> /*
> * For FIMC-IS handled sensors, that are placed under i2c-isp device
> diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
> index fd1ae65..3678ba5 100644
> --- a/drivers/media/platform/exynos4-is/mipi-csis.c
> +++ b/drivers/media/platform/exynos4-is/mipi-csis.c
> @@ -772,7 +772,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
> /* Get port node and validate MIPI-CSI channel id. */
> v4l2_of_parse_endpoint(node,&endpoint);
>
> - state->index = endpoint.port - FIMC_INPUT_MIPI_CSI2_0;
> + state->index = endpoint.base.port - FIMC_INPUT_MIPI_CSI2_0;
> if (state->index< 0 || state->index>= CSIS_MAX_ENTITIES)
> return -ENXIO;
>
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> index f919db3..b4ed9a9 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -127,17 +127,9 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> int v4l2_of_parse_endpoint(const struct device_node *node,
> struct v4l2_of_endpoint *endpoint)
> {
> - struct device_node *port_node = of_get_parent(node);
> -
> - memset(endpoint, 0, offsetof(struct v4l2_of_endpoint, head));
> -
> - endpoint->local_node = node;
> - /*
> - * It doesn't matter whether the two calls below succeed.
> - * If they don't then the default value 0 is used.
> - */
> - of_property_read_u32(port_node, "reg",&endpoint->port);
> - of_property_read_u32(node, "reg",&endpoint->id);
> + of_graph_parse_endpoint(node,&endpoint->base);
> + endpoint->bus_type = 0;
> + memset(&endpoint->bus, 0, sizeof(endpoint->bus));
>
> v4l2_of_parse_csi_bus(node, endpoint);
> /*
> @@ -147,8 +139,6 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
> if (endpoint->bus.mipi_csi2.flags == 0)
> v4l2_of_parse_parallel_bus(node, endpoint);
>
> - of_node_put(port_node);
> -
> return 0;
> }
> EXPORT_SYMBOL(v4l2_of_parse_endpoint);
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 8ecca7a..ba3cfca 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1985,6 +1985,37 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
> }
>
> /**
> + * of_graph_parse_endpoint() - parse common endpoint node properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to the OF endpoint data structure
> + *
> + * All properties are optional. If none are found, we don't set any flags.
> + * This means the port has a static configuration and no properties have
> + * to be specified explicitly.
I don't think these two sentences are needed, it's all described in the
DT binding documentation. And struct of_endpoint doesn't contain any
"flags" field.
> + * The caller should hold a reference to @node.
> + */
> +int of_graph_parse_endpoint(const struct device_node *node,
> + struct of_endpoint *endpoint)
> +{
> + struct device_node *port_node = of_get_parent(node);
> +
> + memset(endpoint, 0, sizeof(*endpoint));
> +
> + endpoint->local_node = node;
> + /*
> + * It doesn't matter whether the two calls below succeed.
> + * If they don't then the default value 0 is used.
> + */
> + of_property_read_u32(port_node, "reg",&endpoint->port);
> + of_property_read_u32(node, "reg",&endpoint->id);
> +
> + of_node_put(port_node);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(of_graph_parse_endpoint);
> +
> +/**
> * of_graph_get_next_endpoint() - get next endpoint node
> * @parent: pointer to the parent device node
> * @prev: previous endpoint node, or NULL to get first
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index 3bbeb60..2b233db 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -14,7 +14,21 @@
> #ifndef __LINUX_OF_GRAPH_H
> #define __LINUX_OF_GRAPH_H
>
> +/**
> + * struct of_endpoint - the OF graph endpoint data structure
> + * @port: identifier (value of reg property) of a port this endpoint belongs to
> + * @id: identifier (value of reg property) of this endpoint
> + * @local_node: pointer to device_node of this endpoint
> + */
> +struct of_endpoint {
> + unsigned int port;
> + unsigned int id;
> + const struct device_node *local_node;
> +};
> +
> #ifdef CONFIG_OF
> +int of_graph_parse_endpoint(const struct device_node *node,
> + struct of_endpoint *endpoint);
> struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> struct device_node *previous);
> struct device_node *of_graph_get_remote_port_parent(
> @@ -22,6 +36,12 @@ struct device_node *of_graph_get_remote_port_parent(
> struct device_node *of_graph_get_remote_port(const struct device_node *node);
> #else
>
> +static inline int of_graph_parse_endpoint(const struct device_node *node,
> + struct of_endpoint *endpoint);
> +{
> + return -ENOSYS;
> +}
> +
> static inline struct device_node *of_graph_get_next_endpoint(
> const struct device_node *parent,
> struct device_node *previous)
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> index 3a49735..70fa7b7 100644
> --- a/include/media/v4l2-of.h
> +++ b/include/media/v4l2-of.h
> @@ -51,17 +51,13 @@ struct v4l2_of_bus_parallel {
>
> /**
> * struct v4l2_of_endpoint - the endpoint data structure
> - * @port: identifier (value of reg property) of a port this endpoint belongs to
> - * @id: identifier (value of reg property) of this endpoint
> - * @local_node: pointer to device_node of this endpoint
> + * @base: struct of_endpoint containing port, id, and local of_node
> * @bus_type: bus type
> * @bus: bus configuration data structure
> * @head: list head for this structure
> */
> struct v4l2_of_endpoint {
> - unsigned int port;
> - unsigned int id;
> - const struct device_node *local_node;
> + struct of_endpoint base;
> enum v4l2_mbus_type bus_type;
> union {
> struct v4l2_of_bus_parallel parallel;
Otherwise looks good to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists