Add GS1662 driver (a SPI video serializer)
Subsystems (2)
Charles-Antoine Couret
maintainerPATCH 1/1 Add GS1662 driver (a SPI video serializer)
Quoted 1 line
From
65b48bf1c77801c210bf93c07bc7f513efdbcbb5Mon Sep 17 00:00:00 2001
From: Charles-Antoine Couret <[email protected]>
Date: Fri, 1 Apr 2016 17:19:26 +0200
Subject: [PATCH] Add GS1662 driver (a SPI video serializer)
--- drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 +- drivers/media/spi/Kconfig | 5 ++ drivers/media/spi/Makefile | 1 + drivers/media/spi/gs1662.c | 128 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 drivers/media/spi/Kconfig create mode 100644 drivers/media/spi/Makefile create mode 100644 drivers/media/spi/gs1662.c
drivers/media/Kconfig +1 -0
@@ -215,5 +215,6 @@ config MEDIA_ATTACH source "drivers/media/i2c/Kconfig" source "drivers/media/tuners/Kconfig" source "drivers/media/dvb-frontends/Kconfig" +source "drivers/media/spi/Kconfig" endif # MEDIA_SUPPORT
drivers/media/Makefile +1 -1
@@ -28,6 +28,6 @@ obj-y += rc/ # Finally, merge the drivers that require the core # -obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ +obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ spi/ obj-$(CONFIG_VIDEO_DEV) += radio/
drivers/media/spi/Kconfig new file +5 -0
@@ -0,0 +1,5 @@ +config VIDEO_GS1662 + tristate "Gennum Serializer 1662 video" + depends on SPI + ---help--- + Enable the GS1662 driver which serializes video streams.
drivers/media/spi/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
drivers/media/spi/gs1662.c new file +128 -0
128 lines not on this page · raw message
Signature · 1 line
2.5.5
Charles-Antoine Couret
maintainerPATCH 1/1 Add GS1662 driver (a SPI video serializer)
Quoted 1 line
From
65b48bf1c77801c210bf93c07bc7f513efdbcbb5Mon Sep 17 00:00:00 2001
From: Charles-Antoine Couret <[email protected]>
Date: Fri, 1 Apr 2016 17:19:26 +0200
Subject: [PATCH] Add GS1662 driver (a SPI video serializer)
--- drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 +- drivers/media/spi/Kconfig | 5 ++ drivers/media/spi/Makefile | 1 + drivers/media/spi/gs1662.c | 128 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 drivers/media/spi/Kconfig create mode 100644 drivers/media/spi/Makefile create mode 100644 drivers/media/spi/gs1662.c
drivers/media/Kconfig +1 -0
@@ -215,5 +215,6 @@ config MEDIA_ATTACH source "drivers/media/i2c/Kconfig" source "drivers/media/tuners/Kconfig" source "drivers/media/dvb-frontends/Kconfig" +source "drivers/media/spi/Kconfig" endif # MEDIA_SUPPORT
drivers/media/Makefile +1 -1
@@ -28,6 +28,6 @@ obj-y += rc/ # Finally, merge the drivers that require the core # -obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ +obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ spi/ obj-$(CONFIG_VIDEO_DEV) += radio/
drivers/media/spi/Kconfig new file +5 -0
@@ -0,0 +1,5 @@ +config VIDEO_GS1662 + tristate "Gennum Serializer 1662 video" + depends on SPI + ---help--- + Enable the GS1662 driver which serializes video streams.
drivers/media/spi/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
drivers/media/spi/gs1662.c new file +128 -0
128 lines not on this page · raw message
Signature · 1 line
2.5.5
Hi Charles-Antoine,
Thank you for the patch.
FIrst of all, we, on the ML, do prefer reading patches as sent by git send-email tool.
It is easier to comment the patch.
Next, you should add a complete description to your commit. Just having an object and a signed-off-by line is not enough. You also have to use the scripts/checkpatch.pl script to verify that everything is ok with it.
Last thing, I can't see anything related to V4L2 in your patch. It is just used to initialize the chip and the spi bus, that's all. Adding a subdev is a start, and some operations if it can do something else than just serializing.
That's it for the moment, when you submit a v2, I (and others) may have some more comments :).
Thanks,
JM
2016-04-01 18:28 GMT+02:00 Charles-Antoine Couret
<[email protected]>:
Quoted 1 line
Charles-Antoine Couret
maintainerQuoted 1 line · Jean-Michel Hautbois
Hi Charles-Antoine,
Hi,
Quoted 2 lines
FIrst of all, we, on the ML, do prefer reading patches as sent by git send-email tool.
Ok, I will configure that.
Quoted 2 lines
Next, you should add a complete description to your commit. Just having an object and a signed-off-by line is not enough.
Oh, I'm sorry, I don't have any idea to explicit more details. I will find something for that.
Quoted 2 lines
You also have to use the
scripts/checkpatch.plscript to verify that everything is ok with it.
I have executed this script before to send it. And it noticed nothing about that.
Quoted 4 lines
Last thing, I can't see anything related to V4L2 in your patch. It is just used to initialize the chip and the spi bus, that's all. Adding a subdev is a start, and some operations if it can do something else than just serializing.
Maybe I'm in the wrong list for that in fact. I didn't know this list was about V4L2 and related topics.
This driver is only to configure the component to manage the video stream in electronic card, it is not to capture video stream via V4L.
I should improve my driver to be configurable by userspace. But maybe I should submit my future patch in another ML.
Thanks for all.
Regards,
Charles-Antoine
Quoted 4 lines
Quoted 2 lines
Next, you should add a complete description to your commit. Just having an object and a signed-off-by line is not enough.
Oh, I'm sorry, I don't have any idea to explicit more details. I will find something for that.
Just get the description from the datasheet as a start ;-).
Quoted 9 lines
Quoted 2 lines
You also have to use the
scripts/checkpatch.plscript to verify that everything is ok with it.I have executed this script before to send it. And it noticed nothing about that.
Quoted 4 lines
Last thing, I can't see anything related to V4L2 in your patch. It is just used to initialize the chip and the spi bus, that's all. Adding a subdev is a start, and some operations if it can do something else than just serializing.
Quoted 4 lines
Maybe I'm in the wrong list for that in fact. I didn't know this list was about V4L2 and related topics.
This driver is only to configure the component to manage the video stream in electronic card, it is not to capture video stream via V4L.I should improve my driver to be configurable by userspace. But maybe I should submit my future patch in another ML.
Well, I am not really sure about that. I added Hans in cc as he may have a better view.
Quoted 1 line
From my point of view, it can be a V4L2 subdev, even a simple one, as
you can configure outputs, etc.
JM
Quoted 19 lines · Jean-Michel Hautbois
Quoted 4 lines
Quoted 2 lines
Next, you should add a complete description to your commit. Just having an object and a signed-off-by line is not enough.
Oh, I'm sorry, I don't have any idea to explicit more details. I will find something for that.
Just get the description from the datasheet as a start ;-).
Quoted 12 lines
Quoted 2 lines
You also have to use the
scripts/checkpatch.plscript to verify that everything is ok with it.I have executed this script before to send it. And it noticed nothing about that.
Quoted 4 lines
Last thing, I can't see anything related to V4L2 in your patch. It is just used to initialize the chip and the spi bus, that's all. Adding a subdev is a start, and some operations if it can do something else than just serializing.
Maybe I'm in the wrong list for that in fact. I didn't know this list was about V4L2 and related topics.
This driver is only to configure the component to manage the video stream in electronic card, it is not to capture video stream via V4L.
Quoted 6 lines
Quoted 1 line
I should improve my driver to be configurable by userspace. But maybe I should submit my future patch in another ML.
Well, I am not really sure about that. I added Hans in cc as he may have a better view.
From my point of view, it can be a V4L2 subdev, even a simple one, as you can configure outputs, etc.
I think the media subsystem is definitely the right place for this.
I would use the cs3308.c driver as a starting point. This is also a minimal driver (and you can remove the code under CONFIG_VIDEO_ADV_DEBUG for your driver), but it uses v4l2_subdev and that makes it ready to be extended in the future, which you will likely need to do eventually.
Regards,
Hans
Charles-Antoine Couret
maintainerQuoted 6 lines · Hans Verkuil
I think the media subsystem is definitely the right place for this.
I would use the
cs3308.cdriver as a starting point. This is also a minimal driver (and you can remove the code underCONFIG_VIDEO_ADV_DEBUGfor your driver), but it usesv4l2_subdevand that makes it ready to be extended in the future, which you will likely need to do eventually.
Yes, I agree with that now. :)
I'm writing a new version of patch to integrate it into media subdev. It's "ready" but I need some tests to validate all behaviours.
Thanks.
Charles-Antoine Couret
Charles-Antoine Couret
maintainerPATCH 1/1 Add GS1662 driver (a SPI video serializer)
In reply to Charles-Antoine CouretQuoted 1 line
From
65b48bf1c77801c210bf93c07bc7f513efdbcbb5Mon Sep 17 00:00:00 2001
From: Charles-Antoine Couret <[email protected]>
Date: Fri, 1 Apr 2016 17:19:26 +0200
Subject: [PATCH] Add GS1662 driver (a SPI video serializer)
--- drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 +- drivers/media/spi/Kconfig | 5 ++ drivers/media/spi/Makefile | 1 + drivers/media/spi/gs1662.c | 128 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 drivers/media/spi/Kconfig create mode 100644 drivers/media/spi/Makefile create mode 100644 drivers/media/spi/gs1662.c
drivers/media/Kconfig +1 -0
@@ -215,5 +215,6 @@ config MEDIA_ATTACH source "drivers/media/i2c/Kconfig" source "drivers/media/tuners/Kconfig" source "drivers/media/dvb-frontends/Kconfig" +source "drivers/media/spi/Kconfig" endif # MEDIA_SUPPORT
drivers/media/Makefile +1 -1
@@ -28,6 +28,6 @@ obj-y += rc/ # Finally, merge the drivers that require the core # -obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ +obj-y += common/ platform/ pci/ usb/ mmc/ firewire/ spi/ obj-$(CONFIG_VIDEO_DEV) += radio/
drivers/media/spi/Kconfig new file +5 -0
@@ -0,0 +1,5 @@ +config VIDEO_GS1662 + tristate "Gennum Serializer 1662 video" + depends on SPI + ---help--- + Enable the GS1662 driver which serializes video streams.
drivers/media/spi/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
drivers/media/spi/gs1662.c new file +128 -0
128 lines not on this page · raw message
Signature · 1 line
2.5.5