[PATCH] media: i2c: adv7343: add support for asynchronous probing
From: "Lad, Prabhakar"
Both synchronous and asynchronous adv7343 subdevice probing is supported by
this patch.
Signed-off-by: Lad, Prabhakar
Cc: Guennadi Liakhovetski
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
---
drivers/media/i2c/adv7343.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
index 7606218..8080c2c 100644
--- a/drivers/media/i2c/adv7343.c
+++ b/drivers/media/i2c/adv7343.c
@@ -27,6 +27,7 @@
#include
#include
+#include
#include
#include
@@ -445,16 +446,21 @@ static int adv7343_probe(struct i2c_client *client,
ADV7343_GAIN_DEF);
state->sd.ctrl_handler = >hdl;
if (state->hdl.error) {
- int err = state->hdl.error;
-
- v4l2_ctrl_handler_free(>hdl);
- return err;
+ err = state->hdl.error;
+ goto done;
}
v4l2_ctrl_handler_setup(>hdl);
err = adv7343_initialize(>sd);
if (err)
+ goto done;
+
+ err = v4l2_async_register_subdev(>sd);
+
+done:
+ if (err < 0)
v4l2_ctrl_handler_free(>hdl);
+
return err;
}
@@ -463,6 +469,7 @@ static int adv7343_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct adv7343_state *state = to_state(sd);
+ v4l2_async_unregister_subdev(>sd);
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] media: i2c: tvp514x: add support for asynchronous probing
From: "Lad, Prabhakar"
Both synchronous and asynchronous tvp514x subdevice probing is supported by
this patch.
Signed-off-by: Prabhakar Lad
Cc: Guennadi Liakhovetski
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
---
drivers/media/i2c/tvp514x.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 864eb14..d090caf 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -36,6 +36,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1148,9 +1149,9 @@ tvp514x_probe(struct i2c_client *client, const struct
i2c_device_id *id)
/* Register with V4L2 layer as slave device */
sd = >sd;
v4l2_i2c_subdev_init(sd, client, _ops);
- strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name));
#if defined(CONFIG_MEDIA_CONTROLLER)
+ strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name));
decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
decoder->sd.entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
@@ -1176,16 +1177,22 @@ tvp514x_probe(struct i2c_client *client, const struct
i2c_device_id *id)
sd->ctrl_handler = >hdl;
if (decoder->hdl.error) {
ret = decoder->hdl.error;
-
- v4l2_ctrl_handler_free(>hdl);
- return ret;
+ goto done;
}
v4l2_ctrl_handler_setup(>hdl);
- v4l2_info(sd, "%s decoder driver registered !!\n", sd->name);
-
- return 0;
+ ret = v4l2_async_register_subdev(>sd);
+ if (!ret)
+ v4l2_info(sd, "%s decoder driver registered !!\n", sd->name);
+done:
+ if (ret < 0) {
+ v4l2_ctrl_handler_free(>hdl);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ media_entity_cleanup(>sd.entity);
+#endif
+ }
+ return ret;
}
/**
@@ -1200,6 +1207,7 @@ static int tvp514x_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct tvp514x_decoder *decoder = to_decoder(sd);
+ v4l2_async_unregister_subdev(>sd);
v4l2_device_unregister_subdev(sd);
#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC v3] media: OF: add video sync endpoint property
From: "Lad, Prabhakar"
This patch adds video sync properties as part of endpoint
properties and also support to parse them in the parser.
Signed-off-by: Lad, Prabhakar
Cc: Hans Verkuil
Cc: Laurent Pinchart
Cc: Mauro Carvalho Chehab
Cc: Guennadi Liakhovetski
Cc: Sylwester Nawrocki
Cc: Sakari Ailus
Cc: Grant Likely
Cc: Rob Herring
Cc: Rob Landley
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: Kyungmin Park
---
This patch has 10 warnings for line over 80 characters
for which I think can be ignored.
RFC v2 https://patchwork.kernel.org/patch/2578091/
RFC V1 https://patchwork.kernel.org/patch/2572341/
Changes for v3:
1: Fixed review comments pointed by Laurent and Sylwester.
.../devicetree/bindings/media/video-interfaces.txt |1 +
drivers/media/v4l2-core/v4l2-of.c | 20 ++
include/dt-bindings/media/video-interfaces.h | 17 +++
include/media/v4l2-mediabus.h | 22 +++-
4 files changed, 50 insertions(+), 10 deletions(-)
create mode 100644 include/dt-bindings/media/video-interfaces.h
diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index e022d2d..2081278 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -101,6 +101,7 @@ Optional endpoint properties
array contains only one entry.
- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous
clock mode.
+- video-sync: property indicating to sync the video on a signal in RGB.
Example
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
index aa59639..1a54530 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -100,6 +100,26 @@ static void v4l2_of_parse_parallel_bus(const struct
device_node *node,
if (!of_property_read_u32(node, "data-shift", ))
bus->data_shift = v;
+ if (!of_property_read_u32(node, "video-sync", )) {
+ switch (v) {
+ case V4L2_MBUS_VIDEO_SEPARATE_SYNC:
+ flags |= V4L2_MBUS_VIDEO_SEPARATE_SYNC;
+ break;
+ case V4L2_MBUS_VIDEO_COMPOSITE_SYNC:
+ flags |= V4L2_MBUS_VIDEO_COMPOSITE_SYNC;
+ break;
+ case V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE:
+ flags |= V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE;
+ break;
+ case V4L2_MBUS_VIDEO_SYNC_ON_GREEN:
+ flags |= V4L2_MBUS_VIDEO_SYNC_ON_GREEN;
+ break;
+ case V4L2_MBUS_VIDEO_SYNC_ON_LUMINANCE:
+ flags |= V4L2_MBUS_VIDEO_SYNC_ON_LUMINANCE;
+ break;
+ }
+ }
+
bus->flags = flags;
}
diff --git a/include/dt-bindings/media/video-interfaces.h
b/include/dt-bindings/media/video-interfaces.h
new file mode 100644
index 000..1a083dd
--- /dev/null
+++ b/include/dt-bindings/media/video-interfaces.h
@@ -0,0 +1,17 @@
+/*
+ * This header provides constants for video interface.
+ *
+ */
+
+#ifndef _DT_BINDINGS_VIDEO_INTERFACES_H
+#define _DT_BINDINGS_VIDEO_INTERFACES_H
+
+#define V4L2_MBUS_VIDEO_SEPARATE_SYNC (1 << 2)
+#define V4L2_MBUS_VIDEO_COMPOSITE_SYNC (1 << 3)
+#define V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE (1 << 4)
+#define V4L2_MBUS_VIDEO_SYNC_ON_GREEN (1 << 5)
+#define V4L2_MBUS_VIDEO_SYNC_ON_LUMINANCE (1 << 6)
+
+#define V4L2_MBUS_VIDEO_INTERFACES_END 6
+
+#endif
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 83ae07e..a4676dd 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -11,6 +11,8 @@
#ifndef V4L2_MEDIABUS_H
#define V4L2_MEDIABUS_H
+#include
+
#include
/* Parallel flags */
@@ -28,18 +30,18 @@
* V4L2_MBUS_[HV]SYNC* flags should be also used for specifying
* configuration of hardware that uses [HV]REF signals
*/
-#define V4L2_MBUS_HSYNC_ACTIVE_HIGH(1 << 2)
-#define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3)
-#define V4L2_MBUS_VSYNC_ACTIVE_HIGH(1 << 4)
-#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 5)
-#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 6)
-#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7)
-#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8)
-#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9)
+#define V4L2_MBUS_HSYNC_ACTIVE_HIGH(1 <<
(V4L2_MBUS_VIDEO_INTERFACES_END + 1))
+#define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 <<
(V4L2_MBUS_VIDEO_INTERFACES_END + 2))
+#define V4L2_MBUS_VSYNC_ACTIVE_HIGH(1 <<
(V4L2_MBUS_VIDEO_INTERFACES_END + 3))
+#define
[PATCH v2 0/2] media: i2c: tvp7002: feature enhancement
From: "Lad, Prabhakar" The first patch of the series add support for asynchronous probing and the second patch adds OF support to tvp7002 driver. Lad, Prabhakar (2): media: i2c: tvp7002: add support for asynchronous probing media: i2c: tvp7002: add OF support .../devicetree/bindings/media/i2c/tvp7002.txt | 43 drivers/media/i2c/tvp7002.c| 73 ++-- 2 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] media: i2c: tvp7002: add support for asynchronous probing
From: "Lad, Prabhakar" Both synchronous and asynchronous tvp7002 subdevice probing is supported by this patch. Signed-off-by: Lad, Prabhakar Cc: Hans Verkuil Cc: Laurent Pinchart Cc: Mauro Carvalho Chehab Cc: Guennadi Liakhovetski Cc: Sylwester Nawrocki Cc: Sakari Ailus Cc: linux-kernel@vger.kernel.org Cc: davinci-linux-open-sou...@linux.davincidsp.com --- drivers/media/i2c/tvp7002.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c index 36ad565..b577548 100644 --- a/drivers/media/i2c/tvp7002.c +++ b/drivers/media/i2c/tvp7002.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -1040,6 +1041,10 @@ static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id) } v4l2_ctrl_handler_setup(>hdl); + error = v4l2_async_register_subdev(>sd); + if (error) + goto error; + return 0; error: @@ -1064,6 +1069,7 @@ static int tvp7002_remove(struct i2c_client *c) v4l2_dbg(1, debug, sd, "Removing tvp7002 adapter" "on address 0x%x\n", c->addr); + v4l2_async_unregister_subdev(>sd); #if defined(CONFIG_MEDIA_CONTROLLER) media_entity_cleanup(>sd.entity); #endif -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] media: i2c: tvp7002: add OF support
From: "Lad, Prabhakar"
add OF support for the tvp7002 driver.
Signed-off-by: Lad, Prabhakar
Cc: Hans Verkuil
Cc: Laurent Pinchart
Cc: Mauro Carvalho Chehab
Cc: Guennadi Liakhovetski
Cc: Sylwester Nawrocki
Cc: Sakari Ailus
Cc: Grant Likely
Cc: Rob Herring
Cc: Rob Landley
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
---
Depends on patch https://patchwork.kernel.org/patch/2765851/
.../devicetree/bindings/media/i2c/tvp7002.txt | 43 +
drivers/media/i2c/tvp7002.c| 67 ++--
2 files changed, 103 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
new file mode 100644
index 000..9daebe1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
@@ -0,0 +1,43 @@
+* Texas Instruments TV7002 video decoder
+
+The TVP7002 device supports digitizing of video and graphics signal in RGB and
+YPbPr color space.
+
+Required Properties :
+- compatible : Must be "ti,tvp7002"
+
+- hsync-active: HSYNC Polarity configuration for endpoint.
+
+- vsync-active: VSYNC Polarity configuration for endpoint.
+
+- pclk-sample: Clock polarity of the endpoint.
+
+- video-sync: Video sync property of the endpoint.
+
+- ti,tvp7002-fid-polarity: Active-high Field ID polarity of the endpoint.
+
+
+For further reading of port node refer Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ i2c0@1c22000 {
+ ...
+ ...
+ tvp7002@5c {
+ compatible = "ti,tvp7002";
+ reg = <0x5c>;
+
+ port {
+ tvp7002_1: endpoint {
+ hsync-active = <1>;
+ vsync-active = <1>;
+ pclk-sample = <0>;
+ video-sync =
;
+ ti,tvp7002-fid-polarity;
+ };
+ };
+ };
+ ...
+ };
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index b577548..4896024 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -35,6 +35,8 @@
#include
#include
#include
+#include
+
#include "tvp7002_reg.h"
MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver");
@@ -943,6 +945,48 @@ static const struct v4l2_subdev_ops tvp7002_ops = {
.pad = _pad_ops,
};
+static struct tvp7002_config *
+tvp7002_get_pdata(struct i2c_client *client)
+{
+ struct v4l2_of_endpoint bus_cfg;
+ struct tvp7002_config *pdata;
+ struct device_node *endpoint;
+ unsigned int flags;
+
+ if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
+ return client->dev.platform_data;
+
+ endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ if (!endpoint)
+ return NULL;
+
+ pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ goto done;
+
+ v4l2_of_parse_endpoint(endpoint, _cfg);
+ flags = bus_cfg.bus.parallel.flags;
+
+ if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+ pdata->hs_polarity = 1;
+
+ if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+ pdata->vs_polarity = 1;
+
+ if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
+ pdata->clk_polarity = 1;
+
+ if (flags & V4L2_MBUS_VIDEO_SYNC_ON_GREEN)
+ pdata->sog_polarity = 1;
+
+ pdata->fid_polarity = of_property_read_bool(endpoint,
+ "ti,tvp7002-fid-polarity");
+
+done:
+ of_node_put(endpoint);
+ return pdata;
+}
+
/*
* tvp7002_probe - Probe a TVP7002 device
* @c: ptr to i2c_client struct
@@ -954,32 +998,32 @@ static const struct v4l2_subdev_ops tvp7002_ops = {
*/
static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id)
{
+ struct tvp7002_config *pdata = tvp7002_get_pdata(c);
struct v4l2_subdev *sd;
struct tvp7002 *device;
struct v4l2_dv_timings timings;
int polarity_a;
int polarity_b;
u8 revision;
-
int error;
+ if (pdata == NULL) {
+ dev_err(>dev, "No platform data\n");
+ return -EINVAL;
+ }
+
/* Check if the adapter supports the needed features */
if (!i2c_check_functionality(c->adapter,
I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
return -EIO;
- if (!c->dev.platform_data) {
- v4l_err(c, "No platform data!!\n");
-
[PATCH v3 2/6] ARM: davinci: da850: add DT node for mdio device
From: "Lad, Prabhakar"
Add mdio device tree node information to da850 by
providing register details and bus frequency of mdio.
Signed-off-by: Lad, Prabhakar
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: net...@vger.kernel.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Sekhar Nori
Cc: Heiko Schocher
---
Changes for v2: none
Changes for v3:
a> Added pin entries for mdio.
arch/arm/boot/dts/da850-evm.dts |6 ++
arch/arm/boot/dts/da850.dtsi| 13 +
2 files changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index c914357..661219e 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -90,6 +90,12 @@
};
};
};
+ mdio: mdio@1e24000 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ bus_freq = <220>;
+ };
};
nand_cs3@6200 {
status = "okay";
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 4660693..bcef5a9 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -125,6 +125,13 @@
0x14 0x0010 0x00f0
>;
};
+ mdio_pins: pinmux_mdio_pins {
+ pinctrl-single,bits = <
+ /* MDIO_CLK, MDIO_D */
+ 0x10 0x0088 0x00ff
+ >;
+ };
+
};
serial0: serial@1c42000 {
compatible = "ns16550a";
@@ -216,6 +223,12 @@
interrupts = <56>;
status = "disabled";
};
+ mdio: mdio@1e24000 {
+ compatible = "ti,davinci_mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x224000 0x1000>;
+ };
};
nand_cs3@6200 {
compatible = "ti,davinci-nand";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/6] ARM: davinci: da850: add ethernet driver DT support
From: "Lad, Prabhakar" This patch set enables Ethernet support through device tree model. This patch set enables mii interface only and is being tested to boot via rootfs. The rmii phy is present on the i2c gpio expander chip (UI board) for which yet support needs to be added, once the DT support for the chip is enabled, enabling rmii will be subsequnet patch. Changes for v2: 1: Enabled mdio device. 2: Fixed clock lookup. Changes for v3: 1: Fixed review comments pointed out by Sekhar. Lad, Prabhakar (6): ARM: davinci: da8xx: fix clock lookup for mdio device ARM: davinci: da850: add DT node for mdio device ARM: davinci: da850: add OF_DEV_AUXDATA entry for mdio. ARM: davinci: da850: add DT node for eth0. ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0. ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac arch/arm/boot/dts/da850-evm.dts| 11 +++ arch/arm/boot/dts/da850.dtsi | 43 ++ arch/arm/mach-davinci/board-da850-evm.c| 36 ++ arch/arm/mach-davinci/da830.c |1 + arch/arm/mach-davinci/da850.c | 46 arch/arm/mach-davinci/da8xx-dt.c | 20 arch/arm/mach-davinci/devices-da8xx.c |8 ++--- arch/arm/mach-davinci/include/mach/da8xx.h |1 + 8 files changed, 126 insertions(+), 40 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 3/6] ARM: davinci: da850: add OF_DEV_AUXDATA entry for mdio.
From: "Lad, Prabhakar"
Add OF_DEV_AUXDATA for mdio driver in da850 board dt
file to use mdio clock.
Signed-off-by: Lad, Prabhakar
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: net...@vger.kernel.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Sekhar Nori
Cc: Heiko Schocher
---
Changes for V2: none
Changes for V3: none
arch/arm/mach-davinci/da8xx-dt.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 961aea8..f33f767 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -47,6 +47,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[]
__initdata = {
OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap", NULL),
OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL),
OF_DEV_AUXDATA("ti,da830-spi", 0x01f0e000, "spi_davinci.1", NULL),
+ OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
{}
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/6] ARM: davinci: da8xx: fix clock lookup for mdio device
From: "Lad, Prabhakar"
This patch removes the clock alias for mdio device and adds a entry
in clock lookup table, this entry can now be used by both DT and NON
DT case.
Signed-off-by: Lad, Prabhakar
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: net...@vger.kernel.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Sekhar Nori
Cc: Heiko Schocher
---
changes for v2: None
Changes for v3:
a> added clock lookup entry for davinci_mdio in da830 file.
arch/arm/mach-davinci/da830.c |1 +
arch/arm/mach-davinci/da850.c |1 +
arch/arm/mach-davinci/devices-da8xx.c |8 ++--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index abbaf02..1f949a9 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -417,6 +417,7 @@ static struct clk_lookup da830_clks[] = {
CLK(NULL, "aintc",_clk),
CLK(NULL, "secu_mgr", _mgr_clk),
CLK("davinci_emac.1", NULL, _clk),
+ CLK("davinci_mdio.0", "fck", _clk),
CLK(NULL, "gpio", _clk),
CLK("i2c_davinci.2",NULL, _clk),
CLK(NULL, "usb11",_clk),
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 183a7dd..c43abee 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -461,6 +461,7 @@ static struct clk_lookup da850_clks[] = {
CLK(NULL, "arm", _clk),
CLK(NULL, "rmii", _clk),
CLK("davinci_emac.1", NULL, _clk),
+ CLK("davinci_mdio.0", "fck", _clk),
CLK("davinci-mcasp.0", NULL, _clk),
CLK("da8xx_lcdc.0", "fck", _clk),
CLK("da830-mmc.0", NULL, _clk),
diff --git a/arch/arm/mach-davinci/devices-da8xx.c
b/arch/arm/mach-davinci/devices-da8xx.c
index bf57252..a5a78d9 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -452,12 +452,8 @@ int __init da8xx_register_emac(void)
ret = platform_device_register(_mdio_device);
if (ret < 0)
return ret;
- ret = platform_device_register(_emac_device);
- if (ret < 0)
- return ret;
- ret = clk_add_alias(NULL, dev_name(_mdio_device.dev),
- NULL, _emac_device.dev);
- return ret;
+
+ return platform_device_register(_emac_device);
}
static struct resource da830_mcasp1_resources[] = {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 4/6] ARM: davinci: da850: add DT node for eth0.
From: "Lad, Prabhakar"
Add eth0 device tree node information and pinmux for mii to da850 by
providing interrupt details and local mac address of eth0.
Signed-off-by: Lad, Prabhakar
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: net...@vger.kernel.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Sekhar Nori
Cc: Heiko Schocher
---
Changes for v2: None
Changes for v3:
a> Split the pinmux for mdio and mii.
arch/arm/boot/dts/da850-evm.dts |5 +
arch/arm/boot/dts/da850.dtsi| 30 ++
2 files changed, 35 insertions(+)
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 661219e..590d884 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -96,6 +96,11 @@
pinctrl-0 = <_pins>;
bus_freq = <220>;
};
+ eth0: emac@1e2 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ };
};
nand_cs3@6200 {
status = "okay";
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index bcef5a9..0109b65 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -131,6 +131,22 @@
0x10 0x0088 0x00ff
>;
};
+ mii_pins: pinmux_mii_pins {
+ pinctrl-single,bits = <
+ /*
+* MII_TXEN, MII_TXCLK, MII_COL
+* MII_TXD_3, MII_TXD_2, MII_TXD_1
+* MII_TXD_0
+*/
+ 0x8 0x8880 0xfff0
+ /*
+* MII_RXER, MII_CRS, MII_RXCLK
+* MII_RXDV, MII_RXD_3, MII_RXD_2
+* MII_RXD_1, MII_RXD_0
+*/
+ 0xc 0x 0x
+ >;
+ };
};
serial0: serial@1c42000 {
@@ -229,6 +245,20 @@
#size-cells = <0>;
reg = <0x224000 0x1000>;
};
+ eth0: emac@1e2 {
+ compatible = "ti,davinci-dm6467-emac";
+ reg = <0x22 0x4000>;
+ ti,davinci-ctrl-reg-offset = <0x3000>;
+ ti,davinci-ctrl-mod-reg-offset = <0x2000>;
+ ti,davinci-ctrl-ram-offset = <0>;
+ ti,davinci-ctrl-ram-size = <0x2000>;
+ local-mac-address = [ 00 00 00 00 00 00 ];
+ interrupts = <33
+ 34
+ 35
+ 36
+ >;
+ };
};
nand_cs3@6200 {
compatible = "ti,davinci-nand";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac
From: "Lad, Prabhakar"
This patch makes a common function for to configure emac and calls
it appropriately in DT and non DT boot mode. The system configuration
chip CFGCHIP3, controls the emac module. This patch appropriately
configures this register for emac and sets DA850_MII_MDIO_CLKEN_PIN
GPIO pin appropriately.
Signed-off-by: Lad, Prabhakar
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: net...@vger.kernel.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Sekhar Nori
Cc: Heiko Schocher
---
Changes for v2: none
Changes for v3:
a> added a common function in da850.c to configure
the CFGCHIP3 chip.
arch/arm/mach-davinci/board-da850-evm.c| 36 ++
arch/arm/mach-davinci/da850.c | 45
arch/arm/mach-davinci/da8xx-dt.c | 16 ++
arch/arm/mach-davinci/include/mach/da8xx.h |1 +
4 files changed, 64 insertions(+), 34 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
index 8a24b6c..03dd1df 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -50,7 +50,6 @@
#include
#include
-#define DA850_EVM_PHY_ID "davinci_mdio-0:00"
#define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8)
#define DA850_LCD_BL_PIN GPIO_TO_PIN(2, 15)
@@ -60,8 +59,6 @@
#define DA850_WLAN_EN GPIO_TO_PIN(6, 9)
#define DA850_WLAN_IRQ GPIO_TO_PIN(6, 10)
-#define DA850_MII_MDIO_CLKEN_PIN GPIO_TO_PIN(2, 6)
-
static struct mtd_partition da850evm_spiflash_part[] = {
[0] = {
.name = "UBL",
@@ -1033,26 +1030,18 @@ static const short da850_evm_rmii_pins[] = {
static int __init da850_evm_config_emac(void)
{
- void __iomem *cfg_chip3_base;
- int ret;
- u32 val;
struct davinci_soc_info *soc_info = _soc_info;
u8 rmii_en = soc_info->emac_pdata->rmii_en;
+ int ret;
if (!machine_is_davinci_da850_evm())
return 0;
- cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
-
- val = __raw_readl(cfg_chip3_base);
-
if (rmii_en) {
- val |= BIT(8);
ret = davinci_cfg_reg_list(da850_evm_rmii_pins);
pr_info("EMAC: RMII PHY configured, MII PHY will not be"
" functional\n");
} else {
- val &= ~BIT(8);
ret = davinci_cfg_reg_list(da850_evm_mii_pins);
pr_info("EMAC: MII PHY configured, RMII PHY will not be"
" functional\n");
@@ -1062,28 +1051,7 @@ static int __init da850_evm_config_emac(void)
pr_warn("%s: CPGMAC/RMII mux setup failed: %d\n",
__func__, ret);
- /* configure the CFGCHIP3 register for RMII or MII */
- __raw_writel(val, cfg_chip3_base);
-
- ret = davinci_cfg_reg(DA850_GPIO2_6);
- if (ret)
- pr_warn("%s:GPIO(2,6) mux setup failed\n", __func__);
-
- ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
- if (ret) {
- pr_warn("Cannot open GPIO %d\n", DA850_MII_MDIO_CLKEN_PIN);
- return ret;
- }
-
- /* Enable/Disable MII MDIO clock */
- gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, rmii_en);
-
- soc_info->emac_pdata->phy_id = DA850_EVM_PHY_ID;
-
- ret = da8xx_register_emac();
- if (ret)
- pr_warn("%s: EMAC registration failed: %d\n", __func__, ret);
-
+ da850_config_emac(0, rmii_en);
return 0;
}
device_initcall(da850_evm_config_emac);
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index c43abee..d8021bb 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1197,6 +1197,51 @@ no_ddrpll_mem:
return ret;
}
+void __init da850_config_emac(u32 dt_mode, u32 rmii_enabled)
+{
+#define DA850_MII_MDIO_CLKEN_PIN GPIO_TO_PIN(2, 6)
+#define DA850_EVM_PHY_ID "davinci_mdio-0:00"
+
+ void __iomem *cfg_chip3_base;
+ int ret;
+ u32 val;
+
+ cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
+
+ val = readl(cfg_chip3_base);
+
+ if (rmii_enabled)
+ val |= BIT(8);
+ else
+ val &= ~BIT(8);
+
+ /* configure the CFGCHIP3 register for RMII or MII */
+ writel(val, cfg_chip3_base);
+
+ ret = davinci_cfg_reg(DA850_GPIO2_6);
+ if (ret)
+ pr_warn("%s:GPIO(2,6) mux setup failed\n", __func__);
+
+ ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
+ if (ret) {
+ pr_warn("Cannot open GPIO %d\n", DA850_MII_MDIO_CLKEN_PIN);
+ return;
+ }
+
+ /*
[PATCH v3 5/6] ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.
From: "Lad, Prabhakar"
Add OF_DEV_AUXDATA for eth0 driver in da850 board dt
file to use emac clock.
Signed-off-by: Lad, Prabhakar
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: net...@vger.kernel.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Sekhar Nori
Cc: Heiko Schocher
---
Changes for V2: none
Changes for V3: none
arch/arm/mach-davinci/da8xx-dt.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index f33f767..1eb3fa8 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -48,6 +48,9 @@ static struct of_dev_auxdata da850_auxdata_lookup[]
__initdata = {
OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL),
OF_DEV_AUXDATA("ti,da830-spi", 0x01f0e000, "spi_davinci.1", NULL),
OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
+ OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e2, "davinci_emac.1",
+ NULL),
+
{}
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: i2c: tvp514x: add support for asynchronous probing
Hi Guennadi,
Thanks for the review.
On Sun, Jun 23, 2013 at 8:49 PM, Guennadi Liakhovetski
wrote:
> On Sat, 22 Jun 2013, Prabhakar Lad wrote:
>
>> From: "Lad, Prabhakar"
>>
>> Both synchronous and asynchronous tvp514x subdevice probing is supported by
>> this patch.
>>
>> Signed-off-by: Prabhakar Lad
>> Cc: Guennadi Liakhovetski
>> Cc: Laurent Pinchart
>> Cc: Hans Verkuil
>> Cc: Sakari Ailus
>> Cc: Mauro Carvalho Chehab
>> ---
>> drivers/media/i2c/tvp514x.c | 22 +++---
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
>> index 864eb14..d090caf 100644
>> --- a/drivers/media/i2c/tvp514x.c
>> +++ b/drivers/media/i2c/tvp514x.c
>> @@ -36,6 +36,7 @@
>> #include
>> #include
>>
>> +#include
>> #include
>> #include
>> #include
>
> Ok, but this one really does too many things in one patch:
>
>> @@ -1148,9 +1149,9 @@ tvp514x_probe(struct i2c_client *client, const struct
>> i2c_device_id *id)
>> /* Register with V4L2 layer as slave device */
>> sd = >sd;
>> v4l2_i2c_subdev_init(sd, client, _ops);
>> - strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name));
>>
>> #if defined(CONFIG_MEDIA_CONTROLLER)
>> + strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name));
>
> This is unrelated
>
OK I'll split the patch or may be a line in a commit message can do ?
>> decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
>> decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> decoder->sd.entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
>> @@ -1176,16 +1177,22 @@ tvp514x_probe(struct i2c_client *client, const
>> struct i2c_device_id *id)
>> sd->ctrl_handler = >hdl;
>> if (decoder->hdl.error) {
>> ret = decoder->hdl.error;
>> -
>> - v4l2_ctrl_handler_free(>hdl);
>> - return ret;
>> + goto done;
>> }
>> v4l2_ctrl_handler_setup(>hdl);
>>
>> - v4l2_info(sd, "%s decoder driver registered !!\n", sd->name);
>> -
>> - return 0;
>> + ret = v4l2_async_register_subdev(>sd);
>> + if (!ret)
>> + v4l2_info(sd, "%s decoder driver registered !!\n", sd->name);
>>
>> +done:
>> + if (ret < 0) {
>> + v4l2_ctrl_handler_free(>hdl);
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> + media_entity_cleanup(>sd.entity);
>
> So is this - it wasn't called before in the "if (decoder->hdl.error)" case
> above.
>
Yes so fixed it up here.
Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] media: i2c: tvp7002: add support for asynchronous probing
Hi Hans, On Mon, Jun 24, 2013 at 12:44 PM, Hans Verkuil wrote: > On Sat June 22 2013 19:44:14 Prabhakar Lad wrote: >> From: "Lad, Prabhakar" >> >> Both synchronous and asynchronous tvp7002 subdevice probing >> is supported by this patch. > > Can I merge this patch without patch 2/2? Or should I wait with both until > the video sync properties have been approved? > You can go ahead and merge this one no need to wait for 2/2, I know the video sync will take more time :) Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: i2c: tvp514x: add support for asynchronous probing
Hi Hans, On Mon, Jun 24, 2013 at 12:41 PM, Hans Verkuil wrote: > On Sun June 23 2013 17:48:20 Prabhakar Lad wrote: >> Hi Guennadi, >> >> Thanks for the review. >> >> On Sun, Jun 23, 2013 at 8:49 PM, Guennadi Liakhovetski >> wrote: >> > On Sat, 22 Jun 2013, Prabhakar Lad wrote: >> > >> >> From: "Lad, Prabhakar" >> >> >> >> Both synchronous and asynchronous tvp514x subdevice probing is supported >> >> by >> >> this patch. >> >> >> >> Signed-off-by: Prabhakar Lad >> >> Cc: Guennadi Liakhovetski >> >> Cc: Laurent Pinchart >> >> Cc: Hans Verkuil >> >> Cc: Sakari Ailus >> >> Cc: Mauro Carvalho Chehab >> >> --- >> >> drivers/media/i2c/tvp514x.c | 22 +++--- >> >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c >> >> index 864eb14..d090caf 100644 >> >> --- a/drivers/media/i2c/tvp514x.c >> >> +++ b/drivers/media/i2c/tvp514x.c >> >> @@ -36,6 +36,7 @@ >> >> #include >> >> #include >> >> >> >> +#include >> >> #include >> >> #include >> >> #include >> > >> > Ok, but this one really does too many things in one patch: >> > >> >> @@ -1148,9 +1149,9 @@ tvp514x_probe(struct i2c_client *client, const >> >> struct i2c_device_id *id) >> >> /* Register with V4L2 layer as slave device */ >> >> sd = >sd; >> >> v4l2_i2c_subdev_init(sd, client, _ops); >> >> - strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name)); >> >> >> >> #if defined(CONFIG_MEDIA_CONTROLLER) >> >> + strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name)); >> > >> > This is unrelated >> > >> OK I'll split the patch or may be a line in a commit message can do ? > > Please split it up in two patches. > > Why is sd->name set anyway? And why is it moved under CONFIG_MEDIA_CONTROLLER? > It's not obvious to me. > while using tvp514x subdev with media controller based drivers, when we enumerate entities (MEDIA_IOC_ENUM_ENTITIES) to get the index id of the entity we compare the entity name with "tvp514x", So I moved it under CONFIG_MEDIA_CONTROLLER config. I hope you are OK with moving this in a separate patch. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: i2c: tvp514x: add support for asynchronous probing
Hi Hans, On Mon, Jun 24, 2013 at 2:09 PM, Hans Verkuil wrote: > On Mon June 24 2013 10:24:02 Prabhakar Lad wrote: >> Hi Hans, >> >> On Mon, Jun 24, 2013 at 12:41 PM, Hans Verkuil wrote: >> > On Sun June 23 2013 17:48:20 Prabhakar Lad wrote: >> >> Hi Guennadi, >> >> >> >> Thanks for the review. >> >> >> >> On Sun, Jun 23, 2013 at 8:49 PM, Guennadi Liakhovetski >> >> wrote: >> >> > On Sat, 22 Jun 2013, Prabhakar Lad wrote: >> >> > >> >> >> From: "Lad, Prabhakar" >> >> >> >> >> >> Both synchronous and asynchronous tvp514x subdevice probing is >> >> >> supported by >> >> >> this patch. >> >> >> >> >> >> Signed-off-by: Prabhakar Lad >> >> >> Cc: Guennadi Liakhovetski >> >> >> Cc: Laurent Pinchart >> >> >> Cc: Hans Verkuil >> >> >> Cc: Sakari Ailus >> >> >> Cc: Mauro Carvalho Chehab >> >> >> --- >> >> >> drivers/media/i2c/tvp514x.c | 22 +++--- >> >> >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> >> >> >> >> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c >> >> >> index 864eb14..d090caf 100644 >> >> >> --- a/drivers/media/i2c/tvp514x.c >> >> >> +++ b/drivers/media/i2c/tvp514x.c >> >> >> @@ -36,6 +36,7 @@ >> >> >> #include >> >> >> #include >> >> >> >> >> >> +#include >> >> >> #include >> >> >> #include >> >> >> #include >> >> > >> >> > Ok, but this one really does too many things in one patch: >> >> > >> >> >> @@ -1148,9 +1149,9 @@ tvp514x_probe(struct i2c_client *client, const >> >> >> struct i2c_device_id *id) >> >> >> /* Register with V4L2 layer as slave device */ >> >> >> sd = >sd; >> >> >> v4l2_i2c_subdev_init(sd, client, _ops); >> >> >> - strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name)); >> >> >> >> >> >> #if defined(CONFIG_MEDIA_CONTROLLER) >> >> >> + strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name)); >> >> > >> >> > This is unrelated >> >> > >> >> OK I'll split the patch or may be a line in a commit message can do ? >> > >> > Please split it up in two patches. >> > >> > Why is sd->name set anyway? And why is it moved under >> > CONFIG_MEDIA_CONTROLLER? >> > It's not obvious to me. >> > >> while using tvp514x subdev with media controller based drivers, when we >> enumerate entities (MEDIA_IOC_ENUM_ENTITIES) to get the index id >> of the entity we compare the entity name with "tvp514x", So I moved it >> under CONFIG_MEDIA_CONTROLLER config. I hope you are OK with >> moving this in a separate patch. > > Sorry, but this approach is wrong. sd->name must be a unique name, so manually > setting sd->name will fail if you have two tvp514x devices. > > There is no reason to override sd->name here, and it is actually a bug. I see > that tvp7002 has the same problem (and a bunch of others as well). > > When trying to find a tvp514x you can just use strstr() in your application. > That will work all the time as long as there is only one tvp514x. OK, then I will send out a cleanup patch removing sd->name from this driver and others too. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] tvp514x/tvp7002 remove manual setting of subdev names
From: "Lad, Prabhakar" This patch series removes manual setting of subdev names, as ideally the subdev names must be unique and this would not be the case if there are two devices. Lad, Prabhakar (2): media: i2c: tvp7002: remove manual setting of subdev name media: i2c: tvp514x: remove manual setting of subdev name drivers/media/i2c/tvp514x.c |1 - drivers/media/i2c/tvp7002.c |1 - 2 files changed, 2 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] media: i2c: tvp7002: remove manual setting of subdev name
From: "Lad, Prabhakar" This patch removes manual setting of subdev name in the probe, ideally subdev names must be unique. Signed-off-by: Lad, Prabhakar Cc: Hans Verkuil --- drivers/media/i2c/tvp7002.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c index 4896024..ba8a7b5 100644 --- a/drivers/media/i2c/tvp7002.c +++ b/drivers/media/i2c/tvp7002.c @@ -1065,7 +1065,6 @@ static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id) error = tvp7002_s_dv_timings(sd, ); #if defined(CONFIG_MEDIA_CONTROLLER) - strlcpy(sd->name, TVP7002_MODULE_NAME, sizeof(sd->name)); device->pad.flags = MEDIA_PAD_FL_SOURCE; device->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; device->sd.entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] media: i2c: tvp514x: remove manual setting of subdev name
From: "Lad, Prabhakar" This patch removes manual setting of subdev name in the probe, ideally subdev names must be unique. Signed-off-by: Lad, Prabhakar Cc: Hans Verkuil --- drivers/media/i2c/tvp514x.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c index 864eb14..6578976 100644 --- a/drivers/media/i2c/tvp514x.c +++ b/drivers/media/i2c/tvp514x.c @@ -1148,7 +1148,6 @@ tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id) /* Register with V4L2 layer as slave device */ sd = >sd; v4l2_i2c_subdev_init(sd, client, _ops); - strlcpy(sd->name, TVP514X_MODULE_NAME, sizeof(sd->name)); #if defined(CONFIG_MEDIA_CONTROLLER) decoder->pad.flags = MEDIA_PAD_FL_SOURCE; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] media: i2c: tvp514x: add support for asynchronous probing
From: "Lad, Prabhakar"
Both synchronous and asynchronous tvp514x subdevice probing
is supported by this patch.
This patch also fixes the error path by calling
media_entity_cleanup() on failure in probe when
CONFIG_MEDIA_CONTROLLER is enabled.
Signed-off-by: Prabhakar Lad
Cc: Guennadi Liakhovetski
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
---
Changes for v2:
1: Fixed review comments pointed by Hans and Guennadi.
drivers/media/i2c/tvp514x.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 6578976..b9e621b 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -36,6 +36,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1175,16 +1176,22 @@ tvp514x_probe(struct i2c_client *client, const struct
i2c_device_id *id)
sd->ctrl_handler = >hdl;
if (decoder->hdl.error) {
ret = decoder->hdl.error;
-
- v4l2_ctrl_handler_free(>hdl);
- return ret;
+ goto done;
}
v4l2_ctrl_handler_setup(>hdl);
- v4l2_info(sd, "%s decoder driver registered !!\n", sd->name);
-
- return 0;
+ ret = v4l2_async_register_subdev(>sd);
+ if (!ret)
+ v4l2_info(sd, "%s decoder driver registered !!\n", sd->name);
+done:
+ if (ret < 0) {
+ v4l2_ctrl_handler_free(>hdl);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ media_entity_cleanup(>sd.entity);
+#endif
+ }
+ return ret;
}
/**
@@ -1199,6 +1206,7 @@ static int tvp514x_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct tvp514x_decoder *decoder = to_decoder(sd);
+ v4l2_async_unregister_subdev(>sd);
v4l2_device_unregister_subdev(sd);
#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v3] media: OF: add video sync endpoint property
Hi Hans,
Thanks for the review.
On Mon, Jun 24, 2013 at 1:21 PM, Hans Verkuil wrote:
> Hi Prabhakar,
>
> On Sat June 22 2013 17:03:03 Prabhakar Lad wrote:
>> From: "Lad, Prabhakar"
>>
>> This patch adds video sync properties as part of endpoint
>> properties and also support to parse them in the parser.
>>
>> Signed-off-by: Lad, Prabhakar
>> Cc: Hans Verkuil
>
> FYI: using my private email when CC-ing me generally works better.
> I often skip v4l2-related emails to my work address since I assume those
> have either been CC-ed to my private email and/or linux-media.
>
OK hence forth I'll take care of it.
> I wondered why I never saw RFC v1/2, now I know :-)
[Snip]
>>clock mode.
>> +- video-sync: property indicating to sync the video on a signal in RGB.
>
> Please document what the various syncs actually mean.
>
OK
>>
>>
>> Example
>> diff --git a/drivers/media/v4l2-core/v4l2-of.c
>> b/drivers/media/v4l2-core/v4l2-of.c
>> index aa59639..1a54530 100644
>> --- a/drivers/media/v4l2-core/v4l2-of.c
>> +++ b/drivers/media/v4l2-core/v4l2-of.c
>> @@ -100,6 +100,26 @@ static void v4l2_of_parse_parallel_bus(const struct
>> device_node *node,
>> if (!of_property_read_u32(node, "data-shift", ))
>> bus->data_shift = v;
>>
>> + if (!of_property_read_u32(node, "video-sync", )) {
>> + switch (v) {
>> + case V4L2_MBUS_VIDEO_SEPARATE_SYNC:
>> + flags |= V4L2_MBUS_VIDEO_SEPARATE_SYNC;
>> + break;
>> + case V4L2_MBUS_VIDEO_COMPOSITE_SYNC:
>> + flags |= V4L2_MBUS_VIDEO_COMPOSITE_SYNC;
>> + break;
>> + case V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE:
>> + flags |= V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE;
>> + break;
>> + case V4L2_MBUS_VIDEO_SYNC_ON_GREEN:
>> + flags |= V4L2_MBUS_VIDEO_SYNC_ON_GREEN;
>> + break;
>> + case V4L2_MBUS_VIDEO_SYNC_ON_LUMINANCE:
>> + flags |= V4L2_MBUS_VIDEO_SYNC_ON_LUMINANCE;
>> + break;
>> + }
>> + }
>> +
>> bus->flags = flags;
>>
>> }
>> diff --git a/include/dt-bindings/media/video-interfaces.h
>> b/include/dt-bindings/media/video-interfaces.h
>> new file mode 100644
>> index 000..1a083dd
>> --- /dev/null
>> +++ b/include/dt-bindings/media/video-interfaces.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * This header provides constants for video interface.
>> + *
>> + */
>> +
>> +#ifndef _DT_BINDINGS_VIDEO_INTERFACES_H
>> +#define _DT_BINDINGS_VIDEO_INTERFACES_H
>> +
>> +#define V4L2_MBUS_VIDEO_SEPARATE_SYNC(1 << 2)
>> +#define V4L2_MBUS_VIDEO_COMPOSITE_SYNC (1 << 3)
>> +#define V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE(1 << 4)
>
> What on earth is the difference between "COMPOSITE_SYNC" and
> "SYNC_ON_COMPOSITE"?!
>
Ahh my bad.
>> +#define V4L2_MBUS_VIDEO_SYNC_ON_GREEN(1 << 5)
>> +#define V4L2_MBUS_VIDEO_SYNC_ON_LUMINANCE(1 << 6)
>> +
>> +#define V4L2_MBUS_VIDEO_INTERFACES_END 6
>> +
>> +#endif
>
> Why would this be here? It isn't Device Tree specific, the same defines can
> be used without DT as well.
>
This is in here because we cannot include header files from other folder in
device tree files.
>> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
>> index 83ae07e..a4676dd 100644
>> --- a/include/media/v4l2-mediabus.h
>> +++ b/include/media/v4l2-mediabus.h
>> @@ -11,6 +11,8 @@
>> #ifndef V4L2_MEDIABUS_H
>> #define V4L2_MEDIABUS_H
>>
>> +#include
>> +
>> #include
>>
>> /* Parallel flags */
>> @@ -28,18 +30,18 @@
>> * V4L2_MBUS_[HV]SYNC* flags should be also used for specifying
>> * configuration of hardware that uses [HV]REF signals
>> */
>> -#define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2)
>> -#define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3)
>> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4)
>> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 5)
>> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 6)
>> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING(1 << 7)
>> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8)
>> -
Re: [PATCH RFC v3] media: OF: add video sync endpoint property
Hi Hans, On Mon, Jun 24, 2013 at 1:21 PM, Hans Verkuil wrote: > Hi Prabhakar, > > On Sat June 22 2013 17:03:03 Prabhakar Lad wrote: >> From: "Lad, Prabhakar" >> [snip] >> +#ifndef _DT_BINDINGS_VIDEO_INTERFACES_H >> +#define _DT_BINDINGS_VIDEO_INTERFACES_H >> + >> +#define V4L2_MBUS_VIDEO_SEPARATE_SYNC(1 << 2) >> +#define V4L2_MBUS_VIDEO_COMPOSITE_SYNC (1 << 3) >> +#define V4L2_MBUS_VIDEO_SYNC_ON_COMPOSITE(1 << 4) > > What on earth is the difference between "COMPOSITE_SYNC" and > "SYNC_ON_COMPOSITE"?! > This link http://en.wikipedia.org/wiki/Component_video_sync would give a better explanation about it. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] media: davinci: vpif: capture: add V4L2-async support
From: "Lad, Prabhakar"
Add support for asynchronous subdevice probing, using the v4l2-async API.
The legacy synchronous mode is still supported too, which allows to
gradually update drivers and platforms.
Signed-off-by: Prabhakar Lad
Cc: Guennadi Liakhovetski
Cc: Hans Verkuil
Cc: Laurent Pinchart
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
---
drivers/media/platform/davinci/vpif_capture.c | 151 +
drivers/media/platform/davinci/vpif_capture.h |2 +
include/media/davinci/vpif_types.h|2 +
3 files changed, 107 insertions(+), 48 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index 5514175..b11d7a7 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1979,6 +1979,76 @@ vpif_init_free_channel_objects:
return err;
}
+static int vpif_async_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_subdev *asd)
+{
+ int i;
+
+ for (i = 0; i < vpif_obj.config->subdev_count; i++)
+ if (!strcmp(vpif_obj.config->subdev_info[i].name,
+ subdev->name)) {
+ vpif_obj.sd[i] = subdev;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int vpif_probe_complete(void)
+{
+ struct common_obj *common;
+ struct channel_obj *ch;
+ int i, j, err, k;
+
+ for (j = 0; j < VPIF_CAPTURE_MAX_DEVICES; j++) {
+ ch = vpif_obj.dev[j];
+ ch->channel_id = j;
+ common = &(ch->common[VPIF_VIDEO_INDEX]);
+ spin_lock_init(>irqlock);
+ mutex_init(>lock);
+ ch->video_dev->lock = >lock;
+ /* Initialize prio member of channel object */
+ v4l2_prio_init(>prio);
+ video_set_drvdata(ch->video_dev, ch);
+
+ /* select input 0 */
+ err = vpif_set_input(vpif_obj.config, ch, 0);
+ if (err)
+ goto probe_out;
+
+ err = video_register_device(ch->video_dev,
+ VFL_TYPE_GRABBER, (j ? 1 : 0));
+ if (err)
+ goto probe_out;
+ }
+
+ v4l2_info(_obj.v4l2_dev, "VPIF capture driver initialized\n");
+ return 0;
+
+probe_out:
+ for (k = 0; k < j; k++) {
+ /* Get the pointer to the channel object */
+ ch = vpif_obj.dev[k];
+ /* Unregister video device */
+ video_unregister_device(ch->video_dev);
+ }
+ kfree(vpif_obj.sd);
+ for (i = 0; i < VPIF_CAPTURE_MAX_DEVICES; i++) {
+ ch = vpif_obj.dev[i];
+ /* Note: does nothing if ch->video_dev == NULL */
+ video_device_release(ch->video_dev);
+ }
+ v4l2_device_unregister(_obj.v4l2_dev);
+
+ return err;
+}
+
+static int vpif_async_complete(struct v4l2_async_notifier *notifier)
+{
+ return vpif_probe_complete();
+}
+
/**
* vpif_probe : This function probes the vpif capture driver
* @pdev: platform device pointer
@@ -1989,12 +2059,10 @@ vpif_init_free_channel_objects:
static __init int vpif_probe(struct platform_device *pdev)
{
struct vpif_subdev_info *subdevdata;
- struct vpif_capture_config *config;
- int i, j, k, err;
+ int i, j, err;
int res_idx = 0;
struct i2c_adapter *i2c_adap;
struct channel_obj *ch;
- struct common_obj *common;
struct video_device *vfd;
struct resource *res;
int subdev_count;
@@ -2068,10 +2136,9 @@ static __init int vpif_probe(struct platform_device
*pdev)
}
}
- i2c_adap = i2c_get_adapter(1);
- config = pdev->dev.platform_data;
+ vpif_obj.config = pdev->dev.platform_data;
- subdev_count = config->subdev_count;
+ subdev_count = vpif_obj.config->subdev_count;
vpif_obj.sd = kzalloc(sizeof(struct v4l2_subdev *) * subdev_count,
GFP_KERNEL);
if (vpif_obj.sd == NULL) {
@@ -2080,54 +2147,42 @@ static __init int vpif_probe(struct platform_device
*pdev)
goto vpif_sd_error;
}
- for (i = 0; i < subdev_count; i++) {
- subdevdata = >subdev_info[i];
- vpif_obj.sd[i] =
- v4l2_i2c_new_subdev_board(_obj.v4l2_dev,
- i2c_adap,
- >board_info,
- NULL);
-
- if (!vpif_obj.sd[i]) {
- vpif_err("Error
Re: [PATCH v4 3/6] gpio: davinci: use irqdomain
Hi grygorii,
Thanks for the review.
On Mon, Nov 4, 2013 at 11:57 PM, Grygorii Strashko
wrote:
> Hi Prabhakar Lad,
>
> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>> From: "Lad, Prabhakar"
>>
>> This patch converts the davinci gpio driver to use irqdomain
>> support.
>
> This patch needs to be splitted in two:
> 1) add IRQ domain support
> 2) remove intc_irq_num
>
OK
>>
>> Signed-off-by: Lad, Prabhakar
>> ---
>> arch/arm/mach-davinci/da830.c |1 -
>> arch/arm/mach-davinci/da850.c |1 -
>> arch/arm/mach-davinci/dm355.c |1 -
>> arch/arm/mach-davinci/dm365.c |1 -
>> arch/arm/mach-davinci/dm644x.c |1 -
>> arch/arm/mach-davinci/dm646x.c |1 -
>> drivers/gpio/gpio-davinci.c| 49
>> ++--
>> include/linux/platform_data/gpio-davinci.h |3 +-
>> 8 files changed, 32 insertions(+), 26 deletions(-)
>>
>
> [...]
>
>>
>> int __init dm646x_gpio_register(void)
>> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
>> index 95c6df1..bcb6d8d 100644
>> --- a/drivers/gpio/gpio-davinci.c
>> +++ b/drivers/gpio/gpio-davinci.c
>> @@ -16,6 +16,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -292,7 +293,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>> __raw_writel(status, >intstat);
>>
>> /* now demux them to the right lowlevel handler */
>> - n = d->irq_base;
>> + n = irq_find_mapping(d->irq_domain, 0);
>
> Sorry, but I don't understand why have you used <0> as hwirq?
>
> All below logic may not work in case if we switch to use Linear IRQ domain :(
> - irq_create_mapping() may return ANY Linux IRQ number. It means, for
> example, for bank0(ngpio=32)[0] it may return Linux_IRQ=200 or 201 or any
> other.
> Also, for bank3(ngpio=16)[0] it may return Linux_IRQ=150, etc.
> - More over, if you call irq_create_mapping() 32 times you may NOT get
> sequential Linux_IRQ numbers.
>
> So, the better sequence here can be smth. as below
> (I can't verify it - my HW support only unbanked IRQs):
>
Yeah below logic works fine for banked IRQs.
> if (irq & 1)
> mask <<= 16;
>
> while (1) {
> u32 status;
> int bit;
>
> /* ack any irqs */
> status = __raw_readl(>intstat) & mask;
> if (!status)
> break;
> __raw_writel(status, >intstat);
>
> /* now demux them to the right lowlevel handler */
> while (status) {
> bit = __ffs(status);
> status &= ~(1 << bit);
> generic_handle_irq(irq_find_mapping(d->irq_domain,
> bit));
> }
> }
>
>
>> if (irq & 1) {
>> n += 16;
>> status >>= 16;
>> @@ -313,10 +314,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
>> unsigned offset)
>> {
>> struct davinci_gpio_controller *d = chip2controller(chip);
>>
>> - if (d->irq_base >= 0)
>> - return d->irq_base + offset;
>> - else
>> - return -ENODEV;
>> + return irq_find_mapping(d->irq_domain, offset);
>
> I think you can use irq_create_mapping() here instead of
> irq_find_mapping(), so IRQ will be mapped/allocated on demand.
> Also, it seems, above code may crash in case if SoC has >1 GPIO banks and
> gpio_unbanked > 0 and someone will call gpio_to_irq(>31).
>
Fixed it.
>> }
>>
>> static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
>> @@ -373,6 +371,7 @@ static int davinci_gpio_irq_setup(struct platform_device
>> *pdev)
>> struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
>> struct davinci_gpio_platform_data *pdata = dev->platform_data;
>> struct davinci_gpio_regs __iomem *g;
>> + int gpio_irq = 0;
>>
>> ngpio = pdata->ngpio;
>> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> @@ -402,9 +401,15 @@ static int davinci_gpio_irq_setup(struct
>> platform_device *pdev)
>>*/
>> for (gpio = 0, bank = 0; gpio < ngpio;
Re: [PATCH v4 4/6] gpio: davinci: add OF support
Hi Grygorii,
Thanks for the review.
On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
wrote:
> Hi Prabhakar Lad,
>
>
> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>
>> From: KV Sujith
>>
>> This patch adds OF parser support for davinci gpio
>> driver and also appropriate documentation in gpio-davinci.txt
>> located at Documentation/devicetree/bindings/gpio/.
>
>
> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
I looked for the other OF GPIO implementations with same "ngpio"
property (marvel, msm) but I don’t see of_xlate() callback implemented.
> - From one side, Davinci GPIO controller in DT described by one entry
> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>
> - From other side, on Linux level more than one gpio_chip objects are
> instantiated (one per each 32 GPIO).
>
> How the standard GPIO biding will work in this case? .. And will they?
>
> Linus, I'd very appreciate if you will be able to clarify this point.
>
>
>>
>> Signed-off-by: KV Sujith
>> Signed-off-by: Philip Avinash
>> [prabhakar.cse...@gmail.com: simplified the OF code, removed
>> unnecessary DT property and also simplified
>> the commit message]
>> Signed-off-by: Lad, Prabhakar
>> ---
>> .../devicetree/bindings/gpio/gpio-davinci.txt | 32
>> drivers/gpio/gpio-davinci.c| 54
>> ++--
>> 2 files changed, 83 insertions(+), 3 deletions(-)
>> create mode 100644
>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> new file mode 100644
>> index 000..55aae1c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> @@ -0,0 +1,32 @@
>> +Davinci GPIO controller bindings
>> +
>> +Required Properties:
>> +- compatible: should be "ti,dm6441-gpio"
>> +
>> +- reg: Physical base address of the controller and the size of memory
>> mapped
>> + registers.
>> +
>> +- gpio-controller : Marks the device node as a gpio controller.
>> +
>> +- interrupts: Array of GPIO interrupt number.
>
>
> May be meaning of property need to be extended, because,
> as of now, only banked or unbanked IRQs are supported - and not both.
>
>
OK
>> +
>> +- ti,ngpio: The number of GPIO pins supported.
>> +
>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>> interrupt
>> +line to processor.
>
>
> Should interrupt-controller; specifier be added here?
>
No
>
>> +
>> +Example:
>> +
>> +gpio: gpio@1e26000 {
>> + compatible = "ti,dm6441-gpio";
>> + gpio-controller;
>> + reg = <0x226000 0x1000>;
>> + interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>> + 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>> + 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>> + 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>> + 50 IRQ_TYPE_EDGE_BOTH>;
>> + ti,ngpio = <144>;
>> + ti,davinci-gpio-irq-base = <101>;
>
>
> ^^ Is it still needed?
>
OOps missed to remove that.
Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
Hi Linus, On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij wrote: > On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar > wrote: > >> From: "Lad, Prabhakar" >> >> This patch fixes the check for the offset in >> gpio_to_irq_unbanked() function. >> >> Signed-off-by: Lad, Prabhakar > > Is this a regression that should go in right now? > Yes it needs too. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 4/6] gpio: davinci: add OF support
Hi Grygorii,
On Tue, Nov 5, 2013 at 10:29 PM, Grygorii Strashko
wrote:
> On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
>
>>
>> Thanks for the review.
>>
>> On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
>> wrote:
>>> Hi Prabhakar Lad,
>>>
>>>
>>> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>>>
>>>> From: KV Sujith
>>>>
>>>> This patch adds OF parser support for davinci gpio
>>>> driver and also appropriate documentation in gpio-davinci.txt
>>>> located at Documentation/devicetree/bindings/gpio/.
>>>
>>>
>>> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
>>
>> I looked for the other OF GPIO implementations with same "ngpio"
>> property (marvel, msm) but I don’t see of_xlate() callback implemented.
>
> The question: will below definitions in DT work or not after this series?
> Will of_get_gpio()/of_get_named_gpio() work?
>
> Example1 - leds:
> leds {
> compatible = "gpio-leds";
> debug0 {
> label = "green:debug0";
> gpios = < 29 GPIO_ACTIVE_HIGH>;
> };
> };
>
> Example2 - any dev:
> devA {
> compatible = "devA";
> gpios = < 120 GPIO_ACTIVE_LOW>;
>
> }
>
>
Agreed of_get_gpio()/of_get_named_gpio() wont work without
xlate callback implemented, but I think this can be added as a
incremental patch later.
>>
>>> - From one side, Davinci GPIO controller in DT described by one entry
>>> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>>>
>>> - From other side, on Linux level more than one gpio_chip objects are
>>> instantiated (one per each 32 GPIO).
>>>
>>> How the standard GPIO biding will work in this case? .. And will they?
>>>
>>> Linus, I'd very appreciate if you will be able to clarify this point.
>>>
>>>
>>>>
>>>> Signed-off-by: KV Sujith
>>>> Signed-off-by: Philip Avinash
>>>> [prabhakar.cse...@gmail.com: simplified the OF code, removed
>>>> unnecessary DT property and also simplified
>>>> the commit message]
>>>> Signed-off-by: Lad, Prabhakar
>>>> ---
>>>>.../devicetree/bindings/gpio/gpio-davinci.txt | 32
>>>>
>>>>drivers/gpio/gpio-davinci.c| 54
>>>> ++--
>>>>2 files changed, 83 insertions(+), 3 deletions(-)
>>>>create mode 100644
>>>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> new file mode 100644
>>>> index 000..55aae1c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> @@ -0,0 +1,32 @@
>>>> +Davinci GPIO controller bindings29
>
>>>> +
>>>> +Required Properties:
>>>> +- compatible: should be "ti,dm6441-gpio"
>>>> +
>>>> +- reg: Physical base address of the controller and the size of memory
>>>> mapped
>>>> + registers.
>>>> +
>>>> +- gpio-controller : Marks the device node as a gpio controller.
>>>> +
>>>> +- interrupts: Array of GPIO interrupt number.
>>>
>>>
>>> May be meaning of property need to be extended, because,
>>> as of now, only banked or unbanked IRQs are supported - and not both.
>>>
>>>
>> OK
>>
>>>> +
>>>> +- ti,ngpio: The number of GPIO pins supported.
>>>> +
>>>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>>>> interrupt
>>>> +line to processor.
>>>
>>>
>>> Should interrupt-controller; specifier be added here?
>>>
>> No
>
> So, it would be impossible to map GPIO IRQ to device through DT. Right?
> Like:
> devX@0 {
> compatible = "devX";
> interrupt-parent = <>;
> interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */
>
>
> };
>
>
may be I took you wrong here, the interrupt-controller is inherited
property taken from its parent, so didn’t mention that in the documentation
Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
Hi Linus, On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij wrote: > On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad > wrote: >> On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij >> wrote: >>> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar >>> wrote: >>> >>>> From: "Lad, Prabhakar" >>>> >>>> This patch fixes the check for the offset in >>>> gpio_to_irq_unbanked() function. >>>> >>>> Signed-off-by: Lad, Prabhakar >>> >>> Is this a regression that should go in right now? >>> >> Yes it needs too. > > But on top of *what* exactly? > > This does not apply to my gpio tree devel branch and > not even on the mainline kernel. > Looks like this needs to go via ARM tree as the earlier patches have gone via ARM tree itself [1]. If you can ACK it Sekhar can get it in via ARM tree. > Is this something that should go on top of the davinci > GPIO patch set that is still being elaborated on? > Nope. [1] http://www.spinics.net/lists/arm-kernel/msg275267.html Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ths7303: Declare as static a private function
Hi Ricardo, Thanks for the patch. On Wed, Nov 6, 2013 at 8:10 PM, Ricardo Ribalda Delgado wrote: > git grep shows that the function is only called from ths7303.c > > Fix this build warning: > > CC drivers/media/i2c/ths7303.o > drivers/media/i2c/ths7303.c:86:5: warning: no previous prototype for > ‘ths7303_setval’ [-Wmissing-prototypes] >int ths7303_setval(struct v4l2_subdev *sd, enum ths7303_filter_mode mode) > ^ > > Signed-off-by: Ricardo Ribalda Delgado > Acked-by: Laurent Pinchart Acked-by: Lad, Prabhakar Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] DaVinci: GPIO: fixes
From: "Lad, Prabhakar" This patch series fixes gpio driver regestration and offset check for unbanked gpio. Lad, Prabhakar (2): gpio: davinci: Fix a check for unbanked gpio ARM: davinci: Fix number of resources passed to davinci_gpio_register() call arch/arm/mach-davinci/dm355.c |2 +- arch/arm/mach-davinci/dm365.c |2 +- arch/arm/mach-davinci/dm644x.c |2 +- arch/arm/mach-davinci/dm646x.c |2 +- drivers/gpio/gpio-davinci.c|4 +++- 5 files changed, 7 insertions(+), 5 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ARM: davinci: Fix number of resources passed to davinci_gpio_register() call
From: "Lad, Prabhakar"
The davinci_gpio_register() function expects the number of
resources as the second parameter, but size of resources
was passed to it due to which it was causing unexpected
behaviour. This patch fixes the same by passing the
ARRAY_SIZE of resources.
Reported-by: Grygorii Strashko
Signed-off-by: Lad, Prabhakar
---
arch/arm/mach-davinci/dm355.c |2 +-
arch/arm/mach-davinci/dm365.c |2 +-
arch/arm/mach-davinci/dm644x.c |2 +-
arch/arm/mach-davinci/dm646x.c |2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index ef9ff1f..536ce52 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -906,7 +906,7 @@ static struct davinci_gpio_platform_data
dm355_gpio_platform_data = {
int __init dm355_gpio_register(void)
{
return davinci_gpio_register(dm355_gpio_resources,
-sizeof(dm355_gpio_resources),
+ARRAY_SIZE(dm355_gpio_resources),
_gpio_platform_data);
}
/*--*/
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 1511a06..9c96520 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -720,7 +720,7 @@ static struct davinci_gpio_platform_data
dm365_gpio_platform_data = {
int __init dm365_gpio_register(void)
{
return davinci_gpio_register(dm365_gpio_resources,
-sizeof(dm365_gpio_resources),
+ARRAY_SIZE(dm365_gpio_resources),
_gpio_platform_data);
}
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 143a321..72a3aa7 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -792,7 +792,7 @@ static struct davinci_gpio_platform_data
dm644_gpio_platform_data = {
int __init dm644x_gpio_register(void)
{
return davinci_gpio_register(dm644_gpio_resources,
-sizeof(dm644_gpio_resources),
+ARRAY_SIZE(dm644_gpio_resources),
_gpio_platform_data);
}
/*--*/
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 2a73f29..d1b646c 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -769,7 +769,7 @@ static struct davinci_gpio_platform_data
dm646x_gpio_platform_data = {
int __init dm646x_gpio_register(void)
{
return davinci_gpio_register(dm646x_gpio_resources,
-sizeof(dm646x_gpio_resources),
+ARRAY_SIZE(dm646x_gpio_resources),
_gpio_platform_data);
}
/*--*/
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] gpio: davinci: Fix a check for unbanked gpio
From: "Lad, Prabhakar" This patch fixes a check for offset in gpio_to_irq_unbanked() and also assigns gpio_irq, gpio_unbanked of chips[0] to appropriate values which is used in gpio_to_irq_unbanked() function. Reported-by: Grygorii Strashko Signed-off-by: Lad, Prabhakar --- drivers/gpio/gpio-davinci.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c index 8847adf..84be701 100644 --- a/drivers/gpio/gpio-davinci.c +++ b/drivers/gpio/gpio-davinci.c @@ -327,7 +327,7 @@ static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset) * NOTE: we assume for now that only irqs in the first gpio_chip * can provide direct-mapped IRQs to AINTC (up to 32 GPIOs). */ - if (offset < d->irq_base) + if (offset < d->gpio_unbanked) return d->gpio_irq + offset; else return -ENODEV; @@ -419,6 +419,8 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev) /* pass "bank 0" GPIO IRQs to AINTC */ chips[0].chip.to_irq = gpio_to_irq_unbanked; + chips[0].gpio_irq = bank_irq; + chips[0].gpio_unbanked = pdata->gpio_unbanked; binten = BIT(0); /* AINTC handles mask/unmask; GPIO handles triggering */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 0/7] gpio: daVinci: cleanup and feature enhancement
From: KV Sujith This patch series does the following 1> Ports the driver to use irqdomain. 2> Adds dt binding support for gpio-davinci. 3> Adds DA850 dt support goio. Changes for v5: 1: Fixed review comments pointed by Grygorii. 2: Dropped the fixup patch This patch series is based on following fixup series http://us.generation-nt.com/answer/patch-0-2-davinci-gpio-fixes-help-212978272.html KV Sujith (3): gpio: davinci: add OF support ARM: davinci: da850: add GPIO DT node ARM: davinci: da850 evm: add GPIO pinumux entries DT node Lad, Prabhakar (4): gpio: davinci: remove unnecessary printk gpio: davinci: use readl/writel instead of __raw_* gpio: davinci: use irqdomain gpio: davinci: remove unused variable intc_irq_num .../devicetree/bindings/gpio/gpio-davinci.txt | 39 + arch/arm/boot/dts/da850-evm.dts| 20 +++ arch/arm/boot/dts/da850.dtsi | 14 ++ arch/arm/mach-davinci/da830.c |1 - arch/arm/mach-davinci/da850.c |1 - arch/arm/mach-davinci/dm355.c |1 - arch/arm/mach-davinci/dm365.c |1 - arch/arm/mach-davinci/dm644x.c |1 - arch/arm/mach-davinci/dm646x.c |1 - drivers/gpio/gpio-davinci.c| 170 +--- include/linux/platform_data/gpio-davinci.h |3 +- 11 files changed, 185 insertions(+), 67 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/7] gpio: davinci: remove unnecessary printk
From: "Lad, Prabhakar"
the devm_*() helper prints error messages in case of
errors no need to do the same in the driver.
Signed-off-by: Lad, Prabhakar
---
drivers/gpio/gpio-davinci.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 84be701..7230c43 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -389,11 +389,9 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
}
clk = devm_clk_get(dev, "gpio");
- if (IS_ERR(clk)) {
- printk(KERN_ERR "Error %ld getting gpio clock?\n",
- PTR_ERR(clk));
+ if (IS_ERR(clk))
return PTR_ERR(clk);
- }
+
clk_prepare_enable(clk);
/*
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 4/7] gpio: davinci: remove unused variable intc_irq_num
From: "Lad, Prabhakar"
As the davinci-gpio driver is migrated to use irqdomain
there is no need to pass the irq base for the gpio driver.
This patch removes this variable from davinci_gpio_platform_data
and also the references from the machine file.
Signed-off-by: Lad, Prabhakar
---
arch/arm/mach-davinci/da830.c |1 -
arch/arm/mach-davinci/da850.c |1 -
arch/arm/mach-davinci/dm355.c |1 -
arch/arm/mach-davinci/dm365.c |1 -
arch/arm/mach-davinci/dm644x.c |1 -
arch/arm/mach-davinci/dm646x.c |1 -
include/linux/platform_data/gpio-davinci.h |1 -
7 files changed, 7 deletions(-)
diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 0813b51..fb72035 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -1153,7 +1153,6 @@ static struct davinci_id da830_ids[] = {
static struct davinci_gpio_platform_data da830_gpio_platform_data = {
.ngpio = 128,
- .intc_irq_num = DA830_N_CP_INTC_IRQ,
};
int __init da830_register_gpio(void)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 352984e..4379317 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1283,7 +1283,6 @@ int __init da850_register_vpif_capture(struct
vpif_capture_config
static struct davinci_gpio_platform_data da850_gpio_platform_data = {
.ngpio = 144,
- .intc_irq_num = DA850_N_CP_INTC_IRQ,
};
int __init da850_register_gpio(void)
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index 536ce52..a1713cc 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -900,7 +900,6 @@ static struct resource dm355_gpio_resources[] = {
static struct davinci_gpio_platform_data dm355_gpio_platform_data = {
.ngpio = 104,
- .intc_irq_num = DAVINCI_N_AINTC_IRQ,
};
int __init dm355_gpio_register(void)
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 9c96520..2595097 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -713,7 +713,6 @@ static struct resource dm365_gpio_resources[] = {
static struct davinci_gpio_platform_data dm365_gpio_platform_data = {
.ngpio = 104,
- .intc_irq_num = DAVINCI_N_AINTC_IRQ,
.gpio_unbanked = 8,
};
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 72a3aa7..d83436d 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -786,7 +786,6 @@ static struct resource dm644_gpio_resources[] = {
static struct davinci_gpio_platform_data dm644_gpio_platform_data = {
.ngpio = 71,
- .intc_irq_num = DAVINCI_N_AINTC_IRQ,
};
int __init dm644x_gpio_register(void)
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index d1b646c..5d289f4 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -763,7 +763,6 @@ static struct resource dm646x_gpio_resources[] = {
static struct davinci_gpio_platform_data dm646x_gpio_platform_data = {
.ngpio = 43,
- .intc_irq_num = DAVINCI_N_AINTC_IRQ,
};
int __init dm646x_gpio_register(void)
diff --git a/include/linux/platform_data/gpio-davinci.h
b/include/linux/platform_data/gpio-davinci.h
index 0c3551b..fbe2f75 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -28,7 +28,6 @@ enum davinci_gpio_type {
struct davinci_gpio_platform_data {
u32 ngpio;
u32 gpio_unbanked;
- u32 intc_irq_num;
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 5/7] gpio: davinci: add OF support
From: KV Sujith
This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.
Signed-off-by: KV Sujith
Signed-off-by: Philip Avinash
[prabhakar.cse...@gmail.com: simplified the OF code, removed
unnecessary DT property and also simplified
the commit message]
Signed-off-by: Lad, Prabhakar
---
.../devicetree/bindings/gpio/gpio-davinci.txt | 39 ++
drivers/gpio/gpio-davinci.c| 54 ++--
2 files changed, 90 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 000..d677bbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,39 @@
+Davinci GPIO controller bindings
+
+Required Properties:
+- compatible: should be "ti,dm6441-gpio"
+
+- reg: Physical base address of the controller and the size of memory mapped
+ registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupt-parent: phandle of the parent interrupt controller.
+
+- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are
+ supported at a time.
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
interrupt
+line to processor.
+
+The GPIO controller also acts as an interrupt controller. It uses the default
+two cells specifier as described in Documentation/devicetree/bindings/
+interrupt-controller/interrupts.txt.
+
+Example:
+
+gpio: gpio@1e26000 {
+ compatible = "ti,dm6441-gpio";
+ gpio-controller;
+ reg = <0x226000 0x1000>;
+ interrupt-parent = <>;
+ interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
+ 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
+ 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
+ 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
+ 50 IRQ_TYPE_EDGE_BOTH>;
+ ti,ngpio = <144>;
+ ti,davinci-gpio-unbanked = <0>;
+};
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index b149239..ed04835 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -17,6 +17,9 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
@@ -134,6 +137,40 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset,
int value)
writel((1 << offset), value ? >set_data : >clr_data);
}
+static struct davinci_gpio_platform_data *
+davinci_gpio_get_pdata(struct platform_device *pdev)
+{
+ struct device_node *dn = pdev->dev.of_node;
+ struct davinci_gpio_platform_data *pdata;
+ int ret;
+ u32 val;
+
+ if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+ return pdev->dev.platform_data;
+
+ pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ ret = of_property_read_u32(dn, "ti,ngpio", );
+ if (ret)
+ goto of_err;
+
+ pdata->ngpio = val;
+
+ ret = of_property_read_u32(dn, "ti,davinci-gpio-unbanked", );
+ if (ret)
+ goto of_err;
+
+ pdata->gpio_unbanked = val;
+
+ return pdata;
+
+of_err:
+ dev_err(>dev, "Populating pdata from DT failed: err %d\n", ret);
+ return NULL;
+}
+
static int davinci_gpio_probe(struct platform_device *pdev)
{
int i, base;
@@ -144,12 +181,14 @@ static int davinci_gpio_probe(struct platform_device
*pdev)
struct device *dev = >dev;
struct resource *res;
- pdata = dev->platform_data;
+ pdata = davinci_gpio_get_pdata(pdev);
if (!pdata) {
dev_err(dev, "No platform data found\n");
return -EINVAL;
}
+ dev->platform_data = pdata;
+
/*
* The gpio banks conceptually expose a segmented bitmap,
* and "ngpio" is one more than the largest zero-based
@@ -496,11 +535,20 @@ done:
return 0;
}
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id davinci_gpio_ids[] = {
+ { .compatible = "ti,dm6441-gpio", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
+#endif
+
static struct platform_driver davinci_gpio_driver = {
.probe = davinci_gpio_probe,
.driver = {
- .name = "davinci_gpio",
- .owner = THIS_MODULE,
+ .name = "davinci_gpio",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(davinci_gpio_ids),
},
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe
[PATCH v5 2/7] gpio: davinci: use readl/writel instead of __raw_*
From: "Lad, Prabhakar"
This patch replaces the __raw_readl/writel with
readl and writel, Although the code runs on ARMv5
based SOCs, changing this will help copying the code
for other uses.
Signed-off-by: Lad, Prabhakar
---
drivers/gpio/gpio-davinci.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7230c43..ca3d7fd 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -82,14 +82,14 @@ static inline int __davinci_direction(struct gpio_chip
*chip,
u32 mask = 1 << offset;
spin_lock_irqsave(>lock, flags);
- temp = __raw_readl(>dir);
+ temp = readl(>dir);
if (out) {
temp &= ~mask;
- __raw_writel(mask, value ? >set_data : >clr_data);
+ writel(mask, value ? >set_data : >clr_data);
} else {
temp |= mask;
}
- __raw_writel(temp, >dir);
+ writel(temp, >dir);
spin_unlock_irqrestore(>lock, flags);
return 0;
@@ -118,7 +118,7 @@ static int davinci_gpio_get(struct gpio_chip *chip,
unsigned offset)
struct davinci_gpio_controller *d = chip2controller(chip);
struct davinci_gpio_regs __iomem *g = d->regs;
- return (1 << offset) & __raw_readl(>in_data);
+ return (1 << offset) & readl(>in_data);
}
/*
@@ -130,7 +130,7 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset,
int value)
struct davinci_gpio_controller *d = chip2controller(chip);
struct davinci_gpio_regs __iomem *g = d->regs;
- __raw_writel((1 << offset), value ? >set_data : >clr_data);
+ writel((1 << offset), value ? >set_data : >clr_data);
}
static int davinci_gpio_probe(struct platform_device *pdev)
@@ -227,8 +227,8 @@ static void gpio_irq_disable(struct irq_data *d)
struct davinci_gpio_regs __iomem *g = irq2regs(d->irq);
u32 mask = (u32) irq_data_get_irq_handler_data(d);
- __raw_writel(mask, >clr_falling);
- __raw_writel(mask, >clr_rising);
+ writel(mask, >clr_falling);
+ writel(mask, >clr_rising);
}
static void gpio_irq_enable(struct irq_data *d)
@@ -242,9 +242,9 @@ static void gpio_irq_enable(struct irq_data *d)
status = IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING;
if (status & IRQ_TYPE_EDGE_FALLING)
- __raw_writel(mask, >set_falling);
+ writel(mask, >set_falling);
if (status & IRQ_TYPE_EDGE_RISING)
- __raw_writel(mask, >set_rising);
+ writel(mask, >set_rising);
}
static int gpio_irq_type(struct irq_data *d, unsigned trigger)
@@ -286,10 +286,10 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
int res;
/* ack any irqs */
- status = __raw_readl(>intstat) & mask;
+ status = readl(>intstat) & mask;
if (!status)
break;
- __raw_writel(status, >intstat);
+ writel(status, >intstat);
/* now demux them to the right lowlevel handler */
n = d->irq_base;
@@ -346,9 +346,9 @@ static int gpio_irq_type_unbanked(struct irq_data *data,
unsigned trigger)
if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
return -EINVAL;
- __raw_writel(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
+ writel(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
? >set_falling : >clr_falling);
- __raw_writel(mask, (trigger & IRQ_TYPE_EDGE_RISING)
+ writel(mask, (trigger & IRQ_TYPE_EDGE_RISING)
? >set_rising : >clr_rising);
return 0;
@@ -430,8 +430,8 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* default trigger: both edges */
g = gpio2regs(0);
- __raw_writel(~0, >set_falling);
- __raw_writel(~0, >set_rising);
+ writel(~0, >set_falling);
+ writel(~0, >set_rising);
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio < pdata->gpio_unbanked; gpio++, irq++) {
@@ -454,8 +454,8 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
- __raw_writel(~0, >clr_falling);
- __raw_writel(~0, >clr_rising);
+ writel(~0, >clr_falling);
+ writel(~0, >clr_rising);
/* set up all irqs in this bank */
irq_set_chained_handler(bank_irq, gpio_irq_handler);
@@ -483,7 +483,7 @@ done:
* BINTEN -- per-bank interrupt enable. genirq would also let these
* bits be set/cleared dynamically.
*/
- __raw_writel(binten, gpio_base + BINTEN);
+
[PATCH v5 3/7] gpio: davinci: use irqdomain
From: "Lad, Prabhakar"
This patch converts the davinci gpio driver to use irqdomain
support.
Signed-off-by: Lad, Prabhakar
---
drivers/gpio/gpio-davinci.c| 74 +++-
include/linux/platform_data/gpio-davinci.h |2 +-
2 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index ca3d7fd..b149239 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -282,8 +283,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
desc->irq_data.chip->irq_ack(>irq_data);
while (1) {
u32 status;
- int n;
- int res;
+ int bit;
/* ack any irqs */
status = readl(>intstat) & mask;
@@ -292,17 +292,11 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
writel(status, >intstat);
/* now demux them to the right lowlevel handler */
- n = d->irq_base;
- if (irq & 1) {
- n += 16;
- status >>= 16;
- }
-
while (status) {
- res = ffs(status);
- n += res;
- generic_handle_irq(n - 1);
- status >>= res;
+ bit = __ffs(status);
+ status &= ~(1 << bit);
+ generic_handle_irq(irq_find_mapping(d->irq_domain,
+ bit));
}
}
desc->irq_data.chip->irq_unmask(>irq_data);
@@ -313,10 +307,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip,
unsigned offset)
{
struct davinci_gpio_controller *d = chip2controller(chip);
- if (d->irq_base >= 0)
- return d->irq_base + offset;
- else
- return -ENODEV;
+ return irq_create_mapping(d->irq_domain, offset);
}
static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -354,6 +345,28 @@ static int gpio_irq_type_unbanked(struct irq_data *data,
unsigned trigger)
return 0;
}
+static int davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ struct davinci_gpio_controller *chip = d->host_data;
+ unsigned gpio = chip->chip.base + hw;
+ struct davinci_gpio_regs __iomem *g = gpio2regs(gpio);
+
+ irq_set_chip_and_handler_name(irq, _irqchip, handle_simple_irq,
+ "davinci_gpio");
+ irq_set_irq_type(irq, IRQ_TYPE_NONE);
+ irq_set_chip_data(irq, (__force void *)g);
+ irq_set_handler_data(irq, (void *)__gpio_mask(gpio));
+ set_irq_flags(irq, IRQF_VALID);
+
+ return 0;
+}
+
+static const struct irq_domain_ops davinci_gpio_irq_ops = {
+ .map = davinci_gpio_irq_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -402,9 +415,16 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
*/
for (gpio = 0, bank = 0; gpio < ngpio; bank++, gpio += 32) {
chips[bank].chip.to_irq = gpio_to_irq_banked;
- chips[bank].irq_base = pdata->gpio_unbanked
- ? -EINVAL
- : (pdata->intc_irq_num + gpio);
+ if (!pdata->gpio_unbanked) {
+ chips[bank].irq_domain =
+ irq_domain_add_linear(NULL,
+ chips[bank].chip.ngpio,
+ _gpio_irq_ops,
+ [bank]);
+
+ if (!chips[bank].irq_domain)
+ return -ENOMEM;
+ }
}
/*
@@ -447,11 +467,7 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
* Or, AINTC can handle IRQs for banks of 16 GPIO IRQs, which we
* then chain through our own handler.
*/
- for (gpio = 0, irq = gpio_to_irq(0), bank = 0;
- gpio < ngpio;
- bank++, bank_irq++) {
- unsignedi;
-
+ for (gpio = 0, bank = 0; gpio < ngpio; bank++, bank_irq++, gpio += 16) {
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
writel(~0, >clr_falling);
@@ -467,14 +483,6 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
*/
irq_set_handler_data(bank_irq, [gpio / 32]);
-
[PATCH v5 6/7] ARM: davinci: da850: add GPIO DT node
From: KV Sujith
Add DT node for Davinci GPIO driver.
Signed-off-by: KV Sujith
Signed-off-by: Philip Avinash
Signed-off-by: Lad, Prabhakar
---
arch/arm/boot/dts/da850.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 8d17346..b695548 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -8,6 +8,7 @@
* option) any later version.
*/
#include "skeleton.dtsi"
+#include
/ {
arm {
@@ -256,6 +257,19 @@
36
>;
};
+ gpio: gpio@1e26000 {
+ compatible = "ti,dm6441-gpio";
+ gpio-controller;
+ reg = <0x226000 0x1000>;
+ interrupts = <42 IRQ_TYPE_EDGE_BOTH
+ 43 IRQ_TYPE_EDGE_BOTH 44 IRQ_TYPE_EDGE_BOTH
+ 45 IRQ_TYPE_EDGE_BOTH 46 IRQ_TYPE_EDGE_BOTH
+ 47 IRQ_TYPE_EDGE_BOTH 48 IRQ_TYPE_EDGE_BOTH
+ 49 IRQ_TYPE_EDGE_BOTH 50 IRQ_TYPE_EDGE_BOTH>;
+ ti,ngpio = <144>;
+ ti,davinci-gpio-unbanked = <0>;
+ status = "disabled";
+ };
};
nand_cs3@6200 {
compatible = "ti,davinci-nand";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 7/7] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
From: KV Sujith
Add GPIO DT node and pinmux entries for DA850 EVM. GPIO is
configurable differently on different boards. So add GPIO
pinmuxing in dts file.
Signed-off-by: KV Sujith
Signed-off-by: Philip Avinash
Signed-off-by: Lad, Prabhakar
---
arch/arm/boot/dts/da850-evm.dts | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 588ce58..f82c129 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -17,6 +17,21 @@
soc {
pmx_core: pinmux@1c14120 {
status = "okay";
+
+ gpio_pins: pinmux_gpio_pins {
+ pinctrl-single,bits = <
+ /* GPIO2_4 GPIO2_6 */
+ 0x18 0x8080 0xf0f0
+ /* GPIO2_8 GPIO2_15 */
+ 0x14 0x8008 0xf00f
+ /* GPIO3_12 GPIO3_13 */
+ 0x1C 0x8800 0xff00
+ /* GPIO4_0 GPIO4_1 */
+ 0x28 0x8800 0xff00
+ /* GPIO6_9 GPIO6_10 GPIO6_13 */
+ 0x34 0x08800800 0x0ff00f00
+ >;
+ };
};
serial0: serial@1c42000 {
status = "okay";
@@ -101,6 +116,11 @@
pinctrl-names = "default";
pinctrl-0 = <_pins>;
};
+ gpio: gpio@1e26000 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ };
};
nand_cs3@6200 {
status = "okay";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
Hi Taras, On Fri, Nov 22, 2013 at 3:38 PM, Taras Kondratiuk wrote: > On 21 November 2013 20:15, Prabhakar Lad wrote: >> From: "Lad, Prabhakar" >> >> This patch replaces the __raw_readl/writel with >> readl and writel, Altough the code runs on ARMv5 >> based SOCs, changing this will help copying the code >> for other uses. > > This replacement has a functional impact: it adds memory barriers. > Please note this in the description. > Also please add a bit of explanation on why do you need to add barriers. > Agreed this adds memory barriers, I'll add a note about it. Thanks, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
Hi Sekhar, Thanks for the review. On Mon, Nov 25, 2013 at 4:30 PM, Sekhar Nori wrote: > On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: >> From: KV Sujith >> >> This patch adds OF parser support for davinci gpio >> driver and also appropriate documentation in gpio-davinci.txt >> located at Documentation/devicetree/bindings/gpio/. >> >> Acked-by: Linus Walleij >> Acked-by: Rob Herring >> Signed-off-by: KV Sujith >> Signed-off-by: Philip Avinash >> [prabhakar.cse...@gmail.com: simplified the OF code, removed >> unnecessary DT property and also simplified >> the commit message] >> Signed-off-by: Lad, Prabhakar >> --- >> .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ >> drivers/gpio/gpio-davinci.c| 57 >> ++-- >> 2 files changed, 95 insertions(+), 3 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt >> >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >> new file mode 100644 >> index 000..a2e839d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >> @@ -0,0 +1,41 @@ >> +Davinci GPIO controller bindings >> + >> +Required Properties: >> +- compatible: should be "ti,dm6441-gpio" >> + >> +- reg: Physical base address of the controller and the size of memory mapped >> + registers. >> + >> +- gpio-controller : Marks the device node as a gpio controller. >> + >> +- interrupt-parent: phandle of the parent interrupt controller. >> + >> +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs >> are >> + supported at a time. > > If this is true.. > >> + >> +- ti,ngpio: The number of GPIO pins supported. >> + >> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual >> interrupt >> + line to processor. > > .. then why do you need to maintain this separately? Number of elements > in interrupts property should give you this answer, no? > > There can certainly be devices (past and future) which use a mixture of > banked and unbanked IRQs. So a binding which does not take care of this > is likely to change in future and that is a problem since it brings in > backward compatibility of the binding into picture. > > The right thing would be to define the DT node per-bank similar to what > is done on OMAP rather than for all banks together. That way there can > be a separate property which determines whether that bank supports > direct-mapped or banked IRQs (or that could be inferred if the number of > tuples in the interrupts property is more than one). > Can you point me to the OMAP implementation. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
Hi Sekhar,
On Mon, Nov 25, 2013 at 4:04 PM, Sekhar Nori wrote:
> Prabhakar,
>
> On Monday 25 November 2013 09:42 AM, Prabhakar Lad wrote:
>> Hi Taras,
>>
>> On Fri, Nov 22, 2013 at 3:38 PM, Taras Kondratiuk
>> wrote:
>>> On 21 November 2013 20:15, Prabhakar Lad wrote:
>>>> From: "Lad, Prabhakar"
>>>>
>>>> This patch replaces the __raw_readl/writel with
>>>> readl and writel, Altough the code runs on ARMv5
>>>> based SOCs, changing this will help copying the code
>>>> for other uses.
>>>
>>> This replacement has a functional impact: it adds memory barriers.
>>> Please note this in the description.
>>> Also please add a bit of explanation on why do you need to add barriers.
>>>
>> Agreed this adds memory barriers, I'll add a note about it.
>
> Well the barriers certainly make it easier to debug by having both
> device and memory accesses happen in program order. That said, if there
> is no pressing reason to add barriers, you can use
> {readl|writel}_relaxed() instead. That will make the code protable
> across endianess.
>
OK will use {readl|writel}_relaxed() instead.
Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] gpio: davinci: get rid of DAVINCI_N_GPIO
On Thu, Nov 21, 2013 at 9:04 PM, Grygorii Strashko wrote: > Since Davinci GPIO driver is moved to support gpiolib it has to use > ARCH_NR_GPIOS (can be configured using CONFIG_ARCH_NR_GPIO Kconfig > option) configuration instead of any mach/platform specific options. > > Hence, replace private DAVINCI_N_GPIO with common ARCH_NR_GPIOS. This is > safe because default value for ARCH_NR_GPIOS=256 and maximum number of > supported GPIOs for Davinci is DAVINCI_N_GPIO=144. > > More over, this is one of steps to re-use Davinci GPIO driver by other > mach/platform. > > Signed-off-by: Grygorii Strashko Acked-by: Lad, Prabhakar Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] gpio: introduce GPIO_DAVINCI kconfig option
On Thu, Nov 21, 2013 at 9:04 PM, Grygorii Strashko wrote: > The compatible to Davinci GPIO HW block is used by other TI SoCs, like > Keystone, where GPIO support is declared as optional. > > Hence, introduce GPIO_DAVINCI Kconfig option which will allow to enable > Davinci GPIO driver for Keystone SoCs when needed. At same time, kept > Davinci GPIO driver enabled for Davinci SoCs by default. > > Signed-off-by: Grygorii Strashko Acked-by: Lad, Prabhakar Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement
Hi Linus, On Fri, Nov 29, 2013 at 1:17 PM, Linus Walleij wrote: > On Thu, Nov 21, 2013 at 7:15 PM, Prabhakar Lad > wrote: > >> From: "Lad, Prabhakar" >> >> This patch series does the following >> 1> Ports the driver to use irqdomain. >> 2> Adds dt binding support for gpio-davinci. >> 3> Adds DA850 dt support goio. >> >> Changes for v6: >> 1: GPIO driver now migrated to irq domain legacy. >> 2: Fixed review comments pointed by Grygorii. >> 3: Included Ack's. > > This series is looking nice, I assume that Sekhar will take this through > the DaVinci tree once he's happy with it. I think I've ACKed all relevant > patches, else tell me. > While you are at it, you missed to ACK v6 2/6 patch [1] [1] https://lkml.org/lkml/2013/11/21/425 Regards, --Prabhakar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 5/7] gpio: davinci: add OF support
Hi Grygorii,
Thanks for the review.
On Mon, Nov 11, 2013 at 8:48 PM, Grygorii Strashko
wrote:
> On 11/08/2013 12:11 PM, Prabhakar Lad wrote:
[Snip]
>> @@ -134,6 +137,40 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned
>> offset, int value)
>> writel((1 << offset), value ? >set_data : >clr_data);
>> }
>>
>> +static struct davinci_gpio_platform_data *
>> +davinci_gpio_get_pdata(struct platform_device *pdev)
>
> Minor: usually such functions have "_of" in their names:
> davinci_gpio_of_get_pdata()
>
Ahh but actual this function is intended to get pdata in both the
cases DT and NON-DT, so kept it generic :)
>> +{
>> + struct device_node *dn = pdev->dev.of_node;
>> + struct davinci_gpio_platform_data *pdata;
>> + int ret;
>> + u32 val;
>> +
>> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>> + return pdev->dev.platform_data;
>> +
>> + pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
>> + if (!pdata)
>> + return NULL;
>> +
>> + ret = of_property_read_u32(dn, "ti,ngpio", );
>> + if (ret)
>> + goto of_err;
>> +
>> + pdata->ngpio = val;
>> +
>> + ret = of_property_read_u32(dn, "ti,davinci-gpio-unbanked", );
>> + if (ret)
>> + goto of_err;
>> +
>> + pdata->gpio_unbanked = val;
>> +
>> + return pdata;
>> +
>> +of_err:
>> + dev_err(>dev, "Populating pdata from DT failed: err %d\n", ret);
>> + return NULL;
>> +}
>> +
>> static int davinci_gpio_probe(struct platform_device *pdev)
>> {
>> int i, base;
>> @@ -144,12 +181,14 @@ static int davinci_gpio_probe(struct platform_device
>> *pdev)
>> struct device *dev = >dev;
>> struct resource *res;
>>
>> - pdata = dev->platform_data;
>> + pdata = davinci_gpio_get_pdata(pdev);
>> if (!pdata) {
>> dev_err(dev, "No platform data found\n");
>> return -EINVAL;
>> }
>>
>> + dev->platform_data = pdata;
>> +
>
> Pls, add following code to GPIO chip initialization:
>
> @@ -233,6 +245,9 @@ static int davinci_gpio_probe(struct platform_device
> *pdev)
> chips[i].chip.ngpio = ngpio - base;
> if (chips[i].chip.ngpio > 32)
> chips[i].chip.ngpio = 32;
> +#ifdef CONFIG_OF_GPIO
> + chips[i].chip.of_node = dev->of_node;
> +#endif
>
>
OK
Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/7] gpio: davinci: remove unnecessary printk
Hi Grygorii, Thanks for the review. On Fri, Nov 8, 2013 at 9:09 PM, Grygorii Strashko wrote: > On 11/08/2013 12:11 PM, Prabhakar Lad wrote: >> >> From: "Lad, Prabhakar" >> >> the devm_*() helper prints error messages in case of >> errors no need to do the same in the driver. > > > Pls, drop it - devm_clk_get() doesn't always print error messages > OK will drop in the next version. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/3] gpio: davinci: add OF support
Hi Grygorii, On Mon, Oct 14, 2013 at 5:55 PM, Grygorii Strashko wrote: > Hi Prabhakar Lad, > > On 10/11/2013 07:18 PM, Prabhakar Lad wrote: >> Hi Linus, >> >> On 10/11/13, Linus Walleij wrote: >>> On Fri, Oct 11, 2013 at 4:59 PM, Prabhakar Lad >>> wrote: >>>> On 10/11/13, Linus Walleij wrote: >>>>> On Fri, Oct 4, 2013 at 6:03 PM, Prabhakar Lad >>>>> wrote: >>> >>>>>> +- ti,davinci-gpio-irq-base: Base from where GPIO interrupt numbering >>>>>> starts. >>>>> >>>>> What is this? >>>>> >>>>> If I have ever ACKed this I have been drunk. I take it back. >>>>> >>>> here is the ACK https://patchwork.kernel.org/patch/2721181/ >>> >>> And as suspected that version of the patch did not contain >>> this strange node property. >>> >> The property did exist in the patch 'intc_irq_num', I just renamed >> it and gave a proper description to it. >> >>> Don't keep my ACK on patches if you change basic stuff like >>> that, they need to be re-acked, this runs the risk of abusing >>> my trust amongst other subsystem maintainers who might >>> go and merge this because "aha the GPIO maintainer >>> thinks that this is OK". >>> >> Agreed, I carry forwarded the ACK since it had minor changes. >> >>>>> This "base" is a Linux-specific thing and has no place in the >>>>> device tree, and shall not be there. You have to find some way to >>>>> avoid this, what do you think some other OS should do with >>>>> this value... >>>>> >>>>> All IRQs in Linux are assumed to be dynamically assigned numbers >>>>> nowadays, with a property like this you can never switch on >>>>> SPARSE_IRQ for the DaVinci. >>>>> >>>> Can you point to any alternative solution if you have any ? >>> >>> First convert this GPIO driver to use an irqdomain to map >>> HW IRQs to Linux IRQs, and grab a few IRQ descriptors >>> dynamically off the irq descriptor heap. >>> Example: commit >>> a6c45b99a658521291cfb66ecf035cc58b38f206 >>> "pinctrl/coh901: use irqdomain, allocate irqdescs" >>> >>> Then on a longer term convert DaVinci to use dynamically >>> allocated IRQs for all interrupt controllers, and move it over >>> to SPARSE_IRQ so you know this works. >>> >> Thanks for the pointers. >> > > Could it be possible to use "interrupts" and "interrupt-names" to identify > assigned banked & unbanked IRQs? > I did give thought for it but found out its taken as a single interrupt, and comparing the names in the driver would be tedious. so as of now I have kept as is for this. > For example DM646x (http://www.ti.com/lit/ug/sprueq8a/sprueq8a.pdf): > > interrupts = <48 IRQ_TYPE_EDGE_BOTH>, > <49 IRQ_TYPE_EDGE_BOTH>, > <50 IRQ_TYPE_EDGE_BOTH>, > <51 IRQ_TYPE_EDGE_BOTH>, > <52 IRQ_TYPE_EDGE_BOTH>, > <53 IRQ_TYPE_EDGE_BOTH>, > <54 IRQ_TYPE_EDGE_BOTH>, > <55 IRQ_TYPE_EDGE_BOTH>, > <56 IRQ_TYPE_EDGE_BOTH>, > <57 IRQ_TYPE_EDGE_BOTH>, > <58 IRQ_TYPE_EDGE_BOTH>; > interrupt-names = "gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5", > "gpio6", "gpio7", "bank0", "bank1", "bank2"; > > For example OMAP-L138 (http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf): > > interrupts = <42 IRQ_TYPE_EDGE_BOTH>, > <43 IRQ_TYPE_EDGE_BOTH>, > <44 IRQ_TYPE_EDGE_BOTH>, > <45 IRQ_TYPE_EDGE_BOTH>, > <46 IRQ_TYPE_EDGE_BOTH>, > <47 IRQ_TYPE_EDGE_BOTH>, > <48 IRQ_TYPE_EDGE_BOTH>, > <49 IRQ_TYPE_EDGE_BOTH>, > <50 IRQ_TYPE_EDGE_BOTH>; > > interrupt-names = "bank0", "bank1", "bank2", "bank3", "bank4", "bank5", > "bank6", "bank7", "bank8"; > > For example Keystone 66AK2E05/02 > (http://www.ti.com/lit/ds/sprs865a/sprs865a.pdf and > http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf): > > interrupts = , > , > [..] > , > ; > > > interrupt-names = "gpio0", "gpio1", [...], "gpio30", "gpio31"; > > So then, following properties would not be needed at all, because all inf. > can be > taken from interrupt's properties: > +- ti,davinci-gpio-irq-base: Base from where GPIO interrupt numbering starts. Yeah dropped this. > +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual > interrupt line to processor. > Kept as is > It should work good if Davinci-gpio driver will be converted to use > linear IRQ domains for banked irqs. > Yeah migrated it to use IRQ domains. I have posted the updated patch series at [1], please have a look at it. [1] http://linux.omap.com/pipermail/davinci-linux-open-source/2013-November/028220.html Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7] gpio: davinci: use {readl|writel}_relaxed() instead of __raw_*
From: "Lad, Prabhakar"
This patch replaces the __raw_readl/writel with
{readl|writel}_relaxed(), Altough the code runs on ARMv5
based SOCs, changing this will help copying the code
for other uses.
Signed-off-by: Lad, Prabhakar
---
This patch is part of series [1] rest of the patches
are Acked/reviewed so posting this patch independently
and marking it as v7.
[1] http://www.spinics.net/lists/devicetree/msg13037.html
drivers/gpio/gpio-davinci.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 84be701..805552c 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -82,14 +82,14 @@ static inline int __davinci_direction(struct gpio_chip
*chip,
u32 mask = 1 << offset;
spin_lock_irqsave(>lock, flags);
- temp = __raw_readl(>dir);
+ temp = readl_relaxed(>dir);
if (out) {
temp &= ~mask;
- __raw_writel(mask, value ? >set_data : >clr_data);
+ writel_relaxed(mask, value ? >set_data : >clr_data);
} else {
temp |= mask;
}
- __raw_writel(temp, >dir);
+ writel_relaxed(temp, >dir);
spin_unlock_irqrestore(>lock, flags);
return 0;
@@ -118,7 +118,7 @@ static int davinci_gpio_get(struct gpio_chip *chip,
unsigned offset)
struct davinci_gpio_controller *d = chip2controller(chip);
struct davinci_gpio_regs __iomem *g = d->regs;
- return (1 << offset) & __raw_readl(>in_data);
+ return (1 << offset) & readl_relaxed(>in_data);
}
/*
@@ -130,7 +130,7 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset,
int value)
struct davinci_gpio_controller *d = chip2controller(chip);
struct davinci_gpio_regs __iomem *g = d->regs;
- __raw_writel((1 << offset), value ? >set_data : >clr_data);
+ writel_relaxed((1 << offset), value ? >set_data : >clr_data);
}
static int davinci_gpio_probe(struct platform_device *pdev)
@@ -227,8 +227,8 @@ static void gpio_irq_disable(struct irq_data *d)
struct davinci_gpio_regs __iomem *g = irq2regs(d->irq);
u32 mask = (u32) irq_data_get_irq_handler_data(d);
- __raw_writel(mask, >clr_falling);
- __raw_writel(mask, >clr_rising);
+ writel_relaxed(mask, >clr_falling);
+ writel_relaxed(mask, >clr_rising);
}
static void gpio_irq_enable(struct irq_data *d)
@@ -242,9 +242,9 @@ static void gpio_irq_enable(struct irq_data *d)
status = IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING;
if (status & IRQ_TYPE_EDGE_FALLING)
- __raw_writel(mask, >set_falling);
+ writel_relaxed(mask, >set_falling);
if (status & IRQ_TYPE_EDGE_RISING)
- __raw_writel(mask, >set_rising);
+ writel_relaxed(mask, >set_rising);
}
static int gpio_irq_type(struct irq_data *d, unsigned trigger)
@@ -286,10 +286,10 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
int res;
/* ack any irqs */
- status = __raw_readl(>intstat) & mask;
+ status = readl_relaxed(>intstat) & mask;
if (!status)
break;
- __raw_writel(status, >intstat);
+ writel_relaxed(status, >intstat);
/* now demux them to the right lowlevel handler */
n = d->irq_base;
@@ -346,9 +346,9 @@ static int gpio_irq_type_unbanked(struct irq_data *data,
unsigned trigger)
if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
return -EINVAL;
- __raw_writel(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
+ writel_relaxed(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
? >set_falling : >clr_falling);
- __raw_writel(mask, (trigger & IRQ_TYPE_EDGE_RISING)
+ writel_relaxed(mask, (trigger & IRQ_TYPE_EDGE_RISING)
? >set_rising : >clr_rising);
return 0;
@@ -432,8 +432,8 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* default trigger: both edges */
g = gpio2regs(0);
- __raw_writel(~0, >set_falling);
- __raw_writel(~0, >set_rising);
+ writel_relaxed(~0, >set_falling);
+ writel_relaxed(~0, >set_rising);
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio < pdata->gpio_unbanked; gpio++, irq++) {
@@ -456,8 +456,8 @@ static int davinci_gpio_irq_setup(struct platform_device
*pdev)
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
- __raw_writel(~0, >clr_falling);
- __raw_writel(~0, >clr_rising);
+ writel_relaxed(~0, >clr_falling);
+ writel_relaxed(~0, >clr_rising);
Re: [RFC v1 0/9] gpio: davinci: reuse for keystone arch
Hi Grygorii, Thanks for the patches. On Wed, Nov 27, 2013 at 1:10 AM, Grygorii Strashko wrote: > This series is intended to update Davinci GPIO driver and reuse > it for Keystone SoCs, because Keystone uses the similar GPIO IP like Davinci. > Keystone GPIO IP: supports: > - up to 32 GPIO lines; > - only unbanked irqs; > > See Documentation: > Keystone - http://www.ti.com/lit/ug/sprugv1/sprugv1.pdf > > [1] Depends on patch: > "[PATCH 1/2] gpio: davinci: Fix a check for unbanked gpio" > https://lkml.org/lkml/2013/11/8/22 > > [2] and depends on series from Prabhakar Lad: > "[PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement" > https://www.mail-archive.com/devicetree@vger.kernel.org/msg05970.html > > Based on: > git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git > branch: keystone/master > > This series has been marked as RFC because it's based on an unfinished > series of patches [2], but with hope that it will help to reach final > decisions > and satisfy all interested parties. > > Grygorii Strashko (9): > gpio: davinci: get rid of DAVINCI_N_GPIO > gpio: introduce GPIO_DAVINCI kconfig option > gpio: davinci: use chained_irq_enter/chained_irq_exit API > gpio: davinci: make IRQ initialization soc specific > gpio: davinci: reuse for Keystone SoC > arm: dts: keystone: add GPIO device entry > ARM: keystone_defconfig: enable gpio support > arm: dts: keystone-evm: add LEDs supports > ARM: keystone_defconfig: enable LED support > For patches 1, 2 and 3 Acked-and-tested-by: Lad, Prabhakar Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7] gpio: davinci: use {readl|writel}_relaxed() instead of __raw_*
Hi Linus,
On Fri, Dec 13, 2013 at 2:01 AM, Linus Walleij wrote:
> On Wed, Dec 11, 2013 at 6:52 PM, Prabhakar Lad
> wrote:
>
>> From: "Lad, Prabhakar"
>>
>> This patch replaces the __raw_readl/writel with
>> {readl|writel}_relaxed(), Altough the code runs on ARMv5
>> based SOCs, changing this will help copying the code
>> for other uses.
>>
>> Signed-off-by: Lad, Prabhakar
>> ---
>> This patch is part of series [1] rest of the patches
>> are Acked/reviewed so posting this patch independently
>> and marking it as v7.
>>
>> [1] http://www.spinics.net/lists/devicetree/msg13037.html
>>
>> drivers/gpio/gpio-davinci.c | 36 ++--
>
> Acked-by: Linus Walleij
>
Thanks for the Ack.
> Should I take this into the GPIO tree, or should it go
> in through the DaVinci tree?
>
To avoid dependencies its better it goes via DaVinci tree.
Thanks,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v6 0/2] ARM: davinci: add support for dm365 vpbe display
From: Lad, Prabhakar This patch series enables VPBE display driver on DM365. This patch is work on on following pull request from Hans http://davinci-linux-open-source.1494791.n2.nabble.com/GIT-PULL-FOR-v3-10-tvp7002-davinci-blackfin-legacy-cleanups-td7582451.html Changes for v6: 1: Fixed review comments pointed by Sekhar. Changes for v5: 1: pass different platform names to handle different ip's. 2: Fixed review comments pointed by Sekhar. Changes for v4: 1: Replaced the obsolete preset API by timings API. Changes for v3: 1: Removed VPSS clock alias for master and slave which was sent for VPSS driver. since this patch was dependent on patch[1]. I will revist this patch once MC(captrure driver) goes into mainline. [1] http://www.spinics.net/lists/linux-media/msg50562.html Changes for v2: 1: Added VPSS clock so that capture and display can work independent. Lad, Prabhakar (2): ARM: davinci: dm365: add support for v4l2 video display ARM: davinci: dm365 EVM: add support for VPBE display arch/arm/mach-davinci/board-dm365-evm.c | 166 +- arch/arm/mach-davinci/davinci.h |2 +- arch/arm/mach-davinci/dm365.c | 203 +-- 3 files changed, 357 insertions(+), 14 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v6 1/2] ARM: davinci: dm365: add support for v4l2 video display
From: Lad, Prabhakar
Create platform devices for various video modules like venc,osd,
vpbe and v4l2 driver for dm365.
Signed-off-by: Lad, Prabhakar
---
arch/arm/mach-davinci/board-dm365-evm.c |4 +-
arch/arm/mach-davinci/davinci.h |2 +-
arch/arm/mach-davinci/dm365.c | 203 +--
3 files changed, 195 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
b/arch/arm/mach-davinci/board-dm365-evm.c
index c2d4958..cf77c46 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -564,8 +564,6 @@ static struct davinci_uart_config uart_config __initdata = {
static void __init dm365_evm_map_io(void)
{
- /* setup input configuration for VPFE input devices */
- dm365_set_vpfe_config(_cfg);
dm365_init();
}
@@ -597,6 +595,8 @@ static __init void dm365_evm_init(void)
davinci_setup_mmc(0, _mmc_config);
+ dm365_init_video(_cfg, NULL);
+
/* maybe setup mmc1/etc ... _after_ mmc0 */
evm_init_cpld();
diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
index 12d544b..1c2670f 100644
--- a/arch/arm/mach-davinci/davinci.h
+++ b/arch/arm/mach-davinci/davinci.h
@@ -84,7 +84,7 @@ void __init dm365_init_ks(struct davinci_ks_platform_data
*pdata);
void __init dm365_init_rtc(void);
void dm365_init_spi0(unsigned chipselect_mask,
const struct spi_board_info *info, unsigned len);
-void dm365_set_vpfe_config(struct vpfe_config *cfg);
+int __init dm365_init_video(struct vpfe_config *, struct vpbe_config *);
/* DM644x function declarations */
void __init dm644x_init(void);
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 6c39805..ec8b06e 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -40,10 +40,16 @@
#define DM365_REF_FREQ 2400/* 24 MHz on the DM365 EVM */
+#define DM3XX_VDAC_CONFIG 0x01c4002c
+
+#define DM365_RTC_BASE 0x01c69000
+
/* Base of key scan register bank */
#define DM365_KEYSCAN_BASE 0x01c69400
-#define DM365_RTC_BASE 0x01c69000
+#define DM365_OSD_BASE 0x01c71c00
+
+#define DM365_VENC_REG_BASE0x01c71e00
#define DAVINCI_DM365_VC_BASE 0x01d0c000
#define DAVINCI_DMA_VC_TX 2
@@ -56,6 +62,11 @@
#define DM365_EMAC_CNTRL_RAM_OFFSET0x1000
#define DM365_EMAC_CNTRL_RAM_SIZE 0x2000
+#define DM365_VPSS_CLK_CTRL_ADDR 0x44
+#define DM365_VPSS_VENCCLKEN_ENABLEBIT(3)
+#define DM365_VPSS_DACCLKEN_ENABLE BIT(4)
+#define DM365_VPSS_PLLC2SYSCLK5_ENABLE BIT(5)
+
static struct pll_data pll1_data = {
.num= 1,
.phys_base = DAVINCI_PLL1_BASE,
@@ -1226,6 +1237,186 @@ static struct platform_device dm365_isif_dev = {
},
};
+static struct resource dm365_osd_resources[] = {
+ {
+ .start = DM365_OSD_BASE,
+ .end = DM365_OSD_BASE + 0x100,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static u64 dm365_video_dma_mask = DMA_BIT_MASK(32);
+
+static struct platform_device dm365_osd_dev = {
+ .name = DM365_VPBE_OSD_SUBDEV_NAME,
+ .id = -1,
+ .num_resources = ARRAY_SIZE(dm365_osd_resources),
+ .resource = dm365_osd_resources,
+ .dev= {
+ .dma_mask = _video_dma_mask,
+ .coherent_dma_mask = DMA_BIT_MASK(32),
+ },
+};
+
+static struct resource dm365_venc_resources[] = {
+ {
+ .start = IRQ_VENCINT,
+ .end = IRQ_VENCINT,
+ .flags = IORESOURCE_IRQ,
+ },
+ /* venc registers io space */
+ {
+ .start = DM365_VENC_REG_BASE,
+ .end = DM365_VENC_REG_BASE + 0x180,
+ .flags = IORESOURCE_MEM,
+ },
+ /* vdaccfg registers io space */
+ {
+ .start = DM3XX_VDAC_CONFIG,
+ .end = DM3XX_VDAC_CONFIG + 4,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct resource dm365_v4l2_disp_resources[] = {
+ {
+ .start = IRQ_VENCINT,
+ .end = IRQ_VENCINT,
+ .flags = IORESOURCE_IRQ,
+ },
+ /* venc registers io space */
+ {
+ .start = DM365_VENC_REG_BASE,
+ .end = DM365_VENC_REG_BASE + 0x180,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static int dm365_vpbe_setup_pinmux(enum v4l2_mbus_pixelcode if_type,
+ int field)
+{
+ switch (if_type) {
+ case V4L2_MBUS_FMT_SGRBG8_1X8:
+ davinci_cfg_reg(DM365_VOUT_FIELD_G81);
+ davinci_cfg_reg(DM365_VOUT_COUTL_EN);
+ davinci_cfg_reg(DM365_VOUT_COUTH_EN);
+
[PATCH v6 2/2] ARM: davinci: dm365 EVM: add support for VPBE display
From: Lad, Prabhakar
add support for V4L2 video display to DM365 EVM.
Support for SD and ED modes is provided, along with Composite
and Component outputs.
Signed-off-by: Lad, Prabhakar
---
arch/arm/mach-davinci/board-dm365-evm.c | 164 ++-
1 files changed, 163 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
b/arch/arm/mach-davinci/board-dm365-evm.c
index cf77c46..0518ce5 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -39,6 +40,7 @@
#include
#include
+#include
#include
#include "davinci.h"
@@ -374,6 +376,166 @@ static struct vpfe_config vpfe_cfg = {
.ccdc = "ISIF",
};
+/* venc standards timings */
+static struct vpbe_enc_mode_info dm365evm_enc_std_timing[] = {
+ {
+ .name = "ntsc",
+ .timings_type = VPBE_ENC_STD,
+ .std_id = V4L2_STD_525_60,
+ .interlaced = 1,
+ .xres = 720,
+ .yres = 480,
+ .aspect = {11, 10},
+ .fps= {3, 1001},
+ .left_margin= 0x79,
+ .upper_margin = 0x10,
+ },
+ {
+ .name = "pal",
+ .timings_type = VPBE_ENC_STD,
+ .std_id = V4L2_STD_625_50,
+ .interlaced = 1,
+ .xres = 720,
+ .yres = 576,
+ .aspect = {54, 59},
+ .fps= {25, 1},
+ .left_margin= 0x7E,
+ .upper_margin = 0x16,
+ },
+};
+
+/* venc dv timings */
+static struct vpbe_enc_mode_info dm365evm_enc_preset_timing[] = {
+ {
+ .name = "480p59_94",
+ .timings_type = VPBE_ENC_DV_TIMINGS,
+ .dv_timings = V4L2_DV_BT_CEA_720X480P59_94,
+ .interlaced = 0,
+ .xres = 720,
+ .yres = 480,
+ .aspect = {1, 1},
+ .fps= {5994, 100},
+ .left_margin= 0x8F,
+ .upper_margin = 0x2D,
+ },
+ {
+ .name = "576p50",
+ .timings_type = VPBE_ENC_DV_TIMINGS,
+ .dv_timings = V4L2_DV_BT_CEA_720X576P50,
+ .interlaced = 0,
+ .xres = 720,
+ .yres = 576,
+ .aspect = {1, 1},
+ .fps= {50, 1},
+ .left_margin= 0x8C,
+ .upper_margin = 0x36,
+ },
+ {
+ .name = "720p60",
+ .timings_type = VPBE_ENC_DV_TIMINGS,
+ .dv_timings = V4L2_DV_BT_CEA_1280X720P60,
+ .interlaced = 0,
+ .xres = 1280,
+ .yres = 720,
+ .aspect = {1, 1},
+ .fps= {60, 1},
+ .left_margin= 0x117,
+ .right_margin = 70,
+ .upper_margin = 38,
+ .lower_margin = 3,
+ .hsync_len = 80,
+ .vsync_len = 5,
+ },
+ {
+ .name = "1080i60",
+ .timings_type = VPBE_ENC_DV_TIMINGS,
+ .dv_timings = V4L2_DV_BT_CEA_1920X1080I60,
+ .interlaced = 1,
+ .xres = 1920,
+ .yres = 1080,
+ .aspect = {1, 1},
+ .fps= {30, 1},
+ .left_margin= 0xc9,
+ .right_margin = 80,
+ .upper_margin = 30,
+ .lower_margin = 3,
+ .hsync_len = 88,
+ .vsync_len = 5,
+ },
+};
+
+#define VENC_STD_ALL (V4L2_STD_NTSC | V4L2_STD_PAL)
+
+/*
+ * The outputs available from VPBE + ecnoders. Keep the
+ * the order same as that of encoders. First those from venc followed by that
+ * from encoders. Index in the output refers to index on a particular
+ * encoder.Driver uses this index to pass it to encoder when it supports more
+ * than one output. Application uses index of the array to set an output.
+ */
+static struct vpbe_output dm365evm_vpbe_outputs[] = {
+ {
+ .output = {
+ .index = 0,
+ .name = "Composite",
+ .type = V4L2_OUTPUT_TYPE_ANALOG,
+ .std= VENC_STD_ALL,
+ .capabilities = V4L2_OUT_CAP_STD,
+ },
+ .subdev_name= DM365_VPBE_VENC_SUBDEV_NAME,
+ .default_mode = "ntsc",
+
[PATCH v5 0/2] ARM: davinci: dm355: add support for vpbe display
From: Lad, Prabhakar This patch series enables VPBE display driver on DM355. Changes for v5: 1: Rebased on 3.9, fix review comments pointed by Sekhar for DM365 series. Changes for v4: 1: pass different platform names to handle different ip's. Changes for v3: 1: Replaced obsolete preset API by timings API. Changes for v2: 1: Removed VPSS clock alias for master and slave which was sent for VPSS driver. since this patch was dependent on patch[1]. I will revist this patch once MC(captrure driver) goes into mainline. [1] http://www.spinics.net/lists/linux-media/msg50562.html Lad, Prabhakar (2): ARM: davinci: dm355: add support for v4l2 video display ARM: davinci: dm355 EVM: add support for VPBE display arch/arm/mach-davinci/board-dm355-evm.c | 71 +++- arch/arm/mach-davinci/davinci.h |2 +- arch/arm/mach-davinci/dm355.c | 202 +-- 3 files changed, 264 insertions(+), 11 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/2] ARM: davinci: dm355: add support for v4l2 video display
From: Lad, Prabhakar
Create platform devices for various video modules like venc,osd,
vpbe and v4l2 driver for dm355.
Signed-off-by: Lad, Prabhakar
---
arch/arm/mach-davinci/board-dm355-evm.c |4 +-
arch/arm/mach-davinci/davinci.h |2 +-
arch/arm/mach-davinci/dm355.c | 202 +--
3 files changed, 197 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm355-evm.c
b/arch/arm/mach-davinci/board-dm355-evm.c
index 147b8e1..37d12cc 100644
--- a/arch/arm/mach-davinci/board-dm355-evm.c
+++ b/arch/arm/mach-davinci/board-dm355-evm.c
@@ -253,8 +253,6 @@ static struct davinci_uart_config uart_config __initdata = {
static void __init dm355_evm_map_io(void)
{
- /* setup input configuration for VPFE input devices */
- dm355_set_vpfe_config(_cfg);
dm355_init();
}
@@ -344,6 +342,8 @@ static __init void dm355_evm_init(void)
davinci_setup_mmc(0, _mmc_config);
davinci_setup_mmc(1, _mmc_config);
+ dm355_init_video(_cfg, NULL);
+
dm355_init_spi0(BIT(0), dm355_evm_spi_info,
ARRAY_SIZE(dm355_evm_spi_info));
diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
index 1c2670f..acfe0bb 100644
--- a/arch/arm/mach-davinci/davinci.h
+++ b/arch/arm/mach-davinci/davinci.h
@@ -74,7 +74,7 @@ void __init dm355_init(void);
void dm355_init_spi0(unsigned chipselect_mask,
const struct spi_board_info *info, unsigned len);
void __init dm355_init_asp1(u32 evt_enable, struct snd_platform_data *pdata);
-void dm355_set_vpfe_config(struct vpfe_config *cfg);
+int __init dm355_init_video(struct vpfe_config *, struct vpbe_config *);
/* DM365 function declarations */
void __init dm365_init(void);
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index b49c3b7..761fcba 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -36,6 +36,17 @@
#define DM355_UART2_BASE (IO_PHYS + 0x206000)
+#define DM3XX_VDAC_CONFIG_BASE 0x01c4002c
+
+#define DM355_OSD_BASE 0x01c70200
+
+#define DM355_VENC_BASE0x01c70400
+
+#define DM355_VPSS_CLK_CTRL_ADDR 0x44
+#define DM355_VPSS_MUXSEL_EXTCLK_ENABLEBIT(1)
+#define DM355_VPSS_VENCCLKEN_ENABLEBIT(3)
+#define DM355_VPSS_DACCLKEN_ENABLE BIT(4)
+
/*
* Device specific clocks
*/
@@ -744,11 +755,165 @@ static struct platform_device vpfe_capture_dev = {
},
};
-void dm355_set_vpfe_config(struct vpfe_config *cfg)
+static struct resource dm355_osd_resources[] = {
+ {
+ .start = DM355_OSD_BASE,
+ .end= DM355_OSD_BASE + 0x180,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct platform_device dm355_osd_dev = {
+ .name = DM355_VPBE_OSD_SUBDEV_NAME,
+ .id = -1,
+ .num_resources = ARRAY_SIZE(dm355_osd_resources),
+ .resource = dm355_osd_resources,
+ .dev= {
+ .dma_mask = _capture_dma_mask,
+ .coherent_dma_mask = DMA_BIT_MASK(32),
+ },
+};
+
+static struct resource dm355_venc_resources[] = {
+ {
+ .start = IRQ_VENCINT,
+ .end= IRQ_VENCINT,
+ .flags = IORESOURCE_IRQ,
+ },
+ /* venc registers io space */
+ {
+ .start = DM355_VENC_BASE,
+ .end= DM355_VENC_BASE + 0x180,
+ .flags = IORESOURCE_MEM,
+ },
+ /* VDAC config register io space */
+ {
+ .start = DM3XX_VDAC_CONFIG_BASE,
+ .end= DM3XX_VDAC_CONFIG_BASE + 4,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct resource dm355_v4l2_disp_resources[] = {
+ {
+ .start = IRQ_VENCINT,
+ .end= IRQ_VENCINT,
+ .flags = IORESOURCE_IRQ,
+ },
+ /* venc registers io space */
+ {
+ .start = DM355_VENC_BASE,
+ .end= DM355_VENC_BASE + 0x180,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static int dm355_vpbe_setup_pinmux(enum v4l2_mbus_pixelcode if_type,
+ int field)
{
- vpfe_capture_dev.dev.platform_data = cfg;
+ switch (if_type) {
+ case V4L2_MBUS_FMT_SGRBG8_1X8:
+ davinci_cfg_reg(DM355_VOUT_FIELD_G70);
+ break;
+
+ case V4L2_MBUS_FMT_YUYV10_1X20:
+ /*
+* This was VPBE_DIGITAL_IF_YCC16. Replace the enum
+* accordingly when the right one gets into open source
+*/
+ if (field)
+ davinci_cfg_reg(DM355_VOUT_FIELD);
+ else
+ davinci_cfg_reg(DM355_VOUT_FIELD_G70);
+ break;
+
+ default:
+ return
[PATCH v5 2/2] ARM: davinci: dm355 EVM: add support for VPBE display
From: Lad, Prabhakar
add support for V4L2 video display to DM355 EVM.
Support for SD modes is provided, along with Composite
output
Signed-off-by: Lad, Prabhakar
---
arch/arm/mach-davinci/board-dm355-evm.c | 69 ++-
1 files changed, 68 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm355-evm.c
b/arch/arm/mach-davinci/board-dm355-evm.c
index 37d12cc..1043506 100644
--- a/arch/arm/mach-davinci/board-dm355-evm.c
+++ b/arch/arm/mach-davinci/board-dm355-evm.c
@@ -242,6 +242,73 @@ static struct vpfe_config vpfe_cfg = {
.ccdc = "DM355 CCDC",
};
+/* venc standards timings */
+static struct vpbe_enc_mode_info dm355evm_enc_preset_timing[] = {
+ {
+ .name = "ntsc",
+ .timings_type = VPBE_ENC_STD,
+ .std_id = V4L2_STD_525_60,
+ .interlaced = 1,
+ .xres = 720,
+ .yres = 480,
+ .aspect = {11, 10},
+ .fps= {3, 1001},
+ .left_margin= 0x79,
+ .upper_margin = 0x10,
+ },
+ {
+ .name = "pal",
+ .timings_type = VPBE_ENC_STD,
+ .std_id = V4L2_STD_625_50,
+ .interlaced = 1,
+ .xres = 720,
+ .yres = 576,
+ .aspect = {54, 59},
+ .fps= {25, 1},
+ .left_margin= 0x7E,
+ .upper_margin = 0x16
+ },
+};
+
+#define VENC_STD_ALL (V4L2_STD_NTSC | V4L2_STD_PAL)
+
+/*
+ * The outputs available from VPBE + ecnoders. Keep the
+ * the order same as that of encoders. First those from venc followed by that
+ * from encoders. Index in the output refers to index on a particular encoder.
+ * Driver uses this index to pass it to encoder when it supports more than
+ * one output. Application uses index of the array to set an output.
+ */
+static struct vpbe_output dm355evm_vpbe_outputs[] = {
+ {
+ .output = {
+ .index = 0,
+ .name = "Composite",
+ .type = V4L2_OUTPUT_TYPE_ANALOG,
+ .std= VENC_STD_ALL,
+ .capabilities = V4L2_OUT_CAP_STD,
+ },
+ .subdev_name= DM355_VPBE_VENC_SUBDEV_NAME,
+ .default_mode = "ntsc",
+ .num_modes = ARRAY_SIZE(dm355evm_enc_preset_timing),
+ .modes = dm355evm_enc_preset_timing,
+ .if_params = V4L2_MBUS_FMT_FIXED,
+ },
+};
+
+static struct vpbe_config dm355evm_display_cfg = {
+ .module_name= "dm355-vpbe-display",
+ .i2c_adapter_id = 1,
+ .osd= {
+ .module_name= DM355_VPBE_OSD_SUBDEV_NAME,
+ },
+ .venc = {
+ .module_name= DM355_VPBE_VENC_SUBDEV_NAME,
+ },
+ .num_outputs= ARRAY_SIZE(dm355evm_vpbe_outputs),
+ .outputs= dm355evm_vpbe_outputs,
+};
+
static struct platform_device *davinci_evm_devices[] __initdata = {
_dm9000,
_nand_device,
@@ -342,7 +409,7 @@ static __init void dm355_evm_init(void)
davinci_setup_mmc(0, _mmc_config);
davinci_setup_mmc(1, _mmc_config);
- dm355_init_video(_cfg, NULL);
+ dm355_init_video(_cfg, _display_cfg);
dm355_init_spi0(BIT(0), dm355_evm_spi_info,
ARRAY_SIZE(dm355_evm_spi_info));
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] media: staging: davinci_vpfe: fix build error
From: Lad, Prabhakar add missing header file delay.h required for msleep(). This patch fixes following build error: drivers/staging/media/davinci_vpfe/dm365_isif.c: In function 'isif_enable': drivers/staging/media/davinci_vpfe/dm365_isif.c:129: error: implicit declaration of function 'msleep' make[4]: *** [drivers/staging/media/davinci_vpfe/dm365_isif.o] Error 1 Signed-off-by: Lad, Prabhakar Cc: Mauro Carvalho Chehab Cc: Greg Kroah-Hartman Cc: "Lad, Prabhakar" Cc: Sakari Ailus Cc: Laurent Pinchart Cc: Hans Verkuil Cc: de...@driverdev.osuosl.org Cc: linux-kernel@vger.kernel.org --- drivers/staging/media/davinci_vpfe/dm365_isif.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c b/drivers/staging/media/davinci_vpfe/dm365_isif.c index ebeea72..e4e6fcc 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c @@ -19,6 +19,8 @@ * Prabhakar Lad */ +#include + #include "dm365_isif.h" #include "vpfe_mc_capture.h" -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers: staging: davinci_vpfe: use resource_size()
Hi,
Thanks for the patch!
On Sun, Mar 17, 2013 at 1:13 PM, Silviu-Mihai Popescu
wrote:
> This uses the resource_size() function instead of explicit computation.
>
> Signed-off-by: Silviu-Mihai Popescu
> ---
> drivers/staging/media/davinci_vpfe/dm365_ipipe.c |3 ++-
> drivers/staging/media/davinci_vpfe/dm365_isif.c|6 --
> drivers/staging/media/davinci_vpfe/dm365_resizer.c |4 +++-
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> index 9285353..de3f202 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -27,6 +27,7 @@
> */
>
> #include
> +#include
>
> #include "dm365_ipipe.h"
> #include "dm365_ipipe_hw.h"
> @@ -1859,5 +1860,5 @@ void vpfe_ipipe_cleanup(struct vpfe_ipipe_device *ipipe,
> iounmap(ipipe->isp5_base_addr);
> res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
> if (res)
> - release_mem_region(res->start, res->end - res->start + 1);
> + release_mem_region(res->start, resource_size(res));
> }
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> index ebeea72..cd263d5 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> @@ -19,6 +19,8 @@
> * Prabhakar Lad
> */
>
> +#include
> +
> #include "dm365_isif.h"
> #include "vpfe_mc_capture.h"
>
> @@ -1953,7 +1955,7 @@ static void isif_remove(struct vpfe_isif_device *isif,
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> if (res)
> release_mem_region(res->start,
> - res->end - res->start + 1);
> + resource_size(res));
> i++;
> }
> }
> @@ -2003,7 +2005,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif,
> struct platform_device *pdev)
> status = -ENOENT;
> goto fail_nobase_res;
> }
> - res_len = res->end - res->start + 1;
> + res_len = resource_size(res);
> res = request_mem_region(res->start, res_len, res->name);
> if (!res) {
> status = -EBUSY;
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> index 9cb0262..c351ea1 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> @@ -24,6 +24,8 @@
> * same input image, but can have different output resolution.
> */
>
> +#include
> +
did you build test this patch ? the above header file(ioport.h) is not
required in all the
above files which you included.
Cheers,
--Prabhakar Lad
http://in.linkedin.com/pub/prabhakar-lad/19/92b/955
> #include "dm365_ipipe_hw.h"
> #include "dm365_resizer.h"
>
> @@ -1995,5 +1997,5 @@ vpfe_resizer_cleanup(struct vpfe_resizer_device
> *vpfe_rsz,
> res = platform_get_resource(pdev, IORESOURCE_MEM, 5);
> if (res)
> release_mem_region(res->start,
> - res->end - res->start + 1);
> + resource_size(res));
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] ARM: davinci: dm355: add support for v4l2 video display
Sekhar,
On Mon, Mar 18, 2013 at 2:11 PM, Sekhar Nori wrote:
> On 3/16/2013 3:47 PM, Prabhakar lad wrote:
>> From: Lad, Prabhakar
>>
>> Create platform devices for various video modules like venc,osd,
>> vpbe and v4l2 driver for dm355.
>>
>> Signed-off-by: Lad, Prabhakar
>
> Comments on the DM365 series apply to this one too. In addition:
>
OK.
>> +static int dm355_vpbe_setup_pinmux(enum v4l2_mbus_pixelcode if_type,
>> + int field)
>> {
>> - vpfe_capture_dev.dev.platform_data = cfg;
>> + switch (if_type) {
>> + case V4L2_MBUS_FMT_SGRBG8_1X8:
>> + davinci_cfg_reg(DM355_VOUT_FIELD_G70);
>> + break;
>> +
>> + case V4L2_MBUS_FMT_YUYV10_1X20:
>> + /*
>> + * This was VPBE_DIGITAL_IF_YCC16. Replace the enum
>
> "was VPBE_DIGITAL_IF_YCC16" when? This is the first time this code is
> getting in.
>
>> + * accordingly when the right one gets into open source
>
> What is the "right one"? And "open source" == kernel? Then may be just
> call it that.
>
Ah this comment doesn't make any sense ' V4L2_MBUS_FMT_YUYV10_1X20'
is the correct one itself. I'll remove this comment.
>> + */
>> + if (field)
>> + davinci_cfg_reg(DM355_VOUT_FIELD);
>> + else
>> + davinci_cfg_reg(DM355_VOUT_FIELD_G70);
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + davinci_cfg_reg(DM355_VOUT_COUTL_EN);
>> + davinci_cfg_reg(DM355_VOUT_COUTH_EN);
>> +
>> + return 0;
>> }
>>
>> +static inline u32 dm355_reg_modify(void *reg, u32 val, u32 mask)
>> +{
>> + u32 new_val = (readl(reg) & ~mask) | (val & mask);
>> +
>> + writel(new_val, reg);
>> +
>> + return new_val;
>> +}
>
> This function needs to be removed.
>
Ah missed, I wonder how the compiler didn't catch it!
Regards,
--Prabhakar
> Thanks,
> Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 1/2] ARM: davinci: dm365: add support for v4l2 video display
Sekhar,
On Mon, Mar 18, 2013 at 1:48 PM, Sekhar Nori wrote:
> On 3/15/2013 8:41 PM, Prabhakar lad wrote:
>> From: Lad, Prabhakar
>>
>> Create platform devices for various video modules like venc,osd,
>> vpbe and v4l2 driver for dm365.
>>
>> Signed-off-by: Lad, Prabhakar
>> ---
>> arch/arm/mach-davinci/board-dm365-evm.c |4 +-
>> arch/arm/mach-davinci/davinci.h |2 +-
>> arch/arm/mach-davinci/dm365.c | 203
>> +--
>> 3 files changed, 195 insertions(+), 14 deletions(-)
>
>> diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
>> index 6c39805..ec8b06e 100644
>> --- a/arch/arm/mach-davinci/dm365.c
>> +++ b/arch/arm/mach-davinci/dm365.c
>> @@ -40,10 +40,16 @@
>>
>> #define DM365_REF_FREQ 2400/* 24 MHz on the DM365
>> EVM */
>>
>> +#define DM3XX_VDAC_CONFIG0x01c4002c
>
> DM365_VDAC_CONFIG since this is a DM365 specific file and to be
> consistent with rest of the names?
>
OK
>> +
>> +#define DM365_RTC_BASE 0x01c69000
>> +
>> /* Base of key scan register bank */
>> #define DM365_KEYSCAN_BASE 0x01c69400
>>
>> -#define DM365_RTC_BASE 0x01c69000
>> +#define DM365_OSD_BASE 0x01c71c00
>> +
>> +#define DM365_VENC_REG_BASE 0x01c71e00
>>
>> #define DAVINCI_DM365_VC_BASE0x01d0c000
>> #define DAVINCI_DMA_VC_TX2
>> @@ -56,6 +62,11 @@
>> #define DM365_EMAC_CNTRL_RAM_OFFSET 0x1000
>> #define DM365_EMAC_CNTRL_RAM_SIZE0x2000
>>
>> +#define DM365_VPSS_CLK_CTRL_ADDR 0x44
>> +#define DM365_VPSS_VENCCLKEN_ENABLE BIT(3)
>> +#define DM365_VPSS_DACCLKEN_ENABLE BIT(4)
>> +#define DM365_VPSS_PLLC2SYSCLK5_ENABLE BIT(5)
>> +
>> static struct pll_data pll1_data = {
>> .num= 1,
>> .phys_base = DAVINCI_PLL1_BASE,
>> @@ -1226,6 +1237,186 @@ static struct platform_device dm365_isif_dev = {
>> },
>> };
>>
>> +static struct resource dm365_osd_resources[] = {
>> + {
>> + .start = DM365_OSD_BASE,
>> + .end = DM365_OSD_BASE + 0x100,
>> + .flags = IORESOURCE_MEM,
>> + },
>> +};
>> +
>> +static u64 dm365_video_dma_mask = DMA_BIT_MASK(32);
>> +
>> +static struct platform_device dm365_osd_dev = {
>> + .name = DM365_VPBE_OSD_SUBDEV_NAME,
>> + .id = -1,
>> + .num_resources = ARRAY_SIZE(dm365_osd_resources),
>> + .resource = dm365_osd_resources,
>> + .dev= {
>> + .dma_mask = _video_dma_mask,
>> + .coherent_dma_mask = DMA_BIT_MASK(32),
>> + },
>> +};
>> +
>> +static struct resource dm365_venc_resources[] = {
>> + {
>> + .start = IRQ_VENCINT,
>> + .end = IRQ_VENCINT,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + /* venc registers io space */
>> + {
>> + .start = DM365_VENC_REG_BASE,
>> + .end = DM365_VENC_REG_BASE + 0x180,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + /* vdaccfg registers io space */
>> + {
>> + .start = DM3XX_VDAC_CONFIG,
>> + .end = DM3XX_VDAC_CONFIG + 4,
>
> DM3XX_VDAC_CONFIG + 3. Check for similar errors in rest of the patch.
>
>> + .flags = IORESOURCE_MEM,
>> + },
>> +};
>> +
>> +static struct resource dm365_v4l2_disp_resources[] = {
>> + {
>> + .start = IRQ_VENCINT,
>> + .end = IRQ_VENCINT,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + /* venc registers io space */
>> + {
>> + .start = DM365_VENC_REG_BASE,
>> + .end = DM365_VENC_REG_BASE + 0x180,
>> + .flags = IORESOURCE_MEM,
>> + },
>> +};
>> +
>
> [...]
>
>> +static int dm365_venc_setup_clock(enum vpbe_enc_timings_type type,
>> + unsigned int pclock)
>> +{
>> + void __iomem *vpss_clkctl_reg;
>> + u32 val;
>> +
>> + vpss_clkctl_reg = DAVINCI_SYSMOD_VIRT(DM365_VPSS_CLK_CTRL_ADDR);
>> +
>> + switch (type) {
>> + case VPBE_ENC_STD:
>> + vpss_enable_clock(VPSS_VENC_CLOCK_SEL, 1);
>
&g
Re: [PATCH v2] drivers: staging: davinci_vpfe: use resource_size()
Hi Wei,
Thanks for the patch! I'll queue it up for v3.10
On Tue, Mar 19, 2013 at 12:36 AM, Silviu-Mihai Popescu
wrote:
> This uses the resource_size() function instead of explicit computation.
>
> Signed-off-by: Silviu-Mihai Popescu
Acked-by: Lad, Prabhakar
Regards,
--Prabhakar Lad
> ---
> drivers/staging/media/davinci_vpfe/dm365_ipipe.c |2 +-
> drivers/staging/media/davinci_vpfe/dm365_isif.c|4 ++--
> drivers/staging/media/davinci_vpfe/dm365_resizer.c |2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> index 9285353..05673ed 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -1859,5 +1859,5 @@ void vpfe_ipipe_cleanup(struct vpfe_ipipe_device *ipipe,
> iounmap(ipipe->isp5_base_addr);
> res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
> if (res)
> - release_mem_region(res->start, res->end - res->start + 1);
> + release_mem_region(res->start, resource_size(res));
> }
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> index ebeea72..c4a5660 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> @@ -1953,7 +1953,7 @@ static void isif_remove(struct vpfe_isif_device *isif,
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> if (res)
> release_mem_region(res->start,
> - res->end - res->start + 1);
> + resource_size(res));
> i++;
> }
> }
> @@ -2003,7 +2003,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif,
> struct platform_device *pdev)
> status = -ENOENT;
> goto fail_nobase_res;
> }
> - res_len = res->end - res->start + 1;
> + res_len = resource_size(res);
> res = request_mem_region(res->start, res_len, res->name);
> if (!res) {
> status = -EBUSY;
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> index 9cb0262..126f84c 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
> @@ -1995,5 +1995,5 @@ vpfe_resizer_cleanup(struct vpfe_resizer_device
> *vpfe_rsz,
> res = platform_get_resource(pdev, IORESOURCE_MEM, 5);
> if (res)
> release_mem_region(res->start,
> - res->end - res->start + 1);
> + resource_size(res));
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] davinci: vpif: Fix module build for capture and display
Hi Mauro,
On Wed, Mar 20, 2013 at 12:00 AM, Mauro Carvalho Chehab
wrote:
> Em Fri, 8 Mar 2013 16:19:07 +0530
> Prabhakar lad escreveu:
>
>> From: Lad, Prabhakar
>>
>> export the symbols which are used by two modules vpif_capture and
>> vpif_display.
>>
>> This patch fixes following error:
>> ERROR: "ch_params" [drivers/media/platform/davinci/vpif_display.ko]
>> undefined!
>> ERROR: "vpif_ch_params_count"
>> [drivers/media/platform/davinci/vpif_display.ko] undefined!
>> ERROR: "vpif_base" [drivers/media/platform/davinci/vpif_display.ko]
>> undefined!
>> ERROR: "ch_params" [drivers/media/platform/davinci/vpif_capture.ko]
>> undefined!
>> ERROR: "vpif_ch_params_count"
>> [drivers/media/platform/davinci/vpif_capture.ko] undefined!
>> ERROR: "vpif_base" [drivers/media/platform/davinci/vpif_capture.ko]
>> undefined!
>> make[1]: *** [__modpost] Error 1
>>
>> Reported-by: Sekhar Nori
>> Signed-off-by: Lad, Prabhakar
>> ---
>> Changes for v2:
>> 1: use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL() as pointed by
>> Sekhar.
>>
>> drivers/media/platform/davinci/vpif.c |4
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif.c
>> b/drivers/media/platform/davinci/vpif.c
>> index 28638a8..42c7eba 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -44,6 +44,8 @@ static struct resource *res;
>> spinlock_t vpif_lock;
>>
>> void __iomem *vpif_base;
>> +EXPORT_SYMBOL_GPL(vpif_base);
>> +
>> struct clk *vpif_clk;
>>
>> /**
>> @@ -220,8 +222,10 @@ const struct vpif_channel_config_params ch_params[] = {
>> .stdid = V4L2_STD_625_50,
>> },
>> };
>> +EXPORT_SYMBOL_GPL(ch_params);
>
> Please don't use simple names like that. It would be very easy that some
> other driver could try to declare the same symbol. Instead, prefix it
> with the driver name (vpif_ch_params).
>
Agreed, I'll respin a new version fixing it.
Cheers,
--Prabhakar
>>
>> const unsigned int vpif_ch_params_count = ARRAY_SIZE(ch_params);
>> +EXPORT_SYMBOL_GPL(vpif_ch_params_count);
>>
>> static inline void vpif_wr_bit(u32 reg, u32 bit, u32 val)
>> {
>
> Regards,
> Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] davinci: vpif: Fix module build for capture and display
From: Lad, Prabhakar
export the symbols which are used by two modules vpif_capture and
vpif_display. renamed "ch_params" to "vpif_ch_params" so as to avoid
name collision.
This patch fixes following error:
ERROR: "ch_params" [drivers/media/platform/davinci/vpif_display.ko] undefined!
ERROR: "vpif_ch_params_count" [drivers/media/platform/davinci/vpif_display.ko]
undefined!
ERROR: "vpif_base" [drivers/media/platform/davinci/vpif_display.ko] undefined!
ERROR: "ch_params" [drivers/media/platform/davinci/vpif_capture.ko] undefined!
ERROR: "vpif_ch_params_count" [drivers/media/platform/davinci/vpif_capture.ko]
undefined!
ERROR: "vpif_base" [drivers/media/platform/davinci/vpif_capture.ko] undefined!
make[1]: *** [__modpost] Error 1
Reported-by: Sekhar Nori
Signed-off-by: Lad, Prabhakar
Cc: Mauro Carvalho Chehab
---
Changes for v3:
1: prefixed "ch_params" with driver name to make it "vpif_ch_params"
as pointed by Mauro.
Changes for v2:
1: use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL() as pointed by
Sekhar.
drivers/media/platform/davinci/vpif.c | 10 +++---
drivers/media/platform/davinci/vpif.h |2 +-
drivers/media/platform/davinci/vpif_capture.c |2 +-
drivers/media/platform/davinci/vpif_display.c |2 +-
4 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif.c
b/drivers/media/platform/davinci/vpif.c
index 28638a8..3bc4db8 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -44,13 +44,15 @@ static struct resource *res;
spinlock_t vpif_lock;
void __iomem *vpif_base;
+EXPORT_SYMBOL_GPL(vpif_base);
+
struct clk *vpif_clk;
/**
- * ch_params: video standard configuration parameters for vpif
+ * vpif_ch_params: video standard configuration parameters for vpif
* The table must include all presets from supported subdevices.
*/
-const struct vpif_channel_config_params ch_params[] = {
+const struct vpif_channel_config_params vpif_ch_params[] = {
/* HDTV formats */
{
.name = "480p59_94",
@@ -220,8 +222,10 @@ const struct vpif_channel_config_params ch_params[] = {
.stdid = V4L2_STD_625_50,
},
};
+EXPORT_SYMBOL_GPL(vpif_ch_params);
-const unsigned int vpif_ch_params_count = ARRAY_SIZE(ch_params);
+const unsigned int vpif_ch_params_count = ARRAY_SIZE(vpif_ch_params);
+EXPORT_SYMBOL_GPL(vpif_ch_params_count);
static inline void vpif_wr_bit(u32 reg, u32 bit, u32 val)
{
diff --git a/drivers/media/platform/davinci/vpif.h
b/drivers/media/platform/davinci/vpif.h
index a1ab6a0..9956e67 100644
--- a/drivers/media/platform/davinci/vpif.h
+++ b/drivers/media/platform/davinci/vpif.h
@@ -638,7 +638,7 @@ struct vpif_channel_config_params {
};
extern const unsigned int vpif_ch_params_count;
-extern const struct vpif_channel_config_params ch_params[];
+extern const struct vpif_channel_config_params vpif_ch_params[];
struct vpif_video_params;
struct vpif_params;
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index b15e948..1846cc5 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -563,7 +563,7 @@ static int vpif_update_std_info(struct channel_obj *ch)
vpif_dbg(2, debug, "vpif_update_std_info\n");
for (index = 0; index < vpif_ch_params_count; index++) {
- config = _params[index];
+ config = _ch_params[index];
if (config->hd_sd == 0) {
vpif_dbg(2, debug, "SD format\n");
if (config->stdid & vid_ch->stdid) {
diff --git a/drivers/media/platform/davinci/vpif_display.c
b/drivers/media/platform/davinci/vpif_display.c
index efdd462..48ed400 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -511,7 +511,7 @@ static int vpif_update_std_info(struct channel_obj *ch)
int i;
for (i = 0; i < vpif_ch_params_count; i++) {
- config = _params[i];
+ config = _ch_params[i];
if (config->hd_sd == 0) {
vpif_dbg(2, debug, "SD format\n");
if (config->stdid & vid_ch->stdid) {
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] media: i2c: tvp514x: add OF support
Hi All, On Tue, May 14, 2013 at 1:36 PM, Lad Prabhakar wrote: > From: Lad, Prabhakar > > add OF support for the tvp514x driver. Alongside this patch > removes unnecessary header file inclusion and sorts them alphabetically. > Ahh just noticed it now, I'll just rebase and resend this patch on [1] so that slab.h can also be removed. [1] https://patchwork.kernel.org/patch/2539411/ Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] media: i2c: tvp7002: add OF support
Hi Laurent, Thanks for the review. On Tue, May 14, 2013 at 9:04 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > Thank you for the patch. > > On Tuesday 14 May 2013 16:15:34 Lad Prabhakar wrote: >> From: Lad, Prabhakar >> >> add OF support for the tvp7002 driver. >> >> Signed-off-by: Lad, Prabhakar >> Cc: Hans Verkuil >> Cc: Laurent Pinchart >> Cc: Mauro Carvalho Chehab >> Cc: Guennadi Liakhovetski >> Cc: Sylwester Nawrocki >> Cc: Sakari Ailus >> Cc: Grant Likely >> Cc: Rob Herring >> Cc: Rob Landley >> Cc: devicetree-disc...@lists.ozlabs.org >> Cc: linux-...@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: davinci-linux-open-sou...@linux.davincidsp.com >> --- >> .../devicetree/bindings/media/i2c/tvp7002.txt | 42 + >> drivers/media/i2c/tvp7002.c| 64 +++-- >> 2 files changed, 99 insertions(+), 7 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt >> b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt new file mode >> 100644 >> index 000..1ebd8b1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt >> @@ -0,0 +1,42 @@ >> +* Texas Instruments TV7002 video decoder >> + >> +The TVP7002 device supports digitizing of video and graphics signal in RGB >> and >> +YPbPr color space. >> + >> +Required Properties : >> +- compatible : Must be "ti,tvp7002" >> + >> +- hsync-active: HSYNC Polarity configuration for endpoint. >> + >> +- vsync-active: VSYNC Polarity configuration for endpoint. >> + >> +- pclk-sample: Clock polarity of the endpoint. >> + >> +- ti,tvp7002-fid-polarity: Active-high Field ID polarity of the endpoint. >> + >> +- ti,tvp7002-sog-polarity: Sync on Green output polarity of the endpoint. > > Would it make sense to define field-active and sog-active properties in the > V4L2 bindings instead of having per-chip properties ? > yes you are right these properties need to be in the device node rather than the port node. I'll send alone this patch of the series as v2 fixing the above. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: davinci: dm365 evm: remove init_enable from ths7303 pdata
Hi Laurent, Thanks for the review. On Wed, May 15, 2013 at 5:35 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > Thank you for the patch. > > On Wednesday 15 May 2013 17:27:18 Lad Prabhakar wrote: >> From: Lad, Prabhakar >> >> remove init_enable from ths7303 pdata as it is no longer exists. > > You should move this before 1/6, otherwise you will break bisection. > OK I'll fix it in next version. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] media: OF: add field-active and sync-on-green endpoint properties
Hi Laurent, On Wed, May 15, 2013 at 6:54 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > Thank you for the patch. > > On Wednesday 15 May 2013 18:22:29 Lad Prabhakar wrote: >> From: Lad, Prabhakar >> >> This patch adds "field-active" and "sync-on-green" as part of >> endpoint properties and also support to parse them in the parser. >> >> Signed-off-by: Lad, Prabhakar >> Cc: Hans Verkuil >> Cc: Laurent Pinchart >> Cc: Mauro Carvalho Chehab >> Cc: Guennadi Liakhovetski >> Cc: Sylwester Nawrocki >> Cc: Sakari Ailus >> Cc: Grant Likely >> Cc: Rob Herring >> Cc: Rob Landley >> Cc: devicetree-disc...@lists.ozlabs.org >> Cc: linux-...@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: davinci-linux-open-sou...@linux.davincidsp.com >> Cc: Kyungmin Park >> --- >> .../devicetree/bindings/media/video-interfaces.txt |4 >> drivers/media/v4l2-core/v4l2-of.c |6 ++ >> include/media/v4l2-mediabus.h |2 ++ >> 3 files changed, 12 insertions(+), 0 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt >> b/Documentation/devicetree/bindings/media/video-interfaces.txt index >> e022d2d..6bf87d0 100644 >> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt >> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt >> @@ -101,6 +101,10 @@ Optional endpoint properties >>array contains only one entry. >> - clock-noncontinuous: a boolean property to allow MIPI CSI-2 >> non-continuous clock mode. >> +-field-active: a boolean property indicating active high filed ID output >> + polarity is inverted. > > Looks like we already have field-even-active property to describe the level of > the field signal. Could you please check whether it fulfills your use cases ? > Sorry for not pointing you to it earlier. > I had looked at it earlier it only means "field signal level during the even field data transmission" it only speaks of even filed. Ideally the field ID output is set to logic 1 for odd field and set to 0 for even field, what I want is to invert the FID out polarity when "field-active" property is set. May be we rename "field-active" to "fid-pol" ? Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] media: OF: add field-active and sync-on-green endpoint properties
Hi Sylwester, On Thu, May 16, 2013 at 4:13 PM, Sylwester Nawrocki wrote: > Hi, > > > On 05/16/2013 06:53 AM, Prabhakar Lad wrote: >>>> [Snip] >> May be we rename "field-active" to "fid-pol" ? > > > I guess we failed to clearly describe the 'field-even-active' property then. > It seems to be exactly what you need. > > It is not enough to say e.g. field-active = <1>;, because it would not have > been clear which field it refers to, odd or even ? Unlike VSYNC, HSYNC both > levels of the FIELD signal are "active", there is no "idle" state for FIELD. > > So field-even-active = <1>; means the FIELD signal at logic high level > indicates EVEN field _and_ this implies FIELD = 0 indicates ODD field, i.e. > > FIELD = 0 => odd field > FIELD = 1 => even field > > For field-even-active = <0>; it is the other way around: > > FIELD = 0 => even field > FIELD = 1 => odd field > Thanks that makes it clear :) > It looks like only "sync-on-green" property is missing. BTW, is it really > commonly used ? What drivers would need it ? > I'm not against making it a common property, it's just first time I see it. > I have comes across a decoder tvp7002 which uses it, may be Laurent/Hans/Sakari may point much more devices. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] media: OF: add field-active and sync-on-green endpoint properties
Hi Sylwester, Thanks for the review. On Thu, May 16, 2013 at 4:15 PM, Sylwester Nawrocki wrote: > On 05/15/2013 02:52 PM, Lad Prabhakar wrote: >> >> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt >> b/Documentation/devicetree/bindings/media/video-interfaces.txt >> index e022d2d..6bf87d0 100644 >> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt >> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt >> @@ -101,6 +101,10 @@ Optional endpoint properties >> array contains only one entry. >> - clock-noncontinuous: a boolean property to allow MIPI CSI-2 >> non-continuous >> clock mode. >> +-field-active: a boolean property indicating active high filed ID output >> + polarity is inverted. > > > You can drop this property and use the existing 'field-even-active' property > instead. > OK > >> +-sync-on-green: a boolean property indicating to sync with the green >> signal in >> + RGB. > > >> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h >> index 83ae07e..b95553d 100644 >> --- a/include/media/v4l2-mediabus.h >> +++ b/include/media/v4l2-mediabus.h >> @@ -40,6 +40,8 @@ >> #define V4L2_MBUS_FIELD_EVEN_HIGH (1<< 10) >> /* FIELD = 1/0 - Field1 (odd)/Field2 (even) */ >> #define V4L2_MBUS_FIELD_EVEN_LOW (1<< 11) >> +#define V4L2_MBUS_FIELD_ACTIVE (1<< 12) >> +#define V4L2_MBUS_SOG (1<< 13) > > > How about V4L2_MBUS_SYNC_ON_GREEN ? > OK makes it more readable. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: davinci: dm365 evm: remove init_enable from ths7303 pdata
Hi Laurent, On Wed, May 15, 2013 at 5:41 PM, Prabhakar Lad wrote: > Hi Laurent, > > Thanks for the review. > > On Wed, May 15, 2013 at 5:35 PM, Laurent Pinchart > wrote: >> Hi Prabhakar, >> >> Thank you for the patch. >> >> On Wednesday 15 May 2013 17:27:18 Lad Prabhakar wrote: >>> From: Lad, Prabhakar >>> >>> remove init_enable from ths7303 pdata as it is no longer exists. >> >> You should move this before 1/6, otherwise you will break bisection. >> How about I just reshuffles while issuing a pull rather than resending the whole series ? Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] media: i2c: tvp514x: add OF support
Hi Laurent, Thanks for the review. On Thu, May 16, 2013 at 5:40 PM, Laurent Pinchart wrote: > Hi Prabhakar, > [Snip] >> + >> + pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) > > I've started playing with the V4L2 OF bindings, and realized that should > should call of_node_put() here. > you were referring of_node_get() here rite ? of_node_get/put() got recently added I guess coz of which I missed it :) >> + return NULL; >> + >> + v4l2_of_parse_endpoint(endpoint, _cfg); >> + flags = bus_cfg.bus.parallel.flags; >> + >> + if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) >> + pdata->hs_polarity = 1; >> + >> + if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) >> + pdata->vs_polarity = 1; >> + >> + if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING) >> + pdata->clk_polarity = 1; >> + > > As well as here. Maybe a > > done: > of_node_put(endpoint); > return pdata; > > with a goto done in the devm_kzalloc error path would be better. > OK Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] media: i2c: tvp514x: add OF support
Hi Laurent, On Thu, May 16, 2013 at 6:20 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > On Thursday 16 May 2013 18:13:38 Prabhakar Lad wrote: >> On Thu, May 16, 2013 at 5:40 PM, Laurent Pinchart wrote: >> > Hi Prabhakar, >> >> [Snip] >> >> >> + >> >> + pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL); >> >> + if (!pdata) >> > >> > I've started playing with the V4L2 OF bindings, and realized that should >> > should call of_node_put() here. >> >> you were referring of_node_get() here rite ? > > No, I'm referring to of_node_put(). The v4l2_of_get_next_endpoint() function > mentions > > * Return: An 'endpoint' node pointer with refcount incremented. Refcount > * of the passed @prev node is not decremented, the caller have to use > * of_node_put() on it when done. > Ahh I see thanks for clarifying, I'll fix it for v3. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] net: davinci_mdio: trivial cleanup
Hi Sergei, Thanks for the review. On Fri, May 17, 2013 at 12:28 AM, Sergei Shtylyov wrote: > On 16-05-2013 11:30, Lad Prabhakar wrote: > >> From: Lad, Prabhakar > > >> remove unwanted header inclusion and sort the alphabetically > > >s/the/them/. OK > > >> also guard the davinci_mdio_of_mtable table and davinci_mdio_probe_dt() >> with CONFIG_OF. > > >Saying "also" in the changelog is often a good sign there should be one > more patch -- as in this case. > OK I'll split the patch. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] net: davinci_emac: remove unwanted header inclusion and sort the alphabetically
Hi Sergei, Thanks for the review. On Fri, May 17, 2013 at 12:25 AM, Sergei Shtylyov wrote: > On 16-05-2013 11:30, Lad Prabhakar wrote: > >> From: Lad, Prabhakar > > >> This patch removes unwanted header inclusion > > >Why are they unwanted? > The driver builds without this includes. This is arguable, if I would have added a new driver with the only #includes which were required to build thats accepted. But when I remove unnecessary #includes from the existing drivers that's not acceptable ? This applies to rest of the similar patches in the series. >> and sorts them alphabetically > > >In the subject you typed "the" instead of "them". > Hmm carry forwarded this copy paste error every where will fix it in V2. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] media: i2c: ths7303: remove init_enable option from pdata
Hi Sakari, Thanks for the review. On Fri, May 17, 2013 at 4:07 AM, Sakari Ailus wrote: > Hi Prabhakar, > > Thanks for the patch! > > On Wed, May 15, 2013 at 05:27:17PM +0530, Lad Prabhakar wrote: >> diff --git a/drivers/media/i2c/ths7303.c b/drivers/media/i2c/ths7303.c >> index 65853ee..8cddcd0 100644 >> --- a/drivers/media/i2c/ths7303.c >> +++ b/drivers/media/i2c/ths7303.c >> @@ -356,9 +356,7 @@ static int ths7303_setup(struct v4l2_subdev *sd) >> int ret; >> u8 mask; >> >> - state->stream_on = pdata->init_enable; >> - >> - mask = state->stream_on ? 0xff : 0xf8; >> + mask = 0xf8; > > You can assign mask in declaration. It'd be nice to have a human-readable > name for the mask, too. > This function gets removed in the preceding patch of this series. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: davinci: dma: Convert to devm_* api
Hi Sergei,
Thanks for the review.
On Thu, May 16, 2013 at 6:47 PM, Sergei Shtylyov
wrote:
> Hello.
>
>
> On 16-05-2013 10:58, Lad Prabhakar wrote:
>
>> From: Lad, Prabhakar
>
>
>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>> devm_request_irq() instead of request_irq().
>
>
>> This ensures more consistent error values and simplifies error paths.
>
>
>> Signed-off-by: Lad, Prabhakar
>> ---
>> NOte:- Boot tested on Logic-PD OMAP-L138 EVM
>
>
>> arch/arm/mach-davinci/dma.c | 63
>> --
>> 1 files changed, 24 insertions(+), 39 deletions(-)
>
>
>> diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
>> index 45b7c71..aeda496 100644
>> --- a/arch/arm/mach-davinci/dma.c
>> +++ b/arch/arm/mach-davinci/dma.c
>
> [...]
>
>> @@ -1422,25 +1421,16 @@ static int __init edma_probe(struct
>> platform_device *pdev)
>> found = 1;
>> }
>>
>> - len[j] = resource_size(r[j]);
>> -
>> - r[j] = request_mem_region(r[j]->start, len[j],
>> - dev_name(>dev));
>> - if (!r[j]) {
>> - status = -EBUSY;
>> - goto fail1;
>> - }
>> -
>> - edmacc_regs_base[j] = ioremap(r[j]->start, len[j]);
>> - if (!edmacc_regs_base[j]) {
>> + edmacc_regs_base[j] = devm_ioremap_resource(>dev,
>> r[j]);
>> + if (IS_ERR(edmacc_regs_base[j])) {
>> status = -EBUSY;
>
>
>And you call that "more consistent error values"? Why not:
>
> status = PTR_ERR(edmacc_regs_base[j]);
>
Yes missed it will fix it in v2.
>
>> edma_cc[j] = kzalloc(sizeof(struct edma), GFP_KERNEL);
>
>
>Maybe it's worth using devm_kzalloc() too?
>
OK
Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/7] media: davinci: vpif: remove unwanted header includes
Hi Laurent, Thanks for the review. On Thu, May 16, 2013 at 6:32 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > Thank you for the patch. > > On Thursday 16 May 2013 18:28:16 Lad Prabhakar wrote: >> From: Lad, Prabhakar >> >> Signed-off-by: Lad, Prabhakar >> --- >> drivers/media/platform/davinci/vpif.c |7 --- >> 1 files changed, 0 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/media/platform/davinci/vpif.c >> b/drivers/media/platform/davinci/vpif.c index ea82a8b..d354d50 100644 >> --- a/drivers/media/platform/davinci/vpif.c >> +++ b/drivers/media/platform/davinci/vpif.c >> @@ -17,18 +17,11 @@ >> * GNU General Public License for more details. >> */ >> >> -#include >> #include >> #include >> -#include >> -#include >> -#include >> -#include >> #include >> #include > > I think you should keep most of those includes. For instance this file uses > spinlock functions, so linux/spinlock.h should be included. It might work fine > now due to nested includes, but if someone reorganizes the kernel headers > internal includes then the driver might break. As a general rule of good > practice you should include headers for all the APIs you use. > OK, do you want me too drop the similar patches from this series ? Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] davinci: dm644x: fix enum ccdc_gama_width and enum ccdc_data_size comparision warning
Hi Mauro,
On Tue, Feb 5, 2013 at 9:54 PM, Mauro Carvalho Chehab
wrote:
> Em Wed, 2 Jan 2013 17:23:50 +0530
> "Lad, Prabhakar" escreveu:
>
>> while the effect is harmless this patch
>
> I disagree that this is a harmless warning. It is here for a reason:
> you should not be relying on the enum "magic" value, since the main
> reason to use an enum is to fill/compare the enum fields only by their
> names, and not by their number.
>
OK
>> fixes following build warning,
>>
>> drivers/media/platform/davinci/dm644x_ccdc.c: In function
>> ‘validate_ccdc_param’:
>> drivers/media/platform/davinci/dm644x_ccdc.c:233:32: warning: comparison
>> between
>> ‘enum ccdc_gama_width’ and ‘enum ccdc_data_size’ [-Wenum-compare]
>>
>> Signed-off-by: Lad, Prabhakar
>> ---
>> drivers/media/platform/davinci/dm644x_ccdc.c |5 -
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c
>> b/drivers/media/platform/davinci/dm644x_ccdc.c
>> index ee7942b..42b473a 100644
>> --- a/drivers/media/platform/davinci/dm644x_ccdc.c
>> +++ b/drivers/media/platform/davinci/dm644x_ccdc.c
>> @@ -228,9 +228,12 @@ static void ccdc_readregs(void)
>> static int validate_ccdc_param(struct ccdc_config_params_raw *ccdcparam)
>> {
>> if (ccdcparam->alaw.enable) {
>> + u32 gama_wd = ccdcparam->alaw.gama_wd;
>> + u32 data_sz = ccdcparam->data_sz;
>> +
>> if ((ccdcparam->alaw.gama_wd > CCDC_GAMMA_BITS_09_0) ||
>> (ccdcparam->alaw.gama_wd < CCDC_GAMMA_BITS_15_6) ||
>> - (ccdcparam->alaw.gama_wd < ccdcparam->data_sz)) {
>> + (gama_wd < data_sz)) {
>
> hmm... from include/media/davinci/dm644x_ccdc.h:
> enum ccdc_gama_width {
> CCDC_GAMMA_BITS_15_6, // 0
> CCDC_GAMMA_BITS_14_5, // 1
> CCDC_GAMMA_BITS_13_4, // 2
> CCDC_GAMMA_BITS_12_3, // 3
> CCDC_GAMMA_BITS_11_2, // 4
> CCDC_GAMMA_BITS_10_1, // 5
> CCDC_GAMMA_BITS_09_0// 6
> };
>
> enum ccdc_data_size {
> CCDC_DATA_16BITS, // 0
> CCDC_DATA_15BITS, // 1
> CCDC_DATA_14BITS, // 2
> CCDC_DATA_13BITS, // 3
> CCDC_DATA_12BITS, // 4
> CCDC_DATA_11BITS, // 5
> CCDC_DATA_10BITS, // 6
> CCDC_DATA_8BITS // 7
> };
>
> That doesn't seem right, as comparing the enum integer value won't
> warrant that the number of bits of gamma. For example, gamma == 6
> means 9 bits, while ccdc == 6 means 10 bits.
>
> In any case, the code is just crappy, as one could anytime add more
> values at the enum or reorder.
>
yes
> So, a better fix would be to have an array that would convert from the
> enum "magic" number into the number of bits.
>
> Hmm... wait a moment: why are you using an enum here at the first place???
>
> It seems that it would be a way better to just use 2 unsigned integers:
> ccdc_data_num_bits and ccdc_gama_num_bits, and just fill it with the
> number of bits, instead of declaring an enum for it.
>
I will pick this option :)
Regards,
--Prabhakar
> Another alternative would be to merge them into just one enum, like:
>
> enum ccdc_bits {
> CCDC_8_BITS = 8,
> CCDC_9_BITS = 9,
> CCDC_10_BITS = 10,
> CCDC_11_BITS = 11,
> CCDC_12_BITS = 12,
> CCDC_13_BITS = 13,
> CCDC_14_BITS = 14,
> CCDC_15_BITS = 15,
> CCDC_16_BITS = 16,
> };
>
> and replace all occurrences of ccdc_data_size and ccdc_gama_width by
> the new enum.
>
> This way, you could trust on compare one field with the other.
>
>> dev_dbg(ccdc_cfg.dev, "\nInvalid data line select");
>> return -1;
>> }
>
> Regards,
> Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] media: ths7353: add support for ths7353 video amplifier
From: Lad, Prabhakar
The patch adds support for THS7353 video amplifier.
The the THS7353 amplifier is very much similar to the
existing THS7303 video amplifier driver.
This patch appropriately makes changes to the existing
ths7303 driver and adds support for the THS7353.
This patch also adds V4L2_IDENT_THS7353 for the THS7353
chip and appropriate changes to Kconfig file for building.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Hans Verkuil
Signed-off-by: Martin Bugge
Cc: Chaithrika U S
---
Changes for v2:
1: Merged the driver in existing ths7303 driver.
2: Merged the patch which adds the chip indent in same patch.
drivers/media/i2c/Kconfig |4 +-
drivers/media/i2c/ths7303.c | 334 +--
include/media/ths7303.h | 35
include/media/v4l2-chip-ident.h |3 +
4 files changed, 327 insertions(+), 49 deletions(-)
create mode 100644 include/media/ths7303.h
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 1e4b2d0..2e02916 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -563,9 +563,9 @@ comment "Miscelaneous helper chips"
config VIDEO_THS7303
tristate "THS7303 Video Amplifier"
- depends on I2C
+ depends on VIDEO_V4L2 && I2C
help
- Support for TI THS7303 video amplifier
+ Support for TI THS7303/53 video amplifier
To compile this driver as a module, choose M here: the
module will be called ths7303.
diff --git a/drivers/media/i2c/ths7303.c b/drivers/media/i2c/ths7303.c
index e747524..e7f8c59 100644
--- a/drivers/media/i2c/ths7303.c
+++ b/drivers/media/i2c/ths7303.c
@@ -1,7 +1,15 @@
/*
- * ths7303- THS7303 Video Amplifier driver
+ * ths7303/53- THS7303/53 Video Amplifier driver
*
* Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright 2013 Cisco Systems, Inc. and/or its affiliates.
+ *
+ * Author: Chaithrika U S
+ *
+ * Contributors:
+ * Lad, Prabhakar
+ * Hans Verkuil
+ * Martin Bugge
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
@@ -13,25 +21,27 @@
* GNU General Public License for more details.
*/
-#include
-#include
-#include
-#include
#include
-#include
-#include
#include
-#include
-#include
+#include
-#include
-#include
+#include
#include
+#include
#define THS7303_CHANNEL_1 1
#define THS7303_CHANNEL_2 2
#define THS7303_CHANNEL_3 3
+struct ths7303_state {
+ struct v4l2_subdev sd;
+ struct ths7303_platform_data pdata;
+ struct v4l2_dv_timings dv_timings;
+ int std_id;
+ int stream_on;
+ int driver_data;
+};
+
enum ths7303_filter_mode {
THS7303_FILTER_MODE_480I_576I,
THS7303_FILTER_MODE_480P_576P,
@@ -48,64 +58,97 @@ static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
+static inline struct ths7303_state *to_state(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct ths7303_state, sd);
+}
+
+static int ths7303_read(struct v4l2_subdev *sd, u8 reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ return i2c_smbus_read_byte_data(client, reg);
+}
+
+static int ths7303_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ int ret;
+ int i;
+
+ for (i = 0; i < 3; i++) {
+ ret = i2c_smbus_write_byte_data(client, reg, val);
+ if (ret == 0)
+ return 0;
+ }
+ return ret;
+}
+
/* following function is used to set ths7303 */
int ths7303_setval(struct v4l2_subdev *sd, enum ths7303_filter_mode mode)
{
- u8 input_bias_chroma = 3;
- u8 input_bias_luma = 3;
- int disable = 0;
- int err = 0;
- u8 val = 0;
- u8 temp;
-
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ths7303_state *state = to_state(sd);
+ struct ths7303_platform_data *pdata = >pdata;
+ u8 val, sel = 0;
+ int err, disable = 0;
if (!client)
return -EINVAL;
+ if (!state->stream_on) {
+ ths7303_write(sd, THS7303_CHANNEL_1,
+ (ths7303_read(sd, THS7303_CHANNEL_1) & 0xf8) |
+ 0x00);
+ ths7303_write(sd, THS7303_CHANNEL_2,
+ (ths7303_read(sd, THS7303_CHANNEL_2) & 0xf8) |
+ 0x00);
+ ths7303_write(sd, THS7303_CHANNEL_3,
+ (ths7303_read(sd, THS7303_CHANNEL_3) & 0xf8) |
+ 0x00);
+ return 0;
+ }
+
switch (mode) {
case THS7303_FILTER_MODE_1080P:
- val = (3 << 6);
- val |= (3 << 3);
+ sel = 0x3; /*1080p and SXGA/UXGA */
Re: [PATCH v2] media: ths7353: add support for ths7353 video amplifier
Hi Hans,
Thanks for the review.
On Wed, Feb 6, 2013 at 2:33 PM, Hans Verkuil wrote:
> Hi Prabhakar,
>
> Here is my review. There are a few things that need to be fixed before I
> can Ack it.
>
OK
> On Tue 5 February 2013 20:20:21 Prabhakar Lad wrote:
>> From: Lad, Prabhakar
>>
>> The patch adds support for THS7353 video amplifier.
>> The the THS7353 amplifier is very much similar to the
>> existing THS7303 video amplifier driver.
>> This patch appropriately makes changes to the existing
>> ths7303 driver and adds support for the THS7353.
>> This patch also adds V4L2_IDENT_THS7353 for the THS7353
>> chip and appropriate changes to Kconfig file for building.
>>
>> Signed-off-by: Lad, Prabhakar
>> Signed-off-by: Hans Verkuil
>> Signed-off-by: Martin Bugge
>> Cc: Chaithrika U S
>> ---
>> Changes for v2:
>> 1: Merged the driver in existing ths7303 driver.
>> 2: Merged the patch which adds the chip indent in same patch.
>>
>> drivers/media/i2c/Kconfig |4 +-
>> drivers/media/i2c/ths7303.c | 334
>> +--
>> include/media/ths7303.h | 35
>> include/media/v4l2-chip-ident.h |3 +
>> 4 files changed, 327 insertions(+), 49 deletions(-)
>> create mode 100644 include/media/ths7303.h
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 1e4b2d0..2e02916 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -563,9 +563,9 @@ comment "Miscelaneous helper chips"
>>
>> config VIDEO_THS7303
>> tristate "THS7303 Video Amplifier"
>> - depends on I2C
>> + depends on VIDEO_V4L2 && I2C
>> help
>> - Support for TI THS7303 video amplifier
>> + Support for TI THS7303/53 video amplifier
>>
>> To compile this driver as a module, choose M here: the
>> module will be called ths7303.
>> diff --git a/drivers/media/i2c/ths7303.c b/drivers/media/i2c/ths7303.c
>> index e747524..e7f8c59 100644
>> --- a/drivers/media/i2c/ths7303.c
>> +++ b/drivers/media/i2c/ths7303.c
>> @@ -1,7 +1,15 @@
>> /*
>> - * ths7303- THS7303 Video Amplifier driver
>> + * ths7303/53- THS7303/53 Video Amplifier driver
>> *
>> * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
>> + * Copyright 2013 Cisco Systems, Inc. and/or its affiliates.
>> + *
>> + * Author: Chaithrika U S
>> + *
>> + * Contributors:
>> + * Lad, Prabhakar
>> + * Hans Verkuil
>> + * Martin Bugge
>> *
>> * This program is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU General Public License as
>> @@ -13,25 +21,27 @@
>> * GNU General Public License for more details.
>> */
>>
>> -#include
>> -#include
>> -#include
>> -#include
>> #include
>> -#include
>> -#include
>> #include
>> -#include
>> -#include
>> +#include
>>
>> -#include
>> -#include
>> +#include
>> #include
>> +#include
>>
>> #define THS7303_CHANNEL_11
>> #define THS7303_CHANNEL_22
>> #define THS7303_CHANNEL_33
>>
>> +struct ths7303_state {
>> + struct v4l2_subdev sd;
>> + struct ths7303_platform_data pdata;
>> + struct v4l2_dv_timings dv_timings;
>> + int std_id;
>> + int stream_on;
>> + int driver_data;
>> +};
>> +
>> enum ths7303_filter_mode {
>> THS7303_FILTER_MODE_480I_576I,
>> THS7303_FILTER_MODE_480P_576P,
>> @@ -48,64 +58,97 @@ static int debug;
>> module_param(debug, int, 0644);
>> MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>> +static inline struct ths7303_state *to_state(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct ths7303_state, sd);
>> +}
>> +
>> +static int ths7303_read(struct v4l2_subdev *sd, u8 reg)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + return i2c_smbus_read_byte_data(client, reg);
>> +}
>> +
>> +static int ths7303_write(struct v4l2_subdev *sd, u8 reg, u8 val)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> + int ret;
>> + int i;
>> +
>> + for (i = 0; i < 3; i++) {
>> + ret = i2c_smbus_write_byte_data(client, reg, val);
>> + if (ret == 0)
>
[PATCH v3] media: ths7353: add support for ths7353 video amplifier
From: Lad, Prabhakar
The patch adds support for THS7353 video amplifier.
The the THS7353 amplifier is very much similar to the
existing THS7303 video amplifier driver.
This patch appropriately makes changes to the existing
ths7303 driver and adds support for the THS7353.
This patch also adds V4L2_IDENT_THS7353 for the THS7353
chip and appropriate changes to Kconfig file for building.
Signed-off-by: Lad, Prabhakar
Signed-off-by: Hans Verkuil
Signed-off-by: Martin Bugge
Cc: Chaithrika U S
---
Changes for v3:
1: Fixed review comments pointed out by Hans.
Changes for v2:
1: Merged the driver in existing ths7303 driver.
2: Merged the patch which adds the chip indent in same patch.
drivers/media/i2c/Kconfig |6 +-
drivers/media/i2c/ths7303.c | 353 ---
include/media/ths7303.h | 42 +
include/media/v4l2-chip-ident.h |3 +
4 files changed, 343 insertions(+), 61 deletions(-)
create mode 100644 include/media/ths7303.h
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index ecdf7e3..bd08541 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -576,10 +576,10 @@ config VIDEO_UPD64083
comment "Miscelaneous helper chips"
config VIDEO_THS7303
- tristate "THS7303 Video Amplifier"
- depends on I2C
+ tristate "THS7303/53 Video Amplifier"
+ depends on VIDEO_V4L2 && I2C
help
- Support for TI THS7303 video amplifier
+ Support for TI THS7303/53 video amplifier
To compile this driver as a module, choose M here: the
module will be called ths7303.
diff --git a/drivers/media/i2c/ths7303.c b/drivers/media/i2c/ths7303.c
index e747524..7300abc 100644
--- a/drivers/media/i2c/ths7303.c
+++ b/drivers/media/i2c/ths7303.c
@@ -1,7 +1,15 @@
/*
- * ths7303- THS7303 Video Amplifier driver
+ * ths7303/53- THS7303/53 Video Amplifier driver
*
* Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright 2013 Cisco Systems, Inc. and/or its affiliates.
+ *
+ * Author: Chaithrika U S
+ *
+ * Contributors:
+ * Lad, Prabhakar
+ * Hans Verkuil
+ * Martin Bugge
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
@@ -13,25 +21,27 @@
* GNU General Public License for more details.
*/
-#include
-#include
-#include
-#include
#include
-#include
-#include
#include
-#include
-#include
+#include
-#include
-#include
+#include
#include
+#include
#define THS7303_CHANNEL_1 1
#define THS7303_CHANNEL_2 2
#define THS7303_CHANNEL_3 3
+struct ths7303_state {
+ struct v4l2_subdev sd;
+ struct ths7303_platform_datapdata;
+ struct v4l2_bt_timings bt;
+ int std_id;
+ int stream_on;
+ int driver_data;
+};
+
enum ths7303_filter_mode {
THS7303_FILTER_MODE_480I_576I,
THS7303_FILTER_MODE_480P_576P,
@@ -48,64 +58,84 @@ static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
+static inline struct ths7303_state *to_state(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct ths7303_state, sd);
+}
+
+static int ths7303_read(struct v4l2_subdev *sd, u8 reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ return i2c_smbus_read_byte_data(client, reg);
+}
+
+static int ths7303_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ int ret;
+ int i;
+
+ for (i = 0; i < 3; i++) {
+ ret = i2c_smbus_write_byte_data(client, reg, val);
+ if (ret == 0)
+ return 0;
+ }
+ return ret;
+}
+
/* following function is used to set ths7303 */
int ths7303_setval(struct v4l2_subdev *sd, enum ths7303_filter_mode mode)
{
- u8 input_bias_chroma = 3;
- u8 input_bias_luma = 3;
- int disable = 0;
- int err = 0;
- u8 val = 0;
- u8 temp;
-
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ths7303_state *state = to_state(sd);
+ struct ths7303_platform_data *pdata = >pdata;
+ u8 val, sel = 0;
+ int err, disable = 0;
if (!client)
return -EINVAL;
switch (mode) {
case THS7303_FILTER_MODE_1080P:
- val = (3 << 6);
- val |= (3 << 3);
+ sel = 0x3; /*1080p and SXGA/UXGA */
break;
case THS7303_FILTER_MODE_720P_1080I:
- val = (2 << 6);
- val |= (2 << 3);
+ sel = 0x2; /*720p, 1080i and SVGA/XGA */
break;
case THS7303_FILTER_MODE_480P_576P:
- val = (1 << 6);
- val |= (1 << 3);
+ sel = 0x1; /* EDTV 480p/576p and VGA */
break;
case
Re: [PATCH v2] media: add support for decoder as one of media entity types
Hi Sylwester,
On Thu, Feb 7, 2013 at 5:18 PM, Sylwester Nawrocki
wrote:
> Hi Prabhakar,
>
> On 01/28/2013 12:50 PM, Prabhakar Lad wrote:
>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>> index 0ef8833..dac06d7 100644
>> --- a/include/uapi/linux/media.h
>> +++ b/include/uapi/linux/media.h
>> @@ -56,6 +56,8 @@ struct media_device_info {
>> #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV + 1)
>> #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH(MEDIA_ENT_T_V4L2_SUBDEV + 2)
>> #define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV + 3)
>> +/* DECODER: Converts analogue video to digital */
>
> The patch looks good to me, I would just change this comment to
> something like:
>
> /* A converter of analogue video to its digital representation. */
>
> But that's really a nitpicking.
>
OK will fix it and post a v3.
Regards,
--Prabhakar
>> +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV + 4)
>>
>> #define MEDIA_ENT_FL_DEFAULT (1 << 0)
>
> Reviewed-by: Sylwester Nawrocki
>
> --
>
> Thanks,
> Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] media: add support for decoder as one of media entity types
From: Manjunath Hadli
A lot of SOCs including Texas Instruments Davinci family mainly use
video decoders as input devices. This patch adds a flag
'MEDIA_ENT_T_V4L2_SUBDEV_DECODER' media entity type for decoder's.
Along side updates the documentation for this media entity type.
Signed-off-by: Manjunath Hadli
Signed-off-by: Lad, Prabhakar
Cc: Rob Landley
Cc: Mauro Carvalho Chehab
Reviewed-by: Sylwester Nawrocki
---
Changes for v3:
1: Fixed Nit pointed by Sylwester.
Changes for v2:
1: Sending as a separate patch.
2: Added documentation for the added media entity type.
3: Improved the commit message.
.../DocBook/media/v4l/media-ioc-enum-entities.xml | 10 ++
include/uapi/linux/media.h |2 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
index 576b68b..116c301 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
@@ -272,6 +272,16 @@
MEDIA_ENT_T_V4L2_SUBDEV_LENS
Lens controller
+
+ MEDIA_ENT_T_V4L2_SUBDEV_DECODER
+ Video decoder, the basic function of the video decoder is to
+ accept analogue video from a wide variety of sources such as
+ broadcast, DVD players, cameras and video cassette recorders, in
+ either NTSC, PAL or HD format and still occasionally SECAM, separate
+ it into its component parts, luminance and chrominance, and output
+ it in some digital video standard, with appropriate embedded timing
+ signals.
+
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 0ef8833..ed49574 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -56,6 +56,8 @@ struct media_device_info {
#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV + 1)
#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV + 2)
#define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV + 3)
+/* A converter of analogue video to its digital representation. */
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER(MEDIA_ENT_T_V4L2_SUBDEV + 4)
#define MEDIA_ENT_FL_DEFAULT (1 << 0)
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Sylwester,
Thanks for the review.
On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki
wrote:
> Hi Prabhakar,
>
> On 01/29/2013 02:07 PM, Prabhakar Lad wrote:
> [...]
>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>> new file mode 100644
>> index 000..55d3ffd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>> @@ -0,0 +1,38 @@
>> +* Texas Instruments TVP514x video decoder
>> +
>> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality,
>> single-chip
>> +digital video decoder that digitizes and decodes all popular baseband
>> analog
>> +video formats into digital video component. The tvp514x decoder supports
>> analog-
>> +to-digital (A/D) conversion of component RGB and YPbPr signals as well as
>> A/D
>> +conversion and decoding of NTSC, PAL and SECAM composite and S-video into
>> +component YCbCr.
>> +
>> +Required Properties :
>> +- compatible: Must be "ti,tvp514x-decoder"
>
>
> There are no significant differences among TVP514* devices as listed above,
> you would like to handle above ?
>
> I'm just wondering if you don't need ,for instance, two separate compatible
> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ?
>
There are few differences in init/power sequence tough, I would still
like to have
single compatible property "ti,tvp514x-decoder", If you feel we need separate
property I will change it please let me know on this.
>
>> +- hsync-active: HSYNC Polarity configuration for current interface.
>> +- vsync-active: VSYNC Polarity configuration for current interface.
>> +- data-active: Clock polarity of the current interface.
>
>
> I guess you mean "pclk-sample: Clock polarity..." ?
>
>
yeah this needs to be 'pclk-sample' property, thanks for pointing it.
>> +
>> +Example:
>> +
>> +i2c0@1c22000 {
>> + ...
>> + ...
>> +
>> + tvp514x@5c {
>> + compatible = "ti,tvp514x-decoder";
>> + reg =<0x5c>;
>> +
>> + port {
>> + tvp514x_1: endpoint {
>> + /* Active high (Defaults to 0) */
>> + hsync-active =<1>;
>> + /* Active high (Defaults to 0) */
>> + hsync-active =<1>;
>
>
> Should it be vsync-active ?
>
Yes, I'll change it.
>
>> + /* Active low (Defaults to 0) */
>> + data-active =<0>;
>
>
> and this pclk-sample ?
>
Yes.
>> + };
>> + };
>> + };
>> + ...
>> +};
>> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
>> index a4f0a70..24ce759 100644
>> --- a/drivers/media/i2c/tvp514x.c
>> +++ b/drivers/media/i2c/tvp514x.c
>> @@ -12,6 +12,7 @@
>>* Hardik Shah
>>* Manjunath Hadli
>>* Karicheri Muralidharan
>> + * Prabhakar Lad
>>*
>>* This package is free software; you can redistribute it and/or modify
>>* it under the terms of the GNU General Public License version 2 as
>> @@ -33,7 +34,9 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -930,6 +933,58 @@ static struct tvp514x_decoder tvp514x_dev = {
>>
>> };
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id tvp514x_of_match[] = {
>> + {.compatible = "ti,tvp514x-decoder", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, tvp514x_of_match);
>> +
>> +static struct tvp514x_platform_data
>> + *tvp514x_get_pdata(struct i2c_client *client)
>> +{
>> + if (!client->dev.platform_data&& client->dev.of_node) {
>>
>> + struct tvp514x_platform_data *pdata;
>> + struct v4l2_of_endpoint bus_cfg;
>> + struct device_node *endpoint;
>> +
>> + pdata = devm_kzalloc(>dev,
>> + sizeof(struct tvp514x_platform_data),
>> + GFP_KERNEL);
>> + client->dev.platform_data = pdata;
>
>
> Do you really need to set client->dev.platform_data this way ?
> What about p
Re: [PATCH v2 1/6] ARM: davinci: da850: fix clock lookup for mdio device
Sekhar,
On Sun, Feb 3, 2013 at 4:44 PM, Sekhar Nori wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar
>>
>> This patch removes the clock alias for mdio device and adds a entry
>> in clock lookup table, this entry can now be used by both DT and NON
>> DT case.
>>
>> Signed-off-by: Lad, Prabhakar
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-sou...@linux.davincidsp.com
>> Cc: net...@vger.kernel.org
>> Cc: devicetree-disc...@lists.ozlabs.org
>> Cc: Sekhar Nori
>> Cc: Heiko Schocher
>> ---
>> arch/arm/mach-davinci/da850.c |1 +
>> arch/arm/mach-davinci/devices-da8xx.c |8 ++--
>> 2 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index 86056ca..f74bfb6 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -402,6 +402,7 @@ static struct clk_lookup da850_clks[] = {
>> CLK(NULL, "arm", _clk),
>> CLK(NULL, "rmii", _clk),
>> CLK("davinci_emac.1", NULL, _clk),
>> + CLK("davinci_mdio.0", "fck", _clk),
>> CLK("davinci-mcasp.0", NULL, _clk),
>> CLK("da8xx_lcdc.0", "fck", _clk),
>> CLK("davinci_mmc.0",NULL, _clk),
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c
>> b/arch/arm/mach-davinci/devices-da8xx.c
>> index 2d5502d..52faa05 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -444,12 +444,8 @@ int __init da8xx_register_emac(void)
>> ret = platform_device_register(_mdio_device);
>> if (ret < 0)
>> return ret;
>> - ret = platform_device_register(_emac_device);
>> - if (ret < 0)
>> - return ret;
>> - ret = clk_add_alias(NULL, dev_name(_mdio_device.dev),
>> - NULL, _emac_device.dev);
>> - return ret;
>> +
>> + return platform_device_register(_emac_device);
>
> You added the look-up only for DA850 which means after this patch da830
> network will be broken. Did you test on DA830?
Ah yes my bad this will break network on da830, I'll fix it and respin
this patch
alone only, if there aren't any comments for the rest of the series.
Regards,
--Prabhakar
> Thanks,
> Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/6] ARM: davinci: da850: add DT node for mdio device
Sekhar,
On Sun, Feb 3, 2013 at 5:23 PM, Sekhar Nori wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar
>>
>> Add mdio device tree node information to da850 by
>> providing register details and bus frequency of mdio.
>>
>> Signed-off-by: Lad, Prabhakar
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-sou...@linux.davincidsp.com
>> Cc: net...@vger.kernel.org
>> Cc: devicetree-disc...@lists.ozlabs.org
>> Cc: Sekhar Nori
>> Cc: Heiko Schocher
>> ---
>> arch/arm/boot/dts/da850-evm.dts |3 +++
>> arch/arm/boot/dts/da850.dtsi|7 +++
>> 2 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/da850-evm.dts
>> b/arch/arm/boot/dts/da850-evm.dts
>> index 98c1a48..a319491 100644
>> --- a/arch/arm/boot/dts/da850-evm.dts
>> +++ b/arch/arm/boot/dts/da850-evm.dts
>> @@ -27,6 +27,9 @@
>> serial2: serial@1d0d000 {
>> status = "okay";
>> };
>> + mdio: davinci_mdio@1e24000 {
>
> This should just be:
>
> mdio: mdio@1e24000
>
> inline with how we avoid adding davinci prefix to each node name.
>
Ok
>> + status = "okay";
>> + };
>> };
>> nand_cs3@6200 {
>> status = "okay";
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 7c84822..ba28f2d 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -81,6 +81,13 @@
>> interrupts = <61>;
>> status = "disabled";
>> };
>> + mdio: davinci_mdio@1e24000 {
>> + compatible = "ti,davinci_mdio";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0x224000 0x1000>;
>> + bus_freq = <220>;
>
> The bus frequency is board specific and it is better this is defined in
> da850-evm.dts
>
Ok I'll move this to dts file.
Regards,
--Prabhakar
> Thanks,
> Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v2] media: tvp514x: add OF support
Sekhar, On Sun, Feb 3, 2013 at 6:40 PM, Sekhar Nori wrote: > On 2/3/2013 3:43 PM, Prabhakar Lad wrote: >> Hi Sylwester, >> >> Thanks for the review. >> >> On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki >> wrote: >>> Hi Prabhakar, >>> >>> On 01/29/2013 02:07 PM, Prabhakar Lad wrote: >>> [...] >>> >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >>>> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >>>> new file mode 100644 >>>> index 000..55d3ffd >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >>>> @@ -0,0 +1,38 @@ >>>> +* Texas Instruments TVP514x video decoder >>>> + >>>> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, >>>> single-chip >>>> +digital video decoder that digitizes and decodes all popular baseband >>>> analog >>>> +video formats into digital video component. The tvp514x decoder supports >>>> analog- >>>> +to-digital (A/D) conversion of component RGB and YPbPr signals as well as >>>> A/D >>>> +conversion and decoding of NTSC, PAL and SECAM composite and S-video into >>>> +component YCbCr. >>>> + >>>> +Required Properties : >>>> +- compatible: Must be "ti,tvp514x-decoder" >>> >>> >>> There are no significant differences among TVP514* devices as listed above, >>> you would like to handle above ? >>> >>> I'm just wondering if you don't need ,for instance, two separate compatible >>> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ? >>> >> There are few differences in init/power sequence tough, I would still >> like to have >> single compatible property "ti,tvp514x-decoder", If you feel we need separate >> property I will change it please let me know on this. > > Compatible properties should not use generic part numbers. See one past > discussion here: http://en.usenet.digipedia.org/thread/18472/20788/ > Thanks for the link, I'll have separate compatible properties. Regards, --Prabhakar > Thanks, > Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Laurent, On Sun, Feb 3, 2013 at 6:44 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > On Sunday 03 February 2013 15:43:49 Prabhakar Lad wrote: >> On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki wrote: >> > On 01/29/2013 02:07 PM, Prabhakar Lad wrote: >> > [...] >> > >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >> >> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >> >> new file mode 100644 >> >> index 000..55d3ffd >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >> >> @@ -0,0 +1,38 @@ >> >> +* Texas Instruments TVP514x video decoder >> >> + >> >> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, >> >> +single-chip digital video decoder that digitizes and decodes all popular >> >> +baseband analog video formats into digital video component. The tvp514x >> >> +decoder supports analog-to-digital (A/D) conversion of component RGB and >> >> +YPbPr signals as well as A/D conversion and decoding of NTSC, PAL and >> >> +SECAM composite and S-video into component YCbCr. >> >> + >> >> +Required Properties : >> >> +- compatible: Must be "ti,tvp514x-decoder" >> > >> > There are no significant differences among TVP514* devices as listed >> > above, you would like to handle above ? >> > >> > I'm just wondering if you don't need, for instance, two separate >> > compatible properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" >> > ? >> >> There are few differences in init/power sequence tough, I would still >> like to have single compatible property "ti,tvp514x-decoder", If you feel we >> need separate property I will change it please let me know on this. > > If there's any difference between the chips that need to be handled in the > driver, using two compatible properties is a good practice. Your driver will > then be able to easily differentiate between the two chips, and there's no > real drawback in doing so. > Ok I'll will have separate compatible properties. Regards, --Prabhakar > -- > Regards, > > Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Sylwester,
On Sun, Feb 3, 2013 at 6:57 PM, Sylwester Nawrocki
wrote:
> Hi Prabhakar,
>
> On 02/03/2013 11:13 AM, Prabhakar Lad wrote:
> [...]
>
>>>> +Required Properties :
>>>> +- compatible: Must be "ti,tvp514x-decoder"
>>>
>>>
>>>
>>> There are no significant differences among TVP514* devices as listed
>>> above,
>>> you would like to handle above ?
>
>
> Sorry for the mangled sentence, I intended to write "in the driver" instead
> of the last "above".
>
>
Not a problem I got what you intended :)
>>> I'm just wondering if you don't need ,for instance, two separate
>>> compatible
>>> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ?
>>>
>> There are few differences in init/power sequence tough, I would still
>> like to have
>> single compatible property "ti,tvp514x-decoder", If you feel we need
>> separate
>> property I will change it please let me know on this.
>
>
> As Sekhar already mentioned, wildcards in the compatible property should
> not be used. You could just use exact part names in the compatible
> properties and list them all in the tvp514x_of_match[] array. Even though
> this driver doesn't care about the differences between various tvp514?
> chips, there might be others that do.
>
> [...]
>
Ok, I'll have separate compatible properties.
>>>> +#if defined(CONFIG_OF)
>>>> +static const struct of_device_id tvp514x_of_match[] = {
>>>> + {.compatible = "ti,tvp514x-decoder", },
>>>> + {},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, tvp514x_of_match);
>>>> +
>>>> +static struct tvp514x_platform_data
>>>> + *tvp514x_get_pdata(struct i2c_client *client)
>>>> +{
>>>> + if (!client->dev.platform_data&& client->dev.of_node) {
>>>>
>>>> + struct tvp514x_platform_data *pdata;
>>>> + struct v4l2_of_endpoint bus_cfg;
>>>> + struct device_node *endpoint;
>>>> +
>>>> + pdata = devm_kzalloc(>dev,
>>>> + sizeof(struct tvp514x_platform_data),
>>>> + GFP_KERNEL);
>>>> + client->dev.platform_data = pdata;
>>>
>>>
>>>
>>> Do you really need to set client->dev.platform_data this way ?
>>> What about passing struct tvp514x_decoder pointer to this function
>>> and initializing struct tvp514x_decoder::pdata instead ?
>>>
>>>
>> Yes that can work too, I'll do the same.
>
>
> Ok, thanks.
>
>
>>>> + if (!pdata)
>>>> + return NULL;
>>>> +
>>>> + endpoint = of_get_child_by_name(client->dev.of_node,
>>>> "port");
>>>> + if (endpoint)
>>>> + endpoint = of_get_child_by_name(endpoint,
>>>> "endpoint");
>>>
>>>
>>>
>>> I think you could replace these two calls above with
>>>
>>> endpoint =
>>> v4l2_of_get_next_endpoint(client->dev.of_node);
>>>
>> Ok
>>
>>> Now I see I should have modified this function to ensure it works also
>>> when
>>> 'port' nodes are grouped in a 'ports' node.
>>>
>> So V5 series of V4l OF parser doesn't have this fix ?
>
>
> No, it doesn't. I think we need something along the lines of:
>
> diff --git a/drivers/media/v4l2-core/v4l2-of.c
> b/drivers/media/v4l2-core/v4l2-of.c
> index e1f570b..8a286f1 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -185,10 +185,15 @@ struct device_node *v4l2_of_get_next_endpoint(const
> struct device_node *parent,
> * This is the first call, we have to find a port within
> * this node.
> */
> - for_each_child_of_node(parent, port) {
> + while (port = of_get_next_child(parent, port)) {
> if (!of_node_cmp(port->name, "port"))
> break;
> - }
> + if (!of_node_cmp(port->name, "ports")) {
> + parent = port;
> + of_node_put(port);
> + port = NULL:
> + }
> + };
> if (port) {
> /* Found a port, get an endpoint. */
> endpoint = of_get_next_child(port, NULL);
>
> However this shouldn't affect you, as you don't use the 'ports' node...
> I will likely post v6 including this fix tomorrow.
>
yes I don't need it, Just asked so that I can test my driver on latest
version :)
Regards,
--Prabhakar
> --
>
> Regards,
> Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] ARM: davinci: da850: configure system configuration chip(CFGCHIP3) for emac
Sekhar ,
On Sun, Feb 3, 2013 at 6:15 PM, Sekhar Nori wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar
>>
>> The system configuration chip CFGCHIP3, controls the emac module.
>> This patch appropriately configures this register for emac and
>> sets DA850_MII_MDIO_CLKEN_PIN GPIO pin appropriately.
>>
>> Signed-off-by: Lad, Prabhakar
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-sou...@linux.davincidsp.com
>> Cc: net...@vger.kernel.org
>> Cc: devicetree-disc...@lists.ozlabs.org
>> Cc: Sekhar Nori
>> Cc: Heiko Schocher
>> ---
>> arch/arm/mach-davinci/da8xx-dt.c | 28
>> 1 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/da8xx-dt.c
>> b/arch/arm/mach-davinci/da8xx-dt.c
>> index e533a0a..4a096e3 100644
>> --- a/arch/arm/mach-davinci/da8xx-dt.c
>> +++ b/arch/arm/mach-davinci/da8xx-dt.c
>> @@ -8,6 +8,7 @@
>> * published by the Free Software Foundation.
>> */
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -39,6 +40,32 @@ static void __init da8xx_init_irq(void)
>>
>> #ifdef CONFIG_ARCH_DAVINCI_DA850
>>
>> +static void __init da8xx_config_emac(void)
>> +{
>> +#define DA850_MII_MDIO_CLKEN_PIN GPIO_TO_PIN(2, 6)
>> +#define DA850_EMAC_MODE_SELECT BIT(8)
>> + void __iomem *cfg_chip3_base;
>> + int ret;
>> + u32 val;
>> +
>> + cfg_chip3_base = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
>> +
>> + val = __raw_readl(cfg_chip3_base);
>> + val &= ~DA850_EMAC_MODE_SELECT;
>> + /* configure the CFGCHIP3 register for MII */
>> + __raw_writel(val, cfg_chip3_base);
>
> Use readl/writel instead.
>
Ok.
>> + pr_info("EMAC: MII PHY configured\n");
>> +
>> + ret = gpio_request(DA850_MII_MDIO_CLKEN_PIN, "mdio_clk_en");
>> + if (ret) {
>> + pr_warn("Cannot open GPIO %d\n",
>> + DA850_MII_MDIO_CLKEN_PIN);
>> + return;
>> + }
>> + /* Enable/Disable MII MDIO clock */
>> + gpio_direction_output(DA850_MII_MDIO_CLKEN_PIN, 0);
>> +}
>> +
>> struct of_dev_auxdata da8xx_auxdata[] __initdata = {
>> OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
>> OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e2, "davinci_emac.1",
>> @@ -52,6 +79,7 @@ static void __init da850_init_machine(void)
>>da8xx_auxdata, NULL);
>>
>> da8xx_uart_clk_enable();
>> + da8xx_config_emac();
>
> There are couple of issues with this implementation.
>
> 1) da8xx_config_emac() is specific to DA850 EVM, but masquerades as
>generic for da8xx. Looks like you need two functions, one for soc
>specific configuration and one board specific.
Ok.
> 2) da8xx_config_emac() goes through all the time, whether the
>particular board has emac module or not. Shouldn't
>da8xx_config_emac() check if emac is actually enabled in the passed
>dtb and only the do the configuration?
Ok. Can you give some pointers how we can access the dtb in board files.
> 3) The function assumes mii is used always, you can use the rmii_en dt
>property to check if rmii/mii is enabled and configure the soc/board
>accordingly.
Ok.
> 4) If the same function can work both for da850 and da830, then it can
>be implemented outside of CONFIG_ARCH_DAVINCI_DA850.
>
Ok.
Regards,
--Prabhakar
> Thanks,
> Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/6] ARM: davinci: da850: add DT node for eth0.
Sekhar ,
On Sun, Feb 3, 2013 at 5:33 PM, Sekhar Nori wrote:
> On 1/28/2013 7:17 PM, Prabhakar Lad wrote:
>> From: Lad, Prabhakar
>>
>> Add eth0 device tree node information and pinmux for mii to da850 by
>> providing interrupt details and local mac address of eth0.
>>
>> Signed-off-by: Lad, Prabhakar
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-sou...@linux.davincidsp.com
>> Cc: net...@vger.kernel.org
>> Cc: devicetree-disc...@lists.ozlabs.org
>> Cc: Sekhar Nori
>> Cc: Heiko Schocher
>> ---
>> arch/arm/boot/dts/da850-evm.dts |5 +
>> arch/arm/boot/dts/da850.dtsi| 35 +++
>> 2 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/da850-evm.dts
>> b/arch/arm/boot/dts/da850-evm.dts
>> index a319491..19aa2b3 100644
>> --- a/arch/arm/boot/dts/da850-evm.dts
>> +++ b/arch/arm/boot/dts/da850-evm.dts
>> @@ -30,6 +30,11 @@
>> mdio: davinci_mdio@1e24000 {
>> status = "okay";
>> };
>> + eth0: emac@1e2 {
>> + status = "okay";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <_pins>;
>> + };
>> };
>> nand_cs3@6200 {
>> status = "okay";
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index ba28f2d..76905f3 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -56,6 +56,26 @@
>> 0x30 0x0110 0x0ff0
>> >;
>> };
>> + mii_pins: pinmux_mii_pins {
>> + pinctrl-single,bits = <
>> + /*
>> + * MII_TXEN, MII_TXCLK, MII_COL
>> + * MII_TXD_3, MII_TXD_2, MII_TXD_1
>> + * MII_TXD_0
>> + */
>> + 0x8 0x8880 0xfff0
>> + /*
>> + * MII_RXER, MII_CRS, MII_RXCLK
>> + * MII_RXDV, MII_RXD_3, MII_RXD_2
>> + * MII_RXD_1, MII_RXD_0
>> + */
>> + 0xc 0x 0x
>> + /* MDIO_CLK, MDIO_D */
>
> You call this mii_pins, but include mdio pins in there as well. Can you
> separate them out? Then some board which uses rmii can simply reuse the
> entry.
>
Ok makes sense.
>> + 0x10 0x0088 0x00ff
>> + /* GPIO2_6 */
>> + 0x18 0x0080 0x00f0
>
> This is SoC specific pin list. Such board specific pins should not make
> it here.
>
Ok, so this should be set up using GPIO API's ?
>> + >;
>> + };
>> };
>> serial0: serial@1c42000 {
>> compatible = "ns16550a";
>> @@ -88,6 +108,21 @@
>> reg = <0x224000 0x1000>;
>> bus_freq = <220>;
>> };
>> + eth0: emac@1e2 {
>> + compatible = "ti,davinci-dm6467-emac";
>> + reg = <0x22 0x4000>;
>> + ti,davinci-ctrl-reg-offset = <0x3000>;
>> + ti,davinci-ctrl-mod-reg-offset = <0x2000>;
>> + ti,davinci-ctrl-ram-offset = <0>;
>> + ti,davinci-ctrl-ram-size = <0x2000>;
>> + local-mac-address = [ 00 00 00 00 00 00 ];
>> + interrupts = <33
>> + 34
>> + 35
>> + 36
>> + >;
>> + phy-handle = <>;
>
> I doubt this is required. This property is to pass a handle to the phy,
> not mdio bus.
>
Ok I'll check on this.
Regards,
--Prabhakar
> Thanks,
> Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] media: davinci: vpif: align the buffers size to page page size boundary
Hi Marek, On Tue, Apr 16, 2013 at 4:48 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > (CC'ing Marek) > > On Tuesday 16 April 2013 16:24:30 Prabhakar lad wrote: >> From: Lad, Prabhakar >> >> with recent commit with id 068a0df76023926af958a336a78bef60468d2033 >> which adds add length check for mmap, the application were failing to >> mmap the buffers. >> >> This patch aligns the the buffer size to page size boundary for both >> capture and display driver so the it pass the check. >> >> Signed-off-by: Lad, Prabhakar >> Cc: Laurent Pinchart >> Cc: Hans Verkuil >> Cc: Mauro Carvalho Chehab >> --- >> Changes for v2: >> 1: Fixed a typo in commit message. >> >> drivers/media/platform/davinci/vpif_capture.c |1 + >> drivers/media/platform/davinci/vpif_display.c |1 + >> 2 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c >> b/drivers/media/platform/davinci/vpif_capture.c index 5f98df1..25981d6 >> 100644 >> --- a/drivers/media/platform/davinci/vpif_capture.c >> +++ b/drivers/media/platform/davinci/vpif_capture.c >> @@ -183,6 +183,7 @@ static int vpif_buffer_queue_setup(struct vb2_queue *vq, >> *nbuffers = config_params.min_numbuffers; >> >> *nplanes = 1; >> + size = PAGE_ALIGN(size); > > I wonder if that's the best fix. > > The queue_setup operation is supposed to return the size required by the > driver for each plane. Depending on the hardware requirements, that size might > not be a multiple of the page size. > > As we can't mmap() a fraction of a page, the allocated plane size needs to be > rounded up to the next page boundary to allow mmap() support. The dma-contig > and dma-sg allocators already do so in their alloc operation, but the vmalloc > allocator doesn't. > > The recent "media: vb2: add length check for mmap" patch verifies that the > mmap() size requested by userspace doesn't exceed the buffer size. As the > mmap() size is rounded up to the next page boundary the check will fail for > buffer sizes that are not multiple of the page size. > > Your fix will not result in overallocation (as the allocator already rounds > the size up), but will prevent the driver from importing a buffer large enough > for the hardware but not rounded up to the page size. > > A better fix might be to round up the buffer size in the buffer size check at > mmap() time, and fix the vmalloc allocator to round up the size. That the > allocator, not drivers, is responsible for buffer size alignment should be > documented in videobuf2-core.h. > Do you plan to post a patch fixing it as per Laurent's suggestion ? Regards, --Prabhakar >> sizes[0] = size; >> alloc_ctxs[0] = common->alloc_ctx; >> >> diff --git a/drivers/media/platform/davinci/vpif_display.c >> b/drivers/media/platform/davinci/vpif_display.c index 1b3fb5c..3414715 >> 100644 >> --- a/drivers/media/platform/davinci/vpif_display.c >> +++ b/drivers/media/platform/davinci/vpif_display.c >> @@ -162,6 +162,7 @@ static int vpif_buffer_queue_setup(struct vb2_queue *vq, >> *nbuffers = config_params.min_numbuffers; >> >> *nplanes = 1; >> + size = PAGE_ALIGN(size); >> sizes[0] = size; >> alloc_ctxs[0] = common->alloc_ctx; >> return 0; > > -- > Regards, > > Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] media: videobuf2: fix the length check for mmap
From: Lad, Prabhakar
>From commit 068a0df76023926af958a336a78bef60468d2033
"[media] media: vb2: add length check for mmap"
patch verifies that the mmap() size requested by userspace
doesn't exceed the buffer size.
As the mmap() size is rounded up to the next page boundary
the check will fail for buffer sizes that are not multiple
of the page size.
This patch fixes the check by aligning the buffer size to page
size during the check. Alongside fixes the vmalloc allocator
to round up the size.
Signed-off-by: Lad, Prabhakar
Cc: Laurent Pinchart
Cc: Marek Szyprowski
Cc: Seung-Woo Kim
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/videobuf2-core.c|2 +-
drivers/media/v4l2-core/videobuf2-vmalloc.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 58c1744..223fcd4 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1886,7 +1886,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct
*vma)
vb = q->bufs[buffer];
- if (vb->v4l2_planes[plane].length < (vma->vm_end - vma->vm_start)) {
+ if (PAGE_ALIGN(vb->v4l2_planes[plane].length) < (vma->vm_end -
vma->vm_start)) {
dprintk(1, "Invalid length\n");
return -EINVAL;
}
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 313d977..bf3b95c 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -44,7 +44,7 @@ static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long
size, gfp_t gfp_fl
return NULL;
buf->size = size;
- buf->vaddr = vmalloc_user(buf->size);
+ buf->vaddr = vmalloc_user(PAGE_ALIGN(buf->size));
buf->handler.refcount = >refcount;
buf->handler.put = vb2_vmalloc_put;
buf->handler.arg = buf;
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] media: davinci: vpif trivial cleanup
From: Lad, Prabhakar The first patch of the series cleanups the header file inclusion, and second patch fixes displaying of error when there is actual failure of request_irq(). Lad, Prabhakar (2): media: davinci: vpif: remove unwanted header file inclusion media: davinci: vpif_display: move displaying of error to approppraite place drivers/media/platform/davinci/vpif_capture.c | 19 --- drivers/media/platform/davinci/vpif_capture.h |5 + drivers/media/platform/davinci/vpif_display.c | 25 - drivers/media/platform/davinci/vpif_display.h |5 + 4 files changed, 10 insertions(+), 44 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] media: davinci: vpif: remove unwanted header file inclusion
From: Lad, Prabhakar
this patch removes unwanted header file inclusion and sorts
header alphabetically.
Signed-off-by: Lad, Prabhakar
---
drivers/media/platform/davinci/vpif_capture.c | 19 ---
drivers/media/platform/davinci/vpif_capture.h |5 +
drivers/media/platform/davinci/vpif_display.c | 23 +++
drivers/media/platform/davinci/vpif_display.h |5 +
4 files changed, 9 insertions(+), 43 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index 5f98df1..a1b42b0 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -18,28 +18,17 @@
* TODO : add support for VBI & HBI data service
* add static buffer allocation
*/
-#include
-#include
+
#include
-#include
-#include
-#include
#include
-#include
-#include
-#include
-#include
-#include
-#include
#include
-#include
#include
-#include
-#include
+
#include
+#include
-#include "vpif_capture.h"
#include "vpif.h"
+#include "vpif_capture.h"
MODULE_DESCRIPTION("TI DaVinci VPIF Capture driver");
MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/davinci/vpif_capture.h
b/drivers/media/platform/davinci/vpif_capture.h
index 3d3c1e5..0ebb312 100644
--- a/drivers/media/platform/davinci/vpif_capture.h
+++ b/drivers/media/platform/davinci/vpif_capture.h
@@ -22,11 +22,8 @@
#ifdef __KERNEL__
/* Header files */
-#include
-#include
-#include
#include
-#include
+#include
#include "vpif.h"
diff --git a/drivers/media/platform/davinci/vpif_display.c
b/drivers/media/platform/davinci/vpif_display.c
index 1b3fb5c..d833056 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -14,33 +14,16 @@
* GNU General Public License for more details.
*/
-#include
-#include
-#include
-#include
-#include
-#include
#include
-#include
-#include
-#include
-#include
-#include
-#include
+#include
#include
-#include
#include
-#include
-#include
-
-#include
-#include
-#include
#include
+#include
-#include "vpif_display.h"
#include "vpif.h"
+#include "vpif_display.h"
MODULE_DESCRIPTION("TI DaVinci VPIF Display driver");
MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/davinci/vpif_display.h
b/drivers/media/platform/davinci/vpif_display.h
index a5a18f7..5d87fc8 100644
--- a/drivers/media/platform/davinci/vpif_display.h
+++ b/drivers/media/platform/davinci/vpif_display.h
@@ -17,11 +17,8 @@
#define DAVINCIHD_DISPLAY_H
/* Header files */
-#include
-#include
-#include
#include
-#include
+#include
#include "vpif.h"
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] media: davinci: vpif_display: move displaying of error to approppraite place
From: Lad, Prabhakar
this patch moves the displaying out error case "VPIF IRQ request failed\n"
when there is actual request_irq() fail.
Signed-off-by: Lad, Prabhakar
---
drivers/media/platform/davinci/vpif_display.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif_display.c
b/drivers/media/platform/davinci/vpif_display.c
index d833056..7b17368 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1725,6 +1725,7 @@ static __init int vpif_probe(struct platform_device *pdev)
for (j = 0; j < i; j++)
free_irq(j, (void *)
(_obj.dev[res_idx]->channel_id));
+ vpif_err("VPIF IRQ request failed\n");
goto vpif_int_err;
}
}
@@ -1878,7 +1879,6 @@ vpif_sd_error:
}
vpif_int_err:
v4l2_device_unregister(_obj.v4l2_dev);
- vpif_err("VPIF IRQ request failed\n");
for (i = 0; i < res_idx; i++) {
res = platform_get_resource(pdev, IORESOURCE_IRQ, i);
for (j = res->start; j <= res->end; j++)
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] media: videobuf2: fix the length check for mmap
On Fri, Apr 19, 2013 at 4:48 PM, Mauro Carvalho Chehab
wrote:
> Em Fri, 19 Apr 2013 15:16:56 +0530
> Prabhakar lad escreveu:
>
>> From: Lad, Prabhakar
>>
>> From commit 068a0df76023926af958a336a78bef60468d2033
>> "[media] media: vb2: add length check for mmap"
>> patch verifies that the mmap() size requested by userspace
>> doesn't exceed the buffer size.
>>
>> As the mmap() size is rounded up to the next page boundary
>> the check will fail for buffer sizes that are not multiple
>> of the page size.
>>
>> This patch fixes the check by aligning the buffer size to page
>> size during the check. Alongside fixes the vmalloc allocator
>> to round up the size.
>>
>> Signed-off-by: Lad, Prabhakar
>> Cc: Laurent Pinchart
>> Cc: Marek Szyprowski
>> Cc: Seung-Woo Kim
>> Cc: Hans Verkuil
>> Cc: Mauro Carvalho Chehab
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c|2 +-
>> drivers/media/v4l2-core/videobuf2-vmalloc.c |2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 58c1744..223fcd4 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1886,7 +1886,7 @@ int vb2_mmap(struct vb2_queue *q, struct
>> vm_area_struct *vma)
>>
>> vb = q->bufs[buffer];
>>
>> - if (vb->v4l2_planes[plane].length < (vma->vm_end - vma->vm_start)) {
>> + if (PAGE_ALIGN(vb->v4l2_planes[plane].length) < (vma->vm_end -
>> vma->vm_start)) {
>> dprintk(1, "Invalid length\n");
>> return -EINVAL;
>> }
>
> That is tricky, as it assumes that vb->v4l2_planes[plane].length was round
> up to PAGE_SIZE at each memops driver, but the vb2 core doesn't enforce it.
>
> IMO, it would be cleaner to round vb->v4l2_planes[plane].length up
> at VB2 core, before calling the memops alloc functions at the drivers.
>
> Also, VB2 is already complex enough to put it there without proper
> comments (and there's a minor codingstyle issue there: line is bigger
> than 80 cols).
>
>> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
>> b/drivers/media/v4l2-core/videobuf2-vmalloc.c
>> index 313d977..bf3b95c 100644
>> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
>> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
>> @@ -44,7 +44,7 @@ static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned
>> long size, gfp_t gfp_fl
>> return NULL;
>>
>> buf->size = size;
>> - buf->vaddr = vmalloc_user(buf->size);
>> + buf->vaddr = vmalloc_user(PAGE_ALIGN(buf->size));
>
> See? You needed to put an alignment here as well, not because vmalloc
> needs it, but because this is needed by VB2 core.
>
> Also, on the other drivers, buf->size is stored page aligned, while
> here, you're doing different, without any documented reason for doing
> that, instead of doing the same as on the other memops drivers.
>
> That mistake reflects, for example, when the driver prints the failure:
>
> if (!buf->vaddr) {
> pr_debug("vmalloc of size %ld failed\n", buf->size);
>
> as it will show a different size than what you actually required.
> As those memory starving errors can also produce a dump at the mm
> core, the size there won't match the size on the above printed message.
>
> Also, it is a very bad idea to delegate the core's requirement of
> do page alignment from the core to the memops drivers, as other
> patches may change the logic there, or a new memops could be added,
> and the same problem will hit again (and unnoticed, as the check
> routine do page alignments).
>
>> buf->handler.refcount = >refcount;
>> buf->handler.put = vb2_vmalloc_put;
>> buf->handler.arg = buf;
>
> IMO, a cleaner version would be the following (untested) code.
>
> -
>
> [media] videobuf2: fix the length check for mmap
>
> Memory maps typically require that the buffer size to be page
> aligned. Currently, two memops drivers do such alignment
> internally, but videobuf-vmalloc doesn't.
>
> Also, the buffer overflow check doesn't take it into account.
>
> So, instead of doing it at each memops driver, enforce it at
> VB2 core.
>
> Reported-by: Prabhakar lad
> Signed-off-by: Mauro Carvalho Chehab
>
Reported-and-tested-by: Lad, Prabhakar
Regards,
--Prabhakar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
