Re: [PATCH v4 1/2] [media] Add Rockchip RK1608 driver
Hi Leo,
Some more review comments below:
On 08/03/18 07:38, Wen Nuan wrote:
> From: Leo Wen
>
> Rk1608 is used as a PreISP to link on Soc, which mainly has two functions.
> One is to download the firmware of RK1608, and the other is to match the
> extra sensor such as camera and enable sensor by calling sensor's s_power.
>
> use below v4l2-ctl command to capture frames.
>
> v4l2-ctl --verbose -d /dev/video1 --stream-mmap=2
> --stream-to=/tmp/stream.out --stream-count=60 --stream-poll
>
> use below command to playback the video on your PC.
>
> mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
> w=640:h=480:size=$((640*480*3/2)):format=NV12
>
> Changes V4:
> - Request the GPIO as input with the flag GPIOD_IN.
> - Request the GPIO as output with the flag GPIOD_OUT_LOW.
> - Delete these redundant functions(gpiod_direction_input..etc.).
>
> Signed-off-by: Leo Wen
> ---
> MAINTAINERS|6 +
> drivers/media/spi/Makefile |1 +
> drivers/media/spi/rk1608.c | 1374
>
> drivers/media/spi/rk1608.h | 442 ++
> 4 files changed, 1823 insertions(+)
> create mode 100644 drivers/media/spi/rk1608.c
> create mode 100644 drivers/media/spi/rk1608.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 93a12af..b2a98e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -136,6 +136,12 @@ Maintainers List (try to look for most precise areas
> first)
>
> ---
>
> +ROCKCHIP RK1608 DRIVER
> +M: Leo Wen
> +S: Maintained
> +F: drivers/media/spi/rk1608.c
> +F: drivers/media/spi/rk1608.h
> +
> 3C59X NETWORK DRIVER
> M: Steffen Klassert
> L: net...@vger.kernel.org
> diff --git a/drivers/media/spi/Makefile b/drivers/media/spi/Makefile
> index ea64013..6d0e6ee 100644
> --- a/drivers/media/spi/Makefile
> +++ b/drivers/media/spi/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
> +obj-$(CONFIG_VIDEO_RK1608) += rk1608.o
> diff --git a/drivers/media/spi/rk1608.c b/drivers/media/spi/rk1608.c
> new file mode 100644
> index 000..fedcd0b
> --- /dev/null
> +++ b/drivers/media/spi/rk1608.c
> @@ -0,0 +1,1374 @@
> +/**
> + * Rockchip rk1608 driver
> + *
> + * SPDX-License-Identifier: GPL-2.0
This doesn't follow the SPDX guidelines.
This should be a single line starting with '//' at the beginning of the source.
See e.g. drivers/media/cec/cec-pin.c
You probably also want to use GPL-2.0-only.
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "rk1608.h"
> +
> +/**
> + * Rk1608 is used as the Pre-ISP to link on Soc, which mainly has two
> + * functions. One is to download the firmware of RK1608, and the other
> + * is to match the extra sensor such as camera and enable sensor by
> + * calling sensor's s_power.
> + * |---|
> + * | Sensor Camera |
> + * |---|
> + * |---||--|
> + * |---||--|
> + * |---\/--|
> + * | Pre-ISP RK1608|
> + * |---|
> + * |---||--|
> + * |---||--|
> + * |---\/--|
> + * | Rockchip Soc |
> + * |---|
> + * Data Transfer As shown above. In RK1608, the data received from the
> + * extra sensor,and it is passed to the Soc through ISP.
> + */
> +
> +static inline struct rk1608_state *to_state(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct rk1608_state, sd);
> +}
> +
> +/**
> + * rk1608_operation_query - RK1608 last operation state query
> + *
> + * @spi: device from which data will be read
> + * @state: last operation state [out]
> + * Context: can sleep
> + *
> + * It returns zero on success, else a negative error code.
> + */
> +static int rk1608_operation_query(struct spi_device *spi, s32 *state)
> +{
> + s32 query_cmd = RK1608_CMD_QUERY;
> + struct spi_transfer query_cmd_packet = {
> + .tx_buf = &query_cmd,
> + .len= sizeof(query_cmd),
> + };
> + struct spi_transfer state_packet = {
> + .rx_buf = state,
> + .len= sizeof(*state),
> + };
> + struct spi_message m;
> +
> + spi_message_init(&m);
> + spi_message_add_tail(&query_cmd_packet, &m);
> + spi_message_add_tail(&state_packet, &m);
> + spi_sync(spi, &m);
> +
> + return ((*state & RK1608_STATE_ID_MASK) == RK1608_STATE_ID) ? 0 : -1;
> +}
> +
> +static int rk1608_write(struct spi_device *spi,
> + s32 addr, const s32 *data, size_t data_len)
> +{
> + s32 write_cmd = RK1608_CMD_WRITE;
> + struct spi_transfer write_cmd_packet = {
> + .tx_buf = &write_cmd,
> + .len= sizeof(write_cmd),
> + };
> + struct spi
[PATCH v4 1/2] [media] Add Rockchip RK1608 driver
From: Leo Wen
Rk1608 is used as a PreISP to link on Soc, which mainly has two functions.
One is to download the firmware of RK1608, and the other is to match the
extra sensor such as camera and enable sensor by calling sensor's s_power.
use below v4l2-ctl command to capture frames.
v4l2-ctl --verbose -d /dev/video1 --stream-mmap=2
--stream-to=/tmp/stream.out --stream-count=60 --stream-poll
use below command to playback the video on your PC.
mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
w=640:h=480:size=$((640*480*3/2)):format=NV12
Changes V4:
- Request the GPIO as input with the flag GPIOD_IN.
- Request the GPIO as output with the flag GPIOD_OUT_LOW.
- Delete these redundant functions(gpiod_direction_input..etc.).
Signed-off-by: Leo Wen
---
MAINTAINERS|6 +
drivers/media/spi/Makefile |1 +
drivers/media/spi/rk1608.c | 1374
drivers/media/spi/rk1608.h | 442 ++
4 files changed, 1823 insertions(+)
create mode 100644 drivers/media/spi/rk1608.c
create mode 100644 drivers/media/spi/rk1608.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 93a12af..b2a98e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -136,6 +136,12 @@ Maintainers List (try to look for most precise areas first)
---
+ROCKCHIP RK1608 DRIVER
+M: Leo Wen
+S: Maintained
+F: drivers/media/spi/rk1608.c
+F: drivers/media/spi/rk1608.h
+
3C59X NETWORK DRIVER
M: Steffen Klassert
L: net...@vger.kernel.org
diff --git a/drivers/media/spi/Makefile b/drivers/media/spi/Makefile
index ea64013..6d0e6ee 100644
--- a/drivers/media/spi/Makefile
+++ b/drivers/media/spi/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
+obj-$(CONFIG_VIDEO_RK1608) += rk1608.o
diff --git a/drivers/media/spi/rk1608.c b/drivers/media/spi/rk1608.c
new file mode 100644
index 000..fedcd0b
--- /dev/null
+++ b/drivers/media/spi/rk1608.c
@@ -0,0 +1,1374 @@
+/**
+ * Rockchip rk1608 driver
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "rk1608.h"
+
+/**
+ * Rk1608 is used as the Pre-ISP to link on Soc, which mainly has two
+ * functions. One is to download the firmware of RK1608, and the other
+ * is to match the extra sensor such as camera and enable sensor by
+ * calling sensor's s_power.
+ * |---|
+ * | Sensor Camera |
+ * |---|
+ * |---||--|
+ * |---||--|
+ * |---\/--|
+ * | Pre-ISP RK1608|
+ * |---|
+ * |---||--|
+ * |---||--|
+ * |---\/--|
+ * | Rockchip Soc |
+ * |---|
+ * Data Transfer As shown above. In RK1608, the data received from the
+ * extra sensor,and it is passed to the Soc through ISP.
+ */
+
+static inline struct rk1608_state *to_state(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct rk1608_state, sd);
+}
+
+/**
+ * rk1608_operation_query - RK1608 last operation state query
+ *
+ * @spi: device from which data will be read
+ * @state: last operation state [out]
+ * Context: can sleep
+ *
+ * It returns zero on success, else a negative error code.
+ */
+static int rk1608_operation_query(struct spi_device *spi, s32 *state)
+{
+ s32 query_cmd = RK1608_CMD_QUERY;
+ struct spi_transfer query_cmd_packet = {
+ .tx_buf = &query_cmd,
+ .len= sizeof(query_cmd),
+ };
+ struct spi_transfer state_packet = {
+ .rx_buf = state,
+ .len= sizeof(*state),
+ };
+ struct spi_message m;
+
+ spi_message_init(&m);
+ spi_message_add_tail(&query_cmd_packet, &m);
+ spi_message_add_tail(&state_packet, &m);
+ spi_sync(spi, &m);
+
+ return ((*state & RK1608_STATE_ID_MASK) == RK1608_STATE_ID) ? 0 : -1;
+}
+
+static int rk1608_write(struct spi_device *spi,
+ s32 addr, const s32 *data, size_t data_len)
+{
+ s32 write_cmd = RK1608_CMD_WRITE;
+ struct spi_transfer write_cmd_packet = {
+ .tx_buf = &write_cmd,
+ .len= sizeof(write_cmd),
+ };
+ struct spi_transfer addr_packet = {
+ .tx_buf = &addr,
+ .len= sizeof(addr),
+ };
+ struct spi_transfer data_packet = {
+ .tx_buf = data,
+ .len= data_len,
+ };
+ struct spi_message m;
+
+ spi_message_init(&m);
+ spi_message_add_tail(&write_cmd_packet, &m);
+ spi_message_add_tail(&addr_packet, &m);
+ spi_message_add_tail(&data_packet, &m);
+ return spi_sync(spi, &m);
+}
+
+/**
+ * rk1608_
